From 462844a8858a3d3e30e1f6c0e3f93af36faa9c5a Mon Sep 17 00:00:00 2001 From: pseudo Date: Fri, 21 Aug 2026 16:35:07 -0600 Subject: [PATCH 01/10] chore(porch): 22 init air --- .../22-add-opencode-as-a-consult-lane/status.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 codev/projects/22-add-opencode-as-a-consult-lane/status.yaml diff --git a/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml b/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml new file mode 100644 index 000000000..e9094f367 --- /dev/null +++ b/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml @@ -0,0 +1,14 @@ +id: '22' +title: add-opencode-as-a-consult-lane +protocol: air +phase: implement +plan_phases: [] +current_plan_phase: null +gates: + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-08-21T22:35:07.319Z' +updated_at: '2026-08-21T22:35:07.319Z' From c78b246181666214af15f425a8628232c68d3757 Mon Sep 17 00:00:00 2001 From: pseudo Date: Fri, 21 Aug 2026 16:50:13 -0600 Subject: [PATCH 02/10] [AIR #22] feat: add opencode as a consult lane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit consult had four lanes and no reason for opencode not to be one. Adding it gives reviews a third independent reviewer on an account none of the existing lanes share — which is what today cost us: codex has been quota-exhausted since before 08:00 UTC, and four merges shipped with two of three lanes. The lane is `opencode run -m `, defaulting to `xai/grok-4.6`. Role folds into the prompt (opencode has no system-prompt flag), following hermes rather than inventing a second approach. Verdict parsing is untouched: stdout is plain assistant text with the VERDICT line intact, live-probed rather than assumed. Two things it does differently from the lanes it sits beside: **The model id is checked before the spawn.** `x-ai/grok-4.6` — the spelling most other tooling uses — comes back from the provider as `UnknownError: Unexpected server error` with empty stdout, naming neither the model nor the mistake. So the id is checked against `opencode models`, and a miss says which prefix this machine actually has. That is not the hardcoded catalog consult-lanes.ts forbids: the list is the provider tool answering for itself at call time, and if the listing fails the check stands down and the provider is the authority again. **Nothing degrades into a passing review.** The agy lane skips non-blockingly because an unauthenticated agy is a routine state; opencode has no such property, and #20 is standing evidence that a lane producing nothing is counted as an approval. Missing CLI, unknown id, non-zero exit and empty output all throw, and all discard any stale review file first so porch cannot read an earlier iteration's verdict as this one's. The review names the model that wrote it — Grok 4.6 and Grok 4.3 are not interchangeable evidence. `porch.consultation.models` still defaults to gemini/codex/claude; opencode is available, not conscripted. Co-Authored-By: Claude Opus 5 --- .../consult/__tests__/default-models.test.ts | 8 + .../consult/__tests__/opencode-lane.test.ts | 315 ++++++++++++++++++ .../codev/src/commands/consult/cli-options.ts | 4 +- packages/codev/src/commands/consult/index.ts | 275 ++++++++++++++- packages/codev/src/lib/config.ts | 2 +- packages/codev/src/lib/consult-lanes.ts | 44 ++- packages/codev/src/lib/test-env.ts | 33 ++ 7 files changed, 673 insertions(+), 8 deletions(-) create mode 100644 packages/codev/src/commands/consult/__tests__/opencode-lane.test.ts diff --git a/packages/codev/src/commands/consult/__tests__/default-models.test.ts b/packages/codev/src/commands/consult/__tests__/default-models.test.ts index b4584ebf9..6e99e45b1 100644 --- a/packages/codev/src/commands/consult/__tests__/default-models.test.ts +++ b/packages/codev/src/commands/consult/__tests__/default-models.test.ts @@ -3,6 +3,7 @@ import { DEFAULT_CLAUDE_MODEL, DEFAULT_CODEX_MODEL, DEFAULT_CODEX_REASONING_EFFORT, + DEFAULT_OPENCODE_MODEL, computeCodexCost, } from '../index.js'; @@ -28,6 +29,13 @@ describe('shipped consult lane defaults', () => { expect(DEFAULT_CODEX_MODEL).toBe('gpt-5.6-sol'); expect(DEFAULT_CODEX_REASONING_EFFORT).toBe('medium'); }); + + it('pins the opencode lane to xai/grok-4.6', () => { + // The `xai/` prefix is load-bearing in the other direction from `-sol`: `x-ai/grok-4.6`, the + // spelling most other tooling uses, is rejected by the provider with a bare + // `UnknownError: Unexpected server error`. Live-probed 2026-08-21. + expect(DEFAULT_OPENCODE_MODEL).toBe('xai/grok-4.6'); + }); }); describe('computeCodexCost', () => { diff --git a/packages/codev/src/commands/consult/__tests__/opencode-lane.test.ts b/packages/codev/src/commands/consult/__tests__/opencode-lane.test.ts new file mode 100644 index 000000000..be3a54743 --- /dev/null +++ b/packages/codev/src/commands/consult/__tests__/opencode-lane.test.ts @@ -0,0 +1,315 @@ +/** + * The opencode consult lane (#22) — a Grok reviewer on an account no other lane shares. + * + * Two properties carry this lane, and both are here because of what they prevent: + * + * 1. **An unknown model id is named before the spawn.** Live-probed 2026-08-21: + * `opencode run -m x-ai/grok-4.6` exits 1 with EMPTY stdout and + * `UnknownError: Unexpected server error` on stderr — text that identifies neither the model + * nor the mistake. The pre-flight against `opencode models` is the only place a message can + * say "you wrote `x-ai/`, this machine has `xai/`". + * + * 2. **Nothing degrades into a passing review.** #20: porch counts a lane that never produced + * a verdict as an approval. So a missing CLI, a rejected id, a non-zero exit, and empty + * output all throw, and none of them leaves a review file behind for porch to find. + * + * Uses a real fake `opencode` binary (the `agy-lane-model.test.ts` pattern) rather than a module + * mock, so argv, exit codes and stream routing are genuinely exercised — `opencode` writes its + * banner to stderr and only the review to stdout, and that split is load-bearing. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { + _runOpencodeConsultation, + resolveOpencodeBin, + listOpencodeModels, + opencodeReviewHeader, + resolveLaneModelChoice, + DEFAULT_OPENCODE_MODEL, +} from '../index.js'; +import { + assertOpencodeModelAvailable, + MODEL_CONFIGURABLE_LANES, + VALID_LANE_NAMES, + validateLaneList, + validateConsultModels, +} from '../../../lib/consult-lanes.js'; + +const ENV_KEYS = [ + 'CODEV_OPENCODE_BIN', + 'FAKE_OPENCODE_ARGV_LOG', + 'FAKE_OPENCODE_MODE', + 'HOME', + 'CODEV_METRICS_DB', +] as const; + +/** + * Fake opencode. Records argv, answers `models` with a catalog shaped like the real one (note + * `xai/`, not `x-ai/`), then behaves per FAKE_OPENCODE_MODE. + * + * The banner on stderr is not decoration: it reproduces the real CLI's stream split, so a test + * that asserts "stdout IS the review" is asserting something real. + */ +const FAKE_OPENCODE_SOURCE = `#!/usr/bin/env node +const fs = require('node:fs'); +const argv = process.argv.slice(2); +if (argv[0] === 'models') { + process.stdout.write('opencode/big-pickle\\nxai/grok-4.3\\nxai/grok-4.6\\n'); + process.exit(0); +} +fs.writeFileSync(process.env.FAKE_OPENCODE_ARGV_LOG, JSON.stringify(argv)); +process.stderr.write('\\n> build · fake\\n'); +const mode = process.env.FAKE_OPENCODE_MODE || 'ok'; +if (mode === 'reject') { + process.stderr.write('Error: {"name":"UnknownError","data":{"message":"Unexpected server error."}}\\n'); + process.exit(1); +} +if (mode === 'empty') { process.exit(0); } +process.stdout.write('Looks fine to me.\\n\\nVERDICT: APPROVE\\nSUMMARY: ok\\nCONFIDENCE: HIGH\\n'); +process.exit(0); +`; + +let dir: string; +let savedEnv: Record; +let argvLog: string; + +function writeConfig(config: unknown): void { + fs.mkdirSync(path.join(dir, '.codev'), { recursive: true }); + fs.writeFileSync(path.join(dir, '.codev', 'config.json'), JSON.stringify(config)); +} + +function opencodeArgv(): string[] { + return JSON.parse(fs.readFileSync(argvLog, 'utf-8')); +} + +beforeEach(() => { + savedEnv = {}; + for (const k of ENV_KEYS) savedEnv[k] = process.env[k]; + + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'opencode-lane-')); + const fakeBin = path.join(dir, 'opencode'); + fs.writeFileSync(fakeBin, FAKE_OPENCODE_SOURCE, { mode: 0o755 }); + argvLog = path.join(dir, 'argv.json'); + + process.env.CODEV_OPENCODE_BIN = fakeBin; + process.env.FAKE_OPENCODE_ARGV_LOG = argvLog; + process.env.FAKE_OPENCODE_MODE = 'ok'; + // A real ~/.codev/config.json would otherwise leak an opencode model into every assertion. + process.env.HOME = path.join(dir, 'fake-home'); + + vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); + +afterEach(() => { + for (const k of ENV_KEYS) { + if (savedEnv[k] === undefined) delete process.env[k]; + else process.env[k] = savedEnv[k]; + } + vi.restoreAllMocks(); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +// --- registration --------------------------------------------------------------------- + +describe('opencode is a first-class lane', () => { + it('is selectable in porch.consultation lane lists', () => { + expect(VALID_LANE_NAMES).toContain('opencode'); + expect(() => validateLaneList(['gemini', 'claude', 'opencode'], 'porch.consultation.models')) + .not.toThrow(); + }); + + it('accepts a configured model id, which is the point of adding it', () => { + expect(MODEL_CONFIGURABLE_LANES).toContain('opencode'); + expect(() => validateConsultModels({ opencode: 'xai/grok-4.6' })).not.toThrow(); + }); + + it('rejects a syntactically invalid id like any other configurable lane', () => { + expect(() => validateConsultModels({ opencode: '-leading-dash' })).toThrow(/Invalid model id/); + }); +}); + +// --- argv ----------------------------------------------------------------------------- + +describe('argv', () => { + it('runs `opencode run -m `', async () => { + await _runOpencodeConsultation('the query', 'the role', dir); + const argv = opencodeArgv(); + expect(argv[0]).toBe('run'); + expect(argv).toContain('-m'); + expect(argv[argv.indexOf('-m') + 1]).toBe(DEFAULT_OPENCODE_MODEL); + }); + + it('folds the role into the prompt — opencode has no system-prompt flag', async () => { + await _runOpencodeConsultation('the query', 'the role', dir); + const prompt = opencodeArgv().at(-1)!; + expect(prompt).toContain('the role'); + expect(prompt).toContain('the query'); + }); + + it('sends the configured model id rather than the shipped default', async () => { + writeConfig({ consult: { models: { opencode: 'xai/grok-4.3' } } }); + await _runOpencodeConsultation('q', 'role', dir); + const argv = opencodeArgv(); + expect(argv[argv.indexOf('-m') + 1]).toBe('xai/grok-4.3'); + }); + + it('lets --model-id outrank config', async () => { + writeConfig({ consult: { models: { opencode: 'xai/grok-4.3' } } }); + const choice = resolveLaneModelChoice(dir, 'opencode', DEFAULT_OPENCODE_MODEL, 'xai/grok-4.6'); + await _runOpencodeConsultation('q', 'role', dir, undefined, undefined, choice); + const argv = opencodeArgv(); + expect(argv[argv.indexOf('-m') + 1]).toBe('xai/grok-4.6'); + }); +}); + +// --- the review ----------------------------------------------------------------------- + +describe('the review output', () => { + it('is stdout, with the VERDICT line intact', async () => { + const outputPath = path.join(dir, 'review.md'); + await _runOpencodeConsultation('q', 'role', dir, outputPath); + const content = fs.readFileSync(outputPath, 'utf-8'); + expect(content).toContain('VERDICT: APPROVE'); + // The banner opencode writes to stderr must not end up in the review. + expect(content).not.toContain('> build · fake'); + }); + + it('names the model that produced it — Grok 4.6 and Grok 4.3 are not interchangeable evidence', async () => { + writeConfig({ consult: { models: { opencode: 'xai/grok-4.3' } } }); + const outputPath = path.join(dir, 'review.md'); + await _runOpencodeConsultation('q', 'role', dir, outputPath); + const content = fs.readFileSync(outputPath, 'utf-8'); + expect(content).toContain('xai/grok-4.3'); + expect(content).toContain('consult.models.opencode'); + }); + + it('puts the header where it cannot shadow the verdict', () => { + const header = opencodeReviewHeader({ + id: 'xai/grok-4.6', key: null, source: null, fromFlag: false, + }); + // parseVerdict scans last→first; a header carrying its own VERDICT token would win. + expect(header).not.toContain('VERDICT'); + expect(header).toContain('xai/grok-4.6'); + }); +}); + +// --- failing loudly -------------------------------------------------------------------- + +describe('an unknown model id fails before the spawn', () => { + it('names the right prefix when only the prefix is wrong', () => { + const available = ['xai/grok-4.6', 'xai/grok-4.3', 'opencode/big-pickle']; + expect(() => assertOpencodeModelAvailable('x-ai/grok-4.6', available, 'consult.models.opencode')) + .toThrow(/xai\/grok-4\.6/); + expect(() => assertOpencodeModelAvailable('x-ai/grok-4.6', available, 'consult.models.opencode')) + .toThrow(/Did you mean/); + }); + + it('names where the bad id came from', () => { + expect(() => assertOpencodeModelAvailable('x-ai/grok-4.6', ['xai/grok-4.6'], '--model-id')) + .toThrow(/--model-id/); + }); + + it('lists the real catalog when the name is wrong too', () => { + expect(() => assertOpencodeModelAvailable('xai/grok-9', ['xai/grok-4.6'], null)) + .toThrow(/opencode models.* on this machine offers/s); + }); + + it('never falls back to a working id', () => { + expect(() => assertOpencodeModelAvailable('x-ai/grok-4.6', ['xai/grok-4.6'], null)) + .toThrow(/does not fall back/); + }); + + it('passes an id the catalog does offer', () => { + expect(() => assertOpencodeModelAvailable('xai/grok-4.6', ['xai/grok-4.6'], null)).not.toThrow(); + }); + + it('treats an unreadable catalog as unknown, not as "nothing is valid"', () => { + // A broken `opencode models` must not fail every review — the provider stays the authority. + expect(() => assertOpencodeModelAvailable('xai/anything', [], null)).not.toThrow(); + }); + + it('rejects through the lane, without spawning a review', async () => { + writeConfig({ consult: { models: { opencode: 'x-ai/grok-4.6' } } }); + await expect(_runOpencodeConsultation('q', 'role', dir)).rejects.toThrow(/Unknown opencode model/); + expect(fs.existsSync(argvLog)).toBe(false); + }); +}); + +describe('nothing degrades into a passing review (#20)', () => { + it('a non-zero exit rejects', async () => { + process.env.FAKE_OPENCODE_MODE = 'reject'; + await expect(_runOpencodeConsultation('q', 'role', dir)) + .rejects.toThrow(/opencode exited with code 1/); + }); + + it('a non-zero exit carries opencode\'s own diagnostic text', async () => { + process.env.FAKE_OPENCODE_MODE = 'reject'; + await expect(_runOpencodeConsultation('q', 'role', dir)).rejects.toThrow(/UnknownError/); + }); + + it('empty output rejects rather than passing as a silent skip', async () => { + process.env.FAKE_OPENCODE_MODE = 'empty'; + await expect(_runOpencodeConsultation('q', 'role', dir)) + .rejects.toThrow(/produced no review output/); + }); + + it('a missing CLI rejects', async () => { + process.env.CODEV_OPENCODE_BIN = path.join(dir, 'not-installed'); + await expect(_runOpencodeConsultation('q', 'role', dir)).rejects.toThrow(/opencode not found/); + }); + + it('leaves no stale review file for porch to accept', async () => { + // The failure mode this guards: consult writes to a deterministic per-iteration path, so a + // review from an EARLIER run of the same iteration would be read as this one's. + const outputPath = path.join(dir, 'review.md'); + fs.writeFileSync(outputPath, 'stale review from a previous run\nVERDICT: APPROVE\n'); + process.env.FAKE_OPENCODE_MODE = 'reject'; + + await _runOpencodeConsultation('q', 'role', dir, outputPath).catch(() => {}); + + expect(fs.existsSync(outputPath)).toBe(false); + }); + + it('discards a stale review on a rejected model id too', async () => { + const outputPath = path.join(dir, 'review.md'); + fs.writeFileSync(outputPath, 'stale review\nVERDICT: APPROVE\n'); + writeConfig({ consult: { models: { opencode: 'x-ai/grok-4.6' } } }); + + await _runOpencodeConsultation('q', 'role', dir, outputPath).catch(() => {}); + + expect(fs.existsSync(outputPath)).toBe(false); + }); +}); + +// --- binary resolution ------------------------------------------------------------------ + +describe('binary resolution', () => { + it('honours CODEV_OPENCODE_BIN', () => { + expect(resolveOpencodeBin()).toBe(process.env.CODEV_OPENCODE_BIN); + }); + + it('returns null for an override that does not exist, rather than silently using PATH', () => { + process.env.CODEV_OPENCODE_BIN = path.join(dir, 'nope'); + expect(resolveOpencodeBin()).toBeNull(); + }); + + it('refuses to reach the real binary from an unpinned test', () => { + delete process.env.CODEV_OPENCODE_BIN; + // A billed Grok call per spawn is not something a suite should reach by omission. + expect(() => resolveOpencodeBin()).toThrow(/CODEV_OPENCODE_BIN/); + }); + + it('reads the catalog from the resolved binary', () => { + expect(listOpencodeModels(process.env.CODEV_OPENCODE_BIN!)).toEqual([ + 'opencode/big-pickle', 'xai/grok-4.3', 'xai/grok-4.6', + ]); + }); + + it('returns an empty catalog rather than throwing when the listing fails', () => { + expect(listOpencodeModels(path.join(dir, 'nope'))).toEqual([]); + }); +}); diff --git a/packages/codev/src/commands/consult/cli-options.ts b/packages/codev/src/commands/consult/cli-options.ts index 7a169a209..c39733fe9 100644 --- a/packages/codev/src/commands/consult/cli-options.ts +++ b/packages/codev/src/commands/consult/cli-options.ts @@ -28,7 +28,7 @@ export const STATS_ONLY_FLAGS = ['days', 'project', 'last', 'json'] as const; /** Register every `consult` flag on a command. */ export function registerConsultOptions(cmd: Command): Command { return cmd - .option('-m, --model ', 'Model to use (gemini, codex, claude, hermes, or aliases: pro, gpt, opus)') + .option('-m, --model ', 'Model to use (gemini, codex, claude, hermes, opencode, or aliases: pro, gpt, opus)') .option('--prompt ', 'Inline prompt (general mode)') .option('--prompt-file ', 'Prompt file path (general mode)') .option('--protocol ', 'Protocol name: spir, aspir, air, bugfix, pir, maintain') @@ -36,7 +36,7 @@ export function registerConsultOptions(cmd: Command): Command { .option('--issue ', 'Issue number (required from architect context)') .option('--branch ', 'Read spec/plan artifacts from this git ref instead of the local workspace (e.g. `origin/builder/777-foo` or `builder/777-foo`). Defaults to the PR\'s head branch when --issue resolves to a PR. Note: this only changes the artifact source — for --type impl, the diff scope is always the PR\'s head→base, not the --branch ref.') .option('--base ', 'For --type integration: anchor the diff on this base branch (e.g. `ci`), computed locally as `git diff origin/...origin/` (three-dot). Use in repos with a long-lived integration branch ahead of the default branch so the review sees only the PR\'s actual change, not the whole integration-over-trunk delta. Defaults to config `consult.integrationBranch`; unset → the PR\'s host base (`gh pr diff`).') - .option('--model-id ', 'Override the provider model id for this invocation, outranking config `consult.models.` (e.g. `--model-id gpt-5.6-sol`). Supported for the claude, codex, and gemini lanes; using it with a lane that has no model selector (hermes) is an error rather than a silent no-op. Codev validates syntax only — whether the id exists is the provider\'s call, and a rejection fails loudly with no fallback.') + .option('--model-id ', 'Override the provider model id for this invocation, outranking config `consult.models.` (e.g. `--model-id gpt-5.6-sol`). Supported for the claude, codex, gemini, and opencode lanes; using it with a lane that has no model selector (hermes) is an error rather than a silent no-op. Codev validates syntax only — whether the id exists is the provider\'s call, and a rejection fails loudly with no fallback. The one exception is opencode, whose id is checked against `opencode models` before the run, because the provider rejects an unknown one with an untraceable server error.') .option('--output ', 'Write consultation output to file (used by porch)') .option('--plan-phase ', 'Scope review to a specific plan phase (used by porch)') .option('--context ', 'Context file with previous iteration feedback (used by porch)') diff --git a/packages/codev/src/commands/consult/index.ts b/packages/codev/src/commands/consult/index.ts index 2952771c5..1f3ecaa7c 100644 --- a/packages/codev/src/commands/consult/index.ts +++ b/packages/codev/src/commands/consult/index.ts @@ -22,6 +22,7 @@ import { resolveReasoningEffort, validateModelId, assertLaneAcceptsModelOverride, + assertOpencodeModelAvailable, type ConfigurableLane, } from '../../lib/consult-lanes.js'; import type { ModelReasoningEffort } from '@openai/codex-sdk'; @@ -30,7 +31,7 @@ import { MetricsDB } from './metrics.js'; import { extractUsage, extractReviewText, type SDKResultLike, type UsageData } from './usage-extractor.js'; import { executeForgeCommandSync } from '../../lib/forge.js'; import { preflightAgyAuth, recordAgyAuthState, type AgyAuthState } from './agy-auth-cache.js'; -import { assertAgyLaneAllowedUnderTest } from '../../lib/test-env.js'; +import { assertAgyLaneAllowedUnderTest, assertOpencodeLaneAllowedUnderTest } from '../../lib/test-env.js'; // Content reference — resolved artifact content with a display label interface ContentRef { @@ -51,6 +52,10 @@ const MODEL_CONFIGS: Record = { // cli/args are NOT used for dispatch (agy's binary path is resolved at runtime). gemini: { cli: 'agy', args: [], envVar: null }, hermes: { cli: 'hermes', args: ['chat', '-q'], envVar: null }, + // opencode dispatches via runOpencodeConsultation (it needs `-m` and a pre-flight + // catalog check), so cli/args here are the shape the runner builds on, not a + // literal argv — the prompt and `-m ` are appended by the runner. + opencode: { cli: 'opencode', args: ['run'], envVar: null }, }; // Models that use an Agent SDK instead of CLI subprocess @@ -418,6 +423,20 @@ export const DEFAULT_CODEX_REASONING_EFFORT = 'medium' as const; /** Shipped default model id for the claude consult lane (#1288). */ export const DEFAULT_CLAUDE_MODEL = 'claude-opus-5'; +/** + * Shipped default model id for the opencode consult lane (#22). + * + * The `xai/` prefix is LOAD-BEARING. `x-ai/grok-4.6` — the spelling most other tooling uses — was + * live-probed on 2026-08-21 and rejected by the provider with a bare `UnknownError: Unexpected + * server error` and empty stdout. `assertOpencodeModelAvailable` exists so that mistake is named + * before the spawn instead of arriving as that. + * + * The lane exists to supply a reviewer on an account shared with no other lane, so it defaults to + * the strongest Grok `opencode models` lists rather than to whatever opencode would pick — an + * unpinned default could silently land on a model from a provider a sibling lane already uses. + */ +export const DEFAULT_OPENCODE_MODEL = 'xai/grok-4.6'; + interface CodexModelPricing { inputPer1M: number; cachedInputPer1M: number; @@ -1249,6 +1268,244 @@ async function runAgyConsultation( }); } +// --- opencode lane (#22) ------------------------------------------------------ + +/** Codev-owned hard cap on a single `opencode run`. Matches the agy lane's budget. */ +const OPENCODE_TIMEOUT_MS = 6 * 60 * 1000; + +/** How long `opencode models` gets to print its catalog before the pre-flight gives up. */ +const OPENCODE_MODELS_TIMEOUT_MS = 30_000; + +/** Bounded tail of the lane's output, retained so a failure can quote why it failed. */ +const OPENCODE_FAILURE_TAIL_MAX_CHARS = 2000; + +/** + * Resolve the `opencode` binary, or `null` when it isn't installed. + * + * The single chokepoint for the test-isolation guard, and it has to be resolution rather than the + * spawn: the pre-flight below *executes* the binary (`opencode models`) before any review runs, so + * guarding only `runOpencodeConsultation`'s spawn would still let a suite reach the real CLI. + */ +export function resolveOpencodeBin(): string | null { + // Explicit override (tests, or a non-PATH install): honoured as given, never quietly replaced + // with a different binary the caller did not ask for. + const override = process.env.CODEV_OPENCODE_BIN; + if (override) return fs.existsSync(override) ? override : null; + + assertOpencodeLaneAllowedUnderTest(); + + return commandExists('opencode') ? 'opencode' : null; +} + +/** + * The model ids `opencode` offers on this machine, or `[]` if the catalog could not be read. + * + * `[]` deliberately means "unknown", not "none": the pre-flight below then skips the existence + * check and lets the provider be the authority, which is the pre-1286 behaviour. Failing the lane + * because a *catalog listing* broke would turn a diagnostic into an outage. + */ +export function listOpencodeModels(bin = 'opencode'): string[] { + try { + const out = execFileSync(bin, ['models'], { + encoding: 'utf-8', + timeout: OPENCODE_MODELS_TIMEOUT_MS, + stdio: ['ignore', 'pipe', 'pipe'], + }); + return out.split('\n').map(l => l.trim()).filter(l => l.length > 0); + } catch { + return []; + } +} + +/** + * Provenance banner prepended to an opencode review. + * + * The issue's requirement, verbatim: "Record which model the lane used in the review output. `Grok + * 4.6` and `Grok 4.3` are not interchangeable evidence." Stderr logging is not enough — the review + * file is what outlives the run and what a later reader actually opens. + * + * Safe to prepend: `parseVerdict` scans lines LAST→FIRST, so a header cannot shadow the verdict at + * the end, and this line carries no `VERDICT:` token of its own. + */ +export function opencodeReviewHeader(choice: LaneModelChoice): string { + const from = choice.key ? ` (from ${choice.key})` : ' (shipped default)'; + return `_Reviewed by the opencode lane — model: \`${choice.id}\`${from}._\n\n`; +} + +/** + * Run the `opencode` consult lane (`opencode run -m `). + * + * ## Why this lane hard-fails where the agy lane skips + * + * The gemini/agy lane degrades to a non-blocking COMMENT skip because it is OAuth-fragile: an + * unauthenticated `agy` is a routine state on a developer's machine, and wedging every phase on it + * would be worse than losing a lane. opencode has no equivalent failure mode — it authenticates + * once and stays that way. + * + * So the trade-off runs the other way here, and #20 is why it matters: porch counts a lane that + * produced nothing as an approval. A lane that quietly emits a skip is a lane that quietly lowers + * the bar. Missing CLI, unknown model, non-zero exit, and empty output all throw. + */ +export async function runOpencodeConsultation( + queryText: string, + role: string, + workspaceRoot: string, + outputPath?: string, + metricsCtx?: MetricsContext, + modelChoice?: LaneModelChoice, +): Promise { + const startTime = Date.now(); + const choice = modelChoice + ?? resolveLaneModelChoice(workspaceRoot, 'opencode', DEFAULT_OPENCODE_MODEL); + + const bin = resolveOpencodeBin(); + if (!bin) { + // A missing CLI is a hard failure here, unlike the agy lane's skip. See the header: a lane that + // silently produces nothing is counted as an approval (#20), and "not installed" is a + // configuration mistake with an obvious fix, not a transient environment state. + discardStaleOutput(outputPath); + throw new Error( + 'opencode not found. Install it (https://opencode.ai), or drop "opencode" from ' + + 'porch.consultation in .codev/config.json.' + ); + } + + // Pre-flight the id against opencode's own catalog. The provider's rejection is + // `UnknownError: Unexpected server error` with empty stdout — useless for finding a typo'd + // prefix — so the check has to happen while we still know what was asked for. + try { + assertOpencodeModelAvailable(choice.id, listOpencodeModels(bin), choice.key); + } catch (err) { + discardStaleOutput(outputPath); + throw err; + } + + // opencode has no system-prompt flag, so the role folds into the prompt (hermes/agy precedent). + const prompt = `${role}\n\n---\n\n${queryText}`; + let tempFile: string | null = null; + let promptArg = prompt; + // Large inline argv can exceed ARG_MAX (E2BIG) — write it out and point opencode at the file. + // The temp file lands in the consult sandbox dir, the same one the agy lane uses. + if (prompt.length > CLI_PROMPT_INLINE_MAX_CHARS) { + tempFile = path.join(consultSandboxDir(), `codev-consult-prompt-${Date.now()}.md`); + fs.writeFileSync(tempFile, prompt); + promptArg = [ + `Read the full consultation prompt from this file: ${tempFile}`, + 'You have file access. Read files directly from disk to review code.', + ].join('\n\n'); + } + + const args = [...MODEL_CONFIGS.opencode.args, '-m', choice.id, promptArg]; + + const cleanup = () => { + if (tempFile && fs.existsSync(tempFile)) { + try { fs.unlinkSync(tempFile); } catch { /* best-effort */ } + } + }; + + return new Promise((resolve, reject) => { + const proc = spawn(bin, args, { + cwd: workspaceRoot, + // stderr is piped, not inherited: opencode writes its banner and its tool-call trace there, + // and that trace is the only text explaining a rejection, so it is retained rather than + // spilled into the parent's stream. + stdio: ['ignore', 'pipe', 'pipe'], + }); + + const outChunks: Buffer[] = []; + let outputTail = ''; + let settled = false; + + const fail = (message: string, exitCode: number) => { + if (settled) return; + settled = true; + clearTimeout(timer); + try { proc.kill('SIGTERM'); } catch { /* already gone */ } + cleanup(); + recordOpencodeMetrics(metricsCtx, startTime, exitCode, message, choice.id); + console.error(`\n[opencode FAILED: ${message}]`); + // "No review file" must mean none EXISTS, not merely that this run wrote none — porch keys + // off the file's presence and would otherwise advance on an earlier iteration's review. + discardStaleOutput(outputPath); + const err = new Error( + `${message}` + + (outputTail.trim() + ? `\n\nopencode output (last ${OPENCODE_FAILURE_TAIL_MAX_CHARS} chars):\n${outputTail.trim()}` + : '') + ); + reject(annotateModelError(err, 'opencode', choice)); + }; + + const timer = setTimeout( + () => fail(`opencode timed out after ${OPENCODE_TIMEOUT_MS / 1000}s`, 1), + OPENCODE_TIMEOUT_MS, + ); + + const watch = (buf: Buffer, isStdout: boolean) => { + if (isStdout) outChunks.push(buf); + outputTail = (outputTail + buf.toString('utf-8')).slice(-OPENCODE_FAILURE_TAIL_MAX_CHARS); + }; + proc.stdout?.on('data', (b: Buffer) => watch(b, true)); + proc.stderr?.on('data', (b: Buffer) => watch(b, false)); + + proc.on('error', (err) => fail(`opencode failed to start: ${err.message}`, 1)); + + proc.on('close', (code) => { + if (settled) return; + const raw = Buffer.concat(outChunks).toString('utf-8').trim(); + + // fail() owns settling on both error paths — it clears the timer and cleans up itself, so + // this handler must NOT pre-settle or those paths would be swallowed by its own guard. + if (code !== 0) { + fail(`opencode exited with code ${code}`, code ?? 1); + return; + } + // A zero exit with nothing on stdout is the #20 shape exactly: no review, but nothing that + // looks like a failure either. Refuse to let it pass as one. + if (raw.length === 0) { + fail('opencode produced no review output', 0); + return; + } + + settled = true; + clearTimeout(timer); + cleanup(); + + // `opencode run` prints the assistant's plain text to stdout (its banner and tool trace go to + // stderr), so stdout IS the review. Live-probed 2026-08-21, including the `VERDICT:` line + // surviving verbatim — which is why verdict parsing needs no opencode-specific case. + const content = opencodeReviewHeader(choice) + raw; + process.stdout.write(content); + writeConsultOutput(outputPath, content); + recordOpencodeMetrics(metricsCtx, startTime, 0, null, choice.id); + console.error(`\n[opencode completed in ${((Date.now() - startTime) / 1000).toFixed(1)}s]`); + resolve(); + }); + }); +} + +/** Metrics row for the opencode lane. `opencode run` reports no token usage, so those stay null. */ +function recordOpencodeMetrics( + metricsCtx: MetricsContext | undefined, + startTime: number, + exitCode: number, + errorMessage: string | null, + modelId: string, +): void { + if (!metricsCtx) return; + recordMetrics(metricsCtx, { + // Always a real id — unlike hermes, this lane never runs without one. + modelId, + durationSeconds: (Date.now() - startTime) / 1000, + inputTokens: null, + cachedInputTokens: null, + outputTokens: null, + costUsd: null, + exitCode, + errorMessage, + }); +} + /** * Record the model a lane actually ran, so a transcript answers "what did this use?". * @@ -1318,6 +1575,17 @@ async function runConsultation( return; } + // opencode lane → `opencode run` (#22). Dispatched here rather than through the generic + // MODEL_CONFIGS path below because it needs `-m` and a pre-flight catalog check. + if (model === 'opencode') { + const startTime = Date.now(); + const choice = resolveLaneModelChoice(workspaceRoot, 'opencode', DEFAULT_OPENCODE_MODEL, modelIdOverride); + logResolvedModel(model, choice.id, choice.key); + await runOpencodeConsultation(query, role, workspaceRoot, outputPath, metricsCtx, choice); + logQuery(workspaceRoot, model, query, (Date.now() - startTime) / 1000); + return; + } + const config = MODEL_CONFIGS[model]; if (!config) { @@ -2372,8 +2640,8 @@ export async function consult(options: ConsultOptions): Promise { } } - // Add file access instruction for Gemini - if (model === 'gemini' || model === 'hermes') { + // Add file access instruction for the agentic CLI lanes + if (model === 'gemini' || model === 'hermes' || model === 'opencode') { query += '\n\nYou have file access. Read files directly from disk to review code.'; } @@ -2426,5 +2694,6 @@ export { MODEL_CONFIGS as _MODEL_CONFIGS, MODEL_ALIASES as _MODEL_ALIASES, runAgyConsultation as _runAgyConsultation, + runOpencodeConsultation as _runOpencodeConsultation, agySkipContent as _agySkipContent, }; diff --git a/packages/codev/src/lib/config.ts b/packages/codev/src/lib/config.ts index 714bc165c..67d4a68fe 100644 --- a/packages/codev/src/lib/config.ts +++ b/packages/codev/src/lib/config.ts @@ -81,7 +81,7 @@ export interface CodevConfig { * local catalog of ids goes stale the moment a provider ships a new model. The provider is the * authority: a rejected id fails the consultation loudly, with no fallback to the default. */ - models?: Partial>; + models?: Partial>; /** Codex-only; a closed enum bound to the SDK's ModelReasoningEffort union. */ reasoningEffort?: { codex?: ModelReasoningEffort }; /** Codex-only per-1M token rates; all three required together. */ diff --git a/packages/codev/src/lib/consult-lanes.ts b/packages/codev/src/lib/consult-lanes.ts index 812135a4e..a76591be3 100644 --- a/packages/codev/src/lib/consult-lanes.ts +++ b/packages/codev/src/lib/consult-lanes.ts @@ -26,7 +26,7 @@ import { canonicalProtocolName, listProtocolNames, listReviewTypes } from './ske // --------------------------------------------------------------------------- /** Lanes whose model id can be configured. `hermes` is absent: `hermes chat -q` has no model selector. */ -export const MODEL_CONFIGURABLE_LANES = ['claude', 'codex', 'gemini'] as const; +export const MODEL_CONFIGURABLE_LANES = ['claude', 'codex', 'gemini', 'opencode'] as const; export type ConfigurableLane = (typeof MODEL_CONFIGURABLE_LANES)[number]; /** Lanes exposing a reasoning-effort knob. Deliberately narrower than MODEL_CONFIGURABLE_LANES. */ @@ -57,7 +57,7 @@ const _REASONING_EFFORTS_ARE_EXHAUSTIVE: UncoveredEffort extends never ? true : void _REASONING_EFFORTS_ARE_EXHAUSTIVE; /** Lane names accepted in `porch.consultation.*` lists (includes hermes — it IS a review backend). */ -export const VALID_LANE_NAMES = ['gemini', 'codex', 'claude', 'hermes']; +export const VALID_LANE_NAMES = ['gemini', 'codex', 'claude', 'hermes', 'opencode']; /** Whole-value special modes, accepted wherever a lane list is accepted. */ export const SPECIAL_MODES = ['none', 'parent'] as const; @@ -163,6 +163,46 @@ export function assertLaneAcceptsModelOverride(lane: string, flag = '--model-id' ); } +/** + * Reject an `opencode` model id the local `opencode` install does not offer. + * + * This is NOT the hardcoded catalog the header of this file forbids. `available` is whatever + * `opencode models` printed on this machine at call time — the provider tool answering for itself, + * which is the same authority the no-catalog rule defers to. The only difference is that it is + * reachable *before* the spawn. + * + * Reaching it before the spawn is the whole point. A wrong provider prefix (`x-ai/` for `xai/`) + * comes back from the provider as `UnknownError: Unexpected server error` with empty stdout — text + * naming neither the model nor the mistake (live-probed 2026-08-21). So the useful message can only + * be built here, where the intended id and the real catalog are both in hand. + * + * An empty `available` means the catalog could not be read; the caller decides what that means + * rather than this function reading "no list" as "nothing is valid". + */ +export function assertOpencodeModelAvailable( + id: string, + available: readonly string[], + key: string | null, +): void { + if (available.length === 0 || available.includes(id)) return; + + // Same model name, different provider prefix — by far the likeliest way to get here, and exactly + // what the provider's own error is useless for. + const bare = (m: string) => m.slice(m.indexOf('/') + 1); + const samePart = available.filter(m => bare(m) === bare(id)); + + const where = key ? ` (from ${key})` : ''; + const hint = samePart.length > 0 + ? `\nDid you mean ${quoted(samePart)}? The model name is right; the provider prefix is not.` + : ''; + + fail( + `Unknown opencode model ${JSON.stringify(id)}${where}.\n` + + `\`opencode models\` on this machine offers: ${quoted([...available].sort())}.${hint}\n` + + `Codev does not fall back to a default model — correct the id at the source above.` + ); +} + export function validateConsultModels(models: unknown): void { if (models === undefined) return; if (typeof models !== 'object' || models === null || Array.isArray(models)) { diff --git a/packages/codev/src/lib/test-env.ts b/packages/codev/src/lib/test-env.ts index 40e9e6285..b16fb2fe6 100644 --- a/packages/codev/src/lib/test-env.ts +++ b/packages/codev/src/lib/test-env.ts @@ -92,6 +92,39 @@ export function assertAgyLaneAllowedUnderTest(): void { ); } +/** + * Explicit opt-in for deliberately exercising the REAL `opencode` binary from a test. + * + * Same shape as `realAgyOptIn`, for the same reason with a different cost: an unpinned opencode + * lane in a suite does not open a browser window, it bills a real Grok call and takes minutes. + */ +export function realOpencodeOptIn(): boolean { + const raw = process.env.CODEV_ALLOW_REAL_OPENCODE; + return raw === '1' || raw === 'true'; +} + +/** + * Guard the opencode lane against reaching the real binary from a test (#22). + * + * The agy guard's reasoning applies unchanged: the single call site is `resolveOpencodeBin()`, in + * the branch taken when `CODEV_OPENCODE_BIN` is unset, because resolution is not passive — the + * lane's pre-flight *executes* the candidate with `models` before any review runs. + * + * The lane hard-fails rather than skipping, so an unpinned suite would surface as a test failure + * either way; this makes the failure say what is actually wrong instead of "opencode exited 1". + */ +export function assertOpencodeLaneAllowedUnderTest(): void { + if (!isUnderTestRunner()) return; + if (realOpencodeOptIn()) return; + if (process.env.CODEV_OPENCODE_BIN) return; + throw new Error( + 'Refusing to resolve the opencode binary under a test runner without a pinned ' + + 'CODEV_OPENCODE_BIN (#22). This test reached the opencode consult lane by omission and ' + + 'would have spawned the real CLI — a billed Grok call per spawn. Pin a fake binary, or set ' + + 'CODEV_ALLOW_REAL_OPENCODE=1 if this test genuinely means to run the real CLI.', + ); +} + /** * True when cloud-mutating side effects must be refused because we are running * under a test (#1515). From 2915b74e190a8a6c5c77ff9c951c8070fe7f82e1 Mon Sep 17 00:00:00 2001 From: pseudo Date: Fri, 21 Aug 2026 16:50:21 -0600 Subject: [PATCH 03/10] [AIR #22] docs: document the opencode lane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both trees, since a lane adopters can select is framework content: the command reference and the consult skill (all four byte-identical copies), plus CLAUDE.md and its AGENTS.md twin. The `protocol-schema.json` lane enum was missing `hermes` as well as `opencode` — editor tooling only, but it red-squiggled a lane that has been valid for a long time, so it now lists everything VALID_LANE_NAMES accepts. Co-Authored-By: Claude Opus 5 --- .claude/skills/consult/SKILL.md | 15 ++-- .codex/skills/consult/SKILL.md | 15 ++-- AGENTS.md | 9 ++- CLAUDE.md | 9 ++- .../.claude/skills/consult/SKILL.md | 15 ++-- codev-skeleton/.codex/skills/consult/SKILL.md | 15 ++-- codev-skeleton/protocol-schema.json | 2 +- codev-skeleton/resources/commands/consult.md | 36 +++++++-- codev/resources/commands/consult.md | 36 +++++++-- codev/state/air-22_thread.md | 77 +++++++++++++++++++ 10 files changed, 188 insertions(+), 41 deletions(-) create mode 100644 codev/state/air-22_thread.md diff --git a/.claude/skills/consult/SKILL.md b/.claude/skills/consult/SKILL.md index bf6a7bd4a..da627dabf 100644 --- a/.claude/skills/consult/SKILL.md +++ b/.claude/skills/consult/SKILL.md @@ -28,6 +28,7 @@ is not a thing, `-m codex --model-id gpt-5.6-sol` is. | `gemini` | `pro` | Antigravity CLI (`agy`); agentic file access (`--sandbox`), OAuth login; skips non-blockingly if unavailable | | `codex` | `gpt` | Thorough (~200-250s), shell exploration | | `claude` | `opus` | Agent SDK with tool use (~60-120s) | +| `opencode` | - | `opencode run` (Grok, default `xai/grok-4.6`); agentic file access; hard-fails rather than skipping | ## All flags @@ -35,11 +36,15 @@ is not a thing, `-m codex --model-id gpt-5.6-sol` is. -m, --model Lane to use (required except stats) --model-id Pin the provider model for this run, e.g. `--model-id gpt-5.6-sol`. Outranks config - `consult.models.`. Works on the claude, codex - and gemini lanes; using it with a lane that has no - model selector (hermes) is an error, not a no-op. - Syntax is validated here; whether the model exists - is the provider's call and fails loudly. + `consult.models.`. Works on the claude, codex, + gemini and opencode lanes; using it with a lane that + has no model selector (hermes) is an error, not a + no-op. Syntax is validated here; whether the model + exists is the provider's call and fails loudly. + Exception: an opencode id is checked against + `opencode models` first — the prefix is `xai/`, not + `x-ai/`, and the provider's own rejection says so + nowhere. --prompt Inline prompt (general mode) --prompt-file Prompt file path (general mode) --protocol Protocol: spir, aspir, air, bugfix, maintain diff --git a/.codex/skills/consult/SKILL.md b/.codex/skills/consult/SKILL.md index bf6a7bd4a..da627dabf 100644 --- a/.codex/skills/consult/SKILL.md +++ b/.codex/skills/consult/SKILL.md @@ -28,6 +28,7 @@ is not a thing, `-m codex --model-id gpt-5.6-sol` is. | `gemini` | `pro` | Antigravity CLI (`agy`); agentic file access (`--sandbox`), OAuth login; skips non-blockingly if unavailable | | `codex` | `gpt` | Thorough (~200-250s), shell exploration | | `claude` | `opus` | Agent SDK with tool use (~60-120s) | +| `opencode` | - | `opencode run` (Grok, default `xai/grok-4.6`); agentic file access; hard-fails rather than skipping | ## All flags @@ -35,11 +36,15 @@ is not a thing, `-m codex --model-id gpt-5.6-sol` is. -m, --model Lane to use (required except stats) --model-id Pin the provider model for this run, e.g. `--model-id gpt-5.6-sol`. Outranks config - `consult.models.`. Works on the claude, codex - and gemini lanes; using it with a lane that has no - model selector (hermes) is an error, not a no-op. - Syntax is validated here; whether the model exists - is the provider's call and fails loudly. + `consult.models.`. Works on the claude, codex, + gemini and opencode lanes; using it with a lane that + has no model selector (hermes) is an error, not a + no-op. Syntax is validated here; whether the model + exists is the provider's call and fails loudly. + Exception: an opencode id is checked against + `opencode models` first — the prefix is `xai/`, not + `x-ai/`, and the provider's own rejection says so + nowhere. --prompt Inline prompt (general mode) --prompt-file Prompt file path (general mode) --protocol Protocol: spir, aspir, air, bugfix, maintain diff --git a/AGENTS.md b/AGENTS.md index 92039a19a..b007e2621 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -135,11 +135,16 @@ apps/streamdeck; Elgato channel: profiles, Maker Console, packaging) · core · ## Multi-agent consultation -**Enabled by default.** Three reviewers: **Gemini** via the Antigravity CLI (`agy`, skips -non-blockingly if unauthenticated), **GPT-5.6 Sol** (`gpt-5.6-sol` — the `-sol` suffix is +**Enabled by default.** Three reviewers by default: **Gemini** via the Antigravity CLI (`agy`, +skips non-blockingly if unauthenticated), **GPT-5.6 Sol** (`gpt-5.6-sol` — the `-sol` suffix is load-bearing) via the Codex SDK, and **Claude Opus 5** via the Agent SDK. Disable only when the user says "without consultation". +A fourth lane, **`opencode`** (`opencode run`, default `xai/grok-4.6` — the prefix is `xai/`, not +`x-ai/`), is available but not in the default rotation. Reach for it when a default lane is +quota-exhausted or unauthenticated: it is the one reviewer on an account none of the others share. +Unlike the agy lane it never skips — every failure is loud. + Consult after writing implementation code and after writing tests, before presenting results. **"cmap"** means run all three in parallel *in the background* and return control immediately. diff --git a/CLAUDE.md b/CLAUDE.md index 92039a19a..b007e2621 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -135,11 +135,16 @@ apps/streamdeck; Elgato channel: profiles, Maker Console, packaging) · core · ## Multi-agent consultation -**Enabled by default.** Three reviewers: **Gemini** via the Antigravity CLI (`agy`, skips -non-blockingly if unauthenticated), **GPT-5.6 Sol** (`gpt-5.6-sol` — the `-sol` suffix is +**Enabled by default.** Three reviewers by default: **Gemini** via the Antigravity CLI (`agy`, +skips non-blockingly if unauthenticated), **GPT-5.6 Sol** (`gpt-5.6-sol` — the `-sol` suffix is load-bearing) via the Codex SDK, and **Claude Opus 5** via the Agent SDK. Disable only when the user says "without consultation". +A fourth lane, **`opencode`** (`opencode run`, default `xai/grok-4.6` — the prefix is `xai/`, not +`x-ai/`), is available but not in the default rotation. Reach for it when a default lane is +quota-exhausted or unauthenticated: it is the one reviewer on an account none of the others share. +Unlike the agy lane it never skips — every failure is loud. + Consult after writing implementation code and after writing tests, before presenting results. **"cmap"** means run all three in parallel *in the background* and return control immediately. diff --git a/codev-skeleton/.claude/skills/consult/SKILL.md b/codev-skeleton/.claude/skills/consult/SKILL.md index bf6a7bd4a..da627dabf 100644 --- a/codev-skeleton/.claude/skills/consult/SKILL.md +++ b/codev-skeleton/.claude/skills/consult/SKILL.md @@ -28,6 +28,7 @@ is not a thing, `-m codex --model-id gpt-5.6-sol` is. | `gemini` | `pro` | Antigravity CLI (`agy`); agentic file access (`--sandbox`), OAuth login; skips non-blockingly if unavailable | | `codex` | `gpt` | Thorough (~200-250s), shell exploration | | `claude` | `opus` | Agent SDK with tool use (~60-120s) | +| `opencode` | - | `opencode run` (Grok, default `xai/grok-4.6`); agentic file access; hard-fails rather than skipping | ## All flags @@ -35,11 +36,15 @@ is not a thing, `-m codex --model-id gpt-5.6-sol` is. -m, --model Lane to use (required except stats) --model-id Pin the provider model for this run, e.g. `--model-id gpt-5.6-sol`. Outranks config - `consult.models.`. Works on the claude, codex - and gemini lanes; using it with a lane that has no - model selector (hermes) is an error, not a no-op. - Syntax is validated here; whether the model exists - is the provider's call and fails loudly. + `consult.models.`. Works on the claude, codex, + gemini and opencode lanes; using it with a lane that + has no model selector (hermes) is an error, not a + no-op. Syntax is validated here; whether the model + exists is the provider's call and fails loudly. + Exception: an opencode id is checked against + `opencode models` first — the prefix is `xai/`, not + `x-ai/`, and the provider's own rejection says so + nowhere. --prompt Inline prompt (general mode) --prompt-file Prompt file path (general mode) --protocol Protocol: spir, aspir, air, bugfix, maintain diff --git a/codev-skeleton/.codex/skills/consult/SKILL.md b/codev-skeleton/.codex/skills/consult/SKILL.md index bf6a7bd4a..da627dabf 100644 --- a/codev-skeleton/.codex/skills/consult/SKILL.md +++ b/codev-skeleton/.codex/skills/consult/SKILL.md @@ -28,6 +28,7 @@ is not a thing, `-m codex --model-id gpt-5.6-sol` is. | `gemini` | `pro` | Antigravity CLI (`agy`); agentic file access (`--sandbox`), OAuth login; skips non-blockingly if unavailable | | `codex` | `gpt` | Thorough (~200-250s), shell exploration | | `claude` | `opus` | Agent SDK with tool use (~60-120s) | +| `opencode` | - | `opencode run` (Grok, default `xai/grok-4.6`); agentic file access; hard-fails rather than skipping | ## All flags @@ -35,11 +36,15 @@ is not a thing, `-m codex --model-id gpt-5.6-sol` is. -m, --model Lane to use (required except stats) --model-id Pin the provider model for this run, e.g. `--model-id gpt-5.6-sol`. Outranks config - `consult.models.`. Works on the claude, codex - and gemini lanes; using it with a lane that has no - model selector (hermes) is an error, not a no-op. - Syntax is validated here; whether the model exists - is the provider's call and fails loudly. + `consult.models.`. Works on the claude, codex, + gemini and opencode lanes; using it with a lane that + has no model selector (hermes) is an error, not a + no-op. Syntax is validated here; whether the model + exists is the provider's call and fails loudly. + Exception: an opencode id is checked against + `opencode models` first — the prefix is `xai/`, not + `x-ai/`, and the provider's own rejection says so + nowhere. --prompt Inline prompt (general mode) --prompt-file Prompt file path (general mode) --protocol Protocol: spir, aspir, air, bugfix, maintain diff --git a/codev-skeleton/protocol-schema.json b/codev-skeleton/protocol-schema.json index 96a8ccc8a..a8f193cc5 100644 --- a/codev-skeleton/protocol-schema.json +++ b/codev-skeleton/protocol-schema.json @@ -169,7 +169,7 @@ "description": "Models to consult", "items": { "type": "string", - "enum": ["gemini", "codex", "claude"] + "enum": ["gemini", "codex", "claude", "hermes", "opencode"] }, "minItems": 1 }, diff --git a/codev-skeleton/resources/commands/consult.md b/codev-skeleton/resources/commands/consult.md index 1adcaff74..42a1e86cd 100644 --- a/codev-skeleton/resources/commands/consult.md +++ b/codev-skeleton/resources/commands/consult.md @@ -21,7 +21,8 @@ consult stats [options] --model-id Override the provider model id for THIS invocation ``` -`-m/--model` picks the **lane** (`claude`, `codex`, `gemini`, `hermes`); `--model-id` picks the +`-m/--model` picks the **lane** (`claude`, `codex`, `gemini`, `hermes`, `opencode`); `--model-id` +picks the **model that lane runs**. The two are independent — see [Configuration](#configuration) for setting an id persistently instead. @@ -30,12 +31,15 @@ consult -m codex --model-id gpt-5.6-sol --prompt "Review this design" ``` - **Precedence**: `--model-id` > `consult.models.` > the lane's shipped default. -- **Supported lanes**: `claude`, `codex`, `gemini`. Using it with `hermes` is an **error**, not a - silent no-op — `hermes chat -q` has no model selector, so accepting the flag there would mean - ignoring it. -- **Validation is syntax-only.** Whether the id exists is the provider's call; a rejection fails - loudly with no fallback to the default. See - [the fail-fast contract](#the-fail-fast-contract-and-where-it-stops). +- **Supported lanes**: `claude`, `codex`, `gemini`, `opencode`. Using it with `hermes` is an + **error**, not a silent no-op — `hermes chat -q` has no model selector, so accepting the flag + there would mean ignoring it. +- **Validation is syntax-only**, with one exception. Whether the id exists is normally the + provider's call; a rejection fails loudly with no fallback to the default. See + [the fail-fast contract](#the-fail-fast-contract-and-where-it-stops). The exception is + `opencode`, whose id is checked against `opencode models` *before* the run — the provider + rejects an unknown id with a bare `UnknownError: Unexpected server error` and empty output, + which names neither the model nor the mistake. ## Models @@ -45,6 +49,7 @@ consult -m codex --model-id gpt-5.6-sol --prompt "Review this design" | `codex` | `gpt` | @openai/codex | `gpt-5.6-sol` (medium reasoning effort) | Read-only sandbox, thorough | | `claude` | `opus` | Claude Agent SDK | `claude-opus-5` | Balanced analysis with tool use | | `hermes` | - | hermes CLI (`hermes chat -q`) | *(hermes' own default)* | Uses Hermes agent as consult backend | +| `opencode` | - | opencode CLI (`opencode run`) | `xai/grok-4.6` | Agentic file access. A reviewer on an account no other lane shares. **Hard-fails** — never a silent skip. | > **The codex lane's `-sol` suffix is load-bearing.** Plain `gpt-5.6` and `gpt-5.6-codex` are both > rejected by Codex when running on a ChatGPT account (`The '' model is not supported when @@ -90,11 +95,14 @@ single invocation by [`--model-id`](#model-selection-options). { "consult": { "models": { "claude": "claude-opus-5", "codex": "gpt-5.6-sol" } } } ``` -Valid lanes: `claude`, `codex`, `gemini`. **`hermes` is rejected** — it is invoked as +Valid lanes: `claude`, `codex`, `gemini`, `opencode`. **`hermes` is rejected** — it is invoked as `hermes chat -q` and exposes no model selector, so configuring one would silently do nothing. (`hermes` remains valid in `porch.consultation` lane lists; the two key spaces differ on purpose.) The `gemini` lane passes the id to `agy --model`, so the id space is agy's, not Google's API's. +The `opencode` lane passes it to `opencode run -m`, so the id space is opencode's: a +`provider/model` pair exactly as `opencode models` prints it. The prefix is `xai/`, **not** +`x-ai/` — the wrong one is rejected before the run, naming the right one. ### `consult.reasoningEffort` @@ -211,11 +219,23 @@ Codev checks a model id's *syntax* only (ASCII alphanumerics plus `. _ : / @ + - no leading punctuation) — never its existence. **There is no allowlist of model ids anywhere in Codev, by design**: a new model must work the day the provider ships it, without a Codev release. +The `opencode` lane looks like an exception and is not one. It checks the id against +`opencode models` before spawning — but that list is the provider tool answering for itself at call +time, not a catalog Codev ships, so it cannot go stale. The check exists because opencode's own +rejection is unusable: a wrong provider prefix comes back as `UnknownError: Unexpected server +error` with empty output, naming neither the model nor the mistake. If the listing itself fails, +the check stands down and the provider is the authority again. + So a typo'd model id is not caught at config time. It reaches the backend, which rejects it; that lane exits non-zero, the provider's error text is surfaced, the config key that supplied the id is named, and **no review file is written** — so porch cannot advance on a lane that never ran. What you do *not* get is a silent substitution of the default model. +The `opencode` lane takes the strict side of this contract with no exceptions at all: a missing +CLI, an unknown id, a non-zero exit, and a clean exit that produced nothing all fail the lane and +leave no review file. It has no OAuth-fragility to accommodate, and a lane that quietly produces +nothing is a lane porch counts as an approval. + One deliberate exception: a `gemini` lane **with no model id resolved** still skips non-blockingly when `agy` is missing or unauthenticated (consultation is best-effort there). Once an id *is* resolved — from either `consult.models.gemini` **or** `--model-id` — a rejected model becomes a hard diff --git a/codev/resources/commands/consult.md b/codev/resources/commands/consult.md index 1adcaff74..42a1e86cd 100644 --- a/codev/resources/commands/consult.md +++ b/codev/resources/commands/consult.md @@ -21,7 +21,8 @@ consult stats [options] --model-id Override the provider model id for THIS invocation ``` -`-m/--model` picks the **lane** (`claude`, `codex`, `gemini`, `hermes`); `--model-id` picks the +`-m/--model` picks the **lane** (`claude`, `codex`, `gemini`, `hermes`, `opencode`); `--model-id` +picks the **model that lane runs**. The two are independent — see [Configuration](#configuration) for setting an id persistently instead. @@ -30,12 +31,15 @@ consult -m codex --model-id gpt-5.6-sol --prompt "Review this design" ``` - **Precedence**: `--model-id` > `consult.models.` > the lane's shipped default. -- **Supported lanes**: `claude`, `codex`, `gemini`. Using it with `hermes` is an **error**, not a - silent no-op — `hermes chat -q` has no model selector, so accepting the flag there would mean - ignoring it. -- **Validation is syntax-only.** Whether the id exists is the provider's call; a rejection fails - loudly with no fallback to the default. See - [the fail-fast contract](#the-fail-fast-contract-and-where-it-stops). +- **Supported lanes**: `claude`, `codex`, `gemini`, `opencode`. Using it with `hermes` is an + **error**, not a silent no-op — `hermes chat -q` has no model selector, so accepting the flag + there would mean ignoring it. +- **Validation is syntax-only**, with one exception. Whether the id exists is normally the + provider's call; a rejection fails loudly with no fallback to the default. See + [the fail-fast contract](#the-fail-fast-contract-and-where-it-stops). The exception is + `opencode`, whose id is checked against `opencode models` *before* the run — the provider + rejects an unknown id with a bare `UnknownError: Unexpected server error` and empty output, + which names neither the model nor the mistake. ## Models @@ -45,6 +49,7 @@ consult -m codex --model-id gpt-5.6-sol --prompt "Review this design" | `codex` | `gpt` | @openai/codex | `gpt-5.6-sol` (medium reasoning effort) | Read-only sandbox, thorough | | `claude` | `opus` | Claude Agent SDK | `claude-opus-5` | Balanced analysis with tool use | | `hermes` | - | hermes CLI (`hermes chat -q`) | *(hermes' own default)* | Uses Hermes agent as consult backend | +| `opencode` | - | opencode CLI (`opencode run`) | `xai/grok-4.6` | Agentic file access. A reviewer on an account no other lane shares. **Hard-fails** — never a silent skip. | > **The codex lane's `-sol` suffix is load-bearing.** Plain `gpt-5.6` and `gpt-5.6-codex` are both > rejected by Codex when running on a ChatGPT account (`The '' model is not supported when @@ -90,11 +95,14 @@ single invocation by [`--model-id`](#model-selection-options). { "consult": { "models": { "claude": "claude-opus-5", "codex": "gpt-5.6-sol" } } } ``` -Valid lanes: `claude`, `codex`, `gemini`. **`hermes` is rejected** — it is invoked as +Valid lanes: `claude`, `codex`, `gemini`, `opencode`. **`hermes` is rejected** — it is invoked as `hermes chat -q` and exposes no model selector, so configuring one would silently do nothing. (`hermes` remains valid in `porch.consultation` lane lists; the two key spaces differ on purpose.) The `gemini` lane passes the id to `agy --model`, so the id space is agy's, not Google's API's. +The `opencode` lane passes it to `opencode run -m`, so the id space is opencode's: a +`provider/model` pair exactly as `opencode models` prints it. The prefix is `xai/`, **not** +`x-ai/` — the wrong one is rejected before the run, naming the right one. ### `consult.reasoningEffort` @@ -211,11 +219,23 @@ Codev checks a model id's *syntax* only (ASCII alphanumerics plus `. _ : / @ + - no leading punctuation) — never its existence. **There is no allowlist of model ids anywhere in Codev, by design**: a new model must work the day the provider ships it, without a Codev release. +The `opencode` lane looks like an exception and is not one. It checks the id against +`opencode models` before spawning — but that list is the provider tool answering for itself at call +time, not a catalog Codev ships, so it cannot go stale. The check exists because opencode's own +rejection is unusable: a wrong provider prefix comes back as `UnknownError: Unexpected server +error` with empty output, naming neither the model nor the mistake. If the listing itself fails, +the check stands down and the provider is the authority again. + So a typo'd model id is not caught at config time. It reaches the backend, which rejects it; that lane exits non-zero, the provider's error text is surfaced, the config key that supplied the id is named, and **no review file is written** — so porch cannot advance on a lane that never ran. What you do *not* get is a silent substitution of the default model. +The `opencode` lane takes the strict side of this contract with no exceptions at all: a missing +CLI, an unknown id, a non-zero exit, and a clean exit that produced nothing all fail the lane and +leave no review file. It has no OAuth-fragility to accommodate, and a lane that quietly produces +nothing is a lane porch counts as an approval. + One deliberate exception: a `gemini` lane **with no model id resolved** still skips non-blockingly when `agy` is missing or unauthenticated (consultation is best-effort there). Once an id *is* resolved — from either `consult.models.gemini` **or** `--model-id` — a rejected model becomes a hard diff --git a/codev/state/air-22_thread.md b/codev/state/air-22_thread.md new file mode 100644 index 000000000..e22b7f8bc --- /dev/null +++ b/codev/state/air-22_thread.md @@ -0,0 +1,77 @@ +# air-22 — Add opencode as a consult lane (Grok) + +Issue #22. AIR protocol, strict mode. + +## Live probes before writing code (2026-08-21) + +Everything below was run against the real `opencode` on this machine (v1.18.18), +not inferred: + +- `opencode run -m xai/grok-4.6 "…"` → exit 0, **clean plain-text stdout**, the + `VERDICT: APPROVE` line survives verbatim. Progress/banner noise goes to stderr. + So `parseVerdict` needs no change — confirmed rather than assumed, per the issue. +- `opencode run -m x-ai/grok-4.6 "…"` → exit 1, **empty stdout**, stderr carries + `UnknownError: Unexpected server error`. Provider gives no usable hint that the + prefix is wrong, which is exactly why the lane pre-validates. +- `opencode models` → 19 ids, authoritative and live (`xai/grok-4.6`, `xai/grok-4.3`, …). + That is the catalog to validate against. +- Agentic file reads work with no `--auto` and no extra flags, cwd = workspace root. + +## Design decisions + +**Pre-flight model validation against `opencode models`, not a hardcoded list.** +`consult-lanes.ts` opens with an explicit rule: model ids are never validated +against a local catalog, because a catalog goes stale the day a provider ships a +model. That rule is about *hardcoded* catalogs. `opencode models` is the provider +tool answering for itself at call time, so it is the same authority the rule +defers to — just reachable before the spawn instead of after. Validating there +lets `x-ai/grok-4.6` be rejected by name with the right prefix, which the +provider's own `UnknownError` never does. + +**The lane hard-fails; it does not skip.** The gemini/agy lane degrades to a +non-blocking COMMENT skip because it is OAuth-fragile. opencode has no such +property, and #20 is the standing evidence that a lane which produces nothing +gets counted as an approval. So: unknown model, missing CLI, non-zero exit, or +empty output all throw. + +**Default model `xai/grok-4.6`** — the whole point of the lane is a reviewer on a +different account from every existing one, and 4.6 is the strongest Grok listed. + +## What changed + +- `consult-lanes.ts` — `opencode` joins `VALID_LANE_NAMES` and `MODEL_CONFIGURABLE_LANES`; + new pure `assertOpencodeModelAvailable(id, available, key)`. +- `consult/index.ts` — `MODEL_CONFIGS.opencode`, `DEFAULT_OPENCODE_MODEL`, + `resolveOpencodeBin()`, `listOpencodeModels()`, `opencodeReviewHeader()`, + `runOpencodeConsultation()`, plus dispatch and the file-access hint. +- `test-env.ts` — `assertOpencodeLaneAllowedUnderTest()`. Not incidental: without it a + suite that forgets to pin `CODEV_OPENCODE_BIN` bills a real Grok call per spawn. The + agy lane already had exactly this guard for exactly this reason (#1323). +- Docs: `resources/commands/consult.md`, the consult SKILL (4 byte-identical copies), + CLAUDE.md + AGENTS.md, and the `protocol-schema.json` lane enum — which was missing + `hermes` too, so it now lists every lane `VALID_LANE_NAMES` accepts. + +`codev doctor` already probed for OpenCode as an AI CLI dependency. Nothing to add there, +and further evidence for the issue's read that the lane's absence was an oversight, not a +decision. + +`porch/next.ts` needed no change — it iterates whatever lanes are configured rather than +matching against a hardcoded list, so `opencode` in `porch.consultation.models` works. + +## End-to-end verification (the issue asks for this explicitly, not unit tests alone) + +Real `opencode`, real Grok, built CLI: + +- `consult -m opencode --prompt "…"` → exit 0, 36.3s, review written. Fed the resulting + file to porch's own `parseVerdict` → `APPROVE`. The banner opencode writes to stderr + stays out of the review; the model provenance line is at the top and carries no + `VERDICT` token, so it cannot shadow the real verdict (which is found last→first). +- `consult -m opencode --model-id x-ai/grok-4.6` → exit 1, and the message names + `xai/grok-4.6` as the id meant, lists the machine's real catalog, and says Codev does + not fall back. + +## Defaults left alone + +`porch.consultation.models` still defaults to `["gemini", "codex", "claude"]`. Changing the +default rotation is an architectural call the issue does not make, so opencode is available +but opt-in. From 3b0dd875677397501e847b708f73a778f1dfc203 Mon Sep 17 00:00:00 2001 From: pseudo Date: Fri, 21 Aug 2026 16:50:49 -0600 Subject: [PATCH 04/10] chore(porch): 22 pr phase-transition --- codev/projects/22-add-opencode-as-a-consult-lane/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml b/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml index e9094f367..741d8776c 100644 --- a/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml +++ b/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml @@ -1,7 +1,7 @@ id: '22' title: add-opencode-as-a-consult-lane protocol: air -phase: implement +phase: pr plan_phases: [] current_plan_phase: null gates: @@ -11,4 +11,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-08-21T22:35:07.319Z' -updated_at: '2026-08-21T22:35:07.319Z' +updated_at: '2026-08-21T22:50:49.055Z' From 4658aceb7cebd2b69f07cdf17b37dbf4146330c3 Mon Sep 17 00:00:00 2001 From: pseudo Date: Fri, 21 Aug 2026 17:03:22 -0600 Subject: [PATCH 05/10] [AIR #22] fix: refuse a review that states no verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by the opencode lane reviewing its own PR, on its first run, in the exact failure class the lane exists to reduce. `opencodeReviewHeader` — the provenance banner added to satisfy the issue's "record which model the lane used" — is 76 characters. `parseVerdict` treats output under 50 characters as REQUEST_CHANGES; anything longer with no verdict line falls through to COMMENT, and `allApprove` counts COMMENT as an approval. So the banner converted the safe default into the unsafe one for any short review: parseVerdict('ok') -> REQUEST_CHANGES parseVerdict(header + 'ok') -> COMMENT The PR that claims to reduce #20's blast radius was opening a fresh #20 hole. The header is not really the bug. That 50-character floor is a PROXY — length standing in for "a review actually happened" — and it was never measuring the right thing. It held by luck, and any lane that prefixes provenance, a banner, a model id or a timestamp defeats it. Raising 50 to 100 reintroduces the problem with the next slightly longer header. So ask the real question. `findVerdict()` is extracted from `parseVerdict()` and returns the verdict a review STATES, or null — the distinction `parseVerdict` structurally cannot express, since a stated COMMENT and no verdict at all come back identical. It is the same distinction issue #20 needs; whoever picks that up should reuse this rather than write a second one. `parseVerdict` keeps its exact prior behaviour, now expressed as `findVerdict(output) ?? 'COMMENT'`. The opencode lane then hard-fails a protocol-mode review with no verdict: a reviewer that produced output but no verdict has not reviewed. General mode (`--prompt`) is exempt — a question is not a review, and none was asked for. Also from claude's review, all verified against the source first: - a stale `CODEV_OPENCODE_BIN` said "install opencode" rather than naming the override that moved - `listOpencodeModels` keeps only `provider/model`-shaped lines, so a future decorated listing degrades to "catalog unreadable" — handing authority back to the provider — instead of turning a valid id into a hard failure - tests for the large-prompt temp-file branch, which had none - `persistent-output.test.ts` and `lane-models.test.ts` lane loops extended Co-Authored-By: Claude Opus 5 --- .../consult/__tests__/lane-models.test.ts | 4 +- .../consult/__tests__/opencode-lane.test.ts | 129 +++++++++++++++++- .../__tests__/persistent-output.test.ts | 4 +- packages/codev/src/commands/consult/index.ts | 63 ++++++++- .../porch/__tests__/parse-verdict.test.ts | 57 +++++++- packages/codev/src/commands/porch/verdict.ts | 33 +++-- 6 files changed, 266 insertions(+), 24 deletions(-) diff --git a/packages/codev/src/commands/consult/__tests__/lane-models.test.ts b/packages/codev/src/commands/consult/__tests__/lane-models.test.ts index 8f7ba802c..6e515ad87 100644 --- a/packages/codev/src/commands/consult/__tests__/lane-models.test.ts +++ b/packages/codev/src/commands/consult/__tests__/lane-models.test.ts @@ -227,13 +227,13 @@ describe('--model-id is refused by lanes with no model selector', () => { } catch (err) { message = (err as Error).message; } - for (const lane of ['claude', 'codex', 'gemini']) expect(message).toContain(lane); + for (const lane of ['claude', 'codex', 'gemini', 'opencode']) expect(message).toContain(lane); }); it('accepts every configurable lane, gemini included', () => { // gemini is configurable by spec; its passthrough lands in phase_3. Asserting it here means // phase_3 cannot narrow this contract without failing a test. - for (const lane of ['claude', 'codex', 'gemini']) { + for (const lane of ['claude', 'codex', 'gemini', 'opencode']) { expect(() => assertLaneAcceptsModelOverride(lane)).not.toThrow(); } }); diff --git a/packages/codev/src/commands/consult/__tests__/opencode-lane.test.ts b/packages/codev/src/commands/consult/__tests__/opencode-lane.test.ts index be3a54743..e5b6b2a2f 100644 --- a/packages/codev/src/commands/consult/__tests__/opencode-lane.test.ts +++ b/packages/codev/src/commands/consult/__tests__/opencode-lane.test.ts @@ -30,6 +30,7 @@ import { resolveLaneModelChoice, DEFAULT_OPENCODE_MODEL, } from '../index.js'; +import { findVerdict, parseVerdict } from '../../porch/verdict.js'; import { assertOpencodeModelAvailable, MODEL_CONFIGURABLE_LANES, @@ -68,6 +69,11 @@ if (mode === 'reject') { process.exit(1); } if (mode === 'empty') { process.exit(0); } +if (mode === 'no-verdict-short') { process.stdout.write('ok'); process.exit(0); } +if (mode === 'no-verdict-long') { + process.stdout.write('A long review that discusses the code at length but never states a verdict line.\\n'); + process.exit(0); +} process.stdout.write('Looks fine to me.\\n\\nVERDICT: APPROVE\\nSUMMARY: ok\\nCONFIDENCE: HIGH\\n'); process.exit(0); `; @@ -257,9 +263,13 @@ describe('nothing degrades into a passing review (#20)', () => { .rejects.toThrow(/produced no review output/); }); - it('a missing CLI rejects', async () => { + it('an unresolvable binary rejects rather than skipping', async () => { + // The agy lane emits a COMMENT skip here; this one refuses. Which of the two resolution + // failures produced it is asserted separately, under "a stale CODEV_OPENCODE_BIN says so" — + // the bare not-on-PATH branch is unreachable from a suite by design, because the isolation + // guard fires before resolution can reach the real install. process.env.CODEV_OPENCODE_BIN = path.join(dir, 'not-installed'); - await expect(_runOpencodeConsultation('q', 'role', dir)).rejects.toThrow(/opencode not found/); + await expect(_runOpencodeConsultation('q', 'role', dir)).rejects.toThrow(/does not exist/); }); it('leaves no stale review file for porch to accept', async () => { @@ -313,3 +323,118 @@ describe('binary resolution', () => { expect(listOpencodeModels(path.join(dir, 'nope'))).toEqual([]); }); }); + +// --- the verdict requirement ----------------------------------------------------------- + +/** + * A protocol-mode review that states no verdict is refused. + * + * This is the defect the lane found in its own first review, and it is worth stating precisely + * because the obvious "fix" is wrong. `parseVerdict` treats output under 50 characters as + * REQUEST_CHANGES — a floor meant to catch a lane that produced nothing useful. But length was + * always a *proxy* for "a review happened", and this lane's 76-character provenance banner clears + * the floor on its own: a two-word non-answer becomes COMMENT, and `allApprove` counts COMMENT as + * an approval. The header did not break a working guard; it exposed one that was already measuring + * the wrong thing, and raising 50 to 100 would only postpone the next banner. + * + * So the lane asks the real question — did the reviewer state a verdict? — via `findVerdict`. + */ +describe('a protocol-mode review must state a verdict', () => { + it('rejects a short verdict-less review the header would otherwise smuggle past the floor', async () => { + process.env.FAKE_OPENCODE_MODE = 'no-verdict-short'; + await expect(_runOpencodeConsultation('q', 'role', dir, undefined, undefined, undefined, true)) + .rejects.toThrow(/no VERDICT line/); + }); + + it('rejects a LONG verdict-less review too — this is not about length', async () => { + process.env.FAKE_OPENCODE_MODE = 'no-verdict-long'; + await expect(_runOpencodeConsultation('q', 'role', dir, undefined, undefined, undefined, true)) + .rejects.toThrow(/no VERDICT line/); + }); + + it('leaves no review file behind when it rejects', async () => { + process.env.FAKE_OPENCODE_MODE = 'no-verdict-long'; + const outputPath = path.join(dir, 'review.md'); + await _runOpencodeConsultation('q', 'role', dir, outputPath, undefined, undefined, true) + .catch(() => {}); + expect(fs.existsSync(outputPath)).toBe(false); + }); + + it('does not require a verdict in general mode, where none was asked for', async () => { + // `consult -m opencode --prompt "..."` is a question, not a review. + process.env.FAKE_OPENCODE_MODE = 'no-verdict-long'; + await expect(_runOpencodeConsultation('q', 'role', dir, undefined, undefined, undefined, false)) + .resolves.toBeUndefined(); + }); + + it('accepts a review that does state one', async () => { + const outputPath = path.join(dir, 'review.md'); + await _runOpencodeConsultation('q', 'role', dir, outputPath, undefined, undefined, true); + expect(fs.readFileSync(outputPath, 'utf-8')).toContain('VERDICT: APPROVE'); + }); + + it('checks the verdict against the review, not against the header', () => { + // Guards the regression directly: were the check applied to header+review, a future header + // carrying the word VERDICT would satisfy it without the reviewer saying anything. + const header = opencodeReviewHeader({ + id: 'xai/grok-4.6', key: null, source: null, fromFlag: false, + }); + expect(findVerdict(header + 'ok')).toBeNull(); + expect(parseVerdict(header + 'ok')).toBe('COMMENT'); // what porch would have believed + }); +}); + +// --- large prompts ---------------------------------------------------------------------- + +describe('a prompt too large for argv', () => { + it('goes to a temp file that opencode is pointed at', async () => { + const huge = 'x'.repeat(150_000); + await _runOpencodeConsultation(huge, 'role', dir); + const promptArg = opencodeArgv().at(-1)!; + // ARG_MAX: the prompt must NOT be inline. + expect(promptArg.length).toBeLessThan(1000); + expect(promptArg).toMatch(/Read the full consultation prompt from this file: .*\.md/); + + const tempFile = promptArg.match(/from this file: (\S+\.md)/)![1]; + // Cleaned up after the run — the file existed only for opencode to read. + expect(fs.existsSync(tempFile)).toBe(false); + }); + + it('keeps a normal prompt inline', async () => { + await _runOpencodeConsultation('a short query', 'role', dir); + expect(opencodeArgv().at(-1)!).toContain('a short query'); + }); +}); + +// --- catalog parsing -------------------------------------------------------------------- + +describe('catalog parsing tolerates a decorated listing', () => { + it('keeps only provider/model lines', () => { + const decorated = path.join(dir, 'opencode-decorated'); + fs.writeFileSync(decorated, `#!/usr/bin/env node +process.stdout.write('Available models:\\n\\n xai/grok-4.6 (default)\\nxai/grok-4.3\\n'); +process.exit(0); +`, { mode: 0o755 }); + // The header and the annotated line are dropped rather than parsed as ids. + expect(listOpencodeModels(decorated)).toEqual(['xai/grok-4.3']); + }); + + it('a listing with no parseable line reads as "unknown", never as "nothing is valid"', () => { + // Otherwise a cosmetic change in someone else's CLI turns every valid id into a hard failure. + const garbage = path.join(dir, 'opencode-garbage'); + fs.writeFileSync(garbage, `#!/usr/bin/env node +process.stdout.write('Error: could not reach the model registry\\n'); +process.exit(0); +`, { mode: 0o755 }); + expect(listOpencodeModels(garbage)).toEqual([]); + expect(() => assertOpencodeModelAvailable('xai/grok-4.6', [], null)).not.toThrow(); + }); +}); + +describe('a stale CODEV_OPENCODE_BIN says so', () => { + it('does not tell you to install opencode when the override is the problem', async () => { + process.env.CODEV_OPENCODE_BIN = path.join(dir, 'moved-away'); + await expect(_runOpencodeConsultation('q', 'role', dir)) + .rejects.toThrow(/CODEV_OPENCODE_BIN points at .*moved-away, which does not exist/); + }); +}); diff --git a/packages/codev/src/commands/consult/__tests__/persistent-output.test.ts b/packages/codev/src/commands/consult/__tests__/persistent-output.test.ts index e1dd8c073..8007a8f6e 100644 --- a/packages/codev/src/commands/consult/__tests__/persistent-output.test.ts +++ b/packages/codev/src/commands/consult/__tests__/persistent-output.test.ts @@ -80,11 +80,11 @@ describe('computePersistentOutputPath', () => { projectDir: '/workspace/codev/projects/0073-my-feature', }; - for (const model of ['gemini', 'codex', 'claude', 'hermes']) { + for (const model of ['gemini', 'codex', 'claude', 'hermes', 'opencode']) { const result = computePersistentOutputPath(state, model); const fileName = result.split('/').pop()!; // Must match: --iter-.txt - expect(fileName).toMatch(/^0073-phase_1-iter1-(gemini|codex|claude|hermes)\.txt$/); + expect(fileName).toMatch(/^0073-phase_1-iter1-(gemini|codex|claude|hermes|opencode)\.txt$/); } }); }); diff --git a/packages/codev/src/commands/consult/index.ts b/packages/codev/src/commands/consult/index.ts index 1f3ecaa7c..1dc96434a 100644 --- a/packages/codev/src/commands/consult/index.ts +++ b/packages/codev/src/commands/consult/index.ts @@ -27,6 +27,7 @@ import { } from '../../lib/consult-lanes.js'; import type { ModelReasoningEffort } from '@openai/codex-sdk'; import { getResolver, GitRefResolver, type ArtifactResolver } from '../porch/artifacts.js'; +import { findVerdict } from '../porch/verdict.js'; import { MetricsDB } from './metrics.js'; import { extractUsage, extractReviewText, type SDKResultLike, type UsageData } from './usage-extractor.js'; import { executeForgeCommandSync } from '../../lib/forge.js'; @@ -1305,18 +1306,30 @@ export function resolveOpencodeBin(): string | null { * because a *catalog listing* broke would turn a diagnostic into an outage. */ export function listOpencodeModels(bin = 'opencode'): string[] { + let out: string; try { - const out = execFileSync(bin, ['models'], { + out = execFileSync(bin, ['models'], { encoding: 'utf-8', timeout: OPENCODE_MODELS_TIMEOUT_MS, stdio: ['ignore', 'pipe', 'pipe'], }); - return out.split('\n').map(l => l.trim()).filter(l => l.length > 0); } catch { return []; } + // Keep only `provider/model`-shaped lines. Today `opencode models` prints nothing else (probed + // 2026-08-21), but if it ever gains a header or an annotation, parsing those as ids would turn a + // *valid* model into "Unknown opencode model" — a hard failure caused by a cosmetic change in + // someone else's CLI. Filtering means a decorated listing degrades to "catalog unreadable", which + // hands authority back to the provider instead of blocking the lane. (Raised by claude at review.) + return out + .split('\n') + .map(l => l.trim()) + .filter(l => OPENCODE_MODEL_LINE_RE.test(l)); } +/** A bare `provider/model` line, the entire shape `opencode models` emits. */ +const OPENCODE_MODEL_LINE_RE = /^[A-Za-z0-9][A-Za-z0-9._-]*\/[A-Za-z0-9][A-Za-z0-9._-]*$/; + /** * Provenance banner prepended to an opencode review. * @@ -1353,6 +1366,7 @@ export async function runOpencodeConsultation( outputPath?: string, metricsCtx?: MetricsContext, modelChoice?: LaneModelChoice, + requireVerdict = false, ): Promise { const startTime = Date.now(); const choice = modelChoice @@ -1363,10 +1377,17 @@ export async function runOpencodeConsultation( // A missing CLI is a hard failure here, unlike the agy lane's skip. See the header: a lane that // silently produces nothing is counted as an approval (#20), and "not installed" is a // configuration mistake with an obvious fix, not a transient environment state. + // + // A stale CODEV_OPENCODE_BIN reaches this same branch, so it gets its own message: "install + // opencode" is the wrong instruction for someone whose override points at a path that moved. discardStaleOutput(outputPath); + const override = process.env.CODEV_OPENCODE_BIN; throw new Error( - 'opencode not found. Install it (https://opencode.ai), or drop "opencode" from ' + - 'porch.consultation in .codev/config.json.' + override + ? `CODEV_OPENCODE_BIN points at ${override}, which does not exist. ` + + `Correct it or unset it to fall back to opencode on PATH.` + : 'opencode not found. Install it (https://opencode.ai), or drop "opencode" from ' + + 'porch.consultation in .codev/config.json.' ); } @@ -1457,7 +1478,15 @@ export async function runOpencodeConsultation( // fail() owns settling on both error paths — it clears the timer and cleans up itself, so // this handler must NOT pre-settle or those paths would be swallowed by its own guard. if (code !== 0) { - fail(`opencode exited with code ${code}`, code ?? 1); + // `code === null` means a signal killed it (OOM, external kill). Still a hard failure — this + // lane has no skip path — but saying "exited with code null" sends the reader hunting for a + // provider error that was never printed. + fail( + code === null + ? 'opencode was killed by a signal before producing a review' + : `opencode exited with code ${code}`, + code ?? 1, + ); return; } // A zero exit with nothing on stdout is the #20 shape exactly: no review, but nothing that @@ -1466,6 +1495,24 @@ export async function runOpencodeConsultation( fail('opencode produced no review output', 0); return; } + // Same shape one step further in: a protocol-mode run that answered, but never stated a + // verdict. `parseVerdict` cannot distinguish that from a stated COMMENT, and `allApprove` + // counts COMMENT as an approval — so silence would become consent. + // + // This guard also covers what the header would otherwise break. `parseVerdict` treats output + // under 50 characters as REQUEST_CHANGES, a floor that exists to catch exactly this; the + // 76-character provenance banner lifts a two-word non-answer over it and converts a would-be + // REQUEST_CHANGES into an approval. Found by the opencode lane reviewing its own PR, which is + // a better argument for the lane than anything in the PR body. + if (requireVerdict && findVerdict(raw) === null) { + fail( + 'opencode produced a review with no VERDICT line. A review that states no verdict is ' + + 'not a verdict — porch would read it as a non-blocking COMMENT and count it as an ' + + 'approval.', + 0, + ); + return; + } settled = true; clearTimeout(timer); @@ -1581,7 +1628,11 @@ async function runConsultation( const startTime = Date.now(); const choice = resolveLaneModelChoice(workspaceRoot, 'opencode', DEFAULT_OPENCODE_MODEL, modelIdOverride); logResolvedModel(model, choice.id, choice.key); - await runOpencodeConsultation(query, role, workspaceRoot, outputPath, metricsCtx, choice); + // `generalMode` is an ad-hoc `--prompt`, where no verdict is expected or asked for. Protocol + // mode is a review, and a review owes a verdict. + await runOpencodeConsultation( + query, role, workspaceRoot, outputPath, metricsCtx, choice, !generalMode, + ); logQuery(workspaceRoot, model, query, (Date.now() - startTime) / 1000); return; } diff --git a/packages/codev/src/commands/porch/__tests__/parse-verdict.test.ts b/packages/codev/src/commands/porch/__tests__/parse-verdict.test.ts index a4f7eafb4..9badf17de 100644 --- a/packages/codev/src/commands/porch/__tests__/parse-verdict.test.ts +++ b/packages/codev/src/commands/porch/__tests__/parse-verdict.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { parseVerdict } from '../verdict'; +import { parseVerdict, findVerdict } from '../verdict'; describe('parseVerdict', () => { it('returns REQUEST_CHANGES for empty output', () => { @@ -121,3 +121,58 @@ But it does not contain any VERDICT: line because the reviewer went off-task or expect(parseVerdict(output)).toBe('COMMENT'); }); }); + +/** + * `findVerdict` — "did the reviewer state a verdict?", which `parseVerdict` cannot answer (#22). + * + * The distinction matters because of what sits downstream: `allApprove` counts COMMENT as an + * approval, and `parseVerdict` returns COMMENT both when a reviewer wrote `VERDICT: COMMENT` and + * when a reviewer wrote no verdict at all. Silence and consent are indistinguishable there. + * + * The 50-character floor was the only thing standing between those two cases, and it was a proxy — + * length standing in for "a review happened". Any lane that prefixes provenance, a banner, a model + * id or a timestamp clears the floor without reviewing anything. This function measures the thing + * itself instead. + */ +describe('findVerdict', () => { + it('returns null when the reviewer stated no verdict', () => { + expect(findVerdict('Some long review text with no verdict line anywhere in it at all.')).toBeNull(); + }); + + it('distinguishes a stated COMMENT from a missing verdict', () => { + const stated = 'Review text long enough to clear the floor.\n\nVERDICT: COMMENT'; + const missing = 'Review text long enough to clear the floor, but stating nothing.'; + // parseVerdict collapses these two into the same answer; that collapse is the bug. + expect(parseVerdict(stated)).toBe(parseVerdict(missing)); + expect(findVerdict(stated)).toBe('COMMENT'); + expect(findVerdict(missing)).toBeNull(); + }); + + it('ignores a template placeholder, as parseVerdict does', () => { + expect(findVerdict('VERDICT: [APPROVE | REQUEST_CHANGES | COMMENT]')).toBeNull(); + }); + + it('reads the LAST verdict, as parseVerdict does', () => { + expect(findVerdict('VERDICT: REQUEST_CHANGES\n\nlater...\n\nVERDICT: APPROVE')).toBe('APPROVE'); + }); + + it('strips markdown emphasis, as parseVerdict does', () => { + expect(findVerdict('**VERDICT: APPROVE**')).toBe('APPROVE'); + }); + + it('leaves parseVerdict behaviour unchanged for short output', () => { + // The refactor must not move the floor: parseVerdict still shortcuts before consulting this. + expect(findVerdict('ok')).toBeNull(); + expect(parseVerdict('ok')).toBe('REQUEST_CHANGES'); + }); + + it('shows why a provenance header is dangerous without a verdict check', () => { + // The concrete defect the opencode lane found in its own implementation: a 76-character banner + // lifts a two-word non-answer over the floor, and REQUEST_CHANGES silently becomes an approval. + const header = '_Reviewed by the opencode lane — model: `xai/grok-4.6` (shipped default)._\n\n'; + expect(parseVerdict('ok')).toBe('REQUEST_CHANGES'); + expect(parseVerdict(header + 'ok')).toBe('COMMENT'); + // findVerdict is indifferent to length, so it catches what the floor cannot. + expect(findVerdict(header + 'ok')).toBeNull(); + }); +}); diff --git a/packages/codev/src/commands/porch/verdict.ts b/packages/codev/src/commands/porch/verdict.ts index 8465638e4..54828371a 100644 --- a/packages/codev/src/commands/porch/verdict.ts +++ b/packages/codev/src/commands/porch/verdict.ts @@ -7,9 +7,9 @@ import type { Verdict, ReviewResult } from './types.js'; /** - * Parse verdict from consultation output. + * The verdict a review explicitly states, or `null` when it states none. * - * Looks for the verdict line in format: + * Recognises the verdict line in the format: * VERDICT: APPROVE * VERDICT: REQUEST_CHANGES * VERDICT: COMMENT @@ -18,15 +18,12 @@ import type { Verdict, ReviewResult } from './types.js'; * **VERDICT: APPROVE** * *VERDICT: APPROVE* * - * Safety: If no explicit verdict found (empty output, crash, malformed), - * defaults to REQUEST_CHANGES to prevent proceeding with unverified code. + * Split out of `parseVerdict` so a caller can tell "the reviewer said COMMENT" apart from "the + * reviewer said nothing and COMMENT is what we defaulted to". `parseVerdict` cannot express that + * difference — both come back as COMMENT, and `allApprove` counts COMMENT as an approval — so a + * lane that wants to refuse a verdict-less review has to ask this instead (#22). */ -export function parseVerdict(output: string): Verdict { - // Empty or very short output = something went wrong - if (!output || output.trim().length < 50) { - return 'REQUEST_CHANGES'; - } - +export function findVerdict(output: string): Verdict | null { // Scan lines LAST→FIRST so the actual verdict (at the end) takes priority // over template text echoed by codex CLI at the start of output. // Skip template lines containing "[" (e.g., "VERDICT: [APPROVE | REQUEST_CHANGES | COMMENT]") @@ -42,9 +39,23 @@ export function parseVerdict(output: string): Verdict { if (value.startsWith('COMMENT')) return 'COMMENT'; } } + return null; +} + +/** + * Parse verdict from consultation output. + * + * Safety: If no explicit verdict found (empty output, crash, malformed), + * defaults to REQUEST_CHANGES to prevent proceeding with unverified code. + */ +export function parseVerdict(output: string): Verdict { + // Empty or very short output = something went wrong + if (!output || output.trim().length < 50) { + return 'REQUEST_CHANGES'; + } // No valid VERDICT: line found but the consult ran — treat as COMMENT (non-blocking skip) - return 'COMMENT'; + return findVerdict(output) ?? 'COMMENT'; } /** From db493958be9c12f11b41854476c274133d132db3 Mon Sep 17 00:00:00 2001 From: pseudo Date: Fri, 21 Aug 2026 17:03:32 -0600 Subject: [PATCH 06/10] [AIR #22] docs: the 50-char floor is a proxy, and why not to raise it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the verdict requirement, the opencode lane's prerequisites and expected timing, and — the part worth writing down — why `parseVerdict`'s length floor was never the right guard. Someone reading only the fix would reasonably conclude the number needs raising. It does not; it needs replacing, which `findVerdict()` does. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 7 +-- CLAUDE.md | 7 +-- codev-skeleton/resources/commands/consult.md | 21 ++++++-- codev/resources/commands/consult.md | 21 ++++++-- codev/state/air-22_thread.md | 51 ++++++++++++++++++++ 5 files changed, 95 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b007e2621..464f8d3c4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,10 +140,11 @@ skips non-blockingly if unauthenticated), **GPT-5.6 Sol** (`gpt-5.6-sol` — the load-bearing) via the Codex SDK, and **Claude Opus 5** via the Agent SDK. Disable only when the user says "without consultation". -A fourth lane, **`opencode`** (`opencode run`, default `xai/grok-4.6` — the prefix is `xai/`, not -`x-ai/`), is available but not in the default rotation. Reach for it when a default lane is +An additional lane, **`opencode`** (`opencode run`, default `xai/grok-4.6` — the prefix is `xai/`, +not `x-ai/`), is available but not in the default rotation. Reach for it when a default lane is quota-exhausted or unauthenticated: it is the one reviewer on an account none of the others share. -Unlike the agy lane it never skips — every failure is loud. +Unlike the agy lane it never skips — every failure is loud, including a review that states no +verdict. Consult after writing implementation code and after writing tests, before presenting results. **"cmap"** means run all three in parallel *in the background* and return control immediately. diff --git a/CLAUDE.md b/CLAUDE.md index b007e2621..464f8d3c4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -140,10 +140,11 @@ skips non-blockingly if unauthenticated), **GPT-5.6 Sol** (`gpt-5.6-sol` — the load-bearing) via the Codex SDK, and **Claude Opus 5** via the Agent SDK. Disable only when the user says "without consultation". -A fourth lane, **`opencode`** (`opencode run`, default `xai/grok-4.6` — the prefix is `xai/`, not -`x-ai/`), is available but not in the default rotation. Reach for it when a default lane is +An additional lane, **`opencode`** (`opencode run`, default `xai/grok-4.6` — the prefix is `xai/`, +not `x-ai/`), is available but not in the default rotation. Reach for it when a default lane is quota-exhausted or unauthenticated: it is the one reviewer on an account none of the others share. -Unlike the agy lane it never skips — every failure is loud. +Unlike the agy lane it never skips — every failure is loud, including a review that states no +verdict. Consult after writing implementation code and after writing tests, before presenting results. **"cmap"** means run all three in parallel *in the background* and return control immediately. diff --git a/codev-skeleton/resources/commands/consult.md b/codev-skeleton/resources/commands/consult.md index 42a1e86cd..a2ca847cd 100644 --- a/codev-skeleton/resources/commands/consult.md +++ b/codev-skeleton/resources/commands/consult.md @@ -232,9 +232,19 @@ named, and **no review file is written** — so porch cannot advance on a lane t you do *not* get is a silent substitution of the default model. The `opencode` lane takes the strict side of this contract with no exceptions at all: a missing -CLI, an unknown id, a non-zero exit, and a clean exit that produced nothing all fail the lane and -leave no review file. It has no OAuth-fragility to accommodate, and a lane that quietly produces -nothing is a lane porch counts as an approval. +CLI, an unknown id, a non-zero exit, a clean exit that produced nothing, and — in protocol mode — a +review that states no `VERDICT:` line all fail the lane and leave no review file. It has no +OAuth-fragility to accommodate, and a lane that quietly produces nothing is a lane porch counts as +an approval. + +That last one deserves its own note, because the guard it replaces looks adequate and is not. +`parseVerdict` treats output under 50 characters as `REQUEST_CHANGES`; anything longer with no +verdict line falls through to `COMMENT`, which `allApprove` counts as an approval. The floor is a +**proxy** — length standing in for "a review actually happened" — and any lane that prefixes +provenance, a banner, a model id or a timestamp clears it without reviewing anything. Raising 50 to +100 postpones the problem to the next slightly longer header. Ask the real question instead: +`findVerdict()` returns the verdict a review *states*, or `null`, which is the distinction issue +\#20 also needs. Reuse it rather than writing a second one. One deliberate exception: a `gemini` lane **with no model id resolved** still skips non-blockingly when `agy` is missing or unauthenticated (consultation is best-effort there). Once an id *is* @@ -368,6 +378,7 @@ consult -m hermes --protocol spir --type spec | Gemini | ~120-180s | Antigravity CLI (`agy`); agentic file access via `--sandbox`, plain text output | | Codex | ~200-250s | Shell command exploration, read-only sandbox | | Claude | ~60-120s | Agent SDK with Read/Glob/Grep tools | +| opencode | ~40-120s | `opencode run` (Grok); agentic file access, plain text output. Add ~2-5s for the `opencode models` pre-flight | ## Prerequisites @@ -383,6 +394,10 @@ npm install -g @openai/codex # Gemini lane → Antigravity CLI (`agy`), replacing the retired Gemini CLI curl -fsSL https://antigravity.google/cli/install.sh | bash agy # run once and sign in (OAuth / Google subscription) + +# opencode lane (Grok, and whatever else your opencode account reaches) +npm install -g opencode-ai +opencode models # confirms auth and prints the ids this lane will accept ``` Configure auth: diff --git a/codev/resources/commands/consult.md b/codev/resources/commands/consult.md index 42a1e86cd..a2ca847cd 100644 --- a/codev/resources/commands/consult.md +++ b/codev/resources/commands/consult.md @@ -232,9 +232,19 @@ named, and **no review file is written** — so porch cannot advance on a lane t you do *not* get is a silent substitution of the default model. The `opencode` lane takes the strict side of this contract with no exceptions at all: a missing -CLI, an unknown id, a non-zero exit, and a clean exit that produced nothing all fail the lane and -leave no review file. It has no OAuth-fragility to accommodate, and a lane that quietly produces -nothing is a lane porch counts as an approval. +CLI, an unknown id, a non-zero exit, a clean exit that produced nothing, and — in protocol mode — a +review that states no `VERDICT:` line all fail the lane and leave no review file. It has no +OAuth-fragility to accommodate, and a lane that quietly produces nothing is a lane porch counts as +an approval. + +That last one deserves its own note, because the guard it replaces looks adequate and is not. +`parseVerdict` treats output under 50 characters as `REQUEST_CHANGES`; anything longer with no +verdict line falls through to `COMMENT`, which `allApprove` counts as an approval. The floor is a +**proxy** — length standing in for "a review actually happened" — and any lane that prefixes +provenance, a banner, a model id or a timestamp clears it without reviewing anything. Raising 50 to +100 postpones the problem to the next slightly longer header. Ask the real question instead: +`findVerdict()` returns the verdict a review *states*, or `null`, which is the distinction issue +\#20 also needs. Reuse it rather than writing a second one. One deliberate exception: a `gemini` lane **with no model id resolved** still skips non-blockingly when `agy` is missing or unauthenticated (consultation is best-effort there). Once an id *is* @@ -368,6 +378,7 @@ consult -m hermes --protocol spir --type spec | Gemini | ~120-180s | Antigravity CLI (`agy`); agentic file access via `--sandbox`, plain text output | | Codex | ~200-250s | Shell command exploration, read-only sandbox | | Claude | ~60-120s | Agent SDK with Read/Glob/Grep tools | +| opencode | ~40-120s | `opencode run` (Grok); agentic file access, plain text output. Add ~2-5s for the `opencode models` pre-flight | ## Prerequisites @@ -383,6 +394,10 @@ npm install -g @openai/codex # Gemini lane → Antigravity CLI (`agy`), replacing the retired Gemini CLI curl -fsSL https://antigravity.google/cli/install.sh | bash agy # run once and sign in (OAuth / Google subscription) + +# opencode lane (Grok, and whatever else your opencode account reaches) +npm install -g opencode-ai +opencode models # confirms auth and prints the ids this lane will accept ``` Configure auth: diff --git a/codev/state/air-22_thread.md b/codev/state/air-22_thread.md index e22b7f8bc..d9ab9e59f 100644 --- a/codev/state/air-22_thread.md +++ b/codev/state/air-22_thread.md @@ -75,3 +75,54 @@ Real `opencode`, real Grok, built CLI: `porch.consultation.models` still defaults to `["gemini", "codex", "claude"]`. Changing the default rotation is an architectural call the issue does not make, so opencode is available but opt-in. + +## The lane found a real bug in itself, on its first run + +CMAP on PR #24 came back: claude=APPROVE, opencode=COMMENT, codex=quota-blocked +(resets Aug 27), gemini=skipped (agy exited 1, which emits the non-blocking COMMENT +that `allApprove` counts as an approval — #20, live, in the review of the PR about #20). + +The opencode lane's finding, verified before acting on it: + + parseVerdict('ok') -> REQUEST_CHANGES + parseVerdict(header + 'ok') -> COMMENT // header is 76 chars + +`opencodeReviewHeader` — the provenance banner added to satisfy the issue's "record +which model the lane used" — lifts a short verdict-less review over `parseVerdict`'s +50-character floor. REQUEST_CHANGES becomes COMMENT, and COMMENT counts as approval. +The PR that claims to reduce #20's blast radius was opening a fresh #20 hole. + +**The deeper point, which the architect named and which belongs in the PR body:** the +50-char floor is a *proxy* — length standing in for "a review actually happened". It was +never measuring the right thing. It held by luck, and any lane prefixing provenance, a +banner, a model id or a timestamp defeats it. My header did not break a working guard; it +exposed one that was already wrong. Anyone "fixing" this by bumping 50 to 100 reintroduces +it with the next slightly longer header. + +Fix: extract `findVerdict()` from `parseVerdict()` — it returns the verdict a review +*states*, or `null`. That is the distinction `parseVerdict` structurally cannot express +(a stated COMMENT and no verdict at all return the same value), and it is the same +distinction #20 needs. Whoever picks up #20 should reuse it rather than write a second one. +The lane then hard-fails a protocol-mode review with no verdict — a reviewer that produced +output but no verdict has not reviewed. General mode (`--prompt`) is exempt: a question is +not a review. + +Worth stating plainly rather than hiding: a lane approving the PR that adds it is weak +evidence. A lane finding a real defect in its own implementation, in the exact failure +class the PR exists to reduce, is much stronger — and stronger than an APPROVE from it +would have been. + +Also fixed from claude's review: a stale `CODEV_OPENCODE_BIN` said "install opencode" +instead of naming the override; `listOpencodeModels` now keeps only `provider/model`-shaped +lines, so a future decorated listing degrades to "catalog unreadable" (provider stays the +authority) instead of turning a valid id into a hard failure; tests added for the +large-prompt temp-file branch; `persistent-output.test.ts` and `lane-models.test.ts` lane +loops extended. + +## Flaky tests + +`src/terminal/__tests__/session-manager.test.ts > auto-restart logic > respects maxRestarts +limit` timed out once in the full run (5533 passed / 1 failed), while I had two vitest +suites running concurrently. Passes standalone: 91/91 in 28s. Timing-sensitive under load, +unrelated to consult. Not skipped — a test that passes cleanly on its own should not be +annotated away. From 237df466c1826a15980244bf16bb16090c4571ec Mon Sep 17 00:00:00 2001 From: pseudo Date: Fri, 21 Aug 2026 17:06:38 -0600 Subject: [PATCH 07/10] [AIR #22] docs: name opencode in the intros that enumerate lanes Caught by the opencode lane's confirming re-review: the lane table and flag docs listed it, but consult.md's opening sentence and the skill's description and one-liner still enumerated the old set. A reader who never scrolls past the first paragraph would conclude the lane does not exist. Co-Authored-By: Claude Opus 5 --- .claude/skills/consult/SKILL.md | 4 ++-- .codex/skills/consult/SKILL.md | 4 ++-- codev-skeleton/.claude/skills/consult/SKILL.md | 4 ++-- codev-skeleton/.codex/skills/consult/SKILL.md | 4 ++-- codev-skeleton/resources/commands/consult.md | 2 +- codev/resources/commands/consult.md | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.claude/skills/consult/SKILL.md b/.claude/skills/consult/SKILL.md index da627dabf..281341045 100644 --- a/.claude/skills/consult/SKILL.md +++ b/.claude/skills/consult/SKILL.md @@ -1,11 +1,11 @@ --- name: consult -description: AI consultation CLI — query Gemini, Codex, or Claude for reviews and analysis. ALWAYS check this skill before running any `consult` command. Use when reviewing specs, plans, implementations, or PRs with external models, running parallel 3-way reviews (cmap), or checking consultation stats. The `-m` model flag is always required except for `consult stats`. +description: AI consultation CLI — query Gemini, Codex, Claude, or opencode/Grok for reviews and analysis. ALWAYS check this skill before running any `consult` command. Use when reviewing specs, plans, implementations, or PRs with external models, running parallel 3-way reviews (cmap), or checking consultation stats. The `-m` model flag is always required except for `consult stats`. --- # consult - AI Consultation CLI -Query external AI models for reviews and analysis. Supports Gemini, Codex, and Claude. +Query external AI models for reviews and analysis. Supports Gemini, Codex, Claude, hermes, and opencode. ## Synopsis diff --git a/.codex/skills/consult/SKILL.md b/.codex/skills/consult/SKILL.md index da627dabf..281341045 100644 --- a/.codex/skills/consult/SKILL.md +++ b/.codex/skills/consult/SKILL.md @@ -1,11 +1,11 @@ --- name: consult -description: AI consultation CLI — query Gemini, Codex, or Claude for reviews and analysis. ALWAYS check this skill before running any `consult` command. Use when reviewing specs, plans, implementations, or PRs with external models, running parallel 3-way reviews (cmap), or checking consultation stats. The `-m` model flag is always required except for `consult stats`. +description: AI consultation CLI — query Gemini, Codex, Claude, or opencode/Grok for reviews and analysis. ALWAYS check this skill before running any `consult` command. Use when reviewing specs, plans, implementations, or PRs with external models, running parallel 3-way reviews (cmap), or checking consultation stats. The `-m` model flag is always required except for `consult stats`. --- # consult - AI Consultation CLI -Query external AI models for reviews and analysis. Supports Gemini, Codex, and Claude. +Query external AI models for reviews and analysis. Supports Gemini, Codex, Claude, hermes, and opencode. ## Synopsis diff --git a/codev-skeleton/.claude/skills/consult/SKILL.md b/codev-skeleton/.claude/skills/consult/SKILL.md index da627dabf..281341045 100644 --- a/codev-skeleton/.claude/skills/consult/SKILL.md +++ b/codev-skeleton/.claude/skills/consult/SKILL.md @@ -1,11 +1,11 @@ --- name: consult -description: AI consultation CLI — query Gemini, Codex, or Claude for reviews and analysis. ALWAYS check this skill before running any `consult` command. Use when reviewing specs, plans, implementations, or PRs with external models, running parallel 3-way reviews (cmap), or checking consultation stats. The `-m` model flag is always required except for `consult stats`. +description: AI consultation CLI — query Gemini, Codex, Claude, or opencode/Grok for reviews and analysis. ALWAYS check this skill before running any `consult` command. Use when reviewing specs, plans, implementations, or PRs with external models, running parallel 3-way reviews (cmap), or checking consultation stats. The `-m` model flag is always required except for `consult stats`. --- # consult - AI Consultation CLI -Query external AI models for reviews and analysis. Supports Gemini, Codex, and Claude. +Query external AI models for reviews and analysis. Supports Gemini, Codex, Claude, hermes, and opencode. ## Synopsis diff --git a/codev-skeleton/.codex/skills/consult/SKILL.md b/codev-skeleton/.codex/skills/consult/SKILL.md index da627dabf..281341045 100644 --- a/codev-skeleton/.codex/skills/consult/SKILL.md +++ b/codev-skeleton/.codex/skills/consult/SKILL.md @@ -1,11 +1,11 @@ --- name: consult -description: AI consultation CLI — query Gemini, Codex, or Claude for reviews and analysis. ALWAYS check this skill before running any `consult` command. Use when reviewing specs, plans, implementations, or PRs with external models, running parallel 3-way reviews (cmap), or checking consultation stats. The `-m` model flag is always required except for `consult stats`. +description: AI consultation CLI — query Gemini, Codex, Claude, or opencode/Grok for reviews and analysis. ALWAYS check this skill before running any `consult` command. Use when reviewing specs, plans, implementations, or PRs with external models, running parallel 3-way reviews (cmap), or checking consultation stats. The `-m` model flag is always required except for `consult stats`. --- # consult - AI Consultation CLI -Query external AI models for reviews and analysis. Supports Gemini, Codex, and Claude. +Query external AI models for reviews and analysis. Supports Gemini, Codex, Claude, hermes, and opencode. ## Synopsis diff --git a/codev-skeleton/resources/commands/consult.md b/codev-skeleton/resources/commands/consult.md index a2ca847cd..80723e5bb 100644 --- a/codev-skeleton/resources/commands/consult.md +++ b/codev-skeleton/resources/commands/consult.md @@ -1,6 +1,6 @@ # consult - AI Consultation CLI -The `consult` command provides a unified interface for AI consultation with external models (Gemini, Codex, Claude, Hermes). It operates in three modes: general (ad-hoc prompts), protocol-based (structured reviews), and stats. +The `consult` command provides a unified interface for AI consultation with external models (Gemini, Codex, Claude, Hermes, opencode). It operates in three modes: general (ad-hoc prompts), protocol-based (structured reviews), and stats. ## Synopsis diff --git a/codev/resources/commands/consult.md b/codev/resources/commands/consult.md index a2ca847cd..80723e5bb 100644 --- a/codev/resources/commands/consult.md +++ b/codev/resources/commands/consult.md @@ -1,6 +1,6 @@ # consult - AI Consultation CLI -The `consult` command provides a unified interface for AI consultation with external models (Gemini, Codex, Claude, Hermes). It operates in three modes: general (ad-hoc prompts), protocol-based (structured reviews), and stats. +The `consult` command provides a unified interface for AI consultation with external models (Gemini, Codex, Claude, Hermes, opencode). It operates in three modes: general (ad-hoc prompts), protocol-based (structured reviews), and stats. ## Synopsis From 8f81353e76d795e680d0fbf9ef845895ce5e3b31 Mon Sep 17 00:00:00 2001 From: pseudo Date: Fri, 21 Aug 2026 17:06:58 -0600 Subject: [PATCH 08/10] chore(porch): 22 record PR #24 --- .../projects/22-add-opencode-as-a-consult-lane/status.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml b/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml index 741d8776c..cb66096dc 100644 --- a/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml +++ b/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml @@ -11,4 +11,9 @@ iteration: 1 build_complete: false history: [] started_at: '2026-08-21T22:35:07.319Z' -updated_at: '2026-08-21T22:50:49.055Z' +updated_at: '2026-08-21T23:06:58.920Z' +pr_history: + - phase: pr + pr_number: 24 + branch: builder/air-22 + created_at: '2026-08-21T23:06:58.918Z' From 171f3346323f0aa4814b73ccd8afbac5bff48db2 Mon Sep 17 00:00:00 2001 From: pseudo Date: Fri, 21 Aug 2026 17:07:07 -0600 Subject: [PATCH 09/10] chore(porch): 22 pr gate-requested --- codev/projects/22-add-opencode-as-a-consult-lane/status.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml b/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml index cb66096dc..98f71bc17 100644 --- a/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml +++ b/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml @@ -7,13 +7,15 @@ current_plan_phase: null gates: pr: status: pending + requested_at: '2026-08-21T23:07:07.852Z' iteration: 1 build_complete: false history: [] started_at: '2026-08-21T22:35:07.319Z' -updated_at: '2026-08-21T23:06:58.920Z' +updated_at: '2026-08-21T23:07:07.852Z' pr_history: - phase: pr pr_number: 24 branch: builder/air-22 created_at: '2026-08-21T23:06:58.918Z' +pr_ready_for_human: true From 4d5c92c9e4ab3370934d697fc52177171ddb4839 Mon Sep 17 00:00:00 2001 From: pseudo Date: Fri, 21 Aug 2026 17:07:42 -0600 Subject: [PATCH 10/10] chore(porch): 22 pr gate-approved --- .../projects/22-add-opencode-as-a-consult-lane/status.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml b/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml index 98f71bc17..dd4661071 100644 --- a/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml +++ b/codev/projects/22-add-opencode-as-a-consult-lane/status.yaml @@ -6,16 +6,17 @@ plan_phases: [] current_plan_phase: null gates: pr: - status: pending + status: approved requested_at: '2026-08-21T23:07:07.852Z' + approved_at: '2026-08-21T23:07:42.712Z' iteration: 1 build_complete: false history: [] started_at: '2026-08-21T22:35:07.319Z' -updated_at: '2026-08-21T23:07:07.852Z' +updated_at: '2026-08-21T23:07:42.713Z' pr_history: - phase: pr pr_number: 24 branch: builder/air-22 created_at: '2026-08-21T23:06:58.918Z' -pr_ready_for_human: true +pr_ready_for_human: false