mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
fix(remote): poll MinionJob.status, not .state — ping now sees completion (#2950)
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
d165e99f0b
commit
3a5c4c194c
+18
-12
@@ -105,13 +105,19 @@ function printHelp(): void {
|
||||
async function runRemotePing(config: NonNullable<ReturnType<typeof loadConfig>>, args: string[]): Promise<void> {
|
||||
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<ReturnType<typeof loadConfig>>,
|
||||
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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');
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user