Skip to content

fix(spend): classify synced state paths with POSIX rules on every host - #6414

Merged
lidge-jun merged 1 commit into
devfrom
codex/rel-l5-synced-dir-windows
Oct 1, 2026
Merged

lidge-jun merged 1 commit into
devfrom
codex/rel-l5-synced-dir-windows

Conversation

@lidge-jun

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

Copy link
Copy Markdown
Owner

Summary

Fixes the dev Cross-platform CI failure in windows 5/9 (run 36891940246 on dev 8a3a7762fe). Five cases in tests/lib/synced-state-location.test.ts from #6398 received undefined where they expected icloud-drive, file-provider or icloud-desktop-documents.

The detector classifies macOS paths only and returns early unless the platform is darwin. The tests inject platform: "darwin" with POSIX fixtures, so on a Windows host the detection ran, but it built and resolved paths with the host's node:path. That produced \\ separators and drive-letter prefixes that never matched the fixture paths. The module now uses node:path's posix implementation. On macOS, the only platform where detection runs, posix is the native implementation, so runtime behavior is unchanged. On Windows and Linux the function still returns before touching any path.

A new case checks that every path the detector probes stays in POSIX form, with no backslash, even when the host is Windows.

Verification

  • I did not run local test suites or typecheck, on the project owner's explicit instruction. Hosted CI at the exact head is the evidence. The PR-lane Cross-platform CI skips the Windows test shards, so the Windows fix will be shown by the next full dev Cross-platform CI run, not by this PR's checks.
  • Static checks run locally: git diff --check, bun run scripts/privacy-scan.ts and bun run structure:check pass.

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. This is a path-module swap in an advisory, read-only detector, with no security boundary.

Summary by CodeRabbit

  • Bug Fixes
    • Improved detection of synced-state locations for macOS paths, including paths under Documents. Classification now uses consistent slash-based paths even when running on another operating system.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 1, 2026 16:39
@chatgpt-codex-connector

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

Copy link
Copy Markdown

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-01T16:41:48.730965Z 4dfc950 PR opened
ℹ️ 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.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
src/AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 202d2b88-9b20-4147-b2fb-2df941c05062

📥 Commits

Reviewing files that changed from the base of the PR and between 06cc381 and 4dfc950.

📒 Files selected for processing (2)
  • src/lib/synced-state-location.ts
  • tests/lib/synced-state-location.test.ts

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


📝 Walkthrough

Walkthrough

The synced state location module now uses POSIX path operations on every host. A test checks that a Darwin probe classifies a Documents path as icloud-desktop-documents and that paths passed to entryExists use the expected home-directory prefix without backslashes.

Changes

Synced state path handling

Layer / File(s) Summary
POSIX path behavior
src/lib/synced-state-location.ts, tests/lib/synced-state-location.test.ts
The module uses posix.join and posix.resolve. The test verifies macOS path classification and the paths passed to entryExists.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 4dfc9

No actionable production issue is established in this change. POSIX path classification remains limited to macOS, so the change is ready to merge subject to normal checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: using POSIX path rules on every host to fix synced-state path classification.
Docstring Coverage ✅ Passed 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 2…
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.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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 github-actions Bot added the bug Something isn't working label Oct 1, 2026
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration record (MAINTAINERS.md, dev-only)

  • Maintainer PR fix(spend): classify synced state paths with POSIX rules on every host #6414. It fixes the dev Cross-platform CI windows 5/9 failure from fix(spend): name the refused ledger file and condition, warn on synced state directories #6398's new tests (run 36891940246, dev 8a3a7762fe). Exact head 4dfc9503cb75f362195c412903459131b2187d48, base 06cc3815c0.
  • Root cause: src/lib/synced-state-location.ts used the host node:path, so on Windows the darwin-injected fixtures were joined and resolved with backslashes and drive letters. The fix is to use posix path ops. Runtime behavior on macOS is unchanged, and other platforms still return early. A new case asserts that the probed paths stay POSIX.
  • Hosted CI at the head: Cross-platform CI 36893662340 succeeded, as did React Doctor 36893662324 and PR hygiene 36893844786. The PR lane skips the Windows test shards, so only the next full dev Cross-platform run can prove the Windows fix. enforce-target runs 36893662370 and 36893844656 were still queued for a runner at report time.
  • Local suites: not run, on explicit owner instruction.
  • Security: no boundary involved. This is a path-module swap in an advisory, read-only detector.

@lidge-jun
lidge-jun merged commit 328ce95 into dev Oct 1, 2026
33 of 35 checks passed
@lidge-jun
lidge-jun deleted the codex/rel-l5-synced-dir-windows branch October 1, 2026 17:25
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