Fix Kiro ACP image attachment errors

Author: declancowen

.reviews/kiro-provider-appearance-review.md

@@ -6,14 +6,15 @@
 | -------------- | ---------------------------------------- |
 | **Repository** | `declancowen/t3code`                     |
 | **Remote**     | `origin`                                 |
-| **Branch**     | `main`                                   |
+| **Branch**     | `merge-upstream-main-check`              |
 | **Stack**      | TypeScript, Effect, React/Vite, Electron |
 
 ## Scope
 
 - `apps/server/src/provider/acp/StandardAcpAdapter.ts` — ACP prompt lifecycle and active-prompt steering.
 - `apps/server/src/provider/Layers/KiroAdapter.ts` — Kiro `_message/send` payload mapping.
 - `packages/effect-acp/src/protocol.ts` — ACP JSON-RPC transport compatibility for provider-originated requests.
+- `packages/effect-acp/src/client.test.ts`, `packages/effect-acp/src/protocol.test.ts` — ACP transport error-channel regression coverage.
 - `apps/server/src/provider/acp/AcpAdapterSupport.ts`, `apps/server/src/provider/acp/AcpRuntimeModel.ts` — ACP permission outcome mapping and tool-call classification.
 - `apps/server/src/provider/acp/StandardAcpAdapter.test.ts` — ACP steering regression coverage.
 - `apps/web/src/components/ChatView.tsx` — running-turn image send guard removal.
@@ -26,6 +27,7 @@
 - Kiro cancel behavior because Kiro currently rejects `session/cancel`.
 - ACP permission requests with provider-owned UUID request IDs and provider-owned option IDs.
 - Active-prompt steering payload compatibility for text and image attachments.
+- ACP JSON-RPC response error normalization and provider error data surfacing.
 - Running-turn UI send behavior across provider adapters.
 - Sidebar/translucency surface consistency across route wrappers.
 - macOS app icon visual bounds, corner radius, and generated package assets.
@@ -35,12 +37,66 @@
 | Field                 | Value                |
 | --------------------- | -------------------- |
 | **Review started**    | 2026-05-20           |
-| **Last reviewed**     | 2026-05-21 07:08 BST |
-| **Total turns**       | 8                    |
+| **Last reviewed**     | 2026-05-23 21:24 BST |
+| **Total turns**       | 9                    |
 | **Open findings**     | 0                    |
 | **Resolved findings** | 6                    |
 | **Accepted findings** | 0                    |
 
+## Turn 9 — 2026-05-23 21:24 BST
+
+| Field           | Value        |
+| --------------- | ------------ |
+| **Commit**      | working tree |
+| **IDE / Agent** | Codex        |
+
+**Summary:** Reviewed the Kiro/ACP image attachment fix that preserves native ACP image prompts while surfacing provider JSON-RPC errors as request errors instead of Effect decode defects.
+**Outcome:** No findings.
+**Risk score:** High — this changes shared ACP transport response handling, the shared ACP adapter attachment path, and Kiro provider-specific attachment validation.
+**Change archetypes:** protocol compatibility, provider capability guard, attachment validation, error-channel preservation, regression coverage.
+**Intended change:** Keep image upload support for Kiro, reject unsupported image paths clearly, and preserve provider error `data` so invalid images no longer show the low-level `Internal error at decode` stack.
+**Intent vs actual:** The diff matches the intent. JSON-RPC response errors that the Effect RPC layer decoded as `Die` defects are normalized back to protocol failures before reaching core RPC callers or generic extension callers. `mapAcpToAdapterError` can now read the preserved `data`. Shared ACP content-block construction checks the agent's advertised image prompt capability and lets providers supply their own image MIME allowlist. Kiro wires the allowlist to PNG/JPEG/GIF/WebP without moving provider policy into the transport layer.
+**Confidence:** High for transport and adapter behavior because both core RPC and extension request error paths now have direct tests. Medium for Kiro's private active-prompt `_message/send` image payload contract because that path is covered by the shared adapter unit test, while the live provider probe covered normal `session/prompt` image handling and invalid image error surfacing.
+**Coverage note:** Added focused tests for ACP JSON-RPC error surfacing, extension error surfacing, image-capability rejection, and provider MIME allowlist rejection.
+**Finding triage:** No open findings. The main candidate failures were checked directly: protocol errors still route through typed `AcpRequestError`, non-image-capable agents cannot receive image blocks, and Kiro rejects unsupported MIME types before file read/provider dispatch.
+**Static/analyzer evidence:** No analyzer policy changes. Fallow is not available in this repo context. Lint passed with the existing 9 warnings unrelated to this diff.
+**Architecture impact:** The capability invariant lives in the shared ACP adapter, where prompt content is materialized for all standard ACP providers. Kiro's narrower MIME policy remains in `KiroAdapter`, preserving provider ownership. Transport normalization stays in `effect-acp` and is not Kiro-specific.
+**Bug classes / invariants checked:** provider JSON-RPC errors preserve code/message/data; core RPC and generic extension errors use the request error channel; image content is only sent to agents advertising image prompt support; provider MIME allowlists reject unsupported image types before dispatch; Kiro image types align with the existing Claude/common vision set.
+**Branch totality:** Reviewed the scoped local diff for the ACP/Kiro image fix. The working tree still contains unrelated `.gitignore`, app-logo, public, and release artifact changes that are intentionally outside this review and commit scope.
+**Sibling closure:** Checked `AcpAdapterSupport` error formatting, ACP error classes, attachment path resolution, Claude/Codex/Cursor image attachment patterns, shared ACP prompt construction, Kiro active-prompt send mapping, and web send attachment handoff.
+**Remediation impact surface:** Affects Kiro and any future provider using `makeStandardAcpAdapter` with image attachments. Cursor remains on its separate adapter path. Existing text-only ACP prompt behavior is unchanged.
+**Residual risk / unknowns:** Kiro's private `_message/send` method is not an ACP standard contract; the shared adapter payload shape is tested, but provider behavior could still change outside this repo.
+
+### Validation
+
+- `bun run test src/client.test.ts src/protocol.test.ts` from `packages/effect-acp` — passed, 17 tests.
+- `bun run test src/provider/acp/StandardAcpAdapter.test.ts` from `apps/server` — passed, 10 tests.
+- `bun fmt` — passed.
+- `bun lint` — passed with 9 existing warnings.
+- `bun typecheck` — passed, 13 packages.
+- `git diff --check -- apps/server/src/provider/Layers/KiroAdapter.ts apps/server/src/provider/acp/StandardAcpAdapter.test.ts apps/server/src/provider/acp/StandardAcpAdapter.ts packages/effect-acp/src/client.test.ts packages/effect-acp/src/protocol.test.ts packages/effect-acp/src/protocol.ts` — passed.
+
+### Branch-totality proof
+
+- **Non-delta files/systems re-read:** diff-review gates, all-clear antipatterns, architecture implementation checklist, `AcpAdapterSupport`, `effect-acp` error classes, attachment store/mime helpers, Claude/Codex/Cursor attachment handling, web send attachment handoff.
+- **Prior open findings rechecked:** No open findings remained from Turn 8.
+- **Prior resolved/adjacent areas revalidated:** Active-prompt attachment steering remains routed through the existing Kiro `_message/send` hook; Kiro stop/cancel behavior is unchanged by this turn.
+- **Hotspots or sibling paths revisited:** Shared ACP protocol response routing, generic extension pending requests, normal `session/prompt`, active-prompt content block construction, provider-specific MIME policy, attachment path resolution.
+- **Dependency/adjacent surfaces revalidated:** `mapAcpToAdapterError` already formats preserved provider `data`, so the transport fix now feeds the UI-facing detail path.
+- **Why this is enough:** The riskiest invariant is error-channel classification, and both the core RPC path and extension path are directly covered. The attachment support checks are enforced before dispatch and have unit coverage for both capability and MIME rejection.
+
+### Challenger pass
+
+Done — assumed the fix could either swallow true provider defects or block valid image-capable providers. The normalization only rewrites failure entries shaped like JSON-RPC protocol errors, and the image guard follows ACP `promptCapabilities.image` with Kiro-specific MIME policy isolated to the Kiro adapter.
+
+### Resolved / Carried / New findings
+
+- None.
+
+### Recommendations
+
+1. **Fix first:** none.
+
 ## Turn 8 — 2026-05-21 07:08 BST
 
 | Field           | Value        |

apps/server/src/provider/Layers/KiroAdapter.ts

@@ -6,6 +6,12 @@ import { type EventNdjsonLogger } from "./EventNdjsonLogger.ts";
 
 const PROVIDER = ProviderDriverKind.make("kiro");
 const KIRO_ACTIVE_PROMPT_MESSAGE_METHOD = "_message/send";
+const SUPPORTED_KIRO_IMAGE_MIME_TYPES = new Set([
+  "image/gif",
+  "image/jpeg",
+  "image/png",
+  "image/webp",
+]);
 
 export interface KiroAdapterLiveOptions {
   readonly environment?: NodeJS.ProcessEnv;
@@ -23,6 +29,7 @@ export function makeKiroAdapter(kiroSettings: KiroSettings, options?: KiroAdapte
     ...(options?.nativeEventLogger ? { nativeEventLogger: options.nativeEventLogger } : {}),
     ...(options?.instanceId ? { instanceId: options.instanceId } : {}),
     activePromptMessageMethod: KIRO_ACTIVE_PROMPT_MESSAGE_METHOD,
+    supportedImageMimeTypes: SUPPORTED_KIRO_IMAGE_MIME_TYPES,
     stopSessionOnInterruptCancelUnsupported: true,
     sendMessageWhilePromptActive: ({ runtime, sessionId, content, contentBlocks }) =>
       runtime.request(KIRO_ACTIVE_PROMPT_MESSAGE_METHOD, {

apps/server/src/provider/acp/StandardAcpAdapter.test.ts

@@ -14,6 +14,7 @@ import type * as EffectAcpSchema from "effect-acp/schema";
 import { type ChatAttachment, ProviderDriverKind, ThreadId } from "@t3tools/contracts";
 
 import { ServerConfig } from "../../config.ts";
+import { ProviderAdapterRequestError, ProviderAdapterValidationError } from "../Errors.ts";
 import type { AcpSessionRuntimeShape } from "./AcpSessionRuntime.ts";
 import { makeStandardAcpAdapter } from "./StandardAcpAdapter.ts";
 
@@ -26,6 +27,7 @@ function makeFakeAcpRuntime(input: {
   readonly cancel?: Effect.Effect<void, AcpRequestError>;
   readonly prompt?: () => Effect.Effect<EffectAcpSchema.PromptResponse, unknown>;
   readonly request?: (method: string, payload: unknown) => Effect.Effect<unknown, unknown>;
+  readonly supportsImagePrompts?: boolean;
 }): AcpSessionRuntimeShape {
   const ignoreHandler = () => Effect.void;
   return {
@@ -49,7 +51,12 @@ function makeFakeAcpRuntime(input: {
         sessionId: "fake-session",
         initializeResult: {
           protocolVersion: 1,
-          agentCapabilities: { loadSession: true },
+          agentCapabilities: {
+            loadSession: true,
+            ...(input.supportsImagePrompts === false
+              ? {}
+              : { promptCapabilities: { image: true } }),
+          },
         } as EffectAcpSchema.InitializeResponse,
         sessionSetupResult: {
           sessionId: "fake-session",
@@ -123,6 +130,92 @@ it.effect("keeps interrupted ACP turns active until session/prompt resolves", ()
   }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)),
 );
 
+it.effect("rejects image attachments when the ACP session does not advertise image prompts", () =>
+  Effect.gen(function* () {
+    const provider = ProviderDriverKind.make("cursor");
+    const threadId = ThreadId.make("standard-acp-image-capability-required");
+    const cancelCalled = yield* Deferred.make<void>();
+    const runtime = makeFakeAcpRuntime({
+      cancelCalled,
+      supportsImagePrompts: false,
+    });
+    const attachment: ChatAttachment = {
+      type: "image",
+      id: "image-capability-required",
+      name: "image-capability-required.png",
+      mimeType: "image/png",
+      sizeBytes: 1,
+    };
+
+    const adapter = yield* makeStandardAcpAdapter({
+      provider,
+      runtimeLabel: "Fake ACP",
+      makeRuntime: () => Effect.succeed(runtime),
+    });
+
+    yield* adapter.startSession({
+      threadId,
+      provider,
+      cwd: process.cwd(),
+      runtimeMode: "full-access",
+    });
+
+    const error = yield* adapter
+      .sendTurn({
+        threadId,
+        input: "inspect",
+        attachments: [attachment],
+      })
+      .pipe(Effect.flip, Effect.timeout("1 second"));
+
+    assert.instanceOf(error, ProviderAdapterValidationError);
+    assert.equal(error.issue, "Fake ACP session does not support image attachments.");
+    yield* adapter.stopSession(threadId);
+  }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)),
+);
+
+it.effect("rejects image attachments outside a provider image MIME allowlist", () =>
+  Effect.gen(function* () {
+    const provider = ProviderDriverKind.make("kiro");
+    const threadId = ThreadId.make("standard-acp-image-mime-allowlist");
+    const cancelCalled = yield* Deferred.make<void>();
+    const runtime = makeFakeAcpRuntime({ cancelCalled });
+    const attachment: ChatAttachment = {
+      type: "image",
+      id: "image-mime-allowlist",
+      name: "image-mime-allowlist.svg",
+      mimeType: "image/svg+xml",
+      sizeBytes: 1,
+    };
+
+    const adapter = yield* makeStandardAcpAdapter({
+      provider,
+      runtimeLabel: "Kiro",
+      supportedImageMimeTypes: new Set(["image/png"]),
+      makeRuntime: () => Effect.succeed(runtime),
+    });
+
+    yield* adapter.startSession({
+      threadId,
+      provider,
+      cwd: process.cwd(),
+      runtimeMode: "full-access",
+    });
+
+    const error = yield* adapter
+      .sendTurn({
+        threadId,
+        input: "inspect",
+        attachments: [attachment],
+      })
+      .pipe(Effect.flip, Effect.timeout("1 second"));
+
+    assert.instanceOf(error, ProviderAdapterRequestError);
+    assert.equal(error.detail, "Unsupported Kiro image attachment type 'image/svg+xml'.");
+    yield* adapter.stopSession(threadId);
+  }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)),
+);
+
 it.effect("forwards session/cancel when no local active prompt is registered", () =>
   Effect.gen(function* () {
     const provider = ProviderDriverKind.make("cursor");

apps/server/src/provider/acp/StandardAcpAdapter.ts

@@ -75,6 +75,7 @@ export interface StandardAcpAdapterOptions {
   readonly nativeEventLogger?: EventNdjsonLogger;
   readonly instanceId?: ProviderInstanceId;
   readonly activePromptMessageMethod?: string;
+  readonly supportedImageMimeTypes?: ReadonlySet<string>;
   readonly stopSessionOnInterruptCancelUnsupported?: boolean;
   readonly sendMessageWhilePromptActive?: (input: {
     readonly runtime: AcpSessionRuntimeShape;
@@ -107,6 +108,7 @@ interface StandardAcpSessionContext {
   readonly scope: Scope.Closeable;
   readonly acp: AcpSessionRuntimeShape;
   readonly acpSessionId: string;
+  readonly supportsImagePrompts: boolean;
   notificationFiber: Fiber.Fiber<void, never> | undefined;
   readonly pendingApprovals: Map<ApprovalRequestId, PendingApproval>;
   readonly pendingUserInputs: Map<ApprovalRequestId, PendingUserInput>;
@@ -138,6 +140,10 @@ function parseStandardAcpResume(raw: unknown): { sessionId: string } | undefined
   return { sessionId: raw.sessionId.trim() };
 }
 
+function supportsImagePromptContent(initializeResult: EffectAcpSchema.InitializeResponse): boolean {
+  return initializeResult.agentCapabilities?.promptCapabilities?.image === true;
+}
+
 function normalizeModeSearchText(mode: AcpSessionMode): string {
   return [mode.id, mode.name, mode.description]
     .filter((value): value is string => typeof value === "string" && value.length > 0)
@@ -400,14 +406,33 @@ export function makeStandardAcpAdapter(
     const buildPromptContentBlocks = (
       input: Parameters<ProviderAdapterShape<ProviderAdapterError>["sendTurn"]>[0],
       method: string,
+      supportsImagePrompts: boolean,
     ) =>
       Effect.gen(function* () {
         const promptParts: Array<EffectAcpSchema.ContentBlock> = [];
         if (input.input?.trim()) {
           promptParts.push({ type: "text", text: input.input.trim() });
         }
         if (input.attachments && input.attachments.length > 0) {
+          if (!supportsImagePrompts) {
+            return yield* new ProviderAdapterValidationError({
+              provider,
+              operation: "sendTurn",
+              issue: `${options.runtimeLabel} session does not support image attachments.`,
+            });
+          }
           for (const attachment of input.attachments) {
+            const supportedImageMimeTypes = options.supportedImageMimeTypes;
+            if (
+              supportedImageMimeTypes &&
+              !supportedImageMimeTypes.has(attachment.mimeType.toLowerCase())
+            ) {
+              return yield* new ProviderAdapterRequestError({
+                provider,
+                method,
+                detail: `Unsupported ${options.runtimeLabel} image attachment type '${attachment.mimeType}'.`,
+              });
+            }
             const attachmentPath = resolveAttachmentPath({
               attachmentsDir: serverConfig.attachmentsDir,
               attachment,
@@ -639,6 +664,7 @@ export function makeStandardAcpAdapter(
             scope: sessionScope,
             acp,
             acpSessionId: started.sessionId,
+            supportsImagePrompts: supportsImagePromptContent(started.initializeResult),
             notificationFiber: undefined,
             pendingApprovals,
             pendingUserInputs,
@@ -752,7 +778,11 @@ export function makeStandardAcpAdapter(
           ctx.activePrompt?.turnId ?? ctx.activeTurnId ?? ctx.session.activeTurnId;
         if (activePromptTurnId && options.sendMessageWhilePromptActive) {
           const method = options.activePromptMessageMethod ?? "session/message";
-          const contentBlocks = yield* buildPromptContentBlocks(input, method);
+          const contentBlocks = yield* buildPromptContentBlocks(
+            input,
+            method,
+            ctx.supportsImagePrompts,
+          );
           const content = contentBlocks
             .filter(
               (block): block is Extract<EffectAcpSchema.ContentBlock, { type: "text" }> =>
@@ -813,7 +843,11 @@ export function makeStandardAcpAdapter(
             mapAcpToAdapterError(provider, input.threadId, method, cause),
         });
 
-        const promptParts = yield* buildPromptContentBlocks(input, "session/prompt");
+        const promptParts = yield* buildPromptContentBlocks(
+          input,
+          "session/prompt",
+          ctx.supportsImagePrompts,
+        );
 
         const previousActivePrompt = ctx.activePrompt;
         const previousActiveTurnId = ctx.activeTurnId;