Merge pull request #9 from Eilodon/claude/ci-friction-analysis-trivjm

fix(mcp,hook,resolver,diff-impact,tools): friction fixes, taxonomy sweep, and tools.rs god-file split

Author: Eilodon

.claude/hooks/ci-nudge.sh

@@ -73,6 +73,35 @@ is_code_file() {
   esac
 }
 
+# Resolve the git repo root that `cmd` will actually operate on, so the
+# commit/push gate only fires for *this* project's repo — not an unrelated
+# repo the agent is inspecting/debugging elsewhere (e.g. a scratch clone
+# under /tmp, or a test fixture repo). PreToolUse hooks always run with cwd
+# pinned to the project root regardless of any `cd` inside `cmd` (this
+# harness resets the shell's cwd between Bash calls), so neither `pwd` nor
+# the hook JSON's `cwd` field can distinguish this — the command text itself
+# is the only signal available. Best-effort, not a real shell parser: honors
+# the last explicit `git -C <dir>` / `cd <dir> &&`/`;` before the git call;
+# anything it can't confidently resolve falls back to "this repo" (fail
+# toward enforcing the gate, not silently skipping it).
+resolve_git_target_root() {
+  local cmd="$1" explicit_dir=""
+
+  explicit_dir=$(grep -oE 'git[[:space:]]+-C[[:space:]]+[^[:space:]]+' <<<"$cmd" \
+    | tail -1 | awk '{print $NF}')
+
+  if [ -z "$explicit_dir" ]; then
+    explicit_dir=$(grep -oE 'cd[[:space:]]+[^[:space:]&;]+[[:space:]]*(&&|;)' <<<"$cmd" \
+      | tail -1 | sed -E 's/^cd[[:space:]]+//; s/[[:space:]]*(&&|;)$//')
+  fi
+
+  if [ -n "$explicit_dir" ]; then
+    git -C "$explicit_dir" rev-parse --show-toplevel 2>/dev/null
+  else
+    git rev-parse --show-toplevel 2>/dev/null
+  fi
+}
+
 # Record mcp__ci__edit_context / mcp__ci__diff_impact calls as they happen —
 # recorded on PreToolUse (before the call runs) since attempting the check is
 # what matters here, and PreToolUse is all that's needed to observe it.
@@ -103,9 +132,19 @@ case "$tool_name" in
     nudge 'MANDATORY per AGENTS.md Stage 5 — call mcp__ci__edit_context(symbol) before this write if it modifies existing code, never skip (especially if is_hub).'
     ;;
   Bash)
-    if grep -qE '\bgit[[:space:]]+(commit|push)\b' <<<"$command"; then
+    # Broad on purpose: `git -C <dir> commit` / `git --git-dir=<dir> push` put
+    # flags between the subcommand and "commit"/"push", so a tight
+    # `git commit` adjacency check misses them entirely (a false negative —
+    # worse than the false positive this file otherwise guards against).
+    # resolve_git_target_root() + the scope check below is what keeps this
+    # broad match from over-firing on unrelated repos.
+    if grep -qE '\bgit\b.*\b(commit|push)\b' <<<"$command"; then
       if [ "$needs_diff_impact" = "true" ]; then
-        deny 'MANDATORY per AGENTS.md Stage 7 — call mcp__ci__diff_impact(staged=true) before this commit/push, never skip. Files changed since the last diff_impact check.'
+        target_root=$(resolve_git_target_root "$command")
+        project_root=$(git rev-parse --show-toplevel 2>/dev/null)
+        if [ -z "$target_root" ] || [ "$target_root" = "$project_root" ]; then
+          deny 'MANDATORY per AGENTS.md Stage 7 — call mcp__ci__diff_impact(staged=true) before this commit/push, never skip. Files changed since the last diff_impact check.'
+        fi
       fi
     elif grep -qE '\b(grep|rg|ag)\b' <<<"$command"; then
       nudge 'CI available in this repo — prefer mcp__ci__search / mcp__ci__locate instead of grep via Bash (AGENTS.md Stage 2).'

.claude/hooks/session-start-build-ci.sh

@@ -0,0 +1,31 @@
+#!/usr/bin/env bash
+# SessionStart hook: pre-build the ci-cli binary synchronously so the "ci"
+# stdio MCP server (.mcp.json: `cargo run --quiet -p ci-cli -- serve ...`)
+# can finish its handshake inside the MCP client's fixed 30s connection
+# timeout.
+#
+# On a fresh checkout (empty target/), `cargo build -p ci-cli` compiles the
+# full dependency tree (tree-sitter grammars, rusqlite bundled, stack-graphs,
+# embeddings) — measured ~60s even with a warm crates.io registry cache,
+# over 2x the client's timeout, so `cargo run` in .mcp.json reliably times
+# out on the very first connection of every fresh session/container.
+# Once target/ is warm, `cargo run` reconnects in well under 1s (cargo's own
+# freshness check + exec), so paying the compile cost here — before the
+# session (and the MCP client's timer) starts — fixes it for the rest of
+# the session. Must stay synchronous: async mode would let the MCP connect
+# attempt race the build, which is the failure this hook exists to avoid.
+set -uo pipefail
+
+if ! command -v cargo >/dev/null 2>&1; then
+  exit 0
+fi
+
+build_output=$(cargo build --quiet -p ci-cli 2>&1)
+build_status=$?
+
+if [ "$build_status" -ne 0 ]; then
+  jq -n --arg msg "ci-cli pre-build failed (exit $build_status) — the ci MCP server will likely fail to connect (30s client timeout). Build output:
+$build_output" \
+    '{hookSpecificOutput: {hookEventName: "SessionStart", additionalContext: $msg}}'
+fi
+exit 0

.claude/settings.json

@@ -8,6 +8,11 @@
             "type": "command",
             "command": "bash .claude/hooks/session-start-agents-md.sh",
             "timeout": 5
+          },
+          {
+            "type": "command",
+            "command": "bash .claude/hooks/session-start-build-ci.sh",
+            "timeout": 300
           }
         ]
       }

AGENTS.md

@@ -153,12 +153,13 @@ diff_impact(diff="<raw diff text>")   # verify without git
 diff_impact(commits="HEAD~1..HEAD")   # verify already-committed changes
 ```
 
-**Done when**: `aggregate_risk == "low"` and `unindexed_files == []`. Safe to commit.
+**Done when**: `aggregate_risk == "low"` and no `unindexed_files` entry has `reason == "pending_scan"`. Safe to commit.
 
 **Signals**:
 - `aggregate_risk == "critical"` or `"high"` → call `callers` on `affected_symbols[0]` to verify manually
-- `aggregate_risk == "unknown"` → unindexed files present; wait for index to reach `ready`
-- `unindexed_files non-empty` → index incomplete; DO NOT treat diff as safe to push
+- `aggregate_risk == "unknown"` → a `pending_scan` file is present; wait for index to reach `ready`, then retry
+- `unindexed_files[].reason == "pending_scan"` → that file's index is stale/missing; DO NOT treat diff as safe to push yet
+- `unindexed_files[].reason == "out_of_scope"` → not a source file (docs/config/etc.); permanent, harmless, does not affect `aggregate_risk`
 - `suggested_reviewers` present → notify these owners before merging
 
 **Rule: Never commit or push** without calling `diff_impact` first. Under Claude Code with this repo's bundled hook (`.claude/hooks/ci-nudge.sh`), this is enforced: `git commit`/`git push` is denied whenever a file was edited since the last `diff_impact` call.

crates/ci-core/src/analysis/dead_code.rs

@@ -35,7 +35,26 @@ pub fn compute_dead_code_confidence(
     is_private: bool,
     scope_clear: bool,
     coverage: &CoverageData,
+    kind: &str,
 ) -> (&'static str, &'static str) {
+    // Type-level definitions (struct/class/...) aren't "called" the way
+    // functions and methods are — they're referenced via construction
+    // syntax (`Foo { .. }` in Rust) that the call-graph extractor doesn't
+    // track as a call at all, so `caller_count` is 0 for essentially every
+    // one of them regardless of real usage (confirmed: 100% of this repo's
+    // own `struct` symbols have caller_count=0). "Dead code" isn't a
+    // well-formed question for a kind that structurally can't accrue
+    // callers — answering "high confidence dead" here was the single
+    // largest source of `dead_code_pct` false positives.
+    if !matches!(kind, "function" | "method") {
+        let source = if coverage.source != "none" {
+            "static+coverage"
+        } else {
+            "static"
+        };
+        return ("none", source);
+    }
+
     if is_entry_point || is_test || caller_count > 0 {
         let source = if coverage.source != "none" {
             "static+coverage"
@@ -100,6 +119,7 @@ mod tests {
             false,
             false,
             &no_coverage(),
+            "function",
         );
         assert_eq!(conf, "none");
         assert_eq!(src, "static");
@@ -117,6 +137,7 @@ mod tests {
             false,
             false,
             &no_coverage(),
+            "function",
         );
         assert_eq!(conf, "none");
         assert_eq!(src, "static");
@@ -137,6 +158,7 @@ mod tests {
             true,
             true,
             &no_coverage(),
+            "function",
         );
         assert_eq!(conf, "none");
         assert_eq!(src, "static");
@@ -145,8 +167,9 @@ mod tests {
     #[test]
     fn test_runtime_covered_returns_low() {
         let cov = with_coverage("/f.py", &[5]);
-        let (conf, src) =
-            compute_dead_code_confidence("/f.py", 1, 10, 0, false, false, false, false, &cov);
+        let (conf, src) = compute_dead_code_confidence(
+            "/f.py", 1, 10, 0, false, false, false, false, &cov, "function",
+        );
         assert_eq!(conf, "low");
         assert_eq!(src, "static+coverage");
     }
@@ -163,6 +186,7 @@ mod tests {
             true,
             false,
             &no_coverage(),
+            "function",
         );
         assert_eq!(conf, "high");
     }
@@ -179,6 +203,7 @@ mod tests {
             false,
             true,
             &no_coverage(),
+            "function",
         );
         assert_eq!(conf, "medium");
     }
@@ -195,6 +220,7 @@ mod tests {
             false,
             false,
             &no_coverage(),
+            "function",
         );
         assert_eq!(conf, "low");
     }
@@ -211,12 +237,57 @@ mod tests {
             false,
             false,
             &no_coverage(),
+            "function",
         );
         assert_eq!(src, "static");
 
         let cov = with_coverage("/other.py", &[1]);
-        let (_, src) =
-            compute_dead_code_confidence("/f.py", 1, 10, 5, false, false, false, false, &cov);
+        let (_, src) = compute_dead_code_confidence(
+            "/f.py", 1, 10, 5, false, false, false, false, &cov, "function",
+        );
         assert_eq!(src, "static+coverage");
     }
+
+    /// Regression: a `struct` (or any non-callable kind) must never be
+    /// scored "high confidence dead" just because it has zero callers — it
+    /// *always* has zero callers in this codebase's call-graph model (type
+    /// construction isn't tracked as a call), so the old behavior flagged
+    /// essentially every private struct in the project.
+    #[test]
+    fn test_non_callable_kind_is_never_flagged_dead_even_when_private() {
+        let (conf, src) = compute_dead_code_confidence(
+            "/f.rs",
+            1,
+            10,
+            0,
+            false,
+            false,
+            true, // is_private — would return "high" for a function/method
+            true,
+            &no_coverage(),
+            "struct",
+        );
+        assert_eq!(conf, "none");
+        assert_eq!(src, "static");
+    }
+
+    #[test]
+    fn test_method_kind_is_still_evaluated() {
+        let (conf, _) = compute_dead_code_confidence(
+            "/f.rs",
+            1,
+            10,
+            0,
+            false,
+            false,
+            true,
+            true,
+            &no_coverage(),
+            "method",
+        );
+        assert_eq!(
+            conf, "high",
+            "method is a callable kind — still subject to normal dead-code rules"
+        );
+    }
 }