[Mikola Lysenko (mikolalysenko)]

Problem

Scheduled CI on main has failed 5 nights running (11-06 → 11-10). On the latest run, 42763778797 (main @ c751724), the only failing jobs are test (windows-latest, 1) and test (windows-latest, 2). Those legs run only at night (see #1720), so PR and merge-queue CI never catch these failures. Both legs use --no-fail-fast, so this is the full set:

Test	Cause
patch::user_patches::tests::an_absolute_patch_dir_inside_the_repository_is_read	package.json built with format!, so C:\... became invalid JSON escapes. The fix below also found a real bug behind this test.
patch::user_patches::tests::the_repository_bound_holds_for_workspace_runs_only	ends_with("fixes/left-pad+1.3.0.patch") on a \-separated path
utils::cargo_build_cache::tests::a_config_above_the_repository_is_named_when_its_target_dir_is_unread	contains(".cargo/config.toml") checked against .cargo\config.toml
utils::repo_root::upward_tests::pnpm_workspace_file_above_the_repository_fails_closed	expected the canonical \\?\C:\... path, but governing_workspace_file deliberately returns it through without_verbatim_prefix
vex::discover::user_patched::tests::patch_package_patches_mark_yarn_classic_vlt_and_npm_pins	/ substring checked against patches\left-pad+1.3.0.patch
vex::discover::user_patched::tests::workspace_member_patches_agree_with_the_consent_gate	packages/a checked against packages\a\package.json
hosted_memory_engine::user_patched_npm_packages_are_never_pinned (cli)	same separator mismatch in the skip detail

Root cause

• Six of the failures are test assertions that assume Unix paths: / separators, unescaped JSON, and \\?\-prefixed expectations.
• One is a real production bug. Bound::outside (user_patches.rs) compares the path against the canonical repo root, which on Windows is \\?\C:\proj. A script's --patch-dir C:\proj\fixes is written the way users type it, and Path::starts_with treats the VerbatimDisk and Disk prefixes as different. So on Windows every absolute in-repo --patch-dir was reported as outside the repository, and every npm package was protected. The old test passed a \\?\ path, which no real package.json contains.

Fix

• Production: Bound::outside now compares both sides through pnpm_workspace::without_verbatim_prefix. This changes nothing on Unix.
• Tests:
  • The absolute --patch-dir test now spells the directory the way a user does: unprefixed, quoted, and built through serde_json so the backslashes are escaped.
  • Separator-sensitive substring checks run on the text after mapping \ to /.
  • The pnpm test expects without_verbatim_prefix(file).

No assertion is weaker.

Proof

• Linux:
  • cargo test -p socket-patch-core --lib -- patch::user_patches vex::discover::user_patched utils::repo_root utils::cargo_build_cache: 73 passed.
  • cargo test -p socket-patch-cli --test hosted_memory_engine --test user_patches: 57 passed.
• rustfmt --check on the touched files is clean.
• Clippy adds no new findings. Local 1.93.1 reports the same pre-existing sites on unmodified main.
• Windows: verified. A CI run dispatched with os=windows on this head, 42772455462, passed test (windows-latest, 1) and test (windows-latest, 2). Its only failures are the mode_migration_vlt e2e legs, which fail the same way on main's nightly on every OS and are not this PR's (see the comment below).

Review findings not fixed

/code-review high:

• Hosted engine detail text uses OS separators. In the hosted in-memory engine, the user_patch_conflict detail text uses OS separators (Path::display). Production servers run Linux, so changing how in-memory sources are rendered is a larger change and out of scope here. Worth a follow-up.
• No shared slashed helper. The .replace('\\', "/") call is repeated inline, and there is no shared helper for it. Core has no test-util module that all of these tests share. That refactor is left out of a CI fix.

Where tests run

No tests were removed or moved.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aeRDbxjatib28cpxPqQ

---

Note

Medium Risk
Changes workspace-bound patch discovery on Windows (consent gate / user-patch conflicts); incorrect comparison previously over-protected npm packages.

Overview
Fixes Windows nightly CI failures and a production bug where in-repo absolute patch-package --patch-dir paths were treated as outside the git repository.

Production: Bound::outside in user_patches.rs now strips Windows \\?\ verbatim prefixes on both sides before starts_with, so user-typed C:\repo\fixes matches canonical \\?\C:\repo. Unix behavior is unchanged.

Tests: Assertions that assumed / separators or \\?\ paths now normalize with .replace('\\', "/") or expect without_verbatim_prefix. The absolute --patch-dir test builds package.json via serde_json with a quoted, user-style path so JSON escaping is valid on Windows.

^{Reviewed by Cursor Bugbot for commit c40cda6. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit c40cda6. Configure here.}

[Mikola Lysenko (mikolalysenko)]

In the Windows dispatch run 42772455462, e2e-windows (mode_migration_vlt, 1.0.0-rc.14 | 1.2.0) fail. This failure isn't caused by this PR.

The same legs failed on main in last night's scheduled run 42763778797 (main @ c751724). They failed on all three OSes there: e2e-extended on ubuntu (3 legs), e2e-macos (1.2.0) and e2e-windows (2 legs).

The failing assertion is vlt_pinned_matrix_migration_hosted_scan_keeps_agent_patched_tree (assert_advisory). The redirect_vlt_reinstall_required detail came back as the "could not check 1 installed copies … run vlt ci" variant. The test expected the "fresh checkouts install them with vlt ci" variant. This PR's diff doesn't touch vlt redirect or install checks, and no fix exists yet.

I'm leaving it out of this PR. It's the next CI-janitor target. This PR's own target is the test (windows-latest, *) legs, which are still running.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review by the burn-down agent.

• Head: c40cda63fb7bd7a6517d726b26836d14809e1821
• CI: 95/98 checks on this head passed or were skipped. The 3 failures are main-wide: e2e-windows (mode_migration_vlt, 1.0.0-rc.14 | 1.2.0) and the ci-ok (windows only) gate that rolls them up. The same vlt legs fail on main's latest scheduled CI run 42763778797 on all three OSes. Fix nightly vlt migration e2e after #2021's heal change #2050 is fixing them.
• That main run's test (windows-latest, 1|2) failures are the ones this PR fixes, and they pass here.
• Bugbot: reviewed this head, no findings; no open review threads.
• Mergeable, no conflicts, doesn't touch CHANGELOG.md.
• For reviewers: the one production change is Bound::outside in crates/socket-patch-core/src/patch/user_patches.rs. It strips the Windows \\?\ prefix on both sides before comparing paths. The other changes are test-only.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. Fixes the 7 tests that fail only on the nightly Windows test legs (#1720). Six were test assertions that assumed / separators or \\?\ paths. The seventh exposed a real Windows bug: Bound::outside compared a user-typed C:\proj\fixes --patch-dir against the canonical \\?\C:\proj repo root, so every absolute in-repo patch dir counted as outside the repository. Both sides are now compared without the verbatim prefix.

Risk: low. One production line, a no-op on Unix. On Windows it makes an in-repo absolute --patch-dir count as inside the repository; .. is resolved lexically before the prefix is stripped, so nothing outside the repo becomes readable. Everything else is test-only, and no assertion is weaker.

Look here

• user_patches.rs:1235-1240: the production fix.
• user_patches.rs:2119-2134: the test now writes the path the way a user does (unprefixed, quoted, JSON-escaped). The old test passed a \\?\ path, which no real package.json contains, so it hid the bug.
• repo_root_upward_tests.rs:139: expects the unprefixed path that governing_workspace_file deliberately returns.
• The other 4 test files only map \ to / before substring checks.

Verified

• Read the full diff (5 files, +35/−18). CHANGELOG.md is untouched. No debug leftovers, nothing out of scope.
• Checked without_verbatim_prefix (\\?\C: and \\?\UNC\ cases) and the containment logic for traversal.
• Ran cargo test -p socket-patch-core --lib -- patch::user_patches locally: 35 passed.
• Required checks ci-ok and clippy are green on this head. The Windows dispatch run 42772455462 passes test (windows-latest, 1|2).
• Its only red checks are e2e-windows (mode_migration_vlt, …) and the non-required ci-ok (windows only) rollup. Those fail on main's nightly on every OS too, and Fix nightly vlt migration e2e after #2021's heal change #2050 fixes them.
• Bugbot reviewed this head with no findings. No open review threads.

Changes I made. None.

Open questions. None blocking. The description lists two follow-ups it chose not to do: OS separators in the hosted engine's detail text, and a shared slashed test helper.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code