Merge pull request #159 from pylon-code/fix/followup-attachment-paths

fix(server): stop dropping attachments on queued follow-ups

Author: rynfar

apps/server/src/provider/Layers/ProviderService.test.ts

@@ -6,6 +6,7 @@ import * as NodePath from "node:path";
 import type {
   ProviderApprovalDecision,
   ProviderRuntimeEvent,
+  ProviderFollowUpInput,
   ProviderSendTurnInput,
   ProviderSession,
   ProviderTurnStartResult,
@@ -368,13 +369,27 @@ function makeFakeCodexAdapter(
       }),
   );
 
+  const followUp = vi.fn(
+    (
+      input: ProviderFollowUpInput,
+    ): Effect.Effect<SessionInputQueueUpdatedPayload, ProviderAdapterError> => {
+      if (!sessions.has(input.threadId)) {
+        return Effect.fail(
+          new ProviderAdapterSessionNotFoundError({ provider, threadId: input.threadId }),
+        );
+      }
+      return Effect.succeed(inputQueue);
+    },
+  );
+
   const adapter: ProviderAdapterShape<ProviderAdapterError> = {
     provider,
     capabilities: {
       sessionModelSwitch: "in-session",
     },
     startSession,
     sendTurn,
+    followUp,
     interruptTurn,
     respondToRequest,
     respondToUserInput,
@@ -430,6 +445,7 @@ function makeFakeCodexAdapter(
     updateSession,
     startSession,
     sendTurn,
+    followUp,
     respondToInteraction,
     reloadSessionResources,
     askSessionSideQuestion,
@@ -1588,6 +1604,20 @@ routing.layer("ProviderServiceLive routing", (it) => {
       assert.include(fileOnlyInput.input ?? "", '[Attached file "report.pdf" is saved at: ');
       assert.deepEqual(fileOnlyInput.attachments, [fileAttachment]);
 
+      // Follow-ups need the same path lines. Every adapter except OpenCode skips
+      // non-image attachments, so without this a file attached to a queued
+      // follow-up reaches the agent as nothing at all.
+      yield* provider.followUp({
+        threadId: session.threadId,
+        input: "and this one",
+        attachments: [fileAttachment],
+      });
+      const followUpInput = routing.codex.followUp.mock.calls[0]?.[0] as ProviderFollowUpInput;
+      assert.include(followUpInput.input ?? "", "and this one");
+      assert.include(followUpInput.input ?? "", '[Attached file "report.pdf" is saved at: ');
+      assert.include(followUpInput.input ?? "", `${fileAttachment.id}.pdf]`);
+      assert.deepEqual(followUpInput.attachments, [fileAttachment]);
+
       yield* provider.stopSession({ threadId: session.threadId });
     }),
   );

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

@@ -238,6 +238,43 @@ const correlateRuntimeEventWithInstance = (
   return { ...event, providerInstanceId: source.instanceId };
 };
 
+/**
+ * Appends an on-disk path line for every attachment so the model's tools can
+ * dereference the actual file.
+ *
+ * Every attachment also reaches the adapter, and each adapter decides what its
+ * provider ingests natively: OpenCode sends generic files as file parts, the
+ * others send images only and rely on these lines for everything else. That
+ * makes the path line the sole channel a non-image attachment has on those
+ * providers, which is why follow-ups need it exactly as much as turns do.
+ *
+ * Unresolvable ids are skipped here and surface as adapter errors when the file
+ * is read.
+ */
+const appendAttachmentPathLines = (
+  attachmentsDir: string,
+  input: string | undefined,
+  attachments: ReadonlyArray<{
+    readonly id: string;
+    readonly type: string;
+    readonly name: string;
+  }>,
+): string | undefined => {
+  const lines = attachments.flatMap((attachment) => {
+    const attachmentPath = resolveAttachmentPath({
+      attachmentsDir,
+      attachment: attachment as Parameters<typeof resolveAttachmentPath>[0]["attachment"],
+    });
+    return attachmentPath === null
+      ? []
+      : [`[Attached ${attachment.type} "${attachment.name}" is saved at: ${attachmentPath}]`];
+  });
+  if (lines.length === 0) return input;
+  return [input, lines.join("\n")]
+    .filter((part): part is string => typeof part === "string" && part.length > 0)
+    .join("\n\n");
+};
+
 const makeProviderService = Effect.fn("makeProviderService")(function* (
   options?: ProviderServiceLiveOptions,
 ) {
@@ -773,27 +810,11 @@ const makeProviderService = Effect.fn("makeProviderService")(function* (
       );
     }
 
-    // Every attachment gets an on-disk path in the prompt so the model's tools
-    // can dereference the actual file. All attachments then go to the adapter,
-    // and each adapter decides what its provider ingests natively: OpenCode
-    // sends generic files as file parts, the others send images only and rely
-    // on the path line for everything else. Unresolvable ids are skipped here
-    // and surface as adapter errors when the file is read.
-    const attachmentPathLines = attachments.flatMap((attachment) => {
-      const attachmentPath = resolveAttachmentPath({
-        attachmentsDir: serverConfig.attachmentsDir,
-        attachment,
-      });
-      return attachmentPath === null
-        ? []
-        : [`[Attached ${attachment.type} "${attachment.name}" is saved at: ${attachmentPath}]`];
-    });
-    const inputTextWithAttachmentPaths =
-      attachmentPathLines.length === 0
-        ? parsed.input
-        : [parsed.input, attachmentPathLines.join("\n")]
-            .filter((part): part is string => typeof part === "string" && part.length > 0)
-            .join("\n\n");
+    const inputTextWithAttachmentPaths = appendAttachmentPathLines(
+      serverConfig.attachmentsDir,
+      parsed.input,
+      attachments,
+    );
 
     const input = {
       ...parsed,
@@ -1337,7 +1358,19 @@ const makeProviderService = Effect.fn("makeProviderService")(function* (
       "provider.thread_id": input.threadId,
       "provider.attachment_count": input.attachments.length,
     });
-    return yield* followUpSession(input);
+    // Same path lines `sendTurn` appends. Without them a generic file attached
+    // to a follow-up is lost outright: every adapter except OpenCode skips
+    // non-images, so the path line is the only thing that tells the agent the
+    // file exists.
+    const followUpInputText = appendAttachmentPathLines(
+      serverConfig.attachmentsDir,
+      input.input,
+      input.attachments,
+    );
+    return yield* followUpSession({
+      ...input,
+      ...(followUpInputText !== undefined ? { input: followUpInputText } : {}),
+    });
   });
 
   const getSessionInputQueue: ProviderServiceMethod<"getSessionInputQueue"> = Effect.fn(