Skip to content

fix: return empty response for Allow decisions to preserve Claude permissions #97

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.

Sign up for GitHub

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

Merged
backnotprop merged 1 commit into from
Jan 16, 2026
Merged

fix: return empty response for Allow decisions to preserve Claude permissions #97

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 25 additions & 48 deletions cupcake-core/src/harness/response/claude_code/permission_request.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,12 @@ impl PermissionRequestResponseBuilder {
pub fn build(decision: &EngineDecision, suppress_output: bool) -> CupcakeResponse {
let mut response = CupcakeResponse::empty();

// PermissionRequest uses hookSpecificOutput with nested decision object
// PermissionRequest uses hookSpecificOutput with nested decision object for non-Allow decisions
// For Allow, we return empty response to let Claude use its own permission settings
match decision {
EngineDecision::Allow { .. } => {
response.hook_specific_output = Some(HookSpecificOutput::PermissionRequest {
decision: PermissionRequestDecision {
behavior: PermissionRequestBehavior::Allow,
updated_input: None,
message: None,
interrupt: None,
},
});
// Return empty response for passthrough - let Claude use its own permission settings
// No hook_specific_output means "I have no objections"
}
EngineDecision::Block { feedback } => {
// Block/Deny → deny with message (tells model why denied)
Expand All @@ -63,15 +58,7 @@ impl PermissionRequestResponseBuilder {
}
EngineDecision::Ask { .. } => {
// Ask doesn't make sense for PermissionRequest - it IS the ask dialog
// Treat as Allow (let the normal permission dialog show to user)
response.hook_specific_output = Some(HookSpecificOutput::PermissionRequest {
decision: PermissionRequestDecision {
behavior: PermissionRequestBehavior::Allow,
updated_input: None,
message: None,
interrupt: None,
},
});
// Return empty response to let the normal permission dialog show to user
}
// Modify implies Allow with updated input
EngineDecision::Modify { updated_input, .. } => {
Expand Down Expand Up @@ -124,21 +111,17 @@ mod tests {
use serde_json::json;

#[test]
fn test_permission_request_allow() {
fn test_permission_request_allow_returns_empty_for_passthrough() {
let decision = EngineDecision::Allow {
reason: Some("Test reason".to_string()),
};
let response = PermissionRequestResponseBuilder::build(&decision, false);

match response.hook_specific_output {
Some(HookSpecificOutput::PermissionRequest { decision }) => {
assert_eq!(decision.behavior, PermissionRequestBehavior::Allow);
assert!(decision.message.is_none()); // message is for deny
assert!(decision.updated_input.is_none());
assert!(decision.interrupt.is_none());
}
_ => panic!("Expected PermissionRequest hook output"),
}
// Allow returns empty response for passthrough - let Claude use its own permission settings
assert!(
response.hook_specific_output.is_none(),
"Expected no hook_specific_output for passthrough"
);
assert_eq!(response.suppress_output, None);
}

Expand All @@ -162,22 +145,19 @@ mod tests {
}

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

match response.hook_specific_output {
Some(HookSpecificOutput::PermissionRequest { decision }) => {
assert_eq!(decision.behavior, PermissionRequestBehavior::Allow);
assert!(decision.message.is_none());
assert!(decision.updated_input.is_none());
}
_ => panic!("Expected PermissionRequest hook output"),
}
// Ask returns empty response for passthrough - let the dialog show normally
assert!(
response.hook_specific_output.is_none(),
"Expected no hook_specific_output for passthrough"
);
}

#[test]
Expand Down Expand Up @@ -217,23 +197,20 @@ mod tests {
}

#[test]
fn test_permission_request_json_format() {
fn test_permission_request_allow_json_format_is_empty() {
let decision = EngineDecision::Allow {
reason: Some("Allowed".to_string()),
};
let response = PermissionRequestResponseBuilder::build(&decision, false);

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

// Check nested structure
assert!(json["hookSpecificOutput"]["hookEventName"]
.as_str()
.unwrap()
.eq("PermissionRequest"));
assert_eq!(json["hookSpecificOutput"]["decision"]["behavior"], "allow");
// message should not be present for allow
assert!(json["hookSpecificOutput"]["decision"]["message"].is_null());
// Allow returns empty response - no hookSpecificOutput
assert!(
json["hookSpecificOutput"].is_null(),
"Expected no hookSpecificOutput for passthrough"
);
}

#[test]
Expand Down
30 changes: 11 additions & 19 deletions cupcake-core/src/harness/response/claude_code/pre_tool_use.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@ impl PreToolUseResponseBuilder {
pub fn build(decision: &EngineDecision, suppress_output: bool) -> CupcakeResponse {
let mut response = CupcakeResponse::empty();

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

#[test]
fn test_pre_tool_use_allow() {
fn test_pre_tool_use_allow_returns_empty_for_passthrough() {
let decision = EngineDecision::Allow {
reason: Some("Test reason".to_string()),
};
let response = PreToolUseResponseBuilder::build(&decision, false);

match response.hook_specific_output {
Some(HookSpecificOutput::PreToolUse {
permission_decision,
permission_decision_reason,
..
}) => {
assert_eq!(permission_decision, PermissionDecision::Allow);
assert_eq!(permission_decision_reason, Some("Test reason".to_string()));
}
_ => panic!("Expected PreToolUse hook output"),
}
// Allow returns empty response for passthrough - let Claude use its own permission settings
assert!(
response.hook_specific_output.is_none(),
"Expected no hook_specific_output for passthrough"
);
assert_eq!(response.suppress_output, None);
}

Expand Down