-
Notifications
You must be signed in to change notification settings - Fork 0
feat(hook): PreToolUse redirect classifier, ported from lean-ctx #170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| // PreToolUse redirect classifier — nudges the agent toward agentflare-backend's | ||
| // own tools instead of ad-hoc file-based tracking. Flow ported from lean-ctx's | ||
| // hook_handlers (classify -> fail-open timeout -> dual JSON decision): a | ||
| // synchronous classify step run under a hard wall-clock budget, so a future | ||
| // redirect rule that needs IO (e.g. a backend DB lookup) can never wedge the | ||
| // host's tool call — it just falls through to allow instead. | ||
| use serde_json::{Value, json}; | ||
| use std::sync::mpsc; | ||
| use std::thread; | ||
| use std::time::Duration; | ||
|
|
||
| /// Hard wall-clock budget for classify_and_decide. Sized well under the 5s | ||
| /// timeout `init.rs` wires into `~/.claude/settings.json`'s PreToolUse entry, | ||
| /// so a hang here can never eat the whole hook budget. | ||
| const GATING_TIMEOUT: Duration = Duration::from_millis(2000); | ||
|
|
||
| /// Run `work` under a hard timeout, returning `None` (allow-passthrough) if | ||
| /// it doesn't finish in time. `work` only sends to a channel, never prints, | ||
| /// so a timed-out worker can't double-write stdout once it eventually | ||
| /// finishes. | ||
| fn decide_with_timeout<F>(timeout: Duration, work: F) -> Option<Value> | ||
| where | ||
| F: FnOnce() -> Option<Value> + Send + 'static, | ||
| { | ||
| let (tx, rx) = mpsc::channel(); | ||
| thread::spawn(move || { | ||
| let _ = tx.send(work()); | ||
| }); | ||
| rx.recv_timeout(timeout).unwrap_or(None) | ||
| } | ||
|
|
||
| fn is_spec_like_path(path: &str) -> bool { | ||
| let normalized = path.replace('\\', "/"); | ||
| normalized.contains("/specs/") && normalized.ends_with(".md") | ||
| } | ||
|
|
||
| /// Classify one PreToolUse payload into a redirect reason, if any. Returns | ||
| /// `None` for every tool call that isn't one of agentflare's own redirect | ||
| /// targets. | ||
| fn classify(tool_name: &str, tool_input: Option<&Value>) -> Option<String> { | ||
| match tool_name { | ||
| "TodoWrite" => Some( | ||
| "agentflare-backend's item tracker is wired up for this repo — use the `item` MCP tool (action=create) instead of TodoWrite for anything that should survive past this session.".to_string(), | ||
| ), | ||
| "Write" | "Edit" => { | ||
| let path = tool_input | ||
| .and_then(|v| v.get("file_path").or_else(|| v.get("path"))) | ||
| .and_then(Value::as_str)?; | ||
| is_spec_like_path(path).then(|| { | ||
| format!( | ||
| "specs/design docs/plans belong attached to the relevant item as an asset (the `asset` tool, action=attach), not committed to the repo at '{path}' — create/assign an item first if one doesn't already track this work." | ||
| ) | ||
| }) | ||
| } | ||
| _ => None, | ||
| } | ||
| } | ||
|
|
||
| /// Build the PreToolUse deny decision for a classified redirect, or `None` to | ||
| /// let the call through unchanged. | ||
| pub fn redirect_decision(tool_name: &str, tool_input: Option<&Value>) -> Option<Value> { | ||
| let tool_name = tool_name.to_string(); | ||
| let tool_input = tool_input.cloned(); | ||
| decide_with_timeout(GATING_TIMEOUT, move || { | ||
| let reason = classify(&tool_name, tool_input.as_ref())?; | ||
| Some(json!({ | ||
| "hookSpecificOutput": { | ||
| "hookEventName": "PreToolUse", | ||
| "permissionDecision": "deny", | ||
| "permissionDecisionReason": reason, | ||
| } | ||
| })) | ||
| }) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn classify_redirects_todo_write() { | ||
| let reason = classify("TodoWrite", None).unwrap(); | ||
| assert!(reason.contains("`item` MCP tool")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn classify_redirects_spec_path_write() { | ||
| let input = json!({ "file_path": "docs/superpowers/specs/2026-07-13-foo.md" }); | ||
| let reason = classify("Write", Some(&input)).unwrap(); | ||
| assert!(reason.contains("`asset` tool")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn classify_redirects_spec_path_edit_on_windows_backslashes() { | ||
| let input = json!({ "file_path": "docs\\superpowers\\specs\\foo.md" }); | ||
| assert!(classify("Edit", Some(&input)).is_some()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn classify_ignores_non_spec_write() { | ||
| let input = json!({ "file_path": "src/main.rs" }); | ||
| assert!(classify("Write", Some(&input)).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn classify_ignores_unrelated_tools() { | ||
| assert!(classify("Read", None).is_none()); | ||
| assert!(classify("Bash", None).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn classify_write_with_no_path_falls_through() { | ||
| assert!(classify("Write", None).is_none()); | ||
| assert!(classify("Write", Some(&json!({}))).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn redirect_decision_builds_deny_shape_for_todo_write() { | ||
| let decision = redirect_decision("TodoWrite", None).unwrap(); | ||
| assert_eq!( | ||
| decision["hookSpecificOutput"]["hookEventName"], | ||
| "PreToolUse" | ||
| ); | ||
| assert_eq!(decision["hookSpecificOutput"]["permissionDecision"], "deny"); | ||
| assert!( | ||
| decision["hookSpecificOutput"]["permissionDecisionReason"] | ||
| .as_str() | ||
| .unwrap() | ||
| .contains("item") | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn redirect_decision_is_none_for_unmatched_tool() { | ||
| assert!(redirect_decision("Grep", None).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn decide_with_timeout_fails_open_on_slow_work() { | ||
| let out = decide_with_timeout(Duration::from_millis(50), || { | ||
| std::thread::sleep(Duration::from_millis(500)); | ||
| Some(json!({ "should": "never observe this" })) | ||
| }); | ||
| assert!( | ||
| out.is_none(), | ||
| "a worker slower than the timeout must fail open to None" | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,10 @@ | ||
| // `agentflare init --agent X` — the one explicit, consent-is-the-invocation | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Fragile handling of pre-existing hooks JSON in both
🤖 Prompt for AI Agents |
||
| // setup command. Runs every component (installs included — no separate | ||
| // confirm step, since running this command IS the consent), then wires the | ||
| // host's hook config directly where a hook mechanism exists and can be | ||
| // written without going through a plugin marketplace (Claude Code, Cursor). | ||
| // Codex's hook only activates through its plugin system, so that wiring | ||
| // lives in .codex-plugin/ instead, not here. | ||
| // host's hook config directly where a hook mechanism exists (Claude Code, | ||
| // Cursor, Codex). Codex's hooks are gated behind an experimental feature | ||
| // flag (`[features] codex_hooks = true` in config.toml) that Codex itself | ||
| // requires — wire_codex_hooks() upserts it alongside hooks.json. | ||
| use crate::components::{get_components, rule_targets}; | ||
| use crate::paths::{agentflare_binary, home}; | ||
| use crate::rule_text; | ||
|
|
@@ -215,6 +215,9 @@ pub fn run(agent: &str, yes: bool) { | |
| wire_ponytail_hooks(agent); | ||
| } | ||
| } | ||
| "codex" => { | ||
| wire_codex_hooks(); | ||
| } | ||
| "opencode" => { | ||
| wire_opencode(); | ||
| if has_existing_ponytail_opencode() { | ||
|
|
@@ -361,35 +364,142 @@ fn wire_claude_code() { | |
|
|
||
| fn wire_cursor() { | ||
| let path = cwd().join(".cursor").join("hooks.json"); | ||
| if path.exists() { | ||
| let existing = fs::read_to_string(&path).unwrap_or_default(); | ||
| if existing.contains("agentflare") { | ||
| println!(" skip .cursor/hooks.json (already wired)"); | ||
| return; | ||
| } | ||
| let existing = fs::read_to_string(&path).unwrap_or_default(); | ||
| if !existing.is_empty() && !existing.contains("agentflare") { | ||
| println!(" skip .cursor/hooks.json (exists, not agentflare's — not overwriting)"); | ||
| return; | ||
| } | ||
|
|
||
| let mut content: Value = serde_json::from_str(&existing).unwrap_or_else(|_| json!({})); | ||
| if !content.is_object() { | ||
| content = json!({}); | ||
| } | ||
| let bin = agentflare_binary(); | ||
| let content = json!({ | ||
| "version": 1, | ||
| "hooks": { | ||
| "sessionStart": [{ "command": format!("\"{bin}\" hook session-start"), "type": "command", "timeout": 30 }], | ||
| "beforeSubmitPrompt": [{ "command": format!("\"{bin}\" hook prompt-submit"), "type": "command", "timeout": 10 }] | ||
| } | ||
| }); | ||
| let obj = content.as_object_mut().unwrap(); | ||
| obj.entry("version").or_insert_with(|| json!(1)); | ||
| let hooks = obj | ||
| .entry("hooks") | ||
| .or_insert_with(|| json!({})) | ||
| .as_object_mut() | ||
| .unwrap(); | ||
|
|
||
| let mut added = false; | ||
| if !hooks | ||
| .get("sessionStart") | ||
| .is_some_and(|v| v.to_string().contains("hook session-start")) | ||
| { | ||
| hooks.insert( | ||
| "sessionStart".to_string(), | ||
| json!([{ "command": format!("\"{bin}\" hook session-start"), "type": "command", "timeout": 30 }]), | ||
| ); | ||
| added = true; | ||
| } | ||
| if !hooks | ||
| .get("beforeSubmitPrompt") | ||
| .is_some_and(|v| v.to_string().contains("hook prompt-submit")) | ||
| { | ||
| hooks.insert( | ||
| "beforeSubmitPrompt".to_string(), | ||
| json!([{ "command": format!("\"{bin}\" hook prompt-submit"), "type": "command", "timeout": 10 }]), | ||
| ); | ||
| added = true; | ||
| } | ||
| // preToolUse entries carry a `matcher` instead of `type`/`timeout` — Cursor's | ||
| // own hook schema for this event (Shell|Read|Write|Grep|Delete|Task|MCP:* are | ||
| // the only valid matchers; Cursor has no Glob/Edit/TodoWrite tools, so `Write` | ||
| // is the only matcher agentflare's redirect classifier needs here). | ||
| let pre_arr = hooks | ||
| .entry("preToolUse") | ||
| .or_insert_with(|| json!([])) | ||
| .as_array_mut() | ||
| .unwrap(); | ||
| if !pre_arr | ||
| .iter() | ||
| .any(|v| v.to_string().contains("hook pre-tool-use")) | ||
| { | ||
| pre_arr | ||
| .push(json!({ "matcher": "Write", "command": format!("\"{bin}\" hook pre-tool-use") })); | ||
| added = true; | ||
| } | ||
|
Comment on lines
365
to
+423
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
See consolidated comment (shared root cause with 🤖 Prompt for AI Agents |
||
|
|
||
| if !added { | ||
| println!(" skip .cursor/hooks.json (already wired)"); | ||
| return; | ||
| } | ||
|
|
||
| if let Some(parent) = path.parent() { | ||
| let _ = fs::create_dir_all(parent); | ||
| } | ||
| match fs::write( | ||
| &path, | ||
| serde_json::to_string_pretty(&content).unwrap() + "\n", | ||
| ) { | ||
| Ok(_) => println!(" ok .cursor/hooks.json written"), | ||
| Ok(_) => println!(" ok .cursor/hooks.json wired"), | ||
| Err(e) => println!(" fail writing .cursor/hooks.json: {e}"), | ||
| } | ||
| } | ||
|
|
||
| /// Codex hooks (`~/.codex/hooks.json`, same shape as Claude Code's | ||
| /// settings.json hooks) are gated behind an experimental feature flag Codex | ||
| /// itself requires. Source: https://learn.chatgpt.com/docs/extend/mcp?surface=cli | ||
| /// (verified 2026-07-13) — the flag lives in `config.toml`, not hooks.json. | ||
| fn wire_codex_hooks() { | ||
| let codex_dir = home().join(".codex"); | ||
| let hooks_path = codex_dir.join("hooks.json"); | ||
| let mut settings: Value = fs::read_to_string(&hooks_path) | ||
| .ok() | ||
| .and_then(|s| serde_json::from_str(&s).ok()) | ||
| .unwrap_or_else(|| json!({})); | ||
| if !settings.is_object() { | ||
| settings = json!({}); | ||
| } | ||
| let bin = agentflare_binary(); | ||
|
|
||
| let obj = settings.as_object_mut().unwrap(); | ||
| let hooks = obj.entry("hooks").or_insert_with(|| json!({})); | ||
| let hooks_obj = hooks.as_object_mut().unwrap(); | ||
|
|
||
|
Comment on lines
+442
to
+461
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
See consolidated comment (shared root cause with 🤖 Prompt for AI Agents |
||
| let mut added = false; | ||
| added |= add_hook_entry( | ||
| hooks_obj, | ||
| "PreToolUse", | ||
| "hook pre-tool-use", | ||
| format!("\"{bin}\" hook pre-tool-use"), | ||
| 5, | ||
| ); | ||
|
|
||
| if let Some(parent) = hooks_path.parent() { | ||
| let _ = fs::create_dir_all(parent); | ||
| } | ||
| if added { | ||
| match fs::write( | ||
| &hooks_path, | ||
| serde_json::to_string_pretty(&settings).unwrap() + "\n", | ||
| ) { | ||
| Ok(_) => println!(" ok ~/.codex/hooks.json wired"), | ||
| Err(e) => println!(" fail writing ~/.codex/hooks.json: {e}"), | ||
| } | ||
| } else { | ||
| println!(" skip ~/.codex/hooks.json (already wired)"); | ||
| } | ||
|
|
||
| let config_path = codex_dir.join("config.toml"); | ||
| let config_content = fs::read_to_string(&config_path).unwrap_or_default(); | ||
| if config_content.contains("codex_hooks") { | ||
| println!(" skip ~/.codex/config.toml (codex_hooks flag already present)"); | ||
| return; | ||
| } | ||
| let mut updated = config_content.clone(); | ||
| if !updated.is_empty() && !updated.ends_with('\n') { | ||
| updated.push('\n'); | ||
| } | ||
| updated.push_str("[features]\ncodex_hooks = true\n"); | ||
| match fs::write(&config_path, updated) { | ||
| Ok(_) => println!(" ok ~/.codex/config.toml: codex_hooks feature flag enabled"), | ||
| Err(e) => println!(" fail writing ~/.codex/config.toml: {e}"), | ||
| } | ||
| } | ||
|
Comment on lines
+486
to
+501
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the relevant section of src/init.rs with line numbers and nearby context.
sed -n '430,540p' src/init.rs | cat -n
# Find other references to config.toml / codex_hooks / features handling in the repo.
rg -n "codex_hooks|config\.toml|\[features\]" src tests . -g '!target' -g '!node_modules'Repository: getappz/agentflare Length of output: 1959 Avoid duplicating 🤖 Prompt for AI Agents |
||
|
|
||
| fn wire_opencode() { | ||
| let path = home() | ||
| .join(".config") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
is_spec_like_pathmisses repo-root-relativespecs/*.mdpaths.normalized.contains("/specs/")requires a leading path segment beforespecs. A bare relative path likespecs/design.md(no parent directory) won't match, even though it's exactly the pattern the PR objective describes ("Write/Edit operations targetingspecs/*.md"). No test covers this case either.🐛 Proposed fix
fn is_spec_like_path(path: &str) -> bool { let normalized = path.replace('\\', "/"); - normalized.contains("/specs/") && normalized.ends_with(".md") + (normalized.starts_with("specs/") || normalized.contains("/specs/")) && normalized.ends_with(".md") }📝 Committable suggestion
🤖 Prompt for AI Agents