Repository navigation
Conversation
AttemptLedger now owns the send obligations (admittedSendsByTaskId) and the reawakenings in flight, with the code that reads them: admission (admitTaskWorkspaceTurn and its token callbacks), discharge, the stale and dequeue gates, the stream-attempt lookup, lineage and unowned-settlement proofs, and the workflow claim and tombstone. TaskService keeps same-name facades for admitTaskWorkspaceTurn, evaluateAttemptLineage, resolveStreamAttemptAtEvent, claimRetiredAttempt and tombstoneUnpublishedReservation, and reads sends through sendsOf. Stop latch and stream-end state stay with their owners and are reached through TaskService closures. Move only: no behaviour change. Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
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. |
🛡️ 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. |
🟢 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 19s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 7.9k tokens (22% cached) All 3 tests in 3 files
e2e 0.20.0 · 17.4s · 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 |
|
@codex review |
🛡️ 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. |
|
Codex Review: Didn't find any major issues. Chef's kiss. 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". |
## 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>
Summary
Moves send admission out of
TaskServiceinto theAttemptLedgerclass that #7677 added. The move is move-only.taskService.tsdrops from 20,972 to 20,307 lines. PR 9 of the TaskService split, on top of #7677.Implementation
admittedSendsByTaskIdandreawakeningsInFlight. The new accessorsendsOf(taskId)returns the live set (or a shared empty set), never a copy.beginReawakening/endReawakeningreplace the four direct writes, and the end is still a conditional delete.admitTaskWorkspaceTurn(with theadmitSendToCurrentAttemptstep from 🤖 refactor: split the five longest stop and attempt methods in TaskService #7674),createAdmittedSendand its four token steps,resolveEnqueuedSendDispatchanddischargeAdmittedSend.attemptAdmissionOpen,hasPendingAdmissions,hasLiveReawakeningandresolveStreamAttemptAtEvent.evaluateAttemptLineage,readUnownedSettlementProofand their module helpers.claimRetiredAttemptandtombstoneUnpublishedReservation.admitTaskWorkspaceTurn(seam),evaluateAttemptLineage,resolveStreamAttemptAtEvent,claimRetiredAttemptandtombstoneUnpublishedReservation. Tests spy on these or call them through casts.captureReawakeningStopFenceandreawakeningOvertakenByStoptouch only stop epochs and the config, not ledger state.recordWorkspaceTurnSettledandrecordWorkspaceTurnSupersedednow read the sends throughsendsOf.TaskStopLatch'sadmittedSendsclosure now returnsthis.attemptLedger.sendsOf(taskId), still through TaskService. The ledger reaches stop state and stream-end decisions through TaskService closures.ATTEMPT_CLOSURE_SETTLE_WAIT_MSnow lives inattemptLedger.ts, andtaskService.tsre-exports it. Without that, the ledger would import a value fromtaskService.tsand create a runtime import cycle.Validation
createAdmittedSend, and the re-reads after awaits are unchanged.attemptLedger.test.tsadds tests on fake deps. They cover:stopStreamand the recheckreadUnownedSettlementProofclaimRetiredAttemptat 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