🤖 refactor: publish pending compaction state and history atomically - #4183
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c04bf343af
ℹ️ 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".
3aeadcd to
a361023
Compare
Provide a recovery-aware locked pending-state view with verified boundary provenance, exact partial retirement and synchronous cleanup receipts. Keep all runtime consumers unchanged. 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: I7c4844dedd565aa942d28029d96e22c1863cc0ae
Hold pending-state and history ownership through optional preparation, strict final admission, mandatory boundary publication, synchronous receipt capture, and exact failure cleanup. Preserve unsupported future sidecars and guard publication, rollback, and eager rotation against physical lock reclamation. Keep the publisher inactive until producers and consumers adopt the protocol together. 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: I7f414895d0bef93adc71d174d65aea7a1f9e311e
Stage appended summary and tail metadata on new rows so final admission or rename failure leaves caller-owned requests retryable. Publish caller sequences at the durable boundary before synchronous receipt delivery, and reject aliased appended objects before sequence allocation. 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: I18e8d46147b19dab7318d4e8bb0ee6adcf076b73
a361023 to
94de3b4
Compare
|
@codex review Please review current head Generated with |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ 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. |
Adds a local lifecycle for compaction preparation and attachment consumption. Admission is claimed before asynchronous work; only the history commit callback installs a publication. Captured requests share consumption facts so delayed completion, failed enrichment, and heartbeat rollback cannot revive discarded attachments. Runtime activation is isolated in coder#4190. Each publication receives a fresh `compactionPublicationId` committed with its history boundary and reused as the existing pending V1 `writeId`. It distinguishes foreign replacements even when boundary ID, sequence, timestamp, and summary content match. Same-boundary finalization, follow-up clearing, and rotation preserve that occurrence. Loading, acknowledged warmth, request adoption, suppression, and rollback cleanup use this exact identity. Compatible unmarked rows still load persisted attachments; absent-file warmth needs occurrence proof, except for proven initial history. History-only publications retain cleanup authority for attachments they omitted. The store captures private predecessor provenance under the publication lock. Consumption can recover the matching receipt after a failed preparation read, using one queued sidecar read for the retained chain and the existing exact-identity cleanup. A stale cached owner cannot substitute for the locked predecessor. Already authenticated legacy receipts retain their authority; newly inferred cleanup requires a marked occurrence. Copied or mutated callback metadata cannot enlarge the store's authority. Consecutive history-only publications carry unresolved predecessors only through an exact matching local publication, and pruning follows the reachable rollback horizon. Acknowledging an empty request retires omitted attachments without granting them acknowledgment; independently acknowledged warmth remains eligible. Before rollback removes history, a checkpoint under the same locks durably retires already-consumed ownership. If a required read or retirement is unavailable, rollback leaves history unchanged for retry. A consumption epoch makes final admission skip when acknowledgement or discard changes during staging. Older heartbeat rollback skips while a successor owns the boundary, preserving that successor's fallback and payload. Ordinary captures probe sidecar existence in the store queue after earlier publications settle. Local ownership is checked at execution, so a queued history-only publication cannot be missed. Only ENOENT with no local owner skips the extra locked history scan, and every call probes again. Present or uncertain storage, probe errors, warmth, and history-only tokens retain full qualification; there is no negative cache. The merge queue also exposed a lost-abort race in the plugin-install watchdog fixtures. Both fake checkout callbacks now check an already-aborted signal after awaited file creation and before registering their listener. A count-quota regression forces this ordering using the real watchdog. Production quotas, watchdog code, and timeouts are unchanged. Validation: - The foreign-predecessor, queued-capture, and absent-probe FIFO regressions fail before their respective fixes. Queued successful and history-only publications are covered while ordinary absence still avoids history locks and scans. The final lifecycle/store/runtime-preparation run passes 171 tests and 752 assertions, covering restart rollback, legacy receipts, history-only chains, fallback preservation, foreign replacements, resets, future formats, failed cleanup, pruning, and authority mutation. - Canonical `make static-check` passes on combined tree `4638cf400821ae9e9bb3d18ef48fd12b35ff5809`, including both TypeScript configurations, lint, formatting, and documentation checks. Nix is unavailable locally. - The watchdog suite passes 130 tests. The current-main provider/MCP integration passed 411 tests. Earlier combined compaction coverage passed 2,321 tests across 40 files; these groups overlap and are not a unique total. Risk: uncertain storage conservatively withholds attachment context. Failed disk retirement remains local debt until successful cleanup; the in-memory capability does not persist discard intent across a crash. Unknown future formats remain untouched. The pending file stays V1; no new journal, migration, or sidecar version is introduced. This is the remaining pair of the pending-state phase: coder#4181, coder#4183, and coder#4186 have merged. Keep coder#4189 and coder#4190 together through review, CI, and the merge queue. --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable --> --------- Signed-off-by: Thomas Kosiewski <tk@coder.com>
…er#4190) Activates the compaction preparation lifecycle across ordinary, idle, heartbeat, continuous, and recovery compaction. Producers capture admission before asynchronous work and publish boundaries through the same pending-state owner. Immediate and periodic attachments retain their exact qualified receipt for acknowledgement or discard; metadata reads use that qualification too. This completes activation above coder#4189 and removes the handler's duplicate pending-file parser, writer queue, rollback protocol, and skill/read caches. The pending file remains V1. Boundary admission compares the privacy-filtered, archive-aware provider history used to prepare the summary. Restored source history can remain eligible after heartbeat rollback; changed history, generations, partials, or ownership prevent publication. Reset decisions and generation fences share the recovered history locks and physical lease, including explicit clear or replacement of empty history. Successful destructive cleanup removes recognized legacy V1 bytes so preceding versions cannot reload pre-reset attachments. Unknown future formats and current-generation successors remain intact. All compaction producers retire malformed partial content before strict publication. Empty directories at `partial.json` recover through nonrecursive removal after final ownership checks. Nonempty directories, symlinks, valid replacement partials, lost logical or physical ownership, and unrelated I/O failures remain protected. Disposal and carryover fixtures use real history and store publication, including the occurrence identity required for acknowledged warmth. Ordinary-send and emergency-retry rollovers retire carryover after the history writer settles, so cleanup observes the committed generation. Reconciliation also runs when an append commits before reporting an error. Failed appends and current-generation successors retain their protection. If an append succeeds but cleanup fails, the diagnosis is logged and the successful writer result continues through reset and acceptance. Original append errors remain authoritative when both operations fail. Kernel and local reset effects remain ordered before publication. Metadata fetches use a fresh reader and skip the extra history proof only when the sidecar is absent, retaining the normal history fallback and tracked-file data. Every fetch probes again, so later foreign publication is discovered without a negative cache. Ordinary completion continues accounting, acknowledgement, continuous observation, queue drain, and goal work when compaction generation storage is unreadable. `handleCompletion` captures admission and generation before its first history read, retains capture as a Result, and propagates an error only after confirming a compaction request. Successful compaction still publishes against that early generation, preserving the foreign-reset fence. The runtime changes are unchanged by the latest parent repairs. The parent contains exact predecessor retirement for history-only publications and the watchdog-fixture CI repair; this child preserves runtime activation on that corrected foundation and current main `bad3c4f2`. Validation: canonical static checks pass on final tree `4638cf400821ae9e9bb3d18ef48fd12b35ff5809`; the final lifecycle/store/runtime-preparation run passes 171 tests. The new-main provider/MCP integration passed 411 tests. Earlier evidence includes 229 completion/admission tests, all 509 WorkspaceService tests, and 2,321 combined tests across 40 files plus actual preceding-handler compatibility controls. CI is re-evaluated on each published head; earlier passing runs do not establish readiness for this revision. Test groups overlap. Local provider calls were blocked, with loopback HTTP allowed for local MCP fixtures; Nix was unavailable locally. Risk: publication and carryover ownership affect every compaction path; regressions can omit or repeat context. Tests use real files, deterministic barriers, multiple store/service instances, and restart behavior. If durable cleanup remains unavailable after a committed reset, current generation checks exclude stale attachments, but compatible bytes can remain readable by a preceding version after downgrade. Cleanup errors stay logged without misclassifying a committed send. This is the remaining pair of the pending-state phase: coder#4181, coder#4183, and coder#4186 have merged. Keep coder#4189 and coder#4190 together through review, CI, and the merge queue. --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable --> Signed-off-by: Thomas Kosiewski <tk@coder.com>
Separate pending-state preparation and history publication can let another backend replace the fallback before the boundary commits. This adds an inactive publisher that holds the pending queue and both history locks through preparation, final row/partial admission, boundary publication, synchronous receipt delivery, and exact rollback.
Mandatory history publication succeeds independently of optional attachment enrichment. Unsupported future-version sidecars retain their exact bytes on success and failure. Physical ownership checks cover preparation, history rename, rollback, eager archive rotation, and the inherited provenance finalization. Caller sequence metadata changes only at commit, so failed attempts can retry the same prepared rows; duplicate appended object references are rejected before allocation.
This is G2b, stacked on G2a (#4181). The publisher remains inactive until C2 adopts every producer and consumer together, including reset and restart restoration.
Validation of the combined five-layer replay: 2,321 tests and 18,866 assertions pass across 40 files on runtime
013d22c6d68c50be1370e1c4df6b1ccbe776f304over lifecycle29b43c63f53040d43feecf38a36f2ff14b53aa41, based on pinned maina0a2dab98da8efe7e659fb0cb8b9897f376d9641. The final runtime candidateb0abd1481d5dbfb025c484fbf5efc7fdaef73689adds only a mechanical Error-type-guard correction to a diagnostic assertion; its 12 affected rollover controls and targeted ESLint pass, with production unchanged. This covers pending/history ownership, publication, rollback, partial recovery, all runtime compaction paths, token-budget and model-requestednew_contextrollover,session_history, and append provenance. Tests use real local storage with outbound provider traffic blocked. These are combined-phase results, not a separate test run at each intermediate layer. Canonicalmake static-checkpasses on the final runtime candidate; Nix formatting skips because Nix is unavailable. Final coordinated PR readiness remains pending.Risk is concentrated in the shared history-writer extraction and eager rotation path. Existing public behavior remains covered by the history regression suites; this layer does not activate the new publisher.
Integration: candidate
94de3b46d58149075b845ab995a88d471c9b7296replays this PR's complete owning interval onto8e5bc5a8824ef821bb189319030057ce6adeda44without conflicts or manual resolutions. Range-diff is unchanged, and source authors and complete commit messages are preserved. Main's strict partial-read option and newer history behavior remain in the prepared chain. Runtime activation remains owned by #4190.Generated with
xum• Model:unavailable• Thinking:unavailable• Cost:$unavailable