mirror of
https://github.com/davidkaya/aryx.git
synced 2026-08-27 05:13:58 +02:00
feat: add structured tool approval details to sidecar protocol
Expose SDK permission request details on approval-requested events via a new permissionDetail payload. This preserves the existing title/detail summary while carrying the command, URL, diff, args, and other kind-specific data needed for richer approval UI in the renderer. Also adds sidecar coverage for all supported permission request kinds and verifies the new payload serializes over the protocol. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -251,6 +251,31 @@ public sealed class AgentActivityEventDto : SidecarEventDto
|
|||||||
public string? ToolName { get; init; }
|
public string? ToolName { get; init; }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public sealed class PermissionDetailDto
|
||||||
|
{
|
||||||
|
public string Kind { get; init; } = string.Empty;
|
||||||
|
public string? Intention { get; init; }
|
||||||
|
public string? Command { get; init; }
|
||||||
|
public string? Warning { get; init; }
|
||||||
|
public IReadOnlyList<string>? PossiblePaths { get; init; }
|
||||||
|
public IReadOnlyList<string>? PossibleUrls { get; init; }
|
||||||
|
public bool? HasWriteFileRedirection { get; init; }
|
||||||
|
public string? FileName { get; init; }
|
||||||
|
public string? Diff { get; init; }
|
||||||
|
public string? NewFileContents { get; init; }
|
||||||
|
public string? Path { get; init; }
|
||||||
|
public string? ServerName { get; init; }
|
||||||
|
public string? ToolTitle { get; init; }
|
||||||
|
public object? Args { get; init; }
|
||||||
|
public bool? ReadOnly { get; init; }
|
||||||
|
public string? Url { get; init; }
|
||||||
|
public string? Subject { get; init; }
|
||||||
|
public string? Fact { get; init; }
|
||||||
|
public string? Citations { get; init; }
|
||||||
|
public string? ToolDescription { get; init; }
|
||||||
|
public string? HookMessage { get; init; }
|
||||||
|
}
|
||||||
|
|
||||||
public sealed class ApprovalRequestedEventDto : SidecarEventDto
|
public sealed class ApprovalRequestedEventDto : SidecarEventDto
|
||||||
{
|
{
|
||||||
public string SessionId { get; init; } = string.Empty;
|
public string SessionId { get; init; } = string.Empty;
|
||||||
@@ -262,6 +287,7 @@ public sealed class ApprovalRequestedEventDto : SidecarEventDto
|
|||||||
public string? PermissionKind { get; init; }
|
public string? PermissionKind { get; init; }
|
||||||
public string Title { get; init; } = string.Empty;
|
public string Title { get; init; } = string.Empty;
|
||||||
public string? Detail { get; init; }
|
public string? Detail { get; init; }
|
||||||
|
public PermissionDetailDto? PermissionDetail { get; init; }
|
||||||
}
|
}
|
||||||
|
|
||||||
public sealed class CommandErrorEventDto : SidecarEventDto
|
public sealed class CommandErrorEventDto : SidecarEventDto
|
||||||
|
|||||||
@@ -10,6 +10,14 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
private const string RejectedDecision = "rejected";
|
private const string RejectedDecision = "rejected";
|
||||||
private const string ToolCallApprovalKind = "tool-call";
|
private const string ToolCallApprovalKind = "tool-call";
|
||||||
private const string WebFetchToolName = "web_fetch";
|
private const string WebFetchToolName = "web_fetch";
|
||||||
|
private const string ShellPermissionKind = "shell";
|
||||||
|
private const string WritePermissionKind = "write";
|
||||||
|
private const string ReadPermissionKind = "read";
|
||||||
|
private const string McpPermissionKind = "mcp";
|
||||||
|
private const string UrlPermissionKind = "url";
|
||||||
|
private const string MemoryPermissionKind = "memory";
|
||||||
|
private const string CustomToolPermissionKind = "custom-tool";
|
||||||
|
private const string HookPermissionKind = "hook";
|
||||||
|
|
||||||
private readonly ConcurrentDictionary<string, PendingApprovalRequest> _pendingApprovals = new(StringComparer.Ordinal);
|
private readonly ConcurrentDictionary<string, PendingApprovalRequest> _pendingApprovals = new(StringComparer.Ordinal);
|
||||||
|
|
||||||
@@ -131,6 +139,77 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
PermissionKind = permissionKind,
|
PermissionKind = permissionKind,
|
||||||
Title = title,
|
Title = title,
|
||||||
Detail = detail,
|
Detail = detail,
|
||||||
|
PermissionDetail = BuildPermissionDetail(request),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
internal static PermissionDetailDto BuildPermissionDetail(PermissionRequest request)
|
||||||
|
{
|
||||||
|
ArgumentNullException.ThrowIfNull(request);
|
||||||
|
|
||||||
|
return request switch
|
||||||
|
{
|
||||||
|
PermissionRequestShell shell => new PermissionDetailDto
|
||||||
|
{
|
||||||
|
Kind = ShellPermissionKind,
|
||||||
|
Intention = NormalizeOptionalString(shell.Intention),
|
||||||
|
Command = NormalizeOptionalString(shell.FullCommandText),
|
||||||
|
Warning = NormalizeOptionalString(shell.Warning),
|
||||||
|
PossiblePaths = NormalizeOptionalStringList(shell.PossiblePaths),
|
||||||
|
PossibleUrls = NormalizeOptionalStringList(shell.PossibleUrls.Select(static candidate => candidate.Url)),
|
||||||
|
HasWriteFileRedirection = shell.HasWriteFileRedirection,
|
||||||
|
},
|
||||||
|
PermissionRequestWrite write => new PermissionDetailDto
|
||||||
|
{
|
||||||
|
Kind = WritePermissionKind,
|
||||||
|
Intention = NormalizeOptionalString(write.Intention),
|
||||||
|
FileName = NormalizeOptionalString(write.FileName),
|
||||||
|
Diff = NormalizeOptionalString(write.Diff),
|
||||||
|
NewFileContents = NormalizeOptionalString(write.NewFileContents),
|
||||||
|
},
|
||||||
|
PermissionRequestRead read => new PermissionDetailDto
|
||||||
|
{
|
||||||
|
Kind = ReadPermissionKind,
|
||||||
|
Intention = NormalizeOptionalString(read.Intention),
|
||||||
|
Path = NormalizeOptionalString(read.Path),
|
||||||
|
},
|
||||||
|
PermissionRequestMcp mcp => new PermissionDetailDto
|
||||||
|
{
|
||||||
|
Kind = McpPermissionKind,
|
||||||
|
ServerName = NormalizeOptionalString(mcp.ServerName),
|
||||||
|
ToolTitle = NormalizeOptionalString(mcp.ToolTitle),
|
||||||
|
Args = mcp.Args,
|
||||||
|
ReadOnly = mcp.ReadOnly,
|
||||||
|
},
|
||||||
|
PermissionRequestUrl url => new PermissionDetailDto
|
||||||
|
{
|
||||||
|
Kind = UrlPermissionKind,
|
||||||
|
Intention = NormalizeOptionalString(url.Intention),
|
||||||
|
Url = NormalizeOptionalString(url.Url),
|
||||||
|
},
|
||||||
|
PermissionRequestMemory memory => new PermissionDetailDto
|
||||||
|
{
|
||||||
|
Kind = MemoryPermissionKind,
|
||||||
|
Subject = NormalizeOptionalString(memory.Subject),
|
||||||
|
Fact = NormalizeOptionalString(memory.Fact),
|
||||||
|
Citations = NormalizeOptionalString(memory.Citations),
|
||||||
|
},
|
||||||
|
PermissionRequestCustomTool customTool => new PermissionDetailDto
|
||||||
|
{
|
||||||
|
Kind = CustomToolPermissionKind,
|
||||||
|
ToolDescription = NormalizeOptionalString(customTool.ToolDescription),
|
||||||
|
Args = customTool.Args,
|
||||||
|
},
|
||||||
|
PermissionRequestHook hook => new PermissionDetailDto
|
||||||
|
{
|
||||||
|
Kind = HookPermissionKind,
|
||||||
|
Args = hook.ToolArgs,
|
||||||
|
HookMessage = NormalizeOptionalString(hook.HookMessage),
|
||||||
|
},
|
||||||
|
_ => new PermissionDetailDto
|
||||||
|
{
|
||||||
|
Kind = NormalizeOptionalString(request.Kind) ?? "unknown",
|
||||||
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -307,6 +386,17 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
return string.IsNullOrWhiteSpace(value) ? null : value.Trim();
|
return string.IsNullOrWhiteSpace(value) ? null : value.Trim();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static IReadOnlyList<string>? NormalizeOptionalStringList(IEnumerable<string?> values)
|
||||||
|
{
|
||||||
|
List<string> normalized = values
|
||||||
|
.Select(NormalizeOptionalString)
|
||||||
|
.Where(static value => value is not null)
|
||||||
|
.Cast<string>()
|
||||||
|
.ToList();
|
||||||
|
|
||||||
|
return normalized.Count > 0 ? normalized : null;
|
||||||
|
}
|
||||||
|
|
||||||
private sealed record PendingApprovalRequest(
|
private sealed record PendingApprovalRequest(
|
||||||
string RequestId,
|
string RequestId,
|
||||||
string SessionId,
|
string SessionId,
|
||||||
|
|||||||
@@ -876,6 +876,9 @@ public sealed class CopilotWorkflowRunnerTests
|
|||||||
Assert.Equal("lsp_ts_hover", approvalEvent.ToolName);
|
Assert.Equal("lsp_ts_hover", approvalEvent.ToolName);
|
||||||
Assert.Equal("Approve lsp_ts_hover", approvalEvent.Title);
|
Assert.Equal("Approve lsp_ts_hover", approvalEvent.Title);
|
||||||
Assert.Contains("tool \"lsp_ts_hover\"", approvalEvent.Detail);
|
Assert.Contains("tool \"lsp_ts_hover\"", approvalEvent.Detail);
|
||||||
|
Assert.NotNull(approvalEvent.PermissionDetail);
|
||||||
|
Assert.Equal("custom-tool", approvalEvent.PermissionDetail!.Kind);
|
||||||
|
Assert.Equal("Hover information", approvalEvent.PermissionDetail.ToolDescription);
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
@@ -907,6 +910,197 @@ public sealed class CopilotWorkflowRunnerTests
|
|||||||
Assert.Contains("url permission", approvalEvent.Detail);
|
Assert.Contains("url permission", approvalEvent.Detail);
|
||||||
Assert.Contains("tool \"web_fetch\"", approvalEvent.Detail);
|
Assert.Contains("tool \"web_fetch\"", approvalEvent.Detail);
|
||||||
Assert.Contains("https://example.com/docs", approvalEvent.Detail);
|
Assert.Contains("https://example.com/docs", approvalEvent.Detail);
|
||||||
|
Assert.NotNull(approvalEvent.PermissionDetail);
|
||||||
|
Assert.Equal("url", approvalEvent.PermissionDetail!.Kind);
|
||||||
|
Assert.Equal("Fetch the requested page", approvalEvent.PermissionDetail.Intention);
|
||||||
|
Assert.Equal("https://example.com/docs", approvalEvent.PermissionDetail.Url);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void BuildPermissionDetail_ExtractsShellRequestData()
|
||||||
|
{
|
||||||
|
PermissionDetailDto detail = CopilotApprovalCoordinator.BuildPermissionDetail(
|
||||||
|
new PermissionRequestShell
|
||||||
|
{
|
||||||
|
Kind = "shell",
|
||||||
|
ToolCallId = "tool-call-shell",
|
||||||
|
FullCommandText = "curl https://example.com/docs > docs.json",
|
||||||
|
Intention = "Fetch documentation with curl",
|
||||||
|
Commands =
|
||||||
|
[
|
||||||
|
new PermissionRequestShellCommandsItem
|
||||||
|
{
|
||||||
|
Identifier = "curl",
|
||||||
|
ReadOnly = true,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
PossiblePaths = ["docs.json"],
|
||||||
|
PossibleUrls =
|
||||||
|
[
|
||||||
|
new PermissionRequestShellPossibleUrlsItem
|
||||||
|
{
|
||||||
|
Url = "https://example.com/docs",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
HasWriteFileRedirection = true,
|
||||||
|
CanOfferSessionApproval = false,
|
||||||
|
Warning = "Downloads remote content and writes it to disk.",
|
||||||
|
});
|
||||||
|
|
||||||
|
Assert.Equal("shell", detail.Kind);
|
||||||
|
Assert.Equal("curl https://example.com/docs > docs.json", detail.Command);
|
||||||
|
Assert.Equal("Fetch documentation with curl", detail.Intention);
|
||||||
|
Assert.Equal("Downloads remote content and writes it to disk.", detail.Warning);
|
||||||
|
Assert.Equal(["docs.json"], detail.PossiblePaths);
|
||||||
|
Assert.Equal(["https://example.com/docs"], detail.PossibleUrls);
|
||||||
|
Assert.True(detail.HasWriteFileRedirection);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void BuildPermissionDetail_ExtractsWriteRequestData()
|
||||||
|
{
|
||||||
|
PermissionDetailDto detail = CopilotApprovalCoordinator.BuildPermissionDetail(
|
||||||
|
new PermissionRequestWrite
|
||||||
|
{
|
||||||
|
Kind = "write",
|
||||||
|
ToolCallId = "tool-call-write",
|
||||||
|
Intention = "Update README guidance",
|
||||||
|
FileName = "README.md",
|
||||||
|
Diff = "@@ -1 +1 @@\n-Hello\n+Hello world",
|
||||||
|
NewFileContents = "# README",
|
||||||
|
});
|
||||||
|
|
||||||
|
Assert.Equal("write", detail.Kind);
|
||||||
|
Assert.Equal("Update README guidance", detail.Intention);
|
||||||
|
Assert.Equal("README.md", detail.FileName);
|
||||||
|
Assert.Equal("@@ -1 +1 @@\n-Hello\n+Hello world", detail.Diff);
|
||||||
|
Assert.Equal("# README", detail.NewFileContents);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void BuildPermissionDetail_ExtractsReadRequestData()
|
||||||
|
{
|
||||||
|
PermissionDetailDto detail = CopilotApprovalCoordinator.BuildPermissionDetail(
|
||||||
|
new PermissionRequestRead
|
||||||
|
{
|
||||||
|
Kind = "read",
|
||||||
|
ToolCallId = "tool-call-read",
|
||||||
|
Intention = "Inspect the README",
|
||||||
|
Path = "README.md",
|
||||||
|
});
|
||||||
|
|
||||||
|
Assert.Equal("read", detail.Kind);
|
||||||
|
Assert.Equal("Inspect the README", detail.Intention);
|
||||||
|
Assert.Equal("README.md", detail.Path);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void BuildPermissionDetail_ExtractsMcpRequestData()
|
||||||
|
{
|
||||||
|
PermissionDetailDto detail = CopilotApprovalCoordinator.BuildPermissionDetail(
|
||||||
|
new PermissionRequestMcp
|
||||||
|
{
|
||||||
|
Kind = "mcp",
|
||||||
|
ToolCallId = "tool-call-mcp",
|
||||||
|
ServerName = "Git MCP",
|
||||||
|
ToolName = "git.status",
|
||||||
|
ToolTitle = "Git Status",
|
||||||
|
Args = new Dictionary<string, object?>
|
||||||
|
{
|
||||||
|
["path"] = ".",
|
||||||
|
},
|
||||||
|
ReadOnly = true,
|
||||||
|
});
|
||||||
|
|
||||||
|
Assert.Equal("mcp", detail.Kind);
|
||||||
|
Assert.Equal("Git MCP", detail.ServerName);
|
||||||
|
Assert.Equal("Git Status", detail.ToolTitle);
|
||||||
|
Assert.True(detail.ReadOnly);
|
||||||
|
|
||||||
|
Dictionary<string, object?> args = Assert.IsType<Dictionary<string, object?>>(detail.Args);
|
||||||
|
Assert.Equal(".", args["path"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void BuildPermissionDetail_ExtractsUrlRequestData()
|
||||||
|
{
|
||||||
|
PermissionDetailDto detail = CopilotApprovalCoordinator.BuildPermissionDetail(
|
||||||
|
new PermissionRequestUrl
|
||||||
|
{
|
||||||
|
Kind = "url",
|
||||||
|
ToolCallId = "tool-call-url",
|
||||||
|
Intention = "Fetch the requested page",
|
||||||
|
Url = "https://example.com/docs",
|
||||||
|
});
|
||||||
|
|
||||||
|
Assert.Equal("url", detail.Kind);
|
||||||
|
Assert.Equal("Fetch the requested page", detail.Intention);
|
||||||
|
Assert.Equal("https://example.com/docs", detail.Url);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void BuildPermissionDetail_ExtractsMemoryRequestData()
|
||||||
|
{
|
||||||
|
PermissionDetailDto detail = CopilotApprovalCoordinator.BuildPermissionDetail(
|
||||||
|
new PermissionRequestMemory
|
||||||
|
{
|
||||||
|
Kind = "memory",
|
||||||
|
ToolCallId = "tool-call-memory",
|
||||||
|
Subject = "repo conventions",
|
||||||
|
Fact = "Use Bun for script execution.",
|
||||||
|
Citations = "package.json",
|
||||||
|
});
|
||||||
|
|
||||||
|
Assert.Equal("memory", detail.Kind);
|
||||||
|
Assert.Equal("repo conventions", detail.Subject);
|
||||||
|
Assert.Equal("Use Bun for script execution.", detail.Fact);
|
||||||
|
Assert.Equal("package.json", detail.Citations);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void BuildPermissionDetail_ExtractsCustomToolRequestData()
|
||||||
|
{
|
||||||
|
PermissionDetailDto detail = CopilotApprovalCoordinator.BuildPermissionDetail(
|
||||||
|
new PermissionRequestCustomTool
|
||||||
|
{
|
||||||
|
Kind = "custom tool",
|
||||||
|
ToolName = "lsp_ts_hover",
|
||||||
|
ToolDescription = "Hover information",
|
||||||
|
Args = new Dictionary<string, object?>
|
||||||
|
{
|
||||||
|
["file"] = "src/index.ts",
|
||||||
|
["line"] = 12,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
Assert.Equal("custom-tool", detail.Kind);
|
||||||
|
Assert.Equal("Hover information", detail.ToolDescription);
|
||||||
|
|
||||||
|
Dictionary<string, object?> args = Assert.IsType<Dictionary<string, object?>>(detail.Args);
|
||||||
|
Assert.Equal("src/index.ts", args["file"]);
|
||||||
|
Assert.Equal(12, args["line"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void BuildPermissionDetail_ExtractsHookRequestData()
|
||||||
|
{
|
||||||
|
PermissionDetailDto detail = CopilotApprovalCoordinator.BuildPermissionDetail(
|
||||||
|
new PermissionRequestHook
|
||||||
|
{
|
||||||
|
Kind = "hook",
|
||||||
|
ToolName = "web_fetch",
|
||||||
|
ToolArgs = new Dictionary<string, object?>
|
||||||
|
{
|
||||||
|
["url"] = "https://example.com",
|
||||||
|
},
|
||||||
|
HookMessage = "Review required before fetch",
|
||||||
|
});
|
||||||
|
|
||||||
|
Assert.Equal("hook", detail.Kind);
|
||||||
|
Assert.Equal("Review required before fetch", detail.HookMessage);
|
||||||
|
|
||||||
|
Dictionary<string, object?> args = Assert.IsType<Dictionary<string, object?>>(detail.Args);
|
||||||
|
Assert.Equal("https://example.com", args["url"]);
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
@@ -249,6 +249,13 @@ public sealed class SidecarProtocolHostTests
|
|||||||
AgentName = "Primary",
|
AgentName = "Primary",
|
||||||
PermissionKind = "tool access",
|
PermissionKind = "tool access",
|
||||||
Title = "Approve tool access",
|
Title = "Approve tool access",
|
||||||
|
PermissionDetail = new PermissionDetailDto
|
||||||
|
{
|
||||||
|
Kind = "shell",
|
||||||
|
Command = "git status",
|
||||||
|
Intention = "Inspect repository state",
|
||||||
|
PossiblePaths = ["README.md"],
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
return [];
|
return [];
|
||||||
@@ -284,6 +291,11 @@ public sealed class SidecarProtocolHostTests
|
|||||||
Assert.Equal("approval-1", approvalEvent.GetProperty("approvalId").GetString());
|
Assert.Equal("approval-1", approvalEvent.GetProperty("approvalId").GetString());
|
||||||
Assert.Equal("tool-call", approvalEvent.GetProperty("approvalKind").GetString());
|
Assert.Equal("tool-call", approvalEvent.GetProperty("approvalKind").GetString());
|
||||||
Assert.Equal("Approve tool access", approvalEvent.GetProperty("title").GetString());
|
Assert.Equal("Approve tool access", approvalEvent.GetProperty("title").GetString());
|
||||||
|
JsonElement permissionDetail = approvalEvent.GetProperty("permissionDetail");
|
||||||
|
Assert.Equal("shell", permissionDetail.GetProperty("kind").GetString());
|
||||||
|
Assert.Equal("git status", permissionDetail.GetProperty("command").GetString());
|
||||||
|
Assert.Equal("Inspect repository state", permissionDetail.GetProperty("intention").GetString());
|
||||||
|
Assert.Equal("README.md", Assert.Single(permissionDetail.GetProperty("possiblePaths").EnumerateArray()).GetString());
|
||||||
},
|
},
|
||||||
completionEvent =>
|
completionEvent =>
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user