From 3a5c4c194c0c11af6cf9723c55fc65e4652d2b8d Mon Sep 17 00:00:00 2001 From: Sailesh Sivakumar <32437884+ss251@users.noreply.github.com> Date: Mon, 20 Jul 2026 23:46:21 +0530 Subject: [PATCH] =?UTF-8?q?fix(remote):=20poll=20MinionJob.status,=20not?= =?UTF-8?q?=20.state=20=E2=80=94=20ping=20now=20sees=20completion=20(#2950?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit submit_job/get_job return the MinionJob row verbatim; its lifecycle field is `status` (src/core/minions/types.ts), not `state`. remote ping typed and read `state`, so every poll saw undefined, the terminal check never matched, and ping always burned its full --timeout and exited 1 even when the autopilot-cycle had completed — printing "Job #N is still undefined." on the way out. Reads fixed to `status`; the ping's own JSON output keys (`state`, `last_state`) are unchanged for consumers. Source-audit regression test pins the field reads. Co-authored-by: Claude Fable 5 --- src/commands/remote.ts | 30 +++++++++------ test/remote-ping-status-field.test.ts | 53 +++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 12 deletions(-) create mode 100644 test/remote-ping-status-field.test.ts diff --git a/src/commands/remote.ts b/src/commands/remote.ts index 9d663d665..a62ec4374 100644 --- a/src/commands/remote.ts +++ b/src/commands/remote.ts @@ -105,13 +105,19 @@ function printHelp(): void { async function runRemotePing(config: NonNullable>, args: string[]): Promise { const { json, timeoutMs } = parseFlags(args); - let submitted: { id: number; name: string; state: string }; + // submit_job / get_job return the MinionJob row verbatim — the lifecycle + // field is `status` (src/core/minions/types.ts), not `state`. Reading + // `state` here made every poll see `undefined`, so the terminal check + // never matched and ping always exhausted its timeout (exit 1) even when + // the cycle completed. The ping's own JSON *output* keys (`state`, + // `last_state`) are kept as-is for consumers. + let submitted: { id: number; name: string; status: string }; try { const res = await callRemoteTool(config, 'submit_job', { name: 'autopilot-cycle', data: { phases: ['sync', 'extract', 'embed'] }, }); - submitted = unpackToolResult<{ id: number; name: string; state: string }>(res); + submitted = unpackToolResult<{ id: number; name: string; status: string }>(res); } catch (e) { return failPing(e, json); } @@ -122,43 +128,43 @@ async function runRemotePing(config: NonNullable>, const startMs = Date.now(); let attempt = 0; - let lastState = submitted.state; + let lastState = submitted.status; while (Date.now() - startMs < timeoutMs) { const elapsed = Date.now() - startMs; const intervalMs = elapsed < 30_000 ? 1_000 : elapsed < 5 * 60_000 + 30_000 ? 5_000 : 10_000; await sleep(intervalMs); attempt++; - let job: { id: number; state: string; failed_reason?: string }; + let job: { id: number; status: string; failed_reason?: string }; try { const res = await callRemoteTool(config, 'get_job', { id: submitted.id }); - job = unpackToolResult<{ id: number; state: string; failed_reason?: string }>(res); + job = unpackToolResult<{ id: number; status: string; failed_reason?: string }>(res); } catch (e) { // Network blip mid-poll: log and keep going. Surface only if persistent. if (!json) console.error(` poll #${attempt} failed (${e instanceof Error ? e.message : String(e)}); continuing...`); continue; } - if (job.state !== lastState) { - lastState = job.state; - if (!json) console.error(` job #${submitted.id} → ${job.state}`); + if (job.status !== lastState) { + lastState = job.status; + if (!json) console.error(` job #${submitted.id} → ${job.status}`); } const terminal = ['completed', 'failed', 'dead', 'cancelled']; - if (terminal.includes(job.state)) { - const ok = job.state === 'completed'; + if (terminal.includes(job.status)) { + const ok = job.status === 'completed'; if (json) { console.log(JSON.stringify({ status: ok ? 'success' : 'error', job_id: submitted.id, - state: job.state, + state: job.status, ...(job.failed_reason ? { failed_reason: job.failed_reason } : {}), elapsed_ms: Date.now() - startMs, })); } else { console.log(ok ? `\nautopilot-cycle complete (${Math.round((Date.now() - startMs) / 1000)}s).` - : `\nautopilot-cycle ended ${job.state}${job.failed_reason ? `: ${job.failed_reason}` : ''}.`); + : `\nautopilot-cycle ended ${job.status}${job.failed_reason ? `: ${job.failed_reason}` : ''}.`); } process.exit(ok ? 0 : 1); } diff --git a/test/remote-ping-status-field.test.ts b/test/remote-ping-status-field.test.ts new file mode 100644 index 000000000..11e74e905 --- /dev/null +++ b/test/remote-ping-status-field.test.ts @@ -0,0 +1,53 @@ +/** + * Regression guard: `gbrain remote ping` must poll the MinionJob `status` + * field, never `state`. + * + * submit_job and get_job (src/core/operations.ts) return the MinionJob row + * verbatim, whose lifecycle field is `status` + * (src/core/minions/types.ts). remote.ts once typed and read `state` + * instead: every poll then saw `undefined`, the terminal check + * (`['completed','failed','dead','cancelled'].includes(job.state)`) never + * matched, and ping exhausted its full --timeout and exited 1 even when + * the autopilot-cycle had completed — printing + * "Job #N is still undefined." on the way out. + * + * Source-audit style (same idiom as thin-client-routing-audit.test.ts): + * pins the reads without needing a live MCP transport. + */ + +import { describe, test, expect } from 'bun:test'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +const REMOTE_TS_PATH = join(import.meta.dir, '..', 'src', 'commands', 'remote.ts'); +const REMOTE_SOURCE = readFileSync(REMOTE_TS_PATH, 'utf8'); + +describe('remote ping polls MinionJob.status, not .state', () => { + test('no `.state` property reads on job objects remain', () => { + // Catches `submitted.state`, `job.state` — any resurrection of the + // wrong field. The ping's JSON *output* keys (`state:`, `last_state:`) + // are object-literal keys, not property reads, and don't match this. + expect(REMOTE_SOURCE).not.toMatch(/\b(?:job|submitted)\.state\b/); + }); + + test('poll loop reads job.status', () => { + expect(REMOTE_SOURCE).toMatch(/\bjob\.status\b/); + expect(REMOTE_SOURCE).toMatch(/\bsubmitted\.status\b/); + }); + + test('terminal-state check tests job.status', () => { + expect(REMOTE_SOURCE).toMatch(/terminal\.includes\(job\.status\)/); + }); + + test('unpack generics type the lifecycle field as status', () => { + // Both the submit and poll unpack sites must carry `status: string` in + // their type argument, and none may reintroduce `state: string`. + const unpackShapes = REMOTE_SOURCE.match(/unpackToolResult<\{[^}]*\}>/g) ?? []; + const jobShapes = unpackShapes.filter((s) => s.includes('id: number')); + expect(jobShapes.length).toBeGreaterThanOrEqual(2); + for (const shape of jobShapes) { + expect(shape).toContain('status: string'); + expect(shape).not.toContain('state: string'); + } + }); +});