fix(agent): avoid duplicate "missing artifact" warning on the pre-refetch resolve

Review feedback (Greptile): getArtifactsById logs a per-id warning for each
unresolved id, and the new refetch path calls it twice, so a genuinely missing
artifact produced two identical low-level warnings on top of the high-level
summary. Add a warnOnMissing option (default true) and pass false on the
speculative pre-refetch resolve, where a miss is expected and is exactly what
triggers the refetch.

Generated-By: PostHog Code
Task-Id: 62e30413-5b19-4752-9520-b0e772c462ce

Author: fercgomes

packages/agent/src/server/agent-server.test.ts

@@ -2284,4 +2284,44 @@ describe("AgentServer pending user attachments", () => {
     expect(result).toBeNull();
     expect(getTaskRun).not.toHaveBeenCalled();
   });
+
+  it("warns once (not twice) about a missing artifact across the speculative and post-refetch resolves", async () => {
+    const internals = buildInternals();
+    // A non-empty manifest that never lists the requested id — so getArtifactsById
+    // reaches its per-id "missing" warning on both the pre- and post-refetch calls
+    // (an empty manifest would short-circuit before warning at all).
+    const decoyManifest = [
+      {
+        id: "unrelated-artifact",
+        name: "other.txt",
+        type: "user_attachment" as const,
+      },
+    ];
+    internals.posthogAPI.getTaskRun = vi.fn(async () =>
+      createTaskRun({
+        state: { pending_user_artifact_ids: ["missing-attachment"] },
+        artifacts: decoyManifest,
+      }),
+    );
+    const loggerHost = internals as unknown as {
+      logger: { warn: (...args: unknown[]) => void };
+    };
+    const warnSpy = vi
+      .spyOn(loggerHost.logger, "warn")
+      .mockImplementation(() => {});
+
+    await internals.getPendingUserPrompt(
+      createTaskRun({
+        state: { pending_user_artifact_ids: ["missing-attachment"] },
+        artifacts: decoyManifest,
+      }),
+    );
+
+    // The speculative pre-refetch resolve stays quiet (a miss there is expected);
+    // only the post-refetch resolve emits the per-id "missing" warning.
+    const manifestWarnings = warnSpy.mock.calls.filter(
+      ([message]) => message === "Pending artifact missing from run manifest",
+    );
+    expect(manifestWarnings).toHaveLength(1);
+  });
 });

packages/agent/src/server/agent-server.ts

@@ -1772,7 +1772,9 @@ export class AgentServer {
     // a transient gap doesn't drop the attachment and send the agent the bare
     // "Attached files: …" description instead of the file it was promised.
     let manifest = taskRun.artifacts ?? [];
-    let resolvedArtifacts = this.getArtifactsById(manifest, artifactIds);
+    let resolvedArtifacts = this.getArtifactsById(manifest, artifactIds, {
+      warnOnMissing: false,
+    });
     if (
       artifactIds.length > 0 &&
       resolvedArtifacts.length < artifactIds.length
@@ -1924,6 +1926,10 @@ export class AgentServer {
   private getArtifactsById(
     artifacts: TaskRunArtifact[] | undefined,
     artifactIds: string[],
+    // The speculative pre-refetch resolve passes false: a miss there is expected
+    // (it's what triggers the refetch), so warning would be premature and would
+    // double up with the post-refetch warning for a genuinely missing artifact.
+    { warnOnMissing = true }: { warnOnMissing?: boolean } = {},
   ): TaskRunArtifact[] {
     if (!artifacts?.length || artifactIds.length === 0) {
       return [];
@@ -1941,9 +1947,11 @@ export class AgentServer {
     return artifactIds.flatMap((artifactId) => {
       const artifact = artifactsById.get(artifactId);
       if (!artifact) {
-        this.logger.warn("Pending artifact missing from run manifest", {
-          artifactId,
-        });
+        if (warnOnMissing) {
+          this.logger.warn("Pending artifact missing from run manifest", {
+            artifactId,
+          });
+        }
         return [];
       }