fix(3621): release-sdk hotfix cherry-picks test-fixture commits (#3623)

* fix(3621): cherry-pick test-fixture commits in hotfix runs

The release-sdk hotfix loop excluded test-fixture updates that align CI
with a cherry-picked production fix, leaving the hotfix branch with new
production behavior and stale test assertions. Broke v1.42.3 CI (run
25949422676) when fix(3562) was cherry-picked but its bundled test
correction in docs(3562) commit 08848df8 was POLICY_SKIPPED by the
prefix filter.

Two-part fix:

1. release-sdk.yml prefix regex now accepts test: alongside fix:/chore:.
   feat:, docs:, refactor: still POLICY_SKIPPED as before.

2. scripts/diff-touches-shipped-paths.cjs treats tests/-rooted paths and
   sdk/src vitest specs as CI-gating-equivalent. A test: commit touching
   only those paths now passes the shipped-paths gate.

The #2980 push-blocking guard is preserved as a separate first-priority
check: any commit touching .github/workflows/<file> still skips
regardless of test paths in the same bundle, because the default
GITHUB_TOKEN lacks the workflow scope and the push step would fail.

New regression coverage in tests/bug-3621-cherry-pick-test-fixtures.test.cjs:
- workflow prefix regex includes test:
- isCiGating accepts tests/ and sdk/src vitest specs, rejects
  non-spec sdk/src paths and incidental "test" name occurrences
- classifier exits 0 for test-only, mixed test+docs, and the original
  shipped paths
- classifier exits 1 for pure docs-only and workflow-only diffs
- new explicit assertion that #2980 push-blocking wins over #3621:
  workflow + test + changelog bundle still skips

Adjusted one pre-existing bug-2980 test fixture to use a non-
push-blocking non-shipped path (planning/notes.md) instead of
.github/workflows/release-sdk.yml. The original assertion was
documenting "mixed diff includes shipped path → include" but its
fixture happened to also trigger the push-blocking guard now made
explicit by this PR.

Fixes #3621

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(release-sdk): align hotfix summary labels with test matcher

* fix(3621): align operator-facing strings with the fix/chore/test matcher

The candidate-loop regex was updated to accept test: but several
human-facing strings in the same job still read fix/chore. Update every
description/comment/summary line for consistency so operators reading
the run summary or workflow_dispatch inputs see the same set of accepted
prefixes the matcher actually applies.

Also corrected the NON_SHIPPED_SKIPPED summary text — it claimed test
changes belong on main, not in a hotfix. That assumption is what #3621
fixes; tests under tests/ and sdk/src vitest specs are now CI-gating
candidates and may be picked. The summary now scopes the never-pick
guidance to CI / docs / planning paths only.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-15 22:43:21 -04:00
committed by GitHub
parent 79ea076daa
commit 0efe4f0612
5 changed files with 288 additions and 17 deletions

View File

@@ -0,0 +1,5 @@
---
"get-shit-done-cc": patch
---
**Fixed: hotfix releases now correctly cherry-pick test-fixture updates.** The `release-sdk.yml` auto-cherry-pick loop accepts `test:` commits (in addition to `fix:` and `chore:`), and the shipped-paths classifier treats `tests/<name>` and `sdk/src/<name>/<file>.test.<ts|cjs|mjs|js>` as CI-gating-equivalent. When a production fix is bundled with a `fix:` commit but the matching test-fixture alignment lands in a separate `test:` commit, both are now picked together — preventing the CI-red state that broke the v1.42.3 hotfix attempt. The `.github/workflows/<file>` push-blocking guard is preserved (#2980): bundles touching workflow files still skip regardless of other content.

View File

@@ -26,7 +26,7 @@ on:
workflow_dispatch:
inputs:
action:
description: 'publish = normal dev/next/latest publish; hotfix = create hotfix/X.YY.Z branch from latest vX.YY.* tag, cherry-pick fix:/chore: from main, publish to @latest'
description: 'publish = normal dev/next/latest publish; hotfix = create hotfix/X.YY.Z branch from latest vX.YY.* tag, cherry-pick fix:/chore:/test: from main, publish to @latest'
required: true
type: choice
default: publish
@@ -51,7 +51,7 @@ on:
required: false
type: string
auto_cherry_pick:
description: 'Hotfix only: auto-cherry-pick fix:/chore: commits from origin/main since base tag.'
description: 'Hotfix only: auto-cherry-pick fix:/chore:/test: commits from origin/main since base tag.'
required: false
type: boolean
default: true
@@ -74,7 +74,7 @@ jobs:
#
# action=publish → outputs inputs.ref verbatim (may be empty = workflow ref)
# action=hotfix → branches hotfix/X.YY.Z from highest existing vX.YY.* tag,
# auto-cherry-picks fix:/chore: from origin/main, pushes,
# auto-cherry-picks fix:/chore:/test: from origin/main, pushes,
# and outputs the new branch as ref. Idempotent: if branch
# already exists (operator pre-prepared it via hotfix.yml),
# we just check it out and re-run the cherry-pick step
@@ -179,10 +179,10 @@ jobs:
| grep -F -f <(echo "$CANDIDATES") || true)
INCLUDED=""
# POLICY_SKIPPED — commits intentionally not picked because they
# don't match the fix/chore filter (feat/refactor/docs/etc).
# CONFLICT_SKIPPED — fix/chore commits whose cherry-pick failed
# don't match the fix/chore/test filter (feat/refactor/docs/etc).
# CONFLICT_SKIPPED — fix/chore/test commits whose cherry-pick failed
# and were skipped per the full-automation policy (#2968).
# NON_SHIPPED_SKIPPED — fix/chore commits whose diff doesn't
# NON_SHIPPED_SKIPPED — fix/chore/test commits whose diff doesn't
# touch any path in the npm tarball's `files` whitelist
# (CI / test / docs / planning-only changes). They can't
# affect the published package's behavior, so picking them
@@ -199,7 +199,7 @@ jobs:
while IFS= read -r SHA; do
[ -z "$SHA" ] && continue
SUBJECT=$(git log -1 --format='%s' "$SHA")
if echo "$SUBJECT" | grep -qE '^(fix|chore)(\([^)]+\))?!?: '; then
if echo "$SUBJECT" | grep -qE '^(fix|chore|test)(\([^)]+\))?!?: '; then
# Merge commits with fix:/chore: titles can't be cherry-picked
# without `-m <parent>` and we can't pick the parent
# automatically. They fail BEFORE entering cherry-pick state
@@ -358,16 +358,16 @@ jobs:
echo "Base: \`$BASE_TAG\` → Branch: \`$BRANCH\`$([ "$DRY_RUN" = "true" ] && echo " (DRY RUN — local only)")"
echo ""
if [ -n "$INCLUDED" ]; then
echo "### Included (fix/chore)"
echo "### Included (fix/chore/test)"
echo ""
echo "$INCLUDED"
else
echo "_No fix/chore commits to include._"
echo "_No fix/chore/test commits to include._"
fi
if [ -n "$NON_SHIPPED_SKIPPED" ]; then
echo "### Skipped — touches no shipped paths (informational)"
echo ""
echo "These fix/chore commits don't touch any path in the npm tarball's \`files\` whitelist (or \`package.json\`), so they cannot change the published package's behavior. CI / test / docs / planning-only changes belong on \`main\`, not in a hotfix. No action needed."
echo "These fix/chore/test commits don't touch any path the hotfix CI cares about — neither the npm tarball's \`files\` whitelist (or \`package.json\`) nor a CI-gating test under \`tests/\` or \`sdk/src/**\` vitest specs (#3621). CI / docs / planning-only changes belong on \`main\`, not in a hotfix. No action needed."
echo ""
echo "$NON_SHIPPED_SKIPPED"
fi

View File

@@ -12,6 +12,12 @@
* - package.json (always included by `npm pack`, regardless of `files`)
* - every entry in package.json `files`, treated as either an exact
* file match or a directory prefix (matching `npm pack` semantics).
* - CI-gating test paths: `tests/<anything>` plus
* `sdk/src/<anything>/<name>.test.<ts|cjs|mjs|js>` and `.spec.` variants
* — these don't ship in the tarball, but they gate the hotfix-branch
* test job. A test fixture update that aligns with a cherry-picked
* production fix MUST be pickable or CI fails on the hotfix run.
* #3621 — root cause of the v1.42.3 hotfix red CI.
*
* `package-lock.json` is intentionally NOT considered shipped — `npm pack`
* excludes it from the tarball unless it's explicitly in `files`, and at
@@ -48,6 +54,24 @@ function loadShipPrefixes(pkgPath) {
return ['package.json', ...files];
}
// #3621: paths that gate hotfix-branch CI even though they don't appear
// in the npm tarball. When a cherry-picked production fix changes behavior
// that an existing test on the v1.42.2 base asserts against, the matching
// test fixture from `main` must also be cherry-picked or CI fails on the
// hotfix run (exactly what happened on v1.42.3 — production fix(3562) was
// picked, the bundled test-fixture correction in commit 08848df8 was not).
// Combined with the `test:` prefix being added to the candidate-loop regex
// in release-sdk.yml, this lets `test(####):` fixture-alignment commits be
// cherry-picked alongside their production counterparts.
function isCiGating(diffPath) {
if (diffPath.startsWith('tests/')) return true;
// SDK vitest specs live next to source. Production source ships via
// sdk/dist/ (already in package.json `files`); the test files are what's
// missing from that surface.
if (diffPath.startsWith('sdk/src/') && /\.(test|spec)\.(ts|cjs|mjs|js)$/.test(diffPath)) return true;
return false;
}
function isShipped(diffPath, shipPrefixes) {
// Normalize Windows-style separators just in case (git always emits
// forward slashes, but a developer running this locally on a different
@@ -56,6 +80,14 @@ function isShipped(diffPath, shipPrefixes) {
return shipPrefixes.some((s) => p === s || p.startsWith(s + '/'));
}
// #2980: commits that touch `.github/workflows/*` cannot be cherry-picked
// onto a hotfix branch because the default GITHUB_TOKEN lacks the
// `workflow` permission and the push step fails. Detect them upfront so a
// `test:`-eligible commit bundling a workflow edit still gets skipped.
function isPushBlocking(diffPath) {
return diffPath.replace(/\\/g, '/').startsWith('.github/workflows/');
}
function fail(message, err) {
process.stderr.write(`diff-touches-shipped-paths: ${message}\n`);
if (err && err.stack) process.stderr.write(`${err.stack}\n`);
@@ -86,8 +118,22 @@ function main() {
process.stdin.on('end', () => {
try {
const paths = buf.split('\n').map((s) => s.trim()).filter(Boolean);
const hit = paths.some((p) => isShipped(p, shipPrefixes));
process.exit(hit ? EXIT_SHIPPED : EXIT_NOT_SHIPPED);
// #2980 still wins over #3621: any commit touching .github/workflows/*
// is unpickable regardless of other content because the push step
// fails on workflow scope rejection. Check this first.
if (paths.some(isPushBlocking)) {
process.exit(EXIT_NOT_SHIPPED);
}
if (paths.some((p) => isShipped(p, shipPrefixes))) {
process.exit(EXIT_SHIPPED);
}
// #3621: a commit whose only relevant paths are CI-gating tests is
// still pickable — it can change whether the hotfix CI passes even
// though it doesn't change what the npm tarball ships.
if (paths.some(isCiGating)) {
process.exit(EXIT_SHIPPED);
}
process.exit(EXIT_NOT_SHIPPED);
} catch (err) {
fail('classification failed', err);
}
@@ -98,4 +144,4 @@ if (require.main === module) {
main();
}
module.exports = { loadShipPrefixes, isShipped, EXIT_SHIPPED, EXIT_NOT_SHIPPED, EXIT_ERROR };
module.exports = { loadShipPrefixes, isShipped, isCiGating, isPushBlocking, EXIT_SHIPPED, EXIT_NOT_SHIPPED, EXIT_ERROR };

View File

@@ -265,12 +265,15 @@ describe('bug-2980: scripts/diff-touches-shipped-paths.cjs classifier semantics'
});
test('mixed diff is shipped if ANY path is shipped', () => {
// A commit that touches both a shipped file and a non-shipped file
// must be classified as shipped — the non-shipped paths are along
// for the ride, but the commit can still affect what ships.
// A commit that touches both a shipped file and an unrelated non-
// shipped, non-push-blocking file must be classified as shipped — the
// non-shipped paths are along for the ride, but the commit can still
// affect what ships. Note: this fixture deliberately avoids
// `.github/workflows/*`, which is push-blocking (#3621) and forces a
// skip regardless of other shipped paths in the same bundle.
const tmp = makeFixtureRepo(['bin']);
try {
const stdin = '.github/workflows/release-sdk.yml\nbin/foo.js\ntests/bar.test.cjs\n';
const stdin = 'CHANGELOG.md\nbin/foo.js\nplanning/notes.md\n';
assert.equal(runClassifier(stdin, tmp).status, 0, 'mixed diff with at least one shipped path must classify as shipped');
} finally {
fs.rmSync(tmp, { recursive: true, force: true });

View File

@@ -0,0 +1,217 @@
/**
* Regression test for bug #3621
*
* The release-sdk hotfix cherry-pick loop's `fix:`/`chore:` prefix filter
* and the shipped-paths classifier together excluded a test-fixture update
* that was required to align CI with a cherry-picked production fix.
*
* Concrete failure shape (v1.42.3 hotfix, run 25949422676):
* - Commit 36534059 fix(3562) was picked. It changed the installer to
* materialize gsd-named SKILL.md files under the codex skills dir.
* - Commit 08848df8 (docs(3562) prefix) was POLICY_SKIPPED by the prefix
* filter. It bundled the matching test-fixture correction (removing an
* "if (runtime === 'codex') return new Set()" short-circuit in
* tests/install-minimal-all-runtimes.test.cjs).
* - Result: hotfix branch has new production behavior + stale test
* assertion → 3 tests fail.
*
* Two-part fix:
* 1. release-sdk.yml prefix filter now includes test: commits.
* 2. scripts/diff-touches-shipped-paths.cjs treats tests/-rooted paths
* and sdk/src vitest specs (.test.ts / .spec.ts and friends) as
* CI-gating-equivalent so a test: commit that touches only those
* paths passes the shipped-paths gate.
*
* Without both halves the bundling failure mode persists.
*/
'use strict';
// allow-test-rule: source-text-is-the-product
// release-sdk.yml IS the product for hotfix automation; this test reads
// the workflow's prefix-filter regex line directly because the regex IS
// the behavior contract — there is no runtime that consumes it.
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const { spawnSync } = require('node:child_process');
const REPO_ROOT = path.join(__dirname, '..');
const WORKFLOW_PATH = path.join(REPO_ROOT, '.github', 'workflows', 'release-sdk.yml');
const CLASSIFIER_PATH = path.join(REPO_ROOT, 'scripts', 'diff-touches-shipped-paths.cjs');
const { isCiGating } = require('../scripts/diff-touches-shipped-paths.cjs');
function workflowText() {
return fs.readFileSync(WORKFLOW_PATH, 'utf8');
}
function runClassifier(stdin, cwd) {
return spawnSync(process.execPath, [CLASSIFIER_PATH], {
cwd,
input: stdin,
encoding: 'utf8',
});
}
function writeMinimalPkg(tmp, files) {
fs.writeFileSync(
path.join(tmp, 'package.json'),
JSON.stringify({ name: 'test-pkg', version: '0.0.0', files }, null, 2),
);
}
describe('bug-3621: release-sdk hotfix accepts `test:` commits', () => {
test('candidate-loop regex includes test: prefix alongside fix:/chore:', () => {
const text = workflowText();
// The regex must accept all three prefixes — fix, chore, test — with
// optional (scope) and optional ! for breaking. Asserted by extracting
// the literal regex line.
const regexLine = text.split('\n').find((l) => l.includes("grep -qE '^(") && l.includes(')(\\([^)]+\\))?!?: '));
assert.ok(regexLine, 'release-sdk.yml must contain the candidate-prefix regex line');
assert.ok(
/\^\(fix\|chore\|test\)/.test(regexLine),
`prefix regex must include fix|chore|test (got: ${regexLine.trim()})`,
);
});
test('regex does NOT silently drop fix: or chore: while adding test:', () => {
const text = workflowText();
const regexLine = text.split('\n').find((l) => l.includes("grep -qE '^("));
assert.ok(regexLine.includes('fix|chore|test'), 'must keep fix and chore as before');
assert.ok(!regexLine.includes('feat|'), 'must NOT silently add feat: — features are not hotfix-eligible');
assert.ok(!regexLine.includes('docs|'), 'must NOT silently add docs: — docs commits remain POLICY_SKIPPED');
});
});
describe('bug-3621: shipped-paths classifier treats test paths as CI-gating', () => {
test('isCiGating recognizes tests/** as CI-gating', () => {
assert.equal(isCiGating('tests/bug-foo.test.cjs'), true);
assert.equal(isCiGating('tests/helpers.cjs'), true);
assert.equal(isCiGating('tests/fixtures/adversarial/roadmap/duplicate-keys.md'), true);
});
test('isCiGating recognizes sdk/src vitest specs as CI-gating', () => {
assert.equal(isCiGating('sdk/src/query/init.test.ts'), true);
assert.equal(isCiGating('sdk/src/query/init.spec.ts'), true);
assert.equal(isCiGating('sdk/src/golden/golden.integration.test.ts'), true);
});
test('isCiGating rejects non-test sdk/src paths (those ship via sdk/dist)', () => {
assert.equal(isCiGating('sdk/src/query/init.ts'), false);
assert.equal(isCiGating('sdk/src/config.ts'), false);
});
test('isCiGating rejects paths that merely contain "test" in their name', () => {
// Guard against /test/ false positives.
assert.equal(isCiGating('docs/test-strategy.md'), false);
assert.equal(isCiGating('bin/install-test-helper.js'), false);
// .test.md is not a recognized spec extension here.
assert.equal(isCiGating('docs/install.test.md'), false);
});
test('classifier exits 0 for a test-only diff (the v1.42.3 case)', () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'bug-3621-classifier-'));
try {
writeMinimalPkg(tmp, ['bin', 'sdk/dist']);
const result = runClassifier('tests/install-minimal-all-runtimes.test.cjs\n', tmp);
assert.equal(
result.status,
0,
`tests/** path must classify as shipped-equivalent (status=${result.status}, stderr=${result.stderr})`,
);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
test('classifier exits 0 for a mixed test+docs diff (the v1.42.3 commit shape)', () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'bug-3621-classifier-mixed-'));
try {
writeMinimalPkg(tmp, ['bin', 'sdk/dist']);
const stdin = [
'README.md',
'docs/CONFIGURATION.md',
'docs/USER-GUIDE.md',
'tests/install-minimal-all-runtimes.test.cjs',
'tests/installer-migration-install-integration.test.cjs',
].join('\n') + '\n';
const result = runClassifier(stdin, tmp);
assert.equal(
result.status,
0,
`mixed docs+test diff must classify as shipped-equivalent because at least one path is CI-gating (status=${result.status})`,
);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
test('classifier still exits 1 for a pure docs-only diff (no test paths)', () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'bug-3621-classifier-docs-only-'));
try {
writeMinimalPkg(tmp, ['bin', 'sdk/dist']);
const stdin = 'README.md\ndocs/CONFIGURATION.md\n';
const result = runClassifier(stdin, tmp);
assert.equal(
result.status,
1,
`pure docs-only diff must remain NOT_SHIPPED — they aren't CI-gating either (status=${result.status})`,
);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
test('classifier still exits 0 for a normal shipped-path diff (pre-fix behavior preserved)', () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'bug-3621-classifier-shipped-'));
try {
writeMinimalPkg(tmp, ['bin', 'sdk/dist']);
const result = runClassifier('bin/install.js\n', tmp);
assert.equal(result.status, 0, 'shipped-path classification must not regress');
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
test('classifier still exits 1 for CI-only workflow diffs (.github/** is neither shipped nor CI-gating)', () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'bug-3621-classifier-ci-'));
try {
writeMinimalPkg(tmp, ['bin', 'sdk/dist']);
const result = runClassifier('.github/workflows/release-sdk.yml\n', tmp);
assert.equal(
result.status,
1,
'workflow-only changes must remain NOT_SHIPPED — they\'d otherwise fail the push step (default GITHUB_TOKEN lacks workflow scope, #2980)',
);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
test('push-blocking guard wins: workflow + test bundle classifies as NOT_SHIPPED (preserves #2980)', () => {
// The canonical #2980 case is a `fix(release-sdk):` commit touching
// both the workflow file and its regression test. Under #3621 the test
// path alone would otherwise satisfy the CI-gating check; the
// .github/workflows/* push-blocker must still skip the whole commit.
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'bug-3621-classifier-pushblock-'));
try {
writeMinimalPkg(tmp, ['bin', 'sdk/dist']);
const stdin = [
'.github/workflows/release-sdk.yml',
'tests/bug-2980-hotfix-only-picks-shipping-changes.test.cjs',
'CHANGELOG.md',
].join('\n') + '\n';
const result = runClassifier(stdin, tmp);
assert.equal(
result.status,
1,
'#2980 preservation: bundle with .github/workflows/* must skip regardless of test paths in the same commit',
);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
});