Repository navigation
🤖 refactor: move attempt identity and settlement receipts into AttemptLedger - #7677
Merged
ThomasK33 merged 2 commits intoOct 27, 2026
Merged
Conversation
…tLedger Move the owned attempts, the closure/settlement entries, their listeners, the failed-reawaken settlements owed and the write-only currentAttemptIdByTaskId (#7664, moved as is) from TaskService into a plain AttemptLedger class in attemptLedger.ts, with the methods that own them: attempt begin/close/settle, the receipt decision and write, the no-report boundary, rotation publication, release of reported attempts, the closure and settlement waits and the resume-failure recheck. TaskService builds it with call-time closures. Stop records, send obligations and stream-end decisions stay with their owners and are reached through those closures. beginOwnedTaskAttempt, closeAttemptAdmission, settleOwnedTaskAttempt, decideSettlementReceipt, writeSettlementReceipt, notifyAttemptSettlementListeners and the public waitForAttemptSettlement stay on TaskService as one-line facades, and the ledger calls the spied ones (and inspectAttemptOutcome) back through TaskService. recordWorkspaceTurnSettled/Superseded and currentTaskAttemptId stay on TaskService. rowSupersedes moves to taskUtils so both files can use it. Tests that cast a moved field change only the cast target. No behavior change: moved code is verbatim, statement order is kept and no await is added. Signed-off-by: Thomas Kosiewski <tk@coder.com>
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. |
Member
Author
|
@codex review |
… in AttemptLedger tests
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Codex Review: Didn't find any major issues. Keep them coming! 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. |
Contributor
🟢 e2e stories (Haiku): 14 passed22 agent steps · 7 replayed from cache · 15 model calls · 46.3k tokens (24% cached) All 14 tests in 9 files
e2e 0.20.0 · 1m 14s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 8.0k tokens (22% cached) All 3 tests in 3 files
e2e 0.20.0 · 21.1s · electron · run artifacts 🟢 e2e agent repros (Haiku): no tests selectedSpend: $0.00 of the $1.00 cap (proxy ledger) e2e 0.20.0 · 5.3s · 2 targets (web, phone) · run artifacts |
This was referenced Oct 27, 2026
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 27, 2026
## Summary Moves send admission out of `TaskService` into the `AttemptLedger` class that coder#7677 added. The move is move-only. `taskService.ts` drops from 20,972 to 20,307 lines. PR 9 of the TaskService split, on top of coder#7677. ## Implementation - **Fields that moved, by reference:** `admittedSendsByTaskId` and `reawakeningsInFlight`. The new accessor `sendsOf(taskId)` returns the live set (or a shared empty set), never a copy. `beginReawakening` / `endReawakening` replace the four direct writes, and the end is still a conditional delete. - **Methods that moved:** - Admission: `admitTaskWorkspaceTurn` (with the `admitSendToCurrentAttempt` step from coder#7674), `createAdmittedSend` and its four token steps, `resolveEnqueuedSendDispatch` and `dischargeAdmittedSend`. - Queries: `attemptAdmissionOpen`, `hasPendingAdmissions`, `hasLiveReawakening` and `resolveStreamAttemptAtEvent`. - Lineage: `evaluateAttemptLineage`, `readUnownedSettlementProof` and their module helpers. - Retirement: `claimRetiredAttempt` and `tombstoneUnpublishedReservation`. - **Facades:** TaskService keeps one-line facades for `admitTaskWorkspaceTurn` (seam), `evaluateAttemptLineage`, `resolveStreamAttemptAtEvent`, `claimRetiredAttempt` and `tombstoneUnpublishedReservation`. Tests spy on these or call them through casts. - **Stays on TaskService:** - `captureReawakeningStopFence` and `reawakeningOvertakenByStop` touch only stop epochs and the config, not ledger state. - The coordinators `recordWorkspaceTurnSettled` and `recordWorkspaceTurnSuperseded` now read the sends through `sendsOf`. - **Cross-class calls:** `TaskStopLatch`'s `admittedSends` closure now returns `this.attemptLedger.sendsOf(taskId)`, still through TaskService. The ledger reaches stop state and stream-end decisions through TaskService closures. - **Import cycle:** `ATTEMPT_CLOSURE_SETTLE_WAIT_MS` now lives in `attemptLedger.ts`, and `taskService.ts` re-exports it. Without that, the ledger would import a value from `taskService.ts` and create a runtime import cycle. ## Validation - Statement order, the sync/async shape, the "add to the set last" order in `createAdmittedSend`, and the re-reads after awaits are unchanged. - `attemptLedger.test.ts` adds tests on fake deps. They cover: - send identity and ownership - every refusal leaving no obligation behind - disposal order - first admission wins, with the extra `stopStream` and the recheck - staleness and the dequeue gate - reawakening begin and end - the lineage re-read after the closing wait - the row-changed and row-unreadable refusals in `readUnownedSettlementProof` - each outcome of the claim and the tombstone - Three TaskService test files changed only their cast targets. - CRAP: every moved function scores CRAP 15 or less with coverage. The highest is `claimRetiredAttempt` at CC 15 with 100 % line coverage, and move relief also applies because its facade drops from 15 to 1. ## Risks Medium. Send admission decides whether a message may start a turn under a task's current attempt. A wiring mistake would refuse a valid send or let a stale send through. The attempt-admission, stale-attempt-effects and report-decision suites pass, and they changed only their cast targets. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$68.74`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=68.74 --> Signed-off-by: Thomas Kosiewski <tk@coder.com>
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 27, 2026
## Summary Moves the stream-end decisions out of `TaskService` into a new plain class, `StreamEndLedger` (`src/node/services/streamEndLedger.ts`). The move is move-only. `taskService.ts` drops from 20,307 to 20,023 lines. PR 10, the last PR of the TaskService stop-and-attempt split, on top of coder#7678. Across this stack (coder#7674, coder#7675, coder#7677, coder#7678 and this PR), `taskService.ts` goes from 21,742 to 20,023 lines. The stop and attempt state now lives in three owner classes: `TaskStopLatch`, `AttemptLedger` and `StreamEndLedger`. ## Implementation - **Fields that moved:** `streamEndDecisionsByTaskId` (readonly, because tests read it) and `deferredTaskStreamEnds`. The inline value type of `deferredTaskStreamEnds` is now the named interface `TaskStreamEndDeferral`, with the same fields and comments. - **Methods that moved, with names and statement order kept:** `runStreamEndHandler`, `deferChildStreamEnd`, `registerStreamEndDecision`, `findStreamEndDecision`, `findPendingStreamEndDecision`, `resolveStreamEndDecision`, `pruneStreamEndDecisions`, `reconcileDeferredTaskStreamEnd` and `resolveDecisionOfReportedAttempt`. - **Coordinators stay:** `recordWorkspaceTurnSettled` and `recordWorkspaceTurnSuperseded` stay on TaskService. Their decision loops become one `settleTurn` / `rebindTurn` call each, at the same position, so the order across the three owners does not change. - **Same-tick order:** the stream-end listener in the constructor keeps its one-tick order (snapshot, origin, register, run). - **Stays on TaskService:** `handleStreamEnd`, `inspectAttemptOutcome` and `reawakenInterruptedTask`. A later plan covers these long methods. - **Cross-class calls:** the `AttemptLedger` and `TaskStopLatch` closures `findStreamEndDecision`, `pruneStreamEndDecisions` and `discardStreamEndDecisions` now point at `streamEndLedger` through TaskService. The three classes never call each other directly. - **Facades:** none. No test spies on, patches or casts a moved method. ## Validation - The new file `streamEndLedger.test.ts` has 21 tests on fake deps. They cover: - decision identity - find order - the order of resolve, prune, stop recheck and drain - the prune rules - settle and rebind of turns - handler outcomes (a return gives nonreport, a throw gives indeterminate) - the deferral listener - each branch of `reconcileDeferredTaskStreamEnd` - Three TaskService test files changed only their cast targets. Prettier re-wrapped some of the longer lines. - CRAP: every moved function scores CRAP 13 or less with coverage. The highest is `reconcileDeferredTaskStreamEnd` at CC 13 with 100 % line coverage. ## Risks Medium-low. The stream-end decision says whether a child's last stream ended in a report. A wiring mistake would hold or drop a report, or hold a stop latch. The report-decision, settlement-receipt and child-goal suites pass, and they changed only their cast targets. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$68.74`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=68.74 --> Signed-off-by: Thomas Kosiewski <tk@coder.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moves attempt identity and settlement receipts out of
TaskServiceinto a new plain class,AttemptLedger(src/node/services/attemptLedger.ts). The move is move-only.taskService.tsdrops from 21,470 to 20,972 lines. PR 8 of the TaskService split, on top of #7675. PR 9 adds send admission to the same class.Implementation
ownedAttemptByTaskId,attemptSettlementByTaskId,attemptSettlementListenersByTaskId,resumeFailureSettlementByTaskIdandcurrentAttemptIdByTaskId. The last one is still write-only. It moved as it is, and 🤖 refactor: remove the unread currentAttemptIdByTaskId map and an orphaned comment in TaskService #7664 tracks it.attemptCannotStillReport); the settlement listeners and waits (waitForAttemptSettlement,waitForAttemptClosureToSettle,runUnderEventLockBounded);publishAttemptRotation; andrecheckResumeFailureSettlement.waitForAttemptSettlement(public) and for the methods tests spy on or call through casts:closeAttemptAdmission,decideSettlementReceipt,writeSettlementReceipt,beginOwnedTaskAttempt,settleOwnedTaskAttemptandnotifyAttemptSettlementListeners. Inside the ledger, calls to these methods go back through the TaskService facade, so spies see every call. Each such call is one extra synchronous hop, with no new await.recordWorkspaceTurnSettledandrecordWorkspaceTurnSuperseded, which coordinate three owners. AlsocurrentTaskAttemptId,rotateAttemptForStartupRedrive,inspectAttemptOutcomeandreadAttemptOutcome.TaskStopLatchnever callsAttemptLedgerdirectly. Both reach each other through TaskService closures.rowSupersedesmoved totaskUtils.ts, because importing a value fromtaskService.tswould create a runtime import cycle.AttemptSettlementEntryis now exported forimport type.deferChildStreamEndnow callsaddAttemptSettlementListenerinstead of an inline copy of the same three statements.Validation
attemptLedger.test.tshas 43 tests on fake deps. They cover:runUnderEventLockBounded), 12, 10 and 10, under the CI limit of 15.Risks
Medium-low. This code decides whether a task attempt can still report and writes its settlement receipts. A wiring mistake would refuse or lose a report. The settlement, settlement-receipt, attempt-admission and report-decision suites pass, and they changed only their cast targets.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$68.74