Skip to content

[AI-assisted] fix(agents): resolve CLI runtime in preflight + memory-flush gates#86224

Closed
adele-with-a-b wants to merge 5 commits into
openclaw:mainfrom
adele-with-a-b:fix/agent-runner-memory-resolve-cli-runtime-before-gate
Closed

[AI-assisted] fix(agents): resolve CLI runtime in preflight + memory-flush gates#86224
adele-with-a-b wants to merge 5 commits into
openclaw:mainfrom
adele-with-a-b:fix/agent-runner-memory-resolve-cli-runtime-before-gate

Conversation

@adele-with-a-b

@adele-with-a-b adele-with-a-b commented May 24, 2026

Copy link
Copy Markdown
Contributor

Summary

runPreflightCompactionIfNeeded and runMemoryFlushIfNeeded in src/auto-reply/reply/agent-runner-memory.ts gate on isCliProvider(params.followupRun.run.provider, params.cfg). isCliProvider only returns true when the passed provider id is itself a key in agents.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 via models.providers.<provider>.agentRuntime.id (or per-agent / per-model agentRuntime policy), the gate evaluates isCli=false and 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 resolveCliRuntimeExecutionProvider first, then ask isCliProvider about the resolved id. The two preflight/flush gates predate that resolve and were never updated.

Existing pattern

src/auto-reply/reply/agent-runner-execution.ts resolves the CLI execution provider before gating on isCliProvider:

const cliExecutionProvider =
  resolveCliRuntimeExecutionProvider({
    provider,
    cfg: runtimeConfig,
    agentId: params.followupRun.run.agentId,
    modelId: model,
  }) ?? provider;

if (isCliProvider(cliExecutionProvider, runtimeConfig)) {
  // ... CLI dispatch path
}

The fix in this PR uses the same shape at both agent-runner-memory.ts call sites.

Reproduction context

A long-lived agent routed through a CLI runtime via agentRuntime.id accumulates context inside the CLI's own session. The model reports a large promptTokens to OC, which OC stores as entry.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's containsRealConversationMessages filter strips those, returning compacted:false reason:"no real conversation messages". Preflight then throws Preflight 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:core and pnpm tsgo:core:test green.
  • 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 --check clean on both touched files.

The two new tests configure agents.defaults.cliBackends["claude-cli"] plus models.providers.anthropic.agentRuntime.id = "claude-cli" and assert that a followupRun with provider="anthropic" short-circuits both runMemoryFlushIfNeeded (returns the entry without invoking runEmbeddedPiAgent) and runPreflightCompactionIfNeeded (returns the entry without invoking compactEmbeddedPiSession).

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: runPreflightCompactionIfNeeded evaluated isCliProvider("anthropic", cfg) (false because cliBackends keys on claude-cli, not anthropic), 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's containsRealConversationMessages filter then strips delivery-mirror records and returns compacted: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:

  1. Patched ~/.local/bin/openclaw-cli-runtime-preflight-gate-patch.py applied to the brew-installed OC dist (mirrors this PR's source change exactly — same anchored swap of isCliProvider(params.followupRun.run.provider, ...) for resolve-then-isCli).
  2. Restarted the gateway: openclaw gateway restart --safe.
  3. Enabled debug logging via OPENCLAW_LOG_LEVEL=debug so the preflightCompaction check / memoryFlush check verbose lines emit.
  4. Sent one Telegram inbound to a claude-cli-routed agent on the same session-key shape that previously failed.
  5. Sent one Telegram inbound to a different claude-cli-routed agent for cross-agent confirmation.
  6. Inspected the gateway log for the verbose isCli= value at both gates.
  7. Reverted OPENCLAW_LOG_LEVEL=debug and restarted the gateway.

Evidence after fix:

Pre-fix gateway log (six historical failures across ~2h, redacted):

[compaction-diag] end runId=<redacted> sessionKey=<redacted-agent-session-key> diagId=cmp-<redacted>
  trigger=budget provider=anthropic/claude-opus-4-7 attempt=1 maxAttempts=1
  outcome=failed reason=unknown detail=No_API_key_found_for_provider_anthropic_._Auth_store:_<path>
  durationMs=7540
[compaction] skipping — no real conversation messages (sessionKey=<redacted>)
message dispatch completed: ... outcome=error duration=17203ms
  error="Error: Preflight compaction required but failed: no real conversation messages"

Pre-fix verbose memoryFlush check (debug log, before patch was applied; raw provider="anthropic" misclassified):

memoryFlush check: sessionKey=<redacted> tokenCount=51762 contextWindow=1048576 threshold=1024576
  isHeartbeat=false isCli=false
  persistedFresh=true ...

Post-fix verbose memoryFlush check (same env, same agent kind, debug log after patch + this PR's source change):

memoryFlush check: sessionKey=<redacted> tokenCount=48024 contextWindow=1048576 threshold=1024576
  isHeartbeat=false isCli=true
  ...

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 logVerbose line on the run-preflight branch.

Observed result after fix: All three claude-cli-routed agents now resolve isCli=true at both runPreflightCompactionIfNeeded and runMemoryFlushIfNeeded. The Telegram dispatch path no longer hits the OC-transcript compaction path, no Preflight compaction required but failed errors, 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.

@openclaw-barnacle openclaw-barnacle Bot added size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 24, 2026
@clawsweeper

clawsweeper Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed June 24, 2026, 12:22 PM ET / 16:22 UTC.

Summary
The PR resolves effective CLI runtime routing before preflight-compaction and memory-flush gates, with regression tests for model policy, auth-profile, and persisted runtime override cases.

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
Relationship: canonical
Canonical: #86224
Summary: This PR remains the canonical open item for aligning auto-reply memory gates with dispatch-style CLI runtime resolution; nearby merged work only covers subsets or adjacent preflight/memory behavior.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Rebase onto current main and repair followupUsesCliRuntime instead of the old two call sites.
  • Rerun the focused memory-gate, dispatch, follow-up runner, and tsgo validation after the rebase.

Risk before merge

  • [P1] The PR head is conflicting and edits the older duplicated call-site shape; a manual conflict resolution that misses current main's followupUsesCliRuntime helper could leave configured model-policy and auth-order CLI routing broken in the memory gates.
  • [P1] These gates decide whether embedded compaction or memory flush mutates OpenClaw transcript/session state for a turn, so resolver divergence can still surface as auth-provider failures or session-state drift for CLI-owned conversations.

Maintainer options:

  1. Repair the shared helper before merge (recommended)
    Rebase onto current main and update followupUsesCliRuntime to use the same provider-compatible override plus auth-profile-aware CLI runtime resolution that dispatch uses.
  2. Pause for owner boundary review
    If maintainers want a different canonical helper boundary, pause the PR because the current conflicting head cannot be merged safely as-is.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Rebase onto current main and replace followupUsesCliRuntime with dispatch-equivalent effective CLI runtime resolution, preserving compatible and stale session override behavior, model-policy routing, auth-profile pinning, auth.order CLI routing, and pi/openclaw cases with focused tests.

Next step before merge

  • [P2] A narrow automated repair can rebase the PR and move the resolver parity into the current shared helper with focused routing tests.

Security
Cleared: The diff changes TypeScript runtime classification logic and tests only, with no dependency, workflow, credential, package, download, or supply-chain surface added.

Review findings

  • [P2] Move resolver parity into the shared memory gate helper — src/auto-reply/reply/agent-runner-memory.ts:731-745
Review details

Best 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:

  • [P2] Move resolver parity into the shared memory gate helper — src/auto-reply/reply/agent-runner-memory.ts:731-745
    Current main routes both preflight and flush through followupUsesCliRuntime, but this branch still adds resolver blocks at the old two call sites and GitHub reports the head as conflicting. Rebase and update the helper instead, or the conflict resolution can leave configured model-policy/auth-order CLI routing unfixed in the current memory gates.
    Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 3217165be770.

Label changes

Label justifications:

  • P1: The bug can break live agent replies by running embedded maintenance against sessions whose actual runtime and auth live in a CLI backend.
  • merge-risk: 🚨 auth-provider: The diff changes how auth-profile and provider routing are classified before choosing embedded maintenance versus CLI-owned execution.
  • merge-risk: 🚨 session-state: The diff controls whether preflight compaction and memory flush mutate OpenClaw transcript/session state for CLI-owned conversations.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (logs): The PR body includes redacted live gateway logs showing the pre-fix failure, post-fix isCli=true classification, and successful replies on a real claude-cli-routed setup.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted live gateway logs showing the pre-fix failure, post-fix isCli=true classification, and successful replies on a real claude-cli-routed setup.
Evidence reviewed

PR surface:

Source +71, Tests +508. Total +579 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 74 3 +71
Tests 1 508 0 +508
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 582 3 +579

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-memory.test.ts.
  • [P1] node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-execution.test.ts src/auto-reply/reply/followup-runner.test.ts.
  • [P1] pnpm tsgo:core.
  • [P1] pnpm tsgo:core:test.
  • [P1] git diff --check.

What I checked:

  • Root policy read: Root AGENTS.md was read completely; its ClawSweeper PR review policy applies because this change touches provider routing, auth, session state, and auto-reply memory gates. (AGENTS.md:1, 3217165be770)
  • Scoped policy read: The relevant scoped agents guide was read; it mainly affects test/import-performance expectations for agent-runtime adjacent code. (src/agents/AGENTS.md:1, 3217165be770)
  • Current-main memory gate gap: Current main funnels both preflight and flush through followupUsesCliRuntime, but that helper only checks direct CLI providers and compatible persisted runtime pins; it does not use the dispatch resolver for configured model policy or auth-profile/order CLI routing. (src/auto-reply/reply/agent-runner-memory.ts:263, 3217165be770)
  • Dispatch contract: The dispatch path resolves a provider-compatible session override, then falls through to resolveCliRuntimeExecutionProvider with the selected auth profile before checking isCliProvider. (src/auto-reply/reply/agent-runner-execution.ts:2208, 3217165be770)
  • Follow-up runner sibling contract: The follow-up runner uses the same dispatch-style chain of session runtime override, auth-profile-aware CLI runtime resolution, and isCliProvider classification. (src/auto-reply/reply/followup-runner.ts:904, 3217165be770)
  • Resolver covers missing cases: resolveCliRuntimeExecutionProvider handles configured runtime policy and auth-profile/order derived CLI runtime resolution, which is the missing branch in current-main followupUsesCliRuntime. (src/agents/model-runtime-aliases.ts:232, 3217165be770)

Likely related people:

  • vincentkoc: Current-main blame points to this author for followupUsesCliRuntime and dispatch resolver code, and merged PR history shows a follow-up memory-path commit for compatible CLI session pins. (role: recent area contributor; confidence: high; commits: 303b2f794f6c, e9720c27fa69; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/agent-runner-execution.ts)
  • yu-xin-c: Authored the merged provider-compatible CLI session runtime pin work that changed the dispatch and memory-path invariants this PR must preserve. (role: adjacent feature contributor; confidence: high; commits: 1917cc4b8591, 9974641d1ee1; files: src/auto-reply/reply/agent-runner-execution.ts, src/auto-reply/reply/followup-runner.ts, src/auto-reply/reply/agent-runner-memory.ts)
  • potterdigital: Authored the merged runtime-policy resolver fix and the closed narrower PR that reported the same memory-gate CLI runtime classification gap. (role: adjacent runtime-policy contributor; confidence: medium; commits: efd88dc00d8d; files: src/agents/model-runtime-policy.ts, src/agents/model-runtime-aliases.ts, src/auto-reply/reply/agent-runner-memory.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. labels May 24, 2026
@clawsweeper

clawsweeper Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

✨ Hatched: 🌱 uncommon Mossy Proofling

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🌱 uncommon.
Trait: collects tiny proofs.
Image traits: location diff observatory; accessory shell-shaped keyboard; palette charcoal, cyan, and signal green; mood celebratory; pose nestled inside a glowing shell; shell soft velvet shell; lighting subtle sparkle highlights; background little resolved-comment flags.
Share on X: post this hatch
Copy: My PR egg hatched a 🌱 uncommon Mossy Proofling in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

adele-with-a-b added a commit to adele-with-a-b/openclaw that referenced this pull request May 25, 2026
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.
@openclaw-barnacle openclaw-barnacle Bot added size: M proof: supplied External PR includes structured after-fix real behavior proof. and removed size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 25, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 25, 2026
@potterdigital

potterdigital commented May 25, 2026

Copy link
Copy Markdown
Contributor

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 resolveCliRuntimeExecutionProvider-based gate as a bundle patch on a 24/7 production gateway (claude-cli backend, Claude subscription OAuth) since 2026-05-24. Composition differs from yours — mine is isCliProvider(p) || resolveCliRuntimeExecutionProvider(...) !== void 0, yours is isCliProvider(resolveCliRuntimeExecutionProvider(...) ?? p) — but they behave the same at the gate boundary. No recurrence of the 1.3M-context 404 cascade the original bug was tracking.

On the persisted-override P2: My prod failure path was the per-model agentRuntime.id route, so I never exercised the session-persisted agentRuntimeOverride branch clawsweeper flagged. Can't add evidence on that branch from my config — flagging in case it's useful that my prod validation doesn't cover it.

Adjacent merge context: #85970 (merged yesterday) patched modelEntryMatchKind in model-runtime-policy.ts for empty-provider session entries. That policy table is what resolveCliRuntimeExecutionProvider consults via resolveConfiguredRuntime — without #85970, the resolver call here could silently miss per-model agentRuntime.id entries on empty-provider sessions. Different files (src/agents/ vs src/auto-reply/), so no merge conflict, but the two fixes ride together.

@clawsweeper

@adele-with-a-b

adele-with-a-b commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

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 agentRuntimeOverride session-persisted branch. My config doesn't exercise that path either (I hit the bug via auth.profiles mapping, not per-model runtime overrides). I'll add a unit test covering the persisted-override case before requesting final review so clawsweeper's P2 has coverage regardless of prod configs.

Re #85970 adjacency: agreed, these ride together. The empty-provider session entries that #85970 fixes would have caused resolveConfiguredRuntime to return undefined on lookup, which means resolveCliRuntimeExecutionProvider falls through to the ?? p passthrough — effectively reverting to the broken isCliProvider(configured_provider) behavior for those sessions. Good that it merged first. I'll add a note in the PR description calling out the dependency.

@adele-with-a-b
adele-with-a-b force-pushed the fix/agent-runner-memory-resolve-cli-runtime-before-gate branch from 62b2fb1 to 2d4f556 Compare May 25, 2026 15:41
adele-with-a-b added a commit to adele-with-a-b/openclaw that referenced this pull request May 25, 2026
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.
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 25, 2026
adele-with-a-b added a commit to adele-with-a-b/openclaw that referenced this pull request May 25, 2026
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.
@adele-with-a-b

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Pushed 26befa57 addressing the P2 — both memory gates now apply the same three-tier sessionRuntimeOverride precedence as the dispatch path:

  1. agentRuntimeOverride === "pi" → use canonical provider (forces embedded)
  2. agentRuntimeOverride is a CLI alias for the provider → use the override directly
  3. Fall through to resolveCliRuntimeExecutionProvider (the auth-profile-aware resolver from the previous commit)

Plus 3 regression tests covering CLI-override-skips-flush, CLI-override-skips-preflight, and pi-override-runs-flush-despite-CLI-auth-order. pnpm tsgo:core + tsgo:core:test + oxfmt --check all clean; vitest run agent-runner-memory.test.ts is 32 passing.

Also rebased onto current main to clear the unrelated check-dependencies failure on the prior head (extensions/acpx + canvas + diffs unused-files were a transient main-side issue, gone post-rebase).

@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 25, 2026
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.
@adele-with-a-b
adele-with-a-b force-pushed the fix/agent-runner-memory-resolve-cli-runtime-before-gate branch from 872f6d0 to c0f7292 Compare June 8, 2026 16:04
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 8, 2026
@adele-with-a-b

Copy link
Copy Markdown
Contributor Author

Rebased onto current upstream/main (c0f7292) and addressed the P1 gate↔dispatch divergence.

Root cause of the P1: the preflight/memory-flush gates imported resolveSessionRuntimeOverrideForProvider from session-runtime-compat.ts (which collapses "pi""openclaw" and returns "openclaw"), while dispatch imports a same-named but different function from agent-runner-execution.ts (which returns undefined for "pi"). The gate then papered over this with a === "pi" short-circuit forcing canonical/embedded, which dispatch has no equivalent for — so a "pi"-pinned session on a CLI-routed config ran embedded maintenance in the gate but went CLI in dispatch.

Fix (maintainer option 1 — align the gate with dispatch): removed the gate's "pi" short-circuit at both sites so the gate falls through to the same config-driven resolveCliRuntimeExecutionProvider(...) resolution dispatch uses. The gate now skips embedded maintenance exactly when dispatch routes to CLI, and runs it when dispatch runs embedded — verified in both directions.

Tests: flipped the test that codified the divergence (pi + CLI-preferring auth.order now SKIPS, matching dispatch) and added the inverse regression (pi + no CLI route still RUNS embedded). All green on the rebased tree: agent-runner-memory.test.ts 50/50, agent-runner-execution.test.ts + followup-runner.test.ts 254/254, tsgo:core + tsgo:core:test clean.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 8, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 21, 2026
@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 12, 2026
@openclaw-barnacle

Copy link
Copy Markdown

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: L stale Marked as stale due to inactivity status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants