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

kilo commented

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

Member

Goal

Priority: P1 (owner-declared 2026-10-27 backlog disposition, D-083). Complete client-disconnect/cancel propagation on the execution-stream paths outside the already-proven established-stream path. Active successor of excluded wontfix issue #165; #165 stays open/excluded and unchanged.

Why and Evidence

#165 recorded four production paths deferred from the PR #164 review. The 2026-10-27 reconciliation verified each against develop 4a182affa4775a4a38b47860ad7622a099c56c4a (no develop commit changed them since filing):

Chunk-branch cancelled return without cancel send — worker_bridged_adapter.py _await_outcome() (~213-214) and _await_resume() (~452-454): when cancel_event is set while blocked in pending.take() and a chunk then arrives, the branch returns AdapterResult(status="cancelled") without _send_cancel_best_effort; the loop never reaches the next top-of-loop check. Latent under current coordinator wiring (today cancel_event is set only on the covered emit-raise/post-return paths) but a live adapter-contract hole (e.g. a future deadline timer).
SSE header writes outside disconnect translation — gateway_server.py _StreamState._begin() (~775-779): send_response/send_header/end_headers sit outside _write_frame()'s BrokenPipeError/ConnectionResetError/OSError → ClientDisconnectedError translation (~827-835), so a header-write failure escapes as raw OSError through the generic unwind; coordinator cancel_event never set.
Same shape duplicated in the Responses stream — _ResponsesStreamState._write_events (~919-923, added with #181 D-080 on 2026-10-25) replicates the untranslated header-emission pattern of path 2.
Tool-suspension emissions uncaught — worker_bridged_adapter.py _await_outcome() (~261-273) and _await_resume() (~487-502): suspension-branch context.emit_chunk(...) calls for tool-call/finish frames sit outside any ClientDisconnectedError catch, unlike the ordinary chunk branch — disconnect during these emissions triggers no worker cancellation and leaves the backend turn suspended endpoint-side.

Intentional contract to preserve (path 4 of #165): worker connection loss at submit → AdapterAmbiguousError; interrupted outcome → AdapterAmbiguousError; TOOL_RESULT_AMBIGUOUS → AdapterAmbiguousError. Honest ambiguity, never retry. Existing regression tests cover only the already-covered branches.

Scope

Verify and deterministically reproduce each defect at current HEAD before any behavior change (red tests first, synthetic probes/seams only — no timeout/sleep-based correctness). Then make the minimal local fixes:

cancel-send-before-cancelled-return on the chunk branches (respecting cancel_sent so cancellation effects are never duplicated);
translate header-write failures in _begin() and _write_events to ClientDisconnectedError so the existing coordinator cancellation path engages for Chat Completions and Responses;
wrap suspension-branch emissions with the same cancel-before-rethrow handling as the ordinary chunk branch, including cleanup of suspended continuation handles where the established contract requires it.

Non-goals: no execution-coordination redesign, no worker-protocol change, no automatic retry of ambiguous execution, no change to the ambiguous-outcome contract, no change to PR #164's merged test synchronization.

Acceptance Criteria

Deterministic red/green reproductions for paths 1, 2, 2b (Responses) and 3 committed with the fix; each defect observable failing on the pre-fix source.
Client disconnect on every covered path reaches the worker as cancellation exactly once (no duplicate cancel effects), with honest disconnect outcomes; no false completed/cancelled outcomes.
Chat Completions and Responses SSE both covered by equivalent disconnect tests (header-write failure and mid-stream failure).
Suspended continuation handles cleaned up correctly on disconnect-induced cancellation; existing tool-continuation (D-062) and streaming contracts unchanged and green.
Ambiguous-outcome semantics pinned by tests remain unchanged; no retry of ambiguous execution introduced.
Full gate (uv sync --only-dev, make check, uv run basedpyright, git diff --check), deterministic offline tests only; fresh independent technical review (security assessment included) at exact frozen HEAD/base.

Constraints

Keep the fix local to the identified paths. Preserve #143/PR #164 outcomes. No new public contract, protocol version, credential handling or network surface. Security-relevant correctness fix, not architecture.

Links

Excluded predecessor #165 (historical, unchanged); D-083 disposition; D-043 audit/ambiguity, D-062 continuation boundary, D-080 Responses ingress; worker_bridged_adapter.py, gateway_server.py, gateway_coordinator.py.

## Goal **Priority: P1 (owner-declared 2026-10-27 backlog disposition, D-083).** Complete client-disconnect/cancel propagation on the execution-stream paths outside the already-proven established-stream path. Active successor of excluded `wontfix` issue #165; #165 stays open/excluded and unchanged. ## Why and Evidence #165 recorded four production paths deferred from the PR #164 review. The 2026-10-27 reconciliation verified each against `develop` `4a182affa4775a4a38b47860ad7622a099c56c4a` (no develop commit changed them since filing): 1. **Chunk-branch cancelled return without cancel send** — `worker_bridged_adapter.py` `_await_outcome()` (~213-214) and `_await_resume()` (~452-454): when `cancel_event` is set while blocked in `pending.take()` and a chunk then arrives, the branch returns `AdapterResult(status="cancelled")` without `_send_cancel_best_effort`; the loop never reaches the next top-of-loop check. Latent under current coordinator wiring (today `cancel_event` is set only on the covered emit-raise/post-return paths) but a live adapter-contract hole (e.g. a future deadline timer). 2. **SSE header writes outside disconnect translation** — `gateway_server.py` `_StreamState._begin()` (~775-779): `send_response/send_header/end_headers` sit outside `_write_frame()`'s `BrokenPipeError/ConnectionResetError/OSError → ClientDisconnectedError` translation (~827-835), so a header-write failure escapes as raw `OSError` through the generic unwind; coordinator `cancel_event` never set. 3. **Same shape duplicated in the Responses stream** — `_ResponsesStreamState._write_events` (~919-923, added with #181 D-080 on 2026-10-25) replicates the untranslated header-emission pattern of path 2. 4. **Tool-suspension emissions uncaught** — `worker_bridged_adapter.py` `_await_outcome()` (~261-273) and `_await_resume()` (~487-502): suspension-branch `context.emit_chunk(...)` calls for tool-call/finish frames sit outside any `ClientDisconnectedError` catch, unlike the ordinary chunk branch — disconnect during these emissions triggers no worker cancellation and leaves the backend turn suspended endpoint-side. **Intentional contract to preserve (path 4 of #165)**: worker connection loss at submit → `AdapterAmbiguousError`; `interrupted` outcome → `AdapterAmbiguousError`; `TOOL_RESULT_AMBIGUOUS` → `AdapterAmbiguousError`. Honest ambiguity, never retry. Existing regression tests cover only the already-covered branches. ## Scope Verify and deterministically reproduce each defect at current HEAD before any behavior change (red tests first, synthetic probes/seams only — no timeout/sleep-based correctness). Then make the minimal local fixes: - cancel-send-before-cancelled-return on the chunk branches (respecting `cancel_sent` so cancellation effects are never duplicated); - translate header-write failures in `_begin()` and `_write_events` to `ClientDisconnectedError` so the existing coordinator cancellation path engages for Chat Completions and Responses; - wrap suspension-branch emissions with the same cancel-before-rethrow handling as the ordinary chunk branch, including cleanup of suspended continuation handles where the established contract requires it. Non-goals: no execution-coordination redesign, no worker-protocol change, no automatic retry of ambiguous execution, no change to the ambiguous-outcome contract, no change to PR #164's merged test synchronization. ## Acceptance Criteria 1. Deterministic red/green reproductions for paths 1, 2, 2b (Responses) and 3 committed with the fix; each defect observable failing on the pre-fix source. 2. Client disconnect on every covered path reaches the worker as cancellation exactly once (no duplicate cancel effects), with honest disconnect outcomes; no false `completed`/`cancelled` outcomes. 3. Chat Completions and Responses SSE both covered by equivalent disconnect tests (header-write failure and mid-stream failure). 4. Suspended continuation handles cleaned up correctly on disconnect-induced cancellation; existing tool-continuation (D-062) and streaming contracts unchanged and green. 5. Ambiguous-outcome semantics pinned by tests remain unchanged; no retry of ambiguous execution introduced. 6. Full gate (`uv sync --only-dev`, `make check`, `uv run basedpyright`, `git diff --check`), deterministic offline tests only; fresh independent technical review (security assessment included) at exact frozen HEAD/base. ## Constraints Keep the fix local to the identified paths. Preserve #143/PR #164 outcomes. No new public contract, protocol version, credential handling or network surface. Security-relevant correctness fix, not architecture. ## Links Excluded predecessor #165 (historical, unchanged); D-083 disposition; D-043 audit/ambiguity, D-062 continuation boundary, D-080 Responses ingress; `worker_bridged_adapter.py`, `gateway_server.py`, `gateway_coordinator.py`.

kilo added the

bug

P1

labels

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

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 referenced this issue

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

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

kilo referenced this issue

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

Record D-083 backlog disposition: re-home excluded wontfix requirements (#228) #229

adrian.tkacz referenced this issue from a commit

2026-10-27 22:53:55 +00:00

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

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:10 +00:00

Record D-083 backlog disposition: re-home excluded wontfix requirements (#228) #229

kilo referenced this issue from a commit

2026-10-28 11:58:12 +00:00

Merge pull request #230: P1 cancellation propagation

kilo commented

2026-10-28 11:58:43 +00:00

Author

Member

Completed/integrated via PR230 on 2026-10-28, merge04f40ddf59ea1abba6719208166daa94aeeddb1a on develop. Parents reviewed base2c40e0e1fb79397c97e6fa2b7dac0365d9712c0c and HEADebb5a4093feda19d4e73e0769ecf4b081596398c; merge-base4a182af. Fresh Git verified ancestry and integrated tree7b0416513ab3859277ffbcb98e517d2066ce1b07 EXACTLY equals full-gate-tested candidate. No implementation restart/history rewrite. Six deterministic original red/green regressions cover initial/resume chunk-cancel windows, initial/sequential suspension-emission disconnects, Chat/Responses SSE headers; existing real-socket midstream tests on both APIs remain green. Full sanitized integration-tree gate:uv sync --only-dev; make check3063 OK/18 skips(725.455s);basedpyright0/0/0;whitespace clean. Native independent GLM-5.3 pr-reviewer session ses_e7dae7b1cffe7M3L5csT38SQmR ordinal1/10:APPROVE/findings[], correctness+security+UX assessed, including clarified effect-idempotent cancellation and existing bounded-expiry cleanup of WAITING handles. No ambiguous-execution retry, continuation/protocol/security weakening or release action. Historical165 remains open/wontfix as required by D-083. Acceptance satisfied; closing225 and resuming canonical loop.

kilo closed this issue

2026-10-28 11:58:49 +00:00

Rows
Columns

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

Goal

Why and Evidence

Scope

Acceptance Criteria

Constraints

Links