From 9c343bd637578d9639c38dc0ce6bc6c640a83a9b Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 25 Jul 2026 05:49:13 -0700 Subject: [PATCH 01/11] chore(porch): bugfix-1077 init bugfix --- .../status.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml diff --git a/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml b/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml new file mode 100644 index 000000000..7afc919c6 --- /dev/null +++ b/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml @@ -0,0 +1,14 @@ +id: bugfix-1077 +title: consult-pre-flight-auth-state- +protocol: bugfix +phase: investigate +plan_phases: [] +current_plan_phase: null +gates: + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-07-25T12:49:13.351Z' +updated_at: '2026-07-25T12:49:13.351Z' From 3d15f964a5c51104032ec25d906154382b576f4c Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 25 Jul 2026 06:08:35 -0700 Subject: [PATCH 02/11] [Bugfix #1077] Fix: pre-flight agy auth cache to stop the OAuth tab burst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An unauthenticated `agy` opens an OAuth browser tab before it prints the URL that AGY_OAUTH_MARKERS detection keys off, so the gemini lane's kill closes the barn door after the horse has left. A CMAP round is N independent consult processes, so N tabs stranded (12-15 observed in the wild). Add a filesystem-backed, cross-process auth-state cache consulted BEFORE the spawn. A file lock elects one prober per cold window; it publishes the verdict as soon as it is knowable and the rest short-circuit without spawning, so an unauthenticated agy is spawned at most once per TTL window instead of once per consult. The real consult run doubles as the probe rather than adding a dedicated one, so the authenticated path costs no extra agy spawn and marker detection stays in one place. Only positive marker evidence records 'unauth', and an undecided wait proceeds anyway — failures land on "one unnecessary tab", never on "silently dropped a working reviewer". doctor's verifyAgy shares the cache in both directions: it reports a fresh verdict without probing, and records the verdicts its own probe produces. --- .../src/commands/consult/agy-auth-cache.ts | 344 ++++++++++++++++++ packages/codev/src/commands/consult/index.ts | 48 +++ packages/codev/src/commands/doctor.ts | 26 +- 3 files changed, 416 insertions(+), 2 deletions(-) create mode 100644 packages/codev/src/commands/consult/agy-auth-cache.ts diff --git a/packages/codev/src/commands/consult/agy-auth-cache.ts b/packages/codev/src/commands/consult/agy-auth-cache.ts new file mode 100644 index 000000000..16a6159cc --- /dev/null +++ b/packages/codev/src/commands/consult/agy-auth-cache.ts @@ -0,0 +1,344 @@ +/** + * Cross-process auth-state cache for the Antigravity CLI (`agy`) — Issue #1077. + * + * ## Why this exists + * + * An unauthenticated `agy --print` opens an OAuth browser tab *before* it prints + * the OAuth URL that `AGY_OAUTH_MARKERS` detection keys off. So by the time the + * gemini consult lane recognises "not signed in" and kills the child, the tab is + * already on screen. A CMAP burst runs N consults as N separate OS processes, so + * N tabs land — 12-15 stranded tabs were observed in the wild. + * + * A module-level memo cannot help: each `consult` invocation is its own process. + * The state has to be shared through the filesystem. + * + * ## The protocol + * + * 1. Read `~/.cache/codev/agy-auth.json`. A *fresh* entry decides immediately: + * `unauth` → skip without spawning; `auth` → spawn as usual. + * 2. Cold or expired cache → race for an exclusive lock. Exactly one process + * wins and becomes the **prober**: it spawns agy for real and publishes the + * verdict the moment it is knowable (see `runAgyConsultation`). + * 3. Losers do **not** block on the lock — the prober may hold it for minutes on + * a real review. They poll the cache for a few seconds and act on the verdict + * as soon as it appears. + * + * ## Fail-open bias + * + * Only positive OAuth-marker evidence records `unauth`. A waiter that never sees + * a verdict proceeds with its spawn. Both choices push failures toward "one + * unnecessary browser tab" (the status quo) and away from "silently skipped a + * working reviewer", which would corrupt a CMAP round. + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { homedir } from 'node:os'; + +export type AgyAuthState = 'auth' | 'unauth'; + +/** Cache entry persisted to `agy-auth.json`. */ +interface AgyAuthCacheEntry { + state: AgyAuthState; + /** epoch ms of the probe that produced `state`. */ + checkedAt: number; + /** Binary the verdict applies to — a different binary invalidates it. */ + agyBinPath: string; + /** Newest mtime across agy's candidate credential files, or null if none found. */ + agyAuthMtime: number | null; +} + +/** + * `unauth` expires fast: a user who signs in elsewhere should get the lane back + * without clearing anything by hand. `auth` lasts longer because re-probing a + * working agy is pure overhead. + */ +const DEFAULT_TTL_UNAUTH_MS = 5 * 60 * 1000; +const DEFAULT_TTL_AUTH_MS = 30 * 60 * 1000; + +/** How long a waiter polls for the prober's verdict before failing open. */ +const DEFAULT_WAIT_MS = 6000; +const WAIT_POLL_INTERVAL_MS = 100; + +/** + * A lock older than this is treated as abandoned (prober crashed / was killed + * mid-run) and reclaimed. Generous enough to cover a prober that is still + * deciding, short enough that a crash does not wedge the lane for a whole TTL. + */ +const LOCK_STALE_MS = 60 * 1000; + +/** + * Files agy plausibly writes when an OAuth login completes. Purely a *hint*: a + * changed mtime invalidates the cache early, so a sign-in is picked up before + * the TTL lapses. agy's credential location is not documented and has moved + * between releases, so this list is best-effort by design and every entry is + * optional — when none exist, `agyAuthMtime` is null and the TTL is the only + * recovery window (which is sufficient on its own). + * + * Deliberately excludes general settings/state files: those churn for unrelated + * reasons, and a spurious invalidation costs a browser tab. + */ +const AGY_CREDENTIAL_CANDIDATES = [ + ['.antigravity', 'credentials.json'], + ['Library', 'Application Support', 'Antigravity', 'credentials.json'], + ['.gemini', 'antigravity-cli', 'credentials.json'], + ['.gemini', 'antigravity-cli', 'oauth_creds.json'], + ['.gemini', 'oauth_creds.json'], + ['.gemini', 'google_accounts.json'], +]; + +function envInt(name: string, fallback: number): number { + const raw = process.env[name]; + if (!raw) return fallback; + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed >= 0 ? parsed : fallback; +} + +function ttlFor(state: AgyAuthState): number { + return state === 'unauth' + ? envInt('CODEV_AGY_AUTH_CACHE_TTL_UNAUTH_MS', DEFAULT_TTL_UNAUTH_MS) + : envInt('CODEV_AGY_AUTH_CACHE_TTL_AUTH_MS', DEFAULT_TTL_AUTH_MS); +} + +/** + * Opt-out escape hatch: restores the pre-#1077 always-spawn behaviour. + * + * Also inert under a test runner unless the suite names a cache directory. This + * cache is a *user-global* side effect: without the guard, any test that reaches + * `doctor()` or the gemini lane would record a verdict into the developer's real + * `~/.cache/codev` and, worse, later tests would silently take the cached branch + * and stop spawning what they assert on. Suites that mean to exercise the cache + * set `CODEV_AGY_AUTH_CACHE_DIR` and get the full behaviour. + */ +export function agyAuthCacheDisabled(): boolean { + const raw = process.env.CODEV_AGY_AUTH_CACHE_DISABLE; + if (raw === '1' || raw === 'true') return true; + return Boolean(process.env.VITEST) && !process.env.CODEV_AGY_AUTH_CACHE_DIR; +} + +/** + * Cache directory. `CODEV_AGY_AUTH_CACHE_DIR` keeps tests off the real user + * cache; otherwise XDG_CACHE_HOME, else `~/.cache`. + */ +export function agyAuthCacheDir(): string { + const override = process.env.CODEV_AGY_AUTH_CACHE_DIR; + if (override) return override; + const xdg = process.env.XDG_CACHE_HOME; + return path.join(xdg && xdg.startsWith('/') ? xdg : path.join(homedir(), '.cache'), 'codev'); +} + +function cacheFilePath(): string { + return path.join(agyAuthCacheDir(), 'agy-auth.json'); +} + +function lockFilePath(): string { + return path.join(agyAuthCacheDir(), 'agy-auth.lock'); +} + +/** Newest mtime across the candidate credential files; null when none exist. */ +function currentAgyAuthMtime(): number | null { + let newest: number | null = null; + for (const parts of AGY_CREDENTIAL_CANDIDATES) { + try { + const stat = fs.statSync(path.join(homedir(), ...parts)); + const mtime = stat.mtimeMs; + if (newest === null || mtime > newest) newest = mtime; + } catch { + // Absent or unreadable — this list is a hint, not a requirement. + } + } + return newest; +} + +function readEntry(): AgyAuthCacheEntry | null { + try { + const parsed = JSON.parse(fs.readFileSync(cacheFilePath(), 'utf-8')) as unknown; + if (!parsed || typeof parsed !== 'object') return null; + const e = parsed as Partial; + if (e.state !== 'auth' && e.state !== 'unauth') return null; + if (typeof e.checkedAt !== 'number' || !Number.isFinite(e.checkedAt)) return null; + if (typeof e.agyBinPath !== 'string') return null; + return { + state: e.state, + checkedAt: e.checkedAt, + agyBinPath: e.agyBinPath, + agyAuthMtime: typeof e.agyAuthMtime === 'number' ? e.agyAuthMtime : null, + }; + } catch { + // Missing, truncated, or concurrently-rewritten file — treat as cold. + return null; + } +} + +/** + * The cached auth state for `agyBinPath`, or null when there is nothing + * trustworthy to act on (cold, expired, different binary, or a credential file + * changed since the probe). + */ +export function checkCachedAgyAuth(agyBinPath: string): AgyAuthState | null { + if (agyAuthCacheDisabled()) return null; + const entry = readEntry(); + if (!entry) return null; + // A reinstall or a CODEV_AGY_BIN change means the verdict describes a + // different binary than the one we are about to run. + if (entry.agyBinPath !== agyBinPath) return null; + if (Date.now() - entry.checkedAt >= ttlFor(entry.state)) return null; + // Credentials moved under us → someone signed in (or out); re-probe. + if (entry.agyAuthMtime !== currentAgyAuthMtime()) return null; + return entry.state; +} + +/** + * Persist a verdict. Written via a temp file + rename so a concurrent reader + * never observes a half-written JSON document. + */ +export function recordAgyAuthState(state: AgyAuthState, agyBinPath: string): void { + if (agyAuthCacheDisabled()) return; + const entry: AgyAuthCacheEntry = { + state, + checkedAt: Date.now(), + agyBinPath, + agyAuthMtime: currentAgyAuthMtime(), + }; + try { + const dir = agyAuthCacheDir(); + fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); + const tmp = path.join(dir, `agy-auth.${process.pid}.tmp`); + fs.writeFileSync(tmp, JSON.stringify(entry), { mode: 0o600 }); + fs.renameSync(tmp, cacheFilePath()); + } catch { + // A cache we cannot write is a missed optimisation, never a failed consult. + } +} + +/** Test/diagnostic helper: drop the cache file and any lock. */ +export function clearAgyAuthCache(): void { + for (const p of [cacheFilePath(), lockFilePath()]) { + try { fs.unlinkSync(p); } catch { /* already gone */ } + } +} + +/** + * Try to become the prober. Returns a release function on success, null when + * another process holds the lock. An abandoned lock (older than LOCK_STALE_MS) + * is reclaimed once. + */ +function acquireProbeLock(): (() => void) | null { + const lockPath = lockFilePath(); + const attempt = (): boolean => { + try { + fs.mkdirSync(agyAuthCacheDir(), { recursive: true, mode: 0o700 }); + // 'wx' fails when the file exists — the atomic test-and-set we need. + const fd = fs.openSync(lockPath, 'wx', 0o600); + fs.writeSync(fd, `${process.pid} ${Date.now()}\n`); + fs.closeSync(fd); + return true; + } catch { + return false; + } + }; + + if (attempt()) return () => { try { fs.unlinkSync(lockPath); } catch { /* already gone */ } }; + + try { + if (Date.now() - fs.statSync(lockPath).mtimeMs > LOCK_STALE_MS) { + fs.unlinkSync(lockPath); + if (attempt()) return () => { try { fs.unlinkSync(lockPath); } catch { /* already gone */ } }; + } + } catch { + // Lock vanished mid-inspection (holder finished) — fall through to waiting. + } + return null; +} + +/** + * Poll for the prober's verdict. Resolves with the state as soon as one lands, + * or null if the wait elapses first. + * + * Polls the *cache* rather than waiting on the lock: the prober holds the lock + * for its entire agy run, which can be minutes, but publishes its verdict within + * seconds. + */ +export async function waitForAgyAuthState( + agyBinPath: string, + timeoutMs = envInt('CODEV_AGY_AUTH_CACHE_WAIT_MS', DEFAULT_WAIT_MS), +): Promise { + const deadline = Date.now() + timeoutMs; + for (;;) { + const state = checkCachedAgyAuth(agyBinPath); + if (state) return state; + if (Date.now() >= deadline) return null; + await new Promise((r) => setTimeout(r, WAIT_POLL_INTERVAL_MS)); + } +} + +export interface AgyAuthPreflight { + /** `skip` → do not spawn agy at all; `proceed` → spawn. */ + action: 'skip' | 'proceed'; + /** Skip reason, suitable for a user-facing message. Set iff action is `skip`. */ + reason?: string; + /** + * True when this process won the probe lock. Such a process MUST report what + * it learns via `publish` — every other process is waiting on it. + */ + isProber: boolean; + /** + * Record a verdict and release the lock. Idempotent, and a no-op for + * non-probers, so callers can invoke it from every exit path unconditionally. + */ + publish: (state: AgyAuthState) => void; + /** Release the lock without recording anything (abnormal exits). Idempotent. */ + release: () => void; +} + +const SKIP_REASON_CACHED_UNAUTH = + 'agy unauthenticated (cached); run `agy` once to sign in'; + +/** + * Decide whether to spawn agy, before spawning it. + * + * Callers that receive `proceed` with `isProber: true` are on the hook to + * `publish` what the spawn reveals; see the module header for the full protocol. + */ +export async function preflightAgyAuth(agyBinPath: string): Promise { + const inert: AgyAuthPreflight = { + action: 'proceed', + isProber: false, + publish: () => {}, + release: () => {}, + }; + if (agyAuthCacheDisabled()) return inert; + + const cached = checkCachedAgyAuth(agyBinPath); + if (cached === 'unauth') { + return { ...inert, action: 'skip', reason: SKIP_REASON_CACHED_UNAUTH }; + } + if (cached === 'auth') return inert; + + const release = acquireProbeLock(); + if (release) { + let done = false; + const finish = (state?: AgyAuthState) => { + if (done) return; + done = true; + if (state) recordAgyAuthState(state, agyBinPath); + release(); + }; + return { + action: 'proceed', + isProber: true, + publish: (state) => finish(state), + release: () => finish(), + }; + } + + // Another process is probing. Wait briefly for its verdict rather than adding + // a spawn (and a browser tab) of our own. + const observed = await waitForAgyAuthState(agyBinPath); + if (observed === 'unauth') { + return { ...inert, action: 'skip', reason: SKIP_REASON_CACHED_UNAUTH }; + } + // `auth`, or no verdict in time: proceed. Failing open keeps a slow or crashed + // prober from silently dropping a reviewer that would have worked. + return inert; +} diff --git a/packages/codev/src/commands/consult/index.ts b/packages/codev/src/commands/consult/index.ts index 28d3bfc0a..d0a3fbc67 100644 --- a/packages/codev/src/commands/consult/index.ts +++ b/packages/codev/src/commands/consult/index.ts @@ -21,6 +21,7 @@ import { getResolver, GitRefResolver, type ArtifactResolver } from '../porch/art import { MetricsDB } from './metrics.js'; import { extractUsage, extractReviewText, type SDKResultLike, type UsageData } from './usage-extractor.js'; import { executeForgeCommandSync } from '../../lib/forge.js'; +import { preflightAgyAuth, type AgyAuthState } from './agy-auth-cache.js'; // Content reference — resolved artifact content with a display label interface ContentRef { @@ -635,6 +636,14 @@ const AGY_PRINT_TIMEOUT = '5m'; // passed to `agy --print-timeou const AGY_TIMEOUT_MS = 6 * 60 * 1000; // Codev-owned hard cap (> agy's own timeout) // OAuth banner appears before any review text; only scan the early stream. const AGY_MARKER_SCAN_LIMIT = 8192; +/** + * How long a prober waits, marker-free, before publishing `auth` to the shared + * cache (#1077). The OAuth banner is the very first thing an unauthenticated agy + * emits, so silence this far in is good evidence we are signed in — and it lets + * the processes waiting on our verdict get moving instead of stalling behind a + * review that may run for minutes. + */ +const AGY_AUTH_GRACE_MS = 3000; // agy's own print-timeout message: on an agentic task that outruns --print-timeout, // it returns this (often with a "monitoring the task" note) instead of a review. // Treat it as a non-response → non-blocking skip rather than a garbage "review". @@ -788,6 +797,21 @@ async function runAgyConsultation( return; } + // Pre-flight the shared auth cache BEFORE spawning (#1077). An unauthenticated + // agy opens a browser tab before it prints the OAuth URL we detect below, so + // post-spawn detection cannot prevent the tab — only not spawning can. Across a + // CMAP burst this collapses N stranded tabs into at most one per TTL window. + const preflight = await preflightAgyAuth(bin); + if (preflight.action === 'skip') { + const reason = preflight.reason ?? 'agy unauthenticated (cached)'; + const content = agySkipContent(reason); + process.stdout.write(content); + writeConsultOutput(outputPath, content); + recordAgyMetrics(metricsCtx, startTime, 0, reason); + console.error(`\n[gemini (agy) skipped without spawning: ${reason}]`); + return; + } + // agy has no system-prompt flag — fold the role into the prompt (hermes precedent). // Prepend strict constraint directive to force agy to remain on-task, prevent exploratory actions // that lead to wandering off-task, and avoid unrelated local skills/directories (Issue #1032). @@ -841,12 +865,28 @@ async function runAgyConsultation( let scanBuf = ''; let settled = false; + // When we hold the probe lock, other consult processes are polling the cache + // for our verdict — publish it as soon as it is knowable, and always release + // the lock, whatever this run turns into. + const graceTimer = preflight.isProber + ? setTimeout(() => preflight.publish('auth'), AGY_AUTH_GRACE_MS) + : null; + const publishAuth = (state?: AgyAuthState) => { + if (graceTimer) clearTimeout(graceTimer); + if (state) preflight.publish(state); + else preflight.release(); + }; + const settleSkip = (reason: string, exitCode = 0) => { if (settled) return; settled = true; clearTimeout(timer); try { proc.kill('SIGTERM'); } catch { /* already gone */ } cleanup(); + // No-op if a verdict was already published (e.g. the OAuth-marker path); + // otherwise this just releases the lock — a timeout or a spawn failure is + // not evidence either way about authentication. + publishAuth(); const content = agySkipContent(reason); process.stdout.write(content); writeConsultOutput(outputPath, content); @@ -865,6 +905,9 @@ async function runAgyConsultation( if (scanBuf.length < AGY_MARKER_SCAN_LIMIT) { scanBuf += buf.toString('utf-8'); if (AGY_OAUTH_MARKERS.some((m) => scanBuf.includes(m))) { + // The one place we have positive proof of being signed out. Publish it + // so the rest of the burst skips without spawning (and without tabs). + publishAuth('unauth'); settleSkip('agy not authenticated — run `agy` once to sign in (OAuth)', 1); } } @@ -883,6 +926,9 @@ async function runAgyConsultation( cleanup(); const raw = Buffer.concat(outChunks).toString('utf-8').trim(); if (code !== 0 || raw.length === 0 || raw.includes(AGY_NONRESPONSE_MARKER)) { + // A broken run tells us nothing about auth — release without a verdict + // and let the next call re-probe. + publishAuth(); const reason = code !== 0 ? `agy exited with code ${code}` : raw.includes(AGY_NONRESPONSE_MARKER) @@ -896,6 +942,8 @@ async function runAgyConsultation( resolve(); return; } + // A real review is conclusive proof agy is signed in. + publishAuth('auth'); // Plain-text stdout IS the review. process.stdout.write(raw); writeConsultOutput(outputPath, raw); diff --git a/packages/codev/src/commands/doctor.ts b/packages/codev/src/commands/doctor.ts index 1bab37d30..a92b962ca 100644 --- a/packages/codev/src/commands/doctor.ts +++ b/packages/codev/src/commands/doctor.ts @@ -29,6 +29,7 @@ import { STALE_BACKUP_AGE_DAYS, } from '../lib/migration-backup-audit.js'; import { resolveAgyBin, AGY_OAUTH_MARKERS } from './consult/index.js'; +import { checkCachedAgyAuth, recordAgyAuthState } from './consult/agy-auth-cache.js'; import { AGENT_FARM_DIR } from '@cluesmith/codev-core/constants'; import { measureSessionLogs, @@ -468,11 +469,24 @@ function checkAgy(): CheckResult { * unauthenticated agy reports "needs login" promptly (it would otherwise print * the URL and wait ~30s) — rather than stalling `codev doctor` for the full * auth wait. Always resolves (never throws). + * + * Shares the consult lane's auth cache (#1077): a fresh verdict is reported + * without probing, so `codev doctor` on an unauthenticated agy does not open yet + * another OAuth browser tab. When it does probe, it records the result for the + * consult lane's benefit. */ function verifyAgy(): Promise { const bin = resolveAgyBin(); if (!bin) return Promise.resolve({ status: 'skip', version: 'not installed', note: AGY_INSTALL_HINT }); + const cached = checkCachedAgyAuth(bin); + if (cached === 'unauth') { + return Promise.resolve({ status: 'fail', version: 'needs login', note: 'run `agy` once to sign in (OAuth)' }); + } + if (cached === 'auth') { + return Promise.resolve({ status: 'ok', version: 'operational (cached)' }); + } + return new Promise((resolve) => { const proc = spawn(bin, ['--print-timeout', '20s', '--print', 'Reply with just OK'], { stdio: ['ignore', 'pipe', 'pipe'], @@ -502,6 +516,8 @@ function verifyAgy(): Promise { scan += s; // Fast path: OAuth URL appears immediately on an unauthenticated run. if (AGY_OAUTH_MARKERS.some((m) => scan.includes(m))) { + // Share the verdict so a concurrent CMAP burst skips without spawning. + recordAgyAuthState('unauth', bin); finish({ status: 'fail', version: 'needs login', note: 'run `agy` once to sign in (OAuth)' }); } } @@ -511,8 +527,14 @@ function verifyAgy(): Promise { proc.on('error', () => finish({ status: 'fail', version: 'error', note: 'run `agy` to verify sign-in' })); proc.on('close', (code) => { const text = out.join('').trim(); - if (code === 0 && text.length > 0) finish({ status: 'ok', version: 'operational' }); - else finish({ status: 'fail', version: 'not responding', note: 'run `agy` to verify sign-in' }); + if (code === 0 && text.length > 0) { + recordAgyAuthState('auth', bin); + finish({ status: 'ok', version: 'operational' }); + } else { + // Neither authenticated nor proven signed-out — record nothing, so the + // consult lane re-probes rather than skipping a lane that may work. + finish({ status: 'fail', version: 'not responding', note: 'run `agy` to verify sign-in' }); + } }); }); } From d85faf483c1cefa56204497189ca4a3b507963ca Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 25 Jul 2026 06:08:35 -0700 Subject: [PATCH 03/11] [Bugfix #1077] Test: regression coverage for the agy spawn burst The headline test drives the real lane (_runAgyConsultation) against a fake agy that logs every invocation, so 'did we spawn?' is asserted against observed process starts rather than mocks: a 5-wide unauthenticated burst must produce exactly one spawn. A companion case asserts 3 spawns with the cache disabled, which is what proves the burst test measures the cache and not something else. Also covers TTL expiry per state, bin-path and credential-mtime invalidation, corrupt/unknown entries, lock election, stale-lock reclaim, sign-in recovery after the unauth TTL, and that an authenticated burst still runs every lane. consult/doctor suites pin CODEV_AGY_AUTH_CACHE_DIR: the cache is a user-global side effect, and an unpinned run both wrote the developer's ~/.cache/codev and let one case's verdict suppress the spawn a sibling asserts on. --- packages/codev/src/__tests__/consult.test.ts | 6 + packages/codev/src/__tests__/doctor.test.ts | 9 + .../consult/__tests__/agy-auth-cache.test.ts | 355 ++++++++++++++++++ 3 files changed, 370 insertions(+) create mode 100644 packages/codev/src/commands/consult/__tests__/agy-auth-cache.test.ts diff --git a/packages/codev/src/__tests__/consult.test.ts b/packages/codev/src/__tests__/consult.test.ts index ac8cde623..8ff853e74 100644 --- a/packages/codev/src/__tests__/consult.test.ts +++ b/packages/codev/src/__tests__/consult.test.ts @@ -731,6 +731,11 @@ describe('consult command', () => { agyBin = path.join(testBaseDir, 'agy-fake'); fs.writeFileSync(agyBin, '#!/bin/sh\n'); process.env.CODEV_AGY_BIN = agyBin; + // The lane consults a cross-process auth cache before spawning (#1077). + // Pin it inside the per-test dir: otherwise a verdict recorded by one case + // changes whether the next one spawns at all, and the run would write into + // the developer's real ~/.cache/codev. + process.env.CODEV_AGY_AUTH_CACHE_DIR = path.join(testBaseDir, 'agy-auth-cache'); fs.mkdirSync(path.join(testBaseDir, 'codev', 'roles'), { recursive: true }); fs.writeFileSync( path.join(testBaseDir, 'codev', 'roles', 'consultant.md'), @@ -741,6 +746,7 @@ describe('consult command', () => { afterEach(() => { delete process.env.CODEV_AGY_BIN; + delete process.env.CODEV_AGY_AUTH_CACHE_DIR; }); async function loadAgy() { diff --git a/packages/codev/src/__tests__/doctor.test.ts b/packages/codev/src/__tests__/doctor.test.ts index 9292e9084..192d2df7d 100644 --- a/packages/codev/src/__tests__/doctor.test.ts +++ b/packages/codev/src/__tests__/doctor.test.ts @@ -717,15 +717,24 @@ describe('doctor command', () => { describe('AI model verification (Issue #128)', () => { const testBaseDir = path.join(tmpdir(), `codev-doctor-ai-test-${Date.now()}`); let originalCwd: string; + let savedAgyCacheDir: string | undefined; beforeEach(() => { originalCwd = process.cwd(); fs.mkdirSync(path.join(testBaseDir, 'codev', 'consult-types'), { recursive: true }); + // verifyAgy shares the consult lane's auth cache (#1077): pin it to a + // per-test directory so these cases neither read a verdict left by a + // sibling case (which would suppress the probe they assert on) nor write + // into the developer's real ~/.cache/codev. + savedAgyCacheDir = process.env.CODEV_AGY_AUTH_CACHE_DIR; + process.env.CODEV_AGY_AUTH_CACHE_DIR = path.join(testBaseDir, 'agy-auth-cache'); process.chdir(testBaseDir); }); afterEach(() => { process.chdir(originalCwd); + if (savedAgyCacheDir === undefined) delete process.env.CODEV_AGY_AUTH_CACHE_DIR; + else process.env.CODEV_AGY_AUTH_CACHE_DIR = savedAgyCacheDir; if (fs.existsSync(testBaseDir)) { fs.rmSync(testBaseDir, { recursive: true }); } diff --git a/packages/codev/src/commands/consult/__tests__/agy-auth-cache.test.ts b/packages/codev/src/commands/consult/__tests__/agy-auth-cache.test.ts new file mode 100644 index 000000000..82a2fab5f --- /dev/null +++ b/packages/codev/src/commands/consult/__tests__/agy-auth-cache.test.ts @@ -0,0 +1,355 @@ +/** + * Regression tests for the agy auth-state pre-flight cache (Issue #1077). + * + * The bug: an unauthenticated `agy` opens an OAuth browser tab *before* printing + * the OAuth URL that the gemini lane detects, so post-spawn detection cannot stop + * the tab. A CMAP burst is N separate consult processes, so N tabs landed + * (12-15 observed in the wild). + * + * The lane's guarantee, pinned below: across a burst against an unauthenticated + * agy, **agy is spawned at most once per TTL window**. Every other call decides + * from the shared filesystem cache and never spawns. + * + * These tests use a fake `agy` (via `CODEV_AGY_BIN`) that records every + * invocation to a log file — so "did we spawn?" is asserted against observed + * process starts, not mocks. Because the cache and lock live entirely on disk, + * concurrent in-process calls exercise exactly the same coordination path that + * separate `consult` processes do. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { + checkCachedAgyAuth, + recordAgyAuthState, + clearAgyAuthCache, + preflightAgyAuth, + waitForAgyAuthState, + agyAuthCacheDir, +} from '../agy-auth-cache.js'; +import { _runAgyConsultation } from '../index.js'; + +/** Env keys this suite manipulates; restored wholesale after each test. */ +const ENV_KEYS = [ + 'CODEV_AGY_BIN', + 'CODEV_AGY_AUTH_CACHE_DIR', + 'CODEV_AGY_AUTH_CACHE_DISABLE', + 'CODEV_AGY_AUTH_CACHE_TTL_UNAUTH_MS', + 'CODEV_AGY_AUTH_CACHE_TTL_AUTH_MS', + 'CODEV_AGY_AUTH_CACHE_WAIT_MS', + 'FAKE_AGY_LOG', + 'FAKE_AGY_MODE', +] as const; + +let dir: string; +let savedEnv: Record; + +/** Absolute path of the fake agy binary for the current test. */ +let fakeAgy: string; +/** File the fake agy appends to on every invocation. */ +let spawnLog: string; + +/** + * A stand-in for the `agy` CLI. `FAKE_AGY_MODE=unauth` reproduces the upstream + * behaviour this issue is about: print the OAuth banner, then sit there waiting + * for an interactive login that can never complete headlessly. + */ +const FAKE_AGY_SOURCE = `#!/usr/bin/env node +const fs = require('node:fs'); +fs.appendFileSync(process.env.FAKE_AGY_LOG, process.pid + '\\n'); +if (process.argv[2] === '--version') { console.log('1.0.10-fake'); process.exit(0); } +if (process.env.FAKE_AGY_MODE === 'unauth') { + process.stderr.write('Please visit https://accounts.google.com/o/oauth2/auth?client_id=fake\\n'); + setTimeout(() => process.exit(1), 30000); // hang like real agy; the lane kills us +} else { + process.stdout.write('---\\nVERDICT: APPROVE\\nSUMMARY: ok\\nCONFIDENCE: HIGH\\n---\\n'); + process.exit(0); +} +`; + +/** Invocations of the fake agy so far — i.e. how many browser tabs would exist. */ +function spawnCount(): number { + if (!fs.existsSync(spawnLog)) return 0; + return fs.readFileSync(spawnLog, 'utf-8').split('\n').filter(Boolean).length; +} + +function setMode(mode: 'auth' | 'unauth'): void { + process.env.FAKE_AGY_MODE = mode; +} + +beforeEach(() => { + savedEnv = {}; + for (const k of ENV_KEYS) savedEnv[k] = process.env[k]; + + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'agy-auth-cache-test-')); + fakeAgy = path.join(dir, 'agy'); + fs.writeFileSync(fakeAgy, FAKE_AGY_SOURCE, { mode: 0o755 }); + spawnLog = path.join(dir, 'spawns.log'); + fs.writeFileSync(spawnLog, ''); + + process.env.CODEV_AGY_BIN = fakeAgy; + process.env.CODEV_AGY_AUTH_CACHE_DIR = path.join(dir, 'cache'); + process.env.FAKE_AGY_LOG = spawnLog; + delete process.env.CODEV_AGY_AUTH_CACHE_DISABLE; +}); + +afterEach(() => { + for (const k of ENV_KEYS) { + if (savedEnv[k] === undefined) delete process.env[k]; + else process.env[k] = savedEnv[k]; + } + fs.rmSync(dir, { recursive: true, force: true }); + vi.restoreAllMocks(); +}); + +describe('agy auth-state cache', () => { + it('reports nothing on a cold cache', () => { + expect(checkCachedAgyAuth(fakeAgy)).toBeNull(); + }); + + it('round-trips both states', () => { + recordAgyAuthState('unauth', fakeAgy); + expect(checkCachedAgyAuth(fakeAgy)).toBe('unauth'); + recordAgyAuthState('auth', fakeAgy); + expect(checkCachedAgyAuth(fakeAgy)).toBe('auth'); + }); + + it('writes the cache file 0600 in a 0700 directory', () => { + recordAgyAuthState('unauth', fakeAgy); + const cacheFile = path.join(agyAuthCacheDir(), 'agy-auth.json'); + expect(fs.statSync(cacheFile).mode & 0o777).toBe(0o600); + expect(fs.statSync(agyAuthCacheDir()).mode & 0o777).toBe(0o700); + }); + + it('expires an unauth verdict once its TTL lapses', async () => { + process.env.CODEV_AGY_AUTH_CACHE_TTL_UNAUTH_MS = '20'; + recordAgyAuthState('unauth', fakeAgy); + expect(checkCachedAgyAuth(fakeAgy)).toBe('unauth'); + await new Promise((r) => setTimeout(r, 40)); + expect(checkCachedAgyAuth(fakeAgy)).toBeNull(); + }); + + it('applies the auth and unauth TTLs independently', async () => { + process.env.CODEV_AGY_AUTH_CACHE_TTL_UNAUTH_MS = '20'; + process.env.CODEV_AGY_AUTH_CACHE_TTL_AUTH_MS = '60000'; + recordAgyAuthState('auth', fakeAgy); + await new Promise((r) => setTimeout(r, 40)); + // Past the *unauth* TTL but well inside the auth one. + expect(checkCachedAgyAuth(fakeAgy)).toBe('auth'); + }); + + it('ignores a verdict recorded for a different agy binary', () => { + recordAgyAuthState('unauth', '/some/other/agy'); + expect(checkCachedAgyAuth(fakeAgy)).toBeNull(); + }); + + it('ignores a verdict whose credential-mtime hint no longer matches', () => { + // Simulates the user completing OAuth: agy rewrites its credentials, so the + // recorded hint goes stale and the verdict must not be trusted. + fs.mkdirSync(agyAuthCacheDir(), { recursive: true }); + fs.writeFileSync( + path.join(agyAuthCacheDir(), 'agy-auth.json'), + JSON.stringify({ state: 'unauth', checkedAt: Date.now(), agyBinPath: fakeAgy, agyAuthMtime: 12345 }), + ); + expect(checkCachedAgyAuth(fakeAgy)).toBeNull(); + }); + + it('treats a corrupt cache file as cold rather than throwing', () => { + fs.mkdirSync(agyAuthCacheDir(), { recursive: true }); + fs.writeFileSync(path.join(agyAuthCacheDir(), 'agy-auth.json'), '{not json'); + expect(checkCachedAgyAuth(fakeAgy)).toBeNull(); + }); + + it('rejects an entry with an unrecognised state', () => { + fs.mkdirSync(agyAuthCacheDir(), { recursive: true }); + fs.writeFileSync( + path.join(agyAuthCacheDir(), 'agy-auth.json'), + JSON.stringify({ state: 'maybe', checkedAt: Date.now(), agyBinPath: fakeAgy, agyAuthMtime: null }), + ); + expect(checkCachedAgyAuth(fakeAgy)).toBeNull(); + }); + + it('goes inert when disabled, neither reading nor writing', () => { + recordAgyAuthState('unauth', fakeAgy); + process.env.CODEV_AGY_AUTH_CACHE_DISABLE = '1'; + expect(checkCachedAgyAuth(fakeAgy)).toBeNull(); + recordAgyAuthState('auth', fakeAgy); // must not overwrite + delete process.env.CODEV_AGY_AUTH_CACHE_DISABLE; + expect(checkCachedAgyAuth(fakeAgy)).toBe('unauth'); + }); + + it('clears the cache on request', () => { + recordAgyAuthState('unauth', fakeAgy); + clearAgyAuthCache(); + expect(checkCachedAgyAuth(fakeAgy)).toBeNull(); + }); + + it('stops waiting and returns null when no verdict arrives', async () => { + const started = Date.now(); + expect(await waitForAgyAuthState(fakeAgy, 150)).toBeNull(); + expect(Date.now() - started).toBeGreaterThanOrEqual(140); + }); + + it('returns the verdict as soon as another process publishes it', async () => { + setTimeout(() => recordAgyAuthState('unauth', fakeAgy), 50); + expect(await waitForAgyAuthState(fakeAgy, 3000)).toBe('unauth'); + }); +}); + +describe('agy auth pre-flight', () => { + it('elects exactly one prober among concurrent callers', async () => { + // Nothing publishes a verdict here, so the losers ride out the fail-open + // wait — shorten it so the test does not sit through the real one. + process.env.CODEV_AGY_AUTH_CACHE_WAIT_MS = '150'; + const results = await Promise.all([ + preflightAgyAuth(fakeAgy), + preflightAgyAuth(fakeAgy), + preflightAgyAuth(fakeAgy), + ]); + // Losers fall through to the (short) wait, then fail open — so they all say + // "proceed", but only one of them is on the hook to publish a verdict. + expect(results.filter((r) => r.isProber)).toHaveLength(1); + results.forEach((r) => r.release()); + }); + + it('skips without probing when a fresh unauth verdict exists', async () => { + recordAgyAuthState('unauth', fakeAgy); + const pre = await preflightAgyAuth(fakeAgy); + expect(pre.action).toBe('skip'); + expect(pre.reason).toMatch(/cached/); + expect(pre.isProber).toBe(false); + }); + + it('proceeds without probing when a fresh auth verdict exists', async () => { + recordAgyAuthState('auth', fakeAgy); + const pre = await preflightAgyAuth(fakeAgy); + expect(pre.action).toBe('proceed'); + expect(pre.isProber).toBe(false); + }); + + it('releases the lock so the next caller can probe', async () => { + const first = await preflightAgyAuth(fakeAgy); + expect(first.isProber).toBe(true); + first.release(); + const second = await preflightAgyAuth(fakeAgy); + expect(second.isProber).toBe(true); + second.release(); + }); + + it('reclaims a lock abandoned by a crashed prober', async () => { + const lock = path.join(agyAuthCacheDir(), 'agy-auth.lock'); + fs.mkdirSync(agyAuthCacheDir(), { recursive: true }); + fs.writeFileSync(lock, '99999 0\n'); + // Backdate past the staleness threshold (60s). + const old = new Date(Date.now() - 5 * 60 * 1000); + fs.utimesSync(lock, old, old); + + const pre = await preflightAgyAuth(fakeAgy); + expect(pre.isProber).toBe(true); + pre.release(); + }); + + it('is inert when the cache is disabled', async () => { + process.env.CODEV_AGY_AUTH_CACHE_DISABLE = '1'; + recordAgyAuthState('unauth', fakeAgy); // no-op while disabled + const pre = await preflightAgyAuth(fakeAgy); + expect(pre.action).toBe('proceed'); + expect(pre.isProber).toBe(false); + }); +}); + +describe('gemini lane burst behaviour (#1077 regression)', () => { + /** Silence the lane's review/skip output so test logs stay readable. */ + function muteStdout() { + vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + vi.spyOn(console, 'error').mockImplementation(() => {}); + } + + async function runLane(n: number): Promise { + await Promise.all( + Array.from({ length: n }, (_, i) => + _runAgyConsultation('review this', 'be terse', dir, path.join(dir, `review-${i}.txt`)), + ), + ); + } + + it('spawns agy at most once across a 5-wide burst when unauthenticated', async () => { + setMode('unauth'); + muteStdout(); + + await runLane(5); + + // The heart of #1077: one spawn means one browser tab, not five. + expect(spawnCount()).toBe(1); + expect(checkCachedAgyAuth(fakeAgy)).toBe('unauth'); + }, 30_000); + + it('still emits a non-blocking COMMENT skip for every call it short-circuits', async () => { + setMode('unauth'); + muteStdout(); + + await runLane(5); + + // A skipped lane must never silently produce nothing — porch aggregates these. + for (let i = 0; i < 5; i++) { + const review = fs.readFileSync(path.join(dir, `review-${i}.txt`), 'utf-8'); + expect(review).toContain('VERDICT: COMMENT'); + expect(review).toMatch(/Gemini lane skipped/); + } + }, 30_000); + + it('does not spawn at all once the unauth verdict is cached', async () => { + setMode('unauth'); + muteStdout(); + recordAgyAuthState('unauth', fakeAgy); + + await runLane(3); + + expect(spawnCount()).toBe(0); + }, 30_000); + + it('re-probes after the unauth TTL lapses, recovering once the user signs in', async () => { + process.env.CODEV_AGY_AUTH_CACHE_TTL_UNAUTH_MS = '50'; + setMode('unauth'); + muteStdout(); + + await runLane(1); + expect(checkCachedAgyAuth(fakeAgy)).toBe('unauth'); + + // User completes OAuth in another window; the stale verdict times out. + setMode('auth'); + await new Promise((r) => setTimeout(r, 80)); + + await runLane(1); + expect(checkCachedAgyAuth(fakeAgy)).toBe('auth'); + const review = fs.readFileSync(path.join(dir, 'review-0.txt'), 'utf-8'); + expect(review).toContain('VERDICT: APPROVE'); + }, 30_000); + + it('runs every lane in a burst when authenticated (no reviewer dropped)', async () => { + setMode('auth'); + muteStdout(); + + await runLane(5); + + // Each call is real work and must reach agy; the cache must not suppress any. + expect(spawnCount()).toBe(5); + for (let i = 0; i < 5; i++) { + expect(fs.readFileSync(path.join(dir, `review-${i}.txt`), 'utf-8')).toContain('VERDICT: APPROVE'); + } + }, 30_000); + + it('keeps spawning per call when the cache is disabled (pre-#1077 behaviour)', async () => { + process.env.CODEV_AGY_AUTH_CACHE_DISABLE = '1'; + setMode('unauth'); + muteStdout(); + + await runLane(3); + + // The escape hatch genuinely restores the old always-spawn path — this is + // also the assertion that proves the burst test above measures the cache. + expect(spawnCount()).toBe(3); + }, 30_000); +}); From 094250f7e95a38d2b7eeb6218faf2f8cd1a53ea5 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 25 Jul 2026 06:08:40 -0700 Subject: [PATCH 04/11] [Bugfix #1077] Docs: document the agy auth-state cache consult.md gets the mechanism, the recovery story (sign in elsewhere and the lane comes back on its own), and the TTL / opt-out env vars. CLAUDE.md and AGENTS.md get the one-paragraph version in the gemini lane bullet. Mirrored into codev-skeleton/ so adopters get it too; CLAUDE.md and AGENTS.md kept byte-identical. --- AGENTS.md | 6 ++++- CLAUDE.md | 6 ++++- codev-skeleton/resources/commands/consult.md | 26 ++++++++++++++++++++ codev/resources/commands/consult.md | 26 ++++++++++++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d70bab1da..a483432ac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -339,7 +339,11 @@ Use sequential numbering with descriptive names (no leading zeros): **DEFAULT BEHAVIOR**: Consultation is ENABLED by default with: - **Gemini** via the **Antigravity CLI (`agy`)** for deep analysis (the retired Gemini CLI's replacement; OAuth/subscription, agy's default model — no pinned model id). Skips non-blockingly - if `agy` is missing/unauthenticated. + if `agy` is missing/unauthenticated. An unauthenticated `agy` is spawned **at most once per TTL + window** rather than once per consult: the verdict is cached across processes in + `~/.cache/codev/agy-auth.json`, because each spawn opens an OAuth browser tab before Codev can + detect the missing login (#1077). Sign in with `agy` in any terminal and the lane recovers on its + own; see `codev/resources/commands/consult.md` for the TTL/opt-out env vars. - **GPT-5.4 Codex** (gpt-5.4-codex) for coding and architecture perspective To disable: User must explicitly say "without multi-agent consultation" diff --git a/CLAUDE.md b/CLAUDE.md index d70bab1da..a483432ac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -339,7 +339,11 @@ Use sequential numbering with descriptive names (no leading zeros): **DEFAULT BEHAVIOR**: Consultation is ENABLED by default with: - **Gemini** via the **Antigravity CLI (`agy`)** for deep analysis (the retired Gemini CLI's replacement; OAuth/subscription, agy's default model — no pinned model id). Skips non-blockingly - if `agy` is missing/unauthenticated. + if `agy` is missing/unauthenticated. An unauthenticated `agy` is spawned **at most once per TTL + window** rather than once per consult: the verdict is cached across processes in + `~/.cache/codev/agy-auth.json`, because each spawn opens an OAuth browser tab before Codev can + detect the missing login (#1077). Sign in with `agy` in any terminal and the lane recovers on its + own; see `codev/resources/commands/consult.md` for the TTL/opt-out env vars. - **GPT-5.4 Codex** (gpt-5.4-codex) for coding and architecture perspective To disable: User must explicitly say "without multi-agent consultation" diff --git a/codev-skeleton/resources/commands/consult.md b/codev-skeleton/resources/commands/consult.md index dd2260127..72a06cd1c 100644 --- a/codev-skeleton/resources/commands/consult.md +++ b/codev-skeleton/resources/commands/consult.md @@ -170,6 +170,32 @@ Configure auth: - Gemini (`agy`): **OAuth / subscription** — run `agy` once and sign in (no API key). If `agy` is missing or unauthenticated, the gemini lane skips non-blockingly (the run proceeds without it). +#### agy auth-state cache (unauthenticated tab burst) + +An unauthenticated `agy` opens an OAuth browser tab *before* it prints the URL +Codev detects, so the lane cannot suppress the tab after spawning — only by not +spawning. Since a CMAP round is several independent `consult` processes, this used +to strand one tab per consult (#1077). + +Codev keeps a shared auth verdict in `~/.cache/codev/agy-auth.json` (0600, +honours `XDG_CACHE_HOME`). The first call probes; the rest read the verdict and +short-circuit, so **an unauthenticated agy is spawned at most once per TTL +window** instead of once per consult. A file lock keeps parallel consults from +probing simultaneously. `codev doctor`'s agy check shares the same cache. + +Verdicts expire on their own — sign in with `agy` in another terminal and the lane +recovers within the unauth TTL, no cache clearing required. The cache only ever +suppresses a lane on positive "signed out" evidence; anything ambiguous falls +through to a normal spawn. + +| Variable | Default | Purpose | +|---|---|---| +| `CODEV_AGY_AUTH_CACHE_TTL_UNAUTH_MS` | `300000` (5 min) | How long a "signed out" verdict is trusted — the sign-in recovery window. | +| `CODEV_AGY_AUTH_CACHE_TTL_AUTH_MS` | `1800000` (30 min) | How long a "signed in" verdict is trusted. | +| `CODEV_AGY_AUTH_CACHE_WAIT_MS` | `6000` | How long a consult waits for another process's in-flight probe before proceeding anyway. | +| `CODEV_AGY_AUTH_CACHE_DIR` | `~/.cache/codev` | Cache location (mainly for tests). | +| `CODEV_AGY_AUTH_CACHE_DISABLE` | unset | `1` restores the always-spawn behaviour. | + ### Claude auth: subscription vs. metered API `consult -m claude` runs on the Claude Agent SDK. When `CLAUDE_CODE_OAUTH_TOKEN` diff --git a/codev/resources/commands/consult.md b/codev/resources/commands/consult.md index dd2260127..72a06cd1c 100644 --- a/codev/resources/commands/consult.md +++ b/codev/resources/commands/consult.md @@ -170,6 +170,32 @@ Configure auth: - Gemini (`agy`): **OAuth / subscription** — run `agy` once and sign in (no API key). If `agy` is missing or unauthenticated, the gemini lane skips non-blockingly (the run proceeds without it). +#### agy auth-state cache (unauthenticated tab burst) + +An unauthenticated `agy` opens an OAuth browser tab *before* it prints the URL +Codev detects, so the lane cannot suppress the tab after spawning — only by not +spawning. Since a CMAP round is several independent `consult` processes, this used +to strand one tab per consult (#1077). + +Codev keeps a shared auth verdict in `~/.cache/codev/agy-auth.json` (0600, +honours `XDG_CACHE_HOME`). The first call probes; the rest read the verdict and +short-circuit, so **an unauthenticated agy is spawned at most once per TTL +window** instead of once per consult. A file lock keeps parallel consults from +probing simultaneously. `codev doctor`'s agy check shares the same cache. + +Verdicts expire on their own — sign in with `agy` in another terminal and the lane +recovers within the unauth TTL, no cache clearing required. The cache only ever +suppresses a lane on positive "signed out" evidence; anything ambiguous falls +through to a normal spawn. + +| Variable | Default | Purpose | +|---|---|---| +| `CODEV_AGY_AUTH_CACHE_TTL_UNAUTH_MS` | `300000` (5 min) | How long a "signed out" verdict is trusted — the sign-in recovery window. | +| `CODEV_AGY_AUTH_CACHE_TTL_AUTH_MS` | `1800000` (30 min) | How long a "signed in" verdict is trusted. | +| `CODEV_AGY_AUTH_CACHE_WAIT_MS` | `6000` | How long a consult waits for another process's in-flight probe before proceeding anyway. | +| `CODEV_AGY_AUTH_CACHE_DIR` | `~/.cache/codev` | Cache location (mainly for tests). | +| `CODEV_AGY_AUTH_CACHE_DISABLE` | unset | `1` restores the always-spawn behaviour. | + ### Claude auth: subscription vs. metered API `consult -m claude` runs on the Claude Agent SDK. When `CLAUDE_CODE_OAUTH_TOKEN` From 5188f6f260f42851f0bbcf2994a53c9a5f276b6a Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 25 Jul 2026 06:09:49 -0700 Subject: [PATCH 05/11] [Bugfix #1077] Thread: builder narrative log --- codev/state/bugfix-1077_thread.md | 100 ++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 codev/state/bugfix-1077_thread.md diff --git a/codev/state/bugfix-1077_thread.md b/codev/state/bugfix-1077_thread.md new file mode 100644 index 000000000..19d5ab17a --- /dev/null +++ b/codev/state/bugfix-1077_thread.md @@ -0,0 +1,100 @@ +# bugfix-1077 — agy auth-state pre-flight cache + +Issue #1077: consult's gemini lane spawns `agy` before it can know agy is +unauthenticated. Each spawn opens an OAuth browser tab (upstream agy behavior), +so an N-wide CMAP burst strands N tabs. The existing `AGY_OAUTH_MARKERS` kill +fires *after* the tab is already open. + +## Investigate + +- Confirmed the code path: `packages/codev/src/commands/consult/index.ts` + `runAgyConsultation` → `spawn(bin, args)` unconditionally; marker detection is + in the stdout/stderr `watch()` handler, i.e. post-spawn (index.ts:834-873). +- Confirmed the amplifiers named in the issue: + - CMAP-N dispatch emits one `consult -m ` process per model + (`porch/next.ts:529-531`), gemini lane = 1 agy spawn per round. + - Partial-review re-emit (`porch/next.ts:544+`) re-issues the same consult + commands, so a malformed/racy gemini skip causes another agy spawn. +- `doctor.ts:472 verifyAgy()` probes agy on every `codev doctor` run — same + tab-opening cost, separate entry point. +- Fix is cross-process, not in-process: each consult is its own OS process + (`bin/consult.js`), so a module-level memo would not help. Needs a + filesystem-backed cache + lock, as the issue proposes. +- agy credential path: not discoverable on this machine. `~/.gemini/antigravity-cli/` + holds settings/state but no credentials/token file; nothing in the login + keychain under "Antigravity". So mtime-based invalidation is best-effort over + a candidate list, and the TTL is the real recovery window (the issue + anticipates exactly this fallback). +- Scope: 1 new module + 2 integration points + tests. Well within BUGFIX. + +### Reproduced + +Fake unauthenticated agy (`CODEV_AGY_BIN` → script that logs each invocation, +then prints an `accounts.google.com/o/oauth2` banner on stderr like real agy), +5 parallel `consult -m gemini --prompt`: + +``` +=== AGY SPAWNS (== browser tabs): 5 +``` + +All 5 correctly emitted the non-blocking COMMENT skip — *after* spawning. That +is the bug precisely: the semantics are right, the timing is too late. + +### Detour: main's build was broken + +`doctor.ts` imported `formatBytes` from two modules (TS2300), landed via PR #1243. +I fixed it locally to unblock, then the architect asked me to drop it — they are +shipping it as a separate fast PR. Reverted; will merge main once that lands. +My branch stays scoped to the auth cache. + +### Design decision: the real run IS the probe + +The issue proposes a dedicated `--print-timeout 5s` probe when the cache is cold. +Implemented instead: the lock holder's *real* consult run doubles as the probe, +publishing the auth verdict as soon as it is knowable. Rationale: + +- No extra agy spawn per TTL window in the authenticated (common) case — a + dedicated probe would mean probe-spawn + real-spawn. +- Reuses the already-tested `AGY_OAUTH_MARKERS` detection instead of duplicating + marker logic in a second code path that could classify differently. +- Same observable guarantee the ACs ask for: at most 1 agy spawn per TTL window + when unauthenticated. + +Waiters poll the cache (they do NOT block on the lock, which the holder may hold +for minutes) and **fail open** — an undecided wait proceeds with the spawn. The +bias is deliberate: only positive marker evidence records `unauth`, so the +failure mode is "status-quo tab" and never "silently skip a working lane". + +`doctor.verifyAgy` becomes both consumer and producer of the same cache. + +## Fix + +Three commits: fix, tests, docs (+ a merge of main for the hotfix). + +Verified with a fake agy (`CODEV_AGY_BIN`) that logs every invocation, so spawn +counts are observed rather than mocked. Real 5-process bursts: + +| Scenario | Spawns | Wanted | +|---|---|---| +| unauthenticated, 5 parallel `consult -m gemini` | **1** | 1 | +| authenticated, 5 parallel | **5** | 5 (every lane is real work) | +| unauth verdict already cached | **0** | 0 | +| cache disabled, 3 calls | **3** | 3 (pre-fix behaviour) | + +Sign-in recovery also verified end-to-end: unauth cached → flip the fake to +authenticated → wait out the TTL → next call re-probes, finds `auth`, delivers a +review. No manual cache clear. + +### Test-pollution trap worth remembering + +The cache lives at a **user-global** path, so the first full-suite run wrote a +verdict into my own `~/.cache/codev` — and worse, a verdict recorded by one +doctor case made a sibling case stop spawning the thing it asserted on (one real +test failure, `doctor.test.ts` "passes the probe text immediately after --print"). + +Two-part fix: the consult/doctor suites pin `CODEV_AGY_AUTH_CACHE_DIR` into their +temp dirs, and the module itself is inert under `VITEST` unless a cache dir is +explicitly set. The guard is what makes this safe for *future* tests — otherwise +every new test that reaches `doctor()` silently re-acquires the hazard. + +Full suite: 3720 passed, 0 failed, and `~/.cache/codev` is not created by a test run. From 1320369c8ec55b81d157bf1fc81bcddf8cbed72e Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 25 Jul 2026 06:10:27 -0700 Subject: [PATCH 06/11] chore(porch): bugfix-1077 fix phase-transition --- .../bugfix-1077-consult-pre-flight-auth-state-/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml b/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml index 7afc919c6..17a43b437 100644 --- a/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml +++ b/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml @@ -1,7 +1,7 @@ id: bugfix-1077 title: consult-pre-flight-auth-state- protocol: bugfix -phase: investigate +phase: fix plan_phases: [] current_plan_phase: null gates: @@ -11,4 +11,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-07-25T12:49:13.351Z' -updated_at: '2026-07-25T12:49:13.351Z' +updated_at: '2026-07-25T13:10:27.468Z' From 587120588c14d90d84caac0be83cefd0c1821cdd Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 25 Jul 2026 06:11:31 -0700 Subject: [PATCH 07/11] chore(porch): bugfix-1077 pr phase-transition --- .../bugfix-1077-consult-pre-flight-auth-state-/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml b/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml index 17a43b437..fbbd9dbf0 100644 --- a/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml +++ b/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml @@ -1,7 +1,7 @@ id: bugfix-1077 title: consult-pre-flight-auth-state- protocol: bugfix -phase: fix +phase: pr plan_phases: [] current_plan_phase: null gates: @@ -11,4 +11,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-07-25T12:49:13.351Z' -updated_at: '2026-07-25T13:10:27.468Z' +updated_at: '2026-07-25T13:11:31.792Z' From efec369b9b6233bd9d4b576548bd78dc7e729768 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 25 Jul 2026 06:15:16 -0700 Subject: [PATCH 08/11] chore(porch): bugfix-1077 pr gate-requested --- .../bugfix-1077-consult-pre-flight-auth-state-/status.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml b/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml index fbbd9dbf0..fc11e205a 100644 --- a/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml +++ b/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml @@ -7,8 +7,10 @@ current_plan_phase: null gates: pr: status: pending + requested_at: '2026-07-25T13:15:16.670Z' iteration: 1 build_complete: false history: [] started_at: '2026-07-25T12:49:13.351Z' -updated_at: '2026-07-25T13:11:31.792Z' +updated_at: '2026-07-25T13:15:16.670Z' +pr_ready_for_human: true From 42cf18e894d08c76868669bf3d9c361986b9c8c4 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 25 Jul 2026 06:19:44 -0700 Subject: [PATCH 09/11] [Bugfix #1077] Fix: let positive OAuth evidence overwrite the auth guess MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lane guesses `auth` after a marker-free grace period so waiters are not stuck behind a review that runs for minutes. That guess went through the once-only publish, so if agy merely took longer than the grace period to print its banner, the guess stood and the real `unauth` evidence a moment later was a no-op — caching `auth` for the full 30-minute TTL and re-opening a tab on every consult in that window. The fix silently reverted to the status quo on exactly the slow machines that need it most. Record `unauth` directly from the marker handler so positive evidence wins regardless of arrival order. This also fixes a second case: a caller that failed open (another process held the lock) still spawns and can be the one to see the banner, but published nothing because publish is inert for non-probers. Its evidence now corrects a misfired guess on the very next spawn instead of waiting out the TTL. Both new tests were confirmed to fail against the previous code. Raised by the architect in integration review of PR #1250. --- .../consult/__tests__/agy-auth-cache.test.ts | 40 ++++++++++++++++++- packages/codev/src/commands/consult/index.ts | 16 ++++++-- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/packages/codev/src/commands/consult/__tests__/agy-auth-cache.test.ts b/packages/codev/src/commands/consult/__tests__/agy-auth-cache.test.ts index 82a2fab5f..196725b3e 100644 --- a/packages/codev/src/commands/consult/__tests__/agy-auth-cache.test.ts +++ b/packages/codev/src/commands/consult/__tests__/agy-auth-cache.test.ts @@ -41,6 +41,7 @@ const ENV_KEYS = [ 'CODEV_AGY_AUTH_CACHE_WAIT_MS', 'FAKE_AGY_LOG', 'FAKE_AGY_MODE', + 'FAKE_AGY_BANNER_DELAY_MS', ] as const; let dir: string; @@ -60,8 +61,13 @@ const FAKE_AGY_SOURCE = `#!/usr/bin/env node const fs = require('node:fs'); fs.appendFileSync(process.env.FAKE_AGY_LOG, process.pid + '\\n'); if (process.argv[2] === '--version') { console.log('1.0.10-fake'); process.exit(0); } -if (process.env.FAKE_AGY_MODE === 'unauth') { - process.stderr.write('Please visit https://accounts.google.com/o/oauth2/auth?client_id=fake\\n'); +if (process.env.FAKE_AGY_MODE === 'unauth' || process.env.FAKE_AGY_MODE === 'unauth-slow') { + // 'unauth-slow' emits the banner *after* the lane's marker-free grace period, + // the case that used to leave a false 'auth' verdict cached. + const delay = process.env.FAKE_AGY_MODE === 'unauth-slow' ? Number(process.env.FAKE_AGY_BANNER_DELAY_MS) : 0; + setTimeout(() => { + process.stderr.write('Please visit https://accounts.google.com/o/oauth2/auth?client_id=fake\\n'); + }, delay); setTimeout(() => process.exit(1), 30000); // hang like real agy; the lane kills us } else { process.stdout.write('---\\nVERDICT: APPROVE\\nSUMMARY: ok\\nCONFIDENCE: HIGH\\n---\\n'); @@ -341,6 +347,36 @@ describe('gemini lane burst behaviour (#1077 regression)', () => { } }, 30_000); + it('lets a late OAuth banner overwrite the speculative auth verdict', async () => { + // The lane guesses `auth` after a marker-free grace period so waiters are not + // stuck behind a long review. If agy is merely slow to print its banner, that + // guess is wrong — and left standing it would cache `auth` for the full auth + // TTL, re-opening a tab on every consult in that window. Positive marker + // evidence must win regardless of arrival order. + setMode('unauth-slow'); + process.env.FAKE_AGY_BANNER_DELAY_MS = '3400'; // > the lane's 3s grace + muteStdout(); + + await runLane(1); + + expect(checkCachedAgyAuth(fakeAgy)).toBe('unauth'); + }, 30_000); + + it('records the verdict even when the run that sees the banner is not the prober', async () => { + // A caller that fails open (another process holds the lock) still spawns, so + // it can be the one to observe the banner. Dropping that evidence would leave + // a misfired guess uncorrected until the TTL lapsed. + fs.mkdirSync(agyAuthCacheDir(), { recursive: true }); + fs.writeFileSync(path.join(agyAuthCacheDir(), 'agy-auth.lock'), '99999 0\n'); + process.env.CODEV_AGY_AUTH_CACHE_WAIT_MS = '200'; // fail open promptly + setMode('unauth'); + muteStdout(); + + await runLane(1); + + expect(checkCachedAgyAuth(fakeAgy)).toBe('unauth'); + }, 30_000); + it('keeps spawning per call when the cache is disabled (pre-#1077 behaviour)', async () => { process.env.CODEV_AGY_AUTH_CACHE_DISABLE = '1'; setMode('unauth'); diff --git a/packages/codev/src/commands/consult/index.ts b/packages/codev/src/commands/consult/index.ts index d0a3fbc67..ca6fc7eb8 100644 --- a/packages/codev/src/commands/consult/index.ts +++ b/packages/codev/src/commands/consult/index.ts @@ -21,7 +21,7 @@ import { getResolver, GitRefResolver, type ArtifactResolver } from '../porch/art import { MetricsDB } from './metrics.js'; import { extractUsage, extractReviewText, type SDKResultLike, type UsageData } from './usage-extractor.js'; import { executeForgeCommandSync } from '../../lib/forge.js'; -import { preflightAgyAuth, type AgyAuthState } from './agy-auth-cache.js'; +import { preflightAgyAuth, recordAgyAuthState, type AgyAuthState } from './agy-auth-cache.js'; // Content reference — resolved artifact content with a display label interface ContentRef { @@ -905,8 +905,18 @@ async function runAgyConsultation( if (scanBuf.length < AGY_MARKER_SCAN_LIMIT) { scanBuf += buf.toString('utf-8'); if (AGY_OAUTH_MARKERS.some((m) => scanBuf.includes(m))) { - // The one place we have positive proof of being signed out. Publish it - // so the rest of the burst skips without spawning (and without tabs). + // The one place we have positive proof of being signed out, so it is + // written unconditionally rather than through the once-only publish: + // - If our own grace timer already guessed `auth` (agy took longer + // than AGY_AUTH_GRACE_MS to emit its banner), that guess would + // otherwise stand for the full auth TTL and every burst in that + // window would spawn tabs again. + // - A non-prober that failed open and spawned holds real evidence + // too; discarding it would leave a misfired guess uncorrected + // until the TTL lapsed, instead of on the very next spawn. + // recordAgyAuthState carries its own disabled-guard, so this is safe + // on every path. + recordAgyAuthState('unauth', bin); publishAuth('unauth'); settleSkip('agy not authenticated — run `agy` once to sign in (OAuth)', 1); } From 517bfcb9caaabee2c28b879743ee841d72b88d21 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 25 Jul 2026 06:20:10 -0700 Subject: [PATCH 10/11] [Bugfix #1077] Thread: record the architect's grace-timer finding --- codev/state/bugfix-1077_thread.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/codev/state/bugfix-1077_thread.md b/codev/state/bugfix-1077_thread.md index 19d5ab17a..1b0d2f707 100644 --- a/codev/state/bugfix-1077_thread.md +++ b/codev/state/bugfix-1077_thread.md @@ -98,3 +98,30 @@ explicitly set. The guard is what makes this safe for *future* tests — otherwi every new test that reaches `doctor()` silently re-acquires the hazard. Full suite: 3720 passed, 0 failed, and `~/.cache/codev` is not created by a test run. + +## Architect integration review — one real defect + +Both architect lanes APPROVE, and both my judgment calls (probe deviation, scope) +were accepted. One pre-merge change requested, and it was a genuine bug that +inverted the fail-safe bias I had *claimed* in the PR body: + +The lane guesses `auth` after a marker-free grace period (3s) so waiters are not +stuck behind a long review. That guess went through the once-only `publish`, so a +banner arriving later than the grace period left the guess standing — caching +`auth` for the full 30-min TTL and re-opening a tab on every consult in that +window. The fix silently reverted to the status quo on exactly the slow machines +that need it most. + +Investigating it surfaced a **second** case the architect had not named and I had +also missed: `publish` is inert for non-probers, so a caller that failed open, +spawned, and saw the banner wrote *nothing at all*. Even with ordering fixed, a +misfired guess would sit uncorrected until the TTL lapsed instead of being +corrected by the next spawn. + +Both fixed by recording `unauth` directly from the marker handler (42cf18e8). +Both new tests confirmed to FAIL against the previous code before restoring the +fix — a test that only passes with the fix proves much less. Suite now 3722. + +Lesson worth carrying: I wrote "only positive marker evidence records unauth" as a +design invariant, but never tested the *ordering* that invariant depends on. A +stated invariant with no test pinning it is a comment, not a property. From c3a9728f66797b3fdcf093115b8a0663c2236ae7 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 25 Jul 2026 06:23:32 -0700 Subject: [PATCH 11/11] chore(porch): bugfix-1077 pr gate-approved --- .../bugfix-1077-consult-pre-flight-auth-state-/status.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml b/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml index fc11e205a..585907425 100644 --- a/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml +++ b/codev/projects/bugfix-1077-consult-pre-flight-auth-state-/status.yaml @@ -6,11 +6,12 @@ plan_phases: [] current_plan_phase: null gates: pr: - status: pending + status: approved requested_at: '2026-07-25T13:15:16.670Z' + approved_at: '2026-07-25T13:23:32.434Z' iteration: 1 build_complete: false history: [] started_at: '2026-07-25T12:49:13.351Z' -updated_at: '2026-07-25T13:15:16.670Z' -pr_ready_for_human: true +updated_at: '2026-07-25T13:23:32.435Z' +pr_ready_for_human: false