mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-16 09:52:22 +00:00
Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d1f03cb346 | ||
|
|
e41e3948cd | ||
|
|
edad6b1d5f |
+42
-110
@@ -38,6 +38,7 @@ import { logSelfUpgrade } from '../core/audit/self-upgrade-audit.ts';
|
||||
import { detectInstallMethod } from './upgrade.ts';
|
||||
import { evaluateQuietHours } from '../core/minions/quiet-hours.ts';
|
||||
import { inspectLock } from '../core/db-lock.ts';
|
||||
import { registerCleanup } from '../core/process-cleanup.ts';
|
||||
|
||||
/**
|
||||
* v0.37.7.0 #1162 — classify autopilot reconnect-loop errors.
|
||||
@@ -151,100 +152,6 @@ export function shouldSpawnAutopilotWorker(args: string[]): boolean {
|
||||
return !args.includes('--no-worker');
|
||||
}
|
||||
|
||||
/**
|
||||
* #1525 — positional subcommand translation.
|
||||
*
|
||||
* Pre-fix, `gbrain autopilot status` silently fell through to "start daemon"
|
||||
* because `runAutopilot()` only branched on flag forms (`--status`, etc.).
|
||||
* `status` was treated as a stray positional and ignored.
|
||||
*
|
||||
* This translator maps known positional subcommands to their flag form so
|
||||
* `autopilot status` is equivalent to `autopilot --status`, then rejects
|
||||
* any unrecognized positional with a fail-loud error before any side
|
||||
* effect (lockfile, daemon spawn, sync dispatch) runs.
|
||||
*
|
||||
* Scope decisions:
|
||||
* - Known aliases: `status` → `--status`, `install` → `--install`,
|
||||
* `uninstall` → `--uninstall`, `start` → (drop; default daemon launch).
|
||||
* - `stop` is intentionally NOT aliased here. Stopping a running daemon
|
||||
* is a new behavior (read PID from lock, SIGTERM, drain) that deserves
|
||||
* its own design and PR. Users typing `gbrain autopilot stop` today get
|
||||
* the unknown-positional error with the canonical alternatives.
|
||||
* - At most one positional allowed; multiple positionals fail loud.
|
||||
*/
|
||||
// Every flag that consumes the NEXT argv token. Missing one here makes the
|
||||
// translator misread the flag's value as a positional subcommand and exit 2
|
||||
// (e.g. `--install --target linux-cron`). Keep in sync with parseArg call sites.
|
||||
const AUTOPILOT_VALUE_FLAGS = new Set(['--repo', '--interval', '--target']);
|
||||
const AUTOPILOT_POSITIONAL_ALIASES: Record<string, string | null> = {
|
||||
status: '--status',
|
||||
install: '--install',
|
||||
uninstall: '--uninstall',
|
||||
start: null, // drop the positional; default behavior is daemon launch
|
||||
};
|
||||
|
||||
export type PositionalTranslation =
|
||||
| { ok: true; args: string[] }
|
||||
| {
|
||||
ok: false;
|
||||
reason: 'unknown_subcommand' | 'multiple_subcommands';
|
||||
message: string;
|
||||
};
|
||||
|
||||
export function translatePositionalSubcommands(args: string[]): PositionalTranslation {
|
||||
const out: string[] = [];
|
||||
let positionalSeen = false;
|
||||
let i = 0;
|
||||
while (i < args.length) {
|
||||
const a = args[i];
|
||||
if (AUTOPILOT_VALUE_FLAGS.has(a)) {
|
||||
// Pass through the flag and its value untouched. If the value is
|
||||
// missing at end-of-argv, fall through so the existing parseArg
|
||||
// path can report the broken usage.
|
||||
out.push(a);
|
||||
if (i + 1 < args.length) {
|
||||
out.push(args[i + 1]);
|
||||
i += 2;
|
||||
} else {
|
||||
i += 1;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (a.startsWith('-')) {
|
||||
out.push(a);
|
||||
i += 1;
|
||||
continue;
|
||||
}
|
||||
// Positional subcommand.
|
||||
if (positionalSeen) {
|
||||
const known = Object.keys(AUTOPILOT_POSITIONAL_ALIASES).join(', ');
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'multiple_subcommands',
|
||||
message: `Multiple subcommands given. Use only one of: ${known}.`,
|
||||
};
|
||||
}
|
||||
positionalSeen = true;
|
||||
if (a in AUTOPILOT_POSITIONAL_ALIASES) {
|
||||
const alias = AUTOPILOT_POSITIONAL_ALIASES[a];
|
||||
if (alias) out.push(alias);
|
||||
i += 1;
|
||||
continue;
|
||||
}
|
||||
const known = Object.keys(AUTOPILOT_POSITIONAL_ALIASES).join(', ');
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'unknown_subcommand',
|
||||
message:
|
||||
`Unknown subcommand: \`${a}\`.\n` +
|
||||
`Allowed subcommands: ${known}.\n` +
|
||||
`Or use the flag form: --status, --install, --uninstall.\n` +
|
||||
`Run \`gbrain autopilot --help\` for full usage.`,
|
||||
};
|
||||
}
|
||||
return { ok: true, args: out };
|
||||
}
|
||||
|
||||
export function isPidAlive(pid: number): boolean {
|
||||
if (!Number.isFinite(pid) || pid <= 0) return false;
|
||||
try {
|
||||
@@ -457,11 +364,6 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
|
||||
' gbrain autopilot --install [--repo <path>]\n' +
|
||||
' gbrain autopilot --uninstall\n' +
|
||||
' gbrain autopilot --status [--json]\n\n' +
|
||||
'Subcommand aliases:\n' +
|
||||
' gbrain autopilot status → --status\n' +
|
||||
' gbrain autopilot install → --install\n' +
|
||||
' gbrain autopilot uninstall → --uninstall\n' +
|
||||
' gbrain autopilot start → (default daemon launch)\n\n' +
|
||||
'Self-maintaining brain daemon. Runs the full maintenance cycle\n' +
|
||||
'(lint + backlinks + sync + extract + embed + orphans) on an interval.\n\n' +
|
||||
'For a one-shot cron-triggered cycle, see `gbrain dream`.',
|
||||
@@ -469,16 +371,6 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
|
||||
return;
|
||||
}
|
||||
|
||||
// #1525: translate positional subcommands to their flag form BEFORE any
|
||||
// side effect (lockfile, daemon spawn, sync dispatch). Unknown positionals
|
||||
// fail loud here rather than silently starting the daemon.
|
||||
const translated = translatePositionalSubcommands(args);
|
||||
if (!translated.ok) {
|
||||
console.error(translated.message);
|
||||
process.exit(2);
|
||||
}
|
||||
args = translated.args;
|
||||
|
||||
if (args.includes('--install')) {
|
||||
await installDaemon(engine, args);
|
||||
return;
|
||||
@@ -542,6 +434,37 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
|
||||
let stopping = false;
|
||||
let childSupervisor: ChildWorkerSupervisor | null = null;
|
||||
|
||||
// #1872: graceful engine shutdown. On PGLite the cycle steps run INLINE in
|
||||
// this process, so a hard `process.exit` mid-write (systemctl stop →
|
||||
// SIGTERM) kills WASM Postgres with the WAL dirty and can corrupt the
|
||||
// brain. Two exit paths must both close the engine:
|
||||
// - autopilot's own shutdown() below (owns SIGINT + internal stops like
|
||||
// max_crashes / cycle-failure-cap), and
|
||||
// - process-cleanup's SIGTERM handler (installed at cli.ts module load;
|
||||
// it runs the cleanup registry with a 3s deadline and then exits) —
|
||||
// which is why closeEngine is ALSO registered there.
|
||||
// closeEngine aborts the in-flight inline cycle (runCycle checks the
|
||||
// signal between phases and threads it into phase sub-work), gives it a
|
||||
// short bounded window to wind down, then disconnects. PGLite's
|
||||
// disconnect() drains the pending query and checkpoints before closing;
|
||||
// a second call is a no-op (disconnect snapshots + nulls the handle), so
|
||||
// both paths firing is safe.
|
||||
const shutdownAbort = new AbortController();
|
||||
let inflightInlineCycle: Promise<unknown> | null = null;
|
||||
const closeEngine = async () => {
|
||||
shutdownAbort.abort(new Error('autopilot shutdown'));
|
||||
if (inflightInlineCycle) {
|
||||
// ponytail: 2s cap keeps us inside process-cleanup's 3s deadline; a
|
||||
// between-phase abort resolves instantly, a mid-phase one may not.
|
||||
await Promise.race([
|
||||
inflightInlineCycle.catch(() => { /* cycle errors already logged by the loop */ }),
|
||||
new Promise((r) => setTimeout(r, 2_000)),
|
||||
]);
|
||||
}
|
||||
try { await engine.disconnect(); } catch { /* best-effort */ }
|
||||
};
|
||||
const deregisterEngineClose = registerCleanup('autopilot-engine-close', closeEngine);
|
||||
|
||||
if (spawnManagedWorker) {
|
||||
const cliPath = resolveGbrainCliPath();
|
||||
// Cgroup-aware auto-sized RSS watchdog cap (issue #1678). The old flat
|
||||
@@ -629,6 +552,10 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
|
||||
childSupervisor.killChild('SIGKILL');
|
||||
}
|
||||
}
|
||||
// #1872: abort the in-flight inline cycle and close the engine BEFORE
|
||||
// process.exit — a hard exit mid-write corrupts PGLite's WASM Postgres.
|
||||
await closeEngine();
|
||||
deregisterEngineClose();
|
||||
try { unlinkSync(lockPath); } catch { /* already gone */ }
|
||||
process.exit(0);
|
||||
};
|
||||
@@ -1117,16 +1044,21 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
|
||||
// path's phase set). Now both converge on the same primitive.
|
||||
try {
|
||||
const { runCycle } = await import('../core/cycle.ts');
|
||||
const report = await runCycle(engine, {
|
||||
// #1872: track the promise so closeEngine can drain it on shutdown,
|
||||
// and pass the abort signal so the cycle winds down between phases.
|
||||
const cyclePromise = runCycle(engine, {
|
||||
brainDir: repoPath,
|
||||
// Autopilot daemon path: pulls by default (matches
|
||||
// pre-v0.17 autopilot behavior). CLI dream defaults false
|
||||
// for cron safety; that choice is scoped to dream only.
|
||||
pull: true,
|
||||
signal: shutdownAbort.signal,
|
||||
yieldBetweenPhases: async () => {
|
||||
await new Promise(r => setImmediate(r));
|
||||
},
|
||||
});
|
||||
inflightInlineCycle = cyclePromise;
|
||||
const report = await cyclePromise.finally(() => { inflightInlineCycle = null; });
|
||||
// Only 'failed' (every attempted phase failed) trips the autopilot
|
||||
// circuit breaker. 'partial' means at least one phase warned or
|
||||
// failed while others ran — that's a soft signal, not a fatal
|
||||
|
||||
+23
-3
@@ -26,6 +26,7 @@
|
||||
import type { BrainEngine } from '../core/engine.ts';
|
||||
import {
|
||||
runCycle,
|
||||
resolveSourceForDir,
|
||||
ALL_PHASES,
|
||||
type CyclePhase,
|
||||
type CycleReport,
|
||||
@@ -380,9 +381,9 @@ Options:
|
||||
|
||||
--source <id> Scope the cycle to one source so doctor's
|
||||
cycle_freshness check sees a fresh stamp on
|
||||
completion. Without this, gbrain dream's
|
||||
timestamp never lands and federated brains
|
||||
see "stale cycle" forever.
|
||||
completion. When omitted, gbrain derives the
|
||||
source from --dir / the configured checkout
|
||||
when it matches a source's local_path (#1869).
|
||||
--source-id <id> Alias for --source. Matches the v0.37.7.0+
|
||||
naming used by import/extract/graph-query.
|
||||
|
||||
@@ -634,6 +635,25 @@ export async function runDream(engine: BrainEngine | null, args: string[]): Prom
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
// #1869: a path-scoped run (--dir, or the configured sync.repo_path) whose
|
||||
// directory matches a registered source's local_path IS that source's cycle
|
||||
// — derive the source id so runCycle writes last_source_cycle_at /
|
||||
// last_full_cycle_at on success and doctor's cycle_freshness check stops
|
||||
// reading perpetually stale. Explicit --source still wins (resolved above).
|
||||
// Fixed here at the command level, NOT in runCycle's stamp gate, so legacy
|
||||
// global callers (autopilot-global-maintenance runs GLOBAL_PHASES with a
|
||||
// brainDir and no sourceId) can't falsely stamp per-source freshness.
|
||||
// A derived match on an archived source is skipped silently (falls back to
|
||||
// legacy unscoped behavior) — stamping it would mask staleness on restore,
|
||||
// mirroring the explicit --source archived guard above.
|
||||
if (resolvedSourceId === undefined && engine !== null && brainDir !== null) {
|
||||
const derived = await resolveSourceForDir(engine, brainDir);
|
||||
if (derived !== undefined) {
|
||||
const src = await fetchSource(engine, derived);
|
||||
if (src?.archived !== true) resolvedSourceId = derived;
|
||||
}
|
||||
}
|
||||
// ─── issue #1678: bounded single-hold extract_atoms drain ──────────
|
||||
if (opts.drain) {
|
||||
if (engine === null) {
|
||||
|
||||
+9
-1
@@ -855,8 +855,16 @@ 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).
|
||||
*
|
||||
* Exported for dream.ts (#1869): a `gbrain dream --dir <path>` run whose
|
||||
* path matches a registered source's local_path is a per-source cycle in
|
||||
* everything but name, so dream derives the source id up front and passes
|
||||
* it as opts.sourceId — landing the freshness stamp without changing
|
||||
* runCycle's stamp/lock semantics for legacy global callers (the
|
||||
* autopilot-global-maintenance handler runs GLOBAL_PHASES with a brainDir
|
||||
* and MUST NOT stamp per-source freshness; see rejected PR #2549).
|
||||
*/
|
||||
async function resolveSourceForDir(
|
||||
export async function resolveSourceForDir(
|
||||
engine: BrainEngine,
|
||||
brainDir: string | null,
|
||||
): Promise<string | undefined> {
|
||||
|
||||
@@ -1,197 +0,0 @@
|
||||
/**
|
||||
* Tests for translatePositionalSubcommands() — the v0.41.x #1525 fix that
|
||||
* prevents `gbrain autopilot status` from silently starting the daemon.
|
||||
*
|
||||
* IRON RULE regression guard: the exact ticket repro (`gbrain autopilot
|
||||
* status`) MUST translate to `--status`, not fall through to the default
|
||||
* daemon launch. Verified by the "ticket-exact repro" case below.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { translatePositionalSubcommands } from '../src/commands/autopilot.ts';
|
||||
|
||||
describe('translatePositionalSubcommands — known aliases', () => {
|
||||
test('IRON RULE — `autopilot status` translates to `--status` (ticket #1525 repro)', () => {
|
||||
const r = translatePositionalSubcommands(['status']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--status']);
|
||||
});
|
||||
|
||||
test('`install` translates to `--install`', () => {
|
||||
const r = translatePositionalSubcommands(['install']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--install']);
|
||||
});
|
||||
|
||||
test('`uninstall` translates to `--uninstall`', () => {
|
||||
const r = translatePositionalSubcommands(['uninstall']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--uninstall']);
|
||||
});
|
||||
|
||||
test('`start` drops the positional (default daemon launch)', () => {
|
||||
const r = translatePositionalSubcommands(['start']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual([]);
|
||||
});
|
||||
|
||||
test('`start --json` drops only the positional, keeps the flag', () => {
|
||||
const r = translatePositionalSubcommands(['start', '--json']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--json']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('translatePositionalSubcommands — flag/positional interleaving', () => {
|
||||
test('`status --json` preserves the trailing flag', () => {
|
||||
const r = translatePositionalSubcommands(['status', '--json']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--status', '--json']);
|
||||
});
|
||||
|
||||
test('`--json status` preserves the leading flag', () => {
|
||||
const r = translatePositionalSubcommands(['--json', 'status']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--json', '--status']);
|
||||
});
|
||||
|
||||
test('`--repo /foo status` does not mis-classify the path as positional', () => {
|
||||
const r = translatePositionalSubcommands(['--repo', '/foo', 'status']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--repo', '/foo', '--status']);
|
||||
});
|
||||
|
||||
test('`--interval 300 install` does not mis-classify the number as positional', () => {
|
||||
const r = translatePositionalSubcommands(['--interval', '300', 'install']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--interval', '300', '--install']);
|
||||
});
|
||||
|
||||
test('`--install --target linux-cron` does not mis-classify the target as positional', () => {
|
||||
// --target is installDaemon's value flag; its value must never be read
|
||||
// as a positional subcommand (regression guard for the review fix).
|
||||
const r = translatePositionalSubcommands(['--install', '--target', 'linux-cron']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--install', '--target', 'linux-cron']);
|
||||
});
|
||||
|
||||
test('`install --target macos` keeps the alias translation and the target value', () => {
|
||||
const r = translatePositionalSubcommands(['install', '--target', 'macos']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--install', '--target', 'macos']);
|
||||
});
|
||||
|
||||
test('value-flag at end of argv with missing value passes through (so parseArg can report it)', () => {
|
||||
const r = translatePositionalSubcommands(['--repo']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--repo']);
|
||||
});
|
||||
|
||||
test('value-flag whose value looks like an alias is NOT translated', () => {
|
||||
// `--repo status` means "use repo path 'status'", not "show status".
|
||||
// Translator must not destructure the value of --repo.
|
||||
const r = translatePositionalSubcommands(['--repo', 'status']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--repo', 'status']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('translatePositionalSubcommands — pass-through cases', () => {
|
||||
test('empty args returns empty args', () => {
|
||||
const r = translatePositionalSubcommands([]);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual([]);
|
||||
});
|
||||
|
||||
test('flag-only invocation passes through unchanged', () => {
|
||||
const r = translatePositionalSubcommands(['--status', '--json']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--status', '--json']);
|
||||
});
|
||||
|
||||
test('short flag `-h` passes through unchanged', () => {
|
||||
const r = translatePositionalSubcommands(['-h']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['-h']);
|
||||
});
|
||||
|
||||
test('all known bare flags pass through unchanged', () => {
|
||||
const flags = ['--help', '--install', '--uninstall', '--status', '--json', '--inline', '--no-worker'];
|
||||
const r = translatePositionalSubcommands(flags);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(flags);
|
||||
});
|
||||
});
|
||||
|
||||
describe('translatePositionalSubcommands — rejection of unknown positionals', () => {
|
||||
test('unknown positional `foo` fails with reason=unknown_subcommand + structured message', () => {
|
||||
const r = translatePositionalSubcommands(['foo']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('unknown_subcommand');
|
||||
expect(r.message).toContain('Unknown subcommand: `foo`');
|
||||
expect(r.message).toContain('status');
|
||||
expect(r.message).toContain('install');
|
||||
expect(r.message).toContain('uninstall');
|
||||
expect(r.message).toContain('--help');
|
||||
}
|
||||
});
|
||||
|
||||
test('unknown positional `stop` fails with reason=unknown_subcommand (NOT silently aliased)', () => {
|
||||
// Stop is mentioned in the ticket but deliberately NOT aliased in this
|
||||
// PR — stopping a running daemon is a new behavior, not just an alias.
|
||||
// Until that feature lands separately, `stop` must fail loud rather
|
||||
// than starting the daemon (the bug we're fixing).
|
||||
const r = translatePositionalSubcommands(['stop']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('unknown_subcommand');
|
||||
expect(r.message).toContain('Unknown subcommand: `stop`');
|
||||
}
|
||||
});
|
||||
|
||||
test('unknown positional `status-detail` (close-but-not-matching) fails', () => {
|
||||
const r = translatePositionalSubcommands(['status-detail']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('unknown_subcommand');
|
||||
expect(r.message).toContain('Unknown subcommand: `status-detail`');
|
||||
}
|
||||
});
|
||||
|
||||
test('multiple positionals fail with reason=multiple_subcommands (`start install`)', () => {
|
||||
const r = translatePositionalSubcommands(['start', 'install']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('multiple_subcommands');
|
||||
expect(r.message).toContain('Multiple subcommands');
|
||||
}
|
||||
});
|
||||
|
||||
test('multiple positionals fail even when both are known aliases (`status install`)', () => {
|
||||
const r = translatePositionalSubcommands(['status', 'install']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('multiple_subcommands');
|
||||
expect(r.message).toContain('Multiple subcommands');
|
||||
}
|
||||
});
|
||||
|
||||
test('known-then-unknown rejects with multiple_subcommands (first-positional-wins)', () => {
|
||||
// First positional is known, second is not. Rejection comes from the
|
||||
// multiple-positional rule, which fires before the unknown check; the
|
||||
// intent is "only one subcommand allowed."
|
||||
const r = translatePositionalSubcommands(['status', 'garbage']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) expect(r.reason).toBe('multiple_subcommands');
|
||||
});
|
||||
|
||||
test('unknown-then-known rejects on the unknown (unknown fires before second-positional check)', () => {
|
||||
const r = translatePositionalSubcommands(['garbage', 'status']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('unknown_subcommand');
|
||||
expect(r.message).toContain('garbage');
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,63 @@
|
||||
/**
|
||||
* #1872 — autopilot SIGTERM/SIGINT must close the engine before exit.
|
||||
*
|
||||
* On PGLite the cycle steps run INLINE in the autopilot process, so a hard
|
||||
* `process.exit` mid-write (systemctl stop → SIGTERM) kills WASM Postgres
|
||||
* with the WAL dirty and can corrupt the brain. Two exit paths must both
|
||||
* close the engine:
|
||||
*
|
||||
* - autopilot's own shutdown() (owns SIGINT + internal stops like
|
||||
* max_crashes / cycle-failure-cap), and
|
||||
* - process-cleanup's SIGTERM handler (installed at cli.ts module load,
|
||||
* which exits within its 3s cleanup deadline) — reached via the
|
||||
* registered 'autopilot-engine-close' cleanup callback.
|
||||
*
|
||||
* Because the shutdown path is deep inside `runAutopilot()` (a long-running
|
||||
* daemon loop that ends in process.exit), a behavioral test would have to
|
||||
* spawn + signal a real daemon. Following the established precedent
|
||||
* (test/autopilot-supervisor-wiring.test.ts, test/autopilot-fanout-wiring.test.ts),
|
||||
* these static-shape regressions pin the load-bearing wiring instead.
|
||||
*/
|
||||
import { describe, expect, it } from 'bun:test';
|
||||
import { readFileSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
|
||||
const AUTOPILOT_SRC = readFileSync(
|
||||
join(import.meta.dir, '..', 'src', 'commands', 'autopilot.ts'),
|
||||
'utf8',
|
||||
);
|
||||
|
||||
describe('autopilot.ts graceful engine shutdown (#1872)', () => {
|
||||
it('registers an engine-close callback in the process-cleanup registry (SIGTERM path)', () => {
|
||||
// process-cleanup owns SIGTERM (installed at cli.ts:10) and hard-exits
|
||||
// after its cleanup pass; without this registration the engine is never
|
||||
// closed on `systemctl stop`.
|
||||
expect(AUTOPILOT_SRC).toContain(
|
||||
"import { registerCleanup } from '../core/process-cleanup.ts';",
|
||||
);
|
||||
expect(AUTOPILOT_SRC).toContain(
|
||||
"registerCleanup('autopilot-engine-close', closeEngine)",
|
||||
);
|
||||
});
|
||||
|
||||
it('closeEngine aborts the in-flight inline cycle then disconnects the engine', () => {
|
||||
// Abort first (runCycle checks the signal between phases and threads it
|
||||
// into phase sub-work), bounded drain, then disconnect.
|
||||
expect(AUTOPILOT_SRC).toMatch(
|
||||
/const closeEngine = async \(\) => \{[\s\S]{0,900}shutdownAbort\.abort\([\s\S]{0,900}engine\.disconnect\(\)/,
|
||||
);
|
||||
});
|
||||
|
||||
it('the inline runCycle call carries the shutdown abort signal and is tracked as in-flight', () => {
|
||||
// PGLite / --inline path: the cycle runs in-process, so shutdown must be
|
||||
// able to (a) signal it to wind down and (b) await it before closing.
|
||||
expect(AUTOPILOT_SRC).toMatch(/signal:\s*shutdownAbort\.signal/);
|
||||
expect(AUTOPILOT_SRC).toMatch(/inflightInlineCycle\s*=\s*cyclePromise/);
|
||||
});
|
||||
|
||||
it('shutdown() awaits closeEngine() before process.exit(0) (SIGINT + internal-stop path)', () => {
|
||||
expect(AUTOPILOT_SRC).toMatch(
|
||||
/await closeEngine\(\);[\s\S]{0,400}process\.exit\(0\)/,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,99 @@
|
||||
/**
|
||||
* #1869 — `gbrain dream --dir <path>` stamps cycle freshness when the path
|
||||
* matches a registered source's local_path.
|
||||
*
|
||||
* Pre-fix, only `--source <id>` runs wrote last_source_cycle_at /
|
||||
* last_full_cycle_at (runCycle's stamp gate reads opts.sourceId, and dream
|
||||
* never derived one from --dir), so a path-scoped brain showed doctor's
|
||||
* cycle_freshness as perpetually stale.
|
||||
*
|
||||
* The fix lives in dream.ts (derive the source id from the resolved brain
|
||||
* dir via resolveSourceForDir), NOT in runCycle's stamp gate — a runCycle-
|
||||
* wide change would make the autopilot-global-maintenance handler (global
|
||||
* phases, brainDir set, no sourceId) falsely stamp per-source freshness
|
||||
* (the #2194 poisoning class; see rejected PR #2549).
|
||||
*
|
||||
* Same real-PGLite/no-mocks discipline as test/dream.test.ts; same
|
||||
* GBRAIN_HOME isolation as test/cycle-last-full-cycle-at.test.ts (the
|
||||
* cycle's PGLite file lock lives under ~/.gbrain).
|
||||
*/
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test';
|
||||
import { mkdtempSync, rmSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { resetPgliteState } from './helpers/reset-pglite.ts';
|
||||
import { runDream } from '../src/commands/dream.ts';
|
||||
import { withEnv } from './helpers/with-env.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
let brainDir: string;
|
||||
let gbrainHome: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
}, 60_000);
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.disconnect();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await resetPgliteState(engine);
|
||||
brainDir = mkdtempSync(join(tmpdir(), 'gbrain-dream-stamp-'));
|
||||
gbrainHome = mkdtempSync(join(tmpdir(), 'gbrain-dream-stamp-home-'));
|
||||
}, 60_000);
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(brainDir, { recursive: true, force: true });
|
||||
rmSync(gbrainHome, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function seedSource(id: string, archived = false): Promise<void> {
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config, archived, created_at)
|
||||
VALUES ($1, $2, $3, '{}'::jsonb, $4, NOW())`,
|
||||
[id, id, brainDir, archived],
|
||||
);
|
||||
}
|
||||
|
||||
async function readLastFullCycleAt(sourceId: string): Promise<string | null> {
|
||||
const rows = await engine.executeRaw<{ config: Record<string, unknown> | null }>(
|
||||
`SELECT config FROM sources WHERE id = $1`,
|
||||
[sourceId],
|
||||
);
|
||||
const raw = rows[0]?.config?.last_full_cycle_at;
|
||||
return typeof raw === 'string' ? raw : null;
|
||||
}
|
||||
|
||||
describe('gbrain dream --dir <path> freshness stamp (#1869)', () => {
|
||||
test('--dir matching a source local_path stamps last_full_cycle_at', async () => {
|
||||
await withEnv({ GBRAIN_HOME: gbrainHome }, async () => {
|
||||
await seedSource('path-scoped');
|
||||
expect(await readLastFullCycleAt('path-scoped')).toBeNull();
|
||||
|
||||
const report = await runDream(engine, ['--dir', brainDir, '--phase', 'lint', '--json']);
|
||||
expect(report).toBeTruthy();
|
||||
if (report) expect(['ok', 'clean']).toContain(report.status);
|
||||
|
||||
// Pre-fix this stays null forever: dream never passed a sourceId, so
|
||||
// runCycle's stamp gate skipped the write.
|
||||
expect(await readLastFullCycleAt('path-scoped')).not.toBeNull();
|
||||
});
|
||||
}, 60_000);
|
||||
|
||||
test('--dir matching an ARCHIVED source does not stamp it', async () => {
|
||||
await withEnv({ GBRAIN_HOME: gbrainHome }, async () => {
|
||||
await seedSource('mothballed', true);
|
||||
|
||||
const report = await runDream(engine, ['--dir', brainDir, '--phase', 'lint', '--json']);
|
||||
expect(report).toBeTruthy();
|
||||
|
||||
// Stamping an archived source would mask data staleness when it is
|
||||
// later restored (mirrors the explicit --source archived guard).
|
||||
expect(await readLastFullCycleAt('mothballed')).toBeNull();
|
||||
});
|
||||
}, 60_000);
|
||||
});
|
||||
+14
-4
@@ -562,12 +562,22 @@ describe('runDream — --source / --source-id (v0.41.13)', () => {
|
||||
|
||||
// ─── Back-compat: bare `gbrain dream` does NOT write per-source stamp ─
|
||||
|
||||
test('gbrain dream (no --source) leaves all sources untouched (back-compat regression)', async () => {
|
||||
await seedSource('alpha');
|
||||
await seedSource('beta');
|
||||
test('gbrain dream (no --source) stamps only the source whose local_path matches --dir (#1869)', async () => {
|
||||
// Pre-#1869 this asserted NO source was ever stamped without an explicit
|
||||
// --source — which is exactly the bug: a path-scoped `gbrain dream --dir`
|
||||
// run never landed a freshness stamp and doctor's cycle_freshness stayed
|
||||
// stale forever. New truth: the source whose local_path matches the
|
||||
// resolved brain dir is derived and stamped; unrelated sources stay
|
||||
// untouched (cross-source isolation).
|
||||
await seedSource('alpha'); // local_path = repo → derived + stamped
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config, archived, created_at)
|
||||
VALUES ($1, $2, $3, '{}'::jsonb, false, NOW())`,
|
||||
['beta', 'beta', '/somewhere/else'],
|
||||
);
|
||||
const report = await runDream(engine, ['--dir', repo, '--phase', 'lint', '--json']);
|
||||
expect(report).toBeTruthy();
|
||||
expect(await readLastFullCycleAt('alpha')).toBeNull();
|
||||
expect(await readLastFullCycleAt('alpha')).not.toBeNull();
|
||||
expect(await readLastFullCycleAt('beta')).toBeNull();
|
||||
}, 60_000);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user