From d299b64cd2286dd61b9df72b1233bf290792ea0e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:19:22 +0000 Subject: [PATCH] Version Packages --- .changeset/autofix-docs-scope.md | 13 --------- .changeset/autofix.md | 15 ----------- .changeset/review-dispatch-roster-split.md | 7 ----- .changeset/review-documentation-lens.md | 15 ----------- .../review-thread-suppression-author.md | 5 ---- workflows/autofix/CHANGELOG.md | 27 +++++++++++++++++++ workflows/autofix/autofix.md | 4 +-- workflows/autofix/package.json | 2 +- workflows/review/CHANGELOG.md | 24 +++++++++++++++++ workflows/review/package.json | 2 +- workflows/review/review.md | 2 +- 11 files changed, 56 insertions(+), 60 deletions(-) delete mode 100644 .changeset/autofix-docs-scope.md delete mode 100644 .changeset/autofix.md delete mode 100644 .changeset/review-dispatch-roster-split.md delete mode 100644 .changeset/review-documentation-lens.md delete mode 100644 .changeset/review-thread-suppression-author.md create mode 100644 workflows/autofix/CHANGELOG.md diff --git a/.changeset/autofix-docs-scope.md b/.changeset/autofix-docs-scope.md deleted file mode 100644 index 4dfe6e9a..00000000 --- a/.changeset/autofix-docs-scope.md +++ /dev/null @@ -1,13 +0,0 @@ ---- -"autofix": minor ---- - -Add `docs` to the scope axis: `autofix: docs` / `/autofix docs` fixes only the `documentation` reviewer's threads, selected by the label that reviewer mints (`suggestion (non-blocking, documentation)`). - -It is a **subset of `nits`, not a peer of it**. Documentation findings are non-blocking, so `nits` already covers them and arming both is the same as arming `nits`; the containment runs one way only, and `docs` exists because arming `nits` to clear three stale comments also invites the fixer into every other cosmetic thread on the PR. The flat token namespace cannot express that, so `scope.ts` and the README both say it. `findingLabelsForScope` becomes an exhaustive switch rather than a blocking/non-blocking ternary, and the tests pin the containment and the absence of any blocking label from docs scope. - -`docs` is **not loop-eligible**, which is worth stating because it looks like the exception to "nits never loop" and is only half one: its deletion half converges (a comment that restates the code is either gone or not), while the documentation reviewer's *missing explanation* findings are answered with prose, and prose can always be wanted better. Ineligible until something measures which half dominates. - -One prompt rule comes with it: a documentation item changes text, never code. Deleting a redundant comment is the expected fix (and such findings often carry no suggestion block, since a deletion cannot be expressed as one), but if the honest fix would touch an executable line the item is left unfixed and reported rather than becoming a code change wearing a documentation label. That property — edits that cannot alter behavior — makes `docs` the safest scope to trial first in a repo new to autofix. - -Note the version coupling: autofix selects threads by parsing the label off each posted comment, so this scope finds threads only in repos whose **installed** `review` release mints the documentation label. Against an older reviewer it is not broken, just always empty. diff --git a/.changeset/autofix.md b/.changeset/autofix.md deleted file mode 100644 index 7f2b0478..00000000 --- a/.changeset/autofix.md +++ /dev/null @@ -1,15 +0,0 @@ ---- -"autofix": minor ---- - -Add the `autofix` workflow: opt-in, one-shot fixing of the PR reviewer's own feedback. - -Arm a PR with an `autofix: blocking` / `autofix: nits` label or an `/autofix [scope]` comment; the run fixes the reviewer's open threads in that scope, pushes one commit, replies in each thread, and removes the label. Both arming surfaces are peers resolving through one shared token vocabulary, and the trigger decides which is read, so a stale label cannot widen an explicit command. - -Everything except the code edit is deterministic. `lib/stage.ts` runs as a pre-agent step and fetches the inputs before the agent starts; `lib/plan.ts` then resolves scope, checks review currency, builds the work list, and renders the commit trailer. The plan is final: the prompt may execute it or stop, never widen it. - -Guards fail closed. Currency is checked per file so one unrelated push doesn't refuse the whole run; unparseable labels, outdated anchors, threads a human opened, an unreadable diff, and a head that moves mid-run are all excluded. Refusal is reserved for a PR with no review at all, and for a thread fetch that fails: GitHub reports GraphQL rate limits and node-access failures as HTTP 200 with an `errors` array, so staging treats any `errors` entry or an unparseable body as fatal rather than as "this PR has no threads", which would clear the arming label while the findings it was armed for stayed open. - -The reviewer's `skip-ai-review` label does not disarm autofix. It stops the reviewer's next run without withdrawing a review already posted, so a labelled PR can still carry current findings, and an explicit `autofix:` label or `/autofix` from someone with write access is the authorisation to act on them. A PR with no review is still refused, by the guard that checks for one. - -The push uses `KHAN_ACTIONS_BOT_TOKEN`, because GitHub creates no workflow runs for `GITHUB_TOKEN`-triggered events, and the re-review of the autofix commit is the intended verification a fix gets. That verification is best-effort rather than guaranteed: the chain from the push to a posted review has several links, whether a break is visible depends on how the consumer triggers its reviewer, and the human re-arming loop is the accepted backstop for v1. The run's summary comment says so on every push. Ships with a documented workaround for gh-aw's unbounded PR-branch fetch, which is otherwise fatal on large monorepos. diff --git a/.changeset/review-dispatch-roster-split.md b/.changeset/review-dispatch-roster-split.md deleted file mode 100644 index 7279a772..00000000 --- a/.changeset/review-dispatch-roster-split.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -"review": patch ---- - -Split the Step 3 roster concern out of `dispatch.ts` into `dispatch-roster.ts` (`DEFAULT_FINDERS`, `SHED_RANKING`, `Roster`, `RosterShed`, `computeRoster`), re-exported from `dispatch.ts` so callers and tests keep one import surface. No behaviour change: the moved code is byte-identical and every existing test passes unmodified. - -The trigger was a lint failure on main that neither contributing PR could see. `@khanacademy/eslint-config` caps a file at 1000 lines; #302 took `dispatch.ts` to exactly 1000, and the one line #299 added to the shed ranking took it to 1001. Both were green against their own bases, so the collision existed only in the merge. `dispatch.ts` is now 920 lines, and the split follows the precedent `dispatch-contracts.ts` already set for the same budget. diff --git a/.changeset/review-documentation-lens.md b/.changeset/review-documentation-lens.md deleted file mode 100644 index f1bcc341..00000000 --- a/.changeset/review-documentation-lens.md +++ /dev/null @@ -1,15 +0,0 @@ ---- -"review": minor ---- - -Add an opt-in `documentation` reviewer: an advisory-only whole-change reviewer that checks the comments and prose docs a diff adds or changes against a documentation policy. It flags a comment that restates the code, one that narrates the change rather than describing the code, one this diff falsified, commented-out code, and (the other direction) a specific unexplained constant, workaround, or ordering constraint the change introduces. The policy lives inline in the reviewer's definition rather than in a fifth required consumer import: the baseline is universal, and repo-specific calibration already has a home in the per-directory `REVIEW.md` contracts. It never reasons about whether a human or a model wrote the text (it cannot tell, and the policy is the same either way), and it does not review the PR title or description. Off everywhere until a repo adds `enable documentation` to its `ROUTING` file. - -Its findings render with a new non-blocking label, `suggestion (non-blocking, documentation)`, code-assigned from the lens exactly as the `, best-practice` variants are. The variant is not cosmetic: the autofix workflow selects threads by parsing the Conventional-Comment label off each posted comment, so this label is the only channel by which a documentation-scoped autofix can tell a documentation thread from any other nit. The scripted dispatcher now assigns the lens of a label-shape reviewer by name (`dispatch-contracts.ts`: previously `conventions` for `skill-auditor` and `correctness` for every other one), so a documentation finding carries `lens: documentation` downstream and `labelForFinding` agrees with the label the reviewer emitted. There is deliberately no blocking documentation variant, so `BLOCKING_LABELS` (what the blocking autofix scope acts on) is unchanged. `thumbs-sweep-github.ts` picks the new label up automatically; `verdict.ts` is unaffected, since the verdict counts blocking labels only. - -Two supporting changes. The `claim-validator` gains a documentation-claim rule: verify the quoted comment text and the code it is contrasted with, and refute whenever the comment carries information the code does not show, which is this reviewer's characteristic false positive. And the live eval producer now dispatches the opt-in reviewers a case `enable`s (`routerConfig.enabledReviewers`, canonical order, unknown names throw) and maps their label-shape output; before this, no opt-in reviewer had a live arm at all and so could not earn its `enable` line through the eval suite the way the policy says it must. Cases that enable nothing are unaffected. - -New eval corpus pair, both live-enabled: `golden-documentation-stale-and-narrated` (a sound behavior change that leaves a falsified comment and a change-narrating comment, and carries a must-not-flag trap whose comment reads like a restatement but records a real constraint) and `clean-documentation-earned-comments` (every added comment earns its line; the run must post nothing). - -Eval-harness fix that this PR's own corpus pair needs: an opt-in reviewer a case `enable`s but the arm's `review.md` does not define is now recorded as an absent dimension instead of throwing. The baseline arm of the A/B that graduates any new reviewer is built from the base tip and so cannot define it, and `runArm` does not wrap its produce call, so the throw killed the whole run before any report. The name is already validated against `ENABLEABLE_REVIEWERS`, so tolerating absence cannot mask a typo; every other roster member (the always-on finders, the validator, the reconciler, a routed lens) still hard-errors. The A/B report calls the asymmetry out under `Arm asymmetry`, since the candidate arm's findings on that dimension are gain by construction rather than a measured improvement. - -Three discriminating corpus cases, added after the first A/B measured a zero recall delta on the original pair: the baseline arm caught both seeded defects 6/6 without a documentation reviewer, because `correctness-reviewer` flags a stale comment and a change-narrating comment itself when the diff holds nothing else. Those two defects have a truth-value problem (a comment the change falsified reads as a bug risk), which is what made them visible to a correctness pass. The new cases seed defects with no truth-value problem at all, inside diffs carrying real logic work: a docstring that restates the signature (`golden-documentation-restated-docstring`), the one added constant whose neighbours explain themselves and it does not (`golden-documentation-missing-why`), and commented-out code as a matched pair, one call kept with a dated reason and a ticket, one with nothing (`golden-documentation-commented-out-code`). Each carries its must-not-flag traps in the same diff, and every diff is verified against `computeChangedLines` with every added line matching the fixture tree byte for byte. diff --git a/.changeset/review-thread-suppression-author.md b/.changeset/review-thread-suppression-author.md deleted file mode 100644 index 4faf8037..00000000 --- a/.changeset/review-thread-suppression-author.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"review": patch ---- - -Open-thread suppression was unreachable on every conforming run, so re-reviews re-posted findings an open bot thread already tracked. `openThreadsFromStaged` accepted an opener author of `github-actions[bot]` only, but `get_review_comments` — the tool review.md tells the orchestrator to copy `threads.json` from, verbatim — renders that same account as bare `github-actions`, and the filter fails closed, so a *correct* staging produced zero usable threads. It also read `path` from the thread while the tool carries `path` per comment, and `suppressOpenThreadDuplicates` matches on `path`, so a thread that cleared the author check still suppressed nothing. Both spellings are now accepted (`BOT_AUTHORS`, documented against the REST `user.login` surface that legitimately renders the bracketed form and that `stage-pr.ts` reads for prior reviews), and `path` falls back to the opening comment. Measured on webapp#41197's three-round seeded lifecycle, where suppression reported `threadSuppressions: []` in all three rounds while the re-reviews duplicated open threads: 6 of round 3's 8 comments landed on the exact path and line of a thread that was already open, and the reconciler's own `keep` list held those thread IDs in the same run, so the data suppression needed was present and unusable. Every unit fixture spelled the bot the way the code did, which is why the suite passed throughout; the regression cases now use the tool's real shape verbatim, assert an end-to-end suppression (with the blocking thread still flooring the verdict) rather than only the parse, and keep a human-opened thread refused so the widened author check cannot drop a finding outright. The prompt's selection layer is widened to match, which is the same premise one layer up: review.md told the orchestrator to stage "the unresolved `github-actions[bot]` threads" and to classify a human thread as "any author other than `github-actions[bot]`", both bracketed-only, so a literal reading could stage zero bot threads before the widened code filter ever ran — or worse, misfile a bot thread into `human-threads.json`, where it lands in `skipLines` and makes the submission drop a fresh finding on that line rather than merely duplicate one. Both instructions now name either spelling and say why. Second half of the fix, since a fail-open guard that cannot be seen failing is how this survived a whole release: `stagedThreadShapeFailure` reports a staging whose shape defeats the filter entirely, as a `threadSuppressionUnavailable` field on `dispatch-result.json` and a run-log warning, so "nothing to suppress" is no longer indistinguishable from "suppression silently did nothing." It counts staged threads per `thread_id` against the reconciler's resolved set rather than by list length, so a long `resolve` list cannot mask a total shape failure in a short staging, and it documents its own limit: one usable thread returns nothing, making it a total-failure tripwire rather than a per-thread audit. diff --git a/workflows/autofix/CHANGELOG.md b/workflows/autofix/CHANGELOG.md new file mode 100644 index 00000000..1bd4a31d --- /dev/null +++ b/workflows/autofix/CHANGELOG.md @@ -0,0 +1,27 @@ +# autofix + +## 0.1.0 + +### Minor Changes + +- 7d62a7a: Add `docs` to the scope axis: `autofix: docs` / `/autofix docs` fixes only the `documentation` reviewer's threads, selected by the label that reviewer mints (`suggestion (non-blocking, documentation)`). + + It is a **subset of `nits`, not a peer of it**. Documentation findings are non-blocking, so `nits` already covers them and arming both is the same as arming `nits`; the containment runs one way only, and `docs` exists because arming `nits` to clear three stale comments also invites the fixer into every other cosmetic thread on the PR. The flat token namespace cannot express that, so `scope.ts` and the README both say it. `findingLabelsForScope` becomes an exhaustive switch rather than a blocking/non-blocking ternary, and the tests pin the containment and the absence of any blocking label from docs scope. + + `docs` is **not loop-eligible**, which is worth stating because it looks like the exception to "nits never loop" and is only half one: its deletion half converges (a comment that restates the code is either gone or not), while the documentation reviewer's _missing explanation_ findings are answered with prose, and prose can always be wanted better. Ineligible until something measures which half dominates. + + One prompt rule comes with it: a documentation item changes text, never code. Deleting a redundant comment is the expected fix (and such findings often carry no suggestion block, since a deletion cannot be expressed as one), but if the honest fix would touch an executable line the item is left unfixed and reported rather than becoming a code change wearing a documentation label. That property — edits that cannot alter behavior — makes `docs` the safest scope to trial first in a repo new to autofix. + + Note the version coupling: autofix selects threads by parsing the label off each posted comment, so this scope finds threads only in repos whose **installed** `review` release mints the documentation label. Against an older reviewer it is not broken, just always empty. + +- 486bb92: Add the `autofix` workflow: opt-in, one-shot fixing of the PR reviewer's own feedback. + + Arm a PR with an `autofix: blocking` / `autofix: nits` label or an `/autofix [scope]` comment; the run fixes the reviewer's open threads in that scope, pushes one commit, replies in each thread, and removes the label. Both arming surfaces are peers resolving through one shared token vocabulary, and the trigger decides which is read, so a stale label cannot widen an explicit command. + + Everything except the code edit is deterministic. `lib/stage.ts` runs as a pre-agent step and fetches the inputs before the agent starts; `lib/plan.ts` then resolves scope, checks review currency, builds the work list, and renders the commit trailer. The plan is final: the prompt may execute it or stop, never widen it. + + Guards fail closed. Currency is checked per file so one unrelated push doesn't refuse the whole run; unparseable labels, outdated anchors, threads a human opened, an unreadable diff, and a head that moves mid-run are all excluded. Refusal is reserved for a PR with no review at all, and for a thread fetch that fails: GitHub reports GraphQL rate limits and node-access failures as HTTP 200 with an `errors` array, so staging treats any `errors` entry or an unparseable body as fatal rather than as "this PR has no threads", which would clear the arming label while the findings it was armed for stayed open. + + The reviewer's `skip-ai-review` label does not disarm autofix. It stops the reviewer's next run without withdrawing a review already posted, so a labelled PR can still carry current findings, and an explicit `autofix:` label or `/autofix` from someone with write access is the authorisation to act on them. A PR with no review is still refused, by the guard that checks for one. + + The push uses `KHAN_ACTIONS_BOT_TOKEN`, because GitHub creates no workflow runs for `GITHUB_TOKEN`-triggered events, and the re-review of the autofix commit is the intended verification a fix gets. That verification is best-effort rather than guaranteed: the chain from the push to a posted review has several links, whether a break is visible depends on how the consumer triggers its reviewer, and the human re-arming loop is the accepted backstop for v1. The run's summary comment says so on every push. Ships with a documented workaround for gh-aw's unbounded PR-branch fetch, which is otherwise fatal on large monorepos. diff --git a/workflows/autofix/autofix.md b/workflows/autofix/autofix.md index 0f435c13..8e675a0b 100644 --- a/workflows/autofix/autofix.md +++ b/workflows/autofix/autofix.md @@ -242,7 +242,7 @@ pre-agent-steps: uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 with: repository: Khan/actions - ref: autofix-v0.0.0 + ref: autofix-v0.1.0 path: gh-aw-autofix-lib persist-credentials: false @@ -336,7 +336,7 @@ env: GIT_CONFIG_KEY_1: remote.origin.partialclonefilter GIT_CONFIG_VALUE_1: "blob:none" -source: Khan/actions/workflows/autofix/autofix.md@autofix-v0.0.0 +source: Khan/actions/workflows/autofix/autofix.md@autofix-v0.1.0 --- # PR Autofixer diff --git a/workflows/autofix/package.json b/workflows/autofix/package.json index a1368133..804e3aef 100644 --- a/workflows/autofix/package.json +++ b/workflows/autofix/package.json @@ -1,4 +1,4 @@ { "name": "autofix", - "version": "0.0.0" + "version": "0.1.0" } diff --git a/workflows/review/CHANGELOG.md b/workflows/review/CHANGELOG.md index 2159ba02..b79264b0 100644 --- a/workflows/review/CHANGELOG.md +++ b/workflows/review/CHANGELOG.md @@ -1,5 +1,29 @@ # review +## 1.9.0 + +### Minor Changes + +- 6532c6b: Add an opt-in `documentation` reviewer: an advisory-only whole-change reviewer that checks the comments and prose docs a diff adds or changes against a documentation policy. It flags a comment that restates the code, one that narrates the change rather than describing the code, one this diff falsified, commented-out code, and (the other direction) a specific unexplained constant, workaround, or ordering constraint the change introduces. The policy lives inline in the reviewer's definition rather than in a fifth required consumer import: the baseline is universal, and repo-specific calibration already has a home in the per-directory `REVIEW.md` contracts. It never reasons about whether a human or a model wrote the text (it cannot tell, and the policy is the same either way), and it does not review the PR title or description. Off everywhere until a repo adds `enable documentation` to its `ROUTING` file. + + Its findings render with a new non-blocking label, `suggestion (non-blocking, documentation)`, code-assigned from the lens exactly as the `, best-practice` variants are. The variant is not cosmetic: the autofix workflow selects threads by parsing the Conventional-Comment label off each posted comment, so this label is the only channel by which a documentation-scoped autofix can tell a documentation thread from any other nit. The scripted dispatcher now assigns the lens of a label-shape reviewer by name (`dispatch-contracts.ts`: previously `conventions` for `skill-auditor` and `correctness` for every other one), so a documentation finding carries `lens: documentation` downstream and `labelForFinding` agrees with the label the reviewer emitted. There is deliberately no blocking documentation variant, so `BLOCKING_LABELS` (what the blocking autofix scope acts on) is unchanged. `thumbs-sweep-github.ts` picks the new label up automatically; `verdict.ts` is unaffected, since the verdict counts blocking labels only. + + Two supporting changes. The `claim-validator` gains a documentation-claim rule: verify the quoted comment text and the code it is contrasted with, and refute whenever the comment carries information the code does not show, which is this reviewer's characteristic false positive. And the live eval producer now dispatches the opt-in reviewers a case `enable`s (`routerConfig.enabledReviewers`, canonical order, unknown names throw) and maps their label-shape output; before this, no opt-in reviewer had a live arm at all and so could not earn its `enable` line through the eval suite the way the policy says it must. Cases that enable nothing are unaffected. + + New eval corpus pair, both live-enabled: `golden-documentation-stale-and-narrated` (a sound behavior change that leaves a falsified comment and a change-narrating comment, and carries a must-not-flag trap whose comment reads like a restatement but records a real constraint) and `clean-documentation-earned-comments` (every added comment earns its line; the run must post nothing). + + Eval-harness fix that this PR's own corpus pair needs: an opt-in reviewer a case `enable`s but the arm's `review.md` does not define is now recorded as an absent dimension instead of throwing. The baseline arm of the A/B that graduates any new reviewer is built from the base tip and so cannot define it, and `runArm` does not wrap its produce call, so the throw killed the whole run before any report. The name is already validated against `ENABLEABLE_REVIEWERS`, so tolerating absence cannot mask a typo; every other roster member (the always-on finders, the validator, the reconciler, a routed lens) still hard-errors. The A/B report calls the asymmetry out under `Arm asymmetry`, since the candidate arm's findings on that dimension are gain by construction rather than a measured improvement. + + Three discriminating corpus cases, added after the first A/B measured a zero recall delta on the original pair: the baseline arm caught both seeded defects 6/6 without a documentation reviewer, because `correctness-reviewer` flags a stale comment and a change-narrating comment itself when the diff holds nothing else. Those two defects have a truth-value problem (a comment the change falsified reads as a bug risk), which is what made them visible to a correctness pass. The new cases seed defects with no truth-value problem at all, inside diffs carrying real logic work: a docstring that restates the signature (`golden-documentation-restated-docstring`), the one added constant whose neighbours explain themselves and it does not (`golden-documentation-missing-why`), and commented-out code as a matched pair, one call kept with a dated reason and a ticket, one with nothing (`golden-documentation-commented-out-code`). Each carries its must-not-flag traps in the same diff, and every diff is verified against `computeChangedLines` with every added line matching the fixture tree byte for byte. + +### Patch Changes + +- 5943268: Split the Step 3 roster concern out of `dispatch.ts` into `dispatch-roster.ts` (`DEFAULT_FINDERS`, `SHED_RANKING`, `Roster`, `RosterShed`, `computeRoster`), re-exported from `dispatch.ts` so callers and tests keep one import surface. No behaviour change: the moved code is byte-identical and every existing test passes unmodified. + + The trigger was a lint failure on main that neither contributing PR could see. `@khanacademy/eslint-config` caps a file at 1000 lines; #302 took `dispatch.ts` to exactly 1000, and the one line #299 added to the shed ranking took it to 1001. Both were green against their own bases, so the collision existed only in the merge. `dispatch.ts` is now 920 lines, and the split follows the precedent `dispatch-contracts.ts` already set for the same budget. + +- 157449c: Open-thread suppression was unreachable on every conforming run, so re-reviews re-posted findings an open bot thread already tracked. `openThreadsFromStaged` accepted an opener author of `github-actions[bot]` only, but `get_review_comments` — the tool review.md tells the orchestrator to copy `threads.json` from, verbatim — renders that same account as bare `github-actions`, and the filter fails closed, so a _correct_ staging produced zero usable threads. It also read `path` from the thread while the tool carries `path` per comment, and `suppressOpenThreadDuplicates` matches on `path`, so a thread that cleared the author check still suppressed nothing. Both spellings are now accepted (`BOT_AUTHORS`, documented against the REST `user.login` surface that legitimately renders the bracketed form and that `stage-pr.ts` reads for prior reviews), and `path` falls back to the opening comment. Measured on webapp#41197's three-round seeded lifecycle, where suppression reported `threadSuppressions: []` in all three rounds while the re-reviews duplicated open threads: 6 of round 3's 8 comments landed on the exact path and line of a thread that was already open, and the reconciler's own `keep` list held those thread IDs in the same run, so the data suppression needed was present and unusable. Every unit fixture spelled the bot the way the code did, which is why the suite passed throughout; the regression cases now use the tool's real shape verbatim, assert an end-to-end suppression (with the blocking thread still flooring the verdict) rather than only the parse, and keep a human-opened thread refused so the widened author check cannot drop a finding outright. The prompt's selection layer is widened to match, which is the same premise one layer up: review.md told the orchestrator to stage "the unresolved `github-actions[bot]` threads" and to classify a human thread as "any author other than `github-actions[bot]`", both bracketed-only, so a literal reading could stage zero bot threads before the widened code filter ever ran — or worse, misfile a bot thread into `human-threads.json`, where it lands in `skipLines` and makes the submission drop a fresh finding on that line rather than merely duplicate one. Both instructions now name either spelling and say why. Second half of the fix, since a fail-open guard that cannot be seen failing is how this survived a whole release: `stagedThreadShapeFailure` reports a staging whose shape defeats the filter entirely, as a `threadSuppressionUnavailable` field on `dispatch-result.json` and a run-log warning, so "nothing to suppress" is no longer indistinguishable from "suppression silently did nothing." It counts staged threads per `thread_id` against the reconciler's resolved set rather than by list length, so a long `resolve` list cannot mask a total shape failure in a short staging, and it documents its own limit: one usable thread returns nothing, making it a total-failure tripwire rather than a per-thread audit. + ## 1.8.0 ### Minor Changes diff --git a/workflows/review/package.json b/workflows/review/package.json index dcc11878..deffbe41 100644 --- a/workflows/review/package.json +++ b/workflows/review/package.json @@ -1,6 +1,6 @@ { "name": "review", - "version": "1.8.0", + "version": "1.9.0", "dependencies": { "octokit": "5.0.5", "@anthropic-ai/claude-agent-sdk": "0.3.205", diff --git a/workflows/review/review.md b/workflows/review/review.md index f76c200f..fde4de51 100644 --- a/workflows/review/review.md +++ b/workflows/review/review.md @@ -227,7 +227,7 @@ pre-agent-steps: uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 with: repository: Khan/actions - ref: review-v1.8.0 + ref: review-v1.9.0 path: gh-aw-review-lib persist-credentials: false