Skip to content

tests/test_scaffold_pin_and_reuse.py: two tests run the scaffolded project's validate-pins.py with no --pin-source, so it calls gh api repos/opensoft/<product>/git/trees/<sha> whenever gh is on PATH and the pin's digest is skipped instead of rechecked #272

Closed

Description

@brettheap

Lane: openreposhape-4 (openRepoShape-4)

Found by this lane's adversarial review of the #241 fix branch (PR #256, head a25c2e1; its out-of-scope item O1), which logged the calls with a gh shim tagged by PYTEST_CURRENT_TEST. Re-verified on main 39ac59c, and the calling process identified (below), which the review did not do. It is pre-existing and not caused by any open branch; the cited test and validator lines read the same on main d89fffa. Not claimed.

What is wrong. tests/conftest.py:4-7 says "NO NETWORK AND NO GITHUB ... if a test ever needs gh, it is the wrong test". Two tests start a real gh api whenever a gh is on PATH:

  • tests/test_scaffold_pin_and_reuse.py::test_a_declared_descendant_scaffolds_with_legs (openGlass)
  • tests/test_scaffold_pin_and_reuse.py::test_a_descendant_that_pins_a_layer_scaffolds_through_the_chain (openXdox)

Both still pass under a gh that exits 97, because the caller turns a failed read into a SKIP, so the call is tolerated and nothing flags it. These tests only read, and gh api repos/opensoft/<product>/git/trees/<sha> for a stand-in's local commit is a 404 on a real login, so nothing is created or changed. But the tests depend on the operator's gh login and network, take a round trip each, and the digest they appear to check is never checked. AGENTS.md's rule (AGENTS.md:464) is "Never test by creating a real GitHub repository"; this is the read-only neighbour of that rule.

The call is NOT in scaffold-project.py. Both tests pass --pin-source, so the scaffold reads the stand-in's tree locally (declared_pin_values, scaffold-project.py:337-340, takes the pin_digest_from_source branch; its gh branch, pin_digest_from_gh, is not reached): the shim log is empty after the scaffold step. The caller is the SCAFFOLDED PROJECT'S OWN scripts/validate-pins.py, which each test runs afterwards, with no arguments, in the loops at tests/test_scaffold_pin_and_reuse.py:173-178 and :287-297 (("validate-pins.py", []), lines 176 and 290).

# $U: a clean checkout at 39ac59c. gh is a PATH shim that logs argv and its parent's command line, then exits 97.
# $G: a local stand-in `openGlass` (one commit), $SHA that commit. Same flags as the first test.
python3 $U/scaffold-project.py --org MedxSoft --project MedxGlass --elected-by 'Brett Heap' --elected-on 2026-09-02 \
    --pin openGlass@$SHA --pin-source $G --local-remote-dir $R/remotes --work-dir $R/work
#   exit 0; gh shim log: 0 lines
cd $R/work/MedxGlass
python3 scripts/validate-pins.py          # what the test runs
#   ok  spec: ... ok  code: ... ok  contracts/shape-pin.yaml: 11 copied shape file(s) match their digests
#   skip openGlass: pin commit 3165d184ef05 digest NOT rechecked — no local checkout (gh-unreadable: `gh api` could not read opensoft/openGlass @ 3165d184ef05a95331d61febfae8ea22e3367124: gh shim: refused). A --pin-source, SHAPE_PIN_SOURCE_OPENGLASS, or a checkout at ../openGlass beside this assembly root would verify it offline.
#   pins ok          exit 0
#   gh shim log: api repos/opensoft/openGlass/git/trees/3165d184...?recursive=1   ppid-cmd=python3 scripts/validate-pins.py
python3 scripts/validate-pins.py --pin-source openGlass=$G
#   ok  openGlass: pin commit 3165d184ef05 digest recomputes (--pin-source openGlass=...)
#   pins ok          exit 0, gh shim log: 0 lines

(Run under pytest, pytest tests/test_scaffold_pin_and_reuse.py::test_a_declared_descendant_scaffolds_with_legs with that shim first on PATH logs the same line, with ppid-cmd=/usr/bin/python3 .../work/MedxGlass/scripts/validate-pins.py. The second test's call, api repos/opensoft/openXdox/git/trees/<sha>?recursive=1, is the review's log line; the second test was not re-run for this issue.)

Why. validate-pins.py resolves a neutral pin's source in resolve_neutral_pin_source (templates/assembly-root/scripts/validate-pins.py:350-385): --pin-source <product>=<path>, then SHAPE_PIN_SOURCE_<PRODUCT>, then a checkout beside the assembly root named for the pinned repository (../openGlass), then a bare --pin-source. When none answers, _recompute_neutral_pin_digest (:468) falls to tree_digest_from_gh(repository, commit) (:494), whose subprocess.run(["gh", "api", ...]) is scripts/repo_shape.py:805-806; a failure there is reported as skip and exits 0 (:495-502). The tests pass none of the four: run_script blanks every SHAPE_PIN_SOURCE_* the caller did not name (tests/conftest.py:197-213), the stand-in sits at tmp_path/<product> and not beside work/<Project>, and the loop passes []. A third test in the same file already does it right: test_a_neutral_product_root_may_also_pin_a_neutral_product passes ["--pin-source", f"openDox={product}"] (tests/test_scaffold_pin_and_reuse.py:526-527). tests/test_neutral_pin_validator.py:164-189 is the one test meant to exercise the gh fallback, and it stubs gh first.

What it should do. No test reaches GitHub, and the pin digest in these two tests is really rechecked. The decisions are the author's:

  1. What the two tests pass. The minimal change is the third test's own precedent: ["--pin-source", f"openGlass={product}"] for the first and ["--pin-source", f"openXdox={layer}"] for the second, on the validate-pins.py row only (the other two validators are unchanged; the second test's "declared-unverified" assertion is on those two and does not move). That turns the skip into ok ... digest recomputes (shown above for the first; the second spelling was not run).
  2. Whether to add a guard so a future test cannot reach gh unnoticed, for example run_script (or an autouse fixture) putting a failing gh first on PATH, or asserting no skip in the output of validate-pins.py where a source was given. Optional; the one test that needs gh already stubs it.

How to test. Put a shim gh that logs and exits 97 first on PATH, run the two tests (/usr/bin/python3 -m pytest tests/test_scaffold_pin_and_reuse.py -q -k "declared_descendant_scaffolds_with_legs or pins_a_layer_scaffolds_through_the_chain"), and assert the log is empty. Before the change the log has two lines; after it, none. Also assert validate-pins.py's stdout holds digest recomputes for the pinned product and no skip, so the tests fail if the source stops being found.

Related: #241 (PR #256, whose review found it), tests/test_neutral_pin_validator.py (the stubbed-gh precedent).

Activity

  1. brettheap commented on Oct 11, 2026

    @brettheap
    ContributorAuthor

    CLAIMED — lane openRepoShape-4, session 3431c370-df80-4c11-a041-41bce42d8b38@Eagle, 2026-10-11T23:16:07Z, for #272

    Logged in lanes/log/openRepoShape-4.md at 9723bf013c354fff4ce8fc561afd9a60d4a6c0a3.

    The three reads (lane-collision-protocol Rule 1):

    1. existing `CLAIMED —` comments on opensoft/openRepoShape#272:
       none
    2. `gh pr list --repo opensoft/openRepoShape --state all --search "#272"`, kept where the row or its body names `#272`:
       none
    3. `git ls-remote --heads git@github.com:opensoft/openRepoShape` branches naming `272` as a whole token:
       none
    
  2. added 3 commits that reference this issue on Oct 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions