Skip to content

Commit 1876184

Browse files
henrikjeclaude
andcommitted
feat(extract): enforce merge-point floor and remove --after-merge
Remove the --after-merge flag from arb extract. Users now specify split points explicitly with --starting-with, which is clearer about what gets extracted. The first post-merge commit SHA is visible in arb status -v and arb log. Add merge-point floor validation: in repos where the branch has been merged into its base, split points below the merge point are rejected with a clear error. This closes an asymmetry where extract allowed grabbing pre-merge commits that the integrate/rebase path would never replay. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1ccc38f commit 1876184

9 files changed

Lines changed: 177 additions & 43 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# Extract: enforce merge-point floor and remove --after-merge
2+
3+
Date: 2026-03-28
4+
5+
## Context
6+
7+
`arb extract` splits a branch at a boundary commit, creating a new stacked workspace. The command validates that split points are above the merge-base (the point where the branch diverged from its base). However, when a branch has been merged into its base (e.g., via squash-merge on the remote) and the user continued committing, the pre-merge commits are already represented on the base. The merge-base floor check does not catch this — it only checks against the diverge point, not the merge point.
8+
9+
Meanwhile, the integrate/rebase path (`classify-integrate.ts`) correctly limits replay to only `newCommitsAfter` commits, effectively using the merge point as its floor. This created an asymmetry: extract allowed grabbing commits that rebase would never replay.
10+
11+
Separately, `--after-merge` auto-detected the merge boundary and extracted post-merge commits. Since the merge point is the natural lower bound for valid extraction, `--after-merge` was equivalent to "start at the lowest valid split point" — a convenience, but not a primitive. Users can look up the first post-merge commit from `arb status -v` or `arb log` and use `--starting-with` explicitly, which is clearer about what gets extracted.
12+
13+
## Options
14+
15+
### Tighten bounds only (keep --after-merge)
16+
Fix the validation gap by rejecting split points below the merge point. Keep `--after-merge` as a convenience for auto-detecting the boundary.
17+
- **Pros:** Fixes the bug. Non-breaking.
18+
- **Cons:** `--after-merge` is now just "use the lowest valid split point" — a thin convenience over `--starting-with`.
19+
20+
### Tighten bounds and remove --after-merge
21+
Fix the validation and remove the `--after-merge` flag. Users specify split points explicitly.
22+
- **Pros:** Simpler command surface. Extract requires users to be explicit about what they're splitting. No implicit-and-possible-to-misinterpret behavior.
23+
- **Cons:** Slightly more steps for the post-merge extraction workflow.
24+
25+
## Decision
26+
27+
Tighten the merge-point floor validation and remove `--after-merge`. The flag was a convenience for a specific scenario, and the explicit `--starting-with` workflow is more transparent. Pre-release status makes this a clean time to simplify the command surface.
28+
29+
## Reasoning
30+
31+
The whole point of extract is to choose where to split a branch — making the user specify the split point explicitly is consistent with this intent. `--after-merge` obscured which commits were being moved. With tighter bound validation, invalid split points produce clear errors explaining why, guiding users toward valid choices.
32+
33+
The integrate/rebase path already enforced the merge-point floor implicitly by limiting replay. Making extract consistent with this boundary closes the asymmetry.
34+
35+
## Consequences
36+
37+
- Users who relied on `--after-merge` must now identify the first post-merge commit SHA manually. `arb status -v` and `arb log` both surface this information.
38+
- The `arb push` hint for merged-new-work repos now suggests `arb extract <workspace> --starting-with <sha>` instead of `--after-merge`.
39+
- Shell completions and help topics are updated to remove the flag.
40+
- New `below-merge-point` skip flag blocks extraction when a split point falls before the merge point.

docs/stacked-branches.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,13 @@ This extracts everything from commit `abc123` through the tip into a new workspa
9797

9898
### Post-merge continuation
9999

100-
If your branch was merged but you kept committing:
100+
If your branch was merged but you kept committing, use `arb log` or `arb status -v` to find the first post-merge commit SHA, then extract from there:
101101

102102
```bash
103-
arb extract next-feature --after-merge --yes
103+
arb extract next-feature --starting-with <first-post-merge-sha> --yes
104104
```
105105

106-
This auto-detects the merge point and extracts the post-merge commits into a new workspace.
106+
Split points in base-merged workspaces must be at or after the merge point — pre-merge commits are already on the default branch.
107107

108108
### Split points across repos
109109

shell/arb.bash

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ __arb_complete_merge() {
536536
__arb_complete_extract() {
537537
local base_dir="$1" cur="$2"
538538
if [[ "$cur" == -* ]]; then
539-
COMPREPLY=($(compgen -W "--ending-with --starting-with --after-merge -b --branch --fetch -N --no-fetch -y --yes --dry-run -v --verbose --autostash --include-wrong-branch --continue --abort" -- "$cur"))
539+
COMPREPLY=($(compgen -W "--ending-with --starting-with -b --branch --fetch -N --no-fetch -y --yes --dry-run -v --verbose --autostash --include-wrong-branch --continue --abort" -- "$cur"))
540540
return
541541
fi
542542
}

shell/arb.zsh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,6 @@ _arb() {
519519
_arguments \
520520
'--ending-with[Extract prefix (base through boundary) into new workspace]:specs:' \
521521
'--starting-with[Extract suffix (boundary through tip) into new workspace]:specs:' \
522-
'--after-merge[Extract suffix after merge point (auto-detect)]' \
523522
'(-b --branch)'{-b,--branch}'[Branch name for new workspace]:branch:' \
524523
'(-N --fetch --no-fetch)--fetch[Fetch before extract (default)]' \
525524
'(-N --fetch --no-fetch)'{-N,--no-fetch}'[Skip fetching before extract]' \

src/commands/extract.ts

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import { assessExtractRepo } from "../lib/sync/classify-extract";
3131
import { runContinueFlow } from "../lib/sync/continue-flow";
3232
import { parseSplitPoints, resolveSplitPoints } from "../lib/sync/parse-split-points";
3333
import type { ExtractAssessment } from "../lib/sync/types";
34-
import { dryRunNotice, error, info, inlineResult, inlineStart, plural, yellow } from "../lib/terminal";
34+
import { dryRunNotice, error, inlineResult, inlineStart, plural, yellow } from "../lib/terminal";
3535
import { shouldColor } from "../lib/terminal/tty";
3636
import { addWorktrees, requireBranch, requireWorkspace, workspaceRepoDirs } from "../lib/workspace";
3737
import { validateWorkspaceName } from "../lib/workspace/validation";
@@ -40,19 +40,14 @@ export function registerExtractCommand(program: Command): void {
4040
program
4141
.command("extract <workspace>")
4242
.addOption(
43-
new Option("--ending-with <specs...>", "Extract prefix (base through boundary) into new workspace")
44-
.conflicts("startingWith")
45-
.conflicts("afterMerge"),
43+
new Option("--ending-with <specs...>", "Extract prefix (base through boundary) into new workspace").conflicts(
44+
"startingWith",
45+
),
4646
)
4747
.addOption(
48-
new Option("--starting-with <specs...>", "Extract suffix (boundary through tip) into new workspace")
49-
.conflicts("endingWith")
50-
.conflicts("afterMerge"),
51-
)
52-
.addOption(
53-
new Option("--after-merge", "Extract suffix after merge point (auto-detect)")
54-
.conflicts("endingWith")
55-
.conflicts("startingWith"),
48+
new Option("--starting-with <specs...>", "Extract suffix (boundary through tip) into new workspace").conflicts(
49+
"endingWith",
50+
),
5651
)
5752
.option("-b, --branch <name>", "Branch name for new workspace (defaults to workspace name)")
5853
.option("--fetch", "Fetch from all remotes before extract (default)")
@@ -68,17 +63,17 @@ export function registerExtractCommand(program: Command): void {
6863
)
6964
.summary("Extract commits into a new workspace")
7065
.description(
71-
"Examples:\n\n arb extract prereq --ending-with abc123 Extract prefix into 'prereq'\n arb extract cont --starting-with abc123 Extract suffix into 'cont'\n arb extract cont --after-merge Extract post-merge commits\n arb extract prereq --ending-with abc123,def456 Multiple repos (auto-detect)\n arb extract prereq --ending-with api:HEAD~3 Per-repo with explicit prefix\n\nSplits the current workspace's branch at a boundary commit, creating a new stacked workspace.\n\nWith --ending-with, extracts the prefix (base through boundary, inclusive) into a new lower workspace. The original workspace is rebased to stack on top.\n\nWith --starting-with, extracts the suffix (boundary through tip, inclusive) into a new upper workspace. The original workspace is reset to before the boundary.\n\nWith --after-merge, auto-detects the merge point and extracts post-merge commits into a new workspace.\n\nSplit points are specified as commit SHAs (auto-detect repo), <repo>:<commit-ish> (explicit), or tags. Multiple values can be comma-separated.\n\nRepos without an explicit split point have zero commits extracted — they are included in both workspaces but just track the base.",
66+
"Examples:\n\n arb extract prereq --ending-with abc123 Extract prefix into 'prereq'\n arb extract cont --starting-with abc123 Extract suffix into 'cont'\n arb extract prereq --ending-with abc123,def456 Multiple repos (auto-detect)\n arb extract prereq --ending-with api:HEAD~3 Per-repo with explicit prefix\n\nSplits the current workspace's branch at a boundary commit, creating a new stacked workspace.\n\nWith --ending-with, extracts the prefix (base through boundary, inclusive) into a new lower workspace. The original workspace is rebased to stack on top.\n\nWith --starting-with, extracts the suffix (boundary through tip, inclusive) into a new upper workspace. The original workspace is reset to before the boundary.\n\nSplit points are specified as commit SHAs (auto-detect repo), <repo>:<commit-ish> (explicit), or tags. Multiple values can be comma-separated.\n\nRepos without an explicit split point have zero commits extracted — they are included in both workspaces but just track the base.\n\nIn base-merged workspaces, split points must be at or after the merge point — pre-merge commits are already on the default branch.",
7267
)
7368
.action(
7469
arbAction(async (ctx, workspaceName: string, options) => {
7570
const { wsDir, workspace } = requireWorkspace(ctx);
7671

7772
// ── Operation lifecycle: --continue, --abort, gate ──
78-
if ((options.continue || options.abort) && (options.endingWith || options.startingWith || options.afterMerge)) {
73+
if ((options.continue || options.abort) && (options.endingWith || options.startingWith)) {
7974
const flag = options.continue ? "--continue" : "--abort";
80-
error(`${flag} does not accept --ending-with, --starting-with, or --after-merge`);
81-
throw new ArbError(`${flag} does not accept --ending-with, --starting-with, or --after-merge`);
75+
error(`${flag} does not accept --ending-with or --starting-with`);
76+
throw new ArbError(`${flag} does not accept --ending-with or --starting-with`);
8277
}
8378

8479
const inProgress = readInProgressOperation(wsDir, "extract") as
@@ -182,14 +177,12 @@ export function registerExtractCommand(program: Command): void {
182177
}
183178

184179
// Direction from flags
185-
if (!options.endingWith && !options.startingWith && !options.afterMerge) {
186-
const msg =
187-
"Specify --ending-with (prefix extraction), --starting-with (suffix extraction), or --after-merge";
180+
if (!options.endingWith && !options.startingWith) {
181+
const msg = "Specify --ending-with (prefix extraction) or --starting-with (suffix extraction)";
188182
error(msg);
189183
throw new ArbError(msg);
190184
}
191185
const direction: "prefix" | "suffix" = options.endingWith ? "prefix" : "suffix";
192-
const afterMerge = options.afterMerge === true;
193186

194187
const targetBranch = options.branch ?? workspaceName;
195188

@@ -262,19 +255,38 @@ export function registerExtractCommand(program: Command): void {
262255
cache,
263256
analysisCache: ctx.analysisCache,
264257
classify: async ({ repo, repoDir, status, fetchFailed }) => {
265-
let boundary = resolvedSplitPoints.get(repo)?.commitSha ?? null;
258+
const boundary = resolvedSplitPoints.get(repo)?.commitSha ?? null;
266259

267-
// --from-merge: auto-detect boundary from merge detection
268-
if (afterMerge && !boundary && status.base?.merge?.newCommitsAfter != null) {
260+
// Merge-point floor: in merged repos, reject split points below the merge point
261+
if (boundary && status.base?.merge?.newCommitsAfter) {
269262
const n = status.base.merge.newCommitsAfter;
270-
if (n > 0) {
271-
try {
272-
// The first post-merge commit is the boundary (inclusive in extracted set)
273-
const { stdout } = await gitLocal(repoDir, "rev-parse", `HEAD~${n - 1}`);
274-
boundary = stdout.trim();
275-
} catch {
276-
// Cannot resolve merge boundary — treat as no-op for this repo
263+
try {
264+
// HEAD~(n-1) is the first post-merge commit (the inclusive floor)
265+
const { stdout: floorStr } = await gitLocal(repoDir, "rev-parse", `HEAD~${n - 1}`);
266+
const floor = floorStr.trim();
267+
const { exitCode } = await gitLocal(repoDir, "merge-base", "--is-ancestor", floor, boundary);
268+
if (exitCode !== 0) {
269+
return {
270+
repo,
271+
repoDir,
272+
branch,
273+
direction,
274+
targetBranch,
275+
boundary,
276+
mergeBase: mergeBaseMap.get(repo) ?? "",
277+
commitsExtracted: 0,
278+
commitsRemaining: 0,
279+
headSha: status.headSha ?? "",
280+
shallow: status.identity.shallow,
281+
baseRemote: status.base?.remote ?? "",
282+
baseResolvedLocally: status.base?.resolvedVia === "local",
283+
outcome: "skip" as const,
284+
skipReason: "split point is before the merge point — only post-merge commits can be extracted",
285+
skipFlag: "below-merge-point" as const,
286+
};
277287
}
288+
} catch {
289+
// Cannot resolve merge point — fall through to normal assessment
278290
}
279291
}
280292

@@ -386,9 +398,6 @@ export function registerExtractCommand(program: Command): void {
386398
if (willExtract.length === 0) {
387399
if (skipped.length > 0) {
388400
error("Cannot extract: all repos are blocked or have no commits to extract.");
389-
} else if (afterMerge) {
390-
error("Nothing to extract — no repos have been merged.");
391-
info("Use --ending-with or --starting-with to specify a split point.");
392401
} else {
393402
error("Nothing to extract — no repos have commits at the specified split points.");
394403
}

src/commands/push.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ export function buildPushPlanNodes(
267267
nodes.push({
268268
kind: "hint",
269269
cell: cell(
270-
` hint: ${plural(mergedNewWorkCount, "repo")} ${mergedNewWorkCount === 1 ? "was" : "were"} merged but ${mergedNewWorkCount === 1 ? "has" : "have"} new commits since — run 'arb rebase' or 'arb extract --after-merge <workspace>' to split`,
270+
` hint: ${plural(mergedNewWorkCount, "repo")} ${mergedNewWorkCount === 1 ? "was" : "were"} merged but ${mergedNewWorkCount === 1 ? "has" : "have"} new commits since — run 'arb rebase' or 'arb extract <workspace> --starting-with <sha>' to split`,
271271
"muted",
272272
),
273273
});

src/lib/help/stacked.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,7 @@ export const stackedTopic: HelpTopic = {
8787
out(" Extract the suffix (commit through tip) into a new upper workspace.");
8888
out(" The original workspace keeps the prefix.");
8989
out("");
90-
out(` ${dim("arb extract cont --after-merge")}`);
91-
out(" Extract post-merge commits into a new workspace (auto-detects merge point).");
90+
out(" In base-merged workspaces, split points must be at or after the merge point.");
9291
out("");
9392
out(" See 'arb extract --help' for split point syntax and multi-repo options.");
9493
},

src/lib/status/skip-flags.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ export type SkipFlag =
2424
| "retarget-no-default"
2525
| "retarget-same-base"
2626
// Extract-specific
27-
| "extract-target-exists";
27+
| "extract-target-exists"
28+
| "below-merge-point";
2829

2930
export const BENIGN_SKIPS: ReadonlySet<SkipFlag> = new Set([
3031
"already-merged",

test/integration/extract.test.ts

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test";
22
import { existsSync } from "node:fs";
33
import { readFile } from "node:fs/promises";
44
import { join } from "node:path";
5-
import { arb, git, withEnv, write } from "./helpers/env";
5+
import { arb, fetchAllRepos, git, withEnv, write } from "./helpers/env";
66

77
// ── Helpers ──
88

@@ -518,3 +518,89 @@ describe("extract --verbose", () => {
518518
expect(result.output).toContain("Dry run");
519519
}));
520520
});
521+
522+
// ── Merge-point floor validation ──
523+
524+
/**
525+
* Set up a workspace whose branch has been squash-merged into the base,
526+
* with additional post-merge commits.
527+
*
528+
* Returns { preMergeShas, postMergeShas } — the SHAs of commits before/after the merge point.
529+
*/
530+
async function setupMergedWithNewCommits(
531+
env: { projectDir: string; testDir: string },
532+
wsName: string,
533+
preMergeCount: number,
534+
postMergeCount: number,
535+
): Promise<{ preMergeShas: string[]; postMergeShas: string[] }> {
536+
await arb(env, ["create", wsName, "-b", wsName, "repo-a"]);
537+
const wt = join(env.projectDir, `${wsName}/repo-a`);
538+
539+
// Make pre-merge commits and push
540+
const preMergeShas: string[] = [];
541+
for (let i = 1; i <= preMergeCount; i++) {
542+
await write(join(wt, `pre${i}.txt`), `pre ${i}`);
543+
await git(wt, ["add", `pre${i}.txt`]);
544+
await git(wt, ["commit", "-m", `pre-merge commit ${i}`]);
545+
preMergeShas.push((await git(wt, ["rev-parse", "HEAD"])).trim());
546+
}
547+
await git(wt, ["push", "-u", "origin", wsName]);
548+
549+
// Squash-merge the branch into main on the canonical repo
550+
const mainRepo = join(env.projectDir, ".arb/repos/repo-a");
551+
await git(mainRepo, ["merge", "--squash", `origin/${wsName}`]);
552+
await git(mainRepo, ["commit", "-m", `squash: ${wsName}`]);
553+
await git(mainRepo, ["push"]);
554+
555+
// Make post-merge commits on the branch
556+
const postMergeShas: string[] = [];
557+
for (let i = 1; i <= postMergeCount; i++) {
558+
await write(join(wt, `post${i}.txt`), `post ${i}`);
559+
await git(wt, ["add", `post${i}.txt`]);
560+
await git(wt, ["commit", "-m", `post-merge commit ${i}`]);
561+
postMergeShas.push((await git(wt, ["rev-parse", "HEAD"])).trim());
562+
}
563+
564+
// Fetch so merge detection picks up the squash
565+
await fetchAllRepos(env);
566+
567+
return { preMergeShas, postMergeShas };
568+
}
569+
570+
describe("extract merge-point floor validation", () => {
571+
test("rejects --starting-with pointing to a pre-merge commit", () =>
572+
withEnv(async (env) => {
573+
const { preMergeShas } = await setupMergedWithNewCommits(env, "ws", 2, 2);
574+
const result = await arb(
575+
env,
576+
["extract", "cont", "--starting-with", preMergeShas[0] ?? "", "--dry-run", "--no-fetch"],
577+
{ cwd: join(env.projectDir, "ws") },
578+
);
579+
expect(result.exitCode).not.toBe(0);
580+
expect(result.output).toContain("before the merge point");
581+
}));
582+
583+
test("rejects --ending-with pointing to a pre-merge commit", () =>
584+
withEnv(async (env) => {
585+
const { preMergeShas } = await setupMergedWithNewCommits(env, "ws", 2, 2);
586+
const result = await arb(
587+
env,
588+
["extract", "prereq", "--ending-with", preMergeShas[0] ?? "", "--dry-run", "--no-fetch"],
589+
{ cwd: join(env.projectDir, "ws") },
590+
);
591+
expect(result.exitCode).not.toBe(0);
592+
expect(result.output).toContain("before the merge point");
593+
}));
594+
595+
test("accepts --starting-with pointing to a post-merge commit", () =>
596+
withEnv(async (env) => {
597+
const { postMergeShas } = await setupMergedWithNewCommits(env, "ws", 2, 3);
598+
const result = await arb(
599+
env,
600+
["extract", "cont", "--starting-with", postMergeShas[1] ?? "", "--yes", "--no-fetch"],
601+
{ cwd: join(env.projectDir, "ws") },
602+
);
603+
expect(result.exitCode).toBe(0);
604+
expect(result.output).toContain("Created workspace");
605+
}));
606+
});

0 commit comments

Comments
 (0)