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/plans/929-support-codex-and-gemini-clis-.md b/codev/plans/929-support-codex-and-gemini-clis-.md new file mode 100644 index 000000000..00cdbc699 --- /dev/null +++ b/codev/plans/929-support-codex-and-gemini-clis-.md @@ -0,0 +1,235 @@ +# PIR Plan: Support `codex` and `agy` (Antigravity CLI) as architects + +> **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. + +## Understanding + +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 + +### 1. Add `AGY_HARNESS` (`agent-farm/utils/harness.ts`) + +```ts +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, +}; +``` + +- 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 `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 — 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 (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; 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/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..4c3b59337 --- /dev/null +++ b/codev/projects/929-support-codex-and-gemini-clis-/status.yaml @@ -0,0 +1,29 @@ +id: '929' +title: support-codex-and-gemini-clis- +protocol: pir +phase: plan +plan_phases: [] +current_plan_phase: null +gates: + plan-approval: + status: pending + requested_at: '2026-06-17T02:45:09.085Z' + dev-approval: + status: pending + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-05-31T19:04:30.567Z' +updated_at: '2026-06-17T02:45:09.086Z' +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: false diff --git a/codev/resources/arch.md b/codev/resources/arch.md index b6e866519..750b4dd52 100644 --- a/codev/resources/arch.md +++ b/codev/resources/arch.md @@ -253,7 +253,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) @@ -269,6 +269,16 @@ 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, 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. + #### 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/resources/lessons-learned.md b/codev/resources/lessons-learned.md index 90762a4af..b9af69264 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 1052] Shared *shape* is not shared *substance* — resist extracting on resemblance. The VSCode replay buffer-and-flush and the web client's `flushInitialBuffer` do the same conceptual thing (hold replay → paint once at the settled size), but their triggers and bodies diverge: web uses a fixed 500ms delay entangled with `FitAddon`/`ScrollController`; VSCode uses a `setDimensions` debounce + PTY-resize + paced `writeChunked`. The only common part is a ~10-line "accumulate then emit once" skeleton; the valuable logic (trigger policy, flush body) differs. Extracting would couple two independently-evolving strategies behind one leaky name — a net negative at two call sites. Contrast the primitives that *are* genuinely identical and correctly centralized in core: `reconnect-policy` and `escape-buffer` (the dashboard's `escapeBuffer.ts` is a 5-line re-export of core's, not a copy). Corollary, painfully relearned here: a claim of "duplication" from an *import path* (`../lib/escapeBuffer.js`) is not evidence — read the file before filing a dedup; it was a re-export shim all along. - [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. 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..0c2990030 --- /dev/null +++ b/codev/reviews/929-support-codex-and-gemini-clis-.md @@ -0,0 +1,89 @@ +# 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 + +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 + +- `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) +- `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 + +**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. +- **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 + +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: + +- **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 new file mode 100644 index 000000000..913ed7033 --- /dev/null +++ b/codev/state/pir-929_thread.md @@ -0,0 +1,113 @@ +# PIR #929 — Support codex & gemini CLIs as architects + +## 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: + +- **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`. + +### 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. + +### 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. + +### 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. + +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. + +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. + +### 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. + +### 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. 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__/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/__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-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..217398f43 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,32 @@ 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; + if (!harness.buildResume) { + logger.info('This harness does not support conversation resume; starting a fresh session.'); + return undefined; } - logger.info('No prior Claude conversation found for this worktree; starting a fresh session.'); + const resume = harness.buildResume(worktreePath); + 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; } @@ -456,7 +468,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 +482,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 +847,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..b52230c5f 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,17 +513,25 @@ export async function launchInstance(workspacePath: string): Promise<{ success: // an unrelated jsonl. safeToResume = false; } - const resumeSessionId = safeToResume ? findLatestSessionId(workspacePath) : null; - if (!safeToResume) { + const architectHarness = getArchitectHarness(workspacePath); + const resume = safeToResume ? (architectHarness.buildResume?.(workspacePath) ?? null) : null; + // 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.`); } + 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 { + // 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/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 { 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 `; /**