diff --git a/diffr/Cargo.lock b/diffr/Cargo.lock index 0b2e15bdc6..e9de81e1aa 100644 --- a/diffr/Cargo.lock +++ b/diffr/Cargo.lock @@ -759,6 +759,7 @@ version = "0.1.21" dependencies = [ "anyhow", "assert_cmd", + "base64", "bytes", "clap", "cliclack", diff --git a/diffr/Cargo.toml b/diffr/Cargo.toml index aa3f6a7245..28b8ca026a 100644 --- a/diffr/Cargo.toml +++ b/diffr/Cargo.toml @@ -54,6 +54,7 @@ bytes = "1.12" wasmtime-wasi-http = { version = "49.0.2", default-features = false, features = ["p2", "p3", "default-send-request"] } cliclack = "0.5.6" dirs = "5" +base64 = "0.22" [target.'cfg(not(any(windows, target_os = "illumos", target_os = "freebsd")))'.dependencies] diff --git a/diffr/crates/diffr-core/src/config/store.rs b/diffr/crates/diffr-core/src/config/store.rs index 42704fd32e..26d2e99257 100644 --- a/diffr/crates/diffr-core/src/config/store.rs +++ b/diffr/crates/diffr-core/src/config/store.rs @@ -498,7 +498,7 @@ mod tests { ); assert_eq!( error("plugins.shape.bundled.summarize.provider", "mistral"), - "plugins.shape.bundled.summarize.provider: expected one of \"gemini\", \"openai\", \"anthropic\", got \"mistral\"" + "plugins.shape.bundled.summarize.provider: expected one of \"gemini\", \"openai\", \"anthropic\", \"codex\", got \"mistral\"" ); assert_eq!( error("plugins.shape.mine.enabled", "true"), diff --git a/diffr/docs/agents.md b/diffr/docs/agents.md index e0c6da36df..c742e150f6 100644 --- a/diffr/docs/agents.md +++ b/diffr/docs/agents.md @@ -40,14 +40,15 @@ use. The bundled plugins' settings: | `deleted-bodies.min_lines` | 12 | Deleted bodies shorter than this stay open. | | `removed-runs.min_lines` | 5 | Removed stretches shorter than this stay open. | | `test-bodies.min_lines` | 3 | Test bodies shorter than this stay open. | -| `summarize.enabled` | false | LLM pseudocode summaries of large new bodies and tests. Needs a key. | -| `summarize.provider` | `gemini` | `gemini`, `openai` or `anthropic`. Changing it clears `api_key`. | +| `summarize.enabled` | false | LLM pseudocode summaries of large new bodies and tests. Needs a key or the Codex sign-in. | +| `summarize.provider` | `gemini` | `gemini`, `openai`, `anthropic` or `codex`. Changing it clears `api_key`. | | `summarize.model` | the provider's | The model name sent to the provider. | -| `summarize.api_key` | unset | Without it, the provider's variable in the environment is used: `GEMINI_API_KEY` or `GOOGLE_API_KEY`, `OPENAI_API_KEY`, `ANTHROPIC_API_KEY`. | +| `summarize.api_key` | unset | Without it, the provider's variable in the environment is used: `GEMINI_API_KEY` or `GOOGLE_API_KEY`, `OPENAI_API_KEY`, `ANTHROPIC_API_KEY`. `codex` reads Codex's saved sign-in instead; `diffr config logins` says whether it is ready. | | `summarize.endpoint` | the provider's | Another base URL: a proxy, OpenRouter, or a local OpenAI-compatible server with `provider = "openai"`. | | `summarize.min_lines` | 20 | New bodies shorter than this are shown as code. | | `summarize.tests` | true | Summarize new test bodies too. | | `summarize.test_min_lines` | 20 | Shorter test bodies keep their ordinary labels. | +| `summarize.max_length_ratio` | 0.67 | Pseudocode longer than this fraction of the body is discarded. | | `summarize.system_prompt` | built in | The instruction sent with each body. Change it to change the style of the summaries. | | `summarize.request_timeout_ms` | 60000 | Timeout for each request. | | `summarize.retries` | 3 | Retries after a timeout, rate limit or server error. | diff --git a/diffr/plugins/shape/summarize/plugin.toml b/diffr/plugins/shape/summarize/plugin.toml index 2fcb8c9f49..3337f35224 100644 --- a/diffr/plugins/shape/summarize/plugin.toml +++ b/diffr/plugins/shape/summarize/plugin.toml @@ -11,10 +11,10 @@ description = "Summarize large new function bodies and new tests as pseudocode. default = false [options.provider] -enum = ["gemini", "openai", "anthropic"] -"x-enum-titles" = ["Gemini", "OpenAI", "Anthropic"] +enum = ["gemini", "openai", "anthropic", "codex"] +"x-enum-titles" = ["Gemini", "OpenAI", "Anthropic", "Codex"] title = "Provider" -description = "Which model API to call. `openai` also reaches OpenRouter and any other OpenAI-compatible server through `endpoint`." +description = "Which model API to call. `openai` also reaches OpenRouter and any other OpenAI-compatible server through `endpoint`. `codex` uses Codex's saved sign-in." default = "gemini" [options.model] @@ -25,7 +25,7 @@ description = "The model name sent to the provider. Unset uses the provider's de [options.model."x-default-by"] key = "provider" -values = { gemini = "gemini-3.8-flash", openai = "gpt-6-luna", anthropic = "claude-haiku-5-5" } +values = { gemini = "gemini-3.8-flash", openai = "gpt-6-luna", anthropic = "claude-haiku-5-5", codex = "gpt-6-luna" } [options.min_lines] type = "integer" @@ -34,6 +34,13 @@ title = "Shortest body to summarize (lines)" description = "New function bodies shorter than this are shown as code, not summarized: pseudocode only pays off once the body is long." default = 20 +[options.max_length_ratio] +type = "number" +exclusiveMinimum = 0 +title = "Longest summary" +description = "Summaries longer than this fraction of the body are discarded." +default = 0.67 + [options.api_key] type = "string" "x-reset-by" = "provider" @@ -151,6 +158,7 @@ additionalProperties = false endpoint = { type = "string" } key_variables = { type = "array", items = { type = "string" } } keyless_custom_endpoint = { type = "boolean" } +login = { enum = ["codex"] } [options.provider_details."x-default-by"] key = "provider" @@ -170,6 +178,12 @@ endpoint = "https://api.anthropic.com" key_variables = ["ANTHROPIC_API_KEY"] keyless_custom_endpoint = false +[options.provider_details."x-default-by".values.codex] +endpoint = "https://chatgpt.com/backend-api/codex" +key_variables = [] +keyless_custom_endpoint = false +login = "codex" + # The fold queries for each language, as files in this folder. [queries] rust = "queries/rust.scm" diff --git a/diffr/plugins/shape/summarize/plugin.wasm b/diffr/plugins/shape/summarize/plugin.wasm index b351ac257d..ad68b3f405 100644 Binary files a/diffr/plugins/shape/summarize/plugin.wasm and b/diffr/plugins/shape/summarize/plugin.wasm differ diff --git a/diffr/plugins/shape/summarize/rust/src/lib.rs b/diffr/plugins/shape/summarize/rust/src/lib.rs index 3e9a401715..20b17dec95 100644 --- a/diffr/plugins/shape/summarize/rust/src/lib.rs +++ b/diffr/plugins/shape/summarize/rust/src/lib.rs @@ -28,6 +28,7 @@ pub struct Options { pub min_lines: usize, pub tests: bool, pub test_min_lines: usize, + pub max_length_ratio: f64, pub api_key: Option, pub endpoint: Option, pub request_timeout_ms: u64, @@ -81,7 +82,7 @@ impl Summarize { }; let headers = provider.headers(key); let failed = |message: String| anyhow!("{}: {message}", self.options.model); - let text: serde_json::Value = { + let body = { let mut attempt = 0; loop { let result = http::post( @@ -92,10 +93,7 @@ impl Summarize { ) .await; let retry = match result { - Ok((status, body)) if (200..300).contains(&status) => { - break serde_json::from_slice(&body) - .map_err(|error| failed(error.to_string()))?; - } + Ok((status, body)) if (200..300).contains(&status) => break body, Ok((status, _)) if status == 429 || status >= 500 => format!("HTTP {status}"), Ok((status, body)) => { return Err(failed(format!( @@ -116,13 +114,23 @@ impl Summarize { } }; let pseudocode = provider - .text(&text) - .ok_or_else(|| failed("no text in the response".to_owned()))? - .trim(); + .text(&body) + .ok_or_else(|| failed("no text in the response".to_owned()))?; + let pseudocode = unfenced(pseudocode.trim()).trim(); Ok((!pseudocode.is_empty()).then(|| pseudocode.to_owned())) } } +fn unfenced(text: &str) -> &str { + let Some((opening, rest)) = text.split_once('\n') else { + return text; + }; + match rest.strip_suffix("```") { + Some(inner) if opening.starts_with("```") => inner, + _ => text, + } +} + /// The API key: the `api_key` option, or else the first of the provider's /// environment variables that is set and not empty. `Missing` says why on /// stderr. @@ -146,6 +154,9 @@ fn resolve_key(config: &Options, custom_endpoint: bool) -> Key { if config.provider_details.keyless_custom_endpoint && custom_endpoint { return Key::NotRequired; } + if config.provider_details.login.is_some() { + return Key::Missing; + } eprintln!( "summarize: off for this run: no API key: set plugins.shape.bundled.summarize.api_key, or {} in the environment, or turn the summarizer off with plugins.shape.bundled.summarize.enabled = false", variables.join(" or ") @@ -204,10 +215,10 @@ impl GuestPlugin for Summarize { else { return Ok(false); }; - // Pseudocode earns its place only when it is at most two thirds as - // long as the body; otherwise the body keeps its original visibility. - let size = |text: &str| text.chars().filter(|c| !c.is_whitespace()).count(); - if size(&pseudocode) * 3 > size(&text) * 2 { + // Pseudocode earns its place only when it is short enough; otherwise + // the body keeps its original visibility. + let size = |text: &str| text.chars().filter(|c| !c.is_whitespace()).count() as f64; + if size(&pseudocode) > size(&text) * self.options.max_length_ratio { return Ok(false); } cursor.set_collapsed(data.id, true)?; diff --git a/diffr/plugins/shape/summarize/rust/src/provider.rs b/diffr/plugins/shape/summarize/rust/src/provider.rs index 2e957b2068..2da1e34aba 100644 --- a/diffr/plugins/shape/summarize/rust/src/provider.rs +++ b/diffr/plugins/shape/summarize/rust/src/provider.rs @@ -14,6 +14,8 @@ pub struct Details { /// Whether a custom endpoint may go without a key, as OpenAI-compatible /// servers such as Ollama often do. pub keyless_custom_endpoint: bool, + #[serde(default)] + pub login: Option, } #[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize)] @@ -22,6 +24,7 @@ pub enum Provider { Gemini, OpenAi, Anthropic, + Codex, } impl Provider { @@ -31,6 +34,7 @@ impl Provider { Self::Gemini => format!("{endpoint}/v1beta/models/{model}:generateContent"), Self::OpenAi => format!("{endpoint}/chat/completions"), Self::Anthropic => format!("{endpoint}/v1/messages"), + Self::Codex => format!("{endpoint}/responses"), } } @@ -41,6 +45,7 @@ impl Provider { Self::Gemini => ("x-goog-api-key", key.to_owned()), Self::OpenAi => ("authorization", format!("Bearer {key}")), Self::Anthropic => ("x-api-key", key.to_owned()), + Self::Codex => ("authorization", format!("Bearer {key}")), }); } if self == Self::Anthropic { @@ -52,6 +57,7 @@ impl Provider { /// Only Gemini gets a temperature: current reasoning models reject one. /// OpenAI gets no length limit either, since compatible servers name it /// differently; Anthropic's limit also covers thinking, so it has a floor. + /// The ChatGPT backend only streams. pub fn body(self, model: &str, system: &str, user: &str, max_tokens: usize) -> Value { match self { Self::Gemini => json!({ @@ -76,22 +82,54 @@ impl Provider { "system": system, "messages": [{"role": "user", "content": user}], }), + Self::Codex => json!({ + "model": model, + "instructions": system, + "input": [{"role": "user", "content": [{"type": "input_text", "text": user}]}], + "reasoning": {"effort": "low"}, + "store": false, + "stream": true, + }), } } - pub fn text(self, response: &Value) -> Option<&str> { - match self { - Self::Gemini => response["candidates"][0]["content"]["parts"] + pub fn text(self, body: &[u8]) -> Option { + let json = || serde_json::from_slice::(body).ok(); + let text = match self { + Self::Codex => return completed(body), + Self::Gemini => json()?["candidates"][0]["content"]["parts"] .as_array()? .last()?["text"] - .as_str(), - Self::OpenAi => response["choices"][0]["message"]["content"].as_str(), - Self::Anthropic => response["content"] + .as_str()? + .to_owned(), + Self::OpenAi => json()?["choices"][0]["message"]["content"] + .as_str()? + .to_owned(), + Self::Anthropic => json()?["content"] .as_array()? .iter() .rev() .find(|block| block["type"] == "text")?["text"] - .as_str(), - } + .as_str()? + .to_owned(), + }; + Some(text) } } + +fn completed(body: &[u8]) -> Option { + let event = String::from_utf8_lossy(body) + .lines() + .filter_map(|line| line.strip_prefix("data:")) + .filter_map(|data| serde_json::from_str::(data.trim_start()).ok()) + .find(|event| event["type"] == "response.completed")?; + let text: String = event["response"]["output"] + .as_array()? + .iter() + .filter(|item| item["type"] == "message") + .flat_map(|item| item["content"].as_array().into_iter().flatten()) + .filter(|part| part["type"] == "output_text") + .filter_map(|part| part["text"].as_str()) + .collect(); + Some(text) +} diff --git a/diffr/src/cli.rs b/diffr/src/cli.rs index b57c3358d1..f27bb4abdd 100644 --- a/diffr/src/cli.rs +++ b/diffr/src/cli.rs @@ -220,6 +220,8 @@ enum ConfigCommand { #[arg(long)] json: bool, }, + /// Print which agent sign-ins are ready, as JSON + Logins, } pub(crate) fn run(runtime: &tokio::runtime::Runtime) -> Result { @@ -735,6 +737,10 @@ fn run_config(config: &ConfigArgs) -> Result { })(); return config_result(&mut stdout, result, *json); } + Some(ConfigCommand::Logins) => { + serde_json::to_writer_pretty(&mut stdout, &crate::login::status())?; + stdout.write_all(b"\n")?; + } Some(ConfigCommand::Init { json, answers }) => { return crate::init::run(*json, answers.as_deref()) } diff --git a/diffr/src/init.rs b/diffr/src/init.rs index 7e91bb0c98..7d7b5c33d9 100644 --- a/diffr/src/init.rs +++ b/diffr/src/init.rs @@ -16,6 +16,7 @@ const KEY_INSTRUCTIONS: &str = "To enable pseudocode summarization, we need: 1. An API Key from a supported provider (OpenAI, Anthropic, or Gemini-API compatible) 2. This API key can also be sourced from the environment 3. Otherwise, if that's not convenient, you can grab an API key and provide it to your agent (securely). +Codex needs no key: diffr uses its saved ChatGPT sign-in. "; /// The summarizer's options in `config schema`. @@ -72,6 +73,7 @@ struct State { config: Config, /// Each provider's id, title and key variables, from the schema. providers: Vec<(String, String, Vec)>, + signed_in: Vec, } impl State { @@ -110,10 +112,19 @@ impl State { ) }) .collect(); + let signed_in = ids + .iter() + .filter_map(|id| { + let id = id.as_str()?; + let login = details[id]["login"].as_str()?; + crate::login::token(login).is_ok().then(|| id.to_owned()) + }) + .collect(); Ok(Self { found: Agent::found(), config: Config::load()?, providers, + signed_in, }) } @@ -129,8 +140,8 @@ impl State { .any(|(name, _)| name == "bundled.summarize") } - /// The saved provider when summaries are on; otherwise the first with a - /// key, or the saved one. + /// The saved provider when summaries are on; otherwise Codex, the first + /// with a key, or the saved one. fn provider(&self) -> String { let saved = self.summarize()["provider"] .as_str() @@ -138,6 +149,9 @@ impl State { if self.on() { return saved.to_owned(); } + if self.signed_in.iter().any(|id| id == "codex") { + return "codex".to_owned(); + } self.providers .iter() .map(|(id, _, _)| id.as_str()) @@ -156,8 +170,11 @@ impl State { } /// Whether the summarizer has a key for `provider`: the saved one, which - /// belongs to the saved provider, or one in the environment. + /// belongs to the saved provider, one in the environment, or a sign-in. fn has_key(&self, provider: &str) -> bool { + if self.signed_in.iter().any(|id| id == provider) { + return true; + } let saved = self.summarize()["provider"] == provider && self .summarize() diff --git a/diffr/src/login.rs b/diffr/src/login.rs new file mode 100644 index 0000000000..8364ac13ee --- /dev/null +++ b/diffr/src/login.rs @@ -0,0 +1,103 @@ +//! Agent sign-ins used in place of an API key. They are never refreshed, so +//! the agent's own session is untouched. +use base64::Engine; +use serde_json::{json, Map, Value}; +use std::{ + path::PathBuf, + time::{SystemTime, UNIX_EPOCH}, +}; + +pub(crate) const LOGINS: [&str; 1] = ["codex"]; + +/// Fill `api_key` from the provider's `login` when no key is set. +pub(crate) fn fill(name: &str, options: &mut Map) { + let details = options.get("provider_details"); + let Some(login) = details.and_then(|details| details.get("login")?.as_str()) else { + return; + }; + let set = |key: &str| !key.is_empty(); + let in_environment = details + .and_then(|details| details.get("key_variables")?.as_array()) + .into_iter() + .flatten() + .filter_map(Value::as_str) + .any(|variable| std::env::var(variable).is_ok_and(|key| set(&key))); + let in_options = options + .get("api_key") + .and_then(Value::as_str) + .is_some_and(set); + if in_options || in_environment { + return; + } + match token(login) { + Ok(token) => { + options.insert("api_key".into(), token.into()); + } + Err(why) => eprintln!("plugins.shape.{name}: off for this run: {why}"), + } +} + +pub(crate) fn status() -> Value { + LOGINS + .iter() + .map(|login| { + let state = match token(login) { + Ok(_) => json!({ "ready": true }), + Err(why) => json!({ "ready": false, "message": why }), + }; + (login.to_string(), state) + }) + .collect::>() + .into() +} + +pub(crate) fn token(login: &str) -> Result { + match login { + "codex" => codex(), + _ => Err(format!("unknown sign-in {login}")), + } +} + +fn now_ms() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |since| since.as_millis() as u64) +} + +fn home(variable: &str, default: &str) -> Option { + std::env::var_os(variable) + .filter(|dir| !dir.is_empty()) + .map(PathBuf::from) + .or_else(|| dirs::home_dir().map(|home| home.join(default))) +} + +fn codex() -> Result { + let path = home("CODEX_HOME", ".codex") + .ok_or("no home directory for this user")? + .join("auth.json"); + let saved = std::fs::read_to_string(&path).map_err(|_| { + format!( + "no Codex sign-in found in {}: run `codex login`, with cli_auth_credentials_store = \"file\" if Codex keeps it in the system keyring", + path.display() + ) + })?; + let saved: Value = + serde_json::from_str(&saved).map_err(|_| "the Codex sign-in is unreadable")?; + let token = saved["tokens"]["access_token"] + .as_str() + .ok_or("Codex is not signed in with ChatGPT: run `codex login`, or use the OpenAI provider with an API key")?; + if expiry_ms(token).is_some_and(|at| at <= now_ms()) { + return Err("the Codex sign-in expired: open Codex to renew it".into()); + } + Ok(token.to_owned()) +} + +/// A JWT's `exp`, in milliseconds. +fn expiry_ms(token: &str) -> Option { + let payload = token.split('.').nth(1)?; + let payload = base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(payload.trim_end_matches('=')) + .ok()?; + let claims: Value = serde_json::from_slice(&payload).ok()?; + claims["exp"].as_u64().map(|exp| exp * 1000) +} diff --git a/diffr/src/main.rs b/diffr/src/main.rs index 391f2acae6..b220666304 100644 --- a/diffr/src/main.rs +++ b/diffr/src/main.rs @@ -47,6 +47,7 @@ mod files; mod git; mod init; mod install; +mod login; mod options; mod plugin; mod pprint; diff --git a/diffr/src/plugin/tests/summarize.rs b/diffr/src/plugin/tests/summarize.rs index f7787fd58b..a6ddbe940d 100644 --- a/diffr/src/plugin/tests/summarize.rs +++ b/diffr/src/plugin/tests/summarize.rs @@ -207,6 +207,22 @@ fn long_summaries_are_discarded_without_changing_initial_folding() { assert_eq!(folds, vec![(false, String::new())]); } +#[test] +fn a_higher_length_ratio_keeps_a_long_summary() { + let (file, mut sides) = project("a.py", "", LARGE); + let (endpoint, server) = serve(vec![(200, gemini_answer("a()\nb()\nc()"))]); + let pipeline = summarizer_with(json!({ + "api_key": "test", + "endpoint": endpoint, + "min_lines": 3, + "retries": 0, + "max_length_ratio": 100.0, + })); + shape(&pipeline, &file, &mut sides).unwrap(); + server.join().unwrap(); + assert_eq!(fold_label(&sides), "a()\nb()\nc()"); +} + #[test] fn summaries_collapse_selected_folds_behind_pseudocode() { let (file, mut sides) = project("a.py", "", LARGE); @@ -318,7 +334,7 @@ fn each_provider_sends_its_own_request_and_reads_its_own_answer() { ( "anthropic", "", - json!({"content": [{"type": "thinking", "thinking": "hmm"}, {"type": "text", "text": "a b c"}]}), + json!({"content": [{"type": "thinking", "thinking": "hmm"}, {"type": "text", "text": "```pseudo\na b c\n```"}]}), "x-api-key: test-key", ), ] { @@ -368,6 +384,34 @@ fn each_provider_sends_its_own_request_and_reads_its_own_answer() { } } +#[test] +fn codex_streams_a_response_and_reads_the_completed_event() { + let (file, mut sides) = project("a.py", "", LARGE); + let completed = json!({"type": "response.completed", "response": {"output": [ + {"type": "reasoning", "summary": []}, + {"type": "message", "content": [{"type": "output_text", "text": "a b c"}]}, + ]}}); + let stream = format!( + "event: response.created\ndata: {{\"type\":\"response.created\"}}\n\nevent: response.completed\ndata: {completed}\n\n" + ); + let (endpoint, server) = serve_requests(vec![(200, stream)]); + let pipeline = summarizer_with(json!({ + "provider": "codex", + "model": "test-model", + "api_key": "test-token", + "endpoint": format!("{endpoint}/codex"), + "min_lines": 3, + "retries": 0, + })); + shape(&pipeline, &file, &mut sides).unwrap(); + assert_eq!(fold_label(&sides), "a b c"); + let request = server.join().unwrap().remove(0); + assert_eq!(request.line, "POST /codex/responses HTTP/1.1"); + assert!(request + .headers + .contains(&"authorization: bearer test-token".to_owned())); +} + /// The default chain with the summarizer on, answering from `endpoint`. fn default_chain(endpoint: &str) -> Pipeline { configured(&format!( diff --git a/diffr/src/plugin/wasm.rs b/diffr/src/plugin/wasm.rs index a803066f96..641fdda42b 100644 --- a/diffr/src/plugin/wasm.rs +++ b/diffr/src/plugin/wasm.rs @@ -661,11 +661,13 @@ fn compile( let pre = DiffrPluginPre::new(link(engine, linker, &entry.folder().component())?) .map_err(anyhow::Error::from) .context("not a shape plugin: it must export diffr:plugin/api")?; + let mut options = entry.options.clone(); + crate::login::fill(name, &mut options); Ok(Component { name: entry.folder().name().into(), reference: name.to_owned(), pre, - options: Value::Object(entry.options.clone()).to_string(), + options: Value::Object(options).to_string(), deferred: entry.folder().deferred(), }) } diff --git a/docs/README.md b/docs/README.md index adf1ea9384..a70ccd8c8a 100644 --- a/docs/README.md +++ b/docs/README.md @@ -5,6 +5,7 @@ Published at [dev.fast/docs](https://dev.fast/docs). - [Quick start](quick-start.md) - [Connect your agent](connect-your-agent.md) - [Create a review](create-a-review.md) +- [diffr configuration](diffr-configuration.md) - [Remote hosts](remote-hosts.md) - [Troubleshooting](troubleshooting.md) - [Privacy](privacy.md) diff --git a/docs/diffr-configuration.md b/docs/diffr-configuration.md new file mode 100644 index 0000000000..e036b0e224 --- /dev/null +++ b/docs/diffr-configuration.md @@ -0,0 +1,78 @@ +# diffr configuration + +diffr computes the diffs in Whiteboard. Its settings apply to every +repository, and the diffr command line uses the same settings. + +## Change a setting + +- In Whiteboard, open Settings, then **Tools**, then **Diff display and AI + summaries**. +- In a terminal, run `diffr config` to open the settings screen, or + `diffr config set ` to change one setting. +- Edit `~/.config/diffr/config.toml`. Reopen the review after you edit it. + +## Display + +| Setting | What it does | Key | +| --- | --- | --- | +| Collapse unchanged lines | Keeps a few unchanged lines around each change and collapses the rest. **Context lines** sets how many. | `plugins.shape.bundled.context` | +| Collapse test bodies | Collapses the bodies of tests. | `plugins.shape.bundled.test-bodies` | +| Collapse deleted function bodies | Collapses long functions that were deleted. | `plugins.shape.bundled.deleted-bodies` | +| Collapse long removed stretches | Collapses long runs of removed lines. | `plugins.shape.bundled.removed-runs` | +| Hide files by tag | Hides generated and vendored files. See [Hidden files](configuration.md#hidden-files). | `plugins.classify.bundled.hide` | +| Hide deleted files | Hides files that were deleted. | `plugins.classify.bundled.hide_deleted` | + +## AI summaries + +diffr can replace large new functions and tests with short pseudocode. It +sends those bodies to the provider you pick, so summaries are off until you +turn them on. + +### Turn on AI summaries + +1. Open Settings, then **Tools**, then **Diff display and AI summaries**. +2. Pick a provider under **AI summaries**. +3. Check **Enable summaries** and click **Save summaries**. + +**Test setup** sends a sample function and shows the summary without saving. + +### Providers + +| Provider | Credential | +| --- | --- | +| Codex | Codex's ChatGPT sign-in. No key needed. | +| Gemini, OpenAI, Anthropic | An API key, entered in Settings or set as `GEMINI_API_KEY`, `OPENAI_API_KEY` or `ANTHROPIC_API_KEY`. | + +If Codex is signed in and no other provider has a key, Settings picks Codex. + +### Codex + +Sign in once with `codex login`. diffr reads `~/.codex/auth.json` (or +`$CODEX_HOME/auth.json`) on every run. Summaries count against your ChatGPT +plan. + +If Codex keeps its sign-in in the system keyring, add this to +`~/.codex/config.toml` and sign in again: + +```toml +cli_auth_credentials_store = "file" +``` + +diffr never refreshes the sign-in, so Codex's own session is untouched. The +token lasts about 10 days after Codex last ran. If it has expired, summaries +skip that run and Settings says so. Open Codex once to renew it. + +### Short summaries only + +A summary that is longer than two thirds of the function is dropped, and the +function stays as code. To change that, set `max_length_ratio`. + +### From the command line + +```toml +[plugins.shape.bundled.summarize] +enabled = true +provider = "codex" +``` + +`diffr config logins` prints whether the Codex sign-in is ready. diff --git a/packages/review-protocol/src/contracts.ts b/packages/review-protocol/src/contracts.ts index 371e98f832..f4dbdaaa64 100644 --- a/packages/review-protocol/src/contracts.ts +++ b/packages/review-protocol/src/contracts.ts @@ -771,12 +771,21 @@ export interface ReviewDiffrProvider { endpoint: string; keyVariables: string[]; keylessCustomEndpoint: boolean; + login?: string; } +export const reviewDiffrLoginsSchema = z.record( + z.string(), + z.object({ ready: z.boolean(), message: z.string().optional() }), +); + +export type ReviewDiffrLogin = z.infer[string]; + export interface ReviewDiffrConfig { values: JsonObject; - credentialSource: "config" | "environment" | "missing"; + credentialSource: "config" | "environment" | "login" | "missing"; providers?: ReviewDiffrProvider[]; + logins?: Record; defaultPrompt?: string; defaultHiddenTags?: string[]; changed?: boolean; @@ -800,7 +809,7 @@ export type ReviewDiffrSummarizerInput = z.infer< const reviewDiffrConfigSchema = z.object({ values: z.custom(isJsonObject), - credentialSource: z.enum(["config", "environment", "missing"]), + credentialSource: z.enum(["config", "environment", "login", "missing"]), providers: z .array( z.object({ @@ -810,9 +819,11 @@ const reviewDiffrConfigSchema = z.object({ endpoint: z.string(), keyVariables: z.array(z.string()), keylessCustomEndpoint: z.boolean(), + login: z.string().optional(), }), ) .optional(), + logins: reviewDiffrLoginsSchema.optional(), defaultPrompt: z.string().optional(), defaultHiddenTags: z.array(z.string()).optional(), changed: z.boolean().optional(), diff --git a/packages/review/app/src/diffr-config-section.browser.test.tsx b/packages/review/app/src/diffr-config-section.browser.test.tsx index b68f5e7a96..caef5d5507 100644 --- a/packages/review/app/src/diffr-config-section.browser.test.tsx +++ b/packages/review/app/src/diffr-config-section.browser.test.tsx @@ -483,3 +483,53 @@ test("offers whatever providers diffr describes, with their defaults", async () .element(page.getByLabelText("Endpoint URL", { exact: true })) .toHaveAttribute("placeholder", "https://api.mistral.ai/v1"); }); + +test("prefers a signed-in Codex when nothing has a key, and shows its sign-in in place of a key", async () => { + const providers = [ + ...config().providers!, + { + id: "codex", + title: "Codex", + model: "gpt-6-luna", + endpoint: "https://chatgpt.com/backend-api/codex", + keyVariables: [], + keylessCustomEndpoint: false, + login: "codex", + }, + ]; + + const { actions } = await mount({ + credentialSource: "missing", + providers, + logins: { + codex: { ready: true }, + }, + }); + + await open(); + await expect + .element(page.getByRole("button", { name: "Codex" })) + .toHaveAttribute("aria-pressed", "true"); + await expect + .element(page.getByLabelText("Model", { exact: true })) + .toHaveValue("gpt-6-luna"); + await expect + .element(page.getByText(/Uses the Codex sign-in saved on this computer/)) + .toBeVisible(); + expect(document.body.textContent).not.toContain("API key"); + + await act(async () => { + await page.getByLabelText("Enable summaries", { exact: true }).click(); + }); + await act(async () => { + await page.getByRole("button", { name: "Save summaries" }).click(); + }); + expect(actions.saveSummarizer).toHaveBeenCalledWith( + expect.objectContaining({ + enabled: true, + provider: "codex", + model: "gpt-6-luna", + apiKey: "", + }), + ); +}); diff --git a/packages/review/app/src/diffr-config-section.tsx b/packages/review/app/src/diffr-config-section.tsx index 27385cda5b..0e10775d28 100644 --- a/packages/review/app/src/diffr-config-section.tsx +++ b/packages/review/app/src/diffr-config-section.tsx @@ -64,17 +64,31 @@ function title(config: ReviewDiffrConfig, id: string): string { return described(config, id)?.title ?? id; } +/** Codex when summaries are off, nothing else has a key, and it is signed in. */ +function preferCodex(config: ReviewDiffrConfig): boolean { + return ( + setting(config, "plugins.shape.bundled.summarize.enabled") !== true && + config.credentialSource === "missing" && + !!described(config, "codex") && + !!config.logins?.codex?.ready + ); +} + function summaryDraft(config: ReviewDiffrConfig): ReviewDiffrSummarizerInput { + const codex = preferCodex(config); + return { enabled: setting(config, "plugins.shape.bundled.summarize.enabled") === true, - provider: provider(config), - model: String( - setting(config, "plugins.shape.bundled.summarize.model") ?? "", - ), - endpoint: String( - setting(config, "plugins.shape.bundled.summarize.endpoint") ?? "", - ), + provider: codex ? "codex" : provider(config), + model: codex + ? (described(config, "codex")?.model ?? "") + : String(setting(config, "plugins.shape.bundled.summarize.model") ?? ""), + endpoint: codex + ? "" + : String( + setting(config, "plugins.shape.bundled.summarize.endpoint") ?? "", + ), systemPrompt: String( setting(config, "plugins.shape.bundled.summarize.system_prompt") ?? config.defaultPrompt ?? @@ -171,6 +185,7 @@ export function DiffrConfigSection({ setting(config, "plugins.shape.bundled.summarize.enabled") === undefined; const summaryValid = !!draft?.model.trim(); + const login = config && draft && described(config, draft.provider)?.login; const savedDraft = config && summaryDraft(config); const hiddenTags = config && setting(config, hideKey); @@ -304,35 +319,46 @@ export function DiffrConfigSection({ /> )} - - - setDraft({ ...draft, apiKey: event.target.value }) - } - /> - -

- {draft.provider !== provider(config) || - draft.endpoint !== savedDraft?.endpoint - ? config.credentialSource !== "config" - ? `Enter a key for ${title(config, draft.provider)}, or leave blank to use its environment variable.` - : draft.provider !== provider(config) - ? `Saving clears the key saved for ${title(config, provider(config))}.` - : "Saving clears the saved key, since the endpoint changed." - : `${ - config.credentialSource === "config" - ? "Saved key" - : config.credentialSource === "environment" - ? "Environment key available" - : "Not configured" - }. Replacement keys are stored in diffr’s config. Leave blank to keep the current key.`} -

+ {login ? ( +

+ {config.logins?.[login]?.ready + ? `Uses the ${title(config, draft.provider)} sign-in saved on this computer. No key needed.` + : (config.logins?.[login]?.message ?? + "Sign-in status unavailable.")} +

+ ) : ( + <> + + + setDraft({ ...draft, apiKey: event.target.value }) + } + /> + +

+ {draft.provider !== provider(config) || + draft.endpoint !== savedDraft?.endpoint + ? config.credentialSource !== "config" + ? `Enter a key for ${title(config, draft.provider)}, or leave blank to use its environment variable.` + : draft.provider !== provider(config) + ? `Saving clears the key saved for ${title(config, provider(config))}.` + : "Saving clears the saved key, since the endpoint changed." + : `${ + config.credentialSource === "config" + ? "Saved key" + : config.credentialSource === "environment" + ? "Environment key available" + : "Not configured" + }. Replacement keys are stored in diffr’s config. Leave blank to keep the current key.`} +

+ + )} { }); }); +test("a ready agent sign-in is a credential and a missing one says why", async () => { + const fake = await diffrTestProcess(); + const values = path.join(fake.root, "values.json"); + await writeFile( + values, + JSON.stringify({ + plugins: { + // oxlint-disable-next-line anti-slop/no-shape-in-symbol-names -- Native diffr config key. + shape: { + bundled: { summarize: { enabled: false, provider: "codex" } }, + }, + }, + }), + ); + vi.stubEnv("TEST_VALUES", values); + vi.stubEnv("TEST_LOGINS", JSON.stringify({ codex: { ready: true } })); + const config = await readDiffrConfig(); + expect(config.credentialSource).toBe("login"); + expect(config.providers).toContainEqual( + expect.objectContaining({ id: "codex", login: "codex" }), + ); + + vi.stubEnv( + "TEST_LOGINS", + JSON.stringify({ codex: { ready: false, message: "run codex login" } }), + ); + expect( + await saveDiffrSummarizer({ ...draft, provider: "codex" }), + ).toMatchObject({ changed: false, error: "run codex login" }); + expect((await fake.calls()).some((args) => args[1] === "set")).toBe(false); +}); + test("synthetic test returns a summary without saving and cleans its config", async () => { const fake = await diffrTestProcess("saved-secret"); const before = await readFile(fake.state, "utf8"); diff --git a/packages/review/src/server/diffr-config.ts b/packages/review/src/server/diffr-config.ts index 3d34cf1300..70299617f6 100644 --- a/packages/review/src/server/diffr-config.ts +++ b/packages/review/src/server/diffr-config.ts @@ -13,10 +13,12 @@ import { } from "@dev.fast/json"; import { type ReviewDiffrConfig, + type ReviewDiffrLogin, type ReviewDiffrProvider, type ReviewDiffrSummarizerInput, type StructuralRegion, decodeStructuralDiffEvent, + reviewDiffrLoginsSchema, reviewDiffrSummarizerInputSchema, } from "@dev.fast/review-protocol"; import { stringify } from "smol-toml"; @@ -39,6 +41,7 @@ function serialized(operation: () => Promise): Promise { type DiffrArguments = | { command: "schema" } + | { command: "logins" } | { command: "show"; reveal: boolean } | { command: "set"; patch: JsonObject } | { command: "migrate" } @@ -48,6 +51,8 @@ function diffrArgv(args: DiffrArguments): string[] { switch (args.command) { case "schema": return ["config", "schema"]; + case "logins": + return ["config", "logins"]; case "show": return ["config", "show", "--json", ...(args.reveal ? ["--reveal"] : [])]; case "set": @@ -188,6 +193,32 @@ function environmentKey( .find((key) => !!key); } +function signedIn( + providers: ReviewDiffrProvider[], + logins: Record, + id: string, +): boolean { + const login = providers.find((provider) => provider.id === id)?.login; + + return !!login && !!logins[login]?.ready; +} + +/** Older diffr builds have no sign-ins. */ +async function readLogins( + rootPath?: string, + signal?: AbortSignal, +): Promise> { + try { + const result = reviewDiffrLoginsSchema.safeParse( + json(await diffr({ command: "logins" }, rootPath, { signal })), + ); + + return result.success ? result.data : {}; + } catch { + return {}; + } +} + function keyOptional( providers: ReviewDiffrProvider[], id: string, @@ -203,27 +234,30 @@ function strings(value: JsonValue | undefined): string[] { return Array.isArray(value) ? value.filter(isStringValue) : []; } +const summarizeSchema = + "properties.plugins.properties.shape.properties.bundled.properties.summarize.properties"; + +async function readSchema(rootPath?: string): Promise { + try { + return json(await diffr({ command: "schema" }, rootPath)); + } catch { + return undefined; + } +} + /** * Defaults supplied by diffr's schema for tag hiding and summary controls. * If the schema cannot be read, resolved settings still work. */ -async function configDefaults( - rootPath?: string, -): Promise< - Pick +function configDefaults( + schema: JsonObject | undefined, +): Pick< + ReviewDiffrConfig, + "providers" | "defaultPrompt" | "defaultHiddenTags" > { - let schema: JsonObject; + if (!schema) return {}; - try { - schema = json(await diffr({ command: "schema" }, rootPath)); - } catch { - return {}; - } - - const options = valueAt( - schema, - "properties.plugins.properties.shape.properties.bundled.properties.summarize.properties", - ); + const options = valueAt(schema, summarizeSchema); const defaultHiddenTags = strings( valueAt( @@ -246,6 +280,7 @@ async function configDefaults( const detail = isJsonObject(details) ? details[id] : undefined; const model = isJsonObject(models) ? models[id] : undefined; const endpoint = isJsonObject(detail) ? detail.endpoint : undefined; + const login = isJsonObject(detail) ? detail.login : undefined; return { id, @@ -255,6 +290,7 @@ async function configDefaults( keyVariables: isJsonObject(detail) ? strings(detail.key_variables) : [], keylessCustomEndpoint: isJsonObject(detail) && detail.keyless_custom_endpoint === true, + ...(isStringValue(login) && { login }), }; }); @@ -269,15 +305,19 @@ async function configDefaults( async function read(rootPath?: string): Promise { const config = await values(rootPath); - const defaults = await configDefaults(rootPath); + const defaults = configDefaults(await readSchema(rootPath)); + const logins = await readLogins(rootPath); + const providers = defaults.providers ?? []; const saved = valueAt(config, `${prefix}.api_key`); const credentialSource = isStringValue(saved) && saved.length > 0 ? "config" - : environmentKey(defaults.providers ?? [], savedProvider(config)) + : environmentKey(providers, savedProvider(config)) ? "environment" - : "missing"; + : signedIn(providers, logins, savedProvider(config)) + ? "login" + : "missing"; const plugins = valueAt(config, "plugins.shape"); @@ -290,7 +330,7 @@ async function read(rootPath?: string): Promise { if (isJsonObject(plugin)) delete plugin.api_key; } - return { values: config, credentialSource, ...defaults }; + return { values: config, credentialSource, logins, ...defaults }; } export function readDiffrConfig(rootPath?: string): Promise { @@ -368,12 +408,19 @@ export function saveDiffrSummarizer( !draft.apiKey && !(savedKey && !moving) && !environmentKey(providers, draft.provider) && + !signedIn(providers, current.logins ?? {}, draft.provider) && !keyOptional(providers, draft.provider, draft.endpoint) ) { + const login = providers.find( + (provider) => provider.id === draft.provider, + )?.login; + return { ...current, changed: false, - error: "Add an API key before enabling summaries.", + error: + (login && current.logins?.[login]?.message) || + "Add an API key before enabling summaries.", }; } @@ -421,7 +468,8 @@ export async function testDiffrSummarizer( throw new Error("The summarizer is not available in this configuration."); const { provider, endpoint, model, systemPrompt } = parsed.data; - const providers = (await configDefaults(rootPath)).providers ?? []; + const schema = await readSchema(rootPath); + const providers = configDefaults(schema).providers ?? []; const variables = providers.find((known) => known.id === provider)?.keyVariables ?? []; @@ -433,7 +481,11 @@ export async function testDiffrSummarizer( : "") || environmentKey(providers, provider); - if (!apiKey && !keyOptional(providers, provider, endpoint)) + if ( + !apiKey && + !keyOptional(providers, provider, endpoint) && + !signedIn(providers, await readLogins(rootPath, deadline), provider) + ) throw new Error("Add an API key to test summaries."); directory = await mkdtemp(join(tmpdir(), "review-summary-")); @@ -444,6 +496,12 @@ export async function testDiffrSummarizer( model, ...(systemPrompt.trim() && { system_prompt: systemPrompt }), min_lines: 1, + ...(schema && + isJsonObject( + valueAt(schema, `${summarizeSchema}.max_length_ratio`), + ) && { + max_length_ratio: 1000, + }), retries: 0, request_timeout_ms: 45_000, }; @@ -523,7 +581,7 @@ export async function testDiffrSummarizer( } throw new Error( - "No summary was produced. Check the API key, model, and network connection.", + "No summary was produced. Check the API key or sign-in, the model, and the network connection.", ); } finally { testing = false;