Skip to content

ci(gate): pin the merge gate, reject unvouched dependency names, correct the provenance claims - #54

Merged
wshallwshall merged 7 commits into
mainfrom
claude/artifact-review-standards-ac2b2a
Jul 29, 2026
Merged

ci(gate): pin the merge gate, reject unvouched dependency names, correct the provenance claims#54
wshallwshall merged 7 commits into
mainfrom
claude/artifact-review-standards-ac2b2a

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

Three commits from an audit of this repo's controls against an external "reviewing AI-generated code" guide. The guide's own advice mostly assumes a team with human reviewers; what survived translation to a solo, auto-merge, AI-authored repo is below.

1. f534a4fc — make the merge gate knowable, and not neuterable in one line

security.yml:7 documented the one-line downgrade verbatim: "To temporarily downgrade one, add continue-on-error: true back to its job." A job-level continue-on-error makes the job report SUCCESS, so the context branch protection requires stays green while the scanner's findings are discarded — and with required_approving_review_count: 0 and auto-merge armed, the PR merges unread. Nothing guarded that.

And "is this check blocking?" could not be answered from a clone. Five in-repo statements disagreed with each other and with the server: docs/CI.md listed 8, manifest-lint.yml said 4, docs/design/freethread.md said 7, tests/test_push_guard.py said 12. The live API says 12. docs/CI.md also called zizmor "Blocking" (it isn't), said Scorecard runs on PRs (no pull_request trigger), and named the CLA context CLA Assistant when the real string is cla.

  • .github/required-contexts.txt — the checked-in claim, with the required-but-absent trap documented for anyone adding one
  • tests/test_required_contexts.py — reconciles every prose statement against that file
  • tests/test_security_posture.py — refuses continue-on-error, || true/--exit-zero, and skippable if: on any job backing a required context; pins the advisory jobs the other way so an accidental promotion is equally deliberate
  • actionlint — pre-commit hook + a checksum-verified step in zizmor.yml, for the invalid-${{ }} class that aborts workflow compilation so required contexts silently never appear (zizmor does not catch it; docs/CI.md asked for actionlint and nothing ran it)

2. 7b224fa9 — reject a dependency name that PyPI cannot vouch for

pip-audit asks "does this pinned version have a known CVE". A freshly registered hallucinated name has no advisory, so it resolves through uv lock, hashes into requirements.lock and installs under --require-hashes — every DEP-1 control passes it. The only thing in the way was that uv lock can't resolve a name PyPI doesn't serve: a resolver side effect, not a control, and exactly what slopsquatting defeats. docs/Secure_AI_Development_Standards.md has carried this as the "highest-priority deferred gate"; its own specified interim control (a PR-template line) had never been added either.

scripts/security/new_dependency_check.py sweeps every distribution in pyproject.toml — core and every extra — and fails on a name that doesn't exist, publishes no files, was first released inside a 90-day floor, or is served under another project's canonical name. Age comes from the earliest release across all versions, so a squatter who registers then publishes later doesn't look established.

Runs as a step in the already-required pip-audit job, so it blocks today with no branch-protection change (the ledger-gate backstop pattern). Stdlib-only. Fails closed: unreachable PyPI exits 2; examining zero distributions exits 2, so a schema change can't read as a clean sweep.

Verified against live PyPI: 41 distributions clean, injected hl7-dicom-bridge>=1.0 rejected.

Documented blind spot, pinned by a named test rather than glossed: it cannot tell a real package is the wrong one. Measured — py-webauthn exists, publishes, is years old and is canonically named, yet is AS207960's project rather than the intended webauthn. That class stays on human verify-before-add plus the dated vet note.

3. cb9e4597 — stop claiming a trailer we don't write; pin the gates' own toolchain

Provenance. The standard prescribed a Co-Authored-By + Tier: trailer, listed it under Built (in code today), put it in §11's retained-evidence set, and cited it for two A.4 claims aimed at "adopters and auditors". Measured: 0 trailers and 0 Tier: lines in 300 commits, while 81 tracked files instruct omitting it — because cla.yml allowlists three identities, a trailer co-author reads as an unsigned contributor, and cla is required. The prescription and the prohibition were both in the tree, neither referencing the other.

Fixed by making the documents honest, not by building a hook. §6.7 now states it is not in use and why; it's out of the Built list and the evidence set; the claim "Provenance recorded at commit granularity by convention" is WITHDRAWN with a replacement wording; the A.6 deviation is corrected to the broader truth — no per-commit model/version record at any granularity. tests/test_ai_provenance_claims.py stops it reverting, asserting consistency rather than counting git log (CI checks out shallow; "0 of 0 commits" is a gate measuring nothing).

Toolchain. The pip-audit job — whose purpose is proving nothing unpinned enters the tree — installed its auditor with a bare pip install pip-audit. DEP-1 bootstrapped uv, the resolver producing every lockfile it then audits, unpinned. semgrep rode ~=1.90. gitleaks was curl | tar with no integrity check, while the sbomqs step in the same file already verified a checksum. Now pip-audit==2.10.1, uv==0.12.0, semgrep==1.172.0 (each is what the prior spec already resolved to, so no behaviour change today) and gitleaks verified with sha256sum. tests/test_ci_venv_pinning.py grows two guards.

Verification

154 tests pass across every suite touched; actionlint clean; the dependency gate green against live PyPI.

Nothing was accepted for being green — every guard was made to fail first:

  • 7/7 mutations into security.yml caught by the intended assertion, files verified restored byte-identical
  • reverting the doc fixes reproduced 5/8 required-context failures and 4/6 provenance failures
  • actionlint catches an invalid ${{ }} inside a comment — the exact class docs/CI.md describes

Four bugs that pass found in this work: an invariant asserting 12 contexts == 12 jobs (they're 10 — the test legs share a matrix job); find("**Built") slicing a section 400 lines off target so a guard checked nothing; a detector flagging -r requirements.lock, the most pinned install in the file; and a detector matching | tar inside its own comment explaining the pipe it replaced.

Owner action still needed

Branch protection is server-side and unchanged by this PR. required-contexts.txt deliberately mirrors the live server, so backlog-hygiene is recorded as pending promotion rather than added — adding it first would make the file lie in the one direction it exists to prevent.

Branch protection lives on the server, so "is this check blocking?" could not be
answered from a clone -- and five in-repo statements answered it differently.
docs/CI.md said 8 contexts (naming the CLA one "CLA Assistant", which matches no
status check at all), manifest-lint.yml said 4, freethread.md said 7, cla.yml told
a reader to require that non-matching string, and test_push_guard.py said 12. The
live API says 12. With required_approving_review_count 0 and auto-merge armed,
required-set membership is the only thing separating "reviewed" from "merged
unread" -- so a session reasoning from docs/CI.md would conclude that gitleaks,
semgrep, npm-audit and crypto-inventory were advisory, i.e. that four blocking
security gates were safe to weaken.

.github/required-contexts.txt is now the checked-in claim, and
tests/test_required_contexts.py asserts every prose statement agrees with it --
plus a reality check that each context resolves to a real job name (expanding
matrix templates). That last part matters: a doc-drift test comparing two wrong
things passes happily, and a required context that nothing reports blocks every
PR forever.

tests/test_security_posture.py closes the second hole. security.yml's own header
documents the downgrade -- "add `continue-on-error: true` back to its job" --
which is exactly the edit that makes a REQUIRED context report SUCCESS while
scanning nothing. Nothing guarded it: the three tests that read a workflow's
continue-on-error cover quality-advisory.yml and freethread-smoke.yml, and
test_lint_scope_parity.py opens security.yml only to compare scan scope. The new
suite refuses continue-on-error (job and step), `|| true` / `--exit-zero`, and a
skippable job-level `if:` for every job backing a required context; it pins the
advisory jobs the other way so an accidental promotion is equally deliberate; and
test_every_security_job_is_classified fails on any new security.yml job, so the
module cannot quietly stop covering the file it is named for.

Verified by mutation rather than by being green. Seven injected regressions --
job- and step-level continue-on-error, `|| true` on the bandit scan, a
schedule-only `if:` on the leak guard, trivy silently promoted, a new
unclassified scanner, a dropped required context -- are each caught by the
intended assertion; reverting the doc fixes reproduces 5 of the 8 drift failures.
Both mutated files were confirmed restored byte-identical. One assertion caught a
modelling error of my own on the way: 12 contexts resolve to 10 distinct jobs,
because the three `test` legs share one matrix job.

Also wires actionlint, which docs/CI.md has prescribed for months while nothing
ran it. An invalid `${{ }}` anywhere in a `run:` body -- comments included --
aborts workflow compilation, so no jobs are created and required contexts simply
never appear. The PR does not go red, it goes stuck, and the tempting remedy for
a stuck PR is relaxing branch protection. Now a version-pinned pre-commit hook
(the load-bearing half; zizmor.yml is deliberately not a required check) plus a
checksum-verified step in zizmor.yml, which is already paths-filtered to
.github/**. Its first run produced one real finding -- a custom self-hosted
runner label -- declared in .github/actionlint.yaml rather than by disabling the
runner-label rule, which would have traded one finding for the whole class.

backlog-hygiene is recorded as PENDING PROMOTION rather than added: this file
mirrors the live server, so adding it before branch protection changes would make
it lie in the one direction it exists to prevent. That promotion is the owner's
call.
pip-audit answers "does this pinned version have a known CVE". It cannot answer
"is this package the one I meant". A freshly registered hallucinated name has no
advisory, so it resolves through `uv lock`, hashes into requirements.lock and
installs under --require-hashes -- every DEP-1 control passes it clean. The only
thing in its way was that `uv lock` cannot resolve a name PyPI does not serve,
which is a resolver side effect rather than a control, and is exactly the case
slopsquatting is built to defeat.

That matters here specifically because this project's dependencies are chosen by
an AI. Spracklen et al. (USENIX Security 2025) measured 19.7% hallucinated
package references, 58% of invented names recurring across runs -- predictable,
therefore registrable. docs/Secure_AI_Development_Standards.md has carried this as
the "highest-priority deferred gate" while recording that verify-before-add is
"enforced only by the human remembering", and its own specified interim control --
a verify-before-add line in the PR template -- had never been added either.

scripts/security/new_dependency_check.py sweeps every distribution declared in
pyproject.toml, core AND every extra (an extra is where a niche,
plausible-sounding name lands), and fails on a name that does not exist, publishes
no files, was first released inside a 90-day floor, or is served under another
project's canonical name. Age is taken from the EARLIEST release across all
versions, not the current one: otherwise a squatter who registers a name and
publishes later looks established, and a decade-old project looks fresh the day it
ships.

It runs as a step inside the already-REQUIRED pip-audit job rather than as a new
context, so it blocks today with no branch-protection change -- the same reasoning
as the ledger-gate backstop in ci.yml, and the lesson from this branch's first
commit: a hard-failing job that is not a required context does not stop auto-merge,
it only looks like it does.

Fail-closed throughout. Unreachable PyPI exits 2 rather than passing, because a
supply-chain gate that goes green while blind is worse than no gate. Examining
zero distributions also exits 2, so a pyproject schema change cannot read as a
clean sweep. The transport contract is explicit -- only a 404 is "absent",
everything else raises -- because mapping a timeout onto None would turn a PyPI
outage into a wall of false "hallucinated dependency" failures.

Verified rather than assumed: 41 distributions clean against live PyPI, and an
injected `hl7-dicom-bridge>=1.0` rejected end-to-end. 29 offline tests drive the
shipped functions through the fetch seam, never a local copy of the rule.

Its blind spot is documented rather than glossed. The gate cannot tell that a real
package is the WRONG one. The counter-example is in this tree: pyproject warns
that py-webauthn is AS207960's project while the intended name is exactly
webauthn -- and measured against live PyPI today, py-webauthn exists, publishes,
is years old and is canonically named, so every check passes it. That class stays
on human verify-before-add plus the dated vet note, the module docstring says so,
and a named test pins it -- following tests/test_gate_liveness.py, which likewise
documents its own blindness instead of hiding it. The standard's deferred-gate
entry now records what is built and what remains (no check yet REQUIRES the vet
note on a new dependency; that needs a merge-base diff that fails loudly rather
than silently finding no base).
…; pin the gates' own toolchain

TWO UNRELATED-LOOKING FIXES, ONE THEME: a control is only worth what it actually
does, and both of these were being scored on their description.

1. THE PROVENANCE TRAILER IS NOT IN USE, AND CANNOT BE.

docs/Secure_AI_Development_Standards.md prescribed a `Co-Authored-By` + `Tier:`
commit trailer, listed it under "Built (in code today)", put it in the section-11
retained auditable evidence set, and cited it as evidence for two claims in the
A.4 register -- a register whose stated audience is "adopters and auditors".

Measured: ZERO. `git log -n 300` contains 0 `Co-Authored-By` trailers and 0
`Tier:` lines, while 81 tracked files under docs/ instruct omitting it. And the
cause is structural, not slippage: cla.yml allowlists exactly three identities,
the CLA bot reads a trailer co-author as a contributor who must sign, and `cla` is
a REQUIRED status check -- so adding the trailer reds the merge gate. The
prescription and the prohibition were both in the tree, neither referencing the
other, so the contradiction was invisible from either side.

Fixed by making the documents honest rather than by building a hook. A commit-msg
trailer hook would reverse a standing project decision the tree already reflects,
and would have a solo author satisfying a ceremony on themselves. So section 6.7
now states the trailer is not in use AND why; it is out of the Built list, out of
the evidence set, and the "Provenance recorded at commit granularity by
convention" claim is WITHDRAWN with a defensible replacement wording. The A.6
deviation is corrected to the broader truth it always was: there is no per-commit
model/version record at any granularity, so the project cannot answer "which model
wrote this, under what policy" for any commit in its history.

The note that the trailer is not CI-enforced was already there and was fine. What
was wrong was calling it Built and citing it as evidence.

tests/test_ai_provenance_claims.py stops it reverting. It asserts consistency
rather than counting trailers in `git log`, deliberately: CI checks out shallow, so
a commit-count guard would measure a different history than a developer sees, and
"0 of 0 commits" is a gate measuring nothing. Either resolution of the
contradiction passes -- adopt the trailer and clear the omission instructions, or
keep omitting it and do not claim it.

Proved by reverting the doc: 4 of its 6 assertions catch the pre-fix text. The two
that do not read other files and correctly should not. Building it also caught two
bugs in itself: `find("**Built")` was matching an inline mention 400 lines above
the real list (so the guard checked the wrong section entirely), and the
why-is-it-blocked assertion was loose enough to pass on the broken doc.

2. THE BLOCKING GATES INSTALLED THEIR OWN TOOLING UNPINNED.

The pip-audit job -- whose entire purpose is proving nothing unpinned enters the
tree -- installed its auditor with a bare `pip install pip-audit`. The DEP-1 step
bootstrapped `uv`, the resolver that produces every lockfile the job then audits,
unpinned. semgrep rode `~=1.90`, so any new 1.x could change taint propagation and
red a green PR for reasons unrelated to its diff. gitleaks was `curl | tar` with no
integrity check -- while the sbomqs step in the SAME FILE already verified against
the release's own checksums file. Unfinished scope, not an accepted risk. All four
sit in REQUIRED contexts, and all four are dependency intake no audited lockfile
covers.

Now: pip-audit==2.10.1, uv==0.12.0, semgrep==1.172.0 (each is what the previous
spec already resolved to today, so no behaviour change), and gitleaks verified
with sha256sum against gitleaks_8.18.4_checksums.txt, mirroring the sbomqs shape
exactly rather than inventing a new one.

tests/test_ci_venv_pinning.py grows two guards so it cannot regress. Writing them
surfaced the trap they were meant to catch, in themselves: the first flagged
`-r requirements.lock` (the most rigorously pinned install in the workflow) and the
second matched the words `| tar` inside my own comment EXPLAINING the pipe it had
replaced -- a detector counting itself. Both fixed, and the comment-stripping is
now the documented reason rather than an incidental.
main's #48 pinned the RELEASE path (sigstore, build, cyclonedx-bom, packaging,
pip) and built a blanket-scan + registry to keep it that way. This branch pinned
the BLOCKING SCANNERS in security.yml (pip-audit, uv, semgrep) and checksum-verified
the gitleaks download. Same subject, opposite halves, so they conflicted in
tests/test_ci_venv_pinning.py.

Resolved toward main's machinery, which is the better base: its _PIP_INSTALL scan
catches a NEW unpinned install anywhere on the release path, which a fixed job list
cannot. My test_blocking_jobs_pin_the_tools_they_install is DROPPED rather than
merged -- it duplicated that scan for three named jobs and would have been a second
gate on the same property.

Two substantive corrections to main's side, both load-bearing:

1. SECURITY_YML_ACCEPTED_UNPINNED registered `uv` and `pip-audit` as accepted
   unpinned, reasoning that security.yml's jobs are "schedule/dispatch-only" and
   "produce nothing anyone installs". The first half is factually wrong: that file
   triggers on `pull_request` and SEVEN of its jobs are required contexts,
   pip-audit among them. So those were unpinned dependency intake on every PR,
   inside the gate whose stated purpose is proving nothing unpinned enters the
   tree -- and `uv` is the resolver producing every lockfile that job then audits,
   so an implicit upgrade can change the exported set and red the
   `git diff --exit-code` for a reason unrelated to the change. Both are now pinned
   and out of the registry, which now holds only the `pip` bootstraps. The
   docstring records the correction rather than quietly rewriting the rationale.

2. RELEASE_PINNED_TOOLS gains the four security.yml scanners, so a pin that
   vanishes WITH its step is caught -- the direction the blanket scan structurally
   cannot see.

test_release_asset_downloads_in_blocking_jobs_are_checksum_verified is kept: it is
the non-pip half of the same intake and main has no equivalent. It excludes trivy
(continue-on-error, so it cannot turn a required context green) and strips comments
before matching, because the gitleaks step's rationale comment QUOTES the
`curl | tar` it replaced -- a detector that would otherwise count itself.

security.yml auto-merged clean: main touched only the SBOM step, this branch only
the pip-audit/semgrep/gitleaks steps.

Verified after resolution: 154 tests pass across every affected suite (including
main's byte-identical-SBOM twin check and the release-pipeline structure pins),
actionlint clean.
…cally; make the shared pre-commit shim worktree-aware

1. ACTIONLINT WAS TWO DIFFERENT LINTERS.

The actionlint step added earlier in this branch passed locally and went RED in CI
on ~30 findings in ci.yml and release.yml that predate it. Cause: actionlint shells
out to shellcheck for every `run:` body IF shellcheck is on PATH. GitHub runners
ship shellcheck; a local actionlint binary does not. So the same command was a
workflow-syntax linter on my machine and workflow-syntax + shellcheck in CI --
precisely the environment-dependent reach this repo keeps getting bitten by, and my
local green was not evidence of anything.

Both sides now pass `-shellcheck=`. The step's purpose is the compile-aborting
expression-syntax class docs/CI.md describes; adopting shellcheck over every `run:`
body is a real question but a separate PR that also clears or annotates ~30
findings, several of which are shellcheck misparsing a GitHub interpolation inside a
shell script (release.yml:392 "'(' is invalid here"). It is not something to smuggle
in as a side effect of which tools a runner happens to have installed.

2. THE GATE THEN CAUGHT ME, WITH THE EXACT DEFECT IT EXISTS FOR.

Writing the comment explaining that class, I put a literal empty interpolation in it
-- inside the `run:` body. actionlint parses expressions in run bodies INCLUDING
shell comments, which is the whole reason this step exists, so the illustration WAS
the defect: `zizmor.yml:52: unexpected end of input while parsing variable access`.
The comment now names the hazard in words and warns the next reader not to
illustrate it literally.

Worth recording precisely, because the distinction is the bug: an empty
interpolation in a YAML-LEVEL comment is harmless -- YAML strips it before anything
evaluates expressions -- and five such comments exist in ci.yml and
dependabot-lock-resync.yml. Inside a `run:` block a comment is part of the script
string, so it is parsed. Only the second kind breaks.

3. THE SHARED PRE-COMMIT SHIM ASSUMED A SINGLE CHECKOUT.

`core.hooksPath` points all 10 worktrees at one .git/hooks/pre-commit that
pre-commit generated with two assumptions that a multi-worktree checkout breaks:

  * INSTALL_PYTHON was hardcoded to MessageFoundry-ledger's .venv -- a DISPOSABLE
    worktree. Remove it and the `command -v pre-commit` fallback finds nothing on a
    bare PATH, so every commit in every worktree exits 1. Fails closed, but blocks
    every session at once.
  * ruff/bandit/ledger-gate/forbidden-content are `language: system` -- deliberately,
    so pre-commit can never disagree with the CI/dev ruff on a version. But
    `language: system` resolves from PATH, and PATH holds no venv unless the shell
    activated one. Measured from a bare PATH: `ruff` not found, hooks fail.

install-git-hooks.ps1 now patches the generated shim to resolve BOTH against the
worktree being committed in, so `language: system` gets exactly what that worktree's
venv holds -- what the config always intended, now true from any shell. Marker-
guarded and idempotent, it refuses to splice if pre-commit's `# end templated`
anchor is missing (a wrong insertion point would break commits everywhere), backs up
to pre-commit.prepatch, and re-running is a no-op. `pre-commit install` regenerates
the file and drops the patch, so -Status now reports whether it is applied, prints
the hardcoded INSTALL_PYTHON, and says outright when that path no longer exists.

-Status also compares this worktree's ruff against constraints.lock. That is not
hypothetical: one worktree carried ruff 0.16.0 against pyproject's `<0.16` cap
(installed standalone, so nothing capped it), producing ~829 findings CI does not
have -- and its `ruff check --fix` hook then STRIPPED `# noqa` directives the pinned
0.15.22 still wants. A worktree that lints with a different linter than CI and
rewrites files accordingly is worth one line of output.

A bug the splice found in itself, before it ran anywhere real: implemented with
PowerShell `-replace`, in whose replacement syntax `$_` means THE ENTIRE INPUT
STRING. The patch contains $_mf_root / $_mf_dir / $_mf_py, so it spliced the whole
hook file into itself at each one -- 681 bytes to 9556, with `PATH="#!/bin/sh...`
written into the middle of the PATH assignment. Now String.Replace, which has no
substitution semantics, plus an anchor-count check. Verified on a copy: 2318 bytes,
one patch block, every shell variable intact, `sh -n` and `bash -n` clean, and under
a bare PATH it takes ruff from not-found to this worktree's 0.15.22.

NOT DONE HERE, and it needs the owner: the live .git/hooks/pre-commit is unchanged.
The worktree gate blocks writes under the primary checkout, and .git/hooks is inside
it. Apply with:  pwsh -NoProfile -File scripts\coord\install-git-hooks.ps1
The previous commit taught install-git-hooks.ps1 to splice a worktree-aware block
into .git/hooks/pre-commit. tests/test_ledger_check.py
::test_the_installer_no_longer_writes_a_pre_commit_hook caught it, and the test is
right: the last time this script owned that file, `pre-commit install` moved it to
pre-commit.legacy and chained to it, which FAILS on Windows and blocked every commit
in the repo. That invariant is not incidental, it is scar tissue.

I read pre-commit's own installer before deciding. `is_our_script()` tests whether
the file CONTAINS the template hash on its `# ID:` line, which the patch preserved --
so it would in fact have been recognised as pre-commit's own and overwritten, not
moved aside, and the .legacy hazard would not have fired. But that same code settles
the question the other way: `_install_hook_script` rewrites the file from the
template unconditionally, so ANY third-party block is erased by the next
`pre-commit install` -- the command a fresh clone runs. The patch was fragile by
construction, test or no test. Dropped, not exempted.

The supported fix needs no patching at all: `pre-commit install` bakes INSTALL_PYTHON
to whichever interpreter runs it, so running it from the primary checkout re-anchors
the shim off a removable worktree. -Status now says so, in place of a fix nobody
should apply.

What survives is the diagnosis, which is where the durable value was:

  interp   : ...\MessageFoundry-ledger\.venv\Scripts\python.exe
             ^ NOT the primary checkout, so that worktree is removable. If it goes,
               every commit in all 10 worktrees fails at once (it fails closed).
  ruff     : 0.15.22 (matches constraints.lock)

The ruff line is not decoration. A worktree whose ruff disagrees with
constraints.lock lints with a DIFFERENT linter than CI *and* its `ruff check --fix`
hook rewrites files to match: measured, one worktree carried ruff 0.16.0 against
pyproject's `<0.16` cap (installed standalone, so nothing capped it), reporting ~829
findings CI does not have and STRIPPING `# noqa` directives the pinned 0.15.22 wants.

Three bugs in the is-this-the-primary predicate, each found by running it against
cases rather than reading it:

  1. `"...\MessageFoundry-ledger\...".StartsWith("...\MessageFoundry")` is TRUE, so a
     bare prefix test reported a SIBLING worktree as the primary -- the exact path
     this repo actually has.
  2. git returns the common dir with FORWARD slashes while INSTALL_PYTHON uses
     backslashes, so an un-normalised compare never matched at all.
  3. Even a boundary-correct prefix test is wrong: a worktree at
     `<primary>\.claude\worktrees\<name>` is INSIDE the primary's path and is still
     removable. Only the primary's own `.venv` is non-removable, so the predicate is
     exact equality against that, not containment.

Verified on all three: sibling -> not primary, primary -> primary, nested worktree ->
not primary.
…o claude/artifact-review-standards-ac2b2a
@wshallwshall
wshallwshall merged commit f4ed795 into main Jul 29, 2026
34 checks passed
@wshallwshall
wshallwshall deleted the claude/artifact-review-standards-ac2b2a branch July 29, 2026 23:27
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.

1 participant