Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions decisions/0100-extract-merge-point-floor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Extract: enforce merge-point floor and remove --after-merge

Date: 2026-03-28

## Context

`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.

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.

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.

## Options

### Tighten bounds only (keep --after-merge)
Fix the validation gap by rejecting split points below the merge point. Keep `--after-merge` as a convenience for auto-detecting the boundary.
- **Pros:** Fixes the bug. Non-breaking.
- **Cons:** `--after-merge` is now just "use the lowest valid split point" — a thin convenience over `--starting-with`.

### Tighten bounds and remove --after-merge
Fix the validation and remove the `--after-merge` flag. Users specify split points explicitly.
- **Pros:** Simpler command surface. Extract requires users to be explicit about what they're splitting. No implicit-and-possible-to-misinterpret behavior.
- **Cons:** Slightly more steps for the post-merge extraction workflow.

## Decision

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.

## Reasoning

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.

The integrate/rebase path already enforced the merge-point floor implicitly by limiting replay. Making extract consistent with this boundary closes the asymmetry.

## Consequences

- 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.
- The `arb push` hint for merged-new-work repos now suggests `arb extract <workspace> --starting-with <sha>` instead of `--after-merge`.
- Shell completions and help topics are updated to remove the flag.
- New `below-merge-point` skip flag blocks extraction when a split point falls before the merge point.
6 changes: 3 additions & 3 deletions docs/stacked-branches.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,13 @@ This extracts everything from commit `abc123` through the tip into a new workspa

### Post-merge continuation

If your branch was merged but you kept committing:
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:

```bash
arb extract next-feature --after-merge --yes
arb extract next-feature --starting-with <first-post-merge-sha> --yes
```

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

### Split points across repos

Expand Down
2 changes: 1 addition & 1 deletion shell/arb.bash
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ __arb_complete_merge() {
__arb_complete_extract() {
local base_dir="$1" cur="$2"
if [[ "$cur" == -* ]]; then
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"))
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"))
return
fi
}
Expand Down
1 change: 0 additions & 1 deletion shell/arb.zsh
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,6 @@ _arb() {
_arguments \
'--ending-with[Extract prefix (base through boundary) into new workspace]:specs:' \
'--starting-with[Extract suffix (boundary through tip) into new workspace]:specs:' \
'--after-merge[Extract suffix after merge point (auto-detect)]' \
'(-b --branch)'{-b,--branch}'[Branch name for new workspace]:branch:' \
'(-N --fetch --no-fetch)--fetch[Fetch before extract (default)]' \
'(-N --fetch --no-fetch)'{-N,--no-fetch}'[Skip fetching before extract]' \
Expand Down
75 changes: 42 additions & 33 deletions src/commands/extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { assessExtractRepo } from "../lib/sync/classify-extract";
import { runContinueFlow } from "../lib/sync/continue-flow";
import { parseSplitPoints, resolveSplitPoints } from "../lib/sync/parse-split-points";
import type { ExtractAssessment } from "../lib/sync/types";
import { dryRunNotice, error, info, inlineResult, inlineStart, plural, yellow } from "../lib/terminal";
import { dryRunNotice, error, inlineResult, inlineStart, plural, yellow } from "../lib/terminal";
import { shouldColor } from "../lib/terminal/tty";
import { addWorktrees, requireBranch, requireWorkspace, workspaceRepoDirs } from "../lib/workspace";
import { validateWorkspaceName } from "../lib/workspace/validation";
Expand All @@ -40,19 +40,14 @@ export function registerExtractCommand(program: Command): void {
program
.command("extract <workspace>")
.addOption(
new Option("--ending-with <specs...>", "Extract prefix (base through boundary) into new workspace")
.conflicts("startingWith")
.conflicts("afterMerge"),
new Option("--ending-with <specs...>", "Extract prefix (base through boundary) into new workspace").conflicts(
"startingWith",
),
)
.addOption(
new Option("--starting-with <specs...>", "Extract suffix (boundary through tip) into new workspace")
.conflicts("endingWith")
.conflicts("afterMerge"),
)
.addOption(
new Option("--after-merge", "Extract suffix after merge point (auto-detect)")
.conflicts("endingWith")
.conflicts("startingWith"),
new Option("--starting-with <specs...>", "Extract suffix (boundary through tip) into new workspace").conflicts(
"endingWith",
),
)
.option("-b, --branch <name>", "Branch name for new workspace (defaults to workspace name)")
.option("--fetch", "Fetch from all remotes before extract (default)")
Expand All @@ -68,17 +63,17 @@ export function registerExtractCommand(program: Command): void {
)
.summary("Extract commits into a new workspace")
.description(
"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.",
"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.",
)
.action(
arbAction(async (ctx, workspaceName: string, options) => {
const { wsDir, workspace } = requireWorkspace(ctx);

// ── Operation lifecycle: --continue, --abort, gate ──
if ((options.continue || options.abort) && (options.endingWith || options.startingWith || options.afterMerge)) {
if ((options.continue || options.abort) && (options.endingWith || options.startingWith)) {
const flag = options.continue ? "--continue" : "--abort";
error(`${flag} does not accept --ending-with, --starting-with, or --after-merge`);
throw new ArbError(`${flag} does not accept --ending-with, --starting-with, or --after-merge`);
error(`${flag} does not accept --ending-with or --starting-with`);
throw new ArbError(`${flag} does not accept --ending-with or --starting-with`);
}

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

// Direction from flags
if (!options.endingWith && !options.startingWith && !options.afterMerge) {
const msg =
"Specify --ending-with (prefix extraction), --starting-with (suffix extraction), or --after-merge";
if (!options.endingWith && !options.startingWith) {
const msg = "Specify --ending-with (prefix extraction) or --starting-with (suffix extraction)";
error(msg);
throw new ArbError(msg);
}
const direction: "prefix" | "suffix" = options.endingWith ? "prefix" : "suffix";
const afterMerge = options.afterMerge === true;

const targetBranch = options.branch ?? workspaceName;

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

// --from-merge: auto-detect boundary from merge detection
if (afterMerge && !boundary && status.base?.merge?.newCommitsAfter != null) {
// Merge-point floor: in merged repos, reject split points below the merge point
if (boundary && status.base?.merge?.newCommitsAfter) {
const n = status.base.merge.newCommitsAfter;
if (n > 0) {
try {
// The first post-merge commit is the boundary (inclusive in extracted set)
const { stdout } = await gitLocal(repoDir, "rev-parse", `HEAD~${n - 1}`);
boundary = stdout.trim();
} catch {
// Cannot resolve merge boundary — treat as no-op for this repo
try {
// HEAD~(n-1) is the first post-merge commit (the inclusive floor)
const { stdout: floorStr } = await gitLocal(repoDir, "rev-parse", `HEAD~${n - 1}`);
const floor = floorStr.trim();
const { exitCode } = await gitLocal(repoDir, "merge-base", "--is-ancestor", floor, boundary);
if (exitCode !== 0) {
return {
repo,
repoDir,
branch,
direction,
targetBranch,
boundary,
mergeBase: mergeBaseMap.get(repo) ?? "",
commitsExtracted: 0,
commitsRemaining: 0,
headSha: status.headSha ?? "",
shallow: status.identity.shallow,
baseRemote: status.base?.remote ?? "",
baseResolvedLocally: status.base?.resolvedVia === "local",
outcome: "skip" as const,
skipReason: "split point is before the merge point — only post-merge commits can be extracted",
skipFlag: "below-merge-point" as const,
};
}
} catch {
// Cannot resolve merge point — fall through to normal assessment
}
}

Expand Down Expand Up @@ -386,9 +398,6 @@ export function registerExtractCommand(program: Command): void {
if (willExtract.length === 0) {
if (skipped.length > 0) {
error("Cannot extract: all repos are blocked or have no commits to extract.");
} else if (afterMerge) {
error("Nothing to extract — no repos have been merged.");
info("Use --ending-with or --starting-with to specify a split point.");
} else {
error("Nothing to extract — no repos have commits at the specified split points.");
}
Expand Down
2 changes: 1 addition & 1 deletion src/commands/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ export function buildPushPlanNodes(
nodes.push({
kind: "hint",
cell: cell(
` 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`,
` 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`,
"muted",
),
});
Expand Down
3 changes: 1 addition & 2 deletions src/lib/help/stacked.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ export const stackedTopic: HelpTopic = {
out(" Extract the suffix (commit through tip) into a new upper workspace.");
out(" The original workspace keeps the prefix.");
out("");
out(` ${dim("arb extract cont --after-merge")}`);
out(" Extract post-merge commits into a new workspace (auto-detects merge point).");
out(" In base-merged workspaces, split points must be at or after the merge point.");
out("");
out(" See 'arb extract --help' for split point syntax and multi-repo options.");
},
Expand Down
3 changes: 2 additions & 1 deletion src/lib/status/skip-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export type SkipFlag =
| "retarget-no-default"
| "retarget-same-base"
// Extract-specific
| "extract-target-exists";
| "extract-target-exists"
| "below-merge-point";

export const BENIGN_SKIPS: ReadonlySet<SkipFlag> = new Set([
"already-merged",
Expand Down
88 changes: 87 additions & 1 deletion test/integration/extract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test";
import { existsSync } from "node:fs";
import { readFile } from "node:fs/promises";
import { join } from "node:path";
import { arb, git, withEnv, write } from "./helpers/env";
import { arb, fetchAllRepos, git, withEnv, write } from "./helpers/env";

// ── Helpers ──

Expand Down Expand Up @@ -518,3 +518,89 @@ describe("extract --verbose", () => {
expect(result.output).toContain("Dry run");
}));
});

// ── Merge-point floor validation ──

/**
* Set up a workspace whose branch has been squash-merged into the base,
* with additional post-merge commits.
*
* Returns { preMergeShas, postMergeShas } — the SHAs of commits before/after the merge point.
*/
async function setupMergedWithNewCommits(
env: { projectDir: string; testDir: string },
wsName: string,
preMergeCount: number,
postMergeCount: number,
): Promise<{ preMergeShas: string[]; postMergeShas: string[] }> {
await arb(env, ["create", wsName, "-b", wsName, "repo-a"]);
const wt = join(env.projectDir, `${wsName}/repo-a`);

// Make pre-merge commits and push
const preMergeShas: string[] = [];
for (let i = 1; i <= preMergeCount; i++) {
await write(join(wt, `pre${i}.txt`), `pre ${i}`);
await git(wt, ["add", `pre${i}.txt`]);
await git(wt, ["commit", "-m", `pre-merge commit ${i}`]);
preMergeShas.push((await git(wt, ["rev-parse", "HEAD"])).trim());
}
await git(wt, ["push", "-u", "origin", wsName]);

// Squash-merge the branch into main on the canonical repo
const mainRepo = join(env.projectDir, ".arb/repos/repo-a");
await git(mainRepo, ["merge", "--squash", `origin/${wsName}`]);
await git(mainRepo, ["commit", "-m", `squash: ${wsName}`]);
await git(mainRepo, ["push"]);

// Make post-merge commits on the branch
const postMergeShas: string[] = [];
for (let i = 1; i <= postMergeCount; i++) {
await write(join(wt, `post${i}.txt`), `post ${i}`);
await git(wt, ["add", `post${i}.txt`]);
await git(wt, ["commit", "-m", `post-merge commit ${i}`]);
postMergeShas.push((await git(wt, ["rev-parse", "HEAD"])).trim());
}

// Fetch so merge detection picks up the squash
await fetchAllRepos(env);

return { preMergeShas, postMergeShas };
}

describe("extract merge-point floor validation", () => {
test("rejects --starting-with pointing to a pre-merge commit", () =>
withEnv(async (env) => {
const { preMergeShas } = await setupMergedWithNewCommits(env, "ws", 2, 2);
const result = await arb(
env,
["extract", "cont", "--starting-with", preMergeShas[0] ?? "", "--dry-run", "--no-fetch"],
{ cwd: join(env.projectDir, "ws") },
);
expect(result.exitCode).not.toBe(0);
expect(result.output).toContain("before the merge point");
}));

test("rejects --ending-with pointing to a pre-merge commit", () =>
withEnv(async (env) => {
const { preMergeShas } = await setupMergedWithNewCommits(env, "ws", 2, 2);
const result = await arb(
env,
["extract", "prereq", "--ending-with", preMergeShas[0] ?? "", "--dry-run", "--no-fetch"],
{ cwd: join(env.projectDir, "ws") },
);
expect(result.exitCode).not.toBe(0);
expect(result.output).toContain("before the merge point");
}));

test("accepts --starting-with pointing to a post-merge commit", () =>
withEnv(async (env) => {
const { postMergeShas } = await setupMergedWithNewCommits(env, "ws", 2, 3);
const result = await arb(
env,
["extract", "cont", "--starting-with", postMergeShas[1] ?? "", "--yes", "--no-fetch"],
{ cwd: join(env.projectDir, "ws") },
);
expect(result.exitCode).toBe(0);
expect(result.output).toContain("Created workspace");
}));
});
Loading