fix(routing): isolate policy retry body snapshot

[luvs01]

Motivation

• A policy-fallback retry path retained a reference to the core-parsed, mutable request body so an in-place agent-task recovery could swap ciphertext for plaintext and that plaintext could be serialized into a retry and end up persisted to disk.

Description

• Take an immutable one-time snapshot of the initially parsed wire body in handleResponsesWithPolicyFallback by structuredCloneing the parsed object in the onRequestBodyParsed hook instead of retaining the original mutable object reference (in src/server/responses/policy-fallback.ts).
• Keep retry construction using the preserved rawBody snapshot so attempt-local mutations (such as recovered plaintext) cannot be serialized into a subsequent retry and lose the object-identity non-persistence marker.
• Add a focused regression test that mutates the first attempt's parsed input, forces a retryable response, and asserts the next candidate receives the original (immutable) snapshot rather than the mutation (tests/routing/routing-policy-fallback.test.ts).
• Preserve the original design constraint that request parsing remains the core handler's responsibility and that the fallback wrapper does not clone the Request body itself.

Testing

• Ran the focused suite ./node_modules/.bin/bun test tests/routing/routing-policy-fallback.test.ts, which passed (13 tests, 0 failures).
• Ran bun run typecheck, bun run structure:check, and bun run privacy:scan, all of which completed successfully.
• Attempted the full bun run test; focused checks and the required programmatic validations passed, while an unrelated test in the parallel full suite produced a separate assertion failure and did not affect this regression coverage.

---

Codex Task

---

Summary by CodeRabbit

• Bug Fixes

  • Improved policy fallback reliability by ensuring retries consistently use the original request data.
  • Prevented changes made during an earlier attempt from affecting subsequent fallback attempts, including nested request content.

• Tests

  • Added coverage for request data preservation across retries, including deeply nested content and successful completion after an initial rate-limit response.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 71a1d75b-1807-4a90-be54-aa21c86509e2

📥 Commits

Reviewing files that changed from the base of the PR and between 6add6ae and 7f20f93.

📒 Files selected for processing (1)

• tests/routing/routing-policy-fallback.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The policy fallback stores one deep-cloned snapshot of the parsed request body. Tests verify that retries receive the original input after shallow or nested mutations during the first attempt.

Changes

Policy fallback snapshot

Layer / File(s)	Summary
Snapshot capture and retry validation src/server/responses/policy-fallback.ts, tests/routing/routing-policy-fallback.test.ts	onRequestBodyParsed captures the parsed body only when rawBody is null and stores a structuredClone. Tests verify that direct and nested mutations during a 429 retry do not change the original "hello" input, and that the fallback returns status 200.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 7f20f

The fallback now preserves original request input across retries, including nested mutations. Covered behavior is merge-ready with no remaining actionable risk.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: isolating the request body snapshot used for policy fallback retries.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ READY

• all PR quality gates passed.

Hygiene

✅ Deterministic PR hygiene checks passed.

[lidge-jun]

리뷰 · 우선순위 70 / 80

정책 프로필로 고른 첫 모델이 429나 5xx로 막히면, 다음 후보에게 같은 요청을 다시 보냅니다. 예전 코드는 처음 읽은 본문 객체를 그대로 잡아 두었습니다. 복호화와 본문 정리는 그 객체를 그 자리에서 고칩니다. 재시도는 처음 들어온 글이 아니라, 풀어 둔 과제 문장이나 지워 둔 조각을 다음 제공자에게 넘겼습니다.

이 PR은 본문을 처음 읽은 순간 structuredClone으로 깊은 복사본을 남겨 두고, 재시도는 그 복사본만 JSON으로 만듭니다. 첫 시도가 나중에 본문을 바꿔도 복사본은 그대로입니다. 같은 호출에서 본문 콜백이 다시 와도 복사본을 갈아끼우지 않습니다. 테스트는 첫 시도가 input을 "recovered plaintext"로 바꾼 뒤 429를 내면, 둘째 시도의 input이 "hello"인지 봅니다. base는 dev입니다. types.ts와 config.ts 분할과는 무관하고, 같은 수정의 다른 열린 PR은 없습니다.

src/server/responses/policy-fallback.ts:149 - 복사는 이 요청이 정책 재시도인지 알기 전에 일어납니다. onRequestBodyParsed는 라우트 판정보다 앞입니다 (request-prepare.ts:174). 첫 시도가 성공해도 본문 전체를 한 번 더 만듭니다. 대화가 길거나 그림이 크면 성공한 요청도 메모리와 시간을 더 씁니다.

tests/routing/routing-policy-fallback.test.ts:125 - 테스트는 맨 위 input을 다른 문자열로 바꿉니다. 그 정도면 얕은 복사({ ...body })도 통과합니다. 실제 누수는 그 아래입니다. sanitizeEncryptedContentInPlace는 input 배열을 그 자리에서 splice하고, 과제 복구의 injectAssignment도 같은 배열 안을 고칩니다. 얕은 복사본은 그 배열을 같이 가리키므로, 복사를 얕게 되돌려도 이 테스트는 그대로 초록입니다.

메인테이너의 판단이 필요한 지점

주석은 재시도가 객체에만 붙는 표시를 잃지 않는다고 합니다. 재시도는 복사본을 JSON.stringify로 새 요청에 넣습니다 (requestWithCandidate, policy-fallback.ts:62). markBodyNonPersistable은 본문 객체를 WeakSet에 넣을 뿐이라, 그 표시는 JSON에 실리지 않습니다. 다음 시도는 본문을 새로 읽고, 복호화가 다시 되면 그때 다시 표시합니다. 이번 복사가 지키는 것은 바뀐 글을 다음 제공자에게 보내지 않는 일입니다.

모든 응답 요청에서 본문을 통째로 복사할지도 정하면 됩니다. PR은 아직 초안입니다.

너의 추천

방향은 유지하면 됩니다. 재시도 본문은 처음 들어온 글이어야 하고, 첫 시도가 풀어 둔 문장을 다음 제공자에게 보내면 안 됩니다. 머지 전에 input 배열 안을 고치는 테스트를 하나 두면, 얕은 복사로 되돌아가는 일을 테스트가 잡아 냅니다. 성공한 요청까지 복사하는 비용이 크면, 정책이 아닌 경로는 복사하지 않는 자리를 따로 보면 됩니다. 닫을 중복 PR은 없습니다.

이 댓글은 grok-bot이 작성했습니다

[luvs01]

Review feedback applied on a240fc9: added the retry snapshot survives mutation inside the input array - the first attempt splices a nested input element in place before returning 429, and the second attempt must see the original array. A shallow copy now fails this test, which the top-level field swap alone could not catch. On the always-clone cost and the WeakSet marker: noted - the clone happens before policy classification, so non-policy requests pay one extra copy; the marker re-marks on the next attempt own parse, which is the correct place. Both left as documented tradeoffs. Tests: routing-policy-fallback 14 pass.

[luvs01]

Consolidated into #5553 as a single related-function aggregate.

Source head: 7f20f93a6112c5c4d241eeb35d0006663237a8cd. Replacement head: 67c4f579e4fb5201dcd36be225ee54306d78e63e.

Implementation 8e2a0fe and nested-mutation regression a240fc9 match carried db854bf and b037810 by stable patch ID. Empty CI commit 44571de and the dev synchronization merge have no omitted implementation/resolution delta. The final follow-up clarifies that request-body values are deep-snapshotted while identity metadata is re-established per attempt. The aggregate passed 604 distinct focused tests and preserves prepared contribution files after latest-dev integration. Deep-copy cost remains an explicit tradeoff; full-suite completion, exact-head hosted CI and security review remain pending on #5553.

Closing this duplicate standalone review entry as part of the requested consolidation after verifying coverage. This is not a merge or release claim; remaining integration checks and reviews are tracked on the replacement. Original branches are retained.