Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 0 additions & 4 deletions md/design/common-issues.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@

The following issues were identified by auditing our hook implementations against the agent reference docs (`md/design/agent-details/`). They don't cause crashes (the fallback path handles events without agent-specific handlers) but mean some features are incomplete.

### `toolArgs` not parsed (Copilot)

Copilot sends `toolArgs` as a JSON *string* (not an object). Our `CopilotPreToolUsePayload` declares it as `serde_json::Value` and passes it through as-is in `to_hook_payload()`. Downstream code expecting structured tool args will get a raw string. Should parse the JSON string into a `Value` during conversion.

### `permissionDecision` dropped (Copilot)

`CopilotPreToolUseOutput::from_hook_output()` never maps `permissionDecision` or `permissionDecisionReason` from the builtin hook output. If a builtin handler wants to deny a tool call, the decision is silently lost in Copilot output.
Expand Down
90 changes: 86 additions & 4 deletions src/hook_schema/copilot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,28 @@ use crate::hook_schema::{
Agent, AgentHookEvent, AgentHookInput, AgentHookOutput, erase_agent_hook_event, symposium,
};

/// Copilot sends `toolArgs` as a JSON-encoded string (per Copilot's hook protocol),
/// while the canonical `PreToolUseInput.tool_input` is a structured `Value`. Parse
/// the string into a `Value` when crossing into canonical form. If parsing fails
/// or the field is already structured, pass it through unchanged.
fn parse_copilot_tool_args(raw: &serde_json::Value) -> serde_json::Value {
if let Some(s) = raw.as_str()
&& let Ok(v) = serde_json::from_str::<serde_json::Value>(s)
{
return v;
}
raw.clone()
}

/// Inverse of [`parse_copilot_tool_args`]: when writing out to Copilot wire format,
/// re-encode a structured `Value` back into a JSON string.
fn stringify_for_copilot(v: &serde_json::Value) -> serde_json::Value {
match v {
serde_json::Value::String(_) => v.clone(),
_ => serde_json::Value::String(v.to_string()),
}
}

pub struct Copilot;
impl Agent for Copilot {
fn event(&self, event: super::HookEvent) -> Option<Box<dyn super::ErasedAgentHookEvent>> {
Expand Down Expand Up @@ -117,7 +139,7 @@ impl AgentHookInput for CopilotPreToolUseInput {
fn to_symposium(&self) -> symposium::InputEvent {
symposium::InputEvent::PreToolUse(symposium::PreToolUseInput {
tool_name: self.tool_name.clone(),
tool_input: self.tool_args.clone(),
tool_input: parse_copilot_tool_args(&self.tool_args),
session_id: None,
cwd: self.cwd.clone(),
})
Expand All @@ -130,7 +152,7 @@ impl AgentHookInput for CopilotPreToolUseInput {
timestamp: None,
cwd: p.cwd.clone(),
tool_name: p.tool_name.clone(),
tool_args: p.tool_input.clone(),
tool_args: stringify_for_copilot(&p.tool_input),
rest: serde_json::Map::new(),
}
}
Expand Down Expand Up @@ -183,7 +205,7 @@ impl AgentHookInput for CopilotPostToolUseInput {
fn to_symposium(&self) -> symposium::InputEvent {
symposium::InputEvent::PostToolUse(symposium::PostToolUseInput {
tool_name: self.tool_name.clone(),
tool_input: self.tool_args.clone(),
tool_input: parse_copilot_tool_args(&self.tool_args),
tool_response: self.tool_response.clone(),
session_id: None,
cwd: self.cwd.clone(),
Expand All @@ -197,7 +219,7 @@ impl AgentHookInput for CopilotPostToolUseInput {
timestamp: None,
cwd: p.cwd.clone(),
tool_name: p.tool_name.clone(),
tool_args: p.tool_input.clone(),
tool_args: stringify_for_copilot(&p.tool_input),
tool_response: p.tool_response.clone(),
rest: serde_json::Map::new(),
}
Expand Down Expand Up @@ -323,3 +345,63 @@ copilot_output_impl!(
SessionStart,
SessionStartOutput {}
);

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn pre_tool_use_parses_string_tool_args() {
let raw = r#"{"toolName":"bash","toolArgs":"{\"command\":\"ls\"}"}"#;
let input: CopilotPreToolUseInput = serde_json::from_str(raw).unwrap();
let symposium::InputEvent::PreToolUse(canon) = input.to_symposium() else {
panic!("wrong event type")
};
assert_eq!(canon.tool_input, serde_json::json!({"command": "ls"}));
}

#[test]
fn pre_tool_use_round_trips_object_to_copilot_string() {
let canon = symposium::InputEvent::PreToolUse(symposium::PreToolUseInput {
tool_name: "bash".into(),
tool_input: serde_json::json!({"command": "ls"}),
session_id: None,
cwd: None,
});
let out = CopilotPreToolUseInput::from_symposium(&canon);
let s = out
.tool_args
.as_str()
.expect("toolArgs must be a JSON string on the Copilot wire");
let reparsed: serde_json::Value = serde_json::from_str(s).unwrap();
assert_eq!(reparsed, serde_json::json!({"command": "ls"}));
}

#[test]
fn post_tool_use_parses_string_tool_args() {
let raw = r#"{"toolName":"bash","toolArgs":"{\"command\":\"ls\"}","toolResponse":{}}"#;
let input: CopilotPostToolUseInput = serde_json::from_str(raw).unwrap();
let symposium::InputEvent::PostToolUse(canon) = input.to_symposium() else {
panic!("wrong event type")
};
assert_eq!(canon.tool_input, serde_json::json!({"command": "ls"}));
}

#[test]
fn post_tool_use_round_trips_object_to_copilot_string() {
let canon = symposium::InputEvent::PostToolUse(symposium::PostToolUseInput {
tool_name: "bash".into(),
tool_input: serde_json::json!({"command": "ls"}),
tool_response: serde_json::json!({"exit_code": 0}),
session_id: None,
cwd: None,
});
let out = CopilotPostToolUseInput::from_symposium(&canon);
let s = out
.tool_args
.as_str()
.expect("toolArgs must be a JSON string on the Copilot wire");
let reparsed: serde_json::Value = serde_json::from_str(s).unwrap();
assert_eq!(reparsed, serde_json::json!({"command": "ls"}));
}
}
Loading