test(windows): create the external prompt fixture parent directory - #2972
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughTest 34 now creates the external base file directory, writes its configuration with ChangesCodex prompt route tests
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change prepares the Windows test fixture correctly and strengthens its path-selection assertion without changing production behavior. No actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Title checkExplanation The title accurately describes the main change: the Windows test fixture now creates the external prompt fixture parent directory before writing the file. It is specific, concise, and consistent with the changes in tests/codex-prompt-route.test.ts. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
리뷰 · 우선순위 66 / 80설명 이 PR은 #2968이 고친 같은 테스트가 Windows에서 한 겹 더 깨지던 자리를 메웁니다. 지금 고친 내용은 테스트 한 파일입니다. 부모 디렉터리를 생산 경로( 라인 - 문제 tests/codex-prompt-route.test.ts 1138줄 근처 - tests/codex-prompt-route.test.ts TOML 기록 - .github/workflows/ci.yml (이 PR 밖) - Windows가 여전히 dispatch 전용이라, 이번에도 진짜 확인은 디스패치 런입니다. 메인테이너의 판단이 필요한 지점
너의 추천 디스패치 Windows CI가 초록이면 이 댓글은 grok-bot이 작성했습니다 |
Ingwannu
left a comment
There was a problem hiding this comment.
Approved exact head 97a69728cfe9cbf1d0081bfbc2ebb69a5c3aa412 against dev@d2a802275b6c16c3194e9a9cc55645c98e92089f.
The fixture change is scoped and correct: it creates the Windows parent directory before writing, uses an independent compatible string encoder, and proves readPromptLayers(...).baseSelection before the in-flight fingerprint assertion. Current dev is two commits ahead with no overlapping path.
Local isolated execution produced 74/75; the only failure is the unrelated Bun 1.3.14 TOML-overflow premise in case 39, and current clean dev@c2b64dbc3 fails the identical assertion. The changed case 34 passes locally.
More importantly, the exact-head workflow-dispatch run exercised tests\codex-prompt-route.test.ts on Windows and case 34 passed in windows 1/4. The normal exact-head Cross-platform CI run 33289202104 is fully green. The dispatch run still has an unrelated red windows 2/4 job (ocx init piped stdin, launchd retargeting, and two storage-coordinator timing assertions), so this approval is for the patch and its target regression; I am not merging while the author-defined dispatch gate remains red. No repository-wide security scan was run.
Summary
The last Windows-only failure from the dispatched leg. #2968 fixed the first layer of this case and the
run at
1494acd98showed it still failing — 4ms instead of 243ms, so it was failing earlier, whichis what identified the remaining defect.
The production path was never wrong. It decodes escaped paths, hashes the external path together with
the complete file bytes, and returns busy when the fingerprint differs.
What each layer was
Two distinct test defects stacked on the same case:
model_instructions_file = "C:\Users\..."as raw TOML.decodeBasicStringaccepts only
\\,\"and\n, so\Uwas rejected, the base was classifieddefault, theexternal file's bytes dropped out of the probe fingerprint, and the second request joined the stale
flight — exactly what the case exists to forbid.
Windows, where
external\base.mdis a nested path whose parent directory the test never created,so it failed with
ENOENTbefore reaching the assertion.The fixture now creates the parent directory, encodes the TOML value with an oracle independent of the
production encoder, and asserts
readPromptLayers(...).baseSelectionbefore the probe starts ratherthan assuming it. The backslash stays in the fixture so the TOML-escaping boundary is still exercised on
POSIX while being a real separator on Windows.
Verification
Based on
dev@d2a802275.bun test tests/codex-prompt-route.test.ts→ 75 pass, 0 failbaseSelection: defaultand one failure.bun x tsc --noEmit→ cleanA dispatched Windows run on this branch is the gate I will check before merging, since
platform-windowsdoes not run on push.
Checklist
Summary by CodeRabbit