🤖 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
|
Addressed both current findings:
The affected suites pass 90 tests / 490 assertions; canonical static and independent review pass. Final phase integration on main Generated with |
|
@codex review Please review the current head after the complete phase was composed on current main. All known findings have fixes and replies in their owning layers; the eight-member phase stays held until all members are approved and green. 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: 2391f02cb4
ℹ️ 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".
2391f02 to
0582c26
Compare
|
Addressed 3990222839: old-reader summary/partial neutralization now precedes Stop sidecar publication. Failed initial cleanup preserves the predecessor bytes or absence and exact local retry debt, rather than reporting a durable Stop. Live compaction requests retain their admission generation through AgentSession and pass it to completion publication. This prevents a late producer from recreating stopped recovery even after that Stop has been retired. Authored follow-up text, attachment data, and request metadata remain intact. Real storage crash-boundary controls and real-session late-completion controls distinguish this from merely checking whether a sidecar currently exists. Addressed 3990222848: service send and Resume acquire the persisted frontier before pricing/settings awaits and carry it through preparation to the existing publication CAS. Repair and capture occur under the same history lock, retaining one-call explicit malformed/unsupported-state recovery. A newer foreign Stop cannot be adopted by an older request; explicit Send Now retains its exact initiating receipt. The two-backend pricing regressions failed on the previous implementation and pass with the carried frontier. Context-budget continuations retain that same originating capture. Legitimate queued input can cross an owned reset or Stop retirement only through its exact committed predecessor-to-successor receipt; this is not a later storage recapture, and foreign-Stop interleavings remain fenced. Combined phase validation: 3,715 tests across 50 affected service suites passed (34,595 assertions). Full Generated with |
|
@codex review Please review the updated 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: 0582c26778
ℹ️ 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".
0582c26 to
5a63d1b
Compare
|
Addressed 3991360908: full Clear and destructive replacement execute the existing history deletion transaction under the cancellation lock before publishing the sidecar, after workspace monitor/workflow barriers. Tests cover malformed history, chat/archive/foreign partial removal at the publication boundary, failed barriers, uncertain deletion, observer failure, sidecar-only retry preserving a newer append, and deletion accounting after a later publication failure. No new persisted transaction format was added. Addressed 3991360914: the captured admission travels through ordinary and prepared request startup and is rechecked before provider construction. Manual Send/Resume attachment-window tests and actual engine/provider tests reproduce foreign Stop refusal. If refusal follows construction, the captured provider is aborted before resource release; local cancellation and successor registrations retain their existing ownership. The A rollover CI fixture now holds the real guarded publication receipt, preserving both rollback outcomes. The foreign-Stop rollover cases now require zero provider calls while retaining any already accepted input and the exact newer Stop. The cancellation layer passed 3,996 tests across 54 suites; the settled-Stop layer passed 4,064 across the same 54 suites. Its final test-only assertion adjustment also passed the complete affected 52-test suite. Both final runtime branches passed canonical static checks. Generated with |
|
@codex review Please review the updated 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: 5a63d1b27e
ℹ️ 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".
5a63d1b to
266cf35
Compare
|
Addressed 3992045941: the legacy The #4209 projection retains the existing exception for already-qualified settled V2 automatic replacement. A valid V2 record is unresolved; unsupported scoped V2 bytes remain preserved and refuse admission. No new persisted state or broader automatic-admission restriction was added. Regression coverage includes failed-cleanup automatic input, manual replacement, no-Stop compaction, and the settled-V2 exception. Final integrated validation passes on #4191 Generated with |
|
@codex review Please review the updated head Final integrated validation passes on #4191 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: 266cf35aac
ℹ️ 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".
--- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->
266cf35 to
052fc08
Compare
|
Addressed 3992409150: the request’s original nonce/generation comparison and synchronous provider construction/registration now share the existing physical history lock. The capture is copied before awaiting entry; no later Stop is adopted. The final prepared caller supplies the fence. Supervision and cleanup ownership are installed before lock release can await, while provider playback, envelopes and cleanup stay outside the lock. The original source fails the deterministic Stop-after-last-check regression by constructing the provider. The corrected source refuses it and releases resources. Additional real-history controls cover a competing Stop during construction, caller capture mutation, factory and post-registration fence failures, scope shutdown during the release window, and preserved mock playback. A/B four-suite runs pass 514 and 561 tests respectively; final focused controls pass 13 tests / 99 assertions on both layers, with full types, lint and formatting. Final focused runs include the added fence-failure control and fixture-only lint correction. Final integrated validation passes on #4191 Generated with |
|
@codex review Please review updated head Final integrated validation passes on #4191 All eight members remain held until current-head review approval and required CI are complete. Generated with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 052fc08451
ℹ️ 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".
| await this.compactionCancellation | ||
| .retireReplacement(witness, (predecessor, successor) => { | ||
| this.advanceOwnedCompactionAdmission(predecessor, successor, preparing); | ||
| }) |
There was a problem hiding this comment.
With XUM_ALLOW_MULTIPLE_INSTANCES, after backend A stamps a manual replacement row, backend B can discover that witness and unlink the same cancellation sidecar before A finishes this retirement. A's retireReplacement() then returns superseded without invoking onRetired, leaving attempt.admissionCapture at { nonce: N } while disk now reports { nonce: null }; the provider-start revalidation rejects the already-accepted manual turn, so the user's durable row is never sent. Treat a same-generation peer retirement as the expected successor and advance A's capture, while still rejecting a newer Stop.
AGENTS.md reference: AGENTS.md:L101-L106
Useful? React 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 V1 layer defers unresolved automatic recovery; #4209 adds settlement proof for eligible fresh input.
Requests capture the persisted Stop frontier before pricing/settings preflight and retain it through preparation and queued dispatch. Publication must match that original nonce/generation, so an older request cannot adopt another backend's later Stop. Exact Send Now admission carries its initiating Stop receipt; manual malformed-record recovery remains a single operation. Context-budget continuations retain their originating capture, and committed local resets and verified Stop retirement advance only queued or active captures matching the exact predecessor nonce and generation. This preserves legitimate queued input across rollover without adopting a foreign Stop. The recorded admission is checked again after request preparation and immediately before provider construction, including prepared requests. A refusal after stream construction aborts that captured provider before releasing its resources.
Stop joins physical execution and terminal policy outside history locks. Legacy summary/partial follow-up metadata is neutralized before the Stop sidecar is published, and live compaction completion retains its original admission generation so it cannot republish stopped recovery. Failed initial cleanup preserves the predecessor and exact local retry debt; it does not claim a successful durable Stop. Authored follow-up request content remains intact. Full Clear runs the existing deletion transaction under the cancellation history lock, after workspace cleanup barriers and before sidecar publication. This preserves malformed-history Clear while removing downgrade-readable recovery. Uncertain deletion is not retried over later foreign input; a confirmed deletion receipt permits only sidecar retry and retains UI/monitor accounting if publication fails.
Edit/truncation receipts follow directory durability. Reset heartbeats refuse scoped-summary cancellation debt while ordinary scoped-V1 automatic admission remains supported. Rejected manual input keeps preserve-mode receipts so committed rejection rows are not duplicated into the composer and uncommitted drafts can be restored.
Outstanding scoped cleanup debt prevents automatic input from starting legacy token-pressure compaction. Manual replacement remains eligible to compact, and the existing scoped-V1 ordinary-input path remains supported.
The original persisted nonce/generation is revalidated while holding the existing shared history lock through synchronous provider construction, registration, and supervision. Prepared requests use the final caller’s fence. Playback, envelope writes, and cleanup are outside the lock. Factory or fence failure retains exact registered-resource cleanup ownership; mock playback keeps its existing immediately awaited admission check.
Validation covers two-backend preflight races, queue/Send Now admission, downgrade publication boundaries, late compaction completion, directory flush failures, and real budget rejection. Final integrated validation passes on #4191
052fc084517c1a6fd8cfbecdf08c05b635358a32(4026 tests / 36442 assertions) and #420902651d365f63c9dc04d95744207f42f8065d6f35(4095 tests / 36847 assertions), across 54 affected suites each. Full source/test TypeScript andmake static-checkpass on both exact commits.Risk: ownership mistakes can resume canceled work or strand valid input. Held preflight/cleanup and foreign-supersession controls exercise those boundaries directly.
This is one layer of the cancellation phase: #4214 → #4215 → #4219 → #4221 → #4182 → #4187 → #4191 → #4209. All eight PRs merge together after every member has current-head approval and passing CI.
Generated with
xum• Model:unavailable• Thinking:unavailable• Cost:$unavailable