Skip to content

Commit feeee5e

Browse files
authored
feat: add unified node diagnostics and error normalization (#73)
* feat: add unified node diagnostics and error normalization * chore: clarify diagnostics session scope and retry logging docs * chore: simplify and de-stale AGENTS guidance * chore: add diagnostics context for settings actions
1 parent 0418b0c commit feeee5e

24 files changed

Lines changed: 1260 additions & 289 deletions

AGENTS.md

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,20 @@ Minimal operating guide for AI coding agents in this repo.
4848
- Use `inferFillText` and `uniqueStrings` from `src/daemon/action-utils.ts`. Do not duplicate.
4949
- Use `evaluateIsPredicate` from `src/daemon/is-predicates.ts` for assertion logic. Do not inline.
5050

51+
## Diagnostics & Errors
52+
53+
- Use `src/utils/diagnostics.ts` as the diagnostics source of truth:
54+
- `withDiagnosticsScope`
55+
- `emitDiagnostic`
56+
- `withDiagnosticTimer`
57+
- `flushDiagnosticsToSessionFile`
58+
- Do not add ad-hoc stderr/file logging in handlers/platform modules when diagnostics helpers can be used.
59+
- Normalize user-facing failures through `src/utils/errors.ts` (`normalizeError`).
60+
- Failure payload contract should include: `code`, `message`, `hint`, `diagnosticId`, `logPath`, `details`.
61+
- When wrapping/rethrowing daemon errors (batch/replay/handler wrappers), preserve `hint`, `diagnosticId`, and `logPath` from inner errors.
62+
- `--debug` is canonical; `--verbose` remains backward-compatible alias.
63+
- Keep redaction centralized in `src/utils/diagnostics.ts`; do not duplicate redaction logic in handlers/CLI.
64+
5165
## Key Files
5266
- CLI parse + formatting: `src/bin.ts`, `src/cli.ts`, `src/utils/args.ts`
5367
- Daemon client transport: `src/daemon-client.ts`
@@ -56,7 +70,7 @@ Minimal operating guide for AI coding agents in this repo.
5670
- `is` predicate evaluation: `src/daemon/is-predicates.ts`
5771
- Shared action helpers: `src/daemon/action-utils.ts`
5872
- Snapshot shaping + labels: `src/daemon/snapshot-processing.ts`
59-
- Handler context helpers: `src/daemon/context.ts`, `src/daemon/device-ready.ts`, `src/daemon/app-state.ts`
73+
- Handler context helpers: `src/daemon/context.ts`, `src/daemon/device-ready.ts`
6074
- Dispatcher and capability source of truth: `src/core/dispatch.ts`, `src/core/capabilities.ts`
6175
- Platform backends: `src/platforms/ios/*`, `ios-runner/*`, `src/platforms/android/*`
6276

@@ -93,16 +107,12 @@ Run integration tests when behavior crosses platform boundaries:
93107
- `pnpm test:integration`
94108

95109
## Measurement
96-
- Use `docs/daemon-refactor-impact.md`.
97110
- Track files touched per fix, cycle time, and iOS/Android regressions.
98111

99112
## Local Commands
100113

101114
- Run CLI: `pnpm ad <command>`
102-
- Typecheck: `pnpm typecheck`
103-
- Unit tests: `pnpm test:unit`
104-
- Smoke tests: `pnpm test:smoke`
105-
- Integration tests: `pnpm test:integration`
115+
- For verification commands, use the **Testing** section above.
106116

107117
## Pull Requests
108118
- Before opening PR: ensure no conflict markers and no unmerged paths.

README.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ Flags:
165165
- `--double-tap` use a double-tap gesture per `press`/`click` iteration (cannot be combined with `--hold-ms` or `--jitter-px`)
166166
- `--pause-ms <ms>` delay between `swipe` iterations
167167
- `--pattern one-way|ping-pong` repeat pattern for `swipe`
168-
- `--verbose` for daemon and runner logs
168+
- `--debug` (alias: `--verbose`) for debug diagnostics + daemon/runner logs
169169
- `--json` for structured output
170170
- `--steps <json>` batch: JSON array of steps
171171
- `--steps-file <path>` batch: read step JSON from file
@@ -290,7 +290,13 @@ Boot diagnostics:
290290
- Reason codes: `IOS_BOOT_TIMEOUT`, `IOS_RUNNER_CONNECT_TIMEOUT`, `ANDROID_BOOT_TIMEOUT`, `ADB_TRANSPORT_UNAVAILABLE`, `CI_RESOURCE_STARVATION_SUSPECTED`, `BOOT_COMMAND_FAILED`, `UNKNOWN`.
291291
- Android boot waits fail fast for permission/tooling issues and do not always collapse into timeout errors.
292292
- Use `agent-device boot --platform ios|android` when starting a new session only if `open` cannot find/connect to an available target.
293-
- Set `AGENT_DEVICE_RETRY_LOGS=1` to print structured retry telemetry (attempt, phase, delay, elapsed/remaining deadline, reason).
293+
- `--debug` captures retry telemetry in diagnostics logs.
294+
- Set `AGENT_DEVICE_RETRY_LOGS=1` to also print retry telemetry directly to stderr (ad-hoc troubleshooting).
295+
296+
Diagnostics files:
297+
- Failed commands persist diagnostics in `~/.agent-device/logs/<session>/<date>/<timestamp>-<diagnosticId>.ndjson`.
298+
- `--debug` persists diagnostics for successful commands too and streams live diagnostic events.
299+
- JSON failures include `error.hint`, `error.diagnosticId`, and `error.logPath`.
294300

295301
## App resolution
296302
- Bundle/package identifiers are accepted directly (e.g., `com.apple.Preferences`).
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import test from 'node:test';
2+
import assert from 'node:assert/strict';
3+
import { runCli } from '../cli.ts';
4+
import type { DaemonRequest, DaemonResponse } from '../daemon-client.ts';
5+
6+
class ExitSignal extends Error {
7+
public readonly code: number;
8+
9+
constructor(code: number) {
10+
super(`EXIT_${code}`);
11+
this.code = code;
12+
}
13+
}
14+
15+
type RunResult = {
16+
code: number | null;
17+
stdout: string;
18+
stderr: string;
19+
calls: Omit<DaemonRequest, 'token'>[];
20+
};
21+
22+
async function runCliCapture(
23+
argv: string[],
24+
responder: (req: Omit<DaemonRequest, 'token'>) => Promise<DaemonResponse>,
25+
): Promise<RunResult> {
26+
let stdout = '';
27+
let stderr = '';
28+
let code: number | null = null;
29+
const calls: Array<Omit<DaemonRequest, 'token'>> = [];
30+
31+
const originalExit = process.exit;
32+
const originalStdoutWrite = process.stdout.write.bind(process.stdout);
33+
const originalStderrWrite = process.stderr.write.bind(process.stderr);
34+
35+
(process as any).exit = ((nextCode?: number) => {
36+
throw new ExitSignal(nextCode ?? 0);
37+
}) as typeof process.exit;
38+
(process.stdout as any).write = ((chunk: unknown) => {
39+
stdout += String(chunk);
40+
return true;
41+
}) as typeof process.stdout.write;
42+
(process.stderr as any).write = ((chunk: unknown) => {
43+
stderr += String(chunk);
44+
return true;
45+
}) as typeof process.stderr.write;
46+
47+
const sendToDaemon = async (req: Omit<DaemonRequest, 'token'>): Promise<DaemonResponse> => {
48+
calls.push(req);
49+
return await responder(req);
50+
};
51+
52+
try {
53+
await runCli(argv, { sendToDaemon });
54+
} catch (error) {
55+
if (error instanceof ExitSignal) code = error.code;
56+
else throw error;
57+
} finally {
58+
process.exit = originalExit;
59+
process.stdout.write = originalStdoutWrite;
60+
process.stderr.write = originalStderrWrite;
61+
}
62+
63+
return { code, stdout, stderr, calls };
64+
}
65+
66+
test('cli forwards --debug as verbose/debug metadata', async () => {
67+
const result = await runCliCapture(['open', 'settings', '--debug', '--json'], async () => ({
68+
ok: true,
69+
data: { app: 'settings' },
70+
}));
71+
assert.equal(result.code, null);
72+
assert.equal(result.calls.length, 1);
73+
assert.equal(result.calls[0]?.flags?.verbose, true);
74+
assert.equal(result.calls[0]?.meta?.debug, true);
75+
assert.equal(typeof result.calls[0]?.meta?.requestId, 'string');
76+
});
77+
78+
test('cli returns normalized JSON failures with diagnostics fields', async () => {
79+
const result = await runCliCapture(['open', 'settings', '--json'], async () => ({
80+
ok: false,
81+
error: {
82+
code: 'COMMAND_FAILED',
83+
message: 'boom',
84+
hint: 'retry later',
85+
diagnosticId: 'diag-123',
86+
logPath: '/tmp/diag.ndjson',
87+
details: { token: 'secret', safe: 'ok' },
88+
},
89+
}));
90+
assert.equal(result.code, 1);
91+
const payload = JSON.parse(result.stdout);
92+
assert.equal(payload.success, false);
93+
assert.equal(payload.error.code, 'COMMAND_FAILED');
94+
assert.equal(payload.error.hint, 'retry later');
95+
assert.equal(payload.error.diagnosticId, 'diag-123');
96+
assert.equal(payload.error.logPath, '/tmp/diag.ndjson');
97+
assert.equal(payload.error.details.token, '[REDACTED]');
98+
assert.equal(payload.error.details.safe, 'ok');
99+
});
100+
101+
test('cli parse failures include diagnostic references in JSON mode', async () => {
102+
const previousHome = process.env.HOME;
103+
process.env.HOME = '/tmp';
104+
try {
105+
const result = await runCliCapture(['open', '--unknown-flag', '--json'], async () => ({
106+
ok: true,
107+
data: {},
108+
}));
109+
assert.equal(result.code, 1);
110+
assert.equal(result.calls.length, 0);
111+
const payload = JSON.parse(result.stdout);
112+
assert.equal(payload.success, false);
113+
assert.equal(payload.error.code, 'INVALID_ARGS');
114+
assert.equal(typeof payload.error.diagnosticId, 'string');
115+
assert.equal(typeof payload.error.logPath, 'string');
116+
} finally {
117+
process.env.HOME = previousHome;
118+
}
119+
});

0 commit comments

Comments
 (0)