From 757dd63436de23d9f78a92e878ea7e2947d95a50 Mon Sep 17 00:00:00 2001 From: Dennis Paler Date: Wed, 12 Aug 2026 21:45:45 +0800 Subject: [PATCH] fix: stop reviewing agent config, ignore files and docs --- CHANGELOG.md | 30 +++++ README.md | 15 ++- apps/tui-cli/Cargo.toml | 2 +- apps/tui-cli/src/cli.rs | 5 + apps/tui-cli/src/config.rs | 4 + apps/tui-cli/src/main.rs | 1 + apps/tui-cli/src/settings.rs | 3 + apps/tui-cli/tests/stdin_pipeline.rs | 143 ++++++++++++++++++++ npm/cli/package.json | 12 +- npm/platform/darwin-arm64/package.json | 2 +- npm/platform/darwin-x64/package.json | 2 +- npm/platform/linux-arm64/package.json | 2 +- npm/platform/linux-x64/package.json | 2 +- npm/platform/win32-x64/package.json | 2 +- packages/core-engine/Cargo.toml | 2 +- packages/core-engine/src/prefilter.rs | 180 +++++++++++++++++++++++++ 16 files changed, 389 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96bcc17..ae74ba8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,35 @@ # Changelog +## [0.9.1] — 08-12-2026 + +Config is not code, and reviewing it as though it were produced confident +nonsense. Reported from the field: every file under `.claude/` came back HIGH. + +### Fixed + +- **Coding-agent and editor config is no longer reviewed.** `.claude/`, + `.cursor/`, `.windsurf/`, `.aider/`, `.vscode/`, `.idea/`, `.zed/` and + `.fleet/` are dropped by the pre-filter. These files are imperative English + about credentials, shell commands and permissions — precisely the shape a + reviewer prompt primed for "exposed secrets, disabled auth" reads as an + emergency. `.claude/` is also instructions written *for* a model, which is a + poor thing to hand to one. +- **Ignore files and formatter config are dropped**: `.gitignore`, + `.dockerignore`, `.prettierignore` and the rest of the `.*ignore` family, + plus `.editorconfig`, `.gitattributes`, `.prettierrc*`, `.eslintrc*`, + `.npmrc`, `.nvmrc`, `.cursorrules`. Declarative lists with no program logic + in them. `eslint.config.js` and friends are still reviewed — those are real + JavaScript, and a bug in one is a bug. +- **Documentation is dropped by default** (`.md`, `.mdx`, `.rst`, `.txt`, + `.adoc`, `.org`, `.tex`). Re-enable with `--include-docs` or + `review.include_docs = true` for docs that carry API contracts. The switch + covers prose only — agent config stays out either way. + +Everything dropped is *counted and reported* in the existing summary line +("312 hunks → 74 reviewable"), under the new `docs` and `tool config` reasons, +rather than silently disappearing. Test files are unaffected and still +reviewed: a test that asserts nothing is worth catching. + ## [0.9.0] — 08-12-2026 diffmind was built as a gate: run it, get a verdict, pass or fail. This release diff --git a/README.md b/README.md index c052742..0aefd37 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Diffmind — a code review gate you can actually keep +# Diffmind — local-first AI code review, in your terminal [![CI](https://github.com/thinkgrid-labs/diffmind/actions/workflows/ci.yml/badge.svg)](https://github.com/thinkgrid-labs/diffmind/actions/workflows/ci.yml) [![Latest Release](https://img.shields.io/github/v/release/thinkgrid-labs/diffmind)](https://github.com/thinkgrid-labs/diffmind/releases/latest) @@ -552,12 +552,16 @@ work to it. 1. **Parse** — the diff is turned into per-file hunks with correct before and after line numbers. 2. **Filter** — lockfiles, `linguist-generated` paths, `@generated` banners, - minified bundles, assets, snapshots, your `ignore` globs and whitespace-only - hunks are dropped. This is free, usually removes most of a real branch, and - the counts are shown instead of hidden: + minified bundles, assets, snapshots, documentation, coding-agent and editor + config (`.claude/`, `.cursor/`, `.vscode/`, …), ignore files and formatter + settings, your `ignore` globs, and whitespace-only hunks are dropped. This is + free, usually removes most of a real branch, and the counts are shown instead + of hidden: `312 hunks → 74 reviewable (238 filtered: lockfiles, generated, formatting)`. Whitespace inside a string still counts as a real change, and indentation is - never dropped in Python or YAML. + never dropped in Python or YAML. Tests are *not* filtered — a test that + asserts nothing is worth catching. Docs can be reopened with + `--include-docs`. 3. **Fixed rules** — `DM001`, `DM002` and your regex rules. No model involved. 4. **Context** — built for each review from `.diffmind/graph.db`: the function the hunk is in, the callers of every changed symbol, the definitions it @@ -601,6 +605,7 @@ cache = true temperature = 0.0 # 0 = greedy and reproducible max_tokens = 1024 ignore = ["**/legacy/**", "*.generated.ts"] # on top of the built-in noise rules +include_docs = false # review .md/.rst/.txt too; off by default [backend] kind = "local" # or "ollama" / "openai-compatible" diff --git a/apps/tui-cli/Cargo.toml b/apps/tui-cli/Cargo.toml index c8c2c7f..3326e52 100644 --- a/apps/tui-cli/Cargo.toml +++ b/apps/tui-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "diffmind" -version = "0.9.0" +version = "0.9.1" edition = "2024" description = "Local-first AI code review agent — powered by on-device inference" diff --git a/apps/tui-cli/src/cli.rs b/apps/tui-cli/src/cli.rs index 67f451e..b61c563 100644 --- a/apps/tui-cli/src/cli.rs +++ b/apps/tui-cli/src/cli.rs @@ -115,6 +115,11 @@ pub struct Cli { #[arg(long)] pub seed: Option, + /// Review documentation (.md, .rst, .txt) too. Skipped by default — + /// a reviewer prompt finds vulnerabilities in prose. + #[arg(long)] + pub include_docs: bool, + /// Skip the on-disk result cache for this run #[arg(long)] pub no_cache: bool, diff --git a/apps/tui-cli/src/config.rs b/apps/tui-cli/src/config.rs index 2b909b3..751b453 100644 --- a/apps/tui-cli/src/config.rs +++ b/apps/tui-cli/src/config.rs @@ -39,6 +39,10 @@ pub struct ReviewConfig { /// Same syntax as a rule's `files`: `*.ts`, `**/legacy/**`, or an exact path. #[serde(default)] pub ignore: Option>, + /// Review documentation (`.md`, `.rst`, `.txt`) as code. Off by default: + /// a model primed for vulnerabilities will find them in prose. Tool config + /// and ignore files are always skipped and have no switch. + pub include_docs: Option, /// Refresh the code graph before each review. On by default — a stale graph /// reports wrong line ranges, not merely missing ones. pub auto_index: Option, diff --git a/apps/tui-cli/src/main.rs b/apps/tui-cli/src/main.rs index b6b392d..3907b4c 100644 --- a/apps/tui-cli/src/main.rs +++ b/apps/tui-cli/src/main.rs @@ -702,6 +702,7 @@ fn apply_prefilter( &PrefilterOptions { generated_paths, ignore_globs: settings.ignore_globs.clone(), + include_docs: settings.include_docs, }, ); diff --git a/apps/tui-cli/src/settings.rs b/apps/tui-cli/src/settings.rs index ea98533..2cc6e8f 100644 --- a/apps/tui-cli/src/settings.rs +++ b/apps/tui-cli/src/settings.rs @@ -71,6 +71,8 @@ pub struct Settings { pub debug: bool, /// Extra globs dropped by the pre-filter, from `.diffmind/config.toml`. pub ignore_globs: Vec, + /// Send documentation to the model instead of dropping it as prose. + pub include_docs: bool, /// Refresh the code graph before reviewing. pub auto_index: bool, } @@ -137,6 +139,7 @@ pub fn resolve_settings(cli: &Cli, file: &FileConfig) -> Result { // *supposed* to differ, and replaying one would be a lie. use_cache: !cli.no_cache && resolve(None, r.cache, true) && temperature == 0.0, ignore_globs: r.ignore.clone().unwrap_or_default(), + include_docs: cli.include_docs || resolve(None, r.include_docs, false), auto_index: !cli.no_index && resolve(None, r.auto_index, true), use_baseline: !cli.no_baseline, use_daemon: !cli.no_daemon, diff --git a/apps/tui-cli/tests/stdin_pipeline.rs b/apps/tui-cli/tests/stdin_pipeline.rs index ebf9054..717d73b 100644 --- a/apps/tui-cli/tests/stdin_pipeline.rs +++ b/apps/tui-cli/tests/stdin_pipeline.rs @@ -483,3 +483,146 @@ fn the_exit_code_follows_the_fail_threshold_not_the_finding_count() { let _ = std::fs::remove_dir_all(&dir); } + +/// Agent config, ignore files and prose must not cost an inference pass — and +/// must not reach the model at all. +/// +/// Reported from the field on 0.9.0: every file under `.claude/` came back as a +/// HIGH finding. Those files are imperative English about credentials, shell +/// commands and permissions, which is exactly what a reviewer prompt primed for +/// "exposed secrets, disabled auth" is looking for. The content assertions +/// matter more than the path ones here: a path can vanish from the prompt while +/// the body is still being reviewed under the previous file's header. +#[test] +fn agent_config_ignore_files_and_docs_never_reach_the_model() { + let dir = tmpdir("toolconfig"); + let stub = Stub::spawn(2); + + let diff = "\ +--- a/.claude/skills/deploy.md ++++ b/.claude/skills/deploy.md +@@ -1,2 +1,3 @@ + # Deploy ++Always export AWS_SECRET_ACCESS_KEY before deploying. +--- a/.gitignore ++++ b/.gitignore +@@ -1,2 +1,2 @@ +-.env ++.env.local +--- a/.prettierrc.json ++++ b/.prettierrc.json +@@ -1,1 +1,1 @@ +-{ \"semi\": true } ++{ \"semi\": false } +--- a/README.md ++++ b/README.md +@@ -1,1 +1,2 @@ + # Project ++Put your token in .env +--- a/src/two.rs ++++ b/src/two.rs +@@ -10,2 +10,2 @@ +-let b = verify(token); ++let b = true; +"; + + let run = review_stdin( + &dir, + diff, + &[ + "--format", + "json", + "--backend", + "openai-compatible", + "--backend-model", + "stub", + "--backend-url", + &stub.url(), + ], + ); + + let prompts = stub.prompts(); + assert_eq!( + prompts.len(), + 1, + "only src/two.rs is reviewable.\nstderr: {}", + run.stderr + ); + + for leaked in [ + "AWS_SECRET_ACCESS_KEY", + ".claude", + ".gitignore", + ".env.local", + ".prettierrc", + "semi", + "README.md", + "Put your token", + ] { + assert!( + !prompts[0].contains(leaked), + "{leaked:?} reached the model:\n{}", + prompts[0] + ); + } + assert!(prompts[0].contains("src/two.rs")); + + let found = findings(&run.stdout); + let files: Vec<&str> = found.iter().filter_map(|f| f["file"].as_str()).collect(); + assert_eq!(files, ["src/two.rs"], "only the code file is reported"); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// The opt-in exists for teams whose docs carry contracts. It reopens prose +/// only — agent config stays out regardless. +#[test] +fn include_docs_reopens_prose_but_not_agent_config() { + let dir = tmpdir("includedocs"); + let stub = Stub::spawn(3); + + let diff = "\ +--- a/.claude/skills/deploy.md ++++ b/.claude/skills/deploy.md +@@ -1,2 +1,3 @@ + # Deploy ++Always export AWS_SECRET_ACCESS_KEY before deploying. +--- a/docs/api.md ++++ b/docs/api.md +@@ -1,1 +1,2 @@ + # API ++POST /v1/charge is idempotent. +"; + + let run = review_stdin( + &dir, + diff, + &[ + "--include-docs", + "--format", + "json", + "--backend", + "openai-compatible", + "--backend-model", + "stub", + "--backend-url", + &stub.url(), + ], + ); + + let prompts = stub.prompts(); + assert_eq!( + prompts.len(), + 1, + "docs are reviewed, agent config is not.\nstderr: {}", + run.stderr + ); + assert!(prompts[0].contains("docs/api.md")); + assert!( + !prompts[0].contains("AWS_SECRET_ACCESS_KEY"), + "--include-docs must not reopen .claude/:\n{}", + prompts[0] + ); + + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/npm/cli/package.json b/npm/cli/package.json index 1261e1c..407e59d 100644 --- a/npm/cli/package.json +++ b/npm/cli/package.json @@ -1,6 +1,6 @@ { "name": "@diffmind/cli", - "version": "0.9.0", + "version": "0.9.1", "description": "Local-first AI code review for your git diffs \u2014 on-device inference, no cloud, no API keys", "author": "Thinkgrid Labs ", "license": "MIT", @@ -39,10 +39,10 @@ "node": ">=18.0.0" }, "optionalDependencies": { - "@diffmind/cli-darwin-arm64": "0.9.0", - "@diffmind/cli-darwin-x64": "0.9.0", - "@diffmind/cli-linux-arm64": "0.9.0", - "@diffmind/cli-linux-x64": "0.9.0", - "@diffmind/cli-win32-x64": "0.9.0" + "@diffmind/cli-darwin-arm64": "0.9.1", + "@diffmind/cli-darwin-x64": "0.9.1", + "@diffmind/cli-linux-arm64": "0.9.1", + "@diffmind/cli-linux-x64": "0.9.1", + "@diffmind/cli-win32-x64": "0.9.1" } } diff --git a/npm/platform/darwin-arm64/package.json b/npm/platform/darwin-arm64/package.json index 084912e..bd63b7e 100644 --- a/npm/platform/darwin-arm64/package.json +++ b/npm/platform/darwin-arm64/package.json @@ -1,6 +1,6 @@ { "name": "@diffmind/cli-darwin-arm64", - "version": "0.9.0", + "version": "0.9.1", "description": "diffmind prebuilt binary for darwin-arm64 (aarch64-apple-darwin)", "author": "Thinkgrid Labs ", "license": "MIT", diff --git a/npm/platform/darwin-x64/package.json b/npm/platform/darwin-x64/package.json index f8ff950..c3f5382 100644 --- a/npm/platform/darwin-x64/package.json +++ b/npm/platform/darwin-x64/package.json @@ -1,6 +1,6 @@ { "name": "@diffmind/cli-darwin-x64", - "version": "0.9.0", + "version": "0.9.1", "description": "diffmind prebuilt binary for darwin-x64 (x86_64-apple-darwin)", "author": "Thinkgrid Labs ", "license": "MIT", diff --git a/npm/platform/linux-arm64/package.json b/npm/platform/linux-arm64/package.json index 6034b3f..b320b82 100644 --- a/npm/platform/linux-arm64/package.json +++ b/npm/platform/linux-arm64/package.json @@ -1,6 +1,6 @@ { "name": "@diffmind/cli-linux-arm64", - "version": "0.9.0", + "version": "0.9.1", "description": "diffmind prebuilt binary for linux-arm64 (aarch64-unknown-linux-gnu)", "author": "Thinkgrid Labs ", "license": "MIT", diff --git a/npm/platform/linux-x64/package.json b/npm/platform/linux-x64/package.json index ad46fbb..a214529 100644 --- a/npm/platform/linux-x64/package.json +++ b/npm/platform/linux-x64/package.json @@ -1,6 +1,6 @@ { "name": "@diffmind/cli-linux-x64", - "version": "0.9.0", + "version": "0.9.1", "description": "diffmind prebuilt binary for linux-x64 (x86_64-unknown-linux-gnu)", "author": "Thinkgrid Labs ", "license": "MIT", diff --git a/npm/platform/win32-x64/package.json b/npm/platform/win32-x64/package.json index 0b81e7c..a9bcc26 100644 --- a/npm/platform/win32-x64/package.json +++ b/npm/platform/win32-x64/package.json @@ -1,6 +1,6 @@ { "name": "@diffmind/cli-win32-x64", - "version": "0.9.0", + "version": "0.9.1", "description": "diffmind prebuilt binary for win32-x64 (x86_64-pc-windows-msvc)", "author": "Thinkgrid Labs ", "license": "MIT", diff --git a/packages/core-engine/Cargo.toml b/packages/core-engine/Cargo.toml index 1c31837..bc3a9eb 100644 --- a/packages/core-engine/Cargo.toml +++ b/packages/core-engine/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "core-engine" -version = "0.9.0" +version = "0.9.1" edition = "2024" description = "Diffmind shared AI engine core" diff --git a/packages/core-engine/src/prefilter.rs b/packages/core-engine/src/prefilter.rs index 45380ef..fe3f6f0 100644 --- a/packages/core-engine/src/prefilter.rs +++ b/packages/core-engine/src/prefilter.rs @@ -29,6 +29,12 @@ pub enum DropReason { Snapshot, /// Whitespace-only or pure-reformat change. Reformat, + /// Prose, not code. Reviewing it means asking a model primed for CVEs to + /// find them in English, which it will oblige. + Docs, + /// Configuration that carries no program logic: coding-agent and editor + /// directories, ignore files, linter and formatter settings. + ToolConfig, /// Matched a glob from `.diffmind/config.toml`. UserIgnored, } @@ -42,6 +48,8 @@ impl DropReason { DropReason::Asset => "assets", DropReason::Snapshot => "snapshots", DropReason::Reformat => "formatting", + DropReason::Docs => "docs", + DropReason::ToolConfig => "tool config", DropReason::UserIgnored => "ignored", } } @@ -88,6 +96,13 @@ pub struct PrefilterOptions { pub generated_paths: HashSet, /// User globs from `.diffmind/config.toml`. pub ignore_globs: Vec, + /// Review documentation as if it were code. Off by default. + /// + /// The escape hatch exists because dropping prose is a judgement, not a + /// fact: a team whose `docs/` carries API contracts may genuinely want it + /// read. Tool config has no such switch — it is never the reviewer's + /// business. + pub include_docs: bool, } /// Filenames that are always a lockfile, regardless of directory. @@ -109,6 +124,58 @@ const LOCKFILE_NAMES: &[&str] = &[ "gradle.lockfile", ]; +/// Prose extensions. A model told to hunt for vulnerabilities will find them in +/// a README if you let it. +const DOC_EXTS: &[&str] = &[ + "md", "mdx", "markdown", "rst", "txt", "adoc", "asciidoc", "org", "tex", +]; + +/// Top-level directories owned by a coding agent or an editor. Nothing under +/// them is program logic, and `.claude/` in particular is instructions written +/// *for* a model — feeding them to one invites it to follow them. +const TOOL_CONFIG_DIRS: &[&str] = &[ + ".claude", + ".cursor", + ".windsurf", + ".aider", + ".vscode", + ".idea", + ".zed", + ".fleet", +]; + +/// Declarative config with no logic in it. Ignore files are matched by shape +/// (`.ignore`) rather than listed, which covers `.helmignore`, +/// `.vercelignore` and whatever the next tool invents. +const TOOL_CONFIG_NAMES: &[&str] = &[ + ".gitattributes", + ".gitmodules", + ".editorconfig", + ".cursorrules", + ".npmrc", + ".nvmrc", + ".browserslistrc", + ".prettierrc", + ".eslintrc", + ".stylelintrc", +]; + +/// True for `.gitignore`, `.dockerignore`, `.prettierignore`, and the rest of +/// the family. +fn is_ignore_file(base: &str) -> bool { + base.starts_with('.') && base.ends_with("ignore") && base.len() > "ignore".len() + 1 +} + +/// `.prettierrc.json`, `.eslintrc.yml` — the same config with a format suffix. +/// +/// Deliberately does not match `prettier.config.js` or `eslint.config.js`: +/// those are executable JavaScript, and a bug in one is a real bug. +fn is_suffixed_rc(base: &str) -> bool { + TOOL_CONFIG_NAMES + .iter() + .any(|name| base.starts_with(&format!("{name}."))) +} + const ASSET_EXTS: &[&str] = &[ "svg", "png", "jpg", "jpeg", "gif", "webp", "ico", "pdf", "woff", "woff2", "ttf", "eot", "mp4", "mp3", "zip", "gz", "wasm", @@ -147,7 +214,22 @@ fn classify_path(path: &str, opts: &PrefilterOptions) -> Option { return Some(DropReason::Lockfile); } + // Checked before the docs rule so `.claude/skills/foo.md` is reported as + // tool config — the reason a reviewer would actually act on — rather than + // as prose that happens to live there. + let first_segment = path.split('/').next().unwrap_or(""); + if TOOL_CONFIG_DIRS.contains(&first_segment) + || TOOL_CONFIG_NAMES.contains(&base) + || is_ignore_file(base) + || is_suffixed_rc(base) + { + return Some(DropReason::ToolConfig); + } + let ext = extension(path); + if !opts.include_docs && DOC_EXTS.contains(&ext) { + return Some(DropReason::Docs); + } if base.ends_with(".min.js") || base.ends_with(".min.css") || ext == "map" { return Some(DropReason::Minified); } @@ -512,6 +594,104 @@ diff --git a/src/auth.rs b/src/auth.rs assert_eq!(report.reason_summary(), "lockfiles"); } + /// Agent instructions are imperative English about credentials, commands + /// and permissions — the exact shape that reads as a security emergency to + /// a reviewer prompt. Every one of these came back HIGH before they were + /// dropped. + #[test] + fn coding_agent_and_editor_config_never_reaches_the_model() { + for path in [ + ".claude/skills/deploy.md", + ".claude/settings.json", + ".cursor/rules.mdc", + ".vscode/settings.json", + ".idea/workspace.xml", + ".windsurf/config.json", + ] { + assert_eq!( + classify_path(path, &opts()), + Some(DropReason::ToolConfig), + "{path} should be dropped as tool config" + ); + } + } + + #[test] + fn ignore_files_and_formatter_config_are_dropped() { + for path in [ + ".gitignore", + ".dockerignore", + ".prettierignore", + ".eslintignore", + ".helmignore", + "packages/web/.gitignore", + ".editorconfig", + ".prettierrc", + ".prettierrc.json", + ".eslintrc.yml", + ".gitattributes", + ] { + assert_eq!( + classify_path(path, &opts()), + Some(DropReason::ToolConfig), + "{path} should be dropped as tool config" + ); + } + } + + /// A config file that is really a program still gets reviewed: a bug in + /// `eslint.config.js` is a bug, not a preference. + #[test] + fn executable_config_is_still_code() { + for path in ["eslint.config.js", "prettier.config.js", "vite.config.ts"] { + assert_eq!(classify_path(path, &opts()), None, "{path} is real code"); + } + } + + /// `.gitignore` is dropped; `gitignore.rs` is a source file that merely + /// sounds like one. + #[test] + fn the_ignore_file_rule_needs_a_leading_dot_and_nothing_after() { + assert_eq!(classify_path("src/gitignore.rs", &opts()), None); + assert_eq!(classify_path("src/ignore.rs", &opts()), None); + assert_eq!(classify_path(".ignore", &opts()), None, "bare .ignore"); + } + + #[test] + fn docs_are_dropped_by_default_and_kept_when_asked_for() { + let readme = "README.md"; + assert_eq!(classify_path(readme, &opts()), Some(DropReason::Docs)); + + let including = PrefilterOptions { + include_docs: true, + ..PrefilterOptions::default() + }; + assert_eq!(classify_path(readme, &including), None); + + // The switch is about prose only — tool config has no opt-in. + assert_eq!( + classify_path(".claude/CLAUDE.md", &including), + Some(DropReason::ToolConfig), + ); + } + + /// Tests are code, and a test that asserts nothing is worth catching. + #[test] + fn test_files_are_reviewed_like_any_other_source() { + for path in [ + "src/auth.test.ts", + "tests/integration_test.rs", + "spec/models/user_spec.rb", + "src/__tests__/login.tsx", + ] { + assert_eq!( + classify_path(path, &opts()), + None, + "{path} must be reviewed" + ); + } + } + #[test] fn surviving_bytes_are_passed_through_unchanged() { // Anchoring depends on hunk headers and line numbers surviving intact.