Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions products/desktop/docs/cloud-task-artifacts.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,9 @@ On success the tool returns a presigned download URL for the uploaded file (mint
Repository changes should continue to be delivered through git rather than duplicated as task artifacts. A single uploaded artifact is limited to 30 MB.

The desktop app runs scripts embedded in HTML artifacts inside an isolated preview process. The preview cannot access Node.js, Electron, PostHog credentials, remote resources, downloads, or device permissions. Use **Stop preview** if a script becomes unresponsive, then use **Restart preview** to load it in a fresh process.

## Versions and dismissal

Uploading a file under a name the run already has does not add a second file. Every upload stays on the manifest as its own entry, and clients group entries by name into one file with a version history: the newest upload is what the app shows, and the earlier ones sit behind a version picker on the row. That is how an agent revises a deliverable — upload it again under the same name.

A user can dismiss a file they don't want to see. `POST .../runs/<run_id>/artifacts/dismiss/` takes `artifact_ids` and a `dismissed` boolean, and stamps `dismissed_at` on each named manifest entry. Nothing is deleted from object storage, and clients only hide a file once every version of it is dismissed, so dismissing the current version cannot resurface the one it replaced. Passing `dismissed: false` restores the file.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ export const uploadArtifactTool = defineLocalTool({
"Deliver a file you created to the user as a downloadable task artifact. " +
"Call this for every non-code deliverable (reports, images, archives, data files, and similar output) " +
"before your final response. The file must be inside the session workspace. Repository changes belong in git and should not be uploaded. " +
"To revise a file you already delivered, upload it again under the same name: the app shows the newest version " +
"and keeps the earlier ones available. " +
"On success the result includes a download URL for the uploaded file, which you can reference in your final response.",
schema: {
path: z
Expand Down
1 change: 1 addition & 0 deletions products/desktop/packages/api-client/src/generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11061,6 +11061,7 @@ export namespace Schemas {
content_type?: string | undefined;
storage_path: string;
uploaded_at: string;
dismissed_at?: string | undefined;
};
export type TaskRunDetail = {
id: string;
Expand Down
31 changes: 31 additions & 0 deletions products/desktop/packages/api-client/src/posthog-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ import type {
TaskMention,
TaskRun,
TaskRunArtefact,
TaskRunArtifact,
TaskThreadMessage,
UserBasic,
} from "@posthog/shared/domain-types";
Expand Down Expand Up @@ -141,7 +142,9 @@ import type {
import type { SpendAnalysisResponse } from "./spend-analysis";
import {
normalizeTaskResponse,
normalizeTaskRunArtifact,
normalizeTaskRunResponse,
type TaskRunArtifactDTO,
} from "./task-normalization";

export type * from "./mcp-gateway";
Expand Down Expand Up @@ -3296,6 +3299,34 @@ export class PostHogAPIClient {
});
}

/** Hide or restore every version of a file on the run, returning the updated manifest. */
async setTaskRunArtifactsDismissed(
taskId: string,
runId: string,
artifactIds: string[],
dismissed: boolean,
): Promise<TaskRunArtifact[]> {
const teamId = await this.getTeamId();
const path = `/api/projects/${teamId}/tasks/${taskId}/runs/${runId}/artifacts/dismiss/`;
const response = await this.api.fetcher.fetch({
method: "post",
url: new URL(`${this.api.baseUrl}${path}`),
path,
overrides: {
body: JSON.stringify({ artifact_ids: artifactIds, dismissed }),
},
});

if (!response.ok) {
throw new Error(`Failed to update artifact: ${response.statusText}`);
}

const data = (await response.json()) as {
artifacts?: TaskRunArtifactDTO[];
};
return (data.artifacts ?? []).map(normalizeTaskRunArtifact);
}

async getTaskSessionStorageAccess(
taskId: string,
runId: string,
Expand Down
15 changes: 10 additions & 5 deletions products/desktop/packages/api-client/src/task-normalization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import type {
} from "@posthog/shared/domain-types";
import type { Schemas } from "./generated";

export type TaskRunArtifactDTO = Schemas.TaskRunArtifactResponse & {
metadata?: unknown;
};
Comment on lines +11 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Avoid duplicating generated response fields

TaskRunArtifactDTO manually adds metadata and dismissed_at to the generated backend response type, allowing the handwritten contract to drift from the serializer-generated schema and requiring duplicate maintenance. Please update and consume the generated response type instead.

Context Used: docs/published/handbook/engineering/type-system.md (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: products/desktop/packages/api-client/src/task-normalization.ts
Line: 11-14

Comment:
**Avoid duplicating generated response fields**

`TaskRunArtifactDTO` manually adds `metadata` and `dismissed_at` to the generated backend response type, allowing the handwritten contract to drift from the serializer-generated schema and requiring duplicate maintenance. Please update and consume the generated response type instead.

**Context Used:** docs/published/handbook/engineering/type-system.md ([source](https://github.com/posthog/posthog/blob/master/docs/published/handbook/engineering/type-system.md))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took the intent in 2229b03, though the mechanism does not apply here.

packages/api-client/src/generated.ts is not generated despite its name. The desktop package keeps a hand-maintained mirror of the schemas it uses, which .agents/skills/posthog-desktop/SKILL.md lists explicitly as the desktop counterpart to hogli build:openapi. It is also deliberately partial: TaskRunArtifactResponse there omits url and metadata, which the serializer does define. So there is no regeneration step to run, and consuming a generated type is not available for this package.

The drift you point at was real all the same. dismissed_at now sits on TaskRunArtifactResponse in that mirror, matching the merged serializer (required=False, no allow_null, so optional and never null), and TaskRunArtifactDTO no longer redeclares it. The remaining metadata?: unknown predates this PR and stays, since its shape is polymorphic across artifact types.


type TaskRunResponseDTO = Partial<
Omit<Schemas.TaskRunDetail, "artifacts" | "status">
> & {
id: string;
artifacts?: Array<
Schemas.TaskRunArtifactResponse & { metadata?: unknown }
> | null;
artifacts?: Array<TaskRunArtifactDTO> | null;
status?: Schemas.StatusA35Enum | "started" | null;
team?: number | null;
};
Expand Down Expand Up @@ -101,8 +103,8 @@ function normalizeArtifactMetadata(
};
}

function normalizeTaskRunArtifact(
artifact: NonNullable<TaskRunResponseDTO["artifacts"]>[number],
export function normalizeTaskRunArtifact(
artifact: TaskRunArtifactDTO,
): TaskRunArtifact {
const metadata = normalizeArtifactMetadata(artifact.metadata);

Expand All @@ -126,6 +128,9 @@ function normalizeTaskRunArtifact(
...(artifact.uploaded_at === undefined
? {}
: { uploaded_at: artifact.uploaded_at }),
...(artifact.dismissed_at === undefined
? {}
: { dismissed_at: artifact.dismissed_at }),
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { describe, expect, it } from "vitest";
import { OUTPUT_ARTIFACT_TYPES, parseRunArtifacts } from "./runArtifactSchemas";
import {
groupRunArtifactVersions,
OUTPUT_ARTIFACT_TYPES,
parseRunArtifacts,
} from "./runArtifactSchemas";

describe("parseRunArtifacts", () => {
it.each([
Expand Down Expand Up @@ -66,3 +70,54 @@ describe("parseRunArtifacts", () => {
).toEqual([]);
});
});

describe("groupRunArtifactVersions", () => {
it("collapses re-uploads of a name into one newest-first group", () => {
const groups = groupRunArtifactVersions([
{ id: "a", name: "report.md", uploaded_at: "2026-07-27T08:00:00Z" },
{ id: "b", name: "chart.png", uploaded_at: "2026-07-27T08:30:00Z" },
{ id: "c", name: "report.md", uploaded_at: "2026-07-27T09:00:00Z" },
]);

expect(groups.map((group) => group.name)).toEqual([
"report.md",
"chart.png",
]);
expect(groups[0]?.versions.map((version) => version.id)).toEqual([
"c",
"a",
]);
expect(groups[0]?.latest.id).toBe("c");
});

// A file is only gone once every upload of it is dismissed — otherwise
// dismissing the current version would resurrect the one it replaced.
it.each([
{ name: "no version", dismissedIds: [] as string[], dismissed: false },
{ name: "only the newest version", dismissedIds: ["b"], dismissed: false },
{ name: "every version", dismissedIds: ["a", "b"], dismissed: true },
])(
"reports dismissed as $dismissed when $name is",
({ dismissedIds, dismissed }) => {
const groups = groupRunArtifactVersions(
[
{ id: "a", name: "report.md", uploaded_at: "2026-07-27T08:00:00Z" },
{ id: "b", name: "report.md", uploaded_at: "2026-07-27T09:00:00Z" },
].map((artifact) => ({
...artifact,
dismissed_at: dismissedIds.includes(artifact.id)
? "2026-07-27T10:00:00Z"
: null,
})),
);

expect(groups[0]?.dismissed).toBe(dismissed);
},
);

it("skips artifacts with no name", () => {
expect(
groupRunArtifactVersions([{ uploaded_at: "2026-07-27T08:00:00Z" }]),
).toEqual([]);
});
});
65 changes: 65 additions & 0 deletions products/desktop/packages/core/src/canvas/runArtifactSchemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const runArtifactSchema = z.object({
content_type: z.string().optional(),
storage_path: z.string().optional(),
uploaded_at: z.string().optional(),
dismissed_at: z.string().nullish(),
});
export type RunArtifact = z.infer<typeof runArtifactSchema>;

Expand All @@ -31,3 +32,67 @@ export function parseRunArtifacts(
return type && types.includes(type) ? [parsed.data] : [];
});
}

/** Names a version by its position in a newest-first group. */
export function runArtifactVersionLabel(index: number, total: number): string {
return index === 0 ? "Latest" : `Version ${total - index}`;
}

/**
* A render key for one version of a file. Every identifying field goes in
* because a manifest entry is only guaranteed to carry its name — two versions
* collide only when they are indistinguishable, and then their order is moot.
*/
export function runArtifactVersionKey(artifact: {
id?: string;
storage_path?: string;
uploaded_at?: string;
}): string {
return [artifact.id, artifact.storage_path, artifact.uploaded_at].join(":");
}

interface VersionedArtifact {
name?: string;
uploaded_at?: string;
dismissed_at?: string | null;
}

export interface RunArtifactVersions<T extends VersionedArtifact> {
name: string;
/** Newest upload first. Always holds at least one entry. */
versions: T[];
latest: T;
/** Every version is dismissed, so the file as a whole is hidden. */
dismissed: boolean;
}

/**
* Group a run's artifacts into one entry per file name, newest upload first.
*
* Re-uploading a file is how an agent revises a deliverable, so the copies share
* a name and only the newest is the current file. Earlier ones stay in the group
* rather than being dropped, so a version the agent replaced is still reachable.
*/
export function groupRunArtifactVersions<T extends VersionedArtifact>(
artifacts: T[],
): RunArtifactVersions<T>[] {
const byName = new Map<string, T[]>();
for (const artifact of artifacts) {
if (!artifact.name) continue;
const group = byName.get(artifact.name);
if (group) group.push(artifact);
else byName.set(artifact.name, [artifact]);
}

return [...byName].map(([name, group]) => {
const versions = [...group].sort((a, b) =>
(b.uploaded_at ?? "").localeCompare(a.uploaded_at ?? ""),
);
return {
name,
versions,
latest: versions[0] as T,
dismissed: versions.every((version) => Boolean(version.dismissed_at)),
};
});
}
19 changes: 19 additions & 0 deletions products/desktop/packages/core/src/sessions/sessionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7578,6 +7578,25 @@ export class SessionService {
);
}

async setCloudRunArtifactsDismissed(
taskId: string,
runId: string,
artifactIds: string[],
dismissed: boolean,
): Promise<TaskRunArtifact[]> {
const authStatus = await this.getAuthCredentialsStatus();
if (authStatus.kind !== "ready") {
throw new Error("Not signed in to PostHog");
}

return authStatus.auth.client.setTaskRunArtifactsDismissed(
taskId,
runId,
artifactIds,
dismissed,
);
}

private getCloudAttachmentManifest(
client: AuthClient,
authIdentity: string,
Expand Down
1 change: 1 addition & 0 deletions products/desktop/packages/shared/src/domain-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ export interface TaskRunArtifact {
metadata?: TaskRunArtifactMetadata;
storage_path?: string;
uploaded_at?: string;
dismissed_at?: string | null;
}

export const TERMINAL_STATUSES = ["completed", "failed", "cancelled"] as const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,46 @@ describe("TaskArtifactsList", () => {
expect(screen.getByText("File · 2 KB")).toBeInTheDocument();
});

// A file dismissed in the chat's Files box has to go from this pane too, but
// only once every version of it is dismissed.
it.each([
{
name: "keeps a file whose newest upload alone was dismissed",
dismissedNewest: true,
dismissedOldest: false,
visible: true,
},
{
name: "leaves out a file whose every version was dismissed",
dismissedNewest: true,
dismissedOldest: true,
visible: false,
},
])("$name", ({ dismissedNewest, dismissedOldest, visible }) => {
const dismissedAt = "2026-07-27T10:00:00+00:00";
mocks.runs = [
run("run-1", {
artifacts: [
outputFile({
id: "a",
uploaded_at: "2026-07-27T08:00:00+00:00",
...(dismissedOldest ? { dismissed_at: dismissedAt } : {}),
}),
outputFile({
id: "b",
storage_path: "runs/1/report-v2.md",
uploaded_at: "2026-07-27T09:00:00+00:00",
...(dismissedNewest ? { dismissed_at: dismissedAt } : {}),
}),
],
}),
];

render(<TaskArtifactsList task={task} timeline={[]} />);

expect(screen.queryByText("report.md") !== null).toBe(visible);
});

it.each([
{ name: "a plan", type: "plan" as const },
{ name: "a user attachment", type: "user_attachment" as const },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,14 @@ export function buildRows(
// revise a deliverable and upload it again under the same name, so keeping
// every copy would bury the current one under its own drafts.
const newestByName = new Map<string, { file: RunArtifact; runId: string }>();
const undismissedNames = new Set<string>();
for (const run of allRuns) {
for (const outputPr of readPrUrls(run.output)) {
addPr(outputPr, `output-pr:${outputPr}`);
}
for (const file of readRunOutputs(run)) {
if (!file.name) continue;
if (!file.dismissed_at) undismissedNames.add(file.name);
const previous = newestByName.get(file.name);
const isNewer =
!previous ||
Expand All @@ -182,6 +184,9 @@ export function buildRows(
}
}
for (const [name, { file, runId }] of newestByName) {
// A file goes only when every version of it is dismissed, so dismissing the
// one on show cannot resurface the copy it replaced.
if (!undismissedNames.has(name)) continue;
rows.push({
kind: "file",
key: `file:${file.id ?? file.storage_path ?? name}`,
Expand Down
Loading
Loading