fix(node): a cancel before agent/launch answers ends the agent it started

[akushonkamen]

Summary

A cancel that lands while open_channel still waits for the daemon's agent/launch answer left the agent running on the machine, with the channel stuck in the table and no way for the caller to end it (#990).

• The root cause: the channel is in the table before the launch answer, and no arm caught the cancel. NodePool.open_channel registers the channel into node.channels (raven/node/client.py:588) before it awaits agent/launch (raven/node/client.py:591). Its three except arms (raven/node/client.py:592-604) cover AcpRemoteError, AcpTimeoutError and AcpConnectionError only, so a CancelledError raised while the answer waited passed through all three: the channel stayed in the link's channel table, no agent/cancel ever went out, and the endpoint never reached the caller - the cli launcher's try begins only after open_channel returns (raven/agent/subagent/backends/cli_agent.py:681), and the ACP launcher closes the journal and re-raises (raven/acp_client/pool.py:476-481). The agent went on running under the machine's daemon, and no run record existed for a later start to pick up.
• The fix: read the launch's answer even when the caller is cancelled, then end the agent. The launch request runs as its own task, NodePool._launch_agent (raven/node/client.py:606), awaited under asyncio.shield (raven/node/client.py:589-599). When the caller is cancelled, the arm still awaits that answer, then asks endpoint.end() and re-raises. The daemon answers agent/launch only after it has registered the channel (raven/node/server.py:1495-1498); a cancel sent before that is answered no agent on channel (raven/node/server.py:1477), which end reads as nothing left to cancel, so waiting for the answer is what lets the agent/cancel find the channel. A refused or failed launch keeps its handling (NodeError, the channel popped) and the arm re-raises the cancel. ChannelEndpoint.end keeps a cancel for the next link while the link is down; when this computer is stopping, it leaves the run with the machine's daemon, as it already does when the cancel lands after the answer. The channel leaves the link's table on agent/exit, as it does for a cancel after the answer.
• The regressions hold the launch answer to make the window deterministic. A shared _hold_launch_answers helper (tests/test_subagent_acp_remote.py:226, imported by the cli test at tests/test_subagent_cli_remote.py:42) keeps the AGENT_LAUNCH answer half a second once it is in; the cancel waits for the answered event first, so by the time it lands the answer is already local and the daemon has necessarily registered its channel - the window is hit for certain. Each test then asserts the agent process is gone from the machine and waits for the channel to leave node.channels. One test per entry through open_channel: tests/test_subagent_cli_remote.py:349 (cli) and tests/test_subagent_acp_remote.py:732 (ACP). Both are red on the base commit and green here.
• The window before the daemon registers the channel. The daemon's first launch captures the login environment by running the machine user's login shell (raven/node/server.py:1220, capture_login_env), then spawns, and only then registers the channel; a daemon exits after IDLE_EXIT_S (600 s) without activity, so a later dispatch can be a first launch again. The stand-in ssh runs everything under SHELL=/bin/sh, which the capture skips, so tests/test_subagent_cli_remote.py:370 switches it to bash with a one-second ~/.bash_profile, cancels while the daemon is in that login shell, waits for the daemon's task record and requires that no agent is left. Two cases on a held fake link (tests/test_node_client.py:590, :606) pin the order: agent/cancel only after the answer, and none for a refused launch.
• Two tests that read the run record too early wait for it. test_a_cli_run_left_on_the_machine_when_this_computer_stops_is_picked_up_by_the_next_start (tests/test_subagent_cli_remote.py:857) and test_a_cancel_while_this_computer_goes_on_ends_the_command_and_keeps_no_record (:937) asserted remote_runs.pending() the moment pid.txt appeared, while the record is made once the launch answer arrives; with the answer held, both fail every time. They now wait for the record first, as tests/test_node_stderr_replay.py does on main.

Edges recorded and left out of scope:

• attach carries the same shape (raven/node/client.py:644 registers, Acp* arms only) but no cancel arm; after a cancel there the agent still has a run record on the daemon side that a later attach can find, a different failure mode. Untouched.
• The shutdown stays-window (issue Notes feat(security): label untrusted content and contain injection blast radius #2) keeps its gap: end's stays branch behaves as it does post-answer (the run stays with the machine's daemon), so the missing local run record in that window is unchanged. Stated for the record.

Type

• Fix
• Feature
• Docs
• CI / tooling
• Refactor
• Other

Verification

Docker Linux (Debian bookworm, Python 3.12.12, --init), at this head and with this head merged into current main (edb00f54):

• Red -> green against the base commit: test_a_cancel_while_the_launch_answer_waits_ends_the_agent_too, test_a_cancel_while_the_acp_launch_answer_waits_ends_the_agent_too and test_a_cancel_while_the_daemon_still_prepares_the_launch_ends_the_agent_too each fail there with the agent left under the daemon (1 failed, 1 error), and the two tests/test_node_client.py cases fail there too; all five pass here.

• Removing one construct of the arm at a time turns tests red - the shield 5, the endpoint.end() 4, waiting for the answer 3, the NodeError suppression 1 - over tests/test_node_client.py and the four machine cancel tests (89 tests).

• With the launch answer held, the two record-reading tests fail without the wait and pass with it.

• tests/test_node_client.py, tests/test_subagent_cli_remote.py, tests/test_subagent_acp_remote.py, tests/test_node_server.py, tests/test_node_relay.py, tests/test_node_bridge.py, tests/test_remote_file_tools.py, tests/test_resume_remote.py, tests/test_cli_ops_connection_commands.py: 413 passed, 1 skipped at this head; merged into main, with tests/test_node_stderr_replay.py and tests/test_rpc_machines.py added, 478 passed, 1 skipped in each of three runs.

• Native Windows, Python 3.12.13: tests/test_node_client.py 48 passed, 37 skipped (its machine tests need /bin/sh).

• Gates over origin/main...HEAD: pre-commit, scripts/check_commit_messages.py, commitlint, scripts/check_source_language.py, scripts/check_large_files.py, git diff --check, lint-imports (10 contracts kept), ty check raven/node.

• Relevant tests pass locally

• Relevant lint / type checks pass locally

• User-facing docs or screenshots are updated when needed

Risk

• Security impact considered - none: the new arm runs only on a cancel and ends, through endpoint.end, only the agent that call's own launch started, the path the code already takes elsewhere; no new reach.
• Backward compatibility considered - the arm re-raises after cleanup, so callers see the same CancelledError as before. A cancel that lands during a launch now waits for the daemon's answer before it returns: milliseconds normally, the login shell's run time on a daemon's first launch, and at most the launch's own budget (_LAUNCH_TIMEOUT_S, 60 s) when the daemon does not answer; at shutdown SubagentManager.cancel_all already bounds its wait for cancelled runs.
• Rollback path is clear for risky changes - a plain revert of the squash commit.

Related Issues

Fixes #990

[akushonkamen]

Small housekeeping note on the red CI entry: the "pull request title" job failure was from the PR body, not the code - scripts/check_pr_body.py (via check_pr_body in scripts/commit_lint.py) requires the body to be ASCII-only, since it becomes the squash commit body, and the description used em-dashes (U+2014, 10 occurrences) and one arrow (U+2192) on lines 5-7, 14, 27, 29-31. The title itself passed, and no branch changes were needed.

I have edited the PR body replacing those with ASCII equivalents (- and ->) - wording otherwise unchanged. The re-run triggered by the edit is green: "pull request title" and "commit messages" both pass, alongside all other checks.

[LivXue]

Thanks -- merging this one in maintainer mode, with two commits on top of yours in the branch. I have not rewritten your commit; both sit after it.

Why. The arm sent agent/cancel as soon as the caller was cancelled, but the daemon registers the channel only after Daemon.launch returns: it first awaits the login environment (raven/node/server.py:1220) and the spawn, then Node.launch records the channel (raven/node/server.py:1495-1498). A cancel that arrives before that is answered no agent on channel (raven/node/server.py:1477), ChannelEndpoint.end reads the refusal as nothing left to cancel and clears _cancel_wanted (raven/node/client.py:301-303), and the launch then completes with the agent running.

It is not sub-millisecond. The first launch of each daemon captures the login environment by running the machine user's login shell (capture_login_env), and a daemon exits after IDLE_EXIT_S (600 s) without activity, so a later dispatch can be a first launch again. The suite never reached it because the stand-in ssh starts everything with SHELL=/bin/sh (tests/test_subagent_acp_remote.py:79), which the capture skips.

c6efee26 -- the launch request runs as its own task under asyncio.shield; on a cancel the answer is still read, and only then is the agent ended through endpoint.end(), so the cancel finds the channel. A refused launch keeps its handling; the arm re-raises the cancel. One behaviour change worth knowing: a cancel during a launch now returns after the daemon answers -- milliseconds normally, the login shell's run time on a first launch, and at most _LAUNCH_TIMEOUT_S when the daemon does not answer. The channel leaves node.channels on agent/exit now, as it does for a cancel after the answer, so the last assertion of both of your regressions waits for that instead of reading the table once.

New regression: tests/test_subagent_cli_remote.py::test_a_cancel_while_the_daemon_still_prepares_the_launch_ends_the_agent_too switches the stand-in to bash with a one-second ~/.bash_profile, cancels while the daemon is in that login shell, waits for the daemon's task record and requires no agent left. It fails on the base commit and on this branch before this commit. Two cases on a held fake link (tests/test_node_client.py) pin the order instead of the timing.

870a1f70 -- the two tests your description listed as flaky (test_a_cli_run_left_on_the_machine... and test_a_cancel_while_this_computer_goes_on...) read remote_runs.pending() as soon as pid.txt appeared, while the record is made once the launch answer arrives. Holding the answer fails both every time; they now wait for the record first, as tests/test_node_stderr_replay.py does on main.

Verification, Docker Linux (Debian bookworm, Python 3.12.12, --init), at this head and merged into main (edb00f54):

• The five new or affected cases fail on 9f0e2507 and pass here.
• Removing one construct of the arm at a time turns tests red: the shield 5, the endpoint.end() 4, waiting for the answer 3, the NodeError suppression 1, over tests/test_node_client.py and the four machine cancel tests (89 tests).
• The nine files that cover raven/node and remote dispatch: 413 passed, 1 skipped here; with tests/test_node_stderr_replay.py and tests/test_rpc_machines.py added on the merged tree, 478 passed, 1 skipped in each of three runs.
• Native Windows, Python 3.12.13: tests/test_node_client.py 48 passed, 37 skipped.
• The one red check on the previous head, test_trajectory_upload.py::test_blocked_upload_hits_the_deadline - assert 0 < 0, is unrelated to this change: it is the same failure main hit on 2026-10-16 (baf76335), and the shard's 7188 other tests passed.

The description now records this; it is the squash body, edited so it describes this head rather than the previous one.