Skip to content

fix: compare CLI install and update artifact identity with bigint stat - #8031

Merged
lidge-jun merged 2 commits into
devfrom
fix/bigint-file-identity-cli-20261031
Oct 30, 2026
Merged

lidge-jun merged 2 commits into
devfrom
fix/bigint-file-identity-cli-20261031

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 30, 2026 •

Copy link
Copy Markdown
Owner

Summary

Continues #8019/#8023 (compare file identity with bigint stat) for the Codex CLI install and update checks. NTFS file IDs can exceed 2^53, where adjacent IDs are equal as JavaScript numbers. These checks run on Windows: the Windows updater verifies archives through verifyCodexCliArchive.

  • src/codex/cli-update-artifacts.ts: the archive's descriptor identity before and after reading, and its lexical path identity, now come from bigint stats and are compared with sameFileIdentity. Size and timestamp checks are unchanged in meaning; statTimeMs gives exact numeric milliseconds.
  • src/codex/cli-install-provenance.ts: readBoundedFile and observeHeldFileSet now take every descriptor, path and injected observation from bigint stats. Number() is used only for bounded byte sizes. No identity enters provenance records or the update job store.
  • Test doubles:
    • tests/helpers/codex-cli-content-fs.ts: the virtual fs returns BigIntStats-shaped values for bigint observations (bigint mode, size, dev and ino, truncated bigint *Ms, and exact *Ns from fractional milliseconds).
    • tests/helpers/codex-cli-apply-admission.ts: the lstatSync spy forwards the options argument instead of dropping it.

Verification

  • Three regressions inject adjacent ino values above 2^53, one each for the archive path, the provenance manifest path and the provenance backing observation. Each fails against the previous source and passes with this change.
  • tests/lib/file-identity-callers.test.ts: the tripwire covers the migrated declarations (29 pass).
  • On Windows with both the normal and an 8.3 short TEMP: install-provenance 28, update-artifacts 25, update-admission 8, windows-layout 40 and windows-apply 7 pass, along with the file-size ratchet. An independent reviewer ran these plus all codex-cli-update-windows-* files and recovery with --isolate: 312 pass / 22 skip (POSIX-only) / 0 fail.
  • bun run typecheck, bun run structure:check and bun run privacy:scan pass.
  • NOT RUN: full local suite (shared host). POSIX-only provenance cases run in this PR's Linux CI; macOS runs in the batched lane=all.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (No user-facing change.)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (Install-provenance and update-archive trust checks; reviewed by a dedicated security reviewer, who found no residual concern. What is read or trusted is unchanged; only identity comparison precision changes.)

Summary by CodeRabbit

  • Bug Fixes
    • Improved file verification for CLI installation and update artifacts, including files with identities or sizes beyond JavaScript’s numeric precision.
    • Files are rejected when their identity, size, timestamps, or path do not match expectations, helping prevent incomplete or inconsistent file observations.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 30, 2026 23:31
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 30, 2026
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between c618c77 and 3c37716.

Files selected for processing: 7
  • src/codex/cli-install-provenance.ts
  • src/codex/cli-update-artifacts.ts
  • tests/codex-integration/codex-cli-install-provenance.test.ts
  • tests/codex-integration/codex-cli-update-artifacts.test.ts
  • tests/helpers/codex-cli-apply-admission.ts
  • tests/helpers/codex-cli-content-fs.ts
  • tests/lib/file-identity-callers.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

Install provenance observations and archive verification now use bigint filesystem stats and shared identity and timestamp helpers. Regression tests cover identity values above JavaScript’s numeric precision, and test filesystem helpers support bigint stat options.

Changes: CLI file observation hardening

Layer / File(s) Summary
Install provenance observations
src/codex/cli-install-provenance.ts, tests/codex-integration/codex-cli-install-provenance.test.ts, tests/helpers/codex-cli-apply-admission.ts, tests/helpers/codex-cli-content-fs.ts
Install-content checks use bigint stats for file identity, size, and timestamps across pre-read, held-file, and post-read observations. Tests cover manifest and backing-file identity changes above JavaScript’s numeric precision. Virtual filesystem and stat spies accept bigint options.
Archive identity verification
src/codex/cli-update-artifacts.ts, tests/codex-integration/codex-cli-update-artifacts.test.ts, tests/lib/file-identity-callers.test.ts
Archive verification compares opened-file and lexical-path identity, size, and normalized timestamps using bigint stats. A regression test checks that differing bigint inode values cause artifact_file_changed. The stat-call tripwire includes the archive and install-provenance callers.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Possibly related issues

Possibly related PRs

  • lidge-jun/opencodex#8019: This PR adopts the shared file-identity helpers and extends the caller tripwire introduced there.
  • lidge-jun/opencodex#7546: This PR hardens the install-content observation and archive-verification paths introduced by that PR.
  • lidge-jun/opencodex#8023: Both PRs apply bigint stats and shared identity checks to distinct CLI file-observation callers.

Suggested labels: bug

Suggested reviewers: ingwannu, luvs01

Merge Risk: Minimal · up to 3c377

No merge-blocking issue was identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the main change: comparing CLI install and update artifact file identity using bigint stats. The raw summary confirms bigint stat use in src/codex/cli-install-provenance.ts and src/cod…
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.
Full details: Title check

Explanation

The title names the main change: comparing CLI install and update artifact file identity using bigint stats. The raw summary confirms bigint stat use in src/codex/cli-install-provenance.ts and src/codex/cli-update-artifacts.ts, plus test doubles that forward bigint options. The title is specific and concise, and it uses the 'fix:' prefix. It omits the motivating NTFS file ID precision issue, which is acceptable.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 7 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration (MAINTAINERS.md, dev maintainer-integration exception), lane L1.

  • Exact head: 3c37716 (base dev c618c77; dev unchanged since, so file-size ratchet, union and test-layout state against current dev are unchanged)
  • Exact-head CI: https://github.com/lidge-jun/opencodex/actions/runs/41144997714 (gates, test changed, ci, hygiene, enforce-target, storage policy, api usage, docker smoke, npm-global: completed/success)
  • Review: fresh code reviewer PASS; dedicated security review PASS, no residual concern
  • Post-merge: batched lane=all dispatch on the new dev head

@lidge-jun
lidge-jun merged commit 5103065 into dev Oct 30, 2026
31 checks passed
@lidge-jun
lidge-jun deleted the fix/bigint-file-identity-cli-20261031 branch October 30, 2026 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant