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..79a4a9c7441 --- /dev/null +++ b/src/browser/components/ProgressBar/ProgressBar.tsx @@ -0,0 +1,22 @@ +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..4a91378ac41 100644 --- a/src/browser/features/Messages/InitMessage.tsx +++ b/src/browser/features/Messages/InitMessage.tsx @@ -1,94 +1,156 @@ -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 + // 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]); + }, [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..731d8c960df 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); @@ -3128,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 c60abbbd56c..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,7 +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, () => aggregator.flushPendingInitOutput()); }, "init-end": (workspaceId, aggregator, data) => { applyWorkspaceChatEventToAggregator(aggregator, data); @@ -1625,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); @@ -1645,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 ); @@ -1936,6 +1949,7 @@ export class WorkspaceStore { } this.deltaIdleHandles.delete(workspaceId); } + this.idleBumpPreludes.delete(workspaceId); } /** 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..4a97915f9ea 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,13 +3069,27 @@ 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, 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.flushPendingInitOutput(), + StreamingMessageAggregator.INIT_OUTPUT_THROTTLE_MS + ); + } return true; } @@ -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,20 @@ 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(); + resultMessages.splice(insertionIndex, 0, initMessage); } // 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/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/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/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..3f1995e8601 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 */ @@ -204,6 +205,27 @@ 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; +} + +/** 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; } /** @@ -214,6 +236,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; } /** @@ -474,7 +498,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 @@ -538,6 +562,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: WorkspaceMaterializeParams, + 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..e784202738d 100644 --- a/src/node/runtime/WorktreeRuntime.ts +++ b/src/node/runtime/WorktreeRuntime.ts @@ -1,12 +1,14 @@ import type { EnsureReadyOptions, EnsureReadyResult, + PendingMaterialization, WorkspaceCreationParams, WorkspaceCreationResult, WorkspaceInitParams, WorkspaceInitResult, WorkspaceForkParams, WorkspaceForkResult, + WorkspaceMaterializeParams, } from "./Runtime"; import { WORKSPACE_REPO_MISSING_ERROR } from "./Runtime"; import { LocalBaseRuntime } from "./LocalBaseRuntime"; @@ -103,9 +105,30 @@ export class WorktreeRuntime extends LocalBaseRuntime { abortSignal: params.abortSignal, env: params.env, trusted: params.trusted, + deferMaterialization: params.deferMaterialization, }); } + async materializeWorkspace( + params: WorkspaceMaterializeParams, + 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, + checkoutAbortSignal: params.checkoutAbortSignal, + env: params.env, + trusted: params.trusted, + }, + pending + ); + } + async initWorkspace(params: WorkspaceInitParams): Promise { return this.initLocalWorkspace(params, "worktree"); } 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/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 efadc52de4a..e7675b223ce 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"; @@ -191,6 +192,76 @@ 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 = []; + + 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 = []; @@ -211,6 +282,124 @@ 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) ?? null + ); + 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"; @@ -258,14 +447,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 3c2847cc8c3..c25eb2ace38 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. @@ -12,6 +13,7 @@ import { getErrorMessage } from "@/common/utils/errors"; export interface TimedLine { line: string; isError: boolean; // true if from stderr + step?: true; timestamp: number; } @@ -40,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. Check the checkout before using it, or recreate the workspace."; + /** * InitStateManager - Manages init hook lifecycle with persistence and replay. * @@ -140,6 +146,7 @@ export class InitStateManager extends EventEmitter { workspaceId, line: timedLine.line, isError: timedLine.isError, + step: timedLine.step ? true : undefined, timestamp: timedLine.timestamp, // Use original timestamp for replay lineNumber: truncatedLines + index, replay: true, @@ -180,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 @@ -244,7 +258,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 +268,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 || undefined }; // Truncation: keep only the most recent MAX_LINES if (state.lines.length >= INIT_HOOK_MAX_LINES) { @@ -267,13 +281,26 @@ export class InitStateManager extends EventEmitter { this.emit("init-output", { type: "init-output", workspaceId, - line, - isError, - timestamp, + ...timedLine, 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: clamp(Math.round(percent), 0, 100), + timestamp: Date.now(), + } satisfies WorkspaceInitEvent & { workspaceId: string }); + } + /** * Finalize init hook execution. * Updates state, persists to disk, emits init-end event, and resolves completion promise. @@ -344,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. @@ -363,6 +401,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/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/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/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.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 205e7a2126e..2b1dd864326 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,145 @@ 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, 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; + 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" + ); + // 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 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)) checkoutAbort.abort(); + }; + args.initAbortController.signal.addEventListener("abort", forwardRemovalAbort); + let materializeError: unknown; + try { + forwardRemovalAbort(); + await runtime.materializeWorkspace( + { ...initParams, checkoutAbortSignal: checkoutAbort.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). + 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; + } + 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); + } + setWorkspaceGoalService(service: WorkspaceGoalService): void { this.workspaceGoalService = service; } @@ -4010,8 +4155,10 @@ 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) => + this.initStateManager.reportProgress(workspaceId, label, percent), logStdout: (line: string) => { if (!hasInitState()) { return; @@ -4151,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", @@ -4885,7 +5037,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)) { @@ -5022,7 +5182,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. @@ -5066,6 +5226,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; @@ -5133,15 +5295,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). @@ -5189,52 +5355,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 @@ -5263,24 +5398,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(); @@ -8797,6 +8938,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/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 466acff9b4c..d3a0f00f6f4 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,179 @@ 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.includes("checkout") && !checkoutStarted) { + checkoutStarted = true; + expect(existsSync(path.join(workspacePath, "README.md"))).toBe(false); + } + const proc = realExecFile(file, args, options); + if (file === "git" && args[2] === "worktree" && args[3] === "add") { + // Inject stdout so forwarding coverage does not depend on Git's output. + 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(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(stdout.some((line) => /^Updating files: 100% \(1\/1\), done\.$/.test(line))).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(); + 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 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", "-A"], { + 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, + 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"); + // 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( + 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", @@ -256,6 +430,530 @@ 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("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("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("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(); + 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, 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(); + } + }, 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); + // 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, + true, + 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(); + 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 7ed25fa9044..69457cc5ac6 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -1,6 +1,8 @@ +import { randomUUID } from "crypto"; import * as fsPromises from "fs/promises"; import * as path from "path"; import type { + PendingMaterialization, WorkspaceCreationResult, WorkspaceForkParams, WorkspaceForkResult, @@ -26,6 +28,7 @@ import { } from "@/constants/terminationTimeouts"; import { syncLocalGitSubmodules } from "@/node/runtime/submoduleSync"; import { syncXumignoreFiles } from "./xumignore"; +import { GitProgressParser, isGitProgressLine } from "./gitProgress"; type GitExecOptions = Pick | undefined; @@ -94,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). @@ -169,58 +174,66 @@ 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( - "git", - ["-C", projectPath, "worktree", "add", "-b", branchName, workspacePath, newBranchBase], - noHooksEnv - ); - await proc.result; - createdBranch = true; - } + // 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( + "git", + [ + "-C", + projectPath, + "worktree", + "add", + "--no-checkout", + ...(branchExists + ? [workspacePath, branchName] + : ["-b", branchName, workspacePath, newBranchBase]), + ], + noHooksEnv + ); + const addResult = await addProc.result; + createdBranch = !branchExists; worktreeCreated = true; + // 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); + } - 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 }; } 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 { @@ -250,6 +263,169 @@ 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: { + projectPath: string; + workspacePath: string; + branchName: string; + trunkBranch: string; + initLogger: InitLogger; + abortSignal?: AbortSignal; + checkoutAbortSignal?: 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..."); + // 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) => 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, + }; + 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 + // 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 populateProc = execFileAsync( + "git", + [ + "-C", + workspacePath, + "-c", + "core.hooksPath=/dev/null", + "checkout", + "--quiet", + "--progress", + "--no-recurse-submodules", + branchName, + ], + { + ...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" }, + } + ); + 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; + headMoved = true; + 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.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 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. + const restoreOptions = noHooksEnv?.env ? { env: noHooksEnv.env } : undefined; + try { + 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"], + restoreOptions + ); + await resetProc.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)); + } + + 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, params.abortSignal); + + 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/src/node/worktree/gitProgress.test.ts b/src/node/worktree/gitProgress.test.ts new file mode 100644 index 00000000000..731b56ce7f1 --- /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(["Updating files: 34% (34/100)"]); + }); + + 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..9cc34b58820 --- /dev/null +++ b/src/node/worktree/gitProgress.ts @@ -0,0 +1,45 @@ +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; + 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 (let index = 0; index < lines.length; index += 2) { + const line = lines[index]; + if (!line) continue; + const match = PROGRESS_LINE.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 || lines[index + 1] === "\n") this.onOutput(line); + } + } + + flush(): void { + if (this.buffer) this.onOutput(this.buffer); + this.buffer = ""; + } +} 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/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/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( diff --git a/tests/ipc/workspace/init.test.ts b/tests/ipc/workspace/init.test.ts index 2581c485520..d3a52d72fe1 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,291 @@ 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( + "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( + "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( + "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 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 }); + 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"]); + 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 { + await cleanupTestEnvironment(env); + await cleanupTempGitRepo(tempGitRepo); + } + }, + 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 () => { @@ -360,15 +645,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"); + }); +}); 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.