From 1de3d2e109c6a09a3ade0f5266a2b8f96ba45a61 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 08:33:47 +0000 Subject: [PATCH] feat: detect externally squash-merged slots at harvest preview Closes a documented sharp edge from the plan's deferred list. A clean slot with no ancestry trail into base but whose content a merge would not change (proven via git merge-tree --write-tree tree containment, git >= 2.38) is reported as squash_merged and marked merged, instead of conflicting when the user re-merges work they already squashed in. Conflicts and older gits fall through to the previous behavior. Prune deliberately still keeps such branches: its ancestry test cannot see squashes, and deleting on a weaker signal is not its contract. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LSffk4LorDDDit18ENESsj --- CHANGELOG.md | 7 +++++++ README.md | 11 ++++++++--- bin/renderer.mjs | 8 +++++--- scripts/harvest-step.sh | 26 ++++++++++++++++++++++++++ tests/harvest.test.mjs | 38 ++++++++++++++++++++++++++++++++++++++ tests/renderer.test.mjs | 8 ++++---- 6 files changed, 88 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20c9a62..3e98d9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ Notable changes to the Swarm plugin. Format follows ## [Unreleased] ### Added +- Harvest preview now detects externally **squash-merged** (and + cherry-pick-merged) slots: a clean slot whose content is already fully + contained in base is marked merged (`squash_merged`) instead of + conflicting on re-merge. Uses `git merge-tree --write-tree` containment + (git >= 2.38); older gits keep the previous skip-by-hand behavior. Prune + still deliberately keeps such branches — its ancestry test cannot see + squashes. - CI now runs the full test suite on macOS as well as Linux, and gates every script with shellcheck at default severity. - Test coverage for the last two recorded gaps: archiving a slot whose agent diff --git a/README.md b/README.md index f5a8b8d..5f7c5a3 100644 --- a/README.md +++ b/README.md @@ -305,9 +305,14 @@ printed during dependency install stays on disk until you delete the log. ## Sharp edges -- **Squash merges are invisible.** A slot you squash-merged yourself still - shows as pending and will conflict on re-merge — skip it by hand. Fast- - forward and plain external merges *are* auto-detected via ancestry. +- **Squash merges are detected, but prune still keeps their branches.** A + clean slot whose content is already fully contained in base (you squash- or + cherry-pick-merged it yourself) is auto-detected via `git merge-tree` + containment (git >= 2.38; older gits fall back to the old behavior: skip it + by hand) and marked merged instead of conflicting on re-merge. Fast-forward + and plain external merges are auto-detected via ancestry. Prune's ancestry + test cannot see squashes, so a squash-merged branch is deliberately kept as + "unmerged" — delete it by hand once you're sure. - **Ignored files are not in commits or discard snapshots.** Every slot or detached-harvest worktree removal recursively inventories them and keeps the resource by default. Deletion requires the exact one-use preview approval; diff --git a/bin/renderer.mjs b/bin/renderer.mjs index ad40599..fc780ef 100644 --- a/bin/renderer.mjs +++ b/bin/renderer.mjs @@ -919,15 +919,17 @@ export class HarvestRenderer { await this.doMerge(slot); } } else if ( - // "empty" (auto-skipped: nothing past the fork) and "external_merged" - // (the user merged it themselves) are terminal too — the preview verb + // "empty" (auto-skipped: nothing past the fork), "external_merged" + // (the user merged it themselves), and "squash_merged" (content in + // base with no ancestry trail) are terminal too — the preview verb // already wrote skipped/merged to the manifest. Omitting them left the // only route to archiving those slots a manual re-preview. p.state === "merged" || p.state === "skipped" || p.state === "failed" || p.state === "empty" || - p.state === "external_merged" + p.state === "external_merged" || + p.state === "squash_merged" ) { await this.doArchive(slot); await this.reload(); diff --git a/scripts/harvest-step.sh b/scripts/harvest-step.sh index 353e8cd..41893ff 100755 --- a/scripts/harvest-step.sh +++ b/scripts/harvest-step.sh @@ -306,6 +306,20 @@ swap_base() { printf 'merged\t%s\n' "$new" } +# A squash- or cherry-pick-merged slot leaves no ancestry trail, but its +# CONTENT is already in base: a real merge of tip into base would change +# nothing. `git merge-tree --write-tree` (git >= 2.38) proves that without +# touching any worktree — the merged tree equals base's own tree exactly when +# the slot has nothing left to contribute. Conflicts and older gits exit +# nonzero here, which callers treat as "not proven": those slots fall through +# to the normal clean flow and the documented sharp edge still applies. +squash_merged_into_base() { + local tip="$1" base="$2" merged + merged="$(git -C "$REPO_ROOT" merge-tree --write-tree "$base" "$tip" 2>/dev/null)" || return 1 + [ "$(git -C "$REPO_ROOT" rev-parse --verify --quiet "$merged^{tree}" 2>/dev/null)" = \ + "$(git -C "$REPO_ROOT" rev-parse "$base^{tree}")" ] +} + require_slot_arg() { case "${1-}" in '' | *[!0-9]*) @@ -368,6 +382,18 @@ do_preview() { _preview_report_idle return 0 fi + if [ "$n" -eq 0 ] && [ "$tip" != "$FORK_SHA" ] && + squash_merged_into_base "$tip" "$base"; then + # The user squash- or cherry-pick-merged this slot themselves: no + # ancestry, but a merge would change nothing. Detected, not re-merged — + # re-merging a squashed slot is exactly the conflict the sharp edge + # used to warn about. Prune still keeps the branch (its ancestry test + # cannot see squashes); that remains the conservative call. + printf 'state\tsquash_merged\n' + manifest_update_slot "$1" '{"status":"merged"}' || return 1 + _preview_report_idle + return 0 + fi if [ "$n" -gt 0 ]; then printf 'state\tdirty\n' else diff --git a/tests/harvest.test.mjs b/tests/harvest.test.mjs index a786e79..e3f9b1a 100644 --- a/tests/harvest.test.mjs +++ b/tests/harvest.test.mjs @@ -1719,3 +1719,41 @@ exit 0`, ); h.writeHerdrStub(); // restore the default stub for later tests }); + +// ---- Squash-merge detection (deferred follow-up, now shipped) --------------- +// A squash-merged slot has no ancestry trail, so the external_merged check +// cannot see it; merge-tree containment can. + +test("preview detects an externally squash-merged slot and marks it merged", () => { + h.writeHerdrStub(); + const run = mkRun(); + commitIn(run.wt(1), "feat.txt", "squashed content\n", "slot work"); + // The user squash-merges the slot themselves: content lands, ancestry + // does not. + h.git(run.repo, "merge", "--squash", run.branch(1)); + h.git(run.repo, "commit", "-q", "-m", "squash of s1"); + const r = step(run, "preview", [1]); + assert.equal(r.status, 0, `${r.stdout}\n${r.stderr}`); + assert.match(r.stdout, /state\tsquash_merged/); + assert.equal(run.slotRow(1).status, "merged"); + // Never re-merged: base tip is still the user's own squash commit. + assert.match( + h.git(run.repo, "log", "-1", "--format=%s", "main").stdout, + /squash of s1/, + ); +}); + +test("preview never squash-detects a slot that still has unlanded commits", () => { + h.writeHerdrStub(); + const run = mkRun(); + commitIn(run.wt(1), "first.txt", "landed\n", "landed work"); + h.git(run.repo, "merge", "--squash", run.branch(1)); + h.git(run.repo, "commit", "-q", "-m", "squash of first half"); + // New committed work after the squash: the slot still has something to + // contribute, so it must stay an ordinary clean slot. + commitIn(run.wt(1), "second.txt", "not landed\n", "later work"); + const r = step(run, "preview", [1]); + assert.equal(r.status, 0, `${r.stdout}\n${r.stderr}`); + assert.match(r.stdout, /state\tclean/); + assert.equal(run.slotRow(1).status, "running", "not marked merged"); +}); diff --git a/tests/renderer.test.mjs b/tests/renderer.test.mjs index 81d6e36..0f5a5e2 100644 --- a/tests/renderer.test.mjs +++ b/tests/renderer.test.mjs @@ -705,10 +705,10 @@ test("the resume queue hands off to the stale queue instead of dropping it", asy assert.deepEqual(r.phase.slots, [4]); }); -test("terminal preview states — empty and external_merged included — archive", async () => { - // The preview verb already wrote skipped/merged to the manifest for these - // two; without them the only route to archiving was a manual re-preview. - for (const state of ["merged", "skipped", "failed", "empty", "external_merged"]) { +test("terminal preview states — empty, external_merged, squash_merged included — archive", async () => { + // The preview verb already wrote skipped/merged to the manifest for + // these; without them the only route to archiving was a manual re-preview. + for (const state of ["merged", "skipped", "failed", "empty", "external_merged", "squash_merged"]) { const r = mkHarvest(); r.rows = [ { slot: 1, label: "s1", branch: "b", status: "merged", preview: { state, dirty: 0 } },