review: stop recommending gh aw update in the consumer README #371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

jwbron merged 10 commits into main from jwbron/review-readme-manual-merge

Aug 24, 2026

.changeset/readme-no-gh-aw-update.md

-Original file line number
+Diff line change
@@ -0,0 +1,5 @@
+    ---
+    "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 `<STOP: ...>` 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.

.claude/skills/review-consumer-bump/SKILL.md

-Original file line number
+Diff line change
@@ Expand Up @@
     The manual merge below is what the tool would do if it worked.
+    Both failures were observed on gh-aw v0.85.4 and neither is filed upstream,
+    so this ban carries no expiry: before trusting a newer gh-aw release with a
+    bump, reproduce both failures on a scratch install first.
     ## Step 2: the merge
     Work in a fresh clone of the consumer, on a new branch. From a `Khan/actions`
@@ Expand Down @@

.claude/skills/review-onboarding/SKILL.md

-Original file line number
+Diff line change
@@ Expand Up / @@ -428,6 +428,7 @@ the diff shows what a version bump changed. @@
     cd <consumer-repo> && git switch -c bump-shared-pr-reviewer
     # 3-way merge per .claude/skills/review-consumer-bump/SKILL.md -- NOT `gh aw update`,
     # which repins review-v* tags to main's head SHA and once emptied review.md to 0 bytes
+    # STOP HERE: run the 3-way merge from that skill and commit it, then return
     gh aw compile                    # --approve only after reviewing any new secret
     ```
@@ Expand Down @@

.github/workflows/review.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

.github/workflows/review.md

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -165,7 +165,7 @@ network:
  
    # Both secrets are hard-required while this block is present: a missing one compiles to

    # an empty value that the MCP gateway's OTLP config schema rejects, so the agent job

    # dies at startup instead of skipping trace export. A repo without them must comment

    # this block out in its installed review.md (a local edit `gh aw update` preserves)

    # this block out in its installed review.md (a local edit the 3-way merge update flow preserves)

    # and recompile.

    #

    # KHAN/ACTIONS LOCAL OVERRIDE: the `observability:` block is disabled here because this

workflows/review/README.md

-Original file line number
+Diff line change
@@ Expand Up @@
     flow in [`review-consumer-bump`](../../.claude/skills/review-consumer-bump/SKILL.md),
     **not** `gh aw update`: the tool does not recognize the `review-v<version>` tag
     scheme, repins to main's head SHA instead, and has emptied an installed
-    `review.md` to 0 bytes.
+    `review.md` to 0 bytes (both observed on gh-aw v0.85.4; neither failure is
+    filed upstream yet, so re-test both on a scratch install before trusting a
+    newer gh-aw release with this).
     The tag is self-consistent: the `review.md` inside each `review-v<version>` tag
     pins its own `pre-agent-steps` checkout `ref:` to that same version (the release
@@ Expand Down @@

workflows/review/gh-aw-update-ban.test.ts

-Original file line number
+Diff line change
@@ -0,0 +1,81 @@
+    /**
+     * CI backstop for the `gh aw update` ban (the tool repins `review-v*` tags to
+     * main's head SHA and once emptied an installed `review.md` to 0 bytes; see
+     * `.claude/skills/review-consumer-bump/SKILL.md`). The ban lives entirely in
+     * prose, which no automation touches, so nothing but this test stops a future
+     * doc edit from recommending the tool again. Same shape as the repo's other
+     * untouched-by-automation backstops (review-pins.test.ts, the cache-miss
+     * guard): every tracked mention must sit in a file known to talk ABOUT the
+     * ban, and a mention anywhere else fails red so the author reads the skill
+     * before re-recommending.
+     *
+     * Known limit: the allowlist is file-granular. It catches the ban RELOCATING
+     * (a mention appearing in a new file) but not a recommending sentence added
+     * to a file already allowed to mention the tool; that stays a review-time
+     * judgment. Excluded from the sweep entirely: `.changeset/` and the package
+     * CHANGELOG.md files (changeset bodies describing the ban are copied
+     * verbatim into the CHANGELOG at `changeset version`, so both carry the
+     * phrase legitimately and transiently grow), and the eval corpus (case
+     * fixtures quote arbitrary text; vitest.config.ts already carves out its
+     * tree/ dirs for the same reason).
+     */
+    import {spawnSync} from "node:child_process";
+    import * as fs from "fs";
+    import * as path from "path";
+    import {describe, expect, it} from "vitest";
+    const repoRoot = path.resolve(new URL(".", import.meta.url).pathname, "../..");
+    /**
+     * Files allowed to mention the tool: the ban's own section in the bump skill,
+     * the two onboarding-skill warnings, the consumer README's warning,
+     * review-pins.test.ts's doc comments about why the pins need a backstop at
+     * all, and this file, which carries the search string itself (the one
+     * non-prohibitive mention).
+     */
+    const ALLOWED = new Set([
+        ".claude/skills/review-consumer-bump/SKILL.md",
+        ".claude/skills/review-onboarding/SKILL.md",
+        ".github/workflows/review-pins.test.ts",
+        "workflows/review/README.md",
+        "workflows/review/gh-aw-update-ban.test.ts",
+    ]);
+    describe("the gh aw update ban", () => {
+        it("is mentioned only where it is being banned", () => {
+            // Every tracked file, not an extension list: the ban's residue has
+            // already turned up in .md prose, .ts warning strings, and a
+            // compiled .lock.yml, so an extension filter is just a bet on where
+            // the next one lands. Reading a binary as utf8 cannot match the
+            // phrase, so no file type needs excluding for safety.
+            const ls = spawnSync("git", ["ls-files"], {
+                cwd: repoRoot,
+                encoding: "utf8",
+            });
+            expect(ls.status).toBe(0);
+            const offenders = ls.stdout.split("\n").filter(
+                (file) =>
+                    file !== "" &&
+                    !file.startsWith(".changeset/") &&
+                    // `changeset version` copies each changeset body verbatim
+                    // into the package CHANGELOG.md, so the released notes
+                    // inherit whatever mentions .changeset/ was excused for.
+                    !file.endsWith("CHANGELOG.md") &&
+                    !file.startsWith("workflows/review/eval/corpus/") &&
+                    !ALLOWED.has(file) &&
+                    fs
+                        .readFileSync(path.join(repoRoot, file), "utf8")
+                        .includes("gh aw update"),
+            );
+            expect(offenders).toEqual([]);
+        });
+        it("keeps every allowlisted file actually mentioning it (stale allowlist detector)", () => {
+            for (const file of ALLOWED) {
+                expect(
+                    fs.readFileSync(path.join(repoRoot, file), "utf8"),
+                    `${file} no longer mentions the tool; prune it from ALLOWED`,
+                ).toContain("gh aw update");
+            }
+        });
+    });

workflows/review/lib/check-consumer-config.ts

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -544,7 +544,7 @@ export const checkConsumerConfig = (
  
            if (installed.source === undefined) {

                warn(

                    "source-missing",

                    `${workflowPath} carries no \`source:\` field, so \`gh aw update\` cannot find its upstream.`,

                    `${workflowPath} carries no \`source:\` field, so the manual bump flow cannot tell which upstream release this install was copied from.`,

                );

            } else if (installed.pinnedRef === undefined) {

                warn(

Navigation Menu

Uh oh!

review: stop recommending gh aw update in the consumer README #371

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!