| author | |
| committer | |
| log | 18656a3402459d9d145bb3fa5767723531b27ac7 |
| tree | 6635e841d84a73c648b983841b5fe00a7eb91bcb |
| parent | 1f6df6bf1f50ab5b8e3253716773da2571f8243e |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>13 files changed, 555 insertions(+), 7 deletions(-)
apps/server/src/mcp/McpHttpServer.test.ts+2| ... | @@ -27,6 +27,7 @@ import { | ... | @@ -27,6 +27,7 @@ import { |
| 27 | } from "effect/http"; | 27 | } from "effect/http"; |
| 28 | 28 | ||
| 29 | import * as ProjectService from "../project/ProjectService.ts"; | 29 | import * as ProjectService from "../project/ProjectService.ts"; |
| 30 | import * as PullRequestReviewGuideService from "../pullRequest/PullRequestReviewGuideService.ts"; | ||
| 30 | import * as ServerConfig from "../config.ts"; | 31 | import * as ServerConfig from "../config.ts"; |
| 31 | import * as McpHttpServer from "./McpHttpServer.ts"; | 32 | import * as McpHttpServer from "./McpHttpServer.ts"; |
| 32 | import * as McpToolAccess from "./McpToolAccess.ts"; | 33 | import * as McpToolAccess from "./McpToolAccess.ts"; |
| ... | @@ -77,6 +78,7 @@ const layerPullRequestsTest = McpHttpServer.layerPullRequestsToolkit.pipe( | ... | @@ -77,6 +78,7 @@ const layerPullRequestsTest = McpHttpServer.layerPullRequestsToolkit.pipe( |
| 77 | Layer.provide( | 78 | Layer.provide( |
| 78 | Layer.mergeAll( | 79 | Layer.mergeAll( |
| 79 | Layer.mock(ProjectService.ProjectService)({}), | 80 | Layer.mock(ProjectService.ProjectService)({}), |
| 81 | Layer.mock(PullRequestReviewGuideService.PullRequestReviewGuideService)({}), | ||
| 80 | Layer.mock(Orchestrator.OrchestratorV2)({}), | 82 | Layer.mock(Orchestrator.OrchestratorV2)({}), |
| 81 | Layer.mock(ProjectionStore.ProjectionStoreV2)({}), | 83 | Layer.mock(ProjectionStore.ProjectionStoreV2)({}), |
| 82 | NodeServices.layer, | 84 | NodeServices.layer, |
apps/server/src/mcp/McpToolAccess.test.ts+8| ... | @@ -87,6 +87,7 @@ const ProbeToolkit = Toolkit.make( | ... | @@ -87,6 +87,7 @@ const ProbeToolkit = Toolkit.make( |
| 87 | Tool.make("reads_as_caller", probe), | 87 | Tool.make("reads_as_caller", probe), |
| 88 | Tool.make("acts_as_caller", probe), | 88 | Tool.make("acts_as_caller", probe), |
| 89 | Tool.make("writes", probe), | 89 | Tool.make("writes", probe), |
| 90 | Tool.make("writes_with_capability", probe), | ||
| 90 | Tool.make("writes_threads", { | 91 | Tool.make("writes_threads", { |
| 91 | ...probe, | 92 | ...probe, |
| 92 | parameters: Schema.Struct({ threadId: Schema.optional(ThreadId) }), | 93 | parameters: Schema.Struct({ threadId: Schema.optional(ThreadId) }), |
| ... | @@ -106,6 +107,7 @@ const probeHandlers: McpToolAccess.Handlers<typeof ProbeToolkit.tools> = { | ... | @@ -106,6 +107,7 @@ const probeHandlers: McpToolAccess.Handlers<typeof ProbeToolkit.tools> = { |
| 106 | reads_as_caller: McpToolAccess.readsAsCaller(() => ran), | 107 | reads_as_caller: McpToolAccess.readsAsCaller(() => ran), |
| 107 | acts_as_caller: McpToolAccess.actsAsCaller(() => ran), | 108 | acts_as_caller: McpToolAccess.actsAsCaller(() => ran), |
| 108 | writes: McpToolAccess.writes(() => ran), | 109 | writes: McpToolAccess.writes(() => ran), |
| 110 | writes_with_capability: McpToolAccess.writesWithCapability(() => ran), | ||
| 109 | writes_threads: McpToolAccess.writesThreads( | 111 | writes_threads: McpToolAccess.writesThreads( |
| 110 | (input) => [input.threadId], | 112 | (input) => [input.threadId], |
| 111 | () => ran, | 113 | () => ran, |
| ... | @@ -152,6 +154,7 @@ export const refusedAtCompileTime = () => { | ... | @@ -152,6 +154,7 @@ export const refusedAtCompileTime = () => { |
| 152 | reads_as_caller: unchecked, | 154 | reads_as_caller: unchecked, |
| 153 | acts_as_caller: unchecked, | 155 | acts_as_caller: unchecked, |
| 154 | writes: unchecked, | 156 | writes: unchecked, |
| 157 | writes_with_capability: unchecked, | ||
| 155 | writes_threads: unchecked, | 158 | writes_threads: unchecked, |
| 156 | starts_threads: unchecked, | 159 | starts_threads: unchecked, |
| 157 | writes_environment: unchecked, | 160 | writes_environment: unchecked, |
| ... | @@ -224,6 +227,11 @@ it.effect.each([ | ... | @@ -224,6 +227,11 @@ it.effect.each([ |
| 224 | ["writes", previewOnly, {}, "capability_denied"], | 227 | ["writes", previewOnly, {}, "capability_denied"], |
| 225 | ["writes_environment", previewOnly, {}, "capability_denied"], | 228 | ["writes_environment", previewOnly, {}, "capability_denied"], |
| 226 | 229 | ||
| 230 | // A tool's own capability stands in for thread control; the rest of a change still applies. | ||
| 231 | ["writes_with_capability", previewOnly, {}, "ran"], | ||
| 232 | ["writes_with_capability", readOnlyClient, {}, "capability_denied"], | ||
| 233 | ["writes_with_capability", ended, {}, "parent_not_active"], | ||
| 234 | |||
| 227 | // Changing a thread: only one that runs within the caller's modes. | 235 | // Changing a thread: only one that runs within the caller's modes. |
| 228 | ["writes_threads", supervised, {}, "ran"], | 236 | ["writes_threads", supervised, {}, "ran"], |
| 229 | ["writes_threads", supervised, { threadId: planThreadId }, "ran"], | 237 | ["writes_threads", supervised, { threadId: planThreadId }, "ran"], |
apps/server/src/mcp/McpToolAccess.ts+8| ... | @@ -157,6 +157,14 @@ export const actsAsCaller = <P, A, E, R>(handle: (params: P) => Effect.Effect<A, | ... | @@ -157,6 +157,14 @@ export const actsAsCaller = <P, A, E, R>(handle: (params: P) => Effect.Effect<A, |
| 157 | export const writes = <P, A, E, R>(handle: (params: P) => Effect.Effect<A, E, R>) => | 157 | export const writes = <P, A, E, R>(handle: (params: P) => Effect.Effect<A, E, R>) => |
| 158 | declare((params: P) => orchestratingCaller.pipe(Effect.flatMap(() => handle(params)))); | 158 | declare((params: P) => orchestratingCaller.pipe(Effect.flatMap(() => handle(params)))); |
| 159 | 159 | ||
| 160 | /** | ||
| 161 | * Changes something that belongs to no thread under a capability of the tool's own, such as | ||
| 162 | * writing a pull request's review guide. Unlike `writes` it does not need thread control; the | ||
| 163 | * handler checks its capability, as the capability toolkits do. | ||
| 164 | */ | ||
| 165 | export const writesWithCapability = <P, A, E, R>(handle: (params: P) => Effect.Effect<A, E, R>) => | ||
| 166 | declare((params: P) => writingCaller.pipe(Effect.flatMap(() => handle(params)))); | ||
| 167 | |||
| 160 | /** | 168 | /** |
| 161 | * Changes the threads `threads` names. An omitted id is the caller's own | 169 | * Changes the threads `threads` names. An omitted id is the caller's own |
| 162 | * thread; any other thread must run within the caller's modes, checked here | 170 | * thread; any other thread must run within the caller's modes, checked here |
apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts+189-1| ... | @@ -4,7 +4,10 @@ import { | ... | @@ -4,7 +4,10 @@ import { |
| 4 | ProviderInstanceId, | 4 | ProviderInstanceId, |
| 5 | ThreadId, | 5 | ThreadId, |
| 6 | type OrchestrationV2ServerCommand as OrchestrationCommand, | 6 | type OrchestrationV2ServerCommand as OrchestrationCommand, |
| 7 | PullRequestOperationError, | ||
| 7 | type OrchestrationProjectShell, | 8 | type OrchestrationProjectShell, |
| 9 | type PullRequestRef, | ||
| 10 | type PullRequestReviewGuideResult, | ||
| 8 | type ThreadPullRequestLink, | 11 | type ThreadPullRequestLink, |
| 9 | } from "@t3tools/contracts"; | 12 | } from "@t3tools/contracts"; |
| 10 | import { describe, expect, it } from "@effect/vitest"; | 13 | import { describe, expect, it } from "@effect/vitest"; |
| ... | @@ -22,6 +25,7 @@ import { | ... | @@ -22,6 +25,7 @@ import { |
| 22 | v2PullRequestThread, | 25 | v2PullRequestThread, |
| 23 | } from "../../../orchestration-v2/testkit/pullRequestFixtures.ts"; | 26 | } from "../../../orchestration-v2/testkit/pullRequestFixtures.ts"; |
| 24 | import * as ProjectService from "../../../project/ProjectService.ts"; | 27 | import * as ProjectService from "../../../project/ProjectService.ts"; |
| 28 | import * as PullRequestReviewGuideService from "../../../pullRequest/PullRequestReviewGuideService.ts"; | ||
| 25 | import * as McpInvocationContext from "../../McpInvocationContext.ts"; | 29 | import * as McpInvocationContext from "../../McpInvocationContext.ts"; |
| 26 | import * as McpToolAccess from "../../McpToolAccess.ts"; | 30 | import * as McpToolAccess from "../../McpToolAccess.ts"; |
| 27 | import * as McpToolAccessTestkit from "../../McpToolAccess.testkit.ts"; | 31 | import * as McpToolAccessTestkit from "../../McpToolAccess.testkit.ts"; |
| ... | @@ -135,12 +139,24 @@ interface HarnessOptions { | ... | @@ -135,12 +139,24 @@ interface HarnessOptions { |
| 135 | readonly project?: OrchestrationProjectShell | null; | 139 | readonly project?: OrchestrationProjectShell | null; |
| 136 | /** A rejection the orchestrator reports as the dispatch error's cause. */ | 140 | /** A rejection the orchestrator reports as the dispatch error's cause. */ |
| 137 | readonly reject?: (command: OrchestrationCommand) => string | null; | 141 | readonly reject?: (command: OrchestrationCommand) => string | null; |
| 142 | /** Every project in the environment; defaults to the thread's own. */ | ||
| 143 | readonly projects?: ReadonlyArray<OrchestrationProjectShell>; | ||
| 144 | /** What the guide service answers for a pull request it is asked to write. */ | ||
| 145 | readonly generate?: ( | ||
| 146 | input: PullRequestRef, | ||
| 147 | ) => Effect.Effect< | ||
| 148 | PullRequestReviewGuideResult & { started: boolean }, | ||
| 149 | PullRequestOperationError | ||
| 150 | >; | ||
| 138 | } | 151 | } |
| 139 | 152 | ||
| 153 | const NO_GUIDE: PullRequestReviewGuideResult = { guide: null, run: null, currentHeadSha: "head-1" }; | ||
| 154 | |||
| 140 | const makeHarness = Effect.fn("makePullRequestsToolkitHarness")(function* ( | 155 | const makeHarness = Effect.fn("makePullRequestsToolkitHarness")(function* ( |
| 141 | options: HarnessOptions = {}, | 156 | options: HarnessOptions = {}, |
| 142 | ) { | 157 | ) { |
| 143 | const commands = yield* Ref.make<ReadonlyArray<OrchestrationCommand>>([]); | 158 | const commands = yield* Ref.make<ReadonlyArray<OrchestrationCommand>>([]); |
| 159 | const guideRequests = yield* Ref.make<ReadonlyArray<PullRequestRef>>([]); | ||
| 144 | const thread = options.thread === undefined ? makeThread([]) : options.thread; | 160 | const thread = options.thread === undefined ? makeThread([]) : options.thread; |
| 145 | const project = options.project === undefined ? makeProject() : options.project; | 161 | const project = options.project === undefined ? makeProject() : options.project; |
| 146 | const dispatch: Orchestrator.OrchestratorV2Shape["dispatch"] = (command) => | 162 | const dispatch: Orchestrator.OrchestratorV2Shape["dispatch"] = (command) => |
| ... | @@ -158,6 +174,25 @@ const makeHarness = Effect.fn("makePullRequestsToolkitHarness")(function* ( | ... | @@ -158,6 +174,25 @@ const makeHarness = Effect.fn("makePullRequestsToolkitHarness")(function* ( |
| 158 | const layerDependencies = Layer.mergeAll( | 174 | const layerDependencies = Layer.mergeAll( |
| 159 | Layer.mock(ProjectService.ProjectService)({ | 175 | Layer.mock(ProjectService.ProjectService)({ |
| 160 | getShell: () => Effect.succeed(Option.fromNullishOr(project)), | 176 | getShell: () => Effect.succeed(Option.fromNullishOr(project)), |
| 177 | listShells: () => Effect.succeed(options.projects ?? (project ? [project] : [])), | ||
| 178 | }), | ||
| 179 | Layer.mock(PullRequestReviewGuideService.PullRequestReviewGuideService)({ | ||
| 180 | get: () => Effect.succeed(NO_GUIDE), | ||
| 181 | generate: (input) => | ||
| 182 | Ref.update(guideRequests, (recorded) => [...recorded, input]).pipe( | ||
| 183 | Effect.andThen( | ||
| 184 | options.generate?.(input) ?? | ||
| 185 | Effect.succeed({ | ||
| 186 | ...NO_GUIDE, | ||
| 187 | run: { | ||
| 188 | _tag: "Running" as const, | ||
| 189 | headSha: "head-1", | ||
| 190 | startedAt: "2026-10-11T00:00:00.000Z", | ||
| 191 | }, | ||
| 192 | started: true, | ||
| 193 | }), | ||
| 194 | ), | ||
| 195 | ), | ||
| 161 | }), | 196 | }), |
| 162 | Layer.mock(Orchestrator.OrchestratorV2)({ | 197 | Layer.mock(Orchestrator.OrchestratorV2)({ |
| 163 | getThreadShell: (id) => | 198 | getThreadShell: (id) => |
| ... | @@ -189,7 +224,7 @@ const makeHarness = Effect.fn("makePullRequestsToolkitHarness")(function* ( | ... | @@ -189,7 +224,7 @@ const makeHarness = Effect.fn("makePullRequestsToolkitHarness")(function* ( |
| 189 | Effect.provideService(McpInvocationContext.McpInvocationContext, invocation(capabilities)), | 224 | Effect.provideService(McpInvocationContext.McpInvocationContext, invocation(capabilities)), |
| 190 | Effect.provide(layerDependencies), | 225 | Effect.provide(layerDependencies), |
| 191 | ); | 226 | ); |
| 192 | return { commands, call }; | 227 | return { commands, guideRequests, call }; |
| 193 | }); | 228 | }); |
| 194 | 229 | ||
| 195 | describe("pull request toolkit handlers", () => { | 230 | describe("pull request toolkit handlers", () => { |
| ... | @@ -546,6 +581,159 @@ describe("listThreadPullRequests", () => { | ... | @@ -546,6 +581,159 @@ describe("listThreadPullRequests", () => { |
| 546 | }); | 581 | }); |
| 547 | }); | 582 | }); |
| 548 | 583 | ||
| 584 | describe("pull request guide tools", () => { | ||
| 585 | it.effect( | ||
| 586 | "writes guides through the project whose checkout is each pull request's repository", | ||
| 587 | () => | ||
| 588 | Effect.gen(function* () { | ||
| 589 | const other = { | ||
| 590 | ...makeProject({ | ||
| 591 | canonicalKey: "github.com/acme/web", | ||
| 592 | locator: { | ||
| 593 | source: "git-remote", | ||
| 594 | remoteName: "origin", | ||
| 595 | remoteUrl: "git@github.com:acme/web.git", | ||
| 596 | }, | ||
| 597 | provider: "github", | ||
| 598 | displayName: "acme/web", | ||
| 599 | owner: "acme", | ||
| 600 | name: "web", | ||
| 601 | }), | ||
| 602 | id: ProjectId.make("project-web"), | ||
| 603 | }; | ||
| 604 | const harness = yield* makeHarness({ projects: [makeProject(), other] }); | ||
| 605 | const result = yield* harness.call("write_pull_request_guides", { | ||
| 606 | pullRequests: [ | ||
| 607 | { url: "https://github.com/acme/web/pull/7" }, | ||
| 608 | { repository: "t3tools/t3code", number: 12 }, | ||
| 609 | { url: "https://github.com/someone/else/pull/1" }, | ||
| 610 | ], | ||
| 611 | }); | ||
| 612 | expect( | ||
| 613 | result.guides.map((guide) => [guide.repository, guide.number, guide.outcome]), | ||
| 614 | ).toEqual([ | ||
| 615 | ["acme/web", 7, "started"], | ||
| 616 | ["t3tools/t3code", 12, "started"], | ||
| 617 | ["someone/else", 1, "failed"], | ||
| 618 | ]); | ||
| 619 | expect(result.guides[2]?.detail).toContain("No project in this environment"); | ||
| 620 | expect( | ||
| 621 | (yield* Ref.get(harness.guideRequests)).map((ref) => [ref.projectId, ref.number]), | ||
| 622 | ).toEqual([ | ||
| 623 | ["project-web", 7], | ||
| 624 | [PROJECT_ID, 12], | ||
| 625 | ]); | ||
| 626 | }), | ||
| 627 | ); | ||
| 628 | |||
| 629 | it.effect("reports a refused run on its own pull request without failing the rest", () => | ||
| 630 | Effect.gen(function* () { | ||
| 631 | const harness = yield* makeHarness({ | ||
| 632 | generate: (input) => | ||
| 633 | input.number === 1 | ||
| 634 | ? Effect.fail( | ||
| 635 | new PullRequestOperationError({ | ||
| 636 | operation: "generateReviewGuide", | ||
| 637 | detail: "Review guides are off. Turn them on in Settings → Source Control.", | ||
| 638 | }), | ||
| 639 | ) | ||
| 640 | : Effect.succeed({ | ||
| 641 | ...NO_GUIDE, | ||
| 642 | run: { | ||
| 643 | _tag: "Running" as const, | ||
| 644 | headSha: "head-1", | ||
| 645 | startedAt: "2026-10-11T00:00:00.000Z", | ||
| 646 | }, | ||
| 647 | started: true, | ||
| 648 | }), | ||
| 649 | }); | ||
| 650 | const result = yield* harness.call("write_pull_request_guides", { | ||
| 651 | pullRequests: [ | ||
| 652 | { repository: "t3tools/t3code", number: 1 }, | ||
| 653 | { repository: "t3tools/t3code", number: 2 }, | ||
| 654 | ], | ||
| 655 | }); | ||
| 656 | expect(result.guides.map((guide) => [guide.number, guide.outcome, guide.detail])).toEqual([ | ||
| 657 | [1, "failed", "Review guides are off. Turn them on in Settings → Source Control."], | ||
| 658 | [2, "started", null], | ||
| 659 | ]); | ||
| 660 | }), | ||
| 661 | ); | ||
| 662 | |||
| 663 | it.effect("reports an unreadable entry on its own and still writes the rest of the batch", () => | ||
| 664 | Effect.gen(function* () { | ||
| 665 | const harness = yield* makeHarness(); | ||
| 666 | const result = yield* harness.call("write_pull_request_guides", { | ||
| 667 | pullRequests: [ | ||
| 668 | { url: "https://example.com/not-a-pull-request" }, | ||
| 669 | { repository: "t3tools/t3code" }, | ||
| 670 | { repository: "t3tools/t3code", number: 5 }, | ||
| 671 | ], | ||
| 672 | }); | ||
| 673 | expect(result.guides.map((guide) => guide.outcome)).toEqual(["failed", "failed", "started"]); | ||
| 674 | expect(result.guides[0]?.detail).toContain("not a recognised pull request URL"); | ||
| 675 | expect(result.guides[1]?.detail).toContain("Pass either url"); | ||
| 676 | expect((yield* Ref.get(harness.guideRequests)).map((ref) => ref.number)).toEqual([5]); | ||
| 677 | }), | ||
| 678 | ); | ||
| 679 | |||
| 680 | it.effect("reports a run the service joined as running, not as a new one", () => | ||
| 681 | Effect.gen(function* () { | ||
| 682 | const harness = yield* makeHarness({ | ||
| 683 | generate: () => | ||
| 684 | Effect.succeed({ | ||
| 685 | ...NO_GUIDE, | ||
| 686 | run: { _tag: "Running", headSha: "head-1", startedAt: "2026-10-11T00:00:00.000Z" }, | ||
| 687 | started: false, | ||
| 688 | }), | ||
| 689 | }); | ||
| 690 | const result = yield* harness.call("write_pull_request_guides", { | ||
| 691 | pullRequests: [{ repository: "t3tools/t3code", number: 4 }], | ||
| 692 | }); | ||
| 693 | expect(result.guides.map((guide) => guide.outcome)).toEqual(["running"]); | ||
| 694 | }), | ||
| 695 | ); | ||
| 696 | |||
| 697 | it.effect("finds an Azure DevOps checkout for a pull request named by its full path", () => | ||
| 698 | Effect.gen(function* () { | ||
| 699 | const azure = { | ||
| 700 | ...makeProject({ | ||
| 701 | canonicalKey: "dev.azure.com/org/project/_git/web", | ||
| 702 | locator: { | ||
| 703 | source: "git-remote", | ||
| 704 | remoteName: "origin", | ||
| 705 | remoteUrl: "https://dev.azure.com/org/project/_git/web", | ||
| 706 | }, | ||
| 707 | provider: "azure-devops", | ||
| 708 | displayName: "org/project/_git/web", | ||
| 709 | owner: "org", | ||
| 710 | name: "web", | ||
| 711 | }), | ||
| 712 | id: ProjectId.make("project-azure"), | ||
| 713 | }; | ||
| 714 | const harness = yield* makeHarness({ projects: [makeProject(), azure] }); | ||
| 715 | const result = yield* harness.call("write_pull_request_guides", { | ||
| 716 | pullRequests: [{ host: "dev.azure.com", repository: "org/project/_git/web", number: 9 }], | ||
| 717 | }); | ||
| 718 | expect(result.guides.map((guide) => guide.outcome)).toEqual(["started"]); | ||
| 719 | expect((yield* Ref.get(harness.guideRequests)).map((ref) => ref.projectId)).toEqual([ | ||
| 720 | "project-azure", | ||
| 721 | ]); | ||
| 722 | }), | ||
| 723 | ); | ||
| 724 | |||
| 725 | it.effect("reads a stored guide and says when a push left it behind", () => | ||
| 726 | Effect.gen(function* () { | ||
| 727 | const harness = yield* makeHarness(); | ||
| 728 | const result = yield* harness.call("read_pull_request_guide", { | ||
| 729 | repository: "t3tools/t3code", | ||
| 730 | number: 3, | ||
| 731 | }); | ||
| 732 | expect(result).toMatchObject({ number: 3, guide: null, run: null, stale: false }); | ||
| 733 | }), | ||
| 734 | ); | ||
| 735 | }); | ||
| 736 | |||
| 549 | it("keeps failure diagnostics as the cause rather than exposing them in the tool message", () => { | 737 | it("keeps failure diagnostics as the cause rather than exposing them in the tool message", () => { |
| 550 | const cause = new Error("database internals"); | 738 | const cause = new Error("database internals"); |
| 551 | const failure = new PullRequestLinkFailedError({ cause }); | 739 | const failure = new PullRequestLinkFailedError({ cause }); |
apps/server/src/mcp/toolkits/pullRequests/handlers.ts+183| ... | @@ -12,6 +12,7 @@ import { | ... | @@ -12,6 +12,7 @@ import { |
| 12 | type ThreadPullRequestLink, | 12 | type ThreadPullRequestLink, |
| 13 | } from "@t3tools/contracts"; | 13 | } from "@t3tools/contracts"; |
| 14 | import { changeRequestUrlFor, parseChangeRequestUrl } from "@t3tools/shared/changeRequestUrl"; | 14 | import { changeRequestUrlFor, parseChangeRequestUrl } from "@t3tools/shared/changeRequestUrl"; |
| 15 | import { sourceControlRepositorySelector } from "@t3tools/shared/sourceControl"; | ||
| 15 | import { | 16 | import { |
| 16 | normalizeThreadPullRequestKey, | 17 | normalizeThreadPullRequestKey, |
| 17 | resolveThreadPullRequestChains, | 18 | resolveThreadPullRequestChains, |
| ... | @@ -25,6 +26,8 @@ import * as Option from "effect/Option"; | ... | @@ -25,6 +26,8 @@ import * as Option from "effect/Option"; |
| 25 | 26 | ||
| 26 | import * as Orchestrator from "../../../orchestration-v2/Orchestrator.ts"; | 27 | import * as Orchestrator from "../../../orchestration-v2/Orchestrator.ts"; |
| 27 | import * as ProjectService from "../../../project/ProjectService.ts"; | 28 | import * as ProjectService from "../../../project/ProjectService.ts"; |
| 29 | import * as PullRequestReviewGuideService from "../../../pullRequest/PullRequestReviewGuideService.ts"; | ||
| 30 | import { pullRequestSyncKey } from "../../../pullRequest/pullRequestSyncKey.ts"; | ||
| 28 | import * as McpInvocationContext from "../../McpInvocationContext.ts"; | 31 | import * as McpInvocationContext from "../../McpInvocationContext.ts"; |
| 29 | import * as McpToolAccess from "../../McpToolAccess.ts"; | 32 | import * as McpToolAccess from "../../McpToolAccess.ts"; |
| 30 | import { | 33 | import { |
| ... | @@ -41,6 +44,8 @@ import { | ... | @@ -41,6 +44,8 @@ import { |
| 41 | PullRequestWatchFromSubagentError, | 44 | PullRequestWatchFromSubagentError, |
| 42 | PullRequestThreadNotFoundError, | 45 | PullRequestThreadNotFoundError, |
| 43 | PullRequestThreadRequiredError, | 46 | PullRequestThreadRequiredError, |
| 47 | PullRequestGuideFailedError, | ||
| 48 | PullRequestProjectNotFoundError, | ||
| 44 | PullRequestsToolkit, | 49 | PullRequestsToolkit, |
| 45 | type ThreadPullRequestEntry, | 50 | type ThreadPullRequestEntry, |
| 46 | } from "./tools.ts"; | 51 | } from "./tools.ts"; |
| ... | @@ -153,8 +158,48 @@ export function listThreadPullRequests( | ... | @@ -153,8 +158,48 @@ export function listThreadPullRequests( |
| 153 | }; | 158 | }; |
| 154 | } | 159 | } |
| 155 | 160 | ||
| 161 | /** | ||
| 162 | * The project a pull request is read through: the caller's own when its checkout is that | ||
| 163 | * repository, else the first project here whose checkout is. Host reads are scoped to a project, | ||
| 164 | * so a repository with no checkout in this environment cannot be guided from it. | ||
| 165 | */ | ||
| 166 | function projectForPullRequest( | ||
| 167 | projects: ReadonlyArray<OrchestrationProjectShell>, | ||
| 168 | target: { readonly host: string; readonly repository: string }, | ||
| 169 | preferred: OrchestrationProjectShell | undefined, | ||
| 170 | ): OrchestrationProjectShell | undefined { | ||
| 171 | // The same rule the guide service uses: a pull request reads through a project whose checkout | ||
| 172 | // names it with the same host-level key, which also maps Azure DevOps' org/project/_git/repo. | ||
| 173 | const matches = (project: OrchestrationProjectShell) => { | ||
| 174 | const own = pullRequestSyncKey( | ||
| 175 | { projectId: project.id, host: target.host, repository: target.repository, number: 1 }, | ||
| 176 | project.repositoryIdentity ?? undefined, | ||
| 177 | ); | ||
| 178 | const projectKey = | ||
| 179 | project.repositoryIdentity == null | ||
| 180 | ? null | ||
| 181 | : pullRequestSyncKey( | ||
| 182 | { | ||
| 183 | projectId: project.id, | ||
| 184 | repository: sourceControlRepositorySelector(project.repositoryIdentity) ?? "", | ||
| 185 | number: 1, | ||
| 186 | }, | ||
| 187 | project.repositoryIdentity, | ||
| 188 | ); | ||
| 189 | return ( | ||
| 190 | own !== null && | ||
| 191 | projectKey !== null && | ||
| 192 | own.host === projectKey.host && | ||
| 193 | own.repository === projectKey.repository | ||
| 194 | ); | ||
| 195 | }; | ||
| 196 | if (preferred !== undefined && matches(preferred)) return preferred; | ||
| 197 | return projects.find(matches); | ||
| 198 | } | ||
| 199 | |||
| 156 | const make = Effect.gen(function* () { | 200 | const make = Effect.gen(function* () { |
| 157 | const engine = yield* Orchestrator.OrchestratorV2; | 201 | const engine = yield* Orchestrator.OrchestratorV2; |
| 202 | const reviewGuides = yield* PullRequestReviewGuideService.PullRequestReviewGuideService; | ||
| 158 | 203 | ||
| 159 | const projects = yield* ProjectService.ProjectService; | 204 | const projects = yield* ProjectService.ProjectService; |
| 160 | const crypto = yield* Crypto.Crypto; | 205 | const crypto = yield* Crypto.Crypto; |
| ... | @@ -264,6 +309,56 @@ const make = Effect.gen(function* () { | ... | @@ -264,6 +309,56 @@ const make = Effect.gen(function* () { |
| 264 | }; | 309 | }; |
| 265 | }); | 310 | }); |
| 266 | 311 | ||
| 312 | /** | ||
| 313 | * Resolves each requested pull request and the project it is read through. The caller's own | ||
| 314 | * project wins where it is a checkout of that repository; with no thread, any project here may. | ||
| 315 | */ | ||
| 316 | const locateAll = Effect.fn("PullRequestsToolkit.locateAll")(function* ( | ||
| 317 | targets: ReadonlyArray<Omit<PullRequestTargetInput, "threadId">>, | ||
| 318 | ) { | ||
| 319 | const scope = yield* McpInvocationContext.requireMcpCapability("pull-requests"); | ||
| 320 | const callerThread = | ||
| 321 | scope.thread === undefined | ||
| 322 | ? undefined | ||
| 323 | : yield* engine.getThreadShell(scope.thread.threadId).pipe( | ||
| 324 | Effect.orElseSucceed(() => null), | ||
| 325 | Effect.map((shell) => shell ?? undefined), | ||
| 326 | ); | ||
| 327 | const known = yield* projects | ||
| 328 | .listShells() | ||
| 329 | .pipe(Effect.mapError((cause) => new PullRequestListFailedError({ cause }))); | ||
| 330 | const callerProject = known.find((project) => project.id === callerThread?.projectId); | ||
| 331 | // Each target resolves on its own, so one bad URL in a batch is that entry's failure only. | ||
| 332 | return yield* Effect.forEach(targets, (input) => | ||
| 333 | resolveTarget(input, callerProject).pipe( | ||
| 334 | Effect.map((target) => ({ | ||
| 335 | input, | ||
| 336 | target, | ||
| 337 | project: projectForPullRequest(known, target, callerProject), | ||
| 338 | failure: null, | ||
| 339 | })), | ||
| 340 | Effect.catch((error) => | ||
| 341 | Effect.succeed({ input, target: null, project: undefined, failure: error.message }), | ||
| 342 | ), | ||
| 343 | ), | ||
| 344 | ); | ||
| 345 | }); | ||
| 346 | |||
| 347 | /** The service reads a pull request through one project's checkout and credentials. */ | ||
| 348 | const guideReference = ( | ||
| 349 | project: OrchestrationProjectShell, | ||
| 350 | target: { readonly host: string; readonly repository: string; readonly number: number }, | ||
| 351 | ) => ({ | ||
| 352 | projectId: project.id, | ||
| 353 | host: target.host, | ||
| 354 | repository: target.repository, | ||
| 355 | number: target.number, | ||
| 356 | }); | ||
| 357 | |||
| 358 | /** A host or service error, said as its own detail rather than its operation's wrapper. */ | ||
| 359 | const guideFailureDetail = (error: { readonly message: string }) => | ||
| 360 | "detail" in error && typeof error.detail === "string" ? error.detail : error.message; | ||
| 361 | |||
| 267 | /** A tool that changes `threadId`, or the caller's own thread when it is omitted. */ | 362 | /** A tool that changes `threadId`, or the caller's own thread when it is omitted. */ |
| 268 | const writesThread = <P extends { readonly threadId?: ThreadId | undefined }, A, E, R>( | 363 | const writesThread = <P extends { readonly threadId?: ThreadId | undefined }, A, E, R>( |
| 269 | handle: (params: P) => Effect.Effect<A, E, R>, | 364 | handle: (params: P) => Effect.Effect<A, E, R>, |
| ... | @@ -342,6 +437,94 @@ const make = Effect.gen(function* () { | ... | @@ -342,6 +437,94 @@ const make = Effect.gen(function* () { |
| 342 | ), | 437 | ), |
| 343 | watch_pull_request: writesThread((input) => setWatching(input, true)), | 438 | watch_pull_request: writesThread((input) => setWatching(input, true)), |
| 344 | unwatch_pull_request: writesThread((input) => setWatching(input, false)), | 439 | unwatch_pull_request: writesThread((input) => setWatching(input, false)), |
| 440 | write_pull_request_guides: McpToolAccess.writesWithCapability((input) => | ||
| 441 | Effect.gen(function* () { | ||
| 442 | const located = yield* locateAll(input.pullRequests); | ||
| 443 | // One pull request that cannot be guided does not stop the rest; it reports why. | ||
| 444 | const guides = yield* Effect.forEach( | ||
| 445 | located, | ||
| 446 | (entry) => | ||
| 447 | Effect.gen(function* () { | ||
| 448 | if (entry.target === null) { | ||
| 449 | return { | ||
| 450 | host: entry.input.host ?? "", | ||
| 451 | repository: entry.input.repository ?? "", | ||
| 452 | number: entry.input.number ?? 0, | ||
| 453 | url: entry.input.url ?? "", | ||
| 454 | outcome: "failed" as const, | ||
| 455 | detail: entry.failure, | ||
| 456 | }; | ||
| 457 | } | ||
| 458 | const identity = { | ||
| 459 | host: entry.target.host, | ||
| 460 | repository: entry.target.repository, | ||
| 461 | number: entry.target.number, | ||
| 462 | url: entry.target.url, | ||
| 463 | }; | ||
| 464 | if (entry.project === undefined) { | ||
| 465 | return { | ||
| 466 | ...identity, | ||
| 467 | outcome: "failed" as const, | ||
| 468 | detail: new PullRequestProjectNotFoundError({ | ||
| 469 | repository: entry.target.repository, | ||
| 470 | }).message, | ||
| 471 | }; | ||
| 472 | } | ||
| 473 | return yield* reviewGuides.generate(guideReference(entry.project, entry.target)).pipe( | ||
| 474 | Effect.map((result) => ({ | ||
| 475 | ...identity, | ||
| 476 | outcome: result.started ? ("started" as const) : ("running" as const), | ||
| 477 | detail: null, | ||
| 478 | })), | ||
| 479 | Effect.catch((error) => | ||
| 480 | Effect.succeed({ | ||
| 481 | ...identity, | ||
| 482 | outcome: "failed" as const, | ||
| 483 | detail: guideFailureDetail(error), | ||
| 484 | }), | ||
| 485 | ), | ||
| 486 | ); | ||
| 487 | }), | ||
| 488 | { concurrency: 4 }, | ||
| 489 | ); | ||
| 490 | return { guides }; | ||
| 491 | }), | ||
| 492 | ), | ||
| 493 | read_pull_request_guide: McpToolAccess.reads((input) => | ||
| 494 | Effect.gen(function* () { | ||
| 495 | const [entry] = yield* locateAll([input]); | ||
| 496 | if (entry !== undefined && entry.target === null) { | ||
| 497 | return yield* new PullRequestGuideFailedError({ detail: entry.failure ?? "" }); | ||
| 498 | } | ||
| 499 | if (entry === undefined || entry.target === null || entry.project === undefined) { | ||
| 500 | return yield* new PullRequestProjectNotFoundError({ | ||
| 501 | repository: entry?.target.repository ?? input.repository ?? "", | ||
| 502 | }); | ||
| 503 | } | ||
| 504 | const result = yield* reviewGuides.get(guideReference(entry.project, entry.target)).pipe( | ||
| 505 | Effect.mapError( | ||
| 506 | (error) => | ||
| 507 | new PullRequestGuideFailedError({ | ||
| 508 | detail: guideFailureDetail(error), | ||
| 509 | cause: error, | ||
| 510 | }), | ||
| 511 | ), | ||
| 512 | ); | ||
| 513 | return { | ||
| 514 | host: entry.target.host, | ||
| 515 | repository: entry.target.repository, | ||
| 516 | number: entry.target.number, | ||
| 517 | url: entry.target.url, | ||
| 518 | guide: result.guide, | ||
| 519 | run: result.run, | ||
| 520 | currentHeadSha: result.currentHeadSha, | ||
| 521 | stale: | ||
| 522 | result.guide !== null && | ||
| 523 | result.currentHeadSha !== null && | ||
| 524 | result.guide.headSha !== result.currentHeadSha, | ||
| 525 | }; | ||
| 526 | }), | ||
| 527 | ), | ||
| 345 | } satisfies McpToolAccess.Handlers<typeof PullRequestsToolkit.tools>; | 528 | } satisfies McpToolAccess.Handlers<typeof PullRequestsToolkit.tools>; |
| 346 | }); | 529 | }); |
| 347 | 530 |
apps/server/src/mcp/toolkits/pullRequests/tools.ts+94| ... | @@ -2,6 +2,8 @@ import { | ... | @@ -2,6 +2,8 @@ import { |
| 2 | McpCapabilityUnavailableError, | 2 | McpCapabilityUnavailableError, |
| 3 | OrchestratorMcpFailure, | 3 | OrchestratorMcpFailure, |
| 4 | PositiveInt, | 4 | PositiveInt, |
| 5 | PullRequestReviewGuide, | ||
| 6 | PullRequestReviewGuideRun, | ||
| 5 | PullRequestState, | 7 | PullRequestState, |
| 6 | ThreadPullRequestLinkSource, | 8 | ThreadPullRequestLinkSource, |
| 7 | TrimmedNonEmptyString, | 9 | TrimmedNonEmptyString, |
| ... | @@ -15,6 +17,7 @@ import * as McpInvocationContext from "../../McpInvocationContext.ts"; | ... | @@ -15,6 +17,7 @@ import * as McpInvocationContext from "../../McpInvocationContext.ts"; |
| 15 | import * as Orchestrator from "../../../orchestration-v2/Orchestrator.ts"; | 17 | import * as Orchestrator from "../../../orchestration-v2/Orchestrator.ts"; |
| 16 | import * as ThreadManagementService from "../../../orchestration-v2/ThreadManagementService.ts"; | 18 | import * as ThreadManagementService from "../../../orchestration-v2/ThreadManagementService.ts"; |
| 17 | import * as ProjectService from "../../../project/ProjectService.ts"; | 19 | import * as ProjectService from "../../../project/ProjectService.ts"; |
| 20 | import * as PullRequestReviewGuideService from "../../../pullRequest/PullRequestReviewGuideService.ts"; | ||
| 18 | 21 | ||
| 19 | const dependencies = [ | 22 | const dependencies = [ |
| 20 | McpInvocationContext.McpInvocationContext, | 23 | McpInvocationContext.McpInvocationContext, |
| ... | @@ -23,6 +26,11 @@ const dependencies = [ | ... | @@ -23,6 +26,11 @@ const dependencies = [ |
| 23 | ProjectService.ProjectService, | 26 | ProjectService.ProjectService, |
| 24 | ]; | 27 | ]; |
| 25 | 28 | ||
| 29 | const guideDependencies = [ | ||
| 30 | ...dependencies, | ||
| 31 | PullRequestReviewGuideService.PullRequestReviewGuideService, | ||
| 32 | ]; | ||
| 33 | |||
| 26 | const REGISTER_EVERY_PR = | 34 | const REGISTER_EVERY_PR = |
| 27 | "Register every pull request you open for this thread, including each layer of a stack, right after creating it."; | 35 | "Register every pull request you open for this thread, including each layer of a stack, right after creating it."; |
| 28 | 36 | ||
| ... | @@ -162,6 +170,24 @@ export class PullRequestListFailedError extends Schema.TaggedError<PullRequestLi | ... | @@ -162,6 +170,24 @@ export class PullRequestListFailedError extends Schema.TaggedError<PullRequestLi |
| 162 | } | 170 | } |
| 163 | } | 171 | } |
| 164 | 172 | ||
| 173 | export class PullRequestProjectNotFoundError extends Schema.TaggedError<PullRequestProjectNotFoundError>()( | ||
| 174 | "PullRequestProjectNotFoundError", | ||
| 175 | { repository: Schema.String }, | ||
| 176 | ) { | ||
| 177 | override get message(): string { | ||
| 178 | 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.`; | ||
| 179 | } | ||
| 180 | } | ||
| 181 | |||
| 182 | export class PullRequestGuideFailedError extends Schema.TaggedError<PullRequestGuideFailedError>()( | ||
| 183 | "PullRequestGuideFailedError", | ||
| 184 | { detail: Schema.String, cause: Schema.optional(Schema.Defect()) }, | ||
| 185 | ) { | ||
| 186 | override get message(): string { | ||
| 187 | return this.detail; | ||
| 188 | } | ||
| 189 | } | ||
| 190 | |||
| 165 | export const PullRequestToolError = Schema.Union([ | 191 | export const PullRequestToolError = Schema.Union([ |
| 166 | OrchestratorMcpFailure, | 192 | OrchestratorMcpFailure, |
| 167 | McpCapabilityUnavailableError, | 193 | McpCapabilityUnavailableError, |
| ... | @@ -176,6 +202,8 @@ export const PullRequestToolError = Schema.Union([ | ... | @@ -176,6 +202,8 @@ export const PullRequestToolError = Schema.Union([ |
| 176 | PullRequestWatchFailedError, | 202 | PullRequestWatchFailedError, |
| 177 | PullRequestNotOpenError, | 203 | PullRequestNotOpenError, |
| 178 | PullRequestWatchFromSubagentError, | 204 | PullRequestWatchFromSubagentError, |
| 205 | PullRequestProjectNotFoundError, | ||
| 206 | PullRequestGuideFailedError, | ||
| 179 | ]); | 207 | ]); |
| 180 | export type PullRequestToolError = typeof PullRequestToolError.Type; | 208 | export type PullRequestToolError = typeof PullRequestToolError.Type; |
| 181 | 209 | ||
| ... | @@ -319,10 +347,76 @@ const UnwatchPullRequestTool = Tool.make("unwatch_pull_request", { | ... | @@ -319,10 +347,76 @@ const UnwatchPullRequestTool = Tool.make("unwatch_pull_request", { |
| 319 | .annotate(Tool.Idempotent, true) | 347 | .annotate(Tool.Idempotent, true) |
| 320 | .annotate(Tool.OpenWorld, false); | 348 | .annotate(Tool.OpenWorld, false); |
| 321 | 349 | ||
| 350 | /** A pull request to write or read a guide for, by URL or by repository and number. */ | ||
| 351 | const GuideTarget = Schema.Struct({ | ||
| 352 | url: PullRequestTargetInput.fields.url, | ||
| 353 | repository: PullRequestTargetInput.fields.repository, | ||
| 354 | number: PullRequestTargetInput.fields.number, | ||
| 355 | host: PullRequestTargetInput.fields.host, | ||
| 356 | }); | ||
| 357 | |||
| 358 | const GuideOutcome = Schema.Struct({ | ||
| 359 | ...PullRequestIdentity, | ||
| 360 | outcome: Schema.Literals(["started", "running", "failed"]).annotate({ | ||
| 361 | description: | ||
| 362 | "started: a run began. running: one was already going for this head and the call joined it. failed: this pull request could not be guided; see detail.", | ||
| 363 | }), | ||
| 364 | detail: Schema.NullOr(Schema.String), | ||
| 365 | }); | ||
| 366 | |||
| 367 | export const WritePullRequestGuidesResult = Schema.Struct({ | ||
| 368 | guides: Schema.Array(GuideOutcome), | ||
| 369 | }); | ||
| 370 | export type WritePullRequestGuidesResult = typeof WritePullRequestGuidesResult.Type; | ||
| 371 | |||
| 372 | export const ReadPullRequestGuideResult = Schema.Struct({ | ||
| 373 | ...PullRequestIdentity, | ||
| 374 | guide: Schema.NullOr(PullRequestReviewGuide), | ||
| 375 | run: Schema.NullOr(PullRequestReviewGuideRun), | ||
| 376 | currentHeadSha: Schema.NullOr(Schema.String), | ||
| 377 | stale: Schema.Boolean.annotate({ | ||
| 378 | description: | ||
| 379 | "True when the guide was written for an earlier head than the pull request has now.", | ||
| 380 | }), | ||
| 381 | }); | ||
| 382 | export type ReadPullRequestGuideResult = typeof ReadPullRequestGuideResult.Type; | ||
| 383 | |||
| 384 | const WritePullRequestGuidesTool = Tool.make("write_pull_request_guides", { | ||
| 385 | description: | ||
| 386 | "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.", | ||
| 387 | parameters: Schema.Struct({ | ||
| 388 | pullRequests: Schema.Array(GuideTarget).check(Schema.isMinLength(1), Schema.isMaxLength(20)), | ||
| 389 | }), | ||
| 390 | success: WritePullRequestGuidesResult, | ||
| 391 | failure: PullRequestToolError, | ||
| 392 | dependencies: guideDependencies, | ||
| 393 | }) | ||
| 394 | .annotate(Tool.Title, "Write pull request guides") | ||
| 395 | .annotate(Tool.Readonly, false) | ||
| 396 | .annotate(Tool.Destructive, false) | ||
| 397 | .annotate(Tool.Idempotent, false) | ||
| 398 | .annotate(Tool.OpenWorld, true); | ||
| 399 | |||
| 400 | const ReadPullRequestGuideTool = Tool.make("read_pull_request_guide", { | ||
| 401 | description: | ||
| 402 | "Read a pull request's stored review guide and its run: run.Running while one is being written, run.Failed with the reason when the last one failed, guide null when none was written. stale is true after a push the guide has not caught up with. Never starts a run.", | ||
| 403 | parameters: GuideTarget, | ||
| 404 | success: ReadPullRequestGuideResult, | ||
| 405 | failure: PullRequestToolError, | ||
| 406 | dependencies: guideDependencies, | ||
| 407 | }) | ||
| 408 | .annotate(Tool.Title, "Read pull request guide") | ||
| 409 | .annotate(Tool.Readonly, true) | ||
| 410 | .annotate(Tool.Destructive, false) | ||
| 411 | .annotate(Tool.Idempotent, true) | ||
| 412 | .annotate(Tool.OpenWorld, true); | ||
| 413 | |||
| 322 | export const PullRequestsToolkit = Toolkit.make( | 414 | export const PullRequestsToolkit = Toolkit.make( |
| 323 | LinkPullRequestTool, | 415 | LinkPullRequestTool, |
| 324 | UnlinkPullRequestTool, | 416 | UnlinkPullRequestTool, |
| 325 | ListThreadPullRequestsTool, | 417 | ListThreadPullRequestsTool, |
| 326 | WatchPullRequestTool, | 418 | WatchPullRequestTool, |
| 327 | UnwatchPullRequestTool, | 419 | UnwatchPullRequestTool, |
| 420 | WritePullRequestGuidesTool, | ||
| 421 | ReadPullRequestGuideTool, | ||
| 328 | ); | 422 | ); |
apps/server/src/mcp/toolkits/worktree/registration.test.ts+2| ... | @@ -16,6 +16,7 @@ import * as GitWorkflowService from "../../../git/GitWorkflowService.ts"; | ... | @@ -16,6 +16,7 @@ import * as GitWorkflowService from "../../../git/GitWorkflowService.ts"; |
| 16 | import * as ProviderAdapterRegistry from "../../../orchestration-v2/ProviderAdapterRegistry.ts"; | 16 | import * as ProviderAdapterRegistry from "../../../orchestration-v2/ProviderAdapterRegistry.ts"; |
| 17 | import * as ThreadManagementService from "../../../orchestration-v2/ThreadManagementService.ts"; | 17 | import * as ThreadManagementService from "../../../orchestration-v2/ThreadManagementService.ts"; |
| 18 | import * as ProjectService from "../../../project/ProjectService.ts"; | 18 | import * as ProjectService from "../../../project/ProjectService.ts"; |
| 19 | import * as PullRequestReviewGuideService from "../../../pullRequest/PullRequestReviewGuideService.ts"; | ||
| 19 | import * as ProjectSetupScriptRunner from "../../../project/ProjectSetupScriptRunner.ts"; | 20 | import * as ProjectSetupScriptRunner from "../../../project/ProjectSetupScriptRunner.ts"; |
| 20 | import * as ProviderRegistry from "../../../provider/ProviderRegistry.ts"; | 21 | import * as ProviderRegistry from "../../../provider/ProviderRegistry.ts"; |
| 21 | import * as ScheduledTaskService from "../../../scheduledTasks/ScheduledTaskService.ts"; | 22 | import * as ScheduledTaskService from "../../../scheduledTasks/ScheduledTaskService.ts"; |
| ... | @@ -44,6 +45,7 @@ const layerStubServices = Layer.mergeAll( | ... | @@ -44,6 +45,7 @@ const layerStubServices = Layer.mergeAll( |
| 44 | Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), | 45 | Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), |
| 45 | Layer.mock(SecretRequests.SecretRequests)({}), | 46 | Layer.mock(SecretRequests.SecretRequests)({}), |
| 46 | Layer.mock(ProjectService.ProjectService)({}), | 47 | Layer.mock(ProjectService.ProjectService)({}), |
| 48 | Layer.mock(PullRequestReviewGuideService.PullRequestReviewGuideService)({}), | ||
| 47 | ServerSettings.layerTest({}), | 49 | ServerSettings.layerTest({}), |
| 48 | Layer.mock(GitWorkflowService.GitWorkflowService)({}), | 50 | Layer.mock(GitWorkflowService.GitWorkflowService)({}), |
| 49 | Layer.mock(ProjectSetupScriptRunner.ProjectSetupScriptRunner)({}), | 51 | Layer.mock(ProjectSetupScriptRunner.ProjectSetupScriptRunner)({}), |
apps/server/src/pullRequest/PullRequestReviewGuideService.test.ts+3-1| ... | @@ -303,6 +303,7 @@ it.layer(layerBase, { excludeTestServices: true })("PullRequestReviewGuideServic | ... | @@ -303,6 +303,7 @@ it.layer(layerBase, { excludeTestServices: true })("PullRequestReviewGuideServic |
| 303 | const started = yield* service.generate(reference); | 303 | const started = yield* service.generate(reference); |
| 304 | assert.strictEqual(started.run?._tag, "Running"); | 304 | assert.strictEqual(started.run?._tag, "Running"); |
| 305 | assert.strictEqual(started.guide, null); | 305 | assert.strictEqual(started.guide, null); |
| 306 | assert.isTrue(started.started); | ||
| 306 | 307 | ||
| 307 | // A run that fails before reaching the model reports why rather than leaving this waiting. | 308 | // A run that fails before reaching the model reports why rather than leaving this waiting. |
| 308 | const failedRun = service.get(reference).pipe( | 309 | const failedRun = service.get(reference).pipe( |
| ... | @@ -329,7 +330,8 @@ it.layer(layerBase, { excludeTestServices: true })("PullRequestReviewGuideServic | ... | @@ -329,7 +330,8 @@ it.layer(layerBase, { excludeTestServices: true })("PullRequestReviewGuideServic |
| 329 | 330 | ||
| 330 | // A second press while running joins the run instead of starting another. | 331 | // A second press while running joins the run instead of starting another. |
| 331 | const joined = yield* service.generate(reference); | 332 | const joined = yield* service.generate(reference); |
| 332 | assert.strictEqual(joined.run?._tag, "Running"); | 333 | assert.strictEqual(joined.run, started.run); |
| 334 | assert.isFalse(joined.started); | ||
| 333 | 335 | ||
| 334 | yield* Deferred.succeed(finish, undefined); | 336 | yield* Deferred.succeed(finish, undefined); |
| 335 | yield* Deferred.await(stored); | 337 | yield* Deferred.await(stored); |
apps/server/src/pullRequest/PullRequestReviewGuideService.ts+8-4| ... | @@ -62,11 +62,15 @@ export class PullRequestReviewGuideService extends Context.Service< | ... | @@ -62,11 +62,15 @@ export class PullRequestReviewGuideService extends Context.Service< |
| 62 | /** | 62 | /** |
| 63 | * Start writing a guide for the current head and return once it is under way. Joins a run | 63 | * Start writing a guide for the current head and return once it is under way. Joins a run |
| 64 | * already going for that head. A stored guide stays readable until the new one replaces it, | 64 | * already going for that head. A stored guide stays readable until the new one replaces it, |
| 65 | * so a reader can rewrite one after changing the guide model or instructions. | 65 | * so a reader can rewrite one after changing the guide model or instructions. `started` says |
| 66 | * whether this call began the run or joined one already going. | ||
| 66 | */ | 67 | */ |
| 67 | readonly generate: ( | 68 | readonly generate: ( |
| 68 | input: PullRequestRef, | 69 | input: PullRequestRef, |
| 69 | ) => Effect.Effect<PullRequestReviewGuideResult, PullRequestService.PullRequestError>; | 70 | ) => Effect.Effect< |
| 71 | PullRequestReviewGuideResult & { readonly started: boolean }, | ||
| 72 | PullRequestService.PullRequestError | ||
| 73 | >; | ||
| 70 | } | 74 | } |
| 71 | >()("t3/pullRequest/PullRequestReviewGuideService") {} | 75 | >()("t3/pullRequest/PullRequestReviewGuideService") {} |
| 72 | 76 | ||
| ... | @@ -439,7 +443,7 @@ const make = Effect.gen(function* () { | ... | @@ -439,7 +443,7 @@ const make = Effect.gen(function* () { |
| 439 | ), | 443 | ), |
| 440 | ), | 444 | ), |
| 441 | ); | 445 | ); |
| 442 | if (claimed !== running) return { guide: stored, run: claimed, currentHeadSha }; | 446 | if (claimed !== running) return { guide: stored, run: claimed, currentHeadSha, started: false }; |
| 443 | 447 | ||
| 444 | // The run is identified by its own Running value: a later claim replaces it, and from then | 448 | // The run is identified by its own Running value: a later claim replaces it, and from then |
| 445 | // on this run's completion leaves the newer run's state and guide alone. | 449 | // on this run's completion leaves the newer run's state and guide alone. |
| ... | @@ -475,7 +479,7 @@ const make = Effect.gen(function* () { | ... | @@ -475,7 +479,7 @@ const make = Effect.gen(function* () { |
| 475 | ), | 479 | ), |
| 476 | Effect.forkIn(serviceScope), | 480 | Effect.forkIn(serviceScope), |
| 477 | ); | 481 | ); |
| 478 | return { guide: stored, run: running, currentHeadSha }; | 482 | return { guide: stored, run: running, currentHeadSha, started: true }; |
| 479 | }); | 483 | }); |
| 480 | 484 | ||
| 481 | return PullRequestReviewGuideService.of({ get, generate }); | 485 | return PullRequestReviewGuideService.of({ get, generate }); |
docs/user/source-control.md+3| ... | @@ -209,6 +209,9 @@ about them; comments wait with the ones from the **Code** tab until you send the | ... | @@ -209,6 +209,9 @@ about them; comments wait with the ones from the **Code** tab until you send the |
| 209 | suggested question, or **Ask about this stop**, to open a thread that already knows which stop you | 209 | suggested question, or **Ask about this stop**, to open a thread that already knows which stop you |
| 210 | are on. **Finish review** on the last stop opens the review with everything you collected. | 210 | are on. **Finish review** on the last stop opens the review with everything you collected. |
| 211 | 211 | ||
| 212 | Agents can write guides too: ask one to prepare guides for a set of pull requests, and it starts | ||
| 213 | them for you. Each is read through the project whose checkout has that repository. | ||
| 214 | |||
| 212 | To shape guides to your taste, write **Review guide instructions** in Settings → Source Control, | 215 | To shape guides to your taste, write **Review guide instructions** in Settings → Source Control, |
| 213 | such as what to show first or what to skip; projects can override them. They change emphasis and | 216 | such as what to show first or what to skip; projects can override them. They change emphasis and |
| 214 | ordering, not the guide's layout or which files it covers. | 217 | ordering, not the guide's layout or which files it covers. |
packages/client-runtime/src/t3ToolSummary.test.ts+16| ... | @@ -28,6 +28,22 @@ describe("summarizeT3ToolCalls", () => { | ... | @@ -28,6 +28,22 @@ describe("summarizeT3ToolCalls", () => { |
| 28 | ).toBe("Created 1 thread"); | 28 | ).toBe("Created 1 thread"); |
| 29 | }); | 29 | }); |
| 30 | 30 | ||
| 31 | it("reports review guides by outcome, and a call where none started as a failure", () => { | ||
| 32 | const guides = (...outcomes: ReadonlyArray<string>) => ({ | ||
| 33 | guides: outcomes.map((outcome, index) => ({ number: index + 1, outcome })), | ||
| 34 | }); | ||
| 35 | expect( | ||
| 36 | summarizeT3ToolCalls("write-pr-guides", [ | ||
| 37 | completed({}, guides("started", "running", "failed")), | ||
| 38 | ]), | ||
| 39 | ).toEqual({ label: "Started 2 review guides, 1 failed", failedCount: 1 }); | ||
| 40 | const allFailed = summarizeT3ToolCalls("write-pr-guides", [ | ||
| 41 | completed({}, guides("failed", "failed")), | ||
| 42 | ]); | ||
| 43 | expect(allFailed.label).toBe("Could not start 2 review guides"); | ||
| 44 | expect(allFailed.failedCount).toBe(2); | ||
| 45 | }); | ||
| 46 | |||
| 31 | it.each([ | 47 | it.each([ |
| 32 | ["queue-read", "Read 1 queued message"], | 48 | ["queue-read", "Read 1 queued message"], |
| 33 | ["queue-edit", "Edited 1 queued message"], | 49 | ["queue-edit", "Edited 1 queued message"], |
packages/client-runtime/src/t3ToolSummary.ts+27-1| ... | @@ -94,7 +94,7 @@ export function summarizeT3ToolCalls( | ... | @@ -94,7 +94,7 @@ export function summarizeT3ToolCalls( |
| 94 | }; | 94 | }; |
| 95 | }); | 95 | }); |
| 96 | const completed = results.filter((call) => call.outcome === "completed"); | 96 | const completed = results.filter((call) => call.outcome === "completed"); |
| 97 | const failedCount = results.filter((call) => call.outcome === "failed").length; | 97 | let failedCount = results.filter((call) => call.outcome === "failed").length; |
| 98 | const selected = completed.length > 0 ? completed : results; | 98 | const selected = completed.length > 0 ? completed : results; |
| 99 | const times = quantity(selected.length, "time"); | 99 | const times = quantity(selected.length, "time"); |
| 100 | const phrase = (past: string, infinitive: string, object: string) => | 100 | const phrase = (past: string, infinitive: string, object: string) => |
| ... | @@ -399,6 +399,32 @@ export function summarizeT3ToolCalls( | ... | @@ -399,6 +399,32 @@ export function summarizeT3ToolCalls( |
| 399 | `linked pull requests${selected.length === 1 ? "" : ` ${times}`}`, | 399 | `linked pull requests${selected.length === 1 ? "" : ` ${times}`}`, |
| 400 | ); | 400 | ); |
| 401 | break; | 401 | break; |
| 402 | case "write-pr-guides": { | ||
| 403 | // One call writes many guides, and reports a refusal per guide rather than failing. | ||
| 404 | const outcomes = selected.flatMap((call) => | ||
| 405 | Array.isArray(call.output?.guides) | ||
| 406 | ? call.output.guides.map((guide) => asRecord(guide)?.outcome) | ||
| 407 | : [], | ||
| 408 | ); | ||
| 409 | const started = outcomes.filter((outcome) => outcome !== "failed").length; | ||
| 410 | const failed = outcomes.length - started; | ||
| 411 | failedCount += failed; | ||
| 412 | if (outcomes.length > 0 && started === 0) { | ||
| 413 | label = `Could not start ${quantity(failed, "review guide")}`; | ||
| 414 | } else { | ||
| 415 | label = phrase( | ||
| 416 | "Started", | ||
| 417 | "start", | ||
| 418 | outcomes.length === 0 | ||
| 419 | ? `review guides ${times}` | ||
| 420 | : `${quantity(started, "review guide")}${failed > 0 ? `, ${failed} failed` : ""}`, | ||
| 421 | ); | ||
| 422 | } | ||
| 423 | break; | ||
| 424 | } | ||
| 425 | case "read-pr-guide": | ||
| 426 | label = phrase("Read", "read", quantity(selected.length, "review guide")); | ||
| 427 | break; | ||
| 402 | case "browser": | 428 | case "browser": |
| 403 | label = phrase("Used", "use", `browser ${times}`); | 429 | label = phrase("Used", "use", `browser ${times}`); |
| 404 | break; | 430 | break; |
packages/shared/src/t3McpToolPresentation.ts+12| ... | @@ -59,6 +59,8 @@ export type T3McpToolSummaryAction = | ... | @@ -59,6 +59,8 @@ export type T3McpToolSummaryAction = |
| 59 | | "list-prs" | 59 | | "list-prs" |
| 60 | | "watch-pr" | 60 | | "watch-pr" |
| 61 | | "unwatch-pr" | 61 | | "unwatch-pr" |
| 62 | | "write-pr-guides" | ||
| 63 | | "read-pr-guide" | ||
| 62 | | "browser" | 64 | | "browser" |
| 63 | | "device" | 65 | | "device" |
| 64 | | "html-preview" | 66 | | "html-preview" |
| ... | @@ -109,6 +111,16 @@ const T3_MCP_TOOLS: Readonly<Record<string, T3McpToolDefinition>> = { | ... | @@ -109,6 +111,16 @@ const T3_MCP_TOOLS: Readonly<Record<string, T3McpToolDefinition>> = { |
| 109 | "unwatch-pr", | 111 | "unwatch-pr", |
| 110 | "pull-request", | 112 | "pull-request", |
| 111 | ), | 113 | ), |
| 114 | write_pull_request_guides: tool( | ||
| 115 | ["Write", "Writing", "Started", "review guides"], | ||
| 116 | "write-pr-guides", | ||
| 117 | "pull-request", | ||
| 118 | ), | ||
| 119 | read_pull_request_guide: tool( | ||
| 120 | ["Read", "Reading", "Read", "a review guide"], | ||
| 121 | "read-pr-guide", | ||
| 122 | "pull-request", | ||
| 123 | ), | ||
| 112 | orchestrator_capabilities: tool( | 124 | orchestrator_capabilities: tool( |
| 113 | ["Get", "Getting", "Got", "orchestration capabilities"], | 125 | ["Get", "Getting", "Got", "orchestration capabilities"], |
| 114 | "capabilities", | 126 | "capabilities", |