Skip to content

Commit 8b2dfb9

Browse files
thymikeethymikee
committed
fix(cli): preserve failures when diagnostics paths are invalid
1 parent 18c3a75 commit 8b2dfb9

3 files changed

Lines changed: 98 additions & 30 deletions

File tree

‎packages/remote/src/__tests__/remote-request-diagnostics.test.ts‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,10 @@ test('the record fetch names the held lease and sends no lease header without on
180180
}
181181
});
182182

183-
test.each(['.', '..'])(
184-
'invalid remote session %s cannot write or remove outside records',
185-
async (session) => {
183+
for (const session of ['.', '..']) {
184+
test(`invalid remote session ${session} cannot write or remove outside records`, async (t) => {
185+
if (await skipWhenLoopbackUnavailable(t)) return;
186+
186187
const stateDir = mkdtempForTestSync('remote-diagnostics-contained-');
187188
const outside = path.join(
188189
stateDir,
@@ -192,12 +193,25 @@ test.each(['.', '..'])(
192193
);
193194
fs.mkdirSync(path.dirname(outside), { recursive: true });
194195
fs.writeFileSync(outside, 'sentinel');
195-
const localized = await localizeRemoteDaemonError(
196-
{ ...remoteError(), diagnosticsRecord: { session, requestId: 'abc123' } },
197-
{ endpoint: { baseUrl: 'http://127.0.0.1:1', token: 'secret' }, stateDir },
198-
);
199-
assert.equal(localized.logPath, undefined);
200-
assert.equal(typeof localized.logPathUnavailable, 'string');
201-
assert.equal(fs.readFileSync(outside, 'utf8'), 'sentinel');
202-
},
203-
);
196+
let requests = 0;
197+
const server = http.createServer((_req, res) => {
198+
requests += 1;
199+
res.writeHead(200, { 'content-type': 'application/x-ndjson' });
200+
res.end(RECORD);
201+
});
202+
try {
203+
const port = await listenOnLoopback(server);
204+
const localized = await localizeRemoteDaemonError(
205+
{ ...remoteError(), diagnosticsRecord: { session, requestId: 'abc123' } },
206+
{ endpoint: { baseUrl: `http://127.0.0.1:${port}`, token: 'secret' }, stateDir },
207+
);
208+
assert.equal(fs.readFileSync(outside, 'utf8'), 'sentinel');
209+
assert.equal(localized.logPath, undefined);
210+
assert.equal(typeof localized.logPathUnavailable, 'string');
211+
assert.equal(requests, 0);
212+
} finally {
213+
await closeLoopbackServer(server);
214+
fs.rmSync(stateDir, { recursive: true, force: true });
215+
}
216+
});
217+
}

‎src/cli/failure-output.test.ts‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import assert from 'node:assert/strict';
2+
import fs from 'node:fs';
3+
import path from 'node:path';
24
import { test } from 'vitest';
35
import { AppError } from '@agent-device/kernel/errors';
4-
import { withCapturedStderrAsync } from '@agent-device/test-kit';
6+
import { mkdtempForTestSync, withCapturedStderrAsync } from '@agent-device/test-kit';
57
import { printHumanError } from './failure-output.ts';
68

79
test('printHumanError writes a body-less failure as its one summary line', async () => {
@@ -32,3 +34,58 @@ test('printHumanError closes a multi-line failure with the same summary line', a
3234
'Error (COMMAND_FAILED): boom. May have reached the device. Next: Run snapshot to check the screen before you retry. Read the diagnostics log for the failing step.',
3335
]);
3436
});
37+
38+
test.each(['.', '..'])(
39+
'printHumanError preserves the command failure for invalid remote session %s',
40+
async (session) => {
41+
const stateDir = mkdtempForTestSync('failure-output-invalid-record-');
42+
const outside = path.join(
43+
stateDir,
44+
...(session === '.' ? ['remote-diagnostics'] : []),
45+
'requests',
46+
'abc123.ndjson',
47+
);
48+
fs.mkdirSync(path.dirname(outside), { recursive: true });
49+
fs.writeFileSync(outside, 'private outside record');
50+
const stderr = await withCapturedStderrAsync(async () => {
51+
await printHumanError(new AppError('COMMAND_FAILED', 'Original command failed'), {
52+
debugTier: true,
53+
logTail: {
54+
daemonBaseUrl: 'https://remote.example.test',
55+
daemonPaths: { baseDir: stateDir, logPath: path.join(stateDir, 'daemon.log') },
56+
record: { session, requestId: 'abc123' },
57+
},
58+
});
59+
});
60+
assert.ok(stderr.startsWith('Error (COMMAND_FAILED): Original command failed'));
61+
assert.equal(stderr.includes('private outside record'), false);
62+
assert.equal(stderr.includes('[remote diagnostics]'), false);
63+
},
64+
);
65+
66+
test.each(['local', 'remote'] as const)(
67+
'printHumanError includes available %s diagnostics',
68+
async (source) => {
69+
const stateDir = mkdtempForTestSync('failure-output-valid-record-');
70+
const logPath = path.join(stateDir, 'daemon.log');
71+
const recordPath =
72+
source === 'local'
73+
? logPath
74+
: path.join(stateDir, 'remote-diagnostics', 'valid-session', 'requests', 'abc123.ndjson');
75+
fs.mkdirSync(path.dirname(recordPath), { recursive: true });
76+
fs.writeFileSync(recordPath, 'original failure evidence\n');
77+
const stderr = await withCapturedStderrAsync(async () => {
78+
await printHumanError(new AppError('COMMAND_FAILED', 'Original command failed'), {
79+
debugTier: true,
80+
logTail: {
81+
daemonBaseUrl: source === 'remote' ? 'https://remote.example.test' : undefined,
82+
daemonPaths: { baseDir: stateDir, logPath },
83+
record: { session: 'valid-session', requestId: 'abc123' },
84+
},
85+
});
86+
});
87+
assert.ok(stderr.startsWith('Error (COMMAND_FAILED): Original command failed'));
88+
assert.ok(stderr.includes(`[${source === 'local' ? 'daemon log' : 'remote diagnostics'}]`));
89+
assert.ok(stderr.includes('original failure evidence'));
90+
},
91+
);

‎src/cli/failure-output.ts‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -81,29 +81,26 @@ function failedParseCommand(argv: string[]): string | null {
8181
* a second round trip (#1801).
8282
*/
8383
function failureLogTail(options: FailureLogSource): string | undefined {
84-
if (!options.daemonBaseUrl) {
85-
return logFileTail('daemon log', options.daemonPaths.logPath);
86-
}
87-
if (!options.record) return undefined;
88-
// Recomputed from the locator through the same helper that wrote the copy, so
89-
// the tail can only ever come from the fetched record (absent when the fetch
90-
// failed, since nothing was written).
91-
return logFileTail(
92-
'remote diagnostics',
93-
resolveRemoteRequestDiagnosticsPath(options.daemonPaths.baseDir, options.record),
94-
);
95-
}
96-
97-
function logFileTail(label: string, logPath: string): string | undefined {
9884
try {
99-
if (!fs.existsSync(logPath)) return undefined;
100-
const tail = readTrailingLines(logPath, DAEMON_LOG_TAIL_MAX_BYTES, 200);
101-
return tail.trim().length > 0 ? `\n[${label}]\n${tail.trimEnd()}` : undefined;
85+
if (!options.daemonBaseUrl) {
86+
return logFileTail('daemon log', options.daemonPaths.logPath);
87+
}
88+
if (!options.record) return undefined;
89+
return logFileTail(
90+
'remote diagnostics',
91+
resolveRemoteRequestDiagnosticsPath(options.daemonPaths.baseDir, options.record),
92+
);
10293
} catch {
10394
return undefined;
10495
}
10596
}
10697

98+
function logFileTail(label: string, logPath: string): string | undefined {
99+
if (!fs.existsSync(logPath)) return undefined;
100+
const tail = readTrailingLines(logPath, DAEMON_LOG_TAIL_MAX_BYTES, 200);
101+
return tail.trim().length > 0 ? `\n[${label}]\n${tail.trimEnd()}` : undefined;
102+
}
103+
107104
/** The last `maxLines` lines within the file's last `maxBytes` bytes; the daemon log grows unbounded. */
108105
function readTrailingLines(filePath: string, maxBytes: number, maxLines: number): string {
109106
const fd = fs.openSync(filePath, 'r');

0 commit comments

Comments
 (0)