From 3aa034d3abf5c52522c0ea6ad78ded0ed60594ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 18 Aug 2026 09:01:46 +0200 Subject: [PATCH] fix: serve remote daemon request diagnostics to the caller (#1801) --- packages/kernel/src/errors.ts | 76 +++++- src/__tests__/cli-remote-diagnostics.test.ts | 180 +++++++++++++++ src/cli.ts | 39 +++- src/cli/parser/cli-help.ts | 1 + .../request-diagnostics-http.test.ts | 159 +++++++++++++ .../__tests__/request-execution-scope.test.ts | 4 +- src/daemon/__tests__/runtime-session.test.ts | 2 +- src/daemon/client/daemon-client-rpc.ts | 90 ++++++-- src/daemon/client/daemon-client-transport.ts | 16 +- src/daemon/downloadable-artifact-http.ts | 17 +- src/daemon/http-errors.ts | 16 ++ src/daemon/http-request-target.ts | 17 ++ src/daemon/request-admission.ts | 5 +- src/daemon/request-diagnostics-http.ts | 169 ++++++++++++++ src/daemon/request-execution-scope.ts | 13 +- src/daemon/request-finalization.ts | 6 +- src/daemon/request-router.ts | 5 +- src/daemon/request-save-script-policy.ts | 4 +- src/daemon/server/daemon-runtime.ts | 6 +- src/daemon/server/http-server.ts | 36 ++- src/daemon/session-store.ts | 36 ++- src/daemon/session-tenant-scope.ts | 17 ++ .../__tests__/device-input-state.test.ts | 2 +- .../android/__tests__/snapshot.test.ts | 8 +- .../core/__tests__/runner-client.test.ts | 2 +- .../core/__tests__/runner-session.test.ts | 2 +- .../remote-request-diagnostics.test.ts | 150 ++++++++++++ src/remote/remote-request-diagnostics.ts | 217 ++++++++++++++++++ src/utils/__tests__/diagnostics.test.ts | 2 +- src/utils/__tests__/exec.test.ts | 6 +- src/utils/__tests__/retry.test.ts | 2 +- src/utils/diagnostics.ts | 28 ++- src/utils/output.ts | 6 + .../remote-daemon-client.test.ts | 9 +- test/wire-compat/ledger.json | 70 +++++- test/wire-compat/surface.ts | 40 +++- website/docs/docs/configuration.md | 6 + 37 files changed, 1372 insertions(+), 92 deletions(-) create mode 100644 src/__tests__/cli-remote-diagnostics.test.ts create mode 100644 src/daemon/__tests__/request-diagnostics-http.test.ts create mode 100644 src/daemon/http-request-target.ts create mode 100644 src/daemon/request-diagnostics-http.ts create mode 100644 src/daemon/session-tenant-scope.ts create mode 100644 src/remote/__tests__/remote-request-diagnostics.test.ts create mode 100644 src/remote/remote-request-diagnostics.ts diff --git a/packages/kernel/src/errors.ts b/packages/kernel/src/errors.ts index e2ad36c8dd..bb47d6c07e 100644 --- a/packages/kernel/src/errors.ts +++ b/packages/kernel/src/errors.ts @@ -44,11 +44,24 @@ export function toAppErrorCode( return fallback; } +/** + * Locator for one request's diagnostics record on the daemon host, in the + * daemon's own vocabulary rather than as a filesystem path: `logPath` names + * that same record as a path, which only a caller on the daemon host can read. + * A remote caller fetches the record by this locator instead + * (`GET /sessions//requests//diagnostics`). + */ +export type DiagnosticsRecordRef = { + session: string; + requestId: string; +}; + /** * Details bag for AppError. Free-form context is allowed, but these keys carry * meaning at normalize/render time and must keep their types: * - `hint` — overrides `defaultHintForCode`; re-wraps preserve an existing hint. - * - `diagnosticId` / `logPath` — lifted onto the normalized error, stripped from details. + * - `diagnosticId` / `logPath` / `logPathUnavailable` / `diagnosticsRecord` — + * lifted onto the normalized error, stripped from details. * - `processExitError` + `stdout`/`stderr`/`exitCode` — marks a wrap of a real * process exit so normalizeError can surface the first meaningful stderr line; * build these via `execFailureDetails`/`requireExecSuccess` in src/utils/exec.ts @@ -60,6 +73,8 @@ export type AppErrorDetails = Record & { hint?: string; diagnosticId?: string; logPath?: string; + logPathUnavailable?: string; + diagnosticsRecord?: DiagnosticsRecordRef; retriable?: boolean; supportedOn?: string; processExitError?: boolean; @@ -75,7 +90,22 @@ export type NormalizedError = { message: string; hint?: string; diagnosticId?: string; + /** + * Diagnostics record path **the reader of this error can open**. A daemon + * renders its own host path here; a client talking to a REMOTE daemon + * replaces it with the caller-local copy it fetched, or drops it and sets + * `logPathUnavailable` (see `localizeRemoteDaemonError`). A path the reader + * cannot open never belongs in this field (#1801). + */ logPath?: string; + /** + * Why no readable `logPath` could be produced, e.g. + * `remote daemon https://host, request 8f2c: 404`. Set only in place of + * `logPath`, and never carries a daemon-host path. + */ + logPathUnavailable?: string; + /** Locator the record can be fetched by when it lives on a remote daemon. */ + diagnosticsRecord?: DiagnosticsRecordRef; /** * Lifted from `details.retriable` when a throw site classified the failure as * clearly transient (or clearly not). Included only when set, so the default @@ -96,7 +126,16 @@ export type DaemonError = { message: string; hint?: string; diagnosticId?: string; + /** Path on the DAEMON host. Meaningful to a local caller only (#1801). */ logPath?: string; + /** Why no readable path is named; set by the client, never by the daemon. */ + logPathUnavailable?: string; + /** + * Additive locator (#1801) for the request diagnostics record `logPath` + * names, so a remote caller can fetch it over the daemon API instead of + * being handed a path on a filesystem it cannot read. + */ + diagnosticsRecord?: DiagnosticsRecordRef; details?: Record; /** Additive retry and platform-support signals; absent when not derivable. */ retriable?: boolean; @@ -123,6 +162,8 @@ export function throwDaemonError(error: DaemonError): never { hint: error.hint, diagnosticId: error.diagnosticId, logPath: error.logPath, + logPathUnavailable: error.logPathUnavailable, + diagnosticsRecord: error.diagnosticsRecord, retriable: error.retriable, supportedOn: error.supportedOn, }); @@ -140,21 +181,27 @@ export function isAgentDeviceError(err: unknown): err is AppError { return err instanceof AppError; } +export type NormalizeErrorContext = { + diagnosticId?: string; + logPath?: string; + diagnosticsRecord?: DiagnosticsRecordRef; +}; + export function normalizeAgentDeviceError( err: unknown, - context: { diagnosticId?: string; logPath?: string } = {}, + context: NormalizeErrorContext = {}, ): NormalizedError { return normalizeError(err, context); } -export function normalizeError( - err: unknown, - context: { diagnosticId?: string; logPath?: string } = {}, -): NormalizedError { +export function normalizeError(err: unknown, context: NormalizeErrorContext = {}): NormalizedError { const appErr = asAppError(err); const details = appErr.details ? redactDiagnosticData(appErr.details) : undefined; const diagnosticId = stringDetail(details, 'diagnosticId') ?? context.diagnosticId; const logPath = stringDetail(details, 'logPath') ?? context.logPath; + const logPathUnavailable = stringDetail(details, 'logPathUnavailable'); + const diagnosticsRecord = + readDiagnosticsRecordRef(details?.diagnosticsRecord) ?? context.diagnosticsRecord; const hint = stringDetail(details, 'hint') ?? defaultHintForCode(appErr.code); const retriable = booleanDetail(details, 'retriable') ?? retriableForErrorCode(appErr.code); const supportedOn = stringDetail(details, 'supportedOn'); @@ -167,6 +214,8 @@ export function normalizeError( hint, diagnosticId, logPath, + ...(logPathUnavailable !== undefined ? { logPathUnavailable } : {}), + ...(diagnosticsRecord !== undefined ? { diagnosticsRecord } : {}), // Typed-error signals stay absent unless confidently known (#939 wire shape). ...(retriable !== undefined ? { retriable } : {}), ...(supportedOn !== undefined ? { supportedOn } : {}), @@ -226,6 +275,19 @@ function stringDetail( return typeof value === 'string' ? value : undefined; } +/** + * Narrows an untrusted `diagnosticsRecord` — off the wire or out of a details + * bag — to the locator type, or `undefined`. One reader, so a daemon payload + * and a details bag can never be accepted on different terms. + */ +export function readDiagnosticsRecordRef(value: unknown): DiagnosticsRecordRef | undefined { + if (!value || typeof value !== 'object') return undefined; + const { session, requestId } = value as Partial; + if (typeof session !== 'string' || typeof requestId !== 'string') return undefined; + if (session.length === 0 || requestId.length === 0) return undefined; + return { session, requestId }; +} + function booleanDetail( details: Record | undefined, key: string, @@ -242,6 +304,8 @@ function stripDiagnosticMeta( delete output.hint; delete output.diagnosticId; delete output.logPath; + delete output.logPathUnavailable; + delete output.diagnosticsRecord; delete output.retriable; delete output.supportedOn; return Object.keys(output).length > 0 ? output : undefined; diff --git a/src/__tests__/cli-remote-diagnostics.test.ts b/src/__tests__/cli-remote-diagnostics.test.ts new file mode 100644 index 0000000000..0d18048d14 --- /dev/null +++ b/src/__tests__/cli-remote-diagnostics.test.ts @@ -0,0 +1,180 @@ +/** + * #1801: a caller driving a REMOTE daemon must never be handed a diagnostics + * path on the daemon's filesystem. These run the real CLI against a real daemon + * HTTP server on loopback — the only place the whole chain (daemon error → + * locator → fetch → rendered line) is observable end to end. + */ + +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { normalizeError, AppError } from '@agent-device/kernel/errors'; +import { createDaemonHttpServer } from '../daemon/server/http-server.ts'; +import { resolveSessionRequestLogPath } from '../daemon/session-store.ts'; +import { safeSessionName } from '../daemon/session-paths.ts'; +import type { DaemonRequest, DaemonResponse } from '../daemon/types.ts'; +import { runCliCapture, type CapturedCliRun } from './cli-capture.ts'; +import { + closeLoopbackServer, + listenOnLoopback, + skipWhenLoopbackUnavailable, +} from './test-utils/index.ts'; +import { mkdtempForTestSync } from './test-utils/tmp-dir.ts'; + +const DAEMON_TOKEN = 'daemon-secret'; +const DAEMON_SESSION = 'cwd:abcdef0123456789:default'; +const RECORD_SENTINEL = 'REMOTE_RECORD_SENTINEL'; + +type RemoteRun = { + run: CapturedCliRun; + clientStateDir: string; + daemonLogPath: string; + requestId: string; +}; + +/** + * Runs `argv` against a loopback daemon that always fails the command and + * writes the request's diagnostics record, exactly as the real router does. + * `serveRecord: false` models a daemon with no diagnostics route (an older + * release), so the fetch fails. + */ +async function runAgainstRemoteDaemon( + argv: string[], + options: { serveRecord: boolean }, +): Promise { + const clientStateDir = mkdtempForTestSync('agent-device-remote-client-'); + const daemonStateDir = mkdtempForTestSync('agent-device-remote-daemon-'); + const daemonSessionsDir = path.join(daemonStateDir, 'sessions'); + const resolveRecordPath = (session: string, requestId: string | undefined): string => + resolveSessionRequestLogPath(path.join(daemonSessionsDir, safeSessionName(session)), requestId); + let requestId = ''; + let daemonLogPath = ''; + + const handleRequest = async (req: DaemonRequest): Promise => { + requestId = req.meta?.requestId ?? ''; + daemonLogPath = resolveRecordPath(DAEMON_SESSION, requestId); + fs.mkdirSync(path.dirname(daemonLogPath), { recursive: true }); + fs.writeFileSync(daemonLogPath, `{"phase":"request_failed","data":"${RECORD_SENTINEL}"}\n`); + return { + ok: false, + error: normalizeError(new AppError('COMMAND_FAILED', 'wait timed out'), { + logPath: daemonLogPath, + diagnosticsRecord: { session: DAEMON_SESSION, requestId }, + }), + }; + }; + + const server = await createDaemonHttpServer({ + token: DAEMON_TOKEN, + handleRequest, + ...(options.serveRecord + ? { resolveRequestDiagnosticsPath: (ref) => resolveRecordPath(ref.session, ref.requestId) } + : {}), + }); + + try { + const port = await listenOnLoopback(server); + const run = await runCliCapture(argv, { + useRealDaemonClient: true, + env: { + AGENT_DEVICE_STATE_DIR: clientStateDir, + AGENT_DEVICE_DAEMON_BASE_URL: `http://127.0.0.1:${port}`, + AGENT_DEVICE_DAEMON_AUTH_TOKEN: DAEMON_TOKEN, + }, + }); + return { run, clientStateDir, daemonLogPath, requestId }; + } finally { + await closeLoopbackServer(server); + fs.rmSync(daemonStateDir, { recursive: true, force: true }); + } +} + +test('a remote failure names a record the caller can actually read', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + + const { run, clientStateDir, daemonLogPath, requestId } = await runAgainstRemoteDaemon( + ['clipboard', 'write', 'hello'], + { serveRecord: true }, + ); + try { + const localPath = path.join( + clientStateDir, + 'remote-diagnostics', + safeSessionName(DAEMON_SESSION), + 'requests', + `${requestId}.ndjson`, + ); + assert.equal(run.code, 1); + assert.match(run.stderr, new RegExp(`Diagnostics Log: ${escapeRegExp(localPath)}`)); + assert.equal(run.stderr.includes(daemonLogPath), false, 'daemon-host path must not be printed'); + assert.match(fs.readFileSync(localPath, 'utf8'), new RegExp(RECORD_SENTINEL)); + } finally { + fs.rmSync(clientStateDir, { recursive: true, force: true }); + } +}); + +test('--json carries the caller-local record path, never the daemon-host one', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + + const { run, clientStateDir, daemonLogPath } = await runAgainstRemoteDaemon( + ['clipboard', 'write', 'hello', '--json'], + { serveRecord: true }, + ); + try { + const payload = JSON.parse(run.stdout) as { error: { logPath?: string } }; + assert.equal(payload.error.logPath?.startsWith(clientStateDir), true); + assert.equal(fs.existsSync(payload.error.logPath ?? ''), true); + assert.equal(run.stdout.includes(daemonLogPath), false); + } finally { + fs.rmSync(clientStateDir, { recursive: true, force: true }); + } +}); + +test('--debug prints the fetched record inline and not the local daemon log', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + + const { run, clientStateDir } = await runAgainstRemoteDaemon( + ['clipboard', 'write', 'hello', '--debug'], + { serveRecord: true }, + ); + try { + // A local daemon log in the caller's state dir must stay unread: it belongs + // to a different daemon than the one that failed. + assert.equal(run.stderr.includes('[daemon log]'), false); + assert.match(run.stderr, /\[remote diagnostics\]/); + assert.match(run.stderr, new RegExp(RECORD_SENTINEL)); + } finally { + fs.rmSync(clientStateDir, { recursive: true, force: true }); + } +}); + +test('an unfetchable remote record says so instead of naming a path', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + + const { run, clientStateDir, daemonLogPath, requestId } = await runAgainstRemoteDaemon( + ['clipboard', 'write', 'hello', '--json'], + { serveRecord: false }, + ); + try { + const payload = JSON.parse(run.stdout) as { + error: { logPath?: string; logPathUnavailable?: string }; + }; + // A path may still be named — the CLI's own client-side record, written on + // this machine — but never the daemon host's, and never a record the fetch + // failed to produce. + assert.notEqual(payload.error.logPath, daemonLogPath); + assert.equal(payload.error.logPath?.includes('remote-diagnostics') ?? false, false); + assert.match( + payload.error.logPathUnavailable ?? '', + new RegExp(`^remote daemon http://127\\.0\\.0\\.1:\\d+, request ${requestId}: HTTP 404$`), + ); + assert.equal(run.stdout.includes(daemonLogPath), false); + } finally { + fs.rmSync(clientStateDir, { recursive: true, force: true }); + } +}); + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} diff --git a/src/cli.ts b/src/cli.ts index b8952a6d75..dfa39cff55 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -5,7 +5,9 @@ import { AppError, normalizeError, throwDaemonError, + type NormalizedError, } from '@agent-device/kernel/errors'; +import { resolveRemoteRequestDiagnosticsPath } from './daemon/session-store.ts'; import { printHumanError, printJson } from './utils/output.ts'; import { exitAfterFlush } from './utils/process-exit.ts'; import { readVersion } from './utils/version.ts'; @@ -202,7 +204,7 @@ async function parseCliInputOrExit( }); const normalized = normalizeError(error, { diagnosticId: getDiagnosticsMeta().diagnosticId, - logPath: flushDiagnosticsToSessionFile({ force: true }) ?? undefined, + logPath: flushDiagnosticsToSessionFile({ force: true })?.path, }); if (options.jsonRequested) { printJson({ success: false, error: normalized }); @@ -320,7 +322,7 @@ async function resolveRunContextOrExit( const appErr = asAppError(err); const normalized = normalizeError(appErr, { diagnosticId: getDiagnosticsMeta().diagnosticId, - logPath: flushDiagnosticsToSessionFile({ force: true }) ?? undefined, + logPath: flushDiagnosticsToSessionFile({ force: true })?.path, }); if (parsed.flags.json) { printJson({ success: false, error: normalized }); @@ -548,7 +550,7 @@ async function handleRunCliFailure( const appErr = asAppError(err); const normalized = normalizeError(appErr, { diagnosticId: getDiagnosticsMeta().diagnosticId, - logPath: flushDiagnosticsToSessionFile({ force: true }) ?? undefined, + logPath: flushDiagnosticsToSessionFile({ force: true })?.path, }); if (ctx.command === 'close' && isDaemonStartupFailure(appErr)) { if (ctx.effectiveFlags.json) { @@ -564,7 +566,7 @@ async function handleRunCliFailure( } else { printHumanError(normalized, { showDetails: ctx.debugOutputEnabled }); if (ctx.debugOutputEnabled) { - printDaemonLogTailOnError(ctx.daemonPaths.logPath); + printFailureLogTail(ctx, normalized); } } if (logTailStopper) logTailStopper(); @@ -577,7 +579,32 @@ async function handleRunCliFailure( const DAEMON_LOG_TAIL_MAX_BYTES = 64_000; -function printDaemonLogTailOnError(logPath: string): void { +/** + * The evidence `--debug` puts inline in the caller's own log after a failure. + * + * For a LOCAL daemon that is the daemon log this process can read. For a REMOTE + * one that file belongs to another machine (the same reason + * `maybeStartDaemonLogTail` does not follow it), so the tail comes from the + * request record fetched to this host instead — which is what makes a CI job's + * transcript carry the evidence without a second round trip (#1801). + */ +function printFailureLogTail(ctx: CliRunContext, normalized: NormalizedError): void { + if (!ctx.effectiveFlags.daemonBaseUrl) { + printLogFileTail('daemon log', ctx.daemonPaths.logPath); + return; + } + const record = normalized.diagnosticsRecord; + if (!record) return; + // Recomputed from the locator through the same helper that wrote the copy, so + // the tail can only ever come from the fetched record (absent when the fetch + // failed, since nothing was written). + printLogFileTail( + 'remote diagnostics', + resolveRemoteRequestDiagnosticsPath(ctx.daemonPaths.baseDir, record), + ); +} + +function printLogFileTail(label: string, logPath: string): void { try { if (fs.existsSync(logPath)) { const content = fs.readFileSync(logPath, 'utf8'); @@ -587,7 +614,7 @@ function printDaemonLogTailOnError(logPath: string): void { tail = tail.slice(tail.length - DAEMON_LOG_TAIL_MAX_BYTES); } if (tail.trim().length > 0) { - process.stderr.write(`\n[daemon log]\n${tail}\n`); + process.stderr.write(`\n[${label}]\n${tail}\n`); } } } catch {} diff --git a/src/cli/parser/cli-help.ts b/src/cli/parser/cli-help.ts index 808be759a4..2482ee4869 100644 --- a/src/cli/parser/cli-help.ts +++ b/src/cli/parser/cli-help.ts @@ -392,6 +392,7 @@ Diagnostics and traces: Open output includes Session state; JSON also includes runnerLogPath and requestLogPath. For open timing under --debug, run open --debug --json and inspect requestLogPath for the open_timing event. Session requests/.ndjson holds daemon request diagnostics; session runner.log holds Apple runner/xcodebuild output. + Against a remote daemon the Diagnostics Log path is always local: the failing request's record is fetched to /remote-diagnostics/, and if it cannot be fetched the line reads "unavailable" with the reason instead of a daemon-host path. daemon.log is global daemon lifecycle evidence, not the primary per-run log. Use trace for low-level session diagnostics around one repro: agent-device trace start ./traces/diagnostics.trace diff --git a/src/daemon/__tests__/request-diagnostics-http.test.ts b/src/daemon/__tests__/request-diagnostics-http.test.ts new file mode 100644 index 0000000000..5c1cfd8899 --- /dev/null +++ b/src/daemon/__tests__/request-diagnostics-http.test.ts @@ -0,0 +1,159 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import http from 'node:http'; +import path from 'node:path'; +import { createDaemonHttpServer } from '../server/http-server.ts'; +import { resolveSessionRequestLogPath } from '../session-store.ts'; +import { safeSessionName } from '../session-paths.ts'; +import type { DaemonResponse } from '../types.ts'; +import { + closeLoopbackServer, + listenOnLoopback, + skipWhenLoopbackUnavailable, +} from '../../__tests__/test-utils/index.ts'; +import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; + +const RECORD = '{"phase":"request_start"}\n{"phase":"request_failed"}\n'; + +function writeRecord(sessionsDir: string, session: string, requestId: string): string { + const recordPath = resolveSessionRequestLogPath( + path.join(sessionsDir, safeSessionName(session)), + requestId, + ); + fs.mkdirSync(path.dirname(recordPath), { recursive: true }); + fs.writeFileSync(recordPath, RECORD); + return recordPath; +} + +async function withDiagnosticsServer( + run: (context: { baseUrl: string; sessionsDir: string }) => Promise, +): Promise { + const stateDir = mkdtempForTestSync('agent-device-request-diagnostics-'); + const sessionsDir = path.join(stateDir, 'sessions'); + const server = await createDaemonHttpServer({ + token: 'daemon-secret', + handleRequest: async (): Promise => ({ ok: true, data: {} }), + resolveRequestDiagnosticsPath: (ref) => + resolveSessionRequestLogPath( + path.join(sessionsDir, safeSessionName(ref.session)), + ref.requestId, + ), + }); + try { + const port = await listenOnLoopback(server); + await run({ baseUrl: `http://127.0.0.1:${port}`, sessionsDir }); + } finally { + await closeLoopbackServer(server); + fs.rmSync(stateDir, { recursive: true, force: true }); + } +} + +async function requestRawStatus(baseUrl: string, requestTarget: string): Promise { + const url = new URL(baseUrl); + return await new Promise((resolve, reject) => { + const request = http.request( + { + host: url.hostname, + port: url.port, + method: 'GET', + path: requestTarget, + headers: { authorization: 'Bearer daemon-secret' }, + }, + (res) => { + res.resume(); + res.on('end', () => resolve(res.statusCode ?? 0)); + }, + ); + request.on('error', reject); + request.end(); + }); +} + +function diagnosticsUrl(baseUrl: string, session: string, requestId: string): string { + return `${baseUrl}/sessions/${encodeURIComponent(session)}/requests/${encodeURIComponent(requestId)}/diagnostics`; +} + +test('request diagnostics route serves one record as ndjson to an authorized caller', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + + await withDiagnosticsServer(async ({ baseUrl, sessionsDir }) => { + writeRecord(sessionsDir, 'cwd:9f1:default', 'abc123'); + const response = await fetch(diagnosticsUrl(baseUrl, 'cwd:9f1:default', 'abc123'), { + headers: { authorization: 'Bearer daemon-secret' }, + }); + assert.equal(response.status, 200); + assert.equal(response.headers.get('content-type'), 'application/x-ndjson'); + assert.equal(response.headers.get('content-length'), String(Buffer.byteLength(RECORD))); + assert.equal(await response.text(), RECORD); + }); +}); + +test('request diagnostics route refuses an unauthenticated caller', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + + await withDiagnosticsServer(async ({ baseUrl, sessionsDir }) => { + writeRecord(sessionsDir, 'default', 'abc123'); + const response = await fetch(diagnosticsUrl(baseUrl, 'default', 'abc123')); + assert.equal(response.status, 401); + assert.equal((await response.text()).includes('request_start'), false); + }); +}); + +test('request diagnostics route answers 404 for a record that was never written', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + + await withDiagnosticsServer(async ({ baseUrl, sessionsDir }) => { + writeRecord(sessionsDir, 'default', 'abc123'); + const auth = { authorization: 'Bearer daemon-secret' }; + const unknownRequest = await fetch(diagnosticsUrl(baseUrl, 'default', 'nope'), { + headers: auth, + }); + assert.equal(unknownRequest.status, 404); + const unknownSession = await fetch(diagnosticsUrl(baseUrl, 'other', 'abc123'), { + headers: auth, + }); + assert.equal(unknownSession.status, 404); + }); +}); + +test('request diagnostics route rejects ids that do not name one record', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + + await withDiagnosticsServer(async ({ baseUrl, sessionsDir }) => { + writeRecord(sessionsDir, 'default', 'abc123'); + // Sent over a raw request rather than `fetch`: URL parsing resolves `..` + // away client-side, so only a hand-built request target reaches the daemon + // with the segment a traversal attempt would actually carry. + for (const [session, requestId] of [ + ['..', 'abc123'], + ['default', '..'], + ['', 'abc123'], + ['default', ''], + ] as const) { + const status = await requestRawStatus( + baseUrl, + `/sessions/${session}/requests/${requestId}/diagnostics`, + ); + assert.equal(status, 400, `expected 400 for session=${session} request=${requestId}`); + } + }); +}); + +test('request diagnostics route keeps a tenant inside its own session namespace', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + + await withDiagnosticsServer(async ({ baseUrl, sessionsDir }) => { + writeRecord(sessionsDir, 'tenant-a:default', 'abc123'); + const auth = { authorization: 'Bearer daemon-secret' }; + const owner = await fetch(diagnosticsUrl(baseUrl, 'tenant-a:default', 'abc123'), { + headers: { ...auth, 'x-agent-device-tenant': 'tenant-a' }, + }); + assert.equal(owner.status, 200); + const otherTenant = await fetch(diagnosticsUrl(baseUrl, 'tenant-a:default', 'abc123'), { + headers: { ...auth, 'x-agent-device-tenant': 'tenant-b' }, + }); + assert.equal(otherTenant.status, 401); + assert.equal((await otherTenant.text()).includes('request_start'), false); + }); +}); diff --git a/src/daemon/__tests__/request-execution-scope.test.ts b/src/daemon/__tests__/request-execution-scope.test.ts index 2dfe85f2a5..bea1818b6f 100644 --- a/src/daemon/__tests__/request-execution-scope.test.ts +++ b/src/daemon/__tests__/request-execution-scope.test.ts @@ -129,7 +129,7 @@ test('request diagnostics flush into the effective session request log', async ( }); return { expectedPath: scope.requestLogPath, - flushedPath: flushDiagnosticsToSessionFile({ force: true }), + flushedPath: flushDiagnosticsToSessionFile({ force: true })?.path ?? null, }; }, ); @@ -587,7 +587,7 @@ test('tenant lease rejection flushes diagnostics into the effective session requ leaseRegistry: new LeaseRegistry(), }); await expect(scope.runLocked(async () => 'ran')).rejects.toThrow(/Lease is not active/); - flushedPath = flushDiagnosticsToSessionFile({ force: true }); + flushedPath = flushDiagnosticsToSessionFile({ force: true })?.path ?? null; }); const expectedPath = resolveSessionRequestLogPath( diff --git a/src/daemon/__tests__/runtime-session.test.ts b/src/daemon/__tests__/runtime-session.test.ts index 604a7610f8..c0ed07bacd 100644 --- a/src/daemon/__tests__/runtime-session.test.ts +++ b/src/daemon/__tests__/runtime-session.test.ts @@ -31,7 +31,7 @@ test('createDaemonRuntimeSessionStore hides non-matching sessions and scopes wri { session: 'qa-ios', command: 'snapshot' }, async () => { await store.set({ name: 'other', appBundleId: 'com.example.other' }); - return flushDiagnosticsToSessionFile({ force: true }); + return flushDiagnosticsToSessionFile({ force: true })?.path; }, ); diff --git a/src/daemon/client/daemon-client-rpc.ts b/src/daemon/client/daemon-client-rpc.ts index 7fd1766ac5..219e1cf11d 100644 --- a/src/daemon/client/daemon-client-rpc.ts +++ b/src/daemon/client/daemon-client-rpc.ts @@ -1,7 +1,13 @@ -import { AppError, toAppErrorCode } from '@agent-device/kernel/errors'; +import { + AppError, + readDiagnosticsRecordRef, + toAppErrorCode, + type DaemonError, +} from '@agent-device/kernel/errors'; import { createRequestId } from '../../utils/diagnostics.ts'; import type { DaemonRequest, DaemonResponse } from '../types.ts'; import { materializeRemoteArtifacts } from '../../remote/daemon-artifacts.ts'; +import { localizeRemoteDaemonError } from '../../remote/remote-request-diagnostics.ts'; import type { DaemonInfo } from './daemon-client-metadata.ts'; import { leaseScopeFromRequest, @@ -14,15 +20,16 @@ export function handleDaemonHttpResponseBody( options: { info: DaemonInfo; req: DaemonRequest; + stateDir: string; resolve: (response: DaemonResponse | PromiseLike) => void; reject: (error: unknown) => void; }, ): void { - const { info, req, resolve, reject } = options; + const { info, req, stateDir, resolve, reject } = options; try { const parsed = parseDaemonHttpResponseBody(body); if (parsed.error) { - reject(toDaemonHttpRpcError(parsed.error, req.meta?.requestId)); + void rejectDaemonHttpRpcError(parsed.error, { info, req, stateDir, reject }); return; } if (!parsed.result || typeof parsed.result !== 'object') { @@ -59,24 +66,69 @@ function parseDaemonHttpResponseBody(body: string): { }; } -function toDaemonHttpRpcError( +/** + * #1801: a REMOTE daemon's error names a `logPath` on ITS host, so the failure + * is rehydrated only after that record has been made readable here — the error + * the caller sees then names a caller-local copy, or no path at all. A local + * daemon shares the filesystem, so its error is rehydrated as-is. + */ +async function rejectDaemonHttpRpcError( error: { message?: string; data?: Record }, - requestId: string | undefined, -): AppError { + options: { + info: DaemonInfo; + req: DaemonRequest; + stateDir: string; + reject: (error: unknown) => void; + }, +): Promise { + const { info, req, stateDir, reject } = options; + const requestId = req.meta?.requestId; + const payload = toDaemonHttpRpcError(error); + if (!info.baseUrl) { + reject(appErrorFromDaemonError(payload, requestId)); + return; + } + const localized = await localizeRemoteDaemonError(payload, { + endpoint: { baseUrl: info.baseUrl, token: info.token, tenantId: req.meta?.tenantId }, + stateDir, + requestId, + }); + reject(appErrorFromDaemonError(localized, requestId)); +} + +function toDaemonHttpRpcError(error: { + message?: string; + data?: Record; +}): DaemonError { const data = error.data ?? {}; - return new AppError( - toAppErrorCode(data.code != null ? String(data.code) : undefined, 'COMMAND_FAILED'), - String(data.message ?? error.message ?? 'Daemon RPC request failed'), - { - ...(typeof data.details === 'object' && data.details ? data.details : {}), - hint: typeof data.hint === 'string' ? data.hint : undefined, - diagnosticId: typeof data.diagnosticId === 'string' ? data.diagnosticId : undefined, - logPath: typeof data.logPath === 'string' ? data.logPath : undefined, - retriable: typeof data.retriable === 'boolean' ? data.retriable : undefined, - supportedOn: typeof data.supportedOn === 'string' ? data.supportedOn : undefined, - requestId, - }, - ); + return { + code: toAppErrorCode(data.code != null ? String(data.code) : undefined, 'COMMAND_FAILED'), + message: String(data.message ?? error.message ?? 'Daemon RPC request failed'), + details: + typeof data.details === 'object' && data.details + ? (data.details as Record) + : undefined, + hint: typeof data.hint === 'string' ? data.hint : undefined, + diagnosticId: typeof data.diagnosticId === 'string' ? data.diagnosticId : undefined, + logPath: typeof data.logPath === 'string' ? data.logPath : undefined, + diagnosticsRecord: readDiagnosticsRecordRef(data.diagnosticsRecord), + retriable: typeof data.retriable === 'boolean' ? data.retriable : undefined, + supportedOn: typeof data.supportedOn === 'string' ? data.supportedOn : undefined, + }; +} + +function appErrorFromDaemonError(error: DaemonError, requestId: string | undefined): AppError { + return new AppError(toAppErrorCode(error.code, 'COMMAND_FAILED'), error.message, { + ...(error.details ?? {}), + hint: error.hint, + diagnosticId: error.diagnosticId, + logPath: error.logPath, + logPathUnavailable: error.logPathUnavailable, + diagnosticsRecord: error.diagnosticsRecord, + retriable: error.retriable, + supportedOn: error.supportedOn, + requestId, + }); } async function resolveDaemonHttpResult( diff --git a/src/daemon/client/daemon-client-transport.ts b/src/daemon/client/daemon-client-transport.ts index 6a73306c4b..3305fc552d 100644 --- a/src/daemon/client/daemon-client-transport.ts +++ b/src/daemon/client/daemon-client-transport.ts @@ -401,14 +401,26 @@ async function sendHttpRequest( if (timeoutHandle) clearTimeout(timeoutHandle); }, handleResponseBody: (body) => - handleDaemonHttpResponseBody(body, { info, req, resolve, reject }), + handleDaemonHttpResponseBody(body, { + info, + req, + stateDir: statePaths.baseDir, + resolve, + reject, + }), }); return; } void readNodeHttpResponseBody(res) .then((body) => { if (timeoutHandle) clearTimeout(timeoutHandle); - handleDaemonHttpResponseBody(body, { info, req, resolve, reject }); + handleDaemonHttpResponseBody(body, { + info, + req, + stateDir: statePaths.baseDir, + resolve, + reject, + }); }) .catch((err: unknown) => { if (timeoutHandle) clearTimeout(timeoutHandle); diff --git a/src/daemon/downloadable-artifact-http.ts b/src/daemon/downloadable-artifact-http.ts index 49e6b501c4..aedf0755cc 100644 --- a/src/daemon/downloadable-artifact-http.ts +++ b/src/daemon/downloadable-artifact-http.ts @@ -7,7 +7,8 @@ import { listDownloadableArtifacts, prepareDownloadableArtifact, } from './artifact-tracking.ts'; -import { sendRestJsonError, statusCodeForNormalizedError } from './http-errors.ts'; +import { failStreamedHttpResponse, sendRestJsonError } from './http-errors.ts'; +import { decodeUriSegment } from './http-request-target.ts'; type DownloadableArtifactHttpRoute = | { kind: 'inventory' } @@ -99,13 +100,7 @@ async function handleArtifactDownload( ); } stream.on('error', (error) => { - if (!res.headersSent) { - const normalized = normalizeError(error); - res.statusCode = statusCodeForNormalizedError(normalized.code); - res.end(normalized.message); - } else { - res.destroy(error as Error); - } + failStreamedHttpResponse(res, error); }); let didCleanupArtifact = false; const cleanupCompletedDownload = () => { @@ -161,11 +156,7 @@ async function handleArtifactInventory( function readArtifactId(pathname: string): string { const encoded = pathname.slice('/artifacts/'.length); if (!encoded || encoded.includes('/')) return ''; - try { - return decodeURIComponent(encoded); - } catch { - return ''; - } + return decodeUriSegment(encoded); } function readRequestPathname(requestUrl: string | undefined): string { diff --git a/src/daemon/http-errors.ts b/src/daemon/http-errors.ts index eba4c469a7..51bddf6438 100644 --- a/src/daemon/http-errors.ts +++ b/src/daemon/http-errors.ts @@ -24,3 +24,19 @@ export function sendRestJsonError(res: http.ServerResponse, normalized: Normaliz res.setHeader('content-type', 'application/json'); res.end(JSON.stringify({ ok: false, error: normalized.message, code: normalized.code })); } + +/** + * A file stream that fails mid-download: still an error status when nothing has + * been written, but once headers are out the only honest signal left is + * destroying the connection, so the client sees a truncated body rather than a + * complete one. + */ +export function failStreamedHttpResponse(res: http.ServerResponse, error: Error): void { + if (res.headersSent) { + res.destroy(error); + return; + } + const normalized = normalizeError(error); + res.statusCode = statusCodeForNormalizedError(normalized.code); + res.end(normalized.message); +} diff --git a/src/daemon/http-request-target.ts b/src/daemon/http-request-target.ts new file mode 100644 index 0000000000..dd3962ca9d --- /dev/null +++ b/src/daemon/http-request-target.ts @@ -0,0 +1,17 @@ +/** + * Decoding one segment of an HTTP request target, shared by the daemon's + * auxiliary routes (`/artifacts/*`, the request diagnostics route). + * + * A route that decoded ids its own way would disagree with its siblings about + * what a request addresses, and an id that fails to decode must resolve to a + * name no record can have rather than throwing inside the dispatcher. + */ + +/** The decoded segment, or `''` when it is not decodable — never a throw. */ +export function decodeUriSegment(segment: string): string { + try { + return decodeURIComponent(segment); + } catch { + return ''; + } +} diff --git a/src/daemon/request-admission.ts b/src/daemon/request-admission.ts index b9cb43477b..a4f302173e 100644 --- a/src/daemon/request-admission.ts +++ b/src/daemon/request-admission.ts @@ -1,5 +1,6 @@ import { AppError } from '@agent-device/kernel/errors'; import { normalizeTenantId, resolveSessionIsolationMode } from './config.ts'; +import { isTenantOwnedSessionName, tenantScopedSessionName } from './session-tenant-scope.ts'; import { isLeaseAdmissionExempt } from './daemon-command-registry.ts'; import { DEFAULT_PROXY_LEASE_TTL_MS, @@ -36,7 +37,7 @@ export function scopeRequestSession(req: DaemonRequest): DaemonRequest { ); } const requestedSession = req.session || 'default'; - if (requestedSession.startsWith(`${tenant}:`)) { + if (isTenantOwnedSessionName(tenant, requestedSession)) { return { ...req, meta: { @@ -48,7 +49,7 @@ export function scopeRequestSession(req: DaemonRequest): DaemonRequest { } return { ...req, - session: `${tenant}:${requestedSession}`, + session: tenantScopedSessionName(tenant, requestedSession), meta: { ...req.meta, tenantId: tenant, diff --git a/src/daemon/request-diagnostics-http.ts b/src/daemon/request-diagnostics-http.ts new file mode 100644 index 0000000000..2282ccca71 --- /dev/null +++ b/src/daemon/request-diagnostics-http.ts @@ -0,0 +1,169 @@ +/** + * `GET /sessions//requests//diagnostics` — the one request's + * diagnostics record, streamed as ndjson (#1801). + * + * A failed command names its diagnostics record by path, and that path is on the + * DAEMON host. A remote caller — a CI runner driving an EAS/limrun simulator — + * cannot read that filesystem, so the record has to be reachable over the same + * base URL and token the caller already holds. The client side is + * `src/remote/remote-request-diagnostics.ts`. + * + * Scope: exactly one record, addressed the way the error names it + * (`DaemonError.diagnosticsRecord`). A record that was never written (the + * request never reached a session scope, or the daemon has since been reset) + * answers 404 — the route deliberately does not enumerate what exists. + */ + +import fs from 'node:fs'; +import http from 'node:http'; +import { AppError, normalizeError, type DiagnosticsRecordRef } from '@agent-device/kernel/errors'; +import type { DaemonRequest } from './types.ts'; +import { isSafeSessionSegment } from './session-paths.ts'; +import { decodeUriSegment } from './http-request-target.ts'; +import { isTenantOwnedSessionName } from './session-tenant-scope.ts'; +import { failStreamedHttpResponse, sendRestJsonError } from './http-errors.ts'; + +const REQUEST_DIAGNOSTICS_CONTENT_TYPE = 'application/x-ndjson'; + +/** + * `/sessions//requests//diagnostics` as data: the literal + * segments a request must match, with `null` where the caller supplies a name. + * The leading `''` is the empty segment before the first slash. + */ +const REQUEST_DIAGNOSTICS_ROUTE_SHAPE = [ + '', + 'sessions', + null, + 'requests', + null, + 'diagnostics', +] as const; +const SESSION_SEGMENT_INDEX = 2; +const REQUEST_ID_SEGMENT_INDEX = 4; + +type RequestDiagnosticsHttpAuthorizer = (params: { + req: http.IncomingMessage; + res: http.ServerResponse; + daemonRequest: Pick; +}) => Promise<{ tenantId?: string } | null>; + +export type RequestDiagnosticsHttpOptions = { + req: http.IncomingMessage; + res: http.ServerResponse; + authorize: RequestDiagnosticsHttpAuthorizer; + /** + * Where the daemon keeps the record for `ref`. Supplied by the runtime that + * owns the session store, so this module never builds a session artifact path + * of its own (AGENTS.md, "Session artifact paths come from session-store"). + */ + resolveRecordPath: (ref: DiagnosticsRecordRef) => string; +}; + +export function tryHandleRequestDiagnosticsHttpRoute( + options: RequestDiagnosticsHttpOptions, +): boolean { + const ref = resolveRequestDiagnosticsHttpRoute(options.req); + if (ref === null) return false; + void handleRequestDiagnostics(ref, options); + return true; +} + +/** + * The record locator this request addresses, or `null` when the request is not + * for this route at all. A malformed locator inside the route's own shape + * resolves to a ref that `handleRequestDiagnostics` rejects, so an unusable + * name answers 400 rather than falling through to the RPC dispatcher's 404. + */ +function resolveRequestDiagnosticsHttpRoute( + req: http.IncomingMessage, +): DiagnosticsRecordRef | null { + if (req.method !== 'GET') return null; + const segments = (req.url ?? '').split('?', 1)[0]!.split('/'); + if (segments.length !== REQUEST_DIAGNOSTICS_ROUTE_SHAPE.length) return null; + const shapeMatches = REQUEST_DIAGNOSTICS_ROUTE_SHAPE.every( + (literal, index) => literal === null || segments[index] === literal, + ); + if (!shapeMatches) return null; + return { + session: decodeUriSegment(segments[SESSION_SEGMENT_INDEX] ?? ''), + requestId: decodeUriSegment(segments[REQUEST_ID_SEGMENT_INDEX] ?? ''), + }; +} + +async function handleRequestDiagnostics( + ref: DiagnosticsRecordRef, + options: RequestDiagnosticsHttpOptions, +): Promise { + const { req, res, authorize, resolveRecordPath } = options; + try { + if (!isSafeSessionSegment(ref.session) || !isSafeSessionSegment(ref.requestId)) { + sendRestJsonError( + res, + normalizeError( + new AppError('INVALID_ARGS', 'Invalid session or request id in diagnostics route'), + ), + ); + return; + } + + const auth = await authorize({ + req, + res, + daemonRequest: { + command: 'request_diagnostics', + positionals: [ref.session, ref.requestId], + }, + }); + if (!auth) return; + + if (auth.tenantId && !isTenantOwnedSessionName(auth.tenantId, ref.session)) { + sendRestJsonError( + res, + normalizeError( + new AppError('UNAUTHORIZED', 'Session is outside this tenant', { + session: ref.session, + }), + ), + ); + return; + } + + const recordPath = resolveRecordPath(ref); + const stats = await statRecord(recordPath); + if (!stats) { + sendRestJsonError( + res, + normalizeError( + new AppError( + 'SESSION_NOT_FOUND', + `No diagnostics record for request ${ref.requestId} in session ${ref.session}`, + { + hint: 'The daemon keeps a request record only for the session it ran in; re-run the command against this daemon to produce a fresh one.', + }, + ), + ), + ); + return; + } + + const stream = fs.createReadStream(recordPath); + res.statusCode = 200; + res.setHeader('content-type', REQUEST_DIAGNOSTICS_CONTENT_TYPE); + res.setHeader('content-length', String(stats.size)); + stream.on('error', (error) => { + failStreamedHttpResponse(res, error); + }); + stream.pipe(res); + } catch (error) { + sendRestJsonError(res, normalizeError(error)); + } +} + +async function statRecord(recordPath: string): Promise { + try { + const stats = await fs.promises.stat(recordPath); + return stats.isFile() ? stats : null; + } catch { + return null; + } +} diff --git a/src/daemon/request-execution-scope.ts b/src/daemon/request-execution-scope.ts index 543b731015..d755e39155 100644 --- a/src/daemon/request-execution-scope.ts +++ b/src/daemon/request-execution-scope.ts @@ -34,7 +34,7 @@ import { } from './session-event-log.ts'; import type { LeaseRegistry } from './lease-registry.ts'; import { - resolveSessionRequestLogPath, + resolveSessionRequestLog, resolveSessionRunnerLogPath, type SessionStore, } from './session-store.ts'; @@ -119,14 +119,17 @@ export async function createRequestExecutionScope(params: { const sessionName = resolveEffectiveSessionName(scopedReq, sessionStore); const diagnosticsMeta = getDiagnosticsMeta(); const sessionDir = sessionStore.resolveSessionDir(sessionName); - const requestLogPath = resolveSessionRequestLogPath( + const requestLog = resolveSessionRequestLog({ sessionDir, - scopedReq.meta?.requestId ?? diagnosticsMeta.requestId, - ); + session: sessionName, + requestId: scopedReq.meta?.requestId ?? diagnosticsMeta.requestId, + }); + const requestLogPath = requestLog.path; const runnerLogPath = resolveSessionRunnerLogPath(sessionDir); updateDiagnosticsScope({ session: sessionName, - logPath: requestLogPath, + logPath: requestLog.path, + logRecord: requestLog.ref, }); emitDiagnostic({ level: 'info', diff --git a/src/daemon/request-finalization.ts b/src/daemon/request-finalization.ts index 6ca62bcb2e..962d6b904c 100644 --- a/src/daemon/request-finalization.ts +++ b/src/daemon/request-finalization.ts @@ -28,7 +28,7 @@ export function finalizeDaemonResponse( message: response.error.message, }, }); - const logPathOnFailure = flushDiagnosticsToSessionFile({ force: true }) ?? undefined; + const flushed = flushDiagnosticsToSessionFile({ force: true }); // ADR 0012 decision 6, BLOCKER 2 (second follow-up): every handler-RETURNED // (as opposed to thrown) failure response is rebuilt here into a fresh // AppError before re-normalizing — this used to copy `hint`/`diagnosticId`/ @@ -49,6 +49,7 @@ export function finalizeDaemonResponse( : undefined), diagnosticId: response.error.diagnosticId, logPath: response.error.logPath, + diagnosticsRecord: response.error.diagnosticsRecord, retriable: response.error.retriable ?? (typeof response.error.details?.retriable === 'boolean' @@ -62,7 +63,8 @@ export function finalizeDaemonResponse( }), { diagnosticId: details.diagnosticId, - logPath: logPathOnFailure, + logPath: flushed?.path, + diagnosticsRecord: flushed?.ref, }, ); return { ok: false, error: normalizedError }; diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 70efce56a8..385732cf92 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -471,10 +471,11 @@ function finalizeThrownRequestError(error: unknown): DaemonResponse { }, }); const details = getDiagnosticsMeta(); - const logPathOnFailure = flushDiagnosticsToSessionFile({ force: true }) ?? undefined; + const flushed = flushDiagnosticsToSessionFile({ force: true }); const normalizedError = normalizeError(error, { diagnosticId: details.diagnosticId, - logPath: logPathOnFailure, + logPath: flushed?.path, + diagnosticsRecord: flushed?.ref, }); return { ok: false, error: normalizedError }; } diff --git a/src/daemon/request-save-script-policy.ts b/src/daemon/request-save-script-policy.ts index 8c867db257..4f04cf2d77 100644 --- a/src/daemon/request-save-script-policy.ts +++ b/src/daemon/request-save-script-policy.ts @@ -52,14 +52,14 @@ export function unsupportedSaveScriptFlagResponse(req: DaemonRequest): DaemonRes // ADR 0010 decision 6: a failed request always carries its diagnosticId + // ndjson logPath, so this rejection is as traceable as a thrown one. const meta = getDiagnosticsMeta(); - const logPath = flushDiagnosticsToSessionFile({ force: true }) ?? undefined; + const flushed = flushDiagnosticsToSessionFile({ force: true }); return { ok: false, error: normalizeError( new AppError('INVALID_ARGS', UNSUPPORTED_SAVE_SCRIPT_MESSAGE, { hint: UNSUPPORTED_SAVE_SCRIPT_HINT, }), - { diagnosticId: meta.diagnosticId, logPath }, + { diagnosticId: meta.diagnosticId, logPath: flushed?.path, diagnosticsRecord: flushed?.ref }, ), }; } diff --git a/src/daemon/server/daemon-runtime.ts b/src/daemon/server/daemon-runtime.ts index ac4a6e387d..f1fc7a3959 100644 --- a/src/daemon/server/daemon-runtime.ts +++ b/src/daemon/server/daemon-runtime.ts @@ -1,6 +1,6 @@ import crypto from 'node:crypto'; import { asAppError, AppError } from '@agent-device/kernel/errors'; -import { SessionStore } from '../session-store.ts'; +import { resolveSessionRequestLogPath, SessionStore } from '../session-store.ts'; import { resolveDaemonPaths, resolveDaemonServerMode } from '../config.ts'; import { createDaemonHttpServer } from './http-server.ts'; import { trackDownloadableArtifact } from '../artifact-tracking.ts'; @@ -399,6 +399,10 @@ export async function startDaemonRuntime( handleRequest, token, retainArtifacts, + // #1801: the same record `DaemonError.logPath` names, addressed by its + // locator so a remote caller can fetch what it cannot read by path. + resolveRequestDiagnosticsPath: (ref) => + resolveSessionRequestLogPath(sessionStore.resolveSessionDir(ref.session), ref.requestId), }); servers.push(httpServer); httpPort = await listenHttpServer(httpServer); diff --git a/src/daemon/server/http-server.ts b/src/daemon/server/http-server.ts index 111855baae..cbac9a2056 100644 --- a/src/daemon/server/http-server.ts +++ b/src/daemon/server/http-server.ts @@ -1,6 +1,11 @@ import type { RequestProgressEvent } from '@agent-device/contracts/progress'; import http, { type IncomingHttpHeaders } from 'node:http'; -import { AppError, normalizeError, toAppErrorCode } from '@agent-device/kernel/errors'; +import { + AppError, + normalizeError, + toAppErrorCode, + type DiagnosticsRecordRef, +} from '@agent-device/kernel/errors'; import { emitDiagnostic } from '../../utils/diagnostics.ts'; import { timingSafeStringEqual } from '../../utils/timing-safe-equal.ts'; import type { @@ -31,6 +36,7 @@ import { DAEMON_HTTP_TENANT_HEADER } from '../http-contract.ts'; import { sendRestJsonError, statusCodeForNormalizedError } from '../http-errors.ts'; import { tryHandleUploadHttpRoute } from '../upload-http.ts'; import { tryHandleDownloadableArtifactHttpRoute } from '../downloadable-artifact-http.ts'; +import { tryHandleRequestDiagnosticsHttpRoute } from '../request-diagnostics-http.ts'; type JsonRpcRequest = JsonRpcRequestEnvelope; @@ -512,9 +518,16 @@ export async function createDaemonHttpServer(options: { handleRequest: DaemonInvokeFn; token?: string; retainArtifacts?: boolean; + /** + * Resolves a request diagnostics record path for the `/sessions/.../requests/...` + * route (#1801). Omitted by embedded servers with no session store; the route + * then does not exist and a remote caller is told the record is unavailable + * rather than handed a daemon-host path. + */ + resolveRequestDiagnosticsPath?: (ref: DiagnosticsRecordRef) => string; }): Promise { const authHook = await loadHttpAuthHook(); - const { handleRequest, token, retainArtifacts = false } = options; + const { handleRequest, token, retainArtifacts = false, resolveRequestDiagnosticsPath } = options; return http.createServer((req, res) => { if (req.method === 'GET' && req.url === '/health') { res.statusCode = 200; @@ -559,6 +572,25 @@ export async function createDaemonHttpServer(options: { return; } + if ( + resolveRequestDiagnosticsPath && + tryHandleRequestDiagnosticsHttpRoute({ + req, + res, + resolveRecordPath: resolveRequestDiagnosticsPath, + authorize: async (request) => + await authorizeAuxiliaryHttpRequest({ + req: request.req, + res: request.res, + authHook, + expectedToken: token, + daemonRequest: request.daemonRequest, + }), + }) + ) { + return; + } + if (req.method !== 'POST' || req.url !== '/rpc') { res.statusCode = 404; res.end('Not found'); diff --git a/src/daemon/session-store.ts b/src/daemon/session-store.ts index ec04bbfb5b..f6c8694f81 100644 --- a/src/daemon/session-store.ts +++ b/src/daemon/session-store.ts @@ -1,6 +1,6 @@ import path from 'node:path'; import fs from 'node:fs'; -import { AppError } from '@agent-device/kernel/errors'; +import { AppError, type DiagnosticsRecordRef } from '@agent-device/kernel/errors'; import { emitDiagnostic } from '../utils/diagnostics.ts'; import type { SessionRuntimeHints, SessionState } from './types.ts'; import { recordActionEntry, type RecordActionEntry } from './session-action-recorder.ts'; @@ -404,3 +404,37 @@ export function resolveSessionRequestLogPath( const safeRequestId = safeSessionName(requestId && requestId.length > 0 ? requestId : 'unknown'); return path.join(sessionDir, 'requests', `${safeRequestId}.ndjson`); } + +/** + * The request diagnostics record for one request: the path it is written to on + * this host, and the locator a remote caller fetches the same record by + * (#1801). Built in one call so the two can never name different records. + */ +export function resolveSessionRequestLog(params: { + sessionDir: string; + session: string; + requestId: string | undefined; +}): { path: string; ref: DiagnosticsRecordRef } { + return { + path: resolveSessionRequestLogPath(params.sessionDir, params.requestId), + ref: { + session: params.session, + requestId: params.requestId && params.requestId.length > 0 ? params.requestId : 'unknown', + }, + }; +} + +/** + * Where a CLIENT keeps its own copy of a remote daemon's request diagnostics + * record (#1801). Mirrors the daemon-side layout under the caller's state dir + * so a CI job can archive `remote-diagnostics/` wholesale. + */ +export function resolveRemoteRequestDiagnosticsPath( + stateDir: string, + ref: DiagnosticsRecordRef, +): string { + return resolveSessionRequestLogPath( + path.join(stateDir, 'remote-diagnostics', safeSessionName(ref.session)), + ref.requestId, + ); +} diff --git a/src/daemon/session-tenant-scope.ts b/src/daemon/session-tenant-scope.ts new file mode 100644 index 0000000000..33683407a9 --- /dev/null +++ b/src/daemon/session-tenant-scope.ts @@ -0,0 +1,17 @@ +/** + * The one rule that says which sessions belong to a tenant: under tenant + * isolation every session name lives beneath its own `:` prefix. + * + * It is written once because two sides depend on it agreeing — `scopeRequestSession` + * (`request-admission.ts`) names the session, and the request diagnostics route + * (`request-diagnostics-http.ts`, #1801) decides from the name alone whether a + * caller may read that session's record long after the session itself is gone. + */ + +export function tenantScopedSessionName(tenant: string, session: string): string { + return isTenantOwnedSessionName(tenant, session) ? session : `${tenant}:${session}`; +} + +export function isTenantOwnedSessionName(tenant: string, sessionName: string): boolean { + return sessionName.startsWith(`${tenant}:`); +} diff --git a/src/platforms/android/__tests__/device-input-state.test.ts b/src/platforms/android/__tests__/device-input-state.test.ts index a85d4f1f52..f187b80a31 100644 --- a/src/platforms/android/__tests__/device-input-state.test.ts +++ b/src/platforms/android/__tests__/device-input-state.test.ts @@ -119,7 +119,7 @@ test('getAndroidKeyboardState diagnoses fallback IME ownership classification', process.env.HOME = homeDir; const state = await withDiagnosticsScope({ session: 'keyboard-ime-fallback' }, async () => { const keyboardState = await getAndroidKeyboardState(device); - diagnosticsPath = flushDiagnosticsToSessionFile({ force: true }); + diagnosticsPath = flushDiagnosticsToSessionFile({ force: true })?.path ?? null; return keyboardState; }); diff --git a/src/platforms/android/__tests__/snapshot.test.ts b/src/platforms/android/__tests__/snapshot.test.ts index 1f34642d6a..c339128212 100644 --- a/src/platforms/android/__tests__/snapshot.test.ts +++ b/src/platforms/android/__tests__/snapshot.test.ts @@ -612,7 +612,7 @@ test('snapshotAndroid emits helper phase diagnostics', async () => { helperAdb, helperArtifact, }); - return flushDiagnosticsToSessionFile({ force: true }); + return flushDiagnosticsToSessionFile({ force: true })?.path ?? null; }, ); @@ -1077,7 +1077,7 @@ test('snapshotAndroid emits helper failure diagnostics', async () => { return true; }, ); - return flushDiagnosticsToSessionFile({ force: true }); + return flushDiagnosticsToSessionFile({ force: true })?.path ?? null; }, ); @@ -1101,7 +1101,7 @@ test('snapshotAndroid emits unavailable diagnostics when helper artifact is miss () => snapshotAndroid(device), /Android snapshot helper is unavailable/, ); - return flushDiagnosticsToSessionFile({ force: true }); + return flushDiagnosticsToSessionFile({ force: true })?.path ?? null; }, ); @@ -1205,7 +1205,7 @@ test('snapshotAndroid emits timeout diagnostics when helper capture times out', () => snapshotAndroidWithHelper(helperAdb), /Android snapshot helper failed: helper capture timed out/, ); - return flushDiagnosticsToSessionFile({ force: true }); + return flushDiagnosticsToSessionFile({ force: true })?.path ?? null; }, ); diff --git a/src/platforms/apple/core/__tests__/runner-client.test.ts b/src/platforms/apple/core/__tests__/runner-client.test.ts index c22152d1cf..618578f137 100644 --- a/src/platforms/apple/core/__tests__/runner-client.test.ts +++ b/src/platforms/apple/core/__tests__/runner-client.test.ts @@ -973,7 +973,7 @@ async function captureParseRunnerDiagnostics(callback: () => Promise): Pro { session: 'runner-parse-test', requestId: 'request-1', command: 'drag' }, async () => { await callback(); - const diagnosticsPath = flushDiagnosticsToSessionFile({ force: true }); + const diagnosticsPath = flushDiagnosticsToSessionFile({ force: true })?.path; assert.ok(diagnosticsPath); return fs.readFileSync(diagnosticsPath, 'utf8'); }, diff --git a/src/platforms/apple/core/__tests__/runner-session.test.ts b/src/platforms/apple/core/__tests__/runner-session.test.ts index 57ce978a51..920fc9b50a 100644 --- a/src/platforms/apple/core/__tests__/runner-session.test.ts +++ b/src/platforms/apple/core/__tests__/runner-session.test.ts @@ -2051,7 +2051,7 @@ async function captureDiagnostics(callback: () => Promise): Promise { await callback(); - const diagnosticsPath = flushDiagnosticsToSessionFile({ force: true }); + const diagnosticsPath = flushDiagnosticsToSessionFile({ force: true })?.path; assert.ok(diagnosticsPath); return fs.readFileSync(diagnosticsPath, 'utf8'); }, diff --git a/src/remote/__tests__/remote-request-diagnostics.test.ts b/src/remote/__tests__/remote-request-diagnostics.test.ts new file mode 100644 index 0000000000..18d756a0f9 --- /dev/null +++ b/src/remote/__tests__/remote-request-diagnostics.test.ts @@ -0,0 +1,150 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import http from 'node:http'; +import path from 'node:path'; +import { normalizeError, throwDaemonError, type DaemonError } from '@agent-device/kernel/errors'; +import { localizeRemoteDaemonError } from '../remote-request-diagnostics.ts'; +import { + closeLoopbackServer, + listenOnLoopback, + skipWhenLoopbackUnavailable, +} from '../../__tests__/test-utils/index.ts'; +import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; + +const DAEMON_LOG_PATH = '/Users/daemon-host/.agent-device/sessions/default/requests/abc123.ndjson'; +const RECORD = '{"phase":"request_failed","data":{"error":"no element"}}\n'; + +function remoteError(): DaemonError { + return { + code: 'COMMAND_FAILED', + message: 'wait timed out', + logPath: DAEMON_LOG_PATH, + diagnosticsRecord: { session: 'cwd:9f1:default', requestId: 'abc123' }, + }; +} + +/** Serves the record for any diagnostics request, or fails every request. */ +async function withRecordServer( + behavior: 'serve' | 'reject', + run: (baseUrl: string) => Promise, +): Promise { + const requestTargets: string[] = []; + const server = http.createServer((req, res) => { + requestTargets.push(req.url ?? ''); + if (behavior === 'reject') { + res.statusCode = 404; + res.end('Not found'); + return; + } + res.statusCode = 200; + res.setHeader('content-type', 'application/x-ndjson'); + res.end(RECORD); + }); + try { + const port = await listenOnLoopback(server); + await run(`http://127.0.0.1:${port}`); + } finally { + await closeLoopbackServer(server); + } + if (behavior === 'serve') { + assert.deepEqual(requestTargets, ['/sessions/cwd%3A9f1%3Adefault/requests/abc123/diagnostics']); + } +} + +test('a fetched remote record replaces the daemon-host path with a caller-local one', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + + const stateDir = mkdtempForTestSync('agent-device-remote-diagnostics-'); + try { + await withRecordServer('serve', async (baseUrl) => { + const localized = await localizeRemoteDaemonError(remoteError(), { + endpoint: { baseUrl, token: 'daemon-secret' }, + stateDir, + }); + const expectedPath = path.join( + stateDir, + 'remote-diagnostics', + 'cwd_9f1_default', + 'requests', + 'abc123.ndjson', + ); + assert.equal(localized.logPath, expectedPath); + assert.equal(fs.readFileSync(expectedPath, 'utf8'), RECORD); + assert.equal(localized.logPathUnavailable, undefined); + assert.equal( + JSON.stringify(localized).includes(DAEMON_LOG_PATH), + false, + 'nothing on the localized error may carry the daemon-host path', + ); + + // The rendered and JSON-rendered views must never name the daemon's path. + const normalized = normalizeError( + (() => { + try { + throwDaemonError(localized); + } catch (error) { + return error; + } + })(), + ); + assert.equal(normalized.logPath, expectedPath); + assert.notEqual(normalized.logPath, DAEMON_LOG_PATH); + }); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); + +test('an unreachable record leaves no path and names the daemon and request instead', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + + const stateDir = mkdtempForTestSync('agent-device-remote-diagnostics-'); + try { + await withRecordServer('reject', async (baseUrl) => { + const localized = await localizeRemoteDaemonError(remoteError(), { + endpoint: { baseUrl, token: 'daemon-secret' }, + stateDir, + }); + assert.equal(localized.logPath, undefined); + assert.equal( + localized.logPathUnavailable, + `remote daemon ${baseUrl}, request abc123: HTTP 404`, + ); + assert.equal(JSON.stringify(localized).includes(DAEMON_LOG_PATH), false); + assert.equal( + fs.existsSync(path.join(stateDir, 'remote-diagnostics', 'cwd_9f1_default')), + true, + 'the destination directory is created, but no partial record is left behind', + ); + assert.equal( + fs.existsSync( + path.join(stateDir, 'remote-diagnostics', 'cwd_9f1_default', 'requests', 'abc123.ndjson'), + ), + false, + ); + }); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); + +test('a daemon that names no record still never surfaces its own path', async () => { + const stateDir = mkdtempForTestSync('agent-device-remote-diagnostics-'); + try { + const { diagnosticsRecord: _ref, ...withoutRef } = remoteError(); + const localized = await localizeRemoteDaemonError(withoutRef, { + endpoint: { baseUrl: 'https://remote.example.test', token: 'daemon-secret' }, + stateDir, + requestId: 'abc123', + }); + assert.equal(localized.logPath, undefined); + assert.equal( + localized.logPathUnavailable, + 'remote daemon https://remote.example.test, request abc123: the daemon named no diagnostics record', + ); + assert.equal(JSON.stringify(localized).includes(DAEMON_LOG_PATH), false); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); diff --git a/src/remote/remote-request-diagnostics.ts b/src/remote/remote-request-diagnostics.ts new file mode 100644 index 0000000000..501956588e --- /dev/null +++ b/src/remote/remote-request-diagnostics.ts @@ -0,0 +1,217 @@ +/** + * Makes a remote daemon's request diagnostics record readable by the caller + * (#1801). + * + * A daemon renders `logPath` as a path on its own host. For a remote daemon — + * a CI runner driving an EAS/limrun simulator on someone else's macOS box — + * that path names a filesystem the caller cannot open, so an agent that follows + * it loses a turn and a CI job cannot keep the record as an artifact. + * + * The daemon-host path is therefore removed from the error at this boundary + * (`stripDaemonLogPath`, whose result type forbids carrying it further) and a + * caller-local `logPath` can then come only from the record this module + * actually fetched. When the fetch cannot happen the error names no path at all + * and says why instead. + * + * Bounded on purpose: one attempt, a short timeout, and a size cap. This runs + * on a request that already failed, so it must not turn one failure into two. + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import type { DaemonError, DiagnosticsRecordRef } from '@agent-device/kernel/errors'; +import { loadNodeHttpRequester } from '../utils/node-http.ts'; +import { + buildDaemonHttpAuthHeaders, + buildDaemonHttpTenantHeaders, + buildDaemonHttpUrl, +} from '../daemon/http-contract.ts'; +import { resolveRemoteRequestDiagnosticsPath } from '../daemon/session-store.ts'; + +const REMOTE_DIAGNOSTICS_FETCH_TIMEOUT_MS = 10_000; +const REMOTE_DIAGNOSTICS_MAX_BYTES = 8 * 1024 * 1024; + +export type RemoteDiagnosticsEndpoint = { + baseUrl: string; + token: string; + tenantId?: string; +}; + +/** + * An error payload as it arrived from a REMOTE daemon. `logPath` is typed away + * rather than merely deleted, so nothing downstream can put the daemon's path + * back on a value the caller will read. + */ +export type RemoteDaemonErrorPayload = Omit & { logPath?: never }; + +function buildRemoteRequestDiagnosticsUrl(baseUrl: string, ref: DiagnosticsRecordRef): string { + return buildDaemonHttpUrl( + baseUrl, + `sessions/${encodeURIComponent(ref.session)}/requests/${encodeURIComponent(ref.requestId)}/diagnostics`, + ); +} + +/** + * Drops the daemon-host path from a remote error — the only place that happens. + * It is dropped rather than kept under another key because `diagnosticsRecord` + * already identifies the same record precisely, without leaving a path shaped + * like something the caller could open. + */ +function stripDaemonLogPath(error: DaemonError): RemoteDaemonErrorPayload { + const { logPath: _daemonLogPath, ...rest } = error; + return rest; +} + +/** + * Rewrites a remote daemon's error so every path it names is one the caller can + * open: the record is fetched into `/remote-diagnostics/...` and + * `logPath` points there, or no `logPath` is named and `logPathUnavailable` + * says which daemon and request the missing record belongs to. + * + * Total by contract — it always answers with the daemon's error, never with a + * failure of its own. Every way of not getting the record is one of the + * `logPathUnavailable` reasons, so the request that already failed cannot fail + * a second time on the way to being reported. + */ +export async function localizeRemoteDaemonError( + error: DaemonError, + params: { endpoint: RemoteDiagnosticsEndpoint; stateDir: string; requestId?: string }, +): Promise { + const payload = stripDaemonLogPath(error); + const ref = error.diagnosticsRecord; + const requestId = ref?.requestId ?? params.requestId; + if (!ref) { + return withUnavailableRecord( + payload, + params.endpoint, + requestId, + // A daemon too old to carry the locator, or a failure that never reached + // a session scope, has no record this route could serve. + 'the daemon named no diagnostics record', + ); + } + + try { + const destinationPath = resolveRemoteRequestDiagnosticsPath(params.stateDir, ref); + const failure = await fetchRemoteRequestDiagnostics({ + endpoint: params.endpoint, + ref, + destinationPath, + }); + if (failure) { + return withUnavailableRecord(payload, params.endpoint, requestId, failure); + } + return { ...payload, logPath: destinationPath }; + } catch (localizationFailure) { + return withUnavailableRecord( + payload, + params.endpoint, + requestId, + localizationFailure instanceof Error + ? localizationFailure.message + : String(localizationFailure), + ); + } +} + +function withUnavailableRecord( + payload: RemoteDaemonErrorPayload, + endpoint: RemoteDiagnosticsEndpoint, + requestId: string | undefined, + reason: string, +): DaemonError { + return { + ...payload, + logPathUnavailable: `remote daemon ${endpoint.baseUrl}, request ${requestId ?? 'unknown'}: ${reason}`, + }; +} + +/** + * Downloads one record to `destinationPath`. Returns `undefined` on success, or + * the reason the caller has no record — never throws, because the request this + * belongs to already failed with its own error. + */ +async function fetchRemoteRequestDiagnostics(params: { + endpoint: RemoteDiagnosticsEndpoint; + ref: DiagnosticsRecordRef; + destinationPath: string; +}): Promise { + const { endpoint, ref, destinationPath } = params; + let url: URL; + try { + url = new URL(buildRemoteRequestDiagnosticsUrl(endpoint.baseUrl, ref)); + } catch { + return 'daemon base URL is not a valid URL'; + } + try { + const transport = await loadNodeHttpRequester(url.protocol); + await fs.promises.mkdir(path.dirname(destinationPath), { recursive: true }); + return await new Promise((resolve) => { + let settled = false; + const settle = (reason: string | undefined) => { + if (settled) return; + settled = true; + clearTimeout(timeoutHandle); + if (reason === undefined) { + resolve(undefined); + return; + } + void fs.promises.rm(destinationPath, { force: true }).finally(() => { + resolve(reason); + }); + }; + const request = transport.request( + { + protocol: url.protocol, + host: url.hostname, + port: url.port, + method: 'GET', + path: url.pathname + url.search, + headers: { + ...buildDaemonHttpAuthHeaders(endpoint.token), + ...buildDaemonHttpTenantHeaders(endpoint.tenantId), + }, + }, + (res) => { + const statusCode = res.statusCode ?? 500; + if (statusCode !== 200) { + res.resume(); + settle(`HTTP ${statusCode}`); + return; + } + const sink = fs.createWriteStream(destinationPath); + let bytes = 0; + res.on('data', (chunk: Buffer) => { + bytes += chunk.length; + if (bytes > REMOTE_DIAGNOSTICS_MAX_BYTES) { + request.destroy(); + sink.destroy(); + settle(`record exceeds ${REMOTE_DIAGNOSTICS_MAX_BYTES} bytes`); + } + }); + res.on('error', (error: Error) => { + sink.destroy(); + settle(error.message); + }); + sink.on('error', (error: Error) => { + settle(error.message); + }); + sink.on('finish', () => { + settle(undefined); + }); + res.pipe(sink); + }, + ); + const timeoutHandle = setTimeout(() => { + settle(`fetch timed out after ${REMOTE_DIAGNOSTICS_FETCH_TIMEOUT_MS}ms`); + request.destroy(); + }, REMOTE_DIAGNOSTICS_FETCH_TIMEOUT_MS); + request.on('error', (error: Error) => { + settle(error.message); + }); + request.end(); + }); + } catch (error) { + return error instanceof Error ? error.message : String(error); + } +} diff --git a/src/utils/__tests__/diagnostics.test.ts b/src/utils/__tests__/diagnostics.test.ts index c3576d6da5..c8ee586172 100644 --- a/src/utils/__tests__/diagnostics.test.ts +++ b/src/utils/__tests__/diagnostics.test.ts @@ -38,7 +38,7 @@ test('diagnostics redacts sensitive fields', async () => { safe: 'ok', }, }); - return flushDiagnosticsToSessionFile({ force: true }); + return flushDiagnosticsToSessionFile({ force: true })?.path; }, ); diff --git a/src/utils/__tests__/exec.test.ts b/src/utils/__tests__/exec.test.ts index 7fee263bdf..d4d89fef9b 100644 --- a/src/utils/__tests__/exec.test.ts +++ b/src/utils/__tests__/exec.test.ts @@ -69,7 +69,7 @@ test('runCmd emits exec_command diagnostics when the scope is debug-enabled', as }, async () => { await runCmd(process.execPath, ['-e', 'process.stdout.write("ok")']); - return flushDiagnosticsToSessionFile(); + return flushDiagnosticsToSessionFile()?.path ?? null; }, ); @@ -121,7 +121,7 @@ test.sequential('runCmdBackground emits bounded exec_command diagnostics when AG 'f', ]); await wait; - return flushDiagnosticsToSessionFile(); + return flushDiagnosticsToSessionFile()?.path ?? null; }, ), ); @@ -167,7 +167,7 @@ test.sequential('runCmd stays silent when exec tracing is not enabled', async () }, async () => { await runCmd(process.execPath, ['-e', 'process.stdout.write("ok")']); - return flushDiagnosticsToSessionFile(); + return flushDiagnosticsToSessionFile()?.path ?? null; }, ); diff --git a/src/utils/__tests__/retry.test.ts b/src/utils/__tests__/retry.test.ts index f9f268f8a0..d7b9196ee8 100644 --- a/src/utils/__tests__/retry.test.ts +++ b/src/utils/__tests__/retry.test.ts @@ -57,7 +57,7 @@ test('retryWithPolicy publishes retry diagnostics events', async () => { }, { maxAttempts: 2, baseDelayMs: 1, maxDelayMs: 1, jitter: 0 }, ); - return flushDiagnosticsToSessionFile({ force: true }); + return flushDiagnosticsToSessionFile({ force: true })?.path; }, ); assert.ok(outPath); diff --git a/src/utils/diagnostics.ts b/src/utils/diagnostics.ts index c8475f9b72..c0a21e018e 100644 --- a/src/utils/diagnostics.ts +++ b/src/utils/diagnostics.ts @@ -4,6 +4,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { redactDiagnosticData } from '@agent-device/kernel/redaction'; +import type { DiagnosticsRecordRef } from '@agent-device/kernel/errors'; type DiagnosticLevel = 'info' | 'warn' | 'error' | 'debug'; @@ -25,6 +26,13 @@ type DiagnosticsScopeOptions = { debug?: boolean; flushOnSuccess?: boolean; logPath?: string; + /** + * Set together with `logPath` whenever that path is a session request + * diagnostics record — the locator a remote caller fetches the same record + * by. Both come from one `resolveSessionRequestLog` result, so the path and + * the locator cannot name different records (#1801). + */ + logRecord?: DiagnosticsRecordRef; traceLogPath?: string; }; @@ -180,7 +188,20 @@ export async function withDiagnosticTimer( } } -export function flushDiagnosticsToSessionFile(options: { force?: boolean } = {}): string | null { +/** + * Where a flushed diagnostics record landed: a path on THIS host, plus the + * locator a remote caller can fetch the same record by when the record is a + * session request record. `ref` is absent for the homedir fallback file below, + * which no daemon route serves. + */ +export type FlushedDiagnosticsRecord = { + path: string; + ref?: DiagnosticsRecordRef; +}; + +export function flushDiagnosticsToSessionFile( + options: { force?: boolean } = {}, +): FlushedDiagnosticsRecord | null { const scope = diagnosticsStorage.getStore(); if (!scope) return null; if (!options.force && !scope.debug && !scope.flushOnSuccess) return null; @@ -194,9 +215,10 @@ export function flushDiagnosticsToSessionFile(options: { force?: boolean } = {}) appendDiagnosticLine(scope.logPath, `${lines.join('\n')}\n`); } const logPath = scope.logPath; + const logRecord = scope.logRecord; scope.events = []; scope.liveWrittenEventCount = 0; - return logPath; + return { path: logPath, ...(logRecord ? { ref: logRecord } : {}) }; } const sessionDir = sanitizePathPart(scope.session ?? 'default'); @@ -208,7 +230,7 @@ export function flushDiagnosticsToSessionFile(options: { force?: boolean } = {}) const lines = scope.events.map((entry) => JSON.stringify(redactScopeData(scope, entry))); fs.writeFileSync(filePath, `${lines.join('\n')}\n`); scope.events = []; - return filePath; + return { path: filePath }; } catch { return null; } diff --git a/src/utils/output.ts b/src/utils/output.ts index c8bd0b1699..0f6e93244f 100644 --- a/src/utils/output.ts +++ b/src/utils/output.ts @@ -50,9 +50,15 @@ export function printHumanError( if (normalized.diagnosticId) { process.stderr.write(`Diagnostic ID: ${normalized.diagnosticId}\n`); } + // #1801: `logPath` is always a path on THIS machine. A record that stayed on + // a remote daemon is reported as unavailable, with the reason, on its own + // line — never as a path the reader cannot open. if (normalized.logPath) { process.stderr.write(`Diagnostics Log: ${normalized.logPath}\n`); } + if (normalized.logPathUnavailable) { + process.stderr.write(`Remote Diagnostics: unavailable (${normalized.logPathUnavailable})\n`); + } // ADR 0012: the divergence compact report always renders; --debug's raw // details dump below remains the full-object view. const divergenceText = formatReplayDivergenceReport(normalized.details); diff --git a/test/integration/provider-scenarios/remote-daemon-client.test.ts b/test/integration/provider-scenarios/remote-daemon-client.test.ts index d91c42a53e..1b1ae2cb70 100644 --- a/test/integration/provider-scenarios/remote-daemon-client.test.ts +++ b/test/integration/provider-scenarios/remote-daemon-client.test.ts @@ -608,7 +608,14 @@ async function assertRemoteRpcErrorNormalization(client: RemoteClient): Promise< assert.equal(normalized.message, 'remote invalid args'); assert.equal(normalized.hint, 'remote hint'); assert.equal(normalized.diagnosticId, 'diag-remote'); - assert.equal(normalized.logPath, '/remote/log.txt'); + // #1801: `/remote/log.txt` is a path on the daemon host. It never reaches + // the caller; with no fetchable record the error says so instead. + assert.equal(normalized.logPath, undefined); + assert.match( + normalized.logPathUnavailable ?? '', + /^remote daemon http:\/\/127\.0\.0\.1:\d+, request \w+: the daemon named no diagnostics record$/, + ); + assert.equal(JSON.stringify(normalized).includes('/remote/log.txt'), false); assert.equal(normalized.details?.remote, true); assert.equal(typeof normalized.details?.requestId, 'string'); return true; diff --git a/test/wire-compat/ledger.json b/test/wire-compat/ledger.json index 8440c3cd93..afd2a0293e 100644 --- a/test/wire-compat/ledger.json +++ b/test/wire-compat/ledger.json @@ -31,31 +31,36 @@ "packages/kernel/src/contracts.ts#jsonRpcRequestSchema": "sha256:67e6b8a28b39a3883424a565ae7033c336dc2ea6b193e9b32bb98eb3e18dca7c", "packages/kernel/src/device.ts#PLATFORM_SELECTORS": "sha256:36e9da1cc660c0ddfb4cf52521f7f230341ff20e3ebda3405ba7c1799476c42d", "packages/kernel/src/device.ts#PlatformSelector": "sha256:61de3f003507ea2f53b396b0146c17670bf674a2db2132e19c462ca6c10cc8fd", - "packages/kernel/src/errors.ts#DaemonError": "sha256:398b23dfcdfb972119b7506dd789f7b78ad1504832203f98119524ec4b26a40b", - "packages/kernel/src/errors.ts#NormalizedError": "sha256:0d6be3fd0f72a5f74cdd0ed042a5a361c99606979c5b4ad9971a5aa72f688be9", - "packages/kernel/src/errors.ts#normalizeError": "sha256:a544353ea11d1c0e50c0eb08c11bcd7a9745642f22b299dc5d3bd42104b76404", + "packages/kernel/src/errors.ts#DaemonError": "sha256:af0f0f420277a57338dc59b7b443ec83d371e18768d064b7a265949e6435e976", + "packages/kernel/src/errors.ts#DiagnosticsRecordRef": "sha256:c5ef4185d01814fbfddcdcf797ce892f6ebfe1fd3c3fa73c7560ce2d3b6fce60", + "packages/kernel/src/errors.ts#NormalizeErrorContext": "sha256:98567378fc456335c8751474152edcf989e676ad0d9f4b54afdd8e6a96b03ae2", + "packages/kernel/src/errors.ts#NormalizedError": "sha256:ac2c600668420fe5b3d459f8e677676af83951fbad8ee1c2149092fd48e9d47c", + "packages/kernel/src/errors.ts#normalizeError": "sha256:f91f2fa4789075c838be9c42c04046b42d4ebb368eeb1b21194bdb000181e6ce", + "packages/kernel/src/errors.ts#readDiagnosticsRecordRef": "sha256:3a210f401221c0a15eb75d488450034885c7b73be39f5e93424c72874d14c395", "src/commands/cli-grammar/types.ts#DaemonCommandRequest": "sha256:ea3f4118244711f0eaec0e471f62b0de42ca97181c085df165d37b63301a4619", "src/core/lease-scope.ts#LeaseRpcCommand": "sha256:714f66efa725a5d0654e492a76daeb4059caad31b79562cfc9133029ea95bfe5", "src/daemon/client/daemon-client-progress.ts#ProgressResponseFormat": "sha256:6d7a99ac13fd422671fd86f8fe093efa40ca369f4ae1f97c98d40a7132543637", "src/daemon/client/daemon-client-progress.ts#createInvalidDaemonResponseError": "sha256:0af812346b667a23fdfef641b242cd86d07edaa05eb928c4c531628c09a4031b", "src/daemon/client/daemon-client-progress.ts#shouldReadDaemonProgressStream": "sha256:6811404f41d8db3fa6fc751332c7186a2399ec7ecbc4e9c547d9cb9f14dbb350", + "src/daemon/client/daemon-client-rpc.ts#appErrorFromDaemonError": "sha256:6188ea43d4da42ea1dd8a61ab1f3d4a6e287ec0a0210b65f850dc3030fdcd508", "src/daemon/client/daemon-client-rpc.ts#buildHttpRpcPayload": "sha256:c12b5945e89defe3ede7d699a5caa9f006e1bb2d47ce9adbc7e7dac5670ab5c0", "src/daemon/client/daemon-client-rpc.ts#buildLeaseRpcParams": "sha256:1755f46be8c62e7a8eb409e9a94d61781c96ccbef3306702121aa94264d63267", - "src/daemon/client/daemon-client-rpc.ts#handleDaemonHttpResponseBody": "sha256:9bb325c99ca8bda6a682eae9552f03913574ca2fbdffcc1000d430e65b6dc7df", + "src/daemon/client/daemon-client-rpc.ts#handleDaemonHttpResponseBody": "sha256:8f1c4bba1918545db6296ad29b95f0baa9f949e7c23bc04d59e261130f14611e", "src/daemon/client/daemon-client-rpc.ts#isLeaseRpcCommand": "sha256:955dc1b73462d40593e5728dbcbaeec1307944b08262a22627e84cc5dc507e8f", "src/daemon/client/daemon-client-rpc.ts#leaseRpcMethodForCommand": "sha256:3613555c6002c23ab5bec4b98bc1b4d6947014b52fa100176177f955cf762881", "src/daemon/client/daemon-client-rpc.ts#parseDaemonHttpResponseBody": "sha256:0cbe39dbd09e321ec00854a1626a7d89b6344e59c6faabe2fe02504268b2d886", + "src/daemon/client/daemon-client-rpc.ts#rejectDaemonHttpRpcError": "sha256:83b0312fe88bc3b3497de799e23d8d14455f665b7cf880f4617cd62b181351cd", "src/daemon/client/daemon-client-rpc.ts#resolveDaemonHttpResult": "sha256:296f9d376ce67c8cb20209bdf1c59c2423ffcfa35b5565a99e70271263149048", - "src/daemon/client/daemon-client-rpc.ts#toDaemonHttpRpcError": "sha256:32c27a34c2c72c74367bf865159ee2f0c1235589584408ac7c7d2de6d077c4c0", + "src/daemon/client/daemon-client-rpc.ts#toDaemonHttpRpcError": "sha256:ceb3286173ff05415ae71fe8dd974d9a2dfe4d33c9144ae219a3ee992b501e0e", "src/daemon/client/daemon-client-transport.ts#RemoteDaemonHealth": "sha256:38fe712390d247de59a74b9dc209fdf407ff5f9b1e32b698ae71ebc57569b66d", "src/daemon/client/daemon-client-transport.ts#readDaemonHttpHealth": "sha256:5e75af39e96045ce7faba986c900744cee76cdaa1e0df360ea38f3c020520711", "src/daemon/client/daemon-client-transport.ts#readHealthPayload": "sha256:5b15e14319b16aebf32d07336986a7f1f2a6b13cec1823ca9aa7c9f01bf1b0b6", "src/daemon/client/daemon-client-transport.ts#readRemoteDaemonHealth": "sha256:b22111f693ecb65705195a66bce617e58441483ebfd67cf264d6900ba1dcb401", "src/daemon/downloadable-artifact-http.ts#DownloadableArtifactHttpAuthorizer": "sha256:1b2702a929ca9170db2ca97c08e3ab67e17edb3ee75325a576c4c1b9cdbebb44", "src/daemon/downloadable-artifact-http.ts#DownloadableArtifactHttpRoute": "sha256:e63c4581ccde668913914149c9092d16ecf8a5cbd7ab33c6eb8617e77fc1015e", - "src/daemon/downloadable-artifact-http.ts#handleArtifactDownload": "sha256:2fe9778960869a80d99517c9bc6a18093b96ac4c9d7346e185dd1a7fecf06201", + "src/daemon/downloadable-artifact-http.ts#handleArtifactDownload": "sha256:7f96d17b7c605230fb3cc21ceaa5b2e515d7445653ff95b2b0f6f15fa6214d4e", "src/daemon/downloadable-artifact-http.ts#handleArtifactInventory": "sha256:24f071c082af80f2c50fb4f390bd8949f0f2c91e0bb58e67518b6cdcf6108aff", - "src/daemon/downloadable-artifact-http.ts#readArtifactId": "sha256:07f70f27471263f766de347dd1b757f9d296e86521d60f722ae786330e06a355", + "src/daemon/downloadable-artifact-http.ts#readArtifactId": "sha256:3472ee52d951fdf7ef00ee500298602c68f0e1e3b8c44ff51b09981efabf348c", "src/daemon/downloadable-artifact-http.ts#readRequestPathname": "sha256:6b3a49c72365cf7da4552abef59862c073461f746025e28d409048815dd586f2", "src/daemon/downloadable-artifact-http.ts#resolveDownloadableArtifactHttpRoute": "sha256:9fcdc9619e8d378e8f04ef7b3584863af446e855702ddce83a16ec194a6d98af", "src/daemon/http-contract.ts#DAEMON_HTTP_BASE_PATH": "sha256:a1ada25c6f90d9c69c8c836538e8882547bf239559f7c1accacd0654355802dd", @@ -65,10 +70,17 @@ "src/daemon/http-contract.ts#buildDaemonHttpTenantHeaders": "sha256:e38a5f0b5ab07ee3a5fe3db229d0de750888660c55bf31692e100002be96de1d", "src/daemon/http-contract.ts#buildDaemonHttpUrl": "sha256:d38f1f8877588fbcb1fea8c52a5cb7dc205043e3fc698b3d78fdcf821fa80471", "src/daemon/http-errors.ts#NormalizedHttpError": "sha256:2c2111802fe5f193acbeacc454de9cfab82bc8e4274b896f485e19a14f5518cf", + "src/daemon/http-errors.ts#failStreamedHttpResponse": "sha256:7199933eca7244bc801e0601b17cc1aed584bc2353879830fd0686e99f1200c4", "src/daemon/http-errors.ts#sendRestJsonError": "sha256:981abb859649419604d555601b5c4bd6616e78215327e055e5f8023c6c15f556", "src/daemon/http-errors.ts#statusCodeForNormalizedError": "sha256:20ad272162e28425920bd2dcb4da5ec104bd7e8be2028b902b05bfa4da3df966", "src/daemon/http-health.ts#DaemonHealthPayload": "sha256:050650184ad3e61d9359bf5866e7fc1e15e13f14f736a8c92ca57cad6d388967", "src/daemon/http-health.ts#buildDaemonHealthPayload": "sha256:5eebf35539d04e3693bb6d7081ffdfcb0d2d8b7d53066e30ca6b4017fa4a55a3", + "src/daemon/http-request-target.ts#decodeUriSegment": "sha256:b37622771ea4e61d1a9820169dbdc5809b5c38889bfad79277d7b22f3c3f1ef1", + "src/daemon/request-diagnostics-http.ts#REQUEST_DIAGNOSTICS_CONTENT_TYPE": "sha256:16bbd9ae7eaeaaa8c8cbd6e65c4ce786a6c5230c5351288228bbf1a6f11bdcdc", + "src/daemon/request-diagnostics-http.ts#RequestDiagnosticsHttpAuthorizer": "sha256:20761c39f434c675bb3405838dded5640bb623737957257e0452ebd46035eef1", + "src/daemon/request-diagnostics-http.ts#RequestDiagnosticsHttpOptions": "sha256:cd2c0dda77d2bc434857aea07206a5491ce5f0b82cb86628569f607d6c2cebcf", + "src/daemon/request-diagnostics-http.ts#handleRequestDiagnostics": "sha256:83d80f1386028ee56e8954949206dbb723d8411e5c13d25565d32ce3ee84dd9e", + "src/daemon/request-diagnostics-http.ts#resolveRequestDiagnosticsHttpRoute": "sha256:4ff07623b27a6f7461e1addb87aabd149904da63f7462149ae4114d14336cde0", "src/daemon/request-progress-protocol.ts#DaemonProgressEnvelope": "sha256:16162d01cfc43fc6a0198dbba3358981c1a278a70f7494ebe70d051f517cfb22", "src/daemon/request-progress-protocol.ts#DaemonResponseEnvelope": "sha256:202ae64836af890549a1d95963903f9a506294b80bd0862c1b708e72d7f286d0", "src/daemon/request-progress-protocol.ts#isDaemonProgressEnvelope": "sha256:e38ec26b64d20e257ff00860548e9c12ac22faf5a6308749d4715129814b381f", @@ -106,6 +118,7 @@ "src/daemon/server/http-server.ts#toReleaseMaterializedPathsDaemonRequest": "sha256:708736ca03d6a3fc449bb408382dbd509e490de1f84ecc8771fd77d4adda0154", "src/daemon/server/http-server.ts#writeProgressEnvelope": "sha256:ca661f6dd2de6e517c520dbd1e20b59b24b58d925157ed06123149ed2bc0fa3b", "src/daemon/server/http-server.ts#writeRpcResponseEnvelope": "sha256:7a8155e9fcbb53485489728250a85109b5dab0d96cdd4aa8b7e7eb87a4898ddb", + "src/daemon/session-tenant-scope.ts#isTenantOwnedSessionName": "sha256:c3cff883507d462548846554b89779cb815ceb265714575e57bee022b57c4bd1", "src/daemon/upload-http.ts#AuxiliaryHttpAuthorizer": "sha256:6521ffa355c57af97bfc8095699f2a2b38745a6ef8d61a34c8cfc80ee690b6b5", "src/daemon/upload-http.ts#DIRECT_UPLOAD_PATH_PREFIX": "sha256:6fe7d42a65baceb595d96b6aa83fe9aa84527d9aa2e448d7ef47882d31a03276", "src/daemon/upload-http.ts#UploadFinalizeBody": "sha256:c358aea9b3c341f51f3fa263032dfda8a3fbbf729396fcf7f2d8aa051b231b8b", @@ -129,6 +142,11 @@ "src/remote/daemon-artifacts.ts#isRemoteDaemon": "sha256:0f6a714c5f60ce4e8548d811b516d8b6241eca9f7e5414733732da395b8a00e7", "src/remote/daemon-artifacts.ts#materializeRemoteArtifacts": "sha256:1dcab98fba2674f87053ae888c2235b6a6780405aa70648dc57eb6b9077a02fd", "src/remote/daemon-artifacts.ts#resolveMaterializedArtifactPath": "sha256:db5c44effc41ff9fa40bd094945e56ac60fac89a42fa68c65c9293e6de6fb1dc", + "src/remote/remote-request-diagnostics.ts#RemoteDaemonErrorPayload": "sha256:8fbcbcdf4bae7b66a08e465a72593794f36364759f18f1a814cd39dfdb76f309", + "src/remote/remote-request-diagnostics.ts#RemoteDiagnosticsEndpoint": "sha256:bdf63219ecea77d1290f0f5eeef6d5adc55077fa2fa6750ba4608aa3efd540f0", + "src/remote/remote-request-diagnostics.ts#buildRemoteRequestDiagnosticsUrl": "sha256:30a1a9bf0df24d3564a616699916a3bb4360c2b4d21c4ad0562fb0c1e738d0a9", + "src/remote/remote-request-diagnostics.ts#fetchRemoteRequestDiagnostics": "sha256:d856cc83f831491a4c34b1e4feb0867741310bc435808dc469800a643c6b5c31", + "src/remote/remote-request-diagnostics.ts#localizeRemoteDaemonError": "sha256:35cfa186ad64663ae4dbb4e102d49238532cd7e3b75e8337edcc3d1fd77f92d5", "src/remote/upload-client-artifact.ts#PreparedUploadArtifact": "sha256:5c40027d47feda3c946e836372b9f08ad866ab1612708b0d506551545f931ea4", "src/remote/upload-client.ts#ARTIFACT_HASH_ALGORITHM": "sha256:328c135ea5f9d89fd2c142b440d5402a61d19f9044bf0504dfc2c525e58c1909", "src/remote/upload-client.ts#UploadPreflightResponse": "sha256:196a1d9ef41a1aec192589fa7ccb30cf71d971a005349d5d73eed42e6ab21327", @@ -153,5 +171,41 @@ "src/remote/upload-stream.ts#streamFileToHttpRequest": "sha256:eaf2ea49957034f6a7e17d34092dc6cb1998812bddfd68381a0ac34d8a2de58c", "src/remote/upload-stream.ts#streamFileToHttpRequestAttempt": "sha256:da39a79fa7c1f81e55caf613eedc347c0f3a9a9711b265a4db185677532d9552" }, - "compatibleChanges": [] + "compatibleChanges": [ + { + "declaration": "packages/kernel/src/errors.ts#DaemonError", + "digest": "sha256:af0f0f420277a57338dc59b7b443ec83d371e18768d064b7a265949e6435e976", + "rationale": "#1801 adds two optional error fields: `diagnosticsRecord` (session + request id locating the diagnostics record the daemon already names by path) and `logPathUnavailable` (client-set only). A peer on protocol 2 that does not send them is unchanged, and one that does not read them keeps parsing every field it read before." + }, + { + "declaration": "packages/kernel/src/errors.ts#NormalizedError", + "digest": "sha256:ac2c600668420fe5b3d459f8e677676af83951fbad8ee1c2149092fd48e9d47c", + "rationale": "#1801 adds the same two optional fields to the normalized error the daemon serializes; both are omitted unless set, so a released client parsing this payload sees the exact bytes it saw before whenever they are absent, and ignores them otherwise." + }, + { + "declaration": "src/daemon/client/daemon-client-rpc.ts#handleDaemonHttpResponseBody", + "digest": "sha256:8f1c4bba1918545db6296ad29b95f0baa9f949e7c23bc04d59e261130f14611e", + "rationale": "#1801 client-side only: the RPC error branch now runs through `rejectDaemonHttpRpcError`, which reads a superset of the fields it read before and, for a remote daemon, replaces the daemon-host `logPath` with a caller-local copy. Every field a released daemon sends is still accepted; nothing new is required of the daemon." + }, + { + "declaration": "src/daemon/client/daemon-client-rpc.ts#toDaemonHttpRpcError", + "digest": "sha256:ceb3286173ff05415ae71fe8dd974d9a2dfe4d33c9144ae219a3ee992b501e0e", + "rationale": "#1801 client-side only: this now returns the parsed `DaemonError` instead of an `AppError` (construction moved to `appErrorFromDaemonError`) and additionally reads the optional `diagnosticsRecord`. Parsing of every previously read field is unchanged, so a released daemon\u2019s error payload is accepted exactly as before." + }, + { + "declaration": "src/daemon/downloadable-artifact-http.ts#readArtifactId", + "digest": "sha256:3472ee52d951fdf7ef00ee500298602c68f0e1e3b8c44ff51b09981efabf348c", + "rationale": "Inlined `decodeURIComponent`+catch replaced by the shared `decodeUriSegment` (src/daemon/http-request-target.ts), which is listed and byte-identical in behaviour: the same artifact ids decode, and an undecodable id still resolves to the empty id the route rejects." + }, + { + "declaration": "src/daemon/downloadable-artifact-http.ts#handleArtifactDownload", + "digest": "sha256:7f96d17b7c605230fb3cc21ceaa5b2e515d7445653ff95b2b0f6f15fa6214d4e", + "rationale": "The mid-stream error branch moved verbatim into the shared `failStreamedHttpResponse` (listed): same status codes before headers, same destroy after. Response framing on the success path is untouched." + }, + { + "declaration": "packages/kernel/src/errors.ts#normalizeError", + "digest": "sha256:f91f2fa4789075c838be9c42c04046b42d4ebb368eeb1b21194bdb000181e6ce", + "rationale": "#1801 lifts `diagnosticsRecord` / `logPathUnavailable` from details (and from a new optional context field) onto the normalized error and strips them from `details`, the same treatment `logPath` and `diagnosticId` already had. No previously emitted field changes shape or value." + } + ] } diff --git a/test/wire-compat/surface.ts b/test/wire-compat/surface.ts index 279665913d..d291635e2a 100644 --- a/test/wire-compat/surface.ts +++ b/test/wire-compat/surface.ts @@ -48,6 +48,9 @@ const HTTP_ERRORS = 'src/daemon/http-errors.ts'; const HTTP_SERVER = 'src/daemon/server/http-server.ts'; const UPLOAD_HTTP = 'src/daemon/upload-http.ts'; const ARTIFACT_HTTP = 'src/daemon/downloadable-artifact-http.ts'; +const REQUEST_DIAGNOSTICS_HTTP = 'src/daemon/request-diagnostics-http.ts'; +const HTTP_REQUEST_TARGET = 'src/daemon/http-request-target.ts'; +const REMOTE_REQUEST_DIAGNOSTICS = 'src/remote/remote-request-diagnostics.ts'; const PROGRESS_PROTOCOL = 'src/daemon/request-progress-protocol.ts'; const CLIENT_RPC = 'src/daemon/client/daemon-client-rpc.ts'; const CLIENT_PROGRESS = 'src/daemon/client/daemon-client-progress.ts'; @@ -93,6 +96,15 @@ export const WIRE_SURFACE: readonly WireSurfaceGroup[] = [ 'readArtifactId', 'readRequestPathname', ), + // Shared by every auxiliary route: how one segment of a request target + // becomes the id a route matches on. + ...from(HTTP_REQUEST_TARGET, 'decodeUriSegment'), + // `/sessions//requests//diagnostics` (#1801): the + // route a remote caller fetches a failed request's record by. The + // resolver owns the path shape and the segment vocabulary; the client's + // URL builder below is its counterpart. + ...from(REQUEST_DIAGNOSTICS_HTTP, 'resolveRequestDiagnosticsHttpRoute'), + ...from(REMOTE_REQUEST_DIAGNOSTICS, 'buildRemoteRequestDiagnosticsUrl'), // Consumer side of /health: the client reads this payload and refuses a // mismatched peer from it, so a narrowed reader defeats the very check // ADR 0006 built. `readRemoteDaemonHealth` is where the comparison lives. @@ -139,6 +151,11 @@ export const WIRE_SURFACE: readonly WireSurfaceGroup[] = [ ), ...from(UPLOAD_HTTP, 'AuxiliaryHttpAuthorizer', 'buildUploadTicketAuthHeaders'), ...from(ARTIFACT_HTTP, 'DownloadableArtifactHttpAuthorizer'), + // The diagnostics route's authorization: the same token/auth-hook gate as + // the artifact routes, plus the tenant rule that decides which sessions a + // caller may read a record from (#1801). + ...from(REQUEST_DIAGNOSTICS_HTTP, 'RequestDiagnosticsHttpAuthorizer'), + ...from('src/daemon/session-tenant-scope.ts', 'isTenantOwnedSessionName'), ], }, { @@ -217,7 +234,7 @@ export const WIRE_SURFACE: readonly WireSurfaceGroup[] = [ 'DaemonArtifactType', 'DaemonArtifactKnownType', ), - ...from(KERNEL_ERRORS, 'DaemonError'), + ...from(KERNEL_ERRORS, 'DaemonError', 'DiagnosticsRecordRef', 'readDiagnosticsRecordRef'), // "These are wire values" — the progress module says so itself: the daemon // serializes them onto the response stream and the CLI reconstructs them. ...from( @@ -251,6 +268,7 @@ export const WIRE_SURFACE: readonly WireSurfaceGroup[] = [ 'NormalizedHttpError', 'statusCodeForNormalizedError', 'sendRestJsonError', + 'failStreamedHttpResponse', ), ...from( UPLOAD_HTTP, @@ -266,8 +284,24 @@ export const WIRE_SURFACE: readonly WireSurfaceGroup[] = [ // `NormalizedHttpError` is `ReturnType`, so the // function and its return type — not a type alias — are what fix the // REST error payload a released client parses. - ...from(KERNEL_ERRORS, 'normalizeError', 'NormalizedError'), + ...from(KERNEL_ERRORS, 'normalizeError', 'NormalizedError', 'NormalizeErrorContext'), ...from(ARTIFACT_HTTP, 'handleArtifactInventory', 'handleArtifactDownload'), + // Producer and consumer of the diagnostics record body (#1801): what the + // daemon streams back (status, content type/length) and what the client + // accepts before it will name the fetched copy as the caller's log path. + ...from( + REQUEST_DIAGNOSTICS_HTTP, + 'REQUEST_DIAGNOSTICS_CONTENT_TYPE', + 'RequestDiagnosticsHttpOptions', + 'handleRequestDiagnostics', + ), + ...from( + REMOTE_REQUEST_DIAGNOSTICS, + 'RemoteDiagnosticsEndpoint', + 'RemoteDaemonErrorPayload', + 'localizeRemoteDaemonError', + 'fetchRemoteRequestDiagnostics', + ), // Consumer side: what the client accepts back. A parser narrowed here // rejects a released daemon's response without any server change. ...from( @@ -275,6 +309,8 @@ export const WIRE_SURFACE: readonly WireSurfaceGroup[] = [ 'handleDaemonHttpResponseBody', 'parseDaemonHttpResponseBody', 'toDaemonHttpRpcError', + 'rejectDaemonHttpRpcError', + 'appErrorFromDaemonError', 'resolveDaemonHttpResult', ), ...from( diff --git a/website/docs/docs/configuration.md b/website/docs/docs/configuration.md index 803d19767c..74bc8c57c9 100644 --- a/website/docs/docs/configuration.md +++ b/website/docs/docs/configuration.md @@ -69,6 +69,12 @@ protected, operator-controlled configuration. Do not put either value in `./agen For non-loopback remote daemon URLs, the client still requires authentication. Saved `connect` profiles and explicit `--remote-config` workflows remain supported; generated profiles do not persist tokens. +When a command fails against a remote daemon, the `Diagnostics Log:` path is always on the calling +machine: the failing request's record is fetched over the same base URL and token into +`/remote-diagnostics//.ndjson`, so a CI job can keep it as a build +artifact. If the record cannot be fetched the line reads `unavailable` with the remote daemon, the +request id, and the reason — never a path on the daemon host. + Project-safe keys include command defaults such as `platform`, `target`, `device`, `session`, `snapshotDepth`, recording/capture options, and action timing. Connection and provider keys below are user- or explicit-config only: