Skip to content

Commit 8697199

Browse files
authored
fix: scope runner diagnostics to sessions (#704)
1 parent 76cee98 commit 8697199

20 files changed

Lines changed: 285 additions & 45 deletions

AGENTS.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,14 @@ Command-only flags (like `find --first`) that do not flow to the platform layer
159159
## Logs Contract
160160
- Logs backend/source of truth is `src/daemon/app-log.ts`.
161161
- `session.ts` should orchestrate only (start/stop/path/doctor/mark), not duplicate backend logic.
162+
- App logs are distinct from runner/platform output. Keep app/device log capture in `app.log`; Apple runner and `xcodebuild` subprocess output belongs in the session-scoped `runner.log`.
162163
- Preserve external grep/tail workflow in docs/skills.
163164

164165
## Diagnostics & Errors
165166
- Diagnostics source of truth: `src/utils/diagnostics.ts`
166-
- `withDiagnosticsScope`, `emitDiagnostic`, `withDiagnosticTimer`, `flushDiagnosticsToSessionFile`
167+
- `withDiagnosticsScope`, `updateDiagnosticsScope`, `emitDiagnostic`, `withDiagnosticTimer`, `flushDiagnosticsToSessionFile`
168+
- Request diagnostics belong in `sessions/<effective-session>/requests/<request-id>.ndjson` once the effective session is resolved. The top-level daemon log is for daemon lifecycle/startup and pre-session failures.
169+
- Session artifact paths are centralized in `src/daemon/session-store.ts`; do not hand-build session log paths in handlers.
167170
- Do not add ad-hoc stderr/file logging where diagnostics helpers apply.
168171
- Normalize user-facing failures via `src/utils/errors.ts` (`normalizeError`).
169172
- Failure payload contract: `code`, `message`, `hint`, `diagnosticId`, `logPath`, `details`.

src/client-normalizers.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ export function normalizeSession(value: unknown): AgentDeviceSession {
111111
return {
112112
name,
113113
createdAt: readRequiredNumber(record, 'createdAt'),
114+
sessionStateDir: readOptionalString(record, 'sessionStateDir'),
115+
runnerLogPath: readOptionalString(record, 'runnerLogPath'),
114116
device: {
115117
platform,
116118
target,

src/client-shared.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ function serializeSessionDevice(
6767
export function serializeSessionListEntry(session: AgentDeviceSession): Record<string, unknown> {
6868
return {
6969
name: session.name,
70+
...(session.sessionStateDir ? { sessionStateDir: session.sessionStateDir } : {}),
71+
...(session.runnerLogPath ? { runnerLogPath: session.runnerLogPath } : {}),
7072
...serializeSessionDevice(session.device, { includeAndroidSerial: false }),
7173
createdAt: session.createdAt,
7274
};
@@ -141,6 +143,8 @@ export function serializeOpenResult(result: AppOpenResult): Record<string, unkno
141143
{
142144
session: result.session,
143145
...(result.sessionStateDir ? { sessionStateDir: result.sessionStateDir } : {}),
146+
...(result.runnerLogPath ? { runnerLogPath: result.runnerLogPath } : {}),
147+
...(result.requestLogPath ? { requestLogPath: result.requestLogPath } : {}),
144148
...(result.appName ? { appName: result.appName } : {}),
145149
...(result.appBundleId ? { appBundleId: result.appBundleId } : {}),
146150
...(result.startup ? { startup: result.startup } : {}),

src/client-types.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ export type AgentDeviceSessionDevice = {
134134
export type AgentDeviceSession = {
135135
name: string;
136136
createdAt: number;
137+
sessionStateDir?: string;
138+
runnerLogPath?: string;
137139
device: AgentDeviceSessionDevice;
138140
identifiers: AgentDeviceIdentifiers;
139141
};
@@ -185,6 +187,8 @@ export type AppOpenOptions = AgentDeviceRequestOverrides &
185187
export type AppOpenResult = {
186188
session: string;
187189
sessionStateDir?: string;
190+
runnerLogPath?: string;
191+
requestLogPath?: string;
188192
appName?: string;
189193
appBundleId?: string;
190194
appId?: string;

src/daemon/__tests__/request-execution-scope.test.ts

Lines changed: 104 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { afterAll, test, expect } from 'vitest';
22
import fs from 'node:fs';
33
import os from 'node:os';
44
import path from 'node:path';
5-
import { withDiagnosticsScope } from '../../utils/diagnostics.ts';
5+
import { flushDiagnosticsToSessionFile, withDiagnosticsScope } from '../../utils/diagnostics.ts';
66
import {
77
makeAndroidSession,
88
makeIosSession,
@@ -14,6 +14,7 @@ import {
1414
createRequestExecutionScope,
1515
prepareLockedRequestScope,
1616
} from '../request-execution-scope.ts';
17+
import { resolveSessionRequestLogPath } from '../session-store.ts';
1718
import type { DaemonRequest } from '../types.ts';
1819

1920
const TEST_ROOT = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-request-execution-scope-'));
@@ -48,6 +49,52 @@ test('createRequestExecutionScope applies tenant scoping and lease admission', a
4849
expect(scope.sessionName).toBe('tenant-a:default');
4950
});
5051

52+
test('createRequestExecutionScope resolves session-scoped request and runner log paths', async () => {
53+
const sessionStore = makeSessionStore('agent-device-request-scope-');
54+
const cwd = fs.mkdtempSync(path.join(TEST_ROOT, 'cwd-scope-'));
55+
fs.mkdirSync(path.join(cwd, '.git'));
56+
57+
const scope = await withDiagnosticsScope(
58+
{ command: 'snapshot', requestId: 'request-logs-1', logPath: LOG_PATH },
59+
async () =>
60+
await createRequestExecutionScope({
61+
req: makeRequest({ meta: { cwd, requestId: 'request-logs-1' } }),
62+
sessionStore,
63+
leaseRegistry: new LeaseRegistry(),
64+
}),
65+
);
66+
67+
expect(scope.sessionName).toMatch(/^cwd:[a-f0-9]{16}:default$/);
68+
expect(scope.requestLogPath).toMatch(
69+
/cwd_[a-f0-9]{16}_default\/requests\/request-logs-1\.ndjson$/,
70+
);
71+
expect(scope.runnerLogPath).toMatch(/cwd_[a-f0-9]{16}_default\/runner\.log$/);
72+
});
73+
74+
test('request diagnostics flush into the effective session request log', async () => {
75+
const sessionStore = makeSessionStore('agent-device-request-scope-');
76+
const cwd = fs.mkdtempSync(path.join(TEST_ROOT, 'diag-scope-'));
77+
fs.mkdirSync(path.join(cwd, '.git'));
78+
79+
const result = await withDiagnosticsScope(
80+
{ command: 'snapshot', requestId: 'request-diag-1', logPath: LOG_PATH },
81+
async () => {
82+
const scope = await createRequestExecutionScope({
83+
req: makeRequest({ meta: { cwd, requestId: 'request-diag-1' } }),
84+
sessionStore,
85+
leaseRegistry: new LeaseRegistry(),
86+
});
87+
return {
88+
expectedPath: scope.requestLogPath,
89+
flushedPath: flushDiagnosticsToSessionFile({ force: true }),
90+
};
91+
},
92+
);
93+
94+
expect(result.flushedPath).toBe(result.expectedPath);
95+
expect(fs.readFileSync(result.expectedPath, 'utf8')).toContain('"phase":"request_start"');
96+
});
97+
5198
test('createRequestExecutionScope rejects tenant requests without an active lease', async () => {
5299
await expect(
53100
createRequestExecutionScope({
@@ -67,6 +114,40 @@ test('createRequestExecutionScope rejects tenant requests without an active leas
67114
).rejects.toThrow(/Lease is not active/);
68115
});
69116

117+
test('tenant lease rejection flushes diagnostics into the effective session request log', async () => {
118+
const sessionStore = makeSessionStore('agent-device-request-scope-');
119+
const requestId = 'tenant-lease-rejection';
120+
let flushedPath: string | null = null;
121+
122+
await withDiagnosticsScope({ command: 'snapshot', requestId, logPath: LOG_PATH }, async () => {
123+
await expect(
124+
createRequestExecutionScope({
125+
req: makeRequest({
126+
session: 'default',
127+
command: 'snapshot',
128+
meta: {
129+
tenantId: 'tenant-a',
130+
runId: 'run-1',
131+
leaseId: '0'.repeat(32),
132+
sessionIsolation: 'tenant',
133+
requestId,
134+
},
135+
}),
136+
sessionStore,
137+
leaseRegistry: new LeaseRegistry(),
138+
}),
139+
).rejects.toThrow(/Lease is not active/);
140+
flushedPath = flushDiagnosticsToSessionFile({ force: true });
141+
});
142+
143+
const expectedPath = resolveSessionRequestLogPath(
144+
sessionStore.resolveSessionDir('tenant-a:default'),
145+
requestId,
146+
);
147+
expect(flushedPath).toBe(expectedPath);
148+
expect(fs.readFileSync(expectedPath, 'utf8')).toContain('"phase":"request_start"');
149+
});
150+
70151
test('prepareLockedRequestScope preserves existing-session selector validation', async () => {
71152
const sessionStore = makeSessionStore('agent-device-request-scope-');
72153
sessionStore.set('default', makeAndroidSession('default'));
@@ -84,7 +165,6 @@ test('prepareLockedRequestScope preserves existing-session selector validation',
84165
expect(() =>
85166
prepareLockedRequestScope({
86167
scope,
87-
logPath: LOG_PATH,
88168
sessionStore,
89169
trackDownloadableArtifact: () => 'artifact-id',
90170
}),
@@ -116,7 +196,6 @@ test('prepareLockedRequestScope blocks commands for invalidated recordings befor
116196
const result = await withDiagnosticsScope({ command: 'snapshot', logPath: LOG_PATH }, async () =>
117197
prepareLockedRequestScope({
118198
scope,
119-
logPath: LOG_PATH,
120199
sessionStore,
121200
trackDownloadableArtifact: () => 'artifact-id',
122201
}),
@@ -131,6 +210,28 @@ test('prepareLockedRequestScope blocks commands for invalidated recordings befor
131210
}
132211
});
133212

213+
test('prepareLockedRequestScope passes the session runner log path into handler context', async () => {
214+
const sessionStore = makeSessionStore('agent-device-request-scope-');
215+
sessionStore.set('default', makeIosSession('default'));
216+
const scope = await createRequestExecutionScope({
217+
req: makeRequest({ command: 'snapshot' }),
218+
sessionStore,
219+
leaseRegistry: new LeaseRegistry(),
220+
});
221+
222+
const result = prepareLockedRequestScope({
223+
scope,
224+
sessionStore,
225+
trackDownloadableArtifact: () => 'artifact-id',
226+
});
227+
228+
expect(result.type).toBe('scope');
229+
if (result.type === 'scope') {
230+
expect(result.scope.logPath).toBe(scope.runnerLogPath);
231+
expect(result.scope.contextFromFlags(undefined).logPath).toBe(scope.runnerLogPath);
232+
}
233+
});
234+
134235
test('runLocked rejects a canceled request before executing work', async () => {
135236
const requestId = 'request-scope-canceled-before-lock';
136237
const scope = await createRequestExecutionScope({

src/daemon/__tests__/request-router-open.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ test('open returns and creates the session state directory', async () => {
7171
if (response.ok) {
7272
expect(response.data?.session).toBe('session-a');
7373
expect(response.data?.sessionStateDir).toEqual(expect.stringContaining('session-a'));
74+
expect(response.data?.runnerLogPath).toEqual(
75+
path.join(String(response.data?.sessionStateDir), 'runner.log'),
76+
);
77+
expect(response.data?.requestLogPath).toEqual(
78+
path.join(String(response.data?.sessionStateDir), 'requests', 'req-open-state.ndjson'),
79+
);
7480
expect(fs.existsSync(String(response.data?.sessionStateDir))).toBe(true);
7581
}
7682
});

src/daemon/handlers/session-inventory.ts

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
resolveIosSimulatorDeviceSetPath,
1414
} from '../../utils/device-isolation.ts';
1515
import type { DaemonRequest, DaemonResponse } from '../types.ts';
16-
import { SessionStore } from '../session-store.ts';
16+
import { resolveSessionRunnerLogPath, SessionStore } from '../session-store.ts';
1717
import { listAndroidApps } from '../../platforms/android/app-lifecycle.ts';
1818
import { listIosApps } from '../../platforms/ios/apps.ts';
1919
import { requireSessionOrExplicitSelector, resolveCommandDevice } from './session-device-utils.ts';
@@ -35,20 +35,25 @@ export async function handleSessionInventoryCommands(params: {
3535
sessions: sessionStore
3636
.toArray()
3737
.filter((session) => sessionMatchesScope(session, scope))
38-
.map((session) => ({
39-
name: session.name,
40-
platform: session.device.platform,
41-
target: session.device.target ?? 'mobile',
42-
surface: session.surface ?? 'app',
43-
device: session.device.name,
44-
id: session.device.id,
45-
device_id: session.device.id,
46-
createdAt: session.createdAt,
47-
...(session.device.platform === 'ios' && {
48-
device_udid: session.device.id,
49-
ios_simulator_device_set: session.device.simulatorSetPath ?? null,
50-
}),
51-
})),
38+
.map((session) => {
39+
const sessionStateDir = sessionStore.resolveSessionDir(session.name);
40+
return {
41+
name: session.name,
42+
sessionStateDir,
43+
runnerLogPath: resolveSessionRunnerLogPath(sessionStateDir),
44+
platform: session.device.platform,
45+
target: session.device.target ?? 'mobile',
46+
surface: session.surface ?? 'app',
47+
device: session.device.name,
48+
id: session.device.id,
49+
device_id: session.device.id,
50+
createdAt: session.createdAt,
51+
...(session.device.platform === 'ios' && {
52+
device_udid: session.device.id,
53+
ios_simulator_device_set: session.device.simulatorSetPath ?? null,
54+
}),
55+
};
56+
}),
5257
},
5358
};
5459
}

src/daemon/handlers/session-open-surface.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import type { StartupPerfSample } from './session-startup-metrics.ts';
88

99
export function buildOpenResult(params: {
1010
sessionName: string;
11-
sessionStateDir?: string;
11+
sessionStateDir: string;
12+
runnerLogPath: string;
13+
requestLogPath: string;
1214
appName?: string;
1315
appBundleId?: string;
1416
surface: SessionSurface;
@@ -21,6 +23,8 @@ export function buildOpenResult(params: {
2123
const {
2224
sessionName,
2325
sessionStateDir,
26+
runnerLogPath,
27+
requestLogPath,
2428
appName,
2529
appBundleId,
2630
surface,
@@ -30,8 +34,13 @@ export function buildOpenResult(params: {
3034
runtime,
3135
runtimeHintCount,
3236
} = params;
33-
const result: Record<string, unknown> = { session: sessionName, surface };
34-
if (sessionStateDir) result.sessionStateDir = sessionStateDir;
37+
const result: Record<string, unknown> = {
38+
session: sessionName,
39+
surface,
40+
sessionStateDir,
41+
runnerLogPath,
42+
requestLogPath,
43+
};
3544
if (appName) result.appName = appName;
3645
if (appBundleId) result.appBundleId = appBundleId;
3746
if (startup) result.startup = startup;

src/daemon/handlers/session-open.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ import {
1010
import { applyRuntimeHintsToApp } from '../runtime-hints.ts';
1111
import type { DeviceInfo } from '../../utils/device.ts';
1212
import type { DaemonRequest, DaemonResponse, SessionRuntimeHints, SessionState } from '../types.ts';
13-
import { SessionStore } from '../session-store.ts';
13+
import {
14+
resolveSessionRequestLogPath,
15+
resolveSessionRunnerLogPath,
16+
SessionStore,
17+
} from '../session-store.ts';
1418
import {
1519
IOS_SIMULATOR_POST_CLOSE_SETTLE_MS,
1620
IOS_SIMULATOR_POST_OPEN_SETTLE_MS,
@@ -24,6 +28,7 @@ import { buildNextOpenSession, buildOpenResult } from './session-open-surface.ts
2428
import { markAndroidSnapshotFreshness } from '../android-snapshot-freshness.ts';
2529
import { resetAndroidFramePerfStats } from '../../platforms/android/perf.ts';
2630
import { withKeyedLock } from '../../utils/keyed-lock.ts';
31+
import { getDiagnosticsMeta } from '../../utils/diagnostics.ts';
2732
import { inferAndroidPackageAfterOpen } from './session-open-target.ts';
2833
import {
2934
invalidOpenArgs,
@@ -255,10 +260,16 @@ async function completeOpenCommand(params: {
255260
setSessionRuntimeHintsForOpen(sessionStore, sessionName, runtime);
256261
}
257262
const sessionStateDir = sessionStore.ensureSessionDir(sessionName);
263+
const requestLogPath = resolveSessionRequestLogPath(
264+
sessionStateDir,
265+
req.meta?.requestId ?? getDiagnosticsMeta().requestId,
266+
);
258267
timing.totalDurationMs = Math.max(0, Date.now() - openCommandStartedAtMs);
259268
const openResult = buildOpenResult({
260269
sessionName: nextSession.name,
261270
sessionStateDir,
271+
runnerLogPath: resolveSessionRunnerLogPath(sessionStateDir),
272+
requestLogPath,
262273
appName,
263274
appBundleId: sessionAppBundleId,
264275
surface,

0 commit comments

Comments
 (0)