Skip to content

Commit 2c12e37

Browse files
authored
fix: return empty response for Allow decisions to preserve Claude permissions (#97)
For PreToolUse and PermissionRequest hooks, returning an explicit `permissionDecision: "allow"` bypasses Claude's native permission system. Per Claude Code docs, returning an empty `{}` response means "I have no objections" and lets Claude fall through to its own permission settings. This fix ensures cupcake only intervenes with explicit responses for Block/Deny/Ask decisions, not for Allow. Fixes #96
1 parent 47d859b commit 2c12e37

File tree

2 files changed

+36
-67
lines changed

2 files changed

+36
-67
lines changed

cupcake-core/src/harness/response/claude_code/permission_request.rs

Lines changed: 25 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,12 @@ impl PermissionRequestResponseBuilder {
3838
pub fn build(decision: &EngineDecision, suppress_output: bool) -> CupcakeResponse {
3939
let mut response = CupcakeResponse::empty();
4040

41-
// PermissionRequest uses hookSpecificOutput with nested decision object
41+
// PermissionRequest uses hookSpecificOutput with nested decision object for non-Allow decisions
42+
// For Allow, we return empty response to let Claude use its own permission settings
4243
match decision {
4344
EngineDecision::Allow { .. } => {
44-
response.hook_specific_output = Some(HookSpecificOutput::PermissionRequest {
45-
decision: PermissionRequestDecision {
46-
behavior: PermissionRequestBehavior::Allow,
47-
updated_input: None,
48-
message: None,
49-
interrupt: None,
50-
},
51-
});
45+
// Return empty response for passthrough - let Claude use its own permission settings
46+
// No hook_specific_output means "I have no objections"
5247
}
5348
EngineDecision::Block { feedback } => {
5449
// Block/Deny → deny with message (tells model why denied)
@@ -63,15 +58,7 @@ impl PermissionRequestResponseBuilder {
6358
}
6459
EngineDecision::Ask { .. } => {
6560
// Ask doesn't make sense for PermissionRequest - it IS the ask dialog
66-
// Treat as Allow (let the normal permission dialog show to user)
67-
response.hook_specific_output = Some(HookSpecificOutput::PermissionRequest {
68-
decision: PermissionRequestDecision {
69-
behavior: PermissionRequestBehavior::Allow,
70-
updated_input: None,
71-
message: None,
72-
interrupt: None,
73-
},
74-
});
61+
// Return empty response to let the normal permission dialog show to user
7562
}
7663
// Modify implies Allow with updated input
7764
EngineDecision::Modify { updated_input, .. } => {
@@ -124,21 +111,17 @@ mod tests {
124111
use serde_json::json;
125112

126113
#[test]
127-
fn test_permission_request_allow() {
114+
fn test_permission_request_allow_returns_empty_for_passthrough() {
128115
let decision = EngineDecision::Allow {
129116
reason: Some("Test reason".to_string()),
130117
};
131118
let response = PermissionRequestResponseBuilder::build(&decision, false);
132119

133-
match response.hook_specific_output {
134-
Some(HookSpecificOutput::PermissionRequest { decision }) => {
135-
assert_eq!(decision.behavior, PermissionRequestBehavior::Allow);
136-
assert!(decision.message.is_none()); // message is for deny
137-
assert!(decision.updated_input.is_none());
138-
assert!(decision.interrupt.is_none());
139-
}
140-
_ => panic!("Expected PermissionRequest hook output"),
141-
}
120+
// Allow returns empty response for passthrough - let Claude use its own permission settings
121+
assert!(
122+
response.hook_specific_output.is_none(),
123+
"Expected no hook_specific_output for passthrough"
124+
);
142125
assert_eq!(response.suppress_output, None);
143126
}
144127

@@ -162,22 +145,19 @@ mod tests {
162145
}
163146

164147
#[test]
165-
fn test_permission_request_ask_becomes_allow() {
148+
fn test_permission_request_ask_returns_empty_for_passthrough() {
166149
// Ask doesn't make sense for PermissionRequest (it IS the ask dialog)
167-
// So Ask is treated as Allow (let the dialog show normally)
150+
// So Ask returns empty response to let the dialog show normally
168151
let decision = EngineDecision::Ask {
169152
reason: "Please confirm action".to_string(),
170153
};
171154
let response = PermissionRequestResponseBuilder::build(&decision, false);
172155

173-
match response.hook_specific_output {
174-
Some(HookSpecificOutput::PermissionRequest { decision }) => {
175-
assert_eq!(decision.behavior, PermissionRequestBehavior::Allow);
176-
assert!(decision.message.is_none());
177-
assert!(decision.updated_input.is_none());
178-
}
179-
_ => panic!("Expected PermissionRequest hook output"),
180-
}
156+
// Ask returns empty response for passthrough - let the dialog show normally
157+
assert!(
158+
response.hook_specific_output.is_none(),
159+
"Expected no hook_specific_output for passthrough"
160+
);
181161
}
182162

183163
#[test]
@@ -217,23 +197,20 @@ mod tests {
217197
}
218198

219199
#[test]
220-
fn test_permission_request_json_format() {
200+
fn test_permission_request_allow_json_format_is_empty() {
221201
let decision = EngineDecision::Allow {
222202
reason: Some("Allowed".to_string()),
223203
};
224204
let response = PermissionRequestResponseBuilder::build(&decision, false);
225205

226-
// Serialize to JSON to verify format
206+
// Serialize to JSON to verify format - should be empty object for passthrough
227207
let json = serde_json::to_value(&response).unwrap();
228208

229-
// Check nested structure
230-
assert!(json["hookSpecificOutput"]["hookEventName"]
231-
.as_str()
232-
.unwrap()
233-
.eq("PermissionRequest"));
234-
assert_eq!(json["hookSpecificOutput"]["decision"]["behavior"], "allow");
235-
// message should not be present for allow
236-
assert!(json["hookSpecificOutput"]["decision"]["message"].is_null());
209+
// Allow returns empty response - no hookSpecificOutput
210+
assert!(
211+
json["hookSpecificOutput"].is_null(),
212+
"Expected no hookSpecificOutput for passthrough"
213+
);
237214
}
238215

239216
#[test]

cupcake-core/src/harness/response/claude_code/pre_tool_use.rs

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,12 @@ impl PreToolUseResponseBuilder {
1212
pub fn build(decision: &EngineDecision, suppress_output: bool) -> CupcakeResponse {
1313
let mut response = CupcakeResponse::empty();
1414

15-
// PreToolUse always uses hookSpecificOutput with permissionDecision
15+
// PreToolUse uses hookSpecificOutput with permissionDecision for non-Allow decisions
16+
// For Allow, we return empty response to let Claude use its own permission settings
1617
match decision {
17-
EngineDecision::Allow { reason } => {
18-
response.hook_specific_output = Some(HookSpecificOutput::PreToolUse {
19-
permission_decision: PermissionDecision::Allow,
20-
permission_decision_reason: reason.clone(),
21-
updated_input: None,
22-
});
18+
EngineDecision::Allow { .. } => {
19+
// Return empty response for passthrough - let Claude use its own permission settings
20+
// No hook_specific_output means "I have no objections"
2321
}
2422
EngineDecision::Block { feedback } => {
2523
response.hook_specific_output = Some(HookSpecificOutput::PreToolUse {
@@ -62,23 +60,17 @@ mod tests {
6260
use super::*;
6361

6462
#[test]
65-
fn test_pre_tool_use_allow() {
63+
fn test_pre_tool_use_allow_returns_empty_for_passthrough() {
6664
let decision = EngineDecision::Allow {
6765
reason: Some("Test reason".to_string()),
6866
};
6967
let response = PreToolUseResponseBuilder::build(&decision, false);
7068

71-
match response.hook_specific_output {
72-
Some(HookSpecificOutput::PreToolUse {
73-
permission_decision,
74-
permission_decision_reason,
75-
..
76-
}) => {
77-
assert_eq!(permission_decision, PermissionDecision::Allow);
78-
assert_eq!(permission_decision_reason, Some("Test reason".to_string()));
79-
}
80-
_ => panic!("Expected PreToolUse hook output"),
81-
}
69+
// Allow returns empty response for passthrough - let Claude use its own permission settings
70+
assert!(
71+
response.hook_specific_output.is_none(),
72+
"Expected no hook_specific_output for passthrough"
73+
);
8274
assert_eq!(response.suppress_output, None);
8375
}
8476

0 commit comments

Comments
 (0)