refactor(server): share attachment message intake#10580

merged3
agents/mcp-shared/project-inputs
agents/mcp-shared/attachment-intake
openedSep 7, 2026byJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 3d ago
4 unresolved threads ยท Jump to file
Selected thread: apps/server/src/orchestration-v2/ThreadMessageIntake.ts:49 โ€” ๐ŸŸ  **High** `orchestration-v2/ThreadMessageIntake.ts:46` `launchThread` leaks the copied attachment whenever `launches.launch` fails before accepting the initial message. `claimPendingAttachments` creates a non-pending final path, so the pending sweep never reclaims it and repeated failed launches accumulate orphaned files; release `claimed.claimedPaths` on launch failure. ```diff - return yield* launches.launch({ + return yield* launches.launch({ ...input, initialMessage: { ...input.initialMessage, attachments: claimed.attachments }, - }); + }).pipe( + Effect.tapError(() => + AttachmentClaims.releaseClaimedAttachments(claimed.claimedPaths), + ), + ); ``` <details> <summary>Also found in 2 other location(s)</summary> `apps/server/src/ws.ts:1319` > `ThreadMessageIntake.dispatchCommand` claims and copies pending attachments before dispatch, but the new path no longer releases `claimedPaths` if `threads.dispatch` fails before accepting the command. For example, dispatching a valid staged upload to a deleted/nonexistent thread copies it under that thread id, then `ensureCommandTranscripts`/the orchestrator can fail. `sweepStalePendingAttachments` only deletes ids in the pending segment, so this final-path copy is retained indefinitely despite no message referencing it; repeated failed submissions accumulate orphaned attachment files. `apps/server/src/ws.ts:1437` > `ThreadMessageIntake.launchThread` copies pending attachments before `launches.launch`, but no longer removes the copy when launch fails before the initial message is committed. A launch using a staged attachment and a nonexistent project fails during `ThreadLaunchService.launch`&#39;s project lookup, leaving the copied final attachment unreferenced. These ids are not in the pending segment, so `sweepStalePendingAttachments` will never reclaim them; failed launch attempts leak attachment files. </details> <details> <summary>๐Ÿš€ Reply "<strong>fix it for me</strong>" or copy this <strong>AI Prompt</strong> for your agent:</summary> ```text In file @apps/server/src/orchestration-v2/ThreadMessageIntake.ts around lines 46-49: `launchThread` leaks the copied attachment whenever `launches.launch` fails before accepting the initial message. `claimPendingAttachments` creates a non-pending final path, so the pending sweep never reclaims it and repeated failed launches accumulate orphaned files; release `claimed.claimedPaths` on launch failure. Also found in 2 other location(s): - apps/server/src/ws.ts:1319 -- `ThreadMessageIntake.dispatchCommand` claims and copies pending attachments before dispatch, but the new path no longer releases `claimedPaths` if `threads.dispatch` fails before accepting the command. For example, dispatching a valid staged upload to a deleted/nonexistent thread copies it under that thread id, then `ensureCommandTranscripts`/the orchestrator can fail. `sweepStalePendingAttachments` only deletes ids in the pending segment, so this final-path copy is retained indefinitely despite no message referencing it; repeated failed submissions accumulate orphaned attachment files. - apps/server/src/ws.ts:1437 -- `ThreadMessageIntake.launchThread` copies pending attachments before `launches.launch`, but no longer removes the copy when launch fails before the initial message is committed. A launch using a staged attachment and a nonexistent project fails during `ThreadLaunchService.launch`'s project lookup, leaving the copied final attachment unreferenced. These ids are not in the pending segment, so `sweepStalePendingAttachments` will never reclaim them; failed launch attempts leak attachment files. ``` </details> <!-- macroscope-meta: {"kind":"code_review","variant":"f57f0b61c1d8f13d673564c863408287"} -->
0 / 4 viewed4 files changedLine totals unavailable

apps/server/src/orchestration-v2/ThreadMessageIntake.ts ยท Earlier revision 43e6ee2a ยท right 42

Original diff
@@ -0,0 +1,50 @@
+import type { OrchestrationV2Command } from "@t3tools/contracts";
+import * as Effect from "effect/Effect";
+
+import * as AttachmentClaims from "./AttachmentClaims.ts";
+import * as ThreadLaunch from "./ThreadLaunchService.ts";
+import * as ThreadManagement from "./ThreadManagementService.ts";
+
+// Claim failures clean up their partial batch. Once dispatch/launch starts, its
+// failure can follow a durable commit, so callers must not delete claimed files.
+export const dispatchCommand = Effect.fn("ThreadMessageIntake.dispatchCommand")(function* (
+  command: OrchestrationV2Command,
+) {
+  const threads = yield* ThreadManagement.ThreadManagementService;
+  if (command.type !== "message.dispatch") return yield* threads.dispatch(command);
+  const claimed = yield* AttachmentClaims.claimPendingAttachments({
+    threadId: command.threadId,
+    attachments: command.attachments,
+  });
+  return yield* threads.dispatch({ ...command, attachments: claimed.attachments });
+});
+
+export const sendToThread = Effect.fn("ThreadMessageIntake.sendToThread")(function* (
+  input: ThreadManagement.ThreadManagementSendInput,
+) {
+  const threads = yield* ThreadManagement.ThreadManagementService;
+  const claimed = yield* AttachmentClaims.claimPendingAttachments(input);
+  return yield* threads.sendToThread({ ...input, attachments: claimed.attachments });
+});
+
+export const launchThread = Effect.fn("ThreadMessageIntake.launchThread")(function* (
+  input: ThreadLaunch.ThreadLaunchInput,
+) {
+  const launches = yield* ThreadLaunch.ThreadLaunchService;
+  if (!input.initialMessage?.attachments.some(AttachmentClaims.attachmentIsPendingUpload)) {
+    return yield* launches.launch(input);
+  }
+  if (input.threadId === undefined) {
+    return yield* new AttachmentClaims.AttachmentClaimError({
+      message: "Uploaded attachments need a thread id at launch.",
+    });
+  }
+  const claimed = yield* AttachmentClaims.claimPendingAttachments({
M
macroscopeappBotSep 7, 2026

๐ŸŸ  High orchestration-v2/ThreadMessageIntake.ts:42

Retrying an already accepted launch after a lost response copies the pending attachment to a new random thread-scoped path, then launches.launch returns the existing receipt without referencing that copy. The unreferenced file is never released, so each retry permanently leaks an attachment; perform the idempotency check before claiming or release claimed.claimedPaths when the launch is already committed.

๐Ÿš€ Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @apps/server/src/orchestration-v2/ThreadMessageIntake.ts around line 42:
2
3Retrying an already accepted launch after a lost response copies the pending attachment to a new random thread-scoped path, then `launches.launch` returns the existing receipt without referencing that copy. The unreferenced file is never released, so each retry permanently leaks an attachment; perform the idempotency check before claiming or release `claimed.claimedPaths` when the launch is already committed.
J
Julius MarmingeSep 7, 2026

Fixed in c80aae28752480aca3d0ec0d9a773dca4586e821. Shared intake now removes claims not referenced by the returned durable message/events, including accepted launch, command, and send replays. It also removes copies on typed receipt/planning rejections and launch project/receipt failures before message dispatch. The real V2 regression now proves replays leave only accepted files and missing-project/thread requests leave no orphaned copies. The same regression retains the lost-result-after-acceptance case and verifies the original bytes survive. Generic launch/dispatch errors remain ambiguous, so blanket tapError cleanup would reintroduce data loss; those copies are intentionally retained. No new receipt store, journal, lock, or read-after-failure query was added. Focused suites: 38 passed; server typecheck and targeted lint/format passed.

apps/server/src/orchestration-v2/ThreadMessageIntake.ts ยท Earlier revision 43e6ee2a ยท right 19

Original diff
@@ -0,0 +1,50 @@
+import type { OrchestrationV2Command } from "@t3tools/contracts";
+import * as Effect from "effect/Effect";
+
+import * as AttachmentClaims from "./AttachmentClaims.ts";
+import * as ThreadLaunch from "./ThreadLaunchService.ts";
+import * as ThreadManagement from "./ThreadManagementService.ts";
+
+// Claim failures clean up their partial batch. Once dispatch/launch starts, its
+// failure can follow a durable commit, so callers must not delete claimed files.
+export const dispatchCommand = Effect.fn("ThreadMessageIntake.dispatchCommand")(function* (
+  command: OrchestrationV2Command,
+) {
+  const threads = yield* ThreadManagement.ThreadManagementService;
+  if (command.type !== "message.dispatch") return yield* threads.dispatch(command);
+  const claimed = yield* AttachmentClaims.claimPendingAttachments({
+    threadId: command.threadId,
+    attachments: command.attachments,
+  });
+  return yield* threads.dispatch({ ...command, attachments: claimed.attachments });
M
macroscopeappBotSep 7, 2026

๐ŸŸ  High orchestration-v2/ThreadMessageIntake.ts:19

Failed dispatchCommand and launchThread calls permanently retain the thread-scoped copies created by claimPendingAttachments, so repeated requests for nonexistent threads or projects leave unreferenced upload payloads on disk and can exhaust server storage. These failures occur before a message or thread is accepted, but no cleanup follows threads.dispatch(...) or launches.launch(...), and the pending-file sweep does not remove the copies. Release the claimed attachments whenever dispatch or launch fails before acceptance.

๐Ÿš€ Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @apps/server/src/orchestration-v2/ThreadMessageIntake.ts around line 19:
2
3Failed `dispatchCommand` and `launchThread` calls permanently retain the thread-scoped copies created by `claimPendingAttachments`, so repeated requests for nonexistent threads or projects leave unreferenced upload payloads on disk and can exhaust server storage. These failures occur before a message or thread is accepted, but no cleanup follows `threads.dispatch(...)` or `launches.launch(...)`, and the pending-file sweep does not remove the copies. Release the claimed attachments whenever dispatch or launch fails before acceptance.
J
Julius MarmingeSep 7, 2026

Fixed in c80aae28752480aca3d0ec0d9a773dca4586e821. Shared intake now removes claims not referenced by the returned durable message/events, including accepted launch, command, and send replays. It also removes copies on typed receipt/planning rejections and launch project/receipt failures before message dispatch. The real V2 regression now proves replays leave only accepted files and missing-project/thread requests leave no orphaned copies. The same regression retains the lost-result-after-acceptance case and verifies the original bytes survive. Generic launch/dispatch errors remain ambiguous, so blanket tapError cleanup would reintroduce data loss; those copies are intentionally retained. No new receipt store, journal, lock, or read-after-failure query was added. Focused suites: 38 passed; server typecheck and targeted lint/format passed.

apps/server/src/orchestration-v2/ThreadMessageIntake.ts ยท Earlier revision 43e6ee2a ยท right 46โ€“49

Original diff
@@ -0,0 +1,50 @@
+import type { OrchestrationV2Command } from "@t3tools/contracts";
+import * as Effect from "effect/Effect";
+
+import * as AttachmentClaims from "./AttachmentClaims.ts";
+import * as ThreadLaunch from "./ThreadLaunchService.ts";
+import * as ThreadManagement from "./ThreadManagementService.ts";
+
+// Claim failures clean up their partial batch. Once dispatch/launch starts, its
+// failure can follow a durable commit, so callers must not delete claimed files.
+export const dispatchCommand = Effect.fn("ThreadMessageIntake.dispatchCommand")(function* (
+  command: OrchestrationV2Command,
+) {
+  const threads = yield* ThreadManagement.ThreadManagementService;
+  if (command.type !== "message.dispatch") return yield* threads.dispatch(command);
+  const claimed = yield* AttachmentClaims.claimPendingAttachments({
+    threadId: command.threadId,
+    attachments: command.attachments,
+  });
+  return yield* threads.dispatch({ ...command, attachments: claimed.attachments });
+});
+
+export const sendToThread = Effect.fn("ThreadMessageIntake.sendToThread")(function* (
+  input: ThreadManagement.ThreadManagementSendInput,
+) {
+  const threads = yield* ThreadManagement.ThreadManagementService;
+  const claimed = yield* AttachmentClaims.claimPendingAttachments(input);
+  return yield* threads.sendToThread({ ...input, attachments: claimed.attachments });
+});
+
+export const launchThread = Effect.fn("ThreadMessageIntake.launchThread")(function* (
+  input: ThreadLaunch.ThreadLaunchInput,
+) {
+  const launches = yield* ThreadLaunch.ThreadLaunchService;
+  if (!input.initialMessage?.attachments.some(AttachmentClaims.attachmentIsPendingUpload)) {
+    return yield* launches.launch(input);
+  }
+  if (input.threadId === undefined) {
+    return yield* new AttachmentClaims.AttachmentClaimError({
+      message: "Uploaded attachments need a thread id at launch.",
+    });
+  }
+  const claimed = yield* AttachmentClaims.claimPendingAttachments({
+    threadId: input.threadId,
+    attachments: input.initialMessage.attachments,
+  });
+  return yield* launches.launch({
+    ...input,
+    initialMessage: { ...input.initialMessage, attachments: claimed.attachments },
+  });

apps/server/src/orchestration-v2/ThreadMessageIntake.ts ยท right 46โ€“49

M
macroscopeappBotSep 7, 2026

๐ŸŸ  High orchestration-v2/ThreadMessageIntake.ts:46

launchThread leaks the copied attachment whenever launches.launch fails before accepting the initial message. claimPendingAttachments creates a non-pending final path, so the pending sweep never reclaims it and repeated failed launches accumulate orphaned files; release claimed.claimedPaths on launch failure.

diff
1- return yield* launches.launch({
2+ return yield* launches.launch({
3 ...input,
4 initialMessage: { ...input.initialMessage, attachments: claimed.attachments },
5- });
6+ }).pipe(
7+ Effect.tapError(() =>
8+ AttachmentClaims.releaseClaimedAttachments(claimed.claimedPaths),
9+ ),
10+ );
Also found in 2 other location(s)

apps/server/src/ws.ts:1319

ThreadMessageIntake.dispatchCommand claims and copies pending attachments before dispatch, but the new path no longer releases claimedPaths if threads.dispatch fails before accepting the command. For example, dispatching a valid staged upload to a deleted/nonexistent thread copies it under that thread id, then ensureCommandTranscripts/the orchestrator can fail. sweepStalePendingAttachments only deletes ids in the pending segment, so this final-path copy is retained indefinitely despite no message referencing it; repeated failed submissions accumulate orphaned attachment files.

apps/server/src/ws.ts:1437

ThreadMessageIntake.launchThread copies pending attachments before launches.launch, but no longer removes the copy when launch fails before the initial message is committed. A launch using a staged attachment and a nonexistent project fails during ThreadLaunchService.launch's project lookup, leaving the copied final attachment unreferenced. These ids are not in the pending segment, so sweepStalePendingAttachments will never reclaim them; failed launch attempts leak attachment files.

๐Ÿš€ Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @apps/server/src/orchestration-v2/ThreadMessageIntake.ts around lines 46-49:
2
3`launchThread` leaks the copied attachment whenever `launches.launch` fails before accepting the initial message. `claimPendingAttachments` creates a non-pending final path, so the pending sweep never reclaims it and repeated failed launches accumulate orphaned files; release `claimed.claimedPaths` on launch failure.
4
5Also found in 2 other location(s):
6- apps/server/src/ws.ts:1319 -- `ThreadMessageIntake.dispatchCommand` claims and copies pending attachments before dispatch, but the new path no longer releases `claimedPaths` if `threads.dispatch` fails before accepting the command. For example, dispatching a valid staged upload to a deleted/nonexistent thread copies it under that thread id, then `ensureCommandTranscripts`/the orchestrator can fail. `sweepStalePendingAttachments` only deletes ids in the pending segment, so this final-path copy is retained indefinitely despite no message referencing it; repeated failed submissions accumulate orphaned attachment files.
7- apps/server/src/ws.ts:1437 -- `ThreadMessageIntake.launchThread` copies pending attachments before `launches.launch`, but no longer removes the copy when launch fails before the initial message is committed. A launch using a staged attachment and a nonexistent project fails during `ThreadLaunchService.launch`'s project lookup, leaving the copied final attachment unreferenced. These ids are not in the pending segment, so `sweepStalePendingAttachments` will never reclaim them; failed launch attempts leak attachment files.
J
Julius MarmingeSep 7, 2026

Fixed in c80aae28752480aca3d0ec0d9a773dca4586e821. Shared intake now removes claims not referenced by the returned durable message/events, including accepted launch, command, and send replays. It also removes copies on typed receipt/planning rejections and launch project/receipt failures before message dispatch. The real V2 regression now proves replays leave only accepted files and missing-project/thread requests leave no orphaned copies. The same regression retains the lost-result-after-acceptance case and verifies the original bytes survive. Generic launch/dispatch errors remain ambiguous, so blanket tapError cleanup would reintroduce data loss; those copies are intentionally retained. No new receipt store, journal, lock, or read-after-failure query was added. Focused suites: 38 passed; server typecheck and targeted lint/format passed.

apps/server/src/ws.ts ยท Earlier revision c80aae28 ยท right 1484โ€“1495

Original diff
@@ -1454,104 +1432,68 @@ const makeWsRpcLayer = (
         [ORCHESTRATION_V2_WS_METHODS.launchThread]: (input) =>
           observeRpcEffect(
             ORCHESTRATION_V2_WS_METHODS.launchThread,
-            Effect.gen(function* () {
-              const pendingUploads =
-                input.initialMessage?.attachments.some(attachmentIsPendingUpload) ?? false;
-              if (pendingUploads && input.threadId === undefined) {
-                return yield* new OrchestrationV2ThreadLaunchError({
+            startup
+              .enqueueCommand(
+                ThreadMessageIntake.launchThread({
                   commandId: input.commandId,
+                  ...(input.threadId === undefined ? {} : { threadId: input.threadId }),
+                  ...(input.reuseExistingThread === undefined
+                    ? {}
+                    : { reuseExistingThread: input.reuseExistingThread }),
                   projectId: input.projectId,
-                  message: "Uploaded attachments need a thread id at launch.",
-                });
-              }
-              const claimed =
-                pendingUploads && input.threadId !== undefined && input.initialMessage !== undefined
-                  ? yield* claimPendingAttachments({
-                      threadId: input.threadId,
-                      attachments: input.initialMessage.attachments,
-                    }).pipe(
-                      Effect.mapError(
-                        (cause) =>
-                          new OrchestrationV2ThreadLaunchError({
-                            commandId: input.commandId,
-                            projectId: input.projectId,
-                            message: cause.message,
-                            cause,
-                          }),
-                      ),
-                    )
-                  : null;
-              const initialMessage =
-                input.initialMessage === undefined
-                  ? undefined
-                  : claimed === null
-                    ? input.initialMessage
-                    : { ...input.initialMessage, attachments: claimed.attachments };
-              return yield* startup
-                .enqueueCommand(
-                  threadLaunch.launch({
-                    commandId: input.commandId,
-                    ...(input.threadId === undefined ? {} : { threadId: input.threadId }),
-                    ...(input.reuseExistingThread === undefined
-                      ? {}
-                      : { reuseExistingThread: input.reuseExistingThread }),
-                    projectId: input.projectId,
-                    title: input.title,
-                    ...(input.generateTitle === undefined
-                      ? {}
-                      : { generateTitle: input.generateTitle }),
-                    modelSelection: input.modelSelection,
-                    runtimeMode: input.runtimeMode,
-                    interactionMode: input.interactionMode,
-                    workspaceStrategy: input.workspaceStrategy,
-                    ...(initialMessage === undefined
-                      ? {}
-                      : {
-                          initialMessage: {
-                            ...(initialMessage.messageId === undefined
-                              ? {}
-                              : { messageId: initialMessage.messageId }),
-                            text: initialMessage.text,
-                            attachments: initialMessage.attachments,
-                          },
-                        }),
-                    createdBy: "user",
-                    creationSource: input.creationSource ?? "web",
-                  }),
-                )
-                .pipe(
-                  Effect.tapError(() =>
-                    claimed === null
-                      ? Effect.void
-                      : releaseClaimedAttachments(claimed.claimedPaths),
-                  ),
-                  Effect.tap(() =>
-                    analytics
-                      .record("client.thread.started", originProps)
-                      .pipe(
-                        Effect.andThen(
-                          input.initialMessage === undefined
-                            ? Effect.void
-                            : analytics.record("client.turn.requested", originProps),
-                        ),
-                        Effect.ignore,
-                      ),
-                  ),
-                  Effect.map((result) => ({
-                    ...result,
-                    projection: projectThreadProjectionForWire(result.projection),
-                  })),
-                  Effect.mapError(
-                    (cause) =>
-                      new OrchestrationV2ThreadLaunchError({
-                        commandId: input.commandId,
-                        projectId: input.projectId,
-                        message: "Failed to launch thread",
-                        cause,
+                  title: input.title,
+                  ...(input.generateTitle === undefined
+                    ? {}
+                    : { generateTitle: input.generateTitle }),
+                  modelSelection: input.modelSelection,
+                  runtimeMode: input.runtimeMode,
+                  interactionMode: input.interactionMode,
+                  workspaceStrategy: input.workspaceStrategy,
+                  ...(input.initialMessage === undefined
+                    ? {}
+                    : {
+                        initialMessage: {
+                          ...(input.initialMessage.messageId === undefined
+                            ? {}
+                            : { messageId: input.initialMessage.messageId }),
+                          text: input.initialMessage.text,
+                          attachments: input.initialMessage.attachments,
+                        },
                       }),
-                  ),
-                );
-            }),
+                  createdBy: "user",
+                  creationSource: input.creationSource ?? "web",
+                }).pipe(Effect.provide(intakeContext)),
+              )
+              .pipe(
+                Effect.tap(() =>
+                  analytics
+                    .record("client.thread.started", originProps)
+                    .pipe(
+                      Effect.andThen(
+                        input.initialMessage === undefined
+                          ? Effect.void
+                          : analytics.record("client.turn.requested", originProps),
+                      ),
+                      Effect.ignore,
+                    ),
+                ),
+                Effect.map((result) => ({
+                  ...result,
+                  projection: projectThreadProjectionForWire(result.projection),
+                })),
+                Effect.mapError(
+                  (cause) =>
+                    new OrchestrationV2ThreadLaunchError({
+                      commandId: input.commandId,
+                      projectId: input.projectId,
+                      message:
+                        cause._tag === "AttachmentClaimError"
+                          ? cause.message
+                          : "Failed to launch thread",
+                      cause,
+                    }),
+                ),

apps/server/src/ws.ts ยท right 1484โ€“1495

M
macroscopeappBotSep 7, 2026

This catch-all mapError now inspects cause._tag and derives the wrapper's message from cause.message. Per the error conventions, a wrapper's message should come from its own attributes, and statically known tagged failures should be handled with Effect.catchTags where their context is known, instead of tag-sniffing a whole multi-step pipeline's error channel.

suggestion
1 Effect.catchTags({
2 AttachmentClaimError: (error) =>
3 new OrchestrationV2ThreadLaunchError({
4 commandId: input.commandId,
5 projectId: input.projectId,
6 message: error.message,
7 cause: error,
8 }),
9 }),
10 Effect.mapError(
11 (cause) =>
12 new OrchestrationV2ThreadLaunchError({
13 commandId: input.commandId,
14 projectId: input.projectId,
15 message: "Failed to launch thread",
16 cause,
17 }),
18 ),

Posted via Macroscope โ€” Effect Service Conventions

J
Julius MarmingeSep 7, 2026

Fixed in 4cfe3581d4: the RPC launch boundary now handles AttachmentClaimError, ThreadLaunchError, and ServerRuntimeStartupError explicitly with Effect.catchTags. The attachment claim message remains unchanged because it is an existing, safe caller-facing validation message; launch/startup failures retain the existing fixed message. The suggested catchTags-then-mapError sequence would rewrap the translated claim error and erase that message, so the mapping is exhaustive in one catchTags instead. Canonical server typecheck, targeted type-aware lint, formatter and diff checks pass. No behavior or new test machinery was added.