From 1440ec742259646f9c8eae944e301599764d217c Mon Sep 17 00:00:00 2001 From: Henry Park Date: Mon, 9 Mar 2026 16:58:56 -0700 Subject: [PATCH 1/5] fix(ci): secrets can't be used in step if conditions [skip-regression-check] (#787) GitHub Actions step-level `if:` doesn't have access to `secrets` context. Replace `if: secrets.X != ''` with `continue-on-error: true` and let the Set token step handle the fallback. Co-authored-by: Claude Sonnet 4.6 --- .github/workflows/staging-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/staging-ci.yml b/.github/workflows/staging-ci.yml index 9e887436..22d70030 100644 --- a/.github/workflows/staging-ci.yml +++ b/.github/workflows/staging-ci.yml @@ -115,7 +115,7 @@ jobs: - name: Generate GitHub App token id: app-token - if: ${{ secrets.GH_RELEASES_MANAGER_APP_ID != '' }} + continue-on-error: true uses: actions/create-github-app-token@v2 with: app-id: ${{ secrets.GH_RELEASES_MANAGER_APP_ID }} @@ -230,7 +230,7 @@ jobs: - name: Generate GitHub App token id: app-token - if: ${{ secrets.GH_RELEASES_MANAGER_APP_ID != '' }} + continue-on-error: true uses: actions/create-github-app-token@v2 with: app-id: ${{ secrets.GH_RELEASES_MANAGER_APP_ID }} From b53986f00b994060f36808ca18a60903cb8626ef Mon Sep 17 00:00:00 2001 From: Henry Park Date: Mon, 9 Mar 2026 17:35:06 -0700 Subject: [PATCH 2/5] =?UTF-8?q?fix(ci):=20clean=20up=20staging=20pipeline?= =?UTF-8?q?=20=E2=80=94=20remove=20hacks,=20skip=20redundant=20checks=20[s?= =?UTF-8?q?kip-regression-check]=20(#794)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove continue-on-error from staging-ci.yml app token steps (secrets are configured) - Skip test.yml and code_style.yml on PRs targeting staging (staging-ci.yml already runs tests before promoting, promotion PR gets full CI on main) - Allow ironclaw-ci[bot] in Claude Code review for bot-created promotion PRs Co-authored-by: Claude Opus 4.6 --- .github/workflows/claude-review.yml | 1 + .github/workflows/code_style.yml | 2 ++ .github/workflows/staging-ci.yml | 2 -- .github/workflows/test.yml | 2 ++ 4 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 86d1bb2f..24d2fe98 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -28,6 +28,7 @@ jobs: uses: anthropics/claude-code-action@v1 with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + allowed_bots: "ironclaw-ci[bot]" claude_args: "--max-turns 50 --model claude-haiku-4-5-20251001 --allowedTools 'Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*),Bash(gh issue view:*),Bash(gh issue list:*),Bash(gh search:*),Bash(git blame:*),Bash(git log:*),Bash(git diff:*)'" prompt: | Code review this pull request. Follow these steps precisely: diff --git a/.github/workflows/code_style.yml b/.github/workflows/code_style.yml index 526c7740..c65aa0df 100644 --- a/.github/workflows/code_style.yml +++ b/.github/workflows/code_style.yml @@ -1,6 +1,8 @@ name: Code Style on: pull_request: + branches: + - main jobs: format: diff --git a/.github/workflows/staging-ci.yml b/.github/workflows/staging-ci.yml index 22d70030..8e3693b2 100644 --- a/.github/workflows/staging-ci.yml +++ b/.github/workflows/staging-ci.yml @@ -115,7 +115,6 @@ jobs: - name: Generate GitHub App token id: app-token - continue-on-error: true uses: actions/create-github-app-token@v2 with: app-id: ${{ secrets.GH_RELEASES_MANAGER_APP_ID }} @@ -230,7 +229,6 @@ jobs: - name: Generate GitHub App token id: app-token - continue-on-error: true uses: actions/create-github-app-token@v2 with: app-id: ${{ secrets.GH_RELEASES_MANAGER_APP_ID }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index efa28648..bb29dd2a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -2,6 +2,8 @@ name: Run Tests on: workflow_call: pull_request: + branches: + - main push: branches: - main From 6e12ce6f2db8e6632ff5c95bbd0b8285ba90d4ca Mon Sep 17 00:00:00 2001 From: Henry Park Date: Mon, 9 Mar 2026 18:43:16 -0700 Subject: [PATCH 3/5] fix(ci): run fmt + clippy on staging PRs, skip Windows clippy [skip-regression-check] (#802) - Remove branches:[main] filter from code_style.yml so it runs on all PRs - Gate clippy-windows with `if: github.base_ref == 'main'` (skip on staging PRs) - Update rollup job to allow skipped clippy-windows - Simplify claude-review.yml to only trigger on labeled event (avoids duplicate runs) Co-authored-by: Claude Opus 4.6 --- .github/workflows/claude-review.yml | 2 +- .github/workflows/code_style.yml | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 24d2fe98..3836a5f9 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -2,7 +2,7 @@ name: Claude Code Review on: pull_request: - types: [opened, labeled] + types: [labeled] permissions: contents: read diff --git a/.github/workflows/code_style.yml b/.github/workflows/code_style.yml index c65aa0df..620760ae 100644 --- a/.github/workflows/code_style.yml +++ b/.github/workflows/code_style.yml @@ -1,8 +1,6 @@ name: Code Style on: pull_request: - branches: - - main jobs: format: @@ -46,6 +44,7 @@ jobs: clippy-windows: name: Clippy Windows (${{ matrix.name }}) + if: github.base_ref == 'main' runs-on: windows-latest strategy: fail-fast: false @@ -78,7 +77,12 @@ jobs: needs: [format, clippy, clippy-windows] steps: - run: | - if [[ "${{ needs.format.result }}" != "success" || "${{ needs.clippy.result }}" != "success" || "${{ needs.clippy-windows.result }}" != "success" ]]; then + if [[ "${{ needs.format.result }}" != "success" || "${{ needs.clippy.result }}" != "success" ]]; then echo "One or more jobs failed" exit 1 fi + # clippy-windows only runs on main PRs, so skip/success are both acceptable + if [[ "${{ needs.clippy-windows.result }}" == "failure" ]]; then + echo "Windows clippy failed" + exit 1 + fi From bcef04b82108222c9041e733de459130badd4cd7 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Tue, 10 Mar 2026 01:51:43 +0000 Subject: [PATCH 4/5] feat: persist user_id in save_job and expose job_id on routine runs (#709) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: persist worker events to DB and fix activity tab rendering In-process Worker (used by Scheduler::dispatch_job) now persists events via save_job_event at key execution points: plan creation, LLM responses, tool_use, tool_result, and job completion/failure/stuck. Event data shapes match the container worker format so the gateway activity tab renders them correctly. Frontend: tool_result errors now show a red X icon with danger styling instead of a silent empty output. The result event falls back to the error field when message is absent. Co-Authored-By: Claude Opus 4.6 * feat: wire RoutineEngine into gateway for direct manual trigger firing Replace the message-channel hack in routines_trigger_handler with a direct call to RoutineEngine::fire_manual(), ensuring FullJob routines dispatch correctly when triggered from the web UI. Inject the engine into GatewayState from Agent::run after construction. Also persists user_id in save_job for both PG and libSQL backends, removes the source='sandbox' filter so all jobs are visible, and exposes job_id on RoutineRunInfo for the frontend job link. Co-Authored-By: Claude Opus 4.6 * fix: remove stale gateway_state argument from Agent::new test call sites The gateway_state parameter was removed from Agent::new during rebase (replaced by post-construction set_routine_engine_slot), but three test call sites still passed the extra None argument. Co-Authored-By: Claude Opus 4.6 * fix: address PR review — restore sandbox source filter, remove blank lines - Revert removal of `source = 'sandbox'` filter in all SandboxStore queries (8 sites across PG and libSQL). Sandbox-specific APIs should stay scoped to sandbox jobs; unified job listing for the Jobs tab should use a separate query path. - Remove extra blank lines in agent_loop.rs and worker.rs that caused formatting CI failure. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 * fix: address review — regenerate Cargo.lock, add user_id regression test - Regenerate Cargo.lock from main's lockfile to eliminate dependency version downgrades (anyhow, syn, etc.) that were churn from rebase. - Add regression test verifying user_id round-trips through save_job and get_job in the libSQL backend. Co-Authored-By: Claude Opus 4.6 * style: remove trailing blank line in libsql jobs.rs [skip-regression-check] Co-Authored-By: Claude Opus 4.6 * test: add Postgres-side regression test for user_id persistence in save_job Mirrors the existing libSQL test (test_save_job_persists_user_id) for the Postgres backend. Gated behind #[cfg(feature = "postgres")] + #[ignore] since it requires a running PostgreSQL instance (integration tier). Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- src/channels/web/handlers/routines.rs | 2 ++ src/channels/web/server.rs | 2 ++ src/channels/web/types.rs | 1 + src/db/libsql/jobs.rs | 5 +++- src/db/libsql/mod.rs | 18 +++++++++++++ src/history/store.rs | 37 ++++++++++++++++++++++++++- 6 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/channels/web/handlers/routines.rs b/src/channels/web/handlers/routines.rs index d7c4f764..8fbcc97b 100644 --- a/src/channels/web/handlers/routines.rs +++ b/src/channels/web/handlers/routines.rs @@ -108,6 +108,7 @@ pub async fn routines_detail_handler( status: format!("{:?}", run.status), result_summary: run.result_summary.clone(), tokens_used: run.tokens_used, + job_id: run.job_id, }) .collect(); @@ -252,6 +253,7 @@ pub async fn routines_runs_handler( status: format!("{:?}", run.status), result_summary: run.result_summary.clone(), tokens_used: run.tokens_used, + job_id: run.job_id, }) .collect(); diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index d6605eee..f454363d 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -2017,6 +2017,7 @@ async fn routines_detail_handler( status: format!("{:?}", run.status), result_summary: run.result_summary.clone(), tokens_used: run.tokens_used, + job_id: run.job_id, }) .collect(); @@ -2169,6 +2170,7 @@ async fn routines_runs_handler( status: format!("{:?}", run.status), result_summary: run.result_summary.clone(), tokens_used: run.tokens_used, + job_id: run.job_id, }) .collect(); diff --git a/src/channels/web/types.rs b/src/channels/web/types.rs index 4d85c671..b6d0d05a 100644 --- a/src/channels/web/types.rs +++ b/src/channels/web/types.rs @@ -776,6 +776,7 @@ pub struct RoutineRunInfo { pub status: String, pub result_summary: Option, pub tokens_used: Option, + pub job_id: Option, } // --- Settings --- diff --git a/src/db/libsql/jobs.rs b/src/db/libsql/jobs.rs index d5172360..0750873d 100644 --- a/src/db/libsql/jobs.rs +++ b/src/db/libsql/jobs.rs @@ -28,14 +28,16 @@ impl JobStore for LibSqlBackend { r#" INSERT INTO agent_jobs ( id, conversation_id, title, description, category, status, source, + user_id, budget_amount, budget_token, bid_amount, estimated_cost, estimated_time_secs, actual_cost, repair_attempts, created_at, started_at, completed_at - ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17) + ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18) ON CONFLICT (id) DO UPDATE SET title = excluded.title, description = excluded.description, category = excluded.category, status = excluded.status, + user_id = excluded.user_id, estimated_cost = excluded.estimated_cost, estimated_time_secs = excluded.estimated_time_secs, actual_cost = excluded.actual_cost, @@ -51,6 +53,7 @@ impl JobStore for LibSqlBackend { opt_text(ctx.category.as_deref()), status, "direct", + ctx.user_id.as_str(), opt_text_owned(ctx.budget.map(|d| d.to_string())), opt_text(ctx.budget_token.as_deref()), opt_text_owned(ctx.bid_amount.map(|d| d.to_string())), diff --git a/src/db/libsql/mod.rs b/src/db/libsql/mod.rs index 2845c757..404441e6 100644 --- a/src/db/libsql/mod.rs +++ b/src/db/libsql/mod.rs @@ -482,6 +482,24 @@ mod tests { assert_eq!(timeout, 5000); } + /// Regression test: save_job must persist user_id and get_job must return it. + #[tokio::test] + async fn test_save_job_persists_user_id() { + use crate::context::JobContext; + use crate::db::JobStore; + + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("test_user_id.db"); + let backend = LibSqlBackend::new_local(&db_path).await.unwrap(); + backend.run_migrations().await.unwrap(); + + let ctx = JobContext::with_user("test-user-42", "Test Job", "A test job"); + backend.save_job(&ctx).await.unwrap(); + + let loaded = backend.get_job(ctx.job_id).await.unwrap().unwrap(); + assert_eq!(loaded.user_id, "test-user-42"); + } + #[tokio::test] async fn test_concurrent_writes_succeed() { // Use a temp file so connections share state (in-memory DBs are connection-local) diff --git a/src/history/store.rs b/src/history/store.rs index f0b0b144..1153f3e4 100644 --- a/src/history/store.rs +++ b/src/history/store.rs @@ -149,14 +149,16 @@ impl Store { r#" INSERT INTO agent_jobs ( id, conversation_id, title, description, category, status, source, + user_id, budget_amount, budget_token, bid_amount, estimated_cost, estimated_time_secs, actual_cost, repair_attempts, created_at, started_at, completed_at - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17) + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18) ON CONFLICT (id) DO UPDATE SET title = EXCLUDED.title, description = EXCLUDED.description, category = EXCLUDED.category, status = EXCLUDED.status, + user_id = EXCLUDED.user_id, estimated_cost = EXCLUDED.estimated_cost, estimated_time_secs = EXCLUDED.estimated_time_secs, actual_cost = EXCLUDED.actual_cost, @@ -172,6 +174,7 @@ impl Store { &ctx.category, &status, &"direct", // source + &ctx.user_id, &ctx.budget, &ctx.budget_token, &ctx.bid_amount, @@ -2133,4 +2136,36 @@ mod tests { assert_eq!(summary.channel, ch); } } + + /// Regression test: save_job must persist user_id and get_job must return it. + /// Requires a running PostgreSQL instance (integration tier). + #[cfg(feature = "postgres")] + #[tokio::test] + #[ignore] + async fn test_save_job_persists_user_id() { + use crate::config::Config; + use crate::context::JobContext; + + let _ = dotenvy::dotenv(); + let config = Config::from_env().await.expect("Failed to load config"); + let store = Store::new(&config.database) + .await + .expect("Failed to connect to database"); + store + .run_migrations() + .await + .expect("Failed to run migrations"); + + let ctx = JobContext::with_user("test-user-42", "PG user_id test", "regression test"); + store.save_job(&ctx).await.unwrap(); + + let loaded = store.get_job(ctx.job_id).await.unwrap().unwrap(); + assert_eq!(loaded.user_id, "test-user-42"); + + // Clean up + let conn = store.conn().await.unwrap(); + conn.execute("DELETE FROM agent_jobs WHERE id = $1", &[&ctx.job_id]) + .await + .unwrap(); + } } From 2016693b0c26eabaae00c4fd1602d3d7819fdbaa Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Tue, 10 Mar 2026 07:11:26 +0000 Subject: [PATCH 5/5] feat(llm): per-provider unsupported parameter filtering (#749, #728) (#809) Add declarative `unsupported_params` field to provider definitions in providers.json. Parameters listed are stripped from requests before sending, preventing 400 errors from providers that reject them (e.g. gpt-5 family and kimi-k2.5 rejecting custom temperature values). - Add `unsupported_params` to ProviderDefinition and RegistryProviderConfig - Propagate from registry through config resolution - Generic strip helpers handle temperature, max_tokens, stop_sequences - Apply filtering in RigAdapter and AnthropicOAuthProvider - Mark openai and tinfoil providers as unsupporting temperature - Update openai default model to gpt-5-mini Co-authored-by: Claude Opus 4.6 --- providers.json | 4 +- src/config/llm.rs | 10 +++ src/llm/anthropic_oauth.rs | 44 ++++++++++- src/llm/config.rs | 4 + src/llm/mod.rs | 12 ++- src/llm/registry.rs | 56 ++++++++++++++ src/llm/rig_adapter.rs | 146 ++++++++++++++++++++++++++++++++++++- src/setup/wizard.rs | 1 + 8 files changed, 267 insertions(+), 10 deletions(-) diff --git a/providers.json b/providers.json index a9398a87..f7574f4c 100644 --- a/providers.json +++ b/providers.json @@ -9,8 +9,9 @@ "api_key_required": true, "base_url_env": "OPENAI_BASE_URL", "model_env": "OPENAI_MODEL", - "default_model": "gpt-4o", + "default_model": "gpt-5-mini", "description": "OpenAI GPT models (direct API)", + "unsupported_params": ["temperature"], "setup": { "kind": "api_key", "secret_name": "llm_openai_api_key", @@ -86,6 +87,7 @@ "model_env": "TINFOIL_MODEL", "default_model": "kimi-k2-5", "description": "Tinfoil private inference (hardware-attested TEE)", + "unsupported_params": ["temperature"], "setup": { "kind": "api_key", "secret_name": "llm_tinfoil_api_key", diff --git a/src/config/llm.rs b/src/config/llm.rs index 08a59866..cc02cd31 100644 --- a/src/config/llm.rs +++ b/src/config/llm.rs @@ -209,6 +209,7 @@ impl LlmConfig { extra_headers_env, api_key_required, base_url_required, + unsupported_params, ) = if let Some(def) = def { ( def.id.as_str(), @@ -221,6 +222,7 @@ impl LlmConfig { def.extra_headers_env.as_deref(), def.api_key_required, def.base_url_required, + def.unsupported_params.clone(), ) } else { // Absolute fallback: treat as generic openai_completions @@ -235,6 +237,7 @@ impl LlmConfig { Some("LLM_EXTRA_HEADERS"), false, true, + Vec::new(), ) }; @@ -338,6 +341,7 @@ impl LlmConfig { extra_headers, oauth_token, cache_retention, + unsupported_params, }) } } @@ -624,6 +628,12 @@ mod tests { let provider = cfg.provider.expect("provider config should be present"); assert_eq!(provider.base_url, "https://inference.tinfoil.sh/v1"); assert_eq!(provider.model, "kimi-k2-5"); + assert!( + provider + .unsupported_params + .contains(&"temperature".to_string()), + "tinfoil should propagate unsupported_params from registry" + ); } #[test] diff --git a/src/llm/anthropic_oauth.rs b/src/llm/anthropic_oauth.rs index 104778ad..0badda93 100644 --- a/src/llm/anthropic_oauth.rs +++ b/src/llm/anthropic_oauth.rs @@ -6,6 +6,8 @@ //! //! Pattern follows `nearai_chat.rs`: direct HTTP calls via `reqwest::Client`. +use std::collections::HashSet; + use async_trait::async_trait; use reqwest::Client; use rust_decimal::Decimal; @@ -35,6 +37,8 @@ pub struct AnthropicOAuthProvider { model: String, base_url: Option, active_model: std::sync::RwLock, + /// Parameter names that this provider does not support. + unsupported_params: HashSet, } impl AnthropicOAuthProvider { @@ -61,15 +65,45 @@ impl AnthropicOAuthProvider { Some(config.base_url.clone()) }; + let unsupported_params: HashSet = + config.unsupported_params.iter().cloned().collect(); + Ok(Self { client, token, model: config.model.clone(), base_url, active_model, + unsupported_params, }) } + /// Strip unsupported fields from a `CompletionRequest` in place. + fn strip_unsupported_completion_params(&self, req: &mut CompletionRequest) { + if self.unsupported_params.is_empty() { + return; + } + if self.unsupported_params.contains("temperature") { + req.temperature = None; + } + if self.unsupported_params.contains("max_tokens") { + req.max_tokens = None; + } + } + + /// Strip unsupported fields from a `ToolCompletionRequest` in place. + fn strip_unsupported_tool_params(&self, req: &mut ToolCompletionRequest) { + if self.unsupported_params.is_empty() { + return; + } + if self.unsupported_params.contains("temperature") { + req.temperature = None; + } + if self.unsupported_params.contains("max_tokens") { + req.max_tokens = None; + } + } + fn api_url(&self) -> String { if let Some(ref base) = self.base_url { let base = base.trim_end_matches('/'); @@ -197,8 +231,9 @@ impl AnthropicOAuthProvider { #[async_trait] impl LlmProvider for AnthropicOAuthProvider { - async fn complete(&self, req: CompletionRequest) -> Result { - let model = req.model.unwrap_or_else(|| self.active_model_name()); + async fn complete(&self, mut req: CompletionRequest) -> Result { + let model = req.model.take().unwrap_or_else(|| self.active_model_name()); + self.strip_unsupported_completion_params(&mut req); let (system, messages) = convert_messages(req.messages); let request = AnthropicRequest { @@ -233,9 +268,10 @@ impl LlmProvider for AnthropicOAuthProvider { async fn complete_with_tools( &self, - req: ToolCompletionRequest, + mut req: ToolCompletionRequest, ) -> Result { - let model = req.model.unwrap_or_else(|| self.active_model_name()); + let model = req.model.take().unwrap_or_else(|| self.active_model_name()); + self.strip_unsupported_tool_params(&mut req); let (system, messages) = convert_messages(req.messages); let tools: Vec = req diff --git a/src/llm/config.rs b/src/llm/config.rs index c36280c2..1902f128 100644 --- a/src/llm/config.rs +++ b/src/llm/config.rs @@ -87,6 +87,10 @@ pub struct RegistryProviderConfig { pub oauth_token: Option, /// Prompt cache retention (Anthropic-specific). pub cache_retention: CacheRetention, + /// Parameter names that this provider does not support (e.g., `["temperature"]`). + /// Supported keys: `"temperature"`, `"max_tokens"`, `"stop_sequences"`. + /// Listed parameters are stripped from requests before sending to avoid 400 errors. + pub unsupported_params: Vec, } /// Configuration for AWS Bedrock (native Converse API). diff --git a/src/llm/mod.rs b/src/llm/mod.rs index ffcc70a6..ebbbd31e 100644 --- a/src/llm/mod.rs +++ b/src/llm/mod.rs @@ -228,7 +228,9 @@ fn create_openai_compat_from_registry( "Using OpenAI-compatible provider" ); - Ok(Arc::new(RigAdapter::new(model, &config.model))) + let adapter = RigAdapter::new(model, &config.model) + .with_unsupported_params(config.unsupported_params.clone()); + Ok(Arc::new(adapter)) } fn create_anthropic_from_registry( @@ -296,7 +298,9 @@ fn create_anthropic_from_registry( ); Ok(Arc::new( - RigAdapter::new(model, &config.model).with_cache_retention(cache_retention), + RigAdapter::new(model, &config.model) + .with_cache_retention(cache_retention) + .with_unsupported_params(config.unsupported_params.clone()), )) } @@ -324,7 +328,9 @@ fn create_ollama_from_registry( "Using Ollama provider" ); - Ok(Arc::new(RigAdapter::new(model, &config.model))) + let adapter = RigAdapter::new(model, &config.model) + .with_unsupported_params(config.unsupported_params.clone()); + Ok(Arc::new(adapter)) } /// Create a cheap/fast LLM provider for lightweight tasks (heartbeat, routing, evaluation). diff --git a/src/llm/registry.rs b/src/llm/registry.rs index 273690a1..36cb7001 100644 --- a/src/llm/registry.rs +++ b/src/llm/registry.rs @@ -152,6 +152,11 @@ pub struct ProviderDefinition { /// Setup wizard hints. #[serde(default)] pub setup: Option, + /// Parameter names that this provider does not support (e.g., `["temperature"]`). + /// Supported keys: `"temperature"`, `"max_tokens"`, `"stop_sequences"`. + /// Listed parameters are stripped from requests before sending to avoid 400 errors. + #[serde(default)] + pub unsupported_params: Vec, } /// Registry of known LLM providers. @@ -378,6 +383,7 @@ mod tests { description: "Custom tinfoil".to_string(), extra_headers_env: None, setup: None, + unsupported_params: vec![], }); let registry = ProviderRegistry::new(all); let tf = registry.find("tinfoil").expect("tinfoil should exist"); @@ -517,6 +523,7 @@ mod tests { description: "No setup".to_string(), extra_headers_env: None, setup: None, // no setup hint + unsupported_params: vec![], }]; let registry = ProviderRegistry::new(providers.clone()); @@ -546,6 +553,7 @@ mod tests { can_list_models: false, models_filter: None, }), + unsupported_params: vec![], }); let registry = ProviderRegistry::new(providers); @@ -587,6 +595,7 @@ mod tests { can_list_models: false, models_filter: None, }), + unsupported_params: vec![], }, // User override removes setup ProviderDefinition { @@ -603,6 +612,7 @@ mod tests { description: "No setup now".to_string(), extra_headers_env: None, setup: None, + unsupported_params: vec![], }, ]; @@ -640,6 +650,7 @@ mod tests { display_name: "A".to_string(), can_list_models: false, }), + unsupported_params: vec![], }, ProviderDefinition { id: "bbb".to_string(), @@ -658,6 +669,7 @@ mod tests { display_name: "B".to_string(), can_list_models: false, }), + unsupported_params: vec![], }, ProviderDefinition { id: "ccc".to_string(), @@ -676,6 +688,7 @@ mod tests { display_name: "C".to_string(), can_list_models: false, }), + unsupported_params: vec![], }, // User override for B ProviderDefinition { @@ -695,6 +708,7 @@ mod tests { display_name: "B".to_string(), can_list_models: false, }), + unsupported_params: vec![], }, ]; @@ -708,6 +722,48 @@ mod tests { ); } + #[test] + fn test_unsupported_params_deserialized() { + let providers: Vec = + serde_json::from_str(include_str!("../../providers.json")).unwrap(); + + // Tinfoil should have temperature in unsupported_params + let tinfoil = providers.iter().find(|p| p.id == "tinfoil").unwrap(); + assert!( + tinfoil + .unsupported_params + .contains(&"temperature".to_string()), + "tinfoil should have 'temperature' in unsupported_params" + ); + + // OpenAI should also have temperature in unsupported_params + let openai = providers.iter().find(|p| p.id == "openai").unwrap(); + assert!( + openai + .unsupported_params + .contains(&"temperature".to_string()), + "openai should have 'temperature' in unsupported_params" + ); + + // Providers without the field in JSON should deserialize to empty vec + let groq = providers.iter().find(|p| p.id == "groq").unwrap(); + assert!( + groq.unsupported_params.is_empty(), + "groq should have empty unsupported_params (field absent in JSON)" + ); + + // Every non-empty entry should contain valid param names + for def in &providers { + for param in &def.unsupported_params { + assert!( + !param.is_empty(), + "{}: unsupported_params contains empty string", + def.id + ); + } + } + } + #[test] fn test_all_builtin_api_key_providers_have_api_key_env() { // Every built-in provider with SetupHint::ApiKey must have api_key_env diff --git a/src/llm/rig_adapter.rs b/src/llm/rig_adapter.rs index 87a0b65c..5b835536 100644 --- a/src/llm/rig_adapter.rs +++ b/src/llm/rig_adapter.rs @@ -42,6 +42,9 @@ pub struct RigAdapter { /// via `additional_params` for Anthropic automatic caching. Also controls /// the cost multiplier for cache-creation tokens. cache_retention: CacheRetention, + /// Parameter names that this provider does not support (e.g., `"temperature"`). + /// These are stripped from requests before sending to avoid 400 errors. + unsupported_params: HashSet, } impl RigAdapter { @@ -56,6 +59,7 @@ impl RigAdapter { input_cost, output_cost, cache_retention: CacheRetention::None, + unsupported_params: HashSet::new(), } } @@ -84,6 +88,44 @@ impl RigAdapter { } self } + + /// Set the list of unsupported parameter names for this provider. + /// + /// Parameters in this set are stripped from requests before sending. + /// Supported parameter names: `"temperature"`, `"max_tokens"`, `"stop_sequences"`. + pub fn with_unsupported_params(mut self, params: Vec) -> Self { + self.unsupported_params = params.into_iter().collect(); + self + } + + /// Strip unsupported fields from a `CompletionRequest` in place. + fn strip_unsupported_completion_params(&self, req: &mut CompletionRequest) { + if self.unsupported_params.is_empty() { + return; + } + if self.unsupported_params.contains("temperature") { + req.temperature = None; + } + if self.unsupported_params.contains("max_tokens") { + req.max_tokens = None; + } + if self.unsupported_params.contains("stop_sequences") { + req.stop_sequences = None; + } + } + + /// Strip unsupported fields from a `ToolCompletionRequest` in place. + fn strip_unsupported_tool_params(&self, req: &mut ToolCompletionRequest) { + if self.unsupported_params.is_empty() { + return; + } + if self.unsupported_params.contains("temperature") { + req.temperature = None; + } + if self.unsupported_params.contains("max_tokens") { + req.max_tokens = None; + } + } } // -- Type conversion helpers -- @@ -539,7 +581,10 @@ where } } - async fn complete(&self, request: CompletionRequest) -> Result { + async fn complete( + &self, + mut request: CompletionRequest, + ) -> Result { if let Some(requested_model) = request.model.as_deref() && requested_model != self.model_name.as_str() { @@ -550,6 +595,8 @@ where ); } + self.strip_unsupported_completion_params(&mut request); + let mut messages = request.messages; crate::llm::provider::sanitize_tool_messages(&mut messages); let (preamble, history) = convert_messages(&messages); @@ -599,7 +646,7 @@ where async fn complete_with_tools( &self, - request: ToolCompletionRequest, + mut request: ToolCompletionRequest, ) -> Result { if let Some(requested_model) = request.model.as_deref() && requested_model != self.model_name.as_str() @@ -611,6 +658,8 @@ where ); } + self.strip_unsupported_tool_params(&mut request); + let known_tool_names: HashSet = request.tools.iter().map(|t| t.name.clone()).collect(); @@ -1156,4 +1205,97 @@ mod tests { assert!(!supports_prompt_cache("gpt-4o")); assert!(!supports_prompt_cache("llama3")); } + + #[test] + fn test_with_unsupported_params_populates_set() { + use rig::client::CompletionClient; + use rig::providers::openai; + + let client: openai::Client = openai::Client::builder() + .api_key("test-key") + .base_url("http://localhost:0") + .build() + .unwrap(); + let client = client.completions_api(); + let model = client.completion_model("test-model"); + let adapter = RigAdapter::new(model, "test-model") + .with_unsupported_params(vec!["temperature".to_string()]); + + assert!(adapter.unsupported_params.contains("temperature")); + assert!(!adapter.unsupported_params.contains("max_tokens")); + } + + #[test] + fn test_strip_unsupported_completion_params() { + use rig::client::CompletionClient; + use rig::providers::openai; + + let client: openai::Client = openai::Client::builder() + .api_key("test-key") + .base_url("http://localhost:0") + .build() + .unwrap(); + let client = client.completions_api(); + let model = client.completion_model("test-model"); + let adapter = RigAdapter::new(model, "test-model").with_unsupported_params(vec![ + "temperature".to_string(), + "stop_sequences".to_string(), + ]); + + let mut req = CompletionRequest::new(vec![ChatMessage::user("hi")]); + req.temperature = Some(0.7); + req.max_tokens = Some(100); + req.stop_sequences = Some(vec!["STOP".to_string()]); + + adapter.strip_unsupported_completion_params(&mut req); + + assert!(req.temperature.is_none(), "temperature should be stripped"); + assert_eq!(req.max_tokens, Some(100), "max_tokens should be preserved"); + assert!( + req.stop_sequences.is_none(), + "stop_sequences should be stripped" + ); + } + + #[test] + fn test_strip_unsupported_tool_params() { + use rig::client::CompletionClient; + use rig::providers::openai; + + let client: openai::Client = openai::Client::builder() + .api_key("test-key") + .base_url("http://localhost:0") + .build() + .unwrap(); + let client = client.completions_api(); + let model = client.completion_model("test-model"); + let adapter = RigAdapter::new(model, "test-model") + .with_unsupported_params(vec!["temperature".to_string(), "max_tokens".to_string()]); + + let mut req = ToolCompletionRequest::new(vec![ChatMessage::user("hi")], vec![]); + req.temperature = Some(0.5); + req.max_tokens = Some(200); + + adapter.strip_unsupported_tool_params(&mut req); + + assert!(req.temperature.is_none(), "temperature should be stripped"); + assert!(req.max_tokens.is_none(), "max_tokens should be stripped"); + } + + #[test] + fn test_unsupported_params_empty_by_default() { + use rig::client::CompletionClient; + use rig::providers::openai; + + let client: openai::Client = openai::Client::builder() + .api_key("test-key") + .base_url("http://localhost:0") + .build() + .unwrap(); + let client = client.completions_api(); + let model = client.completion_model("test-model"); + let adapter = RigAdapter::new(model, "test-model"); + + assert!(adapter.unsupported_params.is_empty()); + } } diff --git a/src/setup/wizard.rs b/src/setup/wizard.rs index 38a5d46d..86142bff 100644 --- a/src/setup/wizard.rs +++ b/src/setup/wizard.rs @@ -3787,6 +3787,7 @@ mod tests { description: "Custom provider with no setup wizard".to_string(), extra_headers_env: None, setup: None, + unsupported_params: vec![], }); let registry = crate::llm::ProviderRegistry::new(providers);