Skip to content

docs(conventions): own the Git Bash to Windows path-emit rule and detect its residue - #2841

Merged
kyle-sexton merged 2 commits into
mainfrom
fix/2834-msys-path-emit-convention
Aug 16, 2026
Merged

docs(conventions): own the Git Bash to Windows path-emit rule and detect its residue#2841
kyle-sexton merged 2 commits into
mainfrom
fix/2834-msys-path-emit-convention

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Closes #2834

Summary

Git Bash spells D:\dir as /d/dir. Handed to PowerShell, cmd, or a Windows-native interpreter, the leading / anchors to the root of the current drive, so the literal resolves to <current-drive>:\d\dir — silently. In #2834 a verification harness wrote its replacement fixtures to such a literal after deleting the real ones, so two named test cases ran against a directory with no fixture in it at all: mechanically identical to a third case, with two green rows recorded for scenarios never exercised. The drive-root litter was the visible symptom; the invalid test results were the cost.

This PR implements asks 2 and 3 of the issue. Ask 1 (the harness itself) is untracked scratch outside this repo and is deliberately not touched.

Fix

  • docs/conventions/windows-path-emit/README.md — the owner doc, in the repo's established convention home, registered in the docs/PLUGIN-PHILOSOPHY.md shared-concern table. Four rules in priority order: prefer a path the native side computes itself (relative — what would have prevented Windows verification harnesses: an MSYS /d/ path literal silently redirected two tzdata sabotage cases into a duplicate of case E #2834 outright); convert at the boundary, not at the source; convert with cygpath, preferring mixed form; fail loud when conversion is unavailable. It also records why neither existing lib/hook-utils.sh path helper is a drop-in: hook::normalize_path is comparison-only and uses no cygpath, hook::expand_8dot3 is 8.3-specific, and both fail open — correct for a comparison, wrong for an emit.
  • scripts/emit-windows-path.sh — the emit-safe helper the repo did not have. cygpath -m (mixed) by default, because backslashes are one escape rule away from becoming something else in every layer a path crosses (C:\temp\new carries a newline in a Python literal); -w on request; exit 2 rather than emitting the unconverted literal when cygpath is missing or fails; pass-through on a POSIX host so callers stay cross-platform. It is not in lib/: lib/ means "canonical source of a byte-identical copy synced into carrying plugins", and adding this to hook-utils.sh would push every carrying plugin through a version bump for a function none of them calls.
  • scripts/check-drive-root-litter.sh — the detection net. Fails on a directory at a drive root whose name is a single letter that is itself a mounted drive on that host. Requiring the letter to name a real drive is the precision constraint: a one-character folder at a drive root is unremarkable on its own (<drive>:\a is the workspace root on a GitHub-hosted Windows runner) and only becomes this defect's signature when the letter is one an author could have spelled into an MSYS path. A candidate containing the cwd is excluded, so a checkout that genuinely lives under one is not called residue. Reported no-op on non-Windows.

Test plan

bash scripts/emit-windows-path.test.sh          # 19 assertions
bash scripts/check-drive-root-litter.test.sh    # 14 assertions
scripts/check-drive-root-litter.sh              # live scan: 0 clean, 1 litter, 2 usage

shellcheck -x scripts/emit-windows-path.sh scripts/emit-windows-path.test.sh \
               scripts/check-drive-root-litter.sh scripts/check-drive-root-litter.test.sh
scripts/check-shell-portability.sh --paths <the four new scripts>
actionlint .github/workflows/ci.yml
npx markdownlint-cli2 --config .markdownlint-cli2.jsonc docs/conventions/windows-path-emit/README.md docs/PLUGIN-PHILOSOPHY.md
scripts/affected-tests.sh --base origin/main

Both suites drive the SUT's Windows branch by exporting a Windows OSTYPE into the child shell and, for the detector, pointing DRIVE_ROOT_LITTER_MOUNT_ROOT at fixture trees — so a Linux runner exercises the detection logic rather than skipping it.

Verification

The detector fires and does not false-positive — constructed on a real Windows host, not reasoned about:

$ bash scripts/check-drive-root-litter.sh
check-drive-root-litter.sh: 2 drive root(s) scanned under '/'; no drive-root litter found.   # exit 0

$ mkdir /d/c && bash scripts/check-drive-root-litter.sh
check-drive-root-litter.sh: drive-root litter found (1):
  /d/c    (D:\c\)
...                                                                                           # exit 1

$ rmdir /d/c && bash scripts/check-drive-root-litter.sh
check-drive-root-litter.sh: 2 drive root(s) scanned under '/'; no drive-root litter found.   # exit 0

An independent fresh-context verifier reproduced this and probed further on the same host: the literal D:\d shape of #2834 fires; an uppercase /d/C fires (no case-sensitivity false negative); and none of a multi-character /d/cc, a single letter naming no mounted drive /d/q, or a file named /d/c fires. Every fixture was removed and the host confirmed clean afterwards.

Coverage: fires on the fingerprint on every drive root; does not fire on single letters that name no mounted drive, on multi-character names, or on a candidate containing the cwd; the fixture seam cannot bypass the non-Windows gate; missing cygpath exits 2 and never prints the unconverted path.

CI wiring — required vs advisory, stated explicitly. ci-status is the required aggregate and it fails on any non-success result among the lanes in its needs list; both new jobs were added to that list, so both are required. (Being defined in ci.yml does not by itself make a job required — two existing jobs are absent from that list, filed as #2856.) What is required here is the detectors' own unit contract: windows-path-emit-gate (Linux) runs both self-tests plus a bare invocation of the drive-root scan, which asserts the non-Windows no-op; windows-path-emit-windows re-runs both suites on windows-2025, because cygpath's answer cannot be faked on Linux and a suite that reports NOT EXERCISED is exactly how an inert Windows-only surface shipped green in #2774. Confirmed in the first run: the Linux job logged NOT EXERCISED - cygpath conversion cases, the Windows job logged the real conversions (C:/emit-fixture/out.zip, C:\emit-fixture\out.zip).

What is not wired is the live drive-root scan on a runner — that would put an unquantified false-positive tail on the required aggregate for every merge in the repo, which ADR 0003 rules out until a guard has measured precision. The live scan is an operator/harness-author command; promote it when there is precision to point at.

Changelog parity: the diff is confined to docs/, scripts/, and .github/workflows/. No plugin is touched, so no plugin version bump and no plugin CHANGELOG.md entry is required, and no docs/conventions/*/CHANGELOG.md is added (eight existing convention directories carry none).

Related

…ect its residue

An MSYS path literal (`/d/...`) handed to PowerShell, cmd, or a Windows-native
interpreter re-anchors to the CURRENT drive's root, so the consumer silently
creates a phantom `<drive>:\<letter>\` tree and writes there. The residue is the
cheap symptom; the real cost is that the run measured something other than what
it claims -- in #2834 two named harness cases degenerated into duplicates of a
third and recorded green rows for scenarios never exercised.

Adds the owner doc, the emit-safe helper the repo did not have, and the
detection net for the on-disk fingerprint:

* docs/conventions/windows-path-emit/README.md -- four rules, prefer-relative
  first and conversion second, plus why neither hook-utils path helper is safe
  to emit with (both fail open, which is right for a comparison and wrong for an
  emit) and why the helper does not live in lib/.
* scripts/emit-windows-path.sh -- cygpath -m by default (mixed form survives
  every escaping layer a path crosses), -w on request, fail-loud when cygpath is
  absent, pass-through on a POSIX host.
* scripts/check-drive-root-litter.sh -- fails on a directory at a drive root
  whose name is a single letter that is itself a mounted drive, ignoring a
  candidate that contains the cwd. Reported no-op on non-Windows.

CI requires the detectors' unit contract (both self-tests plus the non-Windows
no-op assertion) on Linux and re-runs both suites on a Windows runner so the NT
branch is never green-by-absence. The live drive-root scan is deliberately NOT
wired into the required aggregate -- ADR 0003's measured-precision rule.

Closes #2834

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 35s —— View job


Claude finished the code review

  • Gather context (PR description, changed files)
  • Read repo CLAUDE.md and conventions (REVIEW.md)
  • Invoke /review:code-review procedure
  • Read changed files in detail
  • Post findings as inline comments (none needed)
  • Post summary comment

Summary

Reviewed all 7 changed files against the code-review lane's criteria (REVIEW.md, docs/conventions/). This repo carries claude-security-review.yml, so security findings are scoped to that lane and out of this review.

No high-signal findings. This PR is unusually well-scoped and well-verified:

  • scripts/check-drive-root-litter.sh — traced the drive-scan and cwd-exclusion logic (the double loop over mounted-drive letters, the pwd -P physical-path comparison for the checkout exclusion, the host gate ordering) against its test suite. The logic is correct and the precision constraint (letter must name a mounted drive) is implemented as described.
  • scripts/emit-windows-path.sh — the mixed-form default, -w backslash option, fail-loud-on-missing-cygpath behavior, and POSIX pass-through all check out. Per-argument failure handling (continue converting remaining args, exit 2 overall) is reasonable and doesn't leak an unconverted path to stdout.
  • Both .test.sh suites exercise the Windows branch on Linux via an inherited OSTYPE + fixture mount roots, and the "cygpath conversion can't be faked" cases are correctly gated to run only on real Windows/Git Bash with a visible NOT EXERCISED fallback elsewhere — directly addressing the disk-hygiene: read-only Bash allowlist is inert on Windows (0 commands accepted); engine-gate hard-allow leak on Linux #2774 failure mode cited in the PR body.
  • docs/conventions/windows-path-emit/README.md — cross-checked its claims about lib/hook-utils.sh's hook::normalize_path (comparison-only, no cygpath, fails open) and hook::expand_8dot3 (8.3-specific, fails open) directly against lib/hook-utils.sh:281-334; both are accurate. All doc cross-references (docs/adr/0003-..., scripts/cross-plugin-source-registry.txt, plugins/guardrails/hooks/block-windows-drive-tmp.sh, scripts/check-shell-portability.sh) resolve to real files.
  • .github/workflows/ci.ymlwindows-path-emit-gate and windows-path-emit-windows are correctly added to ci-status's needs list; the "gate" job's bare invocation of the detector on ubuntu-24.04 asserts the non-Windows no-op (not a live drive scan), matching the PR's stated required-vs-advisory split.
  • docs/PLUGIN-PHILOSOPHY.md — the new registry row is consistent with the (non-alphabetical) existing table and links correctly.

I was unable to run shellcheck/actionlint/markdownlint-cli2 myself in this environment (tool not available to this lane), so I did not independently verify those author-claimed local gates — noting per lane instructions that this is unverified, not confirmed. That said, per the lane's high-signal bar, anything a linter/typechecker would catch is out of scope for this review regardless.

@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 13s —— View job


Security review

  • Gather context (PR diff, changed files, repo conventions)
  • Run skip-gate check
  • Review .github/workflows/ci.yml changes for Actions security issues
  • Review new shell scripts (emit-windows-path.sh, check-drive-root-litter.sh) for injection/logic issues
  • Review test scripts for unsafe patterns
  • Post findings

No security issues found.

This PR is docs/scripts/CI-only and adds no new externally-reachable input surface:

  • .github/workflows/ci.yml (+53 lines) — two new jobs (windows-path-emit-gate, windows-path-emit-windows) follow the existing pattern exactly: pinned actions/checkout SHA, persist-credentials: false, no new secrets, no permission widening, no pull_request_target/workflow_run, no untrusted github context interpolated into a run: block.
  • scripts/emit-windows-path.sh — path arguments are consistently quoted ("$@", "$p", cygpath "$form" -- "$p"), the -- separator guards against option-injection from a path that starts with -, and it fails loud (exit 2) rather than silently emitting an unconverted path when cygpath is unavailable — the correct posture for an emit path per its own documented fail-open/fail-loud distinction from lib/hook-utils.sh.
  • scripts/check-drive-root-litter.sh — read-only filesystem probing ([[ -d ... ]], pwd -P), no eval, no command execution built from scanned path contents, host-gated before any probing, and the fixture seam (DRIVE_ROOT_LITTER_MOUNT_ROOT) cannot bypass the non-Windows gate (verified in the test suite, case 2). The live drive-root scan is correctly kept advisory/non-required per ADR 0003 rather than wired into the required gate, avoiding an unquantified false-positive tail on merges.
  • Test scripts (*.test.sh) are self-contained, operate only inside their own mktemp -d trees, and clean up via trap ... EXIT.

No injection surfaces, no secret handling, no permission changes, no supply-chain changes (no new dependency/action pins). Nothing to report.

@github-actions

github-actions Bot commented Aug 16, 2026

Copy link
Copy Markdown

Last security-reviewed head: 3c71615eba64f5fc4fbffcef529bd5d31b666391. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@github-actions

github-actions Bot commented Aug 16, 2026

Copy link
Copy Markdown

Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

The header comment documented `-m` / `--mixed` and the parser accepted them,
but the printed banner advertised only `-w`, so a caller reading the banner
would conclude the default form had no explicit selector. Adds the missing
lines and pins the shape: every accepted option must appear in the banner, and
every advertised option must actually be accepted.

Refs #2834

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 33s —— View job


Security review

  • Gather context (PR description, changed files)
  • Run skip-gate check
  • Diff against last security-reviewed head
  • Post findings

Skip gate: no new security-relevant surface since the last review.

This lane already reviewed head 5eab5e7 and found no security issues (see comment above). The only change since then is commit 3c71615:

diff 5eab5e7..3c71615 --stat
 scripts/emit-windows-path.sh      | 4 +++-
 scripts/emit-windows-path.test.sh | 18 ++++++++++++++++++

scripts/emit-windows-path.sh — the usage banner text is expanded to document the pre-existing -m/--mixed option alongside -w/--backslash; no logic, argument-parsing, or code-path change (blob).

scripts/emit-windows-path.test.sh — new assertions that every accepted option is documented in the banner and that -m/--mixed are actually accepted; test-only, no production code touched, no injection surface introduced (it invokes the SUT with a fixed literal path, not attacker-controlled input).

No new externally-reachable input surface, no .github/workflows/ changes in this delta, no secret/token handling, no permission changes. Nothing to report.

@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 2s —— View job


Code review — /review:code-review

  • Gather context (PR description, changed files)
  • Invoke /review:code-review procedure
  • Read changed files in detail
  • Post findings (inline comments and/or summary)
  • Post final summary comment

Summary

Reviewed the full PR (7 changed files) against REVIEW.md/CLAUDE.md criteria for this lane. This repo carries a claude-security-review workflow, so security-shaped findings are scoped to that lane and out of this review (already covered separately for this PR).

No high-signal findings — nothing to block on. Independently re-verified (not just re-stating the PR's own claimed verification):

  • scripts/check-drive-root-litter.sh — traced the double loop over mounted-drive letters, the mount_root == "" branch that gates the friendly D:\c\ annotation (only for the real / host scan, not a fixture root), and the pwd -P physical-path comparison for the cwd-exclusion. All match the doc's stated precision constraint and the test suite's 13 assertions, including that the fixture seam (DRIVE_ROOT_LITTER_MOUNT_ROOT) cannot bypass the non-Windows host gate (test case 2).
  • scripts/emit-windows-path.sh — mixed-form default, -w option, fail-loud exit 2 on missing/failing cygpath, and POSIX pass-through all check out. The newest commit (3c71615) fixes a real (if minor) doc/behavior mismatch: the usage banner previously advertised only -w, though the header comment and parser already accepted -m/--mixed. The fix adds both to the printed banner and the new test asserts every accepted option string appears in --help output and that -m/--mixed are actually accepted — verified this logic directly against the script's argument parser, it's correct.
  • docs/conventions/windows-path-emit/README.md — cross-checked its claims about lib/hook-utils.sh's hook::normalize_path (comparison-only, no cygpath, fails open) and hook::expand_8dot3 (8.3-only, fails open) against lib/hook-utils.sh; both accurate. All cross-references (ADR 0003, block-windows-drive-tmp.sh, check-shell-portability.sh, cross-plugin-source-registry.txt) resolve to real files.
  • .github/workflows/ci.ymlwindows-path-emit-gate and windows-path-emit-windows are both correctly listed in ci-status's needs: (confirmed at lines 1321–1322), so they're required checks despite the advisory-only live scan being deliberately left out of the gate — matches the PR's stated required-vs-advisory split and ADR 0003.
  • docs/PLUGIN-PHILOSOPHY.md — new registry row is well-formed and links correctly to the new convention doc.

One environment note: this checkout is shallow and outbound git fetch/network operations were blocked in this sandbox, so the PR-vs-origin/main diff was reconstructed from the provided changed-files list plus a direct two-dot git diff rather than a fetched merge-base — this did not limit review depth, just the diffing mechanism.

Branch: fix/2834-msys-path-emit-convention

@kyle-sexton
kyle-sexton merged commit 2d20a27 into main Aug 16, 2026
48 checks passed
@kyle-sexton
kyle-sexton deleted the fix/2834-msys-path-emit-convention branch August 16, 2026 09:48
kyle-sexton added a commit that referenced this pull request Aug 16, 2026
## Summary

`ci-status` is the single check the org `ci-gate` ruleset keys on, and
its own comment calls its
`needs` list "the single source of truth for the lane list". Nothing
enforced the other direction. A
job defined in `.github/workflows/ci.yml` but absent from that list
still runs, still reports, and
still turns red in the run list — while `ci-status` reports success and
the merge proceeds. The
`hook-utils-windows` comment already states the doctrine in prose ("a
lane missing from that list is
informational no matter how loudly a comment here calls it a gate");
prose is not a gate.

The gap was computed, not read. Against `origin/main` at `2d20a277`
(i.e. after #2841 landed and
added two lanes of its own):

```console
$ bash derive-gap.sh
defined jobs (incl. ci-status): 37
ci-status.needs entries:        34
--- defined but NOT in ci-status.needs (excluding ci-status itself) ---
managed-scope-sync
state-key-sync
--- in ci-status.needs but NOT defined ---
```

(The issue's "38 defined / 34 named" was an eyeball count on a pre-#2841
tree; the derived numbers
differ, the two missing lanes do not.)

## Fix

**Both missing lanes gate — neither omission was deliberate.**

| Job | Decision | Why |
| --- | --- | --- |
| `managed-scope-sync` | **Add to `needs`** | Structurally identical to
the four sync lanes already in `needs` (`hook-utils-sync`,
`parse-concern-value-sync`, `resolve-convention-pattern-sync`,
`standards-contract-sync`): same `--check` / lib self-test /
`--check-bump` triple. Not advisory, not schedule- or event-scoped — the
`if: github.event_name == 'pull_request'` sits on the bump **step**,
never on the job, so the job runs and reports on every PR and every
push. `scripts/cross-plugin-source-registry.txt` already advertises it
as the dedicated check for `lib/managed-scope.sh`. |
| `state-key-sync` | **Add to `needs`** | Same shape, same registry
claim for `lib/state-key.sh`. |

No job in this workflow is intentionally omitted, so this PR ships
**zero** opt-out annotations.

`cross-plugin-source-drift` (already in `needs`) covers a *drifted copy*
of either cluster, so this
was never a total hole — but it does not cover the `--check-bump` half
(lib changed, carrying plugin
version not bumped), which only the dedicated lanes run.

**The guard.** `scripts/check-lane-coverage.sh --check` proves the
defined-job set and
`ci-status.needs` are equal in both directions, and is itself wired into
`ci-status.needs` as
`lane-coverage-gate` (self-covering: its own absence from `needs` would
be caught by itself). It
reports four classes:

- `UNGATED LANE` — defined, not in `needs`, not annotated. The class
#2856 filed.
- `DANGLING NEED` — a `needs` entry naming no defined job.
- `STALE OPT-OUT` — a job annotated as deliberately ungated that *is* in
`needs`.
- `BARE OPT-OUT` — `# lane-coverage-ok:` with no reason after the colon.

**Required, not advisory** — deliberately. An advisory lane-coverage
check would be a green-and-silent
surface whose entire purpose is detecting green-and-silent surfaces,
which
`docs/conventions/liveness-assertion/` names as non-conforming. The
false-positive risk that normally
argues for advisory is absent here: the gate reads one file, takes no
diff, no base ref and no
network, and every YAML shape it does not model (flow-sequence `needs:
[a, b]`, scalar `needs:`, an
aggregate with no `needs:`, an unmodelled 2-space key under `jobs:`)
exits **2 (inconclusive)** rather
than 0 — reporting coverage from a file it did not parse would be the
gate committing the very defect
it detects.

**The opt-out** is the same annotated-exemption shape `#
silent-skip-ok:` uses for
`scripts/check-silent-skips.sh` — a `# lane-coverage-ok: <reason>`
comment in the contiguous 2-space
block immediately above the job key, or trailing the key itself. The
reason lives next to the thing it
excuses, in the file a reviewer is already reading, with no separate
list to drift. It carries a stale
guard (the annotation cannot outlive what it excuses) and a bare
annotation **fails** rather than
passing as "annotated", so it can never become a silent off switch.

No job's logic, `runs-on`, triggers, or `if:` conditions changed. The
`ci.yml` diff is
**31 insertions, 0 deletions**: three `needs:` entries and one new job
block.

## Verification

**The guard fires on the real defect and stops firing once fixed** —
constructed and run, not reasoned
about. On the unfixed `ci.yml` (pre-edit, at `origin/main`):

```console
$ bash scripts/check-lane-coverage.sh --check ; echo "EXIT=$?"
UNGATED LANE: job 'managed-scope-sync' is defined in .github/workflows/ci.yml but absent from ci-status.needs, so it cannot gate a merge. ...
UNGATED LANE: job 'state-key-sync' is defined in .github/workflows/ci.yml but absent from ci-status.needs, so it cannot gate a merge. ...
check-lane-coverage: 2 coverage defect(s) in .github/workflows/ci.yml
EXIT=1
```

After the wiring, the same derivation returns an empty gap and the gate
is green:

```console
$ bash derive-gap.sh
defined jobs (incl. ci-status): 38
ci-status.needs entries:        37
--- defined but NOT in ci-status.needs (excluding ci-status itself) ---
--- in ci-status.needs but NOT defined ---

$ bash scripts/check-lane-coverage.sh --check
check-lane-coverage: .github/workflows/ci.yml — all 37 lane(s) reachable from ci-status.needs
```

## Test plan

`scripts/check-lane-coverage.test.sh` — 20 cases, synthetic workflow
fixtures built per case, all
`ALL PASS`:

- a job absent from `needs` fails **1** and names the job; every job
present passes **0**
- annotated opt-out passes **0**; trailing-comment form passes **0**
- opt-out with no reason fails **1** (`BARE OPT-OUT`)
- opt-out on a job that *is* in `needs` fails **1** (`STALE OPT-OUT`)
- an annotation separated from its key by a blank line does **not**
exempt the job
- `needs` entry naming no defined job fails **1** (`DANGLING NEED`)
- flow-sequence `needs`, scalar `needs`, absent `needs`, empty `needs`,
unmodelled 2-space key,
unknown aggregate id, missing file, bad usage, no `jobs:` mapping — all
exit **2**, never 0
- the repository's own `ci.yml` exits **0**

Fixtures are plain files under `mktemp` addressed by absolute path — no
scratch git repo, so the
`git config user.email` caller-config-clobber class (#2839) cannot recur
here.

Also run clean locally: `shellcheck` and `actionlint` on the touched
files;
`check-shell-portability.sh --paths` (both new scripts);
`check-silent-skips.sh`;
`check-discriminating-test-skips.sh`; `check-orphaned-fixtures.sh`;
`check-changelog-parity.sh
--check` and `--check-bump origin/main` (no plugin touched, so no
version bump is owed);
`check-stale-base-overlap.sh --check origin/main`; `affected-tests.sh`
(both new files map to a suite
— no unmapped-file error); `affected-tests.test.sh` (32/32) and
`check-docs-only.test.sh` (21/21),
the two suites the selector picked for the `ci.yml` change.

An independent fresh-context verifier re-derived both sets with its own
script, rebuilt the
fired/not-fired evidence from scratch, and diff-checked that no job's
logic, triggers, or `if:`
conditions changed.

## Related

- Fixes #2856
- Refs #2834, #2841#2841 is where "a new job is automatically
required" was tested and found
false; it added `windows-path-emit-gate` and `windows-path-emit-windows`
and wired both by hand.
This PR makes the hand-wiring mechanical. Branched off `origin/main`
after #2841 merged.
- Refs #532 — `docs/conventions/liveness-assertion/`, the false-green
class this defect belongs to.
- Does not touch `scripts/check-silent-revert.sh` or
`scripts/silent-revert-incidents.txt` (#2843).

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 17, 2026
…2878)

## Summary

`D:\d` was recreated for the third time on 2026-08-16 at 16:46:30 EDT,
this time by an ordinary agent lane creating a worktree. #2841 shipped
the convention, the helper and the detector for this class; none of them
is a preventive control, and the detector is structurally incapable of
firing before the damage.

The trigger turns out **not** to be the path form. The creating lane ran
`export MSYS_NO_PATHCONV=1` earlier in the same command string — to work
around MSYS mangling an unrelated `<rev>:<path>` argument — and that
export silently unconverted a `git worktree add /d/worktrees/...` seven
segments later. The same lane had already run the identical path
argument successfully. **A guard keyed on the `/[a-z]/` path shape has a
false negative on the real incident**, which is the same false-green
shape this body of work exists to remove.

This PR adds a guard keyed on the environment instead, and fixes a
shipped defect in the detector's remediation advice that the same
incident exposed.

## Fix

**1. New `PreToolUse` guard:
`plugins/guardrails/hooks/block-exported-msys-pathconv.sh`**
(`Bash|PowerShell`, Windows hosts only, default on, kill switch
`block_exported_msys_pathconv_enabled`).

Blocks `export MSYS_NO_PATHCONV` / `export MSYS2_ARG_CONV_EXCL` and the
`declare -x` / `typeset -x` spellings. Leaves the per-command prefix
form and bare assignments alone. A cheap substring pre-filter runs
before any parsing, so a command that never names either variable exits
immediately.

**2. `scripts/check-drive-root-litter.sh` remediation text.** Its footer
told the reader to "remove the phantom tree once you have confirmed it
holds nothing else." The first real hit on this machine,
`D:\d\worktrees\ccp-measure2`, is a **live registered git worktree**
(`.git` → `gitdir: D:/repos/.../.git/worktrees/ccp-measure2`, present in
`git worktree list`). Deleting it as advised strands the registry entry
and leaves `git worktree prune` as the only cleanup — on a repository
currently carrying 69 registered worktrees across live lanes. The text
now sends the reader to `git worktree remove --force` first. `git grep
worktree` over the script previously returned nothing.

**3. `docs/conventions/windows-path-emit/README.md`** gains rule 5
(never export a suppressor) and a section on the conditional
colon-argument mangling that sends authors reaching for one.

Doc surfaces wired: guard table, kill-switch table, generated options
table, `docs/CATALOG.md`, telemetry schema + example + producer
registry, `skills/setup/SKILL.md`, count-bearing prose, CHANGELOG,
version `0.28.30` → `0.29.0`.

### Why not extend `block-windows-drive-tmp.sh`

Reuse-or-replace applies, and the answer is an openly-scoped sibling
rather than a silent second way. That guard is a **path-shape and
write-target** matcher scoped to the literal component `tmp`, with
tmp-specific allowances (`/var/tmp`, `%TEMP%`). This one is an
**environment-variable** matcher with no path component at all. Neither
would fire on the other's cases. Folding two disjoint matchers and two
exclusion sets into one hook makes both harder to reason about and risks
regressing a guard that works. Both headers now carry a reciprocal
cross-reference naming the split.

### Why not `scripts/check-shell-portability.sh`

An authoring-time lint class for this idiom is genuinely complementary
and worth doing — but a parallel lane owns that dispatcher right now
(#2840). Routed as a follow-up in #2870 rather than colliding.

## Verification

**Mechanism, by execution.** No artifact was created at any real drive
root; `subst` mapped throwaway virtual drives onto scratch
subdirectories, all removed with removal verified by listing. `D:\d` was
never modified, moved or removed, and `git worktree prune` was never
run.

The exact incident command shape, PowerShell, throwaway repo on `subst`
drive `P:`:

```text
git -C P:\repo worktree add --detach /d/worktrees/ccp-reproA
```

created and registered `P:\d\worktrees\ccp-reproA`. Drive-dependence,
same argument from two drives:

```text
Set-Location P:\ ; git init /d/worktrees/probe   ->  P:\d\worktrees\probe
Set-Location Q:\ ; git init /d/worktrees/probe   ->  Q:\d\worktrees\probe
```

An independent agent, given no sight of the above reasoning, re-derived
the same result from scratch on `X:`/`Y:` and isolated the responsible
layer — the result flips with argv conversion alone, so `git.exe`
resolves `/y/...` against the current drive and Git Bash normally masks
it.

**The predicate, four ways in one measurement.** `git rev-parse
--sq-quote` prints exactly what `git.exe` received and creates nothing:

```text
bash -c 'git rev-parse --sq-quote /d/probe'                      ->  'D:/probe'   converted
bash -c 'MSYS_NO_PATHCONV=1; git rev-parse --sq-quote /d/probe'  ->  'D:/probe'   bare assignment: no effect
bash -c 'export MSYS_NO_PATHCONV=1; git rev-parse ... /d/probe'  ->  '/d/probe'   THE DEFECT
bash -c 'MSYS_NO_PATHCONV=1 git ... /d/a; git ... /d/b'          ->  '/d/a' then 'D:/b'   prefix scopes it
```

**Measured precision (ADR-0003).** Corpus: 16,919 `tool_input.command`
strings from 701 local transcript JSONL files (388 MB), split by tool,
plus 710 lines of PowerShell console history.

| matcher | corpus | n | fires | rate |
| --- | --- | --- | --- | --- |
| path shape `/[a-z]/` (**rejected**) | Bash tool | 14,234 | 6,506 |
**45.7 %** |
| path shape, narrowed to `git worktree add` (**rejected**) | Bash tool
| 137 | 111 | **81 %** |
| **exported suppressor (shipped)** | Bash tool | 14,234 | **46** |
**0.32 %** |
| exported suppressor | PowerShell tool + console history | 3,395 | 0 |
0 % |
| per-command prefix — the safe idiom, correctly **not** fired on | Bash
tool | — | 193 | — |

Numbers are not a regex approximation: **every command in the corpus was
replayed through the built hook binary**, with `OSTYPE=msys`, as a real
PreToolUse payload on stdin.

```text
MSYS-mentioning Bash cmds     total=204   blocked=46   allowed=158
control sample (no mention)   total=400   blocked=0    allowed=400
```

The 46 span four distinct lanes (`ccp-measure2`,
`ccp-silent-revert-calib`, `ccp-silent-revert-fixture`,
`ccp-verify-2843`) and two repositories, including the lane family that
produced this incident. They are **unseeded** — the reproductions run
for this work used the safe prefix form and are among the 193 correctly
left alone — so this is ADR-0003's measured-precision path, not its
seeded-defect exemption.

**Precision claim, stated exactly.** All 46 are genuine instances of the
anti-pattern: the exported form always has unbounded blast radius over
the rest of the command string, and the remedy is one keystroke. What
the measurement **cannot** claim is a per-fire count of "would have
produced litter" — that depends on which later command consumed a path,
which is not statically decidable. Two honesty notes: the corpus is one
machine's, the deployment surface but not a fleet; and it contains this
investigation, which is why no count keyed on a bare mention of the
variables is used.

**Declared coverage gaps**, in the hook header rather than hidden: a
suppressor exported by a script the command invokes; `set -a` plus a
bare assignment; an expansion-built value; and any spawner outside the
two tool surfaces (CI runners, `subprocess`), which no PreToolUse hook
can see.

## Test plan

New `plugins/guardrails/hooks/block-exported-msys-pathconv.test.sh` —
**PASS=50 FAIL=0**. Auto-discovered by `scripts/run-plugin-tests.sh`; no
`ci.yml` change, so `check-lane-coverage.sh` is unaffected. Covers: the
POSIX host gate; the real #2870 incident shape; `export`, `declare -x`,
`typeset -x`, chained-`&&`, both-variables and
leading-unrelated-assignment forms; the safe per-command-prefix and
bare-assignment forms; six mention-not-setting cases (grep, commit
prose, `gh issue create` title, `unset`, a similarly-named variable,
`echo`); an ordinary `git worktree add` with an MSYS path, asserting the
design decision not to match a path shape; the PowerShell surface;
fail-closed over-length and payload cases; and the kill switch.

Everything else run locally against the branch:

| gate | result |
| --- | --- |
| `scripts/check-drive-root-litter.test.sh` | PASS 16/16 (2 new
assertions on the remediation text) |
| `plugins/guardrails/hooks/block-windows-drive-tmp.test.sh` | PASS
93/93 (unchanged behavior after the header edit) |
| `require-jq-posture.test.sh` (auto-enrolls the new hook) | PASS 40/40
|
| `require-jq-notice-isolation.test.sh` | PASS 2/2, 10 distinct keys |
| `scripts/check-silent-skips.sh` | clean |
| `scripts/check-hook-exec-form.sh` | clean |
| `scripts/check-hook-userconfig-argv.sh` | clean |
| `shellcheck -x` (repo `.shellcheckrc`) on all changed shell | rc=0 |
| `scripts/check-shell-portability.sh origin/main` | clean, 8 files |
| `check-changelog-parity.sh` `--check` / `--check-bump` /
`--check-order` | all pass |
| `scripts/validate-plugins.sh` | pass |
| `sync-plugin-options-docs.py --check` / `generate-catalog.mjs --check`
/ `generate-cheatsheet.mjs --check` | all in sync |
| `markdownlint-cli2` on every changed markdown | 0 errors |

Both new files are mode `100755`.

## Related

- Closes #2870
- #2841 — landed the convention, helper and detector this completes
- #2834 (closed by #2841) — same family, harness-authored producer
- #2594 (closed) — produced `block-windows-drive-tmp.sh`, the sibling
guard
- #2611 (closed by #2643) — produced `worktree-add-containment-gate.sh`,
the second guard that did not fire
-
`docs/adr/0003-verification-guards-earn-default-on-by-measured-precision.md`
- melodic-software/dotfiles#486 — machine-level rule for the same
mechanism

## Review-round matcher changes (added after the five review threads)

The three review-round commits (`6f462b3b8`, `a8462fe82`, `acbb881f0`)
changed the matcher's firing envelope in both directions, and one
widening is easy to miss from the commit messages alone:

- **Narrowed** (review thread on quoted prose): without a shell word in
the command string, the export keyword must now sit at command position,
so commit messages, `echo` arguments, and grep patterns quoting `export
MSYS_NO_PATHCONV=1` are allowed.
- **Widened** (verification finding): quote normalization now strips
quotes from BOTH sides of every token, so a shell name at the very end
of a quoted string flips the matcher into loose mode. Concretely, `git
commit -m "do not export MSYS_NO_PATHCONV=1 in bash"` now blocks (the
trailing `bash"` reads as the word `bash`), while the same sentence not
ending in the shell name — `"...in bash, do not export
MSYS_NO_PATHCONV=1"` — stays allowed. This is inside the header's
declared residual false-positive class, errs fail-closed with an
instructive message, and is the accepted cost of closing a true-leak
false negative: a fully quoted shell word (`'bash'`, or `"C:\Program
Files\Git\bin\bash.exe"` behind PowerShell's `&`) previously evaded the
shell-word check entirely.
- **Widened** (verification finding, pre-existing gap closed): the
assignment side of the token walk gets the same quote normalization, so
`bash -c "MSYS_NO_PATHCONV=1 bash -c '...'"` — where the suppressor
prefix is the first word of a quoted child command string — now blocks.

All declared residuals are documented in the hook header and pinned by
tests (127 assertions, up from 66 at review time).

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows verification harnesses: an MSYS /d/ path literal silently redirected two tzdata sabotage cases into a duplicate of case E

1 participant