Conversation
Refs #2268 ## Summary The hub's own prose said `spec/secrets.json` has no vocabulary for per-environment secrets, and that a repo declares them in its own `environments` block. Both claims are false. They are also how Blog's registry note went wrong (#2269). The real gap is different: the schema's `environments` block has no per-repo dimension, and downstream copies of `spec/secrets.json` are retired (`spec/divergences.json`), so no repository can declare its environment secret names there. | File | Now says | | --- | --- | | `spec/secrets.json` top note | The schema still allows an `environments` block, whose `environmentSecrets` map lives inside it, but it has no per-repo place, so this file carries none, and no tool would read one | | `spec/secrets.json` `deploy-ssh` | Drops the "no vocabulary" claim and the pointer to a repo's own `environments` block | | `spec/project-types.json` | No tool here checks these credentials. A repo leaves them out of `requiredSecrets` because the audit would look for them in the repository store | | `docs/reusable-workflows.md` | `spec/secrets.json` declares none of the deploy secrets, and no hub tool checks the secrets an environment holds. The environment boundary's own branch policy is checked elsewhere | | `TODO.md` | The settled entry stops saying a repo may declare its environment names there | The edited note's one semicolon is also recast. `STANDUP.md` makes the same claim in its prerequisite list. Two rewrites of it each drew a new false claim under review, so it stays at its develop wording, and #2270 tracks it. ## Verification - `local-strict-review`: across four passes, each changed sentence was checked against `spec/secrets.schema.json`, `spec/divergences.json`, `docs/repo-config.md`, `spec/audit.py`, `spec/validate.py` and `repo-config/configure.sh`. The four statements here verified true. The one finding left, `STANDUP.md`, is filed as #2270, and 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 <noreply@anthropic.com>
## Summary Implements the maintainer's answer on #2272, option 1. A full Copilot file table on an earlier round now counts as coverage of a later head whose rounds carry no table, when the pull request changes exactly the same set of files at both commits. That is the bound `carry_holds` already applies to a carried coverage statement. - `carried_table` picks the newest non-refusal round carrying a table. `table_reading` consults it only where no round covering the head carries a table of its own, and only after the partial-on-record and review-window guards pass. The carry also requires that table to name exactly the changed files and `carry_holds` to confirm the change set at both commits. - `status` prints `coverage=carried:table` with the round's commit and the delta. `report_verdict` and the digest share the reading, and the memoized compare keeps them in agreement. - A partial table, a partial on record anywhere on the pull request, a review history past the window, or a change set that moved or could not be read each still goes to the maintainer. - `table_covers` had no callers left once `table_reading` existed, so it is gone and its reasoning moved into `table_reading`. - One sentence states the carry on each surface that states the Merge Gate's table reading: `GOVERNANCE.md`, `pr-review-conduct`, the Copilot runbook, and `scripts/README.md`. ## Verification - `python3 -m unittest tests.test_pr_review`: 507 tests pass. `TestFileTableCarriesForward` covers the carry, a moved and an unreadable change set, a partial table, a partial on record, a truncated history, a head table that misses the diff, the newest-table choice, and a refusal carrying a table. - Each new test was shown to fail with its behavior reverted or mutated. - Live check against #2275, an open PR in this exact shape: develop's `pr_review.py` exits 45 there, and this branch reads `coverage=carried:table` and exits 0. - Two local strict-review passes ran. Round 1 raised 7 findings, 6 fixed and 1 declined: the newest table is picked by submission time, the same rule `carried_coverage` uses. Round 2 raised 4 low findings, all fixed. Closes #2272 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…rectory in Notes (#2275) ## What was wrong `resolve_selection` ended its loop on a bare `[[ ... ]] && SELECTED+=(...)`. When the last managed tool was not requested, the function returned 1 and `main` stopped silently under `set -e`, so `install-tools.sh git` exited 1 with no output (#2256). Report notes also wrote the absolute path of a shadowing or downloaded copy verbatim, which put the account name into a report meant to be pasted elsewhere. The Windows installer already routes printed paths through `Hide-Home`, the Linux one had no peer (#2260). ## Fix - The loop in `resolve_selection` uses an `if`, so the function returns 0. - New `hide_home` rewrites a path equal to `$HOME`, or starting with `$HOME/`, to `~`. The ripgrep, jq, uv, and git-restore-mtime notes that name a resolved path use it, so the table and `--json` both benefit. - Scanned the rest of the script for the same bare `cond && action` ending a function. The remaining instances are either followed by more statements or sit in a `while true` that only ends through `break`, so none can return 1. ## Tests In `tests/test_install_tools.py`: selection naming a tool other than the last succeeds, a note path under a constructed home renders with `~` in the `--json` report, and `hide_home` leaves a sibling directory sharing the home prefix alone. With the fix reverted the first two fail, with it restored all pass. Fixes #2256 Fixes #2260 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
## Summary The quota-safety half of #2261, per the maintainer's answers recorded on that issue: a generic error round is a possible quota hit after one occurrence, and the tooling never requests into it. - **Error rounds.** "Copilot encountered an error and was unable to review this pull request" is what the weekly rate limit posts. The maintainer pointed out that the real cause shows in the round's details. `pr_review.py` now finds the reviewer's own failed Actions run on that commit (`dynamic/agents/copilot-pull-request-reviewer`) and reads its job log. A logged `errorType: 'rate_limit'` reports as `refusal=QUOTA` with the reset time the log states. Anything else, an unreadable log included, reports as `refusal=ERROR`, a possible quota hit. - **`wait` stops requesting.** No request goes out while the pull request's newest Copilot review is a quota or error refusal, on this head or an earlier one, and `wait` exits `46`. A request already pending is still polled for. Where nothing covers the head, the digest's `refusal=` field reads that refusal from the earlier head. - **Defect fixed.** `wait` used to send its auto-request before it consulted the repository-wide quota signal, so an exhausted account still spent a request. It now holds the request back under that signal too. - `--ignore-quota-signal` overrides both stops. The defined-moments half of #2261 (the ruleset triggers, the local-pass attestation, and the Merge Gate change) follows in a separate pull request, since it rewrites this same `wait` region. ## Verification - `python3 -m unittest tests.test_pr_review`: 507 tests pass. The liveness payload in the stop tests is body-less, as the real `Q_LIVE` is. Each new behavior was shown to fail with the change reverted or mutated. - Live check: `run_cause` against the real error round on #2254 returns `rate_limit` with "reset on October 5, 2026 at 12:00 AM". - Two local strict-review passes. Round 1 found 12 findings, including a critical one: the stop was read from the body-less liveness payload, so it never fired. Round 2 found 6. All are fixed except two deliberate declines, the time bound from the parsed reset date and the unchanged `#` comment at the request site. Refs #2261 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ted Local Pass (#2278) ## Summary This is the defined-moments half of #2261, per the maintainer's answers recorded on that issue. A Copilot round is requested when a pull request opens and on the head of a pull request into the default branch, a promotion among them, rather than on every push. On a pull request into any other branch, a fix push after the first round is covered by the recorded local strict-review pass the push already owes. The PR publishes that pass so the review gate can read it. - **Ruleset.** `repo-config/develop.json` and `main.json` set `review_on_push` and `review_draft_pull_requests` to false and keep review on open. Applying the change to live repositories is a separate config run, which needs its own go-ahead. Narrowing the account-level setting is a UI step for the maintainer. - **`pr_review.py attest`.** It publishes the local pass as a comment carrying `<!-- fleet-local-review: head=<sha> findings=<n> -->`, after confirming four things: - the checkout is at the pushed head - the checkout holds no other change - its merge base is the pull request's own - `local_review.py check` passes against the base The comment carries the findings count the pass recorded. Per the maintainer's answer, the count is shown and not gated, since local findings are advisory. `local_review.py status` now reports the per-reviewer `findings` map that this reads. - **`status`.** An attested head on a pull request into a non-default branch, after Copilot's first round, reads as `review_on_head=local` and `coverage=local`. That holds only for an owner, member, or collaborator comment whose marker stands as a line of its own outside a fence, and only where the whole review history is in view and no round on record states or appears to state partial coverage. - **`wait`.** On such a head `wait` requests nothing. An attested head ends as covered, and one with no attestation exits `49` (`AWAITING_LOCAL_PASS`). `--request` asks for a round anyway. A promotion, a first round, a partial on record, and a history past the window are all requested as before. - **A post-merge finding on #2277, fixed here.** A newer quota or error refusal now outranks older genuine coverage of the same head, in the digest's `refusal=` field and in `wait`'s verdict, since it is the newest word on the pull request. - **CodeRabbit.** A drive prompts it at most once per pull request, on the head it judges final, and never after a rate-limit notice. - **Docs.** The rule is stated in `GOVERNANCE.md` "PR Review Etiquette", `pr-review-conduct` (loop step 3, step 8, Merge Gate item 2, and the reviewer bullets), the Copilot runbook, `scripts/README.md`, `repo-config/README.md`, `docs/pr-reviewer-reference.md`, and the one-liners in `README.md`, `RESYNC.md`, and `AUDIT.md`. ## Verification - `tests.test_pr_review` and `tests.test_local_review` pass (658 tests). Each new behavior was shown to fail with it reverted or mutated, including the stale-refusal, partial, old-answer, and marker-shape cases. - A live GraphQL read confirmed that `baseRepository.defaultBranchRef` and `IssueComment.authorAssociation` exist. - Two local strict-review passes ran. Round 1 raised 16 findings: 15 are fixed, and #5 went to the maintainer, who chose to show the count without gating on it. Round 2 raised 3 low findings, all fixed. Closes #2261 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2284 +/- ##
==========================================
+ Coverage 57.31% 58.22% +0.91%
==========================================
Files 16 16
Lines 7611 7821 +210
==========================================
+ Hits 4362 4554 +192
- Misses 3249 3267 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate findings remain in scripts/pr_review.py.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Promotes develop to main with review automation, quota safety, local attestations, documentation, configuration, and installer updates.
Changes:
- Adds quota/error handling, review-table carry-forward, and local attestations.
- Updates review triggers, secret guidance, tests, and installer behavior.
- Regenerates related review skills and documentation.
Review findings:
- Critical —
scripts/pr_review.py:3864: Revalidate the merge base before publishing attestations. - Moderate —
scripts/pr_review.py:4489: Let--ignore-quota-signalbypass the local-pass hold. - Moderate —
scripts/pr_review.py:3988: Ensure current reviews do not bypass--min-rounds. - Nit —
scripts/pr_review.py:3849: Correctly name the merge-base mismatch status.
| File | Summary |
|---|---|
TODO.md |
Corrects environment-secret documentation. |
tests/test_pr_review.py |
Tests review, quota, table, and attestation behavior. |
tests/test_local_review.py |
Tests findings reporting. |
tests/test_install_tools.py |
Tests installer selection and path hiding. |
spec/secrets.json |
Clarifies environment-secret scope. |
spec/project-types.json |
Corrects Hugo secret guidance. |
scripts/README.md |
Documents the updated review workflow. |
scripts/pr_review.py |
Implements quota safety, table carry, and attestations. |
scripts/local_review.py |
Reports findings per reviewer. |
RESYNC.md |
Updates review-loop guidance. |
repo-config/README.md |
Documents review trigger configuration. |
repo-config/main.json |
Disables push and draft review triggers. |
repo-config/develop.json |
Disables push and draft review triggers. |
README.md |
Updates the review-loop summary. |
host-setup/linux/install-tools.sh |
Fixes selection status and hides home paths. |
GOVERNANCE.md |
Defines review moments and local-pass coverage. |
docs/reusable-workflows.md |
Corrects environment-secret documentation. |
docs/pr-reviewer-reference.md |
Documents local-pass behavior. |
AUDIT.md |
Updates review guidance. |
.github/skills/pr-review-conduct/SKILL.md |
Regenerated review skill. |
.github/copilot-instructions.md |
Updates Copilot review instructions. |
.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.md |
Regenerated Claude skill. |
.claude-plugin/fleet-skills/.source-digests/pr-review-conduct |
Updates the generated-source digest. |
.agents/skills/pr-review-conduct/SKILL.md |
Updates the canonical review skill. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Summary
Promotes develop to main with the pull requests below. Each was already reviewed and merged into develop.
install-tools.shno longer exits 1 silently when the tools named leave out the last managed tool, and its report notes show a home path as~.pr_review.pyreads the reviewer's own Actions run log, which states the rate limit and the time it resets, andwaitno longer requests a review into either case.pr_review.py attestand read bystatusasreview_on_head=local.repo-config/develop.jsonandmain.jsonnow reviews on open only, not on push or for drafts.Applying that ruleset change to the live fleet repositories is a separate config run after this merges, and it needs the maintainer's go-ahead. Until it runs, GitHub still reviews every push, including pushes to this pull request.
Closes #2256
Closes #2260
Closes #2261
Closes #2268
Closes #2272
🤖 Generated with Claude Code