History

fix(mcp): preserve OpenCode file attachments

Julius Marminge committed 2 months ago

Commit 0d93b711 parent 181502a5 files · +93 −33Browse files

Unified · Split

5 files
  1. apps/server/src/mcp/OrchestratorMcpService.test.ts +36 −11
  2. apps/server/src/mcp/OrchestratorMcpService.ts +1 −3
  3. apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.ts +6 −19
  4. apps/server/src/provider/opencodeRuntime.cliParsers.test.ts +33 −0
  5. apps/server/src/provider/opencodeRuntime.ts +17 −0
apps/server/src/mcp/OrchestratorMcpService.test.ts+36−11
@@ -12,6 +12,7 @@ import { · 11 unmodified lines
12 type OrchestrationV2ThreadProjection,12 type OrchestrationV2ThreadProjection,
13} from "@t3tools/contracts";13} from "@t3tools/contracts";
14import * as DateTime from "effect/DateTime";14import * as DateTime from "effect/DateTime";
15import * as Deferred from "effect/Deferred";
15import * as Effect from "effect/Effect";16import * as Effect from "effect/Effect";
16import * as Layer from "effect/Layer";17import * as Layer from "effect/Layer";
17import * as Option from "effect/Option";18import * as Option from "effect/Option";
@@ -53,6 +54,9 @@ describe("OrchestratorMcpService", () => { · 35 unmodified lines
53 messages: [{ id: messageId, runId, attachments: [], text: "Accepted once." }],54 messages: [{ id: messageId, runId, attachments: [], text: "Accepted once." }],
54 } as unknown as OrchestrationV2ThreadProjection;55 } as unknown as OrchestrationV2ThreadProjection;
55 const projectionReads = yield* Ref.make(0);56 const projectionReads = yield* Ref.make(0);
57 const committed = yield* Ref.make(false);
58 const receiptRequested = yield* Deferred.make<void>();
59 const releaseReceipt = yield* Deferred.make<void>();
56 const dependencies = Layer.mergeAll(60 const dependencies = Layer.mergeAll(
57 NodeServices.layer,61 NodeServices.layer,
58 ServerConfig.layerTest(process.cwd(), {62 ServerConfig.layerTest(process.cwd(), {
@@ -61,11 +65,21 @@ describe("OrchestratorMcpService", () => { · 2 unmodified lines
61 Layer.mock(ThreadManagementService)({65 Layer.mock(ThreadManagementService)({
62 getThreadProjection: () =>66 getThreadProjection: () =>
63 Ref.updateAndGet(projectionReads, (count) => count + 1).pipe(67 Ref.updateAndGet(projectionReads, (count) => count + 1).pipe(
64 Effect.map((count) => (count === 1 ? staleProjection : acceptedProjection)),68 Effect.flatMap((count) =>
69 count === 1
70 ? Effect.succeed(staleProjection)
71 : Ref.get(committed).pipe(
72 Effect.map((isCommitted) =>
73 isCommitted ? acceptedProjection : staleProjection,
74 ),
75 ),
76 ),
65 ),77 ),
66 getCommandReceipt: () =>78 getCommandReceipt: () =>
67 Effect.succeed(79 Effect.gen(function* () {
68 Option.some({80 yield* Deferred.succeed(receiptRequested, undefined);
81 yield* Deferred.await(releaseReceipt);
82 return Option.some({
69 commandId: CommandId.make("command:mcp-send-replay-race"),83 commandId: CommandId.make("command:mcp-send-replay-race"),
70 threadId,84 threadId,
71 commandType: "message.dispatch",85 commandType: "message.dispatch",
@@ -73,8 +87,8 @@ describe("OrchestratorMcpService", () => { · 1 unmodified line
73 resultSequence: 2,87 resultSequence: 2,
74 status: "accepted",88 status: "accepted",
75 error: null,89 error: null,
76 }),90 });
77 ),91 }),
78 dispatch: () => Effect.die("Accepted send replay must not dispatch again."),92 dispatch: () => Effect.die("Accepted send replay must not dispatch again."),
79 }),93 }),
80 Layer.mock(ProviderRegistry)({ getProviders: Effect.die("Provider lookup is mutable.") }),94 Layer.mock(ProviderRegistry)({ getProviders: Effect.die("Provider lookup is mutable.") }),
@@ -89,18 +103,29 @@ describe("OrchestratorMcpService", () => { · 8 unmodified lines
89 issuedAt: 1,103 issuedAt: 1,
90 };104 };
91 105
92 yield* Effect.gen(function* () {106 const call = Effect.gen(function* () {
93 const service = yield* OrchestratorMcpService.OrchestratorMcpService;107 const service = yield* OrchestratorMcpService.OrchestratorMcpService;
94 const result = yield* service.sendToThread(scope, {108 return yield* service.sendToThread(scope, {
95 threadId,109 threadId,
96 message: "Accepted once.",110 message: "Accepted once.",
97 clientRequestId: requestKey,111 clientRequestId: requestKey,
98 });112 });
99 assert.equal(result.messageId, messageId);
100 assert.equal(result.runId, runId);
101 assert.equal(result.delivery, "queued");
102 assert.equal(yield* Ref.get(projectionReads), 2);
103 }).pipe(Effect.provide(OrchestratorMcpService.layer.pipe(Layer.provide(dependencies))));113 }).pipe(Effect.provide(OrchestratorMcpService.layer.pipe(Layer.provide(dependencies))));
114 const [result] = yield* Effect.all(
115 [
116 call,
117 Effect.gen(function* () {
118 yield* Deferred.await(receiptRequested);
119 yield* Ref.set(committed, true);
120 yield* Deferred.succeed(releaseReceipt, undefined);
121 }),
122 ],
123 { concurrency: "unbounded" },
124 );
125 assert.equal(result.messageId, messageId);
126 assert.equal(result.runId, runId);
127 assert.equal(result.delivery, "queued");
128 assert.equal(yield* Ref.get(projectionReads), 2);
104 }),129 }),
105 );130 );
106 131
apps/server/src/mcp/OrchestratorMcpService.ts+1−3
@@ -79,7 +79,6 @@ import { · 78 unmodified lines
79 ThreadManagementService,79 ThreadManagementService,
80} from "../orchestration-v2/ThreadManagementService.ts";80} from "../orchestration-v2/ThreadManagementService.ts";
81import { ProviderRegistry } from "../provider/Services/ProviderRegistry.ts";81import { ProviderRegistry } from "../provider/Services/ProviderRegistry.ts";
82import { isOpenCodeNativeFilePart } from "../provider/opencodeRuntime.ts";
83import { ScheduledTaskService } from "../scheduledTasks/ScheduledTaskService.ts";82import { ScheduledTaskService } from "../scheduledTasks/ScheduledTaskService.ts";
84import * as ServerConfig from "../config.ts";83import * as ServerConfig from "../config.ts";
85import type { McpInvocationScope } from "./McpInvocationContext.ts";84import type { McpInvocationScope } from "./McpInvocationContext.ts";
@@ -195,8 +194,7 @@ function validateProviderAttachments( · 109 unmodified lines
195 (attachment.type !== "image" && attachment.type !== "file") ||194 (attachment.type !== "image" && attachment.type !== "file") ||
196 !kinds.includes(attachment.type) ||195 !kinds.includes(attachment.type) ||
197 (attachment.type === "image" &&196 (attachment.type === "image" &&
198 !isProviderSendTurnSupportedImageMimeType(attachment.mimeType)) ||197 !isProviderSendTurnSupportedImageMimeType(attachment.mimeType)),
199 (provider.driver === "opencode" && !isOpenCodeNativeFilePart(attachment)),
200 );198 );
201 return unsupported === undefined199 return unsupported === undefined
202 ? Effect.void200 ? Effect.void
apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.ts+6−19
@@ -71,7 +71,7 @@ import { · 70 unmodified lines
71 openCodeRuntimeErrorDetail,71 openCodeRuntimeErrorDetail,
72 parseOpenCodeModelSlug,72 parseOpenCodeModelSlug,
73 runOpenCodeSdk,73 runOpenCodeSdk,
74 toOpenCodeFileParts,74 toOpenCodePromptParts,
75 toOpenCodePermissionReply,75 toOpenCodePermissionReply,
76 toOpenCodeQuestionAnswers,76 toOpenCodeQuestionAnswers,
77 type OpenCodeRuntimeShape,77 type OpenCodeRuntimeShape,
@@ -79,7 +79,6 @@ import { · 1 unmodified line
79import { IdAllocatorV2, type IdAllocatorV2Shape } from "../IdAllocator.ts";79import { IdAllocatorV2, type IdAllocatorV2Shape } from "../IdAllocator.ts";
80import { makeProviderFailure } from "../ProviderFailure.ts";80import { makeProviderFailure } from "../ProviderFailure.ts";
81import { turnScopedSelectionTransition } from "../ProviderSelectionTransition.ts";81import { turnScopedSelectionTransition } from "../ProviderSelectionTransition.ts";
82import { providerMessageTextWithAttachmentPaths } from "../AttachmentPrompt.ts";
83import {82import {
84 ProviderAdapterEnsureThreadError,83 ProviderAdapterEnsureThreadError,
85 ProviderAdapterForkThreadError,84 ProviderAdapterForkThreadError,
@@ -2629,20 +2628,16 @@ export function makeOpenCodeAdapterV2(options: OpenCodeAdapterV2Options): Provid · 2543 unmodified lines
2629 };2628 };
2630 2629
2631 const resolvePromptParts = (turnInput: ProviderAdapterV2TurnInput) => {2630 const resolvePromptParts = (turnInput: ProviderAdapterV2TurnInput) => {
2632 const text = providerMessageTextWithAttachmentPaths({2631 const parts = toOpenCodePromptParts({
2633 text: turnInput.message.text,2632 text: turnInput.message.text,
2634 attachments: turnInput.message.attachments,
2635 attachmentsDir: serverConfig.attachmentsDir,
2636 }).trim();
2637 const files = toOpenCodeFileParts({
2638 attachments: turnInput.message.attachments,2633 attachments: turnInput.message.attachments,
2639 resolveAttachmentPath: (attachment) =>2634 resolveAttachmentPath: (attachment) =>
2640 resolveAttachmentPath({ attachmentsDir: serverConfig.attachmentsDir, attachment }),2635 resolveAttachmentPath({ attachmentsDir: serverConfig.attachmentsDir, attachment }),
2641 });2636 });
2642 if (text.length === 0 && files.length === 0) {2637 if (parts.length === 0) {
2643 throw protocolError("OpenCode turns require text or at least one valid attachment");2638 throw protocolError("OpenCode turns require text or at least one valid attachment");
2644 }2639 }
2645 return [...(text.length === 0 ? [] : [{ type: "text" as const, text }]), ...files];2640 return parts;
2646 };2641 };
2647 2642
2648 const readSnapshot = Effect.fnUntraced(function* (2643 const readSnapshot = Effect.fnUntraced(function* (
@@ -2997,12 +2992,8 @@ export function makeOpenCodeAdapterV2(options: OpenCodeAdapterV2Options): Provid · 348 unmodified lines
2997 `OpenCode model '${turn.modelSelection.model}' must use provider/model format`,2992 `OpenCode model '${turn.modelSelection.model}' must use provider/model format`,
2998 );2993 );
2999 }2994 }
3000 const text = providerMessageTextWithAttachmentPaths({2995 const parts = toOpenCodePromptParts({
3001 text: steerInput.message.text,2996 text: steerInput.message.text,
3002 attachments: steerInput.message.attachments,
3003 attachmentsDir: serverConfig.attachmentsDir,
3004 }).trim();
3005 const files = toOpenCodeFileParts({
3006 attachments: steerInput.message.attachments,2997 attachments: steerInput.message.attachments,
3007 resolveAttachmentPath: (attachment) =>2998 resolveAttachmentPath: (attachment) =>
3008 resolveAttachmentPath({2999 resolveAttachmentPath({
@@ -3010,13 +3001,9 @@ export function makeOpenCodeAdapterV2(options: OpenCodeAdapterV2Options): Provid · 1 unmodified line
3010 attachment,3001 attachment,
3011 }),3002 }),
3012 });3003 });
3013 if (text.length === 0 && files.length === 0) {3004 if (parts.length === 0) {
3014 return yield* protocolError("OpenCode steering requires text or an attachment");3005 return yield* protocolError("OpenCode steering requires text or an attachment");
3015 }3006 }
3016 const parts = [
3017 ...(text.length === 0 ? [] : [{ type: "text" as const, text }]),
3018 ...files,
3019 ];
3020 turn.admissionGeneration = state.nextAdmissionGeneration++;3007 turn.admissionGeneration = state.nextAdmissionGeneration++;
3021 turn.admissionMessageId = yield* makeOpenCodeMessageId();3008 turn.admissionMessageId = yield* makeOpenCodeMessageId();
3022 turn.admissionPending = true;3009 turn.admissionPending = true;
apps/server/src/provider/opencodeRuntime.cliParsers.test.ts+33−0
@@ -7,6 +7,7 @@ import { · 6 unmodified lines
7 parseModelsCliOutput,7 parseModelsCliOutput,
8 parseSkillsCliOutput,8 parseSkillsCliOutput,
9 toOpenCodeFileParts,9 toOpenCodeFileParts,
10 toOpenCodePromptParts,
10} from "./opencodeRuntime.ts";11} from "./opencodeRuntime.ts";
11 12
12describe("parseModelsCliOutput", () => {13describe("parseModelsCliOutput", () => {
@@ -318,4 +319,36 @@ describe("toOpenCodeFileParts", () => { · 305 unmodified lines
318 ["application/pdf", "text/markdown", "image/png"],319 ["application/pdf", "text/markdown", "image/png"],
319 );320 );
320 });321 });
322
323 it("keeps non-native and oversized files available through resolved paths", () => {
324 const paths = new Map([
325 ["oversized.pdf", "/tmp/oversized.pdf"],
326 ["archive.zip", "/tmp/archive.zip"],
327 ["vector.svg", "/tmp/vector.svg"],
328 ["native.png", "/tmp/native.png"],
329 ]);
330 const parts = toOpenCodePromptParts({
331 text: "Inspect every attachment.",
332 attachments: [
333 { ...attachment("application/pdf", 25 * 1024 * 1024), name: "oversized.pdf" },
334 { ...attachment("application/zip"), name: "archive.zip" },
335 { ...attachment("image/svg+xml"), name: "vector.svg" },
336 { ...attachment("image/png"), name: "native.png" },
337 ],
338 resolveAttachmentPath: (candidate) => paths.get(candidate.name) ?? null,
339 });
340
341 NodeAssert.deepEqual(parts.slice(1), [
342 {
343 type: "file",
344 mime: "image/png",
345 filename: "native.png",
346 url: "file:///tmp/native.png",
347 },
348 ]);
349 NodeAssert.match(parts[0]?.type === "text" ? parts[0].text : "", /oversized\.pdf/);
350 NodeAssert.match(parts[0]?.type === "text" ? parts[0].text : "", /archive\.zip/);
351 NodeAssert.match(parts[0]?.type === "text" ? parts[0].text : "", /vector\.svg/);
352 NodeAssert.doesNotMatch(parts[0]?.type === "text" ? parts[0].text : "", /native\.png/);
353 });
321});354});
apps/server/src/provider/opencodeRuntime.ts+17−0
@@ -481,6 +481,23 @@ export function toOpenCodeFileParts(input: { · 480 unmodified lines
481 return parts;481 return parts;
482}482}
483 483
484export function toOpenCodePromptParts(input: {
485 readonly text: string;
486 readonly attachments: ReadonlyArray<ChatAttachment> | undefined;
487 readonly resolveAttachmentPath: (attachment: ChatAttachment) => string | null;
488}) {
489 const nativeFiles = toOpenCodeFileParts(input);
490 const fallbackPaths = (input.attachments ?? []).flatMap((attachment) => {
491 if (isOpenCodeNativeFilePart(attachment)) return [];
492 const attachmentPath = input.resolveAttachmentPath(attachment);
493 return attachmentPath === null
494 ? []
495 : [`[Attached ${attachment.type} "${attachment.name}" is saved at: ${attachmentPath}]`];
496 });
497 const text = [input.text.trim(), fallbackPaths.join("\n")].filter(Boolean).join("\n\n");
498 return [...(text.length === 0 ? [] : [{ type: "text" as const, text }]), ...nativeFiles];
499}
500
484export function buildOpenCodePermissionRules(runtimeMode: RuntimeMode): PermissionRuleset {501export function buildOpenCodePermissionRules(runtimeMode: RuntimeMode): PermissionRuleset {
485 if (runtimeMode === "full-access") {502 if (runtimeMode === "full-access") {
486 return [503 return [