diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 3c35008..2adccc8 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -4,7 +4,11 @@ on: push: branches: [main, develop] pull_request: - branches: ["*"] + # `**` rather than `*`: in Actions branch filters `*` does not match `/`, + # so a pull request targeting any branch with a slash in its name -- every + # stacked pull request, and anything under a `feat/` or `fix/` prefix -- + # ran no checks at all. + branches: ["**"] env: CARGO_TERM_COLOR: always diff --git a/docs/audits/agent-compatibility-2026-09.md b/docs/audits/agent-compatibility-2026-09.md index 1732d78..f021f6d 100644 --- a/docs/audits/agent-compatibility-2026-09.md +++ b/docs/audits/agent-compatibility-2026-09.md @@ -72,21 +72,21 @@ or maintenance cost · **P2** = polish. ### 3.1 Defects to fix -**P0-1 — The Ollama provider cannot work as written.** +**P0-1 — The Ollama provider could not work as written.** ✅ **Fixed.** `src/ai/mod.rs:160-205` posts to `/api/chat` without `stream: false`. Ollama defaults to `stream: true`, so the response is newline-delimited JSON events, and `resp.json::()` will fail to deserialize on essentially every call. Every `--ai-provider ollama` run therefore ends in `LlmError::Parse` and silently falls back to the default message. Fix: send `stream: false`. -**P0-2 — Ollama JSON mode is a no-op.** +**P0-2 — Ollama JSON mode was a no-op.** ✅ **Fixed.** `format` is nested inside `options` (`src/ai/mod.rs:174-196`). Ollama reads `format` as a **top-level** request field, so JSON mode has never taken effect. Fix: hoist `format` to the request body, and prefer passing the `AiCommitSuggestion` JSON Schema rather than the string `"json"` — Ollama has supported schema-constrained decoding since 0.5. -**P0-3 — AI in default (safe) mode is given no signal at all.** +**P0-3 — AI in default (safe) mode was given no signal at all.** ✅ **Fixed.** `src/cli/commands/group_commit.rs:400-413`: unless `--ai-allow-sensitive` is set, the user prompt contains only the group name and the default type/short, and instructs the model to work "without revealing code or filenames". The model is @@ -96,20 +96,24 @@ real signal by default — path *shapes* (extension histogram, directory prefixe add/modify/delete counts, scope mappings from `.committy/config.toml`) — none of which is file content. Reserve `--ai-allow-sensitive` for actual diff hunks. -**P0-4 — `--ai` is silently ignored in `--mode apply`.** -The AI block lives only in the `"plan"` arm (`group_commit.rs:308-507`); the -`"apply"` arm (`:533+`) rebuilds groups from scratch with no LLM call. -`docs/src/content/docs/reference/ai-flags.mdx` documents -`--mode apply ... --ai --ai-provider ollama` as a working example. Either wire -apply to reuse the planned messages, or reject `--ai` with `--mode apply` and fix -the doc. Silently dropping a flag is the worst of the three options. +**P0-4 — WITHDRAWN. This finding was wrong.** ❌ +The original text claimed `--ai` was silently ignored in `--mode apply`. It is +not: the apply arm has its own AI block (`group_commit.rs:586`) with its own +`suggest_commit` calls. The error came from reading a `grep` run through +`awk 'NR>=530'`, whose renumbered output was mistaken for line numbers in the +plan arm. The documented `--mode apply ... --ai` example always worked. -**P1-5 — `--ai-diff-lines-per-file` is a dead flag.** +What is real in that area is **duplication**: `plan` and `apply` carried +byte-identical copies of both the group-building loop and the ~150-line AI +enrichment block. That duplication is what made the misreading possible. Both +are now extracted into `build_groups()` and `ai_user_prompt()` and shared. ✅ **Fixed.** + +**P1-5 — `--ai-diff-lines-per-file` was a dead flag.** ✅ **Fixed** (removed). Declared at `group_commit.rs:143-147` as `_ai_diff_lines_per_file` and never read. It is documented in `ai-flags.mdx` and accepted on the command line. Remove it, or implement it as part of P0-3. -**P1-6 — `ai-flags.mdx` overstates what is sent and understates what is not.** +**P1-6 — `ai-flags.mdx` overstated what is sent.** ✅ **Fixed.** It claims `--ai-allow-sensitive` "may include snippets or diff lines from code" — no diff is ever sent, in either mode. It also recommends `--ai-model openrouter/anthropic/claude-3.5-sonnet`, a model generation that is two @@ -250,14 +254,13 @@ steal both shorts. A test pins that trade-off. 0. ~~**P1-7**, **E-1**, **E-2**, **E-3**~~ — done: capabilities expanded with consent metadata, global flag placement, `invalid_usage` envelope, env var documented. See §3.5. -1. **P0-1, P0-2** — make Ollama actually function. -2. **P0-4, P1-5, P1-6, §3.3 removals** — stop documenting and accepting things - that do not happen. -3. **P0-3** — give the default-safe AI path real, non-sensitive signal. -4. **P1-8, P1-9, P1-10** — cross-tool portability (`AGENTS.md` canonical, +1. ~~**P0-1, P0-2, P0-3**, **P1-5**, **P1-6**~~ — done: Ollama functions, + the safe AI path carries redacted shape, dead flag and docs corrected. + **P0-4 was withdrawn as a false finding.** +2. **P1-8, P1-9, P1-10** — cross-tool portability (`AGENTS.md` canonical, `.agents/skills/`, spec-complete frontmatter). -5. **P1-12** — manifest and frontmatter tests so the step above cannot regress. -6. **P1-11, P2-13, P2-14, P2-15** — reach, CI cost, consistency, guardrail honesty. +3. **P1-12** — manifest and frontmatter tests so the step above cannot regress. +4. **P1-11, P2-13, P2-14, P2-15** — reach, CI cost, consistency, guardrail honesty. ## 5. Sources diff --git a/docs/src/content/docs/reference/ai-flags.mdx b/docs/src/content/docs/reference/ai-flags.mdx index f290cf4..8a4fcea 100644 --- a/docs/src/content/docs/reference/ai-flags.mdx +++ b/docs/src/content/docs/reference/ai-flags.mdx @@ -11,7 +11,7 @@ This page enumerates flags, best practices, and security considerations. ## Enabling AI -Add `--ai` to `group-commit`: +Add `--ai` to `group-commit`. It applies in both `--mode plan` and `--mode apply`: ```bash committy --non-interactive group-commit --mode plan --output json \ @@ -29,48 +29,80 @@ committy --non-interactive group-commit --mode plan --output json \ - `--ai-max-tokens ` — model tokens cap for responses - `--ai-temperature ` — creativity setting - `--ai-timeout-ms ` — request timeout -- `--no-ai-json-mode` — do not force JSON-mode (advanced; prefer default) +- `--no-ai-json-mode` — do not request structured output (advanced; prefer default) - `--ai-system-prompt ` — inline system prompt override - `--ai-system-prompt-file ` — load system prompt from file -- `--ai-file-limit ` — limit how many files are described to AI -- `--ai-allow-sensitive` — allow sending sensitive file content (off by default) +- `--ai-file-limit ` — cap how many paths are listed under `--ai-allow-sensitive` +- `--ai-allow-sensitive` — send real file paths instead of a redacted summary (off by default) + +## What is actually sent + +__No file content or diff text is ever sent__, in either mode. The difference +between the default and `--ai-allow-sensitive` is paths, not content. + +By default Committy sends a redacted *shape* of each group — how many files, +an extension histogram, and the top-level directories involved: + +```text +Group: code +Default type: feat +Default short: update source +Change shape (no filenames or content): 7 file(s); types: .rs x6, .toml x1; areas: src, tests +``` + +Filenames, paths below the first segment, and file contents are all withheld. +This is enough signal for a model to write a sensible subject line without the +provider learning your module, product, or customer names. + +With `--ai-allow-sensitive`, the group's file paths are listed instead, capped +by `--ai-file-limit` and elided with `...` beyond it. Use it only when you trust +the provider with your repository layout. ## Security & privacy -- __Sensitive content is not sent__ by default. The tool shares filenames and summarized context. -- Only pass `--ai-allow-sensitive` if required and you trust the provider. This may include snippets or diff lines from code. -- Prefer using an API key via env var name with `--ai-api-key-env` to avoid leaking keys in process lists or logs. -- Use `--ai-file-limit` to reduce context size and exposure. -- Consider on-premise providers (`ollama`) for maximum privacy. +- Prefer the default. The redacted summary is usually sufficient. +- Pass the API key by env var *name* with `--ai-api-key-env` so the secret never + appears in a process list, shell history, or CI log. +- Use `--ai-file-limit` to bound exposure when `--ai-allow-sensitive` is on. +- Consider an on-premise provider (`ollama`) when no data should leave the host. ## Failure handling -- AI suggestions are validated by Committy's linter. If invalid or timed out, Committy falls back to safe default messages per group. -- JSON parsing is strict; malformed outputs are rejected and replaced with defaults. +- AI suggestions are validated by Committy's linter. If a suggestion is invalid + or the request times out, Committy keeps the safe default message for that group + and records the reason in `errors`. +- Structured output is requested by default, and malformed responses are rejected + rather than committed. ## Provider examples ### OpenRouter ```bash -# export your key and reference by env var name +# export your key and reference it by env var name export OPENROUTER_API_KEY=sk-or-... committy --non-interactive group-commit --mode plan --output json \ --ai --ai-provider openrouter \ - --ai-model openrouter/anthropic/claude-3.5-sonnet \ + --ai-model openrouter/anthropic/claude-sonnet-4.5 \ --ai-api-key-env OPENROUTER_API_KEY \ --ai-max-tokens 600 --ai-temperature 0.2 ``` +JSON mode sends `response_format: {"type": "json_object"}`. + ### Ollama (local) ```bash -# ensure Ollama is running locally; default base URL is http://127.0.0.1:11434 +# ensure Ollama is running locally; default base URL is http://localhost:11434 # no API key needed for local Ollama committy --non-interactive group-commit --mode apply --output json \ --ai --ai-provider ollama \ --ai-model llama3.1:8b \ - --ai-timeout-ms 30000 --ai-file-limit 64 + --ai-timeout-ms 30000 ``` + +Requests to Ollama set `stream: false` and pass a JSON Schema in the top-level +`format` field, so the model's output is constrained to the expected object +rather than merely asked for as JSON. diff --git a/src/ai/mod.rs b/src/ai/mod.rs index 4dfdf83..d245779 100644 --- a/src/ai/mod.rs +++ b/src/ai/mod.rs @@ -1,4 +1,4 @@ -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use std::time::Duration; use thiserror::Error; @@ -8,8 +8,6 @@ pub enum LlmError { NotConfigured, #[error("AI provider request failed: {0}")] RequestFailed(String), - #[error("AI provider timeout")] - _Timeout, #[error("AI response parse error: {0}")] Parse(String), } @@ -27,12 +25,79 @@ pub trait LlmClient: Send + Sync { ) -> Result; } -// #[derive(Debug, Clone, Copy, Serialize, Deserialize)] -// #[serde(rename_all = "kebab-case")] -// pub enum LlmProvider { -// OpenRouter, -// Ollama, -// } +/// JSON Schema describing [`AiCommitSuggestion`]. +/// +/// Ollama constrains decoding to a supplied schema, which is stricter than +/// asking for "some JSON" and removes most parse failures. +fn suggestion_schema() -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "commit_type": {"type": "string"}, + "short": {"type": "string"}, + "scope": {"type": "string"}, + "long": {"type": "string"}, + "message": {"type": "string"}, + }, + }) +} + +fn chat_messages(system_prompt: &str, user_prompt: &str) -> serde_json::Value { + serde_json::json!([ + {"role": "system", "content": system_prompt}, + {"role": "user", "content": user_prompt}, + ]) +} + +/// Build the OpenRouter `/chat/completions` request body. +fn openrouter_request_body( + model: &str, + system_prompt: &str, + user_prompt: &str, + json_mode: bool, + max_tokens: u32, + temperature: f32, +) -> serde_json::Value { + let mut body = serde_json::json!({ + "model": model, + "messages": chat_messages(system_prompt, user_prompt), + "max_tokens": max_tokens, + "temperature": temperature, + }); + if json_mode { + body["response_format"] = serde_json::json!({"type": "json_object"}); + } + body +} + +/// Build the Ollama `/api/chat` request body. +/// +/// Two details are load-bearing and were wrong before: `stream` must be false +/// (the endpoint defaults to true and would return newline-delimited events +/// that cannot deserialize as one object), and `format` is a **top-level** +/// field, not an entry under `options`. +fn ollama_request_body( + model: &str, + system_prompt: &str, + user_prompt: &str, + json_mode: bool, + max_tokens: u32, + temperature: f32, +) -> serde_json::Value { + let mut body = serde_json::json!({ + "model": model, + "messages": chat_messages(system_prompt, user_prompt), + "stream": false, + "options": { + "temperature": temperature, + "num_predict": max_tokens as i32, + }, + }); + if json_mode { + body["format"] = suggestion_schema(); + } + body +} pub struct OpenRouterClient { pub base_url: String, @@ -63,42 +128,14 @@ impl LlmClient for OpenRouterClient { let url = format!("{}/chat/completions", self.base_url.trim_end_matches('/')); - #[derive(Serialize)] - struct Message<'a> { - role: &'a str, - content: &'a str, - } - #[derive(Serialize)] - struct RequestBody<'a> { - model: &'a str, - messages: Vec>, - max_tokens: u32, - temperature: f32, - #[serde(skip_serializing_if = "Option::is_none")] - response_format: Option, - } - let response_format = if json_mode { - Some(serde_json::json!({"type": "json_object"})) - } else { - None - }; - - let body = RequestBody { - model: &self.model, - messages: vec![ - Message { - role: "system", - content: system_prompt, - }, - Message { - role: "user", - content: user_prompt, - }, - ], + let body = openrouter_request_body( + &self.model, + system_prompt, + user_prompt, + json_mode, max_tokens, temperature, - response_format, - }; + ); let client = reqwest::Client::builder() .timeout(Duration::from_millis(timeout_ms)) @@ -159,43 +196,14 @@ impl LlmClient for OllamaClient { ) -> Result { let url = format!("{}/api/chat", self.base_url.trim_end_matches('/')); - #[derive(Serialize)] - struct Message<'a> { - role: &'a str, - content: &'a str, - } - #[derive(Serialize)] - struct RequestBody<'a> { - model: &'a str, - messages: Vec>, - options: OllamaOptions, - } - #[derive(Serialize)] - struct OllamaOptions { - temperature: f32, - num_predict: i32, - #[serde(skip_serializing_if = "Option::is_none")] - format: Option<&'static str>, - } - - let body = RequestBody { - model: &self.model, - messages: vec![ - Message { - role: "system", - content: system_prompt, - }, - Message { - role: "user", - content: user_prompt, - }, - ], - options: OllamaOptions { - temperature, - num_predict: max_tokens as i32, - format: if json_mode { Some("json") } else { None }, - }, - }; + let body = ollama_request_body( + &self.model, + system_prompt, + user_prompt, + json_mode, + max_tokens, + temperature, + ); let client = reqwest::Client::builder() .timeout(Duration::from_millis(timeout_ms)) @@ -239,3 +247,73 @@ pub struct AiCommitSuggestion { pub long: Option, pub message: Option, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ollama_body_disables_streaming() { + let body = ollama_request_body("llama3.2", "sys", "user", true, 256, 0.2); + assert_eq!( + body["stream"], + serde_json::Value::Bool(false), + "Ollama defaults /api/chat to stream:true; a streaming response is \ + newline-delimited and will not deserialize as one object" + ); + } + + #[test] + fn ollama_body_puts_format_at_the_top_level() { + let body = ollama_request_body("llama3.2", "sys", "user", true, 256, 0.2); + assert!( + body["format"].is_object(), + "Ollama reads `format` as a top-level field; nesting it under `options` \ + silently disables constrained decoding. Got: {body}" + ); + assert!( + body["options"]["format"].is_null(), + "`format` must not also be left inside `options`" + ); + } + + #[test] + fn ollama_body_constrains_to_the_suggestion_schema() { + let body = ollama_request_body("llama3.2", "sys", "user", true, 256, 0.2); + let props = &body["format"]["properties"]; + for field in ["commit_type", "short", "scope", "long", "message"] { + assert_eq!( + props[field]["type"], + serde_json::Value::String("string".into()), + "schema must describe AiCommitSuggestion::{field}" + ); + } + } + + #[test] + fn ollama_body_omits_format_when_json_mode_is_off() { + let body = ollama_request_body("llama3.2", "sys", "user", false, 256, 0.2); + assert!(body["format"].is_null()); + assert_eq!(body["stream"], serde_json::Value::Bool(false)); + } + + #[test] + fn ollama_body_carries_sampling_options() { + let body = ollama_request_body("llama3.2", "sys", "user", true, 512, 0.7); + assert_eq!(body["options"]["num_predict"], serde_json::json!(512)); + assert_eq!(body["model"], serde_json::json!("llama3.2")); + assert_eq!(body["messages"][0]["role"], serde_json::json!("system")); + assert_eq!(body["messages"][1]["content"], serde_json::json!("user")); + } + + #[test] + fn openrouter_body_requests_json_object_when_asked() { + let body = openrouter_request_body("m", "sys", "user", true, 256, 0.2); + assert_eq!( + body["response_format"]["type"], + serde_json::json!("json_object") + ); + let plain = openrouter_request_body("m", "sys", "user", false, 256, 0.2); + assert!(plain["response_format"].is_null()); + } +} diff --git a/src/cli/commands/group_commit.rs b/src/cli/commands/group_commit.rs index c13e869..20d540e 100644 --- a/src/cli/commands/group_commit.rs +++ b/src/cli/commands/group_commit.rs @@ -142,10 +142,6 @@ pub struct GroupCommitCommand { #[structopt(long = "ai-file-limit", default_value = "20")] ai_file_limit: usize, - /// Diff lines per file sent to AI - #[structopt(long = "ai-diff-lines-per-file", default_value = "80")] - _ai_diff_lines_per_file: usize, - /// Allow sending sensitive content to external AI providers #[structopt(long = "ai-allow-sensitive")] ai_allow_sensitive: bool, @@ -181,7 +177,6 @@ impl Default for GroupCommitCommand { ai_system_prompt: None, ai_system_prompt_file: None, ai_file_limit: 20, - _ai_diff_lines_per_file: 80, ai_allow_sensitive: false, git_config: vec![], repo_path: PathBuf::from("."), @@ -318,41 +313,7 @@ impl Command for GroupCommitCommand { })?; let _git_command_config = crate::git::resolve_git_command_config(repo_path, &self.git_config)?; - let mut by_group: std::collections::BTreeMap> = [ - (GroupName::Docs, vec![]), - (GroupName::Tests, vec![]), - (GroupName::Ci, vec![]), - (GroupName::Deps, vec![]), - (GroupName::Build, vec![]), - (GroupName::Chore, vec![]), - (GroupName::Code, vec![]), - ] - .into_iter() - .collect(); - - for f in files { - let g = classify_file(&f); - if let Some(v) = by_group.get_mut(&g) { - v.push(f); - } - } - - let mut groups: Vec = Vec::new(); - for (name, files) in by_group.into_iter() { - if files.is_empty() { - continue; - } - let commit_type = default_type_for(&name).to_string(); - let short = default_short_for(&name).to_string(); - let message = format_commit_message(&commit_type, false, "", &short, ""); - groups.push(PlanGroup { - name, - commit_type, - files, - suggested_message: message, - issues: None, - }); - } + let mut groups = build_groups(files); let mut errors: Vec = Vec::new(); @@ -383,38 +344,14 @@ impl Command for GroupCommitCommand { .map_err(|e| CliError::Generic(e.to_string()))?; for g in groups.iter_mut() { - let files_preview: Vec = if self.ai_allow_sensitive { - let mut v = g - .files - .iter() - .take(self.ai_file_limit) - .cloned() - .collect::>(); - if g.files.len() > self.ai_file_limit { - v.push("...".to_string()); - } - v - } else { - vec![] - }; - let user_prompt = if self.ai_allow_sensitive { - format!( - "Group: {}\nDefault type: {}\nDefault short: {}\nFiles (truncated):\n- {}\nReturn a JSON object with fields: commit_type, short, scope, long, message.", - group_name_str(g.name), - g.commit_type, - default_short_for(&g.name), - files_preview.join("\n- ") - ) - } else { - format!( - "Group: {}\nDefault type: {}\nDefault short: {}\nWithout revealing code or filenames, suggest a better short description if needed. Return JSON.", - group_name_str(g.name), - g.commit_type, - default_short_for(&g.name) - ) - }; + let user_prompt = ai_user_prompt( + &g.name, + &g.commit_type, + &g.files, + self.ai_allow_sensitive, + self.ai_file_limit, + ); - // Prepare client per provider let result: Result = match provider { "openrouter" => { let base = self @@ -544,41 +481,7 @@ impl Command for GroupCommitCommand { crate::git::resolve_git_command_config(repo_path, &self.git_config)?; // Build groups as in plan let files = list_changed_files_from(&self.repo_path, self.include_unstaged)?; - let mut by_group: std::collections::BTreeMap> = [ - (GroupName::Docs, vec![]), - (GroupName::Tests, vec![]), - (GroupName::Ci, vec![]), - (GroupName::Deps, vec![]), - (GroupName::Build, vec![]), - (GroupName::Chore, vec![]), - (GroupName::Code, vec![]), - ] - .into_iter() - .collect(); - - for f in files { - let g = classify_file(&f); - if let Some(v) = by_group.get_mut(&g) { - v.push(f); - } - } - - let mut groups: Vec = Vec::new(); - for (name, files) in by_group.into_iter() { - if files.is_empty() { - continue; - } - let commit_type = default_type_for(&name).to_string(); - let short = default_short_for(&name).to_string(); - let message = format_commit_message(&commit_type, false, "", &short, ""); - groups.push(PlanGroup { - name, - commit_type, - files, - suggested_message: message, - issues: None, - }); - } + let mut groups = build_groups(files); let mut errors: Vec = Vec::new(); @@ -608,32 +511,13 @@ impl Command for GroupCommitCommand { .map_err(|e| CliError::Generic(e.to_string()))?; for g in groups.iter_mut() { - let files_preview: Vec = if self.ai_allow_sensitive { - let mut v = g - .files - .iter() - .take(self.ai_file_limit) - .cloned() - .collect::>(); - if g.files.len() > self.ai_file_limit { - v.push("...".to_string()); - } - v - } else { - vec![] - }; - - let user_prompt = if self.ai_allow_sensitive { - format!( - "Group: {}\nDefault type: {}\nDefault short: {}\nFiles (truncated):\n- {}\nReturn a JSON object with fields: commit_type, short, scope, long, message.", - group_name_str(g.name), g.commit_type, default_short_for(&g.name), files_preview.join("\n- ") - ) - } else { - format!( - "Group: {}\nDefault type: {}\nDefault short: {}\nWithout revealing code or filenames, suggest a better short description if needed. Return JSON.", - group_name_str(g.name), g.commit_type, default_short_for(&g.name) - ) - }; + let user_prompt = ai_user_prompt( + &g.name, + &g.commit_type, + &g.files, + self.ai_allow_sensitive, + self.ai_file_limit, + ); let result: Result = match provider { "openrouter" => { @@ -889,6 +773,129 @@ impl Command for GroupCommitCommand { } } +/// Summarise a change set without naming any file. +/// +/// The default (non-sensitive) AI path used to send only the group name, which +/// gave the model nothing to improve on. This sends *shape* — how many files, +/// which extensions, which top-level areas — and never a filename, a path +/// below the first segment, or any file content. +fn redacted_shape_summary(files: &[String]) -> String { + use std::collections::BTreeMap; + + let mut extensions: BTreeMap = BTreeMap::new(); + let mut areas: std::collections::BTreeSet = std::collections::BTreeSet::new(); + + for file in files { + let trimmed = file.trim_start_matches("./"); + let extension = std::path::Path::new(trimmed) + .extension() + .and_then(|e| e.to_str()) + .map(|e| format!(".{e}")) + .unwrap_or_else(|| "(no extension)".to_string()); + *extensions.entry(extension).or_insert(0) += 1; + + // First path segment only: "src", "docs", "tests". Anything deeper can + // carry product or customer names. + let area = trimmed + .split('/') + .next() + .filter(|segment| !segment.is_empty() && trimmed.contains('/')) + .unwrap_or("(repository root)"); + areas.insert(area.to_string()); + } + + let extension_list = extensions + .iter() + .map(|(extension, count)| format!("{extension} x{count}")) + .collect::>() + .join(", "); + + format!( + "{} file(s); types: {}; areas: {}", + files.len(), + extension_list, + areas.into_iter().collect::>().join(", ") + ) +} + +/// The per-group user prompt sent to the model. +/// +/// `allow_sensitive` is the only switch that lets real paths leave the machine. +fn ai_user_prompt( + name: &GroupName, + commit_type: &str, + files: &[String], + allow_sensitive: bool, + file_limit: usize, +) -> String { + let header = format!( + "Group: {}\nDefault type: {}\nDefault short: {}", + group_name_str(*name), + commit_type, + default_short_for(name) + ); + + if allow_sensitive { + let mut listed: Vec = files.iter().take(file_limit).cloned().collect(); + if files.len() > file_limit { + listed.push("...".to_string()); + } + format!( + "{header}\nFiles (truncated):\n- {}\nReturn a JSON object with fields: \ + commit_type, short, scope, long, message.", + listed.join("\n- ") + ) + } else { + format!( + "{header}\nChange shape (no filenames or content): {}\nSuggest a better short \ + description for this group. Return a JSON object with fields: commit_type, \ + short, scope, long, message.", + redacted_shape_summary(files) + ) + } +} + +/// Classify a change set into commit groups. +/// +/// Shared by `plan` and `apply`; they previously carried byte-identical copies. +fn build_groups(files: Vec) -> Vec { + let mut by_group: std::collections::BTreeMap> = [ + (GroupName::Docs, vec![]), + (GroupName::Tests, vec![]), + (GroupName::Ci, vec![]), + (GroupName::Deps, vec![]), + (GroupName::Build, vec![]), + (GroupName::Chore, vec![]), + (GroupName::Code, vec![]), + ] + .into_iter() + .collect(); + + for file in files { + let group = classify_file(&file); + if let Some(bucket) = by_group.get_mut(&group) { + bucket.push(file); + } + } + + by_group + .into_iter() + .filter(|(_, files)| !files.is_empty()) + .map(|(name, files)| { + let commit_type = default_type_for(&name).to_string(); + let short = default_short_for(&name).to_string(); + let suggested_message = format_commit_message(&commit_type, false, "", &short, ""); + PlanGroup { + name, + commit_type, + files, + suggested_message, + issues: None, + } + }) + .collect() +} + fn validate_group_messages( repo_path: &Path, groups: &mut [PlanGroup], @@ -912,3 +919,85 @@ fn validate_group_messages( Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + fn files() -> Vec { + vec![ + "src/cli/commands/secret_feature.rs".to_string(), + "src/cli/commands/other.rs".to_string(), + "docs/internal/roadmap.md".to_string(), + ] + } + + #[test] + fn shape_summary_describes_without_naming() { + let summary = redacted_shape_summary(&files()); + + assert!( + summary.contains('3'), + "must report the file count: {summary}" + ); + assert!(summary.contains(".rs"), "must report extensions: {summary}"); + assert!(summary.contains(".md"), "must report extensions: {summary}"); + assert!( + summary.contains("src"), + "must report top-level areas: {summary}" + ); + assert!( + summary.contains("docs"), + "must report top-level areas: {summary}" + ); + + for leaked in ["secret_feature", "other.rs", "roadmap.md", "internal"] { + assert!( + !summary.contains(leaked), + "shape summary leaked `{leaked}`: {summary}" + ); + } + } + + #[test] + fn default_prompt_carries_shape_but_no_paths() { + let prompt = ai_user_prompt(&GroupName::Code, "feat", &files(), false, 20); + + assert!( + prompt.contains(&redacted_shape_summary(&files())), + "the safe prompt must include the redacted shape: {prompt}" + ); + for leaked in ["secret_feature", "roadmap.md", "src/cli/commands"] { + assert!(!prompt.contains(leaked), "safe prompt leaked `{leaked}`"); + } + } + + #[test] + fn sensitive_prompt_lists_paths_and_respects_the_limit() { + let prompt = ai_user_prompt(&GroupName::Code, "feat", &files(), true, 2); + + assert!(prompt.contains("src/cli/commands/secret_feature.rs")); + assert!( + prompt.contains("..."), + "over-limit file lists must be elided: {prompt}" + ); + assert!( + !prompt.contains("docs/internal/roadmap.md"), + "third path exceeds the limit of 2" + ); + } + + #[test] + fn build_groups_is_shared_between_plan_and_apply() { + let groups = build_groups(files()); + + let names: Vec = groups.iter().map(|g| g.name).collect(); + assert!(names.contains(&GroupName::Code)); + assert!(names.contains(&GroupName::Docs)); + assert!( + groups.iter().all(|g| !g.files.is_empty()), + "empty groups must be dropped" + ); + assert!(groups.iter().all(|g| !g.suggested_message.is_empty())); + } +}