diff --git a/apps/server/integration/OrchestrationEngineHarness.integration.ts b/apps/server/integration/OrchestrationEngineHarness.integration.ts index 682b5ab08..64f4735a7 100644 --- a/apps/server/integration/OrchestrationEngineHarness.integration.ts +++ b/apps/server/integration/OrchestrationEngineHarness.integration.ts @@ -360,6 +360,7 @@ export const makeOrchestrationIntegrationHarness = ( refreshStatus: () => Effect.die("refreshStatus should not be called in this test"), streamStatus: () => Stream.empty, observeLocalStatus: () => Stream.empty, + observeMissingCheckouts: () => Stream.empty, }), ), Layer.provideMerge( diff --git a/apps/server/src/git/GitWorkflowService.test.ts b/apps/server/src/git/GitWorkflowService.test.ts index b7e7fd039..fb9233557 100644 --- a/apps/server/src/git/GitWorkflowService.test.ts +++ b/apps/server/src/git/GitWorkflowService.test.ts @@ -1,6 +1,9 @@ import { assert, describe, it, vi } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import * as NodeOS from "node:os"; +import * as NodePath from "node:path"; import * as GitManager from "./GitManager.ts"; import * as GitWorkflowService from "./GitWorkflowService.ts"; @@ -9,6 +12,7 @@ import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; function makeLayer(input: { readonly detect: VcsDriverRegistry.VcsDriverRegistryShape["detect"] }) { return GitWorkflowService.layer.pipe( + Layer.provideMerge(NodeServices.layer), Layer.provide( Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({ detect: input.detect, @@ -43,6 +47,7 @@ describe("GitWorkflowService", () => { }), ); const testLayer = GitWorkflowService.layer.pipe( + Layer.provideMerge(NodeServices.layer), Layer.provide( Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({ detect: () => Effect.succeed(makeGitHandle("/repo")), @@ -67,7 +72,7 @@ describe("GitWorkflowService", () => { it.effect("returns an empty local status when no VCS repository is detected", () => Effect.gen(function* () { const workflow = yield* GitWorkflowService.GitWorkflowService; - const status = yield* workflow.localStatus({ cwd: "/not-a-repo" }); + const status = yield* workflow.localStatus({ cwd: NodeOS.tmpdir() }); assert.deepStrictEqual(status, { isRepo: false, @@ -94,7 +99,7 @@ describe("GitWorkflowService", () => { it.effect("returns an empty full status when no VCS repository is detected", () => Effect.gen(function* () { const workflow = yield* GitWorkflowService.GitWorkflowService; - const status = yield* workflow.status({ cwd: "/not-a-repo" }); + const status = yield* workflow.status({ cwd: NodeOS.tmpdir() }); assert.deepStrictEqual(status, { isRepo: false, @@ -123,12 +128,53 @@ describe("GitWorkflowService", () => { ), ); + // Both cases reach here as "no driver handle", but they mean opposite things + // to the user: an empty directory invites `git init`, a deleted checkout must + // not. Offering to initialize a repository at a path the user's agent removed + // would quietly replace their work with an unrelated empty repo. + it.effect("marks a status whose directory no longer exists", () => + Effect.gen(function* () { + const workflow = yield* GitWorkflowService.GitWorkflowService; + const gone = NodePath.join(NodeOS.tmpdir(), "threadlines-definitely-not-here"); + + const local = yield* workflow.localStatus({ cwd: gone }); + const full = yield* workflow.status({ cwd: gone }); + + assert.isFalse(local.isRepo); + assert.isTrue(local.pathMissing); + assert.isTrue(full.pathMissing); + }).pipe( + Effect.provide( + makeLayer({ + detect: () => Effect.succeed(null), + }), + ), + ), + ); + + it.effect("leaves pathMissing unset for a directory that is simply not a repository", () => + Effect.gen(function* () { + const workflow = yield* GitWorkflowService.GitWorkflowService; + const status = yield* workflow.localStatus({ cwd: NodeOS.tmpdir() }); + + assert.isFalse(status.isRepo); + assert.isUndefined(status.pathMissing); + }).pipe( + Effect.provide( + makeLayer({ + detect: () => Effect.succeed(null), + }), + ), + ), + ); + it.effect("does not call GitManager status methods when no VCS repository is detected", () => { const localStatus = vi.fn(); const remoteStatus = vi.fn(); const status = vi.fn(); const testLayer = GitWorkflowService.layer.pipe( + Layer.provideMerge(NodeServices.layer), Layer.provide( Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({ detect: () => Effect.succeed(null), @@ -215,6 +261,7 @@ describe("GitWorkflowService", () => { }); const testLayer = GitWorkflowService.layer.pipe( + Layer.provideMerge(NodeServices.layer), Layer.provide( Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({ resolve: () => Effect.succeed(makeGitHandle()), @@ -252,6 +299,7 @@ describe("GitWorkflowService", () => { const mergeRef = vi.fn(); const pushCurrentBranch = vi.fn(); const testLayer = GitWorkflowService.layer.pipe( + Layer.provideMerge(NodeServices.layer), Layer.provide( Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({ resolve: () => Effect.succeed(makeGitHandle()), diff --git a/apps/server/src/git/GitWorkflowService.ts b/apps/server/src/git/GitWorkflowService.ts index 76ed22705..5460c981a 100644 --- a/apps/server/src/git/GitWorkflowService.ts +++ b/apps/server/src/git/GitWorkflowService.ts @@ -2,6 +2,7 @@ import * as nodePath from "node:path"; import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import { @@ -66,6 +67,7 @@ import { type GitWorktreeEntry, } from "../vcs/GitVcsDriver.ts"; import { VcsDriverRegistry, type VcsDriverHandle } from "../vcs/VcsDriverRegistry.ts"; +import { checkoutPresence } from "../vcs/CheckoutPresence.ts"; export interface GitWorkflowServiceShape { readonly status: ( @@ -189,9 +191,10 @@ const unsupportedGitCommand = (operation: string, cwd: string, detail: string) = detail, }); -function nonRepositoryLocalStatus(): VcsStatusLocalResult { +function nonRepositoryLocalStatus(pathMissing = false): VcsStatusLocalResult { return { isRepo: false, + ...(pathMissing ? { pathMissing: true } : {}), hasPrimaryRemote: false, isDefaultRef: false, refName: null, @@ -205,9 +208,9 @@ function nonRepositoryLocalStatus(): VcsStatusLocalResult { }; } -function nonRepositoryStatus(): VcsStatusResult { +function nonRepositoryStatus(pathMissing = false): VcsStatusResult { return { - ...nonRepositoryLocalStatus(), + ...nonRepositoryLocalStatus(pathMissing), hasUpstream: false, aheadCount: 0, behindCount: 0, @@ -237,6 +240,19 @@ export const make = Effect.fn("makeGitWorkflowService")(function* () { const registry = yield* VcsDriverRegistry; const git = yield* GitVcsDriver; const gitManager = yield* GitManager; + const fileSystem = yield* FileSystem.FileSystem; + + /** + * "Not a repository" and "not there at all" both arrive here as a null + * driver handle, and the UI turns the first into an "Initialize Git" call to + * action. Offering that for a checkout the user's agent deleted is worse than + * unhelpful, so the two are separated before the status leaves the server. + */ + const isPathMissing = (cwd: string) => + checkoutPresence(cwd).pipe( + Effect.provideService(FileSystem.FileSystem, fileSystem), + Effect.map((presence) => presence === "missing"), + ); const ensureGit = Effect.fn("GitWorkflowService.ensureGit")(function* ( operation: string, @@ -356,7 +372,7 @@ export const make = Effect.fn("makeGitWorkflowService")(function* () { ? gitManager .status(input) .pipe(Effect.map((status) => withRepositoryContext(status, input.cwd, handle))) - : Effect.succeed(nonRepositoryStatus()), + : isPathMissing(input.cwd).pipe(Effect.map(nonRepositoryStatus)), ), ), localStatus: (input) => @@ -366,7 +382,7 @@ export const make = Effect.fn("makeGitWorkflowService")(function* () { ? gitManager .localStatus(input) .pipe(Effect.map((status) => withRepositoryContext(status, input.cwd, handle))) - : Effect.succeed(nonRepositoryLocalStatus()), + : isPathMissing(input.cwd).pipe(Effect.map(nonRepositoryLocalStatus)), ), ), remoteStatus: (input, options) => diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts index 0162731b2..99612b5c0 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts @@ -401,6 +401,7 @@ describe("CheckpointReactor", () => { refreshStatus: () => Effect.die("refreshStatus should not be called in this test"), streamStatus: () => Stream.empty, observeLocalStatus: () => Stream.empty, + observeMissingCheckouts: () => Stream.empty, }); const layer = CheckpointReactorLive.pipe( diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts index 01c05fa41..7524962ee 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts @@ -30,6 +30,7 @@ import * as Layer from "effect/Layer"; import * as ManagedRuntime from "effect/ManagedRuntime"; import * as Option from "effect/Option"; import * as PubSub from "effect/PubSub"; +import * as Queue from "effect/Queue"; import * as Scope from "effect/Scope"; import * as Stream from "effect/Stream"; import { afterEach, describe, expect, it, vi } from "vite-plus/test"; @@ -78,6 +79,12 @@ const asTurnId = (value: string): TurnId => TurnId.make(value); const deriveServerPathsSync = (baseDir: string, devUrl: URL | undefined) => Effect.runSync(deriveServerPaths(baseDir, devUrl).pipe(Effect.provide(NodeServices.layer))); +// Real directories, not invented paths: the reactor refuses to start a provider +// session in a checkout that is not on disk, so a fabricated workspace root +// would make every turn in this suite fail the pre-flight. +const PROJECT_ROOT = fs.mkdtempSync(path.join(os.tmpdir(), "threadlines-project-")); +const PROJECT_WORKTREE_ROOT = fs.mkdtempSync(path.join(os.tmpdir(), "threadlines-worktree-")); + async function waitFor( predicate: () => boolean | Promise, timeoutMs = 10_000, @@ -167,6 +174,8 @@ describe("ProviderCommandReactor", () => { const { stateDir } = deriveServerPathsSync(baseDir, undefined); createdStateDirs.add(stateDir); const runtimeEventPubSub = Effect.runSync(PubSub.unbounded()); + // Feed for the broadcaster's confirmed-missing-checkout signal. + const missingCheckouts = Effect.runSync(Queue.unbounded<{ readonly cwd: string }>()); let nextSessionIndex = 1; const runtimeSessions: Array = []; const seedBuildInputs: ThreadContextSeedBuildInput[] = []; @@ -343,6 +352,19 @@ describe("ProviderCommandReactor", () => { pr: null, }), ); + // Pushed by the reactor when it finds a checkout missing, so the thread + // view's recovery affordance does not wait on the watcher's next pass. + const refreshLocalStatus = vi.fn((_: string) => + Effect.succeed({ + isRepo: false, + pathMissing: true, + hasPrimaryRemote: false, + isDefaultRef: false, + refName: null, + hasWorkingTreeChanges: false, + workingTree: { files: [], insertions: 0, deletions: 0 }, + }), + ); const generateBranchName = vi.fn((_) => Effect.fail( new TextGenerationError({ @@ -462,12 +484,14 @@ describe("ProviderCommandReactor", () => { Layer.provideMerge( Layer.succeed(VcsStatusBroadcaster, { getStatus: () => Effect.die("getStatus should not be called in this test"), - refreshLocalStatus: () => - Effect.die("refreshLocalStatus should not be called in this test"), + refreshLocalStatus, refreshStatus, streamStatus: () => Stream.die("streamStatus should not be called in this test"), observeLocalStatus: () => Stream.die("observeLocalStatus should not be called in this test"), + // Consumed by the reactor's missing-checkout watcher on start; tests + // that exercise it push through `missingCheckouts` below. + observeMissingCheckouts: () => Stream.fromQueue(missingCheckouts), }), ), Layer.provideMerge( @@ -495,7 +519,7 @@ describe("ProviderCommandReactor", () => { commandId: CommandId.make("cmd-project-create"), projectId: asProjectId("project-1"), title: "Provider Project", - workspaceRoot: "/tmp/provider-project", + workspaceRoot: PROJECT_ROOT, defaultModelSelection: modelSelection, createdAt: now, }), @@ -534,15 +558,111 @@ describe("ProviderCommandReactor", () => { stopSession, renameBranch, refreshStatus, + refreshLocalStatus, generateBranchName, generateThreadTitle, seedBuildInputs, runtimeSessions, stateDir, + missingCheckouts, drain, }; } + // The incident this guards: an agent merged its PR and deleted the worktree + // it was running in. The next turn spawned into a directory that was gone and + // surfaced as "Claude Code native binary not found", with a Retry that failed + // the same way. The session must never be started at all. + it("refuses to start a session in a checkout that no longer exists", async () => { + const harness = await createHarness(); + const now = "2026-01-01T00:00:00.000Z"; + const deletedWorktree = path.join(PROJECT_WORKTREE_ROOT, "deleted-by-agent"); + + await Effect.runPromise( + harness.engine.dispatch({ + type: "thread.meta.update", + commandId: CommandId.make("cmd-thread-dead-checkout"), + threadId: ThreadId.make("thread-1"), + branch: "feature/merged", + worktreePath: deletedWorktree, + }), + ); + + await Effect.runPromise( + harness.engine.dispatch({ + type: "thread.turn.start", + commandId: CommandId.make("cmd-turn-start-dead-checkout"), + threadId: ThreadId.make("thread-1"), + message: { + messageId: asMessageId("user-message-dead-checkout"), + role: "user", + text: "Keep going.", + attachments: [], + }, + interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE, + runtimeMode: "approval-required", + createdAt: now, + }), + ); + + await waitFor(async () => { + const model = await harness.readModel(); + return ( + model.threads.find((entry) => entry.id === ThreadId.make("thread-1"))?.activities ?? [] + ).some((activity) => activity.kind === "thread.checkout.missing"); + }); + + // The adapter is never reached: no process is spawned in a directory that + // is not there, so nothing can misreport it as a missing binary. + expect(harness.startSession.mock.calls.length).toBe(0); + // The checkout's status is refreshed so the thread view can show the + // recovery actions straight away rather than on the watcher's next pass. + expect(harness.refreshLocalStatus.mock.calls[0]?.[0]).toBe(deletedWorktree); + + const model = await harness.readModel(); + const activity = ( + model.threads.find((entry) => entry.id === ThreadId.make("thread-1"))?.activities ?? [] + ).find((entry) => entry.kind === "thread.checkout.missing"); + expect(activity?.payload).toMatchObject({ + cwd: deletedWorktree, + branch: "feature/merged", + projectCwd: PROJECT_ROOT, + }); + // No generic provider failure alongside it: the thread view keys the + // recovery affordance off this activity instead of a dead-end Retry. + expect( + ( + model.threads.find((entry) => entry.id === ThreadId.make("thread-1"))?.activities ?? [] + ).some((entry) => entry.kind === "provider.turn.start.failed"), + ).toBe(false); + }); + + it("announces a checkout the watcher saw disappear without waiting for a turn", async () => { + const harness = await createHarness(); + const deletedWorktree = path.join(PROJECT_WORKTREE_ROOT, "vanished"); + + await Effect.runPromise( + harness.engine.dispatch({ + type: "thread.meta.update", + commandId: CommandId.make("cmd-thread-watched-checkout"), + threadId: ThreadId.make("thread-1"), + branch: "feature/watched", + worktreePath: deletedWorktree, + }), + ); + + await Effect.runPromise(Queue.offer(harness.missingCheckouts, { cwd: deletedWorktree })); + + await waitFor(async () => { + const model = await harness.readModel(); + return ( + model.threads.find((entry) => entry.id === ThreadId.make("thread-1"))?.activities ?? [] + ).some((activity) => activity.kind === "thread.checkout.missing"); + }); + + expect(harness.startSession.mock.calls.length).toBe(0); + }); + it("reacts to thread.turn.start by ensuring session and sending provider turn", async () => { const harness = await createHarness(); const now = "2026-01-01T00:00:00.000Z"; @@ -560,7 +680,7 @@ describe("ProviderCommandReactor", () => { skills: [ { name: "review", - path: "/tmp/provider-project/.codex/skills/review/SKILL.md", + path: `${PROJECT_ROOT}/.codex/skills/review/SKILL.md`, }, ], }, @@ -579,13 +699,13 @@ describe("ProviderCommandReactor", () => { skills: [ { name: "review", - path: "/tmp/provider-project/.codex/skills/review/SKILL.md", + path: `${PROJECT_ROOT}/.codex/skills/review/SKILL.md`, }, ], }); expect(harness.startSession.mock.calls[0]?.[0]).toEqual(ThreadId.make("thread-1")); expect(harness.startSession.mock.calls[0]?.[1]).toMatchObject({ - cwd: "/tmp/provider-project", + cwd: PROJECT_ROOT, modelSelection: { instanceId: ProviderInstanceId.make("codex"), model: "gpt-5-codex", @@ -1118,7 +1238,7 @@ describe("ProviderCommandReactor", () => { commandId: CommandId.make("cmd-thread-branch"), threadId: ThreadId.make("thread-1"), branch: "t3code/1234abcd", - worktreePath: "/tmp/provider-project-worktree", + worktreePath: PROJECT_WORKTREE_ROOT, }), ); @@ -1159,7 +1279,7 @@ describe("ProviderCommandReactor", () => { expect(harness.generateBranchName.mock.calls[0]?.[0]).toMatchObject({ message: "Add a safer reconnect backoff.", }); - expect(harness.refreshStatus.mock.calls[0]?.[0]).toBe("/tmp/provider-project-worktree"); + expect(harness.refreshStatus.mock.calls[0]?.[0]).toBe(PROJECT_WORKTREE_ROOT); }); it("forwards codex model options through session start and turn send", async () => { @@ -2024,7 +2144,7 @@ describe("ProviderCommandReactor", () => { await waitFor(() => harness.startSession.mock.calls.length === 1); await waitFor(() => harness.sendTurn.mock.calls.length === 1); expect(harness.startSession.mock.calls[0]?.[1]).toMatchObject({ - cwd: "/tmp/provider-project", + cwd: PROJECT_ROOT, }); await Effect.runPromise( @@ -2032,7 +2152,7 @@ describe("ProviderCommandReactor", () => { type: "thread.meta.update", commandId: CommandId.make("cmd-thread-worktree-change"), threadId: ThreadId.make("thread-1"), - worktreePath: "/tmp/provider-project-worktree", + worktreePath: PROJECT_WORKTREE_ROOT, }), ); @@ -2058,7 +2178,7 @@ describe("ProviderCommandReactor", () => { expect(harness.stopSession.mock.calls.length).toBe(0); expect(harness.startSession.mock.calls[1]?.[1]).toMatchObject({ threadId: ThreadId.make("thread-1"), - cwd: "/tmp/provider-project-worktree", + cwd: PROJECT_WORKTREE_ROOT, resumeCursor: { opaque: "resume-1" }, modelSelection: { instanceId: ProviderInstanceId.make("claudeAgent"), @@ -2073,7 +2193,7 @@ describe("ProviderCommandReactor", () => { const snapshot = await harness.readModel(); return ( snapshot.threads.find((thread) => thread.id === ThreadId.make("thread-1"))?.session - ?.checkoutCwd === "/tmp/provider-project-worktree" + ?.checkoutCwd === PROJECT_WORKTREE_ROOT ); }); }); @@ -2121,7 +2241,7 @@ describe("ProviderCommandReactor", () => { ); await waitFor(() => harness.sendTurn.mock.calls.length === 1); expect(harness.startSession.mock.calls[0]?.[1]).toMatchObject({ - cwd: "/tmp/provider-project", + cwd: PROJECT_ROOT, }); // A subagent is still running inside the live runtime, and the user queues @@ -2132,7 +2252,7 @@ describe("ProviderCommandReactor", () => { type: "thread.meta.update", commandId: CommandId.make("cmd-thread-worktree-change-defer"), threadId, - worktreePath: "/tmp/provider-project-worktree", + worktreePath: PROJECT_WORKTREE_ROOT, }), ); @@ -2168,8 +2288,8 @@ describe("ProviderCommandReactor", () => { expect(deferredActivities?.[0]).toMatchObject({ tone: "info", payload: { - fromCwd: "/tmp/provider-project", - toCwd: "/tmp/provider-project-worktree", + fromCwd: PROJECT_ROOT, + toCwd: PROJECT_WORKTREE_ROOT, pendingBackgroundTaskCount: 1, }, }); @@ -2195,7 +2315,7 @@ describe("ProviderCommandReactor", () => { ); await waitFor(() => harness.startSession.mock.calls.length === 2); expect(harness.startSession.mock.calls[1]?.[1]).toMatchObject({ - cwd: "/tmp/provider-project-worktree", + cwd: PROJECT_WORKTREE_ROOT, }); }); @@ -2250,14 +2370,14 @@ describe("ProviderCommandReactor", () => { type: "thread.meta.update", commandId: CommandId.make("cmd-thread-worktree-change-idle-apply"), threadId, - worktreePath: "/tmp/provider-project-worktree", + worktreePath: PROJECT_WORKTREE_ROOT, }), ); await waitFor(async () => { const snapshot = await harness.readModel(); return ( snapshot.threads.find((thread) => thread.id === threadId)?.worktreePath === - "/tmp/provider-project-worktree" + PROJECT_WORKTREE_ROOT ); }); expect(harness.startSession.mock.calls.length).toBe(1); @@ -2268,7 +2388,7 @@ describe("ProviderCommandReactor", () => { await waitFor(() => harness.startSession.mock.calls.length === 2); expect(harness.startSession.mock.calls[1]?.[1]).toMatchObject({ threadId, - cwd: "/tmp/provider-project-worktree", + cwd: PROJECT_WORKTREE_ROOT, resumeCursor: { opaque: "resume-1" }, }); expect(harness.sendTurn.mock.calls.length).toBe(1); @@ -2280,7 +2400,7 @@ describe("ProviderCommandReactor", () => { const snapshot = await harness.readModel(); return ( snapshot.threads.find((thread) => thread.id === threadId)?.session?.checkoutCwd === - "/tmp/provider-project-worktree" + PROJECT_WORKTREE_ROOT ); }); }); @@ -2334,13 +2454,13 @@ describe("ProviderCommandReactor", () => { type: "thread.meta.update", commandId: CommandId.make("cmd-thread-worktree-change-idle-pick"), threadId, - worktreePath: "/tmp/provider-project-worktree", + worktreePath: PROJECT_WORKTREE_ROOT, }), ); await waitFor(() => harness.startSession.mock.calls.length === 2); expect(harness.startSession.mock.calls[1]?.[1]).toMatchObject({ threadId, - cwd: "/tmp/provider-project-worktree", + cwd: PROJECT_WORKTREE_ROOT, resumeCursor: { opaque: "resume-1" }, }); expect(harness.sendTurn.mock.calls.length).toBe(1); @@ -2382,7 +2502,7 @@ describe("ProviderCommandReactor", () => { type: "thread.meta.update", commandId: CommandId.make("cmd-thread-worktree-change-while-running"), threadId, - worktreePath: "/tmp/provider-project-worktree", + worktreePath: PROJECT_WORKTREE_ROOT, }), ); @@ -3315,7 +3435,7 @@ describe("ProviderCommandReactor", () => { status: "ready", runtimeMode: "approval-required", threadId: ThreadId.make("thread-1"), - cwd: "/tmp/provider-project", + cwd: PROJECT_ROOT, resumeCursor: { opaque: "resume-without-instance" }, createdAt: now, updatedAt: now, diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts index 167685577..8819f6f0d 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts @@ -4,6 +4,7 @@ import * as path from "node:path"; import { type ChatAttachment, type ChatSkillReference, + CheckoutMissingError, CommandId, EventId, type MessageId, @@ -17,6 +18,8 @@ import { type ProviderSessionForkFrom, type RuntimeMode, type ThreadContextSeed, + ThreadCheckoutMissingActivityKind, + type ThreadCheckoutMissingPayload, ThreadCheckoutSwitchDeferredActivityKind, type ThreadCheckoutSwitchDeferredPayload, ThreadForkContextPayload, @@ -39,6 +42,7 @@ import * as Cause from "effect/Cause"; import * as DateTime from "effect/DateTime"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; @@ -77,8 +81,10 @@ import { ServerSettingsService, } from "../../serverSettings.ts"; import { VcsStatusBroadcaster } from "../../vcs/VcsStatusBroadcaster.ts"; +import { checkoutPresence } from "../../vcs/CheckoutPresence.ts"; import { GitWorkflowService } from "../../git/GitWorkflowService.ts"; const isProviderAdapterRequestError = Schema.is(ProviderAdapterRequestError); +const isCheckoutMissingError = Schema.is(CheckoutMissingError); const isProviderDriverKind = Schema.is(ProviderDriverKind); const isThreadForkContextPayload = Schema.is(ThreadForkContextPayload); @@ -331,6 +337,11 @@ const make = Effect.gen(function* () { const vcsStatusBroadcaster = yield* VcsStatusBroadcaster; const textGeneration = yield* TextGeneration; const serverSettingsService = yield* ServerSettingsService; + const fileSystem = yield* FileSystem.FileSystem; + + /** Checkout existence check, bound to the layer's filesystem. Never fails. */ + const checkoutPresenceFor = (cwd: string) => + checkoutPresence(cwd).pipe(Effect.provideService(FileSystem.FileSystem, fileSystem)); const handledTurnStartKeys = yield* Cache.make({ capacity: HANDLED_TURN_START_KEY_MAX, timeToLive: HANDLED_TURN_START_KEY_TTL, @@ -354,6 +365,13 @@ const make = Effect.gen(function* () { */ const deferredCheckoutSwitchThreads = new Set(); + /** + * Checkout path most recently reported missing per thread, so the same dead + * folder is announced once rather than on every retry. Cleared as soon as the + * thread starts a session somewhere that exists. + */ + const reportedMissingCheckouts = new Map(); + const noteCheckoutSwitchDeferred = Effect.fnUntraced(function* (input: { readonly threadId: ThreadId; readonly fromCwd: string; @@ -433,6 +451,65 @@ const make = Effect.gen(function* () { createdAt: input.createdAt, }); + /** + * Records that a thread's checkout is gone, so the thread view can offer the + * way out (switch to the project root, or recreate the worktree) instead of a + * Retry that is guaranteed to fail the same way. + * + * Idempotent per checkout: the same path is only reported once per streak, so + * a user retrying, or the watcher and the pre-flight both noticing, does not + * stack duplicate rows in the conversation. The record clears once the thread + * is running somewhere that exists again. + */ + const noteCheckoutMissing = Effect.fnUntraced(function* (input: { + readonly threadId: ThreadId; + readonly payload: ThreadCheckoutMissingPayload; + readonly createdAt: string; + }) { + const alreadyReported = reportedMissingCheckouts.get(input.threadId); + if ( + alreadyReported !== undefined && + areFilesystemPathsEqual(alreadyReported, input.payload.cwd) + ) { + return; + } + reportedMissingCheckouts.set(input.threadId, input.payload.cwd); + // The thread view decides to show the recovery actions from this checkout's + // VCS status, which may still be a healthy snapshot taken before the folder + // was deleted. Refresh it here so the affordance appears with the failure + // instead of waiting for the watcher's next pass. + yield* vcsStatusBroadcaster + .refreshLocalStatus(input.payload.cwd) + .pipe(Effect.ignoreCause({ log: true })); + yield* orchestrationEngine + .dispatch({ + type: "thread.activity.append", + commandId: serverCommandId("thread-checkout-missing"), + threadId: input.threadId, + activity: { + id: EventId.make(crypto.randomUUID()), + tone: "warning", + kind: ThreadCheckoutMissingActivityKind, + summary: "This thread's folder no longer exists", + payload: input.payload, + turnId: null, + createdAt: input.createdAt, + }, + createdAt: input.createdAt, + }) + .pipe(Effect.ignoreCause({ log: true })); + }); + + /** Extracts the missing-checkout failure out of a turn/follow-up cause. */ + const checkoutMissingFromCause = (cause: Cause.Cause): CheckoutMissingError | null => { + for (const reason of cause.reasons) { + if (Cause.isFailReason(reason) && isCheckoutMissingError(reason.error)) { + return reason.error; + } + } + return null; + }; + const formatFailureDetail = (cause: Cause.Cause): string => { const failReason = cause.reasons.find(Cause.isFailReason); const providerError = isProviderAdapterRequestError(failReason?.error) @@ -777,6 +854,26 @@ const make = Effect.gen(function* () { projects: project ? [project] : [], }); + // Every provider start, restart and handoff funnels through here with the + // checkout already resolved, so this is the one place that has to confirm + // the directory is still on disk. Spawning into a directory that is gone + // fails deep inside the provider SDK with an errno that reads as a missing + // binary, which is how a deleted worktree used to present itself as a + // broken Claude install. `checkoutPresence` only answers "missing" when it + // is sure; a stat that fails for any other reason lets the start proceed. + if (effectiveCwd !== undefined && project?.kind !== "general-chat") { + const presence = yield* checkoutPresenceFor(effectiveCwd); + if (presence === "missing") { + return yield* new CheckoutMissingError({ + threadId, + cwd: effectiveCwd, + branch: thread.branch ?? null, + projectCwd: project?.workspaceRoot ?? null, + }); + } + reportedMissingCheckouts.delete(threadId); + } + const startProviderSession = ( input?: { readonly resumeCursor?: unknown; @@ -1418,6 +1515,34 @@ const make = Effect.gen(function* () { if (Cause.hasInterruptsOnly(cause)) { return Effect.void; } + // A checkout that is gone is not a provider failure and has no useful + // Retry: report it as the recoverable condition it is so the thread view + // can offer switching checkouts or recreating the folder. + const checkoutMissing = checkoutMissingFromCause(cause); + if (checkoutMissing) { + // The session still has to be released from "starting" so the composer + // is usable again; the recovery activity is what the thread view reads + // to replace the useless Retry with the two actions that can work. + return setThreadSessionErrorOnTurnStartFailure({ + threadId: event.payload.threadId, + detail: checkoutMissing.message, + createdAt: event.payload.createdAt, + }).pipe( + Effect.flatMap(() => + noteCheckoutMissing({ + threadId: event.payload.threadId, + payload: { + cwd: checkoutMissing.cwd, + ...(checkoutMissing.branch !== undefined ? { branch: checkoutMissing.branch } : {}), + ...(checkoutMissing.projectCwd !== undefined + ? { projectCwd: checkoutMissing.projectCwd } + : {}), + }, + createdAt: event.payload.createdAt, + }), + ), + ); + } const detail = formatFailureDetail(cause); return setThreadSessionErrorOnTurnStartFailure({ threadId: event.payload.threadId, @@ -2284,6 +2409,51 @@ const make = Effect.gen(function* () { processDomainEventSafely(event), ); + /** + * Fans a confirmed checkout disappearance out to every thread working in it. + * + * One deleted folder usually strands several threads, and each needs its own + * record: the recovery actions (move to the project root, recreate the + * worktree) are per-thread. Threads already reported for this same path are + * skipped by `noteCheckoutMissing`, so a repeated signal is harmless. + */ + const announceMissingCheckoutToThreads = Effect.fnUntraced(function* (observation: { + readonly cwd: string; + }) { + const snapshot = yield* projectionSnapshotQuery.getShellSnapshot().pipe( + Effect.catchCause((cause) => + Effect.logWarning( + "provider command reactor could not resolve threads for missing checkout", + { + cwd: observation.cwd, + cause: Cause.pretty(cause), + }, + ).pipe(Effect.as(null)), + ), + ); + if (snapshot === null) { + return; + } + const createdAt = yield* nowIso; + for (const thread of snapshot.threads) { + const threadCwd = thread.effectiveCwd ?? thread.worktreePath; + if (threadCwd === null || !areFilesystemPathsEqual(threadCwd, observation.cwd)) { + continue; + } + const projectCwd = + snapshot.projects.find((project) => project.id === thread.projectId)?.workspaceRoot ?? null; + yield* noteCheckoutMissing({ + threadId: thread.id, + payload: { + cwd: threadCwd, + branch: thread.branch, + projectCwd, + }, + createdAt, + }); + } + }); + const start: ProviderCommandReactorShape["start"] = Effect.fn("start")(function* () { const processEvent = Effect.fn("processEvent")(function* (event: OrchestrationEvent) { if ( @@ -2310,6 +2480,16 @@ const make = Effect.gen(function* () { yield* Effect.forkScoped( Stream.runForEach(orchestrationEngine.streamDomainEvents, processEvent), ); + + // A checkout deleted between turns is reported the moment the watcher + // confirms it, so the thread shows its way out immediately instead of + // waiting for the user to send a message that is guaranteed to fail. + yield* Effect.forkScoped( + Stream.runForEach( + vcsStatusBroadcaster.observeMissingCheckouts(), + announceMissingCheckoutToThreads, + ), + ); }); return { diff --git a/apps/server/src/orchestration/Layers/ThreadDiffStatBaselineReactor.test.ts b/apps/server/src/orchestration/Layers/ThreadDiffStatBaselineReactor.test.ts index 71b63ce0c..371310794 100644 --- a/apps/server/src/orchestration/Layers/ThreadDiffStatBaselineReactor.test.ts +++ b/apps/server/src/orchestration/Layers/ThreadDiffStatBaselineReactor.test.ts @@ -137,6 +137,7 @@ describe("ThreadDiffStatBaselineReactor", () => { Stream.fromIterable(input.observations).pipe( Stream.ensuring(Deferred.succeed(streamDone, undefined)), ), + observeMissingCheckouts: () => Stream.empty, }; const layer = ThreadDiffStatBaselineReactorLive.pipe( diff --git a/apps/server/src/provider/Layers/ClaudeAdapter.ts b/apps/server/src/provider/Layers/ClaudeAdapter.ts index 8710f0838..09882b053 100644 --- a/apps/server/src/provider/Layers/ClaudeAdapter.ts +++ b/apps/server/src/provider/Layers/ClaudeAdapter.ts @@ -63,7 +63,16 @@ import { getModelSelectionStringOptionValue, getProviderOptionDescriptors, } from "@threadlines/shared/model"; -import { renderThreadContextSeed, withContextSeedPreamble } from "@threadlines/shared/contextSeed"; +import { + MANAGED_WORKTREE_INSTRUCTION, + renderThreadContextSeed, + withContextSeedPreamble, +} from "@threadlines/shared/contextSeed"; +import { + classifySpawnFailure, + isLinkedWorktreeCheckout, + missingWorkingDirectoryDetail, +} from "../../vcs/CheckoutPresence.ts"; import * as Cause from "effect/Cause"; import * as DateTime from "effect/DateTime"; import * as Duration from "effect/Duration"; @@ -5268,6 +5277,34 @@ export const makeClaudeAdapter = Effect.fn("makeClaudeAdapter")(function* ( } }); + /** + * Replaces a process failure's detail when the session's working directory + * turned out to be gone. + * + * The SDK reports a spawn that failed because its `cwd` does not exist as a + * missing Claude binary — the errno is the same, and it only checks its own + * executable. Left alone, a deleted worktree reads as a broken install and + * sends the user to reinstall a CLI that is sitting right where it should be. + * A directory that is confirmed gone outranks whatever the SDK guessed. + */ + const withMissingCheckoutDetail = ( + error: ProviderAdapterProcessError, + cwd: string | undefined, + ): Effect.Effect => + classifySpawnFailure({ cwd, error: error.cause ?? error, errorHidesErrno: true }).pipe( + Effect.provideService(FileSystem.FileSystem, fileSystem), + Effect.map((classification) => + classification === "missing-working-directory" && cwd !== undefined + ? new ProviderAdapterProcessError({ + provider: PROVIDER, + threadId: error.threadId, + detail: missingWorkingDirectoryDetail(cwd), + ...(error.cause !== undefined ? { cause: error.cause } : {}), + }) + : error, + ), + ); + const runSdkStream = ( context: ClaudeSessionContext, ): Effect.Effect => @@ -5276,6 +5313,9 @@ export const makeClaudeAdapter = Effect.fn("makeClaudeAdapter")(function* ( ).pipe( Stream.takeWhile(() => !context.stopped), Stream.runForEach((message) => handleSdkMessage(context, message)), + Effect.catch((error) => + withMissingCheckoutDetail(error, context.session.cwd).pipe(Effect.flatMap(Effect.fail)), + ), ); const handleStreamExit = Effect.fn("handleStreamExit")(function* ( @@ -5921,6 +5961,14 @@ export const makeClaudeAdapter = Effect.fn("makeClaudeAdapter")(function* ( // and with a credential that names the thread, because the tools take no // thread argument and must not. const browserCredential = yield* mcpSessionRegistry.credentialFor(threadId); + // Agents treat `git worktree remove` as ordinary post-merge tidying. Here + // it deletes the session's own working directory, so a session running in + // a Threadlines-managed worktree is told once, up front, not to. + const runsInManagedWorktree = input.cwd + ? yield* isLinkedWorktreeCheckout(input.cwd).pipe( + Effect.provideService(FileSystem.FileSystem, fileSystem), + ) + : false; const queryOptions: ClaudeQueryOptions = { ...(input.cwd ? { cwd: input.cwd } : {}), mcpServers: { @@ -5932,7 +5980,11 @@ export const makeClaudeAdapter = Effect.fn("makeClaudeAdapter")(function* ( }, ...(apiModelId ? { model: apiModelId } : {}), pathToClaudeCodeExecutable: claudeBinaryPath, - systemPrompt: { type: "preset", preset: "claude_code" }, + systemPrompt: { + type: "preset", + preset: "claude_code", + ...(runsInManagedWorktree ? { append: MANAGED_WORKTREE_INSTRUCTION } : {}), + }, settingSources: [...CLAUDE_SETTING_SOURCES], ...(effectiveEffort ? { @@ -6012,7 +6064,11 @@ export const makeClaudeAdapter = Effect.fn("makeClaudeAdapter")(function* ( detail: toMessage(cause, "Failed to start Claude runtime session."), cause, }), - }); + }).pipe( + Effect.catch((error) => + withMissingCheckoutDetail(error, input.cwd).pipe(Effect.flatMap(Effect.fail)), + ), + ); const session: ProviderSession = { threadId, diff --git a/apps/server/src/provider/Layers/CodexAdapter.ts b/apps/server/src/provider/Layers/CodexAdapter.ts index 527ba40c4..29c87bde3 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.ts @@ -2811,6 +2811,7 @@ export const makeCodexAdapter = Effect.fn("makeCodexAdapter")(function* ( const runtime = yield* createRuntime(runtimeInput).pipe( Effect.provideService(Scope.Scope, sessionScope), Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, childProcessSpawner), + Effect.provideService(FileSystem.FileSystem, fileSystem), Effect.mapError( (cause) => new ProviderAdapterProcessError({ diff --git a/apps/server/src/provider/Layers/CodexSessionRuntime.ts b/apps/server/src/provider/Layers/CodexSessionRuntime.ts index 91d44046a..601a0193d 100644 --- a/apps/server/src/provider/Layers/CodexSessionRuntime.ts +++ b/apps/server/src/provider/Layers/CodexSessionRuntime.ts @@ -26,6 +26,8 @@ import { } from "@threadlines/contracts"; import { hideWindowsConsole } from "@threadlines/shared/childProcess"; import { planCliSpawn } from "../../cliSpawn.ts"; +import { isLinkedWorktreeCheckout } from "../../vcs/CheckoutPresence.ts"; +import { MANAGED_WORKTREE_INSTRUCTION } from "@threadlines/shared/contextSeed"; import { normalizeModelSlug } from "@threadlines/shared/model"; import { isProviderAuthErrorMessage } from "@threadlines/shared/providerAuth"; import * as DateTime from "effect/DateTime"; @@ -34,6 +36,7 @@ import * as Duration from "effect/Duration"; import { randomUUIDv4 } from "@threadlines/shared/uuid"; import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; +import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Queue from "effect/Queue"; @@ -525,6 +528,8 @@ function buildCodexCollaborationMode(input: { readonly interactionMode?: ProviderInteractionMode; readonly model?: string; readonly effort?: EffectCodexSchema.V2TurnStartParams__ReasoningEffort; + /** Session runs in a git worktree Threadlines created and must not delete. */ + readonly managedWorktree?: boolean; }): EffectCodexSchema.V2TurnStartParams__CollaborationMode | undefined { if (input.interactionMode === undefined) { return undefined; @@ -542,6 +547,7 @@ function buildCodexCollaborationMode(input: { ? CODEX_PLAN_MODE_DEVELOPER_INSTRUCTIONS : CODEX_DEFAULT_MODE_DEVELOPER_INSTRUCTIONS, CODEX_PREVIEW_PANEL_DEVELOPER_INSTRUCTIONS, + ...(input.managedWorktree ? [MANAGED_WORKTREE_INSTRUCTION] : []), ].join("\n\n"), }, }; @@ -558,6 +564,8 @@ export function buildTurnStartParams(input: { readonly serviceTier?: CodexServiceTier; readonly effort?: EffectCodexSchema.V2TurnStartParams__ReasoningEffort; readonly interactionMode?: ProviderInteractionMode; + /** Session runs in a git worktree Threadlines created and must not delete. */ + readonly managedWorktree?: boolean; }): Effect.Effect< CodexTurnStartParamsWithCollaborationMode, CodexErrors.CodexAppServerProtocolParseError @@ -581,6 +589,7 @@ export function buildTurnStartParams(input: { ...(input.interactionMode ? { interactionMode: input.interactionMode } : {}), ...(input.model ? { model: input.model } : {}), ...(input.effort ? { effort: input.effort } : {}), + ...(input.managedWorktree ? { managedWorktree: true } : {}), }); return decodeCodexTurnStartParams({ @@ -1416,11 +1425,18 @@ export const makeCodexSessionRuntime = ( ): Effect.Effect< CodexSessionRuntimeShape, CodexErrors.CodexAppServerError, - ChildProcessSpawner.ChildProcessSpawner | Scope.Scope + ChildProcessSpawner.ChildProcessSpawner | FileSystem.FileSystem | Scope.Scope > => Effect.gen(function* () { const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; const runtimeScope = yield* Scope.Scope; + const fileSystem = yield* FileSystem.FileSystem; + // Resolved once per session: the checkout cannot change kind underneath a + // running runtime, and every turn asks the same question. + const runsInManagedWorktree = yield* isLinkedWorktreeCheckout(options.cwd).pipe( + Effect.provideService(FileSystem.FileSystem, fileSystem), + Effect.cached, + ); const events = yield* Queue.unbounded(); const pendingApprovalsRef = yield* Ref.make(new Map()); const approvalCorrelationsRef = yield* Ref.make(new Map()); @@ -2135,6 +2151,7 @@ export const makeCodexSessionRuntime = ( ...(input.serviceTier ? { serviceTier: input.serviceTier } : {}), ...(input.effort ? { effort: input.effort } : {}), ...(input.interactionMode ? { interactionMode: input.interactionMode } : {}), + ...((yield* runsInManagedWorktree) ? { managedWorktree: true } : {}), }); const rawResponse = yield* withCodexRequestTimeout( "start a Codex turn", diff --git a/apps/server/src/vcs/CheckoutPresence.test.ts b/apps/server/src/vcs/CheckoutPresence.test.ts new file mode 100644 index 000000000..b4f93b379 --- /dev/null +++ b/apps/server/src/vcs/CheckoutPresence.test.ts @@ -0,0 +1,165 @@ +// @effect-diagnostics nodeBuiltinImport:off +import { assert, describe, it } from "@effect/vitest"; +import * as NodeFS from "node:fs/promises"; +import * as NodeOS from "node:os"; +import * as NodePath from "node:path"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import * as Effect from "effect/Effect"; + +import { + checkoutPresence, + classifySpawnFailure, + isLinkedWorktreeCheckout, + systemErrorCode, +} from "./CheckoutPresence.ts"; + +const makeTempDir = Effect.acquireRelease( + Effect.promise(() => NodeFS.mkdtemp(NodePath.join(NodeOS.tmpdir(), "checkout-presence-"))), + (dir) => Effect.promise(() => NodeFS.rm(dir, { recursive: true, force: true })), +); + +describe("checkoutPresence", () => { + it.effect("reports an existing directory as present", () => + Effect.scoped( + Effect.gen(function* () { + const dir = yield* makeTempDir; + assert.strictEqual(yield* checkoutPresence(dir), "present"); + }), + ).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("reports a deleted directory as missing", () => + Effect.scoped( + Effect.gen(function* () { + const dir = yield* makeTempDir; + const gone = NodePath.join(dir, "worktree"); + assert.strictEqual(yield* checkoutPresence(gone), "missing"); + }), + ).pipe(Effect.provide(NodeServices.layer)), + ); + + // A file where a checkout should be cannot be worked in either, and the + // recovery path is the same, so it is not a separate state. + it.effect("treats a non-directory as missing", () => + Effect.scoped( + Effect.gen(function* () { + const dir = yield* makeTempDir; + const filePath = NodePath.join(dir, "not-a-dir"); + yield* Effect.promise(() => NodeFS.writeFile(filePath, "x")); + assert.strictEqual(yield* checkoutPresence(filePath), "missing"); + }), + ).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("answers unknown for an empty path rather than claiming it is missing", () => + Effect.gen(function* () { + assert.strictEqual(yield* checkoutPresence(" "), "unknown"); + }).pipe(Effect.provide(NodeServices.layer)), + ); +}); + +describe("systemErrorCode", () => { + it("finds the errno on a raw Node error", () => { + assert.strictEqual( + systemErrorCode(Object.assign(new Error("nope"), { code: "ENOENT" })), + "ENOENT", + ); + }); + + it("follows the cause chain when the errno is wrapped", () => { + const wrapped = new Error("outer", { + cause: Object.assign(new Error("inner"), { code: "ENOENT" }), + }); + assert.strictEqual(systemErrorCode(wrapped), "ENOENT"); + }); + + it("returns null when nothing looks like an errno", () => { + assert.strictEqual(systemErrorCode(new Error("just words")), null); + }); +}); + +describe("classifySpawnFailure", () => { + // The incident this exists for: deleting a worktree made every turn report + // "Claude Code native binary not found at claude". The binary was fine; the + // spawn's cwd was gone, and both fail with ENOENT. + it.effect("blames the working directory when the cwd is gone", () => + Effect.scoped( + Effect.gen(function* () { + const dir = yield* makeTempDir; + const gone = NodePath.join(dir, "worktree"); + const classification = yield* classifySpawnFailure({ + cwd: gone, + error: Object.assign(new Error("spawn claude ENOENT"), { code: "ENOENT" }), + }); + assert.strictEqual(classification, "missing-working-directory"); + }), + ).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("leaves a genuine missing-binary failure alone when the cwd is fine", () => + Effect.scoped( + Effect.gen(function* () { + const dir = yield* makeTempDir; + const classification = yield* classifySpawnFailure({ + cwd: dir, + error: Object.assign(new Error("spawn claude ENOENT"), { code: "ENOENT" }), + }); + assert.strictEqual(classification, "other"); + }), + ).pipe(Effect.provide(NodeServices.layer)), + ); + + // Provider SDKs rewrap the spawn error and drop its `code`, so the errno + // pre-filter has to be skippable or the fix never fires where it matters. + it.effect("still blames the working directory when the SDK hid the errno", () => + Effect.scoped( + Effect.gen(function* () { + const dir = yield* makeTempDir; + const gone = NodePath.join(dir, "worktree"); + const classification = yield* classifySpawnFailure({ + cwd: gone, + error: new Error("Claude Code native binary not found at claude"), + errorHidesErrno: true, + }); + assert.strictEqual(classification, "missing-working-directory"); + }), + ).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("does not blame the working directory for an unrelated failure", () => + Effect.scoped( + Effect.gen(function* () { + const dir = yield* makeTempDir; + const classification = yield* classifySpawnFailure({ + cwd: dir, + error: new Error("EACCES: permission denied"), + }); + assert.strictEqual(classification, "other"); + }), + ).pipe(Effect.provide(NodeServices.layer)), + ); +}); + +describe("isLinkedWorktreeCheckout", () => { + it.effect("recognizes a linked worktree by its .git pointer file", () => + Effect.scoped( + Effect.gen(function* () { + const dir = yield* makeTempDir; + yield* Effect.promise(() => + NodeFS.writeFile(NodePath.join(dir, ".git"), "gitdir: /repo/.git/worktrees/feature\n"), + ); + assert.isTrue(yield* isLinkedWorktreeCheckout(dir)); + }), + ).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("does not treat a primary checkout as a managed worktree", () => + Effect.scoped( + Effect.gen(function* () { + const dir = yield* makeTempDir; + yield* Effect.promise(() => NodeFS.mkdir(NodePath.join(dir, ".git"))); + assert.isFalse(yield* isLinkedWorktreeCheckout(dir)); + }), + ).pipe(Effect.provide(NodeServices.layer)), + ); +}); diff --git a/apps/server/src/vcs/CheckoutPresence.ts b/apps/server/src/vcs/CheckoutPresence.ts new file mode 100644 index 000000000..1d3f61cc4 --- /dev/null +++ b/apps/server/src/vcs/CheckoutPresence.ts @@ -0,0 +1,167 @@ +/** + * The single answer to "is this checkout still on disk?". + * + * A thread's checkout can vanish underneath it: the agent removes the worktree + * after merging its own PR, or the user deletes the folder in a terminal. Three + * separate places have to react to that — the turn pre-flight that refuses to + * start a session in a directory that is gone, the VCS status path that must + * say "the folder is missing" instead of "this is not a git repository", and + * the status watcher that surfaces an out-of-band deletion before the next turn + * crashes on it. They all ask here so they cannot disagree. + * + * The third state matters as much as the other two. A stat that fails for any + * reason other than "not found" (a permission problem, an unresponsive network + * mount, an fs flake) is reported as `unknown`, never as `missing`: callers + * treat `unknown` exactly like `present` and let normal operation proceed. Only + * a confirmed absence opens the recovery paths. + * + * @module CheckoutPresence + */ +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; + +/** + * - `present`: the path exists and is a directory. + * - `missing`: the path is confirmed absent, or exists but is not a directory. + * - `unknown`: the check itself failed; the caller must assume the checkout is fine. + */ +export type CheckoutPresence = "present" | "missing" | "unknown"; + +/** + * Stat `cwd` and classify it. Never fails: a check error is `unknown`, which + * every caller treats as "carry on". + */ +export const checkoutPresence = ( + cwd: string, +): Effect.Effect => + Effect.gen(function* () { + const trimmed = cwd.trim(); + if (trimmed.length === 0) { + return "unknown" as const; + } + const fs = yield* FileSystem.FileSystem; + return yield* fs.stat(trimmed).pipe( + Effect.map((info): CheckoutPresence => (info.type === "Directory" ? "present" : "missing")), + Effect.catch((error): Effect.Effect => { + // Effect's platform errors carry the underlying errno in `describe` + // rather than as a discriminated reason on every implementation, so + // classify off the raw code and default to `unknown`. + const code = systemErrorCode(error); + return Effect.succeed(code === "ENOENT" || code === "ENOTDIR" ? "missing" : "unknown"); + }), + ); + }); + +/** True only for a checkout confirmed to be gone. */ +export const isCheckoutMissing = ( + cwd: string, +): Effect.Effect => + checkoutPresence(cwd).pipe(Effect.map((presence) => presence === "missing")); + +/** + * Dig the OS errno out of whatever the platform layer wrapped it in. Effect's + * `SystemError` exposes it on `.cause`/`.description` depending on the backing + * call, and a raw Node error carries it directly. + */ +export function systemErrorCode(error: unknown): string | null { + const seen = new Set(); + let current: unknown = error; + while (current !== null && current !== undefined && !seen.has(current)) { + seen.add(current); + if (typeof current === "object") { + const code = (current as { readonly code?: unknown }).code; + if (typeof code === "string" && code.length > 0) { + return code; + } + const reason = (current as { readonly reason?: unknown }).reason; + if (reason === "NotFound") { + return "ENOENT"; + } + const description = (current as { readonly description?: unknown }).description; + if (typeof description === "string") { + const matched = /\b(E[A-Z]{2,})\b/u.exec(description); + if (matched?.[1]) { + return matched[1]; + } + } + current = (current as { readonly cause?: unknown }).cause; + continue; + } + if (typeof current === "string") { + const matched = /\b(E[A-Z]{2,})\b/u.exec(current); + return matched?.[1] ?? null; + } + return null; + } + return null; +} + +/** + * Classify a failed process spawn. + * + * A spawn whose `cwd` does not exist fails with the same `ENOENT` a missing + * executable produces. The provider SDKs see that errno, check whether their + * own binary is where they expect, and report "Claude Code native binary not + * found at claude" — sending the user to reinstall a CLI that was never the + * problem. That is how a deleted worktree presented itself during the incident + * this guards against. + * + * A confirmed-missing working directory is decisive on its own, without + * requiring the errno to have survived: no process can be spawned in a + * directory that is not there, so it is always the better explanation than + * whatever the SDK guessed. The errno is only consulted to avoid a stat on + * failures that plainly have nothing to do with paths. + */ +export const classifySpawnFailure = (input: { + readonly cwd: string | undefined; + readonly error: unknown; + /** + * Skip the errno pre-filter and decide purely on whether the directory is + * there. Used for provider SDK failures, which wrap the original error and + * drop its `code`. + */ + readonly errorHidesErrno?: boolean; +}): Effect.Effect<"missing-working-directory" | "other", never, FileSystem.FileSystem> => + Effect.gen(function* () { + if (input.cwd === undefined) { + return "other" as const; + } + if (input.errorHidesErrno !== true && systemErrorCode(input.error) !== "ENOENT") { + return "other" as const; + } + const presence = yield* checkoutPresence(input.cwd); + return presence === "missing" ? ("missing-working-directory" as const) : ("other" as const); + }); + +/** + * True when `cwd` is a linked git worktree rather than a primary checkout. + * + * Git marks a linked worktree by making `.git` a *file* holding a `gitdir:` + * pointer, where a normal clone has a `.git` directory. Threads only ever get a + * worktree checkout from Threadlines' own creation flow, so a linked worktree + * here is by definition one the app manages. + * + * Answers false on any error: telling an agent its directory is managed when we + * are not sure is worse than staying quiet. + */ +export const isLinkedWorktreeCheckout = ( + cwd: string, +): Effect.Effect => + Effect.gen(function* () { + const trimmed = cwd.trim(); + if (trimmed.length === 0) { + return false; + } + const fs = yield* FileSystem.FileSystem; + const separator = trimmed.includes("\\") && !trimmed.includes("/") ? "\\" : "/"; + const dotGit = `${trimmed.replace(/[/\\]+$/u, "")}${separator}.git`; + return yield* fs.stat(dotGit).pipe( + Effect.map((info) => info.type === "File"), + Effect.orElseSucceed(() => false), + ); + }); + +/** User-facing explanation for a spawn that failed because its folder is gone. */ +export function missingWorkingDirectoryDetail(cwd: string): string { + return `This thread's folder no longer exists: ${cwd}`; +} diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 1f4f70de8..5eecfc1e4 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -830,6 +830,39 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { assert.equal(yield* fileSystem.exists(worktreePath), false); }), ); + + // A worktree whose folder was deleted out-of-band leaves its registration + // behind, and `git worktree add` refuses to reuse the path. Recreating at + // the same path is the recovery flow for a thread whose checkout was + // deleted, so the driver clears the dead registration and retries. + it.effect("recreates a worktree over a stale registration whose folder was deleted", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const pathService = yield* Path.Path; + const worktreePath = pathService.join(yield* makeTmpDir("git-worktrees-"), "doomed"); + const driver = yield* GitVcsDriver.GitVcsDriver; + + yield* driver.createWorktree({ + cwd, + path: worktreePath, + refName: initialBranch, + newRefName: "feature/doomed", + }); + // Delete the folder directly, the way an agent or a terminal would — + // not through removeWorktree — so the registration stays behind. + const fileSystem = yield* FileSystem.FileSystem; + yield* fileSystem.remove(worktreePath, { recursive: true }); + + const recreated = yield* driver.createWorktree({ + cwd, + path: worktreePath, + refName: "feature/doomed", + }); + assert.equal(recreated.worktree.path, worktreePath); + assert.equal(yield* git(worktreePath, ["branch", "--show-current"]), "feature/doomed"); + }), + ); }); describe("commit context", () => { diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 2357ce439..ccbbe67dc 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -4227,7 +4227,26 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* executeGit("GitVcsDriver.createWorktree", input.cwd, args, { fallbackErrorMessage: "git worktree add failed", - }); + }).pipe( + Effect.catchIf( + // A worktree whose folder was deleted out-of-band leaves its + // registration behind, and git refuses to reuse the path ("missing but + // already registered worktree"). Recreating at the same path is exactly + // the recovery this serves, so clear the dead registrations — prune + // only ever removes entries whose directories are gone — and retry. + (error) => /missing but already registered/iu.test(error.detail ?? ""), + () => + executeGit("GitVcsDriver.createWorktree.prune", input.cwd, ["worktree", "prune"], { + fallbackErrorMessage: "git worktree prune failed", + }).pipe( + Effect.andThen( + executeGit("GitVcsDriver.createWorktree", input.cwd, args, { + fallbackErrorMessage: "git worktree add failed", + }), + ), + ), + ), + ); const expectedRef = `refs/heads/${targetBranch}`; const readCreatedWorktreeHead = Effect.fn("GitVcsDriver.createWorktree.readHead")(function* () { diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts index 876c75655..4adc14a08 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts @@ -565,6 +565,158 @@ describe("VcsStatusBroadcaster", () => { }).pipe(Effect.provide(makeTestLayer(state))); }); + // Deleting a checkout out of band (an agent's own `git worktree remove`, or + // the user in a terminal) used to be discovered only when the next turn + // crashed inside the provider SDK. The watcher reports it directly. + it.live("reports a watched checkout that disappears", () => { + const state = { + currentLocalStatus: baseLocalStatus, + currentRemoteStatus: baseRemoteStatus, + localStatusCalls: 0, + remoteStatusCalls: 0, + localInvalidationCalls: 0, + remoteInvalidationCalls: 0, + }; + + return Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const parent = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "t3-vcs-checkout-gone-", + }); + const resolvedParent = yield* fileSystem.realPath(parent); + const repoDir = path.join(parent, "worktree"); + yield* fileSystem.makeDirectory(path.join(repoDir, ".git"), { recursive: true }); + yield* fileSystem.writeFileString( + path.join(repoDir, ".git", "HEAD"), + "ref: refs/heads/main\n", + ); + + const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; + const missing = yield* Deferred.make<{ readonly cwd: string }>(); + yield* Stream.runForEach(broadcaster.observeMissingCheckouts(), (observation) => + Deferred.succeed(missing, observation).pipe(Effect.ignore), + ).pipe(Effect.forkScoped); + // Subscribing is what starts the per-cwd monitor that watches presence. + yield* Stream.runForEach(broadcaster.streamStatus({ cwd: repoDir }), () => Effect.void).pipe( + Effect.forkScoped, + ); + yield* Effect.sleep(Duration.millis(200)); + + // Nothing is reported while the checkout is still there. + assert.isTrue(Option.isNone(yield* Deferred.poll(missing))); + + yield* Effect.promise(() => NodeFS.rm(repoDir, { recursive: true, force: true })); + + const observed = yield* Deferred.await(missing).pipe(Effect.timeout(Duration.seconds(20))); + // Monitors are keyed by the resolved real path (temp dirs are symlinked + // on macOS), so compare against that rather than the path as handed in. + assert.strictEqual(observed.cwd, path.join(resolvedParent, "worktree")); + }).pipe(Effect.provide(makeTestLayer(state))); + }); + + // A subscription opened while the checkout is already gone must share its + // cache/stream key with the statuses published once the folder is recreated. + // `realPath` alone can't provide that: it resolves /tmp/x to /private/tmp/x + // only while the directory exists, so a missing-at-subscribe path would key + // on its raw form and never hear another update after recreation. + it.live("keys a checkout that is missing at subscribe time like one that exists", () => { + const state = { + currentLocalStatus: baseLocalStatus, + currentRemoteStatus: baseRemoteStatus, + localStatusCalls: 0, + remoteStatusCalls: 0, + localInvalidationCalls: 0, + remoteInvalidationCalls: 0, + }; + + return Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + // The temp parent is symlinked on macOS, which is exactly the class of + // path this guards: the canonical form differs from the requested one. + const parent = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "t3-vcs-checkout-key-", + }); + const resolvedParent = yield* fileSystem.realPath(parent); + const repoDir = path.join(parent, "worktree"); + + const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; + const missing = yield* Deferred.make<{ readonly cwd: string }>(); + yield* Stream.runForEach(broadcaster.observeMissingCheckouts(), (observation) => + Deferred.succeed(missing, observation).pipe(Effect.ignore), + ).pipe(Effect.forkScoped); + // Subscribe while the directory does not exist yet. + yield* Stream.runForEach(broadcaster.streamStatus({ cwd: repoDir }), () => Effect.void).pipe( + Effect.forkScoped, + ); + yield* Effect.sleep(Duration.millis(200)); + + // Create the checkout, let a presence poll see it, then delete it: the + // announced key must be the canonical path, proving the monitor key did + // not stick to the raw form from the missing-at-subscribe moment. + yield* fileSystem.makeDirectory(path.join(repoDir, ".git"), { recursive: true }); + yield* fileSystem.writeFileString( + path.join(repoDir, ".git", "HEAD"), + "ref: refs/heads/main\n", + ); + yield* Effect.sleep(Duration.seconds(3)); + yield* Effect.promise(() => NodeFS.rm(repoDir, { recursive: true, force: true })); + + const observed = yield* Deferred.await(missing).pipe(Effect.timeout(Duration.seconds(20))); + assert.strictEqual(observed.cwd, path.join(resolvedParent, "worktree")); + }).pipe(Effect.provide(makeTestLayer(state))); + }); + + // Git renames paths in place during ordinary operations. A directory that + // reads as missing for an instant must not be announced as deleted. + it.live("does not report a checkout that reappears inside the confirmation window", () => { + const state = { + currentLocalStatus: baseLocalStatus, + currentRemoteStatus: baseRemoteStatus, + localStatusCalls: 0, + remoteStatusCalls: 0, + localInvalidationCalls: 0, + remoteInvalidationCalls: 0, + }; + + return Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const parent = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "t3-vcs-checkout-blip-", + }); + const repoDir = path.join(parent, "worktree"); + const makeCheckout = Effect.gen(function* () { + yield* fileSystem.makeDirectory(path.join(repoDir, ".git"), { recursive: true }); + yield* fileSystem.writeFileString( + path.join(repoDir, ".git", "HEAD"), + "ref: refs/heads/main\n", + ); + }); + yield* makeCheckout; + + const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; + const missing = yield* Deferred.make<{ readonly cwd: string }>(); + yield* Stream.runForEach(broadcaster.observeMissingCheckouts(), (observation) => + Deferred.succeed(missing, observation).pipe(Effect.ignore), + ).pipe(Effect.forkScoped); + yield* Stream.runForEach(broadcaster.streamStatus({ cwd: repoDir }), () => Effect.void).pipe( + Effect.forkScoped, + ); + yield* Effect.sleep(Duration.millis(200)); + + // Gone and back well inside the confirmation window. + yield* Effect.promise(() => NodeFS.rm(repoDir, { recursive: true, force: true })); + yield* Effect.sleep(Duration.millis(150)); + yield* makeCheckout; + + // Past a full poll plus confirmation window with nothing announced. + yield* Effect.sleep(Duration.seconds(6)); + assert.isTrue(Option.isNone(yield* Deferred.poll(missing))); + }).pipe(Effect.provide(makeTestLayer(state))); + }); + it.effect("stops the remote poller after the last stream subscriber disconnects", () => { const state = { currentLocalStatus: baseLocalStatus, diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.ts b/apps/server/src/vcs/VcsStatusBroadcaster.ts index 19fc53480..57f55468e 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.ts @@ -24,6 +24,7 @@ import type { import { mergeGitStatusParts } from "@threadlines/shared/git"; import * as GitWorkflowService from "../git/GitWorkflowService.ts"; +import { checkoutPresence } from "./CheckoutPresence.ts"; const DEFAULT_VCS_STATUS_REFRESH_INTERVAL = Duration.minutes(2); const VCS_STATUS_REFRESH_FAILURE_BASE_DELAY = Duration.minutes(2); @@ -41,6 +42,14 @@ const GIT_DIR_RESOLVE_RETRY_INTERVAL = Duration.seconds(30); // watcher cannot catch them). const SNAPSHOT_LOCAL_REVALIDATE_AGE = Duration.seconds(1); const SNAPSHOT_REMOTE_REVALIDATE_AGE = Duration.seconds(30); +// How often a watched checkout is confirmed to still be on disk. A single stat +// per subscribed cwd, so the cost is irrelevant next to the git commands the +// same monitor already runs. +const CHECKOUT_PRESENCE_POLL_INTERVAL = Duration.seconds(2); +// Git rewrites paths in place during ordinary operations, so a directory that +// reads as missing once is not news. It has to still be missing after this +// window before anyone is told about it. +const CHECKOUT_MISSING_CONFIRMATION_WINDOW = Duration.seconds(2); interface VcsStatusChange { readonly cwd: string; @@ -61,6 +70,18 @@ export interface VcsLocalStatusObservation { readonly local: VcsStatusLocalResult; } +/** + * A watched checkout that is confirmed gone from disk. + * + * Published once per disappearance, not once per check, so consumers can react + * directly (a thread whose folder vanished needs to say so now, not when its + * next turn crashes) without debouncing again themselves. + */ +export interface VcsCheckoutMissingObservation { + /** Path that was being watched, as the subscriber asked for it. */ + readonly cwd: string; +} + interface CachedValue { readonly fingerprint: string; readonly updatedAtMs: number; @@ -127,6 +148,12 @@ export interface VcsStatusBroadcasterShape { * nothing is subscribed are dropped. */ readonly observeLocalStatus: () => Stream.Stream; + /** + * Checkouts that disappeared while being watched. Hot, like + * {@link observeLocalStatus}: a disappearance with no subscriber is dropped, + * and the next turn's pre-flight catches it instead. + */ + readonly observeMissingCheckouts: () => Stream.Stream; } export class VcsStatusBroadcaster extends Context.Service< @@ -138,11 +165,39 @@ function fingerprintStatusPart(status: unknown): string { return JSON.stringify(status); } +/** + * Canonical cache/stream key for a checkout path. + * + * `realPath` alone is unstable across existence: on macOS `/tmp/x` resolves to + * `/private/tmp/x` while the directory exists but stays `/tmp/x` once it is + * deleted. A subscription opened while the checkout was missing would then key + * differently from the statuses published after the folder is recreated — and + * never hear them. When the leaf is gone, canonicalize the nearest existing + * ancestor and reattach the missing remainder so the key survives + * delete/recreate cycles. + */ const normalizeCwd = (cwd: string) => - Effect.service(FileSystem.FileSystem).pipe( - Effect.flatMap((fs) => fs.realPath(cwd)), - Effect.orElseSucceed(() => cwd), - ); + Effect.gen(function* () { + const fs = yield* Effect.service(FileSystem.FileSystem); + const resolved = yield* fs.realPath(cwd).pipe(Effect.orElseSucceed(() => null)); + if (resolved !== null) { + return resolved; + } + let prefix = cwd.replace(/[/\\]+$/u, ""); + let suffix = ""; + while (true) { + const cut = Math.max(prefix.lastIndexOf("/"), prefix.lastIndexOf("\\")); + if (cut <= 0) { + return cwd; + } + suffix = prefix.slice(cut) + suffix; + prefix = prefix.slice(0, cut); + const resolvedPrefix = yield* fs.realPath(prefix).pipe(Effect.orElseSucceed(() => null)); + if (resolvedPrefix !== null) { + return resolvedPrefix + suffix; + } + } + }); export const layer = Layer.effect( VcsStatusBroadcaster, @@ -158,6 +213,10 @@ export const layer = Layer.effect( PubSub.unbounded(), (pubsub) => PubSub.shutdown(pubsub), ); + const missingCheckoutsPubSub = yield* Effect.acquireRelease( + PubSub.unbounded(), + (pubsub) => PubSub.shutdown(pubsub), + ); const broadcasterScope = yield* Effect.acquireRelease(Scope.make(), (scope) => Scope.close(scope, Exit.void), ); @@ -496,14 +555,67 @@ export const layer = Layer.effect( }); }; + /** + * Notices when a watched checkout is deleted out from under Threadlines. + * + * The git-dir watcher cannot carry this: `fs.watch` handles die with the + * directory they watch, and the deletion of a linked worktree touches the + * shared git dir in ways that are indistinguishable from ordinary activity. + * A stat every couple of seconds is both cheaper to reason about and the + * only check that still works once the directory is gone. + * + * A single missing reading is never enough: git renames paths in place + * during normal operations, so the path has to still be missing after the + * confirmation window. One announcement per disappearance — the flag resets + * only when the directory comes back, so recreating the worktree re-arms it. + */ + const makeCheckoutPresenceLoop = (cwd: string): Effect.Effect => + Effect.gen(function* () { + let announced = false; + while (true) { + const presence = yield* checkoutPresence(cwd).pipe(withFileSystem); + if (presence !== "missing") { + if (announced && presence === "present") { + announced = false; + // The folder came back — recreated by the app or by hand. Push a + // fresh status so the recovery surfaces clear now rather than at + // the next scheduled refresh. + yield* refreshLocalStatus(cwd).pipe(Effect.ignoreCause({ log: true })); + } + yield* Effect.sleep(CHECKOUT_PRESENCE_POLL_INTERVAL); + continue; + } + if (announced) { + yield* Effect.sleep(CHECKOUT_PRESENCE_POLL_INTERVAL); + continue; + } + yield* Effect.sleep(CHECKOUT_MISSING_CONFIRMATION_WINDOW); + const confirmed = yield* checkoutPresence(cwd).pipe(withFileSystem); + if (confirmed === "missing") { + announced = true; + yield* Effect.logWarning("VCS checkout disappeared", { cwd }); + yield* PubSub.publish(missingCheckoutsPubSub, { cwd }); + // Push the absence into the status feed too, so every open source + // control surface stops claiming the folder just isn't a repo. + yield* refreshLocalStatus(cwd).pipe(Effect.ignoreCause({ log: true })); + } + yield* Effect.sleep(CHECKOUT_PRESENCE_POLL_INTERVAL); + } + }); + // One background monitor per subscribed cwd: the periodic remote-status - // poller plus the git dir watcher that reacts to local git activity. + // poller, the git dir watcher that reacts to local git activity, and the + // presence check that catches the checkout being deleted entirely. const makeCwdMonitor = ( cwd: string, automaticRemoteRefreshInterval: Effect.Effect, ): Effect.Effect => Effect.all( - [makeRemoteRefreshLoop(cwd, automaticRemoteRefreshInterval), makeGitDirWatchLoop(cwd)], + [ + makeRemoteRefreshLoop(cwd, automaticRemoteRefreshInterval), + makeGitDirWatchLoop(cwd), + makeCheckoutPresenceLoop(cwd), + ], { concurrency: "unbounded" }, ).pipe(Effect.asVoid); @@ -628,12 +740,18 @@ export const layer = Layer.effect( PubSub.subscribe(localObservationsPubSub).pipe(Effect.map(Stream.fromSubscription)), ); + const observeMissingCheckouts: VcsStatusBroadcasterShape["observeMissingCheckouts"] = () => + Stream.unwrap( + PubSub.subscribe(missingCheckoutsPubSub).pipe(Effect.map(Stream.fromSubscription)), + ); + return VcsStatusBroadcaster.of({ getStatus, refreshLocalStatus, refreshStatus, streamStatus, observeLocalStatus, + observeMissingCheckouts, }); }), ); diff --git a/apps/server/src/vcs/WorktreeRemovalGuard.test.ts b/apps/server/src/vcs/WorktreeRemovalGuard.test.ts new file mode 100644 index 000000000..dcc953073 --- /dev/null +++ b/apps/server/src/vcs/WorktreeRemovalGuard.test.ts @@ -0,0 +1,130 @@ +import { assert, describe, it } from "@effect/vitest"; +import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; +import * as Cause from "effect/Cause"; + +import { + ensureWorktreeRemovable, + findWorktreeBlockingThreads, + type WorktreeUsageThread, +} from "./WorktreeRemovalGuard.ts"; + +const WORKTREE = "/repo/.worktrees/feature"; +const OTHER = "/repo/.worktrees/other"; + +const thread = (overrides: Partial = {}): WorktreeUsageThread => ({ + id: "thread-a", + title: "Feature work", + worktreePath: null, + ...overrides, +}); + +describe("findWorktreeBlockingThreads", () => { + // The incident: an agent merged its PR and deleted the worktree it was + // running in, leaving its own thread with nowhere to run. + it("blocks a path a live session is running in", () => { + const blocking = findWorktreeBlockingThreads({ + worktreePath: WORKTREE, + threads: [ + thread({ worktreePath: WORKTREE, session: { status: "running", checkoutCwd: WORKTREE } }), + ], + }); + assert.deepStrictEqual(blocking, [ + { threadId: "thread-a", title: "Feature work", hasLiveSession: true }, + ]); + }); + + it("blocks a path a thread is bound to even with no session running", () => { + const blocking = findWorktreeBlockingThreads({ + worktreePath: WORKTREE, + threads: [thread({ worktreePath: WORKTREE, session: null })], + }); + assert.strictEqual(blocking.length, 1); + assert.isFalse(blocking[0]?.hasLiveSession); + }); + + // Deleting a thread stops its session and clears it from the projection + // first, so the app's own cleanup of a now-orphaned worktree still works. + it("allows removal once no thread is bound and no session runs there", () => { + assert.deepStrictEqual( + findWorktreeBlockingThreads({ + worktreePath: WORKTREE, + threads: [ + thread({ worktreePath: OTHER, session: { status: "ready", checkoutCwd: OTHER } }), + ], + }), + [], + ); + }); + + it("ignores a stopped session that merely remembers the path", () => { + assert.deepStrictEqual( + findWorktreeBlockingThreads({ + worktreePath: WORKTREE, + threads: [ + thread({ worktreePath: null, session: { status: "stopped", checkoutCwd: WORKTREE } }), + ], + }), + [], + ); + }); + + // The agent can move itself into a worktree mid-session without the thread's + // configured checkout ever pointing there. + it("blocks a path a session wandered into", () => { + const blocking = findWorktreeBlockingThreads({ + worktreePath: WORKTREE, + threads: [ + thread({ worktreePath: null, effectiveCwd: WORKTREE, session: { status: "running" } }), + ], + }); + assert.strictEqual(blocking.length, 1); + assert.isTrue(blocking[0]?.hasLiveSession); + }); + + it("matches paths across trailing separators and case", () => { + const blocking = findWorktreeBlockingThreads({ + worktreePath: WORKTREE, + threads: [thread({ worktreePath: `${WORKTREE}/` })], + }); + assert.strictEqual(blocking.length, 1); + }); +}); + +describe("ensureWorktreeRemovable", () => { + it.effect("fails with the blocking threads named", () => + Effect.gen(function* () { + const exit = yield* ensureWorktreeRemovable({ + worktreePath: WORKTREE, + readThreads: Effect.succeed([ + thread({ worktreePath: WORKTREE, session: { status: "running", checkoutCwd: WORKTREE } }), + ]), + }).pipe(Effect.exit); + + assert.isTrue(Exit.isFailure(exit)); + const error = Exit.isFailure(exit) ? Cause.squash(exit.cause) : null; + assert.strictEqual((error as { _tag?: string })?._tag, "VcsWorktreeInUseError"); + assert.include(String((error as { message?: string })?.message), "Feature work"); + }), + ); + + it.effect("succeeds when nothing uses the path", () => + Effect.gen(function* () { + yield* ensureWorktreeRemovable({ + worktreePath: WORKTREE, + readThreads: Effect.succeed([thread({ worktreePath: OTHER })]), + }); + }), + ); + + // The guard protects against a common mistake; it must not become a reason + // that deleting a folder stops working when an unrelated query fails. + it.effect("allows removal when the thread list cannot be read", () => + Effect.gen(function* () { + yield* ensureWorktreeRemovable({ + worktreePath: WORKTREE, + readThreads: Effect.fail(new Error("projection unavailable")), + }); + }), + ); +}); diff --git a/apps/server/src/vcs/WorktreeRemovalGuard.ts b/apps/server/src/vcs/WorktreeRemovalGuard.ts new file mode 100644 index 000000000..ef6d5cf2c --- /dev/null +++ b/apps/server/src/vcs/WorktreeRemovalGuard.ts @@ -0,0 +1,122 @@ +/** + * Refuses to delete a worktree that something is still working in. + * + * Removing the folder out from under a live session is precisely what bricks a + * thread: its next turn fails inside the provider SDK with an error that blames + * the CLI, and the thread has no way back. The app therefore declines its own + * removal requests rather than performing them, and names the threads that are + * in the way so the user can stop or move them first. + * + * There is deliberately no force override. Every blocking condition already has + * a resolution in the existing UI (stop the session, switch the thread's + * checkout, delete the thread), so an override would only exist to let someone + * skip the step that keeps the thread usable. + * + * @module WorktreeRemovalGuard + */ +import * as Effect from "effect/Effect"; +import { VcsWorktreeInUseError } from "@threadlines/contracts"; + +import { normalizeWorkspacePath } from "../checkpointing/Utils.ts"; + +/** The subset of a thread shell this guard reads. */ +export interface WorktreeUsageThread { + readonly id: string; + readonly title?: string | null | undefined; + readonly worktreePath: string | null; + readonly effectiveCwd?: string | null | undefined; + readonly session?: + | { + readonly status: string; + readonly checkoutCwd?: string | null | undefined; + } + | null + | undefined; +} + +export interface WorktreeBlockingThread { + readonly threadId: string; + readonly title: string | null; + readonly hasLiveSession: boolean; +} + +const samePath = (left: string | null | undefined, right: string): boolean => + typeof left === "string" && + left.length > 0 && + normalizeWorkspacePath(left) === normalizeWorkspacePath(right); + +/** + * A session counts as live unless it has been stopped. `error` and + * `interrupted` sessions still own a runtime that can be resumed in place, so + * they block too; only an explicitly stopped session releases the folder. + */ +const hasLiveSessionIn = (thread: WorktreeUsageThread, worktreePath: string): boolean => { + const session = thread.session; + if (!session || session.status === "stopped") { + return false; + } + return ( + samePath(session.checkoutCwd, worktreePath) || + samePath(thread.effectiveCwd, worktreePath) || + samePath(thread.worktreePath, worktreePath) + ); +}; + +/** + * Threads that still use `worktreePath`, either as their configured checkout or + * as the directory a live session is actually running in. Pure so the policy is + * testable without a database. + */ +export function findWorktreeBlockingThreads(input: { + readonly worktreePath: string; + readonly threads: ReadonlyArray; +}): ReadonlyArray { + const blocking: WorktreeBlockingThread[] = []; + for (const thread of input.threads) { + const live = hasLiveSessionIn(thread, input.worktreePath); + const bound = samePath(thread.worktreePath, input.worktreePath); + if (!live && !bound) { + continue; + } + blocking.push({ + threadId: thread.id, + title: thread.title?.trim() || null, + hasLiveSession: live, + }); + } + return blocking; +} + +/** + * Fails with {@link VcsWorktreeInUseError} when the path is still in use. + * + * Reading the thread list is best-effort by design: if the projection cannot be + * queried we let the removal through rather than blocking a legitimate cleanup + * on an unrelated failure. The guard exists to catch the common case, not to be + * an availability dependency of deleting a folder. + */ +export const ensureWorktreeRemovable = (input: { + readonly worktreePath: string; + readonly readThreads: Effect.Effect, E>; +}): Effect.Effect => + Effect.gen(function* () { + const threads = yield* input.readThreads.pipe( + Effect.catchCause((cause) => + Effect.logWarning("worktree removal guard could not read threads; allowing removal", { + worktreePath: input.worktreePath, + detail: cause.toString(), + }).pipe(Effect.as([] as ReadonlyArray)), + ), + ); + const blockingThreads = findWorktreeBlockingThreads({ + worktreePath: input.worktreePath, + threads, + }); + if (blockingThreads.length === 0) { + return; + } + return yield* new VcsWorktreeInUseError({ + worktreePath: input.worktreePath, + blockingThreads, + }); + }); diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index c22b1d152..393131b60 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -115,6 +115,7 @@ import { WorkspaceEntries } from "./workspace/Services/WorkspaceEntries.ts"; import { WorkspaceFileSystem } from "./workspace/Services/WorkspaceFileSystem.ts"; import { WorkspacePathOutsideRootError } from "./workspace/Services/WorkspacePaths.ts"; import { VcsStatusBroadcaster } from "./vcs/VcsStatusBroadcaster.ts"; +import { ensureWorktreeRemovable } from "./vcs/WorktreeRemovalGuard.ts"; import { VcsProvisioningService } from "./vcs/VcsProvisioningService.ts"; import { GitAuthRemediationService } from "./git/GitAuthRemediationService.ts"; import { GitWorkflowService } from "./git/GitWorkflowService.ts"; @@ -1934,7 +1935,20 @@ const makeWsRpcLayer = (currentSessionId: AuthSessionId) => [WS_METHODS.vcsRemoveWorktree]: (input) => observeRpcEffect( WS_METHODS.vcsRemoveWorktree, - gitWorkflow.removeWorktree(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))), + // Guarded before the git call, not after: once the directory is + // gone the thread that lived in it has no way back. + ensureWorktreeRemovable({ + worktreePath: input.path, + readThreads: projectionSnapshotQuery + .getShellSnapshot() + .pipe(Effect.map((snapshot) => snapshot.threads)), + }).pipe( + Effect.andThen( + gitWorkflow + .removeWorktree(input) + .pipe(Effect.tap(() => refreshGitStatus(input.cwd))), + ), + ), { "rpc.aggregate": "vcs" }, ), [WS_METHODS.vcsCreateRef]: (input) => diff --git a/apps/web/src/components/BranchToolbar.logic.test.ts b/apps/web/src/components/BranchToolbar.logic.test.ts index c3ea0d256..7c12dc7d3 100644 --- a/apps/web/src/components/BranchToolbar.logic.test.ts +++ b/apps/web/src/components/BranchToolbar.logic.test.ts @@ -1,8 +1,10 @@ import { EnvironmentId, type VcsRef } from "@threadlines/contracts"; import { describe, expect, it } from "vite-plus/test"; import { + annotateMissingCheckoutLabel, dedupeRemoteBranchesWithLocalMatches, deriveLocalBranchNameFromRemoteRef, + resolveCheckoutPickerRefsCwd, resolveEnvironmentOptionLabel, resolveActiveWorktreePath, resolveBranchSelectionTarget, @@ -580,3 +582,48 @@ describe("shouldIncludeBranchPickerItem", () => { ).toBe(false); }); }); + +describe("resolveCheckoutPickerRefsCwd", () => { + // The picker used to list refs from the thread's own checkout only. Once that + // folder was deleted the list came back empty, so the picker was on screen + // with nothing in it and no way back to the project root. + it("falls back to the project root when the selected checkout is gone", () => { + expect( + resolveCheckoutPickerRefsCwd({ + selectedCwd: "/repo/.worktrees/feature", + projectCwd: "/repo", + selectedCheckoutMissing: true, + }), + ).toBe("/repo"); + }); + + it("lists refs from the selected checkout while it exists", () => { + expect( + resolveCheckoutPickerRefsCwd({ + selectedCwd: "/repo/.worktrees/feature", + projectCwd: "/repo", + selectedCheckoutMissing: false, + }), + ).toBe("/repo/.worktrees/feature"); + }); + + it("keeps the selected checkout when there is no project root to fall back to", () => { + expect( + resolveCheckoutPickerRefsCwd({ + selectedCwd: "/repo/.worktrees/feature", + projectCwd: null, + selectedCheckoutMissing: true, + }), + ).toBe("/repo/.worktrees/feature"); + }); +}); + +describe("annotateMissingCheckoutLabel", () => { + it("marks a selection whose folder is gone", () => { + expect(annotateMissingCheckoutLabel("feature/x", true)).toBe("feature/x (missing)"); + }); + + it("leaves a healthy selection untouched", () => { + expect(annotateMissingCheckoutLabel("feature/x", false)).toBe("feature/x"); + }); +}); diff --git a/apps/web/src/components/BranchToolbar.logic.ts b/apps/web/src/components/BranchToolbar.logic.ts index d56609951..707589d9e 100644 --- a/apps/web/src/components/BranchToolbar.logic.ts +++ b/apps/web/src/components/BranchToolbar.logic.ts @@ -158,6 +158,36 @@ export function hasActiveThreadTurn( return session.orchestrationStatus === "running" || session.orchestrationStatus === "starting"; } +/** + * Which checkout the picker lists refs from. + * + * Normally the thread's own. When that folder has been deleted, listing refs + * there returns nothing, which left the picker on screen but empty: no project + * root, no other worktrees, nothing to switch to. Falling back to the project + * root keeps the valid alternatives listed so there is always a way out. + */ +export function resolveCheckoutPickerRefsCwd(input: { + readonly selectedCwd: string | null; + readonly projectCwd: string | null; + readonly selectedCheckoutMissing: boolean; +}): string | null { + if (input.selectedCheckoutMissing && input.projectCwd) { + return input.projectCwd; + } + return input.selectedCwd; +} + +/** + * Marks the picker's current selection as a folder that is no longer there. + * + * Plain trailing text rather than a badge or colour: the picker's job here is + * to say what is selected and let the user pick something else, and the + * recovery actions live in the notice above the composer. + */ +export function annotateMissingCheckoutLabel(label: string, missing: boolean): string { + return missing ? `${label} (missing)` : label; +} + /** How a checkout reads in prose: worktrees by name, the project root by role. */ export function resolveCheckoutDisplayLabel( checkoutCwd: string, diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index 417ac5829..c5263bf49 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -26,12 +26,14 @@ import { getSourceControlPresentation } from "../sourceControlPresentation"; import { useStore } from "../store"; import { createProjectSelectorByRef, createThreadSelectorByRef } from "../storeSelectors"; import { + annotateMissingCheckoutLabel, deriveLocalBranchNameFromRemoteRef, hasActiveThreadTurn, queuedCheckoutSwitchToast, resolveActiveWorktreePath, resolveBranchSelectionTarget, resolveBranchToolbarValue, + resolveCheckoutPickerRefsCwd, resolveDraftEnvModeAfterBranchChange, resolveEffectiveEnvMode, shouldIncludeBranchPickerItem, @@ -223,15 +225,24 @@ export function BranchToolbarBranchSelector({ const deferredBranchQuery = useDeferredValue(branchQuery); const branchStatusQuery = useGitStatus({ environmentId, cwd: branchCwd }); + // A checkout that was deleted lists no refs at all, which would leave the + // picker open and empty. Refs then come from the project root so the valid + // alternatives are still there to switch to. + const selectedCheckoutMissing = branchStatusQuery.data?.pathMissing === true; + const refsCwd = resolveCheckoutPickerRefsCwd({ + selectedCwd: branchCwd, + projectCwd: activeProjectCwd, + selectedCheckoutMissing, + }); const trimmedBranchQuery = branchQuery.trim(); const deferredTrimmedBranchQuery = deferredBranchQuery.trim(); useEffect(() => { - if (!branchCwd) return; + if (!refsCwd) return; void queryClient.prefetchInfiniteQuery( - gitBranchSearchInfiniteQueryOptions({ environmentId, cwd: branchCwd, query: "" }), + gitBranchSearchInfiniteQueryOptions({ environmentId, cwd: refsCwd, query: "" }), ); - }, [branchCwd, environmentId, queryClient]); + }, [refsCwd, environmentId, queryClient]); const { data: branchesSearchData, @@ -242,7 +253,7 @@ export function BranchToolbarBranchSelector({ } = useInfiniteQuery( gitBranchSearchInfiniteQueryOptions({ environmentId, - cwd: branchCwd, + cwd: refsCwd, query: deferredTrimmedBranchQuery, }), ); @@ -250,8 +261,12 @@ export function BranchToolbarBranchSelector({ () => branchesSearchData?.pages.flatMap((page) => page.refs) ?? [], [branchesSearchData?.pages], ); + // With the checkout deleted, `refs` list the project root's branches, whose + // current branch is the root's, not this thread's. Skipping that fallback + // lets the label resolve from the thread's own recorded branch instead. const currentGitBranch = - branchStatusQuery.data?.refName ?? refs.find((refName) => refName.current)?.name ?? null; + branchStatusQuery.data?.refName ?? + (selectedCheckoutMissing ? null : (refs.find((refName) => refName.current)?.name ?? null)); const sourceControlPresentation = useMemo( () => getSourceControlPresentation(branchStatusQuery.data?.sourceControlProvider), [branchStatusQuery.data?.sourceControlProvider], @@ -583,11 +598,14 @@ export function BranchToolbarBranchSelector({ maybeFetchNextBranchPage(); }, [refs.length, maybeFetchNextBranchPage, shouldVirtualizeBranchList]); - const triggerLabel = getBranchTriggerLabel({ - activeWorktreePath, - effectiveEnvMode, - resolvedActiveBranch, - }); + const triggerLabel = annotateMissingCheckoutLabel( + getBranchTriggerLabel({ + activeWorktreePath, + effectiveEnvMode, + resolvedActiveBranch, + }), + selectedCheckoutMissing, + ); function renderPickerItem(itemValue: string, index: number) { if (checkoutPullRequestItemValue && itemValue === checkoutPullRequestItemValue) { diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index 72fb4d474..89d727737 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -83,6 +83,9 @@ import { BrowserWsRpcHarness, type NormalizedWsRpcRequestBody } from "../../test import { DEFAULT_CLIENT_SETTINGS } from "@threadlines/contracts/settings"; vi.mock("../lib/gitStatusState", () => ({ + // Read synchronously by the draft seeder to skip a checkout already known + // to be missing; these suites never exercise a deleted checkout. + getGitStatusSnapshot: () => ({ data: null, error: null, cause: null, isPending: false }), GIT_STATUS_STALE_MESSAGE: "Source control status isn't updating.", useGitStatus: () => ({ data: null, error: null, cause: null, isPending: false }), useGitStatuses: () => new Map(), diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 77282fc07..afa0bdf87 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -41,6 +41,9 @@ import { lazy, memo, Suspense, useCallback, useEffect, useMemo, useRef, useState import { useNavigate, useSearch } from "@tanstack/react-router"; import { useShallow } from "zustand/react/shallow"; import { useGitStatus } from "~/lib/gitStatusState"; +import { shouldShowCheckoutPicker } from "~/lib/checkoutRecovery"; +import { useCheckoutRecovery } from "../hooks/useCheckoutRecovery"; +import { buildCheckoutMissingNotice } from "./chat/checkoutMissingNotice"; import { usePrimaryEnvironmentId } from "../environments/primary"; import { readEnvironmentApi } from "../environmentApi"; import { ELECTRON_HEADER_HEIGHT_CLASS } from "../desktopChrome"; @@ -2411,6 +2414,14 @@ export default function ChatView(props: ChatViewProps) { }) : null; const gitStatusQuery = useGitStatus({ environmentId, cwd: gitCwd }); + // Watched separately from the thread's own checkout: when that checkout is + // gone, the project root's status is what tells us whether there is still a + // repository to fall back to. Same key as every other subscriber of this + // path, so it costs a ref-count rather than a second subscription. + const projectRootStatusQuery = useGitStatus({ + environmentId, + cwd: activeProject?.cwd ?? null, + }); const keybindings = useServerKeybindings(); const availableEditors = useServerAvailableEditors(); // Prefer an instance-id match so a custom Codex instance (e.g. @@ -2637,8 +2648,13 @@ export default function ChatView(props: ChatViewProps) { terminalLaunchContext?.threadId === activeThreadId ? terminalLaunchContext : (storeServerTerminalLaunchContext ?? null); - // Default true while loading to avoid toolbar flicker. - const isGitRepo = gitStatusQuery.data?.isRepo ?? true; + // Stays true for a checkout that was deleted, as long as the project root is + // still a repository: hiding the picker there would remove the only way back + // to a working checkout at exactly the moment the user needs it. + const isGitRepo = shouldShowCheckoutPicker({ + selectedStatus: gitStatusQuery.data, + projectRootStatus: projectRootStatusQuery.data, + }); const browserPanelState = useBrowserPanelStore((store) => selectThreadBrowserState(store.browserStateByThreadKey, routeThreadRef), ); @@ -5358,6 +5374,33 @@ export default function ChatView(props: ChatViewProps) { turnRetryDispatchingThreadId, ]); + // The thread's folder can vanish underneath it (an agent removing its own + // worktree after a merge, or the user deleting it in a terminal). The hook + // owns the detection and both ways out; this surface only renders them. + const checkoutRecoveryView = useCheckoutRecovery({ + environmentId, + threadId: activeThread?.id ?? null, + cwd: gitCwd, + projectCwd: activeProject?.cwd ?? null, + branch: activeThread?.branch ?? null, + status: gitStatusQuery.data, + }); + const checkoutRecovery = checkoutRecoveryView.recovery; + const checkoutMissingNotice = useMemo( + () => + checkoutRecovery + ? buildCheckoutMissingNotice({ + recovery: checkoutRecovery, + actions: { + isBusy: checkoutRecoveryView.isBusy, + onSwitchToProjectRoot: checkoutRecoveryView.onSwitchToProjectRoot, + onRecreateWorktree: checkoutRecoveryView.onRecreateWorktree, + }, + }) + : null, + [checkoutRecovery, checkoutRecoveryView], + ); + const providerStatusNotice = useProviderStatusNotice({ status: activeProviderStatus, activeTurnInProgress, @@ -5379,7 +5422,13 @@ export default function ChatView(props: ChatViewProps) { const threadErrorNotice = useMemo( () => buildThreadErrorNotice({ - error: threadErrorNoticeVisible ? (activeThread?.error ?? null) : null, + // A missing checkout has its own notice with actions that can actually + // work; showing "Turn failed … Retry" beside it would only offer the + // user the one button guaranteed to fail again. + error: + threadErrorNoticeVisible && checkoutRecovery === null + ? (activeThread?.error ?? null) + : null, authReconnect: providerAuthReconnectPrompt, usageReset: threadErrorUsageResetAction, retry: threadErrorRetryAction, @@ -5391,6 +5440,7 @@ export default function ChatView(props: ChatViewProps) { activeProviderLabel, activeThread?.error, activeThread?.id, + checkoutRecovery, composerSignInView, providerAuthReconnectPrompt, setThreadError, @@ -5424,6 +5474,7 @@ export default function ChatView(props: ChatViewProps) { const composerNotices = useMemo( () => selectComposerNotices([ + checkoutMissingNotice, threadErrorNotice, sendPreflightNotice, providerStatusNotice, @@ -5431,6 +5482,7 @@ export default function ChatView(props: ChatViewProps) { ...infrastructureComposerNotices, ]), [ + checkoutMissingNotice, infrastructureComposerNotices, providerStatusNotice, sendPreflightNotice, diff --git a/apps/web/src/components/KeybindingsToast.browser.tsx b/apps/web/src/components/KeybindingsToast.browser.tsx index a0e895813..8f4778ec4 100644 --- a/apps/web/src/components/KeybindingsToast.browser.tsx +++ b/apps/web/src/components/KeybindingsToast.browser.tsx @@ -43,6 +43,9 @@ import { createAuthenticatedSessionHandlers } from "../../test/authHttpHandlers" import { BrowserWsRpcHarness } from "../../test/wsRpcHarness"; vi.mock("../lib/gitStatusState", () => ({ + // Read synchronously by the draft seeder to skip a checkout already known + // to be missing; these suites never exercise a deleted checkout. + getGitStatusSnapshot: () => ({ data: null, error: null, cause: null, isPending: false }), GIT_STATUS_STALE_MESSAGE: "Source control status isn't updating.", useGitStatus: () => ({ data: null, error: null, cause: null, isPending: false }), useGitStatuses: () => new Map(), diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index ec190a2bb..e8969d47e 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -3374,6 +3374,13 @@ function classifySummarizableActivityEntry( return null; } + // A warning is a message to the user, never routine activity. The keyword + // heuristics below would happily fold "This thread's folder no longer + // exists" into "Explored project" because it mentions a folder. + if (entry.tone === "warning" || entry.tone === "error") { + return null; + } + if (isCommandWorkEntry(entry) && entry.command) { const summary = classifyCommandActivity(entry.command); // Consequential commands (anything that isn't routine exploration or diff --git a/apps/web/src/components/chat/checkoutMissingNotice.tsx b/apps/web/src/components/chat/checkoutMissingNotice.tsx new file mode 100644 index 000000000..4509feef8 --- /dev/null +++ b/apps/web/src/components/chat/checkoutMissingNotice.tsx @@ -0,0 +1,68 @@ +/** + * The composer notice for a thread whose folder was deleted. + * + * This replaces the generic "Turn failed … Retry" row, which was a dead end + * here: the turn failed because the directory the session runs in is gone, so + * retrying reproduces it exactly. The two things that actually resolve it are + * offered instead — move the thread to the project root, or put the folder back. + * + * Warning severity, not error: the thread is recoverable in one click, and the + * notice dock reads severity as urgency. + * + * @module checkoutMissingNotice + */ +import { FolderXIcon, RotateCcwIcon } from "lucide-react"; + +import type { CheckoutRecoveryState } from "../../lib/checkoutRecovery"; +import { Button } from "../ui/button"; +import type { ComposerNotice } from "./composerNotices"; + +export interface CheckoutRecoveryActions { + readonly onSwitchToProjectRoot: () => void; + readonly onRecreateWorktree: () => void; + /** A recovery action is running; both buttons wait it out. */ + readonly isBusy: boolean; +} + +export function buildCheckoutMissingNotice({ + recovery, + actions, +}: { + recovery: CheckoutRecoveryState; + actions: CheckoutRecoveryActions; +}): ComposerNotice { + return { + id: "checkout-missing", + severity: "warning", + lead: "This thread's folder no longer exists.", + detail: {recovery.cwd}, + actions: ( + <> + {recovery.canSwitchToProjectRoot ? ( + + ) : null} + {recovery.canRecreateWorktree ? ( + + ) : null} + + ), + }; +} diff --git a/apps/web/src/components/chat/rightPanelLauncherState.test.ts b/apps/web/src/components/chat/rightPanelLauncherState.test.ts index e7183a5c6..dc851a543 100644 --- a/apps/web/src/components/chat/rightPanelLauncherState.test.ts +++ b/apps/web/src/components/chat/rightPanelLauncherState.test.ts @@ -99,6 +99,26 @@ describe("buildRightPanelLauncherStates", () => { expect(states.diff).toEqual({ description: "No changes to review.", empty: true }); }); + it("says the folder is missing instead of describing a tree that is not there", () => { + const states = buildRightPanelLauncherStates({ + workingTreeFileCount: null, + diffHasExplicitTarget: false, + checkoutMissing: true, + ...EMPTY_THREAD, + }); + + // Source stays lit — the recovery actions live behind it — but neither git + // surface may claim working-tree facts about a deleted checkout. + expect(states.sourceControl).toEqual({ + description: "This thread's folder is missing.", + empty: false, + }); + expect(states.diff).toEqual({ + description: "This thread's folder is missing.", + empty: true, + }); + }); + it("reports the same change count on both git surfaces and dims neither", () => { const states = buildRightPanelLauncherStates({ workingTreeFileCount: 12, diff --git a/apps/web/src/components/chat/rightPanelLauncherState.ts b/apps/web/src/components/chat/rightPanelLauncherState.ts index aaaff324c..c20fd75fd 100644 --- a/apps/web/src/components/chat/rightPanelLauncherState.ts +++ b/apps/web/src/components/chat/rightPanelLauncherState.ts @@ -102,13 +102,24 @@ function changedFilesDescription(fileCount: number): string { return `${pluralize(fileCount, "file")} changed.`; } +/** What both tree-backed rows say when the checkout folder itself is gone. */ +const MISSING_CHECKOUT_DESCRIPTION = "This thread's folder is missing."; + /** * Source reports the working tree but is never dimmed by it: switching branches, * committing, pushing and opening a pull request are all reasons to go there * with nothing changed, so a clean tree is a fact about the tree rather than a * surface with nothing in it. */ -function sourceControlState(fileCount: number | null): RightPanelSurfaceState { +function sourceControlState( + fileCount: number | null, + checkoutMissing: boolean, +): RightPanelSurfaceState { + // Source is where the recovery actions live, so the row stays lit — but it + // must not describe a working tree that is not there. + if (checkoutMissing) { + return { description: MISSING_CHECKOUT_DESCRIPTION, empty: false }; + } if (fileCount === null) { return staticState("sourceControl"); } @@ -132,7 +143,11 @@ function diffState(input: { readonly fileCount: number | null; readonly reviewableTurnCount: number | null; readonly hasExplicitTarget: boolean; + readonly checkoutMissing: boolean; }): RightPanelSurfaceState { + if (input.checkoutMissing) { + return { description: MISSING_CHECKOUT_DESCRIPTION, empty: true }; + } // A tab aimed at one file or one turn is not the working tree's story at all. if (input.hasExplicitTarget || input.fileCount === null) { return staticState("diff"); @@ -196,13 +211,18 @@ export function buildRightPanelLauncherStates(input: { * tree, so a clean tree says nothing about whether it is empty. */ readonly diffHasExplicitTarget: boolean; readonly agents: RightPanelLauncherAgentsInput | null; + /** The checkout folder itself no longer exists; the tree-backed rows must + * not describe a working tree that is not there. */ + readonly checkoutMissing?: boolean; }): RightPanelLauncherStates { + const checkoutMissing = input.checkoutMissing === true; return { - sourceControl: sourceControlState(input.workingTreeFileCount), + sourceControl: sourceControlState(input.workingTreeFileCount, checkoutMissing), diff: diffState({ fileCount: input.workingTreeFileCount, reviewableTurnCount: input.reviewableTurnCount, hasExplicitTarget: input.diffHasExplicitTarget, + checkoutMissing, }), agents: agentsState(input.agents), }; @@ -234,6 +254,7 @@ export function useRightPanelLauncherStates(input: { const workingTreeFileCount = resolveWorkingTreeFileCount(gitStatus.data); const reviewableTurnCount = countReviewableTurnDiffs(input.turnDiffSummaries); const diffHasExplicitTarget = rightPanelDiffTargetIsExplicit(input.diffTarget); + const checkoutMissing = gitStatus.data?.pathMissing === true; return useMemo( () => enabled @@ -242,8 +263,16 @@ export function useRightPanelLauncherStates(input: { reviewableTurnCount, diffHasExplicitTarget, agents, + checkoutMissing, }) : undefined, - [agents, diffHasExplicitTarget, enabled, reviewableTurnCount, workingTreeFileCount], + [ + agents, + checkoutMissing, + diffHasExplicitTarget, + enabled, + reviewableTurnCount, + workingTreeFileCount, + ], ); } diff --git a/apps/web/src/components/sidebar/ThreadHoverCard.browser.tsx b/apps/web/src/components/sidebar/ThreadHoverCard.browser.tsx index 4900118cd..147c1598b 100644 --- a/apps/web/src/components/sidebar/ThreadHoverCard.browser.tsx +++ b/apps/web/src/components/sidebar/ThreadHoverCard.browser.tsx @@ -11,6 +11,9 @@ import { describe, expect, it, vi } from "vite-plus/test"; const gitStatusRef = vi.hoisted(() => ({ current: null as string | null })); vi.mock("../../lib/gitStatusState", () => ({ + // Read synchronously by the draft seeder to skip a checkout already known + // to be missing; these suites never exercise a deleted checkout. + getGitStatusSnapshot: () => ({ data: null, error: null, cause: null, isPending: false }), useGitStatus: () => ({ data: gitStatusRef.current === null ? null : { refName: gitStatusRef.current }, error: null, diff --git a/apps/web/src/components/source-control/SourceControlPanel.browser.tsx b/apps/web/src/components/source-control/SourceControlPanel.browser.tsx index 107654cfb..bfe4740e0 100644 --- a/apps/web/src/components/source-control/SourceControlPanel.browser.tsx +++ b/apps/web/src/components/source-control/SourceControlPanel.browser.tsx @@ -58,6 +58,9 @@ const environmentRuntimeMock = vi.hoisted(() => ({ })); vi.mock("~/lib/gitStatusState", () => ({ + // Read synchronously by the draft seeder to skip a checkout already known + // to be missing; these suites never exercise a deleted checkout. + getGitStatusSnapshot: () => ({ data: null, error: null, cause: null, isPending: false }), GIT_STATUS_STALE_MESSAGE: "Source control status isn't updating.", useGitStatus: () => ({ data: gitStatusMock.data, diff --git a/apps/web/src/components/source-control/SourceControlPanel.tsx b/apps/web/src/components/source-control/SourceControlPanel.tsx index 067fd3b00..f70d5070b 100644 --- a/apps/web/src/components/source-control/SourceControlPanel.tsx +++ b/apps/web/src/components/source-control/SourceControlPanel.tsx @@ -107,6 +107,7 @@ import { copyTextToClipboard } from "~/lib/clipboard"; import { cn, newCommandId, newThreadId, randomUUID } from "~/lib/utils"; import { useLocalStorage } from "~/hooks/useLocalStorage"; import { useSettings } from "~/hooks/useSettings"; +import { useCheckoutRecovery } from "~/hooks/useCheckoutRecovery"; import { readLocalApi } from "~/localApi"; import { useComposerDraftStore } from "~/composerDraftStore"; import { @@ -2145,6 +2146,17 @@ export function SourceControlPanel({ // render model while its API is absent, otherwise that stale `isRepo` value // enables graph, stash, and branch queries against a missing connection. const status = environmentApiAvailable ? gitStatus.data : null; + // A deleted checkout reaches this panel as a status with no repository, which + // used to read as "not a repo yet" and offered to initialize one. The same + // hook the composer notice uses tells the two apart and carries the actions. + const checkoutRecoveryView = useCheckoutRecovery({ + environmentId, + threadId: activeThreadRef?.threadId ?? null, + cwd, + projectCwd: reviewProject?.cwd ?? null, + branch: reviewThread?.branch ?? null, + status, + }); const parentRepositoryRoot = status?.isRepo && status.repositoryRootRelation === "ancestor" ? (status.repositoryRoot ?? null) @@ -4349,7 +4361,43 @@ export function SourceControlPanel({ ) : null} - {status?.isRepo === false ? ( + {checkoutRecoveryView.recovery ? ( +
+

This thread's folder no longer exists.

+

+ {checkoutRecoveryView.recovery.cwd} +

+
+ {checkoutRecoveryView.recovery.canSwitchToProjectRoot ? ( + + ) : null} + {checkoutRecoveryView.recovery.canRecreateWorktree ? ( + + ) : null} +
+
+ ) : null} + {/* Only offered for a folder that exists but holds no repository. + A checkout that was deleted gets the recovery section above: running + `git init` there would recreate the directory as an unrelated empty + repository and quietly strand the thread's real work. */} + {status?.isRepo === false && !checkoutRecoveryView.recovery ? (

No Git repository

diff --git a/apps/web/src/composerDraftStore.test.ts b/apps/web/src/composerDraftStore.test.ts index 542c1c9d0..91152893b 100644 --- a/apps/web/src/composerDraftStore.test.ts +++ b/apps/web/src/composerDraftStore.test.ts @@ -58,6 +58,21 @@ function selectionsByProvider( } import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; +// The draft seeder asks the git status the client already holds whether an +// inherited checkout still exists. Narrow module, stubbed per test. +const missingCheckoutPaths = new Set(); +vi.mock("./lib/gitStatusState", () => ({ + getGitStatusSnapshot: (target: { readonly cwd: string | null }) => ({ + data: + target.cwd !== null && missingCheckoutPaths.has(target.cwd) + ? { isRepo: false, pathMissing: true } + : null, + error: null, + cause: null, + isPending: false, + }), +})); + import { COMPOSER_DRAFT_STORAGE_KEY, composerDraftHasUserContent, @@ -1030,6 +1045,51 @@ describe("composerDraftStore project draft thread mapping", () => { beforeEach(() => { resetComposerDraftStore(); + missingCheckoutPaths.clear(); + }); + + // Reusing an empty draft for a new thread carries its checkout forward. When + // that folder has been deleted the carried-over path spread one broken thread + // across the project: no git status, no source control, and a checkout picker + // that had hidden itself, so there was no way back to the project root. + it("drops a carried-over checkout that is known to be gone", () => { + const deadWorktree = "/tmp/worktree-deleted"; + useComposerDraftStore.getState().setProjectDraftThreadId(projectRef, draftId, { + threadId, + branch: "feature/x", + worktreePath: deadWorktree, + }); + missingCheckoutPaths.add(deadWorktree); + + // Reusing the draft for a new thread: no checkout stated, so it inherits. + useComposerDraftStore + .getState() + .setProjectDraftThreadId(projectRef, draftId, { threadId: otherThreadId }); + + expect(useComposerDraftStore.getState().getDraftThread(draftId)).toMatchObject({ + worktreePath: null, + branch: null, + envMode: "local", + }); + }); + + it("still carries over a checkout that is still on disk", () => { + const liveWorktree = "/tmp/worktree-live"; + useComposerDraftStore.getState().setProjectDraftThreadId(projectRef, draftId, { + threadId, + branch: "feature/x", + worktreePath: liveWorktree, + }); + + useComposerDraftStore + .getState() + .setProjectDraftThreadId(projectRef, draftId, { threadId: otherThreadId }); + + expect(useComposerDraftStore.getState().getDraftThread(draftId)).toMatchObject({ + worktreePath: liveWorktree, + branch: "feature/x", + envMode: "worktree", + }); }); it("stores and reads project draft thread ids via actions", () => { diff --git a/apps/web/src/composerDraftStore.ts b/apps/web/src/composerDraftStore.ts index c69f05611..590238f68 100644 --- a/apps/web/src/composerDraftStore.ts +++ b/apps/web/src/composerDraftStore.ts @@ -53,6 +53,8 @@ import { normalizeFileSelectionContextDraft, } from "./lib/fileSelectionContext"; import type { DrawingContextDraft } from "./lib/drawingContext"; +import { seedDraftWorktreePath } from "./lib/checkoutRecovery"; +import { getGitStatusSnapshot } from "./lib/gitStatusState"; import { normalizePickedElementContextDraft, type PickedElementContextDraft, @@ -1480,6 +1482,21 @@ function toProjectDraftSession( }; } +/** + * Whether the client already knows this checkout is gone. + * + * Reads the VCS status the app is holding for that path rather than asking the + * server, so seeding a draft stays synchronous and costs nothing. An unknown + * path (never subscribed, still loading) answers false: a draft is only + * redirected on positive knowledge, never on the absence of it. + */ +function isKnownMissingCheckout(target: { + readonly environmentId: EnvironmentId; + readonly cwd: string; +}): boolean { + return getGitStatusSnapshot(target).data?.pathMissing === true; +} + function createDraftThreadState( projectRef: ScopedProjectRef, threadId: ThreadId, @@ -1499,15 +1516,25 @@ function createDraftThreadState( existingThread !== undefined && (existingThread.environmentId !== projectRef.environmentId || existingThread.projectId !== projectRef.projectId); - const nextWorktreePath = - options?.worktreePath === undefined - ? projectChanged - ? null - : (existingThread?.worktreePath ?? null) - : (options.worktreePath ?? null); + const inheritedWorktreePath = existingThread?.worktreePath ?? null; + const nextWorktreePath = seedDraftWorktreePath({ + requested: options?.worktreePath, + inherited: inheritedWorktreePath, + projectChanged, + isCheckoutMissing: (cwd) => + isKnownMissingCheckout({ environmentId: projectRef.environmentId, cwd }), + }); + // A checkout dropped because it no longer exists takes its branch with it: + // that branch named a ref inside the folder that is gone, so carrying it onto + // a project-root draft would just point the picker at the wrong thing. + const droppedMissingCheckout = + options?.worktreePath === undefined && + !projectChanged && + inheritedWorktreePath !== null && + nextWorktreePath === null; const nextBranch = options?.branch === undefined - ? projectChanged + ? projectChanged || droppedMissingCheckout ? null : (existingThread?.branch ?? null) : (options.branch ?? null); @@ -1526,7 +1553,11 @@ function createDraftThreadState( options?.envMode ?? (nextWorktreePath ? "worktree" - : projectChanged + : // A checkout dropped for being missing lands the draft at the project + // root. Leaving the mode on "worktree" would instead silently cut a + // brand-new worktree on the next send, which is not what the user + // asked for and hides that anything went wrong. + projectChanged || droppedMissingCheckout ? "local" : (existingThread?.envMode ?? "local")), promotedTo: null, diff --git a/apps/web/src/hooks/useCheckoutRecovery.ts b/apps/web/src/hooks/useCheckoutRecovery.ts new file mode 100644 index 000000000..4c9d77c90 --- /dev/null +++ b/apps/web/src/hooks/useCheckoutRecovery.ts @@ -0,0 +1,147 @@ +/** + * Recovery state and actions for a thread whose folder was deleted. + * + * Two surfaces have to offer the same way out — the notice above the composer + * and the source control panel — and they must agree on whether the checkout is + * missing, whether it can be recreated, and what happens when the user acts. So + * the branch lookup, the two mutations, and the busy state live here once + * instead of being rebuilt per surface. + * + * @module useCheckoutRecovery + */ +import { useInfiniteQuery, useQueryClient } from "@tanstack/react-query"; +import { useCallback, useMemo, useState } from "react"; + +import type { EnvironmentId, ThreadId } from "@threadlines/contracts"; +import { readEnvironmentApi } from "../environmentApi"; +import { + type CheckoutRecoveryState, + type CheckoutStatusLike, + selectCheckoutRecovery, +} from "../lib/checkoutRecovery"; +import { gitBranchSearchInfiniteQueryOptions, invalidateGitQueries } from "../lib/gitReactQuery"; +import { newCommandId } from "../lib/utils"; +import { stackedThreadToast, toastManager } from "../components/ui/toast"; + +export interface CheckoutRecoveryView { + /** Null when nothing is wrong with this thread's checkout. */ + readonly recovery: CheckoutRecoveryState | null; + readonly isBusy: boolean; + readonly onSwitchToProjectRoot: () => void; + readonly onRecreateWorktree: () => void; +} + +export function useCheckoutRecovery(input: { + readonly environmentId: EnvironmentId | null; + readonly threadId: ThreadId | null; + /** Working directory the thread's surfaces operate in. */ + readonly cwd: string | null; + readonly projectCwd: string | null; + readonly branch: string | null; + readonly status: CheckoutStatusLike | null | undefined; +}): CheckoutRecoveryView { + const queryClient = useQueryClient(); + const [isBusy, setIsBusy] = useState(false); + const pathMissing = input.status?.pathMissing === true; + const branch = input.branch; + + // Only runs once a checkout is known to be missing, so the ordinary case + // costs nothing. Recreating a worktree needs its branch to still exist; + // offering the action without checking would hand the user a button that + // fails. + const branchQuery = useInfiniteQuery( + gitBranchSearchInfiniteQueryOptions({ + environmentId: input.environmentId, + cwd: input.projectCwd, + query: branch ?? "", + enabled: pathMissing && branch !== null && input.projectCwd !== null, + }), + ); + + const branchExists = useMemo(() => { + if (!pathMissing || branch === null) { + return undefined; + } + if (branchQuery.isPending || branchQuery.data === undefined) { + return undefined; + } + return branchQuery.data.pages.some((page) => + page.refs.some((ref) => !ref.isRemote && ref.name === branch), + ); + }, [branch, branchQuery.data, branchQuery.isPending, pathMissing]); + + const recovery = useMemo( + () => + selectCheckoutRecovery({ + cwd: input.cwd, + projectCwd: input.projectCwd, + branch, + status: input.status, + branchExists, + }), + [branch, branchExists, input.cwd, input.projectCwd, input.status], + ); + + const run = useCallback( + async ( + action: (api: NonNullable>) => Promise, + ) => { + const environmentId = input.environmentId; + if (environmentId === null) { + return; + } + const api = readEnvironmentApi(environmentId); + if (!api) { + return; + } + setIsBusy(true); + try { + await action(api); + await invalidateGitQueries(queryClient, { environmentId }); + } catch (error) { + toastManager.add( + stackedThreadToast({ + type: "error", + title: "Could not recover this thread's folder", + description: error instanceof Error ? error.message : "Unknown error.", + }), + ); + } finally { + setIsBusy(false); + } + }, + [input.environmentId, queryClient], + ); + + const onSwitchToProjectRoot = useCallback(() => { + const threadId = input.threadId; + if (!threadId) { + return; + } + // The same command the checkout picker dispatches: clearing the worktree + // moves the thread to the project root, and the server cycles the session + // into it on the next turn. + void run((api) => + api.orchestration.dispatchCommand({ + type: "thread.meta.update", + commandId: newCommandId(), + threadId, + worktreePath: null, + }), + ); + }, [input.threadId, run]); + + const onRecreateWorktree = useCallback(() => { + if (!recovery?.projectCwd || !recovery.branch) { + return; + } + const projectCwd = recovery.projectCwd; + const refName = recovery.branch; + const path = recovery.cwd; + // Recreated at the same path on the same branch, so the thread resumes + // exactly where it was rather than being moved somewhere new. + void run((api) => api.vcs.createWorktree({ cwd: projectCwd, refName, path })); + }, [recovery, run]); + + return { recovery, isBusy, onSwitchToProjectRoot, onRecreateWorktree }; +} diff --git a/apps/web/src/lib/checkoutRecovery.test.ts b/apps/web/src/lib/checkoutRecovery.test.ts new file mode 100644 index 000000000..22c0a8d70 --- /dev/null +++ b/apps/web/src/lib/checkoutRecovery.test.ts @@ -0,0 +1,181 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { + seedDraftWorktreePath, + selectCheckoutRecovery, + shouldShowCheckoutPicker, +} from "./checkoutRecovery"; + +const WORKTREE = "/repo/.worktrees/feature"; +const PROJECT = "/repo"; + +const missingStatus = { isRepo: false, pathMissing: true } as const; +const emptyDirStatus = { isRepo: false } as const; +const healthyStatus = { isRepo: true } as const; + +describe("selectCheckoutRecovery", () => { + it("offers both ways out when the worktree is gone and its branch still exists", () => { + expect( + selectCheckoutRecovery({ + cwd: WORKTREE, + projectCwd: PROJECT, + branch: "feature/x", + status: missingStatus, + branchExists: true, + }), + ).toEqual({ + cwd: WORKTREE, + label: "feature", + projectCwd: PROJECT, + branch: "feature/x", + canSwitchToProjectRoot: true, + canRecreateWorktree: true, + }); + }); + + it("withholds recreate when the branch is gone from the repository", () => { + const recovery = selectCheckoutRecovery({ + cwd: WORKTREE, + projectCwd: PROJECT, + branch: "feature/x", + status: missingStatus, + branchExists: false, + }); + expect(recovery?.canRecreateWorktree).toBe(false); + expect(recovery?.canSwitchToProjectRoot).toBe(true); + }); + + it("withholds recreate until the branch lookup answers", () => { + expect( + selectCheckoutRecovery({ + cwd: WORKTREE, + projectCwd: PROJECT, + branch: "feature/x", + status: missingStatus, + branchExists: undefined, + })?.canRecreateWorktree, + ).toBe(false); + }); + + // A directory that exists but holds no repository is the "Initialize Git" + // case, which must keep working; only a missing path is a recovery case. + it("stays silent for a directory that is simply not a repository", () => { + expect( + selectCheckoutRecovery({ + cwd: WORKTREE, + projectCwd: PROJECT, + branch: null, + status: emptyDirStatus, + }), + ).toBeNull(); + }); + + it("stays silent while the status is unknown", () => { + expect( + selectCheckoutRecovery({ + cwd: WORKTREE, + projectCwd: PROJECT, + branch: null, + status: null, + }), + ).toBeNull(); + }); + + it("offers nothing to switch to when the project root itself is the missing path", () => { + const recovery = selectCheckoutRecovery({ + cwd: PROJECT, + projectCwd: PROJECT, + branch: "main", + status: missingStatus, + branchExists: true, + }); + expect(recovery?.canSwitchToProjectRoot).toBe(false); + expect(recovery?.canRecreateWorktree).toBe(false); + }); +}); + +describe("shouldShowCheckoutPicker", () => { + // The regression this exists for: hiding the picker for a deleted checkout + // removed the only control that could move the thread back to the project + // root, turning one broken thread into a bricked project. + it("keeps the picker visible for a missing checkout when the project root is a repository", () => { + expect( + shouldShowCheckoutPicker({ + selectedStatus: missingStatus, + projectRootStatus: healthyStatus, + }), + ).toBe(true); + }); + + it("hides the picker when the project root is not a repository either", () => { + expect( + shouldShowCheckoutPicker({ + selectedStatus: missingStatus, + projectRootStatus: emptyDirStatus, + }), + ).toBe(false); + }); + + it("hides the picker for a directory that exists but is not a repository", () => { + expect( + shouldShowCheckoutPicker({ + selectedStatus: emptyDirStatus, + projectRootStatus: healthyStatus, + }), + ).toBe(false); + }); + + it("stays visible while the status is still loading", () => { + expect( + shouldShowCheckoutPicker({ selectedStatus: undefined, projectRootStatus: undefined }), + ).toBe(true); + }); +}); + +describe("seedDraftWorktreePath", () => { + // Without this, a draft started from a thread whose worktree was deleted + // inherits the dead path and is born broken. + it("drops an inherited checkout that is known to be missing", () => { + expect( + seedDraftWorktreePath({ + requested: undefined, + inherited: WORKTREE, + projectChanged: false, + isCheckoutMissing: (cwd) => cwd === WORKTREE, + }), + ).toBeNull(); + }); + + it("inherits a checkout that is still there", () => { + expect( + seedDraftWorktreePath({ + requested: undefined, + inherited: WORKTREE, + projectChanged: false, + isCheckoutMissing: () => false, + }), + ).toBe(WORKTREE); + }); + + it("passes an explicitly chosen path through even when it reads as missing", () => { + expect( + seedDraftWorktreePath({ + requested: WORKTREE, + inherited: null, + projectChanged: false, + isCheckoutMissing: () => true, + }), + ).toBe(WORKTREE); + }); + + it("drops the inherited checkout on a project change", () => { + expect( + seedDraftWorktreePath({ + requested: undefined, + inherited: WORKTREE, + projectChanged: true, + isCheckoutMissing: () => false, + }), + ).toBeNull(); + }); +}); diff --git a/apps/web/src/lib/checkoutRecovery.ts b/apps/web/src/lib/checkoutRecovery.ts new file mode 100644 index 000000000..93bd17208 --- /dev/null +++ b/apps/web/src/lib/checkoutRecovery.ts @@ -0,0 +1,160 @@ +/** + * What the UI offers when a thread's folder is gone. + * + * A thread pinned to a git worktree stops working the moment that folder is + * deleted — by the agent tidying up after merging its own branch, or by the + * user in a terminal. Before this existed the failure surfaced as a provider + * error blaming the CLI, with only a Retry that failed the same way, and the + * source control panel offered to run `git init` on a path that wasn't there. + * + * Every surface that has to react (the composer notice, the source control + * panel, the checkout picker) derives its state from here so they cannot + * disagree about whether a checkout is missing or what can be done about it. + * + * The signal is the server's `pathMissing` flag on the VCS status, which is + * deliberately distinct from `isRepo: false` ("the folder is there but holds no + * repository"). Only the second is an invitation to initialize a repository. + * + * @module checkoutRecovery + */ + +/** The subset of a VCS status this module reads. */ +export interface CheckoutStatusLike { + readonly isRepo: boolean; + readonly pathMissing?: boolean | undefined; +} + +export interface CheckoutRecoveryState { + /** Absolute path of the folder that is gone. */ + readonly cwd: string; + /** Folder name, for compact display where the full path won't fit. */ + readonly label: string; + /** Project root the thread can move to, when there is one to move to. */ + readonly projectCwd: string | null; + /** Branch the missing checkout held, when the thread recorded one. */ + readonly branch: string | null; + /** Moving the thread to the project root is possible. */ + readonly canSwitchToProjectRoot: boolean; + /** + * The folder can be recreated exactly as it was. Requires the branch to still + * exist in the repository: without it there is nothing to check out, and the + * only way forward is the project root. + */ + readonly canRecreateWorktree: boolean; +} + +/** Trailing folder name of a path, tolerating either separator. */ +export function checkoutFolderLabel(cwd: string): string { + const trimmed = cwd.replace(/[/\\]+$/u, ""); + const segments = trimmed.split(/[/\\]/u); + return segments.at(-1) || trimmed; +} + +function samePath(left: string | null | undefined, right: string | null | undefined): boolean { + if (!left || !right) { + return false; + } + const normalize = (value: string) => value.replaceAll("\\", "/").replace(/\/+$/u, ""); + return normalize(left) === normalize(right); +} + +/** + * The recovery state for a thread's checkout, or null when nothing is wrong. + * + * Returns null while the status is still loading or absent: a checkout is only + * treated as missing on a positive answer from the server, never on the absence + * of one. That keeps a slow or disconnected status from spuriously telling the + * user their folder was deleted. + */ +export function selectCheckoutRecovery(input: { + readonly cwd: string | null | undefined; + readonly projectCwd: string | null | undefined; + readonly branch: string | null | undefined; + readonly status: CheckoutStatusLike | null | undefined; + /** + * Whether {@link input.branch} still exists in the project repository. + * Undefined means "not known yet", which withholds the recreate action rather + * than offering one that would fail. + */ + readonly branchExists?: boolean | undefined; +}): CheckoutRecoveryState | null { + const cwd = input.cwd?.trim(); + if (!cwd || input.status?.pathMissing !== true) { + return null; + } + const projectCwd = input.projectCwd?.trim() || null; + const branch = input.branch?.trim() || null; + // When the project root *is* the missing folder there is nowhere to fall back + // to and nothing to recreate the worktree from; the problem is stated plainly + // and both actions stay off. + const hasSeparateProjectRoot = projectCwd !== null && !samePath(projectCwd, cwd); + + return { + cwd, + label: checkoutFolderLabel(cwd), + projectCwd, + branch, + canSwitchToProjectRoot: hasSeparateProjectRoot, + canRecreateWorktree: hasSeparateProjectRoot && branch !== null && input.branchExists === true, + }; +} + +/** + * Whether the checkout picker must stay on screen. + * + * The picker used to be hidden whenever the selected checkout had no usable git + * status, which is exactly the situation where the user most needs it: with the + * worktree deleted, the picker was the only way back to the project root and it + * had disappeared. It now survives an invalid selection as long as the project + * root is a repository, so there is always a way out. + */ +export function shouldShowCheckoutPicker(input: { + readonly selectedStatus: CheckoutStatusLike | null | undefined; + readonly projectRootStatus: CheckoutStatusLike | null | undefined; +}): boolean { + // Default to visible while a status is pending so the toolbar does not flicker. + if (!input.selectedStatus) { + return true; + } + if (input.selectedStatus.isRepo) { + return true; + } + if (input.selectedStatus.pathMissing === true) { + return input.projectRootStatus ? input.projectRootStatus.isRepo : true; + } + return false; +} + +/** + * The checkout a new draft should start in. + * + * A draft inherits its checkout from the thread the user came from, which is + * how one broken thread used to spread: start a new thread from a thread whose + * worktree was deleted and the new one is born pointing at the same dead path, + * with the same broken source control and the same missing picker. A checkout + * already known to be gone is therefore not inherited — the draft falls back to + * the project root, where it will work. + * + * Only inheritance is filtered. An explicitly requested path is the user's + * choice and is passed through untouched. + */ +export function seedDraftWorktreePath(input: { + /** Explicitly requested path; `undefined` means "inherit". */ + readonly requested: string | null | undefined; + readonly inherited: string | null; + readonly projectChanged: boolean; + /** Known-missing test, from whatever status the client already holds. */ + readonly isCheckoutMissing: (cwd: string) => boolean; +}): string | null { + if (input.requested !== undefined) { + return input.requested ?? null; + } + if (input.projectChanged) { + return null; + } + const inherited = input.inherited; + if (inherited === null) { + return null; + } + return input.isCheckoutMissing(inherited) ? null : inherited; +} diff --git a/packages/contracts/src/git.ts b/packages/contracts/src/git.ts index 74c04cf68..084cc339d 100644 --- a/packages/contracts/src/git.ts +++ b/packages/contracts/src/git.ts @@ -422,6 +422,13 @@ const VcsStatusChangeRequest = Schema.Struct({ const VcsStatusLocalShape = { isRepo: Schema.Boolean, + /** + * The directory the status was asked for no longer exists. Distinct from + * `isRepo: false`, which means the directory is there but holds no + * repository: only the latter is an invitation to run `git init`. Absent on + * statuses from servers that predate the distinction. + */ + pathMissing: Schema.optional(Schema.Boolean), /** Resolved repository root used for status and source-control actions. */ repositoryRoot: Schema.optional(TrimmedNonEmptyStringSchema), /** Whether the opened working directory is the repository root or a nested path within it. */ @@ -698,6 +705,68 @@ export class GitCommandError extends Schema.TaggedErrorClass()( } } +/** + * A thread's checkout directory is gone. + * + * Raised by the pre-flight that runs before any provider session starts or + * resumes, so a session is never spawned into a directory that no longer + * exists. Kept distinct from the provider process errors it used to masquerade + * as: spawning into a missing cwd fails with the same `ENOENT` a missing + * executable does, and the provider SDKs report that as a missing binary, which + * sends the user looking for the wrong problem. + */ +export class CheckoutMissingError extends Schema.TaggedErrorClass()( + "CheckoutMissingError", + { + threadId: TrimmedNonEmptyStringSchema, + /** Absolute path that no longer exists. */ + cwd: TrimmedNonEmptyStringSchema, + /** Branch the checkout held, when known; decides whether it can be recreated. */ + branch: Schema.optional(Schema.NullOr(TrimmedNonEmptyStringSchema)), + /** Project root the thread can fall back to, when known. */ + projectCwd: Schema.optional(Schema.NullOr(TrimmedNonEmptyStringSchema)), + cause: Schema.optional(Schema.Defect()), + }, +) { + override get message(): string { + return `This thread's folder no longer exists: ${this.cwd}`; + } +} + +/** + * A worktree removal was refused because something still runs in it. + * + * Removing the folder out from under a live session is what bricks a thread, so + * the app declines instead of doing it and names who is still there. There is + * deliberately no force flag: the caller stops or moves those threads first, + * which the existing UI already allows. + */ +export class VcsWorktreeInUseError extends Schema.TaggedErrorClass()( + "VcsWorktreeInUseError", + { + /** Absolute path of the worktree that was not removed. */ + worktreePath: TrimmedNonEmptyStringSchema, + /** Threads still bound to that path, newest-known title first. */ + blockingThreads: Schema.Array( + Schema.Struct({ + threadId: TrimmedNonEmptyStringSchema, + title: Schema.NullOr(Schema.String), + /** True when the thread also has a provider session running there. */ + hasLiveSession: Schema.Boolean, + }), + ), + }, +) { + override get message(): string { + const names = this.blockingThreads + .map((thread) => thread.title?.trim() || thread.threadId) + .join(", "); + return names.length > 0 + ? `${this.worktreePath} is still used by ${names}.` + : `${this.worktreePath} is still in use.`; + } +} + export class TextGenerationError extends Schema.TaggedErrorClass()( "TextGenerationError", { diff --git a/packages/contracts/src/orchestration.ts b/packages/contracts/src/orchestration.ts index 480203c0b..f45ee6b06 100644 --- a/packages/contracts/src/orchestration.ts +++ b/packages/contracts/src/orchestration.ts @@ -474,6 +474,31 @@ export const ThreadCheckoutSwitchDeferredPayload = Schema.Struct({ }); export type ThreadCheckoutSwitchDeferredPayload = typeof ThreadCheckoutSwitchDeferredPayload.Type; +/** + * Payload of the `thread.checkout.missing` activity: the folder this thread + * works in is gone. + * + * Emitted from two directions — the pre-flight that refuses to start a provider + * session in a directory that no longer exists, and the status watcher that + * notices an out-of-band deletion between turns. Both carry the same payload so + * the recovery affordance in the thread view reads one shape, and so the + * watcher can surface it without waiting for a turn to fail. + * + * `branch` decides whether recreating the worktree is offered at all: with the + * branch still in the repository the folder can be recreated exactly as it was; + * without it the only way out is moving the thread to the project root. + */ +export const ThreadCheckoutMissingActivityKind = "thread.checkout.missing"; +export const ThreadCheckoutMissingPayload = Schema.Struct({ + /** Absolute path of the checkout that is no longer on disk. */ + cwd: TrimmedNonEmptyString, + /** Branch the missing checkout held, when the thread recorded one. */ + branch: Schema.optional(Schema.NullOr(TrimmedNonEmptyString)), + /** Project root to fall back to. Absent when the project could not be resolved. */ + projectCwd: Schema.optional(Schema.NullOr(TrimmedNonEmptyString)), +}); +export type ThreadCheckoutMissingPayload = typeof ThreadCheckoutMissingPayload.Type; + export const OrchestrationProposedPlanId = TrimmedNonEmptyString; export type OrchestrationProposedPlanId = typeof OrchestrationProposedPlanId.Type; diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index 6eacb1fe7..bf4644d65 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -34,6 +34,7 @@ import { VcsSwitchRefInput, VcsSwitchRefResult, GitCommandError, + VcsWorktreeInUseError, GitGenerateCommitMessageInput, GitGenerateCommitMessageResult, VcsCreateRefInput, @@ -923,7 +924,9 @@ export const WsVcsCreateWorktreeRpc = Rpc.make(WS_METHODS.vcsCreateWorktree, { export const WsVcsRemoveWorktreeRpc = Rpc.make(WS_METHODS.vcsRemoveWorktree, { payload: VcsRemoveWorktreeInput, - error: GitCommandError, + // Removal is refused outright while a thread still works in the folder, so + // callers get a distinct error naming them rather than a git failure. + error: Schema.Union([GitCommandError, VcsWorktreeInUseError]), }); export const WsVcsCreateRefRpc = Rpc.make(WS_METHODS.vcsCreateRef, { diff --git a/packages/shared/src/contextSeed.ts b/packages/shared/src/contextSeed.ts index cc3148332..cf70a703a 100644 --- a/packages/shared/src/contextSeed.ts +++ b/packages/shared/src/contextSeed.ts @@ -19,6 +19,22 @@ import type { ThreadContextSeedEntry, } from "@threadlines/contracts"; +/** + * Told to every session whose working directory is a git worktree Threadlines + * created. + * + * Agents routinely tidy up after themselves once a branch is merged, and + * `git worktree remove` looks like part of that tidying. It is not: the folder + * is the session's own working directory and the thread it belongs to, so + * deleting it strands the conversation with nowhere to run. Threadlines owns + * that lifecycle and removes worktrees on the user's behalf. + * + * One sentence on purpose. It is prepended to every provider's instructions, + * where every extra line competes with the user's actual task. + */ +export const MANAGED_WORKTREE_INSTRUCTION = + "Your working directory is a git worktree that Threadlines created and manages; never remove or prune it, including as post-merge cleanup."; + const ROLE_LABEL: Record = { user: "User", assistant: "Assistant", diff --git a/packages/shared/src/git.test.ts b/packages/shared/src/git.test.ts index a42c3761b..40966028d 100644 --- a/packages/shared/src/git.test.ts +++ b/packages/shared/src/git.test.ts @@ -357,4 +357,29 @@ describe("applyGitStatusStreamEvent", () => { pr: null, }); }); + + // A deleted checkout's status must not flip back to a plain "not a repo" the + // first time a remote update rebuilds the local part — that collapses the + // whole missing-checkout recovery UI moments after it appears. + it("keeps pathMissing across a remote update", () => { + const current: VcsStatusResult = { + isRepo: false, + pathMissing: true, + hasPrimaryRemote: false, + isDefaultRef: false, + refName: null, + hasWorkingTreeChanges: false, + workingTree: { files: [], insertions: 0, deletions: 0 }, + hasUpstream: false, + aheadCount: 0, + behindCount: 0, + pr: null, + }; + + const updated = applyGitStatusStreamEvent(current, { + _tag: "remoteUpdated", + remote: { hasUpstream: false, aheadCount: 0, behindCount: 0, pr: null }, + }); + expect(updated.pathMissing).toBe(true); + }); }); diff --git a/packages/shared/src/git.ts b/packages/shared/src/git.ts index 1584c11d9..561592fce 100644 --- a/packages/shared/src/git.ts +++ b/packages/shared/src/git.ts @@ -533,6 +533,9 @@ function toRemoteStatusPart(status: VcsStatusResult): VcsStatusRemoteResult { function toLocalStatusPart(status: VcsStatusResult): VcsStatusLocalResult { return { isRepo: status.isRepo, + // Dropping this on a remote update would flip a deleted checkout back to a + // plain "not a repo", collapsing the recovery UI moments after it appeared. + ...(status.pathMissing === undefined ? {} : { pathMissing: status.pathMissing }), ...(status.repositoryRoot === undefined ? {} : { repositoryRoot: status.repositoryRoot }), ...(status.repositoryRootRelation === undefined ? {}