NEXT: removing a pending dictation recording stops its upload

Merged
TE
7d ago
+289-39

After #26953 landed (e1f81d2a)

No capture: nothing on screen changes. The pending recording chip and its × were already there; this PR changes only what the host does after a × (it stops the finish, tears down the peer call and cleans up the stage), which the Swift and webview tests cover.

#26953 landed as a squash that already has this PR's first pair: the dictation.recording.cancel {path} op, and the chip's × that sends it, stops the upload before its next chunk and cancels the stage. The merge of feat-cmux-next (6a9fa4bfb23) keeps base's code for that pair. This PR no longer stacks on #26966 or carries its change; #26966 lands on its own.

What base lacked is still here, written on base's code:

  • A cancel during the finish. The stager's finish runs in its own task, held in AgentPaneDictationUploads.finishing while it runs. The × cancels that task, so the remote transfer stops before it commits. The upload answers cancelled, and a cancelled upload is not given back for Retry. Test: AgentPaneDictationUploadTests.aCancelWhileTheFinishIsPendingStopsItBeforeItCommits.
  • A peer call is torn down on cancel. withAgentPaneDeadline runs onTimeout (a connection's cancel()) when the caller is cancelled. AcpmuxStatusClient.start refuses a cancelled caller. Test: AgentPaneDeadlineTests.aCancelledCallerTearsTheOperationDown.
  • file_put_cancel from a fresh task. A cancelled put still drops what the peer staged. Test: AgentPaneFilePutTests.aPutCancelledDuringItsFinishCleansUpFromATaskThatIsNotCancelled. #27073 carries the same cancelStaged helper, so whichever lands second merges clean or with a trivial conflict.
  • A composer that goes away stops the upload. If the composer goes away while a host-made chip is still being made, it calls that chip's stop, and it asks nothing once the upload has answered. Tests: the last two in dictation-recording-hold.test.tsx.

Not carried over: the host-side check that only the chat the upload is for may cancel it. Base's cancel is already scoped to the pane's own model.

The sections below describe the PR before the merge.

Rebased onto #26966's review fix (1b246e2cc82c2146a83bbd38dd2d245bec8e3b12: a failed recording upload keeps the waiting send blocked until Retry or ×). git diff 54ddfcd00f5 bebbe55ce7b equals that change; the only hand merges were doc comments in App.tsx, Composer.tsx and attachmentIntake.tsx, and stop alongside holdsSend in attachLater. Like #26966, recordings over today's stager caps depend on #26409 (peer 1 GiB) and #27073 (Cloud 32 MiB and the peer checks).

Leo's standing order: "Attachments take arbitrarily large files of any type; there is no size cap." A long dictation recording for a chat on another computer streams from the host to that chat's machine (#26953), shown as a pending chip while it goes (#26966). Pressing × on that chip used to drop it from the composer only: the host kept sending the whole file, wasting bandwidth on a large recording and leaving an orphan copy on the chat's machine. Now × stops the host's upload.

What changes

  • Page (attachments.ts stopRemoteRecording, composer/attachmentIntake.tsx attachLater): attachLater takes an optional stop. If the chip's make hasn't answered yet, its × calls stop, and so does the composer unmounting (the chip would have nowhere to land). Once make has answered, stop is dropped, so a later × or a Retry on a failed chip sends no cancel. App.tsx passes stop for the remote recording, which sends dictation.recording.cancel {path} with the same path as the upload. The upload's cancelled answer is ignored because the chip is gone. A refused cancel is ignored too (it already finished, or the host is older).
  • Host request: dictation.recording.cancel {path} is decoded with the same path rules as the upload (absolute, ≤ 4096 bytes). It is added to AgentPageOps.methods, and its safe description doesn't leak the path.
  • Host upload (AgentPaneDictationUpload): respond registers a Running {session, cancelled} in AgentPaneModel.dictationUploads[path] before any await and removes it in a defer. cancel(_:in:) marks it, but only when this pane is the one sending that path and the pane still shows the chat the upload is for. Any other cancel answers dictation.not_uploading, and that covers another pane, a chat the pane has since switched to, and an upload that is no longer running. send(..., cancelled:) checks the flag after the size read, before every chunk and before the finish. Once it is set, send reads nothing more, calls the stager's .cancel(token:) (the same path a failed chunk already takes) and answers cancelled. A cancelled recording is not given back for Retry.

Task ownership and memory (repo rule for new tasks)

  • No new Task. The upload still runs in the page request's own task: the bridge's Task { replyHandler(...) } or the page router's call. The existing per-chunk Task.detached reads stay as they were.
  • Owner of the cancellation: the Running entry, which belongs to that upload's respond. It is added before the first await and removed by defer on every exit (finished, failed or cancelled). The === check keeps a removal from touching anything else.
  • Cleanup: the stager's .cancel(token:) drops what was staged on the chat's machine.
  • Memory: no new O(file) memory. There is still one chunk (≤ 512 KiB) in flight, plus one small entry per running upload.

Chat switch

In this stack a chat switch doesn't drop the pending chip. The composer isn't keyed by session, and a chip that finishes after the switch is handed to the chat it was made for (elsewhere → that chat's kept chips). So the upload rightly goes on. The cancel is sent only when the chip really goes: on × or when the composer unmounts. A cancel sent after the pane moved to another chat is refused by the host, which enforces the rule that only the chat whose recording it is may cancel it.

A cancel during the finish (review fix)

The stager's finish is the remote transfer itself (Cloud: cmux link access upload on the staged file; peer: _acpmux/file_put* to the peer). It now runs in its own task, kept in Running.finishing only while it runs. Running.cancel() cancels it, so a × while the finish is pending stops the transfer before it commits. For Cloud, ProcessRunner's withTaskCancellationHandler kills the CLI child and the finish answers a failure. The upload then answers cancelled and does not give the recording back. A cancelled request task still cancels the finish (withTaskCancellationHandler forwards it), as it did when the finish ran inline. Task ownership: the finish task is the one new Task. Its owner is the upload's send, which awaits it right away, holds it in Running.finishing only while it runs (cleared on return) and cancels it through Running.cancel. No await separates the last cancel check from the task's creation, so a cancel either lands before the finish (and the stage is cancelled) or finds the task.

Rollback isn't possible: no stager can delete a file it already committed. acpmux has only file_put*, and its file_put_cancel drops an upload that hasn't finished. cmux link access has upload and no delete. So in two cases the copy stays on the chat's machine and nothing is attached (the page ignores the answer): a finish that commits before the cancel lands, or a peer transfer whose acpmux exchange doesn't stop on task cancellation. Not verified here: whether AcpmuxStatusClient.call stops a peer exchange mid-flight on cancellation. Its putFileChunked catch sends file_put_cancel on any thrown error.

Commits

  • Restack and compile fix. dep-fleet's compile at 60cf5fd5 failed with AgentPaneDictationUpload.swift: escaping closure captures non-escaping parameter 'stager'. send(_:through:...) now takes through stager: @escaping AgentPaneUploadStager. The fix sits in the first commit that makes the finish a Task, the cancel-during-finish green; its red has no Task and compiled as it was. The PR is rebased onto #26966's 588887b1fe597ca2e1e52f4ab815eeffeac4e965 (O_NOFOLLOW reads, pending uploads keep their chat). The conflicts were resolved by keeping both sides: the cancel checks around the openRegular descriptor, stop together with hostMade, and both sets of tests. The diff over the new base equals the old one plus @escaping, apart from one moved blank line.
    • New SHAs. × stops the upload: red eed99f8b7acf31af2e918af360161c5681db6993, green 87d558e28e840b67070c9329bae0921d2635d7a2. Cancel during finish: red e85b191b9dfb989c2c95d631d71eb74bbbc0a714, green dcd3be135ed84e4155c6bc2d7b4247d32959e741. Peer teardown: red 7b98fb1993fa8de59f5a85c38ce34cac101e09b0, green (head) d1da82630d896a56fb1824d2add09627f881582d.
    • At the head, per file: dictation-recording-hold 8/8, dictation-audio, composer-send-hold, composer-intake-events, composer-kept-attachments pass. tsc, oxlint, oxfmt, check-concurrency.sh and prepush-gate are clean.
    • The SHAs in the items below are the ones from before this restack.
  • Review fix (a peer finish is torn down on cancel, and cleans up). On a cancelled caller, withAgentPaneDeadline now runs its teardown (onTimeout: the NWConnection's cancel() in AcpmuxStatusClient.call and LocalRouterClient) through withTaskCancellationHandler. Before, a cancel ended only the deadline's sleeper, and the group then waited on a socket read that ignores cancellation, as long as the peer took or forever. AcpmuxStatusClient.start refuses a cancelled caller, because a cancelled connection may never report a state. A chunked put's _acpmux/file_put_cancel now runs in a fresh task that doesn't inherit the put's cancellation (one call, bounded by its own deadline and awaited; task-owner noted), so a cancelled finish still drops what the peer staged.
    • Red: 1a9cebe09ac38d6578715b5ba46694e260d6d965, no stubs.
      • AgentPaneDeadlineTests.aCancelledCallerTearsTheOperationDown should fail on tornDown. The test then releases the read itself so the run ends.
      • AgentPaneFilePutTests.aPutCancelledDuringItsFinishCleansUpFromATaskThatIsNotCancelled should fail on !cancelled for the file_put_cancel call.
    • Green (head): 60cf5fd59bcd4e2bd5fb9b17c4866b2ce9075b19. Both should pass. check-concurrency.sh and prepush-gate are clean. Not compiled here.
    • Fleet: swift test --filter 'AgentPaneDeadlineTests|AgentPaneFilePutTests|AgentPaneDictationUploadTests' at both SHAs.
  • Restacked onto #26966's bffa70f164af0ceff2872d23966dbf77096408a4. git diff 9763d8ce40e of the old stack equals this PR's diff over the new base. New SHAs: red 5004d461cc0fc1586549d68fdee62150d481262b, green 4baf5793c2883fed071a6a7eda1f358f1453842a; cancel-during-finish red dd861d3fa657605d97c86d50c26dd2d43645b44d, green 6939b32a67d38a20948c1fd31ceacc6b13ad131e. The SHAs below are the ones before the restack.
  • Review fix (cancel during finish): red b12a35c4a811577ee13df4166a592e45f1347b4f adds AgentPaneDictationUploadTests.aCancelWhileTheFinishIsPendingStopsItBeforeItCommits. It uses a fake stager whose .finish is held pending while the page's cancel arrives and is then released, and a finish whose task was cancelled commits nothing. The test needs no stubs and should compile, then fail on committed.isEmpty, the cancelled code and path == nil. Green 9763d8ce40e2acbcafcfc900b77d06359e529e71: the same test should pass along with the rest of the suite. No web files changed. prepush-gate and scripts/cmux-next/check-concurrency.sh are clean. Not compiled here (lane VM): Swift fleet run swift test --filter AgentPaneDictationUploadTests at both SHAs.
  • After the rebase: red 9743bdac5a66fd058c4c1d24b56c6021168d1a0e, green bebbe55ce7ba8529c42bb3e5148b61cbcbaefa57. At green: CMUX_WEB_TEST_RETRIES=0 CMUX_WEB_TEST_FILES=… ../scripts/ci/run-webviews-tests.sh per file on dictation-recording-hold (7 pass), dictation-audio, composer-send-hold, composer-intake-events: all pass; tsc clean; oxlint and oxfmt (printWidth 120) clean on the changed web files; prepush-gate no new violations. Swift for the fleet: swift test --filter AgentPaneDictationUploadTests at both SHAs.
  • Before the rebase, red: d8ba746fe422d6f63d1dbf389eeffe46f87f7097. Tests plus stubs that compile, so the tests fail on their assertions, not on the build.
    • Stubs: web stopRemoteRecording (asks nothing) and an optional stop on attachLater that is never called; Swift AgentPaneRequest.dictationRecordingCancel(path:) (never decoded; the model answers it unsupported) and its safe description.
    • bun node_modules/.bin/tsc --noEmit -p .: clean.
    • cd webviews && CMUX_WEB_TEST_FILES="test/dictation-recording-hold.test.tsx" ../scripts/ci/run-webviews-tests.sh: 4 pass, 2 fail, on expect(host.asked) (no dictation.recording.cancel is asked on × or on unmount). The guard that an answered upload is never cancelled passes at red, as it should.
    • Mac run needed: swift test --filter AgentPaneDictationUploadTests in Packages/macOS/CmuxNext should compile and fail on the three cancel tests' #expects.
  • Green: 54ddfcd00f5a5f469548e66b047c7793085932b8. The fix (the tree equals the old head fb6c33782ea plus #26559's new head a31b4ac4c62).
    • bun node_modules/.bin/tsc --noEmit -p .: clean.
    • cd webviews && CMUX_WEB_TEST_FILES="test/dictation-recording-hold.test.tsx" ../scripts/ci/run-webviews-tests.sh: 6 pass. dictation-audio.test.tsx still passes 11/11.
    • Mac run needed: the same Swift filter should pass.

Tests

  • webviews/test/dictation-recording-hold.test.tsx (bun, jsdom):
    • The pending recording's × sends dictation.recording.cancel <path>, and the cancelled answer attaches nothing.
    • An upload that already answered is not cancelled by a later Retry.
    • A composer that unmounts while the recording uploads sends the cancel.
  • Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneDictationUploadTests.swift (swift-testing):
    • thePageCancelsWithTheSamePath: decoding, the op list, and the safe description.
    • aCancelMidStreamStopsReadingAndCancelsTheStage: a cancel during chunk 0 means exactly one 512 KiB chunk is read and sent. The last stager request is .cancel(token:), no .finish is sent, the reply is cancelled, and the recording is not given back. A later cancel gets dictation.not_uploading.
    • onlyTheChatThatIssuedTheRecordingMayCancelIt: a cancel from another pane, and one after the pane adopts another chat, are both refused, and the upload completes.

Changelog

  • NEXT: removing a long dictation recording's pending chip, in a chat on another computer, stops sending it to that machine.

Observed / Not run

  • Observed on this lane VM (Node 18, so no vp):
    • CMUX_WEB_TEST_FILES=… ../scripts/ci/run-webviews-tests.sh on dictation-recording-hold, dictation-audio, composer-send-hold, attachment-file-chip and the other intake test files: all pass, except composer-paste-images.test.tsx. It fails the same way with this change stashed: image chips time out on this VM.
    • tsc --noEmit -p . is clean.
    • oxlint and oxfmt (printWidth 120) are clean on the changed files.
    • scripts/cmux-next/prepush-gate.py reports no new violations. CmuxNextApp did not grow.
  • Red observed: the web test file fails at the red commit.
  • Not compiled here: none of the Swift (sources and tests) was built on this Linux VM. It needs a Mac compile plus swift test --filter AgentPaneDictationUploadTests in Packages/macOS/CmuxNext.
  • Not run: the live app with an ssh peer or Cloud chat.
  • No UI change: the chip and its × look the same, so there are no screenshots.

Agent session: agent session

🤖 Generated with Claude Code