Files
msd-core/tests/bug-2964-release-sdk-empty-cherry-pick.test.cjs
Tom Boucher b8d9bd69b2 fix(release-sdk): skip all cherry-pick conflicts in hotfix loop (full automation) (#2970)
* fix(release-sdk): skip all cherry-pick conflicts in hotfix loop

Full-automation policy: any conflict the cherry-pick can't auto-resolve
— context-missing (#2966) or real merge conflict — is now skipped, not
aborted. The hotfix run completes with whatever applies cleanly; the
SKIPPED list in the run summary becomes the operator's post-hoc review
queue.

Surfaced in run 25227493387 (1.39.1 dry-run): commit 0fb992d
("fix(git): add git.base_branch config") produced real conflicts in
config.cjs / ship.md / complete-milestone.md / tests/config.test.cjs.
v1.39.0 was tagged on the feat/hermes-runtime-2841 branch (#2920),
which restructured those files. 0fb992d was authored against the
pre-restructure shape, so cherry-pick can't auto-resolve.

Pre-#2968 behavior: the workflow distinguished context-missing (skip)
from real (abort + push partial + exit 1). Real conflicts blocked every
hotfix from a base tag whose lineage diverged from main — exactly the
v1.39.x situation. The user has called explicitly for full automation:
"this needs to be fully automated, no one is going to sit there and
tag fixes."

Behavior change:
  - Both classification branches now `git cherry-pick --skip` and
    append to SKIPPED with a reason category:
      * "context absent at base" — empty-HEAD markers (#2966)
      * "merge conflict — manual review" — non-empty HEAD (#2968)
  - Removed: `git cherry-pick --abort`, partial-state push,
    "Cherry-pick conflict" GITHUB_STEP_SUMMARY block, `exit 1`.
  - Operator's manual recovery path via `auto_cherry_pick=false`
    remains intact.

Trade-off (acknowledged in #2968): a critical fix can be silently
dropped if no one reviews the SKIPPED list. The release job's
install-smoke + full test suite still runs and would catch any
test-covered regression. Fixes that aren't test-covered could ship
missing — accepted cost of full automation per the issue.

Tests:
  - tests/bug-2968-cherry-pick-skip-on-any-conflict.test.cjs (new) —
    extracts the cherry-pick failure block via bash if/fi nesting walk
    (no raw-text grep) and asserts the abort path is removed, --skip
    is unconditional, and "merge conflict" + "context absent at base"
    annotations both exist.
  - tests/bug-2966-cherry-pick-context-missing.test.cjs (renamed
    describe + first test name) — assertions still valid since the
    classifier survives for skip-reason annotation.
  - tests/bug-2964-release-sdk-empty-cherry-pick.test.cjs — unchanged
    and still green.

Local: `node --test tests/bug-2964-...test.cjs tests/bug-2966-...test.cjs
tests/bug-2968-...test.cjs` → 8/8 pass.
Local: `npm run lint:tests` → 0 violations.

https://claude.ai/code/session_01LApueb9PVs2uSBhsLprVzG

* fix(release-sdk): split cherry-pick conflict skips from policy skips

CodeRabbit flagged on PR #2970 that conflict skips and policy skips
share the SKIPPED bucket. The run summary heading
"Skipped (feat/refactor/etc — not auto-included)" buries manual-review
conflicts (which the operator must triage) under the same list as
intentional policy exclusions (commits that don't match fix/chore by
design and need no action). Operators reviewing the summary can't
distinguish the two without reading every entry.

Split into two variables:
  - POLICY_SKIPPED — feat/refactor/docs/etc filtered out by the
    fix/chore regex (informational, no action needed)
  - CONFLICT_SKIPPED — fix/chore commits whose cherry-pick failed and
    were skipped per the full-automation policy (#2968) (manual
    review queue)

Run summary now emits two sections with distinct headings:
  - "Skipped — cherry-pick conflict (manual review)"
  - "Not auto-included (feat/refactor/docs/etc)"

The new bug-2968 test asserts both buckets are populated correctly:
  - failure path appends to CONFLICT_SKIPPED, not SKIPPED
  - both bucket variables are echoed in the summary
  - both section headings are present

Local: `node --test tests/bug-2964-...test.cjs tests/bug-2966-...test.cjs
tests/bug-2968-...test.cjs` → 9/9 pass.

https://claude.ai/code/session_01LApueb9PVs2uSBhsLprVzG

* fix(release-sdk): handle merge commits and guard cherry-pick --skip

CodeRabbit flagged a real major issue on PR #2970: merge commits with
fix:/chore: titles fail BEFORE entering cherry-pick state because they
need `-m <parent>` to specify the diff base. Without it, the cherry-pick
errors out and CHERRY_PICK_HEAD is never created. The unconditional
`git cherry-pick --skip` call that follows then fails too (no in-progress
cherry-pick to skip), bricking the loop — defeating the full-automation
policy this PR set out to deliver.

Two guards added:

1. Pre-skip merge commits before invoking cherry-pick. The loop checks
   parent count via `git rev-list --parents -n 1 "$SHA"`; if > 1, the
   commit goes straight to CONFLICT_SKIPPED with reason "merge commit —
   manual -m parent selection required". Operator decides which parent
   to keep when reviewing the run summary.

2. Guard `git cherry-pick --skip` with a CHERRY_PICK_HEAD existence
   check. Catches any other failure mode where the cherry-pick aborts
   before entering conflict state (unreadable commit, ref problems,
   etc.) so the loop still continues cleanly.

Also bumped the bug-2964 test's regex slice window from 2000 to 4000
chars so the merge-commit pre-skip block doesn't push the cherry-pick
line out of the test's match range.

Tests added in tests/bug-2968-cherry-pick-skip-on-any-conflict.test.cjs:
  - merge-commit detection: workflow must call
    `git rev-list --parents -n 1 "$SHA"` before cherry-pick and annotate
    skips with the distinct "manual -m parent selection required"
    reason.
  - guard: failure block must check CHERRY_PICK_HEAD before --skip.

Local: `node --test tests/bug-2964-...test.cjs tests/bug-2966-...test.cjs
tests/bug-2968-...test.cjs` → 11/11 pass.

https://claude.ai/code/session_01LApueb9PVs2uSBhsLprVzG

* fix(release-sdk): guard awk classifier against degenerate unmerged paths

CodeRabbit raised two issues on PR #2970:

1. Major (workflow): the `awk` classifier runs under `set -euo pipefail`.
   If a CONFLICTED path is missing/unreadable, awk exits non-zero and
   terminates the entire step — bricking the loop on a degenerate file.
   Also, an unmerged path with no `<<<<<<< ` markers (path-level conflict
   or anomalous git state) was misclassified as "context absent at base"
   (the auto-skip path), letting potentially-real conflicts skip silently.

   Fix: before invoking awk, check `[ ! -r "$CONFLICTED" ]` and
   `grep -q '^<<<<<<< ' "$CONFLICTED"`. Either failure marks
   ALL_EMPTY_HEAD=false → REASON falls through to "merge conflict —
   manual review", landing the pick in the operator review queue.
   Also added `2>/dev/null || echo "real"` on the awk call so a
   transient awk failure can't slip into the auto-skip bucket.

2. Nitpick (tests): regex assertions on `failureBlock` could match
   commented lines (e.g. comment text mentioning "CONFLICT_SKIPPED"
   or "git cherry-pick --skip" satisfied the assertions without the
   real command being present).

   Fix: anchor with `^\s*...` + `m` flag so only executable shell lines
   count.

Plus a new test asserting all three workflow guards
(`[ ! -r "$CONFLICTED" ]`, `grep -q '^<<<<<<< '`, `awk ... || echo
"real"`) are present in the failure block.

Local: `node --test tests/bug-2964-...test.cjs tests/bug-2966-...test.cjs
tests/bug-2968-...test.cjs` → 12/12 pass.

https://claude.ai/code/session_01LApueb9PVs2uSBhsLprVzG

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-05-01 15:15:20 -04:00

143 lines
7.1 KiB
JavaScript

/**
* Regression test for bug #2964
*
* The release-sdk hotfix workflow's auto_cherry_pick loop aborted the entire
* run if any commit between the base tag and origin/main had an empty diff
* against its parent (e.g. a squash-merge whose contents were already merged
* via an earlier PR). `git cherry-pick -x` exits non-zero on empty commits
* with "The previous cherry-pick is now empty", and the workflow's loop
* (`if ! git cherry-pick -x "$SHA"; then ... exit 1`) treated any non-zero
* as a hard conflict — bricking every hotfix the moment a no-op commit
* landed on main.
*
* Fix: pass `--allow-empty --keep-redundant-commits` so empty picks are
* preserved on the hotfix branch (with `-x` provenance, matching main 1:1)
* and picks whose diff resolves to empty after applying to the new base
* also pass cleanly. Real conflicts still surface — the flags only change
* the empty-commit exit code.
*
* This test asserts both:
* 1. Static — the workflow YAML carries the flags on the cherry-pick call
* inside the auto_cherry_pick loop. If a future edit drops them, this
* regresses immediately.
* 2. Behavioral — `git cherry-pick -x --allow-empty --keep-redundant-commits`
* against a real empty commit in a throwaway repo exits 0 (proves the
* flags semantically do what we claim), while plain `git cherry-pick -x`
* exits non-zero against the same commit (proves the bug exists without
* the flags).
*/
'use strict';
// allow-test-rule: source-text-is-the-product
// The release-sdk.yml workflow IS the product for hotfix automation —
// GitHub Actions executes the YAML's shell verbatim. Testing the text
// content tests the deployed contract: if the flags are absent, the
// empty-commit guarantee is absent.
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 WORKFLOW_PATH = path.join(__dirname, '..', '.github', 'workflows', 'release-sdk.yml');
function git(cwd, args, env = {}) {
// Force-disable signing inline so a developer's global gpgsign / sshsign
// config can't fail commits in this throwaway repo. Don't rely on env
// because gpg.format/user.signingkey live in gitconfig, not env vars.
const signingOff = ['-c', 'commit.gpgsign=false', '-c', 'tag.gpgsign=false', '-c', 'gpg.format=openpgp', '-c', 'user.signingkey='];
return spawnSync('git', [...signingOff, ...args], {
cwd,
encoding: 'utf8',
env: { ...process.env, ...env, GIT_AUTHOR_NAME: 'test', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 'test', GIT_COMMITTER_EMAIL: 't@t' },
});
}
describe('bug-2964: release-sdk hotfix cherry-pick survives empty commits', () => {
test('release-sdk.yml passes --allow-empty --keep-redundant-commits in the auto_cherry_pick loop', () => {
const yaml = fs.readFileSync(WORKFLOW_PATH, 'utf8');
// Find the auto_cherry_pick block by anchoring on a line unique to it,
// then assert the cherry-pick invocation inside that block carries both
// flags. We deliberately scope to the loop — a stray `git cherry-pick`
// elsewhere in the file (none today) would not satisfy this contract.
const loopAnchor = yaml.indexOf('CANDIDATES=$(git cherry HEAD origin/main');
assert.ok(
loopAnchor !== -1,
'release-sdk.yml must contain the auto_cherry_pick loop that derives candidates via `git cherry HEAD origin/main` (#2964)'
);
// The cherry-pick call lives within the auto_cherry_pick loop. Bound
// the slice to ~4 KB after the anchor — generous enough that future
// pre-skip guards / classification scaffolding (e.g. the merge-commit
// pre-skip added on PR #2970) don't push the call out of range, but
// still tight enough to avoid matching unrelated cherry-pick refs
// elsewhere in the workflow file.
// Allow arbitrary git options between `git` and `cherry-pick` (e.g.
// `git -c merge.conflictStyle=merge cherry-pick ...` added for #2966)
// so this test doesn't false-fail on legitimate option additions.
const window = yaml.slice(loopAnchor, loopAnchor + 4000);
const pickMatch = /git\b[^\n]*?cherry-pick[^\n]*"\$SHA"/.exec(window);
assert.ok(
pickMatch,
'auto_cherry_pick loop must invoke `git ... cherry-pick ... "$SHA"` (#2964)'
);
const pickLine = pickMatch[0];
assert.ok(
pickLine.includes('--allow-empty'),
`auto_cherry_pick must pass --allow-empty so empty no-op commits on main do not abort the hotfix (#2964). Found: ${pickLine}`
);
assert.ok(
pickLine.includes('--keep-redundant-commits'),
`auto_cherry_pick must pass --keep-redundant-commits so commits whose diff resolves to empty after rebasing onto the base tag do not abort the hotfix (#2964). Found: ${pickLine}`
);
});
test('git cherry-pick with --allow-empty --keep-redundant-commits succeeds on an empty commit; without them it fails', () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'bug-2964-'));
try {
// Build a synthetic repo with one real commit on main and one truly
// empty commit on top — same shape as the real upstream artifact
// (b328f326 on origin/main has tree == its parent's tree).
assert.equal(git(tmp, ['init', '-q', '-b', 'main']).status, 0, 'git init');
fs.writeFileSync(path.join(tmp, 'README.md'), 'base\n');
assert.equal(git(tmp, ['add', 'README.md']).status, 0, 'git add');
assert.equal(git(tmp, ['commit', '-q', '-m', 'base']).status, 0, 'base commit');
assert.equal(git(tmp, ['tag', 'v0.0.0']).status, 0, 'tag base');
// Make a genuinely empty commit on main.
assert.equal(git(tmp, ['commit', '--allow-empty', '-q', '-m', 'fix: noop on main']).status, 0, 'empty commit');
const empty = git(tmp, ['rev-parse', 'HEAD']).stdout.trim();
assert.ok(empty.length === 40, `expected sha, got ${empty}`);
// Reset to the base tag (simulates the hotfix branch starting from v0.0.0).
assert.equal(git(tmp, ['checkout', '-q', '-b', 'hotfix/0.0.1', 'v0.0.0']).status, 0, 'checkout hotfix');
// Without the flags: cherry-pick of an empty commit fails.
const without = git(tmp, ['cherry-pick', '-x', empty]);
assert.notEqual(
without.status,
0,
'plain `git cherry-pick -x` MUST fail on an empty commit — if this passes, git semantics changed and the bug premise is gone (#2964)'
);
// Reset cherry-pick state for the next run.
git(tmp, ['cherry-pick', '--abort']);
// git may have already auto-resolved to a clean state; ensure we're back to v0.0.0.
git(tmp, ['reset', '--hard', 'v0.0.0']);
// With the flags (matching what the workflow now uses): success.
const withFlags = git(tmp, ['cherry-pick', '-x', '--allow-empty', '--keep-redundant-commits', empty]);
assert.equal(
withFlags.status,
0,
`git cherry-pick -x --allow-empty --keep-redundant-commits MUST succeed on an empty commit (#2964). stderr: ${withFlags.stderr}`
);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
});