diff --git a/apps/server/src/git/GitWorkflowService.test.ts b/apps/server/src/git/GitWorkflowService.test.ts index 477bf2d399810de0a279f76cb191fa67a95c62f6..4617965803b9c35d7e73b067df7704d871427f16 100644 --- a/apps/server/src/git/GitWorkflowService.test.ts +++ b/apps/server/src/git/GitWorkflowService.test.ts @@ -6,6 +6,11 @@ import * as Option from "effect/Option"; import { VcsRepositoryDetectionError } from "@t3tools/contracts"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import * as FileSystem from "effect/FileSystem"; + +import * as ServerConfig from "../config.ts"; +import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as GitManager from "./GitManager.ts"; import * as GitWorkflowService from "./GitWorkflowService.ts"; import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; @@ -220,4 +225,47 @@ describe("GitWorkflowService", () => { ), ); }); + + it.effect("finds a name among the refs of every namespace", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const cwd = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-ref-named-" }); + const workflow = yield* GitWorkflowService.GitWorkflowService; + const driver = yield* GitVcsDriver.GitVcsDriver; + for (const args of [ + ["init", "-b", "main"], + ["-c", "user.email=t@t", "-c", "user.name=t", "commit", "--allow-empty", "-m", "init"], + ["update-ref", "refs/remotes/upstream/feature/topic", "HEAD"], + ["symbolic-ref", "refs/remotes/origin/alias", "refs/heads/main"], + ["update-ref", "refs/mirror/origin/mirrored", "HEAD"], + ]) { + yield* driver.execute({ operation: "test.setupRepo", cwd, args }); + } + const found = yield* Effect.forEach( + ["main", "feature/topic", "alias", "mirrored", "t3/renamed", "feature/*"], + (refName) => workflow.hasRefNamed({ cwd, refName }), + ); + assert.deepEqual(found, [true, true, true, true, false, false]); + }).pipe( + Effect.scoped, + Effect.provide( + GitWorkflowService.layer.pipe( + Layer.provide( + Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({ + resolve: () => Effect.succeed({ kind: "git" } as VcsDriverRegistry.VcsDriverHandle), + }), + ), + Layer.provide(Layer.mock(GitManager.GitManager)({})), + Layer.provideMerge(GitVcsDriver.layer), + Layer.provideMerge(VcsProcess.layer), + Layer.provideMerge( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-ref-named-" }).pipe( + Layer.provide(NodeServices.layer), + ), + ), + Layer.provideMerge(NodeServices.layer), + ), + ), + ), + ); }); diff --git a/apps/server/src/git/GitWorkflowService.ts b/apps/server/src/git/GitWorkflowService.ts index bc7f2d9fb2ea3fafce2fb969909075b9537a2b7d..f7eddd7f3c3fc51f4706b930df69830adf0a5e54 100644 --- a/apps/server/src/git/GitWorkflowService.ts +++ b/apps/server/src/git/GitWorkflowService.ts @@ -40,6 +40,15 @@ export class GitWorkflowService extends Context.Service< readonly cwd: string; readonly refName: string; }) => Effect.Effect; + /** + * Whether a ref in any namespace ends in this name: a local branch, a + * remote-tracking branch of any remote, a tag. Git falls back to these + * when a branch name is not a revision by itself. + */ + readonly hasRefNamed: (input: { + readonly cwd: string; + readonly refName: string; + }) => Effect.Effect; readonly status: ( input: VcsStatusInput, ) => Effect.Effect; @@ -300,6 +309,25 @@ export const make = Effect.gen(function* () { ), Effect.map((result) => result.exitCode === 0), ), + hasRefNamed: (input) => + // No ref name holds a pattern character, and one here would widen the match. + /[*?[\\]/.test(input.refName) + ? Effect.succeed(false) + : ensureGitCommand("GitWorkflowService.hasRefNamed", input.cwd).pipe( + Effect.andThen( + git.execute({ + operation: "GitWorkflowService.hasRefNamed", + cwd: input.cwd, + args: [ + "for-each-ref", + "--count=1", + "--format=%(refname)", + `refs/**/${input.refName}`, + ], + }), + ), + Effect.map((result) => result.stdout.trim().length > 0), + ), status: (input) => detectGitRepositoryForStatus("GitWorkflowService.status", input.cwd).pipe( Effect.flatMap((isGitRepository) => diff --git a/apps/server/src/mcp/toolkits/project/handlers.test.ts b/apps/server/src/mcp/toolkits/project/handlers.test.ts index d657938ff20674647a5ae636b9abf023b732a5de..b74df9dae1f996e68d56d652103d99f97f3612cb 100644 --- a/apps/server/src/mcp/toolkits/project/handlers.test.ts +++ b/apps/server/src/mcp/toolkits/project/handlers.test.ts @@ -301,6 +301,7 @@ const clientLaunchHarness = (input: { readonly runtimeModeCeiling: "approval-required" | "auto-accept-edits" | "auto" | "full-access"; readonly launched: Array; readonly workspaceRoot?: string; + readonly checkWorktreeBase?: ThreadLaunch.ThreadLaunchService["Service"]["checkWorktreeBase"]; }) => { const projectId = ProjectId.make("project:client-target"); const modelSelection = { instanceId: ProviderInstanceId.make("claude"), model: "claude-opus" }; @@ -320,6 +321,7 @@ const clientLaunchHarness = (input: { }), Layer.mock(ThreadManagement.ThreadManagementService)({}), Layer.mock(ThreadLaunch.ThreadLaunchService)({ + checkWorktreeBase: input.checkWorktreeBase ?? (() => Effect.void), launch: (launch) => { input.launched.push(launch); return Effect.succeed({ @@ -454,3 +456,47 @@ it.effect("a launch binds only an existing checkout that is one of the project's }); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); + +it.effect("a launch into a new worktree creates nothing when its base ref has no commit", () => + Effect.gen(function* () { + const launched: Array = []; + const { projectId, dependencies } = clientLaunchHarness({ + runtimeModeCeiling: "auto", + launched, + checkWorktreeBase: ({ projectId, workspaceStrategy }) => + workspaceStrategy.type === "worktree" && workspaceStrategy.baseRef === "t3/renamed" + ? Effect.fail( + new ThreadLaunch.ThreadLaunchBaseRefError({ + projectId, + projectTitle: "Client target", + workspaceRoot: "/projects/client-target", + baseRef: workspaceStrategy.baseRef, + }), + ) + : Effect.void, + }); + const toolkit = yield* ProjectToolkit.pipe( + Effect.provide( + McpToolAccess.HandlersLayer.layer(ProjectHandlers.layer).pipe(Layer.provide(dependencies)), + ), + ); + const launchFrom = (baseRef: string) => + toolkit + .handle("t3_thread_launch", { + title: "Fix", + projectId, + workspaceStrategy: { type: "worktree", baseRef, startFromOrigin: false }, + }) + .pipe(Stream.unwrap, Stream.runCollect, Effect.provide(dependencies)); + + const refused = (yield* launchFrom("t3/renamed")).at(-1)?.result; + expect(refused).toMatchObject({ code: "invalid_request" }); + expect((refused as { message: string }).message).toContain( + 'Base ref "t3/renamed" does not resolve to a commit in project "Client target" (/projects/client-target). No thread was created.', + ); + expect(launched).toHaveLength(0); + + expect((yield* launchFrom("main")).at(-1)?.result).toMatchObject({ projectId }); + expect(launched).toHaveLength(1); + }), +); diff --git a/apps/server/src/mcp/toolkits/project/handlers.ts b/apps/server/src/mcp/toolkits/project/handlers.ts index 9a7d7024111ef2a864de00ad8a91f633a13b9d16..edb732e400bc5ba45ee7cd00d773edf1cd5f379e 100644 --- a/apps/server/src/mcp/toolkits/project/handlers.ts +++ b/apps/server/src/mcp/toolkits/project/handlers.ts @@ -2,6 +2,7 @@ import { MessageId, ThreadId, OrchestratorMcpFailure, ProjectId } from "@t3tools import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; +import * as ThreadLaunch from "../../../orchestration-v2/ThreadLaunchService.ts"; import * as ThreadMessageIntake from "../../../orchestration-v2/ThreadMessageIntake.ts"; import * as Claims from "../../../orchestration-v2/AttachmentClaims.ts"; import * as Project from "../../../project/ProjectService.ts"; @@ -103,6 +104,26 @@ export const layer = McpToolAccess.toLayer(ProjectToolkit, { }); yield* assertProjectWorktree(project.workspaceRoot, input.workspaceStrategy.worktreePath); } + // A launch is accepted before its worktree is provisioned, so a base + // ref that cannot resolve is refused while the agent can still act on it. + const strategy = input.workspaceStrategy; + if (strategy?.type === "worktree") + yield* ThreadLaunch.ThreadLaunchService.pipe( + Effect.flatMap((launches) => + launches.checkWorktreeBase({ projectId, workspaceStrategy: strategy }), + ), + Effect.mapError( + (error) => + new OrchestratorMcpFailure({ + code: "invalid_request", + message: `${error.message} No thread was created. Pass a branch, tag, or commit that exists there${ + strategy.startFromOrigin === true + ? "; the project has no origin remote to fetch it from." + : ", or set startFromOrigin:true if it exists only on origin." + }`, + }), + ), + ); const modelSelection = input.modelSelection ?? caller?.modelSelection ?? diff --git a/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts b/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts index dfa02f4f514bf254f48917161d2e1e62d808078c..90248b0e1f6b85bed7921283bac2729cf1b491af 100644 --- a/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts +++ b/apps/server/src/orchestration-v2/ThreadLaunchService.test.ts @@ -107,6 +107,7 @@ interface HarnessOptions { readonly generateBranchName?: TextGeneration.TextGeneration["Service"]["generateBranchName"]; readonly serverSettings?: Parameters[0]; readonly providers?: ReadonlyArray; + readonly git?: Partial; } function makeHarness(options: HarnessOptions = {}) { @@ -175,6 +176,7 @@ function makeHarness(options: HarnessOptions = {}) { removeWorktree, resolveRemoteTrackingCommit: () => Effect.succeed({ commitSha: "remote-main-sha", remoteRefName: "origin/main" }), + ...options.git, }), Layer.succeed(ProjectSetupScriptRunner.ProjectSetupScriptRunner, { runForThread: runSetup, @@ -2024,6 +2026,7 @@ it.effect("shared intake preserves durable attachment bytes after a lost launch }), ), ), + checkWorktreeBase: launches.checkWorktreeBase, retryPreparation: launches.retryPreparation, }), Effect.flip, @@ -2269,3 +2272,105 @@ it.effect.each([0, 1])("releases an async setup before its completion with exit }).pipe(Effect.provide(harness.layer)); }), ); + +it.effect.each([ + { name: "is missing", baseRef: "t3/renamed", startFromOrigin: false, origin: true }, + { + name: "has no origin to come from", + baseRef: "t3/renamed", + startFromOrigin: true, + origin: false, + }, + { + name: "is a previous checkout there never was", + baseRef: "-", + startFromOrigin: false, + origin: true, + }, +])("refuses a new worktree whose base ref $name", (testCase) => + Effect.gen(function* () { + const harness = makeHarness({ + hasCommit: ({ refName }) => Effect.succeed(refName === "HEAD"), + git: { + remoteExists: () => Effect.succeed(testCase.origin), + hasRefNamed: () => Effect.succeed(false), + }, + }); + yield* Effect.gen(function* () { + const launches = yield* ThreadLaunch.ThreadLaunchService; + const error = yield* launches + .checkWorktreeBase({ + projectId, + workspaceStrategy: { + type: "worktree", + baseRef: testCase.baseRef, + startFromOrigin: testCase.startFromOrigin, + }, + }) + .pipe(Effect.flip); + assert.deepInclude(error, { + _tag: "ThreadLaunchBaseRefError", + projectId, + baseRef: testCase.baseRef, + }); + assert.equal( + error.message, + `Base ref "${testCase.baseRef}" does not resolve to a commit in project "Project" (/repo).`, + ); + }).pipe(Effect.provide(harness.layer)); + }), +); + +it.effect.each([ + { name: "a base ref that resolves", baseRef: "main", commits: ["main", "HEAD"] }, + // Provisioning fetches it; the ref may exist nowhere else yet. + { + name: "a base ref only origin has", + baseRef: "remote-only", + startFromOrigin: true, + commits: ["HEAD"], + }, + // `git worktree add` starts from a remote-tracking branch of that name. + { + name: "a base ref a remote branch carries", + baseRef: "remote-only", + commits: ["HEAD"], + remoteBranch: true, + }, + // Provisioning runs such a repository without a worktree. + { name: "a repository with no commits yet", baseRef: "main", commits: [] }, + { name: "the previous checkout", baseRef: "-", commits: ["@{-1}", "HEAD"] }, + { name: "a commit search", baseRef: ":/fix", commits: ["HEAD"] }, + { name: "a folder that is not a repository", baseRef: "main", commits: null }, +])("leaves $name to the launch", (testCase) => + Effect.gen(function* () { + const { commits } = testCase; + const harness = makeHarness({ + git: { hasRefNamed: () => Effect.succeed(testCase.remoteBranch === true) }, + hasCommit: ({ refName }) => + commits === null + ? Effect.fail( + new GitCommandError({ + operation: "GitWorkflowService.hasCommit", + command: "git rev-parse", + cwd: "/repo", + detail: "Not a Git repository.", + }), + ) + : Effect.succeed(commits.includes(refName)), + }); + yield* Effect.gen(function* () { + const launches = yield* ThreadLaunch.ThreadLaunchService; + yield* launches.checkWorktreeBase({ + projectId, + workspaceStrategy: { + type: "worktree", + baseRef: testCase.baseRef, + ...(testCase.startFromOrigin === undefined + ? {} + : { startFromOrigin: testCase.startFromOrigin }), + }, + }); + }).pipe(Effect.provide(harness.layer)); + }), +); diff --git a/apps/server/src/orchestration-v2/ThreadLaunchService.ts b/apps/server/src/orchestration-v2/ThreadLaunchService.ts index 10af1aef454d2d00e6b9f1b4d2de68d70d392aff..d1a432bc9b52fcded66495d5e1c65bc6d5adcd4c 100644 --- a/apps/server/src/orchestration-v2/ThreadLaunchService.ts +++ b/apps/server/src/orchestration-v2/ThreadLaunchService.ts @@ -147,12 +147,35 @@ export class ThreadLaunchError extends Schema.TaggedError()( } } +/** A new worktree was asked to start from a ref that gives it no commit. */ +export class ThreadLaunchBaseRefError extends Schema.TaggedError()( + "ThreadLaunchBaseRefError", + { + projectId: ProjectId, + projectTitle: Schema.String, + workspaceRoot: Schema.String, + baseRef: Schema.String, + }, +) { + override get message(): string { + return `Base ref "${this.baseRef}" does not resolve to a commit in project "${this.projectTitle}" (${this.workspaceRoot}).`; + } +} + export class ThreadLaunchService extends Context.Service< ThreadLaunchService, { readonly launch: ( input: ThreadLaunchInput, ) => Effect.Effect; + /** + * Fails when a new worktree's base ref cannot give provisioning a commit. + * `launch` accepts such a launch and reports the failure inside the + * thread; a caller that needs it before a thread exists checks first. + */ + readonly checkWorktreeBase: ( + input: Pick, + ) => Effect.Effect; /** Dispatches prepared-run.retry and prepares the run's workspace again. */ readonly retryPreparation: ( input: ThreadLaunchRetryInput, @@ -229,6 +252,43 @@ const make = Effect.gen(function* () { } }); + const checkWorktreeBase: ThreadLaunchService["Service"]["checkWorktreeBase"] = Effect.fn( + "ThreadLaunchService.checkWorktreeBase", + )(function* (input) { + const strategy = input.workspaceStrategy; + // `:/text` is a commit search, which cannot be asked for a commit type. + if (strategy.type !== "worktree" || strategy.baseRef.startsWith(":")) return; + const { baseRef } = strategy; + // Anything this cannot establish is left for the launch to report: a + // missing project, a folder that is not a repository, a failing Git. + const unresolved = yield* Effect.gen(function* () { + const project = Option.getOrUndefined(yield* projects.getById(input.projectId)); + if (project === undefined) return undefined; + const cwd = project.workspaceRoot; + // `git worktree add` reads `-` as the previous checkout. + if (yield* git.hasCommit({ cwd, refName: baseRef === "-" ? "@{-1}" : baseRef })) + return undefined; + // Provisioning fetches the base from origin when it can, and runs a + // repository with no commits yet without a worktree. + if ( + strategy.startFromOrigin === true && + (yield* git.remoteExists({ cwd, remoteName: "origin" })) + ) + return undefined; + // `git worktree add` also starts from a remote-tracking branch of that + // name; any ref carrying it is reason enough to let Git decide. + if (yield* git.hasRefNamed({ cwd, refName: baseRef })) return undefined; + return (yield* git.hasCommit({ cwd, refName: "HEAD" })) ? project : undefined; + }).pipe(Effect.orElseSucceed(() => undefined)); + if (unresolved !== undefined) + return yield* new ThreadLaunchBaseRefError({ + projectId: unresolved.id, + projectTitle: unresolved.title, + workspaceRoot: unresolved.workspaceRoot, + baseRef, + }); + }); + const prepareInBackground = Effect.fn("ThreadLaunchService.prepareInBackground")(function* ( input: PreparationInput, threadId: ThreadId, @@ -977,7 +1037,7 @@ const make = Effect.gen(function* () { ); }; - return ThreadLaunchService.of({ launch, retryPreparation }); + return ThreadLaunchService.of({ launch, checkWorktreeBase, retryPreparation }); }); export const layer = Layer.effect(ThreadLaunchService, make);