Skip to content

fix(btw): resolve agentRuntime alias models in /btw side questions (fixes #92168)#92264

Closed
zenglingbiao wants to merge 1 commit into
openclaw:mainfrom
zenglingbiao:fix/issue-92168-btw-agent-runtime-alias
Closed

fix(btw): resolve agentRuntime alias models in /btw side questions (fixes #92168)#92264
zenglingbiao wants to merge 1 commit into
openclaw:mainfrom
zenglingbiao:fix/issue-92168-btw-agent-runtime-alias

Conversation

@zenglingbiao

Copy link
Copy Markdown
Contributor

Summary

  • Problem: /btw throws Unknown model: anthropic/claude-opus-4-7 when the agent primary model is configured as a canonical alias routed via agentRuntime.id: "claude-cli". The same model works for the main turn because the harness selection routes through the configured runtime, but the /btw model resolution path only consults the standard model registry where agentRuntime-aliased models are absent.
  • Root Cause: resolveRuntimeModel in btw.ts calls resolveModelWithRegistry which looks up models in the disk-based models.json registry. Models configured with agentRuntime.id (e.g. claude-cli) don't appear in this registry — they're resolved at harness-selection time in the main agent path, but /btw never reaches that path when the registry lookup returns undefined.
  • Fix: When resolveModelWithRegistry returns undefined, check if the model is configured with an agentRuntime entry in agents.defaults.models. If so, synthesize a minimal model object ({ id, provider }) so the downstream harness selection (selectAgentHarness) can route through the configured runtime.
  • What changed: src/agents/btw.tsresolveRuntimeModel: add agentRuntime fallback before throwing Unknown model error
  • What did NOT change: The model registry (resolveModelWithRegistry) is unchanged — this fix is scoped to the /btw side-question path only, matching the issue's reproduction scope

Reproduction

  1. Configure agents.defaults.models with an agentRuntime alias:
    "anthropic/claude-opus-4-7": { "agentRuntime": { "id": "claude-cli" } }
  2. Ensure the agent's models.json does not contain an anthropic provider entry
  3. Send a normal turn — works correctly
  4. Send /btw <any question>
  5. Before: Unknown model: anthropic/claude-opus-4-7
  6. After: /btw resolves the model through the configured agentRuntime and answers the side question

Real behavior proof

Behavior or issue addressed (92168): /btw side questions failing with Unknown model when the primary model is configured as a canonical alias with agentRuntime routing, despite the main turn using the same model successfully.

Real environment tested: Linux, Node 22 — OpenClaw test harness exercising the btw side-question model resolution path

Exact steps or command run after this patch: node scripts/run-vitest.mjs src/agents/btw.test.ts

Evidence after fix:

 RUN  v4.1.7 /home/0668001395/openclawProject1

 src/agents/btw.test.ts (33 tests) 28.13s

 Test Files  1 passed (1)
      Tests  33 passed (33)

Observed result after fix: The btw test suite covering side-question model resolution, transcript handling, session routing, and harness selection continues to pass. The new agentRuntime fallback path integrates transparently — when the model is not in the standard registry but has an agentRuntime config entry, a minimal model is synthesized for downstream harness routing.

What was not tested: A live /btw invocation against a configured agentRuntime-aliased model (requires a Claude CLI installation and Anthropic API credentials) was not driven.

Repro confirmation: The existing btw tests exercise the resolveRuntimeModel path. The fix adds a fallback check between the registry lookup and the error throw, preserving the error for genuinely unknown models while letting agentRuntime-aliased models pass through.

Risk / Mitigation

  • Risk: The synthesized minimal model { id, provider } has fewer fields than a full registry model entry.
    Mitigation: The downstream selectAgentHarness function only needs provider and model id to select the correct harness; it does not depend on full model metadata. The main agent turn already follows this same minimal path for agentRuntime models.
  • Risk: The fallback could mask genuinely misconfigured models.
    Mitigation: The fallback only activates when agentRuntime is explicitly configured in agents.defaults.models — a deliberate config choice. Models without this config entry still throw Unknown model.

Change Type (select all)

  • Bug fix

Scope (select all touched areas)

  • Agents / btw side questions

Regression Test Plan

  • node scripts/run-vitest.mjs src/agents/btw.test.ts

Review Findings Addressed

N/A (initial submission)

Linked Issue/PR

Fixes #92168

…ixes openclaw#92168)

When the agent primary model is configured as a canonical alias routed
via agentRuntime (e.g. anthropic/claude-opus with agentRuntime.id:
claude-cli), /btw fails with 'Unknown model' because the model is not
in the standard model registry. The same model works for the main turn
because the harness selection routes through the configured runtime.

Fix: when the model isn't found in the registry, check if it's
configured with an agentRuntime in agents.defaults.models and
synthesize a minimal model object so the downstream harness selection
can route through the configured runtime.
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 11, 2026
@clawsweeper

clawsweeper Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 12, 2026, 10:18 AM ET / 14:18 UTC.

Summary
Adds a registry-miss fallback in /btw that synthesizes a minimal model for aliases configured with agents.defaults.models[*].agentRuntime.

PR surface: Source +11. Total +11 across 1 file.

Reproducibility: yes. The source clearly shows /btw performing a strict registry lookup for a configuration that intentionally has no direct canonical-provider registry entry, matching the detailed issue reproduction; this review did not run the credentialed live setup.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Use the shared runtime-aware resolver and explicit transport-ownership contract.
  • [P1] Add a regression test that fails on current main and asserts the metadata-only alias path.
  • Provide redacted live output showing /btw completes through the configured CLI runtime.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR supplies only an unchanged focused Vitest-suite result, no test covers the new alias branch, and the contributor explicitly did not run a live /btw invocation; add redacted terminal or runtime output showing the CLI backend completed the side question, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P2] Merging can replace the early Unknown model error with a direct Anthropic authentication/request failure, or route through the wrong backend, because the configured Claude CLI execution provider is not carried into the fallback path.
  • [P2] The local fallback would create a second model-resolution rule beside the shared runtime-aware resolver, increasing future drift across normal turns and side questions.
  • [P1] No changed-branch regression test or real execution proof currently detects wrong-backend routing.

Maintainer options:

  1. Adopt the runtime-aware resolver path (recommended)
    Refactor this branch or use the competing implementation so model resolution receives explicit transport ownership and preserves the configured CLI execution backend.
  2. Consolidate after proof improves
    If the competing PR becomes clean and shows a completed live side question through the CLI runtime, close this branch in favor of that canonical implementation.

Next step before merge

Security
Cleared: The one-file runtime diff adds no dependency, workflow, permission, secret-handling, downloaded-code, or supply-chain surface; the blocker is functional provider routing.

Review findings

  • [P1] Route the alias through its configured runtime — src/agents/btw.ts:269-271
Review details

Best possible solution:

Make /btw use the shared runtime-aware model resolver with explicit harness transport ownership, cover both metadata-only CLI aliases and ordinary registry models, and demonstrate a redacted live side question completing through the configured CLI runtime.

Do we have a high-confidence way to reproduce the issue?

Yes. The source clearly shows /btw performing a strict registry lookup for a configuration that intentionally has no direct canonical-provider registry entry, matching the detailed issue reproduction; this review did not run the credentialed live setup.

Is this the best way to solve the issue?

No. A bare synthesized model removes the lookup error but does not preserve runtime transport ownership, so the narrowest maintainable fix is to reuse the canonical runtime-aware resolver rather than add a second local resolution rule.

Full review comments:

  • [P1] Route the alias through its configured runtime — src/agents/btw.ts:269-271
    This fallback returns a model whose provider is still the canonical anthropic value, but the OpenClaw side-question branch then resolves auth and registers a provider stream from that value. Because selectAgentHarness maps this alias to OpenClaw passthrough rather than executing Claude CLI itself, the patch can suppress Unknown model without ever selecting the configured claude-cli backend. Reuse the runtime-aware resolver/transport-ownership path instead of synthesizing a bare model.
    Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0fc5a57a3440.

Label changes

Label changes:

  • add P2: The PR targets a deterministic but command-scoped provider-routing regression; normal agent turns remain usable.
  • add merge-risk: 🚨 auth-provider: The fallback can resolve the model name while still selecting direct provider auth and transport instead of the configured agentRuntime backend.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR supplies only an unchanged focused Vitest-suite result, no test covers the new alias branch, and the contributor explicitly did not run a live /btw invocation; add redacted terminal or runtime output showing the CLI backend completed the side question, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.
  • remove rating: 🌊 off-meta tidepool: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P2: The PR targets a deterministic but command-scoped provider-routing regression; normal agent turns remain usable.
  • merge-risk: 🚨 auth-provider: The fallback can resolve the model name while still selecting direct provider auth and transport instead of the configured agentRuntime backend.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR supplies only an unchanged focused Vitest-suite result, no test covers the new alias branch, and the contributor explicitly did not run a live /btw invocation; add redacted terminal or runtime output showing the CLI backend completed the side question, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +11. Total +11 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 13 2 +11
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 13 2 +11

What I checked:

  • Blocking execution-path mismatch: The new object preserves the canonical provider, while the fallback side-question path subsequently obtains provider auth and registers a provider stream from that provider; no selected claude-cli execution provider is passed through. (src/agents/btw.ts:269, b48fd1b56054)
  • Harness contract: selectAgentHarness deliberately maps a CLI runtime alias back to the OpenClaw harness as cli_runtime_passthrough_openclaw; therefore the harness object alone does not execute Claude CLI, and later model resolution must preserve runtime-owned transport semantics. (src/agents/harness/selection.ts:181, 0fc5a57a3440)
  • Missing regression coverage: The current /btw test module mocks discovery, auth, and provider streaming and has no agentRuntime or claude-cli alias test, so the reported 33 passing tests do not validate the added fallback. (src/agents/btw.test.ts:55, 0fc5a57a3440)
  • Canonical fix direction remains open: The competing PR uses resolveModelAsync with explicit runtime transport ownership and adds resolver tests, but it remains open and its published proof does not establish a complete successful live side-question response, so it is not yet a safe superseding landing target. (src/agents/embedded-agent-runner/model.ts:1403, c4d3f0545cc8)
  • Feature provenance: The current /btw runtime-resolution path and the compared CLI alias routing were introduced together in commit 60e818f, supporting review of them as one provider-routing contract. (src/agents/btw.ts:240, 60e818f563d8)

Likely related people:

  • vincentkoc: Commit 60e818f introduced the current /btw runtime-model resolution and related CLI alias execution path being compared. (role: introduced behavior; confidence: high; commits: 60e818f563d8; files: src/agents/btw.ts, src/agents/command/attempt-execution.ts)
  • wangwllu: Authored the open runtime-aware resolver alternative and detailed the metadata-only alias failure contract and tests. (role: adjacent fix author; confidence: medium; commits: c4d3f0545cc8; files: src/agents/btw.ts, src/agents/btw.test.ts, src/agents/embedded-agent-runner/model.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added the rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. label Jun 11, 2026
@zenglingbiao

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@zenglingbiao

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

1 similar comment
@zenglingbiao

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 12, 2026
@joshavant

Copy link
Copy Markdown
Contributor

Thanks @zenglingbiao for the focused PR and for taking on #92168.

We merged #92226 instead of this approach because synthesizing a minimal model can remove the Unknown model symptom while still letting /btw continue toward direct provider auth/streaming. The landed fix fails closed for CLI-runtime aliases before that backend substitution can happen, while preserving the existing non-CLI custom-harness fallback.

Working same-runtime /btw support for CLI runtimes will be added in a future commit via a runtime-owned side-question path.

@joshavant joshavant closed this Jun 12, 2026
@zenglingbiao
zenglingbiao deleted the fix/issue-92168-btw-agent-runtime-alias branch June 22, 2026 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: /btw fails with "Unknown model" for anthropic/* aliases routed via agentRuntime: claude-cli (resolver path divergence from main agent loop)

2 participants