Improve approval request projections

This commit is contained in:
murashit 2026-06-18 12:12:18 +09:00
parent 629f6df984
commit 119fc2820b
5 changed files with 246 additions and 24 deletions

View file

@ -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) });

View file

@ -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";
}

View file

@ -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";

View file

@ -109,7 +109,7 @@ function ApprovalCard({
<div className="codex-panel__pending-request-actions">
{approval.actions.map((option) => (
<ActionButton
key={option.label}
key={option.id}
label={option.label}
className={option.className}
onClick={() => {

View file

@ -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({