Pin M2 spec to verified claude/codex CLI invocations

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-05-31 13:39:10 -04:00
parent 332e17d742
commit b2104f6442
@@ -72,15 +72,42 @@ type Backend interface {
} }
``` ```
Two real adapters: Two real adapters. The exact invocations below were verified empirically on
this machine (claude 2.1.154, codex-cli 0.135.0); both authenticate via the
existing CLI login — **no API keys**.
- `claudeBackend` runs `claude --print` with the prompt delivered on **stdin**. - **`claudeBackend`** runs:
- `codexBackend` runs `codex exec --` with the prompt delivered on **stdin**. ```
claude --print --tools "" --no-session-persistence --output-format text
```
The prompt is delivered on **stdin** (avoids argv limits and shell-escaping;
also dodges a quirk where an empty `--tools ""` positional can be mistaken for
the prompt). The model's answer is exactly **stdout** (trailing newline
trimmed). `--tools ""` disables all tools so it just answers;
`--no-session-persistence` avoids writing resumable session files. Do **not**
use `--bare` (it forces `ANTHROPIC_API_KEY` and ignores the machine's login).
- **`codexBackend`** runs:
```
codex exec --skip-git-repo-check --ignore-user-config --ignore-rules \
-s read-only -a never --ephemeral -o <tmpfile> -
```
The prompt is delivered on **stdin** (the trailing `-` tells codex to read it
from stdin). codex's **stdout is not clean** (it includes session preamble),
so the adapter writes the final answer to a per-call **temp file** via `-o`,
then reads and returns that file's contents. The adapter creates the temp file
(`os.CreateTemp`) and removes it on return. The flags matter:
`--ignore-user-config --ignore-rules -s read-only` stop codex from executing
shell commands driven by local config (observed: it otherwise runs tool calls
even for a trivial prompt, adding latency); `-a never` disables approval
prompts for headless use; `--ephemeral` skips persisting session files;
`--skip-git-repo-check` lets it run anywhere.
Both use `os/exec` with the `ctx` passed to `exec.CommandContext` so a timeout Both use `os/exec` with the `ctx` passed to `exec.CommandContext` so a timeout
cancels the child process. Each adapter stores its command name and base args in cancels the child process. Each adapter stores its command name and base args in
struct fields so argument construction is unit-testable without spawning a struct fields so argument construction is unit-testable without spawning a
process. process. The codex adapter's temp-file handling lives inside its `Run` so the
`Backend` interface stays uniform (`Run(ctx, prompt) (string, error)`).
A selector constructs the configured backend: A selector constructs the configured backend:
@@ -150,6 +177,14 @@ All parse/validation failures return a non-nil error; the caller degrades
gracefully (see below). Sentinel errors: `ErrEmptyResponse`, `ErrNoJSON`, gracefully (see below). Sentinel errors: `ErrEmptyResponse`, `ErrNoJSON`,
`ErrInvalidProposal`. `ErrInvalidProposal`.
Both CLIs also offer native structured-output flags (claude
`--output-format json --json-schema`; codex `--output-schema <file>`) that would
guarantee shape. We deliberately do **not** use them in M2: they diverge the two
adapters (different flags, envelope vs file) and would push schema concerns into
the `Backend` layer. Prompt-instructed JSON + tolerant `extractJSON` keeps the
`Backend` interface uniform and the parsing in one place. Native schemas remain a
clean future robustness upgrade behind the same `Coach` boundary.
### `session.Controller` — async coach orchestration ### `session.Controller` — async coach orchestration
A new method drives the coach using the **exact concurrency pattern** already in A new method drives the coach using the **exact concurrency pattern** already in
@@ -179,7 +214,9 @@ Behavior of `RequestCoach`:
unlock, `notify()` (broadcasts the pending state). unlock, `notify()` (broadcasts the pending state).
4. Launch a goroutine: 4. Launch a goroutine:
- `ctx, cancel := context.WithTimeout(context.Background(), coachTimeout)` - `ctx, cancel := context.WithTimeout(context.Background(), coachTimeout)`
(`coachTimeout = 30 * time.Second`); `defer cancel()`. (`coachTimeout = 60 * time.Second` — codex in particular runs tens of
seconds even for trivial prompts; 60s gives a real coaching prompt
headroom); `defer cancel()`.
- Call `coach.Coach(ctx, intent)`. - Call `coach.Coach(ctx, intent)`.
- Lock. **If `gen != c.coachGen` or `runtimeState != RuntimePlanning`, - Lock. **If `gen != c.coachGen` or `runtimeState != RuntimePlanning`,
unlock and return** (stale result — a newer request superseded this one, or unlock and return** (stale result — a newer request superseded this one, or
@@ -332,7 +369,7 @@ as a broken Planning view:
| ------- | ------ | | ------- | ------ |
| No backend wired (`SetCoach` never called / nil) | `RequestCoach` sets `status=error`, "coach unavailable"; returns nil | | No backend wired (`SetCoach` never called / nil) | `RequestCoach` sets `status=error`, "coach unavailable"; returns nil |
| CLI binary missing | `backend.Run` errors → goroutine sets `status=error` | | CLI binary missing | `backend.Run` errors → goroutine sets `status=error` |
| CLI timeout (>30s) | `context` cancels child → error → `status=error` | | CLI timeout (>60s) | `context` cancels child → error → `status=error` |
| Empty / non-JSON output | `extractJSON`/`parseProposal` error → `status=error` | | Empty / non-JSON output | `extractJSON`/`parseProposal` error → `status=error` |
| Missing/empty fields, non-positive timebox | `parseProposal` error → `status=error` | | Missing/empty fields, non-positive timebox | `parseProposal` error → `status=error` |
| Request issued outside planning | `RequestCoach` returns `ErrNotPlanning` → HTTP 400 | | Request issued outside planning | `RequestCoach` returns `ErrNotPlanning` → HTTP 400 |
@@ -364,8 +401,10 @@ constraint.
- `Service.Coach` against a `fakeBackend` returning canned strings: success, - `Service.Coach` against a `fakeBackend` returning canned strings: success,
chatty-wrapped success, malformed → error. chatty-wrapped success, malformed → error.
- `claudeBackend`/`codexBackend`: argument construction is correct and the prompt - `claudeBackend`/`codexBackend`: argument construction is correct and the prompt
is routed to stdin (assert on the built `*exec.Cmd` fields; do not spawn the is routed to stdin (assert on the built `*exec.Cmd` `Args`/`Stdin` fields; do
real CLI). not spawn the real CLI). For codex, assert the `-o <tmpfile>` flag is present
and that `Run` would read that path (factor the temp-file path out so it is
injectable/observable in the test).
- `NewBackend`: returns claude by default, codex by name, error on unknown. - `NewBackend`: returns claude by default, codex by name, error on unknown.
**`session` package** (with a fake `ai.Coach`): **`session` package** (with a fake `ai.Coach`):