Skip to content

scripts/shape_materialize.py and adopt-project.py: a --pin-source that is no clone's top level is refused pin-source-not-a-clone, and check finds a recorded one before a partial clone's NOTE, so a folder inside a clone no longer records the digest of part of its tree, or of nothing (#800) - #842

Merged
brettheap merged 6 commits into
mainfrom
fix/800-pin-source-top-level
Oct 18, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openreposhape-1 (openRepoShape-1)
Claim: #800 (comment)

What

A --pin-source must be a clone's top level. adopt-project.py plan and scaffold-project.py (its --dry-run and the real run) refuse any other directory by name, and adopt-project.py check finds one a plan records. So no pin records the digest of part of a tree, or of nothing.

One question, asked before any digest is read. _digest_from_pin_source (scripts/shape_materialize.py) asks git where the path is, after its two stat checks (a missing path, a file) and before tree_digest runs:

  • a working tree's top level passes: git rev-parse --show-prefix prints nothing, and the path is not inside the git directory. A linked worktree's top level passes the same way;
  • a bare clone passes where --is-bare-repository is true and --absolute-git-dir is the path itself;
  • anything else is refused as pin-source-not-a-clone, which names the top level git finds, or says git finds none.
REFUSED pin-source-not-a-clone: --pin-source /…/glass/sub is not a clone's top level: it is a folder inside the clone at /…/glass, and a digest read there covers only what the commit holds under that folder
Remediation: pass --pin-source /…/glass, the clone's top level, where a pin's digest is read over its commit's whole tree.

The other answers git can give, in the same refusal:

  • a folder of a bare clone: it is a folder inside the bare clone at /…/glass.git, with the same remediation naming that clone;
  • a path inside a clone's git directory: it is inside /…/glass/.git, the git directory of a clone, where git finds no top level;
  • a directory that is no clone at all: git finds no clone there: `git rev-parse …` in /…/plain exited 128: fatal: not a git repository ….

The last two name no top level, so their remediation is: "pass --pin-source the top level of a clone of the pinned product (git rev-parse --show-toplevel prints it in that clone's working tree), or drop --pin-source and let gh api read the forge."

check asks a recorded path the same question, first. check reads the clone a plan's digest_source: local-clone (<path>) names. It now asks this question before _why_not_rechecked, and so before a partial clone's NOTE. A recorded path that is no clone's top level is a new finding, plan-pin-source-not-a-clone, which execute refuses before any leg:

FINDING plan-pin-source-not-a-clone: `pins:` entry 1 ('openGlass') records that `plan` read its digest from /…/glass-partial/notaclone, which is not a clone's top level: it is a folder inside the clone at /…/glass-partial, and a digest read there covers only what the commit holds under that folder. `plan` refuses such a --pin-source as `pin-source-not-a-clone`: re-run `plan` with `--pin-source /…/glass-partial`, its top level.

The code pair. pin-source-not-a-clone (plan and scaffold) and plan-pin-source-not-a-clone (check's finding) mirror #651's pin-not-a-commit and plan-pin-not-a-commit. The lane-1 coordinator accepted the pair for this PR. Both tools' --pin-source help names the refusal, and docs/cli.md is regenerated.

Unchanged:

  • What a top level digests, byte for byte, so existing pins stay valid.

  • tree_digest in scripts/repo_shape.py, a pinned shape copy. Nothing in that file changes.

  • L1-82, first case (a folder of a clone). plan, the scaffold's --dry-run and its real run each refuse --pin-source glass/sub as pin-source-not-a-clone, exit 2, naming glass as the path to pass. check of a plan main wrote from glass/sub is plan-pin-source-not-a-clone, exit 1, where main said "digest recomputes" and plan ok. A folder of a partial clone is the same finding, where main called it "a partial clone" and passed it.

  • L1-04 (a folder inside a partial clone that is no clone). plan --pin-source glass-partial/notaclone is pin-source-not-a-clone, naming glass-partial. check of a plan main wrote from it (tree e3b0c44298fc…, the sha256 of nothing) is plan-pin-source-not-a-clone, exit 1, where main passed it with the partial-clone NOTE.

Why (repro on main bf1f099)

The product clone glass holds README.md, sub/a.txt and lib/b.py; its true digest is d15921cfbaa1… (from tree_digest at its top level). Every run used a stub gh that logs its calls, and none was logged.

  • adopt-project.py plan --pin openGlass@<head> --pin-source glass/sub: exit 0, pin opensoft/openGlass @ 1cbe383906e2 tree a17948e338f4…, the digest of sub/ alone.
  • adopt-project.py check on that plan: NOTE … digest recomputes at 1cbe383906e2 in the clone plan read it from, …/glass/sub. and plan ok, exit 0.
  • scaffold-project.py … --pin-source glass/sub --dry-run: exit 0, pin opensoft/openGlass @ 1cbe383906e2 tree a17948e338f4…, --dry-run: nothing was created.
  • plan … --pin-source glass-partial/notaclone: exit 0, tree e3b0c44298fc…. check on that plan: NOTE … digest NOT rechecked here: the clone at …/glass-partial/notaclone is a partial clone … and plan ok, exit 0.

The cause: tree_digest runs git ls-tree -r inside the given path, and without --full-tree git lists only the subtree under the folder it runs in. PR #725 (#651) set this fix aside because it "changes what both tools accept"; what they accepted was a wrong digest.

How

  • scripts/shape_materialize.py:
    • pin_source_not_top_level(source) asks git rev-parse --is-bare-repository --is-inside-git-dir --show-prefix in one call (the prefix printed last, so a folder whose name holds a line break is read whole). Then, where needed, it asks --absolute-git-dir (bare clone, git directory) or --show-toplevel (a folder).
    • It reads git through repo_shape.git_out. A git that fails gives git_out's own refusal text, with the folder and git's reason spelled, and that is the "no clone" answer. A path cwd= cannot enter still raises the OSError that pin-source-unreadable names, as before.
    • _refuse_unless_a_clone_top_level raises pin-source-not-a-clone from it.
    • _digest_from_pin_source calls it after its two stat checks.
    • pin_digest_from_source lets that one refusal through instead of wrapping it as pin-source-unreadable.
    • One import line is added: from repo_shape import git_out.
  • adopt-project.py:
    • _not_a_top_level_finding is asked first in _pin_digest_findings, before _why_not_rechecked. It asks only a recorded path that is a directory it can enter, and anything else goes on to the questions after it, as before.
    • New constant PIN_SOURCE_NOT_A_CLONE = "plan-pin-source-not-a-clone".
    • The comment that called "a directory that is no clone" plan-pin-source-unreadable is corrected.
    • The question reads no object, so in a partial clone it reads no network.
  • Help: both tools' --pin-source help names pin-source-not-a-clone, and docs/cli.md is regenerated (python3 scripts/render-cli-reference.py).
  • tests/test_pin_source_unreadable.py: the not-a-clone row (now lacks-the-commit) and test_git_s_own_reason_names_the_source_spelled_too were a plain directory. They are now a git init clone that lacks the commit. That is still pin-source-unreadable with git's own ls-tree reason, and it keeps both tests on the path they were written for (scaffold-project.py: pin_digest_from_source catches only Refusal, so a --pin-source that does not exist, is a file, or sits under a parent this user cannot enter ends --dry-run and the real run alike in a FileNotFoundError, NotADirectoryError or PermissionError traceback (exit 1), where a directory that is not a clone is the pin-source-unreadable refusal (exit 2) with its remediation #339). A directory that is no clone is now pin-source-not-a-clone, covered by the new file.

Where the refusals sit relative to gh:

  • plan refuses in declared_pin_values, after --source is read. A --source org/repo is cloned before any pin is read, so the order allows nothing earlier. With a local --source and --pin-source, plan calls no gh at all.
  • The scaffold refuses in _declaration, which runs before _leg_visibility and remote_is_empty ask gh repo view. So against GitHub the dry run refuses before any gh call; the test's control, a top level, does reach gh.
  • check and execute run no gh.

Tests

New: tests/test_pin_source_top_level.py, 48 tests. On main bf1f099, 32 fail. The other 16 are the controls, which pass on main and with this change: the top-level digest, and check's NOTE, are what they were.

  • test_a_path_that_is_no_clone_s_top_level_is_refused (21). Seven paths:

    • a folder of a full clone (L1-82);
    • a folder of a partial clone;
    • an untracked folder;
    • an untracked folder of a partial clone (L1-04);
    • a folder of a bare clone;
    • a .git directory;
    • a directory that is no clone.

    Each runs through plan, the scaffold's --dry-run and the real scaffold, and must be exit 2 with the one refusal, nothing made, and no gh call.

  • test_check_finds_a_recorded_path_that_is_no_clone_s_top_level (7): the same seven, recorded with the digest main's plan read there. Each must be plan-pin-source-not-a-clone, exit 1, with no NOTE for the pin, and execute must refuse it before any leg.

  • Controls, test_a_top_level_is_read_as_it_always_was (12): a top level, a bare clone, a linked worktree and a partial clone, each through all three runs. Each must give the same digest as repo_shape.tree_digest at the clone's top level.

  • Controls, test_check_asks_a_recorded_top_level_as_it_always_did (4): "digest recomputes", and the partial clone's NOTE, as before.

  • test_the_scaffold_refuses_before_it_asks_github: against GitHub, no gh call before the refusal; the top-level control does reach gh.

  • test_a_path_holding_a_control_character_is_named_spelled: the folder and the top level are both repr'd, and the only line beginning REFUSED is the tool's own.

  • test_each_tool_names_the_refusal_in_its_help (2).

Changed: tests/test_pin_source_unreadable.py. The not-a-clone row is now lacks-the-commit, and the spelled-reason test uses a git init clone (see How).

Revert table (each change reverted alone on the merged head a2dc2b6, run over tests/test_pin_source_top_level.py, tests/test_pin_source_unreadable.py and tests/test_cli_reference.py):

# Reverted alone Fails
R1 the call _refuse_unless_a_clone_top_level(source) in _digest_from_pin_source 23: the 21 refusal cases, the GitHub-order test, the control-character test
R2 the re-raise in pin_digest_from_source (the refusal wrapped as pin-source-unreadable) the same 23
R3 check's question (found = None) 7: every check finding case
R4 the bare-clone branch 8: the bare folder's 3 refusals and its check finding; the bare clone's 3 digest controls and its check control
R5 the bare clone's samefile comparison 4: the bare folder's 3 refusals and its check finding
R6 the git-directory branch 4: the .git directory's 3 refusals and its check finding
R7 the scaffold's --pin-source help 6: the scaffold's help test and 5 in tests/test_cli_reference.py
R8 adopt-project.py plan's --pin-source help 6: adopt's help test and the same 5
R9 docs/cli.md as on main 5 in tests/test_cli_reference.py
R10 git init in the lacks-the-commit row 3: that row through all three runs
R11 git init in the spelled-reason test 2: both its runs

Both modes, on the merged head a2dc2b6 (main 24a977c merged), dirty 0, each shard ending EXIT=0. The 112 files are:

  • every file grep -l -E 'adopt-project|adopt_project|\bADOPT\b' tests/*.py names;
  • every test file that touches --pin-source;
  • tests/test_shape_doctor.py, tests/test_scaffold_leg_path_collisions.py, tests/test_cli_reference.py and tests/test_agents_md_codes.py.

They ran in 8 shards with /usr/bin/python3 -m pytest -q -p no:cacheprovider, a stub gh first on PATH that logs each call, GH_TOKEN=bogus and an empty GH_CONFIG_DIR.

  • With git-filter-repo on PATH: 7,710 passed, 13 skipped, 1 xfailed.
  • With it hidden (PATH=<stub>:/usr/local/bin:/usr/bin:/bin): 7,331 passed, 392 skipped, 1 xfailed.

Windows

CI is Linux only (#597). The new tests write their stand-ins as bytes, read files back with read_text()/parse_yaml, and create only NTFS-legal names. The control-character test is POSIX-only, as its #339 neighbours are, and the test whose control needs the stub gh to run is WINDOWS_SKIP, as #651's are.

Not in this PR

  • scripts/repo_shape.py (tree_digest, a pinned shape copy) and the root's templates/assembly-root/scripts/validate-pins.py, which reads its own --pin-source and is not asked this question here.
  • The scaffold's --referent-chain declaration read from a bare --pin-source (link_pins_from_trees, in scripts/repo_shape.py), which reads a project.yaml path and no digest.
  • docs/release-notes/.

Known limits

  • A path inside a non-bare clone's git directory (glass/.git) is refused although its digest would have been the whole tree's. This is the brief's rule: the path is not the top level, and the refusal names the git directory rather than guessing its working tree.
  • A git that refuses a clone it does find (safe.directory, "dubious ownership") is "git finds no clone there", followed by git's own reason, which says which it was.

Closes #800

🤖 Generated with Claude Code

Summary by Sourcery

Require pin sources to identify clone top levels so recorded pin digests always cover the complete pinned tree.

New Features:

  • Require pin sources to be the top level of a working or bare clone, with actionable refusals for nested, git-directory, and non-clone paths.
  • Have plan checks detect recorded non-top-level pin sources before partial-clone recheck notes and block execution.

Bug Fixes:

  • Prevent recording digests for only a subtree or an empty directory when pinning from an invalid nested path.

Enhancements:

  • Preserve existing digest behavior for valid working trees, linked worktrees, bare clones, and partial clones while centralizing clone-top-level validation.

Documentation:

  • Update CLI help and the generated CLI reference with the new pin-source validation behavior.

Tests:

  • Add coverage for invalid pin-source paths, valid clone top levels, check findings, GitHub call ordering, path escaping, and help text.
  • Adjust unreadable-source tests to distinguish clones lacking the requested commit from directories that are not clone top levels.

Brett Heap and others added 3 commits October 17, 2026 11:04
…t_pin_source_top_level.py, the failing tests for a --pin-source that is no clone's top level (#800)

The tests are written first and have not run yet. The fix in scripts/shape_materialize.py and adopt-project.py, the help texts and docs/cli.md are not written yet.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hat is no clone's top level is refused `pin-source-not-a-clone`, and `check` finds a recorded one before a partial clone's NOTE, so a folder inside a clone no longer records the digest of part of its tree, or of nothing (#800)

`tree_digest` runs `git ls-tree -r` in the `--pin-source`, and there git lists only what is under the folder it runs in. On main bf1f099, `plan --pin-source glass/sub` recorded the digest of `sub/` alone, the scaffold's `--dry-run` printed it, and `check` recomputed it and said `plan ok` (#457's L1-82). A folder the commit does not hold, inside a partial clone, recorded the sha256 of nothing and passed `check` with the partial-clone NOTE (L1-04).

- `_digest_from_pin_source` asks git, after its two `stat` checks and before any digest, whether the path is a clone's top level (`pin_source_not_top_level`). A working tree's top level, a linked worktree's included, has an empty `rev-parse --show-prefix` outside the git directory; a bare clone's `--absolute-git-dir` is the path itself. Anything else is `pin-source-not-a-clone`, naming the top level git finds ("pass --pin-source <top level>"), or saying git finds none. `pin_digest_from_source` lets that one refusal through instead of wrapping it as `pin-source-unreadable`.
- `check` asks a recorded `local-clone (<path>)` the same question first, before `_why_not_rechecked`'s partial-clone NOTE, and finds `plan-pin-source-not-a-clone`, which `execute` refuses before any leg.
- Both tools' `--pin-source` help names the refusal; docs/cli.md is regenerated.
- tests/test_pin_source_unreadable.py: the `not-a-clone` row and the spelled-reason test now use a `git init` clone that lacks the commit, which is still `pin-source-unreadable` with git's own reason.

What a top level digests is unchanged, and scripts/repo_shape.py is not touched. Tests: tests/test_pin_source_top_level.py (48).

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 17, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR prevents pins from reading partial or empty subtrees by requiring --pin-source to identify a clone’s top level, applies the same validation to recorded sources during check/execute, documents the new refusal, and adds broad regression coverage while preserving valid top-level digest behavior.

Sequence diagram for validating a pin source before digesting

sequenceDiagram
    participant Caller
    participant ShapeMaterialize
    participant Git
    participant TreeDigest

    Caller->>ShapeMaterialize: pin_digest_from_source(source, commit, repository)
    ShapeMaterialize->>ShapeMaterialize: _digest_from_pin_source(source, commit)
    ShapeMaterialize->>ShapeMaterialize: stat(source)
    ShapeMaterialize->>Git: rev-parse --is-bare-repository --is-inside-git-dir --show-prefix
    Git-->>ShapeMaterialize: clone location and prefix
    alt source is clone top level
        ShapeMaterialize->>TreeDigest: tree_digest(source, commit)
        TreeDigest-->>ShapeMaterialize: whole-tree digest
    else source is not clone top level
        ShapeMaterialize-->>Caller: Refusal pin-source-not-a-clone
    end
Loading

Sequence diagram for check finding an invalid recorded pin source

sequenceDiagram
    participant Check
    participant AdoptProject
    participant ShapeMaterialize
    participant Git

    Check->>AdoptProject: _pin_digest_findings(plan)
    AdoptProject->>ShapeMaterialize: pin_source_not_top_level(recorded_path)
    ShapeMaterialize->>Git: rev-parse location query
    Git-->>ShapeMaterialize: path is inside clone or not a clone
    ShapeMaterialize-->>AdoptProject: why and optional clone top level
    AdoptProject-->>Check: FINDING plan-pin-source-not-a-clone
    Check-->>Check: refuse execute before any leg
Loading

File-Level Changes

Change Details Files
Enforce that local pin sources are clone top levels before computing digests, with explicit refusals and remediation for nested working-tree folders, bare-clone subfolders, git directories, and non-repositories.
  • Added Git-based top-level detection covering regular clones, linked worktrees, bare clones, git directories, and paths Git cannot identify.
  • Added pin-source-not-a-clone refusal handling so it is not wrapped as pin-source-unreadable.
  • Preserved existing stat, unreadable-source, and digest behavior for valid top-level sources.
scripts/shape_materialize.py
Make recorded local pin sources undergo the same top-level validation during plan checking and execution.
  • Added the plan-pin-source-not-a-clone finding and checked recorded paths before recomputation or partial-clone notes.
  • Ensure execution refuses invalid recorded sources before running any leg.
  • Corrected related comments and documented the new refusal in plan help.
adopt-project.py
Expose the new pin-source constraint in scaffold help and regenerated CLI documentation.
  • Updated scaffold --pin-source help to describe the accepted top-level paths and refusal code.
  • Regenerated the corresponding CLI reference entries.
scaffold-project.py
docs/cli.md
Add comprehensive coverage for top-level validation, ordering, preserved behavior, and updated unreadable-source cases.
  • Added tests for nested paths in regular, partial, bare, linked-worktree, git-directory, and non-repository scenarios across plan and scaffold modes.
  • Added check/execute finding tests, no-network and no-GitHub ordering checks, path escaping coverage, help assertions, and valid top-level controls.
  • Reclassified plain directories from pin-source-unreadable to pin-source-not-a-clone while retaining coverage for clones that lack the requested commit.
tests/test_pin_source_top_level.py
tests/test_pin_source_unreadable.py

Assessment against linked issues

Issue Objective Addressed Explanation
#800 Refuse --pin-source paths that are not a clone's top level in plan and the scaffold, before reading a digest, and identify the top level to pass when available. ✅
#800 Have check flag a recorded pin-source that is not a clone's top level before issuing a partial-clone note, so the invalid recorded digest cannot pass. ✅
#800 Preserve whole-tree digest behavior for valid clone top levels, including bare clones and linked worktrees, without changing the pinned tree_digest implementation. ✅

Possibly linked issues


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

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-1, 2026-10-17T19:15:21Z) Lane 1's independent review of #842 at a2dc2b6: PR-READY

Reviewer r800, read-only. I tested a local-only merge, d665dd47, of head a2dc2b67 with main bad05d21 (#723). The merge is clean: git auto-merged adopt-project.py, scaffold-project.py, scripts/shape_materialize.py and tests/test_pin_source_unreadable.py with no conflict.

Every probe ran with:

  • a stub gh first on PATH that logs each call and exits 97;
  • GH_TOKEN=bogus-r800 and an empty GH_CONFIG_DIR;
  • GIT_CONFIG_GLOBAL=/dev/null and GIT_ALLOW_PROTOCOL=file.

<P> below stands for the probe's base folder.

The issue's lines, on main and on the merge

The product clone glass holds README.md, sub/a.txt and lib/b.py. Its true digest is 1c1c705a8851….

Line main bad05d2 merge d665dd47 Verdict
L1-82: plan / scaffold --dry-run / scaffold real run with --pin-source glass/sub exit 0, tree a17948e338f4… (the real run made remotes and work) exit 2 REFUSED pin-source-not-a-clone: --pin-source <P>/glass/sub is not a clone's top level: it is a folder inside the clone at <P>/glass, …, made nothing; dry run and real run print the same bytes fixed
L1-82: check of the plan main wrote from glass/sub NOTE … digest recomputes … in the clone plan read it from, <P>/glass/sub. FINDING plan-pin-source-not-a-clone: … re-run plan with --pin-source <P>/glass, its top level.; no NOTE fixed
L1-82: a folder of a partial clone (partial/sub) plan records a17948e338f4…; check NOTEs "is a partial clone" refused; check FINDING naming <P>/partial fixed
L1-04: partial/notaclone plan records tree e3b0c44298fc…; check passes it with the partial-clone NOTE refused; check FINDING, no NOTE fixed

Attack 1: can a source that is not a top level still record a partial or empty digest? No.

A digest is partial exactly when git's prefix in the source is not empty. tree_digest and the new question both run git in the same cwd with the same environment, so they get the same prefix from git, and the question refuses every non-empty prefix.

I ran 27 sources through plan, the scaffold's --dry-run and its real run, on both trees: 162 runs, every one with 0 gh calls.

Refused on the merge. Main recorded the digest shown for each.

  • Sources main recorded a partial or empty digest for:
    • a folder of a clone: a17948…;
    • an untracked folder: e3b0c4…;
    • a folder of a partial clone: a17948…;
    • an untracked folder of a partial clone: e3b0c4…;
    • a folder inside a submodule checkout (super/mods/glass/sub): a17948…;
    • a symlink to glass/sub: a17948….
  • Sources main read the full 1c1c70… from, refused by the brief's design and named in the PR's Known limits:
    • a folder of a bare clone (glass.git/refs);
    • glass/.git, and a symlink to it;
    • a separate git directory (sep.gitdir).
  • Sources main already refused as pin-source-unreadable, now pin-source-not-a-clone:
    • a superproject folder (super/mods);
    • a directory that is no clone (plain).

Accepted on the merge with main's exact output. Each reads 1c1c705a8851…:

  • a top level;
  • a bare clone;
  • a linked worktree;
  • a worktree of a bare clone (git -C glass.git worktree add);
  • a partial clone;
  • a top level whose .git is a gitdir file (--separate-git-dir);
  • a submodule checkout's top level;
  • a nested independent clone (outer/inner);
  • an untracked bare clone inside a working tree;
  • a symlinked clone root, and a symlinked bare clone;
  • a clone under a symlinked parent;
  • glass/sub/...

Environment variables:

  • GIT_DIR=glass/.git with GIT_WORK_TREE=glass and --pin-source glass/sub: refused. Main recorded a17948….
  • GIT_DIR with GIT_WORK_TREE=/: glass is refused as "a folder inside the clone at /". Main recorded e3b0c4….
  • GIT_DIR alone: any directory passes and reads the full tree of GIT_DIR, as on main. That is the right digest of the clone GIT_DIR names, not a partial one.

Case twins. I could not test a case-insensitive filesystem here. From the code: a different-case top level still gets an empty prefix, a different-case folder still gets a non-empty one, and the bare test uses os.path.samefile. So I found no false refusal and no false pass.

safe.directory (simulated with GIT_TEST_ASSUME_DIFFERENT_OWNER=1): a top level is refused pin-source-not-a-clone: … git finds no clone there: … detected dubious ownership …. Main refused it as pin-source-unreadable. See finding 2.

Attack 2: order and side effects

  • Scaffold against GitHub (no --local-remote-dir), --dry-run and the real run, each with glass/sub: exit 2, gh_calls=0, and the work directory was not made. The control, glass, makes its first gh call as gh repo view testorg/MedxGlass --json name. _declaration runs before _leg_visibility and _reusable_remotes.
  • plan with a local --source: 0 gh calls in every run, refused or not.
  • check:
    • The finding comes once per pin.
    • The pin's NOTE is gone (why = None if found).
    • Nothing else is said about that pin: no plan-pin-not-a-commit, no mismatch.
    • The finding count rises from 3 to 4 in each case.
    • 0 gh calls.
    • git rev-parse reads no object, so a partial clone fetches nothing.
  • Control-character paths (ESC, LF and CR in the name), for the no-clone and .git cases: exit 2, no raw ESC, and two lines, the tool's REFUSED line and its remediation. No forged line appears.

Attack 3: byte identity for valid sources

All 13 valid sources above, main against the merge:

  • plan: stdout, stderr and exit code identical, and plan.yaml identical (cmp).
  • Scaffold --dry-run: identical once the shape opensoft/openRepoShape @ <tool's own HEAD> line is masked. That line is the tool's own commit, so it differs between any two commits.
  • Scaffold real run: contracts/openglass-pin.yaml in the remote is byte-identical. stdout differs only in the shape line and in the leg commits that record it.
  • check of main-written plans for 11 valid sources: stdout, stderr and exit code identical, NOTE … digest recomputes … included.
  • scripts/repo_shape.py: git diff bad05d21 HEAD -- scripts/repo_shape.py is empty.

Attack 4: the re-pointed not-a-clone row is honest

#339's not-a-clone row was the control for git's own failure: tree_digest fails, the Refusal becomes pin-source-unreadable, and the refusal carries git's sentence (`git ls-tree -r -z <commit>` in <source> exited) and the remediation.

A plain directory no longer reaches ls-tree. So the row now uses a git init clone that lacks the commit, which takes exactly that path again. Its assert is unchanged. test_git_s_own_reason_names_the_source_spelled_too likewise keeps asserting the spelled path inside git's reason. The no-clone directory moved to the new file (row no-clone), so it is still covered.

Revert rows I re-ran

Each revert was applied alone in a fresh clone at a2dc2b67, run over test_pin_source_top_level.py and test_pin_source_unreadable.py:

Row Reverted Writer said r800 got
R1 the call _refuse_unless_a_clone_top_level(source) 23 23 failed, 106 passed: the 21 refusal cases, the GitHub-order test, the control-character test
R3 found = None in _pin_digest_findings 7 7 failed, 122 passed: every check finding case
R5 the bare branch's samefile test 4 4 failed, 125 passed: the bare folder's three refusals and its finding
R10 git init in lacks-the-commit 3 3 failed, 126 passed: that row through all three runs

My gate

On merge d665dd47, dirty 0, with git-filter-repo on PATH, using /usr/bin/python3 -m pytest -q -p no:cacheprovider -o tmp_path_retention_policy=failed. The files:

  • test_pin_source_top_level.py and test_pin_source_unreadable.py;
  • test_shape_doctor.py, test_scaffold_leg_path_collisions.py and test_cli_reference.py;
  • extra: test_agents_md_codes.py, test_adopt_pins.py and test_pin_annotated_tag.py.

Result: 906 passed, 3 skipped, 1 xfailed in 774.09s, EXIT=0.

CI

  • SonarCloud Code Analysis: pass, "0 New issues", 0.0% duplication on new code, 0 annotations.
  • package: pass.
  • tests-git-bash: pass.
  • tests: still pending at review time.
  • From reading the code I see no S3776 over 15, no S9073, no S5778 and no S1192 in the new code.

Findings, ranked (none blocks)

1. Low (Windows only; CI does not run it): the git-directory reason names a path with forward slashes. In pin_source_not_top_level, the bare branch and the folder branch wrap git's answer in Path, but the git-directory branch keeps the raw string:

gitdir = _rev_parse(source, "--absolute-git-dir")

On Windows, git prints C:/…/openGlass/.git. So the refusal and check's finding would say --pin-source C:\…\openGlass\.git is not a clone's top level: it is inside C:/…/openGlass/.git. The new test's a-git-directory row compares against str(base / "openGlass/.git"), which has backslashes, so it would fail on Windows. The Git Bash job runs only the launcher and installer suites, so nothing in CI sees it.
Fix: gitdir = Path(_rev_parse(source, "--absolute-git-dir")). This changes no byte on POSIX.

2. Low (named in Known limits): a clone git refuses is called "no clone" and "not a clone's top level".

  • Repro: GIT_TEST_ASSUME_DIFFERENT_OWNER=1 adopt-project.py plan … --pin-source <P>/glass.
  • Main: REFUSED pin-source-unreadable: <P>/glass could not answer … detected dubious ownership ….
  • Merge: REFUSED pin-source-not-a-clone: --pin-source <P>/glass is not a clone's top level: git finds no clone there: … detected dubious ownership …. Its remediation, "pass --pin-source the top level of a clone", points at the path already given.
  • check of such a recorded top level changes the same way, from plan-pin-source-unreadable to plan-pin-source-not-a-clone.
  • Both versions refuse, so no digest is ever wrong. A later fix could treat only git's "not a git repository" answer as "no clone", and let any other git failure fall through to the reader's pin-source-unreadable as on main.

3. Nit: three comments in tests/test_pin_source_unreadable.py still say "the not-a-clone refusal" for #339's old case. That now reads like the new code. The writer listed this too. The replacements are below.

Exact text replacements

The PR title and body are true at the head. I checked:

  • every How claim;
  • the gh order;
  • the test counts (21+7+12+4+1+1+2 = 48);
  • the four revert rows I re-ran;
  • both Known limits.

The body is 14,480 characters and has the Lane line, the Claim line and Closes #800. One note for the landing: the Sourcery bot appended a "## Summary by Sourcery" block after the robot line, so the robot line is no longer the last line. The coordinator may want to strip that block. No sentence of the writer's needs changing.

Optional code text for findings 1 and 3:

  • scripts/shape_materialize.py: gitdir = _rev_parse(source, "--absolute-git-dir") becomes gitdir = Path(_rev_parse(source, "--absolute-git-dir")).
  • tests/test_pin_source_unreadable.py, module docstring: "with the remediation the not-a-clone refusal already" becomes "with the remediation git's own failure (lacks-the-commit) already".
  • Same file, line 60: #: What every refusal is told to do, as the not-a-clone refusal always said it. becomes #: What every refusal is told to do, as git's own failure always said it.
  • Same file, unreadable docstring: "as the not-a-clone refusal always named" becomes "as git's own failure always named".

For the checklist (#457), outside #800

  1. The question's answers are uneven for git directories. None of these gives a wrong digest; each reads the full 1c1c70…, as on main.
    • super/.git/modules/mods/glass, a submodule's git directory with core.worktree, is accepted: git answers false|false| there.
    • glass.git/worktrees/bare-wt, the admin folder of a bare clone's worktree, is accepted as a bare clone.
    • glass/.git and glass.git/refs are refused.
  2. No test holds a worktree of a bare clone (git -C x.git worktree add), a common layout that passes today. Add it to TOP_LEVELS.
  3. A copied .git folder (non-bare, core.bare=false) is refused as "the git directory of a clone, where git finds no top level", with a remediation that does not fit. Main read the full digest there.
  4. With GIT_DIR exported and no GIT_WORK_TREE, any --pin-source, even a plain folder, passes and reads the whole tree of the clone GIT_DIR names, on main and on the merge alike.
  5. A typed symlink is named by its target. For a symlink to glass/sub, the refusal names <P>/glass/sub, because _resolved_pin_source resolves it before anything else runs. The digest is unaffected.
  6. update-shape.py --upstream <path> accepts any folder holding a .git entry or a HEAD file. Its tree_sha256 then calls tree_digest without --full-tree, while files_at passes --full-tree. So a folder of a clone that happens to track a file named HEAD would read a partial digest. This is the same class of bug as adopt-project.py and scaffold-project.py: a --pin-source that is a directory inside a clone records the digest of that directory's subtree alone, or of nothing, so plan, check and the scaffold pass a pin whose tree_sha256 is not the product's tree #800, in another tool.
  7. I confirm the writer's item 1: the root's validate-pins.py --pin-source still digests a folder as if it were the clone.

Lane 1 takes findings 1 and 3 in a small fix round on the same branch; the same reviewer re-checks the new head.

Lane: openreposhape-1 (openRepoShape-1)

… the git-directory reason names the path as a Path, and three comments say git's own failure, so the refusal no longer spells a git directory with forward slashes on Windows, and #339's comments no longer read as the new code (#800)

Review r800's findings 1 and 3, as the lane-1 coordinator took them:
- pin_source_not_top_level's git-directory branch wraps git's --absolute-git-dir in Path, as its bare and folder branches do. On Windows git prints forward slashes; this changes no byte on POSIX.
- tests/test_pin_source_unreadable.py: three comments that said "the not-a-clone refusal" for #339's old case now say "git's own failure".

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-1, 2026-10-17T19:21:22Z) Lane 1's re-check of #842 at 321b526: PR-READY

Linux tests has not finished at this head, so landing still needs it green.

1. The diff a2dc2b67..321b526f is exactly the two fixes. It is one commit, whose only parent is a2dc2b67. It changes 2 files, with 7 insertions and 6 deletions:

  • In scripts/shape_materialize.py the one changed line is now gitdir = Path(_rev_parse(source, "--absolute-git-dir")).
  • In tests/test_pin_source_unreadable.py, the three comments now say "git's own failure" where they said "the not-a-clone refusal". These are the module docstring, the comment on REMEDIATION, and the unreadable docstring. No code or assert changed.
  • scripts/repo_shape.py is still untouched.

2. The printed bytes on Linux are unchanged. I compared the new head with a2dc2b67:

  • pin_source_not_top_level gives byte-identical answers for eight sources. They cover:
    • git directories: glass/.git, a symlink to it, a separate git directory, glass/.git/refs and a folder inside a submodule's git directory;
    • controls: a folder of a bare clone, a folder of a clone, and a directory that is no clone.
  • The scaffold's --dry-run refusals for glass/.git, a separate git directory, and a .git under a folder whose name holds an ESC and a line break: exit 2 each time, with identical stdout and stderr.

3. Tests. I made a local-only merge of 321b526f with current main fc867207. It is clean: the four overlapping files auto-merged with no conflict. On that merge, with git-filter-repo on PATH, a stub gh, and 0 files dirty, I ran test_pin_source_top_level.py, test_pin_source_unreadable.py and test_cli_reference.py. Result: 152 passed in 78.00s, exit 0.

4. CI at 321b526f:

My earlier finding 2 is not part of this commit and stays a known limit: a clone git refuses as "dubious ownership" is called "no clone". Nothing else is open.

Lane: openreposhape-1 (openRepoShape-1)

@sonarqubecloud

Copy link
Copy Markdown

@brettheap
brettheap marked this pull request as ready for review October 18, 2026 00:04
@sourcery-ai

sourcery-ai Bot commented Oct 18, 2026

Copy link
Copy Markdown

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 42 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap merged commit bee84d5 into main Oct 18, 2026
5 checks passed
@brettheap
brettheap deleted the fix/800-pin-source-top-level branch October 18, 2026 00:04
brettheap pushed a commit that referenced this pull request Oct 18, 2026
Both parents have landed (#747 as 6626a3c, #738 as e62b5e1, each merged
above). This brings main up to 7b21059 (#842, #814 and #786's
validate.yml change, whose one sentence in templates/family-root/README.md
sits apart from this PR's). No conflict.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 18, 2026
…trap_after_bump.py` imports both `appended_to_command_scope` and `needs_a_byte_name` (#715)

Main moved from `0dee22b3` to `ca47b71d` while round 3 was paused (#846, #779, #772, #738, #842, #814, #790, #820, #774, #830 and #841). `git merge-tree` of `8b4dbcf6` with it stopped in one path:

- `tests/test_bootstrap_after_bump.py`, its imports. Main's `aec3b38f` (tests append what they tell git to the command scope) added `appended_to_command_scope` to the `from conftest import (...)` list. This branch added `from test_bootstrap_spelling import needs_a_byte_name` below it (#715, round 1). Both are kept.

`scaffold-project.py`, `scripts/bump-leg.py` and `scripts/shape_materialize.py` merged without a conflict.

Lane: openreposhape-4 (openRepoShape-4)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 18, 2026
…take the `--local-remote-dir` that `check_plan` gives, as `test_adopt_pins.py`'s do, so the empty gh log still proves that `check` reads no network for a pin (#798)

Main's #842 (bee84d5) added tests/test_pin_source_top_level.py after
this branch's last merge. Its two `check` runs passed no
`--local-remote-dir` and asserted `gh_calls(gh_env) == ""`; since this
PR, `check` without that flag asks GitHub whether each leg repository
exists, so the stub logged the two views and CI's `tests` failed 11
cases (7 of `test_check_finds_a_recorded_path_that_is_no_clone_s_top_level`,
4 of `test_check_asks_a_recorded_top_level_as_it_always_did`). Both now
run `check_plan(plan, env=gh_env)`, imported from `test_adopt_pins`, as
r798's re-verification gave it. No assertion changed. The file gives
48 passed.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 18, 2026
…disk or on GitHub, before it makes either, and `check` asks the same, so a code leg that already exists no longer leaves the spec leg made, and a `--local-remote-dir` that cannot hold them no longer ends in a traceback after `plan ok` (#798) (#861)

`adopt-project.py execute` made the spec leg's repository first and asked about the code leg's only when it made it (#798). Measured on main `bf1f099b`:

- **L1-73.** With the code leg's bare repository already under `--local-remote-dir`, or its name taken on GitHub, `check` said `plan ok`, and `execute` made the spec leg, then was refused at the code leg with `NOTHING has been rolled back` naming the leg it had just made.
- **L1-72.** `check --local-remote-dir` said `plan ok` for a directory `execute` could not use: a file at the path ended in a `FileExistsError` traceback, a folder above it this user cannot write in in a `PermissionError` traceback, and a folder this user cannot write in in `a git or gh command failed`.

Now:

- **`execute` asks about both leg repositories before it makes either,** right before `_create_leg_remotes`, where `leg-remote-exists` was always refused, and refuses everything in their way at once, saying that no leg repository was created and nothing was pushed.
  - Under `--local-remote-dir` it asks whether each bare repository path is taken (`os.path.lexists`, so a dangling link counts) and whether the directory can be made and written in, by #609's question (`folder_in_the_way`, split out of `folder_cannot_be_made` in `scripts/shape_materialize.py`, which makes nothing). Each case is refused by name, as `leg-remote-exists` or `adopt-folder-cannot-be-made`, with no traceback.
  - Against GitHub it runs `gh repo view <org>/<P>-spec --json nameWithOwner`, then the code leg's. gh's not-found answer means absent. The name is taken only when the answer's `nameWithOwner` is the name asked, compared without case, so a name GitHub only redirects (a rename or a transfer) is free. Any other failure, or an answer with no `nameWithOwner` to read, is `adopt-remote-unreadable`, naming the command and what gh said.
  - The `mkdir` of `--local-remote-dir` after the question refuses by name too (`_make_the_remote_dir`).
- **`check` reports the same findings,** last among its findings, as `execute` asks them last. Given `--local-remote-dir` it reads no network; without it, it asks GitHub the same two views, and a gh failure is a NOTE there, so `check` stays usable offline.
- **#188 N4's notice stays,** for what can still fail after the first create (a private name the token cannot see, a rate limit, Ctrl-C, a full disk), and now says that a re-run is refused as `leg-remote-exists` before either leg is made.
- **`tests/gh_guard.py`** answers adopt's exact `--json nameWithOwner` view as a `gh` with no login does, so every `check` in the suite given no `--local-remote-dir` prints the NOTE and passes; scaffold's and family's `--json name` view is still refused. The tests whose own gh stubs or gh-log assertions meet the new call pass `--local-remote-dir` or answer the view; among them is #842's `tests/test_pin_source_top_level.py`, whose two `check` runs now use `check_plan`.
- The module docstring, both `--local-remote-dir` helps and docs/cli.md say so.

Review: lane 1's independent adversarial review found defects at `99cc97e8`. Fix round 1 (`748c8e6a`) fixed them and merged main; fix round 2 (`70801c91`) cleared SonarCloud's four issues in the new tests; the merge round (`79d518db`) merged main over #825's `cmd_check`, as lane 3 ruled on #473, and took #842's new test into account. The re-check found that head PR-READY.

It lands by fast refresh (Brett Heap, 2026-10-15T22:48:35Z). Its reviewed head is green on Linux and SonarCloud. The branch merged main four times and was never rebased; the last merge is update-branch's, clean, giving 25226ab, which is green on SonarCloud and Git Bash. The full suite passed on its tree in lane 1's gate: 244 test files, 15,218 passed and 48 skipped.

Closes #798

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.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

1 participant