diff --git a/src/features/chat/domain/pending-requests/approval.ts b/src/features/chat/domain/pending-requests/approval.ts index a92781b8..c60f7ca6 100644 --- a/src/features/chat/domain/pending-requests/approval.ts +++ b/src/features/chat/domain/pending-requests/approval.ts @@ -1,4 +1,5 @@ import { jsonPreview } from "../../../../utils"; +import { pathRelativeToRoot } from "../message-stream/format/path-labels"; import { permissionRows, type MessageStreamPermissionProfile } from "../message-stream/format/permission-rows"; import type { PendingApproval } from "./model"; @@ -9,6 +10,24 @@ interface ApprovalSummaryParts { lines: string[]; } +interface DetailRow { + key: string; + value: string; +} + +interface CommandAction { + type: string; + command?: unknown; + name?: unknown; + path?: unknown; + query?: unknown; +} + +interface NetworkApprovalContext { + host?: unknown; + protocol?: unknown; +} + export function approvalTitle(approval: PendingApproval): string { switch (approval.method) { case "item/commandExecution/requestApproval": @@ -29,16 +48,18 @@ export function approvalResultSummary(approval: PendingApproval): string { return summary.reason ?? summary.target ?? summary.fallback; } -export function approvalDetails(approval: PendingApproval): { key: string; value: string }[] { - const rows: { key: string; value: string }[] = []; +export function approvalDetails(approval: PendingApproval): DetailRow[] { + const rows: DetailRow[] = []; addOptional(rows, "reason", approval.params.reason); switch (approval.method) { case "item/commandExecution/requestApproval": addOptional(rows, "command", approval.params.command); addOptional(rows, "cwd", approval.params.cwd); - addOptional(rows, "actions", approval.params.commandActions); + addOptional(rows, "network", networkApprovalContextLabel(approval.params.networkApprovalContext)); + rows.push(...commandActionRows(approval.params.commandActions, approval.params.cwd)); + rows.push(...prefixedPermissionRows("additional", approval.params.additionalPermissions)); addOptional(rows, "future command rule", approval.params.proposedExecpolicyAmendment); - addOptional(rows, "future network rules", approval.params.proposedNetworkPolicyAmendments); + addOptional(rows, "future network rules", networkPolicyAmendmentsLabel(approval.params.proposedNetworkPolicyAmendments)); break; case "item/fileChange/requestApproval": addOptional(rows, "grant root", approval.params.grantRoot); @@ -80,7 +101,72 @@ function summaryParts(reason: string | null, target: string | null, fallback: st }; } -function addOptional(rows: { key: string; value: string }[], key: string, value: unknown): void { +function commandActionRows(value: unknown, cwd: string | null | undefined): DetailRow[] { + if (!Array.isArray(value) || value.length === 0) return []; + return [ + { + key: value.length === 1 ? "action" : "actions", + value: value + .map((action) => (isCommandAction(action) ? commandActionLabel(action, cwd) : stringValue(action, "unknown action"))) + .join("\n"), + }, + ]; +} + +function commandActionLabel(action: CommandAction, cwd: string | null | undefined): string { + if (action.type === "read") { + const path = pathLabel(action.path, cwd); + return path ? `read ${path}` : `read ${nonEmptyString(action.name) ?? "file"}`; + } + if (action.type === "search") { + const query = nonEmptyString(action.query); + const path = pathLabel(action.path, cwd); + if (query && path) return `search "${query}" in ${path}`; + if (query) return `search "${query}"`; + if (path) return `search ${path}`; + return "search"; + } + if (action.type === "listFiles") { + return `list files ${pathLabel(action.path, cwd) ?? "workspace"}`; + } + return nonEmptyString(action.command) ?? action.type; +} + +function isCommandAction(value: unknown): value is CommandAction { + return value !== null && typeof value === "object" && typeof (value as { type?: unknown }).type === "string"; +} + +function pathLabel(path: unknown, cwd: string | null | undefined): string | null { + const value = nonEmptyString(path); + return value ? pathRelativeToRoot(value, cwd) : null; +} + +function networkApprovalContextLabel(value: unknown): string | null { + if (!value || typeof value !== "object") return null; + const context = value as NetworkApprovalContext; + const host = nonEmptyString(context.host); + if (!host) return null; + const protocol = nonEmptyString(context.protocol); + return protocol ? `${protocol}://${host}` : host; +} + +function prefixedPermissionRows(prefix: string, permissions: unknown): DetailRow[] { + if (!permissions) return []; + return permissionRows(permissions as MessageStreamPermissionProfile).map((row) => ({ ...row, key: `${prefix} ${row.key}` })); +} + +function networkPolicyAmendmentsLabel(value: unknown): string | null { + if (!Array.isArray(value) || value.length === 0) return null; + return value.map(networkPolicyAmendmentLabel).join("\n"); +} + +function networkPolicyAmendmentLabel(value: unknown): string { + if (!value || typeof value !== "object") return stringValue(value, "rule"); + const amendment = value as { action?: unknown; host?: unknown }; + return `${nonEmptyString(amendment.action) ?? "rule"} ${nonEmptyString(amendment.host) ?? "(unknown host)"}`; +} + +function addOptional(rows: DetailRow[], key: string, value: unknown): void { if (value === null || value === undefined) return; if (Array.isArray(value) && value.length === 0) return; rows.push({ key, value: stringValue(value) }); diff --git a/src/features/chat/domain/pending-requests/model.ts b/src/features/chat/domain/pending-requests/model.ts index 57033d58..4e8ac49a 100644 --- a/src/features/chat/domain/pending-requests/model.ts +++ b/src/features/chat/domain/pending-requests/model.ts @@ -109,10 +109,7 @@ export interface PendingUserInput { export function approvalActionKind(action: ApprovalAction): "accept" | "accept-session" | "decline" | "cancel" { if (!isCommandDecisionAction(action)) return action; const decision = action.decision; - if (decision === "accept") return "accept"; - if (decision === "acceptForSession") return "accept-session"; - if (decision === "cancel") return "cancel"; - if (decision === "decline") return "decline"; + if (typeof decision === "string") return simpleApprovalActionKind(decision); if ("acceptWithExecpolicyAmendment" in decision) return "accept-session"; if ("applyNetworkPolicyAmendment" in decision) { return decision.applyNetworkPolicyAmendment.network_policy_amendment.action === "allow" ? "accept-session" : "decline"; @@ -120,6 +117,13 @@ export function approvalActionKind(action: ApprovalAction): "accept" | "accept-s return "decline"; } +function simpleApprovalActionKind(decision: string): "accept" | "accept-session" | "decline" | "cancel" { + if (decision === "accept") return "accept"; + if (decision === "acceptForSession") return "accept-session"; + if (decision === "cancel") return "cancel"; + return "decline"; +} + export function isCommandDecisionAction(action: ApprovalAction): action is CommandApprovalDecisionAction { return typeof action === "object"; } diff --git a/src/features/chat/presentation/pending-requests/approval-view.ts b/src/features/chat/presentation/pending-requests/approval-view.ts index 29637a18..475fb4ea 100644 --- a/src/features/chat/presentation/pending-requests/approval-view.ts +++ b/src/features/chat/presentation/pending-requests/approval-view.ts @@ -6,6 +6,7 @@ import { } from "../../domain/pending-requests/model"; export interface ApprovalActionOption { + id: string; label: string; action: ApprovalAction; className: string; @@ -15,7 +16,8 @@ export function approvalActionOptions(approval: PendingApproval): ApprovalAction if (approval.method !== "item/commandExecution/requestApproval") return defaultApprovalActionOptions(); const decisions = approval.params.availableDecisions; if (!decisions || decisions.length === 0) return defaultApprovalActionOptions(); - return decisions.map((decision) => ({ + return decisions.map((decision, index) => ({ + id: `command-decision:${String(index)}:${commandDecisionKey(decision)}`, label: commandDecisionLabel(decision), action: { kind: "command-decision", decision }, className: commandDecisionClassName(decision), @@ -24,18 +26,15 @@ export function approvalActionOptions(approval: PendingApproval): ApprovalAction function defaultApprovalActionOptions(): ApprovalActionOption[] { return [ - { label: "Allow", action: "accept", className: "mod-cta" }, - { label: "Allow session", action: "accept-session", className: "" }, - { label: "Deny", action: "decline", className: "mod-warning" }, - { label: "Cancel", action: "cancel", className: "" }, + { id: "accept", label: "Allow", action: "accept", className: "mod-cta" }, + { id: "accept-session", label: "Allow session", action: "accept-session", className: "" }, + { id: "decline", label: "Deny", action: "decline", className: "mod-warning" }, + { id: "cancel", label: "Cancel", action: "cancel", className: "" }, ]; } function commandDecisionLabel(decision: CommandApprovalDecision): string { - if (decision === "accept") return "Allow"; - if (decision === "acceptForSession") return "Allow session"; - if (decision === "decline") return "Deny"; - if (decision === "cancel") return "Cancel"; + if (typeof decision === "string") return simpleCommandDecisionLabel(decision); if ("acceptWithExecpolicyAmendment" in decision) return "Allow rule"; if ("applyNetworkPolicyAmendment" in decision) { return decision.applyNetworkPolicyAmendment.network_policy_amendment.action === "allow" ? "Allow network rule" : "Deny network rule"; @@ -43,6 +42,25 @@ function commandDecisionLabel(decision: CommandApprovalDecision): string { return "Choose"; } +function simpleCommandDecisionLabel(decision: string): string { + if (decision === "accept") return "Allow"; + if (decision === "acceptForSession") return "Allow session"; + if (decision === "decline") return "Deny"; + if (decision === "cancel") return "Cancel"; + return "Choose"; +} + +function commandDecisionKey(decision: CommandApprovalDecision): string { + if (typeof decision === "string") return decision; + if ("acceptWithExecpolicyAmendment" in decision) return "acceptWithExecpolicyAmendment"; + if ("applyNetworkPolicyAmendment" in decision) { + const amendment = decision.applyNetworkPolicyAmendment.network_policy_amendment; + const host = amendment["host"]; + return `applyNetworkPolicyAmendment:${amendment.action}:${typeof host === "string" ? host : ""}`; + } + return "unknown"; +} + function commandDecisionClassName(decision: CommandApprovalDecision): string { const kind = approvalActionKind({ kind: "command-decision", decision }); if (kind === "accept") return "mod-cta"; diff --git a/src/features/chat/ui/message-stream/pending-request-block.tsx b/src/features/chat/ui/message-stream/pending-request-block.tsx index f23b931a..60c908d1 100644 --- a/src/features/chat/ui/message-stream/pending-request-block.tsx +++ b/src/features/chat/ui/message-stream/pending-request-block.tsx @@ -109,7 +109,7 @@ function ApprovalCard({
{approval.actions.map((option) => ( { diff --git a/tests/features/chat/protocol/server-requests/approval.test.ts b/tests/features/chat/protocol/server-requests/approval.test.ts index b8754380..2cb99762 100644 --- a/tests/features/chat/protocol/server-requests/approval.test.ts +++ b/tests/features/chat/protocol/server-requests/approval.test.ts @@ -63,9 +63,12 @@ describe("approval model", () => { }); it("uses command approval decisions supplied by app-server", () => { - const networkDecision = { + const allowRegistryDecision = { applyNetworkPolicyAmendment: { network_policy_amendment: { host: "registry.npmjs.org", action: "allow" } }, } satisfies CommandApprovalDecision; + const allowApiDecision = { + applyNetworkPolicyAmendment: { network_policy_amendment: { host: "api.github.com", action: "allow" } }, + } satisfies CommandApprovalDecision; const request: ServerRequest = { id: 30, method: "item/commandExecution/requestApproval", @@ -81,15 +84,50 @@ describe("approval model", () => { commandActions: [], proposedExecpolicyAmendment: null, proposedNetworkPolicyAmendments: [], - availableDecisions: [networkDecision, "decline"], + availableDecisions: [allowRegistryDecision, allowApiDecision, "decline"], }, }; const approval = expectPresent(toPendingApproval(request)); const options = approvalActionOptions(approval); - expect(options.map((option) => option.label)).toEqual(["Allow network rule", "Deny"]); - expect(approvalResponse(approval, expectPresent(options[0]).action)).toEqual({ decision: networkDecision }); - expect(approvalResponse(approval, expectPresent(options[1]).action)).toEqual({ decision: "decline" }); + expect(options.map((option) => option.label)).toEqual(["Allow network rule", "Allow network rule", "Deny"]); + expect(new Set(options.map((option) => option.id)).size).toBe(options.length); + expect(approvalResponse(approval, expectPresent(options[0]).action)).toEqual({ decision: allowRegistryDecision }); + expect(approvalResponse(approval, expectPresent(options[1]).action)).toEqual({ decision: allowApiDecision }); + expect(approvalResponse(approval, expectPresent(options[2]).action)).toEqual({ decision: "decline" }); + }); + + it("keeps future simple command decisions renderable", () => { + const futureDecision = "restartWithNetwork" as CommandApprovalDecision; + const request = { + id: 32, + method: "item/commandExecution/requestApproval", + params: { + command: null, + cwd: "/tmp/project", + threadId: "thread", + turnId: "turn", + itemId: "command", + startedAtMs: 1, + reason: null, + commandActions: [], + proposedExecpolicyAmendment: null, + proposedNetworkPolicyAmendments: [], + availableDecisions: [futureDecision], + }, + } as unknown as ServerRequest; + const approval = expectPresent(toPendingApproval(request)); + const options = approvalActionOptions(approval); + + expect(options).toEqual([ + { + id: "command-decision:0:restartWithNetwork", + label: "Choose", + action: { kind: "command-decision", decision: futureDecision }, + className: "mod-warning", + }, + ]); + expect(approvalResponse(approval, expectPresent(options[0]).action)).toEqual({ decision: futureDecision }); }); it("falls back to generic command approval actions when app-server omits decisions", () => { @@ -196,6 +234,82 @@ describe("approval model", () => { ]); }); + it("formats command approval request payloads as compact detail rows", () => { + const approval = expectPresent( + toPendingApproval({ + id: 25, + method: "item/commandExecution/requestApproval", + params: { + command: "rg TODO src && sed -n '1,20p' src/main.ts", + cwd: "/tmp/project", + threadId: "thread", + turnId: "turn", + itemId: "command", + startedAtMs: 1, + reason: "Needs network access", + networkApprovalContext: { host: "registry.npmjs.org", protocol: "https" }, + commandActions: [ + { type: "search", command: "rg TODO src", query: "TODO", path: "/tmp/project/src" }, + { type: "read", command: "sed -n '1,20p' src/main.ts", name: "main.ts", path: "/tmp/project/src/main.ts" }, + { type: "listFiles", command: "ls", path: null }, + ], + additionalPermissions: { + network: { enabled: true }, + fileSystem: { + read: null, + write: null, + entries: [{ path: { type: "path", path: "/tmp/project/generated" }, access: "write" }], + }, + }, + proposedExecpolicyAmendment: ["rg", "TODO"], + proposedNetworkPolicyAmendments: [ + { host: "registry.npmjs.org", action: "allow" }, + { host: "example.com", action: "deny" }, + ], + }, + }), + ); + + expect(approvalDetails(approval)).toEqual([ + { key: "reason", value: "Needs network access" }, + { key: "command", value: "rg TODO src && sed -n '1,20p' src/main.ts" }, + { key: "cwd", value: "/tmp/project" }, + { key: "network", value: "https://registry.npmjs.org" }, + { key: "actions", value: 'search "TODO" in src\nread src/main.ts\nlist files workspace' }, + { key: "additional network", value: "enabled" }, + { key: "additional filesystem", value: "/tmp/project/generated (write)" }, + { key: "future command rule", value: "rg\nTODO" }, + { key: "future network rules", value: "allow registry.npmjs.org\ndeny example.com" }, + ]); + }); + + it("keeps unknown command approval detail payload entries visible", () => { + const approval = expectPresent( + toPendingApproval({ + id: 26, + method: "item/commandExecution/requestApproval", + params: { + command: null, + cwd: "/tmp/project", + threadId: "thread", + turnId: "turn", + itemId: "command", + startedAtMs: 1, + reason: null, + commandActions: [{ path: "/tmp/project/src/main.ts" }, "legacy action"], + proposedExecpolicyAmendment: null, + proposedNetworkPolicyAmendments: [{ host: "api.github.com" }, { action: "allow" }, "legacy rule"], + }, + }), + ); + + expect(approvalDetails(approval)).toEqual([ + { key: "cwd", value: "/tmp/project" }, + { key: "actions", value: '{\n "path": "/tmp/project/src/main.ts"\n}\nlegacy action' }, + { key: "future network rules", value: "rule api.github.com\nallow (unknown host)\nlegacy rule" }, + ]); + }); + it("omits empty approval detail rows", () => { const approval = expectPresent( toPendingApproval({