[🔥AUDIT🔥] Bump the installed reviewer copy to review-v1.19.0 - #379
Conversation
|
Guidance for reviewersTriage notes for reviewers: risky files by owning team, repeated changes, and files excluded from review. github-actions (1 file)
Common patterns2 files: Version pin bumped from - ref: review-v1.18.0
+ ref: review-v1.19.0Excluded from review (1 file)Not individually reviewed — generated, formatting-only, or
review detailsreview-v1.19.0 | schema 2 | depth full | re-review scoped blocking-only | enable holistic,completeness,test-adequacy,first-principles,conventions,documentation |
| # Even though this IS Khan/actions, the reviewer runs the released lib, not | ||
| # the PR head; a PR must not be able to change the code that reviews it. | ||
| ref: review-v1.18.0 | ||
| ref: review-v1.19.0 |
There was a problem hiding this comment.
question (non-blocking): The pin covers the lib, but the prompt still comes from the PR head. review.lock.yml builds the prompt via {{#runtime-import .github/workflows/review.md}} in the activation job, whose actions/checkout carries no ref: (the tag-pinned checkout is the separate gh-aw-review-lib step in the agent job), so only the lib half is pinned while the prompt half is read from the PR merge ref. Does this comment's "a PR must not be able to change the code that reviews it" — and the review-v1.19.0 footer the 1.18.0 changelog tells auditors to key on — hold on a PR that edits review.md itself, like this one?
review details
found by first-principles| # (the two values must stay in sync per the upstream comment). | ||
| REVIEW_MAX_AI_CREDITS: "2500" | ||
| source: Khan/actions/workflows/review/review.md@review-v1.18.0 | ||
| source: Khan/actions/workflows/review/review.md@review-v1.19.0 |
There was a problem hiding this comment.
suggestion (non-blocking): Producer and consumer are the same repo, so these overrides need not be a fork. Four of the six LOCAL OVERRIDE blocks (fork guard, disabled observability, raised max-ai-credits, its env mirror) diverge from workflows/review/review.md sitting in this same repo, which this team owns and can change. The divergence is load-bearing enough to need review-pins.test.ts, a dedicated review-consumer-bump skill, and a hand-resolved conflict on this bump.
A sketch, not a committable replacement:
Make the four divergent knobs upstream-configurable — observability conditioned on the OTEL secrets being present, the credit cap read from its existing env value, the fork guard as a documented public-repo setting — so this repo's installed copy converges on the released file and a bump becomes a three-line ref change with no merge.
🖍 This is an audit! 🖍
Bumps this repo's own installed reviewer copy from review-v1.18.0 to review-v1.19.0 (the tag cut from #374).
The hop is just the 1.19.0 release: adjudicated-corpus suppression drops its same-path key, author acknowledgments read as "fix pending" instead of unaddressed, the thumbs sweep is deleted (this repo never installed the sweep workflow), dispatcher deaths post a PR comment, a subject restating the discussion gets dropped, plus the entity-decoding gate fix and the checker's loud failure on a bad --repo path.
The 3-way merge had one conflict, the expected shape: the pre-agent-steps checkout ref sits inside its own LOCAL OVERRIDE comment, resolved by keeping the comment and taking ref: review-v1.19.0. All 6 override blocks survived (fork-PR skip, disabled observability, raised max-ai-credits and its env mirror, the two comment-only markers). Lock recompiled with gh-aw v0.85.4; the compile's merge=ours strip on .gitattributes was reverted. Full suite green locally (92 files, 1954 tests, review-pins.test.ts included).
no-jira: routine reviewer pin bump, no tracked lane owns this rollout