🤖 fix: stop idle auto-compaction loop on repeated/model-not-found failures#3567
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a65f175c2
ℹ️ 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".
Address Codex review: a clean provider stream-end does not guarantee the post-stream history compaction succeeded. Drive the idle success/failure outcome from CompactionHandler.handleCompletion (after the summary is persisted, or on a post-stream persistence failure) via a new onIdleCompactionOutcome callback, instead of reporting success on stream-end.
|
Addressed in 083e71d. The idle-compaction outcome is no longer reported from the provider
Added |
|
@codex review Please take another look — addressed the post-stream persistence concern. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 083e71d465
ℹ️ 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".
Address Codex review: the requireIdle path returns a busy-skip when the workspace becomes active after eligibility but before sendMessage. That is an expected race, not a failure. Skip reporting it as an outcome so two normal user-interaction races cannot suppress idle compaction for a healthy workspace. Factor the busy-skip message into a shared constant.
|
Addressed in c1a1b76. The |
|
@codex review Please take another look — the requireIdle busy-skip is now treated as neutral. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1a1b76bc4
ℹ️ 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".
Address Codex review: an in-flight retry can persist a successful compaction after suppression was set. recordOutcome(success) now also clears the suppressed set, so the workspace self-heals and becomes eligible again.
|
Addressed in 866a615.
|
|
@codex review Please take another look — suppression is now lifted on a later success. |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
Summary
Stops the hourly idle auto-compaction loop from re-attempting a workspace that keeps failing: it now gives up immediately when an attempt fails with a model-not-found error, and otherwise after two consecutive failures of any kind. Previously a persistently failing workspace (e.g. a misconfigured/typo'd compaction model) was re-queued every hour forever, since a failed compaction neither marks the workspace
compactednor refreshes recency, so it stayed eligible indefinitely.Background
IdleCompactionServicere-checks eligibility every hour and re-queues any idle workspace. There was no failure tracking, so a workspace whose compaction always fails would be retried on every cycle with no cap. The most common cause is a compaction model the provider rejects.A key subtlety: the failure can surface in two different places.
sendMessageResultthatexecuteIdleCompactionalready inspects.model_not_foundcase for a well-formed but non-existent Anthropic/OpenAI model) are not returned bysendMessage— they arrive later as an async"error"stream event and are only persisted transiently topartial.json, so they can't be recovered from chat history on the next check.Implementation
IdleCompactionServicenow tracks per-workspace consecutive failures and a stickysuppressedWorkspaceIdsset.checkEligibilityshort-circuits suppressed workspaces (reason: "suppressed_after_failures"). A newrecordOutcome(workspaceId, outcome)applies the policy: success resets the streak;model_not_foundsuppresses immediately; any other failure suppresses on the second consecutive occurrence.WorkspaceServicereports terminal idle-compaction outcomes through a listener:executeIdleCompaction(model-not-found classified viaformatSendMessageError),aiService.on("error")handler when the workspace is mid idle-compaction (read beforestopStreamingStatusclears the marker),handleStreamCompletion.error/stream-end/stream-abortare mutually exclusive per turn, so a failed turn never also reports success. Aborts are intentionally neutral (not a failure).ServiceContainerwires the listener toIdleCompactionService.recordOutcome.Suppression is in-memory and sticky for the service lifetime; restarting the app (e.g. after fixing the configured compaction model) clears it.
Validation
idleCompactionService.test.ts: suppress after two consecutive failures, suppress immediately onmodel_not_found, success resets the streak, andcheckAllWorkspacesno longer queues a suppressed workspace.workspaceService.test.tstests:executeIdleCompactionreportsmodelNotFound: truefor an invalid compaction model andfalsefor a generic pre-stream failure.make static-checkand targeted tests pass.Risks
Low. Behavior is additive and scoped to the background idle-compaction loop. The outcome listener is optional (no-op until wired), and normal interactive sends are untouched. Worst case of a wrong suppression is that a workspace stops auto-compacting on idle until restart, which a user can already trigger manually.
Generated with
mux• Model:anthropic:claude-opus-4-8• Thinking:xhigh• Cost:$1.48