7424271aa062ee6d7447dfb4b1d3b194e81084a7
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Tom Boucher
|
7424271aa0 |
fix(#2980): hotfix cherry-pick only picks commits that change what ships (#2981)
* fix(#2980): pre-skip workflow-file cherry-picks in release-sdk hotfix loop The default GITHUB_TOKEN issued to the release-sdk run lacks the `workflow` scope, so the prepare job's `git push origin "$BRANCH"` is rejected by GitHub when any cherry-picked commit modifies a file under `.github/workflows/`: ! [remote rejected] hotfix/X.YY.Z -> hotfix/X.YY.Z (refusing to allow a GitHub App to create or update workflow ... without `workflows` permission) Pre-#2980 behavior: the auto_cherry_pick loop happily picked workflow-file commits, then the trailing push exploded with no clear signal which commit was the culprit. v1.39.1 hit this on PR #2977 (run 25232010071) — earlier release-sdk fixes (#2965, #2967, #2970) had been skipped on conflict so their workflow-file changes never reached the push step, masking the bug; #2977 was the first workflow-file commit to apply cleanly and the push immediately exploded. Fix: pre-pick guard in the cherry-pick loop. Inspect each candidate commit's file list via `git diff-tree --no-commit-id --name-only -r` BEFORE attempting the pick. If any path matches `^\.github/workflows/`, skip the commit, emit a `::warning::` annotation naming the dropped commit, and append to a new `WORKFLOW_SKIPPED` bucket. The run summary surfaces this bucket in its own section, distinct from `CONFLICT_SKIPPED` (real merge conflicts) and `POLICY_SKIPPED` (feat/refactor exclusions), so operators reviewing the run never confuse the remediation paths. The loud-warning piece is non-negotiable: silent drops were explicitly rejected as a failure mode during the option-1/2/3 tradeoff discussion. If a workflow-file fix genuinely needs to ship in a hotfix, the operator applies it manually on the hotfix branch using a token with `workflow` scope, or lands it on main and re-cuts the release. Regression covered by tests/bug-2980-skip-workflow-file-cherrypicks.test.cjs (5 assertions: pre-pick guard exists, uses `git diff-tree`, emits `::warning::`, lands in dedicated bucket, surfaces in summary). The bug-2964 test's 4 KB window after the cherry-pick-loop anchor was nudged to 6 KB to accommodate the new pre-pick scaffolding — the test's own comment had already anticipated this kind of growth (citing #2970's merge-commit pre-skip as prior precedent). Closes #2980 * refactor(#2980): replace workflow-file pre-skip with shipped-paths filter The previous commit on this branch caught only the .github/workflows/* subset of the bug, treating the symptom (push rejection on workflow-file changes) rather than the root cause (the fix:/chore: filter is too broad — it picks any commit with that conventional-commit type even when the diff cannot affect the published npm package). CI-only fixes (release-sdk.yml itself, hotfix tooling, test-only commits) shouldn't flow through hotfix runs at all — they cannot change what `npm install get-shit-done-cc@X.YY.Z` produces. The .github/workflows/* push rejection is just the loudest of these "shouldn't have been picked" cases; tests/, docs/, .planning/ commits get picked silently with the same lack of effect on consumers. Replace the workflow-file pre-skip with a shipped-paths filter: - New scripts/diff-touches-shipped-paths.cjs reads package.json `files`, plus package.json itself (always-shipped per `npm pack` semantics), and exits 0 iff any input path is in the shipped set. Lockfile is not shipped (npm pack excludes it unless explicitly in `files`). - Workflow loop now pipes `git diff-tree --no-commit-id --name-only -r` through the classifier; on exit 1 the commit is skipped and appended to a new NON_SHIPPED_SKIPPED bucket (replaces WORKFLOW_SKIPPED). - Run summary surfaces NON_SHIPPED_SKIPPED as informational — no ::warning:: annotation. A non-shipping commit cannot affect the package, so a yellow alert would imply remediation is possible and would mislead operators. The classifier in a separate .cjs file (rather than inline bash heredoc) is so its rules — directory-prefix vs exact-match, package.json-always-shipped, lockfile-not-shipped — are unit-testable in tests/bug-2980-hotfix-only-picks-shipping-changes.test.cjs (11 new assertions: 4 static workflow + 6 classifier behavioral + 1 mixed- diff edge case). Why this dissolves the original push-rejection bug: workflow files aren't in `files`, so workflow-only commits are skipped pre-pick. The push step never sees them. If a workflow-file fix genuinely needs to ship in a hotfix release (extremely rare — the hotfix workflow is read from main's ref, not the hotfix branch's), the operator applies it manually using a token with `workflow` scope. The pre-skip puts that requirement in the run summary explicitly. Closes #2980 |
||
|
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 |
||
|
Tom Boucher
|
0d25ef0c47 |
fix(release-sdk): skip cherry-picks whose target context is absent at base (#2967)
* fix(release-sdk): skip cherry-picks whose target context is absent at base When auto_cherry_pick processed a fix:/chore: commit whose patch modified code that didn't exist at the hotfix base tag — typically because the surrounding infrastructure was added later in a feat/refactor commit excluded by the filter — `git cherry-pick` failed with a conflict that no operator could meaningfully resolve, and the loop bricked the run. Discovered re-running the 1.39.1 dry-run after #2965 merged: cherry-pick of `a3467792` (the #2965 merge itself) failed because the auto_cherry_pick block it modifies was added in #2956 ("Add automated cherry-pick + SDK- bundle parity to hotfix flow") — an Add/feat commit, so the fix/chore filter excludes it. v1.39.0 has no such block, so the patch had no anchor. The conflict is unmistakably distinguishable from a real content conflict: git emits marker blocks where every `<<<<<<< HEAD ... =======` HEAD section is empty (no anchor lines to reconcile against), while real conflicts have content on both sides. After cherry-pick fails: 1. List unmerged paths via `git diff --diff-filter=U`. 2. For each, scan conflict markers with awk. If every HEAD section is blank/whitespace-only across every block, classify as context-missing. 3. Context-missing → `git cherry-pick --skip` and append to SKIPPED list with reason "(context absent at base)". 4. Otherwise fall through to the existing abort/push-partial/error path that surfaces the conflict for operator resolution. Real conflicts still surface with the same workflow as before. Tests in tests/bug-2966-cherry-pick-context-missing.test.cjs cover: - Static — extracts the "Prepare hotfix branch" run block via indentation-aware YAML parsing (no raw-text grep) and asserts the classification predicate, --skip call, and skipped-reason annotation are present. - Behavioral — synthetic repo reproducing the real shape of the failure, asserts cherry-pick exits non-zero and produces the empty-HEAD marker shape. - Predicate — pulls the awk script out of the deployed workflow and feeds it sample conflict shapes (empty-HEAD, real, mixed, whitespace-only); asserts each is classified as the workflow will behave. Local: `node --test tests/bug-2966-...test.cjs` → 3/3 pass. Local: `npm run lint:tests` → 0 violations. https://claude.ai/code/session_01LApueb9PVs2uSBhsLprVzG * fix(release-sdk): pin merge.conflictStyle=merge on hotfix cherry-pick CodeRabbit flagged on #2967 that the awk classifier introduced for #2966 assumes default conflict-marker style (plain `<<<<<<< HEAD ... ======= ... >>>>>>>`). If a runner has merge.conflictStyle=diff3 or zdiff3 set (globally, repo-config, or via git defaults shift), the marker emits an extra `||||||| ancestor` section between HEAD and =======. The awk's `in_head` mode would accumulate that ancestor content into the HEAD buffer, and a context-missing conflict would misclassify as real — sending the workflow into the abort path on a pick that should be silently skipped. Pass `-c merge.conflictStyle=merge` on the cherry-pick command itself (scoped to that one git invocation; doesn't leak to other commands). This guarantees marker shape regardless of the runner's git config. Updated the existing static assertion in tests/bug-2966-cherry-pick-context-missing.test.cjs to require the pin — a future edit dropping it fails the test. Local: `node --test tests/bug-2966-...test.cjs` → 3/3 pass. https://claude.ai/code/session_01LApueb9PVs2uSBhsLprVzG * test(#2964): allow git options between `git` and `cherry-pick` The previous commit on this branch (d6530190) added `git -c merge.conflictStyle=merge cherry-pick ...` to release-sdk.yml. The bug-2964 static test's regex `/git cherry-pick[^\n]*"\$SHA"/` required `cherry-pick` to be the literal next token after `git`, so it no longer matched the line and CI failed on Node 22 / Node 24 / macOS. Loosen to `/git\b[^\n]*?cherry-pick[^\n]*"\$SHA"/` so any options between `git` and `cherry-pick` (e.g. `-c key=value`) are tolerated. The flag assertions on the matched line still verify --allow-empty and --keep-redundant-commits are present, which is what bug-2964 actually guards. Local: `node --test tests/bug-2964-...test.cjs tests/bug-2966-...test.cjs` → 5/5 pass. https://claude.ai/code/session_01LApueb9PVs2uSBhsLprVzG * test(#2966): pin merge.conflictStyle in test git wrapper, assert awk status CodeRabbit raised two issues on PR #2967: 1. The synthetic-repo cherry-pick reproducer asserted `<<<<<<< HEAD ...` blocks have empty HEAD sections, but the cherry-pick itself didn't pin `merge.conflictStyle`. A developer or CI runner with global diff3/zdiff3 config would inject `||||||| ancestor` lines into the HEAD scan and the test would fail for environment reasons rather than the bug premise. Pin the style on the test's `git()` wrapper so every git operation in the test is deterministic regardless of user config. 2. `classify()` ran awk and consumed `r.stdout.trim()` without checking `r.status` or `r.error`. A failed awk invocation (missing binary, syntax error, signal) returns empty stdout, which would falsely classify as "context-missing" and the test would silently pass on broken predicates. Add `assert.ok(!r.error, ...)` and `assert.equal(r.status, 0, ...)` before reading stdout. Local: `node --test tests/bug-2966-...test.cjs tests/bug-2964-...test.cjs` → 5/5 pass. https://claude.ai/code/session_01LApueb9PVs2uSBhsLprVzG --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Tom Boucher
|
a346779213 | fix(release-sdk): allow empty/redundant commits during hotfix cherry-pick (#2965) |