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

[mohidmakhdoomi]

Summary

Retires the standalone Gemini CLI as a supported built-in builder/architect harness. Google ended consumer Gemini CLI access (free, Pro, Ultra) on 2026-06-18, so it can no longer be presented as supported. A gemini harness now fails closed at every spawn (including --shell) / architect launch / reconnect / clean-exit boundary with a specific retirement message (no orphaned worktree, no stray shell PTY or DB row, no Tower crash); codev doctor flags a persisted gemini config for both roles; and the docs no longer present it as a built-in. Retained-access users (Standard/Enterprise or API-key) keep a sanctioned escape hatch: an explicit custom gemini harness selected via shell.builderHarness / shell.architectHarness.

Closes #1338

Changes

• Resolver (Phase 1): a RETIRED_HARNESSES sentinel in the shared resolveHarness fails closed on both the explicit path (specific retirement, not a generic "unknown" error) and the auto-detect path (never the silent CLAUDE_HARNESS fallback). Built-in GEMINI_HARNESS removed; gemini retained in the detector so a gemini command is recognized-and-retired, not misclassified.
• Spawn/launch (Phase 2): fail-closed guards at all four boundaries — a spawn preflight that runs for every mode including --shell (before any worktree / porch / PTY / DB state is created), architect launch, reconnect (buildArchitectReconnectRestartOptions → fail closed), and clean-exit relaunch (a new FreshLaunch { stop: true } contract). No orphaned state, no Tower crash.
• doctor (Phase 3): flags a retired gemini builder/architect config with the retirement message, for both roles; preserves the custom-harness escape hatch (an explicit <role>Harness backed by a matching custom def is not flagged). The custom-harness recommendation interpolates the configured role.name rather than a hard-coded "gemini", so the advice stays correct as RETIRED_HARNESSES grows.
• Docs (Phase 4 + Review): README retirement framing with a working custom-harness snippet (GEMINI_SYSTEM_MD env injection); the runtime message, doctor recommendation, and README all name the explicit selector; governance docs (arch.md, lessons-learned.md) refreshed. The upstream release CHANGELOG.md is intentionally left untouched — contributors don't edit it.
• Diagnostics (Review): siblingRegistrationIsLive logs the retirement reason when it prunes a retired-architect registration, so the reconcile loop's generic "no resumable session" prune line can't misattribute the cause.
• Unaffected: Claude / Codex / OpenCode / custom builders; the gemini consult lane (now agy).

Testing

• Full unit suite: 4148 passing / 48 pre-existing skips / 0 failing (e2e excluded).
• spawn-retirement.test.ts drives the real spawn() against a real temp workspace and asserts 0 orphaned .builders/ + 0 porch state after a rejected gemini spawn — for both a worktree spawn and a --shell spawn (shell mode is no longer exempt from the preflight).
• Regression coverage across harness / config / doctor / tower-utils / tower-instances / session-manager; built-ins (claude/codex/opencode) regression-tested for both roles. Added: doctor asserts the interpolated custom-harness clause; tower-utils asserts the retired-prune log fires (and does not fire for a live sibling).

Spec

codev/specs/1338-retire-gemini-cli-as-a-builder.md

Review

codev/reviews/1338-retire-gemini-cli-as-a-builder.md

🤖 Generated with Claude Code

[mohidmakhdoomi]

Architect Integration Review — PR #1342

Contributor-side integration review (we are not upstream maintainers; final merge is a maintainer's call). Ran a 3-way CMAP plus an independent line-by-line pass of the production diff (resolver, config/spawn preflight, all four architect boundaries, session-manager contract, the tower-terminals consolidation, and doctor).

3-way CMAP verdicts

Reviewer	Verdict	Confidence
Gemini	APPROVE	HIGH
Codex	REQUEST_CHANGES	HIGH
Claude	APPROVE	HIGH

The design itself is sound and consistently applied: both resolver footguns are closed (no silent CLAUDE_HARNESS fallback, no undefined on the auto-detect path), the typed RetiredHarnessError keeps every catch narrowly scoped, the four architect boundaries are each handled correctly (launch propagates cleanly, the liveness predicate returns false instead of aborting the reconcile pass, the clean-exit relaunch fails closed via FreshLaunch { stop: true }, and reconnect fails closed to undefined), and the tower-terminals de-dup is a genuine maintainability win. Typecheck + full unit suite green (4145 pass / 0 fail) independently reproduced.

Blocking (must address before merge)

1. afx spawn --shell bypasses the retirement (Codex — verified).
spawn() exempts shell mode from the preflight (spawn.ts:933, mode !== 'shell'), but spawnShell() runs commands.builder directly via startShellSession() → bash -c <builder cmd> (spawn.ts:643, spawn-worktree.ts:990) and upsertBuilder(...). So a shell.builder: "gemini …" config still launches the retired Gemini via afx spawn --shell and persists a shell row — contradicting AC1 ("no longer treated as a supported builder option") and this PR's own "fails closed at every spawn boundary" summary. The exemption's comment ("shell spawns a plain terminal with no builder harness") is also factually wrong — shell mode does run the builder command.
Fix: extend the retirement preflight to shell mode (it delegates to getBuilderHarness, so the custom-gemini escape hatch is preserved), correct the comment, and add a test asserting no PTY / no DB row is created for a rejected gemini --shell spawn.

2. Revert the CHANGELOG.md edit (contributor-scope).
As contributors rather than upstream maintainers, we shouldn't be editing the release changelog — the [Unreleased]/version history is the maintainers' to manage. Dropping the ### Removed (Spec 1338…) block; the retirement is already fully described in the PR summary, the spec, and the review doc. Happy to re-add on a maintainer's request.

Non-blocking (cheap to fold in while revising)

• Doctor recommendation: hard-codes "gemini" (doctor.ts:851/:875) inside a branch otherwise generic over RETIRED_HARNESSES; the console text and issue: already interpolate ${role.name}. Two-token fix keeps the next retirement correct.
• siblingRegistrationIsLive retirement path logs nothing, so the subsequent prune line misattributes the cause as "no live terminal and no resumable session." A one-line log() closes the diagnosability gap.

Follow-ups (not this PR)

Doctor doesn't flag an unknown explicit harness name; .claude/skills/codev/SKILL.md (both trees) still lists "Gemini" as a checked CLI (stale since the #778 agy migration); afx reset on a pre-existing gemini builder now says "cannot determine harness" (accepted in the spec's Assumptions).

---

Merge is held pending the two blocking items above, human validation, and an upstream maintainer's approval. Not self-merging.

Architect integration review

[mohidmakhdoomi]

Architect Integration Review — Update: all items resolved ✅

Follow-up to my integration review above. Every blocking and recommended item is addressed, and I re-verified each against the diff (not just the builder's summary).

Blocking — resolved

#	Item	Resolution	Verified
1	afx spawn --shell bypassed the retirement	Preflight made unconditional (spawn.ts; the mode !== 'shell' exemption + its wrong "no builder harness" comment removed) + a regression test asserting a rejected gemini --shell creates no PTY and no DB row (507c4074)	git diff shows preflight at spawn.ts:934 with no mode guard; new test in spawn-retirement.test.ts
2	Revert CHANGELOG.md (contributor scope)	### Removed (Spec 1338…) block dropped (5da41839); stale CHANGELOG claims scrubbed from the PR body + review doc	git diff main...HEAD -- CHANGELOG.md is empty

Recommended — folded in

• doctor recommendation: no longer hard-codes "gemini" — interpolates ${role.name}, so the branch stays correct for any future RETIRED_HARNESSES entry (b97d96a6).
• siblingRegistrationIsLive retirement path now logs its reason — the prune line no longer misattributes the cause (a2b8b90a).

Re-review on the fixed code (fresh 3-way)

Reviewer	Verdict
Gemini	APPROVE
Claude	APPROVE / HIGH — independently reproduced tsc --noEmit clean + full suite 4148/0, and confirmed all four fixes in code
Codex	REQUEST_CHANGES — audit-record only ("No production correctness or security blockers found"): commit the 5 consultation *-context.md artifacts, and refresh stale review-doc metrics. Both resolved (f29c1cc5, c36f2df3).

State

• Full unit suite 4148 pass / 48 skip / 0 fail; tsc clean; working tree clean.
• Both resolver footguns closed; all four architect boundaries handled; spawn preflight (now incl. --shell) prevents orphaned state; doctor flags both roles with the custom-harness escape hatch preserved; the agy / consult -m gemini lane is untouched.

From the contributing architect's side this is ready for maintainer review and merge. We are not upstream maintainers, so we are not self-merging — the GitHub merge is a cluesmith/codev maintainer's call.

Architect integration review (update)

[waleedkadous]

Architect Integration Review

Excellent work — this is a model retirement: fail-closed at every boundary with zero orphaned state, a principled escape hatch (explicit custom name wins; auto-detect never consults customs), and the FreshLaunch { stop: true } contract is exactly the right answer to "next() can't change the command". I read the resolver, spawn preflight, addArchitect, and session-manager seams line-by-line and they all hold. Ran a 2-way CMAP (codex + claude; the agy lane is down for structured reviews): claude APPROVE, codex REQUEST_CHANGES with one finding I verified against the source.

I'd like to see one change before merge:

• Reconnect leaves the PTY's restart flag inconsistent with SessionManager. buildArchitectReconnectRestartOptions() correctly returns undefined for a retired harness, but both reconnect paths in tower-terminals.ts (ptySession.restartOnExit = true at ~808 and ~1019, pre-existing code) force the flag for every architect unconditionally. When a retired-harness architect's process exits, the pane reports it's restarting and holds clients through the restart wait, though no restart can occur. Suggested: set the PTY flag only when restartOptions actually exists, and add a test at the consumer path (not just the helper's undefined return).

Optional, take or file as follow-ups (from the claude lane — all non-blocking):

• siblingRegistrationIsLive returning false makes the reconcile loop delete the sibling row; returning true routes to addArchitect, which now fails closed anyway — same safety, but the user's config row survives for when they fix their harness. One-liner if you agree.
• console.debug in agent-farm/utils/config.ts → logger, since Tower imports that module.
• Pre-existing: BUILTIN_HARNESSES[name] could take the same hasOwnProperty guard you gave RETIRED_HARNESSES.

The migration story (doctor → runtime message → README, all naming the same explicit selector) is coherent across all three surfaces, and thank you for the disciplined contributor-side CMAP — it made this review fast.

[mohidmakhdoomi]

@waleedkadous thanks for the careful review. Pushed the blocking fix + all three optional items as four atomic commits (7bd4267f..050335b8). Each summarized below, verified against the pushed diff.

BLOCKING — reconnect restartOnExit inconsistent with SessionManager (d6a0554d)

Both reconnect paths in tower-terminals.ts forced ptySession.restartOnExit = true for every architect. For a retired-harness architect, buildArchitectReconnectRestartOptions() returns undefined, so reconnectSession was told not to configure an auto-restart — and SessionManager already reflects that with restartOnExit: hasRestart (session-manager.ts:577). Forcing the PTY flag true anyway made PtySession hold clients in a "restarting…" wait for a process that can never come back.

Fix: gate both sites on dbSession.type === 'architect' && restartOptions:

• startup reconcile (_reconcileTerminalSessionsInner): added restartOptions to the probeResults destructure so it's in scope at the assignment.
• on-the-fly (getTerminalsForWorkspace): restartOptions was already in local scope.

The PTY flag now tracks whether a restart is actually configured, matching hasRestart.

Consumer-path test (per your ask — not just the helper's undefined-return unit test): new tower-terminals-restart-gating.test.ts drives both real reconnect paths (reconcileTerminalSessions + getTerminalsForWorkspace) end-to-end with a live shellper client, asserting the resulting PtySession.restartOnExit is false when restartOptions is undefined and true when it's defined. buildArchitectReconnectRestartOptions is mocked so its return is the controlled input — this exercises the consumer's gating branch directly. (An earlier real-config version tripped a Vitest module-identity artifact: under that file's vi.mock graph the helper's err instanceof RetiredHarnessError check and config.ts's throw bound different harness.js instances, so the check missed and fell to the generic fallback. Not a production issue — the single-graph chain is already covered by the retirement tests in tower-utils.test.ts.)

Optional 1 — siblingRegistrationIsLive returns true on retirement (37277cfb)

Returning false made the reconcile loop DELETE the user's sibling config row (setArchitectByName(..., null)). Now returns true, which routes to addArchitect — which fails closed on the same retirement (tower-instances.ts:1094, before any state is created) and leaves the row intact so it survives a harness fix. Docstring + the diagnostic INFO updated (now "keeping the row", not "pruning"). Adjusted the two tower-utils.test.ts retirement tests (false → true; log-message assertion). The tower-instances.ts reconcile tests mock this predicate, so the false→prune / true→respawn contract they assert is unchanged — I only corrected a now-stale "retired-harness prune" comment there.

Optional 2 — console.debug → logger (24bcb252)

assertBuilderHarnessNotRetired's deferred-error branch now uses logger.debug (agent-farm utils/logger) instead of console.debug. Since Tower imports this module, the bare console.debug always hit Tower's stdout; logger.debug is DEBUG-gated, so it stays quiet in normal Tower operation and prints under DEBUG. Strengthened the config.test.ts "defers on unknown harness" test to spy logger.debug.

Optional 3 — own-property guard on BUILTIN_HARNESSES[name] (bea86496)

Added getBuiltinHarness(name) — an own-property accessor mirroring the guard isRetiredHarness gives RETIRED_HARNESSES — and applied it at both user-controlled lookup sites: resolveHarness (config shell.*Harness) and, for consistency, harnessProviderFor in reset/context.ts (a running builder's launch-script harness name) which had the identical footgun. Inherited keys like constructor/toString now resolve to undefined instead of a truthy Object member masquerading as a provider. The constrained auto-detect lookup (resolveHarness, detected ∈ {claude,codex,opencode}) stays a direct index. Tests: resolveHarness rejects inherited Object keys with "Unknown harness"; a getBuiltinHarness block covers real names, unknown names, and the proto-key cases.

Verification

pnpm build clean (tsc + dashboard + skeleton). Full unit suite: 4156 passed / 48 skipped / 0 failed (+8 new tests). Not self-merging — leaving the GitHub merge to you.