From 9670568ab67580a0fa8a581870df4e3e83556694 Mon Sep 17 00:00:00 2001 From: Alejiri Date: Tue, 4 Aug 2026 15:50:03 +0000 Subject: [PATCH] fix: address CodeRabbit review on provider-opencode-go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - iii-permissions.yaml: deny provider::opencode_go::abort (agents must not cancel router-owned streams; matches provider-claude-code) - sse.rs: relay delta.reasoning_content as thinking blocks (the OpenCode Go wire emits it, live-verified); bound tool-call index to 64 (malformed upstream could grow the vec unboundedly) - upstream.rs: data_line per SSE spec — accept data: without a space and join repeated data: lines instead of silently dropping output - curated.rs: per-model max_output_tokens from models.dev limit.output; 4096 stays the unknown-id fallback - reasoning.rs: Minimal maps to minimal then none (gpt-5.6-luna floor) - register.rs: log router::ready trigger registration failures - router_client.rs: narrow module doc claim - README: repair split provider-openai table row; III_WS_URL -> III_URL (code + engine convention); thinking-delta relay note --- README.md | 3 +- provider-opencode-go/README.md | 7 ++-- provider-opencode-go/iii-permissions.yaml | 1 + provider-opencode-go/src/curated.rs | 28 ++++++++++++++- provider-opencode-go/src/reasoning.rs | 43 +++++++++++++++++------ provider-opencode-go/src/register.rs | 9 +++-- provider-opencode-go/src/router_client.rs | 6 ++-- provider-opencode-go/src/sse.rs | 30 ++++++++++++++++ provider-opencode-go/src/upstream.rs | 23 ++++++++---- 9 files changed, 122 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 793d02971..efa6fcd85 100644 --- a/README.md +++ b/README.md @@ -68,9 +68,8 @@ npx skills add iii-hq/iii --all | [`provider-anthropic`](provider-anthropic/) | Rust | Anthropic Messages API provider behind `llm-router` — `provider::anthropic::stream` with prompt caching, thinking, and live model discovery. | | [`provider-claude-code`](provider-claude-code/) | Rust | Claude Code (Pro/Max subscription) Messages API provider behind `llm-router` — `provider::claude-code::stream` using OAuth credentials from the auth-credentials vault or `~/.claude/.credentials.json`, namespaced `claude-code/*` catalog. Local/personal dev only (ToS caveat). | | [`provider-llamacpp`](provider-llamacpp/) | Rust | llama.cpp server (`llama-server`) Chat Completions provider behind `llm-router` — `provider::llamacpp::stream` with optional (no-`--api-key`) auth, real json_schema-constrained output, and live model discovery via `/v1/models` + `/props`. | -| [`provider-openai`](provider-openai/) | Rust | OpenAI Chat Completions provider behind `llm-router` — `provider::open +| [`provider-openai`](provider-openai/) | Rust | OpenAI Chat Completions provider behind `llm-router` — `provider::openai::stream` with reasoning support and live chat-model discovery, plus `provider::openai::embed` for batch embeddings (OpenAI-compatible endpoints included). | | [`provider-opencode-go`](provider-opencode-go/) | Rust | OpenCode Go Chat Completions provider behind `llm-router` — `provider::opencode_go::stream`, live models.dev-enriched catalog via `refresh_models` | -ai::stream` with reasoning support and live chat-model discovery, plus `provider::openai::embed` for batch embeddings (OpenAI-compatible endpoints included). | | [`provider-xai`](provider-xai/) | Rust | xAI (Grok) Chat Completions provider behind `llm-router` — `provider::xai::stream` with grok reasoning support and live model discovery against `api.x.ai`. | | [`provider-zai`](provider-zai/) | Rust | Z.AI (GLM) Chat Completions provider behind `llm-router` — `provider::zai::stream` with GLM thinking/effort support and a curated catalog against `api.z.ai` (no upstream model listing). | | [`shell`](shell/) | Rust | Unix shell + filesystem worker — `shell::exec` with denylist/timeout/output caps and background jobs; `fs::ls`/`stat`/`mkdir`/`rm`/`chmod`/`mv`/`grep`/`sed`/`read`/`write` with host jail, denylist, and size caps. | diff --git a/provider-opencode-go/README.md b/provider-opencode-go/README.md index 407d9a833..e09e00036 100644 --- a/provider-opencode-go/README.md +++ b/provider-opencode-go/README.md @@ -53,8 +53,9 @@ configuration entry (`providers.opencode_go.api_key`, default endpoint when the model's curated effort list accepts the level (e.g. `grok-4.5` accepts `low`/`medium`/`high`, `deepseek-v4-flash` accepts `high`/`max`); models that reason without published effort levels, and unknown ids, stream - without the field. Thinking content is not streamed — the OpenCode Go Chat - Completions wire carries no reasoning deltas. + without the field. When the upstream emits `reasoning_content` deltas they + are relayed as thinking blocks; models that never emit them stream text + only. - **Structured output:** a `response_format` with a schema maps to strict `json_schema` mode; without one, `json_object` mode (the caller must mention "JSON" in the prompt per OpenAI-compatible API rules). @@ -74,7 +75,7 @@ provider, and a local stub upstream — no external API calls anywhere. ## Running The binary takes the standard worker CLI flags: `--url` (engine WebSocket, -default `ws://127.0.0.1:49134`, falls back to the `III_WS_URL` environment +default `ws://127.0.0.1:49134`, falls back to the `III_URL` environment variable), `--manifest` (print the registry manifest and exit), and `--config` (accepted but ignored with a warning — provider config comes from the `llm-router` configuration entry). diff --git a/provider-opencode-go/iii-permissions.yaml b/provider-opencode-go/iii-permissions.yaml index b90363e5c..882619884 100644 --- a/provider-opencode-go/iii-permissions.yaml +++ b/provider-opencode-go/iii-permissions.yaml @@ -5,5 +5,6 @@ rules: # Direct provider calls bypass the router's accounting, budgets, and retry # policy — never agent-callable. The router invokes these worker-to-worker. - '!provider::opencode_go::stream' + - '!provider::opencode_go::abort' - '!provider::opencode_go::refresh_models' - '!provider::opencode_go::on_router_ready' diff --git a/provider-opencode-go/src/curated.rs b/provider-opencode-go/src/curated.rs index 0bc632af7..50e7b4ae5 100644 --- a/provider-opencode-go/src/curated.rs +++ b/provider-opencode-go/src/curated.rs @@ -21,6 +21,7 @@ use llm_router::types::model::{Model, ReasoningEffort}; /// be omitted rather than guessed. pub(crate) struct ModelMeta { pub(crate) context_window: u64, + pub(crate) max_output: u64, pub(crate) reasoning: bool, pub(crate) reasoning_efforts: &'static [&'static str], pub(crate) tool_call: bool, @@ -31,6 +32,7 @@ pub(crate) struct ModelMeta { pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { match id { "grok-4.5" => Some(&ModelMeta { + max_output: 500000, context_window: 500_000, reasoning: true, reasoning_efforts: &["low", "medium", "high"], @@ -38,6 +40,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: true, }), "glm-5.2" => Some(&ModelMeta { + max_output: 131072, context_window: 1_000_000, reasoning: true, reasoning_efforts: &["high", "max"], @@ -45,6 +48,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: true, }), "glm-5.1" => Some(&ModelMeta { + max_output: 32768, context_window: 202_752, reasoning: true, reasoning_efforts: &[], @@ -52,6 +56,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "glm-5" => Some(&ModelMeta { + max_output: 32768, context_window: 202_752, reasoning: true, reasoning_efforts: &[], @@ -59,6 +64,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "gpt-5.6-luna" => Some(&ModelMeta { + max_output: 128000, context_window: 1_050_000, reasoning: true, reasoning_efforts: &["none", "low", "medium", "high", "xhigh", "max"], @@ -66,6 +72,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: true, }), "kimi-k3" => Some(&ModelMeta { + max_output: 131072, context_window: 1_048_576, reasoning: true, reasoning_efforts: &["max"], @@ -73,6 +80,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: true, }), "kimi-k2.7-code" => Some(&ModelMeta { + max_output: 262144, context_window: 262_144, reasoning: true, reasoning_efforts: &[], @@ -80,6 +88,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: true, }), "kimi-k2.6" => Some(&ModelMeta { + max_output: 65536, context_window: 262_144, reasoning: true, reasoning_efforts: &[], @@ -87,6 +96,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "kimi-k2.5" => Some(&ModelMeta { + max_output: 65536, context_window: 262_144, reasoning: true, reasoning_efforts: &[], @@ -94,6 +104,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "minimax-m3" => Some(&ModelMeta { + max_output: 131072, context_window: 1_000_000, reasoning: true, reasoning_efforts: &[], @@ -101,6 +112,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "minimax-m2.7" => Some(&ModelMeta { + max_output: 131072, context_window: 204_800, reasoning: true, reasoning_efforts: &[], @@ -108,6 +120,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "minimax-m2.5" => Some(&ModelMeta { + max_output: 65536, context_window: 204_800, reasoning: true, reasoning_efforts: &[], @@ -115,6 +128,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "qwen3.7-max" => Some(&ModelMeta { + max_output: 65536, context_window: 1_000_000, reasoning: true, reasoning_efforts: &[], @@ -122,6 +136,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "qwen3.7-plus" => Some(&ModelMeta { + max_output: 65536, context_window: 1_000_000, reasoning: true, reasoning_efforts: &[], @@ -129,6 +144,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "qwen3.8-max" => Some(&ModelMeta { + max_output: 131072, context_window: 1_000_000, reasoning: true, reasoning_efforts: &[], @@ -136,6 +152,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: true, }), "qwen3.5-plus" => Some(&ModelMeta { + max_output: 65536, context_window: 262_144, reasoning: true, reasoning_efforts: &[], @@ -143,6 +160,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "qwen3.6-plus" => Some(&ModelMeta { + max_output: 65536, context_window: 1_000_000, reasoning: true, reasoning_efforts: &[], @@ -150,6 +168,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "deepseek-v4-pro" => Some(&ModelMeta { + max_output: 384000, context_window: 1_000_000, reasoning: true, reasoning_efforts: &["high", "max"], @@ -157,6 +176,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: true, }), "deepseek-v4-flash" => Some(&ModelMeta { + max_output: 384000, context_window: 1_000_000, reasoning: true, reasoning_efforts: &["high", "max"], @@ -164,6 +184,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: true, }), "mimo-v2-omni" => Some(&ModelMeta { + max_output: 128000, context_window: 262_144, reasoning: true, reasoning_efforts: &[], @@ -171,6 +192,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "mimo-v2-pro" => Some(&ModelMeta { + max_output: 128000, context_window: 1_048_576, reasoning: true, reasoning_efforts: &[], @@ -178,6 +200,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "mimo-v2.5" => Some(&ModelMeta { + max_output: 128000, context_window: 1_000_000, reasoning: true, reasoning_efforts: &[], @@ -185,6 +208,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "mimo-v2.5-pro" => Some(&ModelMeta { + max_output: 128000, context_window: 1_048_576, reasoning: true, reasoning_efforts: &[], @@ -192,6 +216,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { structured_output: false, }), "hy3" => Some(&ModelMeta { + max_output: 64000, context_window: 256_000, reasoning: true, reasoning_efforts: &["none", "low", "high"], @@ -201,6 +226,7 @@ pub(crate) fn meta(id: &str) -> Option<&'static ModelMeta> { // Preview variant in the subscription catalog but not on models.dev — // conservative defaults rather than guessing hy3-like metadata. "hy3-preview" => Some(&ModelMeta { + max_output: 4096, context_window: 128_000, reasoning: false, reasoning_efforts: &[], @@ -221,7 +247,7 @@ pub fn enrich(id: &str) -> Model { provider: PROVIDER_ID.into(), display_name: Some(id.into()), context_window: m.context_window, - max_output_tokens: 4096, + max_output_tokens: m.max_output, input_limit: None, supports_thinking: if m.reasoning { Some(true) } else { None }, supports_xhigh: if m.reasoning_efforts.contains(&"xhigh") { diff --git a/provider-opencode-go/src/reasoning.rs b/provider-opencode-go/src/reasoning.rs index e5ebbb6b4..f5aac9cd0 100644 --- a/provider-opencode-go/src/reasoning.rs +++ b/provider-opencode-go/src/reasoning.rs @@ -27,13 +27,16 @@ fn supported_efforts(model: &str) -> &'static [&'static str] { .unwrap_or(&[]) } -fn level_str(level: ThinkingLevel) -> &'static str { +fn level_efforts(level: ThinkingLevel) -> &'static [&'static str] { match level { - ThinkingLevel::Minimal => "minimal", - ThinkingLevel::Low => "low", - ThinkingLevel::Medium => "medium", - ThinkingLevel::High => "high", - ThinkingLevel::Xhigh => "xhigh", + // Some catalogs publish "none" as their floor instead of "minimal" + // (e.g. gpt-5.6-luna); prefer the literal level, fall back to the + // closest accepted floor rather than omitting the param entirely. + ThinkingLevel::Minimal => &["minimal", "none"], + ThinkingLevel::Low => &["low"], + ThinkingLevel::Medium => &["medium"], + ThinkingLevel::High => &["high"], + ThinkingLevel::Xhigh => &["xhigh"], } } @@ -45,11 +48,10 @@ pub fn reasoning_effort_for(level: Option, model: &str) -> Option if ladder.is_empty() { return None; } - let want = level_str(level?); - if ladder.contains(&want) { - return Some(want); - } - None + level_efforts(level?) + .iter() + .find(|want| ladder.contains(want)) + .copied() } #[cfg(test)] @@ -171,6 +173,25 @@ mod tests { ); } + #[test] + fn minimal_falls_back_to_none_when_not_published() { + // gpt-5.6-luna publishes "none" as its floor — minimal maps to it. + assert_eq!( + reasoning_effort_for(Some(ThinkingLevel::Minimal), "gpt-5.6-luna"), + Some("none") + ); + // grok-4.5 publishes neither minimal nor none — omit the param. + assert_eq!( + reasoning_effort_for(Some(ThinkingLevel::Minimal), "grok-4.5"), + None + ); + // hy3 publishes "none" in its ladder. + assert_eq!( + reasoning_effort_for(Some(ThinkingLevel::Minimal), "hy3"), + Some("none") + ); + } + #[test] fn absent_level_omits_the_param() { assert_eq!(reasoning_effort_for(None, "deepseek-v4-flash"), None); diff --git a/provider-opencode-go/src/register.rs b/provider-opencode-go/src/register.rs index 7238630a9..17ef37fb5 100644 --- a/provider-opencode-go/src/register.rs +++ b/provider-opencode-go/src/register.rs @@ -164,12 +164,17 @@ pub async fn register_provider(iii: IIIClient) -> Result<(), Error> { .metadata(json!({ "internal": true })), ); } - let _ = iii.register_trigger(RegisterTriggerInput { + if let Err(e) = iii.register_trigger(RegisterTriggerInput { trigger_type: "router::ready".into(), function_id: surface::ON_ROUTER_READY_ID.into(), config: json!({}), metadata: None, - }); + }) { + tracing::warn!( + error = %e, + "failed to bind the router::ready trigger; the provider will not re-declare on router restarts" + ); + } // Boot declare, off the boot path. tokio::spawn(declare_and_refresh(iii, http)); diff --git a/provider-opencode-go/src/router_client.rs b/provider-opencode-go/src/router_client.rs index 8b777c0f5..012367543 100644 --- a/provider-opencode-go/src/router_client.rs +++ b/provider-opencode-go/src/router_client.rs @@ -1,6 +1,8 @@ //! Provider-scoped shims over the shared router-protocol client -//! (`llm_router::provider_scaffold::router_client`): every call binds this -//! crate's `PROVIDER_ID` and carries the registration token. +//! (`llm_router::provider_scaffold::router_client`): the resolve, reconcile, +//! and models_get wrappers bind this crate's `PROVIDER_ID` and carry the +//! registration token. `register` forwards a declaration payload that already +//! carries both (see `register::declare_once`). use crate::PROVIDER_ID; use iii_sdk::errors::Error; use iii_sdk::IIIClient; diff --git a/provider-opencode-go/src/sse.rs b/provider-opencode-go/src/sse.rs index b0546a35d..4c7505e0d 100644 --- a/provider-opencode-go/src/sse.rs +++ b/provider-opencode-go/src/sse.rs @@ -8,9 +8,14 @@ use llm_router::types::events::{AssistantMessageEvent, ErrorKind, StopReason, Us use llm_router::types::messages::{AssistantMessage, AssistantRoleTag}; use serde_json::Value; +/// Upper bound on a tool-call index accepted from the upstream stream; +/// larger indices are dropped (the vec would otherwise grow to reach them). +const MAX_TOOL_CALL_INDEX: usize = 64; + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum OpenBlock { Text, + Thinking, Call(usize), } @@ -190,6 +195,9 @@ fn close_open_block( Some(OpenBlock::Text) => events.push(AssistantMessageEvent::TextEnd { partial: build_partial(state, model), }), + Some(OpenBlock::Thinking) => events.push(AssistantMessageEvent::ThinkingEnd { + partial: build_partial(state, model), + }), Some(OpenBlock::Call(_)) => events.push(AssistantMessageEvent::FunctioncallEnd { partial: build_partial(state, model), }), @@ -249,9 +257,31 @@ pub fn handle_chunk( }); } } + if let Some(text) = delta.get("reasoning_content").and_then(Value::as_str) { + if !text.is_empty() { + if state.open_block != Some(OpenBlock::Thinking) { + close_open_block(state, model, &mut events); + state.open_block = Some(OpenBlock::Thinking); + events.push(AssistantMessageEvent::ThinkingStart { + partial: build_partial(state, model), + }); + } + state.thinking.push_str(text); + events.push(AssistantMessageEvent::ThinkingDelta { + partial: None, + delta: text.to_string(), + }); + } + } if let Some(tool_calls) = delta.get("tool_calls").and_then(Value::as_array) { for tc in tool_calls { let index = tc.get("index").and_then(Value::as_u64).unwrap_or(0) as usize; + // A hostile or malformed upstream could name an arbitrary + // index; the while loop below grows the vec to reach it. + if index >= MAX_TOOL_CALL_INDEX { + tracing::debug!(index, "dropping tool-call delta with oversized index"); + continue; + } while state.function_calls.len() <= index { state.function_calls.push(PartialFunctionCall::default()); } diff --git a/provider-opencode-go/src/upstream.rs b/provider-opencode-go/src/upstream.rs index 8e5dadc6e..9dbc392fe 100644 --- a/provider-opencode-go/src/upstream.rs +++ b/provider-opencode-go/src/upstream.rs @@ -40,12 +40,21 @@ pub fn spawn_upstream( rx } -/// Last `data: ` payload in an SSE block, if any. -fn data_line(block: &str) -> Option<&str> { - block - .lines() - .filter_map(|l| l.strip_prefix("data: ")) - .next_back() +/// All `data` field values in an SSE block, joined with `\n` per the SSE +/// spec (event-stream format). The optional single space after the colon is +/// stripped; a frame may also repeat `data:` across lines. +fn data_line(block: &str) -> Option { + let mut parts = block.lines().filter_map(|l| { + l.strip_prefix("data:") + .map(|v| v.strip_prefix(' ').unwrap_or(v)) + }); + let first = parts.next()?; + let mut out = first.to_string(); + for p in parts { + out.push('\n'); + out.push_str(p); + } + Some(out) } async fn run_upstream( @@ -121,7 +130,7 @@ async fn run_upstream( }, ]; } - let Ok(parsed) = serde_json::from_str::(data) else { + let Ok(parsed) = serde_json::from_str::(&data) else { return vec![]; }; handle_chunk(&parsed, state, model)