From bfd3cead9113c8540ab4a3f5b28b0965e31d7462 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Thu, 9 Jul 2026 14:22:34 -0700 Subject: [PATCH 1/2] [jwies/review-version-sync] review: keep the pinned review.md ref in sync with the released version The pre-agent-steps checkout ref in review.md is supposed to name the release the file ships in, but changesets only bumps package.json, so every tag since review-v1.2.2 shipped with a stale ref (and v1.4.0's prompt invokes lib/provenance.ts, which a v1.2.2 checkout lacks). The release flow's version command now runs utils/sync-review-version.ts after changeset version, rewriting every review-v literal in review.md to the version being released so the bump lands in the same Version Packages commit that gets tagged. version-sync.test.ts is the CI backstop: it fails any PR where the ref diverges from the review package version. Also bumps the currently stale ref to review-v1.4.0. --- .changeset/review-version-sync.md | 5 + .github/workflows/release.yml | 5 + package.json | 1 + utils/sync-review-version-lib.test.ts | 133 ++++++++++++++++++++++++++ utils/sync-review-version-lib.ts | 79 +++++++++++++++ utils/sync-review-version.ts | 13 +++ workflows/review/README.md | 19 +++- workflows/review/review.md | 14 ++- workflows/review/version-sync.test.ts | 37 +++++++ 9 files changed, 299 insertions(+), 7 deletions(-) create mode 100644 .changeset/review-version-sync.md create mode 100644 utils/sync-review-version-lib.test.ts create mode 100644 utils/sync-review-version-lib.ts create mode 100644 utils/sync-review-version.ts create mode 100644 workflows/review/version-sync.test.ts diff --git a/.changeset/review-version-sync.md b/.changeset/review-version-sync.md new file mode 100644 index 00000000..7778646a --- /dev/null +++ b/.changeset/review-version-sync.md @@ -0,0 +1,5 @@ +--- +"review": patch +--- + +Keep the pinned `ref:` inside review.md in sync with the released version. The release flow's version command now runs `utils/sync-review-version.ts` after `changeset version`, rewriting every `review-v` literal in review.md to the version being released so the bump lands in the same Version Packages commit that gets tagged; `workflows/review/version-sync.test.ts` fails CI whenever the ref and the `review` package version diverge. Also bumps the currently stale ref from review-v1.2.2 to review-v1.4.0 (v1.3.0 through v1.4.0 shipped with the lagging ref, so an un-overridden consumer checked out a lib without `lib/provenance.ts`). diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3ddf0bb3..5b737ff3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -30,6 +30,11 @@ jobs: if: steps.has-changesets.outputs.result == 'true' uses: changesets/action@6a0a831ff30acef54f2c6aa1cbbc1096b066edaf # v1 with: + # Not the default `changeset version`: version-packages also syncs + # the pinned `ref:` inside workflows/review/review.md to the version + # being released, so the bump lands in the same Version Packages + # commit that gets tagged (see utils/sync-review-version.ts). + version: pnpm run version-packages publish: exit 1 env: GITHUB_TOKEN: ${{ secrets.KHAN_ACTIONS_BOT_TOKEN }} diff --git a/package.json b/package.json index f81bb81a..ae9c0cc6 100644 --- a/package.json +++ b/package.json @@ -2,6 +2,7 @@ "repository": "git@github.com:Khan/actions.git", "private": true, "scripts": { + "version-packages": "changeset version && node -r @swc-node/register utils/sync-review-version.ts", "lint": "eslint --ext .js,.mjs,.ts .", "fix": "pnpm run lint --fix", "test": "vitest", diff --git a/utils/sync-review-version-lib.test.ts b/utils/sync-review-version-lib.test.ts new file mode 100644 index 00000000..15126940 --- /dev/null +++ b/utils/sync-review-version-lib.test.ts @@ -0,0 +1,133 @@ +import {describe, expect, it, vi} from "vitest"; +import { + syncReviewVersion, + syncReviewVersionContent, +} from "./sync-review-version-lib.ts"; + +describe("syncReviewVersionContent", () => { + it("rewrites the pinned checkout ref to the given version", () => { + // Arrange: a stale ref, as shipped in review-v1.3.0 through v1.4.0. + const content = [ + "pre-agent-steps:", + " - uses: actions/checkout@abc # v5", + " with:", + " repository: Khan/actions", + " ref: review-v1.2.2", + ].join("\n"); + + // Act + const result = syncReviewVersionContent(content, "9.9.9"); + + // Assert + expect(result.replaced).toBe(1); + expect(result.content).toContain("ref: review-v9.9.9"); + expect(result.content).not.toContain("review-v1.2.2"); + }); + + it("rewrites every semver literal but leaves templates alone", () => { + // Arrange + const content = + "ref: review-v1.2.2\n" + + "the marker is `v=review-v` (a template)\n" + + "released as review-v1.3.0\n"; + + // Act + const result = syncReviewVersionContent(content, "2.0.0"); + + // Assert + expect(result.replaced).toBe(2); + expect(result.content).toBe( + "ref: review-v2.0.0\n" + + "the marker is `v=review-v` (a template)\n" + + "released as review-v2.0.0\n", + ); + }); + + it("returns unchanged content when already at the version", () => { + // Act + const result = syncReviewVersionContent("ref: review-v1.4.0", "1.4.0"); + + // Assert + expect(result.replaced).toBe(1); + expect(result.content).toBe("ref: review-v1.4.0"); + }); +}); + +describe("syncReviewVersion", () => { + const makeDeps = (files: Record) => { + const writes: Record = {}; + return { + writes, + deps: { + readFileSyncImpl: ((p: string) => { + const content = files[p]; + if (content === undefined) { + throw new Error(`unexpected read: ${p}`); + } + return content; + }) as never, + writeFileSyncImpl: ((p: string, data: string) => { + writes[p] = data; + }) as never, + log: vi.fn(), + }, + }; + }; + + it("writes review.md with the package version substituted", () => { + // Arrange + const {writes, deps} = makeDeps({ + "workflows/review/package.json": + '{"name": "review", "version": "1.5.0"}', + "workflows/review/review.md": " ref: review-v1.2.2\n", + }); + + // Act + syncReviewVersion(deps); + + // Assert + expect(writes["workflows/review/review.md"]).toBe( + " ref: review-v1.5.0\n", + ); + }); + + it("does not write when the ref is already current", () => { + // Arrange + const {writes, deps} = makeDeps({ + "workflows/review/package.json": + '{"name": "review", "version": "1.5.0"}', + "workflows/review/review.md": " ref: review-v1.5.0\n", + }); + + // Act + syncReviewVersion(deps); + + // Assert + expect(writes).toEqual({}); + }); + + it("throws when review.md has no review-v literal to sync", () => { + // Arrange + const {deps} = makeDeps({ + "workflows/review/package.json": + '{"name": "review", "version": "1.5.0"}', + "workflows/review/review.md": "no pinned ref here\n", + }); + + // Act + Assert + expect(() => syncReviewVersion(deps)).toThrow( + /No review-v literals/, + ); + }); + + it("throws when package.json has no version", () => { + // Arrange + const {deps} = makeDeps({ + "workflows/review/package.json": '{"name": "review"}', + "workflows/review/review.md": "ref: review-v1.2.2\n", + }); + + // Act + Assert + expect(() => syncReviewVersion(deps)).toThrow(/No version found/); + }); +}); diff --git a/utils/sync-review-version-lib.ts b/utils/sync-review-version-lib.ts new file mode 100644 index 00000000..fe999f1f --- /dev/null +++ b/utils/sync-review-version-lib.ts @@ -0,0 +1,79 @@ +import * as fs from "fs"; + +/** + * Keep the released-version literals inside workflows/review/review.md in + * sync with the "review" package version. + * + * review.md checks out Khan/actions at a pinned `ref: review-v` tag + * to fetch the deterministic lib the prompt invokes at runtime; that ref is + * supposed to name the release the file ships in. Changesets bumps + * package.json but knows nothing about prose in a markdown file, so this + * script runs alongside `changeset version` (see the root package.json + * "version-packages" script, used by .github/workflows/release.yml) to + * rewrite every review-v literal in review.md to the version being + * released. That way the bump lands in the same Version Packages commit that + * gets tagged, and the tag's review.md pins its own release. + * + * workflows/review/version-sync.test.ts is the CI backstop: it fails when + * the literals and the package version diverge. + */ + +export const REVIEW_MD_PATH = "workflows/review/review.md"; +export const REVIEW_PKG_PATH = "workflows/review/package.json"; + +export const REVIEW_VERSION_RE = /review-v\d+\.\d+\.\d+/g; + +export const syncReviewVersionContent = ( + content: string, + version: string, +): {content: string; replaced: number} => { + let replaced = 0; + const next = content.replace(REVIEW_VERSION_RE, () => { + replaced += 1; + return `review-v${version}`; + }); + return {content: next, replaced}; +}; + +export type SyncReviewVersionDeps = { + readFileSyncImpl?: typeof fs.readFileSync; + writeFileSyncImpl?: typeof fs.writeFileSync; + log?: (message: string) => void; +}; + +export const syncReviewVersion = (deps: SyncReviewVersionDeps = {}): void => { + const readFileSyncImpl = deps.readFileSyncImpl ?? fs.readFileSync; + const writeFileSyncImpl = deps.writeFileSyncImpl ?? fs.writeFileSync; + const log = deps.log ?? console.log; + + const pkg = JSON.parse(readFileSyncImpl(REVIEW_PKG_PATH, "utf-8")); + if (!pkg.version) { + throw new Error(`No version found in ${REVIEW_PKG_PATH}`); + } + + const content = readFileSyncImpl(REVIEW_MD_PATH, "utf-8"); + const {content: synced, replaced} = syncReviewVersionContent( + content, + pkg.version, + ); + + if (replaced === 0) { + // At minimum the pinned-checkout `ref:` line must match; finding + // nothing means the file was restructured and the sync went blind. + throw new Error( + `No review-v literals found in ${REVIEW_MD_PATH}; ` + + `expected at least the pinned checkout ref line.`, + ); + } + + if (synced === content) { + log(`${REVIEW_MD_PATH}: already at review-v${pkg.version}`); + return; + } + + writeFileSyncImpl(REVIEW_MD_PATH, synced); + log( + `${REVIEW_MD_PATH}: synced ${replaced} review-v literal(s) ` + + `to review-v${pkg.version}`, + ); +}; diff --git a/utils/sync-review-version.ts b/utils/sync-review-version.ts new file mode 100644 index 00000000..6c09a9dd --- /dev/null +++ b/utils/sync-review-version.ts @@ -0,0 +1,13 @@ +/** + * Rewrite the review-v literals in workflows/review/review.md (the + * pinned Khan/actions checkout ref) to the current "review" package version. + * + * Runs as part of the changesets version command (root package.json + * "version-packages", wired into .github/workflows/release.yml) so the bump + * lands in the same Version Packages commit that gets tagged. + * + * Usage: node -r @swc-node/register utils/sync-review-version.ts + */ +import {syncReviewVersion} from "./sync-review-version-lib.ts"; + +syncReviewVersion(); diff --git a/workflows/review/README.md b/workflows/review/README.md index da755173..793cec30 100644 --- a/workflows/review/README.md +++ b/workflows/review/README.md @@ -67,8 +67,8 @@ ceiling with everything spent and nothing posted. gh aw add Khan/actions/workflows/review/review.md # Or pin to a published version (recommended for stability): -gh aw add Khan/actions/workflows/review/review.md@review-v0 -gh aw add Khan/actions/workflows/review/review.md@review-v0.1.0 +gh aw add Khan/actions/workflows/review/review.md@review-v +gh aw add Khan/actions/workflows/review/review.md@review-v.. ``` This copies `review.md` into the consuming repo's `.github/workflows/`, records a @@ -76,6 +76,12 @@ This copies `review.md` into the consuming repo's `.github/workflows/`, records plus the consumer config files below. Pull future updates with `gh aw update` (a 3-way merge that preserves your local edits). +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 +flow rewrites it; see [Versioning](#versioning)), so after `gh aw add` or +`gh aw update` the imported file already fetches the matching lib code and needs +no manual fix-up of the ref. + ## Consumer configuration The workflow imports the following files **from the consuming repo** (they resolve @@ -213,6 +219,15 @@ tag) is cut **at the real commit tree** (not the rewritten-subtree bare tags tha the `actions/` packages use), so the nested `workflows/review/review.md@` path resolves for `gh aw add`. +The pinned checkout `ref:` inside `review.md` is part of the release: the version +command (`pnpm run version-packages`, wired into `release.yml`) runs +`utils/sync-review-version.ts` after `changeset version`, rewriting every +`review-v` literal in `review.md` to the version being released, so the +bump lands in the same Version Packages commit that gets tagged. +`version-sync.test.ts` here is the CI backstop: it fails any PR where those +literals do not match the `review` package version (releases v1.3.0 through +v1.4.0 shipped still pointing at v1.2.2, before the sync existed). + ### Version attribution Semver is the behavior contract: a release that changes the reviewer's behavior bumps diff --git a/workflows/review/review.md b/workflows/review/review.md index ce516974..86dd4e60 100644 --- a/workflows/review/review.md +++ b/workflows/review/review.md @@ -196,16 +196,20 @@ models: # repo, so the job fetches the code itself: check out Khan/actions at the pinned # release below. The `ref` is the single version # surface for prompt + code: it names the Khan/actions release this file ships in -# (changesets tag, `review-v`), and any release that changes the prompt or -# the lib bumps it. Steps that run lib scripts invoke them from `gh-aw-review-lib/` -# via `npx -y tsx