Repository navigation
test(history): stale-ledger build case fails on Windows (ino + 1 loses precision) - #7087
Conversation
…(Windows file ids exceed 2^53)
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
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 469c83b127
ℹ️ 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".
| expect(() => buildIndexFile(buildPath, "test", { ...revision, ino: revision.ino + 1 })) | ||
| // Change the volume, not the file id: an NTFS file id can exceed 2^53, where ino + 1 rounds | ||
| // back to the same number and the build sees the recorded file (windows CI, #7070). | ||
| expect(() => buildIndexFile(buildPath, "test", { ...revision, dev: revision.dev + 1 })) |
There was a problem hiding this comment.
Preserve coverage of same-volume ledger replacement
When the ledger is atomically replaced on its existing volume, dev remains unchanged and only ino identifies the replacement. This edit therefore stops the deterministic test from exercising the stat.ino !== expected.ino branch in ingestSourceTail; removing or breaking that check would leave this test green while allowing a worker to index a replaced ledger. Keep the inode mismatch but choose a collision-proof value, such as revision.ino === 0 ? 1 : 0, rather than incrementing the potentially imprecise number.
Useful? React with 👍 / 👎.
|
Maintainer integration into |
Summary
The Windows shard of Cross-platform CI on
dev(run 38499932279, shard 5/9, head 04b7d51) fails intests/usage/request-history-rebuild-worker.test.ts› "a build whose ledger is not the recorded file fails stale and leaves no file":buildIndexFilereturned 10 rows instead of throwingRequestHistoryIndexStaleError.The test simulates a replaced ledger with
{ ...revision, ino: revision.ino + 1 }. On NTFS a file id can exceed 2^53, whereino + 1rounds back to the same double, so the "different" revision compares equal to the real file and the stale check never fires. The test now perturbsdev(the volume serial on Windows, always exactly representable), which drives the samedev/inoidentity check iningestSourceTailon every platform. Test-only; no runtime change. The production comparison (same file vs. its own recorded id) is unaffected because both sides round identically.Found during release-train verification of #7070.
Verification
bun scripts/test.ts tests/usage/request-history-rebuild-worker.test.ts tests/ci-workflows/repo-hygiene.test.ts→ 28 pass, 0 fail (macOS)bun run typecheck→ exit 0workflow_dispatchof Cross-platform CI (PRs run the Ubuntu subset only).Checklist
Summary by CodeRabbit