Skip to content

No test pins release.yml's ref step to fetch the tag without --depth, so --depth=1 there passes every test #287

Closed
opensoft/openRepoTools-code
#10

Description

@brettheap

Found by lane openRepoTools-2's independent re-check of #284 (PR 3b of #228), its comment 6284132726, NIT 5 (marked "Trivial"). Lane 2's own weighting: harmless today.

Line numbers are .github/workflows/release.yml at 7376b34, as lane 2 gives them. That file is blob 9a27e2e at 7376b34, at #284's head 2ff7094 and in tree 7ad2bbe (the tree main holds once #284 lands), so the lines are #284's and, once it lands, main's. test :N is tests/test_release_workflow.py at 2ff7094 (blob 8273f40, also in 7ad2bbe).

What happens

  • The ref step fetches git's copy of the release tag again: git fetch --quiet --no-tags origin "+refs/tags/$tag:refs/tags/$tag" (:409). Its comment (:377-380) says why there is no depth: the checkout holds full history, and a depth "would cut that history off at this commit" (the header says it too, :25-28).
  • No test holds that. The test file pins the checkout's fetch-depth as 0 (test :977) but has no --depth, no shallow and no git fetch; the tag cases (test :1412-1446) check the exit status, the success line and the tag's object. Lane 2: --depth=1 there passes every test.
  • Harmless today (lane 2). A grep finds no git command in release.yml after :409 that reads history: :461, :466, :675, :770, :827 and :843 read the run's commit or tree, as does the pinned tree_digest, and the branch step asks the forge (:438).

Where

  • release.yml:409 and its comment at :377-380. The tag cases run against a file:// origin (tagged_origin, test :715-732).

How it was found

The fix

  • Lane 2 proposed none; its NIT names the gap.
  • What a fix has to settle (nothing here is decided):
    • whether a test holds the fetch line's text (no --depth) or its effect (the checkout is not shallow afterwards);
    • for the effect, a fixture with history to cut: tagged_origin makes one parentless commit (test :725) and an empty checkout.

Why it is not in v1.1.0

Filed in milestone v1.2.0 on Brett Heap's word (2026-10-20T09:40Z, "yes, file the four NITs in v1.2.0"). It is unclaimed.

Lane: openRepoTools-3

🤖 Generated with Claude Code

Activity

  1. added this to the v1.2.0 milestone on Oct 20, 2026
  2. brettheap commented on Oct 20, 2026

    @brettheap
    ContributorAuthor

    CLAIMED — lane openRepoTools-2, session 202778d2-0e63-4478-9c95-fdde314b0524@Eagle, 2026-10-20T23:00:37Z, for #287

    Logged in lanes/log/openRepoTools-2.md at b14a846305324f99c00f8ccdbbc0572133b036eb.

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

    1. existing `CLAIMED —` comments on opensoft/openRepoTools#287:
       none
    2. `gh pr list --repo opensoft/openRepoTools --state all --search "#287"`, kept where the row or its body names `#287`:
    284	PR 3's re-check survivors are killed: tests/test_release_workflow.py runs the fetch, pipx, approval, tag, ref, clean, stamp, dist and digest steps instead of reading them, and release.yml changes only in comments (#228, PR 3b)	006-release-pypi-pr3b	MERGED
    3. `git ls-remote --heads git@github.com:opensoft/openRepoTools` branches naming `287` as a whole token:
       none
    
  3. brettheap commented on Oct 21, 2026

    @brettheap
    ContributorAuthor

    Lane openRepoTools-2, 2026-10-21T02:2xZ: #287 is done, on a branch only. It is branch test/287-ref-fetch-no-depth-lane2 at 15f5e2b, one commit on main a42946e, to be cherry-picked into openRepoTools-code after v1.1.0 is tagged.

    • The new test is test_release_build_the_ref_step_fetches_the_tag_again_without_cutting_the_checkout_s_history.
      • Its origin has four commits, and the checkout is what fetch-depth: 0 leaves.
      • The ref step must exit 0 and leave the checkout whole: not shallow, with four commits.
      • Its one git fetch line must name no --depth, --deepen, --shallow-since or --shallow-exclude.
    • The proof: with --depth=1 the test fails at the shallow check. With --depth=100, which leaves a short history whole, it fails at the text check. Without either, it passes.
    • -k test_release_workflow passes 500.

    Lane: openRepoTools-2

  4. brettheap commented on Oct 21, 2026

    @brettheap
    ContributorAuthor

    Lane openRepoTools-2, 2026-10-21T14:5xZ: #287's review fixes are on the branch. Branch test/287-ref-fetch-no-depth-lane2 is at 132e478, one new commit on 15f5e2b (a fast-forward push, no force). It is still a branch only, to be cherry-picked into openRepoTools-code after v1.1.0 is tagged.

    • RV-A's two NITs, both in docstrings, and nothing else changes: the file's syntax tree is the same with its docstrings blanked, so no test is added, removed or changed.
      • tagged_origin's docstring now says in plain sentences that its commits are in a line, the first with no parent and each later one the child of the one before it, and that history is 1 unless a case asks for more.
      • The ref-step test's docstring now says that a depth beyond the four commits that the fetch line takes from a variable would pass both checks, that this is contrived, and that no case holds it. No test is added for it.
    • A correction to the NIT's boundary: RV-A wrote "a depth of 4 or more". On a four-commit history git leaves the checkout shallow at --depth=4 (shallow=true commits=4), which the shallow check refuses, and whole from --depth=5. So the docstring says "beyond the four commits". Run through the harness, with limit="--depth=N" and git fetch $limit origin ..., on -k ref_step: 100 gives 23 passed, 4 gives 1 failed, 22 passed (the ref-step test fails at its shallow check, "the step left the checkout shallow"), and 5 gives 23 passed.
    • -k test_release_workflow on the tree at 132e478: 500 passed, 2434 deselected.
    • Still open: nothing from RV-A's findings. The variable-depth gap is left open on purpose, as RV-A called it contrived.

    Lane: openRepoTools-2

  5. added a commit that references this issue on Oct 21, 2026
  6. brettheap commented on Oct 21, 2026

    @brettheap
    ContributorAuthor

    Lane openRepoTools-2, 2026-10-21T16:2xZ: the final review of #287 is READY, with no MUST and no SHOULD. It was an opus pass, on Brett Heap's word.

    • Head: 43891a2. It holds the branch's own commits 15f5e2b and 132e478, plus a merge of main 33ce26d that carries only main's delta.
    • 132e478 changes docstrings only. The syntax tree with docstrings blanked is identical, with 80 test functions before and after.
    • The corrected boundary holds:
      • --depth=1 is killed;
      • depth 4 through a variable is killed, because it leaves the checkout shallow;
      • depth 5 through a variable passes. That is the documented gap, which is left open on purpose and stated in the test's docstring.
    • CI: run 39622352896 on 43891a2 has all four jobs green.
    • One optional NIT: the docstring sentence at tests/test_release_workflow.py:1470-1472 can be read with "that the line takes from a variable" describing the commits. Lane 2 rewords it in a small follow-up commit.

    Lane: openRepoTools-2

  7. brettheap commented on Oct 21, 2026

    @brettheap
    ContributorAuthor

    Lane openRepoTools-2, 2026-10-21T16:3xZ: the final review's optional NIT (comment 6321040376) is fixed in e14c2f7, one docstring-only commit on top of 43891a2. The sentence at tests/test_release_workflow.py:1470-1472 now reads "A depth that the fetch line takes from a variable, and that is beyond the four commits, would pass both checks", so the variable qualifies the depth and not the commits; the meaning is unchanged (depth 5 through a variable passes both checks, depth 4 is refused as shallow). No mutant proof applies, because no code changed: the syntax tree with docstrings blanked is identical before and after (80 test functions each, and the only docstring whose text differs is this test's), and -k ref_step gives 23 passed. CI is dispatched as run 39636430608 on e14c2f7. Still open, on purpose: no case holds a depth beyond the four commits taken from a variable, as the review accepted and the docstring says.

    Lane: openRepoTools-2

  8. modified the milestones: v1.2.0, v1.2.1 on Nov 8, 2026
  9. brettheap commented on Nov 9, 2026

    @brettheap
    ContributorAuthor

    RELEASED — lane openRepoTools-2, session 202778d2-0e63-4478-9c95-fdde314b0524@Eagle, 2026-11-09T23:28:07Z, for #287 — W2 landed it as opensoft/openRepoTools-code#10 merge 8400ddc and the issue is closed

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

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions