diff --git a/.changeset/checker-repo-path-guard.md b/.changeset/checker-repo-path-guard.md deleted file mode 100644 index 53a14c0f..00000000 --- a/.changeset/checker-repo-path-guard.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"review": patch ---- - -check-consumer-config: fail loudly when `--repo` names a path that does not exist. The flag takes a path to the consumer checkout, but an `owner/name` argument parses fine and resolves as a relative path, so every check reported missing and the report read as a catastrophically broken install instead of a typo (hit live during the 2026-08-20 rollout; the review-consumer-bump skill documents the footgun). The checker now throws naming the bad path. Also deduplicates parseArgs's twice-declared inline arg type into one `CliArgs` alias to stay under the max-lines cap the file already sits at. diff --git a/.changeset/gate-absorb-entity-decode.md b/.changeset/gate-absorb-entity-decode.md deleted file mode 100644 index e900bccd..00000000 --- a/.changeset/gate-absorb-entity-decode.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"review": patch ---- - -The dispatch gate's rule 7 no longer false-blocks a review whose plan quotes angle brackets. Run 32758584548 (Khan/actions#371) staged a footer quoting `<STOP: ...>` (the renderer escapes angle brackets it quotes), gh-aw's ingest sanitizer decoded the entities and convertXmlTags parenthesised the result on the queued side, and the plan-vs-queue comparison went red on a fully conforming run, withholding the whole review. HTML entity decoding was a documented-not-absorbed residual of `sanitizer-normalize.ts`; it is now absorbed the way the `` placeholder shape was: `normalizeBody` mirrors the sanitizer's decodeHtmlEntities (named, decimal, hex, one level of `&` double-encoding, gh-aw v0.85.4) on both comparison sides, sequenced before the invisible-character strips exactly as hardenUnicodeText sequences it, so an entity-vs-literal splice that changes text still fails. diff --git a/.changeset/pra11-adjudicated-crossfile.md b/.changeset/pra11-adjudicated-crossfile.md deleted file mode 100644 index af502550..00000000 --- a/.changeset/pra11-adjudicated-crossfile.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"review": minor ---- - -The adjudicated-corpus suppression drops its same-path key: a human-settled defect's rephrasing routinely re-anchors on another file (the spec instead of the implementation, the test instead of the function), and the path key is what let the webapp#41290 duplicate families re-post for two weeks after the author had adjudicated them. Measured on that frozen corpus (12 adjudicated threads, 33 labeled candidates, kept privately in the planning tree), dropping the key tripled recall (2/12 to 6/12 true variants suppressed) and added zero false suppressions (both variants make the same single mistake, folding two distinct same-file findings whose wording shares the file's vocabulary). The open-thread corpus stays path-keyed, blocking candidates are still never suppressed here, an adjudicated thread staged without a usable path stays inert rather than becoming a PR-wide matcher, and every other #332 fail-closed guard is unchanged. diff --git a/.changeset/pra47-acknowledged-threads.md b/.changeset/pra47-acknowledged-threads.md deleted file mode 100644 index f40ec20f..00000000 --- a/.changeset/pra47-acknowledged-threads.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"review": minor ---- - -An author acknowledgment stops reading as "unaddressed": the thread-reconciler now reports kept threads whose reply chain shows the author conceded the finding (will fix, TODO stands in) as `acknowledged`, code verifies the mechanical preconditions for each id against the staged reply chain (keep membership required, the PR author must actually have replied, bot replies never count, no staged author verifies nothing; whether the reply concedes rather than pushes back stays the reconciler's judgment), and the re-review recap counts those threads as "acknowledged (fix pending)" instead of unaddressed. An acknowledged blocking thread still renders visibly and still counts toward `keptBlockingCount`, so the flip gate is unchanged. The verified ids are recorded in `rereview.json`; nothing consumes them yet. diff --git a/.changeset/readme-no-gh-aw-update.md b/.changeset/readme-no-gh-aw-update.md deleted file mode 100644 index dfb9333c..00000000 --- a/.changeset/readme-no-gh-aw-update.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"review": patch ---- - -Docs only, following up on the `gh aw update` ban Khan/actions#357 landed. The consumer-facing README now names the gh-aw version both failures were observed on (v0.85.4) and the condition for revisiting the ban: neither failure is filed upstream yet, so re-test both on a scratch install before trusting a newer gh-aw release. The consumer-config checker's `source-missing` warning no longer frames `gh aw update` as the update mechanism; it now cites the reason `source:` still matters (the manual bump flow reads it to tell which release the install was copied from). The onboarding skill's update block gains an explicit stop comment between the merge instructions and `gh aw compile`, telling the reader to run the 3-way merge and commit it before compiling (advisory: a shell comment cannot stop a straight paste, but the prior `` pseudo-tag was not valid shell at all). This repo's own installed `review.md` changes only in a frontmatter comment (the observability local-edit note no longer names `gh aw update` as the merger; lock recompiled); no behavior change. diff --git a/.changeset/retire-thumbs-followup.md b/.changeset/retire-thumbs-followup.md deleted file mode 100644 index a81fc426..00000000 --- a/.changeset/retire-thumbs-followup.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"review": minor ---- - -Delete the thumbs sweep entirely (`thumbs-sweep.ts`, `thumbs-sweep-github.ts`, `run-thumbs-sweep.ts`, and their tests), along with its `octokit` dependency; the lib scripts consumers run are dependency-free again. The 2026-08-20 version audit measured 2 reason replies across the 31 "why?" follow-ups ever posted, 26 of them landing as bursts on one PR, each follow-up also registered as an implicit empty COMMENTED review event that pollutes run counts and the PR timeline, and nobody consumed the read-side tallies. Nothing depends on the sweep: a bare thumbs-down has adjudicated the finding directly since v1.17.0 (staging reads thread-opener reactions itself, identity-filtered), and feedback reporting is done manually today via the `review-feedback-audit` skill (the reconciler and claim validation still read thread replies on re-review; no automated job turns them into feedback reports, the weekly feedback report is the planned instrument). Consumer repos should delete their `review-feedback.yml` sweep workflow when bumping; the `review-counters.yml` weekly counters workflow is unaffected. Anyone counting review runs should key on the v1.14.0+ version footer rather than review events: the autofix workflow's thread replies still carry the implicit-review shape. diff --git a/.changeset/review-consumer-bump-skill.md b/.changeset/review-consumer-bump-skill.md deleted file mode 100644 index a86df9c5..00000000 --- a/.changeset/review-consumer-bump-skill.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"review": patch ---- - -Skill-doc only: add the `review-consumer-bump` skill, the playbook for rolling a `review-v` release out to every consuming repo. Consumers are discovered by org-wide code search on the installed `source:` line, never from a remembered list. The core procedure is a manual `git merge-file` 3-way merge (base: the consumer's current pin, theirs: the target tag, ours: the installed copy) because `gh aw update` fails twice against this repo's tag scheme, both observed live on 2026-08-20: it treats `review-v` as a branch and repins to main's head SHA, and its own merge emptied a consumer's review.md to 0 bytes. The skill also records the compile side effects to keep or revert (setup-action pin moves, the stripped `merge=ours` gitattribute, gh-aw v0.85.x deleting its maintenance workflow), the two verification gates (the pricing overlay's `providers` in both awf-config payloads; the consumer-config checker run from a version-matched tag checkout with a path, not a name, as `--repo`), and the Khan/actions-specific rule that review-pins.test.ts requires every divergent hunk to carry a LOCAL OVERRIDE marker. Also corrects the sibling `review-onboarding` skill, the consumer-facing `workflows/review/README.md`, and one frontmatter comment in `workflows/review/review.md`, all of which still prescribed `gh aw update` or claimed it preserves local edits. Reference bumps: kore-marketplace#11, agent-settings#76, webapp#41661, Khan/actions#356. No behavior change to the shipped review workflow; the review.md edit is comment-only. diff --git a/.changeset/review-dispatcher-death-comment.md b/.changeset/review-dispatcher-death-comment.md deleted file mode 100644 index 5c2bfd16..00000000 --- a/.changeset/review-dispatcher-death-comment.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -"review": minor ---- - -Make a dispatcher death visible on the PR. When the dispatcher's Bash call dies without writing `dispatch-result.json` (killed at the engine ceiling, or crashed), the orchestrator now posts one standalone PR comment saying the review died mid-dispatch and posted no review, with a link to the run, instead of only writing an incomplete report. Run 32418662895 (Khan/actions#362) hit exactly this: the run stayed green, the incomplete report tried to file an issue on a repo with issues disabled, and the PR showed nothing. - -The death comment collapses the standing risks/patterns guidance comment (`hide-older-comments`), exactly like a hold comment does, so the Step 9 cache CLI now recognizes the no-plan death shape (a queued `add_comment` with no submission plan staged) and drops `risksPatternsKey` from the prior record; the next run reposts the guidance, and fingerprints stand untouched so it reviews in full. diff --git a/.changeset/review-subject-restatement-and-body-stats.md b/.changeset/review-subject-restatement-and-body-stats.md deleted file mode 100644 index 79bd3acd..00000000 --- a/.changeset/review-subject-restatement-and-body-stats.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -"review": minor ---- - -Stop posting a subject line that restates the discussion, and stage body-size stats with every submission plan. - -The 2026-08-20 by-version audit of webapp reviews found a prose repetition cluster in the v1.11.0-v1.13.0 windows (5 of 29 blind-judged bodies restated one fact two to four times, vs 1 of 60 before), and the v1.8.0 task-mode removal (#284/#288/#289) deleted the orchestrator rewrite pass that used to absorb subject/discussion overlap; `joinProse` concatenates the label contract's `subject` and `discussion` verbatim with no overlap check. This fixes the mechanical subset (a subject that token-for-token restates the discussion's lede); the audited fail bodies are mostly paraphrase-level restatement, which stays producer-side (finding-contract wording, tracked in PRA-46). `joinProse` now drops a subject whose folded tokens are all contained in the discussion's first sentence (stopwords ignored on the subject side, light inflection folding on both sides, markdown and trailing punctuation stripped); a subject restating a later sentence or carrying any token the first sentence lacks is kept whole. `buildClaims`' first-sentence split then recovers the discussion's own opening claim as the subject, so no downstream field goes empty, and the `failure_scenario` salvage for a dropped subject reads the discussion instead, keeping dedup's `comparedText` on its discussion branch. Expected output-shape effect: median inline comment bodies shrink by up to one sentence each (the drop removes exactly the duplicated subject line and adds nothing); review bodies move only where a PR-level claim folds in (`renderPrLevelFold` renders the same joined prose, and a dropped subject changes the one-line header the HOLD and over-cap collapsed lists print), shrinking by the same one duplicated sentence. - -The same audit found the +60% median-body step (557 to 889 chars across webapp's v1.7.0 to v1.11.0 bump) shipped with no changeset naming it, and a by-version audit four versions later was the detection mechanism. `submission-plan.json` now carries `bodyStats` (comment count, median/p90/max/total rendered chars over the final comment bodies, footer rides included, plus the review body's length), echoed in the CLI's run-log summary, so the next render-path regression is visible in the first runs' artifacts; a copy of the plan is staged under `out/`, the one directory the run uploads. Artifact-only; nothing gates on it. The README's versioning section now requires render-path changesets to state their expected output-shape effect. diff --git a/workflows/review/CHANGELOG.md b/workflows/review/CHANGELOG.md index 87e2689e..7c871d05 100644 --- a/workflows/review/CHANGELOG.md +++ b/workflows/review/CHANGELOG.md @@ -1,5 +1,29 @@ # review +## 1.19.0 + +### Minor Changes + +- 1326331: The adjudicated-corpus suppression drops its same-path key: a human-settled defect's rephrasing routinely re-anchors on another file (the spec instead of the implementation, the test instead of the function), and the path key is what let the webapp#41290 duplicate families re-post for two weeks after the author had adjudicated them. Measured on that frozen corpus (12 adjudicated threads, 33 labeled candidates, kept privately in the planning tree), dropping the key tripled recall (2/12 to 6/12 true variants suppressed) and added zero false suppressions (both variants make the same single mistake, folding two distinct same-file findings whose wording shares the file's vocabulary). The open-thread corpus stays path-keyed, blocking candidates are still never suppressed here, an adjudicated thread staged without a usable path stays inert rather than becoming a PR-wide matcher, and every other #332 fail-closed guard is unchanged. +- d9efa85: An author acknowledgment stops reading as "unaddressed": the thread-reconciler now reports kept threads whose reply chain shows the author conceded the finding (will fix, TODO stands in) as `acknowledged`, code verifies the mechanical preconditions for each id against the staged reply chain (keep membership required, the PR author must actually have replied, bot replies never count, no staged author verifies nothing; whether the reply concedes rather than pushes back stays the reconciler's judgment), and the re-review recap counts those threads as "acknowledged (fix pending)" instead of unaddressed. An acknowledged blocking thread still renders visibly and still counts toward `keptBlockingCount`, so the flip gate is unchanged. The verified ids are recorded in `rereview.json`; nothing consumes them yet. +- 298f4cb: Delete the thumbs sweep entirely (`thumbs-sweep.ts`, `thumbs-sweep-github.ts`, `run-thumbs-sweep.ts`, and their tests), along with its `octokit` dependency; the lib scripts consumers run are dependency-free again. The 2026-08-20 version audit measured 2 reason replies across the 31 "why?" follow-ups ever posted, 26 of them landing as bursts on one PR, each follow-up also registered as an implicit empty COMMENTED review event that pollutes run counts and the PR timeline, and nobody consumed the read-side tallies. Nothing depends on the sweep: a bare thumbs-down has adjudicated the finding directly since v1.17.0 (staging reads thread-opener reactions itself, identity-filtered), and feedback reporting is done manually today via the `review-feedback-audit` skill (the reconciler and claim validation still read thread replies on re-review; no automated job turns them into feedback reports, the weekly feedback report is the planned instrument). Consumer repos should delete their `review-feedback.yml` sweep workflow when bumping; the `review-counters.yml` weekly counters workflow is unaffected. Anyone counting review runs should key on the v1.14.0+ version footer rather than review events: the autofix workflow's thread replies still carry the implicit-review shape. +- 1d66f91: Make a dispatcher death visible on the PR. When the dispatcher's Bash call dies without writing `dispatch-result.json` (killed at the engine ceiling, or crashed), the orchestrator now posts one standalone PR comment saying the review died mid-dispatch and posted no review, with a link to the run, instead of only writing an incomplete report. Run 32418662895 (Khan/actions#362) hit exactly this: the run stayed green, the incomplete report tried to file an issue on a repo with issues disabled, and the PR showed nothing. + + The death comment collapses the standing risks/patterns guidance comment (`hide-older-comments`), exactly like a hold comment does, so the Step 9 cache CLI now recognizes the no-plan death shape (a queued `add_comment` with no submission plan staged) and drops `risksPatternsKey` from the prior record; the next run reposts the guidance, and fingerprints stand untouched so it reviews in full. + +- 5dcfdcc: Stop posting a subject line that restates the discussion, and stage body-size stats with every submission plan. + + The 2026-08-20 by-version audit of webapp reviews found a prose repetition cluster in the v1.11.0-v1.13.0 windows (5 of 29 blind-judged bodies restated one fact two to four times, vs 1 of 60 before), and the v1.8.0 task-mode removal (#284/#288/#289) deleted the orchestrator rewrite pass that used to absorb subject/discussion overlap; `joinProse` concatenates the label contract's `subject` and `discussion` verbatim with no overlap check. This fixes the mechanical subset (a subject that token-for-token restates the discussion's lede); the audited fail bodies are mostly paraphrase-level restatement, which stays producer-side (finding-contract wording, tracked in PRA-46). `joinProse` now drops a subject whose folded tokens are all contained in the discussion's first sentence (stopwords ignored on the subject side, light inflection folding on both sides, markdown and trailing punctuation stripped); a subject restating a later sentence or carrying any token the first sentence lacks is kept whole. `buildClaims`' first-sentence split then recovers the discussion's own opening claim as the subject, so no downstream field goes empty, and the `failure_scenario` salvage for a dropped subject reads the discussion instead, keeping dedup's `comparedText` on its discussion branch. Expected output-shape effect: median inline comment bodies shrink by up to one sentence each (the drop removes exactly the duplicated subject line and adds nothing); review bodies move only where a PR-level claim folds in (`renderPrLevelFold` renders the same joined prose, and a dropped subject changes the one-line header the HOLD and over-cap collapsed lists print), shrinking by the same one duplicated sentence. + + The same audit found the +60% median-body step (557 to 889 chars across webapp's v1.7.0 to v1.11.0 bump) shipped with no changeset naming it, and a by-version audit four versions later was the detection mechanism. `submission-plan.json` now carries `bodyStats` (comment count, median/p90/max/total rendered chars over the final comment bodies, footer rides included, plus the review body's length), echoed in the CLI's run-log summary, so the next render-path regression is visible in the first runs' artifacts; a copy of the plan is staged under `out/`, the one directory the run uploads. Artifact-only; nothing gates on it. The README's versioning section now requires render-path changesets to state their expected output-shape effect. + +### Patch Changes + +- ccf3504: check-consumer-config: fail loudly when `--repo` names a path that does not exist. The flag takes a path to the consumer checkout, but an `owner/name` argument parses fine and resolves as a relative path, so every check reported missing and the report read as a catastrophically broken install instead of a typo (hit live during the 2026-08-20 rollout; the review-consumer-bump skill documents the footgun). The checker now throws naming the bad path. Also deduplicates parseArgs's twice-declared inline arg type into one `CliArgs` alias to stay under the max-lines cap the file already sits at. +- cc3589d: The dispatch gate's rule 7 no longer false-blocks a review whose plan quotes angle brackets. Run 32758584548 (Khan/actions#371) staged a footer quoting `<STOP: ...>` (the renderer escapes angle brackets it quotes), gh-aw's ingest sanitizer decoded the entities and convertXmlTags parenthesised the result on the queued side, and the plan-vs-queue comparison went red on a fully conforming run, withholding the whole review. HTML entity decoding was a documented-not-absorbed residual of `sanitizer-normalize.ts`; it is now absorbed the way the `` placeholder shape was: `normalizeBody` mirrors the sanitizer's decodeHtmlEntities (named, decimal, hex, one level of `&` double-encoding, gh-aw v0.85.4) on both comparison sides, sequenced before the invisible-character strips exactly as hardenUnicodeText sequences it, so an entity-vs-literal splice that changes text still fails. +- 1dde4d3: Docs only, following up on the `gh aw update` ban Khan/actions#357 landed. The consumer-facing README now names the gh-aw version both failures were observed on (v0.85.4) and the condition for revisiting the ban: neither failure is filed upstream yet, so re-test both on a scratch install before trusting a newer gh-aw release. The consumer-config checker's `source-missing` warning no longer frames `gh aw update` as the update mechanism; it now cites the reason `source:` still matters (the manual bump flow reads it to tell which release the install was copied from). The onboarding skill's update block gains an explicit stop comment between the merge instructions and `gh aw compile`, telling the reader to run the 3-way merge and commit it before compiling (advisory: a shell comment cannot stop a straight paste, but the prior `` pseudo-tag was not valid shell at all). This repo's own installed `review.md` changes only in a frontmatter comment (the observability local-edit note no longer names `gh aw update` as the merger; lock recompiled); no behavior change. +- d66766d: Skill-doc only: add the `review-consumer-bump` skill, the playbook for rolling a `review-v` release out to every consuming repo. Consumers are discovered by org-wide code search on the installed `source:` line, never from a remembered list. The core procedure is a manual `git merge-file` 3-way merge (base: the consumer's current pin, theirs: the target tag, ours: the installed copy) because `gh aw update` fails twice against this repo's tag scheme, both observed live on 2026-08-20: it treats `review-v` as a branch and repins to main's head SHA, and its own merge emptied a consumer's review.md to 0 bytes. The skill also records the compile side effects to keep or revert (setup-action pin moves, the stripped `merge=ours` gitattribute, gh-aw v0.85.x deleting its maintenance workflow), the two verification gates (the pricing overlay's `providers` in both awf-config payloads; the consumer-config checker run from a version-matched tag checkout with a path, not a name, as `--repo`), and the Khan/actions-specific rule that review-pins.test.ts requires every divergent hunk to carry a LOCAL OVERRIDE marker. Also corrects the sibling `review-onboarding` skill, the consumer-facing `workflows/review/README.md`, and one frontmatter comment in `workflows/review/review.md`, all of which still prescribed `gh aw update` or claimed it preserves local edits. Reference bumps: kore-marketplace#11, agent-settings#76, webapp#41661, Khan/actions#356. No behavior change to the shipped review workflow; the review.md edit is comment-only. + ## 1.18.0 ### Minor Changes diff --git a/workflows/review/package.json b/workflows/review/package.json index e28c14b4..43544ba8 100644 --- a/workflows/review/package.json +++ b/workflows/review/package.json @@ -1,6 +1,6 @@ { "name": "review", - "version": "1.18.0", + "version": "1.19.0", "dependencies": { "@anthropic-ai/claude-agent-sdk": "0.3.205", "zod": "4.4.3" diff --git a/workflows/review/review.md b/workflows/review/review.md index c3db46f9..87fb7f27 100644 --- a/workflows/review/review.md +++ b/workflows/review/review.md @@ -238,7 +238,7 @@ pre-agent-steps: uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 with: repository: Khan/actions - ref: review-v1.18.0 + ref: review-v1.19.0 path: gh-aw-review-lib persist-credentials: false @@ -1058,7 +1058,7 @@ fully explained by a common pattern above:
review details -review-v1.18.0 | schema 2 | depth full | re-review scoped blocking-only | enable holistic,completeness +review-v1.19.0 | schema 2 | depth full | re-review scoped blocking-only | enable holistic,completeness
````