From 8951aa50733a6de2be5d8a66b56733adf582c22a Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Mon, 7 Sep 2026 14:19:20 -0700 Subject: [PATCH] Fix per-app design resolution in Rust hooks Port the approach proposed by tylerjryan in #367, preserving per-file design scope and batch notice attribution. AI assistance: Codex, under maintainer direction. --- crates/hook/src/before_edit.rs | 2 +- crates/hook/src/hook.rs | 33 +++++--- crates/hook/src/hook_lib.rs | 29 ++++++- crates/hook/tests/hook_tests.rs | 135 ++++++++++++++++++++++++++++++++ docs/CLI-CONTRACT.md | 2 + 5 files changed, 188 insertions(+), 13 deletions(-) diff --git a/crates/hook/src/before_edit.rs b/crates/hook/src/before_edit.rs index 666ac8166..e021a9e34 100644 --- a/crates/hook/src/before_edit.rs +++ b/crates/hook/src/before_edit.rs @@ -800,7 +800,7 @@ fn main_flow(rt: &Runtime, stdin: &str) -> Out { { return skip(&audit, "config-ignore-file"); } - let scan = design_system_options(&config, &cwd); + let scan = design_system_options_for_file(rt, &config, &cwd, &file_path); let use_html_engine = match configured { Some(c) => c.engine == "html", None => ext_name == ".html" || ext_name == ".htm", diff --git a/crates/hook/src/hook.rs b/crates/hook/src/hook.rs index c8bf501ed..bb895e36c 100644 --- a/crates/hook/src/hook.rs +++ b/crates/hook/src/hook.rs @@ -5,6 +5,7 @@ use impeccable_core::findings::Finding; use impeccable_core::js; use serde_json::{Map, Value}; +use std::collections::HashMap; use crate::hook_lib::*; use crate::stop_baseline; @@ -168,7 +169,7 @@ pub fn run_hook(rt: &Runtime, stdin: &str) -> RunResult { let mut cache = read_cache(&project_cwd); let session_id = session_key(&session_value); - let scan = design_system_options(&config, &project_cwd); + let mut scans = HashMap::new(); let tiered = per_edit_tiering_active(&config, harness); struct Pending { @@ -275,9 +276,12 @@ pub fn run_hook(rt: &Runtime, stdin: &str) -> RunResult { }; } }; + let scan = scans.entry(file_path.clone()).or_insert_with(|| { + design_system_options_for_file(rt, &config, &project_cwd, file_path) + }); let mut detector_threw = false; let findings: Vec = if use_html_engine { - match detector_detect_html(rt, file_path, &scan) { + match detector_detect_html(rt, file_path, scan) { Ok(f) => f, Err(_) => { detector_threw = true; @@ -285,7 +289,7 @@ pub fn run_hook(rt: &Runtime, stdin: &str) -> RunResult { } } } else { - detector_detect_text(&content, file_path, &scan) + detector_detect_text(&content, file_path, scan) }; if !detector_threw && !use_html_engine { stop_baseline::reconcile(&mut cache, &session_id, file_path, &findings); @@ -352,8 +356,9 @@ pub fn run_hook(rt: &Runtime, stdin: &str) -> RunResult { } if !fresh_groups.is_empty() { + let scan = &scans[&fresh_groups[0].file_path]; let short = footer_mode_short(&mut cache, &session_id); - let reserve = design_note_reserve(rt, &scan, &mut cache, &session_id); + let reserve = design_note_reserve(rt, scan, &mut cache, &session_id); let rendered = render_grouped_template( rt, &fresh_groups, @@ -365,7 +370,7 @@ pub fn run_hook(rt: &Runtime, stdin: &str) -> RunResult { }, ); let text = - append_design_system_note_once(rt, &rendered, &scan, &mut cache, &session_id, &config); + append_design_system_note_once(rt, &rendered, scan, &mut cache, &session_id, &config); commit_footer_shown(rt, &mut cache, &session_id, &text); persist_cache(rt, &project_cwd, &cache); let all: usize = fresh_groups.iter().map(|g| g.findings.len()).sum(); @@ -396,10 +401,11 @@ pub fn run_hook(rt: &Runtime, stdin: &str) -> RunResult { .filter(|p| should_emit_ack_for_file(&p.file_path, &config)) { let base = render_pending_ack(rt, &p.file_path, &p.known, &project_cwd); + let scan = &scans[&p.file_path]; ack = Some(Ack::Pending(append_design_system_note_once( rt, &base, - &scan, + scan, &mut cache, &session_id, &config, @@ -410,10 +416,11 @@ pub fn run_hook(rt: &Runtime, stdin: &str) -> RunResult { .filter(|c| should_emit_ack_for_file(c, &config)) { let base = render_clean_ack(rt, c, &project_cwd); + let scan = &scans[c]; ack = Some(Ack::Clean(append_design_system_note_once( rt, &base, - &scan, + scan, &mut cache, &session_id, &config, @@ -663,7 +670,7 @@ pub fn run_stop_hook(rt: &Runtime, stdin: &str) -> RunResult { ], ); } - let scan = design_system_options(&config, &project_cwd); + let mut scans = HashMap::new(); let mut fresh_groups: Vec = Vec::new(); let mut scanned = 0usize; @@ -704,17 +711,20 @@ pub fn run_stop_hook(rt: &Runtime, stdin: &str) -> RunResult { Some(c) => c.engine == "html", None => ext == ".html" || ext == ".htm", }; + let scan = scans.entry(file_path.clone()).or_insert_with(|| { + design_system_options_for_file(rt, &config, &project_cwd, file_path) + }); // JS: a detector failure tells us nothing about the file. Leave // whatever was remembered alone rather than recording an empty scan // as truth. (detectText cannot throw here: the Rust engine returns // findings directly.) let findings = if use_html_engine { - match detector_detect_html(rt, file_path, &scan) { + match detector_detect_html(rt, file_path, scan) { Ok(f) => f, Err(_) => continue, } } else { - detector_detect_text(&content, file_path, &scan) + detector_detect_text(&content, file_path, scan) }; if !use_html_engine { stop_baseline::reconcile(&mut cache, &session_id, file_path, &findings); @@ -755,6 +765,7 @@ pub fn run_stop_hook(rt: &Runtime, stdin: &str) -> RunResult { ], ); } + let scan = &scans[&fresh_groups[0].file_path]; let short = footer_mode_short(&mut cache, &session_id); let first_unknown = fresh_groups.iter().flat_map(|group| &group.findings) .position(|f| f.name.starts_with("[attribution unknown]")); @@ -801,7 +812,7 @@ pub fn run_stop_hook(rt: &Runtime, stdin: &str) -> RunResult { } let text = if shows_unknown { format!("{attribution_note}\n\n{rendered}") } else { rendered }; let text = - append_design_system_note_once(rt, &text, &scan, &mut cache, &session_id, &config); + append_design_system_note_once(rt, &text, scan, &mut cache, &session_id, &config); commit_footer_shown(rt, &mut cache, &session_id, &text); persist_cache(rt, &project_cwd, &cache); let all: usize = fresh_groups.iter().map(|g| g.findings.len()).sum(); diff --git a/crates/hook/src/hook_lib.rs b/crates/hook/src/hook_lib.rs index 0d57edc81..160b6994b 100644 --- a/crates/hook/src/hook_lib.rs +++ b/crates/hook/src/hook_lib.rs @@ -16,7 +16,7 @@ use impeccable_detect::config::{ normalize_ignore_rule, normalize_ignore_value, normalize_ignore_value_entries, DetectionConfig, IgnoreValueEntry, }; -use impeccable_detect::design_system::{load_design_system_for_cwd, DesignSystem}; +use impeccable_detect::design_system::{load_design_system_for_cwd, resolve_design_md_path, DesignSystem}; use impeccable_detect::detect_text::{detect_text, TextOptions}; use impeccable_detect::engines::{HtmlEngine, ScanOptions}; use once_cell::sync::Lazy; @@ -1645,6 +1645,33 @@ pub fn design_system_options(config: &HookConfig, project_cwd: &str) -> HookScan } } +/// Resolve design rules for the edited workspace without moving hook state. +pub fn design_system_options_for_file( + rt: &Runtime, + config: &HookConfig, + project_cwd: &str, + file_path: &str, +) -> HookScanOptions { + if !config.design_system_enabled { + return HookScanOptions::default(); + } + let project = impeccable_context::context::resolve_project( + project_cwd, + &impeccable_context::target_args::TargetOptions { + target_path: Some(file_path.to_string()), + }, + &rt.env, + ); + // A local DESIGN.md owns the scope even if it has no usable frontmatter. + // Fall back only when the app has no document, never to a sibling app. + let root = if resolve_design_md_path(&project.project_root).is_some() { + &project.project_root + } else { + &project.repo_root + }; + design_system_options(config, root) +} + /// The detector the hook drives: the regex engine from `impeccable-detect` /// and the static HTML engine through the `HtmlEngine` seam. pub fn detector_detect_text( diff --git a/crates/hook/tests/hook_tests.rs b/crates/hook/tests/hook_tests.rs index 1a44d1b26..3ffffd376 100644 --- a/crates/hook/tests/hook_tests.rs +++ b/crates/hook/tests/hook_tests.rs @@ -124,6 +124,141 @@ fn stop_event(cwd: &str, session: &str) -> String { const GRADIENT_CSS: &str = ".title { background: linear-gradient(90deg, #f472b6, #a78bfa); -webkit-background-clip: text; color: transparent; }\n"; const SIDE_TAB_CSS: &str = ".card { border-left: 4px solid #6366f1; border-radius: 8px; }\n"; +fn monorepo_design_fixture(root_design: bool) -> Tmp { + let t = Tmp::new(); + t.write("package.json", r#"{"workspaces":["apps/*"]}"#); + t.write("apps/a/package.json", "{}"); + t.write("apps/b/package.json", "{}"); + t.write("apps/a/DESIGN.md", "---\ncolors:\n primary: '#112233'\n---\n"); + if root_design { + t.write("DESIGN.md", "---\ncolors:\n primary: '#224466'\n---\n"); + } + t.write(".impeccable/config.json", r#"{"hook":{"perEditRules":"all"},"detector":{"advisoryRules":"include"}}"#); + t +} + +// Run identical cases through all three hook entry points. The probe that is +// allowed by the repo palette must still fail against app A's own palette. +fn check_monorepo_design_hook(mode: &str) { + for (root_design, app, color, expected) in [ + (false, "a", "#ff00aa", true), + (false, "b", "#ff00aa", false), + (true, "a", "#224466", true), + (true, "b", "#ff00aa", true), + (true, "b", "#224466", false), + ] { + let t = monorepo_design_fixture(root_design); + let cwd = t.path(); + let source = format!(".probe {{ color: {color}; }}\n"); + let file = t.write(&format!("apps/{app}/src/probe.css"), &source); + let r = rt(&cwd); + let out = match mode { + "post" => hook::run_hook(&r, &edit_event(&cwd, &file, "s1")).stdout, + "before" => { + // A proposed new file must resolve its owning app too. + std::fs::remove_file(&file).unwrap(); + hbe(&r, &cursor(&cwd, "Write", json!({ + "file_path": file, "content": source, + }))).0 + } + "stop" => { + let mut cache = read_cache(&cwd); + touch_file(&mut cache, "s1", &file); + persist_cache(&r, &cwd, &cache); + hook::run_stop_hook(&r, &stop_event(&cwd, "s1")).stdout + } + _ => unreachable!(), + }; + assert_eq!(out.contains("design-system-color"), expected, + "{mode}: root_design={root_design}, app={app}, color={color}: {out}"); + assert!(!t.exists(&format!("apps/{app}/.impeccable/hook.cache.json")), + "design resolution must not relocate hook state"); + } +} + +#[test] +fn monorepo_design_post_edit() { check_monorepo_design_hook("post"); } + +#[test] +fn monorepo_design_before_edit() { check_monorepo_design_hook("before"); } + +#[test] +fn monorepo_design_stop() { check_monorepo_design_hook("stop"); } + +#[test] +fn monorepo_design_document_locations_and_sidecars() { + for location in ["DESIGN.md", "docs/DESIGN.md", ".agents/context/DESIGN.md"] { + let t = monorepo_design_fixture(true); + let cwd = t.path(); + let file = t.write("apps/b/src/probe.css", ".probe {}\n"); + let md = t.write(&format!("apps/b/{location}"), + "---\ntypography:\n body:\n fontFamily: Georgia\nrounded:\n md: 8px\ncolors:\n primary: '#abcdef'\n---\n"); + let sidecar = t.write("apps/b/.impeccable/design.json", "{}"); + let scan = design_system_options_for_file(&rt(&cwd), &read_config(&cwd), &cwd, &file); + let ds = scan.design_system.as_ref().unwrap(); + assert_eq!(ds.source_path.as_deref(), Some(md.as_str())); + assert_eq!(ds.sidecar_path.as_deref(), Some(sidecar.as_str())); + let findings = detector_detect_text( + ".probe { color: #ff0000; font-family: Verdana; border-radius: 19px; }", &file, &scan); + for rule in ["design-system-color", "design-system-font", "design-system-radius"] { + assert!(findings.iter().any(|f| f.antipattern == rule), "{location}: {rule}"); + } + let allowed = detector_detect_text( + ".probe { color: #abcdef; font-family: Georgia; border-radius: 8px; }", &file, &scan); + assert!(allowed.iter().all(|f| !f.antipattern.starts_with("design-system-"))); + } +} + +#[test] +fn monorepo_design_local_document_and_disabled_config_do_not_inherit() { + let t = monorepo_design_fixture(true); + let cwd = t.path(); + let file = t.write("apps/a/src/probe.css", ".probe {}\n"); + t.write("apps/a/DESIGN.md", "# App-specific prose, with no machine-readable tokens\n"); + let r = rt(&cwd); + let mut config = read_config(&cwd); + assert!(design_system_options_for_file(&r, &config, &cwd, &file).design_system.is_none()); + config.design_system_enabled = false; + let sibling = t.write("apps/b/src/probe.css", ".probe {}\n"); + assert!(design_system_options_for_file(&r, &config, &cwd, &sibling).design_system.is_none()); +} + +#[test] +fn monorepo_design_batch_notes_follow_the_displayed_file() { + for mode in ["post-fresh", "post-pending", "post-clean", "stop"] { + for stale_app in ["a", "b"] { + let t = monorepo_design_fixture(true); + let cwd = t.path(); + let source = if mode == "post-clean" { ".probe { color: #112233; }" } + else { ".probe { color: #ff00aa; }" }; + let a = t.write("apps/a/src/probe.css", source); + let b = t.write("apps/b/src/probe.css", ".probe { color: #224466; }"); + let r = rt(&cwd); + if mode == "post-pending" { + hook::run_hook(&r, &edit_event(&cwd, &a, "s1")); + } + let sidecar = t.write(if stale_app == "a" { "apps/a/.impeccable/design.json" } + else { ".impeccable/design.json" }, "{}"); + std::fs::File::options().write(true).open(sidecar).unwrap() + .set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_secs(1_600_000_000)).unwrap(); + let out = if mode == "stop" { + let mut cache = read_cache(&cwd); + touch_file(&mut cache, "s1", &a); + touch_file(&mut cache, "s1", &b); + persist_cache(&r, &cwd, &cache); + hook::run_stop_hook(&r, &stop_event(&cwd, "s1")).stdout + } else { + let event = json!({"session_id":"s1", "cwd":cwd, "hook_event_name":"PostToolUse", + "tool_name":"apply_patch", "tool_input":{"command":format!( + "*** Begin Patch\n*** Update File: {a}\n*** Update File: {b}\n*** End Patch")}}); + hook::run_hook(&r, &event.to_string()).stdout + }; + assert!(out.contains("apps/a/src/probe.css"), "{mode}: {out}"); + assert_eq!(out.contains("DESIGN.md is newer"), stale_app == "a", "{mode}: {out}"); + } + } +} + fn edit_with_original(cwd: &str, file: &str, session: &str, before: &str, old: &str, new: &str) -> String { json!({ "session_id": session, "cwd": cwd, "hook_event_name": "PostToolUse", diff --git a/docs/CLI-CONTRACT.md b/docs/CLI-CONTRACT.md index 8ab809cf3..698705698 100644 --- a/docs/CLI-CONTRACT.md +++ b/docs/CLI-CONTRACT.md @@ -1026,6 +1026,8 @@ Note the global cap across groups is `maxFindings` (5) TOTAL, so later files may `designSystemOptions(config, det, projectCwd)`: `{}` if `config.designSystem.enabled === false` or detector lacks `loadDesignSystemForCwd`; else `{designSystem}` if `det.loadDesignSystemForCwd(projectCwd)` returns truthy (DESIGN.md found walking up to a project boundary; object includes `mdNewerThanJson` = DESIGN.md mtime > `.impeccable/design.json` mtime + 1000ms). +The Rust post-edit, before-edit, and Stop hooks resolve design rules per target file using the shared context project resolver. An app's DESIGN.md (including the usual `.agents/context` and `docs` locations) takes precedence; an app with no document falls back to its repository's document, never a sibling's. The sidecar comes from the selected design scope, and batch notices follow the displayed file's scope. Hook configuration, platform gating, and session cache locations are unchanged. + `appendDesignSystemNote(text, scanOptions)` → `text + '\n\n' + DESIGN_STALE_NOTE` when `scanOptions.designSystem.mdNewerThanJson`. `appendDesignSystemNoteOnce(text, scanOptions, cache, sid, config)`: same, but only if `text.length + NOTE.length + 2 <= max(500, limits.maxChars)` and session flag `designNoteShown` not yet set (sets it). `designNoteReserve(scanOptions, cache, sid)` = `NOTE.length + 2` when note pending and not yet shown, else 0.