Skip to content

fix(*): let the page's stop cancel a delegated result re-entering - #981

Merged
LivXue merged 5 commits into
EverMind-AI:mainfrom
akushonkamen:fix/stop_delegated_reentry_turn
Oct 23, 2026
Merged

LivXue merged 5 commits into
EverMind-AI:mainfrom
akushonkamen:fix/stop_delegated_reentry_turn

Conversation

@akushonkamen

@akushonkamen akushonkamen commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A delegated sub-agent result re-entering the conversation runs as the lane's own scheduler turn and binds no rpc handle. turn.cancel answered {cancelled: false} because _cancellable (raven/rpc/methods/turn.py) reaches only _active_turns and _pending_injects, neither of which such a turn appears in, and the page marked it uncancellable on turn.started (ui-web/src/state/session/stages.ts): the button drew Send, a click queued a message behind the busy turn, and stop() returned silently. The lane itself was cancellable all along -- Lane.cancel_running cancels whoever filed the turn onto it.

The turn is now named on the wire, and the cancel reaches it by that name.

turn.cancel takes an optional turn_id (TurnCancelParams, rpc-schema/openrpc.json, and both generated clients):

  • With turn_id: the lane's running turn is cancelled only when it carries that id -- whether turn.send bound a handle or the runtime opened it. That is the one way to stop a turn that binds no handle. Scheduler.running_turn returns the running turn's id together with a TurnHandle, so the caller reaches it through the same cancel / result pair every submitter gets: cancel ends only the running turn and leaves queued work in its place.
  • Without turn_id: exactly the previous reach -- the active turn.send handle, else the oldest pending inject, else {cancelled: false}. Callers that send no id (the terminal UI, the ACP adapter's session/cancel) keep their old reach to the byte. That is deliberate: an unnamed cancel has nothing to match an id against, and reaching for whatever the lane happens to be running would stop a cron wake, or a delegated turn that began after the request left.

A named cancel also goes past a pending inject to the running turn, which is what makes the button work in the case the composer invites -- busy, so the reader's next message is sent as busy: 'inject'. An inject already merged into the turn ends with it, as it does when a bound turn is cancelled; one still in the lane's mailbox falls back to a turn of its own once the cancelled turn unwinds, so the reader's correction runs rather than vanishing under the stop.

turn.started already carries turn_id; the page stores it (runtime.ts openedTurn) and sends it on stop(), so a late stop names the turn the reader actually saw begin instead of whichever turn is running when it lands.

Waiting for a cancelled turn is now shielded. Every cancel path answers only after the turn unwinds, and a connection closing under the handler doing that waiting used to reach the lane's own future through the await: asyncio cancels it with the waiter, the lane's worker -- its sole resolver -- then raised InvalidStateError at its set_result, and the lane stayed marked running with the queue stalled behind a dead worker. TurnHandle.result awaits asyncio.shield(self._fut), which covers all three waiting paths (turn.cancel named, turn.cancel unnamed, session.interrupt) at one seam.

The stale "not cancellable" comments and the characterisation test asserting that state are updated with the behavior.

Claimed in the issue before starting (comment with the same root-cause reading).

Type

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

Verification

All commands run on this branch at 95a4d5faf.

Behavior, tests/test_rpc_turn_cancel.py + tests/test_spine_scheduler.py together: 75 passed.

The new tests are pinned by mutation, not just by passing -- each remedy was reverted on its own, tests untouched, and the named tests went red:

reverted tests that turn red
the turn_id branch disabled test_turn_cancel_reaches_a_runtime_opened_turn, test_cancelling_a_runtime_turn_leaves_queued_work_queued, test_a_named_cancel_goes_past_an_inject_still_in_the_mailbox, test_a_named_cancel_ends_an_inject_already_merged_with_its_host
asyncio.shield removed test_a_waiter_cancelled_mid_unwind_does_not_wedge_the_lane (all three parameterisations: named turn.cancel, bound turn.cancel, session.interrupt), test_a_cancelled_waiter_leaves_the_turn_and_its_lane_alone
the id comparison dropped (named branch kept) test_a_cancel_naming_another_turn_leaves_the_running_one_alone
the "task already ended" guard dropped in running_turn test_running_turn_is_none_once_the_turn_task_has_ended
unnamed cancel allowed to reach whatever is running (the pre-fix ACP behaviour) test_an_unnamed_cancel_leaves_a_turn_it_did_not_bind_running (delegated, cron)

One test, test_a_named_cancel_reaches_a_sent_turn_as_well, stays green even with the turn_id branch disabled -- a sent turn is still found by the unnamed path, so it cannot distinguish id-routing from the old fallback. It is kept as a behaviour check, not as a regression guard. test_an_unnamed_cancel_of_a_sent_turn_with_a_merged_inject_is_unchanged is a deliberate control for unchanged behaviour.

Contract and checks (CI at this head: 21 pass, 2 skipped, 0 failing):

  • ui rpc contract -> pass; npm run gen:check equivalent is covered there (generated.ts matches the contract), and both ui-tui/src/rpc/generated.ts and ui-web/src/rpc/generated.ts carry the new optional parameter.
  • page checks -> pass, covering the ui-web state-machine and runtime tests.
  • python lint (ruff + lint-imports, 10 kept / 0 broken) -> pass.
  • unit shards 1/4-4/4 and coverage gates -> pass.
  • scripts/check_source_language.py origin/main..HEAD -> exit 0.

Not rebased onto the current main tip; GitHub reports no conflict (mergeable: MERGEABLE), and the merge base's turn.py / scheduler.py hunks are untouched by the commits landed since.

Not run: the full Python suite (~28k tests) and make ci's bridge tree, which this change does not touch.

Risk

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

User-visible behavior: while a delegated result re-enters the conversation, Stop now cancels that turn instead of silently doing nothing, and the turn's row reports it as cancelled by the client.

Backward compatibility: turn_id is optional, and a caller that omits it keeps its previous reach exactly -- a bound turn.send turn, else the oldest pending inject, else {cancelled: false}. That includes the ACP adapter's session/cancel, which fires whether or not a prompt is still open and must not stop a cron wake running on the same session lane. The page and its assets are served from the same raven serve artifact as this server (ws.py static transport), so the two ship together; an old page still works against this server because the id is optional, and a page paired with an older server is not a supported combination -- _Strict models use extra="forbid", so such a server would reject the extra field rather than ignore it.

Security surface is unchanged -- no new endpoints or privileges; the named path reuses the lane's existing cancel path, and the id can only reach a turn already running on the caller's own session lane. Rollback: revert the PR.

Related Issues

Fixes #496

A runtime-opened turn (a sub-agent's delegated result re-entering the
conversation) binds no handle in the rpc surface, so turn.cancel answered
{cancelled: false} and the page marked the turn uncancellable: the go
button drew Send, a click queued a message behind the busy turn, and
stop() returned silently. The lane itself was cancellable all along --
Lane.cancel_running cancels whoever filed the turn onto it.

Reach the turn when no handle is bound: turn.cancel falls back to the
scheduler, cancelling only the lane's running turn (queued work keeps its
place, as a bound handle's cancel leaves it), emitting the one cancelled
frame the sink omits for a cancelled TurnFailed, and holding until the
lane has unwound before answering. The seam is probed, not assumed, so a
scheduler without it answers cancelled: false as before. The page marks
the re-entry turn cancellable on turn.started, so the button draws Stop
and the same turn.cancel reaches it; the stale "not cancellable"
characterisation is updated with it.
@akushonkamen
akushonkamen force-pushed the fix/stop_delegated_reentry_turn branch from 65b9841 to 6618e3d Compare October 20, 2026 04:26
@LivXue
LivXue requested review from 0xKT, LivXue and gloryfromca October 22, 2026 03:00

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the full diff and the surrounding RPC cancellation, scheduler lane lifecycle, delegated-result submission, and web turn-state paths. I also checked the relevant history and callers, backward compatibility with scheduler fakes, the AGENTS.md and Runtime/Web UI context constraints, architecture boundaries, and that the tests were extended rather than weakened.

Verification:

  • uv run pytest tests/test_rpc_turn_cancel.py -x - 23 passed after syncing the declared dev extra (the first attempt could not import raven_everos from the unsynced environment)
  • npm test -- --run src/state/session/pipeline.events.test.ts src/state/session/runtime.cancel.test.ts - 45 passed
  • git diff --check github/main...HEAD - passed
  • GitHub CI is green, including all unit shards, page checks, RPC contract checks, lint, and coverage

… a cancelled wait

turn.cancel takes an optional turn_id. With it, the lane's running turn is
cancelled only when it carries that id, whoever opened it, and pending
injects are passed over: one already merged ends with the turn, one still
in the mailbox falls back to a turn of its own, as when a sent turn is
stopped. Without it, the reach is what it was before: the bound handle or
the oldest pending inject, else cancelled: false. So an ACP session/cancel
no longer stops a cron wake on the session's lane, and a late stop cannot
land on the delegated result that started after it was sent.

The page keeps the id turn.started carries and names it on stop; a turn a
send opened is stopped without one, as before.

TurnHandle.result shields the lane's future. A waiter cancelled mid-await
(a connection closing under the handler) used to cancel the future the
worker resolves, so the worker raised InvalidStateError and died with the
lane still marked running and its queue stalled. The fix covers the named
cancel, the bound turn.cancel and session.interrupt, which all wait there.

Scheduler.running_turn replaces running_future and cancel_running and hands
back a TurnHandle, so no caller awaits the lane's own future directly.
The payload builder still described the bound turn as the only source of the id, which stopped being true once a named cancel and a pending inject could supply it.
Without the live-task check, a named cancel landing between the turn's task ending and the worker resolving it would report a finished turn as stopped; nothing failed when the check was removed.
@LivXue
LivXue requested a review from gloryfromca October 22, 2026 06:59

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: preserve a replacement turn when a stale named cancel is refused.

I reviewed the delta from 6618e3da93eb and rechecked the full affected cancellation path: the RPC schema/models and generated clients, scheduler lane/future lifecycle, named versus unnamed callers, delegated re-entry and inject behavior, page state transitions, relevant history, backward compatibility, the Runtime/Web UI context and architecture constraints, and whether tests were weakened. The new turn-ID matching and shielded future are otherwise well covered, but the page does not consume the new cancelled: false result safely.

Verification:

  • uv run pytest tests/test_rpc_turn_cancel.py tests/test_spine_scheduler.py -x - 75 passed
  • npm test -- --run src/state/session/pipeline.events.test.ts src/state/session/runtime.cancel.test.ts - 48 passed when rerun alone; an earlier parallel run had one 5-second timeout with no assertion failure
  • npm run gen:check - generated client matches the 240-method contract
  • npm run type-check - passed
  • git diff --check github/main...HEAD - passed
  • GitHub CI is green

Comment thread ui-web/src/state/session/runtime.ts Resolved
…verMind-AI#981)

The stop button names a runtime-opened turn by id, and the server refuses
the id once that turn has ended. The reply handler ignored the result and
always went idle and drained the queue, so a delegated turn that opened
between the press and the reply lost its streaming phase and its stop
button, and a queued message was released into it.

A refused reply now leaves the page alone when the phase has already moved
off cancelling, which only a newer turn's opening frame does. A refusal
with nothing newer still goes idle and drains, so the page never waits in
cancelling.

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

The stale-cancel race is fixed: a refused named cancel preserves any replacement turn whose opening frame has already moved the phase past cancelling, while the no-replacement path still returns the page to idle and releases its queue. I checked the delta, active and parked runtime phase ownership, the surrounding cancel/event callers, backward behavior for accepted and failed requests, the Web UI context and repository rules, and that the new tests exercise the reported interleavings without weakening existing coverage. My prior thread is resolved.

Verification:

  • npm test -- --run src/state/session/runtime.cancel.test.ts src/state/session/pipeline.events.test.ts - 51 passed
  • npm run type-check - passed
  • git diff --check github/main...HEAD - passed
  • Page checks, UI RPC contract, lint, commit checks, and the other completed CI jobs are green; the full unit shards were still running when reviewed

@akushonkamen

Copy link
Copy Markdown
Contributor Author

Thanks for the quick follow-up review and for confirming the stale-cancel race is fixed in c2228d8e4850. Glad the added tests cover both the delegated and send-opened replacement interleavings as well as the no-replacement path. I appreciate the thorough verification across type-check, contract checks, and CI.

@LivXue

LivXue commented Oct 22, 2026

Copy link
Copy Markdown
Member

Windows compatibility review of head c2228d8e4850718ebfccc184521f38a46e51e267 (target base 7376de6828cca99668ac6199dc32940a8517237a, merge base 3c5b47421dcf2b09f966a91ec24465bb9603ddc4).

Code inspection: no actionable Windows compatibility issue found in the inspected scope. Read the complete 12-file local PR diff, description, all discussion pages/threads, scheduler task/future ownership and cancel callers, RPC models/schema and generated clients, delegated submission, TUI cancellation and web state/queue handling. Also checked the target-only changes in the cancellation paths. The changes use in-process asyncio cancellation and JSON turn IDs; no new POSIX API, shell or filesystem dependency is introduced. The existing stale-cancel discussion is resolved at this head; it is not re-filed here.

Native Windows verification (CPython 3.13.12, default loop observed as ProactorEventLoop): 75 cancellation/scheduler tests, 500 RPC contract/registration tests and 51 web tests passed. TypeScript, generated-client check (240 methods) and local diff whitespace check exited 0. From ui-web, ran npm.cmd ci --ignore-scripts --cache C:/Users/livxu/AppData/Local/Temp/raven-windows-review/a5c63052057e4f91bb839e4a58944859/web-cache --no-audit --no-fund, npm.cmd test -- src/state/session/runtime.cancel.test.ts src/state/session/pipeline.events.test.ts --no-file-parallelism, npm.cmd run type-check, and npm.cmd run gen:check (all exit 0). The loop observation used uv run --frozen --no-sync python -c "import asyncio; loop=asyncio.new_event_loop(); print(type(loop).__name__); loop.close()".

Source integrity/limits: Git fetch timed out. Used the GitHub source archive pinned to this exact head in an isolated, unattached checkout, then reconstructed its content and file modes into Git tree 07ea508a14a30c32676081bc984d2d694af3d238, matching the head commit's tree before generating the complete local diff. Tests use stub runners, mocked RPC/browser facilities and simulated waiter cancellation; no live LLM, real WebSocket disconnect/browser walkthrough, TUI/ACP end-to-end, subprocess shutdown or full repository suite was run. This is Windows coverage of the inspected changes, not a general merge verdict.

Exact verification commands and results

Commands ran in an isolated checkout pinned to the reviewed revision, with temporary profile, Raven/OAuth home and test directories. Imports were checked against that checkout. UTF-8 mode was enabled.

$run = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\a5c63052057e4f91bb839e4a58944859'
$env:UV_PROJECT_ENVIRONMENT = Join-Path $run 'envs/review'
$env:UV_PYTHON = '3.13'
$env:PYTHONUTF8 = '1'
# 981-cancel: 75 passed, 0 failed, 0 skipped; exit 0
Set-Location -LiteralPath 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\a5c63052057e4f91bb839e4a58944859\pr981\head'
$env:PYTHONPATH = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\a5c63052057e4f91bb839e4a58944859\pr981\head'
uv 'run' '--frozen' '--no-sync' 'pytest' 'tests/test_rpc_turn_cancel.py' 'tests/test_spine_scheduler.py' '-n' '0' '-x' '--tb=short' '--basetemp' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\a5c63052057e4f91bb839e4a58944859\checks\981-cancel\pytest' '--junitxml' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\a5c63052057e4f91bb839e4a58944859\checks\981-cancel\junit.xml'

# 981-contract: 500 passed, 0 failed, 0 skipped; exit 0
Set-Location -LiteralPath 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\a5c63052057e4f91bb839e4a58944859\pr981\head'
$env:PYTHONPATH = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\a5c63052057e4f91bb839e4a58944859\pr981\head'
uv 'run' '--frozen' '--no-sync' 'pytest' 'tests/test_rpc_schema_match.py' 'tests/test_rpc_registration.py' '-n' '0' '-x' '--tb=short' '--basetemp' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\a5c63052057e4f91bb839e4a58944859\checks\981-contract\pytest' '--junitxml' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\a5c63052057e4f91bb839e4a58944859\checks\981-contract\junit.xml'

$bare = Join-Path $run 'objects.git'
git --git-dir=$bare diff --check 7376de6828cca99668ac6199dc32940a8517237a 07ea508a14a30c32676081bc984d2d694af3d238 # exit 0

@LivXue
LivXue merged commit 644da8f into EverMind-AI:main Oct 23, 2026
25 checks passed
0xKT pushed a commit that referenced this pull request Oct 26, 2026
…1078)

## Summary

A follow-up to #981. A stop that names a turn which has just finished
is refused (`{cancelled: false}`), and the web UI keeps the newer
turn's stop button only once that turn's opening frame (`turn.started`
or `message.start`) has moved it out of `cancelling`. That frame waits
out the subscription's 16 ms coalescing window
(`raven/rpc/subscriptions.py`), while the RPC reply is written as soon
as the handler returns (`raven/rpc/transports/ws.py`), so a refusal
handled within one window of the next turn starting reaches the web UI
first: it goes idle, sends its queued message under the turn now
running, and the late frame then puts it back to streaming.

The refusal now says whether the lane is still busy. `TurnCancelResult`
gains an optional `busy`, set on both refusal paths of `turn.cancel`
(`_refused` in `raven/rpc/methods/turn.py`): whether the lane still has
a turn to run -- one running, one queued, or an inject that will run as
a turn of its own. `Scheduler.is_busy` answers it through `Lane.busy`;
unlike `has_inflight` it does not count a turn that has stopped and
only waits for the worker to resolve it, so a lane with nothing behind
a finished turn reads as free. An accepted cancel does not carry the
field. `rpc-schema/openrpc.json`, `raven/rpc/models.py` and both
generated clients carry it, and the offline fixture library's shape
gate lists it as unsent, since the offline stop always lands.

On `busy: true` the web UI (`stop()` in
`ui-web/src/state/session/runtime.ts`) keeps the conversation busy, with
no stop button and its queue held, until the newer turn's first frame
names it; a turn that sends no opening frame, such as a cron wake,
keeps it busy until the frame that ends it, which every turn on the
lane sends. `busy: false`, or a server that does not send the field,
releases it as before.

The hold is now correlated with the turn it is holding the lane for,
which is what the first version of it missed, and which the review of
it found. A refusal is written while the frames wait out the
subscription's window, and the turn the reader stopped had already
ended when the server read the cancel, so that turn's completion
arrived behind the refusal: the page held the phase off `cancelling` to
keep the queue from going out under the turn that was starting, and the
completion stage drops a late completion on that phase -- it folded it
instead, which released the queue into the turn that was starting. The
page knows that turn's id, from the frame that opened it
(`message.start` for a turn a send opened, `turn.started` for one the
runtime opened), so a frame naming it is now dropped rather than folded:
the stop already folded that turn. Any other frame ends the hold,
because the turn that ended is not the one the hold was waiting for.

The hold also keeps the question of the turn it was waiting for. A hold
made the machine busy without a turn running in it, so the
`message.start` that opens that turn read as a message this window had
already drawn: its question was never drawn and its bubble was never
moved out of the ended turn. Both happen now
(`ui-web/src/state/session/stages.ts`).

`busy` also answers for the lane it resolved to and for no other: the
direct-chat smoke case binds the real scheduler, so it cannot pass
through the "no scheduler, so not busy" fallback, and a scheduler test
asks a second lane the same question.

`test_a_waiter_cancelled_mid_unwind_does_not_wedge_the_lane`, from
#981, waited without a bound for the cancel to reach its turn, so
disabling the named path hung the run instead of failing it; that wait
is now bounded at 5 s.

## Type

- [x] Fix
- [ ] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [ ] Other

## Verification

Run at this branch's head on `main` at `8a7ea1ccf`, in the branch's own
environment (`uv sync --frozen --all-extras`, and `npm ci` in `ui-web`
and `ui-tui`).

- `uv run --frozen --all-extras pytest tests/test_rpc_turn_cancel.py
  tests/test_spine_scheduler.py -q` -> 80 passed.
- `uv run --frozen --all-extras pytest tests/test_rpc_schema_match.py
  tests/test_rpc_registration.py -q` -> 522 passed.
- `uv run --frozen --all-extras pytest -q -n 4` -> 34330 passed,
  8 failed, 182 skipped, 2 xfailed. The eight fail the same way
  without this change: the five in
  `tests/test_config_update_providers.py` read this machine's own
  `HTTP(S)_PROXY` and pass (193 of 193) with those variables unset, and
  these three fail on an export of `main` at `8a7ea1ccf` run the same
  day:

`test_install_script.py::test_resolve_node_dir_answers_each_case_it_exists_for`,

`test_knowledge_parser_excel.py::test_a_legacy_workbook_is_converted_and_read`,

`test_subagent_node_runtime.py::test_what_cannot_be_read_names_nothing`.
- `ui-web`: `node node_modules/vitest/vitest.mjs run` -> 253 files,
  4231 passed; `npm run gen:check` (the 248-method contract),
  `npm run lint` and `npm run type-check` -> exit 0; the `page checks`
  steps (`npm run build`, `python3 ui-web/build.py`, `check-page.mjs`,
  `check-css.mjs`, `check-class-namespace.mjs`) -> OK.
- `ui-tui`: `npm run lint`, `lint:rpc`, `lint:i18n`, `type-check` and
  `build` -> exit 0; the suite after its `pretest` build -> 144 files,
  2249 passed, 13 skipped.
- Python lint: `ruff check`, `ruff format --check`, `lint-imports`
  (10 kept, 0 broken), `ty check` and `deptry` -> pass.
- `scripts/check_source_language.py origin/main...HEAD`,
  `scripts/check_large_files.py origin/main...HEAD` and
  `scripts/check_commit_messages.py origin/main..HEAD` -> exit 0.

The new behaviour is pinned by mutation: each remedy was reverted on
its own at this head, tests untouched, and these went red.

- `Lane.busy` without its queue, without its mailbox, or asking
  `running_future` instead of `running_turn`: one case each of

`test_a_lane_is_busy_only_with_what_waits_behind_a_turn_that_has_stopped`
  (`append`, `inject`, nothing behind).
- `busy` always false:
  `test_a_cancel_naming_another_turn_leaves_the_running_one_alone`,
  `test_an_unnamed_cancel_leaves_a_turn_it_did_not_bind_running`
  (`delegated`, `cron`).
- The named refusal answering `{cancelled: false}` again:
  `test_a_cancel_naming_another_turn_leaves_the_running_one_alone`,
  `test_a_refused_cancel_with_nothing_left_on_the_lane_says_so`; the
  unnamed one: the six test cases that assert its reply.
- The web UI's `busy` branch dropped: "stays busy for a newer turn
  whose opening frame the refusal outran", "lets a busy conversation go
  when the turn it waited for ends without ever opening"; a stop
  offered on it: the first of those; `busy: false` read as busy: "lets
  the conversation go when a refused id had nothing newer behind it:
  the server says the lane is free".
- With the wait bounded, disabling the named path of `turn.cancel`
  fails `test_a_waiter_cancelled_mid_unwind_does_not_wedge_the_lane`
  (named `turn.cancel`) beside the four tests that already caught it,
  instead of hanging the run.

The hold's own remedies are pinned the same way, one construct at a
time rather than by reverting the whole of them.

- The hold not correlated with the turn it holds the lane for: five of
  the six new web cases fail -- "stays busy when the refusal lands
  before the turn it stopped completes", "stays busy when a send
  opened the turn the refusal was about", "drops the late unwind of the
  turn the hold was about", "draws the question of the turn the hold
  was waiting for", "moves the bubble of a mid-turn message under the
  turn the hold was waiting for".
- `Scheduler.is_busy` answering for any lane instead of the one it was
  asked about: `test_is_busy_answers_for_the_lane_it_is_asked_about`
  and `test_an_untargeted_cancel_does_not_touch_a_direct_turn` (the
  smoke case, with the real scheduler bound) go red.
- Deleting the completion stage's drop of the held turn's own frame, the
  error stage's, the `message.start` draw, either `turnId` assignment,
  or either half of the hold's own state turns its own case red.
- The hold's flag is read at one place -- the `message.start` stage --
  so each of the writes that end it is individually redundant with the
  others, and removing one of them leaves the suite green. Removing all
  of them at once turns "does not draw a question the machine already
  has once the hold is over" red, and that is what pins the end. They
  are kept because the hold has to end at the frame that names the
  lane, not at the next one that happens to arrive.

At this head, on Windows with Python 3.12.13 and the worktree's own
`npm ci`:

- `pytest tests/test_rpc_turn_cancel.py tests/test_spine_scheduler.py
  tests/test_rpc_schema_match.py tests/test_rpc_registration.py
  tests/integration/test_direct_chat_smoke.py -q -p no:randomly` -> 608
  passed, 3 failed. The three are the agent-name fixture mismatches
  (`raven` against `Raven-General`) that fail the same way on the
untouched tree:
`test_direct_chat_round_trip_leaves_records_and_a_handoff`,
  `test_a_direct_turn_registers_the_instance_it_addressed`,
  `test_a_turn_on_an_instance_lane_still_records_against_the_session`.
- `ui-web`: `node node_modules/vitest/vitest.mjs run` -> 253 files,
  4238 passed, no `Errors` line beside the summary; the second test
  root, `node node_modules/vitest/vitest.mjs run --no-file-parallelism
  scripts/gates` -> 51 files, 229 passed; `npm run type-check` -> exit
  0; `npm run lint` -> 0 errors and 4 warnings, all of them
  pre-existing and in a file this change does not touch; `npm run
  gen:check` -> the 248-method contract matches.
- `ruff check` and `ruff format --check` over the lint targets ->
  pass; `lint-imports` -> 10 kept, 0 broken; the repo's own gates --
  `scripts/check_commit_messages.py origin/main..HEAD`,
  `scripts/check_large_files.py origin/main...HEAD`,
  `scripts/check_source_language.py origin/main...HEAD`,
  `scripts/check_pr_title.py` and `scripts/check_pr_body.py` -> exit 0;
  the pinned `commitlint` over the branch's commits -> exit 0.
- `deptry` reports 12 findings, none of them in a file this change
  touches, and this change adds no import at all; `ty check` could not
  be run in this environment (it resolved a different interpreter's
  site-packages), so its verdict is the `python lint` job's, which
  passes on this head.

Not run at this head: `tests/integration/` as a whole, the full
`pytest -q -n 4` sweep, the `ui-tui` suite, and `make ci`'s `bridge`
tree, none of which this change touches.

- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [ ] User-facing docs or screenshots are updated when needed

No user-facing doc describes the stop button, so none changes.

## Risk

- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes

User-visible behavior: a stop that lands just after its turn finished
leaves the next turn running and holds the send queue, instead of
going idle and sending the queued message under it; the stop button
comes back once that turn's first frame arrives. Until then the button
reads Send, which queues. The question of that turn is drawn when its
opening frame arrives, and a message that ran as a turn of its own has
its bubble moved out of the ended turn.

Backward compatibility: `busy` is optional. The web UI reads a refusal
without it the way it read every refusal before, and the terminal UI
and the ACP adapter ignore the reply. The assertions on a refused
reply in `tests/test_rpc_turn_cancel.py` now include its `busy` value.

Security surface is unchanged -- no new endpoints or privileges;
`busy` describes only the lane the cancel resolved to, on the caller's
own session. Rollback: revert the PR.

## Related Issues

#981
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(ui-web): the page's stop goes inert while a delegated result re-enters the conversation

3 participants