fix(sdk): default Claude steps to Opus 5

[miyaontherelay]

A dollar-budgeted authored step such as f.agent("implementer", { cli: "claude", task }) was refused when model was omitted, even though Claude is a recognized adapter with a stable priced model available.

This gives the Claude adapter an explicit claude-opus-5 default and resolves the model once in step → named agent → adapter priority order. The same resolved model now drives project allowlist enforcement, frozen-dollar budget preflight, readiness probing, checked kernel input, provider invocation, and usage accounting. Explicit models still win. Codex and unknown/custom CLIs retain the existing fail-closed behavior when no model is declared.

The frozen standard Opus 5 rate is $5/M input and $25/M output, matching Anthropic's model documentation: https://platform.claude.com/docs/en/models/opus-5/whats-new-opus-5

The revision also addresses the first review swarm's findings: unresolved CLIs now stop before model, budget, or environment probes; model provenance is centralized; the adapter boundary and priority are documented; and regression coverage includes registered-no-default, custom, unpriced-default, token-budget, named-agent precedence, allowlist, exact argv, and daemon-backed authored execution paths. The schema parity fixture keeps its additional model authority local instead of changing shared testdata.

Validation:

• npm run typecheck
• npm run typecheck:tests
• npm run build
• 205 focused SDK tests across adapter, preflight, pricing, worker invocation, PTY, and authored execution paths
• schema generator byte-stable across two runs
• schema package: 77/77 tests
• installed Claude Code 2.1.271 exact claude-opus-5 readiness round trip: exit 0, expected token returned

The previous broad SDK run had 1,627 passing tests; its remaining local failures were existing tests that hard-code kernel/target/* while ops/cargo.sh deliberately builds outside the worktree. The hosted Linux artifact workflow supplies RELAYFLOWD_BIN and is the authoritative full-suite gate.

---

Note

Medium Risk
Changes model selection, budget preflight, and worker CLI invocation paths; mis-resolution could affect provider calls and dollar accounting, though explicit step/named models still win.

Overview
Introduces per-CLI adapter default models and a single resolution order (step → named agent → adapter default) used by flows check, preflight, and workers. The Claude adapter now declares claude-opus-5 as its default, with frozen pricing added so dollar budgets can accept Claude steps that omit an explicit model.

Preflight records modelSource on CLI resolutions, runs CLI resolution refusals before model allowlist/budget checks, and treats adapter-chosen models like explicit ones for probing, registry enforcement, and frozen-budget pricing (Codex/custom CLIs still fail closed with no default). flows check materializes adapter defaults onto the checked step spec; LlmWorker, AgentWorker, and runAgentCli call resolveCliModel so execution and spend accounting match preflight.

Docs and JSON Schema copy are updated to describe independent CLI vs model priority and the new default behavior.

^{Reviewed by Cursor Bugbot for commit c77be19. Bugbot is set up for automated code reviews on this repo. Configure here.}

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: f06ebb2f-63a2-4ef4-ace8-ac66ab8386e9

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Review swarm: maintainability

Maintainability Review: PR #418 — Default Claude steps to Opus 5

Reviewer: maintainability-418
Date: 2026-09-15 17:35
PR: #418
Branch: fix/claude-default-model-0915
Commit: c77be19

Lens: Could a stranger read this in six months and change it safely?

This review examines PR #418 through the maintainability lens only. The question: if someone unfamiliar with this code needs to modify the model resolution logic in six months, will the code guide them correctly?

---

Summary

PASS with critical observations.

The implementation introduces a model resolution priority system (step → named agent → adapter default) with budget enforcement. The code is maintainable overall but has one critical implicit contract and one missing failure boundary that would mislead future maintainers.

---

Critical Findings

C1: Implicit sequencing contract between CLI resolution and model resolution

Location: packages/sdk/src/preflight.ts:263-290

Issue: The code enforces a strict sequencing dependency that is not documented or enforced by types:

// L263-276: CLI resolution accumulates diagnostics into cliResolutionDiagnostics
for (const step of compiled.steps) {
  const resolution = resolveCli(step, compiled, options.projectCli);
  if (resolution === undefined) {
    cliResolutionDiagnostics.push({ ... });
  } else {
    resolutions.push(resolution);
    resolutionByStep.set(step.id, resolution);
  }
}

// L277-290: Return early if CLI resolution failed, BEFORE model/budget checks
diagnostics.push(...cliResolutionDiagnostics);
if (cliResolutionDiagnostics.length > 0) {
  return { ok: false, gates: ..., resolutions, diagnostics };
}
// Only now run model and budget diagnostics
diagnostics.push(...unknownModelDiagnostics(compiled, options, resolutionByStep));
diagnostics.push(...budgetDiagnostics(compiled, new Map(...)));

Why this is a maintainability hazard:

1. The contract is implicit: Nothing in the type system or documentation explains that unknownModelDiagnostics and budgetDiagnostics assume complete CLI resolution has succeeded. A future maintainer might reorder these calls or remove the early return.

2. Silent corruption if violated: If the early return is removed, budgetDiagnostics would receive an incomplete resolutionByStep map. It would fall back to checking step.model only, silently skipping the adapter default check. This would pass steps that should fail.

3. The test at preflight.test.ts:145-174 validates this but doesn't name it: The test "runs no probes when any step is statically unresolved" verifies the sequencing but describes it as avoiding wasteful probe work, not as a correctness requirement for the resolution map.

What future maintainers need to know:

• CLI resolution MUST complete fully before model/budget checks run
• This is not just optimization — the model checks depend on resolutionByStep being complete
• A partial resolution map would cause model/budget validation to skip adapter defaults

Recommendation: Add a comment at the early return explaining the dependency:

// Early return is required: unknownModelDiagnostics and budgetDiagnostics
// both depend on resolutionByStep being complete. A partial map would cause
// model checks to incorrectly fall back to step.model and skip adapter defaults.
if (cliResolutionDiagnostics.length > 0) {
  return { ok: false, gates: ..., resolutions, diagnostics };
}

---

C2: Missing failure path for adapter default with frozen budget but no price

Location: packages/sdk/src/budget-preflight.ts:119-124

Issue: The code refuses an unpriced adapter default under a frozen dollar budget:

else if (resolved?.source === 'adapter'
  && !Object.hasOwn(MODEL_PRICING, model)) diagnostics.push({
  severity: 'refusal', kind: 'budget_missing_price', stepId: step.id, model,
  message: `Model "${model}" has no frozen price for budget accounting.`,
});

The hidden assumption: This path assumes the adapter's defaultModel is defined. But HeadlessAdapter.defaultModel is optional (readonly defaultModel?: string). The codex and wrapper adapters have no default.

What happens when the assumption breaks:

1. A step with cli: 'codex', no explicit model, and budget: '$20/run' hits the existing check at L115-118: refuses with "Step needs a declared, priced model"
2. But if someone adds defaultModel: 'future-unpriced-model' to codex's adapter, that same step would bypass L115-118 (because resolved.model is now defined), reach L119, and refuse with the "has no frozen price" message
3. If the pricing table is then updated but MODEL_PRICING is loaded from a stale cache or a different version, the step would pass budget checks but fail at runtime when the worker tries to compute spend with an unpriced model

Why this is maintainable but fragile:

• The test at budget-preflight.test.ts:509-517 validates this path exists
• But the test constructs the resolved map manually — it never exercises the path through a real adapter default
• The failure mode (unpriced adapter default) is explicitly tested, but the boundary condition (adapter default defined but not in pricing table) relies on deployment synchronization between the adapter registry and MODEL_PRICING

What makes this safe today:

• Claude is the only adapter with a default, and claude-opus-5 is in MODEL_PRICING
• The test explicitly validates the refusal path

What would make this safer for six months from now:

• Nothing required for correctness, but the comment at L97-98 could be more explicit:

  /** Frozen dollar budgets require an exact model with a frozen table price.
   *  Adapter defaults are checked at L119 only when defined; codex/wrapper have no default. */

---

Design Observations

D1: Model resolution priority is clear but has one naming confusion

Location: packages/sdk/src/cli-adapter.ts:135-155

What's good:

• resolveCliModelSelection has one job: resolve the model priority chain
• Returns both the model and its source, so callers know provenance
• Priority order is explicit in the code and matches SURFACE.md

Naming confusion:

export function resolveCliModel(executable: string, model?: string): string | undefined {
  return resolveCliModelSelection(executable, { step: model }).model;
}

The parameter name model looks like "any model" but is specifically step-level in the function's contract ("Resolve a runtime model, where any materialized value is step-owned"). A future maintainer might pass a named agent's model here and get wrong priority.

Fix: Rename the parameter to match its meaning:

export function resolveCliModel(executable: string, stepModel?: string): string | undefined {
  return resolveCliModelSelection(executable, { step: stepModel }).model;
}

This is a minor issue — the function is only called from controlled sites (llm-worker.ts:196, worker.ts:442, worker-cli.ts:387) — but the generic name invites misuse in future call sites.

---

D2: The adapter registry is global mutable state with no guard against partial initialization

Location: packages/sdk/src/adapters/base.ts:23-33, called via registeredAdapters() in cli-adapter.ts

Current reality:

// Implicit contract: registeredAdapters() is called AFTER all adapters are registered
const model = registeredAdapters()[resolveAdapterKind(executable)].defaultModel;

The contract that isn't written:

• registeredAdapters() is a getter over a module-level Map<CliAdapterKind, HeadlessAdapter>
• Adapters register themselves via side effects during module import
• The code assumes claudeAdapter is already registered when resolveCliModelSelection runs

What happens if the assumption breaks:

• If resolveCliModelSelection is called before claude.ts imports, registeredAdapters()['claude'] is undefined, and the code throws Cannot read property 'defaultModel' of undefined
• This can't happen in normal execution (module graph guarantees order) but could happen in partial test setups or if someone imports cli-adapter.ts before adapters/index.ts

Why this is maintainable:

• It's a standard module registration pattern
• Tests import the full SDK, so they exercise the real initialization order
• The failure mode is loud (immediate throw) rather than silent corruption

What would make this more obvious to maintainers:

• A comment at resolveCliModelSelection noting the dependency:

  // Assumes adapters are registered (via their module imports) before resolution.
  const model = registeredAdapters()[resolveAdapterKind(executable)].defaultModel;

---

D3: Budget check sequencing is clear

Location: packages/sdk/src/budget-preflight.ts:98-126

The budget check runs in the right order:

1. Check step model or selected named agent model (L108-118)
2. Then check adapter default (L119-124)

The logic matches the resolution priority (step → named → adapter) and the early returns prevent double-checking. This is readable.

One clarity point: The variable name resolved shadows the meaning. It's not "the step's resolved model" — it's the ResolvedCliModel struct from preflight. The name works but could be more specific if touched in the future:

const resolution = resolvedModels.get(step.id);
const model = resolution?.model ?? step.model ?? ...

---

Test Coverage

What the tests verify:

1. Adapter default resolution: budget-preflight.test.ts:490-502 verifies Claude steps without explicit model resolve to claude-opus-5
2. Adapter default probing: L493-502 verify the probe runs with the default
3. Named agent priority: L529-543 verify named agent model wins over adapter default
4. Unpriced adapter default refusal: L509-517 verify a step refuses when the adapter default has no frozen price
5. Allowlist enforcement on defaults: L544-554 verify adapter defaults are checked against the model registry
6. Runtime use of adapter default: flow-executor-chain.test.ts:602-638, worker-cli.test.ts:820-840 verify the resolved default actually reaches the CLI invocation

What the tests do NOT verify:

1. The critical sequencing dependency: No test explicitly validates that budget/model checks depend on complete CLI resolution (C1). The test at preflight.test.ts:145-174 validates the effect (no probes run) but not the resolution map completeness.
2. Adapter default synchronization: No test validates the failure mode where an adapter defines a default but MODEL_PRICING is out of sync. The test at L509-517 constructs the map manually instead of going through a real adapter.

These gaps are not defects — the uncovered paths are integration-level contract violations rather than code paths — but they mean the implicit contracts (C1, C2) rely on code review and discipline rather than automated verification.

---

Comments that Assert What the Code Does Not Do

None found.

All comments describe what happens or why, not what doesn't happen. The doc comments accurately reflect behavior.

---

Tests That Would Not Fail If Behavior Broke

T1: Adapter default priority is tested but not pinned against CLI output

Location: budget-preflight.test.ts:490-502

The test verifies preflight resolves the adapter default and probes it:

expect(result.resolutions).toContainEqual(expect.objectContaining({
  stepId: 'ask', cli: 'claude', model: 'claude-opus-5', modelSource: 'adapter',
}));
expect(o.probes.cli).toHaveBeenCalledWith('claude', 'step', 'claude-opus-5');

What it doesn't verify: That the worker actually passes --model claude-opus-5 to the CLI. The probes are mocked, so the test never confirms the flag makes it to the real invocation.

But flow-executor-chain.test.ts:602-638 and worker-cli.test.ts:820-840 do verify this — they capture the real argv and assert on --model claude-opus-5. So the gap is covered, just not in the same test file. This is fine for maintainability.

---

T2: Budget refusal for unpriced adapter default is validated but assumes pricing table is in sync

Location: budget-preflight.test.ts:509-517

The test manually constructs a resolution with source: 'adapter' and an unpriced model, then asserts the refusal:

expect(budgetDiagnostics({...input, steps:[step]}, new Map([
  ['ask', { model: 'future-default', source: 'adapter' as const }],
]))).toEqual([expect.objectContaining({
  kind: 'budget_missing_price', model: 'future-default',
})]);

What it doesn't verify: That if claudeAdapter.defaultModel = 'future-default' and MODEL_PRICING lacks that entry, preflight would refuse. The test bypasses adapter resolution entirely.

Why this is acceptable: The test is explicitly exercising the budget check logic, not the adapter registration logic. The integration path (adapter default → preflight → budget check) is validated by the "runs a dollar-budgeted authored Claude agent" test at flow-executor-chain.test.ts:602-638, which uses a real flow config and would fail if the default were unpriced.

So again, the gap is covered at integration level. The unit test's manual construction is appropriate for isolating the budget check logic.

---

Verdict

PASS — The code is maintainable with named caveats.

What makes it maintainable:

1. Model resolution priority is clear, single-purpose, and matches the spec
2. Budget checks run in the right order with appropriate early returns
3. Test coverage validates the full integration path (adapter default → preflight → worker invocation)
4. Failure modes are loud (refusals, not silent fallbacks)

What requires care when changing:

1. C1 (critical): The sequencing dependency between CLI resolution and model/budget checks is not documented. Removing the early return at preflight.ts:277 would corrupt the resolution map and cause silent validation skips.
2. C2 (observable): The adapter default pricing check assumes defaultModel is defined when it reaches that branch. Adding an unpriced default to any adapter would shift the failure message but not break correctness.
3. D1 (minor): The parameter name model in resolveCliModel could mislead future callers into passing non-step models.

Recommended additions for long-term maintainability:

• Comment at preflight.ts:277 explaining why the early return is required for correctness
• Comment at cli-adapter.ts:153 noting the adapter registration dependency
• Consider renaming model → stepModel in resolveCliModel if touched in the future

None of these are blocking defects. The code works correctly and the implicit contracts are validated by tests, even if not explicitly documented. A maintainer who reads the code carefully will understand it. A maintainer in a hurry might remove the early return and create a silent bug.

---

Specific Maintainability Questions

Could a stranger locate the model resolution logic?
Yes. resolveCliModelSelection is named clearly and used consistently.

Could they understand the priority order?
Yes. The priority is explicit in the code and matches SURFACE.md.

Could they safely add a new adapter with a default model?
Yes, with one requirement: they must add the model to MODEL_PRICING if any flow might use frozen dollar budgets. The budget-preflight.test.ts:509-517 test documents this requirement.

Could they safely change the priority order?
No, without understanding the sequencing dependency in C1. They would need to trace through preflight to understand that budget checks depend on complete CLI resolution.

Would they know to update MODEL_PRICING when adding a default?
Probably not from the code alone. The comment at budget-preflight.ts:97 says "frozen dollar budgets require an exact model with a frozen table price" but doesn't explicitly say "adapter defaults must be in this table." The test at L509-517 implies it, but a maintainer might not read tests before adding an adapter.

Could they debug a "model has no frozen price" error?
Yes. The error message names the model and the failure is loud.

---

Conclusion

The PR is maintainable. The design is sound, the priority logic is clear, and the tests validate correctness. The implicit contracts (C1, C2) are the main hazard: a maintainer could violate them without realizing, and the code would compile and pass partial tests.

The fix is documentation, not code changes. Adding three comments (at the early return, at adapter registration, and at budget check) would make the implicit contracts explicit and prevent future corruption.

REVIEW_PASSED

[github-actions]

Review swarm: history

No fresh transcript was produced for run 8dfe91e9-f9b8-4284-bffa-6901f6efa644 (MISSING).

[github-actions]

Review swarm: structure

No fresh transcript was produced for run 8dfe91e9-f9b8-4284-bffa-6901f6efa644 (MISSING).

[github-actions]

Review swarm: FAILED

• maintainability: PASSED
• history: MISSING
• structure: MISSING

Cloud run: 8dfe91e9-f9b8-4284-bffa-6901f6efa644