From e46254a7b957087b7f8802db62ce268b270b8b34 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 14:15:13 +0000 Subject: [PATCH 1/2] park's not-carried report counts a leg's unpushed commits on the branch its own row declares, else the root's tracking_branch (#242) report_repos gave every leg's row the ROOT's tracking_branch, but the root's 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 asked about the wrong one, found nothing ahead and ended without naming the leg: a silent miss in the report that exists to say what park did not carry. Both loops of report_repos (the standalone root and a family member's) now take each leg's branch from leg_branch_of, the shared helper #238 gave status and resume, and fall back to the root's branch on an empty answer, as status's estate_repos does. The false comment says the leg row's own branch:, else the root's tracking_branch, and points at #235 and #238. The shared resolver block is untouched. Four tests beside the not-carried report's: a standalone root's spec leg on branch: develop, the same inside a family member's clone, a leg with no branch: unchanged, and one root with a leg of each kind. The first, second and fourth fail on main's park and pass with this change. Fixes #242 Co-Authored-By: Claude Sonnet 5.5 --- park | 21 +++--- tests/test_park_resume_commands.py | 111 +++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 9 deletions(-) diff --git a/park b/park index f8f04a53..06d211b6 100755 --- a/park +++ b/park @@ -599,7 +599,7 @@ submodule_paths() { # working clone BESIDE the holder is where a person works, so it is what is # read - the same reason the holder's own `park` target dispatches there (#80). report_repos() { - local leg member branch + local leg member branch legbranch if [ "$ESTATE_KIND" = "family" ]; then printf 'holder\t%s\t%s\n' "$(tracking_branch_of "$ESTATE_ROOT")" \ "$ESTATE_ROOT" @@ -608,12 +608,12 @@ report_repos() { branch="$(tracking_branch_of "$ESTATE_DIR/$member")" printf 'root\t%s\t%s\n' "$branch" "$ESTATE_DIR/$member" while IFS= read -r leg; do - # A LEG'S branch here is the ROOT's `tracking_branch:`, which - # is NOT what `scripts/bootstrap.py` places a leg on when its - # row declares its own `branch:` (`leg_branch_of`): such a - # leg's unpushed commits are counted against the wrong - # branch (#242). - printf 'leg\t%s\t%s\n' "$branch" "$ESTATE_DIR/$member/$leg" + # A LEG'S branch is the `branch:` its own row declares, else + # the ROOT's `tracking_branch:` — what `scripts/bootstrap.py` + # places it on (#235, #238; #242 here). Its unpushed commits + # are on THAT branch, so that is the one the report counts. + legbranch="$(leg_branch_of "$ESTATE_DIR/$member" "$leg")" + printf 'leg\t%s\t%s\n' "${legbranch:-$branch}" "$ESTATE_DIR/$member/$leg" done < <(submodule_paths "$ESTATE_DIR/$member") done < <(family_members "$ESTATE_ROOT/family.yaml") return 0 @@ -621,12 +621,15 @@ report_repos() { branch="$(tracking_branch_of "$ESTATE_ROOT")" printf 'root\t%s\t%s\n' "$branch" "$ESTATE_ROOT" while IFS= read -r leg; do - printf 'leg\t%s\t%s\n' "$branch" "$ESTATE_ROOT/$leg" + # The leg's own `branch:`, else the root's: see the family loop above. + legbranch="$(leg_branch_of "$ESTATE_ROOT" "$leg")" + printf 'leg\t%s\t%s\n' "${legbranch:-$branch}" "$ESTATE_ROOT/$leg" done < <(submodule_paths "$ESTATE_ROOT") } # Commits on the TRACKING BRANCH that origin has never seen. The branch is -# the root's own `tracking_branch:` and not a hard-coded `main`: a project +# the root's own `tracking_branch:` — or, for a leg, the `branch:` its own row +# declares (`report_repos`, #242) — and not a hard-coded `main`: a project # that ships from `trunk` would otherwise have been asked a question about a # branch it does not have and answered "nothing", which is worse than not # asking. BOTH refs must exist locally or there is no question to ask — a diff --git a/tests/test_park_resume_commands.py b/tests/test_park_resume_commands.py index a1b98997..2f73cb71 100644 --- a/tests/test_park_resume_commands.py +++ b/tests/test_park_resume_commands.py @@ -1387,6 +1387,117 @@ def test_park_says_when_it_left_nothing_behind(estate, home): assert "0 thing(s) above are not park's to carry" in result.stdout +def clone_leg_member(dest: Path, base: Path, member: str, + **seed) -> dict[str, Path]: + """`seed_leg_member`'s root, cloned into `dest` with its legs mounted. + + Built per test under `base` rather than into the session's `remotes`, so a + leg has a `develop` to sit on only where a test asks for one (`seed` goes + to `seed_leg_member`). The legs arrive DETACHED at their pins, as every + fresh clone's do. + """ + base.mkdir() + bare, _ = seed_leg_member(base, member, **seed) + root = dest / member + git("-c", "protocol.file.allow=always", "clone", "-q", + "--recurse-submodules", str(bare), str(root), cwd=dest) + return {"root": root, "spec": root / "spec", "code": root / "code"} + + +def commits_on(leg: Path, branch: str, count: int) -> None: + """`count` commits on `leg`'s local `branch` that origin has not seen.""" + git("checkout", "-q", "-B", branch, f"origin/{branch}", cwd=leg) + for number in range(count): + (leg / f"{branch}-{number}.md").write_text("never pushed\n", + encoding="utf-8") + commit_all(leg, f"commit {number} on {branch}, not pushed") + + +def test_park_reports_a_leg_on_the_branch_its_own_row_declares(home, tmp_path): + """#242. `scripts/bootstrap.py` places a leg on `str(leg.get("branch") or + branch)`, so a leg whose row in `project.yaml` declares `branch: develop` + keeps its work on `develop`. The report asked about the ROOT's + `tracking_branch` for every leg, found nothing ahead on `main`, and ended + without naming the leg: work `park` did not carry, missed silently.""" + root = clone_leg_member(home / "projects", tmp_path / "remotes", + "Delta", leg_branch="develop") + commits_on(root["spec"], "develop", 3) + + result = run(PARK, "Delta", home=home) + assert result.returncode == 0, result.stderr + result.stdout + assert f"unpushed develop leg {root['spec']}" in result.stdout, ( + "a leg's unpushed commits on its own `branch:` were not reported:\n" + + result.stdout) + assert "3 commit(s) on develop that origin has not seen" in result.stdout + assert "A leg goes up as a pull request too" in result.stdout + assert f"unpushed main leg {root['spec']}" not in result.stdout, ( + "the leg was asked about the root's branch") + + +def test_park_reports_a_family_members_leg_on_its_own_declared_branch( + home, tmp_path): + """#242, in a family: the holder's report reads the member's working clone + beside it, and that member's legs, which are built by a second loop. Its + leg rows carried the member's `tracking_branch` too.""" + folder = home / "projects" / FAMILY + holder = folder / FAMILY + holder.mkdir(parents=True) + (holder / "family.yaml").write_text( + FAMILY_YAML.format( + id=FAMILY.lower(), name=FAMILY, org=ORG, + members=(f"\n - project: Delta\n id: delta\n" + f" repository: {ORG}/Delta\n" + f" path: members/Delta\n")), encoding="utf-8") + (holder / "Makefile").write_text(PROBE_MAKEFILE, encoding="utf-8") + member = clone_leg_member(folder, tmp_path / "remotes", "Delta", + leg_branch="develop") + commits_on(member["spec"], "develop", 2) + + result = run(PARK, FAMILY, home=home) + assert result.returncode == 0, result.stderr + result.stdout + assert f"unpushed develop leg {member['spec']}" in result.stdout, ( + "a member's leg on its own `branch:` was not reported:\n" + + result.stdout) + assert "2 commit(s) on develop that origin has not seen" in result.stdout + assert f"unpushed main leg {member['spec']}" not in result.stdout + + +def test_park_still_reports_a_leg_with_no_branch_of_its_own_on_the_roots( + home, tmp_path): + """UNCHANGED for a row with no `branch:`: bootstrap places that leg on the + root's `tracking_branch`, so that is the branch its unpushed commits are + counted on. Green before the fix of #242 and after it.""" + root = clone_leg_member(home / "projects", tmp_path / "remotes", "Echo") + commits_on(root["spec"], "main", 1) + + result = run(PARK, "Echo", home=home) + assert result.returncode == 0, result.stderr + result.stdout + assert f"unpushed main leg {root['spec']}" in result.stdout + assert "1 commit(s) on main that origin has not seen" in result.stdout + + +def test_park_asks_each_leg_of_one_root_about_its_own_branch(home, tmp_path): + """One root, two legs: `spec` declares `branch: develop` and `code` + declares none. Each is read against the branch bootstrap puts it on, so + the report names `spec` on `develop` and `code` on `main`, with their own + counts and neither leg's branch lent to the other.""" + root = clone_leg_member(home / "projects", tmp_path / "remotes", "Foxtrot", + leg_branch="develop", code_leg=True) + commits_on(root["spec"], "develop", 3) + commits_on(root["code"], "main", 1) + + result = run(PARK, "Foxtrot", home=home) + assert result.returncode == 0, result.stderr + result.stdout + assert f"unpushed develop leg {root['spec']}" in result.stdout, ( + result.stdout) + assert f"unpushed main leg {root['code']}" in result.stdout, ( + result.stdout) + assert "3 commit(s) on develop that origin has not seen" in result.stdout + assert "1 commit(s) on main that origin has not seen" in result.stdout + assert f"unpushed main leg {root['spec']}" not in result.stdout + assert f"unpushed develop leg {root['code']}" not in result.stdout + + def test_park_never_reads_the_pinned_copy(estate, home): """`members/` is pinned and DETACHED; nobody's work is there, and a report that named it would be reporting on the wrong copy.""" From 61883f199c3a5fcf7df52b21b63819b7436a2737 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 14:41:14 +0000 Subject: [PATCH 2/2] Apply the independent review of #249: survive an unreadable member manifest, pin the family loop's fallback, and fix status's comment about park (#242) Review 5563423499 of #249: 1. status's estate_repos header said park's report gives every leg the root's branch. It now gives every leg the same column as status does. 2. report_repos runs under set -e in a process substitution, and leg_branch_of fails when a project.yaml that exists cannot be read, so the bare assignment ended the subshell and dropped that leg's row and every row after it, with no word from park. Both loops now say || true and fall back to the root's branch; a test makes a family member's project.yaml mode 000 (skipped as root, which mode 000 does not bind) and fails without the guard. 3. The family loop's fallback to the member's tracking_branch was reached by no test: the member of test (b) now has a code leg declaring no branch:, and the report must name it on main. That fails with the family printf using "$legbranch" with no fallback. Co-Authored-By: Claude Sonnet 5.5 --- park | 11 ++++-- status | 6 +-- tests/test_park_resume_commands.py | 63 ++++++++++++++++++++++++------ 3 files changed, 63 insertions(+), 17 deletions(-) diff --git a/park b/park index 06d211b6..034b85f2 100755 --- a/park +++ b/park @@ -612,7 +612,11 @@ report_repos() { # the ROOT's `tracking_branch:` — what `scripts/bootstrap.py` # places it on (#235, #238; #242 here). Its unpushed commits # are on THAT branch, so that is the one the report counts. - legbranch="$(leg_branch_of "$ESTATE_DIR/$member" "$leg")" + # `|| true`: a `project.yaml` that cannot be read fails + # `leg_branch_of`, and under `set -e` that would end this + # subshell and drop this leg's row and every row after it, + # silently; the root's branch is the answer then. + legbranch="$(leg_branch_of "$ESTATE_DIR/$member" "$leg" || true)" printf 'leg\t%s\t%s\n' "${legbranch:-$branch}" "$ESTATE_DIR/$member/$leg" done < <(submodule_paths "$ESTATE_DIR/$member") done < <(family_members "$ESTATE_ROOT/family.yaml") @@ -621,8 +625,9 @@ report_repos() { branch="$(tracking_branch_of "$ESTATE_ROOT")" printf 'root\t%s\t%s\n' "$branch" "$ESTATE_ROOT" while IFS= read -r leg; do - # The leg's own `branch:`, else the root's: see the family loop above. - legbranch="$(leg_branch_of "$ESTATE_ROOT" "$leg")" + # The leg's own `branch:`, else the root's, and `|| true` for the + # reason the family loop gives. + legbranch="$(leg_branch_of "$ESTATE_ROOT" "$leg" || true)" printf 'leg\t%s\t%s\n' "${legbranch:-$branch}" "$ESTATE_ROOT/$leg" done < <(submodule_paths "$ESTATE_ROOT") } diff --git a/status b/status index d26b89d8..2df1569d 100755 --- a/status +++ b/status @@ -731,9 +731,9 @@ declared_legs() { # not-carried report's enumeration, with two rows it does not need: `missing`, # for a member with no working clone beside the holder, and `unmounted`, for a # leg declared and not there. Both are things out of sync with the estate as -# recorded, which is what this command is for. 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). +# 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). # # `members/` INSIDE A FAMILY HOLDER IS DELIBERATELY NOT HERE, for # park's reason: that copy is pinned and DETACHED, for `bootstrap` and diff --git a/tests/test_park_resume_commands.py b/tests/test_park_resume_commands.py index 2f73cb71..18c2ac4e 100644 --- a/tests/test_park_resume_commands.py +++ b/tests/test_park_resume_commands.py @@ -1413,6 +1413,22 @@ def commits_on(leg: Path, branch: str, count: int) -> None: commit_all(leg, f"commit {number} on {branch}, not pushed") +def family_folder_naming_delta(home: Path) -> Path: + """A family folder whose holder (a probe Makefile) names ONE member, + `Delta`, and whose working clone `clone_leg_member` puts beside it.""" + folder = home / "projects" / FAMILY + holder = folder / FAMILY + holder.mkdir(parents=True) + (holder / "family.yaml").write_text( + FAMILY_YAML.format( + id=FAMILY.lower(), name=FAMILY, org=ORG, + members=(f"\n - project: Delta\n id: delta\n" + f" repository: {ORG}/Delta\n" + f" path: members/Delta\n")), encoding="utf-8") + (holder / "Makefile").write_text(PROBE_MAKEFILE, encoding="utf-8") + return folder + + def test_park_reports_a_leg_on_the_branch_its_own_row_declares(home, tmp_path): """#242. `scripts/bootstrap.py` places a leg on `str(leg.get("branch") or branch)`, so a leg whose row in `project.yaml` declares `branch: develop` @@ -1439,19 +1455,11 @@ def test_park_reports_a_family_members_leg_on_its_own_declared_branch( """#242, in a family: the holder's report reads the member's working clone beside it, and that member's legs, which are built by a second loop. Its leg rows carried the member's `tracking_branch` too.""" - folder = home / "projects" / FAMILY - holder = folder / FAMILY - holder.mkdir(parents=True) - (holder / "family.yaml").write_text( - FAMILY_YAML.format( - id=FAMILY.lower(), name=FAMILY, org=ORG, - members=(f"\n - project: Delta\n id: delta\n" - f" repository: {ORG}/Delta\n" - f" path: members/Delta\n")), encoding="utf-8") - (holder / "Makefile").write_text(PROBE_MAKEFILE, encoding="utf-8") + folder = family_folder_naming_delta(home) member = clone_leg_member(folder, tmp_path / "remotes", "Delta", - leg_branch="develop") + leg_branch="develop", code_leg=True) commits_on(member["spec"], "develop", 2) + commits_on(member["code"], "main", 1) result = run(PARK, FAMILY, home=home) assert result.returncode == 0, result.stderr + result.stdout @@ -1460,6 +1468,39 @@ def test_park_reports_a_family_members_leg_on_its_own_declared_branch( + result.stdout) assert "2 commit(s) on develop that origin has not seen" in result.stdout assert f"unpushed main leg {member['spec']}" not in result.stdout + # The leg that declares none is the member's `tracking_branch`'s: the + # family loop's fallback, which nothing else here reaches. + 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 + + +@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, + reason="mode 000 does not bind root, so the manifest " + "stays readable and there is nothing to survive") +def test_park_reports_a_leg_when_its_members_manifest_cannot_be_read( + home, tmp_path): + """#242, the review of #249: `leg_branch_of` fails when `project.yaml` + exists and cannot be read, and `report_repos` runs under `set -e`, so an + unguarded `legbranch="$(leg_branch_of ...)"` ended the report's subshell: + that leg's row and every row after it were dropped, `make park`'s exit + code was untouched and `park` said nothing. The leg is reported against + the member's own branch instead (main's `park` did, because it never read + the file for a leg).""" + folder = family_folder_naming_delta(home) + member = clone_leg_member(folder, tmp_path / "remotes", "Delta") + commits_on(member["spec"], "main", 2) + manifest = member["root"] / "project.yaml" + manifest.chmod(0o000) + try: + result = run(PARK, FAMILY, home=home) + finally: + manifest.chmod(0o644) + assert result.returncode == 0, result.stderr + result.stdout + assert f"unpushed main leg {member['spec']}" in result.stdout, ( + "an unreadable manifest dropped the leg's row from the report:\n" + + result.stdout) + assert "2 commit(s) on main that origin has not seen" in result.stdout def test_park_still_reports_a_leg_with_no_branch_of_its_own_on_the_roots(