Skip to content

fix(reply): don't wedge preflight compaction on benign skip reasons#86663

Closed
ottoclaw wants to merge 1 commit into
openclaw:mainfrom
ottoclaw:garrett/patches
Closed

fix(reply): don't wedge preflight compaction on benign skip reasons#86663
ottoclaw wants to merge 1 commit into
openclaw:mainfrom
ottoclaw:garrett/patches

Conversation

@ottoclaw

Copy link
Copy Markdown

Summary

runPreflightCompactionIfNeeded throws unconditionally on any non-ok/non-compacted embedded compaction result. For per-model claude-cli runtime sessions (the model's provider stays anthropic via a per-model agentRuntime.id pin), the embedded compaction legitimately returns a benign skip — e.g. no real conversation messages (the CLI session's OpenClaw-side transcript is empty) or Already compacted — and the throw then fails every subsequent dispatch on that session until it is reset. The user-visible symptom is a "context limit"-style error even though real resident context is small.

Same class as #84878 (which skipped the harness compaction preflight for CLI runtimes), but on the still-unguarded preflight gate in runPreflightCompactionIfNeeded.

Fix: reuse the existing isCompactionSkipReason classifier (already used on the manual /compact path) at the throw site, returning the session entry for benign skips (nothing to compact | below threshold | already compacted | no real conversation messages) instead of throwing.

  • src/auto-reply/reply/commands-compact.ts — export isCompactionSkipReason
  • src/auto-reply/reply/agent-runner-memory.ts — graceful skip before the throw
  • src/auto-reply/reply/agent-runner-memory.test.ts — parametrized regression test

Real behavior proof

Behavior addressed: per-model claude-cli runtime sessions no longer wedge in preflight compaction when it returns a benign skip reason.

Real environment tested: built from this branch (2026.5.25, Node 24, macOS) and running as the gateway; agent default anthropic/claude-opus-4-7 with a per-model agentRuntime.id=claude-cli pin (provider resolves to anthropic).

Exact steps or command run after this patch: node scripts/run-vitest.mjs run src/auto-reply/reply/agent-runner-memory.test.ts

Evidence after fix: Test Files 1 passed (1) / Tests 28 passed (28), including a parametrized case over no real conversation messages, Already compacted, nothing to compact, and below threshold. The gateway built from this branch logs no Preflight compaction required but failed: … occurrences.

Observed result after fix: benign compaction skips return the existing session entry and the turn proceeds.

What was not tested: did not also add a runtime-aware guard at the isCliProvider check (agent-runner-memory.ts:612) — the more complete complementary fix, left as follow-up; full CI suite not run locally (narrow unit + live behavior only).

claude-cli-runtime sessions (provider stays "anthropic" via a per-model
agentRuntime pin) wedged every dispatch when pre-flight compaction returned
a benign skip ("no real conversation messages", "Already compacted", etc.):
the unconditional throw failed the turn and the session could not recover.
Defer to the existing isCompactionSkipReason classifier and return the
session entry instead of throwing.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added size: S triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 25, 2026
@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: the same CLI-runtime preflight failure is already owned by #86224, which is open, proof-positive, maintainer-modifiable, and fixes the root gate mismatch more directly; this draft branch is dirty, has insufficient real behavior proof, and broadens skip handling in a way that can mask the separate terminal-session bug tracked at #86593.

Canonical path: Close this draft and review or land #86224 for the CLI-runtime gate fix; leave terminal-session routing behavior to #86593 rather than hiding it in preflight.

So I’m closing this here and keeping the remaining discussion on #86224 and #86593.

Review details

Best possible solution:

Close this draft and review or land #86224 for the CLI-runtime gate fix; leave terminal-session routing behavior to #86593 rather than hiding it in preflight.

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

Yes from source inspection. Current main calls embedded preflight compaction for canonical-provider runs that are not recognized by isCliProvider, then throws on non-compacted results such as no real conversation messages.

Is this the best way to solve the issue?

No. The better current solution is the resolved-runtime gate in #86224, which fixes the CLI-runtime cause without globally making every skip-like preflight result proceed.

Security review:

Security review cleared: The diff introduces no dependency, workflow, secret-storage, or supply-chain change; the auth/session concern is covered as functional merge risk rather than a concrete security defect.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • steipete: Current blame in this checkout points the preflight throw and compact skip classifier to 2afb819, and recent adjacent PR 86408 also names Peter Steinberger in co-author provenance. (role: current source provenance and adjacent reviewer/co-author; confidence: medium; commits: 2afb8198c126, f4cfa012e117; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/commands-compact.ts)
  • funmerlin: Commit f4cfa01 recently changed preflight compaction threshold behavior and tests in the same agent-runner memory surface. (role: recent adjacent contributor; confidence: medium; commits: f4cfa012e117; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/agent-runner-memory.test.ts)
  • potterdigital: Merged commit efd88dc changed runtime policy matching that the canonical replacement PR depends on, and the related PR discussion links this work to the same CLI-runtime gate boundary. (role: adjacent runtime-policy contributor; confidence: high; commits: efd88dc00d8d; files: src/agents/model-runtime-policy.ts, src/agents/model-runtime-aliases.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against a4f12699cf50.

@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. labels May 25, 2026
@ottoclaw

Copy link
Copy Markdown
Author

Closing as superseded by #86224, which resolves the CLI-runtime preflight failure more directly — gating on the resolved runtime at both the preflight-compaction and memory-flush sites rather than broadening skip handling. Terminal-session resume behavior is tracked separately in #86593.

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

Labels

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. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant