Skip to content

Commit 7e3682f

Browse files
claudeclaude
committed
revert(daemon): restore archiving directory artifacts for the inventory
Making DaemonArtifactInventoryEntry.sizeBytes optional is not an additive change under ADR 0006: released CLIs print the field without a guard and would show "undefined bytes". The lazy inventory needs either a protocol bump or a client that tolerates a missing size, so it moves to its own change. This restores the inventory behavior, the hostEnv chain, the removed tests and the wire ledger entry. This reverts commits 92de317 and 79ee947 and the ledger part of 12c6599. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_016qAu53rMak1bXMuR5U2gF4
1 parent 12c6599 commit 7e3682f

13 files changed

Lines changed: 218 additions & 159 deletions

File tree

‎packages/command-output/src/management/output.test.ts‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -256,30 +256,16 @@ describe('artifactsCliOutput', () => {
256256
createdAt: '2026-07-02T12:00:00.000Z',
257257
expiresAt: '2026-07-02T12:15:00.000Z',
258258
},
259-
{
260-
id: 'artifact-2',
261-
artifactType: 'trace-log',
262-
filename: 'capture.trace.tar.gz',
263-
mimeType: 'application/gzip',
264-
createdAt: '2026-07-02T12:00:00.000Z',
265-
expiresAt: '2026-07-02T12:15:00.000Z',
266-
},
267259
],
268260
},
269261
});
270262

271263
expect(output.text).toBe(
272-
[
273-
'screenshot.png (screenshot): application/octet-stream 123 bytes id=artifact-1',
274-
'capture.trace.tar.gz (trace-log): application/gzip id=artifact-2',
275-
].join('\n'),
264+
'screenshot.png (screenshot): application/octet-stream 123 bytes id=artifact-1',
276265
);
277266
expect(output.data).toMatchObject({
278267
source: 'daemon',
279-
artifacts: [
280-
{ id: 'artifact-1', artifactType: 'screenshot', filename: 'screenshot.png' },
281-
{ id: 'artifact-2', artifactType: 'trace-log', filename: 'capture.trace.tar.gz' },
282-
],
268+
artifacts: [{ id: 'artifact-1', artifactType: 'screenshot', filename: 'screenshot.png' }],
283269
});
284270
});
285271
});

‎packages/command-output/src/management/output.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,7 @@ function formatCloudArtifactLine(artifact: CloudArtifactsResult['cloudArtifacts'
272272

273273
function formatDaemonArtifactLine(artifact: DaemonArtifactsResult['artifacts'][number]): string {
274274
const type = artifact.artifactType ? ` (${artifact.artifactType})` : '';
275-
const size = artifact.sizeBytes === undefined ? '' : ` ${artifact.sizeBytes} bytes`;
276-
return `${artifact.filename}${type}: ${artifact.mimeType}${size} id=${artifact.id}`;
275+
return `${artifact.filename}${type}: ${artifact.mimeType} ${artifact.sizeBytes} bytes id=${artifact.id}`;
277276
}
278277

279278
function formatCloudArtifactsRetryCommand(result: CloudArtifactsResult): string | undefined {

‎packages/contracts/src/cloud-artifacts.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,7 @@ export type DaemonArtifactInventoryEntry = {
4444
artifactType?: DaemonArtifactType;
4545
filename: string;
4646
mimeType: string;
47-
/**
48-
* Byte size of the download. Absent for a directory artifact that has not been downloaded yet:
49-
* the daemon builds its archive on the first download, not to list it.
50-
*/
51-
sizeBytes?: number;
47+
sizeBytes: number;
5248
createdAt: string;
5349
expiresAt: string;
5450
/** Display rotation a screenshot was rendered in; absent when the capture reported none. */

‎packages/daemon/src/resources/artifact-tracking.test.ts‎

Lines changed: 68 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -17,44 +17,75 @@ import {
1717
type OpenedDownloadableArtifact,
1818
} from './artifact-tracking.ts';
1919

20-
test('listing a directory artifact builds no archive until its first download', async () => {
21-
const root = mkdtempForTestSync('fu-art-inventory-lazy-');
22-
const artifactPath = path.join(root, 'capture.trace');
23-
fs.mkdirSync(artifactPath);
24-
fs.writeFileSync(path.join(artifactPath, 'payload'), 'directory artifact');
25-
const runtime = createRuntimeStateHandle();
26-
const archive = vi.spyOn(command, 'runCmd');
27-
const downloads: OpenedDownloadableArtifact[] = [];
28-
try {
29-
const id = runtime.run(() =>
30-
trackDownloadableArtifact({ artifactPath, artifactType: 'trace-log' }),
31-
);
32-
const listed = await runtime.run(() => listDownloadableArtifacts(undefined));
33-
expect(listed).toEqual([
34-
expect.objectContaining({
35-
id,
36-
filename: 'capture.trace.tar.gz',
37-
mimeType: 'application/gzip',
38-
}),
39-
]);
40-
expect(listed[0]).not.toHaveProperty('sizeBytes');
41-
expect(archive).not.toHaveBeenCalled();
42-
43-
const download = await runtime.run(() => openDownloadableArtifact(id, undefined, process.env));
44-
downloads.push(download);
45-
expect(archive).toHaveBeenCalledTimes(1);
46-
const relisted = await runtime.run(() => listDownloadableArtifacts(undefined));
47-
expect(relisted[0]?.sizeBytes).toBe(download.sizeBytes);
48-
} finally {
49-
for (const download of downloads) {
50-
download.stream.destroy();
51-
await vi.waitFor(() => expect(download.stream.closed).toBe(true));
52-
download.release();
20+
test.each(['cleanup', 'expiry'] as const)(
21+
'%s aborts an inventory-only archive and removes its source after tar settles',
22+
async (trigger) => {
23+
const root = mkdtempForTestSync('fu-art-inventory-expiry-');
24+
const artifactPath = path.join(root, 'capture.trace');
25+
fs.mkdirSync(artifactPath);
26+
fs.writeFileSync(path.join(artifactPath, 'payload'), 'pending inventory archive');
27+
const runtime = createRuntimeStateHandle();
28+
let start!: () => void;
29+
const started = new Promise<void>((resolve) => {
30+
start = resolve;
31+
});
32+
let finish!: () => void;
33+
const stopped = new Promise<void>((resolve) => {
34+
finish = resolve;
35+
});
36+
let archivePath = '';
37+
let signal: AbortSignal | undefined;
38+
const runCmd = command.runCmd;
39+
const archive = vi.spyOn(command, 'runCmd').mockImplementationOnce(async (...args) => {
40+
archivePath = String(args[1]?.[1]);
41+
signal = args[2]?.signal;
42+
fs.writeFileSync(archivePath, 'partial tar output');
43+
start();
44+
await stopped;
45+
return await runCmd(...args);
46+
});
47+
let listing: ReturnType<typeof listDownloadableArtifacts> | undefined;
48+
try {
49+
const timers = vi.spyOn(globalThis, 'setTimeout');
50+
let id: string;
51+
let expire: (() => void) | undefined;
52+
try {
53+
id = runtime.run(() =>
54+
trackDownloadableArtifact({ artifactPath, artifactType: 'trace-log' }),
55+
);
56+
expire = timers.mock.calls.find(([, delay]) => delay === 15 * 60 * 1000)?.[0];
57+
} finally {
58+
timers.mockRestore();
59+
}
60+
listing = runtime.run(() => listDownloadableArtifacts(undefined, process.env));
61+
await started;
62+
expect(fs.existsSync(archivePath)).toBe(true);
63+
if (trigger === 'expiry') {
64+
expect(expire).toBeTypeOf('function');
65+
runtime.run(() => expire!());
66+
} else {
67+
runtime.run(() => cleanupDownloadableArtifact(id));
68+
}
69+
expect(signal?.aborted).toBe(true);
70+
expect(signal?.reason).toMatchObject({
71+
code: 'COMMAND_FAILED',
72+
details: { reason: 'RESOURCE_EXPIRED' },
73+
});
74+
expect(fs.existsSync(artifactPath)).toBe(true);
75+
expect(fs.existsSync(archivePath)).toBe(true);
76+
finish();
77+
expect(await listing).toEqual([]);
78+
expect(fs.existsSync(artifactPath)).toBe(false);
79+
expect(fs.existsSync(path.dirname(archivePath))).toBe(false);
80+
expect(archive).toHaveBeenCalledTimes(1);
81+
} finally {
82+
finish();
83+
if (listing) await listing;
84+
runtime.dispose();
85+
archive.mockRestore();
5386
}
54-
runtime.dispose();
55-
archive.mockRestore();
56-
}
57-
});
87+
},
88+
);
5889

5990
test('a missing cached directory archive is rebuilt from the live source', async () => {
6091
const root = mkdtempForTestSync('fu-art-cache-rebuild-');

‎packages/daemon/src/resources/artifact-tracking.ts‎

Lines changed: 34 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -245,72 +245,47 @@ function cancelArtifactDownload(
245245

246246
export async function listDownloadableArtifacts(
247247
tenantId: string | undefined,
248+
hostEnv: NodeJS.ProcessEnv,
248249
): Promise<DaemonArtifactInventoryEntry[]> {
249250
const artifacts: DaemonArtifactInventoryEntry[] = [];
250251
for (const [id, entry] of pendingArtifacts()) {
251252
if (!canReadArtifact(entry, tenantId)) continue;
252-
const listed = await readInventoryEntry(id, entry);
253-
if (listed) artifacts.push(listed);
253+
let stat: fs.Stats;
254+
try {
255+
stat = fs.statSync(entry.artifactPath);
256+
} catch {
257+
cleanupDownloadableArtifact(id);
258+
continue;
259+
}
260+
const payload = await prepareArtifactPayloadForInventory(id, entry, stat, hostEnv);
261+
if (!payload || entry.cleanupRequested) continue;
262+
artifacts.push({
263+
id,
264+
...(entry.artifactType !== undefined ? { artifactType: entry.artifactType } : {}),
265+
filename: payload.fileName ?? id,
266+
mimeType: payload.mimeType,
267+
sizeBytes: payload.sizeBytes,
268+
createdAt: new Date(entry.createdAt).toISOString(),
269+
expiresAt: new Date(entry.createdAt + ARTIFACT_CLEANUP_TIMEOUT_MS).toISOString(),
270+
...(entry.displayRotation !== undefined ? { displayRotation: entry.displayRotation } : {}),
271+
...(entry.sessionName !== undefined ? { sessionName: entry.sessionName } : {}),
272+
});
254273
}
255274
return artifacts;
256275
}
257276

258-
/** One entry's inventory row, or undefined when its source is gone or it is being cleaned up. */
259-
async function readInventoryEntry(
260-
id: string,
261-
entry: ArtifactEntry,
262-
): Promise<DaemonArtifactInventoryEntry | undefined> {
263-
let stat: fs.Stats;
264-
try {
265-
stat = await fs.promises.stat(entry.artifactPath);
266-
} catch {
267-
cleanupDownloadableArtifact(id);
268-
return undefined;
269-
}
270-
const listing = describeArtifactForInventory(id, entry, stat);
271-
if (!listing || entry.cleanupRequested) return undefined;
272-
return {
273-
id,
274-
...(entry.artifactType !== undefined ? { artifactType: entry.artifactType } : {}),
275-
filename: listing.fileName,
276-
mimeType: listing.mimeType,
277-
...(listing.sizeBytes !== undefined ? { sizeBytes: listing.sizeBytes } : {}),
278-
createdAt: new Date(entry.createdAt).toISOString(),
279-
expiresAt: new Date(entry.createdAt + ARTIFACT_CLEANUP_TIMEOUT_MS).toISOString(),
280-
...(entry.displayRotation !== undefined ? { displayRotation: entry.displayRotation } : {}),
281-
...(entry.sessionName !== undefined ? { sessionName: entry.sessionName } : {}),
282-
};
283-
}
284-
285-
/**
286-
* What an inventory reports without preparing a payload. A directory is archived only on its first
287-
* download, so its size is known only after that archive exists.
288-
*/
289-
function describeArtifactForInventory(
277+
async function prepareArtifactPayloadForInventory(
290278
artifactId: string,
291279
entry: ArtifactEntry,
292280
stat: fs.Stats,
293-
): { fileName: string; mimeType: string; sizeBytes?: number } | null {
294-
if (stat.isFile()) {
295-
return {
296-
fileName: safeArtifactFilename(entry.fileName, artifactId),
297-
mimeType: DEFAULT_DOWNLOAD_MIME_TYPE,
298-
sizeBytes: stat.size,
299-
};
281+
hostEnv: NodeJS.ProcessEnv,
282+
): Promise<PreparedDownloadableArtifact | null> {
283+
try {
284+
return await prepareArtifactPayload(artifactId, entry, stat, hostEnv);
285+
} catch {
286+
// Inventory is best-effort; direct downloads still surface artifact-specific errors.
287+
return null;
300288
}
301-
if (!stat.isDirectory()) return null;
302-
const archive = entry.directoryArchive;
303-
return {
304-
fileName: directoryArchiveFilename(artifactId, entry),
305-
mimeType: DIRECTORY_ARCHIVE_MIME_TYPE,
306-
...(archive && fs.existsSync(archive.archivePath) ? { sizeBytes: archive.sizeBytes } : {}),
307-
};
308-
}
309-
310-
function directoryArchiveFilename(artifactId: string, entry: ArtifactEntry): string {
311-
return archiveFilename(
312-
safeArtifactFilename(entry.fileName, path.basename(entry.artifactPath) || artifactId),
313-
);
314289
}
315290

316291
function readDownloadableArtifactEntry(
@@ -418,7 +393,11 @@ async function createDirectoryArchive(
418393
try {
419394
cleanupDirectoryArchive(entry);
420395

421-
const fileName = directoryArchiveFilename(artifactId, entry);
396+
const sourceName = safeArtifactFilename(
397+
entry.fileName,
398+
path.basename(entry.artifactPath) || artifactId,
399+
);
400+
const fileName = archiveFilename(sourceName);
422401
tempDir = fs.mkdtempSync(
423402
path.join(os.tmpdir(), `agent-device-download-${crypto.randomUUID()}-`),
424403
);

‎packages/daemon/src/server/__tests__/http-server-artifacts.test.ts‎

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
listDownloadableArtifacts,
1111
trackDownloadableArtifact,
1212
} from '../../resources/artifact-tracking.ts';
13-
import { runCmdSync } from '@agent-device/host-kit/command';
13+
import { runCmdSync, withCommandExecutorOverride } from '@agent-device/host-kit/command';
1414
import {
1515
closeLoopbackServer,
1616
listenOnLoopback,
@@ -61,15 +61,21 @@ test(ANDROID_ARTIFACTS_CONTRACT_EVIDENCE.testName, async () => {
6161

6262
try {
6363
assert.deepEqual(
64-
(await listDownloadableArtifacts(undefined)).map((artifact) => artifact.filename),
64+
(await listDownloadableArtifacts(undefined, process.env)).map(
65+
(artifact) => artifact.filename,
66+
),
6567
['public.txt'],
6668
);
6769
assert.deepEqual(
68-
(await listDownloadableArtifacts('tenant-a')).map((artifact) => artifact.filename),
70+
(await listDownloadableArtifacts('tenant-a', process.env)).map(
71+
(artifact) => artifact.filename,
72+
),
6973
['public.txt', 'tenant-a.txt'],
7074
);
7175
assert.deepEqual(
72-
(await listDownloadableArtifacts('tenant-b')).map((artifact) => artifact.filename),
76+
(await listDownloadableArtifacts('tenant-b', process.env)).map(
77+
(artifact) => artifact.filename,
78+
),
7379
['public.txt', 'tenant-b.txt'],
7480
);
7581
} finally {
@@ -80,6 +86,46 @@ test(ANDROID_ARTIFACTS_CONTRACT_EVIDENCE.testName, async () => {
8086
}
8187
});
8288

89+
test('downloadable artifact inventory skips directory artifacts that fail to archive', async () => {
90+
const tempDir = mkdtempForTestSync('agent-device-artifacts-archive-error-');
91+
const filePath = path.join(tempDir, 'report.json');
92+
const tracePath = path.join(tempDir, '--profile.trace');
93+
fs.writeFileSync(filePath, '{}\n');
94+
fs.mkdirSync(tracePath, { recursive: true });
95+
fs.writeFileSync(path.join(tracePath, 'metadata.json'), '{}\n');
96+
const artifactIds = [
97+
trackDownloadableArtifact({
98+
artifactPath: filePath,
99+
artifactType: 'test-report',
100+
fileName: 'report.json',
101+
}),
102+
trackDownloadableArtifact({
103+
artifactPath: tracePath,
104+
artifactType: 'trace-log',
105+
fileName: 'profile.trace',
106+
}),
107+
];
108+
109+
try {
110+
const artifacts = await withCommandExecutorOverride(
111+
(cmd) => {
112+
if (cmd !== 'tar') return undefined;
113+
throw new Error('tar unavailable');
114+
},
115+
async () => await listDownloadableArtifacts(undefined, process.env),
116+
);
117+
assert.deepEqual(
118+
artifacts.map((artifact) => artifact.filename),
119+
['report.json'],
120+
);
121+
} finally {
122+
for (const artifactId of artifactIds) {
123+
cleanupDownloadableArtifact(artifactId);
124+
}
125+
fs.rmSync(tempDir, { recursive: true, force: true });
126+
}
127+
});
128+
83129
test('daemon artifact inventory exposes directory artifacts as tar.gz downloads', async (t) => {
84130
if (await skipWhenLoopbackUnavailable(t)) return;
85131

@@ -111,8 +157,7 @@ test('daemon artifact inventory exposes directory artifacts as tar.gz downloads'
111157
assert.ok(artifact, `expected ${artifactId} in artifact inventory`);
112158
assert.equal(artifact.filename, 'profile.trace.tar.gz');
113159
assert.equal(artifact.mimeType, 'application/gzip');
114-
// The archive is built on the first download, so the listing cannot know its size yet.
115-
assert.equal(artifact.sizeBytes, undefined);
160+
assert.ok(artifact.sizeBytes > 0);
116161

117162
const consumingDownload = await fetch(
118163
`${baseUrl}/artifacts/${encodeURIComponent(artifactId)}?download=1`,
@@ -122,7 +167,7 @@ test('daemon artifact inventory exposes directory artifacts as tar.gz downloads'
122167
);
123168
assert.equal(consumingDownload.status, 200);
124169
assert.equal(consumingDownload.headers.get('content-type'), 'application/gzip');
125-
assert.ok(Number(consumingDownload.headers.get('content-length')) > 0);
170+
assert.equal(consumingDownload.headers.get('content-length'), String(artifact.sizeBytes));
126171
assert.match(
127172
consumingDownload.headers.get('content-disposition') ?? '',
128173
/profile\.trace\.tar\.gz/,
@@ -386,7 +431,7 @@ test('downloadable artifact inventory reports a screenshot display rotation only
386431
});
387432

388433
try {
389-
const inventory = await listDownloadableArtifacts(undefined);
434+
const inventory = await listDownloadableArtifacts(undefined, process.env);
390435
const rotated = inventory.find((entry) => entry.id === rotatedId);
391436
const plain = inventory.find((entry) => entry.id === plainId);
392437
assert.equal(rotated?.displayRotation, 'landscape-right');

0 commit comments

Comments
 (0)