fix(responses): reject unscoped reasoning replay - #1474
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change removes the shared ChangesReasoning replay scope isolation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
Wibias
left a comment
There was a problem hiding this comment.
Reviewed the replay-cache privacy change. I did not find an additional blocker in this head. Removing the implicit process-wide global namespace at every read/write call site and making unscoped cache access a no-op closes the demonstrated cross-request call-id collision without inventing a replacement identity.
I am not approving this head because it is currently not mergeable against the latest dev. Please rebase/resolve conflicts and request a short re-review of the integrated cache call sites.
4e22f98 to
1ecb5a7
Compare
|
|
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
Thanks @luvs01 for tightening this up. This is useful because provider-generated tool call IDs are not globally unique, so removing the unscoped global reasoning replay namespace prevents reasoning from one conversation being recovered by an unrelated request with the same call ID. Keeping replay only when an explicit thread scope exists gives us the compatibility benefit without the cross-request privacy risk. Merged, thank you! |
Summary
"global"fallback from the bridge, OpenAI chat adapter, Responses core, image loop, and web-search loopRoot cause and impact
Provider-generated tool call IDs such as
call_1are not globally unique. The existing optional scope still fell back to a process-wide namespace wheneverx-codex-parent-thread-idwas absent, so an unrelated unscoped continuation could recover another request's cached raw reasoning for up to the cache TTL and forward it asreasoning_content.There is no safe stable conversation identity for generic unscoped clients. This change therefore fails closed on cache read/write when the explicit client thread scope is absent. Preserve-listed DeepSeek models continue to use the existing minimal placeholder on a cache miss, so an unscoped request remains valid without replaying unrelated reasoning.
Refs #950.
Verification
tsc --noEmitpassedtsc --noEmitpassedbun run privacy:scanpassedgit diff --checkpassedChecklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit