From f354aaccfc55b46ec686bc8a3dba0bfc74380edd Mon Sep 17 00:00:00 2001 From: Parfii-bot Date: Tue, 12 May 2026 18:30:01 +0800 Subject: [PATCH] fix(kei-conflict-scan): close 3 backlog bugs + Phase C draft emission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes engine bugs #1, #2, #3 from the user's backlog.md entry dated 2026-05-11 "kei-refactor-engine — 4 false-positive bugs". Bug #4 was fixed in 6cd99982 (wikilink path-norm + handoff scanner removal). ## Bug #1 — vendored marketplaces skip Engine was scanning `plugins/marketplaces/claude-plugins-official/` — vendored upstream code where Constructor Pattern thresholds don't apply. ~246 cp-violations were from this tree. Fix: `tree::should_skip_path()` central filter. Skips any path component named `marketplaces`, `target`, `node_modules`, or `.git`. Applied via `WalkDir::filter_entry()` in `collect_markdown`, `collect_with_ext`, `scanners::cp::scan`, `scanners::orphans::scan`, `scanners::orphans::all_basenames`. `scanners::cp::skip_dir` now delegates to `should_skip_path` (removed the older inline `/target/`-substring check). ## Bug #2 — hooks-share-matcher false-positive class Claude Code hook chains are designed to support N hooks per event by design. `scanners::hooks` was flagging every pair sharing a matcher as a "redundancy conflict" — 9 hooks/medium findings in the last deep-sleep run, every one false-positive. Fix: `scanners::hooks::scan` reduced to a no-op stub returning `Vec::new()`. Module docstring documents the retraction + future direction (a real `hooks-validity` scanner for broken shebangs, missing chmod, syntax errors would replace it). ## Bug #3 — `.patch` file not unified diff Already resolved in prior commit (v0.14.1 retraction in patch.rs): CLI default is `plan-autoresolve.md`, Phase C template references `-autoresolve.md` suffix, `write_patch` is deprecated shim. Only legacy `.patch` artefacts in sync-repo/reports/ remain — those are audit trail, not active. ## Phase C draft file emission (deep-sleep-trigger-prompt.md §6.d) The earlier Phase C template emitted `proposed_rule` markdown blocks only — no actionable artefacts. Extended §6 with step 6.d: when WITH_FORK=1 AND fork branch was created, ALSO write skeleton draft files into the branch: sync-repo/sleep-deep/YYYY-MM-DD/drafts/rules/.md sync-repo/sleep-deep/YYYY-MM-DD/drafts/hooks/.sh Drafts follow pattern-codifier-agent Phase 3 templates. Phase C does NOT register hooks — that's pattern-codifier's job via /sleep-review morning click-flow (skill Phase 3a added in ~/.claude commit 49a320d). This closes the loop: Phase C surfaces draft → morning review clicks approve → pattern-codifier installs → settings.json registered. Smoke-test required in §6.d: every emitted `.sh` MUST `bash -n` clean or be excluded from commit + listed in plan markdown. ## Results on ~/.claude/memory/sync-repo (live data) | Scanner | Before | After | Delta | |-----------|-------:|------:|------:| | orphans | 108 | 1 | -107 | | hooks | 2 | 0 | -2 | | cp | 174 | 0 | -174 | | **TOTAL** | 284 | 1 | -283 | On full ~/.claude scan: total drops from ~1614 (per 2026-05-11 backlog) to 983 (cp=186 + orphans=797 — orphan count high because ~/.claude tree has many memory/chatlogs/ refs out-of-tree). ## Tests 12/12 pass on kei-conflict-scan workspace (4 unit + 8 integration). Pre-existing `oversize_file_flagged` + `orphan_wikilinks_flagged` still green; new `cross_repo_wikilink_not_flagged` + `path_prefixed_wikilink_matches_basename` from 6cd99982 still green. Private mirror at ~/Projects/KeiSeiKit/_primitives/_rust/ synced (4 files: tree.rs, scanners/cp.rs, scanners/orphans.rs, scanners/hooks.rs). Closes backlog "engine-noise-2026-05-11" tag bugs #1, #2, #3. --- .../kei-conflict-scan/src/scanners/cp.rs | 5 +- .../kei-conflict-scan/src/scanners/hooks.rs | 83 +++++-------------- .../kei-conflict-scan/src/scanners/orphans.rs | 14 +++- .../_rust/kei-conflict-scan/src/tree.rs | 19 +++++ .../templates/deep-sleep-trigger-prompt.md | 28 +++++++ 5 files changed, 81 insertions(+), 68 deletions(-) diff --git a/_primitives/_rust/kei-conflict-scan/src/scanners/cp.rs b/_primitives/_rust/kei-conflict-scan/src/scanners/cp.rs index 158d2da..884a85a 100644 --- a/_primitives/_rust/kei-conflict-scan/src/scanners/cp.rs +++ b/_primitives/_rust/kei-conflict-scan/src/scanners/cp.rs @@ -4,7 +4,7 @@ //! Read-only: we do NOT propose a refactor here; refactor-engine decides. use crate::conflict::{Category, Conflict, Severity}; -use crate::tree::{read_lossy, rel}; +use crate::tree::{read_lossy, rel, should_skip_path}; use regex::Regex; use std::path::Path; use walkdir::WalkDir; @@ -42,8 +42,7 @@ pub fn scan(root: &Path) -> Vec { } fn skip_dir(path: &Path) -> bool { - let s = path.to_string_lossy(); - s.contains("/target/") || s.contains("/.git/") || s.contains("/node_modules/") + should_skip_path(path) } fn long_fns(content: &str, ext: &str) -> Vec<(String, usize)> { diff --git a/_primitives/_rust/kei-conflict-scan/src/scanners/hooks.rs b/_primitives/_rust/kei-conflict-scan/src/scanners/hooks.rs index 590f19c..6f21e36 100644 --- a/_primitives/_rust/kei-conflict-scan/src/scanners/hooks.rs +++ b/_primitives/_rust/kei-conflict-scan/src/scanners/hooks.rs @@ -1,67 +1,26 @@ -//! Hook-overlap detector. +//! Hook-overlap detector — DISABLED (2026-05-12). //! -//! Heuristic: two hook scripts in `hooks/` whose first line-match of -//! `tool_name|matcher|event|PreToolUse|PostToolUse|UserPromptSubmit` -//! targets the same value. Flags the pair as possibly-redundant. +//! Previous heuristic flagged any two hook scripts sharing a matcher (event +//! name like `PreToolUse:Edit`, `Stop`, etc.) as a "redundancy conflict". +//! +//! This is fundamentally wrong: Claude Code's hook chain is designed to +//! support N hooks per matcher — they run in registration order, each +//! contributes its own side effect (logging, validation, advisory). Two +//! `Stop`-event hooks are not a conflict, they are the normal architecture. +//! +//! Backlog entry (`~/.claude/memory/sync-repo/backlog.md` 2026-05-11): +//! > "Несколько хуков на один matcher" = false conflict. Claude Code +//! > поддерживает N hooks per event by design. 9 hooks/medium findings — +//! > все ложные. Убрать класс `hooks/medium "shares matcher"` целиком. +//! +//! Scanner kept as a stub returning `Vec::new()` rather than removed from +//! the scanner registry, so the `--only hooks` CLI flag still validates. +//! Real hook-related conflicts (broken shebangs, missing chmod, syntax +//! errors) belong in a future `hooks-validity` scanner — not here. -use crate::conflict::{Category, Conflict, Severity}; -use crate::tree::{collect_with_ext, read_lossy, rel}; -use regex::Regex; +use crate::conflict::Conflict; use std::path::Path; -fn extract_matcher(content: &str) -> Vec { - let rx = Regex::new( - r#"(?i)(?:tool[_ ]?name|matcher|event)\s*[:=]\s*["']?([A-Za-z0-9_|/-]+)["']?"#, - ) - .expect("static regex"); - let mut out = Vec::new(); - for c in rx.captures_iter(content) { - out.push(c[1].to_lowercase()); - } - out.sort(); - out.dedup(); - out -} - -pub fn scan(root: &Path) -> Vec { - let mut files = collect_with_ext(root, "hooks", "sh"); - files.extend(collect_with_ext(root, "hooks", "py")); - files.extend(collect_with_ext(root, "hooks", "rs")); - - let indexed: Vec<(String, Vec)> = files - .iter() - .map(|f| (rel(root, f), extract_matcher(&read_lossy(f)))) - .collect(); - - pairs(&indexed) -} - -fn pairs(indexed: &[(String, Vec)]) -> Vec { - let mut out = Vec::new(); - for i in 0..indexed.len() { - for j in (i + 1)..indexed.len() { - let shared: Vec<&String> = - indexed[i].1.iter().filter(|m| indexed[j].1.contains(m)).collect(); - if !shared.is_empty() { - out.push(overlap_conflict(&indexed[i].0, &indexed[j].0, &shared)); - } - } - } - out -} - -fn overlap_conflict(a: &str, b: &str, shared: &[&String]) -> Conflict { - let shared_str = shared - .iter() - .map(|s| s.as_str()) - .collect::>() - .join(","); - Conflict::new( - Category::Hooks, - Severity::Medium, - vec![a.to_string(), b.to_string()], - format!("hooks share matcher(s): {}", shared_str), - "consider merging into a single hook with union of patterns; keep separate if responsibilities are genuinely distinct".to_string(), - false, - ) +pub fn scan(_root: &Path) -> Vec { + Vec::new() } diff --git a/_primitives/_rust/kei-conflict-scan/src/scanners/orphans.rs b/_primitives/_rust/kei-conflict-scan/src/scanners/orphans.rs index d1c3b30..8876813 100644 --- a/_primitives/_rust/kei-conflict-scan/src/scanners/orphans.rs +++ b/_primitives/_rust/kei-conflict-scan/src/scanners/orphans.rs @@ -10,7 +10,7 @@ //! prose markdown. use crate::conflict::{Category, Conflict, Severity}; -use crate::tree::{read_lossy, rel}; +use crate::tree::{read_lossy, rel, should_skip_path}; use regex::Regex; use std::collections::HashSet; use std::path::Path; @@ -18,7 +18,11 @@ use walkdir::WalkDir; fn all_basenames(root: &Path) -> HashSet { let mut out = HashSet::new(); - for e in WalkDir::new(root).into_iter().filter_map(|e| e.ok()) { + for e in WalkDir::new(root) + .into_iter() + .filter_entry(|e| !should_skip_path(e.path())) + .filter_map(|e| e.ok()) + { if e.file_type().is_file() { if let Some(stem) = e.path().file_stem().and_then(|s| s.to_str()) { out.insert(stem.to_lowercase()); @@ -57,7 +61,11 @@ fn normalize_target(raw: &str) -> Option { pub fn scan(root: &Path) -> Vec { let index = all_basenames(root); let mut out = Vec::new(); - for e in WalkDir::new(root).into_iter().filter_map(|e| e.ok()) { + for e in WalkDir::new(root) + .into_iter() + .filter_entry(|e| !should_skip_path(e.path())) + .filter_map(|e| e.ok()) + { if !e.file_type().is_file() { continue; } diff --git a/_primitives/_rust/kei-conflict-scan/src/tree.rs b/_primitives/_rust/kei-conflict-scan/src/tree.rs index 0c9a665..bb6159c 100644 --- a/_primitives/_rust/kei-conflict-scan/src/tree.rs +++ b/_primitives/_rust/kei-conflict-scan/src/tree.rs @@ -4,6 +4,23 @@ use std::fs; use std::path::{Path, PathBuf}; use walkdir::WalkDir; +/// True if a path should be excluded from every scanner. +/// +/// Skip rules: +/// - `plugins/marketplaces/...` — vendored upstream plugin code; Constructor +/// Pattern thresholds don't apply, refs are external. (backlog #1, 2026-05-12) +/// - `target/`, `node_modules/`, `.git/` — build/vendor noise that should never +/// contribute to architectural conflict counts. +/// +/// Public because the standalone `WalkDir::new(root)` callers in +/// `scanners::cp` and `scanners::orphans` also need to apply it. +pub fn should_skip_path(path: &Path) -> bool { + path.components().any(|c| { + let s = c.as_os_str().to_string_lossy(); + s == "target" || s == "node_modules" || s == ".git" || s == "marketplaces" + }) +} + pub fn collect_markdown(root: &Path, sub: &str) -> Vec { let base = root.join(sub); if !base.exists() { @@ -11,6 +28,7 @@ pub fn collect_markdown(root: &Path, sub: &str) -> Vec { } WalkDir::new(&base) .into_iter() + .filter_entry(|e| !should_skip_path(e.path())) .filter_map(|e| e.ok()) .filter(|e| e.file_type().is_file()) .filter(|e| e.path().extension().is_some_and(|ext| ext == "md")) @@ -25,6 +43,7 @@ pub fn collect_with_ext(root: &Path, sub: &str, ext: &str) -> Vec { } WalkDir::new(&base) .into_iter() + .filter_entry(|e| !should_skip_path(e.path())) .filter_map(|e| e.ok()) .filter(|e| e.file_type().is_file()) .filter(|e| e.path().extension().is_some_and(|e2| e2 == ext)) diff --git a/_primitives/templates/deep-sleep-trigger-prompt.md b/_primitives/templates/deep-sleep-trigger-prompt.md index aa75b19..d650ff5 100644 --- a/_primitives/templates/deep-sleep-trigger-prompt.md +++ b/_primitives/templates/deep-sleep-trigger-prompt.md @@ -120,6 +120,34 @@ v0.12.0 rules AND Phase C is skipped too — the marathon owns the night. to codify them. Without step 6, the affect matrix is a passive log; with it, the matrix feeds back into rule formation per RULE 0.10. + d. **(2026-05-12 extension) Emit DRAFT FILES, not just markdown.** + For each `proposed_rule` block in §6b, also write skeleton files + into the deep-sleep fork branch (only if `WITH_FORK=1` and the + branch was successfully created in §3): + + sync-repo/sleep-deep/YYYY-MM-DD/drafts/rules/.md + sync-repo/sleep-deep/YYYY-MM-DD/drafts/hooks/.sh + + Where `` = kebab-cased pattern name (e.g. + `response-conservatism-check`). File contents follow the + `pattern-codifier-agent` Phase 3 template — frontmatter + Why + + Rule + Severity ladder + Bypass for `.md`; bash skeleton with + stdin JSON parsing + severity exit code + bypass env for `.sh`. + + DO NOT register the hook in `settings.json` here. The morning + `/sleep-review` skill Phase 3a presents each draft pair to the + user via `AskUserQuestion`; user approval triggers + `pattern-codifier-agent` which performs the actual install + + registration. Phase C's job is just to provide click-ready + drafts so the morning review is a 30-second click flow, not a + 30-minute drafting flow. + + Smoke-test: every emitted draft `.sh` MUST `bash -n ` + cleanly (syntax check, no execution). Any draft failing this + check is removed before commit + listed in the plan markdown + under "draft generation failures" so user knows the proposed + rule needs manual drafting. + ## Zero-conflict guarantee Any conflict the refactor-engine marks `requires_human_decision` is