diff --git a/.github/labeler.yml b/.github/labeler.yml index fd7da0be2fd..6ac08552b00 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -1,5 +1,5 @@ -# Scope labels for actions/labeler@v6 -# Maps file path globs to scope labels. Multiple labels can apply per PR. +# Labels for actions/labeler@v6 +# Maps file path globs to labels. Multiple labels can apply per PR. "scope: agent": - changed-files: @@ -164,3 +164,9 @@ - any-glob-to-any-file: - Cargo.toml - Cargo.lock + +"DB MIGRATION": + - changed-files: + - any-glob-to-any-file: + - migrations/** + - src/db/libsql_migrations.rs diff --git a/.github/scripts/create-labels.sh b/.github/scripts/create-labels.sh index 66f07ea9ce1..6b6d10d3cd1 100755 --- a/.github/scripts/create-labels.sh +++ b/.github/scripts/create-labels.sh @@ -62,6 +62,9 @@ create "scope: ci" "546E7A" "CI/CD workflows" create "scope: docs" "78909C" "Documentation" create "scope: dependencies" "90A4AE" "Dependency updates" +echo "==> Creating coordination labels..." +create "DB MIGRATION" "C62828" "PR adds or modifies PostgreSQL or libSQL migration definitions" + echo "==> Creating workflow labels..." create "skip-regression-check" "9E9E9E" "Acknowledged: fix without regression test" diff --git a/.github/workflows/pr-label-scope.yml b/.github/workflows/pr-label-scope.yml index c798f09bce6..b8a282472ba 100644 --- a/.github/workflows/pr-label-scope.yml +++ b/.github/workflows/pr-label-scope.yml @@ -6,12 +6,19 @@ on: permissions: contents: read + issues: write pull-requests: write jobs: scope: runs-on: ubuntu-latest steps: + - name: Ensure DB MIGRATION label exists + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + run: gh label create "DB MIGRATION" --repo "$REPO" --color C62828 --description "PR adds or modifies PostgreSQL or libSQL migration definitions" --force + - uses: actions/labeler@8558fd74291d67161a8a78ce36a881fa63b766a9 # v5 with: configuration-path: .github/labeler.yml diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b237d28665d..87563f8da2d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -470,6 +470,17 @@ jobs: # shellcheck disable=SC2086 # PRERELEASE_FLAG is '--prerelease' or empty gh release create "$RELEASE_TAG" --target "$RELEASE_COMMIT" $PRERELEASE_FLAG --title "$ANNOUNCEMENT_TITLE" --notes-file "$RUNNER_TEMP/notes.txt" artifacts/* + # Build and push Docker Hub images (:version, :latest, :sha-*) after the GitHub Release exists. + docker-image: + needs: host + if: ${{ always() && needs.host.result == 'success' }} + permissions: + contents: read + packages: read + actions: write + uses: ./.github/workflows/docker.yml + secrets: inherit + # Commit patched manifest SHA256 checksums back to main so the repo # stays in sync with the released artifacts. update-registry-checksums: diff --git a/channels-src/telegram/src/lib.rs b/channels-src/telegram/src/lib.rs index 238b458b47c..c66c55d1300 100644 --- a/channels-src/telegram/src/lib.rs +++ b/channels-src/telegram/src/lib.rs @@ -376,6 +376,26 @@ const TELEGRAM_STATUS_MAX_CHARS: usize = 600; /// Telegram's hard limit for message text length. const TELEGRAM_MAX_MESSAGE_LEN: usize = 4096; +fn utf16_code_unit_len(text: &str) -> usize { + text.encode_utf16().count() +} + +fn prefix_within_utf16_limit(text: &str, max_units: usize) -> usize { + let mut units = 0; + let mut end = 0; + + for (byte_idx, ch) in text.char_indices() { + let ch_units = ch.len_utf16(); + if units + ch_units > max_units { + break; + } + units += ch_units; + end = byte_idx + ch.len_utf8(); + } + + end +} + fn truncate_status_message(input: &str, max_chars: usize) -> String { let mut iter = input.chars(); let truncated: String = iter.by_ref().take(max_chars).collect(); @@ -386,7 +406,7 @@ fn truncate_status_message(input: &str, max_chars: usize) -> String { } } -/// Split a long message into chunks that fit within Telegram's 4096-char limit. +/// Split a long message into chunks that fit within Telegram's 4096 UTF-16-unit limit. /// /// Tries to split at the most natural boundary available (in priority order): /// 1. Double newline (paragraph break) @@ -395,7 +415,7 @@ fn truncate_status_message(input: &str, max_chars: usize) -> String { /// 4. Word boundary (space) /// 5. Hard cut at the limit (last resort for pathological input) fn split_message(text: &str) -> Vec { - if text.chars().count() <= TELEGRAM_MAX_MESSAGE_LEN { + if utf16_code_unit_len(text) <= TELEGRAM_MAX_MESSAGE_LEN { return vec![text.to_string()]; } @@ -403,13 +423,8 @@ fn split_message(text: &str) -> Vec { let mut remaining = text; while !remaining.is_empty() { - // Count chars to find the byte offset for our window. - let window_bytes = remaining - .char_indices() - .take(TELEGRAM_MAX_MESSAGE_LEN) - .last() - .map(|(byte_idx, ch)| byte_idx + ch.len_utf8()) - .unwrap_or(remaining.len()); + // Find the longest UTF-8 prefix that fits within Telegram's UTF-16 limit. + let window_bytes = prefix_within_utf16_limit(remaining, TELEGRAM_MAX_MESSAGE_LEN); if window_bytes >= remaining.len() { // Remainder fits entirely. @@ -417,6 +432,19 @@ fn split_message(text: &str) -> Vec { break; } + if window_bytes == 0 { + // Defensive fallback: make progress even if a future caller uses a + // smaller limit than a single scalar value can fit within. + let first_char_len = remaining + .chars() + .next() + .map(|ch| ch.len_utf8()) + .unwrap_or(remaining.len()); + chunks.push(remaining[..first_char_len].to_string()); + remaining = &remaining[first_char_len..]; + continue; + } + let window = &remaining[..window_bytes]; // 1. Double newline โ€” best paragraph boundary @@ -2268,6 +2296,10 @@ export!(TelegramChannel); mod tests { use super::*; + fn utf16_len(text: &str) -> usize { + text.encode_utf16().count() + } + #[test] fn test_split_message_short() { let text = "Hello, world!"; @@ -2295,7 +2327,7 @@ mod tests { let chunks = split_message(&text); assert!(chunks.len() > 1, "expected multiple chunks"); for chunk in &chunks { - assert!(chunk.chars().count() <= TELEGRAM_MAX_MESSAGE_LEN); + assert!(utf16_len(chunk) <= TELEGRAM_MAX_MESSAGE_LEN); } // Rejoined chunks must equal the original text exactly. let rejoined = chunks.join(" "); @@ -2311,7 +2343,7 @@ mod tests { assert!(text.len() > TELEGRAM_MAX_MESSAGE_LEN); let chunks = split_message(&text); for chunk in &chunks { - assert!(chunk.chars().count() <= TELEGRAM_MAX_MESSAGE_LEN); + assert!(utf16_len(chunk) <= TELEGRAM_MAX_MESSAGE_LEN); } } @@ -2341,7 +2373,7 @@ mod tests { let chunks = split_message(&text); assert!(chunks.len() >= 2); for chunk in &chunks { - assert!(chunk.chars().count() <= TELEGRAM_MAX_MESSAGE_LEN); + assert!(utf16_len(chunk) <= TELEGRAM_MAX_MESSAGE_LEN); } // Rejoined must preserve all characters let rejoined: String = chunks.concat(); @@ -2358,12 +2390,25 @@ mod tests { let chunks = split_message(&text); assert!(chunks.len() >= 2); for chunk in &chunks { - assert!(chunk.chars().count() <= TELEGRAM_MAX_MESSAGE_LEN); + assert!(utf16_len(chunk) <= TELEGRAM_MAX_MESSAGE_LEN); // Every char should be a complete emoji assert!(chunk.chars().all(|c| c == '\u{1F600}')); } } + #[test] + fn test_split_message_exact_utf16_limit_for_surrogate_pairs() { + let emoji = "\u{1F600}"; // ๐Ÿ˜€ + let text = emoji.repeat(TELEGRAM_MAX_MESSAGE_LEN); + + let chunks = split_message(&text); + + assert_eq!(chunks.len(), 2); + assert!(chunks + .iter() + .all(|chunk| utf16_len(chunk) <= TELEGRAM_MAX_MESSAGE_LEN)); + } + #[test] fn test_clean_message_text() { // Without bot_username: strips any leading @mention diff --git a/crates/ironclaw_gateway/static/app.js b/crates/ironclaw_gateway/static/app.js index 0b8662d4908..30e5b8ed081 100644 --- a/crates/ironclaw_gateway/static/app.js +++ b/crates/ironclaw_gateway/static/app.js @@ -7084,6 +7084,12 @@ function loadSettingsSubtab(subtab) { // --- Structured Settings Definitions --- var INFERENCE_SETTINGS = [ + { + group: 'cfg.group.inference', + settings: [ + { key: 'temperature', label: 'cfg.temperature.label', description: 'cfg.temperature.desc', type: 'float', min: 0, max: 2, step: 0.1 }, + ] + }, { group: 'cfg.group.embeddings', settings: [ @@ -7429,25 +7435,25 @@ function renderStructuredSettingsRow(def, value, activeValue) { return function() { saveSetting(k, el.value === '' ? null : el.value); }; })(def.key, sel)); inputWrap.appendChild(sel); - } else if (def.type === 'number') { + } else if (def.type === 'number' || def.type === 'float') { var numInp = document.createElement('input'); numInp.type = 'number'; - numInp.step = '1'; + numInp.step = def.step !== undefined ? String(def.step) : (def.type === 'float' ? 'any' : '1'); numInp.className = 'settings-input'; numInp.setAttribute('aria-label', ariaLabel); numInp.value = (value === null || value === undefined) ? '' : value; if (!value && value !== 0) numInp.placeholder = placeholderText; if (def.min !== undefined) numInp.min = def.min; if (def.max !== undefined) numInp.max = def.max; - numInp.addEventListener('change', (function(k, el) { + numInp.addEventListener('change', (function(k, el, isFloat) { return function() { if (el.value === '') return saveSetting(k, null); - var parsed = parseInt(el.value, 10); + var parsed = isFloat ? parseFloat(el.value) : parseInt(el.value, 10); if (isNaN(parsed)) return; el.value = parsed; saveSetting(k, parsed); }; - })(def.key, numInp)); + })(def.key, numInp, def.type === 'float')); inputWrap.appendChild(numInp); } else if (def.type === 'list') { var listInp = document.createElement('input'); diff --git a/crates/ironclaw_gateway/static/i18n/en.js b/crates/ironclaw_gateway/static/i18n/en.js index c8fdd9b9d25..2b5a06a29cd 100644 --- a/crates/ironclaw_gateway/static/i18n/en.js +++ b/crates/ironclaw_gateway/static/i18n/en.js @@ -521,6 +521,9 @@ I18n.register('en', { 'cfg.llm_backend.desc': 'LLM inference provider', 'cfg.selected_model.label': 'Model', 'cfg.selected_model.desc': 'Model name or ID for the selected backend', + 'cfg.temperature.label': 'Temperature', + 'cfg.temperature.desc': 'Default sampling temperature (0.0โ€“2.0). Lower = more deterministic, higher = more creative', + 'cfg.group.inference': 'Inference', 'cfg.ollama_base_url.label': 'Ollama URL', 'cfg.ollama_base_url.desc': 'Base URL for Ollama API', 'cfg.openai_compatible_base_url.label': 'OpenAI-compatible URL', diff --git a/crates/ironclaw_gateway/static/i18n/ko.js b/crates/ironclaw_gateway/static/i18n/ko.js index e70e4cd0d83..adfc09ac929 100644 --- a/crates/ironclaw_gateway/static/i18n/ko.js +++ b/crates/ironclaw_gateway/static/i18n/ko.js @@ -520,6 +520,9 @@ I18n.register('ko', { 'cfg.llm_backend.desc': 'LLM ์ถ”๋ก  ๊ณต๊ธ‰์ž', 'cfg.selected_model.label': '๋ชจ๋ธ', 'cfg.selected_model.desc': '์„ ํƒํ•œ ๋ฐฑ์—”๋“œ์˜ ๋ชจ๋ธ ์ด๋ฆ„ ๋˜๋Š” ID', + 'cfg.temperature.label': '์˜จ๋„', + 'cfg.temperature.desc': '๊ธฐ๋ณธ ์ƒ˜ํ”Œ๋ง ์˜จ๋„ (0.0โ€“2.0). ๋‚ฎ์„์ˆ˜๋ก ๊ฒฐ์ •์ , ๋†’์„์ˆ˜๋ก ์ฐฝ์˜์ ', + 'cfg.group.inference': '์ถ”๋ก ', 'cfg.ollama_base_url.label': 'Ollama URL', 'cfg.ollama_base_url.desc': 'Ollama API์˜ ๋ฒ ์ด์Šค URL', 'cfg.openai_compatible_base_url.label': 'OpenAI ํ˜ธํ™˜ URL', diff --git a/crates/ironclaw_gateway/static/i18n/zh-CN.js b/crates/ironclaw_gateway/static/i18n/zh-CN.js index e3dd19f82a1..b4250a1486a 100644 --- a/crates/ironclaw_gateway/static/i18n/zh-CN.js +++ b/crates/ironclaw_gateway/static/i18n/zh-CN.js @@ -520,6 +520,9 @@ I18n.register('zh-CN', { 'cfg.llm_backend.desc': 'LLM ๆŽจ็†ๆไพ›ๅ•†', 'cfg.selected_model.label': 'ๆจกๅž‹', 'cfg.selected_model.desc': 'ๆ‰€้€‰ๅŽ็ซฏ็š„ๆจกๅž‹ๅ็งฐๆˆ– ID', + 'cfg.temperature.label': 'ๆธฉๅบฆ', + 'cfg.temperature.desc': '้ป˜่ฎค้‡‡ๆ ทๆธฉๅบฆ๏ผˆ0.0โ€“2.0๏ผ‰ใ€‚่ถŠไฝŽ่ถŠ็กฎๅฎšๆ€ง๏ผŒ่ถŠ้ซ˜่ถŠๆœ‰ๅˆ›ๆ„', + 'cfg.group.inference': 'ๆŽจ็†', 'cfg.ollama_base_url.label': 'Ollama URL', 'cfg.ollama_base_url.desc': 'Ollama API ๅŸบ็ก€ URL', 'cfg.openai_compatible_base_url.label': 'OpenAI ๅ…ผๅฎน URL', diff --git a/src/agent/dispatcher.rs b/src/agent/dispatcher.rs index 875a9bb70e3..5f34abeec3d 100644 --- a/src/agent/dispatcher.rs +++ b/src/agent/dispatcher.rs @@ -27,6 +27,24 @@ fn selected_model_override(value: &serde_json::Value) -> Option { crate::llm::normalized_model_override(value.as_str()).map(str::to_string) } +/// Decide whether a settings-derived temperature should override the +/// per-request value already on the reasoning context. +/// +/// Returns `Some(new_value)` only when there is no per-request value yet +/// AND the settings value parses as a number. The result is clamped to the +/// supported `[0.0, 2.0]` range to guard against bad DB values. +fn resolve_settings_temperature( + current: Option, + settings_value: Option<&serde_json::Value>, +) -> Option { + if current.is_some() { + return None; + } + settings_value + .and_then(|v| v.as_f64()) + .map(|t| (t as f32).clamp(0.0, 2.0)) +} + /// Result of the agentic loop execution. pub(super) enum AgenticLoopResult { /// Completed with a response. @@ -583,16 +601,31 @@ impl<'a> LoopDelegate for ChatDelegate<'a> { .into()); } - // Apply per-user model override from settings (first iteration only + // Apply per-user overrides from settings (first iteration only // to avoid repeated DB lookups within the same agentic loop). - // Uses "selected_model" โ€” the same key the /model command persists to - // via SettingsStore (per-user scoped via TenantScope). + // Uses admin-fallback so admin-set defaults propagate to members + // who haven't overridden the value themselves. if iteration == 0 && let Some(store) = self.tenant.store() - && let Ok(Some(value)) = store.get_setting("selected_model").await - && let Some(model) = selected_model_override(&value) { - reason_ctx.model_override = Some(model); + // Model override: "selected_model" โ€” the same key the /model command + // persists to via SettingsStore (per-user scoped via TenantScope). + if let Ok(Some(value)) = store + .get_setting_with_admin_fallback("selected_model") + .await + && let Some(model) = selected_model_override(&value) + { + reason_ctx.model_override = Some(model); + } + + // Temperature override from user or admin settings. Per-request + // values already on the context take precedence over settings. + if let Ok(setting) = store.get_setting_with_admin_fallback("temperature").await + && let Some(t) = + resolve_settings_temperature(reason_ctx.temperature, setting.as_ref()) + { + reason_ctx.temperature = Some(t); + } } let output = match reasoning.respond_with_tools(reason_ctx).await { @@ -1693,7 +1726,8 @@ mod tests { use super::{ capture_auth_prompt, check_auth_required, extract_auth_prompt, parse_auth_result, - persist_selected_auth_prompt, restore_selected_auth_prompt, selected_model_override, + persist_selected_auth_prompt, resolve_settings_temperature, restore_selected_auth_prompt, + selected_model_override, }; use crate::agent::session::PendingAuthPrompt; @@ -3041,6 +3075,47 @@ mod tests { } } + #[test] + fn resolve_settings_temperature_keeps_per_request_value() { + // Regression: a per-request temperature already on the context must + // win over a settings-derived value, otherwise API callers cannot + // override the user/admin default for a single call. + assert_eq!( + resolve_settings_temperature(Some(0.42), Some(&serde_json::json!(1.5))), + None, + "must not return Some when current is set" + ); + } + + #[test] + fn resolve_settings_temperature_uses_settings_when_unset() { + assert_eq!( + resolve_settings_temperature(None, Some(&serde_json::json!(0.9))), + Some(0.9), + ); + } + + #[test] + fn resolve_settings_temperature_clamps_out_of_range_db_value() { + assert_eq!( + resolve_settings_temperature(None, Some(&serde_json::json!(9.0))), + Some(2.0), + ); + assert_eq!( + resolve_settings_temperature(None, Some(&serde_json::json!(-1.0))), + Some(0.0), + ); + } + + #[test] + fn resolve_settings_temperature_returns_none_when_settings_missing() { + assert_eq!(resolve_settings_temperature(None, None), None); + assert_eq!( + resolve_settings_temperature(None, Some(&serde_json::json!("not-a-number"))), + None, + ); + } + #[test] fn selected_model_override_ignores_default_sentinel() { assert_eq!(selected_model_override(&serde_json::json!("default")), None); diff --git a/src/channels/web/handlers/settings.rs b/src/channels/web/handlers/settings.rs index 9f022524455..b7a5ea0c9eb 100644 --- a/src/channels/web/handlers/settings.rs +++ b/src/channels/web/handlers/settings.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use axum::{ Json, - extract::{Path, State}, + extract::{Path, Query, State}, http::StatusCode, }; use secrecy::SecretString; @@ -17,6 +17,31 @@ use crate::secrets::{CreateSecretParams, SecretsStore}; /// Sentinel value the frontend sends to mean "key is unchanged, don't touch it". const API_KEY_UNCHANGED: &str = "โ€ขโ€ขโ€ขโ€ขโ€ขโ€ขโ€ขโ€ข"; +/// Resolve the effective user_id for a settings operation. +/// +/// When `scope=admin`, the operation targets the shared admin-default scope +/// (`__admin__`). Only admin users may use this scope; non-admins get 403. +/// Without the scope parameter (or any other value), operations target the +/// calling user's own settings. +fn resolve_settings_scope( + user: &crate::channels::web::auth::UserIdentity, + query: &SettingScopeQuery, +) -> Result { + if query.scope.as_deref() == Some("admin") { + if user.role != "admin" { + tracing::warn!( + user_id = %user.user_id, + role = %user.role, + "Non-admin attempted to use scope=admin on settings endpoint" + ); + return Err(StatusCode::FORBIDDEN); + } + Ok(crate::tools::permissions::ADMIN_SETTINGS_USER_ID.to_string()) + } else { + Ok(user.user_id.clone()) + } +} + pub async fn settings_list_handler( State(state): State>, AuthenticatedUser(user): AuthenticatedUser, @@ -68,13 +93,16 @@ pub async fn settings_get_handler( State(state): State>, AuthenticatedUser(user): AuthenticatedUser, Path(key): Path, + Query(query): Query, ) -> Result, StatusCode> { + let effective_user_id = resolve_settings_scope(&user, &query)?; + let store = state .store .as_ref() .ok_or(StatusCode::SERVICE_UNAVAILABLE)?; let row = store - .get_setting_full(&user.user_id, &key) + .get_setting_full(&effective_user_id, &key) .await .map_err(|e| { tracing::error!("Failed to get setting '{}': {}", key, e); @@ -88,7 +116,7 @@ pub async fn settings_get_handler( "llm_builtin_overrides" | "llm_custom_providers" ) { let mut map = std::collections::HashMap::from([(key.clone(), row.value.clone())]); - annotate_secret_key_presence(&state, &user.user_id, &mut map).await; + annotate_secret_key_presence(&state, &effective_user_id, &mut map).await; mask_settings_api_keys(&mut map); map.remove(&key).unwrap_or(row.value) } else { @@ -106,8 +134,10 @@ pub async fn settings_set_handler( State(state): State>, AuthenticatedUser(user): AuthenticatedUser, Path(key): Path, + Query(query): Query, Json(body): Json, ) -> Result { + let effective_user_id = resolve_settings_scope(&user, &query)?; ensure_setting_write_allowed(&user, &key)?; let store = state @@ -117,7 +147,7 @@ pub async fn settings_set_handler( // Guard: cannot remove a custom provider that is currently active. if key == "llm_custom_providers" { - guard_active_provider_not_removed(store, &user.user_id, &body.value).await?; + guard_active_provider_not_removed(store, &effective_user_id, &body.value).await?; validate_custom_providers(&body.value)?; } @@ -125,16 +155,16 @@ pub async fn settings_set_handler( // The sanitized value has api_key fields removed (stored encrypted instead). let sanitized_value = match key.as_str() { "llm_builtin_overrides" => { - extract_builtin_override_keys(&state, &user.user_id, &body.value).await? + extract_builtin_override_keys(&state, &effective_user_id, &body.value).await? } "llm_custom_providers" => { - extract_custom_provider_keys(&state, &user.user_id, &body.value).await? + extract_custom_provider_keys(&state, &effective_user_id, &body.value).await? } _ => body.value.clone(), }; store - .set_setting(&user.user_id, &key, &sanitized_value) + .set_setting(&effective_user_id, &key, &sanitized_value) .await .map_err(|e| { tracing::error!("Failed to set setting '{}': {}", key, e); @@ -241,7 +271,9 @@ pub async fn settings_delete_handler( State(state): State>, AuthenticatedUser(user): AuthenticatedUser, Path(key): Path, + Query(query): Query, ) -> Result { + let effective_user_id = resolve_settings_scope(&user, &query)?; ensure_setting_write_allowed(&user, &key)?; let store = state @@ -252,12 +284,16 @@ pub async fn settings_delete_handler( // Guard: deleting llm_custom_providers is equivalent to setting it to []. // Reject if the active backend is a custom provider that would be removed. if key == "llm_custom_providers" { - guard_active_provider_not_removed(store, &user.user_id, &serde_json::Value::Array(vec![])) - .await?; + guard_active_provider_not_removed( + store, + &effective_user_id, + &serde_json::Value::Array(vec![]), + ) + .await?; } store - .delete_setting(&user.user_id, &key) + .delete_setting(&effective_user_id, &key) .await .map_err(|e| { tracing::error!("Failed to delete setting '{}': {}", key, e); @@ -1217,6 +1253,7 @@ mod tests { workspace_read_scopes: Vec::new(), }), Path("ollama_base_url".to_string()), + Query(SettingScopeQuery::default()), Json(SettingWriteRequest { value: serde_json::json!("http://192.168.1.50:11434"), }), @@ -1240,6 +1277,7 @@ mod tests { workspace_read_scopes: Vec::new(), }), Path("llm_custom_providers".to_string()), + Query(SettingScopeQuery::default()), ) .await .unwrap_err(); diff --git a/src/channels/web/responses_api.rs b/src/channels/web/responses_api.rs index 8c6c2bd5a7d..caf4bbb2fe1 100644 --- a/src/channels/web/responses_api.rs +++ b/src/channels/web/responses_api.rs @@ -610,7 +610,7 @@ pub async fn create_response_handler( if req.temperature.is_some() { return Err(api_error( StatusCode::BAD_REQUEST, - "The 'temperature' field is not yet supported", + "Per-request 'temperature' is not supported on this endpoint; configure the default via settings", "invalid_request_error", )); } diff --git a/src/channels/web/types.rs b/src/channels/web/types.rs index 19cf317f21a..610fc8729f1 100644 --- a/src/channels/web/types.rs +++ b/src/channels/web/types.rs @@ -911,6 +911,14 @@ pub struct SettingWriteRequest { pub value: serde_json::Value, } +/// Query parameters for settings endpoints. +/// `?scope=admin` writes to / reads from the admin-default scope. +#[derive(Debug, Default, Deserialize)] +pub struct SettingScopeQuery { + #[serde(default)] + pub scope: Option, +} + #[derive(Debug, Deserialize)] pub struct SettingsImportRequest { pub settings: std::collections::HashMap, diff --git a/src/config/channels.rs b/src/config/channels.rs index a1ecce402cc..a6c3a70e365 100644 --- a/src/config/channels.rs +++ b/src/config/channels.rs @@ -369,7 +369,8 @@ impl ChannelsConfig { }; let cli_enabled = db_first_bool(cs.cli_enabled, defaults.cli_enabled, "CLI_ENABLED")?; - let cli_mode = db_first_optional_string(&cs.cli_mode, "CLI_MODE")?.unwrap_or_default(); + let cli_mode = db_first_optional_string(&cs.cli_mode, "CLI_MODE")? + .unwrap_or_else(|| "tui".to_string()); let tui = if cli_mode.eq_ignore_ascii_case("tui") { Some(TuiChannelConfig { theme: optional_env("TUI_THEME")?.unwrap_or_else(|| "dark".to_string()), diff --git a/src/config/mod.rs b/src/config/mod.rs index 76383f34387..6efbd45c7f5 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -280,12 +280,32 @@ impl Config { let _ = dotenvy::dotenv(); crate::bootstrap::load_ironclaw_env(); - // Start with defaults, apply deployment profile, then TOML overlay. + // Resolution layers (lowest -> highest priority): + // defaults -> deployment profile -> TOML -> admin DB -> per-user DB let mut settings = Settings::default(); profile::apply_profile(&mut settings)?; Self::apply_toml_overlay(&mut settings, toml_path)?; - // Overlay DB settings on top so DB values win over TOML. + // Layer admin-scope defaults between TOML and per-user settings. + // This lets an admin set instance-wide defaults (e.g. temperature, + // model) that members inherit unless they override per-user. + // Skip if the user IS the admin scope to avoid a redundant merge. + let admin_scope = crate::tools::permissions::ADMIN_SETTINGS_USER_ID; + if user_id != admin_scope + && let Ok(mut admin_map) = store.get_all_settings(admin_scope).await + && !admin_map.is_empty() + { + // Defense-in-depth: even though the admin-scope map is written + // by an operator, never let admin-only LLM endpoint settings + // (private/loopback URLs) propagate down to non-operators. + if !is_operator { + crate::config::helpers::strip_admin_only_llm_keys(&mut admin_map); + } + let admin_settings = Settings::from_db_map(&admin_map); + settings.merge_from(&admin_settings); + } + + // Overlay per-user DB settings on top (highest priority). match store.get_all_settings(user_id).await { Ok(mut map) => { if !is_operator { @@ -392,10 +412,21 @@ impl Config { is_operator: bool, ) -> Result<(), ConfigError> { let mut settings = if let Some(store) = store { - // Profile as base, then TOML, then DB on top (DB wins). + // Resolution layers: profile -> TOML -> admin DB -> per-user DB. let mut s = Settings::default(); profile::apply_profile(&mut s)?; Self::apply_toml_overlay(&mut s, toml_path)?; + let admin_scope = crate::tools::permissions::ADMIN_SETTINGS_USER_ID; + if user_id != admin_scope + && let Ok(mut admin_map) = store.get_all_settings(admin_scope).await + && !admin_map.is_empty() + { + if !is_operator { + crate::config::helpers::strip_admin_only_llm_keys(&mut admin_map); + } + let admin_settings = Settings::from_db_map(&admin_map); + s.merge_from(&admin_settings); + } if let Ok(mut map) = store.get_all_settings(user_id).await { if !is_operator { crate::config::helpers::strip_admin_only_llm_keys(&mut map); @@ -1064,6 +1095,78 @@ mod tests { .expect("resolve should succeed for operator"); } + #[tokio::test] + async fn re_resolve_llm_strips_admin_scope_admin_only_keys_for_non_operator() { + // Regression: a non-operator member must not inherit admin-only LLM + // keys from the admin-defaults scope, even when the admin scope was + // populated by an actual operator. The poisoned model below would + // propagate to `cfg.llm.nearai.model` if the strip filter was not + // applied to the admin-scope merge inside `re_resolve_llm_with_secrets`. + let store = FakeSettingsStore::new(); + store + .seed( + crate::tools::permissions::ADMIN_SETTINGS_USER_ID, + "llm_builtin_overrides", + serde_json::json!({ + "nearai": { + "model": "admin-poison-model" + } + }), + ) + .await; + + let mut cfg = config_for_owner("operator-user"); + cfg.re_resolve_llm_with_secrets( + Some(&store as &(dyn crate::db::SettingsStore + Sync)), + "member-user", + None, + None, + false, + ) + .await + .expect("resolve should succeed for non-operator member"); + + assert_ne!( + cfg.llm.nearai.model, "admin-poison-model", + "admin-scope llm_builtin_overrides must not propagate to a non-operator member" + ); + } + + #[tokio::test] + async fn re_resolve_llm_keeps_admin_scope_admin_only_keys_for_operator() { + // Mirror of the above: an operator may legitimately inherit admin + // defaults, including admin-only LLM keys, since they could set them + // themselves directly. + let store = FakeSettingsStore::new(); + store + .seed( + crate::tools::permissions::ADMIN_SETTINGS_USER_ID, + "llm_builtin_overrides", + serde_json::json!({ + "nearai": { + "model": "admin-set-model" + } + }), + ) + .await; + + let mut cfg = config_for_owner("operator-user"); + cfg.re_resolve_llm_with_secrets( + Some(&store as &(dyn crate::db::SettingsStore + Sync)), + "another-operator", + None, + None, + true, + ) + .await + .expect("resolve should succeed for operator"); + + assert_eq!( + cfg.llm.nearai.model, "admin-set-model", + "operator must inherit admin-scope builtin override model" + ); + } + #[tokio::test] async fn hydrate_skips_when_key_already_present() { let secrets = test_secrets_store(); diff --git a/src/llm/reasoning.rs b/src/llm/reasoning.rs index e3c3a54264b..497300b5b85 100644 --- a/src/llm/reasoning.rs +++ b/src/llm/reasoning.rs @@ -212,6 +212,10 @@ pub struct ReasoningContext { /// instead of the provider's default. Only effective with providers that /// support per-request model overrides (e.g. NearAI). pub model_override: Option, + /// User-configured default temperature. When set, overrides the hardcoded + /// 0.7 default in `respond_with_tools`. Per-request temperature from API + /// callers takes precedence over this. + pub temperature: Option, } impl ReasoningContext { @@ -226,6 +230,7 @@ impl ReasoningContext { force_text: false, system_prompt: None, model_override: None, + temperature: None, } } @@ -265,6 +270,12 @@ impl ReasoningContext { self.metadata = metadata; self } + + /// Set default temperature for LLM requests. + pub fn with_temperature(mut self, temperature: f32) -> Self { + self.temperature = Some(temperature); + self + } } impl Default for ReasoningContext { @@ -727,11 +738,16 @@ Respond in JSON format: context.available_tools.clone() }; + // Clamp to the provider-supported range. The frontend enforces this + // too, but a bad DB value or per-request override must not reach the + // provider โ€” some backends reject out-of-range temperatures outright. + let temperature = context.temperature.unwrap_or(0.7).clamp(0.0, 2.0); + // If we have tools, use tool completion mode if !effective_tools.is_empty() { let mut request = ToolCompletionRequest::new(messages, effective_tools) .with_max_tokens(4096) - .with_temperature(0.7) + .with_temperature(temperature) .with_tool_choice("auto"); request.metadata = context.metadata.clone(); if let Some(ref model) = context.model_override { @@ -848,7 +864,7 @@ Respond in JSON format: // No tools, use simple completion let mut request = CompletionRequest::new(messages) .with_max_tokens(4096) - .with_temperature(0.7); + .with_temperature(temperature); request.metadata = context.metadata.clone(); if let Some(ref model) = context.model_override { request.model = Some(model.clone()); diff --git a/src/settings.rs b/src/settings.rs index 75d42f6be6b..da246b943c9 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -150,6 +150,12 @@ pub struct Settings { #[serde(default)] pub selected_model: Option, + /// Default sampling temperature for LLM requests (0.0โ€“2.0). + /// When set, used as the default for conversational turns. + /// Per-request temperature (e.g. from the API) takes precedence. + #[serde(default)] + pub temperature: Option, + // === Step 5: Embeddings === /// Embeddings configuration. #[serde(default)] @@ -446,7 +452,7 @@ impl Default for ChannelSettings { wasm_channels: Vec::new(), wasm_channels_enabled: true, wasm_channels_dir: None, - cli_mode: None, + cli_mode: Some("tui".to_string()), } } } diff --git a/src/tenant.rs b/src/tenant.rs index cbea00f29ee..df30c71113f 100644 --- a/src/tenant.rs +++ b/src/tenant.rs @@ -257,6 +257,27 @@ impl TenantScope { .await } + /// Like `get_setting`, but falls back to the admin scope (`__admin__`) + /// when the per-user value is absent. Use this for settings where an + /// admin should be able to set an instance-wide default that members + /// inherit unless they override it themselves. + pub async fn get_setting_with_admin_fallback( + &self, + key: &str, + ) -> Result, DatabaseError> { + if let Some(value) = self + .inner + .get_setting(self.identity.owner_id.as_str(), key) + .await? + { + return Ok(Some(value)); + } + // Fall back to admin scope. + self.inner + .get_setting(crate::tools::permissions::ADMIN_SETTINGS_USER_ID, key) + .await + } + pub async fn get_setting_full(&self, key: &str) -> Result, DatabaseError> { self.inner .get_setting_full(self.identity.owner_id.as_str(), key) @@ -1168,6 +1189,63 @@ mod tests { assert_eq!(scope.user_id(), "admin-user"); } + // ---- get_setting_with_admin_fallback tests ---- + + #[tokio::test] + async fn test_admin_fallback_returns_user_value_when_set() { + let (db, _tmp) = crate::testing::test_db().await; + // Set both admin and user values. + db.set_setting( + crate::tools::permissions::ADMIN_SETTINGS_USER_ID, + "temperature", + &serde_json::json!(0.3), + ) + .await + .unwrap(); + db.set_setting("alice", "temperature", &serde_json::json!(0.9)) + .await + .unwrap(); + + let scope = TenantScope::new("alice", db); + let value = scope + .get_setting_with_admin_fallback("temperature") + .await + .unwrap(); + // User value wins. + assert_eq!(value, Some(serde_json::json!(0.9))); + } + + #[tokio::test] + async fn test_admin_fallback_returns_admin_value_when_user_unset() { + let (db, _tmp) = crate::testing::test_db().await; + db.set_setting( + crate::tools::permissions::ADMIN_SETTINGS_USER_ID, + "temperature", + &serde_json::json!(0.5), + ) + .await + .unwrap(); + + let scope = TenantScope::new("alice", db); + let value = scope + .get_setting_with_admin_fallback("temperature") + .await + .unwrap(); + // Falls back to admin value. + assert_eq!(value, Some(serde_json::json!(0.5))); + } + + #[tokio::test] + async fn test_admin_fallback_returns_none_when_neither_set() { + let (db, _tmp) = crate::testing::test_db().await; + let scope = TenantScope::new("alice", db); + let value = scope + .get_setting_with_admin_fallback("temperature") + .await + .unwrap(); + assert_eq!(value, None); + } + // ---- TenantRateRegistry tests ---- #[tokio::test]