From 1030f120fc50563c3ce5354b530ab5c9e826c8ac Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 10:08:28 -0700 Subject: [PATCH 1/7] Track Qodo's Open-Source Login and State Its Star Gate (#2253) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The paid Qodo app is uninstalled. Qodo now reviews only through its open-source app, `qodo-free-for-open-source-projects`, which covers public repositories with more than 200 stars. In this fleet that is `PlexCleaner` alone, and the app offers no trigger below the gate. - `scripts/pr_review.py` now tracks the open-source login instead of the paid one, so that app's threads count toward `unresolved=` and its findings comment is read again. One test fixture carries the literal login, so a wrong constant fails the suite. - `docs/pr-reviewer-reference.md` states Qodo's star gate, says Qodo is absent below it with nothing to request, names the login, and notes that the hub's `.pr_agent.toml` currently reaches no reviewer here. - `docs/pr-reviewer-evaluation.md` limits the Qodo half of "record every finding" to repositories past the gate. - In `pr-review-conduct`, the skip-notice bullet no longer offers Qodo's `/review`. It now says to comment the trigger the notice names. Not in this PR: with the open-source login tracked, `qodo_open` double-counts that app's threaded findings and never clears them. That error is loud, never silent, and it's filed as #2252. Three attempts at matching findings to threads were backed out, and the issue records why. #1404's decided "count every unresolved thread" fix also remains its own change. Closes #1465. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Documentation** * Clarified when Qodo and CodeRabbit review pull requests, including Qodo’s public-repository eligibility threshold. * Updated review guidance to use the trigger specified in the notice. * **Chores** * Updated Qodo reviewer identification in review tracking and related tests. --------- Co-authored-by: Claude Opus 5.5 --- .agents/skills/pr-review-conduct/SKILL.md | 7 ++-- .../.source-digests/pr-review-conduct | 2 +- .../skills/pr-review-conduct/SKILL.md | 7 ++-- .github/skills/pr-review-conduct/SKILL.md | 7 ++-- docs/pr-reviewer-evaluation.md | 2 +- docs/pr-reviewer-reference.md | 10 +++-- scripts/pr_review.py | 10 ++--- tests/test_pr_review.py | 38 +++++++++++-------- 8 files changed, 47 insertions(+), 36 deletions(-) diff --git a/.agents/skills/pr-review-conduct/SKILL.md b/.agents/skills/pr-review-conduct/SKILL.md index 33f3d485..90a1a98c 100644 --- a/.agents/skills/pr-review-conduct/SKILL.md +++ b/.agents/skills/pr-review-conduct/SKILL.md @@ -135,9 +135,10 @@ the pull request in front of you, rather than deciding from a repository propert must have done. - **A reviewer that posted a skip notice is available for the asking.** It says it did not review - automatically, which is not the same as not reviewing at all. Comment `@coderabbitai review`, or - Qodo's `/review`, and wait for the result as with any other requested review. The agent driving - the loop posts that comment itself, on the same standing as requesting a review after a push. + automatically, which is not the same as not reviewing at all. Comment the reviewer's documented + review command, such as `@coderabbitai review`, and wait for the result as with any other + requested review. The agent driving the loop posts that comment itself, on the same standing as + requesting a review after a push. - **A notice naming when the reviewer can next run is a rate limit, and asking does not clear it.** It reads like the skip notice above and is the opposite case: the trigger returns the same notice rather than a review, so a loop that keeps asking waits on something no amount of asking diff --git a/.claude-plugin/fleet-skills/.source-digests/pr-review-conduct b/.claude-plugin/fleet-skills/.source-digests/pr-review-conduct index 660b110f..7916f766 100644 --- a/.claude-plugin/fleet-skills/.source-digests/pr-review-conduct +++ b/.claude-plugin/fleet-skills/.source-digests/pr-review-conduct @@ -1 +1 @@ -7ab7baf90da9b766 +0331a2ae5b800214 diff --git a/.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.md b/.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.md index 33f3d485..90a1a98c 100644 --- a/.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.md @@ -135,9 +135,10 @@ the pull request in front of you, rather than deciding from a repository propert must have done. - **A reviewer that posted a skip notice is available for the asking.** It says it did not review - automatically, which is not the same as not reviewing at all. Comment `@coderabbitai review`, or - Qodo's `/review`, and wait for the result as with any other requested review. The agent driving - the loop posts that comment itself, on the same standing as requesting a review after a push. + automatically, which is not the same as not reviewing at all. Comment the reviewer's documented + review command, such as `@coderabbitai review`, and wait for the result as with any other + requested review. The agent driving the loop posts that comment itself, on the same standing as + requesting a review after a push. - **A notice naming when the reviewer can next run is a rate limit, and asking does not clear it.** It reads like the skip notice above and is the opposite case: the trigger returns the same notice rather than a review, so a loop that keeps asking waits on something no amount of asking diff --git a/.github/skills/pr-review-conduct/SKILL.md b/.github/skills/pr-review-conduct/SKILL.md index 33f3d485..90a1a98c 100644 --- a/.github/skills/pr-review-conduct/SKILL.md +++ b/.github/skills/pr-review-conduct/SKILL.md @@ -135,9 +135,10 @@ the pull request in front of you, rather than deciding from a repository propert must have done. - **A reviewer that posted a skip notice is available for the asking.** It says it did not review - automatically, which is not the same as not reviewing at all. Comment `@coderabbitai review`, or - Qodo's `/review`, and wait for the result as with any other requested review. The agent driving - the loop posts that comment itself, on the same standing as requesting a review after a push. + automatically, which is not the same as not reviewing at all. Comment the reviewer's documented + review command, such as `@coderabbitai review`, and wait for the result as with any other + requested review. The agent driving the loop posts that comment itself, on the same standing as + requesting a review after a push. - **A notice naming when the reviewer can next run is a rate limit, and asking does not clear it.** It reads like the skip notice above and is the opposite case: the trigger returns the same notice rather than a review, so a loop that keeps asking waits on something no amount of asking diff --git a/docs/pr-reviewer-evaluation.md b/docs/pr-reviewer-evaluation.md index d4442636..9c6c3cee 100644 --- a/docs/pr-reviewer-evaluation.md +++ b/docs/pr-reviewer-evaluation.md @@ -152,7 +152,7 @@ The existing Copilot adapter remains behaviorally unchanged during extraction. P ## Next Evaluation Steps -1. Record every CodeRabbit and Qodo finding on subsequent public pull requests. +1. Record every CodeRabbit finding on subsequent public pull requests, and every Qodo finding on the repositories past its star gate. 2. Measure time to review, current-head coverage, duplicates, and interaction effort. 3. Recheck candidate plan terms periodically, since product plans are external state. 4. Decide whether either candidate meets the first-class support criteria. diff --git a/docs/pr-reviewer-reference.md b/docs/pr-reviewer-reference.md index 67e3175a..57afc9ba 100644 --- a/docs/pr-reviewer-reference.md +++ b/docs/pr-reviewer-reference.md @@ -14,9 +14,9 @@ Much of what follows is external product state that the fleet observes rather th ## Coverage -As of September 2026 both candidates run on their open-source tiers. CodeRabbit and Qodo review the maintainer's public repositories and never a private one, so a private repository has Copilot as its only pull request reviewer, and Copilot's own review budget, a self-configured premium request cap, runs out under concurrent pull requests. CodeRabbit's open-source tier auto-reviews only a repository with at least ten stars, so on `ProjectTemplate`, which holds fewer, it reviews only on an explicit trigger. The Claude GitHub App is installed on the account and is not configured as a reviewer, since the `local-strict-review` Skill already runs a review pass before every push toward a pull request. +As of October 2026 both candidates run on their open-source tiers alone. Neither CodeRabbit nor Qodo reviews a private repository, so a private repository has Copilot as its only pull request reviewer, and Copilot's own review budget, a self-configured premium request cap, runs out under concurrent pull requests. CodeRabbit's open-source tier auto-reviews only a repository with at least ten stars, so on `ProjectTemplate`, which holds fewer, it reviews only on an explicit trigger. Qodo's open-source program auto-reviews only a public repository past a 200-star threshold, so in this fleet it auto-reviews `PlexCleaner` alone. The Claude GitHub App is installed on the account and is not configured as a reviewer, since the `local-strict-review` Skill already runs a review pass before every push toward a pull request. -The consequence a review loop actually needs: **a private repository has Copilot alone**, and no trigger produces a candidate review there, while **a public repository below CodeRabbit's star gate still has a CodeRabbit review for the asking**. `.agents/skills/pr-review-conduct/SKILL.md` "Which Reviewers a Repository Actually Has" states the rule an agent applies, in a form that needs none of the facts above, since it is carried into repositories that hold no copy of this file. +The consequence a review loop actually needs: **a private repository has Copilot alone**, and no trigger produces a candidate review there, while **a public repository below CodeRabbit's star gate still has a CodeRabbit review for the asking**, and **a repository below Qodo's star gate gets no automatic Qodo review**, so its absence there is not waited on. `.agents/skills/pr-review-conduct/SKILL.md` "Which Reviewers a Repository Actually Has" states the rule an agent applies, in a form that needs none of the facts above, since it is carried into repositories that hold no copy of this file. ## Interaction and Operations @@ -54,6 +54,8 @@ Incremental follow-up needed an explicit command on [pull request #892][pr-892]. ### Qodo +It posts as `qodo-free-for-open-source-projects`, its open-source app's identity and the login `scripts/pr_review.py` tracks. + The findings are individually anchored and usually concise after HTML presentation is removed. After a corrective push it updates the existing review comment and its resolved state rather than creating another review, so a reader comparing review creation events alone sees no second round. @@ -73,12 +75,12 @@ The formal review body was empty. All useful state lived in inline comments, so Each reviewer's behavior is shaped by a committed file rather than accepted as given, and each setting below carries the finding or the cost that earned it. - **CodeRabbit**, in `.coderabbit.yaml`, whose [settings reference][coderabbit-config] names each option: auto review on pull requests into `develop`, which the open-source tier honors only at ten stars or more, no pause after five reviewed commits, since a fleet pull request routinely passes five pushes and the pause reads as a reviewer that stopped. A path instruction for Markdown asks for false, stale, unverifiable, or unfollowable claims only, since CI lints style and the local review pass reads canonical prose whole. Sequence diagrams, suggested labels and reviewers, and the in-progress fortune are off. The markdownlint, actionlint, shellcheck, and ruff tools are off, since CI runs the same four and fails the pull request on them. -- **Qodo**, in `.pr_agent.toml`, whose [configuration reference][qodo-config] names each option: an issues guideline asks for a reproduction with any claimed crash, after a claimed `IsADirectoryError` on this repository's build was disproven by running it. A compliance guideline asks for the rule's own sentence and routes a rule against unchanged text to the summary. Informational findings go to the [summary][qodo-verbosity] rather than a thread, since a thread blocks the merge until resolved. Images are off so a finding's title is plain text a matcher can see. Qodo's [review standards][qodo-rules] import from `AGENTS.md`, `CLAUDE.md`, `copilot-instructions.md`, and `SKILL.md` files, each scoped to its folder at any depth, when changes merge, and only new rules are added, so an edited or deleted rule is changed in its portal instead. +- **Qodo**, in `.pr_agent.toml`, whose [configuration reference][qodo-config] names each option: an issues guideline asks for a reproduction with any claimed crash, after a claimed `IsADirectoryError` on this repository's build was disproven by running it. A compliance guideline asks for the rule's own sentence and routes a rule against unchanged text to the summary. Informational findings go to the [summary][qodo-verbosity] rather than a thread, since a thread blocks the merge until resolved. Images are off so a finding's title is plain text a matcher can see. Qodo's [review standards][qodo-rules] import from `AGENTS.md`, `CLAUDE.md`, `copilot-instructions.md`, and `SKILL.md` files, each scoped to its folder at any depth, when changes merge, and only new rules are added, so an edited or deleted rule is changed in its portal instead. This repository sits below Qodo's star gate and gets no automatic Qodo review. - **GitHub Copilot**: the carried `.github/copilot-instructions.md`, which bootstraps the `fleet-code-review` Skill, is the lever this repository uses. GitHub also documents path-scoped `.github/instructions/*.instructions.md` files, unused here. ## Plan and Repository Scope -The paid trials are over, and both candidates run on their no-cost open-source tiers. Candidate use is therefore limited to public repositories unless the maintainer approves a later plan change. +The paid trials are over, and both candidates run on their no-cost open-source tiers. Qodo's paid app is uninstalled rather than suspended, so its `qodo-code-review` identity posts nothing, its billing notice included. Candidate use is therefore limited to public repositories unless the maintainer approves a later plan change. [CodeRabbit's current plan documentation][coderabbit-plans] provides an open-source tier for public repositories with rate limits. Product plans are external state, so confirm the terms again before relying on them. diff --git a/scripts/pr_review.py b/scripts/pr_review.py index afe17bf1..8d676bb6 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -63,9 +63,10 @@ regardless. `wait` is where that state gets its own exit codes, 46 and 47 below, because only `wait` is the command a caller might otherwise poll out a timeout on. `unresolved` counts every tracked reviewer's own open thread, not only Copilot's: - CodeRabbit (`coderabbitai`) and qodo (`qodo-code-review`) are tracked at the identity - and thread-resolution level, since an open thread blocks a ruleset-gated merge - whoever opened it and `unresolved=0` once hid one of theirs that still did. + CodeRabbit (`coderabbitai`) and qodo (`qodo-free-for-open-source-projects`) are + tracked at the identity and thread-resolution level, since an open thread blocks a + ruleset-gated merge whoever opened it and `unresolved=0` once hid one of theirs that + still did. Both `threads=` and `unresolved=` are read from a single 100-thread page with no further pagination, so a pull request carrying more than that undercounts silently past that point: both fields print a trailing `+` and a `THREADS TRUNCATED` block @@ -254,9 +255,8 @@ # Each format read here is its own reader, and doing that well is a separate task per bot. # What generalizes without reading any of their prose is thread resolution. # An open thread blocks a ruleset-gated merge whoever opened it, and `status`'s `unresolved=0` once silently hid a CodeRabbit/qodo thread that did block one. -# Login spellings are read off this repository's own history (`gh pr view --json reviews,comments`) rather than guessed. CODERABBIT_LOGIN = "coderabbitai" -QODO_LOGIN = "qodo-code-review" +QODO_LOGIN = "qodo-free-for-open-source-projects" # Named rather than inlined at each of their own readers below, so a login rename updates one spelling instead of silently leaving a hardcoded copy matching nothing. OTHER_REVIEWERS = (CODERABBIT_LOGIN, QODO_LOGIN) KNOWN_REVIEWERS = (REVIEWER, *OTHER_REVIEWERS) diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index 86d857ce..cc261e85 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -750,7 +750,7 @@ def test_a_coderabbit_and_a_qodo_thread_both_count_toward_unresolved(self) -> No [review()], [ thread("T1", login="coderabbitai"), - thread("T2", login="qodo-code-review"), + thread("T2", login="qodo-free-for-open-source-projects"), thread("T3", resolved=True, login="coderabbitai"), ], ) @@ -787,13 +787,13 @@ def test_other_reviewed_names_only_the_bots_that_posted_on_this_exact_head(self) [ review(), self.other_review("coderabbitai"), - self.other_review("qodo-code-review", oid=OLD), + self.other_review(pr_review.QODO_LOGIN, oid=OLD), ] ) ) out, _ = pr_review.digest("o", "r", 7) self.assertIn("other_reviewed=coderabbitai", out) - self.assertNotIn("qodo-code-review", out) + self.assertNotIn(pr_review.QODO_LOGIN, out) def test_other_reviewed_is_absent_where_neither_has_posted(self) -> None: """The common case, a repository not trialing either, stays silent rather than `none`.""" @@ -805,7 +805,7 @@ def test_reviewer_nodes_defaults_to_copilot_but_reads_any_login_explicitly(self) pr = payload([review(), self.other_review("coderabbitai")]) self.assertEqual(1, len(pr_review.reviewer_nodes(pr, "reviews"))) self.assertEqual(1, len(pr_review.reviewer_nodes(pr, "reviews", "coderabbitai"))) - self.assertEqual(0, len(pr_review.reviewer_nodes(pr, "reviews", "qodo-code-review"))) + self.assertEqual(0, len(pr_review.reviewer_nodes(pr, "reviews", pr_review.QODO_LOGIN))) def test_other_rate_limited_reads_the_structural_marker_on_a_plain_comment(self) -> None: """The one observed shape: a rate-limit notice riding a PR comment, not a formal review.""" @@ -839,7 +839,7 @@ def test_rate_limited_by_reads_either_connection_and_names_no_untracked_login(se [review()], comments=[comment(login="coderabbitai", body=RATE_LIMITED_COMMENT)] ) self.assertEqual("coderabbit.ai", pr_review.rate_limited_by(pr, "coderabbitai")) - self.assertIsNone(pr_review.rate_limited_by(pr, "qodo-code-review")) + self.assertIsNone(pr_review.rate_limited_by(pr, pr_review.QODO_LOGIN)) def test_rate_limited_by_reads_the_reviews_connection_too(self) -> None: """The marker rides a plain comment on the one observed instance, but the reader @@ -849,7 +849,7 @@ def test_rate_limited_by_reads_the_reviews_connection_too(self) -> None: comments=[], ) self.assertEqual("coderabbit.ai", pr_review.rate_limited_by(pr, "coderabbitai")) - self.assertIsNone(pr_review.rate_limited_by(pr, "qodo-code-review")) + self.assertIsNone(pr_review.rate_limited_by(pr, pr_review.QODO_LOGIN)) def test_thread_author_defaults_a_deleted_account_rather_than_crashing(self) -> None: orphan = thread("T1") @@ -1384,7 +1384,7 @@ class TestQodoOpenFindings(GqlCase): def test_an_open_finding_counts_and_prints_without_its_nested_subsections(self) -> None: self.answer( payload( - [review()], comments=[comment(login="qodo-code-review", body=qodo_review_body())] + [review()], comments=[comment(login=pr_review.QODO_LOGIN, body=qodo_review_body())] ) ) out, _ = pr_review.digest("o", "r", 7) @@ -1398,7 +1398,9 @@ def test_a_resolved_finding_does_not_count_as_open(self) -> None: self.answer( payload( [review()], - comments=[comment(login="qodo-code-review", body=qodo_review_body(resolved=True))], + comments=[ + comment(login=pr_review.QODO_LOGIN, body=qodo_review_body(resolved=True)) + ], ) ) out, _ = pr_review.digest("o", "r", 7) @@ -1410,7 +1412,9 @@ def test_qodo_open_prints_zero_rather_than_staying_silent_once_it_has_commented( self.answer( payload( [review()], - comments=[comment(login="qodo-code-review", body=qodo_review_body(resolved=True))], + comments=[ + comment(login=pr_review.QODO_LOGIN, body=qodo_review_body(resolved=True)) + ], ) ) out, _ = pr_review.digest("o", "r", 7) @@ -1424,7 +1428,9 @@ def test_qodo_open_is_absent_where_it_has_never_commented(self) -> None: def test_the_pr_summary_comment_is_not_read_as_the_findings_comment(self) -> None: """Qodo posts two comments per round, and only `Code Review by Qodo` carries findings.""" summary = "

PR Summary by Qodo

\n\nAdds a thing.\n" - self.answer(payload([review()], comments=[comment(login="qodo-code-review", body=summary)])) + self.answer( + payload([review()], comments=[comment(login=pr_review.QODO_LOGIN, body=summary)]) + ) out, _ = pr_review.digest("o", "r", 7) self.assertNotIn("qodo_open", out) @@ -1433,9 +1439,9 @@ def test_the_newest_findings_comment_wins_on_a_re_reviewed_pull_request(self) -> payload( [review()], comments=[ - comment(login="qodo-code-review", at=EARLY, body=qodo_review_body()), + comment(login=pr_review.QODO_LOGIN, at=EARLY, body=qodo_review_body()), comment( - login="qodo-code-review", at=LATE, body=qodo_review_body(resolved=True) + login=pr_review.QODO_LOGIN, at=LATE, body=qodo_review_body(resolved=True) ), ], ) @@ -1449,7 +1455,7 @@ def test_a_finding_titled_about_the_badge_word_itself_is_not_read_as_carrying_it finding. """ body = qodo_review_body(heading="isResolved handling is inconsistent") - self.answer(payload([review()], comments=[comment(login="qodo-code-review", body=body)])) + self.answer(payload([review()], comments=[comment(login=pr_review.QODO_LOGIN, body=body)])) out, _ = pr_review.digest("o", "r", 7) self.assertIn("qodo_open=1", out) self.assertIn("isResolved handling is inconsistent", out) @@ -1478,7 +1484,7 @@ def test_qodo_open_is_unknown_where_only_the_paired_summary_comment_is_in_view(s """ summary = "

PR Summary by Qodo

\n\nAdds a thing.\n" full = [comment(login="ptr727") for _ in range(pr_review.WINDOW - 1)] + [ - comment(login="qodo-code-review", body=summary) + comment(login=pr_review.QODO_LOGIN, body=summary) ] self.answer(payload([review()], comments=full, older=True)) out, _ = pr_review.digest("o", "r", 7) @@ -1495,7 +1501,7 @@ def test_a_summary_comment_mentioning_the_findings_heading_in_prose_is_not_selec """ summary = "

PR Summary by Qodo

\n\nA Code Review by Qodo will follow shortly.\n" full = [comment(login="ptr727") for _ in range(pr_review.WINDOW - 1)] + [ - comment(login="qodo-code-review", body=summary) + comment(login=pr_review.QODO_LOGIN, body=summary) ] self.answer(payload([review()], comments=full, older=True)) out, _ = pr_review.digest("o", "r", 7) @@ -1510,7 +1516,7 @@ def test_a_finding_titled_with_the_bare_badge_word_and_no_glyph_is_not_read_as_t badge, only something adjacent enough to be mistaken for it on the word alone. """ body = qodo_review_body(heading="Resolved flag ignored on retry") - self.answer(payload([review()], comments=[comment(login="qodo-code-review", body=body)])) + self.answer(payload([review()], comments=[comment(login=pr_review.QODO_LOGIN, body=body)])) out, _ = pr_review.digest("o", "r", 7) self.assertIn("qodo_open=1", out) From b52ab3e36d7559fbad2badede4643edb350988b5 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 12:15:26 -0700 Subject: [PATCH 2/7] Give install-tools a JSON Report Mode a Program Can Read (#2259) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Addresses #1645. `install-tools.sh --json` and `install-tools.ps1 -Json` write the report as one JSON object, so a scheduled reporter can read it without parsing the table. - **Shape:** `schema` (1), `platform`, a `tools` list, and a top-level `notes` list for what belongs to no tool. Each tool row carries `tool`, `installed`, `available`, `source`, `mechanism`, `status`, and that tool's own `notes`. Windows rows also carry `scope`. A version that was not read is `null`. - **`mechanism`:** like `source`, it names how the script manages the tool. On Linux it is `apt`, `binary`, or `docker-desktop` (docker inside WSL). On Windows it is `winget`. - **Encoding:** both sides write ASCII only. Bash has a pure-bash UTF-8 encoder, since `jq` is one of the tools the script installs and glibc `iconv -c` passes some malformed sequences through. It drops bytes that do not decode. PowerShell uses `-EscapeHandling EscapeNonAscii`. - **Refusal:** the flag is refused beside any other action, before the host is read. - **Table output:** byte-identical to `develop` on a live host. ## Verification - `tests/test_install_tools.py` (new, 8 tests) drives each installer's own report functions with host reads stubbed. Every guard in the encoder and in the note slicing was mutated, and a test failed for each one. - On this Linux host, a live `--json` run parses as valid ASCII JSON, and an unsupported locale adds no stderr noise. - The encoder gives the same output on bash 4.4, 5.1, and 5.2. - shellcheck, shfmt, PSScriptAnalyzer, markdownlint, editorconfig-checker, cspell, ruff, mypy, pyright, the prose gate, and `spec/validate.py` are all clean. - **Windows is not natively verified.** The PowerShell report was exercised under pwsh on Linux with winget stubbed. A native check is `install-tools.ps1 -Json | ConvertFrom-Json` on a Windows host. ## Filed Along the Way (pre-existing) - #2256: naming a tool other than the last makes `install-tools.sh` exit 1 silently, so `--json ` fails today. - #2257: under a minimal cron `PATH`, the managed binaries in `/usr/local/bin` are missed. - #2258: a winget version token such as `Unknown` is carried forward as the installed version. - #2260: a Linux note prints a path under the home directory as it is, where the Windows script hides it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **New Features** * Linux and Windows setup tools can now generate structured JSON reports with tool versions, availability, installation source and method, status, and relevant notes. * The existing table report remains the default. JSON reporting is limited to report mode; combining it with install, upgrade, or list actions is rejected. * **Documentation** * Setup guides now describe the JSON report fields, including platform-specific details. --------- Co-authored-by: Claude Opus 5.5 --- host-setup/linux/README.md | 2 + host-setup/linux/install-tools.sh | 156 ++++++++++++-- host-setup/windows/README.md | 2 + host-setup/windows/install-tools.ps1 | 65 +++++- tests/test_install_tools.py | 300 +++++++++++++++++++++++++++ 5 files changed, 505 insertions(+), 20 deletions(-) create mode 100755 tests/test_install_tools.py diff --git a/host-setup/linux/README.md b/host-setup/linux/README.md index c7ebfb23..aa4fd1f1 100644 --- a/host-setup/linux/README.md +++ b/host-setup/linux/README.md @@ -57,6 +57,8 @@ A report changes nothing and reads the apt cache as it stands. An available vers `unmanaged` means the tool is installed from the distro while its upstream repository is unconfigured. The one thing the report must not say is that such a tool is current against the distro's own version. The Windows report uses the same word for a different mechanism, a tool on `PATH` that `winget` knows no package for. +`--json` writes the same report as one JSON object, for a program to read instead of the table. It carries a `schema` number, the `platform`, a `tools` list, and a top-level `notes` list for what belongs to no tool. Each entry in `tools` carries `tool`, `installed`, `available`, `source`, `mechanism`, `status`, and that tool's own `notes`, and a version that was not read is `null`. Like `source`, `mechanism` names how this script manages the tool rather than where the installed copy came from. It is `apt` for a tool any apt upgrade moves, and `binary` for a released binary in `/usr/local/bin` that only this script moves. Inside a WSL distribution, `docker` reads `docker-desktop`, since this script leaves it to Docker Desktop. A change to what a field means raises `schema`. + An install or upgrade collects a tool whose install fails and carries on, so one failure does not strand the rest of the run. A refusal is different and ends the run. An unverifiable keyring, a checksum mismatch, or a declined prompt stops everything, because continuing past one would install something nobody vouched for. ## Docker, node, dotnet, and powershell diff --git a/host-setup/linux/install-tools.sh b/host-setup/linux/install-tools.sh index 7edd98b5..ddd0770f 100755 --- a/host-setup/linux/install-tools.sh +++ b/host-setup/linux/install-tools.sh @@ -32,6 +32,7 @@ readonly PYTHON_OPTIONAL=(python-is-python3 python-dev-is-python3 pipx python3-s MODE="report" DRY_RUN=false +JSON_OUTPUT=false ASSUME_YES=false WITH_OPTIONAL=false APT_REFRESHED=false @@ -48,6 +49,7 @@ REPO="" declare -A REPO_PACKAGES=() declare -A REPO_KEYS=() NOTES=() +NOTE_TEXTS=() FAILED=() CHANGED=() @@ -65,7 +67,82 @@ die() { exit 1 } -note() { NOTES+=("$1: $2"); } +note() { + NOTES+=("$1: $2") + NOTE_TEXTS+=("$2") +} + +json_string() ( + export LC_ALL=C + local s="$1" out="" i n b c cp need min j hi lo + if [[ $s != *[!\ -~]* && $s != *[\"\\]* ]]; then + printf '"%s"' "$s" + return 0 + fi + n=${#s} + for ((i = 0; i < n; i++)); do + printf -v b '%d' "'${s:i:1}" + b=$((b & 0xff)) + if ((b == 34 || b == 92)); then + out+="\\${s:i:1}" + elif ((b < 32)); then + printf -v c '\\u%04x' "$b" + out+=$c + elif ((b < 128)); then + out+=${s:i:1} + else + if ((b >= 0xc2 && b <= 0xdf)); then + need=1 cp=$((b & 0x1f)) min=0x80 + elif ((b >= 0xe0 && b <= 0xef)); then + need=2 cp=$((b & 0x0f)) min=0x800 + elif ((b >= 0xf0 && b <= 0xf4)); then + need=3 cp=$((b & 0x07)) min=0x10000 + else + continue + fi + for ((j = 1; j <= need; j++)); do + ((i + j < n)) || break + printf -v c '%d' "'${s:i+j:1}" + c=$((c & 0xff)) + ((c >= 0x80 && c <= 0xbf)) || break + cp=$(((cp << 6) | (c & 0x3f))) + done + if ((j <= need || cp < min || cp > 0x10ffff || (cp >= 0xd800 && cp <= 0xdfff))); then + continue + fi + i=$((i + need)) + if ((cp < 0x10000)); then + printf -v c '\\u%04x' "$cp" + else + cp=$((cp - 0x10000)) + hi=$((0xd800 + (cp >> 10))) + lo=$((0xdc00 + (cp & 0x3ff))) + printf -v c '\\u%04x\\u%04x' "$hi" "$lo" + fi + out+=$c + fi + done + printf '"%s"' "$out" +) + +json_value() { + if [[ -n $1 ]]; then + json_string "$1" + else + printf 'null' + fi +} + +json_notes() { + local i sep="" + printf '[' + for ((i = $1; i < $2; i++)); do + printf '%s' "$sep" + json_string "${NOTE_TEXTS[i]}" + sep=", " + done + printf ']' +} usage() { cat <<'EOF' @@ -85,6 +162,7 @@ Actions, name one, default --report: -h, --help Show this help Options: + -j, --json Write the report as JSON rather than a table, for a program to read -n, --dry-run Print the commands instead of running them -y, --yes Do not prompt before changing the host -o, --optional Include the optional tool and the optional package sets, where a tool has one @@ -94,6 +172,13 @@ Versions read as apt versions for an apt-managed tool and as upstream versions f binary, so a column compares like with like. A report reads the apt cache as it stands and does not refresh it, so an available version is as current as the last apt update. +--json writes one object carrying the same rows: "schema" (1), "platform" ("linux"), "tools", one +entry per tool with "tool", "installed", "available", "source", "mechanism", "status" and that +tool's own "notes", and a top-level "notes" for what belongs to no tool. A version that was not +read is null. Like "source", "mechanism" names how this script manages the tool rather than where +the installed copy came from: "apt", which any apt upgrade moves, "binary", which only this script +moves, or "docker-desktop" for docker inside a WSL distribution, which it leaves to Docker Desktop. + --sudo-timestamp writes a sudoers drop-in for the invoking user alone, so one "sudo -v" covers every terminal that user has open rather than only the one it ran in. It touches no tool. Removing the file it names undoes it, and "update-alternatives --auto sudo" undoes the @@ -101,6 +186,7 @@ implementation switch it asks for on a host whose sudo parses no timestamp_type. Examples: install-tools.sh Report on every tool + install-tools.sh --json Report on every tool, as JSON install-tools.sh --install Install what is missing install-tools.sh --upgrade --yes Bring every tool current, no prompt install-tools.sh --upgrade node jq Bring two tools current @@ -1006,33 +1092,74 @@ tool_note() { return 0 } +tool_mechanism() { + case "$1" in + jq | uv | git-restore-mtime) printf 'binary' ;; + docker) + if [[ $IS_WSL == true ]]; then + printf 'docker-desktop' + else + printf 'apt' + fi + ;; + *) printf 'apt' ;; + esac +} + report() { # Wide enough for an Ubuntu backport version, which is the longest of these in practice. local format="%-18s %-26s %-26s %-22s %s\n" - # shellcheck disable=SC2059 # Format string is a constant defined above. - printf "$format" "TOOL" "INSTALLED" "AVAILABLE" "SOURCE" "STATUS" + local -a rows=() + if [[ $JSON_OUTPUT == false ]]; then + # shellcheck disable=SC2059 # Format string is a constant defined above. + printf "$format" "TOOL" "INSTALLED" "AVAILABLE" "SOURCE" "STATUS" + fi if ! command -v curl >/dev/null; then note "report" "curl is not installed, so an upstream that is not an apt repository cannot be read yet" fi + local report_notes=${#NOTES[@]} - local tool installed target + local tool installed target source mechanism status first for tool in "${SELECTED[@]}"; do + first=${#NOTES[@]} if [[ -n ${REPO_PACKAGES[$tool]:-} ]]; then installed=$(apt_installed_version "${REPO_PACKAGES[$tool]}") target=$(apt_candidate_version "${REPO_PACKAGES[$tool]}") + source="apt:${REPO_PACKAGES[$tool]}" + mechanism="apt" + status=$(tool_status "$installed" "$target") + else + installed=$("$(tool_function "$tool" version)" 2>/dev/null || true) + target=$("$(tool_function "$tool" target)" 2>/dev/null || true) + source=$("$(tool_function "$tool" source)") + mechanism=$(tool_mechanism "$tool") + status=$(tool_effective_status "$tool" "$installed" "$target") + fi + if [[ $JSON_OUTPUT == false ]]; then # shellcheck disable=SC2059 # Format string is a constant defined above. - printf "$format" "$tool" "${installed:--}" "${target:--}" "apt:${REPO_PACKAGES[$tool]}" "$(tool_status "$installed" "$target")" - continue + printf "$format" "$tool" "${installed:--}" "${target:--}" "$source" "$status" + fi + [[ -n ${REPO_PACKAGES[$tool]:-} ]] || tool_note "$tool" + if [[ $JSON_OUTPUT == true ]]; then + rows+=("$(printf '{"tool": %s, "installed": %s, "available": %s, "source": %s, "mechanism": %s, "status": %s, "notes": %s}' \ + "$(json_string "$tool")" "$(json_value "$installed")" "$(json_value "$target")" "$(json_string "$source")" \ + "$(json_string "$mechanism")" "$(json_string "$status")" "$(json_notes "$first" "${#NOTES[@]}")")") fi - installed=$("$(tool_function "$tool" version)" 2>/dev/null || true) - target=$("$(tool_function "$tool" target)" 2>/dev/null || true) - # shellcheck disable=SC2059 # Format string is a constant defined above. - printf "$format" "$tool" "${installed:--}" "${target:--}" \ - "$("$(tool_function "$tool" source)")" "$(tool_effective_status "$tool" "$installed" "$target")" - tool_note "$tool" done + if [[ $JSON_OUTPUT == true ]]; then + local i sep="" + printf '{\n "schema": 1,\n "platform": "linux",\n "tools": [' + for ((i = 0; i < ${#rows[@]}; i++)); do + printf '%s\n %s' "$sep" "${rows[i]}" + sep="," + done + [[ ${#rows[@]} -eq 0 ]] || printf '\n ' + printf '],\n "notes": %s\n}\n' "$(json_notes 0 "$report_notes")" + return 0 + fi + [[ ${#NOTES[@]} -eq 0 ]] && return 0 log "" log "Notes:" @@ -1562,6 +1689,7 @@ parse_args() { -u | --upgrade) actions+=(upgrade) ;; -l | --list) actions+=(list) ;; --sudo-timestamp) actions+=(sudo-timestamp) ;; + -j | --json) JSON_OUTPUT=true ;; -n | --dry-run) DRY_RUN=true ;; -y | --yes) ASSUME_YES=true ;; -o | --optional) WITH_OPTIONAL=true ;; @@ -1588,6 +1716,10 @@ parse_args() { fi [[ ${#actions[@]} -eq 1 ]] && MODE="${actions[0]}" + if [[ $JSON_OUTPUT == true && $MODE != "report" ]]; then + die "--json changes how a report is written, so it applies only to --report, and the $MODE action was given" + fi + # The sudo-timestamp refusal needs the whole set of tool names a run asked for, so it is checked after the loop rather than inside it. if [[ $MODE == "sudo-timestamp" && ${#REQUESTED[@]} -gt 0 ]]; then die "--sudo-timestamp changes the host rather than a tool, so it takes no tool, and \"${REQUESTED[*]}\" names one" diff --git a/host-setup/windows/README.md b/host-setup/windows/README.md index df1f48c6..9cd0deb3 100644 --- a/host-setup/windows/README.md +++ b/host-setup/windows/README.md @@ -110,6 +110,7 @@ This moved a boundary the tool used to hold: WSL used to be read-only here, and | `sudo` re-runs a command as root | nothing elevates | `winget` raises UAC per installer, which is the path with the fewest failures | | `install-tools.sh --sudo-timestamp` shares one sudo credential cache across a user's terminals | no peer | Nothing here elevates on Windows, so there is no cached credential to share | | `unmanaged` means the upstream repository is unconfigured | `unmanaged` means the tool is on `PATH` and winget knows no package for it | The same question, by a different mechanism | +| `--json` reports `mechanism` as `apt`, `binary`, or `docker-desktop` | `-Json` reports `mechanism` as `winget`, `source` as the winget package id, and adds each tool's `scope` list | `winget` is the one package manager here, and scope is the one column Linux has no peer for. The report's shape is otherwise the one the [Linux README][linux-readme] describes, and a `multiple` row's `installed` is `null`, its note naming the versions | | `python-is-python3` supplies the `python` name, under `--optional` | `install-tools.ps1` supplies the `python3` name itself, always | Debian packages the alias and Windows does not, so the one platform installs a package and the other writes the file. This is the only action here that deletes something winget did not install, so it is narrow by construction: it removes an app execution alias only where the reparse tag and the target package both say it is the Microsoft Store placeholder, and reports rather than removes anything else | | `credential.helper cache --timeout=3600` | `credential.helper manager`, and only where unset | Git Credential Manager ships with Git for Windows | | `ssh-agent` is a socket, started per shell | `ssh-agent` is a Windows service, reported and not started | Starting it needs administrator, and nothing here elevates | @@ -157,6 +158,7 @@ The scripts are checked by `PSScriptAnalyzer`, which runs in CI as the peer of t [host-setup]: ../../docs/host-setup.md [host-setup-readme]: ../README.md [install-tools]: ./install-tools.ps1 +[linux-readme]: ../linux/README.md [menu]: ../menu.sh [menu-ps1]: ../menu.ps1 [setup-github]: ./setup-github.ps1 diff --git a/host-setup/windows/install-tools.ps1 b/host-setup/windows/install-tools.ps1 index 8cda9198..a8d34cf0 100644 --- a/host-setup/windows/install-tools.ps1 +++ b/host-setup/windows/install-tools.ps1 @@ -18,6 +18,7 @@ param( [Alias('u')][switch]$Upgrade, [switch]$Reinstall, [Alias('l')][switch]$List, + [Alias('j')][switch]$Json, [Alias('n')][switch]$DryRun, [Alias('y')][switch]$Yes, [Alias('o')][switch]$Optional, @@ -51,6 +52,7 @@ $WANT_TOOLS = @($Name | Where-Object { $_ }) $MODE = 'report' $DRY_RUN = [bool]$DryRun +$JSON_OUTPUT = [bool]$Json $ASSUME_YES = [bool]$Yes $WITH_OPTIONAL = [bool]$Optional $WANT_SCOPE = $Scope @@ -58,6 +60,7 @@ $REPO = $Repo $ELEVATED = $false $SELECTED = @() $NOTES = @() +$NOTE_TEXTS = @() $FAILED = @() $CHANGED = @() $EXPLICIT = $null @@ -70,7 +73,17 @@ function step { param([string]$Message) Write-Host "`n==> $Message" } function warn { param([string]$Message) [Console]::Error.WriteLine("WARNING: $Message") } function die { param([string]$Message) [Console]::Error.WriteLine("ERROR: $Message"); exit 1 } -function note { param([string]$Tool, [string]$Message) $script:NOTES += "${Tool}: $Message" } +function note { + param([string]$Tool, [string]$Message) + $script:NOTES += "${Tool}: $Message" + $script:NOTE_TEXTS += $Message +} + +function Get-NoteText { + param([int]$First, [int]$Last) + if ($Last -le $First) { return , @() } + return , @($script:NOTE_TEXTS[$First..($Last - 1)]) +} # A path with the home directory replaced by the variable that names it. # A report is written to be pasted into an issue or a pull request, so a path it prints carries the account name into wherever it is pasted, and the comments here already avoid writing one for the same reason. @@ -101,12 +114,19 @@ Actions, name one, default -Report: -h, -Help Show this help Options: + -j, -Json Write the report as JSON rather than a table, for a program to read -n, -DryRun Print the commands instead of running them -y, -Yes Do not prompt before changing the host -o, -Optional Include the optional package set, where a tool has one -Scope Name a scope, either user or machine, for the copy to act on -Repo PATH Include winget packages declared by PATH\host-tools.json +-Json writes one object carrying the same rows: "schema" (1), "platform" ("windows"), "tools", +one entry per tool with "tool", "installed", "available", "source" (the winget package id), +"mechanism" ("winget", how this script manages every tool), "status", "scope" and that tool's +own "notes", and a top-level "notes" for what belongs to no tool. A version that was not read, or +that did not resolve to one, is null. + Run this without elevation. No scope is passed unless -Scope names one, so winget acts on the copy it finds and an installer that needs administrator asks for it itself. Naming a scope that disagrees with the installed copy would add a second copy beside the first rather than replacing @@ -120,6 +140,7 @@ prompt, which this refuses to start unattended where nothing could answer it, ra Examples: install-tools.ps1 Report on every tool + install-tools.ps1 -Json Report on every tool, as JSON install-tools.ps1 -Install Install what is missing install-tools.ps1 -Upgrade -Yes Bring every tool current, no prompt install-tools.ps1 -Upgrade uv jq Bring two tools current @@ -1006,23 +1027,48 @@ function Show-List { function Show-Report { $format = '{0,-10} {1,-16} {2,-16} {3,-24} {4,-13} {5}' - log ($format -f 'TOOL', 'INSTALLED', 'AVAILABLE', 'SOURCE', 'SCOPE', 'STATUS') + if (-not $script:JSON_OUTPUT) { log ($format -f 'TOOL', 'INSTALLED', 'AVAILABLE', 'SOURCE', 'SCOPE', 'STATUS') } + $rows = @() foreach ($tool in $script:SELECTED) { + $first = $script:NOTE_TEXTS.Count $record = Get-Tool $tool $state = Get-ToolState -Tool $record # Every row is printed only where they did not resolve to one version, since a dotnet line carrying three side by side builds resolves cleanly and listing all three would overflow the column for nothing. - $installed = if ($state.Status -eq 'multiple') { $state.Rows -join ',' } elseif ($state.Installed) { $state.Installed } else { '-' } - $available = if ($state.Available) { $state.Available } else { '-' } - $scope = if ($state.Scope.Count -gt 0) { $state.Scope -join '+' } else { '-' } - log ($format -f $record.Name, $installed, $available, $state.Package, $scope, $state.Status) + if (-not $script:JSON_OUTPUT) { + $installed = if ($state.Status -eq 'multiple') { $state.Rows -join ',' } elseif ($state.Installed) { $state.Installed } else { '-' } + $available = if ($state.Available) { $state.Available } else { '-' } + $scope = if ($state.Scope.Count -gt 0) { $state.Scope -join '+' } else { '-' } + log ($format -f $record.Name, $installed, $available, $state.Package, $scope, $state.Status) + } Add-ToolNote -Tool $record -State $state + $rows += [ordered]@{ + tool = $record.Name + installed = $(if ($state.Installed) { $state.Installed } else { $null }) + available = $(if ($state.Available) { $state.Available } else { $null }) + source = $state.Package + mechanism = 'winget' + status = $state.Status + scope = @($state.Scope) + notes = (Get-NoteText -First $first -Last $script:NOTE_TEXTS.Count) + } } + $last = $script:NOTE_TEXTS.Count if ($script:ELEVATED) { note 'report' 'this pwsh is elevated, and some installers fail when launched from an elevated process, so an unelevated run is the one to prefer' } + if ($script:JSON_OUTPUT) { + [ordered]@{ + schema = 1 + platform = 'windows' + tools = $rows + notes = (Get-NoteText -First $last -Last $script:NOTE_TEXTS.Count) + } | ConvertTo-Json -Depth 4 -EscapeHandling EscapeNonAscii + return + } + if ($script:NOTES.Count -eq 0) { return } log '' log 'Notes:' @@ -1203,8 +1249,11 @@ function Invoke-Apply { function Resolve-Mode { $given = @($script:ACTIONS.Keys | Where-Object { $script:ACTIONS[$_] }) if ($given.Count -gt 1) { die "More than one action given ($($given -join ', ')), name one" } - if ($given.Count -eq 0) { return 'report' } - return $given[0] + $mode = if ($given.Count -eq 0) { 'report' } else { $given[0] } + if ($script:JSON_OUTPUT -and $mode -ne 'report') { + die "-Json changes how a report is written, so it applies only to -Report, and the $mode action was given" + } + return $mode } function Resolve-Selection { diff --git a/tests/test_install_tools.py b/tests/test_install_tools.py new file mode 100755 index 00000000..5356acec --- /dev/null +++ b/tests/test_install_tools.py @@ -0,0 +1,300 @@ +#!/usr/bin/env python3 +"""Tests for the host tool installers' JSON report, on both platforms. + +The JSON report exists so a program can read what the table tells a person, so its contract is the +shape a program parses: every field present on every row, a version that was not read written as +null, and each note filed under the tool that raised it. Each installer's report is driven through +its own functions with the host reads stubbed, since a real report reads the host it runs on. + +Run as `python3 tests/test_install_tools.py`, or under `python3 -m unittest discover -s tests`. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +LINUX_INSTALLER = ROOT / "host-setup" / "linux" / "install-tools.sh" +WINDOWS_INSTALLER = ROOT / "host-setup" / "windows" / "install-tools.ps1" + +AWKWARD = 'a "quoted" back\\slash\ttab\nnewline\rreturn \x01\x1f \u00e9 end' + + +def linux_functions() -> str: + """`install-tools.sh` without its closing `main "$@"`, so a test can source its functions alone.""" + lines = LINUX_INSTALLER.read_text(encoding="utf-8").rstrip("\n").split("\n") + if lines[-1] != 'main "$@"': + raise AssertionError(f"install-tools.sh no longer ends with its main call: {lines[-1]!r}") + return "\n".join(lines[:-1]) + "\n" + + +@unittest.skipUnless(sys.platform == "linux", "drives the Linux installer's own functions") +class TestLinuxJsonReport(unittest.TestCase): + """`install-tools.sh --json`, driven through the script's own functions.""" + + def setUp(self) -> None: + self.dir = Path(self.enterContext(tempfile.TemporaryDirectory())) + self.functions = self.dir / "functions.sh" + self.functions.write_text(linux_functions(), encoding="utf-8") + + def run_bash(self, body: str, argument: str = "") -> subprocess.CompletedProcess[str]: + script = f'source "{self.functions}"\n{body}\n' + return subprocess.run( + ["bash", "-c", script, "bash", argument], + capture_output=True, + text=True, + encoding="utf-8", + check=False, + timeout=30, + ) + + def test_json_string_round_trips_every_escaped_character(self) -> None: + result = self.run_bash('json_string "$1"', AWKWARD) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(json.loads(result.stdout), AWKWARD) + + def test_report_writes_each_row_and_files_each_note(self) -> None: + body = f""" +host_path=$PATH +PATH="{self.dir}/empty" +JSON_OUTPUT=true +SELECTED=(jq extra) +REPO_PACKAGES[extra]=extra-package +jq_version() {{ :; }} +jq_target() {{ printf '1.8.2'; }} +apt_installed_version() {{ printf '2.0'; }} +apt_candidate_version() {{ printf '2.0'; }} +tool_note() {{ note "$1" "$AWKWARD"; }} +AWKWARD=$1 +report +PATH=$host_path +""" + result = self.run_bash(body, AWKWARD) + self.assertEqual(result.returncode, 0, result.stderr) + report = json.loads(result.stdout) + self.assertEqual(report["schema"], 1) + self.assertEqual(report["platform"], "linux") + self.assertEqual( + report["tools"], + [ + { + "tool": "jq", + "installed": None, + "available": "1.8.2", + "source": "jqlang/jq", + "mechanism": "binary", + "status": "missing", + "notes": [AWKWARD], + }, + { + "tool": "extra", + "installed": "2.0", + "available": "2.0", + "source": "apt:extra-package", + "mechanism": "apt", + "status": "current", + "notes": [], + }, + ], + ) + self.assertEqual(len(report["notes"]), 1) + self.assertIn("curl is not installed", report["notes"][0]) + + def test_json_string_drops_bytes_that_are_not_utf8(self) -> None: + never_valid = b"\xff" + above_the_last_code_point = b"\xf4\x90\x80\x80" + overlong = b"\xe0\x80\x80" + surrogate = b"\xed\xa0\x80" + truncated = b"\xe2\x82" + lead_before_ascii = b"\xc3" + malformed = ( + b"a" + + never_valid + + above_the_last_code_point + + overlong + + surrogate + + lead_before_ascii + + "b\u00e9\U0001f600".encode() + + truncated + ) + for locale in ("C", "C.UTF-8"): + with self.subTest(locale=locale): + result = subprocess.run( + [ + "bash", + "-c", + f'source "{self.functions}"\njson_string "$1"', + "bash", + malformed, + ], + capture_output=True, + check=False, + timeout=30, + env={"PATH": os.environ["PATH"], "LC_ALL": locale}, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stderr, b"") + self.assertTrue(result.stdout.isascii(), result.stdout) + self.assertEqual(json.loads(result.stdout), "ab\u00e9\U0001f600") + + def test_docker_inside_wsl_names_docker_desktop_as_its_mechanism(self) -> None: + for is_wsl, expected in (("true", "docker-desktop"), ("false", "apt")): + with self.subTest(is_wsl=is_wsl): + result = self.run_bash(f"IS_WSL={is_wsl}\ntool_mechanism docker") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, expected) + + def test_report_with_no_rows_is_still_an_object(self) -> None: + result = self.run_bash("JSON_OUTPUT=true\nSELECTED=()\nreport") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(json.loads(result.stdout)["tools"], []) + + def test_json_is_refused_beside_another_action(self) -> None: + for action in ("--install", "--upgrade", "--list", "--sudo-timestamp"): + with self.subTest(action=action): + result = subprocess.run( + [str(LINUX_INSTALLER), action, "--json"], + capture_output=True, + text=True, + encoding="utf-8", + check=False, + timeout=30, + ) + self.assertEqual(result.returncode, 1) + self.assertIn("--json", result.stderr) + self.assertEqual(result.stdout, "") + + +WINDOWS_HARNESS = r""" +param([string]$Installer, [string]$Awkward, [string]$Mode) +$ErrorActionPreference = 'Stop' +Set-StrictMode -Version Latest +$tokens = $null +$errors = $null +$ast = [System.Management.Automation.Language.Parser]::ParseFile($Installer, [ref]$tokens, [ref]$errors) +$wanted = @('note', 'Get-NoteText', 'Show-Report', 'Resolve-Mode', 'die') +foreach ($definition in $ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $wanted -contains $node.Name }, $true)) { + . ([scriptblock]::Create($definition.Extent.Text)) +} +function log { param([string]$Message = '') Write-Host $Message } +function info { param([string]$Message) Write-Host " $Message" } +function Get-Tool { param([string]$Name) @{ Name = $Name } } +function Get-ToolState { + param([hashtable]$Tool) + if ($Tool.Name -eq 'jq') { + return @{ Package = 'jqlang.jq'; Installed = $null; Rows = @(); Available = '1.8.2'; Scope = @(); Status = 'missing' } + } + if ($Tool.Name -eq 'dotnet') { + return @{ Package = 'Microsoft.DotNet.SDK.10'; Installed = $null; Rows = @('8.0.1', '10.0.1'); Available = '10.0.1'; Scope = @('machine'); Status = 'multiple' } + } + return @{ Package = 'astral-sh.uv'; Installed = '0.12.4'; Rows = @('0.12.4'); Available = '0.12.4'; Scope = @('user', 'machine'); Status = 'current' } +} +function Add-ToolNote { param([hashtable]$Tool, [hashtable]$State) if ($Tool.Name -eq 'jq') { note 'jq' $Awkward } } +$NOTES = @() +$NOTE_TEXTS = @() +$JSON_OUTPUT = $true +$ELEVATED = $true +$SELECTED = @('jq', 'uv', 'dotnet') +if ($Mode) { + $ACTIONS = [ordered]@{ report = $false; install = $false; list = $false } + $ACTIONS[$Mode] = $true + Resolve-Mode +} else { + Show-Report +} +""" + + +@unittest.skipUnless( + shutil.which("pwsh"), "needs pwsh to drive the Windows installer's own functions" +) +class TestWindowsJsonReport(unittest.TestCase): + """`install-tools.ps1 -Json`, driven through the script's own functions with winget stubbed.""" + + def run_harness(self, mode: str = "") -> subprocess.CompletedProcess[str]: + with tempfile.TemporaryDirectory() as directory: + harness = Path(directory, "harness.ps1") + harness.write_text(WINDOWS_HARNESS, encoding="utf-8") + return subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(harness), + "-Installer", + str(WINDOWS_INSTALLER), + "-Awkward", + AWKWARD, + "-Mode", + mode, + ], + capture_output=True, + text=True, + encoding="utf-8", + check=False, + timeout=120, + ) + + def test_report_writes_each_row_and_files_each_note(self) -> None: + result = self.run_harness() + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue(result.stdout.isascii(), result.stdout) + report = json.loads(result.stdout) + self.assertEqual(report["schema"], 1) + self.assertEqual(report["platform"], "windows") + self.assertEqual( + report["tools"], + [ + { + "tool": "jq", + "installed": None, + "available": "1.8.2", + "source": "jqlang.jq", + "mechanism": "winget", + "status": "missing", + "scope": [], + "notes": [AWKWARD], + }, + { + "tool": "uv", + "installed": "0.12.4", + "available": "0.12.4", + "source": "astral-sh.uv", + "mechanism": "winget", + "status": "current", + "scope": ["user", "machine"], + "notes": [], + }, + { + "tool": "dotnet", + "installed": None, + "available": "10.0.1", + "source": "Microsoft.DotNet.SDK.10", + "mechanism": "winget", + "status": "multiple", + "scope": ["machine"], + "notes": [], + }, + ], + ) + self.assertEqual(len(report["notes"]), 1) + self.assertIn("elevated", report["notes"][0]) + + def test_json_is_refused_beside_another_action(self) -> None: + for mode in ("install", "list"): + with self.subTest(mode=mode): + result = self.run_harness(mode) + self.assertEqual(result.returncode, 1) + self.assertIn("-Json", result.stderr) + + +if __name__ == "__main__": + unittest.main() From 15c70b1b6f0bc5cbca1e6e8c97c1542e34d3594e Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 15:48:26 -0700 Subject: [PATCH 3/7] Keep the Fork Iteration PR Open and Let a Fork Host a Handoff Chain (#2265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #2264 ## Summary Two fork sessions ran `upstream-contribution-workflow` and `session-handoff`, and hit two gaps. **The fork-internal iteration PR.** The skill never said what happens to it, so an agent offered to merge it into the fork's base branch once it was green. The skill now says: - The fork's copy of the upstream base branch is a mirror. It moves only by syncing from upstream. - The iteration PR targets that mirror. It stays open for the life of the contribution and is never merged. - Upstream changes are merged into the dirty branch, not rebased, and the fallout is fixed in the iteration PR. - The clean branch is a plain branch, cut fresh from the mirror tip. - The iteration PR is closed, unmerged, only once upstream merges or declines the upstream PR. A constructed example (upstream moves from `A` to `B`) anchors this. **A fork keeping state in a handoff chain.** When the `handoff` label was missing, `handoff.py` refused and pointed at `configure.sh apply`. That command would put the whole fleet label set and repo config on someone else's project. When the label is missing: | Repository | Behavior | | --- | --- | | Registered fleet repo | Refuses, unchanged | | Another owner | Refuses before any further call | | Unregistered repo of the owner's, not a fork | Refuses as registry drift | | Unregistered fork under the owner | Reads warn and answer as an empty chain does. `new` refuses until given `--create-label` | `--create-label` creates only the `handoff` label, using the definition in `repo-config/labels.json`, reads it back to confirm, then files the first link. If the fork has issues turned off, it stops before any write. `session-handoff`, `scripts/README.md`, and the carried `AGENTS.md` "Session Scope" sentence are updated to match. The `AGENTS.md` section is carried at verbatim fidelity, so fleet repos pick the change up on their next resync. ## Verification - `tests/test_handoff.py`: 134 tests, 18 of them new. Mutation checks (owner guard, fork guard, label read-back, dry run, case-insensitive compare, label definition) each fail a named test. - The full `unittest discover` run passes (1937 tests). - ruff, mypy, `prose_lint --diff origin/develop`, `spec/validate.py`, `build_dist.py --check`, and `docker_lint.py` are all clean. - `local-strict-review`: three passes. The first two raised 8 and 6 findings. All were fixed except one race, which is declined because it is safe and visible. The third pass raised none, and its receipt is recorded. - Live read-only check: `handoff.py tracks --repo ptr727/liquidraver-ZephCore` still lists its open handoff. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- .agents/skills/session-handoff/SKILL.md | 19 +- .../upstream-contribution-workflow/SKILL.md | 63 +++- .../.source-digests/session-handoff | 2 +- .../upstream-contribution-workflow | 2 +- .../skills/session-handoff/SKILL.md | 19 +- .../upstream-contribution-workflow/SKILL.md | 63 +++- .github/skills/session-handoff/SKILL.md | 19 +- .../upstream-contribution-workflow/SKILL.md | 63 +++- AGENTS.md | 2 +- scripts/README.md | 2 +- scripts/handoff.py | 196 ++++++++++- tests/test_handoff.py | 304 +++++++++++++++++- 12 files changed, 682 insertions(+), 72 deletions(-) diff --git a/.agents/skills/session-handoff/SKILL.md b/.agents/skills/session-handoff/SKILL.md index ee98d9cb..5dd83ca7 100644 --- a/.agents/skills/session-handoff/SKILL.md +++ b/.agents/skills/session-handoff/SKILL.md @@ -308,13 +308,24 @@ It is accepted by `new` and `link`, the two subcommands that write, and by no ot Exit `0` is success, `1` a refusal the caller can act on, a usage error included, and `2` the command not having run to an answer, so a refusal and a failure to reach one never share a code. A -repository missing the `handoff` label is a refusal rather than a degraded empty answer, and it -names the command that applies the fleet label set except where the label read filled its window, -which is the one case where the label's absence is unproven rather than established. +fleet repository missing the `handoff` label, one the hub's `registry/repos.json` lists, is a +refusal rather than a degraded empty answer, and it names the command that applies the fleet label +set except where the label read filled its window, which is the one case where the label's absence +is unproven rather than established. + +A fork under the registry's owner that the registry does not list, such as one kept for an +upstream contribution per `upstream-contribution-workflow`, can host a chain to keep a session's +state without taking on any fleet configuration. Missing the label there, a read prints a warning +and answers as an empty chain does, and `new` refuses until it is given `--create-label`, which +creates the one `handoff` label, confirms it, and then files the first link. Issues turned off stop +it before any write, naming the command that turns them on. Never apply the fleet label set to such +a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, +refuses before any write. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than -being a write of its own. `link` edits a body, comments, and closes. Each of them is bound by +being a write of its own, and `new --create-label` adds one write ahead of those, the label itself. +`link` edits a body, comments, and closes. Each of them is bound by `GOVERNANCE.md` "Repository Boundaries and Write Safety" exactly as any other write is. Point them at the repository `AGENTS.md` "Session Scope" sends the link to, the one holding the work the next session resumes, and at no other. `link` also reaches an issue this chain never created, since the diff --git a/.agents/skills/upstream-contribution-workflow/SKILL.md b/.agents/skills/upstream-contribution-workflow/SKILL.md index 65458614..6ba840d6 100644 --- a/.agents/skills/upstream-contribution-workflow/SKILL.md +++ b/.agents/skills/upstream-contribution-workflow/SKILL.md @@ -3,12 +3,14 @@ name: upstream-contribution-workflow description: >- Governs how the maintainer contributes to a third-party repository he does not control (for example esphome/esphome), distinct from the fleet's own internal branching model: a dirty work - branch on his own fork for the actual work and review iteration, squashed once clean to a second - branch that carries only the intended minimal history, that clean branch opened as the PR - against the upstream repo, and reviewer feedback applied to the dirty branch first, then - re-squashed into the clean one. Use this whenever about to open a pull request against a - repository outside the ptr727 fleet, whenever forking a third-party project to contribute a fix - or feature, whenever an upstream reviewer requests changes on a PR opened this way, and whenever + branch on his own fork for the actual work and review iteration, its fork-internal PR kept open + and never merged, squashed once clean to a second branch that carries only the intended minimal + history, that clean branch opened as the PR against the upstream repo, and reviewer feedback + applied to the dirty branch first, then re-squashed into the clean one. Use this whenever about + to open a pull request against a repository outside the ptr727 fleet, whenever forking a + third-party project to contribute a fix or feature, whenever an upstream reviewer requests + changes on a PR opened this way, whenever a fork-internal iteration PR looks ready to merge or + close, whenever the upstream base branch moves under a contribution in flight, and whenever deciding which issue or PR template to use for a third-party repository. Triggers regardless of the target repo's own type or workflow model, since this skill is about the shape of a contribution to someone else's repo, not the target repo's own internal conventions, which this @@ -29,15 +31,24 @@ fleet's internal model so the two are never conflated. ## The two-branch shape -1. **Fork the upstream repo**, if not already forked. +1. **Fork the upstream repo**, if not already forked. The fork's copy of the upstream base branch + (`main`, `dev`, whichever upstream PRs target) is the **mirror branch**. It moves only by + syncing from upstream, and nothing of the maintainer's is ever merged into it. 2. **Do the actual work on a dirty work branch**, on the maintainer's own fork. This branch is allowed to be messy: false starts, fixup commits, back-and-forth in response to review, whatever - the real work looks like while it's happening. Open a PR from this branch into a branch on the - maintainer's **own fork** (not upstream), so all the iteration happens there, visible and - reviewable, without touching the upstream repo at all. + the real work looks like while it's happening. Open a PR from this branch into the mirror + branch on the maintainer's **own fork** (not upstream), so all the iteration happens there, + visible and reviewable by bots and the maintainer, without touching the upstream repo at all. + **That fork-internal PR is the iteration record, never a merge candidate.** It stays open for + the life of the contribution, however green it gets, and is never merged, per "The iteration PR + is never merged" below. 3. **Once the dirty branch is clean and the change is ready, squash it to a second branch** that carries only the intended, minimal commit history, one commit (or a small, deliberate set) that - states what the change is, not how it was arrived at. + states what the change is, not how it was arrived at. Cut that branch fresh from the mirror + branch's current tip and squash the dirty branch's changes onto it. It is a plain branch, not a + PR on the fork, since the review it needs already happened on the iteration PR. One dirty branch + may feed several clean branches when upstream wants the change split, each carrying only its + own slice. 4. **Open the PR against the upstream repo from that second, clean branch.** This is the only branch upstream ever sees. An upstream draft may be opened only after that clean presentation branch exists and is published. When more preparation is needed, continue on the dirty branch, @@ -63,6 +74,29 @@ fleet's internal model so the two are never conflated. Never reverse this: never iterate directly on the branch that's open against upstream, and never skip the squash step because the dirty branch "looks clean enough." +## The iteration PR is never merged + +Merging the iteration PR into the mirror branch looks like the natural finish once it is green, and +it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it stops +being a mirror: it can no longer be fast-forwarded from upstream, every later sync has to merge +upstream into the maintainer's own change, and any sync touching the same lines conflicts. And the +iteration PR is the place where upstream drift is absorbed and its fallout fixed before the +upstream PR has to absorb the same drift, so merging or closing it removes that place. + +When the upstream base moves, sync the mirror branch from upstream, merge the mirror branch into +the dirty branch, and fix whatever breaks in the iteration PR. That is a merge rather than a rebase, +since the dirty branch is append-only and is never force-pushed. Then re-squash onto the new mirror +tip whenever the clean branch needs to follow, per step 5. + +For example, the fork's mirror branch and upstream both sit at commit `A`, with the iteration PR +from `work/x` into the mirror branch green. Upstream advances to `B`. The fork syncs its mirror +branch to `B`, merges `B` into `work/x`, and fixes the breakage in the iteration PR. Had `work/x` +been merged into the mirror branch at `A`, the mirror branch could not have fast-forwarded to `B`. + +The contribution ends when upstream merges or declines the upstream PR. Only then close the +iteration PR, unmerged, and delete the dirty and clean branches. A merged change then reaches the +mirror branch through the next sync from upstream, the same way anyone else's change does. + ## Use the upstream repo's own conventions, not the fleet's Always use the upstream repo's own issue and PR templates, its own contribution guidelines, and @@ -82,3 +116,10 @@ write-safety rules (never write to a repository outside explicit authorization, GitHub id) also still apply in full. A fork the maintainer owns is within scope to push to, and the upstream repository itself is written to only through the PR the maintainer explicitly asked for. + +The fork is not a fleet repository. It carries none of the fleet's instruction files or repository +settings, and a resync or the fleet label set is never applied to it. A session working +the contribution can still keep its state in the `session-handoff` chain on the fork. That needs +issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new +--create-label` creates on a fork under the fleet's owner, and nothing else of the fleet's +configuration. diff --git a/.claude-plugin/fleet-skills/.source-digests/session-handoff b/.claude-plugin/fleet-skills/.source-digests/session-handoff index cb8a9fd8..24500fc8 100644 --- a/.claude-plugin/fleet-skills/.source-digests/session-handoff +++ b/.claude-plugin/fleet-skills/.source-digests/session-handoff @@ -1 +1 @@ -8dfe7946b9693ac7 +c1684259d7d09c94 diff --git a/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow b/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow index baf4d808..3ab96a45 100644 --- a/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow +++ b/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow @@ -1 +1 @@ -f98c8047451b8448 +1436e605eb7e114f diff --git a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md index ee98d9cb..5dd83ca7 100644 --- a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md @@ -308,13 +308,24 @@ It is accepted by `new` and `link`, the two subcommands that write, and by no ot Exit `0` is success, `1` a refusal the caller can act on, a usage error included, and `2` the command not having run to an answer, so a refusal and a failure to reach one never share a code. A -repository missing the `handoff` label is a refusal rather than a degraded empty answer, and it -names the command that applies the fleet label set except where the label read filled its window, -which is the one case where the label's absence is unproven rather than established. +fleet repository missing the `handoff` label, one the hub's `registry/repos.json` lists, is a +refusal rather than a degraded empty answer, and it names the command that applies the fleet label +set except where the label read filled its window, which is the one case where the label's absence +is unproven rather than established. + +A fork under the registry's owner that the registry does not list, such as one kept for an +upstream contribution per `upstream-contribution-workflow`, can host a chain to keep a session's +state without taking on any fleet configuration. Missing the label there, a read prints a warning +and answers as an empty chain does, and `new` refuses until it is given `--create-label`, which +creates the one `handoff` label, confirms it, and then files the first link. Issues turned off stop +it before any write, naming the command that turns them on. Never apply the fleet label set to such +a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, +refuses before any write. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than -being a write of its own. `link` edits a body, comments, and closes. Each of them is bound by +being a write of its own, and `new --create-label` adds one write ahead of those, the label itself. +`link` edits a body, comments, and closes. Each of them is bound by `GOVERNANCE.md` "Repository Boundaries and Write Safety" exactly as any other write is. Point them at the repository `AGENTS.md` "Session Scope" sends the link to, the one holding the work the next session resumes, and at no other. `link` also reaches an issue this chain never created, since the diff --git a/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md b/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md index 65458614..6ba840d6 100644 --- a/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md @@ -3,12 +3,14 @@ name: upstream-contribution-workflow description: >- Governs how the maintainer contributes to a third-party repository he does not control (for example esphome/esphome), distinct from the fleet's own internal branching model: a dirty work - branch on his own fork for the actual work and review iteration, squashed once clean to a second - branch that carries only the intended minimal history, that clean branch opened as the PR - against the upstream repo, and reviewer feedback applied to the dirty branch first, then - re-squashed into the clean one. Use this whenever about to open a pull request against a - repository outside the ptr727 fleet, whenever forking a third-party project to contribute a fix - or feature, whenever an upstream reviewer requests changes on a PR opened this way, and whenever + branch on his own fork for the actual work and review iteration, its fork-internal PR kept open + and never merged, squashed once clean to a second branch that carries only the intended minimal + history, that clean branch opened as the PR against the upstream repo, and reviewer feedback + applied to the dirty branch first, then re-squashed into the clean one. Use this whenever about + to open a pull request against a repository outside the ptr727 fleet, whenever forking a + third-party project to contribute a fix or feature, whenever an upstream reviewer requests + changes on a PR opened this way, whenever a fork-internal iteration PR looks ready to merge or + close, whenever the upstream base branch moves under a contribution in flight, and whenever deciding which issue or PR template to use for a third-party repository. Triggers regardless of the target repo's own type or workflow model, since this skill is about the shape of a contribution to someone else's repo, not the target repo's own internal conventions, which this @@ -29,15 +31,24 @@ fleet's internal model so the two are never conflated. ## The two-branch shape -1. **Fork the upstream repo**, if not already forked. +1. **Fork the upstream repo**, if not already forked. The fork's copy of the upstream base branch + (`main`, `dev`, whichever upstream PRs target) is the **mirror branch**. It moves only by + syncing from upstream, and nothing of the maintainer's is ever merged into it. 2. **Do the actual work on a dirty work branch**, on the maintainer's own fork. This branch is allowed to be messy: false starts, fixup commits, back-and-forth in response to review, whatever - the real work looks like while it's happening. Open a PR from this branch into a branch on the - maintainer's **own fork** (not upstream), so all the iteration happens there, visible and - reviewable, without touching the upstream repo at all. + the real work looks like while it's happening. Open a PR from this branch into the mirror + branch on the maintainer's **own fork** (not upstream), so all the iteration happens there, + visible and reviewable by bots and the maintainer, without touching the upstream repo at all. + **That fork-internal PR is the iteration record, never a merge candidate.** It stays open for + the life of the contribution, however green it gets, and is never merged, per "The iteration PR + is never merged" below. 3. **Once the dirty branch is clean and the change is ready, squash it to a second branch** that carries only the intended, minimal commit history, one commit (or a small, deliberate set) that - states what the change is, not how it was arrived at. + states what the change is, not how it was arrived at. Cut that branch fresh from the mirror + branch's current tip and squash the dirty branch's changes onto it. It is a plain branch, not a + PR on the fork, since the review it needs already happened on the iteration PR. One dirty branch + may feed several clean branches when upstream wants the change split, each carrying only its + own slice. 4. **Open the PR against the upstream repo from that second, clean branch.** This is the only branch upstream ever sees. An upstream draft may be opened only after that clean presentation branch exists and is published. When more preparation is needed, continue on the dirty branch, @@ -63,6 +74,29 @@ fleet's internal model so the two are never conflated. Never reverse this: never iterate directly on the branch that's open against upstream, and never skip the squash step because the dirty branch "looks clean enough." +## The iteration PR is never merged + +Merging the iteration PR into the mirror branch looks like the natural finish once it is green, and +it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it stops +being a mirror: it can no longer be fast-forwarded from upstream, every later sync has to merge +upstream into the maintainer's own change, and any sync touching the same lines conflicts. And the +iteration PR is the place where upstream drift is absorbed and its fallout fixed before the +upstream PR has to absorb the same drift, so merging or closing it removes that place. + +When the upstream base moves, sync the mirror branch from upstream, merge the mirror branch into +the dirty branch, and fix whatever breaks in the iteration PR. That is a merge rather than a rebase, +since the dirty branch is append-only and is never force-pushed. Then re-squash onto the new mirror +tip whenever the clean branch needs to follow, per step 5. + +For example, the fork's mirror branch and upstream both sit at commit `A`, with the iteration PR +from `work/x` into the mirror branch green. Upstream advances to `B`. The fork syncs its mirror +branch to `B`, merges `B` into `work/x`, and fixes the breakage in the iteration PR. Had `work/x` +been merged into the mirror branch at `A`, the mirror branch could not have fast-forwarded to `B`. + +The contribution ends when upstream merges or declines the upstream PR. Only then close the +iteration PR, unmerged, and delete the dirty and clean branches. A merged change then reaches the +mirror branch through the next sync from upstream, the same way anyone else's change does. + ## Use the upstream repo's own conventions, not the fleet's Always use the upstream repo's own issue and PR templates, its own contribution guidelines, and @@ -82,3 +116,10 @@ write-safety rules (never write to a repository outside explicit authorization, GitHub id) also still apply in full. A fork the maintainer owns is within scope to push to, and the upstream repository itself is written to only through the PR the maintainer explicitly asked for. + +The fork is not a fleet repository. It carries none of the fleet's instruction files or repository +settings, and a resync or the fleet label set is never applied to it. A session working +the contribution can still keep its state in the `session-handoff` chain on the fork. That needs +issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new +--create-label` creates on a fork under the fleet's owner, and nothing else of the fleet's +configuration. diff --git a/.github/skills/session-handoff/SKILL.md b/.github/skills/session-handoff/SKILL.md index ee98d9cb..5dd83ca7 100644 --- a/.github/skills/session-handoff/SKILL.md +++ b/.github/skills/session-handoff/SKILL.md @@ -308,13 +308,24 @@ It is accepted by `new` and `link`, the two subcommands that write, and by no ot Exit `0` is success, `1` a refusal the caller can act on, a usage error included, and `2` the command not having run to an answer, so a refusal and a failure to reach one never share a code. A -repository missing the `handoff` label is a refusal rather than a degraded empty answer, and it -names the command that applies the fleet label set except where the label read filled its window, -which is the one case where the label's absence is unproven rather than established. +fleet repository missing the `handoff` label, one the hub's `registry/repos.json` lists, is a +refusal rather than a degraded empty answer, and it names the command that applies the fleet label +set except where the label read filled its window, which is the one case where the label's absence +is unproven rather than established. + +A fork under the registry's owner that the registry does not list, such as one kept for an +upstream contribution per `upstream-contribution-workflow`, can host a chain to keep a session's +state without taking on any fleet configuration. Missing the label there, a read prints a warning +and answers as an empty chain does, and `new` refuses until it is given `--create-label`, which +creates the one `handoff` label, confirms it, and then files the first link. Issues turned off stop +it before any write, naming the command that turns them on. Never apply the fleet label set to such +a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, +refuses before any write. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than -being a write of its own. `link` edits a body, comments, and closes. Each of them is bound by +being a write of its own, and `new --create-label` adds one write ahead of those, the label itself. +`link` edits a body, comments, and closes. Each of them is bound by `GOVERNANCE.md` "Repository Boundaries and Write Safety" exactly as any other write is. Point them at the repository `AGENTS.md` "Session Scope" sends the link to, the one holding the work the next session resumes, and at no other. `link` also reaches an issue this chain never created, since the diff --git a/.github/skills/upstream-contribution-workflow/SKILL.md b/.github/skills/upstream-contribution-workflow/SKILL.md index 65458614..6ba840d6 100644 --- a/.github/skills/upstream-contribution-workflow/SKILL.md +++ b/.github/skills/upstream-contribution-workflow/SKILL.md @@ -3,12 +3,14 @@ name: upstream-contribution-workflow description: >- Governs how the maintainer contributes to a third-party repository he does not control (for example esphome/esphome), distinct from the fleet's own internal branching model: a dirty work - branch on his own fork for the actual work and review iteration, squashed once clean to a second - branch that carries only the intended minimal history, that clean branch opened as the PR - against the upstream repo, and reviewer feedback applied to the dirty branch first, then - re-squashed into the clean one. Use this whenever about to open a pull request against a - repository outside the ptr727 fleet, whenever forking a third-party project to contribute a fix - or feature, whenever an upstream reviewer requests changes on a PR opened this way, and whenever + branch on his own fork for the actual work and review iteration, its fork-internal PR kept open + and never merged, squashed once clean to a second branch that carries only the intended minimal + history, that clean branch opened as the PR against the upstream repo, and reviewer feedback + applied to the dirty branch first, then re-squashed into the clean one. Use this whenever about + to open a pull request against a repository outside the ptr727 fleet, whenever forking a + third-party project to contribute a fix or feature, whenever an upstream reviewer requests + changes on a PR opened this way, whenever a fork-internal iteration PR looks ready to merge or + close, whenever the upstream base branch moves under a contribution in flight, and whenever deciding which issue or PR template to use for a third-party repository. Triggers regardless of the target repo's own type or workflow model, since this skill is about the shape of a contribution to someone else's repo, not the target repo's own internal conventions, which this @@ -29,15 +31,24 @@ fleet's internal model so the two are never conflated. ## The two-branch shape -1. **Fork the upstream repo**, if not already forked. +1. **Fork the upstream repo**, if not already forked. The fork's copy of the upstream base branch + (`main`, `dev`, whichever upstream PRs target) is the **mirror branch**. It moves only by + syncing from upstream, and nothing of the maintainer's is ever merged into it. 2. **Do the actual work on a dirty work branch**, on the maintainer's own fork. This branch is allowed to be messy: false starts, fixup commits, back-and-forth in response to review, whatever - the real work looks like while it's happening. Open a PR from this branch into a branch on the - maintainer's **own fork** (not upstream), so all the iteration happens there, visible and - reviewable, without touching the upstream repo at all. + the real work looks like while it's happening. Open a PR from this branch into the mirror + branch on the maintainer's **own fork** (not upstream), so all the iteration happens there, + visible and reviewable by bots and the maintainer, without touching the upstream repo at all. + **That fork-internal PR is the iteration record, never a merge candidate.** It stays open for + the life of the contribution, however green it gets, and is never merged, per "The iteration PR + is never merged" below. 3. **Once the dirty branch is clean and the change is ready, squash it to a second branch** that carries only the intended, minimal commit history, one commit (or a small, deliberate set) that - states what the change is, not how it was arrived at. + states what the change is, not how it was arrived at. Cut that branch fresh from the mirror + branch's current tip and squash the dirty branch's changes onto it. It is a plain branch, not a + PR on the fork, since the review it needs already happened on the iteration PR. One dirty branch + may feed several clean branches when upstream wants the change split, each carrying only its + own slice. 4. **Open the PR against the upstream repo from that second, clean branch.** This is the only branch upstream ever sees. An upstream draft may be opened only after that clean presentation branch exists and is published. When more preparation is needed, continue on the dirty branch, @@ -63,6 +74,29 @@ fleet's internal model so the two are never conflated. Never reverse this: never iterate directly on the branch that's open against upstream, and never skip the squash step because the dirty branch "looks clean enough." +## The iteration PR is never merged + +Merging the iteration PR into the mirror branch looks like the natural finish once it is green, and +it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it stops +being a mirror: it can no longer be fast-forwarded from upstream, every later sync has to merge +upstream into the maintainer's own change, and any sync touching the same lines conflicts. And the +iteration PR is the place where upstream drift is absorbed and its fallout fixed before the +upstream PR has to absorb the same drift, so merging or closing it removes that place. + +When the upstream base moves, sync the mirror branch from upstream, merge the mirror branch into +the dirty branch, and fix whatever breaks in the iteration PR. That is a merge rather than a rebase, +since the dirty branch is append-only and is never force-pushed. Then re-squash onto the new mirror +tip whenever the clean branch needs to follow, per step 5. + +For example, the fork's mirror branch and upstream both sit at commit `A`, with the iteration PR +from `work/x` into the mirror branch green. Upstream advances to `B`. The fork syncs its mirror +branch to `B`, merges `B` into `work/x`, and fixes the breakage in the iteration PR. Had `work/x` +been merged into the mirror branch at `A`, the mirror branch could not have fast-forwarded to `B`. + +The contribution ends when upstream merges or declines the upstream PR. Only then close the +iteration PR, unmerged, and delete the dirty and clean branches. A merged change then reaches the +mirror branch through the next sync from upstream, the same way anyone else's change does. + ## Use the upstream repo's own conventions, not the fleet's Always use the upstream repo's own issue and PR templates, its own contribution guidelines, and @@ -82,3 +116,10 @@ write-safety rules (never write to a repository outside explicit authorization, GitHub id) also still apply in full. A fork the maintainer owns is within scope to push to, and the upstream repository itself is written to only through the PR the maintainer explicitly asked for. + +The fork is not a fleet repository. It carries none of the fleet's instruction files or repository +settings, and a resync or the fleet label set is never applied to it. A session working +the contribution can still keep its state in the `session-handoff` chain on the fork. That needs +issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new +--create-label` creates on a fork under the fleet's owner, and nothing else of the fleet's +configuration. diff --git a/AGENTS.md b/AGENTS.md index 5ffbb17f..22d505ab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,7 +37,7 @@ An agent session is billed on the context it carries, not the work it does. Ever - **One deliverable, one session.** A session covers one branch and one deliverable, and ends when that work merges. A multi-step task is one deliverable and stays in one session. Two unrelated tasks are two sessions even when they run back to back. - **End a session at any of these, without being asked:** the branch changes, the pull request merges, or the next task is unrelated to the last. A review round is none of them. A loop still producing findings is the deliverable in progress, and a round count is not a reason to leave one open. - **A session orchestrating dispatched work is an exception, and a narrow one.** Its deliverable is the run rather than any branch, so it spans many branches and many merges by construction, and ending it at the first dispatched merge would end the run. The triggers above land on each dispatched task instead, one branch and one deliverable each, which is this rule applied rather than waived. What keeps the exception narrow is that such a session holds no branch of its own and authors none of the work it dispatches, so the file context every other session accumulates is context it never takes on, and it re-derives each round's state from live sources rather than holding it, per "Re-derive state, do not carry it" below. A session that starts editing the files a dispatched task would have edited is an ordinary one again and ends on the triggers above. -- **Hand off in an issue, never in a scratch file and never in context.** Close a session by filing the next link in the handoff chain of the repository holding the work the next session resumes, which for a session that stayed in one repository is this one, and a session that spanned several names the others in the handoff's state section. A track is a lane of work named by a short slug, `default` where a session names none, and a track in use holds exactly one open issue carrying the `handoff` label. The track and the predecessor are recorded in the issue body rather than in its title, so a retitled or hand-edited issue still chains and a reader can tell which lane an open issue belongs to. The new link names its predecessor that way, a forward-link comment then goes onto that predecessor, and the predecessor is closed last, in that order, so a failure part way leaves a discoverable new issue rather than a closed chain with no successor. Each of those three is an outward-facing write, so the write-safety rules in `GOVERNANCE.md` "Repository Boundaries and Write Safety" bind all three exactly as they bind any other write, the identifier rule most of all, since the link a comment and a close target is read live in the same run rather than remembered. The chain needs the `handoff` label to be findable at all, so a repository not carrying it cannot host one until the fleet label set is applied there, which is a change to that repository's configuration rather than anything a handoff writes. A session that cannot file a link reports that it could not hand off and leaves the previous link open, whether it is stopped by a repository carrying no such label, by a write it may not make, or by anything else. That is the one alternative this rule allows to a track still in use, and it is a report rather than a file, because a report says the round's record is missing while a file claims to be it. A track whose work is complete is closed out instead, its last link carrying the outcome as a comment and closed with no successor, which leaves the track no longer in use rather than breaking its chain. A scratch file fails three ways the chain closes. It is not found where the next session looks. More than one candidate is found and nothing says which is current. And it holds no history, so a later round re-runs a path an earlier round already tried and already wrote down, which is the one thing a handoff exists to prevent. The handoff carries the next steps in priority order, the external blockers and internal dependencies among them, the state a resume re-reads rather than trusts, listed so the resume knows what to re-read, the account of parked decisions that `GOVERNANCE.md` "Communicating with the User" requires, what the last round did, what not to repeat, and what was learned. That section states the account whole, and it requires the session to present those decisions as well as record them. **The size rule is stated per section.** An entry earns its place by being specific enough to change a later session's behavior, a section ranks what it keeps and drops whatever does not meet that bar, and what belongs somewhere durable goes there and appears here as one line and a pointer, a defect as an issue, a rule as rule text, a lesson as governance prose. The parked-decision account keeps the count and the ranked questions one round can carry, and names every issue past those by number alone, which is what keeps a queue larger than one round inside this rule. A summary held in context is re-billed until the session ends, a scratch file is read only on the machine holding it, and a closed link stays readable from any machine to every session after it. +- **Hand off in an issue, never in a scratch file and never in context.** Close a session by filing the next link in the handoff chain of the repository holding the work the next session resumes, which for a session that stayed in one repository is this one, and a session that spanned several names the others in the handoff's state section. A track is a lane of work named by a short slug, `default` where a session names none, and a track in use holds exactly one open issue carrying the `handoff` label. The track and the predecessor are recorded in the issue body rather than in its title, so a retitled or hand-edited issue still chains and a reader can tell which lane an open issue belongs to. The new link names its predecessor that way, a forward-link comment then goes onto that predecessor, and the predecessor is closed last, in that order, so a failure part way leaves a discoverable new issue rather than a closed chain with no successor. Each of those three is an outward-facing write, so the write-safety rules in `GOVERNANCE.md` "Repository Boundaries and Write Safety" bind all three exactly as they bind any other write, the identifier rule most of all, since the link a comment and a close target is read live in the same run rather than remembered. The chain needs the `handoff` label to be findable at all, so a repository not carrying it cannot host one until the label exists there, which on a fleet repository is a change to that repository's configuration rather than anything a handoff writes. A session that cannot file a link reports that it could not hand off and leaves the previous link open, whether it is stopped by a repository carrying no such label, by a write it may not make, or by anything else. That is the one alternative this rule allows to a track still in use, and it is a report rather than a file, because a report says the round's record is missing while a file claims to be it. A track whose work is complete is closed out instead, its last link carrying the outcome as a comment and closed with no successor, which leaves the track no longer in use rather than breaking its chain. A scratch file fails three ways the chain closes. It is not found where the next session looks. More than one candidate is found and nothing says which is current. And it holds no history, so a later round re-runs a path an earlier round already tried and already wrote down, which is the one thing a handoff exists to prevent. The handoff carries the next steps in priority order, the external blockers and internal dependencies among them, the state a resume re-reads rather than trusts, listed so the resume knows what to re-read, the account of parked decisions that `GOVERNANCE.md` "Communicating with the User" requires, what the last round did, what not to repeat, and what was learned. That section states the account whole, and it requires the session to present those decisions as well as record them. **The size rule is stated per section.** An entry earns its place by being specific enough to change a later session's behavior, a section ranks what it keeps and drops whatever does not meet that bar, and what belongs somewhere durable goes there and appears here as one line and a pointer, a defect as an issue, a rule as rule text, a lesson as governance prose. The parked-decision account keeps the count and the ranked questions one round can carry, and names every issue past those by number alone, which is what keeps a queue larger than one round inside this rule. A summary held in context is re-billed until the session ends, a scratch file is read only on the machine holding it, and a closed link stays readable from any machine to every session after it. - **Re-derive state, do not carry it.** "This session already has the context" is the signal to split, not to continue. Context that has gone stale is worse than absent, because a file read hundreds of requests ago no longer describes the file. - **Compaction is a fallback, not the strategy.** It restarts context from a floor and climbs again, where a fresh session starts from zero. diff --git a/scripts/README.md b/scripts/README.md index 3c825cd0..17fd3c05 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -261,7 +261,7 @@ The invariant is one open handoff per track per repository, read from the metada `new` creates the new issue, comments the forward link on the previous one, and then closes it, in that order, printing each step's own result. Creating first means a failure at any later step leaves a discoverable new issue rather than a closed chain with no successor, and `link` finishes what a failure interrupted without a second issue being filed for it. `link` takes no `--track`, so the two issues' own metadata blocks are the only thing that can say they belong to one lane, and it refuses an issue named as its own predecessor and a pair whose blocks name different tracks rather than closing one lane's handoff into another's. Nothing suppresses a write's output or converts a failure into success, per [GOVERNANCE.md][governance] "Repository Boundaries and Write Safety". Where the caller names an issue, which is `link` alone, that issue is read live before the write and only what the read returned is written back, and every other identifier a write consumes comes from a read in the same run rather than being constructed. A close is confirmed by reading the state back, because a write that appears to have failed may have succeeded on the server. -`--repo` is required and carries no default, for the reason `pr_review.py` states about a pull request number: an issue number resolves in every repository, and a chain read out of the wrong one is well formed. `--track` defaults to `default`, and omitting it on a named lane does more than read the wrong chain, since `new` then files onto `default` and comments on and closes whatever that lane had open. A missing `handoff` label is a refusal naming `repo-config/configure.sh apply`, never a degraded empty answer, because `decision` was declared on the hub and applied nowhere else and every downstream session then enumerated an empty queue and reported it healthy. Exit `0` is success, `1` a refusal the caller can act on, and `2` the command not having run to an answer, so the two never share a code: a usage error is moved off argparse's own `2` onto `1` because it is a refusal, and an exception nobody modeled is caught onto `2` rather than reaching CPython's own `1`. A body over 12 KB warns and one over 60 KB refuses, under GitHub's own 65536-character limit, and both are backstops against a body GitHub would reject: what bounds a handoff is the Skill's per-section rules. +`--repo` is required and carries no default, for the reason `pr_review.py` states about a pull request number: an issue number resolves in every repository, and a chain read out of the wrong one is well formed. `--track` defaults to `default`, and omitting it on a named lane does more than read the wrong chain, since `new` then files onto `default` and comments on and closes whatever that lane had open. A fleet repository missing the `handoff` label refuses, naming `repo-config/configure.sh apply`, never with a degraded empty answer, because `decision` was declared on the hub and applied nowhere else and every downstream session then enumerated an empty queue and reported it healthy. The one exception is a fork under the registry's owner that the registry does not list, such as one kept for an upstream contribution, where no issue can carry a label that does not exist: a read there warns and answers as an empty chain does, and `new --create-label` creates the one `handoff` label from `repo-config/labels.json`, confirms it, and files the first link, refusing first where the fork has issues turned off. A repository missing the label under another owner, or an unregistered one of the owner's that is not a fork, refuses before any write, the first because these calls run as subprocesses a write guard on the caller never sees and the second because it is registry drift a lone label would hide. Exit `0` is success, `1` a refusal the caller can act on, and `2` the command not having run to an answer, so the two never share a code: a usage error is moved off argparse's own `2` onto `1` because it is a refusal, and an exception nobody modeled is caught onto `2` rather than reaching CPython's own `1`. A body over 12 KB warns and one over 60 KB refuses, under GitHub's own 65536-character limit, and both are backstops against a body GitHub would reject: what bounds a handoff is the Skill's per-section rules. `--dry-run` prints the calls a writing subcommand would make and sends none of them, which is the first thing anyone wants before trusting this to close an issue. It sits on the one path every call goes through rather than on each write separately. diff --git a/scripts/handoff.py b/scripts/handoff.py index 74812810..efa19b31 100755 --- a/scripts/handoff.py +++ b/scripts/handoff.py @@ -39,9 +39,10 @@ Exit codes 0 the command did what it says. - 1 a refusal the caller can act on: the label is missing from the repository, no handoff is - open on the track, a track is ambiguous, a handoff carries no metadata block, a body is - over the hard cap, or the command line itself was wrong. A usage error is a refusal, so + 1 a refusal the caller can act on: the label is missing, other than where `tracks` answers + an unregistered fork under the fleet's owner or `new --create-label` creates the label there, + no handoff is open on the track, a track is ambiguous, a handoff carries no metadata block, a + body is over the hard cap, or the command line itself was wrong. A usage error is a refusal, so `Parser` below moves it here off argparse's own 2. 2 the command did not run to an answer: `gh` failed, a write did not confirm, or an exception nobody modeled reached the top. A refusal and a failure to reach one never share a code, @@ -52,12 +53,21 @@ python3 scripts/handoff.py resume --repo OWNER/NAME [--track T] [--history N] python3 scripts/handoff.py chain --repo OWNER/NAME [--track T] [--limit N] [--grep PATTERN] python3 scripts/handoff.py new --repo OWNER/NAME [--track T] --title S --body-file PATH + [--create-label] python3 scripts/handoff.py link --repo OWNER/NAME --new N --previous N python3 scripts/handoff.py tracks --repo OWNER/NAME Every subcommand takes `--repo`, with no default, for the reason `pr_review.py` requires one: an issue number resolves in every repository, and a chain read out of the wrong one is well formed. Add `--dry-run` to any writing subcommand to print the calls it would make and write nothing. + +A repository missing the label is drift on a fleet repository, one `registry/repos.json` lists, and +every subcommand refuses there. A fork under the registry's owner that the registry does not list, +such as one kept for an upstream contribution, can still use a chain to hold state, and the fleet +label set does not belong on it. There the reads warn and answer as an empty chain does, and +`new --create-label` creates the one `handoff` label and nothing else. Every other repository +missing the label refuses, a repository under another owner and an unregistered repository of the +owner's that is not a fork among them. """ from __future__ import annotations @@ -84,6 +94,11 @@ # A `handoff` label declared and not applied fails the same silent way, so this refuses instead. APPLY = "repo-config/configure.sh apply OWNER/NAME release|operational" +REGISTRY = Path(__file__).resolve().parent.parent / "registry" / "repos.json" +LABELS = Path(__file__).resolve().parent.parent / "repo-config" / "labels.json" + +READS = frozenset({"current", "resume", "chain", "tracks"}) + DEFAULT_TRACK = "default" # A track is a short kebab-case slug naming a lane of work. @@ -215,11 +230,11 @@ def rows_of(data: object, what: str, field: str) -> list[dict]: return data -def require_label(repo: str) -> None: - """Refuse where the target repository does not carry the label the chain is indexed by. +def label_present(repo: str) -> bool: + """Whether the target repository carries the label the chain is indexed by. - Degrading instead would report an empty chain on a repository that has one, which is the - failure mode `decision` already demonstrated fleet-wide. + A read that filled its window refuses rather than answering, since there the label's absence is + unproven rather than established. """ rows = rows_of( gh_json(["label", "list", "--repo", repo, "--limit", str(WINDOW), "--json", "name"]), @@ -233,11 +248,145 @@ def require_label(repo: str) -> None: "could sit past it. Reporting the label as absent here would send you to re-apply a " "set that may already be applied." ) - if LABEL not in names: + return LABEL in names + + +def registry() -> tuple[str, set[str]]: + """The registry's owner and its repository names, lowercased, as GitHub compares them. + + An unreadable registry is a failure to answer rather than an answer, since reading it as empty + would move every fleet repository onto the warning path. + """ + try: + data = json.loads(REGISTRY.read_text(encoding="utf-8")) + owner = data["owner"] + names = {row["name"].lower() for row in data["repos"]} + if not isinstance(owner, str): + raise TypeError(f"owner is {owner!r}, not a string") + except (OSError, ValueError, KeyError, TypeError, AttributeError) as exc: + raise Execution(f"could not read the fleet registry at {REGISTRY}: {exc}") from exc + return owner.lower(), names + + +def repo_flags(repo: str) -> dict[str, bool]: + """Whether the repository has issues turned on and whether it is a fork, read live.""" + data = gh_json(["repo", "view", repo, "--json", "hasIssuesEnabled,isFork"]) + fields = ("hasIssuesEnabled", "isFork") + if not isinstance(data, dict) or not all(isinstance(data.get(f), bool) for f in fields): + raise Execution(f"repo view for {repo} returned no {' and '.join(fields)}: {data!r}") + return {field: data[field] for field in fields} + + +def label_definition() -> tuple[str, str]: + """The `handoff` label's color and description, read from the fleet label set it belongs to. + + One definition keeps a label created here identical to the one the fleet set applies, should the + repository it lands on join the fleet later. + """ + try: + rows = json.loads(LABELS.read_text(encoding="utf-8")) + row = next(row for row in rows if row["name"] == LABEL) + color, description = row["color"], row["description"] + if not isinstance(color, str) or not isinstance(description, str): + raise TypeError(f"the `{LABEL}` entry is {row!r}") + except (OSError, ValueError, KeyError, TypeError, StopIteration) as exc: + raise Execution(f"could not read the `{LABEL}` label from {LABELS}: {exc!r}") from exc + return color, description + + +def create_label(repo: str, dry_run: bool) -> None: + """Create the one `handoff` label and confirm it by reading the label set back.""" + color, description = label_definition() + out = gh( + [ + "label", + "create", + LABEL, + "--repo", + repo, + "--description", + description, + "--color", + color, + ], + dry_run=dry_run, + ).strip() + if dry_run: + return + print(f" label created: {out or LABEL}") + if not label_present(repo): + raise Execution( + f"{repo} still carries no `{LABEL}` label after the create, so the create is not " + "confirmed. A write that appears to have failed is verified, never assumed harmless." + ) + + +def without_label(a: argparse.Namespace) -> int | None: + """What a repository missing the label gets, or None where the subcommand goes on to run. + + On a fleet repository it is drift, and every subcommand refuses, since degrading there would + report an empty chain on a repository that has one, the failure `decision` demonstrated. + On an unregistered fork under the registry's owner no issue can carry a label that does not + exist, so an empty chain is the true answer there rather than a degraded one, and only + `new --create-label` writes anything. + + The body is read once, before the label is created, and that read is the one `new` files, so a + body `new` would refuse leaves no label behind. Everything else refuses before any write. A + repository under another owner never gets a label created here, since these calls run as + subprocesses a write guard on the caller never sees. An unregistered repository of the owner's + that is not a fork is registry drift rather than a fork, and a lone label there would hide it. + """ + repo = a.repo + create = getattr(a, "create_label", False) + owner, names = registry() + repo_owner, _, name = repo.partition("/") + missing = f"{repo} carries no `{LABEL}` label, so no handoff can be found or filed there." + if repo_owner.lower() != owner: + raise Refusal( + f"{missing} It is not under {owner}, so this script neither reads a chain there nor " + "creates the label." + ) + if name.lower() in names: + flag = " `--create-label` is for an unregistered fork." if create else "" + raise Refusal(f"{missing}{flag} Apply the fleet label set from a hub checkout: {APPLY}") + flags = repo_flags(repo) + if not flags["isFork"]: + raise Refusal( + f"{missing} It is neither in registry/repos.json nor a fork, so it is registry drift " + f"rather than a fork keeping state. Register it, then apply the fleet label set: {APPLY}" + ) + fix = f"gh label create {LABEL} --repo {repo}" + if not flags["hasIssuesEnabled"]: + fix = f"gh repo edit {repo} --enable-issues, then {fix}" + if create: + raise Refusal( + f"{repo} has issues turned off, so no handoff can be filed there. Run " + f"gh repo edit {repo} --enable-issues first, then rerun with --create-label." + ) + if a.cmd in READS: + print( + f"warning: {repo} is a fork outside the fleet and carries no `{LABEL}` label, so it " + f"holds no handoff chain. `new --create-label` creates that one label, or run: {fix}", + file=sys.stderr, + ) + if a.cmd == "tracks": + print(f"(no open `{LABEL}` issues in {repo})") + return 0 raise Refusal( - f"{repo} carries no `{LABEL}` label, so no handoff can be found or filed there. " - f"Apply the fleet label set from a hub checkout: {APPLY}" + f"{repo} has no handoff on track {a.track!r}. Zero is the state before the first " + "handoff on a track, so `new` is what follows, not a retry of this." ) + if a.cmd == "new" and create: + a.body = body_from(Path(a.body_file)) + print(f"0. create the `{LABEL}` label on {repo}, a fork outside the fleet") + create_label(repo, a.dry_run) + a.fresh_label = True + return None + raise Refusal( + f"{repo} is a fork outside the fleet and carries no `{LABEL}` label, so no handoff can be " + f"filed there. Rerun `new` with --create-label to create that one label, or run: {fix}. " + "The fleet label set does not belong on a fork outside the fleet." + ) def parse_marker(body: str, number: int) -> dict[str, str] | None: @@ -764,11 +913,20 @@ def cmd_new(a: argparse.Namespace) -> int: filing on one track at once both resolve the same predecessor and both create. That leaves the two open handoffs every later command refuses over, which is detectable rather than silent, and naming a track per lane is what keeps two sessions off one chain in the first place. + + After `--create-label` the chain is empty by construction, since a label this run just created + sits on no issue yet, so no chain read is made, which also keeps a dry run from querying a label + it never created. """ - body = body_from(Path(a.body_file)) - rows = open_handoffs(a.repo) - require_marked(rows) - previous = on_track(rows, a.track) or newest_closed(a.repo, a.track) + body = getattr(a, "body", None) + if body is None: + body = body_from(Path(a.body_file)) + if getattr(a, "fresh_label", False): + rows, previous = [], None + else: + rows = open_handoffs(a.repo) + require_marked(rows) + previous = on_track(rows, a.track) or newest_closed(a.repo, a.track) previous_number = previous["number"] if previous else None round_ = int(previous["marker"]["round"]) + 1 if previous else 1 # A head can already have a successor, whatever key resolved it. @@ -1040,6 +1198,11 @@ def build_parser() -> argparse.ArgumentParser: p.add_argument( "--body-file", required=True, metavar="PATH", help="the handoff body, per the skill" ) + p.add_argument( + "--create-label", + action="store_true", + help="on an unregistered fork with no handoff label, create that one label first", + ) p = sub.add_parser("link", help="finish a chain that half-applied") add_repo(p) @@ -1080,7 +1243,10 @@ def main(argv: list[str] | None = None) -> int: if getattr(a, flag, None) is not None and getattr(a, flag) < 1: ap.error(f"--{flag} takes an issue number, so it cannot be below 1") try: - require_label(a.repo) + if not label_present(a.repo): + answered = without_label(a) + if answered is not None: + return answered return HANDLERS[a.cmd](a) except Refusal as exc: print(f"refused: {exc}", file=sys.stderr) diff --git a/tests/test_handoff.py b/tests/test_handoff.py index 0b782b87..facbde26 100755 --- a/tests/test_handoff.py +++ b/tests/test_handoff.py @@ -7,8 +7,10 @@ each write case asserts the argv it produced rather than only its effect. Two refusals earn cases of their own because the chain exists to fix them. An ambiguous track is -never resolved by picking, and a repository missing the label refuses rather than reporting an -empty chain, which is the silent failure the `decision` label already demonstrated fleet-wide. +never resolved by picking, and a fleet repository missing the label refuses rather than reporting +an empty chain, which is the silent failure the `decision` label already demonstrated fleet-wide. +A repository outside the fleet warns instead, and its cases prove the warning path never writes +anything but the one label `new --create-label` asks for. Run as `python3 tests/test_handoff.py`, or under `python3 -m unittest discover -s tests`. @@ -70,15 +72,27 @@ class FakeGh: It honors `--json`, `--state`, `--label`, and `--limit`, because a stub looser than the tool it stands in for is a stub that green-lights a crash. - It also refuses any call carrying no `--repo`, which real `gh` would answer by resolving the - repository from the working directory's remote. That is the failure this whole script is built + It also refuses any call carrying no `--repo`, apart from `repo view`, which names its + repository positionally and is refused instead where that name is no OWNER/NAME. Real `gh` + would answer a call missing its repository by resolving it from the working directory's + remote. That is the failure this whole script is built against, and without this the argument could be dropped from any of nine call sites with every case still green. """ - def __init__(self, issues: dict[int, dict] | None = None, *, label: bool = True) -> None: + def __init__( + self, + issues: dict[int, dict] | None = None, + *, + label: bool = True, + issues_on: bool = True, + fork: bool = True, + ) -> None: self.issues = issues or {} self.label = label + self.issues_on = issues_on + self.fork = fork + self.refuse_label = False self.calls: list[list[str]] = [] self.next_number = 1001 # A close that reports success and leaves the issue open. @@ -87,6 +101,11 @@ def __init__(self, issues: dict[int, dict] | None = None, *, label: bool = True) def __call__(self, argv: list[str]) -> str: self.calls.append(list(argv)) + if argv[:2] == ["repo", "view"]: + if "/" not in argv[2]: + raise AssertionError(f"{' '.join(argv)} names no OWNER/NAME repository") + flags = {"hasIssuesEnabled": self.issues_on, "isFork": self.fork} + return json.dumps(projected(flags, argv)) if "--repo" not in argv: raise AssertionError( f"{' '.join(argv)} carries no --repo, so real gh would resolve the repository " @@ -95,6 +114,10 @@ def __call__(self, argv: list[str]) -> str: head = (argv[0], argv[1]) if head == ("label", "list"): return json.dumps([{"name": handoff.LABEL}] if self.label else [{"name": "bug"}]) + if head == ("label", "create"): + if not self.refuse_label: + self.label = argv[2] == handoff.LABEL + return "" if head == ("issue", "list"): return json.dumps([projected(row, argv) for row in self._list(argv)]) if head == ("issue", "view"): @@ -258,8 +281,19 @@ def test_a_retitled_issue_still_chains(self) -> None: self.assertEqual(read_marker(row["body"], 20)["track"], "default") +def fleet(case: unittest.TestCase, member: bool = True) -> None: + """Pin a registry owned by `o` that lists `o/r` or not, for the rest of the case.""" + names = {"r"} if member else set() + patcher = unittest.mock.patch.object(handoff, "registry", lambda: ("o", names)) + patcher.start() + case.addCleanup(patcher.stop) + + class LabelCase(unittest.TestCase): - """A repository missing the label refuses rather than reporting an empty chain.""" + """A fleet repository missing the label refuses rather than reporting an empty chain.""" + + def setUp(self) -> None: + fleet(self) def test_a_missing_label_refuses_and_names_the_fix(self) -> None: code, _, err = run(FakeGh(label=False), "current", "--repo", "o/r") @@ -267,6 +301,25 @@ def test_a_missing_label_refuses_and_names_the_fix(self) -> None: self.assertIn("configure.sh apply", err) self.assertIn(handoff.LABEL, err) + def test_create_label_is_refused_on_a_fleet_repository(self) -> None: + """There the fleet label set is the fix, and one label alone would hide the drift.""" + fake = FakeGh(label=False) + code, _, err = run( + fake, + "new", + "--repo", + "o/r", + "--title", + "T", + "--body-file", + body_file(self, "w"), + "--create-label", + ) + self.assertEqual(code, 1) + self.assertIn("unregistered fork", err) + self.assertIn("configure.sh apply", err) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"]]) + def test_a_full_label_window_refuses_rather_than_reporting_the_label_absent(self) -> None: """Otherwise the caller is sent to re-apply a set that may already be applied.""" @@ -303,6 +356,241 @@ def test_the_label_is_checked_before_every_subcommand(self) -> None: self.assertEqual(fake.calls[0][:2], ["label", "list"]) +class OutsideFleetLabelCase(unittest.TestCase): + """An unregistered fork with no label warns, and writes only the label it is asked to.""" + + def setUp(self) -> None: + fleet(self, member=False) + + def new(self, fake: FakeGh, *extra: str) -> tuple[int, str, str]: + return run( + fake, + "new", + "--repo", + "o/r", + "--title", + "T", + "--body-file", + body_file(self, "w"), + *extra, + ) + + def test_a_read_warns_and_answers_as_an_empty_chain(self) -> None: + for cmd in ("current", "resume", "chain"): + with self.subTest(cmd=cmd): + fake = FakeGh(label=False) + code, _, err = run(fake, cmd, "--repo", "o/r") + self.assertEqual(code, 1) + self.assertIn("warning:", err) + self.assertIn(f"gh label create {handoff.LABEL} --repo o/r", err) + self.assertIn("no handoff on track", err) + self.assertNotIn("configure.sh", err) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) + + def test_tracks_warns_and_lists_nothing(self) -> None: + code, out, err = run(FakeGh(label=False), "tracks", "--repo", "o/r") + self.assertEqual(code, 0) + self.assertIn("(no open", out) + self.assertIn("warning:", err) + + def test_new_without_the_flag_refuses_and_names_it(self) -> None: + fake = FakeGh(label=False) + code, _, err = self.new(fake) + self.assertEqual(code, 1) + self.assertIn("--create-label", err) + self.assertNotIn("configure.sh", err) + self.assertFalse(any(c[:2] == ["label", "create"] for c in fake.calls)) + self.assertFalse(any(c[:2] == ["issue", "create"] for c in fake.calls)) + + def test_new_with_the_flag_creates_the_label_then_the_first_link(self) -> None: + """The label is confirmed before the issue that needs it, and no chain read is made.""" + fake = FakeGh(label=False) + code, out, _ = self.new(fake, "--create-label") + self.assertEqual(code, 0, out) + self.assertEqual( + [c[:2] for c in fake.calls], + [ + ["label", "list"], + ["repo", "view"], + ["label", "create"], + ["label", "list"], + ["issue", "create"], + ], + ) + self.assertEqual(fake.calls[2][2], handoff.LABEL) + filed = fake.issues[1001] + self.assertEqual([label["name"] for label in filed["labels"]], [handoff.LABEL]) + marker = read_marker(filed["body"], 1001) + self.assertEqual((marker["round"], marker["previous"]), ("1", "none")) + + def test_a_dry_run_with_the_flag_writes_nothing(self) -> None: + fake = FakeGh(label=False) + code, out, _ = self.new(fake, "--create-label", "--dry-run") + self.assertEqual(code, 0, out) + self.assertIn(f"would run: gh label create {handoff.LABEL}", out) + self.assertIn("would run: gh issue create", out) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) + self.assertFalse(fake.label) + + def test_a_body_new_would_refuse_creates_no_label(self) -> None: + """Otherwise the refusal comes after a write, which leaves the fork half-changed.""" + fake = FakeGh(label=False) + missing = str(Path(tempfile.mkdtemp()) / "absent.md") + self.addCleanup(shutil.rmtree, Path(missing).parent) + code, _, err = run( + fake, + "new", + "--repo", + "o/r", + "--title", + "T", + "--body-file", + missing, + "--create-label", + ) + self.assertEqual(code, 1) + self.assertIn("could not read the body file", err) + self.assertFalse(any(c[:2] == ["label", "create"] for c in fake.calls)) + + def test_the_body_is_read_once_and_that_read_is_filed(self) -> None: + """A second read could see a changed file after the label exists, and warns twice.""" + fake = FakeGh(label=False) + path = body_file(self, "x" * (handoff.WARN_BYTES + 1)) + reads: list[Path] = [] + real = handoff.body_from + + def counted(p: Path) -> str: + reads.append(p) + return real(p) + + with unittest.mock.patch.object(handoff, "body_from", counted): + code, out, err = run( + fake, + "new", + "--repo", + "o/r", + "--title", + "T", + "--body-file", + path, + "--create-label", + ) + self.assertEqual(code, 0, out + err) + self.assertEqual(len(reads), 1) + + def test_an_unconfirmed_label_create_fails_before_filing(self) -> None: + fake = FakeGh(label=False) + fake.refuse_label = True + code, _, err = self.new(fake, "--create-label") + self.assertEqual(code, 2) + self.assertIn("not confirmed", err) + self.assertFalse(any(c[:2] == ["issue", "create"] for c in fake.calls)) + + def test_issues_turned_off_are_named_and_stop_the_label_create(self) -> None: + code, _, err = run(FakeGh(label=False, issues_on=False), "current", "--repo", "o/r") + self.assertEqual(code, 1) + self.assertIn("gh repo edit o/r --enable-issues", err) + fake = FakeGh(label=False, issues_on=False) + code, _, err = self.new(fake, "--create-label") + self.assertEqual(code, 1) + self.assertIn("--enable-issues", err) + self.assertFalse(any(c[:2] == ["label", "create"] for c in fake.calls)) + + def test_a_repository_under_another_owner_refuses_before_any_other_call(self) -> None: + """A mistyped owner would otherwise put a label on a stranger's repository, then an issue.""" + for argv in (("current",), ("tracks",)): + with self.subTest(argv=argv): + fake = FakeGh(label=False) + code, _, err = run(fake, *argv, "--repo", "stranger/r") + self.assertEqual(code, 1) + self.assertIn("not under o", err) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"]]) + fake = FakeGh(label=False) + code, _, _ = run( + fake, + "new", + "--repo", + "stranger/r", + "--title", + "T", + "--body-file", + body_file(self, "w"), + "--create-label", + ) + self.assertEqual(code, 1) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"]]) + + def test_an_unregistered_repository_that_is_not_a_fork_is_drift(self) -> None: + """It belongs in the registry, so it refuses as drift rather than taking a lone label.""" + fake = FakeGh(label=False, fork=False) + code, _, err = run(fake, "tracks", "--repo", "o/r") + self.assertEqual(code, 1) + self.assertIn("registry drift", err) + self.assertIn("configure.sh apply", err) + fake = FakeGh(label=False, fork=False) + code, _, _ = self.new(fake, "--create-label") + self.assertEqual(code, 1) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) + + def test_the_label_created_is_the_fleet_set_s_own_definition(self) -> None: + rows = json.loads(handoff.LABELS.read_text(encoding="utf-8")) + want = next(row for row in rows if row["name"] == handoff.LABEL) + fake = FakeGh(label=False) + self.assertEqual(self.new(fake, "--create-label")[0], 0) + argv = next(c for c in fake.calls if c[:2] == ["label", "create"]) + self.assertEqual(argv[argv.index("--color") + 1], want["color"]) + self.assertEqual(argv[argv.index("--description") + 1], want["description"]) + + def test_link_refuses_without_the_label(self) -> None: + fake = FakeGh(label=False) + code, _, err = run(fake, "link", "--repo", "o/r", "--new", "2", "--previous", "1") + self.assertEqual(code, 1) + self.assertIn("--create-label", err) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) + + +class FleetRegistryCase(unittest.TestCase): + """Membership is read from the hub's registry, and an unreadable one is not an empty one.""" + + def test_the_registry_reads_lowercased_with_the_hub_in_it(self) -> None: + owner, names = handoff.registry() + self.assertEqual(owner, "ptr727") + self.assertIn("projecttemplate", names) + + def test_membership_is_compared_case_insensitively_on_the_live_path(self) -> None: + """A mixed-case registered repository is drift on the fleet path, never a fork's warning.""" + fleet(self) + for repo in ("O/R", "o/R"): + with self.subTest(repo=repo): + fake = FakeGh(label=False) + code, _, err = run(fake, "tracks", "--repo", repo) + self.assertEqual(code, 1) + self.assertIn("configure.sh apply", err) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"]]) + + def test_an_unreadable_registry_fails_rather_than_reading_as_empty(self) -> None: + missing = Path(tempfile.mkdtemp()) / "repos.json" + self.addCleanup(shutil.rmtree, missing.parent) + with ( + unittest.mock.patch.object(handoff, "REGISTRY", missing), + self.assertRaises(handoff.Execution), + ): + handoff.registry() + + def test_a_malformed_registry_fails_rather_than_reading_as_empty(self) -> None: + bad = Path(tempfile.mkdtemp()) / "repos.json" + self.addCleanup(shutil.rmtree, bad.parent) + for data in ({"owner": "o", "repos": [{"name": 7}]}, {"owner": 7, "repos": []}): + with self.subTest(data=data): + bad.write_text(json.dumps(data), encoding="utf-8") + with ( + unittest.mock.patch.object(handoff, "REGISTRY", bad), + self.assertRaises(handoff.Execution) as caught, + ): + handoff.registry() + self.assertIn("could not read the fleet registry", str(caught.exception)) + + class CurrentCase(unittest.TestCase): """Exactly one open handoff per track, and anything else is reported rather than resolved.""" @@ -1154,7 +1442,7 @@ def test_a_row_that_is_not_an_object_is_named_too(self) -> None: ), self.assertRaises(handoff.Execution) as caught, ): - handoff.require_label("o/r") + handoff.label_present("o/r") self.assertIn("carrying no name", str(caught.exception)) def test_a_label_list_that_is_not_an_array_is_an_execution_failure(self) -> None: @@ -1163,7 +1451,7 @@ def test_a_label_list_that_is_not_an_array_is_an_execution_failure(self) -> None unittest.mock.patch.object(handoff, "run_gh", lambda argv: '{"not": "an array"}'), self.assertRaises(handoff.Execution) as caught, ): - handoff.require_label("o/r") + handoff.label_present("o/r") self.assertIn("did not read as an array", str(caught.exception)) def test_an_unreadable_body_file_refuses_rather_than_filing_an_empty_handoff(self) -> None: From e0aeb7291dbeebb573274b26565feed1112de50f Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 15:49:35 -0700 Subject: [PATCH 4/7] Declare Python and Codecov on Blog's Registry Entry (#2263) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Blog adopted the hub Python profiles (Blog #364, promoted via #366, closing Blog #356). This brings its registry entry into line: - `types` gains `python`. - `requiredSecrets` becomes `["CODECOV_TOKEN"]`, which the `python` type's codecov mechanism requires. The token is in both the actions and Dependabot stores. - `pythonDirectories` becomes `["checks", "scripts"]`, matching what every validate-task caller passes. - The environment-secrets driftNote no longer says `requiredSecrets` stays empty. - A new driftNote records the maintainer's decision in Blog #356 to keep `capture/` undeclared, and the obligations that decision forgoes. ## Verification - Every claim above checked against live `ptr727/Blog` main. - `spec/validate.py` passes. - `spec/audit.py Blog` reports 0 defect/letter/error findings. The only Python drift left is the expected one naming the `capture/` files. That check has no driftNote suppression on a repo that declares `python`. - Local review pass recorded over the final content (three rounds; the first two found wording problems in the new driftNote, now fixed). ## Pre-existing, not addressed here - The `Deploy credentials...` driftNote says the environment secret names are declared in Blog's own `secrets.json` environments block, and that `spec/secrets.json` has no vocabulary for them. Blog carries no `secrets.json`, and `spec/secrets.schema.json` does define an `environments` block. - Blog declares no Python `profiles` while `checks/` is lint-only and `scripts/` is the pip build shape. No rule says what a mixed-shape repo declares, and several other Python entries also omit it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- registry/repos.json | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/registry/repos.json b/registry/repos.json index 9e341a09..a120f221 100644 --- a/registry/repos.json +++ b/registry/repos.json @@ -306,17 +306,18 @@ "name": "Blog", "url": "https://github.com/ptr727/Blog", "status": "cataloged", - "types": ["hugo", "source-only"], + "types": ["hugo", "source-only", "python"], "groundTruthBranch": "main", "workflowModel": "release", "lineEndings": "lf", "hasDevelop": true, "publish": [{ "target": "github-release", "mechanism": "none" }, { "target": "self-hosted", "mechanism": "static-secret" }], - "requiredSecrets": [], + "requiredSecrets": ["CODECOV_TOKEN"], "environments": [{ "name": "production", "branchPolicy": "none" }, { "name": "staging", "branchPolicy": "none" }], "consumerModel": "pull", "releaseTrigger": "dispatch-only", - "driftNotes": ["Hugo static site migrated off WordPress.com, stood up 2026-08-01; release model with a dispatch-only publisher that cuts the tag and a source archive.", "lineEndings lf on a release repo, where the rule grants the native-platform default to operational repos only: every consumer is Linux (the Hugo build, the Caddy container, and the rsync deploy).", "content/ is an imported WordPress archive, so the prose, spelling, and style gates are scoped to exclude it.", "Deploy credentials are per-environment GitHub Environment secrets and variables, which spec/secrets.json has no vocabulary for, so requiredSecrets stays empty and the names are declared in this repo's own secrets.json environments block; listing them here would make the audit demand them in the repository actions store.", "Neither deployment environment carries a branch policy, deliberately: the ref gate that admits production from the default branch only runs as a job in the deploy workflow, before anything is installed or written (WORKFLOW.md D2.1). branchPolicy none records that, so a policy appearing later reads as a change rather than as the gate arriving."] + "pythonDirectories": ["checks", "scripts"], + "driftNotes": ["Hugo static site migrated off WordPress.com, stood up 2026-08-01; release model with a dispatch-only publisher that cuts the tag and a source archive.", "lineEndings lf on a release repo, where the rule grants the native-platform default to operational repos only: every consumer is Linux (the Hugo build, the Caddy container, and the rsync deploy).", "content/ is an imported WordPress archive, so the prose, spelling, and style gates are scoped to exclude it.", "Deploy credentials are per-environment GitHub Environment secrets and variables, which spec/secrets.json has no vocabulary for, so requiredSecrets leaves them out and the names are declared in this repo's own secrets.json environments block; listing them here would make the audit demand them in the repository actions store.", "Neither deployment environment carries a branch policy, deliberately: the ref gate that admits production from the default branch only runs as a job in the deploy workflow, before anything is installed or written (WORKFLOW.md D2.1). branchPolicy none records that, so a policy appearing later reads as a change rather than as the gate arriving.", "capture/ is one-shot WordPress migration tooling that has already run, so the maintainer kept it out of pythonDirectories (Blog #356, 2026-10-01), forgoing the lint, format, type-check, test, and coverage obligations every Python directory owes. The audit's python-directories drift naming capture/ files is therefore expected."] }, { "name": "DiskSpeedTest", From 18be0e953fae2d72a49ebf94d069185ccb52f1f4 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 16:09:48 -0700 Subject: [PATCH 5/7] Bound handoff.py's Writes Whatever the Label State (#2266) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Copilot's review of promotion PR #2254 (High) found that `handoff.py` checked the owner and fork status only when the `handoff` label was missing. That came in with #2265. So `new` or `link` against another owner's repository still wrote there whenever that repository happened to carry a `handoff` label. An unregistered non-fork under the owner slipped past the registry-drift refusal the same way. These writes run as `gh` subprocesses, which the calling agent's write guard never sees, so the script has to enforce the boundary itself, the way `pr_review.py` refuses its own cross-owner writes. `require_write_scope` now runs in `main` before the label is read, and only for the two writing subcommands, `new` and `link`: | Repository | Writes | | --- | --- | | Under another owner | Refused, with no `gh` call at all | | Registered in `registry/repos.json` | Allowed, with no extra request | | Unregistered under the owner, a fork | Allowed, after one `gh repo view` that the missing-label path reuses | | Unregistered under the owner, not a fork | Refused as registry drift | The write scope bounds no read. `scripts/README.md` and `session-handoff` are updated to match. ## Verification - `tests/test_handoff.py` passes all 141 tests. A module-level patch now makes `o/r` a fleet repository by default, so existing write cases keep testing what they name. The new `WriteScopeCase` covers: - a labeled repository under another owner, for `new` and `link`; - a labeled, unregistered non-fork, for `new` and `link`; - a labeled, unregistered fork, which still files; - reads, which stay unbounded; - a registered repository, which costs no extra request. - Mutation checks each fail named tests: removing the scope call, the owner check, or the fork check; applying the scope to reads; dropping the cache. - The full `unittest discover` run passes (1944 tests). mypy, ruff, `prose_lint --diff origin/develop`, `spec/validate.py`, `build_dist.py --check`, and `docker_lint.py` are all clean. - `local-strict-review`: the first pass found 2 prose overclaims, both fixed. The second pass found nothing, and its receipt is recorded. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- .agents/skills/session-handoff/SKILL.md | 3 +- .../.source-digests/session-handoff | 2 +- .../skills/session-handoff/SKILL.md | 3 +- .github/skills/session-handoff/SKILL.md | 3 +- scripts/README.md | 2 +- scripts/handoff.py | 58 +++++++++++--- tests/test_handoff.py | 80 +++++++++++++++++-- 7 files changed, 129 insertions(+), 22 deletions(-) diff --git a/.agents/skills/session-handoff/SKILL.md b/.agents/skills/session-handoff/SKILL.md index 5dd83ca7..b84ad5f5 100644 --- a/.agents/skills/session-handoff/SKILL.md +++ b/.agents/skills/session-handoff/SKILL.md @@ -320,7 +320,8 @@ and answers as an empty chain does, and `new` refuses until it is given `--creat creates the one `handoff` label, confirms it, and then files the first link. Issues turned off stop it before any write, naming the command that turns them on. Never apply the fleet label set to such a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, -refuses before any write. +refuses before any write. `new` and `link` refuse both whatever the label state, since a label on +such a repository opens no write there, and that refusal bounds no read. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than diff --git a/.claude-plugin/fleet-skills/.source-digests/session-handoff b/.claude-plugin/fleet-skills/.source-digests/session-handoff index 24500fc8..6d54cf54 100644 --- a/.claude-plugin/fleet-skills/.source-digests/session-handoff +++ b/.claude-plugin/fleet-skills/.source-digests/session-handoff @@ -1 +1 @@ -c1684259d7d09c94 +71ea890bf0050608 diff --git a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md index 5dd83ca7..b84ad5f5 100644 --- a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md @@ -320,7 +320,8 @@ and answers as an empty chain does, and `new` refuses until it is given `--creat creates the one `handoff` label, confirms it, and then files the first link. Issues turned off stop it before any write, naming the command that turns them on. Never apply the fleet label set to such a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, -refuses before any write. +refuses before any write. `new` and `link` refuse both whatever the label state, since a label on +such a repository opens no write there, and that refusal bounds no read. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than diff --git a/.github/skills/session-handoff/SKILL.md b/.github/skills/session-handoff/SKILL.md index 5dd83ca7..b84ad5f5 100644 --- a/.github/skills/session-handoff/SKILL.md +++ b/.github/skills/session-handoff/SKILL.md @@ -320,7 +320,8 @@ and answers as an empty chain does, and `new` refuses until it is given `--creat creates the one `handoff` label, confirms it, and then files the first link. Issues turned off stop it before any write, naming the command that turns them on. Never apply the fleet label set to such a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, -refuses before any write. +refuses before any write. `new` and `link` refuse both whatever the label state, since a label on +such a repository opens no write there, and that refusal bounds no read. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than diff --git a/scripts/README.md b/scripts/README.md index 17fd3c05..4c9e2a1e 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -261,7 +261,7 @@ The invariant is one open handoff per track per repository, read from the metada `new` creates the new issue, comments the forward link on the previous one, and then closes it, in that order, printing each step's own result. Creating first means a failure at any later step leaves a discoverable new issue rather than a closed chain with no successor, and `link` finishes what a failure interrupted without a second issue being filed for it. `link` takes no `--track`, so the two issues' own metadata blocks are the only thing that can say they belong to one lane, and it refuses an issue named as its own predecessor and a pair whose blocks name different tracks rather than closing one lane's handoff into another's. Nothing suppresses a write's output or converts a failure into success, per [GOVERNANCE.md][governance] "Repository Boundaries and Write Safety". Where the caller names an issue, which is `link` alone, that issue is read live before the write and only what the read returned is written back, and every other identifier a write consumes comes from a read in the same run rather than being constructed. A close is confirmed by reading the state back, because a write that appears to have failed may have succeeded on the server. -`--repo` is required and carries no default, for the reason `pr_review.py` states about a pull request number: an issue number resolves in every repository, and a chain read out of the wrong one is well formed. `--track` defaults to `default`, and omitting it on a named lane does more than read the wrong chain, since `new` then files onto `default` and comments on and closes whatever that lane had open. A fleet repository missing the `handoff` label refuses, naming `repo-config/configure.sh apply`, never with a degraded empty answer, because `decision` was declared on the hub and applied nowhere else and every downstream session then enumerated an empty queue and reported it healthy. The one exception is a fork under the registry's owner that the registry does not list, such as one kept for an upstream contribution, where no issue can carry a label that does not exist: a read there warns and answers as an empty chain does, and `new --create-label` creates the one `handoff` label from `repo-config/labels.json`, confirms it, and files the first link, refusing first where the fork has issues turned off. A repository missing the label under another owner, or an unregistered one of the owner's that is not a fork, refuses before any write, the first because these calls run as subprocesses a write guard on the caller never sees and the second because it is registry drift a lone label would hide. Exit `0` is success, `1` a refusal the caller can act on, and `2` the command not having run to an answer, so the two never share a code: a usage error is moved off argparse's own `2` onto `1` because it is a refusal, and an exception nobody modeled is caught onto `2` rather than reaching CPython's own `1`. A body over 12 KB warns and one over 60 KB refuses, under GitHub's own 65536-character limit, and both are backstops against a body GitHub would reject: what bounds a handoff is the Skill's per-section rules. +`--repo` is required and carries no default, for the reason `pr_review.py` states about a pull request number: an issue number resolves in every repository, and a chain read out of the wrong one is well formed. `--track` defaults to `default`, and omitting it on a named lane does more than read the wrong chain, since `new` then files onto `default` and comments on and closes whatever that lane had open. A fleet repository missing the `handoff` label refuses, naming `repo-config/configure.sh apply`, never with a degraded empty answer, because `decision` was declared on the hub and applied nowhere else and every downstream session then enumerated an empty queue and reported it healthy. The one exception is a fork under the registry's owner that the registry does not list, such as one kept for an upstream contribution, where no issue can carry a label that does not exist: a read there warns and answers as an empty chain does, and `new --create-label` creates the one `handoff` label from `repo-config/labels.json`, confirms it, and files the first link, refusing first where the fork has issues turned off. A repository missing the label under another owner, or an unregistered one of the owner's that is not a fork, refuses before any write, the first because these calls run as subprocesses a write guard on the caller never sees and the second because it is registry drift a lone label would hide. The two writing subcommands, `new` and `link`, refuse both whatever the label state, checked before the label is read, so a label a stranger's repository happens to carry opens no write there. That is an in-process refusal of the kind `pr_review.py` makes, with the owner read from the registry rather than from `origin`, and it bounds no read. Exit `0` is success, `1` a refusal the caller can act on, and `2` the command not having run to an answer, so the two never share a code: a usage error is moved off argparse's own `2` onto `1` because it is a refusal, and an exception nobody modeled is caught onto `2` rather than reaching CPython's own `1`. A body over 12 KB warns and one over 60 KB refuses, under GitHub's own 65536-character limit, and both are backstops against a body GitHub would reject: what bounds a handoff is the Skill's per-section rules. `--dry-run` prints the calls a writing subcommand would make and sends none of them, which is the first thing anyone wants before trusting this to close an issue. It sits on the one path every call goes through rather than on each write separately. diff --git a/scripts/handoff.py b/scripts/handoff.py index efa19b31..71eed74c 100755 --- a/scripts/handoff.py +++ b/scripts/handoff.py @@ -68,6 +68,12 @@ `new --create-label` creates the one `handoff` label and nothing else. Every other repository missing the label refuses, a repository under another owner and an unregistered repository of the owner's that is not a fork among them. + +The two writing subcommands, `new` and `link`, are bounded whatever the label state, an +in-process refusal of the kind `pr_review.py` makes, with the owner read from the registry. They +refuse a repository under another owner before any call, and an unregistered repository of the +owner's unless it is a fork, since these writes run as subprocesses a write guard on the caller +never sees. That write scope bounds no read. """ from __future__ import annotations @@ -268,13 +274,47 @@ def registry() -> tuple[str, set[str]]: return owner.lower(), names -def repo_flags(repo: str) -> dict[str, bool]: - """Whether the repository has issues turned on and whether it is a fork, read live.""" - data = gh_json(["repo", "view", repo, "--json", "hasIssuesEnabled,isFork"]) +def repo_flags(a: argparse.Namespace) -> dict[str, bool]: + """Whether the repository has issues turned on and whether it is a fork, read live once a run. + + The write scope and the missing-label path both ask, so the answer rides on the parsed + arguments rather than costing a second request. + """ + cached = getattr(a, "repo_flags", None) + if cached is not None: + return cached + data = gh_json(["repo", "view", a.repo, "--json", "hasIssuesEnabled,isFork"]) fields = ("hasIssuesEnabled", "isFork") if not isinstance(data, dict) or not all(isinstance(data.get(f), bool) for f in fields): - raise Execution(f"repo view for {repo} returned no {' and '.join(fields)}: {data!r}") - return {field: data[field] for field in fields} + raise Execution(f"repo view for {a.repo} returned no {' and '.join(fields)}: {data!r}") + a.repo_flags = {field: data[field] for field in fields} + return a.repo_flags + + +def require_write_scope(a: argparse.Namespace) -> None: + """Refuse a write outside the registry's owner, or to an unregistered repository not a fork. + + It runs before the label is read, so a label a stranger's repository happens to carry opens no + write there. A registered repository costs no request, and the reads are never bounded here. + """ + if a.cmd in READS: + return + owner, names = registry() + repo_owner, _, name = a.repo.partition("/") + if repo_owner.lower() != owner: + raise Refusal( + f"{a.repo} is not under {owner}, so this script writes nothing there. A different " + "owner goes through the runbook's own `gh` path, where the write guard reads the " + "maintainer's grant." + ) + if name.lower() in names: + return + if not repo_flags(a)["isFork"]: + raise Refusal( + f"{a.repo} is neither in registry/repos.json nor a fork, so it is registry drift " + "rather than a fork keeping state, and this script writes nothing there. Register it, " + f"then apply the fleet label set: {APPLY}" + ) def label_definition() -> tuple[str, str]: @@ -332,9 +372,8 @@ def without_label(a: argparse.Namespace) -> int | None: The body is read once, before the label is created, and that read is the one `new` files, so a body `new` would refuse leaves no label behind. Everything else refuses before any write. A - repository under another owner never gets a label created here, since these calls run as - subprocesses a write guard on the caller never sees. An unregistered repository of the owner's - that is not a fork is registry drift rather than a fork, and a lone label there would hide it. + repository under another owner, or an unregistered one of the owner's that is not a fork, + refuses here for a read and in `require_write_scope` for a write, which runs first. """ repo = a.repo create = getattr(a, "create_label", False) @@ -349,7 +388,7 @@ def without_label(a: argparse.Namespace) -> int | None: if name.lower() in names: flag = " `--create-label` is for an unregistered fork." if create else "" raise Refusal(f"{missing}{flag} Apply the fleet label set from a hub checkout: {APPLY}") - flags = repo_flags(repo) + flags = repo_flags(a) if not flags["isFork"]: raise Refusal( f"{missing} It is neither in registry/repos.json nor a fork, so it is registry drift " @@ -1243,6 +1282,7 @@ def main(argv: list[str] | None = None) -> int: if getattr(a, flag, None) is not None and getattr(a, flag) < 1: ap.error(f"--{flag} takes an issue number, so it cannot be below 1") try: + require_write_scope(a) if not label_present(a.repo): answered = without_label(a) if answered is not None: diff --git a/tests/test_handoff.py b/tests/test_handoff.py index facbde26..c73842c5 100755 --- a/tests/test_handoff.py +++ b/tests/test_handoff.py @@ -32,6 +32,19 @@ sys.path.insert(0, str(SCRIPTS)) import handoff +REAL_REGISTRY = handoff.registry + + +def setUpModule() -> None: + """Read `o/r` as a fleet repository unless a case says otherwise. + + The write scope reads the registry before every write, and the real one lists no `o`, so + without this every write case would test the scope refusal instead of what it names. + """ + patcher = unittest.mock.patch.object(handoff, "registry", lambda: ("o", {"r"})) + patcher.start() + unittest.addModuleCleanup(patcher.stop) + def marked(body: str, track: str, round_: int, previous: int | None) -> str: return handoff.with_marker(body, track, round_, previous) @@ -410,8 +423,8 @@ def test_new_with_the_flag_creates_the_label_then_the_first_link(self) -> None: self.assertEqual( [c[:2] for c in fake.calls], [ - ["label", "list"], ["repo", "view"], + ["label", "list"], ["label", "create"], ["label", "list"], ["issue", "create"], @@ -429,7 +442,7 @@ def test_a_dry_run_with_the_flag_writes_nothing(self) -> None: self.assertEqual(code, 0, out) self.assertIn(f"would run: gh label create {handoff.LABEL}", out) self.assertIn("would run: gh issue create", out) - self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) + self.assertEqual([c[:2] for c in fake.calls], [["repo", "view"], ["label", "list"]]) self.assertFalse(fake.label) def test_a_body_new_would_refuse_creates_no_label(self) -> None: @@ -518,7 +531,7 @@ def test_a_repository_under_another_owner_refuses_before_any_other_call(self) -> "--create-label", ) self.assertEqual(code, 1) - self.assertEqual([c[:2] for c in fake.calls], [["label", "list"]]) + self.assertEqual([c[:2] for c in fake.calls], []) def test_an_unregistered_repository_that_is_not_a_fork_is_drift(self) -> None: """It belongs in the registry, so it refuses as drift rather than taking a lone label.""" @@ -530,7 +543,7 @@ def test_an_unregistered_repository_that_is_not_a_fork_is_drift(self) -> None: fake = FakeGh(label=False, fork=False) code, _, _ = self.new(fake, "--create-label") self.assertEqual(code, 1) - self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) + self.assertEqual([c[:2] for c in fake.calls], [["repo", "view"]]) def test_the_label_created_is_the_fleet_set_s_own_definition(self) -> None: rows = json.loads(handoff.LABELS.read_text(encoding="utf-8")) @@ -546,14 +559,65 @@ def test_link_refuses_without_the_label(self) -> None: code, _, err = run(fake, "link", "--repo", "o/r", "--new", "2", "--previous", "1") self.assertEqual(code, 1) self.assertIn("--create-label", err) - self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) + self.assertEqual([c[:2] for c in fake.calls], [["repo", "view"], ["label", "list"]]) + + +class WriteScopeCase(unittest.TestCase): + """The writes are bounded whatever the label state, since a label opens no write by itself.""" + + def setUp(self) -> None: + fleet(self, member=False) + + def new(self, fake: FakeGh, repo: str) -> tuple[int, str, str]: + return run(fake, "new", "--repo", repo, "--title", "T", "--body-file", body_file(self, "w")) + + def test_a_labeled_repository_under_another_owner_takes_no_call_and_no_write(self) -> None: + fake = FakeGh() + code, _, err = self.new(fake, "stranger/r") + self.assertEqual(code, 1) + self.assertIn("writes nothing there", err) + self.assertEqual(fake.calls, []) + fake = FakeGh() + code, _, _ = run(fake, "link", "--repo", "stranger/r", "--new", "2", "--previous", "1") + self.assertEqual(code, 1) + self.assertEqual(fake.calls, []) + + def test_a_labeled_unregistered_repository_that_is_not_a_fork_takes_no_write(self) -> None: + fake = FakeGh(fork=False) + code, _, err = self.new(fake, "o/r") + self.assertEqual(code, 1) + self.assertIn("registry drift", err) + self.assertEqual([c[:2] for c in fake.calls], [["repo", "view"]]) + fake = FakeGh(fork=False) + code, _, _ = run(fake, "link", "--repo", "o/r", "--new", "2", "--previous", "1") + self.assertEqual(code, 1) + self.assertEqual([c[:2] for c in fake.calls], [["repo", "view"]]) + + def test_a_labeled_unregistered_fork_files_and_asks_once_whether_it_is_one(self) -> None: + fake = FakeGh() + code, out, err = self.new(fake, "o/r") + self.assertEqual(code, 0, out + err) + self.assertEqual(sum(c[:2] == ["repo", "view"] for c in fake.calls), 1) + self.assertTrue(any(c[:2] == ["issue", "create"] for c in fake.calls)) + + def test_a_read_of_a_labeled_repository_under_another_owner_stays_open(self) -> None: + fake = FakeGh() + code, out, _ = run(fake, "tracks", "--repo", "stranger/r") + self.assertEqual(code, 0) + self.assertIn("(no open", out) + + def test_a_registered_repository_costs_no_extra_request(self) -> None: + fleet(self) + fake = FakeGh() + self.assertEqual(self.new(fake, "o/r")[0], 0) + self.assertFalse(any(c[:2] == ["repo", "view"] for c in fake.calls)) class FleetRegistryCase(unittest.TestCase): """Membership is read from the hub's registry, and an unreadable one is not an empty one.""" def test_the_registry_reads_lowercased_with_the_hub_in_it(self) -> None: - owner, names = handoff.registry() + owner, names = REAL_REGISTRY() self.assertEqual(owner, "ptr727") self.assertIn("projecttemplate", names) @@ -575,7 +639,7 @@ def test_an_unreadable_registry_fails_rather_than_reading_as_empty(self) -> None unittest.mock.patch.object(handoff, "REGISTRY", missing), self.assertRaises(handoff.Execution), ): - handoff.registry() + REAL_REGISTRY() def test_a_malformed_registry_fails_rather_than_reading_as_empty(self) -> None: bad = Path(tempfile.mkdtemp()) / "repos.json" @@ -587,7 +651,7 @@ def test_a_malformed_registry_fails_rather_than_reading_as_empty(self) -> None: unittest.mock.patch.object(handoff, "REGISTRY", bad), self.assertRaises(handoff.Execution) as caught, ): - handoff.registry() + REAL_REGISTRY() self.assertIn("could not read the fleet registry", str(caught.exception)) From 3fec4f893b86eebb9c9725eb8ccdabbf34948570 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 16:18:37 -0700 Subject: [PATCH 6/7] State the Install-Tools JSON Report in the Host Contract (#2267) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Copilot's review of promotion PR #2254 raised a "previously missed" finding against #2259's install-tools JSON report. `add-host-tool`'s procedure says a change to report behavior must update `docs/host-setup.md` as well as the platform READMEs, and #2259 updated only the READMEs. `docs/host-setup.md` now has one paragraph saying: - the flags are `--report --json` on Linux and `-Report -Json` on Windows, and the JSON flag applies to no other action; - the output is one object with a `schema` number, the `platform`, a `tools` list, and a top-level `notes` list; - a change to what a field means raises `schema`. For the shared fields it links to the Linux README, and for its one added field and the values that differ it links to the Windows README. It doesn't restate them, so a third copy of the field list can't drift. ## Verification - `local-strict-review` checked each claim against both installers and both READMEs and found nothing. Its receipt is recorded. - `prose_lint --diff origin/develop`, `docker_lint.py` (markdownlint, cspell, editorconfig), and `spec/validate.py` are all clean. The full `unittest discover` run passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 --- docs/host-setup.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/host-setup.md b/docs/host-setup.md index f8ccdbce..7a8ff7d8 100644 --- a/docs/host-setup.md +++ b/docs/host-setup.md @@ -76,6 +76,8 @@ host-setup\windows\install-tools.ps1 -Install -Repo C:\path\to\repository The report, list, dry-run, install, and upgrade actions accept the same repository option. A platform with no matching metadata reports no constrained installer for that tool. The Linux reader needs `jq`, which the fleet tools provide. Install the fleet tools first when a minimal host does not carry it. +**A report can be written for a program to read.** Both installers take a JSON flag beside their report action, `--report --json` on Linux and `-Report -Json` on Windows, and the flag applies to no other action. Each writes one object carrying a `schema` number, the `platform`, a `tools` list with one entry per tool, and a top-level `notes` list for what belongs to no tool. A change to what a field means raises `schema`, so a reader checks it before trusting any field. The [Linux README][host-setup-linux-readme] defines the fields every platform shares, and the [Windows README][host-setup-windows-readme] names the one field it adds and the values it reports differently. + ## Git Identity Configure your name and email, used for commit authorship. **The email is the committing account's GitHub `noreply` address, never a private, personal, or invented one**, per [GOVERNANCE.md "Git and Commit Rules"][governance-git-and-commit-rules], which owns the rule and states the fleet's value. A private address trips GitHub's email-privacy push protection (GH007), and an invented one pollutes history. @@ -374,7 +376,9 @@ A host that fails any row is not ready for the procedure that row names, and the [governance-git-and-commit-rules]: ../GOVERNANCE.md#git-and-commit-rules [host-gate]: ../scripts/host_gate.py [host-setup-dir]: ../host-setup/ +[host-setup-linux-readme]: ../host-setup/linux/README.md [host-setup-windows]: ../host-setup/windows/ +[host-setup-windows-readme]: ../host-setup/windows/README.md [host-tools]: ../spec/host-tools.json [install-tools-windows]: ../host-setup/windows/install-tools.ps1 [issue-483]: https://github.com/ptr727/ProjectTemplate/issues/483 From 73b9f7730a5ad47c2f7a3ebbf304e6ba72d2ab34 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 16:43:36 -0700 Subject: [PATCH 7/7] Drop Two False Pointers From Blog's Deploy-Secret Drift Note (#2269) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #2268 ## Summary Copilot's Balanced review of promotion PR #2254 raised a "previously missed" finding on Blog's deploy-secret drift note, which #2263 reworded. The note made two claims, and both are false: - that `spec/secrets.json` has no vocabulary for per-environment secrets, although `spec/secrets.schema.json` defines an `environments` object; - that the names are declared in Blog's own `secrets.json` environments block, although Blog's `main` has no `secrets.json`. The note now says only what holds: environment-scoped deploy credentials are not checked by the audit, so `requiredSecrets` leaves them out, and listing them there would make the audit demand them in the repository actions store. The same two false claims come from `spec/secrets.json` and `spec/project-types.json`. Those predate this change and are filed as #2268, rather than widening this one-line fix. ## Verification - `local-strict-review` checked every clause against `spec/audit.py` and `repo-config/configure.sh`, and against Blog's real environment and actions stores (read-only). Its two wording points are taken, and the confirming pass found nothing. The receipt is recorded. - `spec/validate.py`, `prose_lint --diff origin/develop`, and `docker_lint.py` are all clean. The full `unittest discover` run passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- registry/repos.json | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/registry/repos.json b/registry/repos.json index a120f221..9ee7b8b3 100644 --- a/registry/repos.json +++ b/registry/repos.json @@ -21,7 +21,7 @@ "consumerModel": "pull", "releaseTrigger": "two-phase", "pythonDirectories": ["."], - "driftNotes": ["Governance hub; audits its own rules against itself."] + "driftNotes": ["Governance hub, which audits its own rules against itself."] }, { "name": "Utilities", @@ -34,7 +34,7 @@ "requiredSecrets": ["NUGET_USERNAME", "CODECOV_TOKEN"], "consumerModel": "pull", "releaseTrigger": "two-phase", - "driftNotes": ["No get-version-task; relies on validate-task."] + "driftNotes": ["No get-version-task, relying on validate-task instead."] }, { "name": "LanguageTags", @@ -47,7 +47,7 @@ "requiredSecrets": ["NUGET_USERNAME", "CODECOV_TOKEN"], "consumerModel": "pull", "releaseTrigger": "two-phase", - "driftNotes": ["No get-version-task; relies on validate-task."] + "driftNotes": ["No get-version-task, relying on validate-task instead."] }, { "name": "aiopurpleair", @@ -91,7 +91,7 @@ "consumerModel": "pull", "releaseTrigger": "dispatch-only", "driftNotes": [ - "Personal Python toolkit (uv/pyproject, src/ + tests/ + analysis/data/docs); private.", + "Personal Python toolkit (uv/pyproject, src/ + tests/ + analysis/data/docs), private.", "Source-release repo: source-only, no PyPI - a tag plus a source zip on manual dispatch (releaseTrigger dispatch-only).", "PR CI established (test-pull-request.yml -> validate-task.yml: ruff + mypy + pytest/coverage). Python profile: mypy is the CI type checker (pyright editor-only via Pylance), deps via PEP 621 [project.optional-dependencies], static version (no _version.py).", "Carries the AGENTS.md router split: GOVERNANCE.md with the verbatim sections, repo-specific content extracted to OPERATIONS.md, plus .markdownlint-cli2.jsonc, CODESTYLE.md, .editorconfig, .gitattributes, WORKFLOW.md, version.json + NBGV, the dispatch publisher, and dependabot.yml." @@ -112,7 +112,7 @@ "consumerModel": "pull", "releaseTrigger": "two-phase", "pythonDirectories": ["RegressionTests"], - "driftNotes": ["Carries ARCHITECTURE.md and codecov.yml beyond the baseline.", "First csharp+python repo: a .NET application at the root plus a stdlib-only Python tooling subtree (RegressionTests/, uvx scripts profile - no uv.lock, pyproject carries only ruff+mypy config; PlexCleaner#855). python.uvlock.pinned is N/A for that subtree (no uv project). The subtree has no tests yet and its validator callers pass no python-directories input, both owed on adoption (python.directories.declared). codecov.yml stays required for the C# side. Reference for the csharp+python shape (issue #339)."] + "driftNotes": ["Carries ARCHITECTURE.md and codecov.yml beyond the baseline.", "First csharp+python repo: a .NET application at the root plus a stdlib-only Python tooling subtree (RegressionTests/, uvx scripts profile - no uv.lock, pyproject carries only ruff+mypy config, PlexCleaner#855). python.uvlock.pinned is N/A for that subtree (no uv project). The subtree has no tests yet and its validator callers pass no python-directories input, both owed on adoption (python.directories.declared). codecov.yml stays required for the C# side. Reference for the csharp+python shape (issue #339)."] }, { "name": "ESPHome-NonRoot", @@ -151,7 +151,7 @@ "requiredSecrets": ["DOCKER_HUB_USERNAME", "DOCKER_HUB_ACCESS_TOKEN", "CODECOV_TOKEN"], "consumerModel": "pull", "releaseTrigger": "two-phase", - "driftNotes": ["Docker image wrapping upstream Nx products; C# (CreateMatrix) is the codegen generator, not a shipped package (IsPackable=false, no nuget push).", "Release is the two-phase model (weekly schedule + workflow_dispatch publish; ordinary merges do not) plus an extra Make/Matrix.json path-scoped push that republishes when the codegen version pin bumps.", "Docker Hub README published per-image via a Matrix.json-derived matrix.", "Branch hygiene: 3 stale Dependabot nuget branches (PRs closed/superseded) linger, safe to delete; main+develop otherwise clean after the 2026-07 sweep."] + "driftNotes": ["Docker image wrapping upstream Nx products. C# (CreateMatrix) is the codegen generator, not a shipped package (IsPackable=false, no nuget push).", "Release is the two-phase model (weekly schedule + workflow_dispatch publish, not ordinary merges) plus an extra Make/Matrix.json path-scoped push that republishes when the codegen version pin bumps.", "Docker Hub README published per-image via a Matrix.json-derived matrix.", "Branch hygiene: 3 stale Dependabot nuget branches (PRs closed/superseded) linger, safe to delete, and main+develop are otherwise clean after the 2026-07 sweep."] }, { "name": "HomeAutomation-Config", @@ -166,7 +166,7 @@ "requiredSecrets": [], "consumerModel": "pull", "releaseTrigger": "dispatch-only", - "driftNotes": ["Maintainer home-lab code repo (docker-compose stacks, host install and lifecycle scripts, Firewalla configs), installed onto the Proxmox host and the VPS rather than edited in place, so it runs the release model; Linux-consumed, so lineEndings lf.", "Reclassified from operational to release on 2026-09-24 (HomeAutomation-Config #425); the -Config suffix in its name predates the reclassification.", "Renamed from HomeAutomation for fleet naming consistency (config repos are *-Config). The Vantage controller config is split out to its own Vantage-Config repo (lf default, only `.dc` pinned CRLF), not carried here; the legacy Vantage/ subtree is stripped."] + "driftNotes": ["Maintainer home-lab code repo (docker-compose stacks, host install and lifecycle scripts, Firewalla configs), installed onto the Proxmox host and the VPS rather than edited in place, so it runs the release model. It is Linux-consumed, so lineEndings lf.", "Reclassified from operational to release on 2026-09-24 (HomeAutomation-Config #425). The -Config suffix in its name predates the reclassification.", "Renamed from HomeAutomation for fleet naming consistency (config repos are *-Config). The Vantage controller config is split out to its own Vantage-Config repo (lf default, only `.dc` pinned CRLF), not carried here, and the legacy Vantage/ subtree is stripped."] }, { "name": "KiCadLibrary", @@ -178,7 +178,7 @@ "requiredSecrets": [], "consumerModel": "pull", "releaseTrigger": "two-phase", - "driftNotes": ["EDA/KiCad part library; delivers a github-release data zip.", "main is stale (data + README only): the full fleet CI, NBGV version.json, and the Python build/verify pipeline live only on develop - promote to main to converge.", "Python tooling uses requirements-dev.txt, not pyproject.toml; no repo-config/ rulesets."] + "driftNotes": ["EDA/KiCad part library that delivers a github-release data zip.", "main is stale (data + README only): the full fleet CI, NBGV version.json, and the Python build/verify pipeline live only on develop - promote to main to converge.", "Python tooling uses requirements-dev.txt, not pyproject.toml. No repo-config/ rulesets."] }, { "name": "EspDinIoT", @@ -190,7 +190,7 @@ "requiredSecrets": [], "consumerModel": "pull", "releaseTrigger": "none", - "driftNotes": ["EDA/KiCad PCB design; main is a stub - the real design lives on develop and feature/schematic, populate main.", "No CI on any branch; develop has partial governance adoption (AGENTS.md/.editorconfig) but no workflows."] + "driftNotes": ["EDA/KiCad PCB design. Its main is a stub - the real design lives on develop and feature/schematic, populate main.", "No CI on any branch, and develop has partial governance adoption (AGENTS.md/.editorconfig) but no workflows."] }, { "name": "ESPHome-Config", @@ -221,7 +221,7 @@ "requiredSecrets": [], "consumerModel": "pull", "releaseTrigger": "dispatch-only", - "driftNotes": ["Home Assistant CONFIGURATION (configuration.yaml + automations/blueprints), NOT a HACS integration (no custom_components/manifest.json, no hacs.json).", "Operational onboarding completed 2026-07-17 (HomeAssistant-Config #16): master->main rename + develop created, advisory lint CI (Check pull request workflow status job required check), dispatch-only source release (version.json + NBGV + publish-release.yml), repo-config operational carry (rulesets/settings applied and verified in sync), Dependabot + App merge-bot with the CODEGEN_APP_* pair in both stores, adapted self-audit (AUDIT.md + spec/secrets.json); baseline promoted develop->main via HomeAssistant-Config #17.", "groundTruthBranch intentionally main: develop is the working branch (direct signed commits), main the promoted stable snapshot the audit targets - deliberately not flipped to develop (ptr727/ProjectTemplate#340).", "Private; deployed by git pull into the HA config dir."] + "driftNotes": ["Home Assistant CONFIGURATION (configuration.yaml + automations/blueprints), NOT a HACS integration (no custom_components/manifest.json, no hacs.json).", "Operational onboarding completed 2026-07-17 (HomeAssistant-Config #16): master->main rename + develop created, advisory lint CI (Check pull request workflow status job required check), dispatch-only source release (version.json + NBGV + publish-release.yml), repo-config operational carry (rulesets/settings applied and verified in sync), Dependabot + App merge-bot with the CODEGEN_APP_* pair in both stores, adapted self-audit (AUDIT.md + spec/secrets.json). The baseline was promoted develop->main via HomeAssistant-Config #17.", "groundTruthBranch intentionally main: develop is the working branch (direct signed commits), main the promoted stable snapshot the audit targets - deliberately not flipped to develop (ptr727/ProjectTemplate#340).", "Private, deployed by git pull into the HA config dir."] }, { "name": "DevKitCIoT", @@ -234,7 +234,7 @@ "requiredSecrets": [], "consumerModel": "pull", "releaseTrigger": "none", - "driftNotes": ["EDA/KiCad PCB design + fabrication data (gerbers/BOM); no CI on any branch, no release pipeline.", "Depends on KiCadLibrary as an upstream part source (manual git clone).", "Branch hygiene: develop and main have diverged (not forward-only); feature/kicad10-upgrade is merged (PR#4) but retained for the in-progress KiCad 10 migration. A stale THT branch was removed in the 2026-07 sweep."] + "driftNotes": ["EDA/KiCad PCB design + fabrication data (gerbers/BOM), with no CI on any branch, no release pipeline.", "Depends on KiCadLibrary as an upstream part source (manual git clone).", "Branch hygiene: develop and main have diverged (not forward-only), and feature/kicad10-upgrade is merged (PR#4) but retained for the in-progress KiCad 10 migration. A stale THT branch was removed in the 2026-07 sweep."] }, { "name": "PhotoCleaner", @@ -259,7 +259,7 @@ "requiredSecrets": ["NUGET_USERNAME", "CODECOV_TOKEN"], "consumerModel": "pull", "releaseTrigger": "two-phase", - "driftNotes": ["Rulesets applied from the hub canonical repo-config/, not committed in-repo (no repo-config/ directory), as with NxWitness.", "No WORKFLOW.md sibling doc (pending fleet-wide ratification, ptr727/ProjectTemplate#223); workflow comments carry the rationale inline."] + "driftNotes": ["Rulesets applied from the hub canonical repo-config/, not committed in-repo (no repo-config/ directory), as with NxWitness.", "No WORKFLOW.md sibling doc (pending fleet-wide ratification, ptr727/ProjectTemplate#223). Workflow comments carry the rationale inline."] }, { "name": "AudioCleaner", @@ -288,7 +288,7 @@ "requiredSecrets": [], "consumerModel": "pull", "releaseTrigger": "dispatch-only", - "driftNotes": ["Follows the fleet LF default and pins only `*.dc` to CRLF (`.gitattributes` `*.dc text eol=crlf`, `.editorconfig` `[*.dc] end_of_line = crlf`), a repo-local exception for Design Center's own Windows-native XML export format (ptr727/Vantage-Config#28).", "Recreated lean and single-platform: Design Center is freely available, so no installer archives are kept; split out of HomeAutomation-Config.", "Operational onboarding completed 2026-07-16 (Vantage-Config #9): baseline docs, advisory lint CI, dispatch-only publisher, repo-config operational carry (rulesets/settings applied and verified in sync), Dependabot + App merge-bot with the secret pair in both stores, adapted self-audit (AUDIT.md + spec/secrets.json)."] + "driftNotes": ["Follows the fleet LF default and pins only `*.dc` to CRLF (`.gitattributes` `*.dc text eol=crlf`, `.editorconfig` `[*.dc] end_of_line = crlf`), a repo-local exception for Design Center's own Windows-native XML export format (ptr727/Vantage-Config#28).", "Recreated lean and single-platform: Design Center is freely available, so no installer archives are kept. Split out of HomeAutomation-Config.", "Operational onboarding completed 2026-07-16 (Vantage-Config #9): baseline docs, advisory lint CI, dispatch-only publisher, repo-config operational carry (rulesets/settings applied and verified in sync), Dependabot + App merge-bot with the secret pair in both stores, adapted self-audit (AUDIT.md + spec/secrets.json)."] }, { "name": "HolidayLights", @@ -300,7 +300,7 @@ "requiredSecrets": [], "consumerModel": "pull", "releaseTrigger": "none", - "driftNotes": ["xLights show sequences/models (asset repo).", "main is near-empty - all content lives on develop; promote to main.", "No CI/governance scaffolding."] + "driftNotes": ["xLights show sequences/models (asset repo).", "main is near-empty - all content lives on develop. Promote to main.", "No CI/governance scaffolding."] }, { "name": "Blog", @@ -317,7 +317,7 @@ "consumerModel": "pull", "releaseTrigger": "dispatch-only", "pythonDirectories": ["checks", "scripts"], - "driftNotes": ["Hugo static site migrated off WordPress.com, stood up 2026-08-01; release model with a dispatch-only publisher that cuts the tag and a source archive.", "lineEndings lf on a release repo, where the rule grants the native-platform default to operational repos only: every consumer is Linux (the Hugo build, the Caddy container, and the rsync deploy).", "content/ is an imported WordPress archive, so the prose, spelling, and style gates are scoped to exclude it.", "Deploy credentials are per-environment GitHub Environment secrets and variables, which spec/secrets.json has no vocabulary for, so requiredSecrets leaves them out and the names are declared in this repo's own secrets.json environments block; listing them here would make the audit demand them in the repository actions store.", "Neither deployment environment carries a branch policy, deliberately: the ref gate that admits production from the default branch only runs as a job in the deploy workflow, before anything is installed or written (WORKFLOW.md D2.1). branchPolicy none records that, so a policy appearing later reads as a change rather than as the gate arriving.", "capture/ is one-shot WordPress migration tooling that has already run, so the maintainer kept it out of pythonDirectories (Blog #356, 2026-10-01), forgoing the lint, format, type-check, test, and coverage obligations every Python directory owes. The audit's python-directories drift naming capture/ files is therefore expected."] + "driftNotes": ["Hugo static site migrated off WordPress.com, stood up 2026-08-01. Release model with a dispatch-only publisher that cuts the tag and a source archive.", "lineEndings lf on a release repo, where the rule grants the native-platform default to operational repos only: every consumer is Linux (the Hugo build, the Caddy container, and the rsync deploy).", "content/ is an imported WordPress archive, so the prose, spelling, and style gates are scoped to exclude it.", "Deploy credentials are per-environment GitHub Environment secrets and variables, which the audit does not check, so requiredSecrets leaves them out. Listing them in requiredSecrets would make the audit demand them in the repository actions store.", "Neither deployment environment carries a branch policy, deliberately: the ref gate that admits production from the default branch only runs as a job in the deploy workflow, before anything is installed or written (WORKFLOW.md D2.1). branchPolicy none records that, so a policy appearing later reads as a change rather than as the gate arriving.", "capture/ is one-shot WordPress migration tooling that has already run, so the maintainer kept it out of pythonDirectories (Blog #356, 2026-10-01), forgoing the lint, format, type-check, test, and coverage obligations every Python directory owes. The audit's python-directories drift naming capture/ files is therefore expected."] }, { "name": "DiskSpeedTest",