diff --git a/.changeset/readme-no-gh-aw-update.md b/.changeset/readme-no-gh-aw-update.md new file mode 100644 index 00000000..dfb9333c --- /dev/null +++ b/.changeset/readme-no-gh-aw-update.md @@ -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 `` 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/.claude/skills/review-consumer-bump/SKILL.md b/.claude/skills/review-consumer-bump/SKILL.md index 2b46f9d9..d3cbccc1 100644 --- a/.claude/skills/review-consumer-bump/SKILL.md +++ b/.claude/skills/review-consumer-bump/SKILL.md @@ -70,6 +70,10 @@ It is the obvious tool and it fails twice, both observed live: 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` diff --git a/.claude/skills/review-onboarding/SKILL.md b/.claude/skills/review-onboarding/SKILL.md index 36cb2aef..ec5dbe91 100644 --- a/.claude/skills/review-onboarding/SKILL.md +++ b/.claude/skills/review-onboarding/SKILL.md @@ -428,6 +428,7 @@ the diff shows what a version bump changed. cd && 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 ``` diff --git a/.github/workflows/review.lock.yml b/.github/workflows/review.lock.yml index 0d7cf05d..bf72b4e3 100644 --- a/.github/workflows/review.lock.yml +++ b/.github/workflows/review.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"440b6a7a70150991e38e4bc64a8b6dcd53ff8d19bec9246cbed7690505b624e2","body_hash":"7950eb3d594a732455b3e39cc3830969953bffff1328686e0ad4ba3e1e73656d","compiler_version":"v0.85.4","strict":true,"agent_id":"claude","agent_model":"claude-opus-5","engine_versions":{"claude":"2.1.222"}} +# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"6e0bf8c9a89f6e981b8444b08bae42b9ebbfbf9f6bdf52d1a11342bb7b3a7a46","body_hash":"7950eb3d594a732455b3e39cc3830969953bffff1328686e0ad4ba3e1e73656d","compiler_version":"v0.85.4","strict":true,"agent_id":"claude","agent_model":"claude-opus-5","engine_versions":{"claude":"2.1.222"}} # gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GITHUB_TOKEN","KHAN_ACTIONS_BOT_TOKEN","REVIEW_JIRA_API_TOKEN","REVIEW_JIRA_EMAIL"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"3d3c42e5aac5ba805825da76410c181273ba90b1","version":"v7.0.1"},{"repo":"actions/checkout","sha":"93cb6efe18208431cddfb8368fd83d5badbf9bfd","version":"93cb6efe18208431cddfb8368fd83d5badbf9bfd"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"},{"repo":"github/gh-aw-actions/setup","sha":"2709137ea6c5b0e19aa621454dc643ea8dc526b1","version":"v0.85.4"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.44","digest":"sha256:0d727725c737b58c7bdf51f640cffb928385ec46517e0917c7f1a02f1bada8b4","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.44@sha256:0d727725c737b58c7bdf51f640cffb928385ec46517e0917c7f1a02f1bada8b4"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.44","digest":"sha256:b50fbadba138f6e9aba94aca09711335c489bb3b15861220cb66f6092e042dc7","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.44@sha256:b50fbadba138f6e9aba94aca09711335c489bb3b15861220cb66f6092e042dc7"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.44","digest":"sha256:83e48bbe12c634be8c228a576832fe45f66c529ac3659db92bddbcf2eeb6d627","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.44@sha256:83e48bbe12c634be8c228a576832fe45f66c529ac3659db92bddbcf2eeb6d627"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.8","digest":"sha256:38bbea36cdb46a3c9d04d1db05e672966f5239b431a2022eb35881688e5721d8","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.8@sha256:38bbea36cdb46a3c9d04d1db05e672966f5239b431a2022eb35881688e5721d8"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:0d9f1fb5fd6610c0ac1f5194a38e45a8a1e81f8a390d5142d8e4e6f26a4b3196","pinned_image":"ghcr.io/github/gh-aw-node@sha256:0d9f1fb5fd6610c0ac1f5194a38e45a8a1e81f8a390d5142d8e4e6f26a4b3196"},{"image":"ghcr.io/github/github-mcp-server:v1.8.0","digest":"sha256:d5a18c04b92714c309eb46a2305087e91a4dbd80420f6e462656699f95093520","pinned_image":"ghcr.io/github/github-mcp-server:v1.8.0@sha256:d5a18c04b92714c309eb46a2305087e91a4dbd80420f6e462656699f95093520"}],"has_pull_request":true} # This file was automatically generated by gh-aw (v0.85.4). DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # diff --git a/.github/workflows/review.md b/.github/workflows/review.md index 328d8020..f52fb781 100644 --- a/.github/workflows/review.md +++ b/.github/workflows/review.md @@ -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 diff --git a/workflows/review/README.md b/workflows/review/README.md index 70aae8a7..37e8cf62 100644 --- a/workflows/review/README.md +++ b/workflows/review/README.md @@ -194,7 +194,9 @@ plus the consumer config files below. Pull future updates with the 3-way merge flow in [`review-consumer-bump`](../../.claude/skills/review-consumer-bump/SKILL.md), **not** `gh aw update`: the tool does not recognize the `review-v` 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` tag pins its own `pre-agent-steps` checkout `ref:` to that same version (the release diff --git a/workflows/review/gh-aw-update-ban.test.ts b/workflows/review/gh-aw-update-ban.test.ts new file mode 100644 index 00000000..30f383e9 --- /dev/null +++ b/workflows/review/gh-aw-update-ban.test.ts @@ -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"); + } + }); +}); diff --git a/workflows/review/lib/check-consumer-config.ts b/workflows/review/lib/check-consumer-config.ts index a0a63e68..94d67dbe 100644 --- a/workflows/review/lib/check-consumer-config.ts +++ b/workflows/review/lib/check-consumer-config.ts @@ -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(