fix(runtime): apply graph stops while child work is running by chihumyum · Pull Request #5977 · apache/maka

chihumyum · 2026-10-06T17:13:34Z

Summary

A durable update_agent_graph.stop now reaches a running child while its dispatch wave is still active. Schedule changes wake the existing reconciliation owner to apply controls, while the original wave retains its results, admission fences, and activation budget.

Stops carry the selected Session/Run/Turn identity through SessionManager, the Hosted root authority, and the Runtime backend owner. Queued sibling work and later activations remain intact; successful cleanup releases the existing Session queue. Global Session stop keeps its broader cancellation behavior.

Verification

Added a real SessionManager regression that failed before the fix: a committed stop timed out while the child's natural-completion gate remained closed.
Regression coverage includes multiple running children, queued work on one Session, new work after stop, concurrent natural completion, durable terminal/result uniqueness, coordinator recovery, missed-wake windows, repeated schedule wakes, and transient control/cleanup failures.
Hosted fixtures verify that the durable Host stop fence precedes backend stop and that scoped and global stops preserve their respective target boundaries.
Affected runtime, AgentRun recovery/continuation, Graph, and Hosted root suites: 472 passed.
Root lint, format, build, typecheck, both required knip checks, strict renderer architecture checks, Astryx inventory/tests, ASF headers, and model metadata checks passed.
Host delivery faults still follow the existing fail-stop/drain policy; this change does not promise in-place recovery from every provider or Host failure.
Providers are in-memory fixtures; no real model inference or Desktop interaction was exercised for this change.

AI use

No generative tool made a substantive contribution
Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex (GPT-6 Astra) investigated the defect, compared cancellation designs, and authored the implementation and regression tests.

Checklist

Tests cover the change and fail without it
Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

Yes — described under Summary above
No

Generated-by: Codex

Astro-Han

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Combined review of head 0f24999d from two independent review passes (a correctness pass and an adversarial pass), synthesized and graded by the review lead. Verdict: COMMENT, no P0/P1, four P2s, six P3s.

Status. On current main 3597abe8, mergeable, CI (test, label) green. Locally, the runtime graph-stop / reconcile / coordinator / session-manager suites passed 240/240 and runtime-host root-turn-coordinator 87/87. No runtime-host protocol files changed, so no epoch claim is needed. About 76% of the added lines are tests. No conflict or contradiction with #5512; merge order doesn't matter.

What holds up. The durable Host stop fence is written before the backend stop; stops target the exact Run and Turn, so they never hit a newer Run; repeated stops are deduped; the new wake can't miss a schedule commit; a stop racing natural completion still yields one terminal and one result; listeners are removed in finally.

P2

stream-graph-schedule-reconcile.ts:764: already_terminal targets used to resolve without a runtime call; now every historical stop/replace target (never pruned) is re-stopped on every reconcile and mid-wave wake. In hosted mode each is a full stopRoot, and one failing stale target makes the pass return failed (:241) before any new dispatch, stalling the whole graph. (Both passes; one graded it P1, ruled P2 because the stall needs a failing stop.)
stream-graph-schedule-reconcile.ts:486 with session-manager.ts:2983: after a transient cleanup failure waitForExecutionStop stays pending, so the wave never settles and only another schedule commit retries. An idle supervisor can deadlock the graph. (Both passes.)
root-turn-coordinator.ts:1252: stopRoot now always uses the scoped stop, which also changes the user Stop button (turn.stop), host shutdown and the graph-wide supervisor stop. The PR body says global Session stop is unchanged; please confirm or narrow.
session-manager.ts:3204: a stop that lands after hostSubmitted but before the Host admission is durable gets Turn was not admitted, the local execution is never aborted, and the child runs unstopped. Main's session-level stop covered this window. (Both passes.)

P3

stream-graph-schedule-reconcile.ts:476: a child whose backend ignores abort can delay stops on cooperative siblings.
session-manager.ts:3183: an activation stop and a whole-graph supervisor stop both record graph.supervisor. Also, a crash between projecting aborted and writing the terminal fact could leave the operator refusing future claims; there is no restart test.
runtime-kernel.ts:1967: the scoped stop throws if the generation owns another live run, with no session-wide fallback.
runtime-kernel.ts:3026: the quarantine bypass now accepts a possibly stale stop intent; untested.
runtime-kernel.ts:2306: a rejected cleanup completion replaces the child's dispatch result with the cleanup error (the durable result is preserved).

Test gaps. The reconcile tests use a fake stop controller, so the hosted cost and the lost-stop window aren't exercised; end-to-end tests don't run hosted, only use abort-cooperative children, and never restart.

Address review of the activation-scoped graph stop: - Public turn.stop, Host shutdown, WorkHub and whole-graph supervisor stops call stopRoot without a scope again, so they keep the Session-level Runtime stop that also reaches a claim whose Run has not attached yet. Only stopAgentGraphActivation asks for { scope: 'run' }. - A hosted graph claim counts as Host-owned only once the Host admission gate admits it. The gate runs in the Session admission lease right before the durable admission and refuses a claim whose local capability was stopped, so a stop between submission and admission cancels the activation instead of failing with "Turn was not admitted" while the child runs on. - A terminal stop target reaches the runtime only while a failed stop still retains cleanup for that exact Run, and a stop that succeeded in this reconciliation is not repeated on later passes or wakes. Historical targets no longer cost a Host stop per wake or fail the pass before new dispatch. - A failed control pass during a live wave schedules a capped backoff wake (100 ms doubling to 5 s), so a stop failure that parks the only running child is retried without another schedule commit. A repeated failure for the same target replaces the earlier one without notifying the supervisor again. Tests cover a public stop before Run attach and a graph stop before durable admission on the real Host, historical and repeated stop targets, and a retry without a wake. Pinning tests record that a graph-supervisor Session stop lets an operator accept its next claim while a user stop does not, and that a failed backend delivery is reported to its stopper while the child's result stays authoritative.

chihumyum · 2026-10-08T01:08:12Z

Pushed b2b9ff9 for the review: all four P2s are fixed with fail-before/pass-after tests, and the P3s are answered inline. turn.stop, host shutdown, WorkHub and the whole-graph supervisor stop are Session-level again, so the PR body's "global Session stop is unchanged" holds.

Residual risks worth a reviewer's look:

A persistent stop failure during a live wave now retries every ≤5 s until the wave settles. The rate is capped; the count is not. In hosted mode each retry is one Host stop.
hasPendingAgentGraphActivationStop is optional on the coordinator runtime. A runtime without it falls back to main: a terminal target never retries cleanup.
If a future RuntimeHostedRootAuthority drops the new { scope: 'run' } argument, graph stops widen to the whole Session instead of failing.
The admission gate can refuse a claim whose intent was already moved to executing. The only production caller (the schedule reconciler) cancels the intent before stopping, so this is consistent today. A future caller that stops an activation id directly would need to settle the intent itself.

Verification: runtime 3736/3750 pass (14 skipped, 0 fail), runtime-host 2233/2245 (12 skipped, 0 fail), typecheck, lint, format, ASF headers, Windows inventory, Knip, and check:release (minus check:stale) all pass. No protocol changes.

🤖 Addressed by Claude Code

Astro-Han

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of 0f24999d → b2b9ff9a (one commit, same merge-base 3597abe8). Verdict: COMMENT. All four prior P2s are fixed. No P0, P1 or P2 issues remain. The new notes are P3.

Prior P2s

Historical stop targets re-stopped on every pass and wake: fixed (stream-graph-schedule-reconcile.ts:805). A terminal target reaches the runtime only while hasPendingRunStop reports a retained stop operation for that exact Run. Successful stops are also memoized per reconciliation (resolvedStops). The coordinator's runtime is the SessionManager (execution-composition.ts:1620), so the optional hook is wired in production. Only a genuinely retained cleanup failure can still fail a pass graph-wide.
Wave hangs after a transient cleanup failure: fixed (:509). A failed control pass during a live wave arms a 100 ms → 5 s backoff wake. The timer is cleared on every wake and in finally, a clean pass resets the delay, and a repeated failure replaces its earlier record without notifying the supervisor again. A later success clears the failure (recordScheduleStops).
stopRoot always scoped: fixed. Scope is now opt-in ({ scope: 'run' }), and only stopAgentGraphActivation passes it. I compared the unscoped path with main: same fence, same single delivery (shouldDeliverStop = !stopRequested), same deliverHostedRootStop → kernel stopSession, and the same unfiltered foreground child stop. A new Host test covers a public stop before the Run attaches.
Stop before Host admission is durable: fixed. hostAdmitted is now set inside the Host admission gate, which RootTurnCoordinator runs under the Session admission lease immediately before admitRootTurn. The handoff is linearizable. stopExecution aborts the claim's controller synchronously, and the gate re-checks stopSignal after its await and before it sets hostAdmitted, so either the gate refuses the Turn or the stop sees hostAdmitted and queues behind the lease for the Host fence. The existing-Run path (:3116) marks admitted only after requireClaimedGraphAdmissionIdentity. A real-Host test is added.

Prior P3s

:476 abort-ignoring sibling: the rationale is accepted. allSettled starts every stop concurrently, so only the next pass waits.
:3183 supervisor marker: intended, and now pinned by tests for both graph_supervisor and stop_button. The duplicate listInvocations read is fixed. The crash window between the aborted projection and the terminal fact is still untested.
:1967 scoped-stop throw: the one-Run-per-operator rationale is accepted, and the backoff now retries it.
:3026 quarantine bypass: the rationale (an intent clears only after all its claims settle) is plausible. Still no test.
:2306 rejected completion: a test now pins the local path, and the hosted path drains. Accepted.
Test gaps: real-Host tests have been added for the stop-before-attach and stop-before-admission windows. A restart test and a non-cooperative backend are still missing.

New (P3): uncapped retry count during a live wave (inline) and the optional stopRoot options parameter (inline). Both are among the residual risks the author listed.

Status. Mergeable (BLOCKED on review only). CI test passes. No runtime-host protocol change; the epoch stays 205, so no claim is needed.

Local (exact head). All suites pass: graph schedule stop 5, reconcile 19 (3 more loops, no flakes), coordinator 15, session-manager 207, terminal-ledger 44, kernel-interaction 12, Host root-turn-coordinator 89, execution-composition 46. tsc --noEmit is clean for runtime and runtime-host.

An authority that ignored the optional `{ scope: 'run' }` argument of stopRoot still satisfied RuntimeHostedRootAuthority, and a graph activation stop through it would silently widen to a Session stop that also cancels queued sibling claims. TypeScript accepts a function with fewer parameters, so no option shape can catch that. Replace the option with a required stopRootRun(identity, input). An authority without exact-Run support is now a compile error; building the tests caught one more Host fixture that forwarded only stopRoot. RootTurnCoordinator.stopRoot is again byte-for-byte main's Session-level stop, and stopRootRun carries the exact-Run delivery, redelivery and completed-Run cleanup retry. SessionManager.stopAgentGraphActivation is its only caller.

A stop whose cleanup keeps failing parks its child, and so its wave, while the backoff retries every 5 s. Each retry replaced the failure silently, so the graph sat parked behind the first failure record. Count consecutive failures per target. The eighth one reaches the supervisor once, through the existing reconciliation failure notification, with a distinct "is stuck after N consecutive failures and is still retrying" reason that also lands in the durable client failure. Eight failures take about 11 s, after the backoff reaches its cap, so a fault that clears during the ramp is never reported as stuck. The stop is never dropped, and a later success clears the failure and resets the count. A driver abort no longer ends the control loop while the wave is still running. Before, the loop stopped retrying on the next wake after an abort, so a child parked behind a transient stop failure kept the aborted reconciliation waiting forever. Repeated control read failures now notify once per streak as well.

Astro-Han

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of b2b9ff9a → 8620455c (two commits, same merge-base 3597abe8). Verdict: COMMENT. Both prior P3s are fixed. One new P3, inline.

Prior findings

Unbounded retry count (stream-graph-schedule-reconcile.ts:509): fixed by surfacing it rather than capping it. A per-target streak notifies the supervisor at the first failure and once more at the 8th with "stuck … still retrying". A success resets it. The new mocked-timer test covers attempts 7, 8, 12 and 13.
Optional scope widening to Session (message-authority.ts:78): fixed. stopRootRun is a required method, stopRoot is byte-identical to main again, and every implementer forwards it (production composition plus the test fakes). The only caller is stopAgentGraphActivation.
Quarantine bypass (runtime-kernel.ts:3026): still untested.
Restart and non-cooperative backend tests: still missing.

New code

Dropping the abortSignal break in the wave loop is an improvement. Before, an aborted driver already waited on the parked wave in finally without retrying. Now it keeps retrying until the wave settles. The new test pins this.
Streaks are per reconciliation, every stop failure carries a targetId, and a later success clears the stuck failure from the result. No issues found.
Nit: stopRootRun duplicates about 30 lines of stopRoot. That's understandable while stopRoot is kept identical to main.

New (P3): a persistent control read failure never reaches the stuck escalation (inline).

Status. MERGEABLE, BLOCKED on review only. CI test passes at 8620455. Main moved to 72afbf1 (#5902). It touches execution-composition.ts in unrelated hunks, and the merge is clean. No runtime-host protocol change, so no epoch claim is needed (main is 206).

A schedule control read that kept failing notified the supervisor on its first failure and then retried every 5 s in silence while the wave stayed parked, the same silent park the stop path no longer has. Count read failures the same way: notify on the first and, once, on the eighth consecutive failure as stuck and still retrying. A successful read resets the streak and clears the failure. The threshold is now CONTROL_STUCK_FAILURES, shared by stops and reads.

Astro-Han

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of 60bcaf1.

P3 (stuck control read): fixed. The read-failure streak now notifies at failures 1 and 8 under the shared CONTROL_STUCK_FAILURES threshold, resets on success, and has a mocked-timer test that also covers recovery.
Still open, both test gaps only: the quarantine bypass in runtime-kernel.ts, and restart / non-cooperative backend coverage.

No new findings. CI is green and the PR merges cleanly with main. It touches no runtime-host protocol file, so there is no epoch claim.

…uarantine Two review gaps, both test-only. A graph stop pass now stops two children together, one of whose provider neither returns from stop nor ends its send. The cooperative sibling still reaches its aborted terminal while the other stop is pending, so one stuck provider delays only the pass, not its siblings. Serializing the stop calls makes this test fail. A stop whose cleanup cannot persist its terminal keeps the operation retained. An unrelated Turn on that Session, which holds no intent of that stop, is refused as quarantined until a retry settles the cleanup, and then runs. Widening the quarantine bypass to any attached execution makes this test fail.

chihumyum · 2026-10-08T07:19:32Z

Added two test-only pins for the remaining gaps (38a632d):

Non-cooperative backend: when one provider never returns from stop(), its sibling's stop in the same pass still reaches its aborted terminal right away. Making the stops run one at a time fails this test.
Quarantine: a retained stop operation refuses an unrelated Turn on that Session until a retry settles the cleanup.

A stale stop intent paired with a retained operation can't be built through the public API. stopSessionAttempt clears a Session intent only after every claim it holds has settled, and a scoped intent belongs to one claim for that claim's whole life. So that side is covered by reasoning rather than a test. A restart test is out of scope for this PR.

🤖 Addressed by Claude Code

Astro-Han

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of 60bcaf19 → 38a632d9 (one test-only commit, same merge-base 3597abe8).

Quarantine bypass untested: fixed. session-manager.test.ts:2345 keeps a stop retained (abort append rejected) and asserts that an unrelated Turn is refused as quarantined, then runs once the retry clears the stop.
Non-cooperative backend: fixed. stream-graph-schedule-stop.test.ts:453: a sibling stop settles aborted while the other provider ignores its stop. Because the ignoring target is listed first, serialized stops would fail the test.
Restart: partly covered, not blocking. The existing fresh-coordinator test recovers a stop that has already settled. A restart while a stop is still retained remains untested.

No new findings. CI test passes, the PR merges cleanly with main f10a74d8, and there is no protocol change.

fix(runtime): apply graph stops while child work is running

0f24999

Generated-by: Codex

github-actions Bot added the effort/XL Under 2500 readable lines label Oct 6, 2026

Astro-Han reviewed Oct 6, 2026

View reviewed changes

Astro-Han reviewed Oct 8, 2026

View reviewed changes

Comment thread packages/runtime/src/stream-graph-schedule-reconcile.ts

Comment thread packages/runtime/src/message-authority.ts Outdated

chihumyum added 2 commits October 8, 2026 11:29

github-actions Bot added effort/XXL Over 2500 readable lines and removed effort/XL Under 2500 readable lines labels Oct 8, 2026

Astro-Han reviewed Oct 8, 2026

View reviewed changes

Comment thread packages/runtime/src/stream-graph-schedule-reconcile.ts Outdated

Astro-Han reviewed Oct 8, 2026

View reviewed changes

Conversation

chihumyum commented Oct 6, 2026

Summary

Verification

AI use

Checklist

Uh oh!

Astro-Han left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chihumyum commented Oct 8, 2026

Uh oh!

Astro-Han left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Astro-Han left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Astro-Han left a comment

Choose a reason for hiding this comment

Uh oh!

chihumyum commented Oct 8, 2026

Uh oh!

Astro-Han left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants