fix: gateway-sign streams with synthesized done markers - #996
fix: gateway-sign streams with synthesized done markers#996hanakannzashi wants to merge 2 commits into
Conversation
2665baf to
7880388
Compare
|
Review: gateway-sign streams with synthesized done markers No prior review threads on this PR, so this is a first pass. The core idea is right: a provider TEE signature covers only the upstream byte stream, so it must not be published once Cloud API appends its own terminator. Pre-seeding Issues below.
let may_need_synthesized_done_gateway_signature = model_attestation_supported == Some(true);
So with metadata
// `None` means metadata was unavailable, not that the model is unattested —
// hash so the tail can still mint a signature rather than publishing none.
let may_need_synthesized_done_gateway_signature = model_attestation_supported != Some(false);
Now an E2EE stream whose upstream omits the terminator stores a
4. Outer timeout can leave a half-written signature pair
5. Per-chunk String allocation plus async mutex on the hot path
let mut chat_id = public_signature_chat_id.lock().await;
if chat_id.is_none() {
*chat_id = Some(match chunk {
inference_providers::StreamChunk::Chat(c) => c.id.clone(),
inference_providers::StreamChunk::Text(c) => c.id.clone(),
});
}That 6. Dead branch
if event.is_done_marker() {
if gateway_signature_enabled { return None; }That arm is guarded by 7. Test readability
Not blocking, but there is no coverage for the two behaviour changes above: an E2EE stream without an upstream terminator, and an attested stream with |
|
✅ OpenCodeReview: Review failed: 0 finding(s); 4 of 4 selected item(s) failed. |
think-in-universe
left a comment
There was a problem hiding this comment.
Found one incorrect signature-finalization branch.
|
@ironloopai review |
Review · Status🟩 CompletedIronLoop completed the review and posted it to GitHub. ResultRun detailsManual command by think-in-universe · attempt 1 of 3 · completed in 23m 22s |
There was a problem hiding this comment.
Review · Summary
🟢 No actionable findings
No additional actionable findings beyond the existing review feedback.
Validation
- ✅ e2e_all test target — The changed end-to-end test target compiles successfully.
Review details
- Run:
15a8bdcf-733f-43dd-9cc7-b015eabb1c1b - Attempts: 1
Summary
[DONE]frameThis is stacked on #992 because both changes use the same Gateway-signing lifecycle.
Fixes #994
Testing
cargo fmt --checkcargo test -p services completions:: --libcargo test -p api --libcargo test -p api --test e2e_all --no-runFocused E2E execution is blocked locally by PostgreSQL authentication during test bootstrap.