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
2 changes: 2 additions & 0 deletions apps/server/src/mcp/McpHttpServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
} from "effect/http";

import * as ProjectService from "../project/ProjectService.ts";
import * as PullRequestReviewGuideService from "../pullRequest/PullRequestReviewGuideService.ts";
import * as ServerConfig from "../config.ts";
import * as McpHttpServer from "./McpHttpServer.ts";
import * as McpToolAccess from "./McpToolAccess.ts";
Expand Down Expand Up @@ -77,6 +78,7 @@ const layerPullRequestsTest = McpHttpServer.layerPullRequestsToolkit.pipe(
Layer.provide(
Layer.mergeAll(
Layer.mock(ProjectService.ProjectService)({}),
Layer.mock(PullRequestReviewGuideService.PullRequestReviewGuideService)({}),
Layer.mock(Orchestrator.OrchestratorV2)({}),
Layer.mock(ProjectionStore.ProjectionStoreV2)({}),
NodeServices.layer,
Expand Down
8 changes: 8 additions & 0 deletions apps/server/src/mcp/McpToolAccess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ const ProbeToolkit = Toolkit.make(
Tool.make("reads_as_caller", probe),
Tool.make("acts_as_caller", probe),
Tool.make("writes", probe),
Tool.make("writes_with_capability", probe),
Tool.make("writes_threads", {
...probe,
parameters: Schema.Struct({ threadId: Schema.optional(ThreadId) }),
Expand All @@ -106,6 +107,7 @@ const probeHandlers: McpToolAccess.Handlers<typeof ProbeToolkit.tools> = {
reads_as_caller: McpToolAccess.readsAsCaller(() => ran),
acts_as_caller: McpToolAccess.actsAsCaller(() => ran),
writes: McpToolAccess.writes(() => ran),
writes_with_capability: McpToolAccess.writesWithCapability(() => ran),
writes_threads: McpToolAccess.writesThreads(
(input) => [input.threadId],
() => ran,
Expand Down Expand Up @@ -152,6 +154,7 @@ export const refusedAtCompileTime = () => {
reads_as_caller: unchecked,
acts_as_caller: unchecked,
writes: unchecked,
writes_with_capability: unchecked,
writes_threads: unchecked,
starts_threads: unchecked,
writes_environment: unchecked,
Expand Down Expand Up @@ -224,6 +227,11 @@ it.effect.each([
["writes", previewOnly, {}, "capability_denied"],
["writes_environment", previewOnly, {}, "capability_denied"],

// A tool's own capability stands in for thread control; the rest of a change still applies.
["writes_with_capability", previewOnly, {}, "ran"],
["writes_with_capability", readOnlyClient, {}, "capability_denied"],
["writes_with_capability", ended, {}, "parent_not_active"],

// Changing a thread: only one that runs within the caller's modes.
["writes_threads", supervised, {}, "ran"],
["writes_threads", supervised, { threadId: planThreadId }, "ran"],
Expand Down
8 changes: 8 additions & 0 deletions apps/server/src/mcp/McpToolAccess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,14 @@ export const actsAsCaller = <P, A, E, R>(handle: (params: P) => Effect.Effect<A,
export const writes = <P, A, E, R>(handle: (params: P) => Effect.Effect<A, E, R>) =>
declare((params: P) => orchestratingCaller.pipe(Effect.flatMap(() => handle(params))));

/**
* Changes something that belongs to no thread under a capability of the tool's own, such as
* writing a pull request's review guide. Unlike `writes` it does not need thread control; the
* handler checks its capability, as the capability toolkits do.
*/
export const writesWithCapability = <P, A, E, R>(handle: (params: P) => Effect.Effect<A, E, R>) =>
declare((params: P) => writingCaller.pipe(Effect.flatMap(() => handle(params))));

/**
* Changes the threads `threads` names. An omitted id is the caller's own
* thread; any other thread must run within the caller's modes, checked here
Expand Down
190 changes: 189 additions & 1 deletion apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import {
ProviderInstanceId,
ThreadId,
type OrchestrationV2ServerCommand as OrchestrationCommand,
PullRequestOperationError,
type OrchestrationProjectShell,
type PullRequestRef,
type PullRequestReviewGuideResult,
type ThreadPullRequestLink,
} from "@t3tools/contracts";
import { describe, expect, it } from "@effect/vitest";
Expand All @@ -22,6 +25,7 @@ import {
v2PullRequestThread,
} from "../../../orchestration-v2/testkit/pullRequestFixtures.ts";
import * as ProjectService from "../../../project/ProjectService.ts";
import * as PullRequestReviewGuideService from "../../../pullRequest/PullRequestReviewGuideService.ts";
import * as McpInvocationContext from "../../McpInvocationContext.ts";
import * as McpToolAccess from "../../McpToolAccess.ts";
import * as McpToolAccessTestkit from "../../McpToolAccess.testkit.ts";
Expand Down Expand Up @@ -135,12 +139,24 @@ interface HarnessOptions {
readonly project?: OrchestrationProjectShell | null;
/** A rejection the orchestrator reports as the dispatch error's cause. */
readonly reject?: (command: OrchestrationCommand) => string | null;
/** Every project in the environment; defaults to the thread's own. */
readonly projects?: ReadonlyArray<OrchestrationProjectShell>;
/** What the guide service answers for a pull request it is asked to write. */
readonly generate?: (
input: PullRequestRef,
) => Effect.Effect<
PullRequestReviewGuideResult & { started: boolean },
PullRequestOperationError
>;
}

const NO_GUIDE: PullRequestReviewGuideResult = { guide: null, run: null, currentHeadSha: "head-1" };

const makeHarness = Effect.fn("makePullRequestsToolkitHarness")(function* (
options: HarnessOptions = {},
) {
const commands = yield* Ref.make<ReadonlyArray<OrchestrationCommand>>([]);
const guideRequests = yield* Ref.make<ReadonlyArray<PullRequestRef>>([]);
const thread = options.thread === undefined ? makeThread([]) : options.thread;
const project = options.project === undefined ? makeProject() : options.project;
const dispatch: Orchestrator.OrchestratorV2Shape["dispatch"] = (command) =>
Expand All @@ -158,6 +174,25 @@ const makeHarness = Effect.fn("makePullRequestsToolkitHarness")(function* (
const layerDependencies = Layer.mergeAll(
Layer.mock(ProjectService.ProjectService)({
getShell: () => Effect.succeed(Option.fromNullishOr(project)),
listShells: () => Effect.succeed(options.projects ?? (project ? [project] : [])),
}),
Layer.mock(PullRequestReviewGuideService.PullRequestReviewGuideService)({
get: () => Effect.succeed(NO_GUIDE),
generate: (input) =>
Ref.update(guideRequests, (recorded) => [...recorded, input]).pipe(
Effect.andThen(
options.generate?.(input) ??
Effect.succeed({
...NO_GUIDE,
run: {
_tag: "Running" as const,
headSha: "head-1",
startedAt: "2026-10-11T00:00:00.000Z",
},
started: true,
}),
),
),
}),
Layer.mock(Orchestrator.OrchestratorV2)({
getThreadShell: (id) =>
Expand Down Expand Up @@ -189,7 +224,7 @@ const makeHarness = Effect.fn("makePullRequestsToolkitHarness")(function* (
Effect.provideService(McpInvocationContext.McpInvocationContext, invocation(capabilities)),
Effect.provide(layerDependencies),
);
return { commands, call };
return { commands, guideRequests, call };
});

describe("pull request toolkit handlers", () => {
Expand Down Expand Up @@ -546,6 +581,159 @@ describe("listThreadPullRequests", () => {
});
});

describe("pull request guide tools", () => {
it.effect(
"writes guides through the project whose checkout is each pull request's repository",
() =>
Effect.gen(function* () {
const other = {
...makeProject({
canonicalKey: "github.com/acme/web",
locator: {
source: "git-remote",
remoteName: "origin",
remoteUrl: "git@github.com:acme/web.git",
},
provider: "github",
displayName: "acme/web",
owner: "acme",
name: "web",
}),
id: ProjectId.make("project-web"),
};
const harness = yield* makeHarness({ projects: [makeProject(), other] });
const result = yield* harness.call("write_pull_request_guides", {
pullRequests: [
{ url: "https://github.com/acme/web/pull/7" },
{ repository: "t3tools/t3code", number: 12 },
{ url: "https://github.com/someone/else/pull/1" },
],
});
expect(
result.guides.map((guide) => [guide.repository, guide.number, guide.outcome]),
).toEqual([
["acme/web", 7, "started"],
["t3tools/t3code", 12, "started"],
["someone/else", 1, "failed"],
]);
expect(result.guides[2]?.detail).toContain("No project in this environment");
expect(
(yield* Ref.get(harness.guideRequests)).map((ref) => [ref.projectId, ref.number]),
).toEqual([
["project-web", 7],
[PROJECT_ID, 12],
]);
}),
);

it.effect("reports a refused run on its own pull request without failing the rest", () =>
Effect.gen(function* () {
const harness = yield* makeHarness({
generate: (input) =>
input.number === 1
? Effect.fail(
new PullRequestOperationError({
operation: "generateReviewGuide",
detail: "Review guides are off. Turn them on in Settings → Source Control.",
}),
)
: Effect.succeed({
...NO_GUIDE,
run: {
_tag: "Running" as const,
headSha: "head-1",
startedAt: "2026-10-11T00:00:00.000Z",
},
started: true,
}),
});
const result = yield* harness.call("write_pull_request_guides", {
pullRequests: [
{ repository: "t3tools/t3code", number: 1 },
{ repository: "t3tools/t3code", number: 2 },
],
});
expect(result.guides.map((guide) => [guide.number, guide.outcome, guide.detail])).toEqual([
[1, "failed", "Review guides are off. Turn them on in Settings → Source Control."],
[2, "started", null],
]);
}),
);

it.effect("reports an unreadable entry on its own and still writes the rest of the batch", () =>
Effect.gen(function* () {
const harness = yield* makeHarness();
const result = yield* harness.call("write_pull_request_guides", {
pullRequests: [
{ url: "https://example.com/not-a-pull-request" },
{ repository: "t3tools/t3code" },
{ repository: "t3tools/t3code", number: 5 },
],
});
expect(result.guides.map((guide) => guide.outcome)).toEqual(["failed", "failed", "started"]);
expect(result.guides[0]?.detail).toContain("not a recognised pull request URL");
expect(result.guides[1]?.detail).toContain("Pass either url");
expect((yield* Ref.get(harness.guideRequests)).map((ref) => ref.number)).toEqual([5]);
}),
);

it.effect("reports a run the service joined as running, not as a new one", () =>
Effect.gen(function* () {
const harness = yield* makeHarness({
generate: () =>
Effect.succeed({
...NO_GUIDE,
run: { _tag: "Running", headSha: "head-1", startedAt: "2026-10-11T00:00:00.000Z" },
started: false,
}),
});
const result = yield* harness.call("write_pull_request_guides", {
pullRequests: [{ repository: "t3tools/t3code", number: 4 }],
});
expect(result.guides.map((guide) => guide.outcome)).toEqual(["running"]);
}),
);

it.effect("finds an Azure DevOps checkout for a pull request named by its full path", () =>
Effect.gen(function* () {
const azure = {
...makeProject({
canonicalKey: "dev.azure.com/org/project/_git/web",
locator: {
source: "git-remote",
remoteName: "origin",
remoteUrl: "https://dev.azure.com/org/project/_git/web",
},
provider: "azure-devops",
displayName: "org/project/_git/web",
owner: "org",
name: "web",
}),
id: ProjectId.make("project-azure"),
};
const harness = yield* makeHarness({ projects: [makeProject(), azure] });
const result = yield* harness.call("write_pull_request_guides", {
pullRequests: [{ host: "dev.azure.com", repository: "org/project/_git/web", number: 9 }],
});
expect(result.guides.map((guide) => guide.outcome)).toEqual(["started"]);
expect((yield* Ref.get(harness.guideRequests)).map((ref) => ref.projectId)).toEqual([
"project-azure",
]);
}),
);

it.effect("reads a stored guide and says when a push left it behind", () =>
Effect.gen(function* () {
const harness = yield* makeHarness();
const result = yield* harness.call("read_pull_request_guide", {
repository: "t3tools/t3code",
number: 3,
});
expect(result).toMatchObject({ number: 3, guide: null, run: null, stale: false });
}),
);
});

it("keeps failure diagnostics as the cause rather than exposing them in the tool message", () => {
const cause = new Error("database internals");
const failure = new PullRequestLinkFailedError({ cause });
Expand Down
Loading
Loading