Skip to content

Handle downstream client disconnects consistently for all SSE writes #18

Closed

Description

@NOirBRight

Outcome

Centralize every downstream SSE write in the Python Gateway so a client close is classified once, unwinds upstream work, and completes the request as HTTP 499 without being mistaken for an upstream transport or protocol failure.

Design and scope

  • Route all SSE header, event, keepalive, terminal, error, converted-protocol, and transparent relay writes in src-python/codex_proxy.py through one downstream-write seam.
  • The seam must handle BrokenPipeError, ConnectionResetError, and other downstream OSError values consistently, record a sanitized downstream-close disposition, and prevent duplicate terminals or retries after exposure.
  • When the downstream closes, promptly close/cancel the associated upstream response/reader and let the handler unwind. Preserve normal stream bytes and ordering.
  • Final request accounting is exactly one request_complete with status 499 for a downstream close. Do not relabel it as an upstream error and do not synthesize a protocol terminal to the closed client.
  • Preserve transparent route identity and existing successful Responses and Chat behavior.

Deterministic acceptance

  • Every production SSE write path uses the central seam; no direct streaming wfile.write path bypasses it.
  • Tests inject downstream close before first output, mid-event, during keepalive, during converted Responses/Chat output, and after upstream terminal.
  • Tests prove upstream close/cancel, no further write, no fallback/retry, exactly one sanitized completion, and 499 classification.
  • Targeted tests cover tests/test_routing.py, tests/test_proxy_event_logging.py, tests/test_chat_completions_gateway.py, and tests/test_proxy_shutdown.py.

Final-SHA real-client gate

After the E2E foundation issue, every final candidate SHA must build Debug and pass all twelve cases in VM snapshot codexhub-real-client-e2e-v1:

  • Desktop and CLI: gpt-5.6-luna and volc/glm-5.2.
  • OpenCode, ZCode, Pi, and OMP: the candidate-managed Luna selector and exact codexhub-volc/glm-5.2.
  • Each case uses a disposable sentinel file, a real read-only tool call, and a streamed short sentinel; it proves canonical model, expected terminal, exactly one request_complete=200, and no duplicate, fallback, or unclassified reconnect.
  • Desktop and ZCode are structured human GUI cases. Login, credentials, and GUI are fail-closed; never use host shared sessions or fabricate evidence.
  • Only one retry is allowed for a pre-output provider-capacity 429/503. A new SHA invalidates all evidence.

Issue-specific real-client cases additionally require Desktop/Luna Stop and OMP/Volc Ctrl+C. Both must classify the downstream close as 499, cancel upstream work, emit one completion, and leave no orphaned reader/process.

Verification and integration

python -m pytest -q tests/test_routing.py tests/test_proxy_event_logging.py tests/test_chat_completions_gateway.py tests/test_proxy_shutdown.py
python -m pytest -q
git diff --check
python scripts/report_quality_gates.py

The quality-gate command is report-only. The final SHA also requires Debug build, GitHub Actions, a strict heavy Reviewer, and the human VM gate above. Squash to dev only; never merge to main.

Hotset

  • src-python/codex_proxy.py
  • tests/test_routing.py
  • tests/test_proxy_event_logging.py
  • tests/test_chat_completions_gateway.py
  • tests/test_proxy_shutdown.py

Activity

  1. added
    needs-triageMaintainer needs to evaluate this issue
    bugSomething isn't working
    ready-for-agentFully specified, ready for an AFK agent
    and removed
    needs-triageMaintainer needs to evaluate this issue
    on Jul 9, 2026
  2. NOirBRight commented on Jul 9, 2026

    @NOirBRight
    OwnerAuthor

    This was generated by AI during triage.

    Agent Brief

    Category: bug
    Summary: Classify downstream client disconnects separately from upstream stream failures.

    Current behavior:
    System.Collections.Hashtable.Current

    Desired behavior:
    System.Collections.Hashtable.Desired

    Key interfaces:

    • SSE downstream write helpers
    • Stream relay error classification
    • Proxy event telemetry payload

    Acceptance criteria:

    • Downstream write failures are caught consistently.
    • Telemetry has a distinct downstream-write/client-disconnect classification.
    • Tests cover helper writes and at least one relay branch.

    Out of scope:

    • Do not implement circuit breaker behavior.
    • Do not change protocol conversion semantics except where required to report the disconnect.
  3. added
    P1Priority 1: current iteration goal
    tier:heavyOrchestrator provider tier: heavy
    on Jul 20, 2026
  4. NOirBRight commented on Jul 20, 2026

    @NOirBRight
    OwnerAuthor
    ```json { "contract": { "acceptance": [ "A static audit finds no production SSE header, event, keepalive, terminal, converted-route, or transparent-route client write outside the shared downstream commit seam or an explicitly non-streaming response path.", "Deterministic tests cover downstream close before output, mid-event, during keepalive, converted Responses and Chat output, transparent output, and after upstream terminal; all close upstream work and prevent further writes.", "Every downstream close records one and only one sanitized request_complete=499, with no request_error substitute, retry, fallback, synthetic write to the closed client, duplicate terminal, or later success.", "Upstream error, protocol failure, timeout, Gateway shutdown, and operator cancellation retain their own existing classifications and cannot be mislabeled as a downstream 499.", "Official, transparent third-party, and converted successful streams retain exact protocol ordering, tool continuity, usage, canonical model/route identity, and one normal terminal.", "The exact #223 local gates, strict reviews, single push, and exact-SHA CI are durable and merged to dev before #18 closes.", "No intermediate 12-case real-client run is performed; bounded #220 evidence and final #227 qualification consume the merged behavior." ], "change_claims": { "paths": [], "resources": [ "issue-18-parent-acceptance", "gateway-downstream-stream-commit", "gateway-request-terminal-ledger" ] }, "dependencies": { "dispatch_after": [ 223 ], "merge_after": [ 223 ] }, "design": [ "Treat #18 as the parent acceptance and closure record for #223. Do not dispatch another implementation Worker or create a second downstream-write abstraction.", "Consume the merged #223 exact-SHA delivery, strict review, and exact-SHA CI to prove every production SSE write, terminal, and stream-error path is owned by the one request-scoped downstream commit seam.", "Require consistent fail-closed handling for BrokenPipeError, ConnectionResetError, and downstream OSError before output, mid-event, during keepalive, during converted and transparent output, and after an upstream terminal. The seam closes owned upstream work and prevents all later writes, retries, fallbacks, and duplicate terminals.", "A downstream client close produces exactly one sanitized request_complete with status 499 and remains distinct from upstream transport, protocol, timeout, and operator cancellation classifications. Normal successful streams preserve protocol bytes, ordering, tools, usage, and canonical route identity.", "Close #18 only after #223 is integrated into dev and the exact parent acceptance is read back. Do not run a full real-client matrix here; #220 reuses its bounded Ollama Cloud cancellation assertion and #227 owns the sole final 12-case qualification." ], "difficulty": "heavy", "done_when": [ "Issue #223 is merged to dev from its one frozen candidate with no unresolved strict review finding and successful exact-SHA Actions.", "The merged dev tree and #223 delivery prove every #18 acceptance item through deterministic local tests and static scope audit.", "A parent closure comment records the merged SHA, #223 PR/review/CI evidence, exact 499 contract, and final-E2E deferral to #227.", "Issue #18 is closed without another Worker, PR, candidate SHA, or intermediate real-client matrix." ], "priority": "P1", "risk": "strict", "sha256": "ffcc57079b31c8964ea8328c4729375ae3512f4dfdb2bd6281cd34a64a58a008", "unresolved_decisions": [] }, "dispatch": null } ```
  5. added
    orch:readyOrchestrator V6: valid hashed contract is complete and dispatchable
    on Jul 20, 2026
  6. 1 remaining item

  7. added
    orch:blockedOrchestrator V6: blocked pending a decision or external state change
    and removed
    orch:readyOrchestrator V6: valid hashed contract is complete and dispatchable
    on Jul 25, 2026
  8. NOirBRight commented on Jul 25, 2026

    @NOirBRight
    OwnerAuthor

    Phase 0 decomposition note: this parent outcome is intentionally orch:blocked while bounded implementation slices #221 → #222 → #223 land. After #223 satisfies the parent contract, the Coordinator will close #18 rather than dispatching the obsolete all-at-once contract.

  9. added
    orch:blockedOrchestrator V6: blocked pending a decision or external state change
    orch:readyOrchestrator V6: valid hashed contract is complete and dispatchable
    and removed
    ready-for-agentFully specified, ready for an AFK agent
    orch:blockedOrchestrator V6: blocked pending a decision or external state change
    on Jul 25, 2026
  10. NOirBRight commented on Jul 25, 2026

    @NOirBRight
    OwnerAuthor

    Parent work completed by Issue #223 / PR #231 and merged into dev at f00373f7cf28ab5288691b1aebfc696ec187e9c9. Exact candidate CI was 7/7 successful and the commit-bound review passed with no findings.

  11. removed
    orch:readyOrchestrator V6: valid hashed contract is complete and dispatchable
    on Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P1Priority 1: current iteration goalbugSomething isn't workingready-for-humanRequires human implementationtier:heavyOrchestrator provider tier: heavywayfinder:taskWayfinder implementation task

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions