From c91f1a47428ba1c8fb5b1e312cb3dd85c0958e89 Mon Sep 17 00:00:00 2001 From: George Ng Date: Sun, 20 Sep 2026 23:23:43 -0700 Subject: [PATCH 1/3] Declare audited read-only policies for core actions Cover eight actions across five agents with production-manifest and structured-service regression tests and an action rationale table. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../architecture/read-only-action-policies.md | 78 +++ .../github-cli/src/github-cliManifest.json | 3 + ts/packages/agents/list/src/listManifest.json | 4 + .../player/src/agent/playerManifest.json | 4 + .../agents/timer/src/timerManifest.json | 3 + .../agents/weather/src/weatherManifest.json | 4 + ts/packages/defaultAgentProvider/package.json | 1 + .../test/readOnlyActionPolicies.spec.ts | 483 ++++++++++++++++++ ts/pnpm-lock.yaml | 3 + 9 files changed, 583 insertions(+) create mode 100644 docs/architecture/read-only-action-policies.md create mode 100644 ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts diff --git a/docs/architecture/read-only-action-policies.md b/docs/architecture/read-only-action-policies.md new file mode 100644 index 0000000000..fb5d82b60e --- /dev/null +++ b/docs/architecture/read-only-action-policies.md @@ -0,0 +1,78 @@ +# Read-only structured actions + +This first pass covers eight exact actions across five built-in agents. +Their schema manifests declare `effects: "read-only"` without an explicit +confirmation requirement, so structured discovery reports +`confirmation: "not-required"` and execution skips only the dispatcher's +outer effect-confirmation prompt. Natural-language routing is unchanged. + +| Agent | Action (exact schemaName.actionName) | Why no confirmation | +| ---------- | ------------------------------------ | ----------------------------------------------------------------------------------------------------------- | +| List | `list.listLists` | It reads the names of existing lists without changing their contents. | +| List | `list.getList` | It reads one named list without adding, removing, or saving items. | +| Weather | `weather.getCurrentConditions` | It queries fixed Open-Meteo endpoints for a supplied place without changing settings. | +| Weather | `weather.getForecast` | It reads a one-to-seven-day forecast from fixed Open-Meteo endpoints. | +| Player | `player.listDevices` | It reads Spotify device and playback metadata without selecting a device or controlling playback. | +| Player | `player.showSelectedDevice` | It reports the selected or default Spotify device without changing that selection. | +| GitHub CLI | `github-cli.prFiles` | It reads bounded pull-request file details using fixed GitHub CLI commands without changing the repository. | +| Timer | `timer.listReminders` | It reads pending reminders without scheduling, cancelling, or firing them. | + +## Boundaries + +Validation, enabled-agent checks, readiness, authorization, live policy +rechecks, and handler-originated questions still apply. An explicit +`confirmation: "required"` overrides a read-only declaration. Every omitted +action remains unclassified and requires outer confirmation, including list +edits, timer changes, player controls, GitHub mutations, and generic commands. +This is not a new permission system or an exemption for every action whose +name sounds like a read. + +Read-only describes the action's domain effects, not a guarantee of zero +infrastructure activity: normal authorized agent startup, token refresh, +read caches, result/history bookkeeping, and existing timer background tasks +are unchanged. List storage is initialized when the agent is enabled, not by +these read actions. Tests use an isolated instance and preseed disposable data. +External reads return data to the requesting client under existing service +authorization; these policies do not authorize arbitrary uploads or execution. +Weather sends the supplied place to Open-Meteo. `prFiles` uses the existing +GitHub CLI account/host selection and fixed PR metadata/files GET operations; +optional patch excerpts remain bounded. A bare repository name can still +require the handler's repository-choice prompt. + +Narrow exclusions from this pass: + +- Utility: unrestricted file paths, browser navigation, and model/tool execution + need a separate boundary audit rather than blanket read exemptions. +- Calendar: the Microsoft date-range adapter currently passes an object to a + client that interpolates it into a query string and retains the failing + pagination link on error; defer calendar scenarios until that path is fixed. +- GitHub CLI: `prFailedChecks` derives annotation hosts from check links; + `authStatus` can expose tokens, and generic API/CLI actions are not covered. +- Weather: `getAlerts` currently returns placeholder data, so it is not a useful + real-service scenario. Player searches/playlists and all playback/UI changes + are outside this device-inventory-only pass. + +## Safe manual scenarios + +Use an isolated instance with existing lists and already-configured services. +Do not approve setup, mutation, or handler questions unattended. + +| Scenario | Structured action and parameters | +| -------------------------------- | ----------------------------------------------------------------------------------- | +| Inventory existing lists | `list.listLists {}` | +| Read two disposable lists | `list.getList {"listName":"groceries"}`, then `{"listName":"packing"}` | +| Read weather in two places | `weather.getCurrentConditions {"location":"Seattle"}`, then `{"location":"Boston"}` | +| Read a short forecast | `weather.getForecast {"location":"Seattle","days":3}` | +| Inspect Spotify devices | `player.listDevices`, then `player.showSelectedDevice` (omit parameters) | +| Inspect PR files without patches | `github-cli.prFiles {"repo":"microsoft/TypeAgent","number":2991,"maxFiles":5}` | +| Inventory existing reminders | `timer.listReminders {}` | + +For timing, compare discovery plus the first completed structured action, +a repeat using its known identity/contract, and similar natural-language +wording with different parameters. Record completed outcomes and separate +connection, model, discovery, and execution time. Removing a confirmation +blocker does not establish an end-to-end speedup. + +`ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts` checks +this rationale table against the selected production manifests and exercises +their contracts through the real structured dispatcher. diff --git a/ts/packages/agents/github-cli/src/github-cliManifest.json b/ts/packages/agents/github-cli/src/github-cliManifest.json index 4aca11097e..0e4eb47072 100644 --- a/ts/packages/agents/github-cli/src/github-cliManifest.json +++ b/ts/packages/agents/github-cli/src/github-cliManifest.json @@ -7,6 +7,9 @@ "originalSchemaFile": "./github-cliSchema.ts", "schemaFile": "../dist/github-cliSchema.pas.json", "grammarFile": "../dist/github-cliSchema.ag.json", + "actionPolicies": { + "prFiles": { "effects": "read-only" } + }, "schemaType": "GithubCliActions" } } diff --git a/ts/packages/agents/list/src/listManifest.json b/ts/packages/agents/list/src/listManifest.json index 0b7495e0c4..bee6279bbf 100644 --- a/ts/packages/agents/list/src/listManifest.json +++ b/ts/packages/agents/list/src/listManifest.json @@ -6,6 +6,10 @@ "originalSchemaFile": "./listSchema.ts", "schemaFile": "../dist/listSchema.pas.json", "grammarFile": "../dist/listSchema.ag.json", + "actionPolicies": { + "listLists": { "effects": "read-only" }, + "getList": { "effects": "read-only" } + }, "schemaType": { "action": "ListAction", "activity": "ListActivity" diff --git a/ts/packages/agents/player/src/agent/playerManifest.json b/ts/packages/agents/player/src/agent/playerManifest.json index 7f1b131f70..d97fad9f82 100644 --- a/ts/packages/agents/player/src/agent/playerManifest.json +++ b/ts/packages/agents/player/src/agent/playerManifest.json @@ -6,6 +6,10 @@ "originalSchemaFile": "./playerSchema.ts", "schemaFile": "../../dist/agent/playerSchema.pas.json", "grammarFile": "../../dist/agent/playerSchema.ag.json", + "actionPolicies": { + "listDevices": { "effects": "read-only" }, + "showSelectedDevice": { "effects": "read-only" } + }, "schemaType": { "action": "PlayerActions", "entity": "PlayerEntities" diff --git a/ts/packages/agents/timer/src/timerManifest.json b/ts/packages/agents/timer/src/timerManifest.json index 98f0e2f907..b77b2b1b98 100644 --- a/ts/packages/agents/timer/src/timerManifest.json +++ b/ts/packages/agents/timer/src/timerManifest.json @@ -6,6 +6,9 @@ "originalSchemaFile": "./timerSchema.ts", "schemaFile": "../dist/timerSchema.pas.json", "grammarFile": "../dist/timerSchema.ag.json", + "actionPolicies": { + "listReminders": { "effects": "read-only" } + }, "schemaType": { "action": "TimerAction" } diff --git a/ts/packages/agents/weather/src/weatherManifest.json b/ts/packages/agents/weather/src/weatherManifest.json index faf7ab5fa5..bc923f4e00 100644 --- a/ts/packages/agents/weather/src/weatherManifest.json +++ b/ts/packages/agents/weather/src/weatherManifest.json @@ -6,6 +6,10 @@ "originalSchemaFile": "weatherSchema.ts", "schemaFile": "../dist/weatherSchema.pas.json", "grammarFile": "../dist/weatherSchema.ag.json", + "actionPolicies": { + "getCurrentConditions": { "effects": "read-only" }, + "getForecast": { "effects": "read-only" } + }, "schemaType": { "action": "WeatherAction" } diff --git a/ts/packages/defaultAgentProvider/package.json b/ts/packages/defaultAgentProvider/package.json index b067ab005a..a09858ce32 100644 --- a/ts/packages/defaultAgentProvider/package.json +++ b/ts/packages/defaultAgentProvider/package.json @@ -98,6 +98,7 @@ "zod": "^4.1.13" }, "devDependencies": { + "@jest/globals": "^29.7.0", "@types/debug": "^4.1.12", "@types/file-size": "^1.0.3", "@types/jest": "^29.5.7", diff --git a/ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts b/ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts new file mode 100644 index 0000000000..5518584811 --- /dev/null +++ b/ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts @@ -0,0 +1,483 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { jest } from "@jest/globals"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import type { + AppAgent, + AppAgentManifest, + ReadinessReport, + Storage, +} from "@typeagent/agent-sdk"; +import type { + AppAgentProvider, + Dispatcher, + ExecuteActionRequest, + StructuredActionExecutionResult, +} from "agent-dispatcher"; +import { + closeCommandHandlerContext, + createDispatcherFromContext, + initializeCommandHandlerContext, + type CommandHandlerContext, +} from "agent-dispatcher/internal"; +import { getFsStorageProvider } from "dispatcher-node-providers"; +import { instantiate as instantiateList } from "@typeagent/list-agent/agent/handlers"; +import { instantiate as instantiateTimer } from "@typeagent/timer-agent/agent/handlers"; +import { instantiate as instantiateWeather } from "@typeagent/weather-agent/agent/handlers"; +import { getDefaultAppAgentProviders } from "../src/defaultAgentProviders.js"; + +const allowed = [ + ["list", "listLists"], + ["list", "getList"], + ["weather", "getCurrentConditions"], + ["weather", "getForecast"], + ["player", "listDevices"], + ["player", "showSelectedDevice"], + ["github-cli", "prFiles"], + ["timer", "listReminders"], +] as const; +const agentNames = [...new Set(allowed.map(([name]) => name))]; +const seed = JSON.stringify([ + { name: "groceries", items: ["milk", "eggs"] }, + { name: "packing", items: ["coat"] }, + { name: "empty", items: [] }, +]); + +function requirePrompt(result: StructuredActionExecutionResult) { + if (result.status !== "requires_interaction") { + throw new Error(`Expected interaction, got ${result.status}`); + } + return result; +} + +describe("built-in read-only action policies", () => { + let context: CommandHandlerContext; + let dispatcher: Dispatcher; + let directory: string; + let storage: Storage; + let manifests: Map; + let executionAllowed: boolean; + let readiness: ReadinessReport; + let listAgent: AppAgent; + const scope = {}; + const externalExecute = jest.fn>(); + const setup = jest.fn>(); + + beforeEach(async () => { + directory = await fs.mkdtemp(path.join(os.tmpdir(), "ta-read-policy-")); + storage = getFsStorageProvider().getStorage("list", directory); + await storage.write("lists.json", seed); + jest.spyOn(storage, "write"); + executionAllowed = true; + readiness = { state: "ready" }; + externalExecute.mockReset().mockImplementation(async () => { + throw new Error( + "External handler must not execute in this fixture", + ); + }); + setup.mockReset(); + const bundled = getDefaultAppAgentProviders(undefined)[0]; + manifests = new Map( + await Promise.all( + agentNames.map( + async (name) => + [ + name, + await bundled.getAppAgentManifest(name), + ] as const, + ), + ), + ); + listAgent = instantiateList(); + const provider: AppAgentProvider = { + getAppAgentNames: () => agentNames, + getAppAgentManifest: async (name) => { + const manifest = manifests.get(name); + if (!manifest) throw new Error(`Missing manifest: ${name}`); + return manifest; + }, + loadAppAgent: async (name) => { + if (name === "list") { + return { + ...listAgent, + executeAction: (action, actionContext) => + listAgent.executeAction!(action, actionContext), + checkReadiness: async () => readiness, + setup, + updateAgentContext: (enable, session, schemaName) => + listAgent.updateAgentContext!( + enable, + { ...session, sessionStorage: storage }, + schemaName, + ), + }; + } + if (name === "timer") return instantiateTimer(); + if (name === "weather") return instantiateWeather(); + // Only discovery and the pre-handler confirmation gate are + // exercised for Spotify/GitHub here, never auth or live I/O. + return { executeAction: externalExecute }; + }, + unloadAppAgent: async () => {}, + }; + context = await initializeCommandHandlerContext("read-policy-test", { + appAgentProviders: [provider], + agents: agentNames, + translation: { enabled: false }, + explainer: { enabled: false }, + cache: { enabled: false }, + dblogging: false, + conversationMemorySettings: { + requestKnowledgeExtraction: false, + actionResultEntityStorage: false, + actionResultKnowledgeExtraction: false, + }, + }); + dispatcher = createDispatcherFromContext( + context, + "policy-test", + undefined, + () => ({ + scope, + canDiscoverSchema: () => true, + canExecute: executionAllowed, + isActive: () => true, + }), + ); + }); + + afterEach(async () => { + try { + await closeCommandHandlerContext(context); + } finally { + jest.restoreAllMocks(); + await fs.rm(directory, { recursive: true, force: true }); + } + }); + + async function request( + schemaName: string, + actionName: string, + parameters: Record = {}, + ): Promise { + const search = await dispatcher.searchActions({ + query: `${schemaName} ${actionName}`, + }); + expect(search.actions).toEqual([ + expect.objectContaining({ schemaName, actionName }), + ]); + return { + protocolVersion: 1, + scopeId: search.scopeId, + schemaName, + actionName, + parameters, + }; + } + + async function cancel( + result: StructuredActionExecutionResult, + status: "cancelled" | "execution_uncertain" = "cancelled", + ) { + const prompt = requirePrompt(result); + expect( + ( + await dispatcher.cancelAction({ + protocolVersion: 1, + scopeId: prompt.scopeId, + operationId: prompt.operationId, + interactionId: prompt.interactionId, + }) + ).status, + ).toBe(status); + } + + it("keeps the rationale table synchronized with exact production declarations", async () => { + const declared = []; + for (const [name, manifest] of manifests) { + for (const [actionName, policy] of Object.entries( + manifest.schema?.actionPolicies ?? {}, + )) { + expect(policy).toEqual({ effects: "read-only" }); + const config = context.agents.getActionConfig(name); + const schema = + context.agents.getActionSchemaFileForConfig(config); + expect( + schema.parsedActionSchema.actionSchemas.has(actionName), + ).toBe(true); + declared.push(`${name}.${actionName}`); + } + } + expect(declared.sort()).toEqual( + allowed.map(([name, action]) => `${name}.${action}`).sort(), + ); + const document = await fs.readFile( + new URL( + "../../../../../docs/architecture/read-only-action-policies.md", + import.meta.url, + ), + "utf8", + ); + const rows = [ + ...document.matchAll( + /^\|[^|]+\|[ \t]+`([^`]+)`[ \t]+\|([^|]+)\|$/gm, + ), + ]; + expect(rows.map((row) => row[1]).sort()).toEqual(declared); + for (const row of rows) expect(row[2].trim()).toMatch(/\.$/); + }); + + it.each(allowed)( + "discovers %s.%s as read-only without outer confirmation", + async (schemaName, actionName) => { + const result = await dispatcher.searchActions({ + query: `${schemaName} ${actionName}`, + }); + expect(result.actions).toEqual([ + expect.objectContaining({ + schemaName, + actionName, + policy: { + effects: "read-only", + confirmation: "not-required", + }, + interactions: expect.objectContaining({ + mode: "may-require-interaction", + }), + }), + ]); + }, + ); + + it("leaves every unclassified sibling requiring confirmation", async () => { + for (const name of agentNames) { + const config = context.agents.getActionConfig(name); + const schema = context.agents.getActionSchemaFileForConfig(config); + for (const actionName of schema.parsedActionSchema.actionSchemas.keys()) { + if (config.actionPolicies?.[actionName]) continue; + const result = await dispatcher.searchActions({ + query: `${name} ${actionName}`, + }); + expect(result.actions[0]?.policy).toEqual({ + effects: "unknown", + confirmation: "required", + }); + } + } + }); + + it("executes real list reads and reuses the contract with different parameters without writes", async () => { + const inventory = await dispatcher.executeAction( + await request("list", "listLists"), + ); + expect(inventory.status).toBe("completed"); + expect(inventory.results[0].result).toMatchObject({ + entities: [ + { name: "groceries" }, + { name: "packing" }, + { name: "empty" }, + ], + }); + const input = await request("list", "getList", { + listName: "groceries", + }); + for (const [listName, items] of [ + ["groceries", ["milk", "eggs"]], + ["packing", ["coat"]], + ["empty", []], + ] as const) { + const result = await dispatcher.executeAction({ + ...input, + parameters: { listName }, + }); + expect(result.status).toBe("completed"); + expect(result.results[0].result).toMatchObject({ + displayContent: { rawData: { name: listName, items } }, + }); + } + const missing = await dispatcher.executeAction({ + ...input, + parameters: { listName: "missing" }, + }); + expect(missing.status).toBe("failed"); + expect(await storage.read("lists.json", "utf8")).toBe(seed); + expect(storage.write).not.toHaveBeenCalled(); + }); + + it("executes real weather handlers using only mocked fixed-endpoint reads", async () => { + const fetch = jest + .spyOn(globalThis, "fetch") + .mockImplementation(async (input, init) => { + expect(init?.method ?? "GET").toBe("GET"); + const url = new URL(String(input)); + if (url.hostname === "geocoding-api.open-meteo.com") { + expect(url.pathname).toBe("/v1/search"); + return Response.json({ + results: [ + { + latitude: 47.6, + longitude: -122.3, + name: "Seattle", + }, + ], + }); + } + expect(url.origin + url.pathname).toBe( + "https://api.open-meteo.com/v1/forecast", + ); + return Response.json({ + current: { + temperature_2m: 55, + apparent_temperature: 54, + weather_code: 0, + relative_humidity_2m: 50, + wind_speed_10m: 3, + wind_direction_10m: 90, + }, + daily: { + time: ["2026-09-20"], + temperature_2m_max: [60], + temperature_2m_min: [45], + weather_code: [0], + precipitation_probability_max: [10], + }, + }); + }); + for (const actionName of ["getCurrentConditions", "getForecast"]) { + const result = await dispatcher.executeAction( + await request("weather", actionName, { + location: "Seattle", + ...(actionName === "getForecast" ? { days: 1 } : {}), + }), + ); + expect(result.status).toBe("completed"); + expect(result.results[0].result).toMatchObject({ + displayContent: { rawData: { location: "Seattle" } }, + }); + } + expect(fetch).toHaveBeenCalledTimes(4); + }); + + it("executes the real empty reminder inventory without scheduling anything", async () => { + const result = await dispatcher.executeAction( + await request("timer", "listReminders"), + ); + expect(result.status).toBe("completed"); + expect(result.results[0].result).toMatchObject({ + displayContent: "No pending reminders.", + }); + }); + + it.each([ + ["list", "addItems", { listName: "groceries", items: ["bread"] }], + ["player", "setVolume", { newVolumeLevel: 50 }], + [ + "github-cli", + "prFailedChecks", + { repo: "microsoft/TypeAgent", number: 2991 }, + ], + ["timer", "cancelReminder", { id: "all" }], + ["weather", "getAlerts", { location: "Seattle" }], + ] as const)( + "still confirms %s.%s before entering its handler", + async (schemaName, actionName, parameters) => { + const result = await dispatcher.executeAction( + await request(schemaName, actionName, parameters), + ); + expect(requirePrompt(result).prompt.type).toBe("confirmation"); + await cancel(result); + expect(externalExecute).not.toHaveBeenCalled(); + expect(storage.write).not.toHaveBeenCalled(); + }, + ); + + it("rechecks an explicit required override after discovery", async () => { + const input = await request("list", "getList", { + listName: "groceries", + }); + context.agents.getActionConfig("list").actionPolicies = { + getList: { effects: "read-only", confirmation: "required" }, + }; + const result = await dispatcher.executeAction(input); + expect(requirePrompt(result).prompt.type).toBe("confirmation"); + await cancel(result); + expect(storage.write).not.toHaveBeenCalled(); + }); + + it("preserves validation, authorization, and readiness for a real read action", async () => { + const input = await request("list", "getList", { + listName: "groceries", + }); + const execute = jest.spyOn(listAgent, "executeAction"); + expect( + await dispatcher.executeAction({ + ...input, + parameters: { listName: 42 }, + }), + ).toMatchObject({ + status: "failed", + error: { + code: "execution_failed", + message: expect.stringContaining("listName"), + }, + }); + executionAllowed = false; + expect((await dispatcher.executeAction(input)).status).toBe( + "unavailable", + ); + executionAllowed = true; + readiness = { + state: "setup-required", + message: "Offline prerequisite unavailable", + }; + await context.agents.refreshReadiness("list"); + expect((await dispatcher.executeAction(input)).status).toBe( + "unavailable", + ); + expect(execute).not.toHaveBeenCalled(); + expect(setup).not.toHaveBeenCalled(); + }); + + it("rechecks availability when list actions are disabled after discovery", async () => { + const input = await request("list", "getList", { + listName: "groceries", + }); + const execute = jest.spyOn(listAgent, "executeAction"); + const settings = context.session.getConfig(); + await context.agents.setState(context, { + ...settings, + actions: { ...settings.actions, list: false }, + }); + expect((await dispatcher.executeAction(input)).status).toBe( + "unavailable", + ); + expect(execute).not.toHaveBeenCalled(); + expect(storage.write).not.toHaveBeenCalled(); + }); + + it("preserves the real list handler's separate deletion question", async () => { + const outer = requirePrompt( + await dispatcher.executeAction( + await request("list", "deleteList", { listName: "groceries" }), + ), + ); + expect(outer.prompt.type).toBe("confirmation"); + const inner = await dispatcher.continueAction({ + protocolVersion: 1, + scopeId: outer.scopeId, + operationId: outer.operationId, + interactionId: outer.interactionId, + response: { type: "confirmation", approved: true }, + }); + expect(requirePrompt(inner).prompt).toMatchObject({ + type: "yesNo", + message: "Delete list 'groceries'? This cannot be undone.", + }); + await cancel(inner, "execution_uncertain"); + expect(await storage.read("lists.json", "utf8")).toBe(seed); + expect(storage.write).not.toHaveBeenCalled(); + }); +}); diff --git a/ts/pnpm-lock.yaml b/ts/pnpm-lock.yaml index f06fc047b3..72ad44f0cc 100644 --- a/ts/pnpm-lock.yaml +++ b/ts/pnpm-lock.yaml @@ -4892,6 +4892,9 @@ importers: specifier: ^4.1.13 version: 4.1.13 devDependencies: + '@jest/globals': + specifier: ^29.7.0 + version: 29.7.0 '@types/debug': specifier: ^4.1.12 version: 4.1.12 From 95097742c3206c217d8e5bdf08092e25114400b0 Mon Sep 17 00:00:00 2001 From: George Ng Date: Sun, 20 Sep 2026 23:27:23 -0700 Subject: [PATCH 2/3] Exclude interactive Spotify auth from read-only exemptions Use fixed Windows ipconfig help and configuration reads instead; preserve the eight-action rationale table and production contract coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../architecture/read-only-action-policies.md | 49 ++++++++++--------- .../agents/ipconfig/src/ipconfigManifest.json | 4 ++ .../player/src/agent/playerManifest.json | 4 -- .../test/readOnlyActionPolicies.spec.ts | 8 +-- 4 files changed, 35 insertions(+), 30 deletions(-) diff --git a/docs/architecture/read-only-action-policies.md b/docs/architecture/read-only-action-policies.md index fb5d82b60e..8496a8bd04 100644 --- a/docs/architecture/read-only-action-policies.md +++ b/docs/architecture/read-only-action-policies.md @@ -6,16 +6,16 @@ confirmation requirement, so structured discovery reports `confirmation: "not-required"` and execution skips only the dispatcher's outer effect-confirmation prompt. Natural-language routing is unchanged. -| Agent | Action (exact schemaName.actionName) | Why no confirmation | -| ---------- | ------------------------------------ | ----------------------------------------------------------------------------------------------------------- | -| List | `list.listLists` | It reads the names of existing lists without changing their contents. | -| List | `list.getList` | It reads one named list without adding, removing, or saving items. | -| Weather | `weather.getCurrentConditions` | It queries fixed Open-Meteo endpoints for a supplied place without changing settings. | -| Weather | `weather.getForecast` | It reads a one-to-seven-day forecast from fixed Open-Meteo endpoints. | -| Player | `player.listDevices` | It reads Spotify device and playback metadata without selecting a device or controlling playback. | -| Player | `player.showSelectedDevice` | It reports the selected or default Spotify device without changing that selection. | -| GitHub CLI | `github-cli.prFiles` | It reads bounded pull-request file details using fixed GitHub CLI commands without changing the repository. | -| Timer | `timer.listReminders` | It reads pending reminders without scheduling, cancelling, or firing them. | +| Agent | Action (exact schemaName.actionName) | Why no confirmation | +| ---------- | ---------------------------------------------- | ----------------------------------------------------------------------------------------------------------- | +| List | `list.listLists` | It reads the names of existing lists without changing their contents. | +| List | `list.getList` | It reads one named list without adding, removing, or saving items. | +| Weather | `weather.getCurrentConditions` | It queries fixed Open-Meteo endpoints for a supplied place without changing settings. | +| Weather | `weather.getForecast` | It reads a one-to-seven-day forecast from fixed Open-Meteo endpoints. | +| IP config | `ipconfig.displayHelpMessage` | It runs the fixed ipconfig help command without accepting command arguments or changing network settings. | +| IP config | `ipconfig.displayFullConfigurationInformation` | It reads local network configuration using a fixed ipconfig command without changing adapters or DNS. | +| GitHub CLI | `github-cli.prFiles` | It reads bounded pull-request file details using fixed GitHub CLI commands without changing the repository. | +| Timer | `timer.listReminders` | It reads pending reminders without scheduling, cancelling, or firing them. | ## Boundaries @@ -23,7 +23,7 @@ Validation, enabled-agent checks, readiness, authorization, live policy rechecks, and handler-originated questions still apply. An explicit `confirmation: "required"` overrides a read-only declaration. Every omitted action remains unclassified and requires outer confirmation, including list -edits, timer changes, player controls, GitHub mutations, and generic commands. +edits, timer changes, network changes, player controls, GitHub mutations, and generic commands. This is not a new permission system or an exemption for every action whose name sounds like a read. @@ -38,6 +38,9 @@ Weather sends the supplied place to Open-Meteo. `prFiles` uses the existing GitHub CLI account/host selection and fixed PR metadata/files GET operations; optional patch excerpts remain bounded. A bare repository name can still require the handler's repository-choice prompt. +The ipconfig actions run only the fixed Windows commands `ipconfig /?` and +`ipconfig /all`; network configuration details stay in the response to the +requesting client, not a new external destination. Narrow exclusions from this pass: @@ -48,24 +51,26 @@ Narrow exclusions from this pass: pagination link on error; defer calendar scenarios until that path is fixed. - GitHub CLI: `prFailedChecks` derives annotation hosts from check links; `authStatus` can expose tokens, and generic API/CLI actions are not covered. +- Player: even device reads delegate to `getAccessToken()` with interactive + fallback when refresh credentials are missing or rejected; no player action + is exempted until that auth path can be guaranteed non-interactive. - Weather: `getAlerts` currently returns placeholder data, so it is not a useful - real-service scenario. Player searches/playlists and all playback/UI changes - are outside this device-inventory-only pass. + real-service scenario. ## Safe manual scenarios Use an isolated instance with existing lists and already-configured services. Do not approve setup, mutation, or handler questions unattended. -| Scenario | Structured action and parameters | -| -------------------------------- | ----------------------------------------------------------------------------------- | -| Inventory existing lists | `list.listLists {}` | -| Read two disposable lists | `list.getList {"listName":"groceries"}`, then `{"listName":"packing"}` | -| Read weather in two places | `weather.getCurrentConditions {"location":"Seattle"}`, then `{"location":"Boston"}` | -| Read a short forecast | `weather.getForecast {"location":"Seattle","days":3}` | -| Inspect Spotify devices | `player.listDevices`, then `player.showSelectedDevice` (omit parameters) | -| Inspect PR files without patches | `github-cli.prFiles {"repo":"microsoft/TypeAgent","number":2991,"maxFiles":5}` | -| Inventory existing reminders | `timer.listReminders {}` | +| Scenario | Structured action and parameters | +| ------------------------------------- | ---------------------------------------------------------------------------------------- | +| Inventory existing lists | `list.listLists {}` | +| Read two disposable lists | `list.getList {"listName":"groceries"}`, then `{"listName":"packing"}` | +| Read weather in two places | `weather.getCurrentConditions {"location":"Seattle"}`, then `{"location":"Boston"}` | +| Read a short forecast | `weather.getForecast {"location":"Seattle","days":3}` | +| Inspect Windows network configuration | `ipconfig.displayHelpMessage {}`, then `ipconfig.displayFullConfigurationInformation {}` | +| Inspect PR files without patches | `github-cli.prFiles {"repo":"microsoft/TypeAgent","number":2991,"maxFiles":5}` | +| Inventory existing reminders | `timer.listReminders {}` | For timing, compare discovery plus the first completed structured action, a repeat using its known identity/contract, and similar natural-language diff --git a/ts/packages/agents/ipconfig/src/ipconfigManifest.json b/ts/packages/agents/ipconfig/src/ipconfigManifest.json index dd84c5ca5f..a0636f6a1f 100644 --- a/ts/packages/agents/ipconfig/src/ipconfigManifest.json +++ b/ts/packages/agents/ipconfig/src/ipconfigManifest.json @@ -7,6 +7,10 @@ "originalSchemaFile": "./ipconfigSchema.ts", "schemaFile": "../dist/ipconfigSchema.pas.json", "grammarFile": "../dist/ipconfigSchema.ag.json", + "actionPolicies": { + "displayHelpMessage": { "effects": "read-only" }, + "displayFullConfigurationInformation": { "effects": "read-only" } + }, "schemaType": "IpconfigActions" } } diff --git a/ts/packages/agents/player/src/agent/playerManifest.json b/ts/packages/agents/player/src/agent/playerManifest.json index d97fad9f82..7f1b131f70 100644 --- a/ts/packages/agents/player/src/agent/playerManifest.json +++ b/ts/packages/agents/player/src/agent/playerManifest.json @@ -6,10 +6,6 @@ "originalSchemaFile": "./playerSchema.ts", "schemaFile": "../../dist/agent/playerSchema.pas.json", "grammarFile": "../../dist/agent/playerSchema.ag.json", - "actionPolicies": { - "listDevices": { "effects": "read-only" }, - "showSelectedDevice": { "effects": "read-only" } - }, "schemaType": { "action": "PlayerActions", "entity": "PlayerEntities" diff --git a/ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts b/ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts index 5518584811..5353b7a3bb 100644 --- a/ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts +++ b/ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts @@ -34,8 +34,8 @@ const allowed = [ ["list", "getList"], ["weather", "getCurrentConditions"], ["weather", "getForecast"], - ["player", "listDevices"], - ["player", "showSelectedDevice"], + ["ipconfig", "displayHelpMessage"], + ["ipconfig", "displayFullConfigurationInformation"], ["github-cli", "prFiles"], ["timer", "listReminders"], ] as const; @@ -118,7 +118,7 @@ describe("built-in read-only action policies", () => { if (name === "timer") return instantiateTimer(); if (name === "weather") return instantiateWeather(); // Only discovery and the pre-handler confirmation gate are - // exercised for Spotify/GitHub here, never auth or live I/O. + // exercised for ipconfig/GitHub here, never live CLI I/O. return { executeAction: externalExecute }; }, unloadAppAgent: async () => {}, @@ -373,7 +373,7 @@ describe("built-in read-only action policies", () => { it.each([ ["list", "addItems", { listName: "groceries", items: ["bread"] }], - ["player", "setVolume", { newVolumeLevel: 50 }], + ["ipconfig", "purgeDNSResolverCache", {}], [ "github-cli", "prFailedChecks", From 54a306dfb09afb05b4ca2533ed74f80bd629e0cc Mon Sep 17 00:00:00 2001 From: George Ng Date: Mon, 21 Sep 2026 00:28:43 -0700 Subject: [PATCH 3/3] Expand audited read-only policies to 25 actions Keep the rationale table in local review documents, exercise real handlers with isolated storage and mocked transport, and treat bare GitHub repository queries as literal arguments. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../architecture/read-only-action-policies.md | 83 --- .../github-cli/src/github-cliActionHandler.ts | 3 +- .../github-cli/src/github-cliManifest.json | 13 +- .../agents/ipconfig/src/ipconfigManifest.json | 3 +- .../src/agent/localPlayerManifest.json | 5 + .../agents/powershell/src/manifest.json | 5 +- ts/packages/agents/taskflow/manifest.json | 5 +- .../test/readOnlyActionPolicies.spec.ts | 495 +++++++++++++++++- 8 files changed, 499 insertions(+), 113 deletions(-) delete mode 100644 docs/architecture/read-only-action-policies.md diff --git a/docs/architecture/read-only-action-policies.md b/docs/architecture/read-only-action-policies.md deleted file mode 100644 index 8496a8bd04..0000000000 --- a/docs/architecture/read-only-action-policies.md +++ /dev/null @@ -1,83 +0,0 @@ -# Read-only structured actions - -This first pass covers eight exact actions across five built-in agents. -Their schema manifests declare `effects: "read-only"` without an explicit -confirmation requirement, so structured discovery reports -`confirmation: "not-required"` and execution skips only the dispatcher's -outer effect-confirmation prompt. Natural-language routing is unchanged. - -| Agent | Action (exact schemaName.actionName) | Why no confirmation | -| ---------- | ---------------------------------------------- | ----------------------------------------------------------------------------------------------------------- | -| List | `list.listLists` | It reads the names of existing lists without changing their contents. | -| List | `list.getList` | It reads one named list without adding, removing, or saving items. | -| Weather | `weather.getCurrentConditions` | It queries fixed Open-Meteo endpoints for a supplied place without changing settings. | -| Weather | `weather.getForecast` | It reads a one-to-seven-day forecast from fixed Open-Meteo endpoints. | -| IP config | `ipconfig.displayHelpMessage` | It runs the fixed ipconfig help command without accepting command arguments or changing network settings. | -| IP config | `ipconfig.displayFullConfigurationInformation` | It reads local network configuration using a fixed ipconfig command without changing adapters or DNS. | -| GitHub CLI | `github-cli.prFiles` | It reads bounded pull-request file details using fixed GitHub CLI commands without changing the repository. | -| Timer | `timer.listReminders` | It reads pending reminders without scheduling, cancelling, or firing them. | - -## Boundaries - -Validation, enabled-agent checks, readiness, authorization, live policy -rechecks, and handler-originated questions still apply. An explicit -`confirmation: "required"` overrides a read-only declaration. Every omitted -action remains unclassified and requires outer confirmation, including list -edits, timer changes, network changes, player controls, GitHub mutations, and generic commands. -This is not a new permission system or an exemption for every action whose -name sounds like a read. - -Read-only describes the action's domain effects, not a guarantee of zero -infrastructure activity: normal authorized agent startup, token refresh, -read caches, result/history bookkeeping, and existing timer background tasks -are unchanged. List storage is initialized when the agent is enabled, not by -these read actions. Tests use an isolated instance and preseed disposable data. -External reads return data to the requesting client under existing service -authorization; these policies do not authorize arbitrary uploads or execution. -Weather sends the supplied place to Open-Meteo. `prFiles` uses the existing -GitHub CLI account/host selection and fixed PR metadata/files GET operations; -optional patch excerpts remain bounded. A bare repository name can still -require the handler's repository-choice prompt. -The ipconfig actions run only the fixed Windows commands `ipconfig /?` and -`ipconfig /all`; network configuration details stay in the response to the -requesting client, not a new external destination. - -Narrow exclusions from this pass: - -- Utility: unrestricted file paths, browser navigation, and model/tool execution - need a separate boundary audit rather than blanket read exemptions. -- Calendar: the Microsoft date-range adapter currently passes an object to a - client that interpolates it into a query string and retains the failing - pagination link on error; defer calendar scenarios until that path is fixed. -- GitHub CLI: `prFailedChecks` derives annotation hosts from check links; - `authStatus` can expose tokens, and generic API/CLI actions are not covered. -- Player: even device reads delegate to `getAccessToken()` with interactive - fallback when refresh credentials are missing or rejected; no player action - is exempted until that auth path can be guaranteed non-interactive. -- Weather: `getAlerts` currently returns placeholder data, so it is not a useful - real-service scenario. - -## Safe manual scenarios - -Use an isolated instance with existing lists and already-configured services. -Do not approve setup, mutation, or handler questions unattended. - -| Scenario | Structured action and parameters | -| ------------------------------------- | ---------------------------------------------------------------------------------------- | -| Inventory existing lists | `list.listLists {}` | -| Read two disposable lists | `list.getList {"listName":"groceries"}`, then `{"listName":"packing"}` | -| Read weather in two places | `weather.getCurrentConditions {"location":"Seattle"}`, then `{"location":"Boston"}` | -| Read a short forecast | `weather.getForecast {"location":"Seattle","days":3}` | -| Inspect Windows network configuration | `ipconfig.displayHelpMessage {}`, then `ipconfig.displayFullConfigurationInformation {}` | -| Inspect PR files without patches | `github-cli.prFiles {"repo":"microsoft/TypeAgent","number":2991,"maxFiles":5}` | -| Inventory existing reminders | `timer.listReminders {}` | - -For timing, compare discovery plus the first completed structured action, -a repeat using its known identity/contract, and similar natural-language -wording with different parameters. Record completed outcomes and separate -connection, model, discovery, and execution time. Removing a confirmation -blocker does not establish an end-to-end speedup. - -`ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts` checks -this rationale table against the selected production manifests and exercises -their contracts through the real structured dispatcher. diff --git a/ts/packages/agents/github-cli/src/github-cliActionHandler.ts b/ts/packages/agents/github-cli/src/github-cliActionHandler.ts index a71eef413f..fc064291d0 100644 --- a/ts/packages/agents/github-cli/src/github-cliActionHandler.ts +++ b/ts/packages/agents/github-cli/src/github-cliActionHandler.ts @@ -1846,11 +1846,12 @@ async function searchRepoCandidates(query: string): Promise { const stdout = await runGh([ "search", "repos", - query, "--limit", "5", "--json", "name,owner", + "--", + query, ]); const data = JSON.parse(stdout) as Array<{ name?: string; diff --git a/ts/packages/agents/github-cli/src/github-cliManifest.json b/ts/packages/agents/github-cli/src/github-cliManifest.json index 0e4eb47072..7fa4eeb83e 100644 --- a/ts/packages/agents/github-cli/src/github-cliManifest.json +++ b/ts/packages/agents/github-cli/src/github-cliManifest.json @@ -8,7 +8,18 @@ "schemaFile": "../dist/github-cliSchema.pas.json", "grammarFile": "../dist/github-cliSchema.ag.json", "actionPolicies": { - "prFiles": { "effects": "read-only" } + "prFiles": { "effects": "read-only" }, + "codespaceList": { "effects": "read-only" }, + "gistList": { "effects": "read-only" }, + "orgList": { "effects": "read-only" }, + "cacheList": { "effects": "read-only" }, + "issueList": { "effects": "read-only" }, + "issueView": { "effects": "read-only" }, + "prList": { "effects": "read-only" }, + "prView": { "effects": "read-only" }, + "prMergedStatus": { "effects": "read-only" }, + "prChecks": { "effects": "read-only" }, + "releaseList": { "effects": "read-only" } }, "schemaType": "GithubCliActions" } diff --git a/ts/packages/agents/ipconfig/src/ipconfigManifest.json b/ts/packages/agents/ipconfig/src/ipconfigManifest.json index a0636f6a1f..5a25e1e88c 100644 --- a/ts/packages/agents/ipconfig/src/ipconfigManifest.json +++ b/ts/packages/agents/ipconfig/src/ipconfigManifest.json @@ -9,7 +9,8 @@ "grammarFile": "../dist/ipconfigSchema.ag.json", "actionPolicies": { "displayHelpMessage": { "effects": "read-only" }, - "displayFullConfigurationInformation": { "effects": "read-only" } + "displayFullConfigurationInformation": { "effects": "read-only" }, + "displayDNSResolverCacheContents": { "effects": "read-only" } }, "schemaType": "IpconfigActions" } diff --git a/ts/packages/agents/playerLocal/src/agent/localPlayerManifest.json b/ts/packages/agents/playerLocal/src/agent/localPlayerManifest.json index 8a6a9d9999..7a294b5e88 100644 --- a/ts/packages/agents/playerLocal/src/agent/localPlayerManifest.json +++ b/ts/packages/agents/playerLocal/src/agent/localPlayerManifest.json @@ -6,6 +6,11 @@ "originalSchemaFile": "./localPlayerSchema.ts", "schemaFile": "../../dist/agent/localPlayerSchema.pas.json", "grammarFile": "../../dist/agent/localPlayerSchema.ag.json", + "actionPolicies": { + "status": { "effects": "read-only" }, + "showQueue": { "effects": "read-only" }, + "showMusicFolder": { "effects": "read-only" } + }, "schemaType": { "action": "LocalPlayerActions", "entity": "LocalPlayerEntities" diff --git a/ts/packages/agents/powershell/src/manifest.json b/ts/packages/agents/powershell/src/manifest.json index f07693cc83..7b43e1072a 100644 --- a/ts/packages/agents/powershell/src/manifest.json +++ b/ts/packages/agents/powershell/src/manifest.json @@ -8,7 +8,10 @@ "originalSchemaFile": "./schema/scriptActions.mts", "schemaFile": "../dist/powershellSchema.pas.json", "grammarFile": "../dist/powershellSchema.ag.json", - "schemaType": "PowerShellActions" + "schemaType": "PowerShellActions", + "actionPolicies": { + "listPowerShellFlows": { "effects": "read-only" } + } }, "subActionManifests": { "powershell-files": { diff --git a/ts/packages/agents/taskflow/manifest.json b/ts/packages/agents/taskflow/manifest.json index c7aef90ee8..35f0e16ed1 100644 --- a/ts/packages/agents/taskflow/manifest.json +++ b/ts/packages/agents/taskflow/manifest.json @@ -5,6 +5,9 @@ "schema": { "description": "Task flow actions. Flows are taught from examples and registered for reuse.", "schemaFile": "./src/schema/userActions.mts", - "schemaType": "TaskFlowActions" + "schemaType": "TaskFlowActions", + "actionPolicies": { + "listTaskFlows": { "effects": "read-only" } + } } } diff --git a/ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts b/ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts index 5353b7a3bb..a2ef3a094c 100644 --- a/ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts +++ b/ts/packages/defaultAgentProvider/test/readOnlyActionPolicies.spec.ts @@ -2,6 +2,8 @@ // Licensed under the MIT License. import { jest } from "@jest/globals"; +import * as childProcess from "node:child_process"; +import { promisify } from "node:util"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -29,6 +31,41 @@ import { instantiate as instantiateTimer } from "@typeagent/timer-agent/agent/ha import { instantiate as instantiateWeather } from "@typeagent/weather-agent/agent/handlers"; import { getDefaultAppAgentProviders } from "../src/defaultAgentProviders.js"; +const runCli = + jest.fn< + ( + command: string, + args: readonly string[], + options: unknown, + ) => Promise<{ stdout: string; stderr: string }> + >(); +const execFile = Object.assign( + () => { + throw new Error("Unexpected callback-based subprocess invocation"); + }, + { [promisify.custom]: runCli }, +); +// Mock only transport; policies, validation, readiness, handlers and choices +// are the production implementations. +for (const module of ["node:child_process", "child_process"]) { + jest.unstable_mockModule(module, () => ({ ...childProcess, execFile })); +} +const { instantiate: instantiateGitHub } = await import( + "@typeagent/github-cli-agent/agent/handlers" +); +const { instantiate: instantiateIpconfig } = await import( + "@typeagent/ipconfig-agent/agent/handlers" +); +const { instantiate: instantiateLocalPlayer } = await import( + "@typeagent/music-local/agent/handlers" +); +const { instantiate: instantiatePowerShell } = await import( + "@typeagent/powershell-typeagent/agent/handlers" +); +const { instantiate: instantiateTaskflow } = await import( + "@typeagent/taskflow-typeagent/agent/handlers" +); + const allowed = [ ["list", "listLists"], ["list", "getList"], @@ -36,8 +73,25 @@ const allowed = [ ["weather", "getForecast"], ["ipconfig", "displayHelpMessage"], ["ipconfig", "displayFullConfigurationInformation"], + ["ipconfig", "displayDNSResolverCacheContents"], ["github-cli", "prFiles"], + ["github-cli", "codespaceList"], + ["github-cli", "gistList"], + ["github-cli", "orgList"], + ["github-cli", "cacheList"], + ["github-cli", "issueList"], + ["github-cli", "issueView"], + ["github-cli", "prList"], + ["github-cli", "prView"], + ["github-cli", "prMergedStatus"], + ["github-cli", "prChecks"], + ["github-cli", "releaseList"], ["timer", "listReminders"], + ["localPlayer", "status"], + ["localPlayer", "showQueue"], + ["localPlayer", "showMusicFolder"], + ["powershell", "listPowerShellFlows"], + ["taskflow", "listTaskFlows"], ] as const; const agentNames = [...new Set(allowed.map(([name]) => name))]; const seed = JSON.stringify([ @@ -62,8 +116,8 @@ describe("built-in read-only action policies", () => { let executionAllowed: boolean; let readiness: ReadinessReport; let listAgent: AppAgent; + let agentStorage: Map; const scope = {}; - const externalExecute = jest.fn>(); const setup = jest.fn>(); beforeEach(async () => { @@ -73,11 +127,15 @@ describe("built-in read-only action policies", () => { jest.spyOn(storage, "write"); executionAllowed = true; readiness = { state: "ready" }; - externalExecute.mockReset().mockImplementation(async () => { + runCli.mockReset().mockImplementation(async (command, args) => { + if (command === "gh" && args.join(" ") === "auth status") { + return { stdout: "Authenticated test account", stderr: "" }; + } throw new Error( - "External handler must not execute in this fixture", + `Unexpected subprocess: ${command} ${args.join(" ")}`, ); }); + agentStorage = new Map(); setup.mockReset(); const bundled = getDefaultAppAgentProviders(undefined)[0]; manifests = new Map( @@ -117,9 +175,34 @@ describe("built-in read-only action policies", () => { } if (name === "timer") return instantiateTimer(); if (name === "weather") return instantiateWeather(); - // Only discovery and the pre-handler confirmation gate are - // exercised for ipconfig/GitHub here, never live CLI I/O. - return { executeAction: externalExecute }; + if (name === "github-cli") return instantiateGitHub(); + if (name === "ipconfig") return instantiateIpconfig(); + const factories: Record AppAgent> = { + localPlayer: instantiateLocalPlayer, + powershell: instantiatePowerShell, + taskflow: instantiateTaskflow, + }; + const agent = factories[name]?.(); + if (!agent) throw new Error(`Unexpected agent: ${name}`); + const isolated = getFsStorageProvider().getStorage( + name, + directory, + ); + agentStorage.set(name, isolated); + jest.spyOn(isolated, "write"); + return { + ...agent, + updateAgentContext: (enable, session, schemaName) => + agent.updateAgentContext!( + enable, + { + ...session, + instanceStorage: isolated, + sessionStorage: isolated, + }, + schemaName, + ), + }; }, unloadAppAgent: async () => {}, }; @@ -147,6 +230,10 @@ describe("built-in read-only action policies", () => { isActive: () => true, }), ); + runCli.mockClear(); + for (const isolated of agentStorage.values()) { + jest.mocked(isolated.write).mockClear(); + } }); afterEach(async () => { @@ -161,7 +248,7 @@ describe("built-in read-only action policies", () => { async function request( schemaName: string, actionName: string, - parameters: Record = {}, + parameters?: Record, ): Promise { const search = await dispatcher.searchActions({ query: `${schemaName} ${actionName}`, @@ -174,7 +261,12 @@ describe("built-in read-only action policies", () => { scopeId: search.scopeId, schemaName, actionName, - parameters, + ...(parameters === undefined && + (schemaName === "localPlayer" || + schemaName === "powershell" || + schemaName === "taskflow") + ? {} + : { parameters: parameters ?? {} }), }; } @@ -195,7 +287,7 @@ describe("built-in read-only action policies", () => { ).toBe(status); } - it("keeps the rationale table synchronized with exact production declarations", async () => { + it("matches the audited exact allowlist to valid production declarations", () => { const declared = []; for (const [name, manifest] of manifests) { for (const [actionName, policy] of Object.entries( @@ -214,20 +306,8 @@ describe("built-in read-only action policies", () => { expect(declared.sort()).toEqual( allowed.map(([name, action]) => `${name}.${action}`).sort(), ); - const document = await fs.readFile( - new URL( - "../../../../../docs/architecture/read-only-action-policies.md", - import.meta.url, - ), - "utf8", - ); - const rows = [ - ...document.matchAll( - /^\|[^|]+\|[ \t]+`([^`]+)`[ \t]+\|([^|]+)\|$/gm, - ), - ]; - expect(rows.map((row) => row[1]).sort()).toEqual(declared); - for (const row of rows) expect(row[2].trim()).toMatch(/\.$/); + expect(declared).toHaveLength(25); + expect(agentNames).toHaveLength(8); }); it.each(allowed)( @@ -253,7 +333,13 @@ describe("built-in read-only action policies", () => { ); it("leaves every unclassified sibling requiring confirmation", async () => { - for (const name of agentNames) { + const schemaNames = agentNames.flatMap((name) => [ + name, + ...Object.keys(manifests.get(name)?.subActionManifests ?? {}).map( + (child) => `${name}.${child}`, + ), + ]); + for (const name of schemaNames) { const config = context.agents.getActionConfig(name); const schema = context.agents.getActionSchemaFileForConfig(config); for (const actionName of schema.parsedActionSchema.actionSchemas.keys()) { @@ -371,6 +457,362 @@ describe("built-in read-only action policies", () => { }); }); + it("executes every exempt GitHub list/view branch through mocked CLI transport", async () => { + const cases: [string, Record, string[]][] = [ + ["codespaceList", {}, ["codespace", "list"]], + ["gistList", {}, ["gist", "list"]], + ["gistList", { public: true }, ["gist", "list", "--public"]], + ["orgList", {}, ["org", "list"]], + ["cacheList", {}, ["cache", "list"]], + ["releaseList", {}, ["release", "list"]], + [ + "releaseList", + { repo: "owner/repo" }, + ["release", "list", "--repo", "owner/repo"], + ], + [ + "issueView", + { number: 42, repo: "owner/repo" }, + [ + "issue", + "view", + "42", + "--repo", + "owner/repo", + "--json", + "number,title,state,body,author,labels,assignees,comments,url,createdAt,closedAt", + ], + ], + [ + "prView", + { number: 42, repo: "owner/repo" }, + [ + "pr", + "view", + "42", + "--repo", + "owner/repo", + "--json", + "number,title,state,body,author,labels,url,createdAt,headRefName,baseRefName,isDraft,additions,deletions,changedFiles", + ], + ], + [ + "prChecks", + { number: 42, repo: "owner/repo" }, + ["pr", "checks", "42", "--repo", "owner/repo"], + ], + [ + "prMergedStatus", + { branch: "--web" }, + [ + "pr", + "list", + "--head", + "--web", + "--state", + "merged", + "--limit", + "20", + "--json", + "number,title,url,mergedAt,headRefName,baseRefName", + ], + ], + [ + "prMergedStatus", + { + branch: "feature", + base: "main", + repo: "owner/repo", + limit: 3, + }, + [ + "pr", + "list", + "--repo", + "owner/repo", + "--base", + "main", + "--head", + "feature", + "--state", + "merged", + "--limit", + "3", + "--json", + "number,title,url,mergedAt,headRefName,baseRefName", + ], + ], + ]; + for (const kind of ["issue", "pr"]) { + const fields = + kind === "issue" + ? "number,title,state,url,createdAt,labels" + : "number,title,state,url,createdAt,headRefName,isDraft"; + cases.push([`${kind}List`, {}, [kind, "list", "--json", fields]]); + for (const assignee of ["none", "octocat"]) { + cases.push([ + `${kind}List`, + { + repo: "owner/repo", + state: "open", + label: "--web", + author: "@me", + assignee, + limit: 3, + }, + [ + kind, + "list", + "--repo", + "owner/repo", + "--state", + "open", + "--label", + "--web", + "--author", + "@me", + ...(assignee === "none" + ? ["--search", "no:assignee"] + : ["--assignee", assignee]), + "--limit", + "3", + "--json", + fields, + ], + ]); + } + } + for (const [actionName, parameters, args] of cases) { + runCli.mockClear().mockResolvedValue({ + stdout: + actionName === "issueView" || actionName === "prView" + ? JSON.stringify({ + number: 42, + title: "Read fixture", + state: "OPEN", + body: "", + author: { login: "octocat" }, + labels: [], + assignees: [], + comments: [], + url: "https://github.com/owner/repo/pull/42", + }) + : args.includes("--json") + ? "[]" + : "Inventory fixture", + stderr: "", + }); + const result = await dispatcher.executeAction( + await request("github-cli", actionName, parameters), + ); + expect(result.status).toBe("completed"); + expect(result.results[0].result).toHaveProperty("displayContent"); + expect(runCli).toHaveBeenCalledTimes(1); + expect(runCli).toHaveBeenCalledWith( + "gh", + args, + expect.objectContaining({ + timeout: 30_000, + maxBuffer: 1024 * 1024, + }), + ); + } + }); + + it("executes real bounded PR file reads with and without patch excerpts", async () => { + for (const includePatch of [false, true]) { + runCli + .mockClear() + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 42, + title: "Read fixture", + state: "OPEN", + url: "https://github.com/owner/repo/pull/42", + changedFiles: 1, + additions: 1, + deletions: 0, + }), + stderr: "", + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + filename: "file.ts", + status: "modified", + additions: 1, + deletions: 0, + changes: 1, + ...(includePatch ? { patch: "+new line" } : {}), + }, + ]), + stderr: "", + }); + const result = await dispatcher.executeAction( + await request("github-cli", "prFiles", { + number: 42, + repo: "owner/repo", + includePatch, + maxFiles: 1, + maxPatchLines: 1, + }), + ); + expect(result.status).toBe("completed"); + expect(result.results[0].result).toMatchObject({ + displayContent: { + rawData: { + kind: "prFiles", + repo: "owner/repo", + number: 42, + files: [ + expect.objectContaining({ + path: "file.ts", + ...(includePatch ? { patch: "+new line" } : {}), + }), + ], + }, + }, + }); + expect( + runCli.mock.calls.map(([command, args]) => [command, args]), + ).toEqual([ + [ + "gh", + [ + "pr", + "view", + "42", + "--repo", + "owner/repo", + "--json", + "number,title,url,state,isDraft,additions,deletions,changedFiles,headRefName,baseRefName,headRepository,headRepositoryOwner", + ], + ], + [ + "gh", + [ + "api", + "--hostname", + "github.com", + "repos/owner/repo/pulls/42/files?per_page=2&page=1", + ...(includePatch + ? [] + : [ + "--jq", + "[.[] | {filename, status, additions, deletions, changes, previous_filename}]", + ]), + ], + ], + ]); + } + }); + + it("keeps bare repository names literal and preserves the real repository-choice prompt", async () => { + for (const repo of ["TypeAgent", "--web", "-w"]) { + runCli.mockClear().mockResolvedValue({ + stdout: JSON.stringify([ + { name: "TypeAgent", owner: { login: "microsoft" } }, + ]), + stderr: "", + }); + const result = await dispatcher.executeAction( + await request("github-cli", "prFiles", { repo, number: 2991 }), + ); + expect(requirePrompt(result).prompt).toMatchObject({ + type: "multiChoice", + message: expect.stringContaining("Pick the repo"), + }); + expect(runCli).toHaveBeenCalledTimes(1); + expect(runCli).toHaveBeenCalledWith( + "gh", + [ + "search", + "repos", + "--limit", + "5", + "--json", + "name,owner", + "--", + repo, + ], + expect.anything(), + ); + await cancel(result, "execution_uncertain"); + } + }); + + it("keeps GitHub reads unavailable when the real auth probe fails without invoking setup", async () => { + const input = await request("github-cli", "orgList"); + runCli.mockRejectedValue({ code: 1, stderr: "Not authenticated" }); + await context.agents.refreshReadiness("github-cli"); + expect((await dispatcher.executeAction(input)).status).toBe( + "unavailable", + ); + expect( + runCli.mock.calls.map(([command, args]) => [command, args]), + ).toEqual([["gh", ["auth", "status"]]]); + }); + + it("executes the three fixed Windows inventory commands without a shell or network mutation", async () => { + runCli.mockResolvedValue({ + stdout: "Windows IP Configuration\n\n Host Name . . . . . : fixture", + stderr: "", + }); + for (const [actionName, flag] of [ + ["displayHelpMessage", "/?"], + ["displayFullConfigurationInformation", "/all"], + ["displayDNSResolverCacheContents", "/displaydns"], + ]) { + runCli.mockClear(); + const result = await dispatcher.executeAction( + await request("ipconfig", actionName), + ); + expect(result.status).toBe("completed"); + expect(result.results[0].result).toHaveProperty("displayContent"); + expect(runCli).toHaveBeenCalledTimes(1); + expect(runCli).toHaveBeenCalledWith("ipconfig", [flag], { + timeout: 30_000, + }); + } + }); + + it("reads real local-player state without playback, file scans, or storage writes", async () => { + for (const [actionName, text] of [ + ["status", "No track loaded"], + ["showQueue", "Queue is empty"], + ["showMusicFolder", "Music folder"], + ]) { + const result = await dispatcher.executeAction( + await request("localPlayer", actionName), + ); + expect(result.status).toBe("completed"); + expect(JSON.stringify(result.results[0].result)).toContain(text); + } + expect(agentStorage.get("localPlayer")!.write).not.toHaveBeenCalled(); + expect(runCli).not.toHaveBeenCalled(); + }); + + it("lists real registered flows without running scripts or updating their indexes", async () => { + for (const [schemaName, actionName] of [ + ["powershell", "listPowerShellFlows"], + ["taskflow", "listTaskFlows"], + ]) { + const isolated = agentStorage.get(schemaName)!; + const before = await isolated.read("index.json", "utf8"); + const result = await dispatcher.executeAction( + await request(schemaName, actionName), + ); + expect(result.status).toBe("completed"); + expect(result.results[0].result).toHaveProperty("displayContent"); + expect(JSON.stringify(result.results[0].result)).not.toContain( + "store not available", + ); + expect(isolated.write).not.toHaveBeenCalled(); + expect(await isolated.read("index.json", "utf8")).toBe(before); + } + expect(runCli).not.toHaveBeenCalled(); + }); + it.each([ ["list", "addItems", { listName: "groceries", items: ["bread"] }], ["ipconfig", "purgeDNSResolverCache", {}], @@ -381,6 +823,9 @@ describe("built-in read-only action policies", () => { ], ["timer", "cancelReminder", { id: "all" }], ["weather", "getAlerts", { location: "Seattle" }], + ["localPlayer", "pause", undefined], + ["powershell", "deletePowerShellFlow", { name: "fixture" }], + ["taskflow", "deleteTaskFlow", { name: "fixture" }], ] as const)( "still confirms %s.%s before entering its handler", async (schemaName, actionName, parameters) => { @@ -389,7 +834,7 @@ describe("built-in read-only action policies", () => { ); expect(requirePrompt(result).prompt.type).toBe("confirmation"); await cancel(result); - expect(externalExecute).not.toHaveBeenCalled(); + expect(runCli).not.toHaveBeenCalled(); expect(storage.write).not.toHaveBeenCalled(); }, );