mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-08-28 19:11:03 +08:00
Merge pull request #12728 from Kilo-Org/feat/explain-tool-auto-approval-tui
feat(tui): explain tool auto approval
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@kilocode/cli": minor
|
||||
---
|
||||
|
||||
Show why a tool call was auto-approved or denied in the TUI, and record the denial reason on the tool call metadata (visible in `kilo export`) alongside the existing auto-approval reason.
|
||||
@@ -98,4 +98,35 @@ export namespace PermissionProvenance {
|
||||
rule: { permission: rule.permission, pattern: rule.pattern, action: rule.action },
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Classify why a tool call was denied, from the `ruleset` a `DeniedError` carries.
|
||||
*
|
||||
* `DeniedError.ruleset` is untyped (`Schema.Any`). `Permission.ask`'s main deny path sets it to
|
||||
* the exact rule `resolve()` matched against the request's pattern (via `Wildcard.match`), not
|
||||
* merely the deny-permission subset — two deny rules for different patterns under the same
|
||||
* permission (e.g. `bash: { "git push *": deny, "rm -rf *": deny }`) would otherwise be
|
||||
* indistinguishable by permission alone, misattributing the denial to whichever rule happens to
|
||||
* sort last.
|
||||
*
|
||||
* Other denial paths (hard Ask/Plan/Architect vetoes, headless-subagent policy) don't carry a
|
||||
* specific rule, so `ruleset` there is still just the permission subset (or absent). Synthesize
|
||||
* an explicit `deny` rule for the request's permission/pattern in that case: falling through to
|
||||
* `classify({ rule: undefined })` would report the exact same `{ source: "default" }` shape the
|
||||
* *approval* fallback uses for "no rule matched," rendering a refusal as an auto-approval.
|
||||
*/
|
||||
export function classifyDenial(input: {
|
||||
ruleset: unknown
|
||||
permission: string
|
||||
patterns: readonly string[]
|
||||
agent: string
|
||||
origins: Origins
|
||||
}): Approval {
|
||||
const candidate = input.ruleset as Partial<Permission.Rule> | undefined
|
||||
const rule =
|
||||
candidate?.action === "deny" && typeof candidate.pattern === "string"
|
||||
? (candidate as Permission.Rule)
|
||||
: { permission: input.permission, pattern: input.patterns[0] ?? "*", action: "deny" as const }
|
||||
return classify({ rule, agent: input.agent, origins: input.origins })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -232,9 +232,8 @@ export const layer = Layer.effect(
|
||||
}
|
||||
// kilocode_change end
|
||||
if (rule.action === "deny") {
|
||||
return yield* new DeniedError({
|
||||
ruleset: subset(request.permission, ruleset), // kilocode_change
|
||||
})
|
||||
// kilocode_change - carry the deciding rule (not just the permission subset) for provenance
|
||||
return yield* new DeniedError({ ruleset: rule })
|
||||
}
|
||||
// kilocode_change start - skill shell forces a prompt instead of honoring an allow/auto-approve rule
|
||||
if (forceAsk) {
|
||||
|
||||
@@ -26,6 +26,7 @@ import { ModelV2 } from "@opencode-ai/core/model"
|
||||
// kilocode_change start
|
||||
import { SwePruner } from "@/kilocode/swe-pruner"
|
||||
import { Config } from "@/config/config"
|
||||
import { PermissionProvenance } from "@/kilocode/permission/provenance"
|
||||
// kilocode_change end
|
||||
|
||||
export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
|
||||
@@ -82,6 +83,20 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
|
||||
}).pipe(
|
||||
// record why the call was allowed onto the tool part, then discard the outcome for the tool-facing ask
|
||||
Effect.tap((approval) => input.processor.metadata(options.toolCallId, { metadata: { approval } })),
|
||||
// record why the call was denied too, so JSON exports and clients can explain the denial
|
||||
Effect.tapErrorTag("PermissionDeniedError", (err) =>
|
||||
input.processor.metadata(options.toolCallId, {
|
||||
metadata: {
|
||||
approval: PermissionProvenance.classifyDenial({
|
||||
ruleset: err.ruleset,
|
||||
permission: req.permission,
|
||||
patterns: req.patterns,
|
||||
agent: input.agent.name,
|
||||
origins: permissionOrigins,
|
||||
}),
|
||||
},
|
||||
}),
|
||||
),
|
||||
Effect.asVoid,
|
||||
Effect.orDie,
|
||||
),
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
import { describe, expect, test } from "bun:test"
|
||||
import { Cause, Effect, Exit, Layer } from "effect"
|
||||
import { Bus } from "../../../src/bus"
|
||||
import { Permission } from "../../../src/permission"
|
||||
import { PermissionProvenance } from "../../../src/kilocode/permission/provenance"
|
||||
import { EventV2Bridge } from "../../../src/event-v2-bridge"
|
||||
import { Database } from "@opencode-ai/core/database/database"
|
||||
import { SessionID } from "../../../src/session/schema"
|
||||
import * as Config from "../../../src/config/config"
|
||||
import * as CrossSpawnSpawner from "@opencode-ai/core/cross-spawn-spawner"
|
||||
import { provideTmpdirInstance } from "../../fixture/fixture"
|
||||
import { testEffect } from "../../lib/effect"
|
||||
|
||||
const env = Layer.mergeAll(
|
||||
Permission.layer.pipe(
|
||||
Layer.provide(EventV2Bridge.defaultLayer),
|
||||
Layer.provide(Config.defaultLayer),
|
||||
Layer.provide(Database.defaultLayer),
|
||||
),
|
||||
Config.defaultLayer,
|
||||
Bus.layer,
|
||||
CrossSpawnSpawner.defaultLayer,
|
||||
)
|
||||
const it = testEffect(env)
|
||||
|
||||
const ask = (input: Parameters<Permission.Interface["ask"]>[0]) =>
|
||||
Effect.gen(function* () {
|
||||
const permission = yield* Permission.Service
|
||||
return yield* permission.ask(input)
|
||||
})
|
||||
|
||||
function withDir(options: { git?: boolean } | undefined, self: (dir: string) => Effect.Effect<any, any, any>) {
|
||||
return provideTmpdirInstance(self, options)
|
||||
}
|
||||
|
||||
describe("Permission.ask denial provenance", () => {
|
||||
it.live(
|
||||
"attributes a denial to the rule that matched the request's pattern, not just the textually-last deny rule for the permission",
|
||||
() =>
|
||||
withDir({ git: true }, () =>
|
||||
Effect.gen(function* () {
|
||||
// Two deny rules under the same permission for different patterns. Matching by
|
||||
// permission alone (e.g. findLast over rules with action "deny") would pick
|
||||
// "rm -rf *" here since it sorts last, even though "git push *" is the one that
|
||||
// actually matched the request.
|
||||
const ruleset = [
|
||||
{ permission: "bash", pattern: "git push *", action: "deny" as const },
|
||||
{ permission: "bash", pattern: "rm -rf *", action: "deny" as const },
|
||||
]
|
||||
const exit = yield* ask({
|
||||
sessionID: SessionID.make("session_test"),
|
||||
permission: "bash",
|
||||
patterns: ["git push origin main"],
|
||||
metadata: {},
|
||||
always: [],
|
||||
ruleset,
|
||||
}).pipe(Effect.exit)
|
||||
|
||||
expect(Exit.isFailure(exit)).toBe(true)
|
||||
const err = Exit.isFailure(exit) ? Cause.squash(exit.cause) : undefined
|
||||
expect(err).toBeInstanceOf(Permission.DeniedError)
|
||||
|
||||
const approval = PermissionProvenance.classifyDenial({
|
||||
ruleset: (err as Permission.DeniedError).ruleset,
|
||||
permission: "bash",
|
||||
patterns: ["git push origin main"],
|
||||
agent: "build",
|
||||
origins: undefined,
|
||||
})
|
||||
expect(approval.rule).toEqual({ permission: "bash", pattern: "git push *", action: "deny" })
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
||||
test("a denial with no specific rule (e.g. a headless-subagent policy denial) is still reported as denied, not as an ambiguous default approval", () => {
|
||||
// Some denial paths don't carry a specific rule -- Permission.ask's headless-subagent policy
|
||||
// denial, for instance, still sets `ruleset` to the plain deny-permission subset (an array,
|
||||
// with no `.action`/`.pattern` of its own). classify({ rule: undefined }) reports
|
||||
// { source: "default" } -- the same shape the *approval* fallback produces for "no rule
|
||||
// matched" -- so without a synthesized deny rule, a refusal would render (and export) as an
|
||||
// auto-approval.
|
||||
const approval = PermissionProvenance.classifyDenial({
|
||||
ruleset: [{ permission: "bash", pattern: "*", action: "ask" as const }],
|
||||
permission: "bash",
|
||||
patterns: ["rm -rf /"],
|
||||
agent: "build",
|
||||
origins: undefined,
|
||||
})
|
||||
expect(approval.rule?.action).toBe("deny")
|
||||
expect(approval.rule).toEqual({ permission: "bash", pattern: "rm -rf /", action: "deny" })
|
||||
})
|
||||
})
|
||||
@@ -157,14 +157,15 @@ const registry = Layer.effect(
|
||||
const it = testEffect(registry)
|
||||
const mac = process.platform === "darwin" && existsSync("/usr/bin/sandbox-exec") ? it.live : it.live.skip
|
||||
|
||||
function resolve(ctx: InstanceContext) {
|
||||
function resolve(ctx: InstanceContext, metadataCalls: { toolCallID: string; value: Record<string, any> }[] = []) {
|
||||
return SessionTools.resolve({
|
||||
agent,
|
||||
model,
|
||||
session: session(ctx.directory),
|
||||
processor: {
|
||||
message: message(ctx),
|
||||
metadata: () => Effect.void,
|
||||
// capture metadata writes so tests can assert on recorded approval provenance
|
||||
metadata: (toolCallID, value) => Effect.sync(() => void metadataCalls.push({ toolCallID, value })),
|
||||
completeToolCall: () => Effect.void,
|
||||
},
|
||||
bypassAgentCheck: false,
|
||||
@@ -355,3 +356,29 @@ mac("confines a model-originated sandboxed process to the active worktree", () =
|
||||
expect(yield* exists(primary)).toBe(false)
|
||||
}),
|
||||
)
|
||||
|
||||
it.live("records why a denied tool call was refused on the tool part's metadata", () =>
|
||||
Effect.gen(function* () {
|
||||
const dirs = yield* fixture()
|
||||
const metadataCalls: { toolCallID: string; value: Record<string, any> }[] = []
|
||||
const deniedRule = { permission: "bash", pattern: "*", action: "deny" as const, source: "project" as const }
|
||||
const overrides = Layer.mergeAll(
|
||||
TestConfig.layer({ get: () => Effect.succeed({ sandbox: { enabled: false } }) }),
|
||||
Layer.mock(Permission.Service)({
|
||||
ask: () => Effect.fail(new Permission.DeniedError({ ruleset: deniedRule })),
|
||||
}),
|
||||
)
|
||||
const tools = yield* resolve(dirs.ctx, metadataCalls).pipe(Effect.provide(overrides))
|
||||
const shell = tools.bash
|
||||
if (!shell) yield* Effect.die(new Error("bash tool is missing"))
|
||||
|
||||
const result = yield* call(shell, { command: "echo hi", workdir: dirs.a }, "call-denied").pipe(Effect.exit)
|
||||
|
||||
expect(Exit.isFailure(result)).toBe(true)
|
||||
const approval = metadataCalls.find((c) => c.toolCallID === "call-denied")?.value?.metadata?.approval
|
||||
expect(approval).toEqual({
|
||||
source: "project",
|
||||
rule: { permission: "bash", pattern: "*", action: "deny" },
|
||||
})
|
||||
}),
|
||||
)
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
import type { RGBA } from "@opentui/core"
|
||||
import { Show } from "solid-js"
|
||||
import type { PermissionProvenance } from "@/kilocode/permission/provenance"
|
||||
import type { ToolState } from "@kilocode/sdk/v2"
|
||||
|
||||
/** `state.metadata` off any tool state, including the pending variant that lacks the field. */
|
||||
export function stateMetadata(state: ToolState | undefined) {
|
||||
return state && "metadata" in state ? state.metadata : undefined
|
||||
}
|
||||
|
||||
const SOURCES = ["agent", "global", "project", "yolo", "session", "manual", "default"] as const
|
||||
|
||||
/** Read the approval/denial provenance off a tool part's metadata, if present. */
|
||||
export function toolApprovalFrom(metadata: Record<string, unknown> | undefined) {
|
||||
const value = metadata?.approval
|
||||
if (!value || typeof value !== "object") return undefined
|
||||
const approval = value as PermissionProvenance.Approval
|
||||
return (SOURCES as readonly string[]).includes(approval.source) ? approval : undefined
|
||||
}
|
||||
|
||||
function sourceLabel(approval: PermissionProvenance.Approval): string | undefined {
|
||||
switch (approval.source) {
|
||||
case "agent":
|
||||
return approval.agent ? `by the ${approval.agent} agent` : "by the agent"
|
||||
case "global":
|
||||
return "by your global config"
|
||||
case "project":
|
||||
return "by the project config"
|
||||
case "yolo":
|
||||
return "by auto-approve (YOLO) mode"
|
||||
case "session":
|
||||
return "by a session auto-approve rule"
|
||||
case "default":
|
||||
return "by default"
|
||||
default:
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
|
||||
/** A short "why" line describing an auto-approval or denial, for the TUI's plain-text rows. */
|
||||
export function describeApproval(metadata: Record<string, unknown> | undefined): string | undefined {
|
||||
const approval = toolApprovalFrom(metadata)
|
||||
if (!approval) return undefined
|
||||
const manual = approval.source === "manual"
|
||||
const decision = manual ? "approved by you" : approval.rule?.action === "deny" ? "denied" : "auto-approved"
|
||||
if (manual) return decision
|
||||
const source = sourceLabel(approval)
|
||||
const rule = approval.rule
|
||||
// The catch-all "*"/"*" rule carries no useful detail; let the source alone explain it.
|
||||
const ruleText =
|
||||
rule && !(rule.permission === "*" && rule.pattern === "*") ? ` (matched ${rule.permission} \`${rule.pattern}\`)` : ""
|
||||
return source ? `${decision} ${source}${ruleText}` : decision
|
||||
}
|
||||
|
||||
/**
|
||||
* The muted "why" annotation appended inline after a tool's title/summary text, matching the
|
||||
* `RoutedModelMeta.Badge` convention. Rendered on the header line (not after the tool's own
|
||||
* output) so it reads as metadata about the call rather than part of the output itself.
|
||||
*/
|
||||
export function ApprovalBadge(props: { note: string | undefined; color?: RGBA }) {
|
||||
return (
|
||||
<Show when={props.note}>
|
||||
<span style={{ fg: props.color }}> · {props.note}</span>
|
||||
</Show>
|
||||
)
|
||||
}
|
||||
@@ -63,6 +63,7 @@ import { Toast, useToast } from "../../ui/toast"
|
||||
import { useKV } from "../../context/kv.tsx"
|
||||
import stripAnsi from "strip-ansi"
|
||||
import { usePromptRef } from "../../context/prompt"
|
||||
import { ApprovalBadge, describeApproval, stateMetadata } from "../../kilocode/tool-approval" // kilocode_change
|
||||
import { useEpilogue } from "../../context/epilogue"
|
||||
import { normalizePath } from "../../util/path"
|
||||
import { PermissionPrompt } from "./permission"
|
||||
@@ -2204,6 +2205,8 @@ function InlineTool(props: {
|
||||
|
||||
const failed = createMemo(() => Boolean(error() && !denied()))
|
||||
const clickable = createMemo(() => Boolean(props.onClick || failed()))
|
||||
// kilocode_change - explain why the call was auto-approved or denied
|
||||
const approvalNote = createMemo(() => describeApproval(stateMetadata(props.part.state)))
|
||||
const fg = createMemo(() => {
|
||||
if (props.color) return props.color
|
||||
if (permission()) return theme.warning
|
||||
@@ -2228,6 +2231,8 @@ function InlineTool(props: {
|
||||
failure={props.failure}
|
||||
spinner={props.spinner}
|
||||
separate={props.separate}
|
||||
note={approvalNote()} // kilocode_change
|
||||
noteColor={theme.textMuted} // kilocode_change
|
||||
onMouseOver={() => clickable() && setHover(true)}
|
||||
onMouseOut={() => setHover(false)}
|
||||
onMouseUp={() => {
|
||||
@@ -2258,6 +2263,8 @@ export function InlineToolRow(props: {
|
||||
failure?: string
|
||||
spinner?: boolean
|
||||
separate?: boolean
|
||||
note?: string // kilocode_change - why the call was auto-approved or denied
|
||||
noteColor?: RGBA // kilocode_change
|
||||
children: JSX.Element
|
||||
onMouseOver?: () => void
|
||||
onMouseOut?: () => void
|
||||
@@ -2310,6 +2317,8 @@ export function InlineToolRow(props: {
|
||||
attributes={props.denied ? TextAttributes.STRIKETHROUGH : undefined}
|
||||
>
|
||||
{props.failed && !props.complete ? (props.failure ?? props.children) : props.children}
|
||||
{/* kilocode_change - explain why the call was auto-approved or denied, inline on the header */}
|
||||
<ApprovalBadge note={props.note} color={props.noteColor} />
|
||||
</text>
|
||||
</box>
|
||||
</Show>
|
||||
@@ -2330,11 +2339,14 @@ function BlockTool(props: {
|
||||
onClick?: () => void
|
||||
part?: ToolPart
|
||||
spinner?: boolean
|
||||
hideApproval?: boolean // kilocode_change - suppress the auto-approval note (e.g. todowrite)
|
||||
}) {
|
||||
const { theme } = useTheme()
|
||||
const renderer = useRenderer()
|
||||
const [hover, setHover] = createSignal(false)
|
||||
const error = createMemo(() => (props.part?.state.status === "error" ? props.part.state.error : undefined))
|
||||
// kilocode_change - explain why the call was auto-approved or denied
|
||||
const approvalNote = createMemo(() => (props.hideApproval ? undefined : describeApproval(stateMetadata(props.part?.state))))
|
||||
return (
|
||||
<box
|
||||
ref={(el: BoxRenderable) => alwaysSeparate.add(el)}
|
||||
@@ -2361,6 +2373,8 @@ function BlockTool(props: {
|
||||
{props.title}
|
||||
{/* kilocode_change start */}
|
||||
<RoutedModelMeta.View id={props.part?.id} />
|
||||
{/* explain why the call was auto-approved or denied, inline on the title */}
|
||||
<ApprovalBadge note={approvalNote()} color={theme.textMuted} />
|
||||
{/* kilocode_change end */}
|
||||
</text>
|
||||
}
|
||||
@@ -2825,7 +2839,8 @@ function TodoWrite(props: ToolProps) {
|
||||
return (
|
||||
<Switch>
|
||||
<Match when={parseTodos(props.metadata.todos).length}>
|
||||
<BlockTool title="# Todos" part={props.part}>
|
||||
{/* kilocode_change - todo writes are orchestration, not a mutating action to explain */}
|
||||
<BlockTool title="# Todos" part={props.part} hideApproval>
|
||||
<box>
|
||||
<For each={todos()}>{(todo) => <TodoItem status={todo.status} content={todo.content} />}</For>
|
||||
</box>
|
||||
|
||||
Reference in New Issue
Block a user