review: enable scoped re-reviews (follow-up to the v1.7.0 bump) - #277
Merged
Conversation
…1.6.1; enable flip-gated re-reviews The installed copy (.github/workflows/review.md) was still at review-v1.4.1, the version gh aw add pinned on install (#241), and had never been updated: five releases behind the shared source. Found via PR #266 review 4739236283, a REQUEST_CHANGES with the bare body "Changes requested — see inline comments." and zero inline comments; v1.5.0's code-rendered re-review accountability section (#244) exists to name the still-open blocking thread in exactly that case, and v1.6.1 carries the #262 fix that keeps a blocking verdict from silently downgrading to COMMENTED when the body is empty. gh aw update cannot perform this bump: gh-aw v0.81.6 resolves a tag ref only when isSemanticVersionTag() accepts it, which rejects this repo's changesets-style prefixed tags (review-v1.4.1), falls through to branch resolution, and 404s. This commit replicates the documented 3-way merge by hand (base review-v1.4.1, ours the installed copy, theirs review-v1.6.1) and recompiles with gh aw compile. All KHAN/ACTIONS LOCAL OVERRIDE edits are preserved: the fork guard, the disabled observability block (the GH_AW_OTEL_SENTRY_* secrets still do not exist in this repo or the org), and max-ai-credits 2500, now mirrored into the new REVIEW_MAX_AI_CREDITS env var per the KEEP IN SYNC rule the upstream frontmatter added. Also sets the v1.6.0 re-review mode dial to flip-gated in .github/aw/review/ROUTING: repeat reviews run thread reconciliation plus the correctness pass over new hunks, with a REQUEST_CHANGES -> APPROVE flip vetoed by any validated blocking finding; the first full review of a ready PR and the 0.4 divergence tripwire keep full-depth coverage. Verified against the real parser: reReviewMode flip-gated, zero warnings.
…t the re-review mode dial out Review feedback on the v1.6.1 bump: - review-pins.test.ts now diffs the installed .github/workflows/review.md against the shared source at the pinned release tag (fetching the tag when the checkout is shallow) and fails if any hunk lacks a KHAN/ACTIONS LOCAL OVERRIDE marker. gh aw update cannot resolve changesets-style tags, so bumps here are manual 3-way merges; the pin tests checked version consistency but nothing verified the merged content, and the next manual bump could silently drop an override or an upstream hunk. - Drop the flip-gated ROUTING change: the motivating defect (the empty REQUEST_CHANGES body on #266) is fixed by the version bump alone, and the mode dial is a separable cost/depth decision; splitting it keeps the bump revertable and the behavior change independently attributable. It follows in its own PR, starting at scoped per the same review.
|
khan-actions-bot
requested review from
a team,
jaredly and
somewhatabstract
and removed request for
a team
July 21, 2026 05:34
Contributor
Review Guidancegithub-actions (1 file)
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| # review when a push rewrites enough of the PR. Scoped rather than | ||
| # flip-gated: composite actions here run in consuming repos' CI with their | ||
| # credentials, so a small post-review push should still be seen by the | ||
| # whole-change reviewers, not only the correctness pass. Graduate to |
Contributor
There was a problem hiding this comment.
thought (non-blocking): This rationale also holds under flip-gated: both modes stage the same new-hunks diff, and flip-gated's always-on correctness pass still sees every post-review push. The real delta is whether the whole-change reviewers also see the new hunks — and their distinctive value is whole-change context that new-hunks staging strips. So scoped-vs-flip-gated is a risk-posture call, not a "new hunks must be seen" one (the README's measured evidence backs scoped over reconcile-only, not over flip-gated). The planned graduation step reasonably hedges this.
| # flip-gated: composite actions here run in consuming repos' CI with their | ||
| # credentials, so a small post-review push should still be seen by the | ||
| # whole-change reviewers, not only the correctness pass. Graduate to | ||
| # flip-gated once re-reviews here show scoped is overpaying (see |
Contributor
There was a problem hiding this comment.
question (non-blocking): "Graduate to flip-gated once re-reviews here show scoped is overpaying" names no measurable signal. Consider naming a falsifiable exit condition (e.g. N consecutive scoped re-reviews where no whole-change reviewer produced a validated finding the correctness pass missed) so the dial has a defined trigger rather than an open-ended judgment call.
jeresig
approved these changes
Jul 21, 2026
…ew-v1.7.0 Re-ran the 3-way merge against the fresh release (#278: the Fable-5 correctness reviewer, the recap label fix + collapsed non-blocking recap with fail-closed unknown labels, and the unbounded-read lens). One conflict, the ref: pin, resolved override-comment + new version; source:/ref: both at review-v1.7.0; gh aw compile clean. The content guard verifies every remaining hunk vs the v1.7.0 tag carries a LOCAL OVERRIDE marker; full suite 1016 passing.
…is repo Split out of the v1.6.1 bump (#276) per its review: the bump alone fixes the motivating defect, so the mode dial lands separately and independently attributable. Scoped rather than flip-gated, also per review: this repo's composite actions run in consuming repos' CI with their credentials, and under flip-gated a small post-review push under the divergence tripwire would get only the correctness pass, with none of the enabled whole-change reviewers seeing the new hunks. Flip-gated remains the graduation step once re-reviews here show scoped is overpaying. Validated with the real parser: reReviewMode scoped, zero warnings.
jwbron
force-pushed
the
jwies/review-rereview-scoped
branch
from
July 21, 2026 19:47
493f7e0 to
f19c2be
Compare
jwbron
changed the title
Contributor
There was a problem hiding this comment.
Approved — no blocking issues found.
2 of 2 prior review threads are still unaddressed as of f19c2be:
2 non-blocking threads still open
- thought (non-blocking)
.github/aw/review/ROUTING:22: This rationale also holds underflip-gated: both modes stage the same new-hunks diff, and flip-gated's always-on corre... - question (non-blocking)
.github/aw/review/ROUTING:23: "Graduate to flip-gated once re-reviews here show scoped is overpaying" names no measurable signal. Consider naming a fa...
jwbron
added a commit
that referenced
this pull request
Jul 21, 2026
## Why The reviewer installed on this repo (`.github/workflows/review.md`) was still pinned at `review-v1.4.1`, the version `gh aw add` recorded on install (#241, 2026-07-10), and had never been updated since: five releases behind the shared source (now bumped all the way to v1.7.0, released today in #278 with the Fable-correctness reviewer, the recap fixes, and the unbounded-read lens). This surfaced on PR #266, where re-review 4739236283 posted a REQUEST_CHANGES whose entire body was "Changes requested — see inline comments." with **zero** inline comments attached (the blocking thread lives on an earlier review). Later releases address exactly that: - **v1.5.0 (#244/#246)**: the code-rendered re-review accountability section, which enumerates every still-open prior thread (linked, blocking first, "as of \<sha\>") in the verdict body. - **v1.6.1 (#262)**: REQUEST_CHANGES is always submitted with a non-empty body; the empty-body path silently downgraded a blocking verdict to COMMENTED (observed on Khan/webapp#40793). ## Why not `gh aw update` The documented update path fails on this repo: gh-aw v0.81.6's `resolveLatestRef` only treats the pinned ref as a tag when `isSemanticVersionTag()` accepts it, and that check (`semverutil.IsValid`) rejects changesets-style prefixed tags like `review-v1.4.1`. It falls through to branch resolution and 404s (`failed to get latest commit for branch review-v1.4.1`). Might be worth an upstream issue; until then, updates of this workflow are manual. ## What this PR does - Replicates `gh aw update`'s 3-way merge by hand: base = `review-v1.4.1` source, ours = installed copy, theirs = `review-v1.7.0` source (identical to current main), then `gh aw compile review`. Diff vs the v1.6.1 source is exactly the documented `KHAN/ACTIONS LOCAL OVERRIDE` edits and nothing else: - the same-repo fork guard in `if:` (public-repo hardening); - the `observability:` block stays commented out (the `GH_AW_OTEL_SENTRY_*` secrets still exist neither on this repo nor at org level; the block hard-fails the agent job when they are missing); - `max-ai-credits: 2500`, now mirrored into the new `REVIEW_MAX_AI_CREDITS` env var per the KEEP-IN-SYNC rule v1.6.0 added; - `source:` and the lib checkout `ref:` both bumped to `review-v1.7.0` (kept in lockstep, enforced by `review-pins.test.ts`). - **Per review: the "every difference is a documented override" claim is now CI-enforced, not hand-audited.** `review-pins.test.ts` diffs the installed copy against `workflows/review/review.md` at the pinned tag (fetching the tag when the checkout is shallow) and fails when any hunk lacks a `KHAN/ACTIONS LOCAL OVERRIDE` marker, so the next manual bump cannot silently drop an override or an upstream hunk. - **Per review: the re-review mode dial is no longer part of this PR.** The motivating defect is fixed by the version bump alone (the non-empty-body rule and the accountability section apply at the default `full` mode), so enabling a reduced-depth mode is a separable decision; it follows in its own PR starting at `scoped` rather than `flip-gated`, per the review's depth argument, once a re-review or two has been observed on v1.7.0 (#277). ## Verification - `pnpm test`: passes, including the new content guard, `review-pins.test.ts` pin consistency (source/ref/lock literals all `review-v1.7.0`), and `version-sync.test.ts`. The guard was negative-tested: an unmarked stray edit to the installed copy fails it. - `gh aw compile review`: 0 errors, 0 warnings. - No changeset needed: all three files are under `.github/`, the check's default exclusion, and the shared `workflows/review` package is untouched. Author: jwbron Reviewers: github-actions[bot], jwbron, jeresig, kevinb-khan Required Reviewers: Approved By: github-actions[bot], jeresig Checks: ✅ 8 checks were successful, ⏭️ 1 check has been skipped Pull Request URL: #276
jwbron
added a commit
that referenced
this pull request
Aug 3, 2026
🖍 _This is an audit!_ 🖍 ## Summary: ## Why The reviewer installed on this repo (`.github/workflows/review.md`) has been pinned at `review-v1.7.0` since #276 (2026-07-21), four releases behind the shared source. Everything the shared package has shipped since then runs in consuming repos but not here, including one change that makes an existing setting in our own `ROUTING` inert: - **v1.8.0 patch 034181f**: gh-aw's safe-output sanitizer strips XML/HTML comments, so the hidden fingerprint stamp a review body carries never reached the PR; every re-review planned `no-prior-fingerprint` and escalated to full depth. Our `ROUTING` has said `re-review scoped` since #277, and on v1.7.0 that dial does nothing. The plan CLI now falls back to the Step 9 cache-memory record and reports `stampSource`. - **v1.8.0**: the deterministic-orchestrator slices land. Staging becomes a `pre-agent-steps:` step (`lib/stage-pr.ts`), scripted dispatch becomes the only mode (the ROUTING `dispatch` dial is retired; we never set it), Steps 4-6 become code (`lib/submission-plan.ts`), and the dispatch-conformance gate blocks a verdict whose sub-agent outputs do not exist (the v1.7.0 acceptance trial caught the orchestrator submitting a REQUEST_CHANGES after dispatching zero sub-agents). - **v1.9.0/v1.10.0**: open-thread suppression actually fires (it was unreachable on every conforming run, so re-reviews re-posted findings an open bot thread already tracked), `threads.json` / `human-threads.json` are staged by code, and a suppression is attributed to its best-matching thread rather than the first one it clears. - **v1.11.0**: a sub-agent the provider blocks is named as a refusal rather than "malformed output", failure detail and per-agent tool-call counts are kept, and a refused reviewer falls back to `claude-opus-4-8` instead of silently costing coverage. Also picked up: Gerald `.github/NOTIFIED` support, the per-lens consumer payload seam, and the `documentation` reviewer (opt-in; not enabled here). ## Why not `gh aw update` Same as #276: gh-aw's `resolveLatestRef` rejects changesets-style prefixed tags (`review-v1.11.0`) as non-semver, falls through to branch resolution, and 404s. Updates of this workflow stay manual. ## What this PR does - Replicates `gh aw update`'s 3-way merge by hand: base = `review-v1.7.0` source, ours = installed copy, theirs = `review-v1.11.0` source (identical to current main), then `gh aw compile review`. - **Two local overrides retire, because upstream now carries them.** `timeout-minutes: 40` is the shared default as of v1.8.0 (82af000), and the `sandbox.agent.version: v0.27.27` pin plus the `models:` claude-fable-5 pricing block were removed at source in 98f686f. Both override comments said they went away at this bump; they do. - **The remaining differences are the documented overrides and nothing else**, enforced by `review-pins.test.ts`: the same-repo fork guard in `if:` and its `roles: all` comment (public-repo hardening), the commented-out `observability:` block (the `GH_AW_OTEL_SENTRY_*` secrets still exist neither on this repo nor at org level), `max-ai-credits: 2500` with its `REVIEW_MAX_AI_CREDITS` mirror, and the comment on the lib checkout `ref:`. - `source:` and the lib checkout `ref:` both move to `review-v1.11.0` in lockstep, and the recompiled lock picks up the new pre-agent staging step, the dispatcher's `npm ci`, the dispatch-conformance gate `post-steps` step, and `BASH_MAX_TIMEOUT_MS: 1200000` (the blocking dispatcher call). - No consumer-config change is needed: `.github/aw/review/ROUTING` carries no retired `dispatch` line, no `correctness-checks.md` alias to migrate, and the new `documentation` reviewer stays off until a repo adds `enable documentation`. ## Verification - `pnpm test`: 1618 tests across 69 files pass, including `review-pins.test.ts` (source/ref/lock literals all `review-v1.11.0`, and every hunk against the pinned source carries a `KHAN/ACTIONS LOCAL OVERRIDE` marker) and `version-sync.test.ts`. - `pnpm typecheck`: clean. - `gh aw compile review`: 0 errors, 0 warnings. - No changeset needed: both files are under `.github/`, the check's default exclusion, and the shared `workflows/review` package is untouched. Author: jwbron Auditors: jeresig, kevinb-khan Required Reviewers: Approved By: Checks: ⏭️ 8 checks have been skipped, ⌛ 1 check is pending, ✅ 2 checks were successful Pull Request URL: #318
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What
Sets
re-review scopedin.github/aw/review/ROUTING: repeat reviews of a PR run thread reconciliation plus the full enabled roster (holistic, completeness, test-adequacy, first-principles, conventions) over the new-hunks staging. The first full review of a ready PR still runs everything, and the 0.4 divergence tripwire re-arms full review when a push rewrites enough of the PR.Why scoped, not flip-gated
Per the review on #276: under
flip-gated, a small post-review push gets only thread-reconciler plus the correctness pass; on a repo whose own ROUTING rationale is that composite actions run in consuming repos' CI with their credentials, the enabled whole-change reviewers should keep seeing new hunks. No specialist lens is enabled here, so the roster delta between the modes is exactly those whole-change reviewers.flip-gatedstays available as the graduation step once re-reviews here showscopedis overpaying.Verification
parseRoutingConfig:reReviewMode: "scoped", zero warnings..github/only.