From 6eca9a82f67baa77c20c3a878845e22b0d9d2fda Mon Sep 17 00:00:00 2001 From: Hillary Mutisya <150286414+hillary-mutisya@users.noreply.github.com> Date: Tue, 11 Aug 2026 13:20:56 -0700 Subject: [PATCH 1/5] Update PowerShell dev actions and namespace handlers - add cancellation, structured failures, bounded repair, and flow reuse - register and implement all PowerShell namespaces - require confirmation for mutating actions with unattended default-deny - extract namespace actions into typed handlers with a shared registry - add routing, persistence, concurrency, sandbox, and runtime coverage - update Copilot dev-action routing and project documentation --- ts/packages/agentSdk/src/action.ts | 7 + .../scenarios/dev-actions-routing.json | 4 +- .../agents/powershell/scripts/scriptHost.ps1 | 17 +- .../agents/powershell/src/actionHandler.mts | 570 +++++++++++------- .../src/execution/powershellRunner.mts | 27 +- .../agents/powershell/src/manifest.json | 60 ++ .../src/namespaces/actionHandlerRegistry.mts | 65 ++ .../src/namespaces/archives/actionHandler.mts | 55 ++ .../src/namespaces/data/actionHandler.mts | 83 +++ .../src/namespaces/data/dataSchema.agr | 2 +- .../src/namespaces/files/actionHandler.mts | 99 +++ .../src/namespaces/files/filesSchema.agr | 4 +- .../src/namespaces/namespaceActionHandler.mts | 113 ++++ .../src/namespaces/network/actionHandler.mts | 83 +++ .../namespaces/processes/actionHandler.mts | 55 ++ .../src/namespaces/services/actionHandler.mts | 46 ++ .../src/namespaces/system/actionHandler.mts | 41 ++ .../powershell/src/schema/scriptActions.mts | 18 + .../powershell/src/store/powerShellStore.mts | 13 + .../src/types/powerShellAgentContext.mts | 8 + .../src/types/powerShellFailure.mts | 62 ++ .../powershell/test/actionHandler.spec.ts | 432 ++++++++++++- .../powershell/test/powerShellStore.spec.ts | 1 + ts/packages/copilot-plugin/hooks.json | 2 +- .../src/hooks/hook-dev-actions.ts | 77 ++- .../copilot-plugin/src/hooks/hook-router.ts | 84 +-- .../src/shared/typeagent-client.ts | 24 +- .../test/hookDevActions.spec.ts | 124 +++- .../src/reasoning/reasoningProfile.ts | 1 + 29 files changed, 1891 insertions(+), 286 deletions(-) create mode 100644 ts/packages/agents/powershell/src/namespaces/actionHandlerRegistry.mts create mode 100644 ts/packages/agents/powershell/src/namespaces/archives/actionHandler.mts create mode 100644 ts/packages/agents/powershell/src/namespaces/data/actionHandler.mts create mode 100644 ts/packages/agents/powershell/src/namespaces/files/actionHandler.mts create mode 100644 ts/packages/agents/powershell/src/namespaces/namespaceActionHandler.mts create mode 100644 ts/packages/agents/powershell/src/namespaces/network/actionHandler.mts create mode 100644 ts/packages/agents/powershell/src/namespaces/processes/actionHandler.mts create mode 100644 ts/packages/agents/powershell/src/namespaces/services/actionHandler.mts create mode 100644 ts/packages/agents/powershell/src/namespaces/system/actionHandler.mts create mode 100644 ts/packages/agents/powershell/src/types/powerShellAgentContext.mts create mode 100644 ts/packages/agents/powershell/src/types/powerShellFailure.mts diff --git a/ts/packages/agentSdk/src/action.ts b/ts/packages/agentSdk/src/action.ts index c5d0305620..67fd0be711 100644 --- a/ts/packages/agentSdk/src/action.ts +++ b/ts/packages/agentSdk/src/action.ts @@ -35,6 +35,13 @@ export type SerializedError = { export type ActionResultError = { error: string; fallbackToReasoning?: boolean | undefined; + // Stable machine-readable code for callers that need policy or retry + // decisions without parsing the display message. + errorCode?: string | undefined; + // Whether the caller may safely retry after changing the action. + retryable?: boolean | undefined; + // True when the failed action may already have changed external state. + mayHaveSideEffects?: boolean | undefined; // Rich display to show in place of the plain `error` text (e.g. setup // instructions with a config snippet, which need markdown to survive // rendering). Optional — clients fall back to `error` when absent. diff --git a/ts/packages/agents/powershell/benchmark/scenarios/dev-actions-routing.json b/ts/packages/agents/powershell/benchmark/scenarios/dev-actions-routing.json index b64792bd3e..f837979bec 100644 --- a/ts/packages/agents/powershell/benchmark/scenarios/dev-actions-routing.json +++ b/ts/packages/agents/powershell/benchmark/scenarios/dev-actions-routing.json @@ -31,7 +31,7 @@ { "id": "dev-route-02", "category": "dev-actions-routing", - "description": "PowerShell schema family includes the root flow schema", + "description": "PowerShell schema family includes the files namespace", "required": true, "setup": { "requiredFlows": ["listFiles"] @@ -51,7 +51,7 @@ "disposition": { "status": "handled", "path": "action", - "schemas": ["powershell"] + "schemas": ["powershell.powershell-files"] } } } diff --git a/ts/packages/agents/powershell/scripts/scriptHost.ps1 b/ts/packages/agents/powershell/scripts/scriptHost.ps1 index 9066eae1c3..ff6b39f7d5 100644 --- a/ts/packages/agents/powershell/scripts/scriptHost.ps1 +++ b/ts/packages/agents/powershell/scripts/scriptHost.ps1 @@ -50,7 +50,8 @@ try { $expandedAllowedPaths = @() foreach ($ap in $AllowedPaths) { try { - $expandedAllowedPaths += $ExecutionContext.InvokeCommand.ExpandString($ap) + $expandedPath = $ExecutionContext.InvokeCommand.ExpandString($ap) + $expandedAllowedPaths += [System.IO.Path]::GetFullPath($expandedPath).TrimEnd('\', '/') } catch { $expandedAllowedPaths += $ap } @@ -77,11 +78,21 @@ try { try { $isValidPath = Test-Path $val -IsValid } catch { } if ($isValidPath) { $resolvedPath = $null - try { $resolvedPath = (Resolve-Path $val -ErrorAction SilentlyContinue).Path } catch {} + try { + $resolvedPath = (Resolve-Path $val -ErrorAction SilentlyContinue).Path + if (-not $resolvedPath) { + $resolvedPath = [System.IO.Path]::GetFullPath($val) + } + $resolvedPath = $resolvedPath.TrimEnd('\', '/') + } catch {} if ($resolvedPath) { $pathAllowed = $false foreach ($ap in $expandedAllowedPaths) { - if ($resolvedPath -like "$ap*") { + if ( + $resolvedPath.Equals($ap, [System.StringComparison]::OrdinalIgnoreCase) -or + $resolvedPath.StartsWith("$ap\", [System.StringComparison]::OrdinalIgnoreCase) -or + $resolvedPath.StartsWith("$ap/", [System.StringComparison]::OrdinalIgnoreCase) + ) { $pathAllowed = $true break } diff --git a/ts/packages/agents/powershell/src/actionHandler.mts b/ts/packages/agents/powershell/src/actionHandler.mts index 0780efcafb..f91fc6350b 100644 --- a/ts/packages/agents/powershell/src/actionHandler.mts +++ b/ts/packages/agents/powershell/src/actionHandler.mts @@ -38,121 +38,42 @@ import { executeScript, type ScriptExecutionRequest, } from "./execution/powershellRunner.mjs"; +import { + createPowerShellExecutionFailure, + createPowerShellFailure, +} from "./types/powerShellFailure.mjs"; +import type { PowerShellAgentContext } from "./types/powerShellAgentContext.mjs"; +import { executeNamespaceAction } from "./namespaces/actionHandlerRegistry.mjs"; +import type { PowerShellAction } from "./namespaces/namespaceActionHandler.mjs"; import registerDebug from "debug"; const debug = registerDebug("typeagent:powershell:handler"); const __dirname = dirname(fileURLToPath(import.meta.url)); const SAMPLES_DIR = join(__dirname, "..", "samples"); -interface PowerShellAgentContext { - store?: PowerShellStore | undefined; -} - -type StaticPowerShellAction = { - script: string; - allowedCmdlets: string[]; -}; - -const NETWORK_ACTIONS: Record = { - testConnection: { - script: `param([string]$ComputerName, [int]$Port) -if ($Port -gt 0) { - Test-NetConnection -ComputerName $ComputerName -Port $Port -} else { - Test-NetConnection -ComputerName $ComputerName -}`, - allowedCmdlets: ["Test-NetConnection"], - }, - portListeners: { - script: `param([int]$Port) -$listeners = Get-NetTCPConnection -State Listen -ErrorAction SilentlyContinue -if ($Port -gt 0) { - $listeners = $listeners | Where-Object { $_.LocalPort -eq $Port } -} -$listeners | - Sort-Object LocalPort, OwningProcess | - ForEach-Object { - $process = Get-Process -Id $_.OwningProcess -ErrorAction SilentlyContinue - [PSCustomObject]@{ - LocalAddress = $_.LocalAddress - LocalPort = $_.LocalPort - ProcessId = $_.OwningProcess - ProcessName = if ($process) { $process.ProcessName } else { "(unknown)" } - } - }`, - allowedCmdlets: [ - "Get-NetTCPConnection", - "Where-Object", - "Sort-Object", - "ForEach-Object", - "Get-Process", - ], - }, - networkAdapters: { - script: `param([string]$Name) -if ($Name) { - Get-NetAdapter -Name $Name -} else { - Get-NetAdapter -}`, - allowedCmdlets: ["Get-NetAdapter"], - }, - ipConfig: { - script: `param([string]$InterfaceAlias) -if ($InterfaceAlias) { - Get-NetIPConfiguration -InterfaceAlias $InterfaceAlias -} else { - Get-NetIPConfiguration -}`, - allowedCmdlets: ["Get-NetIPConfiguration"], - }, - dnsLookup: { - script: `param([string]$Name, [string]$Type) -if ($Type) { - Resolve-DnsName -Name $Name -Type $Type -} else { - Resolve-DnsName -Name $Name -}`, - allowedCmdlets: ["Resolve-DnsName"], - }, -}; - -async function executeStaticNetworkAction(action: { - schemaName?: string; - actionName: string; - parameters?: Record; -}): Promise { - if (action.schemaName !== "powershell.powershell-network") { - return undefined; - } - const definition = NETWORK_ACTIONS[action.actionName]; - if (!definition) { - return undefined; - } - - const result = await executeScript({ - script: definition.script, - parameters: action.parameters ?? {}, - sandbox: { - allowedCmdlets: definition.allowedCmdlets, - allowedPaths: [], - allowedModules: [], - maxExecutionTime: 30, - networkAccess: true, - }, - workingDirectory: homedir(), +const flowMutationTails = new Map>(); +const repairAttempts = new WeakSet(); + +async function withFlowMutationLock( + flowName: string, + operation: () => Promise, +): Promise { + const previous = flowMutationTails.get(flowName) ?? Promise.resolve(); + let release: () => void; + const current = new Promise((resolve) => { + release = resolve; }); - - if (!result.success) { - return { - error: - result.stderr || `Script exited with code ${result.exitCode}`, - fallbackToReasoning: true, - }; + const tail = previous.then(() => current); + flowMutationTails.set(flowName, tail); + await previous; + try { + return await operation(); + } finally { + release!(); + if (flowMutationTails.get(flowName) === tail) { + flowMutationTails.delete(flowName); + } } - return createActionResultFromTextDisplay( - result.stdout.trim() || "(no output)", - ); } async function seedSampleFlows(store: PowerShellStore): Promise { @@ -189,6 +110,7 @@ async function executeFlowScript( flow: PowerShellFlowDefinition, script: string, parameters: Record, + abortSignal?: AbortSignal, ): Promise { const resolvedParams: Record = {}; for (const paramDef of flow.parameters) { @@ -210,18 +132,20 @@ async function executeFlowScript( }, // Use user's home directory as working directory for consistent path resolution workingDirectory: homedir(), + abortSignal, }; const result = await executeScript(request); + if (result.cancelled) { + abortSignal?.throwIfAborted(); + } if (result.success) { const output = result.stdout.trim() || "(no output)"; return createActionResultFromTextDisplay(output); } - const errorMsg = - result.stderr || `Script exited with code ${result.exitCode}`; - return { error: errorMsg, fallbackToReasoning: true }; + return createPowerShellExecutionFailure(result); } function mapParamsToFlowDefs( @@ -365,7 +289,11 @@ async function validateFlowGrammarPatterns( ? ["Suggestions:", ...validationResult.suggestions] : []), ].join("\n"); - return { error: createActionResultFromError(message) }; + return { + error: createPowerShellFailure("policyDenied", message, { + retryable: false, + }), + }; } if (validationResult.warnings?.length) { @@ -437,7 +365,8 @@ function parseNamedParameters( } if (typeof value !== "string") { return { - error: createActionResultFromError( + error: createPowerShellFailure( + "invalidParameters", `${parameterName} must be a JSON string`, ), }; @@ -454,7 +383,8 @@ function parseNamedParameters( return { parameters: parsed as Record }; } catch (error) { return { - error: createActionResultFromError( + error: createPowerShellFailure( + "invalidParameters", `Invalid JSON in ${parameterName}: ${error instanceof Error ? error.message : String(error)}`, ), }; @@ -464,6 +394,7 @@ function parseNamedParameters( async function executeDraftRecipe( recipe: ScriptRecipe, suppliedParameters: Record, + abortSignal?: AbortSignal, ): Promise<{ output: string } | { error: ActionResult }> { const executionParameters: Record = {}; mapParamsToFlowDefs( @@ -477,7 +408,12 @@ async function executeDraftRecipe( validatePathParameters(executionParameters, recipe.parameters) ?? validateParameterRules(executionParameters, recipe.parameters); if (validationError) { - return { error: createActionResultFromError(validationError) }; + return { + error: createPowerShellFailure( + "invalidParameters", + validationError, + ), + }; } const result = await executeScript({ @@ -485,28 +421,259 @@ async function executeDraftRecipe( parameters: executionParameters, sandbox: recipe.sandbox, workingDirectory: homedir(), + abortSignal, }); + if (result.cancelled) { + abortSignal?.throwIfAborted(); + } if (!result.success) { return { - error: createActionResultFromError( - result.stderr || `Script exited with code ${result.exitCode}`, - ), + error: createPowerShellExecutionFailure(result), }; } return { output: result.stdout.trim() || "(no output)" }; } +async function createOrReusePowerShellFlow( + params: Record, + flowStore: PowerShellStore, + context: ActionContext, +): Promise { + const actionName = params.actionName as string | undefined; + const script = params.script as string | undefined; + if (!actionName) { + return createPowerShellFailure( + "invalidParameters", + "Missing required parameter: actionName", + ); + } + if (!script) { + return createPowerShellFailure( + "invalidParameters", + "Missing required parameter: script", + ); + } + const executionParameters = parseNamedParameters( + params.executionParametersJson, + "executionParametersJson", + ); + if ("error" in executionParameters) { + return executionParameters.error; + } + + return withFlowMutationLock(actionName, async () => { + context.abortSignal?.throwIfAborted(); + const existing = await flowStore.getFlow(actionName); + if (existing) { + const existingScript = await flowStore.getScript(actionName); + if (!existingScript) { + return createPowerShellFailure( + "scriptFailure", + `Script not found for flow: ${actionName}`, + ); + } + const mappedParameters: Record = {}; + mapParamsToFlowDefs( + executionParameters.parameters, + existing.parameters, + mappedParameters, + ); + expandEnvVarsInParams(mappedParameters, existing.parameters); + const validationError = + validatePathParameters(mappedParameters, existing.parameters) ?? + validateParameterRules(mappedParameters, existing.parameters); + if (validationError) { + return createPowerShellFailure( + "invalidParameters", + validationError, + ); + } + const result = await executeFlowScript( + existing, + existingScript, + mappedParameters, + context.abortSignal, + ); + if (result.error === undefined) { + await flowStore.recordUsage(actionName); + } + return result; + } + + const grammarValidation = await validateFlowGrammarPatterns( + actionName, + (params.description as string) ?? "", + (params.grammarPatterns as FlowGrammarPatternInput[]) ?? [], + context, + ); + if ("error" in grammarValidation) { + return grammarValidation.error; + } + const recipe = buildPowerShellRecipe( + params, + grammarValidation.patterns, + ); + context.abortSignal?.throwIfAborted(); + const pendingId = await flowStore.savePending(recipe); + const pendingFile = `${pendingId}.recipe.json`; + let execution: Awaited>; + try { + execution = await executeDraftRecipe( + recipe, + executionParameters.parameters, + context.abortSignal, + ); + context.abortSignal?.throwIfAborted(); + } catch (error) { + await flowStore.deletePending(pendingFile); + throw error; + } + if ("error" in execution) { + await flowStore.deletePending(pendingFile); + return execution.error; + } + + const promoted = await flowStore.promotePending(pendingFile); + if (!promoted) { + await flowStore.deletePending(pendingFile); + return createPowerShellFailure( + "partialSideEffects", + `The script executed, but flow '${actionName}' could not be promoted because that name is already registered. The operation may have caused side effects and was not executed again.`, + ); + } + try { + context.abortSignal?.throwIfAborted(); + await context.sessionContext.reloadAgentSchema(); + context.abortSignal?.throwIfAborted(); + } catch (error) { + await flowStore.deleteFlow(promoted); + if (context.abortSignal?.aborted) { + context.abortSignal.throwIfAborted(); + } + return createPowerShellFailure( + "partialSideEffects", + `The script executed, but the new flow could not be activated: ${error instanceof Error ? error.message : String(error)}. The operation may have caused side effects and was not executed again.`, + ); + } + + return createActionResultFromTextDisplay( + `${execution.output}\n\nCreated reusable PowerShell flow '${promoted}'.`, + ); + }); +} + +async function repairAndExecutePowerShellFlow( + params: Record, + flowStore: PowerShellStore, + context: ActionContext, +): Promise { + const flowName = params.flowName as string | undefined; + const script = params.script as string | undefined; + if (!flowName || !script) { + return createPowerShellFailure( + "invalidParameters", + "Missing required parameter: flowName or script", + ); + } + const repairKey = context.abortSignal ?? context; + if (repairAttempts.has(repairKey)) { + return createPowerShellFailure( + "policyDenied", + "A PowerShell flow repair was already attempted for this request.", + { retryable: false }, + ); + } + repairAttempts.add(repairKey); + const executionParameters = parseNamedParameters( + params.executionParametersJson, + "executionParametersJson", + ); + if ("error" in executionParameters) { + return executionParameters.error; + } + + return withFlowMutationLock(flowName, async () => { + context.abortSignal?.throwIfAborted(); + const existing = await flowStore.getFlow(flowName); + const oldScript = await flowStore.getScript(flowName); + if (!existing || !oldScript) { + return createPowerShellFailure( + "unknownFlow", + `Unknown PowerShell flow '${flowName}'.`, + ); + } + const candidate: ScriptRecipe = { + version: 1, + actionName: existing.actionName, + displayName: existing.displayName, + description: existing.description, + parameters: existing.parameters, + script: { + language: "powershell", + body: script, + expectedOutputFormat: existing.expectedOutputFormat, + }, + grammarPatterns: existing.grammarPatterns, + sandbox: { + ...existing.sandbox, + allowedCmdlets: + (params.allowedCmdlets as string[]) ?? + existing.sandbox.allowedCmdlets, + allowedModules: + (params.allowedModules as string[]) ?? + existing.sandbox.allowedModules, + }, + ...(existing.source ? { source: existing.source } : {}), + }; + const execution = await executeDraftRecipe( + candidate, + executionParameters.parameters, + context.abortSignal, + ); + if ("error" in execution) { + return execution.error; + } + context.abortSignal?.throwIfAborted(); + await flowStore.updateFlowScript( + flowName, + script, + candidate.sandbox.allowedCmdlets, + candidate.sandbox.allowedModules, + ); + try { + context.abortSignal?.throwIfAborted(); + await context.sessionContext.reloadAgentSchema(); + context.abortSignal?.throwIfAborted(); + } catch (error) { + await flowStore.updateFlowScript( + flowName, + oldScript, + existing.sandbox.allowedCmdlets, + existing.sandbox.allowedModules, + ); + if (context.abortSignal?.aborted) { + context.abortSignal.throwIfAborted(); + } + return createPowerShellFailure( + "partialSideEffects", + `The repaired script executed, but the flow could not be activated: ${error instanceof Error ? error.message : String(error)}.`, + ); + } + await flowStore.recordUsage(flowName); + return createActionResultFromTextDisplay( + `${execution.output}\n\nRepaired PowerShell flow '${flowName}' after one retry.`, + ); + }); +} + async function handlePowerShellFlowAction( - action: { - schemaName?: string; - actionName: string; - parameters?: Record; - }, + action: PowerShellAction, context: ActionContext, ): Promise { - const networkResult = await executeStaticNetworkAction(action); - if (networkResult) { - return networkResult; + context.abortSignal?.throwIfAborted(); + const namespaceResult = await executeNamespaceAction(action, context); + if (namespaceResult !== undefined) { + return namespaceResult; } const flowStore = (context as any).__store as PowerShellStore | undefined; @@ -595,8 +762,16 @@ async function handlePowerShellFlowAction( grammarValidation.patterns, ); + context.abortSignal?.throwIfAborted(); await flowStore.saveFlow(recipe, "reasoning"); - await context.sessionContext.reloadAgentSchema(); + try { + context.abortSignal?.throwIfAborted(); + await context.sessionContext.reloadAgentSchema(); + context.abortSignal?.throwIfAborted(); + } catch (error) { + await flowStore.deleteFlow(newActionName); + throw error; + } return createActionResultFromTextDisplay( `Created PowerShell flow '${newActionName}': ${recipe.description}`, ); @@ -608,76 +783,11 @@ async function handlePowerShellFlowAction( "Script flow store not available", ); } - const params = action.parameters as Record; - const actionName = params.actionName as string | undefined; - const script = params.script as string | undefined; - if (!actionName) { - return createActionResultFromError( - "Missing required parameter: actionName", - ); - } - if (!script) { - return createActionResultFromError( - "Missing required parameter: script", - ); - } - if (flowStore.hasFlow(actionName)) { - return createActionResultFromError( - `A PowerShell flow named '${actionName}' already exists. Reuse it or add grammar patterns instead of creating a duplicate.`, - ); - } - - const grammarValidation = await validateFlowGrammarPatterns( - actionName, - (params.description as string) ?? "", - (params.grammarPatterns as FlowGrammarPatternInput[]) ?? [], + return createOrReusePowerShellFlow( + action.parameters as Record, + flowStore, context, ); - if ("error" in grammarValidation) { - return grammarValidation.error; - } - const executionParameters = parseNamedParameters( - params.executionParametersJson, - "executionParametersJson", - ); - if ("error" in executionParameters) { - return executionParameters.error; - } - - const recipe = buildPowerShellRecipe( - params, - grammarValidation.patterns, - ); - const pendingId = await flowStore.savePending(recipe); - const execution = await executeDraftRecipe( - recipe, - executionParameters.parameters, - ); - if ("error" in execution) { - await flowStore.deletePending(`${pendingId}.recipe.json`); - return execution.error; - } - - const pendingFile = `${pendingId}.recipe.json`; - const promoted = await flowStore.promotePending(pendingFile); - if (!promoted) { - await flowStore.deletePending(pendingFile); - return createActionResultFromError( - `The script executed, but flow '${actionName}' could not be promoted because that name is already registered. The operation may have caused side effects and was not executed again.`, - ); - } - try { - await context.sessionContext.reloadAgentSchema(); - } catch (error) { - await flowStore.deleteFlow(promoted); - return createActionResultFromError( - `The script executed, but the new flow could not be activated: ${error instanceof Error ? error.message : String(error)}. The operation may have caused side effects and was not executed again.`, - ); - } - - return createActionResultFromTextDisplay( - `${execution.output}\n\nCreated reusable PowerShell flow '${promoted}'.`, - ); } case "addPowerShellFlowPatterns": { @@ -731,6 +841,19 @@ async function handlePowerShellFlowAction( "PowerShell capability outcome reported.", ); + case "repairAndExecutePowerShellFlow": { + if (!flowStore) { + return createActionResultFromError( + "Script flow store not available", + ); + } + return repairAndExecutePowerShellFlow( + action.parameters as Record, + flowStore, + context, + ); + } + case "editPowerShellFlow": { if (!flowStore) { return createActionResultFromError( @@ -817,9 +940,13 @@ async function handlePowerShellFlowAction( networkAccess, }, workingDirectory: homedir(), + abortSignal: context.abortSignal, }; const result = await executeScript(request); + if (result.cancelled) { + context.abortSignal?.throwIfAborted(); + } if (result.success) { const output = result.stdout.trim() || "(no output)"; @@ -850,10 +977,10 @@ async function handlePowerShellFlowAction( const flow = await flowStore.getFlow(flowName); if (!flow) { - return { - error: `Unknown PowerShell flow '${flowName}'. Use 'listPowerShellFlows' to see available flows.`, - fallbackToReasoning: true, - }; + return createPowerShellFailure( + "unknownFlow", + `Unknown PowerShell flow '${flowName}'. Use 'listPowerShellFlows' to see available flows.`, + ); } const script = await flowStore.getScript(flowName); @@ -903,7 +1030,7 @@ async function handlePowerShellFlowAction( flow.parameters, ); if (pathError) { - return { error: pathError, fallbackToReasoning: true }; + return createPowerShellFailure("invalidParameters", pathError); } // Validate parameter validation rules (pattern, allowedValues) @@ -912,13 +1039,17 @@ async function handlePowerShellFlowAction( flow.parameters, ); if (validationError) { - return { error: validationError, fallbackToReasoning: true }; + return createPowerShellFailure( + "invalidParameters", + validationError, + ); } const result = await executeFlowScript( flow, script, flowParameters, + context.abortSignal, ); if (result.error !== undefined) { return { ...result, fallbackToReasoning: true }; @@ -993,8 +1124,16 @@ async function handlePowerShellFlowAction( ); } + context.abortSignal?.throwIfAborted(); await flowStore.saveFlow(recipe, "manual"); - await context.sessionContext.reloadAgentSchema(); + try { + context.abortSignal?.throwIfAborted(); + await context.sessionContext.reloadAgentSchema(); + context.abortSignal?.throwIfAborted(); + } catch (error) { + await flowStore.deleteFlow(recipe.actionName); + throw error; + } const patternList = recipe.grammarPatterns .map((p) => ` "${p.pattern}"`) @@ -1013,10 +1152,10 @@ async function handlePowerShellFlowAction( const flow = await flowStore.getFlow(action.actionName); if (!flow) { - return { - error: `Unknown PowerShell flow '${action.actionName}'. Use 'list PowerShell flows' to see available flows.`, - fallbackToReasoning: true, - }; + return createPowerShellFailure( + "unknownFlow", + `Unknown PowerShell flow '${action.actionName}'. Use 'list PowerShell flows' to see available flows.`, + ); } const script = await flowStore.getScript(action.actionName); @@ -1033,7 +1172,7 @@ async function handlePowerShellFlowAction( flow.parameters, ); if (pathError) { - return { error: pathError, fallbackToReasoning: true }; + return createPowerShellFailure("invalidParameters", pathError); } // Validate parameter validation rules (pattern, allowedValues) @@ -1042,10 +1181,18 @@ async function handlePowerShellFlowAction( flow.parameters, ); if (validationError) { - return { error: validationError, fallbackToReasoning: true }; + return createPowerShellFailure( + "invalidParameters", + validationError, + ); } - const result = await executeFlowScript(flow, script, directParams); + const result = await executeFlowScript( + flow, + script, + directParams, + context.abortSignal, + ); if (result.error !== undefined) { return { ...result, fallbackToReasoning: true }; } @@ -1368,6 +1515,7 @@ const POWERSHELL_BUILTIN_ACTIONS = new Set([ "createAndExecutePowerShellFlow", "addPowerShellFlowPatterns", "reportPowerShellCapabilityOutcome", + "repairAndExecutePowerShellFlow", "editPowerShellFlow", "importPowerShellFlow", ]); @@ -1415,11 +1563,7 @@ export function instantiate(): AppAgent { executeAction(action, context: ActionContext) { (context as any).__store = agentContext.store; return handlePowerShellFlowAction( - action as { - schemaName?: string; - actionName: string; - parameters?: Record; - }, + action as PowerShellAction, context, ); }, diff --git a/ts/packages/agents/powershell/src/execution/powershellRunner.mts b/ts/packages/agents/powershell/src/execution/powershellRunner.mts index c2ce0c1fd4..d64327a732 100644 --- a/ts/packages/agents/powershell/src/execution/powershellRunner.mts +++ b/ts/packages/agents/powershell/src/execution/powershellRunner.mts @@ -44,6 +44,7 @@ export interface ScriptExecutionRequest { networkAccess: boolean; }; workingDirectory?: string; + abortSignal?: AbortSignal | undefined; } export interface ScriptExecutionResult { @@ -53,11 +54,13 @@ export interface ScriptExecutionResult { exitCode: number; duration: number; truncated: boolean; + cancelled: boolean; } export async function executeScript( request: ScriptExecutionRequest, ): Promise { + request.abortSignal?.throwIfAborted(); const scriptHostPath = join(packageRoot, "scripts", "scriptHost.ps1"); const args = [ @@ -95,6 +98,7 @@ export async function executeScript( let stderr = ""; let truncated = false; let resolved = false; + let cancelled = false; const child = spawn("powershell", args, { cwd: request.workingDirectory, @@ -118,6 +122,7 @@ export async function executeScript( if (!resolved) { resolved = true; child.kill("SIGTERM"); + request.abortSignal?.removeEventListener("abort", onAbort); resolve({ success: false, stdout, @@ -125,21 +130,37 @@ export async function executeScript( exitCode: -1, duration: Date.now() - startTime, truncated, + cancelled: false, }); } }, request.sandbox.maxExecutionTime * 1000); + const onAbort = () => { + if (resolved) { + return; + } + cancelled = true; + child.kill("SIGTERM"); + }; + request.abortSignal?.addEventListener("abort", onAbort, { + once: true, + }); + child.on("close", (code) => { if (!resolved) { resolved = true; clearTimeout(timeout); + request.abortSignal?.removeEventListener("abort", onAbort); resolve({ - success: code === 0, + success: !cancelled && code === 0, stdout, - stderr, + stderr: cancelled + ? "PowerShell execution was cancelled." + : stderr, exitCode: code ?? -1, duration: Date.now() - startTime, truncated, + cancelled, }); } }); @@ -148,6 +169,7 @@ export async function executeScript( if (!resolved) { resolved = true; clearTimeout(timeout); + request.abortSignal?.removeEventListener("abort", onAbort); resolve({ success: false, stdout, @@ -155,6 +177,7 @@ export async function executeScript( exitCode: -1, duration: Date.now() - startTime, truncated, + cancelled: false, }); } }); diff --git a/ts/packages/agents/powershell/src/manifest.json b/ts/packages/agents/powershell/src/manifest.json index 75d50b61cf..f07693cc83 100644 --- a/ts/packages/agents/powershell/src/manifest.json +++ b/ts/packages/agents/powershell/src/manifest.json @@ -11,6 +11,46 @@ "schemaType": "PowerShellActions" }, "subActionManifests": { + "powershell-files": { + "defaultEnabled": true, + "schema": { + "description": "PowerShell actions for listing, reading, writing, copying, moving, deleting, searching, and creating files and directories.", + "originalSchemaFile": "./namespaces/files/filesActionsSchema.mts", + "schemaFile": "../dist/filesSchema.pas.json", + "grammarFile": "../dist/filesSchema.ag.json", + "schemaType": "PowerShellFilesActions" + } + }, + "powershell-processes": { + "defaultEnabled": true, + "schema": { + "description": "PowerShell actions for listing, inspecting, starting, stopping, and waiting for processes.", + "originalSchemaFile": "./namespaces/processes/processesActionsSchema.mts", + "schemaFile": "../dist/processesSchema.pas.json", + "grammarFile": "../dist/processesSchema.ag.json", + "schemaType": "PowerShellProcessesActions" + } + }, + "powershell-system": { + "defaultEnabled": true, + "schema": { + "description": "PowerShell actions for system information, disk usage, updates, uptime, and environment variables.", + "originalSchemaFile": "./namespaces/system/systemActionsSchema.mts", + "schemaFile": "../dist/systemSchema.pas.json", + "grammarFile": "../dist/systemSchema.ag.json", + "schemaType": "PowerShellSystemActions" + } + }, + "powershell-services": { + "defaultEnabled": true, + "schema": { + "description": "PowerShell actions for listing, inspecting, starting, stopping, and restarting Windows services.", + "originalSchemaFile": "./namespaces/services/servicesActionsSchema.mts", + "schemaFile": "../dist/servicesSchema.pas.json", + "grammarFile": "../dist/servicesSchema.ag.json", + "schemaType": "PowerShellServicesActions" + } + }, "powershell-network": { "defaultEnabled": true, "schema": { @@ -20,6 +60,26 @@ "grammarFile": "../dist/networkSchema.ag.json", "schemaType": "PowerShellNetworkActions" } + }, + "powershell-data": { + "defaultEnabled": true, + "schema": { + "description": "PowerShell actions for reading, writing, filtering, and converting JSON and CSV data.", + "originalSchemaFile": "./namespaces/data/dataActionsSchema.mts", + "schemaFile": "../dist/dataSchema.pas.json", + "grammarFile": "../dist/dataSchema.ag.json", + "schemaType": "PowerShellDataActions" + } + }, + "powershell-archives": { + "defaultEnabled": true, + "schema": { + "description": "PowerShell actions for compressing and extracting ZIP archives.", + "originalSchemaFile": "./namespaces/archives/archivesActionsSchema.mts", + "schemaFile": "../dist/archivesSchema.pas.json", + "grammarFile": "../dist/archivesSchema.ag.json", + "schemaType": "PowerShellArchivesActions" + } } } } diff --git a/ts/packages/agents/powershell/src/namespaces/actionHandlerRegistry.mts b/ts/packages/agents/powershell/src/namespaces/actionHandlerRegistry.mts new file mode 100644 index 0000000000..c3371d7ffe --- /dev/null +++ b/ts/packages/agents/powershell/src/namespaces/actionHandlerRegistry.mts @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { ActionContext, ActionResult } from "@typeagent/agent-sdk"; +import type { PowerShellAgentContext } from "../types/powerShellAgentContext.mjs"; +import { archivesActionHandler } from "./archives/actionHandler.mjs"; +import { dataActionHandler } from "./data/actionHandler.mjs"; +import { filesActionHandler } from "./files/actionHandler.mjs"; +import { networkActionHandler } from "./network/actionHandler.mjs"; +import type { + PowerShellAction, + PowerShellNamespaceActionHandler, +} from "./namespaceActionHandler.mjs"; +import { processesActionHandler } from "./processes/actionHandler.mjs"; +import { servicesActionHandler } from "./services/actionHandler.mjs"; +import { systemActionHandler } from "./system/actionHandler.mjs"; + +const handlers = [ + archivesActionHandler, + dataActionHandler, + filesActionHandler, + networkActionHandler, + processesActionHandler, + servicesActionHandler, + systemActionHandler, +] as const; + +const handlersBySchema = new Map(); +for (const handler of handlers) { + if (handlersBySchema.has(handler.schemaName)) { + throw new Error( + `Duplicate PowerShell namespace handler: ${handler.schemaName}`, + ); + } + handlersBySchema.set(handler.schemaName, handler); +} + +export async function executeNamespaceAction( + action: PowerShellAction, + context: ActionContext, +): Promise { + if (!action.schemaName) { + return undefined; + } + return handlersBySchema.get(action.schemaName)?.execute(action, context); +} + +export function hasNamespaceAction( + schemaName: string, + actionName: string, +): boolean { + return handlersBySchema.get(schemaName)?.hasAction(actionName) ?? false; +} + +export function getRegisteredNamespaceActions(): ReadonlyMap< + string, + readonly string[] +> { + return new Map( + [...handlersBySchema].map(([schemaName, handler]) => [ + schemaName, + handler.actionNames, + ]), + ); +} diff --git a/ts/packages/agents/powershell/src/namespaces/archives/actionHandler.mts b/ts/packages/agents/powershell/src/namespaces/archives/actionHandler.mts new file mode 100644 index 0000000000..0d4854796f --- /dev/null +++ b/ts/packages/agents/powershell/src/namespaces/archives/actionHandler.mts @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { PowerShellArchivesActions } from "./archivesActionsSchema.mjs"; +import { + createPowerShellNamespaceActionHandler, + type NamespaceActionDefinitions, +} from "../namespaceActionHandler.mjs"; + +const allowedPaths = ["$env:USERPROFILE", "$PWD", "$env:TEMP"] as const; + +const definitions = { + compress: { + script: `param([string]$SourcePath, [string]$DestinationPath) +Add-Type -AssemblyName System.IO.Compression +Add-Type -AssemblyName System.IO.Compression.FileSystem +if (-not $DestinationPath) { $DestinationPath = "$SourcePath.zip" } +if ([System.IO.Directory]::Exists($SourcePath)) { + [System.IO.Compression.ZipFile]::CreateFromDirectory($SourcePath, $DestinationPath) +} else { + $archive = [System.IO.Compression.ZipFile]::Open( + $DestinationPath, + [System.IO.Compression.ZipArchiveMode]::Create + ) + try { + [System.IO.Compression.ZipFileExtensions]::CreateEntryFromFile( + $archive, + $SourcePath, + [System.IO.Path]::GetFileName($SourcePath) + ) | Out-Null + } finally { + $archive.Dispose() + } +}`, + allowedCmdlets: ["Add-Type", "Out-Null"], + allowedPaths, + confirmation: "Create the requested ZIP archive?", + }, + expand: { + script: `param([string]$ArchivePath, [string]$DestinationPath) +Add-Type -AssemblyName System.IO.Compression +Add-Type -AssemblyName System.IO.Compression.FileSystem +if (-not $DestinationPath) { $DestinationPath = "." } +[System.IO.Compression.ZipFile]::ExtractToDirectory($ArchivePath, $DestinationPath)`, + allowedCmdlets: ["Add-Type"], + allowedPaths, + confirmation: "Extract the requested archive?", + }, +} satisfies NamespaceActionDefinitions; + +export const archivesActionHandler = + createPowerShellNamespaceActionHandler( + "powershell.powershell-archives", + definitions, + ); diff --git a/ts/packages/agents/powershell/src/namespaces/data/actionHandler.mts b/ts/packages/agents/powershell/src/namespaces/data/actionHandler.mts new file mode 100644 index 0000000000..9e357a0183 --- /dev/null +++ b/ts/packages/agents/powershell/src/namespaces/data/actionHandler.mts @@ -0,0 +1,83 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { PowerShellDataActions } from "./dataActionsSchema.mjs"; +import { + createPowerShellNamespaceActionHandler, + type NamespaceActionDefinitions, +} from "../namespaceActionHandler.mjs"; + +const allowedPaths = ["$env:USERPROFILE", "$PWD", "$env:TEMP"] as const; + +const definitions = { + readJson: { + script: `param([string]$Path, [string]$PropertyPath) +$value = Get-Content -LiteralPath $Path -Raw | ConvertFrom-Json +if ($PropertyPath) { + foreach ($part in $PropertyPath.Split(".")) { $value = $value.$part } +} +$value`, + allowedCmdlets: ["Get-Content", "ConvertFrom-Json"], + allowedPaths, + }, + writeJson: { + script: `param([string]$Path, [string]$Data) +$Data | ConvertFrom-Json | ConvertTo-Json -Depth 20 | Set-Content -LiteralPath $Path`, + allowedCmdlets: ["ConvertFrom-Json", "ConvertTo-Json", "Set-Content"], + allowedPaths, + confirmation: "Write JSON data to the requested file?", + }, + readCsv: { + script: `param([string]$Path, [string]$Delimiter) +if (-not $Delimiter) { $Delimiter = "," } +Import-Csv -LiteralPath $Path -Delimiter $Delimiter`, + allowedCmdlets: ["Import-Csv"], + allowedPaths, + }, + writeCsv: { + script: `param([string]$Path, [string]$Data) +$Data | ConvertFrom-Json | Export-Csv -LiteralPath $Path -NoTypeInformation`, + allowedCmdlets: ["ConvertFrom-Json", "Export-Csv"], + allowedPaths, + confirmation: "Write CSV data to the requested file?", + }, + filterCsv: { + script: `param([string]$Path, [string]$Column, [string]$Pattern) +Import-Csv -LiteralPath $Path | Where-Object { $_.$Column -match $Pattern }`, + allowedCmdlets: ["Import-Csv", "Where-Object"], + allowedPaths, + }, + convertFormat: { + script: `param([string]$Input, [string]$Format) +$extension = [System.IO.Path]::GetExtension($Input).ToLowerInvariant() +$value = if ($extension -eq ".csv") { + Import-Csv -LiteralPath $Input +} elseif ($extension -eq ".json") { + Get-Content -LiteralPath $Input -Raw | ConvertFrom-Json +} else { + Get-Content -LiteralPath $Input -Raw +} +if ($Format -eq "json") { + $value | ConvertTo-Json -Depth 20 +} elseif ($Format -eq "csv") { + $value | ConvertTo-Csv -NoTypeInformation +} else { + $value | ConvertTo-Xml -As String -Depth 20 +}`, + allowedCmdlets: [ + "Import-Csv", + "Get-Content", + "ConvertFrom-Json", + "ConvertTo-Json", + "ConvertTo-Csv", + "ConvertTo-Xml", + ], + allowedPaths, + }, +} satisfies NamespaceActionDefinitions; + +export const dataActionHandler = + createPowerShellNamespaceActionHandler( + "powershell.powershell-data", + definitions, + ); diff --git a/ts/packages/agents/powershell/src/namespaces/data/dataSchema.agr b/ts/packages/agents/powershell/src/namespaces/data/dataSchema.agr index a2486f9502..0071bcae01 100644 --- a/ts/packages/agents/powershell/src/namespaces/data/dataSchema.agr +++ b/ts/packages/agents/powershell/src/namespaces/data/dataSchema.agr @@ -18,7 +18,7 @@ import { PowerShellDataActions } from "./dataActionsSchema.mts"; -> { actionName: "readJson", parameters: { path } } | (read|get) $(propertyPath:wildcard) from (json)? $(path:wildcard) -> { actionName: "readJson", parameters: { path, propertyPath } } - | (show|display) (json)? (file)? $(path:wildcard) + | (show|display) (json|json file) $(path:wildcard) -> { actionName: "readJson", parameters: { path } }; = diff --git a/ts/packages/agents/powershell/src/namespaces/files/actionHandler.mts b/ts/packages/agents/powershell/src/namespaces/files/actionHandler.mts new file mode 100644 index 0000000000..a7784ca0e3 --- /dev/null +++ b/ts/packages/agents/powershell/src/namespaces/files/actionHandler.mts @@ -0,0 +1,99 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { PowerShellFilesActions } from "./filesActionsSchema.mjs"; +import { + createPowerShellNamespaceActionHandler, + type NamespaceActionDefinitions, +} from "../namespaceActionHandler.mjs"; + +const allowedPaths = ["$env:USERPROFILE", "$PWD", "$env:TEMP"] as const; + +const definitions = { + listFiles: { + script: `param([string]$Path, [string]$Filter, [bool]$Recurse) +if (-not $Path) { $Path = "." } +if ($Filter) { + Get-ChildItem -Path $Path -Filter $Filter -Recurse:$Recurse +} else { + Get-ChildItem -Path $Path -Recurse:$Recurse +}`, + allowedCmdlets: ["Get-ChildItem"], + allowedPaths, + }, + readFile: { + script: `param([string]$Path, [int]$Tail, [int]$Head) +if ($Tail -gt 0) { + Get-Content -LiteralPath $Path -Tail $Tail +} elseif ($Head -gt 0) { + Get-Content -LiteralPath $Path -TotalCount $Head +} else { + Get-Content -LiteralPath $Path +}`, + allowedCmdlets: ["Get-Content"], + allowedPaths, + }, + writeFile: { + script: `param([string]$Path, [string]$Content, [bool]$Append) +if ($Append) { + Add-Content -LiteralPath $Path -Value $Content +} else { + Set-Content -LiteralPath $Path -Value $Content +}`, + allowedCmdlets: ["Add-Content", "Set-Content"], + allowedPaths, + confirmation: "Write content to the requested file?", + }, + copyFile: { + script: `param([string]$Source, [string]$Destination, [bool]$Recurse) +Copy-Item -LiteralPath $Source -Destination $Destination -Recurse:$Recurse`, + allowedCmdlets: ["Copy-Item"], + allowedPaths, + confirmation: "Copy the requested file or directory?", + }, + moveFile: { + script: `param([string]$Source, [string]$Destination) +Move-Item -LiteralPath $Source -Destination $Destination`, + allowedCmdlets: ["Move-Item"], + allowedPaths, + confirmation: "Move or rename the requested file or directory?", + }, + deleteFile: { + script: `param([string]$Path, [bool]$Recurse) +Remove-Item -LiteralPath $Path -Recurse:$Recurse`, + allowedCmdlets: ["Remove-Item"], + allowedPaths, + confirmation: "Delete the requested file or directory?", + }, + testPath: { + script: `param([string]$Path) +Test-Path -LiteralPath $Path`, + allowedCmdlets: ["Test-Path"], + allowedPaths, + }, + findText: { + script: `param([string]$Pattern, [string]$Path, [string]$Include) +if (-not $Path) { $Path = "." } +if ($Include) { + Get-ChildItem -Path $Path -Filter $Include -File -Recurse | Select-String -Pattern $Pattern +} else { + Get-ChildItem -Path $Path -File -Recurse | Select-String -Pattern $Pattern +}`, + allowedCmdlets: ["Get-ChildItem", "Select-String"], + allowedPaths, + }, + newItem: { + script: `param([string]$Path, [string]$ItemType) +$type = if ($ItemType -eq "directory") { "Directory" } else { "File" } +New-Item -Path $Path -ItemType $type`, + allowedCmdlets: ["New-Item"], + allowedPaths, + confirmation: "Create the requested file or directory?", + }, +} satisfies NamespaceActionDefinitions; + +export const filesActionHandler = + createPowerShellNamespaceActionHandler( + "powershell.powershell-files", + definitions, + ); diff --git a/ts/packages/agents/powershell/src/namespaces/files/filesSchema.agr b/ts/packages/agents/powershell/src/namespaces/files/filesSchema.agr index 8126d8774b..62d49e8d3d 100644 --- a/ts/packages/agents/powershell/src/namespaces/files/filesSchema.agr +++ b/ts/packages/agents/powershell/src/namespaces/files/filesSchema.agr @@ -29,7 +29,9 @@ import { PowerShellFilesActions } from "./filesActionsSchema.mts"; -> { actionName: "listFiles", parameters: { path } }; = - (read|show|display|cat|type) (the)? (contents of|file)? $(path:wildcard) + (read|cat|type) (the)? (contents of|file)? $(path:wildcard) + -> { actionName: "readFile", parameters: { path } } + | (show|display) (the)? (contents of|file) $(path:wildcard) -> { actionName: "readFile", parameters: { path } } | (show|get) (the)? (last|tail) $(tail:number) lines (of|from|in) $(path:wildcard) -> { actionName: "readFile", parameters: { path, tail } } diff --git a/ts/packages/agents/powershell/src/namespaces/namespaceActionHandler.mts b/ts/packages/agents/powershell/src/namespaces/namespaceActionHandler.mts new file mode 100644 index 0000000000..fcc821f044 --- /dev/null +++ b/ts/packages/agents/powershell/src/namespaces/namespaceActionHandler.mts @@ -0,0 +1,113 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { ActionContext, ActionResult } from "@typeagent/agent-sdk"; +import { createActionResultFromTextDisplay } from "@typeagent/agent-sdk/helpers/action"; +import { homedir } from "os"; +import { executeScript } from "../execution/powershellRunner.mjs"; +import type { PowerShellAgentContext } from "../types/powerShellAgentContext.mjs"; +import { + createPowerShellExecutionFailure, + createPowerShellFailure, +} from "../types/powerShellFailure.mjs"; + +export type PowerShellAction = { + schemaName?: string; + actionName: string; + parameters?: Record; +}; + +export type StaticPowerShellActionDefinition = { + script: string; + allowedCmdlets: readonly string[]; + allowedPaths?: readonly string[]; + allowedModules?: readonly string[]; + networkAccess?: boolean; + maxExecutionTime?: number; + confirmation?: string; +}; + +export type NamespaceActionDefinitions = + { + [Name in TAction["actionName"]]: StaticPowerShellActionDefinition; + }; + +export interface PowerShellNamespaceActionHandler { + readonly schemaName: string; + readonly actionNames: readonly string[]; + hasAction(actionName: string): boolean; + execute( + action: PowerShellAction, + context: ActionContext, + ): Promise; +} + +export function createPowerShellNamespaceActionHandler< + TAction extends { actionName: string }, +>( + schemaName: string, + definitions: NamespaceActionDefinitions, +): PowerShellNamespaceActionHandler { + const actionDefinitions = definitions as Record< + string, + StaticPowerShellActionDefinition + >; + const actionNames = Object.freeze(Object.keys(actionDefinitions)); + + return { + schemaName, + actionNames, + hasAction(actionName: string): boolean { + return actionDefinitions[actionName] !== undefined; + }, + async execute( + action: PowerShellAction, + context: ActionContext, + ): Promise { + if (action.schemaName !== schemaName) { + return undefined; + } + const definition = actionDefinitions[action.actionName]; + if (!definition) { + return undefined; + } + if (definition.confirmation) { + const choice = await context.sessionContext.popupQuestion( + definition.confirmation, + ["Run", "Cancel"], + 1, + ); + if (choice !== 0) { + return createPowerShellFailure( + "policyDenied", + "The PowerShell action was not approved.", + { retryable: false }, + ); + } + } + + const result = await executeScript({ + script: definition.script, + parameters: action.parameters ?? {}, + sandbox: { + allowedCmdlets: [...definition.allowedCmdlets], + allowedPaths: [...(definition.allowedPaths ?? [])], + allowedModules: [...(definition.allowedModules ?? [])], + maxExecutionTime: definition.maxExecutionTime ?? 30, + networkAccess: definition.networkAccess ?? false, + }, + workingDirectory: homedir(), + abortSignal: context.abortSignal, + }); + if (result.cancelled) { + context.abortSignal?.throwIfAborted(); + } + if (!result.success) { + return createPowerShellExecutionFailure(result); + } + return createActionResultFromTextDisplay( + result.stdout.trim() || "(no output)", + ); + }, + }; +} diff --git a/ts/packages/agents/powershell/src/namespaces/network/actionHandler.mts b/ts/packages/agents/powershell/src/namespaces/network/actionHandler.mts new file mode 100644 index 0000000000..e6f91c7844 --- /dev/null +++ b/ts/packages/agents/powershell/src/namespaces/network/actionHandler.mts @@ -0,0 +1,83 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { PowerShellNetworkActions } from "./networkActionsSchema.mjs"; +import { + createPowerShellNamespaceActionHandler, + type NamespaceActionDefinitions, +} from "../namespaceActionHandler.mjs"; + +const definitions = { + testConnection: { + script: `param([string]$ComputerName, [int]$Port) +if ($Port -gt 0) { + Test-NetConnection -ComputerName $ComputerName -Port $Port +} else { + Test-NetConnection -ComputerName $ComputerName +}`, + allowedCmdlets: ["Test-NetConnection"], + networkAccess: true, + }, + portListeners: { + script: `param([int]$Port) +$listeners = Get-NetTCPConnection -State Listen -ErrorAction SilentlyContinue +if ($Port -gt 0) { + $listeners = $listeners | Where-Object { $_.LocalPort -eq $Port } +} +$listeners | + Sort-Object LocalPort, OwningProcess | + ForEach-Object { + $process = Get-Process -Id $_.OwningProcess -ErrorAction SilentlyContinue + [PSCustomObject]@{ + LocalAddress = $_.LocalAddress + LocalPort = $_.LocalPort + ProcessId = $_.OwningProcess + ProcessName = if ($process) { $process.ProcessName } else { "(unknown)" } + } + }`, + allowedCmdlets: [ + "Get-NetTCPConnection", + "Where-Object", + "Sort-Object", + "ForEach-Object", + "Get-Process", + ], + networkAccess: true, + }, + networkAdapters: { + script: `param([string]$Name) +if ($Name) { + Get-NetAdapter -Name $Name +} else { + Get-NetAdapter +}`, + allowedCmdlets: ["Get-NetAdapter"], + networkAccess: true, + }, + ipConfig: { + script: `param([string]$InterfaceAlias) +if ($InterfaceAlias) { + Get-NetIPConfiguration -InterfaceAlias $InterfaceAlias +} else { + Get-NetIPConfiguration +}`, + allowedCmdlets: ["Get-NetIPConfiguration"], + networkAccess: true, + }, + dnsLookup: { + script: `param([string]$Name, [string]$Type) +if ($Type) { + Resolve-DnsName -Name $Name -Type $Type +} else { + Resolve-DnsName -Name $Name +}`, + allowedCmdlets: ["Resolve-DnsName"], + networkAccess: true, + }, +} satisfies NamespaceActionDefinitions; + +export const networkActionHandler = + createPowerShellNamespaceActionHandler( + "powershell.powershell-network", + definitions, + ); diff --git a/ts/packages/agents/powershell/src/namespaces/processes/actionHandler.mts b/ts/packages/agents/powershell/src/namespaces/processes/actionHandler.mts new file mode 100644 index 0000000000..f72158d766 --- /dev/null +++ b/ts/packages/agents/powershell/src/namespaces/processes/actionHandler.mts @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { PowerShellProcessesActions } from "./processesActionsSchema.mjs"; +import { + createPowerShellNamespaceActionHandler, + type NamespaceActionDefinitions, +} from "../namespaceActionHandler.mjs"; + +const definitions = { + listProcesses: { + script: `param([string]$Name, [int]$TopN) +$items = if ($Name) { Get-Process -Name $Name } else { Get-Process } +if ($TopN -gt 0) { $items | Select-Object -First $TopN } else { $items }`, + allowedCmdlets: ["Get-Process", "Select-Object"], + }, + processMemory: { + script: `param([int]$TopN, [string]$Name) +if ($TopN -le 0) { $TopN = 10 } +$items = if ($Name) { Get-Process -Name $Name } else { Get-Process } +$items | Sort-Object WorkingSet64 -Descending | Select-Object -First $TopN Name, Id, WorkingSet64`, + allowedCmdlets: ["Get-Process", "Sort-Object", "Select-Object"], + }, + processCpu: { + script: `param([int]$TopN, [string]$Name) +if ($TopN -le 0) { $TopN = 10 } +$items = if ($Name) { Get-Process -Name $Name } else { Get-Process } +$items | Sort-Object CPU -Descending | Select-Object -First $TopN Name, Id, CPU`, + allowedCmdlets: ["Get-Process", "Sort-Object", "Select-Object"], + }, + stopProcess: { + script: `param([string]$Name, [int]$Id) +if ($Id -gt 0) { Stop-Process -Id $Id } else { Stop-Process -Name $Name }`, + allowedCmdlets: ["Stop-Process"], + confirmation: "Stop the requested process?", + }, + startProcess: { + script: `param([string]$Path, [string]$Arguments) +if ($Arguments) { Start-Process -FilePath $Path -ArgumentList $Arguments } else { Start-Process -FilePath $Path }`, + allowedCmdlets: ["Start-Process"], + allowedPaths: ["$env:USERPROFILE", "$PWD", "$env:TEMP"], + confirmation: "Start the requested process?", + }, + waitProcess: { + script: `param([string]$Name, [int]$Id) +if ($Id -gt 0) { Wait-Process -Id $Id } else { Wait-Process -Name $Name }`, + allowedCmdlets: ["Wait-Process"], + }, +} satisfies NamespaceActionDefinitions; + +export const processesActionHandler = + createPowerShellNamespaceActionHandler( + "powershell.powershell-processes", + definitions, + ); diff --git a/ts/packages/agents/powershell/src/namespaces/services/actionHandler.mts b/ts/packages/agents/powershell/src/namespaces/services/actionHandler.mts new file mode 100644 index 0000000000..76603e4bd0 --- /dev/null +++ b/ts/packages/agents/powershell/src/namespaces/services/actionHandler.mts @@ -0,0 +1,46 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { PowerShellServicesActions } from "./servicesActionsSchema.mjs"; +import { + createPowerShellNamespaceActionHandler, + type NamespaceActionDefinitions, +} from "../namespaceActionHandler.mjs"; + +const definitions = { + listServices: { + script: `param([string]$Name, [string]$Status) +$services = if ($Name) { Get-Service -Name $Name } else { Get-Service } +if ($Status) { $services | Where-Object { $_.Status.ToString() -eq $Status } } else { $services }`, + allowedCmdlets: ["Get-Service", "Where-Object"], + }, + serviceStatus: { + script: `param([string]$Name) +Get-Service -Name $Name`, + allowedCmdlets: ["Get-Service"], + }, + startService: { + script: `param([string]$Name) +Start-Service -Name $Name`, + allowedCmdlets: ["Start-Service"], + confirmation: "Start the requested Windows service?", + }, + stopService: { + script: `param([string]$Name) +Stop-Service -Name $Name`, + allowedCmdlets: ["Stop-Service"], + confirmation: "Stop the requested Windows service?", + }, + restartService: { + script: `param([string]$Name) +Restart-Service -Name $Name`, + allowedCmdlets: ["Restart-Service"], + confirmation: "Restart the requested Windows service?", + }, +} satisfies NamespaceActionDefinitions; + +export const servicesActionHandler = + createPowerShellNamespaceActionHandler( + "powershell.powershell-services", + definitions, + ); diff --git a/ts/packages/agents/powershell/src/namespaces/system/actionHandler.mts b/ts/packages/agents/powershell/src/namespaces/system/actionHandler.mts new file mode 100644 index 0000000000..20530533d8 --- /dev/null +++ b/ts/packages/agents/powershell/src/namespaces/system/actionHandler.mts @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { PowerShellSystemActions } from "./systemActionsSchema.mjs"; +import { + createPowerShellNamespaceActionHandler, + type NamespaceActionDefinitions, +} from "../namespaceActionHandler.mjs"; + +const definitions = { + systemInfo: { + script: `Get-CimInstance Win32_OperatingSystem +Get-CimInstance Win32_ComputerSystem`, + allowedCmdlets: ["Get-CimInstance"], + }, + diskUsage: { + script: `param([string]$DriveLetter) +$drives = Get-PSDrive -PSProvider FileSystem +if ($DriveLetter) { $drives | Where-Object { $_.Name -eq $DriveLetter } } else { $drives }`, + allowedCmdlets: ["Get-PSDrive", "Where-Object"], + }, + hotFixes: { + script: "Get-HotFix", + allowedCmdlets: ["Get-HotFix"], + }, + uptime: { + script: `(Get-Date) - (Get-CimInstance Win32_OperatingSystem).LastBootUpTime`, + allowedCmdlets: ["Get-Date", "Get-CimInstance"], + }, + envVars: { + script: `param([string]$Name) +if ($Name) { Get-Item -LiteralPath "Env:$Name" } else { Get-ChildItem Env: }`, + allowedCmdlets: ["Get-Item", "Get-ChildItem"], + }, +} satisfies NamespaceActionDefinitions; + +export const systemActionHandler = + createPowerShellNamespaceActionHandler( + "powershell.powershell-system", + definitions, + ); diff --git a/ts/packages/agents/powershell/src/schema/scriptActions.mts b/ts/packages/agents/powershell/src/schema/scriptActions.mts index 890854127c..7340b55eb6 100644 --- a/ts/packages/agents/powershell/src/schema/scriptActions.mts +++ b/ts/packages/agents/powershell/src/schema/scriptActions.mts @@ -153,6 +153,23 @@ export type EditPowerShellFlow = { }; }; +// Repair an existing flow and retry the requested operation once +export type RepairAndExecutePowerShellFlow = { + actionName: "repairAndExecutePowerShellFlow"; + parameters: { + // Existing flow to repair + flowName: string; + // Replacement script body + script: string; + // Updated cmdlet whitelist + allowedCmdlets: string[]; + // Updated module whitelist + allowedModules?: string[]; + // JSON string of named parameters for the retry + executionParametersJson?: string; + }; +}; + // Import an existing PowerShell script file as a new PowerShell flow export type ImportPowerShellFlow = { actionName: "importPowerShellFlow"; @@ -174,4 +191,5 @@ export type PowerShellActions = | AddPowerShellFlowPatterns | ReportPowerShellCapabilityOutcome | EditPowerShellFlow + | RepairAndExecutePowerShellFlow | ImportPowerShellFlow; diff --git a/ts/packages/agents/powershell/src/store/powerShellStore.mts b/ts/packages/agents/powershell/src/store/powerShellStore.mts index 0a250f3429..55118b12f3 100644 --- a/ts/packages/agents/powershell/src/store/powerShellStore.mts +++ b/ts/packages/agents/powershell/src/store/powerShellStore.mts @@ -525,6 +525,18 @@ export class PowerShellStore { " };", "};", "", + "// Repair an existing flow and retry once", + "export type RepairAndExecutePowerShellFlow = {", + ' actionName: "repairAndExecutePowerShellFlow";', + " parameters: {", + ` flowName: ${flowNameType};`, + " script: string;", + " allowedCmdlets: string[];", + " allowedModules?: string[];", + " executionParametersJson?: string;", + " };", + "};", + "", "// Import an existing PowerShell script file as a new PowerShell flow", "export type ImportPowerShellFlow = {", ' actionName: "importPowerShellFlow";', @@ -545,6 +557,7 @@ export class PowerShellStore { "AddPowerShellFlowPatterns", "ReportPowerShellCapabilityOutcome", "EditPowerShellFlow", + "RepairAndExecutePowerShellFlow", "ImportPowerShellFlow", ]; diff --git a/ts/packages/agents/powershell/src/types/powerShellAgentContext.mts b/ts/packages/agents/powershell/src/types/powerShellAgentContext.mts new file mode 100644 index 0000000000..34d9be9866 --- /dev/null +++ b/ts/packages/agents/powershell/src/types/powerShellAgentContext.mts @@ -0,0 +1,8 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { PowerShellStore } from "../store/powerShellStore.mjs"; + +export interface PowerShellAgentContext { + store?: PowerShellStore | undefined; +} diff --git a/ts/packages/agents/powershell/src/types/powerShellFailure.mts b/ts/packages/agents/powershell/src/types/powerShellFailure.mts new file mode 100644 index 0000000000..ac8f0d6c38 --- /dev/null +++ b/ts/packages/agents/powershell/src/types/powerShellFailure.mts @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { ActionResultError } from "@typeagent/agent-sdk"; +import type { ScriptExecutionResult } from "../execution/powershellRunner.mjs"; + +export type PowerShellFailureKind = + | "unknownFlow" + | "invalidParameters" + | "scriptFailure" + | "policyDenied" + | "cancelled" + | "partialSideEffects"; + +const retryableFailures = new Set([ + "unknownFlow", + "invalidParameters", + "scriptFailure", +]); + +export function createPowerShellFailure( + kind: PowerShellFailureKind, + error: string, + options?: { + retryable?: boolean; + mayHaveSideEffects?: boolean; + }, +): ActionResultError { + const retryable = options?.retryable ?? retryableFailures.has(kind); + return { + error, + errorCode: `powershell.${kind}`, + retryable, + mayHaveSideEffects: + options?.mayHaveSideEffects ?? kind === "partialSideEffects", + fallbackToReasoning: + retryable && kind !== "cancelled" && kind !== "partialSideEffects", + }; +} + +export function createPowerShellExecutionFailure( + result: ScriptExecutionResult, +): ActionResultError { + if (result.cancelled) { + return createPowerShellFailure( + "cancelled", + "PowerShell execution was cancelled.", + { retryable: false }, + ); + } + const error = result.stderr || `Script exited with code ${result.exitCode}`; + if ( + /denied|not allowed|requires networkAccess|outside allowed|unauthorized/i.test( + error, + ) + ) { + return createPowerShellFailure("policyDenied", error, { + retryable: false, + }); + } + return createPowerShellFailure("scriptFailure", error); +} diff --git a/ts/packages/agents/powershell/test/actionHandler.spec.ts b/ts/packages/agents/powershell/test/actionHandler.spec.ts index 478d70e9e6..ac0323e74f 100644 --- a/ts/packages/agents/powershell/test/actionHandler.spec.ts +++ b/ts/packages/agents/powershell/test/actionHandler.spec.ts @@ -8,10 +8,18 @@ import type { TokenCachePersistence, } from "@typeagent/agent-sdk"; import { jest } from "@jest/globals"; -import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { readFileSync } from "node:fs"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { instantiate } from "../src/actionHandler.mjs"; +import { executeScript } from "../src/execution/powershellRunner.mjs"; +import { + getRegisteredNamespaceActions, + hasNamespaceAction, +} from "../src/namespaces/actionHandlerRegistry.mjs"; + +const itOnWindows = process.platform === "win32" ? it : it.skip; const itOnWindows = process.platform === "win32" ? it : it.skip; @@ -69,6 +77,7 @@ class MemoryStorage implements Storage { function createSessionContext( storage: Storage, reloadAgentSchema: () => Promise = jest.fn(async () => {}), + popupQuestion: SessionContext["popupQuestion"] = async () => 1, ): SessionContext { return { agentContext: {}, @@ -77,7 +86,7 @@ function createSessionContext( sessionContextId: "powershell-action-handler-test", notify: jest.fn(), beginAgentThread: jest.fn(), - popupQuestion: jest.fn(), + popupQuestion, toggleTransientAgent: jest.fn(), addDynamicAgent: jest.fn(), removeDynamicAgent: jest.fn(), @@ -93,6 +102,7 @@ function createSessionContext( function createActionContext( sessionContext: SessionContext, + abortSignal?: AbortSignal, ): ActionContext { return { streamingContext: undefined, @@ -104,14 +114,23 @@ function createActionContext( takeAction: jest.fn(), }, sessionContext, + abortSignal, isFromReasoningLoop: true, queueToggleTransientAgent: async () => {}, }; } -async function createAgentHarness(reloadAgentSchema?: () => Promise) { - const storage = new MemoryStorage(); - const sessionContext = createSessionContext(storage, reloadAgentSchema); +async function createAgentHarness( + reloadAgentSchema?: () => Promise, + abortSignal?: AbortSignal, + storage = new MemoryStorage(), + popupQuestion?: SessionContext["popupQuestion"], +) { + const sessionContext = createSessionContext( + storage, + reloadAgentSchema, + popupQuestion, + ); const agent = instantiate(); await agent.initializeAgentContext?.(); await agent.updateAgentContext?.(true, sessionContext, "powershell"); @@ -119,7 +138,7 @@ async function createAgentHarness(reloadAgentSchema?: () => Promise) { agent, storage, sessionContext, - context: createActionContext(sessionContext), + context: createActionContext(sessionContext, abortSignal), }; } @@ -147,9 +166,172 @@ describe("createAndExecutePowerShellFlow", () => { error: expect.stringContaining( "Unknown PowerShell flow 'portListeners'", ), + errorCode: "powershell.unknownFlow", + retryable: true, }); }); + }); + + describe("static namespace coverage", () => { + it("registers exactly the namespaces declared by the manifest", () => { + const manifest = JSON.parse( + readFileSync( + join(process.cwd(), "src", "manifest.json"), + "utf8", + ), + ) as { subActionManifests: Record }; + const manifestSchemas = Object.keys(manifest.subActionManifests) + .map((name) => `powershell.${name}`) + .sort(); + const registeredSchemas = [ + ...getRegisteredNamespaceActions().keys(), + ].sort(); + + expect(registeredSchemas).toEqual(manifestSchemas); + expect( + hasNamespaceAction( + "powershell.powershell-network", + "portListeners", + ), + ).toBe(true); + }); + + itOnWindows("executes a read-only system action", async () => { + const { agent, context } = await createAgentHarness(); + + const result = await agent.executeAction?.( + { + schemaName: "powershell.powershell-system", + actionName: "envVars", + parameters: { name: "TEMP" }, + }, + context, + ); + + expect(result).not.toHaveProperty("error"); + }); + + itOnWindows.each([ + ["powershell.powershell-processes", "listProcesses", { topN: 1 }], + ["powershell.powershell-services", "listServices", {}], + ])( + "executes a read-only %s action", + async (schemaName, actionName, parameters) => { + const { agent, context } = await createAgentHarness(); + + const result = await agent.executeAction?.( + { schemaName, actionName, parameters }, + context, + ); + expect(result).not.toHaveProperty("error"); + }, + ); + + itOnWindows("executes file, data, and archive actions", async () => { + const directory = await mkdtemp( + join(tmpdir(), "typeagent-powershell-static-"), + ); + const textPath = join(directory, "sample.txt"); + const jsonPath = join(directory, "sample.json"); + const archivePath = join(directory, "sample.zip"); + const extractPath = join(directory, "extracted"); + try { + await writeFile(textPath, "sample text"); + await writeFile(jsonPath, JSON.stringify({ value: "sample" })); + const approve = jest.fn(async () => 0); + const { agent, context } = await createAgentHarness( + undefined, + undefined, + undefined, + approve, + ); + + const readText = await agent.executeAction?.( + { + schemaName: "powershell.powershell-files", + actionName: "readFile", + parameters: { path: textPath }, + }, + context, + ); + const readJson = await agent.executeAction?.( + { + schemaName: "powershell.powershell-data", + actionName: "readJson", + parameters: { path: jsonPath }, + }, + context, + ); + const compress = await agent.executeAction?.( + { + schemaName: "powershell.powershell-archives", + actionName: "compress", + parameters: { + sourcePath: textPath, + destinationPath: archivePath, + }, + }, + context, + ); + const expand = await agent.executeAction?.( + { + schemaName: "powershell.powershell-archives", + actionName: "expand", + parameters: { + archivePath, + destinationPath: extractPath, + }, + }, + context, + ); + + expect(readText).not.toHaveProperty("error"); + expect(readJson).not.toHaveProperty("error"); + expect(compress).not.toHaveProperty("error"); + expect(expand).not.toHaveProperty("error"); + expect((await readFile(archivePath)).length).toBeGreaterThan(0); + expect( + await readFile(join(extractPath, "sample.txt"), "utf8"), + ).toBe("sample text"); + expect(approve).toHaveBeenCalledTimes(2); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + + it("denies mutating actions when confirmation is not approved", async () => { + const directory = await mkdtemp( + join(tmpdir(), "typeagent-powershell-denied-"), + ); + const outputPath = join(directory, "denied.txt"); + try { + const { agent, context } = await createAgentHarness(); + + const result = await agent.executeAction?.( + { + schemaName: "powershell.powershell-files", + actionName: "writeFile", + parameters: { + path: outputPath, + content: "should not be written", + }, + }, + context, + ); + + expect(result).toMatchObject({ + errorCode: "powershell.policyDenied", + retryable: false, + }); + await expect(readFile(outputPath, "utf8")).rejects.toThrow(); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + }); + + describe("static network actions", () => { itOnWindows( "executes portListeners without requiring a dynamic flow", async () => { @@ -163,7 +345,6 @@ describe("createAndExecutePowerShellFlow", () => { }, context, ); - expect(result).not.toHaveProperty("error"); }, ); @@ -279,6 +460,8 @@ describe("createAndExecutePowerShellFlow", () => { expect(reloadAgentSchema).toHaveBeenCalledTimes(1); expect(result).toMatchObject({ error: expect.stringContaining("could not be activated"), + errorCode: "powershell.partialSideEffects", + mayHaveSideEffects: true, }); expect(await storage.list("pending")).toEqual([]); expect(await storage.exists("flows/reloadFailure.flow.json")).toBe( @@ -286,4 +469,239 @@ describe("createAndExecutePowerShellFlow", () => { ); }, ); + + itOnWindows("cancels execution and removes the pending draft", async () => { + const controller = new AbortController(); + const { agent, storage, context } = await createAgentHarness( + undefined, + controller.signal, + ); + + const execution = agent.executeAction?.( + { + schemaName: "powershell", + actionName: "createAndExecutePowerShellFlow", + parameters: { + actionName: "cancelledDraft", + description: "A flow cancelled during execution", + script: "Start-Sleep -Seconds 30", + allowedCmdlets: ["Start-Sleep"], + executionParametersJson: "{}", + }, + }, + context, + ); + setTimeout(() => controller.abort(), 250); + + await expect(execution).rejects.toMatchObject({ + name: "AbortError", + }); + expect(await storage.list("pending")).toEqual([]); + expect(await storage.exists("flows/cancelledDraft.flow.json")).toBe( + false, + ); + }); + + itOnWindows( + "deduplicates concurrent creation and reuses the winning flow", + async () => { + const directory = await mkdtemp( + join(tmpdir(), "typeagent-powershell-concurrent-"), + ); + const firstPath = join(directory, "first.txt"); + const secondPath = join(directory, "second.txt"); + try { + const { agent, storage, context } = await createAgentHarness(); + const create = (outputPath: string) => + agent.executeAction?.( + { + schemaName: "powershell", + actionName: "createAndExecutePowerShellFlow", + parameters: { + actionName: "concurrentFlow", + description: "Record a concurrent execution", + script: "param([string]$Path)\nSet-Content -LiteralPath $Path -Value 'run'", + scriptParameters: [ + { + name: "Path", + type: "path", + required: true, + description: "Output file", + }, + ], + allowedCmdlets: ["Set-Content"], + executionParametersJson: JSON.stringify({ + Path: outputPath, + }), + }, + }, + context, + ); + + const [first, second] = await Promise.all([ + create(firstPath), + create(secondPath), + ]); + + expect(first).not.toHaveProperty("error"); + expect(second).not.toHaveProperty("error"); + expect(await storage.list("pending")).toEqual([]); + expect( + await storage.exists("flows/concurrentFlow.flow.json"), + ).toBe(true); + expect(await readFile(firstPath, "utf8")).toContain("run"); + expect(await readFile(secondPath, "utf8")).toContain("run"); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }, + ); + + itOnWindows( + "repairs a stale flow once and keeps the repaired script", + async () => { + const directory = await mkdtemp( + join(tmpdir(), "typeagent-powershell-repair-"), + ); + const outputPath = join(directory, "repair.txt"); + try { + const { agent, context } = await createAgentHarness(); + await agent.executeAction?.( + { + schemaName: "powershell", + actionName: "createAndExecutePowerShellFlow", + parameters: { + actionName: "repairableFlow", + description: "A repairable flow", + script: "param([string]$Path)\nSet-Content -LiteralPath $Path -Value 'original'", + scriptParameters: [ + { + name: "Path", + type: "path", + required: true, + description: "Output file", + }, + ], + allowedCmdlets: ["Set-Content"], + executionParametersJson: JSON.stringify({ + Path: outputPath, + }), + }, + }, + context, + ); + + const repaired = await agent.executeAction?.( + { + schemaName: "powershell", + actionName: "repairAndExecutePowerShellFlow", + parameters: { + flowName: "repairableFlow", + script: "param([string]$Path)\nSet-Content -LiteralPath $Path -Value 'repaired'", + allowedCmdlets: ["Set-Content"], + executionParametersJson: JSON.stringify({ + Path: outputPath, + }), + }, + }, + context, + ); + const secondRepair = await agent.executeAction?.( + { + schemaName: "powershell", + actionName: "repairAndExecutePowerShellFlow", + parameters: { + flowName: "repairableFlow", + script: "throw 'second repair'", + allowedCmdlets: [], + executionParametersJson: JSON.stringify({ + Path: outputPath, + }), + }, + }, + context, + ); + + expect(repaired).not.toHaveProperty("error"); + expect((await readFile(outputPath, "utf8")).trim()).toBe( + "repaired", + ); + expect(secondRepair).toMatchObject({ + errorCode: "powershell.policyDenied", + retryable: false, + }); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }, + ); + + itOnWindows("reloads a promoted flow in a new agent instance", async () => { + const storage = new MemoryStorage(); + const first = await createAgentHarness(undefined, undefined, storage); + const created = await first.agent.executeAction?.( + { + schemaName: "powershell", + actionName: "createAndExecutePowerShellFlow", + parameters: { + actionName: "persistentFlow", + description: "A flow persisted across agent instances", + script: "Write-Output 'persisted'", + allowedCmdlets: ["Write-Output"], + executionParametersJson: "{}", + }, + }, + first.context, + ); + expect(created).not.toHaveProperty("error"); + + const second = await createAgentHarness(undefined, undefined, storage); + const reused = await second.agent.executeAction?.( + { + schemaName: "powershell", + actionName: "persistentFlow", + parameters: {}, + }, + second.context, + ); + + expect(reused).not.toHaveProperty("error"); + }); + + itOnWindows( + "blocks writes to non-existent paths outside the sandbox", + async () => { + const directory = await mkdtemp( + join(tmpdir(), "typeagent-powershell-path-policy-"), + ); + const allowedDirectory = join(directory, "allowed"); + const blockedPath = join( + directory, + "allowed-sibling", + "blocked.txt", + ); + try { + await mkdir(allowedDirectory); + + const result = await executeScript({ + script: `param([string]$Path) +Set-Content -LiteralPath $Path -Value "blocked"`, + parameters: { Path: blockedPath }, + sandbox: { + allowedCmdlets: ["Set-Content"], + allowedPaths: [allowedDirectory], + allowedModules: [], + maxExecutionTime: 10, + networkAccess: false, + }, + }); + + expect(result.success).toBe(false); + expect(result.stderr).toMatch(/Path access denied/i); + await expect(readFile(blockedPath, "utf8")).rejects.toThrow(); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }, + ); }); diff --git a/ts/packages/agents/powershell/test/powerShellStore.spec.ts b/ts/packages/agents/powershell/test/powerShellStore.spec.ts index ca323bded1..dca8dcf673 100644 --- a/ts/packages/agents/powershell/test/powerShellStore.spec.ts +++ b/ts/packages/agents/powershell/test/powerShellStore.spec.ts @@ -141,5 +141,6 @@ describe("PowerShellStore capability lifecycle", () => { expect(schema).toContain("createAndExecutePowerShellFlow"); expect(schema).toContain("addPowerShellFlowPatterns"); expect(schema).toContain("reportPowerShellCapabilityOutcome"); + expect(schema).toContain("repairAndExecutePowerShellFlow"); }); }); diff --git a/ts/packages/copilot-plugin/hooks.json b/ts/packages/copilot-plugin/hooks.json index 6dac12c30d..3ee7c5cdc6 100644 --- a/ts/packages/copilot-plugin/hooks.json +++ b/ts/packages/copilot-plugin/hooks.json @@ -5,7 +5,7 @@ { "type": "command", "command": "node ${PLUGIN_ROOT}/dist/hooks/hook-router.js", - "timeout": 30000 + "timeout": 300000 } ], "agentStop": [ diff --git a/ts/packages/copilot-plugin/src/hooks/hook-dev-actions.ts b/ts/packages/copilot-plugin/src/hooks/hook-dev-actions.ts index fa136d9eb3..582f9d824d 100644 --- a/ts/packages/copilot-plugin/src/hooks/hook-dev-actions.ts +++ b/ts/packages/copilot-plugin/src/hooks/hook-dev-actions.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import type { Dispatcher } from "@typeagent/agent-server-client"; +import { randomUUID } from "node:crypto"; import type { CommandResult, ProcessCommandOptions, @@ -21,18 +22,26 @@ import type { HookInput, HookOutput } from "./types.js"; export type DevActionDependencies = { connectToTypeAgent: typeof connectToTypeAgent; emitProgress: typeof emitProgress; + platform?: NodeJS.Platform; }; const defaultDependencies: DevActionDependencies = { connectToTypeAgent, emitProgress, + platform: process.platform, }; +const unsupportedPlatformMessage = + "TypeAgent PowerShell recording is supported only on Windows. Run the request on a Windows host with the PowerShell agent enabled."; +const unavailablePowerShellMessage = + "TypeAgent could not record this PowerShell flow because the PowerShell schema is unavailable. Enable the PowerShell agent and retry."; + export function getDevActionCommandOptions( prompt: string, ): ProcessCommandOptions { if (parseRecordingDirective(prompt) !== undefined) { return { + activeSchemaFamilies: ["powershell"], noReasoning: false, reasoningProfile: "powershellFlowRecording", }; @@ -64,7 +73,20 @@ function toHandledOutput( export async function handleDevActions( input: HookInput, dependencies: DevActionDependencies = defaultDependencies, + abortSignal?: AbortSignal, ): Promise { + const isRecordingDirective = + parseRecordingDirective(input.prompt) !== undefined; + if ((dependencies.platform ?? process.platform) !== "win32") { + return isRecordingDirective + ? { + handled: true, + responseContent: unsupportedPlatformMessage, + handledBy: "typeagent", + } + : {}; + } + dependencies.emitProgress("Checking TypeAgent development actions...", { temporary: true, }); @@ -107,19 +129,49 @@ export async function handleDevActions( }); let dispatcher: Dispatcher | null = null; - let submitted = false; + let submissionStarted = false; + let requestId: string | undefined; + const clientRequestId = `copilot-dev-${input.sessionId}-${randomUUID()}`; + let earlyCancellation: Promise | undefined; + const cancelAcceptedRequest = () => { + if (!dispatcher) { + return; + } + if (requestId) { + earlyCancellation = dispatcher + .cancelCommand(requestId) + .then(() => undefined); + } else { + dispatcher.cancelCommandByClientId(clientRequestId); + earlyCancellation = Promise.resolve(); + } + void earlyCancellation.catch((error) => { + console.error("TypeAgent dev mode cancellation error:", error); + }); + }; + abortSignal?.addEventListener("abort", cancelAcceptedRequest, { + once: true, + }); + try { + abortSignal?.throwIfAborted(); dispatcher = await dependencies.connectToTypeAgent(clientIO); + abortSignal?.throwIfAborted(); + submissionStarted = true; const submitResult = await dispatcher.submitCommand( input.prompt, undefined, getDevActionCommandOptions(input.prompt), + clientRequestId, ); if (!submitResult.ok) { return {}; } - submitted = true; + requestId = submitResult.entry.requestId; + if (abortSignal?.aborted) { + await dispatcher.cancelCommand(requestId); + } const result = await submitResult.entry.completion; if (!result) { return { @@ -142,15 +194,32 @@ export async function handleDevActions( } if (result.disposition.status === "notHandled") { + if ( + isRecordingDirective && + result.disposition.reason === "noActiveSchema" + ) { + return { + handled: true, + responseContent: unavailablePowerShellMessage, + handledBy: "typeagent", + }; + } return {}; } return toHandledOutput(result, responseCollector.messages); } catch (error) { console.error("TypeAgent dev mode error:", error); - if (!submitted) { + if (!submissionStarted) { return {}; } + if (abortSignal?.aborted) { + return { + handled: true, + responseContent: "TypeAgent request was cancelled.", + handledBy: "typeagent", + }; + } return { handled: true, responseContent: `TypeAgent could not finish the submitted development action: ${ @@ -159,6 +228,8 @@ export async function handleDevActions( handledBy: "typeagent", }; } finally { + abortSignal?.removeEventListener("abort", cancelAcceptedRequest); + await earlyCancellation?.catch(() => {}); if (dispatcher) { await dispatcher.close(); } diff --git a/ts/packages/copilot-plugin/src/hooks/hook-router.ts b/ts/packages/copilot-plugin/src/hooks/hook-router.ts index bcbc65c448..13e295d5ec 100644 --- a/ts/packages/copilot-plugin/src/hooks/hook-router.ts +++ b/ts/packages/copilot-plugin/src/hooks/hook-router.ts @@ -166,46 +166,60 @@ function handleSlashCommand( } async function main(): Promise { - let inputData = ""; - process.stdin.setEncoding("utf8"); + const abortController = new AbortController(); + const abortRequest = () => abortController.abort(); + process.once("SIGINT", abortRequest); + process.once("SIGTERM", abortRequest); - for await (const chunk of process.stdin) { - inputData += chunk; - } - - let input: HookInput; try { - input = JSON.parse(inputData); - } catch { - console.error("Failed to parse hook input"); - process.exit(1); - } + let inputData = ""; + process.stdin.setEncoding("utf8"); - // Check for slash commands first - const slashResult = await handleSlashCommand(input.prompt); - if (slashResult) { - console.log(JSON.stringify(slashResult)); - emitDemoStateForOutput(input, slashResult, "direct"); - return; - } + for await (const chunk of process.stdin) { + inputData += chunk; + } - // Route based on current mode - const mode = getMode(); - let output: HookOutput; - - if (mode === "bypass") { - // Bypass mode: return empty to fall through to other handlers - output = {}; - } else if (mode === "mcp") { - output = handleMcpRedirect(input); - } else if (mode === "dev") { - output = await handleDevActions(input); - } else { - output = await handleDirect(input); - } + let input: HookInput; + try { + input = JSON.parse(inputData); + } catch { + console.error("Failed to parse hook input"); + process.exit(1); + } - console.log(JSON.stringify(output)); - emitDemoStateForOutput(input, output, mode); + // Check for slash commands first + const slashResult = await handleSlashCommand(input.prompt); + if (slashResult) { + console.log(JSON.stringify(slashResult)); + emitDemoStateForOutput(input, slashResult, "direct"); + return; + } + + // Route based on current mode + const mode = getMode(); + let output: HookOutput; + + if (mode === "bypass") { + // Bypass mode: return empty to fall through to other handlers + output = {}; + } else if (mode === "mcp") { + output = handleMcpRedirect(input); + } else if (mode === "dev") { + output = await handleDevActions( + input, + undefined, + abortController.signal, + ); + } else { + output = await handleDirect(input); + } + + console.log(JSON.stringify(output)); + emitDemoStateForOutput(input, output, mode); + } finally { + process.removeListener("SIGINT", abortRequest); + process.removeListener("SIGTERM", abortRequest); + } } /** diff --git a/ts/packages/copilot-plugin/src/shared/typeagent-client.ts b/ts/packages/copilot-plugin/src/shared/typeagent-client.ts index 95dc05347f..b5b38676bc 100644 --- a/ts/packages/copilot-plugin/src/shared/typeagent-client.ts +++ b/ts/packages/copilot-plugin/src/shared/typeagent-client.ts @@ -43,13 +43,6 @@ export function createClientIO(callbacks: DisplayCallbacks): ClientIO { }, appendDiagnosticData(): void {}, setDynamicDisplay(): void {}, - async askYesNo( - _requestId: RequestId, - _message: string, - defaultValue?: boolean, - ): Promise { - return defaultValue ?? true; - }, async proposeAction( _requestId: RequestId, _actionTemplates: TemplateEditConfig, @@ -57,14 +50,6 @@ export function createClientIO(callbacks: DisplayCallbacks): ClientIO { ): Promise { return undefined; }, - async popupQuestion( - _message: string, - _choices: string[], - defaultId: number | undefined, - _source: string, - ): Promise { - return defaultId ?? 0; - }, notify(): void {}, async openLocalView(): Promise {}, async closeLocalView(): Promise {}, @@ -72,8 +57,13 @@ export function createClientIO(callbacks: DisplayCallbacks): ClientIO { requestForm(): void {}, takeAction(): void {}, shutdown(): void {}, - async question(): Promise { - return 0; + async question( + _requestId: RequestId | undefined, + _message: string, + choices: string[], + defaultId?: number, + ): Promise { + return defaultId ?? Math.max(choices.length - 1, 0); }, requestInteraction(): void {}, interactionResolved(): void {}, diff --git a/ts/packages/copilot-plugin/test/hookDevActions.spec.ts b/ts/packages/copilot-plugin/test/hookDevActions.spec.ts index b5d066f287..8e294a96b2 100644 --- a/ts/packages/copilot-plugin/test/hookDevActions.spec.ts +++ b/ts/packages/copilot-plugin/test/hookDevActions.spec.ts @@ -12,6 +12,7 @@ import { handleDevActions, type DevActionDependencies, } from "../src/hooks/hook-dev-actions.js"; +import { createClientIO } from "../src/shared/typeagent-client.js"; const input = { sessionId: "test-session", @@ -25,25 +26,37 @@ function createDependencies( ): { dependencies: DevActionDependencies; submitCommand: jest.Mock; + cancelCommand: jest.Mock; + cancelCommandByClientId: jest.Mock; close: jest.Mock; } { const completion = result instanceof Promise ? result : Promise.resolve(result); const submitCommand = jest.fn(async () => ({ ok: true, - entry: { completion }, + entry: { requestId: "request-1", completion }, })); + const cancelCommand = jest.fn(async () => ({ + kind: "running", + requestId: "request-1", + })); + const cancelCommandByClientId = jest.fn(); const close = jest.fn(async () => {}); const dispatcher = { submitCommand, + cancelCommand, + cancelCommandByClientId, close, } as unknown as Dispatcher; return { dependencies: { connectToTypeAgent: jest.fn(async () => dispatcher), emitProgress: jest.fn(), + platform: "win32", }, submitCommand, + cancelCommand, + cancelCommandByClientId, close, }; } @@ -61,6 +74,7 @@ describe("Copilot dev actions hook", () => { expect( getDevActionCommandOptions("learn: show running processes"), ).toEqual({ + activeSchemaFamilies: ["powershell"], noReasoning: false, reasoningProfile: "powershellFlowRecording", }); @@ -108,6 +122,7 @@ describe("Copilot dev actions hook", () => { throw new Error("server unavailable"); }), emitProgress: jest.fn(), + platform: "win32", }; await expect(handleDevActions(input, dependencies)).resolves.toEqual( @@ -132,4 +147,111 @@ describe("Copilot dev actions hook", () => { }); consoleError.mockRestore(); }); + + it("falls through for ordinary prompts on non-Windows platforms", async () => { + const { dependencies } = createDependencies({ + disposition: { + status: "handled", + path: "action", + schemas: ["powershell"], + }, + }); + dependencies.platform = "linux"; + + await expect(handleDevActions(input, dependencies)).resolves.toEqual( + {}, + ); + expect(dependencies.connectToTypeAgent).not.toHaveBeenCalled(); + }); + + it("reports unsupported explicit recording on non-Windows platforms", async () => { + const { dependencies } = createDependencies(undefined); + dependencies.platform = "darwin"; + + await expect( + handleDevActions( + { ...input, prompt: "learn: show running processes" }, + dependencies, + ), + ).resolves.toEqual({ + handled: true, + responseContent: + "TypeAgent PowerShell recording is supported only on Windows. Run the request on a Windows host with the PowerShell agent enabled.", + handledBy: "typeagent", + }); + }); + + it("reports unavailable PowerShell for explicit recording", async () => { + const { dependencies } = createDependencies({ + disposition: { + status: "notHandled", + reason: "noActiveSchema", + }, + }); + + await expect( + handleDevActions( + { ...input, prompt: "record: show running processes" }, + dependencies, + ), + ).resolves.toEqual({ + handled: true, + responseContent: + "TypeAgent could not record this PowerShell flow because the PowerShell schema is unavailable. Enable the PowerShell agent and retry.", + handledBy: "typeagent", + }); + }); + + it("cancels an accepted request when the hook is aborted", async () => { + let resolveCompletion: + | ((result: CommandResult | undefined) => void) + | undefined; + const completion = new Promise((resolve) => { + resolveCompletion = resolve; + }); + const { dependencies, cancelCommand } = createDependencies(completion); + cancelCommand.mockImplementation(async () => { + resolveCompletion?.({ cancelled: true }); + return { kind: "running", requestId: "request-1" }; + }); + const controller = new AbortController(); + + const handled = handleDevActions( + input, + dependencies, + controller.signal, + ); + await new Promise((resolve) => setImmediate(resolve)); + controller.abort(); + + await expect(handled).resolves.toEqual({ + handled: true, + responseContent: "TypeAgent request was cancelled.", + handledBy: "typeagent", + }); + expect(cancelCommand).toHaveBeenCalledWith("request-1"); + }); + + it("defaults unattended interactions to denial", async () => { + const clientIO = createClientIO({}); + + await expect( + clientIO.question( + undefined, + "Allow action?", + ["Run", "Cancel"], + undefined, + "powershell", + ), + ).resolves.toBe(1); + await expect( + clientIO.question( + undefined, + "Allow action?", + ["Run", "Cancel"], + 1, + "powershell", + ), + ).resolves.toBe(1); + }); }); diff --git a/ts/packages/dispatcher/dispatcher/src/reasoning/reasoningProfile.ts b/ts/packages/dispatcher/dispatcher/src/reasoning/reasoningProfile.ts index d6d1139a8a..5b6aa29015 100644 --- a/ts/packages/dispatcher/dispatcher/src/reasoning/reasoningProfile.ts +++ b/ts/packages/dispatcher/dispatcher/src/reasoning/reasoningProfile.ts @@ -34,6 +34,7 @@ function getPowerShellCapabilityFallbackGuidance(): string { "Decide whether the user's request can be safely completed as a reusable PowerShell flow.", "Use discover_actions for the powershell schema and listPowerShellFlows before creating anything.", "Prefer an existing executable action or flow. If an existing flow covers the task but misses this phrasing, add validated patterns with addPowerShellFlowPatterns, then execute the existing flow once.", + "If an existing flow fails with errorCode powershell.scriptFailure, repair that same flow with repairAndExecutePowerShellFlow at most once. Do not repair policyDenied, cancelled, or partialSideEffects failures.", "If no equivalent exists, use createAndExecutePowerShellFlow. It executes the draft once and promotes it only after success. Do not execute the promoted flow again.", "Do not use shell, Bash, TaskFlow, or WebFlow as substitutes.", "You MUST finish by calling reportPowerShellCapabilityOutcome exactly once.", From 69122203c2f6e43ffb1b35eae3f18f381d18cc25 Mon Sep 17 00:00:00 2001 From: Hillary Mutisya <150286414+hillary-mutisya@users.noreply.github.com> Date: Tue, 11 Aug 2026 18:30:22 -0700 Subject: [PATCH 2/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- ts/packages/agents/powershell/test/actionHandler.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/ts/packages/agents/powershell/test/actionHandler.spec.ts b/ts/packages/agents/powershell/test/actionHandler.spec.ts index ac0323e74f..bb10824888 100644 --- a/ts/packages/agents/powershell/test/actionHandler.spec.ts +++ b/ts/packages/agents/powershell/test/actionHandler.spec.ts @@ -21,8 +21,6 @@ import { const itOnWindows = process.platform === "win32" ? it : it.skip; -const itOnWindows = process.platform === "win32" ? it : it.skip; - class MemoryStorage implements Storage { private readonly files = new Map(); From d8094b8d448ac64625c1496077b333a49d0332ae Mon Sep 17 00:00:00 2001 From: Hillary Mutisya <150286414+hillary-mutisya@users.noreply.github.com> Date: Tue, 11 Aug 2026 18:35:17 -0700 Subject: [PATCH 3/5] Fix copilot review feedback --- ts/packages/copilot-plugin/src/hooks/hook-dev-actions.ts | 6 +++++- ts/packages/copilot-plugin/src/shared/typeagent-client.ts | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/ts/packages/copilot-plugin/src/hooks/hook-dev-actions.ts b/ts/packages/copilot-plugin/src/hooks/hook-dev-actions.ts index 582f9d824d..d2deea8e91 100644 --- a/ts/packages/copilot-plugin/src/hooks/hook-dev-actions.ts +++ b/ts/packages/copilot-plugin/src/hooks/hook-dev-actions.ts @@ -142,7 +142,11 @@ export async function handleDevActions( .cancelCommand(requestId) .then(() => undefined); } else { - dispatcher.cancelCommandByClientId(clientRequestId); + try { + dispatcher.cancelCommandByClientId(clientRequestId); + } catch (error) { + console.error("TypeAgent dev mode cancellation error:", error); + } earlyCancellation = Promise.resolve(); } void earlyCancellation.catch((error) => { diff --git a/ts/packages/copilot-plugin/src/shared/typeagent-client.ts b/ts/packages/copilot-plugin/src/shared/typeagent-client.ts index b5b38676bc..cc3155fb94 100644 --- a/ts/packages/copilot-plugin/src/shared/typeagent-client.ts +++ b/ts/packages/copilot-plugin/src/shared/typeagent-client.ts @@ -62,6 +62,7 @@ export function createClientIO(callbacks: DisplayCallbacks): ClientIO { _message: string, choices: string[], defaultId?: number, + _source?: string, ): Promise { return defaultId ?? Math.max(choices.length - 1, 0); }, From da5dffebea8cff8b88307054d010c77ef062c4c0 Mon Sep 17 00:00:00 2001 From: Hillary Mutisya <150286414+hillary-mutisya@users.noreply.github.com> Date: Tue, 11 Aug 2026 20:09:58 -0700 Subject: [PATCH 4/5] Update powershell tests to handle short path aliases present on CI machines --- .../agents/powershell/scripts/scriptHost.ps1 | 91 +++++++++++++------ .../powershell/test/actionHandler.spec.ts | 20 ++++ 2 files changed, 81 insertions(+), 30 deletions(-) diff --git a/ts/packages/agents/powershell/scripts/scriptHost.ps1 b/ts/packages/agents/powershell/scripts/scriptHost.ps1 index ff6b39f7d5..3be563e216 100644 --- a/ts/packages/agents/powershell/scripts/scriptHost.ps1 +++ b/ts/packages/agents/powershell/scripts/scriptHost.ps1 @@ -27,6 +27,44 @@ param( $ErrorActionPreference = 'Stop' +function Remove-TrailingDirectorySeparator { + param([string]$Path) + + $root = [System.IO.Path]::GetPathRoot($Path) + if ($Path.Equals($root, [System.StringComparison]::OrdinalIgnoreCase)) { + return $root + } + return $Path.TrimEnd('\', '/') +} + +function Get-CanonicalFileSystemPath { + param([string]$Path) + + $fullPath = [System.IO.Path]::GetFullPath($Path) + if (Test-Path -LiteralPath $fullPath) { + $item = Get-Item -LiteralPath $fullPath -Force + return Remove-TrailingDirectorySeparator $item.FullName + } + + $missingSegments = [System.Collections.Generic.List[string]]::new() + $existingPath = $fullPath + while (-not (Test-Path -LiteralPath $existingPath)) { + $leaf = Split-Path -Leaf $existingPath + $parent = Split-Path -Parent $existingPath + if (-not $leaf -or -not $parent -or $parent -eq $existingPath) { + throw "Unable to resolve path '$Path'." + } + $missingSegments.Insert(0, $leaf) + $existingPath = $parent + } + + $canonicalPath = (Get-Item -LiteralPath $existingPath -Force).FullName + foreach ($segment in $missingSegments) { + $canonicalPath = Join-Path $canonicalPath $segment + } + return Remove-TrailingDirectorySeparator ([System.IO.Path]::GetFullPath($canonicalPath)) +} + try { $allowedCmdlets = $AllowedCmdletsJson | ConvertFrom-Json $params = $ParametersJson | ConvertFrom-Json @@ -51,9 +89,10 @@ try { foreach ($ap in $AllowedPaths) { try { $expandedPath = $ExecutionContext.InvokeCommand.ExpandString($ap) - $expandedAllowedPaths += [System.IO.Path]::GetFullPath($expandedPath).TrimEnd('\', '/') + $expandedAllowedPaths += Get-CanonicalFileSystemPath $expandedPath } catch { - $expandedAllowedPaths += $ap + Write-Error "Invalid allowed path '$ap': $_" + exit 1 } } @@ -74,36 +113,28 @@ try { continue } - $isValidPath = $false - try { $isValidPath = Test-Path $val -IsValid } catch { } - if ($isValidPath) { - $resolvedPath = $null - try { - $resolvedPath = (Resolve-Path $val -ErrorAction SilentlyContinue).Path - if (-not $resolvedPath) { - $resolvedPath = [System.IO.Path]::GetFullPath($val) - } - $resolvedPath = $resolvedPath.TrimEnd('\', '/') - } catch {} - if ($resolvedPath) { - $pathAllowed = $false - foreach ($ap in $expandedAllowedPaths) { - if ( - $resolvedPath.Equals($ap, [System.StringComparison]::OrdinalIgnoreCase) -or - $resolvedPath.StartsWith("$ap\", [System.StringComparison]::OrdinalIgnoreCase) -or - $resolvedPath.StartsWith("$ap/", [System.StringComparison]::OrdinalIgnoreCase) - ) { - $pathAllowed = $true - break - } - } - # ENFORCEMENT: Block execution if path not allowed - if (-not $pathAllowed) { - Write-Error "Path access denied: '$resolvedPath' is not in allowedPaths. Allowed paths: $($expandedAllowedPaths -join ', ')" - exit 1 - } + try { + $resolvedPath = Get-CanonicalFileSystemPath $val + } catch { + Write-Error "Invalid path parameter '$($prop.Name)': $_" + exit 1 + } + $pathAllowed = $false + foreach ($ap in $expandedAllowedPaths) { + if ( + $resolvedPath.Equals($ap, [System.StringComparison]::OrdinalIgnoreCase) -or + $resolvedPath.StartsWith("$ap\", [System.StringComparison]::OrdinalIgnoreCase) -or + $resolvedPath.StartsWith("$ap/", [System.StringComparison]::OrdinalIgnoreCase) + ) { + $pathAllowed = $true + break } } + # ENFORCEMENT: Block execution if path not allowed + if (-not $pathAllowed) { + Write-Error "Path access denied: '$resolvedPath' is not in allowedPaths. Allowed paths: $($expandedAllowedPaths -join ', ')" + exit 1 + } } } } diff --git a/ts/packages/agents/powershell/test/actionHandler.spec.ts b/ts/packages/agents/powershell/test/actionHandler.spec.ts index bb10824888..ee159ecb29 100644 --- a/ts/packages/agents/powershell/test/actionHandler.spec.ts +++ b/ts/packages/agents/powershell/test/actionHandler.spec.ts @@ -666,6 +666,26 @@ describe("createAndExecutePowerShellFlow", () => { expect(reused).not.toHaveProperty("error"); }); + itOnWindows( + "accepts a short path alias for an allowed long path", + async () => { + const result = await executeScript({ + script: "param([string]$Path)\nGet-Item -LiteralPath $Path", + parameters: { Path: "C:\\PROGRA~1" }, + sandbox: { + allowedCmdlets: ["Get-Item"], + allowedPaths: ["C:\\Program Files"], + allowedModules: [], + maxExecutionTime: 10, + networkAccess: false, + }, + }); + + expect(result.success).toBe(true); + expect(result.stderr).toBe(""); + }, + ); + itOnWindows( "blocks writes to non-existent paths outside the sandbox", async () => { From 1bec02fc94a10e358b6cd6e1f83286b871fb0644 Mon Sep 17 00:00:00 2001 From: Hillary Mutisya <150286414+hillary-mutisya@users.noreply.github.com> Date: Thu, 13 Aug 2026 08:01:03 -0700 Subject: [PATCH 5/5] Addressing PR feedback - Add an executable parameter role, resolve bare executable names through PowerShell command resolution, canonicalize the resolved file, and enforce `allowedPaths`. - Remove string-shape path inference and validate only parameters explicitly declared as paths. - Split execution, persistence, activation, and usage accounting into distinct failure boundaries. Return `partialSideEffects` when execution succeeded but core completion failed. --- ts/docs/architecture/workflows/workflows.md | 22 +- .../powershell/scripts/compileRecipes.mjs | 12 +- .../agents/powershell/scripts/scriptHost.ps1 | 133 ++++-- .../agents/powershell/src/actionHandler.mts | 435 ++++++++++++------ .../src/analysis/scriptAnalyzer.mts | 6 +- .../src/execution/powershellRunner.mts | 5 + .../src/namespaces/archives/actionHandler.mts | 8 + .../src/namespaces/data/actionHandler.mts | 6 + .../src/namespaces/files/actionHandler.mts | 9 + .../src/namespaces/namespaceActionHandler.mts | 37 +- .../namespaces/processes/actionHandler.mts | 1 + .../powershell/src/schema/scriptActions.mts | 4 +- .../powershell/src/store/powerShellStore.mts | 184 ++++++-- .../powershell/src/types/scriptRecipe.ts | 2 +- .../powershell/test/actionHandler.spec.ts | 394 ++++++++++++++++ .../powershell/test/powerShellStore.spec.ts | 106 +++++ .../dispatcher/src/reasoning/claude.ts | 2 + .../src/reasoning/reasoningProfile.ts | 2 + .../src/reasoning/scriptRecipeGenerator.ts | 7 +- 19 files changed, 1133 insertions(+), 242 deletions(-) diff --git a/ts/docs/architecture/workflows/workflows.md b/ts/docs/architecture/workflows/workflows.md index 9ee6d7b60a..a3d4f7e1c6 100644 --- a/ts/docs/architecture/workflows/workflows.md +++ b/ts/docs/architecture/workflows/workflows.md @@ -356,13 +356,21 @@ PowerShell supports two creation paths: The `powershellRunner.mts` module spawns a PowerShell child process running `scriptHost.ps1`. Arguments are passed via command-line flags: -| Flag | Value | -| --------------------- | ------------------------------------- | -| `-ScriptBody` | The PowerShell script text | -| `-ParametersJson` | JSON-serialized parameter values | -| `-AllowedCmdletsJson` | JSON array of permitted cmdlet names | -| `-TimeoutSeconds` | Maximum execution time | -| `-AllowedPathsJson` | JSON array of permitted path patterns | +| Flag | Value | +| --------------------- | --------------------------------------------------------------------------------------- | +| `-ScriptBody` | The PowerShell script text | +| `-ParametersJson` | JSON-serialized parameter values | +| `-ParameterRolesJson` | Path and executable parameter roles derived from the flow's typed parameter definitions | +| `-AllowedCmdletsJson` | JSON array of permitted cmdlet names | +| `-TimeoutSeconds` | Maximum execution time | +| `-AllowedPathsJson` | JSON array of permitted path patterns | + +PowerShell recipes do not persist a separate `parameterRoles` property. +`scriptParameters[].type` is the source of truth: `path` parameters are +canonicalized as filesystem paths, while `executable` parameters resolve bare +application names through PowerShell command resolution before the resulting +file path is checked against `allowedPaths`. Other string parameters, including +file content, URLs, patterns, and command arguments, are not path-validated. ### Sandbox: constrained runspace diff --git a/ts/packages/agents/powershell/scripts/compileRecipes.mjs b/ts/packages/agents/powershell/scripts/compileRecipes.mjs index 05311842a7..9aa1ee3a6c 100644 --- a/ts/packages/agents/powershell/scripts/compileRecipes.mjs +++ b/ts/packages/agents/powershell/scripts/compileRecipes.mjs @@ -82,7 +82,10 @@ function buildTsType(recipe) { const paramLines = params .map((p) => { const opt = p.required === false ? "?" : ""; - const tsType = p.type === "path" ? "string" : p.type; + const tsType = + p.type === "path" || p.type === "executable" + ? "string" + : p.type; const comment = p.description ? ` // ${p.description}\n` : ""; @@ -97,7 +100,12 @@ function buildTsType(recipe) { function buildFlowJson(recipe) { const params = {}; for (const p of recipe.parameters || []) { - const def = { type: p.type === "path" ? "string" : p.type }; + const def = { + type: + p.type === "path" || p.type === "executable" + ? "string" + : p.type, + }; if (p.required !== undefined) def.required = p.required; if (p.default !== undefined) def.default = p.default; if (p.description) def.description = p.description; diff --git a/ts/packages/agents/powershell/scripts/scriptHost.ps1 b/ts/packages/agents/powershell/scripts/scriptHost.ps1 index 3be563e216..6e7c93bcba 100644 --- a/ts/packages/agents/powershell/scripts/scriptHost.ps1 +++ b/ts/packages/agents/powershell/scripts/scriptHost.ps1 @@ -13,6 +13,8 @@ param( [Parameter(Mandatory=$true)] [string]$ParametersJson, + [string]$ParameterRolesJson = '{}', + [Parameter(Mandatory=$true)] [string]$AllowedCmdletsJson, @@ -65,9 +67,51 @@ function Get-CanonicalFileSystemPath { return Remove-TrailingDirectorySeparator ([System.IO.Path]::GetFullPath($canonicalPath)) } +function Get-CanonicalExecutablePath { + param([string]$Path) + + if ( + [System.IO.Path]::IsPathRooted($Path) -or + $Path.Contains('\') -or + $Path.Contains('/') -or + $Path.StartsWith('.') + ) { + return Get-CanonicalFileSystemPath $Path + } + + $commands = @(Get-Command -Name $Path -CommandType Application -ErrorAction Stop) + if ($commands.Count -ne 1 -or -not $commands[0].Path) { + throw "Unable to resolve executable '$Path' to one application." + } + return Get-CanonicalFileSystemPath $commands[0].Path +} + +function Test-AllowedFileSystemPath { + param( + [string]$Path, + [string[]]$AllowedPaths + ) + + foreach ($allowedPath in $AllowedPaths) { + if ( + $Path.Equals($allowedPath, [System.StringComparison]::OrdinalIgnoreCase) -or + $Path.StartsWith("$allowedPath\", [System.StringComparison]::OrdinalIgnoreCase) -or + $Path.StartsWith("$allowedPath/", [System.StringComparison]::OrdinalIgnoreCase) + ) { + return $true + } + } + return $false +} + try { $allowedCmdlets = $AllowedCmdletsJson | ConvertFrom-Json $params = $ParametersJson | ConvertFrom-Json + $parameterRoles = $ParameterRolesJson | ConvertFrom-Json + if ($null -eq $parameterRoles -or $parameterRoles -isnot [pscustomobject]) { + Write-Error "Parameter roles must be a JSON object." + exit 1 + } # Parse allowed paths - must handle array properly to avoid PowerShell array unwrapping issues $parsedPaths = $AllowedPathsJson | ConvertFrom-Json if ($parsedPaths -is [array]) { @@ -96,46 +140,57 @@ try { } } - # Validate path parameters against allowed paths - # NOTE: Only validate paths that look like absolute or relative file paths. - # Skip short single-word strings (like "videos", "downloads") that might be - # library names - let the script handle those with its own resolution logic. - if ($expandedAllowedPaths.Count -gt 0) { - foreach ($prop in $params.PSObject.Properties) { - $val = $prop.Value - if ($val -is [string]) { - # Skip empty values - if (-not $val -or $val -match '^\s*$') { continue } - - # Skip short single-word values that look like library names - # (no slashes, no drive letter, not starting with dot) - if ($val -notmatch '[/\\]' -and $val -notmatch '^[a-zA-Z]:' -and $val -notmatch '^\.' -and $val.Length -lt 50) { - continue - } - - try { - $resolvedPath = Get-CanonicalFileSystemPath $val - } catch { - Write-Error "Invalid path parameter '$($prop.Name)': $_" - exit 1 - } - $pathAllowed = $false - foreach ($ap in $expandedAllowedPaths) { - if ( - $resolvedPath.Equals($ap, [System.StringComparison]::OrdinalIgnoreCase) -or - $resolvedPath.StartsWith("$ap\", [System.StringComparison]::OrdinalIgnoreCase) -or - $resolvedPath.StartsWith("$ap/", [System.StringComparison]::OrdinalIgnoreCase) - ) { - $pathAllowed = $true - break - } - } - # ENFORCEMENT: Block execution if path not allowed - if (-not $pathAllowed) { - Write-Error "Path access denied: '$resolvedPath' is not in allowedPaths. Allowed paths: $($expandedAllowedPaths -join ', ')" - exit 1 - } + $roleProperties = @($parameterRoles.PSObject.Properties) + if ($roleProperties.Count -gt 0 -and $expandedAllowedPaths.Count -eq 0) { + Write-Error "Path parameter roles require at least one allowed path." + exit 1 + } + + foreach ($roleProperty in $roleProperties) { + $role = [string]$roleProperty.Value + if ($role -ne 'path' -and $role -ne 'executable') { + Write-Error "Unsupported parameter role '$role' for '$($roleProperty.Name)'." + exit 1 + } + + $parameterProperty = @( + $params.PSObject.Properties | + Where-Object { $_.Name -ieq $roleProperty.Name } + ) | Select-Object -First 1 + if ($null -eq $parameterProperty) { + continue + } + + $value = $parameterProperty.Value + if ($null -eq $value -or $value -eq '') { + continue + } + if ($value -isnot [string]) { + Write-Error "Parameter '$($parameterProperty.Name)' with role '$role' must be a string." + exit 1 + } + if ([System.Management.Automation.WildcardPattern]::ContainsWildcardCharacters($value)) { + Write-Error "Parameter '$($parameterProperty.Name)' with role '$role' cannot contain wildcard characters." + exit 1 + } + if ($value -match '^[a-zA-Z][a-zA-Z0-9-]*:' -and $value -notmatch '^[a-zA-Z]:[\\/]') { + Write-Error "Parameter '$($parameterProperty.Name)' uses an unsupported provider or URI path." + exit 1 + } + + try { + $resolvedPath = if ($role -eq 'executable') { + Get-CanonicalExecutablePath $value + } else { + Get-CanonicalFileSystemPath $value } + } catch { + Write-Error "Invalid $role parameter '$($parameterProperty.Name)': $_" + exit 1 + } + if (-not (Test-AllowedFileSystemPath $resolvedPath $expandedAllowedPaths)) { + Write-Error "Path access denied: '$resolvedPath' is not in allowedPaths. Allowed paths: $($expandedAllowedPaths -join ', ')" + exit 1 } } diff --git a/ts/packages/agents/powershell/src/actionHandler.mts b/ts/packages/agents/powershell/src/actionHandler.mts index f91fc6350b..4e09d0ca9c 100644 --- a/ts/packages/agents/powershell/src/actionHandler.mts +++ b/ts/packages/agents/powershell/src/actionHandler.mts @@ -37,6 +37,7 @@ import { import { executeScript, type ScriptExecutionRequest, + type ScriptParameterRole, } from "./execution/powershellRunner.mjs"; import { createPowerShellExecutionFailure, @@ -123,6 +124,7 @@ async function executeFlowScript( const request: ScriptExecutionRequest = { script, parameters: resolvedParams, + parameterRoles: getScriptParameterRoles(flow.parameters), sandbox: { allowedCmdlets: flow.sandbox.allowedCmdlets, allowedPaths: flow.sandbox.allowedPaths, @@ -148,6 +150,24 @@ async function executeFlowScript( return createPowerShellExecutionFailure(result); } +function getScriptParameterRoles( + paramDefs: ScriptParameter[], +): Record { + return Object.fromEntries( + paramDefs + .filter( + ( + parameter, + ): parameter is ScriptParameter & { + type: ScriptParameterRole; + } => + parameter.type === "path" || + parameter.type === "executable", + ) + .map((parameter) => [parameter.name, parameter.type]), + ); +} + function mapParamsToFlowDefs( provided: Record, paramDefs: ScriptParameter[], @@ -189,7 +209,7 @@ function validatePathParameters( paramDefs: ScriptParameter[], ): string | undefined { for (const def of paramDefs) { - if (def.type !== "path") continue; + if (def.type !== "path" && def.type !== "executable") continue; const val = params[def.name]; if (val === undefined || val === "") continue; if (typeof val !== "string") { @@ -391,6 +411,50 @@ function parseNamedParameters( } } +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +function createPostExecutionFailure( + flowName: string, + phase: string, + error?: unknown, +): ActionResult { + const detail = error === undefined ? "" : `: ${errorMessage(error)}`; + return createPowerShellFailure( + "partialSideEffects", + `PowerShell flow '${flowName}' executed, but ${phase}${detail}. The operation may have caused side effects and was not executed again.`, + ); +} + +function getPostExecutionCancellation( + flowName: string, + phase: string, + abortSignal?: AbortSignal, +): ActionResult | undefined { + if (!abortSignal?.aborted) { + return undefined; + } + return createPostExecutionFailure( + flowName, + `the request was cancelled ${phase}`, + ); +} + +async function recordUsageAfterExecution( + flowStore: PowerShellStore, + flowName: string, + context: ActionContext, +): Promise { + try { + await flowStore.recordUsage(flowName); + } catch (error) { + const message = `PowerShell flow '${flowName}' executed successfully, but usage accounting failed: ${errorMessage(error)}`; + debug(message); + context.sessionContext.notify(AppAgentEvent.Warning, message); + } +} + async function executeDraftRecipe( recipe: ScriptRecipe, suppliedParameters: Record, @@ -419,6 +483,7 @@ async function executeDraftRecipe( const result = await executeScript({ script: recipe.script.body, parameters: executionParameters, + parameterRoles: getScriptParameterRoles(recipe.parameters), sandbox: recipe.sandbox, workingDirectory: homedir(), abortSignal, @@ -495,7 +560,7 @@ async function createOrReusePowerShellFlow( context.abortSignal, ); if (result.error === undefined) { - await flowStore.recordUsage(actionName); + await recordUsageAfterExecution(flowStore, actionName, context); } return result; } @@ -523,7 +588,6 @@ async function createOrReusePowerShellFlow( executionParameters.parameters, context.abortSignal, ); - context.abortSignal?.throwIfAborted(); } catch (error) { await flowStore.deletePending(pendingFile); throw error; @@ -533,26 +597,68 @@ async function createOrReusePowerShellFlow( return execution.error; } - const promoted = await flowStore.promotePending(pendingFile); + const cancellation = getPostExecutionCancellation( + actionName, + "before the flow could be promoted", + context.abortSignal, + ); + if (cancellation) { + await flowStore.deletePending(pendingFile); + return cancellation; + } + + let promoted: string | null; + try { + promoted = await flowStore.promotePending(pendingFile); + } catch (error) { + await flowStore.deletePending(pendingFile); + return createPostExecutionFailure( + actionName, + "the flow could not be promoted", + error, + ); + } if (!promoted) { await flowStore.deletePending(pendingFile); - return createPowerShellFailure( - "partialSideEffects", - `The script executed, but flow '${actionName}' could not be promoted because that name is already registered. The operation may have caused side effects and was not executed again.`, + return createPostExecutionFailure( + actionName, + "the flow could not be promoted because that name is already registered", ); } try { - context.abortSignal?.throwIfAborted(); + const activationCancellation = getPostExecutionCancellation( + actionName, + "before the promoted flow could be activated", + context.abortSignal, + ); + if (activationCancellation) { + await flowStore.deleteFlow(promoted); + return activationCancellation; + } await context.sessionContext.reloadAgentSchema(); - context.abortSignal?.throwIfAborted(); + const completedCancellation = getPostExecutionCancellation( + actionName, + "after the flow was activated", + context.abortSignal, + ); + if (completedCancellation) { + return completedCancellation; + } } catch (error) { - await flowStore.deleteFlow(promoted); - if (context.abortSignal?.aborted) { - context.abortSignal.throwIfAborted(); + let cleanupError: unknown; + try { + await flowStore.deleteFlow(promoted); + } catch (deleteError) { + cleanupError = deleteError; } - return createPowerShellFailure( - "partialSideEffects", - `The script executed, but the new flow could not be activated: ${error instanceof Error ? error.message : String(error)}. The operation may have caused side effects and was not executed again.`, + const cleanupDetail = + cleanupError === undefined + ? "" + : ` Cleanup also failed: ${errorMessage(cleanupError)}.`; + return createPostExecutionFailure( + actionName, + `the new flow could not be activated${cleanupDetail}`, + error, ); } @@ -633,33 +739,84 @@ async function repairAndExecutePowerShellFlow( if ("error" in execution) { return execution.error; } - context.abortSignal?.throwIfAborted(); - await flowStore.updateFlowScript( + + const updateCancellation = getPostExecutionCancellation( flowName, - script, - candidate.sandbox.allowedCmdlets, - candidate.sandbox.allowedModules, + "before the repaired script could be saved", + context.abortSignal, ); + if (updateCancellation) { + return updateCancellation; + } try { - context.abortSignal?.throwIfAborted(); - await context.sessionContext.reloadAgentSchema(); - context.abortSignal?.throwIfAborted(); - } catch (error) { await flowStore.updateFlowScript( flowName, - oldScript, - existing.sandbox.allowedCmdlets, - existing.sandbox.allowedModules, + script, + candidate.sandbox.allowedCmdlets, + candidate.sandbox.allowedModules, + ); + } catch (error) { + return createPostExecutionFailure( + flowName, + "the repaired script could not be saved", + error, ); - if (context.abortSignal?.aborted) { - context.abortSignal.throwIfAborted(); + } + + try { + const activationCancellation = getPostExecutionCancellation( + flowName, + "before the repaired flow could be activated", + context.abortSignal, + ); + if (activationCancellation) { + try { + await flowStore.updateFlowScript( + flowName, + oldScript, + existing.sandbox.allowedCmdlets, + existing.sandbox.allowedModules, + ); + return activationCancellation; + } catch (restoreError) { + return createPostExecutionFailure( + flowName, + `the request was cancelled before the repaired flow could be activated, and restoring the previous flow failed: ${errorMessage(restoreError)}`, + ); + } } - return createPowerShellFailure( - "partialSideEffects", - `The repaired script executed, but the flow could not be activated: ${error instanceof Error ? error.message : String(error)}.`, + await context.sessionContext.reloadAgentSchema(); + const completedCancellation = getPostExecutionCancellation( + flowName, + "after the repaired flow was activated", + context.abortSignal, + ); + if (completedCancellation) { + return completedCancellation; + } + } catch (error) { + let restorationError: unknown; + try { + await flowStore.updateFlowScript( + flowName, + oldScript, + existing.sandbox.allowedCmdlets, + existing.sandbox.allowedModules, + ); + } catch (restoreError) { + restorationError = restoreError; + } + const restorationDetail = + restorationError === undefined + ? "" + : ` Restoring the previous flow also failed: ${errorMessage(restorationError)}.`; + return createPostExecutionFailure( + flowName, + `the repaired flow could not be activated${restorationDetail}`, + error, ); } - await flowStore.recordUsage(flowName); + await recordUsageAfterExecution(flowStore, flowName, context); return createActionResultFromTextDisplay( `${execution.output}\n\nRepaired PowerShell flow '${flowName}' after one retry.`, ); @@ -712,16 +869,18 @@ async function handlePowerShellFlowAction( "Missing required parameter: name", ); } - const deleted = await flowStore.deleteFlow(name); - if (!deleted) { - return createActionResultFromError( - `Script flow not found: ${name}`, + return withFlowMutationLock(name, async () => { + const deleted = await flowStore.deleteFlow(name); + if (!deleted) { + return createActionResultFromError( + `Script flow not found: ${name}`, + ); + } + await context.sessionContext.reloadAgentSchema(); + return createActionResultFromTextDisplay( + `Deleted PowerShell flow: ${name}`, ); - } - await context.sessionContext.reloadAgentSchema(); - return createActionResultFromTextDisplay( - `Deleted PowerShell flow: ${name}`, - ); + }); } case "createPowerShellFlow": { @@ -762,19 +921,26 @@ async function handlePowerShellFlowAction( grammarValidation.patterns, ); - context.abortSignal?.throwIfAborted(); - await flowStore.saveFlow(recipe, "reasoning"); - try { - context.abortSignal?.throwIfAborted(); - await context.sessionContext.reloadAgentSchema(); + return withFlowMutationLock(newActionName, async () => { + if (flowStore.hasFlow(newActionName)) { + return createActionResultFromError( + `A PowerShell flow named '${newActionName}' already exists. Reuse it or add grammar patterns instead of overwriting it.`, + ); + } context.abortSignal?.throwIfAborted(); - } catch (error) { - await flowStore.deleteFlow(newActionName); - throw error; - } - return createActionResultFromTextDisplay( - `Created PowerShell flow '${newActionName}': ${recipe.description}`, - ); + await flowStore.saveFlow(recipe, "reasoning"); + try { + context.abortSignal?.throwIfAborted(); + await context.sessionContext.reloadAgentSchema(); + context.abortSignal?.throwIfAborted(); + } catch (error) { + await flowStore.deleteFlow(newActionName); + throw error; + } + return createActionResultFromTextDisplay( + `Created PowerShell flow '${newActionName}': ${recipe.description}`, + ); + }); } case "createAndExecutePowerShellFlow": { @@ -802,38 +968,40 @@ async function handlePowerShellFlowAction( "Missing required parameter: flowName", ); } - const flow = await flowStore.getFlow(flowName); - if (!flow) { - return createActionResultFromError( - `Script flow not found: ${flowName}`, + return withFlowMutationLock(flowName, async () => { + const flow = await flowStore.getFlow(flowName); + if (!flow) { + return createActionResultFromError( + `Script flow not found: ${flowName}`, + ); + } + const validation = await validateFlowGrammarPatterns( + flowName, + flow.description, + (action.parameters + ?.grammarPatterns as FlowGrammarPatternInput[]) ?? [], + context, ); - } - const validation = await validateFlowGrammarPatterns( - flowName, - flow.description, - (action.parameters - ?.grammarPatterns as FlowGrammarPatternInput[]) ?? [], - context, - ); - if ("error" in validation) { - return validation.error; - } - const added = await flowStore.addGrammarPatterns( - flowName, - validation.patterns.map((pattern) => ({ - pattern: pattern.pattern, - isAlias: pattern.isAlias ?? true, - examples: [], - })), - ); - if (added > 0) { - await context.sessionContext.reloadAgentSchema(); - } - return createActionResultFromTextDisplay( - added > 0 - ? `Added ${added} grammar pattern(s) to PowerShell flow '${flowName}'.` - : `PowerShell flow '${flowName}' already contains those grammar patterns.`, - ); + if ("error" in validation) { + return validation.error; + } + const added = await flowStore.addGrammarPatterns( + flowName, + validation.patterns.map((pattern) => ({ + pattern: pattern.pattern, + isAlias: pattern.isAlias ?? true, + examples: [], + })), + ); + if (added > 0) { + await context.sessionContext.reloadAgentSchema(); + } + return createActionResultFromTextDisplay( + added > 0 + ? `Added ${added} grammar pattern(s) to PowerShell flow '${flowName}'.` + : `PowerShell flow '${flowName}' already contains those grammar patterns.`, + ); + }); } case "reportPowerShellCapabilityOutcome": @@ -868,35 +1036,36 @@ async function handlePowerShellFlowAction( "Missing required parameter: flowName", ); } - const existingFlow = await flowStore.getFlow(editFlowName); - if (!existingFlow) { - return createActionResultFromError( - `Script flow not found: ${editFlowName}`, - ); - } const newScript = action.parameters?.script as string | undefined; if (!newScript) { return createActionResultFromError( "Missing required parameter: script", ); } - const newCmdlets = - (action.parameters?.allowedCmdlets as string[]) ?? - existingFlow.sandbox.allowedCmdlets; - const newModules = - (action.parameters?.allowedModules as string[]) ?? - existingFlow.sandbox.allowedModules; - - // Update the script and sandbox policy while preserving everything else - await flowStore.updateFlowScript( - editFlowName, - newScript, - newCmdlets, - newModules, - ); - return createActionResultFromTextDisplay( - `Updated PowerShell flow '${editFlowName}'`, - ); + return withFlowMutationLock(editFlowName, async () => { + const existingFlow = await flowStore.getFlow(editFlowName); + if (!existingFlow) { + return createActionResultFromError( + `Script flow not found: ${editFlowName}`, + ); + } + const newCmdlets = + (action.parameters?.allowedCmdlets as string[]) ?? + existingFlow.sandbox.allowedCmdlets; + const newModules = + (action.parameters?.allowedModules as string[]) ?? + existingFlow.sandbox.allowedModules; + + await flowStore.updateFlowScript( + editFlowName, + newScript, + newCmdlets, + newModules, + ); + return createActionResultFromTextDisplay( + `Updated PowerShell flow '${editFlowName}'`, + ); + }); } case "testPowerShellFlow": { @@ -1055,7 +1224,7 @@ async function handlePowerShellFlowAction( return { ...result, fallbackToReasoning: true }; } - await flowStore.recordUsage(flowName); + await recordUsageAfterExecution(flowStore, flowName, context); return result; } @@ -1118,29 +1287,31 @@ async function handlePowerShellFlowAction( ); } - if (flowStore.hasFlow(recipe.actionName)) { - return createActionResultFromError( - `A flow named '${recipe.actionName}' already exists. Use a different name: @powershell import ${filePath} with actionName set to a new name`, - ); - } + return withFlowMutationLock(recipe.actionName, async () => { + if (flowStore.hasFlow(recipe.actionName)) { + return createActionResultFromError( + `A flow named '${recipe.actionName}' already exists. Use a different name: @powershell import ${filePath} with actionName set to a new name`, + ); + } - context.abortSignal?.throwIfAborted(); - await flowStore.saveFlow(recipe, "manual"); - try { context.abortSignal?.throwIfAborted(); - await context.sessionContext.reloadAgentSchema(); - context.abortSignal?.throwIfAborted(); - } catch (error) { - await flowStore.deleteFlow(recipe.actionName); - throw error; - } + await flowStore.saveFlow(recipe, "manual"); + try { + context.abortSignal?.throwIfAborted(); + await context.sessionContext.reloadAgentSchema(); + context.abortSignal?.throwIfAborted(); + } catch (error) { + await flowStore.deleteFlow(recipe.actionName); + throw error; + } - const patternList = recipe.grammarPatterns - .map((p) => ` "${p.pattern}"`) - .join("\n"); - return createActionResultFromTextDisplay( - `Imported PowerShell flow '${recipe.actionName}': ${recipe.description}\n\nGrammar patterns:\n${patternList}`, - ); + const patternList = recipe.grammarPatterns + .map((p) => ` "${p.pattern}"`) + .join("\n"); + return createActionResultFromTextDisplay( + `Imported PowerShell flow '${recipe.actionName}': ${recipe.description}\n\nGrammar patterns:\n${patternList}`, + ); + }); } default: { @@ -1197,7 +1368,11 @@ async function handlePowerShellFlowAction( return { ...result, fallbackToReasoning: true }; } - await flowStore.recordUsage(action.actionName); + await recordUsageAfterExecution( + flowStore, + action.actionName, + context, + ); return result; } } diff --git a/ts/packages/agents/powershell/src/analysis/scriptAnalyzer.mts b/ts/packages/agents/powershell/src/analysis/scriptAnalyzer.mts index 94eb9d9eca..9b7754a067 100644 --- a/ts/packages/agents/powershell/src/analysis/scriptAnalyzer.mts +++ b/ts/packages/agents/powershell/src/analysis/scriptAnalyzer.mts @@ -101,7 +101,9 @@ Analyze this script and generate a recipe JSON object: 2. **description**: Concise description of what the script does. 3. **displayName**: Human-readable name. 4. **parameters**: Extract from the param() block if present. Map PowerShell types: - [string] -> "string", [int] -> "number", [bool]/[switch] -> "boolean", paths -> "path". + [string] -> "string", [int] -> "number", [bool]/[switch] -> "boolean", + filesystem paths -> "path", and values passed to executable command parameters + such as Start-Process -FilePath -> "executable". Include defaults from the param() block. If no param() block exists, infer likely parameters from hardcoded values in the script. 5. **script.body**: Use the EXACT script content provided. Do NOT modify it. @@ -122,7 +124,7 @@ Return ONLY a JSON object matching this schema (no markdown fences, no explanati "description": "what this script does", "displayName": "Human Readable Name", "parameters": [ - { "name": "paramName", "type": "string|number|boolean|path", "required": true, "description": "...", "default": "optional default" } + { "name": "paramName", "type": "string|number|boolean|path|executable", "required": true, "description": "...", "default": "optional default" } ], "script": { "language": "powershell", diff --git a/ts/packages/agents/powershell/src/execution/powershellRunner.mts b/ts/packages/agents/powershell/src/execution/powershellRunner.mts index d64327a732..51b5a3f482 100644 --- a/ts/packages/agents/powershell/src/execution/powershellRunner.mts +++ b/ts/packages/agents/powershell/src/execution/powershellRunner.mts @@ -33,9 +33,12 @@ const packageRoot = findPackageRoot(); const MAX_OUTPUT_SIZE = 1024 * 1024; // 1MB +export type ScriptParameterRole = "path" | "executable"; + export interface ScriptExecutionRequest { script: string; parameters: Record; + parameterRoles?: Partial>; sandbox: { allowedCmdlets: string[]; allowedPaths: string[]; @@ -73,6 +76,8 @@ export async function executeScript( request.script, "-ParametersJson", JSON.stringify(request.parameters), + "-ParameterRolesJson", + JSON.stringify(request.parameterRoles ?? {}), "-AllowedCmdletsJson", JSON.stringify(request.sandbox.allowedCmdlets), "-NetworkAccess", diff --git a/ts/packages/agents/powershell/src/namespaces/archives/actionHandler.mts b/ts/packages/agents/powershell/src/namespaces/archives/actionHandler.mts index 0d4854796f..c8039623c9 100644 --- a/ts/packages/agents/powershell/src/namespaces/archives/actionHandler.mts +++ b/ts/packages/agents/powershell/src/namespaces/archives/actionHandler.mts @@ -34,6 +34,10 @@ if ([System.IO.Directory]::Exists($SourcePath)) { }`, allowedCmdlets: ["Add-Type", "Out-Null"], allowedPaths, + parameterRoles: { + sourcePath: "path", + destinationPath: "path", + }, confirmation: "Create the requested ZIP archive?", }, expand: { @@ -44,6 +48,10 @@ if (-not $DestinationPath) { $DestinationPath = "." } [System.IO.Compression.ZipFile]::ExtractToDirectory($ArchivePath, $DestinationPath)`, allowedCmdlets: ["Add-Type"], allowedPaths, + parameterRoles: { + archivePath: "path", + destinationPath: "path", + }, confirmation: "Extract the requested archive?", }, } satisfies NamespaceActionDefinitions; diff --git a/ts/packages/agents/powershell/src/namespaces/data/actionHandler.mts b/ts/packages/agents/powershell/src/namespaces/data/actionHandler.mts index 9e357a0183..640937a869 100644 --- a/ts/packages/agents/powershell/src/namespaces/data/actionHandler.mts +++ b/ts/packages/agents/powershell/src/namespaces/data/actionHandler.mts @@ -19,12 +19,14 @@ if ($PropertyPath) { $value`, allowedCmdlets: ["Get-Content", "ConvertFrom-Json"], allowedPaths, + parameterRoles: { path: "path" }, }, writeJson: { script: `param([string]$Path, [string]$Data) $Data | ConvertFrom-Json | ConvertTo-Json -Depth 20 | Set-Content -LiteralPath $Path`, allowedCmdlets: ["ConvertFrom-Json", "ConvertTo-Json", "Set-Content"], allowedPaths, + parameterRoles: { path: "path" }, confirmation: "Write JSON data to the requested file?", }, readCsv: { @@ -33,12 +35,14 @@ if (-not $Delimiter) { $Delimiter = "," } Import-Csv -LiteralPath $Path -Delimiter $Delimiter`, allowedCmdlets: ["Import-Csv"], allowedPaths, + parameterRoles: { path: "path" }, }, writeCsv: { script: `param([string]$Path, [string]$Data) $Data | ConvertFrom-Json | Export-Csv -LiteralPath $Path -NoTypeInformation`, allowedCmdlets: ["ConvertFrom-Json", "Export-Csv"], allowedPaths, + parameterRoles: { path: "path" }, confirmation: "Write CSV data to the requested file?", }, filterCsv: { @@ -46,6 +50,7 @@ $Data | ConvertFrom-Json | Export-Csv -LiteralPath $Path -NoTypeInformation`, Import-Csv -LiteralPath $Path | Where-Object { $_.$Column -match $Pattern }`, allowedCmdlets: ["Import-Csv", "Where-Object"], allowedPaths, + parameterRoles: { path: "path" }, }, convertFormat: { script: `param([string]$Input, [string]$Format) @@ -73,6 +78,7 @@ if ($Format -eq "json") { "ConvertTo-Xml", ], allowedPaths, + parameterRoles: { input: "path" }, }, } satisfies NamespaceActionDefinitions; diff --git a/ts/packages/agents/powershell/src/namespaces/files/actionHandler.mts b/ts/packages/agents/powershell/src/namespaces/files/actionHandler.mts index a7784ca0e3..f811c1dcf4 100644 --- a/ts/packages/agents/powershell/src/namespaces/files/actionHandler.mts +++ b/ts/packages/agents/powershell/src/namespaces/files/actionHandler.mts @@ -20,6 +20,7 @@ if ($Filter) { }`, allowedCmdlets: ["Get-ChildItem"], allowedPaths, + parameterRoles: { path: "path" }, }, readFile: { script: `param([string]$Path, [int]$Tail, [int]$Head) @@ -32,6 +33,7 @@ if ($Tail -gt 0) { }`, allowedCmdlets: ["Get-Content"], allowedPaths, + parameterRoles: { path: "path" }, }, writeFile: { script: `param([string]$Path, [string]$Content, [bool]$Append) @@ -42,6 +44,7 @@ if ($Append) { }`, allowedCmdlets: ["Add-Content", "Set-Content"], allowedPaths, + parameterRoles: { path: "path" }, confirmation: "Write content to the requested file?", }, copyFile: { @@ -49,6 +52,7 @@ if ($Append) { Copy-Item -LiteralPath $Source -Destination $Destination -Recurse:$Recurse`, allowedCmdlets: ["Copy-Item"], allowedPaths, + parameterRoles: { source: "path", destination: "path" }, confirmation: "Copy the requested file or directory?", }, moveFile: { @@ -56,6 +60,7 @@ Copy-Item -LiteralPath $Source -Destination $Destination -Recurse:$Recurse`, Move-Item -LiteralPath $Source -Destination $Destination`, allowedCmdlets: ["Move-Item"], allowedPaths, + parameterRoles: { source: "path", destination: "path" }, confirmation: "Move or rename the requested file or directory?", }, deleteFile: { @@ -63,6 +68,7 @@ Move-Item -LiteralPath $Source -Destination $Destination`, Remove-Item -LiteralPath $Path -Recurse:$Recurse`, allowedCmdlets: ["Remove-Item"], allowedPaths, + parameterRoles: { path: "path" }, confirmation: "Delete the requested file or directory?", }, testPath: { @@ -70,6 +76,7 @@ Remove-Item -LiteralPath $Path -Recurse:$Recurse`, Test-Path -LiteralPath $Path`, allowedCmdlets: ["Test-Path"], allowedPaths, + parameterRoles: { path: "path" }, }, findText: { script: `param([string]$Pattern, [string]$Path, [string]$Include) @@ -81,6 +88,7 @@ if ($Include) { }`, allowedCmdlets: ["Get-ChildItem", "Select-String"], allowedPaths, + parameterRoles: { path: "path" }, }, newItem: { script: `param([string]$Path, [string]$ItemType) @@ -88,6 +96,7 @@ $type = if ($ItemType -eq "directory") { "Directory" } else { "File" } New-Item -Path $Path -ItemType $type`, allowedCmdlets: ["New-Item"], allowedPaths, + parameterRoles: { path: "path" }, confirmation: "Create the requested file or directory?", }, } satisfies NamespaceActionDefinitions; diff --git a/ts/packages/agents/powershell/src/namespaces/namespaceActionHandler.mts b/ts/packages/agents/powershell/src/namespaces/namespaceActionHandler.mts index fcc821f044..28b708341f 100644 --- a/ts/packages/agents/powershell/src/namespaces/namespaceActionHandler.mts +++ b/ts/packages/agents/powershell/src/namespaces/namespaceActionHandler.mts @@ -4,7 +4,10 @@ import type { ActionContext, ActionResult } from "@typeagent/agent-sdk"; import { createActionResultFromTextDisplay } from "@typeagent/agent-sdk/helpers/action"; import { homedir } from "os"; -import { executeScript } from "../execution/powershellRunner.mjs"; +import { + executeScript, + type ScriptParameterRole, +} from "../execution/powershellRunner.mjs"; import type { PowerShellAgentContext } from "../types/powerShellAgentContext.mjs"; import { createPowerShellExecutionFailure, @@ -17,20 +20,34 @@ export type PowerShellAction = { parameters?: Record; }; -export type StaticPowerShellActionDefinition = { +type ActionParameters< + TAction extends { actionName: string; parameters: Record }, + TName extends TAction["actionName"], +> = Extract["parameters"]; + +export type StaticPowerShellActionDefinition< + TParameterName extends string = string, +> = { script: string; allowedCmdlets: readonly string[]; allowedPaths?: readonly string[]; + parameterRoles?: Partial>; allowedModules?: readonly string[]; networkAccess?: boolean; maxExecutionTime?: number; confirmation?: string; }; -export type NamespaceActionDefinitions = - { - [Name in TAction["actionName"]]: StaticPowerShellActionDefinition; - }; +export type NamespaceActionDefinitions< + TAction extends { + actionName: string; + parameters: Record; + }, +> = { + [Name in TAction["actionName"]]: StaticPowerShellActionDefinition< + Extract, string> + >; +}; export interface PowerShellNamespaceActionHandler { readonly schemaName: string; @@ -43,7 +60,10 @@ export interface PowerShellNamespaceActionHandler { } export function createPowerShellNamespaceActionHandler< - TAction extends { actionName: string }, + TAction extends { + actionName: string; + parameters: Record; + }, >( schemaName: string, definitions: NamespaceActionDefinitions, @@ -89,6 +109,9 @@ export function createPowerShellNamespaceActionHandler< const result = await executeScript({ script: definition.script, parameters: action.parameters ?? {}, + ...(definition.parameterRoles + ? { parameterRoles: definition.parameterRoles } + : {}), sandbox: { allowedCmdlets: [...definition.allowedCmdlets], allowedPaths: [...(definition.allowedPaths ?? [])], diff --git a/ts/packages/agents/powershell/src/namespaces/processes/actionHandler.mts b/ts/packages/agents/powershell/src/namespaces/processes/actionHandler.mts index f72158d766..3913834e73 100644 --- a/ts/packages/agents/powershell/src/namespaces/processes/actionHandler.mts +++ b/ts/packages/agents/powershell/src/namespaces/processes/actionHandler.mts @@ -39,6 +39,7 @@ if ($Id -gt 0) { Stop-Process -Id $Id } else { Stop-Process -Name $Name }`, if ($Arguments) { Start-Process -FilePath $Path -ArgumentList $Arguments } else { Start-Process -FilePath $Path }`, allowedCmdlets: ["Start-Process"], allowedPaths: ["$env:USERPROFILE", "$PWD", "$env:TEMP"], + parameterRoles: { path: "executable" }, confirmation: "Start the requested process?", }, waitProcess: { diff --git a/ts/packages/agents/powershell/src/schema/scriptActions.mts b/ts/packages/agents/powershell/src/schema/scriptActions.mts index 7340b55eb6..8eca41f03a 100644 --- a/ts/packages/agents/powershell/src/schema/scriptActions.mts +++ b/ts/packages/agents/powershell/src/schema/scriptActions.mts @@ -43,7 +43,7 @@ export type CreatePowerShellFlow = { // Script parameters scriptParameters: { name: string; - type: "string" | "number" | "boolean" | "path"; + type: "string" | "number" | "boolean" | "path" | "executable"; required: boolean; description: string; default?: string; @@ -78,7 +78,7 @@ export type CreateAndExecutePowerShellFlow = { // Script parameters scriptParameters: { name: string; - type: "string" | "number" | "boolean" | "path"; + type: "string" | "number" | "boolean" | "path" | "executable"; required: boolean; description: string; default?: string; diff --git a/ts/packages/agents/powershell/src/store/powerShellStore.mts b/ts/packages/agents/powershell/src/store/powerShellStore.mts index 55118b12f3..445a2b51ea 100644 --- a/ts/packages/agents/powershell/src/store/powerShellStore.mts +++ b/ts/packages/agents/powershell/src/store/powerShellStore.mts @@ -17,6 +17,28 @@ import registerDebug from "debug"; const debug = registerDebug("typeagent:powershell:store"); +function throwPersistenceError( + error: unknown, + rollbackErrors: unknown[], +): never { + if (rollbackErrors.length === 0) { + throw error; + } + const originalMessage = + error instanceof Error ? error.message : String(error); + const rollbackMessage = rollbackErrors + .map((rollbackError) => + rollbackError instanceof Error + ? rollbackError.message + : String(rollbackError), + ) + .join("; "); + throw new AggregateError( + [error, ...rollbackErrors], + `${originalMessage}. Rollback failed: ${rollbackMessage}`, + ); +} + export interface PowerShellFlowIndex { version: 1; flows: Record; @@ -26,7 +48,7 @@ export interface PowerShellFlowIndex { export interface PowerShellFlowParameterMeta { name: string; - type: "string" | "number" | "boolean" | "path"; + type: "string" | "number" | "boolean" | "path" | "executable"; required: boolean; description: string; } @@ -112,6 +134,7 @@ export class PowerShellStore { } const flowPath = `flows/${actionName}.flow.json`; const scriptPath = `scripts/${actionName}.ps1`; + let addedEntry: PowerShellFlowIndexEntry | undefined; const flowDef: PowerShellFlowDefinition = { version: 1, @@ -126,44 +149,75 @@ export class PowerShellStore { source: recipe.source, }; - await this.storage.write(flowPath, JSON.stringify(flowDef, null, 2)); - await this.storage.write(scriptPath, recipe.script.body); + try { + await this.storage.write( + flowPath, + JSON.stringify(flowDef, null, 2), + ); + await this.storage.write(scriptPath, recipe.script.body); - const grammarRuleText = generateGrammarRuleText( - actionName, - recipe.grammarPatterns, - ); + const grammarRuleText = generateGrammarRuleText( + actionName, + recipe.grammarPatterns, + ); - const paramMeta: PowerShellFlowParameterMeta[] = recipe.parameters.map( - (p) => ({ - name: p.name, - type: p.type, - required: p.required, - description: p.description, - }), - ); + const paramMeta: PowerShellFlowParameterMeta[] = + recipe.parameters.map((p) => ({ + name: p.name, + type: p.type, + required: p.required, + description: p.description, + })); + + const now = new Date().toISOString(); + addedEntry = { + actionName, + displayName: recipe.displayName, + description: recipe.description, + flowPath, + scriptPath, + grammarRuleText, + parameters: paramMeta, + created: now, + updated: now, + source, + usageCount: 0, + enabled: true, + }; + this.index.flows[actionName] = addedEntry; + this.index.lastModified = now; - const now = new Date().toISOString(); - this.index.flows[actionName] = { - actionName, - displayName: recipe.displayName, - description: recipe.description, - flowPath, - scriptPath, - grammarRuleText, - parameters: paramMeta, - created: now, - updated: now, - source, - usageCount: 0, - enabled: true, - }; - this.index.lastModified = now; - - await this.saveIndex(); - await this.writeDynamicGrammarFile(); - debug(`Flow saved: ${actionName}`); - return actionName; + await this.saveIndex(); + await this.writeDynamicGrammarFile(); + debug(`Flow saved: ${actionName}`); + return actionName; + } catch (error) { + const rollbackErrors: unknown[] = []; + const currentEntry = this.index.flows[actionName]; + if (currentEntry === undefined || currentEntry === addedEntry) { + for (const path of [flowPath, scriptPath]) { + try { + await this.storage.delete(path); + } catch (rollbackError) { + rollbackErrors.push(rollbackError); + } + } + } + if (currentEntry === addedEntry) { + delete this.index.flows[actionName]; + } + try { + await this.saveIndex(); + } catch (rollbackError) { + rollbackErrors.push(rollbackError); + } + try { + await this.writeDynamicGrammarFile(); + } catch (rollbackError) { + rollbackErrors.push(rollbackError); + } + throwPersistenceError(error, rollbackErrors); + } } async updateFlowScript( @@ -176,22 +230,54 @@ export class PowerShellStore { const entry = this.index.flows[actionName]; if (!entry) throw new Error(`Flow not found: ${actionName}`); - // Update the script file - await this.storage.write(entry.scriptPath, newScript); - - // Update the flow definition's sandbox cmdlets (and modules if provided) - const json = await this.storage.read(entry.flowPath, "utf8"); - const flow = JSON.parse(json) as PowerShellFlowDefinition; + const previousScript = await this.storage.read( + entry.scriptPath, + "utf8", + ); + const previousFlowJson = await this.storage.read( + entry.flowPath, + "utf8", + ); + const previousEntry = JSON.parse( + JSON.stringify(entry), + ) as PowerShellFlowIndexEntry; + const flow = JSON.parse(previousFlowJson) as PowerShellFlowDefinition; flow.sandbox.allowedCmdlets = newCmdlets; if (newModules !== undefined) { flow.sandbox.allowedModules = newModules; } - await this.storage.write(entry.flowPath, JSON.stringify(flow, null, 2)); - entry.updated = new Date().toISOString(); - this.index.lastModified = entry.updated; - await this.saveIndex(); - debug(`Flow script updated: ${actionName}`); + try { + await this.storage.write(entry.scriptPath, newScript); + await this.storage.write( + entry.flowPath, + JSON.stringify(flow, null, 2), + ); + + entry.updated = new Date().toISOString(); + this.index.lastModified = entry.updated; + await this.saveIndex(); + debug(`Flow script updated: ${actionName}`); + } catch (error) { + const rollbackErrors: unknown[] = []; + try { + await this.storage.write(entry.scriptPath, previousScript); + } catch (rollbackError) { + rollbackErrors.push(rollbackError); + } + try { + await this.storage.write(entry.flowPath, previousFlowJson); + } catch (rollbackError) { + rollbackErrors.push(rollbackError); + } + this.index.flows[actionName] = previousEntry; + try { + await this.saveIndex(); + } catch (rollbackError) { + rollbackErrors.push(rollbackError); + } + throwPersistenceError(error, rollbackErrors); + } } async addGrammarPatterns( @@ -443,7 +529,7 @@ export class PowerShellStore { " script: string;", " scriptParameters: {", " name: string;", - ' type: "string" | "number" | "boolean" | "path";', + ' type: "string" | "number" | "boolean" | "path" | "executable";', " required: boolean;", " description: string;", " default?: string;", @@ -470,7 +556,7 @@ export class PowerShellStore { " script: string;", " scriptParameters: {", " name: string;", - ' type: "string" | "number" | "boolean" | "path";', + ' type: "string" | "number" | "boolean" | "path" | "executable";', " required: boolean;", " description: string;", " default?: string;", diff --git a/ts/packages/agents/powershell/src/types/scriptRecipe.ts b/ts/packages/agents/powershell/src/types/scriptRecipe.ts index 41d108ba40..f4176168e0 100644 --- a/ts/packages/agents/powershell/src/types/scriptRecipe.ts +++ b/ts/packages/agents/powershell/src/types/scriptRecipe.ts @@ -24,7 +24,7 @@ export interface ScriptRecipe { export interface ScriptParameter { name: string; - type: "string" | "number" | "boolean" | "path"; + type: "string" | "number" | "boolean" | "path" | "executable"; required: boolean; description: string; default?: unknown; diff --git a/ts/packages/agents/powershell/test/actionHandler.spec.ts b/ts/packages/agents/powershell/test/actionHandler.spec.ts index ee159ecb29..77f3de646c 100644 --- a/ts/packages/agents/powershell/test/actionHandler.spec.ts +++ b/ts/packages/agents/powershell/test/actionHandler.spec.ts @@ -7,6 +7,7 @@ import type { Storage, TokenCachePersistence, } from "@typeagent/agent-sdk"; +import { AppAgentEvent } from "@typeagent/agent-sdk"; import { jest } from "@jest/globals"; import { readFileSync } from "node:fs"; import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; @@ -23,6 +24,51 @@ const itOnWindows = process.platform === "win32" ? it : it.skip; class MemoryStorage implements Storage { private readonly files = new Map(); + private readonly writeFailures = new Map< + string, + { remainingWrites: number; error: Error } + >(); + private readonly writeBlocks = new Map< + string, + { + started: () => void; + waitForRelease: Promise; + } + >(); + private readonly afterWriteCallbacks = new Map< + string, + () => void | Promise + >(); + + failWriteAfter(path: string, remainingWrites: number, error: Error): void { + this.writeFailures.set(path, { remainingWrites, error }); + } + + blockNextWrite(path: string): { + started: Promise; + release: () => void; + } { + let markStarted: () => void; + let release: () => void; + const started = new Promise((resolve) => { + markStarted = resolve; + }); + const waitForRelease = new Promise((resolve) => { + release = resolve; + }); + this.writeBlocks.set(path, { + started: () => markStarted!(), + waitForRelease, + }); + return { + started, + release: () => release!(), + }; + } + + afterNextWrite(path: string, callback: () => void | Promise): void { + this.afterWriteCallbacks.set(path, callback); + } async read(path: string): Promise; async read(path: string, options: "utf8" | "base64"): Promise; @@ -38,10 +84,29 @@ class MemoryStorage implements Storage { } async write(path: string, data: string | Uint8Array): Promise { + const block = this.writeBlocks.get(path); + if (block) { + this.writeBlocks.delete(path); + block.started(); + await block.waitForRelease; + } + const failure = this.writeFailures.get(path); + if (failure) { + if (failure.remainingWrites === 0) { + this.writeFailures.delete(path); + throw failure.error; + } + failure.remainingWrites--; + } this.files.set( path, typeof data === "string" ? data : new TextDecoder().decode(data), ); + const afterWrite = this.afterWriteCallbacks.get(path); + if (afterWrite) { + this.afterWriteCallbacks.delete(path); + await afterWrite(); + } } async delete(path: string): Promise { @@ -327,6 +392,75 @@ describe("createAndExecutePowerShellFlow", () => { await rm(directory, { recursive: true, force: true }); } }); + + itOnWindows( + "does not validate URL file content as a path", + async () => { + const directory = await mkdtemp( + join(tmpdir(), "typeagent-powershell-url-content-"), + ); + const outputPath = join(directory, "url.txt"); + try { + const approve = jest.fn(async () => 0); + const { agent, context } = await createAgentHarness( + undefined, + undefined, + undefined, + approve, + ); + + const result = await agent.executeAction?.( + { + schemaName: "powershell.powershell-files", + actionName: "writeFile", + parameters: { + path: outputPath, + content: "https://example.test/api", + }, + }, + context, + ); + + expect(result).not.toHaveProperty("error"); + expect(await readFile(outputPath, "utf8")).toContain( + "https://example.test/api", + ); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }, + ); + + itOnWindows( + "denies a bare executable resolved outside allowed paths", + async () => { + const approve = jest.fn(async () => 0); + const { agent, context } = await createAgentHarness( + undefined, + undefined, + undefined, + approve, + ); + + const result = await agent.executeAction?.( + { + schemaName: "powershell.powershell-processes", + actionName: "startProcess", + parameters: { + path: "powershell.exe", + arguments: "-NoProfile -Command Get-Process", + }, + }, + context, + ); + + expect(result).toMatchObject({ + errorCode: "powershell.policyDenied", + retryable: false, + }); + expect(result?.error).toMatch(/Path access denied/i); + }, + ); }); describe("static network actions", () => { @@ -672,6 +806,7 @@ describe("createAndExecutePowerShellFlow", () => { const result = await executeScript({ script: "param([string]$Path)\nGet-Item -LiteralPath $Path", parameters: { Path: "C:\\PROGRA~1" }, + parameterRoles: { Path: "path" }, sandbox: { allowedCmdlets: ["Get-Item"], allowedPaths: ["C:\\Program Files"], @@ -705,6 +840,7 @@ describe("createAndExecutePowerShellFlow", () => { script: `param([string]$Path) Set-Content -LiteralPath $Path -Value "blocked"`, parameters: { Path: blockedPath }, + parameterRoles: { Path: "path" }, sandbox: { allowedCmdlets: ["Set-Content"], allowedPaths: [allowedDirectory], @@ -722,4 +858,262 @@ Set-Content -LiteralPath $Path -Value "blocked"`, } }, ); + + itOnWindows( + "derives executable validation from dynamic flow parameter types", + async () => { + const { agent, storage, context } = await createAgentHarness(); + + const result = await agent.executeAction?.( + { + schemaName: "powershell", + actionName: "createAndExecutePowerShellFlow", + parameters: { + actionName: "startNamedExecutable", + description: "Start a named executable", + script: `param([string]$Path) +Start-Process -FilePath $Path`, + scriptParameters: [ + { + name: "Path", + type: "executable", + required: true, + description: "Executable to start", + }, + ], + allowedCmdlets: ["Start-Process"], + executionParametersJson: JSON.stringify({ + Path: "powershell.exe", + }), + }, + }, + context, + ); + + expect(result).toMatchObject({ + errorCode: "powershell.policyDenied", + retryable: false, + error: expect.stringContaining("Path access denied"), + }); + expect(await storage.list("pending")).toEqual([]); + expect( + await storage.exists("flows/startNamedExecutable.flow.json"), + ).toBe(false); + }, + ); + + itOnWindows( + "returns partial side effects when repair persistence fails", + async () => { + const { agent, storage, context } = await createAgentHarness(); + await agent.executeAction?.( + { + schemaName: "powershell", + actionName: "createAndExecutePowerShellFlow", + parameters: { + actionName: "updateFailureFlow", + description: "Test repair persistence failure", + script: "Write-Output 'original'", + allowedCmdlets: ["Write-Output"], + executionParametersJson: "{}", + }, + }, + context, + ); + storage.failWriteAfter( + "flows/updateFailureFlow.flow.json", + 0, + new Error("flow definition update failed"), + ); + + const result = await agent.executeAction?.( + { + schemaName: "powershell", + actionName: "repairAndExecutePowerShellFlow", + parameters: { + flowName: "updateFailureFlow", + script: "Write-Output 'repaired'", + allowedCmdlets: ["Write-Output"], + executionParametersJson: "{}", + }, + }, + context, + ); + + expect(result).toMatchObject({ + errorCode: "powershell.partialSideEffects", + retryable: false, + mayHaveSideEffects: true, + error: expect.stringContaining("flow definition update failed"), + }); + await expect( + storage.read("scripts/updateFailureFlow.ps1", "utf8"), + ).resolves.toBe("Write-Output 'original'"); + }, + ); + + itOnWindows( + "returns partial side effects when cancelled after repair execution", + async () => { + const controller = new AbortController(); + const { agent, storage, context } = await createAgentHarness( + undefined, + controller.signal, + ); + await agent.executeAction?.( + { + schemaName: "powershell", + actionName: "createAndExecutePowerShellFlow", + parameters: { + actionName: "postExecutionCancellation", + description: "Test post-execution cancellation", + script: "Write-Output 'original'", + allowedCmdlets: ["Write-Output"], + executionParametersJson: "{}", + }, + }, + context, + ); + storage.afterNextWrite("index.json", () => controller.abort()); + + const result = await agent.executeAction?.( + { + schemaName: "powershell", + actionName: "repairAndExecutePowerShellFlow", + parameters: { + flowName: "postExecutionCancellation", + script: "Write-Output 'repaired'", + allowedCmdlets: ["Write-Output"], + executionParametersJson: "{}", + }, + }, + context, + ); + + expect(result).toMatchObject({ + errorCode: "powershell.partialSideEffects", + retryable: false, + mayHaveSideEffects: true, + error: expect.stringContaining( + "before the repaired flow could be activated", + ), + }); + await expect( + storage.read("scripts/postExecutionCancellation.ps1", "utf8"), + ).resolves.toBe("Write-Output 'original'"); + }, + ); + + itOnWindows("preserves success when usage accounting fails", async () => { + const { agent, storage, sessionContext, context } = + await createAgentHarness(); + await agent.executeAction?.( + { + schemaName: "powershell", + actionName: "createAndExecutePowerShellFlow", + parameters: { + actionName: "usageFailureFlow", + description: "Test usage accounting failure", + script: "Write-Output 'original'", + allowedCmdlets: ["Write-Output"], + executionParametersJson: "{}", + }, + }, + context, + ); + storage.failWriteAfter( + "index.json", + 1, + new Error("usage write failed"), + ); + + const result = await agent.executeAction?.( + { + schemaName: "powershell", + actionName: "repairAndExecutePowerShellFlow", + parameters: { + flowName: "usageFailureFlow", + script: "Write-Output 'repaired'", + allowedCmdlets: ["Write-Output"], + executionParametersJson: "{}", + }, + }, + context, + ); + + expect(result).not.toHaveProperty("error"); + expect(sessionContext.notify).toHaveBeenCalledWith( + AppAgentEvent.Warning, + expect.stringContaining("usage accounting failed"), + ); + }); + + itOnWindows("serializes edits and repairs for the same flow", async () => { + const directory = await mkdtemp( + join(tmpdir(), "typeagent-powershell-edit-lock-"), + ); + const outputPath = join(directory, "repair.txt"); + try { + const { agent, storage, context } = await createAgentHarness(); + await agent.executeAction?.( + { + schemaName: "powershell", + actionName: "createAndExecutePowerShellFlow", + parameters: { + actionName: "serializedFlow", + description: "Test edit and repair serialization", + script: "Write-Output 'original'", + allowedCmdlets: ["Write-Output"], + executionParametersJson: "{}", + }, + }, + context, + ); + + const blockedWrite = storage.blockNextWrite( + "scripts/serializedFlow.ps1", + ); + const edit = agent.executeAction?.( + { + schemaName: "powershell", + actionName: "editPowerShellFlow", + parameters: { + flowName: "serializedFlow", + script: "Write-Output 'edited'", + allowedCmdlets: ["Write-Output"], + }, + }, + context, + ); + await blockedWrite.started; + + const repair = agent.executeAction?.( + { + schemaName: "powershell", + actionName: "repairAndExecutePowerShellFlow", + parameters: { + flowName: "serializedFlow", + script: `param([string]$Path) +Set-Content -LiteralPath $Path -Value "repaired"`, + allowedCmdlets: ["Set-Content"], + executionParametersJson: JSON.stringify({ + Path: outputPath, + }), + }, + }, + context, + ); + await new Promise((resolve) => setTimeout(resolve, 50)); + await expect(readFile(outputPath, "utf8")).rejects.toThrow(); + + blockedWrite.release(); + expect(await edit).not.toHaveProperty("error"); + expect(await repair).not.toHaveProperty("error"); + expect((await readFile(outputPath, "utf8")).trim()).toBe( + "repaired", + ); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); }); diff --git a/ts/packages/agents/powershell/test/powerShellStore.spec.ts b/ts/packages/agents/powershell/test/powerShellStore.spec.ts index dca8dcf673..2281a5c598 100644 --- a/ts/packages/agents/powershell/test/powerShellStore.spec.ts +++ b/ts/packages/agents/powershell/test/powerShellStore.spec.ts @@ -7,6 +7,28 @@ import type { ScriptRecipe } from "../src/types/scriptRecipe.js"; class MockStorage implements Storage { private data = new Map(); + private writeFailure: + | { + path: string; + remainingWrites: number; + error: Error; + beforeFailure?: () => Promise; + } + | undefined; + + failWriteAfter( + path: string, + remainingWrites: number, + error: Error, + beforeFailure?: () => Promise, + ): void { + this.writeFailure = { + path, + remainingWrites, + error, + ...(beforeFailure ? { beforeFailure } : {}), + }; + } async read(storagePath: string): Promise; async read( @@ -25,6 +47,15 @@ class MockStorage implements Storage { } async write(storagePath: string, data: string | Uint8Array): Promise { + if (this.writeFailure?.path === storagePath) { + if (this.writeFailure.remainingWrites === 0) { + const { error, beforeFailure } = this.writeFailure; + this.writeFailure = undefined; + await beforeFailure?.(); + throw error; + } + this.writeFailure.remainingWrites--; + } this.data.set( storagePath, typeof data === "string" ? data : new TextDecoder().decode(data), @@ -107,6 +138,81 @@ describe("PowerShellStore capability lifecycle", () => { expect(store.hasFlow("showPorts")).toBe(true); }); + it("removes partial flow state when save fails", async () => { + const storage = new MockStorage(); + const store = new PowerShellStore(storage); + await store.initialize(); + storage.failWriteAfter( + "scripts/showPorts.ps1", + 0, + new Error("script write failed"), + ); + + await expect( + store.saveFlow(createRecipe(), "reasoning"), + ).rejects.toThrow("script write failed"); + expect(store.hasFlow("showPorts")).toBe(false); + await expect(storage.exists("flows/showPorts.flow.json")).resolves.toBe( + false, + ); + await expect(storage.exists("scripts/showPorts.ps1")).resolves.toBe( + false, + ); + }); + + it("preserves unrelated flows when a concurrent save fails", async () => { + const storage = new MockStorage(); + const store = new PowerShellStore(storage); + await store.initialize(); + storage.failWriteAfter( + "scripts/failingFlow.ps1", + 0, + new Error("script write failed"), + async () => { + await store.saveFlow( + createRecipe("concurrentFlow"), + "reasoning", + ); + }, + ); + + await expect( + store.saveFlow(createRecipe("failingFlow"), "reasoning"), + ).rejects.toThrow("script write failed"); + expect(store.hasFlow("failingFlow")).toBe(false); + expect(store.hasFlow("concurrentFlow")).toBe(true); + await expect(store.getScript("concurrentFlow")).resolves.toBe( + "Get-NetTCPConnection -State Listen", + ); + }); + + it("restores flow state when an update fails", async () => { + const storage = new MockStorage(); + const store = new PowerShellStore(storage); + await store.initialize(); + await store.saveFlow(createRecipe(), "reasoning"); + storage.failWriteAfter( + "flows/showPorts.flow.json", + 0, + new Error("flow definition write failed"), + ); + + await expect( + store.updateFlowScript("showPorts", "Write-Output 'changed'", [ + "Write-Output", + ]), + ).rejects.toThrow("flow definition write failed"); + await expect(store.getScript("showPorts")).resolves.toBe( + "Get-NetTCPConnection -State Listen", + ); + await expect(store.getFlow("showPorts")).resolves.toMatchObject({ + sandbox: { + allowedCmdlets: ["Get-NetTCPConnection"], + allowedModules: ["NetTCPIP"], + }, + }); + }); + it("adds new grammar patterns without duplicating existing ones", async () => { const store = new PowerShellStore(new MockStorage()); await store.initialize(); diff --git a/ts/packages/dispatcher/dispatcher/src/reasoning/claude.ts b/ts/packages/dispatcher/dispatcher/src/reasoning/claude.ts index 845816f535..8332c878d8 100644 --- a/ts/packages/dispatcher/dispatcher/src/reasoning/claude.ts +++ b/ts/packages/dispatcher/dispatcher/src/reasoning/claude.ts @@ -1438,6 +1438,8 @@ function getClaudeOptions( "", "BEST PRACTICES:", "- Always include param() block matching scriptParameters", + "- Use script parameter type 'path' for filesystem paths", + "- Use script parameter type 'executable' for values passed to executable command parameters such as Start-Process -FilePath", "- Output objects or text, avoid Format-Table (hard to parse)", "- Use [PSCustomObject] for structured output", "", diff --git a/ts/packages/dispatcher/dispatcher/src/reasoning/reasoningProfile.ts b/ts/packages/dispatcher/dispatcher/src/reasoning/reasoningProfile.ts index 5b6aa29015..9b30ff3077 100644 --- a/ts/packages/dispatcher/dispatcher/src/reasoning/reasoningProfile.ts +++ b/ts/packages/dispatcher/dispatcher/src/reasoning/reasoningProfile.ts @@ -22,6 +22,7 @@ function getPowerShellFlowRecordingGuidance(): string { "Handle this request as a reusable PowerShell development action.", "Use discover_actions for the powershell schema, then use its typed actions.", "If no matching flow exists, use createAndExecutePowerShellFlow so the script executes once and is promoted only after success.", + "Classify filesystem script parameters as type path and parameters passed as executable commands, such as Start-Process -FilePath, as type executable.", "Do not create a TaskFlow or WebFlow unless the user explicitly asks for one.", "Do not use shell or Bash as a substitute for PowerShell agent actions.", "If the task is not suitable for a PowerShell flow, explain that clearly instead of recording a different workflow type.", @@ -36,6 +37,7 @@ function getPowerShellCapabilityFallbackGuidance(): string { "Prefer an existing executable action or flow. If an existing flow covers the task but misses this phrasing, add validated patterns with addPowerShellFlowPatterns, then execute the existing flow once.", "If an existing flow fails with errorCode powershell.scriptFailure, repair that same flow with repairAndExecutePowerShellFlow at most once. Do not repair policyDenied, cancelled, or partialSideEffects failures.", "If no equivalent exists, use createAndExecutePowerShellFlow. It executes the draft once and promotes it only after success. Do not execute the promoted flow again.", + "Classify filesystem script parameters as type path and parameters passed as executable commands, such as Start-Process -FilePath, as type executable.", "Do not use shell, Bash, TaskFlow, or WebFlow as substitutes.", "You MUST finish by calling reportPowerShellCapabilityOutcome exactly once.", "Report handledExisting after an existing action or flow succeeds.", diff --git a/ts/packages/dispatcher/dispatcher/src/reasoning/scriptRecipeGenerator.ts b/ts/packages/dispatcher/dispatcher/src/reasoning/scriptRecipeGenerator.ts index a26dbe590f..ab2536c835 100644 --- a/ts/packages/dispatcher/dispatcher/src/reasoning/scriptRecipeGenerator.ts +++ b/ts/packages/dispatcher/dispatcher/src/reasoning/scriptRecipeGenerator.ts @@ -44,7 +44,7 @@ export interface ScriptRecipe { export interface ScriptParameter { name: string; - type: "string" | "number" | "boolean" | "path"; + type: "string" | "number" | "boolean" | "path" | "executable"; required: boolean; description: string; default?: unknown; @@ -251,7 +251,8 @@ Generate a script recipe JSON object that: 1. Has a camelCase actionName derived from what the script does 2. Has a human-readable displayName 3. Generalizes hardcoded values (paths, search patterns, filenames, counts) into parameters with sensible defaults -4. Parameters use types: "string", "number", "boolean", or "path" (for filesystem paths) +4. Parameters use types: "string", "number", "boolean", "path" (for filesystem paths), + or "executable" (for values passed to executable command parameters such as Start-Process -FilePath) 5. The script body uses PowerShell param() block with the generalized parameters 6. Includes grammarPatterns array with objects containing: - pattern: AGR grammar pattern using $(paramName:wildcard) for strings/paths or $(paramName:number) for numbers @@ -273,7 +274,7 @@ Return ONLY a JSON object matching this schema (no markdown fences, no explanati "description": "what this script does", "displayName": "Human Readable Name", "parameters": [ - { "name": "paramName", "type": "string|number|boolean|path", "required": true|false, "description": "...", "default": "optional default" } + { "name": "paramName", "type": "string|number|boolean|path|executable", "required": true|false, "description": "...", "default": "optional default" } ], "script": { "language": "powershell",