Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 5 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
26 changes: 26 additions & 0 deletions codev-skeleton/resources/commands/consult.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
id: bugfix-1077
title: consult-pre-flight-auth-state-
protocol: bugfix
phase: pr
plan_phases: []
current_plan_phase: null
gates:
pr:
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:23:32.435Z'
pr_ready_for_human: false
26 changes: 26 additions & 0 deletions codev/resources/commands/consult.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
127 changes: 127 additions & 0 deletions codev/state/bugfix-1077_thread.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
# 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 <model>` 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.

## 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.
6 changes: 6 additions & 0 deletions packages/codev/src/__tests__/consult.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand All @@ -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() {
Expand Down
9 changes: 9 additions & 0 deletions packages/codev/src/__tests__/doctor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
Expand Down
Loading
Loading