🤖 fix: clear stale usage state when history is rewritten so the next send does not auto-compact - #3818
🤖 fix: clear stale usage state when history is rewritten so the next send does not auto-compact#3818ibetitsmike wants to merge 30 commits into
Conversation
…oundary Start Here (replaceHistory), /reset (resetContext), and /clear (truncateHistory) rewrite history to a fresh context boundary but left the session's in-memory lastUsageState from the previous stream intact. The next sendMessage fed that stale high usage into the on-send compaction check and synthesized a spurious /compact request against the already-cleared context. Clear the session usage state in all three paths; the next send re-seeds accurate usage from post-boundary history. --- _Generated with `mux` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
Cleanup-gate follow-up: the same stale-usage defect class existed on the heartbeat context reset, exec-subagent hard restart, and edit truncation paths, plus two ordering gaps (destructive replaceHistory cleared history before the summary append could fail, and zero-percentage truncation cleared usage without rewriting anything). Route all context rewrites through clearUsageState() and slim the new tests down to the repo's fake-session wiring pattern. --- _Generated with `mux` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…st bootstrap CI-only failure on this PR: 35 AgentListItem/GitStatusIndicatorView/ TaskGroupListItem tests failed with "Cannot access 'DragPreviewImage' before initialization". Changing test files shifts bun's CI discovery order, letting a pre-existing order bug surface: files that tear down with `globalThis.document = undefined` poison installDom()'s snapshot/restore, which propagates a document-less environment (with baseline observers present) to later files; @react-dnd/asap then crashes at module eval and leaves react-dnd's internal exports in TDZ, so every later import including `?real=1` re-evals fails. Fix at the chokepoint in tests/ui/dom.ts: the uninstaller re-bootstraps a baseline when it would restore a poisoned document, and react-dnd's module graph is evaluated eagerly while the DOM is healthy so re-evals resolve initialized internals. Both guards in tests/ui/domIsolation.test.ts fail against their own toggle. --- _Generated with `mux` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3e6dcd4cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tests/ui/domIsolation.test.ts uses bun:test (it guards bun's single-process module cache behavior), so the Jest-based integration run fails to resolve its import. Skip it via testPathIgnorePatterns like the storybook bun tests. --- _Generated with `mux` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fadf52b1fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ider usage Codex P2: for a partial /clear, clearing the in-memory usage cache was ineffective because the retained newest assistant row still carries pre-truncation contextUsage and partial truncation appends no boundary, so seedUsageStateFromHistory() restored the same stale value on the next send. clearUsageState() now also suppresses history seeding until the provider reports fresh usage (updateUsageStateFromModelUsage resets the flag). Guard tests: a baseline proving the boundary-less fixture seeds, and a suppression test that fails without the new guard. --- _Generated with `mux` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37ced8cace
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2: tests/ui/domIsolation.test.ts was excluded from Jest but no bun target ran it, so the guards never executed in local validation or CI. Add the file to the Makefile test-unit bun invocation and to the CI Unit job's explicit file list. --- _Generated with `mux` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acc9f3d7b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2: the in-memory seeding suppression does not survive an app restart. After a partial /clear plus restart, seedUsageStateFromHistory() reloaded the retained assistant row's pre-truncation contextUsage and could spuriously auto-compact again. Sanitize at the source: the partial-truncation rewrite now strips contextUsage and contextProviderMetadata from retained rows, since those snapshots measured a context that no longer exists. Restart-safe with no extra persisted state; the in-memory suppression flag still covers the same-process window until fresh provider usage arrives. Red-green verified via a historyService test. --- _Generated with `mux` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
Codex P2: queued user input rolls back an undispatched heartbeat reset boundary and restores the pre-reset provider context, but the seeding suppression from the boundary append stayed active, so the queued turn could not recover the restored history's near-limit usage and skipped on-send compaction. Re-enable seeding on successful rollback. Guard test drives the real append -> queue -> rollback path and fails without the re-enable. --- _Generated with `mux` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b25223c6c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9dd1adc41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2: when a partial truncation committed but the monitor-wake restoration step of clearHistoryWithRetiredBashMonitorWakes threw, the invalidation after the wrapper never ran and the session kept its pre-truncation usage. Move the clear into the truncate closure, immediately after the successful rewrite. Guard test injects a wake-restore failure and fails without the move. --- _Generated with `mux` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
Codex P2: message edits truncate a suffix, so the retained prefix's persisted contextUsage still describes the active context. Blanket seeding suppression meant an edit whose stream failed before reporting usage left the next send unable to recover that valid snapshot, bypassing on-send compaction for near-limit prefixes. clearUsageState() gains a preserveHistorySeeding option used by the edit path. Guard test drives edit-then-send and fails without it. --- _Generated with `mux` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db716c50a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6704dbd2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…st boundary Codex P2: after a compaction boundary exists, a 25/50/75% truncation can remove only sealed pre-boundary rows, leaving the active provider context unchanged; stripping retained contextUsage and suppressing the in-memory seed then discarded a valid near-limit snapshot and could skip required on-send compaction. truncateHistory now reports whether the cut reached the active window (removed the latest durable boundary or later rows). Rows are sanitized and session usage cleared only in that case. Red-green: forcing the flag true fails the new preserve test; the strip test still guards boundary-less cuts. --- _Generated with `mux` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e11580c55a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ls before committing
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aca3df96ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…holds active-window rows A normally rotated archive contains only sealed pre-boundary rows, so deleting it leaves the provider context unchanged. Setting windowChanged unconditionally blocked the usage rollback when the subsequent chat cut failed, leaving an unchanged near-limit context with no seedable usage.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb005038f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…leave stale session usage truncateHistory now takes an onActiveContextTruncated callback fired at most once, the moment a committed step first removes provider-eligible rows from the active window, even when a later step fails. Callers (truncation, destructive replace, hard-restart clears) invalidate session usage through it instead of success-only guards, closing the window where a partially committed cut returned Err with window content already deleted.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2cee1951a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…he window In normal rotated layouts the final chat cut is the only window-changing step and strips usage in the same atomic write, so a separate sanitize write only created a crash window stranding an unchanged near-limit context with no seedable usage. Pre-sanitize now happens only when the archive step that runs first is itself window-changing (boundary-less or boundary-in-archive layouts), where stripping first remains the safe direction.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b58b5daaa3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…mit cannot leave stale usage truncateAfterMessage rewrites chat.jsonl and can still fail afterwards (archive removal in the pre-boundary branch, the sequence-floor archive read in the active branch). The new onContextRewritten callback fires the moment the rewrite commits, and AgentSession invalidates usage through it instead of only on success.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b59a3fe9a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…commits A pre-boundary edit that commits chat.jsonl but fails archive removal leaves the archive alongside a duplicated prefix whose usage snapshots no longer measure the real payload. The commit-point callback now only clears usage; seeding is re-enabled solely when the truncation fully succeeds and both files are consistent.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d1e3ebd0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…leave renderer ghost rows truncateHistory/clearHistory now take an onRowsDeleted callback fired with each committed step's deleted sequences, even when a later step fails. The three DeleteMessage emitters (truncateHistory, destructive replaceHistory, exec hard-restart clear) emit accumulated committed deletions on failure paths too, so rows already removed from disk cannot linger in the renderer with sequences no retry can report.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9bb86a073
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tripping usage while the hazard exists The copied prefix is written with usage snapshots stripped while the archive still exists, so a failed or crashed archive removal leaves nothing stale for a fresh session to seed. Once the removal commits the hazard is gone, and a restore write brings the usage back because it is exact for the restored prefix and an edited near-limit prefix must stay monitored on the next send.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 170a0d90f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…after the rewrite The edit-truncation commit callback now reports whether the retained rows' persisted usage stays valid. Active-file suffix cuts create no duplicated prefix, so seeding re-enables at commit and a near-limit prefix stays monitored despite the Err; the archived branch keeps suppression until full success.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Fixes a spurious
/compactsend: after using "Start Here" to clear context (and after/reset,/clear, and other history rewrites), the very next user message triggered on-send auto-compaction against the already-cleared context.Background
AgentSessioncacheslastUsageState(context usage from the last API response) to feed the on-send auto-compaction check insendMessage. History-rewrite operations appended a fresh context boundary but never invalidated that cache.seedUsageStateFromHistory()early-returns when state is already defined, so the stale pre-rewrite usage reachedcheckBeforeSend, which synthesized a compaction request for a context that was just cleared.Repro: let a chat approach the auto-compact threshold, click "Start Here" on an assistant message, then send a message. The send arrives as a
/compactrequest instead of the typed message.Implementation
New
AgentSession.clearUsageState()with one invariant: whoever successfully rewrites the active provider context invalidates cached usage; the next send re-seeds from post-boundary history. Wired into every rewrite path:WorkspaceService.replaceHistory(Start Here), both modes; in destructive mode the clear happens right after history is cleared so a failed summary append cannot leave empty history with stale usageWorkspaceService.resetContext(/reset)WorkspaceService.truncateHistory(/clear), guarded so a zero-percentage no-op preserves usagecontextMode: "reset"boundary (would otherwise immediately re-compact its own follow-up)Validation
compaction-requestis persisted and the typed message dispatches. Red-green verified: fails with the fix reverted./resetand/clear(including the zero-percentage no-op case) and a heartbeat-reset test, each verified against its own toggle.Risks
Low. The clear is a one-line cache invalidation on paths that already rewrote history; the next send re-seeds from persisted history, which is the same recovery path used after app restart. Worst case of over-clearing is one deferred compaction check, not data loss.
CI test-infra fix (second commit)
The first CI run exposed a pre-existing order-dependent test bug unrelated to this change: test files that tear down with
globalThis.document = undefinedpoisoninstallDom()'s snapshot/restore, which propagates a document-less environment to later files;@react-dnd/asapthen crashes at module eval and leaves react-dnd's exports in TDZ (35 AgentListItem/GitStatusIndicatorView failures, only reproducible under CI's file discovery order). Fixed at the chokepoint intests/ui/dom.ts(uninstaller self-heal + eager react-dnd evaluation) with toggle-verified guards intests/ui/domIsolation.test.ts.Generated with
mux• Model:anthropic:claude-fable-5• Thinking:xhigh