🤖 fix: activate durable compaction cancellation across turn admission - #4191
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 9ab5348384
ℹ️ 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".
|
Addressed all three findings in 38b907a:
Validation: 701 tests across cancellation runtime/core/storage, exact cleanup, and WorkspaceService; 743 tests across AgentSession callers and history truncation (overlapping the runtime suite); TypeScript, no-cache ESLint, and full Generated with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38b907ac6b
ℹ️ 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".
38b907a to
1ac4cea
Compare
|
Addressed the three commit-boundary findings in their owning layers:
The lower layer also fixes the Integration CI value-comparison failure: capture CAS compares primitive nonce/generation values, and exact resume comparisons use a realm-local JSON snapshot. The original mock-only IPC test reproduced the normal-send rejection; dedicated append and resume acceptance tests now pass under Jest's VM and complete mock streaming. Final validation: lower commit Generated with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ac4cea9b7
ℹ️ 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".
1ac4cea to
639a4d7
Compare
|
Addressed both findings in
The resume CI failure had a separate subscription window: the helper counted the previous attempt's replayed stream-error as a failure of the resumed attempt. The shared helper now waits for initial subscription replay and clears those events before resume. Existing prefix/continuation assertions are unchanged; mock tests prove both successful resume and rejection of a fresh resumed error. Final validation on this exact combined head: 2,665 tests across 52 files with 13,488 assertions; 15 mock IPC cases; five original Send Mode UI cases; the original queued Send Now case; Generated with |
|
@codex review Please review current head Generated with |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 639a4d78cb
ℹ️ 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".
|
Addressed both findings in the runtime repair pair ending at
The actual older parent Validation: 795 focused tests, 94 queue tests, 16 mock IPC cases, five original Send Mode UI cases, the original queued Send Now case, and four actual-parent compatibility cases; build, types, lint, formatting, and diff checks pass. Fresh full Generated with |
|
@codex review Please review current head Generated with |
There was a problem hiding this comment.
💡 Codex Review
xum/src/node/services/historyService.ts
Lines 491 to 495 in 1843fe9
Keep malformed partials from blocking Stop recovery
When a hard Stop that preserves partials (such as ACP cancellation or the CLI budget stop) encounters a malformed partial.json, this unconditional parse failure occurs after the cancellation sidecar has been committed. The mutation remains blocking, and every subsequent manual send retries the same cleanup and fails before it can publish a replacement, permanently bricking the workspace until the user manually deletes the partial. Quarantine or discard malformed partial state, or allow explicit replacement to supersede this cleanup debt.
AGENTS.md reference: AGENTS.md:L110-L111
ℹ️ 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 Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1843fe9 to
f68020e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f68020eeab
ℹ️ 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".
|
Addressed review-body finding 5159513658, "Keep malformed partials from blocking Stop recovery." Stop now removes an unusable partial under the existing history locks after the final physical lease and logical generation checks. Valid partials retain their recovery fields; ambiguous privacy markers and I/O failures remain protected. The sidecar stays durable through cleanup, including a late settlement writer or a failed deletion retried by manual input. An invalid manual replacement retains the same Stop; an accepted replacement becomes durable before retirement. Real HistoryService regressions cover malformed JSON, invalid message shape, damaged UTF-8, late partial publication, cleanup retry, and successor preservation after lease/generation displacement. The final integrated phase on current main passes 1,884 tests and 9,390 assertions. The independent coordinator review approved the production repair; the final main-integrated candidate also passes the complete Generated with |
|
@codex review Please review current head Generated with |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f68020eeab
ℹ️ 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".
f68020e to
c11d840
Compare
|
Published the remaining review fixes in
Runtime A passed 257 targeted tests covering refusal, queued ownership cleanup and manual recovery; its canonical static result is passed on exact tree Generated with |
|
@codex review Please review current head Generated with |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c11d840d0b
ℹ️ 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".
c11d840 to
46d3f16
Compare
|
Addressed Send Now restoration after cancellation-persistence failure in The six V1 monitor tests now verify the documented intermediate behavior: preserve the original retirement/error assertions, keep owed attention without an idle retry loop, and drain it after manual recovery. #4209 retains the original immediate fresh-wake assertions. Validation: 900 tests across eight suites plus 240 budget tests passed; canonical static passed on Generated with |
|
@codex review Please review current head Generated with |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46d3f16b72
ℹ️ 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".
46d3f16 to
a7a9a86
Compare
|
@codex review Generated with |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7a9a86cfc
ℹ️ 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".
Activate V1 durable Stop and manual recovery, preserving queued input and physical execution ownership. Fresh unresolved automatic input remains deferred until settlement proof is available. --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->
Restore rollback-eligible manual input after preparation failure without duplicating published or budget-rejected rows. Verify V1 monitor debt and manual recovery. --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->
Send Now carries its initiating Stop receipt through workspace cleanup. Reset heartbeats retain captured generation through boundary publication. Rejected manual input uses a real visible/durable publication receipt so failed writes restore drafts without duplicating committed rows; service-boundary lifetime tests retain actual disk and shutdown assertions. --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->
a7a9a86 to
2e524da
Compare
|
Addressed all four current findings:
The combined history/runtime candidate passed 556 tests / 3,208 assertions before the final lifetime fixture correction; the complete upper composition passed 1,077 tests / 4,472 assertions. The final A lifetime suite passes 16 tests / 70 assertions; final B cleanup/test-typing controls pass 39 cases / 169 assertions. Source/delta review and final composed canonical static checks pass. The remaining oversized-history scanner finding is tracked separately in #4182. Generated with |
|
@codex review Generated with |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Activates durable compaction cancellation across Stop, manual send/edit/Resume, queued input and automatic recovery. Failed preparation preserves cancellation; a durably accepted replacement may release it. This intermediate V1 layer defers fresh automatic input behind unresolved Stop; #4209 supplies settlement proof for eligible fresh input.
Send Now carries the initiating Stop's exact nonce/generation receipt through workspace cleanup and queue dispatch. A later local or foreign Stop cannot be adopted accidentally. Soft Send Now retains its existing behavior. Reset heartbeats capture admission before awaiting the gate and keep that generation through boundary publication.
Rejected manual input uses preserve-mode publication receipts. A sequence allocated before a failed file open no longer suppresses draft restoration; committed rejection rows are not duplicated into the composer. Actual oversized budget capsules and service-boundary shutdown controls cover both outcomes. Resume inherits #4182's verified stamped-target recovery after cancellation-sidecar cleanup failure.
Stop joins physical execution and terminal policy outside history locks. Full Clear establishes a retained floor before deletion; failed cleanup and unsupported records preserve manual recovery. Existing scoped-V1 ordinary admission remains distinct from unresolved and retained cancellation.
Validation: independently reviewed source and canonical static checks pass. The integrated candidate passed 556 history/runtime/queue/budget tests / 3,208 assertions; the final A-owned lifetime suite passes 16 tests / 70 assertions. Risks are shared admission ordering and separating visible input, durable acceptance and cleanup debt.
This cancellation phase includes #4182, #4187, #4191, #4209, reader prerequisites #4214/#4215 and the remaining bounded history adapters. Hold the whole phase for integration, current-head review and CI; the oversized-row finding remains open until its adapter is wired into history lookup.
Generated with
xum• Model:unavailable• Thinking:unavailable• Cost:$unavailable