Repository navigation
🤖 refactor: move the stop latch into TaskStopLatch - #7675
Conversation
Move the stop epochs, the stop-in-progress latches, the queue rechecks a held latch owes and the stop records (Phase A capture, Phase B cleanup, Phase C release) from TaskService into a plain TaskStopLatch class in taskStopLatch.ts. TaskService builds it with call-time closures; attempt, send and stream-end state stays on TaskService and is reached through them. Seam methods (bump/getWorkspaceStopEpoch, latchWorkspaceStopsInProgress, isWorkspaceStopInProgress, releaseRetainedStopLatches) and the spied runWorkspaceStopCleanup and waitForWorkspaceStopsToRelease stay on TaskService as one-line facades. recordWorkspaceTurnSettled/Superseded and workspaceUserStopEpochs stay on TaskService. 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 |
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. |
|
Codex Review: Didn't find any major issues. Hooray! 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". |
🟢 e2e stories (Haiku): 14 passed22 agent steps · 7 replayed from cache · 15 model calls · 46.2k tokens (24% cached) All 14 tests in 9 files
e2e 0.20.0 · 1m 7s · 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 · 18.2s · 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 |
Summary
Moves the stop latch out of
TaskServiceinto a new plain class,TaskStopLatch(src/node/services/taskStopLatch.ts). The move is move-only: no behaviour change, no Effect tag.taskService.tsdrops from 21,830 to 21,470 lines. PR 7 of the TaskService split, on top of #7674.Background
TaskServiceis ~21,000 lines. Stack 1 (#7665 to #7669) moved five state groups into owner classes. This stack moves the stop and attempt state: PR 7 the stop latch, PRs 8 and 9 the attempt ledger, PR 10 the stream-end decisions.Implementation
workspaceStopEpochs,workspaceStopsInProgress,queueRechecksOwedByStopLatchandworkspaceStopRecords.workspaceUserStopEpochsstays on TaskService, because it counts user Stops forsendTreeMessageand is not latch state.beginWorkspaceStop,runWorkspaceStopCleanup,markWorkspaceStopPersisted), the release (recheckWorkspaceStopReleaseand the four steps 🤖 refactor: split the five longest stop and attempt methods in TaskService #7674 split out of it), the latch and epoch queries,deferLaunchWhileStopInProgress,releaseRetainedStopLatchesandwaitForWorkspaceStopsToRelease.bumpWorkspaceStopEpoch,getWorkspaceStopEpoch,latchWorkspaceStopsInProgress,isWorkspaceStopInProgress,releaseRetainedStopLatches) and for the two methods tests spy on (runWorkspaceStopCleanup,waitForWorkspaceStopsToRelease).recordWorkspaceTurnSettledandrecordWorkspaceTurnSupersededstay on TaskService. They write three owners in order (admitted sends, stream-end decisions, stop records). Their stop-record tails are nowreleaseCapturedTurn/rebindCapturedTurncalls.TaskStopLatchDeps), so test spies and patches on TaskService still see every call. Stop records and admitted sends move by reference.OwnedTaskAttempt,AdmittedSend,StreamEndDecisionandWorkspaceStopRecordare now exported fromtaskService.tsforimport type. The newSettlementReceiptDecisionalias names the existing return type ofdecideSettlementReceipt.Validation
this.x->this.deps.x()swaps. They match. Statement order is the same, and no await was added.taskStopLatch.test.tshas 23 tests on fake deps. They cover latch refcounting, deferred-launch rechecks, capture by reference, the release hold conditions, the receipt write before release, and the wait's listener cleanup and deadline.beginWorkspaceStopandrunWorkspaceStopCleanupare CC 13 and count as new functions. With coverage they score CRAP 13.0 and 13.5, under the CI limit of 15.Risks
Medium-low. The stop latch gates reawakens, peer sends and queued launches. A wiring mistake would leave a stuck latch or release one early. The stop, reawakening, settlement-receipt and attempt-admission suites cover those paths and pass unchanged.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$68.74