docs(local-paths): document the supported fork-CI route for the declaration file - #4154
docs(local-paths): document the supported fork-CI route for the declaration file#4154L4XB wants to merge 1 commit into
Conversation
…ration file A fork that runs the suite in CI was caught between two guards. `validate-system-paths-coverage.mjs` needs `config/local-paths.txt` visible to that checkout or every path declared in it is reported as an orphan (career-ops-hq#2991), while `tests/user-layer-untracked.test.mjs` refuses a User Layer file that is git-ignored AND tracked — the signature of a late ignore rule hiding an already-committed personal file. Committing it fails the second; not committing it fails the first. `DATA_CONTRACT.md` and the shipped example both described the file as simply gitignored, so nothing pointed at a way out. There is one, and it needs no behaviour change: a fork appends `!config/local-paths.txt` to its own `.gitignore` and commits the file. It is then tracked and NOT ignored, which both guards accept. That is now written down where each guard sends the reader — the example file carries the recipe, `DATA_CONTRACT.md` stops claiming the file is always gitignored, and the career-ops-hq#2991 assertion in the coverage validator names it. Exempting the file inside `user-layer-untracked.test.mjs` was the alternative and is deliberately not taken: the exemption would accept exactly the ignored-and- tracked state the guard exists to catch, for a file that can hold fork-local paths, and it would do so for every fork rather than the ones that opted in. The route is pinned by a fixture rather than by prose alone: a repo that negates the rule and commits the file reports no violation, and the same commit with the negation removed is still flagged. Fixes career-ops-hq#4128
|
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 (4)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe PR documents a supported fork configuration for tracking ChangesLocal paths fork support
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The fork CI guidance and its regression coverage align with the existing validation contract; no actionable merge risk remains. 🚥 Pre-merge checks | ✅ 9✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
🚀 Post-Merge Actions
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 |
Scott-Emberson
left a comment
Scott-Emberson
left a comment
There was a problem hiding this comment.
I reviewed the owned test tests/user-layer-untracked.test.mjs. The new fixture block is sound and mutation-sensitive, and it drives real code.
The added block (the #4128 fork-CI regression fixture) builds isolated real git repos in temp dirs (git init + git add) and runs the same parseUserLayerPaths / trackedIgnoredUserLayerFiles helpers the always-on block uses against the real DATA_CONTRACT.md User Layer table, no mocks or reconstruction. It asserts the invariant with literal expecteds: the negated case pins forkViolations.length === 0 (a fork may commit config/local-paths.txt), and the control case pins stillTracked.length === 1 && stillTracked[0] === 'config/local-paths.txt' (an ignored-and-tracked user-layer file is still flagged, which is the actual privacy-leak invariant). I mutation-checked both directions: dropping --ignored reddens the negated case, forcing an empty return reddens the #4128 control, green again on revert. No process.exit(, so test-all runs it, and the fixture is hermetic.
One thing worth knowing, and it is not introduced by this PR: the pre-existing always-on real-tree block in this same file is host-git-version dependent. On my machine (git 2.55.0.windows.3) it exits 1 flagging data/offers/.gitkeep and jds/.gitkeep, because git ls-files --ignored disagrees with git check-ignore on those directory-nested .gitkeep negations. Your CI is green on all three OSes and your new fixtures use single-file negation in isolated repos, so they are not affected. Flagging it only because it lives in the file you are touching: at some point the real-tree block would be more robust querying git check-ignore per candidate rather than leaning on ls-files --ignored, so it stops depending on the local git build.
Owned test sound, not a false-pass. The substance is DATA_CONTRACT.md and the coverage script, so the routing and merge decision is the maintainer's; I am speaking to the owned test.
2 participants
Fixes #4128.
The bind
A fork that runs the suite in CI is caught between two guards:
validate-system-paths-coverage.mjsneedsconfig/local-paths.txtvisible to that checkout, or every path declared in it is an orphan there (config/local-paths.txt is gitignored, so #2793 cannot be used by any fork that runs test-all.mjs in CI #2991).tests/user-layer-untracked.test.mjsrefuses a User Layer file that is git-ignored and tracked — the signature of a late ignore rule hiding an already-committed personal file.Committing it fails the second. Not committing it fails the first.
DATA_CONTRACT.md(lines 62 and 79) andconfig/local-paths.example.txtboth described the file as simply gitignored, so nothing pointed at a way out.The route, and why it needs no behaviour change
There is a supported state that satisfies both, and it is the one the reporter found: append
!config/local-paths.txtto the fork's own.gitignoreand commit the file. It is then tracked and not ignored — so the coverage guard sees it, and the untracked guard has nothing to flag, because the condition it tests is "ignored and tracked".That is now written down at each place a reader lands: the shipped example carries the recipe (including that the negation must come after upstream's rule, since Git applies the last matching pattern),
DATA_CONTRACT.mdstops claiming the file is always gitignored, and the#2991assertion in the coverage validator names it.The alternative is deliberately not taken. Exempting the file inside
user-layer-untracked.test.mjswould accept exactly the ignored-and-tracked state that guard exists to catch, for a file that holds fork-local paths, and it would do so for every fork rather than the ones that opted in. The negation makes the fork's intent explicit in the fork's own config, where it belongs.Evidence
The route is pinned by a fixture, not by prose alone — with a control, so it cannot pass by testing nothing:
Test count goes up by the two new assertions (8724 → 8726). The single failure (
--status prerequisite/score handling wrong) is pre-existing and unrelated:mainatbb641dc, in a clean worktree with the same dependencies, reports8724 passed, 1 failed.User impact
Forks that run CI can now commit
config/local-paths.txtsafely.To pass validation:
!config/local-paths.txtat the end of the fork’s.gitignore.config/local-paths.txt.The coverage guard can then inspect declared paths. The user-layer validation also accepts the file because it is tracked and not ignored.
The regression fixture verifies both the accepted configuration and the failure when the file remains ignored.
Files changed
DATA_CONTRACT.md:62,79: Documents the tracked and un-ignored configuration.config/local-paths.example.txt:1-16: Documents the fork CI workflow.validate-system-paths-coverage.mjs: self-test comment: Explains the required.gitignorerule and commit.tests/user-layer-untracked.test.mjs: new fixture: Covers accepted and rejected configurations.No validation logic changed.
The following requested system files were not changed:
AGENTS.md,modes/,update-system.mjs,providers/, and.github/.