fix: release signature pins on stream cancellation

[hanakannzashi]

Summary

• attach a route-level lifecycle guard once a streaming chat id is known
• release the provider-routing signature pin if the response body is dropped before normal finalization
• disarm the guard after the normal provider or Gateway signature lifecycle completes
• add a regression test for a client that stops after the first SSE frame

This is stacked on #1000.

Fixes #1001

Testing

• cargo fmt --check
• cargo clippy -p api --tests -- -D warnings
• 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.

[ironloopai]

Review · Status

🟩 Completed

IronLoop completed the review and posted it to GitHub.

Result

Open submitted review →

Run details

• Run: 6440e395-b2a6-4369-943b-d706de6ebf2b
• Base: codex/provider-signature-before-done-999 at 6230341
• Head: codex/release-signature-pins-on-disconnect-1001 at 7ac0086
• Created: 2026-09-01 06:45 UTC
• Updated: 2026-09-01 06:51 UTC

_{Automatic trigger · attempt 1 of 3 · completed in 5m 24s}

[claude]

Review — fix: release signature pins on stream cancellation

The core mechanism is sound: StreamSignaturePinGuard is moved into the terminal .map() closure, so it lives exactly as long as the body that Body::from_stream hands to Axum, and armed is cleared only after the tail future has run every release path. I traced the ordering: InterceptStream's Finalizing state (services/src/completions/mod.rs:559) always completes before the tail once(...) future, so there is no window where armed is cleared ahead of the real unpin, and a redundant release is a no-op (release_chat_signature_pin_impl bails when get_provider_by_chat_id returns None). No prior review comments on this PR.

Three issues below; the first is the one I would block on.

---

⚠️ 1. /v1/completions streaming has the identical leak and is not covered

convert_text_request_to_service sets skip_provider_chat_signature: false (completions.rs:1343), so an attestation-supported model on the text route pins exactly like the chat route does (inference_provider_pool/mod.rs:3314). The only unpin is InterceptStream::create_signature_future, which never runs when the client drops the body. The byte_stream built at completions.rs:2717 gets no guard, so #1001 stays open for POST /v1/completions.

Same treatment needed:

let stream_signature_pin_armed =
    Arc::new(AtomicBool::new(stream_chat_id.is_some()));
let stream_signature_pin_guard = StreamSignaturePinGuard {
    attestation_service: app_state.attestation_service.clone(),
    chat_id: stream_chat_id.clone(),
    armed: stream_signature_pin_armed.clone(),
};
// ... clear `armed` in the terminal `once(...)`, attach the guard via `.map()`

⚠️ 2. The guard snapshots stream_chat_id, but finalization uses the late-filled public_signature_chat_id

armed is initialized from stream_chat_id.is_some() (line 1709) — the value captured by the route's peek loop. That loop breaks with None on Some(Err(_)) or on hitting MAX_LEADING_CONTROL_EVENTS, while public_signature_chat_id is still populated later from the first chunk that carries one (lines 1787-1789 and 1871-1873). In that window the pool may already have pinned, yet the guard is permanently disarmed and the pin leaks — exactly the case this PR is meant to close.

Drop already defers to a spawned task, so resolve the id there instead of snapshotting it:

struct StreamSignaturePinGuard {
    attestation_service: Arc<dyn AttestationServiceTrait>,
    chat_id: Arc<tokio::sync::Mutex<Option<String>>>, // share public_signature_chat_id
    armed: Arc<AtomicBool>,
}
// inside the spawned task:
let Some(chat_id) = chat_id.lock().await.clone() else { return };

and arm unconditionally, letting the None case fall out inside the task.

⚠️ 3. The new comment at lines 2224-2227 is not accurate for non-attested models

// Every normal finalization path above either stores a signature (which releases its pin) or explicitly releases it.

When model_attestation_supported != Some(true) and neither gateway_signature_enabled nor auto_redact_enabled is set, all three branches are skipped. create_signature_future also bails without unpinning (completions/mod.rs:146), while the pool pinned unconditionally on the first StreamChunk::Chat (inference_provider_pool/mod.rs:3314 — no attestation check there). attestation_supported is a per-model DB column independent of provider type, so a non-attested model served by the attested NEAR AI provider is reachable configuration.

That leak is pre-existing, but clearing armed unconditionally means the new guard can never cover it. Either clear armed only on the branches that actually released, or add a trailing else { release_chat_signature_pin(chat_id).await } so the invariant the comment asserts is enforced rather than assumed.

---

Nits

• signature_verification.rs, new test — the for _ in 0..16 { yield_now().await } spin is timing-dependent; release_chat_signature_pin_impl awaits a pool lock before reaching unpin_chat_connection. A short tokio::time::sleep(Duration::from_millis(10)) in the loop body would make it robust without slowing the happy path.
• completions.rs:66 — catch_unwind only covers handle.spawn panicking on a shutting-down runtime (Handle::try_current already returns Err rather than panicking). It is a no-op under panic = "abort" and the default hook still prints the panic. Harmless, just buys less than it looks like.

⚠️

[ironloopai]

Review · Summary

Found one remaining stream-cancellation pin leak in the legacy completions route.

Findings: 🟠 Medium 1

🟠 Medium · Clean up pins for legacy streaming completions too

/v1/completions is translated through the same chat streaming service, which pins the first chat ID. If its response body is dropped after a frame, the normal finalizer never runs and that route has no equivalent cancellation guard, so the provider’s chat-ID routing entry remains pinned. Apply the cleanup there as well, or centralize it in the stream owner, and add a dropped legacy-stream regression test.

Validation

• ✅ Chat stream cancellation path — Static tracing confirms the new guard stays owned by the chat-completions response body and is disarmed after normal finalization.

Review details

• Run: 6440e395-b2a6-4369-943b-d706de6ebf2b
• Attempts: 1

[github-actions]

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

[think-in-universe]

Found one remaining cancellation edge case.

[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: 4b5a18e5-6475-47c1-b6e4-52a6f1ac6c42
• Base: codex/provider-signature-before-done-999 at db4e6b2
• Head: codex/release-signature-pins-on-disconnect-1001 at 63b6ec8
• Created: 2026-09-03 06:41 UTC
• Updated: 2026-09-03 07:05 UTC

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

[ironloopai]

Review · Summary

Found 1 medium-severity issue: canceled Responses API streams still leave provider-routing pins. The focused API end-to-end target compiled successfully; temporary local build artifacts were removed.

Findings: 🟠 Medium 1

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

Validation

• ✅ API end-to-end test target — Compiled successfully.

Review details

• Run: 4b5a18e5-6475-47c1-b6e4-52a6f1ac6c42
• Attempts: 1