feat(release): track prerelease deltas and validate committed notes (#216)

Co-authored-by: sudacode <claude@lmaoxd.lol>

src/prerelease-workflow.test.ts

@@ -2,9 +2,17 @@ import test from 'node:test';
 import assert from 'node:assert/strict';
 import { readFileSync } from 'node:fs';
 import { resolve } from 'node:path';
+import {
+  jobSteps,
+  readWorkflow,
+  stepRunsCommand,
+  stepsMissingEnvDeclaration,
+  templateExpressionsInRunBodies,
+} from './workflow-test-helpers';
 
 const prereleaseWorkflowPath = resolve(__dirname, '../.github/workflows/prerelease.yml');
 const prereleaseWorkflow = readFileSync(prereleaseWorkflowPath, 'utf8').replace(/\r\n/g, '\n');
+const parsedPrereleaseWorkflow = readWorkflow(prereleaseWorkflowPath);
 const packageJsonPath = resolve(__dirname, '../package.json');
 const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf8')) as {
   scripts: Record<string, string>;
@@ -122,3 +130,33 @@ test('prerelease workflow does not publish to AUR', () => {
   assert.doesNotMatch(prereleaseWorkflow, /AUR_SSH_PRIVATE_KEY/);
   assert.doesNotMatch(prereleaseWorkflow, /scripts\/update-aur-package\.sh/);
 });
+
+test('prerelease workflow rejects committed notes generated for a different beta or rc', () => {
+  assert.equal(
+    packageJson.scripts['changelog:check-prerelease-notes'],
+    'bun run scripts/build-changelog.ts check-prerelease-notes',
+  );
+
+  // Matched at command positions only, so commenting the check out or quoting it
+  // inside an echo fails the test rather than silently satisfying it.
+  const steps = jobSteps(parsedPrereleaseWorkflow, 'release');
+  const checkIndex = steps.findIndex((step) =>
+    stepRunsCommand(
+      step,
+      /^bun run changelog:check-prerelease-notes --version "\$RELEASE_VERSION"/,
+    ),
+  );
+  const publishIndex = steps.findIndex((step) =>
+    stepRunsCommand(step, /^gh release (create|edit)\b/),
+  );
+
+  assert.notEqual(checkIndex, -1);
+  assert.notEqual(publishIndex, -1);
+  // Stale notes are already published if the check runs after the release.
+  assert.ok(checkIndex < publishIndex);
+});
+
+test('prerelease workflow keeps tag-derived values out of shell bodies', () => {
+  assert.deepEqual(templateExpressionsInRunBodies(parsedPrereleaseWorkflow), []);
+  assert.deepEqual(stepsMissingEnvDeclaration(parsedPrereleaseWorkflow, 'RELEASE_VERSION'), []);
+});

src/release-workflow.test.ts

@@ -2,11 +2,18 @@ import test from 'node:test';
 import assert from 'node:assert/strict';
 import { readFileSync } from 'node:fs';
 import { resolve } from 'node:path';
+import {
+  readWorkflow,
+  stepsMissingEnvDeclaration,
+  templateExpressionsInRunBodies,
+} from './workflow-test-helpers';
 
 const releaseWorkflowPath = resolve(__dirname, '../.github/workflows/release.yml');
 const releaseWorkflow = readFileSync(releaseWorkflowPath, 'utf8');
 const docsPagesWorkflowPath = resolve(__dirname, '../.github/workflows/docs-pages.yml');
 const docsPagesWorkflow = readFileSync(docsPagesWorkflowPath, 'utf8');
+const parsedReleaseWorkflow = readWorkflow(releaseWorkflowPath);
+const parsedDocsPagesWorkflow = readWorkflow(docsPagesWorkflowPath);
 const makefilePath = resolve(__dirname, '../Makefile');
 const makefile = readFileSync(makefilePath, 'utf8');
 const packageJsonPath = resolve(__dirname, '../package.json');
@@ -249,7 +256,7 @@ test('release workflow publishes subminer-bin to AUR from tagged release artifac
     releaseWorkflow,
     /cp packaging\/aur\/subminer-bin\/\.SRCINFO aur-subminer-bin\/\.SRCINFO/,
   );
-  assert.match(releaseWorkflow, /version_no_v="\$\{\{ steps\.version\.outputs\.VERSION \}\}"/);
+  assert.match(releaseWorkflow, /version_no_v="\$RELEASE_VERSION"/);
   assert.match(releaseWorkflow, /SubMiner-\$\{version_no_v\}\.AppImage/);
   assert.doesNotMatch(
     releaseWorkflow,
@@ -278,3 +285,14 @@ test('Makefile uninstall targets remove bundled runtime plugin app-data copies',
   assert.match(makefile, /Removed:[\s\S]*\$\(LINUX_DATA_DIR\)\/plugin\/subminer/);
   assert.match(makefile, /Removed:[\s\S]*\$\(MACOS_DATA_DIR\)\/plugin\/subminer/);
 });
+
+test('release and docs workflows keep tag-derived values out of shell bodies', () => {
+  assert.deepEqual(templateExpressionsInRunBodies(parsedReleaseWorkflow), []);
+  assert.deepEqual(templateExpressionsInRunBodies(parsedDocsPagesWorkflow), []);
+  assert.deepEqual(stepsMissingEnvDeclaration(parsedReleaseWorkflow, 'RELEASE_VERSION'), []);
+  assert.deepEqual(stepsMissingEnvDeclaration(parsedDocsPagesWorkflow, 'TAG_NAME'), []);
+
+  // The docs tag guard must test the shell variable, not an interpolated value
+  // that would be substituted into the condition before the shell reads it.
+  assert.match(docsPagesWorkflow, /if \[\[ ! "\$TAG_NAME" =~/);
+});

src/workflow-test-helpers.test.ts

@@ -0,0 +1,97 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import {
+  commandPositions,
+  executableRunLines,
+  stepRunsCommand,
+  stepsMissingEnvDeclaration,
+  templateExpressionsInRunBodies,
+} from './workflow-test-helpers';
+
+const runs = (run: string): boolean => stepRunsCommand({ run }, /^bun run verify --flag "\$VALUE"/);
+
+test('stepRunsCommand matches a command that actually executes', () => {
+  assert.equal(runs('bun run verify --flag "$VALUE"'), true);
+  assert.equal(runs('if ! bun run verify --flag "$VALUE"; then\nexit 1\nfi'), true);
+  assert.equal(runs('set -e && bun run verify --flag "$VALUE"'), true);
+  assert.equal(runs('  bun run verify --flag "$VALUE" || exit 1'), true);
+});
+
+test('stepRunsCommand rejects commands that are only mentioned, not run', () => {
+  assert.equal(runs('# bun run verify --flag "$VALUE"'), false);
+  assert.equal(runs('echo \'bun run verify --flag "$VALUE"\''), false);
+  assert.equal(runs("printf '%s\\n' 'bun run verify --flag \"$VALUE\"'"), false);
+  assert.equal(runs('echo "run: bun run verify --flag \\"$VALUE\\"" >> notes.txt'), false);
+  // A different argument list is a different command.
+  assert.equal(runs('bun run verify'), false);
+});
+
+test('stepRunsCommand ignores separators inside quotes and inline comments', () => {
+  assert.equal(runs('echo \'note; bun run verify --flag "$VALUE"\''), false);
+  assert.equal(runs('echo "note && bun run verify --flag \\"$VALUE\\""'), false);
+  assert.equal(runs("printf '%s\\n' 'a | bun run verify --flag \"$VALUE\"'"), false);
+  assert.equal(runs('if false; then # bun run verify --flag "$VALUE"'), false);
+  // A trailing comment does not hide the command in front of it.
+  assert.equal(runs('bun run verify --flag "$VALUE" # keep this'), true);
+  // A pipe is a real separator; a redirect is not.
+  assert.equal(runs('cat notes | bun run verify --flag "$VALUE"'), true);
+  assert.equal(stepRunsCommand({ run: 'gh release view "$V" 2>&1 | tee log' }, /^tee\b/), true);
+});
+
+test('stepRunsCommand treats backslash-escaped separators as literal text', () => {
+  assert.equal(runs(String.raw`echo foo \; bun run verify --flag "$VALUE"`), false);
+  assert.equal(runs(String.raw`echo foo \| bun run verify --flag "$VALUE"`), false);
+  assert.equal(runs(String.raw`find . -exec bun run verify --flag "$VALUE" \;`), false);
+  // An escape does not swallow a following real separator.
+  assert.equal(runs(String.raw`echo a\b; bun run verify --flag "$VALUE"`), true);
+});
+
+test('commandPositions splits on separators and strips control-flow prefixes', () => {
+  assert.deepEqual(
+    commandPositions({ run: 'if gh release view "$V"; then\ngh release edit "$V"\nfi' }),
+    ['gh release view "$V"', 'then', 'gh release edit "$V"', 'fi'],
+  );
+});
+
+test('executableRunLines drops blank and comment-only lines', () => {
+  assert.deepEqual(executableRunLines({ run: '\n# a comment\n  \nreal command\n' }), [
+    'real command',
+  ]);
+});
+
+test('templateExpressionsInRunBodies reports every expression spelling in a run body', () => {
+  const workflow = {
+    jobs: {
+      release: {
+        steps: [
+          { name: 'Safe', env: { V: '${{ steps.version.outputs.VERSION }}' }, run: 'echo "$V"' },
+          { name: 'Dotted', run: 'echo "${{ steps.version.outputs.VERSION }}"' },
+          { name: 'Bracketed', run: 'echo "${{ steps.version.outputs[\'VERSION\'] }}"' },
+          { name: 'Github', run: 'echo "${{ github[\'ref_name\'] }}"' },
+        ],
+      },
+    },
+  };
+
+  assert.deepEqual(templateExpressionsInRunBodies(workflow), [
+    'release/Dotted: ${{ steps.version.outputs.VERSION }}',
+    "release/Bracketed: ${{ steps.version.outputs['VERSION'] }}",
+    "release/Github: ${{ github['ref_name'] }}",
+  ]);
+});
+
+test('stepsMissingEnvDeclaration finds shell reads with no matching env entry', () => {
+  const workflow = {
+    jobs: {
+      release: {
+        steps: [
+          { name: 'Declared', env: { TAG: 'x' }, run: 'echo "$TAG"' },
+          { name: 'Undeclared', run: 'echo "${TAG}"' },
+          { name: 'Unrelated', run: 'echo "$TAGGED"' },
+        ],
+      },
+    },
+  };
+
+  assert.deepEqual(stepsMissingEnvDeclaration(workflow, 'TAG'), ['release/Undeclared']);
+});

src/workflow-test-helpers.ts

@@ -0,0 +1,169 @@
+import { readFileSync } from 'node:fs';
+
+export type WorkflowStep = {
+  name?: string;
+  run?: string;
+  env?: Record<string, unknown>;
+};
+
+export type ParsedWorkflow = {
+  jobs?: Record<string, { steps?: WorkflowStep[] } | undefined>;
+};
+
+// Workflow tests only ever run under `bun test`, which parses YAML natively.
+function parseWorkflowYaml(source: string): ParsedWorkflow {
+  const bunRuntime = globalThis as typeof globalThis & {
+    Bun?: { YAML?: { parse?: (input: string) => unknown } };
+  };
+  const parse = bunRuntime.Bun?.YAML?.parse;
+  if (!parse) {
+    throw new Error('Bun.YAML.parse is unavailable; workflow tests must run under bun.');
+  }
+  return parse(source) as ParsedWorkflow;
+}
+
+export function readWorkflow(workflowPath: string): ParsedWorkflow {
+  return parseWorkflowYaml(readFileSync(workflowPath, 'utf8'));
+}
+
+// Steps of one job, in declaration order. Throws on an unknown job so a renamed
+// job fails loudly instead of silently emptying an ordering assertion.
+export function jobSteps(workflow: ParsedWorkflow, jobName: string): WorkflowStep[] {
+  const job = workflow.jobs?.[jobName];
+  if (!job) {
+    throw new Error(`Workflow has no job named ${jobName}.`);
+  }
+  return job.steps ?? [];
+}
+
+function allSteps(workflow: ParsedWorkflow): Array<{ job: string; step: WorkflowStep }> {
+  return Object.entries(workflow.jobs ?? {}).flatMap(([job, definition]) =>
+    (definition?.steps ?? []).map((step) => ({ job, step })),
+  );
+}
+
+// Lines of a step's shell body that actually execute. Comments are dropped so a
+// commented-out command cannot satisfy a "this step runs X" assertion.
+export function executableRunLines(step: WorkflowStep): string[] {
+  return (typeof step.run === 'string' ? step.run.split('\n') : [])
+    .map((line) => line.trim())
+    .filter((line) => line.length > 0 && !line.startsWith('#'));
+}
+
+// Leading shell keywords and operators that can precede a real command.
+const COMMAND_PREFIX = /^(?:if|elif|while|until|then|else|do|!|&&|\|\||\(|\{)\s+/;
+
+// Splits one shell line on command separators, tracking quotes so a separator
+// inside a string is not treated as a command break, and stopping at an
+// unquoted inline comment.
+function splitCommandSeparators(line: string): string[] {
+  const segments: string[] = [];
+  let current = '';
+  let quote: "'" | '"' | null = null;
+
+  for (let index = 0; index < line.length; index += 1) {
+    const char = line[index]!;
+
+    if (quote) {
+      current += char;
+      if (char === '\\' && quote === '"' && index + 1 < line.length) {
+        current += line[index + 1]!;
+        index += 1;
+      } else if (char === quote) {
+        quote = null;
+      }
+      continue;
+    }
+
+    // An unquoted backslash escapes the next character, so `\;` is literal text
+    // rather than a separator. Checked before comments and separators.
+    if (char === '\\' && index + 1 < line.length) {
+      current += char + line[index + 1]!;
+      index += 1;
+      continue;
+    }
+
+    if (char === "'" || char === '"') {
+      quote = char;
+      current += char;
+      continue;
+    }
+
+    // An unquoted # starts a comment when it opens a word; the rest is inert.
+    if (char === '#' && (current === '' || /\s$/.test(current))) {
+      break;
+    }
+
+    const next = line[index + 1];
+    if (char === ';') {
+      segments.push(current);
+      current = '';
+      continue;
+    }
+    if ((char === '&' || char === '|') && next === char) {
+      segments.push(current);
+      current = '';
+      index += 1;
+      continue;
+    }
+    // A lone pipe separates commands; a redirect such as 2>&1 does not.
+    if (char === '|' && !/[0-9<>&]$/.test(current)) {
+      segments.push(current);
+      current = '';
+      continue;
+    }
+
+    current += char;
+  }
+
+  segments.push(current);
+  return segments;
+}
+
+// Command positions within a step's shell body: each line split on separators,
+// with control-flow prefixes stripped. A pattern anchored with ^ therefore
+// matches only where a command actually starts, so text quoted inside an
+// `echo`/`printf` argument is not mistaken for the command running.
+export function commandPositions(step: WorkflowStep): string[] {
+  return executableRunLines(step).flatMap((line) =>
+    splitCommandSeparators(line)
+      .map((segment) => {
+        let candidate = segment.trim();
+        let stripped = candidate.replace(COMMAND_PREFIX, '');
+        while (stripped !== candidate) {
+          candidate = stripped;
+          stripped = candidate.replace(COMMAND_PREFIX, '');
+        }
+        return candidate;
+      })
+      .filter(Boolean),
+  );
+}
+
+// Whether a step actually executes a command matching the pattern. Anchor the
+// pattern with ^ so it has to match at a command position.
+export function stepRunsCommand(step: WorkflowStep, pattern: RegExp): boolean {
+  return commandPositions(step).some((position) => pattern.test(position));
+}
+
+// GitHub substitutes ${{ }} into a run script before the shell parses it, so any
+// value used that way is executed as script rather than read as data. Reporting
+// every expression (rather than allow-listing known-safe ones) also covers
+// alternate spellings such as ${{ steps.version.outputs['VERSION'] }}.
+export function templateExpressionsInRunBodies(workflow: ParsedWorkflow): string[] {
+  return allSteps(workflow).flatMap(({ job, step }) =>
+    (typeof step.run === 'string' ? (step.run.match(/\$\{\{[\s\S]*?\}\}/g) ?? []) : []).map(
+      (expression) => `${job}/${step.name ?? '<unnamed>'}: ${expression}`,
+    ),
+  );
+}
+
+// Steps whose shell body reads $NAME without the step declaring it in env, which
+// would silently expand to an empty string at run time.
+export function stepsMissingEnvDeclaration(workflow: ParsedWorkflow, name: string): string[] {
+  const reference = new RegExp(`\\$${name}\\b|\\$\\{${name}\\b`);
+  return allSteps(workflow)
+    .filter(({ step }) => typeof step.run === 'string' && reference.test(step.run))
+    .filter(({ step }) => !Object.prototype.hasOwnProperty.call(step.env ?? {}, name))
+    .map(({ job, step }) => `${job}/${step.name ?? '<unnamed>'}`);
+}