fix: suppress signatures for errored streams

[hanakannzashi]

Summary

• do not fetch or store a provider signature after an upstream stream error
• release the provider-routing signature pin instead
• add an E2E regression test that asserts no successful signature record is created

This is stacked on #996, which handles a clean EOF without an upstream terminal frame.

Fixes #997

Testing

• cargo fmt --check
• cargo test -p services completions:: --lib
• cargo test -p api --lib
• cargo test -p api --test e2e_all --no-run

Focused E2E execution is blocked locally by PostgreSQL authentication during test bootstrap.

[claude]

Review: fix: suppress signatures for errored streams

Traced the change through InterceptStream::create_signature_future -> release_chat_signature_pin_impl -> MockProvider::unpin_chat_connection, and cross-checked the route tail in crates/api/src/routes/completions.rs.

The core change is correct. Inverting to last_error.is_some() || !saw_upstream_done_marker closes the case where an errored stream still fetched and published a provider_tee signature over bytes that no longer match what the client received (the route injects an SSE error frame and synthesizes [DONE]). It is also consistent with the route, which already gates both gateway paths on error_count_final == 0 (completions.rs:2022, completions.rs:2065), and with skip_provider_chat_signature (completions.rs:1537), so there is exactly one pin release on every path -- matching the test assertion assert_eq!(mock.unpinned_chat_ids(), vec![chat_id]). No double-unpin, and no regression for the clean-[DONE] path in test_raw_stream_with_upstream_done_retains_provider_signature. The existing with_stream_error_after tests in usage_chat_completions.rs / provider_errors.rs assert nothing about signatures, so nothing else breaks.

No critical issues. Two non-blocking follow-ups:

1. Errored streams now return an opaque 404 from GET /v1/signature/{chat_id}

This is the user-visible consequence of the fix. check_fallback_conditions (crates/services/src/attestation/chat_signatures.rs:237) maps only ClientDisconnect to a structured result:

match stop_reason {
    Some(StopReason::ClientDisconnect) => Ok(SignatureLookupResult::Unavailable { ... }),
    _ => Err(AttestationError::SignatureNotFound(...)),
}

record_usage_and_metrics stores StopReason::from_completion_error(err) for these streams (completions/mod.rs:381), so ProviderError / Timeout / RateLimited are already in the DB and fall through to SignatureNotFound. Clients that previously got a (wrong) signature now get a bare "not found" instead of a reason. Worth extending, e.g.:

Some(StopReason::ProviderError | StopReason::Timeout | StopReason::RateLimited) => {
    Ok(SignatureLookupResult::Unavailable {
        error_code: "STREAM_ERROR".to_string(),
        message: "Verification not available: the stream ended with an upstream error.".to_string(),
    })
}

2. Test asserts storage, not the client contract

test_raw_stream_error_does_not_store_a_signature checks COUNT(*) FROM chat_signatures directly. A GET /v1/signature/{chat_id} assertion would pin the behavior clients actually observe (and would make item 1 explicit). Also note the mock always truncates [DONE] when with_stream_error_after is set, so the new last_error.is_some() disjunct is only ever exercised together with !saw_upstream_done_marker -- the error-after-[DONE] case the condition also now covers is untestable with the current mock. Fine to leave, just flagging the coverage boundary.

Pre-existing, out of scope

create_signature_future runs only from the Poll::Ready(None) arm; Drop calls only record_usage_and_metrics. If the client disconnects mid-stream the routing pin is never released. Not introduced here (the route keeps polling after an in-stream Err, so the error path does reach EOF), but it is the same lifecycle the comment block claims to own.

Logging reviewed against CLAUDE.md -- no new log statements, and the test asserts only on IDs/counts. Approved.

[ironloopai]

Review · Status

🟩 Completed

IronLoop completed the review and posted it to GitHub.

Result

Open submitted review →

Run details

• Run: d6729709-f45f-44c1-9b8b-e1ab751e7cbf
• Base: codex/gateway-sign-synthetic-done-994 at 7880388
• Head: codex/no-signature-after-stream-error-997 at 4384a9c
• Created: 2026-09-01 06:32 UTC
• Updated: 2026-09-01 06:58 UTC

_{Automatic trigger · attempt 1 of 3 · completed in 25m 42s}

[github-actions]

✅ OpenCodeReview: Review failed: 0 finding(s); 2 of 2 selected item(s) failed.

[ironloopai]

Review · Summary

🟢 No actionable findings

No actionable issues found in the reviewed change.

Validation

• ✅ Rust formatting — Formatting checks passed.
• ✅ Focused completion tests — 50 completion-focused service tests passed.
• ✅ API regression-test build — The API end-to-end test target, including the added regression test, compiled successfully.
• ✅ Signature-pin lifecycle — The focused release-without-storage lifecycle test passed.

Review details

• Run: d6729709-f45f-44c1-9b8b-e1ab751e7cbf
• Attempts: 1

[think-in-universe]

No actionable findings. The error path now releases the routing pin without storing a successful signature.

[think-in-universe]

@ironloopai review

[ironloopai]

Review · Status

🟩 Completed

IronLoop completed the review and posted it to GitHub.

Result

Open submitted review →

Run details

• Run: ed3ce10e-9c90-4e4e-b1bb-6b2dc628774f
• Base: codex/gateway-sign-synthetic-done-994 at fc0113b
• Head: codex/no-signature-after-stream-error-997 at 49dbdc1
• Created: 2026-09-03 06:41 UTC
• Updated: 2026-09-03 07:07 UTC

_{Manual command by think-in-universe · attempt 1 of 3 · completed in 26m 29s}

[ironloopai]

Review · Summary

Found one medium-severity error-lifecycle gap in the Responses API path.

Findings: 🟠 Medium 1

_{Code-specific findings are attached to the diff.}

Validation

• ✅ Formatting — Rust formatting check passed.
• ✅ Completions unit tests — 50 focused completions unit tests passed.
• ⚪ Focused E2E signature test — Not run. The database-backed regression test could not execute because PostgreSQL on localhost:5432 was unavailable; the runner has no local PostgreSQL or container runtime.

Review details

• Run: ed3ce10e-9c90-4e4e-b1bb-6b2dc628774f
• Attempts: 1

[think-in-universe]

Approved.

Please resolving the pending comments before merge