Skip to content

Promote develop to main: Table Carry, Quota Safety, Defined-Moments Review - #2284

Merged
ptr727 merged 5 commits into
mainfrom
develop
Oct 2, 2026
Merged

ptr727 merged 5 commits into
mainfrom
develop

Conversation

@ptr727

@ptr727 ptr727 commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Summary

Promotes develop to main with the pull requests below. Each was already reviewed and merged into develop.

  • #2271: Correct the false no-vocabulary claims about environment secrets.
  • #2276: A full Copilot file table on an earlier round carries forward to a later head when the pull request changes the same set of files at both commits, the same bound a coverage statement uses.
  • #2275: install-tools.sh no longer exits 1 silently when the tools named leave out the last managed tool, and its report notes show a home path as ~.
  • #2277: A Copilot round that says only "encountered an error" is read as a possible quota hit. pr_review.py reads the reviewer's own Actions run log, which states the rate limit and the time it resets, and wait no longer requests a review into either case.
  • #2278: A Copilot round is requested when a pull request opens and on the head of a pull request into the default branch, rather than on every push. Covering a fix push into develop:
    • A recorded local strict-review pass covers the push, published with pr_review.py attest and read by status as review_on_head=local.
    • The Copilot rule in repo-config/develop.json and main.json now 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

ptr727 and others added 5 commits October 1, 2026 18:50
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>
Copilot AI lite review requested due to automatic review settings October 2, 2026 15:49
@coderabbitai

coderabbitai Bot commented Oct 2, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: c5ad7288-fcbd-47dd-bb90-704a9cea74c3

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.03540% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.22%. Comparing base (73b9f77) to head (644b297).
⚠️ Report is 303 commits behind head on main.

Files with missing lines Patch % Lines
scripts/pr_review.py 92.03% 18 Missing ⚠️
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     
Flag Coverage Δ
python-3.13 58.22% <92.03%> (+0.91%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate findings remain in scripts/pr_review.py.

Review effort: Lite
Findings: 1 High severity

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-signal bypass 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.

Comment thread scripts/pr_review.py
@ptr727
ptr727 merged commit c65c0cf into main Oct 2, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants