Skip to content
Closed
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
3 changes: 3 additions & 0 deletions packages/chat/src/protocol/items.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ export const userContentSchema = z.discriminatedUnion("type", [
z.looseObject({
type: z.literal("attachment"),
attachmentId: z.string().min(1),
/** The storage object behind a cloud workspace's attachment; set, it is the identity. */
fileId: z.string().uuid().optional(),
name: z.string(),
mimeType: z.string(),
sizeBytes: z.number().int().nonnegative().optional(),
}),
]);
export type UserContent = z.infer<typeof userContentSchema>;
Expand Down
18 changes: 18 additions & 0 deletions packages/host-service/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ import {
} from "./terminal-agents";
import { matchesAgentBinding } from "./terminal-agents/matches-agent-binding";
import { appRouter } from "./trpc/router";
import {
ATTACHMENT_SWEEP_INTERVAL_MS,
sweepAttachmentStore,
} from "./trpc/router/attachments/sweep";
import { gitStatusStore } from "./trpc/router/git/utils/git-status-store";
import {
disposeLocalMobileDevices,
Expand Down Expand Up @@ -385,6 +389,18 @@ export function createApp(options: CreateAppOptions): CreateAppResult {
? null
: setInterval(() => void runProjectPurge(), PROJECT_PURGE_INTERVAL_MS);
projectPurgeTimer?.unref?.();
const runAttachmentSweep = () =>
sweepAttachmentStore().catch((err) => {
console.warn("[host-service] attachment sweep failed:", err);
});
const attachmentSweepTimer =
process.env.SUPERSET_HOST_RUN_MODE === "sandbox"
? null
: setInterval(
() => void runAttachmentSweep(),
ATTACHMENT_SWEEP_INTERVAL_MS,
);
attachmentSweepTimer?.unref?.();
if (process.env.SUPERSET_HOST_RUN_MODE !== "sandbox") {
startLocalMobileDeviceCleanup(db);
}
Expand Down Expand Up @@ -415,6 +431,7 @@ export function createApp(options: CreateAppOptions): CreateAppResult {
console.warn("[host-service] archived-workspace reconcile failed:", err);
});
await runProjectPurge();
await runAttachmentSweep();
// Re-share the default account's Claude/Codex config into the selected
// provider profiles. Last: it touches no host state the sweeps above
// repair, and a slow filesystem must not delay them.
Expand Down Expand Up @@ -546,6 +563,7 @@ export function createApp(options: CreateAppOptions): CreateAppResult {
const ownsDb = options.db === undefined;
const dispose = async (): Promise<void> => {
if (projectPurgeTimer) clearInterval(projectPurgeTimer);
if (attachmentSweepTimer) clearInterval(attachmentSweepTimer);
await disposeWorkspaceTitleJobs(db);
// Each step is best-effort and isolated: a throw in one cleanup must
// not skip the others, otherwise a flaky `.stop()` could leak the
Expand Down
4 changes: 2 additions & 2 deletions packages/host-service/src/chat-v3/acpHarnesses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { dirname, join } from "node:path";
import type { HarnessFactory } from "@superset/chat-runtime";
import { createAcpAdapter } from "@superset/chat-runtime";
import type { HostDb } from "../db";
import { resolveAttachmentPath } from "../trpc/router/attachments/storage";
import { handOutAttachment } from "../trpc/router/attachments/storage";
import { captureSessionAccount } from "../trpc/router/usage/session-account/session-account";
import { ACP_HARNESSES } from "./acpCatalogue";
import { resolveAgentCli } from "./agentCli";
Expand All @@ -13,7 +13,7 @@ import { storeAgentImage } from "./agentImages";
import type { ChatAgentBridge } from "./chatAgentBridge";

async function resolveAttachment(attachmentId: string) {
const resolved = resolveAttachmentPath(attachmentId);
const resolved = handOutAttachment(attachmentId);
if (!resolved) return null;
return {
path: resolved.path,
Expand Down
113 changes: 113 additions & 0 deletions packages/host-service/src/trpc/router/agents/agents.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ import type { HostDb } from "../../../db";
import * as schema from "../../../db/schema";
import { TerminalAgentStore } from "../../../terminal-agents";
import { claudeProjectDirName } from "../../../terminal-agents/harness-sessions/claude";
import { importCloudAttachments } from "../attachments/attachments";
import {
type AttachmentMetadata,
readAttachmentMetadata,
writeAttachment,
} from "../attachments/storage";
import { setDefaultAccountSelection } from "../usage/default-account";
import {
bindResumedSession,
Expand Down Expand Up @@ -1159,3 +1165,110 @@ describe("continuationTarget", () => {
).toEqual({ terminalId, label: "Team Claude" });
});
});

describe("attachments handed to an agent launch", () => {
const workspaceId = "11111111-1111-1111-1111-111111111111";
let manifestDir: string;

beforeEach(() => {
manifestDir = mkdtempSync(join(tmpdir(), "superset-agent-attachments-"));
process.env.HOST_MANIFEST_DIR = manifestDir;
});

afterEach(() => {
rmSync(manifestDir, { recursive: true, force: true });
delete process.env.HOST_MANIFEST_DIR;
});

function seedClaude(db: HostDb) {
db.insert(schema.hostAgentConfigs)
.values({
id: "00000000-0000-0000-0000-00000000000a",
presetId: "claude",
label: "Claude",
command: "claude",
argsJson: "[]",
promptTransport: "argv",
promptArgsJson: "[]",
resumeArgsJson: "[]",
forkArgsJson: "[]",
envJson: "{}",
displayOrder: 0,
})
.run();
}

it("a cloud launch's first-prompt part carries the fileId it was imported from", async () => {
const server = Bun.serve({
port: 0,
fetch: () => new Response("png"),
});
try {
const fileId = crypto.randomUUID();
const [imported] = await importCloudAttachments(
{
api: {
attachment: {
resolve: {
mutate: async () => [
{
fileId,
name: "shot.png",
contentType: "image/png",
sizeBytes: 3,
url: server.url.href,
},
],
},
},
},
},
[fileId],
);
const db = createTestDb();
seedClaude(db);

const target = chatLaunchTarget(db, {
workspaceId,
agent: "claude",
prompt: "look",
surface: "chat",
attachmentIds: [imported?.attachmentId ?? ""],
});
expect(target?.attachments).toEqual([
{
attachmentId: imported?.attachmentId ?? "",
fileId,
name: "shot.png",
mimeType: "image/png",
sizeBytes: 3,
},
]);
} finally {
server.stop(true);
}
});

it("a terminal launch stamps the attachments it lists as used", () => {
const metadata: AttachmentMetadata = {
attachmentId: crypto.randomUUID(),
mediaType: "image/png",
sizeBytes: 1,
createdAt: 0,
lastUsedAt: 0,
};
writeAttachment(new Uint8Array([1]), metadata);
const db = createTestDb();
seedClaude(db);

buildTerminalAgentLaunch(db, {
workspaceId,
agent: "claude",
prompt: "look",
attachmentIds: [metadata.attachmentId],
});
expect(
readAttachmentMetadata(metadata.attachmentId)?.lastUsedAt,
).toBeGreaterThan(0);
});
});
29 changes: 21 additions & 8 deletions packages/host-service/src/trpc/router/agents/agents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ import {
import { hasHarnessSession } from "../../../terminal-agents/harness-sessions";
import type { HostServiceContext } from "../../../types";
import { protectedProcedure, router } from "../../index";
import { resolveAttachmentPath } from "../attachments/storage";
import {
handOutAttachment,
resolveAttachmentPath,
} from "../attachments/storage";
import { createChatSession } from "../sessions/create-chat-session";
import { toTerminalSessionError } from "../terminal/errors";
import { seedAgentFolderTrust } from "../workspace-creation/shared/seed-agent-trust";
Expand Down Expand Up @@ -425,7 +428,7 @@ export function buildTerminalAgentLaunch(

const resolvedAttachments: Array<{ attachmentId: string; path: string }> = [];
for (const attachmentId of input.attachmentIds ?? []) {
const resolved = resolveAttachmentPath(attachmentId);
const resolved = handOutAttachment(attachmentId);
if (!resolved) {
throw new TRPCError({
code: "NOT_FOUND",
Expand Down Expand Up @@ -635,13 +638,25 @@ function bindingRunsConfig(
return binding.agentId === launchIdentity(config);
}

/**
* An imported copy keeps its `fileId` next to the host id: the adapters read
* `attachmentId` today, and `fileId` is what renders the part without this box.
*/
export interface ChatLaunchAttachment {
attachmentId: string;
fileId?: string;
name: string;
mimeType: string;
sizeBytes: number;
}

export function chatLaunchTarget(
db: HostDb,
input: AgentRunInput,
): {
harness: string;
label: string;
attachments: Array<{ attachmentId: string; name: string; mimeType: string }>;
attachments: ChatLaunchAttachment[];
} | null {
if (
input.surface !== "chat" ||
Expand All @@ -651,18 +666,16 @@ export function chatLaunchTarget(
) {
return null;
}
const attachments: Array<{
attachmentId: string;
name: string;
mimeType: string;
}> = [];
const attachments: ChatLaunchAttachment[] = [];
for (const attachmentId of input.attachmentIds ?? []) {
const resolved = resolveAttachmentPath(attachmentId);
Comment on lines +669 to 671

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.

P1 Chat can lose accepted attachments

chatLaunchTarget accepts attachments without refreshing lastUsedAt. The agent reads them only after its async startup finishes. If an attachment is past retention and the sweep runs during startup, the sweep deletes it before the agent reads it. The prompt then goes through without that attachment.

Use handOutAttachment when accepting attachments for a chat launch, as the terminal launch already does.

Suggested change
const attachments: ChatLaunchAttachment[] = [];
for (const attachmentId of input.attachmentIds ?? []) {
const resolved = resolveAttachmentPath(attachmentId);
const attachments: ChatLaunchAttachment[] = [];
for (const attachmentId of input.attachmentIds ?? []) {
const resolved = handOutAttachment(attachmentId);
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/agents/agents.ts
Line: 669-671

Comment:
**Chat can lose accepted attachments**

`chatLaunchTarget` accepts attachments without refreshing `lastUsedAt`. The agent reads them only after its async startup finishes. If an attachment is past retention and the sweep runs during startup, the sweep deletes it before the agent reads it. The prompt then goes through without that attachment.

Use `handOutAttachment` when accepting attachments for a chat launch, as the terminal launch already does.

```suggestion
	const attachments: ChatLaunchAttachment[] = [];
	for (const attachmentId of input.attachmentIds ?? []) {
		const resolved = handOutAttachment(attachmentId);
```

---

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

if (!resolved) return null;
attachments.push({
attachmentId,
...(resolved.metadata.fileId ? { fileId: resolved.metadata.fileId } : {}),
name: resolved.metadata.originalFilename ?? attachmentId,
mimeType: resolved.metadata.mediaType,
sizeBytes: resolved.metadata.sizeBytes,
});
}
const config = resolveHostAgentConfig(db, input.agent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test";
import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
import { homedir, tmpdir } from "node:os";
import { join } from "node:path";
import { MAX_INLINE_ATTACHMENT_BYTES } from "@superset/shared/attachment-limits";
import type { HostServiceContext } from "../../../types";
import { attachmentsRouter } from "./attachments";
import { MAX_INLINE_ATTACHMENT_BYTES } from "./constants";
import { attachmentsRouter, importCloudAttachments } from "./attachments";
import {
getAttachmentDir,
getAttachmentFilePath,
getAttachmentsRoot,
handOutAttachmentByFileId,
readAttachmentMetadata,
writeAttachmentMetadata,
} from "./storage";

let tempBase: string;
Expand Down Expand Up @@ -187,6 +190,20 @@ describe("attachmentsRouter.read", () => {
});
});

it("stamps the attachment as used", async () => {
const caller = createCaller();
const { attachmentId } = await caller.upload({
data: { kind: "base64", data: PNG_BASE64 },
mediaType: "image/png",
});
const metadata = readAttachmentMetadata(attachmentId);
if (!metadata) throw new Error("upload wrote no metadata");
writeAttachmentMetadata({ ...metadata, lastUsedAt: 0 });

await caller.read({ attachmentId });
expect(readAttachmentMetadata(attachmentId)?.lastUsedAt).toBeGreaterThan(0);
});

it("reports a missing attachment as NOT_FOUND", async () => {
await expect(
createCaller().read({
Expand All @@ -196,6 +213,43 @@ describe("attachmentsRouter.read", () => {
});
});

describe("importCloudAttachments", () => {
it("records the fileId a copy came from, so a lookup by fileId finds it", async () => {
const server = Bun.serve({
port: 0,
fetch: () => new Response(Buffer.from(PNG_BASE64, "base64")),
});
try {
const fileId = crypto.randomUUID();
const ctx = {
api: {
attachment: {
resolve: {
mutate: async () => [
{
fileId,
name: "shot.png",
contentType: "image/png",
sizeBytes: 68,
url: server.url.href,
},
],
},
},
},
};

const [imported] = await importCloudAttachments(ctx, [fileId]);
expect(imported?.fileId).toBe(fileId);
expect(handOutAttachmentByFileId(fileId)?.metadata.attachmentId).toBe(
imported?.attachmentId,
);
} finally {
server.stop(true);
}
});
});

describe("attachmentsRouter.delete", () => {
it("removes the attachment directory", async () => {
const caller = createCaller();
Expand Down
Loading
Loading