From 657f253c1d2daebc5ad7268d1bf8277d281397c8 Mon Sep 17 00:00:00 2001
From: Declan Cowen <declan@cowen.co>
Date: Sat, 23 May 2026 21:27:19 +0100
Subject: [PATCH 1/3] Fix Kiro ACP image attachment errors

---
 .reviews/kiro-provider-appearance-review.md   | 62 +++++++++++-
 .../server/src/provider/Layers/KiroAdapter.ts |  7 ++
 .../provider/acp/StandardAcpAdapter.test.ts   | 95 ++++++++++++++++++-
 .../src/provider/acp/StandardAcpAdapter.ts    | 38 +++++++-
 packages/effect-acp/src/client.test.ts        | 44 +++++++++
 packages/effect-acp/src/protocol.test.ts      | 44 +++++++++
 packages/effect-acp/src/protocol.ts           | 76 ++++++++++++---
 7 files changed, 346 insertions(+), 20 deletions(-)

diff --git a/.reviews/kiro-provider-appearance-review.md b/.reviews/kiro-provider-appearance-review.md
index 0890c0960aa..631f2fc7ed6 100644
--- a/.reviews/kiro-provider-appearance-review.md
+++ b/.reviews/kiro-provider-appearance-review.md
@@ -6,7 +6,7 @@
 | -------------- | ---------------------------------------- |
 | **Repository** | `declancowen/t3code`                     |
 | **Remote**     | `origin`                                 |
-| **Branch**     | `main`                                   |
+| **Branch**     | `merge-upstream-main-check`              |
 | **Stack**      | TypeScript, Effect, React/Vite, Electron |
 
 ## Scope
@@ -14,6 +14,7 @@
 - `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        |
diff --git a/apps/server/src/provider/Layers/KiroAdapter.ts b/apps/server/src/provider/Layers/KiroAdapter.ts
index f96811e125c..16439da34b9 100644
--- a/apps/server/src/provider/Layers/KiroAdapter.ts
+++ b/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, {
diff --git a/apps/server/src/provider/acp/StandardAcpAdapter.test.ts b/apps/server/src/provider/acp/StandardAcpAdapter.test.ts
index 551256f971a..557098eb8f5 100644
--- a/apps/server/src/provider/acp/StandardAcpAdapter.test.ts
+++ b/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");
diff --git a/apps/server/src/provider/acp/StandardAcpAdapter.ts b/apps/server/src/provider/acp/StandardAcpAdapter.ts
index 520c76a158c..0b8f0d14299 100644
--- a/apps/server/src/provider/acp/StandardAcpAdapter.ts
+++ b/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,6 +406,7 @@ export function makeStandardAcpAdapter(
     const buildPromptContentBlocks = (
       input: Parameters<ProviderAdapterShape<ProviderAdapterError>["sendTurn"]>[0],
       method: string,
+      supportsImagePrompts: boolean,
     ) =>
       Effect.gen(function* () {
         const promptParts: Array<EffectAcpSchema.ContentBlock> = [];
@@ -407,7 +414,25 @@ export function makeStandardAcpAdapter(
           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;
diff --git a/packages/effect-acp/src/client.test.ts b/packages/effect-acp/src/client.test.ts
index 5c9a6f8336f..e02d527558e 100644
--- a/packages/effect-acp/src/client.test.ts
+++ b/packages/effect-acp/src/client.test.ts
@@ -22,8 +22,15 @@ import { makeInMemoryStdio } from "./_internal/stdio.ts";
 
 const InitializeRequest = jsonRpcRequest("initialize", AcpSchema.InitializeRequest);
 const InitializeResponse = jsonRpcResponse(AcpSchema.InitializeResponse);
+const PromptRequest = jsonRpcRequest("session/prompt", AcpSchema.PromptRequest);
+const PromptErrorResponse = Schema.Struct({
+  jsonrpc: Schema.Literal("2.0"),
+  id: Schema.Union([Schema.Number, Schema.String]),
+  error: AcpSchema.Error,
+});
 const ExtRequest = jsonRpcRequest("x/test", Schema.Struct({ hello: Schema.String }));
 const ExtResponse = jsonRpcResponse(Schema.Struct({ ok: Schema.Boolean }));
+const decodePromptRequest = Schema.decodeEffect(Schema.fromJsonString(PromptRequest));
 const mockPeerPath = Effect.map(Effect.service(Path.Path), (path) =>
   path.join(import.meta.dirname, "../test/fixtures/acp-mock-peer.ts"),
 );
@@ -446,4 +453,41 @@ it.layer(NodeServices.layer)("effect-acp client", (it) => {
       yield* Scope.close(scope, Exit.void);
     }),
   );
+
+  it.effect("surfaces JSON-RPC response errors through the ACP request error channel", () =>
+    Effect.gen(function* () {
+      const { stdio, input, output } = yield* makeInMemoryStdio();
+      const scope = yield* Scope.make();
+      const acp = yield* AcpClient.make(stdio).pipe(Effect.provideService(Scope.Scope, scope));
+
+      const promptFiber = yield* acp.agent
+        .prompt({
+          sessionId: "session-1",
+          prompt: [{ type: "text", text: "hello" }],
+        })
+        .pipe(Effect.flip, Effect.forkScoped);
+
+      const outbound = yield* Queue.take(output);
+      const promptRequest = yield* decodePromptRequest(outbound);
+      yield* Queue.offer(
+        input,
+        yield* encodeJsonl(PromptErrorResponse, {
+          jsonrpc: "2.0",
+          id: promptRequest.id,
+          error: {
+            code: -32603,
+            message: "Internal error",
+            data: "ValidationException: unsupported image format",
+          },
+        }),
+      );
+
+      const error = yield* Fiber.join(promptFiber);
+      assert.instanceOf(error, AcpError.AcpRequestError);
+      assert.equal(error.message, "Internal error");
+      assert.equal(error.data, "ValidationException: unsupported image format");
+
+      yield* Scope.close(scope, Exit.void);
+    }),
+  );
 });
diff --git a/packages/effect-acp/src/protocol.test.ts b/packages/effect-acp/src/protocol.test.ts
index 4df2384a4fe..20b74d19133 100644
--- a/packages/effect-acp/src/protocol.test.ts
+++ b/packages/effect-acp/src/protocol.test.ts
@@ -2,6 +2,7 @@ import * as Path from "effect/Path";
 import * as AcpError from "./errors.ts";
 import * as Effect from "effect/Effect";
 import * as Deferred from "effect/Deferred";
+import * as Exit from "effect/Exit";
 import * as Fiber from "effect/Fiber";
 import * as Queue from "effect/Queue";
 import * as Schema from "effect/Schema";
@@ -41,6 +42,11 @@ const RequestPermissionRequest = jsonRpcRequest(
 const RequestPermissionResponse = jsonRpcResponse(AcpSchema.RequestPermissionResponse);
 const ExtRequest = jsonRpcRequest("x/test", Schema.Struct({ hello: Schema.String }));
 const ExtResponse = jsonRpcResponse(Schema.Struct({ ok: Schema.Boolean }));
+const ExtErrorResponse = Schema.Struct({
+  jsonrpc: Schema.Literal("2.0"),
+  id: Schema.Union([Schema.Number, Schema.String]),
+  error: AcpSchema.Error,
+});
 const decodeSessionCancelNotification = Schema.decodeEffect(
   Schema.fromJsonString(SessionCancelNotification),
 );
@@ -230,6 +236,44 @@ it.layer(NodeServices.layer)("effect-acp protocol", (it) => {
     }),
   );
 
+  it.effect("surfaces generic extension response errors through the request error channel", () =>
+    Effect.gen(function* () {
+      const { stdio, input, output } = yield* makeInMemoryStdio();
+      const transport = yield* AcpProtocol.makeAcpPatchedProtocol({
+        stdio,
+        serverRequestMethods: new Set(),
+      });
+
+      const response = yield* transport
+        .request("x/test", { hello: "world" })
+        .pipe(Effect.flip, Effect.forkScoped);
+      const outbound = yield* Queue.take(output);
+      const decodedRequest = yield* decodeExtRequest(outbound);
+
+      yield* Queue.offer(
+        input,
+        yield* encodeJsonl(ExtErrorResponse, {
+          jsonrpc: "2.0",
+          id: decodedRequest.id,
+          error: {
+            code: -32603,
+            message: "Internal error",
+            data: "extension failed",
+          },
+        }),
+      );
+
+      const exit = yield* Fiber.await(response);
+      if (!Exit.isSuccess(exit)) {
+        assert.fail("Expected request to fail with a response error");
+      }
+      const error = exit.value;
+      assert.instanceOf(error, AcpError.AcpRequestError);
+      assert.equal(error.message, "Internal error");
+      assert.equal(error.data, "extension failed");
+    }),
+  );
+
   it.effect("preserves zero-valued ids for inbound core client requests", () =>
     Effect.gen(function* () {
       const { stdio, input, output } = yield* makeInMemoryStdio();
diff --git a/packages/effect-acp/src/protocol.ts b/packages/effect-acp/src/protocol.ts
index 6aca1611a34..47fa575cac1 100644
--- a/packages/effect-acp/src/protocol.ts
+++ b/packages/effect-acp/src/protocol.ts
@@ -390,28 +390,33 @@ export const makeAcpPatchedProtocol = Effect.fn("makeAcpPatchedProtocol")(functi
     );
   };
 
-  const handleExitEncoded = (message: RpcMessage.ResponseExitEncoded) =>
-    Ref.get(extPending).pipe(
+  const handleExitEncoded = (message: RpcMessage.ResponseExitEncoded) => {
+    const normalizedMessage = normalizeProtocolErrorExit(message);
+    return Ref.get(extPending).pipe(
       Effect.flatMap((pending) => {
-        if (!pending.has(message.requestId)) {
-          return Queue.offer(clientQueue, message).pipe(Effect.asVoid);
+        if (!pending.has(normalizedMessage.requestId)) {
+          return Queue.offer(clientQueue, normalizedMessage).pipe(Effect.asVoid);
         }
-        if (message.exit._tag === "Success") {
-          return completeExtPendingSuccess(message.requestId, message.exit.value);
+        if (normalizedMessage.exit._tag === "Success") {
+          return completeExtPendingSuccess(
+            normalizedMessage.requestId,
+            normalizedMessage.exit.value,
+          );
         }
-        const failure = message.exit.cause.find((entry) => entry._tag === "Fail");
-        if (failure && isProtocolError(failure.error)) {
+        const failure = findProtocolErrorFailure(normalizedMessage.exit.cause);
+        if (failure) {
           return completeExtPendingFailure(
-            message.requestId,
-            AcpError.AcpRequestError.fromProtocolError(failure.error),
+            normalizedMessage.requestId,
+            AcpError.AcpRequestError.fromProtocolError(failure),
           );
         }
         return completeExtPendingFailure(
-          message.requestId,
+          normalizedMessage.requestId,
           AcpError.AcpRequestError.internalError("Extension request failed"),
         );
       }),
     );
+  };
 
   const routeDecodedMessage = (
     message: RpcMessage.FromClientEncoded | RpcMessage.FromServerEncoded,
@@ -592,9 +597,13 @@ export const makeAcpPatchedProtocol = Effect.fn("makeAcpPatchedProtocol")(functi
   } satisfies AcpPatchedProtocol;
 });
 
-function isProtocolError(
-  value: unknown,
-): value is { code: number; message: string; data?: unknown } {
+type ProtocolError = { code: number; message: string; data?: unknown };
+type ProtocolFailureCause = Extract<
+  RpcMessage.ResponseExitEncoded["exit"],
+  { readonly _tag: "Failure" }
+>["cause"];
+
+function isProtocolError(value: unknown): value is ProtocolError {
   return (
     typeof value === "object" &&
     value !== null &&
@@ -605,6 +614,45 @@ function isProtocolError(
   );
 }
 
+function normalizeProtocolErrorExit(
+  message: RpcMessage.ResponseExitEncoded,
+): RpcMessage.ResponseExitEncoded {
+  if (message.exit._tag !== "Failure") {
+    return message;
+  }
+
+  let changed = false;
+  const cause = message.exit.cause.map((entry) => {
+    if (entry._tag !== "Die" || !isProtocolError(entry.defect)) {
+      return entry;
+    }
+    changed = true;
+    return {
+      _tag: "Fail" as const,
+      error: entry.defect,
+    };
+  });
+
+  return changed
+    ? {
+        ...message,
+        exit: {
+          _tag: "Failure" as const,
+          cause,
+        },
+      }
+    : message;
+}
+
+function findProtocolErrorFailure(cause: ProtocolFailureCause): ProtocolError | undefined {
+  for (const entry of cause) {
+    if (entry._tag === "Fail" && isProtocolError(entry.error)) {
+      return entry.error;
+    }
+  }
+  return undefined;
+}
+
 function normalizeToRequestError(error: AcpError.AcpError): AcpError.AcpRequestError {
   return isAcpRequestError(error) ? error : AcpError.AcpRequestError.internalError(error.message);
 }

From dd6e413d243d0c521128713c5aaf7ba902b02c2d Mon Sep 17 00:00:00 2001
From: Declan Cowen <declan@cowen.co>
Date: Sat, 23 May 2026 22:18:54 +0100
Subject: [PATCH 2/3] Address PR review findings

---
 .github/workflows/desktop-release.yml         |  12 +-
 .reviews/pr2793-automation-feedback-review.md | 125 ++++++++++++++++++
 .../src/updates/DesktopUpdates.test.ts        |  31 +++++
 apps/desktop/src/updates/DesktopUpdates.ts    |  21 ++-
 .../chat/ComposerPrimaryActions.test.ts       |  29 +++-
 .../chat/ComposerPrimaryActions.tsx           |   2 +-
 packages/effect-acp/src/protocol.test.ts      | 110 +++++++++++++++
 packages/effect-acp/src/protocol.ts           |  79 ++++++++---
 packages/shared/src/shell.test.ts             |   5 +-
 packages/shared/src/shell.ts                  |   2 +-
 10 files changed, 383 insertions(+), 33 deletions(-)
 create mode 100644 .reviews/pr2793-automation-feedback-review.md

diff --git a/.github/workflows/desktop-release.yml b/.github/workflows/desktop-release.yml
index ef6bb79a005..9ad25e6f124 100644
--- a/.github/workflows/desktop-release.yml
+++ b/.github/workflows/desktop-release.yml
@@ -24,7 +24,7 @@ jobs:
       version: ${{ steps.release_meta.outputs.version }}
       tag: ${{ steps.release_meta.outputs.tag }}
       release_name: ${{ steps.release_meta.outputs.release_name }}
-      ref: ${{ github.sha }}
+      ref: ${{ steps.release_meta.outputs.ref }}
     steps:
       - name: Checkout
         uses: actions/checkout@v6
@@ -65,9 +65,17 @@ jobs:
             exit 1
           fi
 
+          tag="v$version"
+          release_ref="$(git rev-list -n 1 "$tag" 2>/dev/null || true)"
+          if [[ -z "$release_ref" ]]; then
+            echo "Release tag $tag does not exist. Create the tag before running the desktop release workflow." >&2
+            exit 1
+          fi
+
           echo "version=$version" >> "$GITHUB_OUTPUT"
-          echo "tag=v$version" >> "$GITHUB_OUTPUT"
+          echo "tag=$tag" >> "$GITHUB_OUTPUT"
           echo "release_name=T3 Code v$version" >> "$GITHUB_OUTPUT"
+          echo "ref=$release_ref" >> "$GITHUB_OUTPUT"
 
       - name: Format check
         run: bun run fmt:check
diff --git a/.reviews/pr2793-automation-feedback-review.md b/.reviews/pr2793-automation-feedback-review.md
new file mode 100644
index 00000000000..09d706f1b20
--- /dev/null
+++ b/.reviews/pr2793-automation-feedback-review.md
@@ -0,0 +1,125 @@
+# Review: PR 2793 Automation Feedback
+
+## Project context
+
+| Field          | Value                                    |
+| -------------- | ---------------------------------------- |
+| **Repository** | `declancowen/t3code`                     |
+| **Remote**     | `origin`                                 |
+| **Branch**     | `codex/kiro-acp-image-errors`            |
+| **Stack**      | TypeScript, Effect, React/Vite, Electron |
+
+## Scope
+
+- `packages/effect-acp/src/protocol.ts` — provider-originated ACP request-id normalization and response restoration.
+- `packages/shared/src/shell.ts` — POSIX login-shell environment capture.
+- `apps/desktop/src/updates/DesktopUpdates.ts` — desktop updater install failure recovery.
+- `.github/workflows/desktop-release.yml` — manual desktop release tag/ref resolution.
+- `apps/web/src/components/chat/ComposerPrimaryActions.tsx` — compact idle send button rendering.
+
+## Hotspots
+
+- ACP JSON-RPC request identity preservation when Effect RPC requires numeric server request IDs.
+- Optional login-shell environment variables under `set -e`.
+- Desktop update install handoff partial-failure recovery after the backend is stopped.
+- Manual release artifact provenance for versioned desktop releases.
+- Compact composer layout parity.
+
+## Review status
+
+| Field                 | Value                |
+| --------------------- | -------------------- |
+| **Review started**    | 2026-05-23 22:15 BST |
+| **Last reviewed**     | 2026-05-23 22:15 BST |
+| **Total turns**       | 1                    |
+| **Open findings**     | 0                    |
+| **Resolved findings** | 5                    |
+| **Accepted findings** | 0                    |
+
+## Automation
+
+| Field                | Value                              |
+| -------------------- | ---------------------------------- |
+| **Mode**             | `pr-review-automation`             |
+| **PR**               | `pingdotgg/t3code#2793`            |
+| **State authority**  | GitHub review threads              |
+| **Review file role** | Human-readable local review ledger |
+
+## Turn 1 — 2026-05-23 22:15 BST
+
+| Field           | Value        |
+| --------------- | ------------ |
+| **Commit**      | working tree |
+| **IDE / Agent** | Codex        |
+
+### Automation context
+
+| Field                          | Value                                                |
+| ------------------------------ | ---------------------------------------------------- |
+| **Trigger**                    | Manual import of unresolved PR review threads        |
+| **PR**                         | `pingdotgg/t3code#2793`                              |
+| **Base ref**                   | `main`                                               |
+| **Head SHA**                   | `657f253c1d2daebc5ad7268d1bf8277d281397c8`           |
+| **Previous reviewed head SHA** | `657f253c1d2daebc5ad7268d1bf8277d281397c8`           |
+| **Trusted state source**       | `gh api graphql pullRequest.reviewThreads`           |
+| **Verification policy**        | Focused regressions, full repo tests, format/lint/ts |
+
+**Summary:** Imported and fixed all five unresolved automated review findings from Cursor Bugbot, Macroscope, and Codex.
+**Outcome:** All clear for the imported finding scope.
+**Risk score:** High — one finding was a shared ACP transport identity bug, two were partial-failure/release-provenance bugs, and the remaining two touched shared shell/UI behavior.
+**Change archetypes:** protocol compatibility, async lifecycle recovery, release automation, shared utility compatibility, shared UI parity.
+**Intended change:** Close the unresolved PR findings without moving ownership boundaries or weakening the Kiro image attachment fix.
+**Intent vs actual:** The diff addresses the current-tree failure modes: ACP aliases no longer collide with native numeric IDs; missing optional env vars are non-fatal; updater async install errors restart the backend through the same helper as sync failures; manual desktop release builds checkout the commit pointed to by the requested tag; compact composer mode reaches the send button.
+**Confidence:** High — each imported finding now has a direct code change plus focused regression coverage where the repo can test it, and the full repo test script passed after repairing the local Electron binary install.
+**Coverage note:** Focused tests were added or updated for ACP ID collision, shell env capture command shape, desktop async install error recovery, and compact composer send button rendering.
+**Finding triage:** All imported findings were live in the pre-fix tree and resolved in the current tree.
+**Static/analyzer evidence:** No analyzer policy changed. Fallow is unavailable in this repo/PATH. `bun lint` exits 0 with existing unrelated warnings in `ChatView.tsx`, `catalog.test.ts`, and `clientPersistenceStorage.ts`.
+**Architecture impact:** Ownership remains in the correct layers: ACP transport owns request-id normalization, shared shell owns environment capture, desktop updater owns backend restart policy, GitHub Actions owns release provenance, and composer presentation owns compact button layout.
+**Bug classes / invariants checked:** Identity/uniqueness for ACP request IDs; variant state for unset env vars and compact UI; lifecycle/partial failure for updater install errors; contract/provenance for release tag-to-artifact commit mapping.
+**Branch totality:** Reviewed the local fix delta plus the branch Kiro/ACP image diff against `origin/main`. Pre-existing dirty `.gitignore` and untracked app-logo/public/release artifacts remain outside this fix scope.
+**Sibling closure:** Checked native numeric ID cleanup and alias response restoration, multi-variable env capture, sync and async updater install failure paths, release checkout consumers of `needs.preflight.outputs.ref`, and the only `SendButton` call site.
+**Remediation impact surface:** The ACP fix affects all provider-originated core requests using `effect-acp`; the shell fix affects desktop environment hydration; desktop release/update fixes affect manual releases and failed install recovery; composer fix affects compact render surfaces only.
+**Residual risk / unknowns:** The workflow tag lookup is script-reviewed and type/lint/test-covered only through repository gates, not by executing GitHub Actions. Live PR automation will re-evaluate after push.
+
+### External Finding Import
+
+| Source        | Finding                                                                      | Current status | Bug class                                                          | Missed invariant/variant                                         | Action                           |
+| ------------- | ---------------------------------------------------------------------------- | -------------- | ------------------------------------------------------------------ | ---------------------------------------------------------------- | -------------------------------- |
+| Cursor Bugbot | Manual desktop release builds `github.sha` instead of the version tag commit | resolved       | Contract Encoding                                                  | release version/tag must own build provenance                    | fixed in workflow preflight      |
+| Macroscope    | Async updater install error does not restart stopped backend                 | resolved       | Lifecycle And Transient Containers / Atomicity And Partial Failure | event-driven install failure must mirror sync failure recovery   | fixed with shared restart helper |
+| Macroscope    | Compact composer does not pass compact state to `SendButton`                 | resolved       | Variant State / Affordance Parity                                  | compact mode must reach final idle send control                  | fixed and covered                |
+| Codex         | ACP aliased request ID can collide with native numeric provider request IDs  | resolved       | Identity And Uniqueness                                            | internal server request IDs must not overlap active provider IDs | fixed and covered                |
+| Codex         | Login shell `printenv` aborts optional env capture under `set -e`            | resolved       | Variant State / Compatibility                                      | unset optional vars must be non-fatal                            | fixed and covered                |
+
+### Validation
+
+- `bun run --cwd packages/effect-acp test src/protocol.test.ts` — passed, 12 tests.
+- `bun run --cwd packages/shared test src/shell.test.ts` — passed, 25 tests.
+- `bun run --cwd apps/desktop test src/updates/DesktopUpdates.test.ts` — passed, 8 tests.
+- `bun run --cwd apps/web test src/components/chat/ComposerPrimaryActions.test.ts` — passed, 9 tests.
+- `bun fmt` — passed.
+- `bun lint` — passed with 9 existing warnings.
+- `bun typecheck` — passed, 13 packages.
+- `bun run test` — passed, 13 packages, after running Electron's package postinstall to restore the missing local Electron binary.
+
+### Branch-totality proof
+
+- **Non-delta files/systems re-read:** PR review threads, diff-review gates, architecture enforcement guidance, ACP protocol tests, shell tests, desktop updater tests, composer action tests, desktop release workflow checkout path.
+- **Prior open findings rechecked:** All five unresolved GitHub review threads were mapped to current-tree behavior and fixed.
+- **Prior resolved/adjacent areas revalidated:** Kiro image attachment handling and provider error surfacing still use the shared ACP adapter/transport path; desktop sync install failure recovery still restarts the backend; composer submit semantics are otherwise unchanged.
+- **Hotspots or sibling paths revisited:** ACP alias cleanup vs native ID cleanup, missing vs present shell env vars, sync vs event-driven updater install failures, release workflow tag-push vs manual dispatch, compact vs non-compact send button.
+- **Dependency/adjacent surfaces revalidated:** `needs.preflight.outputs.ref` consumers, `DesktopBackendManager.start` restart behavior, `readEnvironmentFromLoginShell` callers, and `ComposerPrimaryActions` idle state rendering.
+- **Why this is enough:** The strongest failure modes now have direct regression tests, and the non-testable workflow branch uses the tag commit as the single release provenance source before build checkout.
+
+### Challenger pass
+
+- Done — assumed one serious issue remained in request-id aliasing. The current code aliases colliding numeric provider requests instead of letting them pass through, so two active requests cannot share the same internal request ID even when one original ID is already numeric.
+
+### Resolved / Carried / New findings
+
+- No open findings remain for this imported PR feedback set.
+
+### Recommendations
+
+1. **Fix first:** none.
+2. **Then address:** push and let GitHub mark the old review threads outdated or rerun automation on the new commit.
diff --git a/apps/desktop/src/updates/DesktopUpdates.test.ts b/apps/desktop/src/updates/DesktopUpdates.test.ts
index 6c9c3fe8e36..5e74522d907 100644
--- a/apps/desktop/src/updates/DesktopUpdates.test.ts
+++ b/apps/desktop/src/updates/DesktopUpdates.test.ts
@@ -319,6 +319,37 @@ describe("DesktopUpdates", () => {
     ).pipe(Effect.provide(Layer.merge(TestClock.layer(), failingHarness.layer)));
   });
 
+  it.effect("restarts the desktop backend when an updater install error arrives later", () => {
+    const harness = makeHarness();
+
+    return Effect.scoped(
+      Effect.gen(function* () {
+        const updates = yield* DesktopUpdates.DesktopUpdates;
+        yield* updates.configure;
+
+        harness.emit("update-available", { version: "1.2.4" });
+        yield* flushCallbacks;
+        harness.emit("update-downloaded", { version: "1.2.4" });
+        yield* flushCallbacks;
+
+        const result = yield* updates.install;
+        assert.equal(result.accepted, true);
+        assert.equal(result.completed, false);
+        assert.equal(harness.stopCount(), 1);
+        assert.equal(harness.startCount(), 0);
+
+        harness.emit("error", new Error("install handoff failed"));
+        yield* flushCallbacks;
+
+        const state = yield* updates.getState;
+        assert.equal(state.status, "downloaded");
+        assert.equal(state.errorContext, "install");
+        assert.equal(state.message, "install handoff failed");
+        assert.equal(harness.startCount(), 1);
+      }),
+    ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer)));
+  });
+
   it.effect("persists channel changes through the settings service", () => {
     const harness = makeHarness();
 
diff --git a/apps/desktop/src/updates/DesktopUpdates.ts b/apps/desktop/src/updates/DesktopUpdates.ts
index 3b7d24be40f..d5717163f36 100644
--- a/apps/desktop/src/updates/DesktopUpdates.ts
+++ b/apps/desktop/src/updates/DesktopUpdates.ts
@@ -355,6 +355,18 @@ const make = Effect.gen(function* () {
     );
   }).pipe(Effect.withSpan("desktop.updates.downloadAvailableUpdate"));
 
+  const restartBackendAfterInstallFailure = Effect.fn(
+    "desktop.updates.restartBackendAfterInstallFailure",
+  )(function* () {
+    yield* backendManager.start.pipe(
+      Effect.catchCause((cause) =>
+        logUpdaterError("failed to restart desktop backend after update install failure", {
+          cause: Cause.pretty(cause),
+        }),
+      ),
+    );
+  });
+
   const installDownloadedUpdate = Effect.gen(function* () {
     const state = yield* Ref.get(updateStateRef);
     if (
@@ -393,13 +405,7 @@ const make = Effect.gen(function* () {
             reduceDesktopUpdateStateOnInstallFailure(current, error.message),
           );
           yield* Ref.set(desktopState.quitting, false);
-          yield* backendManager.start.pipe(
-            Effect.catchCause((cause) =>
-              logUpdaterError("failed to restart desktop backend after update install failure", {
-                cause: Cause.pretty(cause),
-              }),
-            ),
-          );
+          yield* restartBackendAfterInstallFailure();
           yield* logUpdaterError("failed to install update", { message: error.message });
           return { accepted: true, completed: false };
         }),
@@ -475,6 +481,7 @@ const make = Effect.gen(function* () {
       yield* Ref.set(updateInstallInFlightRef, false);
       yield* Ref.set(desktopState.quitting, false);
       yield* updateState((current) => reduceDesktopUpdateStateOnInstallFailure(current, message));
+      yield* restartBackendAfterInstallFailure();
       yield* logUpdaterError("updater error", { message });
       return;
     }
diff --git a/apps/web/src/components/chat/ComposerPrimaryActions.test.ts b/apps/web/src/components/chat/ComposerPrimaryActions.test.ts
index 1982f145e26..72a8e3449fa 100644
--- a/apps/web/src/components/chat/ComposerPrimaryActions.test.ts
+++ b/apps/web/src/components/chat/ComposerPrimaryActions.test.ts
@@ -1,6 +1,8 @@
 import { describe, expect, it } from "vitest";
+import { createElement } from "react";
+import { renderToStaticMarkup } from "react-dom/server";
 
-import { formatPendingPrimaryActionLabel } from "./ComposerPrimaryActions";
+import { ComposerPrimaryActions, formatPendingPrimaryActionLabel } from "./ComposerPrimaryActions";
 
 describe("formatPendingPrimaryActionLabel", () => {
   it("returns 'Submitting...' while responding", () => {
@@ -91,3 +93,28 @@ describe("formatPendingPrimaryActionLabel", () => {
     ).toBe("Submit answers");
   });
 });
+
+describe("ComposerPrimaryActions", () => {
+  it("passes compact mode through to the idle send button", () => {
+    const markup = renderToStaticMarkup(
+      createElement(ComposerPrimaryActions, {
+        compact: true,
+        pendingAction: null,
+        isRunning: false,
+        showPlanFollowUpPrompt: false,
+        promptHasText: true,
+        isSendBusy: false,
+        isConnecting: false,
+        isEnvironmentUnavailable: false,
+        isPreparingWorktree: false,
+        hasSendableContent: true,
+        onPreviousPendingQuestion: () => undefined,
+        onInterrupt: () => undefined,
+        onImplementPlanInNewThread: () => undefined,
+      }),
+    );
+
+    expect(markup).toContain("size-8");
+    expect(markup).not.toContain("h-9 w-9");
+  });
+});
diff --git a/apps/web/src/components/chat/ComposerPrimaryActions.tsx b/apps/web/src/components/chat/ComposerPrimaryActions.tsx
index 53c78ee678e..8e77beff5a7 100644
--- a/apps/web/src/components/chat/ComposerPrimaryActions.tsx
+++ b/apps/web/src/components/chat/ComposerPrimaryActions.tsx
@@ -247,7 +247,7 @@ export const ComposerPrimaryActions = memo(function ComposerPrimaryActions({
 
   return (
     <SendButton
-      compact={false}
+      compact={compact}
       disabled={sendDisabled}
       busy={sendBusy}
       ariaLabel={
diff --git a/packages/effect-acp/src/protocol.test.ts b/packages/effect-acp/src/protocol.test.ts
index 20b74d19133..356315b3afa 100644
--- a/packages/effect-acp/src/protocol.test.ts
+++ b/packages/effect-acp/src/protocol.test.ts
@@ -413,6 +413,116 @@ it.layer(NodeServices.layer)("effect-acp protocol", (it) => {
     }),
   );
 
+  it.effect("avoids alias collisions with native numeric inbound core request ids", () =>
+    Effect.gen(function* () {
+      const { stdio, input, output } = yield* makeInMemoryStdio();
+      const transport = yield* AcpProtocol.makeAcpPatchedProtocol({
+        stdio,
+        serverRequestMethods: new Set(["session/request_permission"]),
+      });
+      const inboundRequests = yield* Queue.unbounded<unknown>();
+      const nonNumericRequestId = "aee865ed-8360-495a-8007-d135a97a0b4a";
+
+      yield* transport.serverProtocol
+        .run((_clientId, message) => Queue.offer(inboundRequests, message).pipe(Effect.asVoid))
+        .pipe(Effect.forkScoped);
+
+      yield* Queue.offer(
+        input,
+        yield* encodeJsonl(RequestPermissionRequest, {
+          jsonrpc: "2.0",
+          id: nonNumericRequestId,
+          method: "session/request_permission",
+          params: {
+            sessionId: "session-1",
+            toolCall: {
+              toolCallId: "tool-1",
+              title: "Editing ComposerPrimaryActions.tsx",
+            },
+            options: [{ optionId: "allow_once", name: "Yes", kind: "allow_once" }],
+          },
+          headers: [],
+        }),
+      );
+
+      const aliasedMessage = (yield* Queue.take(inboundRequests)) as { readonly id: string };
+      assert.notEqual(aliasedMessage.id, nonNumericRequestId);
+
+      yield* Queue.offer(
+        input,
+        yield* encodeJsonl(RequestPermissionRequest, {
+          jsonrpc: "2.0",
+          id: 1,
+          method: "session/request_permission",
+          params: {
+            sessionId: "session-1",
+            toolCall: {
+              toolCallId: "tool-2",
+              title: "Run command",
+            },
+            options: [{ optionId: "allow", name: "Allow", kind: "allow_once" }],
+          },
+          headers: [],
+        }),
+      );
+
+      const numericMessage = (yield* Queue.take(inboundRequests)) as { readonly id: string };
+      assert.notEqual(numericMessage.id, aliasedMessage.id);
+
+      yield* transport.serverProtocol.send(0, {
+        _tag: "Exit",
+        requestId: numericMessage.id,
+        exit: {
+          _tag: "Success",
+          value: {
+            outcome: {
+              outcome: "selected",
+              optionId: "allow",
+            },
+          },
+        },
+      });
+
+      const numericOutbound = yield* Queue.take(output);
+      assert.deepEqual(yield* decodeRequestPermissionResponse(numericOutbound), {
+        jsonrpc: "2.0",
+        id: 1,
+        result: {
+          outcome: {
+            outcome: "selected",
+            optionId: "allow",
+          },
+        },
+      });
+
+      yield* transport.serverProtocol.send(0, {
+        _tag: "Exit",
+        requestId: aliasedMessage.id,
+        exit: {
+          _tag: "Success",
+          value: {
+            outcome: {
+              outcome: "selected",
+              optionId: "allow_once",
+            },
+          },
+        },
+      });
+
+      const aliasedOutbound = yield* Queue.take(output);
+      assert.deepEqual(yield* decodeRequestPermissionResponse(aliasedOutbound), {
+        jsonrpc: "2.0",
+        id: nonNumericRequestId,
+        result: {
+          outcome: {
+            outcome: "selected",
+            optionId: "allow_once",
+          },
+        },
+      });
+    }),
+  );
+
   it.effect("cleans up interrupted extension requests before a late response arrives", () =>
     Effect.gen(function* () {
       const { stdio, input, output } = yield* makeInMemoryStdio();
diff --git a/packages/effect-acp/src/protocol.ts b/packages/effect-acp/src/protocol.ts
index 47fa575cac1..e6bf0cedbfc 100644
--- a/packages/effect-acp/src/protocol.ts
+++ b/packages/effect-acp/src/protocol.ts
@@ -86,6 +86,7 @@ export const makeAcpPatchedProtocol = Effect.fn("makeAcpPatchedProtocol")(functi
   const nextRequestId = yield* Ref.make(1n);
   const nextServerRequestId = yield* Ref.make(1n);
   const serverRequestIdAliases = yield* Ref.make(new Map<string, string>());
+  const serverNativeRequestIds = yield* Ref.make(new Set<string>());
   const terminationHandled = yield* Ref.make(false);
   const extPending = yield* Ref.make(
     new Map<string, Deferred.Deferred<unknown, AcpError.AcpError>>(),
@@ -281,29 +282,53 @@ export const makeAcpPatchedProtocol = Effect.fn("makeAcpPatchedProtocol")(functi
     );
   };
 
+  const allocateServerRequestAliasId = Effect.fn("allocateServerRequestAliasId")(function* () {
+    const nativeRequestIds = yield* Ref.get(serverNativeRequestIds);
+    const aliases = yield* Ref.get(serverRequestIdAliases);
+    return yield* Ref.modify(nextServerRequestId, (current) => {
+      let candidate = current;
+      while (nativeRequestIds.has(String(candidate)) || aliases.has(String(candidate))) {
+        candidate += 1n;
+      }
+      return [String(candidate), candidate + 1n] as const;
+    });
+  });
+
   const normalizeServerRequestId = (
     message: RpcMessage.RequestEncoded,
   ): Effect.Effect<RpcMessage.RequestEncoded> => {
-    if (isRpcServerCompatibleRequestId(message.id)) {
-      return Effect.succeed(message);
-    }
-
-    return Ref.modify(nextServerRequestId, (current) => {
-      const internalRequestId = String(current);
-      return [internalRequestId, current + 1n] as const;
+    return Effect.all({
+      aliases: Ref.get(serverRequestIdAliases),
+      nativeRequestIds: Ref.get(serverNativeRequestIds),
     }).pipe(
-      Effect.flatMap((internalRequestId) =>
-        Ref.update(serverRequestIdAliases, (aliases) => {
-          const next = new Map(aliases);
-          next.set(internalRequestId, message.id);
-          return next;
-        }).pipe(
-          Effect.as({
-            ...message,
-            id: internalRequestId,
-          }),
-        ),
-      ),
+      Effect.flatMap(({ aliases, nativeRequestIds }) => {
+        if (
+          isRpcServerCompatibleRequestId(message.id) &&
+          !aliases.has(message.id) &&
+          !nativeRequestIds.has(message.id)
+        ) {
+          return Ref.update(serverNativeRequestIds, (requestIds) => {
+            const next = new Set(requestIds);
+            next.add(message.id);
+            return next;
+          }).pipe(Effect.as(message));
+        }
+
+        return allocateServerRequestAliasId().pipe(
+          Effect.flatMap((internalRequestId) =>
+            Ref.update(serverRequestIdAliases, (aliases) => {
+              const next = new Map(aliases);
+              next.set(internalRequestId, message.id);
+              return next;
+            }).pipe(
+              Effect.as({
+                ...message,
+                id: internalRequestId,
+              }),
+            ),
+          ),
+        );
+      }),
     );
   };
 
@@ -325,7 +350,21 @@ export const makeAcpPatchedProtocol = Effect.fn("makeAcpPatchedProtocol")(functi
         next.delete(response.requestId);
       }
       return [{ ...response, requestId: originalRequestId }, next] as const;
-    });
+    }).pipe(
+      Effect.flatMap((restoredResponse) => {
+        if (restoredResponse !== response || response._tag !== "Exit") {
+          return Effect.succeed(restoredResponse);
+        }
+        return Ref.update(serverNativeRequestIds, (requestIds) => {
+          if (!requestIds.has(response.requestId)) {
+            return requestIds;
+          }
+          const next = new Set(requestIds);
+          next.delete(response.requestId);
+          return next;
+        }).pipe(Effect.as(response));
+      }),
+    );
   };
 
   const handleRequestEncoded = (message: RpcMessage.RequestEncoded) => {
diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts
index cf299982a19..f9d05216f41 100644
--- a/packages/shared/src/shell.test.ts
+++ b/packages/shared/src/shell.test.ts
@@ -62,7 +62,7 @@ describe("readPathFromLoginShell", () => {
     expect(shell).toBe("/opt/homebrew/bin/fish");
     expect(args).toHaveLength(2);
     expect(args?.[0]).toBe("-ilc");
-    expect(args?.[1]).toContain("printenv PATH");
+    expect(args?.[1]).toContain("printenv PATH || true");
     expect(args?.[1]).toContain("__T3CODE_ENV_PATH_START__");
     expect(args?.[1]).toContain("__T3CODE_ENV_PATH_END__");
     expect(options).toEqual({ encoding: "utf8", timeout: 5000 });
@@ -125,6 +125,9 @@ describe("readEnvironmentFromLoginShell", () => {
       SSH_AUTH_SOCK: "/tmp/secretive.sock",
     });
     expect(execFile).toHaveBeenCalledTimes(1);
+    const firstCall = execFile.mock.calls[0];
+    expect(firstCall?.[1][1]).toContain("printenv PATH || true");
+    expect(firstCall?.[1][1]).toContain("printenv SSH_AUTH_SOCK || true");
   });
 
   it("omits environment variables that are missing or empty", () => {
diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts
index bd1b31aa846..0572aaf989a 100644
--- a/packages/shared/src/shell.ts
+++ b/packages/shared/src/shell.ts
@@ -135,7 +135,7 @@ function buildEnvironmentCaptureCommand(names: ReadonlyArray<string>): string {
 
       return [
         `printf '%s\\n' '${envCaptureStart(name)}'`,
-        `printenv ${name}`,
+        `printenv ${name} || true`,
         `printf '%s\\n' '${envCaptureEnd(name)}'`,
       ].join("; ");
     })

From 7d1ec6d2131fe6935282c73f232042b231755609 Mon Sep 17 00:00:00 2001
From: Declan Cowen <declan@cowen.co>
Date: Sat, 23 May 2026 22:37:37 +0100
Subject: [PATCH 3/3] Restrict credentialed browser CORS origins

---
 .reviews/pr2793-automation-feedback-review.md | 79 ++++++++++++++++++-
 apps/server/src/http.test.ts                  | 15 ++++
 apps/server/src/http.ts                       | 46 ++++++-----
 apps/server/src/httpCors.ts                   | 43 ++++++++--
 apps/server/src/server.test.ts                | 42 ++++++++--
 5 files changed, 189 insertions(+), 36 deletions(-)

diff --git a/.reviews/pr2793-automation-feedback-review.md b/.reviews/pr2793-automation-feedback-review.md
index 09d706f1b20..ab9f362d8c7 100644
--- a/.reviews/pr2793-automation-feedback-review.md
+++ b/.reviews/pr2793-automation-feedback-review.md
@@ -16,6 +16,7 @@
 - `apps/desktop/src/updates/DesktopUpdates.ts` — desktop updater install failure recovery.
 - `.github/workflows/desktop-release.yml` — manual desktop release tag/ref resolution.
 - `apps/web/src/components/chat/ComposerPrimaryActions.tsx` — compact idle send button rendering.
+- `apps/server/src/httpCors.ts` / `apps/server/src/http.ts` — credentialed browser API CORS trust boundary.
 
 ## Hotspots
 
@@ -24,16 +25,17 @@
 - Desktop update install handoff partial-failure recovery after the backend is stopped.
 - Manual release artifact provenance for versioned desktop releases.
 - Compact composer layout parity.
+- Credentialed browser API CORS origin reflection.
 
 ## Review status
 
 | Field                 | Value                |
 | --------------------- | -------------------- |
 | **Review started**    | 2026-05-23 22:15 BST |
-| **Last reviewed**     | 2026-05-23 22:15 BST |
-| **Total turns**       | 1                    |
+| **Last reviewed**     | 2026-05-23 22:35 BST |
+| **Total turns**       | 2                    |
 | **Open findings**     | 0                    |
-| **Resolved findings** | 5                    |
+| **Resolved findings** | 6                    |
 | **Accepted findings** | 0                    |
 
 ## Automation
@@ -45,6 +47,77 @@
 | **State authority**  | GitHub review threads              |
 | **Review file role** | Human-readable local review ledger |
 
+## Turn 2 — 2026-05-23 22:35 BST
+
+| Field           | Value        |
+| --------------- | ------------ |
+| **Commit**      | working tree |
+| **IDE / Agent** | Codex        |
+
+### Automation context
+
+| Field                          | Value                                      |
+| ------------------------------ | ------------------------------------------ |
+| **Trigger**                    | Fresh `@codex review` after push           |
+| **PR**                         | `pingdotgg/t3code#2793`                    |
+| **Base ref**                   | `main`                                     |
+| **Head SHA**                   | `dd6e413d243d0c521128713c5aaf7ba902b02c2d` |
+| **Previous reviewed head SHA** | `657f253c1d2daebc5ad7268d1bf8277d281397c8` |
+| **Trusted state source**       | `gh api graphql pullRequest.reviewThreads` |
+| **Verification policy**        | Focused CORS/server tests, then repo gates |
+
+**Summary:** Imported and fixed Codex's fresh P1 finding that credentialed browser API CORS reflected arbitrary valid HTTP(S) origins.
+**Outcome:** Resolved in the current tree; pending commit/push when required gates pass.
+**Risk score:** High — this is a credentialed cross-origin API boundary on authenticated local/remote server APIs.
+**Change archetypes:** API/transport security, auth-adjacent browser compatibility, hosted/loopback variant hardening.
+**Intended change:** Preserve legitimate hosted-app and local-dev browser access while preventing arbitrary websites from receiving credentialed CORS headers.
+**Intent vs actual:** The server CORS predicate now accepts only loopback origins and the known hosted app origins (`app.t3.codes`, `latest.app.t3.codes`, `nightly.app.t3.codes`). The global CORS middleware was replaced with a server-owned middleware so `access-control-allow-credentials: true` is emitted only for trusted origins, including preflight responses.
+**Confidence:** High for the reviewed boundary and covered variants; hosted custom-domain CORS remains intentionally unsupported without a future explicit configuration surface.
+**Coverage note:** Added focused helper coverage for trusted/hostile origins and an integration regression proving untrusted auth failures are not reflected and do not receive credentialed CORS.
+**Finding triage:** Live in `dd6e413d`; fixed in the current working tree.
+**Static/analyzer evidence:** No analyzer policy changed. Fallow remains unavailable in this repo/PATH.
+**Architecture impact:** The credentialed CORS invariant is now owned by `httpCors.ts` and applied at the HTTP transport middleware boundary in `http.ts`; route handlers keep using the same helper instead of duplicating origin policy.
+**Bug classes / invariants checked:** auth-adjacent cross-origin trust boundary; loopback vs hosted vs hostile origin variants; credential header emission must be tied to explicit trusted origins.
+**Branch totality:** Rechecked the PR automation threads after the first push, resolved stale/outdated prior Codex threads, and imported the only fresh current-SHA Codex finding.
+**Sibling closure:** Checked auth success, auth failure, websocket-token preflight, public environment descriptor, and browser OTLP preflight paths that receive browser API CORS headers.
+**Remediation impact surface:** Affects all browser API routes through the global route layer. Local loopback dev and known hosted app origins continue to work; arbitrary hosted origins receive no credentialed CORS.
+**Residual risk / unknowns:** Custom hosted app origins would need an explicit future server config field rather than falling through to arbitrary origin reflection.
+
+### External Finding Import
+
+| Source | Finding                                              | Current status | Bug class                                 | Missed invariant/variant                                | Action                                             |
+| ------ | ---------------------------------------------------- | -------------- | ----------------------------------------- | ------------------------------------------------------- | -------------------------------------------------- |
+| Codex  | Credentialed CORS reflects arbitrary HTTP(S) origins | resolved       | Auth-Adjacent Cross-Origin Trust Boundary | credentials must only be allowed for trusted UI origins | fixed with trusted-origin predicate and middleware |
+
+### Validation
+
+- `bun run --cwd apps/server test src/http.test.ts src/server.test.ts` — passed, 70 tests.
+
+### Branch-totality proof
+
+- **Non-delta files/systems re-read:** fresh Codex thread, Effect HTTP CORS middleware implementation, server CORS/auth tests, hosted pairing docs, release hosted app domain docs, auth route CORS helper.
+- **Prior open findings rechecked:** First five imported findings remain fixed; their old GitHub threads are resolved or outdated.
+- **Prior resolved/adjacent areas revalidated:** Macroscope correctness passed on `dd6e413d`; Cursor Bugbot passed on `dd6e413d`; stale Codex ACP/shell threads were resolved after current-tree proof.
+- **Hotspots or sibling paths revisited:** credentialed auth responses, auth failures, preflight handling, public descriptor response headers, OTLP trace preflight, local loopback dev redirect helper.
+- **Dependency/adjacent surfaces revalidated:** Hosted pairing expects direct backend reachability from the browser, and hosted release docs define the known app origins that remain trusted.
+- **Why this is enough:** The original defect was arbitrary-origin reflection with credentials; the current middleware no longer reflects untrusted origins and the regression test asserts the hostile-origin failure path.
+
+### Challenger pass
+
+Done — assumed the generic Effect CORS middleware could still leak credential headers even after tightening `allowedOrigins`. The first focused test caught that exact companion issue, so the fix now owns both origin reflection and credential-header emission.
+
+### Resolved / Carried / New findings
+
+- `PR2793-CORS-001` — resolved in the current working tree.
+  - Fingerprint: `apps/server/src/httpCors.ts:isBrowserApiCorsOriginAllowed:credentialed-cors-trust-boundary`
+  - Evidence: trusted origin allowlist and custom middleware conditional credential headers.
+  - Verification: `bun run --cwd apps/server test src/http.test.ts src/server.test.ts`.
+
+### Recommendations
+
+1. **Fix first:** commit and push the CORS remediation after repo gates pass.
+2. **Then address:** monitor PR checks again; only Vercel authorization should remain external if no new findings appear.
+
 ## Turn 1 — 2026-05-23 22:15 BST
 
 | Field           | Value        |
diff --git a/apps/server/src/http.test.ts b/apps/server/src/http.test.ts
index de861cc6645..dafd04db23b 100644
--- a/apps/server/src/http.test.ts
+++ b/apps/server/src/http.test.ts
@@ -1,6 +1,7 @@
 import { describe, expect, it } from "vitest";
 
 import { isLoopbackHostname, resolveDevRedirectUrl } from "./http.ts";
+import { isBrowserApiCorsOriginAllowed } from "./httpCors.ts";
 
 describe("http dev routing", () => {
   it("treats localhost and loopback addresses as local", () => {
@@ -24,4 +25,18 @@ describe("http dev routing", () => {
       "http://127.0.0.1:5173/pair?token=test-token",
     );
   });
+
+  it("allows credentialed browser API CORS only for loopback and hosted app origins", () => {
+    expect(isBrowserApiCorsOriginAllowed("http://localhost:5173")).toBe(true);
+    expect(isBrowserApiCorsOriginAllowed("http://127.0.0.1:5173")).toBe(true);
+    expect(isBrowserApiCorsOriginAllowed("http://[::1]:5173")).toBe(true);
+    expect(isBrowserApiCorsOriginAllowed("https://app.t3.codes")).toBe(true);
+    expect(isBrowserApiCorsOriginAllowed("https://latest.app.t3.codes")).toBe(true);
+    expect(isBrowserApiCorsOriginAllowed("https://nightly.app.t3.codes")).toBe(true);
+
+    expect(isBrowserApiCorsOriginAllowed("https://evil.example")).toBe(false);
+    expect(isBrowserApiCorsOriginAllowed("http://remote-client.test:3773")).toBe(false);
+    expect(isBrowserApiCorsOriginAllowed("https://app.t3.codes.evil.example")).toBe(false);
+    expect(isBrowserApiCorsOriginAllowed("https://user@app.t3.codes")).toBe(false);
+  });
 });
diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts
index 92d128ceb6d..a59df6f88a5 100644
--- a/apps/server/src/http.ts
+++ b/apps/server/src/http.ts
@@ -10,6 +10,7 @@ import {
   HttpBody,
   HttpClient,
   HttpClientResponse,
+  HttpEffect,
   HttpMiddleware,
   HttpRouter,
   HttpServerResponse,
@@ -30,36 +31,43 @@ import { ServerAuth } from "./auth/Services/ServerAuth.ts";
 import { respondToAuthError } from "./auth/http.ts";
 import { ServerEnvironment } from "./environment/Services/ServerEnvironment.ts";
 import {
-  browserApiCorsAllowedHeaders,
-  browserApiCorsAllowedMethods,
+  browserApiCorsHeadersForOrigin,
+  browserApiCorsPreflightHeadersForOrigin,
   browserApiCorsHeaders,
-  isBrowserApiCorsOriginAllowed,
+  isLoopbackHostname,
 } from "./httpCors.ts";
 
 const PROJECT_FAVICON_CACHE_CONTROL = "public, max-age=3600";
 const FALLBACK_PROJECT_FAVICON_SVG = `<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24" height="24" fill="none" stroke="#6b728080" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" data-fallback="project-favicon"><path d="M20 20a2 2 0 0 0 2-2V8a2 2 0 0 0-2-2h-8l-2-2H4a2 2 0 0 0-2 2v12a2 2 0 0 0 2 2Z"/></svg>`;
 const OTLP_TRACES_PROXY_PATH = "/api/observability/v1/traces";
-const LOOPBACK_HOSTNAMES = new Set(["127.0.0.1", "::1", "localhost"]);
+
+export { isLoopbackHostname } from "./httpCors.ts";
 
 export const browserApiCorsLayer = HttpRouter.middleware(
-  HttpMiddleware.cors({
-    allowedOrigins: isBrowserApiCorsOriginAllowed,
-    allowedMethods: [...browserApiCorsAllowedMethods],
-    allowedHeaders: [...browserApiCorsAllowedHeaders],
-    credentials: true,
-    maxAge: 600,
-  }),
+  HttpMiddleware.make((httpApp) =>
+    Effect.gen(function* () {
+      const request = yield* HttpServerRequest.HttpServerRequest;
+      if (request.method === "OPTIONS") {
+        return HttpServerResponse.empty({
+          status: 204,
+          headers: browserApiCorsPreflightHeadersForOrigin(request.headers.origin),
+        });
+      }
+
+      yield* HttpEffect.appendPreResponseHandler((request, response) =>
+        Effect.succeed(
+          HttpServerResponse.setHeaders(
+            response,
+            browserApiCorsHeadersForOrigin(request.headers.origin),
+          ),
+        ),
+      );
+      return yield* httpApp;
+    }),
+  ),
   { global: true },
 );
 
-export function isLoopbackHostname(hostname: string): boolean {
-  const normalizedHostname = hostname
-    .trim()
-    .toLowerCase()
-    .replace(/^\[(.*)\]$/, "$1");
-  return LOOPBACK_HOSTNAMES.has(normalizedHostname);
-}
-
 export function resolveDevRedirectUrl(devUrl: URL, requestUrl: URL): string {
   const redirectUrl = new URL(devUrl.toString());
   redirectUrl.pathname = requestUrl.pathname;
diff --git a/apps/server/src/httpCors.ts b/apps/server/src/httpCors.ts
index 0fb637f8ff9..2c5a41f3f5b 100644
--- a/apps/server/src/httpCors.ts
+++ b/apps/server/src/httpCors.ts
@@ -6,11 +6,27 @@ export const browserApiCorsAllowedHeaders = [
   "content-type",
 ] as const;
 
+const LOOPBACK_HOSTNAMES = new Set(["127.0.0.1", "::1", "localhost"]);
+const BROWSER_API_TRUSTED_ORIGINS = new Set([
+  "https://app.t3.codes",
+  "https://latest.app.t3.codes",
+  "https://nightly.app.t3.codes",
+]);
+
 export const browserApiCorsHeaders = {
   "access-control-allow-origin": "*",
   "access-control-allow-methods": browserApiCorsAllowedMethods.join(", "),
   "access-control-allow-headers": browserApiCorsAllowedHeaders.join(", "),
 } as const;
+export const browserApiCorsMaxAgeSeconds = 600;
+
+export function isLoopbackHostname(hostname: string): boolean {
+  const normalizedHostname = hostname
+    .trim()
+    .toLowerCase()
+    .replace(/^\[(.*)\]$/, "$1");
+  return LOOPBACK_HOSTNAMES.has(normalizedHostname);
+}
 
 export function isBrowserApiCorsOriginAllowed(origin: string | undefined): origin is string {
   if (origin === undefined || origin.trim().length === 0) {
@@ -19,12 +35,16 @@ export function isBrowserApiCorsOriginAllowed(origin: string | undefined): origi
 
   try {
     const url = new URL(origin);
-    return (
-      url.origin === origin &&
-      (url.protocol === "http:" || url.protocol === "https:") &&
-      url.username.length === 0 &&
-      url.password.length === 0
-    );
+    if (
+      url.origin !== origin ||
+      (url.protocol !== "http:" && url.protocol !== "https:") ||
+      url.username.length !== 0 ||
+      url.password.length !== 0
+    ) {
+      return false;
+    }
+
+    return isLoopbackHostname(url.hostname) || BROWSER_API_TRUSTED_ORIGINS.has(url.origin);
   } catch {
     return false;
   }
@@ -42,3 +62,14 @@ export function browserApiCorsHeadersForOrigin(origin: string | undefined) {
     vary: "Origin",
   } as const;
 }
+
+export function browserApiCorsPreflightHeadersForOrigin(origin: string | undefined) {
+  if (!isBrowserApiCorsOriginAllowed(origin)) {
+    return {};
+  }
+
+  return {
+    ...browserApiCorsHeadersForOrigin(origin),
+    "access-control-max-age": String(browserApiCorsMaxAgeSeconds),
+  } as const;
+}
diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts
index a26b7577ec9..d1f224848f8 100644
--- a/apps/server/src/server.test.ts
+++ b/apps/server/src/server.test.ts
@@ -926,7 +926,8 @@ const assertBrowserApiCorsHeaders = (headers: Headers, options?: { readonly orig
     "traceparent",
   ]);
 };
-const crossOriginClientOrigin = "http://remote-client.test:3773";
+const trustedHostedClientOrigin = "https://app.t3.codes";
+const untrustedBrowserClientOrigin = "https://evil.example";
 
 const getWsServerUrl = (
   pathname = "",
@@ -1057,7 +1058,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
       const response = yield* Effect.promise(() =>
         fetch(url, {
           headers: {
-            origin: crossOriginClientOrigin,
+            origin: trustedHostedClientOrigin,
           },
         }),
       );
@@ -1066,7 +1067,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
       )) as typeof testEnvironmentDescriptor;
 
       assert.equal(response.status, 200);
-      assertBrowserApiCorsHeaders(response.headers, { origin: crossOriginClientOrigin });
+      assertBrowserApiCorsHeaders(response.headers, { origin: trustedHostedClientOrigin });
       assert.deepEqual(body, testEnvironmentDescriptor);
     }).pipe(Effect.provide(NodeHttpServer.layerTest)),
   );
@@ -1198,7 +1199,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
     Effect.gen(function* () {
       yield* buildAppUnderTest();
 
-      const origin = crossOriginClientOrigin;
+      const origin = trustedHostedClientOrigin;
       const { response: bootstrapResponse, body: bootstrapBody } = yield* bootstrapBearerSession(
         defaultDesktopBootstrapToken,
         {
@@ -1261,7 +1262,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
           fetch(wsTokenUrl, {
             method: "OPTIONS",
             headers: {
-              origin: crossOriginClientOrigin,
+              origin: trustedHostedClientOrigin,
               "access-control-request-method": "POST",
               "access-control-request-headers": "authorization",
             },
@@ -1269,7 +1270,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
         );
 
         assert.equal(response.status, 204);
-        assertBrowserApiCorsHeaders(response.headers, { origin: crossOriginClientOrigin });
+        assertBrowserApiCorsHeaders(response.headers, { origin: trustedHostedClientOrigin });
       }).pipe(Effect.provide(NodeHttpServer.layerTest)),
   );
 
@@ -1282,7 +1283,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
         fetch(wsTokenUrl, {
           method: "POST",
           headers: {
-            origin: crossOriginClientOrigin,
+            origin: trustedHostedClientOrigin,
           },
         }),
       );
@@ -1291,7 +1292,32 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
       };
 
       assert.equal(response.status, 401);
-      assertBrowserApiCorsHeaders(response.headers, { origin: crossOriginClientOrigin });
+      assertBrowserApiCorsHeaders(response.headers, { origin: trustedHostedClientOrigin });
+      assert.equal(body.error, "Authentication required.");
+    }).pipe(Effect.provide(NodeHttpServer.layerTest)),
+  );
+
+  it.effect("does not reflect untrusted origins on remote websocket-token auth failures", () =>
+    Effect.gen(function* () {
+      yield* buildAppUnderTest();
+
+      const wsTokenUrl = yield* getHttpServerUrl("/api/auth/ws-token");
+      const response = yield* Effect.promise(() =>
+        fetch(wsTokenUrl, {
+          method: "POST",
+          headers: {
+            origin: untrustedBrowserClientOrigin,
+          },
+        }),
+      );
+      const body = (yield* Effect.promise(() => response.json())) as {
+        readonly error?: string;
+      };
+
+      assert.equal(response.status, 401);
+      assertBrowserApiCorsHeaders(response.headers);
+      assert.isNull(response.headers.get("access-control-allow-credentials"));
+      assert.notInclude(splitHeaderTokens(response.headers.get("vary")), "Origin");
       assert.equal(body.error, "Authentication required.");
     }).pipe(Effect.provide(NodeHttpServer.layerTest)),
   );