Compare commits

..
Author SHA1 Message Date
Garry TanandClaude Fable 5 6548e5cffc fix(autopilot): add --target to value-flag set so install targets survive positional translation
Review finding on #3103: --target is installDaemon's value flag
(macos | linux-systemd | ephemeral-container | linux-cron). The
translator only knew --repo/--interval, so `gbrain autopilot
--install --target linux-cron` misread the target value as an
unknown positional subcommand and exited 2 before installDaemon ran.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-22 12:05:46 -07:00
0ac3d4da9c fix(autopilot): translate positional subcommands so autopilot status doesn't start the daemon
Takeover/rebase of #1529 onto current master. `gbrain autopilot status`
(and install/uninstall/start) previously fell through the flag-only
branches in runAutopilot and silently started the daemon (lockfile +
worker spawn + sync dispatch). A pure translatePositionalSubcommands()
now maps known positionals to their flag form before any side effect,
is value-flag aware (--repo/--interval), and fails loud (exit 2) on
unknown positionals. 22 tests including the exact #1525 repro.

Fixes #1525

Co-authored-by: Oszkar <Oszkar@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-21 14:28:16 -07:00
7 changed files with 318 additions and 133 deletions
+5 -8
View File
@@ -21,17 +21,14 @@ GBrain is tuned for the Supabase **Transaction pooler** (port 6543): it
auto-disables prepared statements there and routes `engine.transaction()`
(migrations, DDL, sync imports) to a derived **direct** connection
(`db.<ref>.supabase.co:5432`). That direct host is IPv6-only, so on an
IPv4-only host it is unreachable. When that happens gbrain now falls back to
the pooler automatically (one stderr warning, then single-pool mode for the
rest of the process) — but the pooler's ~2-min statement timeout can truncate
very long migrations or bulk imports.
IPv4-only host, reads work but sync **silently skips most pages**. This is the
number one cause of "sync ran but nothing happened."
Fix: make the direct connection reachable over IPv4. Either set
`GBRAIN_DIRECT_DATABASE_URL` to the **Session pooler** string (port 5432 on the
`pooler.supabase.com` host, IPv4), or enable Supabase's IPv4 add-on.
`GBRAIN_DISABLE_DIRECT_POOL=1` skips the direct pool (and the fallback warning)
entirely. Verify by running `gbrain sync` and checking that the page count in
`gbrain stats` matches the syncable file count in the repo.
`pooler.supabase.com` host, IPv4), or enable Supabase's IPv4 add-on. Verify by
running `gbrain sync` and checking that the page count in `gbrain stats` matches
the syncable file count in the repo.
### The Primitives
+5 -8
View File
@@ -2720,17 +2720,14 @@ GBrain is tuned for the Supabase **Transaction pooler** (port 6543): it
auto-disables prepared statements there and routes `engine.transaction()`
(migrations, DDL, sync imports) to a derived **direct** connection
(`db.<ref>.supabase.co:5432`). That direct host is IPv6-only, so on an
IPv4-only host it is unreachable. When that happens gbrain now falls back to
the pooler automatically (one stderr warning, then single-pool mode for the
rest of the process) — but the pooler's ~2-min statement timeout can truncate
very long migrations or bulk imports.
IPv4-only host, reads work but sync **silently skips most pages**. This is the
number one cause of "sync ran but nothing happened."
Fix: make the direct connection reachable over IPv4. Either set
`GBRAIN_DIRECT_DATABASE_URL` to the **Session pooler** string (port 5432 on the
`pooler.supabase.com` host, IPv4), or enable Supabase's IPv4 add-on.
`GBRAIN_DISABLE_DIRECT_POOL=1` skips the direct pool (and the fallback warning)
entirely. Verify by running `gbrain sync` and checking that the page count in
`gbrain stats` matches the syncable file count in the repo.
`pooler.supabase.com` host, IPv4), or enable Supabase's IPv4 add-on. Verify by
running `gbrain sync` and checking that the page count in `gbrain stats` matches
the syncable file count in the repo.
### The Primitives
+109
View File
@@ -151,6 +151,100 @@ 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 {
@@ -363,6 +457,11 @@ 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`.',
@@ -370,6 +469,16 @@ 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;
-6
View File
@@ -1078,9 +1078,6 @@ async function initPostgres(opts: {
console.warn(' Direct connections are IPv6 only and fail in many environments.');
console.warn(' Use the Transaction pooler connection string instead (port 6543):');
console.warn(' Supabase Dashboard > Connect (top bar) > Connection String > Transaction pooler');
console.warn(' (With a pooler URL, gbrain derives a direct connection for DDL and falls back');
console.warn(' to the pooler automatically if that host is unreachable. Power users:');
console.warn(' GBRAIN_DIRECT_DATABASE_URL overrides the derived URL; GBRAIN_DISABLE_DIRECT_POOL=1 disables it.)');
console.warn('');
}
@@ -1094,9 +1091,6 @@ async function initPostgres(opts: {
if (databaseUrl.includes('supabase.co') && (msg.includes('ECONNREFUSED') || msg.includes('ETIMEDOUT'))) {
console.error('Connection failed. Supabase direct connections (db.*.supabase.co:5432) are IPv6 only.');
console.error('Use the Transaction pooler connection string instead (port 6543).');
console.error('(gbrain derives its own direct connection from pooler URLs for DDL; if that host is');
console.error('unreachable it falls back to the pooler. GBRAIN_DIRECT_DATABASE_URL overrides the');
console.error('derived URL; GBRAIN_DISABLE_DIRECT_POOL=1 disables the direct pool entirely.)');
}
throw e;
}
+2 -48
View File
@@ -167,25 +167,6 @@ export function deriveDirectUrl(url: string): string | null {
}
}
/**
* Error codes that mean "the direct host is unreachable from this network"
* (#1641). The auto-derived db.<ref>.supabase.co host is IPv6-only without
* the paid IPv4 add-on, so ENOTFOUND/ECONNREFUSED here is expected on
* IPv4-only networks — we fall back to the pooler instead of failing init.
*/
const NETWORK_UNREACHABLE_CODES = [
'ENOTFOUND', 'ECONNREFUSED', 'ENETUNREACH', 'EHOSTUNREACH',
'ETIMEDOUT', 'CONNECT_TIMEOUT',
];
/** True when err looks like a network-unreachable failure (not auth/SQL). */
export function isNetworkUnreachableError(err: unknown): boolean {
const code = (err as { code?: unknown } | null)?.code;
if (typeof code === 'string' && NETWORK_UNREACHABLE_CODES.includes(code)) return true;
const msg = err instanceof Error ? err.message : String(err);
return NETWORK_UNREACHABLE_CODES.some(c => msg.includes(c));
}
/**
* Read kill-switch state from env. Subordinate to parent manager's state
* when present (A2 inheritance).
@@ -338,30 +319,7 @@ export class ConnectionManager {
throw err;
});
}
let pool: Sql | null;
try {
pool = await this._directInit;
} catch (err) {
// #1641: the derived direct host (db.<ref>.supabase.co) is IPv6-only
// without Supabase's IPv4 add-on. On IPv4-only networks the direct
// pool can never connect — permanently fall back to the read pool
// (self-activating kill-switch) instead of failing init/migrations.
// Non-network errors (auth, SQL) still throw: they mean misconfig,
// not unreachability.
if (isNetworkUnreachableError(err)) {
const alreadyWarned = this._killSwitch;
this._killSwitch = true;
const msg = err instanceof Error ? err.message : String(err);
if (!alreadyWarned) console.error(
`gbrain: direct connection to ${this._directUrl ? this.hostOnly(this._directUrl) : 'unknown host'} unreachable (${msg}); ` +
'falling back to the pooler for DDL/bulk (long migrations may hit the pooler statement timeout). ' +
'Set GBRAIN_DIRECT_DATABASE_URL to a reachable direct URL (e.g. the Session pooler, port 5432) or enable the Supabase IPv4 add-on; ' +
'GBRAIN_DISABLE_DIRECT_POOL=1 silences this.',
);
return this.getReadPool();
}
throw err;
}
const pool = await this._directInit;
if (!pool) {
// Defensive — initDirectPool should have thrown.
throw new Error('connection-manager: direct pool init returned null');
@@ -392,9 +350,8 @@ export class ConnectionManager {
},
};
const t0 = Date.now();
let pool: Sql | null = null;
try {
pool = postgres(this._directUrl, opts);
const pool = postgres(this._directUrl, opts);
// Probe to validate connectivity early.
await pool`SELECT 1`;
logConnectionEvent({
@@ -405,9 +362,6 @@ export class ConnectionManager {
});
return pool;
} catch (err) {
// Don't leak the failed pool's sockets/timers (#1641 fallback keeps
// the process running afterward).
if (pool) await endPoolBounded(pool);
logConnectionEvent({
pool: 'ddl',
op: 'error',
@@ -0,0 +1,197 @@
/**
* 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');
}
});
});
-63
View File
@@ -3,7 +3,6 @@ import {
isSupabasePoolerUrl,
deriveDirectUrl,
readKillSwitchEnv,
isNetworkUnreachableError,
resolveDirectPoolSize,
ConnectionManager,
DEFAULT_DIRECT_POOL_SIZE,
@@ -239,65 +238,3 @@ describe('ConnectionManager — parent inheritance (A2)', () => {
}
});
});
describe('isNetworkUnreachableError (#1641)', () => {
test('classifies network codes as unreachable', () => {
for (const code of ['ENOTFOUND', 'ECONNREFUSED', 'ENETUNREACH', 'EHOSTUNREACH', 'ETIMEDOUT', 'CONNECT_TIMEOUT']) {
const err = Object.assign(new Error('connect failed'), { code });
expect(isNetworkUnreachableError(err)).toBe(true);
}
});
test('classifies by message when code absent', () => {
expect(isNetworkUnreachableError(new Error('getaddrinfo ENOTFOUND db.abc.supabase.co'))).toBe(true);
});
test('auth/SQL errors are NOT unreachable', () => {
expect(isNetworkUnreachableError(new Error('password authentication failed for user "postgres"'))).toBe(false);
expect(isNetworkUnreachableError(new Error('syntax error at or near "SELEC"'))).toBe(false);
expect(isNetworkUnreachableError(null)).toBe(false);
});
});
describe('ConnectionManager — direct-pool fallback on unreachable host (#1641)', () => {
let originalKillSwitch: string | undefined;
let originalError: typeof console.error;
let errLines: string[];
beforeEach(() => {
originalKillSwitch = process.env.GBRAIN_DISABLE_DIRECT_POOL;
delete process.env.GBRAIN_DISABLE_DIRECT_POOL;
originalError = console.error;
errLines = [];
console.error = (...args: unknown[]) => { errLines.push(args.join(' ')); };
});
afterEach(() => {
console.error = originalError;
if (originalKillSwitch === undefined) delete process.env.GBRAIN_DISABLE_DIRECT_POOL;
else process.env.GBRAIN_DISABLE_DIRECT_POOL = originalKillSwitch;
});
test('ddl() falls back to the read pool when the direct host is unreachable', async () => {
const cm = new ConnectionManager({
url: 'postgresql://postgres.abc:p@aws.pooler.supabase.com:6543/db',
// 127.0.0.1:9 (discard) → instant ECONNREFUSED, the IPv4-only-network shape.
directUrl: 'postgresql://postgres:p@127.0.0.1:9/db',
});
const fakeReadPool = {} as ReturnType<typeof ConnectionManager.prototype.read>;
cm.setReadPool(fakeReadPool);
expect(cm.isDualPoolActive()).toBe(true);
const pool = await cm.ddl(); // without the fix this throws ECONNREFUSED
expect(pool).toBe(fakeReadPool);
// Self-activating kill-switch: subsequent calls skip the direct pool.
expect(cm.isKillSwitchActive()).toBe(true);
expect(cm.isDualPoolActive()).toBe(false);
expect(cm.describeMode().mode).toBe('single (kill-switch)');
// One stderr line mentioning the power-user override.
const warning = errLines.filter(l => l.includes('GBRAIN_DIRECT_DATABASE_URL'));
expect(warning.length).toBe(1);
const again = await cm.ddl();
expect(again).toBe(fakeReadPool);
expect(errLines.filter(l => l.includes('GBRAIN_DIRECT_DATABASE_URL')).length).toBe(1);
}, 20000);
});