Bugfix/stop resumes after one press - #676
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Pressing Stop in a workspace thread reported "Execution stopped by user." and then the agent picked straight back up a moment later, and only a second press made it stick. A stop handler kills the running CLI and clears _channelQueues, but neither reaches the two things that actually restarted the work. The first is the channel worker's drain loop. A message it has already shifted out of the queue is no longer in _channelQueues, so clearing the queue cannot cancel it, and the loop then spends a status post and a pinned-context fetch (two network round trips) before handing it to the CLI. A stop landing in that window kills a process, announces itself, and is immediately followed by a fresh spawn for the message that was in flight. The second is Claude's todo nudge. A turn that ends with unfinished todos queues itself a "please continue working on them" message. When the stop lands as that turn is finishing, the nudge is queued after the stop wiped the queue, so the agent hands itself its own instruction to carry on. The nudge is skipped when the turn being handled is itself a nudge, which is exactly why the second press stopped things. Base adapters now keep a per-channel stop generation, bumped from the control poll before the adapter-specific handler starts killing things so it is on record for every await in that teardown. Channel workers capture the generation they started under and abandon their drain loop the moment it moves. Claude cancels the stopped plan's todos rather than nudging it back to life, matching what the cursor adapter already did.
The first pass stopped a queued message the drain loop had already taken out of the queue, but it checked at the last point the worker controls — and _handleMessage then spends several more HTTP round trips before the CLI is touched. Session lookup, the "thinking..." status, the decision log and glossary fetches, the channel recap on a fresh session, and the spawn itself. A stop landing anywhere in there passed the worker's check long ago, finds no process registered yet to kill, and the turn goes on to spawn one. That is seconds of window, and it sits right where the UI still shows the agent working, which is when a user reaches for Stop. Base adapters now record the stop generation each in-flight message started under, so an adapter can ask whether a stop landed during this turn as opposed to before it, which is the difference between work to abandon and work the user just asked for. Claude checks at its two points of no return, the stdin write for a reused process and the spawn for a fresh one, and gives up there instead of starting a run the user already stopped. The stop notice is posted from that path too. It is deduped per channel, so it stays silent when the handler already announced the stop, and covers the case it could not, which is a stop that found nothing running because the turn had not spawned yet.
The two earlier commits stop the work an agent was already holding. They do nothing about the other way a stopped agent comes back, which is what showed up in a multi-agent thread and is a different bug with the same symptom. In a multi-agent channel every message goes through the router, including one an agent posted. So the sequence is Stop, both agents get killed, the one that was already mid-post lands its message anyway, the router reads that message and decides the other agent should answer it, and the stopped agent is handed what looks to it like ordinary new work. Nothing in the adapter can tell that apart from a user typing, and it should not try to. By the second press the other agent is dead too, so there is nobody left to do the waking, and that is why the second press looked like the one that worked. The router now rests a channel's agent-to-agent turn-taking once a stop lands in it, until a human speaks there again. A human message is the only thing that lifts it, which is what the button means to the person pressing it. The check sits ahead of the LLM router, so a thread that has been stopped costs no routing call at all, and it reads only the control events newer than the channel's last human message, so it stays bounded however long the channel runs. Human messages are never gated, a stop carrying no channel rests every thread the way the stop-everything path intends, a stop in one channel leaves the others alone, and a control event that is not a stop changes nothing.
Testing on this branch, a stopped thread did not end on the stop notice, and the workspace UI decides whether an agent is still running from the thread's latest message. Anything landing after the notice made a stopped thread read as running again, with the Stop button back. Three things posted after it. Cancelling the stopped plan's todos ran after the notice, and PUT /v1/todos emits the updated list into the channel as a message of its own. That ordering came in with the earlier commit on this branch. The CLI's last stdout is still being parsed when the process dies. Those lines sit queued on pp.pendingLines and went on posting thinking blocks and tool statuses, and a result among them posted the whole answer. A turn whose result came back as the stop landed took the success path and posted its answer too. A user-stopped process now drops its leftover output, keeping only the session id so the next message resumes the conversation. The stop handler waits that output out, cancels the todos, and posts the notice last, with the in-flight turn and the pre-CLI bail going through the same deduped step. A channel-scoped stop also no longer falls through to stopping every channel when the named one has nothing running. It touches that channel only, and still posts the notice there so its UI settles.
Pressing Stop in one thread could kill the same agent's work in another thread. The workspace sends Stop to each participant with the channel it was pressed in, but each of the 21 adapters handled it on its own and many got the scope wrong. codex, aider, amp, mini, hermes, gemini, antigravity and llm-direct ignored the channel and killed every run they had. kimi, cline, cursor and copilot did the same whenever the named thread happened to be idle. So with claude, codex and kimi in both thread A and thread B, stopping A always killed codex in B, and killed kimi in B whenever kimi was idle in A. The fixes earlier on this branch that make the stop notice the last message also lived only in claude.js, so every other adapter still posted leftover output after announcing the stop. BaseAdapter now owns the stop. The control poll hands it to _handleUserStop, which records the stop and mutes the channel before its first await, asks the adapter to stop that one channel through _stopChannelWork, cancels the stopped plan's todos, and posts "Execution stopped by user." last. While a channel is muted the send helpers drop everything for it until its next turn starts, which covers output a killed CLI was still flushing in every adapter at once. The notice is a plain message so the thread shows it, without the completed marker that would push it as the agent having finished. The default _stopChannelWork kills the one child registered for the channel, which covers most adapters, and their own stop branches are gone. claude, pi, openworker, deepseek and llm-direct override it for persistent processes, engine sockets, a shared bootstrap and open API requests, and llm-direct now tags each request with its channel so a scoped stop only aborts that thread's. deepseek still reports a stop it could not confirm as failed. Messages carry the stop generation they were accepted under, so one typed after the stop runs even while the stopped turn winds down, and a repeat stop for a thread that has not started a new turn does not move the generation again. The workspace client re-sends Stop after 3s and would otherwise discard that follow-up. gemini, antigravity, hermes and llm-direct used to swallow every control action in their override. They now get the shared ones such as status and model changes. openclaw and nanoclaw run nothing that can be killed, so a stop there mutes the thread and announces itself, and the answer still on its way is dropped.
Preparing a turn takes several round trips before an adapter starts its CLI or calls the model, such as the session lookup, the thinking status, pinned knowledge and the channel recap. Only Claude checked for a stop that landed in that window. Every other adapter started its run anyway, and although the stop now mutes that run's output, the model was still called and the tokens were still spent. Each adapter now calls _stoppedBeforeStart at its last point before starting, whether that is spawning the CLI, sending the prompt over an engine socket or RPC, opening the API request, or injecting into NanoClaw, and gives up there. Where the caller already knows how a user stop ends, the check hands back that same result, so nothing is retried or reported as a failure. Behind that, BaseAdapter watches each adapter's _channelProcesses. A child registered for a turn that was stopped while being prepared is killed on the spot, which covers any start point that misses the check, including adapters added later. OpenClaw ran one CLI per message but never registered it, so a stop could not end it at all and it spent tokens until it finished. It is now registered, runs in its own process group, and is killed like the others. NanoClaw kept its own stop handling in an override that the previous commit on this branch stopped calling, so a stop there no longer detached the thread. It now detaches through _stopChannelWork, and the stop notice keeps its wording that the container task may keep running, since NanoClaw cannot cancel it.
36c9fa9 to
74d6f4b
Compare
zomux
left a comment
There was a problem hiding this comment.
Approve. The diagnosis is right and verified: the drain loop had already shift()ed the next message so clearing the queue couldn't cancel it; _handleMessage spends several round trips before any CLI exists so a stop landing there found nothing to kill; and Claude's todo nudge re-queued 'please continue' after the stop — which is exactly why the second press worked. Per-channel stop generations captured at worker handoff, _stoppedBeforeStart at each adapter's point of no return, the Proxy-backed late-registration kill (race-safe: only clears the registry entry if it still points at that proc), the per-channel mute cleared at the next turn's handoff, and the repeat-stop guard are all sound. stop-control 92/92, full connector suite 1734/0, backend test_llm_router 31/31.
Two things the description omits that reviewers should know: (1) workspace_mod.py gains a routing gate — after a Stop, no agent→agent turns are routed in that thread until a human speaks again — which closes the multi-agent case where a second agent still finishing its post wakes the stopped one. It's index-backed (idx_events_network_type_target_ts), fails open, and only gates openagents:-sourced messages. (2) Stop handling is now centralized in BaseAdapter for all ~20 adapters and channel-scoped — several adapters previously stopped every channel. Please update the PR body to disclose both.
Non-blocking: a channel-less Stop rests every thread until a human speaks in it, so a thread that never sees a human (routine/agent-only) would lose agent→agent handoffs indefinitely — not reachable from the web frontend today (both Stop callers pass channel), but a raw API/mobile caller could trigger it; a bounded lookback or requiring channel would close it. Release-note items: Stop now cancels the channel's pending/in-progress todos, mutes the channel until its next turn, and every adapter's Stop is channel-scoped.
#656 shares zero commits with this and its connector half is superseded here; only its frontend stop-requests.ts part (not on develop) remains distinct.
Resolves the one conflict in codex.js by taking develop's side: #676 centralised Stop handling in BaseAdapter and removed this adapter's override, which this branch still carried.
The develop merge's conflict block in codex.js held both the stale _onControlAction override (removed by #676) and this branch's new _effectiveModel(); resolving toward develop dropped both.
… can actually use (#695) * fix(catalog) sync the backend provider copy and fix invalid goose and cline model ids The backend image ships workspace/backend/cloud_providers, and its openagents.json still named deepseek-4-flash after the repo-root catalog moved Yumi to minimax-m2.5. The backend drift test caught it, but pytest.yml only runs by hand, so nothing failed on the PR. scripts/sync-registry.js now mirrors cloud_providers/ into the backend too, and agent-connector.yml runs its check when either catalog or a backend copy changes. Model ids the CLIs reject - goose claude-4.6-sonnet and claude-4.5-sonnet become claude-sonnet-4-6 and claude-sonnet-4-5, the Anthropic API ids goose sends as they are - cline claude-opus-4.8 becomes claude-opus-4-8, the id form the cline 3.0.62 binary uses for the Anthropic provider * fix(workspace) stop offering catalog models an agent's endpoint cannot serve The model picker in the agent profile listed the registry's models for the agent type, whatever endpoint the agent actually calls. An agent on a relay or on another vendor's endpoint was offered ids that endpoint rejects, and picking one failed its next task. - The daemon roster reports baseUrlHost, the hostname of the base URL the CLI is pointed at (LLM_BASE_URL, a saved provider variable, or one it inherits). Only the hostname leaves the device. - The agent catalog detail names models_provider when the list references a provider. - The profile panel keeps the list when the host is that provider's own endpoint, and otherwise takes a typed model id. Launchers that do not report a host keep the dropdown as before. * fix(workspace) list the models an agent's endpoint really serves, and only where the pick applies The previous commit replaced the catalog with a typed id for an agent on a relay. This one fills that list from the endpoint itself, and stops offering the picker where the model picked there is never used. Live model lists - A new list_models node command. The daemon asks the agent's endpoint for GET /models (OpenAI) or /v1/models (Anthropic) with the key on the device, and reports only the ids. - POST /v1/cloud-agents/{name}/models does the same for a cloud agent with the stored key. The panel uses it for a custom base URL or provider. - The profile panel shows the endpoint's list for a node agent whose catalog does not fit, with a way to type an id, and falls back to typing when the list cannot be read. Where the pick applies - Only the claude, codebuddy, commandcode and openworker adapters read the workspace model. The registry marks them with workspace_model, and for every other node agent the panel says the model is set in the agent's own configuration, keeping only a way to clear a model saved earlier. - llm-direct reported the workspace model as its model but sent its env model. It now sends the one it reports. * fix(workspace): restore codex model selection * fix(workspace): read agent registry as utf-8 * fix(codex): restore _effectiveModel() dropped by the merge resolution The develop merge's conflict block in codex.js held both the stale _onControlAction override (removed by #676) and this branch's new _effectiveModel(); resolving toward develop dropped both. --------- Co-authored-by: Nebu Kaga <nebu.kaga@openagents.org>
Publishes everything merged since the last core: #686 (Kimi watchdog no longer kills working subagent runs; watchdog actually fires on Windows), #676 (Stop sticks on one press across all adapters), #694 (runtime detection no longer wakes WSL), #703 (Cline runs on Windows without an npm .cmd shim), #704 (Hermes preflight + Windows install), #693 (Codex attachments, relay stall deadlines, resume order), #695 (live model listing for the picker).
Where this came from
Reported from cherie's day-to-day use of a workspace thread. Pressing Stop posted "Execution stopped by user." and the agent then picked straight back up a few seconds later. Only a second press made it stick. The screenshot that started this shows the master posting the stop notice while a member agent went on to deliver a full answer a minute later.
What was actually wrong
Every adapter's stop handler does two things: kill the running CLI process, and clear
_channelQueues[channel]. Neither reaches the work that was actually restarting the agent. There are two separate holes, and they sit back to back.Hole 1 — the drain loop, after a message has left the queue
_channelWorkerdrains queued messages after the current turn:Once
shift()has run, clearing the queue cannot cancel that message. A stop landing in the round trips that follow kills a process, announces itself, and is immediately followed by a fresh spawn for the message that was already in flight. The loop also had no stop awareness at all — after handling one message it went right on to the next.Hole 2 — inside
_handleMessage, before the CLI existsEven with hole 1 closed,
_handleMessagespends several more round trips before it touches the CLI: session lookup, thethinking...status, the decision log and glossary fetches, the channel recap on a fresh session, and the spawn itself (1-2s). A stop landing anywhere in there passed the worker's check long ago and finds no process registered yet to kill, so the turn goes on to spawn one.What fed the holes — Claude's todo nudge
A turn that ends with unfinished todos queues itself a "You have N remaining task(s)... Please continue working on them." message. When the stop lands as that turn is finishing, the nudge is queued after the stop wiped the queue, so the agent hands itself its own instruction to carry on.
_queueTodoNudgeskips when the turn being handled is itself a nudge — which is exactly why the second press worked. The first press stopped a normal turn and got nudged back to life; the second stopped the nudge turn, which had nothing left to re-queue.cursor.jsalready cancelled todos on a user stop instead of nudging.claude.jsnever did.Reproducing it
The window is seconds wide, not milliseconds, and it opens exactly where the UI still shows the agent working.
Natural repro (Claude adapter agent):
Deterministic repro — temporarily widen the window by inserting
await new Promise((r) => setTimeout(r, 20000));after thethinking...status in_handleMessage, then press Stop at any point during those 20 seconds. Works with any message, no todo plan needed.The same shape reproduces without todos at all: send a long task, send a second message while it runs (it gets queued), then press Stop the instant the first answer lands.
Before / after
pendingforever, so a later turn in the channel nudges the abandoned plan backcancelledWhat changed
base.js— per-channel stop generations, bumped from the control poll before the adapter-specific handler starts killing things, so the stop is on record for everyawaitin that teardown._markStopRequested(channel)— bumps the generation and drops the channel's queue. A null channel (workspace-wide stop) bumps a global counter that counts for every channel._channelWorkercaptures the generation it started under and abandons its drain loop the moment it moves, checked both as the loop condition and again right before a drained message reaches_handleMessage._stopRequestedDuringTurn(channel)— lets an adapter ask whether a stop landed during the turn it is preparing, as opposed to before it. That distinction matters: a turn that starts after a stop is the user asking for new work.claude.js_bailOnStopDuringTurn(msgChannel)called at the two points of no return — the stdin write for a reused process, the spawn for a fresh one. Gives up there instead of starting a run the user already stopped, and posts the stop notice (deduped per channel, so it stays silent when the handler already announced it and covers the case it could not)._queueTodoNudgecancels the plan instead of nudging when the channel was stopped.cursor.js.Blast radius
Everything in
packages/agent-connector; nothing in the backend, frontend, or SDK. No public API, no schema, no protocol change. No adapter signatures changed.base.jsdrain-loop fix. Behavior change: after a stop control event for a channel, that channel's worker stops draining queued messages. That is what a stop is supposed to mean, and adapters already tried to express it by clearing the queue.claude.jsgets the in-turn bail. Other adapters keep the pre-CLI window from hole 2._stopRequestedDuringTurnis the opt-in hook for them; porting it is a follow-up, not a regression — they are no worse off than before.pending/in_progresstodoscancelled. Intended (it is what the stop means, and what stops the nudge from resurrecting the plan), but it does change what the todo panel shows after a stop.stop()still posts "Task interrupted — daemon restarting. Send another message to continue." and deliberately does not cancel todos, because that work is meant to resume.Testing
Nine regression tests in
test/stop-control.test.js, driving the stop through the real control-event path rather than poking internals, so they fail against the unfixed code for the right reason rather than on a missing method. All nine are red before and green after.Full package suite: 1617/1622 pass. The single failure,
wsl.test.js"an agent that only exists inside the distro", is already red ondevelop(a WSL test on a Linux host) and unrelated to this change.npm run lintcould not be run — the package has noeslint.config.*and ESLint 10 dropped.eslintrcsupport. Pre-existing and untouched here.