Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.

Commit 2215cbb

Browse files
authored
fix(channels): show uploaded files in the task Artifacts pane
The pane read the run artifact manifest and then dropped everything on `type === "plan"` — a type nothing in the product has ever written. The only deliverable type agents actually produce is `output`, via the `upload_artifact` tool, so the manifest branch was unreachable and files handed back by a run never appeared. Verified against a live run's manifest: 6 `output`, 2 `user_attachment`, zero `plan`. The `plan` row was speculative — down to a bare-UUID fallback for names that never arrive — so it goes rather than shipping a UI affordance for data that doesn't exist. It can come back with a producer. Same inversion is why the conversation's Files box showed these while the pane said "No artifacts yet": it filters to `output`, the exact complement. - `parseRunPlans` becomes `parseRunArtifacts(raw, types)`, and reads the `size`/`content_type`/`uploaded_at` the rows need. - Re-uploads collapse to the newest per name: agents revise a deliverable and upload it again, and keeping every copy buries the current one under its own drafts. - `formatFileSize` moves out of CloudArtifactDownloads so both surfaces format sizes the same way. Generated-By: PostHog Code Task-Id: 8d187d00-0633-4706-8443-f79130b65f9f
1 parent d577f0e commit 2215cbb

6 files changed

Lines changed: 220 additions & 61 deletions

File tree

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,68 @@
11
import { describe, expect, it } from "vitest";
2-
import { parseRunPlans } from "./runArtifactSchemas";
2+
import { OUTPUT_ARTIFACT_TYPES, parseRunArtifacts } from "./runArtifactSchemas";
33

4-
describe("parseRunPlans", () => {
4+
describe("parseRunArtifacts", () => {
55
it.each([
66
{ name: "undefined", raw: undefined },
77
{ name: "null", raw: null },
88
{ name: "an object", raw: { artifacts: [] } },
9-
{ name: "a string", raw: "plan" },
9+
{ name: "a string", raw: "output" },
1010
])("reads $name as no artifacts", ({ raw }) => {
11-
expect(parseRunPlans(raw)).toEqual([]);
11+
expect(parseRunArtifacts(raw, OUTPUT_ARTIFACT_TYPES)).toEqual([]);
1212
});
1313

14-
it("keeps only plan artifacts", () => {
15-
const plans = parseRunPlans([
16-
{ id: "a", type: "plan", name: "Plan A" },
17-
{ id: "b", type: "upload", name: "internal blob" },
14+
// A run's manifest mixes deliverables with plumbing nobody asked to see.
15+
it("keeps only the requested types", () => {
16+
const outputs = parseRunArtifacts(
17+
[
18+
{ id: "a", type: "output", name: "report.md" },
19+
{ id: "b", type: "user_attachment", name: "clipboard.png" },
20+
{ id: "c", type: "skill_bundle", name: "skills.zip" },
21+
],
22+
OUTPUT_ARTIFACT_TYPES,
23+
);
24+
expect(outputs).toEqual([{ id: "a", type: "output", name: "report.md" }]);
25+
});
26+
27+
it("reads the size and upload time the row renders", () => {
28+
const artifact = {
29+
id: "a",
30+
type: "output",
31+
name: "report.md",
32+
size: 16861,
33+
content_type: "text/markdown",
34+
uploaded_at: "2026-07-27T08:27:26.896719+00:00",
35+
};
36+
expect(parseRunArtifacts([artifact], OUTPUT_ARTIFACT_TYPES)).toEqual([
37+
artifact,
1838
]);
19-
expect(plans).toEqual([{ id: "a", type: "plan", name: "Plan A" }]);
39+
});
40+
41+
it("keeps an artifact that omits the optional fields", () => {
42+
expect(
43+
parseRunArtifacts([{ type: "output" }], OUTPUT_ARTIFACT_TYPES),
44+
).toEqual([{ type: "output" }]);
2045
});
2146

2247
// One bad entry shouldn't take the Artifacts tab down with it.
2348
it("drops entries that don't match the shape", () => {
24-
const plans = parseRunPlans([
25-
{ id: 42, type: "plan" },
26-
null,
27-
"not an object",
28-
{ type: "plan", storage_path: "runs/1/plan.md" },
49+
const outputs = parseRunArtifacts(
50+
[
51+
{ id: 42, type: "output" },
52+
null,
53+
"not an object",
54+
{ type: "output", storage_path: "runs/1/report.md" },
55+
],
56+
OUTPUT_ARTIFACT_TYPES,
57+
);
58+
expect(outputs).toEqual([
59+
{ type: "output", storage_path: "runs/1/report.md" },
2960
]);
30-
expect(plans).toEqual([{ type: "plan", storage_path: "runs/1/plan.md" }]);
3161
});
3262

3363
it("ignores an artifact with no type", () => {
34-
expect(parseRunPlans([{ id: "a", name: "mystery" }])).toEqual([]);
64+
expect(
65+
parseRunArtifacts([{ id: "a", name: "mystery" }], OUTPUT_ARTIFACT_TYPES),
66+
).toEqual([]);
3567
});
3668
});

packages/core/src/canvas/runArtifactSchemas.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,30 @@ export const runArtifactSchema = z.object({
44
id: z.string().optional(),
55
name: z.string().optional(),
66
type: z.string().optional(),
7+
size: z.number().optional(),
8+
content_type: z.string().optional(),
79
storage_path: z.string().optional(),
10+
uploaded_at: z.string().optional(),
811
});
912
export type RunArtifact = z.infer<typeof runArtifactSchema>;
1013

11-
export function parseRunPlans(raw: unknown): RunArtifact[] {
14+
/** Artifacts the agent hands back as deliverables, via the `upload_artifact` tool. */
15+
export const OUTPUT_ARTIFACT_TYPES = ["output"] as const;
16+
17+
/**
18+
* The run's artifacts of the given types, in manifest order. A run's manifest
19+
* also carries plumbing the user never asked for — skill bundles, their own
20+
* attachments — so callers name the types they want.
21+
*/
22+
export function parseRunArtifacts(
23+
raw: unknown,
24+
types: readonly string[],
25+
): RunArtifact[] {
1226
if (!Array.isArray(raw)) return [];
1327
return raw.flatMap((entry) => {
1428
const parsed = runArtifactSchema.safeParse(entry);
15-
return parsed.success && parsed.data.type === "plan" ? [parsed.data] : [];
29+
if (!parsed.success) return [];
30+
const { type } = parsed.data;
31+
return type && types.includes(type) ? [parsed.data] : [];
1632
});
1733
}

packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx

Lines changed: 102 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,20 @@
1-
import type { Task, TaskRun } from "@posthog/shared/domain-types";
1+
import type { Task, TaskRun, TaskRunArtifact } from "@posthog/shared";
22
import { fireEvent, render, screen } from "@testing-library/react";
33
import { beforeEach, describe, expect, it, vi } from "vitest";
44

55
const mocks = vi.hoisted(() => ({
66
runs: [] as TaskRun[],
7+
presignTaskRunArtifact: vi.fn(),
78
}));
89

910
vi.mock("@posthog/ui/features/canvas/hooks/useTaskRuns", () => ({
1011
useTaskRuns: () => ({ runs: mocks.runs, isLoading: false }),
1112
}));
13+
vi.mock("@posthog/ui/features/auth/authClient", () => ({
14+
useOptionalAuthenticatedClient: () => ({
15+
presignTaskRunArtifact: mocks.presignTaskRunArtifact,
16+
}),
17+
}));
1218
vi.mock("@posthog/ui/features/git-interaction/usePrArtifact", () => ({
1319
usePrArtifact: (url: string) => ({
1420
safeUrl: url,
@@ -33,16 +39,35 @@ const task = {
3339
latest_run: null,
3440
} as unknown as Task;
3541

36-
function run(id: string, prNumber: number): TaskRun {
42+
function run(
43+
id: string,
44+
options: { prNumber?: number; artifacts?: Partial<TaskRunArtifact>[] } = {},
45+
): TaskRun {
3746
return {
3847
id,
39-
output: { pr_url: `https://github.com/acme/repo/pull/${prNumber}` },
48+
output: options.prNumber
49+
? { pr_url: `https://github.com/acme/repo/pull/${options.prNumber}` }
50+
: null,
51+
artifacts: options.artifacts,
4052
} as unknown as TaskRun;
4153
}
4254

55+
function outputFile(
56+
overrides: Partial<TaskRunArtifact>,
57+
): Partial<TaskRunArtifact> {
58+
return {
59+
type: "output",
60+
name: "report.md",
61+
storage_path: "runs/1/report.md",
62+
...overrides,
63+
};
64+
}
65+
4366
describe("TaskArtifactsList", () => {
4467
beforeEach(() => {
45-
mocks.runs = [run("run-1", 1), run("run-2", 2)];
68+
mocks.runs = [run("run-1", { prNumber: 1 }), run("run-2", { prNumber: 2 })];
69+
mocks.presignTaskRunArtifact.mockReset();
70+
mocks.presignTaskRunArtifact.mockResolvedValue("https://signed.example/x");
4671
useReviewNavigationStore.setState({
4772
reviewModes: {},
4873
selectedPrUrls: {},
@@ -60,4 +85,77 @@ describe("TaskArtifactsList", () => {
6085
);
6186
expect(state.reviewModes[task.id]).toBe("split");
6287
});
88+
89+
it("lists the files the agent uploaded, with their size", () => {
90+
mocks.runs = [
91+
run("run-1", { artifacts: [outputFile({ id: "a", size: 16861 })] }),
92+
];
93+
94+
render(<TaskArtifactsList task={task} timeline={[]} />);
95+
96+
expect(screen.getByText("report.md")).toBeTruthy();
97+
expect(screen.getByText("File · 17 KB")).toBeTruthy();
98+
});
99+
100+
it("presigns the artifact of the run that produced it", () => {
101+
mocks.runs = [
102+
run("run-1", { artifacts: [outputFile({ id: "a", name: "old.md" })] }),
103+
run("run-2", {
104+
artifacts: [
105+
outputFile({
106+
id: "b",
107+
name: "new.md",
108+
storage_path: "runs/2/new.md",
109+
}),
110+
],
111+
}),
112+
];
113+
114+
render(<TaskArtifactsList task={task} timeline={[]} />);
115+
fireEvent.click(screen.getByText("new.md"));
116+
117+
expect(mocks.presignTaskRunArtifact).toHaveBeenCalledWith(
118+
"task-1",
119+
"run-2",
120+
"runs/2/new.md",
121+
);
122+
});
123+
124+
// Agents revise a deliverable and upload it again under the same name.
125+
it("keeps only the newest upload of a repeatedly revised file", () => {
126+
mocks.runs = [
127+
run("run-1", {
128+
artifacts: [
129+
outputFile({
130+
id: "a",
131+
size: 1000,
132+
uploaded_at: "2026-07-27T08:00:00+00:00",
133+
}),
134+
outputFile({
135+
id: "b",
136+
size: 2000,
137+
storage_path: "runs/1/report-v2.md",
138+
uploaded_at: "2026-07-27T09:00:00+00:00",
139+
}),
140+
],
141+
}),
142+
];
143+
144+
render(<TaskArtifactsList task={task} timeline={[]} />);
145+
146+
expect(screen.getAllByText("report.md")).toHaveLength(1);
147+
expect(screen.getByText("File · 2 KB")).toBeTruthy();
148+
});
149+
150+
it.each([
151+
{ name: "a plan", type: "plan" as const },
152+
{ name: "a user attachment", type: "user_attachment" as const },
153+
{ name: "a skill bundle", type: "skill_bundle" as const },
154+
])("shows the empty state for a run with only $name", ({ type }) => {
155+
mocks.runs = [run("run-1", { artifacts: [outputFile({ id: "a", type })] })];
156+
157+
render(<TaskArtifactsList task={task} timeline={[]} />);
158+
159+
expect(screen.getByText("No artifacts yet")).toBeTruthy();
160+
});
63161
});

0 commit comments

Comments
 (0)