[fix] Stop ending a displaced turn as a user Stop [#7760] by xubenqiang · Pull Request #7775 · Agenta-AI/agenta

xubenqiang · 2026-10-24T14:48:45Z

Summary

The runner's alive watchdog aborted a turn with the user-Stop label whenever a heartbeat answered is_current_turn: false. That answer is not a user pressing Stop: it means the platform took the turn away — a steer opened a new turn on the session, a kill ended it, or the API's own watchdog declared it lost. All three ended the same way, labelled as a user Stop, so the runner wrote down a Stop nobody performed and the cause of a lost turn was invisible.

The old comment in sessions/stop-signal.ts explains why it was written that way: before the durable command plane there was no other route for a Stop to reach a run, and a shared label was safe rather than merely tolerable, because a steer wants the warm environment for the turn it starts. That is no longer the case. Every real Stop now arrives as a durable command on POST /cancel, routed to the turn's binding, and the applier aborts the run directly with USER_STOP_ABORT_REASON (sessions/control-channel.ts). What is left on the heartbeat path is displacement, so it gets its own label.

One correction to the issue: it says a run with no project scope is not registered for a direct Stop. It is. registerExecution is called unconditionally after admission, and a stored unknown project matches any caller in findExecution, so POST /cancel already reaches such a run today. The comment above that block said the opposite, and that comment is what the release review read. This PR corrects the comment and adds the tests that pin the real behaviour instead of changing code that already worked.

What changed

sessions/stop-signal.ts: new TURN_DISPLACED_ABORT_REASON and isTurnDisplacedAbort. It is part of isCooperativeCancelAbort on purpose — a displaced turn's harness settled, and its environment is worth keeping warm for the turn that took the session over (the steer case the old comment argued for); a kill destroys the pool entry by key either way. Only the recorded ending changes.
server.ts: the watchdog's onInterrupted aborts with the new label, and both ending branches stop treating a displaced turn as a user Stop. When the run left no terminal record of its own (a displacement that aborted a cold acquire), the backstop persists an execution_lost error instead of a cancelled done, and the envelope is not rewritten into the ok: true / stopReason: cancelled shape a real Stop earns. A turn that already emitted its own done keeps that ending: its harness cancel path is shared with a Stop, so its done.stopReason is still cancelled. Naming the take-over in that record needs a record-contract change across the runner, the API and the UI, which this PR deliberately does not make.
Comments in engine.ts and server.ts that called the heartbeat interrupt the Stop call site.
No dependency, no wire field, no schema change.

Testing

Verified locally

Windows 11, Node 24.21.0, pnpm 10.30.0, in services/runner:

pnpm run typecheck — clean.
pnpm exec vitest run --project unit tests/unit/turn-displaced-abort.test.ts tests/unit/control-command-apply.test.ts tests/unit/session-alive-interrupt.test.ts — 3 files, 48 tests pass.
pnpm exec vitest run --project unit tests/unit/server.test.ts — 67 tests pass.
pnpm exec vitest run --project integration tests/integration/server-smoke.test.ts — 9 tests pass.
pnpm exec vitest run --project acceptance — 2 files, 23 tests pass.
Whole unit suite: 31 files fail on this branch, and the same 31 files fail on the base commit 21d79a07 without this change (git stash, same command, same machine). They are all Windows-only: the inprocess/* suites spawn bash/setsid, several assert POSIX mode bits or symlink behaviour, and one asserts a /-separated path. CI runs them on ubuntu. No failure was added or removed by this branch.

Added or updated tests

tests/unit/turn-displaced-abort.test.ts (new): the label is not a user Stop, cannot be forged from text, still counts as a cooperative cancel, and its settled cancellation still parks.
tests/unit/server.test.ts: one new test — a heartbeat displacement during a cold acquire ends as an execution_lost error, with no cancelled ending and no Stop envelope. The five existing tests that drove a "user Stop" through the heartbeat now deliver it through the run's registered Stop handle, the way POST /cancel does; their assertions are unchanged, so the Stop behaviour they covered is still covered.
tests/unit/control-command-apply.test.ts: a run registered with no project scope is stopped by a cancel command for its project, with the user-Stop label.
tests/integration/server-smoke.test.ts: POST /cancel answers 202 for such a run and aborts it.

QA follow-up

Not run against a live deployment. The steer case (a second message while a turn runs, on the same replica, reusing the warm environment) is covered by the park-policy unit test only, not end to end. One manual pass is worth it: start a turn, steer it, then check that the first turn's transcript no longer says it was stopped.
A take-over and a lost turn cannot be told apart: the heartbeat answer is one boolean. Naming the actual cause needs the platform to say which happened.

Demo

N/A — runner behaviour only, no UI change. The capture below is the local verification listed above, in case you prefer to read it as output:

Checklist

Demo shows the real app running this branch (not a mock-up or recreated UI), or is marked N/A
Relevant tests pass locally
Relevant linting and formatting pass locally — services/runner has no formatter or linter gate (the styling workflow covers api/, sdks/python/ and web/); pnpm run typecheck is its static check and it is clean
I have signed the CLA, or I will sign it when the bot prompts me

Closes #7760

The alive watchdog aborted every run with `USER_STOP_ABORT_REASON` when the heartbeat answered `is_current_turn: false`. That answer means the platform took the turn away: a steer opened a new turn, a kill or the API's watchdog ended it. It only stood in for a real Stop because unscoped runs had no direct delivery, and every real Stop now arrives as a durable `POST /cancel` command. So the heartbeat path gets its own label, `TURN_DISPLACED_ABORT_REASON`. The turn still parks (a steer inherits the warm environment), but its ending is no longer written down as a cancellation: the backstop persists an `execution_lost` error instead of a `cancelled` done, and the envelope is not rewritten into the Stop shape. Tests: the five server tests that drove a "user Stop" through the heartbeat now deliver it as the control channel does, through the run's registered Stop handle, and a new test pins the displaced ending. A control-channel test and an HTTP test pin that a run with no project scope is stopped by `/cancel`. The stale comment claiming such a run is not registered is corrected.

CLAassistant · 2026-10-24T14:48:52Z

All committers have signed the CLA.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

coderabbitai · 2026-10-24T14:49:13Z

No actionable comments were generated in the recent review.

Recent review info

Run configuration

Source mode: Auto
Repository: Agenta-AI/agenta
Branch: main
File: .coderabbit.yaml
Commit: 21d79a0
Layers: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: 94ae1a78-cb91-46c2-9194-fe96c21a1972

Commits

Reviewing files that changed from the base of the PR and between 21d79a0 and aebed70.

Files selected for processing: 7

services/runner/src/engines/sandbox_agent/engine.ts
services/runner/src/server.ts
services/runner/src/sessions/stop-signal.ts
services/runner/tests/integration/server-smoke.test.ts
services/runner/tests/unit/control-command-apply.test.ts
services/runner/tests/unit/server.test.ts
services/runner/tests/unit/turn-displaced-abort.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Summary

Summary by CodeRabbit

Bug Fixes
- Explicit Stop requests now correctly cancel runs, including runs without a project selected.
- Runs interrupted because they were displaced are reported as lost or failed, rather than as user-cancelled. Explicit user Stops remain distinct from displacement and genuine failures.
Tests
- Added coverage for cancellation without a project, displacement during startup, and the distinction between user Stops, displacement, and shutdown.

The runner distinguishes user Stops from heartbeat-reported displacement. Displaced runs persist errors as execution_lost rather than user-cancelled results. Tests cover direct cancellation for registered unscoped runs and displacement during cold acquire.

Changes: Runner stop handling

Layer / File(s)	Summary
Abort signal classification `services/runner/src/sessions/stop-signal.ts`, `services/runner/src/engines/sandbox_agent/engine.ts`, `services/runner/tests/unit/turn-displaced-abort.test.ts`	A displacement abort label and predicate are added. Cooperative-cancel classification includes displacement. Documentation and tests cover parking behavior for settled cancellations.
Direct cancellation for unscoped runs `services/runner/src/server.ts`, `services/runner/tests/unit/control-command-apply.test.ts`, `services/runner/tests/integration/server-smoke.test.ts`	Server comments describe execution registration as unconditional. Unit and integration tests exercise `/cancel` for registered executions without project scope.
Displacement outcomes `services/runner/src/server.ts`, `services/runner/tests/unit/server.test.ts`	The watchdog labels no-longer-current turns as displaced. Settled and thrown displaced runs persist as `execution_lost`; tests distinguish these outcomes from user cancellations.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Severity of issue fixed: Medium

Possibly related PRs

Agenta-AI/agenta#7687: Both PRs address unscoped-run handling at different stages: sandbox teardown and direct cancellation.
Agenta-AI/agenta#7476: Both PRs cover stop handling during cold sandbox acquisition; this PR distinguishes heartbeat displacement from a user Stop.

Merge Risk: Minimal · up to aebed

The changed stop and displacement paths have no identified merge-blocking issue. The PR is ready to merge after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Linked Issues check		[`#7760`] Objective 1, real user Stop reaches unscoped runs through POST /cancel. The PR says `registerExecution` runs unconditionally after admission and that `findExecution` matches an unknown project…	Confirm at the reviewed head: (1) the code path that registers a run with no project scope, and the test that sends `POST /cancel` to such a run and expects a user-stop abort; (2) the `onInterrupted` body in `server.ts` that aborts with `TU…

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title names the main behavior change: a displaced turn should no longer be recorded as a user Stop. It matches the summary, which adds a separate displacement label and changes server handling. Th…
Out of Scope Changes check		The changed files map to [`#7760`]. `sessions/stop-signal.ts` adds the displaced-turn label and its predicate. `server.ts` uses the label in the watchdog and in the two ending branches, and documents un…
Docstring Coverage		Docstring coverage is 60.00% which is sufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 7 files.
Description check		The pull request description directly explains the changes to heartbeat displacement handling, abort labels, cancellation behavior, execution registration, and related tests.

Full details: Title check

Explanation

The title names the main behavior change: a displaced turn should no longer be recorded as a user Stop. It matches the summary, which adds a separate displacement label and changes server handling. The title cites an issue number, but that is not noise that hides the change. The PR objectives cite a different PR number (#7775) than the title (#7760), which is a metadata mismatch to note but does not affect the title's relevance to the changeset.

Full details: Out of Scope Changes check

Explanation

The changed files map to [#7760]. sessions/stop-signal.ts adds the displaced-turn label and its predicate. server.ts uses the label in the watchdog and in the two ending branches, and documents unconditional registration. engine.ts changes only shouldPark documentation, per the change summary. server.test.ts redirects five existing user-Stop tests to the registered Stop handle. Their assertions are unchanged. This supports the change, because the heartbeat no longer delivers a user Stop. New tests cover the label, the unscoped /cancel path, and the cold-acquire displacement. The PR adds no dependency, wire field, or schema change. The PR description's image link is not a code change. No unrelated change was identified in the files and summaries reviewed. I read stop-signal.ts, its test file, and the import region of server.ts directly. The other files were judged from the change summary.

Full details: Linked Issues check

Explanation

[#7760] Objective 1, real user Stop reaches unscoped runs through POST /cancel. The PR says registerExecution runs unconditionally after admission and that findExecution matches an unknown project. It also says the PR adds tests in control-command-apply.test.ts and server-smoke.test.ts. I did not read the registration code or these tests. The server.ts read was truncated at 16000 bytes. This objective is unverified at the reviewed head.

[#7760] Objective 2, a heartbeat displacement ends with a reason other than user Stop. Verified at head: sessions/stop-signal.ts defines TURN_DISPLACED_ABORT_REASON and isTurnDisplacedAbort, and includes displacement in isCooperativeCancelAbort. server.ts imports both symbols. tests/unit/turn-displaced-abort.test.ts checks label identity, separation from the user-Stop label, and parking on a settled cancel. The body of the watchdog onInterrupted callback was not read. Its use of the new label rests on the PR description and the change summary.

[#7760] Partial result. The PR states that a displaced turn which already emitted its own done keeps done.stopReason: cancelled. The issue asks that the transcript stop saying the user stopped the turn. The PR says a record-contract change across runner, API and UI is required and is not made here. The issue does not state that this change is in scope, but the case is unresolved.

No test was checked for the issue's requested takeover ending on a turn that already emitted done.

Resolution

Confirm at the reviewed head: (1) the code path that registers a run with no project scope, and the test that sends POST /cancel to such a run and expects a user-stop abort; (2) the onInterrupted body in server.ts that aborts with TURN_DISPLACED_ABORT_REASON. Decide whether the issue requires a non-cancelled recorded ending for a displaced turn that already emitted done. The PR defers that case, so a reviewer must accept the deferral or require the record change.

Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

mmabrouk

thanks for the pr, and for the careful write-up. typecheck, the focused unit tests and the new /cancel integration test all pass for me on aebed70. i found one thing that needs a change before qa.

the pr assumes every real user stop now arrives on POST /cancel. two live stop paths still reach the runner only through the heartbeat:

api/oss/src/core/channels/commands.py dispatch_stop (the !stop command in slack and telegram) calls streams_service.command with no data and force=False. that resolves to CommandMode.cancel, which runs _displace_turns and clears the alive lock. no durable command is created.
web/oss/src/components/AgentChatSlice/AgentChatPanel.tsx closes a running tab with commandSessionStream({sessionId, projectId}), the same cancel mode. the comment there says closing a running tab is a stop.

with this change, both of those user stops get TURN_DISPLACED_ABORT_REASON. if the stop lands during a cold acquire, the runner now writes an execution_lost error instead of a cancelled ending, and the result envelope is no longer the stop shape. so a stop the user did perform gets recorded as a lost turn, which is the opposite of what #7760 wants.

smallest fix: send these two callers through the durable stop (SessionCommandsService cancel on the api side, cancelSessionExecution on the web side), with one test each. if you would rather keep this pr runner-only, say so here and a maintainer can pick the route.

mmabrouk · 2026-10-24T16:08:02Z

+          // aborts through the execution registry with `USER_STOP_ABORT_REASON`; what is left on
+          // this path is displacement, and saying otherwise writes a Stop nobody performed into
+          // the transcript. See `sessions/stop-signal.ts`.
+          controller.abort(TURN_DISPLACED_ABORT_REASON);


this path still carries real user stops today: the channel !stop command and closing a running chat tab both use the stream cancel mode, which only clears the alive lock. until those callers use the durable stop, labelling this as a displacement records their stop as execution_lost on the cold-acquire path.

xubenqiang · 2026-10-25T09:17:31Z

thanks — you are right, and I checked both: dispatch_stop (api/oss/src/core/channels/commands.py:181) calls streams_service.command with no data, so it resolves to cancel and only clears the alive lock; AgentChatPanel.cancelIfRunning (web/oss/src/components/AgentChatSlice/AgentChatPanel.tsx:123) calls commandSessionStream, the same mode. So this PR cannot claim that every user stop arrives on /cancel yet.

I would rather keep this PR runner-only, as you offered, and let you pick the route for those two callers. I am happy to take that follow-up as its own PR — request_cancel on the api side plus cancelSessionExecution for the tab close, one test each — with one caveat up front: the api session tests need a migrated Postgres, which I do not have here, so that half would be verified by CI.

One thing worth knowing for that follow-up: ChannelInboxDispatcher currently holds only channels_service and streams_service (api/oss/src/tasks/asyncio/channels/inbox.py:126), so the commands service has to be threaded in before dispatch_stop can call it.

xubenqiang · 2026-10-30T15:56:56Z

Thanks for the careful review. I would rather keep this PR runner-only: it changes how the runner treats a displaced turn, and I do not want to widen it into the API and web command routing in the same change.

So the two callers you listed (dispatch_stop in api/oss/src/core/channels/commands.py and the tab-close path in web/oss/src/components/AgentChatSlice/AgentChatPanel.tsx) still need to be routed through the durable stop. Happy to leave that to a maintainer, or to open a follow-up PR for those two call sites plus one test each if you prefer that.

mmabrouk

thanks, keeping this runner-only makes sense. no follow-up pr is needed for those two callers: #7835 has since merged and moved both of them to the durable cancel route. dispatch_stop in api/oss/src/core/channels/commands.py now calls commands_service.request_cancel, and the tab-close path in AgentChatPanel.tsx now calls cancelSessionExecution.

so my earlier finding is resolved upstream, and i have no other change to ask for on aebed70. the next step is on our side: running the held required workflows.

Copilot AI balanced review requested due to automatic review settings October 24, 2026 14:48

Copilot AI reviewed Oct 24, 2026

mmabrouk reviewed Oct 24, 2026

View reviewed changes

mmabrouk reviewed Oct 30, 2026

View reviewed changes

Navigation Menu

Repository navigation

[fix] Stop ending a displaced turn as a user Stop [#7760] - #7775

[fix] Stop ending a displaced turn as a user Stop [#7760]#7775
xubenqiang wants to merge 1 commit into
Agenta-AI:mainfrom
xubenqiang:fix/displaced-turn-abort-reason-7760

xubenqiang commented Oct 24, 2026 •

edited

Loading

Uh oh!

CLAassistant commented Oct 24, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

coderabbitai Bot commented Oct 24, 2026 •

edited

Loading

Summary by CodeRabbit

Uh oh!

mmabrouk left a comment

Uh oh!

mmabrouk Oct 24, 2026

Uh oh!

xubenqiang commented Oct 25, 2026

Uh oh!

xubenqiang commented Oct 30, 2026

Uh oh!

mmabrouk left a comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

xubenqiang commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Testing

Verified locally

Added or updated tests

QA follow-up

Demo

Checklist

Uh oh!

CLAassistant commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

mmabrouk left a comment

Choose a reason for hiding this comment

Uh oh!

mmabrouk Oct 24, 2026

Choose a reason for hiding this comment

Uh oh!

xubenqiang commented Oct 25, 2026

Uh oh!

xubenqiang commented Oct 30, 2026

Uh oh!

mmabrouk left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xubenqiang commented Oct 24, 2026 •

edited

Loading

CLAassistant commented Oct 24, 2026 •

edited

Loading

coderabbitai Bot commented Oct 24, 2026 •

edited

Loading