Skip to content

The hygiene check reads every exit-3 die of lanes-edit.sh against its cause and both documented rows, so a die that claims nothing was pushed or a fourth die no row documents fails it (#68 item 5) - #259

Merged
brettheap merged 2 commits into
mainfrom
fix/hygiene-reads-every-exit-3-die-68
Oct 19, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Part of #68 (item 5 only). Items 1-4 wait on #214, which owns lanes-edit.sh and docs/README-lanes.md here, so this PR changes neither.

What was wrong

tests/test_repo_hygiene.py::test_the_exit_3_documentation_agrees_with_the_die_message_it_describes read ONE of the three die "…" 3 in lanes-edit.sh: the rebase conflict's (:2192), found by its own words. It never read the other two:

  • git_timeout_die's own message (:1894);
  • commit_push's six-attempts message (:2125, the branch that skips the pull because a peer's uncommitted file blocks it).

The two rows (the exit-code table in lanes-edit.sh and docs/README-lanes.md's copy) were checked only for the words timeout, push, pull and peer's. For the code's table the "row" it was handed was the whole table text, so another row's push satisfied it. So either die could say anything, "nothing was pushed" included where a push may already have landed, and the test stayed green.

Measured on scratch copies of the real files (below): 9 of 15 mutations left the old test green.

What changed

Only tests/test_repo_hygiene.py. The rule is now a function over text, _exit_3_findings(src, manual=None), and the existing test calls it (code half first, then the manual's, so a standalone code leg still asserts its half before the spec leg's absence skips, #186 T009).

  • It collects EVERY die "…" 3 in lanes-edit.sh. There are exactly three today. A die … 3 that no documented cause explains is a finding, and so is one the extractor cannot read (a message holding a command substitution with quotes of its own): a count of lines ending … 3 after a closing quote (one or more spaces or tabs before the 3) is compared with what was extracted, so a die of that shape the extractor cannot read is a finding. What stays out of its reach is a die whose exit code is not a literal 3 on the line that closes its message: the code held in a variable, or a 3 on a backslash-continued line of its own.
  • It ties each die to its cause by WHERE IT SITS, never by its words: the function git_timeout_die; the branch after the dirty_elsewhere guard; the branch after the git_net … pull call (the rebase conflict's die is in its else arm, reached when that call fails). PR #50 round 5 (Copilot 5203628254): commit_push's recovery block leaves $LANES_REPO unquoted, git cherry unbounded for multi-commit checkouts, and git_timeout_die still push-shaped for a pull timeout #68 item 3 and the act 2 commit_publish will reword these messages, so the anchor survives a rewording that keeps the meaning. The marker is a call and not a word, because the peer branch's own note says "skipping 'pull --rebase'" a few lines above its die.
  • It holds each die, and the exit-3 ROW of the table and of the manual (the row, no longer the whole text), to the same three causes. Each must name its cause, with a concept loose enough for rewording ("timed out", "timeout" and "time-out" are one), and none may claim nothing reached origin. "Not pushed by this attempt" is the one true form of that claim and the one form let through; the rebase conflict's die and both rows must keep it.
  • The network operations git_timeout_die is called for are read off its call sites (today a push and a pull), so a timeout on a fetch that neither row names is a finding. A call may hand it the operation as a quoted command (git_timeout_die "git push origin $B") or as a bare or quoted word (git_timeout_die push "git push origin $B", a shape PR #50 round 5 (Copilot 5203628254): commit_push's recovery block leaves $LANES_REPO unquoted, git cherry unbounded for multi-commit checkouts, and git_timeout_die still push-shaped for a pull timeout #68 item 3 may use). Every mention of the name but its definition is read as a call; one whose shape the reader cannot place is a finding that says exactly that, never "nothing calls it".
  • The docstring's stale line numbers (lanes-edit.sh:1166, :1112/1141/1160) are replaced by the functions and branches they meant. The three current lines are named "at main 08e7550".

Functions and regions edited

  • tests/test_repo_hygiene.py: the import block (adds from collections import namedtuple).
  • tests/test_repo_hygiene.py: new _Exit3Cause and _EXIT_3_CAUSES, _DIE_EXIT_3, _ENDS_IN_EXIT_3, _NOTHING_REACHED_ORIGIN, _function_around, _exit_3_cause_of, _GIT_TIMEOUT_MENTION, _AT_COMMAND_START, _timeout_operations, _exit_3_dies, _flattened, _exit_3_row_of_the_table, _exit_3_row_of_the_manual, _exit_3_row_findings, _exit_3_findings, placed between test_the_exit_code_table_carries_every_code_the_file_exits_with and the exit-3 test.
  • tests/test_repo_hygiene.py: test_the_exit_3_documentation_agrees_with_the_die_message_it_describes (docstring and body, same name: specs/004-migrate-to-triad/test-roots-and-ci-2026-10-08.md maps it to the manual's root and that mapping still holds).
  • tests/test_repo_hygiene.py: new EXIT_3_SOURCE, EXIT_3_MANUAL, _a_fourth_die, EXIT_3_REFUSED, EXIT_3_ACCEPTED, _exit_3_fixture and three tests, directly after the exit-3 test.

Not touched: lanes-edit.sh, docs/README-lanes.md, specs/004-migrate-to-triad/, contracts/openreposhape-pin.yaml, upstream/. None of lane 4's and lane 1's open PRs (#202, #207, #210, #211, #214, #217, #219, #230, #246, #252, read with gh pr diff) changes tests/test_repo_hygiene.py. Lane 3's #262 does: it adds import ast and import sys to this file's import block, and this PR adds from collections import namedtuple in the same spot, so merging #262 with #259 conflicts at line 25. Resolve it by keeping all the imports.

Tests

Added to tests/test_repo_hygiene.py (a fixture shaped like the three die sites and the two rows, with the decoys that fool simpler rules: a die … 3 in a comment, and the skipping 'pull --rebase' note):

  • test_the_exit_3_rule_is_green_on_the_fixture_it_is_mutated_from;
  • test_the_exit_3_rule_is_red_on_a_die_or_a_row_that_disagrees: 22 cases (including a bare operation word no row names, and a mention of git_timeout_die the reader cannot place as a call), each one planted disagreement, and each must be refused in the words of the check that should refuse it, so a case cannot pass for the wrong reason;
  • test_the_exit_3_rule_is_green_on_a_rewording_that_keeps_the_meaning: 9 cases (timeout die reworded, peer's-file die saying "dirty", conflict die's clauses moved, commit_push renamed, a call handing git_timeout_die the operation as a bare word, two spaces before the exit code, a trailing comment after the exit code, the attempt-scoped phrase wrapped onto a continuation line, the manual spelling "time-out").

Fail before, pass after

The gap is in the test, so "before" is the old test run against mutated copies of the REAL lanes-edit.sh and manual, loaded from git show 08e7550:tests/test_repo_hygiene.py, beside the new one (scratch script, not shipped; each mutation staged in its own temp directory, the real files untouched):

mutation old test new test
timeout die says "nothing was pushed" green RED
six-attempts die says "Nothing was pushed" green RED
conflict die says "nothing was pushed" (the one die the old test read) RED RED
timeout die no longer says it timed out green RED
six-attempts die no longer names the peer's file green RED
a fourth die … 3 in refuse_dirty_checkout green RED
a fourth die … 3 whose message holds $(printf "%s" "x") green RED
a git_timeout_die call site now names a fetch green RED
the table's row says "network failure" for "network timeout" green RED
the table's row drops the peer's uncommitted file green RED
the table's row drops the pull RED RED
the manual's row drops the pull RED RED
the manual's row drops the peer's uncommitted file RED RED
the manual's row says "nothing was pushed" RED RED
the manual's row says "network failure" for "network timeout" RED RED

Unmutated real tree: old green, new green. The new test is red on all 15 (rerun at adb6940, same result).

Run only the hygiene file, through the lock, with every LANES_* seam and TMUX unset (env | grep '^LANES_\|^TMUX' prints nothing inside it):

flock "${TMPDIR:-/tmp}/openrepotools-pytest.lock" python3 -m pytest tests/test_repo_hygiene.py -q -p no:cacheprovider
235 passed in 2.00s        (202 existing + the 33 new), at adb6940

python3 -m py_compile tests/test_repo_hygiene.py and git diff --check are clean. No bash file changed, so no bash -n applies.

Forward check: the new rule on the head lanes-edit.sh and docs/README-lanes.md of #214, #219, #230, #246 and #252 (the open PRs that change either file) reports 0 findings on each, so none of them trips it when they land.

The full suite was not run locally (CI's tests job is the suite of record).

Fix round 1 (adb6940, after the independent review of 25ae36d said LAND)

A new commit on the same branch, tests/test_repo_hygiene.py only:

  1. The reader for git_timeout_die's calls saw only a double-quoted first argument, so a call git_timeout_die push "git push …" (a shape PR #50 round 5 (Copilot 5203628254): commit_push's recovery block leaves $LANES_REPO unquoted, git cherry unbounded for multi-commit checkouts, and git_timeout_die still push-shaped for a pull timeout #68 item 3 may use) would have made the rule report "nothing calls git_timeout_die". It now reads every mention of the name but its definition as a call, parses the words after it on its line, and takes the operation from a quoted command, a bare word or a quoted word. A mention it cannot place as a call (not the first word of a command) is a finding that says exactly that. New green case for the bare-word call, new red cases for a bare word no row names (git_timeout_die fetch "…") and for an unplaceable mention.
  2. _ENDS_IN_EXIT_3 (and _DIE_EXIT_3 with it, so the two stay consistent) take spaces or tabs before the 3, not exactly one. New green case: two spaces before the exit code. The "never a hole" wording is gone: the comment now names what stays out of reach (an exit code held in a variable, or a 3 on a backslash-continued line of its own).
  3. Wording only: the rebase conflict's anchor is "after the git_net … pull call", not "after a failed pull" (the comment on _EXIT_3_CAUSES, the test's docstring, and this body).
  4. Trims: the two manual-row fixture cases that repeated table cases (manual loses the pull; manual loses the peer's-file cause) are dropped, since both rows go through the same _exit_3_row_findings, and the ROUND 5 bullets in the test's docstring that restated the comment block at _EXIT_3_CAUSES are now a pointer to it.
  5. This body: the overlap with lane 3's openRepoTools becomes the openrepotools package: pipx installs the wheel with the whole payload, --install leaves pipx's link to pipx, --version names the release, and every count line says where the other commands come from (#228, PR 1 of 4) #262 is stated above (both imports kept), and the counts are updated: 22 red and 9 green cases plus the baseline, 33 new tests.

Local, at adb6940, with the lock and every LANES_* seam and TMUX unset: python3 -m pytest tests/test_repo_hygiene.py gave 235 passed. The scratch 15-mutation run on the real files and the forward check on #214, #219, #230, #246 and #252 were rerun with the same results (new test red on all 15; 0 findings on each PR's head).

🤖 Generated with Claude Code

Summary by Sourcery

Make the exit-3 hygiene validation cover every failure path and its corresponding documentation.

Bug Fixes:

  • Strengthen the exit-code hygiene check so every exit-3 die and both documented exit-3 rows must agree on all documented causes and cannot falsely claim that nothing was pushed.

Enhancements:

  • Anchor exit-3 causes to their code locations and derive timeout operations from call sites, allowing meaningful message rewording while detecting undocumented dies and mismatched causes.

Tests:

  • Add fixture-based regression coverage for undocumented exit-3 dies, unreadable messages, missing causes, unsafe origin claims, missing network operations, and valid meaning-preserving rewordings.

… cause and both documented rows, so a die that claims nothing was pushed or a fourth die no row documents fails it (#68 item 5)

test_the_exit_3_documentation_agrees_with_the_die_message_it_describes
extracted ONE of lanes-edit.sh's three `die "..." 3` (the rebase conflict's,
found by its own words) and read the two exit-3 rows only for the words
timeout, push, pull and peer's anywhere in the table text. git_timeout_die's
message and commit_push's six-attempts message could say anything, "nothing
was pushed" included where a push may already have landed, and the test
stayed green. Measured on scratch copies of the real files: 9 of 15
mutations (either die saying nothing was pushed, either die losing its cause,
a fourth die, a call site naming a fetch, a table row losing its timeout or
its peer's-file cause) left the old test green.

The rule now collects every `die "..." 3`, ties each to its cause by where it
sits (the function git_timeout_die; the branch after the dirty_elsewhere
guard; the branch after a failed `git_net ... pull`) and never by its words,
so the rewording #68 item 3 and the act 2 commit_publish will do cannot blind
it, and holds each die and the exit-3 ROW of the table and of the manual to
the same three causes. A die no cause explains, or one it cannot read, is a
finding; so is a die or a row that claims nothing reached origin (the one
true form is "not pushed by this attempt"); and the network operations
git_timeout_die is called for are read off its call sites, so a timeout on a
fetch the rows never name is caught. The rule is a function over text, with
a fixture and 22 red and 7 green cases, so the suite itself keeps the proof.
The docstring's stale line numbers are replaced by the functions they meant.

Tests only: lanes-edit.sh and docs/README-lanes.md are #214's and are not
touched. Part of #68; items 1-4 wait on #214.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 18, 2026

Copy link
Copy Markdown

Reviewer's Guide

Strengthens exit-3 repository hygiene validation by structurally discovering every die site, mapping it to its control-flow cause, and requiring the code table and manual row to document the same causes and timeout operations, with comprehensive mutation and rewording regression tests.

File-Level Changes

Change Details Files
Replaced the single-message exit-3 hygiene assertion with a structural analyzer that validates every exit-3 die against its documented cause and both exit-code rows.
  • Extracts all literal exit-3 die messages and flags unreadable or undocumented additional dies.
  • Associates dies with causes by function and branch markers rather than message wording, while checking cause-specific concepts and attempt-scoped wording.
  • Reads timeout operations from git_timeout_die call sites and requires both documentation rows to name each operation and all three causes.
  • Checks only the relevant exit-3 table rows, including missing rows and forbidden claims that nothing was pushed.
  • Updates the existing hygiene test to run code-only validation first and then validate the manual copy.
tests/test_repo_hygiene.py
Added focused fixture-based regression coverage for missing causes, misleading wording, extra dies, parser gaps, and meaning-preserving rewordings.
  • Adds refusal cases for 22 planted disagreements, including fourth dies, missing causes, incorrect origin claims, and undocumented timeout operations.
  • Adds acceptance cases for seven compatible rewordings, wrapped lines, comments, renamed functions, and alternate timeout spelling.
  • Verifies the unmodified fixture passes and asserts findings fail for the intended reason.
tests/test_repo_hygiene.py

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

…e word, tolerates more than one space before the 3, and its comments name what the anchors and the reader cannot reach (#68 item 5, review of #259)

The review of #259 found four things in the rule, all in tests/test_repo_hygiene.py:

- The reader for git_timeout_die's calls saw only a double-quoted first
  argument. #68 item 3 may hand it the operation as a word of its own
  (`git_timeout_die push "git push ..."`), and the rule would then report
  "nothing calls git_timeout_die", which is false. Every mention of the name
  but its definition is now read as a call, and the words after it on its
  line, a bare or a quoted operation word, name the operation. A call whose
  shape the reader cannot place is a finding that says exactly that.
- _ENDS_IN_EXIT_3 and _DIE_EXIT_3 took exactly one space before the 3, so a
  die with two spaces slipped past the extractor and the count that guards
  it. Both take spaces or tabs now, and the comment says what stays out of
  reach (an exit code in a variable, a backslash-continued 3) instead of
  "never a hole".
- The comment and the docstring called the rebase conflict's anchor "a
  failed pull". It is the git_net ... pull call that the die sits after.
- The ROUND 5 bullets in the test's docstring restated the comment block at
  _EXIT_3_CAUSES; they are one pointer now, and the two manual-row fixture
  cases that repeated table cases are gone.

Fixture: two red cases added (a bare operation word no row names; a mention of
git_timeout_die the reader cannot place as a call) and two dropped (the manual
repeats of table cases), two green added (the bare-word call; two spaces
before the exit code). 22 red and 9 green, 235 passed in the hygiene file.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@sourcery-ai

sourcery-ai Bot commented Oct 19, 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 2 days and 6 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap merged commit ec4555b into main Oct 19, 2026
5 checks passed
brettheap added a commit that referenced this pull request Oct 19, 2026
…tead of reading it as no answer, and its window ref is checked as a whole string (#72 item 8, #80 item 4) (#260)

Landed on Brett Heap's word "land #259 #260 #261 when green" (2026-10-19), by lane openRepoTools-5. Required Linux CI (guard-launch-mode, tests, tests-no-submodule; run 39246399617) was green on head c249ba6 with main 9b94a59 in its tree. Main ec4555b merged with head c249ba6 is tree 36a1d17, which queue run 39248750040 (workflow_dispatch of tests on 961fcb7) tested whole and green.

Independent review (lane 5, Opus, of c9d681f): LAND. Fix round 1 (c249ba6) adds the canon-lane read after the three rungs, so a case-twin lane is refused whichever rung found it, and refuses a window ref carrying a newline.

Part of #72
Part of #80

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 19, 2026
…, and prints both in its transcript and in CI's step summary (#77, step 1a) (#261)

Landed on Brett Heap's word "land #259 #260 #261 when green" (2026-10-19), by lane openRepoTools-5. Required Linux CI (guard-launch-mode, tests, tests-no-submodule; run 39245315763) was green on head d05dc0b with main 08e7550 in its tree. Main add5c15 merged with head d05dc0b is tree ca3c9d5, which queue run 39248755273 (workflow_dispatch of tests on 800ec88) tested whole and green.

Independent review (lane 5, Opus, of 6335ca8): LAND. Fix round 1 (d05dc0b) reports the teardown once and names a failed removal, and moves the rows-equal-headings check to the report.

Part of #77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 19, 2026
Main now holds lane 4 queue, lane 5 queue (#259 #260 #261) and #246 (55aae16).
The one conflict is tests/test_repo_hygiene.py import block: #262 adds
import sys and #259 adds from collections import namedtuple; both are kept.
The tree is lane3-queue/q1d-228pr1-on-p246c, run 39256880821, green.

Lane: openRepoTools-3
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 19, 2026
Main moved past this branch's base 5e88a2c: #259, #260, #261 and #246.
#246 touches lanes-edit.sh and docs/README-lanes.md too. The merge is
clean, the lock lines are main's own, and lanes-edit.sh parses under
bash 3.2.57 and 5.2. CI tests the moved main.

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

Development

Successfully merging this pull request may close these issues.

1 participant