diff --git a/README.md b/README.md index 633ecff..ac78191 100644 --- a/README.md +++ b/README.md @@ -144,8 +144,11 @@ tmux attach -t my-agent `pluk watch` and `pluk subscribe` deliberately swallow capture-pane failures, input errors and malformed log lines so the pipe-pane process never dies. To tell a healthy quiet session from one that is repeatedly failing, add -`--diagnostics[=secs]` (default every 60 s). It writes one JSON line per period, -plus a final one on exit, to **stderr** — stdout stays pure event JSONL: +`--diagnostics[=secs]` (default every 60 s); the value must be a positive number +of seconds (at most 2147483, the `setInterval` limit), or omitted to use the +default—anything else is rejected with exit 2. +It writes one JSON line per period, plus a final one on exit, to **stderr** — +stdout stays pure event JSONL: ```json {"pluk_diagnostics":1,"command":"watch","uptime_s":120,"final":false,"linesSeen":842,"framesPolled":0,"captureFailures":0,"classifyErrors":0,"inputErrors":0,"eventsEmitted":31,"eventsFiltered":4} diff --git a/changelog.d/fixed-342.md b/changelog.d/fixed-342.md new file mode 100644 index 0000000..861a878 --- /dev/null +++ b/changelog.d/fixed-342.md @@ -0,0 +1 @@ +- `watch`/`subscribe --diagnostics=SECS`: a non-numeric, non-positive or `false` value (`abc`, `0`, `-5`, `5min`, `false`) used to silently run at the 60s default; it is now rejected with exit 2, as are values above 2147483 s that would overflow `setInterval` (#342). diff --git a/src/cli.ts b/src/cli.ts index 99c9a57..07ab9db 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -121,6 +121,24 @@ function parseFilter(raw: string | undefined): PlukEventType[] | undefined { return types as PlukEventType[]; } +// setInterval() stores its delay as a signed 32-bit int; larger values are +// clamped to 1 ms with a TimeoutOverflowWarning, so reject them up front. +const MAX_DIAGNOSTICS_SECONDS = Math.floor((2 ** 31 - 1) / 1000); + +function parseDiagnosticsFlag(raw: string | undefined): string | undefined { + if (raw === undefined || raw === 'true') return raw; + const seconds = Number(raw); + if (!Number.isFinite(seconds) || seconds <= 0) { + console.error(`${paintStderr(ANSI_RED, 'Error:')} --diagnostics expects a positive number of seconds (or a bare --diagnostics for the 60s default)`); + process.exit(2); + } + if (seconds > MAX_DIAGNOSTICS_SECONDS) { + console.error(`${paintStderr(ANSI_RED, 'Error:')} --diagnostics must be at most ${MAX_DIAGNOSTICS_SECONDS} seconds`); + process.exit(2); + } + return raw; +} + function cmdSubscribe(args: string[]): void { const { positional, flags } = parseArgs(args); const session = positional[0]; @@ -132,6 +150,7 @@ function cmdSubscribe(args: string[]): void { } const filter = parseFilter(flags['filter']); + const diagnosticsFlag = parseDiagnosticsFlag(flags['diagnostics']); const sub = new Subscriber({ session, @@ -145,7 +164,7 @@ function cmdSubscribe(args: string[]): void { console.log(JSON.stringify(event)); }); - const stopDiagnostics = startDiagnostics('subscribe', () => ({ ...sub.stats() }), flags['diagnostics']); + const stopDiagnostics = startDiagnostics('subscribe', () => ({ ...sub.stats() }), diagnosticsFlag); sub.on('error', err => { console.error(`${paintStderr(ANSI_RED, 'Error:')} ${(err as Error).message}`); @@ -173,6 +192,7 @@ function cmdWatch(args: string[]): void { const cli = flags['cli'] ?? 'claude'; const filter = parseFilter(flags['filter']); + const diagnosticsFlag = parseDiagnosticsFlag(flags['diagnostics']); const noPatternSet = !hasAnyPattern(getPatterns(cli, flags['patterns-dir'])); if (noPatternSet) { @@ -237,7 +257,7 @@ function cmdWatch(args: string[]): void { // rotation rather than derive a log path from it. } - const stopDiagnostics = startDiagnostics('watch', () => ({ ...watcher.stats() }), flags['diagnostics']); + const stopDiagnostics = startDiagnostics('watch', () => ({ ...watcher.stats() }), diagnosticsFlag); process.on('exit', stopDiagnostics); process.on('SIGINT', () => { diff --git a/src/diagnostics.ts b/src/diagnostics.ts index a13ebc0..0742fa0 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -18,6 +18,11 @@ const DIAGNOSTICS_INTERVAL_MS = 60_000; * no session name, raw output or other user-provided value is included, * and nothing is sent anywhere. Returns a function that writes a final * summary and stops the timer. + * + * `flag` is the raw `--diagnostics` value. The CLI validates it before + * calling (exit 2 on anything that is not a positive, in-range number); + * library callers that pass a non-finite or non-positive value get the + * 60 s default instead. */ export function startDiagnostics( command: 'watch' | 'subscribe', diff --git a/test/cli-watch-guard.test.js b/test/cli-watch-guard.test.js index 21d1b68..fb6e0ac 100644 --- a/test/cli-watch-guard.test.js +++ b/test/cli-watch-guard.test.js @@ -94,6 +94,31 @@ for (const bad of ['0', '-5', 'abc', '', '1.5']) { }); } +// --- watch/subscribe: invalid --diagnostics value is rejected --------------------- + +for (const cmd of ['watch', 'subscribe']) { + for (const bad of ['abc', '0', '-5', '5min', 'false']) { + test(`${cmd} --diagnostics=${bad} exits 2 with a usage error`, () => { + const { code, stderr } = run(CLI, [cmd, 'x', `--diagnostics=${bad}`]); + assert.equal(code, 2); + assert.match(stderr, /--diagnostics expects a positive number of seconds/); + }); + } + + test(`${cmd} --diagnostics above the setInterval limit exits 2 with a usage error`, () => { + const { code, stderr } = run(CLI, [cmd, 'x', '--diagnostics=2147484']); + assert.equal(code, 2); + assert.match(stderr, /--diagnostics must be at most 2147483 seconds/); + assert.doesNotMatch(stderr, /TimeoutOverflowWarning/); + }); +} + +test('watch rejects a bad --diagnostics value before consuming stdin or logging', () => { + const { code, stderr } = run(CLI, ['watch', 'x', '--cli=claude', '--diagnostics=0'], { input: 'hello\n' }); + assert.equal(code, 2); + assert.doesNotMatch(stderr, /pluk_diagnostics/); +}); + test('watch --pane without --capture warns that --pane is ignored', () => { const { code, stderr } = run(CLI, ['watch', 'x', '--pane=%1'], { input: '' }); assert.equal(code, 0);