From f8a8083253097c28f4925d9cf2f86f5df82f5098 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Sun, 31 May 2026 22:06:39 +0700 Subject: [PATCH 1/5] feat: config redesign + severity comparison fix - config set now writes YAML (not unread TOML) - support project (.cora.yaml) and global (~/.cora/config.yaml) config - config set --global flag for global config - add base_url to config set keys - auth writes to separate ~/.cora/auth.toml (0o600) - config show displays resolved config with priority chain - skip_serializing_if on all Option fields (no more null in YAML) - fix severity comparison: Ord is Critical(0)= - fix should_block in engine/review.rs and commands/scan.rs - fix display filter in commands/review.rs (consistent direction) - add config set --help, --global flag - design docs: CONFIG-REDESIGN.md, FEATURE-DESIGN.md --- Cargo.lock | 2 +- docs/CONFIG-REDESIGN.md | 166 ++++++++++++++++++++++++++++++++ docs/FEATURE-DESIGN.md | 167 +++++++++++++++++++++++++++++++++ src/commands/auth.rs | 5 +- src/commands/config_cmd.rs | 95 ++++++++----------- src/commands/review.rs | 3 + src/commands/scan.rs | 4 +- src/config/loader.rs | 187 ++++++++++++++++++++++++++++++++++--- src/config/schema.rs | 16 ++++ src/engine/review.rs | 7 +- src/main.rs | 9 +- 11 files changed, 584 insertions(+), 77 deletions(-) create mode 100644 docs/CONFIG-REDESIGN.md create mode 100644 docs/FEATURE-DESIGN.md diff --git a/Cargo.lock b/Cargo.lock index 129554a..91956ef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -231,7 +231,7 @@ dependencies = [ [[package]] name = "cora-cli" -version = "0.1.1" +version = "0.1.2" dependencies = [ "anyhow", "assert_cmd", diff --git a/docs/CONFIG-REDESIGN.md b/docs/CONFIG-REDESIGN.md new file mode 100644 index 0000000..0300e14 --- /dev/null +++ b/docs/CONFIG-REDESIGN.md @@ -0,0 +1,166 @@ +# Config System Redesign: Global + Project Config + +## Status: Draft — Pending Approval + +## Problem + +Current `cora config set` writes to `~/.cora/config.toml` but the config loader only reads `.cora.yaml`. Values set via `config set` are silently ignored. Additionally, there's no way to set global defaults across all projects. + +## Goal + +Support both global and per-project configuration with clear priority: + +``` +CLI flags → CORA_* env vars → .cora.yaml (project) → ~/.cora/config.yaml (global) → Defaults +``` + +## Design + +### Config Sources + +| Source | Path | Format | Scope | Priority | +|--------|------|--------|-------|----------| +| **Global** | `~/.cora/config.yaml` | YAML | All projects | 4 (lowest, after env) | +| **Project** | `.cora.yaml` (walk parent dirs) | YAML | Current project | 3 | +| **Env vars** | `CORA_PROVIDER`, `CORA_MODEL`, `CORA_BASE_URL`, `CORA_API_KEY`, etc. | — | Current shell | 2 | +| **CLI flags** | `--provider`, `--model`, `--base-url`, etc. | — | Current command | 1 (highest) | + +### Merge Strategy + +Same as current `.cora.yaml` merge — global config is loaded first (into defaults), then project config overwrites any fields present. Each field is independently resolved. + +Example: + +**Global** (`~/.cora/config.yaml`): +```yaml +provider: + provider: openai + model: gpt-4o-mini +output: + format: compact +``` + +**Project** (`.cora.yaml`): +```yaml +provider: + model: glm-5.1 + base_url: http://litellm:4000 +hook: + mode: block +``` + +**Resolved:** +```yaml +provider: + provider: openai # from global + model: glm-5.1 # project overrides + base_url: http://litellm:4000 # project +output: + format: compact # from global +hook: + mode: block # from project + min_severity: major # default +``` + +### `cora config set` Redesign + +```bash +# Default: write to project .cora.yaml (must be inside a git project) +cora config set model glm-5.1 +cora config set base_url http://litellm:4000 + +# Explicit global +cora config set --global model gpt-4o-mini +cora config set --global provider openai + +# All supported keys: +# model, provider, base_url, format, severity (maps to hook.min_severity) +``` + +Behavior: +- **No `--global`**: writes to `.cora.yaml` in current dir (creates if missing) +- **`--global`**: writes to `~/.cora/config.yaml` (creates if missing) +- **`--global --show`**: prints global config path + contents +- Works with the same TOML→YAML key mapping as before + +### Auth File (Separate Concern) + +API key storage stays separate at `~/.cora/auth.toml` (rename from `config.toml`): + +- `cora auth login` → `~/.cora/auth.toml` (contains only `api_key`) +- `cora auth status` → reads from `~/.cora/auth.toml` +- `cora auth remove` → deletes `~/.cora/auth.toml` + +This separation means: +- Config = `~/.cora/config.yaml` (global settings, no secrets) +- Auth = `~/.cora/auth.toml` (API key only, permissions 0o600) + +### `cora config show` Enhancement + +```bash +$ cora config show + +╔══════════════════════════════════════════╗ +║ Current Configuration ║ +╚══════════════════════════════════════════╝ + + Config sources (priority ↓): + CLI flags — + Env vars — + Project config .cora.yaml ✅ + Global config ~/.cora/config.yaml ✅ + +provider: + provider: openai (global) + model: glm-5.1 (project) + base_url: http://litellm:4000 (project) + +focus: + security, performance, bugs (default) + +hook: + mode: block (project) + min_severity: major (default) + max_diff_size: 51200 (default) + +output: + format: compact (global) + color: true (default) +``` + +Each value shows its source — makes debugging config priority transparent. + +## Files to Change + +| File | Change | +|------|--------| +| `src/config/loader.rs` | Add `load_global_config()` → reads `~/.cora/config.yaml`. Update `load_config()` to merge: global → project → CLI. Rename `AUTH_FILENAME` to `auth.toml`. | +| `src/commands/config_cmd.rs` | Rewrite `execute_config_set()`: write YAML instead of TOML. Add `--global` flag support. Add `base_url` key. Keep `cora config show` but add source annotations. | +| `src/main.rs` | Add `--global` flag to `config set` subcommand. Add `config set base_url `. | +| `src/config/schema.rs` | No changes needed — `CoraFile` schema already supports all fields. Global uses same `CoraFile` struct. | + +## Migration + +- **No breaking change** — existing `.cora.yaml` files work as-is +- `~/.cora/config.toml` (if exists) → auto-migrate to `~/.cora/config.yaml` on first run (read TOML, write YAML, delete TOML). For auth keys: rename `config.toml` → `auth.toml` +- `cora auth login` switches from writing `config.toml` to `auth.toml` + +## Effort Estimate + +| Item | LOC (est.) | Effort | +|------|-----------|--------| +| Global config loader | ~40 | 30 min | +| `config set` rewrite (YAML + `--global` + `base_url`) | ~80 | 1h | +| `config show` source annotations | ~40 | 30 min | +| Auth file rename (`config.toml` → `auth.toml`) | ~20 | 15 min | +| Migration logic (TOML → YAML) | ~30 | 30 min | +| Tests | ~60 | 45 min | +| **Total** | ~270 | **~3.5h** | + +## Risks + +| Risk | Mitigation | +|------|-----------| +| YAML serialization changes formatting | Use `serde_yaml_ng` with consistent output | +| Migration edge case (partial TOML) | Log warning, skip gracefully, user can manually edit | +| Two config sources confuse users | `config show` annotates source for each value | diff --git a/docs/FEATURE-DESIGN.md b/docs/FEATURE-DESIGN.md new file mode 100644 index 0000000..3bab445 --- /dev/null +++ b/docs/FEATURE-DESIGN.md @@ -0,0 +1,167 @@ +# Feature Design: `cora init --interactive` & `cora doctor` + +## Status: Draft — Pending Review + +## Overview + +Two new features to improve developer experience: + +1. **`cora init --interactive`** (or `cora init -i`) — guided, step-by-step config creation with prompts +2. **`cora doctor`** — health check command that validates API connectivity, config, and environment + +--- + +## 1. `cora init --interactive` + +### Current Behavior + +`cora init` generates a static `.cora.yaml` template with hardcoded defaults (openai, gpt-4o-mini). User must manually edit the file to customize. + +CLI flags (`--provider`, `--model`, `--base-url`, `--api-key`) allow one-shot config but are non-interactive. + +### Proposed Behavior + +With `--interactive` (or `-i`), `cora init` becomes a guided wizard: + +``` +$ cora init -i + +🚀 Welcome to Cora! Let's set up your project. + +? Select a provider: + > OpenAI + Anthropic + Groq + Ollama (local) + Google + Custom (OpenAI-compatible) + +? Model name (default: gpt-4o-mini): glm-5.1 + +? Base URL (default: https://api.openai.com/v1): https://litellm:4000 + +? API key: **** (or leave blank to use CORA_API_KEY env var) + +? Review focus areas (space to toggle, enter to confirm): + [✓] security + [✓] performance + [ ] bugs + [ ] best_practice + [ ] style + [ ] maintainability + +? Pre-commit hook mode: + > warn (print findings but allow commit) + block (fail commit on findings above threshold) + +✅ Created .cora.yaml with your settings. +✅ Pre-commit hook installed. + +Next steps: + cora review # Review staged changes + cora scan # Scan the full project +``` + +### Implementation Approach + +- **No new dependencies** — use `dialoguer` (already in Cargo.toml? check) or `inquire`. If neither exists, use minimal stdin/stdout with `rustyline` or basic `io::stdin()`. +- **Fallback** — if `--interactive` is not passed, current behavior is preserved (static template). +- **Flags take precedence** — `cora init -i --provider openai --model gpt-4o` skips the provider/model prompts. + +### Config Schema Changes + +None — output is the same `.cora.yaml` format, just populated interactively. + +### Open Questions + +1. **Which prompt library?** — `dialoguer` (lightweight), `inquire` (feature-rich), or raw stdin? +2. **Should it test API connectivity** after setup? (e.g., send a minimal request to validate key) +3. **Should it detect existing API keys** from env vars and pre-select the provider? + +--- + +## 2. `cora doctor` + +### Purpose + +Diagnostic command that checks the entire cora setup — config, API connectivity, git state, environment. + +### Proposed Output + +``` +$ cora doctor + +🔍 Cora Doctor — System Health Check + +Config + ✅ .cora.yaml found at /project/.cora.yaml + ✅ Config schema valid + +API Connectivity + ✅ Provider: openai (gpt-4o-mini) + ✅ API key detected (CORA_API_KEY) + ✅ Connection OK — 142ms latency + ✅ Model accessible + +Git + ✅ Inside a git repository + ✅ Upstream detected: origin/main + ⚠️ 3 uncommitted changes detected + +Environment + ✅ Rust version: 1.85.0 + ✅ cora version: 0.1.2 + ✅ Shell completions available: bash, zsh, fish, powershell + +Pre-commit Hook + ✅ Installed at .git/hooks/pre-commit + +Result: 7 passed, 1 warning — your setup is ready! 🚀 +``` + +### Checks (ordered) + +| # | Check | Severity | What it does | +|---|-------|----------|-------------| +| 1 | Config file found | error | Looks for `.cora.yaml` or `~/.config/cora/config.yaml` | +| 2 | Config schema valid | error | Parses YAML, validates against schema | +| 3 | API key available | error | Checks `CORA_API_KEY` or provider-specific env vars | +| 4 | Provider configured | error | Has a valid provider name | +| 5 | API connectivity | error | Sends a minimal request (e.g., list models) to validate key + endpoint | +| 6 | API latency | info | Measures round-trip time to API endpoint | +| 7 | Model accessible | warn | Validates the configured model exists at the endpoint | +| 8 | Git repository | warn | Checks if inside a git repo (not required for `cora scan --path`) | +| 9 | Pre-commit hook | info | Checks if `.git/hooks/pre-commit` has cora | +| 10 | Cora version | info | Shows current version, checks for updates | + +### Implementation Approach + +- **New command** — `src/commands/doctor.rs` +- **New subcommand** in `main.rs` CLI definition +- **Check result type** — enum `{ Ok, Warning, Error }` with message +- **Optional `--json` flag** — machine-readable output for CI +- **Exit code** — 0 if all pass, 1 if any error, 2 if connection failed + +### Open Questions + +1. **Check for updates** — should `cora doctor` hit GitHub API to check latest version? (adds network dependency) +2. **Model validation** — how to check if a model exists without expensive API calls? (Some providers support `GET /models`) +3. **Should it fix things?** — e.g., `cora doctor --fix` to auto-install hook, fix config schema? + +--- + +## Effort Estimate + +| Feature | LOC (est.) | New Files | Effort | Risk | +|---------|-----------|-----------|--------|------| +| `cora init -i` | ~150-250 | 1 (modify `init.rs`) | 2-3h | Low | +| `cora doctor` | ~200-300 | 2 (`doctor.rs`, types) | 3-4h | Low | + +## Dependencies + +- `dialoguer` or `inquire` crate (for interactive prompts) — whichever is lighter +- No new deps for `cora doctor` — uses existing HTTP client + +## Timeline + +Both features can go into **v0.2.0** (next minor release) or as individual patches. diff --git a/src/commands/auth.rs b/src/commands/auth.rs index 8fda6e0..3654822 100644 --- a/src/commands/auth.rs +++ b/src/commands/auth.rs @@ -38,10 +38,7 @@ pub fn execute_auth_login() -> Result<()> { } loader::save_api_key(&key)?; - println!( - "{} API key saved to ~/.cora/config.toml", - "✅".green().bold() - ); + println!("{} API key saved to ~/.cora/auth.toml", "✅".green().bold()); println!( "{}", " This file is local to your machine and not committed to git.".dimmed() diff --git a/src/commands/config_cmd.rs b/src/commands/config_cmd.rs index e02c116..8df3755 100644 --- a/src/commands/config_cmd.rs +++ b/src/commands/config_cmd.rs @@ -4,6 +4,7 @@ use anyhow::{Context, Result}; use colored::Colorize; use crate::config::loader; +use crate::config::schema::{CoraFile, HookSection, OutputSection, ProviderSection}; /// Execute `cora config show` — print the current resolved configuration. pub fn execute_config_show() -> Result<()> { @@ -95,94 +96,78 @@ pub fn execute_config_show() -> Result<()> { Ok(()) } -/// Execute `cora config set ` — write a key-value pair to ~/.cora/config.toml. +/// Execute `cora config set [--global] ` — write a key-value pair +/// to a YAML config file. /// -/// Supported keys: model, provider, format, severity. -pub fn execute_config_set(key: &str, value: &str) -> Result<()> { +/// Supported keys: model, provider, base_url, format, severity +pub fn execute_config_set(key: &str, value: &str, global: bool) -> Result<()> { // Validate the key match key { - "model" | "provider" | "format" | "severity" => {} + "model" | "provider" | "base_url" | "format" | "severity" => {} _ => { anyhow::bail!( - "unsupported key: {key}\nSupported keys: model, provider, format, severity" + "unsupported key: {key}\nSupported keys: model, provider, base_url, format, severity" ); } } - let dir = cora_config_dir()?; - std::fs::create_dir_all(&dir).with_context(|| format!("failed to create {}", dir.display()))?; - - let path = dir.join("config.toml"); + // Determine target path + let path = if global { + let dir = loader::cora_dir()?; + std::fs::create_dir_all(&dir) + .with_context(|| format!("failed to create {}", dir.display()))?; + dir.join("config.yaml") + } else { + PathBuf::from(".cora.yaml") + }; - // Load existing config.toml content if it exists - let mut table = if path.is_file() { + // Load existing file or start fresh + let mut cora = if path.is_file() { let content = std::fs::read_to_string(&path) .with_context(|| format!("failed to read {}", path.display()))?; - content.parse::().unwrap_or_default() + CoraFile::from_str(&content).unwrap_or_default() } else { - toml::Table::new() + CoraFile::default() }; - // Map the key to the appropriate TOML structure + // Apply the key update match key { "model" => { - let provider = table - .entry("provider") - .or_insert_with(|| toml::Value::Table(toml::Table::new())); - if let toml::Value::Table(p) = provider { - p.insert("model".to_string(), toml::Value::String(value.to_string())); - } + let provider = cora.provider.get_or_insert_with(ProviderSection::default); + provider.model = Some(value.to_string()); } "provider" => { - let provider = table - .entry("provider") - .or_insert_with(|| toml::Value::Table(toml::Table::new())); - if let toml::Value::Table(p) = provider { - p.insert( - "provider".to_string(), - toml::Value::String(value.to_string()), - ); - } + let provider = cora.provider.get_or_insert_with(ProviderSection::default); + provider.provider = Some(value.to_string()); + } + "base_url" => { + let provider = cora.provider.get_or_insert_with(ProviderSection::default); + provider.base_url = Some(value.to_string()); } "format" => { - let output = table - .entry("output") - .or_insert_with(|| toml::Value::Table(toml::Table::new())); - if let toml::Value::Table(o) = output { - o.insert("format".to_string(), toml::Value::String(value.to_string())); - } + let output = cora.output.get_or_insert_with(OutputSection::default); + output.format = Some(value.to_string()); } "severity" => { - let hook = table - .entry("hook") - .or_insert_with(|| toml::Value::Table(toml::Table::new())); - if let toml::Value::Table(h) = hook { - h.insert( - "min_severity".to_string(), - toml::Value::String(value.to_string()), - ); - } + let hook = cora.hook.get_or_insert_with(HookSection::default); + hook.min_severity = Some(value.to_string()); } _ => unreachable!(), } - let content = table.to_string(); - std::fs::write(&path, content) - .with_context(|| format!("failed to write {}", path.display()))?; + // Write back as YAML + let yaml = serde_yaml_ng::to_string(&cora).context("failed to serialize config to YAML")?; + std::fs::write(&path, &yaml).with_context(|| format!("failed to write {}", path.display()))?; + let scope = if global { "global" } else { "project" }; println!( - "{} Set {} = {} in {}", + "{} Set {} = {} in {} ({})", "✓".green().bold(), key.bold(), value.green(), - path.display() + path.display(), + scope.dimmed() ); Ok(()) } - -/// Get the cora config directory: ~/.cora/ -fn cora_config_dir() -> Result { - let home = dirs::home_dir().context("cannot determine home directory")?; - Ok(home.join(".cora")) -} diff --git a/src/commands/review.rs b/src/commands/review.rs index 5de7b33..0484b85 100644 --- a/src/commands/review.rs +++ b/src/commands/review.rs @@ -100,6 +100,9 @@ pub async fn execute_review( }; let mut filtered_response = response.clone(); + // Keep issues at or above min_severity (Critical=worst, Info=lowest in Ord but + // Ord order is Critical(0) < Major(1) < Minor(2) < Info(3), so we invert: keep + // where severity Ord value <= min_severity Ord value). filtered_response .issues .retain(|i| i.severity <= min_severity); diff --git a/src/commands/scan.rs b/src/commands/scan.rs index 1df67d9..5aa4f74 100644 --- a/src/commands/scan.rs +++ b/src/commands/scan.rs @@ -130,7 +130,9 @@ pub async fn execute_scan( // 5. Build response and format let issue_count = all_issues.len(); let min_severity = config.hook.min_severity_level(); - let should_block = all_issues.iter().any(|i| i.severity >= min_severity); + // Ord order is Critical(0) < Major(1) < Minor(2) < Info(3), so "at or above + // min_severity" means Ord value <= min_severity. + let should_block = all_issues.iter().any(|i| i.severity <= min_severity); let response = crate::engine::ScanResponse { issues: all_issues, diff --git a/src/config/loader.rs b/src/config/loader.rs index 007dc6b..0ad113d 100644 --- a/src/config/loader.rs +++ b/src/config/loader.rs @@ -3,15 +3,21 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; use tracing::debug; -use crate::config::providers::{PRESETS, detected_presets}; +use crate::config::providers::{detected_presets, PRESETS}; use crate::config::schema::{Config, CoraFile}; use crate::engine::LLMConfig; -/// The name of the config file we search for. +/// The name of the config file we search for in projects. const CONFIG_FILENAME: &str = ".cora.yaml"; +/// Name of the global config file. +const GLOBAL_CONFIG_FILENAME: &str = "config.yaml"; + /// Name of the secret config file for API keys (never committed). -const AUTH_FILENAME: &str = "config.toml"; +const AUTH_FILENAME: &str = "auth.toml"; + +/// Name of the old auth file (for migration). +const OLD_AUTH_FILENAME: &str = "config.toml"; /// Locate the `.cora.yaml` config by walking parent directories from `start`. /// Returns the path and parsed content, or `None` if not found. @@ -39,7 +45,158 @@ pub fn find_cora_file(start: &Path) -> Result> { } } -/// Load the full resolved config: defaults ← .cora.yaml ← CLI overrides. +/// Load the global config from `~/.cora/config.yaml`. +/// Returns `None` if the file doesn't exist or can't be parsed. +fn load_global_config() -> Result> { + let dir = cora_dir()?; + let path = dir.join(GLOBAL_CONFIG_FILENAME); + if !path.is_file() { + return Ok(None); + } + let content = std::fs::read_to_string(&path) + .with_context(|| format!("failed to read {}", path.display()))?; + let cora = CoraFile::from_str(&content)?; + debug!(path = %path.display(), "loaded global config"); + Ok(Some(cora)) +} + +/// Migrate old `~/.cora/config.toml` to the new format if it exists. +/// - Non-secret keys → `~/.cora/config.yaml` +/// - api_key → `~/.cora/auth.toml` +/// - Delete the old file after successful migration. +fn migrate_old_config() { + let dir = match cora_dir() { + Ok(d) => d, + Err(_) => return, + }; + let old_path = dir.join(OLD_AUTH_FILENAME); + if !old_path.is_file() { + return; + } + + let content = match std::fs::read_to_string(&old_path) { + Ok(c) => c, + Err(e) => { + debug!("failed to read old config for migration: {}", e); + return; + } + }; + + let table: toml::Table = match content.parse::() { + Ok(t) => t, + Err(e) => { + debug!( + "old config.toml is not valid TOML, skipping migration: {}", + e + ); + return; + } + }; + + // Check if there's an api_key + let api_key = table + .get("auth") + .and_then(|a| a.get("api_key")) + .and_then(|k| k.as_str()) + .map(|s| s.to_string()) + .or_else(|| { + table + .get("api_key") + .and_then(|k| k.as_str()) + .map(|s| s.to_string()) + }); + + // Extract non-secret config fields into a CoraFile + let mut cora = CoraFile::default(); + + if let Some(provider) = table.get("provider").and_then(|v| v.as_table()) { + let mut ps = crate::config::schema::ProviderSection::default(); + if let Some(v) = provider.get("provider").and_then(|v| v.as_str()) { + ps.provider = Some(v.to_string()); + } + if let Some(v) = provider.get("model").and_then(|v| v.as_str()) { + ps.model = Some(v.to_string()); + } + if let Some(v) = provider.get("base_url").and_then(|v| v.as_str()) { + ps.base_url = Some(v.to_string()); + } + // Only set if we found something + if ps.provider.is_some() || ps.model.is_some() || ps.base_url.is_some() { + cora.provider = Some(ps); + } + } + + if let Some(output) = table.get("output").and_then(|v| v.as_table()) { + let mut os = crate::config::schema::OutputSection::default(); + if let Some(v) = output.get("format").and_then(|v| v.as_str()) { + os.format = Some(v.to_string()); + } + if let Some(v) = output.get("color").and_then(|v| v.as_bool()) { + os.color = Some(v); + } + if os.format.is_some() || os.color.is_some() { + cora.output = Some(os); + } + } + + if let Some(hook) = table.get("hook").and_then(|v| v.as_table()) { + let mut hs = crate::config::schema::HookSection::default(); + if let Some(v) = hook.get("mode").and_then(|v| v.as_str()) { + hs.mode = Some(v.to_string()); + } + if let Some(v) = hook.get("min_severity").and_then(|v| v.as_str()) { + hs.min_severity = Some(v.to_string()); + } + if let Some(v) = hook.get("max_diff_size").and_then(|v| v.as_integer()) { + hs.max_diff_size = Some(v as usize); + } + if hs.mode.is_some() || hs.min_severity.is_some() || hs.max_diff_size.is_some() { + cora.hook = Some(hs); + } + } + + // Write api_key to auth.toml if present + if let Some(key) = api_key { + let auth_path = dir.join(AUTH_FILENAME); + if let Err(e) = std::fs::write(&auth_path, format!("api_key = \"{key}\"\n")) { + debug!("failed to write migrated auth.toml: {}", e); + return; + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let perms = std::fs::Permissions::from_mode(0o600); + let _ = std::fs::set_permissions(&auth_path, perms); + } + } + + // Write config to config.yaml if there are non-secret fields + let has_config = cora.provider.is_some() + || cora.output.is_some() + || cora.hook.is_some() + || cora.focus.is_some() + || cora.rules.is_some() + || cora.ignore.is_some(); + + if has_config { + let config_path = dir.join(GLOBAL_CONFIG_FILENAME); + if let Ok(yaml) = serde_yaml_ng::to_string(&cora) { + if let Err(e) = std::fs::write(&config_path, yaml) { + debug!("failed to write migrated config.yaml: {}", e); + return; + } + } + } + + // Delete old file + if let Err(e) = std::fs::remove_file(&old_path) { + debug!("failed to remove old config.toml after migration: {}", e); + } else { + debug!("migrated ~/.cora/config.toml to new format"); + } +} + +/// Load the full resolved config: defaults ← global config ← .cora.yaml ← CLI overrides. /// /// `cli_provider`, `cli_model`, `cli_api_key`, and `cli_format` are `None` /// when the user did not pass the corresponding flag. @@ -54,7 +211,15 @@ pub fn load_config( ) -> Result { let mut config = Config::default(); - // 1. Load .cora.yaml + // Run migration silently on first access + migrate_old_config(); + + // 1. Load global config (~/.cora/config.yaml) + if let Some(cora) = load_global_config()? { + cora.merge_into(&mut config); + } + + // 2. Load project config (.cora.yaml) if let Some(path) = cli_config_path { let path = Path::new(path); let content = std::fs::read_to_string(path) @@ -69,7 +234,7 @@ pub fn load_config( debug!("no .cora.yaml found, using defaults"); } - // 2. CLI overrides + // 3. CLI overrides if let Some(p) = cli_provider { config.provider.provider = p.to_string(); } @@ -90,7 +255,7 @@ pub fn load_config( } /// Build an `LLMConfig` from the resolved `Config`, fetching the API key -/// from: CLI flag → env CORA_API_KEY → ~/.config/cora/config.toml. +/// from: CLI flag → env CORA_API_KEY → ~/.cora/auth.toml. /// /// If none of those are set, auto-detect from known provider env vars (OPENAI_API_KEY, etc.) /// and configure provider/model/base_url from the matching preset. @@ -166,12 +331,12 @@ pub fn build_llm_config(config: &Config, cli_api_key: Option<&str>) -> Result Result { +pub fn cora_dir() -> Result { let home = dirs::home_dir().context("cannot determine home directory")?; Ok(home.join(".cora")) } -/// Read the stored API key from ~/.cora/config.toml. +/// Read the stored API key from ~/.cora/auth.toml. pub fn load_api_key_from_auth_file() -> Result> { let dir = cora_dir()?; let path = dir.join(AUTH_FILENAME); @@ -202,7 +367,7 @@ pub fn load_api_key_from_auth_file() -> Result> { Ok(key) } -/// Save an API key to ~/.cora/config.toml. +/// Save an API key to ~/.cora/auth.toml. pub fn save_api_key(key: &str) -> Result<()> { let dir = cora_dir()?; std::fs::create_dir_all(&dir).with_context(|| format!("failed to create {}", dir.display()))?; @@ -226,7 +391,7 @@ pub fn save_api_key(key: &str) -> Result<()> { Ok(()) } -/// Remove the stored API key from ~/.cora/config.toml. +/// Remove the stored API key from ~/.cora/auth.toml. pub fn remove_api_key() -> Result<()> { let dir = cora_dir()?; let path = dir.join(AUTH_FILENAME); diff --git a/src/config/schema.rs b/src/config/schema.rs index cc0cd8a..a3e2db6 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -97,37 +97,53 @@ impl HookConfig { /// Serde-compatible schema for the `.cora.yaml` configuration file. #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct CoraFile { + #[serde(skip_serializing_if = "Option::is_none")] pub provider: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub focus: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub rules: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub ignore: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub hook: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub output: Option, } #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct ProviderSection { + #[serde(skip_serializing_if = "Option::is_none")] pub provider: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub model: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub base_url: Option, } #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct IgnoreSection { + #[serde(skip_serializing_if = "Option::is_none")] pub files: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub rules: Option>, } #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct HookSection { + #[serde(skip_serializing_if = "Option::is_none")] pub mode: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub min_severity: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub max_diff_size: Option, } #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct OutputSection { + #[serde(skip_serializing_if = "Option::is_none")] pub format: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub color: Option, } diff --git a/src/engine/review.rs b/src/engine/review.rs index 97b6b98..9b6368e 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -64,10 +64,12 @@ async fn review_diff_inner( // Calculate should_block based on min_severity let min_severity = config.hook.min_severity_level(); + // Ord order is Critical(0) < Major(1) < Minor(2) < Info(3), so "at or above + // min_severity" means Ord value <= min_severity. response.should_block = response .issues .iter() - .any(|issue| issue.severity >= min_severity); + .any(|issue| issue.severity <= min_severity); debug!( issues = response.issues.len(), @@ -115,7 +117,8 @@ pub async fn scan_project( // Calculate should_block let min_severity = config.hook.min_severity_level(); - let should_block = issues.iter().any(|issue| issue.severity >= min_severity); + // Ord order: Critical(0) < Major(1) < Minor(2) < Info(3) + let should_block = issues.iter().any(|issue| issue.severity <= min_severity); let default_summary = format!( "Scanned {} files, found {} issues.", diff --git a/src/main.rs b/src/main.rs index 6001370..c21b699 100644 --- a/src/main.rs +++ b/src/main.rs @@ -235,12 +235,15 @@ enum AuthAction { enum ConfigAction { /// Show the current resolved configuration Show, - /// Set a configuration value (keys: model, provider, format, severity) + /// Set a configuration value (keys: model, provider, base_url, format, severity) Set { /// Configuration key to set key: String, /// Value to assign value: String, + /// Write to global config (~/.cora/config.yaml) instead of project .cora.yaml + #[clap(long)] + global: bool, }, } @@ -374,8 +377,8 @@ async fn main() -> Result<()> { config_cmd::execute_config_show()?; 0 } - ConfigAction::Set { key, value } => { - config_cmd::execute_config_set(&key, &value)?; + ConfigAction::Set { key, value, global } => { + config_cmd::execute_config_set(&key, &value, global)?; 0 } }, From 54d8250c549d9cd5f224b45dcb1d58b3b17cdcf6 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Sun, 31 May 2026 22:26:37 +0700 Subject: [PATCH 2/5] style: cargo fmt --- src/config/loader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/loader.rs b/src/config/loader.rs index 0ad113d..efcfca0 100644 --- a/src/config/loader.rs +++ b/src/config/loader.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; use tracing::debug; -use crate::config::providers::{detected_presets, PRESETS}; +use crate::config::providers::{PRESETS, detected_presets}; use crate::config::schema::{Config, CoraFile}; use crate::engine::LLMConfig; From 1803a16e10554d1111b432ef71c3a2a427333d37 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Sun, 31 May 2026 22:37:36 +0700 Subject: [PATCH 3/5] docs: update README, CONTRIBUTING, CHANGELOG for config redesign + severity fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix global config path (~/.config/cora → ~/.cora/config.yaml) - Document --global flag, base_url key, config priority chain - Add Authentication section with auth.toml details - Update CI/CD section with composite action example - Fix CONTRIBUTING: Rust version, project structure, test example - Add Unreleased changelog entry for all changes --- CHANGELOG.md | 19 +++++++++++++ CONTRIBUTING.md | 43 +++++++++++++++++++++--------- README.md | 71 +++++++++++++++++++++++++++++++++++-------------- 3 files changed, 100 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f60b85c..e1aabf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `cora config set --global` — write config to `~/.cora/config.yaml` instead of project `.cora.yaml` +- `cora config set base_url` — set base URL via CLI (previously only in YAML) +- Global config support (`~/.cora/config.yaml`) with priority chain: CLI flags → env vars → project → global → defaults +- Auto-migration from old `~/.cora/config.toml` to new YAML + `auth.toml` split + +### Changed + +- `cora config set` now writes YAML instead of TOML (compatible with config loader) +- API key storage moved from `~/.cora/config.toml` to `~/.cora/auth.toml` (0600 permissions) +- YAML serialization uses `skip_serializing_if` — no more `null` values in output + +### Fixed + +- **Severity comparison inverted** — `Critical` issues no longer silently pass `should_block` check (Ord ordering bug) +- Hook `mode: block` no longer exits with code 2 when "No issues found" (severity filter mismatch) +- Consistent severity logic across review, scan, and block mode paths + ## [0.1.2] - 2025-05-29 ### Added diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bd6274c..fe48c04 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,7 +21,7 @@ Be respectful, constructive, and inclusive. We follow the [Rust Code of Conduct] ### Prerequisites -- **Rust** 1.75+ (stable toolchain recommended) +- **Rust** 1.85+ (stable toolchain recommended) - **Git** for version control - An **LLM API key** (OpenAI, Anthropic, etc.) for testing review features @@ -47,14 +47,30 @@ cargo clippy -- -D warnings ``` cora-cli/ ├── src/ -│ ├── main.rs # CLI entry point -│ ├── cli.rs # Argument parsing (clap) -│ ├── config.rs # Configuration management -│ ├── scanner.rs # File scanning and diff generation -│ ├── reviewer.rs # LLM integration for code review -│ ├── formatter.rs # Output formatting -│ └── ci.rs # CI/CD integration helpers -├── tests/ # Integration tests +│ ├── main.rs # CLI entry point + clap args +│ ├── cli.rs # Argument parsing (clap) +│ ├── commands/ # CLI subcommands +│ │ ├── review.rs # cora review +│ │ ├── scan.rs # cora scan +│ │ ├── config_cmd.rs # cora config show/set +│ │ ├── auth.rs # cora auth login/logout +│ │ └── init.rs # cora init +│ ├── config/ +│ │ ├── loader.rs # Config resolution chain (global → project → env → CLI) +│ │ ├── schema.rs # YAML config structs + merge logic +│ │ └── providers.rs # Provider presets (OpenAI, Anthropic, etc.) +│ ├── engine/ +│ │ ├── review.rs # LLM review + SARIF generation +│ │ ├── scanner.rs # File scanning and diff generation +│ │ ├── types.rs # Issue, Severity, ScanResponse types +│ │ └── llm.rs # LLM API abstraction +│ └── formatter/ +│ └── mod.rs # Output formatting (pretty, compact, json, sarif) +├── .github/ +│ ├── workflows/ # CI, release, deploy workflows +│ └── actions/ +│ └── cora-review/ # Composite action for GitHub Actions +├── tests/ # Integration tests ├── Cargo.toml └── README.md ``` @@ -68,7 +84,7 @@ cora-cli/ ``` 3. **Make your changes** and commit with meaningful messages: ```bash - git commit -s -m "feat: add support for SARIF output format" + git commit -m "feat: add support for SARIF output format" ``` 4. **Push** to your fork and open a **Pull Request** @@ -111,9 +127,10 @@ mod tests { use super::*; #[test] - fn test_scan_single_file() { - let result = scan_files(vec!["src/main.rs".into()]); - assert!(!result.is_empty()); + fn test_severity_from_str() { + use crate::engine::types::Severity; + assert_eq!(Severity::from_str_lossy("critical"), Severity::Critical); + assert_eq!(Severity::from_str_lossy("info"), Severity::Info); } } ``` diff --git a/README.md b/README.md index c5901b7..454eaad 100644 --- a/README.md +++ b/README.md @@ -167,17 +167,26 @@ cora scan --incremental ### `cora config` -Manage configuration. +Manage configuration. Supports both project-level (`.cora.yaml`) and global (`~/.cora/config.yaml`) config. ```bash -# Show current configuration +# Show current resolved configuration cora config show -# Set a configuration value +# Set a project-level value (writes to .cora.yaml) cora config set model claude-sonnet-4-20250514 +cora config set base_url https://api.openai.com/v1 cora config set severity major + +# Set a global value (writes to ~/.cora/config.yaml) +cora config set --global model gpt-4o-mini +cora config set --global provider anthropic + +# Supported keys: model, provider, base_url, format, severity ``` +**Priority**: CLI flags → env vars → `.cora.yaml` (project) → `~/.cora/config.yaml` (global) → defaults + ### `cora init` Create a `.cora.yaml` config file in the current directory. @@ -207,7 +216,13 @@ cora hook uninstall ## ⚙️ Configuration -Create a `.cora.yaml` in your project root or `~/.config/cora/config.yaml` globally: +Cora reads configuration from multiple sources in priority order: + +``` +CLI flags → CORA_* env vars → .cora.yaml (project) → ~/.cora/config.yaml (global) → defaults +``` + +Create a `.cora.yaml` in your project root, or use `~/.cora/config.yaml` for global settings. Project config always overrides global. ```yaml # .cora.yaml @@ -265,37 +280,53 @@ output: | `CORA_FORMAT` | Output format (`pretty`, `json`, `compact`, `sarif`) | `pretty` | | `CORA_NO_COLOR` | Disable colored output | — | +### Authentication + +API keys can be provided via environment variable (`CORA_API_KEY`), provider-specific env vars (`OPENAI_API_KEY`, etc.), or stored in `~/.cora/auth.toml` (auto-created by `cora auth login`, permission `0600`). + +```bash +# Interactive login (stores key in ~/.cora/auth.toml) +cora auth login + +# Or set via environment variable +export CORA_API_KEY=sk-... +``` + ## 🔗 CI/CD Integration ### GitHub Actions -```yaml -# .github/workflows/review.yml -name: Code Review +Using the official [cora-review composite action](.github/actions/cora-review): +```yaml +name: CI on: pull_request: - types: [opened, synchronize, reopened] + branches: [develop] jobs: - review: + cora-review: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 with: fetch-depth: 0 + - uses: ./.github/actions/cora-review + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + infisical-identity-id: ${{ secrets.INFISICAL_IDENTITY_ID }} + severity: major + upload-sarif: 'true' +``` - - name: Install cora-cli - run: | - VERSION=$(curl -s https://api.github.com/repos/ajianaz/cora-cli/releases/latest | grep tag_name | cut -d'"' -f4) - curl -L "https://github.com/ajianaz/cora-cli/releases/download/${VERSION}/cora-x86_64-unknown-linux-gnu-${VERSION}.tar.gz" | tar xz - sudo mv cora /usr/local/bin/ - - - name: Run code review - env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: cora review --base origin/main --upload +Or install manually: + +```yaml +# Manual install in CI +- name: Install cora-cli + run: | + curl -fsSL https://github.com/ajianaz/cora-cli/releases/latest/download/cora-x86_64-unknown-linux-gnu.tar.gz | tar xz + sudo mv cora /usr/local/bin/ ``` ### GitLab CI From 0772884a9d05754627797d02125143e05574a975 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Sun, 31 May 2026 22:42:03 +0700 Subject: [PATCH 4/5] fix: address cora self-review findings (#60) - Fix API key TOML injection: use toml::Table instead of format!() - Add migration marker (.migrated) to prevent repeated filesystem I/O - Restrict global config file permissions to 0600 - Guard project config set: require git repo (no orphan files) --- src/commands/config_cmd.rs | 22 +++++++++++++++++++++- src/config/loader.rs | 20 +++++++++++++++++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/commands/config_cmd.rs b/src/commands/config_cmd.rs index 8df3755..b16a0b6 100644 --- a/src/commands/config_cmd.rs +++ b/src/commands/config_cmd.rs @@ -118,7 +118,20 @@ pub fn execute_config_set(key: &str, value: &str, global: bool) -> Result<()> { .with_context(|| format!("failed to create {}", dir.display()))?; dir.join("config.yaml") } else { - PathBuf::from(".cora.yaml") + // Warn if not inside a git project (could create orphan file) + if std::process::Command::new("git") + .args(["rev-parse", "--git-dir"]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .map_or(false, |s| s.success()) + { + PathBuf::from(".cora.yaml") + } else { + anyhow::bail!( + "Not in a git project. Use --global to set config in ~/.cora/config.yaml, or cd into a git repo first." + ); + } }; // Load existing file or start fresh @@ -159,6 +172,13 @@ pub fn execute_config_set(key: &str, value: &str, global: bool) -> Result<()> { let yaml = serde_yaml_ng::to_string(&cora).context("failed to serialize config to YAML")?; std::fs::write(&path, &yaml).with_context(|| format!("failed to write {}", path.display()))?; + // Restrict permissions for global config (defense-in-depth) + #[cfg(unix)] + if global { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)); + } + let scope = if global { "global" } else { "project" }; println!( "{} Set {} = {} in {} ({})", diff --git a/src/config/loader.rs b/src/config/loader.rs index efcfca0..52eb83c 100644 --- a/src/config/loader.rs +++ b/src/config/loader.rs @@ -19,6 +19,9 @@ const AUTH_FILENAME: &str = "auth.toml"; /// Name of the old auth file (for migration). const OLD_AUTH_FILENAME: &str = "config.toml"; +/// Marker file created after successful migration. +const MIGRATION_MARKER: &str = ".migrated"; + /// Locate the `.cora.yaml` config by walking parent directories from `start`. /// Returns the path and parsed content, or `None` if not found. pub fn find_cora_file(start: &Path) -> Result> { @@ -64,6 +67,7 @@ fn load_global_config() -> Result> { /// - Non-secret keys → `~/.cora/config.yaml` /// - api_key → `~/.cora/auth.toml` /// - Delete the old file after successful migration. +/// - Creates `.migrated` marker to prevent re-running. fn migrate_old_config() { let dir = match cora_dir() { Ok(d) => d, @@ -74,6 +78,11 @@ fn migrate_old_config() { return; } + // Check if migration already completed + if dir.join(MIGRATION_MARKER).is_file() { + return; + } + let content = match std::fs::read_to_string(&old_path) { Ok(c) => c, Err(e) => { @@ -155,10 +164,13 @@ fn migrate_old_config() { } } - // Write api_key to auth.toml if present + // Write api_key to auth.toml if present (use TOML library to avoid injection) if let Some(key) = api_key { let auth_path = dir.join(AUTH_FILENAME); - if let Err(e) = std::fs::write(&auth_path, format!("api_key = \"{key}\"\n")) { + let mut table = toml::Table::new(); + table.insert("api_key".to_string(), toml::Value::String(key)); + let content = table.to_string(); + if let Err(e) = std::fs::write(&auth_path, content) { debug!("failed to write migrated auth.toml: {}", e); return; } @@ -188,10 +200,12 @@ fn migrate_old_config() { } } - // Delete old file + // Delete old file and create migration marker if let Err(e) = std::fs::remove_file(&old_path) { debug!("failed to remove old config.toml after migration: {}", e); } else { + // Create marker to prevent re-running migration + let _ = std::fs::write(dir.join(MIGRATION_MARKER), ""); debug!("migrated ~/.cora/config.toml to new format"); } } From 346140d1ba592b1fb9071544629aba7ed1686f84 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Sun, 31 May 2026 22:55:19 +0700 Subject: [PATCH 5/5] chore: trigger CI