From d8d0daf865a9ec53275cc4a850c521d76d8d48e4 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Sun, 31 May 2026 15:04:30 -0400 Subject: [PATCH 01/26] chore(porch): 929 init pir --- .../status.yaml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 codev/projects/929-support-codex-and-gemini-clis-/status.yaml diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml new file mode 100644 index 000000000..e2a82f3a0 --- /dev/null +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -0,0 +1,18 @@ +id: '929' +title: support-codex-and-gemini-clis- +protocol: pir +phase: plan +plan_phases: [] +current_plan_phase: null +gates: + plan-approval: + status: pending + dev-approval: + status: pending + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-05-31T19:04:30.567Z' +updated_at: '2026-05-31T19:04:30.568Z' From fdddc7e252cd6fb551e608ab9758178e19a5c894 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Sun, 31 May 2026 15:09:56 -0400 Subject: [PATCH 02/26] [PIR #929] Plan draft --- .../929-support-codex-and-gemini-clis-.md | 144 ++++++++++++++++++ codev/state/pir-929_thread.md | 20 +++ 2 files changed, 164 insertions(+) create mode 100644 codev/plans/929-support-codex-and-gemini-clis-.md create mode 100644 codev/state/pir-929_thread.md diff --git a/codev/plans/929-support-codex-and-gemini-clis-.md b/codev/plans/929-support-codex-and-gemini-clis-.md new file mode 100644 index 000000000..25962c0e8 --- /dev/null +++ b/codev/plans/929-support-codex-and-gemini-clis-.md @@ -0,0 +1,144 @@ +# PIR Plan: Support `codex` and `gemini` CLIs as architects + +## Understanding + +The harness abstraction (#591) already routes per-CLI role injection for claude/codex/gemini, and builders already run all three engines config-driven. The issue asks to bring **architects** to parity — selectable via `shell.architect` / `shell.architectHarness` in `.codev/config.json`. + +The blocking defect is a **latent resume crash-loop for non-Claude harnesses**, present at two sites with one root cause: session-discovery and `--resume` argument construction are hard-coded to Claude's on-disk session store, never gated on the configured harness. + +### Root cause (verified) + +`findLatestSessionId()` (`packages/codev/src/agent-farm/utils/claude-session-discovery.ts:43`) reads **only** `~/.claude/projects//*.jsonl`. Two callers feed its result straight into a ` --resume ` invocation without checking which CLI `` is: + +1. **Architect** — `packages/codev/src/agent-farm/servers/tower-instances.ts:500` + ```ts + const resumeSessionId = safeToResume ? findLatestSessionId(workspacePath) : null; + ... + cmdArgs = [...cmdParts.slice(1), '--resume', resumeSessionId]; // line 507 + ``` + If `shell.architect` is `codex`/`gemini` **and** any prior Claude session exists for that workspace dir (true for anyone who has run Claude Code there — including this repo), `afx workspace start` launches e.g. `codex --resume ` → invalid invocation → shellper restart-loops (`maxRestarts: 50`, `tower-instances.ts:538`) to death. + +2. **Builder** — `packages/codev/src/agent-farm/commands/spawn.ts:83` (`discoverResumeSession()`, called at `:459` and `:838`) returns a Claude UUID on any `--resume`; `packages/codev/src/agent-farm/commands/spawn-worktree.ts:739-751` then bakes `${baseCmd} --resume "${resumeSessionId}"` into the launch script's restart loop. `packages/codev/src/agent-farm/commands/workspace-recover.ts:254` re-enters via `afx spawn --resume`, inheriting the same bug. A resumed codex/gemini builder crash-loops identically. (Fresh builders never take this branch — which is why "builders already prove the path" does not cover resume.) + +The fresh-launch paths are already correct: `buildArchitectArgs()` (`tower-utils.ts:176`) and `startBuilderSession()`'s role-injection branch (`spawn-worktree.ts:752-786`) both resolve the harness via `getArchitectHarness` / `getBuilderHarness` (`config.ts:252,267`) and inject per-CLI flags. Siblings (`add-architect`) and reconnect already route through `buildArchitectArgs`. **Only the resume seam is harness-blind.** + +## Proposed Change + +Add an optional capability to the `HarnessProvider` interface that encapsulates session discovery. Only Claude implements it; every other harness gets a fresh launch. + +### 1. New `HarnessProvider` capability (`packages/codev/src/agent-farm/utils/harness.ts`) + +Add two optional, paired methods to the interface (per the deep-dive analysis §4.1 recommendation — discovery and invocation both owned by the provider, so the Claude-specific `--resume` flag is never hard-coded at a call site): + +```ts +/** + * Optional: discover a resumable prior session for the given working dir. + * Returns the session id to pass to buildResumeInvocation(), or null when + * none exists / this harness has no cwd-keyed session store. Harnesses that + * leave this undefined are treated as "no resume" → fresh launch. + * Only Claude implements it (store: ~/.claude/projects//.jsonl). + */ +discoverResumeSession?(absolutePath: string, opts?: { homeDir?: string }): string | null; + +/** + * Optional: given a session id from discoverResumeSession(), return the CLI + * args that resume it (e.g. claude → ['--resume', sessionId]). Only harnesses + * that implement discoverResumeSession need this. Keeps the resume flag-shape + * owned by the provider rather than the call sites. + */ +buildResumeInvocation?(sessionId: string): { args: string[] }; +``` + +- `CLAUDE_HARNESS` implements **both**: `discoverResumeSession` delegates to `findLatestSessionId(absolutePath, opts)`; `buildResumeInvocation` returns `{ args: ['--resume', sessionId] }`. +- `CODEX_HARNESS`, `GEMINI_HARNESS`, `OPENCODE_HARNESS`, and custom harnesses leave **both** undefined → callers coerce discovery `?? null` → fresh launch; `buildResumeInvocation` is never reached for them. + +This is the "claude returns the resume invocation; codex/gemini return null" seam the issue and analysis both call for. The two call sites consume the pair: discover an id (null ⇒ fresh), and if non-null, splice in `harness.buildResumeInvocation!(id).args` instead of literal `['--resume', id]`. + +### 2. Architect site (`tower-instances.ts:500-509`) + +Replace the unconditional `findLatestSessionId(workspacePath)` with the harness capability, preserving the existing `safeToResume` sibling-collision guard: + +```ts +const architectHarness = getArchitectHarness(workspacePath); +const resumeSessionId = safeToResume + ? (architectHarness.discoverResumeSession?.(workspacePath) ?? null) + : null; +... +if (resumeSessionId) { + cmdArgs = [...cmdParts.slice(1), ...architectHarness.buildResumeInvocation!(resumeSessionId).args]; + ... +} +``` + +Update the now-inaccurate "if a prior **Claude** session exists" comment to note the harness gate. (`getArchitectHarness` already resolves from `.codev/config.json` `shell.architect`/`architectHarness`, identical to `buildArchitectArgs`, so a codex/gemini config yields `undefined` → fresh, role-injected launch.) + +### 3. Builder site (`spawn.ts:83`, callers `:459`/`:838`) + +Thread the builder harness into `discoverResumeSession` and gate on it: + +```ts +export function discoverResumeSession( + worktreePath: string, + isResume: boolean | undefined, + harness: HarnessProvider, +): string | undefined { + if (!isResume) return undefined; + const found = harness.discoverResumeSession?.(worktreePath) ?? null; + if (found) { logger.kv('Session', `${found.slice(0, 8)}… (resuming conversation)`); return found; } + logger.info('No prior conversation found for this worktree; starting a fresh session.'); + return undefined; +} +``` + +Both callers pass `getBuilderHarness(config.workspaceRoot)`. When the harness returns null, `startBuilderSession` receives `undefined` and takes the fresh role-injection path — so `spawn-worktree.ts:739-751` (the `--resume` restart loop) is naturally never reached for codex/gemini. In the claude resume branch, `startBuilderSession` builds the resume line from `getBuilderHarness(config.workspaceRoot).buildResumeInvocation!(resumeSessionId).args` (joined into the bash script) rather than the literal `--resume ""`, so the flag-shape stays provider-owned. `workspace-recover.ts` inherits the fix for free (it shells out to `afx spawn --resume`). The existing `buildResumeNotice` prompt still prepends for fresh-launched resumed builders (`spawn.ts:462`). + +### 4. Tests + +- **`packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts`** — update calls to pass a harness; add cases asserting `CODEX_HARNESS`/`GEMINI_HARNESS` return `undefined` even when a Claude jsonl exists (the regression guard), and that `CLAUDE_HARNESS` still returns the newest UUID. +- **`packages/codev/src/agent-farm/__tests__/af-architect.test.ts`** (currently Claude-only) — add codex/gemini cases for `buildArchitectArgs` fresh-launch arg/env construction (codex `-c model_instructions_file=`, gemini `GEMINI_SYSTEM_MD`), and a resume-skip assertion: with a codex/gemini architect harness, `discoverResumeSession?.()` is undefined ⇒ resume is skipped even with a stale jsonl present. +- Run the existing reconnect/sibling tests to confirm no regression (they already route through `buildArchitectArgs`). + +### 5. `doctor` / docs + +- No functional `doctor` change needed (`doctor.ts:594` already bars only OpenCode as architect; codex/gemini already pass). Add a short positive affirmation line that codex/gemini are supported architects. +- Document in the relevant resource doc (and CLAUDE.md / AGENTS.md if the architect section warrants it): codex/gemini are supported architects selected via `.codev/config.json` `shell.architect`/`shell.architectHarness`; **conversation resume is Claude-main-only** (codex/gemini architects relaunch fresh with role injection); `.codev/config.json` — not `TOWER_ARCHITECT_CMD`/`--architect-cmd` — is the supported selection mechanism. + +### 6. Submit-strategy hook (conditional — decided at the `dev-approval` gate) + +MVP item 3 (per-harness submit pacing / bracketed-paste in `message-write.ts`) is **conditional on empirical validation**. The current pacing (`message-write.ts:33` `writeMessageToSession`, tuned to Claude's TUI: single write + delayed Enter for ≤3 lines, line-by-line with 10ms gaps for >3) may or may not deliver cleanly on codex/gemini TUIs. If the human's `dev-approval` testing shows flaky multi-line/interrupt/streaming delivery, add an optional `getSubmitStrategy?()` to `HarnessProvider` and branch the writer on it. If delivery is clean, no change. The seam is designed but not implemented preemptively. + +## Files to Change + +- `packages/codev/src/agent-farm/utils/harness.ts` — add `discoverResumeSession?` + `buildResumeInvocation?` to interface; implement both in `CLAUDE_HARNESS` (`discoverResumeSession` delegates to `findLatestSessionId`, `buildResumeInvocation` → `['--resume', id]`); import `findLatestSessionId` from `claude-session-discovery.ts`. +- `packages/codev/src/agent-farm/servers/tower-instances.ts:500-509` — gate architect resume on `getArchitectHarness(...).discoverResumeSession?.()`; build args via `buildResumeInvocation`; update comment. +- `packages/codev/src/agent-farm/commands/spawn.ts:83` — add `harness` param; gate on it. `:459`, `:838` — pass `getBuilderHarness(config.workspaceRoot)`. +- `packages/codev/src/agent-farm/commands/spawn-worktree.ts:739-751` — in the claude resume branch, build the resume line from `getBuilderHarness(...).buildResumeInvocation!(id).args` instead of literal `--resume ""`. +- `packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts` — pass harness; add codex/gemini null-return regression cases. +- `packages/codev/src/agent-farm/__tests__/af-architect.test.ts` — add codex/gemini arg-construction + resume-skip cases. +- `packages/codev/src/commands/doctor.ts` — affirm codex/gemini architect support. +- Docs: a resource doc (e.g. `codev/resources/arch.md` or commands ref) + CLAUDE.md/AGENTS.md note on resume-is-Claude-only + config-driven selection. +- *(Conditional)* `packages/codev/src/agent-farm/servers/message-write.ts` + `harness.ts` — submit strategy, only if `dev-approval` validation reveals flakiness. + +## Risks & Alternatives Considered + +- **Risk: `harness.ts` importing `claude-session-discovery.ts`.** The latter imports only Node builtins — no circular dependency. Low risk. +- **Risk: `TOWER_ARCHITECT_CMD`/`--architect-cmd` env/CLI override without matching `.codev/config.json`.** `getArchitectHarness` resolves the harness from config only, so an env-set `codex` with no config still resolves the claude harness → would still attempt resume. This is the issue's explicit **nice-to-have** ("command-aware harness resolution"); MVP fixes the config-driven path (which all acceptance criteria target) and **documents** the override caveat rather than expanding scope. +- **Risk: empirical acceptance criteria can't be unit-tested here.** Launch-on-stale-jsonl, add-architect, `afx send` multiline/interrupt/streaming, reconnect, affinity, dashboard scrollback all need codex/gemini actually installed. These are validated by the human running the worktree at the `dev-approval` gate — the reason PIR (not AIR/BUGFIX) was chosen. The unit tests cover the deterministic core (arg construction + resume-skip logic). +- **Alternative: a boolean `supportsResume` flag + keep `findLatestSessionId` and literal `--resume` at call sites.** Rejected — leaves the Claude-specific discovery *and* the `--resume` flag-shape spread across call sites. Pairing `discoverResumeSession` with `buildResumeInvocation` (per analysis §4.1) moves both fully into the provider, which is the cleaner seam. The builder bash path consumes `buildResumeInvocation(id).args` by joining them into the script (`${baseCmd} `), so no Claude-specific string survives at the call site. + +## Test Plan + +**Unit (run in this worktree, gate-verifiable from the diff):** +- `pnpm --filter @cluesmith/codev test` (or the af test subset) — new/updated `discover-resume-session.test.ts` + `af-architect.test.ts` green; existing reconnect/sibling/architect tests still green. +- Build: `pnpm --filter @cluesmith/codev build` clean (TS types for the new optional method). + +**Manual / empirical (reviewer at the `dev-approval` gate, needs codex & gemini installed). For each of codex and gemini set `shell.architect` accordingly in `.codev/config.json`:** +- `afx architect` (no-Tower) launches with role injected. +- `afx workspace start` main architect launches on a **clean** cwd. +- `afx workspace start` main architect launches with a **stale Claude `.jsonl` present** in `~/.claude/projects//` — must NOT crash-loop (primary regression target). Confirm no `--resume` in the launched command (check the shellper/PTY command). +- `afx workspace add-architect` sibling launches. +- `afx send` delivers + submits: single-line, multi-line (>3 lines, no swallowed Enter), `--interrupt`, and while the TUI is streaming. (If flaky → triggers the conditional submit-strategy work.) +- Tower stop→start reconnect and shellper auto-restart both relaunch the architect. +- A builder spawned by a codex/gemini architect preserves `CODEV_ARCHITECT_NAME` affinity. +- `afx spawn --resume` on a non-Claude builder → fresh launch + resume notice (NOT `--resume `); inspect `.builder-start.sh`. +- Dashboard web-terminal scrollback survives heavy TUI redraw (nice-to-have; note if it regresses). diff --git a/codev/state/pir-929_thread.md b/codev/state/pir-929_thread.md new file mode 100644 index 000000000..977a9297e --- /dev/null +++ b/codev/state/pir-929_thread.md @@ -0,0 +1,20 @@ +# PIR #929 — Support codex & gemini CLIs as architects + +## Builder thread + +### 2026-05-31 — Plan phase + +Investigated the resume crash-loop bug. Root cause confirmed across two sites, one mechanism: + +- **Architect**: `tower-instances.ts:500` calls `findLatestSessionId(workspacePath)` (reads only `~/.claude/projects//*.jsonl`) without gating on the configured harness, then builds `[...cmdParts.slice(1), '--resume', resumeSessionId]`. A codex/gemini architect + any stale Claude jsonl ⇒ `codex --resume ` ⇒ shellper restart-loop to death. +- **Builder**: `spawn.ts:83 discoverResumeSession()` (called at 459 + 838) has the same Claude-only assumption; `spawn-worktree.ts:746` then bakes `${baseCmd} --resume "${id}"` into the launch script. `workspace-recover.ts` re-enters via `afx spawn --resume`, inheriting it. + +**Fix direction**: add an optional `discoverResumeSession?` capability to `HarnessProvider`. Only `CLAUDE_HARNESS` implements it (delegates to `findLatestSessionId`); codex/gemini/opencode leave it undefined ⇒ null ⇒ fresh launch via `buildArchitectArgs` / role-injection script path. Gate both call sites on the harness. The downstream `--resume` arg/script construction is reached only when claude returned a non-null id, so it stays claude-shaped safely. + +**Scoping decision**: MVP fixes the **config-driven** selection path (`.codev/config.json` `shell.architect`/`architectHarness`), which is what every acceptance criterion is phrased around. The `TOWER_ARCHITECT_CMD`/`--architect-cmd` env/CLI-override-without-matching-config edge stays a documented nice-to-have (issue lists it as such). + +**Empirical validation** (launch on stale-jsonl cwd, add-architect, `afx send` multiline/interrupt/streaming, reconnect, affinity, dashboard scrollback) needs codex/gemini actually installed — that happens at the `dev-approval` gate when the human runs the worktree. That's precisely why PIR was chosen. + +Architect PR-target note: no push access to cluesmith/codev — push branch to fork `mohidmakhdoomi/codev`, open PR with `gh pr create --repo mohidmakhdoomi/codev`. + +Plan written, awaiting `plan-approval`. From 0ff54202e6c5b1e4f0e6301f4ef0bc21a565e456 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Sun, 31 May 2026 15:10:01 -0400 Subject: [PATCH 03/26] chore(porch): 929 plan-approval gate-requested --- codev/projects/929-support-codex-and-gemini-clis-/status.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index e2a82f3a0..0c4c0317c 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -7,6 +7,7 @@ current_plan_phase: null gates: plan-approval: status: pending + requested_at: '2026-05-31T19:10:01.535Z' dev-approval: status: pending pr: @@ -15,4 +16,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-05-31T19:04:30.568Z' +updated_at: '2026-05-31T19:10:01.535Z' From 53374f30b9b3f00b558e2a3825a6b62b09d975de Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Sun, 31 May 2026 15:29:41 -0400 Subject: [PATCH 04/26] =?UTF-8?q?[PIR=20#929]=20Plan=20revised=20=E2=80=94?= =?UTF-8?q?=20address=20architect=20feedback=20(5=20issues)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../929-support-codex-and-gemini-clis-.md | 121 +++++++++++------- codev/state/pir-929_thread.md | 10 ++ 2 files changed, 85 insertions(+), 46 deletions(-) diff --git a/codev/plans/929-support-codex-and-gemini-clis-.md b/codev/plans/929-support-codex-and-gemini-clis-.md index 25962c0e8..e427e9f37 100644 --- a/codev/plans/929-support-codex-and-gemini-clis-.md +++ b/codev/plans/929-support-codex-and-gemini-clis-.md @@ -28,93 +28,119 @@ Add an optional capability to the `HarnessProvider` interface that encapsulates ### 1. New `HarnessProvider` capability (`packages/codev/src/agent-farm/utils/harness.ts`) -Add two optional, paired methods to the interface (per the deep-dive analysis §4.1 recommendation — discovery and invocation both owned by the provider, so the Claude-specific `--resume` flag is never hard-coded at a call site): +Add **one** optional method that bundles discovery + both invocation forms. This mirrors the convention the interface already uses for role injection — `buildRoleInjection()` returns Node argv for `spawn()` call sites (`harness.ts:24`) and `buildScriptRoleInjection()` returns a shell-**escaped** bash fragment (`harness.ts:34`). Bundling discovery and invocation into a single method means the call sites never see an independently-optional second method (no non-null assertion `!`), and the builder bash path gets a pre-escaped fragment instead of word-splitting a raw argv array: ```ts /** - * Optional: discover a resumable prior session for the given working dir. - * Returns the session id to pass to buildResumeInvocation(), or null when - * none exists / this harness has no cwd-keyed session store. Harnesses that - * leave this undefined are treated as "no resume" → fresh launch. - * Only Claude implements it (store: ~/.claude/projects//.jsonl). + * Optional: discover a resumable prior session for the given working dir and + * return how to resume it — in BOTH forms, mirroring buildRoleInjection / + * buildScriptRoleInjection: + * - args: Node argv for spawn() call sites (architect / tower-instances) + * - scriptFragment: shell-escaped fragment for bash script generation (builder) + * Returns null when no resumable session exists or this harness has no + * cwd-keyed session store → callers fall back to a fresh launch. Only Claude + * implements it (store: ~/.claude/projects//.jsonl). */ -discoverResumeSession?(absolutePath: string, opts?: { homeDir?: string }): string | null; - -/** - * Optional: given a session id from discoverResumeSession(), return the CLI - * args that resume it (e.g. claude → ['--resume', sessionId]). Only harnesses - * that implement discoverResumeSession need this. Keeps the resume flag-shape - * owned by the provider rather than the call sites. - */ -buildResumeInvocation?(sessionId: string): { args: string[] }; +buildResume?(absolutePath: string, opts?: { homeDir?: string }): { + sessionId: string; + args: string[]; + scriptFragment: string; +} | null; ``` -- `CLAUDE_HARNESS` implements **both**: `discoverResumeSession` delegates to `findLatestSessionId(absolutePath, opts)`; `buildResumeInvocation` returns `{ args: ['--resume', sessionId] }`. -- `CODEX_HARNESS`, `GEMINI_HARNESS`, `OPENCODE_HARNESS`, and custom harnesses leave **both** undefined → callers coerce discovery `?? null` → fresh launch; `buildResumeInvocation` is never reached for them. +`CLAUDE_HARNESS` implements it: + +```ts +buildResume: (absolutePath, opts) => { + const sessionId = findLatestSessionId(absolutePath, opts); + if (!sessionId) return null; + return { + sessionId, + args: ['--resume', sessionId], + scriptFragment: `--resume '${shellEscapeSingleQuote(sessionId)}'`, + }; +}, +``` -This is the "claude returns the resume invocation; codex/gemini return null" seam the issue and analysis both call for. The two call sites consume the pair: discover an id (null ⇒ fresh), and if non-null, splice in `harness.buildResumeInvocation!(id).args` instead of literal `['--resume', id]`. +`CODEX_HARNESS`, `GEMINI_HARNESS`, `OPENCODE_HARNESS`, and custom harnesses leave `buildResume` **undefined** → callers do `harness.buildResume?.(path) ?? null` → fresh launch. This is the "claude returns the resume invocation; codex/gemini return null" seam the issue and analysis §4.1 call for, with the Node-vs-script split the existing role-injection methods already establish. (Session ids are bare UUIDs so escaping is belt-and-suspenders today, but it keeps the bash path correct-by-construction and consistent with `buildScriptRoleInjection`'s `:769`/`:858` escaping.) ### 2. Architect site (`tower-instances.ts:500-509`) -Replace the unconditional `findLatestSessionId(workspacePath)` with the harness capability, preserving the existing `safeToResume` sibling-collision guard: +Replace the unconditional `findLatestSessionId(workspacePath)` with the harness capability (Node-argv form), preserving the existing `safeToResume` sibling-collision guard: ```ts const architectHarness = getArchitectHarness(workspacePath); -const resumeSessionId = safeToResume - ? (architectHarness.discoverResumeSession?.(workspacePath) ?? null) - : null; +const resume = safeToResume ? (architectHarness.buildResume?.(workspacePath) ?? null) : null; ... -if (resumeSessionId) { - cmdArgs = [...cmdParts.slice(1), ...architectHarness.buildResumeInvocation!(resumeSessionId).args]; - ... +if (resume) { + cmdArgs = [...cmdParts.slice(1), ...resume.args]; + harnessEnv = {}; + _deps.log('INFO', `Resuming main architect session ${resume.sessionId.slice(0, 8)}… for ${workspacePath}`); +} else { + const built = buildArchitectArgs(cmdParts.slice(1), workspacePath); + cmdArgs = built.args; harnessEnv = built.env; } ``` Update the now-inaccurate "if a prior **Claude** session exists" comment to note the harness gate. (`getArchitectHarness` already resolves from `.codev/config.json` `shell.architect`/`architectHarness`, identical to `buildArchitectArgs`, so a codex/gemini config yields `undefined` → fresh, role-injected launch.) -### 3. Builder site (`spawn.ts:83`, callers `:459`/`:838`) +### 3. Builder site (`spawn.ts:83`, callers `:459`/`:838`; consumed in `spawn-worktree.ts:739-751`) -Thread the builder harness into `discoverResumeSession` and gate on it: +Thread the builder harness into `discoverResumeSession`, gate on it, and return the bundled resume object (carrying the escaped `scriptFragment`) so the bash generator never re-derives the flag: ```ts export function discoverResumeSession( worktreePath: string, isResume: boolean | undefined, harness: HarnessProvider, -): string | undefined { +): { sessionId: string; args: string[]; scriptFragment: string } | undefined { if (!isResume) return undefined; - const found = harness.discoverResumeSession?.(worktreePath) ?? null; - if (found) { logger.kv('Session', `${found.slice(0, 8)}… (resuming conversation)`); return found; } + const resume = harness.buildResume?.(worktreePath) ?? null; + if (resume) { logger.kv('Session', `${resume.sessionId.slice(0, 8)}… (resuming conversation)`); return resume; } logger.info('No prior conversation found for this worktree; starting a fresh session.'); return undefined; } ``` -Both callers pass `getBuilderHarness(config.workspaceRoot)`. When the harness returns null, `startBuilderSession` receives `undefined` and takes the fresh role-injection path — so `spawn-worktree.ts:739-751` (the `--resume` restart loop) is naturally never reached for codex/gemini. In the claude resume branch, `startBuilderSession` builds the resume line from `getBuilderHarness(config.workspaceRoot).buildResumeInvocation!(resumeSessionId).args` (joined into the bash script) rather than the literal `--resume ""`, so the flag-shape stays provider-owned. `workspace-recover.ts` inherits the fix for free (it shells out to `afx spawn --resume`). The existing `buildResumeNotice` prompt still prepends for fresh-launched resumed builders (`spawn.ts:462`). +Both callers pass `getBuilderHarness(config.workspaceRoot)` and forward the result into `startBuilderSession`, whose `resumeSessionId?: string` param becomes `resume?: { scriptFragment: string }`. In the resume branch (`spawn-worktree.ts:739-751`) the script line becomes `${baseCmd} ${resume.scriptFragment}` — a single pre-escaped fragment, **not** a `.join(' ')` of a raw argv array (which would word-split / comma-stringify). When the harness returns `undefined` (codex/gemini), `startBuilderSession` takes the fresh role-injection path and the `--resume` restart loop is never reached. `workspace-recover.ts` inherits the fix for free (it shells out to `afx spawn --resume`). The existing `buildResumeNotice` prompt still prepends for fresh-launched resumed builders (`spawn.ts:462`). + +### 4. Tests — placed at the layer where each bug actually lives + +The two crash-loops live in **Tower launch** (`tower-instances.ts`) and **builder script generation** (`spawn-worktree.ts`), so the regression guards must sit there. `af-architect.test.ts` only exercises the no-Tower `afx architect` command (`spawn()` of the local session) — a resume-skip test there would **not** guard the real architect regression. Layering: + +- **`packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts`** (unit, keep — correctly placed) — update calls to pass a harness; add cases asserting `CODEX_HARNESS`/`GEMINI_HARNESS` return `undefined` even when a stale Claude jsonl exists (`buildResume` undefined), and that `CLAUDE_HARNESS` returns `{ sessionId, args:['--resume',id], scriptFragment }` for the newest UUID. +- **`packages/codev/src/agent-farm/__tests__/tower-instances.test.ts`** (exists) — **architect regression guard**: stale Claude jsonl present + codex/gemini architect harness → asserts the launched command is the fresh `buildArchitectArgs` form (role-injected, harness env set) with **no `--resume`** in `cmdArgs`. Claude + stale jsonl → asserts `--resume ` is present (resume still works). +- **`packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts`** (exists) — **builder script-shape guard**: a resumed builder's generated `.builder-start.sh` uses the harness-provided escaped `scriptFragment` (e.g. `--resume ''`), not an unquoted or comma-joined argv; codex/gemini resumed builder → fresh role-injection script, no `--resume`. +- **`packages/codev/src/agent-farm/__tests__/af-architect.test.ts`** — keep Claude-only as is; optionally add codex/gemini `buildArchitectArgs` fresh-launch arg/env construction (codex `-c model_instructions_file=`, gemini `GEMINI_SYSTEM_MD`) as plain coverage, with an explicit note that this file does **not** guard the resume regression (that's `tower-instances.test.ts`). +- Run existing reconnect/sibling tests to confirm no regression (they already route through `buildArchitectArgs`). + +### 5. Gemini project-context manifest (promoted to MVP) -### 4. Tests +A codex architect reads `AGENTS.md` natively for project context; a **gemini** architect ships no `GEMINI.md` (none exists in the repo, nor a `.gemini/` dir), so without help it launches with the injected *role* but no native project *manifesto*. The near-zero-effort fix is `.gemini/settings.json` with `context.fileName` → `AGENTS.md`, which points gemini's context loader at the manifest codex already uses. Promoting from nice-to-have to MVP so gemini doesn't ship context-blind: -- **`packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts`** — update calls to pass a harness; add cases asserting `CODEX_HARNESS`/`GEMINI_HARNESS` return `undefined` even when a Claude jsonl exists (the regression guard), and that `CLAUDE_HARNESS` still returns the newest UUID. -- **`packages/codev/src/agent-farm/__tests__/af-architect.test.ts`** (currently Claude-only) — add codex/gemini cases for `buildArchitectArgs` fresh-launch arg/env construction (codex `-c model_instructions_file=`, gemini `GEMINI_SYSTEM_MD`), and a resume-skip assertion: with a codex/gemini architect harness, `discoverResumeSession?.()` is undefined ⇒ resume is skipped even with a stale jsonl present. -- Run the existing reconnect/sibling tests to confirm no regression (they already route through `buildArchitectArgs`). +- Add an optional `getArchitectFiles?(workspacePath): Array<{ relativePath; content }>` to `HarnessProvider`, parallel to the existing `getWorktreeFiles?` (`harness.ts:44`). `GEMINI_HARNESS` returns `.gemini/settings.json` = `{ "context": { "fileName": "AGENTS.md" } }`. Others omit it. +- The architect launch path (`buildArchitectArgs` in `tower-utils.ts`, or its caller in `tower-instances.ts`) writes these files **only if missing** — never clobbering a user's existing `.gemini/settings.json`. Idempotent and merge-safe. +- Test in `tower-instances.test.ts` (or a small `harness.test.ts`): gemini architect with no `.gemini/settings.json` → file written with `context.fileName: "AGENTS.md"`; pre-existing file → left untouched. -### 5. `doctor` / docs +### 6. `doctor` / docs - No functional `doctor` change needed (`doctor.ts:594` already bars only OpenCode as architect; codex/gemini already pass). Add a short positive affirmation line that codex/gemini are supported architects. -- Document in the relevant resource doc (and CLAUDE.md / AGENTS.md if the architect section warrants it): codex/gemini are supported architects selected via `.codev/config.json` `shell.architect`/`shell.architectHarness`; **conversation resume is Claude-main-only** (codex/gemini architects relaunch fresh with role injection); `.codev/config.json` — not `TOWER_ARCHITECT_CMD`/`--architect-cmd` — is the supported selection mechanism. +- Document in the relevant resource doc (and CLAUDE.md / AGENTS.md if the architect section warrants it): codex/gemini are supported architects selected via `.codev/config.json` `shell.architect`/`shell.architectHarness`; **conversation resume is Claude-main-only** (codex/gemini architects relaunch fresh with role injection); `.codev/config.json` — not `TOWER_ARCHITECT_CMD`/`--architect-cmd`/`--builder-cmd` — is the supported harness-selection mechanism (see the two override-mismatch caveats in Risks). -### 6. Submit-strategy hook (conditional — decided at the `dev-approval` gate) +### 7. Submit-strategy hook (conditional — decided at the `dev-approval` gate) MVP item 3 (per-harness submit pacing / bracketed-paste in `message-write.ts`) is **conditional on empirical validation**. The current pacing (`message-write.ts:33` `writeMessageToSession`, tuned to Claude's TUI: single write + delayed Enter for ≤3 lines, line-by-line with 10ms gaps for >3) may or may not deliver cleanly on codex/gemini TUIs. If the human's `dev-approval` testing shows flaky multi-line/interrupt/streaming delivery, add an optional `getSubmitStrategy?()` to `HarnessProvider` and branch the writer on it. If delivery is clean, no change. The seam is designed but not implemented preemptively. ## Files to Change -- `packages/codev/src/agent-farm/utils/harness.ts` — add `discoverResumeSession?` + `buildResumeInvocation?` to interface; implement both in `CLAUDE_HARNESS` (`discoverResumeSession` delegates to `findLatestSessionId`, `buildResumeInvocation` → `['--resume', id]`); import `findLatestSessionId` from `claude-session-discovery.ts`. -- `packages/codev/src/agent-farm/servers/tower-instances.ts:500-509` — gate architect resume on `getArchitectHarness(...).discoverResumeSession?.()`; build args via `buildResumeInvocation`; update comment. -- `packages/codev/src/agent-farm/commands/spawn.ts:83` — add `harness` param; gate on it. `:459`, `:838` — pass `getBuilderHarness(config.workspaceRoot)`. -- `packages/codev/src/agent-farm/commands/spawn-worktree.ts:739-751` — in the claude resume branch, build the resume line from `getBuilderHarness(...).buildResumeInvocation!(id).args` instead of literal `--resume ""`. -- `packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts` — pass harness; add codex/gemini null-return regression cases. -- `packages/codev/src/agent-farm/__tests__/af-architect.test.ts` — add codex/gemini arg-construction + resume-skip cases. +- `packages/codev/src/agent-farm/utils/harness.ts` — add `buildResume?` (bundled discovery + Node-argv + escaped script fragment) and `getArchitectFiles?` to the interface; implement `buildResume` in `CLAUDE_HARNESS` (delegates to `findLatestSessionId`, returns `args` + escaped `scriptFragment`); implement `getArchitectFiles` in `GEMINI_HARNESS` (`.gemini/settings.json`); import `findLatestSessionId` from `claude-session-discovery.ts`. +- `packages/codev/src/agent-farm/servers/tower-instances.ts:500-514` — gate architect resume on `getArchitectHarness(...).buildResume?.()` (Node-argv form); write `getArchitectFiles?()` if-missing on launch; update comment. +- `packages/codev/src/agent-farm/commands/spawn.ts:83` — `discoverResumeSession` takes `harness`, returns the bundled resume object. `:459`, `:838` — pass `getBuilderHarness(config.workspaceRoot)`, forward object to `startBuilderSession`. +- `packages/codev/src/agent-farm/commands/spawn-worktree.ts:724-751` — `startBuilderSession`'s `resumeSessionId?: string` → `resume?: {...scriptFragment}`; resume branch emits `${baseCmd} ${resume.scriptFragment}`. +- `packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts` — pass harness; codex/gemini null-return + claude bundled-object cases. +- `packages/codev/src/agent-farm/__tests__/tower-instances.test.ts` — architect resume-skip regression guard (codex/gemini + stale jsonl → no `--resume`); gemini `getArchitectFiles` write-if-missing. +- `packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts` — builder resume script uses escaped `scriptFragment`; codex/gemini resumed builder → fresh script. +- `packages/codev/src/agent-farm/__tests__/af-architect.test.ts` — (optional) codex/gemini fresh arg-construction coverage; note it does not guard the resume regression. - `packages/codev/src/commands/doctor.ts` — affirm codex/gemini architect support. - Docs: a resource doc (e.g. `codev/resources/arch.md` or commands ref) + CLAUDE.md/AGENTS.md note on resume-is-Claude-only + config-driven selection. - *(Conditional)* `packages/codev/src/agent-farm/servers/message-write.ts` + `harness.ts` — submit strategy, only if `dev-approval` validation reveals flakiness. @@ -122,9 +148,11 @@ MVP item 3 (per-harness submit pacing / bracketed-paste in `message-write.ts`) i ## Risks & Alternatives Considered - **Risk: `harness.ts` importing `claude-session-discovery.ts`.** The latter imports only Node builtins — no circular dependency. Low risk. -- **Risk: `TOWER_ARCHITECT_CMD`/`--architect-cmd` env/CLI override without matching `.codev/config.json`.** `getArchitectHarness` resolves the harness from config only, so an env-set `codex` with no config still resolves the claude harness → would still attempt resume. This is the issue's explicit **nice-to-have** ("command-aware harness resolution"); MVP fixes the config-driven path (which all acceptance criteria target) and **documents** the override caveat rather than expanding scope. +- **Risk: `TOWER_ARCHITECT_CMD`/`--architect-cmd` architect-override without matching `.codev/config.json`.** `getArchitectHarness` resolves the harness from config only, so an env-set `codex` with no config still resolves the claude harness → would still attempt resume. This is the issue's explicit **nice-to-have** ("command-aware harness resolution"); MVP fixes the config-driven path (which all acceptance criteria target) and **documents** the override caveat rather than expanding scope. +- **Risk: `--builder-cmd`/builder-env override without matching config — the exact builder analog.** The builder command comes from `getResolvedCommands()` (`spawn.ts:469`/`:840`, honors `--builder-cmd` and env), but the harness comes from `getBuilderHarness(config…)` (config only, `config.ts:267`). Set `--builder-cmd codex` with no `builderHarness`/`shell.builder` config and a resumed builder would still resolve the claude harness and attempt `codex --resume `. Same disposition as the architect override: documented, not fixed (config-driven `shell.builder`/`builderHarness` is the supported selection mechanism). - **Risk: empirical acceptance criteria can't be unit-tested here.** Launch-on-stale-jsonl, add-architect, `afx send` multiline/interrupt/streaming, reconnect, affinity, dashboard scrollback all need codex/gemini actually installed. These are validated by the human running the worktree at the `dev-approval` gate — the reason PIR (not AIR/BUGFIX) was chosen. The unit tests cover the deterministic core (arg construction + resume-skip logic). -- **Alternative: a boolean `supportsResume` flag + keep `findLatestSessionId` and literal `--resume` at call sites.** Rejected — leaves the Claude-specific discovery *and* the `--resume` flag-shape spread across call sites. Pairing `discoverResumeSession` with `buildResumeInvocation` (per analysis §4.1) moves both fully into the provider, which is the cleaner seam. The builder bash path consumes `buildResumeInvocation(id).args` by joining them into the script (`${baseCmd} `), so no Claude-specific string survives at the call site. +- **Alternative: a boolean `supportsResume` flag + keep `findLatestSessionId` and literal `--resume` at call sites.** Rejected — leaves the Claude-specific discovery *and* the `--resume` flag-shape spread across call sites. A single `buildResume()` returning both the Node-argv and the **escaped** script fragment moves both fully into the provider (mirroring `buildRoleInjection`/`buildScriptRoleInjection`), so no Claude-specific string and no shell-quoting decision survives at the call site. +- **Alternative: two independently-optional methods (`discoverResumeSession` + `buildResumeInvocation`).** Rejected — independent optionality forces a non-null assertion (`!`) at call sites and tempts a raw-argv `.join(' ')` into the bash script (word-splits / comma-stringifies on any arg with whitespace). Bundling them removes the `!` and guarantees the script form is pre-escaped. ## Test Plan @@ -137,6 +165,7 @@ MVP item 3 (per-harness submit pacing / bracketed-paste in `message-write.ts`) i - `afx workspace start` main architect launches on a **clean** cwd. - `afx workspace start` main architect launches with a **stale Claude `.jsonl` present** in `~/.claude/projects//` — must NOT crash-loop (primary regression target). Confirm no `--resume` in the launched command (check the shellper/PTY command). - `afx workspace add-architect` sibling launches. +- **gemini only**: after launch, `.gemini/settings.json` exists with `context.fileName: "AGENTS.md"` (and a pre-existing one was not clobbered). - `afx send` delivers + submits: single-line, multi-line (>3 lines, no swallowed Enter), `--interrupt`, and while the TUI is streaming. (If flaky → triggers the conditional submit-strategy work.) - Tower stop→start reconnect and shellper auto-restart both relaunch the architect. - A builder spawned by a codex/gemini architect preserves `CODEV_ARCHITECT_NAME` affinity. diff --git a/codev/state/pir-929_thread.md b/codev/state/pir-929_thread.md index 977a9297e..6c221ad8f 100644 --- a/codev/state/pir-929_thread.md +++ b/codev/state/pir-929_thread.md @@ -18,3 +18,13 @@ Investigated the resume crash-loop bug. Root cause confirmed across two sites, o Architect PR-target note: no push access to cluesmith/codev — push branch to fork `mohidmakhdoomi/codev`, open PR with `gh pr create --repo mohidmakhdoomi/codev`. Plan written, awaiting `plan-approval`. + +### 2026-05-31 — Plan revision (architect feedback, 5 verified issues) + +Revised after a 2-way consult + architect verification: +1+2. **Seam redesign**: collapsed the two paired methods (`discoverResumeSession` + `buildResumeInvocation!`) into a single bundled `buildResume?(cwd, opts) → { sessionId, args, scriptFragment } | null`, mirroring `buildRoleInjection`/`buildScriptRoleInjection`. Node-argv `args` for the architect `spawn()` site; **shell-escaped** `scriptFragment` for the builder bash generator. Kills the `!` non-null assertion and the raw-argv `.join(' ')` word-split/quoting bug. +3. **Test layering fix**: regression guards moved to where the bugs live — `tower-instances.test.ts` (architect launch, no `--resume` for codex/gemini + stale jsonl) and `spawn-worktree.test.ts` (builder script uses escaped fragment). `af-architect.test.ts` (no-Tower command only) noted as NOT guarding the real regression. +4. **Gemini context promoted to MVP**: add `getArchitectFiles?` hook; `GEMINI_HARNESS` writes `.gemini/settings.json` `context.fileName → AGENTS.md` (write-if-missing). Codex already reads AGENTS.md natively; gemini shipped context-blind otherwise. +5. **Builder override caveat documented**: `--builder-cmd`/env vs config-only `getBuilderHarness` is the exact analog of the architect `TOWER_ARCHITECT_CMD` trap — added to Risks alongside it. + +Re-signalled, awaiting architect re-review. From dccfbf8a53990c8115e9a8446138023c6544a060 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Sun, 31 May 2026 15:34:05 -0400 Subject: [PATCH 05/26] chore(porch): 929 plan-approval gate-approved --- .../projects/929-support-codex-and-gemini-clis-/status.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index 0c4c0317c..0a478cab9 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -6,8 +6,9 @@ plan_phases: [] current_plan_phase: null gates: plan-approval: - status: pending + status: approved requested_at: '2026-05-31T19:10:01.535Z' + approved_at: '2026-05-31T19:34:05.482Z' dev-approval: status: pending pr: @@ -16,4 +17,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-05-31T19:10:01.535Z' +updated_at: '2026-05-31T19:34:05.483Z' From 139378167675e4c347f1101a73d5f58885c8bccf Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Sun, 31 May 2026 15:35:10 -0400 Subject: [PATCH 06/26] chore(porch): 929 implement phase-transition --- codev/projects/929-support-codex-and-gemini-clis-/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index 0a478cab9..5881fd999 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -1,7 +1,7 @@ id: '929' title: support-codex-and-gemini-clis- protocol: pir -phase: plan +phase: implement plan_phases: [] current_plan_phase: null gates: @@ -17,4 +17,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-05-31T19:34:05.483Z' +updated_at: '2026-05-31T19:35:10.282Z' From 69cf20de22d65c05180770e6d723b5f159e4be57 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Sun, 31 May 2026 15:57:02 -0400 Subject: [PATCH 07/26] [PIR 929][Phase: implement] feat: harness-gated session resume for codex/gemini architects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes a latent crash-loop where a non-Claude architect/builder + a stale Claude .jsonl built an invalid ` --resume ` invocation and shellper restart-looped to death. Session resume is now gated on the configured HarnessProvider: - harness.ts: add optional buildResume (bundled discovery + Node-argv + shell-escaped script fragment) and getArchitectFiles. Only CLAUDE_HARNESS implements buildResume; GEMINI_HARNESS writes .gemini/settings.json → AGENTS.md so gemini launches with project context. - tower-instances.ts: architect resume gated on getArchitectHarness().buildResume, preserving the safeToResume sibling guard; getArchitectFiles written if-absent. - spawn.ts / spawn-worktree.ts: discoverResumeSession takes the builder harness and returns the bundled resume object; the launch script consumes the pre-escaped scriptFragment instead of re-deriving the flag. - doctor.ts / arch.md: affirm codex/gemini architect support; document that conversation resume is Claude-main-only and selection is config-driven. Tests: codex/gemini return no --resume with a stale jsonl (architect + builder); claude still resumes; gemini settings.json write-if-missing + no-clobber. Co-Authored-By: Claude Opus 4.7 --- codev/resources/arch.md | 8 +- codev/state/pir-929_thread.md | 14 ++ .../__tests__/discover-resume-session.test.ts | 45 +++++- .../__tests__/spawn-worktree.test.ts | 75 ++++++++- .../__tests__/tower-instances.test.ts | 152 ++++++++++++++++++ .../src/agent-farm/commands/spawn-worktree.ts | 25 +-- .../codev/src/agent-farm/commands/spawn.ts | 42 +++-- .../src/agent-farm/servers/tower-instances.ts | 41 +++-- .../codev/src/agent-farm/utils/harness.ts | 46 ++++++ packages/codev/src/commands/doctor.ts | 11 +- 10 files changed, 410 insertions(+), 49 deletions(-) diff --git a/codev/resources/arch.md b/codev/resources/arch.md index 755d29472..43d54c8ce 100644 --- a/codev/resources/arch.md +++ b/codev/resources/arch.md @@ -251,7 +251,7 @@ All architect sessions (at all 3 creation points) receive a role prompt injected 1. Loads the architect role from `codev/roles/architect.md` (local) or `skeleton/roles/architect.md` (bundled fallback) via `loadRolePrompt()` 2. Writes the role content to `.architect-role.md` in the project directory -3. Appends `--append-system-prompt ` to the architect command args +3. Delegates the CLI-specific injection to the configured `HarnessProvider` (`agent-farm/utils/harness.ts`, Spec 591): claude `--append-system-prompt`, codex `-c model_instructions_file=`, gemini `GEMINI_SYSTEM_MD` env var **Three architect creation points** where role injection is applied: - `tower-instances.ts` → `launchInstance()` (new project activation) @@ -267,6 +267,12 @@ Framework files (protocol docs, templates) live in the package skeleton (resolve A `codev doctor` audit (`lib/framework-ref-audit.ts`) flags shell-fetch of framework files in a project's local `codev/` overrides; the shipped skeleton is guarded by a CI unit test. +#### Supported Architect Harnesses & Conversation Resume (#929) + +**Supported architect harnesses** (Issue #929): claude, codex, and gemini are all supported as architects, selected via `.codev/config.json` (`shell.architect` / `shell.architectHarness`) — the same config-driven mechanism builders use. `.codev/config.json` (not `TOWER_ARCHITECT_CMD` / `--architect-cmd`) is the supported selection mechanism; an env/CLI command override with no matching harness config still resolves the default (claude) harness flags. OpenCode remains builder-only (file-based injection needs an ephemeral worktree). Gemini additionally gets a `.gemini/settings.json` (`context.fileName` → `AGENTS.md`) written if-absent at launch via the harness's `getArchitectFiles`, so it reads project context the way codex reads `AGENTS.md` natively. + +**Conversation resume is Claude-main-only.** `launchInstance` resumes a prior session only when the configured harness implements `HarnessProvider.buildResume` — currently just claude (its sessions live at `~/.claude/projects//*.jsonl`). Codex/gemini architects (and resumed codex/gemini builders, via `spawn.ts` `discoverResumeSession`) return `null` from `buildResume` and relaunch fresh with role injection. This gating fixes a latent crash-loop where a non-Claude harness + a stale Claude `.jsonl` built an invalid ` --resume ` invocation and shellper restart-looped to death. + #### Multi-Architect Support (Spec 755 / Spec 786) A workspace can host more than one architect terminal. Each architect has a stable name (`main` for the workspace's default; siblings via `afx workspace add-architect`). The primary use case is letting a sibling architect drive a focused workflow without monopolising `main`. diff --git a/codev/state/pir-929_thread.md b/codev/state/pir-929_thread.md index 6c221ad8f..02db140a8 100644 --- a/codev/state/pir-929_thread.md +++ b/codev/state/pir-929_thread.md @@ -28,3 +28,17 @@ Revised after a 2-way consult + architect verification: 5. **Builder override caveat documented**: `--builder-cmd`/env vs config-only `getBuilderHarness` is the exact analog of the architect `TOWER_ARCHITECT_CMD` trap — added to Risks alongside it. Re-signalled, awaiting architect re-review. + +### 2026-05-31 — Implement phase + +Plan approved. Implemented all six tasks: + +1. **harness.ts**: added `buildResume?` (bundled discovery + Node-argv + escaped script fragment) and `getArchitectFiles?` to `HarnessProvider`. `CLAUDE_HARNESS.buildResume` delegates to `findLatestSessionId`; `GEMINI_HARNESS.getArchitectFiles` writes `.gemini/settings.json` → AGENTS.md. codex/gemini/opencode leave `buildResume` undefined. +2. **tower-instances.ts**: architect resume now gated on `getArchitectHarness(...).buildResume?.()` (preserving the `safeToResume` sibling guard); writes `getArchitectFiles?()` if-missing on launch. +3. **spawn.ts / spawn-worktree.ts**: `discoverResumeSession` takes the builder harness and returns the bundled object; `startBuilderSession`'s `resumeSessionId?: string` → `resume?: { sessionId, scriptFragment }`, script emits `${baseCmd} ${resume.scriptFragment}`. +4. **doctor.ts / arch.md**: codex/gemini architect affirmation + "resume is Claude-main-only" docs. +5. **Tests**: `discover-resume-session.test.ts` (harness arg + codex/gemini null-return guards), `tower-instances.test.ts` (architect resume-skip for codex/gemini + stale jsonl; claude still resumes; gemini `.gemini/settings.json` write-if-missing + no-clobber), `spawn-worktree.test.ts` (resume script uses escaped fragment, no prompt/role injection; fresh path has no `--resume`). + +**Test-infra notes**: worktree had no `node_modules` — ran `pnpm install` + built `@cluesmith/codev-core` (vitest loads source which imports the core package's subpath exports). Two mock gaps surfaced and were fixed in the test files: `tower-instances.test.ts` db mock needed `getDb`/`closeDb` (state.getArchitects uses `getDb()`, not the already-mocked `getGlobalDb` — without it the `safeToResume` guard threw and silently skipped the claude resume); `spawn-worktree.test.ts` needed a `../lib/tower-client.js` mock (getTowerClient stub) so `startBuilderSession`→`createPtySession` runs without a live Tower. + +The three affected files: 143 passed. Full suite running. Build green. diff --git a/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts b/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts index c7c57bcaa..e44c47e72 100644 --- a/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts +++ b/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts @@ -1,7 +1,12 @@ /** * Tests for the discoverResumeSession helper — the spawn-CLI wrapper that - * gates findLatestSessionId on the --resume flag and surfaces a user-facing - * log line. (Issues #829 / #831.) + * gates the harness's buildResume on the --resume flag and surfaces a + * user-facing log line. (Issues #829 / #831 / #929.) + * + * Issue #929: resume is now gated on the builder harness, not the Claude + * session store directly. Only the Claude harness implements buildResume; + * codex/gemini return undefined even when a stale Claude jsonl exists (the + * regression guard against `codex --resume ` crash-loops). */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; @@ -11,6 +16,7 @@ import { join } from 'node:path'; import { discoverResumeSession } from '../commands/spawn.js'; import { encodeClaudeProjectDir } from '../utils/claude-session-discovery.js'; +import { CLAUDE_HARNESS, CODEX_HARNESS, GEMINI_HARNESS } from '../utils/harness.js'; // discoverResumeSession reads from $HOME via os.homedir() through // findLatestSessionId. Override the env var for the duration of the test so @@ -54,7 +60,7 @@ describe('discoverResumeSession', () => { const worktree = '/Users/x/repo/.builders/spir-1'; writeSession(projectsRoot, worktree, 'should-not-pick', 1_700_000_000_000); pinHome(fakeHome, () => { - expect(discoverResumeSession(worktree, false)).toBeUndefined(); + expect(discoverResumeSession(worktree, false, CLAUDE_HARNESS)).toBeUndefined(); }); }); @@ -62,23 +68,46 @@ describe('discoverResumeSession', () => { const worktree = '/Users/x/repo/.builders/spir-2'; writeSession(projectsRoot, worktree, 'should-not-pick', 1_700_000_000_000); pinHome(fakeHome, () => { - expect(discoverResumeSession(worktree, undefined)).toBeUndefined(); + expect(discoverResumeSession(worktree, undefined, CLAUDE_HARNESS)).toBeUndefined(); }); }); it('returns undefined when isResume is true but no jsonl exists', () => { const worktree = '/Users/x/repo/.builders/spir-3-no-jsonl'; pinHome(fakeHome, () => { - expect(discoverResumeSession(worktree, true)).toBeUndefined(); + expect(discoverResumeSession(worktree, true, CLAUDE_HARNESS)).toBeUndefined(); }); }); - it('returns the newest jsonl UUID when isResume is true and jsonls exist', () => { + it('returns the newest jsonl resume object (claude) when isResume is true and jsonls exist', () => { const worktree = '/Users/x/repo/.builders/pir-1661'; writeSession(projectsRoot, worktree, 'older-uuid', 1_500_000_000_000); writeSession(projectsRoot, worktree, 'newest-uuid', 1_700_000_000_000); pinHome(fakeHome, () => { - expect(discoverResumeSession(worktree, true)).toBe('newest-uuid'); + const resume = discoverResumeSession(worktree, true, CLAUDE_HARNESS); + expect(resume).toEqual({ + sessionId: 'newest-uuid', + args: ['--resume', 'newest-uuid'], + scriptFragment: "--resume 'newest-uuid'", + }); + }); + }); + + it('returns undefined for codex even when a stale Claude jsonl exists (regression guard)', () => { + // The crash-loop bug: a codex builder must NOT pick up a Claude session id + // and build `codex --resume `. CODEX_HARNESS has no buildResume. + const worktree = '/Users/x/repo/.builders/pir-codex'; + writeSession(projectsRoot, worktree, 'stale-claude-uuid', 1_700_000_000_000); + pinHome(fakeHome, () => { + expect(discoverResumeSession(worktree, true, CODEX_HARNESS)).toBeUndefined(); + }); + }); + + it('returns undefined for gemini even when a stale Claude jsonl exists (regression guard)', () => { + const worktree = '/Users/x/repo/.builders/pir-gemini'; + writeSession(projectsRoot, worktree, 'stale-claude-uuid', 1_700_000_000_000); + pinHome(fakeHome, () => { + expect(discoverResumeSession(worktree, true, GEMINI_HARNESS)).toBeUndefined(); }); }); @@ -86,7 +115,7 @@ describe('discoverResumeSession', () => { // Negative case: isResume=false short-circuits before any filesystem // access happens. Tests pass even if HOME points at /nonexistent. pinHome('/nonexistent-home-path', () => { - expect(discoverResumeSession('/some/worktree', false)).toBeUndefined(); + expect(discoverResumeSession('/some/worktree', false, CLAUDE_HARNESS)).toBeUndefined(); }); }); }); diff --git a/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts b/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts index d042ddc59..8d3809dec 100644 --- a/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts +++ b/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts @@ -7,7 +7,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { - slugify, buildWorktreeLaunchScript, + slugify, buildWorktreeLaunchScript, startBuilderSession, checkDependencies, createWorktree, createWorktreeFromBranch, validateBranchName, validateRemoteName, detectForkRemote, symlinkConfigFiles, @@ -17,6 +17,9 @@ import { validateResumeWorktree, initPorchInWorktree, type GitHubIssue, } from '../commands/spawn-worktree.js'; import { DEFAULT_TOWER_PORT } from '../lib/tower-client.js'; +// node:fs is mocked below; import writeFileSync so resume-script assertions can +// read its captured calls without a per-test dynamic import. +import { writeFileSync } from 'node:fs'; // Mock dependencies vi.mock('node:fs', async (importOriginal) => { @@ -78,6 +81,18 @@ vi.mock('glob', () => ({ globSync: (...args: unknown[]) => globSyncMock(...args), })); +// Mock the Tower REST client so startBuilderSession can run without a live +// Tower (createPtySession → getTowerClient().createTerminal). DEFAULT_TOWER_PORT +// is preserved from the real module (asserted elsewhere in this file). +const createTerminalMock = vi.fn().mockResolvedValue({ id: 'term-test' }); +vi.mock('../lib/tower-client.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getTowerClient: () => ({ createTerminal: createTerminalMock }), + }; +}); + describe('spawn-worktree', () => { beforeEach(() => { vi.clearAllMocks(); @@ -294,6 +309,64 @@ describe('spawn-worktree', () => { }); }); + // ========================================================================= + // startBuilderSession — resume script shape (Issue #929) + // + // The crash-loop lived in the builder restart-loop script: a resumed builder + // baked ` --resume ""` into .builder-start.sh. The fix threads a + // harness-provided, pre-escaped `scriptFragment` through instead of + // re-deriving the flag (which risked a comma-joined / word-split argv), and + // codex/gemini builders never reach this branch (resume === undefined → fresh + // role-injected script, no --resume). + // ========================================================================= + + describe('startBuilderSession resume script (Issue #929)', () => { + function findScript(): string | undefined { + const writeCalls = vi.mocked(writeFileSync).mock.calls; + const scriptCall = writeCalls.find( + call => typeof call[0] === 'string' && call[0].endsWith('.builder-start.sh'), + ); + return scriptCall ? (scriptCall[1] as string) : undefined; + } + + it('resume → script uses the escaped scriptFragment, no prompt/role injection', async () => { + const resume = { sessionId: 'abc-1234-uuid', scriptFragment: "--resume 'abc-1234-uuid'" }; + await startBuilderSession( + { workspaceRoot: '/tmp/ws' } as any, + 'pir-1', '/tmp/worktree', 'claude', + 'PROMPT', 'ROLE', 'codev', resume, + ); + + const script = findScript(); + expect(script).toBeDefined(); + // Exact escaped fragment appended verbatim — not an unquoted or + // comma-joined argv form. + expect(script).toContain("claude --resume 'abc-1234-uuid'"); + expect(script).not.toContain('--resume,'); + expect(script).toContain('while true'); + // Resume path skips role injection and the prompt file. + expect(script).not.toContain('--append-system-prompt'); + const writeCalls = vi.mocked(writeFileSync).mock.calls; + const promptCall = writeCalls.find( + call => typeof call[0] === 'string' && call[0].endsWith('.builder-prompt.txt'), + ); + expect(promptCall).toBeUndefined(); + }); + + it('no resume + role → fresh role-injected script, no --resume', async () => { + await startBuilderSession( + { workspaceRoot: '/tmp/ws' } as any, + 'pir-2', '/tmp/worktree', 'claude', + 'PROMPT', 'ROLE {PORT}', 'codev', + ); + + const script = findScript(); + expect(script).toBeDefined(); + expect(script).not.toContain('--resume'); + expect(script).toContain('--append-system-prompt'); + }); + }); + // ========================================================================= // Collision Detection (unit-level) // ========================================================================= diff --git a/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts b/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts index 50b852a89..b5ba47766 100644 --- a/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts +++ b/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts @@ -24,6 +24,7 @@ import { waitForTerminalExit, type InstanceDeps, } from '../servers/tower-instances.js'; +import { encodeClaudeProjectDir } from '../utils/claude-session-discovery.js'; // --------------------------------------------------------------------------- // Mocks (vi.hoisted ensures these exist before vi.mock factories run) @@ -48,6 +49,11 @@ const { vi.mock('../db/index.js', () => ({ getGlobalDb: () => ({ prepare: mockDbPrepare }), + // state.getArchitects() (used by launchInstance's safeToResume guard) calls + // getDb(), not getGlobalDb — mock it too so the guard doesn't throw and + // default to skip-resume. (Issue #929 resume regression tests depend on it.) + getDb: () => ({ prepare: mockDbPrepare }), + closeDb: () => {}, })); vi.mock('../servers/tower-utils.js', async () => { @@ -593,6 +599,152 @@ describe('tower-instances', () => { }); }); + // ========================================================================= + // Issue #929 — architect resume is gated on the configured harness + // + // Regression guard for the crash-loop: a codex/gemini architect with a stale + // Claude jsonl in ~/.claude/projects// must NOT launch + // ` --resume `. Only the Claude harness implements + // buildResume, so codex/gemini relaunch fresh (role-injected) instead. + // ========================================================================= + + describe('Issue #929 — architect resume gated on harness', () => { + function writeStaleClaudeSession(fakeHome: string, cwd: string, uuid: string): void { + const dir = path.join(fakeHome, '.claude', 'projects', encodeClaudeProjectDir(cwd)); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, `${uuid}.jsonl`), `{"sessionId":"${uuid}"}\n`); + } + + function makeCapturingDeps(): { deps: InstanceDeps; createSession: ReturnType } { + const createSession = vi.fn().mockResolvedValue({ id: 'test-session', pid: 1234 }); + const deps = makeDeps({ + getTerminalManager: vi.fn().mockReturnValue({ + getSession: vi.fn(), + killSession: vi.fn(), + createSession, + createSessionRaw: vi.fn(), + listSessions: vi.fn().mockReturnValue([]), + }) as any, + }); + return { deps, createSession }; + } + + // Pin HOME so the stale-jsonl lookup (os.homedir() inside + // findLatestSessionId) reads our fake store, not the real user's. + function withSetup( + configJson: Record | null, + fn: (tmpDir: string, fakeHome: string, uuid: string) => Promise, + ): () => Promise { + return async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tower-929-')); + const fakeHome = fs.mkdtempSync(path.join(os.tmpdir(), 'tower-929-home-')); + const originalHome = process.env.HOME; + const uuid = 'stale-claude-uuid-1234'; + try { + fs.mkdirSync(path.join(tmpDir, 'codev')); + if (configJson) { + fs.mkdirSync(path.join(tmpDir, '.codev'), { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, '.codev', 'config.json'), + JSON.stringify(configJson), + ); + } + writeStaleClaudeSession(fakeHome, tmpDir, uuid); + process.env.HOME = fakeHome; + await fn(tmpDir, fakeHome, uuid); + } finally { + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + fs.rmSync(tmpDir, { recursive: true, force: true }); + fs.rmSync(fakeHome, { recursive: true, force: true }); + } + }; + } + + it('codex architect + stale Claude jsonl → launches fresh, no --resume', withSetup( + { shell: { architect: 'codex' } }, + async (tmpDir, _fakeHome, uuid) => { + const { deps, createSession } = makeCapturingDeps(); + initInstances(deps); + + const result = await launchInstance(tmpDir); + expect(result.success).toBe(true); + expect(createSession).toHaveBeenCalled(); + + const callStr = JSON.stringify(createSession.mock.calls[0]); + expect(callStr).not.toContain('--resume'); + expect(callStr).not.toContain(uuid); + }, + )); + + it('gemini architect + stale Claude jsonl → launches fresh, no --resume', withSetup( + { shell: { architect: 'gemini' } }, + async (tmpDir, _fakeHome, uuid) => { + const { deps, createSession } = makeCapturingDeps(); + initInstances(deps); + + const result = await launchInstance(tmpDir); + expect(result.success).toBe(true); + expect(createSession).toHaveBeenCalled(); + + const callStr = JSON.stringify(createSession.mock.calls[0]); + expect(callStr).not.toContain('--resume'); + expect(callStr).not.toContain(uuid); + }, + )); + + it('claude architect (default) + stale Claude jsonl → resumes with --resume ', withSetup( + null, + async (tmpDir, _fakeHome, uuid) => { + const { deps, createSession } = makeCapturingDeps(); + initInstances(deps); + + const result = await launchInstance(tmpDir); + expect(result.success).toBe(true); + expect(createSession).toHaveBeenCalled(); + + const callStr = JSON.stringify(createSession.mock.calls[0]); + expect(callStr).toContain('--resume'); + expect(callStr).toContain(uuid); + }, + )); + + it('gemini architect → writes .gemini/settings.json pointing at AGENTS.md', withSetup( + { shell: { architect: 'gemini' } }, + async (tmpDir) => { + const { deps } = makeCapturingDeps(); + initInstances(deps); + + const result = await launchInstance(tmpDir); + expect(result.success).toBe(true); + + const settingsPath = path.join(tmpDir, '.gemini', 'settings.json'); + expect(fs.existsSync(settingsPath)).toBe(true); + const parsed = JSON.parse(fs.readFileSync(settingsPath, 'utf-8')); + expect(parsed.context.fileName).toBe('AGENTS.md'); + }, + )); + + it('gemini architect → does not clobber a pre-existing .gemini/settings.json', withSetup( + { shell: { architect: 'gemini' } }, + async (tmpDir) => { + const geminiDir = path.join(tmpDir, '.gemini'); + fs.mkdirSync(geminiDir, { recursive: true }); + const settingsPath = path.join(geminiDir, 'settings.json'); + const userContent = JSON.stringify({ context: { fileName: 'MY_OWN.md' } }); + fs.writeFileSync(settingsPath, userContent); + + const { deps } = makeCapturingDeps(); + initInstances(deps); + + const result = await launchInstance(tmpDir); + expect(result.success).toBe(true); + + expect(fs.readFileSync(settingsPath, 'utf-8')).toBe(userContent); + }, + )); + }); + // ========================================================================= // killTerminalWithShellper // ========================================================================= diff --git a/packages/codev/src/agent-farm/commands/spawn-worktree.ts b/packages/codev/src/agent-farm/commands/spawn-worktree.ts index a0eb5e91f..afe3a24c1 100644 --- a/packages/codev/src/agent-farm/commands/spawn-worktree.ts +++ b/packages/codev/src/agent-farm/commands/spawn-worktree.ts @@ -715,11 +715,13 @@ function writeWorktreeFiles( /** * Start a terminal session for a builder. * - * When `resumeSessionId` is provided, the launch script invokes - * `claude --resume ` instead of a fresh prompt+role invocation. The - * saved Claude conversation contains the system prompt / role context - * already, so role injection and the initial prompt are intentionally - * skipped on that path. + * When `resume` is provided, the launch script invokes the harness's resume + * form (e.g. `claude --resume `) via the pre-escaped `scriptFragment` + * instead of a fresh prompt+role invocation. The saved conversation contains + * the system prompt / role context already, so role injection and the initial + * prompt are intentionally skipped on that path. Only the Claude harness + * produces a resume object (Issue #929); codex/gemini pass `undefined` here + * and take the fresh role-injection path. */ export async function startBuilderSession( config: Config, @@ -729,21 +731,22 @@ export async function startBuilderSession( prompt: string, roleContent: string | null, roleSource: string | null, - resumeSessionId?: string, + resume?: { sessionId: string; scriptFragment: string }, ): Promise<{ terminalId: string }> { logger.info('Creating terminal session...'); const scriptPath = resolve(worktreePath, '.builder-start.sh'); let scriptContent: string; - if (resumeSessionId) { - // Resume path: load the prior Claude conversation by UUID. No prompt file, - // no role injection — both are already part of the saved conversation. - logger.info(`Resuming Claude session ${resumeSessionId.slice(0, 8)}…`); + if (resume) { + // Resume path: load the prior conversation via the harness-provided, + // shell-escaped resume fragment. No prompt file, no role injection — both + // are already part of the saved conversation. + logger.info(`Resuming session ${resume.sessionId.slice(0, 8)}…`); scriptContent = `#!/bin/bash cd "${worktreePath}" while true; do - ${baseCmd} --resume "${resumeSessionId}" + ${baseCmd} ${resume.scriptFragment} echo "" echo "Agent exited. Restarting in 2 seconds... (Ctrl+C to quit)" sleep 2 diff --git a/packages/codev/src/agent-farm/commands/spawn.ts b/packages/codev/src/agent-farm/commands/spawn.ts index 3f1f86957..2ea2a6bc9 100644 --- a/packages/codev/src/agent-farm/commands/spawn.ts +++ b/packages/codev/src/agent-farm/commands/spawn.ts @@ -17,7 +17,8 @@ import { resolve, basename } from 'node:path'; import { existsSync, writeFileSync, readdirSync } from 'node:fs'; import type { SpawnOptions, BuilderType, Config } from '../types.js'; -import { getConfig, ensureDirectories, getResolvedCommands } from '../utils/index.js'; +import { getConfig, ensureDirectories, getResolvedCommands, getBuilderHarness } from '../utils/index.js'; +import type { HarnessProvider } from '../utils/harness.js'; import { logger, fatal } from '../utils/logger.js'; import { run } from '../utils/shell.js'; import { hasUncommittedTrackedChanges } from '../utils/git.js'; @@ -37,7 +38,6 @@ const SPAWNING_ARCHITECT_NAME = (process.env.CODEV_ARCHITECT_NAME && process.env.CODEV_ARCHITECT_NAME.trim()) || DEFAULT_ARCHITECT_NAME; import { loadRolePrompt } from '../utils/roles.js'; import { buildAgentName, stripLeadingZeros } from '../utils/agent-names.js'; -import { findLatestSessionId } from '../utils/claude-session-discovery.js'; import { fetchIssue as fetchIssueNonFatal } from '../../lib/github.js'; import { type TemplateContext, @@ -74,20 +74,28 @@ import { executeForgeCommand, loadForgeConfig } from '../../lib/forge.js'; // ============================================================================= /** - * On --resume, look up the prior Claude conversation jsonl for the worktree - * so the revived builder can pick up the saved conversation via - * `claude --resume ` instead of starting fresh with a resume-notice - * prompt. (Issue #831.) Returns undefined when not resuming or when no - * jsonl exists; callers fall back to the fresh-spawn path in that case. + * On --resume, ask the builder's harness for a resumable prior session for the + * worktree so the revived builder can pick up the saved conversation (e.g. + * `claude --resume `) instead of starting fresh with a resume-notice + * prompt. (Issues #831 / #929.) Returns undefined when not resuming, when the + * harness has no resumable session, or when the harness doesn't support resume + * (codex/gemini → buildResume undefined); callers fall back to the fresh-spawn + * path in that case. Returning the harness's bundled resume object (with the + * shell-escaped `scriptFragment`) keeps the resume flag-shape owned by the + * provider rather than the bash-script generator. */ -export function discoverResumeSession(worktreePath: string, isResume: boolean | undefined): string | undefined { +export function discoverResumeSession( + worktreePath: string, + isResume: boolean | undefined, + harness: HarnessProvider, +): { sessionId: string; args: string[]; scriptFragment: string } | undefined { if (!isResume) return undefined; - const found = findLatestSessionId(worktreePath); - if (found) { - logger.kv('Claude session', `${found.slice(0, 8)}… (resuming conversation)`); - return found; + const resume = harness.buildResume?.(worktreePath) ?? null; + if (resume) { + logger.kv('Session', `${resume.sessionId.slice(0, 8)}… (resuming conversation)`); + return resume; } - logger.info('No prior Claude conversation found for this worktree; starting a fresh session.'); + logger.info('No prior conversation found for this worktree; starting a fresh session.'); return undefined; } @@ -456,7 +464,7 @@ async function spawnSpec(options: SpawnOptions, config: Config): Promise { templateContext.existing_branch = options.branch; } - const resumeSessionId = discoverResumeSession(worktreePath, options.resume); + const resume = discoverResumeSession(worktreePath, options.resume, getBuilderHarness(config.workspaceRoot)); const initialPrompt = buildPromptFromTemplate(config, protocol, templateContext); const resumeNotice = options.resume ? `\n${buildResumeNotice(projectId)}\n` : ''; @@ -470,7 +478,7 @@ async function spawnSpec(options: SpawnOptions, config: Config): Promise { const { terminalId } = await startBuilderSession( config, builderId, worktreePath, commands.builder, builderPrompt, role?.content ?? null, role?.source ?? null, - resumeSessionId, + resume, ); upsertBuilder({ @@ -835,13 +843,13 @@ async function spawnIssueDrivenBuilder( : ''; const builderPrompt = `You are a Builder. Read codev/roles/builder.md for your full role definition.\n${resumeNotice}${branchNotice}\n${prompt}`; - const resumeSessionId = discoverResumeSession(worktreePath, options.resume); + const resume = discoverResumeSession(worktreePath, options.resume, getBuilderHarness(config.workspaceRoot)); const role = options.noRole ? null : loadRolePrompt(config, 'builder'); const commands = getResolvedCommands(); const { terminalId } = await startBuilderSession( config, builderId, worktreePath, commands.builder, builderPrompt, role?.content ?? null, role?.source ?? null, - resumeSessionId, + resume, ); upsertBuilder({ diff --git a/packages/codev/src/agent-farm/servers/tower-instances.ts b/packages/codev/src/agent-farm/servers/tower-instances.ts index b6dab2837..9cc234d5b 100644 --- a/packages/codev/src/agent-farm/servers/tower-instances.ts +++ b/packages/codev/src/agent-farm/servers/tower-instances.ts @@ -13,7 +13,7 @@ import { exec } from 'node:child_process'; import { promisify } from 'node:util'; import { homedir } from 'node:os'; import { encodeWorkspacePath } from '../lib/tower-client.js'; -import { findLatestSessionId } from '../utils/claude-session-discovery.js'; +import { getArchitectHarness } from '../utils/config.js'; import { loadConfig } from '../../lib/config.js'; const execAsync = promisify(exec); @@ -466,10 +466,18 @@ export async function launchInstance(workspacePath: string): Promise<{ success: const cmdParts = architectCmd.split(/\s+/); const cmd = cmdParts[0]; - // Issue #830 (main architect only): if a prior Claude session exists - // for this workspace cwd, resume it instead of starting fresh. Role - // injection is skipped on the resume path — the saved conversation - // already contains the role/system prompt. + // Issue #830 (main architect only): if the configured harness exposes a + // resumable prior session for this workspace cwd, resume it instead of + // starting fresh. Role injection is skipped on the resume path — the + // saved conversation already contains the role/system prompt. + // + // Issue #929: resume is gated on the harness (via buildResume), not the + // Claude session store directly. Only the Claude harness implements + // buildResume (its sessions live at ~/.claude/projects//*.jsonl); + // codex/gemini return null → fresh, role-injected launch. Previously + // this read findLatestSessionId() unconditionally, so a codex/gemini + // architect with any stale Claude jsonl built `codex --resume ` + // and shellper restart-looped to death. // // Lookup is unconditional here (unlike builders, where spawn.ts gates // discovery behind `options.resume`). The asymmetry is intentional: @@ -505,16 +513,31 @@ export async function launchInstance(workspacePath: string): Promise<{ success: // an unrelated jsonl. safeToResume = false; } - const resumeSessionId = safeToResume ? findLatestSessionId(workspacePath) : null; + const architectHarness = getArchitectHarness(workspacePath); + const resume = safeToResume ? (architectHarness.buildResume?.(workspacePath) ?? null) : null; if (!safeToResume) { _deps.log('WARN', `Skipping main architect conversation resume for ${workspacePath}: persisted sibling architects detected (or state.db unreadable); cannot disambiguate jsonl by cwd. See #832.`); } + + // Write any harness-specific architect-context files (e.g., Gemini's + // .gemini/settings.json → AGENTS.md), only if absent — never clobber a + // user's existing file. (Issue #929.) + if (architectHarness.getArchitectFiles) { + for (const file of architectHarness.getArchitectFiles(workspacePath)) { + const targetPath = path.join(workspacePath, file.relativePath); + if (!fs.existsSync(targetPath)) { + fs.mkdirSync(path.dirname(targetPath), { recursive: true }); + fs.writeFileSync(targetPath, file.content); + } + } + } + let cmdArgs: string[]; let harnessEnv: Record; - if (resumeSessionId) { - cmdArgs = [...cmdParts.slice(1), '--resume', resumeSessionId]; + if (resume) { + cmdArgs = [...cmdParts.slice(1), ...resume.args]; harnessEnv = {}; - _deps.log('INFO', `Resuming main architect Claude session ${resumeSessionId.slice(0, 8)}… for ${workspacePath}`); + _deps.log('INFO', `Resuming main architect session ${resume.sessionId.slice(0, 8)}… for ${workspacePath}`); } else { const built = buildArchitectArgs(cmdParts.slice(1), workspacePath); cmdArgs = built.args; diff --git a/packages/codev/src/agent-farm/utils/harness.ts b/packages/codev/src/agent-farm/utils/harness.ts index bfb7f3b67..d883334df 100644 --- a/packages/codev/src/agent-farm/utils/harness.ts +++ b/packages/codev/src/agent-farm/utils/harness.ts @@ -12,6 +12,8 @@ * @see codev/specs/591-af-workspace-failure-with-code.md */ +import { findLatestSessionId } from './claude-session-discovery.js'; + // ============================================================================= // Types // ============================================================================= @@ -45,6 +47,34 @@ export interface HarnessProvider { relativePath: string; content: string; }>; + + /** + * Optional: discover a resumable prior session for the given working dir and + * return how to resume it — in BOTH forms, mirroring buildRoleInjection / + * buildScriptRoleInjection: + * - args: Node argv for spawn() call sites (architect launch) + * - scriptFragment: shell-escaped fragment for bash script generation (builder) + * Returns null when no resumable session exists or this harness has no + * cwd-keyed session store → callers fall back to a fresh launch. Only Claude + * implements it (store: ~/.claude/projects//.jsonl). + */ + buildResume?(absolutePath: string, opts?: { homeDir?: string }): { + sessionId: string; + args: string[]; + scriptFragment: string; + } | null; + + /** + * Optional: files to write in the workspace (architect cwd) before launching + * the architect, written only if absent so a user's existing file is never + * clobbered. Used by harnesses that need a project-context manifest the role + * injection alone doesn't provide (e.g., Gemini reads .gemini/settings.json's + * context.fileName to locate AGENTS.md). + */ + getArchitectFiles?(workspacePath: string): Array<{ + relativePath: string; + content: string; + }>; } /** Custom harness definition from .codev/config.json */ @@ -68,6 +98,15 @@ export const CLAUDE_HARNESS: HarnessProvider = { fragment: `--append-system-prompt "$(cat '${shellEscapeSingleQuote(filePath)}')"`, env: {}, }), + buildResume: (absolutePath, opts) => { + const sessionId = findLatestSessionId(absolutePath, opts); + if (!sessionId) return null; + return { + sessionId, + args: ['--resume', sessionId], + scriptFragment: `--resume '${shellEscapeSingleQuote(sessionId)}'`, + }; + }, }; export const CODEX_HARNESS: HarnessProvider = { @@ -90,6 +129,13 @@ export const GEMINI_HARNESS: HarnessProvider = { fragment: '', env: { GEMINI_SYSTEM_MD: filePath }, }), + // Gemini reads project context from .gemini/settings.json's context.fileName. + // Codex reads AGENTS.md natively; point Gemini at the same manifest so it + // launches with project context, not just the injected role. + getArchitectFiles: () => ([{ + relativePath: '.gemini/settings.json', + content: JSON.stringify({ context: { fileName: 'AGENTS.md' } }, null, 2) + '\n', + }]), }; export const OPENCODE_HARNESS: HarnessProvider = { diff --git a/packages/codev/src/commands/doctor.ts b/packages/codev/src/commands/doctor.ts index 8e3c434c5..c581e8590 100644 --- a/packages/codev/src/commands/doctor.ts +++ b/packages/codev/src/commands/doctor.ts @@ -682,8 +682,9 @@ export async function doctor(): Promise { const architectCmd = Array.isArray(shell?.architect) ? (shell.architect as string[]).join(' ') : (shell?.architect as string ?? ''); - const isOpencode = architectHarness === 'opencode' || - (architectCmd && detectHarnessFromCommand(architectCmd) === 'opencode'); + const resolvedHarness = architectHarness || + (architectCmd ? detectHarnessFromCommand(architectCmd) : undefined); + const isOpencode = resolvedHarness === 'opencode'; if (isOpencode) { console.log(''); console.log(chalk.yellow(' ⚠') + ' OpenCode is configured as architect shell — this is unsupported.'); @@ -695,6 +696,12 @@ export async function doctor(): Promise { issue: 'OpenCode configured as architect shell (unsupported)', recommendation: 'Set shell.architect to "claude --dangerously-skip-permissions" in .codev/config.json', }); + } else if (resolvedHarness === 'codex' || resolvedHarness === 'gemini') { + // Issue #929: codex/gemini are supported architects (config-driven). + console.log(''); + console.log(chalk.green(' ✓') + ` ${resolvedHarness} is configured as architect shell — supported.`); + console.log(chalk.gray(' ') + 'Conversation resume is Claude-main-only; codex/gemini architects relaunch fresh with role injection.'); + console.log(chalk.gray(' ') + 'Select the architect harness via .codev/config.json (shell.architect / shell.architectHarness).'); } } } catch { From 24bed9d1b1aed0f02362596357a226ce011ac0f7 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 14:58:35 -0400 Subject: [PATCH 08/26] chore(porch): 929 dev-approval gate-requested --- codev/projects/929-support-codex-and-gemini-clis-/status.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index 5881fd999..c1d062ac9 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -11,10 +11,11 @@ gates: approved_at: '2026-05-31T19:34:05.482Z' dev-approval: status: pending + requested_at: '2026-06-16T18:58:35.321Z' pr: status: pending iteration: 1 build_complete: false history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-05-31T19:35:10.282Z' +updated_at: '2026-06-16T18:58:35.321Z' From 4c8d84642c0b32f35955beb9776fdc6ad8754d54 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 15:03:40 -0400 Subject: [PATCH 09/26] chore(porch): 929 dev-approval gate-approved --- .../projects/929-support-codex-and-gemini-clis-/status.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index c1d062ac9..68cec0fd3 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -10,12 +10,13 @@ gates: requested_at: '2026-05-31T19:10:01.535Z' approved_at: '2026-05-31T19:34:05.482Z' dev-approval: - status: pending + status: approved requested_at: '2026-06-16T18:58:35.321Z' + approved_at: '2026-06-16T19:03:40.955Z' pr: status: pending iteration: 1 build_complete: false history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-06-16T18:58:35.321Z' +updated_at: '2026-06-16T19:03:40.955Z' From c7961b02be3bd7b2145a7e6e01b1964128df9318 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 15:03:52 -0400 Subject: [PATCH 10/26] chore(porch): 929 review phase-transition --- codev/projects/929-support-codex-and-gemini-clis-/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index 68cec0fd3..38b812964 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -1,7 +1,7 @@ id: '929' title: support-codex-and-gemini-clis- protocol: pir -phase: implement +phase: review plan_phases: [] current_plan_phase: null gates: @@ -19,4 +19,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-06-16T19:03:40.955Z' +updated_at: '2026-06-16T19:03:52.496Z' From 562637efd47be620aed87bafbe66b104ca1a3c6b Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 15:10:31 -0400 Subject: [PATCH 11/26] [PIR #929] Review + retrospective Co-Authored-By: Claude Opus 4.7 --- codev/resources/lessons-learned.md | 1 + .../929-support-codex-and-gemini-clis-.md | 64 +++++++++++++++++++ codev/state/pir-929_thread.md | 6 ++ 3 files changed, 71 insertions(+) create mode 100644 codev/reviews/929-support-codex-and-gemini-clis-.md diff --git a/codev/resources/lessons-learned.md b/codev/resources/lessons-learned.md index f5cde1c11..fcc982f58 100644 --- a/codev/resources/lessons-learned.md +++ b/codev/resources/lessons-learned.md @@ -66,6 +66,7 @@ Generalizable wisdom extracted from review documents, ordered by impact. Updated ## Architecture +- [From #929] When abstracting per-CLI (or per-provider) behavior behind a provider interface, audit **every** call site that builds a CLI invocation — including resume/restart paths, not just the obvious fresh-launch path. Spec 591's harness abstraction routed fresh-launch role injection through the provider but left the `--resume` seam reading Claude's session store unconditionally; a codex/gemini architect (or resumed builder) with any stale Claude `.jsonl` then built ` --resume ` and shellper restart-looped to death. The bug only fires on the resume branch (fresh launches were already correct), which is exactly why "builders already prove the path" didn't cover it — resume is a *different* call site. Fix: an optional capability (`buildResume`) that returns the resume invocation in both Node-argv and shell-escaped-fragment forms (mirroring `buildRoleInjection`/`buildScriptRoleInjection`), so no CLI-specific string survives at the call site and only the harness that has a cwd-keyed session store opts in (others return null → fresh launch). - [From #907] A workspace package consumed as *source* by one toolchain and as *built output* by another resolves through different `exports` conditions: tsc and vite read `exports.types` (`./src/index.ts`), while esbuild (the VS Code extension bundler) reads the runtime `exports.default` (`./dist/index.js`). `@cluesmith/codev-types` had no `dist` in a fresh worktree, so `tsc --noEmit`, the codev build, and the full test suite all passed while only the extension's esbuild bundle failed with `Could not resolve "@cluesmith/codev-types"`. A green type-check/test run does **not** prove every bundler can resolve a package — if a workspace package ships a `default → ./dist` condition, it must be built before any esbuild consumer. Root `pnpm build` now builds `types` first (types → core → codev). - [From #907] Derived projection fields need a fallback when their source is transiently unreadable. `OverviewBuilder.area` is re-resolved every refresh from the open-only `gh issue list`; once an issue closes (merge), is torn down mid-cleanup, or the fetch fails, the lookup misses and the field snapped back to its `Uncategorized` default while the builder was still listed — a visible mis-render once #818 made `area` control tree placement. Fix: cache the last *resolved* value (`ResolvedEnrichmentCache`), gated on **source reachability, not value emptiness**, so a reachable-but-unlabeled issue still records a genuine `Uncategorized` and a stale entry can't mask a live label change. The value is a stable per-item fact, so this is a cache bridging a read gap, not history of a changing value — which is why it lives on the process-lifetime cache singleton, not the per-refresh builder DTO. - [From #961] A browser `WebSocket` cannot see the HTTP status of a **failed upgrade** — it gets `onerror` + `onclose` with code `1006` and nothing else (browsers hide the response by design). So a "session-unknown fast-path" that works in a Node `ws` client (which surfaces `"Unexpected server response: 404"` as `Error.message`) **cannot** be ported to the dashboard terminal as-is. Tower 404s unknown sessions at the upgrade stage, so the web terminal stays on blind retry until Tower instead accepts-then-closes with a browser-visible app-range close code (e.g. `4404`). Don't wire a classifier against `CloseEvent.code` expecting HTTP-4xx semantics — WS close codes are a different number space. diff --git a/codev/reviews/929-support-codex-and-gemini-clis-.md b/codev/reviews/929-support-codex-and-gemini-clis-.md new file mode 100644 index 000000000..4978cb95a --- /dev/null +++ b/codev/reviews/929-support-codex-and-gemini-clis-.md @@ -0,0 +1,64 @@ +# PIR Review: Support `codex` and `gemini` CLIs as architects + +Fixes #929 + +## Summary + +Brings the OpenAI `codex` and Google `gemini` CLIs to parity with `claude` as Codev **architects**, selectable via `.codev/config.json` (`shell.architect` / `shell.architectHarness`). The core fix routes session-discovery + `--resume` argument construction behind a new optional `HarnessProvider.buildResume` capability, eliminating a latent crash-loop where a non-Claude architect (or resumed builder) with any stale Claude `.jsonl` built an invalid ` --resume ` invocation and shellper restart-looped to death. Gemini additionally gets a write-if-absent `.gemini/settings.json` so it launches with project context (`AGENTS.md`), and `doctor` now affirms codex/gemini architect support. + +## Files Changed + +- `packages/codev/src/agent-farm/utils/harness.ts` (+46 / -0) — `buildResume?` + `getArchitectFiles?` on the interface; `CLAUDE_HARNESS.buildResume` (delegates to `findLatestSessionId`); `GEMINI_HARNESS.getArchitectFiles` (`.gemini/settings.json`) +- `packages/codev/src/agent-farm/servers/tower-instances.ts` (+34 / -7) — architect resume gated on `getArchitectHarness(...).buildResume?.()`; writes `getArchitectFiles?()` if-missing on launch +- `packages/codev/src/agent-farm/commands/spawn.ts` (+25 / -17) — `discoverResumeSession` takes the builder harness, returns the bundled resume object; both call sites pass `getBuilderHarness(...)` +- `packages/codev/src/agent-farm/commands/spawn-worktree.ts` (+14 / -11) — `startBuilderSession`'s `resumeSessionId?: string` → `resume?: { sessionId, scriptFragment }`; script emits the pre-escaped fragment +- `packages/codev/src/commands/doctor.ts` (+9 / -2) — affirm codex/gemini architect support; single resolved-harness check +- `packages/codev/src/agent-farm/__tests__/tower-instances.test.ts` (+152 / -0) — architect resume-skip regression guard + gemini `getArchitectFiles` write-if-missing/no-clobber +- `packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts` (+74 / -1) — builder resume script uses escaped `scriptFragment`; codex/gemini → fresh script +- `packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts` (+44 / -1) — harness-arg threading; codex/gemini null-return + claude bundled-object cases +- `codev/resources/arch.md` (+7 / -1) — supported-architect-harnesses + Claude-only-resume documentation +- `codev/plans/929-support-codex-and-gemini-clis-.md`, `codev/state/pir-929_thread.md`, `codev/projects/929-*/status.yaml` — protocol artifacts + +## Commits + +- `69cf20de` [PIR 929][Phase: implement] feat: harness-gated session resume for codex/gemini architects +- `53374f30` [PIR #929] Plan revised — address architect feedback (5 issues) +- `fdddc7e2` [PIR #929] Plan draft + +## Test Results + +- `pnpm build`: ✓ pass (clean TS types for the new optional methods) +- `pnpm vitest run` (3 affected files): ✓ pass (146 tests) +- Manual verification: empirical codex/gemini lifecycle validation (clean + stale-jsonl launch, add-architect, `afx send` multiline/interrupt/streaming, reconnect, affinity, builder `--resume`, dashboard scrollback) was exercised by the human at the `dev-approval` gate against the running worktree — the reason PIR was chosen over AIR/BUGFIX. + +## Architecture Updates + +**COLD (`codev/resources/arch.md`)** — updated in the implementation commit. Added a "Supported Architect Harnesses & Conversation Resume (#929)" subsection documenting: (1) claude/codex/gemini are all supported architects selected via `.codev/config.json` (not `TOWER_ARCHITECT_CMD`/`--architect-cmd`); (2) gemini's `.gemini/settings.json` → `AGENTS.md` context manifest; (3) conversation resume is Claude-main-only via `HarnessProvider.buildResume`, and the crash-loop it fixes. Also updated the role-injection step to point at the `HarnessProvider` per-CLI flags rather than the claude-only `--append-system-prompt`. + +No **HOT** (`arch-critical.md`) change: the harness abstraction and its provider-method-extension pattern are already implied by the existing "Forge concept commands abstract the VCS provider — add a dedicated concept" entry's spirit; this PR extends an existing abstraction (Spec 591) rather than introducing a new always-on invariant, so a cold-tier reference detail is the correct routing. + +## Lessons Learned Updates + +**COLD (`codev/resources/lessons-learned.md`, Architecture section)** — added one lesson: when abstracting per-CLI behavior behind a provider, every call site that builds a CLI invocation must route through the provider — including resume/restart paths, not just the obvious fresh-launch path. The resume seam was the one path Spec 591 left harness-blind, and it only crash-loops on the `--resume` branch (fresh launches were already correct), which is why "builders already prove the path" didn't cover it. + +No **HOT** (`lessons-critical.md`) change: the existing "Single source of truth beats distributed state" and "Model permissions as roles/capabilities, not booleans" hot entries already carry the general displace-when-full discipline; this is a spec-narrow recipe (audit *all* invocation seams when extending a provider) better suited to the cold archive. + +## Things to Look At During PR Review + +- **The `buildResume` bundling decision** (`harness.ts`): one method returns both the Node-argv `args` (for the `spawn()` architect site) and a shell-escaped `scriptFragment` (for the builder bash generator), mirroring `buildRoleInjection`/`buildScriptRoleInjection`. This deliberately avoids a second independently-optional method (which would force a `!` non-null assertion) and avoids `.join(' ')`-ing a raw argv into bash (word-split/quoting bug). Session ids are bare UUIDs today, so the escaping is belt-and-suspenders — kept for correct-by-construction consistency with the existing script-injection methods. +- **The `safeToResume` interaction** (`tower-instances.ts`): the new harness gate composes with the pre-existing sibling-collision guard (`safeToResume`, #832) — resume happens only when *both* the harness implements `buildResume` *and* no persisted siblings exist. Confirm the ordering reads correctly. +- **`getArchitectFiles` write-if-absent** (`tower-instances.ts`): writes `.gemini/settings.json` only when the target path doesn't exist, so a user's existing file is never clobbered. Test covers both the write and the no-clobber path. +- **Two documented out-of-scope override caveats** (plan Risks): `TOWER_ARCHITECT_CMD`/`--architect-cmd` (and the `--builder-cmd` analog) set a non-Claude command without a matching `.codev/config.json` harness still resolve the claude harness → would still attempt resume. These are the issue's explicit *nice-to-have* ("command-aware harness resolution"); MVP fixes the config-driven path (all acceptance criteria target it) and documents the override caveat rather than expanding scope. + +## How to Test Locally + +For reviewers pulling the branch: + +- **View diff**: VSCode sidebar → right-click builder `pir-929` → **Review Diff** +- **Run dev server**: `afx dev pir-929` +- **What to verify** (needs codex & gemini installed; set `shell.architect` accordingly): + - `afx workspace start` main architect launches with a **stale Claude `.jsonl` present** in `~/.claude/projects//` — must NOT crash-loop, and no `--resume` in the launched command (primary regression target) + - `afx architect` (no-Tower) + `afx workspace add-architect` launch with role injected + - gemini: `.gemini/settings.json` written with `context.fileName: "AGENTS.md"` (pre-existing one untouched) + - `afx send` single-line / multi-line (>3 lines) / `--interrupt` / while streaming + - `afx spawn --resume` on a non-Claude builder → fresh launch + resume notice, inspect `.builder-start.sh` for no `--resume ` diff --git a/codev/state/pir-929_thread.md b/codev/state/pir-929_thread.md index 02db140a8..6f7bf8fab 100644 --- a/codev/state/pir-929_thread.md +++ b/codev/state/pir-929_thread.md @@ -42,3 +42,9 @@ Plan approved. Implemented all six tasks: **Test-infra notes**: worktree had no `node_modules` — ran `pnpm install` + built `@cluesmith/codev-core` (vitest loads source which imports the core package's subpath exports). Two mock gaps surfaced and were fixed in the test files: `tower-instances.test.ts` db mock needed `getDb`/`closeDb` (state.getArchitects uses `getDb()`, not the already-mocked `getGlobalDb` — without it the `safeToResume` guard threw and silently skipped the claude resume); `spawn-worktree.test.ts` needed a `../lib/tower-client.js` mock (getTowerClient stub) so `startBuilderSession`→`createPtySession` runs without a live Tower. The three affected files: 143 passed. Full suite running. Build green. + +### 2026-06-16 — Review phase + +dev-approval gate approved (human validated the running worktree). Re-confirmed build green + 146 tests pass across the 3 affected files. Wrote `codev/reviews/929-support-codex-and-gemini-clis-.md` (Summary / Files / Architecture Updates / Lessons Learned / Things to Look At). arch.md was already updated in the implement commit; routed one cold-tier lesson to `lessons-learned.md` (Architecture): audit **all** invocation seams — including resume/restart — when extending a provider abstraction; the resume branch is a separate call site Spec 591 left harness-blind. + +PR target: branch lives on fork `mohidmakhdoomi/codev` (origin push → fork; `builder/pir-929` confirmed on fork, absent on cluesmith). Opening PR within the fork (base `main`). Recording with porch, then `porch done` runs the single-pass CMAP-2 verify. From 158671ba7543f0bd23986a680fbbf2878884e08c Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 15:10:45 -0400 Subject: [PATCH 12/26] chore(porch): 929 record PR #1 --- .../929-support-codex-and-gemini-clis-/status.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index 38b812964..3132be187 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -19,4 +19,9 @@ iteration: 1 build_complete: false history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-06-16T19:03:52.496Z' +updated_at: '2026-06-16T19:10:45.900Z' +pr_history: + - phase: review + pr_number: 1 + branch: builder/pir-929 + created_at: '2026-06-16T19:10:45.900Z' From 8ee31b4d38df1d7d9892ecc392a3efbd6d340625 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 15:11:14 -0400 Subject: [PATCH 13/26] chore(porch): 929 review build-complete --- codev/projects/929-support-codex-and-gemini-clis-/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index 3132be187..1fbbdc417 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -16,10 +16,10 @@ gates: pr: status: pending iteration: 1 -build_complete: false +build_complete: true history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-06-16T19:10:45.900Z' +updated_at: '2026-06-16T19:11:14.288Z' pr_history: - phase: review pr_number: 1 From 7ae3e4d8317b42dc19be92c2008c825284720664 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 15:14:05 -0400 Subject: [PATCH 14/26] chore(porch): 929 pr gate-requested --- .../projects/929-support-codex-and-gemini-clis-/status.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index 1fbbdc417..69825c718 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -15,13 +15,15 @@ gates: approved_at: '2026-06-16T19:03:40.955Z' pr: status: pending + requested_at: '2026-06-16T19:14:05.958Z' iteration: 1 -build_complete: true +build_complete: false history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-06-16T19:11:14.288Z' +updated_at: '2026-06-16T19:14:05.959Z' pr_history: - phase: review pr_number: 1 branch: builder/pir-929 created_at: '2026-06-16T19:10:45.900Z' +pr_ready_for_human: true From 9b615cdf1baf7970e2dc9b7ec81426614f52146f Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 15:40:21 -0400 Subject: [PATCH 15/26] [PIR #929] Address architect integration review: override-aware harness + centralized context files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BLOCKER B: getArchitectHarness/getBuilderHarness now auto-detect the harness from the override-aware resolved command (getResolvedCommands honoring TOWER_ARCHITECT_CMD / --architect-cmd / --builder-cmd), so a non-claude command override no longer resolves the claude harness and re-arms the resume crash-loop. BLOCKER A: centralize getArchitectFiles writing into the shared buildArchitectArgs (exported writeArchitectContextFiles) and route the no-Tower architect command through it, so sibling / no-Tower / reconnect gemini launches all get .gemini/settings.json — not just first-activation. Nits: gate the resume-skip WARN on buildResume support; distinguish "harness does not support resume" in discoverResumeSession; gitignore .gemini/settings.json (repo + managed adopter list). Adds config.test.ts (override-aware resolution) and tower-utils.test.ts (writeArchitectContextFiles) regression coverage. Co-Authored-By: Claude Opus 4.7 --- .gitignore | 1 + codev/resources/arch.md | 4 +- .../929-support-codex-and-gemini-clis-.md | 44 ++++++++++----- codev/state/pir-929_thread.md | 14 +++++ .../codev/src/__tests__/gitignore.test.ts | 4 +- packages/codev/src/__tests__/update.test.ts | 4 +- .../agent-farm/__tests__/af-architect.test.ts | 17 ++++-- .../src/agent-farm/__tests__/config.test.ts | 55 ++++++++++++++++++- .../agent-farm/__tests__/tower-utils.test.ts | 43 +++++++++++++++ .../src/agent-farm/commands/architect.ts | 28 +++------- .../codev/src/agent-farm/commands/spawn.ts | 6 +- .../src/agent-farm/servers/tower-instances.ts | 21 +++---- .../src/agent-farm/servers/tower-utils.ts | 29 +++++++++- packages/codev/src/agent-farm/utils/config.ts | 16 +++++- packages/codev/src/lib/gitignore.ts | 1 + 15 files changed, 224 insertions(+), 63 deletions(-) diff --git a/.gitignore b/.gitignore index f9a8afb02..265493b8f 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ worktrees/ .env .update-hashes.json .architect-role.md +.gemini/settings.json .codev/config.json # OS files diff --git a/codev/resources/arch.md b/codev/resources/arch.md index 43d54c8ce..b9490738e 100644 --- a/codev/resources/arch.md +++ b/codev/resources/arch.md @@ -269,10 +269,12 @@ A `codev doctor` audit (`lib/framework-ref-audit.ts`) flags shell-fetch of frame #### Supported Architect Harnesses & Conversation Resume (#929) -**Supported architect harnesses** (Issue #929): claude, codex, and gemini are all supported as architects, selected via `.codev/config.json` (`shell.architect` / `shell.architectHarness`) — the same config-driven mechanism builders use. `.codev/config.json` (not `TOWER_ARCHITECT_CMD` / `--architect-cmd`) is the supported selection mechanism; an env/CLI command override with no matching harness config still resolves the default (claude) harness flags. OpenCode remains builder-only (file-based injection needs an ephemeral worktree). Gemini additionally gets a `.gemini/settings.json` (`context.fileName` → `AGENTS.md`) written if-absent at launch via the harness's `getArchitectFiles`, so it reads project context the way codex reads `AGENTS.md` natively. +**Supported architect harnesses** (Issue #929): claude, codex, and gemini are all supported as architects, selected via `.codev/config.json` (`shell.architect` / `shell.architectHarness`) — the same config-driven mechanism builders use, and the *recommended* one. Harness auto-detection is **override-aware**: `getArchitectHarness` / `getBuilderHarness` resolve the harness from the override-aware command (`getResolvedCommands` → `cliOverrides` / `TOWER_ARCHITECT_CMD` / config), so a `--architect-cmd gemini` / `TOWER_ARCHITECT_CMD=gemini` / `--builder-cmd gemini` with no matching harness config still resolves the *gemini* harness, not claude. (Before #929 it auto-detected from the raw config value only — an override launched gemini but resolved the claude harness, re-arming the resume crash-loop below.) An explicit `shell.architectHarness` / `shell.builderHarness` still wins over auto-detection. OpenCode remains builder-only (file-based injection needs an ephemeral worktree). Gemini additionally gets a `.gemini/settings.json` (`context.fileName` → `AGENTS.md`) written if-absent at launch via the harness's `getArchitectFiles`, so it reads project context the way codex reads `AGENTS.md` natively. **Conversation resume is Claude-main-only.** `launchInstance` resumes a prior session only when the configured harness implements `HarnessProvider.buildResume` — currently just claude (its sessions live at `~/.claude/projects//*.jsonl`). Codex/gemini architects (and resumed codex/gemini builders, via `spawn.ts` `discoverResumeSession`) return `null` from `buildResume` and relaunch fresh with role injection. This gating fixes a latent crash-loop where a non-Claude harness + a stale Claude `.jsonl` built an invalid ` --resume ` invocation and shellper restart-looped to death. +**`getArchitectFiles` is centralized in `buildArchitectArgs`** (`tower-utils.ts`), the shared role-injection helper every architect-launch path routes through — `launchInstance` (fresh), `add-architect` (sibling), shellper reconnect (×2), and the no-Tower `afx architect` (refactored in #929 to call `buildArchitectArgs` instead of duplicating injection). So Gemini's context manifest is written on **every** launch path, not just first-activation. + #### Multi-Architect Support (Spec 755 / Spec 786) A workspace can host more than one architect terminal. Each architect has a stable name (`main` for the workspace's default; siblings via `afx workspace add-architect`). The primary use case is letting a sibling architect drive a focused workflow without monopolising `main`. diff --git a/codev/reviews/929-support-codex-and-gemini-clis-.md b/codev/reviews/929-support-codex-and-gemini-clis-.md index 4978cb95a..0565dc0e1 100644 --- a/codev/reviews/929-support-codex-and-gemini-clis-.md +++ b/codev/reviews/929-support-codex-and-gemini-clis-.md @@ -8,16 +8,31 @@ Brings the OpenAI `codex` and Google `gemini` CLIs to parity with `claude` as Co ## Files Changed -- `packages/codev/src/agent-farm/utils/harness.ts` (+46 / -0) — `buildResume?` + `getArchitectFiles?` on the interface; `CLAUDE_HARNESS.buildResume` (delegates to `findLatestSessionId`); `GEMINI_HARNESS.getArchitectFiles` (`.gemini/settings.json`) -- `packages/codev/src/agent-farm/servers/tower-instances.ts` (+34 / -7) — architect resume gated on `getArchitectHarness(...).buildResume?.()`; writes `getArchitectFiles?()` if-missing on launch -- `packages/codev/src/agent-farm/commands/spawn.ts` (+25 / -17) — `discoverResumeSession` takes the builder harness, returns the bundled resume object; both call sites pass `getBuilderHarness(...)` -- `packages/codev/src/agent-farm/commands/spawn-worktree.ts` (+14 / -11) — `startBuilderSession`'s `resumeSessionId?: string` → `resume?: { sessionId, scriptFragment }`; script emits the pre-escaped fragment -- `packages/codev/src/commands/doctor.ts` (+9 / -2) — affirm codex/gemini architect support; single resolved-harness check -- `packages/codev/src/agent-farm/__tests__/tower-instances.test.ts` (+152 / -0) — architect resume-skip regression guard + gemini `getArchitectFiles` write-if-missing/no-clobber -- `packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts` (+74 / -1) — builder resume script uses escaped `scriptFragment`; codex/gemini → fresh script -- `packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts` (+44 / -1) — harness-arg threading; codex/gemini null-return + claude bundled-object cases -- `codev/resources/arch.md` (+7 / -1) — supported-architect-harnesses + Claude-only-resume documentation -- `codev/plans/929-support-codex-and-gemini-clis-.md`, `codev/state/pir-929_thread.md`, `codev/projects/929-*/status.yaml` — protocol artifacts +Resume seam + gemini context (initial implementation): +- `packages/codev/src/agent-farm/utils/harness.ts` — `buildResume?` + `getArchitectFiles?` on the interface; `CLAUDE_HARNESS.buildResume` (delegates to `findLatestSessionId`); `GEMINI_HARNESS.getArchitectFiles` (`.gemini/settings.json`) +- `packages/codev/src/agent-farm/servers/tower-instances.ts` — architect resume gated on `getArchitectHarness(...).buildResume?.()`; fresh path delegates context-file writing to `buildArchitectArgs` +- `packages/codev/src/agent-farm/commands/spawn.ts` — `discoverResumeSession` takes the builder harness, returns the bundled resume object; both call sites pass `getBuilderHarness(...)`; distinct "harness does not support resume" log (nit 2) +- `packages/codev/src/agent-farm/commands/spawn-worktree.ts` — `startBuilderSession`'s `resumeSessionId?: string` → `resume?: { sessionId, scriptFragment }`; script emits the pre-escaped fragment +- `packages/codev/src/commands/doctor.ts` — affirm codex/gemini architect support; single resolved-harness check + +Integration-review fixes (architect punch list — 2 blockers + 3 nits): +- `packages/codev/src/agent-farm/utils/config.ts` — **BLOCKER B**: `getArchitectHarness`/`getBuilderHarness` now auto-detect from the *override-aware* command (`getResolvedCommands`); `getResolvedCommands.architect` honors `TOWER_ARCHITECT_CMD` +- `packages/codev/src/agent-farm/servers/tower-utils.ts` — **BLOCKER A**: exported `writeArchitectContextFiles`, called from the shared `buildArchitectArgs` so every launch path writes gemini's manifest; **nit 1**: (see tower-instances) WARN gated on resume support +- `packages/codev/src/agent-farm/commands/architect.ts` — **BLOCKER A**: no-Tower path refactored to call `buildArchitectArgs` (was duplicating injection) +- `.gitignore`, `packages/codev/src/lib/gitignore.ts` — **nit 3**: ignore `.gemini/settings.json` (repo + managed adopter list) + +Tests: +- `packages/codev/src/agent-farm/__tests__/tower-instances.test.ts` — architect resume-skip regression guard + gemini `getArchitectFiles` write-if-missing/no-clobber +- `packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts` — builder resume script uses escaped `scriptFragment`; codex/gemini → fresh script +- `packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts` — harness-arg threading; codex/gemini null-return + claude bundled-object cases +- `packages/codev/src/agent-farm/__tests__/config.test.ts` — **BLOCKER B regression**: override-aware harness resolution (TOWER_ARCHITECT_CMD / --architect-cmd / --builder-cmd → non-claude harness, no `buildResume`) +- `packages/codev/src/agent-farm/__tests__/tower-utils.test.ts` — **BLOCKER A regression**: `writeArchitectContextFiles` gemini write + no-clobber + claude no-op +- `packages/codev/src/agent-farm/__tests__/af-architect.test.ts` — updated mocks for the `buildArchitectArgs` delegation +- `packages/codev/src/__tests__/gitignore.test.ts`, `update.test.ts` — managed-entry expectations include `.gemini/settings.json` + +Docs / artifacts: +- `codev/resources/arch.md` — supported-architect-harnesses + Claude-only-resume + override-aware resolution + `getArchitectFiles` centralization +- `codev/plans/...`, `codev/reviews/...`, `codev/state/pir-929_thread.md`, `codev/resources/lessons-learned.md`, `codev/projects/929-*/status.yaml` ## Commits @@ -27,9 +42,9 @@ Brings the OpenAI `codex` and Google `gemini` CLIs to parity with `claude` as Co ## Test Results -- `pnpm build`: ✓ pass (clean TS types for the new optional methods) -- `pnpm vitest run` (3 affected files): ✓ pass (146 tests) -- Manual verification: empirical codex/gemini lifecycle validation (clean + stale-jsonl launch, add-architect, `afx send` multiline/interrupt/streaming, reconnect, affinity, builder `--resume`, dashboard scrollback) was exercised by the human at the `dev-approval` gate against the running worktree — the reason PIR was chosen over AIR/BUGFIX. +- `pnpm build`: ✓ pass (clean TS types) +- `pnpm vitest run` (full suite): ✓ 3338 passed, 48 skipped, 0 failures +- Manual verification: empirical codex/gemini lifecycle validation (clean + stale-jsonl launch, add-architect, `afx send` multiline/interrupt/streaming, reconnect, affinity, builder `--resume`, dashboard scrollback) was exercised by the human at the `dev-approval` gate against the running worktree — the reason PIR was chosen over AIR/BUGFIX. The architect's subsequent **integration review** caught the two override/path-dependency blockers below (fixed + regression-tested in this PR). ## Architecture Updates @@ -48,7 +63,8 @@ No **HOT** (`lessons-critical.md`) change: the existing "Single source of truth - **The `buildResume` bundling decision** (`harness.ts`): one method returns both the Node-argv `args` (for the `spawn()` architect site) and a shell-escaped `scriptFragment` (for the builder bash generator), mirroring `buildRoleInjection`/`buildScriptRoleInjection`. This deliberately avoids a second independently-optional method (which would force a `!` non-null assertion) and avoids `.join(' ')`-ing a raw argv into bash (word-split/quoting bug). Session ids are bare UUIDs today, so the escaping is belt-and-suspenders — kept for correct-by-construction consistency with the existing script-injection methods. - **The `safeToResume` interaction** (`tower-instances.ts`): the new harness gate composes with the pre-existing sibling-collision guard (`safeToResume`, #832) — resume happens only when *both* the harness implements `buildResume` *and* no persisted siblings exist. Confirm the ordering reads correctly. - **`getArchitectFiles` write-if-absent** (`tower-instances.ts`): writes `.gemini/settings.json` only when the target path doesn't exist, so a user's existing file is never clobbered. Test covers both the write and the no-clobber path. -- **Two documented out-of-scope override caveats** (plan Risks): `TOWER_ARCHITECT_CMD`/`--architect-cmd` (and the `--builder-cmd` analog) set a non-Claude command without a matching `.codev/config.json` harness still resolve the claude harness → would still attempt resume. These are the issue's explicit *nice-to-have* ("command-aware harness resolution"); MVP fixes the config-driven path (all acceptance criteria target it) and documents the override caveat rather than expanding scope. +- **Override-aware harness resolution precedence (BLOCKER B fix)**: `getResolvedCommands.architect` is now `cliOverrides.architect || TOWER_ARCHITECT_CMD || config`. Within the Tower process `cliOverrides` is empty (it's set in the spawning `afx` process, not the long-lived server), so this matches the launch site's `TOWER_ARCHITECT_CMD || config`. An explicit `shell.architectHarness` / `shell.builderHarness` still wins over auto-detection by design — so a *deliberately* contradictory `architectHarness: claude` + gemini command is the user's call, not auto-resolved. Worth a sanity check that this precedence reads as intended. +- **`getArchitectFiles` centralization (BLOCKER A fix)**: moved the inline write out of `launchInstance` into the shared `buildArchitectArgs` (`writeArchitectContextFiles`), and refactored the no-Tower `architect.ts` to call `buildArchitectArgs` instead of duplicating role injection. Confirm the no-Tower path's arg shape is unchanged (covered by `af-architect.test.ts`) and that the claude resume path — the one path that does *not* call `buildArchitectArgs` — correctly needs no context files (claude has no `getArchitectFiles`). ## How to Test Locally diff --git a/codev/state/pir-929_thread.md b/codev/state/pir-929_thread.md index 6f7bf8fab..36c55ab40 100644 --- a/codev/state/pir-929_thread.md +++ b/codev/state/pir-929_thread.md @@ -48,3 +48,17 @@ The three affected files: 143 passed. Full suite running. Build green. dev-approval gate approved (human validated the running worktree). Re-confirmed build green + 146 tests pass across the 3 affected files. Wrote `codev/reviews/929-support-codex-and-gemini-clis-.md` (Summary / Files / Architecture Updates / Lessons Learned / Things to Look At). arch.md was already updated in the implement commit; routed one cold-tier lesson to `lessons-learned.md` (Architecture): audit **all** invocation seams — including resume/restart — when extending a provider abstraction; the resume branch is a separate call site Spec 591 left harness-blind. PR target: branch lives on fork `mohidmakhdoomi/codev` (origin push → fork; `builder/pir-929` confirmed on fork, absent on cluesmith). Opening PR within the fork (base `main`). Recording with porch, then `porch done` runs the single-pass CMAP-2 verify. + +CMAP-2 verdicts (iter1): gemini=APPROVE, codex=APPROVE, no issues. (`pr_exists` check first failed because `gh` had no default repo → defaulted to cluesmith where the branch doesn't live; fixed with `gh repo set-default mohidmakhdoomi/codev`.) Gate fired pending; architect notified. + +### 2026-06-16 — Architect integration review: REQUEST CHANGES (2 blockers + 3 nits) + +Architect reviewed the running integration and returned a punch list. All addressed: + +- **BLOCKER B (override re-arms crash-loop)**: `getArchitectHarness`/`getBuilderHarness` auto-detected the harness from the *raw config* command, ignoring `TOWER_ARCHITECT_CMD` / `--architect-cmd` / `--builder-cmd`. So `TOWER_ARCHITECT_CMD=gemini` launched gemini but resolved claude → buildResume injects `--resume ` → the exact crash-loop. Fix: both resolvers now source the command from `getResolvedCommands` (override-aware), and `getResolvedCommands.architect` now includes `TOWER_ARCHITECT_CMD`. Regression tests in `config.test.ts`. +- **BLOCKER A (gemini context path-dependent)**: `getArchitectFiles` was written inline only in `launchInstance`, so sibling/no-Tower/reconnect gemini launches were context-blind. Fix: centralized into `buildArchitectArgs` (the shared helper all 4 Tower paths route through) as exported `writeArchitectContextFiles`; refactored the no-Tower `architect.ts` to call `buildArchitectArgs` instead of duplicating injection. Now every launch path writes it. Regression tests in `tower-utils.test.ts`. +- **NIT 1**: `safeToResume` WARN now gated on `architectHarness.buildResume` (no noise for codex/gemini, which never resume). +- **NIT 2**: `discoverResumeSession` logs "This harness does not support conversation resume" when `buildResume` is undefined, distinct from "No prior conversation found". +- **NIT 3**: `.gemini/settings.json` added to both root `.gitignore` and the managed adopter `CODEV_GITIGNORE_ENTRIES` (gitignore.ts); gitignore/update test expectations updated. + +Full suite: 3338 passed, 48 skipped, 0 failures. Build green. Re-running `porch done` for a fresh single CMAP-2 pass, then re-requesting the pr gate. diff --git a/packages/codev/src/__tests__/gitignore.test.ts b/packages/codev/src/__tests__/gitignore.test.ts index 971d1d0ef..6110039a1 100644 --- a/packages/codev/src/__tests__/gitignore.test.ts +++ b/packages/codev/src/__tests__/gitignore.test.ts @@ -137,7 +137,7 @@ describe('Gitignore Utilities', () => { const result = backfillGitignore(targetDir, CODEV_GITIGNORE_ENTRIES, { today: new Date('2026-05-27') }); expect(result.skipped).toBe(false); - expect(result.added).toEqual(['.architect-role.md']); + expect(result.added).toEqual(['.architect-role.md', '.gemini/settings.json']); expect(result.alreadyPresent).toEqual( expect.arrayContaining(['.agent-farm/', '.consult/', 'codev/.update-hashes.json', '.builders/']) ); @@ -208,7 +208,7 @@ describe('Gitignore Utilities', () => { const result = backfillGitignore(targetDir, CODEV_GITIGNORE_ENTRIES, { dryRun: true }); - expect(result.added).toEqual(['.architect-role.md']); + expect(result.added).toEqual(['.architect-role.md', '.gemini/settings.json']); expect(fs.readFileSync(path.join(targetDir, '.gitignore'), 'utf-8')).toBe(original); }); diff --git a/packages/codev/src/__tests__/update.test.ts b/packages/codev/src/__tests__/update.test.ts index efe9fc8cc..bfef0597e 100644 --- a/packages/codev/src/__tests__/update.test.ts +++ b/packages/codev/src/__tests__/update.test.ts @@ -527,7 +527,7 @@ describe('update command', () => { const result = await update({ agent: true }); expect(result.error).toBeUndefined(); - expect(result.gitignoreAdded).toEqual(['.architect-role.md']); + expect(result.gitignoreAdded).toEqual(['.architect-role.md', '.gemini/settings.json']); const content = fs.readFileSync(path.join(projectDir, '.gitignore'), 'utf-8'); expect(content).toContain('.architect-role.md'); @@ -547,7 +547,7 @@ describe('update command', () => { const { update } = await import('../commands/update.js'); const result = await update({ agent: true, dryRun: true }); - expect(result.gitignoreAdded).toEqual(['.architect-role.md']); + expect(result.gitignoreAdded).toEqual(['.architect-role.md', '.gemini/settings.json']); expect(fs.readFileSync(path.join(projectDir, '.gitignore'), 'utf-8')).toBe(stale); }); diff --git a/packages/codev/src/agent-farm/__tests__/af-architect.test.ts b/packages/codev/src/agent-farm/__tests__/af-architect.test.ts index 8941b15cc..e55f52320 100644 --- a/packages/codev/src/agent-farm/__tests__/af-architect.test.ts +++ b/packages/codev/src/agent-farm/__tests__/af-architect.test.ts @@ -14,10 +14,12 @@ vi.mock('node:child_process', () => ({ spawn: (...args: unknown[]) => mockSpawn(...args), })); -// Mock fs.writeFileSync -vi.mock('node:fs', () => ({ - writeFileSync: vi.fn(), -})); +// Mock fs — buildArchitectArgs (tower-utils) uses a default `import fs from 'node:fs'`, +// architect.ts historically used the named export, so provide both. +vi.mock('node:fs', () => { + const fns = { writeFileSync: vi.fn(), existsSync: vi.fn(() => false), mkdirSync: vi.fn() }; + return { ...fns, default: fns }; +}); // Mock config — include getArchitectHarness vi.mock('../utils/index.js', () => ({ @@ -34,6 +36,13 @@ vi.mock('../utils/index.js', () => ({ getArchitectHarness: () => CLAUDE_HARNESS, })); +// architect() now delegates role injection to buildArchitectArgs (tower-utils), +// which resolves the harness via config.js directly (not the index.js barrel +// mocked above). Mock that seam too so the unit test stays filesystem-free. +vi.mock('../utils/config.js', () => ({ + getArchitectHarness: () => CLAUDE_HARNESS, +})); + // Mock role loading vi.mock('../utils/roles.js', () => ({ loadRolePrompt: vi.fn(() => ({ diff --git a/packages/codev/src/agent-farm/__tests__/config.test.ts b/packages/codev/src/agent-farm/__tests__/config.test.ts index 98e471b8e..dee03f099 100644 --- a/packages/codev/src/agent-farm/__tests__/config.test.ts +++ b/packages/codev/src/agent-farm/__tests__/config.test.ts @@ -3,7 +3,13 @@ */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { getConfig, ensureDirectories } from '../utils/config.js'; +import { + getConfig, + ensureDirectories, + getArchitectHarness, + getBuilderHarness, + setCliOverrides, +} from '../utils/config.js'; import { existsSync } from 'node:fs'; import { rm, mkdir } from 'node:fs/promises'; import { resolve } from 'node:path'; @@ -89,3 +95,50 @@ describe('ensureDirectories', () => { await expect(ensureDirectories(testConfig)).resolves.not.toThrow(); }); }); + +// Issue #929 — harness resolution must be override-aware. The mocked config +// above resolves both shells to `claude` with NO explicit *Harness, so the +// harness is auto-detected from the resolved command. A command override +// (TOWER_ARCHITECT_CMD / --architect-cmd / --builder-cmd) without a matching +// harness config previously still resolved the CLAUDE harness, whose buildResume +// would inject `--resume ` into the non-claude command and +// crash-loop. `buildResume` being undefined is the precise property that makes +// codex/gemini relaunch fresh — so it's the regression assertion. +describe('getArchitectHarness / getBuilderHarness override-awareness (#929)', () => { + const savedArchitectCmd = process.env.TOWER_ARCHITECT_CMD; + + afterEach(() => { + setCliOverrides({}); + if (savedArchitectCmd === undefined) { + delete process.env.TOWER_ARCHITECT_CMD; + } else { + process.env.TOWER_ARCHITECT_CMD = savedArchitectCmd; + } + }); + + it('resolves the claude harness (buildResume defined) with no overrides', () => { + delete process.env.TOWER_ARCHITECT_CMD; + setCliOverrides({}); + expect(getArchitectHarness().buildResume).toBeDefined(); + expect(getBuilderHarness().buildResume).toBeDefined(); + }); + + it('TOWER_ARCHITECT_CMD=gemini → gemini architect harness (no claude resume)', () => { + process.env.TOWER_ARCHITECT_CMD = 'gemini'; + const harness = getArchitectHarness(); + expect(harness.buildResume).toBeUndefined(); + // gemini harness writes its context manifest + expect(harness.getArchitectFiles).toBeDefined(); + }); + + it('--architect-cmd codex → codex architect harness (no claude resume)', () => { + delete process.env.TOWER_ARCHITECT_CMD; + setCliOverrides({ architect: 'codex' }); + expect(getArchitectHarness().buildResume).toBeUndefined(); + }); + + it('--builder-cmd gemini → gemini builder harness (no claude resume)', () => { + setCliOverrides({ builder: 'gemini' }); + expect(getBuilderHarness().buildResume).toBeUndefined(); + }); +}); diff --git a/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts b/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts index d78480000..96842bcd4 100644 --- a/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts +++ b/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts @@ -18,7 +18,9 @@ import { normalizeWorkspacePath, isTempDirectory, serveStaticFile, + writeArchitectContextFiles, } from '../servers/tower-utils.js'; +import { CLAUDE_HARNESS, GEMINI_HARNESS } from '../utils/harness.js'; describe('tower-utils', () => { describe('isRateLimited', () => { @@ -197,4 +199,45 @@ describe('tower-utils', () => { } }); }); + + // Issue #929 — writeArchitectContextFiles is the shared helper buildArchitectArgs + // calls, so EVERY architect-launch path (launchInstance, add-architect sibling, + // reconnect, no-Tower `afx architect`) writes harness-specific context files, + // not just first-activation. Previously the write lived inline in launchInstance. + describe('writeArchitectContextFiles (#929)', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(tmpdir(), 'arch-files-')); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('gemini → writes .gemini/settings.json pointing at AGENTS.md', () => { + writeArchitectContextFiles(tmpDir, GEMINI_HARNESS); + const settingsPath = path.join(tmpDir, '.gemini', 'settings.json'); + expect(fs.existsSync(settingsPath)).toBe(true); + expect(JSON.parse(fs.readFileSync(settingsPath, 'utf-8'))).toEqual({ + context: { fileName: 'AGENTS.md' }, + }); + }); + + it('gemini → does not clobber a pre-existing .gemini/settings.json', () => { + const geminiDir = path.join(tmpDir, '.gemini'); + fs.mkdirSync(geminiDir, { recursive: true }); + const settingsPath = path.join(geminiDir, 'settings.json'); + fs.writeFileSync(settingsPath, '{"custom":true}'); + + writeArchitectContextFiles(tmpDir, GEMINI_HARNESS); + + expect(fs.readFileSync(settingsPath, 'utf-8')).toBe('{"custom":true}'); + }); + + it('claude → writes nothing (no getArchitectFiles)', () => { + writeArchitectContextFiles(tmpDir, CLAUDE_HARNESS); + expect(fs.existsSync(path.join(tmpDir, '.gemini'))).toBe(false); + }); + }); }); diff --git a/packages/codev/src/agent-farm/commands/architect.ts b/packages/codev/src/agent-farm/commands/architect.ts index 419d11db1..f8fb55177 100644 --- a/packages/codev/src/agent-farm/commands/architect.ts +++ b/packages/codev/src/agent-farm/commands/architect.ts @@ -7,10 +7,8 @@ */ import { spawn } from 'node:child_process'; -import { writeFileSync } from 'node:fs'; -import { resolve } from 'node:path'; -import { getConfig, getResolvedCommands, getArchitectHarness } from '../utils/index.js'; -import { loadRolePrompt } from '../utils/roles.js'; +import { getConfig, getResolvedCommands } from '../utils/index.js'; +import { buildArchitectArgs } from '../servers/tower-utils.js'; export interface ArchitectOptions { args?: string[]; @@ -23,25 +21,15 @@ export async function architect(options: ArchitectOptions = {}): Promise { const config = getConfig(); const commands = getResolvedCommands(); - const args = [...(options.args || [])]; - let env: Record = {}; - - // Load and inject the architect role prompt via harness - const role = loadRolePrompt(config, 'architect'); - if (role) { - const roleFilePath = resolve(config.workspaceRoot, '.architect-role.md'); - writeFileSync(roleFilePath, role.content); - - const harness = getArchitectHarness(config.workspaceRoot); - const injection = harness.buildRoleInjection(role.content, roleFilePath); - args.push(...injection.args); - env = injection.env; - } - // Split command string into executable + initial args (supports e.g. "claude --dangerously-skip-permissions") const cmdParts = commands.architect.split(/\s+/); const cmd = cmdParts[0]; - const allArgs = [...cmdParts.slice(1), ...args]; + + // Inject the architect role + write any harness-specific context files + // (e.g. Gemini's .gemini/settings.json) via the shared launch helper, so the + // no-Tower path matches every Tower launch path (Issue #929). + const baseArgs = [...cmdParts.slice(1), ...(options.args || [])]; + const { args: allArgs, env } = buildArchitectArgs(baseArgs, config.workspaceRoot); return new Promise((resolve, reject) => { const child = spawn(cmd, allArgs, { diff --git a/packages/codev/src/agent-farm/commands/spawn.ts b/packages/codev/src/agent-farm/commands/spawn.ts index 2ea2a6bc9..217398f43 100644 --- a/packages/codev/src/agent-farm/commands/spawn.ts +++ b/packages/codev/src/agent-farm/commands/spawn.ts @@ -90,7 +90,11 @@ export function discoverResumeSession( harness: HarnessProvider, ): { sessionId: string; args: string[]; scriptFragment: string } | undefined { if (!isResume) return undefined; - const resume = harness.buildResume?.(worktreePath) ?? null; + if (!harness.buildResume) { + logger.info('This harness does not support conversation resume; starting a fresh session.'); + return undefined; + } + const resume = harness.buildResume(worktreePath); if (resume) { logger.kv('Session', `${resume.sessionId.slice(0, 8)}… (resuming conversation)`); return resume; diff --git a/packages/codev/src/agent-farm/servers/tower-instances.ts b/packages/codev/src/agent-farm/servers/tower-instances.ts index 9cc234d5b..b52230c5f 100644 --- a/packages/codev/src/agent-farm/servers/tower-instances.ts +++ b/packages/codev/src/agent-farm/servers/tower-instances.ts @@ -515,23 +515,13 @@ export async function launchInstance(workspacePath: string): Promise<{ success: } const architectHarness = getArchitectHarness(workspacePath); const resume = safeToResume ? (architectHarness.buildResume?.(workspacePath) ?? null) : null; - if (!safeToResume) { + // Only warn about a *skipped* resume when this harness actually supports + // resume (buildResume defined → claude). For codex/gemini, resume was + // never on the table, so the sibling-collision warning is just noise. + if (!safeToResume && architectHarness.buildResume) { _deps.log('WARN', `Skipping main architect conversation resume for ${workspacePath}: persisted sibling architects detected (or state.db unreadable); cannot disambiguate jsonl by cwd. See #832.`); } - // Write any harness-specific architect-context files (e.g., Gemini's - // .gemini/settings.json → AGENTS.md), only if absent — never clobber a - // user's existing file. (Issue #929.) - if (architectHarness.getArchitectFiles) { - for (const file of architectHarness.getArchitectFiles(workspacePath)) { - const targetPath = path.join(workspacePath, file.relativePath); - if (!fs.existsSync(targetPath)) { - fs.mkdirSync(path.dirname(targetPath), { recursive: true }); - fs.writeFileSync(targetPath, file.content); - } - } - } - let cmdArgs: string[]; let harnessEnv: Record; if (resume) { @@ -539,6 +529,9 @@ export async function launchInstance(workspacePath: string): Promise<{ success: harnessEnv = {}; _deps.log('INFO', `Resuming main architect session ${resume.sessionId.slice(0, 8)}… for ${workspacePath}`); } else { + // Fresh launch — buildArchitectArgs writes any harness-specific + // context files (e.g. Gemini's .gemini/settings.json) and injects the + // role. The resume path above is claude-only, which needs neither. const built = buildArchitectArgs(cmdParts.slice(1), workspacePath); cmdArgs = built.args; harnessEnv = built.env; diff --git a/packages/codev/src/agent-farm/servers/tower-utils.ts b/packages/codev/src/agent-farm/servers/tower-utils.ts index 3cd777d54..69ff95a74 100644 --- a/packages/codev/src/agent-farm/servers/tower-utils.ts +++ b/packages/codev/src/agent-farm/servers/tower-utils.ts @@ -13,6 +13,7 @@ import type { ServerResponse } from 'node:http'; import type { RateLimitEntry } from './tower-types.js'; import { loadRolePrompt, type RoleConfig } from '../utils/roles.js'; import { getArchitectHarness } from '../utils/config.js'; +import type { HarnessProvider } from '../utils/harness.js'; // ============================================================================ // Rate Limiting @@ -167,10 +168,32 @@ export const MIME_TYPES: Record = { // Architect Role Prompt // ============================================================================ +/** + * Write any harness-specific architect-context files into the workspace, + * only if absent so a user's existing file is never clobbered. Used by + * harnesses that need a project-context manifest role injection alone doesn't + * provide (e.g., Gemini's .gemini/settings.json → AGENTS.md). Issue #929. + * + * Called from buildArchitectArgs so EVERY architect-launch path that routes + * through it (launchInstance fresh, add-architect sibling, reconnect, and the + * no-Tower `afx architect`) writes these files — not just launchInstance. + */ +export function writeArchitectContextFiles(workspacePath: string, harness: HarnessProvider): void { + if (!harness.getArchitectFiles) return; + for (const file of harness.getArchitectFiles(workspacePath)) { + const targetPath = path.join(workspacePath, file.relativePath); + if (!fs.existsSync(targetPath)) { + fs.mkdirSync(path.dirname(targetPath), { recursive: true }); + fs.writeFileSync(targetPath, file.content); + } + } +} + /** * Build architect command args with role prompt injected via harness provider. * Writes the role to .architect-role.md in the workspace dir and uses the - * configured harness to determine the correct CLI args and env vars. + * configured harness to determine the correct CLI args and env vars. Also + * writes any harness-specific context files (e.g. Gemini's .gemini/settings.json). * Returns args and env for the caller to merge into session creation. */ export function buildArchitectArgs(baseArgs: string[], workspacePath: string): { args: string[]; env: Record } { @@ -178,13 +201,15 @@ export function buildArchitectArgs(baseArgs: string[], workspacePath: string): { const bundledRolesDir = path.resolve(import.meta.dirname, '../../../skeleton/roles'); const config: RoleConfig = { codevDir, bundledRolesDir, workspaceRoot: workspacePath }; + const harness = getArchitectHarness(workspacePath); + writeArchitectContextFiles(workspacePath, harness); + const role = loadRolePrompt(config, 'architect'); if (!role) return { args: baseArgs, env: {} }; const roleFile = path.join(workspacePath, '.architect-role.md'); fs.writeFileSync(roleFile, role.content); - const harness = getArchitectHarness(workspacePath); const injection = harness.buildRoleInjection(role.content, roleFile); return { diff --git a/packages/codev/src/agent-farm/utils/config.ts b/packages/codev/src/agent-farm/utils/config.ts index c5ca82d22..6fd65656e 100644 --- a/packages/codev/src/agent-farm/utils/config.ts +++ b/packages/codev/src/agent-farm/utils/config.ts @@ -237,6 +237,7 @@ export function getResolvedCommands(workspaceRoot?: string): ResolvedCommands { return { architect: cliOverrides.architect || + process.env.TOWER_ARCHITECT_CMD || resolveCommand(userConfig?.shell?.architect, DEFAULT_COMMANDS.architect), builder: cliOverrides.builder || resolveCommand(userConfig?.shell?.builder, DEFAULT_COMMANDS.builder), @@ -248,11 +249,18 @@ export function getResolvedCommands(workspaceRoot?: string): ResolvedCommands { /** * Get the resolved harness provider for the architect shell. * Resolution: explicit architectHarness → auto-detect from architect command → default claude. + * + * The command auto-detect source is the *override-aware* resolved command + * (getResolvedCommands → cliOverrides / TOWER_ARCHITECT_CMD / config), not the + * raw config value. Without this, `--architect-cmd gemini` / `TOWER_ARCHITECT_CMD=gemini` + * with no matching harness config would launch gemini but resolve the claude + * harness, re-arming the #929 resume crash-loop (claude buildResume injecting + * `--resume ` into a non-claude command). */ export function getArchitectHarness(workspaceRoot?: string): HarnessProvider { const root = workspaceRoot || findWorkspaceRoot(); const userConfig = loadUserConfig(root); - const architectCmd = resolveCommand(userConfig?.shell?.architect, DEFAULT_COMMANDS.architect); + const architectCmd = getResolvedCommands(root).architect; return resolveHarness( userConfig?.shell?.architectHarness, userConfig?.harness as Record | undefined, @@ -263,11 +271,15 @@ export function getArchitectHarness(workspaceRoot?: string): HarnessProvider { /** * Get the resolved harness provider for the builder shell. * Resolution: explicit builderHarness → auto-detect from builder command → default claude. + * + * Like getArchitectHarness, the command auto-detect source is the override-aware + * resolved command (so `--builder-cmd gemini` with no matching harness config + * resolves the gemini harness, not claude — see #929). */ export function getBuilderHarness(workspaceRoot?: string): HarnessProvider { const root = workspaceRoot || findWorkspaceRoot(); const userConfig = loadUserConfig(root); - const builderCmd = resolveCommand(userConfig?.shell?.builder, DEFAULT_COMMANDS.builder); + const builderCmd = getResolvedCommands(root).builder; return resolveHarness( userConfig?.shell?.builderHarness, userConfig?.harness as Record | undefined, diff --git a/packages/codev/src/lib/gitignore.ts b/packages/codev/src/lib/gitignore.ts index bf9676e1a..075c6236a 100644 --- a/packages/codev/src/lib/gitignore.ts +++ b/packages/codev/src/lib/gitignore.ts @@ -21,6 +21,7 @@ export const CODEV_GITIGNORE_ENTRIES = `# Codev codev/.update-hashes.json .builders/ .architect-role.md +.gemini/settings.json `; /** From 455c433449baf78b5f25f365bd95f86d1c2211e5 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 15:40:32 -0400 Subject: [PATCH 16/26] chore(porch): 929 review build-complete --- codev/projects/929-support-codex-and-gemini-clis-/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index 69825c718..e05ca69e2 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -17,10 +17,10 @@ gates: status: pending requested_at: '2026-06-16T19:14:05.958Z' iteration: 1 -build_complete: false +build_complete: true history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-06-16T19:14:05.959Z' +updated_at: '2026-06-16T19:40:32.875Z' pr_history: - phase: review pr_number: 1 From dbe9ed0b85426ae09f23a6fb59359a2ddfd8de35 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 15:44:20 -0400 Subject: [PATCH 17/26] [PIR #929] Review: document iter2 CMAP verdicts + codex frontmatter rebuttal Co-Authored-By: Claude Opus 4.7 --- codev/reviews/929-support-codex-and-gemini-clis-.md | 8 ++++++++ codev/state/pir-929_thread.md | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/codev/reviews/929-support-codex-and-gemini-clis-.md b/codev/reviews/929-support-codex-and-gemini-clis-.md index 0565dc0e1..f99937aac 100644 --- a/codev/reviews/929-support-codex-and-gemini-clis-.md +++ b/codev/reviews/929-support-codex-and-gemini-clis-.md @@ -66,6 +66,14 @@ No **HOT** (`lessons-critical.md`) change: the existing "Single source of truth - **Override-aware harness resolution precedence (BLOCKER B fix)**: `getResolvedCommands.architect` is now `cliOverrides.architect || TOWER_ARCHITECT_CMD || config`. Within the Tower process `cliOverrides` is empty (it's set in the spawning `afx` process, not the long-lived server), so this matches the launch site's `TOWER_ARCHITECT_CMD || config`. An explicit `shell.architectHarness` / `shell.builderHarness` still wins over auto-detection by design — so a *deliberately* contradictory `architectHarness: claude` + gemini command is the user's call, not auto-resolved. Worth a sanity check that this precedence reads as intended. - **`getArchitectFiles` centralization (BLOCKER A fix)**: moved the inline write out of `launchInstance` into the shared `buildArchitectArgs` (`writeArchitectContextFiles`), and refactored the no-Tower `architect.ts` to call `buildArchitectArgs` instead of duplicating role injection. Confirm the no-Tower path's arg shape is unchanged (covered by `af-architect.test.ts`) and that the claude resume path — the one path that does *not* call `buildArchitectArgs` — correctly needs no context files (claude has no `getArchitectFiles`). +## Consultation Findings & Dispositions + +The PR diff was reviewed by a 2-way advisory CMAP pass after the integration-review fixes landed: + +- **gemini: APPROVE** (HIGH confidence) — no issues. +- **codex: REQUEST_CHANGES** (MEDIUM) — *"`codev/plans/929-...md` has no YAML approval frontmatter (`approved:` / `validated:`)."* + - **Disposition: REBUTTED (false positive).** The repo's frontmatter convention applies to artifacts the *architect pre-creates and pre-approves before spawning a builder* (CLAUDE.md: "When the architect creates and approves a spec or plan before spawning a builder, it must have YAML frontmatter…"). This plan was **builder-authored during the PIR plan phase and approved through porch's `plan-approval` gate** (recorded in `codev/projects/929-*/status.yaml`: `plan-approval: approved 2026-05-31`) — the gate *is* the approval record for builder-authored plans. None of the existing merged plans in `codev/plans/` carry that frontmatter either (verified: `0001`–`0009` start with a `#` heading / `## Metadata`, no `approved:` key). Adding a self-authored `approved:`/`validated:` block would *fabricate* an approval record that porch already holds authoritatively. No code change warranted. **Escalated to the human at the `pr` gate per PIR single-pass policy.** + ## How to Test Locally For reviewers pulling the branch: diff --git a/codev/state/pir-929_thread.md b/codev/state/pir-929_thread.md index 36c55ab40..15d9648d8 100644 --- a/codev/state/pir-929_thread.md +++ b/codev/state/pir-929_thread.md @@ -62,3 +62,7 @@ Architect reviewed the running integration and returned a punch list. All addres - **NIT 3**: `.gemini/settings.json` added to both root `.gitignore` and the managed adopter `CODEV_GITIGNORE_ENTRIES` (gitignore.ts); gitignore/update test expectations updated. Full suite: 3338 passed, 48 skipped, 0 failures. Build green. Re-running `porch done` for a fresh single CMAP-2 pass, then re-requesting the pr gate. + +Porch quirk: re-running `porch done` landed straight back at `gate_pending` (iter1) without re-issuing consult tasks — because the architect's REQUEST_CHANGES arrived via `afx send`, not a porch gate rejection, so the iteration never advanced. Ran a fresh advisory 2-way consult manually on the updated diff (iter2 outputs) since porch wouldn't re-trigger it. + +iter2 verdicts: gemini=APPROVE (HIGH), codex=REQUEST_CHANGES (MEDIUM, false positive: "plan lacks approval frontmatter"). Rebutted — the frontmatter convention is for architect-pre-approved artifacts; this plan was builder-authored and approved via porch's `plan-approval` gate (status.yaml), and no merged plan in codev/plans (0001–0009) carries that frontmatter. Documented the rebuttal in the review's "Consultation Findings & Dispositions" section; escalating to the human at the pr gate. Gate remains pending. From b78517865d2a9e9ef18c2f165105ce1103072f76 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 15:49:59 -0400 Subject: [PATCH 18/26] chore(porch): 929 record PR #1059 --- .../projects/929-support-codex-and-gemini-clis-/status.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index e05ca69e2..6559289d0 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -20,10 +20,14 @@ iteration: 1 build_complete: true history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-06-16T19:40:32.875Z' +updated_at: '2026-06-16T19:49:59.539Z' pr_history: - phase: review pr_number: 1 branch: builder/pir-929 created_at: '2026-06-16T19:10:45.900Z' + - phase: review + pr_number: 1059 + branch: builder/pir-929 + created_at: '2026-06-16T19:49:59.539Z' pr_ready_for_human: true From 07fe3f72825a9bbc9082a002934617a5e7716e50 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 15:50:20 -0400 Subject: [PATCH 19/26] [PIR #929] Thread: architect APPROVE + PR retargeted upstream (cluesmith#1059, no self-merge) Co-Authored-By: Claude Opus 4.7 --- codev/state/pir-929_thread.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/codev/state/pir-929_thread.md b/codev/state/pir-929_thread.md index 15d9648d8..c6f16196d 100644 --- a/codev/state/pir-929_thread.md +++ b/codev/state/pir-929_thread.md @@ -66,3 +66,11 @@ Full suite: 3338 passed, 48 skipped, 0 failures. Build green. Re-running `porch Porch quirk: re-running `porch done` landed straight back at `gate_pending` (iter1) without re-issuing consult tasks — because the architect's REQUEST_CHANGES arrived via `afx send`, not a porch gate rejection, so the iteration never advanced. Ran a fresh advisory 2-way consult manually on the updated diff (iter2 outputs) since porch wouldn't re-trigger it. iter2 verdicts: gemini=APPROVE (HIGH), codex=REQUEST_CHANGES (MEDIUM, false positive: "plan lacks approval frontmatter"). Rebutted — the frontmatter convention is for architect-pre-approved artifacts; this plan was builder-authored and approved via porch's `plan-approval` gate (status.yaml), and no merged plan in codev/plans (0001–0009) carries that frontmatter. Documented the rebuttal in the review's "Consultation Findings & Dispositions" section; escalating to the human at the pr gate. Gate remains pending. + +### 2026-06-16 — Architect re-review: APPROVE + PR retargeted upstream + +Architect verified all 5 integration-review items at source (affected tests 193/193 green) and confirmed codex's frontmatter finding is a false positive (rebuttal sound). **APPROVE.** + +PR retargeting: this is an **upstream contribution to cluesmith/codev**. Architect opened the correct cross-fork PR → **cluesmith/codev#1059** (head `mohidmakhdoomi:builder/pir-929` → base `cluesmith/codev:main`). Fork PR #1 CLOSED as superseded (wrong base = fork main). + +**MERGE POLICY: pull-only on cluesmith — do NOT self-merge. A cluesmith maintainer merges #1059.** The porch pr gate is the human's to approve as bookkeeping only (no `gh pr merge` from me). Bookkeeping done: reset gh default repo → cluesmith, recorded PR #1059 in porch pr_history. Nothing further to push unless review feedback lands on #1059. Waiting at the pr gate; on gate approval I will NOT merge — I'll record/close out per the no-self-merge policy. From 1f4bfd30cca862c69d6bdd51c6807366225fe6de Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 20:10:40 -0400 Subject: [PATCH 20/26] =?UTF-8?q?[PIR=20#929]=20Doc:=20caveat=20=E2=80=94?= =?UTF-8?q?=20unrecognized=20override=20commands=20default=20to=20claude?= =?UTF-8?q?=20harness=20(cluesmith/codev#1062)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3-way re-review APPROVE-with-caveat. Codex's verified finding: resolveHarness defaults an unrecognized override command (e.g. TOWER_ARCHITECT_CMD=bash) to CLAUDE_HARNESS when no explicit shell.architectHarness/builderHarness is set, so it can still attempt resume against a stale claude jsonl. Pre-existing and narrow (not a #929 regression). Documented in arch.md + review; follow-up #1062. Co-Authored-By: Claude Opus 4.7 --- codev/resources/arch.md | 2 ++ codev/reviews/929-support-codex-and-gemini-clis-.md | 1 + codev/state/pir-929_thread.md | 8 ++++++++ 3 files changed, 11 insertions(+) diff --git a/codev/resources/arch.md b/codev/resources/arch.md index b9490738e..78c543a21 100644 --- a/codev/resources/arch.md +++ b/codev/resources/arch.md @@ -271,6 +271,8 @@ A `codev doctor` audit (`lib/framework-ref-audit.ts`) flags shell-fetch of frame **Supported architect harnesses** (Issue #929): claude, codex, and gemini are all supported as architects, selected via `.codev/config.json` (`shell.architect` / `shell.architectHarness`) — the same config-driven mechanism builders use, and the *recommended* one. Harness auto-detection is **override-aware**: `getArchitectHarness` / `getBuilderHarness` resolve the harness from the override-aware command (`getResolvedCommands` → `cliOverrides` / `TOWER_ARCHITECT_CMD` / config), so a `--architect-cmd gemini` / `TOWER_ARCHITECT_CMD=gemini` / `--builder-cmd gemini` with no matching harness config still resolves the *gemini* harness, not claude. (Before #929 it auto-detected from the raw config value only — an override launched gemini but resolved the claude harness, re-arming the resume crash-loop below.) An explicit `shell.architectHarness` / `shell.builderHarness` still wins over auto-detection. OpenCode remains builder-only (file-based injection needs an ephemeral worktree). Gemini additionally gets a `.gemini/settings.json` (`context.fileName` → `AGENTS.md`) written if-absent at launch via the harness's `getArchitectFiles`, so it reads project context the way codex reads `AGENTS.md` natively. +> **Caveat — unrecognized override commands still default to the claude harness (tracked in cluesmith/codev#1062).** `#929`'s override-awareness only covers *recognized* harness commands (claude/codex/gemini/opencode, matched by `detectHarnessFromCommand`). An override command the detector does **not** recognize — e.g. `TOWER_ARCHITECT_CMD=bash`, a wrapper script, or any custom launcher — with **no** explicit `shell.architectHarness` / `shell.builderHarness` falls through `resolveHarness` to the **claude** harness (`harness.ts`, the final `return CLAUDE_HARNESS`). With a stale Claude `.jsonl` present, that can still build ` --resume ` for the unrecognized command. This is **pre-existing and narrow** (not a #929 regression — #929 strictly *improved* the recognized codex/gemini cases) and separable. Mitigation today: set an explicit `shell.architectHarness` / `shell.builderHarness` when using an unrecognized launcher command. + **Conversation resume is Claude-main-only.** `launchInstance` resumes a prior session only when the configured harness implements `HarnessProvider.buildResume` — currently just claude (its sessions live at `~/.claude/projects//*.jsonl`). Codex/gemini architects (and resumed codex/gemini builders, via `spawn.ts` `discoverResumeSession`) return `null` from `buildResume` and relaunch fresh with role injection. This gating fixes a latent crash-loop where a non-Claude harness + a stale Claude `.jsonl` built an invalid ` --resume ` invocation and shellper restart-looped to death. **`getArchitectFiles` is centralized in `buildArchitectArgs`** (`tower-utils.ts`), the shared role-injection helper every architect-launch path routes through — `launchInstance` (fresh), `add-architect` (sibling), shellper reconnect (×2), and the no-Tower `afx architect` (refactored in #929 to call `buildArchitectArgs` instead of duplicating injection). So Gemini's context manifest is written on **every** launch path, not just first-activation. diff --git a/codev/reviews/929-support-codex-and-gemini-clis-.md b/codev/reviews/929-support-codex-and-gemini-clis-.md index f99937aac..0c2990030 100644 --- a/codev/reviews/929-support-codex-and-gemini-clis-.md +++ b/codev/reviews/929-support-codex-and-gemini-clis-.md @@ -65,6 +65,7 @@ No **HOT** (`lessons-critical.md`) change: the existing "Single source of truth - **`getArchitectFiles` write-if-absent** (`tower-instances.ts`): writes `.gemini/settings.json` only when the target path doesn't exist, so a user's existing file is never clobbered. Test covers both the write and the no-clobber path. - **Override-aware harness resolution precedence (BLOCKER B fix)**: `getResolvedCommands.architect` is now `cliOverrides.architect || TOWER_ARCHITECT_CMD || config`. Within the Tower process `cliOverrides` is empty (it's set in the spawning `afx` process, not the long-lived server), so this matches the launch site's `TOWER_ARCHITECT_CMD || config`. An explicit `shell.architectHarness` / `shell.builderHarness` still wins over auto-detection by design — so a *deliberately* contradictory `architectHarness: claude` + gemini command is the user's call, not auto-resolved. Worth a sanity check that this precedence reads as intended. - **`getArchitectFiles` centralization (BLOCKER A fix)**: moved the inline write out of `launchInstance` into the shared `buildArchitectArgs` (`writeArchitectContextFiles`), and refactored the no-Tower `architect.ts` to call `buildArchitectArgs` instead of duplicating role injection. Confirm the no-Tower path's arg shape is unchanged (covered by `af-architect.test.ts`) and that the claude resume path — the one path that does *not* call `buildArchitectArgs` — correctly needs no context files (claude has no `getArchitectFiles`). +- **Known caveat — unrecognized override commands still default to the claude harness (follow-up: cluesmith/codev#1062).** Surfaced by codex in the 3-way re-review and verified by the architect. The BLOCKER B fix makes *recognized* override commands (claude/codex/gemini/opencode) resolve the right harness, but `resolveHarness` (`harness.ts`) still falls through to `CLAUDE_HARNESS` for an **unrecognized** override command (e.g. `TOWER_ARCHITECT_CMD=bash` or a wrapper script) when no explicit `shell.architectHarness` / `shell.builderHarness` is set — so it can still attempt ` --resume ` against a stale Claude `.jsonl`. This is **pre-existing, narrow, and separable** (not a #929 regression — #929 strictly improved the recognized cases), dispositioned as document + follow-up. Mitigation: set an explicit harness for unrecognized launcher commands. Documented in `arch.md`'s #929 subsection; tracked in cluesmith/codev#1062. ## Consultation Findings & Dispositions diff --git a/codev/state/pir-929_thread.md b/codev/state/pir-929_thread.md index c6f16196d..f5497a01d 100644 --- a/codev/state/pir-929_thread.md +++ b/codev/state/pir-929_thread.md @@ -74,3 +74,11 @@ Architect verified all 5 integration-review items at source (affected tests 193/ PR retargeting: this is an **upstream contribution to cluesmith/codev**. Architect opened the correct cross-fork PR → **cluesmith/codev#1059** (head `mohidmakhdoomi:builder/pir-929` → base `cluesmith/codev:main`). Fork PR #1 CLOSED as superseded (wrong base = fork main). **MERGE POLICY: pull-only on cluesmith — do NOT self-merge. A cluesmith maintainer merges #1059.** The porch pr gate is the human's to approve as bookkeeping only (no `gh pr merge` from me). Bookkeeping done: reset gh default repo → cluesmith, recorded PR #1059 in porch pr_history. Nothing further to push unless review feedback lands on #1059. Waiting at the pr gate; on gate approval I will NOT merge — I'll record/close out per the no-self-merge policy. + +### 2026-06-17 — 3-way re-review: APPROVE-with-caveat (doc-only addendum) + +Architect ran a 3-way re-review: gemini APPROVE, claude APPROVE, codex REQUEST_CHANGES (2/3). Verdict: **APPROVE with one documented caveat**. + +Codex's lone (verified) finding: `resolveHarness` (harness.ts) defaults an *unrecognized* override command (e.g. `TOWER_ARCHITECT_CMD=bash`, a wrapper script) to `CLAUDE_HARNESS` when no explicit `shell.architectHarness`/`builderHarness` is set → can still build ` --resume ` against a stale claude jsonl. **Pre-existing, narrow, separable — not a #929 regression** (#929 strictly improved the recognized codex/gemini cases). Disposition: document + follow-up (cluesmith/codev#1062). + +Doc-only addendum (no code change, per architect instruction): added the caveat to arch.md's #929 subsection and the review's caveat section, noting the unrecognized-command default + mitigation (set explicit harness) + the #1062 follow-up. Commit + push flows into PR #1059; no gate re-request (doc addendum to an already-approved PR). Still no self-merge — maintainer merges #1059. From f337b61fa069bd93eb77b495b4070b6b41bf4ec0 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 20:16:21 -0400 Subject: [PATCH 21/26] chore(porch): 929 pr gate-approved --- .../929-support-codex-and-gemini-clis-/status.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index 6559289d0..7296fc508 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -14,13 +14,14 @@ gates: requested_at: '2026-06-16T18:58:35.321Z' approved_at: '2026-06-16T19:03:40.955Z' pr: - status: pending + status: approved requested_at: '2026-06-16T19:14:05.958Z' + approved_at: '2026-06-17T00:16:21.120Z' iteration: 1 build_complete: true history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-06-16T19:49:59.539Z' +updated_at: '2026-06-17T00:16:21.121Z' pr_history: - phase: review pr_number: 1 @@ -30,4 +31,4 @@ pr_history: pr_number: 1059 branch: builder/pir-929 created_at: '2026-06-16T19:49:59.539Z' -pr_ready_for_human: true +pr_ready_for_human: false From 10d739e4a99a12e7db637f99f4e4602c85f5ed73 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 20:17:38 -0400 Subject: [PATCH 22/26] chore(porch): 929 protocol complete --- codev/projects/929-support-codex-and-gemini-clis-/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index 7296fc508..fd2ca010d 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -1,7 +1,7 @@ id: '929' title: support-codex-and-gemini-clis- protocol: pir -phase: review +phase: verified plan_phases: [] current_plan_phase: null gates: @@ -21,7 +21,7 @@ iteration: 1 build_complete: true history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-06-17T00:16:21.121Z' +updated_at: '2026-06-17T00:17:38.200Z' pr_history: - phase: review pr_number: 1 From f9c28cef931e0c34d0e6dc0e8251c4912aee56d5 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 20:18:12 -0400 Subject: [PATCH 23/26] [PIR #929] Thread: pr gate approved, protocol complete, not self-merged (awaiting maintainer on #1059) Co-Authored-By: Claude Opus 4.7 --- codev/state/pir-929_thread.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/codev/state/pir-929_thread.md b/codev/state/pir-929_thread.md index f5497a01d..0601a62dc 100644 --- a/codev/state/pir-929_thread.md +++ b/codev/state/pir-929_thread.md @@ -82,3 +82,7 @@ Architect ran a 3-way re-review: gemini APPROVE, claude APPROVE, codex REQUEST_C Codex's lone (verified) finding: `resolveHarness` (harness.ts) defaults an *unrecognized* override command (e.g. `TOWER_ARCHITECT_CMD=bash`, a wrapper script) to `CLAUDE_HARNESS` when no explicit `shell.architectHarness`/`builderHarness` is set → can still build ` --resume ` against a stale claude jsonl. **Pre-existing, narrow, separable — not a #929 regression** (#929 strictly improved the recognized codex/gemini cases). Disposition: document + follow-up (cluesmith/codev#1062). Doc-only addendum (no code change, per architect instruction): added the caveat to arch.md's #929 subsection and the review's caveat section, noting the unrecognized-command default + mitigation (set explicit harness) + the #1062 follow-up. Commit + push flows into PR #1059; no gate re-request (doc addendum to an already-approved PR). Still no self-merge — maintainer merges #1059. + +### 2026-06-16/17 — pr gate APPROVED (human-instructed); protocol complete; NOT self-merged + +Human instructed `git pull` + `porch approve 929 pr --a-human-explicitly-approved-this`. `git pull` failed to fast-forward the local branch (origin fetch=cluesmith lacks builder/pir-929, which lives on the fork) but did fetch updates. `porch approve` passed all checks and committed the gate-approved state locally, but its `git push -u origin HEAD` was **rejected**: the architect had merged `main` into the fork's builder/pir-929 (commit 0fd70b72) to keep #1059 current, so the fork was ahead. Resolved by rebasing the single unpushed porch chore commit onto `fork/builder/pir-929` (only touches 929 status.yaml → no conflict) and pushing — fork now at `f337b61f`. Gate status: **approved**; `porch next 929` → status: complete, phase: verified. Per the standing pull-only instruction I did **NOT** run `gh pr merge` / pr-merge.sh — a cluesmith maintainer merges #1059. Architect notified. Project complete pending the upstream merge + architect-driven cleanup. From 780fcdc300f92f4b6eb00ba4cbc85ff4ef552031 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 22:34:59 -0400 Subject: [PATCH 24/26] =?UTF-8?q?chore(porch):=20929=20rollback=20verified?= =?UTF-8?q?=20=E2=86=92=20plan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../status.yaml | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index fd2ca010d..35bf952dc 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -1,27 +1,21 @@ id: '929' title: support-codex-and-gemini-clis- protocol: pir -phase: verified +phase: plan plan_phases: [] current_plan_phase: null gates: plan-approval: - status: approved - requested_at: '2026-05-31T19:10:01.535Z' - approved_at: '2026-05-31T19:34:05.482Z' + status: pending dev-approval: - status: approved - requested_at: '2026-06-16T18:58:35.321Z' - approved_at: '2026-06-16T19:03:40.955Z' + status: pending pr: - status: approved - requested_at: '2026-06-16T19:14:05.958Z' - approved_at: '2026-06-17T00:16:21.120Z' + status: pending iteration: 1 -build_complete: true +build_complete: false history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-06-17T00:17:38.200Z' +updated_at: '2026-06-17T02:34:59.005Z' pr_history: - phase: review pr_number: 1 From 3082c186afa77b44a95fb7078c501abc67b288ae Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 22:44:58 -0400 Subject: [PATCH 25/26] [PIR #929] Plan revised: agy (Antigravity CLI) replaces gemini for architects --- .../929-support-codex-and-gemini-clis-.md | 368 ++++++++++-------- codev/state/pir-929_thread.md | 25 ++ 2 files changed, 240 insertions(+), 153 deletions(-) diff --git a/codev/plans/929-support-codex-and-gemini-clis-.md b/codev/plans/929-support-codex-and-gemini-clis-.md index e427e9f37..00cdbc699 100644 --- a/codev/plans/929-support-codex-and-gemini-clis-.md +++ b/codev/plans/929-support-codex-and-gemini-clis-.md @@ -1,173 +1,235 @@ -# PIR Plan: Support `codex` and `gemini` CLIs as architects +# PIR Plan: Support `codex` and `agy` (Antigravity CLI) as architects -## Understanding - -The harness abstraction (#591) already routes per-CLI role injection for claude/codex/gemini, and builders already run all three engines config-driven. The issue asks to bring **architects** to parity — selectable via `shell.architect` / `shell.architectHarness` in `.codev/config.json`. - -The blocking defect is a **latent resume crash-loop for non-Claude harnesses**, present at two sites with one root cause: session-discovery and `--resume` argument construction are hard-coded to Claude's on-disk session store, never gated on the configured harness. - -### Root cause (verified) - -`findLatestSessionId()` (`packages/codev/src/agent-farm/utils/claude-session-discovery.ts:43`) reads **only** `~/.claude/projects//*.jsonl`. Two callers feed its result straight into a ` --resume ` invocation without checking which CLI `` is: +> **Rescoped 2026-06-17** — `agy` (Antigravity CLI) replaces the retiring `gemini` CLI (#778) as the +> non-Claude architect. PR #1059 already landed the harness-agnostic resume fix, `codex` architect +> parity, and the `getArchitectFiles` context seam. This revision delivers the **agy delta**: add an +> `AGY_HARNESS`, swap `gemini` out of architect support, and update tests + docs. -1. **Architect** — `packages/codev/src/agent-farm/servers/tower-instances.ts:500` - ```ts - const resumeSessionId = safeToResume ? findLatestSessionId(workspacePath) : null; - ... - cmdArgs = [...cmdParts.slice(1), '--resume', resumeSessionId]; // line 507 - ``` - If `shell.architect` is `codex`/`gemini` **and** any prior Claude session exists for that workspace dir (true for anyone who has run Claude Code there — including this repo), `afx workspace start` launches e.g. `codex --resume ` → invalid invocation → shellper restart-loops (`maxRestarts: 50`, `tower-instances.ts:538`) to death. - -2. **Builder** — `packages/codev/src/agent-farm/commands/spawn.ts:83` (`discoverResumeSession()`, called at `:459` and `:838`) returns a Claude UUID on any `--resume`; `packages/codev/src/agent-farm/commands/spawn-worktree.ts:739-751` then bakes `${baseCmd} --resume "${resumeSessionId}"` into the launch script's restart loop. `packages/codev/src/agent-farm/commands/workspace-recover.ts:254` re-enters via `afx spawn --resume`, inheriting the same bug. A resumed codex/gemini builder crash-loops identically. (Fresh builders never take this branch — which is why "builders already prove the path" does not cover resume.) +## Understanding -The fresh-launch paths are already correct: `buildArchitectArgs()` (`tower-utils.ts:176`) and `startBuilderSession()`'s role-injection branch (`spawn-worktree.ts:752-786`) both resolve the harness via `getArchitectHarness` / `getBuilderHarness` (`config.ts:252,267`) and inject per-CLI flags. Siblings (`add-architect`) and reconnect already route through `buildArchitectArgs`. **Only the resume seam is harness-blind.** +PR #1059 (on this branch, not yet merged) already delivered, and we **keep**, the parts that are not +gemini-specific: + +- **Harness-agnostic resume crash-loop fix** — the `HarnessProvider.buildResume` seam. Claude returns + the resume invocation; every other harness returns `null` → fresh, role-injected launch. Applied at + both the architect site (`tower-instances.ts:517`) and the builder `--resume` site + (`spawn.ts` / `spawn-worktree.ts`). This is engine-neutral and stays exactly as-is. +- **`codex` architect parity** — codex is unaffected by the gemini retirement; its `-c + model_instructions_file=` role injection and fresh-launch path are kept. +- **The `getArchitectFiles` context-injection seam** in `HarnessProvider` (`harness.ts:74`) + its + shared writer `writeArchitectContextFiles` (`tower-utils.ts:181`, called from `buildArchitectArgs`). + Built for gemini; the *seam* is reused, but agy turns out **not to need it** (see the finding below). + +The **delta**: the Gemini CLI is retired 2026-06-18. Its replacement is Google's Antigravity CLI +(`agy`), which Codev's `consult` lane already uses (spec/plan 778). So the non-Claude *architect* must +land on `agy`, not the dying `gemini` CLI. Decision (architect, do not relitigate): **swap** — `agy` +replaces `gemini` as an architect; `GEMINI_HARNESS` stays for *builders* but gemini is no longer +offered/affirmed as an architect. Resume stays deferred (agy `buildResume` → `null`, fresh launch). + +### 🔑 Key finding — agy's role/context mechanism differs from gemini's (flagged per the brief) + +I confirmed agy's mechanism from `agy --help`, the `agy` binary's embedded strings, and the existing +`consult` agy lane (`commands/consult/index.ts:614-833`). **agy is closer to `codex` than to the +retired gemini CLI**: + +1. **Project context: agy reads `AGENTS.md` natively** (no settings/pointer file needed). The binary's + embedded guidance states rules are appended to *"`AGENTS.md` in the Workspace Customizations Root"* + (the workspace = the architect's cwd) and a *Global Customizations Root* (`~/.gemini`). This is the + **codex pattern** — codex also reads `AGENTS.md` natively. The retired gemini CLI needed + `.gemini/settings.json` → `context.fileName: AGENTS.md` to be *pointed* at the manifest; **agy does + not.** ⇒ **agy needs no `getArchitectFiles`.** The repo already ships `AGENTS.md`, so an agy + architect gets project context for free. + +2. **Role injection: agy has no `--append-system-prompt` flag and no `GEMINI_SYSTEM_MD`-style env var.** + (`agy --help` exposes no system-prompt flag; the consult lane confirms *"agy has no system-prompt + flag — fold the role into the prompt"*, `consult/index.ts:785`.) The architect role + (`.architect-role.md`) is distinct from the committed `AGENTS.md` and must **not** clobber it. + agy's purpose-built channel for this is **`-i` / `--prompt-interactive`** — *"Run an initial prompt + interactively and continue the session"* (`agy --help`; called out in the issue as the architect + launch shape). So agy injects the role by **folding it into the interactive launch prompt** + (`agy --dangerously-skip-permissions --prompt-interactive ""`), mirroring the consult + "hermes precedent" rather than the gemini `.gemini/settings.json` file pattern the brief + hypothesized. + +3. **Binary resolution:** a bare `agy` on `PATH` may be the Antigravity **IDE launcher symlink** + (resolves to the Electron app), not the headless CLI — launching it opens the GUI and never produces + an architect session. The consult lane already guards this with `resolveAgyBin()` / `isRealAgyCli()` + / `agyRespondsToVersion()` (`consult/index.ts:644-696`), preferring `~/.local/bin/agy` and + realpath-rejecting the IDE bundle. The architect launch must reuse this. + +**Net divergence from the brief's hypothesis:** the brief expected agy role injection via a +`getArchitectFiles` settings file (gemini-style). In reality agy reads `AGENTS.md` natively (so no +context file is needed) and takes the role via `--prompt-interactive` (so no settings file is involved +at all). The `getArchitectFiles` *seam* stays in the interface (kept from #1059) but agy doesn't +implement it; gemini's implementation is removed with the swap. This is the one item the architect +asked me to surface, and it's empirically re-verified at the `dev-approval` gate (PIR's purpose). ## Proposed Change -Add an optional capability to the `HarnessProvider` interface that encapsulates session discovery. Only Claude implements it; every other harness gets a fresh launch. - -### 1. New `HarnessProvider` capability (`packages/codev/src/agent-farm/utils/harness.ts`) - -Add **one** optional method that bundles discovery + both invocation forms. This mirrors the convention the interface already uses for role injection — `buildRoleInjection()` returns Node argv for `spawn()` call sites (`harness.ts:24`) and `buildScriptRoleInjection()` returns a shell-**escaped** bash fragment (`harness.ts:34`). Bundling discovery and invocation into a single method means the call sites never see an independently-optional second method (no non-null assertion `!`), and the builder bash path gets a pre-escaped fragment instead of word-splitting a raw argv array: - -```ts -/** - * Optional: discover a resumable prior session for the given working dir and - * return how to resume it — in BOTH forms, mirroring buildRoleInjection / - * buildScriptRoleInjection: - * - args: Node argv for spawn() call sites (architect / tower-instances) - * - scriptFragment: shell-escaped fragment for bash script generation (builder) - * Returns null when no resumable session exists or this harness has no - * cwd-keyed session store → callers fall back to a fresh launch. Only Claude - * implements it (store: ~/.claude/projects//.jsonl). - */ -buildResume?(absolutePath: string, opts?: { homeDir?: string }): { - sessionId: string; - args: string[]; - scriptFragment: string; -} | null; -``` - -`CLAUDE_HARNESS` implements it: - -```ts -buildResume: (absolutePath, opts) => { - const sessionId = findLatestSessionId(absolutePath, opts); - if (!sessionId) return null; - return { - sessionId, - args: ['--resume', sessionId], - scriptFragment: `--resume '${shellEscapeSingleQuote(sessionId)}'`, - }; -}, -``` - -`CODEX_HARNESS`, `GEMINI_HARNESS`, `OPENCODE_HARNESS`, and custom harnesses leave `buildResume` **undefined** → callers do `harness.buildResume?.(path) ?? null` → fresh launch. This is the "claude returns the resume invocation; codex/gemini return null" seam the issue and analysis §4.1 call for, with the Node-vs-script split the existing role-injection methods already establish. (Session ids are bare UUIDs so escaping is belt-and-suspenders today, but it keeps the bash path correct-by-construction and consistent with `buildScriptRoleInjection`'s `:769`/`:858` escaping.) - -### 2. Architect site (`tower-instances.ts:500-509`) - -Replace the unconditional `findLatestSessionId(workspacePath)` with the harness capability (Node-argv form), preserving the existing `safeToResume` sibling-collision guard: +### 1. Add `AGY_HARNESS` (`agent-farm/utils/harness.ts`) ```ts -const architectHarness = getArchitectHarness(workspacePath); -const resume = safeToResume ? (architectHarness.buildResume?.(workspacePath) ?? null) : null; -... -if (resume) { - cmdArgs = [...cmdParts.slice(1), ...resume.args]; - harnessEnv = {}; - _deps.log('INFO', `Resuming main architect session ${resume.sessionId.slice(0, 8)}… for ${workspacePath}`); -} else { - const built = buildArchitectArgs(cmdParts.slice(1), workspacePath); - cmdArgs = built.args; harnessEnv = built.env; -} +export const AGY_HARNESS: HarnessProvider = { + // agy has no system-prompt flag; fold the role into the interactive launch + // prompt (`--prompt-interactive ""`). The role rides as agy's initial + // turn; project context comes from AGENTS.md, read natively (like codex). + buildRoleInjection: (content, _filePath) => ({ + args: ['--prompt-interactive', content], + env: {}, + }), + buildScriptRoleInjection: (_content, filePath) => ({ + fragment: `--prompt-interactive "$(cat '${shellEscapeSingleQuote(filePath)}')"`, + env: {}, + }), + // Resume deferred (MVP): agy supports --continue/--conversation, but wiring + // real resume is a follow-up. Leaving buildResume undefined → fresh launch. + // No getArchitectFiles: agy reads AGENTS.md natively (no pointer file needed). + resolveBinary: (cmd) => resolveAgyBin() ?? cmd, +}; ``` -Update the now-inaccurate "if a prior **Claude** session exists" comment to note the harness gate. (`getArchitectHarness` already resolves from `.codev/config.json` `shell.architect`/`architectHarness`, identical to `buildArchitectArgs`, so a codex/gemini config yields `undefined` → fresh, role-injected launch.) - -### 3. Builder site (`spawn.ts:83`, callers `:459`/`:838`; consumed in `spawn-worktree.ts:739-751`) - -Thread the builder harness into `discoverResumeSession`, gate on it, and return the bundled resume object (carrying the escaped `scriptFragment`) so the bash generator never re-derives the flag: - -```ts -export function discoverResumeSession( - worktreePath: string, - isResume: boolean | undefined, - harness: HarnessProvider, -): { sessionId: string; args: string[]; scriptFragment: string } | undefined { - if (!isResume) return undefined; - const resume = harness.buildResume?.(worktreePath) ?? null; - if (resume) { logger.kv('Session', `${resume.sessionId.slice(0, 8)}… (resuming conversation)`); return resume; } - logger.info('No prior conversation found for this worktree; starting a fresh session.'); - return undefined; -} -``` - -Both callers pass `getBuilderHarness(config.workspaceRoot)` and forward the result into `startBuilderSession`, whose `resumeSessionId?: string` param becomes `resume?: { scriptFragment: string }`. In the resume branch (`spawn-worktree.ts:739-751`) the script line becomes `${baseCmd} ${resume.scriptFragment}` — a single pre-escaped fragment, **not** a `.join(' ')` of a raw argv array (which would word-split / comma-stringify). When the harness returns `undefined` (codex/gemini), `startBuilderSession` takes the fresh role-injection path and the `--resume` restart loop is never reached. `workspace-recover.ts` inherits the fix for free (it shells out to `afx spawn --resume`). The existing `buildResumeNotice` prompt still prepends for fresh-launched resumed builders (`spawn.ts:462`). - -### 4. Tests — placed at the layer where each bug actually lives - -The two crash-loops live in **Tower launch** (`tower-instances.ts`) and **builder script generation** (`spawn-worktree.ts`), so the regression guards must sit there. `af-architect.test.ts` only exercises the no-Tower `afx architect` command (`spawn()` of the local session) — a resume-skip test there would **not** guard the real architect regression. Layering: - -- **`packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts`** (unit, keep — correctly placed) — update calls to pass a harness; add cases asserting `CODEX_HARNESS`/`GEMINI_HARNESS` return `undefined` even when a stale Claude jsonl exists (`buildResume` undefined), and that `CLAUDE_HARNESS` returns `{ sessionId, args:['--resume',id], scriptFragment }` for the newest UUID. -- **`packages/codev/src/agent-farm/__tests__/tower-instances.test.ts`** (exists) — **architect regression guard**: stale Claude jsonl present + codex/gemini architect harness → asserts the launched command is the fresh `buildArchitectArgs` form (role-injected, harness env set) with **no `--resume`** in `cmdArgs`. Claude + stale jsonl → asserts `--resume ` is present (resume still works). -- **`packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts`** (exists) — **builder script-shape guard**: a resumed builder's generated `.builder-start.sh` uses the harness-provided escaped `scriptFragment` (e.g. `--resume ''`), not an unquoted or comma-joined argv; codex/gemini resumed builder → fresh role-injection script, no `--resume`. -- **`packages/codev/src/agent-farm/__tests__/af-architect.test.ts`** — keep Claude-only as is; optionally add codex/gemini `buildArchitectArgs` fresh-launch arg/env construction (codex `-c model_instructions_file=`, gemini `GEMINI_SYSTEM_MD`) as plain coverage, with an explicit note that this file does **not** guard the resume regression (that's `tower-instances.test.ts`). -- Run existing reconnect/sibling tests to confirm no regression (they already route through `buildArchitectArgs`). - -### 5. Gemini project-context manifest (promoted to MVP) - -A codex architect reads `AGENTS.md` natively for project context; a **gemini** architect ships no `GEMINI.md` (none exists in the repo, nor a `.gemini/` dir), so without help it launches with the injected *role* but no native project *manifesto*. The near-zero-effort fix is `.gemini/settings.json` with `context.fileName` → `AGENTS.md`, which points gemini's context loader at the manifest codex already uses. Promoting from nice-to-have to MVP so gemini doesn't ship context-blind: - -- Add an optional `getArchitectFiles?(workspacePath): Array<{ relativePath; content }>` to `HarnessProvider`, parallel to the existing `getWorktreeFiles?` (`harness.ts:44`). `GEMINI_HARNESS` returns `.gemini/settings.json` = `{ "context": { "fileName": "AGENTS.md" } }`. Others omit it. -- The architect launch path (`buildArchitectArgs` in `tower-utils.ts`, or its caller in `tower-instances.ts`) writes these files **only if missing** — never clobbering a user's existing `.gemini/settings.json`. Idempotent and merge-safe. -- Test in `tower-instances.test.ts` (or a small `harness.test.ts`): gemini architect with no `.gemini/settings.json` → file written with `context.fileName: "AGENTS.md"`; pre-existing file → left untouched. - -### 6. `doctor` / docs - -- No functional `doctor` change needed (`doctor.ts:594` already bars only OpenCode as architect; codex/gemini already pass). Add a short positive affirmation line that codex/gemini are supported architects. -- Document in the relevant resource doc (and CLAUDE.md / AGENTS.md if the architect section warrants it): codex/gemini are supported architects selected via `.codev/config.json` `shell.architect`/`shell.architectHarness`; **conversation resume is Claude-main-only** (codex/gemini architects relaunch fresh with role injection); `.codev/config.json` — not `TOWER_ARCHITECT_CMD`/`--architect-cmd`/`--builder-cmd` — is the supported harness-selection mechanism (see the two override-mismatch caveats in Risks). - -### 7. Submit-strategy hook (conditional — decided at the `dev-approval` gate) - -MVP item 3 (per-harness submit pacing / bracketed-paste in `message-write.ts`) is **conditional on empirical validation**. The current pacing (`message-write.ts:33` `writeMessageToSession`, tuned to Claude's TUI: single write + delayed Enter for ≤3 lines, line-by-line with 10ms gaps for >3) may or may not deliver cleanly on codex/gemini TUIs. If the human's `dev-approval` testing shows flaky multi-line/interrupt/streaming delivery, add an optional `getSubmitStrategy?()` to `HarnessProvider` and branch the writer on it. If delivery is clean, no change. The seam is designed but not implemented preemptively. +- Register `agy: AGY_HARNESS` in `BUILTIN_HARNESSES`. +- Extend `detectHarnessFromCommand` so a command whose basename includes `agy` resolves to `'agy'`. +- **Exact `-i` arg form** (`--prompt-interactive ` as a trailing positional vs `=`) is + confirmed at implement time via a safe, non-hanging probe; the consult lane treats agy's prompt as a + trailing positional after the mode flag (`--print … `), so `--prompt-interactive` is modeled + the same way (mode flag + positional). Re-verified live at `dev-approval`. + +### 2. Binary-resolution seam (reuse consult's guard) + +- **Extract** `resolveAgyBin`, `isRealAgyCli`, `agyRespondsToVersion` from `commands/consult/index.ts` + into a shared module `lib/agy-bin.ts` (no behavior change; consult re-imports from there — verify the + consult tests still pass). +- **Add** `resolveBinary?(cmd: string): string` to `HarnessProvider`. Default: harnesses omit it + (executable used as-is). `AGY_HARNESS` implements it via `resolveAgyBin() ?? cmd` (prefer the real + headless CLI; fall back to the configured token if resolution fails, so the user sees agy's own error + rather than a silent Claude-flag mismatch). +- **Apply** at every architect executable-determination site, resolving the harness first + (`getArchitectHarness(workspacePath)`): + - `agent-farm/commands/architect.ts:26` (no-Tower `afx architect`) + - `agent-farm/servers/tower-instances.ts:467` (main, workspace-start) and `:963` (sibling + `add-architect`) + - `agent-farm/servers/tower-terminals.ts:~657/671` and `:~879` (reconnect / auto-restart relaunch) + Each becomes `const cmd = harness.resolveBinary?.(cmdParts[0]) ?? cmdParts[0];`. The sprawl mirrors + the existing duplicated architect-command parsing; centralizing that parsing is explicitly **out of + scope** (issue nice-to-have "command-aware harness resolution"). + +### 3. Remove `gemini` from architect support (the swap) + +- **`doctor.ts:699-705`**: change the supported-architect affirmation from `codex`/`gemini` to + `codex`/`agy`. Move `gemini` into the barred/warn branch alongside `opencode` (configuring `gemini` + as architect now prints an "unsupported as architect; supported for builders" warning, pointing to + `agy` or `claude`). agy presence/auth is already checked elsewhere in doctor (the consult lane). +- **`GEMINI_HARNESS.getArchitectFiles`** (`harness.ts:135-138`): **remove** it. It is architect-only; + with gemini barred as architect it is dead code. `GEMINI_HARNESS`'s builder surface + (`buildRoleInjection`/`buildScriptRoleInjection` via `GEMINI_SYSTEM_MD`) is untouched. +- **`.gemini/settings.json` gitignore entry** (`lib/gitignore.ts:24`): **remove** — nothing writes that + file anymore. (`codev update`'s gitignore backfill only *appends*, so adopters' existing line is left + in place; no churn. Mirror the removal in `codev-skeleton/` if a copy exists.) + +### 4. Tests + +- **`harness.test.ts`** (or the relevant unit file): `AGY_HARNESS.buildRoleInjection` → + `['--prompt-interactive', ]`; `buildScriptRoleInjection` → escaped `--prompt-interactive + "$(cat …)"`; `buildResume` undefined; `getArchitectFiles` undefined; `resolveBinary` returns the + resolved bin (mock `resolveAgyBin`). `detectHarnessFromCommand('agy …') === 'agy'`. +- **`tower-utils.test.ts:207-239`**: replace the gemini `writeArchitectContextFiles` cases with agy — + assert **no** architect context file is written for agy (it has no `getArchitectFiles`), claude still + writes none, and `buildArchitectArgs` for agy yields `--prompt-interactive ` in `args` with + empty `env`. +- **`tower-instances.test.ts`**: architect resume-skip regression guard extended to agy (stale Claude + `.jsonl` present + agy architect → fresh `buildArchitectArgs` form, **no `--resume`**); claude still + resumes. Keep the codex case. +- **`af-architect.test.ts`**: add agy fresh-launch arg construction (`--prompt-interactive` role) + + `resolveBinary` substitution; keep the note that this file does not guard the resume regression. +- **gitignore tests** (`gitignore.test.ts`, `update.test.ts`): drop `.gemini/settings.json` from + expected entries. +- Mirror any gemini-architect test that becomes agy; **keep** gemini *builder* tests. + +### 5. Docs + +- **`codev/resources/arch.md`** #929 subsection: rewrite the gemini-architect specifics to describe + `agy` — selectable via `.codev/config.json` (`shell.architect: "agy …"` / `shell.architectHarness: + "agy"`); role injected via `--prompt-interactive` (no system-prompt flag); project context from + `AGENTS.md` read natively; binary resolved via `resolveAgyBin` (IDE-launcher guard); resume is + Claude-main-only. +- **`lessons-learned.md`** (COLD, if warranted): one line — "when porting a provider abstraction across + a CLI replacement, re-derive the new CLI's actual context/role mechanism from its `--help` + binary, + don't assume parity with the predecessor (agy reads `AGENTS.md` natively + injects role via `-i`, + unlike the retired gemini CLI's `GEMINI_SYSTEM_MD` + `.gemini/settings.json`)." Route by tier per + Spec 987; HOT files are capped, so this goes COLD unless it displaces a weaker lesson. +- **`CLAUDE.md` / `AGENTS.md`**: only if the architect-harness section needs the gemini→agy swap noted; + keep the two files byte-identical. + +### 6. Submit-strategy hook (conditional — decided at `dev-approval`) + +Unchanged from #1059's posture: if the human's `dev-approval` testing shows flaky multi-line / +interrupt / streaming `afx send` delivery on agy's TUI, add an optional `getSubmitStrategy?()` to +`HarnessProvider` and branch `message-write.ts`. Designed, not implemented preemptively. ## Files to Change -- `packages/codev/src/agent-farm/utils/harness.ts` — add `buildResume?` (bundled discovery + Node-argv + escaped script fragment) and `getArchitectFiles?` to the interface; implement `buildResume` in `CLAUDE_HARNESS` (delegates to `findLatestSessionId`, returns `args` + escaped `scriptFragment`); implement `getArchitectFiles` in `GEMINI_HARNESS` (`.gemini/settings.json`); import `findLatestSessionId` from `claude-session-discovery.ts`. -- `packages/codev/src/agent-farm/servers/tower-instances.ts:500-514` — gate architect resume on `getArchitectHarness(...).buildResume?.()` (Node-argv form); write `getArchitectFiles?()` if-missing on launch; update comment. -- `packages/codev/src/agent-farm/commands/spawn.ts:83` — `discoverResumeSession` takes `harness`, returns the bundled resume object. `:459`, `:838` — pass `getBuilderHarness(config.workspaceRoot)`, forward object to `startBuilderSession`. -- `packages/codev/src/agent-farm/commands/spawn-worktree.ts:724-751` — `startBuilderSession`'s `resumeSessionId?: string` → `resume?: {...scriptFragment}`; resume branch emits `${baseCmd} ${resume.scriptFragment}`. -- `packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts` — pass harness; codex/gemini null-return + claude bundled-object cases. -- `packages/codev/src/agent-farm/__tests__/tower-instances.test.ts` — architect resume-skip regression guard (codex/gemini + stale jsonl → no `--resume`); gemini `getArchitectFiles` write-if-missing. -- `packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts` — builder resume script uses escaped `scriptFragment`; codex/gemini resumed builder → fresh script. -- `packages/codev/src/agent-farm/__tests__/af-architect.test.ts` — (optional) codex/gemini fresh arg-construction coverage; note it does not guard the resume regression. -- `packages/codev/src/commands/doctor.ts` — affirm codex/gemini architect support. -- Docs: a resource doc (e.g. `codev/resources/arch.md` or commands ref) + CLAUDE.md/AGENTS.md note on resume-is-Claude-only + config-driven selection. -- *(Conditional)* `packages/codev/src/agent-farm/servers/message-write.ts` + `harness.ts` — submit strategy, only if `dev-approval` validation reveals flakiness. +- `packages/codev/src/agent-farm/utils/harness.ts` — add `AGY_HARNESS` (+ register, + `detectHarnessFromCommand`); add optional `resolveBinary?` to the interface; import `resolveAgyBin` from the new shared module; **remove** `GEMINI_HARNESS.getArchitectFiles`. +- `packages/codev/src/lib/agy-bin.ts` — **new**: `resolveAgyBin` / `isRealAgyCli` / `agyRespondsToVersion` extracted from consult. +- `packages/codev/src/commands/consult/index.ts` — re-import those three from `lib/agy-bin.ts` (no behavior change). +- `packages/codev/src/agent-farm/commands/architect.ts:26` — `resolveBinary` the executable. +- `packages/codev/src/agent-farm/servers/tower-instances.ts:467,963` — `resolveBinary` at main + sibling launch. +- `packages/codev/src/agent-farm/servers/tower-terminals.ts` (~657/671, ~879) — `resolveBinary` at reconnect/relaunch. +- `packages/codev/src/commands/doctor.ts:699-705` — affirm `codex`/`agy`; bar `gemini` as architect. +- `packages/codev/src/lib/gitignore.ts:24` — remove `.gemini/settings.json` (mirror in `codev-skeleton/` if present). +- Tests: `harness.test.ts`, `tower-utils.test.ts`, `tower-instances.test.ts`, `af-architect.test.ts`, `gitignore.test.ts`, `update.test.ts`, and the consult test that touches the extracted functions. +- Docs: `codev/resources/arch.md` (#929 subsection), optionally `lessons-learned.md`, `CLAUDE.md`/`AGENTS.md`. +- *(Conditional)* `message-write.ts` + `harness.ts` — submit strategy, only if `dev-approval` reveals flakiness. ## Risks & Alternatives Considered -- **Risk: `harness.ts` importing `claude-session-discovery.ts`.** The latter imports only Node builtins — no circular dependency. Low risk. -- **Risk: `TOWER_ARCHITECT_CMD`/`--architect-cmd` architect-override without matching `.codev/config.json`.** `getArchitectHarness` resolves the harness from config only, so an env-set `codex` with no config still resolves the claude harness → would still attempt resume. This is the issue's explicit **nice-to-have** ("command-aware harness resolution"); MVP fixes the config-driven path (which all acceptance criteria target) and **documents** the override caveat rather than expanding scope. -- **Risk: `--builder-cmd`/builder-env override without matching config — the exact builder analog.** The builder command comes from `getResolvedCommands()` (`spawn.ts:469`/`:840`, honors `--builder-cmd` and env), but the harness comes from `getBuilderHarness(config…)` (config only, `config.ts:267`). Set `--builder-cmd codex` with no `builderHarness`/`shell.builder` config and a resumed builder would still resolve the claude harness and attempt `codex --resume `. Same disposition as the architect override: documented, not fixed (config-driven `shell.builder`/`builderHarness` is the supported selection mechanism). -- **Risk: empirical acceptance criteria can't be unit-tested here.** Launch-on-stale-jsonl, add-architect, `afx send` multiline/interrupt/streaming, reconnect, affinity, dashboard scrollback all need codex/gemini actually installed. These are validated by the human running the worktree at the `dev-approval` gate — the reason PIR (not AIR/BUGFIX) was chosen. The unit tests cover the deterministic core (arg construction + resume-skip logic). -- **Alternative: a boolean `supportsResume` flag + keep `findLatestSessionId` and literal `--resume` at call sites.** Rejected — leaves the Claude-specific discovery *and* the `--resume` flag-shape spread across call sites. A single `buildResume()` returning both the Node-argv and the **escaped** script fragment moves both fully into the provider (mirroring `buildRoleInjection`/`buildScriptRoleInjection`), so no Claude-specific string and no shell-quoting decision survives at the call site. -- **Alternative: two independently-optional methods (`discoverResumeSession` + `buildResumeInvocation`).** Rejected — independent optionality forces a non-null assertion (`!`) at call sites and tempts a raw-argv `.join(' ')` into the bash script (word-splits / comma-stringifies on any arg with whitespace). Bundling them removes the `!` and guarantees the script form is pre-escaped. +- **Risk — agy treats the `-i` role as a first user turn.** `--prompt-interactive` runs the role as the + initial prompt and continues the session, so agy may "respond to" the role before the human types. + Acceptable: it's exactly what the flag is for, and the architect role reads as standing instructions. + Verified live at `dev-approval`. Mitigation if jarring: frame the injected content so agy treats it + as configuration (decided after observing real behavior). +- **Risk — large role doc as a CLI arg (ARG_MAX).** The architect role is a few KB, well under ARG_MAX; + the Node-spawn path passes it as one argv element, the script path `cat`s the file. Consult's + temp-file fallback is only needed for very large prompts; not required here. +- **Risk — `resolveAgyBin` returns null (agy not at a trusted path).** `resolveBinary` falls back to the + configured token, so the launch fails with agy's own error rather than silently mis-injecting. doctor + surfaces the install/auth state. Documented. +- **Risk — `TOWER_ARCHITECT_CMD`/`--architect-cmd` override without matching `.codev/config.json`.** + Pre-existing #1059 caveat (override-aware harness resolution already routes through + `getResolvedCommands`; an *unrecognized* override command still defaults to claude — tracked in + #1062). Out of scope; documented. +- **Alternative — implement agy role injection via `getArchitectFiles` writing `.gemini/settings.json` + (the brief's hypothesis).** Rejected after confirming agy reads `AGENTS.md` natively (no pointer file + needed) and exposes no settings key for a *separate* system-prompt file. A `.gemini/settings.json` + would be cargo-culted from the retired gemini CLI and wouldn't inject the role. +- **Alternative — append the role to the workspace `AGENTS.md`.** Rejected: clobbers the project's + committed instructions and pollutes git status. `--prompt-interactive` keeps the role ephemeral. +- **Alternative — add agy as a *builder* harness too.** Out of scope (issue is architect-only; builders + already have gemini via `GEMINI_HARNESS`). `AGY_HARNESS` still provides `buildScriptRoleInjection` for + interface completeness, but no builder wiring/tests are added. ## Test Plan -**Unit (run in this worktree, gate-verifiable from the diff):** -- `pnpm --filter @cluesmith/codev test` (or the af test subset) — new/updated `discover-resume-session.test.ts` + `af-architect.test.ts` green; existing reconnect/sibling/architect tests still green. -- Build: `pnpm --filter @cluesmith/codev build` clean (TS types for the new optional method). - -**Manual / empirical (reviewer at the `dev-approval` gate, needs codex & gemini installed). For each of codex and gemini set `shell.architect` accordingly in `.codev/config.json`:** -- `afx architect` (no-Tower) launches with role injected. +**Unit (this worktree, gate-verifiable from the diff):** +- `pnpm --filter @cluesmith/codev build` clean (new optional `resolveBinary` typing; new module). +- `pnpm --filter @cluesmith/codev test` — new agy harness/arg/resume-skip/`resolveBinary` cases green; + gemini *builder* tests still green; gitignore tests reflect the dropped entry; consult tests still + green after the extraction. + +**Manual / empirical (reviewer at the `dev-approval` gate; needs `agy` installed + signed in). Set +`shell.architect: "agy --dangerously-skip-permissions"` (and/or `shell.architectHarness: "agy"`) in +`.codev/config.json`:** +- `afx architect` (no-Tower) launches the **real** agy CLI (not the IDE) with the role visible as the + initial turn and `AGENTS.md` context available. - `afx workspace start` main architect launches on a **clean** cwd. -- `afx workspace start` main architect launches with a **stale Claude `.jsonl` present** in `~/.claude/projects//` — must NOT crash-loop (primary regression target). Confirm no `--resume` in the launched command (check the shellper/PTY command). -- `afx workspace add-architect` sibling launches. -- **gemini only**: after launch, `.gemini/settings.json` exists with `context.fileName: "AGENTS.md"` (and a pre-existing one was not clobbered). -- `afx send` delivers + submits: single-line, multi-line (>3 lines, no swallowed Enter), `--interrupt`, and while the TUI is streaming. (If flaky → triggers the conditional submit-strategy work.) -- Tower stop→start reconnect and shellper auto-restart both relaunch the architect. -- A builder spawned by a codex/gemini architect preserves `CODEV_ARCHITECT_NAME` affinity. -- `afx spawn --resume` on a non-Claude builder → fresh launch + resume notice (NOT `--resume `); inspect `.builder-start.sh`. -- Dashboard web-terminal scrollback survives heavy TUI redraw (nice-to-have; note if it regresses). +- `afx workspace start` main architect launches with a **stale Claude `.jsonl` present** in + `~/.claude/projects//` — must **not** crash-loop; confirm **no `--resume`** in the + launched command (the primary regression target, now exercised for agy). +- `afx workspace add-architect` sibling launches (real bin + role). +- `afx send` delivers + submits: single-line, multi-line (>3 lines, no swallowed Enter), `--interrupt`, + and while agy's TUI is streaming. (Flakiness → triggers the conditional submit-strategy work.) +- Tower stop→start reconnect and shellper auto-restart both relaunch the agy architect (real bin). +- A builder spawned by an agy architect preserves `CODEV_ARCHITECT_NAME` affinity. +- Confirm agy does **not** litter the worktree with an unignored `.gemini/`/`.agy/` dir; if it does, add + the appropriate gitignore entry as a follow-up edit. +- Dashboard web-terminal scrollback survives heavy agy TUI redraw (nice-to-have; note if it regresses). diff --git a/codev/state/pir-929_thread.md b/codev/state/pir-929_thread.md index 0601a62dc..913ed7033 100644 --- a/codev/state/pir-929_thread.md +++ b/codev/state/pir-929_thread.md @@ -2,6 +2,31 @@ ## Builder thread +### 2026-06-17 — RESCOPE: agy replaces gemini for architects (porch rewound to PLAN) + +Architect rewound porch to the plan phase. Issue #929 rescoped (#778): the Gemini CLI retires +2026-06-18; `agy` (Antigravity CLI) replaces it as the non-Claude **architect**. PR #1059 stays +open/unmerged; we extend it. Keep from #1059: the harness-agnostic `buildResume` seam, codex parity, +the `getArchitectFiles` seam. Delta: add `AGY_HARNESS`, swap gemini out of architect support. + +**Key investigation finding (flagged to architect):** agy's role/context mechanism is NOT gemini's — +it's codex-like. +- agy reads `AGENTS.md` **natively** (binary strings: "append to AGENTS.md in the Workspace + Customizations Root"; global root = `~/.gemini`). So agy needs **no** `getArchitectFiles` pointer + file (the retired gemini CLI needed `.gemini/settings.json` → context.fileName; agy doesn't). +- agy has **no** `--append-system-prompt` flag / no `GEMINI_SYSTEM_MD` env. Role injection rides + `-i`/`--prompt-interactive ""` (agy --help; consult's "hermes precedent"), folding the role + into the interactive launch prompt. So `AGY_HARNESS.buildRoleInjection` → `['--prompt-interactive', + roleContent]`. +- Binary resolution reuses consult's `resolveAgyBin`/`isRealAgyCli`/`agyRespondsToVersion` (bare `agy` + on PATH may be the IDE launcher symlink). Plan: extract those to `lib/agy-bin.ts`, add optional + `HarnessProvider.resolveBinary?`, apply at the ~6 architect executable-determination sites. + +Swap details: doctor affirms codex/`agy` (bars gemini as architect like opencode); remove +`GEMINI_HARNESS.getArchitectFiles` (architect-only dead code) + the `.gemini/settings.json` gitignore +entry; tests/docs cover agy instead of gemini; GEMINI_HARNESS stays for builders. `AGY_HARNESS.buildResume` +undefined → fresh launch (resume deferred). Plan rewritten; committing and sitting at plan-approval. + ### 2026-05-31 — Plan phase Investigated the resume crash-loop bug. Root cause confirmed across two sites, one mechanism: From a7934bda6096ab2f025a28ff235d9492a3183558 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 16 Jun 2026 22:45:09 -0400 Subject: [PATCH 26/26] chore(porch): 929 plan-approval gate-requested --- codev/projects/929-support-codex-and-gemini-clis-/status.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml index 35bf952dc..4c3b59337 100644 --- a/codev/projects/929-support-codex-and-gemini-clis-/status.yaml +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -7,6 +7,7 @@ current_plan_phase: null gates: plan-approval: status: pending + requested_at: '2026-06-17T02:45:09.085Z' dev-approval: status: pending pr: @@ -15,7 +16,7 @@ iteration: 1 build_complete: false history: [] started_at: '2026-05-31T19:04:30.567Z' -updated_at: '2026-06-17T02:34:59.005Z' +updated_at: '2026-06-17T02:45:09.086Z' pr_history: - phase: review pr_number: 1