Repository navigation
🤖 refactor: move terminal attention scheduling into TerminalAttentionNotifier - #7669
Merged
ThomasK33 merged 1 commit intoOct 27, 2026
Merged
Conversation
…Notifier Move-only: TaskService's terminal-attention bookkeeping (notify_on_terminal persists, per-owner drain chains, idle waits, fresh sub-agent edges and the wake-group send backoff) and the methods that schedule over it move into src/node/services/terminalAttentionNotifier.ts. drainTerminalAttention stays on TaskService and runs through a deps closure, so the test harness wrapper still sees every drain. Public, spied and cast-called methods stay as TaskService facades. No behaviour change.
ThomasK33
marked this pull request as ready for review
October 27, 2026 08:00
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 |
|
Codex Review: Didn't find any major issues. Nice work! 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 (28% cached) All 14 tests in 9 files
e2e 0.20.0 · 1m 12s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 8.0k tokens (32% cached) All 3 tests in 3 files
e2e 0.20.0 · 21.8s · 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
…rvice (coder#7674) ## Summary Splits the five longest stop and attempt methods in `TaskService` into smaller private steps, in place. No behaviour change, no new file, no new class. This is PR 6 of the TaskService split (stack 1, coder#7665 to coder#7669, has merged). It prepares PRs 7 to 10, which move the stop latch and the attempt ledgers into their own classes. ## Background PRs 7 and 9 move these methods to new files. The CRAP gate scores a moved method as new, and its move relief allows at most CC 20 (CRAP 30 in CI). The maintainer chose to split all five methods above CC 15 first, so each later move fits with room to spare. | Method (CC before -> after) | New steps (CC) | |---|---| | `recheckWorkspaceStopRelease` 21 -> 7 | `isWorkspaceStopRecordSettled` (module fn) 5, `isStopReceiptDecisionPending` 5, `startStopSettlementReceipt` 4, `releaseWorkspaceStopRecord` 5 | | `createAdmittedSend` 19 -> 4 | `isAdmittedSendStale` 7, `streamEndBlocksNewTurn` 2, `recordAdmittedSendTurn` 5, `disposeSendBeforeAdmission` 5 | | `admitTaskWorkspaceTurn` 17 -> 10 | `admitSendToCurrentAttempt` 8 | | `evaluateAttemptLineage` 16 -> 9 | `ownedAttemptSettlementPhase` 3, `lineageProofFromReceipt` (module fn) 4 | | `readUnownedSettlementProof` 17 -> 10 | `unownedSettlementCandidate` (module fn) 7, `unownedSettlementReceiptGap` (module fn) 4 | ## Implementation - Each method splits along its early returns. The originals keep their names and signatures, because tests spy on several of them. - No await moved and none was added. The token callbacks in `createAdmittedSend` are now one-line arrows that call the new methods, and they still run synchronously. - `evaluateAttemptLineage` now reads the settlement entry through `ownedAttemptSettlementPhase` twice instead of re-reading a `let`. Without the closing wait there is no await between the two reads, so they return the same value. ## Validation - `make crap-check BASE=<stack 1 head>` reports 0 violations and 0 warnings, with and without coverage. Every new helper above CC 4 has at least 69 % line coverage from the existing tests. - The test run `bun test src/node/services/taskService* workspaceTurnManager workflows tools` passes: 3,579 tests, 0 failures. ## Risks Low. The change is a mechanical split of stop-release and send-admission code. A wrong split would show as a stuck stop latch or a refused send. The settlement, stop and admission test suites cover those paths. --- _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 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 27, 2026
## Summary Moves the stop latch out of `TaskService` into a new plain class, `TaskStopLatch` (`src/node/services/taskStopLatch.ts`). The move is move-only: no behaviour change, no Effect tag. `taskService.ts` drops from 21,830 to 21,470 lines. PR 7 of the TaskService split, on top of coder#7674. ## Background `TaskService` is ~21,000 lines. Stack 1 (coder#7665 to coder#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 - **Fields that moved:** `workspaceStopEpochs`, `workspaceStopsInProgress`, `queueRechecksOwedByStopLatch` and `workspaceStopRecords`. `workspaceUserStopEpochs` stays on TaskService, because it counts user Stops for `sendTreeMessage` and is not latch state. - **Methods that moved:** the stop cascade (`beginWorkspaceStop`, `runWorkspaceStopCleanup`, `markWorkspaceStopPersisted`), the release (`recheckWorkspaceStopRelease` and the four steps coder#7674 split out of it), the latch and epoch queries, `deferLaunchWhileStopInProgress`, `releaseRetainedStopLatches` and `waitForWorkspaceStopsToRelease`. - **Facades:** TaskService keeps one-line facades for every seam method (`bumpWorkspaceStopEpoch`, `getWorkspaceStopEpoch`, `latchWorkspaceStopsInProgress`, `isWorkspaceStopInProgress`, `releaseRetainedStopLatches`) and for the two methods tests spy on (`runWorkspaceStopCleanup`, `waitForWorkspaceStopsToRelease`). - **Coordinators stay:** `recordWorkspaceTurnSettled` and `recordWorkspaceTurnSuperseded` stay on TaskService. They write three owners in order (admitted sends, stream-end decisions, stop records). Their stop-record tails are now `releaseCapturedTurn` / `rebindCapturedTurn` calls. - **Deps:** the latch reaches attempt state through call-time TaskService closures (`TaskStopLatchDeps`), so test spies and patches on TaskService still see every call. Stop records and admitted sends move by reference. - **Exported types:** `OwnedTaskAttempt`, `AdmittedSend`, `StreamEndDecision` and `WorkspaceStopRecord` are now exported from `taskService.ts` for `import type`. The new `SettlementReceiptDecision` alias names the existing return type of `decideSettlementReceipt`. ## Validation - I checked the moved blocks against the originals with a whitespace-insensitive comparison, after the `this.x` -> `this.deps.x()` swaps. They match. Statement order is the same, and no await was added. - The new file `taskStopLatch.test.ts` has 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. - Tests changed only their cast targets. - CRAP: `beginWorkspaceStop` and `runWorkspaceStopCleanup` are 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`_ <!-- 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 terminal-attention scheduling and bookkeeping into a new
TerminalAttentionNotifierclass (terminalAttentionNotifier.ts): notify_on_terminal persists, the per-owner drain chains, idle waits, fresh sub-agent edges and the wake-send backoff (6 fields).drainTerminalAttentionstays on TaskService.taskService.tsloses 178 lines in this layer. After the whole stack (#7665 to #7669) it has 21,742 lines, down from 22,800.Background
PR 2 to 5 of the TaskService split stack (#7665 is PR 1). Each layer moves one group of TaskService state and the methods that own it into a plain class that TaskService builds in its constructor. The rules are the same in every layer: public methods stay on TaskService as one-line facades, methods that tests spy on or patch stay reachable on TaskService and the new class calls them through call-time closures, and tests that cast a moved field change only the cast target. No behaviour changes.
Implementation
markBackgroundWorkNotifyOnTerminal,scheduleNotifyOnTerminalPersist,persistNotifyOnTerminalPolicy,schedulePendingTerminalAttentionOwnerDrains,suppressTerminalAttention,enqueueTerminalAttention,scheduleTerminalAttentionDrain,scheduleTerminalAttentionDrainAfterIdle,backOffTerminalWakeSends.enqueueTerminalAttention,schedulePendingTerminalAttentionOwnerDrains). The facades return the inner promise, so they add no ticks.deps.drain, which goes throughthis.drainTerminalAttention, because the test harness replaces that method on the instance.takeFreshTurns(still synchronous, before the drain's first await),isWakeSendBackedOff(the drain's two backoff reads, same expire-on-read logic),trackDrainanduntrackDrain(deferred descendant messages and the withdrawn-turn reconcile),inFlightNotifyOnTerminalPersists(handleStreamEnd).WORKFLOW_TERMINAL_ATTENTION_SWEEP_INTERVAL_MSmoves to the new file, because the backoff needs it. TaskService imports it back for the sweep timer.cutBusyOwnerTurnForSubagentAttention,resolveTerminalWakeCallerSendRestrictions,acknowledgeAgentReportsand the sweep stay. They mostly use other TaskService state.terminalAttentionIdleWait.test.tschange only a cast target. The newterminalAttentionNotifier.test.ts(20 tests) covers per-owner drain order, the idle-wait slot release, fresh edges, backoff expiry and the persist paths.Validation
bun teston taskService, the seven new test files, workspaceTurnManager, workflows, tools and workspaceService: 4,898 pass, 0 fail. Typecheck passes on every layer.make static-check,make lint-eslint,make dry-check: pass.make crap-check: moved methods CC 7 or less.drainTerminalAttentiondrops from CC 106 to 94.Risks
Medium. Terminal attention decides when parents wake. Every ordering point listed in the plan moves unchanged, and the drain body itself does not move.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$53.33