fix(call_logs): prevent primary key collisions on retry and fallback attempts

[HouMinXi]

Description

A 6-character slice of crypto.randomUUID() was stored as call_logs.id. That is a 24-bit key on a TEXT PRIMARY KEY. Against the rows already in the table, a new attempt collides and SQLite drops it with UNIQUE constraint failed: call_logs.id. An explicit id that is already taken fails the same way. This is a birthday collision against stored rows, not a concurrency race. That diagnosis was already in #14341, about 14 hours before #14451.

• Use the full crypto.randomUUID() for the request trace in chatCore.ts.
• persistAttemptLogs no longer uses traceId as the primary key. The dashboard correlation token is unchanged.
• saveCallLog catches UNIQUE on an explicit id and retries once with a generated UUID instead of dropping the row.
• Regression coverage is in tests/unit/call-logs-id-collision.test.ts.

One handleChatCore invocation does not persist twice at the current tip. Of the 16 persistAttemptLogs call sites, 14 return immediately and the other two are mutually exclusive branches. The test calls persistAttemptLogs twice directly to show that a shared dashboard traceId no longer means a shared primary key.

Production reports of UNIQUE constraint failed: call_logs.id came from @VIPKaiser in #14338.

Fixes #14451

Verification

• tests/unit/call-logs-id-collision.test.ts: 4/4 PASS
• tests/unit/chatcore-attempt-logging.test.ts: 7/7 PASS
• tests/unit/attempt-logging-early-keepalive-merge.test.ts: 4/4 PASS
• tests/unit/video-bridge-log-redaction.test.ts: 7/7 PASS
• Combined: 22/22 PASS
• npm run typecheck:core: clean on the previous head. This follow-up only drops a duplicate after(), replaces a stale comment, and credits the changelog.

#14341 and #14343 touch the same two locations. Only one of the three can land as-is.

Maintainer rework (merge-batch 2026-09-24)

• Merged release/v3.8.51 into the branch (clean).
• Confirmed the early-keepalive poll fix (d99a77f): video-bridge-log-redaction.test.ts is 13/13 with it.
• Restored pendingRequestId: ctx.pendingRequestId on the saveCallLog entry in persistAttemptLogs. It is not the row key; saveCallLog uses it to route token usage to the live in-memory request row (fix(dashboard): retain token usage for in-memory request rows #14324). Without it, call-log-in-memory-usage.test.ts "chat attempt logging connects its trace id to the live pending request id" fails (red on the merged tree, green after the fix).
• Validation: 554 tests across every unit file that touches call-log persistence / trace ids, typecheck:core, open-sse typecheck, eslint on the changed files, file-size.
• Still open for the squash: Co-authored-by for @abhisheksharma2411 (fix(logging): widen the trace id so it can key call_logs #14341 carried the same diagnosis first). The "second mechanism" in the body (one handleChatCore persisting twice) was not reproducible at the tip.

[HouMinXi]

Code Review Status

• Commit: b66f298f7b
• Forge Review Pipeline:
  • 3 Passes: qodo, expert, adversarial (all completed)
  • Product Findings: 0 CONFIRMED
  • Status: Clean
• Test Matrix:
  • tests/unit/call-logs-id-collision.test.ts: 4/4 PASS
  • tests/unit/chatcore-attempt-logging.test.ts: 7/7 PASS
  • tests/unit/attempt-logging-early-keepalive-merge.test.ts: 4/4 PASS
  • tests/unit/video-bridge-log-redaction.test.ts: 7/7 PASS
  • Combined suite: 22/22 PASS

[diegosouzapw]

This is the right shape and the one I'd like to land of the three — decoupling the primary key from
the trace id fixes the actual cause rather than widening it, and the generateLogId() change closes
the second surface at the same time. Two credit items before merge, though: PR #14341
(@abhisheksharma2411) already carried this exact diagnosis — 24-bit id space, id TEXT PRIMARY KEY,
the birthday-against-stored-rows table, and the explicit "this is not a concurrency race" — about 14
hours before #14451 was filed, so a Co-authored-by trailer for him belongs on the squash, and
@VIPKaiser's production evidence in #14338 should get a changelog credit.

Three small things: tests/unit/call-logs-id-collision.test.ts registers after() twice (double
resetDbInstance); the #13481 comment above the now-idless saveCallLog call is stale; and the
second mechanism in the body ("one handleChatCore invocation persists more than once, giving a
deterministic UNIQUE") does not reproduce at the tip — 14 of the 16 persistAttemptLogs call sites
are immediately followed by return and the other two are mutually exclusive branches, and your own
test proves it by calling persistAttemptLogs twice directly rather than by driving chatCore.
Either drop that claim or point at the real double-persist path. Note also that #14341 and #14343
touch the same two locations, so only one of the three can land as-is.

[HouMinXi]

Addressed the three items, and the two credits.

• Co-authored-by: Abhishek Sharma <abhicse24@gmail.com> is on the squash. The 24-bit id space, id TEXT PRIMARY KEY, birthday-against-stored-rows, and "not a concurrency race" were already in fix(logging): widen the trace id so it can key call_logs #14341.
• Changelog fragment now credits @VIPKaiser's production UNIQUE constraint failed: call_logs.id reports in fix(backend): call_logs.id UNIQUE constraint race on concurrent inserts #14338.
• Dropped the second after() in tests/unit/call-logs-id-collision.test.ts. One resetDbInstance remains.
• Replaced the #13481 comment above the now-idless saveCallLog call. The primary key is a fresh UUID. correlationId still pairs the row with request.started.
• Dropped the claim that one handleChatCore invocation persists more than once. I could not find that path at the tip: 14 of the 16 persistAttemptLogs sites return immediately, and the other two are mutually exclusive. The test still calls persistAttemptLogs twice directly, which is what it actually proves.

Head is ca22bb6b3e. The four related test files are 22/22.

Understood that #14341 and #14343 touch the same two locations, so only one of the three can land as-is.

[abhisheksharma2411]

Hi @HouMinXi — I had a branch on this (#14341) and @diegosouzapw has picked yours, which I think is the right call: decoupling the primary key from the trace id closes #13099's surface at the same time, where widening the id (what mine did) leaves the column carrying two concerns. Went through this one properly rather than just standing down. It's a good change — the retry-on-UNIQUE for explicit ids is a nicer touch than I expected, and all four of your tests pass locally for me.

One thing I think is a genuine gap, and I ran it rather than eyeballing it.

correlationId || traceId makes the traceId → row link conditional

After this PR the only column carrying traceId is correlation_id, written as:

correlationId: correlationId || traceId,

correlationId is a caller-supplied parameter of handleChatCore (chatCore.ts:484, default null). When a caller does pass one, it wins, and traceId then lives on no column at all — while request.started / request.completed are still emitted with id: traceId. So the dashboard event and its row become unlinkable for exactly those requests.

Before this PR the link was unconditional, because call_logs.id was the traceId. So it's a narrow regression rather than a pre-existing gap.

Every test and fixture here passes correlationId: null — including dashboard-request-failed-redaction-probe.ts:74, which is why its new getCallLogs({ correlationId: traceId }) lookup at :103 finds the row. Nothing currently exercises the other branch.

I wrote a probe against your head (ca22bb6b) with a control, so it isn't just my reading of the code:

✔ CONTROL: with correlationId null, the row is findable by traceId
✔ PROBE:   with a caller-supplied correlationId, traceId is on no column at all
      row.id=540c8016-91a6-41f0-b17a-b098639e6242
      row.correlation_id=caller-supplied-abc
      traceId=22222222-2222-4222-8222-222222222222
      rows findable by traceId: 0

Happy to push the probe to your branch as a test if that's useful — say the word and it's yours, no credit needed.

On the fix, I'd lean toward a dedicated trace_id column over overloading correlation_id, for the same reason your PR is better than mine: correlation_id already means "the caller's identifier for this request", and making it sometimes mean "our internal trace token" is the two-concerns-in-one-column problem moved one column across. A trace_id column written unconditionally keeps the dashboard pairing exact and leaves correlation_id meaning one thing. If that's more surgery than you want here, correlationId ?? traceId wouldn't help (the issue is a non-null caller value), so the cheap version is probably storing traceId unconditionally somewhere and leaving correlation_id alone.

Two much smaller things

• isCallLogIdCollision matches /UNIQUE constraint failed: call_logs\.id/i on the message as a fallback. That's fine today, but it's a string match on a driver message — worth a comment saying the code checks above it are the real path and this is the belt-and-braces, so nobody later "simplifies" it down to only the regex.
• The retry comment says "Keep the already-written artifact path; only the SQLite primary key is regenerated." I couldn't find anywhere the artifact path is derived from the row id, so I believe this is correct — but since it's the one place where the row and its artifact could drift apart, a one-line assertion in the retry test that the retried row still opens its artifact would make that guarantee non-accidental.

Thanks for picking this up — and sorry for the duplicate effort, mine was open before the issue was filed and I didn't see yours until Diego pointed at it.

[maxmad64bis]

Independent corroboration from a single-node production install (image deploy-v3.8.51-20260921, one Node process): 21 UNIQUE constraint failed: call_logs.id lines in two days (14 on 2026-09-23, 7 on 2026-09-24, latest 13:46Z today), against 32,681 6-char ids out of 35,567 call_logs rows. Each error is a silently dropped row while the chat itself succeeds, matching the birthday-collision mechanism in #14451 exactly.

This is the PR of the three I would like to see land: decoupling the primary key from the display token fixes the cause, and the retry-on-UNIQUE closes the residual surface instead of dropping the row. Supporting a merge.