[AI-assisted] fix(agents): resolve CLI runtime in preflight + memory-flush gates#86224
Conversation
|
Codex review: needs changes before merge. Reviewed June 24, 2026, 12:22 PM ET / 16:22 UTC. Summary PR surface: Source +71, Tests +508. Total +579 across 2 files. Reproducibility: yes. Current main's memory gate helper does not call the resolver that dispatch uses for configured model policy and auth-profile CLI routing, and the PR body provides redacted live logs for the same failure shape. Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Rebase the PR and implement dispatch-equivalent CLI runtime classification once in current main's followupUsesCliRuntime helper, preserving provider-compatible override, model-policy, auth-profile, and pi/openclaw regression coverage. Do we have a high-confidence way to reproduce the issue? Yes. Current main's memory gate helper does not call the resolver that dispatch uses for configured model policy and auth-profile CLI routing, and the PR body provides redacted live logs for the same failure shape. Is this the best way to solve the issue? No for the current branch as-is. The resolver-based direction is the right fix, but it needs to move into current main's shared followupUsesCliRuntime helper after rebasing. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3217165be770. Label changesLabel justifications:
Evidence reviewedPR surface: Source +71, Tests +508. Total +579 across 2 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
ClawSweeper PR egg ✨ Hatched: 🌱 uncommon Mossy Proofling Hatch commandComment Hatchability rules:
Rarity: 🌱 uncommon. What is this egg doing here?
|
Mirrors the dispatch path's call to resolveCliRuntimeExecutionProvider, which uses the run's selected auth profile (via resolveRunAuthProfile) to handle the case where auth.order prefers a CLI profile but the session is pinned to a direct-API auth profile. Without authProfileId, the memory gates would resolve "claude-cli" while the dispatch path keeps the direct embedded route — the gates then incorrectly skip preflight compaction / memory flush even when dispatch runs embedded. Adds two regression tests configuring auth.order with a CLI profile first while the run pins authProfileId to the direct-API profile, asserting both gates proceed. Addresses Codex review on PR openclaw#86224.
|
Drive-by note from a parallel investigation — I had #86361 open targeting the per-model runtime path before this PR appeared; closed it because your resolver-based shape is the right one. Production validation: I've been running an equivalent On the persisted-override P2: My prod failure path was the per-model Adjacent merge context: #85970 (merged yesterday) patched |
|
Thanks for the production validation — 24h on a real gateway with the equivalent gate logic is exactly the kind of signal that's hard to get from tests alone. Good call flagging the Re #85970 adjacency: agreed, these ride together. The empty-provider session entries that #85970 fixes would have caused |
62b2fb1 to
2d4f556
Compare
Mirrors the dispatch path's call to resolveCliRuntimeExecutionProvider, which uses the run's selected auth profile (via resolveRunAuthProfile) to handle the case where auth.order prefers a CLI profile but the session is pinned to a direct-API auth profile. Without authProfileId, the memory gates would resolve "claude-cli" while the dispatch path keeps the direct embedded route — the gates then incorrectly skip preflight compaction / memory flush even when dispatch runs embedded. Adds two regression tests configuring auth.order with a CLI profile first while the run pins authProfileId to the direct-API profile, asserting both gates proceed. Addresses Codex review on PR openclaw#86224.
Mirrors the three-tier precedence from runReplyAgent's dispatch path: 1. agentRuntimeOverride === "pi" → use canonical provider (forces embedded) 2. agentRuntimeOverride is a CLI alias for provider → use the override 3. Fall through to resolveCliRuntimeExecutionProvider (the auth-profile-aware resolver already wired into both gates) Without (1) and (2), sessions with persisted agentRuntimeOverride: "claude-cli" were being classified as non-CLI by the memory gates while dispatch routed them through CLI — the same embedded transcript compaction failure mode that this PR's earlier commit fixed for auth-profile mappings. Adds regression tests for the CLI-override case at both gates. Addresses ClawSweeper P2 review comment on PR openclaw#86224.
|
@clawsweeper re-review Pushed
Plus 3 regression tests covering CLI-override-skips-flush, CLI-override-skips-preflight, and pi-override-runs-flush-despite-CLI-auth-order. Also rebased onto current |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
runPreflightCompactionIfNeeded and runMemoryFlushIfNeeded both check isCliProvider on the canonical provider id stored on the followup run. isCliProvider only reports true when that id is itself a key in agents.defaults.cliBackends, so an agent configured with a primary model on a canonical provider (e.g. "anthropic") that resolves through a CLI runtime via models.providers.<provider>.agentRuntime.id evaluates isCli=false. Preflight then runs as if the agent were embedded. The runtime dispatch path in runReplyAgent already resolves the runtime provider via resolveCliRuntimeExecutionProvider before checking isCliProvider (see src/auto-reply/reply/agent-runner-execution.ts around the isCliProvider gate). The preflight + memory-flush gates predate that resolve and were never updated. Mirror the established shape at both call sites so a CLI-routed agent skips the embedded-only preflight transcript compaction. Adds two regression tests asserting that a config with agents.defaults.cliBackends["claude-cli"] plus models.providers.anthropic.agentRuntime.id = "claude-cli" causes both runMemoryFlushIfNeeded and runPreflightCompactionIfNeeded to short-circuit when the followupRun carries provider="anthropic".
Mirrors the dispatch path's call to resolveCliRuntimeExecutionProvider, which uses the run's selected auth profile (via resolveRunAuthProfile) to handle the case where auth.order prefers a CLI profile but the session is pinned to a direct-API auth profile. Without authProfileId, the memory gates would resolve "claude-cli" while the dispatch path keeps the direct embedded route — the gates then incorrectly skip preflight compaction / memory flush even when dispatch runs embedded. Adds two regression tests configuring auth.order with a CLI profile first while the run pins authProfileId to the direct-API profile, asserting both gates proceed. Addresses Codex review on PR openclaw#86224.
Mirrors the three-tier precedence from runReplyAgent's dispatch path: 1. agentRuntimeOverride === "pi" → use canonical provider (forces embedded) 2. agentRuntimeOverride is a CLI alias for provider → use the override 3. Fall through to resolveCliRuntimeExecutionProvider (the auth-profile-aware resolver already wired into both gates) Without (1) and (2), sessions with persisted agentRuntimeOverride: "claude-cli" were being classified as non-CLI by the memory gates while dispatch routed them through CLI — the same embedded transcript compaction failure mode that this PR's earlier commit fixed for auth-profile mappings. Adds regression tests for the CLI-override case at both gates. Addresses ClawSweeper P2 review comment on PR openclaw#86224.
…lution The preflight-compaction and memory-flush gates short-circuited on agentRuntimeOverride === "pi" by forcing canonical/embedded routing, but runReplyAgent's dispatch path (agent-runner-execution.ts:2056, followup-runner.ts:726) has no such "pi" branch: it resolves embedded-vs-CLI purely from a provider-compatible persisted override plus the auth-profile-aware resolver. For a session pinned to "pi" on a CLI-routed config, the gate ran embedded maintenance while dispatch ran CLI -- out of sync. Option A: make the gate match dispatch. Remove the gate's "pi" short-circuit so it falls through to the same config-driven resolution dispatch uses. Now the gate skips embedded maintenance exactly when dispatch routes to CLI, and runs it when dispatch runs embedded -- in both directions. Flip the test that codified the divergence (pi override + CLI-preferring auth.order now SKIPS, matching dispatch) and add the inverse regression (pi override + no CLI route still RUNS embedded), proving gate and dispatch agree both ways.
872f6d0 to
c0f7292
Compare
|
Rebased onto current Root cause of the P1: the preflight/memory-flush gates imported Fix (maintainer option 1 — align the gate with dispatch): removed the gate's Tests: flipped the test that codified the divergence (pi + CLI-preferring @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
This pull request has been automatically marked as stale due to inactivity. |
|
Closing due to inactivity. |
Summary
runPreflightCompactionIfNeededandrunMemoryFlushIfNeededinsrc/auto-reply/reply/agent-runner-memory.tsgate onisCliProvider(params.followupRun.run.provider, params.cfg).isCliProvideronly returns true when the passed provider id is itself a key inagents.defaults.cliBackends. For an agent configured with a primary model on a canonical provider (e.g.anthropic/claude-opus-4-7) that resolves through a CLI runtime viamodels.providers.<provider>.agentRuntime.id(or per-agent / per-modelagentRuntimepolicy), the gate evaluatesisCli=falseand preflight runs as if the agent were embedded — even though the conversation transcript and tokens live in the CLI runtime's own session, not in the OC transcript file.This PR mirrors the resolution shape already used by the runtime dispatch path: resolve the CLI execution provider via
resolveCliRuntimeExecutionProviderfirst, then askisCliProviderabout the resolved id. The two preflight/flush gates predate that resolve and were never updated.Existing pattern
src/auto-reply/reply/agent-runner-execution.tsresolves the CLI execution provider before gating onisCliProvider:The fix in this PR uses the same shape at both
agent-runner-memory.tscall sites.Reproduction context
A long-lived agent routed through a CLI runtime via
agentRuntime.idaccumulates context inside the CLI's own session. The model reports a largepromptTokensto OC, which OC stores asentry.totalTokens. Once that crosses the preflight threshold (contextWindow - reserveTokensFloor - softThresholdTokens), preflight fires and tries to compact the OC-side transcript file. For agents whose real conversation lives outside the OC transcript file (the CLI runtime's own session), OC's transcript file only contains delivery-mirror records, and the compaction safeguard'scontainsRealConversationMessagesfilter strips those, returningcompacted:false reason:"no real conversation messages". Preflight then throwsPreflight compaction required but failed: no real conversation messages, surfacing as a generic dispatch error.The runtime dispatch path doesn't hit this because it had already been migrated to
resolveCliRuntimeExecutionProvider. The preflight + flush gates had not.Verification
pnpm tsgo:coreandpnpm tsgo:core:testgreen.pnpm exec vitest run src/auto-reply/reply/agent-runner-memory.test.ts: 26 passed (existing 24 + 2 new). Both new tests fail without the source fix and pass with it.pnpm exec oxfmt --checkclean on both touched files.The two new tests configure
agents.defaults.cliBackends["claude-cli"]plusmodels.providers.anthropic.agentRuntime.id = "claude-cli"and assert that afollowupRunwithprovider="anthropic"short-circuits bothrunMemoryFlushIfNeeded(returns the entry without invokingrunEmbeddedPiAgent) andrunPreflightCompactionIfNeeded(returns the entry without invokingcompactEmbeddedPiSession).Real behavior proof
Behavior addressed: Telegram dispatch on a long-lived claude-cli-routed Anthropic agent surfaced "Something went wrong while processing your request." The underlying error in the gateway log was
Preflight compaction required but failed: no real conversation messages. Root cause:runPreflightCompactionIfNeededevaluatedisCliProvider("anthropic", cfg)(false because cliBackends keys onclaude-cli, notanthropic), so preflight ran on agents whose real conversation lives in claude-cli's session JSONL — not in OC's transcript file (which holds only delivery-mirror records for that surface). The compactor'scontainsRealConversationMessagesfilter then strips delivery-mirror records and returnscompacted:false, so preflight throws.Real environment tested: A live OpenClaw stack on macOS 14 with three claude-cli-routed Anthropic agents wired to Telegram. Anthropic configured as primary; routes through
auth.profiles["anthropic:claude-cli"]so dispatch resolves to the claude-cli runtime. Six historical failures observed today across one of the three agents over ~2h before manual recovery (delete the broken session-store binding + archive the all-delivery-mirror transcript file).Exact steps or command run after this patch:
~/.local/bin/openclaw-cli-runtime-preflight-gate-patch.pyapplied to the brew-installed OC dist (mirrors this PR's source change exactly — same anchored swap ofisCliProvider(params.followupRun.run.provider, ...)for resolve-then-isCli).openclaw gateway restart --safe.OPENCLAW_LOG_LEVEL=debugso thepreflightCompaction check/memoryFlush checkverbose lines emit.isCli=value at both gates.OPENCLAW_LOG_LEVEL=debugand restarted the gateway.Evidence after fix:
Pre-fix gateway log (six historical failures across ~2h, redacted):
Pre-fix verbose
memoryFlush check(debug log, before patch was applied; rawprovider="anthropic"misclassified):Post-fix verbose
memoryFlush check(same env, same agent kind, debug log after patch + this PR's source change):Post-fix preflight short-circuits at the gate before its own verbose log emits — the early-return at the resolved-runtime-isCli check fires before the
logVerboseline on the run-preflight branch.Observed result after fix: All three claude-cli-routed agents now resolve
isCli=trueat bothrunPreflightCompactionIfNeededandrunMemoryFlushIfNeeded. The Telegram dispatch path no longer hits the OC-transcript compaction path, noPreflight compaction required but failederrors, no user-visible "Something went wrong" message. Six previously-failing inbounds no longer reproduce on the affected session-key shape (verified by sending three test messages across two agents post-patch — all returned the agent's reply normally).What was not tested: The auth-profile-pinned-to-direct-API-while-auth-order-prefers-CLI configuration was added in the follow-up commit (per Codex review) and is covered by the new regression tests but was not exercised live — the live test stack uses the simpler
auth.profiles["anthropic:claude-cli"]mapping rather than a direct-API pin override. No Crabbox / Testbox run; the live verification was on a personal stack with the three claude-cli-routed agents reproducing the exact original failure shape.