fix(FN-5456): never create task branches from arbitrary HEAD in autocorrect
`attemptBranchAutocorrect` was the only branch-creation site in the engine that ran `git checkout -B <expected>` without a start point. When the worktree's HEAD was at a previous occupant's tip, the new label silently captured that commit — the "branch: Created from HEAD" contamination pattern that the cross-contamination guard then refuses to auto-resolve (see FN-5456: orphan FN-5477 commit 268574b9a stranded on fusion/fn-5456). Replace the unsafe fallback with verify-then-`git checkout`: only switch to an existing expected ref; return `failed` when it does not exist so upstream recovery — which knows the proper base SHA — can re-anchor via `prepareForTask` / `reanchorBranchToBase`. Tests updated for the new command sequence; new case covers the "expected ref missing → fail without creating from HEAD" guarantee. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,22 @@
|
||||
---
|
||||
"@fusion/engine": patch
|
||||
---
|
||||
|
||||
fix(engine): never create task branches from arbitrary HEAD in autocorrect
|
||||
|
||||
`attemptBranchAutocorrect` previously fell back to `git checkout -B
|
||||
<expected>` with no start point when rename was not applicable. If the
|
||||
worktree's HEAD happened to be at a previous occupant's commit (e.g. an
|
||||
orphaned tip from a different task), the new branch label silently
|
||||
captured that commit — the "branch: Created from HEAD" contamination
|
||||
pattern that the cross-contamination guard then refuses to auto-resolve.
|
||||
|
||||
This is the only branch-creation site in the engine that did not thread
|
||||
a resolved base SHA; every other path (`prepareForTask`,
|
||||
`reanchorBranchToBase`) already passes the base explicitly.
|
||||
|
||||
Autocorrect now verifies the expected ref exists and uses a plain
|
||||
`git checkout`, so it can only *switch to* an already-existing branch.
|
||||
When the ref is missing it returns `failed`, letting upstream recovery
|
||||
(which knows the proper base) re-anchor with `prepareForTask` /
|
||||
`reanchorBranchToBase`.
|
||||
@@ -77,48 +77,63 @@ describe("attemptBranchAutocorrect", () => {
|
||||
expect(mockedExec.mock.calls.map((c: unknown[]) => c[0])).toContain("git branch -m 'lemon-sage' 'fusion/fn-2'");
|
||||
});
|
||||
|
||||
it("falls back to checkout -B when branch has upstream", async () => {
|
||||
it("falls back to plain checkout when branch has upstream and expected ref exists", async () => {
|
||||
mockedExec
|
||||
.mockResolvedValueOnce({ stdout: "origin/lemon-sage\n" })
|
||||
.mockResolvedValueOnce({ stdout: "" });
|
||||
.mockResolvedValueOnce({ stdout: "origin/lemon-sage\n" }) // upstream check
|
||||
.mockResolvedValueOnce({ stdout: "def456\n" }) // verify expected branch exists
|
||||
.mockResolvedValueOnce({ stdout: "" }); // checkout
|
||||
|
||||
const result = await attemptBranchAutocorrect({ worktreePath: "/tmp/wt", observedBranch: "lemon-sage", expectedBranch: "fusion/fn-2", rootDir: "/tmp" });
|
||||
expect(result).toEqual({ status: "checked-out" });
|
||||
expect(mockedExec.mock.calls.map((c: unknown[]) => c[0])).toEqual([
|
||||
"git rev-parse --abbrev-ref --symbolic-full-name 'lemon-sage'@{u}",
|
||||
"git checkout -B 'fusion/fn-2'",
|
||||
"git rev-parse --verify --quiet 'fusion/fn-2'",
|
||||
"git checkout 'fusion/fn-2'",
|
||||
]);
|
||||
});
|
||||
|
||||
it("falls back to checkout -B when branch sha is shared", async () => {
|
||||
it("falls back to plain checkout when branch sha is shared and expected ref exists", async () => {
|
||||
mockedExec
|
||||
.mockRejectedValueOnce(execError("no upstream"))
|
||||
.mockResolvedValueOnce({ stdout: "abc123\n" })
|
||||
.mockResolvedValueOnce({ stdout: "lemon-sage\nmain\n" })
|
||||
.mockResolvedValueOnce({ stdout: "def456\n" })
|
||||
.mockResolvedValueOnce({ stdout: "" });
|
||||
|
||||
const result = await attemptBranchAutocorrect({ worktreePath: "/tmp/wt", observedBranch: "lemon-sage", expectedBranch: "fusion/fn-2", rootDir: "/tmp" });
|
||||
expect(result).toEqual({ status: "checked-out" });
|
||||
});
|
||||
|
||||
it("falls back to checkout -B when rename fails", async () => {
|
||||
it("falls back to plain checkout when rename fails and expected ref exists", async () => {
|
||||
mockedExec
|
||||
.mockRejectedValueOnce(execError("no upstream"))
|
||||
.mockResolvedValueOnce({ stdout: "abc123\n" })
|
||||
.mockResolvedValueOnce({ stdout: "lemon-sage\n" })
|
||||
.mockRejectedValueOnce(execError("rename denied"))
|
||||
.mockResolvedValueOnce({ stdout: "def456\n" })
|
||||
.mockResolvedValueOnce({ stdout: "" });
|
||||
|
||||
const result = await attemptBranchAutocorrect({ worktreePath: "/tmp/wt", observedBranch: "lemon-sage", expectedBranch: "fusion/fn-2", rootDir: "/tmp" });
|
||||
expect(result).toEqual({ status: "checked-out" });
|
||||
});
|
||||
|
||||
it("returns failed when rename and checkout both fail", async () => {
|
||||
it("returns failed when expected ref does not exist (FN-5456: never create branch from arbitrary HEAD)", async () => {
|
||||
mockedExec
|
||||
.mockResolvedValueOnce({ stdout: "origin/lemon-sage\n" }) // upstream check
|
||||
.mockRejectedValueOnce(execError("")); // rev-parse --verify rejects when ref is unknown
|
||||
|
||||
const result = await attemptBranchAutocorrect({ worktreePath: "/tmp/wt", observedBranch: "lemon-sage", expectedBranch: "fusion/fn-2", rootDir: "/tmp" });
|
||||
expect(result).toEqual({ status: "failed", reason: "expected branch fusion/fn-2 does not exist" });
|
||||
expect(mockedExec.mock.calls.map((c: unknown[]) => c[0])).not.toContain("git checkout -B 'fusion/fn-2'");
|
||||
});
|
||||
|
||||
it("returns failed when rename fails and checkout fails", async () => {
|
||||
mockedExec
|
||||
.mockRejectedValueOnce(execError("no upstream"))
|
||||
.mockResolvedValueOnce({ stdout: "abc123\n" })
|
||||
.mockResolvedValueOnce({ stdout: "lemon-sage\n" })
|
||||
.mockRejectedValueOnce(execError("rename denied"))
|
||||
.mockResolvedValueOnce({ stdout: "def456\n" })
|
||||
.mockRejectedValueOnce(execError("checkout denied"));
|
||||
|
||||
const result = await attemptBranchAutocorrect({ worktreePath: "/tmp/wt", observedBranch: "lemon-sage", expectedBranch: "fusion/fn-2", rootDir: "/tmp" });
|
||||
|
||||
@@ -82,7 +82,17 @@ export async function attemptBranchAutocorrect({
|
||||
}
|
||||
}
|
||||
|
||||
const checkout = await runGit(`git checkout -B ${expectedArg}`, worktreePath);
|
||||
// FN-5456: must NOT use `git checkout -B` with no start point — that would
|
||||
// create (or reset) the expected branch at whatever HEAD currently is,
|
||||
// capturing the previous occupant's tip (the "branch: Created from HEAD"
|
||||
// contamination pattern). Plain `git checkout` only switches to an existing
|
||||
// ref; if `expected` doesn't already exist the caller will surface a
|
||||
// wrong-branch failure with proper base resolution upstream.
|
||||
const verifyExpected = await runGit(`git rev-parse --verify --quiet ${expectedArg}`, worktreePath);
|
||||
if (!verifyExpected.ok) {
|
||||
return { status: "failed", reason: `expected branch ${expected} does not exist` };
|
||||
}
|
||||
const checkout = await runGit(`git checkout ${expectedArg}`, worktreePath);
|
||||
if (checkout.ok) {
|
||||
return { status: "checked-out" };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user