From 62bbd3c077be2dc0009a55b5462a647e2b22c43b Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 12 Aug 2026 12:27:15 -0700 Subject: [PATCH] fix(bootstrap): address two-model adversarial review of the DX wave MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the regressions the 5-specialist + red-team + Claude/Codex adversarial pass found in the F1–F21 changes, each with a test: - Keyless upgrade hint pointed at `config set embedding_model`, which config.ts hard-refuses as a schema-sizing no-op — now names the working re-init recipe (`gbrain init --force --pglite --embedding-model `), zero-key AND multi-key paths. - Multi-key TTY picker offered "continue keyless" but the caller aborted on it — now honors keyless like the zero-key path. - Detached update-refresh spawn used a `/gbrain$/` basename check that misfires for a renamed/official-named compiled binary (`gbrain-darwin-arm64`) and prepends the /$bunfs entrypoint — now detects dev-vs-compiled by the runtime basename (bun|node) so the refresh always runs. - `bootstrap status` reported the wire phase "done" on a hooks-only receipt (host CLI missing at wire time) — now "partial" with a re-run hint, so a resuming agent doesn't trust a false complete. - Post-repair MCP mismatch re-verifies and aborts instead of blessing a registration a racing writer may have re-claimed. - probeOpenAICompat's abort timer now spans the body read (was cleared before it), so a stalled `/v1/models` body can't hang init past the 1s cap. - Centralized the 4-copy stale-cache upgrade predicate into `pendingUpgradeVersion`; UPGRADE_AVAILABLE gains a GBRAIN_FORCE_UPGRADE_MARKER override for PTY-based agent harnesses. - Mode picker's expansion-key gate adds GEMINI_API_KEY; picker prompt is article-aware ("an embedding" / "a chat"); dead `!brainEmpty` clause removed; migrate.ts try/finally widened + stamp failures named in quiet mode. - DX harness: credential copies scrubbed even on SIGINT/interrupt (+chmod 600), child process TREE reaped on teardown, advisory made fail-open, KEY_MAP typed as a literal union. New tests: migrate quiet-replay, self-upgrade pending predicate + negative cache cases, bootstrap 127/scoped-remove/broken-settings dispatch, interview invalidation flag, verify tour-withheld-on-FAIL, init keyless/supabase/multi-key, init-nudge branches, ai-probes model parsing. Regenerated flag registry + template-repo. Co-Authored-By: Claude Fable 5 --- docs/guides/upgrades-auto-update.md | 7 +- scripts/dx-explore.ts | 74 +++++++++-- src/cli.ts | 41 +++--- src/commands/bootstrap.ts | 38 ++++-- src/commands/doctor.ts | 17 +-- src/commands/init-mode-picker.ts | 3 +- src/commands/init-provider-picker.ts | 21 +-- src/commands/init.ts | 21 ++- src/core/advisor/collect-version.ts | 22 +-- src/core/ai/probes.ts | 10 +- src/core/bootstrap/status.ts | 15 ++- src/core/cli-flag-registry.generated.ts | 6 +- src/core/migrate.ts | 17 ++- src/core/onboard/init-nudge.ts | 8 +- src/core/operations.ts | 15 +-- src/core/self-upgrade.ts | 25 +++- src/core/skillpack/post-install-advisory.ts | 73 ++++++---- templates/bootstrap/template-repo/README.md | 2 +- test/ai-probes-models.test.ts | 69 ++++++++++ test/bootstrap-dispatcher.serial.test.ts | 135 ++++++++++++++++++- test/bootstrap-interview.serial.test.ts | 43 ++++++ test/bootstrap-verify.serial.test.ts | 9 ++ test/cli-dispatch-thin-client.test.ts | 4 + test/doctor-self-upgrade.test.ts | 14 +- test/e2e/init-fresh-pglite.test.ts | 49 ++++++- test/e2e/self-upgrade-marker.test.ts | 15 +++ test/helpers/tty-harness.ts | 55 +++++--- test/init-nudge.test.ts | 140 ++++++++++++++++++++ test/migrate-quiet-replay.serial.test.ts | 140 ++++++++++++++++++++ test/post-install-advisory.test.ts | 68 ++++++++++ test/self-upgrade-pending.test.ts | 128 ++++++++++++++++++ test/tty-harness.test.ts | 4 +- 32 files changed, 1122 insertions(+), 166 deletions(-) create mode 100644 test/ai-probes-models.test.ts create mode 100644 test/init-nudge.test.ts create mode 100644 test/migrate-quiet-replay.serial.test.ts create mode 100644 test/self-upgrade-pending.test.ts diff --git a/docs/guides/upgrades-auto-update.md b/docs/guides/upgrades-auto-update.md index 8d6b13de6..52d700d5a 100644 --- a/docs/guides/upgrades-auto-update.md +++ b/docs/guides/upgrades-auto-update.md @@ -20,7 +20,12 @@ schema. The user gets new capabilities automatically. gbrain stays current the way gstack does: it rides invocation frequency. A throttled, cache-read-only check runs at the start of every `gbrain` invocation -(CLI and MCP) and emits an `UPGRADE_AVAILABLE ` marker on stderr. No +(CLI and MCP) and emits an `UPGRADE_AVAILABLE ` marker on stderr. The +raw marker line is suppressed when stderr is an interactive TTY (a human sees +only the plain `gbrain X -> Y available` sentence, not the machine token); set +`GBRAIN_FORCE_UPGRADE_MARKER=1` if an agent harness parses the token but runs +under a PTY. `` is always the RUNNING binary's version, so a stale or +foreign-written cache never nags about an upgrade this binary already has. No host cron required — every agent kind (Claude Code, Codex, OpenClaw, Hermes, the `gbrain serve` host behind a Perplexity thin client) converges to current by construction. The behavior is governed by one file-plane config key, diff --git a/scripts/dx-explore.ts b/scripts/dx-explore.ts index 963280566..52cf65989 100644 --- a/scripts/dx-explore.ts +++ b/scripts/dx-explore.ts @@ -9,7 +9,11 @@ * transcripts land in .context/dx-runs/ (gitignored) and nothing asserts. * * Scenarios (all hermetic — temp HOME/GBRAIN_HOME/CLAUDE_CONFIG_DIR/CODEX_HOME; - * the operator's real config is never read or written): + * the operator's real config is never WRITTEN. Two narrow reads exist for + * auth: codex-install copies ~/.codex/auth.json into the temp CODEX_HOME, and + * the claude seed records the API key's last 20 chars — both copies are + * scrubbed at cleanup even under --keep, so no credential material outlives + * the run): * * help First-touch comprehension surfaces: bare `gbrain`, * `gbrain --help`, `gbrain init --help`, `gbrain bootstrap @@ -65,13 +69,20 @@ import { saveTranscript, seedClaudeTuiConfig, parseDriveCommand, - stripAnsi, type TtySession, - type PtyFrame, } from '../test/helpers/tty-harness.ts'; const REPO_ROOT = path.resolve(import.meta.dir, '..'); +/** Screen patterns that mean the paste-in install reached a passing verify — + * ONE list shared by the claude-install and codex-install scenarios so the + * two can't drift when the bootstrap's success copy changes. */ +const VERIFY_SUCCESS_PATTERNS: Array = [ + /bootstrap verify.*exit(?:ed|s)? 0/i, + /verify\b.*\b(passed|0\b)/i, + /All checks passed/i, +]; + // Same synthetic persona the door tests use — the interview can complete // unattended and nothing real about the operator ever enters a transcript. const PERSONA = { @@ -187,6 +198,10 @@ interface ScenarioCtx { keep: boolean; /** temp dirs to remove on completion unless --keep */ cleanups: string[]; + /** Files carrying credential material (copied auth.json, seeded key + * suffixes). ALWAYS deleted at cleanup — --keep keeps transcripts and + * hermetic dirs for forensics, never credentials. */ + secretPaths: string[]; events: Array<{ tMs: number; kind: 'input' | 'note' | 'screen'; data: string }>; t0: number; } @@ -196,14 +211,17 @@ function newCtx(args: CliArgs, needsGbrain: boolean): ScenarioCtx { args.dir ?? path.join(REPO_ROOT, '.context', 'dx-runs', `${args.scenario}-${nowStamp()}`), ); fs.mkdirSync(outDir, { recursive: true }); - return { + const ctx: ScenarioCtx = { outDir, gbrainBin: needsGbrain ? ensureGbrainBinary(args.gbrainBin, args.rebuild) : '', keep: args.keep, cleanups: [], + secretPaths: [], events: [], t0: Date.now(), }; + installSignalScrub(ctx); + return ctx; } function tmp(ctx: ScenarioCtx, prefix: string): string { @@ -216,11 +234,43 @@ function event(ctx: ScenarioCtx, kind: 'input' | 'note' | 'screen', data: string ctx.events.push({ tMs: Date.now() - ctx.t0, kind, data }); } +/** Delete every credential copy. Idempotent; safe to call from a signal + * handler AND from finishCtx (a second call is a no-op). This is the + * "no credential outlives the run" guarantee — it must run even when a + * 10-25min install is Ctrl-C'd (finally does NOT run on SIGINT default). */ +function scrubSecrets(ctx: ScenarioCtx): void { + for (const p of ctx.secretPaths) { + try { + fs.rmSync(p, { force: true }); + } catch { + /* best-effort */ + } + } +} + +/** Wire SIGINT/SIGTERM so an interrupted run still scrubs credentials before + * the process dies. Registered once per scenario ctx. */ +function installSignalScrub(ctx: ScenarioCtx): void { + const handler = (sig: NodeJS.Signals) => { + scrubSecrets(ctx); + process.stderr.write(`\n[dx-explore] ${sig}: scrubbed credential copies, exiting.\n`); + process.exit(130); + }; + process.once('SIGINT', handler); + process.once('SIGTERM', handler); +} + function finishCtx(ctx: ScenarioCtx): void { + // Scrub credentials FIRST — before any other I/O that could throw (an + // events.jsonl write failure must not strand auth files). + scrubSecrets(ctx); fs.writeFileSync( path.join(ctx.outDir, 'events.jsonl'), ctx.events.map((e) => JSON.stringify(e)).join('\n') + (ctx.events.length ? '\n' : ''), ); + if (ctx.keep && ctx.secretPaths.length > 0) { + log(`--keep: retained hermetic dirs, but scrubbed ${ctx.secretPaths.length} credential file(s)`); + } if (!ctx.keep) { for (const d of ctx.cleanups) { try { @@ -244,8 +294,6 @@ function finishCtx(ctx: ScenarioCtx): void { function mirrorSession(dir: string, session: TtySession): () => void { const sessDir = path.join(dir, 'session'); fs.mkdirSync(sessDir, { recursive: true }); - const framesPath = path.join(sessDir, 'frames.jsonl'); - fs.writeFileSync(framesPath, ''); const timer = setInterval(() => { try { fs.writeFileSync(path.join(sessDir, 'screen.txt'), session.visible().slice(-8000)); @@ -448,6 +496,9 @@ async function scenarioClaudeInstall(ctx: ScenarioCtx): Promise { // against the resolved path, so an unresolved seed misses. trustedDirs: [ws, fs.realpathSync(ws)], }); + // The seed records the key's last 20 chars — credential-adjacent, so it is + // scrubbed at cleanup even with --keep. + ctx.secretPaths.push(path.join(cfg, '.claude.json')); log('REAL interactive claude running the paste-in bootstrap (10-25 min, real API cost)'); log(`watch live: cat ${path.join(ctx.outDir, 'session', 'screen.txt')}`); @@ -478,7 +529,7 @@ async function scenarioClaudeInstall(ctx: ScenarioCtx): Promise { // Run until verify-success copy or exit or wall clock. const done = await Promise.race([ session - .waitForAny([/bootstrap verify.*exit(?:ed|s)? 0/i, /verify\b.*\b(passed|0\b)/i, /All checks passed/i], { + .waitForAny(VERIFY_SUCCESS_PATTERNS, { timeoutMs: 1_500_000, }) .then(() => 'verify-signal') @@ -511,7 +562,12 @@ async function scenarioCodexInstall(ctx: ScenarioCtx): Promise { const codexHome = path.join(home, '.codex'); fs.mkdirSync(codexHome, { recursive: true }); const realAuth = path.join(os.homedir(), '.codex', 'auth.json'); - if (fs.existsSync(realAuth)) fs.copyFileSync(realAuth, path.join(codexHome, 'auth.json')); + if (fs.existsSync(realAuth)) { + const authCopy = path.join(codexHome, 'auth.json'); + fs.copyFileSync(realAuth, authCopy); + fs.chmodSync(authCopy, 0o600); // copyFileSync doesn't preserve source mode + ctx.secretPaths.push(authCopy); // scrubbed at cleanup, even with --keep + } spawnSync('git', ['init', '-q', ws]); spawnSync('git', ['-C', ws, 'config', 'user.email', 'dx@example.com']); spawnSync('git', ['-C', ws, 'config', 'user.name', 'DX Explore']); @@ -541,7 +597,7 @@ async function scenarioCodexInstall(ctx: ScenarioCtx): Promise { session.sendKey('Enter'); const done = await Promise.race([ session - .waitForAny([/bootstrap verify.*exit(?:ed|s)? 0/i, /verify\b.*\b(passed|0\b)/i, /All checks passed/i], { + .waitForAny(VERIFY_SUCCESS_PATTERNS, { timeoutMs: 1_500_000, }) .then(() => 'verify-signal') diff --git a/src/cli.ts b/src/cli.ts index 868265536..d3f5d760e 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -14,6 +14,7 @@ import { spawn } from 'child_process'; import { readUpdateCache, isCacheFresh, + pendingUpgradeVersion, readSnooze, isSnoozeActive, resolveSelfUpgradeMode, @@ -36,7 +37,6 @@ import { callRemoteTool, RemoteMcpError, unpackToolResult } from './core/mcp-cli import { maybePromptForUpgrade } from './core/thin-client-upgrade-prompt.ts'; import { CLI_FLAG_REGISTRY } from './core/cli-flag-registry.generated.ts'; import { VERSION } from './version.ts'; -import { isNewerVersion } from './core/semver.ts'; // Build CLI name -> operation lookup const cliOps = new Map(); @@ -260,29 +260,26 @@ function maybeEmitUpdateMarker(command: string): void { const now = Date.now(); const entry = readUpdateCache(); if (entry && isCacheFresh(entry, now)) { - // Guard against a stale/foreign cache: the cache records the version of - // whatever binary WROTE it (an older gbrain on PATH can write it via the - // detached refresh). Compare the RUNNING binary to latest, and print the - // running version — otherwise a freshly-upgraded binary nags its user - // with "old -> latest available" that self-upgrade cannot satisfy. - if ( - entry.marker.kind === 'upgrade_available' && - entry.marker.latest && - isNewerVersion(VERSION, entry.marker.latest) - ) { + // Shared stale/foreign-cache guard (pendingUpgradeVersion): only nag when + // the cached latest is strictly newer than the RUNNING binary, and print + // the running version — the cache records whatever binary WROTE it. + const latest = pendingUpgradeVersion(VERSION, now); + if (latest) { // notify mode honors a per-version snooze; auto mode ignores it. - if (mode === 'notify' && isSnoozeActive(readSnooze(), entry.marker.latest, now)) return; + if (mode === 'notify' && isSnoozeActive(readSnooze(), latest, now)) return; // The raw `UPGRADE_AVAILABLE ` line is a MACHINE marker // (parsed by the self-upgrade skill / MCP via parseMarker). A human at // an interactive terminal should never see the token as the literal // first line of output — so emit it only when stderr is NOT a TTY // (agent harnesses capture stderr non-interactively and still get it). - // The human sentence prints on both. - if (!process.stderr.isTTY) { - process.stderr.write(`UPGRADE_AVAILABLE ${VERSION} ${entry.marker.latest}\n`); + // GBRAIN_FORCE_UPGRADE_MARKER=1 forces it for the rarer agent harness + // that allocates a PTY yet still parses the token. The human sentence + // prints on both. + if (!process.stderr.isTTY || process.env.GBRAIN_FORCE_UPGRADE_MARKER === '1') { + process.stderr.write(`UPGRADE_AVAILABLE ${VERSION} ${latest}\n`); } process.stderr.write( - `gbrain ${VERSION} -> ${entry.marker.latest} available. Run: gbrain self-upgrade\n`, + `gbrain ${VERSION} -> ${latest} available. Run: gbrain self-upgrade\n`, ); } return; @@ -297,9 +294,15 @@ function maybeEmitUpdateMarker(command: string): void { try { const exec = process.execPath ?? ''; const refreshArgs = ['check-update', '--refresh-cache']; - // Compiled binary: execPath IS gbrain. Dev (bun src/cli.ts): re-exec the - // entrypoint. - const argv = /[/\\]gbrain(\.exe)?$/.test(exec) ? refreshArgs : [process.argv[1], ...refreshArgs]; + // Detect compiled-vs-dev by the RUNTIME's basename, not our own — a + // published binary keeps its official name (`gbrain-darwin-arm64`, a + // `gb` shim), so matching `/gbrain$/` on execPath would misfire and + // prepend the `/$bunfs/root/...` virtual entrypoint (process.argv[1] in + // a compiled Bun binary), producing an unknown-command child that never + // refreshes. Dev mode runs under `bun`/`node`; anything else IS the + // compiled binary and re-execs itself directly. + const isDevRuntime = /[/\\](bun|node)(\.exe)?$/.test(exec); + const argv = isDevRuntime ? [process.argv[1], ...refreshArgs] : refreshArgs; const child = spawn(exec, argv, { detached: true, stdio: 'ignore', diff --git a/src/commands/bootstrap.ts b/src/commands/bootstrap.ts index 512356197..b42f4169e 100644 --- a/src/commands/bootstrap.ts +++ b/src/commands/bootstrap.ts @@ -383,6 +383,15 @@ async function runStatus(ws: string, rest: string[], home: string): Promise` again before render.', + ); +} + async function runInterview(ws: string, rest: string[]): Promise { if (rest.includes('--init')) { const r = initState(ws); @@ -424,12 +433,7 @@ async function runInterview(ws: string, rest: string[]): Promise { console.log(`${key}: routed to the 0600 config file (${routed.configKey}). Not recorded in interview state.`); return 0; } - if (r.invalidatedConfirmation) { - console.error( - 'note: this change voided the prior confirmation — read the full answer set back ' + - 'to the human, then `gbrain bootstrap interview --confirm ` again before render.', - ); - } + if (r.invalidatedConfirmation) warnInvalidatedConfirmation(); console.log(`${key} recorded.`); return 0; } @@ -445,12 +449,7 @@ async function runInterview(ws: string, rest: string[]): Promise { console.error(r.message); return 1; } - if (r.invalidatedConfirmation) { - console.error( - 'note: this change voided the prior confirmation — read the full answer set back ' + - 'to the human, then `gbrain bootstrap interview --confirm ` again before render.', - ); - } + if (r.invalidatedConfirmation) warnInvalidatedConfirmation(); console.log(`${key} skipped.`); return 0; } @@ -784,6 +783,21 @@ async function runHooks(ws: string, rest: string[], home: string, runner: ExecRu console.error(`MCP re-registration failed (${argv.join(' ')}): ${re.stderr.trim() || `exit ${re.code}`}`); return 1; } + // Re-add can itself return "already exists" if a racing writer + // re-claimed the name between our remove and add — that registration + // is NOT ours. Re-verify and abort rather than bless a foreign + // endpoint that would intercept memory ops. (Only the recorded + // warn-then-continue step-2 smoke did this before; here it's fatal.) + const post = await verifyMcpTargetsWorkspace(runner, harness, mcpName, gbrainBin, sourceId); + if (post === 'mismatch') { + console.error( + `after replacing '${mcpName}', it STILL targets a different workspace/binary — ` + + `refusing to continue (a racing registration may have re-claimed the name). ` + + `Inspect \`${argv[0]} mcp get ${mcpName}\`, remove it by hand, then re-run ` + + `\`gbrain bootstrap hooks --harness ${harness} --repair\`.`, + ); + return 1; + } } else { console.log( `MCP server '${mcpName}' already registered — could not confirm it targets this workspace ` + diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 7dd7b7577..04e46023d 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -77,7 +77,6 @@ import { probeLivePgliteHolder, resolveBrainDataDir } from '../core/bootstrap/un import { readRunbookStamp, hooksInstalled, listVerifyRuns } from '../core/bootstrap/status.ts'; import { resolveGbrainHome } from '../core/gbrain-home.ts'; import { VERSION as GBRAIN_BINARY_VERSION } from '../version.ts'; -import { isNewerVersion } from '../core/semver.ts'; import { execFileSync } from 'child_process'; export interface Check { @@ -1151,8 +1150,7 @@ export function checkSelfUpgradeHealth(): Check { const { loadConfig } = require('../core/config.ts'); const { resolveSelfUpgradeMode, - readUpdateCache, - isCacheFresh, + pendingUpgradeVersion, } = require('../core/self-upgrade.ts'); const { readRecentSelfUpgrades } = require('../core/audit/self-upgrade-audit.ts'); @@ -1167,14 +1165,11 @@ export function checkSelfUpgradeHealth(): Check { } const parts: string[] = [`mode=${mode}`]; - const entry = readUpdateCache(); - // Compare against the RUNNING binary (not the cache-writer's recorded - // version) so a stale/foreign cache can't report an already-done upgrade. - if ( - entry && isCacheFresh(entry, Date.now()) && entry.marker.kind === 'upgrade_available' && - entry.marker.latest && isNewerVersion(GBRAIN_BINARY_VERSION, entry.marker.latest) - ) { - parts.push(`update available: ${GBRAIN_BINARY_VERSION} -> ${entry.marker.latest} (run: gbrain self-upgrade)`); + // Shared stale/foreign-cache guard: only report an upgrade strictly newer + // than the RUNNING binary (pendingUpgradeVersion owns the rule). + const pendingLatest = pendingUpgradeVersion(GBRAIN_BINARY_VERSION, Date.now()); + if (pendingLatest) { + parts.push(`update available: ${GBRAIN_BINARY_VERSION} -> ${pendingLatest} (run: gbrain self-upgrade)`); } const failedVersions: string[] = cfg?.self_upgrade?.failed_versions ?? []; if (failedVersions.length > 0) { diff --git a/src/commands/init-mode-picker.ts b/src/commands/init-mode-picker.ts index 6c62abf1c..6c3f7cd93 100644 --- a/src/commands/init-mode-picker.ts +++ b/src/commands/init-mode-picker.ts @@ -117,7 +117,8 @@ async function resolveInputs(engine: BrainEngine): Promise { hasExpansionKey: Boolean( process.env.ANTHROPIC_API_KEY || process.env.OPENAI_API_KEY || - process.env.GOOGLE_GENERATIVE_AI_API_KEY, + process.env.GOOGLE_GENERATIVE_AI_API_KEY || + process.env.GEMINI_API_KEY, // gateway accepts GEMINI_API_KEY as a first-class alias ), pageCount, }; diff --git a/src/commands/init-provider-picker.ts b/src/commands/init-provider-picker.ts index 44781b479..f1a163749 100644 --- a/src/commands/init-provider-picker.ts +++ b/src/commands/init-provider-picker.ts @@ -15,13 +15,15 @@ * probe-gates LOCAL daemons (ollama): daemon-up ≠ model-pulled, so an * unreachable daemon is dropped and a missing model is annotated with * its `ollama pull` fix inline. - * - Embedding pickers always offer `0) none — continue keyless`; when no - * KEYED provider is ready, keyless is the default, so a bare Enter (or - * the 60s timeout) can never select a local daemon the user didn't ask - * for. - * - On Ctrl-D / EOF / timeout / explicit skip: returns null; the - * embedding caller continues keyless with a loud notice (other - * touchpoints treat null as no-pick). + * - Embedding pickers always offer `0) none — continue keyless`. When no + * KEYED provider is ready, keyless is the DEFAULT (bare Enter / 60s + * timeout / EOF all resolve to 0 → null), so a local daemon is never + * auto-selected. When a keyed provider IS ready the default is `1`, so an + * unattended timeout picks that first keyed provider — NOT null; explicit + * `0` is still keyless. + * - Returns null on the keyless choice (and on invalid input); the embedding + * caller continues keyless with a loud notice on BOTH the zero-key and the + * multi-key paths (other touchpoints treat null as no-pick). * - When the user picks a non-Anthropic chat-capable recipe AND * `ANTHROPIC_API_KEY` is missing, prints the subagent caveat from D7 * BEFORE returning the choice so the user sees the implication. @@ -163,7 +165,10 @@ export async function pickProvider(opts: PickProviderOpts): Promise 0) writeStderr(formatRecipeTable(ready, env) + '\n\n'); // Build numbered options (0 = keyless skip for embedding). diff --git a/src/commands/init.ts b/src/commands/init.ts index bacd08035..78a55f729 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -497,7 +497,8 @@ function printNoEmbeddingProviderHint(typos: Array<{ userSet: string; suggested: console.error('\nNo embedding provider configured.'); console.error('Continue without one (keyless — keyword search + memory your agent writes):'); console.error(' gbrain init --pglite --no-embedding'); - console.error(' (add a key later with `gbrain config set embedding_model `)'); + console.error(' (enable semantic search later by re-running with a key:'); + console.error(' gbrain init --force --pglite --embedding-model )'); console.error(''); console.error('Or set a key for semantic search:'); console.error(' export OPENAI_API_KEY=sk-… # openai:text-embedding-3-large (1536d)'); @@ -516,13 +517,16 @@ function printNoEmbeddingProviderHint(typos: Array<{ userSet: string; suggested: } } -/** Loud keyless-continue notice for the no-keys default path. */ +/** Loud keyless-continue notice for the no-keys default path. The upgrade + * command is `init --force` re-init, NOT `config set embedding_model` — that + * key is a schema-sizing file-plane field that `gbrain config set` refuses + * (it would be a silent no-op), so pointing users there is a dead end. */ function printKeylessContinueNotice(): void { console.error( 'No embedding provider keys detected — continuing in keyless mode:\n' + ' keyword search + memory your agent writes down itself. Everything works.\n' + - ' One optional key upgrades search to semantic (import with `gbrain import --no-embed`\n' + - ' meanwhile). Add later: `gbrain config set embedding_model `.', + ' One optional key upgrades search to semantic — set the key, then re-run\n' + + ' `gbrain init --force --pglite --embedding-model ` (re-imports via `gbrain sync`).', ); } @@ -618,8 +622,13 @@ async function resolveEmbeddingByEnv(out: ResolvedAIOptions, nonInteractive: boo const { pickProvider } = await import('./init-provider-picker.ts'); const picked = await pickProvider({ touchpoint: 'embedding', env: process.env, isTTY: true }); if (!picked) { - console.error('Init aborted: no embedding provider picked.'); - process.exit(1); + // The embedding picker offers an explicit "0) none — continue keyless" + // option (and returns null on it). Honor that instead of aborting: a user + // with multiple keys who deliberately chose keyless gets keyless, matching + // the zero-key path. (Ctrl-D / EOF / invalid also land here → keyless.) + printKeylessContinueNotice(); + out.noEmbedding = true; + return; } out.embedding_model = picked.fullModel; out.embedding_dimensions = picked.dim; diff --git a/src/core/advisor/collect-version.ts b/src/core/advisor/collect-version.ts index 90f273ebe..86563ce5e 100644 --- a/src/core/advisor/collect-version.ts +++ b/src/core/advisor/collect-version.ts @@ -6,28 +6,16 @@ * the self-upgrade refresh path. */ -import { readUpdateCache, isCacheFresh } from '../self-upgrade.ts'; -import { isNewerVersion } from '../semver.ts'; +import { pendingUpgradeVersion } from '../self-upgrade.ts'; import type { AdvisorCollector } from './types.ts'; export const collectVersion: AdvisorCollector = { id: 'version', collect: async (ctx) => { - let latest: string | undefined; - try { - const entry = readUpdateCache(); - // Fresh cache only, and compare against the RUNNING version — a stale - // or foreign-binary cache must not nag about an upgrade already done. - if ( - entry && isCacheFresh(entry, Date.now()) && - entry.marker.kind === 'upgrade_available' && entry.marker.latest && - isNewerVersion(ctx.version, entry.marker.latest) - ) { - latest = entry.marker.latest; - } - } catch { - return []; - } + // Shared stale/foreign-cache guard: fresh cache only, and only an upgrade + // strictly newer than the RUNNING version (pendingUpgradeVersion owns the + // rule; never throws). + const latest = pendingUpgradeVersion(ctx.version, Date.now()); if (!latest) return []; return [ { diff --git a/src/core/ai/probes.ts b/src/core/ai/probes.ts index b265c34ac..ab138f501 100644 --- a/src/core/ai/probes.ts +++ b/src/core/ai/probes.ts @@ -26,9 +26,15 @@ export async function probeOpenAICompat(baseUrl: string, timeoutMs: number = 100 signal: controller.signal, headers: { accept: 'application/json' }, }); - clearTimeout(timer); - if (!res.ok) return { reachable: true, models_endpoint_valid: false, error: `HTTP ${res.status}` }; + if (!res.ok) { + clearTimeout(timer); + return { reachable: true, models_endpoint_valid: false, error: `HTTP ${res.status}` }; + } + // Keep the abort timer live through the BODY read — a daemon that accepts, + // returns headers, then stalls the body would otherwise hang past the + // advertised timeout (the probe sits on init's interactive critical path). const body = await res.json().catch(() => null); + clearTimeout(timer); if (!body || typeof body !== 'object') { return { reachable: true, models_endpoint_valid: false, error: 'non-JSON response' }; } diff --git a/src/core/bootstrap/status.ts b/src/core/bootstrap/status.ts index 770119ccd..3528213e1 100644 --- a/src/core/bootstrap/status.ts +++ b/src/core/bootstrap/status.ts @@ -244,7 +244,20 @@ export const PHASES: PhaseSpec[] = [ detect: (ws, ctx) => { const regs = ctx.receipt?.registrations ?? []; if (regs.length > 0) { - return { state: 'done', detail: regs.map((r) => `${r.host} (${r.scope})`).join(', ') }; + // A registration whose detail carries 'mcp' ('mcp' or 'mcp+hooks') + // means MCP actually registered. A 'hooks'-only detail means the host + // binary was missing at wire time (hooks landed, MCP did not) — the + // phase is PARTIAL, not done, so a resuming agent re-runs it once the + // CLI is on PATH instead of trusting a false "done". + const mcpRegistered = regs.some((r) => (r.detail ?? '').includes('mcp')); + if (mcpRegistered) { + return { state: 'done', detail: regs.map((r) => `${r.host} (${r.scope})`).join(', ') }; + } + return { + state: 'partial', + detail: 'hooks installed but MCP not registered (the harness CLI was not on PATH) — ' + + 're-run `gbrain bootstrap hooks --harness ` once it is', + }; } if (hooksInstalled(ws)) return { state: 'done', detail: 'hooks present in .claude/settings.local.json' }; return { state: 'pending' }; diff --git a/src/core/cli-flag-registry.generated.ts b/src/core/cli-flag-registry.generated.ts index 663ed418d..940c74034 100644 --- a/src/core/cli-flag-registry.generated.ts +++ b/src/core/cli-flag-registry.generated.ts @@ -52,7 +52,7 @@ export const CLI_FLAG_REGISTRY: Record = { 'graph-query': ['--aliases', '--all', '--brain', '--depth', '--direction', '--explain', '--fast', '--force', '--from-pages', '--help', '--http', '--include-foreign', '--include-null-signature', '--json', '--lang', '--markdown', '--mcp-only', '--multimodal', '--near-symbol', '--no-embedding', '--no-extract', '--pattern', '--pending', '--reset', '--resolve', '--restore-only', '--source', '--stale', '--supersessions', '--surface', '--symbol-kind', '--thin', '--timeout', '--type'], 'hook': ['--aliases', '--all', '--batch-limit', '--brain', '--budget-ms', '--count', '--delete-brain', '--detach', '--env', '--fast', '--force', '--from-pages', '--harness', '--help', '--http', '--include-null-signature', '--json', '--no-embedding', '--no-extract', '--once', '--path', '--pattern', '--pending', '--porcelain', '--reset', '--resolve', '--show-toplevel', '--source', '--stale', '--stats', '--supersessions', '--surface', '--thin', '--timeout'], 'import': ['--aliases', '--all', '--asof', '--background', '--brain', '--brain-wide-max-cost-usd', '--by-mention', '--cached', '--code', '--compile', '--concurrency', '--embedding-dimensions', '--embedding-model', '--exclude', '--exclude-standard', '--explain', '--fast', '--federated', '--fix', '--follow', '--force', '--force-rechunk', '--fresh', '--from-pages', '--full', '--help', '--http', '--include-gitignored', '--include-null-signature', '--json', '--lang', '--markdown', '--max-age', '--multimodal', '--name-status', '--no-embed', '--no-embedding', '--no-extract', '--no-federated', '--older-than', '--others', '--path', '--pattern', '--pending', '--pglite', '--priority', '--progress-interval', '--progress-json', '--quiet', '--remediate', '--repo', '--reset', '--resolve', '--respect-gitignore', '--restore-only', '--since', '--skip-failed', '--source', '--source-id', '--stale', '--strategy', '--supabase', '--supersessions', '--surface', '--thin', '--timeout', '--url', '--workers'], - 'init': ['--all', '--brain', '--chat-model', '--check', '--ctx-size', '--embedding-dimensions', '--embedding-model', '--entity', '--expansion-model', '--fast', '--force', '--from-pages', '--grant-types', '--help', '--http', '--issuer-url', '--json', '--judge-model', '--key', '--mcp-only', '--mcp-url', '--migrate-only', '--model', '--multimodal', '--no', '--no-embed', '--no-embedding', '--non-interactive', '--oauth-client-id', '--oauth-client-secret', '--path', '--pglite', '--provenance', '--schema-pack', '--scopes', '--skip-embed-check', '--source', '--stale', '--supabase', '--surface', '--to', '--touchpoint', '--url', '--version'], + 'init': ['--all', '--brain', '--chat-model', '--check', '--ctx-size', '--embedding-dimensions', '--embedding-model', '--entity', '--expansion-model', '--fast', '--flag', '--force', '--from-pages', '--grant-types', '--help', '--http', '--issuer-url', '--json', '--judge-model', '--key', '--mcp-only', '--mcp-url', '--migrate-only', '--model', '--multimodal', '--no', '--no-embed', '--no-embedding', '--non-interactive', '--oauth-client-id', '--oauth-client-secret', '--path', '--pglite', '--provenance', '--schema-pack', '--scopes', '--skip-embed-check', '--source', '--stale', '--supabase', '--surface', '--to', '--touchpoint', '--url', '--version'], 'integrations': ['--auto', '--brain', '--dry-run', '--embeddings', '--fast', '--force', '--from-pages', '--help', '--http', '--json', '--no-embedding', '--overwrite', '--refresh', '--reranking', '--source', '--surface', '--target'], 'integrity': ['--aliases', '--all', '--auto', '--backend', '--background', '--brain', '--brain-wide-max-cost-usd', '--check', '--confidence', '--cost', '--dry-run', '--explain', '--fast', '--follow', '--force', '--fresh', '--from-pages', '--help', '--http', '--include-null-signature', '--json', '--limit', '--no-embedding', '--no-extract', '--pattern', '--pending', '--progress-interval', '--progress-json', '--quiet', '--remediate', '--reset', '--resolve', '--review-lower', '--skip-bare-tweet', '--skip-urls', '--source', '--stale', '--supabase', '--supersessions', '--surface', '--thin', '--timeout', '--type', '--url'], 'jobs': ['--abbrev-ref', '--aliases', '--all', '--allow-empty', '--allow-protected', '--allow-shell-jobs', '--apply', '--asof', '--auto', '--auto-fix', '--auto-with-prompt', '--background', '--backoff-delay', '--backoff-jitter', '--backoff-type', '--batch', '--batch-size', '--bound-max-concurrent', '--bound-slug-prefixes', '--bound-source', '--bound-tools', '--brain', '--break-lock', '--budget-usd', '--budget-usd-per-day', '--by-mention', '--by-type', '--cached', '--catch-up', '--check', '--cli-path', '--cluster', '--cluster-errors', '--code', '--concurrency', '--confidence', '--confirm-destructive', '--content', '--date', '--days', '--delay', '--detach', '--diff-filter', '--dimensions', '--dir', '--drain', '--dry-run', '--embedding-dimensions', '--embedding-model', '--empty', '--entities', '--exclude', '--exclude-standard', '--explain', '--fast', '--federated', '--federated-read', '--ff-only', '--file', '--fix', '--follow', '--force', '--force-break-lock', '--force-retry', '--format', '--fresh', '--from-meetings', '--from-pages', '--full', '--hard-deadline', '--health-interval', '--held-out', '--help', '--http', '--idempotency-key', '--image', '--include-frontmatter', '--include-gitignored', '--include-null-signature', '--infer-dates', '--inject-bootstrap', '--inline', '--input', '--install', '--interval', '--is-ancestor', '--json', '--kind', '--lang', '--limit', '--lock', '--markdown', '--max-age', '--max-attempts', '--max-cost-usd', '--max-crashes', '--max-rss', '--max-runtime-min', '--max-sources', '--max-stalled', '--max-usd', '--max-waiting', '--mcp-only', '--migrate-only', '--min-context', '--missing-path', '--mode', '--model', '--multimodal', '--name-only', '--name-status', '--near-symbol', '--ner', '--nice', '--no', '--no-auto-embed', '--no-embed', '--no-embedding', '--no-extract', '--no-federate', '--no-gpg-sign', '--no-hard-deadline', '--no-inject', '--no-mutate', '--no-pull', '--no-renames', '--no-schema-pack', '--no-verify', '--no-worker', '--non-interactive', '--now', '--offset', '--older-than', '--once', '--order', '--others', '--output', '--override-disabled', '--pace', '--pace-max-concurrency', '--pack', '--parallel', '--params', '--path', '--pattern', '--pending', '--phase', '--pid-file', '--priority', '--progress-interval', '--progress-json', '--queue', '--quiet', '--redact-secrets', '--reenrich-after', '--refresh-cache', '--refresh-ms', '--remediate', '--remediation-plan', '--repo', '--reset', '--resolve', '--respect-gitignore', '--restore-only', '--resume', '--retry-failed', '--review-lower', '--run-id', '--save', '--segment-limit', '--serial', '--session', '--session-id', '--short', '--show-toplevel', '--sigkill-rescue', '--since', '--skip-bare-tweet', '--skip-failed', '--skip-urls', '--sleep', '--slug', '--slugs', '--source', '--source-id', '--src-subpath', '--stale', '--stats', '--status', '--strategy', '--supersessions', '--surface', '--swap-only', '--symbol-kind', '--target', '--target-score', '--thin', '--thin-threshold', '--timeout', '--timeout-ms', '--to', '--trusted-extraction', '--type', '--types', '--uninstall', '--unsafe-bypass-dream-guard', '--url', '--user', '--verbose', '--version', '--watch', '--wedge-rescue', '--with-db', '--workers', '--yes'], @@ -67,7 +67,7 @@ export const CLI_FLAG_REGISTRY: Record = { 'orphans': ['--aliases', '--all', '--background', '--brain', '--brain-wide-max-cost-usd', '--count', '--explain', '--follow', '--help', '--include-null-signature', '--include-pseudo', '--json', '--no-extract', '--pattern', '--pending', '--progress-interval', '--progress-json', '--quiet', '--remediate', '--reset', '--resolve', '--source', '--stale', '--supersessions', '--thin', '--timeout'], 'pages': ['--aliases', '--all', '--brain', '--dry-run', '--help', '--include-null-signature', '--json', '--no-extract', '--older-than', '--pattern', '--pending', '--reset', '--resolve', '--source', '--stale', '--supersessions', '--thin'], 'pglite-repair': ['--brain', '--break-lock', '--dry-rnu', '--dry-run', '--fast', '--force', '--from-pages', '--help', '--http', '--json', '--no-embedding', '--path', '--quiet', '--source', '--surface', '--yes'], - 'post-upgrade': ['--all', '--apply-clean-hunks', '--brain', '--check', '--code', '--compile', '--concurrency', '--embedding-dimensions', '--embedding-model', '--fast', '--ff-only', '--force', '--force-all', '--force-orchestrator', '--force-retry', '--force-schema', '--format', '--from-pages', '--help', '--host-dir', '--http', '--inject-bootstrap', '--inline', '--install', '--interval', '--json', '--limit', '--list', '--markdown', '--max-rss', '--migrate-only', '--migration', '--mode', '--model', '--multimodal', '--name-only', '--no', '--no-autopilot-install', '--no-embed', '--no-embedding', '--no-inject', '--no-worker', '--non-interactive', '--now', '--quiet', '--repo', '--since', '--skills-dir', '--skip-verify', '--source', '--stale', '--status', '--surface', '--swap-only', '--target', '--to', '--uninstall', '--user', '--verbose', '--verify', '--version', '--workers', '--yes'], + 'post-upgrade': ['--all', '--apply-clean-hunks', '--brain', '--check', '--code', '--compile', '--concurrency', '--embedding-dimensions', '--embedding-model', '--fast', '--ff-only', '--flag', '--force', '--force-all', '--force-orchestrator', '--force-retry', '--force-schema', '--format', '--from-pages', '--help', '--host-dir', '--http', '--inject-bootstrap', '--inline', '--install', '--interval', '--json', '--limit', '--list', '--markdown', '--max-rss', '--migrate-only', '--migration', '--mode', '--model', '--multimodal', '--name-only', '--no', '--no-autopilot-install', '--no-embed', '--no-embedding', '--no-inject', '--no-worker', '--non-interactive', '--now', '--quiet', '--repo', '--since', '--skills-dir', '--skip-verify', '--source', '--stale', '--status', '--surface', '--swap-only', '--target', '--to', '--uninstall', '--user', '--verbose', '--verify', '--version', '--workers', '--yes'], 'protocol': ['--all', '--allow-empty', '--apply', '--asof', '--auto', '--bound-max-concurrent', '--bound-slug-prefixes', '--bound-source', '--bound-tools', '--brain', '--budget-usd-per-day', '--by-mention', '--content', '--date', '--days', '--entities', '--fast', '--federated', '--file', '--follow', '--force', '--from-pages', '--help', '--http', '--image', '--json', '--kind', '--limit', '--max-usd', '--mode', '--multimodal', '--no-embedding', '--offset', '--path', '--progress-interval', '--progress-json', '--quiet', '--save', '--session', '--session-id', '--since', '--slug', '--slugs', '--source', '--stats', '--surface', '--synthesize', '--target', '--timeout', '--token', '--trusted-extraction', '--url', '--with-db', '--yes'], 'providers': ['--brain', '--ctx-size', '--embedding-dimensions', '--embedding-model', '--embeddings', '--fast', '--force', '--from-pages', '--help', '--http', '--json', '--model', '--multimodal', '--no', '--no-embed', '--no-embedding', '--reranking', '--source', '--surface', '--touchpoint', '--version'], 'publish': ['--accent', '--bg', '--border', '--brain', '--card-bg', '--code-bg', '--error', '--fg', '--help', '--json', '--link', '--muted', '--out', '--password', '--source', '--title'], @@ -103,7 +103,7 @@ export const CLI_FLAG_REGISTRY: Record = { 'takes': ['--aliases', '--all', '--brain', '--bucket-size', '--by', '--claim', '--dir', '--domain', '--dry-run', '--evidence', '--expired', '--fast', '--federated', '--force', '--from-pages', '--help', '--holder', '--http', '--include-covered', '--include-null-signature', '--json', '--kind', '--limit', '--max-pages', '--no-embedding', '--no-extract', '--no-federated', '--outcome', '--path', '--pattern', '--pending', '--quality', '--refresh', '--repo', '--reset', '--resolve', '--restore-only', '--row', '--since', '--slugs', '--sort', '--source', '--source-id', '--stale', '--supersessions', '--surface', '--thin', '--unit', '--until', '--value', '--weight', '--who', '--yes'], 'think': ['--aliases', '--all', '--anchor', '--brain', '--calibration-holder', '--explain', '--fast', '--force', '--from-pages', '--help', '--http', '--include-null-signature', '--json', '--max-usd', '--mcp-only', '--model', '--no-embedding', '--no-extract', '--pattern', '--pending', '--reset', '--resolve', '--rounds', '--save', '--since', '--source', '--stale', '--supersessions', '--surface', '--take', '--thin', '--timeout', '--until', '--with-calibration'], 'transcripts': ['--aliases', '--all', '--brain', '--days', '--full', '--help', '--include-null-signature', '--json', '--limit', '--no-extract', '--pattern', '--pending', '--reset', '--resolve', '--source', '--stale', '--supersessions', '--thin', '--timeout'], - 'upgrade': ['--all', '--apply-clean-hunks', '--brain', '--check', '--code', '--compile', '--concurrency', '--embedding-dimensions', '--embedding-model', '--fast', '--ff-only', '--force', '--force-all', '--force-orchestrator', '--force-retry', '--force-schema', '--format', '--from-pages', '--help', '--host-dir', '--http', '--inject-bootstrap', '--inline', '--install', '--interval', '--json', '--limit', '--list', '--markdown', '--max-rss', '--migrate-only', '--migration', '--mode', '--model', '--multimodal', '--name-only', '--no', '--no-autopilot-install', '--no-embed', '--no-embedding', '--no-inject', '--no-worker', '--non-interactive', '--now', '--quiet', '--repo', '--since', '--skills-dir', '--skip-verify', '--source', '--stale', '--status', '--surface', '--swap-only', '--target', '--to', '--uninstall', '--user', '--verbose', '--verify', '--version', '--workers', '--yes'], + 'upgrade': ['--all', '--apply-clean-hunks', '--brain', '--check', '--code', '--compile', '--concurrency', '--embedding-dimensions', '--embedding-model', '--fast', '--ff-only', '--flag', '--force', '--force-all', '--force-orchestrator', '--force-retry', '--force-schema', '--format', '--from-pages', '--help', '--host-dir', '--http', '--inject-bootstrap', '--inline', '--install', '--interval', '--json', '--limit', '--list', '--markdown', '--max-rss', '--migrate-only', '--migration', '--mode', '--model', '--multimodal', '--name-only', '--no', '--no-autopilot-install', '--no-embed', '--no-embedding', '--no-inject', '--no-worker', '--non-interactive', '--now', '--quiet', '--repo', '--since', '--skills-dir', '--skip-verify', '--source', '--stale', '--status', '--surface', '--swap-only', '--target', '--to', '--uninstall', '--user', '--verbose', '--verify', '--version', '--workers', '--yes'], 'watch': ['--aliases', '--all', '--brain', '--fast', '--federated', '--force', '--from-pages', '--help', '--http', '--include-null-signature', '--json', '--max-pages', '--min-confidence', '--no-embedding', '--no-extract', '--no-federated', '--path', '--pattern', '--pending', '--repo', '--reset', '--resolve', '--restore-only', '--source', '--stale', '--stats', '--supersessions', '--surface', '--thin', '--window-turns'], 'ze-switch': ['--aliases', '--all', '--brain', '--confirm-reembed', '--dry-run', '--force', '--help', '--ignore-env-override', '--ignore-missing-key', '--include-null-signature', '--json', '--markdown', '--no-extract', '--non-interactive', '--pattern', '--pending', '--reset', '--resolve', '--resume', '--source', '--stale', '--supersessions', '--thin', '--undo', '--yes'], }; diff --git a/src/core/migrate.ts b/src/core/migrate.ts index e4ece9f51..c504c25c7 100644 --- a/src/core/migrate.ts +++ b/src/core/migrate.ts @@ -17,6 +17,9 @@ import { repairTimelineDedupIndex } from './timeline-dedup-repair.ts'; * notices are useful diagnostics on an UPGRADE but pure noise as a new user's * first-run output. Module-level (not threaded through the Migration type) * because only a couple of handlers emit them. Guarded via `migrationNotice`. + * Known limitation: concurrent runMigrations calls in one process (two engines + * migrating simultaneously) share this flag — worst case is a suppressed or + * extra stderr NOTICE line; migration execution/stamping is unaffected. */ let quietMigrationNotices = false; @@ -6053,15 +6056,18 @@ export async function runMigrations(engine: BrainEngine): Promise<{ applied: num process.stderr.write(` Schema version ${current} → ${LATEST_VERSION} (${pending.length} migration(s) pending)\n`); } - // Pre-flight: warn about connections that might block DDL - await checkForBlockingConnections(engine); - let applied = 0; try { + // Pre-flight: warn about connections that might block DDL + await checkForBlockingConnections(engine); + for (const m of pending) { if (!quietReplay) process.stderr.write(` [${m.version}] ${m.name}...\n`); try { await applyOneMigration(engine, m); + // Update version after both SQL and handler succeed. Inside the same + // catch so a stamp-write failure is also NAMED in quiet mode. + await engine.setConfig('version', String(m.version)); } catch (err) { // Quiet fresh-install replay: name the failing migration — without the // per-step lines, the error would otherwise be anonymous. @@ -6069,13 +6075,12 @@ export async function runMigrations(engine: BrainEngine): Promise<{ applied: num throw err; } - // Update version after both SQL and handler succeed - await engine.setConfig('version', String(m.version)); if (!quietReplay) process.stderr.write(` [${m.version}] ✓ ${m.name}\n`); applied++; } } finally { - // Never leak the fresh-install quiet flag into a later in-process run. + // Never leak the fresh-install quiet flag into a later in-process run — + // covers every exit path from here on (incl. the pre-flight probe). quietMigrationNotices = false; } diff --git a/src/core/onboard/init-nudge.ts b/src/core/onboard/init-nudge.ts index a099e03b7..69c2b1574 100644 --- a/src/core/onboard/init-nudge.ts +++ b/src/core/onboard/init-nudge.ts @@ -112,8 +112,10 @@ export async function runInitNudge(engine: BrainEngine): Promise { // A brand-new EMPTY brain has no "opportunities" — telling a fresh user // "0 takes" at the end of their first init is jargon-noise on the - // activation surface. Suppress the recommendation arms on empty. + // activation surface. Suppress the ENTIRE nudge on empty (including the + // partial-checks notice below). const brainEmpty = totalPages === 0; + if (brainEmpty) return; // Aggregate: any non-zero metric triggers the nudge. const linkCoverage = totalEntities > 0 ? linkedCount / totalEntities : 1; @@ -122,9 +124,7 @@ export async function runInitNudge(engine: BrainEngine): Promise { totalStale > 0 || (totalEntities > 0 && linkCoverage < 0.7) || (totalEntities > 0 && timelineCoverage < 0.9) - || (takesCount === 0 && !brainEmpty); - - if (brainEmpty) return; + || takesCount === 0; if (!hasRecommendations && !partial) return; // Emit one-line nudge. Be terse — init is the activation surface. diff --git a/src/core/operations.ts b/src/core/operations.ts index 48bc4cc6c..50acc755b 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -2773,17 +2773,12 @@ const get_brain_identity: Operation = { let latest_version: string | null = null; try { const su = await import('./self-upgrade.ts'); - const { isNewerVersion } = await import('./semver.ts'); - const entry = su.readUpdateCache(); - // Compare against the RUNNING version, not the cache-writer's — a - // stale/foreign cache must not report an upgrade this binary already has. - if ( - entry && su.isCacheFresh(entry, Date.now()) && - entry.marker.kind === 'upgrade_available' && - entry.marker.latest && isNewerVersion(VERSION, entry.marker.latest) - ) { + // Shared stale/foreign-cache guard (pendingUpgradeVersion): only an + // upgrade strictly newer than the RUNNING version counts. + const latest = su.pendingUpgradeVersion(VERSION, Date.now()); + if (latest) { update_available = true; - latest_version = entry.marker.latest ?? null; + latest_version = latest; } } catch { /* never let the banner break the op */ diff --git a/src/core/self-upgrade.ts b/src/core/self-upgrade.ts index 45da25150..bce51515b 100644 --- a/src/core/self-upgrade.ts +++ b/src/core/self-upgrade.ts @@ -30,7 +30,7 @@ import { closeSync, mkdirSync, openSync, readFileSync, renameSync, statSync, unl import { dirname, join } from 'node:path'; import { gbrainPath } from './config.ts'; import { acquirePackLock, type PackLockOpts } from './schema-pack/pack-lock.ts'; -import { isValidVersionString, parseSemver, semverGt, semverLte } from './semver.ts'; +import { isNewerVersion, isValidVersionString, parseSemver, semverGt, semverLte } from './semver.ts'; // ── Constants ─────────────────────────────────────────────────────────────── @@ -322,6 +322,29 @@ export function isCacheFresh(entry: CacheEntry, now: number): boolean { return now - entry.mtimeMs < ttl; } +/** + * The one shared "is an upgrade actually pending for THIS binary?" predicate. + * Returns the latest version string when the cache is present, fresh, marks an + * upgrade, AND that upgrade is strictly newer than the RUNNING binary — else + * null. The running-version comparison is the load-bearing part: the cache + * records the version of whatever binary WROTE it (an older gbrain on PATH can + * write it via the detached refresh), so consumers must never trust + * `marker.current` to describe themselves. Every upgrade-nag surface (CLI + * startup marker, doctor, advisor, get_brain_identity) routes through here so + * the suppression rule cannot drift per-surface. Never throws. + */ +export function pendingUpgradeVersion(runningVersion: string, now: number = Date.now()): string | null { + try { + const entry = readUpdateCache(); + if (!entry || !isCacheFresh(entry, now)) return null; + if (entry.marker.kind !== 'upgrade_available' || !entry.marker.latest) return null; + if (!isNewerVersion(runningVersion, entry.marker.latest)) return null; + return entry.marker.latest; + } catch { + return null; + } +} + // ── Snooze (interactive prompting only; never overrides mode=off) ──────────── export function readSnooze(): SnoozeRecord | null { diff --git a/src/core/skillpack/post-install-advisory.ts b/src/core/skillpack/post-install-advisory.ts index fa75dbeb7..7a28ad996 100644 --- a/src/core/skillpack/post-install-advisory.ts +++ b/src/core/skillpack/post-install-advisory.ts @@ -132,36 +132,51 @@ export function printAdvisoryIfRecommended(opts: { targetWorkspace?: string | null; targetSkillsDir?: string | null; }): void { - const advisory = buildAdvisory(opts); - if (!advisory) return; - if (opts.context === 'init') { - // Derive the counts for the compact form from the same detection the - // full banner used (cheap: re-runs the receipt parse). - let workspace = opts.targetWorkspace ?? null; - let skillsDir = opts.targetSkillsDir ?? null; - if (!skillsDir) { - const detected = autoDetectSkillsDir(); - if (detected.dir) { - skillsDir = detected.dir; - if (!workspace) workspace = resolvePath(skillsDir, '..'); + // Fail-open: this is decoration on the init success screen and runs AFTER + // the brain is created (and, since the memory-verbs quickstart now prints + // last, BEFORE it). An unreadable RESOLVER.md must never throw here and + // starve the primary CTA — same posture as runInitNudge. + try { + const advisory = buildAdvisory(opts); + if (!advisory) return; + if (opts.context === 'init') { + // Derive the counts for the compact form from the same detection the + // full banner used. Detection is hoisted OUT of the filter (one receipt + // read+parse total, matching buildAdvisory's own pattern). + let workspace = opts.targetWorkspace ?? null; + let skillsDir = opts.targetSkillsDir ?? null; + if (!skillsDir) { + const detected = autoDetectSkillsDir(); + if (detected.dir) { + skillsDir = detected.dir; + if (!workspace) workspace = resolvePath(skillsDir, '..'); + } } + const all = currentRecommendedSet(); + const installed = workspace && skillsDir ? detectInstalledSlugs(skillsDir, workspace) : null; + const missing = installed ? all.filter((s) => !installed.has(s.slug)) : all; + if (missing.length === 0) return; + const names = missing.map((s) => s.slug); + const preview = names.slice(0, 4).join(', ') + (names.length > 4 ? ', …' : ''); + // No workspace detected → scaffold has no target; say so (the full + // banner carries the same caveat via workspaceNotDetected). + const noWorkspace = installed === null; + // Human-voiced (prints on the init success screen where a person may read + // it) — no `[AGENT]` stage-direction leaking to the human. An agent reading + // the same line still knows the command to offer. + process.stderr.write( + `\n${missing.length} recommended skill(s) not installed yet (${preview}).\n` + + // NOTE: no bare `--flag` tokens in this string — the flag-registry + // generator harvests them from source strings and would register a + // phantom flag on every command that imports this module. + (noWorkspace + ? `Open your agent workspace first (scaffold needs a target), then \`${scaffoldCommandFor(missing, all)}\`; full list: gbrain advisor\n` + : `Ask me to run \`${scaffoldCommandFor(missing, all)}\`, or see the full list: gbrain advisor\n`), + ); + return; } - const all = currentRecommendedSet(); - const missing = - workspace && skillsDir - ? all.filter((s) => !detectInstalledSlugs(skillsDir, workspace).has(s.slug)) - : all; - if (missing.length === 0) return; - const names = missing.map((s) => s.slug); - const preview = names.slice(0, 4).join(', ') + (names.length > 4 ? ', …' : ''); - // Human-voiced (prints on the init success screen where a person may read - // it) — no `[AGENT]` stage-direction leaking to the human. An agent reading - // the same line still knows the command to offer. - process.stderr.write( - `\n${missing.length} recommended skill(s) not installed yet (${preview}).\n` + - `Ask me to run \`${scaffoldCommandFor(missing, all)}\`, or see the full list: gbrain advisor\n`, - ); - return; + process.stderr.write(advisory); + } catch { + /* advisory is best-effort decoration — never break init */ } - process.stderr.write(advisory); } diff --git a/templates/bootstrap/template-repo/README.md b/templates/bootstrap/template-repo/README.md index 493208e6f..6141c5577 100644 --- a/templates/bootstrap/template-repo/README.md +++ b/templates/bootstrap/template-repo/README.md @@ -1,6 +1,6 @@ # gbrain agent workspace — template - + This repository is the **"Use this template"** distribution artifact for a [gbrain](https://github.com/garrytan/gbrain) personal-agent workspace — the same diff --git a/test/ai-probes-models.test.ts b/test/ai-probes-models.test.ts new file mode 100644 index 000000000..cde9fd646 --- /dev/null +++ b/test/ai-probes-models.test.ts @@ -0,0 +1,69 @@ +/** + * Unit tests for src/core/ai/probes.ts (probeOpenAICompat). + * + * Runs against a local Bun.serve fixture on an ephemeral port (port: 0) so + * no real daemon is needed. Pins: + * - models extraction from a valid {object:'list', data:[...]} body, with + * non-string / missing ids filtered out + * - non-list JSON body → models_endpoint_valid false, models undefined + * - connection refused → reachable false + */ + +import { describe, test, expect } from 'bun:test'; +import { probeOpenAICompat } from '../src/core/ai/probes.ts'; + +describe('probeOpenAICompat — models extraction', () => { + test('valid list body extracts string ids only', async () => { + const server = Bun.serve({ + port: 0, + fetch() { + return Response.json({ object: 'list', data: [{ id: 'm1' }, { id: 42 }, {}] }); + }, + }); + try { + const r = await probeOpenAICompat(`http://127.0.0.1:${server.port}`); + expect(r.reachable).toBe(true); + expect(r.models_endpoint_valid).toBe(true); + expect(r.models).toEqual(['m1']); + } finally { + server.stop(true); + } + }); + + test('non-list JSON body → valid false, models undefined', async () => { + const server = Bun.serve({ + port: 0, + fetch() { + return Response.json({ hello: 'world' }); + }, + }); + try { + const r = await probeOpenAICompat(`http://127.0.0.1:${server.port}`); + expect(r.reachable).toBe(true); + expect(r.models_endpoint_valid).toBe(false); + expect(r.models).toBeUndefined(); + } finally { + server.stop(true); + } + }); + + test('connection refused → reachable false', async () => { + // Grab an ephemeral port by binding, then release it before probing so + // the connection is refused (nothing else claims the port that fast). + const server = Bun.serve({ + port: 0, + fetch() { + return new Response('unused'); + }, + }); + const port = server.port; + try { + await server.stop(true); + const r = await probeOpenAICompat(`http://127.0.0.1:${port}`); + expect(r.reachable).toBe(false); + expect(r.error).toBeDefined(); + } finally { + server.stop(true); + } + }); +}); diff --git a/test/bootstrap-dispatcher.serial.test.ts b/test/bootstrap-dispatcher.serial.test.ts index 30c6822a0..d57dfe956 100644 --- a/test/bootstrap-dispatcher.serial.test.ts +++ b/test/bootstrap-dispatcher.serial.test.ts @@ -21,6 +21,7 @@ import { runBootstrap, workspaceBrainStats } from '../src/commands/bootstrap.ts' import type { ExecRunner } from '../src/core/bootstrap/repo.ts'; import { attachWorkspace } from '../src/core/bootstrap/attach.ts'; import { readReceipt, receiptPath, writeManifest, type InstallReceipt } from '../src/core/bootstrap/format.ts'; +import { GBRAIN_HOOK_MARKER_KEY, GBRAIN_HOOK_MARKER_VALUE } from '../src/core/bootstrap/host-specs.ts'; import { initState, setAnswer, skipAnswer, confirm, readBackHash } from '../src/core/bootstrap/interview.ts'; let tmpParent: string; // GBRAIN_HOME parent (configDir appends .gbrain) @@ -435,13 +436,43 @@ describe('MCP registration verification [FIX7]', () => { const r = await runHooks(runner); expect(r.result).toBe(0); expect(r.err).toContain('targets a DIFFERENT workspace'); - expect(calls.some((c) => c[1] === 'mcp' && c[2] === 'remove' && c[3] === 'gbrain')).toBe(true); + // The remove must be SCOPED on claude-code: a scope-less remove can resolve + // to a different scope's registration and leave the blocker in place. + const removes = calls.filter((c) => c[1] === 'mcp' && c[2] === 'remove' && c[3] === 'gbrain'); + expect(removes.length).toBeGreaterThan(0); + for (const c of removes) { + const scopeIdx = c.indexOf('--scope'); + expect(scopeIdx).toBeGreaterThan(3); + expect(c[scopeIdx + 1]).toBe('project'); + } const adds = calls.filter((c) => c[1] === 'mcp' && c[2] === 'add').length; expect(adds).toBe(2); // initial (foreign) + re-add after remove // After the fix, the smoke confirms the corrected registration. expect(r.out).toContain('verified targeting this workspace'); }, 30_000); + test('mismatch + failed remove → exit 1 with the by-hand fix instruction; add never retried', async () => { + // Stateful failure host: add refuses ("already exists"), get shows a + // FOREIGN registration (mismatch), and the scoped remove itself fails. + const calls: string[][] = []; + const runner: ExecRunner = async (argv: string[]) => { + calls.push(argv); + if (argv[1] !== 'mcp') return { code: 0, stdout: '', stderr: '' }; + if (argv[2] === 'add') return { code: 1, stdout: '', stderr: 'MCP server gbrain already exists' }; + if (argv[2] === 'get') return { code: 0, stdout: FOREIGN, stderr: '' }; + if (argv[2] === 'remove') return { code: 1, stdout: '', stderr: 'nope' }; + return { code: 0, stdout: '', stderr: '' }; + }; + const r = await runHooks(runner); + expect(r.result).toBe(1); + expect(r.err).toContain('targets a DIFFERENT workspace'); + // Fail LOUD, not the old silent no-op loop: the message hands the human + // the manual off-ramp instead of re-failing the add. + expect(r.err).toContain('remove the stale registration by hand'); + const adds = calls.filter((c) => c[1] === 'mcp' && c[2] === 'add').length; + expect(adds).toBe(1); // the failed remove halts the flow before any re-add + }, 30_000); + test('host without `mcp get` → inconclusive, kept with a note (never a false bless)', async () => { const { runner } = mcpHost({ initialReg: OURS, getSupported: false }); const r = await runHooks(runner); @@ -452,6 +483,108 @@ describe('MCP registration verification [FIX7]', () => { }, 30_000); }); +describe('MCP host failure × hooks at the dispatcher (exit-127 skip / broken settings fail-closed)', () => { + // Self-contained fixtures (the flip pattern): HOOKS_CONSENT left at its bank + // default ('yes') so the hooks half of the phase is live in both tests. + const scratch: string[] = []; + afterAll(() => { + for (const d of scratch) rmSync(d, { recursive: true, force: true }); + }); + + function failWorkspace(): { fws: string; fhome: string; fparent: string } { + const fparent = mkdtempSync(join(tmpdir(), 'gb-fail-')); + const fhome = join(fparent, '.gbrain'); + mkdirSync(fhome, { recursive: true }); + const fws = mkdtempSync(join(tmpdir(), 'gb-fail-ws-')); + scratch.push(fparent, fws); + const prev = process.env.GBRAIN_HOME; + process.env.GBRAIN_HOME = fparent; + try { + expect(initState(fws).ok).toBe(true); + for (const [key, value] of Object.entries(REQUIRED_ANSWERS)) { + const r = setAnswer(fws, key, value); + if (!r.ok) throw new Error(r.message); + } + expect(setAnswer(fws, 'MCP_SCOPE', 'project').ok).toBe(true); + const h = readBackHash(fws); + if (!h.ok) throw new Error(h.message); + expect(confirm(fws, h.hash).ok).toBe(true); + } finally { + if (prev === undefined) delete process.env.GBRAIN_HOME; + else process.env.GBRAIN_HOME = prev; + } + return { fws, fhome, fparent }; + } + + async function withFailHome(parent: string, fn: () => Promise): Promise { + const prev = process.env.GBRAIN_HOME; + process.env.GBRAIN_HOME = parent; + try { + return await fn(); + } finally { + if (prev === undefined) delete process.env.GBRAIN_HOME; + else process.env.GBRAIN_HOME = prev; + } + } + + test('`claude` missing (exit 127 on mcp add) → MCP skipped, hooks STILL install, exit 2, receipt detail hooks', async () => { + const { fws, fhome, fparent } = failWorkspace(); + const r = await withFailHome(fparent, async () => { + expect((await capture(() => runBootstrap(['render', '--workspace', fws]))).result).toBe(0); + const runner: ExecRunner = async (argv: string[]) => { + if (argv[0] === 'claude' && argv[1] === 'mcp' && argv[2] === 'add') { + return { code: 127, stdout: '', stderr: 'claude: command not found' }; + } + return { code: 0, stdout: '', stderr: '' }; + }; + return capture(() => + runBootstrap(['hooks', '--workspace', fws, '--harness', 'claude-code', '--gbrain-bin', process.execPath], { + runner, + }), + ); + }); + expect(r.result).toBe(2); + expect(r.err).toContain('is not on PATH'); + // The old early-return silently dropped hooks; now they install anyway + // (hooks only write settings.local.json and need no host binary). + expect(r.out).toContain('hooks installed'); + const settingsPath = join(fws, '.claude', 'settings.local.json'); + expect(existsSync(settingsPath)).toBe(true); + const settings = JSON.parse(readFileSync(settingsPath, 'utf8')) as { + hooks?: Record> }>>; + }; + const entries = Object.values(settings.hooks ?? {}).flatMap((groups) => groups.flatMap((g) => g.hooks ?? [])); + expect(entries.length).toBeGreaterThan(0); + expect(entries.some((e) => e[GBRAIN_HOOK_MARKER_KEY] === GBRAIN_HOOK_MARKER_VALUE)).toBe(true); + // Receipt records what actually landed: hooks only, no MCP. + expect(readReceipt(fhome)?.registrations).toEqual([{ host: 'claude-code', scope: 'project', detail: 'hooks' }]); + }, 30_000); + + test('unparseable settings.local.json → hooks fail CLOSED (exit 1), file byte-identical, receipt detail mcp', async () => { + const { fws, fhome, fparent } = failWorkspace(); + const broken = '{ definitely broken'; + const settingsPath = join(fws, '.claude', 'settings.local.json'); + const r = await withFailHome(fparent, async () => { + expect((await capture(() => runBootstrap(['render', '--workspace', fws]))).result).toBe(0); + mkdirSync(join(fws, '.claude'), { recursive: true }); + writeFileSync(settingsPath, broken, 'utf8'); + const { runner } = makeRunner(); + return capture(() => + runBootstrap(['hooks', '--workspace', fws, '--harness', 'claude-code', '--gbrain-bin', process.execPath], { + runner, + }), + ); + }); + expect(r.result).toBe(1); + // The refusal explains WHY (the file may carry permissions/allowlist + // entries gbrain must not clobber) and names the repair path. + expect(r.err).toContain('not valid JSON'); + expect(readFileSync(settingsPath, 'utf8')).toBe(broken); + // MCP (step 1) landed before the hook failure — the receipt says exactly that. + expect(readReceipt(fhome)?.registrations).toEqual([{ host: 'claude-code', scope: 'project', detail: 'mcp' }]); + }, 30_000); +}); + describe('receipt overwrite guard wired into every writer [CX2-12]', () => { function writeNewerReceipt(dir: string): void { mkdirSync(join(dir, 'bootstrap'), { recursive: true }); diff --git a/test/bootstrap-interview.serial.test.ts b/test/bootstrap-interview.serial.test.ts index 4f37af601..2af39618d 100644 --- a/test/bootstrap-interview.serial.test.ts +++ b/test/bootstrap-interview.serial.test.ts @@ -310,6 +310,49 @@ describe('[A8] provenance + read-back confirm', () => { expect(parsed.confirmed).toBeUndefined(); }); + test('setAnswer surfaces invalidatedConfirmation ONLY when a confirm existed', () => { + const ws = makeWs(); + answerAllRequired(ws); + // No prior confirmation → nothing was invalidated (falsy flag). + const r0 = setAnswer(ws, 'SOUL_WINCE', 'Filler openers.'); + expect(r0.ok).toBe(true); + if (!r0.ok || r0.sink !== 'state') throw new Error('expected a state-sink result'); + expect(r0.invalidatedConfirmation).toBeFalsy(); + // Full confirm, then a later set → the result SAYS it voided the confirm + // (the CLI warns at --set time instead of failing much later at render). + const h = readBackHash(ws); + if (!h.ok) throw new Error(h.message); + expect(confirm(ws, h.hash).ok).toBe(true); + const r1 = setAnswer(ws, 'SOUL_GOOD_OUTPUT', 'A finished artifact.'); + expect(r1.ok).toBe(true); + if (!r1.ok || r1.sink !== 'state') throw new Error('expected a state-sink result'); + expect(r1.invalidatedConfirmation).toBe(true); + const st = status(ws); + if (!st.ok) throw new Error(st.message); + expect(st.confirmed).toBe(false); + }); + + test('skipAnswer surfaces invalidatedConfirmation ONLY when a confirm existed (optional key — required keys refuse skip)', () => { + const ws = makeWs(); + answerAllRequired(ws); + // No prior confirmation → falsy flag on an optional-key skip. + const r0 = skipAnswer(ws, 'SOUL_WINCE'); + expect(r0.ok).toBe(true); + if (!r0.ok) throw new Error('unreachable'); + expect(r0.invalidatedConfirmation).toBeFalsy(); + // Full confirm, then a later optional-key skip → invalidation surfaced. + const h = readBackHash(ws); + if (!h.ok) throw new Error(h.message); + expect(confirm(ws, h.hash).ok).toBe(true); + const r1 = skipAnswer(ws, 'SOUL_WORLDVIEW'); + expect(r1.ok).toBe(true); + if (!r1.ok) throw new Error('unreachable'); + expect(r1.invalidatedConfirmation).toBe(true); + const st = status(ws); + if (!st.ok) throw new Error(st.message); + expect(st.confirmed).toBe(false); + }); + test('show returns the read-back payload with the hash once complete', () => { const ws = makeWs(); answerAllRequired(ws); diff --git a/test/bootstrap-verify.serial.test.ts b/test/bootstrap-verify.serial.test.ts index f57cc5cef..48dd277ca 100644 --- a/test/bootstrap-verify.serial.test.ts +++ b/test/bootstrap-verify.serial.test.ts @@ -247,6 +247,15 @@ describe('verifyWorkspace — keyless pass', () => { expect(scan.detail).not.toContain('sk-AAAAAAAAAAAAAAAAAAAAAAAA'); expect(res.ok).toBe(false); + + // Tour gating on FAIL: the report says fix-first and withholds the + // celebration prompts ("broken, but go enjoy it" is a mixed signal) … + expect(res.report).toContain('Fix the FAIL checks above'); + expect(res.report).not.toContain('Who am I to you?'); + // … while the returned tour array stays unconditional so machine + // consumers (--json) keep a stable shape, and the check names the gate. + expect(res.tour).toEqual([...FIRST_RUN_TOUR]); + expect(check(res.checks, 'first_run_tour')[0].detail).toContain('withheld'); } finally { rmSync(githubPath, { force: true }); writeFileSync(userPath, userOriginal); diff --git a/test/cli-dispatch-thin-client.test.ts b/test/cli-dispatch-thin-client.test.ts index 568e1b2db..4d243cf5e 100644 --- a/test/cli-dispatch-thin-client.test.ts +++ b/test/cli-dispatch-thin-client.test.ts @@ -208,6 +208,9 @@ describe('thin-client scratch-DB guard — jobs partial dispatch + config refusa expect(existsSync(join(tmp, '.gbrain', 'brain.pglite'))).toBe(false); expect(r.stdout + r.stderr).not.toContain('Schema version'); expect(r.stdout + r.stderr).not.toContain('migration(s) pending'); + // A scratch store is a FRESH install, so a re-regression would print the + // quiet-replay summary line, not the verbose header — pin both shapes. + expect(r.stdout + r.stderr).not.toContain('Setting up brain schema'); }); test('`gbrain jobs list` never fabricates a scratch local engine', async () => { @@ -216,5 +219,6 @@ describe('thin-client scratch-DB guard — jobs partial dispatch + config refusa const { existsSync } = await import('fs'); expect(existsSync(join(tmp, '.gbrain', 'brain.pglite'))).toBe(false); expect(r.stdout + r.stderr).not.toContain('Schema version'); + expect(r.stdout + r.stderr).not.toContain('Setting up brain schema'); }); }); diff --git a/test/doctor-self-upgrade.test.ts b/test/doctor-self-upgrade.test.ts index 5c9534e40..229f27a82 100644 --- a/test/doctor-self-upgrade.test.ts +++ b/test/doctor-self-upgrade.test.ts @@ -6,6 +6,7 @@ import { withEnv } from './helpers/with-env.ts'; import { checkSelfUpgradeHealth } from '../src/commands/doctor.ts'; import { writeUpdateCache } from '../src/core/self-upgrade.ts'; import { logSelfUpgrade } from '../src/core/audit/self-upgrade-audit.ts'; +import { VERSION } from '../src/version.ts'; async function withHome(fn: (home: string) => T | Promise): Promise { const dir = mkdtempSync(join(tmpdir(), 'gbrain-doctor-su-')); @@ -40,7 +41,18 @@ describe('checkSelfUpgradeHealth', () => { const c = checkSelfUpgradeHealth(); expect(c.status).toBe('ok'); expect(c.message).toContain('update available'); - expect(c.message).toContain('0.99.0'); + expect(c.message).toContain('-> 0.99.0'); + }); + }); + + test('fresh cache with latest == running version → suppressed (no update-available nag)', async () => { + await withHome(() => { + // Stale/foreign cache: the recorded latest is the version we are already + // running. The shared pendingUpgradeVersion guard must suppress the nag. + writeUpdateCache({ kind: 'upgrade_available', current: VERSION, latest: VERSION }); + const c = checkSelfUpgradeHealth(); + expect(c.status).toBe('ok'); + expect(c.message).not.toContain('update available'); }); }); diff --git a/test/e2e/init-fresh-pglite.test.ts b/test/e2e/init-fresh-pglite.test.ts index 0f1e2ab0f..b6e8f4da6 100644 --- a/test/e2e/init-fresh-pglite.test.ts +++ b/test/e2e/init-fresh-pglite.test.ts @@ -107,8 +107,11 @@ describe('v0.45 DX wave — non-TTY no-key defaults to keyless (typo still fail- // Keyless is a first-class posture: the naive first command completes. expect(r.exitCode).toBe(0); expect(r.stderr).toContain('keyless mode'); - // The notice names the upgrade affordance. - expect(r.stderr).toContain('gbrain config set embedding_model'); + // The notice names the upgrade affordance — the re-init recipe that + // actually works, NOT `config set embedding_model` (which config.ts + // hard-refuses as a schema-sizing no-op). + expect(r.stderr).toContain('gbrain init --force --pglite --embedding-model'); + expect(r.stderr).not.toContain('config set embedding_model'); // Config persisted with the deferred-embedding sentinel. const cfg = JSON.parse(readFileSync(join(tmpHome, '.gbrain', 'config.json'), 'utf-8')); expect(cfg.embedding_disabled).toBe(true); @@ -153,6 +156,48 @@ describe('v0.45 DX wave — non-TTY no-key defaults to keyless (typo still fail- // ============================================================================ +describe('v0.45 DX wave — --supabase non-TTY guard + multi-key no-canonical fail-loud', () => { + test('init --supabase without a TTY fails loud and names the --url escape hatch', async () => { + // Legacy behavior was a silent exit-0 no-op (stdin closed → readLine + // never resolved → process ended with NO config written) — the worst + // failure shape for a scripted/agent caller. + const home = makeTempHome(); + try { + const r = await runCli(['init', '--supabase'], { gbrainHome: home, env: {} }); + expect(r.exitCode).toBe(1); + expect(r.stderr).toContain('needs an interactive terminal'); + expect(r.stderr).toContain('--url'); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }, 120000); + + test('multiple provider keys with NO canonical candidate stays fail-loud with disambiguation hint', async () => { + // The canonical default provider (zeroentropyai) has no key here, so the + // non-TTY auto-pick cannot resolve the ambiguity — it must fail loud + // (D2/D3), not guess between openai and voyage. + const home = makeTempHome(); + try { + const r = await runCli(['init', '--pglite', '--non-interactive'], { + gbrainHome: home, + env: { + OPENAI_API_KEY: 'sk-test-only-for-init-resolution-NOT-CALLED', + VOYAGE_API_KEY: 'pa-test-only-for-init-resolution-NOT-CALLED', + }, + }); + expect(r.exitCode).toBe(1); + expect(r.stderr).toMatch(/Multiple embedding providers env-ready/); + expect(r.stderr).toMatch(/Disambiguate by passing --embedding-model/); + // Fail-loud path exits BEFORE any config write. + expect(existsSync(join(home, '.gbrain', 'config.json'))).toBe(false); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }, 120000); +}); + +// ============================================================================ + describe('v0.37 T12 — D6 regression: bug-reporter no-op keys exit 1 with Levenshtein', () => { let tmpHome: string; diff --git a/test/e2e/self-upgrade-marker.test.ts b/test/e2e/self-upgrade-marker.test.ts index 2998d01fa..f1e254779 100644 --- a/test/e2e/self-upgrade-marker.test.ts +++ b/test/e2e/self-upgrade-marker.test.ts @@ -77,6 +77,21 @@ describe('self-upgrade marker on a real invocation', () => { expect(stderr).not.toContain('UPGRADE_AVAILABLE'); }); + test('cache latest == running version → suppressed (no marker, no human sentence)', () => { + writeCache(`UPGRADE_AVAILABLE ${VERSION} ${VERSION}`); + const { stderr } = runGbrain('notify'); + expect(stderr).not.toContain('UPGRADE_AVAILABLE'); + expect(stderr).not.toContain('Run: gbrain self-upgrade'); + }); + + test('foreign-writer cache → marker prints the RUNNING version, not the writer\'s', () => { + // An older gbrain on PATH wrote the cache: marker.current is 0.0.1, not us. + writeCache('UPGRADE_AVAILABLE 0.0.1 0.99.0'); + const { stderr } = runGbrain('notify'); + expect(stderr).toContain(`UPGRADE_AVAILABLE ${VERSION} 0.99.0`); + expect(stderr).not.toContain('UPGRADE_AVAILABLE 0.0.1'); + }); + test('active snooze for the version → no marker (notify mode honors snooze)', () => { writeCache(`UPGRADE_AVAILABLE ${VERSION} 0.99.0`); // snooze record: " " — fresh ts so it's active. diff --git a/test/helpers/tty-harness.ts b/test/helpers/tty-harness.ts index f8d612678..cfc68edd1 100644 --- a/test/helpers/tty-harness.ts +++ b/test/helpers/tty-harness.ts @@ -150,7 +150,7 @@ export function renderStallsReport(stalls: readonly Stall[], totalMs: number): s // a live TUI across separate tool calls) // ──────────────────────────────────────────────────────────────────────────── -export const KEY_MAP: Record = { +export const KEY_MAP = { Enter: '\r', Up: '\x1b[A', Down: '\x1b[B', @@ -163,8 +163,11 @@ export const KEY_MAP: Record = { Backspace: '\x7f', CtrlC: '\x03', CtrlD: '\x04', -}; +} as const satisfies Record; +/** Literal union of key names (not plain string) so sendKey('Entr') is a + * compile error in test code; drive mode's runtime-string path keeps the + * `| string` overload with its runtime throw. */ export type KeyName = keyof typeof KEY_MAP; export type DriveCommand = @@ -282,7 +285,8 @@ export interface TtyLaunchOpts { dropEnv?: string[]; /** Wall-clock kill switch. Default 15 min. */ timeoutMs?: number; - /** Observer for every output burst (drive mode streams frames to disk). */ + /** Observer for every output burst (for callers that want to stream frames + * somewhere as they arrive; saveTranscript already persists them at end). */ onFrame?: (frame: PtyFrame) => void; } @@ -398,13 +402,32 @@ export function launchTty(argv: string[], opts: TtyLaunchOpts = {}): TtySession }); } - const wallTimer = setTimeout(() => { - try { - proc.kill?.('SIGKILL'); - } catch { - /* ignore */ + /** + * Best-effort kill of the child's whole process TREE, not just the parent. + * A PTY child (claude/codex/a shell) is normally the session/group leader of + * its pty, so `process.kill(-pid, sig)` reaches its descendants (MCP servers, + * sub-shells) — otherwise SIGKILL to the parent orphans them, leaking API + * spend and PGLite locks. If the child isn't a group leader the negative-pid + * kill throws (EPERM/ESRCH) and we fall back to killing the parent alone. + */ + function killTree(sig: NodeJS.Signals): void { + const pid = proc.pid as number | undefined; + if (typeof pid === 'number') { + try { + process.kill(-pid, sig); + return; + } catch { + /* not a group leader — fall through to parent-only */ + } } - }, timeoutMs); + try { + proc.kill?.(sig); + } catch { + /* already dead */ + } + } + + const wallTimer = setTimeout(() => killTree('SIGKILL'), timeoutMs); function send(data: string): void { if (exited) return; @@ -416,7 +439,7 @@ export function launchTty(argv: string[], opts: TtyLaunchOpts = {}): TtySession } function sendKey(key: KeyName | string): void { - const seq = KEY_MAP[key as string]; + const seq = (KEY_MAP as Record)[key]; if (seq === undefined) throw new Error(`sendKey: unknown key ${JSON.stringify(key)}`); send(seq); } @@ -494,18 +517,10 @@ export function launchTty(argv: string[], opts: TtyLaunchOpts = {}): TtySession async function close(): Promise { clearTimeout(wallTimer); if (exited) return; - try { - proc.kill?.('SIGINT'); - } catch { - /* ignore */ - } + killTree('SIGINT'); await Promise.race([exitedPromise, Bun.sleep(2000)]); if (!exited) { - try { - proc.kill?.('SIGKILL'); - } catch { - /* ignore */ - } + killTree('SIGKILL'); await Promise.race([exitedPromise, Bun.sleep(1000)]); } } diff --git a/test/init-nudge.test.ts b/test/init-nudge.test.ts new file mode 100644 index 000000000..318aa0f92 --- /dev/null +++ b/test/init-nudge.test.ts @@ -0,0 +1,140 @@ +/** + * Unit tests for src/core/onboard/init-nudge.ts (runInitNudge). + * + * runInitNudge fires 6 parallel COUNT probes (stale chunks, entities, linked + * entities, timeline entities, takes, total pages) and prints a one-line + * nudge to stderr. These tests pin the DX-wave behavior: + * - EMPTY brain (pages count 0) → the whole nudge is suppressed, even + * when takes === 0 (no "0 takes" jargon-noise on first init) + * - pages probe REJECTS → fail-open sentinel treats the brain as + * non-empty, so the takes nudge still fires + * - non-empty + healthy + one rejected arm → partial-checks notice + * - non-empty + takes 0 → "0 takes" opportunity nudge + * + * The gate is process.stderr.isTTY (NOT process.env), so monkeypatching it + * here does not trip the serial-isolation rules for env-mutating tests. + * Both isTTY and stderr.write are restored in finally. + */ + +import { describe, test, expect } from 'bun:test'; +import { runInitNudge } from '../src/core/onboard/init-nudge.ts'; +import type { BrainEngine } from '../src/core/engine.ts'; + +/** Per-probe result: a count, or an Error to make that arm reject. */ +interface ProbeCounts { + stale?: number | Error; + entities?: number | Error; + linked?: number | Error; + timeline?: number | Error; + takes?: number | Error; + pages?: number | Error; +} + +/** + * Stub engine shaped like { executeRaw: async (sql) => [...] }. Routes each + * of runInitNudge's 6 COUNT queries by a distinctive SQL fragment. Order + * matters: the linked/timeline queries also contain "type IN ('person'", + * so they are matched first. + */ +function stubEngine(counts: ProbeCounts): BrainEngine { + const route = (sql: string): number | Error => { + if (sql.includes('content_chunks')) return counts.stale ?? 0; + if (sql.includes('FROM takes')) return counts.takes ?? 0; + if (sql.includes('FROM links')) return counts.linked ?? 0; + if (sql.includes('timeline_entries')) return counts.timeline ?? 0; + if (sql.includes("type IN ('person'")) return counts.entities ?? 0; + // 6th probe: SELECT COUNT(*) FROM pages WHERE deleted_at IS NULL + return counts.pages ?? 0; + }; + return { + executeRaw: async (sql: string) => { + const r = route(sql); + if (r instanceof Error) throw r; + return [{ count: r }]; + }, + } as unknown as BrainEngine; +} + +/** + * Run the nudge with stderr forced to look like a TTY and its writes + * captured. Restores both in finally so no other test sees the patch. + */ +async function runNudgeCaptured(engine: BrainEngine): Promise { + const origIsTTY = process.stderr.isTTY; + const origWrite = process.stderr.write; + let out = ''; + try { + (process.stderr as unknown as { isTTY: boolean }).isTTY = true; + process.stderr.write = ((chunk: unknown) => { + out += String(chunk); + return true; + }) as typeof process.stderr.write; + await runInitNudge(engine); + } finally { + process.stderr.write = origWrite; + (process.stderr as unknown as { isTTY: boolean | undefined }).isTTY = origIsTTY; + } + return out; +} + +describe('runInitNudge — empty-brain suppression', () => { + test('empty brain (pages 0, takes 0) prints NOTHING', async () => { + const out = await runNudgeCaptured( + stubEngine({ stale: 0, entities: 0, linked: 0, timeline: 0, takes: 0, pages: 0 }), + ); + expect(out).toBe(''); + }); +}); + +describe('runInitNudge — pages probe failure is fail-open', () => { + test('pages probe REJECTS with takes 0 → nudge still fires with "0 takes"', async () => { + // The -1 sentinel means "count unknown" — treat as non-empty so the + // pre-existing behavior (nudge on 0 takes) is preserved. + const out = await runNudgeCaptured( + stubEngine({ + stale: 0, + entities: 0, + linked: 0, + timeline: 0, + takes: 0, + pages: new Error('pages probe failed'), + }), + ); + expect(out).toContain('Brain has opportunities'); + expect(out).toContain('0 takes'); + }); +}); + +describe('runInitNudge — partial-checks notice', () => { + test('non-empty healthy brain with one rejected arm → "Init checks incomplete"', async () => { + // pages > 0 (non-empty), takes > 0 (no opportunity part), entities 0 + // (coverage arms vacuous), stale 0 — but the linked probe rejects, so + // partial=true with zero parts → the incomplete-checks line. + const out = await runNudgeCaptured( + stubEngine({ + stale: 0, + entities: 0, + linked: new Error('linked probe failed'), + timeline: 0, + takes: 5, + pages: 10, + }), + ); + expect(out).toContain('Init checks incomplete'); + expect(out).toContain('(5/6)'); + expect(out).toContain('gbrain onboard --check'); + expect(out).not.toContain('Brain has opportunities'); + }); +}); + +describe('runInitNudge — non-empty brain opportunities', () => { + test('non-empty brain with takes 0 → "Brain has opportunities: 0 takes"', async () => { + const out = await runNudgeCaptured( + stubEngine({ stale: 0, entities: 0, linked: 0, timeline: 0, takes: 0, pages: 10 }), + ); + expect(out).toContain('Brain has opportunities: 0 takes'); + expect(out).toContain("Run 'gbrain onboard --check' to see the plan"); + // All 6 probes succeeded — no partial-checks suffix. + expect(out).not.toContain('checks complete'); + }); +}); diff --git a/test/migrate-quiet-replay.serial.test.ts b/test/migrate-quiet-replay.serial.test.ts new file mode 100644 index 000000000..6f9571eb8 --- /dev/null +++ b/test/migrate-quiet-replay.serial.test.ts @@ -0,0 +1,140 @@ +/** + * Quiet fresh-install migration replay (src/core/migrate.ts runMigrations). + * + * Pins: + * - Fresh brain (version 1, all migrations pending) replays quietly: ONE + * "Setting up brain schema (vN)..." line, no per-migration "[N] name..." + * lines, and the v123/v124 handler notices are suppressed. + * - GBRAIN_MIGRATE_VERBOSE=1 escape hatch restores full verbose output on + * a fresh brain. + * - Upgrades (current > 1) keep the full verbose "Schema version X → Y" + * narration — including an in-process upgrade AFTER a quiet fresh replay, + * which proves the module-level quietMigrationNotices flag is reset in + * the finally and does not leak. + * - A no-pending run applies nothing and stays silent. + * + * Serial: mutates process.env (GBRAIN_MIGRATE_VERBOSE, GBRAIN_PGLITE_SNAPSHOT) + * and monkey-patches process.stderr.write; everything is saved in beforeAll / + * restored in afterAll or per-test try/finally (repo rule R1). + */ + +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { runMigrations, LATEST_VERSION } from '../src/core/migrate.ts'; + +let engineA: PGLiteEngine; // fresh quiet replay + later upgrade re-run +let engineB: PGLiteEngine | null = null; // fresh verbose replay (test 2) + +let prevVerbose: string | undefined; +let prevSnapshot: string | undefined; + +/** Capture everything written to process.stderr.write while fn runs. */ +async function captureStderr(fn: () => Promise): Promise { + const orig = process.stderr.write.bind(process.stderr); + let out = ''; + process.stderr.write = ((chunk: string | Uint8Array): boolean => { + out += typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8'); + return true; + }) as typeof process.stderr.write; + try { + await fn(); + } finally { + process.stderr.write = orig; + } + return out; +} + +beforeAll(async () => { + prevVerbose = process.env.GBRAIN_MIGRATE_VERBOSE; + prevSnapshot = process.env.GBRAIN_PGLITE_SNAPSHOT; + // Default state for the file: quiet mode active, and NO snapshot fast-path — + // a snapshot-restored engine skips runMigrations entirely, which would make + // every assertion below vacuous. + delete process.env.GBRAIN_MIGRATE_VERBOSE; + delete process.env.GBRAIN_PGLITE_SNAPSHOT; + + engineA = new PGLiteEngine(); + await engineA.connect({}); + // NOTE: initSchema() deliberately NOT called here — test 1 captures its + // stderr output as the fresh-install replay under test. +}); + +afterAll(async () => { + if (prevVerbose === undefined) delete process.env.GBRAIN_MIGRATE_VERBOSE; + else process.env.GBRAIN_MIGRATE_VERBOSE = prevVerbose; + if (prevSnapshot === undefined) delete process.env.GBRAIN_PGLITE_SNAPSHOT; + else process.env.GBRAIN_PGLITE_SNAPSHOT = prevSnapshot; + + await engineA.disconnect(); + if (engineB) await engineB.disconnect(); +}); + +describe('quiet fresh-install replay', () => { + test('fresh brain prints one summary line, no per-migration or notice lines', async () => { + const out = await captureStderr(async () => { + await engineA.initSchema(); + }); + + expect(out).toContain('Setting up brain schema (v'); + // No per-migration " [N] name..." progress lines. + expect(out).not.toMatch(/\[\d+\] \S+\.\.\./); + // No " [N] ✓ name" completion lines. + expect(out).not.toContain('✓'); + // The verbose header is replaced, not printed alongside. + expect(out).not.toContain('Schema version 1 →'); + // v123/v124 handler notices are suppressed via quietMigrationNotices. + expect(out).not.toContain('v123:'); + expect(out).not.toContain('v124:'); + + // The replay actually ran to completion. + expect(await engineA.getConfig('version')).toBe(String(LATEST_VERSION)); + }); + + test('GBRAIN_MIGRATE_VERBOSE=1 keeps full output on a fresh brain', async () => { + process.env.GBRAIN_MIGRATE_VERBOSE = '1'; + try { + engineB = new PGLiteEngine(); + await engineB.connect({}); + const out = await captureStderr(async () => { + await engineB!.initSchema(); + }); + + expect(out).toContain('Schema version 1 →'); + expect(out).toMatch(/\[\d+\] ✓ /); + expect(out).not.toContain('Setting up brain schema'); + } finally { + delete process.env.GBRAIN_MIGRATE_VERBOSE; + } + }); + + test('upgrade path (current > 1) stays verbose — and the quiet flag did not leak from test 1', async () => { + // Rewind engineA one version so exactly the last migration is pending. + // This runs in the SAME process AFTER test 1's quiet replay, so verbose + // output here also proves runMigrations' finally reset quietMigrationNotices. + await engineA.setConfig('version', String(LATEST_VERSION - 1)); + + let result: { applied: number; current: number } | undefined; + const out = await captureStderr(async () => { + result = await runMigrations(engineA); + }); + + expect(result?.applied).toBe(1); + expect(result?.current).toBe(LATEST_VERSION); + expect(out).toContain(`Schema version ${LATEST_VERSION - 1} → `); + expect(out).toContain('✓'); + expect(out).not.toContain('Setting up brain schema'); + }); + + test('no-pending run applies nothing and emits no setup/migration lines', async () => { + let result: { applied: number; current: number } | undefined; + const out = await captureStderr(async () => { + result = await runMigrations(engineA); + }); + + expect(result?.applied).toBe(0); + expect(result?.current).toBe(LATEST_VERSION); + expect(out).not.toContain('Setting up brain schema'); + expect(out).not.toContain('Schema version'); + expect(out).not.toContain('✓'); + }); +}); diff --git a/test/post-install-advisory.test.ts b/test/post-install-advisory.test.ts index 9058da1b2..4304d1abf 100644 --- a/test/post-install-advisory.test.ts +++ b/test/post-install-advisory.test.ts @@ -25,7 +25,9 @@ import { tmpdir } from 'os'; import { buildAdvisory, detectInstalledSlugs, + printAdvisoryIfRecommended, } from '../src/core/skillpack/post-install-advisory.ts'; +import { currentRecommendedSet } from '../src/core/advisor/recommended-set.ts'; const cleanup: string[] = []; @@ -218,6 +220,72 @@ describe('buildAdvisory — agent-readable framing', () => { }); }); +describe('printAdvisoryIfRecommended — compact init pointer vs full upgrade banner', () => { + function captureStderr(fn: () => void): string { + const orig = process.stderr.write; + let out = ''; + process.stderr.write = ((chunk: unknown) => { + out += String(chunk); + return true; + }) as typeof process.stderr.write; + try { + fn(); + } finally { + process.stderr.write = orig; + } + return out; + } + + it('context init with all skills missing prints the compact human-voiced pointer', () => { + const { workspace, skillsDir } = scratchWorkspace([]); + const out = captureStderr(() => + printAdvisoryIfRecommended({ + version: '0.25.1', + context: 'init', + targetWorkspace: workspace, + targetSkillsDir: skillsDir, + }), + ); + const names = currentRecommendedSet().map((s) => s.slug); + expect(out).toContain('recommended skill(s) not installed yet'); + // Preview truncates at 4 slugs + ellipsis; the 5th slug never appears + // (the scaffold command is --all when everything is missing). + expect(out).toContain(`(${names.slice(0, 4).join(', ')}, …)`); + expect(out).not.toContain(names[4]); + expect(out).toContain('gbrain advisor'); + // The compact init pointer is human-voiced — no agent stage directions. + expect(out).not.toContain('ACTION FOR THE AGENT'); + expect(out).not.toContain('[AGENT]'); + }); + + it('context init with everything installed prints NOTHING', () => { + const allSlugs = currentRecommendedSet().map((s) => s.slug); + const { workspace, skillsDir } = scratchWorkspace(allSlugs); + const out = captureStderr(() => + printAdvisoryIfRecommended({ + version: '0.25.1', + context: 'init', + targetWorkspace: workspace, + targetSkillsDir: skillsDir, + }), + ); + expect(out).toBe(''); + }); + + it('context upgrade with missing skills keeps the full agent-addressed banner', () => { + const { workspace, skillsDir } = scratchWorkspace([]); + const out = captureStderr(() => + printAdvisoryIfRecommended({ + version: '0.25.1', + context: 'upgrade', + targetWorkspace: workspace, + targetSkillsDir: skillsDir, + }), + ); + expect(out).toContain('ACTION FOR THE AGENT'); + }); +}); + describe('buildAdvisory — no workspace detected', () => { it('still renders an advisory with a workspace-detection note', () => { const advisory = buildAdvisory({ diff --git a/test/self-upgrade-pending.test.ts b/test/self-upgrade-pending.test.ts new file mode 100644 index 000000000..e80001b04 --- /dev/null +++ b/test/self-upgrade-pending.test.ts @@ -0,0 +1,128 @@ +/** + * Unit tests for `pendingUpgradeVersion` — the ONE shared "is an upgrade + * actually pending for THIS binary?" predicate (src/core/self-upgrade.ts). + * + * Every upgrade-nag surface (CLI startup marker, doctor, advisor, + * get_brain_identity) routes through it, so this file pins the suppression + * rule centrally: a stale or foreign cache (latest <= running version) must + * return null. The cache records the version of whatever binary WROTE it — + * an older gbrain on PATH can write `UPGRADE_AVAILABLE 0.0.1 X` — so the + * comparison must be against the RUNNING version, never `marker.current`. + * + * Also covers the advisor consumer (collect-version.ts): version_drift fires + * only when the shared predicate says an upgrade is pending. + */ +import { describe, expect, test } from 'bun:test'; +import { mkdtempSync, rmSync, utimesSync, writeFileSync, mkdirSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { withEnv } from './helpers/with-env.ts'; +import { + CACHE_TTL_UPGRADE_AVAILABLE_MS, + pendingUpgradeVersion, + updateCachePath, + writeUpdateCache, +} from '../src/core/self-upgrade.ts'; +import { collectVersion } from '../src/core/advisor/collect-version.ts'; +import type { AdvisorContext } from '../src/core/advisor/types.ts'; + +/** Run `fn` with GBRAIN_HOME pointed at a fresh temp dir (env restored after). */ +async function withHome(fn: (home: string) => T | Promise): Promise { + const dir = mkdtempSync(join(tmpdir(), 'gbrain-pending-')); + try { + return await withEnv({ GBRAIN_HOME: dir }, () => fn(dir)); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +/** Backdate the cache file's mtime so `isCacheFresh` sees it as expired. */ +function backdateCache(byMs: number): void { + const then = new Date(Date.now() - byMs); + utimesSync(updateCachePath(), then, then); +} + +describe('pendingUpgradeVersion', () => { + test('fresh cache, latest > running → returns latest', async () => { + await withHome(() => { + writeUpdateCache({ kind: 'upgrade_available', current: '0.42.0', latest: '0.99.0' }); + expect(pendingUpgradeVersion('0.42.0')).toBe('0.99.0'); + }); + }); + + test('fresh cache, latest == running → null (already current; suppress the nag)', async () => { + await withHome(() => { + writeUpdateCache({ kind: 'upgrade_available', current: '0.42.0', latest: '0.42.0' }); + expect(pendingUpgradeVersion('0.42.0')).toBeNull(); + }); + }); + + test('fresh cache, latest < running → null (downgrade/yanked never nags)', async () => { + await withHome(() => { + writeUpdateCache({ kind: 'upgrade_available', current: '0.0.1', latest: '0.42.0' }); + expect(pendingUpgradeVersion('0.99.0')).toBeNull(); + }); + }); + + test('foreign-writer cache: comparison is against the RUNNING version, not marker.current', async () => { + await withHome(() => { + // An old 0.0.1 binary on PATH wrote the cache. The running binary must + // compare ITS version to latest — marker.current is untrusted. + writeUpdateCache({ kind: 'upgrade_available', current: '0.0.1', latest: '0.99.0' }); + expect(pendingUpgradeVersion('0.99.0')).toBeNull(); // already on latest → suppressed + expect(pendingUpgradeVersion('0.42.0')).toBe('0.99.0'); // genuinely behind → nag + }); + }); + + test('stale cache (mtime beyond upgrade_available TTL) → null', async () => { + await withHome(() => { + writeUpdateCache({ kind: 'upgrade_available', current: '0.42.0', latest: '0.99.0' }); + backdateCache(CACHE_TTL_UPGRADE_AVAILABLE_MS + 60_000); + expect(pendingUpgradeVersion('0.42.0')).toBeNull(); + }); + }); + + test('missing cache → null', async () => { + await withHome(() => { + expect(pendingUpgradeVersion('0.42.0')).toBeNull(); + }); + }); + + test('up_to_date marker kind → null', async () => { + await withHome(() => { + writeUpdateCache({ kind: 'up_to_date', current: '0.42.0' }); + expect(pendingUpgradeVersion('0.42.0')).toBeNull(); + }); + }); + + test('corrupt cache content → null (never throws)', async () => { + await withHome((home) => { + mkdirSync(join(home, '.gbrain'), { recursive: true }); + writeFileSync(updateCachePath(), 'UPGRADE_AVAILABLE not-a-version; rm -rf /\n'); + expect(pendingUpgradeVersion('0.42.0')).toBeNull(); + }); + }); +}); + +describe('advisor collectVersion (consumer of the shared predicate)', () => { + const ctx = (version: string) => ({ version } as unknown as AdvisorContext); + + test('fresh cache with newer latest → one version_drift finding', async () => { + await withHome(async () => { + writeUpdateCache({ kind: 'upgrade_available', current: '0.42.0', latest: '0.99.0' }); + const findings = await collectVersion.collect(ctx('0.42.0')); + expect(findings).toHaveLength(1); + expect(findings[0].id).toBe('version_drift'); + expect(findings[0].title).toContain('0.99.0'); + expect(findings[0].title).toContain('0.42.0'); + }); + }); + + test('latest == ctx.version → no findings (stale/foreign cache suppressed)', async () => { + await withHome(async () => { + writeUpdateCache({ kind: 'upgrade_available', current: '0.42.0', latest: '0.99.0' }); + const findings = await collectVersion.collect(ctx('0.99.0')); + expect(findings).toEqual([]); + }); + }); +}); diff --git a/test/tty-harness.test.ts b/test/tty-harness.test.ts index 23e1eabef..75ad61c9d 100644 --- a/test/tty-harness.test.ts +++ b/test/tty-harness.test.ts @@ -207,9 +207,11 @@ describe.skipIf(!ptySupported())('launchTty (live PTY smoke)', () => { const frames = session.frames(); expect(frames.length).toBeGreaterThanOrEqual(2); // The 600ms sleep shows up as a measurable gap (loose bound: >= 300ms). + // Match the specific stall rather than index 0 — a slow spawn on a loaded + // CI box can prepend a startup stall ('(no output yet)') before it. const stalls = computeStalls(frames, { thresholdMs: 300 }); expect(stalls.length).toBeGreaterThanOrEqual(1); - expect(stalls[0]!.context).toContain('one'); + expect(stalls.some((s) => s.context.includes('one'))).toBe(true); }, 20_000); test('send + waitFor drive an interactive child; child sees a real TTY', async () => {