Skip to content

Commit be1a3e8

Browse files
authored
fix(agent): recover or flag pending attachments that fail to reach the cloud agent (#3039)
## Problem Closes GROW-99 Pasting a prompt into PostHog Code auto-converts it to a `pasted-text.txt` attachment. On **cloud** tasks the file is uploaded as a run artifact and the sandbox agent reads it from `.posthog/attachments/…`. Intermittently the agent instead received the bare task description `Attached files: pasted-text.txt` as plain text — no file, no inline content — and replied that it *"can't find the file… no text was included inline"*, unable to do the task. (The task title also stayed **Untitled**.) ## Root cause In `agent-server.ts → sendInitialTaskMessage`, when `getPendingUserPrompt()` returns `null` (no pending message **and** no *resolvable* artifacts) it falls back to `task.description`. For a pasted-text-only task that description is just the `Attached files: …` summary, so the agent is handed a prompt naming a file it was never given. The artifact fails to resolve when the run's `artifacts` manifest doesn't yet contain the just-uploaded `pending_user_artifact_ids` — most likely a transient manifest lag at run start. ## Fix Hardened `getPendingUserPrompt`: 1. **Best-effort recovery** — refetch the run manifest once when a declared attachment isn't listed yet, so a transient lag doesn't drop the attachment (the pasted text then reaches the agent). 2. **Safe degraded prompt** — if an attachment still can't be hydrated, append an explicit *"the attachment didn't come through, ask the user to paste it directly or re-send"* notice instead of the misleading bare description. Skill bundles (installed silently) are excluded from the count. 3. Adds a `warn("Pending user attachments could not be loaded", …)` log with resolved/expected counts, so a *consistent* failure (artifact genuinely missing server-side) is obvious in run logs. The misleading `task.description` fallback in `sendInitialTaskMessage` is now bypassed for this case, and the resume paths benefit too since they share `getPendingUserPrompt`. ## Tests New `AgentServer pending user attachments` suite in `agent-server.test.ts`: - missing attachment (refetch also empty) → explicit notice appended, not `null` - refetch recovers the artifact → `resource_link` block, no notice - no pending artifacts → `null`, and no refetch Typecheck and Biome are clean. > The file's existing **HTTP-mode** tests run a `git commit` in `createTestRepo`, which is blocked in the signed-commits sandbox this was authored in — so I couldn't run them locally. The new tests are deliberately self-contained (no git/HTTP) and pass; CI covers the rest. ## Notes - This was seen as a **one-off**, so the change hardens the degraded path and adds recovery rather than chasing a root cause I couldn't confirm. If it reproduces every time, the new warn log points at the artifact genuinely not landing in `taskRun.artifacts` server-side. - The earlier title fix (#2850) is unrelated — it only touched name generation, not how the file reaches the agent. Closes GROW-99
1 parent 49fdb91 commit be1a3e8

2 files changed

Lines changed: 279 additions & 5 deletions

File tree

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

Lines changed: 178 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createHash } from "node:crypto";
2-
import { readFile } from "node:fs/promises";
2+
import { mkdtemp, readFile, rm } from "node:fs/promises";
3+
import { tmpdir } from "node:os";
34
import { join } from "node:path";
45
import type { ContentBlock } from "@agentclientprotocol/sdk";
56
import { zipSync } from "fflate";
@@ -2376,3 +2377,179 @@ describe("AgentServer HTTP Mode", () => {
23762377
});
23772378
});
23782379
});
2380+
2381+
// Exercises getPendingUserPrompt directly (no HTTP server / git repo) so we can
2382+
// assert how the initial cloud prompt degrades when an attached file can't be
2383+
// hydrated — the case behind a pasted-text task reaching the agent as a bare
2384+
// "Attached files: …" description with no readable file.
2385+
describe("AgentServer pending user attachments", () => {
2386+
interface PendingPromptInternals {
2387+
posthogAPI: {
2388+
getTaskRun: (taskId: string, runId: string) => Promise<TaskRun>;
2389+
downloadArtifact: (
2390+
taskId: string,
2391+
runId: string,
2392+
storagePath: string,
2393+
) => Promise<ArrayBuffer | null>;
2394+
};
2395+
getPendingUserPrompt(
2396+
taskRun: TaskRun | null,
2397+
): Promise<{ prompt: ContentBlock[] } | null>;
2398+
}
2399+
2400+
let tempDir: string;
2401+
let server: AgentServer | undefined;
2402+
2403+
beforeEach(async () => {
2404+
tempDir = await mkdtemp(join(tmpdir(), "agent-pending-"));
2405+
});
2406+
2407+
afterEach(async () => {
2408+
await server?.stop();
2409+
server = undefined;
2410+
await rm(tempDir, { recursive: true, force: true });
2411+
});
2412+
2413+
const buildInternals = (): PendingPromptInternals => {
2414+
server = new AgentServer({
2415+
port: getNextTestPort(),
2416+
jwtPublicKey: TEST_PUBLIC_KEY,
2417+
repositoryPath: tempDir,
2418+
apiUrl: "http://localhost:8000",
2419+
apiKey: "test-api-key",
2420+
projectId: 1,
2421+
mode: "interactive",
2422+
taskId: "test-task-id",
2423+
runId: "test-run-id",
2424+
});
2425+
return server as unknown as PendingPromptInternals;
2426+
};
2427+
2428+
it("appends an explicit notice when a pending attachment never reaches the manifest", async () => {
2429+
const internals = buildInternals();
2430+
// Refetch still can't see the attachment (truly absent, not just lagging).
2431+
const getTaskRun = vi.fn(async () =>
2432+
createTaskRun({
2433+
state: { pending_user_artifact_ids: ["missing-attachment"] },
2434+
artifacts: [],
2435+
}),
2436+
);
2437+
internals.posthogAPI.getTaskRun = getTaskRun;
2438+
2439+
const result = await internals.getPendingUserPrompt(
2440+
createTaskRun({
2441+
state: { pending_user_artifact_ids: ["missing-attachment"] },
2442+
artifacts: [],
2443+
}),
2444+
);
2445+
2446+
// Refetched once to recover a lagging manifest, then — still missing —
2447+
// surfaced an explicit notice instead of returning null (which would let the
2448+
// caller fall back to the misleading "Attached files: …" description).
2449+
expect(getTaskRun).toHaveBeenCalledTimes(1);
2450+
expect(result).not.toBeNull();
2451+
expect(result?.prompt).toHaveLength(1);
2452+
const [block] = result?.prompt ?? [];
2453+
expect(block?.type).toBe("text");
2454+
expect((block as { text: string }).text).toContain("could not be loaded");
2455+
});
2456+
2457+
it("recovers a pending attachment from a refetched run manifest", async () => {
2458+
const internals = buildInternals();
2459+
internals.posthogAPI.getTaskRun = vi.fn(async () =>
2460+
createTaskRun({
2461+
state: { pending_user_artifact_ids: ["att-1"] },
2462+
artifacts: [
2463+
{
2464+
id: "att-1",
2465+
name: "pasted-text.txt",
2466+
type: "user_attachment",
2467+
storage_path: "tasks/artifacts/pasted-text.txt",
2468+
content_type: "text/plain",
2469+
},
2470+
],
2471+
}),
2472+
);
2473+
const downloadArtifact = vi.fn(async () =>
2474+
exactArrayBuffer(new TextEncoder().encode("pasted body")),
2475+
);
2476+
internals.posthogAPI.downloadArtifact = downloadArtifact;
2477+
2478+
const result = await internals.getPendingUserPrompt(
2479+
createTaskRun({
2480+
state: { pending_user_artifact_ids: ["att-1"] },
2481+
artifacts: [],
2482+
}),
2483+
);
2484+
2485+
expect(downloadArtifact).toHaveBeenCalledWith(
2486+
"task-1",
2487+
"run-1",
2488+
"tasks/artifacts/pasted-text.txt",
2489+
);
2490+
const resourceLinks = result?.prompt.filter(
2491+
(block) => block.type === "resource_link",
2492+
);
2493+
expect(resourceLinks).toHaveLength(1);
2494+
// No "couldn't load" notice once the attachment is recovered.
2495+
const hasNotice = result?.prompt.some(
2496+
(block) =>
2497+
block.type === "text" &&
2498+
(block as { text: string }).text.includes("could not be loaded"),
2499+
);
2500+
expect(hasNotice).toBe(false);
2501+
});
2502+
2503+
it("returns null without refetching when no pending artifacts were declared", async () => {
2504+
const internals = buildInternals();
2505+
const getTaskRun = vi.fn();
2506+
internals.posthogAPI.getTaskRun = getTaskRun;
2507+
2508+
const result = await internals.getPendingUserPrompt(
2509+
createTaskRun({ state: {}, artifacts: [] }),
2510+
);
2511+
2512+
expect(result).toBeNull();
2513+
expect(getTaskRun).not.toHaveBeenCalled();
2514+
});
2515+
2516+
it("warns once (not twice) about a missing artifact across the speculative and post-refetch resolves", async () => {
2517+
const internals = buildInternals();
2518+
// A non-empty manifest that never lists the requested id — so getArtifactsById
2519+
// reaches its per-id "missing" warning on both the pre- and post-refetch calls
2520+
// (an empty manifest would short-circuit before warning at all).
2521+
const decoyManifest = [
2522+
{
2523+
id: "unrelated-artifact",
2524+
name: "other.txt",
2525+
type: "user_attachment" as const,
2526+
},
2527+
];
2528+
internals.posthogAPI.getTaskRun = vi.fn(async () =>
2529+
createTaskRun({
2530+
state: { pending_user_artifact_ids: ["missing-attachment"] },
2531+
artifacts: decoyManifest,
2532+
}),
2533+
);
2534+
const loggerHost = internals as unknown as {
2535+
logger: { warn: (...args: unknown[]) => void };
2536+
};
2537+
const warnSpy = vi
2538+
.spyOn(loggerHost.logger, "warn")
2539+
.mockImplementation(() => {});
2540+
2541+
await internals.getPendingUserPrompt(
2542+
createTaskRun({
2543+
state: { pending_user_artifact_ids: ["missing-attachment"] },
2544+
artifacts: decoyManifest,
2545+
}),
2546+
);
2547+
2548+
// The speculative pre-refetch resolve stays quiet (a miss there is expected);
2549+
// only the post-refetch resolve emits the per-id "missing" warning.
2550+
const manifestWarnings = warnSpy.mock.calls.filter(
2551+
([message]) => message === "Pending artifact missing from run manifest",
2552+
);
2553+
expect(manifestWarnings).toHaveLength(1);
2554+
});
2555+
});

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

Lines changed: 101 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,22 @@ function getTaskRunStateString(
277277
return typeof value === "string" ? value : null;
278278
}
279279

280+
// Prompt block we hand the agent when the user attached files but we could not
281+
// load any of them into the session (missing from the run manifest, no storage
282+
// path, etc.). Without this the caller falls back to the bare task description —
283+
// e.g. "Attached files: pasted-text.txt" — which points the agent at files it
284+
// was never given and makes it hunt the filesystem in vain. Be explicit instead.
285+
function buildMissingAttachmentNotice(count: number): string {
286+
const subject = count === 1 ? "A file" : `${count} files`;
287+
const pronoun = count === 1 ? "it" : "they";
288+
const noun = count === 1 ? "attachment" : "attachments";
289+
return (
290+
`${subject} the user attached to this message could not be loaded into the session, ` +
291+
`so ${pronoun} are unavailable here. Do not guess at the contents. Tell the user the ` +
292+
`${noun} didn't come through, and ask them to paste the text directly or send ${pronoun} again.`
293+
);
294+
}
295+
280296
export class AgentServer {
281297
private config: AgentServerConfig;
282298
private sessionReadyBootMs?: number;
@@ -1800,20 +1816,95 @@ export class AgentServer {
18001816
typeof artifactId === "string" && artifactId.trim().length > 0,
18011817
)
18021818
: [];
1819+
1820+
// The run's artifact manifest can momentarily lag the pending-artifact ids
1821+
// when a run starts right after the attachments were uploaded. If we were
1822+
// asked for artifacts the manifest doesn't list yet, refetch the run once so
1823+
// a transient gap doesn't drop the attachment and send the agent the bare
1824+
// "Attached files: …" description instead of the file it was promised.
1825+
let manifest = taskRun.artifacts ?? [];
1826+
let resolvedArtifacts = this.getArtifactsById(manifest, artifactIds, {
1827+
warnOnMissing: false,
1828+
});
1829+
if (
1830+
artifactIds.length > 0 &&
1831+
resolvedArtifacts.length < artifactIds.length
1832+
) {
1833+
const refreshed = await this.refetchRunArtifacts(taskRun);
1834+
if (refreshed) {
1835+
manifest = refreshed;
1836+
resolvedArtifacts = this.getArtifactsById(manifest, artifactIds);
1837+
}
1838+
}
1839+
18031840
const prompt = await this.buildPromptFromContentAndArtifacts({
18041841
content: typeof message === "string" ? message : undefined,
1805-
artifacts: this.getArtifactsById(taskRun.artifacts, artifactIds),
1842+
artifacts: resolvedArtifacts,
18061843
taskId: taskRun.task,
18071844
runId: taskRun.id,
18081845
});
1846+
1847+
// Skill bundles are installed silently, so only non-skill attachments are
1848+
// expected to surface as content (hydrated into resource_link blocks). Ids
1849+
// the manifest still can't account for are treated as attachments, not
1850+
// skills — better to over-warn than to silently mislead. `message` here is
1851+
// plain text, so every resource_link block is a hydrated attachment.
1852+
const expectedAttachmentCount = artifactIds.filter((artifactId) => {
1853+
const known = manifest.find((artifact) => artifact.id === artifactId);
1854+
return known ? known.type !== "skill_bundle" : true;
1855+
}).length;
1856+
const hydratedAttachmentCount = prompt.prompt.filter(
1857+
(block) => block.type === "resource_link",
1858+
).length;
1859+
const lostAttachmentCount =
1860+
expectedAttachmentCount - hydratedAttachmentCount;
1861+
1862+
if (lostAttachmentCount > 0) {
1863+
this.logger.warn("Pending user attachments could not be loaded", {
1864+
taskId: taskRun.task,
1865+
runId: taskRun.id,
1866+
requestedArtifactCount: artifactIds.length,
1867+
expectedAttachmentCount,
1868+
hydratedAttachmentCount,
1869+
lostAttachmentCount,
1870+
});
1871+
prompt.prompt.push({
1872+
type: "text",
1873+
text: buildMissingAttachmentNotice(lostAttachmentCount),
1874+
});
1875+
}
1876+
18091877
this.logger.debug("Built pending user prompt", {
18101878
hasMessage: typeof message === "string" && message.trim().length > 0,
18111879
requestedArtifactCount: artifactIds.length,
1880+
hydratedAttachmentCount,
1881+
lostAttachmentCount,
18121882
blockTypes: prompt.prompt.map((block) => block.type),
18131883
});
18141884
return prompt.prompt.length > 0 ? prompt : null;
18151885
}
18161886

1887+
// Best-effort refetch of a run's artifact manifest. Returns null on any error
1888+
// so the caller can fall back to the manifest it already has.
1889+
private async refetchRunArtifacts(
1890+
taskRun: TaskRun,
1891+
): Promise<TaskRunArtifact[] | null> {
1892+
try {
1893+
const refreshed = await this.posthogAPI.getTaskRun(
1894+
taskRun.task,
1895+
taskRun.id,
1896+
);
1897+
return refreshed.artifacts ?? null;
1898+
} catch (error) {
1899+
this.logger.debug("Failed to refetch run artifacts for pending prompt", {
1900+
taskId: taskRun.task,
1901+
runId: taskRun.id,
1902+
error,
1903+
});
1904+
return null;
1905+
}
1906+
}
1907+
18171908
private getClearedPendingUserState(taskRun: TaskRun | null): string[] | null {
18181909
const state =
18191910
taskRun?.state && typeof taskRun.state === "object"
@@ -1886,6 +1977,10 @@ export class AgentServer {
18861977
private getArtifactsById(
18871978
artifacts: TaskRunArtifact[] | undefined,
18881979
artifactIds: string[],
1980+
// The speculative pre-refetch resolve passes false: a miss there is expected
1981+
// (it's what triggers the refetch), so warning would be premature and would
1982+
// double up with the post-refetch warning for a genuinely missing artifact.
1983+
{ warnOnMissing = true }: { warnOnMissing?: boolean } = {},
18891984
): TaskRunArtifact[] {
18901985
if (!artifacts?.length || artifactIds.length === 0) {
18911986
return [];
@@ -1903,9 +1998,11 @@ export class AgentServer {
19031998
return artifactIds.flatMap((artifactId) => {
19041999
const artifact = artifactsById.get(artifactId);
19052000
if (!artifact) {
1906-
this.logger.warn("Pending artifact missing from run manifest", {
1907-
artifactId,
1908-
});
2001+
if (warnOnMissing) {
2002+
this.logger.warn("Pending artifact missing from run manifest", {
2003+
artifactId,
2004+
});
2005+
}
19092006
return [];
19102007
}
19112008

0 commit comments

Comments
 (0)