diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d6d32345..3b9c0f71 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,6 +7,27 @@ on: branches: [main, dev] jobs: + # Pre-flight for the hash guard in publish-plugins.yml, which is the last line + # of defence and stays where it is. That guard compares built bytes against + # published bytes, so it needs a build and only runs deep inside a release — + # after `tag` has already made the version tag immutable (issue #92). This job + # answers the cheap proxy question instead (plugin source changed since its + # published tag + manifest version unchanged) in seconds, on the PR. + plugin-versions: + runs-on: ubuntu-latest + steps: + # fetch-depth: 0 — the check reads the plugin-*-v* release tags, and a + # shallow checkout has none of them. + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - uses: actions/setup-node@v6 + with: + node-version: lts/* + + - run: node scripts/check-plugin-versions.mjs + # `pnpm build` runs scripts/build-plugins.mjs, which spawns a child process per # plugin. That spawn is platform-sensitive — on Windows `pnpm` is a .cmd shim — # and until v0.14.0 nothing here ever ran the frontend build on Windows. The diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c99daf51..cd5a9048 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -7,6 +7,10 @@ on: description: 'Release tag to build (e.g. v0.1.51)' required: true type: string + outputs: + published: + description: '"true" once the GitHub release itself is out of draft. Lets a caller run the distribution channels that only consume the published release even when an unrelated job here (e.g. publish-plugins) failed.' + value: ${{ jobs.publish-release.outputs.published }} workflow_dispatch: inputs: tag: @@ -416,8 +420,13 @@ jobs: runs-on: ubuntu-latest permissions: contents: write + outputs: + published: ${{ steps.publish.outputs.published }} steps: - name: Publish the release + id: publish env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: gh release edit "${{ inputs.tag }}" --draft=false --latest -R "${{ github.repository }}" + run: | + gh release edit "${{ inputs.tag }}" --draft=false --latest -R "${{ github.repository }}" + echo "published=true" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/tag-release.yml b/.github/workflows/tag-release.yml index f079e6d4..56e910f8 100644 --- a/.github/workflows/tag-release.yml +++ b/.github/workflows/tag-release.yml @@ -17,7 +17,27 @@ permissions: contents: write jobs: + # ci runs this same check, but it triggers on the SAME push to main and nothing + # makes tagging wait for it. A release cut by pushing straight to main could + # therefore tag while the check is still running or already red — and the tag is + # immutable, which is the whole problem #92 is about. Gating `tag` on it here is + # what actually holds the "fail before the tag exists" property. + plugin-versions: + runs-on: ubuntu-latest + steps: + # fetch-depth: 0 — the check reads the plugin-*-v* release tags. + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - uses: actions/setup-node@v6 + with: + node-version: lts/* + + - run: node scripts/check-plugin-versions.mjs + tag: + needs: plugin-versions runs-on: ubuntu-latest permissions: contents: write @@ -70,12 +90,45 @@ jobs: tag: ${{ needs.tag.outputs.tag }} secrets: inherit + # The distribution channels below consume ONLY the published GitHub release and + # its assets, but used to be `needs: [tag, release]` — so any failure anywhere in + # release.yml skipped them. A plugin-manifest guard failing after the release was + # already published took Homebrew, winget and the apt/yum repos out for v0.16.0 + # with no retry and no fallback trigger (issue #92). Gate them on the release + # being published instead of on the whole workflow succeeding. + # + # `release-published` reads publish-release's output, falling back to the release + # state on GitHub: a reusable workflow whose overall conclusion is failure is the + # exact case that matters here, and it must not be able to leave this gate blank. + release-published: + needs: [tag, release] + if: ${{ !cancelled() && needs.tag.outputs.created == 'true' }} + runs-on: ubuntu-latest + outputs: + ok: ${{ steps.check.outputs.ok }} + steps: + - id: check + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + TAG: ${{ needs.tag.outputs.tag }} + REPO: ${{ github.repository }} + REPORTED: ${{ needs.release.outputs.published }} + run: | + draft=$(gh release view "$TAG" -R "$REPO" --json isDraft -q .isDraft 2>/dev/null || echo missing) + if [ "$REPORTED" = "true" ] || [ "$draft" = "false" ]; then + echo "ok=true" >> "$GITHUB_OUTPUT" + echo "Release $TAG is published (publish-release said '$REPORTED', isDraft=$draft)." + else + echo "ok=false" >> "$GITHUB_OUTPUT" + echo "::warning::Release $TAG is not published (publish-release said '$REPORTED', isDraft=$draft) — skipping the distribution channels." + fi + # Publish Homebrew cask + winget manifest once the release and its assets # exist. Called explicitly here rather than via `release: published`, which # never fires for a GITHUB_TOKEN-authored release (see publish-installers.yml). publish-installers: - needs: [tag, release] - if: needs.tag.outputs.created == 'true' + needs: [tag, release-published] + if: ${{ !cancelled() && needs.release-published.outputs.ok == 'true' }} uses: ./.github/workflows/publish-installers.yml with: tag: ${{ needs.tag.outputs.tag }} @@ -86,7 +139,7 @@ jobs: # release never fires that event, so it's called explicitly here. The script # rebuilds statelessly from the last 5 releases, so it self-heals. publish-repo: - needs: [tag, release] - if: needs.tag.outputs.created == 'true' + needs: [tag, release-published] + if: ${{ !cancelled() && needs.release-published.outputs.ok == 'true' }} uses: ./.github/workflows/publish-repo.yml secrets: inherit diff --git a/scripts/check-plugin-versions.mjs b/scripts/check-plugin-versions.mjs new file mode 100644 index 00000000..b2401580 --- /dev/null +++ b/scripts/check-plugin-versions.mjs @@ -0,0 +1,116 @@ +#!/usr/bin/env node +// Pre-flight for the hash guard in .github/workflows/publish-plugins.yml. +// +// That guard is the authoritative one: it compares the FRESHLY BUILT bytes of a +// plugin against the bytes already attached to its published release, and fails +// when they differ. It can only run after a build, and in release.yml it runs +// after `tag` has made the version tag immutable — far too late to amend. +// +// This is the cheap proxy that runs in ci instead: source changed under +// src/plugins// since that plugin's last published tag + manifest version +// unchanged. It is not byte-exact (a source edit can be a no-op for the bundler, +// and a dependency bump outside the plugin folder can change the bytes without +// tripping this), so it never replaces the release-time guard — it just moves the +// common case to PR review, where bumping the manifest is still a one-line fix. + +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { FIRST_PARTY_PLUGIN_IDS, releaseTagFor } from "./build-plugins.mjs"; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); + +// Nothing under a plugin folder that matches this reaches the published bundle, +// so a change to it cannot make the built bytes diverge from the release. +const NON_BUNDLED = /(^|\/)[^/]*\.(test|spec)\.[jt]sx?$/; + +export function isBundledSource(relPath) { + return !NON_BUNDLED.test(relPath); +} + +/** + * Pure decision for one plugin. Returns null when it is fine, or a violation. + * + * @param {{folderId: string, manifestId: string, version: string, + * publishedTags: string[], changedFiles: string[]}} input + */ +export function violationFor({ folderId, manifestId, version, publishedTags, changedFiles }) { + const tag = releaseTagFor(manifestId, version); + // No release for this version yet: whatever changed will ship as new bytes + // under a new tag, which is exactly what a version bump is for. + if (!publishedTags.includes(tag)) return null; + const files = changedFiles.filter(isBundledSource); + if (files.length === 0) return null; + return { folderId, manifestId, version, tag, files }; +} + +export function formatViolation(v) { + return ( + `${v.manifestId}: source under src/plugins/${v.folderId}/ changed since the published ` + + `release '${v.tag}', but manifest.json still says version ${v.version}. ` + + `Publishing would overwrite bytes the live marketplace catalogue pins a hash to. ` + + `Bump the 'version' field in src/plugins/${v.folderId}/manifest.json.\n` + + v.files.map((f) => ` changed: ${f}`).join("\n") + ); +} + +function git(args) { + return execFileSync("git", args, { cwd: ROOT, encoding: "utf8" }); +} + +function listPublishedTags() { + return git(["tag", "--list", "plugin-*-v*"]) + .split("\n") + .map((s) => s.trim()) + .filter(Boolean); +} + +function changedSince(tag, folderId) { + // Two-dot: tree-vs-tree. The published bytes came from the tag's tree, so the + // merge base is irrelevant here — what matters is how HEAD differs from what + // was actually released. + return git(["diff", "--name-only", `${tag}..HEAD`, "--", `src/plugins/${folderId}/`]) + .split("\n") + .map((s) => s.trim()) + .filter(Boolean); +} + +function main() { + const publishedTags = listPublishedTags(); + if (publishedTags.length === 0) { + // A shallow or tagless clone makes every tag look unpublished, which would + // turn this check into a silent no-op. Fail instead. + console.error( + "check-plugin-versions: no plugin-*-v* tags visible. Fetch tags (checkout with fetch-depth: 0) before running this check.", + ); + process.exit(2); + } + + const violations = []; + for (const folderId of FIRST_PARTY_PLUGIN_IDS) { + const manifest = JSON.parse( + readFileSync(path.join(ROOT, "src/plugins", folderId, "manifest.json"), "utf8"), + ); + const tag = releaseTagFor(manifest.id, manifest.version); + const changedFiles = publishedTags.includes(tag) ? changedSince(tag, folderId) : []; + const v = violationFor({ + folderId, + manifestId: manifest.id, + version: manifest.version, + publishedTags, + changedFiles, + }); + if (v) violations.push(v); + } + + if (violations.length > 0) { + for (const v of violations) console.error(`::error::${formatViolation(v)}`); + process.exit(1); + } + console.log(`check-plugin-versions: all ${FIRST_PARTY_PLUGIN_IDS.length} plugins OK.`); +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + main(); +} diff --git a/tests/pluginVersionPreflight.test.ts b/tests/pluginVersionPreflight.test.ts new file mode 100644 index 00000000..5b3db51c --- /dev/null +++ b/tests/pluginVersionPreflight.test.ts @@ -0,0 +1,61 @@ +import { describe, test, expect } from "vitest"; +import { + isBundledSource, + violationFor, + formatViolation, +} from "../scripts/check-plugin-versions.mjs"; + +const PUBLISHED = ["plugin-gist-sync-v1.0.0", "plugin-gist-sync-v1.1.0", "plugin-docker-v1.2.0"]; + +function check(over: Record = {}) { + return violationFor({ + folderId: "gist-sync", + manifestId: "plugin-gist-sync", + version: "1.1.0", + publishedTags: PUBLISHED, + changedFiles: ["src/plugins/gist-sync/SettingsPage.tsx"], + ...over, + }); +} + +describe("plugin manifest-version pre-flight", () => { + test("flags source changed against an already-published version", () => { + const v = check(); + expect(v).not.toBeNull(); + expect(v!.tag).toBe("plugin-gist-sync-v1.1.0"); + expect(v!.files).toEqual(["src/plugins/gist-sync/SettingsPage.tsx"]); + }); + + test("passes once the manifest version is bumped past every published tag", () => { + expect(check({ version: "1.1.1" })).toBeNull(); + }); + + test("passes when nothing under the plugin folder changed", () => { + expect(check({ changedFiles: [] })).toBeNull(); + }); + + test("ignores changes that never reach the bundle", () => { + expect(check({ changedFiles: ["src/plugins/gist-sync/register.test.ts"] })).toBeNull(); + expect(check({ changedFiles: ["src/plugins/gist-sync/sync-engine.spec.tsx"] })).toBeNull(); + }); + + test("still flags when a bundled file changes alongside a test file", () => { + const v = check({ + changedFiles: ["src/plugins/gist-sync/register.test.ts", "src/plugins/gist-sync/crypto.ts"], + }); + expect(v!.files).toEqual(["src/plugins/gist-sync/crypto.ts"]); + }); + + test("isBundledSource keeps ordinary sources", () => { + expect(isBundledSource("src/plugins/docker/index.ts")).toBe(true); + expect(isBundledSource("src/plugins/docker/manifest.json")).toBe(true); + expect(isBundledSource("src/plugins/docker/latest.test.ts")).toBe(false); + }); + + test("the message names the plugin, the tag and the fix", () => { + const msg = formatViolation(check()!); + expect(msg).toContain("plugin-gist-sync"); + expect(msg).toContain("plugin-gist-sync-v1.1.0"); + expect(msg).toContain("src/plugins/gist-sync/manifest.json"); + }); +});