Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions apps/server/src/git/GitWorkflowService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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),
),
),
),
);
});
28 changes: 28 additions & 0 deletions apps/server/src/git/GitWorkflowService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ export class GitWorkflowService extends Context.Service<
readonly cwd: string;
readonly refName: string;
}) => Effect.Effect<boolean, GitCommandError>;
/**
* 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<boolean, GitCommandError>;
readonly status: (
input: VcsStatusInput,
) => Effect.Effect<VcsStatusResult, GitManagerServiceError>;
Expand Down Expand Up @@ -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) =>
Expand Down
46 changes: 46 additions & 0 deletions apps/server/src/mcp/toolkits/project/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ const clientLaunchHarness = (input: {
readonly runtimeModeCeiling: "approval-required" | "auto-accept-edits" | "auto" | "full-access";
readonly launched: Array<ThreadLaunch.ThreadLaunchInput>;
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" };
Expand All @@ -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({
Expand Down Expand Up @@ -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<ThreadLaunch.ThreadLaunchInput> = [];
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);
}),
);
21 changes: 21 additions & 0 deletions apps/server/src/mcp/toolkits/project/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 ??
Expand Down
105 changes: 105 additions & 0 deletions apps/server/src/orchestration-v2/ThreadLaunchService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ interface HarnessOptions {
readonly generateBranchName?: TextGeneration.TextGeneration["Service"]["generateBranchName"];
readonly serverSettings?: Parameters<typeof ServerSettings.layerTest>[0];
readonly providers?: ReadonlyArray<ServerProvider>;
readonly git?: Partial<GitWorkflow.GitWorkflowService["Service"]>;
}

function makeHarness(options: HarnessOptions = {}) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -2024,6 +2026,7 @@ it.effect("shared intake preserves durable attachment bytes after a lost launch
}),
),
),
checkWorktreeBase: launches.checkWorktreeBase,
retryPreparation: launches.retryPreparation,
}),
Effect.flip,
Expand Down Expand Up @@ -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));
}),
);
Loading
Loading