Repository navigation
fix(ci): resolve Windows script-spec separator/mocked-path class (pm-s5pe) - #317
Conversation
…s5pe) After pm-dita (#316) eliminated the shebang SyntaxError class, the residual windows-latest failures across five `tests/unit/scripts/**` specs were a distinct, pre-existing path-separator / mocked-path class. The gate scripts themselves are already Windows-correct (static-quality-gate normalizes every reported path via `relativeToRepo`; docs-skills-gate builds logical paths with `path.posix`); only the specs mismatched native vs POSIX separators. All fixes are test-only + CI — zero production change. Failure classes and fixes: 1. static-quality-gate (`walkFiles`, `resolveRelativeImport`, `checkOrphanSourceModules`): POSIX-keyed `node:fs` mocks never match the drive-lettered backslash candidates real `node:path` builds on Windows (e.g. `C:\repo\src\dep.ts` vs `/repo/src/dep.ts`). Added `harness.mockPosixPath()` to force `node:path` → `path.posix` in those three mocked-fs helper tests — the inverse of the existing `runtime-schema-path-win32-guard`. Registered `node:path` in the harness `builtinUnmocks` so it is leak-free. 2. docs-skills-gate (`collectMarkdownFiles`): the `readdir` mock keyed on `/docs` while the gate passes native absolute paths; normalize separators in the mock comparison. 3. bundle-cli + contracts-snapshot: `stringContaining("dist/cli.js")` / `("tests/fixtures/contracts/full.json")` assertions compared against the script's native-separator output; compare against `path.join(...)` so the expectation matches per host. 4. smoke-npx-from-pack: `execFileSync` mocks keyed on bare `npm`/`npx`, but the script's `resolveCommand` appends `.cmd` on win32, so `npm pack` returned empty and every case failed with "npm pack did not produce a tarball name". Added `baseCommand()` to strip a trailing `.cmd` in all mocks. Re-broadened the per-PR `windows-regression` job to the full `tests/unit/scripts/` directory (plus the two non-scripts guards) and locked the command in `ci-workflow-contract.spec.ts`, so a fresh Windows-only script regression now fails a required per-PR check rather than only the nightly. Verified on Linux: 4774 tests pass, coverage literal 100/100/100/100, quality:static + docs-skills + contracts:check + security:scan green, isolated temp-dir smoke (init/create/update/list/close/validate) clean. `mockPosixPath` forces POSIX on every host, so the Linux helper-test run reproduces the Windows path exactly; the windows-regression job is the final oracle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WiNZNrE3rjftCC1udbBLGt
|
@coderabbitai full review |
|
/gemini review |
|
@codex review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
✅ Action performedFull review finished. |
|
To use Codex here, create a Codex account and connect to github. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request resolves Windows-specific test failures caused by path separator mismatches and mocked-path issues. Key changes include introducing a mockPosixPath() helper in the test harness to force POSIX path semantics on node:path during mocked filesystem tests, normalizing directory separators in the docs-skills-gate spec, using path.join for native path assertions in bundle-cli and contracts-snapshot specs, and stripping the .cmd suffix from commands in smoke-npx-from-pack mocks. Additionally, the windows-regression CI job has been broadened to cover the entire tests/unit/scripts/ directory. There are no review comments, so no feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request resolves several Windows-specific test failures caused by path-separator mismatches and mocked-path issues. It introduces a mockPosixPath helper in the script harness to force POSIX path semantics in unit tests that use POSIX-keyed filesystem mocks. Additionally, it normalizes directory separators in docs-skills-gate.spec.ts, uses native path.join for path assertions in bundle-cli.spec.ts and contracts-snapshot.spec.ts, and strips the .cmd suffix from commands in smoke-npx-from-pack.spec.ts mocks. Finally, the Windows regression CI job is expanded to run all script unit tests. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…ock (pm-s5pe)
Broadening the windows-regression job to the full tests/unit/scripts/ directory
surfaced a second Windows-only class the nightly had under-reported: three
docs-skills-gate specs that the original pm-s5pe triage missed.
1. Line endings (the two real-fs + the seeded-fixture full-mode tests): with no
`.gitattributes`, a windows-latest checkout converts tracked text files to
CRLF, so the gate reads real SKILL.md files as `---\r\n` and
`extractFrontmatter`'s `startsWith("---\n")` delimiter check fails, reporting
valid skills as missing frontmatter. Added a repo-root `.gitattributes`
(`* text=auto eol=lf`) so every platform checks out text files with LF,
matching Linux/macOS. The repo is POSIX-authored with zero CRLF files today,
so there is no renormalization churn.
2. Mocked SKILL.md path (the two full-mode mock tests): the `readFile` mock keyed
on `/.agents/skills/<name>/SKILL.md` with forward slashes only, so on Windows
the gate's native backslash path missed the regex and fell back to non-skill
content. Normalize separators before matching.
Verified on Linux: docs-skills-gate + harness + ci-workflow-contract specs pass;
the production gate scripts are unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WiNZNrE3rjftCC1udbBLGt
|
Pushed follow-up commit (fcf0c12): the broadened windows-regression job exposed a second class — CRLF line endings (no .gitattributes) breaking docs-skills-gate's extractFrontmatter on Windows checkout, plus a forward-slash-only SKILL.md mock regex. Fixed via repo-root .gitattributes (LF normalization) + separator-agnostic mock. Re-requesting reviews. @coderabbitai full review |
|
/gemini review |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request addresses several Windows-specific test failures related to path separators, mocked paths, and line endings. Key changes include adding a .gitattributes file to enforce LF line endings across all platforms, introducing a mockPosixPath() helper in the script harness to force POSIX path semantics in tests with POSIX-keyed filesystem mocks, updating hardcoded path assertions to use path.join, normalizing separators in file system mocks, and stripping .cmd suffixes from command names in smoke-npx-from-pack mocks. Additionally, the windows-regression CI job has been re-broadened to cover the entire tests/unit/scripts/ directory. There are no review comments, so no further feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Closes pm-s5pe (Windows script-spec separator/mocked-path + line-endings class) now that the broadened windows-regression job is green on the windows-latest oracle. Regenerates CHANGELOG.md from closed tracker state, which also picks up the previously-undrafted pm-dita (#316) Unreleased entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WiNZNrE3rjftCC1udbBLGt
|
Final commit (d816281): closed pm-s5pe and regenerated CHANGELOG.md (also picks up the pm-dita #316 Unreleased entry that fixes the changelog:pm:check gate). The broadened windows-regression job passed green on the prior commit (run 27869550898, job 82479200219). @coderabbitai full review |
|
/gemini review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request resolves several Windows-specific test failures across various script specifications. Key changes include adding a .gitattributes file to enforce LF line endings (preventing CRLF issues on Windows), introducing a mockPosixPath() helper to the test harness to force POSIX path semantics in tests using mocked filesystems, normalizing path separators in mocks, updating path assertions to use path.join(), and stripping .cmd suffixes from mocked Windows commands. The CI workflow has also been broadened to run the full scripts test suite on Windows. There are no review comments, and I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
✅ Action performedFull review finished. |
|
Warning Review limit reached
More reviews will be available in 53 minutes and 12 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughTwo Windows CI failure classes are fixed: (1) POSIX path separator mismatches in mocked tests, addressed by a new ChangesWindows CI Compatibility Fixes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Record final-green CI, bot-review, telemetry, and Sentry verification on the closed pm-s5pe item after merging PR #317. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WiNZNrE3rjftCC1udbBLGt
Summary
Closes the residual windows-latest failures pm-dita (#316) left behind. After the shebang
SyntaxErrorclass was eliminated, the remaining ~28 failures across fivetests/unit/scripts/**specs were a distinct, pre-existing path-separator / mocked-path class (tracked in pm-s5pe).The gate scripts are already Windows-correct —
static-quality-gatenormalizes every reported path viarelativeToRepo, anddocs-skills-gatebuilds its logical relative paths withpath.posix. Only the specs mismatched native vs POSIX separators. Every change here is test-only + CI; there is zero production change.Failure classes & fixes
static-quality-gate(walkFiles,resolveRelativeImport,checkOrphanSourceModules)node:fsmocks never match the drive-lettered backslash candidates realnode:pathbuilds on Windows (C:\repo\src\dep.tsvs/repo/src/dep.ts)harness.mockPosixPath()forcesnode:path→path.posixin the three mocked-fs helper tests — the inverse of the existingruntime-schema-path-win32-guard;node:pathadded to the harnessbuiltinUnmocksdocs-skills-gate(collectMarkdownFiles)readdirmock keyed on/docswhile the gate passes native absolute pathsbundle-cli+contracts-snapshotstringContaining("dist/cli.js")/("tests/fixtures/contracts/full.json")compared against the script's native-separator outputpath.join(...)so the expectation matches per hostsmoke-npx-from-packnpm/npx, butresolveCommandappends.cmdon win32 →npm packreturns empty → "npm pack did not produce a tarball name"baseCommand()strips a trailing.cmdin all mocksCI hardening
Re-broadened the per-PR
windows-regressionjob from a hand-picked subset to the fulltests/unit/scripts/directory (plus the two non-scripts guards), and locked the command inci-workflow-contract.spec.ts. A fresh Windows-only script regression now fails a required per-PR check rather than only the nightly. (Acceptance criteria from pm-s5pe.)Verification (Linux)
4774tests pass; coverage literal 100/100/100/100quality:static,docs-skills-gate --links-only,contracts:check,security:scangreenmockPosixPath()forces POSIX on every host, so the Linux helper-test run reproduces the Windows execution path exactly; thewindows-regressionjob is the final Windows oracle.🤖 Generated with Claude Code