diff --git a/.changeset/review-version-sync.md b/.changeset/review-version-sync.md new file mode 100644 index 00000000..5c4862e2 --- /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-workflow-versions.ts` after `changeset version`, rewriting every workflow's pinned `-v` literals (for this workflow, the `ref:` inside 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..f0a6b502 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -30,6 +30,12 @@ jobs: if: steps.has-changesets.outputs.result == 'true' uses: changesets/action@6a0a831ff30acef54f2c6aa1cbbc1096b066edaf # v1 with: + # `version-packages` runs `changeset version` and then syncs each + # workflow's pinned `-v` literals (e.g. the checkout + # 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-workflow-versions.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..3d5bed5e 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-workflow-versions.ts", "lint": "eslint --ext .js,.mjs,.ts .", "fix": "pnpm run lint --fix", "test": "vitest", diff --git a/utils/sync-workflow-versions-lib.test.ts b/utils/sync-workflow-versions-lib.test.ts new file mode 100644 index 00000000..dfa607af --- /dev/null +++ b/utils/sync-workflow-versions-lib.test.ts @@ -0,0 +1,214 @@ +import {describe, expect, it, vi} from "vitest"; +import { + syncWorkflowVersionContent, + syncWorkflowVersions, + type DirEntryLike, +} from "./sync-workflow-versions-lib.ts"; + +describe("syncWorkflowVersionContent", () => { + 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 = syncWorkflowVersionContent(content, "review", "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 = syncWorkflowVersionContent(content, "review", "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("leaves other workflows' literals alone", () => { + // Arrange + const content = "ref: review-v1.2.2\nsee also triage-v3.0.0\n"; + + // Act + const result = syncWorkflowVersionContent(content, "review", "2.0.0"); + + // Assert + expect(result.replaced).toBe(1); + expect(result.content).toBe( + "ref: review-v2.0.0\nsee also triage-v3.0.0\n", + ); + }); + + it("returns unchanged content when already at the version", () => { + // Act + const result = syncWorkflowVersionContent( + "ref: review-v1.4.0", + "review", + "1.4.0", + ); + + // Assert + expect(result.replaced).toBe(1); + expect(result.content).toBe("ref: review-v1.4.0"); + }); +}); + +describe("syncWorkflowVersions", () => { + const makeDeps = (files: Record) => { + const writes: Record = {}; + const readdirSyncImpl = (dir: string): DirEntryLike[] => { + const children = new Map(); + for (const path of Object.keys(files)) { + if (!path.startsWith(`${dir}/`)) { + continue; + } + const rest = path.slice(dir.length + 1); + const slash = rest.indexOf("/"); + children.set( + slash === -1 ? rest : rest.slice(0, slash), + slash !== -1, + ); + } + return [...children].map(([name, isDir]) => ({ + name, + isDirectory: () => isDir, + isFile: () => !isDir, + })); + }; + return { + writes, + deps: { + readFileSyncImpl: (path: string) => { + const content = files[path]; + if (content === undefined) { + throw new Error(`unexpected read: ${path}`); + } + return content; + }, + writeFileSyncImpl: (path: string, data: string) => { + writes[path] = data; + }, + readdirSyncImpl, + 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 + syncWorkflowVersions(deps); + + // Assert + expect(writes["workflows/review/review.md"]).toBe( + " ref: review-v1.5.0\n", + ); + }); + + it("syncs every workflow to its own version", () => { + // 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", + "workflows/triage/package.json": + '{"name": "triage", "version": "2.1.0"}', + "workflows/triage/triage.md": + "ref: triage-v2.0.0\nsee review-v1.2.2\n", + }); + + // Act + syncWorkflowVersions(deps); + + // Assert: each workflow gets its own version, and a mention of + // another workflow's tag is not rewritten. + expect(writes["workflows/review/review.md"]).toBe( + "ref: review-v1.5.0\n", + ); + expect(writes["workflows/triage/triage.md"]).toBe( + "ref: triage-v2.1.0\nsee review-v1.2.2\n", + ); + }); + + it("never rewrites CHANGELOG.md", () => { + // Arrange: the changelog holds historic versions on purpose. + const {writes, deps} = makeDeps({ + "workflows/review/package.json": + '{"name": "review", "version": "1.5.0"}', + "workflows/review/review.md": "ref: review-v1.5.0\n", + "workflows/review/CHANGELOG.md": "## review-v1.2.2 notes\n", + }); + + // Act + syncWorkflowVersions(deps); + + // Assert + expect(writes).toEqual({}); + }); + + it("is a no-op for a workflow with no version literals", () => { + // Arrange + const {writes, deps} = makeDeps({ + "workflows/triage/package.json": + '{"name": "triage", "version": "2.0.0"}', + "workflows/triage/triage.md": "no pinned ref here\n", + }); + + // Act + syncWorkflowVersions(deps); + + // Assert + expect(writes).toEqual({}); + }); + + 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 + syncWorkflowVersions(deps); + + // Assert + expect(writes).toEqual({}); + }); + + it("throws when a workflow's 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(() => syncWorkflowVersions(deps)).toThrow(/No version found/); + }); +}); diff --git a/utils/sync-workflow-versions-lib.ts b/utils/sync-workflow-versions-lib.ts new file mode 100644 index 00000000..d113788d --- /dev/null +++ b/utils/sync-workflow-versions-lib.ts @@ -0,0 +1,122 @@ +import * as fs from "fs"; + +/** + * Keep released-version literals inside each workflow's markdown in sync + * with that workflow's package version. + * + * A workflow under `workflows//` is released as a `-v` + * git tag named after the directory (see publishWorkflow in + * utils/publish.ts), and its markdown may embed that tag as a literal: + * review.md, for example, checks out Khan/actions at a pinned + * `ref: review-v` tag to fetch the deterministic lib the prompt + * invokes at runtime. 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 `-v` literal + * in each workflow's markdown to the version being released. That way the + * bump lands in the same Version Packages commit that gets tagged, and each + * tag's markdown pins its own release. + * + * CHANGELOG.md is skipped (changesets writes historic versions there on + * purpose), and a workflow whose markdown embeds no literal is a no-op. A + * workflow that must embed one enforces that with its own contract test; + * workflows/review/version-sync.test.ts is the CI backstop that fails when + * review.md's literals and the `review` package version diverge. + */ + +export const WORKFLOWS_DIR = "workflows"; + +const escapeRegExp = (value: string): string => + value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + +export const workflowVersionRe = (name: string): RegExp => + new RegExp(`${escapeRegExp(name)}-v\\d+\\.\\d+\\.\\d+`, "g"); + +export const syncWorkflowVersionContent = ( + content: string, + name: string, + version: string, +): {content: string; replaced: number} => { + let replaced = 0; + const next = content.replace(workflowVersionRe(name), () => { + replaced += 1; + return `${name}-v${version}`; + }); + return {content: next, replaced}; +}; + +export type DirEntryLike = { + name: string; + isDirectory: () => boolean; + isFile: () => boolean; +}; + +export type SyncWorkflowVersionsDeps = { + readFileSyncImpl?: (path: string, encoding: "utf-8") => string; + writeFileSyncImpl?: (path: string, data: string) => void; + readdirSyncImpl?: (path: string) => DirEntryLike[]; + log?: (message: string) => void; +}; + +export const syncWorkflowVersions = ( + deps: SyncWorkflowVersionsDeps = {}, +): void => { + const readFileSyncImpl = + deps.readFileSyncImpl ?? + ((path: string, encoding: "utf-8") => fs.readFileSync(path, encoding)); + const writeFileSyncImpl = deps.writeFileSyncImpl ?? fs.writeFileSync; + const readdirSyncImpl = + deps.readdirSyncImpl ?? + ((path: string) => fs.readdirSync(path, {withFileTypes: true})); + const log = deps.log ?? console.log; + + const workflowNames = readdirSyncImpl(WORKFLOWS_DIR) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name); + + for (const name of workflowNames) { + // The release tag is named after the directory, not package.json's + // "name" field (see publishWorkflow in utils/publish.ts). + const pkgPath = `${WORKFLOWS_DIR}/${name}/package.json`; + const pkg = JSON.parse(readFileSyncImpl(pkgPath, "utf-8")); + if (!pkg.version) { + throw new Error(`No version found in ${pkgPath}`); + } + + const markdownFiles = readdirSyncImpl(`${WORKFLOWS_DIR}/${name}`) + .filter( + (entry) => + entry.isFile() && + entry.name.endsWith(".md") && + // Changesets writes historic versions to the changelog; + // those must stay as released. + entry.name !== "CHANGELOG.md", + ) + .map((entry) => entry.name); + + for (const fileName of markdownFiles) { + const filePath = `${WORKFLOWS_DIR}/${name}/${fileName}`; + const content = readFileSyncImpl(filePath, "utf-8"); + const {content: synced, replaced} = syncWorkflowVersionContent( + content, + name, + pkg.version, + ); + + if (replaced === 0) { + continue; + } + + if (synced === content) { + log(`${filePath}: already at ${name}-v${pkg.version}`); + continue; + } + + writeFileSyncImpl(filePath, synced); + log( + `${filePath}: synced ${replaced} ${name}-v literal(s) ` + + `to ${name}-v${pkg.version}`, + ); + } + } +}; diff --git a/utils/sync-workflow-versions.ts b/utils/sync-workflow-versions.ts new file mode 100644 index 00000000..680c92b1 --- /dev/null +++ b/utils/sync-workflow-versions.ts @@ -0,0 +1,14 @@ +/** + * Rewrite each workflow's `-v` literals (e.g. the pinned + * Khan/actions checkout ref inside workflows/review/review.md) to that + * workflow's current 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-workflow-versions.ts + */ +import {syncWorkflowVersions} from "./sync-workflow-versions-lib.ts"; + +syncWorkflowVersions(); diff --git a/workflows/review/README.md b/workflows/review/README.md index da755173..6f43c20b 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,16 @@ 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-workflow-versions.ts` after `changeset version`, rewriting every +`-v` literal in each workflow's markdown (for this workflow, +every `review-v` 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..90f9a207 100644 --- a/workflows/review/review.md +++ b/workflows/review/review.md @@ -196,16 +196,21 @@ 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