fix(reply): don't wedge preflight compaction on benign skip reasons#86663
fix(reply): don't wedge preflight compaction on benign skip reasons#86663ottoclaw wants to merge 1 commit into
Conversation
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]>
|
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 detailsBest 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 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:
Codex review notes: model gpt-5.5, reasoning high; reviewed against a4f12699cf50. |
Summary
runPreflightCompactionIfNeededthrows unconditionally on any non-ok/non-compacted embedded compaction result. For per-modelclaude-cliruntime sessions (the model's provider staysanthropicvia a per-modelagentRuntime.idpin), the embedded compaction legitimately returns a benign skip — e.g.no real conversation messages(the CLI session's OpenClaw-side transcript is empty) orAlready 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
isCompactionSkipReasonclassifier (already used on the manual/compactpath) 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— exportisCompactionSkipReasonsrc/auto-reply/reply/agent-runner-memory.ts— graceful skip before the throwsrc/auto-reply/reply/agent-runner-memory.test.ts— parametrized regression testReal behavior proof
Behavior addressed: per-model
claude-cliruntime 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-7with a per-modelagentRuntime.id=claude-clipin (provider resolves toanthropic).Exact steps or command run after this patch:
node scripts/run-vitest.mjs run src/auto-reply/reply/agent-runner-memory.test.tsEvidence after fix:
Test Files 1 passed (1)/Tests 28 passed (28), including a parametrized case overno real conversation messages,Already compacted,nothing to compact, andbelow threshold. The gateway built from this branch logs noPreflight 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
isCliProvidercheck (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).