Repository navigation
say() aborts but nothing marks disposal, so a disposal landing inside the shared message path is ungated (follow-up to #1403, DiffViewProvider.ts:1373) #2378
Description
say() aborts but nothing marks disposal, so a disposal landing inside the shared message path is ungated
Filed from work on #1403 (branch head 1471730), review thread PRRT_...607N6M at src/integrations/editor/DiffViewProvider.ts:1373. The thread was answered with evidence and left open; the residual finding does not belong to that pull request, so it is recorded here instead of being resolved away.
Measured at that head
src/core/task/Task.tsis 6,550 lines.async sayis declared at:2828and again at:2947.say()is gated against abort::2842throws[RooCode#say] task ${this.taskId}.${this.instanceId} aborted.- The same file contains no disposal flag:
isDisposingoccurs 0 times andisDisposedoccurs 0 times insrc/core/task/Task.tsat this head. say()awaitsaddToClineMessagesat:2867;addToClineMessagesis a private method ofTaskand is also called from:2080and:2098.DiffViewProviderguards its own disposal at four points (:1277-1280,:1291-1298,:1301-1303, with a raced read at:1327-1331and:1338-1340) and checks again before the emit at:1350-1355. Its ownsay()call is at:1365-1373, and its catch swallows theAbortErrorthatTask.ts:2842raises.
The gap
The guards around a call site cannot cover the await inside the call. A disposal that begins while say() is already awaiting at :2867 is not abort, so the throw at :2842 does not fire, and there is no disposal predicate anywhere in Task.ts for the message path to consult. The message is appended and posted from a task that is being disposed.
This is the shared message path, not the diff view: the same await is reached from every say() caller, including the two sites inside Task.ts itself. No pull request in the guarded-write or file-safety chains owns it, which is why it is filed here rather than patched in a unit.
What an owner has to decide
- Whether disposal gets its own predicate on
Taskalongside the abort latch, or whether disposal is required to set the abort latch first, so thatsay()'s existing check at:2842is sufficient by construction. - If disposal sets abort, the ordering has to be pinned: which latch is set before which teardown step, and what
DiffViewProvider's catch at:1365-1373is then allowed to swallow, because swallowing anAbortErrortoday means a disposal that arrives mid-message is indistinguishable from a user cancel. - Whether the message path should drop the append, or append and mark, when the latch is set between the check and the await.
Per docs/architecture/task-lifecycle-model.md, a new transition or concurrency class reachable in the lifecycle model needs a named action in the shared reducers under src/core/task-persistence/taskLifecycle.ts, and the in-memory run-state submodel at src/core/task/run-state/runState.ts is the place an abort-or-dispose latch would live. No code change is proposed here, and nothing is asserted about which of the two designs is correct. Suppression counts are unaffected by this issue.
Activity
easonLiangWorldedtech commentedon Nov 9, 2026
Link correction, measured now: the token "thread PRRT_...607N6M" in this issue's title and body is an abbreviation of a review-thread node id and resolves to nothing as written. All 148 review comments on #1403 were read over two pages; none contains that token. Narrowing by content, the disposal-gating finding this issue generalises matches the thread rooted at 4222749246 ("Do not start a diagnostics tail after task disposal", DiffViewProvider.ts, original line 1198) and, for the line anchor, the thread rooted at 4237507176 (line 1373, original line 1309). The current thread sitting on line 1373, rooted at 4503929466, carries only coderabbitai replies (4505586211, 4509940477), so it is not the thread our evidence answered. Both candidates are named rather than one picked, because the abbreviation is the only thing that linked this issue to a thread and it cannot be decoded. The claim does not depend on the thread: it stands on the head the issue names (1471730) and the line anchors in its body. The title now cites the pull request and file rather than the unresolvable token.
[-]say() aborts but nothing marks disposal, so a disposal landing inside the shared message path is ungated (follow-up to #1403 thread 607N6M)[/-][+]say() aborts but nothing marks disposal, so a disposal landing inside the shared message path is ungated (follow-up to #1403, DiffViewProvider.ts:1373)[/+]easonLiangWorldedtech commentedon Nov 9, 2026
Owner ruling on the contract question, and this issue is where it stays until a code owner overrules it.
The finding is accepted. Anchors that resolve: pull request #1403 at head 1471730, src/integrations/editor/DiffViewProvider.ts line 1373, and the asynchronous tail that runs after the save completes and emits through the shared message path. The gap is real: that path aborts, but no observable disposal state exists for a disposal landing inside it, so a caller cannot distinguish "finished" from "finished because the task went away".
Where the fix belongs, and where it does not. Not scattered across the call sites in DiffViewProvider: a per-caller flag check at each await point is the shape that produced the gap, because the next caller adds the next await and forgets the check. It belongs where disposal is already modelled. Per AGENTS.md, lifecycle transitions live in the shared reducers at src/core/task-persistence/taskLifecycle.ts and the in-memory run-state submodel is the pure kernel at src/core/task/run-state/runState.ts. So the fix is a disposal state that the shared message path reads, not a promise each caller must remember to inspect; and any change that makes "disposal began inside the shared path" a representable transition must add or update the named semantic landmark for that concurrency bug class and pass pnpm lifecycle:model-check. That is what makes the fix durable rather than local.
Sequencing, which a reader cannot infer from the issue. src/core/task/Task.ts is the contended file in this campaign: a one-character main-side fix at Task.ts:4437 is already queued behind pull request #2372, and #2372 itself still needs a code-owner merge. We are not opening two main-side pull requests against the same four code owners at once. So this lands as its own main-side unit after #2372 merges, and it does not ride inside #1403, whose review budget is already spent on its own save-path diff.
What stays open, and who decides it. Whether say() should refuse outright once disposal has begun, or whether every consumer must re-check disposal after its await, is a contract choice for the code owners rather than for this campaign: the first says no message may land after disposal begins, the second says a message may land and the reader must notice, and they differ for a webview that is still displaying the task. Both candidates are named in the link correction above, because the abbreviated thread token this issue originally cited resolved to nothing: 4222749246 (the diagnostics-tail-after-disposal thread, original line 1198) and 4237507176 (line 1373) match this finding by content, while 4503929466, which now sits on line 1373, carries only coderabbitai replies and so is not the thread our evidence answered. Until a code owner picks one of those two designs, no code lands for this issue.