[Spec 1338] retire-gemini-cli-as-a-builder

README.md

@@ -389,7 +389,7 @@ OpenCode supports 75+ LLM providers (OpenAI, Anthropic, Google, Ollama, etc.). W
 - OpenCode reads `AGENTS.md` for project instructions (already present in Codev projects)
 - OpenCode is only supported as a **builder** shell, not as an architect shell
 
-Other shells (Codex, Gemini) are also supported via the harness system. See `packages/codev/src/agent-farm/utils/harness.ts` for details, or define a custom harness in `.codev/config.json`.
+Codex is also supported via the harness system. See `packages/codev/src/agent-farm/utils/harness.ts` for details, or define a custom harness in `.codev/config.json`. (The built-in **Gemini CLI** harness is **retired** — see the note under Autonomous Builder Flags below.)
 
 ## Architect-Builder Pattern
 
@@ -433,7 +433,6 @@ Builders need permission-skipping flags to run autonomously without human approv
 | CLI Tool | Flag | Purpose |
 |----------|------|---------|
 | Claude Code | `--dangerously-skip-permissions` | Skip permission prompts for file/command operations |
-| Gemini CLI | `--yolo` | Enable autonomous mode without confirmations |
 
 Configure in `.codev/config.json` (created by `codev init` or `codev adopt`):
 ```json
@@ -445,19 +444,45 @@ Configure in `.codev/config.json` (created by `codev init` or `codev adopt`):
 }
 ```
 
-Or for Gemini (the standalone **Gemini CLI** as a *builder/architect* coding agent — a separate
-concern from the `gemini` **consult lane**, which now uses the Antigravity CLI `agy`). Note: Google
-retired the Gemini CLI for Pro/Ultra/free tiers on 2026-06-18, so this builder harness will stop
-working for those tiers — prefer a Claude or Codex builder, or an enterprise Gemini CLI. (Tracked as
-a follow-up; out of scope for the consult-lane migration.)
+The built-in **Gemini CLI** harness is **retired** as a builder/architect shell: Google ended
+consumer Gemini CLI access (Pro, Ultra, and free tiers) on 2026-06-18, so it is no longer offered as
+a supported built-in shell. Switch to a supported harness — `claude` or `codex` for either role, or
+`opencode` for builders (OpenCode is not supported as an architect shell). For example, Codex for
+both roles:
 ```json
 {
   "shell": {
-    "architect": "gemini --yolo",
-    "builder": "gemini --yolo"
+    "architect": "codex",
+    "builder": "codex"
   }
 }
 ```
+If you retain Gemini CLI access (a Standard/Enterprise subscription or API-key auth), you can still
+run it by defining a **custom harness** named `gemini` in `.codev/config.json` **and selecting it
+explicitly** with `shell.builderHarness` / `shell.architectHarness`. The explicit selector is
+**required**: a bare auto-detected `gemini` command (e.g. `shell.builder: "gemini --yolo"` with no
+`builderHarness`) stays retired, because auto-detection resolves the built-in namespace only.
+```json
+{
+  "shell": {
+    "builder": "gemini --yolo",
+    "builderHarness": "gemini"
+  },
+  "harness": {
+    "gemini": {
+      "roleArgs": [],
+      "roleEnv": { "GEMINI_SYSTEM_MD": "${ROLE_FILE}" },
+      "roleScriptFragment": "",
+      "roleScriptEnv": { "GEMINI_SYSTEM_MD": "${ROLE_FILE}" }
+    }
+  }
+}
+```
+The Gemini CLI reads its system prompt from the `GEMINI_SYSTEM_MD` environment variable pointing at
+the role file, so the custom harness injects via `roleEnv` / `roleScriptEnv` (with empty `roleArgs`) —
+reproducing exactly what the retired built-in harness did.
+See `packages/codev/src/agent-farm/utils/harness.ts` for the custom-harness fields. This is separate
+from the `gemini` **consult lane**, which is unaffected and now uses the Antigravity CLI `agy`.
 
 **Warning**: These flags allow the AI to execute commands and modify files without asking. Only use in development environments where you trust the AI's actions.
 

codev/projects/1338-retire-gemini-cli-as-a-builder/1338-phase_2-iter1-rebuttals.md

@@ -0,0 +1,96 @@
+# Phase 2 (1338) — Rebuttals, iteration 1
+
+Reviews: **Gemini APPROVE**, **Codex REQUEST_CHANGES (HIGH)**, **Claude APPROVE (HIGH)**.
+Disposition: **both Codex points accepted and fixed in commit `11527838`.** No point rejected.
+Codex found two *reachable* fail-closed gaps on the restart/reconnect paths that the initial
+"unreachable" analysis (and my own phase_2 design note) missed — both are now fixed and
+regression-tested. Claude's and Gemini's approvals stand; Claude's non-blocking observations are
+dispositioned below. Each claim was re-verified against source before revising.
+
+---
+
+## Codex (REQUEST_CHANGES) — both accepted, both fixed
+
+### C1 — Fail-OPEN on reconnect: retired architect can still relaunch the Gemini binary
+**Accepted (important, real bug).** Verified: `resolveArchitectRestart` (tower-utils.ts:441)
+propagates `RetiredHarnessError`, and BOTH reconnect consumers in `tower-terminals.ts`
+(`_reconcileTerminalSessionsInner` at the startup-reconcile site, and `getTerminalsForWorkspace`
+at the on-the-fly reconnect site) wrapped that call in a `try/catch` that caught **all** harness
+errors and fell back to `{ command: cmdParts[0], args: cmdParts.slice(1) }`. For a gemini architect
+`cmdParts[0]` **is** the retired `gemini` binary, so Tower would auto-restart straight into it with
+no role injection — a direct violation of fail-closed retirement. My phase_2 design had reasoned
+these paths "unreachable" because the *initial* launch throws first; Codex correctly showed they are
+reachable when a config is edited to gemini **mid-session**, or when **Tower restarts** and reads a
+gemini architect config (no initial launch happened in this process).
+
+**Change (commit `11527838`):** Extracted the two duplicated consumer blocks into one exported
+helper, `buildArchitectReconnectRestartOptions` (tower-utils.ts:589), co-located with the
+`resolveArchitectRestart` family. It **fails closed** on `RetiredHarnessError` (tower-utils.ts:630):
+returns `undefined`, so the session reconnects to a live shellper if one exists but Tower **never
+configures an auto-restart into the retired binary**. Any *other* harness-resolution error still
+degrades to the plain configured command (identity preserved via `cleanEnv` / `CODEV_ARCHITECT_NAME`,
+Spec 786) — that transient-failure behavior is deliberately unchanged. The single `includeFreshLaunch`
+parameter preserves each call site's prior behavior exactly: startup reconcile passes `true`
+(tower-terminals.ts:684, wires the #1264 clean-exit rerun), on-the-fly reconnect passes `false`
+(tower-terminals.ts:920, no clean-exit rerun — as before). The #832 resume-bake and #1149
+crash-loop fallback are carried through unchanged.
+
+### C2 — Uncaught throw on clean-exit relaunch: Tower exception
+**Accepted (important, real bug).** Verified: `buildArchitectFreshLaunch().next()` resolved
+`getArchitectHarness` unguarded, and `SessionManager` invokes it with **no** try/catch
+(`session-manager.ts:1175`: `session.options.freshLaunch?.next() ?? null`, inside an async
+clean-exit handler). So an architect whose config flipped to gemini, then exited cleanly, would
+throw `RetiredHarnessError` into that handler — an uncaught Tower exception.
+
+**Change (commit `11527838`):** Guarded the `getArchitectHarness` call inside `next()`
+(tower-utils.ts:544). On `RetiredHarnessError` it logs a WARN and returns the **plain** original
+launch `{ args: baseArgs, env: baseEnv }` — no throw, and no re-injection of the retired harness
+(`baseArgs` come from the *original supported-harness* launch, so they can never be a gemini command).
+Any non-retirement error is a genuine fault and is rethrown unchanged.
+
+### C3 — "Regression tests for both reconnect and clean-exit relaunch paths"
+**Accepted.** Added **+7** tests to `tower-utils.test.ts`:
+- `buildArchitectFreshLaunch retirement (#1338)` (2): gemini → `next()` does **not** throw and
+  returns the plain launch; codex → resolves normally (regression).
+- `buildArchitectReconnectRestartOptions retirement (#1338)` (5): gemini → `undefined` regardless of
+  `includeFreshLaunch`; codex → real restart options with `command`/env preserved; `includeFreshLaunch`
+  toggles `freshLaunch`; a non-retirement harness error → plain-command fallback (proves the
+  fail-closed change did **not** regress the transient-error path).
+
+---
+
+## Claude (APPROVE, HIGH) — non-blocking observations dispositioned
+
+Claude approved and asked that two items be recorded in the review artifact; both are handled here
+and will be carried into the final `codev/reviews/1338-*.md`:
+
+- **(a) Preflight centralized at the `spawn()` dispatcher** rather than per-`createWorktree`-entry.
+  **Intentional deviation from the plan's letter** — one guard above handler dispatch
+  (`spawn.ts`, `mode !== 'shell'`) gives equivalent coverage of all five worktree-creating handlers
+  **and** `createWorktree`'s own internal `getBuilderHarness` (spawn-worktree.ts:912), with one call
+  site instead of five. Recorded; will appear in the review's Deviations section.
+- **(b) `session-manager.ts:1175` `freshLaunch?.next()` is unguarded by design / unreachable.**
+  This is precisely the path **Codex's C2 proved reachable** (config flipped to gemini before a clean
+  exit). So rather than accept it as an unreachable non-issue, the C2 fix now **guards it at the
+  source** (`buildArchitectFreshLaunch().next()`). Claude's non-blocking note and Codex's HIGH finding
+  converge on the same site; it is now actively fail-closed, not merely assumed unreachable.
+
+Claude's other minor notes are acknowledged, no change: **#2** (harness resolved twice on the happy
+spawn path — negligible config read) and **#3** (`afx setup` / `installHarnessWorktreeFiles` still call
+`getBuilderHarness` raw — out of Phase 2 scope; the CLI catch prints the retirement `.message` cleanly,
+so a pre-existing gemini worktree degrades to a readable message rather than a crash).
+
+---
+
+## Gemini (APPROVE)
+No changes requested. Gemini independently confirmed the builder preflight, the `buildArchitectArgs`
+fail-closed launch boundary, the `siblingRegistrationIsLive` guard, and the `tower-instances`
+`addArchitect`/`launchInstance` clean-error surfacing.
+
+---
+
+## Verification after fixes
+- `pnpm --filter @cluesmith/codev build` → exit 0 (tsc + vite clean).
+- `tower-utils.test.ts` → 61 passed (includes the +7 above).
+- Consumer suites `tower-terminals.test.ts` + `tower-instances.test.ts` + `bugfix-430-tower-restart.test.ts`
+  → 133 passed (the reconnect/restart refactor introduces no regression).

codev/projects/1338-retire-gemini-cli-as-a-builder/1338-phase_2-iter2-context.md

@@ -0,0 +1,111 @@
+### Iteration 1 Reviews
+- gemini: APPROVE
+- codex: REQUEST_CHANGES — Builder preflight is correct, but Tower restart paths can still launch Gemini or crash.
+- claude: APPROVE — Phase 2 fails closed at every spawn and architect-launch boundary with no orphaned state and no Tower-crash path; tests and typecheck are green.
+
+### Builder Response to Iteration 1
+# Phase 2 (1338) — Rebuttals, iteration 1
+
+Reviews: **Gemini APPROVE**, **Codex REQUEST_CHANGES (HIGH)**, **Claude APPROVE (HIGH)**.
+Disposition: **both Codex points accepted and fixed in commit `11527838`.** No point rejected.
+Codex found two *reachable* fail-closed gaps on the restart/reconnect paths that the initial
+"unreachable" analysis (and my own phase_2 design note) missed — both are now fixed and
+regression-tested. Claude's and Gemini's approvals stand; Claude's non-blocking observations are
+dispositioned below. Each claim was re-verified against source before revising.
+
+---
+
+## Codex (REQUEST_CHANGES) — both accepted, both fixed
+
+### C1 — Fail-OPEN on reconnect: retired architect can still relaunch the Gemini binary
+**Accepted (important, real bug).** Verified: `resolveArchitectRestart` (tower-utils.ts:441)
+propagates `RetiredHarnessError`, and BOTH reconnect consumers in `tower-terminals.ts`
+(`_reconcileTerminalSessionsInner` at the startup-reconcile site, and `getTerminalsForWorkspace`
+at the on-the-fly reconnect site) wrapped that call in a `try/catch` that caught **all** harness
+errors and fell back to `{ command: cmdParts[0], args: cmdParts.slice(1) }`. For a gemini architect
+`cmdParts[0]` **is** the retired `gemini` binary, so Tower would auto-restart straight into it with
+no role injection — a direct violation of fail-closed retirement. My phase_2 design had reasoned
+these paths "unreachable" because the *initial* launch throws first; Codex correctly showed they are
+reachable when a config is edited to gemini **mid-session**, or when **Tower restarts** and reads a
+gemini architect config (no initial launch happened in this process).
+
+**Change (commit `11527838`):** Extracted the two duplicated consumer blocks into one exported
+helper, `buildArchitectReconnectRestartOptions` (tower-utils.ts:589), co-located with the
+`resolveArchitectRestart` family. It **fails closed** on `RetiredHarnessError` (tower-utils.ts:630):
+returns `undefined`, so the session reconnects to a live shellper if one exists but Tower **never
+configures an auto-restart into the retired binary**. Any *other* harness-resolution error still
+degrades to the plain configured command (identity preserved via `cleanEnv` / `CODEV_ARCHITECT_NAME`,
+Spec 786) — that transient-failure behavior is deliberately unchanged. The single `includeFreshLaunch`
+parameter preserves each call site's prior behavior exactly: startup reconcile passes `true`
+(tower-terminals.ts:684, wires the #1264 clean-exit rerun), on-the-fly reconnect passes `false`
+(tower-terminals.ts:920, no clean-exit rerun — as before). The #832 resume-bake and #1149
+crash-loop fallback are carried through unchanged.
+
+### C2 — Uncaught throw on clean-exit relaunch: Tower exception
+**Accepted (important, real bug).** Verified: `buildArchitectFreshLaunch().next()` resolved
+`getArchitectHarness` unguarded, and `SessionManager` invokes it with **no** try/catch
+(`session-manager.ts:1175`: `session.options.freshLaunch?.next() ?? null`, inside an async
+clean-exit handler). So an architect whose config flipped to gemini, then exited cleanly, would
+throw `RetiredHarnessError` into that handler — an uncaught Tower exception.
+
+**Change (commit `11527838`):** Guarded the `getArchitectHarness` call inside `next()`
+(tower-utils.ts:544). On `RetiredHarnessError` it logs a WARN and returns the **plain** original
+launch `{ args: baseArgs, env: baseEnv }` — no throw, and no re-injection of the retired harness
+(`baseArgs` come from the *original supported-harness* launch, so they can never be a gemini command).
+Any non-retirement error is a genuine fault and is rethrown unchanged.
+
+### C3 — "Regression tests for both reconnect and clean-exit relaunch paths"
+**Accepted.** Added **+7** tests to `tower-utils.test.ts`:
+- `buildArchitectFreshLaunch retirement (#1338)` (2): gemini → `next()` does **not** throw and
+  returns the plain launch; codex → resolves normally (regression).
+- `buildArchitectReconnectRestartOptions retirement (#1338)` (5): gemini → `undefined` regardless of
+  `includeFreshLaunch`; codex → real restart options with `command`/env preserved; `includeFreshLaunch`
+  toggles `freshLaunch`; a non-retirement harness error → plain-command fallback (proves the
+  fail-closed change did **not** regress the transient-error path).
+
+---
+
+## Claude (APPROVE, HIGH) — non-blocking observations dispositioned
+
+Claude approved and asked that two items be recorded in the review artifact; both are handled here
+and will be carried into the final `codev/reviews/1338-*.md`:
+
+- **(a) Preflight centralized at the `spawn()` dispatcher** rather than per-`createWorktree`-entry.
+  **Intentional deviation from the plan's letter** — one guard above handler dispatch
+  (`spawn.ts`, `mode !== 'shell'`) gives equivalent coverage of all five worktree-creating handlers
+  **and** `createWorktree`'s own internal `getBuilderHarness` (spawn-worktree.ts:912), with one call
+  site instead of five. Recorded; will appear in the review's Deviations section.
+- **(b) `session-manager.ts:1175` `freshLaunch?.next()` is unguarded by design / unreachable.**
+  This is precisely the path **Codex's C2 proved reachable** (config flipped to gemini before a clean
+  exit). So rather than accept it as an unreachable non-issue, the C2 fix now **guards it at the
+  source** (`buildArchitectFreshLaunch().next()`). Claude's non-blocking note and Codex's HIGH finding
+  converge on the same site; it is now actively fail-closed, not merely assumed unreachable.
+
+Claude's other minor notes are acknowledged, no change: **#2** (harness resolved twice on the happy
+spawn path — negligible config read) and **#3** (`afx setup` / `installHarnessWorktreeFiles` still call
+`getBuilderHarness` raw — out of Phase 2 scope; the CLI catch prints the retirement `.message` cleanly,
+so a pre-existing gemini worktree degrades to a readable message rather than a crash).
+
+---
+
+## Gemini (APPROVE)
+No changes requested. Gemini independently confirmed the builder preflight, the `buildArchitectArgs`
+fail-closed launch boundary, the `siblingRegistrationIsLive` guard, and the `tower-instances`
+`addArchitect`/`launchInstance` clean-error surfacing.
+
+---
+
+## Verification after fixes
+- `pnpm --filter @cluesmith/codev build` → exit 0 (tsc + vite clean).
+- `tower-utils.test.ts` → 61 passed (includes the +7 above).
+- Consumer suites `tower-terminals.test.ts` + `tower-instances.test.ts` + `bugfix-430-tower-restart.test.ts`
+  → 133 passed (the reconnect/restart refactor introduces no regression).
+
+
+### IMPORTANT: Stateful Review Context
+This is NOT the first review iteration. Previous reviewers raised concerns and the builder has responded.
+Before re-raising a previous concern:
+1. Check if the builder has already addressed it in code
+2. If the builder disputes a concern with evidence, verify the claim against actual project files before insisting
+3. Do not re-raise concerns that have been explained as false positives with valid justification
+4. Check package.json and config files for version numbers before flagging missing configuration