diff --git a/docs/configuration.md b/docs/configuration.md index 080d7aa..1a787fa 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1,12 +1,13 @@ # Configuration -Postil resolves model settings in this order: +With an explicit provider API key (BYOK), Postil resolves model settings in this order: 1. Command-line flags 2. Environment variables 3. `.postil.yaml`, `.postil.yml`, or `.postil.json` -4. Stored `postil login` routing -5. Built-in defaults +4. Built-in defaults + +With a stored `postil login` credential, the hosted service controls model, reasoning effort, and provider settings. Local model configuration, environment overrides, and model or effort flags are ignored with a warning. An explicit provider API key takes precedence over a stored login and enables local model settings. Translated `.coderabbit.yaml` settings can supply review policy but do not select a model. @@ -83,7 +84,7 @@ postil logout `postil login` authenticates against the configured login server over a device-authorization flow (open the printed URL, enter the code) and stores a renewable credential at `${XDG_CONFIG_HOME:-~/.config}/postil/credentials.json`, mode `0600` in a `0700` directory. An ambiguous polling failure retries the same device code so the service can recover the already-issued pair during its 60-second recovery window. The credential records its canonical issuing server. Refresh and logout use that stored issuer; a conflicting explicit `POSTIL_LOGIN_SERVER` fails before any login credential is sent. Issuer-free credentials from older CLI versions infer `https://postil.dev` only when their normalized `apiBase` is the canonical Postil inference endpoint. An issuer-free credential for any custom endpoint requires `postil login` again before refresh or logout. -The stored credential is a fallback used only when none of `POSTIL_API_KEY`, `OPENROUTER_API_KEY`, `MODEL_API_KEY`, or `LLM_API_KEY` is set. Postil rotates it before access expiry and persists the replacement; explicit API keys never trigger a refresh. Its `apiBase` is the only endpoint that may receive its bearer. A different `POSTIL_API_BASE` fails before network access unless an explicit API key is set. `REVIEW_MODEL` may still select a model at the stored endpoint. A legacy access-only login, an expired refresh inactivity window, or a rejected refresh instructs the user to run `postil login` again. A valid `Retry-After` value on a temporary refresh rate limit is reported in seconds. Missing or malformed values use a generic retry message, and all temporary refresh failures retain the credential. +The stored credential is a fallback used only when none of `POSTIL_API_KEY`, `OPENROUTER_API_KEY`, `MODEL_API_KEY`, or `LLM_API_KEY` is set. Postil rotates it before access expiry and persists the replacement; explicit API keys never trigger a refresh. Its `apiBase` is the only endpoint that may receive its bearer. A different `POSTIL_API_BASE` fails before network access unless an explicit API key is set. Its stored model is a request hint; review output records the model returned by the service when available. A legacy access-only login, an expired refresh inactivity window, or a rejected refresh instructs the user to run `postil login` again. A valid `Retry-After` value on a temporary refresh rate limit is reported in seconds. Missing or malformed values use a generic retry message, and all temporary refresh failures retain the credential. `postil logout` removes the active local credential only after remote revocation succeeds. A failed revocation retains its sole retry handle and asks the user to run `postil logout` again. A new login stages the newly issued family and any overwritten family in a private pending-revocation queue before replacing the active credential. A local replacement failure therefore retains the new remote family's revocation handle. Pending retries never block normal review work and never follow `POSTIL_LOGIN_SERVER` overrides. @@ -97,4 +98,4 @@ postil doctor `postil config` prints the resolved non-secret configuration and separate provenance for the model, reviewer reasoning effort, and scorer reasoning effort. `postil doctor` validates endpoint reachability, credential acceptance, and repository setup without printing credential values. Both commands identify renewable logins, access expiry, refresh inactivity expiry, and legacy access-only logins. -Use `--reasoning-effort` and `--scorer-reasoning-effort` for one review. These flags override the matching environment variables, which override `model.reasoningEffort` and `model.scorerReasoningEffort`. The built-in reviewer and scorer defaults are both `low`. Every request carries the resolved value, including retries and repair calls. +With BYOK, use `--reasoning-effort` and `--scorer-reasoning-effort` for one review. These flags override the matching environment variables, which override `model.reasoningEffort` and `model.scorerReasoningEffort`. The built-in reviewer and scorer defaults are both `low`. Every request carries the resolved value, including retries and repair calls. Stored-login reviews use the hosted service's reasoning policy. diff --git a/docs/model-providers.md b/docs/model-providers.md index 3506fbd..e6d91d7 100644 --- a/docs/model-providers.md +++ b/docs/model-providers.md @@ -1,6 +1,6 @@ # Model providers -Postil speaks either an OpenAI-compatible chat-completions interface or the native Anthropic Messages API. Provider requests do not follow redirects, credentials are never written to logs, and private-network endpoints require an explicit opt-in. +With BYOK, Postil speaks either an OpenAI-compatible chat-completions interface or the native Anthropic Messages API. Provider requests do not follow redirects, credentials are never written to logs, and private-network endpoints require an explicit opt-in. A stored hosted login delegates model, reasoning, and provider selection to the service; the local overrides described here require an explicit provider API key. ## Model admission diff --git a/src/cli.rs b/src/cli.rs index 0f9f638..75680d2 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -145,13 +145,13 @@ pub enum Command { /// Explicit config file (bypasses discovery). #[arg(long)] config: Option, - /// Model override (else REVIEW_MODEL, else config, else default). + /// BYOK model override. Ignored when a stored login supplies hosted inference. #[arg(long)] model: Option, - /// Reviewer reasoning effort: max|xhigh|high|medium|low|minimal|none (else REVIEW_REASONING_EFFORT, else config, else low). + /// BYOK reviewer effort: max|xhigh|high|medium|low|minimal|none. Ignored with a stored login. #[arg(long, value_name = "EFFORT")] reasoning_effort: Option, - /// Scorer reasoning effort: max|xhigh|high|medium|low|minimal|none (else REVIEW_SCORER_REASONING_EFFORT, else config, else low). + /// BYOK scorer effort: max|xhigh|high|medium|low|minimal|none. Ignored with a stored login. #[arg(long, value_name = "EFFORT")] scorer_reasoning_effort: Option, /// Keep detailed provider, retry, and batch telemetry in interactive terminals. diff --git a/src/config.rs b/src/config.rs index c7e2d39..e6cffb5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,8 +1,9 @@ //! Resolved review configuration. //! -//! Model precedence: CLI flags > environment > `.postil.{yaml,yml,json}` > -//! stored login routing > embedded defaults. Translated `.coderabbit.yaml` -//! settings do not select a model. +//! BYOK model precedence: CLI flags > environment > +//! `.postil.{yaml,yml,json}` > embedded defaults. A stored login supplies the +//! hosted endpoint and model hint; local model, reasoning, and provider settings +//! are ignored. Translated `.coderabbit.yaml` settings do not select a model. //! //! Exception: `model.apiBase` from a config file is repo-controlled, and the //! resolved base URL receives the deployment's bearer key. It is ignored by @@ -15,9 +16,10 @@ //! valid, unexpired credential exists at //! `${XDG_CONFIG_HOME:-~/.config}/postil/credentials.json`, //! `login::resolve_stored_token` supplies its bearer key and its `apiBase`/ -//! `model` provide the baseline below trusted project configuration and -//! environment overrides. A resolved endpoint override is accepted only with -//! an explicit API key; a stored login remains bound to its issuing endpoint. +//! `model` supply the hosted endpoint and model hint. The service selects the +//! actual model. A resolved endpoint override is accepted +//! only with an explicit API key; a stored login remains bound to its issuing +//! endpoint. //! An expired or unreadable stored credential is left alone here; //! `resolve_api_key` reports it as one actionable "run `postil login` again" //! error, while `postil config` reports the stored-login state without needing @@ -1151,6 +1153,9 @@ pub struct Config { pub source: String, /// Winning source for the generator chain, shown by `postil config`. pub model_source: String, + /// Whether the effective inference credential comes from `postil login`. + /// Explicit provider API keys leave this false even when a login is stored. + pub(crate) stored_login_authority: bool, } impl Default for Config { @@ -1188,6 +1193,7 @@ impl Default for Config { content_policy_disabled: false, source: "defaults".to_string(), model_source: "embedded default".to_string(), + stored_login_authority: false, } } } @@ -1265,9 +1271,9 @@ impl Config { #[cfg(test)] let _environment_guard = crate::test_env_lock().lock().unwrap(); let mut cfg = Config::default(); - // Login supplies a local baseline, below trusted project configuration - // and environment overrides but above the embedded provider defaults. - cfg.apply_stored_login_credential(); + // Resolve credential origin before applying local model configuration. + // A stored login delegates inference policy to its issuing service. + cfg.stored_login_authority = cfg.apply_stored_login_credential(); if let Some(path) = explicit { let file = Self::read_postil_file(path) .with_context(|| format!("reading config {}", path.display()))?; @@ -1328,7 +1334,11 @@ impl Config { } pub fn apply_file(&mut self, f: FileConfig) -> Result<()> { - self.apply_file_inner(f, allow_config_api_base(), repository_model_config_locked()) + self.apply_file_inner( + f, + allow_config_api_base(), + repository_model_config_locked() || self.stored_login_authority, + ) } /// Core of [`apply_file`]. `allow_api_base` decides whether a @@ -1339,7 +1349,7 @@ impl Config { &mut self, f: FileConfig, allow_api_base: bool, - hosted_mode: bool, + model_config_locked: bool, ) -> Result<()> { if let Some(v) = f.enabled { self.enabled = v; @@ -1404,9 +1414,9 @@ impl Config { } } if let Some(m) = f.model { - if hosted_mode { + if model_config_locked { crate::progress::notice(format_args!( - "postil: ignoring repository model configuration in hosted mode; hosted inference selects the provider and model roster" + "postil: ignoring repository model configuration while using hosted inference; the hosted service selects model, reasoning effort, and provider settings" )); } else { if let Some(n) = m.name { @@ -1542,13 +1552,15 @@ impl Config { ), }; } - if let Ok(value) = std::env::var("REVIEW_REASONING_EFFORT") + if !self.stored_login_authority + && let Ok(value) = std::env::var("REVIEW_REASONING_EFFORT") && !value.trim().is_empty() { self.reasoning_effort = ReasoningEffort::parse("REVIEW_REASONING_EFFORT", &value)?; self.reasoning_effort_source = "environment".to_string(); } - if let Ok(value) = std::env::var("REVIEW_SCORER_REASONING_EFFORT") + if !self.stored_login_authority + && let Ok(value) = std::env::var("REVIEW_SCORER_REASONING_EFFORT") && !value.trim().is_empty() { self.scorer_reasoning_effort = @@ -1595,6 +1607,39 @@ impl Config { self.model_source = "qualification candidate profile".to_string(); return Ok(()); } + if self.stored_login_authority { + let ignored = [ + "REVIEW_MODEL", + "REVIEW_MODEL_CASCADE", + "REVIEW_MODEL_CONSENSUS", + "REVIEW_REASONING_EFFORT", + "REVIEW_SCORER_MODEL", + "REVIEW_SCORER_MODEL_CASCADE", + "REVIEW_SCORER_REASONING_EFFORT", + "POSTIL_DISABLE_SCORER", + "POSTIL_API_FORMAT", + "POSTIL_ENDPOINT_AUTH_HEADER", + "POSTIL_ENDPOINT_AUTH_VALUE", + ] + .into_iter() + .filter(|name| std::env::var(name).is_ok_and(|value| !value.trim().is_empty())) + .collect::>(); + if !ignored.is_empty() { + crate::progress::notice(format_args!( + "postil: ignoring local hosted-inference settings {} while using a stored login; set an explicit provider API key to use local model settings", + ignored.join(", ") + )); + } + // Preserve the existing endpoint-binding failure for a conflicting + // POSTIL_API_BASE. Runtime credential resolution compares this + // value with the endpoint recorded by the stored login. + if let Ok(base) = std::env::var("POSTIL_API_BASE") + && !base.is_empty() + { + self.api_base = base; + } + return Ok(()); + } if let Ok(m) = std::env::var("REVIEW_MODEL") && !m.trim().is_empty() { @@ -1658,11 +1703,10 @@ impl Config { Ok(()) } - /// A stored `postil login` credential supplies a local fallback API base - /// and, when present, a model. Legacy credentials with an empty model keep - /// the embedded default. Project configuration and environment variables - /// are applied after this method and therefore retain their documented - /// precedence. + /// A stored `postil login` credential supplies the hosted API base and, + /// when present, model. Legacy credentials with an empty model keep the + /// embedded default. Local model, reasoning, and provider settings are + /// ignored while this credential remains effective. /// Runtime credential resolution rejects an endpoint override when no /// explicit API key is set. The cascade is cleared with the model because /// a BYOK fallback chain does not describe the hosted gateway. @@ -1693,6 +1737,10 @@ impl Config { true } + pub(crate) fn uses_stored_login(&self) -> bool { + self.stored_login_authority + } + /// All models to try, in order, deduplicated. pub fn model_chain(&self) -> Vec { let mut chain = Vec::new(); @@ -3066,9 +3114,13 @@ scorer = { enabled = true, default_model = "provider/scorer", reasoning_effort = unsafe { std::env::remove_var(other) }; } EnvRestore::set(name, "provider-fixture-key"); - let mut cfg = Config::default(); + let mut cfg = Config { + stored_login_authority: true, + ..Config::default() + }; + cfg.stored_login_authority = cfg.apply_stored_login_credential(); assert!( - !cfg.apply_stored_login_credential(), + !cfg.uses_stored_login(), "{name} must suppress login routing" ); assert_eq!(cfg.api_base, model_defaults().api_base, "{name}"); @@ -3079,7 +3131,7 @@ scorer = { enabled = true, default_model = "provider/scorer", reasoning_effort = } #[test] - fn project_model_provenance_replaces_stored_login_provenance() { + fn stored_login_keeps_credential_model_authority_over_project_config() { let _lock = env_lock().lock().unwrap(); let directory = tempfile::tempdir().unwrap(); let xdg = EnvRestore::capture("XDG_CONFIG_HOME"); @@ -3106,12 +3158,13 @@ scorer = { enabled = true, default_model = "provider/scorer", reasoning_effort = .unwrap(); let mut config = Config::default(); - assert!(config.apply_stored_login_credential()); + config.stored_login_authority = config.apply_stored_login_credential(); + assert!(config.uses_stored_login()); assert_eq!(config.model_source, "stored login"); let file: FileConfig = yaml_serde::from_str("model:\n name: project/model\n").unwrap(); config.apply_file(file).unwrap(); - assert_eq!(config.model, "project/model"); - assert_eq!(config.model_source, "trusted project config"); + assert_eq!(config.model, "hosted/model"); + assert_eq!(config.model_source, "stored login"); drop(saved_keys); drop(xdg); @@ -3147,7 +3200,8 @@ scorer = { enabled = true, default_model = "provider/scorer", reasoning_effort = .unwrap(); let mut config = Config::default(); - assert!(config.apply_stored_login_credential()); + config.stored_login_authority = config.apply_stored_login_credential(); + assert!(config.uses_stored_login()); assert_eq!(config.api_base, "https://postil.dev/api/inference/v1"); assert_eq!(config.model, model_defaults().default_model); assert_eq!(config.model_source, "embedded default"); diff --git a/src/llm.rs b/src/llm.rs index 33631d4..5365e40 100644 --- a/src/llm.rs +++ b/src/llm.rs @@ -594,6 +594,7 @@ pub struct LlmClient { request_api_base: String, api_key: String, endpoint_auth: Option, + stored_login_authority: bool, request_decorations: RequestDecorations, request_timeout: Duration, timeout_retry_timeout: Duration, @@ -2754,7 +2755,11 @@ impl LlmClient { review_deadline: Option, total_deadline: Option, ) -> Result { - let endpoint_auth = endpoint_auth_from_env(cfg.api_format)?; + let endpoint_auth = if cfg.uses_stored_login() { + None + } else { + endpoint_auth_from_env(cfg.api_format)? + }; let request_decorations = RequestDecorations::from_config(cfg)?; let request_api_base = qualification_request_api_base(&cfg.api_base)?; Ok(LlmClient { @@ -2765,6 +2770,7 @@ impl LlmClient { request_api_base, api_key, endpoint_auth, + stored_login_authority: cfg.uses_stored_login(), request_decorations, request_timeout, timeout_retry_timeout: request_timeout.min(Duration::from_secs(TIMEOUT_RETRY_CAP_SECS)), @@ -2779,6 +2785,14 @@ impl LlmClient { }) } + fn completed_call_model<'a>(&self, requested: &'a str, calls: &'a [ModelUsage]) -> &'a str { + if self.stored_login_authority { + calls.last().map_or(requested, |call| call.model.as_str()) + } else { + requested + } + } + fn model_usage_event( &self, model: &str, @@ -3681,7 +3695,7 @@ impl LlmClient { parsed } }; - let mut review = into_review(raw, model, usage); + let mut review = into_review(raw, self.completed_call_model(model, &call_usage), usage); review.model_usage = call_usage.clone(); review.model_incidents.append(&mut model_incidents); review.usage_accounting_complete = usage_accounting_complete; @@ -3743,7 +3757,11 @@ impl LlmClient { review.model_usage = call_usage.clone(); review.usage_accounting_complete = usage_accounting_complete; if let Ok(retried_raw) = parse_review(&retried) { - let mut candidate = into_review(retried_raw, model, retry_usage); + let mut candidate = into_review( + retried_raw, + self.completed_call_model(model, &call_usage), + retry_usage, + ); candidate.model_usage = call_usage.clone(); candidate.usage_accounting_complete = usage_accounting_complete; let still_contradictory = @@ -3849,7 +3867,11 @@ impl LlmClient { match retry { Ok(content) => match parse_review(&content) { Ok(raw) => { - let mut candidate = into_review(raw, model, retry_usage); + let mut candidate = into_review( + raw, + self.completed_call_model(model, &call_usage), + retry_usage, + ); candidate.model_usage = call_usage.clone(); candidate.model_incidents = review.model_incidents.clone(); candidate.usage_accounting_complete = retry_accounting_complete; @@ -4009,7 +4031,7 @@ impl LlmClient { }, revised_body: resolution.revised_body, evidence: resolution.evidence, - model_used: model.to_string(), + model_used: self.completed_call_model(model, &call_usage).to_string(), usage, model_usage: call_usage, model_incidents, @@ -4059,7 +4081,7 @@ impl LlmClient { })?; Ok(FindingCompressionReview { body: compression.body, - model_used: model.to_string(), + model_used: self.completed_call_model(model, &call_usage).to_string(), usage, model_usage: call_usage, model_incidents: Vec::new(), @@ -4168,7 +4190,7 @@ impl LlmClient { }; Ok(ScorerReview { scores, - model_used: model.to_string(), + model_used: self.completed_call_model(model, &call_usage).to_string(), usage, model_usage: call_usage, model_incidents, @@ -4441,8 +4463,14 @@ impl LlmClient { summary.failure_source = response.failure_source; summary.failure_reason = response.failure_reason; let elapsed = elapsed_text(attempt_started_at.elapsed()); + // The authenticated gateway owns the model choice. Read its + // actual identifier separately from the redacted log summary. + let hosted_response_model = self + .stored_login_authority + .then(|| actual_response_identity(&response.text).0) + .flatten(); call_usage.push(self.model_usage_event( - model, + hosted_response_model.as_deref().unwrap_or(model), phase, call_phase, attempt, @@ -7900,6 +7928,67 @@ mod tests { } } + #[tokio::test] + async fn stored_login_schema_repair_reports_each_returned_model() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .and(body_string_contains("You repair malformed JSON")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "model": "cloud/repair-model", + "choices": [{"finish_reason": "stop", "message": {"content": "{\"summary\":\"\",\"findings\":[]}"}}], + "usage": {"prompt_tokens": 20, "completion_tokens": 3} + }))) + .with_priority(1) + .expect(1) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "model": "cloud/initial-model", + "choices": [{"finish_reason": "stop", "message": {"content": "malformed"}}], + "usage": {"prompt_tokens": 10, "completion_tokens": 2} + }))) + .with_priority(2) + .expect(1) + .mount(&server) + .await; + let config = Config { + api_base: server.uri(), + model: "stored/stale-model".into(), + stored_login_authority: true, + scorer_enabled: false, + ..Config::default() + }; + let client = LlmClient::build( + &config, + "test-key".into(), + Duration::from_secs(2), + None, + None, + ) + .unwrap(); + *client.http.lock().unwrap() = Some(reqwest::Client::new()); + let result = client + .review_validated_with_safe_output_limit( + &config, + "system", + "user", + 4_000, + ReviewRequestRoute::Source, + |_| Ok(()), + ) + .await + .unwrap(); + assert_eq!(result.model_used, "cloud/repair-model"); + assert_eq!(result.model_usage.len(), 2); + assert_eq!(result.model_usage[0].model, "cloud/initial-model"); + assert_eq!(result.model_usage[1].model, "cloud/repair-model"); + assert_eq!(result.usage.prompt_tokens, 30); + assert_eq!(result.usage.completion_tokens, 5); + } + #[tokio::test] async fn synthesis_schema_repair_keeps_explicit_route_metadata() { let server = MockServer::start().await; diff --git a/src/main.rs b/src/main.rs index 073b7b5..7332cb8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -185,13 +185,13 @@ async fn dispatch(cli: Cli) -> anyhow::Result { println!(" This binary contains no standalone hosted qualification profile."); } println!("\nCheck the configured endpoint and model: postil doctor"); - println!("Override once: postil review --model provider/model"); + println!("BYOK override once: postil review --model provider/model"); println!( - "Override reasoning once: postil review --reasoning-effort high --scorer-reasoning-effort none" + "BYOK reasoning override once: postil review --reasoning-effort high --scorer-reasoning-effort none" ); - println!("Override persistently: REVIEW_MODEL=provider/model postil review"); + println!("BYOK persistent override: REVIEW_MODEL=provider/model postil review"); println!( - "Persist reasoning: REVIEW_REASONING_EFFORT=high REVIEW_SCORER_REASONING_EFFORT=none postil review" + "BYOK persistent reasoning: REVIEW_REASONING_EFFORT=high REVIEW_SCORER_REASONING_EFFORT=none postil review" ); println!( "Native Anthropic config: set model.apiBase, model.apiFormat: anthropic, and model.name, then run postil doctor" @@ -199,6 +199,9 @@ async fn dispatch(cli: Cli) -> anyhow::Result { println!( "Config keys: model.name, model.reasoningEffort, and model.scorerReasoningEffort" ); + println!( + "Hosted login: the Postil service selects model, reasoning, and provider settings" + ); Ok(0) } Command::Plan { envelopes, config } => { diff --git a/src/review.rs b/src/review.rs index 42779cc..bb412bd 100644 --- a/src/review.rs +++ b/src/review.rs @@ -632,18 +632,37 @@ pub async fn run(args: ReviewArgs) -> Result { ); } let mut cfg = Config::load(&cwd, args.config.as_deref())?; - if let Some(m) = &args.model { - cfg.model = m.clone(); - cfg.model_source = "command line".to_string(); - } - if let Some(effort) = &args.reasoning_effort { - cfg.reasoning_effort = ReasoningEffort::parse("--reasoning-effort", effort)?; - cfg.reasoning_effort_source = "command line".to_string(); - } - if let Some(effort) = &args.scorer_reasoning_effort { - cfg.scorer_reasoning_effort = - ReasoningEffort::parse("--scorer-reasoning-effort", effort)?; - cfg.scorer_reasoning_effort_source = "command line".to_string(); + if cfg.uses_stored_login() { + let ignored = [ + args.model.as_ref().map(|_| "--model"), + args.reasoning_effort.as_ref().map(|_| "--reasoning-effort"), + args.scorer_reasoning_effort + .as_ref() + .map(|_| "--scorer-reasoning-effort"), + ] + .into_iter() + .flatten() + .collect::>(); + if !ignored.is_empty() { + crate::progress::notice(format_args!( + "postil: ignoring {} while using a stored login; the hosted service selects model and reasoning settings. Set an explicit provider API key to use these options", + ignored.join(", ") + )); + } + } else { + if let Some(m) = &args.model { + cfg.model = m.clone(); + cfg.model_source = "command line".to_string(); + } + if let Some(effort) = &args.reasoning_effort { + cfg.reasoning_effort = ReasoningEffort::parse("--reasoning-effort", effort)?; + cfg.reasoning_effort_source = "command line".to_string(); + } + if let Some(effort) = &args.scorer_reasoning_effort { + cfg.scorer_reasoning_effort = + ReasoningEffort::parse("--scorer-reasoning-effort", effort)?; + cfg.scorer_reasoning_effort_source = "command line".to_string(); + } } cfg.require_model()?; if let Some(fo) = &args.fail_on { diff --git a/tests/e2e.rs b/tests/e2e.rs index 5741e09..a3730c7 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -1399,7 +1399,7 @@ async fn explicit_byok_key_remains_valid_with_an_api_base_override() { serde_json::to_vec(&json!({ "version": 3, "issuer": "https://postil.dev", - "token": "pcli_e2e-access-not-a-real-secret", + "token": "fixture-stored-login-token", "expiresAt": "2999-01-01T00:00:00.000Z", "refreshToken": "fixture-e2e-refresh-not-a-credential", "refreshExpiresAt": "2999-12-01T00:00:00.000Z", @@ -1418,11 +1418,148 @@ async fn explicit_byok_key_remains_valid_with_an_api_base_override() { .env("POSTIL_API_BASE", server.uri()) .env("MODEL_API_KEY", provider_key) .env("POSTIL_DISABLE_SCORER", "1") - .args(["review", "--diff-file"]) + .args([ + "review", + "--model", + "byok/model", + "--reasoning-effort", + "high", + "--diff-file", + ]) + .arg(&diff) + .args(["--output", "json"]) + .assert() + .success(); + + let requests = server.received_requests().await.unwrap(); + let body: Value = requests[0].body_json().unwrap(); + assert_eq!(body["model"], "byok/model"); + assert_eq!(body["reasoning"], json!({"effort": "high"})); +} + +#[test] +fn stored_login_ignores_repository_and_environment_model_policy() { + let dir = tempfile::tempdir().unwrap(); + let config_home = dir.path().join("config"); + let credentials_dir = config_home.join("postil"); + std::fs::create_dir_all(&credentials_dir).unwrap(); + std::fs::write( + credentials_dir.join("credentials.json"), + serde_json::to_vec(&json!({ + "version": 3, + "issuer": "https://postil.dev", + "token": "pcli_e2e-access-not-a-real-secret", + "expiresAt": "2999-01-01T00:00:00.000Z", + "apiBase": "https://postil.dev/api/inference/v1", + "org": "example", + "model": "hosted/model" + })) + .unwrap(), + ) + .unwrap(); + std::fs::write( + dir.path().join(".postil.yaml"), + "model:\n name: repository/model\n reasoningEffort: max\n cascade: [repository/fallback]\n scorer: repository/scorer\n scorerReasoningEffort: high\n apiFormat: anthropic\n consensus: 3\n", + ) + .unwrap(); + + let assertion = isolated_postil() + .current_dir(dir.path()) + .env("XDG_CONFIG_HOME", &config_home) + .env("REVIEW_MODEL", "environment/model") + .env("REVIEW_REASONING_EFFORT", "turbo") + .env("REVIEW_SCORER_MODEL", "environment/scorer") + .env("POSTIL_API_FORMAT", "anthropic") + .env("POSTIL_ENDPOINT_AUTH_HEADER", "x-provider-auth") + .env("POSTIL_ENDPOINT_AUTH_VALUE", "fixture-value") + .arg("config") + .assert() + .success(); + let stdout = String::from_utf8(assertion.get_output().stdout.clone()).unwrap(); + let stderr = String::from_utf8(assertion.get_output().stderr.clone()).unwrap(); + + assert!(stdout.contains("model.name: hosted/model")); + assert!(stdout.contains("model.source: stored login")); + assert!(stdout.contains("model.reasoningEffort: low")); + assert!(stdout.contains("model.reasoningEffort.source: embedded default")); + assert!(stdout.contains("model.cascade: []")); + assert!(stdout.contains("model.apiFormat: openai-compatible")); + assert!(!stdout.contains("repository/")); + assert!(!stdout.contains("environment/")); + assert!(stderr.contains("ignoring repository model configuration")); + assert!(stderr.contains("ignoring local hosted-inference settings")); + assert!(stderr.contains("REVIEW_MODEL")); + assert!(stderr.contains("REVIEW_REASONING_EFFORT")); + assert!(stderr.contains("POSTIL_API_FORMAT")); + assert!(stderr.contains("POSTIL_ENDPOINT_AUTH_HEADER")); +} + +#[tokio::test] +async fn stored_login_ignores_command_line_model_and_reasoning_policy() { + let server = MockServer::start().await; + let mut response = llm_content(json!([])); + response["model"] = json!("cloud/current-model"); + Mock::given(method("POST")) + .and(path("/chat/completions")) + .respond_with(ResponseTemplate::new(200).set_body_json(response)) + .mount(&server) + .await; + let dir = tempfile::tempdir().unwrap(); + let config_home = dir.path().join("config"); + let credentials_dir = config_home.join("postil"); + std::fs::create_dir_all(&credentials_dir).unwrap(); + std::fs::write( + credentials_dir.join("credentials.json"), + serde_json::to_vec(&json!({ + "version": 3, + "issuer": server.uri(), + "token": "pcli_e2e-access-not-a-real-secret", + "expiresAt": "2999-01-01T00:00:00.000Z", + "apiBase": server.uri(), + "org": "example", + "model": "hosted/model" + })) + .unwrap(), + ) + .unwrap(); + let diff = write_diff(dir.path()); + + let assertion = isolated_postil() + .current_dir(dir.path()) + .env("XDG_CONFIG_HOME", &config_home) + .env("POSTIL_ALLOW_PRIVATE_API_BASE", "1") + .env("POSTIL_ENDPOINT_AUTH_HEADER", "x-provider-auth") + .env("POSTIL_ENDPOINT_AUTH_VALUE", "fixture-value") + .args([ + "review", + "--model", + "command/model", + "--reasoning-effort", + "turbo", + "--scorer-reasoning-effort", + "max", + "--diff-file", + ]) .arg(&diff) .args(["--output", "json"]) .assert() .success(); + + let requests = server.received_requests().await.unwrap(); + let body: Value = requests[0].body_json().unwrap(); + assert_eq!(body["model"], "hosted/model"); + assert_eq!(body["reasoning"], json!({"effort": "low"})); + assert_eq!(body["max_tokens"], 8_000); + assert_eq!(body["temperature"], 0.1); + assert!(body.get("provider").is_none()); + assert!(body.get("response_format").is_none()); + assert!(requests[0].headers.get("x-provider-auth").is_none()); + let envelope: Value = serde_json::from_slice(&assertion.get_output().stdout).unwrap(); + assert_eq!(envelope["modelUsed"], "cloud/current-model"); + assert_eq!(envelope["modelUsage"][0]["model"], "cloud/current-model"); + let stderr = String::from_utf8(assertion.get_output().stderr.clone()).unwrap(); + assert!(stderr.contains("ignoring --model, --reasoning-effort, --scorer-reasoning-effort")); + assert!(stderr.contains("hosted service selects model and reasoning settings")); } #[tokio::test]