Skip to content
Open
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
47 changes: 44 additions & 3 deletions apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ describe("listThreadPullRequests", () => {

describe("pull request guide tools", () => {
it.effect(
"writes guides through the project whose checkout is each pull request's repository",
"prefers matching projects and uses the caller's project for other linked repositories",
() =>
Effect.gen(function* () {
const other = {
Expand Down Expand Up @@ -614,18 +614,59 @@ describe("pull request guide tools", () => {
).toEqual([
["acme/web", 7, "started"],
["t3tools/t3code", 12, "started"],
["someone/else", 1, "failed"],
["someone/else", 1, "started"],
]);
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],
[PROJECT_ID, 1],
]);
}),
);

it.effect("writes and reads a linked guide from a project without a repository", () =>
Effect.gen(function* () {
const harness = yield* makeHarness({ project: makeProject(null) });
const target = { url: "https://github.com/acme/web/pull/7" };
const written = yield* harness.call("write_pull_request_guides", { pullRequests: [target] });
expect(written.guides).toMatchObject([
{ host: "github.com", repository: "acme/web", number: 7, outcome: "started" },
]);
expect(yield* Ref.get(harness.guideRequests)).toEqual([
{ projectId: PROJECT_ID, host: "github.com", repository: "acme/web", number: 7 },
]);
const read = yield* harness.call("read_pull_request_guide", target);
expect(read).toMatchObject({ number: 7, guide: null, run: null, stale: false });
}),
);

it.effect("requires a matching project when the invocation has no caller thread", () =>
Effect.gen(function* () {
const harness = yield* makeHarness({ thread: null, project: makeProject(null) });
const result = yield* harness.call("write_pull_request_guides", {
pullRequests: [{ url: "https://github.com/acme/web/pull/7" }],
});
expect(result.guides.map((guide) => guide.outcome)).toEqual(["failed"]);
expect(result.guides[0]?.detail).toContain("No matching project");
expect(yield* Ref.get(harness.guideRequests)).toEqual([]);
}),
);

it.effect("reports a missing project when the environment has none", () =>
Effect.gen(function* () {
const harness = yield* makeHarness({ project: null });
const target = { url: "https://github.com/acme/web/pull/7" };
const written = yield* harness.call("write_pull_request_guides", { pullRequests: [target] });
expect(written.guides[0]).toMatchObject({ outcome: "failed" });
expect(written.guides[0]?.detail).toContain("No matching project");
const error = yield* harness.call("read_pull_request_guide", target).pipe(Effect.flip);
expect(error).toMatchObject({ _tag: "PullRequestProjectNotFoundError" });
expect(yield* Ref.get(harness.guideRequests)).toEqual([]);
}),
);

it.effect("reports a refused run on its own pull request without failing the rest", () =>
Effect.gen(function* () {
const harness = yield* makeHarness({
Expand Down
14 changes: 6 additions & 8 deletions apps/server/src/mcp/toolkits/pullRequests/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,15 @@ export function listThreadPullRequests(
}

/**
* The project a pull request is read through: the caller's own when its checkout is that
* repository, else the first project here whose checkout is. Host reads are scoped to a project,
* so a repository with no checkout in this environment cannot be guided from it.
* Prefer a matching checkout, then the caller's project. The guide service can fetch a linked
* repository into a temporary checkout; the pull request service owns host access and routing.
*/
function projectForPullRequest(
projects: ReadonlyArray<OrchestrationProjectShell>,
target: { readonly host: string; readonly repository: string },
preferred: OrchestrationProjectShell | undefined,
): OrchestrationProjectShell | undefined {
// The same rule the guide service uses: a pull request reads through a project whose checkout
// names it with the same host-level key, which also maps Azure DevOps' org/project/_git/repo.
// Compare host-level keys, including Azure DevOps' org/project/_git/repo.
const matches = (project: OrchestrationProjectShell) => {
const own = pullRequestSyncKey(
{ projectId: project.id, host: target.host, repository: target.repository, number: 1 },
Expand All @@ -194,7 +192,7 @@ function projectForPullRequest(
);
};
if (preferred !== undefined && matches(preferred)) return preferred;
return projects.find(matches);
return projects.find(matches) ?? preferred;
}

const make = Effect.gen(function* () {
Expand Down Expand Up @@ -311,7 +309,7 @@ const make = Effect.gen(function* () {

/**
* Resolves each requested pull request and the project it is read through. The caller's own
* project wins where it is a checkout of that repository; with no thread, any project here may.
* matching checkout wins; otherwise the caller's project serves it.
*/
const locateAll = Effect.fn("PullRequestsToolkit.locateAll")(function* (
targets: ReadonlyArray<Omit<PullRequestTargetInput, "threadId">>,
Expand Down Expand Up @@ -344,7 +342,7 @@ const make = Effect.gen(function* () {
);
});

/** The service reads a pull request through one project's checkout and credentials. */
/** The project supplies settings and host access, without requiring a matching checkout. */
const guideReference = (
project: OrchestrationProjectShell,
target: { readonly host: string; readonly repository: string; readonly number: number },
Expand Down
4 changes: 2 additions & 2 deletions apps/server/src/mcp/toolkits/pullRequests/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export class PullRequestProjectNotFoundError extends Schema.TaggedError<PullRequ
{ repository: Schema.String },
) {
override get message(): string {
return `No project in this environment is a checkout of ${this.repository}, so its pull requests cannot be read here. Add one with t3_project_create.`;
return `No matching project or calling thread's project is available to read ${this.repository}'s pull requests. Add one with t3_project_create.`;
}
}

Expand Down Expand Up @@ -383,7 +383,7 @@ export type ReadPullRequestGuideResult = typeof ReadPullRequestGuideResult.Type;

const WritePullRequestGuidesTool = Tool.make("write_pull_request_guides", {
description:
"Write review guides for up to 20 pull requests, the tour a maintainer reads in a pull request's Guide tab. Each run reads its change in a temporary checkout and takes minutes, so this returns once the runs are under way; read a guide with read_pull_request_guide to see when it is written. A guide for the same head that is already running is joined, not restarted; a pull request whose guide is current is written again. Each pull request is read through the project in this environment whose checkout has that repository. Fails while review guides are off on this server.",
"Write review guides for up to 20 pull requests, the tour a maintainer reads in a pull request's Guide tab. Each run reads its change in a temporary checkout and takes minutes, so this returns once the runs are under way; read a guide with read_pull_request_guide to see when it is written. A guide for the same head that is already running is joined, not restarted; a pull request whose guide is current is written again. A matching project is preferred; otherwise the calling thread's project is used without requiring a repository checkout. Fails while review guides are off on this server.",
parameters: Schema.Struct({
pullRequests: Schema.Array(GuideTarget).check(Schema.isMinLength(1), Schema.isMaxLength(20)),
}),
Expand Down
Loading
Loading