From cf62cac29c6ab9741f44189160bdffb1e2ff6926 Mon Sep 17 00:00:00 2001 From: DoodleBears Date: Sun, 15 Nov 2026 22:17:49 +0900 Subject: [PATCH 01/11] refactor(approval): declare tool effects and preserve decision paths Issue: #4205 Agent-Task: codex/local/01a1df40-f4f6-7a32-9cd2-245844776694 --- changes/4205-tool-effects.md | 11 + docs/architecture/botharness-architecture.md | 27 ++ docs/bot-cli.md | 2 + docs/bot-cli.zh.md | 2 + package.json | 2 +- packages/core/src/index.ts | 12 + .../core/src/workspaces/grant-execution.ts | 19 +- packages/core/src/workspaces/tool-approval.ts | 59 ++-- packages/core/src/workspaces/tool-effects.ts | 276 ++++++++++++++++++ packages/core/test/tool-approval.test.ts | 46 +++ packages/core/test/tool-effects.test.ts | 177 +++++++++++ scripts/check-tool-effects.mjs | 56 ++++ 12 files changed, 652 insertions(+), 37 deletions(-) create mode 100644 changes/4205-tool-effects.md create mode 100644 packages/core/src/workspaces/tool-effects.ts create mode 100644 packages/core/test/tool-effects.test.ts create mode 100644 scripts/check-tool-effects.mjs diff --git a/changes/4205-tool-effects.md b/changes/4205-tool-effects.md new file mode 100644 index 000000000..bc74e0ba5 --- /dev/null +++ b/changes/4205-tool-effects.md @@ -0,0 +1,11 @@ +--- +section: Added +--- + +## en + +- Exposed first-party tool effect declarations and missing-declaration lint for plugin developers, while preserving existing approval and Workspace Grant behavior ([#4205](https://github.com/BotHarness/DeepSeekBot/issues/4205)). + +## zh + +- 为插件开发者公开第一方工具效果声明与漏声明 lint，保持既有审批与 Workspace Grant 行为（[#4205](https://github.com/BotHarness/DeepSeekBot/issues/4205)）。 diff --git a/docs/architecture/botharness-architecture.md b/docs/architecture/botharness-architecture.md index a5615f7e4..b3ccd521d 100644 --- a/docs/architecture/botharness-architecture.md +++ b/docs/architecture/botharness-architecture.md @@ -1694,3 +1694,30 @@ The Team usage and Role model policy runtime is described in [M10 Host and CLI l [M13 Host and CLI logic](team-mode-m13-runtime.md) supersedes M2's per-Human Tob and M3's default Member grants: only the current Host Owner has a Tob, Member grants exactly `dm.humans`, and Bot creation uses `bots.createPrivate` / `bots.createPublic`. Existing member Tobs retain their records, Memory and DMs as ordinary private Bots. Member `tobHome` succeeds with `{ available: false }`; onboarding allocates nothing and omits Bot/Channel identifiers. The migration preserves customized roles, maps legacy creation grants, and keeps no-Bot usage/offboarding paths valid. Integrated M6 startup relocation follows legacy Workspace registrations on the Bot and active Grants rather than the retired member Tob identity; canonical Memory ownership and account-space file containment survive upgrade and transfer. Grant-based Git/ZIP/Marketplace imports bind ownership, upload sessions and receipts to the caller and retain M6 account containment. Legacy Workspace Grant relocation preserves each relative subpath and write flag. Owner transfer retires the demoted Owner's Tob identity atomically, leaving the former Tob private and allowing a fresh Tob on later promotion. The separate Client/mobile surfaces retain their own acceptance checks. The [M5 approval logic](team-mode-m5-runtime.md) adds caller-scoped masking across Channel projections, Activity and push, Human decision attribution and unattended routing to the Bot owner. Member-owned Sessions refuse full access, suppressed asks and escalation. Team status and invitation responses identify the current isolation as `trial`; M12 owns enforcement qualification. + +## Tool effect declarations (E1) + +The public application-defined `ToolEffects` model describes `changes` (`none`, `reversible`, +`irreversible`), `reach` (`local`, `memory`, `workspace`, `external`), `personalData`, and optional +`authority: management` / `secrets`. Device authorization stays in the decision context. The first-party inventory +lives in `packages/core/src/workspaces/tool-effects.ts`; lint checks tool factories and Browser / +Computer catalogs for missing declarations. Mixed-purpose calls resolve effects from their input +(original attachment read/edit, cloud inspection and draft inspection/drop, Browser tab listing). +Undeclared Shell and third-party calls return `undefined`, preserving ADR-0205 rather than +inventing an irreversible external declaration. Device actions declare their possible effects; +this is not a claim to infer arbitrary programs or a same-OS security boundary. + +E1 preserves the existing admission policy. `requiresHumanToolApproval` delegates its covered +name cases to a pure decision over declarations, while Channel Tool Approval uses `decideToolApproval(effects, context)` +for explainable paths, rememberability and explicit rule scopes. Full Access, saved authorization, +Reviewer and Human decisions retain their existing order. Grants, +ownership, native file roots, device target fences and Secret execution checks remain in their +existing owners. Legacy command-text recognition and unconverted tool-name entry points stay +available. External-effect admission despite Full Access belongs to E2; Reviewer/template changes +belong to E3 and are not enabled by these declarations. + +Performance: no dependency, I/O or model call is added to the decision helper. Declaration lookup +is constant-time; input-sensitive resolution and legacy text checks remain local. Owner regressions +assert zero Reviewer calls for deterministic Full Access hits and retained management prompting; +existing saved-rule/device/Grant tests protect the execution fences. This is not an end-to-end +latency measurement. Runtime and screenshot qualification are recorded separately in the PR. diff --git a/docs/bot-cli.md b/docs/bot-cli.md index 77cd98160..c0e80c863 100644 --- a/docs/bot-cli.md +++ b/docs/bot-cli.md @@ -2102,3 +2102,5 @@ deepseekbot roles update --models all [--as ] deepseekbot roles update --models provider/model,provider/model [--as ] deepseekbot models allowed [--as ] ``` + +Tool effect declarations (E1) preserve the existing tool commands and approval outcomes. Full Access, saved rules, Secrets, device targets and Workspace Grants retain their existing checks; no new route or CLI authority is introduced. diff --git a/docs/bot-cli.zh.md b/docs/bot-cli.zh.md index bd13be174..2285bbf7f 100644 --- a/docs/bot-cli.zh.md +++ b/docs/bot-cli.zh.md @@ -2043,3 +2043,5 @@ token 或原始响应。 Web 手动检查请求最多等待 25 秒，关闭自检卡片会取消请求。公网 WebSocket 在验证 Host 身份前将握手消息限制为 1 KiB，并禁用压缩。 公网阶段仅对 DNS、网络、超时及 HTTP 429/5xx 短暂失败重试：最多四次，间隔 1/3/6 秒，仍受同一个 20 秒总时限约束。证书或身份失败不重试。每次自检完成后，界面刷新一次账号电脑列表。 + +工具效果声明（E1）保持既有工具命令与审批结果。Full Access、保存规则、Secrets、设备目标和 Workspace Grant 仍使用原检查；不新增 route 或 CLI 权限。 diff --git a/package.json b/package.json index e350ca745..8c9109a37 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "scripts": { "build": "tsdown", "typecheck": "tsc --noEmit && pnpm --filter @botharness/mobile typecheck", - "lint": "oxlint && node scripts/check-schema-migrations.mjs && node scripts/check-source-policy.mjs && node scripts/check-no-shell-scripts.mjs && node scripts/check-pr-evidence.mjs && pnpm changelog:check && node scripts/check-adr-numbers.mjs && node scripts/check-cli-parity.mjs && node scripts/check-host-casts.mjs && node scripts/check-feature-map.mjs && node scripts/sync-bot-preset.mjs --check && node scripts/ci-plan.mjs --check && pnpm install:check", + "lint": "oxlint && node scripts/check-schema-migrations.mjs && node scripts/check-source-policy.mjs && node scripts/check-no-shell-scripts.mjs && node scripts/check-pr-evidence.mjs && pnpm changelog:check && node scripts/check-adr-numbers.mjs && node scripts/check-cli-parity.mjs && node scripts/check-tool-effects.mjs && node scripts/check-host-casts.mjs && node scripts/check-feature-map.mjs && node scripts/sync-bot-preset.mjs --check && node scripts/ci-plan.mjs --check && pnpm install:check", "lint:fix": "oxlint --fix", "format": "oxfmt --write .", "format:check": "oxfmt --check .", diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 555714637..1ce40e00a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -260,3 +260,15 @@ export type { ToolDetail, ToolDetailRead, ToolDetailRefusal } from './state/tool export { PERSONABOT_OUTPUT_COMMITTED, personaBotOutputCommitted } from './channels/output.js'; export type { PersonaBotOutputCommitted } from './channels/output.js'; export type { ChannelMessageOrigin } from './channels/store.js'; + +export { + FIRST_PARTY_TOOL_EFFECTS, + toolEffects, + toolApprovalRequired, + decideToolApproval, +} from './workspaces/tool-effects.js'; +export type { + ToolEffects, + ToolApprovalContext, + ToolApprovalPath, +} from './workspaces/tool-effects.js'; diff --git a/packages/core/src/workspaces/grant-execution.ts b/packages/core/src/workspaces/grant-execution.ts index a22338baf..0c3fc141b 100644 --- a/packages/core/src/workspaces/grant-execution.ts +++ b/packages/core/src/workspaces/grant-execution.ts @@ -1,5 +1,5 @@ import { OWNER_HUMAN_CONTEXT } from '../humans/context.js'; -import { requiresHumanDecision } from './approval-templates.js'; +import { toolApprovalRequired } from './tool-effects.js'; import type { Session } from '@deepseek-ai/dsh-session'; import { setSandboxMode, type SandboxPolicyService } from '@deepseek-ai/dsh-sandbox-policy'; import { setApprovalPolicy, type ApprovalService } from '@deepseek-ai/dsh-user-approval'; @@ -408,14 +408,7 @@ export function requiresHumanToolApproval( args?: unknown, templateInUse?: (template: string) => boolean, ): boolean { - if (name === 'channel_attachment_open') - return ( - typeof args !== 'object' || - args === null || - (args as Record).access !== 'read' - ); - if (name === 'secret_cli' || name === 'secret_mcp_connect' || name === 'bot_create') return true; - return isDshGovernedTool(name) && requiresHumanDecision(policyInput(args), templateInUse); + return toolApprovalRequired(name, args, isDshGovernedTool(name), templateInUse); } export function approvalTemplateInUse( @@ -424,14 +417,6 @@ export function approvalTemplateInUse( return (template) => registry.list().some((record) => record.approvalTemplate === template); } -function policyInput(args: unknown): string { - try { - return JSON.stringify(args) ?? ''; - } catch { - return ''; - } -} - export function isDshGovernedTool(name: string): boolean { return ( !BOT_TOOL_NAMES.has(name) && diff --git a/packages/core/src/workspaces/tool-approval.ts b/packages/core/src/workspaces/tool-approval.ts index 6bf157f96..0109e85ec 100644 --- a/packages/core/src/workspaces/tool-approval.ts +++ b/packages/core/src/workspaces/tool-approval.ts @@ -27,6 +27,7 @@ import { type ApprovalReviewResult, } from './approval-reviewer.js'; import { requiresHumanDecision } from './approval-templates.js'; +import { decideToolApproval, toolEffects, type ToolEffects } from './tool-effects.js'; export interface BotCreatePreview { memoryFolder: string; @@ -99,6 +100,7 @@ type TrackedCall = ToolApprovalRequestCard & { rememberable?: boolean; remembered?: boolean; execution: ToolExecution; + effects: ToolEffects | undefined; authorized?: () => boolean; device?: 'computer' | 'browser'; }; @@ -264,6 +266,7 @@ export class ChannelToolApproval { scopeKey, ...(options?.authorizationScope ? { authorizationScope: options.authorizationScope } : {}), execution, + effects: toolEffects(execution.name, execution.arguments), externalDecision, ...(options?.requireExplicitApproval ? { requireExplicitApproval: true } : {}), ...(rememberable && !options?.requireExplicitApproval ? { rememberable } : {}), @@ -303,19 +306,21 @@ export class ChannelToolApproval { this.#currentScope(tracked, owner) !== tracked.scopeKey ) return 'unavailable'; - const policyChange = this.#changesPolicy(tracked); - if ( - !policyChange && - !tracked.requireExplicitApproval && - tracked.externalDecision !== 'each-use' && - this.#fullAccess?.(tracked.sessionId) === true - ) { + const policy = this.#decisionPolicy(tracked, { + fullAccess: this.#fullAccess?.(tracked.sessionId) === true, + }); + if (policy.path === 'full-access') { tracked.authorized = () => this.#fullAccess?.(tracked.sessionId) === true; return 'allowed-once'; } const standing = tracked.externalDecision === 'each-use' ? undefined : this.#matchingRule(tracked); - if (standing !== undefined) { + if ( + standing !== undefined && + this.#decisionPolicy(tracked, { + standingRule: this.#explicitRuleRequired(tracked) ? 'explicit' : 'ordinary', + }).path === 'standing-rule' + ) { tracked.automatic = true; this.#rules?.recordAutoApproval(standing.botSlug, standing.id); return 'allowed-once'; @@ -333,17 +338,16 @@ export class ChannelToolApproval { if (channel === undefined) return 'unavailable'; if (channel.type !== 'group' && channel.botSlug !== tracked.botSlug) return 'unavailable'; if ( - !policyChange && - !tracked.requireExplicitApproval && + this.#decisionPolicy(tracked, { reviewer: true }).path === 'reviewer' && this.#overrides.delete(overrideKey(tracked)) ) { tracked.humanReviewed = true; return 'allowed-once'; } const template = - policyChange || tracked.requireExplicitApproval - ? undefined - : this.#autoApproval?.template(tracked.botSlug); + this.#decisionPolicy(tracked, { reviewer: true }).path === 'reviewer' + ? this.#autoApproval?.template(tracked.botSlug) + : undefined; if (template !== undefined && this.#breakers.get(tracked.sessionId)?.paused !== true) { const reviewed = await this.#review(tracked, template, channelId, request.signal); if (reviewed !== undefined) return reviewed; @@ -362,7 +366,7 @@ export class ChannelToolApproval { cwd: tracked.cwd, input: tracked.input, externalDecision: tracked.externalDecision ?? 'operation', - allowAlways: tracked.externalDecision !== 'each-use' || tracked.rememberable === true, + allowAlways: policy.rememberable, ...(tracked.botCreatePreview === undefined ? {} : { botCreatePreview: tracked.botCreatePreview }), @@ -929,12 +933,29 @@ export class ChannelToolApproval { : scope; } + #decisionPolicy( + tracked: TrackedCall, + options: { + fullAccess?: boolean; + standingRule?: 'none' | 'ordinary' | 'explicit'; + reviewer?: boolean; + } = {}, + ) { + return decideToolApproval(tracked.effects, { + stage: 'request', + legacyProtected: this.#changesPolicy(tracked), + explicitApproval: tracked.requireExplicitApproval === true, + eachUse: tracked.externalDecision === 'each-use', + device: tracked.device !== undefined, + fullAccess: options.fullAccess === true, + standingRule: options.standingRule ?? 'none', + reviewer: options.reviewer === true, + rememberable: tracked.rememberable === true, + }); + } + #explicitRuleRequired(tracked: TrackedCall): boolean { - return ( - this.#changesPolicy(tracked) || - tracked.requireExplicitApproval === true || - tracked.device !== undefined - ); + return this.#decisionPolicy(tracked).explicitRule; } #matchingRule(tracked: TrackedCall): ToolApprovalRule | undefined { diff --git a/packages/core/src/workspaces/tool-effects.ts b/packages/core/src/workspaces/tool-effects.ts new file mode 100644 index 000000000..768c3c532 --- /dev/null +++ b/packages/core/src/workspaces/tool-effects.ts @@ -0,0 +1,276 @@ +import { requiresHumanDecision } from './approval-templates.js'; + +export interface ToolEffects { + readonly changes: 'none' | 'reversible' | 'irreversible'; + readonly reach: 'local' | 'memory' | 'workspace' | 'external'; + readonly personalData: boolean; + readonly authority?: 'management'; + readonly secrets?: boolean; +} + +type Declaration = ToolEffects & { + readonly approval: 'native' | 'existing' | 'always' | 'attachment'; +}; +const read: Declaration = { + changes: 'none', + reach: 'local', + personalData: true, + approval: 'existing', +}; +const write: Declaration = { ...read, changes: 'reversible' }; +const send: Declaration = { ...read, changes: 'irreversible', reach: 'external' }; +const secret: Declaration = { ...send, secrets: true, approval: 'always' }; +const browserRead: Declaration = { ...read, reach: 'external', approval: 'native' }; +const browserAct: Declaration = { ...send, approval: 'native' }; +const computerRead: Declaration = { ...read, approval: 'native' }; +const computerAct: Declaration = { ...send, approval: 'native' }; + +export const FIRST_PARTY_TOOL_EFFECTS = { + archive_assignment: write, + ask_secret: { ...write, secrets: true }, + ask_user_question: write, + bot_create: { ...write, approval: 'always' }, + bot_schedule_create: write, + bot_schedule_delete: { ...write, changes: 'irreversible' }, + bot_schedule_list: read, + bot_schedule_update: write, + bot_send_message: send, + bot_subagent: send, + bot_subagent_fork: send, + bridge_attachment_save: { ...write, reach: 'workspace' }, + bridge_context: read, + bridge_directory: read, + bridge_group_policy_list: read, + bridge_group_policy_set: { ...write, authority: 'management' }, + bridge_outbox: read, + bridge_reachable_groups: read, + bridge_read: read, + bridge_sender_permissions: read, + bridge_share: send, + bridge_targets: read, + bridge_thread_policy_list: read, + bridge_thread_policy_set: { ...write, authority: 'management' }, + browser_click: browserAct, + browser_observe: browserRead, + browser_open: browserAct, + browser_press_key: browserAct, + browser_screenshot: browserRead, + browser_scroll: browserAct, + browser_tabs: browserAct, + browser_type: browserAct, + browser_upload: browserAct, + browser_wait: browserRead, + call_frame: read, + channel_action: send, + channel_attachment_import: { ...write, reach: 'workspace' }, + channel_attachment_open: { ...write, approval: 'attachment' }, + channel_attachment_save: { ...write, reach: 'workspace' }, + channel_list: read, + channel_read: read, + channel_read_image: read, + cloud_work: send, + computer_click: computerAct, + computer_get_desktop_state: computerRead, + computer_get_window_state: computerRead, + computer_hotkey: computerAct, + computer_launch_app: computerAct, + computer_list_windows: computerRead, + computer_press_key: computerAct, + computer_scroll: computerAct, + computer_type_text: computerAct, + computer_verify_state: computerRead, + create_assignment: send, + genui_diagram: write, + genui_preview: write, + group_attention_get: read, + group_attention_set: write, + group_create: write, + group_invite_bot: write, + group_invite_respond: write, + group_join_decide: write, + group_join_request: write, + group_leave: write, + group_remove_member: write, + group_rename: write, + inbox_history: read, + inbox_ignore: write, + inbox_list: read, + inbox_resolve: write, + inbox_snooze: write, + inspect_assignment: read, + list_assignment_models: read, + list_assignments: read, + list_bot_contacts: read, + list_bot_subagent_models: read, + list_subagent_models: read, + list_workspace_grants: read, + memory_continue_from_commit: { ...write, reach: 'memory' }, + memory_switch_branch: { ...write, reach: 'memory' }, + protected_cli: { ...secret, approval: 'native' }, + report_to_orchestrator: write, + request_workspace_grant: write, + secret_cli: secret, + secret_http: { ...secret, approval: 'native' }, + secret_mcp_connect: secret, + secret_mcp_disconnect: { ...write, secrets: true }, + send_assignment_request: send, + source_attention_get: read, + source_attention_reset: write, + source_attention_set: write, + stop_assignment: { ...write, changes: 'irreversible' }, + subagent: send, + subagent_fork: send, + takeover_cancel: write, + takeover_list: read, + takeover_request: write, + todos_add: { ...write, approval: 'native' }, + todos_list: { ...read, approval: 'native' }, + todos_remove: { ...write, approval: 'native' }, + todos_update: { ...write, approval: 'native' }, + unarchive_assignment: write, + wait_for_assignment: read, +} satisfies Record; + +const declarations: Readonly>> = FIRST_PARTY_TOOL_EFFECTS; +function declaration(name: string): Declaration | undefined { + return Object.hasOwn(declarations, name) ? declarations[name] : undefined; +} +function inputText(args: unknown): string { + try { + return JSON.stringify(args) ?? ''; + } catch { + return ''; + } +} + +export function toolEffects(name: string, args: unknown): ToolEffects | undefined { + const declared = declaration(name); + if (!declared) return undefined; + const { approval: _approval, ...effects } = declared; + const operation = typeof args === 'object' && args !== null ? args : undefined; + if ( + declared.approval === 'attachment' && + operation && + 'access' in operation && + operation.access === 'read' + ) + return { ...effects, changes: 'none' }; + if ( + (name === 'cloud_work' && + operation && + 'operation' in operation && + operation.operation === 'inspect') || + (name === 'channel_action' && + operation && + 'action' in operation && + operation.action === 'inspect') || + (name === 'browser_tabs' && operation && 'action' in operation && operation.action === 'list') + ) + return { ...effects, changes: 'none', reach: 'local' }; + if ( + name === 'browser_tabs' && + operation && + 'action' in operation && + operation.action === 'select' + ) + return { ...effects, changes: 'reversible', reach: 'local' }; + if ( + name === 'channel_action' && + operation && + 'action' in operation && + operation.action === 'drop' + ) + return { ...effects, changes: 'reversible', reach: 'local' }; + return effects; +} + +export type ToolApprovalContext = + | { + readonly stage: 'admission'; + readonly admission: Declaration['approval']; + readonly legacyProtected: boolean; + } + | { + readonly stage: 'request'; + readonly fullAccess: boolean; + readonly eachUse: boolean; + readonly explicitApproval: boolean; + readonly legacyProtected: boolean; + readonly device: boolean; + readonly standingRule: 'none' | 'ordinary' | 'explicit'; + readonly reviewer: boolean; + readonly rememberable: boolean; + }; + +export interface ToolApprovalPath { + readonly path: 'native' | 'request' | 'full-access' | 'standing-rule' | 'reviewer' | 'human'; + readonly rememberable: boolean; + readonly explicitRule: boolean; + readonly reason: + | 'undeclared-native' + | 'existing-native' + | 'effect-approval' + | 'protected-operation' + | 'full-access-existing' + | 'saved-authorization' + | 'template-review' + | 'human-required'; +} + +export function decideToolApproval( + effects: ToolEffects | undefined, + context: ToolApprovalContext, +): ToolApprovalPath { + if (context.stage === 'admission') { + const required = + context.admission === 'always' || + (context.admission === 'attachment' && effects?.changes !== 'none') || + context.legacyProtected; + return { + path: required ? 'request' : 'native', + rememberable: true, + explicitRule: context.legacyProtected, + reason: context.legacyProtected + ? 'protected-operation' + : required + ? 'effect-approval' + : effects + ? 'existing-native' + : 'undeclared-native', + }; + } + const explicit = + context.legacyProtected || context.explicitApproval || effects?.authority === 'management'; + const explicitRule = explicit || context.device; + const rememberable = !context.eachUse || context.rememberable; + if (!explicit && !context.eachUse && context.fullAccess) + return { path: 'full-access', rememberable, explicitRule, reason: 'full-access-existing' }; + if ( + !context.eachUse && + (context.standingRule === 'explicit' || (!explicitRule && context.standingRule === 'ordinary')) + ) + return { path: 'standing-rule', rememberable, explicitRule, reason: 'saved-authorization' }; + if (!explicit && context.reviewer) + return { path: 'reviewer', rememberable, explicitRule, reason: 'template-review' }; + return { + path: 'human', + rememberable, + explicitRule, + reason: explicit ? 'protected-operation' : 'human-required', + }; +} + +export function toolApprovalRequired( + name: string, + args: unknown, + governed: boolean, + templateInUse?: (name: string) => boolean, +): boolean { + return ( + decideToolApproval(toolEffects(name, args), { + stage: 'admission', + admission: declaration(name)?.approval ?? 'native', + legacyProtected: governed && requiresHumanDecision(inputText(args), templateInUse), + }).path === 'request' + ); +} diff --git a/packages/core/test/tool-approval.test.ts b/packages/core/test/tool-approval.test.ts index c690d98f0..8ef3fcc7b 100644 --- a/packages/core/test/tool-approval.test.ts +++ b/packages/core/test/tool-approval.test.ts @@ -12,6 +12,7 @@ import { attachOperationalModule, mountOperationalDatabase } from '../src/databa import { BOT_HARNESS_SCHEMA_PLAN } from '../src/database/schema-plan.js'; import { createToolApprovalRuleStore } from '../src/workspaces/tool-approval-rules.js'; import { createTempRoot } from './helpers.js'; +import { toolEffects } from '../src/workspaces/tool-effects.js'; import { ChannelToolApproval } from '../src/workspaces/tool-approval.js'; const botSlug = 'ada'; @@ -57,6 +58,51 @@ function fixture( } describe('Channel tool approval', () => { + it.each([ + ['channel_read', {}, 'none', false], + ['channel_attachment_open', { access: 'read' }, 'none', false], + ['channel_attachment_open', { access: 'write' }, 'reversible', false], + ['bot_send_message', { body: 'hello' }, 'irreversible', false], + ['secret_cli', { program: 'gh' }, 'irreversible', true], + ['protected_cli', { program: 'gh' }, 'irreversible', true], + ['bash', { command: 'deepseekbot grant-write-set ada --enabled' }, undefined, false], + ] as const)( + 'keeps deterministic Full Access decisions for %s effects', + async (name, args, mutation, secrets) => { + const state = fixture(); + const review = vi.fn(); + const broker = new ChannelToolApproval( + state.channels, + state.ownership, + undefined, + undefined, + undefined, + undefined, + undefined, + { template: () => ({ name: 'test', text: 'ask' }), review, log: vi.fn() }, + () => true, + ); + const effects = toolEffects(name, args); + expect(effects?.changes).toBe(mutation); + expect(effects?.secrets ?? false).toBe(secrets); + broker.track({ ...state.execution(), name, arguments: args }); + try { + const answer = broker.ask({ agent: state.agent, toolName: name, callId: 'call-1' }); + if (name === 'bash' || effects?.authority === 'management') { + await vi.waitFor(() => expect(state.messages).toHaveLength(1)); + await broker.decide(OWNER_HUMAN_CONTEXT, botSlug, state.messages[0]!.id, 'rejected'); + expect(await answer).toBe('rejected'); + } else { + expect(await answer).toBe('allowed-once'); + expect(state.messages).toHaveLength(0); + } + expect(review).not.toHaveBeenCalled(); + } finally { + broker.close(); + } + }, + ); + it.each([ ['browser_open', 'browser'], ['computer_click', 'computer'], diff --git a/packages/core/test/tool-effects.test.ts b/packages/core/test/tool-effects.test.ts new file mode 100644 index 000000000..78d688d4f --- /dev/null +++ b/packages/core/test/tool-effects.test.ts @@ -0,0 +1,177 @@ +import { describe, expect, it } from 'vitest'; +import { + decideToolApproval, + toolEffects, + type ToolApprovalContext, + type ToolEffects, +} from '../src/workspaces/tool-effects.js'; + +describe('public tool effects', () => { + it.each([ + ['channel_read', {}, { changes: 'none', reach: 'local', personalData: true }], + [ + 'channel_attachment_open', + { access: 'read' }, + { changes: 'none', reach: 'local', personalData: true }, + ], + [ + 'channel_attachment_open', + { access: 'write' }, + { changes: 'reversible', reach: 'local', personalData: true }, + ], + ['memory_switch_branch', {}, { changes: 'reversible', reach: 'memory', personalData: true }], + [ + 'channel_attachment_save', + {}, + { changes: 'reversible', reach: 'workspace', personalData: true }, + ], + ['bot_send_message', {}, { changes: 'irreversible', reach: 'external', personalData: true }], + [ + 'secret_cli', + {}, + { changes: 'irreversible', reach: 'external', personalData: true, secrets: true }, + ], + [ + 'cloud_work', + { operation: 'inspect' }, + { changes: 'none', reach: 'local', personalData: true }, + ], + [ + 'channel_action', + { action: 'inspect' }, + { changes: 'none', reach: 'local', personalData: true }, + ], + [ + 'channel_action', + { action: 'drop' }, + { changes: 'reversible', reach: 'local', personalData: true }, + ], + [ + 'browser_tabs', + { action: 'select' }, + { changes: 'reversible', reach: 'local', personalData: true }, + ], + ['browser_tabs', { action: 'list' }, { changes: 'none', reach: 'local', personalData: true }], + ] as const)('resolves author effects for %s with %j', (name, args, expected) => { + expect(toolEffects(name, args)).toEqual(expected); + }); + it.each(['bash', 'pwsh', 'third_party_tool', 'toString'])( + 'retains undeclared provenance for %s', + (name) => { + expect(toolEffects(name, { command: 'send everything' })).toBeUndefined(); + expect( + decideToolApproval(undefined, { + stage: 'admission', + admission: 'native', + legacyProtected: false, + }), + ).toMatchObject({ path: 'native', reason: 'undeclared-native' }); + }, + ); +}); + +const request = { + stage: 'request', + fullAccess: false, + eachUse: false, + explicitApproval: false, + legacyProtected: false, + device: false, + standingRule: 'none', + reviewer: false, + rememberable: false, +} satisfies ToolApprovalContext; +const local: ToolEffects = { changes: 'none', reach: 'local', personalData: false }; +const external: ToolEffects = { changes: 'irreversible', reach: 'external', personalData: true }; + +describe('E1 pure approval decision', () => { + it.each([ + [local, { fullAccess: true }, 'full-access', true, false, 'full-access-existing'], + [external, { fullAccess: true }, 'full-access', true, false, 'full-access-existing'], + [ + { ...external, secrets: true }, + { fullAccess: true }, + 'full-access', + true, + false, + 'full-access-existing', + ], + [ + { ...local, authority: 'management' }, + { fullAccess: true, reviewer: true }, + 'human', + true, + true, + 'protected-operation', + ], + [ + { ...local, authority: 'management' }, + { standingRule: 'explicit', reviewer: true }, + 'standing-rule', + true, + true, + 'saved-authorization', + ], + [ + { ...local, authority: 'management' }, + { standingRule: 'ordinary', fullAccess: true, reviewer: true }, + 'human', + true, + true, + 'protected-operation', + ], + [ + undefined, + { legacyProtected: true, fullAccess: true, reviewer: true }, + 'human', + true, + true, + 'protected-operation', + ], + [ + external, + { standingRule: 'ordinary', reviewer: true }, + 'standing-rule', + true, + false, + 'saved-authorization', + ], + [external, { reviewer: true }, 'reviewer', true, false, 'template-review'], + [external, {}, 'human', true, false, 'human-required'], + [ + external, + { eachUse: true, fullAccess: true, standingRule: 'ordinary' }, + 'human', + false, + false, + 'human-required', + ], + [external, { eachUse: true, rememberable: true }, 'human', true, false, 'human-required'], + [ + local, + { device: true, standingRule: 'explicit' }, + 'standing-rule', + true, + true, + 'saved-authorization', + ], + [ + local, + { explicitApproval: true, fullAccess: true, reviewer: true }, + 'human', + true, + true, + 'protected-operation', + ], + ] as const)( + 'routes effects %j and context %j', + (effects, context, path, rememberable, explicitRule, reason) => { + expect(decideToolApproval(effects, { ...request, ...context })).toEqual({ + path, + rememberable, + explicitRule, + reason, + }); + }, + ); +}); diff --git a/scripts/check-tool-effects.mjs b/scripts/check-tool-effects.mjs new file mode 100644 index 000000000..1c9f494bd --- /dev/null +++ b/scripts/check-tool-effects.mjs @@ -0,0 +1,56 @@ +import { readdirSync, readFileSync } from 'node:fs'; +import { join, relative } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const root = fileURLToPath(new URL('..', import.meta.url)); +const source = readFileSync(join(root, 'packages/core/src/workspaces/tool-effects.ts'), 'utf8'); +const inventory = + source.split('export const FIRST_PARTY_TOOL_EFFECTS = {')[1]?.split('} satisfies')[0] ?? ''; +const declared = new Set([...inventory.matchAll(/^ ([a-z_]+):/gm)].map((match) => match[1])); +const required = new Set(); +const unresolved = []; +function scan(directory) { + for (const entry of readdirSync(directory, { withFileTypes: true })) { + const path = join(directory, entry.name); + if (entry.isDirectory()) scan(path); + else if (path.endsWith('.ts')) { + const text = readFileSync(path, 'utf8'); + const calls = [...text.matchAll(/defineTool\s*\(/g)]; + const literal = [...text.matchAll(/defineTool\s*\(\s*\{\s*name:\s*['"]([^'"]+)['"]/g)]; + for (const match of literal) required.add(match[1]); + let dynamic = 0; + if ( + relative(root, path).replaceAll('\\', '/') === + 'packages/core/src/runtime/subagent-model-tools.ts' + ) { + const table = text.match(/const BOT_SUBAGENT_TOOL_NAMES = \{([\s\S]*?)\}/)?.[1] ?? ''; + for (const match of table.matchAll(/: '([^']+)'/g)) required.add(match[1]); + dynamic = [ + ...text.matchAll(/defineTool\(\{\s*(?:name,|name: BOT_SUBAGENT_TOOL_NAMES\.list,)/g), + ].length; + } + if (calls.length !== literal.length + dynamic) unresolved.push(relative(root, path)); + } + } +} +for (const entry of readdirSync(join(root, 'packages'), { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + try { + scan(join(root, 'packages', entry.name, 'src')); + } catch (error) { + if (error.code !== 'ENOENT') throw error; + } +} +for (const kind of ['browser', 'computer']) { + const text = readFileSync(join(root, 'packages', kind, 'src/tool/catalog.ts'), 'utf8'); + for (const match of text.matchAll(/raw: '([^']+)'/g)) required.add(kind + '_' + match[1]); +} +const missing = [...required].filter((name) => !declared.has(name)); +if (missing.length || unresolved.length) { + if (unresolved.length) + process.stderr.write( + 'tool-effects: unresolved dynamic tool factories: ' + unresolved.join(', ') + '\n', + ); + process.stderr.write('tool-effects: missing declarations: ' + missing.join(', ') + '\n'); + process.exitCode = 1; +} else process.stdout.write('Tool effects declared for ' + required.size + ' first-party tools.\n'); From 94aa86ed088c31a7dd512c6b1dde7a314d77a774 Mon Sep 17 00:00:00 2001 From: DoodleBears Date: Sun, 15 Nov 2026 22:22:22 +0900 Subject: [PATCH 02/11] fix(approval): resolve mixed-purpose tool effect declarations Issue: #4205 Agent-Task: codex/local/01a1df40-f4f6-7a32-9cd2-245844776694 --- packages/core/src/workspaces/tool-effects.ts | 4 ++++ packages/core/test/tool-effects.test.ts | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/packages/core/src/workspaces/tool-effects.ts b/packages/core/src/workspaces/tool-effects.ts index 768c3c532..1ccb44b18 100644 --- a/packages/core/src/workspaces/tool-effects.ts +++ b/packages/core/src/workspaces/tool-effects.ts @@ -148,6 +148,10 @@ export function toolEffects(name: string, args: unknown): ToolEffects | undefine if (!declared) return undefined; const { approval: _approval, ...effects } = declared; const operation = typeof args === 'object' && args !== null ? args : undefined; + if (name === 'secret_http' && operation && 'method' in operation && operation.method === 'GET') + return { ...effects, changes: 'none' }; + if (name === 'browser_tabs' && operation && 'action' in operation && operation.action === 'close') + return { ...effects, reach: 'local' }; if ( declared.approval === 'attachment' && operation && diff --git a/packages/core/test/tool-effects.test.ts b/packages/core/test/tool-effects.test.ts index 78d688d4f..3c67b7bf3 100644 --- a/packages/core/test/tool-effects.test.ts +++ b/packages/core/test/tool-effects.test.ts @@ -7,6 +7,25 @@ import { } from '../src/workspaces/tool-effects.js'; describe('public tool effects', () => { + it.each([ + [ + 'secret_http', + { method: 'GET' }, + { changes: 'none', reach: 'external', personalData: true, secrets: true }, + ], + [ + 'secret_http', + { method: 'POST' }, + { changes: 'irreversible', reach: 'external', personalData: true, secrets: true }, + ], + [ + 'browser_tabs', + { action: 'close' }, + { changes: 'irreversible', reach: 'local', personalData: true }, + ], + ] as const)('resolves mixed-purpose effects for %s %j', (name, input, expected) => { + expect(toolEffects(name, input)).toEqual(expected); + }); it.each([ ['channel_read', {}, { changes: 'none', reach: 'local', personalData: true }], [ From d62bbe9a08b950610818f9557cc94f1f76b466dd Mon Sep 17 00:00:00 2001 From: DoodleBears Date: Sun, 15 Nov 2026 23:00:14 +0900 Subject: [PATCH 03/11] fix(approval): align tool effects with execution parameters Resolve remote history scopes, restored Assignment request delivery, and case-insensitive HTTP methods at the public effect boundary. Issue: #4205 Agent-Task: codex/local/01a1df05-43d2-7380-84a0-99084cd0f49c --- packages/core/src/workspaces/tool-effects.ts | 23 ++++++++++++- packages/core/test/tool-effects.test.ts | 34 ++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/packages/core/src/workspaces/tool-effects.ts b/packages/core/src/workspaces/tool-effects.ts index 1ccb44b18..b0151ac1c 100644 --- a/packages/core/src/workspaces/tool-effects.ts +++ b/packages/core/src/workspaces/tool-effects.ts @@ -148,7 +148,28 @@ export function toolEffects(name: string, args: unknown): ToolEffects | undefine if (!declared) return undefined; const { approval: _approval, ...effects } = declared; const operation = typeof args === 'object' && args !== null ? args : undefined; - if (name === 'secret_http' && operation && 'method' in operation && operation.method === 'GET') + if ( + name === 'bridge_context' && + operation && + 'scope' in operation && + (operation.scope === 'group' || operation.scope === 'nearby' || operation.scope === 'thread') + ) + return { ...effects, reach: 'external' }; + if ( + name === 'unarchive_assignment' && + operation && + 'message' in operation && + typeof operation.message === 'string' && + operation.message.trim() !== '' + ) + return { ...effects, changes: 'irreversible', reach: 'external' }; + if ( + name === 'secret_http' && + operation && + 'method' in operation && + typeof operation.method === 'string' && + operation.method.toUpperCase() === 'GET' + ) return { ...effects, changes: 'none' }; if (name === 'browser_tabs' && operation && 'action' in operation && operation.action === 'close') return { ...effects, reach: 'local' }; diff --git a/packages/core/test/tool-effects.test.ts b/packages/core/test/tool-effects.test.ts index 3c67b7bf3..89d4c8781 100644 --- a/packages/core/test/tool-effects.test.ts +++ b/packages/core/test/tool-effects.test.ts @@ -7,6 +7,40 @@ import { } from '../src/workspaces/tool-effects.js'; describe('public tool effects', () => { + it.each(['GET', 'get', 'GeT'])('uses HTTP execution normalization for %s', (method) => { + expect(toolEffects('secret_http', { method })).toEqual({ + changes: 'none', + reach: 'external', + personalData: true, + secrets: true, + }); + }); + it('includes request delivery when restoring an Assignment with a message', () => { + expect(toolEffects('unarchive_assignment', { session_id: 'assignment-1' })).toEqual({ + changes: 'reversible', + reach: 'local', + personalData: true, + }); + expect( + toolEffects('unarchive_assignment', { + session_id: 'assignment-1', + message: 'Continue the approved task.', + }), + ).toEqual({ changes: 'irreversible', reach: 'external', personalData: true }); + }); + it.each([ + ['group', 'external'], + ['nearby', 'external'], + ['thread', 'external'], + ['retained', 'local'], + ['retained-nearby', 'local'], + ] as const)('declares %s context with %s reach', (scope, reach) => { + expect(toolEffects('bridge_context', { scope })).toEqual({ + changes: 'none', + reach, + personalData: true, + }); + }); it.each([ [ 'secret_http', From 4940c44dde9e35fc0aeb17ed4d96512c611891dc Mon Sep 17 00:00:00 2001 From: DoodleBears Date: Mon, 16 Nov 2026 01:02:59 +0900 Subject: [PATCH 04/11] fix(approval): preserve mobile global inheritance selection Expose the inherited global template in the existing mobile selection list and retain globalDefault at the Host client boundary. Keep legacy Hosts without that field on their supported choices; invalid inherited answers remain failed reads. Add a Bot save/readback regression proving successful writes are not replayed and the UI remains unknown rather than falsely Off after failed refresh. Issue: #4211 Agent-Task: codex/local/01a1df05-43d2-7380-84a0-99084cd0f49c --- .../features/approval-templates.md | 5 + apps/mobile/DESIGN.md | 2 +- apps/mobile/src/host/bot-settings.ts | 10 ++ apps/mobile/src/ui/chat/workspace-sheet.tsx | 24 +++ apps/mobile/src/ui/strings.ts | 6 + .../test/approval-template-selection.test.tsx | 150 ++++++++++++++++++ apps/mobile/test/bot-settings.test.ts | 33 ++++ changes/4211-effective-approval-template.md | 4 +- .../test/approval-template-entry.test.ts | 64 +++++++- 9 files changed, 294 insertions(+), 4 deletions(-) create mode 100644 apps/mobile/test/approval-template-selection.test.tsx diff --git a/.agents/skills/verify-deepseekbot/features/approval-templates.md b/.agents/skills/verify-deepseekbot/features/approval-templates.md index 8950d837c..920a47731 100644 --- a/.agents/skills/verify-deepseekbot/features/approval-templates.md +++ b/.agents/skills/verify-deepseekbot/features/approval-templates.md @@ -7,6 +7,7 @@ A Bot inherits the optional global default, selects one complete shared template - `library`: Bot settings → Auto approval → Global default template (DSH Select), existing template library and Reviewer model. No global default means inherited review is off. Read failures remain errors. - `cli`: `approval-template list --bot `, `approval-template-global-set `, `approval-template-set `; library create/edit commands retain existing Markdown contents. - `select`: Channel settings → Auto approval, or composer Workspace Grants panel → Auto approval, uses the same DSH Select. Inheritance shows its effective global source. Specific selection replaces global; Off retains deterministic rules. +- `mobile`: Bot profile → Settings → Permissions → Workspace grants → Auto approval template, inside the existing iOS sheet / Android `ModalBottomSheet`. Inherit global shows the resolved global template (or Off); selecting Off then Inherit global writes null then @global through the same Host owner. Legacy Hosts without a global-selection field retain their supported choices. - `reviewer`: a covered call leaves an auto-approved record with template identity/version; an uncovered or failed review posts a Human card with the reason and captured version when available. No new model rejection or rejection pause occurs. - `guard`: effective template/global/Bot policy changes bypass Full Access and model review, retaining Human or explicit management-scope authorization and execution fences (ADR-0249). @@ -25,6 +26,10 @@ Use an isolated Host and its own authenticated CLI. Confirm health and Client co 5. Edit/switch during an injected Reviewer wait at the Channel Tool Approval seam: this Review retains the old identity/version, the next uses the new one. Missing/unreadable template or model timeout/malformed response posts Human with a reason, even if diagnostics fail. Check canonical Channel records and developer logs. 6. Exercise a policy-management approval under Full Access; Reviewer calls remain zero and only Human or an explicit management rule can authorize effects. Revoke the rule before execution and verify refusal. +## Native mobile qualification + +Use the shared device QA lane and T3 device tools. The same synthetic Bot and isolated QA Host must be used for before/after, with the current PR/source/build identities recorded. In Bot profile → Settings → Permissions → Workspace grants, verify the inherited row is checked and names the global template or Off. Choose Off, return to Inherit global, then choose one specific template; read back each selection using the authenticated CLI. A global switch updates the inherited row after refresh while a specific choice remains specific. Read failures stay visible failures. Capture default/intermediate/maximum system text in light/dark, keep the last choice reachable, and distinguish Android, iOS and Human acceptance. Web screenshots do not qualify this journey. + ## Gotchas - `/tmp` may already be writable; an ordinary file write there need not reach an approval gate. diff --git a/apps/mobile/DESIGN.md b/apps/mobile/DESIGN.md index 8323d7f12..1fc9db303 100644 --- a/apps/mobile/DESIGN.md +++ b/apps/mobile/DESIGN.md @@ -495,7 +495,7 @@ The two-tab shell follows [ADR-0211](../../docs/adr/0211-the-mobile-app-has-two- - **Workspace** opens a sheet titled "Workspace · {Bot}" with three sections, the same actions as the Web composer's workspace panel: 1. **Folders**: each granted folder as a row (name, then "Can edit" or "Read only" and its path) with a × that removes access, then **Add folder**. Add folder turns the sheet into a folder browser of the computer: folders registered on the computer, the path as tappable crumbs, and the subfolders; **Authorize this folder** resolves the current folder and asks for a scoped confirmation ("Confirm authorization for {Bot}" with the resolved path) before granting it. 2. **Access**: a **Full access** switch. Turning it on first shows the danger confirmation in place, in an inset box with a red "I understand, enable" button and Cancel; once on, a warning callout and a **Scope of full access** group (Chat and new Assignments, or New Assignments only) follow. - 3. **Auto approval template**: Off, Cautious and Get the work done (plus any other template on the Host), each with its one-line hint, the current one checked. + 3. **Auto approval template**: Inherit global (with the current global template or Off), Off, Cautious and Get the work done (plus any other template on the Host), each with its one-line hint, the current choice checked. Inheritance is offered only when the Host returns its global selection; legacy Hosts keep Off and named choices. Unreadable inherited selection stays a failed read, never an implicit Off. - The mic dictates with the phone's speech recognizer, and the words fill the field so the Human edits them before sending. On iOS it is a native SwiftUI button: tap to start, then tap **■** to stop; the **×** that replaces **+** cancels and clears the text. Its SF Symbol is centered in the full button frame, with native press and accessibility activation. There is no iOS hold, drag or lock mode. Android retains hold-to-talk/release-to-stop, slide-left cancel, slide-up lock and tap-to-lock. Dictation stays in the same row, and its status shows as the field's placeholder. A refused microphone or speech permission shows a notice and leaves the mic in place. This is speech-to-text, not an audio-message sender; permissions, native recognition events and cleanup stay owned by the speech adapter. - A group Channel opens the same chat screen, with these differences: - The header is the same native header. Its title holds the group's photo (or a round `#` mark), the Channel name and its member count; it opens the **Group profile**. The native three-dot Profile button is the trailing item. diff --git a/apps/mobile/src/host/bot-settings.ts b/apps/mobile/src/host/bot-settings.ts index 131e136c0..6aabfd221 100644 --- a/apps/mobile/src/host/bot-settings.ts +++ b/apps/mobile/src/host/bot-settings.ts @@ -101,9 +101,12 @@ export interface ApprovalTemplate { description: string; } +export const INHERIT_APPROVAL_TEMPLATE = '@global'; + export interface ApprovalTemplates { templates: ApprovalTemplate[]; selected: string | null; + globalDefault?: string | null; } export class HostAnswerInvalid extends Error {} @@ -544,6 +547,12 @@ export async function loadApprovalTemplates( ): Promise { const value = record(await client.call('approvalTemplates', { slug })); const selected = value?.['selected']; + const globalDefault = value?.['globalDefault']; + if ( + (globalDefault !== undefined && globalDefault !== null && !text(globalDefault)) || + (selected === INHERIT_APPROVAL_TEMPLATE && globalDefault === undefined) + ) + throw new HostAnswerInvalid('approvalTemplates returned no readable global selection.'); if (!Array.isArray(value?.['templates']) || (selected !== null && !text(selected))) throw new HostAnswerInvalid('approvalTemplates returned no templates.'); return { @@ -554,6 +563,7 @@ export async function loadApprovalTemplates( : undefined; }), selected, + ...(globalDefault === undefined ? {} : { globalDefault }), }; } diff --git a/apps/mobile/src/ui/chat/workspace-sheet.tsx b/apps/mobile/src/ui/chat/workspace-sheet.tsx index 741004f70..11aa94973 100644 --- a/apps/mobile/src/ui/chat/workspace-sheet.tsx +++ b/apps/mobile/src/ui/chat/workspace-sheet.tsx @@ -4,6 +4,7 @@ import { Pressable, ScrollView, SectionList, View } from 'react-native'; import { errorText, folderBrowsingUnavailable, + INHERIT_APPROVAL_TEMPLATE, type AccessScope, type ApprovalTemplate, type ApprovalTemplates, @@ -100,6 +101,14 @@ export function templateTitle(template: Pick return template.title; } +function inheritedTemplateLabel(selection: ApprovalTemplates): string { + const name = selection.globalDefault; + if (name === undefined) return t.contextUnavailable; + if (name === null) return t.autoApprovalOff; + const template = selection.templates.find((template) => template.name === name); + return template === undefined ? t.autoApprovalUnavailable(name) : templateTitle(template); +} + export function templateDescription(template: ApprovalTemplate): string { if (template.name === 'cautious') return t.autoApprovalCautiousHint; if (template.name === 'get-work-done') return t.autoApprovalGetWorkDoneHint; @@ -610,6 +619,18 @@ export function WorkspaceSheet({ : templates.status === 'failed' ? [{ kind: 'failed', key: 'templates-failed', text: templates.error }] : [ + ...(templates.value.globalDefault === undefined + ? [] + : [ + { + kind: 'template' as const, + key: `template:${INHERIT_APPROVAL_TEMPLATE}`, + name: INHERIT_APPROVAL_TEMPLATE, + title: t.autoApprovalInherit, + detail: t.autoApprovalInherited(inheritedTemplateLabel(templates.value)), + selected: templates.value.selected === INHERIT_APPROVAL_TEMPLATE, + }, + ]), { kind: 'template', key: `template:${OFF}`, @@ -811,6 +832,9 @@ export function WorkspaceSheet({ case 'template': return ( `From global: ${template}`, + autoApprovalUnavailable: (name: string) => `${name} (unavailable)`, autoApprovalOffHint: 'You approve each card', autoApprovalFooter: "A reviewer answers this Bot's approvals by the template. Anything it is unsure about still comes to you.", @@ -4839,6 +4842,9 @@ const zh: typeof en = { '完全访问已开启（仅新事项）：之后新建的事项可访问任何文件且不再逐次提问；聊天会话仍然受限。', autoApproval: '自动审批模板', autoApprovalOff: '关闭', + autoApprovalInherit: '继承全局', + autoApprovalInherited: (template: string) => `来自全局：${template}`, + autoApprovalUnavailable: (name: string) => `${name}（不可用）`, autoApprovalOffHint: '每张卡片由你批准', autoApprovalFooter: '审批员按模板替你处理此 Bot 的审批，拿不准的仍交给你。', autoApprovalCautious: '谨慎', diff --git a/apps/mobile/test/approval-template-selection.test.tsx b/apps/mobile/test/approval-template-selection.test.tsx new file mode 100644 index 000000000..49ec9d0af --- /dev/null +++ b/apps/mobile/test/approval-template-selection.test.tsx @@ -0,0 +1,150 @@ +// @vitest-environment jsdom +import { act, createElement, useState, type PropsWithChildren } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, expect, it, vi } from 'vitest'; +import { botSettings, type ApprovalTemplates } from '../src/host/bot-settings'; +import { createHostClient } from '../src/host/client'; +import { WorkspaceSheet } from '../src/ui/chat/workspace-sheet'; +import { FakeHost } from './fake-host'; +import { t } from '../src/ui/strings'; + +vi.mock('react-native', async () => await import('./react-native-shim')); +vi.mock('../src/ui/primitives/settings-row', () => ({ + SettingsGroupItem: ({ children }: PropsWithChildren) => children, + SettingsRow: ({ + title, + detail, + selected, + onPress, + testID, + }: { + title: string; + detail?: string; + selected?: boolean; + onPress?(): void; + testID?: string; + }) => + createElement( + 'button', + { onClick: onPress, 'aria-pressed': selected === true, 'data-testid': testID }, + title, + detail ? ', ' + detail : '', + ), +})); +vi.mock('../src/ui/chat/sheet', () => { + const pass = ({ children }: PropsWithChildren) => createElement('section', null, children); + return { + Sheet: pass, + SheetActions: pass, + SheetCallout: pass, + SheetFailed: pass, + SheetFooter: pass, + SheetHeading: pass, + SheetLoading: pass, + SheetNote: pass, + SHEET_GROUP: 'canvas', + }; +}); +vi.mock('../src/ui/primitives/app-dialog', () => ({ confirmDestructive: vi.fn() })); +vi.mock('../src/ui/shell/icons', () => ({ + CloseIcon: () => null, + FolderIcon: () => null, + PlusIcon: () => null, +})); + +let root: Root; +let host: HTMLDivElement; +beforeEach(() => { + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true); + host = document.createElement('div'); + document.body.append(host); + root = createRoot(host); +}); +afterEach(async () => { + await act(async () => root.unmount()); + host.remove(); + vi.unstubAllGlobals(); +}); +function named(text: string): HTMLButtonElement { + const row = [...host.querySelectorAll('button')].find((button) => + button.textContent?.startsWith(text), + ); + if (!row) throw new Error('Missing choice ' + text); + return row; +} +async function mount(initial: string | null, globalDefault: string | null | undefined) { + const fake = new FakeHost(); + let selected = initial; + fake.methods.set('approvalTemplates', () => ({ + templates: [{ name: 'cautious', title: 'Cautious', description: 'Read carefully.' }], + selected, + ...(globalDefault === undefined ? {} : { globalDefault }), + })); + fake.methods.set('botApprovalTemplateSet', (args) => { + const value = args['template']; + if (value !== null && typeof value !== 'string') throw new Error('Invalid template'); + selected = value; + return { bot: { slug: 'ada' } }; + }); + const settings = botSettings( + createHostClient({ + routes: ['https://studio.tailnet-example.ts.net'], + credential: fake.credential, + carrier: fake.carrier, + }), + 'ada', + ); + const initialTemplates = await settings.loadTemplates(); + function Harness() { + const [templates, setTemplates] = useState(initialTemplates); + return ( + {}} + onGrantsChanged={() => {}} + onAccess={() => {}} + onTemplates={setTemplates} + onClose={() => {}} + /> + ); + } + await act(async () => root.render(createElement(Harness))); + return fake; +} +it.each([null, 'cautious'])( + 'shows the inherited global default %s and can return to inheritance after Off', + async (globalDefault) => { + const fake = await mount('@global', globalDefault); + const inherited = named(t.autoApprovalInherit); + expect(inherited.textContent).toContain( + globalDefault === null ? t.autoApprovalOff : t.autoApprovalCautious, + ); + expect(inherited.getAttribute('aria-pressed')).toBe('true'); + await act(async () => named(t.autoApprovalOff).click()); + expect(named(t.autoApprovalOff).getAttribute('aria-pressed')).toBe('true'); + await act(async () => named(t.autoApprovalInherit).click()); + expect(named(t.autoApprovalInherit).getAttribute('aria-pressed')).toBe('true'); + expect( + fake.calls + .filter((call) => call.method === 'botApprovalTemplateSet') + .map((call) => call.args), + ).toEqual([ + { slug: 'ada', template: null }, + { slug: 'ada', template: '@global' }, + ]); + }, +); +it('does not offer unsupported inheritance on legacy Hosts that omit global selection', async () => { + await mount(null, undefined); + expect( + [...host.querySelectorAll('button')].some((button) => + button.textContent?.startsWith(t.autoApprovalInherit), + ), + ).toBe(false); + expect(named(t.autoApprovalOff).getAttribute('aria-pressed')).toBe('true'); +}); diff --git a/apps/mobile/test/bot-settings.test.ts b/apps/mobile/test/bot-settings.test.ts index 0a921fe9d..2ac6925aa 100644 --- a/apps/mobile/test/bot-settings.test.ts +++ b/apps/mobile/test/bot-settings.test.ts @@ -419,6 +419,39 @@ describe('Bot workspace from the composer', () => { ); }); + it.each([null, 'cautious', 'missing-template'])( + 'preserves global default %s for inherited approval selection', + async (globalDefault) => { + const fake = new FakeHost(); + fake.methods.set('approvalTemplates', () => ({ + templates: [], + selected: '@global', + globalDefault, + })); + fake.methods.set('botApprovalTemplateSet', () => ({ bot: { slug } })); + const host = client(fake); + expect(await loadApprovalTemplates(host, slug)).toMatchObject({ + selected: '@global', + globalDefault, + }); + await setApprovalTemplate(host, slug, '@global'); + expect(calls(fake)).toContainEqual(['botApprovalTemplateSet', { slug, template: '@global' }]); + }, + ); + + it.each([42, {}, undefined])( + 'refuses an unreadable inherited global default instead of treating it as Off', + async (globalDefault) => { + const fake = new FakeHost(); + fake.methods.set('approvalTemplates', () => ({ + templates: [], + selected: '@global', + globalDefault, + })); + await expect(loadApprovalTemplates(client(fake), slug)).rejects.toBeInstanceOf(Error); + }, + ); + it('reads and sets the auto approval template', async () => { const fake = new FakeHost(); fake.methods.set('approvalTemplates', () => ({ diff --git a/changes/4211-effective-approval-template.md b/changes/4211-effective-approval-template.md index 0b2bdbb6c..c47fb3167 100644 --- a/changes/4211-effective-approval-template.md +++ b/changes/4211-effective-approval-template.md @@ -4,8 +4,8 @@ section: Changed ## en -- Added optional global approval defaults and Bot inheritance, specific selection or Off; each Review captures its template version and either allows this call or asks the Human, preserving deterministic authorization and management fences ([#4211](https://github.com/BotHarness/DeepSeekBot/issues/4211)). +- Added optional global approval defaults and Bot inheritance, specific selection or Off in the Web Client and mobile app; each Review captures its template version and either allows this call or asks the Human, preserving deterministic authorization and management fences ([#4211](https://github.com/BotHarness/DeepSeekBot/issues/4211)). ## zh -- 新增可选全局审批默认及 Bot 继承、特定选择或关闭；Review 固定模板版本，仅本次放行或转 Human，保留确定性授权及管理检查（[#4211](https://github.com/BotHarness/DeepSeekBot/issues/4211)）。 +- 新增可选全局审批默认及 Bot 继承、特定选择或关闭，Web Client 与移动端同步显示继承来源；Review 固定模板版本，仅本次放行或转 Human，保留确定性授权及管理检查（[#4211](https://github.com/BotHarness/DeepSeekBot/issues/4211)）。 diff --git a/packages/client/test/approval-template-entry.test.ts b/packages/client/test/approval-template-entry.test.ts index bb78440dd..94c893fc7 100644 --- a/packages/client/test/approval-template-entry.test.ts +++ b/packages/client/test/approval-template-entry.test.ts @@ -1,5 +1,5 @@ // @vitest-environment jsdom -import { act, createElement, type PropsWithChildren } from 'react'; +import { act, createElement, type PropsWithChildren, type ReactNode } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, expect, it, vi } from 'vitest'; import { ApprovalTemplateEntry } from '../src/client/approval-template-entry.js'; @@ -8,6 +8,28 @@ import { zhTranslate as t } from '../src/client/locale.js'; vi.mock('@deepseek-ai/dsh-client-ui-primitives', async () => ({ ...(await import('./primitive-mocks.js')).comboboxPrimitives(), Tag: ({ children }: PropsWithChildren) => createElement('span', null, children), + Menu: ({ + items, + onSelect, + }: { + items: { id: string; label: ReactNode }[]; + onSelect(id: string): void; + }) => + createElement( + 'div', + { role: 'menu' }, + items.map((item) => + createElement( + 'button', + { + key: item.id, + role: 'menuitem', + onClick: () => onSelect(item.id), + }, + item.label, + ), + ), + ), })); vi.mock('../src/client/avatar.js', () => ({ PersonaBotAvatar: () => null })); vi.mock('../src/client/channel-sidebar-icon.js', () => ({ ChannelSidebarIcon: () => null })); @@ -90,3 +112,43 @@ it('preserves a missing specific template name in the selector', async () => { expect(choice?.textContent).toContain('不可用'); expect(actions.setBotApprovalTemplate).not.toHaveBeenCalled(); }); + +it('does not display Off or replay a mutation when saving succeeds and readback fails', async () => { + let reads = 0; + let saved: string | null = null; + const actions = { + approvalTemplates: async () => { + reads++; + if (reads > 1) throw new Error('refresh failed'); + return { + templates: [ + { name: 'cautious', title: 'Cautious', description: '', builtIn: true, usedBy: [] }, + ], + selected: null, + globalDefault: null, + }; + }, + setBotApprovalTemplate: vi.fn(async (_slug: string, name: string | null) => { + saved = name; + }), + }; + await act(async () => + root.render( + createElement(ApprovalTemplateEntry, { botSlug: 'ada', actions, disabled: false, t }), + ), + ); + const trigger = () => host.querySelector('button[aria-haspopup="menu"]')!; + await act(async () => trigger().click()); + const option = [...document.querySelectorAll('[role="menuitem"]')].find( + (button) => button.textContent === t('autoApproval.template.cautious'), + ); + if (!option) throw new Error('Missing Cautious option'); + await act(async () => option.click()); + expect(saved).toBe('cautious'); + expect(actions.setBotApprovalTemplate).toHaveBeenCalledExactlyOnceWith('ada', 'cautious'); + expect(reads).toBe(2); + expect(trigger().textContent).toContain('读取失败'); + expect(trigger().textContent).not.toContain(t('autoApproval.off')); + expect(trigger().disabled).toBe(true); + expect(host.textContent).not.toContain(t('autoApproval.hintOff')); +}); From 7b85b9ef8d33b07e57bca9b6a9c08fa6734ca39e Mon Sep 17 00:00:00 2001 From: DoodleBears Date: Mon, 16 Nov 2026 01:19:08 +0900 Subject: [PATCH 05/11] fix(permissions): bound template reads and reuse request selection Issue: #4211 AgentTask: bot-approval-implementation --- packages/core/src/bridge/methods.ts | 21 ++++++-- .../core/src/workspaces/approval-templates.ts | 36 ++++++++++++-- .../test/approval-template-library.test.ts | 48 ++++++++++++++++++- 3 files changed, 95 insertions(+), 10 deletions(-) diff --git a/packages/core/src/bridge/methods.ts b/packages/core/src/bridge/methods.ts index d89bb7278..e8e563cae 100644 --- a/packages/core/src/bridge/methods.ts +++ b/packages/core/src/bridge/methods.ts @@ -461,6 +461,7 @@ import { INHERIT_APPROVAL_TEMPLATE, createApprovalTemplateLibrary, type ApprovalTemplate, + type ApprovalTemplateDefault, type ApprovalTemplateLibrary, type ApprovalTemplateWriteResult, } from '../workspaces/approval-templates.js'; @@ -2291,8 +2292,10 @@ function buildBridgeMethods( .list() .filter((record) => record.approvalTemplate === name) .map((record) => record.slug); - const templateEntry = (template: ApprovalTemplate): ApprovalTemplateEntry => { - const global = approvalTemplateLibrary.global(); + const templateEntry = ( + template: ApprovalTemplate, + global: ApprovalTemplateDefault, + ): ApprovalTemplateEntry => { return { ...template, usedBy: [ @@ -5133,7 +5136,6 @@ function buildBridgeMethods( return { ok: true, value: detailOf(result.record) }; }, approvalTemplates(payload) { - const templates = approvalTemplateLibrary.list().map(templateEntry); const global = approvalTemplateLibrary.global(); if (global.kind === 'invalid') return { @@ -5143,6 +5145,9 @@ function buildBridgeMethods( message: 'Approval template selection could not be read.', }, }; + const templates = approvalTemplateLibrary + .list() + .map((template) => templateEntry(template, global)); const globalDefault = global.kind === 'selected' ? global.name : null; if (asObject(payload)['slug'] === undefined) return { ok: true, value: { templates, globalDefault } }; @@ -5185,7 +5190,10 @@ function buildBridgeMethods( if (typeof text !== 'string') return invalidInput('text (Markdown) or from is required'); const result = approvalTemplateLibrary.create(name, text); if (!result.ok) return templateWriteFailure(name, result.reason); - return { ok: true, value: { template: templateEntry(result.template) } }; + return { + ok: true, + value: { template: templateEntry(result.template, approvalTemplateLibrary.global()) }, + }; }, approvalTemplateUpdate(payload) { const object = asObject(payload); @@ -5195,7 +5203,10 @@ function buildBridgeMethods( return invalidInput('name and text (Markdown) are required'); const result = approvalTemplateLibrary.update(name, text); if (!result.ok) return templateWriteFailure(name, result.reason); - return { ok: true, value: { template: templateEntry(result.template) } }; + return { + ok: true, + value: { template: templateEntry(result.template, approvalTemplateLibrary.global()) }, + }; }, promptParts() { return { diff --git a/packages/core/src/workspaces/approval-templates.ts b/packages/core/src/workspaces/approval-templates.ts index 1e6dfb87f..88a4e454f 100644 --- a/packages/core/src/workspaces/approval-templates.ts +++ b/packages/core/src/workspaces/approval-templates.ts @@ -1,4 +1,14 @@ -import { mkdirSync, readdirSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs'; +import { + closeSync, + existsSync, + mkdirSync, + openSync, + readdirSync, + readSync, + renameSync, + rmSync, + writeFileSync, +} from 'node:fs'; import { join } from 'node:path'; import { atomicWriteFile } from '../fs/atomic-write.js'; @@ -165,12 +175,30 @@ function textProblem(text: string): boolean { ); } +function readTemplateFile(path: string): string { + const descriptor = openSync(path, 'r'); + try { + const buffer = Buffer.alloc(APPROVAL_TEMPLATE_TEXT_LIMIT + 1); + let size = 0; + while (size < buffer.length) { + const count = readSync(descriptor, buffer, size, buffer.length - size, null); + if (count === 0) break; + size += count; + } + if (size > APPROVAL_TEMPLATE_TEXT_LIMIT) + throw new Error('Approval template file exceeds byte limit'); + return buffer.toString('utf8', 0, size); + } finally { + closeSync(descriptor); + } +} + export function createApprovalTemplateLibrary(dir?: string): ApprovalTemplateLibrary { const file = (name: string) => join(dir!, name + '.md'); const readCustom = (name: string): ApprovalTemplate | undefined => { if (dir === undefined || !isApprovalTemplateName(name) || approvalTemplate(name)) return; try { - return summaryOf(name, readFileSync(file(name), 'utf8')); + return summaryOf(name, readTemplateFile(file(name))); } catch { return undefined; } @@ -195,7 +223,7 @@ export function createApprovalTemplateLibrary(dir?: string): ApprovalTemplateLib if (dir === undefined) return { kind: 'off' }; let value: unknown; try { - value = JSON.parse(readFileSync(join(dir, '.selection.json'), 'utf8')); + value = JSON.parse(readTemplateFile(join(dir, '.selection.json'))); } catch (error) { return error instanceof Error && 'code' in error && error.code === 'ENOENT' ? { kind: 'off' } @@ -239,7 +267,7 @@ export function createApprovalTemplateLibrary(dir?: string): ApprovalTemplateLib create(name, text) { const problem = check(name, text); if (problem !== undefined) return problem; - if (readCustom(name) !== undefined) return { ok: false, reason: 'exists' }; + if (existsSync(file(name))) return { ok: false, reason: 'exists' }; return { ok: true, template: write(name, text) }; }, update(name, text) { diff --git a/packages/core/test/approval-template-library.test.ts b/packages/core/test/approval-template-library.test.ts index fddfb86f6..5c0bb5759 100644 --- a/packages/core/test/approval-template-library.test.ts +++ b/packages/core/test/approval-template-library.test.ts @@ -3,12 +3,13 @@ import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'no import { createServer } from 'node:http'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { runBotCreateCli } from '../src/bots/bot-create-cli.js'; import type { PersonaBotRecord } from '../src/bots/persona-bot.js'; import { createBridgeMethods, type BridgeMethodsDeps } from '../src/bridge/methods.js'; import { + APPROVAL_TEMPLATE_TEXT_LIMIT, changesApprovalPolicy, createApprovalTemplateLibrary, } from '../src/workspaces/approval-templates.js'; @@ -99,6 +100,27 @@ describe('Approval Template library', () => { expect(library.get('night-shift')).toBeUndefined(); }); + it('bounds externally edited template bytes before exposing them for review', () => { + const dir = tempDir(); + const library = createApprovalTemplateLibrary(dir); + const path = join(dir, 'external.md'); + const atLimit = 'x'.repeat(APPROVAL_TEMPLATE_TEXT_LIMIT); + writeFileSync(path, atLimit); + expect(library.get('external')?.text).toBe(atLimit); + for (const text of [atLimit + 'x', '界'.repeat(11_000), 'x'.repeat(65_559)]) { + writeFileSync(path, text); + expect(library.get('external')).toBeUndefined(); + expect(library.list().some((template) => template.name === 'external')).toBe(false); + expect(library.create('external', MINE)).toEqual({ ok: false, reason: 'exists' }); + expect(readFileSync(path, 'utf8')).toBe(text); + } + writeFileSync( + join(dir, '.selection.json'), + JSON.stringify({ template: 'cautious', padding: atLimit }), + ); + expect(library.global()).toEqual({ kind: 'invalid' }); + }); + it('refuses bad names, empty text, unknown templates and changes to built-ins', () => { const library = createApprovalTemplateLibrary(join(tempDir(), 'approval-templates')); expect(library.create('Bad Name', MINE)).toEqual({ ok: false, reason: 'invalid-name' }); @@ -220,6 +242,30 @@ describe('Approval Template bridge methods', () => { }; } + it('uses one fresh global snapshot for a settings response and all template references', () => { + const { bridge, library, registry } = methods(); + library.create('night-shift', MINE); + library.setGlobal('night-shift'); + registry.setApprovalTemplate('bob', '@global'); + const readGlobal = vi.spyOn(library, 'global'); + const first = bridge.approvalTemplates({ slug: 'bob' }); + expect(readGlobal).toHaveBeenCalledTimes(1); + expect(first).toMatchObject({ + value: { globalDefault: 'night-shift', effective: 'night-shift' }, + }); + expect( + first.ok && first.value.templates.find((template) => template.name === 'night-shift')?.usedBy, + ).toEqual(['ada', 'bob']); + library.setGlobal('cautious'); + readGlobal.mockClear(); + const next = bridge.approvalTemplates({ slug: 'bob' }); + expect(readGlobal).toHaveBeenCalledTimes(1); + expect(next).toMatchObject({ value: { globalDefault: 'cautious', effective: 'cautious' } }); + expect( + next.ok && next.value.templates.find((template) => template.name === 'cautious')?.usedBy, + ).toEqual(['bob']); + }); + it('keeps unreadable global selection distinct from a deliberately empty default', () => { const { bridge, dir } = methods(); expect(bridge.approvalTemplateGlobalSet({ template: null })).toMatchObject({ ok: true }); From 057288126025fb43870b2eb89bf6ded675496942 Mon Sep 17 00:00:00 2001 From: DoodleBears Date: Mon, 16 Nov 2026 01:22:06 +0900 Subject: [PATCH 06/11] test(permissions): guard actual template read volume and cleanup Issue: #4211 AgentTask: bot-approval-implementation --- .../test/approval-template-library.test.ts | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/core/test/approval-template-library.test.ts b/packages/core/test/approval-template-library.test.ts index 5c0bb5759..71e36c708 100644 --- a/packages/core/test/approval-template-library.test.ts +++ b/packages/core/test/approval-template-library.test.ts @@ -1,6 +1,8 @@ import { OWNER_HUMAN_CONTEXT } from '../src/humans/context.js'; import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { createServer } from 'node:http'; +import nativeFs from 'node:fs'; +import { syncBuiltinESMExports } from 'node:module'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -121,6 +123,33 @@ describe('Approval Template library', () => { expect(library.global()).toEqual({ kind: 'invalid' }); }); + it('caps actual file I/O and closes the descriptor after rejecting external growth', () => { + const dir = tempDir(); + const library = createApprovalTemplateLibrary(dir); + const path = join(dir, 'external.md'); + writeFileSync(path, 'x'.repeat(65_559)); + const opened = vi.spyOn(nativeFs, 'openSync'); + const read = vi.spyOn(nativeFs, 'readSync'); + const closed = vi.spyOn(nativeFs, 'closeSync'); + syncBuiltinESMExports(); + try { + expect(library.get('external') === undefined).toBe(true); + expect(opened).toHaveBeenCalledExactlyOnceWith(path, 'r'); + const descriptor = opened.mock.results[0]!.value; + expect(read.mock.calls.length).toBeGreaterThan(0); + expect(read.mock.calls.every((call) => call[0] === descriptor)).toBe(true); + expect(read.mock.results.reduce((total, result) => total + Number(result.value), 0)).toBe( + APPROVAL_TEMPLATE_TEXT_LIMIT + 1, + ); + expect(closed).toHaveBeenCalledExactlyOnceWith(descriptor); + } finally { + opened.mockRestore(); + read.mockRestore(); + closed.mockRestore(); + syncBuiltinESMExports(); + } + }); + it('refuses bad names, empty text, unknown templates and changes to built-ins', () => { const library = createApprovalTemplateLibrary(join(tempDir(), 'approval-templates')); expect(library.create('Bad Name', MINE)).toEqual({ ok: false, reason: 'invalid-name' }); From b7cdf08c436e7777cbb95a989eb21f1b53365234 Mon Sep 17 00:00:00 2001 From: DoodleBears Date: Mon, 16 Nov 2026 01:59:39 +0900 Subject: [PATCH 07/11] Add explicit template sources and three-part Reviewer guidance Issue: #4244 Agent-Task: codex/local/t3-4d0dd206-86059e8a3599fc27 --- .../features/approval-templates.md | 3 +- changes/4244-template-guidance-editor.md | 11 + ...ps-deterministic-authorization-separate.md | 4 + docs/architecture/bot-approval-policy.md | 2 + docs/architecture/bot-approval-policy.zh.md | 2 + docs/bot-cli.md | 18 ++ docs/bot-cli.zh.md | 18 ++ .../src/client/approval-template-library.tsx | 252 +++++++++++++++--- packages/client/src/client/locale.ts | 38 ++- .../test/approval-template-library.test.ts | 110 +++++++- .../core/src/bots/cli-approval-templates.ts | 68 ++++- .../core/src/workspaces/approval-reviewer.ts | 3 +- .../workspaces/approval-template-guidance.ts | 46 ++++ .../core/src/workspaces/approval-templates.ts | 27 +- packages/core/test/approval-reviewer.test.ts | 69 +++++ .../test/approval-template-library.test.ts | 90 +++++++ 16 files changed, 702 insertions(+), 59 deletions(-) create mode 100644 changes/4244-template-guidance-editor.md create mode 100644 packages/core/src/workspaces/approval-template-guidance.ts diff --git a/.agents/skills/verify-deepseekbot/features/approval-templates.md b/.agents/skills/verify-deepseekbot/features/approval-templates.md index 920a47731..aa61d343c 100644 --- a/.agents/skills/verify-deepseekbot/features/approval-templates.md +++ b/.agents/skills/verify-deepseekbot/features/approval-templates.md @@ -5,6 +5,7 @@ A Bot inherits the optional global default, selects one complete shared template ## Sub-features - `library`: Bot settings → Auto approval → Global default template (DSH Select), existing template library and Reviewer model. No global default means inherited review is off. Read failures remain errors. +- `editor`: New template requires Blank or one selected source; edit Always ask/Always allow rows and free text, save alone or apply to a named Bot. Copy a custom or built-in template independently; shared-original editing lists actual users including global inheritors. Read back full content and selection through CLI. - `cli`: `approval-template list --bot `, `approval-template-global-set `, `approval-template-set `; library create/edit commands retain existing Markdown contents. - `select`: Channel settings → Auto approval, or composer Workspace Grants panel → Auto approval, uses the same DSH Select. Inheritance shows its effective global source. Specific selection replaces global; Off retains deterministic rules. - `mobile`: Bot profile → Settings → Permissions → Workspace grants → Auto approval template, inside the existing iOS sheet / Android `ModalBottomSheet`. Inherit global shows the resolved global template (or Off); selecting Off then Inherit global writes null then @global through the same Host owner. Legacy Hosts without a global-selection field retain their supported choices. @@ -33,5 +34,5 @@ Use the shared device QA lane and T3 device tools. The same synthetic Bot and is ## Gotchas - `/tmp` may already be writable; an ordinary file write there need not reach an approval gate. -- Three-part editing, card-rule table and confirmed bulk deletion fallback are separate issues; this journey does not qualify them. +- Card-rule table and confirmed bulk deletion fallback remain separate issues. For the editor, verify legacy Markdown survives conversion and an edit during a Review changes only the next captured version. - Browser screenshots establish Web behavior only. Preserve failed drives and observe before retrying. diff --git a/changes/4244-template-guidance-editor.md b/changes/4244-template-guidance-editor.md new file mode 100644 index 000000000..fd59af417 --- /dev/null +++ b/changes/4244-template-guidance-editor.md @@ -0,0 +1,11 @@ +--- +section: Changed +--- + +## en + +- Added explicit blank or selected-source template creation, independent copies, three-part Reviewer guidance, and separate save/apply choices; shared-original edits disclose the Bots using them ([#4244](https://github.com/BotHarness/DeepSeekBot/issues/4244)). + +## zh + +- 新增明确选择空白或来源模板创建独立副本、三部分 Reviewer 指导及仅保存／保存并应用选项；编辑共享原件时显示使用它的 Bot（[#4244](https://github.com/BotHarness/DeepSeekBot/issues/4244)）。 diff --git a/docs/adr/0342-bot-approval-selects-one-template-and-keeps-deterministic-authorization-separate.md b/docs/adr/0342-bot-approval-selects-one-template-and-keeps-deterministic-authorization-separate.md index af51461ff..02b637b66 100644 --- a/docs/adr/0342-bot-approval-selects-one-template-and-keeps-deterministic-authorization-separate.md +++ b/docs/adr/0342-bot-approval-selects-one-template-and-keeps-deterministic-authorization-separate.md @@ -29,3 +29,7 @@ The existing application-defined approval owner remains responsible for decision This amends ADR-0207's template selection, Markdown-only policy shape, three Reviewer outcomes and rejection circuit breaker as target design, not retroactive runtime fact. ADR-0249 continues to govern explicit management authorization. The existing issue bodies are not rewritten by this record. Rejected alternatives were global-plus-Bot layering (ambiguous effective policy), applying new defaults to old disabled Bots (implicit opt-in), autonomous model rejection (removes the Human's decision on unapproved requests), hidden copies/application (unclear scope), and refusing deletion until every reference is removed (unnecessary repeated work). The selected design makes defaults dynamic and deletion easier, at the cost of a carefully disclosed bulk change and immutable policy snapshots for already-started Reviews. + +## File compatibility for #4244 + +Three-part editing keeps the existing template-file owner and 32 KiB bounded reader. A saved structured template uses the exact marker ``, a newline, then JSON containing `alwaysAsk` and `alwaysAllow` string arrays and `freeText`. This is a versioned content representation inside the existing file, not a SQLite migration. Legacy Markdown without that marker maps in full to free text with empty row arrays; headings are not parsed into rules or rewritten. Invalid marked content is unavailable, so a Review falls back to the Human. The Review content hash covers the complete saved representation; decoding for Reviewer assembly uses only that captured text. Copies contain content only. Global defaults and built-ins are unchanged. diff --git a/docs/architecture/bot-approval-policy.md b/docs/architecture/bot-approval-policy.md index 2a47de29d..fa3c69d4b 100644 --- a/docs/architecture/bot-approval-policy.md +++ b/docs/architecture/bot-approval-policy.md @@ -109,3 +109,5 @@ The User confirmed the design after questions Q1–Q18 and a complete summary. T Future delivery must demonstrate inherited versus specific/Off behavior, Reviewer conflict/uncertainty routing, deterministic bypass without an LLM call, Bot-only card-rule scope, disclosed deletion fallback, captured policy versions, failure routing and management refusal without valid authority. These are acceptance requirements, not claims that tests or the runtime already implement them. Performance assessment: this change edits documentation only, introduces no dependency and changes no runtime hot path, so no runtime benchmark is warranted. Future E2/E3 work must separately budget added admission work and model calls, prove deterministic hits make no Reviewer call, and verify the affected real journey under the repository's performance policy. + +The #4244 editor retains the template-file owner and 32 KiB bounded reads. ADR-0342 records the v1 content representation; legacy Markdown maps intact to free text. Copies contain only content. Save and apply are separate choices, shared-original editing shows global inheritors among actual users, and content edits affect future Reviews. diff --git a/docs/architecture/bot-approval-policy.zh.md b/docs/architecture/bot-approval-policy.zh.md index 35af0dcfa..3d79c534e 100644 --- a/docs/architecture/bot-approval-policy.zh.md +++ b/docs/architecture/bot-approval-policy.zh.md @@ -111,3 +111,5 @@ Human 在 Q1–Q18 分轮访谈后确认了完整汇总；原设计树无未决 后续交付须验证：继承／特定／关闭的有效选择；冲突及不确定转 Human；确定性命中时没有 LLM 调用；卡片规则只授权所属 Bot；删除结果披露及回退；Review 固定版本；故障转 Human；无有效管理授权时拒绝策略变更。这是未来验收要求，不是已实现或已通过测试的声明。 性能评估：本次只有文档变更，无新依赖、无运行热路径变化，因此免运行性能基准。未来 E2／E3 须分别预算新增准入工作及模型调用，证明确定性命中不调用 Reviewer，并依仓库性能规则验证实际旅程。 + +#4244 的三部分编辑沿用模板文件 owner 与 32 KiB 有界读取；使用 ADR-0342 记录的 v1 内容表示，旧 Markdown 完整映射为补充说明。复制只复制内容；保存与应用分开，编辑共享原件显示全局继承者在内的使用者。编辑仅影响后续 Review。 diff --git a/docs/bot-cli.md b/docs/bot-cli.md index e3f8f6d0b..7641e96ab 100644 --- a/docs/bot-cli.md +++ b/docs/bot-cli.md @@ -2102,3 +2102,21 @@ deepseekbot models allowed [--as ] ``` Tool effect declarations (E1) preserve the existing tool commands and approval outcomes. Full Access, saved rules, Secrets, device targets and Workspace Grants retain their existing checks; no new route or CLI authority is introduced. + +### Three-part approval guidance + +Create an empty draft explicitly with `deepseekbot approval-template create my-draft --blank`, or an independent copy with `deepseekbot approval-template create my-copy --from cautious`. No source is selected implicitly; copies retain no origin link. Existing `--file` / `--text-stdin` Markdown input remains supported. + +For three-part editing, write a UTF-8 JSON file: + +```json +{ + "alwaysAsk": ["Send messages or spend money"], + "alwaysAllow": ["Read local project files"], + "freeText": "Ask me when unsure." +} +``` + +Run `deepseekbot approval-template edit my-draft --parts-file guidance.json`. Read it back with `deepseekbot approval-template show my-draft`; `text` contains the versioned representation. Add `--apply-to ` to create/edit to save and then select it for that Bot. Without that flag, saving changes no selection. Applying uses the existing Bot selection authorization; if applying fails, the saved draft remains and can be selected separately. + +Editing a shared original affects future Reviews for every Bot in its `usedBy` list, including global inheritors. Copy first to work independently. Always ask has priority over Always allow, then free text supplements both. These are fallible natural-language instructions, never deterministic Tool Approval Rules. Conflicts, missing coverage or uncertainty ask the Human. Legacy Markdown remains intact as free text when converted; already-started Reviews retain their captured content/version. diff --git a/docs/bot-cli.zh.md b/docs/bot-cli.zh.md index 5d6839d73..7201f8f12 100644 --- a/docs/bot-cli.zh.md +++ b/docs/bot-cli.zh.md @@ -2043,3 +2043,21 @@ Web 手动检查请求最多等待 25 秒，关闭自检卡片会取消请求。 公网阶段仅对 DNS、网络、超时及 HTTP 429/5xx 短暂失败重试：最多四次，间隔 1/3/6 秒，仍受同一个 20 秒总时限约束。证书或身份失败不重试。每次自检完成后，界面刷新一次账号电脑列表。 工具效果声明（E1）保持既有工具命令与审批结果。Full Access、保存规则、Secrets、设备目标和 Workspace Grant 仍使用原检查；不新增 route 或 CLI 权限。 + +### 三部分审批指导 + +使用 `deepseekbot approval-template create my-draft --blank` 明确创建空白草稿，或使用 `deepseekbot approval-template create my-copy --from cautious` 创建独立副本。不会自动选取来源，副本不保留来源关联。仍支持 `--file`／`--text-stdin` Markdown 输入。 + +三部分编辑使用 UTF-8 JSON 文件： + +```json +{ + "alwaysAsk": ["发送消息或花钱"], + "alwaysAllow": ["读取本地项目文件"], + "freeText": "不确定时问我。" +} +``` + +运行 `deepseekbot approval-template edit my-draft --parts-file guidance.json`，再用 `deepseekbot approval-template show my-draft` 读回；`text` 包含带格式版本的内容。创建／编辑时添加 `--apply-to ` 才会在保存后为该 Bot 选用模板。不加时不改变选择。应用仍通过现有 Bot 选择授权；应用失败时已保存的草稿保留，可单独选用。 + +编辑共享原件影响 `usedBy` 中所有 Bot 的后续 Review，包括全局继承者。需要独立修改时先复制。总是询问优先于总是允许，补充说明不能取消前者。这些是可能被模型误解的自然语言指导，不是确定性 Tool Approval Rules；冲突、未覆盖或不确定均交给 Human。转换时旧 Markdown 完整保留为补充说明，已经开始的 Review 保留捕获的内容和版本。 diff --git a/packages/client/src/client/approval-template-library.tsx b/packages/client/src/client/approval-template-library.tsx index 7719b524e..ba9a5056d 100644 --- a/packages/client/src/client/approval-template-library.tsx +++ b/packages/client/src/client/approval-template-library.tsx @@ -1,4 +1,9 @@ -import { useClientFields } from './use-client-store.js'; +import { + decodeApprovalTemplateGuidance, + encodeApprovalTemplateGuidance, + type ApprovalTemplateGuidance, +} from '../../../core/src/workspaces/approval-template-guidance.js'; +import { useClientFields, useStoreSlice } from './use-client-store.js'; import { useState, type ReactElement } from 'react'; import { Button, Tag } from '@deepseek-ai/dsh-client-ui-primitives'; import * as stylex from '@stylexjs/stylex'; @@ -10,6 +15,7 @@ import { errorMessage, loadApprovalTemplates, setGlobalApprovalTemplate, + setBotApprovalTemplate, updateApprovalTemplate, type ApprovalTemplateView, type BridgeCall, @@ -24,7 +30,15 @@ import { space } from './ds/tokens.stylex.js'; import type { BotHarnessTranslate } from './locale.js'; import { LoadingSkeleton } from './loading-skeleton.js'; import { useMountedResource } from './mounted-resource.js'; -import type { ClientStore } from './store.js'; +import type { ClientState, ClientStore } from './store.js'; + +function currentTemplateBot(state: ClientState): string | undefined { + const selection = state.selection; + if (selection?.kind === 'bot') return selection.slug; + if (selection?.kind === 'channel') + return state.channels.find((channel) => channel.id === selection.channelId)?.botSlug; + return undefined; +} const TEMPLATE_LIBRARY_FIELDS = ['bots'] as const; @@ -35,6 +49,7 @@ const styles = stylex.create({ }); type Editor = + | { mode: 'source' } | { mode: 'create'; name: string; text: string } | { mode: 'view'; template: ApprovalTemplateView } | { mode: 'edit'; template: ApprovalTemplateView; text: string }; @@ -52,9 +67,82 @@ function copyName(name: string, taken: readonly ApprovalTemplateView[]): string return candidate; } -function copyText(text: string, suffix: string): string { - return text.replace(/^(#[ \t]+)(.+)$/mu, (_match, mark: string, title: string) => - title.trimEnd().endsWith(suffix) ? mark + title : mark + title.trimEnd() + ' ' + suffix, +function GuidanceFields({ + text, + onChange, + t, +}: { + text: string; + onChange(text: string): void; + t: BotHarnessTranslate; +}): ReactElement { + const parts = decodeApprovalTemplateGuidance(text); + const update = (next: ApprovalTemplateGuidance): void => + onChange(encodeApprovalTemplateGuidance(next)); + return ( + + {t('autoApproval.library.guidanceHint')} + {(['alwaysAsk', 'alwaysAllow'] as const).map((key) => ( + + {parts[key].map((row, index) => ( +
+ + update({ + ...parts, + [key]: parts[key].map((item, position) => (position === index ? value : item)), + }) + } + /> + +
+ ))} + + + ))} + update({ ...parts, freeText })} + /> + ); } @@ -73,9 +161,12 @@ export function ApprovalTemplateLibrarySettings({ const [loadError, setLoadError] = useState(); const [editor, setEditor] = useState(); const [deleting, setDeleting] = useState(); + const [applyTo, setApplyTo] = useState(''); const [busy, setBusy] = useState(false); const [error, setError] = useState(); - useClientFields(TEMPLATE_LIBRARY_FIELDS, store); + const { bots } = useClientFields(TEMPLATE_LIBRARY_FIELDS, store); + const currentBotSlug = useStoreSlice(store, currentTemplateBot); + const currentBot = bots.find((bot) => bot.slug === currentBotSlug); const reload = async (): Promise => { const next = await loadApprovalTemplates(call); @@ -121,9 +212,15 @@ export function ApprovalTemplateLibrarySettings({ const close = (): void => { setEditor(undefined); + setApplyTo(''); setDeleting(undefined); setError(undefined); }; + const createDraft = (draft: Extract): void => { + setApplyTo(''); + setError(undefined); + setEditor(draft); + }; const run = (operation: () => Promise): void => { if (busy) return; setBusy(true); @@ -165,6 +262,63 @@ export function ApprovalTemplateLibrarySettings({ ); } if (editor === undefined) return null; + if (editor.mode === 'source') + return ( + + + ); + const apply = async (name: string): Promise => { + if (applyTo !== '') await setBotApprovalTemplate(call, applyTo, name); + }; if (editor.mode === 'create') { const nameProblem = editor.name !== '' && !NAME.test(editor.name) @@ -183,7 +337,13 @@ export function ApprovalTemplateLibrarySettings({ error={error} onCancel={close} onSubmit={() => - run(() => createApprovalTemplate(call, editor.name, { text: editor.text })) + run(async () => { + const template = await createApprovalTemplate(call, editor.name, { + text: editor.text, + }); + setEditor({ mode: 'edit', template, text: editor.text }); + await apply(editor.name); + }) } > @@ -198,13 +358,12 @@ export function ApprovalTemplateLibrarySettings({ autoComplete="off" onChange={(name) => setEditor({ ...editor, name: name.trim() })} /> - setEditor({ ...editor, text })} + t={t} /> + {saveChoice} ); @@ -222,10 +381,10 @@ export function ApprovalTemplateLibrarySettings({ submitData={{ 'data-approval-template-submit': 'copy' }} onCancel={close} onSubmit={() => - setEditor({ + createDraft({ mode: 'create', name: copyName(template.name, list), - text: copyText(template.text ?? '', t('autoApproval.library.copySuffix')), + text: template.text ?? '', }) } > @@ -248,34 +407,56 @@ export function ApprovalTemplateLibrarySettings({ labels={labels} submitLabel={t('autoApproval.library.save')} submitData={{ 'data-approval-template-submit': 'save' }} - submitDisabled={editor.text.trim() === '' || editor.text === template.text} + submitDisabled={ + editor.text.trim() === '' || (editor.text === template.text && applyTo === '') + } busy={busy} error={error} onCancel={close} - onSubmit={() => run(() => updateApprovalTemplate(call, template.name, editor.text))} + onSubmit={() => + run(async () => { + if (editor.text !== template.text) + await updateApprovalTemplate(call, template.name, editor.text); + await apply(template.name); + }) + } secondary={ - { - setError(undefined); - setDeleting(template); - }} - > - {t('autoApproval.library.delete')} - + <> + + { + setError(undefined); + setDeleting(template); + }} + > + {t('autoApproval.library.delete')} + + } > {names === undefined ? null : ( {t('autoApproval.library.inUse', { names })} )} - setEditor({ ...editor, text })} + t={t} /> + {saveChoice} ); })(); @@ -348,7 +529,7 @@ export function ApprovalTemplateLibrarySettings({ variant="outline" size="sm" disabled={templates === undefined} - onClick={() => setEditor({ mode: 'create', name: '', text: '' })} + onClick={() => setEditor({ mode: 'source' })} > {t('autoApproval.library.create')} @@ -391,6 +572,13 @@ export function ApprovalTemplateLibrarySettings({ - - ))} - - - ))} + {(['alwaysAsk', 'alwaysAllow'] as const).map((key) => { + const label = t( + key === 'alwaysAsk' + ? 'autoApproval.library.alwaysAsk' + : 'autoApproval.library.alwaysAllow', + ); + return ( + + {() => ( + + {parts[key].map((row, index) => { + const rowLabel = label + ' ' + String(index + 1); + return ( + {index + 1}} + title={ + + update({ + ...parts, + [key]: parts[key].map((item, position) => + position === index ? event.currentTarget.value : item, + ), + }) + } + /> + } + trailing={ + + } + /> + ); + })} + update({ ...parts, [key]: [...parts[key], ''] })} + /> + + )} + + ); + })} { expect(button.getAttribute('aria-label')).toContain(button.textContent); } }); + +it('edits and removes independent guidance cards without changing neighbouring rules or applying', async () => { + let template = { + name: 'qa-rules', + title: 'QA rules', + description: '', + builtIn: false, + usedBy: [], + text: encodeApprovalTemplateGuidance({ + alwaysAsk: ['Ask A', 'Ask B', 'Ask C'], + alwaysAllow: ['Allow A', 'Allow B'], + freeText: 'Keep free text unchanged.', + }), + }; + const calls: string[] = []; + const call: BridgeCall = async (endpoint, payload) => { + calls.push(endpoint); + if (endpoint === 'approvalTemplates') + return { ok: true, value: { templates: [template], globalDefault: null } }; + if (endpoint === 'approvalTemplateUpdate') { + if (typeof payload.text !== 'string') throw new Error('Missing saved guidance'); + template = { ...template, text: payload.text }; + return { ok: true, value: { template } }; + } + throw new Error('Unexpected endpoint ' + endpoint); + }; + const ask = t('autoApproval.library.alwaysAsk'); + const allow = t('autoApproval.library.alwaysAllow'); + const input = (label: string): HTMLInputElement => { + const found = document.querySelector('input[aria-label="' + label + '"]'); + if (!found) throw new Error('Missing rule ' + label); + return found; + }; + const fill = async (label: string, value: string): Promise => { + await act(async () => { + Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set!.call( + input(label), + value, + ); + input(label).dispatchEvent(new Event('input', { bubbles: true })); + }); + }; + const remove = async (label: string): Promise => { + const found = document.querySelector( + 'button[aria-label="' + t('autoApproval.library.removeRow') + ' · ' + label + '"]', + ); + if (!found) throw new Error('Missing remove action ' + label); + await act(async () => found.click()); + }; + await render(call); + const edit = host.querySelector('[data-approval-template="qa-rules"] button'); + if (!edit) throw new Error('Missing template'); + await act(async () => edit.click()); + await fill(ask + ' 2', 'Ask B edited'); + await remove(ask + ' 1'); + expect(input(ask + ' 1').value).toBe('Ask B edited'); + expect(input(ask + ' 2').value).toBe('Ask C'); + expect(input(allow + ' 1').value).toBe('Allow A'); + await clickText(t('autoApproval.library.addRow', { section: ask })); + await fill(ask + ' 3', 'Ask D'); + await remove(ask + ' 2'); + await fill(allow + ' 2', 'Allow B edited'); + expect(document.querySelector('textarea')?.value).toBe('Keep free text unchanged.'); + await clickText(t('autoApproval.library.save')); + expect(decodeApprovalTemplateGuidance(template.text)).toEqual({ + alwaysAsk: ['Ask B edited', 'Ask D'], + alwaysAllow: ['Allow A', 'Allow B edited'], + freeText: 'Keep free text unchanged.', + }); + expect(calls).toEqual(['approvalTemplates', 'approvalTemplateUpdate', 'approvalTemplates']); +}); From 43cfc1b7d9a176bcb98b1b6e8fe806fc526990b2 Mon Sep 17 00:00:00 2001 From: BotHarness Date: Mon, 16 Nov 2026 05:42:17 +0900 Subject: [PATCH 11/11] Keep keyboard focus visible within grouped guidance cards Restore a token-based focus-within outline while keeping shared card rows straight at the joins.\n\nRefs #4244\nAgent claim: https://github.com/BotHarness/DeepSeekBot/issues/4244#issuecomment-6899510593 --- packages/client/src/client/approval-template-library.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/client/src/client/approval-template-library.tsx b/packages/client/src/client/approval-template-library.tsx index d5b10a183..ae3466c02 100644 --- a/packages/client/src/client/approval-template-library.tsx +++ b/packages/client/src/client/approval-template-library.tsx @@ -26,7 +26,7 @@ import { ConfirmDialog, DangerTextButton, FormDialog } from './ds/form-dialog.js import { Note } from './ds/note.js'; import { Select } from './ds/select.js'; import { SettingsRow, SettingsSection } from './ds/settings-row.js'; -import { space, text } from './ds/tokens.stylex.js'; +import { border, color, space, text } from './ds/tokens.stylex.js'; import { ChannelSidebarIcon } from './channel-sidebar-icon.js'; import { SidebarCardList, SidebarCardRow } from './sidebar-card.js'; import type { BotHarnessTranslate } from './locale.js'; @@ -56,6 +56,10 @@ const styles = stylex.create({ borderRadius: 0, backgroundColor: 'transparent', fontWeight: text.regular, + outlineStyle: 'solid', + outlineWidth: border.focus, + outlineColor: { default: 'transparent', ':focus-within': color.fg }, + outlineOffset: -2, }, });