Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
194 changes: 193 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,23 @@ 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, PullRequestOperationError>;
/** A run already going when the tool asks, which a write joins. */
readonly running?: PullRequestReviewGuideResult["run"];
}

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 +173,24 @@ 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, run: options.running ?? null }),
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",
},
}),
),
),
}),
Layer.mock(Orchestrator.OrchestratorV2)({
getThreadShell: (id) =>
Expand Down Expand Up @@ -189,7 +222,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 +579,165 @@ 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",
},
}),
});
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("joins a run already going rather than reporting a new one", () =>
Effect.gen(function* () {
const harness = yield* makeHarness({
running: { _tag: "Running", headSha: "head-1", startedAt: "2026-10-11T00:00:00.000Z" },
});
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("reports a run for a new head as started, not as the older run it replaced", () =>
Effect.gen(function* () {
const harness = yield* makeHarness({
running: { _tag: "Running", headSha: "head-old", startedAt: "2026-10-11T00:00:00.000Z" },
});
const result = yield* harness.call("write_pull_request_guides", {
pullRequests: [{ repository: "t3tools/t3code", number: 6 }],
});
expect(result.guides.map((guide) => guide.outcome)).toEqual(["started"]);
}),
);

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