Compare commits

..
Author SHA1 Message Date
Garry TanandClaude Fable 5 09f9b00f86 docs: restore parseNiceFlag docblock displaced by PRUNE_STATUSES; note --status/0d in top-level jobs help
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-22 11:20:16 -07:00
dfe1755da6 fix(jobs): prune --status filter + --older-than 0d as explicit no-age-floor (takeover of #2282)
Salvages the CLI plumbing from PR #2282 (queue.prune already accepted a
status[] param; the CLI exposed neither knob) and repairs the flaws found
in verification:

- --status completed,failed,dead,cancelled passes an explicit terminal
  subset through to queue.prune; anything else fails fast. Parsing lives
  in exported parsePruneStatuses (unit-tested, mirrors parseNiceFlag).
- --older-than 0d is documented and messaged as what it actually does:
  NO age floor — deletes ALL matching terminal jobs — not "same-day
  only" as the original PR body claimed. Help text + success line say so
  ("regardless of age") so an operator can't mistake it for a same-day
  cutoff.
- Real tests this time: queue-level status-filter + zero-age-floor cases
  in test/minions.test.ts, parser cases in test/jobs-prune-flags.test.ts
  (the original PR cited tests in a file that does not exist).

Co-authored-by: brettdavies <brettdavies@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-21 14:20:23 -07:00
10 changed files with 110 additions and 123 deletions
+1 -1
View File
@@ -2387,7 +2387,7 @@ JOBS (Minions)
jobs get <id> Job details + history
jobs cancel <id> Cancel job
jobs retry <id> Re-queue failed/dead job
jobs prune [--older-than 30d] Clean old jobs
jobs prune [--older-than 30d] [--status s,..] Clean old terminal jobs (0d = no age floor)
jobs stats Job health dashboard
jobs work [--queue Q] Start worker daemon (Postgres only)
-12
View File
@@ -186,18 +186,6 @@ export function isSourceStale(src: SourceRow, now = Date.now(), floorMin = FULL_
return ageMin >= floorMin;
}
/**
* #2060: count sources past the per-source cycle freshness floor. Consumed
* by autopilot's dispatch decision — a stale source forces the fanout path
* even when the doctor plan is small (score 7094, plan ≤ 3, est < 300s),
* so targeted mode can't leave cycle_freshness stale indefinitely.
* dispatchPerSource's own throttles (skipped_fresh / fanoutMax / failure
* cooldown) bound the resulting work.
*/
export function countStaleSources(sources: SourceRow[], now = Date.now(), floorMin = FULL_CYCLE_FLOOR_MIN): number {
return sources.filter((s) => isSourceStale(s, now, floorMin)).length;
}
/**
* Most recent SUCCESSFUL cycle for a source. Prefers `last_source_cycle_at`
* (per-source phases, written by the split cycle) and falls back to the legacy
+2 -16
View File
@@ -901,27 +901,13 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
const FULL_CYCLE_FLOOR_MIN = 60;
const minutesSinceLastFull = (Date.now() - lastFullCycleAt) / 60000;
// #2060: stale per-source cycle freshness is a dispatch input. Without
// it, a brain sitting at score 7094 with a small targeted plan (≤3
// steps, <300s) stays in targeted mode indefinitely and no per-source
// cycle is ever dispatched — cycle_freshness never advances. A stale
// source forces the fanout path; dispatchPerSource's throttles
// (skipped_fresh / fanoutMax / failure cooldown) bound the work.
// Fail-open to 0: a read failure must not block dispatch.
let staleCycleSources = 0;
try {
const { countStaleSources } = await import('./autopilot-fanout.ts');
staleCycleSources = countStaleSources(await engine.listAllSources({ localPathOnly: true }));
} catch { /* fail-open: freshness is a dispatch hint, not a gate */ }
const shouldFullCycle =
(score >= 95 && plan.length === 0 && minutesSinceLastFull >= FULL_CYCLE_FLOOR_MIN) ||
plan.length > 3 ||
estTotal >= 300 ||
score < 70 ||
staleCycleSources > 0;
score < 70;
const shouldSleep = score >= 95 && plan.length === 0 && minutesSinceLastFull < FULL_CYCLE_FLOOR_MIN && staleCycleSources === 0;
const shouldSleep = score >= 95 && plan.length === 0 && minutesSinceLastFull < FULL_CYCLE_FLOOR_MIN;
if (shouldSleep) {
if (jsonMode) {
+33 -5
View File
@@ -106,6 +106,21 @@ export function parseMaxRssFlag(args: string[]): number | undefined {
return parsed;
}
/** Terminal statuses `jobs prune --status` accepts (PR #2282). Matches what
* queue.prune can safely delete; anything else (waiting/active/…) is live. */
export const PRUNE_STATUSES = ['completed', 'failed', 'dead', 'cancelled'] as const satisfies readonly MinionJobStatus[];
/** Parse a `--status a,b,c` value into prune statuses. Throws on any value
* outside PRUNE_STATUSES (fail-fast, mirrors parseNiceValue). */
export function parsePruneStatuses(raw: string): MinionJobStatus[] {
const requested = raw.split(',').map(s => s.trim()).filter(Boolean);
const invalid = requested.filter(s => !(PRUNE_STATUSES as readonly string[]).includes(s));
if (requested.length === 0 || invalid.length > 0) {
throw new Error(`--status accepts a comma-separated subset of [${PRUNE_STATUSES.join(', ')}]${invalid.length ? `. Invalid: ${invalid.join(', ')}` : ''}`);
}
return requested as MinionJobStatus[];
}
/** Parse `--nice N` (then `GBRAIN_NICE` env). Returns:
* - undefined if absent (no priority change — inherit)
* - the validated integer in [-20, 19] otherwise
@@ -208,7 +223,9 @@ USAGE
gbrain jobs get <id>
gbrain jobs cancel <id>
gbrain jobs retry <id>
gbrain jobs prune [--older-than 30d]
gbrain jobs prune [--older-than 30d] [--status completed,failed,dead,cancelled]
(--older-than 0d = no age floor: deletes ALL
matching terminal jobs; pair with --status)
gbrain jobs delete <id>
gbrain jobs stats
gbrain jobs smoke
@@ -600,16 +617,27 @@ HANDLER TYPES (built in)
case 'prune': {
const olderThanStr = parseFlag(args, '--older-than') ?? '30d';
const days = parseInt(olderThanStr, 10);
if (isNaN(days) || days <= 0) {
console.error('Error: --older-than must be a positive number (days). Example: --older-than 30d');
if (isNaN(days) || days < 0) {
console.error('Error: --older-than must be a non-negative number (days). Example: --older-than 30d; --older-than 0d removes the age floor (deletes ALL matching terminal jobs).');
process.exit(1);
}
const statusFlag = parseFlag(args, '--status');
let statuses: MinionJobStatus[] | undefined;
if (statusFlag !== undefined) {
try { statuses = parsePruneStatuses(statusFlag); }
catch (e) { console.error(`Error: ${e instanceof Error ? e.message : String(e)}`); process.exit(1); }
}
try { await queue.ensureSchema(); }
catch (e) { console.error(e instanceof Error ? e.message : String(e)); process.exit(1); }
const count = await queue.prune({ olderThan: new Date(Date.now() - days * 86400000) });
console.log(`Pruned ${count} jobs older than ${days} days.`);
const count = await queue.prune({
olderThan: new Date(Date.now() - days * 86400000),
...(statuses ? { status: statuses } : {}),
});
const statusLabel = statuses ? statuses.join('+') : 'completed+dead+cancelled';
const ageLabel = days === 0 ? 'regardless of age' : `older than ${days} days`;
console.log(`Pruned ${count} ${statusLabel} jobs ${ageLabel}.`);
break;
}
+10 -19
View File
@@ -854,10 +854,7 @@ interface SyncPhaseResult extends PhaseResult {
/**
* Resolve the source id for a brain directory by looking up the sources
* table. Returns undefined when no registered source matches (falls back
* to pre-v0.18 global config.sync.* keys) OR when MORE than one source
* claims the path — an ambiguous match must not scope phases or stamp
* last_full_cycle_at for an arbitrarily-picked source (the "freshness
* stamp that lies" this resolution exists to prevent).
* to pre-v0.18 global config.sync.* keys).
*/
async function resolveSourceForDir(
engine: BrainEngine,
@@ -868,10 +865,10 @@ async function resolveSourceForDir(
if (brainDir === null) return undefined;
try {
const rows = await engine.executeRaw<{ id: string }>(
`SELECT id FROM sources WHERE local_path = $1 LIMIT 2`,
`SELECT id FROM sources WHERE local_path = $1 LIMIT 1`,
[brainDir],
);
return rows.length === 1 ? rows[0]!.id : undefined;
return rows[0]?.id;
} catch {
// sources table might not exist on very old brains — fall through.
return undefined;
@@ -2368,23 +2365,17 @@ export async function runCycle(
}
// v0.38 (codex r1 P0-5): persist per-source cycle completion timestamp
// when the cycle ran successfully against a resolvable source. Read by
// autopilot's per-source freshness gate next tick.
//
// #1993: keyed off `cycleSourceId` (opts.sourceId ?? the source resolved
// from brainDir) — the SAME id the cycle locked + scoped its phases to —
// NOT raw opts.sourceId. The autopilot's inline cycle sets brainDir but
// passes no explicit sourceId, so keying off opts.sourceId alone never
// advanced last_full_cycle_at and cycle_freshness stayed stale even while
// the autopilot cycled every interval. Skipped when:
// - no source resolves (engine null, or no checkout AND no opts.sourceId)
// when the cycle ran successfully against an explicit source. Read by
// autopilot's per-source freshness gate next tick. Skipped when:
// - opts.sourceId is unset (legacy callers — autopilot still here)
// - engine is null (no-DB path)
// - status is 'failed' or 'skipped' (don't mark a non-run as fresh)
// - dryRun (writes are out of scope)
//
// Best-effort: a write failure does NOT change the CycleReport status.
// The cost of writing the wrong timestamp post-failure is higher than
// the cost of missing a successful write (next cycle will redo work).
if (cycleSourceId && engine && !dryRun && !aborted && (status === 'ok' || status === 'clean' || status === 'partial')) {
if (opts.sourceId && engine && !dryRun && !aborted && (status === 'ok' || status === 'clean' || status === 'partial')) {
try {
const nowIso = new Date().toISOString();
// #2194 fix #3 (the cycle split): `last_source_cycle_at` is the NEW gate
@@ -2394,13 +2385,13 @@ export async function runCycle(
// phases (those gate on autopilot.last_global_at), so writing it on a
// source-only cycle does not re-introduce the freshness poisoning codex
// flagged in the rejected skip-based design.
await engine.updateSourceConfig(cycleSourceId, {
await engine.updateSourceConfig(opts.sourceId, {
last_source_cycle_at: nowIso,
last_full_cycle_at: nowIso,
});
} catch (e) {
// Best-effort; cycle already succeeded by the time we get here.
console.warn(`[cycle] failed to write last_source_cycle_at for source ${cycleSourceId}: ${e instanceof Error ? e.message : String(e)}`);
console.warn(`[cycle] failed to write last_source_cycle_at for source ${opts.sourceId}: ${e instanceof Error ? e.message : String(e)}`);
}
}
-14
View File
@@ -54,20 +54,6 @@ describe('autopilot.ts ↔ dispatchPerSource wiring', () => {
expect(AUTOPILOT_SRC).toMatch(/lastFullCycleAt\s*=\s*Date\.now\(\)/);
});
test('stale per-source cycle freshness is a shouldFullCycle input (#2060)', () => {
// Targeted mode (score 7094, plan ≤3, est <300s) must not be able to
// starve per-source cycle dispatch: a stale source (per countStaleSources
// over listAllSources) forces the fanout path, and the sleep gate must
// not fire while stale sources exist. Without these terms, cycle
// freshness never advances for a brain that always lands in targeted mode.
expect(AUTOPILOT_SRC).toMatch(/countStaleSources/);
const fullCycleDeclIdx = AUTOPILOT_SRC.indexOf('const shouldFullCycle');
expect(fullCycleDeclIdx).toBeGreaterThan(-1);
const decl = AUTOPILOT_SRC.slice(fullCycleDeclIdx, fullCycleDeclIdx + 700);
expect(decl).toMatch(/staleCycleSources\s*>\s*0/);
expect(decl).toMatch(/const shouldSleep[^;]*staleCycleSources\s*===\s*0/);
});
test('does NOT regress to the single-job dispatch on the full-cycle path', () => {
// Pre-PR: the shouldFullCycle branch did:
// const job = await queue.add('autopilot-cycle', { repoPath }, {
-18
View File
@@ -14,7 +14,6 @@ import { describe, test, expect } from 'bun:test';
import {
readLastFullCycleAt,
isSourceStale,
countStaleSources,
selectSourcesForDispatch,
resolveFanoutMax,
dispatchPerSource,
@@ -75,23 +74,6 @@ describe('isSourceStale', () => {
});
});
describe('countStaleSources (#2060 dispatch-decision input)', () => {
const NOW = Date.parse('2026-05-22T12:00:00.000Z');
test('counts never-cycled + past-floor sources, ignores fresh', () => {
const sources = [
src('never-cycled'), // stale (null)
src('old', new Date(NOW - 2 * 60 * 60_000).toISOString()), // stale (2h)
src('fresh', new Date(NOW - 30 * 60_000).toISOString()), // fresh (30min)
];
expect(countStaleSources(sources, NOW)).toBe(2);
});
test('returns 0 for all-fresh and for empty list', () => {
const fresh = src('a', new Date(NOW - 10 * 60_000).toISOString());
expect(countStaleSources([fresh], NOW)).toBe(0);
expect(countStaleSources([], NOW)).toBe(0);
});
});
describe('selectSourcesForDispatch', () => {
const NOW = Date.parse('2026-05-22T12:00:00.000Z');
const fresh = (id: string, agoMin: number) =>
+8 -38
View File
@@ -3,10 +3,8 @@
* cycles. Closes codex round-1 P0-5 (write site for last_full_cycle_at
* was unspecified pre-PR).
*
* Conditions for write (keyed off `cycleSourceId` = opts.sourceId ?? the
* source resolved from brainDir, so the autopilot's inline cycle brainDir
* set, no explicit sourceId also advances the timestamp, #1993):
* - a source resolves (explicit sourceId, or brainDir matches a source)
* Conditions for write:
* - opts.sourceId is set (legacy callers without sourceId skip the write)
* - engine is non-null (no-DB path skips)
* - status is 'ok' | 'clean' | 'partial' (failed/skipped don't mark fresh)
* - dryRun is false
@@ -92,45 +90,17 @@ describe('runCycle last_full_cycle_at exit hook', () => {
});
});
test('no explicit sourceId but brainDir resolves a source → writes the resolved source timestamp', async () => {
test('legacy caller (no sourceId) does NOT write any source timestamp', async () => {
await withEnv({ GBRAIN_HOME: gbrainHome }, async () => {
// The autopilot's inline cycle sets brainDir but passes no sourceId.
// runCycle resolves the source from brainDir (local_path match) into
// cycleSourceId and stamps last_full_cycle_at for it — otherwise
// cycle_freshness reports the brain stale even while the autopilot
// cycles every interval (#1993).
await seedSource('resolved-from-dir'); // local_path = brainDir
expect(await readLastFullCycleAt('resolved-from-dir')).toBeNull();
const t0 = Date.now();
const report = await runCycle(engine, {
brainDir,
phases: ['lint'],
});
expect(['ok', 'clean']).toContain(report.status);
const after = await readLastFullCycleAt('resolved-from-dir');
expect(after).not.toBeNull();
expect(new Date(after!).getTime()).toBeGreaterThanOrEqual(t0);
});
});
test('no sourceId and brainDir matches no source → does not write', async () => {
await withEnv({ GBRAIN_HOME: gbrainHome }, async () => {
// A source exists but its local_path does NOT match brainDir, so
// resolveSourceForDir returns undefined, cycleSourceId is undefined,
// and no per-source timestamp is written.
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, config, archived, created_at)
VALUES ('unmatched', 'unmatched', '/no/such/repo', '{}'::jsonb, false, NOW())
ON CONFLICT (id) DO UPDATE SET local_path = EXCLUDED.local_path`,
[],
);
await seedSource('default-like');
// No sourceId passed; should remain untouched.
await runCycle(engine, {
brainDir,
phases: ['lint'],
});
expect(await readLastFullCycleAt('unmatched')).toBeNull();
// No per-source write happens; default source's config stays empty.
const after = await readLastFullCycleAt('default-like');
expect(after).toBeNull();
});
});
+30
View File
@@ -0,0 +1,30 @@
/**
* Unit tests for parsePruneStatuses (PR #2282) `jobs prune --status` parsing.
*/
import { describe, test, expect } from 'bun:test';
import { parsePruneStatuses, PRUNE_STATUSES } from '../src/commands/jobs.ts';
describe('parsePruneStatuses', () => {
test('parses a single status', () => {
expect(parsePruneStatuses('failed')).toEqual(['failed']);
});
test('parses a comma-separated list with whitespace', () => {
expect(parsePruneStatuses(' completed, dead ')).toEqual(['completed', 'dead']);
});
test('accepts every documented terminal status', () => {
expect(parsePruneStatuses(PRUNE_STATUSES.join(','))).toEqual([...PRUNE_STATUSES]);
});
test('throws on non-terminal statuses', () => {
expect(() => parsePruneStatuses('waiting')).toThrow(/Invalid: waiting/);
expect(() => parsePruneStatuses('completed,active')).toThrow(/Invalid: active/);
});
test('throws on empty value', () => {
expect(() => parsePruneStatuses('')).toThrow(/comma-separated subset/);
expect(() => parsePruneStatuses(',')).toThrow(/comma-separated subset/);
});
});
+26
View File
@@ -702,6 +702,32 @@ describe('MinionQueue: Prune', () => {
const count = await queue.prune({ olderThan: new Date(Date.now() + 86400000) }); // future date = prune everything old enough
expect(count).toBe(1); // only the cancelled one
});
// PR #2282: `jobs prune --status` passes an explicit status subset through.
test('status filter prunes only the requested terminal statuses', async () => {
const cancelled = await queue.add('sync', {});
await queue.cancelJob(cancelled.id);
const dead = await queue.add('embed', {}, { max_attempts: 1 });
await queue.claim('tok1', 30000, 'default', ['embed']);
await queue.failJob(dead.id, 'tok1', 'boom', 'dead');
const count = await queue.prune({ olderThan: new Date(Date.now() + 86400000), status: ['dead'] });
expect(count).toBe(1); // only the dead one
const remaining = await queue.getJobs({ status: 'cancelled' });
expect(remaining.length).toBe(1);
});
// PR #2282: `--older-than 0d` = no age floor — olderThan of "now" deletes
// terminal jobs that finished moments ago.
test('olderThan now (0d semantics) prunes just-terminated jobs', async () => {
const job = await queue.add('sync', {});
await queue.cancelJob(job.id);
await new Promise(r => setTimeout(r, 5)); // ensure updated_at < now
const count = await queue.prune({ olderThan: new Date() });
expect(count).toBe(1);
});
});
// --- Stats (1 test) ---