Conversation
`scripts/pr_review.py --repo` defaulted to `ptr727/ProjectTemplate`, so a run that omitted it read a pull request **here** rather than the one in front of it. The number resolved, the digest rendered, and no field in the output disagreed. Two runs in other repositories digested a pull request here before the maintainer caught it, one of them recovering with "my run digested the hub's #107". A default cannot be made safe for this argument. Every pull request number is valid in every repository, so the wrong target is never a lookup failure and never surfaces as one. ## Two changes, because the argument and the output each hid the miss | Surface | Change | | --- | --- | | `scripts/pr_review.py` argument | `--repo` is required with no default, and its value must be `OWNER/NAME` | | `scripts/pr_review.py` digest | The summary line leads with `repo=OWNER/NAME` | A bare `ProjectTemplate` is the near-miss a required argument still admits, and it previously raised an unpacking traceback rather than naming which half was missing. It is now rejected as `--repo takes OWNER/NAME, not 'ProjectTemplate'`. The digest names the repository because a digest of the wrong pull request is well-formed, and a number alone reads as correct anywhere. That is what makes a misdirected run visible in its own output rather than only to a reader who already suspects it. ## Coverage `TestCli` runs go through a helper that supplies the repository, and three cases were added: a run naming none is rejected, each malformed shape is rejected by name, and the digest names what it read. The `--pickup-grace -1` case now passes `--repo` and asserts on the grace error. It was about to exit on the missing argument instead, which is the same `SystemExit` and would have passed for the wrong reason. ## Carried surfaces `GOVERNANCE.md` "PR Review Etiquette" and `scripts/README.md` update their invocations to match. The `GOVERNANCE.md` section is carried verbatim fleet-wide, so it reaches downstream repositories on the next re-vendor, where the corrected invocation matters most. ## Verification ```text python3 scripts/test_pr_review.py 70 tests, OK python3 scripts/test_prose_lint.py 163 tests, OK python3 scripts/test_repo_gate.py 23 tests, OK python3 scripts/repo_gate.py eol 0, sha-pin 0 prose_lint, both CI invocations clean on all four changed files ``` Not addressed: `TODO.md` still records the open question of whether `GOVERNANCE.md` may name `scripts/pr_review.py` at all, given the fleet carries the section but not the script. This makes that pointer correct without settling it. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two days of onboarding more repositories to the hub produced seventeen recorded observations. Each was checked against `develop` at `b82c1a3`, the committed `TODO.md`, and the eleven open issues before being written down, because the value of a feedback pass is in what it genuinely uncovers rather than in the count of what it recorded. Ten are new entries. Three amend an entry that had already reasoned about the same surface from a single example and needed the second. One was already fully covered and is left untouched. Two are rules that already exist, already state the common case, and leave unstated exactly the case that keeps recurring. ## Disposition | # | Observation | Disposition | | --- | --- | --- | | 1, 2 | Shorthand for the hub, and for the review loop | New entry, Carried Content | | 3 | `pr_review.py --repo` default | Already shipped as #575 | | 4, 6 | Hub-hosted tooling, drop duplicated downstream config scripts | Amends "Reduce the surface that is copied downstream at all" | | 5 | Agents hand-rolling checks the hub already ships | Amends "Decide where a carried file may name hub-only machinery" | | 7 | A table of contents for a long document | New entry, Carried Content | | 8 | The README intro above the tagline | New entry, Audit and Spec Model | | 9 | An operational repository still opens a pull request for a large change | New entry, Carried Content | | 10 | How a `hugo` repository carries its theme | Amends the `hugo` type entry | | 11 | Local defensive-review subagents ahead of the remote reviewer | New entry, Fleet Chores | | 12 | Shared actions to stop duplication | Already covered, no edit | | 13 | Shell against Python with unit tests | New entry, Gate Honesty | | 14 | Helper functions for the commands that keep failing | New entry, Gate Honesty | | 15 | When an issue is closed by hand | New entry, Carried Content | | 16 | The clickable link ahead of the prompt | New entry, Carried Content | | 17 | A register of disproved review findings | New entry, Fleet Chores | ## The three amendments each add the example their entry was missing Every one of the three had already reasoned correctly from one instance. What each lacked was the second instance that shows the reasoning generalizes, which is what turns a worked example into a decision worth making. The copied-surface entry argued the case for `repo-config/configure.sh` alone. It now states the wanted end state, that a script identical everywhere lives in the hub once and a downstream repository calls it, and that the fleet already runs both models without either having been chosen against the other. It also separates out the half that needs no decision to start on, which is deleting a downstream config script whose content is only ever the hub's. The hub-only-machinery entry had the reader-follows-a-dead-pointer problem and no evidence that anyone had. It now carries the measured cost: several sessions had to be redirected by hand to `scripts/pr_review.py` and `scripts/prose_lint.py` after building worse versions of them, and none of those agents did anything wrong, since the tool they were pointed at is absent from their tree and nothing they can read says how to reach it. The `hugo` entry recorded that Blog's theme is vendored with no upstream ref and never asked whether that is what the type should require. The question is framed as one about the type rather than about Blog, because whatever is decided becomes the type's contract and the second generator inherits it. ## Two rules already exist, and the wording is the defect `Closes #N` is stated at `GOVERNANCE.md` "Git and Commit Rules" and is carried fleet-wide, so downstream agents re-deriving it and reporting it as a discovery are missing it rather than lacking it. What is genuinely absent is the case that produces the confusion, which is work complete on `develop` while a promotion is not imminent, leaving the issue open with nothing recording that it is done. The current text licenses a hand-close only once a promotion has merged without the keyword, which is the narrower case. The clickable-link rule gets the hard part right, that an interactive prompt renders neither a Markdown link nor a bare URL, so the reference inside it is a bare number and the link goes in the message that accompanies it. It says accompanying rather than preceding, and a message emitted after the prompt is not read before the question is answered, which is the entire outcome the rule exists to produce. Reading both recurrences as underspecified wording rather than as the rule being ignored is what makes either of them fixable, and it is the same diagnosis in both cases. ## One entry records a live defect rather than a preference `spec/readme-structure.md` item 1 reads as though the canonical description **is** the paragraph after the H1, and `CODESTYLE.md` then has `HISTORY.md` copy "the same intro paragraph" verbatim. Together they forbid a README from saying anything further about itself above the fold, and `spec/audit.py` measures the first non-empty line, so a legitimate second paragraph is reported as a finding. That is the same axis as declaring the description in `registry/repos.json`, so the entry says the two want one decision rather than two taken in sequence. ## Verification `prose_lint.py --diff`, `repo_gate.py`, and `markdownlint-cli2` are clean over the change. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Promotes the latest develop changes to main, primarily tightening the safety of the PR review digest tooling (by requiring an explicit repository target and surfacing it in output) and folding the onboarding-feedback pass into the long-lived backlog.
Changes:
- Make
scripts/pr_review.pyrequire--repo OWNER/NAME(no default) and includerepo=OWNER/NAMEin the digest output. - Update/extend CLI tests and documentation to match the required
--repobehavior and its error cases. - Expand
TODO.mdwith the onboarding-feedback backlog entries and new cross-references.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TODO.md | Adds onboarding-feedback backlog entries and supporting references/links. |
| scripts/test_pr_review.py | Updates CLI tests to always pass --repo (except when testing errors) and adds new coverage for repo argument validation/output. |
| scripts/README.md | Updates usage examples and documents why --repo is required and how mis-targeting is made visible. |
| scripts/pr_review.py | Requires --repo, validates OWNER/NAME shape, and prefixes digest output with repo=... to prevent silent cross-repo misreads. |
| GOVERNANCE.md | Updates the runbook invocation to include --repo and explains why the repository must be explicit. |
Uh oh!
There was an error while loading. Please reload this page.
ptr727 added a commit
that referenced
this pull request
Aug 6, 2026
…, and the operational pull-request trigger (#589) Promotes the five commits develop carried since #582: #584 (a Copilot refusal is not coverage), #586 (the reference-definition collation), #587 (CI on a pull request into an operational develop), #588 (hub-hosted tooling, de-vendoring configure.sh), and #590 (the two definition blocks #588 pushed out of order). Merge commit, no squash, develop not deleted. git merge-tree reported no conflict and wrote a tree byte-identical to origin/develop. GOVERNANCE.md is verbatim-carried and three of these commits change it, so a fleet re-vendor is owed, including an entirely new carried section. #588 also turns every downstream repo-config/configure.sh copy into a deletion rather than a convergence, and #587 leaves a one-line trigger change owed by four operational repos.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes the two commits
develophas carried since the previous promotion (#574) tomain. Merge commit only, no squash, anddevelopis not deleted.What lands
--repoonscripts/pr_review.py, since the old default silently digested a pull request in the hub when a run in another repository omitted it, and lead the digest summary with the repository it read so a misdirected run is visible in its own output.TODO.md: ten new entries, three amendments that each supply the second example their entry was reasoning from one instance without, and two recurring cases traced to underspecified wording in rules that already exist.Issues closed
None. Neither constituent pull request carried a closing keyword, and the four issues #576 filed (#577, #578, #579, #580) record work that is still open rather than work this promotion completes.
Verification
git merge-tree --write-tree origin/main origin/developreports no conflict, so this promotion needs no throwaway resolution branch, and the tree diff againstmainis exactly the five files the two commits touch. Both constituent pull requests merged green with their review loops closed.