Skip to content

feat(tui): explain tool auto approval - #12728

Merged
bagatao-anaconda merged 10 commits into
mainfrom
feat/explain-tool-auto-approval-tui
Aug 3, 2026
Merged

feat(tui): explain tool auto approval#12728
bagatao-anaconda merged 10 commits into
mainfrom
feat/explain-tool-auto-approval-tui

Conversation

@bagatao-anaconda

@bagatao-anaconda bagatao-anaconda commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Issue

Fixes https://docs.google.com/document/d/1eXKc_A9f_mVuc3LiniP3gbLWm7Ic_cPe58z1Tq_ZlzY/edit?pli=1&tab=t.0

Context

#12494 and #12556 added a "why was this tool call approved" line to the kilo-ui/VS Code chat, sourced from state.metadata.approval on the tool part, but two gaps remained: denied calls never got that metadata (so kilo export's JSON session log had no record of why a call was refused), and the TUI never rendered the line at all, even though the metadata was already available to it.

Implementation

  • CLI: PermissionProvenance.classifyDenial reads the deciding deny rule off a DeniedError's tagged ruleset and classifies it the same way approvals are classified. Wired into SessionTools' ctx.ask via Effect.tapErrorTag, so the denial reason is recorded on the tool call's metadata before the call fails, reusing the existing carryApproval/failToolCall preservation so it survives onto the final error state and shows up in kilo export.
  • TUI: ported the existing kilo-ui explanation as a Kilo-owned packages/tui/src/kilocode/tool-approval.tsx (plain-text describeApproval + a shared ApprovalNote row), wired with a single call at each tool-rendering site (InlineTool/InlineToolRow for Shell, Read, Grep, Glob, WebFetch, etc.; BlockTool for Write, Edit, ApplyPatch, Task). Todo writes are excluded via a hideApproval prop, matching the kilo-ui behavior from fix(ui): refine auto-approval line for subagent and todo tools #12556 that treats them as orchestration rather than an auditable action.
  • Kept shared upstream files (session/tools.ts, tui/routes/session/index.tsx) down to minimal hooks; all classification/formatting logic lives in Kilo-owned kilocode/ modules.

Screenshots / Video

tui

How to Test

Manual/local verification

  • (agent) bun run typecheck clean across the monorepo (bun turbo typecheck, 23/23 tasks).
  • (agent) bun test in packages/opencode (targeted: test/kilocode/sandbox/session-tools.test.ts, test/kilocode/permission/*) and packages/tui — all passing, including a new regression test asserting a denied tool call records { source, rule } on its metadata.
  • (agent) bun run script/check-opencode-annotations.ts --worktree passes.

Reviewer test steps

  1. bun dev -- /path/to/scratch/project
  2. Ask the agent to read a file (e.g. "read package.json") — no prompt appears, and the row shows auto-approved by the code agent.
  3. Add .kilo/kilo.json with { "permission": { "bash": { "echo *": "deny" } } }, restart, ask it to run echo hi — the call fails and the row shows denied by the project config (matched bash \echo *`)`.
  4. Run kilo export <sessionID> after step 3 and confirm messages[].parts[].state.metadata.approval on the tool part has the same { source: "project", rule: {...} } shape for the denial.

Auto-approval provenance was only recorded on the metadata of allowed
tool calls (state.metadata.approval), so denied calls had no
structured explanation of which rule/config/agent denied them. Since
'kilo export' serializes state.metadata verbatim into the JSON session
log, denials showed up with no provenance at all.

Add PermissionProvenance.classifyDenial, which reads the deciding deny
rule off a DeniedError's tagged ruleset and classifies it the same way
approvals are classified. Wire it into SessionTools' ctx.ask via
Effect.tapErrorTag so denials are recorded before the tool call fails,
reusing the existing carryApproval/failToolCall preservation so the
metadata survives onto the final error state.
Ports the auto-approval provenance explanation already shown in
kilo-ui/vscode to the TUI. Adds a Kilo-owned tool-approval.tsx with a
plain-text description helper (describeApproval) and a shared
ApprovalNote row component, then wires a single call into InlineTool/
InlineToolRow (Shell, Read, Grep, Glob, WebFetch, etc.) and BlockTool
(Write, Edit, ApplyPatch, Task), showing a muted line under completed/
failed tool calls.

Todo writes are excluded via a hideApproval prop, mirroring the
kilo-ui behavior that treats them as orchestration rather than an
auditable action.
@bagatao-anaconda bagatao-anaconda self-assigned this Jul 31, 2026
@bagatao-anaconda bagatao-anaconda changed the title feat(tui):explain tool auto approval feat(tui): explain tool auto approval Jul 31, 2026
Comment thread packages/opencode/src/kilocode/permission/provenance.ts Outdated
Comment thread packages/opencode/src/kilocode/permission/provenance.ts
Comment thread packages/tui/src/routes/session/index.tsx Outdated
Comment thread packages/tui/src/routes/session/index.tsx Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 4 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 2
Issue Details (click to expand)

WARNING

File Line Issue
packages/opencode/src/permission/index.ts 236 ruleset: rule replaces upstream's subset(...) array, so the model-facing DeniedError.message now shows one rule instead of the relevant rules, and serializes the runtime-only source tag into the prompt/persisted error
packages/opencode/src/kilocode/permission/provenance.ts 129 The synthesized fallback deny rule has no source, so configSource guesses: headless-policy denials render as denied by the <name> agent, and a request pattern equal to a config key renders as denied by the project config. Hard vetoes also report the request pattern rather than the vetoing rule's pattern

SUGGESTION

File Line Issue
packages/tui/src/routes/session/index.tsx 2321 Badge sits inside the STRIKETHROUGH <text>, so the denial explanation renders struck-through (carried over, still present)
packages/tui/src/routes/session/index.tsx 2304 Denied rows with complete={false} and failed() false (e.g. ApplyPatch) never reach the header branch, so the denial note is not rendered at all (carried over, still present)
Resolved since the previous review
  • deny-provenance.test.ts now exercises the real array-shaped ruleset for the rule-less denial path instead of the synthetic { matches: [] } object.
  • Permission.ask no longer restructures the hard-veto path (veto(...) restored), shrinking the diff against upstream on that hunk.
Files Reviewed (4 files, incremental)
  • packages/opencode/src/kilocode/permission/provenance.ts - 1 issue
  • packages/opencode/src/permission/index.ts - 1 issue
  • packages/opencode/test/kilocode/permission/deny-provenance.test.ts - 0 new issues
  • packages/opencode/test/kilocode/sandbox/session-tools.test.ts - 0 issues

Notes: incremental review of a07661b..c56aad9 (4 files). The two TUI suggestions were re-verified against current HEAD and remain unresolved; packages/tui/src/routes/session/index.tsx is unchanged in this increment, so no new inline comments were added there. The prior withDir test-wrapper suggestion still stands on an unchanged line and already has an open comment. No new subscriptions, handles, or child processes were introduced, so no memory-leak risk found.

Fix these issues in Kilo Cloud

Previous Review Summaries (2 snapshots, latest commit a07661b)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit a07661b)

Status: 4 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 3
Issue Details (click to expand)

WARNING

File Line Issue
packages/opencode/src/kilocode/permission/provenance.ts 127 The synthesized fallback deny rule flows through configSource, so rule-less denials (headless policy) are reported as denied by the <name> agent — or even denied by the project config when the request pattern equals an allow config key

SUGGESTION

File Line Issue
packages/tui/src/routes/session/index.tsx 2321 Badge now sits inside the STRIKETHROUGH <text>, so the denial explanation renders struck-through
packages/tui/src/routes/session/index.tsx 2304 Denied rows whose complete is falsy (ApplyPatch passes complete={false}) never render the header branch, so the denial note is still hidden — carried over from the previous review
packages/opencode/test/kilocode/permission/deny-provenance.test.ts 32 withDir is a redundant wrapper introducing Effect<any, any, any>; it.instance(...) covers this case
Resolved since the previous review
  • findLast(action === "deny") misattribution — Permission.ask now carries the exact matched rule as { rule, matches }.
  • Denials decided by saved/session deny rules — resolve()'s winning rule is now carried, and a rule-less denial no longer renders as auto-approved by default.
  • BlockTool rendering the note while running — the badge now lives in the non-spinner title branch, consistent with RoutedModelMeta.Badge.
Files Reviewed (7 files)
  • packages/opencode/src/kilocode/permission/provenance.ts - 1 issue
  • packages/opencode/src/permission/index.ts - 0 issues
  • packages/opencode/src/session/tools.ts - 0 issues
  • packages/opencode/test/kilocode/permission/deny-provenance.test.ts - 1 issue
  • packages/opencode/test/kilocode/sandbox/session-tools.test.ts - 0 issues
  • packages/tui/src/kilocode/tool-approval.tsx - 0 issues
  • packages/tui/src/routes/session/index.tsx - 2 issues

Notes: the DeniedError.ruleset shape change (Rule[] -> { rule, matches }) also changes the JSON embedded in the model-facing error message; the only other producer of that field (cli/cmd/debug/agent.handler.ts) has its own ask and never reaches classifyDenial, so nothing else consumes the old array shape. Shared-file hooks stay minimal, and no new subscriptions/handles were added in the TUI (memos only), so no memory-leak risk found.

Fix these issues in Kilo Cloud

Previous review (commit 6b27a26)

Status: 4 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 2
Issue Details (click to expand)

WARNING

File Line Issue
packages/opencode/src/kilocode/permission/provenance.ts 110 findLast(action === "deny") ignores the request pattern, so the note can name a deny rule that didn't decide the denial
packages/opencode/src/kilocode/permission/provenance.ts 112 Denials whose deciding rule isn't in DeniedError.ruleset (headless denials, saved/session deny rules) fall back to { source: "default" } and render as auto-approved by default

SUGGESTION

File Line Issue
packages/tui/src/routes/session/index.tsx 2332 Note gated on props.complete, which is falsy for denied apply_patch / question rows, hiding the denial explanation
packages/tui/src/routes/session/index.tsx 2388 BlockTool renders the approval note while the tool is still running, unlike InlineToolRow and its own docstring
Files Reviewed (6 files)
  • .changeset/explain-tool-auto-approval-tui.md - 0 issues
  • packages/opencode/src/kilocode/permission/provenance.ts - 2 issues
  • packages/opencode/src/session/tools.ts - 0 issues
  • packages/opencode/test/kilocode/sandbox/session-tools.test.ts - 0 issues
  • packages/tui/src/kilocode/tool-approval.tsx - 0 issues
  • packages/tui/src/routes/session/index.tsx - 2 issues

Notes: the shared-file hooks in session/tools.ts and routes/session/index.tsx stay minimal with the logic in Kilo-owned modules, the metadata write is preserved by failToolCall/carryApproval, and a changeset is present. No memory-leak risk found in the new TUI components (memos only, no new subscriptions or handles).

Fix these issues in Kilo Cloud


Reviewed by claude-opus-5 · Input: 52 · Output: 17.5K · Cached: 1.7M

Review guidance: REVIEW.md from base branch main

@marius-kilocode

Copy link
Copy Markdown
Collaborator

Why do we show this in the TUI and in vscode on the top? The bottom feels consistent, but right now in the TUI it looks like the annotation is part of the output.

@bagatao-anaconda

Copy link
Copy Markdown
Collaborator Author

@marius-kilocode Good catch on the text discrepancy; it makes sense to make things as consistent as possible.

I played around with the placement of the approval text, and came up with this. WDYT?

tui

The note was appended after the tool's own output (or, in an interim
revert, on its own line above it), which either looked like part of
the output or was visually noisier than desired. Render it inline on
the header/title line instead, matching the existing RoutedModelMeta
badge convention (' · note'), so it reads unambiguously as metadata
about the call rather than output.
…ed them

DeniedError.ruleset only carried the deny-permission subset, so
PermissionProvenance.classifyDenial had to guess the deciding rule via
findLast(action === "deny"). With two deny rules for different
patterns under the same permission (e.g. bash: { "git push *": deny,
"rm -rf *": deny }), this could attribute a denial to whichever rule
sorted last instead of the one that actually matched the request.

Permission.ask now embeds the exact rule resolve()/evaluate() matched
against the request's pattern directly on the error (ruleset: { rule,
matches }), so classifyDenial reads it instead of re-deriving it.

Some denials carry no rule at all (e.g. the headless-subagent policy
denial), where classify({ rule: undefined }) reports the same
{ source: "default" } shape as the *approval* fallback -- silently
rendering a refusal as an auto-approval in the TUI and kilo export.
classifyDenial now synthesizes an explicit deny rule for the request's
permission/pattern in that case, so rule.action always reflects the
real outcome.

Adds test/kilocode/permission/deny-provenance.test.ts covering both
regressions against the real Permission.Service, and updates the
existing session-tools.test.ts denial fixture to the new ruleset
shape.
Comment thread packages/opencode/src/kilocode/permission/provenance.ts Outdated
Comment thread packages/tui/src/routes/session/index.tsx
Comment thread packages/opencode/test/kilocode/permission/deny-provenance.test.ts
…ssion/index.ts

Applies the kilocode-merge-minimizer skill to the prior fix. The
hard-veto and headless-subagent DeniedError sites are reverted to
their exact pre-fix shape -- neither carries a specific rule anyway,
so wrapping their ruleset in a { rule, matches } object added shared
upstream diff for no benefit. Only the main deny path (which already
had the deciding rule in scope) still changes, and now passes the
bare rule instead of a wrapper object, shrinking that hunk from a
multi-line block to a single-line swap.

PermissionProvenance.classifyDenial now duck-types ruleset as a
possible bare Permission.Rule (checking action === "deny" and a
string pattern) instead of expecting a { rule } wrapper, so it still
reads the main deny path's rule directly while falling back to a
synthesized deny rule for the other paths, exactly as before.

Net shared-file diff across permission/index.ts, session/tools.ts, and
the TUI's routes/session/index.tsx for this whole feature is now 9
insertions / 12 deletions, down from ~50+ lines.
Comment thread packages/opencode/src/permission/index.ts
Comment thread packages/opencode/src/kilocode/permission/provenance.ts
@bagatao-anaconda
bagatao-anaconda merged commit cc27d9e into main Aug 3, 2026
30 checks passed
@bagatao-anaconda
bagatao-anaconda deleted the feat/explain-tool-auto-approval-tui branch August 3, 2026 10:41
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
…o-approval-tui

feat(tui): explain tool auto approval
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants