[brettheap]

Lane: openreposhape-5 (openRepoShape-5)
Claim: #275 (comment)

What was wrong

pin_source_env_name turns every character outside [A-Z0-9_] into _, so
open-glass, open_glass and open.glass are three legal product names, with
three pin files (contracts/<product lowercased>-pin.yaml), and one variable,
SHAPE_PIN_SOURCE_OPEN_GLASS. On ab623dd (this branch's main parent; the same
rows reproduce on c7c02fa), with the issue's own probe against local bare
remotes and a gh that exits 97:

step	ab623dd	this head
scaffold-project.py --pin open-glass@<A> --pin open_glass@<B>	exit 0, three repositories	exit 2 scaffold-pin-collides, nothing created
validate-pins.py, no source, on a project that already declares the pair	exit 0, both skips offer SHAPE_PIN_SOURCE_OPEN_GLASS	exit 0, one note neutral-pin-source-collides; the skips offer --pin-source <product>=<path> and a sibling checkout only
SHAPE_PIN_SOURCE_OPEN_GLASS=<open-glass>	neutral-pin-source-unreadable for open_glass, exit 1	exit 0, the note; the variable is read for neither pin, so each pin is read from its own source or skipped
SHAPE_PIN_SOURCE_OPEN_GLASS=<open_glass>	neutral-pin-source-unreadable for open-glass, exit 1	the same
--pin-source open-glass=... --pin-source open_glass=...	exit 0	both pins recompute, exit 0, the note

What changed

Every variable name is kept: an operator may have set one and the docs name
them. What is done instead:

• scripts/repo_shape.py: pin_source_env_collisions(products) is the one
  pure answer, {variable: [products]} for each variable two or more distinct
  products share. Distinct means distinct PIN FILES (pin_product_key,
  lower(), which is what contracts/<product lowercased>-pin.yaml is named
  by), so a product typed twice or in two cases is scaffold-project.py --pin spec@<sha> (or code@, shape@, or two --pins whose products differ only by case) passes --dry-run, then creates all three repositories, pushes both legs and dies with REFUSED materialize-collision, because the pin's contracts/<product>-pin.yaml is a file the shape already writes #241's file collision and
  never this one. pin_source_env_shared is the set of those keys.
• scaffold-project.py: _refuse_colliding_pins (left where it is) names each
  variable's --pin arguments as typed after scaffold-project.py --pin spec@<sha> (or code@, shape@, or two --pins whose products differ only by case) passes --dry-run, then creates all three repositories, pushes both legs and dies with REFUSED materialize-collision, because the pin's contracts/<product>-pin.yaml is a file the shape already writes #241's file sentences, in the same
  scaffold-pin-collides refusal. Dry run and real run say the same no, before
  any process starts and ahead of the leg-path refusal. scaffold-project.py --pin spec@<sha> (or code@, shape@, or two --pins whose products differ only by case) passes --dry-run, then creates all three repositories, pushes both legs and dies with REFUSED materialize-collision, because the pin's contracts/<product>-pin.yaml is a file the shape already writes #241's wording and
  remediation are unchanged when only a file collides. A NEW pair is still
  refused, so the situation is not created anew.
• templates/assembly-root/scripts/validate-pins.py: one NOTE per variable,
  neutral-pin-source-collides, naming every product and the variable, the
  keyed --pin-source <product>=<path> that reads each pin, and that which
  declaration stays is the human's decision. It is never a finding and never
  moves the exit code. A product whose variable is shared does not read it (rule
  2 of resolve_neutral_pin_source), and its skip hint offers the keyed
  --pin-source <product>=<path> and not the variable or the bare form.
  A variable one product has to itself reads and is offered exactly as before.
• scripts/repo_shape.py, the two chain readers: link_pins_from_trees does
  not read a variable two links of one chain share, and the
  declared-unverified hint stops naming it. Only a hand-written chain can have
  two such links, since scaffold takes every link but the first from the naming
  policy's alphanumeric open<Product> form.

Each reader of pin_source_env_name, decided:

site	decision
scaffold-project.py --pin	refuse (above)
validate-pins.py rule 2 and the skip hint	a note, and the variable is neither read nor offered for a shared product
repo_shape.py resolve_link_source (via link_pins_from_trees)	skip the variable when another link of the chain shares it
repo_shape.py _chain_link_findings hint	omit the variable for a shared link
adopt-project.py --pin, plan, check, execute	not in this PR, see below

Fix round 1 (from the review of 34dbd86)

The design change, the lane's choice, and one Brett can reverse. The first
round made neutral-pin-source-collides a FINDING. Main through ab623dd
scaffolds such a project (--pin open-glass@A --pin open_glass@B exits 0), and
update-shape.py apply runs the NEW validate-pins.py over the rewritten root,
so for a project already declaring the pair the finding ended every apply as
REFUSED update-validators-red, rolled back, for this update and every later
one, until a person deleted a true declaration of descent. AGENTS.md tells an
agent never to edit "a pinned file, a digest or a manifest to make a row go
green", and the finding's remediation was exactly that, so the person was
locked out. The coordinator chose
the review's option (b): it is a NOTE now. The guard stays (the variable is read
for neither pin), each pin still verifies from its own source, scaffold still
refuses a new pair, and validate-pins.py says the rest in one line. To put the
finding back, change report.advise to report.finding in
_report_pin_source_collisions, give the finding the remediation 34dbd86 had,
and turn the tests that expect the note (and
test_a_project_declaring_a_colliding_pair_takes_a_shape_update) back into
exit 1; the first round's versions of all of them are on 34dbd86.

• Proved on a real old project. Main (ab623dd) scaffolded the pair into
  local bare remotes; update-shape.py apply to the first round's head 34dbd86
  exited 2 REFUSED update-validators-red and rolled back (git branch shows
  only main); to this head it exits 0, the project's validator passes and
  prints the note, and the doctor's pins row is ok before and after. The
  doctor prints passes for a validator that exits 0 and does not echo its
  stdout, so the note itself is visible in update-shape.py apply's output and
  in validate-pins.py's, and the verdict moves for no reason but the update
  (COMPLIANT, SHAPE BEHIND to COMPLIANT). It is now a test:
  test_a_project_declaring_a_colliding_pair_takes_a_shape_update, over local
  stand-ins beside the project, so no gh is asked.
• N2, collisions by the variable. Pairs were decided by casefold(), but
  the pin file is lower(). openß/openss, openſ/opens, openfi/openfi
  and openς/openσ share a variable and have two pin files, and were never
  found. They are now, from the pin-file fold (pin_product_key). A pool of 31
  names is checked pairwise against neutral_pin_path and
  pin_source_env_name themselves, and no single product is ever a collision.
  They are reachable only from a hand-written project.yaml.
• N1, Windows. The guard's only test needed the #! gh stub and was
  skipped on Windows. test_a_shared_variable_answers_for_neither_pin_with_no_gh
  puts a checkout of each pin beside the project, names one in the variable, and
  needs no gh: on ab623dd it is neutral-pin-source-unreadable, here each pin
  reads its own sibling. The hint and the non-ASCII rows are tested in-process,
  with no skip and no filesystem.
• N3, the hint. For a product two pins share, the skip hint offers the keyed
  --pin-source <product>=<path>: the bare form answers for every pin with no
  more specific source, so with two pins it gives the other the finding this
  issue is about.
• N4, no action. The trailers of the earlier commits are settled in the
  squash message. N5 is this title, which now names repo_shape.py.
• scaffold-project.py is not touched this round: fix/adopt-pin-writes-pin-file
  moves that code, and the change would only conflict with it.

Left out

• adopt-project.py is untouched. Its half arrives with adopt-project.py --pin <name> is accepted by plan, check and execute ("adoption verified"), but writes only the name into neutral_product_pins: and no contracts/<name>-pin.yaml, so the adopted root's own validate-pins.py (and its validate.yml) refuses with neutral-pin-missing from the first run #273's PR, which moves
  scaffold's pin machinery into a shared module and routes adopt through
  pin_source_env_collisions. Once it lands, the one place that still keys by
  casefold() (pin_variable_collisions in scripts/shape_materialize.py)
  should read pin_product_key; it cannot differ for an ASCII --pin, which
  is all PIN_ARG_RE takes.
• scaffold-project.py's own _colliding_pin_variables also keys its
  argument lists by casefold() and one docstring there still says the
  project's validator reports the pair "as a finding"; both go with the code
  when adopt-project.py --pin <name> is accepted by plan, check and execute ("adoption verified"), but writes only the name into neutral_product_pins: and no contracts/<name>-pin.yaml, so the adopted root's own validate-pins.py (and its validate.yml) refuses with neutral-pin-missing from the first run #273 moves it. validate-pins.py: a --pin-source value is PRODUCT=PATH only when the text before its first = names a declared neutral-product pin, so a bare stand-in under a=b/ recomputes the digest instead of falling to gh api and a skip, an explicit source that does not exist or is not a directory is the neutral-pin-source-unreadable finding instead of a FileNotFoundError traceback, and a value no pin used is noted (#295) #312's PinSources keys --pin-source by casefold()
  too, so openß= and openss= are one key there; that is validate-pins.py: a --pin-source value is PRODUCT=PATH only when the text before its first = names a declared neutral-product pin, so a bare stand-in under a=b/ recomputes the digest instead of falling to gh api and a skip, an explicit source that does not exist or is not a directory is the neutral-pin-source-unreadable finding instead of a FileNotFoundError traceback, and a value no pin used is noted (#295) #312's.
• A product in neutral_product_pins: and a chain link in
  naming.referent_chain that share a variable are read by two validators in
  one process, and neither sees both sets. Not detected here.
• The doctor does not print a passing validator's stdout, so its pins row
  does not carry the note. Not changed: it would add a file to this PR.
• No --help text changed, so docs/cli.md is untouched. README.md is at its
  line cap and its blob is pinned by docs/handbook.html, so the one README
  sentence this wants needs a regenerated handbook and waits for its own change.
• validate-pins.py and repo_shape.py ship into projects, so every project's
  shape-pin.yaml reads upstream-changed for both until update-shape.py apply, which now succeeds for a project that declares such a pair.

Evidence

All on the head, with the throwaway venv holding git-filter-repo 2.47.0, a gh
that cannot be reached (a stub first on PATH, GH_TOKEN bogus, an empty
GH_CONFIG_DIR), and the repository-local user.name blanked at command-line
scope, as the standard's checkout sets one that two scaffold tests read.

• tests/test_pin_source_env_collisions.py is 137 tests and passes on this
  head. Laid over 34dbd86, 50 fail and 87 pass: the note and update tests, the
  non-ASCII rows, the hint, the sibling guard, and pin_product_key itself
  (which 34dbd86 does not have); the scaffold refusals, the chain readers and
  the controls hold on both.
• Targeted, run on ad6f569, whose tree differs from this head in
  tests/test_pin_source_env_collisions.py only (the last edits made one
  comparison read the committed blob and wrapped one line): the new file,
  test_pin_validator, test_neutral_pin_validator, test_cli_reference,
  test_manifest_validator, test_naming_policy and test_repo_hygiene
  445 passed; the scaffold pin, collision, referent-chain, leg-path and e2e
  files 245 passed; tests/test_update_shape*.py 218 passed, 3 skipped
  (junctions, NTFS only); tests/test_shape_doctor.py 254 passed, 3 skipped
  (the PowerShell line, a Windows host only).
• On this head itself: the new file, test_repo_hygiene, test_cli_reference,
  test_pin_validator and test_neutral_pin_validator 244 passed.
• This head merged with origin/main a983e6a, in a throwaway worktree: the new
  file, test_pin_validator, test_neutral_pin_validator, the scaffold pin,
  collision, referent-chain and e2e files, the whole doctor file and
  test_cli_reference 590 passed, 3 skipped (the PowerShell line); the new
  file alone, in its final form, 137 passed. origin/main has moved twice
  since; the merge-tree below is against it.
• Twelve mutation probes, each hunk reverted alone: all caught. The
  note-to-finding one is caught by the update test (a committed mutant, since the
  test clones the repository's HEAD) and the guards by the sibling test, run with
  every test that needs a #! stub deselected as a Windows runner would;
  the hint one by an in-process test.
• Nothing was run on macOS or Windows.

Merge-tree

git merge-tree --write-tree of this head (6494714) against origin/main 1f90a0f
is clean, and so is the merge of origin/main a983e6a, which the merged-tree runs
above used. #312, #309, #270, #268, #228, #245 and #351 have landed. The rest
were computed on ad6f569 (the same tree but for one test file) against
a983e6a. Open PRs touching this branch's files, all clean against the head:
#314, #355 and #373 (scaffold-project.py), #343 (validate-pins.py), #390
(repo_shape.py) and #394 (repo_shape.py, which conflicts with origin/main by
itself). The other 25 open PRs and the lane-5 branches
fix/adopt-check-link-and-manifest, fix/scaffold-free-text-values,
fix/family-init-values, docs/adopt-execute-exits,
fix/yaml-flow-brackets-surrogates and fix/adopt-pin-writes-pin-file
(#273's, 7a94b6b, also clean for this head) are clean too. #356, #382 and
fix/adopt-record-submodule-placement conflict with origin/main and not with
this head; #358, fix/adopt-late-failure-notice and
fix/adopt-plan-quote-yaml-keywords conflict with both, with origin/main by
themselves, so they are not this branch's. Neither are the five PRs below #200
that origin/main has not landed (#94, #96, #98, #100 and #161), which conflict
with it too. The PR set is read from the refs under refs/pull/ that
origin/main's history has no (#n) subject for, not from GitHub, so a closed
one may be in it.

One conflict, with fix/link-source-values (#338, 40762ca) in
scripts/repo_shape.py, four hunks. Not resolved in either branch; take both
sides: (1) resolve_link_source's signature, default_source=None, env_shared: bool = False, *, tried: list | None = None; (2) its docstring, both
paragraphs; (3) link_pins_from_trees's docstring, both paragraphs; (4) its
call, default_source, pin_product_key(name) in shared, tried=asked) followed
by #338's if tried is not None: tried[name] = asked. The auto-merged
resolve_link_source body already composes: a shared variable is never an
env candidate and so never appears in #338's tried.

Known limits after the re-verify (APPROVE WITH NITS, 0/0/3, at 6494714). For the four non-ASCII pairs this change now detects (ß/ss, ſ/s, fi/fi, ς/σ), the keyed --pin-source <product>=<path> form does NOT separate the two pins: #312's source lookup folds both keys into one, so one pin can get a false unreadable finding. A sibling checkout does separate them. Only a hand-written project.yaml can declare such a pair. The doctor's pins row still prints only "passes" for a validator that passes, so it does not carry the collision note; that needs shape-doctor.py, and is left for a follow-up. To restore the finding, the report.advise(...) call becomes a report.finding(...) call with that method's own arguments; it is not a literal word swap.

Closes #275

🤖 Generated with Claude Code

Summary by Sourcery

Prevent products that share a pin-source environment variable from being created or verified ambiguously while preserving compatibility for existing projects.

Bug Fixes:

• Prevent ambiguous pin-source environment variables from being used for multiple distinct products, avoiding incorrect pin verification.

Enhancements:

• Centralize detection of pin-source environment collisions based on distinct pin files.
• Refuse scaffolding colliding pins, report existing collisions as non-blocking validator notes, and adjust chain-source hints and resolution accordingly.

Tests:

• Add comprehensive coverage for punctuation, casing, non-ASCII product names, scaffold refusal behavior, validator notes, update compatibility, source hints, and chain readers.

[brettheap]

(lane openRepoShape-5, 2026-10-12T16:19Z) @codex review

[sourcery-ai]

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 2 days and 3 hours by commenting @sourcery-ai review. Upgrade to get a review now.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[sourcery-ai]

Reviewer's Guide

The PR introduces a single pin-file-aware collision model for non-injective SHAPE_PIN_SOURCE_* names, refuses ambiguous pairs during scaffolding, safely treats collisions as non-failing validator notes for legacy projects, avoids ambiguous variables in chain readers, and validates the behavior with extensive regression tests.

Sequence diagram for validating colliding pin sources

sequenceDiagram
    participant V as validate-pins.py
    participant R as repo_shape.py
    participant E as Environment
    participant S as PinSources
    participant C as PinCheckout
    V->>R: pin_source_env_collisions(products)
    R-->>V: shared variable groups
    V->>V: report neutral-pin-source-collides
    loop each declared product
        V->>R: pin_product_key(product)
        R-->>V: shared status
        V->>S: resolve_neutral_pin_source(product, env_shared)
        alt shared variable
            S-->>V: skip environment variable
            S->>C: use keyed source or sibling checkout
        else private variable
            S->>E: read pin_source_env_name(product)
            E-->>S: checkout path
        end
    end

Loading

Flow diagram for pin source collision handling

flowchart TD
    A["Declared products or --pin arguments"] --> B[pin_source_env_collisions]
    B --> C{Shared variable and distinct pin files?}
    C -->|No| D[Normal source resolution]
    C -->|Yes| E{Entry point}
    E -->|Scaffold| F[Refuse scaffold-pin-collides]
    E -->|validate-pins.py| G[Report neutral-pin-source-collides note]
    E -->|Chain reader| H[Skip shared environment variable]
    G --> I[Use keyed --pin-source or sibling checkout]
    H --> I

Loading

File-Level Changes

Change	Details	Files
Centralize detection of distinct products that map to the same pin-source environment variable.	Add pin-file-based product identity using lowercased names. Return shared environment variables and affected products in stable input order. Ignore repeated declarations of the same pin file while detecting collisions.	scripts/repo_shape.py
Prevent newly scaffolded projects from declaring ambiguous pin-source variable pairs.	Reject colliding --pin products with scaffold-pin-collides before any repository/process work. Include typed arguments, products, shared variable, and combined remediation in refusal output. Preserve existing file-collision behavior and make dry-run and real-run responses identical.	scaffold-project.py
Make existing projects safe and updateable when pin-source variables collide.	Report one non-failing neutral-pin-source-collides note per shared variable. Skip the shared environment variable for every affected pin while retaining keyed sources and sibling checkout resolution. Remove the shared variable and bare-source options from affected skip hints, while preserving them for unshared products.	templates/assembly-root/scripts/validate-pins.py
Prevent ambiguous environment variables from being used during referent-chain pin resolution.	Skip shared variables for links in the same chain. Keep explicit keyed link sources functional. Omit shared variables from declared-unverified remediation hints.	scripts/repo_shape.py
Add comprehensive regression coverage for collision detection, CLI refusal, validation behavior, updates, Unicode edge cases, and chain resolution.	Test punctuation and non-ASCII names against actual pin-file identity and environment naming. Cover no-process scaffold refusal, validation notes and hints, sibling/source fallback, and update-shape apply compatibility. Verify unchanged behavior for distinct variables and explicit keyed sources.	tests/test_pin_source_env_collisions.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#275	Prevent a shared SHAPE_PIN_SOURCE_<PRODUCT> variable from being used as the source for multiple distinct neutral products or chain links.	✅
#275	Prevent newly scaffolded projects from declaring colliding pins, and handle already-declared collisions without reporting the wrong checkout as an unreadable-pin finding.	✅
#275	Give operators an unambiguous way to verify colliding pins and avoid suggesting the shared variable in their skip or chain hints.	✅

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Sourcery assessment

Approved.

[sonarqubecloud]

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.9% Duplication on New Code

See analysis details on SonarQube Cloud