fix(claude-sdk-oauth): ignore content-less user messages in continuity hashes - #791
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 794298f7f5
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,112 @@ | |||
| import type { Context } from "@earendil-works/pi-ai"; | |||
There was a problem hiding this comment.
Name the regression after issue 790
This commit explicitly fixes issue #790, but the new regression uses the placeholder 0000, defeating the repository's issue-to-test traceability convention. Rename it to 790-claude-sdk-oauth-empty-user-continuity.test.ts.
AGENTS.md reference: packages/coding-agent/test/AGENTS.md:L40-L40
Useful? React with 👍 / 👎.
…y hashes A user message with no content blocks is transient: present for one provider call and gone by the next. Hashing it shifted every later index, so the next turn reported sent_stream_diverged and re-sent the whole history despite an unchanged conversation. Filter content-less user messages out of the transmitted set. Tool results are untouched, so genuine rewrites of already-sent history still diverge.
794298f to
5baf13f
Compare
|
Good catch — renamed to
|
|
Verification note, since I can't produce a CI run from a fork. I mirrored Static stage on this commit:
Identical, so those failures are pre-existing here and not from this change. One of the three original failures was my own container artifact — Scoped results on this commit: I'm not claiming a full green |
Summary
changes.mdRoot cause
sentMessages()hashes everyuserandtoolResultmessage. A user message whosecontentarray is empty carries nothing to transmit and is transient: it is present for one provider call
and gone by the next.
Because it is hashed, its removal shifts every later index,
commonPrefixLengthstops short ofsentCount, anddecideNativeContinuityresolves the turn tosent_stream_diverged— forking thesession and re-sending the whole history even though the conversation never changed.
Captured on
2026.8.9-2from two consecutive provider calls of one session, both 203 messages:Index 200 is byte-identical. Only the empty message disappears, and the next decision is
flatten / sent_stream_divergedat 203 messages.Cost on one affected turn:
cacheReadcollapsed 582,409 → 61,594 withcacheWrite524,339, about8x its neighbouring turns. The same session logged 12
sent_stream_divergedevents in 12 minutes.Independent of #691 — this reproduces on
5275da1f5, well after #751 merged.Fix
sentMessages()drops user messages whosecontentarray is emptytoolCallIdand
toolNamestay hash-significantmessage that carries content
Why this cannot be an extension: the transmitted set is computed inside the builtin provider before
session-registry continuity is decided, and no extension hook can replace it. A
changes.mdsectionrecords the modification and its expected merge-conflict zones.
RED → GREEN
RED
Against
5275da1f5with only the test added:GREEN
The two control cases pass in both runs, so the guard still fails closed: a plain append stays a
delta, and a genuine rewrite of already-sent history still reportssent_stream_diverged.Verification
A deterministic container reproduces the bug and verifies the fix with no credentials and no tokens
spent. It clones
5275da1f5, runs the regression unpatched, applies the production patch, andre-runs; it exits 0 only when the unpatched tree fails specifically with
sent_stream_divergedandthe patched tree reports all four tests passing: