diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index ea64ece..01854b0 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -256,7 +256,7 @@ Tooling is deliberately split into two levels: - **dynamic runtime tools** reported by the Copilot CLI, with a fallback catalog for startup/offline cases - **global definitions** for MCP servers and LSP profiles -- **pattern defaults** for which known runtime tools can bypass manual approval +- **pattern defaults** where tool-call approval is enabled by default, plus which known runtime tools can bypass manual approval - **per-session overrides** for both tool enablement and tool auto-approval This lets the application treat tooling as reusable workspace capability while still preserving session-level control and safety. diff --git a/README.md b/README.md index 46a9064..e525dc1 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ You can define MCP servers and LSP profiles once in **Settings**, then enable th This keeps machine-wide tooling reusable while still letting each session decide which external tools the agent can use. -Patterns can also store default auto-approval for known MCP and LSP tools, and each session can override those defaults from the Activity panel before a run starts. +Patterns now require tool-call approval by default. They can also store default auto-approval for known MCP and LSP tools, and each session can override those auto-approval defaults from the Activity panel before a run starts. ### Watch runs as they happen @@ -84,7 +84,7 @@ Aryx includes connection status in the app so you can quickly tell whether Copil Use a simple single-agent setup to begin, or choose a saved multi-agent pattern when you want a more structured workflow. 5. **Configure optional tooling** - If you want MCP or LSP support, add the global definitions in settings and then enable the ones you want for the current session from the Activity panel. Aryx also surfaces Copilot CLI runtime tools for tool auto-approval, and you can set pattern-level defaults and override them per session. + If you want MCP or LSP support, add the global definitions in settings and then enable the ones you want for the current session from the Activity panel. Aryx also surfaces Copilot CLI runtime tools for approval management: tool calls require approval by default, and you can set pattern-level auto-approval defaults and override them per session. 6. **Start working** Ask a question, describe a task, or explore a project. As the run progresses, you can watch the participating agents and keep the session for later. diff --git a/src/main/persistence/workspaceRepository.ts b/src/main/persistence/workspaceRepository.ts index affe969..494d850 100644 --- a/src/main/persistence/workspaceRepository.ts +++ b/src/main/persistence/workspaceRepository.ts @@ -9,7 +9,7 @@ import { normalizeWorkspaceSettings, } from '@shared/domain/tooling'; import { - normalizeApprovalPolicy, + applyDefaultToolApprovalPolicy, normalizePendingApprovalState, normalizeSessionApprovalSettings, } from '@shared/domain/approval'; @@ -70,7 +70,7 @@ export class WorkspaceRepository { ...stored, patterns: mergePatterns(stored.patterns ?? []).map((pattern) => ({ ...pattern, - approvalPolicy: normalizeApprovalPolicy(pattern.approvalPolicy), + approvalPolicy: applyDefaultToolApprovalPolicy(pattern.approvalPolicy), graph: resolvePatternGraph(pattern), })), projects, diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index a32b012..b044cac 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -21,6 +21,7 @@ import { normalizePatternModels, resolveReasoningEffort, } from '@shared/domain/models'; +import { createDefaultToolApprovalPolicy } from '@shared/domain/approval'; import { syncPatternGraph, type PatternDefinition } from '@shared/domain/pattern'; import { isScratchpadProject, SCRATCHPAD_PROJECT_ID } from '@shared/domain/project'; import { applyScratchpadSessionConfig } from '@shared/domain/session'; @@ -37,6 +38,7 @@ function createDraftPattern(defaultModelId: string, defaultReasoningEffort: Patt mode: 'single', availability: 'available', maxIterations: 1, + approvalPolicy: createDefaultToolApprovalPolicy(), agents: [ { id: createId('agent'), diff --git a/src/renderer/components/PatternEditor.tsx b/src/renderer/components/PatternEditor.tsx index 9693022..9627da7 100644 --- a/src/renderer/components/PatternEditor.tsx +++ b/src/renderer/components/PatternEditor.tsx @@ -198,7 +198,7 @@ export function PatternEditor({ updateApprovalPolicy((current) => { const otherRules = (current?.rules ?? []).filter((r) => r.kind !== kind); if (!enabled) { - return otherRules.length > 0 ? { rules: otherRules } : undefined; + return { rules: otherRules }; } return { rules: [...otherRules, { kind }] }; }); diff --git a/src/shared/domain/approval.ts b/src/shared/domain/approval.ts index 8eb1cdf..84078aa 100644 --- a/src/shared/domain/approval.ts +++ b/src/shared/domain/approval.ts @@ -46,6 +46,18 @@ const approvalCheckpointKinds: ApprovalCheckpointKind[] = ['tool-call', 'final-r const approvalCheckpointKindSet = new Set(approvalCheckpointKinds); const approvalStatusSet = new Set(['pending', 'approved', 'rejected']); +export function createDefaultToolApprovalPolicy(): ApprovalPolicy { + return { + rules: [{ kind: 'tool-call' }], + }; +} + +export function applyDefaultToolApprovalPolicy( + policy?: Partial, +): ApprovalPolicy { + return normalizeApprovalPolicy(policy) ?? createDefaultToolApprovalPolicy(); +} + export function isApprovalCheckpointKind(value: string | undefined): value is ApprovalCheckpointKind { return value !== undefined && approvalCheckpointKindSet.has(value as ApprovalCheckpointKind); } @@ -55,6 +67,10 @@ export function isApprovalStatus(value: string | undefined): value is ApprovalSt } export function normalizeApprovalPolicy(policy?: Partial): ApprovalPolicy | undefined { + if (policy == null) { + return undefined; + } + const rules = Array.isArray(policy?.rules) ? policy.rules : []; const selectedAgents = new Map>(); const appliesToAllAgents = new Set(); @@ -97,7 +113,9 @@ export function normalizeApprovalPolicy(policy?: Partial): Appro }); if (normalizedRules.length === 0 && autoApprovedToolNames.length === 0) { - return undefined; + return { + rules: [], + }; } return { diff --git a/src/shared/domain/pattern.ts b/src/shared/domain/pattern.ts index 58167a4..75c0ce1 100644 --- a/src/shared/domain/pattern.ts +++ b/src/shared/domain/pattern.ts @@ -1,5 +1,6 @@ import type { ChatMessageRecord } from '@shared/domain/session'; import { + applyDefaultToolApprovalPolicy, normalizeApprovalPolicy, type ApprovalPolicy, validateApprovalPolicy, @@ -513,7 +514,10 @@ export function createBuiltinPatterns(timestamp: string): PatternDefinition[] { }, ]; - return patterns.map((pattern) => syncPatternGraph(pattern)); + return patterns.map((pattern) => syncPatternGraph({ + ...pattern, + approvalPolicy: applyDefaultToolApprovalPolicy(pattern.approvalPolicy), + })); } function countByKind(graph: PatternGraph): Map { diff --git a/tests/shared/approval.test.ts b/tests/shared/approval.test.ts index 1c2650e..6a21d3d 100644 --- a/tests/shared/approval.test.ts +++ b/tests/shared/approval.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from 'bun:test'; import { + applyDefaultToolApprovalPolicy, approvalPolicyRequiresToolCallApproval, approvalPolicyRequiresCheckpoint, normalizeApprovalPolicy, @@ -14,6 +15,20 @@ import { } from '@shared/domain/approval'; describe('approval helpers', () => { + test('applies tool-call approval by default while preserving an explicit empty policy', () => { + expect(applyDefaultToolApprovalPolicy()).toEqual({ + rules: [{ kind: 'tool-call' }], + }); + + expect(normalizeApprovalPolicy({})).toEqual({ + rules: [], + }); + + expect(applyDefaultToolApprovalPolicy({})).toEqual({ + rules: [], + }); + }); + test('normalizes duplicate checkpoint rules and auto-approved tools into stable policy entries', () => { expect(normalizeApprovalPolicy({ rules: [ diff --git a/tests/shared/pattern.test.ts b/tests/shared/pattern.test.ts index 65d8429..336eaa9 100644 --- a/tests/shared/pattern.test.ts +++ b/tests/shared/pattern.test.ts @@ -20,6 +20,14 @@ describe('pattern validation', () => { } }); + test('builtin patterns require tool-call approval by default', () => { + const patterns = createBuiltinPatterns(BUILTIN_TIMESTAMP); + + for (const pattern of patterns) { + expect(pattern.approvalPolicy?.rules).toContainEqual({ kind: 'tool-call' }); + } + }); + test('magentic pattern is marked unavailable', () => { const magentic = createBuiltinPatterns(BUILTIN_TIMESTAMP).find( (pattern) => pattern.mode === 'magentic', diff --git a/tests/shared/workspace.test.ts b/tests/shared/workspace.test.ts index 0136c22..bcc036a 100644 --- a/tests/shared/workspace.test.ts +++ b/tests/shared/workspace.test.ts @@ -31,6 +31,7 @@ describe('workspace seed', () => { for (const pattern of workspace.patterns) { expect(pattern.createdAt).toBe(workspace.lastUpdatedAt); expect(pattern.updatedAt).toBe(workspace.lastUpdatedAt); + expect(pattern.approvalPolicy?.rules).toContainEqual({ kind: 'tool-call' }); } const magentic = workspace.patterns.find((pattern) => pattern.mode === 'magentic');