Skip to content

tests: 149 tests in four files run the operator's real gh (407 calls to gh --version and gh auth status, and gh api user besides with a login) through the doctor's machine row and the installed command's machine block, so a suite whose conftest says "NO NETWORK AND NO GITHUB" reaches the host on a logged-in machine, and the one gh stub the doctor tests have is called by that row without the test seeing it #296

Closed

Description

@brettheap

Lane: openreposhape-4 (openRepoShape-4)

Found by this lane's adversarial review of the #272 fix (PR #274, head d7d6237; its focus (c) and out-of-scope item O1, which counted 133 tests and 347 calls on main c3f4987), and re-counted on main d94d155 (#232's squash) with a gh shim that logs its argv and $PYTEST_CURRENT_TEST and exits 97, over the same four files: 149 tests and 407 calls (the doctor's test file gained tests in between). It is pre-existing and not caused by any open branch. Not claimed: what a real login costs (the shim stops each run at gh auth status, so no call reached GitHub and gh api user was never run here; that the real calls are network calls is the doctor's own statement, shape-doctor.py:3491-3495, not something observed), how many of the 149 would change with no gh on PATH at all, and Windows (decision 4).

What is wrong. Every doctor run ends in the machine row: it is registered for every root kind (shape-doctor.py:3862) and no flag leaves it out (--root, --json and --placement-plan are the only options). It runs setup-project.py --preflight as a child (:3508), and that runs gh --version (setup-project.py:1001), gh auth status (:1002) and, when the login answers, gh api user (_gh_login_suffix, :973-979, called at :1006). The installed command's --install ends with machine_report, which runs gh auth status (openRepoShape:226-236). The tests run both through the PATH they inherit: doctor() is plain run_script (tests/test_shape_doctor.py:152, tests/conftest.py:167) and nothing puts a gh stub in front of it.

# $U: a clean checkout at d94d155. $SHIM: a directory whose `gh` appends its argv, $PYTEST_CURRENT_TEST and its parent's command line to $LOG and exits 97.
# The git environment is isolated (an empty GIT_CONFIG_GLOBAL, GIT_CONFIG_NOSYSTEM=1, a test identity) so nothing reaches GitHub through git either.
cd $U && PATH=$SHIM:$PATH python3 -m pytest -q \
    tests/test_shape_doctor.py tests/test_shape_advisory.py tests/test_openreposhape_command.py tests/test_stdout_encoding.py
#   340 passed, 1 skipped in 217.64s
wc -l < $LOG                                          # 407
cut -f2 $LOG | sort | uniq -c                         #   200 --version      207 auth status
# distinct tests per file (the test id is the second column):
#     115 tests/test_shape_doctor.py    28 tests/test_shape_advisory.py    5 tests/test_openreposhape_command.py    1 tests/test_stdout_encoding.py
# who called: 400 from `python3 <standard>/setup-project.py --preflight` (the doctor's machine row; the full command line was
# checked on two of the tests, the rest of the column is cut inside the path), 7 from `bash <standard>/openRepoShape --install`
# (five tests of test_openreposhape_command.py), which only ever asks `auth status`.

The one stub the doctor tests do have cannot see these calls. refusing_network (tests/test_shape_doctor.py:1531) puts a gh that prints "fake gh: no test here may reach the network" and exits 1 first on PATH, and four tests use it through shim_env (:1545); one of them asserts the sentence is NOT in the stderr of the run (:1576). The machine row calls that stub and the sentence never reaches stderr: a logging stand-in of the same shape, run through openRepoShape --doctor over a scaffolded project, was called by the machine row for --version and for auth status (a third call, a gh api from that project's own pins row, is the separate issue named below) and the sentence's count in stderr was 0, because the preflight captures gh's output (run(["gh", "auth", "status"], capture=True), setup-project.py:1002). The assertion cannot fail on these calls.

Why. tests/conftest.py:4-7 says "NO NETWORK AND NO GITHUB ... if a test ever needs gh, it is the wrong test". The doctor documents the machine row as its one exception, and rightly for the command: "THIS IS THE ONE ROW THAT CAN REACH A NETWORK" (shape-doctor.py:3491; also :27-33), three questions "gh answers by talking to its host". The tests inherit that exception by running the same command, and nothing in the suite stands between the command and the operator's gh. So a run of these four files makes 407 calls to whatever gh is on PATH, and with a login each is a call gh makes to its host with the operator's token. #272 added no guard (its decision 2, a failing gh first on PATH, was optional and was not taken); #274 fixed the two tests that reached gh api, and this class is the one the review found it did not cover.

What it should do. No test reaches the operator's gh unnoticed, and the machine row is still tested. The decisions are the author's:

  1. Where a guard goes. An autouse session fixture in tests/conftest.py that puts a directory holding a failing gh first on os.environ["PATH"] (every child inherits it, whether it started through run_script or subprocess.run: the five installed-command tests go through run_cmd, a helper in tests/test_openreposhape_command.py:122 that calls subprocess.run(["bash", ...]) directly), or run_script alone (it covers only its callers). A test that stubs gh already puts its own directory ahead of PATH, so it keeps winning; the ones to check are tests/test_neutral_pin_validator.py's deliberate gh test, tests/test_setup_sh.py and tests/test_openreposhape_command.py's offline_github.
  2. What the guard does when called. A stub that only prints is invisible to the preflight (shown above), so it should append to a log the fixture reads at teardown and fail the test, and a test that means to run the machine row says so (a marker, or a fixture that hands it a stub answering what it is asked). Whether refusing_network becomes that guard (it exists, but it is POSIX only: a #!/bin/sh file, and its users carry WINDOWS_SKIP) or is replaced by it.
  3. A seam instead of a guard, or as well. The doctor gets a way to leave the machine row out that the suite passes by default, and ONE test runs the row against a stub gh, so the row stays covered. That changes a CLI surface: docs/cli.md is regenerated by scripts/render-cli-reference.py and AGENTS.md's doctor section (item 6) describes the row.
  4. Windows. A stub file named gh is not found there. Whether subprocess.run(["gh", ...]) finds a gh.cmd or gh.bat or needs a real gh.exe, and what shutil.which("gh") (setup-project.py:987) does with PATHEXT, was not checked here (no Windows runner); 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's review noted the same ("a Windows shim needs PATHEXT"). CI's windows job is the check.

How to test. The command above with the guard in: the log is empty, and a test that runs a bare gh with no stub fails by name. Run the whole suite under the guard once, because the survey here covers four files: the other files the #272 review ran under the same kind of shim (seven files, 438 passed, 0 shim lines) are that review's result and were not repeated here, and a full run takes 12-48 minutes on a loaded workstation.

Related: #272 (its decision 2 and its review's O1; PR #274), #279 (the same file's identity hygiene: its decision 2 is an autouse session fixture isolating git's config, and one fixture could carry both), #126 (closed: run_script blanking every inherited SHAPE_PIN_SOURCE_*, the same kind of hygiene), and #297 (the doctor's pins row reaching gh api from a project's validator: that one is the command's behaviour, this one is the tests').

Activity

  1. brettheap commented on Oct 12, 2026

    @brettheap
    ContributorAuthor

    CLAIMED — lane openRepoShape-4, session 3431c370-df80-4c11-a041-41bce42d8b38@Eagle, 2026-10-12T11:04:46Z, for #296

    Logged in lanes/log/openRepoShape-4.md at d678c92e74752c65267ce13399ada478a174b814.

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

    1. existing `CLAIMED —` comments on opensoft/openRepoShape#296:
       none
    2. `gh pr list --repo opensoft/openRepoShape --state all --search "#296"`, kept where the row or its body names `#296`:
    310	tests/conftest.py: `git()` runs git under a default identity given as `-c` config, never as environment, so the seven tests that commit through it with none of their own no longer die with `Author identity unknown` where git cannot work one out, and no call site's own `-c user.name` or exported identity is overridden (#279)	fix/tests-git-identity	OPEN
    3. `git ls-remote --heads git@github.com:opensoft/openRepoShape` branches naming `296` 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