diff --git a/apps/backend/internal/worktree/manager.go b/apps/backend/internal/worktree/manager.go
index d7dce6920c..47c4e772c9 100644
--- a/apps/backend/internal/worktree/manager.go
+++ b/apps/backend/internal/worktree/manager.go
@@ -293,7 +293,13 @@ func (m *Manager) Create(ctx context.Context, req CreateRequest) (*Worktree, err
// was anchored to a PR head or fresh branch that has been deleted upstream
// or never existed in a sibling repo.
fallbackWarning, fallbackDetail := "", ""
- if !m.branchExists(ctx, req.RepositoryPath, baseRef) {
+ baseExists, baseErr := m.branchExists(ctx, req.RepositoryPath, baseRef)
+ if baseErr != nil {
+ // Could not determine existence (timeout / fs stall). Surface the
+ // real cause instead of pretending the branch is missing.
+ return nil, fmt.Errorf("could not verify base branch %q: %w", baseRef, baseErr)
+ }
+ if !baseExists {
fallback := strings.TrimSpace(req.FallbackBaseBranch)
if fallback == "" || fallback == baseRef {
return nil, fmt.Errorf("%w: %s", ErrInvalidBaseBranch, baseRef)
@@ -307,7 +313,11 @@ func (m *Manager) Create(ctx context.Context, req CreateRequest) (*Worktree, err
if req.PullBeforeWorktree {
resolvedFallback = m.pullBaseBranch(ctx, req.RepositoryPath, fallback, nil)
}
- if !m.branchExists(ctx, req.RepositoryPath, resolvedFallback) {
+ fallbackExists, fallbackErr := m.branchExists(ctx, req.RepositoryPath, resolvedFallback)
+ if fallbackErr != nil {
+ return nil, fmt.Errorf("could not verify fallback base branch %q: %w", resolvedFallback, fallbackErr)
+ }
+ if !fallbackExists {
return nil, fmt.Errorf("%w: %s (fallback %q also not found)", ErrInvalidBaseBranch, baseRef, fallback)
}
m.logger.Warn("requested base branch not found, falling back",
@@ -529,7 +539,11 @@ func (m *Manager) fetchBranchToLocal(ctx context.Context, repoPath, branch strin
zap.Error(err))
// Fall back to local branch if it exists.
- if !m.branchExists(ctx, repoPath, branch) {
+ exists, existsErr := m.branchExists(ctx, repoPath, branch)
+ if existsErr != nil {
+ return nil, fmt.Errorf("could not verify local branch %q after fetch failure (%s): %w", branch, strings.TrimSpace(outputStr), existsErr)
+ }
+ if !exists {
return nil, fmt.Errorf("branch %q not found locally or on remote: %s", branch, outputStr)
}
@@ -1329,10 +1343,16 @@ func (m *Manager) isGitRepo(path string) bool {
// branchExists checks if a branch exists in the repository.
// Bounded by m.inspectTimeout so a hung git (credential prompt, stuck filter,
-// filesystem stall) cannot deadlock the caller while holding repoLock. When
-// the bound fires, the ctx error is logged so the root cause is visible in
-// logs rather than surfacing only as a misleading "branch not found".
-func (m *Manager) branchExists(ctx context.Context, repoPath, branch string) bool {
+// filesystem stall) cannot deadlock the caller while holding repoLock.
+//
+// Returns:
+// - (true, nil) branch exists
+// - (false, nil) git ran and reported the branch absent
+// - (false, err) check could not be completed (timeout, fs stall); err
+// carries the underlying ctx error so callers can distinguish a real
+// "missing branch" from a "could not tell" and avoid surfacing a
+// misleading ErrInvalidBaseBranch.
+func (m *Manager) branchExists(ctx context.Context, repoPath, branch string) (bool, error) {
inspectCtx, cancel := context.WithTimeout(ctx, m.inspectTimeout)
defer cancel()
cmd := m.newNonInteractiveGitCmd(inspectCtx, repoPath, "rev-parse", "--verify", branch)
@@ -1342,10 +1362,11 @@ func (m *Manager) branchExists(ctx context.Context, repoPath, branch string) boo
zap.String("repository_path", repoPath),
zap.String("branch", branch),
zap.Error(ctxErr))
+ return false, fmt.Errorf("branch check timed out for %q after %s: %w", branch, m.inspectTimeout, ctxErr)
}
- return false
+ return false, nil
}
- return true
+ return true, nil
}
func (m *Manager) currentBranch(ctx context.Context, repoPath string) string {
@@ -1469,7 +1490,9 @@ func (m *Manager) resolveLocalBaseRef(
if m.currentBranch(ctx, repoPath) == baseBranch {
return m.pullCurrentBranchOrFallback(ctx, repoPath, baseBranch, remoteRef, stepName, onProgress)
}
- if m.branchExists(ctx, repoPath, remoteRef) {
+ // Best-effort sync: a timeout / error here is treated the same as a
+ // missing remote ref — caller falls back to the local baseBranch.
+ if exists, _ := m.branchExists(ctx, repoPath, remoteRef); exists {
m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Synced and using %s", remoteRef), "")
return remoteRef
}
diff --git a/apps/backend/internal/worktree/manager_timeout_test.go b/apps/backend/internal/worktree/manager_timeout_test.go
index 8c904d53ad..daf9c61605 100644
--- a/apps/backend/internal/worktree/manager_timeout_test.go
+++ b/apps/backend/internal/worktree/manager_timeout_test.go
@@ -2,6 +2,7 @@ package worktree
import (
"context"
+ "errors"
"os"
"testing"
"time"
@@ -39,12 +40,15 @@ func TestBranchExists_RespectsContextDeadline(t *testing.T) {
defer cancel()
start := time.Now()
- exists := mgr.branchExists(ctx, t.TempDir(), "main")
+ exists, err := mgr.branchExists(ctx, t.TempDir(), "main")
elapsed := time.Since(start)
if exists {
t.Fatalf("branchExists() = true, want false on hanging git")
}
+ if err == nil {
+ t.Fatalf("branchExists() err = nil, want non-nil on ctx cancellation")
+ }
if elapsed > 2*time.Second {
t.Fatalf("branchExists() took %v, want <2s (ctx not propagated to subprocess)", elapsed)
}
@@ -94,12 +98,15 @@ func TestBranchExists_BoundedWhenCallerHasNoDeadline(t *testing.T) {
mgr.inspectTimeout = 300 * time.Millisecond
start := time.Now()
- exists := mgr.branchExists(context.Background(), t.TempDir(), "main")
+ exists, err := mgr.branchExists(context.Background(), t.TempDir(), "main")
elapsed := time.Since(start)
if exists {
t.Fatalf("branchExists() = true, want false on hanging git")
}
+ if err == nil {
+ t.Fatalf("branchExists() err = nil, want non-nil on inspectTimeout firing")
+ }
// Budget = inspectTimeout + WaitDelay for subprocess pipe cleanup + slack.
if elapsed > 2*time.Second {
t.Fatalf("branchExists() took %v, want <2s (inspectTimeout not applied)", elapsed)
@@ -177,3 +184,45 @@ func TestCreate_HangingRevParseReleasesRepoLock(t *testing.T) {
t.Fatalf("second Create() stuck on repo lock (elapsed %v)", time.Since(lockStart))
}
}
+
+// TestCreate_InspectTimeoutDoesNotSurfaceAsInvalidBaseBranch pins the bug
+// where a hung `git rev-parse --verify ` (e.g. a stalled NFS / SMB
+// mount) caused branchExists to return false and Create to surface
+// ErrInvalidBaseBranch — falsely claiming the base branch did not exist when
+// in fact the check timed out. Callers must see the underlying timeout, not
+// a misleading "branch not found" error that triggers task FAILED states.
+func TestCreate_InspectTimeoutDoesNotSurfaceAsInvalidBaseBranch(t *testing.T) {
+ scriptDir := writeFakeGitScript(t, hangOnRevParseScript)
+ t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH"))
+
+ cfg := newTestConfig(t)
+ mgr, err := NewManager(cfg, newMockStore(), newTestLogger())
+ if err != nil {
+ t.Fatalf("NewManager failed: %v", err)
+ }
+ // Tight inspect timeout so the test completes quickly.
+ mgr.inspectTimeout = 200 * time.Millisecond
+
+ repoPath := t.TempDir()
+ if err := os.MkdirAll(repoPath+"/.git", 0755); err != nil {
+ t.Fatalf("failed to create .git dir: %v", err)
+ }
+
+ req := CreateRequest{
+ TaskID: "task-a",
+ SessionID: "sess-a",
+ TaskTitle: "inspect timeout repro",
+ RepositoryPath: repoPath,
+ BaseBranch: "master",
+ TaskDirName: "task-a",
+ RepoName: "repo-a",
+ }
+
+ _, err = mgr.Create(context.Background(), req)
+ if err == nil {
+ t.Fatalf("Create() err = nil, want timeout error on hanging git rev-parse")
+ }
+ if errors.Is(err, ErrInvalidBaseBranch) {
+ t.Fatalf("Create() err = %v, must NOT be ErrInvalidBaseBranch on inspect timeout (regression: misleading 'base branch does not exist' surfaced to users)", err)
+ }
+}
diff --git a/apps/web/components/diff/use-expandable-diff.test.ts b/apps/web/components/diff/use-expandable-diff.test.ts
new file mode 100644
index 0000000000..9b7c2ae5d5
--- /dev/null
+++ b/apps/web/components/diff/use-expandable-diff.test.ts
@@ -0,0 +1,143 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { renderHook, act } from "@testing-library/react";
+import type { FileDiffMetadata } from "@pierre/diffs";
+
+const processFileMock = vi.fn();
+const requestFileContentMock = vi.fn();
+const requestFileContentAtRefMock = vi.fn();
+const getWebSocketClientMock = vi.fn(() => ({}) as unknown);
+
+vi.mock("@pierre/diffs", () => ({
+ processFile: (...args: unknown[]) => processFileMock(...args),
+}));
+vi.mock("@/lib/ws/connection", () => ({
+ getWebSocketClient: () => getWebSocketClientMock(),
+}));
+vi.mock("@/lib/ws/workspace-files", () => ({
+ requestFileContent: (...args: unknown[]) => requestFileContentMock(...args),
+ requestFileContentAtRef: (...args: unknown[]) => requestFileContentAtRefMock(...args),
+}));
+
+type UseExpandableDiffFn = typeof import("./use-expandable-diff").useExpandableDiff;
+
+let useExpandableDiff: UseExpandableDiffFn;
+beforeEach(async () => {
+ vi.resetModules();
+ processFileMock.mockReset();
+ requestFileContentMock.mockReset();
+ requestFileContentAtRefMock.mockReset();
+ getWebSocketClientMock.mockReset().mockReturnValue({});
+ ({ useExpandableDiff } = await import("./use-expandable-diff"));
+});
+
+/** Build a minimal FileDiffMetadata where the trailing-context counts match. */
+function consistentMeta(overrides: Partial = {}): FileDiffMetadata {
+ return {
+ name: "src/foo.ts",
+ type: "modified" as FileDiffMetadata["type"],
+ hunks: [
+ {
+ additionStart: 1,
+ additionCount: 3,
+ additionLines: 3,
+ additionLineIndex: 0,
+ deletionStart: 1,
+ deletionCount: 3,
+ deletionLines: 3,
+ deletionLineIndex: 0,
+ hunkContent: [],
+ },
+ ],
+ // 5 lines total, hunk uses 3 starting at 0, leaves 2 trailing on each side
+ additionLines: ["a", "b", "c", "d", "e"],
+ deletionLines: ["a", "b", "c", "d", "e"],
+ splitLineCount: 0,
+ unifiedLineCount: 0,
+ isPartial: false,
+ ...overrides,
+ } as FileDiffMetadata;
+}
+
+/** Build a FileDiffMetadata whose trailing additions vs deletions disagree. */
+function inconsistentMeta(): FileDiffMetadata {
+ const m = consistentMeta();
+ // Same hunk indices/counts, but extend deletionLines so trailing remainders
+ // differ (2 trailing additions vs 4 trailing deletions). This is exactly
+ // the shape @pierre/diffs' iterateOverDiff asserts on and throws over.
+ return { ...m, deletionLines: [...m.deletionLines, "f", "g"] };
+}
+
+const PARTIAL = consistentMeta({ isPartial: true });
+
+const baseProps = {
+ sessionId: "sess-1",
+ filePath: "src/foo.ts",
+ baseRef: "HEAD",
+ diff: "diff --git a/src/foo.ts b/src/foo.ts\n--- a/src/foo.ts\n+++ b/src/foo.ts\n@@ -1,3 +1,3 @@\n a\n-b\n+B\n c\n",
+ enableExpansion: true,
+};
+
+async function loadAndSettle(loadContent: () => Promise) {
+ await act(async () => {
+ await loadContent();
+ });
+}
+
+describe("useExpandableDiff", () => {
+ it("returns the partial fileDiffMetadata before content is loaded", () => {
+ const { result } = renderHook(() =>
+ useExpandableDiff({ ...baseProps, fileDiffMetadata: PARTIAL }),
+ );
+ expect(result.current.metadata).toBe(PARTIAL);
+ expect(result.current.isContentLoaded).toBe(false);
+ });
+
+ it("returns the reparsed metadata when trailing-context counts agree", async () => {
+ requestFileContentMock.mockResolvedValue({ content: "new", is_binary: false });
+ requestFileContentAtRefMock.mockResolvedValue({ content: "old", is_binary: false });
+ const reparsed = consistentMeta();
+ processFileMock.mockReturnValue(reparsed);
+
+ const { result } = renderHook(() =>
+ useExpandableDiff({ ...baseProps, fileDiffMetadata: PARTIAL }),
+ );
+ await loadAndSettle(result.current.loadContent);
+ expect(result.current.metadata).toBe(reparsed);
+ });
+
+ it("falls back to the partial metadata when the reparse is inconsistent", async () => {
+ requestFileContentMock.mockResolvedValue({ content: "new", is_binary: false });
+ requestFileContentAtRefMock.mockResolvedValue({ content: "new", is_binary: false });
+ processFileMock.mockReturnValue(inconsistentMeta());
+
+ const { result } = renderHook(() =>
+ useExpandableDiff({ ...baseProps, fileDiffMetadata: PARTIAL }),
+ );
+ await loadAndSettle(result.current.loadContent);
+ expect(result.current.metadata).toBe(PARTIAL);
+ // canExpand must be false: clicking the toolbar button after a rejected
+ // reparse is a silent no-op because the partial metadata can't drive
+ // @pierre/diffs' iterateOverDiff expansion path.
+ expect(result.current.canExpand).toBe(false);
+ });
+
+ it("preserves the lang override from the partial metadata on a successful reparse", async () => {
+ requestFileContentMock.mockResolvedValue({ content: "new", is_binary: false });
+ requestFileContentAtRefMock.mockResolvedValue({ content: "old", is_binary: false });
+ processFileMock.mockReturnValue(consistentMeta({ lang: "go" as never }));
+
+ const overridden = consistentMeta({ isPartial: true, lang: "text" as never });
+ const { result } = renderHook(() =>
+ useExpandableDiff({ ...baseProps, fileDiffMetadata: overridden }),
+ );
+ await loadAndSettle(result.current.loadContent);
+ expect((result.current.metadata as { lang?: string }).lang).toBe("text");
+ });
+
+ it("returns null when no fileDiffMetadata was provided", () => {
+ const { result } = renderHook(() =>
+ useExpandableDiff({ ...baseProps, fileDiffMetadata: null }),
+ );
+ expect(result.current.metadata).toBeNull();
+ });
+});
diff --git a/apps/web/components/diff/use-expandable-diff.ts b/apps/web/components/diff/use-expandable-diff.ts
index 3e511af01e..7cc40171a9 100644
--- a/apps/web/components/diff/use-expandable-diff.ts
+++ b/apps/web/components/diff/use-expandable-diff.ts
@@ -32,6 +32,25 @@ function isFileNotFoundError(error: string): boolean {
return /file not found|not found|no such file|does not exist/i.test(error);
}
+/**
+ * Validate that a reparsed metadata won't trip @pierre/diffs' iterateOverDiff
+ * trailing-context check (which throws and tears down the renderer). When
+ * fetched contents are out of sync with the patch, processFile still produces
+ * a metadata object whose final-hunk indices leave a different number of
+ * trailing addition vs deletion lines — the exact condition iterateOverDiff
+ * asserts on. Detecting it here lets the caller fall back to the partial
+ * metadata instead of crashing.
+ */
+function isReparseConsistent(meta: FileDiffMetadata): boolean {
+ const lastHunk = meta.hunks?.[meta.hunks.length - 1];
+ if (!lastHunk) return true;
+ const addRemain =
+ meta.additionLines.length - (lastHunk.additionLineIndex + lastHunk.additionCount);
+ const delRemain =
+ meta.deletionLines.length - (lastHunk.deletionLineIndex + lastHunk.deletionCount);
+ return addRemain === delRemain;
+}
+
/** Fetch old file content at a git ref. Returns empty string for new files. */
async function fetchOldContent(
client: WsClient,
@@ -149,18 +168,31 @@ export function useExpandableDiff({
}
}, [sessionId, filePath, baseRef, repo, enableExpansion, loadedContent, isLoading]);
- const metadata = useMemo(() => {
- if (!fileDiffMetadata) return null;
- if (!loadedContent || !diff) return fileDiffMetadata;
+ const { metadata, reparseAccepted } = useMemo<{
+ metadata: FileDiffMetadata | null;
+ reparseAccepted: boolean;
+ }>(() => {
+ if (!fileDiffMetadata) return { metadata: null, reparseAccepted: false };
+ if (!loadedContent || !diff) return { metadata: fileDiffMetadata, reparseAccepted: false };
const reparsed = processFile(diff, {
oldFile: { name: filePath, contents: loadedContent.oldContent },
newFile: { name: filePath, contents: loadedContent.newContent },
});
- if (!reparsed) return fileDiffMetadata;
+ if (!reparsed) return { metadata: fileDiffMetadata, reparseAccepted: false };
+ // Reject a reparse whose trailing-context lengths don't match: when the
+ // fetched contents are out of sync with the patch (stale snapshot, wrong
+ // base ref for a committed diff, file edited mid-flight), processFile
+ // still returns a metadata object, but @pierre/diffs' iterateOverDiff
+ // will throw "trailing context mismatch" the moment it tries to render.
+ // Falling back to the original partial metadata loses expansion controls
+ // but renders the patch correctly — same as the dedicated Diff tab.
+ if (!isReparseConsistent(reparsed))
+ return { metadata: fileDiffMetadata, reparseAccepted: false };
// Preserve the lang override that useDiffMetadata sets (e.g. lang:'text'
// for Go files that hit the Shiki backtracking guard). processFile would
// otherwise infer "go" from the filename and silently re-enable Shiki.
- return fileDiffMetadata.lang ? { ...reparsed, lang: fileDiffMetadata.lang } : reparsed;
+ const final = fileDiffMetadata.lang ? { ...reparsed, lang: fileDiffMetadata.lang } : reparsed;
+ return { metadata: final, reparseAccepted: true };
}, [fileDiffMetadata, loadedContent, diff, filePath]);
const isContentLoaded = loadedContent !== null;
@@ -171,6 +203,9 @@ export function useExpandableDiff({
isLoading,
error,
loadContent,
- canExpand: enableExpansion && isContentLoaded && !error,
+ // Gate canExpand on a successful reparse so the toolbar button doesn't
+ // appear when we fell back to the partial metadata — clicking it would
+ // be a silent no-op since the metadata can't drive iterateOverDiff.
+ canExpand: enableExpansion && isContentLoaded && !error && reparseAccepted,
};
}
diff --git a/apps/web/components/review/review-diff-list.tsx b/apps/web/components/review/review-diff-list.tsx
index 91fafeb515..8a3b1f624e 100644
--- a/apps/web/components/review/review-diff-list.tsx
+++ b/apps/web/components/review/review-diff-list.tsx
@@ -374,7 +374,15 @@ function renderDiffContent(opts: {
// belongs to the local branch, not the PR head — when the same file has
// both PR and local changes, the wrong content gets paired with the PR
// patch and @pierre/diffs 1.1.x renders nothing/errors. Disable expansion
- // for PR-sourced rows; uncommitted/committed rows still get it.
+ // for PR-sourced rows.
+ //
+ // Also disable expansion for committed-source rows: the backend computes
+ // the diff against the session's stored base commit, not HEAD. HEAD
+ // already contains the committed changes, so fetching old content at
+ // baseRef="HEAD" returns the same content as the working tree, leaving
+ // processFile's trailing-context counts inconsistent and crashing the
+ // renderer. Using the right base would need the session's base SHA
+ // plumbed in here; that's a follow-up.
//
// Also disable expansion for untracked files: the backend synthesizes a
// single all-additions hunk against /dev/null, so there is no real
@@ -382,7 +390,7 @@ function renderDiffContent(opts: {
// races with concurrent edits — if the file shrinks after the snapshot,
// the cached hunk header advertises more lines than the fetched content
// has and @pierre/diffs' DiffHunksRenderer throws.
- const enableExpansion = file.source !== "pr" && file.status !== "untracked";
+ const enableExpansion = file.source === "uncommitted" && file.status !== "untracked";
return (
<>