[#1288] consult: ship claude-opus-5 and gpt-5.6-sol as default lane models - #1301
Merged
Conversation
…odels Move the shipped consult lane defaults forward: - claude lane: claude-opus-4-6 -> claude-opus-5 - codex lane: gpt-5.4 -> gpt-5.6-sol (reasoning effort unchanged at medium) - gemini lane: unchanged (agy's own default, no pinned id) Both ids were live-probed before adoption. The -sol suffix is load-bearing: plain gpt-5.6 and gpt-5.6-codex are rejected by Codex on a ChatGPT account. The ids are now exported constants (DEFAULT_CLAUDE_MODEL, DEFAULT_CODEX_MODEL, DEFAULT_CODEX_REASONING_EFFORT) rather than inline literals, so the new default-models test can pin them and so #1286 can drop config-driven resolution in front of them without re-extracting. CODEX_PRICING becomes a per-model rate table and cost computation moves to an exported computeCodexCost(). gpt-5.6-sol rates ($5.00 uncached / $0.50 cached / $30.00 output per 1M) are taken from developers.openai.com/api/docs/pricing. An unknown model id now yields costUsd: null instead of being billed at some other model's rates. codex-sdk.test.ts had its own copy of CODEX_PRICING, so its cost suite would have kept passing against stale rates forever; it now imports the real function. Docs: consult.md gains a "Shipped default model id" column and a cost-reporting note (mirrored to the skeleton); the Multi-Agent Consultation section of CLAUDE.md/AGENTS.md is corrected and now names both pinned lanes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1288.
Moves the shipped consult lane defaults forward — the ids every adopter gets when they haven't configured anything.
claude-opus-4-6claude-opus-5gpt-5.4@ mediumgpt-5.6-sol@ mediumReview
What changed and why
The ids are now named constants, not inline literals.
DEFAULT_CLAUDE_MODEL,DEFAULT_CODEX_MODEL, andDEFAULT_CODEX_REASONING_EFFORTare exported fromconsult/index.ts. Two reasons: the guard test (issue item 4) needs something CI-safe to assert against, and #1286 turns these exact values into config fallback defaults — having them already extracted makes that a one-line change rather than a re-extraction of the same lines this PR just touched.Pricing is now per-model, and unknown models cost
null.CODEX_PRICINGwas a bare rate triple hardcoded to gpt-5.4's numbers; it's now aRecord<modelId, rates>behind an exportedcomputeCodexCost(model, input, cached, output): number | null.gpt-5.6-sol's rates were verified against OpenAI's own pricing page (developers.openai.com/api/docs/pricing, fetched 2026-07-30), not an aggregator: $5.00 uncached input / $0.50 cached input / $30.00 output per 1M. So the issue's "record
cost_usdas null if rates can't be verified" fallback didn't need to trigger — but the mechanism is in place anyway, which matters once #1286 lets a workspace point the lane at an arbitrary model. Without it, a configured model would silently inherit gpt-5.6-sol's rates. Now it getsnull.Known gap, documented in code and docs: OpenAI publishes a separate higher long-context tier ($10/$1/$45). We model only the standard tier, so a consultation large enough to cross into it under-reports. Modelling the tier boundary is more than this issue asked for; flagging it rather than silently rounding.
A test was lying.
codex-sdk.test.tshad its own copy of theCODEX_PRICINGconstant and a local reimplementation of the cost formula. Its five "cost computation" assertions would have kept passing against gpt-5.4 rates indefinitely while the shipped rates drifted — it was testing a copy of the thing, not the thing. It now imports the realcomputeCodexCost, and the expected values were recomputed for the new rates.Tests
New
consult/__tests__/default-models.test.tspins both ids plus the reasoning effort, and covers thenull-for-unknown-model path (usinggpt-5.4as one of the unknowns, which doubles as an assertion that the old model's rates really are gone). CI-safe: no network, no model CLIs, no API calls.The specific thing this guards against is a future drive-by "correction" of the
-solsuffix. Both plaingpt-5.6andgpt-5.6-codexwere live-probed on 2026-07-29 and rejected —The '<id>' model is not supported when using Codex with a ChatGPT account.The suffix looks like a typo and isn't one, so the test comment says so out loud.Existing assertions updated:
consult.test.ts(claude lane model),codex-sdk.test.ts(codex lane model, plus a new assertion on reasoning effort that was previously unchecked).Docs
codev/resources/commands/consult.md— Models table gains a Shipped default model id column, plus a-solwarning callout and a "Cost reporting" section explaining thenullsemantics. Mirrored byte-identically tocodev-skeleton/.CLAUDE.md/AGENTS.md— the Multi-Agent Consultation section named "GPT-5.4 Codex (gpt-5.4-codex)", an id the code never actually used. Corrected togpt-5.6-sol, and the claude lane — which the section had simply omitted — is now listed. Both files remain byte-identical (verified withdiff).Coordination with #1286
No PR open for #1286 as of 2026-07-30, so this lands first. Per the issue, #1286's "default preservation" test vectors must then assert the new ids —
default-models.test.tsis the natural place for that to be enforced, since it will fail loudly if the fallback path resolves to anything else.Verification
pnpm buildfrom repo root (core → codev → dashboard → skeleton copy): green.porch check 1288: build ✓ (4.6s), tests ✓ (27.6s).No flaky tests encountered; the 48 skips are pre-existing.
Out of scope (unchanged, per the issue)
Lane composition, the gemini/agy lane's model, and the #1286 config mechanism itself.
Tower Integration Testsfailed on both runs of this branch. I diagnosed it rather than re-rolling the dice; it is a pre-existing teardown defect in a file this PR never touches.Problem.
src/agent-farm/__tests__/send-integration.e2e.test.tsfails withError: Hook timed out in 10000ms. All 5 of its tests pass — the failure is in teardown, after the assertions are green.Root cause. The suite's
afterAllis declared with an explicit10_000ms budget (send-integration.e2e.test.ts:273), overriding the config's generoushookTimeout: 300000. That teardown does twofetch(.../deactivate)calls with no timeout of their own (.catch(() => {})swallows errors but never bounds a hang), then astopServerthat waits up to 2s for SIGTERM before SIGKILL, then fourrmSynccalls. On a loaded ubuntu runner that comfortably exceeds 10s; on a developer machine it doesn't. ThebeforeAllin the same suite is budgeted at120_000— the asymmetry looks unintentional.Evidence it's unrelated to this PR:
vitest run --config vitest.e2e.config.ts src/agent-farm/__tests__/send-integration.e2e.test.ts).commands/consult/**and docs. It touches no Tower, websocket, or messaging code.1546ced5is literally "Fix flaky send-integration e2e test — retry activation during Tower startup".mainis green only because the margin is thin, not because the teardown is safely budgeted.Fix. I did not apply one, and I did not skip the suite — skipping would drop 5 real integration tests to land a two-line model-id change, and either edit is outside #1288's scope. The minimal fix is to raise that
afterAllbudget (and ideally bound the two deactivatefetches). Happy to do it in this PR if you'd prefer, or to file it separately — your call at the gate.