Creatidy/scarcity-router

Fork 0

Cancellation/disconnect semantics: close gaps outside the established-stream path #165

New issue

Open

opened 2026-10-01 00:11:40 +00:00 by kilo · 1 comment

kilo commented

2026-10-01 00:11:40 +00:00

Member

Provenance: independent review evidence deferred from #143 / PR #164 (merged as db4e1bd on develop). PR #164 itself is complete — it was a test-only synchronization fix for the scenario-11 flake and changed no production semantics. This issue captures the production cancellation paths its review surfaced but explicitly did not change or close.

Confirmed-good path (proven; do not redesign)

For an established response stream, client disconnect detected during chunk emission reaches the normal cancellation path end to end:

_StreamState.emit() (scarcity_router/gateway_server.py) checks _client_gone() before each emission and raises ClientDisconnectedError on client disappearance; write failures inside _write_frame() are translated to the same error (gateway_server.py:732-740);
the error propagates through the coordinator, which sets cancel_event on catch (scarcity_router/gateway_coordinator.py:804-806, 1782-1783);
WorkerBridgedAdapter's ordinary chunk-emission branch catches ClientDisconnectedError, sends worker cancellation, then rethrows (scarcity_router/worker_bridged_adapter.py:212-223);
WorkerSession.send_cancel() (scarcity_router/worker_endpoint.py:387) synchronously writes the cancellation frame.

PR #164 fixed only the test synchronization around this behavior; production semantics on this path are unchanged and were not the defect.

Deferred paths to investigate/fix

1. Coordinator `cancel_event` timing vs `pending.take()`

In _await_outcome() (scarcity_router/worker_bridged_adapter.py:200-210), the top-of-loop cancel-send check runs before pending.take() blocks. If cancel_event becomes set while blocked in take(), the subsequent chunk branch returns AdapterResult(status="cancelled") (worker_bridged_adapter.py:209-210) without calling _send_cancel_best_effort() — the loop never reaches the next top-of-loop check. The prior independent review used a deterministic probe and observed zero cancel-send calls on this path.

Inspect the analogous continuation loop _await_resume() (worker_bridged_adapter.py:430-441), which has the identical shape.

2. Header-write failure

_StreamState._begin() (scarcity_router/gateway_server.py:674-684) emits status and headers via send_response() / send_header() / end_headers(), which sit outside _write_frame()'s ClientDisconnectedError translation (gateway_server.py:732-740). A BrokenPipeError during header emission can escape through the generic unwind as a raw OSError: the adapter's cancel-before-rethrow catch never matches, and the coordinator's cancel_event is never set. The prior review reproduced this with a deterministic probe.

3. Initial tool-suspension emissions

The suspension branch of _await_outcome() (scarcity_router/worker_bridged_adapter.py:257-269) and of _await_resume() (worker_bridged_adapter.py:474 ff.) call context.emit_chunk(...) for the tool-call and finish frames outside any ClientDisconnectedError catch — unlike the ordinary chunk branch. A client disconnect during these early emissions is therefore not guaranteed an immediate cancel-before-unwind.

4. Worker connection loss (contract clarification)

Clarify and test the intended contract when the worker transport itself disappears mid-attempt. Current behavior is best-effort cancellation with ambiguous-outcome handling: worker_connection_lost at submit maps to AdapterAmbiguousError (worker_bridged_adapter.py:176-189), and an interrupted outcome raises AdapterAmbiguousError (worker_bridged_adapter.py:303-309). Ambiguity handling may be the correct behavior here — do not assume that guaranteed cancel delivery is required.

Required outcome

The future implementation task must first define the intended invariant for each of the four paths (recording any new decision in docs/decisions.md), then:

fix paths that violate the intended cancellation contract;
preserve explicitly best-effort/ambiguous semantics where appropriate;
add deterministic regression coverage (deterministic probes/seams — no timeout/sleep-based correctness);
avoid broad protocol redesign unless evidence requires it.

Non-goals: no changes to #143's merged test synchronization; no re-litigation of PR #164, which is done.

Provenance: independent review evidence deferred from #143 / PR #164 (merged as `db4e1bd` on `develop`). **PR #164 itself is complete** — it was a test-only synchronization fix for the scenario-11 flake and changed no production semantics. This issue captures the production cancellation paths its review surfaced but explicitly did not change or close. ## Confirmed-good path (proven; do not redesign) For an established response stream, client disconnect detected during chunk emission reaches the normal cancellation path end to end: - `_StreamState.emit()` (`scarcity_router/gateway_server.py`) checks `_client_gone()` before each emission and raises `ClientDisconnectedError` on client disappearance; write failures inside `_write_frame()` are translated to the same error (`gateway_server.py:732-740`); - the error propagates through the coordinator, which sets `cancel_event` on catch (`scarcity_router/gateway_coordinator.py:804-806`, `1782-1783`); - `WorkerBridgedAdapter`'s ordinary chunk-emission branch catches `ClientDisconnectedError`, sends worker cancellation, then rethrows (`scarcity_router/worker_bridged_adapter.py:212-223`); - `WorkerSession.send_cancel()` (`scarcity_router/worker_endpoint.py:387`) synchronously writes the cancellation frame. PR #164 fixed only the test synchronization around this behavior; production semantics on this path are unchanged and were not the defect. ## Deferred paths to investigate/fix ### 1. Coordinator `cancel_event` timing vs `pending.take()` In `_await_outcome()` (`scarcity_router/worker_bridged_adapter.py:200-210`), the top-of-loop cancel-send check runs *before* `pending.take()` blocks. If `cancel_event` becomes set while blocked in `take()`, the subsequent `chunk` branch returns `AdapterResult(status="cancelled")` (`worker_bridged_adapter.py:209-210`) without calling `_send_cancel_best_effort()` — the loop never reaches the next top-of-loop check. The prior independent review used a deterministic probe and observed **zero cancel-send calls** on this path. Inspect the analogous continuation loop `_await_resume()` (`worker_bridged_adapter.py:430-441`), which has the identical shape. ### 2. Header-write failure `_StreamState._begin()` (`scarcity_router/gateway_server.py:674-684`) emits status and headers via `send_response()` / `send_header()` / `end_headers()`, which sit **outside** `_write_frame()`'s `ClientDisconnectedError` translation (`gateway_server.py:732-740`). A `BrokenPipeError` during header emission can escape through the generic unwind as a raw `OSError`: the adapter's cancel-before-rethrow catch never matches, and the coordinator's `cancel_event` is never set. The prior review reproduced this with a deterministic probe. ### 3. Initial tool-suspension emissions The suspension branch of `_await_outcome()` (`scarcity_router/worker_bridged_adapter.py:257-269`) and of `_await_resume()` (`worker_bridged_adapter.py:474` ff.) call `context.emit_chunk(...)` for the tool-call and finish frames **outside** any `ClientDisconnectedError` catch — unlike the ordinary chunk branch. A client disconnect during these early emissions is therefore not guaranteed an immediate cancel-before-unwind. ### 4. Worker connection loss (contract clarification) Clarify and test the intended contract when the worker transport itself disappears mid-attempt. Current behavior is best-effort cancellation with ambiguous-outcome handling: `worker_connection_lost` at submit maps to `AdapterAmbiguousError` (`worker_bridged_adapter.py:176-189`), and an `interrupted` outcome raises `AdapterAmbiguousError` (`worker_bridged_adapter.py:303-309`). Ambiguity handling may be the correct behavior here — do not assume that guaranteed cancel delivery is required. ## Required outcome The future implementation task must **first define the intended invariant for each of the four paths** (recording any new decision in `docs/decisions.md`), then: - fix paths that violate the intended cancellation contract; - preserve explicitly best-effort/ambiguous semantics where appropriate; - add deterministic regression coverage (deterministic probes/seams — no timeout/sleep-based correctness); - avoid broad protocol redesign unless evidence requires it. Non-goals: no changes to #143's merged test synchronization; no re-litigation of PR #164, which is done.

kilo referenced this issue

2026-10-03 08:46:28 +00:00

F. First-class worker service lifecycle (systemd) #138

kilo referenced this issue

2026-10-04 22:16:05 +00:00

Align Scarcity Router with the Creatidy system architecture and register actionable gaps #173

kilo referenced this issue from Creatidy/creatidy-kernel

2026-10-04 22:22:46 +00:00

[Kernel][G01 G04 G10] Prove existing harness control and gateway compatibility #47

kilo referenced this issue from Creatidy/creatidy-kernel

2026-10-04 22:24:52 +00:00

[Kernel][G04 G10] Consume Router execution decisions and preserve Attempt-bound route identity #52

kilo referenced this issue

2026-10-04 22:25:02 +00:00

G10: Prove versioned harness-protocol compatibility and deliver the evidenced Responses subset #181

kilo referenced this issue

2026-10-04 23:11:08 +00:00

Align Scarcity Router with the Creatidy system architecture and register actionable gaps #173

~~kilo referenced this issue 2026-10-04 23:43:29 +00:00~~

Align Scarcity Router with Creatidy architecture and register actionable gaps (#173) #187

kilo referenced this issue

2026-10-05 00:10:49 +00:00

Align Scarcity Router with Creatidy architecture and register actionable gaps (#173) #187

adrian.tkacz added the

wontfix

label

2026-10-05 10:24:22 +00:00

kilo referenced this issue

2026-10-27 21:22:32 +00:00

P1: Complete cancellation/disconnect propagation outside the established stream path (#165 successor) #225

kilo referenced this issue

2026-10-27 21:23:25 +00:00

Record D-083 backlog disposition: re-home excluded wontfix requirements, correct #183/#186 prerequisites #228

kilo referenced this issue

2026-10-27 21:24:30 +00:00

Representative-client compatibility + operations hardening program (ZCode acceptance follow-up) #132

kilo commented

2026-10-27 21:24:49 +00:00

Author

Member

2026-10-27 owner backlog disposition (D-083, governance issue #228). This issue remains open with its wontfix label unchanged; history preserved. The 2026-10-27 reconciliation re-verified all four recorded paths against develop 4a182affa477: paths 1–3 remain present as described (path 2 additionally duplicated by _ResponsesStreamState._write_events added with #181/D-080 on 2026-10-25), and path 4 (ambiguous-outcome contract) is intentional and preserved. The defects are re-homed to active successor #225 with owner-declared priority P1; the established-stream cancellation path and PR #164's merged test synchronization remain proven and untouched. See D-083 in docs/decisions.md.

**2026-10-27 owner backlog disposition (D-083, governance issue #228).** This issue remains open with its `wontfix` label unchanged; history preserved. The 2026-10-27 reconciliation re-verified all four recorded paths against `develop` `4a182affa477`: paths 1–3 remain present as described (path 2 additionally duplicated by `_ResponsesStreamState._write_events` added with #181/D-080 on 2026-10-25), and path 4 (ambiguous-outcome contract) is intentional and preserved. The defects are re-homed to active successor **#225** with owner-declared priority **P1**; the established-stream cancellation path and PR #164's merged test synchronization remain proven and untouched. See D-083 in `docs/decisions.md`.

kilo referenced this issue

2026-10-27 22:54:12 +00:00

P1: Complete cancellation propagation outside the established stream path (#225) #230

kilo referenced this issue

2026-10-28 11:07:28 +00:00

P1: Complete cancellation propagation outside the established stream path (#225) #230