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
6 changed files with 111 additions and 65 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)
+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;
}
+3 -11
View File
@@ -35,11 +35,10 @@
*
* The doctor renders both side by side.
*
* Drift contract: every check name that ships through doctor MUST appear in
* Drift contract: every check name that ships in doctor.ts MUST appear in
* exactly one set below. The drift-guard test in
* `test/doctor-categories.test.ts` enforces this by reading doctor check
* emitter sources via a tagged-string scan and asserting set membership
* exactly.
* `test/doctor-categories.test.ts` enforces this by reading doctor.ts source
* via a tagged-string scan and asserting set membership exactly.
*
* If you add a new doctor check, you MUST add its name to the appropriate
* set here. The categorize step in `src/commands/doctor.ts` falls through
@@ -68,15 +67,12 @@ export const BRAIN_CHECK_NAMES: ReadonlySet<string> = new Set([
'conversation_parser_probe_health',
'cross_modal_modality_backfill',
'cycle_freshness',
'dangling_aliases',
'effective_date_health',
'embed_staleness',
'embedding_column_registry',
'embedding_env_override',
'embedding_provider',
'embedding_width_consistency',
'embeddings',
'entity_link_coverage',
'eval_drift',
'extract_atoms_backlog',
'extract_health',
@@ -106,9 +102,7 @@ export const BRAIN_CHECK_NAMES: ReadonlySet<string> = new Set([
'stub_guard_24h',
'sync_failures',
'sync_freshness',
'takes_count',
'takes_weight_grid',
'timeline_coverage',
'unified_multimodal_coverage',
'voice_gate_health',
]);
@@ -176,14 +170,12 @@ export const META_CHECK_NAMES: ReadonlySet<string> = new Set([
'eval_capture',
'minions_migration',
'multi_source_drift',
'pack_upgrade_available',
'schema_pack_active',
'schema_pack_consistency',
'schema_pack_source_drift',
'schema_version',
'slug_fallback_audit',
'timeline_dedup_index',
'type_proliferation',
'upgrade_errors',
]);
+18 -48
View File
@@ -1,10 +1,10 @@
/**
* Drift guard for src/core/doctor-categories.ts.
*
* Reads doctor check emitter source via a literal-string scan, enumerates every
* `name: '<...>'` Check name, and asserts each appears in exactly ONE category
* set. The union of the four sets must equal the discovered names exactly —
* no orphans, no extras.
* Reads src/commands/doctor.ts source via a literal-string scan, enumerates
* every `name: '<...>'` Check name, and asserts each appears in exactly ONE
* category set. The union of the four sets must equal the discovered names
* exactly — no orphans, no extras.
*
* This is the structural failure the v0.41.19.0 plan-eng-review caught:
* doctor.ts grows new checks regularly; without this guard, the
@@ -25,30 +25,26 @@ import {
} from '../src/core/doctor-categories.ts';
const DOCTOR_TS_PATH = join(import.meta.dir, '..', 'src', 'commands', 'doctor.ts');
const ONBOARD_CHECKS_TS_PATH = join(import.meta.dir, '..', 'src', 'core', 'onboard', 'checks.ts');
const CHECK_SOURCE_PATHS = [DOCTOR_TS_PATH, ONBOARD_CHECKS_TS_PATH];
function enumerateCheckNames(): Set<string> {
const source = readFileSync(DOCTOR_TS_PATH, 'utf-8');
const names = new Set<string>();
for (const path of CHECK_SOURCE_PATHS) {
const source = readFileSync(path, 'utf-8');
// 1) Inline object-literal form: `{ name: 'foo', ... }`.
for (const m of source.matchAll(/name:\s*['"]([a-z][a-z0-9_]+)['"]/g)) {
names.add(m[1]);
}
// 2) Helper-function form: `const name = 'foo';` inside a check helper.
// Catches checks like `nightly_quality_probe_health` and
// `conversation_facts_backlog` that build the Check from a captured
// name constant.
for (const m of source.matchAll(/const\s+name\s*=\s*['"]([a-z][a-z0-9_]+)['"]/g)) {
names.add(m[1]);
}
// 1) Inline object-literal form: `{ name: 'foo', ... }`.
for (const m of source.matchAll(/name:\s*['"]([a-z][a-z0-9_]+)['"]/g)) {
names.add(m[1]);
}
// 2) Helper-function form: `const name = 'foo';` inside a check helper.
// Catches checks like `nightly_quality_probe_health` and
// `conversation_facts_backlog` that build the Check from a captured
// name constant.
for (const m of source.matchAll(/const\s+name\s*=\s*['"]([a-z][a-z0-9_]+)['"]/g)) {
names.add(m[1]);
}
return names;
}
describe('doctor-categories drift guard', () => {
test('every doctor-emitted check name belongs to exactly one category set', () => {
test('every check name in doctor.ts source belongs to exactly one category set', () => {
const discovered = enumerateCheckNames();
const allCategorized = new Set<string>([
...BRAIN_CHECK_NAMES,
@@ -63,7 +59,7 @@ describe('doctor-categories drift guard', () => {
}
if (missing.length > 0) {
throw new Error(
`These check names appear in doctor check emitters but are not categorized in ` +
`These check names appear in doctor.ts but are not categorized in ` +
`src/core/doctor-categories.ts: ${missing.sort().join(', ')}. ` +
`Add each to BRAIN/SKILL/OPS/META_CHECK_NAMES.`,
);
@@ -90,7 +86,7 @@ describe('doctor-categories drift guard', () => {
expect(dupes).toEqual([]);
});
test('every categorized name is currently used in doctor check emitters (no stale entries)', () => {
test('every categorized name is currently used in doctor.ts source (no stale entries)', () => {
const discovered = enumerateCheckNames();
const allCategorized = new Set<string>([
...BRAIN_CHECK_NAMES,
@@ -128,14 +124,6 @@ describe('categorizeCheck', () => {
expect(categorizeCheck('sync_freshness')).toBe('brain');
});
test('returns the right category for onboard data-quality check names', () => {
expect(categorizeCheck('embed_staleness')).toBe('brain');
expect(categorizeCheck('entity_link_coverage')).toBe('brain');
expect(categorizeCheck('timeline_coverage')).toBe('brain');
expect(categorizeCheck('takes_count')).toBe('brain');
expect(categorizeCheck('dangling_aliases')).toBe('brain');
});
test('returns the right category for a known skill name', () => {
expect(categorizeCheck('resolver_health')).toBe('skill');
expect(categorizeCheck('skill_conformance')).toBe('skill');
@@ -152,24 +140,6 @@ describe('categorizeCheck', () => {
expect(categorizeCheck('upgrade_errors')).toBe('meta');
});
test('returns the right category for onboard schema-pack check names without warning', () => {
const originalWrite = process.stderr.write.bind(process.stderr);
const captured: string[] = [];
(process.stderr as { write: typeof process.stderr.write }).write = ((
chunk: string | Uint8Array,
) => {
captured.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString());
return true;
}) as typeof process.stderr.write;
try {
expect(categorizeCheck('pack_upgrade_available')).toBe('meta');
expect(categorizeCheck('type_proliferation')).toBe('meta');
expect(captured.filter((c) => c.includes('[doctor-categories]'))).toEqual([]);
} finally {
(process.stderr as { write: typeof process.stderr.write }).write = originalWrite;
}
});
test('unknown check name falls through to meta with a stderr warn (once per process)', () => {
const originalWrite = process.stderr.write.bind(process.stderr);
const captured: string[] = [];
+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) ---