fix(manifest): normalize Windows extended-length and device path prefixes (#4442) - #4465
fix(manifest): normalize Windows extended-length and device path prefixes (#4442)#4465maoxin1234 wants to merge 2 commits into
Conversation
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This PR stays in draft until every box above is ticked. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change normalizes Windows extended-length and UNC path prefixes before computing Codex history path identities. Win32 integration tests cover local and UNC path equivalence. ChangesWindows history path identity
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Equivalent Windows history paths are normalized for manifest checks, with no confirmed remaining merge-blocking risk. 🚥 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 |
리뷰 · 우선순위 62 / 80이 PR은 이슈 #4442를 고치려는 초안이다. Windows에서 Codex v2 히스토리 매니페스트 무결성 검사가, 같은 파일을 가리키는 경로만 표기가 다를 때 서로 다른 파일로 보고 doctor/restore를 막는 문제다. 보고 증상은 지금 이 PR은 라인 57~68 근처(PR diff 기준 stripWindowsPathPrefix / codexHistoryPathIdentity) - identity를 바꾸면 롤아웃 비교뿐 아니라 백업 파일 이름도 바뀐다. #4442 댓글에 이미 적힌 경계다. 예전에는 tests/codex-integration/codex-history-provider.test.ts 새 테스트들 - stripWindowsPathPrefix UNC 분기 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Lane I1 of the contributor carry train: two Windows defects reported without a source branch. #4425 (wanjinxingoo-bot) — the generated LogonTrigger omitted UserId, so a non-elevated ocx service install failed and the resulting Access is denied was reported as an elevation problem. UserId is now emitted and the guidance stops blaming elevation. Pre-fix registrations carry an unscoped LogonTrigger and keep working; the live validator deliberately does not mark them unhealthy, because that would strand them in preserved-for-manual-review instead of repairing them. #4442 (SeanChengN) — the Windows history manifest integrity check treated C:\... and \\?\C:\... as different paths, so it failed after sessions were archived. Prefix normalization now lives in the shared identity leaf. Because a prefixed state-db path hashed to a different manifest filename before this fix, readers fall back to the legacy manifest name when no canonical file exists, and when both exist the canonical wins with the legacy file left in place rather than silently replaced. Credit for #4442: draft #4465 by maoxin1234, opened 2026-09-13T05:43Z, proposed the same normalization in the same two files before this lane existed. This landing is the more complete implementation but it supersedes that proposal, so the author is credited here per AGENTS.md rather than left out of the contributor graph. Evidence. Cross-platform CI run 34744702809 concluded success on d9faa02, the exact head merged here. Because platform-windows is gated on workflow_dispatch and never runs on a pull request, this lane also obtained workflow_dispatch run 34744707214 of ci.yml with lane=all on that same SHA: all six windows shards executed and passed. #4483 carries no ci check of its own; its head commit carries [skip ci] by design and the cumulative tip run covers its content. Co-authored-by: maoxin1234 <275637173+maoxin1234@users.noreply.github.com>
|
Closing as superseded, with credit rather than without it. The Win32 extended-length prefix normalization for #4442 landed on dev through #4486 (merge commit a3ca64f). You opened this draft first, on 2026-09-13T05:43Z, proposing the same fix in the same two files, so the landing carries a Co-authored-by trailer naming you in the merge commit. That is what GitHub reads for the contributor graph; a sentence in a description is not equivalent, and CREDITS.md exists in this repository because that distinction was missed 27 times. What landed is a superset rather than a different design. It adds the backup-id compatibility fallback, which matters because a prefixed state-db path hashed to a different manifest filename before the fix: readers now fall back to the legacy manifest name when no canonical file exists, and when both exist the canonical wins while the legacy file is left in place rather than silently replaced. It also covers the native-residue observation path and updates the structure document, and it was verified on Windows through a workflow_dispatch run of ci.yml with lane=all, where all six windows shards executed and passed. Your diagnosis was correct and it was first. Thanks for filing it. |
…epseek-flash-vision Lane I3 of the contributor carry train: the remaining #3775 catalog defect, and the deepseek-flash native-multimodal fix carried from #4467 by jaychou0642-create. #4467 was found by the dispatch-time ownership re-check rather than by the candidate harvest — it was opened after the harvest and before this lane, by the person who filed #4436 — so it is carried with a Co-authored-by trailer instead of being reimplemented. That check exists because lane I1 implemented #4442 fresh while contributor draft #4465 had already proposed it. The #3775 link is an implementation with no source branch. It bounds custom native-id effort lists on gateways, which is what #3804 deliberately left open pending Desktop gateway evidence; the original report is that evidence, since Desktop names gpt-6-astra in the 400 rather than the provider prefix. #4349 and the #4409 ladders do not fix this catalog projection. Cross-platform CI run 34751593123 concluded success on 9f318cb, the exact head merged here, and it covers both links because the lane is cumulative. #4499 carries no ci check of its own under the owner-authorized tip-only CI economy for this batch.
Seven of eight wave-1 lanes are on dev; lane S stays green and unmerged pending security review. Records the credit defect the wave surfaced: contributor draft lidge-jun#4465 proposed the lidge-jun#4442 fix after the candidate harvest and before the lane, so the I1 landing carries a Co-authored-by trailer for its author. Also records the two planned carries that were already satisfied on dev, both found by attempting the work rather than by reading the plan.
lidge-jun#4436 has an owning contributor pull request, lidge-jun#4467 by jaychou0642-create, opened after the candidate harvest and before this lane. Lane I3 carries it with a trailer instead of implementing the fix fresh. This is the wave-1 lidge-jun#4465 lesson applied as a dispatch-time check rather than a note.
2 participants
Summary
Fixes #4442. On Windows, file pickers and editors can pass paths with extended-length or device prefixes (such as \\?\, \\.\, or \\?\UNC\). When comparing session history entries against disk paths, mismatched prefixes caused history manifest matching to fail.
This PR:
Verification
Checklist
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.