diff --git a/src/commands/checkpoint_agent/bash_tool.rs b/src/commands/checkpoint_agent/bash_tool.rs index be8a890d61..9326514efc 100644 --- a/src/commands/checkpoint_agent/bash_tool.rs +++ b/src/commands/checkpoint_agent/bash_tool.rs @@ -314,7 +314,7 @@ fn normalize_tool_name(tool_name: &str) -> &str { pub fn classify_tool(agent: Agent, tool_name: &str) -> ToolClass { match agent { Agent::Claude => match tool_name { - "Write" | "Edit" | "MultiEdit" => ToolClass::FileEdit, + "Write" | "Edit" | "MultiEdit" | "NotebookEdit" => ToolClass::FileEdit, "Bash" => ToolClass::Bash, _ => ToolClass::Skip, }, diff --git a/src/commands/checkpoint_agent/presets/claude.rs b/src/commands/checkpoint_agent/presets/claude.rs index 26a978600e..9bcc1c9b0e 100644 --- a/src/commands/checkpoint_agent/presets/claude.rs +++ b/src/commands/checkpoint_agent/presets/claude.rs @@ -48,9 +48,16 @@ impl AgentPreset for ClaudePreset { )); } + let tool_class = parse::optional_str_multi(&data, &["tool_name", "toolName"]) + .map(|name| bash_tool::classify_tool(Agent::Claude, name)) + // Preserve legacy Claude payloads that predate tool_name. + .unwrap_or(ToolClass::FileEdit); + if tool_class == ToolClass::Skip { + return Ok(Vec::new()); + } + let cwd = parse::required_str(&data, "cwd")?; let transcript_path = parse::required_str(&data, "transcript_path")?; - let session_id = parse::optional_str(&data, "session_id") .map(|s| s.to_string()) .unwrap_or_else(|| { @@ -58,13 +65,10 @@ impl AgentPreset for ClaudePreset { .unwrap_or_else(|_| "unknown".to_string()) }); - let tool_name = parse::optional_str_multi(&data, &["tool_name", "toolName"]); let hook_event = parse::optional_str_multi(&data, &["hook_event_name", "hookEventName"]); let tool_use_id = parse::str_or_default_multi(&data, &["tool_use_id", "toolUseId"], "bash"); - let is_bash = tool_name - .map(|n| bash_tool::classify_tool(Agent::Claude, n) == ToolClass::Bash) - .unwrap_or(false); + let is_bash = tool_class == ToolClass::Bash; let context = PresetContext { agent_id: AgentId { @@ -220,6 +224,59 @@ mod tests { } } + #[test] + fn test_claude_ignores_read_only_and_unsupported_tools() { + for hook_event in ["PreToolUse", "PostToolUse"] { + for tool_name in ["Read", "Glob", "Grep", "Task", "UnknownTool"] { + let input = json!({ + "hook_event_name": hook_event, + "tool_name": tool_name, + "transcript_path": "/does/not/exist.jsonl" + }) + .to_string(); + + let events = ClaudePreset.parse(&input, "t_test123456789a").unwrap(); + assert!( + events.is_empty(), + "{hook_event} {tool_name} unexpectedly produced events" + ); + } + } + } + + #[test] + fn test_ignored_claude_hook_produces_no_checkpoint_requests() { + let input = json!({ + "hook_event_name": "PostToolUse", + "tool_name": "Read", + "transcript_path": "/does/not/exist.jsonl" + }) + .to_string(); + + let requests = crate::commands::checkpoint_agent::orchestrator::execute_preset_checkpoint( + "claude", &input, + ) + .unwrap(); + assert!(requests.is_empty()); + } + + #[test] + fn test_claude_preserves_all_mutating_file_tools() { + for tool_name in ["Write", "Edit", "MultiEdit"] { + let pre = make_claude_hook_input("PreToolUse", tool_name); + assert!(matches!( + ClaudePreset.parse(&pre, "t_test123456789a").unwrap()[..], + [ParsedHookEvent::PreFileEdit(_)] + )); + + let post = make_claude_hook_input("PostToolUse", tool_name); + assert!(matches!( + ClaudePreset.parse(&post, "t_test123456789a").unwrap()[..], + [ParsedHookEvent::PostFileEdit(_)] + )); + } + } + #[test] fn test_claude_session_id_from_filename() { let input = json!({ diff --git a/tests/integration/claude_code.rs b/tests/integration/claude_code.rs index 6a497f0927..b96d907d19 100644 --- a/tests/integration/claude_code.rs +++ b/tests/integration/claude_code.rs @@ -76,13 +76,7 @@ fn test_claude_preset_no_filepath_when_tool_input_missing() { .parse(hook_input, "t_test") .expect("Failed to run ClaudePreset"); - assert_eq!(events.len(), 1); - match &events[0] { - ParsedHookEvent::PostFileEdit(e) => { - assert!(e.file_paths.is_empty()); - } - _ => panic!("Expected PostFileEdit"), - } + assert!(events.is_empty()); } #[test] @@ -141,7 +135,7 @@ fn test_claude_preset_ignores_cursor_payload() { } #[test] -fn test_claude_preset_does_not_ignore_when_transcript_path_is_claude() { +fn test_claude_preset_ignores_unsupported_tool_when_transcript_path_is_claude() { let temp = tempfile::tempdir().unwrap(); let claude_dir = temp.path().join(".claude").join("projects"); fs::create_dir_all(&claude_dir).unwrap(); @@ -169,12 +163,7 @@ fn test_claude_preset_does_not_ignore_when_transcript_path_is_claude() { .parse(&hook_input, "t_test") .expect("Expected native Claude preset handling"); - match &events[0] { - ParsedHookEvent::PostFileEdit(e) => { - assert_eq!(e.context.agent_id.tool, "claude"); - } - _ => panic!("Expected PostFileEdit"), - } + assert!(events.is_empty()); } #[test] @@ -387,6 +376,6 @@ crate::reuse_tests_in_worktree!( test_claude_preset_no_filepath_when_tool_input_missing, test_claude_preset_ignores_vscode_copilot_payload, test_claude_preset_ignores_cursor_payload, - test_claude_preset_does_not_ignore_when_transcript_path_is_claude, + test_claude_preset_ignores_unsupported_tool_when_transcript_path_is_claude, test_claude_e2e_prefers_latest_checkpoint_for_prompts, ); diff --git a/tests/integration/rewrite_ops_attribution.rs b/tests/integration/rewrite_ops_attribution.rs index 46de0999c9..4bfd7615a4 100644 --- a/tests/integration/rewrite_ops_attribution.rs +++ b/tests/integration/rewrite_ops_attribution.rs @@ -319,7 +319,7 @@ fn claude_checkpoint(repo: &TestRepo, event: &str, file_path: &Path, session_id: let hook_input = json!({ "cwd": repo.path().to_string_lossy().to_string(), "hook_event_name": event, - "tool_name": "Update", + "tool_name": "Edit", "session_id": session_id, "transcript_path": transcript_path.to_string_lossy().to_string(), "tool_use_id": format!("{session_id}-{event}"),