From a16ba18b90a5022fb56e99ad1b6b162601c5acd4 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 09:29:22 +0000 Subject: [PATCH 01/17] feat: expose workspace initialization steps and progress --- src/common/orpc/schemas/stream.ts | 9 ++ src/common/orpc/types.ts | 6 + src/node/acp/agent.ts | 1 + src/node/acp/streamTranslator.ts | 1 + src/node/runtime/Runtime.ts | 1 + src/node/services/agentSession.ts | 1 + src/node/services/initStateManager.test.ts | 117 +++++++++++++++++++ src/node/services/initStateManager.ts | 22 +++- src/node/services/taskService.testHarness.ts | 1 + src/node/services/taskService.ts | 5 +- src/node/services/workspaceService.ts | 8 +- 11 files changed, 168 insertions(+), 4 deletions(-) diff --git a/src/common/orpc/schemas/stream.ts b/src/common/orpc/schemas/stream.ts index 6f9dc8d84f4..648821f4325 100644 --- a/src/common/orpc/schemas/stream.ts +++ b/src/common/orpc/schemas/stream.ts @@ -591,6 +591,7 @@ export const InitStartEventSchema = z.object({ export const InitOutputEventSchema = z.object({ type: z.literal("init-output"), line: z.string(), + step: z.boolean().optional(), timestamp: z.number(), isError: z.boolean().optional(), lineNumber: z @@ -605,6 +606,13 @@ export const InitOutputEventSchema = z.object({ .meta({ description: "True when this event is emitted during init replay" }), }); +export const InitProgressEventSchema = z.object({ + type: z.literal("init-progress"), + label: z.string(), + percent: z.number().int().min(0).max(100), + timestamp: z.number(), +}); + export const InitEndEventSchema = z.object({ type: z.literal("init-end"), exitCode: z.number(), @@ -621,6 +629,7 @@ export const InitEndEventSchema = z.object({ export const WorkspaceInitEventSchema = z.discriminatedUnion("type", [ InitStartEventSchema, InitOutputEventSchema, + InitProgressEventSchema, InitEndEventSchema, ]); diff --git a/src/common/orpc/types.ts b/src/common/orpc/types.ts index b86bf7aa469..29b66532d8c 100644 --- a/src/common/orpc/types.ts +++ b/src/common/orpc/types.ts @@ -172,6 +172,12 @@ export function isInitOutput( return (msg as { type?: string }).type === "init-output"; } +export function isInitProgress( + msg: WorkspaceChatMessage +): msg is Extract { + return msg.type === "init-progress"; +} + export function isInitEnd( msg: WorkspaceChatMessage ): msg is Extract { diff --git a/src/node/acp/agent.ts b/src/node/acp/agent.ts index 94ad7d67861..8bed157f19c 100644 --- a/src/node/acp/agent.ts +++ b/src/node/acp/agent.ts @@ -1627,6 +1627,7 @@ export class MuxAgent implements Agent { event.type === "advisor-reasoning-output" || event.type === "bash-output" || event.type === "init-output" || + event.type === "init-progress" || // Drop replay history messages under saturation, but keep live message // events so ACP clients do not miss real-time conversation updates. isReplayMessageEvent diff --git a/src/node/acp/streamTranslator.ts b/src/node/acp/streamTranslator.ts index 96d0a0e0007..f5e537d4152 100644 --- a/src/node/acp/streamTranslator.ts +++ b/src/node/acp/streamTranslator.ts @@ -222,6 +222,7 @@ export class StreamTranslator { case "runtime-status": case "init-start": case "init-output": + case "init-progress": case "init-end": return []; diff --git a/src/node/runtime/Runtime.ts b/src/node/runtime/Runtime.ts index 96a1aeae807..3c7e987a0f2 100644 --- a/src/node/runtime/Runtime.ts +++ b/src/node/runtime/Runtime.ts @@ -165,6 +165,7 @@ export interface FileStat { export interface InitLogger { /** Log a creation step (e.g., "Creating worktree", "Syncing files") */ logStep(message: string): void; + logProgress?(label: string, percent: number): void; /** Log stdout line from init hook */ logStdout(line: string): void; /** Log stderr line from init hook */ diff --git a/src/node/services/agentSession.ts b/src/node/services/agentSession.ts index 0061622535b..3c487fd58f5 100644 --- a/src/node/services/agentSession.ts +++ b/src/node/services/agentSession.ts @@ -9014,6 +9014,7 @@ export class AgentSession { forward("init-start", (payload) => this.emitChatEvent(payload)); forward("init-output", (payload) => this.emitChatEvent(payload)); + forward("init-progress", (payload) => this.emitChatEvent(payload)); forward("init-end", (payload) => this.emitChatEvent(payload)); } diff --git a/src/node/services/initStateManager.test.ts b/src/node/services/initStateManager.test.ts index efadc52de4a..4cbc62b30fc 100644 --- a/src/node/services/initStateManager.test.ts +++ b/src/node/services/initStateManager.test.ts @@ -4,6 +4,7 @@ import * as os from "os"; import { describe, it, expect, beforeEach, afterEach } from "bun:test"; import { Config } from "@/node/config"; import { InitStateManager } from "./initStateManager"; +import { createAgentSessionHarness } from "./agentSession.testHarness"; import type { WorkspaceInitEvent } from "@/common/orpc/types"; import { INIT_HOOK_MAX_LINES } from "@/common/constants/toolLimits"; import { workspaceFileLocks } from "@/node/utils/concurrency/workspaceFileLocks"; @@ -211,6 +212,122 @@ describe("InitStateManager", () => { }); }); + describe("creation progress", () => { + it("preserves step markers live, on disk, and on replay without marking raw output", async () => { + const workspaceId = "test-workspace"; + const outputs: Array> = []; + manager.on("init-output", (event: Extract) => + outputs.push(event) + ); + + manager.startInit(workspaceId, "/path/to/hook"); + manager.appendOutput(workspaceId, "Preparing checkout", false, true); + manager.appendOutput(workspaceId, "raw output", false); + manager.appendOutput(workspaceId, "raw error", true, false); + + expect(outputs.map((event) => event.step)).toEqual([true, undefined, undefined]); + expect(outputs.map((event) => event.lineNumber)).toEqual([0, 1, 2]); + const lines = structuredClone(manager.getInitState(workspaceId)?.lines); + expect(lines?.map((line) => line.step)).toEqual([true, undefined, undefined]); + await manager.endInit(workspaceId, 0); + expect((await manager.readInitStatus(workspaceId))?.lines).toEqual(lines); + + const liveOutputs = [...outputs]; + for (const fromDisk of [false, true]) { + if (fromDisk) manager.clearInMemoryState(workspaceId); + outputs.length = 0; + await manager.replayInit(workspaceId); + expect(outputs).toEqual(liveOutputs.map((event) => ({ ...event, replay: true }))); + } + }); + + it("emits progress without changing durable output, persistence, or replay", async () => { + const workspaceId = "test-workspace"; + const progress: WorkspaceInitEvent[] = []; + manager.on("init-progress", (event: WorkspaceInitEvent) => progress.push(event)); + manager.startInit(workspaceId, "/path/to/hook"); + manager.appendOutput(workspaceId, "Preparing checkout", false, true); + const initialState = structuredClone(manager.getInitState(workspaceId)); + + const before = Date.now(); + manager.reportProgress(workspaceId, "Checking out files", 40); + expect(progress).toHaveLength(1); + expect(progress[0]).toMatchObject({ + type: "init-progress", + workspaceId, + label: "Checking out files", + percent: 40, + }); + expect(progress[0].timestamp).toBeGreaterThanOrEqual(before); + expect(progress[0].timestamp).toBeLessThanOrEqual(Date.now()); + expect(manager.getInitState(workspaceId)).toEqual(initialState); + expect(await manager.readInitStatus(workspaceId)).toBeNull(); + progress.length = 0; + await manager.replayInit(workspaceId); + expect(progress).toEqual([]); + + await manager.endInit(workspaceId, 0); + expect(await manager.readInitStatus(workspaceId)).toEqual(manager.getInitState(workspaceId)); + expect((await manager.readInitStatus(workspaceId))?.lines).toEqual(initialState?.lines); + manager.clearInMemoryState(workspaceId); + await manager.replayInit(workspaceId); + expect(progress).toEqual([]); + }); + + it("ignores progress for missing, cleared, successful, and failed init runs", async () => { + const progress: WorkspaceInitEvent[] = []; + manager.on("init-progress", (event: WorkspaceInitEvent) => progress.push(event)); + manager.reportProgress("missing", "Checking out files", 10); + manager.startInit("cleared", "/path/to/hook"); + manager.clearInMemoryState("cleared"); + manager.reportProgress("cleared", "Checking out files", 20); + for (const exitCode of [0, 1]) { + const workspaceId = "completed-" + exitCode; + manager.startInit(workspaceId, "/path/to/hook"); + await manager.endInit(workspaceId, exitCode); + manager.reportProgress(workspaceId, "Checking out files", 30); + } + expect(progress).toEqual([]); + }); + + it("forwards live progress only to the matching agent session and unsubscribes on disposal", async () => { + const workspaceId = "test-workspace"; + const harness = await createAgentSessionHarness({ + workspaceId, + initStateManager: manager, + captureEvents: true, + }); + try { + manager.startInit(workspaceId, "/path/to/hook"); + manager.startInit("other-workspace", "/path/to/hook"); + manager.reportProgress("other-workspace", "Checking out files", 10); + manager.reportProgress(workspaceId, "Checking out files", 20); + const progress = harness.events.filter((event) => event.type === "init-progress"); + expect(progress).toHaveLength(1); + expect(progress[0]).toMatchObject({ label: "Checking out files", percent: 20 }); + expect(progress[0]).not.toHaveProperty("workspaceId"); + await harness.session.dispose(); + expect(manager.listenerCount("init-progress")).toBe(0); + } finally { + await harness.session.dispose(); + await harness.cleanup(); + } + }); + + it("bounds and rounds percentages while ignoring non-finite input", () => { + const workspaceId = "test-workspace"; + const progress: Array> = []; + manager.on("init-progress", (event: Extract) => + progress.push(event) + ); + manager.startInit(workspaceId, "/path/to/hook"); + for (const percent of [-1, 0, 39.8, 100, 101, NaN, Infinity, -Infinity]) { + manager.reportProgress(workspaceId, "Checking out files", percent); + } + expect(progress.map((event) => event.percent)).toEqual([0, 0, 40, 100, 100]); + }); + }); + describe("cleanup", () => { it("should delete persisted state from disk", async () => { const workspaceId = "test-workspace"; diff --git a/src/node/services/initStateManager.ts b/src/node/services/initStateManager.ts index 3c2847cc8c3..9e66bafe520 100644 --- a/src/node/services/initStateManager.ts +++ b/src/node/services/initStateManager.ts @@ -12,6 +12,7 @@ import { getErrorMessage } from "@/common/utils/errors"; export interface TimedLine { line: string; isError: boolean; // true if from stderr + step?: true; timestamp: number; } @@ -140,6 +141,7 @@ export class InitStateManager extends EventEmitter { workspaceId, line: timedLine.line, isError: timedLine.isError, + ...(timedLine.step ? { step: true } : {}), timestamp: timedLine.timestamp, // Use original timestamp for replay lineNumber: truncatedLines + index, replay: true, @@ -244,7 +246,7 @@ export class InitStateManager extends EventEmitter { * Truncation strategy: Keep only the most recent INIT_HOOK_MAX_LINES lines (tail). * Older lines are dropped to prevent OOM with large rsync/build output. */ - appendOutput(workspaceId: string, line: string, isError: boolean): void { + appendOutput(workspaceId: string, line: string, isError: boolean, step = false): void { const state = this.store.getState(workspaceId); if (!state) { @@ -254,7 +256,7 @@ export class InitStateManager extends EventEmitter { const timestamp = Date.now(); const lineNumber = (state.truncatedLines ?? 0) + state.lines.length; - const timedLine: TimedLine = { line, isError, timestamp }; + const timedLine: TimedLine = { line, isError, timestamp, ...(step ? { step: true } : {}) }; // Truncation: keep only the most recent MAX_LINES if (state.lines.length >= INIT_HOOK_MAX_LINES) { @@ -269,11 +271,27 @@ export class InitStateManager extends EventEmitter { workspaceId, line, isError, + ...(step ? { step: true } : {}), timestamp, lineNumber, } satisfies WorkspaceInitEvent & { workspaceId: string }); } + reportProgress(workspaceId: string, label: string, percent: number): void { + if (this.store.getState(workspaceId)?.status !== "running" || !Number.isFinite(percent)) { + return; + } + + // Transient progress must not fill the durable init log or reappear on replay. + this.emit("init-progress", { + type: "init-progress", + workspaceId, + label, + percent: Math.max(0, Math.min(100, Math.round(percent))), + timestamp: Date.now(), + } satisfies WorkspaceInitEvent & { workspaceId: string }); + } + /** * Finalize init hook execution. * Updates state, persists to disk, emits init-end event, and resolves completion promise. diff --git a/src/node/services/taskService.testHarness.ts b/src/node/services/taskService.testHarness.ts index f8b3dbe1074..4fc25173236 100644 --- a/src/node/services/taskService.testHarness.ts +++ b/src/node/services/taskService.testHarness.ts @@ -45,6 +45,7 @@ export function createMockInitStateManager(): InitStateManager { startInit: mock(() => undefined), enterHookPhase: mock(() => undefined), appendOutput: mock(() => undefined), + reportProgress: mock(() => undefined), endInit: mock(() => Promise.resolve()), getInitState: mock(() => undefined), readInitStatus: mock(() => Promise.resolve(null)), diff --git a/src/node/services/taskService.ts b/src/node/services/taskService.ts index dffa11b16b1..943bee0dc04 100644 --- a/src/node/services/taskService.ts +++ b/src/node/services/taskService.ts @@ -2762,7 +2762,10 @@ export class TaskService implements AgentTaskIntegration { this.initStateManager.startInit(workspaceId, projectPath); return { - logStep: (message: string) => this.initStateManager.appendOutput(workspaceId, message, false), + logStep: (message: string) => + this.initStateManager.appendOutput(workspaceId, message, false, true), + logProgress: (label: string, percent: number) => + this.initStateManager.reportProgress(workspaceId, label, percent), logStdout: (line: string) => this.initStateManager.appendOutput(workspaceId, line, false), logStderr: (line: string) => this.initStateManager.appendOutput(workspaceId, line, true), logComplete: (exitCode: number) => void this.initStateManager.endInit(workspaceId, exitCode), diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 205e7a2126e..ceb51df7c86 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -4010,7 +4010,13 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { if (!hasInitState()) { return; } - this.initStateManager.appendOutput(workspaceId, message, false); + this.initStateManager.appendOutput(workspaceId, message, false, true); + }, + logProgress: (label: string, percent: number) => { + if (!hasInitState()) { + return; + } + this.initStateManager.reportProgress(workspaceId, label, percent); }, logStdout: (line: string) => { if (!hasInitState()) { From 1e5a3d85a7b8fe83cda7911dfeb6be420ec3bc19 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 09:29:27 +0000 Subject: [PATCH 02/17] feat: stream local worktree checkout progress --- src/node/worktree/WorktreeManager.test.ts | 157 ++++++++++++++++++++++ src/node/worktree/WorktreeManager.ts | 74 +++++++--- src/node/worktree/gitProgress.test.ts | 72 ++++++++++ src/node/worktree/gitProgress.ts | 38 ++++++ 4 files changed, 322 insertions(+), 19 deletions(-) create mode 100644 src/node/worktree/gitProgress.test.ts create mode 100644 src/node/worktree/gitProgress.ts diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index 466acff9b4c..b163f632425 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, spyOn } from "bun:test"; import * as os from "os"; import * as path from "path"; import * as fsPromises from "fs/promises"; +import { existsSync } from "node:fs"; import { execFileSync, execSync } from "node:child_process"; import * as disposableExec from "@/node/utils/disposableExec"; import type { InitLogger } from "@/node/runtime/Runtime"; @@ -161,6 +162,162 @@ describe("WorktreeManager constructor", () => { }); describe("WorktreeManager.createWorkspace", () => { + for (const existing of [false, true]) { + it(`populates a clean ${existing ? "existing-branch" : "new-branch"} worktree and streams checkout output`, async () => { + const branchName = "feature-progress"; + const fixture = await createWorktreeManagerFixture({ + existingBranchName: existing ? branchName : undefined, + }); + const realExecFile = disposableExec.execFileAsync; + const stdout: string[] = []; + const stderr: string[] = []; + const progress: Array<[string, number]> = []; + const initLogger = { + ...fixture.initLogger, + logStdout: (line: string) => stdout.push(line), + logStderr: (line: string) => stderr.push(line), + logProgress: (label: string, percent: number) => progress.push([label, percent]), + }; + const workspacePath = fixture.manager.getWorkspacePath(fixture.projectPath, branchName); + const hookMarker = path.join(fixture.rootDir, "checkout-hook-ran"); + const hook = path.join(fixture.projectPath, ".git", "hooks", "post-checkout"); + let checkoutStarted = false; + const execSpy = spyOn(disposableExec, "execFileAsync").mockImplementation( + (file, args, options) => { + if (file === "git" && args[2] === "checkout") { + checkoutStarted = true; + expect(existsSync(path.join(workspacePath, "README.md"))).toBe(false); + options?.onStderrData?.("Updating files: 25% (1/4)\rUpdating fi"); + options?.onStderrData?.("les: 100% (4/4), done.\n"); + } + const proc = realExecFile(file, args, options); + if (file === "git" && args[2] === "worktree" && args[3] === "add") { + // --no-checkout usually emits only stderr, so supply stdout to cover both streams. + const result = proc.result; + Object.defineProperty(proc, "result", { + value: result.then((output) => ({ + ...output, + stdout: "worktree metadata ready\r\n", + })), + }); + } + return proc; + } + ); + try { + let expectedContent = "hello\n"; + if (existing) { + execFileSync("git", ["checkout", branchName], { + cwd: fixture.projectPath, + stdio: "ignore", + }); + expectedContent = "existing branch contents\n"; + await fsPromises.writeFile(path.join(fixture.projectPath, "README.md"), expectedContent); + execFileSync("git", ["commit", "-am", "branch contents"], { + cwd: fixture.projectPath, + stdio: "ignore", + }); + execFileSync("git", ["checkout", "main"], { cwd: fixture.projectPath, stdio: "ignore" }); + } + await fsPromises.writeFile(hook, '#!/bin/sh\nprintf ran > "' + hookMarker + '"\n'); + await fsPromises.chmod(hook, 0o755); + const result = await fixture.manager.createWorkspace({ + projectPath: fixture.projectPath, + branchName, + trunkBranch: "main", + skipRemoteSync: true, + trusted: false, + initLogger, + }); + expect(result).toEqual({ success: true, workspacePath }); + expect(checkoutStarted).toBe(true); + expect(await fsPromises.readFile(path.join(workspacePath, "README.md"), "utf8")).toBe( + expectedContent + ); + expect( + execFileSync("git", ["status", "--porcelain"], { cwd: workspacePath }).toString() + ).toBe(""); + expect( + execFileSync("git", ["branch", "--show-current"], { cwd: workspacePath }) + .toString() + .trim() + ).toBe(branchName); + expect(existsSync(hookMarker)).toBe(false); + expect(stdout).toContain("worktree metadata ready"); + expect(stderr.some((line) => line.includes("Preparing worktree"))).toBe(true); + expect(stderr).toContain("Updating files: 100% (4/4), done."); + expect(stderr.some((line) => line.includes(branchName))).toBe(true); + expect(progress).toEqual([ + ["Updating files", 25], + ["Updating files", 100], + ]); + } finally { + execSpy.mockRestore(); + await fixture.cleanup(); + } + }, 20_000); + + it(`removes a failed ${existing ? "existing-branch" : "new-branch"} worktree checkout`, async () => { + const branchName = "feature-checkout-failure"; + const fixture = await createWorktreeManagerFixture({ + existingBranchName: existing ? branchName : undefined, + }); + const stderr: string[] = []; + try { + await fsPromises.writeFile( + path.join(fixture.projectPath, ".gitattributes"), + "README.md filter=fail\n" + ); + execFileSync("git", ["add", ".gitattributes"], { + cwd: fixture.projectPath, + stdio: "ignore", + }); + execFileSync("git", ["commit", "-m", "require checkout filter"], { + cwd: fixture.projectPath, + stdio: "ignore", + }); + if (existing) { + execFileSync("git", ["branch", "-f", branchName, "main"], { + cwd: fixture.projectPath, + stdio: "ignore", + }); + } + execFileSync("git", ["config", "filter.fail.smudge", "exit 1"], { + cwd: fixture.projectPath, + }); + execFileSync("git", ["config", "filter.fail.required", "true"], { + cwd: fixture.projectPath, + }); + const result = await fixture.manager.createWorkspace({ + projectPath: fixture.projectPath, + branchName, + trunkBranch: "main", + skipRemoteSync: true, + trusted: true, + initLogger: { ...fixture.initLogger, logStderr: (line) => stderr.push(line) }, + }); + expect(result.success).toBe(false); + if (result.success) throw new Error("Expected checkout to fail"); + expect(result.error).toContain("smudge filter fail failed"); + expect(stderr.some((line) => line.includes("smudge filter fail failed"))).toBe(true); + const workspacePath = fixture.manager.getWorkspacePath(fixture.projectPath, branchName); + expect(existsSync(workspacePath)).toBe(false); + expect( + execFileSync("git", ["worktree", "list", "--porcelain"], { + cwd: fixture.projectPath, + }).toString() + ).not.toContain(workspacePath); + expect( + execFileSync("git", ["branch", "--list", branchName], { cwd: fixture.projectPath }) + .toString() + .trim() + ).toBe(existing ? branchName : ""); + } finally { + await fixture.cleanup(); + } + }, 20_000); + } + const rollbackCases = [ { name: "rolls back failed new worktrees when submodule materialization fails", diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index 7ed25fa9044..11c4b509df8 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -26,6 +26,7 @@ import { } from "@/constants/terminationTimeouts"; import { syncLocalGitSubmodules } from "@/node/runtime/submoduleSync"; import { syncXumignoreFiles } from "./xumignore"; +import { GitProgressParser } from "./gitProgress"; type GitExecOptions = Pick | undefined; @@ -169,27 +170,62 @@ export class WorktreeManager { params.abortSignal )); - // Create worktree (git worktree is typically fast) - if (branchExists) { - // Branch exists, just add a worktree pointing to the existing ref without rewriting it. - using proc = execFileAsync( - "git", - ["-C", projectPath, "worktree", "add", workspacePath, branchName], - noHooksEnv - ); - await proc.result; - } else { - // Branch doesn't exist, create from the requested start point when provided. Restore flows - // use this to recreate archived branches from exact saved commits instead of fetching origin. - const newBranchBase = - startPoint ?? (shouldUseOrigin ? `origin/${trunkBranch}` : trunkBranch); - using proc = execFileAsync( + // Separate checkout so large repositories can stream file materialization progress. + // Restore flows may supply an exact saved commit instead of the current trunk. + const newBranchBase = startPoint ?? (shouldUseOrigin ? `origin/${trunkBranch}` : trunkBranch); + using addProc = execFileAsync( + "git", + [ + "-C", + projectPath, + "worktree", + "add", + "--no-checkout", + ...(branchExists + ? [workspacePath, branchName] + : ["-b", branchName, workspacePath, newBranchBase]), + ], + noHooksEnv + ); + const addResult = await addProc.result; + createdBranch = !branchExists; + for (const line of addResult.stdout.split(/[\r\n]/)) { + if (line) initLogger.logStdout(line); + } + for (const line of addResult.stderr.split(/[\r\n]/)) { + if (line) initLogger.logStderr(line); + } + + initLogger.logStep("Checking out files..."); + const progress = new GitProgressParser( + (stage, percent) => initLogger.logProgress?.(stage, percent), + (line) => initLogger.logStderr(line) + ); + try { + using checkoutProc = execFileAsync( "git", - ["-C", projectPath, "worktree", "add", "-b", branchName, workspacePath, newBranchBase], - noHooksEnv + ["-C", workspacePath, "checkout", "--progress", "--force", branchName], + { ...noHooksEnv, onStderrData: (chunk) => progress.push(chunk) } ); - await proc.result; - createdBranch = true; + const { stdout } = await checkoutProc.result; + for (const line of stdout.split(/[\r\n]/)) { + if (line) initLogger.logStdout(line); + } + } catch (error) { + try { + await this.rollbackFailedWorkspaceCreation({ + projectPath, + workspacePath, + branchName, + createdBranch, + trusted: params.trusted, + }); + } catch { + // Preserve the checkout error even when best-effort cleanup fails. + } + throw error; + } finally { + progress.flush(); } worktreeCreated = true; diff --git a/src/node/worktree/gitProgress.test.ts b/src/node/worktree/gitProgress.test.ts new file mode 100644 index 00000000000..f3ae1000500 --- /dev/null +++ b/src/node/worktree/gitProgress.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "bun:test"; +import { GitProgressParser } from "./gitProgress"; + +function createParser() { + const progress: Array<{ stage: string; percent: number }> = []; + const output: string[] = []; + const parser = new GitProgressParser( + (stage, percent) => progress.push({ stage, percent }), + (line) => output.push(line) + ); + return { parser, progress, output }; +} + +describe("GitProgressParser", () => { + it("buffers partial chunks and handles carriage returns and newlines", () => { + const { parser, progress, output } = createParser(); + parser.push("Updating fi"); + parser.push("les: 1"); + expect(progress).toEqual([]); + parser.push("2% (12/100)\rUpdating files: 34% (34/100)\n"); + expect(progress).toEqual([ + { stage: "Updating files", percent: 12 }, + { stage: "Updating files", percent: 34 }, + ]); + expect(output).toEqual([]); + }); + + it("deduplicates percentages while allowing a new stage at the same percentage", () => { + const { parser, progress } = createParser(); + parser.push("Updating files: 50% (5/10)\rUpdating files: 50% (50/100)\r"); + parser.push("Filtering content: 50% (1/2)\rFiltering content: 75% (3/4)\r"); + expect(progress).toEqual([ + { stage: "Updating files", percent: 50 }, + { stage: "Filtering content", percent: 50 }, + { stage: "Filtering content", percent: 75 }, + ]); + }); + + it("keeps final done lines as raw output without repeating 100 percent", () => { + const { parser, progress, output } = createParser(); + parser.push("Updating files: 100% (10/10)\r"); + parser.push("Updating files: 100% (10/10), done.\r"); + parser.push("\nFiltering content: 100% (2/2), done.\n"); + expect(progress).toEqual([ + { stage: "Updating files", percent: 100 }, + { stage: "Filtering content", percent: 100 }, + ]); + expect(output).toEqual([ + "Updating files: 100% (10/10), done.", + "Filtering content: 100% (2/2), done.", + ]); + }); + + it("preserves raw diagnostics and flushes an unterminated line only once", () => { + const { parser, progress, output } = createParser(); + parser.push("warning: low disk space\r\n\n extra detail\nfa"); + parser.push("tal: checkout failed"); + expect(output).toEqual(["warning: low disk space", " extra detail"]); + parser.flush(); + parser.flush(); + expect(output).toEqual(["warning: low disk space", " extra detail", "fatal: checkout failed"]); + expect(progress).toEqual([]); + }); + + it("flushes a partial progress record as raw output instead of guessing completion", () => { + const { parser, progress, output } = createParser(); + parser.push("Updating files: 80% (8/10)"); + parser.flush(); + expect(progress).toEqual([]); + expect(output).toEqual(["Updating files: 80% (8/10)"]); + }); +}); diff --git a/src/node/worktree/gitProgress.ts b/src/node/worktree/gitProgress.ts new file mode 100644 index 00000000000..dc21d3ab72f --- /dev/null +++ b/src/node/worktree/gitProgress.ts @@ -0,0 +1,38 @@ +export class GitProgressParser { + private buffer = ""; + private lastStage: string | undefined; + private lastPercent: number | undefined; + + constructor( + private readonly onProgress: (stage: string, percent: number) => void, + private readonly onOutput: (line: string) => void + ) {} + + push(chunk: string): void { + this.buffer += chunk; + const lines = this.buffer.split(/[\r\n]/); + this.buffer = lines.pop() ?? ""; + for (const line of lines) { + if (!line) continue; + const match = /^(.+?):\s+(\d{1,3})%/.exec(line); + if (!match) { + this.onOutput(line); + continue; + } + const stage = match[1]; + const done = /\bdone\.\s*$/.test(line); + const percent = done ? 100 : Number(match[2]); + if (stage !== this.lastStage || percent !== this.lastPercent) { + this.lastStage = stage; + this.lastPercent = percent; + this.onProgress(stage, percent); + } + if (done) this.onOutput(line); + } + } + + flush(): void { + if (this.buffer) this.onOutput(this.buffer); + this.buffer = ""; + } +} From c900320a6f76b1b351da82312cf8f15df047cf1e Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 09:51:19 +0000 Subject: [PATCH 03/17] feat(workspace): show inline creation progress card --- docs/hooks/init.mdx | 6 +- .../components/ProgressBar/ProgressBar.tsx | 25 +++ .../features/Messages/InitMessage.stories.tsx | 184 +++++++++++++----- src/browser/features/Messages/InitMessage.tsx | 178 +++++++++++------ src/browser/stores/WorkspaceStore.test.ts | 4 +- src/browser/stores/WorkspaceStore.ts | 4 + .../stories/App.chatLoading.stories.tsx | 5 +- .../StreamingMessageAggregator.init.test.ts | 104 ++++++++++ .../messages/StreamingMessageAggregator.ts | 43 +++- .../applyWorkspaceChatEventToAggregator.ts | 9 +- .../utils/messages/messageUtils.test.ts | 1 + src/common/types/message.ts | 7 +- .../utils/messages/retryEligibility.test.ts | 1 + .../builtInSkillContent.generated.ts | 6 +- src/node/services/initStateManager.test.ts | 4 +- src/node/services/initStateManager.ts | 3 +- src/node/worktree/WorktreeManager.test.ts | 2 +- src/node/worktree/gitProgress.test.ts | 2 +- src/node/worktree/gitProgress.ts | 7 +- tests/ipc/workspace/init.test.ts | 9 +- tests/ui/chat/initMessage.test.ts | 114 +++++++++++ 21 files changed, 580 insertions(+), 138 deletions(-) create mode 100644 src/browser/components/ProgressBar/ProgressBar.tsx create mode 100644 tests/ui/chat/initMessage.test.ts diff --git a/docs/hooks/init.mdx b/docs/hooks/init.mdx index 85355d97fc8..22eab975c6c 100644 --- a/docs/hooks/init.mdx +++ b/docs/hooks/init.mdx @@ -72,11 +72,9 @@ bun install ## Output -Init output appears in a banner at the top of the workspace. Click to expand/collapse the log. The banner shows: +The creation card appears after the first user message in the transcript. While setup runs, it shows a checklist of steps and checkout progress when available. Select **More details** to see the project path and stdout/stderr output. -- Script path (`.xum/init`) -- Status (running, success, or exit code on failure) -- Full stdout/stderr output +On success, the card collapses to **Workspace created** with the elapsed time. Click the header to expand the log. On failure, the card stays expanded and shows the exit code and error output. ## Idempotency diff --git a/src/browser/components/ProgressBar/ProgressBar.tsx b/src/browser/components/ProgressBar/ProgressBar.tsx new file mode 100644 index 00000000000..137a5813890 --- /dev/null +++ b/src/browser/components/ProgressBar/ProgressBar.tsx @@ -0,0 +1,25 @@ +import { cn } from "@/common/lib/utils"; + +interface ProgressBarProps { + value: number; + className?: string; + "aria-label"?: string; +} + +export function ProgressBar(props: ProgressBarProps) { + return ( +
+
+
+ ); +} diff --git a/src/browser/features/Messages/InitMessage.stories.tsx b/src/browser/features/Messages/InitMessage.stories.tsx index 216be40eb6d..f11c7a73535 100644 --- a/src/browser/features/Messages/InitMessage.stories.tsx +++ b/src/browser/features/Messages/InitMessage.stories.tsx @@ -1,4 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, userEvent, within } from "@storybook/test"; import { InitMessage } from "@/browser/features/Messages/InitMessage"; import { STABLE_TIMESTAMP } from "@/browser/stories/mocks/workspaces"; import { lightweightMeta } from "@/browser/stories/meta.js"; @@ -6,36 +7,43 @@ import type { DisplayedMessage } from "@/common/types/message"; type WorkspaceInitMessage = Extract; -const INIT_SUCCESS_MESSAGE: WorkspaceInitMessage = { +const RUNNING_MESSAGE: WorkspaceInitMessage = { type: "workspace-init", - id: "init-success", - historySequence: 1, + id: "workspace-init", + historySequence: -1, + status: "running", + hookPath: "/home/user/projects/my-app", + lines: [ + { line: "Preparing workspace", isError: false, step: true }, + { line: "Creating git worktree", isError: false, step: true }, + { line: "Preparing worktree (new branch 'feature')", isError: false }, + { line: "Checking out files", isError: false, step: true }, + { line: "HEAD is now at 1234567 Add application", isError: false }, + ], + progress: { label: "Updating files", percent: 87 }, + exitCode: null, + timestamp: STABLE_TIMESTAMP, + durationMs: null, +}; + +const SUCCESS_MESSAGE: WorkspaceInitMessage = { + ...RUNNING_MESSAGE, status: "success", - hookPath: "/home/user/projects/my-app/.mux/init.sh", lines: [ - { line: "Installing dependencies...", isError: false }, - { line: "Setting up environment variables...", isError: false }, - { line: "Starting development server...", isError: false }, + ...RUNNING_MESSAGE.lines, + { line: "Running init hook: .xum/init", isError: false, step: true }, + { line: "Dependencies installed", isError: false }, ], + progress: null, exitCode: 0, - timestamp: STABLE_TIMESTAMP - 106000, durationMs: 3000, }; -const INIT_ERROR_MESSAGE: WorkspaceInitMessage = { - type: "workspace-init", - id: "init-error", - historySequence: 1, +const ERROR_MESSAGE: WorkspaceInitMessage = { + ...SUCCESS_MESSAGE, status: "error", - hookPath: "/home/user/projects/my-app/.mux/init.sh", - lines: [ - { line: "Installing dependencies...", isError: false }, - { line: "Failed to install package 'missing-dep'", isError: true }, - { line: "npm ERR! code E404", isError: true }, - ], + lines: [...SUCCESS_MESSAGE.lines, { line: "Package installation failed", isError: true }], exitCode: 1, - timestamp: STABLE_TIMESTAMP - 107000, - durationMs: 3000, }; const meta = { @@ -44,7 +52,7 @@ const meta = { component: InitMessage, render: (args) => (
-
+
@@ -52,43 +60,121 @@ const meta = { } satisfies Meta; export default meta; - type Story = StoryObj; -/** - * Story showing the InitMessage component in success state. - * Tests the workspace init hook display with completed status. - */ -export const InitHookSuccess: Story = { - args: { - message: INIT_SUCCESS_MESSAGE, +export const Running: Story = { + args: { message: RUNNING_MESSAGE }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.getByRole("progressbar")).toHaveAttribute("aria-valuenow", "87"); + await expect(canvas.getAllByLabelText("Completed")).toHaveLength(2); + await expect(canvas.getByLabelText("In progress")).toBeVisible(); + const details = canvas.getByRole("button", { name: "More details" }); + await expect(details).toHaveAttribute("aria-expanded", "false"); + await expect(canvas.queryByText(RUNNING_MESSAGE.hookPath)).not.toBeInTheDocument(); + await userEvent.click(details); + await expect(canvas.getByText(RUNNING_MESSAGE.hookPath)).toBeVisible(); + await expect(canvas.getByText(RUNNING_MESSAGE.lines[2].line)).toBeVisible(); + await userEvent.click(details); }, - parameters: { - docs: { - description: { - story: - "Shows the InitMessage component after a successful init hook execution. " + - "The message displays with a green checkmark, hook path, and output lines.", - }, - }, +}; + +export const RunningWithoutProgress: Story = { + args: { message: { ...RUNNING_MESSAGE, progress: null } }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.queryByRole("progressbar")).not.toBeInTheDocument(); + await expect(canvas.getByLabelText("In progress")).toBeVisible(); + }, +}; + +export const InitHookSuccess: Story = { + args: { message: SUCCESS_MESSAGE }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const header = canvas.getByRole("button", { name: /Workspace created/ }); + await expect(header).toHaveAttribute("aria-expanded", "false"); + await expect(canvas.queryByRole("list")).not.toBeInTheDocument(); + await userEvent.click(header); + await expect(canvas.getByText("Dependencies installed")).toBeVisible(); + await expect(canvas.getAllByLabelText("Completed")).toHaveLength(4); + await userEvent.click(header); + await expect(canvas.queryByText("Dependencies installed")).not.toBeInTheDocument(); }, }; -/** - * Story showing the InitMessage component in error state. - * Tests the workspace init hook display with failed status. - */ export const InitHookError: Story = { + args: { message: ERROR_MESSAGE }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.getByRole("button", { name: /Workspace setup failed/ })).toHaveAttribute( + "aria-expanded", + "true" + ); + await expect(canvas.getByRole("button", { name: "More details" })).toHaveAttribute( + "aria-expanded", + "true" + ); + await expect(canvas.getByText("Package installation failed")).toHaveClass( + "text-init-output-error-text" + ); + await expect(canvas.getByLabelText("Failed")).toBeVisible(); + }, +}; + +export const LegacySuccess: Story = { args: { - message: INIT_ERROR_MESSAGE, + message: { + ...SUCCESS_MESSAGE, + lines: SUCCESS_MESSAGE.lines.map(({ line, isError }) => ({ line, isError })), + }, }, - parameters: { - docs: { - description: { - story: - "Shows the InitMessage component after a failed init hook execution. " + - "The message displays with a red alert icon, error styling, and error output.", - }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: /Workspace created/ })); + await expect(canvas.getByText("Dependencies installed")).toBeVisible(); + await expect(canvas.queryByRole("button", { name: "More details" })).not.toBeInTheDocument(); + await expect(canvas.queryByRole("list")).not.toBeInTheDocument(); + }, +}; + +export const RunningPhone: Story = { + ...Running, + args: { + message: { + ...RUNNING_MESSAGE, + lines: [ + ...RUNNING_MESSAGE.lines, + { + line: "Checking out files for a workspace with a very long descriptive branch name", + isError: false, + step: true, + }, + ], }, }, + decorators: [ + (Story) => ( +
+ +
+ ), + ], + globals: { viewport: { value: "mobile1", isRotated: false } }, + parameters: { pixel: { matrix: { viewports: ["phone"] } } }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const frame = canvas.getByTestId("init-phone"); + const progress = canvas.getByRole("progressbar"); + await expect(frame.getBoundingClientRect().width).toBeLessThanOrEqual(375); + await expect(frame.scrollWidth).toBeLessThanOrEqual(frame.clientWidth); + await expect(progress.getBoundingClientRect().width).toBeGreaterThan(0); + await expect(progress.getBoundingClientRect().right).toBeLessThanOrEqual( + frame.getBoundingClientRect().right + ); + const percent = canvas.getByText("87%"); + await expect(percent.getBoundingClientRect().right).toBeLessThanOrEqual( + frame.getBoundingClientRect().right + ); + }, }; diff --git a/src/browser/features/Messages/InitMessage.tsx b/src/browser/features/Messages/InitMessage.tsx index 54e7adbd3cf..afd97651297 100644 --- a/src/browser/features/Messages/InitMessage.tsx +++ b/src/browser/features/Messages/InitMessage.tsx @@ -1,94 +1,154 @@ -import React, { useEffect, useRef } from "react"; +import { useEffect, useRef, useState } from "react"; import { cn } from "@/common/lib/utils"; import type { DisplayedMessage } from "@/common/types/message"; -import { Loader2, Wrench, CheckCircle2, AlertCircle } from "lucide-react"; +import { Loader2, GitBranch, ChevronRight, CheckCircle2, AlertCircle } from "lucide-react"; import { Shimmer } from "../AIElements/Shimmer"; import { formatDuration } from "@/common/utils/formatDuration"; +import { ProgressBar } from "@/browser/components/ProgressBar/ProgressBar"; interface InitMessageProps { message: Extract; className?: string; } -export const InitMessage = React.memo(({ message, className }) => { +export function InitMessage(props: InitMessageProps) { + const message = props.message; const isError = message.status === "error"; const isRunning = message.status === "running"; - const isSuccess = message.status === "success"; + const [expandedOverride, setExpandedOverride] = useState(null); + const [detailsOverride, setDetailsOverride] = useState(null); + const expanded = expandedOverride ?? message.status !== "success"; + const steps = message.lines.filter((line) => line.step === true); + const rawLines = message.lines.filter((line) => line.step !== true); + const detailsExpanded = steps.length === 0 || (detailsOverride ?? !isRunning); const preRef = useRef(null); - // Auto-scroll to bottom while running useEffect(() => { if (isRunning && preRef.current) { preRef.current.scrollTop = preRef.current.scrollHeight; } - }, [isRunning, message.lines.length]); + }, [isRunning, message.lines.length, expanded, detailsExpanded]); const durationText = message.durationMs !== null ? ` in ${formatDuration(message.durationMs, "precise")}` : ""; return ( -
-
- +
+
-
{message.hookPath}
- {message.lines.length > 0 && ( - + {detailsExpanded && ( + <> +
+ {message.hookPath} +
+ {(rawLines.length > 0 || !!message.truncatedLines) && ( + 
+                  {message.truncatedLines && (
+                    
+                      ... {message.truncatedLines.toLocaleString()} earlier lines truncated ...
+                      {"\n"}
+                    
+                  )}
+                  {rawLines.map((line, index) => (
+                    
+                      {line.line}
+                      {index < rawLines.length - 1 ? "\n" : ""}
+                    
+                  ))}
+
+ )} + + )} +
)}
); -}); - -InitMessage.displayName = "InitMessage"; +} diff --git a/src/browser/stores/WorkspaceStore.test.ts b/src/browser/stores/WorkspaceStore.test.ts index 66b70457570..ccbe1cfa18f 100644 --- a/src/browser/stores/WorkspaceStore.test.ts +++ b/src/browser/stores/WorkspaceStore.test.ts @@ -3036,6 +3036,7 @@ describe("WorkspaceStore", () => { isError: false, timestamp: 1_001, }; + yield { type: "init-progress", label: "Checkout", percent: 87, timestamp: 1_002 }; return; } @@ -3075,7 +3076,8 @@ describe("WorkspaceStore", () => { return ( state.loading === false && initMessage?.status === "running" && - initMessage.lines[0]?.line === firstLine + initMessage.lines[0]?.line === firstLine && + initMessage.progress?.percent === 87 ); }); expect(sawInitialInit).toBe(true); diff --git a/src/browser/stores/WorkspaceStore.ts b/src/browser/stores/WorkspaceStore.ts index c60abbbd56c..5f5899224e0 100644 --- a/src/browser/stores/WorkspaceStore.ts +++ b/src/browser/stores/WorkspaceStore.ts @@ -1116,6 +1116,10 @@ export class WorkspaceStore { // we update aggregator state immediately but coalesce UI bumps to keep the renderer responsive. this.scheduleIdleStateBump(workspaceId); }, + "init-progress": (workspaceId, aggregator, data) => { + applyWorkspaceChatEventToAggregator(aggregator, data); + this.scheduleIdleStateBump(workspaceId); + }, "init-end": (workspaceId, aggregator, data) => { applyWorkspaceChatEventToAggregator(aggregator, data); // Avoid a double-bump if an init-output idle bump is pending. diff --git a/src/browser/stories/App.chatLoading.stories.tsx b/src/browser/stories/App.chatLoading.stories.tsx index c404fb464aa..86775c2e7ea 100644 --- a/src/browser/stories/App.chatLoading.stories.tsx +++ b/src/browser/stories/App.chatLoading.stories.tsx @@ -288,15 +288,16 @@ function createHydrationStory(workspaceId: string): AppStory { emitChat({ type: "init-output", line: "Preparing workspace", + step: true, isError: false, timestamp: STABLE_TIMESTAMP, replay: true, }); - await expect((await canvas.findAllByText(/Running init hook/))[0]).toBeVisible(); + await expect((await canvas.findAllByText(/Creating workspace/))[0]).toBeVisible(); await expect(await canvas.findByText("Preparing workspace")).toBeVisible(); await expect(canvas.queryByTestId("transcript-hydration-placeholder")).toBeNull(); emitChat({ type: "init-end", exitCode: 0, timestamp: STABLE_TIMESTAMP, replay: true }); - await expect(await canvas.findByText(/Init hook completed/)).toBeVisible(); + await expect(await canvas.findByText(/Workspace created/)).toBeVisible(); await checkTranscriptLayout(canvasElement); emitChat({ type: "stream-lifecycle", diff --git a/src/browser/utils/messages/StreamingMessageAggregator.init.test.ts b/src/browser/utils/messages/StreamingMessageAggregator.init.test.ts index 8cd57f895e0..f421fb03055 100644 --- a/src/browser/utils/messages/StreamingMessageAggregator.init.test.ts +++ b/src/browser/utils/messages/StreamingMessageAggregator.init.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from "bun:test"; import { StreamingMessageAggregator } from "./StreamingMessageAggregator"; import { INIT_HOOK_MAX_LINES } from "@/common/constants/toolLimits"; +import { createMuxMessage } from "@/common/types/message"; interface InitDisplayedMessage { type: "workspace-init"; @@ -14,6 +15,109 @@ interface InitDisplayedMessage { const waitForInitThrottle = () => new Promise((r) => setTimeout(r, 120)); describe("Init display after cleanup changes", () => { + it.each([false, true])( + "places init after the first user regardless of early completion (%s)", + (completed) => { + const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z"); + aggregator.handleMessage({ type: "init-start", hookPath: "/project", timestamp: 1 }); + if (completed) aggregator.handleMessage({ type: "init-end", exitCode: 0, timestamp: 2 }); + expect(aggregator.getDisplayedMessages().map((message) => message.type)).toEqual([ + "workspace-init", + ]); + aggregator.handleMessage({ + type: "message", + ...createMuxMessage("user", "user", "Create this workspace", { + historySequence: 7, + timestamp: 3, + }), + }); + expect(aggregator.getDisplayedMessages().map((message) => message.type)).toEqual([ + "user", + "workspace-init", + ]); + aggregator.handleMessage({ + type: "message", + ...createMuxMessage("assistant", "assistant", "Ready", { + historySequence: 8, + timestamp: 4, + }), + }); + aggregator.handleMessage({ + type: "message", + ...createMuxMessage("next-user", "user", "Continue", { historySequence: 9, timestamp: 5 }), + }); + expect(aggregator.getDisplayedMessages().map((message) => message.type)).toEqual([ + "user", + "workspace-init", + "assistant", + "user", + ]); + } + ); + + it("propagates steps and throttles progress until another step or completion clears it", () => { + const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z"); + const progress = { + type: "init-progress" as const, + label: "Updating files", + percent: 87, + timestamp: 2, + }; + aggregator.handleMessage(progress); + expect(aggregator.getDisplayedMessages()).toEqual([]); + aggregator.handleMessage({ type: "init-start", hookPath: "/project", timestamp: 1 }); + const before = aggregator.getDisplayedMessages(); + aggregator.handleMessage(progress); + expect(aggregator.getDisplayedMessages()).toBe(before); + aggregator.flushPendingInitOutput(); + expect(aggregator.getDisplayedMessages()[0]).toMatchObject({ + progress: { label: "Updating files", percent: 87 }, + }); + const step = { type: "init-output" as const, line: "Running hook", step: true, timestamp: 3 }; + aggregator.handleMessage(step); + aggregator.flushPendingInitOutput(); + expect(aggregator.getDisplayedMessages()[0]).toMatchObject({ + progress: null, + lines: [{ line: step.line, isError: false, step: true }], + }); + aggregator.handleMessage(progress); + aggregator.handleMessage({ type: "init-output", line: "Raw output", timestamp: 4 }); + aggregator.flushPendingInitOutput(); + expect(aggregator.getDisplayedMessages()[0]).toMatchObject({ + progress: { label: "Updating files", percent: 87 }, + }); + aggregator.handleMessage({ type: "init-end", exitCode: 0, timestamp: 5 }); + expect(aggregator.getDisplayedMessages()[0]).toMatchObject({ progress: null }); + const finished = aggregator.getDisplayedMessages(); + aggregator.handleMessage(progress); + aggregator.flushPendingInitOutput(); + expect(aggregator.getDisplayedMessages()).toEqual(finished); + }); + + it("keeps checklist lines and live progress unchanged during reconnect replay", () => { + const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z"); + const start = { type: "init-start" as const, hookPath: "/project", timestamp: 1 }; + const step = { type: "init-output" as const, line: "Checkout", step: true, timestamp: 2 }; + aggregator.handleMessage(start); + aggregator.handleMessage(step); + aggregator.handleMessage({ + type: "init-progress", + label: "Updating files", + percent: 87, + timestamp: 3, + }); + aggregator.flushPendingInitOutput(); + const before = aggregator.getDisplayedMessages(); + aggregator.handleMessage({ ...start, replay: true }); + aggregator.handleMessage({ ...step, replay: true }); + aggregator.flushPendingInitOutput(); + expect(aggregator.getDisplayedMessages()).toEqual(before); + expect(aggregator.getDisplayedMessages()[0]).toMatchObject({ + lines: [{ line: step.line, isError: false, step: true }], + progress: { label: "Updating files", percent: 87 }, + }); + }); + it("should display init messages correctly", async () => { const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z"); diff --git a/src/browser/utils/messages/StreamingMessageAggregator.ts b/src/browser/utils/messages/StreamingMessageAggregator.ts index 7e91e314ac7..3e33610c2b6 100644 --- a/src/browser/utils/messages/StreamingMessageAggregator.ts +++ b/src/browser/utils/messages/StreamingMessageAggregator.ts @@ -56,7 +56,13 @@ import type { OnChatCursor, OnChatHistoryCursor, } from "@/common/orpc/types"; -import { isInitStart, isInitOutput, isInitEnd, isMuxMessage } from "@/common/orpc/types"; +import { + isInitStart, + isInitOutput, + isInitProgress, + isInitEnd, + isMuxMessage, +} from "@/common/orpc/types"; import { buildAggregateResponseCompleteMetadata, buildResponseCompleteMetadata, @@ -591,7 +597,8 @@ export class StreamingMessageAggregator { private initState: { status: "running" | "success" | "error"; hookPath: string; - lines: Array<{ line: string; isError: boolean }>; + lines: Array<{ line: string; isError: boolean; step?: boolean }>; + progress: { label: string; percent: number } | null; exitCode: number | null; startTime: number; endTime: number | null; @@ -3032,6 +3039,7 @@ export class StreamingMessageAggregator { status: "running", hookPath: data.hookPath, lines: [], + progress: null, exitCode: null, startTime: data.timestamp, endTime: null, @@ -3061,7 +3069,10 @@ export class StreamingMessageAggregator { this.initState.lines.shift(); this.initState.truncatedLines = (this.initState.truncatedLines ?? 0) + 1; } - this.initState.lines.push({ line, isError }); + this.initState.lines.push({ line, isError, ...(data.step === true ? { step: true } : {}) }); + if (data.step === true) { + this.initState.progress = null; + } // Throttle cache invalidation during fast streaming to avoid re-render per line. this.initOutputThrottleTimer ??= setTimeout(() => { @@ -3071,6 +3082,17 @@ export class StreamingMessageAggregator { return true; } + if (isInitProgress(data)) { + if (this.initState?.status === "running") { + this.initState.progress = { label: data.label, percent: data.percent }; + this.initOutputThrottleTimer ??= setTimeout(() => { + this.initOutputThrottleTimer = null; + this.invalidateCache(); + }, StreamingMessageAggregator.INIT_OUTPUT_THROTTLE_MS); + } + return true; + } + if (isInitEnd(data)) { this.clearReplayInitVisiblePrefix(); if (!this.initState) { @@ -3080,6 +3102,7 @@ export class StreamingMessageAggregator { this.initState.exitCode = data.exitCode; this.initState.status = data.exitCode === 0 ? "success" : "error"; this.initState.endTime = data.timestamp; + this.initState.progress = null; // Use backend truncation count if larger (covers replay of old data). if (data.truncatedLines && data.truncatedLines > (this.initState.truncatedLines ?? 0)) { this.initState.truncatedLines = data.truncatedLines; @@ -3830,7 +3853,6 @@ export class StreamingMessageAggregator { resultMessages = markRowsBeforeLatestContextBoundary(resultMessages); - // Add init state if present (ephemeral, appears at top) if (this.initState) { const durationMs = this.initState.endTime !== null @@ -3839,16 +3861,23 @@ export class StreamingMessageAggregator { const initMessage: DisplayedMessage = { type: "workspace-init", id: "workspace-init", - historySequence: -1, // Appears before all history + historySequence: -1, status: this.initState.status, hookPath: this.initState.hookPath, - lines: [...this.initState.lines], // Shallow copy for React.memo change detection + lines: [...this.initState.lines], + progress: this.initState.progress, exitCode: this.initState.exitCode, timestamp: this.initState.startTime, durationMs, truncatedLines: this.initState.truncatedLines, }; - resultMessages = [initMessage, ...resultMessages]; + // Creation belongs to the first user turn, even though init starts before it is persisted. + const insertionIndex = resultMessages.findIndex((message) => message.type === "user") + 1; + resultMessages = [ + ...resultMessages.slice(0, insertionIndex), + initMessage, + ...resultMessages.slice(insertionIndex), + ]; } // Return the full array diff --git a/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.ts b/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.ts index e2bc2aeb855..c659926419b 100644 --- a/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.ts +++ b/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.ts @@ -10,6 +10,7 @@ import { isGoalBudgetLimitedEvent, isInitEnd, isInitOutput, + isInitProgress, isInitStart, isMuxMessage, isQueuedMessageChanged, @@ -227,7 +228,13 @@ export function applyWorkspaceChatEventToAggregator( } // init-* and ChatXumMessage are handled via the aggregator's unified handleMessage. - if (isMuxMessage(event) || isInitStart(event) || isInitOutput(event) || isInitEnd(event)) { + if ( + isMuxMessage(event) || + isInitStart(event) || + isInitOutput(event) || + isInitProgress(event) || + isInitEnd(event) + ) { aggregator.handleMessage(event); return "immediate"; } diff --git a/src/browser/utils/messages/messageUtils.test.ts b/src/browser/utils/messages/messageUtils.test.ts index dd1022ac5ac..b468d2b7c8c 100644 --- a/src/browser/utils/messages/messageUtils.test.ts +++ b/src/browser/utils/messages/messageUtils.test.ts @@ -127,6 +127,7 @@ describe("shouldBypassDeferredMessages", () => { status: "running", hookPath: "/tmp/project/.mux/init", lines: [{ line: "Installing dependencies...", isError: false }], + progress: null, exitCode: null, timestamp: 1, durationMs: null, diff --git a/src/common/types/message.ts b/src/common/types/message.ts index a64cf5f2f8b..7254bac27e4 100644 --- a/src/common/types/message.ts +++ b/src/common/types/message.ts @@ -1390,10 +1390,11 @@ export type DisplayedMessage = | { type: "workspace-init"; id: string; // Display ID for UI/React keys - historySequence: number; // Position in message stream (-1 for ephemeral, non-persisted events) + historySequence: number; // -1 for the creation card placed after the first displayed user turn status: "running" | "success" | "error"; - hookPath: string; // Path to the init script being executed - lines: Array<{ line: string; isError: boolean }>; // Accumulated output lines (stderr tagged via isError) + hookPath: string; // Project path being initialized + lines: Array<{ line: string; isError: boolean; step?: boolean }>; + progress: { label: string; percent: number } | null; exitCode: number | null; // Final exit code (null while running) timestamp: number; durationMs: number | null; // Duration in milliseconds (null while running) diff --git a/src/common/utils/messages/retryEligibility.test.ts b/src/common/utils/messages/retryEligibility.test.ts index 2541c782b23..7a6db3440d6 100644 --- a/src/common/utils/messages/retryEligibility.test.ts +++ b/src/common/utils/messages/retryEligibility.test.ts @@ -87,6 +87,7 @@ describe("getLastNonDecorativeMessage", () => { status: "running", hookPath: ".mux/init", lines: [], + progress: null, exitCode: null, timestamp: Date.now(), durationMs: null, diff --git a/src/node/services/agentSkills/builtInSkillContent.generated.ts b/src/node/services/agentSkills/builtInSkillContent.generated.ts index 585ec6ad33d..7dd9c074b87 100644 --- a/src/node/services/agentSkills/builtInSkillContent.generated.ts +++ b/src/node/services/agentSkills/builtInSkillContent.generated.ts @@ -5774,11 +5774,9 @@ export const BUILTIN_SKILL_FILES: Record> = { "", "## Output", "", - "Init output appears in a banner at the top of the workspace. Click to expand/collapse the log. The banner shows:", + "The creation card appears after the first user message in the transcript. While setup runs, it shows a checklist of steps and checkout progress when available. Select **More details** to see the project path and stdout/stderr output.", "", - "- Script path (`.xum/init`)", - "- Status (running, success, or exit code on failure)", - "- Full stdout/stderr output", + "On success, the card collapses to **Workspace created** with the elapsed time. Click the header to expand the log. On failure, the card stays expanded and shows the exit code and error output.", "", "## Idempotency", "", diff --git a/src/node/services/initStateManager.test.ts b/src/node/services/initStateManager.test.ts index 4cbc62b30fc..9ce4c34b1b7 100644 --- a/src/node/services/initStateManager.test.ts +++ b/src/node/services/initStateManager.test.ts @@ -267,7 +267,9 @@ describe("InitStateManager", () => { expect(progress).toEqual([]); await manager.endInit(workspaceId, 0); - expect(await manager.readInitStatus(workspaceId)).toEqual(manager.getInitState(workspaceId)); + expect(await manager.readInitStatus(workspaceId)).toEqual( + manager.getInitState(workspaceId) ?? null + ); expect((await manager.readInitStatus(workspaceId))?.lines).toEqual(initialState?.lines); manager.clearInMemoryState(workspaceId); await manager.replayInit(workspaceId); diff --git a/src/node/services/initStateManager.ts b/src/node/services/initStateManager.ts index 9e66bafe520..14bf976fd6d 100644 --- a/src/node/services/initStateManager.ts +++ b/src/node/services/initStateManager.ts @@ -5,6 +5,7 @@ import type { WorkspaceInitEvent } from "@/common/orpc/types"; import { log } from "@/node/services/log"; import { INIT_HOOK_MAX_LINES } from "@/common/constants/toolLimits"; import { getErrorMessage } from "@/common/utils/errors"; +import { clamp } from "@/common/utils/clamp"; /** * Output line with timestamp for replay timing. @@ -287,7 +288,7 @@ export class InitStateManager extends EventEmitter { type: "init-progress", workspaceId, label, - percent: Math.max(0, Math.min(100, Math.round(percent))), + percent: clamp(Math.round(percent), 0, 100), timestamp: Date.now(), } satisfies WorkspaceInitEvent & { workspaceId: string }); } diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index b163f632425..c43722ffd86 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -192,7 +192,7 @@ describe("WorktreeManager.createWorkspace", () => { } const proc = realExecFile(file, args, options); if (file === "git" && args[2] === "worktree" && args[3] === "add") { - // --no-checkout usually emits only stderr, so supply stdout to cover both streams. + // Inject stdout so forwarding coverage does not depend on Git's output. const result = proc.result; Object.defineProperty(proc, "result", { value: result.then((output) => ({ diff --git a/src/node/worktree/gitProgress.test.ts b/src/node/worktree/gitProgress.test.ts index f3ae1000500..731b56ce7f1 100644 --- a/src/node/worktree/gitProgress.test.ts +++ b/src/node/worktree/gitProgress.test.ts @@ -22,7 +22,7 @@ describe("GitProgressParser", () => { { stage: "Updating files", percent: 12 }, { stage: "Updating files", percent: 34 }, ]); - expect(output).toEqual([]); + expect(output).toEqual(["Updating files: 34% (34/100)"]); }); it("deduplicates percentages while allowing a new stage at the same percentage", () => { diff --git a/src/node/worktree/gitProgress.ts b/src/node/worktree/gitProgress.ts index dc21d3ab72f..cd01bb08cb4 100644 --- a/src/node/worktree/gitProgress.ts +++ b/src/node/worktree/gitProgress.ts @@ -10,9 +10,10 @@ export class GitProgressParser { push(chunk: string): void { this.buffer += chunk; - const lines = this.buffer.split(/[\r\n]/); + const lines = this.buffer.split(/([\r\n])/); this.buffer = lines.pop() ?? ""; - for (const line of lines) { + for (let index = 0; index < lines.length; index += 2) { + const line = lines[index]; if (!line) continue; const match = /^(.+?):\s+(\d{1,3})%/.exec(line); if (!match) { @@ -27,7 +28,7 @@ export class GitProgressParser { this.lastPercent = percent; this.onProgress(stage, percent); } - if (done) this.onOutput(line); + if (done || lines[index + 1] === "\n") this.onOutput(line); } } diff --git a/tests/ipc/workspace/init.test.ts b/tests/ipc/workspace/init.test.ts index 2581c485520..dc8b8799f0e 100644 --- a/tests/ipc/workspace/init.test.ts +++ b/tests/ipc/workspace/init.test.ts @@ -360,15 +360,22 @@ describeIntegration("Workspace init hook", () => { // Should include workspace creation logs + hook output expect(status.lines).toEqual( expect.arrayContaining([ - { line: "Creating git worktree...", isError: false, timestamp: expect.any(Number) }, + { + line: "Creating git worktree...", + isError: false, + step: true, + timestamp: expect.any(Number), + }, { line: "Worktree created successfully", isError: false, + step: true, timestamp: expect.any(Number), }, expect.objectContaining({ line: expect.stringMatching(/Running init hook:/), isError: false, + step: true, }), { line: "Installing dependencies", isError: false, timestamp: expect.any(Number) }, { line: "Done!", isError: false, timestamp: expect.any(Number) }, diff --git a/tests/ui/chat/initMessage.test.ts b/tests/ui/chat/initMessage.test.ts new file mode 100644 index 00000000000..13c25bf8467 --- /dev/null +++ b/tests/ui/chat/initMessage.test.ts @@ -0,0 +1,114 @@ +import "../dom"; +import { createElement } from "react"; +import { afterEach, beforeEach, describe, expect, test } from "@jest/globals"; +import { cleanup, fireEvent, render } from "@testing-library/react"; +import { InitMessage } from "@/browser/features/Messages/InitMessage"; +import type { DisplayedMessage } from "@/common/types/message"; +import { installDom } from "../dom"; + +type Init = Extract; +const running: Init = { + type: "workspace-init", + id: "init", + historySequence: -1, + status: "running", + hookPath: "/project", + timestamp: 1, + durationMs: null, + exitCode: null, + progress: { label: "Checkout", percent: 87 }, + lines: [ + { line: "Prepare", step: true, isError: false }, + { line: "Checkout", step: true, isError: false }, + { line: "Output from setup", isError: false }, + ], +}; +const succeeded: Init = { + ...running, + status: "success", + exitCode: 0, + durationMs: 1000, + progress: null, +}; +const failed: Init = { + ...running, + status: "error", + exitCode: 1, + durationMs: 1000, + progress: null, + lines: [...running.lines, { line: "Setup error", isError: true }], +}; + +describe("workspace creation card", () => { + let cleanupDom: () => void; + beforeEach(() => { + cleanupDom = installDom(); + }); + afterEach(() => { + cleanup(); + cleanupDom(); + }); + + test("shows checklist progress, hides raw details, and auto-collapses on success", () => { + const view = render(createElement(InitMessage, { message: running })); + expect(view.getByRole("progressbar").getAttribute("aria-valuenow")).toBe("87"); + expect(view.getAllByLabelText("Completed")).toHaveLength(1); + expect(view.getByLabelText("In progress")).toBeTruthy(); + expect(view.queryByText("Output from setup")).toBeNull(); + view.rerender(createElement(InitMessage, { message: succeeded })); + const header = view.getByRole("button"); + expect(header.getAttribute("aria-expanded")).toBe("false"); + expect(view.queryByRole("list")).toBeNull(); + fireEvent.click(header); + expect(view.getByText("Output from setup")).toBeTruthy(); + expect(view.getAllByLabelText("Completed")).toHaveLength(2); + expect(view.queryByRole("progressbar")).toBeNull(); + fireEvent.click(header); + expect(view.queryByText("Output from setup")).toBeNull(); + }); + + test("opens details on failure and tags only stderr as error output", () => { + const view = render(createElement(InitMessage, { message: running })); + view.rerender(createElement(InitMessage, { message: failed })); + expect( + view.getByRole("button", { name: /Workspace setup failed/ }).getAttribute("aria-expanded") + ).toBe("true"); + expect(view.getByText("Setup error").classList.contains("text-init-output-error-text")).toBe( + true + ); + expect( + view.getByText("Output from setup").classList.contains("text-init-output-error-text") + ).toBe(false); + expect(view.getByLabelText("Failed")).toBeTruthy(); + }); + + test("keeps explicit expansion and detail choices across status changes", () => { + const view = render(createElement(InitMessage, { message: running })); + const header = view.getByRole("button", { name: /Creating workspace/ }); + fireEvent.click(header); + fireEvent.click(header); + const details = view.getByRole("button", { name: "More details" }); + fireEvent.click(details); + expect(view.getByText("Output from setup")).toBeTruthy(); + fireEvent.click(details); + view.rerender(createElement(InitMessage, { message: succeeded })); + expect(view.getByRole("list")).toBeTruthy(); + expect(view.queryByText("Output from setup")).toBeNull(); + fireEvent.click(view.getByRole("button", { name: "More details" })); + expect(view.getByText("Output from setup")).toBeTruthy(); + }); + + test("renders legacy logs directly after expanding without a checklist or details toggle", () => { + const legacy: Init = { + ...succeeded, + lines: [{ line: "Legacy hook output", isError: false }], + truncatedLines: 9, + }; + const view = render(createElement(InitMessage, { message: legacy })); + fireEvent.click(view.getByRole("button")); + expect(view.getByText("Legacy hook output")).toBeTruthy(); + expect(view.queryByRole("list")).toBeNull(); + expect(view.getAllByRole("button")).toHaveLength(1); + expect(view.container.querySelector("pre")?.textContent).toContain("9 earlier lines truncated"); + }); +}); From 5eac8a4d5e55e9b0c8f89c36f02f0818995e8718 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:07:55 +0000 Subject: [PATCH 04/17] fix(worktree): disable git's progress delay for checkout git only prints checkout progress after 2s, so the creation card's progress bar never appeared for typical repos (a 12k-file checkout finished in ~2s during UAT). Force immediate progress for the worktree checkout only. --- src/node/worktree/WorktreeManager.test.ts | 13 ++++++------- src/node/worktree/WorktreeManager.ts | 7 ++++++- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index c43722ffd86..63bc7c99d99 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -187,8 +187,6 @@ describe("WorktreeManager.createWorkspace", () => { if (file === "git" && args[2] === "checkout") { checkoutStarted = true; expect(existsSync(path.join(workspacePath, "README.md"))).toBe(false); - options?.onStderrData?.("Updating files: 25% (1/4)\rUpdating fi"); - options?.onStderrData?.("les: 100% (4/4), done.\n"); } const proc = realExecFile(file, args, options); if (file === "git" && args[2] === "worktree" && args[3] === "add") { @@ -245,12 +243,13 @@ describe("WorktreeManager.createWorkspace", () => { expect(existsSync(hookMarker)).toBe(false); expect(stdout).toContain("worktree metadata ready"); expect(stderr.some((line) => line.includes("Preparing worktree"))).toBe(true); - expect(stderr).toContain("Updating files: 100% (4/4), done."); + // Real git progress: a one-file checkout only reports progress because the + // checkout disables git's 2s progress delay. + expect(stderr.some((line) => /^Updating files: 100% \(1\/1\), done\.$/.test(line))).toBe( + true + ); expect(stderr.some((line) => line.includes(branchName))).toBe(true); - expect(progress).toEqual([ - ["Updating files", 25], - ["Updating files", 100], - ]); + expect(progress).toEqual([["Updating files", 100]]); } finally { execSpy.mockRestore(); await fixture.cleanup(); diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index 11c4b509df8..f943d35cc54 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -205,7 +205,12 @@ export class WorktreeManager { using checkoutProc = execFileAsync( "git", ["-C", workspacePath, "checkout", "--progress", "--force", branchName], - { ...noHooksEnv, onStderrData: (chunk) => progress.push(chunk) } + { + ...noHooksEnv, + // git delays progress output by 2s, which hides it for most checkouts. + env: { ...noHooksEnv?.env, GIT_PROGRESS_DELAY: "0" }, + onStderrData: (chunk) => progress.push(chunk), + } ); const { stdout } = await checkoutProc.result; for (const line of stdout.split(/[\r\n]/)) { From 4348f56b1b33c4c77b59829f7019be3f40effac1 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:16:20 +0000 Subject: [PATCH 05/17] refactor: polish workspace creation card --- .../messages/StreamingMessageAggregator.ts | 25 ++++++++----------- src/node/services/initStateManager.ts | 9 +++---- src/node/services/workspaceService.ts | 8 ++---- 3 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/browser/utils/messages/StreamingMessageAggregator.ts b/src/browser/utils/messages/StreamingMessageAggregator.ts index 3e33610c2b6..4a97915f9ea 100644 --- a/src/browser/utils/messages/StreamingMessageAggregator.ts +++ b/src/browser/utils/messages/StreamingMessageAggregator.ts @@ -3069,26 +3069,26 @@ export class StreamingMessageAggregator { this.initState.lines.shift(); this.initState.truncatedLines = (this.initState.truncatedLines ?? 0) + 1; } - this.initState.lines.push({ line, isError, ...(data.step === true ? { step: true } : {}) }); + this.initState.lines.push({ line, isError, step: data.step ? true : undefined }); if (data.step === true) { this.initState.progress = null; } // Throttle cache invalidation during fast streaming to avoid re-render per line. - this.initOutputThrottleTimer ??= setTimeout(() => { - this.initOutputThrottleTimer = null; - this.invalidateCache(); - }, StreamingMessageAggregator.INIT_OUTPUT_THROTTLE_MS); + this.initOutputThrottleTimer ??= setTimeout( + () => this.flushPendingInitOutput(), + StreamingMessageAggregator.INIT_OUTPUT_THROTTLE_MS + ); return true; } if (isInitProgress(data)) { if (this.initState?.status === "running") { this.initState.progress = { label: data.label, percent: data.percent }; - this.initOutputThrottleTimer ??= setTimeout(() => { - this.initOutputThrottleTimer = null; - this.invalidateCache(); - }, StreamingMessageAggregator.INIT_OUTPUT_THROTTLE_MS); + this.initOutputThrottleTimer ??= setTimeout( + () => this.flushPendingInitOutput(), + StreamingMessageAggregator.INIT_OUTPUT_THROTTLE_MS + ); } return true; } @@ -3873,11 +3873,8 @@ export class StreamingMessageAggregator { }; // Creation belongs to the first user turn, even though init starts before it is persisted. const insertionIndex = resultMessages.findIndex((message) => message.type === "user") + 1; - resultMessages = [ - ...resultMessages.slice(0, insertionIndex), - initMessage, - ...resultMessages.slice(insertionIndex), - ]; + resultMessages = resultMessages.slice(); + resultMessages.splice(insertionIndex, 0, initMessage); } // Return the full array diff --git a/src/node/services/initStateManager.ts b/src/node/services/initStateManager.ts index 14bf976fd6d..0a64b869b7d 100644 --- a/src/node/services/initStateManager.ts +++ b/src/node/services/initStateManager.ts @@ -142,7 +142,7 @@ export class InitStateManager extends EventEmitter { workspaceId, line: timedLine.line, isError: timedLine.isError, - ...(timedLine.step ? { step: true } : {}), + step: timedLine.step ? true : undefined, timestamp: timedLine.timestamp, // Use original timestamp for replay lineNumber: truncatedLines + index, replay: true, @@ -257,7 +257,7 @@ export class InitStateManager extends EventEmitter { const timestamp = Date.now(); const lineNumber = (state.truncatedLines ?? 0) + state.lines.length; - const timedLine: TimedLine = { line, isError, timestamp, ...(step ? { step: true } : {}) }; + const timedLine: TimedLine = { line, isError, timestamp, step: step || undefined }; // Truncation: keep only the most recent MAX_LINES if (state.lines.length >= INIT_HOOK_MAX_LINES) { @@ -270,10 +270,7 @@ export class InitStateManager extends EventEmitter { this.emit("init-output", { type: "init-output", workspaceId, - line, - isError, - ...(step ? { step: true } : {}), - timestamp, + ...timedLine, lineNumber, } satisfies WorkspaceInitEvent & { workspaceId: string }); } diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index ceb51df7c86..1df060d43c1 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -4012,12 +4012,8 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { } this.initStateManager.appendOutput(workspaceId, message, false, true); }, - logProgress: (label: string, percent: number) => { - if (!hasInitState()) { - return; - } - this.initStateManager.reportProgress(workspaceId, label, percent); - }, + logProgress: (label: string, percent: number) => + this.initStateManager.reportProgress(workspaceId, label, percent), logStdout: (line: string) => { if (!hasInitState()) { return; From 02ec2c2e86ea777c58f5f01053315683cb6899b4 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:44:39 +0000 Subject: [PATCH 06/17] fix: address Codex review on the creation card - Flush the aggregator's throttled init cache right before the coalesced idle bump so a lone init-progress or init-output event is never rendered from the stale cached row. - Let checkout failures flow through the outer worktree rollback path so a failed cleanup is reported instead of swallowed. - Drop the progress bar width transition (no unrequested animation). --- .../components/ProgressBar/ProgressBar.tsx | 5 +- src/browser/stores/WorkspaceStore.test.ts | 54 +++++++++++++++++++ src/browser/stores/WorkspaceStore.ts | 32 +++++++---- src/node/worktree/WorktreeManager.ts | 15 +----- 4 files changed, 77 insertions(+), 29 deletions(-) diff --git a/src/browser/components/ProgressBar/ProgressBar.tsx b/src/browser/components/ProgressBar/ProgressBar.tsx index 137a5813890..79a4a9c7441 100644 --- a/src/browser/components/ProgressBar/ProgressBar.tsx +++ b/src/browser/components/ProgressBar/ProgressBar.tsx @@ -16,10 +16,7 @@ export function ProgressBar(props: ProgressBarProps) { aria-label={props["aria-label"]} className={cn("bg-init-output-bg h-1.5 overflow-hidden rounded-full", props.className)} > -
+
); } diff --git a/src/browser/stores/WorkspaceStore.test.ts b/src/browser/stores/WorkspaceStore.test.ts index ccbe1cfa18f..731d8c960df 100644 --- a/src/browser/stores/WorkspaceStore.test.ts +++ b/src/browser/stores/WorkspaceStore.test.ts @@ -3130,6 +3130,60 @@ describe("WorkspaceStore", () => { expect(stayedVisibleAfterCaughtUp).toBe(true); }); + it("shows init progress on the coalesced bump without waiting for the aggregator throttle", async () => { + const workspaceId = "workspace-init-progress-bump"; + let releaseProgress: (() => void) | undefined; + const progressAtBump: Array = []; + + const readInitProgress = (): number | null => { + const initMessage = store + .getWorkspaceState(workspaceId) + .messages.find( + (message): message is Extract => + message.type === "workspace-init" + ); + return initMessage?.progress?.percent ?? null; + }; + + mockChatStreamFor(workspaceId, async function* () { + yield { type: "caught-up" }; + await Promise.resolve(); + yield { type: "init-start", hookPath: "/tmp/project", timestamp: 1_000 }; + await new Promise((resolve) => { + releaseProgress = resolve; + }); + yield { + type: "init-output", + line: "Checking out files...", + step: true, + isError: false, + timestamp: 1_001, + }; + yield { type: "init-progress", label: "Updating files", percent: 87, timestamp: 1_002 }; + }); + + createAndAddWorkspace(store, workspaceId); + const unsubscribe = store.subscribeKey(workspaceId, () => { + progressAtBump.push(readInitProgress()); + }); + try { + // Reading state here caches the running card without progress, which is the + // stale snapshot a later bump must not re-render. + const sawRunningCard = await waitUntil( + () => readInitProgress() === null && releaseProgress !== undefined + ); + expect(sawRunningCard).toBe(true); + progressAtBump.length = 0; + + releaseProgress?.(); + + expect(await waitUntil(() => progressAtBump.length > 0)).toBe(true); + expect(progressAtBump[0]).toBe(87); + } finally { + unsubscribe(); + } + }); + it("active workspace still shows starting during legitimate startup gap", async () => { const workspaceId = "stream-starting-active-workspace"; diff --git a/src/browser/stores/WorkspaceStore.ts b/src/browser/stores/WorkspaceStore.ts index 5f5899224e0..204e690ba27 100644 --- a/src/browser/stores/WorkspaceStore.ts +++ b/src/browser/stores/WorkspaceStore.ts @@ -839,6 +839,7 @@ export class WorkspaceStore { // Idle callbacks keep high-frequency init logs from blocking the renderer. private deltaIdleHandles = new Map(); + private idleBumpPreludes = new Map void>(); private pendingStreamingMessageBump = new Map(); // Live keyed-channel key per workspace, so every stream-clearing path can @@ -1114,11 +1115,13 @@ export class WorkspaceStore { applyWorkspaceChatEventToAggregator(aggregator, data); // Init output can be very high-frequency (e.g. installs, rsync). Like stream/tool deltas, // we update aggregator state immediately but coalesce UI bumps to keep the renderer responsive. - this.scheduleIdleStateBump(workspaceId); + // The aggregator throttles its own cache invalidation separately, so flush it right before + // the bump or the bump can render the stale cached row. + this.scheduleIdleStateBump(workspaceId, () => aggregator.flushPendingInitOutput()); }, "init-progress": (workspaceId, aggregator, data) => { applyWorkspaceChatEventToAggregator(aggregator, data); - this.scheduleIdleStateBump(workspaceId); + this.scheduleIdleStateBump(workspaceId, () => aggregator.flushPendingInitOutput()); }, "init-end": (workspaceId, aggregator, data) => { applyWorkspaceChatEventToAggregator(aggregator, data); @@ -1629,19 +1632,28 @@ export class WorkspaceStore { * The "presentation clock" (useSmoothStreamingText) handles visual cadence * independently — do not collapse them into a single mechanism. */ - private scheduleIdleStateBump(workspaceId: string): void { + private scheduleIdleStateBump(workspaceId: string, beforeBump?: () => void): void { + // Record the prelude even when a bump is already scheduled so it runs on that bump. + if (beforeBump) { + this.idleBumpPreludes.set(workspaceId, beforeBump); + } // Skip if already scheduled if (this.deltaIdleHandles.has(workspaceId)) { return; } + const bump = () => { + this.deltaIdleHandles.delete(workspaceId); + const prelude = this.idleBumpPreludes.get(workspaceId); + this.idleBumpPreludes.delete(workspaceId); + prelude?.(); + this.states.bump(workspaceId); + }; + // requestIdleCallback is not available in some environments (e.g. Node-based unit tests). // Fall back to a regular timeout so we still throttle bumps. if (typeof requestIdleCallback !== "function") { - const handle = setTimeout(() => { - this.deltaIdleHandles.delete(workspaceId); - this.states.bump(workspaceId); - }, 0); + const handle = setTimeout(bump, 0); // eslint-disable-next-line local/no-chained-type-assertions -- grandfathered when the rule was introduced; fix the underlying type instead of copying this pattern this.deltaIdleHandles.set(workspaceId, handle as unknown as number); @@ -1649,10 +1661,7 @@ export class WorkspaceStore { } const handle = requestIdleCallback( - () => { - this.deltaIdleHandles.delete(workspaceId); - this.states.bump(workspaceId); - }, + bump, { timeout: 100 } // Force update within 100ms even if browser stays busy ); @@ -1940,6 +1949,7 @@ export class WorkspaceStore { } this.deltaIdleHandles.delete(workspaceId); } + this.idleBumpPreludes.delete(workspaceId); } /** diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index f943d35cc54..0dc1b33292d 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -189,6 +189,7 @@ export class WorktreeManager { ); const addResult = await addProc.result; createdBranch = !branchExists; + worktreeCreated = true; for (const line of addResult.stdout.split(/[\r\n]/)) { if (line) initLogger.logStdout(line); } @@ -216,23 +217,9 @@ export class WorktreeManager { for (const line of stdout.split(/[\r\n]/)) { if (line) initLogger.logStdout(line); } - } catch (error) { - try { - await this.rollbackFailedWorkspaceCreation({ - projectPath, - workspacePath, - branchName, - createdBranch, - trusted: params.trusted, - }); - } catch { - // Preserve the checkout error even when best-effort cleanup fails. - } - throw error; } finally { progress.flush(); } - worktreeCreated = true; initLogger.logStep("Worktree created successfully"); From 226742001f2f94e187222614434835469dd9b468 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:10:06 +0000 Subject: [PATCH 07/17] fix(worktree): keep worktree-add semantics for the streamed checkout - Point HEAD at an unborn ref before the progress checkout so trusted post-checkout hooks still receive the null old commit and new-worktree flag exactly as with a plain git worktree add. - Pass --no-recurse-submodules: linked-worktree submodule repos do not exist yet, and submodule.recurse=true made the checkout fail where worktree add succeeded. syncLocalGitSubmodules materializes them. --- src/node/worktree/WorktreeManager.test.ts | 69 +++++++++++++++++++++++ src/node/worktree/WorktreeManager.ts | 21 ++++++- 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index 63bc7c99d99..f8633dcb62b 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -412,6 +412,75 @@ describe("WorktreeManager.createWorkspace", () => { } }); + it("runs post-checkout with the new-worktree arguments of a plain worktree add", async () => { + const branchName = "feature-hook-contract"; + const fixture = await createWorktreeManagerFixture(); + const hookLog = path.join(fixture.rootDir, "post-checkout-args"); + try { + const hook = path.join(fixture.projectPath, ".git", "hooks", "post-checkout"); + await fsPromises.writeFile( + hook, + '#!/bin/sh\nprintf "%s %s %s" "$1" "$2" "$3" >> "' + hookLog + '"\n' + ); + await fsPromises.chmod(hook, 0o755); + const result = await fixture.manager.createWorkspace({ + projectPath: fixture.projectPath, + branchName, + trunkBranch: "main", + skipRemoteSync: true, + trusted: true, + initLogger: fixture.initLogger, + }); + expect(result.success).toBe(true); + const tip = execFileSync("git", ["rev-parse", branchName], { cwd: fixture.projectPath }) + .toString() + .trim(); + expect(await fsPromises.readFile(hookLog, "utf8")).toBe(`${"0".repeat(40)} ${tip} 1`); + } finally { + await fixture.cleanup(); + } + }, 20_000); + + it("checks out a linked worktree when submodule.recurse is enabled", async () => { + const fixture = await createWorktreeManagerFixture(); + try { + const submodulePath = path.join(fixture.rootDir, "sub"); + await fsPromises.mkdir(submodulePath); + initGitRepo(submodulePath); + execFileSync( + "git", + ["-c", "protocol.file.allow=always", "submodule", "add", "--quiet", submodulePath, "sub"], + { cwd: fixture.projectPath, stdio: "ignore" } + ); + execFileSync("git", ["commit", "-qm", "add submodule"], { + cwd: fixture.projectPath, + stdio: "ignore", + }); + execFileSync("git", ["config", "submodule.recurse", "true"], { cwd: fixture.projectPath }); + const result = await fixture.manager.createWorkspace({ + projectPath: fixture.projectPath, + branchName: "feature-submodules", + trunkBranch: "main", + skipRemoteSync: true, + trusted: true, + initLogger: fixture.initLogger, + // The later submodule sync clones from a local path; git only honors this + // permission from command-line scope, never from repository config. + env: { + GIT_CONFIG_COUNT: "1", + GIT_CONFIG_KEY_0: "protocol.file.allow", + GIT_CONFIG_VALUE_0: "always", + }, + }); + expect(result).toEqual({ + success: true, + workspacePath: fixture.manager.getWorkspacePath(fixture.projectPath, "feature-submodules"), + }); + } finally { + await fixture.cleanup(); + } + }, 20_000); + it("skips repo-configured upload-pack commands when project automation is disabled", async () => { const fixture = await createWorktreeManagerFixture(); const marker = path.join(fixture.rootDir, "upload-pack-ran"); diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index 0dc1b33292d..09c21f9332f 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "crypto"; import * as fsPromises from "fs/promises"; import * as path from "path"; import type { @@ -198,14 +199,32 @@ export class WorktreeManager { } initLogger.logStep("Checking out files..."); + // Point HEAD at an unborn ref first so the checkout reports the same post-checkout + // hook arguments as a plain `git worktree add` (null old commit, new-worktree flag). + using unbornProc = execFileAsync( + "git", + ["-C", workspacePath, "symbolic-ref", "HEAD", `refs/heads/xum-unborn-${randomUUID()}`], + noHooksEnv + ); + await unbornProc.result; const progress = new GitProgressParser( (stage, percent) => initLogger.logProgress?.(stage, percent), (line) => initLogger.logStderr(line) ); try { + // Submodule repos do not exist yet in a linked worktree, so recursion would fail; + // syncLocalGitSubmodules materializes them below, like `git worktree add` does. using checkoutProc = execFileAsync( "git", - ["-C", workspacePath, "checkout", "--progress", "--force", branchName], + [ + "-C", + workspacePath, + "checkout", + "--progress", + "--force", + "--no-recurse-submodules", + branchName, + ], { ...noHooksEnv, // git delays progress output by 2s, which hides it for most checkouts. From c68f1dd1d182459375f14eb03f6e4ade9c00a4ae Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:25:06 +0000 Subject: [PATCH 08/17] fix: classify git chatter as output and scroll revealed logs - Forward git's informational stderr (Preparing worktree, Updating files, Switched to branch) as plain output so a successful card never paints routine progress in the error color; creation failures are logged as stderr from the outer catch (skipping caller cancellation). - Scroll the raw log to its end whenever it mounts or grows, so a failed card that opens its details lands on the lines explaining the failure. --- src/browser/features/Messages/InitMessage.tsx | 4 +++- src/node/worktree/WorktreeManager.test.ts | 8 +++++--- src/node/worktree/WorktreeManager.ts | 14 +++++++++----- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/browser/features/Messages/InitMessage.tsx b/src/browser/features/Messages/InitMessage.tsx index afd97651297..4a91378ac41 100644 --- a/src/browser/features/Messages/InitMessage.tsx +++ b/src/browser/features/Messages/InitMessage.tsx @@ -23,8 +23,10 @@ export function InitMessage(props: InitMessageProps) { const detailsExpanded = steps.length === 0 || (detailsOverride ?? !isRunning); const preRef = useRef(null); + // Keep the newest output in view: while lines stream in, and when a finished (often + // failed) card first reveals its log, whose last lines explain the outcome. useEffect(() => { - if (isRunning && preRef.current) { + if (preRef.current) { preRef.current.scrollTop = preRef.current.scrollHeight; } }, [isRunning, message.lines.length, expanded, detailsExpanded]); diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index f8633dcb62b..7e3cbd7cef6 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -242,13 +242,15 @@ describe("WorktreeManager.createWorkspace", () => { ).toBe(branchName); expect(existsSync(hookMarker)).toBe(false); expect(stdout).toContain("worktree metadata ready"); - expect(stderr.some((line) => line.includes("Preparing worktree"))).toBe(true); + expect(stdout.some((line) => line.includes("Preparing worktree"))).toBe(true); // Real git progress: a one-file checkout only reports progress because the // checkout disables git's 2s progress delay. - expect(stderr.some((line) => /^Updating files: 100% \(1\/1\), done\.$/.test(line))).toBe( + expect(stdout.some((line) => /^Updating files: 100% \(1\/1\), done\.$/.test(line))).toBe( true ); - expect(stderr.some((line) => line.includes(branchName))).toBe(true); + expect(stdout.some((line) => line.includes(branchName))).toBe(true); + // git's routine stderr chatter must not render as error output. + expect(stderr).toEqual([]); expect(progress).toEqual([["Updating files", 100]]); } finally { execSpy.mockRestore(); diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index 09c21f9332f..e540f0705ae 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -191,12 +191,11 @@ export class WorktreeManager { const addResult = await addProc.result; createdBranch = !branchExists; worktreeCreated = true; - for (const line of addResult.stdout.split(/[\r\n]/)) { + // git reports routine progress ("Preparing worktree", "Updating files") on stderr; + // failures surface through the thrown error below, so none of this is error output. + for (const line of `${addResult.stdout}\n${addResult.stderr}`.split(/[\r\n]/)) { if (line) initLogger.logStdout(line); } - for (const line of addResult.stderr.split(/[\r\n]/)) { - if (line) initLogger.logStderr(line); - } initLogger.logStep("Checking out files..."); // Point HEAD at an unborn ref first so the checkout reports the same post-checkout @@ -209,7 +208,7 @@ export class WorktreeManager { await unbornProc.result; const progress = new GitProgressParser( (stage, percent) => initLogger.logProgress?.(stage, percent), - (line) => initLogger.logStderr(line) + (line) => initLogger.logStdout(line) ); try { // Submodule repos do not exist yet in a linked worktree, so recursion would fail; @@ -268,6 +267,11 @@ export class WorktreeManager { return { success: true, workspacePath }; } catch (error) { const errorMessage = getErrorMessage(error); + if (!isAbortError(error, params.abortSignal)) { + for (const line of errorMessage.split(/\r?\n/)) { + if (line) initLogger.logStderr(line); + } + } if (!worktreeCreated) { return { From 8704b6179488dfed086ab825ebaf326cffb148b3 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:46:43 +0000 Subject: [PATCH 09/17] tests: let the creation card sit between user-0 and the truncation seam The transcript truncation UI test assumed the first hidden-history marker directly followed user-0; the workspace creation card now renders there. Assert the seam position (after user-0's turn, before user-1) instead of adjacency. --- tests/ui/chat/truncation.test.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/ui/chat/truncation.test.ts b/tests/ui/chat/truncation.test.ts index 6a04acb1611..d549ff0c372 100644 --- a/tests/ui/chat/truncation.test.ts +++ b/tests/ui/chat/truncation.test.ts @@ -161,8 +161,13 @@ describe("Chat truncation UI", () => { node.textContent?.match(/some messages are hidden for performance/i) ); expect(indicatorIndex).toBeGreaterThan(0); - expect(messageBlocks[indicatorIndex - 1]?.textContent).toContain("user-0"); - // The earliest marker still appears at the first omission seam. + // The earliest marker still appears at the first omission seam: after user-0's turn + // (which the workspace creation card follows) and before user-1. + const rowsBeforeIndicator = messageBlocks + .slice(0, indicatorIndex) + .map((node) => node.textContent ?? ""); + expect(rowsBeforeIndicator.some((text) => text.includes("user-0"))).toBe(true); + expect(rowsBeforeIndicator.some((text) => text.includes("user-1"))).toBe(false); expect(messageBlocks[indicatorIndex + 1]?.textContent).toContain("user-1"); // Verify assistant meta rows survive in the recent (non-truncated) section. From 5146dc9446297da818cd8869a596c9e65af5c69b Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:13:08 +0000 Subject: [PATCH 10/17] feat(worktree): defer local checkout into the init phase Local worktree creation used to populate the checkout inside create(), so every checkout progress event fired before the workspace was announced and the creation card could never show the bar. WorktreeManager.createWorkspace now takes deferMaterialization: it reserves the worktree (add --no-checkout, unborn HEAD, branch mapping) and returns, and the streamed checkout, .xumignore sync, fast-forward and submodule sync move to materializeWorkspace(), which WorkspaceService.create runs after announcing the workspace. Plugin-override sanitization for deferred worktrees runs after materialization and before the init hook, like task worktrees; a sanitize failure still tears the creation down. Fork, restore, tasks, multi-project and devcontainer creation stay eager, as does task(kind="workspace"), whose agentId validation reads the checkout under the task mutex. The new IPC test subscribes after create() resolves and asserts an init-progress event arrives, which fails on the previous head. --- src/node/runtime/Runtime.ts | 25 ++- src/node/runtime/WorktreeRuntime.ts | 21 ++ src/node/services/taskWorkspaceSeam.ts | 3 +- src/node/services/workspaceService.ts | 229 ++++++++++++++++------ src/node/services/workspaceTurnManager.ts | 5 +- src/node/worktree/WorktreeManager.test.ts | 110 +++++++++++ src/node/worktree/WorktreeManager.ts | 167 ++++++++++------ tests/ipc/helpers.ts | 6 +- tests/ipc/workspace/init.test.ts | 85 +++++++- 9 files changed, 525 insertions(+), 126 deletions(-) diff --git a/src/node/runtime/Runtime.ts b/src/node/runtime/Runtime.ts index 3c7e987a0f2..0ca8374b356 100644 --- a/src/node/runtime/Runtime.ts +++ b/src/node/runtime/Runtime.ts @@ -205,6 +205,18 @@ export interface WorkspaceCreationParams { env?: Record; /** Whether the project is trusted — when false, git hooks are disabled */ trusted?: boolean; + /** + * Return once the checkout is reserved and leave populating its files to + * materializeWorkspace(), so that work streams to a workspace that is already announced. + * Runtimes whose creation never populates files ignore this. + */ + deferMaterialization?: boolean; +} + +/** Creation-time decisions materializeWorkspace() needs to finish a deferred checkout. */ +export interface PendingMaterialization { + /** The branch already existed and should fast-forward to origin/ once checked out. */ + fastForwardFromOrigin: boolean; } /** @@ -215,6 +227,8 @@ export interface WorkspaceCreationResult { /** Absolute path to workspace (local path for LocalRuntime, remote path for SSHRuntime) */ workspacePath?: string; error?: string; + /** Set when deferMaterialization left populating the checkout to materializeWorkspace(). */ + pendingMaterialization?: PendingMaterialization; } /** @@ -475,7 +489,7 @@ export interface Runtime { /** * Create a workspace for this runtime (fast, returns immediately) - * - LocalRuntime: Creates git worktree + * - LocalRuntime: Creates git worktree (populating it can be deferred to materializeWorkspace) * - SSHRuntime: Creates remote directory only * Does NOT run init hook or sync files. * @param params Workspace creation parameters @@ -539,6 +553,15 @@ export interface Runtime { */ postCreateSetup?(params: WorkspaceInitParams): Promise; + /** + * Populate a checkout reserved by createWorkspace({ deferMaterialization: true }). + * Streams progress via initLogger; throws on failure and leaves the checkout registered. + */ + materializeWorkspace?( + params: WorkspaceInitParams, + pending: PendingMaterialization + ): Promise; + /** * Initialize workspace asynchronously (may be slow, streams progress) * - LocalRuntime: Runs init hook if present diff --git a/src/node/runtime/WorktreeRuntime.ts b/src/node/runtime/WorktreeRuntime.ts index ef7a48f0860..0659d1a3717 100644 --- a/src/node/runtime/WorktreeRuntime.ts +++ b/src/node/runtime/WorktreeRuntime.ts @@ -1,6 +1,7 @@ import type { EnsureReadyOptions, EnsureReadyResult, + PendingMaterialization, WorkspaceCreationParams, WorkspaceCreationResult, WorkspaceInitParams, @@ -103,9 +104,29 @@ export class WorktreeRuntime extends LocalBaseRuntime { abortSignal: params.abortSignal, env: params.env, trusted: params.trusted, + deferMaterialization: params.deferMaterialization, }); } + async materializeWorkspace( + params: WorkspaceInitParams, + pending: PendingMaterialization + ): Promise { + return this.worktreeManager.materializeWorkspace( + { + projectPath: params.projectPath, + workspacePath: params.workspacePath, + branchName: params.branchName, + trunkBranch: params.trunkBranch, + initLogger: params.initLogger, + abortSignal: params.abortSignal, + env: params.env, + trusted: params.trusted, + }, + pending + ); + } + async initWorkspace(params: WorkspaceInitParams): Promise { return this.initLocalWorkspace(params, "worktree"); } diff --git a/src/node/services/taskWorkspaceSeam.ts b/src/node/services/taskWorkspaceSeam.ts index 373c9d1440b..9b23d610c14 100644 --- a/src/node/services/taskWorkspaceSeam.ts +++ b/src/node/services/taskWorkspaceSeam.ts @@ -507,7 +507,8 @@ export interface WorkspaceProvisioningHost { runtimeConfig?: RuntimeConfig, subProjectPath?: string, pendingAutoTitle?: boolean, - tags?: Record + tags?: Record, + options?: { awaitMaterialization?: boolean } ): Promise>; sanitizeMaterializedTaskWorkspace( workspaceId: string, diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 1df060d43c1..8ac8e2505b6 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -227,6 +227,12 @@ import { isWorkflowRunEmittingToolName, } from "@/common/utils/workflowRunMessages"; import type { RuntimeConfig } from "@/common/types/runtime"; +import type { + PendingMaterialization, + Runtime, + WorkspaceCreationResult, + WorkspaceInitParams, +} from "@/node/runtime/Runtime"; import { hasSrcBaseDir, getSrcBaseDir, @@ -3033,6 +3039,116 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { return false; } + /** + * Undo a registration whose checkout could not be sanitized: the config entry, the + * worktree this creation made, and the in-memory state registered for it. Returns whether + * the entry is provably gone. + */ + private async abortUnsanitizedCreation(args: { + workspaceId: string; + runtime: Runtime; + runtimeConfig: RuntimeConfig; + projectPath: string; + workspaceName: string; + trusted: boolean; + initAbortController: AbortController; + }): Promise { + const { workspaceId } = args; + const rolledBack = await this.rollbackUnsanitizedWorkspaceRegistration(workspaceId); + // WORKTREE runtimes created a fresh checkout; without deleting it, + // retrying the same branch collides with the orphaned worktree and leaks + // a suffixed checkout per attempt. LocalRuntime registered an EXISTING + // user directory, which must be preserved (its deleteWorkspace is a + // no-op by design, but we never call it here to keep that contract + // explicit). Only after a successful config rollback: while the entry + // persists, the checkout is still referenced. + if (rolledBack && isWorktreeRuntime(args.runtimeConfig)) { + const deleteResult = await args.runtime + .deleteWorkspace( + args.projectPath, + // Worktree directories are named after the sanitized workspace + // name (branch names may contain "/"). + args.workspaceName, + false, + undefined, + args.trusted + ) + .catch((error: unknown) => ({ + success: false as const, + error: getErrorMessage(error), + })); + if (!deleteResult.success) { + log.warn("Failed to remove created worktree after sanitization aborted creation", { + workspaceId, + error: deleteResult.error, + }); + } + } + // Tear down the in-memory state registered earlier in this creation + // (session, init record, abort controller) exactly like workspace + // removal would; without this every aborted retry against the same bad + // file leaks another unreachable session for the process lifetime. + args.initAbortController.abort(); + this.initAbortControllers.delete(workspaceId); + this.initStateManager.clearInMemoryState(workspaceId); + await this.disposeSession(workspaceId); + return rolledBack; + } + + /** + * Background init for a worktree announced before its files existed: populate the + * checkout (streaming progress to the creation card), then sanitize plugin overrides + * exactly as task worktrees do after materialization, then run the ordinary init. + * A checkout failure fails the init like any deferred runtime's sync failure; a + * sanitize failure tears the creation down, as it would have at registration time. + */ + private async materializeDeferredCheckout(args: { + workspaceId: string; + runtime: Runtime; + runtimeConfig: RuntimeConfig; + workspaceName: string; + initParams: WorkspaceInitParams; + pending: PendingMaterialization; + initAbortController: AbortController; + }): Promise { + const { workspaceId, runtime, initParams } = args; + assert( + runtime.materializeWorkspace !== undefined, + "materializeDeferredCheckout: runtime cannot materialize" + ); + try { + await runtime.materializeWorkspace(initParams, args.pending); + } catch (error) { + log.error(`Workspace checkout failed for ${workspaceId}:`, { error }); + initParams.initLogger.logStderr(`Initialization failed: ${getErrorMessage(error)}`); + initParams.initLogger.logComplete(-1); + return; + } + const sanitizeError = await this.sanitizeMaterializedTaskWorkspace( + workspaceId, + initParams.workspacePath, + args.runtimeConfig + ); + if (sanitizeError !== undefined) { + log.error(`Workspace creation aborted for ${workspaceId}: ${sanitizeError}`); + initParams.initLogger.logStderr(sanitizeError); + await this.abortUnsanitizedCreation({ + workspaceId, + runtime, + runtimeConfig: args.runtimeConfig, + projectPath: initParams.projectPath, + workspaceName: args.workspaceName, + trusted: initParams.trusted ?? false, + initAbortController: args.initAbortController, + }); + initParams.initLogger.logComplete(-1); + // Already announced, unlike a registration-time abort. + this.emit("metadata", { workspaceId, metadata: null }); + return; + } + await runBackgroundInit(runtime, initParams, workspaceId, log); + } + setWorkspaceGoalService(service: WorkspaceGoalService): void { this.workspaceGoalService = service; } @@ -4887,7 +5003,15 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { runtimeConfig?: RuntimeConfig, subProjectPath?: string, pendingAutoTitle?: boolean, - tags?: Record + tags?: Record, + options?: { + /** + * Resolve only once the checkout's files exist. By default a local worktree is + * announced first so its checkout progress streams to the creation card; callers that + * read the checkout right after create() (and cannot wait for init) opt out. + */ + awaitMaterialization?: boolean; + } ): Promise> { if (tags != null) { for (const [tagKey, tagValue] of Object.entries(tags)) { @@ -5024,7 +5148,7 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { let finalBranchName = resolvedBranchName; let finalWorkspaceName = initialWorkspaceName; const hasSanitizedWorkspaceName = finalBranchName !== finalWorkspaceName; - let createResult: { success: boolean; workspacePath?: string; error?: string }; + let createResult: WorkspaceCreationResult; // If runtime uses config-level collision detection (e.g., Coder - can't reach host), // check against existing workspace names before createWorkspace. @@ -5068,6 +5192,8 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { abortSignal: initAbortController.signal, env: createEnv, trusted: projectConfig.trusted ?? false, + deferMaterialization: + options?.awaitMaterialization !== true && runtime.materializeWorkspace !== undefined, }); if (createResult.success) break; @@ -5135,15 +5261,19 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { // registration pending BEFORE the entry persists so an overlapping // creation for the same checkout cannot mistake the not-yet-sanitized // entry for a live sibling and skip its own sanitization. - const isHostLocalCheckout = - finalRuntimeConfig.type === "local" || finalRuntimeConfig.type === "worktree"; + // A deferred worktree has no files yet; it is sanitized once + // materialized, before its init hook (see materializeDeferredCheckout). + const pendingMaterialization = createResult!.pendingMaterialization; + const sanitizeAtRegistration = + (finalRuntimeConfig.type === "local" || finalRuntimeConfig.type === "worktree") && + pendingMaterialization === undefined; let completeMetadata: FrontendWorkspaceMetadata | undefined; - if (isHostLocalCheckout) { + if (sanitizeAtRegistration) { this.pendingPluginSanitizations.add(workspaceId); } let releaseRegistrationLock: (() => Promise) | undefined; try { - if (isHostLocalCheckout) { + if (sanitizeAtRegistration) { // Cross-process: persist + sanitize must not interleave with a // sibling process registering the same checkout (see // acquireRegistrationSanitizeLock). @@ -5191,52 +5321,21 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { // a failure aborts the creation so nothing stale ever activates. // SSH/container runtimes exec off-host, where plugin servers never // spawn (host-path containers only in v1). - if (isHostLocalCheckout) { + if (sanitizeAtRegistration) { const sanitizeError = await this.sanitizeStalePluginOverridesForNewWorkspace( workspaceId, createResult!.workspacePath ); if (sanitizeError !== undefined) { - const rolledBack = await this.rollbackUnsanitizedWorkspaceRegistration(workspaceId); - // WORKTREE runtimes created a fresh checkout above; without - // deleting it, retrying the same branch collides with the - // orphaned worktree and leaks a suffixed checkout per attempt. - // LocalRuntime registered an EXISTING user directory, which must - // be preserved (its deleteWorkspace is a no-op by design, but we - // never call it here to keep that contract explicit). Only after - // a successful config rollback: while the entry persists, the - // checkout is still referenced. - if (rolledBack && isWorktreeRuntime(finalRuntimeConfig)) { - const deleteResult = await runtime - .deleteWorkspace( - owningProjectPath, - // Worktree directories are named after the sanitized - // workspace name (branch names may contain "/"). - finalWorkspaceName, - false, - undefined, - projectConfig.trusted ?? false - ) - .catch((error: unknown) => ({ - success: false as const, - error: getErrorMessage(error), - })); - if (!deleteResult.success) { - log.warn("Failed to remove created worktree after sanitization aborted creation", { - workspaceId, - error: deleteResult.error, - }); - } - } - // Tear down the in-memory state registered earlier in this - // creation (session, init record, abort controller) exactly like - // workspace removal would; without this every aborted retry - // against the same bad file leaks another unreachable session - // for the process lifetime. - initAbortController.abort(); - this.initAbortControllers.delete(workspaceId); - this.initStateManager.clearInMemoryState(workspaceId); - await this.disposeSession(workspaceId); + const rolledBack = await this.abortUnsanitizedCreation({ + workspaceId, + runtime, + runtimeConfig: finalRuntimeConfig, + projectPath: owningProjectPath, + workspaceName: finalWorkspaceName, + trusted: projectConfig.trusted ?? false, + initAbortController, + }); initLogger.logComplete(-1); return Err( rolledBack @@ -5265,24 +5364,30 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { // If the user cancelled creation while create() was still in flight, avoid spawning // additional background work for a workspace that's already being removed. if (!this.removingWorkspaces.has(workspaceId) && !initAbortController.signal.aborted) { + const initParams: WorkspaceInitParams = { + projectPath: owningProjectPath, + branchName: finalBranchName, + trunkBranch: normalizedTrunkBranch, + workspacePath: createResult!.workspacePath, + initLogger, + env: secrets, + abortSignal: initAbortController.signal, + trusted: projectConfig.trusted ?? false, + }; // Retained (not just fired) so archive can await the hook process's actual exit. this.retainInitSettlement( workspaceId, - runBackgroundInit( - runtime, - { - projectPath: owningProjectPath, - branchName: finalBranchName, - trunkBranch: normalizedTrunkBranch, - workspacePath: createResult!.workspacePath, - initLogger, - env: secrets, - abortSignal: initAbortController.signal, - trusted: projectConfig.trusted ?? false, - }, - workspaceId, - log - ) + pendingMaterialization + ? this.materializeDeferredCheckout({ + workspaceId, + runtime, + runtimeConfig: finalRuntimeConfig, + workspaceName: finalWorkspaceName, + initParams, + pending: pendingMaterialization, + initAbortController, + }) + : runBackgroundInit(runtime, initParams, workspaceId, log) ); } else { initAbortController.abort(); diff --git a/src/node/services/workspaceTurnManager.ts b/src/node/services/workspaceTurnManager.ts index 33fafb43ea1..926971bf80e 100644 --- a/src/node/services/workspaceTurnManager.ts +++ b/src/node/services/workspaceTurnManager.ts @@ -1222,7 +1222,10 @@ export class WorkspaceTurnManager { parentMeta.runtimeConfig, parentMeta.subProjectPath, false, - tags + tags, + // The agentId validation below reads the target checkout under the task mutex, so + // a local worktree must be populated before create() resolves. + { awaitMaterialization: true } ); if (!createResult.success) { return Err(`Task.createWorkspaceTurn: workspace create failed (${createResult.error})`); diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index 7e3cbd7cef6..64a39967d94 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -414,6 +414,116 @@ describe("WorktreeManager.createWorkspace", () => { } }); + it("reserves the worktree and populates it later when materialization is deferred", async () => { + const branchName = "feature-deferred"; + const fixture = await createWorktreeManagerFixture({ existingBranchName: branchName }); + const steps: string[] = []; + const progress: Array<[string, number]> = []; + const initLogger = { + ...fixture.initLogger, + logStep: (message: string) => steps.push(message), + logProgress: (label: string, percent: number) => progress.push([label, percent]), + }; + const workspacePath = fixture.manager.getWorkspacePath(fixture.projectPath, branchName); + const hookLog = path.join(fixture.rootDir, "post-checkout-args"); + try { + const hook = path.join(fixture.projectPath, ".git", "hooks", "post-checkout"); + await fsPromises.writeFile( + hook, + '#!/bin/sh\nprintf "%s %s %s" "$1" "$2" "$3" >> "' + hookLog + '"\n' + ); + await fsPromises.chmod(hook, 0o755); + + const result = await fixture.manager.createWorkspace({ + projectPath: fixture.projectPath, + branchName, + trunkBranch: "main", + skipRemoteSync: true, + trusted: true, + initLogger, + deferMaterialization: true, + }); + expect(result).toEqual({ + success: true, + workspacePath, + pendingMaterialization: { fastForwardFromOrigin: false }, + }); + // Reserved but empty: registered with git, no files, no checkout activity yet. + expect( + execFileSync("git", ["worktree", "list", "--porcelain"], { + cwd: fixture.projectPath, + }).toString() + ).toContain(workspacePath); + expect(existsSync(path.join(workspacePath, "README.md"))).toBe(false); + expect(existsSync(hookLog)).toBe(false); + expect(steps).not.toContain("Checking out files..."); + expect(progress).toEqual([]); + + await fixture.manager.materializeWorkspace( + { + projectPath: fixture.projectPath, + workspacePath, + branchName, + trunkBranch: "main", + trusted: true, + initLogger, + }, + result.pendingMaterialization! + ); + expect(await fsPromises.readFile(path.join(workspacePath, "README.md"), "utf8")).toBe( + "hello\n" + ); + expect( + execFileSync("git", ["status", "--porcelain"], { cwd: workspacePath }).toString() + ).toBe(""); + expect( + execFileSync("git", ["branch", "--show-current"], { cwd: workspacePath }).toString().trim() + ).toBe(branchName); + expect(progress).toEqual([["Updating files", 100]]); + expect(steps).toContain("Checking out files..."); + // The deferred checkout still reports itself to hooks as a fresh worktree add. + const tip = execFileSync("git", ["rev-parse", branchName], { cwd: fixture.projectPath }) + .toString() + .trim(); + expect(await fsPromises.readFile(hookLog, "utf8")).toBe(`${"0".repeat(40)} ${tip} 1`); + } finally { + await fixture.cleanup(); + } + }, 20_000); + + it("deletes a reserved worktree that was never materialized", async () => { + const branchName = "feature-deferred-cancelled"; + const fixture = await createWorktreeManagerFixture(); + try { + const result = await fixture.manager.createWorkspace({ + projectPath: fixture.projectPath, + branchName, + trunkBranch: "main", + skipRemoteSync: true, + trusted: true, + initLogger: fixture.initLogger, + deferMaterialization: true, + }); + expect(result.success).toBe(true); + const deleteResult = await fixture.manager.deleteWorkspace( + fixture.projectPath, + branchName, + false, + true + ); + expect(deleteResult.success).toBe(true); + const workspacePath = fixture.manager.getWorkspacePath(fixture.projectPath, branchName); + expect(existsSync(workspacePath)).toBe(false); + expect( + execFileSync("git", ["branch", "--list", branchName], { cwd: fixture.projectPath }) + .toString() + .trim() + ).toBe(""); + } finally { + await fixture.cleanup(); + } + }, 20_000); + it("runs post-checkout with the new-worktree arguments of a plain worktree add", async () => { const branchName = "feature-hook-contract"; const fixture = await createWorktreeManagerFixture(); diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index e540f0705ae..353ce7a1c62 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -2,6 +2,7 @@ import { randomUUID } from "crypto"; import * as fsPromises from "fs/promises"; import * as path from "path"; import type { + PendingMaterialization, WorkspaceCreationResult, WorkspaceForkParams, WorkspaceForkResult, @@ -96,6 +97,8 @@ export class WorktreeManager { abortSignal?: AbortSignal; env?: Record; trusted?: boolean; + /** See WorkspaceCreationParams.deferMaterialization. */ + deferMaterialization?: boolean; }): Promise { const { projectPath, branchName, trunkBranch, initLogger } = params; // Disable git hooks for untrusted projects (prevents post-checkout execution). @@ -171,7 +174,8 @@ export class WorktreeManager { params.abortSignal )); - // Separate checkout so large repositories can stream file materialization progress. + // Files are populated by a separate checkout (materializeWorkspace) so large repositories + // can stream progress, and so callers may announce the workspace before populating it. // Restore flows may supply an exact saved commit instead of the current trunk. const newBranchBase = startPoint ?? (shouldUseOrigin ? `origin/${trunkBranch}` : trunkBranch); using addProc = execFileAsync( @@ -197,71 +201,38 @@ export class WorktreeManager { if (line) initLogger.logStdout(line); } - initLogger.logStep("Checking out files..."); - // Point HEAD at an unborn ref first so the checkout reports the same post-checkout - // hook arguments as a plain `git worktree add` (null old commit, new-worktree flag). + // Point HEAD at an unborn ref so the checkout reports the same post-checkout hook + // arguments as a plain `git worktree add` (null old commit, new-worktree flag). using unbornProc = execFileAsync( "git", ["-C", workspacePath, "symbolic-ref", "HEAD", `refs/heads/xum-unborn-${randomUUID()}`], noHooksEnv ); await unbornProc.result; - const progress = new GitProgressParser( - (stage, percent) => initLogger.logProgress?.(stage, percent), - (line) => initLogger.logStdout(line) - ); - try { - // Submodule repos do not exist yet in a linked worktree, so recursion would fail; - // syncLocalGitSubmodules materializes them below, like `git worktree add` does. - using checkoutProc = execFileAsync( - "git", - [ - "-C", - workspacePath, - "checkout", - "--progress", - "--force", - "--no-recurse-submodules", - branchName, - ], - { - ...noHooksEnv, - // git delays progress output by 2s, which hides it for most checkouts. - env: { ...noHooksEnv?.env, GIT_PROGRESS_DELAY: "0" }, - onStderrData: (chunk) => progress.push(chunk), - } - ); - const { stdout } = await checkoutProc.result; - for (const line of stdout.split(/[\r\n]/)) { - if (line) initLogger.logStdout(line); - } - } finally { - progress.flush(); - } - - initLogger.logStep("Worktree created successfully"); - // Sync gitignored files declared in .xumignore (e.g. .env) - // before init hooks run so they have access to secrets/config - initLogger.logStep("Syncing .xumignore files..."); - await syncXumignoreFiles(projectPath, workspacePath); - - // For existing branches, fast-forward to latest origin (best-effort) - // Only if local can fast-forward (preserves unpushed work) - if (!skipRemoteSync && shouldUseOrigin && branchExists) { - await this.fastForwardToOrigin(workspacePath, trunkBranch, initLogger, noHooksEnv); + // Fast-forward existing branches to origin only when local trunk can fast-forward too + // (preserves unpushed work). + const pending: PendingMaterialization = { + fastForwardFromOrigin: !skipRemoteSync && shouldUseOrigin && branchExists, + }; + if (params.deferMaterialization) { + await this.persistWorkspaceBranchMapping(projectPath, workspaceName, branchName); + return { success: true, workspacePath, pendingMaterialization: pending }; } - // Worktree creation is responsible for materializing the checkout completely. - // Skills, docs, and other repo-managed files may live inside submodules, so make - // them available before any runtime-specific provisioning or init hooks run. - await syncLocalGitSubmodules({ - workspacePath, - initLogger, - abortSignal: params.abortSignal, - env: params.env, - trusted: params.trusted, - }); + await this.materializeWorkspace( + { + projectPath, + workspacePath, + branchName, + trunkBranch, + initLogger, + abortSignal: params.abortSignal, + env: params.env, + trusted: params.trusted, + }, + pending + ); await this.persistWorkspaceBranchMapping(projectPath, workspaceName, branchName); return { success: true, workspacePath }; @@ -301,6 +272,88 @@ export class WorktreeManager { } } + /** + * Populate a worktree reserved by createWorkspace: streamed checkout, .xumignore sync, + * optional fast-forward, submodules. Throws on failure without touching the worktree; a + * deferred checkout is already registered, so its owner decides what happens to it. + */ + async materializeWorkspace( + params: { + projectPath: string; + workspacePath: string; + branchName: string; + trunkBranch: string; + initLogger: InitLogger; + abortSignal?: AbortSignal; + env?: Record; + trusted?: boolean; + }, + pending: PendingMaterialization + ): Promise { + const { projectPath, workspacePath, branchName, trunkBranch, initLogger } = params; + const noHooksEnv = await this.getGitExecOptions( + projectPath, + params.trusted, + params.abortSignal + ); + + initLogger.logStep("Checking out files..."); + const progress = new GitProgressParser( + (stage, percent) => initLogger.logProgress?.(stage, percent), + (line) => initLogger.logStdout(line) + ); + try { + // Submodule repos do not exist yet in a linked worktree, so recursion would fail; + // syncLocalGitSubmodules materializes them below, like `git worktree add` does. + using checkoutProc = execFileAsync( + "git", + [ + "-C", + workspacePath, + "checkout", + "--progress", + "--force", + "--no-recurse-submodules", + branchName, + ], + { + ...noHooksEnv, + // git delays progress output by 2s, which hides it for most checkouts. + env: { ...noHooksEnv?.env, GIT_PROGRESS_DELAY: "0" }, + onStderrData: (chunk) => progress.push(chunk), + } + ); + const { stdout } = await checkoutProc.result; + for (const line of stdout.split(/[\r\n]/)) { + if (line) initLogger.logStdout(line); + } + } finally { + progress.flush(); + } + + initLogger.logStep("Worktree created successfully"); + + // Sync gitignored files declared in .xumignore (e.g. .env) + // before init hooks run so they have access to secrets/config + initLogger.logStep("Syncing .xumignore files..."); + await syncXumignoreFiles(projectPath, workspacePath); + + if (pending.fastForwardFromOrigin) { + await this.fastForwardToOrigin(workspacePath, trunkBranch, initLogger, noHooksEnv); + } + + // Worktree creation is responsible for materializing the checkout completely. + // Skills, docs, and other repo-managed files may live inside submodules, so make + // them available before any runtime-specific provisioning or init hooks run. + await syncLocalGitSubmodules({ + workspacePath, + initLogger, + abortSignal: params.abortSignal, + env: params.env, + trusted: params.trusted, + }); + } + private async rollbackFailedWorkspaceCreation(args: { projectPath: string; workspacePath: string; diff --git a/tests/ipc/helpers.ts b/tests/ipc/helpers.ts index 1732f891ea7..06d0b0e16e1 100644 --- a/tests/ipc/helpers.ts +++ b/tests/ipc/helpers.ts @@ -5,7 +5,7 @@ import type { WorkspaceChatMessage, WorkspaceInitEvent, } from "@/common/orpc/types"; -import { isInitStart, isInitOutput, isInitEnd } from "@/common/orpc/types"; +import { isInitStart, isInitOutput, isInitProgress, isInitEnd } from "@/common/orpc/types"; // Re-export StreamCollector utilities for backwards compatibility export { @@ -472,7 +472,7 @@ export async function waitForInitComplete( const initEvents = collector .getEvents() .filter( - (msg) => isInitStart(msg) || isInitOutput(msg) || isInitEnd(msg) + (msg) => isInitStart(msg) || isInitOutput(msg) || isInitProgress(msg) || isInitEnd(msg) ) as WorkspaceInitEvent[]; // Check if init succeeded (exitCode === 0) @@ -527,7 +527,7 @@ export async function waitForInitEnd( return collector .getEvents() .filter( - (msg) => isInitStart(msg) || isInitOutput(msg) || isInitEnd(msg) + (msg) => isInitStart(msg) || isInitOutput(msg) || isInitProgress(msg) || isInitEnd(msg) ) as WorkspaceInitEvent[]; } finally { collector.stop(); diff --git a/tests/ipc/workspace/init.test.ts b/tests/ipc/workspace/init.test.ts index dc8b8799f0e..f7df00e36a3 100644 --- a/tests/ipc/workspace/init.test.ts +++ b/tests/ipc/workspace/init.test.ts @@ -18,7 +18,7 @@ import { } from "../helpers"; import { createStreamCollector } from "../streamCollector"; import type { WorkspaceInitEvent } from "@/common/orpc/types"; -import { isInitOutput, isInitEnd, isInitStart } from "@/common/orpc/types"; +import { isInitOutput, isInitEnd, isInitProgress, isInitStart } from "@/common/orpc/types"; import * as os from "os"; import * as fs from "fs/promises"; import { exec } from "child_process"; @@ -318,6 +318,89 @@ describeIntegration("Workspace init hook", () => { 15000 ); + test.concurrent( + "streams checkout progress to a subscriber that attaches after create() resolves", + async () => { + // The renderer only subscribes once create() has announced the workspace, so + // checkout progress emitted before that point can never reach the creation card. + const env = await createTestEnvironment(); + const tempGitRepo = await createTempGitRepoWithInitHook({ + exitCode: 0, + stdoutLines: ["hook ran"], + }); + + try { + const branchName = generateBranchName("checkout-progress"); + const createResult = await createWorkspace(env, tempGitRepo, branchName); + expect(createResult.success).toBe(true); + if (!createResult.success) return; + + const initEvents = await collectInitEvents(env, createResult.metadata.id, 10000); + + const progressEvents = initEvents.filter(isInitProgress); + expect(progressEvents.at(-1)).toMatchObject({ label: "Updating files", percent: 100 }); + + // The checkout must be complete before the hook runs against it. + const lines = initEvents.filter(isInitOutput).map((e) => e.line); + const materializedAt = lines.indexOf("Worktree created successfully"); + const hookAt = lines.findIndex((line) => /Running init hook:/.test(line)); + expect(materializedAt).toBeGreaterThanOrEqual(0); + expect(hookAt).toBeGreaterThan(materializedAt); + expect(lines).toContain("hook ran"); + await fs.access(path.join(createResult.metadata.namedWorkspacePath, "README.md")); + } finally { + await cleanupTestEnvironment(env); + await cleanupTempGitRepo(tempGitRepo); + } + }, + 15000 + ); + + test.concurrent( + "prunes committed plugin overrides after the deferred checkout and before the hook", + async () => { + // A repository that tracks .xum/mcp.local.jsonc materializes committed plugin enables + // into every fresh worktree; the sanitization that used to run at registration must + // now run once the files exist, and still ahead of the repo-controlled hook. + const env = await createTestEnvironment(); + const execAsync = promisify(exec); + const tempGitRepo = await createTempGitRepoWithInitHook({ + exitCode: 0, + customScript: "cat .xum/mcp.local.jsonc > hook-saw-overrides", + }); + await fs.mkdir(path.join(tempGitRepo, ".xum"), { recursive: true }); + await fs.writeFile( + path.join(tempGitRepo, ".xum", "mcp.local.jsonc"), + JSON.stringify({ enabledServers: ["plugin:0123456789abcdef:echo", "shots"] }) + ); + await execAsync("git add -A && git commit -m 'track plugin overrides'", { + cwd: tempGitRepo, + }); + + try { + const branchName = generateBranchName("deferred-sanitize"); + const createResult = await createWorkspace(env, tempGitRepo, branchName); + expect(createResult.success).toBe(true); + if (!createResult.success) return; + + await collectInitEvents(env, createResult.metadata.id, 10000); + + const workspacePath = createResult.metadata.namedWorkspacePath; + const pruned = JSON.parse( + await fs.readFile(path.join(workspacePath, ".xum", "mcp.local.jsonc"), "utf8") + ) as { enabledServers: string[] }; + expect(pruned.enabledServers).toEqual(["shots"]); + expect( + JSON.parse(await fs.readFile(path.join(workspacePath, "hook-saw-overrides"), "utf8")) + ).toEqual(pruned); + } finally { + await cleanupTestEnvironment(env); + await cleanupTempGitRepo(tempGitRepo); + } + }, + 15000 + ); + test.concurrent( "should persist init state to disk for replay across page reloads", async () => { From 92baaf9bd9975e7f851ffb7ef72ff197f8317a04 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:24:51 +0000 Subject: [PATCH 11/17] fix(worktree): classify checkout diagnostics once by exit status Git's stderr for the streamed checkout mixes progress with diagnostics. The parser used to forward diagnostics as output immediately, and a failure then re-logged the whole retained buffer (bare separators included) as error output. Hold diagnostics until the exit status is known: log them as output on success, and on failure throw them as the error so the caller reports them once, line by line. --- src/node/services/workspaceService.ts | 6 +++- src/node/worktree/WorktreeManager.test.ts | 22 ++++++++++-- src/node/worktree/WorktreeManager.ts | 15 +++++--- src/node/worktree/gitProgress.ts | 8 ++++- tests/ipc/workspace/init.test.ts | 43 +++++++++++++++++++++++ 5 files changed, 85 insertions(+), 9 deletions(-) diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 8ac8e2505b6..5bb11f3f02c 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -3120,7 +3120,11 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { await runtime.materializeWorkspace(initParams, args.pending); } catch (error) { log.error(`Workspace checkout failed for ${workspaceId}:`, { error }); - initParams.initLogger.logStderr(`Initialization failed: ${getErrorMessage(error)}`); + const [summary, ...details] = getErrorMessage(error).split(/\r?\n/); + initParams.initLogger.logStderr(`Initialization failed: ${summary}`); + for (const line of details) { + if (line) initParams.initLogger.logStderr(line); + } initParams.initLogger.logComplete(-1); return; } diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index 64a39967d94..ac85bf1feb1 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -263,13 +263,18 @@ describe("WorktreeManager.createWorkspace", () => { const fixture = await createWorktreeManagerFixture({ existingBranchName: existing ? branchName : undefined, }); + const stdout: string[] = []; const stderr: string[] = []; try { + // Several files so git reports checkout progress before the filter fails. + for (const name of ["a", "b", "c", "d"]) { + await fsPromises.writeFile(path.join(fixture.projectPath, `${name}.txt`), name); + } await fsPromises.writeFile( path.join(fixture.projectPath, ".gitattributes"), "README.md filter=fail\n" ); - execFileSync("git", ["add", ".gitattributes"], { + execFileSync("git", ["add", "-A"], { cwd: fixture.projectPath, stdio: "ignore", }); @@ -295,12 +300,23 @@ describe("WorktreeManager.createWorkspace", () => { trunkBranch: "main", skipRemoteSync: true, trusted: true, - initLogger: { ...fixture.initLogger, logStderr: (line) => stderr.push(line) }, + initLogger: { + ...fixture.initLogger, + logStdout: (line) => stdout.push(line), + logStderr: (line) => stderr.push(line), + }, }); expect(result.success).toBe(false); if (result.success) throw new Error("Expected checkout to fail"); expect(result.error).toContain("smudge filter fail failed"); - expect(stderr.some((line) => line.includes("smudge filter fail failed"))).toBe(true); + // Git's diagnostics are classified once by the exit status: as error output, not + // streamed as output first and repeated as error afterwards. + const diagnostic = (line: string) => line.includes("external filter"); + expect(stderr.filter(diagnostic)).toHaveLength(2); + expect(stdout.filter(diagnostic)).toEqual([]); + expect(stderr.filter((line) => line.includes("smudge filter fail failed"))).toHaveLength(1); + // Progress separators never leak into a logged line. + expect([...stdout, ...stderr].some((line) => line.includes("\r"))).toBe(false); const workspacePath = fixture.manager.getWorkspacePath(fixture.projectPath, branchName); expect(existsSync(workspacePath)).toBe(false); expect( diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index 353ce7a1c62..0e45f68582f 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -28,7 +28,7 @@ import { } from "@/constants/terminationTimeouts"; import { syncLocalGitSubmodules } from "@/node/runtime/submoduleSync"; import { syncXumignoreFiles } from "./xumignore"; -import { GitProgressParser } from "./gitProgress"; +import { GitProgressParser, isGitProgressLine } from "./gitProgress"; type GitExecOptions = Pick | undefined; @@ -298,9 +298,13 @@ export class WorktreeManager { ); initLogger.logStep("Checking out files..."); + // Git's stderr mixes progress with diagnostics. Progress streams live; diagnostics are + // held until the exit status is known so a failure is reported as error output once, + // rather than streamed as output and then repeated as the error. + const output: string[] = []; const progress = new GitProgressParser( (stage, percent) => initLogger.logProgress?.(stage, percent), - (line) => initLogger.logStdout(line) + (line) => output.push(line) ); try { // Submodule repos do not exist yet in a linked worktree, so recursion would fail; @@ -324,11 +328,14 @@ export class WorktreeManager { } ); const { stdout } = await checkoutProc.result; - for (const line of stdout.split(/[\r\n]/)) { + progress.flush(); + for (const line of [...output, ...stdout.split(/[\r\n]/)]) { if (line) initLogger.logStdout(line); } - } finally { + } catch (error) { progress.flush(); + const diagnostics = output.filter((line) => !isGitProgressLine(line)); + throw new Error(diagnostics.length > 0 ? diagnostics.join("\n") : getErrorMessage(error)); } initLogger.logStep("Worktree created successfully"); diff --git a/src/node/worktree/gitProgress.ts b/src/node/worktree/gitProgress.ts index cd01bb08cb4..9cc34b58820 100644 --- a/src/node/worktree/gitProgress.ts +++ b/src/node/worktree/gitProgress.ts @@ -1,3 +1,9 @@ +const PROGRESS_LINE = /^(.+?):\s+(\d{1,3})%/; + +export function isGitProgressLine(line: string): boolean { + return PROGRESS_LINE.test(line); +} + export class GitProgressParser { private buffer = ""; private lastStage: string | undefined; @@ -15,7 +21,7 @@ export class GitProgressParser { for (let index = 0; index < lines.length; index += 2) { const line = lines[index]; if (!line) continue; - const match = /^(.+?):\s+(\d{1,3})%/.exec(line); + const match = PROGRESS_LINE.exec(line); if (!match) { this.onOutput(line); continue; diff --git a/tests/ipc/workspace/init.test.ts b/tests/ipc/workspace/init.test.ts index f7df00e36a3..1befa22acdd 100644 --- a/tests/ipc/workspace/init.test.ts +++ b/tests/ipc/workspace/init.test.ts @@ -401,6 +401,49 @@ describeIntegration("Workspace init hook", () => { 15000 ); + test.concurrent( + "reports a failed deferred checkout on the card and keeps the workspace", + async () => { + const env = await createTestEnvironment(); + const execAsync = promisify(exec); + const tempGitRepo = await createTempGitRepoWithInitHook({ exitCode: 0 }); + await fs.writeFile(path.join(tempGitRepo, ".gitattributes"), "README.md filter=fail\n"); + await execAsync( + "git add -A && git commit -m 'require checkout filter' && git config filter.fail.smudge 'exit 1' && git config filter.fail.required true", + { cwd: tempGitRepo } + ); + + try { + const branchName = generateBranchName("deferred-checkout-failure"); + const createResult = await createWorkspace(env, tempGitRepo, branchName); + expect(createResult.success).toBe(true); + if (!createResult.success) return; + + const initEvents = await waitForInitEnd(env, createResult.metadata.id, 10000); + const endEvent = initEvents.find(isInitEnd); + expect(endEvent?.exitCode).toBe(-1); + const errorLines = initEvents + .filter((e): e is Extract => isInitOutput(e)) + .filter((e) => e.isError === true) + .map((e) => e.line); + expect( + errorLines.filter((line) => line.includes("smudge filter fail failed")) + ).toHaveLength(1); + expect(initEvents.filter(isInitOutput).some((e) => /Running init hook/.test(e.line))).toBe( + false + ); + // Like a remote sync failure, the workspace stays so the user can inspect and remove it. + const client = resolveOrpcClient(env); + const info = await client.workspace.getInfo({ workspaceId: createResult.metadata.id }); + expect(info?.id).toBe(createResult.metadata.id); + } finally { + await cleanupTestEnvironment(env); + await cleanupTempGitRepo(tempGitRepo); + } + }, + 15000 + ); + test.concurrent( "should persist init state to disk for replay across page reloads", async () => { From 0cda2344304ffc77db3c0f0fee6ece7986867b1d Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:00:09 +0000 Subject: [PATCH 12/17] fix(worktree): keep the branch reserved and sanitize on every materialization exit Repointing HEAD at the unborn placeholder during creation left the branch unclaimed until the deferred checkout ran, so a competing worktree could take it and the announced workspace would fail to check out. Move the placeholder step into materializeWorkspace, immediately before the checkout, so the reserved worktree holds the branch across the gap while post-checkout still sees a fresh worktree add. Materialization can fail after the checkout populated the tracked override file (broken submodules, .xumignore), and sends proceed after a failed init, so run the plugin-override sanitization on every materialization exit rather than only on success. --- src/node/services/workspaceService.ts | 28 +++++++++---- src/node/worktree/WorktreeManager.test.ts | 50 ++++++++++++++++++++++- src/node/worktree/WorktreeManager.ts | 18 ++++---- tests/ipc/workspace/init.test.ts | 48 ++++++++++++++++++++++ 4 files changed, 125 insertions(+), 19 deletions(-) diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 5bb11f3f02c..b671da8a802 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -3099,8 +3099,10 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { * Background init for a worktree announced before its files existed: populate the * checkout (streaming progress to the creation card), then sanitize plugin overrides * exactly as task worktrees do after materialization, then run the ordinary init. - * A checkout failure fails the init like any deferred runtime's sync failure; a - * sanitize failure tears the creation down, as it would have at registration time. + * A checkout failure fails the init like any deferred runtime's sync failure, but the + * checkout is still sanitized first: a later step (submodules, .xumignore) can fail after + * the tracked override file is already on disk, and sends proceed after a failed init. + * A sanitize failure tears the creation down, as it would have at registration time. */ private async materializeDeferredCheckout(args: { workspaceId: string; @@ -3116,16 +3118,14 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { runtime.materializeWorkspace !== undefined, "materializeDeferredCheckout: runtime cannot materialize" ); + let materializeError: unknown; try { await runtime.materializeWorkspace(initParams, args.pending); } catch (error) { - log.error(`Workspace checkout failed for ${workspaceId}:`, { error }); - const [summary, ...details] = getErrorMessage(error).split(/\r?\n/); - initParams.initLogger.logStderr(`Initialization failed: ${summary}`); - for (const line of details) { - if (line) initParams.initLogger.logStderr(line); - } - initParams.initLogger.logComplete(-1); + materializeError = error; + } + if (args.initAbortController.signal.aborted) { + // Removal owns the checkout now (it aborted us and awaits this settlement). return; } const sanitizeError = await this.sanitizeMaterializedTaskWorkspace( @@ -3150,6 +3150,16 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { this.emit("metadata", { workspaceId, metadata: null }); return; } + if (materializeError !== undefined) { + log.error(`Workspace checkout failed for ${workspaceId}:`, { error: materializeError }); + const [summary, ...details] = getErrorMessage(materializeError).split(/\r?\n/); + initParams.initLogger.logStderr(`Initialization failed: ${summary}`); + for (const line of details) { + if (line) initParams.initLogger.logStderr(line); + } + initParams.initLogger.logComplete(-1); + return; + } await runBackgroundInit(runtime, initParams, workspaceId, log); } diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index ac85bf1feb1..f04c15e397b 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -507,6 +507,52 @@ describe("WorktreeManager.createWorkspace", () => { } }, 20_000); + it("keeps the branch reserved between the deferred reservation and its checkout", async () => { + const branchName = "feature-reserved"; + const fixture = await createWorktreeManagerFixture(); + try { + const result = await fixture.manager.createWorkspace({ + projectPath: fixture.projectPath, + branchName, + trunkBranch: "main", + skipRemoteSync: true, + trusted: true, + initLogger: fixture.initLogger, + deferMaterialization: true, + }); + expect(result.success).toBe(true); + if (!result.success || !result.workspacePath) throw new Error("Expected reservation"); + + // Another checkout of the branch must be refused while materialization is pending. + const rival = path.join(fixture.rootDir, "rival"); + expect(() => + execFileSync("git", ["worktree", "add", rival, branchName], { + cwd: fixture.projectPath, + stdio: "pipe", + }) + ).toThrow(/already (checked out|used by worktree)/); + + await fixture.manager.materializeWorkspace( + { + projectPath: fixture.projectPath, + workspacePath: result.workspacePath, + branchName, + trunkBranch: "main", + trusted: true, + initLogger: fixture.initLogger, + }, + result.pendingMaterialization! + ); + expect( + execFileSync("git", ["branch", "--show-current"], { cwd: result.workspacePath }) + .toString() + .trim() + ).toBe(branchName); + } finally { + await fixture.cleanup(); + } + }, 20_000); + it("deletes a reserved worktree that was never materialized", async () => { const branchName = "feature-deferred-cancelled"; const fixture = await createWorktreeManagerFixture(); @@ -521,10 +567,12 @@ describe("WorktreeManager.createWorkspace", () => { deferMaterialization: true, }); expect(result.success).toBe(true); + // Cancelling creation removes with force: git reports a reserved worktree's missing + // files as deletions, so a plain `worktree remove` would refuse it. const deleteResult = await fixture.manager.deleteWorkspace( fixture.projectPath, branchName, - false, + true, true ); expect(deleteResult.success).toBe(true); diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index 0e45f68582f..65777ecf187 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -201,15 +201,6 @@ export class WorktreeManager { if (line) initLogger.logStdout(line); } - // Point HEAD at an unborn ref so the checkout reports the same post-checkout hook - // arguments as a plain `git worktree add` (null old commit, new-worktree flag). - using unbornProc = execFileAsync( - "git", - ["-C", workspacePath, "symbolic-ref", "HEAD", `refs/heads/xum-unborn-${randomUUID()}`], - noHooksEnv - ); - await unbornProc.result; - // Fast-forward existing branches to origin only when local trunk can fast-forward too // (preserves unpushed work). const pending: PendingMaterialization = { @@ -298,6 +289,15 @@ export class WorktreeManager { ); initLogger.logStep("Checking out files..."); + // Point HEAD at an unborn ref only now, so the checkout reports the same post-checkout hook + // arguments as a plain `git worktree add` (null old commit, new-worktree flag) while the + // worktree kept the branch reserved against other checkouts until this moment. + using unbornProc = execFileAsync( + "git", + ["-C", workspacePath, "symbolic-ref", "HEAD", `refs/heads/xum-unborn-${randomUUID()}`], + noHooksEnv + ); + await unbornProc.result; // Git's stderr mixes progress with diagnostics. Progress streams live; diagnostics are // held until the exit status is known so a failure is reported as error output once, // rather than streamed as output and then repeated as the error. diff --git a/tests/ipc/workspace/init.test.ts b/tests/ipc/workspace/init.test.ts index 1befa22acdd..55557736e62 100644 --- a/tests/ipc/workspace/init.test.ts +++ b/tests/ipc/workspace/init.test.ts @@ -444,6 +444,54 @@ describeIntegration("Workspace init hook", () => { 15000 ); + test.concurrent( + "prunes committed plugin overrides even when materialization fails after the checkout", + async () => { + // A broken submodule fails materialization after the tracked override file is on + // disk, and the workspace stays usable after a failed init, so the prune must not + // depend on materialization succeeding. + const env = await createTestEnvironment(); + const execAsync = promisify(exec); + const tempGitRepo = await createTempGitRepoWithInitHook({ exitCode: 0 }); + await fs.mkdir(path.join(tempGitRepo, ".xum"), { recursive: true }); + await fs.writeFile( + path.join(tempGitRepo, ".xum", "mcp.local.jsonc"), + JSON.stringify({ enabledServers: ["plugin:0123456789abcdef:echo", "shots"] }) + ); + await fs.writeFile( + path.join(tempGitRepo, ".gitmodules"), + '[submodule "dep"]\n\tpath = dep\n\turl = /nonexistent/dep.git\n' + ); + await execAsync( + "git add -A && git update-index --add --cacheinfo 160000,4b825dc642cb6eb9a060e54bf8d69288fbee4904,dep && git commit -m 'broken submodule'", + { cwd: tempGitRepo } + ); + + try { + const branchName = generateBranchName("deferred-sanitize-after-failure"); + const createResult = await createWorkspace(env, tempGitRepo, branchName); + expect(createResult.success).toBe(true); + if (!createResult.success) return; + + const initEvents = await waitForInitEnd(env, createResult.metadata.id, 10000); + expect(initEvents.find(isInitEnd)?.exitCode).toBe(-1); + + const workspacePath = createResult.metadata.namedWorkspacePath; + const pruned = JSON.parse( + await fs.readFile(path.join(workspacePath, ".xum", "mcp.local.jsonc"), "utf8") + ) as { enabledServers: string[] }; + expect(pruned.enabledServers).toEqual(["shots"]); + const client = resolveOrpcClient(env); + const info = await client.workspace.getInfo({ workspaceId: createResult.metadata.id }); + expect(info?.id).toBe(createResult.metadata.id); + } finally { + await cleanupTestEnvironment(env); + await cleanupTempGitRepo(tempGitRepo); + } + }, + 15000 + ); + test.concurrent( "should persist init state to disk for replay across page reloads", async () => { From a422daedc9324fe4cf56e213c5f03d6909eeba48 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:38:36 +0000 Subject: [PATCH 13/17] fix(worktree): never clobber or strand a deferred checkout The deferred checkout runs inside an announced workspace, so drop --force: anything a terminal or editor wrote there in the meantime now fails the checkout with git's own message instead of being overwritten. When the checkout fails or is aborted, point HEAD back at the workspace branch so a retained worktree never commits to the unborn placeholder. Only skip the post-materialization sanitize when the workspace is being removed; archive aborts init too but keeps the checkout registered and never reruns init. --- src/node/services/workspaceService.ts | 5 +- src/node/worktree/WorktreeManager.test.ts | 50 +++++++++++++++++++ src/node/worktree/WorktreeManager.ts | 25 ++++++---- tests/ipc/workspace/init.test.ts | 58 +++++++++++++++++++++++ 4 files changed, 127 insertions(+), 11 deletions(-) diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index b671da8a802..5f5d256ede2 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -3124,8 +3124,9 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { } catch (error) { materializeError = error; } - if (args.initAbortController.signal.aborted) { - // Removal owns the checkout now (it aborted us and awaits this settlement). + if (this.removingWorkspaces.has(workspaceId)) { + // Removal owns the checkout now (it aborted us and awaits this settlement). Archive + // also aborts but keeps the checkout registered, so it still gets sanitized below. return; } const sanitizeError = await this.sanitizeMaterializedTaskWorkspace( diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index f04c15e397b..cae8f165a38 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -553,6 +553,56 @@ describe("WorktreeManager.createWorkspace", () => { } }, 20_000); + it("refuses to overwrite files written into the reserved worktree and returns to the branch", async () => { + const branchName = "feature-stray-file"; + const fixture = await createWorktreeManagerFixture(); + try { + const result = await fixture.manager.createWorkspace({ + projectPath: fixture.projectPath, + branchName, + trunkBranch: "main", + skipRemoteSync: true, + trusted: true, + initLogger: fixture.initLogger, + deferMaterialization: true, + }); + expect(result.success).toBe(true); + if (!result.success || !result.workspacePath) throw new Error("Expected reservation"); + + // The workspace is already announced, so a terminal or editor can write here first. + const strayFile = path.join(result.workspacePath, "README.md"); + await fsPromises.writeFile(strayFile, "user data\n"); + + const failure = await fixture.manager + .materializeWorkspace( + { + projectPath: fixture.projectPath, + workspacePath: result.workspacePath, + branchName, + trunkBranch: "main", + trusted: true, + initLogger: fixture.initLogger, + }, + result.pendingMaterialization! + ) + .then( + () => undefined, + (error: unknown) => error + ); + expect(failure).toBeInstanceOf(Error); + expect((failure as Error).message).toContain("README.md"); + expect(await fsPromises.readFile(strayFile, "utf8")).toBe("user data\n"); + // The retained workspace stays on its branch rather than the checkout placeholder. + expect( + execFileSync("git", ["symbolic-ref", "HEAD"], { cwd: result.workspacePath }) + .toString() + .trim() + ).toBe(`refs/heads/${branchName}`); + } finally { + await fixture.cleanup(); + } + }, 20_000); + it("deletes a reserved worktree that was never materialized", async () => { const branchName = "feature-deferred-cancelled"; const fixture = await createWorktreeManagerFixture(); diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index 65777ecf187..6a1656fbd25 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -309,17 +309,11 @@ export class WorktreeManager { try { // Submodule repos do not exist yet in a linked worktree, so recursion would fail; // syncLocalGitSubmodules materializes them below, like `git worktree add` does. + // No --force: a deferred checkout runs in an announced workspace, so anything written + // there meanwhile (a terminal, an editor) must fail the checkout, not be overwritten. using checkoutProc = execFileAsync( "git", - [ - "-C", - workspacePath, - "checkout", - "--progress", - "--force", - "--no-recurse-submodules", - branchName, - ], + ["-C", workspacePath, "checkout", "--progress", "--no-recurse-submodules", branchName], { ...noHooksEnv, // git delays progress output by 2s, which hides it for most checkouts. @@ -334,6 +328,19 @@ export class WorktreeManager { } } catch (error) { progress.flush(); + // A retained (deferred) workspace must not be left on the placeholder ref: later git + // operations would commit to it instead of the workspace branch. Runs without the + // caller's signal because an aborted checkout (archive) needs the restore most. + try { + using restoreProc = execFileAsync( + "git", + ["-C", workspacePath, "symbolic-ref", "HEAD", `refs/heads/${branchName}`], + noHooksEnv?.env ? { env: noHooksEnv.env } : undefined + ); + await restoreProc.result; + } catch { + // The checkout error below is the one worth reporting. + } const diagnostics = output.filter((line) => !isGitProgressLine(line)); throw new Error(diagnostics.length > 0 ? diagnostics.join("\n") : getErrorMessage(error)); } diff --git a/tests/ipc/workspace/init.test.ts b/tests/ipc/workspace/init.test.ts index 55557736e62..f04f501020d 100644 --- a/tests/ipc/workspace/init.test.ts +++ b/tests/ipc/workspace/init.test.ts @@ -492,6 +492,64 @@ describeIntegration("Workspace init hook", () => { 15000 ); + test.concurrent( + "sanitizes a deferred checkout that archiving interrupted", + async () => { + // Archive aborts a running init but keeps the checkout registered (default behaviour), + // and unarchiving does not rerun init, so the interrupted materialization must still + // prune committed plugin enables before the workspace is parked. + const env = await createTestEnvironment(); + const execAsync = promisify(exec); + const tempGitRepo = await createTempGitRepoWithInitHook({ exitCode: 0 }); + await fs.mkdir(path.join(tempGitRepo, ".xum"), { recursive: true }); + await fs.writeFile( + path.join(tempGitRepo, ".xum", "mcp.local.jsonc"), + JSON.stringify({ enabledServers: ["plugin:0123456789abcdef:echo", "shots"] }) + ); + // README.md sorts after .xum/, so the override is on disk while its smudge filter stalls. + await fs.writeFile(path.join(tempGitRepo, ".gitattributes"), "README.md filter=slow\n"); + await execAsync( + "git add -A && git commit -m 'slow checkout' && git config filter.slow.smudge 'sleep 5; cat'", + { cwd: tempGitRepo } + ); + + try { + const branchName = generateBranchName("deferred-archive"); + const createResult = await createWorkspace(env, tempGitRepo, branchName); + expect(createResult.success).toBe(true); + if (!createResult.success) return; + const workspaceId = createResult.metadata.id; + const workspacePath = createResult.metadata.namedWorkspacePath; + + // Archive once the checkout has written the override but is still stalled on README.md. + const overridePath = path.join(workspacePath, ".xum", "mcp.local.jsonc"); + const deadline = Date.now() + 8000; + while (Date.now() < deadline) { + try { + await fs.access(overridePath); + break; + } catch { + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } + const client = resolveOrpcClient(env); + const archiveResult = await client.workspace.archive({ workspaceId }); + expect(archiveResult.success).toBe(true); + + const pruned = JSON.parse(await fs.readFile(overridePath, "utf8")) as { + enabledServers: string[]; + }; + expect(pruned.enabledServers).toEqual(["shots"]); + const { stdout } = await execAsync("git symbolic-ref HEAD", { cwd: workspacePath }); + expect(stdout.trim()).toBe(`refs/heads/${branchName}`); + } finally { + await cleanupTestEnvironment(env); + await cleanupTempGitRepo(tempGitRepo); + } + }, + 20000 + ); + test.concurrent( "should persist init state to disk for replay across page reloads", async () => { From 9d655574fdf95e73a28725199435f7a9882061d0 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:05:46 +0000 Subject: [PATCH 14/17] fix(worktree): finish the checkout for archive, rebuild the index on failure, kill helpers on cancel Archive aborts init but keeps the checkout registered and never reruns it, so only removal may now interrupt the deferred checkout itself; archive waits on the init settlement and therefore parks a complete checkout. A failed checkout restores the index from HEAD along with HEAD so the retained worktree shows the stray file as a modification rather than every tracked file staged for deletion. The streamed checkout kills its process tree on cancellation so a stalled smudge filter cannot hold the settlement open. --- src/node/services/workspaceService.ts | 20 +++++- src/node/worktree/WorktreeManager.test.ts | 80 ++++++++++++++++++++++- src/node/worktree/WorktreeManager.ts | 19 ++++-- tests/ipc/workspace/init.test.ts | 7 +- 4 files changed, 115 insertions(+), 11 deletions(-) diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 5f5d256ede2..40939eac568 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -3118,15 +3118,29 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { runtime.materializeWorkspace !== undefined, "materializeDeferredCheckout: runtime cannot materialize" ); + // Only removal may interrupt the checkout itself: archive aborts init too but keeps + // the checkout registered and never reruns it, so parking a half-populated worktree + // would strand it. Archive awaits this settlement, so it parks a complete checkout; + // the hook phase below still honours its abort. + const materializeAbort = new AbortController(); + const forwardRemovalAbort = () => { + if (this.removingWorkspaces.has(workspaceId)) materializeAbort.abort(); + }; + args.initAbortController.signal.addEventListener("abort", forwardRemovalAbort); let materializeError: unknown; try { - await runtime.materializeWorkspace(initParams, args.pending); + forwardRemovalAbort(); + await runtime.materializeWorkspace( + { ...initParams, abortSignal: materializeAbort.signal }, + args.pending + ); } catch (error) { materializeError = error; + } finally { + args.initAbortController.signal.removeEventListener("abort", forwardRemovalAbort); } if (this.removingWorkspaces.has(workspaceId)) { - // Removal owns the checkout now (it aborted us and awaits this settlement). Archive - // also aborts but keeps the checkout registered, so it still gets sanitized below. + // Removal owns the checkout now (it aborted us and awaits this settlement). return; } const sanitizeError = await this.sanitizeMaterializedTaskWorkspace( diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index cae8f165a38..d47adfb7fdb 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -592,12 +592,90 @@ describe("WorktreeManager.createWorkspace", () => { expect(failure).toBeInstanceOf(Error); expect((failure as Error).message).toContain("README.md"); expect(await fsPromises.readFile(strayFile, "utf8")).toBe("user data\n"); - // The retained workspace stays on its branch rather than the checkout placeholder. + // The retained workspace stays on its branch rather than the checkout placeholder, with + // an index that matches it: the stray file reads as a modification, not as every tracked + // file staged for deletion. expect( execFileSync("git", ["symbolic-ref", "HEAD"], { cwd: result.workspacePath }) .toString() .trim() ).toBe(`refs/heads/${branchName}`); + expect( + execFileSync("git", ["status", "--porcelain"], { cwd: result.workspacePath }) + .toString() + .trimEnd() + ).toBe(" M README.md"); + } finally { + await fixture.cleanup(); + } + }, 20_000); + + it("cancelling a deferred checkout also stops the helpers it spawned", async () => { + const branchName = "feature-stalled-smudge"; + const fixture = await createWorktreeManagerFixture(); + try { + const pidFile = path.join(fixture.rootDir, "smudge-pids"); + const shim = path.join(fixture.rootDir, "stalled-smudge.sh"); + await fsPromises.writeFile( + shim, + `#!/bin/sh\nsleep 600 &\nprintf '%s\\n%s\\n' "$$" "$!" > "${pidFile}"\nwait\n`, + "utf-8" + ); + await fsPromises.chmod(shim, 0o755); + await fsPromises.writeFile( + path.join(fixture.projectPath, ".gitattributes"), + "README.md filter=stall\n" + ); + execFileSync("git", ["add", ".gitattributes"], { cwd: fixture.projectPath, stdio: "ignore" }); + execFileSync("git", ["commit", "-qm", "stall the checkout"], { + cwd: fixture.projectPath, + stdio: "ignore", + }); + execFileSync("git", ["config", "filter.stall.smudge", shim], { cwd: fixture.projectPath }); + + const result = await fixture.manager.createWorkspace({ + projectPath: fixture.projectPath, + branchName, + trunkBranch: "main", + skipRemoteSync: true, + trusted: true, + initLogger: fixture.initLogger, + deferMaterialization: true, + }); + expect(result.success).toBe(true); + if (!result.success || !result.workspacePath) throw new Error("Expected reservation"); + + const controller = new AbortController(); + const materialize = fixture.manager + .materializeWorkspace( + { + projectPath: fixture.projectPath, + workspacePath: result.workspacePath, + branchName, + trunkBranch: "main", + trusted: true, + initLogger: fixture.initLogger, + abortSignal: controller.signal, + }, + result.pendingMaterialization! + ) + .then( + () => "resolved", + () => "rejected" + ); + const deadline = Date.now() + 5_000; + let pids: number[] = []; + while (Date.now() < deadline && pids.length !== 2) { + pids = await fsPromises.readFile(pidFile, "utf-8").then( + (content) => content.trim().split("\n").map(Number), + () => [] + ); + if (pids.length !== 2) await new Promise((resolve) => setTimeout(resolve, 50)); + } + expect(pids).toHaveLength(2); + controller.abort(); + expect(await materialize).toBe("rejected"); + expect(await waitForProcessesToExit(pids)).toBe(true); } finally { await fixture.cleanup(); } diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index 6a1656fbd25..00bb085280c 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -319,6 +319,8 @@ export class WorktreeManager { // git delays progress output by 2s, which hides it for most checkouts. env: { ...noHooksEnv?.env, GIT_PROGRESS_DELAY: "0" }, onStderrData: (chunk) => progress.push(chunk), + // Smudge filters and hooks inherit git's pipes; cancelling must not hang on them. + killTreeOnTermination: true, } ); const { stdout } = await checkoutProc.result; @@ -328,16 +330,25 @@ export class WorktreeManager { } } catch (error) { progress.flush(); - // A retained (deferred) workspace must not be left on the placeholder ref: later git - // operations would commit to it instead of the workspace branch. Runs without the - // caller's signal because an aborted checkout (archive) needs the restore most. + // A retained (deferred) workspace must not be left on the placeholder ref with the empty + // pre-checkout index: a later commit would land on the placeholder or record every + // tracked file as deleted. Put HEAD back on the branch and rebuild the index from it, + // leaving the working tree alone. Runs without the caller's signal because an aborted + // checkout needs the restore most. + const restoreOptions = noHooksEnv?.env ? { env: noHooksEnv.env } : undefined; try { using restoreProc = execFileAsync( "git", ["-C", workspacePath, "symbolic-ref", "HEAD", `refs/heads/${branchName}`], - noHooksEnv?.env ? { env: noHooksEnv.env } : undefined + restoreOptions ); await restoreProc.result; + using resetProc = execFileAsync( + "git", + ["-C", workspacePath, "reset", "--quiet"], + restoreOptions + ); + await resetProc.result; } catch { // The checkout error below is the one worth reporting. } diff --git a/tests/ipc/workspace/init.test.ts b/tests/ipc/workspace/init.test.ts index f04f501020d..c4d925a75a8 100644 --- a/tests/ipc/workspace/init.test.ts +++ b/tests/ipc/workspace/init.test.ts @@ -493,11 +493,11 @@ describeIntegration("Workspace init hook", () => { ); test.concurrent( - "sanitizes a deferred checkout that archiving interrupted", + "archiving during a deferred checkout parks a complete, sanitized checkout", async () => { // Archive aborts a running init but keeps the checkout registered (default behaviour), - // and unarchiving does not rerun init, so the interrupted materialization must still - // prune committed plugin enables before the workspace is parked. + // and unarchiving does not rerun init, so the checkout must finish and prune committed + // plugin enables before the workspace is parked. const env = await createTestEnvironment(); const execAsync = promisify(exec); const tempGitRepo = await createTempGitRepoWithInitHook({ exitCode: 0 }); @@ -540,6 +540,7 @@ describeIntegration("Workspace init hook", () => { enabledServers: string[]; }; expect(pruned.enabledServers).toEqual(["shots"]); + expect(await fs.readFile(path.join(workspacePath, "README.md"), "utf8")).toBe("test\n"); const { stdout } = await execAsync("git symbolic-ref HEAD", { cwd: workspacePath }); expect(stdout.trim()).toBe(`refs/heads/${branchName}`); } finally { From d28e33a6aca2e8ffeff5621989cf664728c33cd9 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:59:34 +0000 Subject: [PATCH 15/17] fix(worktree): reserve the branch through the streamed checkout, let archive stop what follows it - materializeWorkspace populates the files while HEAD still holds the branch (hooks off), so no other worktree can claim it for as long as the checkout streams; only then does HEAD move through the unborn placeholder for a fast second checkout that gives trusted post-checkout hooks the plain worktree-add arguments. - The file checkout honours a separate checkoutAbortSignal; the switch, .xumignore sync, fast-forward and submodule sync honour the init signal. materializeDeferredCheckout forwards only removal to the former, so archive still parks complete files but no longer waits on the phases after them. - Tests: a gated smudge filter holds the checkout open while a rival worktree add is refused (red on the placeholder HEAD); archiving while a trusted post-checkout hook sleeps returns promptly with a complete, clean checkout (timed out before). --- src/node/runtime/Runtime.ts | 11 ++- src/node/runtime/WorktreeRuntime.ts | 4 +- src/node/services/workspaceService.ts | 12 ++-- src/node/worktree/WorktreeManager.test.ts | 84 ++++++++++++++++++++++- src/node/worktree/WorktreeManager.ts | 67 +++++++++++++----- tests/ipc/workspace/init.test.ts | 52 ++++++++++++++ 6 files changed, 202 insertions(+), 28 deletions(-) diff --git a/src/node/runtime/Runtime.ts b/src/node/runtime/Runtime.ts index 0ca8374b356..3f1995e8601 100644 --- a/src/node/runtime/Runtime.ts +++ b/src/node/runtime/Runtime.ts @@ -219,6 +219,15 @@ export interface PendingMaterialization { fastForwardFromOrigin: boolean; } +/** Init params for materializeWorkspace(), plus how far a cancellation may reach. */ +export interface WorkspaceMaterializeParams extends WorkspaceInitParams { + /** + * When given, the only signal the file checkout honours; abortSignal still cancels every + * later phase, so an owner that keeps a cancelled checkout gets whole files. + */ + checkoutAbortSignal?: AbortSignal; +} + /** * Result from workspace creation */ @@ -558,7 +567,7 @@ export interface Runtime { * Streams progress via initLogger; throws on failure and leaves the checkout registered. */ materializeWorkspace?( - params: WorkspaceInitParams, + params: WorkspaceMaterializeParams, pending: PendingMaterialization ): Promise; diff --git a/src/node/runtime/WorktreeRuntime.ts b/src/node/runtime/WorktreeRuntime.ts index 0659d1a3717..e784202738d 100644 --- a/src/node/runtime/WorktreeRuntime.ts +++ b/src/node/runtime/WorktreeRuntime.ts @@ -8,6 +8,7 @@ import type { WorkspaceInitResult, WorkspaceForkParams, WorkspaceForkResult, + WorkspaceMaterializeParams, } from "./Runtime"; import { WORKSPACE_REPO_MISSING_ERROR } from "./Runtime"; import { LocalBaseRuntime } from "./LocalBaseRuntime"; @@ -109,7 +110,7 @@ export class WorktreeRuntime extends LocalBaseRuntime { } async materializeWorkspace( - params: WorkspaceInitParams, + params: WorkspaceMaterializeParams, pending: PendingMaterialization ): Promise { return this.worktreeManager.materializeWorkspace( @@ -120,6 +121,7 @@ export class WorktreeRuntime extends LocalBaseRuntime { trunkBranch: params.trunkBranch, initLogger: params.initLogger, abortSignal: params.abortSignal, + checkoutAbortSignal: params.checkoutAbortSignal, env: params.env, trusted: params.trusted, }, diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 40939eac568..2231f9620ce 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -3118,20 +3118,20 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { runtime.materializeWorkspace !== undefined, "materializeDeferredCheckout: runtime cannot materialize" ); - // Only removal may interrupt the checkout itself: archive aborts init too but keeps + // Only removal may interrupt the file checkout itself: archive aborts init too but keeps // the checkout registered and never reruns it, so parking a half-populated worktree - // would strand it. Archive awaits this settlement, so it parks a complete checkout; - // the hook phase below still honours its abort. - const materializeAbort = new AbortController(); + // would strand it. Archive awaits this settlement, so it parks complete files; every + // phase after them (hooks, .xumignore, fast-forward, submodules) honours its abort. + const checkoutAbort = new AbortController(); const forwardRemovalAbort = () => { - if (this.removingWorkspaces.has(workspaceId)) materializeAbort.abort(); + if (this.removingWorkspaces.has(workspaceId)) checkoutAbort.abort(); }; args.initAbortController.signal.addEventListener("abort", forwardRemovalAbort); let materializeError: unknown; try { forwardRemovalAbort(); await runtime.materializeWorkspace( - { ...initParams, abortSignal: materializeAbort.signal }, + { ...initParams, checkoutAbortSignal: checkoutAbort.signal }, args.pending ); } catch (error) { diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index d47adfb7fdb..79e104bf660 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -184,7 +184,7 @@ describe("WorktreeManager.createWorkspace", () => { let checkoutStarted = false; const execSpy = spyOn(disposableExec, "execFileAsync").mockImplementation( (file, args, options) => { - if (file === "git" && args[2] === "checkout") { + if (file === "git" && args.includes("checkout") && !checkoutStarted) { checkoutStarted = true; expect(existsSync(path.join(workspacePath, "README.md"))).toBe(false); } @@ -553,6 +553,88 @@ describe("WorktreeManager.createWorkspace", () => { } }, 20_000); + it("keeps the branch reserved while the checkout streams", async () => { + const branchName = "feature-reserved-while-streaming"; + const fixture = await createWorktreeManagerFixture(); + try { + // A smudge filter that waits to be released holds the checkout open mid-stream. + const started = path.join(fixture.rootDir, "smudge-started"); + const release = path.join(fixture.rootDir, "smudge-release"); + const shim = path.join(fixture.rootDir, "gated-smudge.sh"); + await fsPromises.writeFile( + shim, + `#!/bin/sh\n: > "${started}"\nwhile [ ! -e "${release}" ]; do sleep 0.05; done\ncat\n`, + "utf-8" + ); + await fsPromises.chmod(shim, 0o755); + await fsPromises.writeFile( + path.join(fixture.projectPath, ".gitattributes"), + "README.md filter=gate\n" + ); + execFileSync("git", ["add", ".gitattributes"], { cwd: fixture.projectPath, stdio: "ignore" }); + execFileSync("git", ["commit", "-qm", "gate the checkout"], { + cwd: fixture.projectPath, + stdio: "ignore", + }); + execFileSync("git", ["config", "filter.gate.smudge", shim], { cwd: fixture.projectPath }); + + const result = await fixture.manager.createWorkspace({ + projectPath: fixture.projectPath, + branchName, + trunkBranch: "main", + skipRemoteSync: true, + trusted: true, + initLogger: fixture.initLogger, + deferMaterialization: true, + }); + expect(result.success).toBe(true); + if (!result.success || !result.workspacePath) throw new Error("Expected reservation"); + + const materialize = fixture.manager.materializeWorkspace( + { + projectPath: fixture.projectPath, + workspacePath: result.workspacePath, + branchName, + trunkBranch: "main", + trusted: true, + initLogger: fixture.initLogger, + }, + result.pendingMaterialization! + ); + const deadline = Date.now() + 5_000; + while (Date.now() < deadline && !existsSync(started)) { + await new Promise((resolve) => setTimeout(resolve, 50)); + } + expect(existsSync(started)).toBe(true); + // The files are still landing: another worktree must not be able to claim the branch. + const rival = path.join(fixture.rootDir, "rival"); + let rivalError: unknown; + try { + execFileSync("git", ["worktree", "add", "--no-checkout", rival, branchName], { + cwd: fixture.projectPath, + stdio: "pipe", + }); + } catch (error) { + rivalError = error; + } + await fsPromises.writeFile(release, ""); + expect(rivalError).toBeInstanceOf(Error); + expect((rivalError as Error).message).toMatch(/already (checked out|used by worktree)/); + + await materialize; + expect( + execFileSync("git", ["branch", "--show-current"], { cwd: result.workspacePath }) + .toString() + .trim() + ).toBe(branchName); + expect(await fsPromises.readFile(path.join(result.workspacePath, "README.md"), "utf8")).toBe( + "hello\n" + ); + } finally { + await fixture.cleanup(); + } + }, 20_000); + it("refuses to overwrite files written into the reserved worktree and returns to the branch", async () => { const branchName = "feature-stray-file"; const fixture = await createWorktreeManagerFixture(); diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index 00bb085280c..b4edf760f63 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -267,6 +267,9 @@ export class WorktreeManager { * Populate a worktree reserved by createWorkspace: streamed checkout, .xumignore sync, * optional fast-forward, submodules. Throws on failure without touching the worktree; a * deferred checkout is already registered, so its owner decides what happens to it. + * abortSignal cancels every phase; when checkoutAbortSignal is given it is the only signal + * the file checkout honours, so an owner that keeps a cancelled worktree gets complete files + * while everything after them still stops. */ async materializeWorkspace( params: { @@ -276,6 +279,7 @@ export class WorktreeManager { trunkBranch: string; initLogger: InitLogger; abortSignal?: AbortSignal; + checkoutAbortSignal?: AbortSignal; env?: Record; trusted?: boolean; }, @@ -289,15 +293,6 @@ export class WorktreeManager { ); initLogger.logStep("Checking out files..."); - // Point HEAD at an unborn ref only now, so the checkout reports the same post-checkout hook - // arguments as a plain `git worktree add` (null old commit, new-worktree flag) while the - // worktree kept the branch reserved against other checkouts until this moment. - using unbornProc = execFileAsync( - "git", - ["-C", workspacePath, "symbolic-ref", "HEAD", `refs/heads/xum-unborn-${randomUUID()}`], - noHooksEnv - ); - await unbornProc.result; // Git's stderr mixes progress with diagnostics. Progress streams live; diagnostics are // held until the exit status is known so a failure is reported as error output once, // rather than streamed as output and then repeated as the error. @@ -306,32 +301,66 @@ export class WorktreeManager { (stage, percent) => initLogger.logProgress?.(stage, percent), (line) => output.push(line) ); + const stdout: string[] = []; + const checkoutOptions = { + ...noHooksEnv, + onStderrData: (chunk: string) => progress.push(chunk), + // Smudge filters and hooks inherit git's pipes; cancelling must not hang on them. + killTreeOnTermination: true, + }; try { + // Populate the files while HEAD still holds the branch, so no other worktree can claim it + // for as long as the checkout streams. Hooks stay off here: the switch below reruns the + // checkout for them. // Submodule repos do not exist yet in a linked worktree, so recursion would fail; // syncLocalGitSubmodules materializes them below, like `git worktree add` does. // No --force: a deferred checkout runs in an announced workspace, so anything written // there meanwhile (a terminal, an editor) must fail the checkout, not be overwritten. - using checkoutProc = execFileAsync( + using populateProc = execFileAsync( "git", - ["-C", workspacePath, "checkout", "--progress", "--no-recurse-submodules", branchName], + [ + "-C", + workspacePath, + "-c", + "core.hooksPath=/dev/null", + "checkout", + "--quiet", + "--progress", + "--no-recurse-submodules", + branchName, + ], { - ...noHooksEnv, + ...checkoutOptions, + signal: params.checkoutAbortSignal ?? params.abortSignal, // git delays progress output by 2s, which hides it for most checkouts. env: { ...noHooksEnv?.env, GIT_PROGRESS_DELAY: "0" }, - onStderrData: (chunk) => progress.push(chunk), - // Smudge filters and hooks inherit git's pipes; cancelling must not hang on them. - killTreeOnTermination: true, } ); - const { stdout } = await checkoutProc.result; + stdout.push((await populateProc.result).stdout); + // The files are in place; switch onto the branch from an unborn ref so trusted + // post-checkout hooks receive the same arguments as a plain `git worktree add` (null old + // commit, new-worktree flag). Nothing is written, so the branch is unclaimed only for the + // instant between these two commands. + using unbornProc = execFileAsync( + "git", + ["-C", workspacePath, "symbolic-ref", "HEAD", `refs/heads/xum-unborn-${randomUUID()}`], + noHooksEnv + ); + await unbornProc.result; + using switchProc = execFileAsync( + "git", + ["-C", workspacePath, "checkout", "--no-recurse-submodules", branchName], + checkoutOptions + ); + stdout.push((await switchProc.result).stdout); progress.flush(); - for (const line of [...output, ...stdout.split(/[\r\n]/)]) { + for (const line of [...output, ...stdout.flatMap((text) => text.split(/[\r\n]/))]) { if (line) initLogger.logStdout(line); } } catch (error) { progress.flush(); - // A retained (deferred) workspace must not be left on the placeholder ref with the empty - // pre-checkout index: a later commit would land on the placeholder or record every + // A retained (deferred) workspace must not be left on the placeholder ref or with the + // empty pre-checkout index: a later commit would land on the placeholder or record every // tracked file as deleted. Put HEAD back on the branch and rebuild the index from it, // leaving the working tree alone. Runs without the caller's signal because an aborted // checkout needs the restore most. diff --git a/tests/ipc/workspace/init.test.ts b/tests/ipc/workspace/init.test.ts index c4d925a75a8..d3a52d72fe1 100644 --- a/tests/ipc/workspace/init.test.ts +++ b/tests/ipc/workspace/init.test.ts @@ -551,6 +551,58 @@ describeIntegration("Workspace init hook", () => { 20000 ); + test.concurrent( + "archiving once the files have landed stops the rest of materialization", + async () => { + // Only the file checkout may outlive an archive; what follows it (here a trusted + // post-checkout hook standing in for submodule or fast-forward work) must stop instead + // of holding the archive for as long as it runs. + const env = await createTestEnvironment(); + const execAsync = promisify(exec); + const tempGitRepo = await createTempGitRepoWithInitHook({ exitCode: 0 }); + const hookStarted = path.join(tempGitRepo, ".git", "post-checkout-started"); + await fs.writeFile( + path.join(tempGitRepo, ".git", "hooks", "post-checkout"), + `#!/bin/sh\n: > "${hookStarted}"\nsleep 600\n`, + { mode: 0o755 } + ); + + try { + const branchName = generateBranchName("deferred-archive-after-checkout"); + const createResult = await createWorkspace(env, tempGitRepo, branchName); + expect(createResult.success).toBe(true); + if (!createResult.success) return; + const workspaceId = createResult.metadata.id; + const workspacePath = createResult.metadata.namedWorkspacePath; + + const deadline = Date.now() + 8000; + while (Date.now() < deadline) { + try { + await fs.access(hookStarted); + break; + } catch { + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } + const client = resolveOrpcClient(env); + const archiveResult = await client.workspace.archive({ workspaceId }); + expect(archiveResult.success).toBe(true); + + expect(await fs.readFile(path.join(workspacePath, "README.md"), "utf8")).toBe("test\n"); + const { stdout: head } = await execAsync("git symbolic-ref HEAD", { cwd: workspacePath }); + expect(head.trim()).toBe(`refs/heads/${branchName}`); + const { stdout: status } = await execAsync("git status --porcelain", { + cwd: workspacePath, + }); + expect(status).toBe(""); + } finally { + await cleanupTestEnvironment(env); + await cleanupTempGitRepo(tempGitRepo); + } + }, + 20000 + ); + test.concurrent( "should persist init state to disk for replay across page reloads", async () => { From e501fbb4fd955786cd516a18b0a1630b6351ca49 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:59:16 +0000 Subject: [PATCH 16/17] fix(worktree): abortable xumignore sync, no double claim after a lost switch, failed card after an app exit - syncXumignoreFiles takes the init abort signal: the git ls-files call is cancellable and each copy checks the signal, so archive no longer waits on large ignored-file syncs. - If another worktree claims the branch in the instant between the placeholder flip and the hook switch, the failure path detaches HEAD at the branch tip instead of re-attaching, so the rival stays the sole holder and the card reports git's error (spy-injected rival test). - startInit persists the running record; replayInit finalizes a running record that no live init owns as exit code -1 with an interruption line, so a workspace whose creation died with the app shows a failed card instead of looking complete. Archive deletes the record it orphans so an archived init is not reported as an app exit. - serverUpdateRestartBlockers waits for the deferred init to settle before enabling the updater (the checkout is a blocker until then). --- src/node/services/initStateManager.test.ts | 47 +++++++++- src/node/services/initStateManager.ts | 30 +++++++ src/node/services/workspaceService.ts | 2 + src/node/worktree/WorktreeManager.test.ts | 87 +++++++++++++++++++ src/node/worktree/WorktreeManager.ts | 32 +++++-- src/node/worktree/xumignore.test.ts | 19 ++++ src/node/worktree/xumignore.ts | 42 +++++---- tests/ipc/serverUpdateRestartBlockers.test.ts | 9 +- 8 files changed, 243 insertions(+), 25 deletions(-) diff --git a/src/node/services/initStateManager.test.ts b/src/node/services/initStateManager.test.ts index 9ce4c34b1b7..f89ad6809ed 100644 --- a/src/node/services/initStateManager.test.ts +++ b/src/node/services/initStateManager.test.ts @@ -192,6 +192,44 @@ describe("InitStateManager", () => { expect((events[3] as { exitCode: number }).exitCode).toBe(1); }); + it("finalizes an init left running by an earlier process as a failed creation on replay", async () => { + const workspaceId = "test-workspace"; + const events: Array = []; + + manager.startInit(workspaceId, "/path/to/hook"); + manager.appendOutput(workspaceId, "Checking out files...", false, true); + // The running record lands asynchronously; a new process would then find it with no + // in-memory state. + let persisted = await manager.readInitStatus(workspaceId); + for (let attempt = 0; persisted === null && attempt < 100; attempt++) { + await new Promise((resolve) => setTimeout(resolve, 10)); + persisted = await manager.readInitStatus(workspaceId); + } + expect(persisted?.status).toBe("running"); + manager.clearInMemoryState(workspaceId); + + manager.on("init-start", (event: WorkspaceInitEvent & { workspaceId: string }) => + events.push(event) + ); + manager.on("init-output", (event: WorkspaceInitEvent & { workspaceId: string }) => + events.push(event) + ); + manager.on("init-end", (event: WorkspaceInitEvent & { workspaceId: string }) => + events.push(event) + ); + + await manager.replayInit(workspaceId); + + expect(events.map((event) => event.type)).toEqual(["init-start", "init-output", "init-end"]); + expect((events[1] as { isError?: boolean }).isError).toBe(true); + expect((events[2] as { exitCode: number }).exitCode).toBe(-1); + // Finalized on disk, so the next replay sees the same failed creation. + expect((await manager.readInitStatus(workspaceId))?.status).toBe("error"); + events.length = 0; + await manager.replayInit(workspaceId); + expect(events.map((event) => event.type)).toEqual(["init-start", "init-output", "init-end"]); + }); + it("should not replay if no state exists", async () => { const workspaceId = "nonexistent-workspace"; const events: Array = []; @@ -377,14 +415,19 @@ describe("InitStateManager", () => { await fs.mkdir(sessionDir, { recursive: true }); let releaseLock: (() => void) | undefined; + let lockAcquired: () => void; + const lockAcquiredPromise = new Promise((resolve) => { + lockAcquired = resolve; + }); const lockHeld = workspaceFileLocks.withLock(workspaceId, async () => { + lockAcquired(); await new Promise((resolve) => { releaseLock = resolve; }); }); - // Let the lock callback run so releaseLock is set. - await Promise.resolve(); + // startInit's running-record write is queued ahead of this lock; wait until it is ours. + await lockAcquiredPromise; if (!releaseLock) { throw new Error("Expected workspace file lock to be held"); } diff --git a/src/node/services/initStateManager.ts b/src/node/services/initStateManager.ts index 0a64b869b7d..8b39462cd13 100644 --- a/src/node/services/initStateManager.ts +++ b/src/node/services/initStateManager.ts @@ -42,6 +42,10 @@ export interface InitStatus { */ type InitHookState = InitStatus; +/** Appended when replay finds a creation record that no live init owns (the app exited mid-way). */ +const INTERRUPTED_INIT_LINE = + "Workspace creation was interrupted: Xum exited before it finished. Delete and recreate this workspace."; + /** * InitStateManager - Manages init hook lifecycle with persistence and replay. * @@ -183,6 +187,13 @@ export class InitStateManager extends EventEmitter { }; this.store.setState(workspaceId, state); + // Persisted while running so an app exit mid-creation leaves a record for replayInit to + // finalize; per-workspace writes are serialized, so endInit's later write lands after it. + void this.store.persist( + workspaceId, + { ...state, lines: [] }, + { shouldWrite: () => this.store.hasState(workspaceId) } + ); // Create completion promise for this init // This allows multiple tools to await the same init without event listeners @@ -379,6 +390,25 @@ export class InitStateManager extends EventEmitter { * init state is visible after page reloads. */ async replayInit(workspaceId: string): Promise { + if (!this.store.hasState(workspaceId)) { + const persisted = await this.store.readPersisted(workspaceId); + if (persisted?.status === "running") { + // Written by startInit and never finalized, with no live init here: the process that ran + // it is gone and the checkout may be empty or partial. Record the failure once so this + // and every later replay show the creation as failed. + const endTime = Date.now(); + await this.store.persist(workspaceId, { + ...persisted, + status: "error", + exitCode: -1, + endTime, + lines: [ + ...(persisted.lines ?? []), + { line: INTERRUPTED_INIT_LINE, isError: true, timestamp: endTime }, + ], + }); + } + } // Pass workspaceId as context for serialization await this.store.replay(workspaceId, { workspaceId }); } diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 2231f9620ce..f42901c424e 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -8933,6 +8933,8 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { } this.initStateManager.clearInMemoryState(workspaceId); + // The running record startInit persisted would otherwise read as an app exit on replay. + await this.initStateManager.deleteInitStatus(workspaceId); // Clearing init state prevents init-end from firing (createInitLogger.logComplete() bails when // state is missing). If archiving fails before we persist archivedAt (e.g., beforeArchive hook diff --git a/src/node/worktree/WorktreeManager.test.ts b/src/node/worktree/WorktreeManager.test.ts index 79e104bf660..d3a0f00f6f4 100644 --- a/src/node/worktree/WorktreeManager.test.ts +++ b/src/node/worktree/WorktreeManager.test.ts @@ -635,6 +635,93 @@ describe("WorktreeManager.createWorkspace", () => { } }, 20_000); + it("detaches instead of re-attaching when the branch is claimed during the hook switch", async () => { + const branchName = "feature-claimed-in-gap"; + const fixture = await createWorktreeManagerFixture(); + const rival = path.join(fixture.rootDir, "rival"); + const realExecFile = disposableExec.execFileAsync; + // Claim the branch from another worktree in the instant HEAD sits on the placeholder. + const execSpy = spyOn(disposableExec, "execFileAsync").mockImplementation( + (file, args, options) => { + const proc = realExecFile(file, args, options); + if ( + file === "git" && + args.includes("symbolic-ref") && + args.some((arg) => arg.startsWith("refs/heads/xum-unborn-")) + ) { + const result = proc.result; + Object.defineProperty(proc, "result", { + value: result.then((output) => { + execFileSync("git", ["worktree", "add", "--no-checkout", rival, branchName], { + cwd: fixture.projectPath, + stdio: "ignore", + }); + return output; + }), + }); + } + return proc; + } + ); + try { + const result = await fixture.manager.createWorkspace({ + projectPath: fixture.projectPath, + branchName, + trunkBranch: "main", + skipRemoteSync: true, + trusted: true, + initLogger: fixture.initLogger, + deferMaterialization: true, + }); + expect(result.success).toBe(true); + if (!result.success || !result.workspacePath) throw new Error("Expected reservation"); + + const failure = await fixture.manager + .materializeWorkspace( + { + projectPath: fixture.projectPath, + workspacePath: result.workspacePath, + branchName, + trunkBranch: "main", + trusted: true, + initLogger: fixture.initLogger, + }, + result.pendingMaterialization! + ) + .then( + () => undefined, + (error: unknown) => error + ); + expect(failure).toBeInstanceOf(Error); + expect((failure as Error).message).toMatch(/already (checked out|used by worktree)/); + // The rival keeps the branch alone; this worktree stays usable, detached at the tip. + const holders = execFileSync("git", ["worktree", "list", "--porcelain"], { + cwd: fixture.projectPath, + }) + .toString() + .split("\n\n") + .filter((block) => block.includes(`branch refs/heads/${branchName}`)) + .map((block) => block.split("\n")[0]); + expect(holders).toEqual([`worktree ${rival}`]); + expect( + execFileSync("git", ["rev-parse", "HEAD"], { cwd: result.workspacePath }).toString().trim() + ).toBe( + execFileSync("git", ["rev-parse", branchName], { cwd: fixture.projectPath }) + .toString() + .trim() + ); + expect( + execFileSync("git", ["status", "--porcelain"], { cwd: result.workspacePath }).toString() + ).toBe(""); + expect(await fsPromises.readFile(path.join(result.workspacePath, "README.md"), "utf8")).toBe( + "hello\n" + ); + } finally { + execSpy.mockRestore(); + await fixture.cleanup(); + } + }, 20_000); + it("refuses to overwrite files written into the reserved worktree and returns to the branch", async () => { const branchName = "feature-stray-file"; const fixture = await createWorktreeManagerFixture(); diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index b4edf760f63..69457cc5ac6 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -308,6 +308,7 @@ export class WorktreeManager { // Smudge filters and hooks inherit git's pipes; cancelling must not hang on them. killTreeOnTermination: true, }; + let headMoved = false; try { // Populate the files while HEAD still holds the branch, so no other worktree can claim it // for as long as the checkout streams. Hooks stay off here: the switch below reruns the @@ -347,6 +348,7 @@ export class WorktreeManager { noHooksEnv ); await unbornProc.result; + headMoved = true; using switchProc = execFileAsync( "git", ["-C", workspacePath, "checkout", "--no-recurse-submodules", branchName], @@ -366,12 +368,28 @@ export class WorktreeManager { // checkout needs the restore most. const restoreOptions = noHooksEnv?.env ? { env: noHooksEnv.env } : undefined; try { - using restoreProc = execFileAsync( - "git", - ["-C", workspacePath, "symbolic-ref", "HEAD", `refs/heads/${branchName}`], - restoreOptions - ); - await restoreProc.result; + if (headMoved) { + // If another worktree claimed the branch during that instant, re-attaching would + // leave two worktrees on it; detach at the tip instead and let the error report it. + const claimedElsewhere = (await this.listWorktreeBlocks(projectPath, restoreOptions)) + .filter((block) => this.findWorktreeBlockByPath([block], workspacePath) === undefined) + .some((block) => this.getWorktreeBranchName(block) === branchName); + using restoreProc = execFileAsync( + "git", + claimedElsewhere + ? [ + "-C", + workspacePath, + "update-ref", + "--no-deref", + "HEAD", + `refs/heads/${branchName}`, + ] + : ["-C", workspacePath, "symbolic-ref", "HEAD", `refs/heads/${branchName}`], + restoreOptions + ); + await restoreProc.result; + } using resetProc = execFileAsync( "git", ["-C", workspacePath, "reset", "--quiet"], @@ -390,7 +408,7 @@ export class WorktreeManager { // Sync gitignored files declared in .xumignore (e.g. .env) // before init hooks run so they have access to secrets/config initLogger.logStep("Syncing .xumignore files..."); - await syncXumignoreFiles(projectPath, workspacePath); + await syncXumignoreFiles(projectPath, workspacePath, params.abortSignal); if (pending.fastForwardFromOrigin) { await this.fastForwardToOrigin(workspacePath, trunkBranch, initLogger, noHooksEnv); diff --git a/src/node/worktree/xumignore.test.ts b/src/node/worktree/xumignore.test.ts index 255476e8299..cfd6cbec021 100644 --- a/src/node/worktree/xumignore.test.ts +++ b/src/node/worktree/xumignore.test.ts @@ -74,6 +74,25 @@ describe("syncXumignoreFiles", () => { expect(copied).toBe("SECRET=abc\n"); }); + it("stops on cancellation instead of copying the rest", async () => { + await fsPromises.writeFile(path.join(projectPath, ".xumignore"), "!.env\n"); + const controller = new AbortController(); + controller.abort(); + + const failure = await syncXumignoreFiles(projectPath, worktreePath, controller.signal).then( + () => undefined, + (error: unknown) => error + ); + + expect(failure).toBeInstanceOf(Error); + expect( + await fsPromises.access(path.join(worktreePath, ".env")).then( + () => true, + () => false + ) + ).toBe(false); + }); + it("falls back to .muxignore when the canonical file is absent", async () => { await fsPromises.writeFile(path.join(projectPath, ".muxignore"), "!.env\n"); diff --git a/src/node/worktree/xumignore.ts b/src/node/worktree/xumignore.ts index a87b1d6f2be..34237de9d15 100644 --- a/src/node/worktree/xumignore.ts +++ b/src/node/worktree/xumignore.ts @@ -22,7 +22,11 @@ export function parseXumignorePatterns(content: string): string[] { * Get gitignored files matching the selected .xumignore or legacy .muxignore patterns. * Uses `git ls-files` for consistency with the project's git-first philosophy. */ -async function getFilesToSync(projectPath: string, patterns: string[]): Promise { +async function getFilesToSync( + projectPath: string, + patterns: string[], + abortSignal?: AbortSignal +): Promise { // Patterns that start with ! are "negative" entries (e.g. from `!!foo`) and // cannot select candidate files on their own, so only positive patterns are // used for git prefiltering. @@ -50,17 +54,21 @@ async function getFilesToSync(projectPath: string, patterns: string[]): Promise< .filter((pattern, index, all) => all.indexOf(pattern) === index); if (includePathspecs.length === 0) return []; - using proc = execFileAsync("git", [ - "-C", - projectPath, - "ls-files", - "--others", - "--ignored", - "--exclude-standard", - "-z", - "--", - ...includePathspecs, - ]); + using proc = execFileAsync( + "git", + [ + "-C", + projectPath, + "ls-files", + "--others", + "--ignored", + "--exclude-standard", + "-z", + "--", + ...includePathspecs, + ], + abortSignal ? { signal: abortSignal } : undefined + ); const { stdout } = await proc.result; const ignoredFiles = stdout .split("\0") @@ -78,11 +86,13 @@ async function getFilesToSync(projectPath: string, patterns: string[]): Promise< * falling back to legacy .muxignore. Runs after `git worktree add` so files * like `.env` are available before project init hooks execute. * - * Best-effort: logs debug details but never throws. + * Best-effort: logs debug details but never throws, except to propagate a cancellation so + * the caller stops with it instead of copying the rest. */ export async function syncXumignoreFiles( projectPath: string, - workspacePath: string + workspacePath: string, + abortSignal?: AbortSignal ): Promise { try { let content: string | undefined; @@ -99,10 +109,11 @@ export async function syncXumignoreFiles( const patterns = parseXumignorePatterns(content); if (patterns.length === 0) return; - const filesToSync = await getFilesToSync(projectPath, patterns); + const filesToSync = await getFilesToSync(projectPath, patterns, abortSignal); let copied = 0; for (const relPath of filesToSync) { + abortSignal?.throwIfAborted(); const src = path.join(projectPath, relPath); const dest = path.join(workspacePath, relPath); @@ -127,6 +138,7 @@ export async function syncXumignoreFiles( log.debug(`xumignore: synced ${copied} file(s) to worktree`); } } catch (err) { + if (abortSignal?.aborted) throw err; // Best-effort — never let ignore-file sync break workspace creation. log.debug("xumignore: sync failed", { error: String(err) }); } diff --git a/tests/ipc/serverUpdateRestartBlockers.test.ts b/tests/ipc/serverUpdateRestartBlockers.test.ts index aa2be075323..f9fe280ccb2 100644 --- a/tests/ipc/serverUpdateRestartBlockers.test.ts +++ b/tests/ipc/serverUpdateRestartBlockers.test.ts @@ -15,7 +15,12 @@ import { shouldRunIntegrationTests, type TestEnvironment, } from "./setup"; -import { cleanupTempGitRepo, createTempGitRepo, createWorkspace } from "./helpers"; +import { + cleanupTempGitRepo, + createTempGitRepo, + createWorkspace, + waitForInitComplete, +} from "./helpers"; function monitorInternals(service: WorkspaceService) { return service as unknown as { @@ -49,6 +54,8 @@ describeIntegration("Server update restart blockers", () => { if (!result.success) throw new Error(result.error); workspaceId = result.metadata.id; workspacePath = result.metadata.namedWorkspacePath ?? repo; + // Creation finishes the checkout in the background and counts as a restart blocker until then. + await waitForInitComplete(env, workspaceId); await monitorInternals(env.services.workspaceService).bashMonitorRecoveryPromise; restart = jest.fn(() => Promise.resolve()); await env.services.updateService.enableServerUpdater( From 0668eb89eb5efa600184d86153e849f6bfaf78b9 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:56:08 +0000 Subject: [PATCH 17/17] Keep the restart blocker until the init status write lands collectRestartBlockers also counts inits whose in-memory state is still running: endInit turns that status final only after the final init-status write has landed, so a server-update restart can no longer slip in between logComplete and the write and replay a finished creation as interrupted. Soften the interrupted-creation line, since a hook-phase interruption leaves a complete checkout. Also give the archive-during-init unit mocks the deleteInitStatus the archive path now calls (red Test / Unit on e501fbb4fd). --- src/node/services/initStateManager.test.ts | 32 ++++++++++++++++++++++ src/node/services/initStateManager.ts | 13 ++++++++- src/node/services/serviceContainer.test.ts | 5 +++- src/node/services/workspaceService.test.ts | 2 ++ src/node/services/workspaceService.ts | 9 ++++-- 5 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/node/services/initStateManager.test.ts b/src/node/services/initStateManager.test.ts index f89ad6809ed..e7675b223ce 100644 --- a/src/node/services/initStateManager.test.ts +++ b/src/node/services/initStateManager.test.ts @@ -192,6 +192,38 @@ describe("InitStateManager", () => { expect((events[3] as { exitCode: number }).exitCode).toBe(1); }); + it("counts an init as running until its final status write has landed", async () => { + const workspaceId = "test-workspace"; + expect(manager.runningInitWorkspaceIds()).toEqual([]); + manager.startInit(workspaceId, "/path/to/hook"); + expect(manager.runningInitWorkspaceIds()).toEqual([workspaceId]); + + // Hold the workspace file lock so endInit's write stays queued behind it. + let releaseLock: (() => void) | undefined; + let lockAcquired: () => void; + const lockAcquiredPromise = new Promise((resolve) => { + lockAcquired = resolve; + }); + const lockHeld = workspaceFileLocks.withLock(workspaceId, async () => { + lockAcquired(); + await new Promise((resolve) => { + releaseLock = resolve; + }); + }); + await lockAcquiredPromise; + + const endInitPromise = manager.endInit(workspaceId, 0); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect((await manager.readInitStatus(workspaceId))?.status).toBe("running"); + expect(manager.runningInitWorkspaceIds()).toEqual([workspaceId]); + + releaseLock!(); + await lockHeld; + await endInitPromise; + expect((await manager.readInitStatus(workspaceId))?.status).toBe("success"); + expect(manager.runningInitWorkspaceIds()).toEqual([]); + }); + it("finalizes an init left running by an earlier process as a failed creation on replay", async () => { const workspaceId = "test-workspace"; const events: Array = []; diff --git a/src/node/services/initStateManager.ts b/src/node/services/initStateManager.ts index 8b39462cd13..c25eb2ace38 100644 --- a/src/node/services/initStateManager.ts +++ b/src/node/services/initStateManager.ts @@ -44,7 +44,7 @@ type InitHookState = InitStatus; /** Appended when replay finds a creation record that no live init owns (the app exited mid-way). */ const INTERRUPTED_INIT_LINE = - "Workspace creation was interrupted: Xum exited before it finished. Delete and recreate this workspace."; + "Workspace creation was interrupted: Xum exited before it finished. Check the checkout before using it, or recreate the workspace."; /** * InitStateManager - Manages init hook lifecycle with persistence and replay. @@ -371,6 +371,17 @@ export class InitStateManager extends EventEmitter { return this.store.getState(workspaceId); } + /** + * Workspaces whose init is still running in memory. endInit turns the in-memory status final + * only after the status write lands, so these are exactly the inits a restart would replay + * as interrupted. + */ + runningInitWorkspaceIds(): string[] { + return this.store + .getActiveWorkspaceIds() + .filter((workspaceId) => this.store.getState(workspaceId)?.status === "running"); + } + /** * Read persisted init status from disk. * Returns null if no status file exists. diff --git a/src/node/services/serviceContainer.test.ts b/src/node/services/serviceContainer.test.ts index 19927f55783..1578b431222 100644 --- a/src/node/services/serviceContainer.test.ts +++ b/src/node/services/serviceContainer.test.ts @@ -495,6 +495,8 @@ describe("ServiceContainer", () => { workspace.initSettlementPromises.set("initializing", new Promise(() => undefined)); workspace.initAbortControllers.set("initializing", new AbortController()); workspace.initAbortControllers.set("provisioning", new AbortController()); + // Controller and settlement already released; the final status write has not landed. + services.initStateManager.startInit("finishing", "/tmp/finishing/.xum/init"); workspace.removingWorkspaces.add("removing"); workspace.archivingWorkspaces.add("archiving"); workspace.archivingWorkspaces.add("removing"); @@ -512,7 +514,7 @@ describe("ServiceContainer", () => { workspace.preflightExecCounts.set("executing", 1); expect(services.collectRestartBlockers()).toEqual([ { kind: "pending-turns", count: 1 }, - { kind: "workspace-inits", count: 2 }, + { kind: "workspace-inits", count: 3 }, { kind: "workspace-lifecycle", count: 3 }, { kind: "background-processes", count: 3 }, { kind: "active-streams", count: 1 }, @@ -529,6 +531,7 @@ describe("ServiceContainer", () => { workspace.preflightExecCounts.clear(); workspace.initSettlementPromises.clear(); workspace.initAbortControllers.clear(); + services.initStateManager.clearInMemoryState("finishing"); workspace.removingWorkspaces.clear(); workspace.archivingWorkspaces.clear(); workspace.renamingWorkspaces.clear(); diff --git a/src/node/services/workspaceService.test.ts b/src/node/services/workspaceService.test.ts index 03e18ec9e7a..93f5c1a290e 100644 --- a/src/node/services/workspaceService.test.ts +++ b/src/node/services/workspaceService.test.ts @@ -16571,6 +16571,7 @@ describe("WorkspaceService archive init cancellation", () => { on: mock(() => undefined as unknown as InitStateManager), getInitState: mock((id: string) => initStates.get(id)), clearInMemoryState: clearInMemoryStateMock, + deleteInitStatus: mock(() => Promise.resolve()), }; let configState: ProjectsConfig = { @@ -18445,6 +18446,7 @@ describe("WorkspaceService init cancellation", () => { }) ), clearInMemoryState: clearInMemoryStateMock, + deleteInitStatus: mock(() => Promise.resolve()), }; const workspaceService = createWorkspaceServiceForTest({ config: mockConfig, diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index f42901c424e..2b1dd864326 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -4298,8 +4298,13 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { { kind: "workspace-inits", // Controllers exist from the start of provisioning; settlements from init start onward. - count: new Set([...this.initAbortControllers.keys(), ...this.initSettlementPromises.keys()]) - .size, + // logComplete queues the final status write without awaiting it, so the in-memory + // running state outlives both until that write lands. + count: new Set([ + ...this.initAbortControllers.keys(), + ...this.initSettlementPromises.keys(), + ...this.initStateManager.runningInitWorkspaceIds(), + ]).size, }, { kind: "workspace-lifecycle",