[brettheap]

The defect

park's not-carried report counts the commits a leg has on its branch that origin has never seen. report_repos emitted every leg's row with the ROOT's tracking_branch, but the root's scripts/bootstrap.py places a leg on str(leg.get("branch") or branch). A leg whose row in project.yaml declares its own branch: keeps its unpushed work on THAT branch, so not_parked_report / unpushed_on_tracking asked about the wrong one, found nothing ahead and ended without naming the leg. The report is what tells a person which work park did NOT carry, so the miss is silent. resume and status had the same assumption and were fixed by #238 (for #235); park was left out because #235's finding did not name it (found by review 5561239715 of #238).

The fix

• report_repos, in both loops (the standalone root and a family member's root, $ESTATE_DIR/$member), gives each leg's row leg_branch_of "<root>" "<leg>" || true, falling back to the root's branch on an empty answer, exactly as status's estate_repos does since resume and status hold each leg to the branch bootstrap puts it on: the leg row's own branch:, else the root's tracking_branch (#235) #238. leg_branch_of was already in park's copy of the shared resolver block, unused; the shared block is not edited.
• || true because report_repos runs under set -e in a process substitution and leg_branch_of fails when a project.yaml that exists cannot be read: a bare assignment would drop that leg's row and every row after it, silently (review 5563423499, finding 2).
• The false comment ("A LEG'S branch is the ROOT's tracking_branch:") now says the leg row's own branch:, else the root's tracking_branch, and points at resume's leg guard checks each leg against the root's tracking_branch, not the leg's own branch:, so it lets bootstrap move a leg (exit 0) and refuses a leg on its own branch #235 and resume and status hold each leg to the branch bootstrap puts it on: the leg row's own branch:, else the root's tracking_branch (#235) #238. The comment above unpushed_on_tracking says so for a leg too, and status's estate_repos header, which said park's report gives every leg the root's branch, now says the two are the same column.

A behaviour change worth stating: a leg that declares its own branch: is no longer asked about the ROOT's branch. Bootstrap places that leg on its own branch, so that is the branch the report asks about, as #242's suggested fix says; unpushed commits on the root's branch inside such a leg are not counted, and the report never covered a leg's other local branches. A leg with no branch: is held to the root's, unchanged.

Files: park, status (a comment only), tests/test_park_resume_commands.py. The shared resolver block is byte-identical in all three commands; its stale "park never asks where a leg belongs" comment is for #243's PR.

Tests

Five tests beside the existing not-carried-report tests, built with seed_leg_member (leg_branch= and code_leg= since #238):

test	on main's park	with the fix
test_park_reports_a_leg_on_the_branch_its_own_row_declares (standalone root, spec leg branch: develop, 3 local commits origin lacks; the report names the leg, develop and 3 commit(s))	FAILED	passed
test_park_reports_a_family_members_leg_on_its_own_declared_branch (the same inside a family member's clone, 2 commits; and a second code leg with no branch:, 1 commit, reported on main, which pins the family loop's fallback to the member's tracking_branch)	FAILED	passed
test_park_reports_a_leg_when_its_members_manifest_cannot_be_read (a family member's project.yaml at mode 000 still reports the leg's 2 unpushed commits on the fallback main; skipped as root, which mode 000 does not bind)	passed	passed
test_park_still_reports_a_leg_with_no_branch_of_its_own_on_the_roots (a leg with no branch: is unchanged)	passed	passed
test_park_asks_each_leg_of_one_root_about_its_own_branch (one root, spec on develop with 3 commits and code with none declared on main with 1)	FAILED	passed

Mutations, run against this head's tests:

• The unreadable-manifest test fails with the || true removed from both loops (the leg's row is dropped), and passes with it.
• The family test and the unreadable-manifest test fail with the family loop's printf using "$legbranch" and no fallback, and pass with "${legbranch:-$branch}".

First round, the four original tests against unmodified park: 3 failed, 1 passed, 181 deselected; with the fix 4 passed.

Modules in full at this head, after merging origin/main (no rebase, no force), under the shared pytest lock with every LANES_* and TMUX* variable unset and a private HOME, TMPDIR and basetemp: tests/test_park_resume_commands.py (186 tests), tests/test_status_command.py (209) and tests/test_repo_hygiene.py (189), together 584 passed. Not the whole suite.

Fixes #242

🤖 Generated with Claude Code

[sourcery-ai]

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 1 day and 10 hours by commenting @sourcery-ai review. Upgrade to get a review now.

[sourcery-ai]

Reviewer's Guide

Fix park's not-carried report to inspect each leg on the branch assigned by its own row, falling back to the root tracking branch, in both standalone and family-member layouts. Four focused tests cover the defect, fallback behavior, and independent branch handling; the documented validation reports all targeted tests and hygiene tests passing.

File-Level Changes

Change	Details	Files
Resolve each leg's effective branch before calculating unpushed commits in both standalone-root and family-member report paths.	Call the existing leg branch resolver for each leg row. Fall back to the root's tracking branch when a leg has no declared branch. Update comments to document the leg-specific branch precedence.	park
Add regression coverage for leg-specific branch reporting across root and family layouts.	Verify declared-branch legs report the correct branch and commit count. Verify family-member reports use the member leg's declared branch. Preserve root-branch behavior for legs without an override. Verify multiple legs are evaluated independently without branch cross-contamination.	tests/test_park_resume_commands.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#242	For standalone roots, report each leg’s unpushed commits against the branch declared on that leg, falling back to the root’s tracking branch when the leg has no branch override.	✅
#242	For family reports, apply the same per-leg branch selection for legs belonging to each family member’s working clone.	✅
#242	Update the misleading branch-assumption comments and add regression coverage for declared branches, fallback behavior, and multiple legs using different branches.	✅

Possibly linked issues

• park's not-carried report counts a leg's unpushed commits against the root's tracking_branch, not the leg's own branch:, so work on a declared branch is never reported #242: The PR fixes the issue by checking each leg’s declared branch in both standalone and family-member reports, with root-branch fallback.

---

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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[brettheap]

Independent review of #249 at e46254a: LAND after fixes

The fix itself is right: both loops of report_repos now ask leg_branch_of with the right root and the right leg spelling, park's actions are unchanged, and the shared block is byte-identical. Three small things need a change before it lands (1 to 3), and two are follow-ups that need no change here (4 and 5). I ran everything in a private clone at e46254a with the submodule at its pin, under the shared pytest lock, with every LANES_*/TMUX* variable unset and a private HOME, TMPDIR, basetemp and pycache prefix. Linux CI's tests and tests-no-submodule jobs were still running when I read the checks, so this verdict also needs them green.

1. LOW: status now carries a false comment this PR made false.
estate_repos' header in status says: "A LEG'S BRANCH COLUMN DIFFERS from park's: here it is the branch the leg is placed on (leg_branch_of), where park's report gives every leg the root's (#242)." (status:734-736). After this PR park gives each leg the same column, so the sentence says the opposite of what the code does. #238's own review found the same kind of stale text and fixed it in that PR. The block is below --- END shared estate resolver (status:685), so editing it does not touch the shared block. Evidence: command grep -rn '#242' . outside park and the tests returns only status:736.
Fix, in status only (it adds a third file to the PR):

# recorded, which is what this command is for. A LEG'S BRANCH COLUMN IS THE
# SAME as `park`'s: the `branch:` the leg's own row declares, else the root's
# `tracking_branch:` (`leg_branch_of`; #235, #242).

I applied exactly this in a throwaway copy: bash -n status is clean and tests/test_repo_hygiene.py is 189 passed.

2. LOW: the new legbranch="$(leg_branch_of ...)" can truncate the report silently under set -e.
park runs under set -euo pipefail (park:43), and report_repos runs in a process substitution (done < <(report_repos), park:688), which keeps -e. leg_branch_of returns 1 when its done <"$root/project.yaml" redirect fails (the file exists but cannot be read). The bare assignment then ends report_repos' subshell, so that leg's row and every row after it are dropped, make park's exit code is untouched, and park prints no failure of its own. Main's park read the same file here too, through tracking_branch_of, but inside an if, so a failed read was tolerated; the new read is unguarded.
Reproduced on a family member with its project.yaml at mode 000 and 2 commits on the leg's main (the leg declares no branch):

• main's park: prints unpushed main leg <P>/TestFam/Delta/spec / 2 commit(s) on main that origin has not seen, and "2 thing(s) above are not park's to carry".
• e46254a's park: stderr gets a second Permission denied (from leg_branch_of's done <), the unpushed row is gone, rc is 0, and the summary says "1 thing(s)". With no dirty root the run would end "nothing else was left behind".

A standalone root cannot reach this (discovery reads project.yaml first and refuses), so the exposure is a family member's manifest, which is narrow. But the report exists to never miss silently.
Fix, in both loops:

legbranch="$(leg_branch_of "$ESTATE_DIR/$member" "$leg" || true)"
...
legbranch="$(leg_branch_of "$ESTATE_ROOT" "$leg" || true)"

With that edit on a throwaway copy the same probe prints the unpushed main leg row again, and the four new tests plus the probe pass (9 passed).

3. LOW: no test pins the family loop's fallback to the member's tracking_branch.
Test (b) has only the leg that declares develop, so the family loop's ${legbranch:-$branch} is never exercised. Mutation check: changing only the family printf to "$legbranch" (empty branch for a leg with no branch:) leaves all four new tests green (4 passed). The other mutations are caught: passing $ESTATE_ROOT as the family root fails (b), passing an absolute leg path fails (b) and, in the standalone loop, (a) and (d), and dropping the fallback in the standalone loop fails (c) and (d).
Fix, in test_park_reports_a_family_members_leg_on_its_own_declared_branch: build the member with code_leg=True and add the second leg's row to the assertions.

    member = clone_leg_member(folder, tmp_path / "remotes", "Delta",
                              leg_branch="develop", code_leg=True)
    commits_on(member["spec"], "develop", 2)
    commits_on(member["code"], "main", 1)
    ...
    assert f"unpushed main   leg     {member['code']}" in result.stdout, (
        result.stdout)
    assert "1 commit(s) on main that origin has not seen" in result.stdout

Run on a throwaway copy: passes on e46254a's park, fails on main's park, and fails on the family-only no-fallback mutation.

4. NIT, follow-up only: the shared block's comment "park never asks where a leg belongs" is now false.
# shellcheck disable=SC2329 # parknever asks where a leg belongs;resumeandstatus do. sits above leg_branch_of in all three copies (park:527, resume:511, status:648). park now calls it. The PR is right not to edit one copy, because the byte-identity test pins all three. Fix: one follow-up that rewrites that comment identically in the three files. shellcheck -s bash park is clean at e46254a and on main.

5. INFO, no change needed: a leg that declares its own branch is no longer asked about the root's.
Probe: the leg declares develop, its local main is 2 ahead of origin/main, and develop equals origin/develop. Main's park printed unpushed main leg ... 2 commit(s) on main. e46254a prints "nothing else was left behind". That follows from #242's own suggested fix (bootstrap places that leg on develop, so develop is the branch the report asks about), and the report never covered other local branches. It is worth a sentence in the PR body, because "asks each leg about its own branch" hides that the root-branch question is dropped for such a leg.

Checklist results, with no defect found

• 1. Both loops. The family loop's root is "$ESTATE_DIR/$member", the same path the existing submodule_paths "$ESTATE_DIR/$member" reads (park:615, park:617). The standalone loop's root is $ESTATE_ROOT (park:625, park:627). The leg argument is the relative .gitmodules path submodule_paths prints, and leg_branch_of strips a leading ./ and a trailing / from both sides. Probes with the manifest spelling the path "./spec", spec/, 'spec' and ./spec all print unpushed develop leg ... 2 commit(s) on develop on e46254a; main's park printed no unpushed row for any of them.
• 2. unpushed_on_tracking / not_parked_report. Same outcomes as before for: local branch with no origin/<branch> (silent, as for the root's branch; unpushed_on_tracking still needs both refs), local branch gone with origin having it (silent), and a leg declaring main (reported, identical on main and e46254a). A detached-HEAD leg with a local develop 2 ahead is now reported (main's park was silent); a commit made on a detached HEAD, on no branch, is reported by neither (pre-existing). The only thing newly silent is item 5.
• 3. The rest of park. report_repos has exactly one consumer, not_parked_report (park:688), which runs after make park and its one_status. leg_branch_of only reads. Nothing park does changes, and the exit code is still make's (rc 0 in every probe above, including the one in item 2).
• 4. Tests. (a), (b) and (d) fail on main's park and pass on e46254a; (c) passes on both: 3 failed, 1 passed on main's park copied into a clone, 4 passed on the head. Each assertion names the leg's path, the branch and the exact count, and each test asserts returncode == 0. The count lines (3 commit(s) on develop ...) are separate substrings, not tied to the leg's row, but the row assertions beside them and the negative unpushed main leg assertions make them unambiguous in these fixtures. The module-level NEEDS_UPSTREAM mark makes the new tests skip, not fail, in tests-no-submodule.
• 5. Hygiene. No host-absolute path on any added line (a grep of git diff 87a5e9d e46254a's added lines for home, workspace, tmp and Users path prefixes finds nothing). The shared block hashes identically in park, status and resume (545c655df59a5c15798c935f5442c1931322ce53bbd03d6bf3ed5320a4cb52e6 three times); every hunk in park starts at line 602 or later and the block ends at line 564. tests/test_repo_hygiene.py and tests/test_park_resume_commands.py together: 374 passed. shellcheck -s bash park: clean.
• 6. PR body. The table and the counts match what I measured (3 failed, 1 passed on main's park; 185 + 189 = 374 passed). The only closing keyword is Fixes #242; "fixed by #238" and "Bug Fixes:" are not followed by an issue reference. It does not mention item 1's status comment or item 5.

🤖 Generated with Claude Code

[brettheap]

Applied the independent review 5563423499 at 33f3be1: findings 1, 2, 3 and 5; finding 4 is in #243's PR.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[brettheap]

Delta re-check of #249 at 33f3be1: LAND

No findings. Linux CI's tests and tests-no-submodule jobs were still running when I read the checks, so the verdict also needs them green. I checked only e46254a..33f3be1, in a private clone with the submodule at its pin, the shared pytest lock, every LANES_*/TMUX* variable unset and private HOME, TMPDIR and basetemp.

• The merge changed nothing of the PR's. git diff 61883f1 33f3be1 -- park status resume tests/test_park_resume_commands.py is empty. The merge brings in #247's files only (AGENTS.md, README.md, openRepoTools and four install tests), and main moved no verb between 87a5e9d and 586dea2.
• Finding 1, status comment: status:734-736 now says the branch column is the same as park's, as I suggested. The diff in status is those three comment lines.
• Finding 2, || true in both loops: present in both loops at park:619 and park:630. The new test test_park_reports_a_leg_when_its_members_manifest_cannot_be_read (family member's project.yaml at mode 000, 2 commits on the leg's main) is the mode-000 probe as a test. With park taken from each version: main 586dea2 passes, e46254a fails, 33f3be1 passes. Removing both || true guards from the head makes it fail again. The skip as root is correct (mode 000 does not bind root). It ran here as an ordinary user, not skipped.
• Finding 3, second code leg in test (b): with only the family printf changed to "$legbranch", test (b) now fails (1 failed) and the unreadable-manifest test fails too, as the PR body says. The head's park passes both. Test (b) still fails on main's park and passes on e46254a's.
• Finding 5, PR body: the "A behaviour change worth stating" paragraph says a leg that declares its own branch: is no longer asked about the root's branch, that unpushed commits on the root's branch inside such a leg are not counted, and that a leg with no branch: is unchanged. That is what my probe showed. The Files: line names the third file as "a comment only". The only closing keyword is still Fixes #242.
• family_folder_naming_delta: harmless and correct. Its body is the removed inline setup moved verbatim (holder family.yaml naming Delta, probe Makefile, returns the family folder). It has exactly two callers, test (b) and the new test, each with a fresh home, so its non-idempotent mkdir is not a problem. os is imported for the new skipif.
• Shared block: its hash is the same in park, status and resume (545c655df59a5c15798c935f5442c1931322ce53bbd03d6bf3ed5320a4cb52e6). The delta's hunks in park are at lines 615-619 and 628-630, after the block's end at line 564, and the status hunk is at 734.
• Tests run: the five tests that matter are 5 passed. tests/test_repo_hygiene.py plus tests/test_park_resume_commands.py give 375 passed (189 + 186). I did not run tests/test_status_command.py, which the PR body counts in its 584, because the status change is a comment.

One thing needing no change: the standalone loop's || true has no test. A standalone root cannot reach it, because discovery reads its project.yaml first and refuses; the guard is for parity with the family loop.

🤖 Generated with Claude Code