perf(attestation): write both chat signatures in one statement

[PierreLeGuen]

Stores provider or gateway chat signatures with one multi-row upsert before stream completion, reducing database round trips. Duplicate algorithms retain the last supplied signature, matching the previous upsert result; existing repository implementations keep a sequential fallback.

Streaming provider fetches share a four-second deadline within the existing five-second finalization budget. The remaining second allows a successful first fetch to be persisted when a later algorithm stalls. Returned fetch errors also preserve successful fetches, and timeout failures use the existing verification metrics and pin cleanup. A database operation that exceeds the overall deadline can still prevent storage. Background signature fetches retain their existing timeout behavior.

The batch is atomic: a database rejection rolls back the whole batch. This differs from separate writes, where an earlier signature could already have committed.

Validation: 168 attestation/completion service tests passed, including six new service regressions covering normal batching, partial errors, stalled fetches, pin cleanup, and healthy six-second background fetches. The stalled-second regression fails with the old implementation. Formatting and strict Clippy passed. A combined #1045 + #1046 revision passed 224 relevant PostgreSQL 16 E2E tests covering signatures, streaming, encryption, credits, and billing.

Related credit-check optimization: #1044. Prepared-statement reuse for the batch is stacked in #1046; merge this PR first.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-11T00:01:16.306162Z	63ca0f9	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Review — perf(attestation): write both chat signatures in one statement

Batching the two upserts is the right call, and keeping add_chat_signature as the default trait impl keeps mocks working. Two issues to resolve before merge.

---

⚠️ 1. Duplicate signing_algo in one batch is now a hard error (was last-write-wins)

crates/database/src/repositories/attestation.rs:79-134

A multi-row INSERT ... ON CONFLICT (chat_id, signing_algo) DO UPDATE fails with SQLSTATE 21000 — "ON CONFLICT DO UPDATE command cannot affect row a second time" — whenever two rows in the same statement collide on the arbiter index. Postgres raises this before touching the table; it is not reachable via retry.

The batch can contain a collision, because the provider path does not key on the requested algorithm:

// crates/services/src/attestation/chat_signatures.rs:80-113
for algo in ["ecdsa", "ed25519"] {
    let provider_signature = provider.get_signature(chat_id, Some(algo.to_string())).await?;
    signatures.push(ChatSignature {
        signing_algo: provider_signature.signing_algo, // ← backend-controlled, not `algo`
        ...
    });
}

signing_algo is deserialized verbatim from the backend JSON (crates/inference_providers/src/attested/nearai/mod.rs:1441-1444, :1377-1381). A backend that ignores the ?signing_algo= query param, echoes a normalized/uppercased value, or returns an empty string yields two rows with the same signing_algo.

Failure scenario: backend returns signing_algo: "ecdsa" for both requests → the single execute errors → RepositoryError → METRIC_VERIFICATION_FAILURE, and neither signature is persisted. Before this PR the two separate statements succeeded (second overwrote the first) and the chat stayed verifiable.

No test catches this: MockProvider::get_signature echoes the requested algo back (crates/inference_providers/src/mock.rs:1520), so every e2e run produces distinct algos.

Fix — dedupe in the repository, preserving the old last-write-wins semantics:

// A multi-row ON CONFLICT raises 21000 on in-statement duplicates, whereas
// the one-at-a-time path simply overwrote. `signing_algo` is backend-supplied,
// so collapse duplicates here and keep the last.
let mut deduped: Vec<ChatSignature> = Vec::with_capacity(signatures.len());
for signature in signatures {
    match deduped.iter_mut().find(|s| s.signing_algo == signature.signing_algo) {
        Some(existing) => *existing = signature,
        None => deduped.push(signature),
    }
}
let signatures = deduped;

Worth a unit test asserting two same-algo entries upsert to a single row rather than erroring.

---

⚠️ 2. Provider path no longer persists ecdsa when the ed25519 fetch fails

crates/services/src/attestation/chat_signatures.rs:79-139

The store moved after the loop, so a failure on the second provider fetch now discards the first signature.

• Before: fetch ecdsa → store ecdsa → fetch ed25519 fails → returns Err, but ecdsa is in the DB and GET /v1/chat/signature?signing_algo=ecdsa succeeds.
• After: ed25519 fetch fails → ? returns before add_chat_signatures → nothing stored → the same lookup hits SignatureNotFound and falls through to check_fallback_conditions.

This is reachable: get_signature retries only 404s and fails fast on any non-404 status or transport error (nearai/mod.rs:1382-1406), so a backend that serves ecdsa but 500s / exhausts 404 retries on ed25519 silently loses a signature that used to be retrievable. In a rolling deploy with mixed backend versions this is exactly the mismatch you'd hit.

Fix — write whatever was collected instead of dropping it. Either turn the per-algo fetch error into a non-fatal continue that still records the failure metric, or store before propagating:

let fetch_result = async { /* existing loop, pushes into `signatures` */ }.await;
if !signatures.is_empty() {
    self.repository.add_chat_signatures(chat_id, signatures).await.map_err(|e| { /* ... */ })?;
}
fetch_result?;

If dropping the partial write is intentional (all-or-nothing verifiability), please say so in the PR description — it's a behavior change the diff doesn't call out.

---

Nit — test coverage moved off the production path

RecordingRepository / FailingRepository / HangingRepository (crates/services/src/attestation/chat_signature_lifecycle_tests.rs:50,78,104) implement only add_chat_signature, so they now exercise the default loop, not the batch SQL that runs in prod. The new statement builder's only coverage is the e2e happy path. A direct PgAttestationRepository::add_chat_signatures test (empty vec, single, two distinct algos, two duplicate algos) would pin both the parameter indexing and the dedupe from #1.

---

Checked and fine: parameter binding is fully placeholder-based with no interpolated values — no injection risk; 12 params is far under the 65535 limit; the empty-vec early return is correct; a single statement is implicitly atomic so no explicit transaction is needed; the ON CONFLICT clause text is byte-identical to the original; no customer data in the new or changed log statements.

⚠️

[ironloopai]

Review · Status

🟩 Completed

IronLoop completed the review and posted it to GitHub.

Result

Open submitted review →

Run details

• Run: e1ed8e19-2e49-46a9-85c2-74c28e5e3f8a
• Base: main at 7c852f2
• Head: perf/batch-chat-signature-writes at 63ca0f9
• Created: 2026-09-11 00:03 UTC
• Updated: 2026-09-11 00:06 UTC

_{Automatic trigger · attempt 1 of 3 · completed in 2m 56s}

[ironloopai]

Review · Summary

🟢 No actionable findings

No additional actionable findings beyond the feedback already captured on the pull request.

Validation

• ✅ Static review — Inspected the complete batching change and its database and attestation call paths.

Review details

• Run: e1ed8e19-2e49-46a9-85c2-74c28e5e3f8a
• Attempts: 1

[github-actions]

🔍 OpenCodeReview found 2 issue(s) in this PR.

• ✅ 2 posted as inline comment(s)
• 📝 0 posted as summary

⚠️ 1 warning(s) occurred during review.

[PierreLeGuen]

Both points addressed in the follow-up commit:

1. Duplicate signing_algo in one batch — the repository now collapses duplicates before building the statement and keeps the last one (last_signature_per_algo), which is exactly what the one-at-a-time upsert produced. Unit-tested for distinct, duplicate and empty inputs.
2. Partial fetch dropped the first signature — the provider path now stores whatever was fetched before propagating a later fetch error, so a backend that serves ecdsa but fails ed25519 still leaves the chat verifiable by ecdsa, as before. The fetch error is still returned and still counts as a verification failure.

On the nit: the lifecycle mocks have no provider stub that can fail a single algorithm, so I did not add a unit test for the partial path; the e2e set that exercises both signature stores (chat_encryption, attestation_auth, auto_redact, general, client_disconnect, 110 tests) passes locally.

[lloydmak99]

Batching both chat signatures into a single upsert looks correct: parameter indexing (base = index*6, $1–$6/$7–$12) and column order match the single-row path, the ON CONFLICT ... DO UPDATE clause is byte-identical, dedup keeps the last signature per algo (matching prior one-at-a-time semantics), and the earlier duplicate-signing_algo and partial-write concerns are resolved in 97529606/d8ad797b.

Optional follow-up (non-blocking):

• crates/services/src/attestation/chat_signatures.rs:95-149 — if the ed25519 fetch fails and the batch store of the already-fetched ecdsa signature also fails, two METRIC_VERIFICATION_FAILURE counts (INFERENCE_ERROR + REPOSITORY_ERROR) are recorded where the old path recorded one. Metrics-only, no correctness/data impact; skip the stored metric when fetched is already Err if it matters.

Checks: git diff --check passed and GitHub CI is green (E2E, integration, unit, lint, cargo audit/deny); local cargo test for the attestation repository tests could not run because the environment has no C compiler to link.