From 04b90374c7ec797579ebfc4e9eb0e5b59f313cb9 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 2 Jun 2026 22:53:03 -0700 Subject: [PATCH] fix(jobs): decouple 'jobs watch' format (--json) from loop (--follow); non-TTY prints one human snapshot (#1784) --- src/commands/jobs-watch.ts | 85 ++++++++++++++++++++------ src/commands/jobs.ts | 7 ++- test/e2e/non-tty-output.serial.test.ts | 61 ++++++++++++++++++ test/jobs-watch-mode.test.ts | 44 +++++++++++++ 4 files changed, 176 insertions(+), 21 deletions(-) create mode 100644 test/e2e/non-tty-output.serial.test.ts create mode 100644 test/jobs-watch-mode.test.ts diff --git a/src/commands/jobs-watch.ts b/src/commands/jobs-watch.ts index ec0dd9381..0c5d5105e 100644 --- a/src/commands/jobs-watch.ts +++ b/src/commands/jobs-watch.ts @@ -9,15 +9,27 @@ * 60fps; 1s keeps the SQL load nominal even when multiple watch sessions * point at the same brain). * - * Rendering: manual ANSI cursor management (no TUI dep). Clears the - * screen on first render, then redraws from the top each tick using - * cursor-home + erase-down. On non-TTY (cron / wrapped redirect), - * falls through to one snapshot line per tick in `--progress-json` - * shape so wrappers can parse. + * Two independent axes (v0.42.11.0, #1784 — decoupled from `isTTY`): + * - FORMAT (what data prints): human by default, JSON only when `--json` is + * passed. NEVER gated on isTTY. + * - LOOP (cadence): `--follow` streams continuously; default is `isTTY` — + * continuous live dashboard in a terminal, ONE snapshot then exit when + * non-TTY (pipe / cron / subagent). Identical data either way, so defaulting + * the loop from isTTY is a cosmetic UX call, not a data gate. * - * Quit: Ctrl-C (SIGINT), 'q', or stdin close — the watcher restores the - * cursor + clears its own region on shutdown so the terminal isn't left - * with a half-rendered dashboard. + * Resulting matrix: + * TTY, no flags → live ANSI dashboard (cursor-managed, loops) + * non-TTY, no flags → ONE human plain-text snapshot, exit + * any + --json → JSON snapshot (one-shot, or JSONL stream w/ --follow) + * any + --follow → continuous (human plain per tick, or JSONL w/ --json) + * + * Rendering: manual ANSI cursor management (no TUI dep) for the live dashboard + * only. Clears the screen on first render, then redraws from the top each tick + * using cursor-home + erase-down. + * + * Quit: in the live dashboard, Ctrl-C (SIGINT) or 'q' restores the cursor + + * clears its region. Non-TTY one-shots (nothing to quit); a non-TTY `--follow` + * stream runs until the process is killed. * * No SSE consumer in v0.41 — local polling against the brain engine is * the foundation. SSE wiring through `serve-http.ts` is filed as a @@ -188,32 +200,63 @@ export async function readSnapshot(engine: BrainEngine): Promise export interface WatchOptions { /** Refresh interval. Default 1000ms. */ refreshMs?: number; - /** Stream JSON snapshots to stdout (non-TTY mode). */ + /** FORMAT axis: emit JSON instead of human text. Default human. Explicit only. */ json?: boolean; + /** + * LOOP axis: stream continuously. Default = `process.stdout.isTTY` — live + * dashboard in a terminal, one snapshot then exit when non-TTY. Pass `true` + * to force a continuous stream even off-TTY (cron tail / log pipe). + */ + follow?: boolean; +} + +export interface WatchMode { + /** FORMAT: emit JSON instead of human text. */ + json: boolean; + /** LOOP: continuous stream vs one-shot. */ + follow: boolean; + /** Live cursor-managed colored dashboard (TTY + human + looping only). */ + useAnsiDashboard: boolean; } /** - * Main entrypoint for `gbrain jobs watch`. Runs until SIGINT or 'q' - * keypress (on TTY). Non-TTY mode loops with --progress-json output. + * Pure resolver for the format × loop matrix (extracted for unit-testing the + * exact TTY-gating contract this command fixes, #1784). The data printed never + * depends on isTTY; only the loop cadence + ANSI cursor management do. + * + * follow default = `isTTY && !json`: a terminal human view is the live + * dashboard (loops), but `--json` (any) and non-TTY both one-shot unless the + * caller passes `--follow` explicitly. Matches the file-header matrix. + */ +export function resolveWatchMode(opts: WatchOptions, isTTY: boolean): WatchMode { + const json = opts.json === true; // FORMAT: explicit only — never from isTTY. + const follow = opts.follow ?? (isTTY && !json); + const useAnsiDashboard = isTTY && !json && follow; + return { json, follow, useAnsiDashboard }; +} + +/** + * Main entrypoint for `gbrain jobs watch`. See the file header for the + * format (`--json`) × loop (`--follow`) matrix. The data printed never depends + * on isTTY; only the loop cadence and the ANSI cursor management do. */ export async function runWatch(engine: BrainEngine, opts: WatchOptions = {}): Promise { const refreshMs = opts.refreshMs ?? 1000; - const isTTY = process.stdout.isTTY === true; - const json = opts.json || !isTTY; + const { json, follow, useAnsiDashboard } = resolveWatchMode(opts, process.stdout.isTTY === true); let stopped = false; const stop = () => { stopped = true; }; - if (isTTY && !json) { + if (useAnsiDashboard) { process.stdout.write(ANSI.cursorHide + ANSI.clear + ANSI.cursorHome); process.on('SIGINT', () => { process.stdout.write(ANSI.cursorShow + ANSI.clear + ANSI.cursorHome); stop(); process.exit(0); }); - // Read stdin for 'q' keypress. + // Read stdin for 'q' keypress (terminal-only affordance). if (process.stdin.isTTY && process.stdin.setRawMode) { process.stdin.setRawMode(true); process.stdin.resume(); @@ -227,15 +270,19 @@ export async function runWatch(engine: BrainEngine, opts: WatchOptions = {}): Pr } } - while (!stopped) { + do { const snap = await readSnapshot(engine); if (json) { process.stdout.write(JSON.stringify({ event: 'jobs.watch.snapshot', ...snap }) + '\n'); - } else { - // TTY: clear + cursor-home + render. + } else if (useAnsiDashboard) { + // Live dashboard: clear + cursor-home + colored render. process.stdout.write(ANSI.cursorHome + ANSI.eraseDown); process.stdout.write(renderSnapshot(snap, { useAnsi: true })); + } else { + // Non-TTY (or --follow without a terminal): plain human snapshot, no ANSI. + process.stdout.write(renderSnapshot(snap, { useAnsi: false }) + '\n'); } + if (!follow) break; // one-shot: render once, exit. await new Promise(r => setTimeout(r, refreshMs)); - } + } while (!stopped); } diff --git a/src/commands/jobs.ts b/src/commands/jobs.ts index b5222a6f4..1d9e5503d 100644 --- a/src/commands/jobs.ts +++ b/src/commands/jobs.ts @@ -1098,14 +1098,17 @@ HANDLER TYPES (built in) } case 'watch': { - // v0.41 D2 — live TTY dashboard (or JSON snapshots on non-TTY). + // v0.41 D2 — live dashboard; v0.42.11.0 (#1784) decoupled output from TTY. + // Flags: --json (FORMAT, human default), --follow (LOOP, default=isTTY so + // non-TTY one-shots), --refresh-ms=N. Non-TTY no-flag → one human snapshot. try { await queue.ensureSchema(); } catch (e) { console.error(e instanceof Error ? e.message : String(e)); process.exit(1); } const { runWatch } = await import('./jobs-watch.ts'); const refreshArg = args.find(a => a.startsWith('--refresh-ms=')); const refreshMs = refreshArg ? parseInt(refreshArg.split('=')[1] ?? '1000', 10) : 1000; const json = hasFlag(args, '--json'); - await runWatch(engine, { refreshMs, json }); + const follow = hasFlag(args, '--follow') ? true : undefined; // undefined → default to isTTY + await runWatch(engine, { refreshMs, json, follow }); break; } diff --git a/test/e2e/non-tty-output.serial.test.ts b/test/e2e/non-tty-output.serial.test.ts new file mode 100644 index 000000000..aee2bf945 --- /dev/null +++ b/test/e2e/non-tty-output.serial.test.ts @@ -0,0 +1,61 @@ +/** + * v0.42.11.0 (#1784) — non-TTY output contract E2E (the `cmd { + let home: string; + let env: NodeJS.ProcessEnv; + + beforeAll(() => { + home = mkdtempSync(join(tmpdir(), 'gbrain-nontty-e2e-')); + env = { ...process.env, GBRAIN_HOME: home }; + // Fresh local PGLite brain so `jobs watch` has an engine to read. + execFileSync('bun', ['run', 'src/cli.ts', 'init', '--pglite', '--no-embedding', '--non-interactive'], { + cwd: process.cwd(), env, stdio: 'ignore', + }); + }, 60_000); + + afterAll(() => { + try { rmSync(home, { recursive: true, force: true }); } catch { /* best-effort */ } + }); + + test('non-TTY, no flags → ONE human snapshot, non-empty, not JSON, exit 0', () => { + // stdin 'ignore' + stdout 'pipe' makes the child non-TTY on both ends. + const out = execFileSync('bun', ['run', 'src/cli.ts', 'jobs', 'watch'], { + cwd: process.cwd(), env, stdio: ['ignore', 'pipe', 'ignore'], encoding: 'utf-8', timeout: 30_000, + }); + expect(out.length).toBeGreaterThan(0); // the bug was (no output) + expect(out).toContain('gbrain jobs watch'); // human renderer header + expect(out).toContain('Queue'); // human panel + expect(out.trimStart().startsWith('{')).toBe(false); // NOT JSON by default + }, 35_000); + + test('non-TTY, --json → ONE JSON snapshot, parses, exit 0', () => { + const out = execFileSync('bun', ['run', 'src/cli.ts', 'jobs', 'watch', '--json'], { + cwd: process.cwd(), env, stdio: ['ignore', 'pipe', 'ignore'], encoding: 'utf-8', timeout: 30_000, + }); + const firstLine = out.split('\n').find(l => l.trim().length > 0) ?? ''; + const parsed = JSON.parse(firstLine); + expect(parsed.event).toBe('jobs.watch.snapshot'); + expect(parsed.queue_health).toBeDefined(); + }, 35_000); +}); diff --git a/test/jobs-watch-mode.test.ts b/test/jobs-watch-mode.test.ts new file mode 100644 index 000000000..0e506fca1 --- /dev/null +++ b/test/jobs-watch-mode.test.ts @@ -0,0 +1,44 @@ +/** + * v0.42.15.0 (#1784) — jobs-watch format × loop matrix. + * + * Pins resolveWatchMode so the TTY-gating contract can't regress. The bug this + * guards: TTY + --json used to loop forever (follow defaulted to isTTY); the + * documented matrix says --json one-shots unless --follow. Caught by codex in + * the pre-landing review; the e2e missed it (non-TTY only). + */ +import { describe, test, expect } from 'bun:test'; +import { resolveWatchMode } from '../src/commands/jobs-watch.ts'; + +describe('resolveWatchMode — format × loop matrix', () => { + test('TTY, no flags → live dashboard (json off, follow on, ansi on)', () => { + expect(resolveWatchMode({}, true)).toEqual({ json: false, follow: true, useAnsiDashboard: true }); + }); + + test('non-TTY, no flags → one human snapshot (the bug fix: not JSON, one-shot)', () => { + expect(resolveWatchMode({}, false)).toEqual({ json: false, follow: false, useAnsiDashboard: false }); + }); + + test('TTY + --json → ONE JSON snapshot, NOT a forever loop (codex finding)', () => { + expect(resolveWatchMode({ json: true }, true)).toEqual({ json: true, follow: false, useAnsiDashboard: false }); + }); + + test('non-TTY + --json → one JSON snapshot', () => { + expect(resolveWatchMode({ json: true }, false)).toEqual({ json: true, follow: false, useAnsiDashboard: false }); + }); + + test('TTY + --follow → live dashboard loops', () => { + expect(resolveWatchMode({ follow: true }, true)).toEqual({ json: false, follow: true, useAnsiDashboard: true }); + }); + + test('non-TTY + --follow → plain human stream (loops, no ansi)', () => { + expect(resolveWatchMode({ follow: true }, false)).toEqual({ json: false, follow: true, useAnsiDashboard: false }); + }); + + test('--json --follow → JSONL stream, never the ansi dashboard', () => { + expect(resolveWatchMode({ json: true, follow: true }, true)).toEqual({ json: true, follow: true, useAnsiDashboard: false }); + }); + + test('explicit --follow false on a TTY → one-shot (override wins)', () => { + expect(resolveWatchMode({ follow: false }, true)).toEqual({ json: false, follow: false, useAnsiDashboard: false }); + }); +});