Skip to content

fix(agents): claude-cli lifecycle cleanup — loopback fail-loud, exit-0 failover, bounded reseed, image sweep, one prepare cleanup owner#99159

Merged
obviyus merged 4 commits into
mainfrom
codex/fix-cli-lifecycle-cleanup
Jul 2, 2026
Merged

fix(agents): claude-cli lifecycle cleanup — loopback fail-loud, exit-0 failover, bounded reseed, image sweep, one prepare cleanup owner#99159
obviyus merged 4 commits into
mainfrom
codex/fix-cli-lifecycle-cleanup

Conversation

@obviyus

@obviyus obviyus commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #98946

What Problem This Solves

Six lifecycle/cleanup defects in the claude-cli backend from the reliability audit — the tail of the same audit that produced #98933, #98934, #98942, and #98983:

  1. A loopback MCP server startup failure was warn-and-continue: runs proceeded with bundled MCP enabled but no openclaw server, so agents ran whole turns with no messaging/gateway tools, and in message_tool_only delivery the visible reply was silently lost.
  2. ~30 lines of dead "drain aborted turn" state in the live session (drainingAbortedTurn/drainTimer) — never activated since their introducing commit.
  3. A live Claude process exiting 0 mid-turn produced a plain Error, bypassing failover entirely, while the identical one-shot outcome rotated the fallback chain via FailoverError("empty_response").
  4. Transcripts over 5MB silently disabled all reseed history (return [], parse errors swallowed) — and since claude-cli owns native compaction, long sessions can cross 5MB, after which any binding invalidation reseeded with zero context, unlogged.
  5. CLI prompt image files were written content-addressed with a deliberate no-op cleanup, but no sweeper existed anywhere — unbounded disk growth per distinct inbound image.
  6. Temp MCP-config and skills-plugin dirs leaked whenever preparation threw between backend prep and the two guarded regions (auth-epoch read, transcript probes, reference-path/sandbox/reseed stages were all unguarded).

Why This Change Was Made

Bundled-MCP runs now fail loudly at prepare when the loopback server cannot start or publishes no runtime (no legitimate loopback-less bundleMcp mode exists — side questions and disableTools already skip bundled MCP). The dead drain machinery is deleted outright. Quiet exit-0/no-result live turns classify as FailoverError("empty_response"), attaching the cli_unknown_empty_failure retry code only when the turn observed no stdout activity — mirroring the one-shot path's gates rather than inventing looser ones. Oversized transcripts reseed from a bounded 5MB tail (first partial JSONL line dropped, session header preserved) and warn once on truncation or malformed lines. writeCliImages opportunistically sweeps files older than 7 days once per root per process (ENOENT-tolerant, debug-only on errors; the content-addressed path-stability contract is preserved — expired files are simply rewritten if needed again). Prepare cleanup now has a single try/catch owner covering every prepared resource from creation until ownership passes to the returned context, absorbing the two previous partial guards; the beforeExecution staging thunk from #98983 threads through the restructure unchanged. Fixes 2 and 6 together are LOC-negative in source (+456/−458).

User Impact

Loopback failures surface as actionable errors instead of silently tool-less agents and lost replies. Mid-turn clean exits now take model fallback and recovery retries. Long-running sessions keep (bounded) reseed context instead of silently losing all of it past 5MB. Image-heavy deployments stop growing disk without bound, and failed preparations stop leaking temp directories.

Evidence

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XL maintainer Maintainer-authored PR labels Jul 2, 2026
@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 2, 2026, 12:30 PM ET / 16:30 UTC.

Summary
The branch changes claude-cli prepare cleanup ownership, bundled MCP loopback failure handling, live exit-0 failover classification, oversized transcript reseeding, and stale CLI image cache sweeping with focused regression tests.

PR surface: Source +175, Tests +400. Total +575 across 9 files.

Reproducibility: yes. at source level: current main still warn-continues on loopback startup failure, drops oversized transcript history, uses no-op image cleanup, and treats live exit 0 as a plain Error. I did not run a live claude-cli repro in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Fail-Closed Startup Paths: 1 changed. Bundled MCP loopback startup now throws instead of warning, which changes what existing users see when the local loopback cannot start.
  • Persistent Transcript Size Handling: 1 reader changed. Oversized session transcripts now feed a bounded tail into history/reseed readers instead of being dropped, so session-state behavior changes before merge.

Stored data model
Persistent data-model change detected: serialized state: src/agents/cli-runner.helpers.test.ts, serialized state: src/agents/cli-runner/session-history.test.ts, serialized state: src/agents/cli-runner/session-history.ts, unknown-data-model-change: src/agents/cli-runner/session-history.test.ts, unknown-data-model-change: src/agents/cli-runner/session-history.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Add redacted real behavior proof, such as terminal output or logs from a claude-cli run showing one changed lifecycle path after the fix.
  • Make the PR body explicitly call out maintainer acceptance of the fail-loud bundled MCP behavior and seven-day image sweep semantics.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides tests, CI, Testbox, and autoreview output, but no redacted terminal/log/recording/artifact showing the changed claude-cli behavior after the fix in a real setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The loopback change intentionally converts an existing warn-and-continue path into a hard prepare failure for bundled MCP, so existing users with broken loopback startup will see runs fail closed instead of running tool-less.
  • [P1] The oversized transcript change can now reseed from a bounded raw tail instead of returning no history, which improves continuity but changes session-state carried into fresh Claude CLI sessions.
  • [P1] The image/cache and prepare cleanup changes delete stale workspace/tmp files and temp directories; the implementation is path-scoped, but maintainers should confirm the seven-day sweep and cleanup timing are acceptable product behavior.
  • [P1] The PR body provides focused tests and CI/Testbox output, but no after-fix real claude-cli behavior proof from a real setup.

Maintainer options:

  1. Confirm Fail-Closed Semantics Before Merge (recommended)
    Have a maintainer explicitly accept that bundled MCP startup failure should hard-fail existing claude-cli runs and add real runtime proof for that path before landing.
  2. Accept The Intentional Runtime Behavior Change
    Maintainers can choose to land as-is if they decide tool-less bundled-MCP execution is unsupported and the regression tests are enough proof for this batch.
  3. Split The Lifecycle Batch
    If the broad batch remains hard to review, split the branch into narrower PRs for loopback failure, failover, transcript reseed, image sweep, and prepare cleanup while preserving the focused tests.

Next step before merge

  • [P1] The remaining action is maintainer review of compatibility-sensitive fail-closed/session semantics plus contributor real behavior proof, not an automated repair task.

Security
Cleared: The diff touches process/runtime cleanup and local file deletion paths, but I found no new dependency, workflow, permission, secret-handling, or broad supply-chain concern beyond the scoped runtime behavior risks.

Review details

Best possible solution:

Land only after maintainer confirmation of the fail-loud/upgrade semantics, fresh real behavior proof for the claude-cli lifecycle path, and normal PR validation; keep #98946 open until a fix merges.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: current main still warn-continues on loopback startup failure, drops oversized transcript history, uses no-op image cleanup, and treats live exit 0 as a plain Error. I did not run a live claude-cli repro in this read-only review.

Is this the best way to solve the issue?

Yes with maintainer confirmation: the patch targets the owner paths and adds focused regression coverage, but fail-loud startup and reseed/cleanup semantics are compatibility-sensitive and need real behavior proof before merge.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a normal-priority claude-cli reliability fix with limited surface but meaningful agent runtime impact.
  • add merge-risk: 🚨 compatibility: The PR changes an existing warn-and-continue bundled MCP startup path into a hard prepare failure for existing setups with loopback startup problems.
  • add merge-risk: 🚨 session-state: The PR changes how oversized persisted transcripts are read into history and reseed prompts, affecting carried session context.
  • add merge-risk: 🚨 availability: Fail-loud loopback startup can make affected claude-cli runs stop at prepare time instead of completing in degraded mode.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides tests, CI, Testbox, and autoreview output, but no redacted terminal/log/recording/artifact showing the changed claude-cli behavior after the fix in a real setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a normal-priority claude-cli reliability fix with limited surface but meaningful agent runtime impact.
  • merge-risk: 🚨 compatibility: The PR changes an existing warn-and-continue bundled MCP startup path into a hard prepare failure for existing setups with loopback startup problems.
  • merge-risk: 🚨 session-state: The PR changes how oversized persisted transcripts are read into history and reseed prompts, affecting carried session context.
  • merge-risk: 🚨 availability: Fail-loud loopback startup can make affected claude-cli runs stop at prepare time instead of completing in degraded mode.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides tests, CI, Testbox, and autoreview output, but no redacted terminal/log/recording/artifact showing the changed claude-cli behavior after the fix in a real setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +175, Tests +400. Total +575 across 9 files.

View PR surface stats
Area Files Added Removed Net
Source 5 646 471 +175
Tests 4 427 27 +400
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 9 1073 498 +575

What I checked:

Likely related people:

  • @vincentkoc: Current-main blame for the loopback warn-continue path, live exit handling, drain state, and session-history size guard maps to a grafted July 1 snapshot authored by this account, and release-tag history also shows recent anthropic live smoke work. (role: recent area contributor; confidence: medium; commits: 4d7d34e6a5f8, e085fa1a3ffd; files: src/agents/cli-runner/prepare.ts, src/agents/cli-runner/claude-live-session.ts, src/agents/cli-runner/session-history.ts)
  • @steipete: History search ties earlier loopback MCP startup behavior and CLI image/helper extraction to commits by this account, which makes them relevant for startup and cleanup ownership questions. (role: adjacent bundle MCP and CLI helper contributor; confidence: medium; commits: 19d8069aea06, d0562a873fad; files: src/agents/cli-runner/prepare.ts, src/agents/cli-runner/helpers.ts, src/agents/cli-runner.helpers.test.ts)
  • @obviyus: Beyond authoring this branch, this account has recent merged prepare-adjacent CLI work in the same backend lifecycle area and authored the related reliability-audit PR series referenced by this PR. (role: recent adjacent contributor; confidence: medium; commits: 5fc9b20d2492, b60803dfaae3, d02d8b1ea5c9; files: src/agents/cli-runner/prepare.ts, src/agents/cli-runner/prepare.test.ts, src/agents/cli-runner/claude-live-session.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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jul 2, 2026
@obviyus
obviyus merged commit 0633876 into main Jul 2, 2026
170 of 175 checks passed
@obviyus
obviyus deleted the codex/fix-cli-lifecycle-cleanup branch July 2, 2026 16:31
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jul 2, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 2, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling maintainer Maintainer-authored PR merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XL status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

1 participant

@obviyus