Skip to content

🤖 refactor: coordinate compaction and durable continuations - #4121

Open
ThomasK33 wants to merge 14 commits into
mainfrom
codex/compaction-continuation-ownership
Open

🤖 refactor: coordinate compaction and durable continuations#4121
ThomasK33 wants to merge 14 commits into
mainfrom
codex/compaction-continuation-ownership

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Sep 7, 2026

Copy link
Copy Markdown
Member

Summary

Compaction can finish after its turn has been canceled or replaced. Give observations and durable continuation handoffs explicit coordinator ownership, and persist Stop independently of transcript cleanup. Late work cannot clear a replacement's state or revive a canceled continuation after restart; temporary admission holds resume recovery when released.

Implementation

  • Track compaction intent, observation stages, and handoff tokens in the turn coordinator. A continuation preserves ownership across its own admission. Stop retains cleanup ownership; committed context replacement starts a new intent. Summary jobs and journal cleanup remain physically leased through reset and shutdown.
  • Revalidate continuation admission under the same cross-process history lock as its append. Summary-based handoffs check exact identity, pending payload, permitted tail rows, and cancellation. Direct legacy and failed-continuous-apply handoffs without durable source identity reject every unwitnessed cancellation scope, including narrowed summary Stops. Skipped writes cannot acknowledge acceptance; real I/O and snapshot cleanup failures remain errors.
  • Match cleanup identity under the history lock and derive edits from the current row. Publish heartbeat rollback state with atomic deletion. If another backend already removed the boundary, restore only when captured, nonempty predecessor IDs and sequences still survive in active/archive history; destructive replacement and ambiguous absence cannot restore stale attachments.
  • Record acceptance at the irrevocable row frontier before goal synchronization or observers. A durable manual row bearing the exact replacement nonce remains accepted when ancillary cancellation deletion fails. Preserve that deletion debt for retry without blocking the new owner. Shared guards recognize active or archived witnesses, while a newer Stop remains effective.
  • Store cancellation in a versioned atomic sidecar. Each explicit Stop gets a distinct nonce and returns success only after durable publication, including when an overlapping Stop supersedes an earlier write. Refresh shared records while preserving unsettled local publication debt; harmless witnessed deletion debt does not hide foreign Stop. Stale reads, receipts, and cleanup cannot overwrite newer intent.
  • Self-heal malformed cancellation bytes under the history write lock: quarantine the original bytes, clear the stale continuous journal, and sanitize active pending follow-ups before removing the corrupt fence. Failures retain cancellation and retryability; permission/I/O failures remain errors. A synchronous repair receipt also invalidates previously captured admission through preparation and append.
  • Record explicit message, context-reset, replacement, and user Retry acceptance in durable history metadata before retiring matching cancellation. Explicit Retry captures nonce or absence and compares shared identity under the same lock as its guarded acceptance write; a newer foreign Stop cannot authorize the provider. Exact durable witnesses distinguish harmless physical deletion debt from current cancellation. Automatic retries cannot create a replacement witness. Capture nonce or known absence before context changes so old operations cannot erase a later Stop.
  • Keep failed cleanup outside physical leases and the live-session registry. Recovery, lazy access, disposal, and shutdown retry bounded work; unresolved cancellation still blocks replacement, while witnessed physical deletion debt remains observable without blocking accepted work. Admission waits stay outside physical I/O leases and retain the recovery checkpoint.
  • Integrate main's token-budget rollovers without bypassing compaction ownership: single and batch admission share the locked guard; raw reset markers remain provider privacy boundaries through repair and conditional rewrites; structural changes invalidate append provenance. Preserve prepared requests and pinned snapshots. A committed rollover retires stale observations while retaining its intentional handoff; workspace callbacks advance admission epochs without adopting a newer Stop.

Validation

  • 1,556 combined session, compaction, history, privacy, provenance, and replay tests pass with real HistoryService instances and the actual WorkspaceStore. All 1,210 workspace/task/container tests and canonical static checks pass. The merged intersection suite passes 446 tests, covering token-budget rollover, guarded batch admission, raw reset preservation, Retry receipts, and compaction ownership.
  • Deterministic held-I/O tests cover Stop before/after claim and row durability, overlapping publications, foreign cancellation during direct preparation, late replacement receipts, stale shared reads, and genuine goal-sync failures after durable acceptance.
  • Fresh-service recovery and fault injection cover failed cleanup, corrupt JSON/schema, every repair stage, foreign Stop during repair, archived replacement witnesses, exact nonce preservation through context changes, and snapshot rollback failures. Heartbeat tests distinguish foreign rollback from clear/replacement and ambiguous empty history.
  • Admission tests cover initial/nested holds, holds arriving during preparation, continuation on release, and cancellation by Stop, replacement, or shutdown. Producer → session → router → WorkspaceStore regressions preserve the merged streaming fix from 🤖 fix: restore live chat streaming after reconnect #4123.
  • A fresh main build of the merged revision passes all nine enabled compaction UI tests, including automatic continuation and notification behavior. The preceding revision also passed all six interruption/send-mode UI tests. One existing compaction case remains skipped. The separate credential-backed context-limit suite requires an unavailable OPENAI_API_KEY.

Risks

This changes ownership across continuous, legacy, and heartbeat compaction and startup recovery. Cancellation ordering, manual-turn priority, and durable cleanup are the main regression risks. Storage errors remain visible and retriable; failure of both transcript cleanup and cancellation publication cannot provide crash durability. Older builds honor successful legacy pending-marker cleanup but cannot read the new fallback sidecar. If cleanup remains impossible, downgrading loses the added restart protection; the parent build already resumes that failed-cleanup state after its own restart.

Pains

Some tests used admission holds as a proxy for committed context replacement; they now signal the actual commit. Tests that artificially awaited Stop while holding its required history write lock now start Stop, release the writer, then await completion. Production callers do not hold that lock while awaiting Stop. A task fixture hit its five-second timeout in Git setup with inherited local template hooks; isolating init.templateDir for the validation process made it pass in 377 ms and the full service suite pass without changing tests or user configuration.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

Compaction work can finish after its originating turn has been canceled or replaced. Give compaction observations and durable continuation handoffs explicit coordinator ownership so late work cannot clear a replacement's state, replay a continuation after Stop, or report a handoff that never reached durable acceptance.

- Track semantic compaction intent, observation stages, and handoff tokens in the turn coordinator. A continuation preserves its ownership across its own admission; unrelated manual or edited turns retire it.
- Keep original eager summary jobs and journal cleanup physically leased through reset and shutdown. Reuse the existing journal queue, and guard completion callbacks against retired intent.
- Guard summary updates and heartbeat-boundary cleanup under the existing history lock and at final publication. Serialize pending-state writes and unlinks, and detach rollback snapshots before awaiting I/O.
- Count a durable continuation as accepted only when its acceptance callback runs. Preserve ordinary continuation queue priority and the existing manual-input vetoes for optional and goal continuations.

- 2,039 backend tests passed across session/compaction, history, workspace/task/container, and CLI suites.
- 15 UI tests passed across compaction, interruption, and send modes; one existing compaction test remains skipped.
- Deterministic regressions cover held preparation, summary generation, journal clearing, history reads/writes, Stop, replacement, rollback, and successful sends that never reach acceptance.
- The automatic-compaction UI fixture now explicitly configures its mock provider and compaction model. The original fixture timed out on unchanged main without those prerequisites; the corrected fixture passes on both main and this branch.
- The separate credential-backed context-limit UI suite could not run locally because `OPENAI_API_KEY` is unavailable. The mock-provider compaction suite covers context-limit recovery.

This changes ownership across continuous, legacy, and heartbeat compaction and startup replay. Cancellation ordering, manual-turn priority, and durable cleanup are the primary regression risks; the held-I/O and UI tests exercise those boundaries.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: I8ff308b1367d223a86fe3914da56c8e928f334df
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-07T18:55:12.896585Z 4c39da4 New commits
🔒 Security Review Completed 2026-09-07T19:01:37.150735Z 4c39da4 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review the compaction and continuation ownership changes, particularly Stop versus replacement cleanup, durable acceptance, and shutdown joining original asynchronous work. Local backend and mock-provider UI validation is complete.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44d48d3b8d

ℹ️ 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".

Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/turnCoordinator.ts
Record supersession at the session failure source so replacement after durable acceptance cannot restart the predecessor's recovery path. Genuine startup failures retain their error handling even when a replacement arrives later.

Reject new dispatch claims after Stop and allow a separate guarded cleanup claim to remove the abandoned durable follow-up without sending it or changing a replacement's history.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: Ia08d923a50745a76ccc2bab5235820ebf5b1e90e
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed both findings:

  • PRRT_kwDOPxxmWM6f5o5F: accepted continuations retain their historical handoff when admission or startup is superseded. The session captures supersession at the failure source; the dispatcher does not infer it from later live state. Deterministic real-send tests cover replacement before PREPARING and during startup, plus a genuine startup failure followed by a later replacement that must still remain a failure.
  • PRRT_kwDOPxxmWM6f5o5J: new dispatch claims now require ready intent. Stop permits only guarded cleanup ownership, which cannot pass send admission. Tests cover Stop before targeted and startup claims, Stop during a held read, and replacement while abandoned cleanup is pending.

The coherent session/compaction suite passes all 772 tests; workspace/task/container suites pass 1,161 tests and full static checks pass.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Both P1 findings are fixed in 4af8d68d3: durable acceptance survives captured supersession, and Stop blocks new dispatch while preserving guarded cleanup. Each finding has a reply and deterministic regression coverage. Please review the revised ownership and error-classification paths.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4af8d68d3b

ℹ️ 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".

Comment thread src/node/services/historyService.ts
Restore the captured pending state and publish boundary deletion synchronously with the guarded history commit. A replacement admitted during later cleanup can no longer observe a deleted boundary with stale attachments. Join the ordered persistence afterward and preserve committed-write facts through cleanup errors.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: If698c58a4512d3a41536e56f413a003e5f0dc82f
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed PRRT_kwDOPxxmWM6f6Iiy. The conditional history delete now invokes a synchronous commit observer immediately after rename. Heartbeat rollback restores and detaches its captured state, queues the immutable persistence update, and publishes deletion plus the state-change callback before yielding. A later admission cannot veto a rollback that already committed.

The regression gates the real post-rename filesystem cleanup and admits a replacement while it is held. It verifies that the replacement sees restored attachments and that the renderer deletion has already been published. Additional cases cover cleanup errors after commit, throwing observers, guard vetoes, archived/missing targets, and late persistence that must not overwrite a successor's snapshot. Validation passes: 777 session/compaction tests, all 102 real HistoryService tests, 1,161 workspace/task/container tests, and full static checks.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

The heartbeat rollback commit/publication race is fixed and covered by a real post-rename cleanup gate. Please review the synchronous commit observer, state restoration/publication ordering, and postcommit error handling. The finding has a detailed reply and is resolved.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4e5bed7367

ℹ️ 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".

Comment thread src/node/services/agentSession.ts Outdated
Keep rollback scoped to the compaction handoff that persisted the rows. Stop retains abandoned cleanup ownership; a superseded handoff cannot invalidate a successor. Cover held append before and after commit, successor preservation, and restart recovery.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: I0b812c3898c4c0c9e54de4f41dde21e8d35eb4de
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed PRRT_kwDOPxxmWM6f6s6P. Persisted-row rollback now checks the captured compaction handoff before resetting semantic state. Stop preserves its abandoned cleanup token, allowing guarded removal of the canceled durable continuation without restoring send admission. A replaced handoff removes only its captured rows and cannot invalidate the successor's compaction or overwrite its summary.

A four-case regression gates the real append both before and after persistence, with and without a manual successor. It reproduced all four failures before the fix and now verifies cleanup, successor row/token/summary preservation, and no canceled continuation on restart. The 781-test session/compaction group and full static checks pass.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

The Stop-during-append cleanup ownership finding is fixed and covered on both sides of the persistence boundary, including manual successor and restart recovery cases. Please review the captured handoff guard and retained abandoned cleanup behavior. The finding has a detailed reply and is resolved.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 75808743a0

ℹ️ 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".

Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/turnCoordinator.ts Outdated
Acknowledge the compaction handoff at the irrevocable row frontier, preserve actual startup errors, and keep abandoned cleanup joined through shutdown. Handle rejected goal work with captured cleanup ownership without removing an accepted continuation's recovery marker.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: I44b6643d5f3c64d8e9bf1f1b8235dce240ffe0b5
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed both findings and adjacent rejection windows:

  • PRRT_kwDOPxxmWM6f7IJ5: a synchronous receipt now records the handoff when its rows become irrevocable, before goal synchronization or row observers. Known admission refusals capture durability at their failure source; later Stop, replacement, shutdown, or disposal cannot erase that historical handoff. Genuine goal/provider/callback failures remain errors and retain the recovery marker.
  • PRRT_kwDOPxxmWM6f7IKF: exact abandoned cleanup remains authoritative while graceful or destructive shutdown drains its execution lease. Already leased terminal work may claim cleanup after shutdown begins; sends and normal claims remain closed. Completed tokens release only their exact cleanup ownership.

The regression matrix covers held goal sync, synchronous row observers, failed rollback, cleanup before and after claim, guarded rewrites through shutdown/disposal, and rejected goal admission/update/publication. Rejection cleanup uses the captured summary and durable receipt, preserving successor state and original error reporting. Independent review reran both additional rejection reproductions red-to-green. The coherent session/compaction group passes 800 tests and full static checks pass.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Both durable-handoff and shutdown-cleanup P1s are fixed. The receipt now precedes all post-durability observers/awaits, abandoned cleanup remains joined through shutdown, and rejected goal work follows the same guarded cleanup obligation without masking the original error or clearing an accepted row's marker. Both findings have a detailed reply and deterministic regression coverage. Please review the durable frontier and exact cleanup ownership paths.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 83ed03e3fb

ℹ️ 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".

Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/turnCoordinator.ts Outdated
Retry an initial failed summary read once under the retained abandoned owner, update only the captured pending handoff from the locked current row, and reset abandonment when a replacement context advances the epoch.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: Ifb99c5fbc1fca3d7c91b98ec29794a96afa00928
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed all three findings:

  • PRRT_kwDOPxxmWM6f74Ch: an initial summary-read failure gets one cleanup-only reread under the same abandoned, unaccepted token. It neither claims a new owner nor sends a continuation, rechecks ownership after I/O, and preserves the original failure even if the bounded retry fails.
  • PRRT_kwDOPxxmWM6f74Cm: cleanup matches row ID, history sequence, captured pending request, and coordinator ownership. An optional locked-row transform derives the edit from the current row, preserving unrelated finalized content, usage, and compaction metadata. A changed pending request or identity is preserved; a removed conditional target settles without chasing another sequence.
  • PRRT_kwDOPxxmWM6f74Co: non-abandoning invalidation creates a ready replacement epoch and retires the old cleanup owner, allowing the new context's continuation to dispatch.

Twelve new regressions cover targeted/untargeted thrown and Result.Err reads, persistent failure bounds, successor preservation, late summary updates, changed pending requests/IDs/sequences, and actual replacement dispatch. Independent review reran all twelve successfully. The coherent session/compaction/history group passes 914 tests, including all 102 real HistoryService tests; full static checks pass.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

The three cleanup-identity findings are fixed and covered by twelve regressions. Please review the bounded same-owner reread, locked-row transform that preserves unrelated fields, and replacement epoch abandonment reset. Each finding has a detailed reply and is resolved. Local coherent validation and static checks pass.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e5a0b0c4b5

ℹ️ 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".

Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/historyService.ts
Comment thread src/node/services/agentSession.ts Outdated
Retain failed canceled cleanup under its exact owner across recovery, disposal, and lazy session recreation. Retry through bounded lifecycle operations and report unresolved teardown after releasing resources. Temporary admission holds preserve cancellation; committed context replacement explicitly starts a new intent. Restore local heartbeat state when another backend already deleted the exact boundary, without duplicate publication.

Integrate main's merged streaming replay fix (#4123) while preserving both coordinator regression suites.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: I3f7ce21ee0d9b837e8fa79354af6a0244c4eb74f
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed both latest findings:

  • 3950869121 — canceled cleanup across process restarts: cancellation now has a versioned atomic sidecar, independent of the in-memory coordinator. Startup reads it before journal or follow-up recovery. Failed cleanup cannot reauthorize the canceled continuation. Explicit messages, context resets, and user Retry record a durable replacement witness before retirement; every Stop gets a distinct nonce so older cleanup cannot erase a later cancellation. Corrupt state blocks automatic recovery but can be repaired by explicit user intervention. Teardown retries the original failed write or deletion and reports unresolved failure after releasing resources.
  • 3950869128 — initial follow-up under an admission hold: recovery waits outside its physical I/O lease before claiming the handoff, rechecks after reads, and captures temporary refusals at the actual send gate. Releasing the last hold resumes the same recovery call once; Stop, replacement, and shutdown cancel stale waits without another reconnect.

Regression coverage uses fresh real HistoryService instances and public startup recovery. It includes failed publication and deletion, replacement acceptance followed by a crash, repeated Stop, held post-rename Retry cleanup, corrupt-state repair, nested holds, and public reset/clear/replace cleanup after retirement failure.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review the latest changes for durable Stop cancellation across restarts and deterministic recovery after temporary admission holds, including explicit Retry acceptance and subsequent Stop ownership.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7ca91fd5ba

ℹ️ 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".

Comment thread src/node/services/compactionCancellation.ts Outdated
Comment thread src/node/services/workspaceService.ts Outdated
Refresh settled cancellation records while preserving local mutation debt and fencing stale reads. Require a locked summary match before narrowing and a durable cleanup receipt before retirement, so obsolete cleanup cannot remove a successor Stop.

Complete replacement append and chat publication before returning cancellation-retirement errors. Capture exact cancellation identity, including absence, across reset, clear, replacement, and no-op reset cleanup. Add real-history regressions for shared backends, held I/O, failed writes, and fresh-service recovery.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: I6381dea8142e3c2fc01bf67d3320a1d7955cfde8
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed both findings from review 5133792357:

  • 3951238322 — stale shared cancellation cache: settled reads refresh the sidecar; in-flight or failed local mutations retain their original retry operation. Generation and mutation identity fence both stale read values and stale errors. Two real backend sessions reproduce cached absence and changed nonces. Obsolete cleanup now requires a locked target match before narrowing and a synchronous commit receipt before retirement, so a conditional no-op cannot erase a foreign successor's Stop.
  • 3951238328 — replacement lost after retirement error: replacement captures and stamps the prior cancellation nonce, then finishes append, ordered delete/message publication, and normal cleanup before returning the retirement error. Tests verify the row and witness on disk, emitted events, and fresh-service continuation. Reset, clear, replacement, and no-op reset carry captured nonce or absence across awaits so a newer Stop survives.

Validation: 1,138 combined session/compaction/history/replay tests and 1,182 workspace/task/container tests pass. Regression cases were reproduced failing before the fixes. Canonical static checks pass after removing one unused test binding.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Addressed both latest P1 findings, with regression coverage for shared-backend cancellation refresh and replacement publication after failed cancellation retirement. Also fenced stale cleanup and captured cancellation identity across context mutations.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 507b70ab6d

ℹ️ 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".

Comment thread src/node/services/compactionCancellation.ts
Comment thread src/node/services/historyService.ts
@ThomasK33

Copy link
Copy Markdown
Member Author

Regarding 3951413562 (downgrade behavior): I checked the parent build, 76f0ce30cd019568f4f32d3b388dac839984781d. It already resumes this same continuation on its own restart if transcript cleanup fails:

This PR continues to remove the legacy marker and retain/retry failed cleanup. Successful cleanup therefore remains compatible with older builds. The independent sidecar adds restart protection when that transcript write is unavailable; an unmodified older binary cannot honor a file it never reads. This is a limitation of the added guarantee, rather than a new legacy failure state. Guaranteeing it in that older binary would require backported reader support or a successful transcript mutation, which the reported scenario excludes.

I will make this limit explicit in the code and PR risk assessment. I am also addressing 3951413565, the separate valid malformed-record recovery issue, with conservative sanitization and regression coverage.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

Distinguish malformed JSON/schema bytes from storage errors. Recheck corruption under the history write lock, preserve a private quarantine copy, and durably sanitize the continuous journal and pending follow-ups before removing the corrupt cancellation fence. Failed repair remains blocked and retriable; newer cancellation and local persistence debt retain ownership.

Publish repair receipts with the history commit and carry them through follow-up admission/rollback checks, preventing stale snapshots from dispatching during concurrent recovery. Cover corruption, each repair-stage failure, fresh-service recovery, and concurrent Stop. Document that the sidecar supplements legacy cleanup and cannot add support to unmodified older readers.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: I972fea04b4cd6bfc34f3a516b0c73f5d534b2de1
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed review 5133990163:

  • 3951413565 — malformed cancellation: JSON/schema errors now trigger a separately locked repair; access and I/O errors remain fail-closed. Repair rechecks current bytes under the history write lock, quarantines the original bytes with private permissions, durably clears the continuous journal and pending follow-ups, and only then removes the corrupt fence. Failures at every stage retain a conservative fence and can retry. Newer valid cancellation and local persistence debt remain protected. A synchronous history-commit receipt also prevents a dispatcher from sending a stale summary captured before repair.
  • 3951413562 — downgrade limitation: the parent-source comparison shows parent 76f0ce3 already resumes this failed-cleanup state on its own restart. Legacy marker cleanup remains unchanged. The added sidecar protection requires a reader that understands it; the code and PR risks now explicitly state that an unmodified older binary only honors successful legacy cleanup. No new fallback can make that older binary read an unknown file while transcript mutation is unavailable.

Validation: 1,155 session/compaction/history/replay tests and canonical static checks pass. The repair implementation also passed all 1,182 workspace/task/container tests. Malformed startup and direct-dispatch cases were reproduced failing before the fixes; the final focused suite passes 92 tests, including stage failures, fresh-service recovery, concurrent repair/Stop, and public startup repair during goal admission, model preparation, and pre-acceptance row writes.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Malformed cancellation now self-heals by quarantining bytes and durably sanitizing automatic compaction intent before removing the corrupt fence. Review 5133990163's downgrade claim was compared against parent 76f0ce3: that build already resumes the same failed-cleanup state on its own restart. The source evidence and compatibility limit are documented in the response and PR risks; healthy legacy cleanup remains unchanged.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4b17334007

ℹ️ 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".

Comment thread src/node/services/agentSession.ts
Revalidate captured summary identity, pending follow-up, permitted trailing rows, and cancellation under the same cross-process history lock as the continuation append. Apply the condition to both direct user-row persistence and the on-send compaction request that carries its follow-up. Ordinary appends retain their existing path.

Skipped writes refuse through existing admission/rollback handling without creating acceptance receipts. Snapshot-only cleanup failures remain errors. Add real shared-history regressions for repair and cancellation racing preparation.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: I1d2d46d3efa6ab60112c136ae0ac64c528607e9e
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed 3951582944 / PRRT_kwDOPxxmWM6f_Hg4.

Durable-summary follow-ups now pass a compaction-only condition into history append. Under the existing cross-process write lock, the append rechecks the exact summary ID, sequence, and pending payload; allows only preserved copies or this preparation's own recorded snapshot rows after it; and reads current cancellation through the raw reader. A matching Stop, repaired/changed summary, or unrelated tail refuses the append. Malformed/storage failures cannot authorize it, and the locked reader never invokes repairing code.

The same guard covers normal continuation rows and on-send compaction requests, which carry the reconstructed follow-up. A skipped append branches before registering a persisted row or emitting an acceptance receipt. Snapshot cleanup failures and genuine locked I/O errors remain failures, including when rollback changes local ownership. Existing local Stop-before-acceptance rollback semantics stay intact.

Two real backend/history instances reproduced the race before the fix. The focused suite passes all 110 tests, including repair during capture/preparation, forced compaction fallback, shared cancellation, permitted/foreign tail rows, and snapshot cleanup failures. Broader validation passes 1,173 combined session/compaction/history/replay tests, all 1,182 workspace/task/container tests, and canonical static checks.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

The cross-backend repair finding is addressed with compaction-only conditional append under the existing shared history lock. Both direct continuation rows and on-send compaction requests revalidate the exact source, trailing rows, and cancellation before writing. Skipped writes cannot publish an acceptance receipt.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e84781767

ℹ️ 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".

Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/compactionHandler.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts
Recheck shared Stop for direct compaction handoffs, acknowledge Stop only after durable publication, and preserve accepted manual rows when witnessed cancellation cleanup fails. Restore absent heartbeat rollback state only when its captured predecessor identities survive.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: I0e7f9c97d751d5197304584df113347eb20ca38b
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed all four findings from review 5134320929:

  • 3951721691 / PRRT_kwDOPxxmWM6f_eFk: Every compaction handoff now passes a locked append condition. Direct legacy and failed-continuous-apply sends without a summary check shared unresolved cancellation immediately before writing, including forced on-send compaction. A skipped append cannot emit an acceptance receipt or leave preparation snapshots accepted.
  • 3951721701 / PRRT_kwDOPxxmWM6f_eFq: Heartbeat rollback captures predecessor IDs and history sequences before publishing its boundary. When the target is already absent, restoration requires those nonempty identities to remain in active/archive history under the same lock. Foreign clear/replacement and ambiguous empty history cannot restore stale attachments; proven foreign rollback still restores without a duplicate delete event.
  • 3951721707 / PRRT_kwDOPxxmWM6f_eFv: The durable nonce-bearing manual row is the replacement receipt. Failed ancillary deletion is retained for retry while renderer publication, acceptance, and the new turn continue. Authorization distinguishes witnessed cleanup debt from unresolved cancellation. Reads still observe foreign Stop, shared append guards recognize exact active/archive witnesses, and stale receipts cannot replace a newer witness marker. Actual goal-sync errors remain errors.
  • 3951721709 / PRRT_kwDOPxxmWM6f_eFx: Stop begins cancellation publication early, then awaits its durable result before returning success. Failed publication returns an error with retryable debt. The wait follows newer pending mutations, so an overlapping Stop cannot make an older skipped write acknowledge success before the replacement publication commits. Existing test-only lock cycles were changed to release the writer before awaiting Stop; the production lock audit found no caller awaiting Stop under its history lock.

Each reported failure was reproduced before the fix. All 134 focused tests, 1,186 combined session/compaction/history/replay tests, 1,182 workspace/task/container tests, and canonical static checks pass. Independent bounded review also checked the heartbeat/direct guards, overlapping Stop acknowledgment, and witnessed-retirement identity races. The service suite used an isolated Git template after a local inherited commit hook made fixture setup exceed the normal timeout; no test or user configuration was changed.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Addressed all four findings: shared cancellation checks for direct compaction handoffs, positive predecessor identity for absent heartbeat rollback, durable manual acceptance despite witnessed cleanup failure, and durable Stop acknowledgment including overlapping Stops. Focused real-history regressions also protect foreign Stop identity and stale replacement receipts.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1fbb58db38

ℹ️ 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".

Comment thread src/node/services/historyService.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
Reject all unwitnessed cancellation scopes for handoffs without durable source identity. Compare explicit Retry's captured nonce or absence under the history write lock before committing acceptance, while preserving witnessed cleanup debt and local Stop guards.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: I6cfad4dad94e65713e31948a5bf5435d148cf491
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed both findings from review 5134517500:

  • 3951908958 / PRRT_kwDOPxxmWM6f_9AC: A direct handoff without durable source identity now rejects every unwitnessed cancellation scope. It cannot establish that a summary-scoped Stop is unrelated. An exact durable replacement witness still permits subsequent work. The regression uses a second real session whose matched summary cleanup fails and narrows Stop; that failure and owner remain alive until the first session attempts admission, with the scoped sidecar verified immediately before release. Legacy, failed-continuous-apply, and forced-compaction paths all reject the stale handoff.
  • 3951908962 / PRRT_kwDOPxxmWM6f_9AE: Every explicit Retry captures cancellation identity, including absence. A dedicated history acceptance method raw-reads shared cancellation under the existing history write lock and invokes the private guarded update only if identity matches. A foreign newer Stop produces no receipt and no provider call. Captured absence can match an old physical sidecar only with its exact durable replacement witness. No repairing reader or public nested history operation runs inside the lock; ordinary updateHistory is unchanged. Exact witnessed retirement happens after lock release, and local Stop-after-commit checks remain in place.

Both Retry races and all three narrowed direct-handoff races failed before the fix. All 130 focused tests pass, including witnessed old debt, ancillary retirement failure, genuine shared-read/write errors, guarded no-ops, and a local Stop after the atomic receipt. Broader validation passes 1,196 session/compaction/history/replay tests, 1,182 workspace/task/container tests, and canonical static checks. Independent bounded review cleared locking and acceptance ordering.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Both findings from review 5134517500 are addressed. Source-less compaction handoffs reject every unwitnessed cancellation scope. Explicit Retry compares its captured nonce or absence with shared cancellation under the same lock as its guarded acceptance write. Ordinary history updates retain their existing path.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cd43821fc9

ℹ️ 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".

Comment on lines +2594 to +2595
else if (history.data.length === 0)
await this.compactionCancellation.retire(cancellation.nonce);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Revalidate empty history before retiring the Stop

With XUM_ALLOW_MULTIPLE_INSTANCES=1, another backend can append a late compaction summary with pendingFollowUp after this method reads an empty active history but before retire() removes the cancellation sidecar. The intervening witness check does not detect that summary because it carries no replacement nonce, so this stale history.data.length === 0 result deletes the only Stop fence and startup recovery can dispatch the late continuation. Revalidate emptiness while holding the same cross-process lock as the nonce retirement, or make the summary append participate in that cancellation check.

AGENTS.md reference: AGENTS.md:L150-L150

Useful? React with 👍 / 👎.

flag: "wx",
});
if (!isCurrent()) return null;
await this.getContinuousCompactionJournal(workspaceId).clear();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Serialize journal repair with foreign journal writers

With XUM_ALLOW_MULTIPLE_INSTANCES=1 and a malformed cancellation sidecar, this clear is not synchronized with another backend's ContinuousCompactionJournalStore.write(), which uses only its process-local queue. A foreign in-flight compaction can therefore publish its journal after this clear; repair then removes the cancellation fence, allowing that journal to be finalized or recovered despite the conservative Stop repair. Fresh evidence beyond the earlier repaired-follow-up issue is this separate unsynchronized journal write path; journal sanitization and publication need a shared cross-process exclusion or an equivalent cancellation revalidation.

AGENTS.md reference: AGENTS.md:L108-L113

Useful? React with 👍 / 👎.

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: cd43821fc9

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Merge main 7c27339 (#4097) into the compaction coordination branch. Preserve raw reset privacy boundaries and append provenance, guard token-budget single/batch admission, retain prepared request snapshots, and keep committed rollover ownership separate from cancellation retirement.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->

Change-Id: I321cbfd3af5c8466dea2834035c459e440b98827
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant