fix(windows): harden long compaction recovery paths with the extended spelling by lidge-jun · Pull Request #7823 · lidge-jun/opencodex

lidge-jun · 2026-10-27T13:59:50Z

Summary

ocx recover-history fails on Windows with EICACLS when its backup or atomic-repair file path is longer than MAX_PATH, for example under a ~80-character %TEMP% or profile path. This is the first symptom in #7810.

Recovery builds a timestamped backup filename and a longer atomic-repair temp filename, then runs required ACL hardening on each before writing. icacls cannot open ordinary paths past the 260-character limit. On the same 261-character file, Bun read the file successfully while icacls exited 3 with the ordinary spelling, 0 with the extended \\?\ spelling, and 3 again with the ordinary spelling. An 80-character control succeeded with both spellings. SQLite, file reads and fixture nesting are therefore not the cause.

writePrivateFile now passes toNamespacedPath(path) to the required hardening call. Hardening is still required. Exclusive creation, owner-only permissions, error propagation, fsync, digest check, atomic replacement and cleanup are unchanged. The timeout memo key stays the ordinary path.

The second half of #7810 (tests that depend on a host proxy) is tracked separately, so this PR does not close the issue.

Verification

Windows 11, Bun 1.4.4, OCX_TEST_NO_QUEUE=1:

TMP/TEMP set to an 81-character directory, bun test --isolate tests/cli/cli-help.test.ts -t recover-history: 2 pass / 1 fail (EICACLS) before, 3 pass / 0 fail after.
New regression for long recovery paths, bun test --isolate tests/codex-integration/history-ocx-compaction-recovery.test.ts -t 'long paths': 0 pass / 2 fail before; the whole file passes 5 / 0 after. The regression builds ordinary and >260-character rollout/backup paths on every host and runs against the real Windows ACL implementation.
tsc --noEmit, structure:check and privacy:scan entrypoints: exit 0. structure/transports/responses-failover.md stays within its budget at 600/600 lines.

NOT RUN: full suite and local test:changed (CI runs test changed and ci:full), Linux and macOS locally, hosted lane=all (to be dispatched on the merge head).

Checklist

Scope stays focused and avoids unrelated cleanup.
Docs or release notes were updated when needed. (Structure doc updated.)
Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (ACL boundary; independent security review before merge.)

Summary by CodeRabbit

Bug Fixes
- Improved protection for compaction history backups and recovery files on Windows when their paths are very long, helping preserve owner-only access controls.
Documentation
- Clarified how access controls apply to Windows history backup and repair-file paths, including extended-length paths.
Tests
- Expanded recovery coverage to include long paths and improved failure details when a recovery command does not complete successfully.

…ery-acl-paths

coderabbitai · 2026-10-27T13:59:58Z

No actionable comments were generated in the recent review.

Recent review info

Run configuration

Source mode: Auto
Repository: lidge-jun/opencodex
Branch: fix/windows-long-recovery-acl-paths
File: .coderabbit.yaml
Commit: b217e57
Layers: Repository YAML
Review profile: ASSERTIVE
Plan: Advanced
Run ID: b523e5e3-dadc-4b8a-b20f-62b528c88f80

Commits

Reviewing files that changed from the base of the PR and between 2b5148b and b217e57.

Files selected for processing: 4

src/codex/ocx-compaction-history.ts
structure/transports/responses-failover.md
tests/cli/cli-help.test.ts
tests/codex-integration/history-ocx-compaction-recovery.test.ts

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

Walkthrough

On Windows, history recovery now passes a namespaced path to ACL hardening. Integration coverage includes short and over-260-character paths. The recovery CLI assertion reports stderr on failure, and the documentation describes extended-length ACL paths for backup and atomic-repair files.

Changes: Windows history recovery

Layer / File(s)	Summary
ACL path handling and recovery coverage `src/codex/ocx-compaction-history.ts`, `tests/codex-integration/history-ocx-compaction-recovery.test.ts`, `tests/cli/cli-help.test.ts`, `structure/transports/responses-failover.md`	At `src/codex/ocx-compaction-history.ts:83-84`, Windows paths are converted to namespaced paths before ACL hardening. The integration test covers short and long paths (`tests/codex-integration/history-ocx-compaction-recovery.test.ts:82-92`). The CLI assertion includes stderr on failure (`tests/cli/cli-help.test.ts:378`), and the documentation describes extended-length ACL paths (`structure/transports/responses-failover.md:323`).

Priority: Normal

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

Change: Bug fix

Possibly related issues

Issue 7810 reports a Windows long-TEMP failure in recover-history. This change updates the recovery ACL path handling and adds long-path test coverage. The issue’s local-proxy failures are outside this change.

Possibly related PRs

lidge-jun/opencodex#7822: Both changes affect compaction-history recovery; this PR addresses Windows long-path ACL handling.
lidge-jun/opencodex#7773: Both changes affect Windows ACL handling for protected files; this PR passes a namespaced path to ACL hardening.

Merge Risk: Minimal · up to b217e

No actionable regression was identified in the changed recovery, test, CLI, or documentation paths.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: fixing Windows compaction recovery failures by using extended-length paths.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 3 files. (1 skipped: 1 unsupported.)

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.}

github-actions · 2026-10-27T14:03:42Z

✅ Deterministic PR hygiene checks passed.

chatgpt-codex-connector · 2026-10-27T15:00:59Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-27T15:02:04.102355Z	`b217e57`	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

lidge-jun · 2026-10-27T18:36:15Z

Maintainer dev integration (L5 lane), per MAINTAINERS.md.

Decision: squash-merge at exact head b217e5742299560cb95852b523db9d716ca642b0.

Independent security review at this exact head: PASS, 0 blocking. The extended path spelling preserves required hardening, SID resolution, path comparisons, sanitization and the ordinary timeout memo key. An in-memory ablation of the conversion brought back the two expected EICACLS failures, including the long-path regression. The reviewer's own runs: history-ocx-compaction-recovery plus windows-secret-acl 178/0, and cli-help -t recover-history 3/0.
PR CI at the exact head (ci:full): Cross-platform CI 40516999348 success, Catalog async contracts 40516999315 success, enforce-target 40526730433 success, hygiene success.
lane=all 40539356383 at the exact head: all 27 jobs succeeded, including the nine Windows shards. The aggregate roll-up job was still waiting for a runner during the repository-wide Actions backlog.
Current with dev 41bd7e45: no newer dev commit touches this PR's four files. On the locally merged tree (git merge-tree, no conflicts), tsc --noEmit and structure:check both exit 0.

This resolves the long-TEMP half of #7810. The tests that depend on a host proxy remain open there.

…opt the long-TEMP fix Adopted from upstream 86b72a6 (lidge-jun#8019) and extended across the fork: NTFS file IDs above 2^53 collide as JS numbers, which broke the fork's lstat -> open -> fstat defence for credential, OAuth, config, CA and lock files. src/lib/file-identity.ts compares dev and ino as bigints from { bigint: true } stats, rejects unsafe numbers instead of falling back, and is used by the 86 callers that compared identities; no caller moved from fstat-on-fd to a path stat. Test doubles now mimic bigint stats; a diagnostics test isolates the external Windows ACL process. Also adopted 95cfa30 (lidge-jun#7823): compaction history hardens the namespaced long path. The upstream triage rows that only carry a stat-derived reason are marked classified-only, three misclassifications are fixed, and the commit baseline stays at 88aaa75. Broad caller suites match main except for failures that already exist there. A near-term row tracks the mtime precision change for persisted migration journals. Co-authored-by: JUN <jun@lidgeai.com> Co-authored-by: JUN <243035832+lidge-jun@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SanHsien <34234698+SanHsien@users.noreply.github.com>

lidge-jun added 2 commits October 27, 2026 22:39

fix(windows): support long compaction recovery paths

d17845d

Merge remote-tracking branch 'origin/dev' into fix/windows-long-recov…

b217e57

…ery-acl-paths

lidge-jun added the ci:full Run the four full Ubuntu test shards on this pull request label Oct 27, 2026

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

lidge-jun marked this pull request as ready for review October 27, 2026 15:00

lidge-jun requested a review from Ingwannu as a code owner October 27, 2026 15:00

lidge-jun merged commit 95cfa30 into dev Oct 27, 2026
71 of 72 checks passed

lidge-jun deleted the fix/windows-long-recovery-acl-paths branch October 27, 2026 18:36

lidge-jun mentioned this pull request Oct 27, 2026

[Bug]: some CLI tests fail on Windows workstations with a long TEMP path or a running local proxy #7810

Closed

2 tasks done

Navigation Menu

Repository navigation

fix(windows): harden long compaction recovery paths with the extended spelling - #7823

fix(windows): harden long compaction recovery paths with the extended spelling#7823
lidge-jun merged 2 commits into
devfrom
fix/windows-long-recovery-acl-paths

lidge-jun commented Oct 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 27, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 27, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 27, 2026 •

edited

Loading

Uh oh!

lidge-jun commented Oct 27, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lidge-jun commented Oct 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 27, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

lidge-jun commented Oct 27, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lidge-jun commented Oct 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 27, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 27, 2026 •

edited

Loading