diff --git a/.gitignore b/.gitignore index 5dd5402..fe6e85c 100644 --- a/.gitignore +++ b/.gitignore @@ -70,14 +70,5 @@ target/ /.idea/ **/graphql.config.yml -.agents/ -/.claude/ -/openspec/ /coverage-reports/ -.cursor/ -/brand/ -/scripts/ -/AGENTS.md -/DESIGN.md -/packages/ -unpackagedMetadata/ +.claude/settings.local.json diff --git a/.husky/pre-push b/.husky/pre-push new file mode 100644 index 0000000..827b884 --- /dev/null +++ b/.husky/pre-push @@ -0,0 +1,7 @@ +# Runs the push check from this checkout. On a branch without it, such as +# public-release, runs the copy committed on main. Clones without it skip the check. +if [ -x scripts/check-public-push.sh ]; then + exec scripts/check-public-push.sh "$@" +elif git cat-file -e main:scripts/check-public-push.sh 2>/dev/null; then + exec bash -c "$(git show main:scripts/check-public-push.sh)" check-public-push "$@" +fi diff --git a/cumulusci.yml b/cumulusci.yml index 57dc2a7..82a69c8 100644 --- a/cumulusci.yml +++ b/cumulusci.yml @@ -196,7 +196,7 @@ flows: options: command: sf package version create --package PlutoConsole --code-coverage --installation-key-bypass --wait 90 # Run with --org qa. Uses the newest package versions, so run build_packages first. - packaged_org: + qa_org: description: Sets up a scratch org like dev_org, but installs Pluto Core, Model Adapters, and Console from their newest unlocked package versions instead of deploying their source. steps: 1: @@ -256,3 +256,6 @@ orgs: dev: config_file: orgs/dev.json days: 30 + qa: + config_file: orgs/dev.json + days: 30 diff --git a/docs-starlight/src/content/docs/approvals/policy.md b/docs-starlight/src/content/docs/approvals/policy.md index 980145c..2811de4 100644 --- a/docs-starlight/src/content/docs/approvals/policy.md +++ b/docs-starlight/src/content/docs/approvals/policy.md @@ -9,26 +9,40 @@ A policy is an Apex class that a developer writes. It implements `IToolAuthoriza ## Turn it on -1. Ask your developer for the policy class and which tools it covers. -2. On the agent, pick the class in **Tool Authorization Policy Class**. -3. On each tool the policy covers, set **Approver Type** and leave **Always Require Approval** unchecked. +1. Ask your developer for the policy class, which tools it covers, and which queues or users it names. +2. Check that each queue the policy names exists and lists Agent Approval under **Supported Objects**. +3. On the agent, pick the class in **Tool Authorization Policy Class**. +4. On each tool the policy covers, set **Approval** to **Decided by Policy**. Leave Approver Type, Approver User, and Approver Queue blank. The policy names the approver. +5. On each of those tools, choose **When Approval Expires**. Optionally set **Approval Timeout (Minutes)** for calls where the policy gives no deadline. -## What a policy can decide +The tool page saves without an error when the setup is valid. It refuses a Decided by Policy tool on an agent with no policy class. It also refuses clearing the policy class while the agent has an active Decided by Policy tool, and names those tools. -Core asks the policy about every tool call, before it runs. +## Who decides -| Decision | What happens | +Core asks the policy only about tools whose Approval is Decided by Policy. Tools set to Not Required or Always Required never reach it. + +| Policy answer | What happens | | --- | --- | -| Allow | The call runs, unless the tool has Always Require Approval checked. | +| Allow | The call runs. | | Deny | The call does not run. The agent gets `TOOL_AUTHORIZATION_DENIED` with the policy's reason and continues. | -| Require Approval | The call waits for approval. The policy gives the reason, and may set a different deadline (1 minute to 7 days) or a different approver, such as a finance queue for large amounts. | +| Require Approval | The call waits for the user or queue the policy names. The policy must name one. The deadline is the policy's, then the tool's Approval Timeout, then 1440 minutes (24 hours). | + +A deadline from the policy replaces the tool's Approval Timeout, so a large refund can get a shorter deadline than a small one. When Approval Expires on the tool applies to every call. + +Core runs the policy again before an approved call runs. A deny still stops the call. Any other answer lets the approved call run, so a call never waits for a second approval. + +A policy can only narrow what the agent may do. It cannot skip a permission check or approve a call. + +Core's configuration check, `AgentConfigurationValidationService.validate`, returns the warning `POLICY_NEVER_RUNS` when the agent has a policy class but no active tool whose Approval is Decided by Policy. + +## See why a call waited -A policy can only narrow what the agent may do. It cannot skip a permission check, and it cannot approve a call. +Open the run's **Run Waiting** event. Its `requiredBy` key is `Tool Setting` for an Always Required tool, or `Policy` for a Decided by Policy tool. ## When a policy fails | Error code | Cause | Fix | | --- | --- | --- | -| `TOOL_AUTHORIZATION_POLICY_INVALID` | The policy asked for approval with no reason, or a deadline out of range. | Ask the developer to fix the class, then rescue the run. | -| `APPROVAL_ASSIGNMENT_UNRESOLVED` | Neither the policy nor the tool named anyone who may approve. | Set Approver Type on the tool, then rescue the run. | +| `TOOL_AUTHORIZATION_POLICY_INVALID` | The policy asked for approval without naming a user or queue, with no reason, or with a deadline out of range. It may also have named a user Id that is not a user. | Ask the developer to fix the class, then rescue the run. | +| `APPROVAL_ASSIGNMENT_UNRESOLVED` | The queue the policy named does not exist or does not support Agent Approval, or the user it named cannot decide approvals. The error names the queue or user. | Create the queue, add Agent Approval to its **Supported Objects**, or give the user `AgentKernelHumanDecision`. Then rescue the run. | | `INITIATING_USER_APPROVAL_UNSUPPORTED` | The policy asked for approval on an agent without an identity. | Give the agent an identity, or change the policy to allow or deny. | diff --git a/docs-starlight/src/content/docs/approvals/require.md b/docs-starlight/src/content/docs/approvals/require.md index a76ba0b..78a33de 100644 --- a/docs-starlight/src/content/docs/approvals/require.md +++ b/docs-starlight/src/content/docs/approvals/require.md @@ -9,19 +9,29 @@ A tool can wait for a person to approve each call before it runs. The run waits, Approvals need an agent with an [Agent Execution Identity](/setup/execution-identity/). An agent without one runs as the starter and cannot wait for someone else. ::: +## Choose Approval + +Each Agent Tool has an **Approval** field. It names who decides each call. + +| Approval | What happens | +| --- | --- | +| Not Required (the default) | Every call runs. The agent's policy is not asked. | +| Always Required | Every call waits for the tool's Approver Type. The agent's policy is not asked. | +| Decided by Policy | The agent's policy decides each call: run it, deny it, or wait for a user or queue the policy names. See [Approve by rule](/approvals/policy/). | + ## Turn it on On the Agent Tool: | Field | What to enter | | --- | --- | -| Always Require Approval | Checked. Every call waits for approval. | +| Approval | Always Required. | | Approver Type | Who decides. See below. | | Approver User or Approver Queue | Depends on Approver Type. | -| Approval Timeout (Hours) | How long the approver has. Blank is 24. | -| When Approval Expires | **Deny and Continue** (the default): the agent is told the call was not approved and carries on. **Pause for Operator**: the run pauses with `WAIT_EXPIRED` for an operator. | +| Approval Timeout (Minutes) | How long the approver has, from 1 to 10080 (7 days). Blank is 1440 (24 hours). | +| When Approval Expires | Required. **Deny and Continue**: the agent is told the call was not approved and carries on. **Pause for Operator**: the run pauses with `WAIT_EXPIRED` for an operator. | -Leave **Always Require Approval** unchecked if only some calls need approval, such as refunds over $200. A policy class then decides call by call, and still sends approvals to the Approver Type. See [Approve by rule](/approvals/policy/). +The tool page saves without an error when the setup is valid. It refuses an approver, Approval Timeout, or When Approval Expires on a tool whose Approval is Not Required. ## Approver types @@ -44,7 +54,7 @@ The person who started the run may approve it when they are the assigned approve ## What the agent tells the approver -For a tool that needs approval, Core asks the model for the change, the facts behind it, and what the approver should check. The approver sees this as the agent's explanation. Core does not check it, so treat it as a claim, not proof. +For a tool whose Approval is Always Required or Decided by Policy, Core asks the model what the call will do, the facts behind it, and each condition it did not confirm with a tool. The approver sees this as the agent's explanation. Core does not check it, so treat it as a claim, not proof. ## What the run looks like diff --git a/docs-starlight/src/content/docs/build/agents.md b/docs-starlight/src/content/docs/build/agents.md index 9034034..f071fe7 100644 --- a/docs-starlight/src/content/docs/build/agents.md +++ b/docs-starlight/src/content/docs/build/agents.md @@ -22,7 +22,7 @@ Core checks the setup when you save it. A mistake shows as a field error on save | Field | Default | Notes | | --- | --- | --- | | Agent Execution Identity | Blank: the run acts as whoever starts it. | Set it for approvals, background work, and long waits. See [Create an execution identity](/setup/execution-identity/). | -| Tool Authorization Policy Class | Blank: no policy. | An Apex class that allows, denies, or sends tool calls for approval by rule. See [Approve by rule](/approvals/policy/). | +| Tool Authorization Policy Class | Blank: no policy. | An Apex class that allows, denies, or sends tool calls for approval by rule. It decides only tools whose Approval is Decided by Policy. See [Approve by rule](/approvals/policy/). | ## Limits diff --git a/docs-starlight/src/content/docs/build/record-tools.md b/docs-starlight/src/content/docs/build/record-tools.md index 9d6e722..6aa2bd1 100644 --- a/docs-starlight/src/content/docs/build/record-tools.md +++ b/docs-starlight/src/content/docs/build/record-tools.md @@ -50,7 +50,7 @@ The tool returns the new record's Id. | `defaultFieldValues` | No | Values every update sets. The agent cannot change them. | | `anchorRecordOnly` | No | `true` updates only the record the run started from, and the agent does not send an Id. Blank or `false` lets the agent pick any record of the object the run's user can edit. | -Set `anchorRecordOnly` to `true` whenever the agent reads customer text, such as emails or case comments. That text could otherwise ask the agent to change a different record. Without it, Core warns when you validate an agent that has an Agent Execution Identity, unless the tool has Always Require Approval checked. +Set `anchorRecordOnly` to `true` whenever the agent reads customer text, such as emails or case comments. That text could otherwise ask the agent to change a different record. Without it, Core warns when you validate an agent that has an Agent Execution Identity, unless the tool's Approval is Always Required. ## Run values diff --git a/docs-starlight/src/content/docs/build/tools.md b/docs-starlight/src/content/docs/build/tools.md index 8a5c6b0..b71ec11 100644 --- a/docs-starlight/src/content/docs/build/tools.md +++ b/docs-starlight/src/content/docs/build/tools.md @@ -29,7 +29,7 @@ Developers can add more tool types in Apex. See [Extend Pluto](/extend/). | Input Schema | The inputs the model may send, as JSON Schema. Leave it blank for a tool type that builds its inputs: every Core tool type, and Search Knowledge, Run Report, and the channel tools. Fill it in for Call API, Read from API, the MCP tools, and a custom type that does not build its inputs. | | Sort Order | Lower numbers are listed to the model first. | | Is Active | Unchecked by default. Check it to give the tool to the agent. | -| Always Require Approval and the approval fields | See [Require approval](/approvals/require/). | +| Approval and the approval fields | See [Require approval](/approvals/require/). | | Allowed Sensitive Data Categories | Leave empty unless the tool needs real values, such as a real email address to send to. See [Sensitive data](/data/sensitive/). | In Pluto Console, the tool editor shows a form for the settings, so most tools need no JSON. diff --git a/docs-starlight/src/content/docs/reference/pause-codes.md b/docs-starlight/src/content/docs/reference/pause-codes.md index 65008e8..e90e299 100644 --- a/docs-starlight/src/content/docs/reference/pause-codes.md +++ b/docs-starlight/src/content/docs/reference/pause-codes.md @@ -16,8 +16,8 @@ A paused run shows its code in **Error Code**. After you fix the cause, rescue t | `UNKNOWN_UNRESOLVED` | Nobody reconciled an Unknown call before the run's payload retention ended. | Reconcile the call, or cancel the run. See [Settle an Unknown call](/operate/unknown/). | | `WAIT_EXPIRED` | An outside wait, or an approval set to **Pause for Operator**, ran out of time. | Rescue to let the agent continue without it, or cancel. | | `TOOL_BINDING_CHANGED` | A tool changed or was deactivated after the call started. | Restore the tool as it was, or cancel the run and start a new one. | -| `TOOL_AUTHORIZATION_POLICY_INVALID` | The agent's policy asked for approval with no reason or a deadline out of range. | Fix the policy class, then rescue. | -| `APPROVAL_ASSIGNMENT_UNRESOLVED` | Nobody eligible could be found to approve the call. | Set Approver Type, Approver User, or Approver Queue on the tool, then rescue. | +| `TOOL_AUTHORIZATION_POLICY_INVALID` | The agent's policy asked for approval without naming a user or queue, with no reason, or with a deadline out of range. A tool whose Approval is Decided by Policy also pauses with this code when its agent has no policy class. | Fix the policy class, then rescue. | +| `APPROVAL_ASSIGNMENT_UNRESOLVED` | Nobody eligible could be found to approve the call. | For an Always Required tool, fix Approver Type, Approver User, or Approver Queue. For a Decided by Policy tool, fix the queue or user the policy names. Then rescue. | | `INITIATING_USER_APPROVAL_UNSUPPORTED` | A call needed approval, but the agent has no execution identity. | Give the agent an identity, or change the policy. Then start a new run. | ## Tool call results the agent sees diff --git a/force-app/contracts/EXTENSIONS.md b/force-app/contracts/EXTENSIONS.md index a2e91b0..3cb1320 100644 --- a/force-app/contracts/EXTENSIONS.md +++ b/force-app/contracts/EXTENSIONS.md @@ -244,8 +244,8 @@ Errors and outcomes: handles failures as it does for `Stays in This Org`: a callout failure or lost worker repeats the call within the run's retry limits, an unexpected exception returns to the model, and a malformed response pauses the run. - The tool is not asked for `_rationale` unless an approval or policy needs - it. A tool that throws `OutcomeUnknownException` is still `Unknown`. + The tool is not asked for `_rationale` unless its Approval is + `Always Required` or `Decided by Policy`. A tool that throws `OutcomeUnknownException` is still `Unknown`. - `Changes Outside Systems`, or a value Core does not recognize: an unexpected exception, a malformed or oversized response, or a lost worker makes the outcome `Unknown`. Core repeats the call only when the tool asks @@ -308,27 +308,61 @@ configuration blank. ## `IToolAuthorizationPolicy` Policies follow the same query-only rule and governor reserve as context -providers. Core evaluates the policy for every call; return `Allow`, `Deny`, -or `RequireApproval`. - -- `Deny` always stops the call. The model gets `TOOL_AUTHORIZATION_DENIED` - with your `reason` and continues, so write the reason for the model. -- If the tool's `Always Require Approval` is checked, the call waits for approval - even on `Allow`. Otherwise `Allow` runs it and `RequireApproval` waits. -- `RequireApproval` needs a nonblank `reason`. Optional `timeoutSeconds` is 60 - through 604,800; blank uses the tool's Approval Timeout. Optional - `approverId` names a user or queue that decides this call instead of the - tool's approver, for example to escalate by amount. +providers. Core evaluates the policy only for calls of tools whose Approval is +`Decided by Policy`, after it checks the call's arguments against the tool's +Input Schema. Calls of `Not Required` and `Always Required` tools never reach it. Build the answer from the request: + +```apex +public AgentKernelModels.ToolAuthorizationResponse evaluate(AgentKernelModels.ToolAuthorizationRequest request) { + if (request.toolType != 'NorthwindIssueRefund') { + return request.allow(); + } + Decimal amount = request.decimalInput('amount'); + if (amount == null) { + return request.deny('Send the refund amount.'); + } + if (amount <= 25) { + return request.allow(); + } + return request.requireApproval('The refund is over 25.').byQueue('Finance_Approvers'); +} +``` + +- Match on `request.toolType`, the API name of the tool's Agent Tool Type. + `toolName` is the tool's API name on the agent, and an admin can change it. + A policy that matches on `toolName` stops covering a tool when it is renamed. +- `request.inputValue(name)` returns one top-level input. `request.decimalInput(name)` + returns a number input, or null when it is missing or not a number. +- `request.deny(reason)` always stops the call. The model gets + `TOOL_AUTHORIZATION_DENIED` with your reason and continues, so write the + reason for the model. +- `allow()` runs the call, and `requireApproval(reason)` makes it wait. +- `requireApproval` needs a nonblank reason. The approver sees it. Describe + the call, such as "The refund is over 200", not who should approve it. +- `requireApproval` returns `AgentKernelModels.ToolApprovalResponse`. + Name who approves this call with `.byQueue(apiName)` or `.byUser(userId)`. + Each replaces the other. The tool has no approver of its own, so a hold that + names neither pauses the run with `TOOL_AUTHORIZATION_POLICY_INVALID`. +- `.withTimeoutMinutes(minutes)` sets this call's deadline in whole minutes, + 1 through 10,080 (7 days). It replaces the tool's Approval Timeout, so a + large refund can get a shorter deadline than a small one. Without it, the + tool's Approval Timeout applies, or 24 hours when that is blank. The tool's + When Approval Expires always applies. +- Core runs the policy again before an approved call runs. A deny still stops + it. Any other answer, including a hold that names someone else, lets the + approved call run, so a call is never approved twice. - Core pauses the run with `TOOL_AUTHORIZATION_POLICY_INVALID` for a blank - reason or out-of-range timeout, and with `APPROVAL_ASSIGNMENT_UNRESOLVED` - when neither policy nor tool names an eligible approver. -- Runs of agents without an identity cannot wait: `RequireApproval` pauses - them with `INITIATING_USER_APPROVAL_UNSUPPORTED`. Return `Allow` or `Deny` + reason, a timeout outside 1 through 10,080 minutes, or a `byUser` Id that is + not a user. +- Core pauses the run with `APPROVAL_ASSIGNMENT_UNRESOLVED` when the named + queue does not exist or does not support Agent Approval records, or the named + user cannot decide approvals. A missing queue never falls back to another + approver. +- The `Run Waiting` event records `requiredBy`: `Tool Setting` for an + `Always Required` tool, or `Policy`. +- Runs of agents without an identity cannot wait: `requireApproval` pauses + them with `INITIATING_USER_APPROVAL_UNSUPPORTED`. Return `allow()` or `deny()` when `request.executionIdentityId` is null. -- Upgrading from `executionMode`: `executionMode == 'Delegated Principal'` - becomes `executionIdentityId != null`, and `'Initiating User'` becomes - `executionIdentityId == null`. `ExecutionPrincipal`'s constructor takes the - identity Id in place of the mode. Core creates the Agent Approval and decides who may approve it; see [Require approval](https://iamsonal.github.io/aiAgentStudio/approvals/require/). diff --git a/force-app/main/default/classes/AgentApprovalService.cls b/force-app/main/default/classes/AgentApprovalService.cls index b5d200e..cef84a9 100644 --- a/force-app/main/default/classes/AgentApprovalService.cls +++ b/force-app/main/default/classes/AgentApprovalService.cls @@ -20,8 +20,9 @@ @SuppressWarnings('PMD.ApexCRUDViolation') public without sharing class AgentApprovalService { public static final String RELAY_PERMISSION = 'AgentKernelApprovalRelay'; - public static final Integer DEFAULT_TIMEOUT_HOURS = 24; - public static final Integer MAX_TIMEOUT_HOURS = 168; + /** Approval timeouts, from the tool or a policy, are whole minutes: 1 through 7 days, blank 24 hours. */ + public static final Integer DEFAULT_TIMEOUT_MINUTES = 1440; + public static final Integer MAX_TIMEOUT_MINUTES = 10080; private static final Integer MAX_MEMBERSHIP_DEPTH = 5; private static final Integer MAX_ROLE_DEPTH = 10; @@ -37,6 +38,16 @@ public without sharing class AgentApprovalService { AgentKernelModels.APPROVER_DELIVERY_CHANNEL }; + public static final String TOOL_APPROVAL_REASON = 'Tool configuration requires approval.'; + public static final String REQUIRED_BY_TOOL = 'Tool Setting'; + public static final String REQUIRED_BY_POLICY = 'Policy'; + + private static final Set APPROVAL_MODES = new Set{ + AgentKernelModels.APPROVAL_NOT_REQUIRED, + AgentKernelModels.APPROVAL_ALWAYS_REQUIRED, + AgentKernelModels.APPROVAL_DECIDED_BY_POLICY + }; + /** The resolved assignment and deadline for one approval, fixed when the wait begins. */ public class Request { public String reason; @@ -46,6 +57,8 @@ public without sharing class AgentApprovalService { public String expiryAction; /** The tool whose call waits; recorded on the approval so routing and reports need no tool names. */ public Id toolId; + /** Tool Setting or Policy: what asked for this approval. */ + public String requiredBy; } /** A wait that cannot be assigned to an eligible approver; the run pauses instead of waiting unseen. */ @@ -58,23 +71,57 @@ public without sharing class AgentApprovalService { public Id relayedById; } + /** The tool's Approval, with blank read as Not Required. */ + public static String mode(AgentTool__c tool) { + return tool == null || String.isBlank(tool.ApprovalMode__c) ? AgentKernelModels.APPROVAL_NOT_REQUIRED : tool.ApprovalMode__c; + } + /** - * Resolves the approver from the policy override or the tool's typed - * settings. Runs as the execution user inside the tool worker. + * Resolves the approver for a waiting call. An Always Required tool waits + * for its own approver. A Decided by Policy tool waits for the queue or + * user the policy named, which must exist and never falls back. Either way + * the tool's When Approval Expires applies, and its Approval Timeout unless + * the policy gave one. held is the policy's Require Approval answer, or + * null for an Always Required tool. Runs as the execution user inside the + * tool worker. */ - public static Request request(AgentRun__c run, AgentTool__c tool, String reason, Integer policyTimeoutSeconds, Id policyApproverId) { + public static Request request(AgentRun__c run, AgentTool__c tool, AgentKernelModels.ToolApprovalResponse held) { + Boolean decidedByPolicy = mode(tool) == AgentKernelModels.APPROVAL_DECIDED_BY_POLICY; + if (decidedByPolicy && held == null) { + throw new AssignmentException('Tool ' + tool.DeveloperName__c + ' is Decided by Policy, but the policy named no approver.'); + } + if (!decidedByPolicy && String.isBlank(tool.ApproverType__c)) { + throw new AssignmentException( + 'Tool ' + tool.DeveloperName__c + ' requires approval but has no Approver Type. Choose an Approver Type on the tool, then rescue the run.' + ); + } Request result = new Request(); result.toolId = tool.Id; - result.reason = String.isBlank(reason) ? 'Tool configuration requires approval.' : reason; + result.requiredBy = decidedByPolicy ? REQUIRED_BY_POLICY : REQUIRED_BY_TOOL; + result.reason = !decidedByPolicy || String.isBlank(held.reason) ? TOOL_APPROVAL_REASON : held.reason.trim(); result.expiryAction = String.isBlank(tool.ApprovalExpiryAction__c) ? AgentKernelModels.APPROVAL_EXPIRY_DENY : tool.ApprovalExpiryAction__c; - Integer timeoutSeconds = policyTimeoutSeconds != null - ? policyTimeoutSeconds - : (tool.ApprovalTimeoutHours__c == null ? DEFAULT_TIMEOUT_HOURS : tool.ApprovalTimeoutHours__c.intValue()) * 3600; - result.expiresAt = System.now().addSeconds(timeoutSeconds); - - if (policyApproverId != null) { - result.approverType = policyApproverId.getSObjectType() == User.SObjectType ? AgentKernelModels.APPROVER_USER : AgentKernelModels.APPROVER_QUEUE; - result.ownerId = policyApproverId; + Integer timeoutMinutes = decidedByPolicy && held.timeoutMinutes != null + ? held.timeoutMinutes + : (tool.ApprovalTimeoutMinutes__c == null ? DEFAULT_TIMEOUT_MINUTES : tool.ApprovalTimeoutMinutes__c.intValue()); + result.expiresAt = System.now().addMinutes(timeoutMinutes); + + if (decidedByPolicy) { + String policyQueue = String.isBlank(held.approverQueue) ? null : held.approverQueue.trim(); + if (policyQueue != null) { + Map named = approvalQueues(new Set{ policyQueue }); + if (named.isEmpty()) { + throw new AssignmentException( + 'The authorization policy named queue ' + + policyQueue + + ', but no queue with that API name supports Agent Approval records. Create the queue or add Agent Approval to its supported objects, then rescue the run.' + ); + } + result.approverType = AgentKernelModels.APPROVER_QUEUE; + result.ownerId = named.values()[0]; + } else { + result.approverType = AgentKernelModels.APPROVER_USER; + result.ownerId = held.approverUserId; + } } else { result.approverType = tool.ApproverType__c; Boolean hasFallbackQueue = result.approverType == AgentKernelModels.APPROVER_ANCHOR_OWNER && !String.isBlank(tool.ApproverQueue__c); @@ -500,16 +547,22 @@ public without sharing class AgentApprovalService { return result; } + public static final String POLICY_REQUIRED_MESSAGE = 'This tool\'s Approval is Decided by Policy, but its agent has no Tool Authorization Policy Class. Set the agent\'s policy, or choose Not Required or Always Required.'; + /** Save-time rules for a tool's approval settings. */ public static void validateTools(List records) { Set approverUserIds = new Set(); Set queueNames = new Set(); + Set policyDefinitionIds = new Set(); for (AgentTool__c record : records) { String error = settingsError(record); if (error != null) { record.addError(error); continue; } + if (record.IsActive__c == true && mode(record) == AgentKernelModels.APPROVAL_DECIDED_BY_POLICY && record.AgentDefinition__c != null) { + policyDefinitionIds.add(record.AgentDefinition__c); + } if (record.ApproverType__c == AgentKernelModels.APPROVER_USER) { approverUserIds.add(record.ApproverUser__c); } else if (record.ApproverType__c == AgentKernelModels.APPROVER_QUEUE) { @@ -518,14 +571,22 @@ public without sharing class AgentApprovalService { queueNames.add(record.ApproverQueue__c.trim()); } } + Set withoutPolicyIds = policyDefinitionIds.isEmpty() + ? new Set() + : new Map( + [SELECT Id FROM AgentDefinition__c WHERE Id IN :policyDefinitionIds AND ToolAuthorizationPolicyClass__c = NULL WITH SYSTEM_MODE] + ) + .keySet(); Map queuesByName = approvalQueues(queueNames); - Set eligibleUserIds = new Set(); - for (Id userId : approverUserIds) { - if (AgentHumanDecisionGuard.isAuthorizedUser(userId)) { - eligibleUserIds.add(userId); - } - } + Set eligibleUserIds = AgentHumanDecisionGuard.usersWithDecisionAuthority(approverUserIds); for (AgentTool__c record : records) { + if ( + record.IsActive__c == true && + mode(record) == AgentKernelModels.APPROVAL_DECIDED_BY_POLICY && + withoutPolicyIds.contains(record.AgentDefinition__c) + ) { + record.ApprovalMode__c.addError(POLICY_REQUIRED_MESSAGE); + } if ( record.ApproverType__c == AgentKernelModels.APPROVER_USER && record.ApproverUser__c != null && @@ -549,12 +610,89 @@ public without sharing class AgentApprovalService { return FeatureManagement.checkPermission(RELAY_PERMISSION); } + /** + * Refuses clearing an agent's Tool Authorization Policy Class while an + * active tool of the agent is Decided by Policy, because nothing would + * decide that tool's calls. + */ + public static void validateDefinitions(List records, Map previousById) { + if (previousById == null) { + return; + } + Set clearedIds = new Set(); + for (AgentDefinition__c record : records) { + AgentDefinition__c previous = previousById.get(record.Id); + if (previous != null && !String.isBlank(previous.ToolAuthorizationPolicyClass__c) && String.isBlank(record.ToolAuthorizationPolicyClass__c)) { + clearedIds.add(record.Id); + } + } + if (clearedIds.isEmpty()) { + return; + } + Map> toolNamesByDefinition = new Map>(); + for (AgentTool__c tool : [ + SELECT AgentDefinition__c, DeveloperName__c + FROM AgentTool__c + WHERE AgentDefinition__c IN :clearedIds AND IsActive__c = TRUE AND ApprovalMode__c = :AgentKernelModels.APPROVAL_DECIDED_BY_POLICY + WITH SYSTEM_MODE + ORDER BY DeveloperName__c + ]) { + if (!toolNamesByDefinition.containsKey(tool.AgentDefinition__c)) { + toolNamesByDefinition.put(tool.AgentDefinition__c, new List()); + } + toolNamesByDefinition.get(tool.AgentDefinition__c).add(tool.DeveloperName__c); + } + for (AgentDefinition__c record : records) { + List toolNames = toolNamesByDefinition.get(record.Id); + if (toolNames != null) { + record.ToolAuthorizationPolicyClass__c.addError( + 'These tools are Decided by Policy, so the agent needs its policy: ' + + String.join(toolNames, ', ') + + '. Change their Approval first, or keep the policy.' + ); + } + } + } + private static String settingsError(AgentTool__c record) { + String approvalMode = mode(record); String approverType = record.ApproverType__c; - if (record.RequiresApproval__c == true && String.isBlank(approverType)) { - return 'Choose an Approver Type for a tool that requires approval.'; + if (!APPROVAL_MODES.contains(approvalMode)) { + return 'Approval is invalid.'; } - if (!String.isBlank(approverType) && !APPROVER_TYPES.contains(approverType)) { + if (approvalMode == AgentKernelModels.APPROVAL_NOT_REQUIRED) { + if ( + !String.isBlank(approverType) || + record.ApproverUser__c != null || + !String.isBlank(record.ApproverQueue__c) || + record.ApprovalTimeoutMinutes__c != null || + !String.isBlank(record.ApprovalExpiryAction__c) + ) { + return 'Approval is Not Required, so clear the approver, Approval Timeout, and When Approval Expires.'; + } + return null; + } + if (String.isBlank(record.ApprovalExpiryAction__c)) { + return 'Choose When Approval Expires for a tool whose calls can wait for approval.'; + } + if ( + record.ApprovalTimeoutMinutes__c != null && + (record.ApprovalTimeoutMinutes__c < 1 || + record.ApprovalTimeoutMinutes__c > MAX_TIMEOUT_MINUTES || + record.ApprovalTimeoutMinutes__c != record.ApprovalTimeoutMinutes__c.intValue()) + ) { + return 'Approval Timeout must be a whole number of minutes from 1 to ' + MAX_TIMEOUT_MINUTES + ' (7 days).'; + } + if (approvalMode == AgentKernelModels.APPROVAL_DECIDED_BY_POLICY) { + if (!String.isBlank(approverType) || record.ApproverUser__c != null || !String.isBlank(record.ApproverQueue__c)) { + return 'Approval is Decided by Policy, so the policy names the approver. Clear Approver Type, Approver User, and Approver Queue.'; + } + return null; + } + if (String.isBlank(approverType)) { + return 'Choose an Approver Type for a tool whose Approval is Always Required.'; + } + if (!APPROVER_TYPES.contains(approverType)) { return 'Approver Type is invalid.'; } if (approverType == AgentKernelModels.APPROVER_USER && record.ApproverUser__c == null) { @@ -573,25 +711,11 @@ public without sharing class AgentApprovalService { ) { return 'Approver Queue applies only when Approver Type is Queue, or Anchor Record Owner as its fallback queue.'; } - if ( - record.ApprovalTimeoutHours__c != null && - (record.ApprovalTimeoutHours__c < 1 || - record.ApprovalTimeoutHours__c > MAX_TIMEOUT_HOURS || - record.ApprovalTimeoutHours__c != record.ApprovalTimeoutHours__c.intValue()) - ) { - return 'Approval Timeout must be a whole number of hours from 1 to ' + MAX_TIMEOUT_HOURS + '.'; - } - if (String.isBlank(approverType) && record.ApprovalTimeoutHours__c != null) { - return 'Approval settings apply only when an Approver Type is chosen.'; - } return null; } private static Id configuredOwner(AgentRun__c run, AgentTool__c tool) { String approverType = tool.ApproverType__c; - if (String.isBlank(approverType)) { - throw new AssignmentException('Tool ' + tool.DeveloperName__c + ' has no Approver Type, and the authorization policy named no approver.'); - } if (approverType == AgentKernelModels.APPROVER_USER) { return tool.ApproverUser__c; } diff --git a/force-app/main/default/classes/AgentConfigService.cls b/force-app/main/default/classes/AgentConfigService.cls index a2f4471..3ef433c 100644 --- a/force-app/main/default/classes/AgentConfigService.cls +++ b/force-app/main/default/classes/AgentConfigService.cls @@ -196,11 +196,11 @@ public inherited sharing class AgentConfigService { ToolType__c, ImplementationConfiguration__c, InputSchema__c, - RequiresApproval__c, + ApprovalMode__c, ApproverType__c, ApproverUser__c, ApproverQueue__c, - ApprovalTimeoutHours__c, + ApprovalTimeoutMinutes__c, ApprovalExpiryAction__c, AllowedSensitiveDataCategories__c, IsActive__c, @@ -410,11 +410,11 @@ public inherited sharing class AgentConfigService { tool.Description__c, AgentToolTypeService.bindingHash(tool), tool.InputSchema__c, - tool.RequiresApproval__c, + tool.ApprovalMode__c, tool.ApproverType__c, tool.ApproverUser__c, tool.ApproverQueue__c, - tool.ApprovalTimeoutHours__c, + tool.ApprovalTimeoutMinutes__c, tool.ApprovalExpiryAction__c, tool.AllowedSensitiveDataCategories__c } diff --git a/force-app/main/default/classes/AgentConfigurationValidationService.cls b/force-app/main/default/classes/AgentConfigurationValidationService.cls index 6b41b5c..fbc0f35 100644 --- a/force-app/main/default/classes/AgentConfigurationValidationService.cls +++ b/force-app/main/default/classes/AgentConfigurationValidationService.cls @@ -105,11 +105,20 @@ global inherited sharing class AgentConfigurationValidationService { 'UNSCOPED_RECORD_UPDATE', 'AgentTool[' + tool.DeveloperName__c + '].ImplementationConfiguration', 'This Update Record tool may change any record the execution user can edit, chosen by the model. Text in records or emails can steer ' + - 'that choice. Check Anchor Record Only, or check Always Require Approval.' + 'that choice. Check Anchor Record Only, or set Approval to Always Required.' ); } } } + if (!String.isBlank(definition.ToolAuthorizationPolicyClass__c) && !hasPolicyDecidedTool(tools)) { + warning( + report, + 'POLICY_NEVER_RUNS', + 'AgentDefinition.ToolAuthorizationPolicyClass', + 'The Tool Authorization Policy Class runs only for tools whose Approval is Decided by Policy, and this agent has none. ' + + 'Set Approval to Decided by Policy on the tools the policy should decide, or clear the policy.' + ); + } warning( report, 'NAMED_CREDENTIAL_ACCESS_NOT_PREFLIGHTED', @@ -136,11 +145,20 @@ global inherited sharing class AgentConfigurationValidationService { return report; } + private static Boolean hasPolicyDecidedTool(List tools) { + for (AgentTool__c tool : tools) { + if (AgentApprovalService.mode(tool) == AgentKernelModels.APPROVAL_DECIDED_BY_POLICY) { + return true; + } + } + return false; + } + /** An Update Record tool without approval whose record the model picks freely. */ private static Boolean isUnscopedUpdate(AgentTool__c tool) { AgentToolType__mdt toolType = AgentToolTypeService.toolType(tool); if ( - tool.RequiresApproval__c == true || + AgentApprovalService.mode(tool) == AgentKernelModels.APPROVAL_ALWAYS_REQUIRED || toolType == null || toolType.ToolClass__c == null || toolType.ToolClass__c.trim() != 'StandardUpdateRecordTool' @@ -220,7 +238,7 @@ global inherited sharing class AgentConfigurationValidationService { ToolType__c, ImplementationConfiguration__c, InputSchema__c, - RequiresApproval__c, + ApprovalMode__c, AllowedSensitiveDataCategories__c FROM AgentTool__c WHERE AgentDefinition__c = :definitionId AND IsActive__c = TRUE diff --git a/force-app/main/default/classes/AgentDefinitionIdentityRules.cls b/force-app/main/default/classes/AgentDefinitionIdentityRules.cls index 407527b..4078ce8 100644 --- a/force-app/main/default/classes/AgentDefinitionIdentityRules.cls +++ b/force-app/main/default/classes/AgentDefinitionIdentityRules.cls @@ -21,8 +21,8 @@ * no one else can approve it and scheduled recovery cannot continue it. */ public inherited sharing class AgentDefinitionIdentityRules { - public static final String APPROVAL_UNSUPPORTED_MESSAGE = 'This agent has no Agent Execution Identity, so it runs as whoever starts it and cannot have tools that need approval: an approved call could not continue in that user\'s job chain. Set an Agent Execution Identity on the agent, or clear Always Require Approval and Approver Type.'; - public static final String CLEAR_IDENTITY_REFUSED_MESSAGE = 'This agent has tools that need approval, so it needs an Agent Execution Identity. Keep the identity, or clear Always Require Approval and Approver Type on those tools first.'; + public static final String APPROVAL_UNSUPPORTED_MESSAGE = 'This agent has no Agent Execution Identity, so it runs as whoever starts it and cannot have tools that need approval: an approved call could not continue in that user\'s job chain. Set an Agent Execution Identity on the agent, or change the tool\'s Approval from Always Required.'; + public static final String CLEAR_IDENTITY_REFUSED_MESSAGE = 'This agent has tools that need approval, so it needs an Agent Execution Identity. Keep the identity, or change the Approval of those tools from Always Required first.'; public static String error(AgentDefinition__c definition) { return chainDepthError(definition); @@ -102,7 +102,7 @@ public inherited sharing class AgentDefinitionIdentityRules { } private static Boolean usesApproval(AgentTool__c record) { - return record.RequiresApproval__c == true || !String.isBlank(record.ApproverType__c); + return AgentApprovalService.mode(record) == AgentKernelModels.APPROVAL_ALWAYS_REQUIRED; } private static Set definitionsWithApprovalTools(Set definitionIds) { @@ -113,7 +113,7 @@ public inherited sharing class AgentDefinitionIdentityRules { for (AgentTool__c tool : [ SELECT AgentDefinition__c FROM AgentTool__c - WHERE AgentDefinition__c IN :definitionIds AND IsActive__c = TRUE AND (RequiresApproval__c = TRUE OR ApproverType__c != NULL) + WHERE AgentDefinition__c IN :definitionIds AND IsActive__c = TRUE AND ApprovalMode__c = :AgentKernelModels.APPROVAL_ALWAYS_REQUIRED WITH SYSTEM_MODE ]) { result.add(tool.AgentDefinition__c); diff --git a/force-app/main/default/classes/AgentKernel.cls b/force-app/main/default/classes/AgentKernel.cls index ee06802..bae75ae 100644 --- a/force-app/main/default/classes/AgentKernel.cls +++ b/force-app/main/default/classes/AgentKernel.cls @@ -1141,7 +1141,7 @@ public without sharing class AgentKernel { AgentKernelModels.EVENT_RUN_WAITING, command.OperationKey__c, 'Tool approval required.', - JSON.serialize(new Map{ 'approvalId' => approval.Id, 'approverType' => request.approverType }) + JSON.serialize(new Map{ 'approvalId' => approval.Id, 'approverType' => request.approverType, 'requiredBy' => request.requiredBy }) ); AgentLifecyclePublisher.publish(run, 'Waiting', eventSequence, command.OperationKey__c); } diff --git a/force-app/main/default/classes/AgentKernelModels.cls b/force-app/main/default/classes/AgentKernelModels.cls index 99f4cc7..45bbd4f 100644 --- a/force-app/main/default/classes/AgentKernelModels.cls +++ b/force-app/main/default/classes/AgentKernelModels.cls @@ -103,6 +103,11 @@ global inherited sharing class AgentKernelModels { public static final String APPROVER_REQUESTER_MANAGER = 'Requester\'s Manager'; public static final String APPROVER_DELIVERY_CHANNEL = 'Delivery Channel'; + /** AgentTool__c.ApprovalMode__c. Blank means Not Required. */ + public static final String APPROVAL_NOT_REQUIRED = 'Not Required'; + public static final String APPROVAL_ALWAYS_REQUIRED = 'Always Required'; + public static final String APPROVAL_DECIDED_BY_POLICY = 'Decided by Policy'; + public static final String APPROVAL_EXPIRY_DENY = 'Deny and Continue'; public static final String APPROVAL_EXPIRY_PAUSE = 'Pause for Operator'; @@ -476,11 +481,20 @@ global inherited sharing class AgentKernelModels { } } + /** + * One call of a Decided by Policy tool for a policy to decide. Build the + * answer with allow(), deny(reason), or requireApproval(reason) and an + * approver, and read inputs with inputValue(name) or decimalInput(name). + */ global class ToolAuthorizationRequest { global Id runId { get; set; } global Id commandId { get; set; } global Id definitionId { get; set; } + /** The tool's API name on this agent. An admin can change it; match on toolType to find a kind of tool. */ global String toolName { get; set; } + /** The API name of the tool's Agent Tool Type, such as UpdateRecord. */ + global String toolType { get; set; } + /** The call's arguments as JSON, already checked against the tool's Input Schema. */ global String input { get; set; } global String operationKey { get; set; } global Integer attemptNumber { get; set; } @@ -492,14 +506,86 @@ global inherited sharing class AgentKernelModels { global Id resolvedPrincipalId { get; set; } /** The run's Agent Execution Identity; null when the run acts as the user who started it. */ global Id executionIdentityId { get; set; } + private transient Map arguments; + + /** One top-level input, or null when the call did not send it. */ + global Object inputValue(String name) { + if (arguments == null) { + Object parsed; + try { + parsed = String.isBlank(input) ? null : JSON.deserializeUntyped(input); + } catch (JSONException malformed) { + parsed = null; + } + arguments = parsed instanceof Map ? (Map) parsed : new Map(); + } + return arguments.get(name); + } + + /** One top-level number input, or null when it is missing or not a number. */ + global Decimal decimalInput(String name) { + Object value = inputValue(name); + if (value instanceof Decimal) { + return (Decimal) value; + } + return value instanceof Integer || value instanceof Long || value instanceof Double ? Decimal.valueOf(String.valueOf(value)) : null; + } + + global ToolAuthorizationResponse allow() { + return new ToolAuthorizationResponse(ToolAuthorizationDecision.Allow, null); + } + + /** The model receives TOOL_AUTHORIZATION_DENIED with this reason, so write it for the model. */ + global ToolAuthorizationResponse deny(String reason) { + return new ToolAuthorizationResponse(ToolAuthorizationDecision.Deny, reason); + } + + /** The approver sees this reason. Name the approver with byQueue or byUser; Core pauses the run if neither is named. */ + global ToolApprovalResponse requireApproval(String reason) { + return new ToolApprovalResponse(reason); + } } - global class ToolAuthorizationResponse { - global ToolAuthorizationDecision decision { get; set; } - global String reason { get; set; } - global Integer timeoutSeconds { get; set; } - /** Optional User or Queue that decides a RequireApproval call instead of the tool's configured approver. */ - global Id approverId { get; set; } + global virtual class ToolAuthorizationResponse { + global ToolAuthorizationDecision decision { get; private set; } + global String reason { get; private set; } + + public ToolAuthorizationResponse(ToolAuthorizationDecision decision, String reason) { + this.decision = decision; + this.reason = reason; + } + } + + /** A Require Approval answer: who approves the call, and optionally a deadline that replaces the tool's Approval Timeout. */ + global class ToolApprovalResponse extends ToolAuthorizationResponse { + /** Whole minutes, 1 through 10,080 (7 days). Blank uses the tool's Approval Timeout. */ + global Integer timeoutMinutes { get; private set; } + /** The API name of a queue that supports Agent Approval. Core pauses the run if it is missing. */ + global String approverQueue { get; private set; } + global Id approverUserId { get; private set; } + + public ToolApprovalResponse(String reason) { + super(ToolAuthorizationDecision.RequireApproval, reason); + } + + /** Replaces any user named with byUser. */ + global ToolApprovalResponse byQueue(String queueApiName) { + approverQueue = queueApiName; + approverUserId = null; + return this; + } + + /** Replaces any queue named with byQueue. */ + global ToolApprovalResponse byUser(Id userId) { + approverUserId = userId; + approverQueue = null; + return this; + } + + global ToolApprovalResponse withTimeoutMinutes(Integer minutes) { + timeoutMinutes = minutes; + return this; + } } /** Persistence-neutral readable projection of one agent run. */ @@ -753,6 +839,7 @@ global inherited sharing class AgentKernelModels { global String developerName { get; set; } global String description { get; set; } global String inputSchema { get; set; } + /** True when every call waits for approval: the tool's Approval is Always Required. */ global Boolean requiresApproval { get; set; } global ToolDefinition() { diff --git a/force-app/main/default/classes/IToolAuthorizationPolicy.cls b/force-app/main/default/classes/IToolAuthorizationPolicy.cls index 163015a..f139297 100644 --- a/force-app/main/default/classes/IToolAuthorizationPolicy.cls +++ b/force-app/main/default/classes/IToolAuthorizationPolicy.cls @@ -12,9 +12,9 @@ */ /** - * Administrator-selected, query-only authorization seam evaluated before a - * tool invocation. It may allow, deny, or require approval; it cannot relax - * a tool's static approval requirement. + * Administrator-selected, query-only decision for each call of a tool whose + * Approval is Decided by Policy. Core never asks it about other tools. It may + * allow the call, deny it, or require approval from a queue or user it names. */ global interface IToolAuthorizationPolicy { AgentKernelModels.ToolAuthorizationResponse evaluate(AgentKernelModels.ToolAuthorizationRequest request); diff --git a/force-app/main/default/classes/ModelRequestAssembler.cls b/force-app/main/default/classes/ModelRequestAssembler.cls index 621fc72..27a3ef6 100644 --- a/force-app/main/default/classes/ModelRequestAssembler.cls +++ b/force-app/main/default/classes/ModelRequestAssembler.cls @@ -60,7 +60,7 @@ public without sharing class ModelRequestAssembler { Result result = new Result(); List toolDefinitions; try { - toolDefinitions = neutralTools(tools, definition); + toolDefinitions = neutralTools(tools); } catch (ToolSchemaUnavailableException unavailable) { return fail(result, 'TOOL_SCHEMA_UNAVAILABLE', unavailable.getMessage()); } @@ -481,10 +481,8 @@ public without sharing class ModelRequestAssembler { } } - private static List neutralTools(List records, AgentDefinition__c definition) { + private static List neutralTools(List records) { List result = new List(); - // A policy may ask for approval of any call, and the schema is sent before Core knows which. - Boolean policyMayAskApproval = definition != null && !String.isBlank(definition.ToolAuthorizationPolicyClass__c); for (AgentTool__c record : records) { AgentKernelModels.ToolDefinition item = new AgentKernelModels.ToolDefinition(); item.developerName = record.DeveloperName__c; @@ -495,41 +493,48 @@ public without sharing class ModelRequestAssembler { 'Tool ' + record.DeveloperName__c + ' cannot build its inputs: ' + schema.problem + ' Fix the tool, then retry the run.' ); } - item.inputSchema = asksRationale(record, policyMayAskApproval) ? withRationale(schema.schema, record.RequiresApproval__c == true) : schema.schema; - item.requiresApproval = record.RequiresApproval__c; + String rationale = rationalePrompt(record); + item.inputSchema = rationale == null ? schema.schema : withRationale(schema.schema, rationale); + item.requiresApproval = AgentApprovalService.mode(record) == AgentKernelModels.APPROVAL_ALWAYS_REQUIRED; result.add(item); } return result; } /** - * The model explains the calls a person may review: tools that require - * approval, tools whose effects leave this org (an operator may reconcile - * them), and every tool of an agent whose policy may ask for approval. - * Read-only calls are not asked. + * The model explains the calls a person may review: tools whose calls can + * wait for approval (Always Required or Decided by Policy) and tools whose + * effects leave this org (an operator may reconcile them). Read-only calls + * are not asked. A policy decides per call after the schema is sent, so a + * Decided by Policy tool is asked as if it will wait. */ @TestVisible private static final String APPROVER_RATIONALE = - 'Shown to the person who must approve this action. In two or three short sentences: what will change (current value to new value), ' + - 'the facts from records or the request that justify it, and anything uncertain the approver should check. Do not claim anything is approved.'; + 'Shown to the person who must approve this action. In two or three short sentences: what this call will do and to which record ' + + '(for an update, the current and new value), the facts from records or the request that support it, and every condition the action ' + + 'depends on that you did not confirm with a tool. If you confirmed every condition, say so. ' + + 'If this call only reads, write one sentence on what you need to learn and why, and nothing else. Do not mention approval.'; @TestVisible private static final String REVIEWER_RATIONALE = 'Shown to a person who may review this action. In one or two short sentences: the facts from records or the request that make it needed now. ' + - 'Do not repeat the inputs or claim anything is approved.'; + 'Do not repeat the inputs or mention approval.'; + /** The rationale description this tool is asked for, or null when the call is not asked why. */ @TestVisible - private static Boolean asksRationale(AgentTool__c tool, Boolean policyMayAskApproval) { - return policyMayAskApproval || tool.RequiresApproval__c == true || AgentToolTypeService.mayChangeOutside(tool); + private static String rationalePrompt(AgentTool__c tool) { + if (AgentApprovalService.mode(tool) != AgentKernelModels.APPROVAL_NOT_REQUIRED) { + return APPROVER_RATIONALE; + } + return AgentToolTypeService.mayChangeOutside(tool) ? REVIEWER_RATIONALE : null; } /** * Adds the required rationale property first, so the model states why before * it writes the arguments. Tool schemas are closed objects (checked at save), - * and a tool may not declare the property itself. An approver decides from - * this text, so an approval tool asks for the change and what to check. + * and a tool may not declare the property itself. */ @TestVisible - private static String withRationale(String inputSchema, Boolean forApprover) { + private static String withRationale(String inputSchema, String description) { Object parsed = String.isBlank(inputSchema) ? null : JSON.deserializeUntyped(inputSchema); if (!(parsed instanceof Map) || ((Map) parsed).get('type') != 'object') { return inputSchema; @@ -558,7 +563,7 @@ public without sharing class ModelRequestAssembler { generator.writeFieldName(AgentKernelModels.TOOL_RATIONALE_PROPERTY); generator.writeStartObject(); generator.writeStringField('type', 'string'); - generator.writeStringField('description', forApprover ? APPROVER_RATIONALE : REVIEWER_RATIONALE); + generator.writeStringField('description', description); generator.writeEndObject(); for (String name : declared.keySet()) { generator.writeObjectField(name, declared.get(name)); diff --git a/force-app/main/default/classes/ToolApprovalFingerprint.cls b/force-app/main/default/classes/ToolApprovalFingerprint.cls index b08ae4a..7470609 100644 --- a/force-app/main/default/classes/ToolApprovalFingerprint.cls +++ b/force-app/main/default/classes/ToolApprovalFingerprint.cls @@ -14,6 +14,7 @@ /** Binds a human approval to the exact pending input and executable tool configuration. */ public inherited sharing class ToolApprovalFingerprint { public static String compute(AgentCommand__c command, AgentTool__c tool, AgentDefinition__c definition) { + String approvalMode = AgentApprovalService.mode(tool); List approvedSurface = new List{ String.valueOf(command.Id), command.OperationKey__c, @@ -22,9 +23,9 @@ public inherited sharing class ToolApprovalFingerprint { tool.DeveloperName__c, AgentToolTypeService.bindingHash(tool), tool.InputSchema__c, - String.valueOf(tool.RequiresApproval__c), + approvalMode, tool.AllowedSensitiveDataCategories__c, - definition.ToolAuthorizationPolicyClass__c + approvalMode == AgentKernelModels.APPROVAL_DECIDED_BY_POLICY ? definition.ToolAuthorizationPolicyClass__c : null }; return EncodingUtil.convertToHex(Crypto.generateDigest('SHA-256', Blob.valueOf(JSON.serialize(approvedSurface)))); } diff --git a/force-app/main/default/classes/ToolCommandExecutor.cls b/force-app/main/default/classes/ToolCommandExecutor.cls index 1b44dd5..56e2178 100644 --- a/force-app/main/default/classes/ToolCommandExecutor.cls +++ b/force-app/main/default/classes/ToolCommandExecutor.cls @@ -203,7 +203,7 @@ public without sharing class ToolCommandExecutor { ); return; } - if (!evaluatePolicy(run, command, tool)) { + if (!authorize(run, command, tool)) { return; } // Built again in this transaction, so a value removed since the model's turn is refused before the tool runs. @@ -451,21 +451,68 @@ public without sharing class ToolCommandExecutor { submit(outcome); } - private Boolean evaluatePolicy(AgentRun__c run, AgentCommand__c command, AgentTool__c tool) { - Boolean approvalValid; + /** + * Not Required runs every call. Always Required waits for the tool's + * approver until a person approves this exact call. Decided by Policy asks + * the policy: a deny always stops the call, an approved call runs, and a + * hold waits for the approver the policy names. + */ + private Boolean authorize(AgentRun__c run, AgentCommand__c command, AgentTool__c tool) { + String approvalMode = AgentApprovalService.mode(tool); + if (approvalMode == AgentKernelModels.APPROVAL_NOT_REQUIRED) { + return true; + } + Boolean approved = + command.PolicyApproved__c == true && + !String.isBlank(command.ApprovalFingerprint__c) && + command.ApprovalFingerprint__c == ToolApprovalFingerprint.compute(command, tool, definition); + if (approvalMode == AgentKernelModels.APPROVAL_ALWAYS_REQUIRED) { + if (!approved) { + waitForApprover(run, command, tool, null); + } + return approved; + } + if (String.isBlank(definition.ToolAuthorizationPolicyClass__c)) { + pauseForIntervention( + 'TOOL_AUTHORIZATION_POLICY_INVALID', + 'Tool ' + tool.DeveloperName__c + ' is Decided by Policy, but the agent has no Tool Authorization Policy Class.' + ); + return false; + } + AgentKernelModels.ToolAuthorizationResponse response = evaluatePolicy(run, command, tool); + if (response == null) { + return false; + } + if (response.decision == AgentKernelModels.ToolAuthorizationDecision.Deny) { + continueAfterFailure( + AgentKernelModels.ERROR_TOOL_AUTHORIZATION_DENIED, + CommandWorkerSupport.defaultMessage(response.reason, 'The authorization policy denied this tool call.') + ); + return false; + } + // A person already approved this exact call; asking again would never end for a policy that names someone new. + if (approved || response.decision == AgentKernelModels.ToolAuthorizationDecision.Allow) { + return true; + } + String problem = approvalAnswerProblem(response); + if (problem != null) { + pauseForIntervention('TOOL_AUTHORIZATION_POLICY_INVALID', problem); + return false; + } + waitForApprover(run, command, tool, (AgentKernelModels.ToolApprovalResponse) response); + return false; + } + + /** The policy's answer, or null after pausing the run because the policy failed or broke its contract. */ + private AgentKernelModels.ToolAuthorizationResponse evaluatePolicy(AgentRun__c run, AgentCommand__c command, AgentTool__c tool) { AgentKernelModels.ToolAuthorizationResponse response; - // Only the policy evaluation is policy's fault; approval assignment and the - // wait transition below report their own errors. try { - approvalValid = - command.PolicyApproved__c == true && - !String.isBlank(command.ApprovalFingerprint__c) && - command.ApprovalFingerprint__c == ToolApprovalFingerprint.compute(command, tool, definition); AgentKernelModels.ToolAuthorizationRequest request = new AgentKernelModels.ToolAuthorizationRequest(); request.runId = run.Id; request.commandId = command.Id; request.definitionId = run.AgentDefinition__c; request.toolName = command.ToolName__c; + request.toolType = tool.ToolType__c; request.input = command.Input__c; request.operationKey = command.OperationKey__c; request.attemptNumber = completedAttemptCount; @@ -474,91 +521,78 @@ public without sharing class ToolCommandExecutor { request.attendedById = run.AttendedBy__c; request.resolvedPrincipalId = run.ResolvedPrincipalUser__c; request.executionIdentityId = run.ExecutionIdentity__c; - if (String.isBlank(definition.ToolAuthorizationPolicyClass__c)) { - response = defaultPolicy(tool); - } else { - Exception policyFailure; - ExtensionExecutionGuard.Boundary boundary = ExtensionExecutionGuard.open(); - try { - response = AgentExtensionInvoker.evaluatePolicy(definition, request); - } catch (Exception caughtFailure) { - policyFailure = caughtFailure; - } - ExtensionExecutionGuard.Result isolation = ExtensionExecutionGuard.close(boundary); - if (isolation.budgetExceeded) { - pauseForIntervention('TOOL_AUTHORIZATION_POLICY_BUDGET_EXCEEDED', 'Tool authorization policy exceeded its governor budget.'); - return false; - } - if (isolation.performedDml || isolation.performedCallout || isolation.performedProhibitedAsync() || policyFailure instanceof CalloutException) { - pauseForIntervention('TOOL_AUTHORIZATION_POLICY_INVALID', 'Tool authorization policy evaluation must remain query-only.'); - return false; - } - if (policyFailure != null) { - throw policyFailure; - } + Exception policyFailure; + ExtensionExecutionGuard.Boundary boundary = ExtensionExecutionGuard.open(); + try { + response = AgentExtensionInvoker.evaluatePolicy(definition, request); + } catch (Exception caughtFailure) { + policyFailure = caughtFailure; + } + ExtensionExecutionGuard.Result isolation = ExtensionExecutionGuard.close(boundary); + if (isolation.budgetExceeded) { + pauseForIntervention('TOOL_AUTHORIZATION_POLICY_BUDGET_EXCEEDED', 'Tool authorization policy exceeded its governor budget.'); + return null; + } + if (isolation.performedDml || isolation.performedCallout || isolation.performedProhibitedAsync() || policyFailure instanceof CalloutException) { + pauseForIntervention('TOOL_AUTHORIZATION_POLICY_INVALID', 'Tool authorization policy evaluation must remain query-only.'); + return null; + } + if (policyFailure != null) { + throw policyFailure; } } catch (Exception policyFailure) { pauseForIntervention('TOOL_AUTHORIZATION_POLICY_ERROR', 'Tool authorization policy evaluation failed closed.'); - return false; + return null; } if (response == null || response.decision == null) { pauseForIntervention('TOOL_AUTHORIZATION_POLICY_INVALID', 'Tool authorization policy returned no decision.'); - return false; + return null; } - if (response.decision == AgentKernelModels.ToolAuthorizationDecision.Deny) { - continueAfterFailure( - AgentKernelModels.ERROR_TOOL_AUTHORIZATION_DENIED, - CommandWorkerSupport.defaultMessage(response.reason, 'The authorization policy denied this tool call.') - ); - return false; + return response; + } + + /** Why a Require Approval answer cannot be assigned, or null when it can. */ + private static String approvalAnswerProblem(AgentKernelModels.ToolAuthorizationResponse response) { + if (String.isBlank(response.reason)) { + return 'The authorization policy asked for approval without a reason. Pass the reason the approver should see to requireApproval.'; } - Boolean policyRequiresApproval = response.decision == AgentKernelModels.ToolAuthorizationDecision.RequireApproval; - if (!policyRequiresApproval && response.decision != AgentKernelModels.ToolAuthorizationDecision.Allow) { - pauseForIntervention('TOOL_AUTHORIZATION_POLICY_INVALID', 'Tool authorization policy returned an unsupported decision.'); - return false; + AgentKernelModels.ToolApprovalResponse held = response instanceof AgentKernelModels.ToolApprovalResponse + ? (AgentKernelModels.ToolApprovalResponse) response + : null; + if (held == null || (String.isBlank(held.approverQueue) && held.approverUserId == null)) { + return 'The authorization policy asked for approval without naming an approver. Add byQueue or byUser to requireApproval.'; } - if (approvalValid || (!policyRequiresApproval && tool.RequiresApproval__c != true)) { - return true; + if (held.approverUserId != null && held.approverUserId.getSObjectType() != User.SObjectType) { + return 'The authorization policy\'s byUser Id is not a user. Use byQueue with the queue\'s API name for a queue.'; + } + if (held.timeoutMinutes != null && (held.timeoutMinutes < 1 || held.timeoutMinutes > AgentApprovalService.MAX_TIMEOUT_MINUTES)) { + return 'The authorization policy\'s timeout must be from 1 to ' + AgentApprovalService.MAX_TIMEOUT_MINUTES + ' minutes (7 days).'; } + return null; + } + + /** Opens the approval wait, or pauses the run when nobody can be asked. */ + private void waitForApprover(AgentRun__c run, AgentCommand__c command, AgentTool__c tool, AgentKernelModels.ToolApprovalResponse held) { if (!ExecutionIdentityService.usesIdentity(run)) { pauseForIntervention( 'INITIATING_USER_APPROVAL_UNSUPPORTED', - 'The authorization policy asked for approval, but this agent has no Agent Execution Identity, so its runs cannot wait for approval.' + (held != null ? 'The authorization policy asked for approval' : 'Tool ' + tool.DeveloperName__c + ' requires approval') + + ', but this agent has no Agent Execution Identity, so its runs cannot wait for approval.' ); - return false; - } - if ( - policyRequiresApproval && - (String.isBlank(response.reason) || (response.timeoutSeconds != null && (response.timeoutSeconds < 60 || response.timeoutSeconds > 604800))) - ) { - pauseForIntervention( - 'TOOL_AUTHORIZATION_POLICY_INVALID', - 'Approval authorization result requires a reason and a timeout from 60 to 604,800 seconds.' - ); - return false; + return; } AgentApprovalService.Request approval; try { - approval = policyRequiresApproval - ? AgentApprovalService.request(run, tool, response.reason, response.timeoutSeconds, response.approverId) - : AgentApprovalService.request(run, tool, 'Tool configuration requires approval.', null, null); + approval = AgentApprovalService.request(run, tool, held); } catch (AgentApprovalService.AssignmentException unassigned) { pauseForIntervention('APPROVAL_ASSIGNMENT_UNRESOLVED', unassigned.getMessage()); - return false; + return; } catch (Exception assignmentFailure) { pauseForIntervention('APPROVAL_ASSIGNMENT_UNRESOLVED', 'The approver could not be resolved for this tool call.'); - return false; + return; } // A failed wait transition is a worker failure: the finalizer recovers the leased command. AgentKernel.waitForApproval(run.Id, command.Id, expectedRevision, operationKey, leaseToken, approval); - return false; - } - - private static AgentKernelModels.ToolAuthorizationResponse defaultPolicy(AgentTool__c tool) { - AgentKernelModels.ToolAuthorizationResponse response = new AgentKernelModels.ToolAuthorizationResponse(); - response.decision = AgentKernelModels.ToolAuthorizationDecision.Allow; - response.reason = 'No contextual authorization policy is configured.'; - return response; } private void waitForExternalInput(AgentKernelModels.ToolResponse response, AgentTool__c tool) { diff --git a/force-app/main/default/classes/tests/AgentConfigurationValidationServiceTest.cls b/force-app/main/default/classes/tests/AgentConfigurationValidationServiceTest.cls index c5c83c7..65d63fc 100644 --- a/force-app/main/default/classes/tests/AgentConfigurationValidationServiceTest.cls +++ b/force-app/main/default/classes/tests/AgentConfigurationValidationServiceTest.cls @@ -95,6 +95,104 @@ private class AgentConfigurationValidationServiceTest { System.assertEquals(false, codes(validate(definition.Id)).contains('UNSCOPED_RECORD_UPDATE')); } + @IsTest + static void aPolicyWithNoToolToDecideIsFlagged() { + AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); + AgentKernelTestFactory.useDelegatedPrincipal(definition); + AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'refund_tool', '{"type":"object"}'); + definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; + update definition; + + AgentConfigurationValidationService.ValidationReport report = validate(definition.Id); + System.assertEquals(true, report.isValid, 'The tools still run; the policy is only never asked.'); + System.assert(codes(report).contains('POLICY_NEVER_RUNS')); + + tool.ApprovalMode__c = AgentKernelModels.APPROVAL_DECIDED_BY_POLICY; + tool.ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY; + AgentKernelTestFactory.updateRecord(tool); + System.assertEquals(false, codes(validate(definition.Id)).contains('POLICY_NEVER_RUNS'), 'The policy decides the tool now.'); + } + + @IsTest + static void aPolicyDecidedToolNeedsTheAgentsPolicy() { + AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); + AgentKernelTestFactory.useDelegatedPrincipal(definition); + AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'refund_tool', '{"type":"object"}'); + AgentTool__c decided = new AgentTool__c( + Id = tool.Id, + ApprovalMode__c = AgentKernelModels.APPROVAL_DECIDED_BY_POLICY, + ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY + ); + + Database.SaveResult refused = Database.update(decided, false); + System.assertEquals(false, refused.isSuccess(), 'Nothing would decide the tool without a policy.'); + System.assertEquals(AgentApprovalService.POLICY_REQUIRED_MESSAGE, refused.getErrors()[0].getMessage()); + + definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; + update definition; + AgentKernelTestFactory.updateRecord(decided); + definition.ToolAuthorizationPolicyClass__c = null; + Database.SaveResult cleared = Database.update(definition, false); + System.assertEquals(false, cleared.isSuccess(), 'Clearing the policy would leave the tool undecided.'); + System.assert(cleared.getErrors()[0].getMessage().contains('refund_tool'), cleared.getErrors()[0].getMessage()); + } + + @IsTest + static void approvalSettingsMatchTheApprovalChoice() { + AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); + AgentKernelTestFactory.useDelegatedPrincipal(definition); + definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; + update definition; + AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'refund_tool', '{"type":"object"}'); + Id approverId = AgentKernelTestFactory.approverUser(); + + assertRefused( + new AgentTool__c(Id = tool.Id, ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY), + 'Approval is Not Required, so clear the approver, Approval Timeout, and When Approval Expires.' + ); + assertRefused( + new AgentTool__c( + Id = tool.Id, + ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED, + ApproverType__c = AgentKernelModels.APPROVER_USER, + ApproverUser__c = approverId + ), + 'Choose When Approval Expires for a tool whose calls can wait for approval.' + ); + assertRefused( + new AgentTool__c( + Id = tool.Id, + ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED, + ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY + ), + 'Choose an Approver Type for a tool whose Approval is Always Required.' + ); + assertRefused( + new AgentTool__c( + Id = tool.Id, + ApprovalMode__c = AgentKernelModels.APPROVAL_DECIDED_BY_POLICY, + ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY, + ApproverType__c = AgentKernelModels.APPROVER_USER, + ApproverUser__c = approverId + ), + 'Approval is Decided by Policy, so the policy names the approver. Clear Approver Type, Approver User, and Approver Queue.' + ); + AgentKernelTestFactory.updateRecord( + new AgentTool__c( + Id = tool.Id, + ApprovalMode__c = AgentKernelModels.APPROVAL_DECIDED_BY_POLICY, + ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_PAUSE, + ApprovalTimeoutMinutes__c = 60 + ) + ); + } + + private static void assertRefused(AgentTool__c tool, String message) { + Database.SaveResult result = Database.update(tool, false); + System.assertEquals(false, result.isSuccess(), message); + System.assertEquals(message, result.getErrors()[0].getMessage()); + } + @IsTest static void initiatingUserDoesNotRequireServiceUser() { AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); @@ -111,7 +209,12 @@ private class AgentConfigurationValidationServiceTest { AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'write_case', '{"type":"object"}'); Database.SaveResult result = Database.update( - new AgentTool__c(Id = tool.Id, RequiresApproval__c = true, ApproverType__c = AgentKernelModels.APPROVER_DELIVERY_CHANNEL), + new AgentTool__c( + Id = tool.Id, + ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED, + ApproverType__c = AgentKernelModels.APPROVER_DELIVERY_CHANNEL, + ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY + ), false ); @@ -124,8 +227,9 @@ private class AgentConfigurationValidationServiceTest { AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); AgentKernelTestFactory.useDelegatedPrincipal(definition); AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'write_case', '{"type":"object"}'); - tool.RequiresApproval__c = true; + tool.ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED; tool.ApproverType__c = AgentKernelModels.APPROVER_DELIVERY_CHANNEL; + tool.ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY; AgentKernelTestFactory.updateRecord(tool); definition.ExecutionIdentity__c = null; @@ -365,7 +469,6 @@ private class AgentConfigurationValidationServiceTest { Description__c = 'Tool-count validation fixture.', ToolType__c = AgentToolTypeTestFixtures.IN_ORG, InputSchema__c = '{"type":"object","properties":{},"additionalProperties":false}', - RequiresApproval__c = false, IsActive__c = true, SortOrder__c = index ) diff --git a/force-app/main/default/classes/tests/AgentExecutionSpineTest.cls b/force-app/main/default/classes/tests/AgentExecutionSpineTest.cls index 393b2db..2231d23 100644 --- a/force-app/main/default/classes/tests/AgentExecutionSpineTest.cls +++ b/force-app/main/default/classes/tests/AgentExecutionSpineTest.cls @@ -93,22 +93,29 @@ private class AgentExecutionSpineTest { @IsTest static void onlyReviewableCallsAreAskedWhyAndTheReasonIsReplayed() { - AgentTool__c readOnly = new AgentTool__c(ToolType__c = AgentToolTypeTestFixtures.IN_ORG, RequiresApproval__c = false); - AgentTool__c approved = new AgentTool__c(ToolType__c = AgentToolTypeTestFixtures.IN_ORG, RequiresApproval__c = true); - AgentTool__c external = new AgentTool__c(ToolType__c = AgentToolTypeTestFixtures.EXTERNAL, RequiresApproval__c = false); - System.assertEquals(false, ModelRequestAssembler.asksRationale(readOnly, false), 'A read-only call is not asked why.'); - System.assertEquals(true, ModelRequestAssembler.asksRationale(readOnly, true), 'A policy may ask for approval of any call.'); - System.assertEquals(true, ModelRequestAssembler.asksRationale(approved, false)); - System.assertEquals(true, ModelRequestAssembler.asksRationale(external, false), 'An operator may reconcile an external effect.'); - AgentTool__c readOnlyExternal = new AgentTool__c(ToolType__c = AgentToolTypeTestFixtures.READ_ONLY_EXTERNAL, RequiresApproval__c = false); - System.assertEquals(false, ModelRequestAssembler.asksRationale(readOnlyExternal, false), 'A read-only outside lookup is not asked why.'); - System.assertEquals(true, ModelRequestAssembler.asksRationale(readOnlyExternal, true), 'A policy may still ask for approval.'); + AgentTool__c readOnly = new AgentTool__c(ToolType__c = AgentToolTypeTestFixtures.IN_ORG); + AgentTool__c approved = new AgentTool__c(ToolType__c = AgentToolTypeTestFixtures.IN_ORG, ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED); + AgentTool__c external = new AgentTool__c(ToolType__c = AgentToolTypeTestFixtures.EXTERNAL); + String approver = ModelRequestAssembler.APPROVER_RATIONALE; + String reviewer = ModelRequestAssembler.REVIEWER_RATIONALE; + System.assertEquals(null, ModelRequestAssembler.rationalePrompt(readOnly), 'A read-only call is not asked why.'); + System.assertEquals(approver, ModelRequestAssembler.rationalePrompt(approved)); + System.assertEquals(reviewer, ModelRequestAssembler.rationalePrompt(external), 'An operator may reconcile an external effect.'); + external.ApprovalMode__c = AgentKernelModels.APPROVAL_DECIDED_BY_POLICY; + System.assertEquals(approver, ModelRequestAssembler.rationalePrompt(external), 'An approver may read a call the policy holds.'); + AgentTool__c readOnlyExternal = new AgentTool__c(ToolType__c = AgentToolTypeTestFixtures.READ_ONLY_EXTERNAL); + System.assertEquals(null, ModelRequestAssembler.rationalePrompt(readOnlyExternal), 'A read-only outside lookup is not asked why.'); + readOnlyExternal.ApprovalMode__c = AgentKernelModels.APPROVAL_DECIDED_BY_POLICY; + System.assertEquals(approver, ModelRequestAssembler.rationalePrompt(readOnlyExternal), 'A policy may still ask for approval.'); String schema = '{"type":"object","properties":{"value":{"type":"string"}},"additionalProperties":false}'; System.assert( - ModelRequestAssembler.withRationale(schema, true).contains(ModelRequestAssembler.APPROVER_RATIONALE), - 'An approval tool asks for what changes and what the approver should check.' + ModelRequestAssembler.withRationale(schema, approver).contains(approver), + 'An approval call asks what it will do and which conditions were not confirmed.' + ); + System.assert( + approver.contains('If this call only reads'), + 'A policy agent asks reads for the approver text too, so the text keeps a read to one sentence.' ); - System.assert(ModelRequestAssembler.withRationale(schema, false).contains(ModelRequestAssembler.REVIEWER_RATIONALE)); resetDoubles(); KernelTestModelProvider.mode = 'toolThenFinal'; @@ -695,7 +702,6 @@ private class AgentExecutionSpineTest { Description__c = 'Looks up an order.', ToolType__c = AgentToolTypeTestFixtures.AUTHORING, ImplementationConfiguration__c = configuration, - RequiresApproval__c = false, IsActive__c = true, SortOrder__c = 1 ); diff --git a/force-app/main/default/classes/tests/AgentKernelTestFactory.cls b/force-app/main/default/classes/tests/AgentKernelTestFactory.cls index 11f9c29..03a9bea 100644 --- a/force-app/main/default/classes/tests/AgentKernelTestFactory.cls +++ b/force-app/main/default/classes/tests/AgentKernelTestFactory.cls @@ -300,7 +300,6 @@ public class AgentKernelTestFactory { Description__c = 'A test-only kernel tool.', ToolType__c = AgentToolTypeTestFixtures.IN_ORG, InputSchema__c = inputSchema, - RequiresApproval__c = false, IsActive__c = true, SortOrder__c = 1 ); @@ -308,6 +307,15 @@ public class AgentKernelTestFactory { return tool; } + /** A tool whose calls the agent's policy decides. Set the definition's policy class first. */ + public static AgentTool__c createPolicyTool(Id definitionId, String developerName) { + AgentTool__c tool = createTool(definitionId, developerName, '{"type":"object"}'); + tool.ApprovalMode__c = AgentKernelModels.APPROVAL_DECIDED_BY_POLICY; + tool.ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY; + updateRecord(tool); + return tool; + } + public static AgentContextProvider__c createContextProvider(Id definitionId, String developerName, String implementationReference) { AgentContextProvider__c provider = new AgentContextProvider__c( Name = 'Kernel Test Context ' + developerName, diff --git a/force-app/main/default/classes/tests/ApprovalDecisionScenarioTest.cls b/force-app/main/default/classes/tests/ApprovalDecisionScenarioTest.cls index 088407c..ef95c27 100644 --- a/force-app/main/default/classes/tests/ApprovalDecisionScenarioTest.cls +++ b/force-app/main/default/classes/tests/ApprovalDecisionScenarioTest.cls @@ -170,11 +170,12 @@ private class ApprovalDecisionScenarioTest { ); AgentTool__c tool = new AgentTool__c( DeveloperName__c = 'refund_tool', + ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED, ApproverType__c = AgentKernelModels.APPROVER_ANCHOR_OWNER, ApproverQueue__c = 'Agent_Approval_Fallback_Queue' ); - AgentApprovalService.Request request = AgentApprovalService.request(run, tool, 'Refund above the threshold.', null, null); + AgentApprovalService.Request request = AgentApprovalService.request(run, tool, null); System.assertEquals(AgentKernelModels.APPROVER_ANCHOR_OWNER, request.approverType, 'The owner started the run and still decides it.'); System.assertEquals(approverId, request.ownerId, 'The fallback queue is only for an owner who cannot decide.'); @@ -203,11 +204,12 @@ private class ApprovalDecisionScenarioTest { ); AgentTool__c tool = new AgentTool__c( DeveloperName__c = 'refund_tool', + ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED, ApproverType__c = AgentKernelModels.APPROVER_ANCHOR_OWNER, ApproverQueue__c = 'Agent_Refund_Review_Queue' ); - AgentApprovalService.Request request = AgentApprovalService.request(run, tool, 'Refund above the threshold.', null, null); + AgentApprovalService.Request request = AgentApprovalService.request(run, tool, null); System.assertEquals(AgentKernelModels.APPROVER_QUEUE, request.approverType, 'A Case queue cannot hold approvals, so the fallback decides.'); System.assertEquals(fallback.Id, request.ownerId); @@ -224,14 +226,112 @@ private class ApprovalDecisionScenarioTest { AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'queued_tool', '{"type":"object"}'); AgentRun__c run = AgentKernelTestFactory.getRun(AgentKernelTestFactory.startRunWithoutDispatch(definition).runId); + tool.ApprovalMode__c = AgentKernelModels.APPROVAL_DECIDED_BY_POLICY; + AgentKernelModels.ToolApprovalResponse held = policyApproval('Needs review.').byQueue(queue.DeveloperName); + try { - AgentApprovalService.request(run, tool, 'Needs review.', null, queue.Id); + AgentApprovalService.request(run, tool, held); System.assert(false, 'An approval must not wait on a queue nobody can decide.'); } catch (AgentApprovalService.AssignmentException expected) { System.assert(expected.getMessage().contains('no active member')); } } + @IsTest + static void aPolicyDecidedToolWaitsForThePolicyApproverAndDeadline() { + Id policyApproverId = AgentKernelTestFactory.approverUser(); + AgentTool__c tool = policyDecidedTool(); + tool.ApprovalTimeoutMinutes__c = 600; + + AgentApprovalService.Request request = AgentApprovalService.request( + attendedRun(), + tool, + policyApproval('Over 200.').byUser(policyApproverId).withTimeoutMinutes(5) + ); + + System.assertEquals(policyApproverId, request.ownerId); + System.assertEquals(AgentKernelModels.APPROVER_USER, request.approverType); + System.assertEquals(AgentApprovalService.REQUIRED_BY_POLICY, request.requiredBy); + System.assertEquals('Over 200.', request.reason); + System.assert(request.expiresAt <= System.now().addMinutes(5), 'The policy deadline replaces the tool one.'); + System.assertEquals(AgentKernelModels.APPROVAL_EXPIRY_PAUSE, request.expiryAction, 'The tool decides what happens on expiry.'); + } + + @IsTest + static void aPolicyWithoutADeadlineUsesTheToolTimeout() { + AgentTool__c tool = policyDecidedTool(); + tool.ApprovalTimeoutMinutes__c = 30; + + AgentApprovalService.Request request = AgentApprovalService.request( + attendedRun(), + tool, + policyApproval('Over 200.').byUser(AgentKernelTestFactory.approverUser()) + ); + + System.assert(request.expiresAt > System.now().addMinutes(29) && request.expiresAt <= System.now().addMinutes(30), 'The tool timeout applies.'); + } + + @IsTest + static void anAlwaysRequiredToolWaitsForItsOwnApprover() { + Id approverId = AgentKernelTestFactory.approverUser(); + AgentTool__c tool = new AgentTool__c( + DeveloperName__c = 'refund_tool', + ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED, + ApproverType__c = AgentKernelModels.APPROVER_USER, + ApproverUser__c = approverId + ); + + AgentApprovalService.Request request = AgentApprovalService.request(attendedRun(), tool, null); + + System.assertEquals(approverId, request.ownerId); + System.assertEquals(AgentApprovalService.REQUIRED_BY_TOOL, request.requiredBy); + System.assertEquals(AgentApprovalService.TOOL_APPROVAL_REASON, request.reason); + System.assertEquals(AgentKernelModels.APPROVAL_EXPIRY_DENY, request.expiryAction, 'A blank expiry action denies.'); + System.assert(request.expiresAt > System.now().addMinutes(1439), 'A blank timeout waits 24 hours.'); + } + + @IsTest + static void aPolicyDecidedToolNeedsTheApproverThePolicyNames() { + try { + AgentApprovalService.request(attendedRun(), policyDecidedTool(), null); + System.assert(false, 'The tool has no approver of its own.'); + } catch (AgentApprovalService.AssignmentException expected) { + System.assert(expected.getMessage().contains('named no approver'), expected.getMessage()); + } + } + + @IsTest + static void aPolicyCannotNameAnApproverWithoutHumanDecision() { + Id outsiderId = AgentKernelTestFactory.standardUserWith(new List()).Id; + try { + AgentApprovalService.request(attendedRun(), policyDecidedTool(), policyApproval('Over 200.').byUser(outsiderId)); + System.assert(false, 'A user without Human Decision must not be assigned.'); + } catch (AgentApprovalService.AssignmentException expected) { + System.assert(expected.getMessage().contains('Human Decision'), expected.getMessage()); + } + } + + private static AgentTool__c policyDecidedTool() { + return new AgentTool__c( + DeveloperName__c = 'refund_tool', + ApprovalMode__c = AgentKernelModels.APPROVAL_DECIDED_BY_POLICY, + ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_PAUSE + ); + } + + private static AgentKernelModels.ToolApprovalResponse policyApproval(String reason) { + return new AgentKernelModels.ToolAuthorizationRequest().requireApproval(reason); + } + + /** An unsaved run of an agent with an identity, started by the running user. */ + private static AgentRun__c attendedRun() { + return new AgentRun__c( + InitiatingUser__c = UserInfo.getUserId(), + ExecutionIdentity__c = AgentKernelTestFactory.unsavedIdentityId(), + ExpectedPrincipalUser__c = AgentKernelTestFactory.servicePrincipalUser().Id + ); + } + private class AcceptedLoopback implements HttpCalloutMock { public HttpResponse respond(HttpRequest request) { HttpResponse response = new HttpResponse(); diff --git a/force-app/main/default/classes/tests/CriticalBehaviorGapTest.cls b/force-app/main/default/classes/tests/CriticalBehaviorGapTest.cls index 983802f..6352afc 100644 --- a/force-app/main/default/classes/tests/CriticalBehaviorGapTest.cls +++ b/force-app/main/default/classes/tests/CriticalBehaviorGapTest.cls @@ -44,7 +44,7 @@ private class CriticalBehaviorGapTest { } @IsTest - static void onlyRequiresApprovalMakesAToolWait() { + static void onlyTheApprovalSettingMakesAToolWait() { resetDoubles(); ToolFixture unattended = leasedTool('unattended_write', true, false, '{}'); executeTool(unattended); @@ -56,7 +56,8 @@ private class CriticalBehaviorGapTest { AgentKernelTestFactory.useDelegatedPrincipal(definition); AgentTool__c gated = AgentKernelTestFactory.createTool(definition.Id, 'gated_read', '{"type":"object"}'); Id approverId = AgentKernelTestFactory.approverUser(); - gated.RequiresApproval__c = true; + gated.ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED; + gated.ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY; gated.ApproverType__c = AgentKernelModels.APPROVER_USER; gated.ApproverUser__c = approverId; AgentKernelTestFactory.updateRecord(gated); @@ -312,7 +313,7 @@ private class CriticalBehaviorGapTest { AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; update definition; - AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'policy_failure', '{"type":"object"}'); + AgentTool__c tool = AgentKernelTestFactory.createPolicyTool(definition.Id, 'policy_failure'); AgentKernelModels.StartResult started = AgentKernelTestFactory.startRunWithoutDispatch(definition); ToolFixture fixture = leaseExistingTool(started, tool, false, '{}'); KernelTestToolPolicy.throwUnexpected = true; diff --git a/force-app/main/default/classes/tests/DuplicateDeliveryTest.cls b/force-app/main/default/classes/tests/DuplicateDeliveryTest.cls index 9ffb3f6..fe925cd 100644 --- a/force-app/main/default/classes/tests/DuplicateDeliveryTest.cls +++ b/force-app/main/default/classes/tests/DuplicateDeliveryTest.cls @@ -99,10 +99,11 @@ private class DuplicateDeliveryTest { '{"type":"object","properties":{"amount":{"type":"number"}},"required":["amount"],"additionalProperties":false}' ); refund.ToolType__c = AgentToolTypeTestFixtures.EXTERNAL; - refund.RequiresApproval__c = true; + refund.ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED; + refund.ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY; refund.ApproverType__c = AgentKernelModels.APPROVER_USER; refund.ApproverUser__c = fixture.approverId; - refund.ApprovalTimeoutHours__c = 24; + refund.ApprovalTimeoutMinutes__c = 1440; AgentKernelTestFactory.updateRecord(refund); AgentConfigService.clearCacheForTest(); AttendedAccessService.attendedForTest = false; diff --git a/force-app/main/default/classes/tests/InlineToolExecutionTest.cls b/force-app/main/default/classes/tests/InlineToolExecutionTest.cls index 82d52cc..cc088b0 100644 --- a/force-app/main/default/classes/tests/InlineToolExecutionTest.cls +++ b/force-app/main/default/classes/tests/InlineToolExecutionTest.cls @@ -147,11 +147,12 @@ private class InlineToolExecutionTest { @IsTest static void aCallThatNeedsApprovalStopsTheRest() { KernelTestToolPolicy.configuredDecision = AgentKernelModels.ToolAuthorizationDecision.RequireApproval; + KernelTestToolPolicy.configuredApproverUserId = AgentKernelTestFactory.approverUser(); AgentDefinition__c definition = multiToolAgent(); definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; AgentKernelTestFactory.updateRecord(definition); - AgentKernelTestFactory.createTool(definition.Id, 'tool_a', '{"type":"object"}'); - AgentKernelTestFactory.createTool(definition.Id, 'tool_b', '{"type":"object"}'); + AgentKernelTestFactory.createPolicyTool(definition.Id, 'tool_a'); + AgentKernelTestFactory.createPolicyTool(definition.Id, 'tool_b'); scriptCalls(new List{ 'tool_a', 'tool_b' }); AgentKernelModels.StartResult started = AgentKernelTestFactory.startRunWithoutDispatch(definition); @@ -287,7 +288,7 @@ private class InlineToolExecutionTest { AgentKernelTestFactory.updateRecord(definition); KernelTestToolPolicy.deniedToolNames.add('tool_b'); typedTool(definition.Id, 'tool_a', AgentToolTypeTestFixtures.READ_ONLY_EXTERNAL); - AgentKernelTestFactory.createTool(definition.Id, 'tool_b', '{"type":"object"}'); + AgentKernelTestFactory.createPolicyTool(definition.Id, 'tool_b'); scriptCalls(new List{ 'tool_a', 'tool_b' }); AgentKernelModels.StartResult started = AgentKernelTestFactory.startRunWithoutDispatch(definition); runModelWorker(started.runId); @@ -304,7 +305,7 @@ private class InlineToolExecutionTest { AgentDefinition__c definition = multiToolAgent(); definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; AgentKernelTestFactory.updateRecord(definition); - AgentKernelTestFactory.createTool(definition.Id, 'tool_a', '{"type":"object"}'); + AgentKernelTestFactory.createPolicyTool(definition.Id, 'tool_a'); KernelTestModelProvider.script = null; scriptCalls(new List{ 'tool_a' }); AgentKernelModels.StartResult started = AgentKernelTestFactory.startRunWithoutDispatch(definition); diff --git a/force-app/main/default/classes/tests/KernelTestToolPolicy.cls b/force-app/main/default/classes/tests/KernelTestToolPolicy.cls index 0fba318..7b7abdd 100644 --- a/force-app/main/default/classes/tests/KernelTestToolPolicy.cls +++ b/force-app/main/default/classes/tests/KernelTestToolPolicy.cls @@ -15,7 +15,8 @@ global with sharing class KernelTestToolPolicy implements IToolAuthorizationPolicy { global static AgentKernelModels.ToolAuthorizationDecision configuredDecision = AgentKernelModels.ToolAuthorizationDecision.Allow; global static String configuredReason = 'Approval required by test policy.'; - global static Id configuredApproverId; + global static Id configuredApproverUserId; + global static String configuredApproverQueue; global static Boolean performDml = false; global static Boolean throwUnexpected = false; global static AgentKernelModels.ToolAuthorizationRequest lastRequest; @@ -30,11 +31,19 @@ global with sharing class KernelTestToolPolicy implements IToolAuthorizationPoli if (throwUnexpected) { throw new AgentKernelException.ConfigurationException('Unexpected policy failure.'); } - AgentKernelModels.ToolAuthorizationResponse response = new AgentKernelModels.ToolAuthorizationResponse(); - response.decision = deniedToolNames.contains(request.toolName) ? AgentKernelModels.ToolAuthorizationDecision.Deny : configuredDecision; - response.reason = configuredReason; - response.approverId = configuredApproverId; - response.timeoutSeconds = 3600; - return response; + if (deniedToolNames.contains(request.toolName) || configuredDecision == AgentKernelModels.ToolAuthorizationDecision.Deny) { + return request.deny(configuredReason); + } + if (configuredDecision != AgentKernelModels.ToolAuthorizationDecision.RequireApproval) { + return request.allow(); + } + AgentKernelModels.ToolApprovalResponse held = request.requireApproval(configuredReason).withTimeoutMinutes(60); + if (configuredApproverQueue != null) { + held.byQueue(configuredApproverQueue); + } + if (configuredApproverUserId != null) { + held.byUser(configuredApproverUserId); + } + return held; } } diff --git a/force-app/main/default/classes/tests/KernelV11LifecycleTest.cls b/force-app/main/default/classes/tests/KernelV11LifecycleTest.cls index c413ed1..101fde6 100644 --- a/force-app/main/default/classes/tests/KernelV11LifecycleTest.cls +++ b/force-app/main/default/classes/tests/KernelV11LifecycleTest.cls @@ -20,7 +20,7 @@ private class KernelV11LifecycleTest { AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; update definition; - AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'denied_tool', '{"type":"object"}'); + AgentTool__c tool = AgentKernelTestFactory.createPolicyTool(definition.Id, 'denied_tool'); AgentKernelModels.StartResult started = AgentKernelTestFactory.startRunWithoutDispatch(definition); AgentKernelTestFactory.applyOutcome( AgentKernelModels.continueWith( @@ -65,7 +65,7 @@ private class KernelV11LifecycleTest { AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; update definition; - AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'isolated_policy_tool', '{"type":"object"}'); + AgentTool__c tool = AgentKernelTestFactory.createPolicyTool(definition.Id, 'isolated_policy_tool'); AgentKernelModels.StartResult started = AgentKernelTestFactory.startRunWithoutDispatch(definition); AgentKernelTestFactory.applyOutcome( AgentKernelModels.continueWith( @@ -89,17 +89,201 @@ private class KernelV11LifecycleTest { KernelTestToolPolicy.performDml = false; } + @IsTest + static void aPolicyQueueThatIsMissingPausesTheRunInsteadOfFallingBack() { + KernelTestToolPolicy.configuredDecision = AgentKernelModels.ToolAuthorizationDecision.RequireApproval; + KernelTestToolPolicy.configuredApproverQueue = 'No_Such_Approval_Queue'; + AgentRun__c run = runPolicyCall('missing_queue_tool'); + + System.assertEquals(AgentKernelModels.RUN_STATUS_PAUSED, run.Status__c); + System.assertEquals('APPROVAL_ASSIGNMENT_UNRESOLVED', run.ErrorCode__c); + System.assert(run.ErrorMessage__c.contains('No_Such_Approval_Queue'), run.ErrorMessage__c); + System.assertEquals(0, [SELECT COUNT() FROM AgentApproval__c WHERE AgentRun__c = :run.Id], 'Nobody else decides instead.'); + } + + @IsTest + static void aHoldThatNamesNoApproverPausesTheRun() { + KernelTestTool.invocationCount = 0; + KernelTestToolPolicy.configuredDecision = AgentKernelModels.ToolAuthorizationDecision.RequireApproval; + AgentRun__c run = runPolicyCall('unnamed_approver_tool'); + + System.assertEquals(AgentKernelModels.RUN_STATUS_PAUSED, run.Status__c); + System.assertEquals('TOOL_AUTHORIZATION_POLICY_INVALID', run.ErrorCode__c); + System.assert(run.ErrorMessage__c.contains('without naming an approver'), run.ErrorMessage__c); + System.assertEquals(0, [SELECT COUNT() FROM AgentApproval__c WHERE AgentRun__c = :run.Id]); + System.assertEquals(0, KernelTestTool.invocationCount, 'A held call never runs without an approval.'); + } + + @IsTest + static void aPolicyDecidedCallWaitsForThePolicyApprover() { + KernelTestToolPolicy.configuredDecision = AgentKernelModels.ToolAuthorizationDecision.RequireApproval; + Id policyApproverId = AgentKernelTestFactory.approverUser(); + KernelTestToolPolicy.configuredApproverUserId = policyApproverId; + AgentRun__c run = runPolicyCall('policy_routed_tool'); + + System.assertEquals(AgentKernelModels.RUN_STATUS_WAITING, run.Status__c); + AgentApproval__c approval = AgentKernelTestFactory.pendingApproval(run.Id); + System.assertEquals(policyApproverId, approval.OwnerId); + System.assert(approval.ExpiresAt__c <= System.now().addMinutes(60), 'The policy deadline replaces the tool one.'); + System.assertEquals(AgentApprovalService.REQUIRED_BY_POLICY, waitingPayload(run.Id).get('requiredBy')); + } + + @IsTest + static void thePolicyIsNeverAskedAboutOtherTools() { + KernelTestTool.invocationCount = 0; + KernelTestToolPolicy.configuredDecision = AgentKernelModels.ToolAuthorizationDecision.Deny; + AgentDefinition__c definition = policyAgent(); + AgentTool__c plain = AgentKernelTestFactory.createTool(definition.Id, 'plain_tool', '{"type":"object"}'); + Id toolApproverId = AgentKernelTestFactory.approverUser(); + AgentTool__c pinned = AgentKernelTestFactory.createTool(definition.Id, 'pinned_tool', '{"type":"object"}'); + pinned.ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED; + pinned.ApproverType__c = AgentKernelModels.APPROVER_USER; + pinned.ApproverUser__c = toolApproverId; + pinned.ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY; + AgentKernelTestFactory.updateRecord(pinned); + + AgentRun__c plainRun = runCall(definition, plain); + System.assertEquals(1, KernelTestTool.invocationCount, 'Not Required runs the call; the denying policy is not asked.'); + System.assertNotEquals(AgentKernelModels.RUN_STATUS_PAUSED, plainRun.Status__c); + System.assertEquals(null, KernelTestToolPolicy.lastRequest); + + AgentRun__c pinnedRun = runCall(definition, pinned); + System.assertEquals(AgentKernelModels.RUN_STATUS_WAITING, pinnedRun.Status__c, 'Always Required waits; the denying policy is not asked.'); + System.assertEquals(toolApproverId, AgentKernelTestFactory.pendingApproval(pinnedRun.Id).OwnerId); + System.assertEquals(AgentApprovalService.REQUIRED_BY_TOOL, waitingPayload(pinnedRun.Id).get('requiredBy')); + System.assertEquals(null, KernelTestToolPolicy.lastRequest); + } + + @IsTest + static void anApprovedCallRunsWhenThePolicyNamesSomeoneElse() { + KernelTestTool.invocationCount = 0; + AgentRun__c run = approvedPolicyCall('escalated_tool'); + KernelTestToolPolicy.configuredApproverUserId = AgentKernelTestFactory.approverUser(); + + AgentApproval__c approval = AgentKernelTestFactory.pendingApproval(run.Id); + AgentCommand__c command = lease(approval.AgentCommand__c, 'escalated-lease'); + System.runAs(AgentKernelTestFactory.worker(run.Id)) { + new ToolCommandQueueable(run.Id, command.Id, command.ExpectedRunRevision__c, command.OperationKey__c, command.LeaseToken__c).execute(null); + } + + System.assertEquals(1, KernelTestTool.invocationCount, 'A person approved this exact call, so it is not asked again.'); + System.assertEquals(1, [SELECT COUNT() FROM AgentApproval__c WHERE AgentRun__c = :run.Id]); + } + + @IsTest + static void aDenyAfterApprovalStillStopsTheCall() { + KernelTestTool.invocationCount = 0; + AgentRun__c run = approvedPolicyCall('revoked_tool'); + KernelTestToolPolicy.configuredDecision = AgentKernelModels.ToolAuthorizationDecision.Deny; + + AgentApproval__c approval = AgentKernelTestFactory.pendingApproval(run.Id); + AgentCommand__c command = lease(approval.AgentCommand__c, 'revoked-lease'); + System.runAs(AgentKernelTestFactory.worker(run.Id)) { + new ToolCommandQueueable(run.Id, command.Id, command.ExpectedRunRevision__c, command.OperationKey__c, command.LeaseToken__c).execute(null); + } + + System.assertEquals(0, KernelTestTool.invocationCount); + System.assertEquals(AgentKernelModels.ERROR_TOOL_AUTHORIZATION_DENIED, AgentKernelTestFactory.getCommand(command.Id).ErrorCode__c); + } + + /** A policy-decided call that its approver has approved, ready to run again. */ + private static AgentRun__c approvedPolicyCall(String toolName) { + KernelTestToolPolicy.configuredDecision = AgentKernelModels.ToolAuthorizationDecision.RequireApproval; + Id supportId = AgentKernelTestFactory.approverUser(); + KernelTestToolPolicy.configuredApproverUserId = supportId; + AgentRun__c run = runPolicyCall(toolName); + AgentApproval__c approval = AgentKernelTestFactory.pendingApproval(run.Id); + System.runAs(new User(Id = supportId)) { + AgentKernel.decideApproval(AgentKernelTestFactory.approvalDecision(approval.Id, true, null)); + } + return AgentKernelTestFactory.getRun(run.Id); + } + + private static Map waitingPayload(Id runId) { + AgentEvent__c waiting; + for (AgentEvent__c event : AgentKernelTestFactory.getEvents(runId)) { + if (event.EventType__c == AgentKernelModels.EVENT_RUN_WAITING) { + waiting = event; + } + } + return (Map) JSON.deserializeUntyped(waiting.Payload__c); + } + + @IsTest + static void policyRequestBuildsAnswersAndReadsInputs() { + AgentKernelModels.ToolAuthorizationRequest request = new AgentKernelModels.ToolAuthorizationRequest(); + request.input = '{"amount":329.97,"count":3,"note":"text"}'; + System.assertEquals(329.97, request.decimalInput('amount')); + System.assertEquals(3, request.decimalInput('count')); + System.assertEquals(null, request.decimalInput('note'), 'Text is not a number.'); + System.assertEquals(null, request.decimalInput('missing')); + System.assertEquals('text', request.inputValue('note')); + + AgentKernelModels.ToolAuthorizationRequest malformed = new AgentKernelModels.ToolAuthorizationRequest(); + malformed.input = 'not json'; + System.assertEquals(null, malformed.decimalInput('amount'), 'Malformed input reads as missing, so a policy denies it.'); + + System.assertEquals(AgentKernelModels.ToolAuthorizationDecision.Allow, request.allow().decision); + AgentKernelModels.ToolAuthorizationResponse denied = request.deny('Too large.'); + System.assertEquals(AgentKernelModels.ToolAuthorizationDecision.Deny, denied.decision); + System.assertEquals('Too large.', denied.reason); + AgentKernelModels.ToolApprovalResponse held = request.requireApproval('Over 200.').byQueue('Finance').withTimeoutMinutes(10); + System.assertEquals(AgentKernelModels.ToolAuthorizationDecision.RequireApproval, held.decision); + System.assertEquals('Finance', held.approverQueue); + System.assertEquals(10, held.timeoutMinutes); + held.byUser(UserInfo.getUserId()); + System.assertEquals(UserInfo.getUserId(), held.approverUserId); + System.assertEquals(null, held.approverQueue, 'An answer names one approver; the last one named wins.'); + } + + private static AgentDefinition__c policyAgent() { + AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); + AgentKernelTestFactory.useDelegatedPrincipal(definition); + definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; + update definition; + return definition; + } + + /** Runs one call of a Decided by Policy tool on a new policy agent, and returns the run. */ + private static AgentRun__c runPolicyCall(String toolName) { + AgentDefinition__c definition = policyAgent(); + return runCall(definition, AgentKernelTestFactory.createPolicyTool(definition.Id, toolName)); + } + + /** Runs one call of the tool in a new run of the agent, and returns the run. */ + private static AgentRun__c runCall(AgentDefinition__c definition, AgentTool__c tool) { + AgentConfigService.clearCacheForTest(); + AgentKernelModels.StartResult started = AgentKernelTestFactory.startRunWithoutDispatch(definition); + AgentKernelTestFactory.acceptLoopback(started); + AgentKernelTestFactory.applyOutcome( + AgentKernelModels.continueWith( + started.runId, + started.commandId, + started.revision, + started.operationKey, + null, + new AgentKernelModels.CommandSpec(AgentKernelModels.COMMAND_TYPE_TOOL, '{}', tool.DeveloperName__c) + ) + ); + AgentRun__c run = AgentKernelTestFactory.getRun(started.runId); + AgentCommand__c command = lease(run.CurrentCommand__c, tool.DeveloperName__c + '-lease'); + System.runAs(AgentKernelTestFactory.worker(run.Id)) { + new ToolCommandQueueable(run.Id, command.Id, command.ExpectedRunRevision__c, command.OperationKey__c, command.LeaseToken__c).execute(null); + } + return AgentKernelTestFactory.getRun(run.Id); + } + @IsTest static void approvalRearmsSameToolOperation() { KernelTestTool.invocationCount = 0; KernelTestToolPolicy.configuredDecision = AgentKernelModels.ToolAuthorizationDecision.RequireApproval; Id approverId = AgentKernelTestFactory.approverUser(); - KernelTestToolPolicy.configuredApproverId = approverId; + KernelTestToolPolicy.configuredApproverUserId = approverId; AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); AgentKernelTestFactory.useDelegatedPrincipal(definition); definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; update definition; - AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'approved_tool', '{"type":"object"}'); + AgentTool__c tool = AgentKernelTestFactory.createPolicyTool(definition.Id, 'approved_tool'); tool.ImplementationConfiguration__c = '{"objectApiName":"Case"}'; AgentKernelTestFactory.updateRecord(tool); AgentKernelModels.StartResult started = AgentKernelTestFactory.startRunWithoutDispatch(definition); @@ -128,6 +312,7 @@ private class KernelV11LifecycleTest { System.assertEquals(command.Id, run.WaitingCommand__c); System.assertEquals(0, KernelTestTool.invocationCount); System.assertEquals(false, KernelTestToolPolicy.lastRequest.hasExternalEffects); + System.assertEquals([SELECT ToolType__c FROM AgentTool__c WHERE Id = :tool.Id].ToolType__c, KernelTestToolPolicy.lastRequest.toolType); System.assertEquals(run.ExecutionIdentity__c, KernelTestToolPolicy.lastRequest.executionIdentityId); System.assertEquals(run.InitiatingUser__c, KernelTestToolPolicy.lastRequest.initiatingActorId); System.assertEquals(run.ResolvedPrincipalUser__c, KernelTestToolPolicy.lastRequest.resolvedPrincipalId); @@ -165,10 +350,11 @@ private class KernelV11LifecycleTest { static void initiatingUserPolicyApprovalPausesInsteadOfWaiting() { KernelTestTool.invocationCount = 0; KernelTestToolPolicy.configuredDecision = AgentKernelModels.ToolAuthorizationDecision.RequireApproval; + KernelTestToolPolicy.configuredApproverUserId = AgentKernelTestFactory.approverUser(); AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; update definition; - AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'attended_tool', '{"type":"object"}'); + AgentTool__c tool = AgentKernelTestFactory.createPolicyTool(definition.Id, 'attended_tool'); AgentKernelModels.StartResult started = AgentKernelTestFactory.startRunWithoutDispatch(definition); AgentKernelTestFactory.applyOutcome( AgentKernelModels.continueWith( @@ -196,12 +382,12 @@ private class KernelV11LifecycleTest { KernelTestTool.invocationCount = 0; KernelTestToolPolicy.configuredDecision = AgentKernelModels.ToolAuthorizationDecision.RequireApproval; Id approverId = AgentKernelTestFactory.approverUser(); - KernelTestToolPolicy.configuredApproverId = approverId; + KernelTestToolPolicy.configuredApproverUserId = approverId; AgentDefinition__c definition = AgentKernelTestFactory.createDefinition(); AgentKernelTestFactory.useDelegatedPrincipal(definition); definition.ToolAuthorizationPolicyClass__c = 'KernelTestToolPolicy'; update definition; - AgentTool__c tool = AgentKernelTestFactory.createTool(definition.Id, 'changed_approved_tool', '{"type":"object"}'); + AgentTool__c tool = AgentKernelTestFactory.createPolicyTool(definition.Id, 'changed_approved_tool'); tool.ImplementationConfiguration__c = '{"objectApiName":"Case"}'; AgentKernelTestFactory.updateRecord(tool); AgentKernelModels.StartResult started = AgentKernelTestFactory.startRunWithoutDispatch(definition); diff --git a/force-app/main/default/classes/tests/ReferenceScenarioTest.cls b/force-app/main/default/classes/tests/ReferenceScenarioTest.cls index 726c9d3..191ec2a 100644 --- a/force-app/main/default/classes/tests/ReferenceScenarioTest.cls +++ b/force-app/main/default/classes/tests/ReferenceScenarioTest.cls @@ -451,10 +451,10 @@ private class ReferenceScenarioTest { '{"type":"object","properties":{"amount":{"type":"number"}},"required":["amount"],"additionalProperties":false}' ); refund.ToolType__c = AgentToolTypeTestFixtures.EXTERNAL; - refund.RequiresApproval__c = true; + refund.ApprovalMode__c = AgentKernelModels.APPROVAL_ALWAYS_REQUIRED; refund.ApproverType__c = AgentKernelModels.APPROVER_USER; refund.ApproverUser__c = fixture.approverId; - refund.ApprovalTimeoutHours__c = 24; + refund.ApprovalTimeoutMinutes__c = 1440; refund.ApprovalExpiryAction__c = AgentKernelModels.APPROVAL_EXPIRY_DENY; AgentKernelTestFactory.updateRecord(refund); AgentConfigService.clearCacheForTest(); diff --git a/force-app/main/default/layouts/AgentDefinition__c-Agent Definition Layout.layout-meta.xml b/force-app/main/default/layouts/AgentDefinition__c-Agent Definition Layout.layout-meta.xml index 66c6bdd..17dfd3b 100644 --- a/force-app/main/default/layouts/AgentDefinition__c-Agent Definition Layout.layout-meta.xml +++ b/force-app/main/default/layouts/AgentDefinition__c-Agent Definition Layout.layout-meta.xml @@ -125,7 +125,7 @@ DeveloperName__c ToolType__c ImplementationType__c - RequiresApproval__c + ApprovalMode__c IsActive__c AgentTool__c.AgentDefinition__c NAME diff --git a/force-app/main/default/layouts/AgentTool__c-Agent Tool Layout.layout-meta.xml b/force-app/main/default/layouts/AgentTool__c-Agent Tool Layout.layout-meta.xml index b5e6131..2fc1a8d 100644 --- a/force-app/main/default/layouts/AgentTool__c-Agent Tool Layout.layout-meta.xml +++ b/force-app/main/default/layouts/AgentTool__c-Agent Tool Layout.layout-meta.xml @@ -71,7 +71,11 @@ Edit - RequiresApproval__c + ApprovalMode__c + + + Edit + ApproverType__c Edit @@ -85,11 +89,7 @@ Edit - ApproverType__c - - - Edit - ApprovalTimeoutHours__c + ApprovalTimeoutMinutes__c Edit diff --git a/force-app/main/default/objects/AgentApproval__c/fields/ApproverType__c.field-meta.xml b/force-app/main/default/objects/AgentApproval__c/fields/ApproverType__c.field-meta.xml index 61261d9..4dd3a36 100644 --- a/force-app/main/default/objects/AgentApproval__c/fields/ApproverType__c.field-meta.xml +++ b/force-app/main/default/objects/AgentApproval__c/fields/ApproverType__c.field-meta.xml @@ -1,7 +1,7 @@ ApproverType__c - How Core chose the approver, copied from the tool or authorization policy when the approval was requested. + How Core chose the approver, copied from the tool when the approval was requested. When the authorization policy named the approver, this is User or Queue. User, Queue, Anchor Record Owner and Requester's Manager approvals are decided by the record owner or a direct queue member. Approval Process approvals are decided only through an installed integration that passes the approval process decision to Core. true diff --git a/force-app/main/default/objects/AgentTool__c/fields/ApprovalExpiryAction__c.field-meta.xml b/force-app/main/default/objects/AgentTool__c/fields/ApprovalExpiryAction__c.field-meta.xml index 0cbd75d..124d5fd 100644 --- a/force-app/main/default/objects/AgentTool__c/fields/ApprovalExpiryAction__c.field-meta.xml +++ b/force-app/main/default/objects/AgentTool__c/fields/ApprovalExpiryAction__c.field-meta.xml @@ -1,8 +1,8 @@ ApprovalExpiryAction__c - What Core does when nobody decides in time. The tool never runs after an expired approval. - Deny and Continue tells the agent the call was not approved and lets it continue. Pause for Operator pauses the run for an administrator. + What Core does when nobody decides in time. The tool never runs after an expired approval. Required when Approval is Always Required or Decided by Policy, and blank when it is Not Required. + Required when calls can wait for approval. Deny and Continue tells the agent the call was not approved and lets it continue. Pause for Operator pauses the run for an administrator. false false @@ -13,7 +13,7 @@ false Deny and Continue - true + false diff --git a/force-app/main/default/objects/AgentTool__c/fields/ApprovalMode__c.field-meta.xml b/force-app/main/default/objects/AgentTool__c/fields/ApprovalMode__c.field-meta.xml new file mode 100644 index 0000000..f9522d0 --- /dev/null +++ b/force-app/main/default/objects/AgentTool__c/fields/ApprovalMode__c.field-meta.xml @@ -0,0 +1,31 @@ + + + ApprovalMode__c + Whether a call of this tool waits for a person. Not Required runs every call, and the agent's authorization policy is not asked. Always Required waits for the Approver Type on every call; the policy is not asked. Decided by Policy asks the agent's authorization policy about each call: it runs the call, denies it, or holds it for an approver the policy names. Blank means Not Required. + Not Required: every call runs. Always Required: every call waits for the approver you choose below. Decided by Policy: the agent's Tool Authorization Policy Class decides each call, and names the approver when a call must wait. Only agents with an execution identity can wait for approval. + + false + false + Picklist + + true + + false + + Not Required + true + + + + Always Required + false + + + + Decided by Policy + false + + + + + diff --git a/force-app/main/default/objects/AgentTool__c/fields/ApprovalTimeoutHours__c.field-meta.xml b/force-app/main/default/objects/AgentTool__c/fields/ApprovalTimeoutHours__c.field-meta.xml deleted file mode 100644 index 8807e52..0000000 --- a/force-app/main/default/objects/AgentTool__c/fields/ApprovalTimeoutHours__c.field-meta.xml +++ /dev/null @@ -1,14 +0,0 @@ - - - ApprovalTimeoutHours__c - Hours an approval waits for a decision, from 1 to 168. Blank uses 24 hours. An authorization policy timeout overrides it. - false - Time allowed for an approval decision, in hours. Blank uses 24. When the deadline passes, Approval Expiry Action determines whether the agent continues without the call or pauses for an operator. - - 3 - false - 0 - false - Number - false - diff --git a/force-app/main/default/objects/AgentTool__c/fields/ApprovalTimeoutMinutes__c.field-meta.xml b/force-app/main/default/objects/AgentTool__c/fields/ApprovalTimeoutMinutes__c.field-meta.xml new file mode 100644 index 0000000..cf6cb92 --- /dev/null +++ b/force-app/main/default/objects/AgentTool__c/fields/ApprovalTimeoutMinutes__c.field-meta.xml @@ -0,0 +1,14 @@ + + + ApprovalTimeoutMinutes__c + Minutes an approval waits for a decision, from 1 to 10080 (7 days). Blank uses 1440 (24 hours). With Decided by Policy, a timeout the policy gives for a call replaces this one. Must be blank when Approval is Not Required. + false + Time allowed for an approval decision, in minutes. Blank uses 1440 (24 hours). The most is 10080 (7 days). With Decided by Policy, the policy may set a different deadline for a call. When the deadline passes, When Approval Expires decides what happens. + + 5 + false + 0 + false + Number + false + diff --git a/force-app/main/default/objects/AgentTool__c/fields/ApproverQueue__c.field-meta.xml b/force-app/main/default/objects/AgentTool__c/fields/ApproverQueue__c.field-meta.xml index c2e62fa..9e1c257 100644 --- a/force-app/main/default/objects/AgentTool__c/fields/ApproverQueue__c.field-meta.xml +++ b/force-app/main/default/objects/AgentTool__c/fields/ApproverQueue__c.field-meta.xml @@ -1,7 +1,7 @@ ApproverQueue__c - API name of the queue that owns this tool's approvals when Approver Type is Queue. With Anchor Record Owner, the fallback queue used when the owner cannot decide or the run has no anchor. The queue must support Agent Approval records. + API name of the queue that owns this tool's approvals when Approval is Always Required and Approver Type is Queue. With Anchor Record Owner, the fallback queue used when the owner cannot decide or the run has no anchor. The queue must support Agent Approval records. false Queue: this queue approves every call. Anchor Record Owner: optional backup; calls go here when the owner cannot decide or the run has no record, instead of pausing. Members need Agent Kernel Human Decision, and the queue must support Agent Approval. diff --git a/force-app/main/default/objects/AgentTool__c/fields/ApproverType__c.field-meta.xml b/force-app/main/default/objects/AgentTool__c/fields/ApproverType__c.field-meta.xml index 55d4f3e..8a6c5ec 100644 --- a/force-app/main/default/objects/AgentTool__c/fields/ApproverType__c.field-meta.xml +++ b/force-app/main/default/objects/AgentTool__c/fields/ApproverType__c.field-meta.xml @@ -1,8 +1,8 @@ ApproverType__c - Who decides this tool's approvals. Required when Always Require Approval is checked; also used when the authorization policy asks for approval without naming an approver. - Who approves this tool's calls: a chosen User or Queue, the Anchor Record Owner, or the starting user's manager. Required when Always Require Approval is checked. Also used when an authorization policy asks for approval without naming an approver. Approval Process requires an installed integration to pick reviewers and pass their decision to Core. Without one, the approval waits until it expires. + Who decides this tool's approvals when Approval is Always Required. Required then, and blank otherwise: with Decided by Policy, the policy names the approver for each call it holds. + Who approves every call when Approval is Always Required: a chosen User or Queue, the Anchor Record Owner, or the starting user's manager. Leave blank for Not Required and Decided by Policy. Approval Process requires an installed integration to pick reviewers and pass their decision to Core. Without one, the approval waits until it expires. false false diff --git a/force-app/main/default/objects/AgentTool__c/fields/RequiresApproval__c.field-meta.xml b/force-app/main/default/objects/AgentTool__c/fields/RequiresApproval__c.field-meta.xml deleted file mode 100644 index 3265002..0000000 --- a/force-app/main/default/objects/AgentTool__c/fields/RequiresApproval__c.field-meta.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - RequiresApproval__c - false - When checked, every call waits for a human decision before the tool runs. When unchecked, the tool runs unless the agent's authorization policy denies it or asks for approval. Execution Safety never adds approval. - Check to have the Approver Type approve every call before it runs. Unchecked, calls wait for approval only when the agent's authorization policy asks, and still go to the Approver Type. Only agents with an execution identity can use approvals. - - false - Checkbox - diff --git a/force-app/main/default/objects/AgentTool__c/listViews/All.listView-meta.xml b/force-app/main/default/objects/AgentTool__c/listViews/All.listView-meta.xml index d2ab230..20c68de 100644 --- a/force-app/main/default/objects/AgentTool__c/listViews/All.listView-meta.xml +++ b/force-app/main/default/objects/AgentTool__c/listViews/All.listView-meta.xml @@ -6,7 +6,7 @@ DeveloperName__c ToolType__c ImplementationType__c - RequiresApproval__c + ApprovalMode__c IsActive__c Everything diff --git a/force-app/main/default/permissionsets/AgentKernelAdmin.permissionset-meta.xml b/force-app/main/default/permissionsets/AgentKernelAdmin.permissionset-meta.xml index 73fb6a6..6cad7ca 100644 --- a/force-app/main/default/permissionsets/AgentKernelAdmin.permissionset-meta.xml +++ b/force-app/main/default/permissionsets/AgentKernelAdmin.permissionset-meta.xml @@ -910,7 +910,12 @@ true - AgentTool__c.ApprovalTimeoutHours__c + AgentTool__c.ApprovalMode__c + true + + + true + AgentTool__c.ApprovalTimeoutMinutes__c true @@ -953,11 +958,6 @@ AgentTool__c.IsActive__c true - - true - AgentTool__c.RequiresApproval__c - true - true AgentTool__c.SortOrder__c diff --git a/force-app/main/default/permissionsets/AgentKernelRunAgents.permissionset-meta.xml b/force-app/main/default/permissionsets/AgentKernelRunAgents.permissionset-meta.xml index 97d517d..03c528d 100644 --- a/force-app/main/default/permissionsets/AgentKernelRunAgents.permissionset-meta.xml +++ b/force-app/main/default/permissionsets/AgentKernelRunAgents.permissionset-meta.xml @@ -715,7 +715,12 @@ false - AgentTool__c.ApprovalTimeoutHours__c + AgentTool__c.ApprovalMode__c + true + + + false + AgentTool__c.ApprovalTimeoutMinutes__c true @@ -758,11 +763,6 @@ AgentTool__c.IsActive__c true - - false - AgentTool__c.RequiresApproval__c - true - false AgentTool__c.SortOrder__c diff --git a/force-app/main/default/permissionsets/AgentKernelServicePrincipal.permissionset-meta.xml b/force-app/main/default/permissionsets/AgentKernelServicePrincipal.permissionset-meta.xml index 30de095..642359f 100644 --- a/force-app/main/default/permissionsets/AgentKernelServicePrincipal.permissionset-meta.xml +++ b/force-app/main/default/permissionsets/AgentKernelServicePrincipal.permissionset-meta.xml @@ -163,7 +163,12 @@ false - AgentTool__c.ApprovalTimeoutHours__c + AgentTool__c.ApprovalMode__c + true + + + false + AgentTool__c.ApprovalTimeoutMinutes__c true @@ -206,11 +211,6 @@ AgentTool__c.IsActive__c true - - false - AgentTool__c.RequiresApproval__c - true - false AgentTool__c.SortOrder__c diff --git a/force-app/main/default/triggers/AgentDefinitionValidationTrigger.trigger b/force-app/main/default/triggers/AgentDefinitionValidationTrigger.trigger index 5c9be5a..543f9f2 100644 --- a/force-app/main/default/triggers/AgentDefinitionValidationTrigger.trigger +++ b/force-app/main/default/triggers/AgentDefinitionValidationTrigger.trigger @@ -14,4 +14,5 @@ trigger AgentDefinitionValidationTrigger on AgentDefinition__c(before insert, before update) { AgentDefinitionIdentityRules.validateDefinitions(Trigger.new, Trigger.isUpdate ? Trigger.oldMap : null); AgentModelService.validateDefinitions(Trigger.new, Trigger.isUpdate ? Trigger.oldMap : null); -} \ No newline at end of file + AgentApprovalService.validateDefinitions(Trigger.new, Trigger.isUpdate ? Trigger.oldMap : null); +} diff --git a/package.json b/package.json index d4dad6c..356db40 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "prettier": "prettier --write \"**/*.{cls,cmp,component,css,html,js,json,md,page,trigger,yaml,yml}\"", "prettier:verify": "prettier --check \"**/*.{cls,cmp,component,css,html,js,json,md,page,trigger,yaml,yml}\"", "lint": "eslint **/{aura,lwc}/**/*.js", + "prepare": "husky", "audit:field-documentation": "node scripts/check-field-documentation.js", "audit:dependencies": "npm audit --audit-level=moderate", "deploy:agent-core": "bash scripts/deploy-agent-core.sh", @@ -47,7 +48,8 @@ "set:api-version": "bash scripts/set-api-version.sh", "license:headers": "node scripts/license-headers.js", "license:check": "node scripts/license-headers.js --check", - "build:micromark": "rollup --config scripts/rollup.micromark.config.mjs" + "build:micromark": "rollup --config scripts/rollup.micromark.config.mjs", + "publish:public": "bash scripts/publish-public.sh" }, "devDependencies": { "@lwc/eslint-plugin-lwc": "^3.0.0", @@ -59,6 +61,7 @@ "eslint": "^9.28.0", "eslint-plugin-import": "^2.31.0", "eslint-plugin-jest": "^28.12.0", + "husky": "^9.1.7", "jest-canvas-mock": "^2.5.2", "prettier": "^3.5.3", "prettier-plugin-apex": "^2.2.6", diff --git a/pluto-console/main/default/classes/ToolAuthoringController.cls b/pluto-console/main/default/classes/ToolAuthoringController.cls index 2edd81a..27bfbcb 100644 --- a/pluto-console/main/default/classes/ToolAuthoringController.cls +++ b/pluto-console/main/default/classes/ToolAuthoringController.cls @@ -31,12 +31,12 @@ public with sharing class ToolAuthoringController { 'Description__c', 'IsActive__c', 'SortOrder__c', - 'RequiresApproval__c', + 'ApprovalMode__c', 'ApproverType__c', 'ApproverUser__c', 'ApproverQueue__c', 'ApprovalRoute__c', - 'ApprovalTimeoutHours__c', + 'ApprovalTimeoutMinutes__c', 'ApprovalExpiryAction__c', 'AllowedSensitiveDataCategories__c' }; @@ -327,7 +327,7 @@ public with sharing class ToolAuthoringController { } else if (EDITABLE_TOOL_FIELDS.contains(fieldName)) { tool.put(fieldName, supplied.get(fieldName)); } else { - throw handled('The tool draft contains a field that cannot be edited here.'); + throw handled('The tool draft contains a field that cannot be edited here: ' + fieldName + '. Refresh the page and try again.'); } } String type = blankToNull(toolType); diff --git a/pluto-console/main/default/classes/ToolTryController.cls b/pluto-console/main/default/classes/ToolTryController.cls index 7073772..58111f4 100644 --- a/pluto-console/main/default/classes/ToolTryController.cls +++ b/pluto-console/main/default/classes/ToolTryController.cls @@ -40,6 +40,7 @@ public with sharing class ToolTryController { /** The editor builds its form from this. */ @AuraEnabled public String inputSchema { get; set; } + /** Calls of this tool can wait for approval in a real run. */ @AuraEnabled public Boolean requiresApproval { get; set; } /** The tool acts on the record the agent starts from, so a dry run needs one. */ @@ -84,7 +85,7 @@ public with sharing class ToolTryController { AgentTool__c tool = loadTool(toolId); TryState state = new TryState(); state.toolName = tool.DeveloperName__c; - state.requiresApproval = tool.RequiresApproval__c == true; + state.requiresApproval = AgentApprovalService.mode(tool) != AgentKernelModels.APPROVAL_NOT_REQUIRED; state.reason = whyNot(tool); state.canTry = state.reason == null; // What the model would be sent now: built with the admin's edits, or hand-written. @@ -235,7 +236,7 @@ public with sharing class ToolTryController { } try { return [ - SELECT Id, AgentDefinition__c, DeveloperName__c, ToolType__c, ImplementationConfiguration__c, InputSchema__c, RequiresApproval__c + SELECT Id, AgentDefinition__c, DeveloperName__c, ToolType__c, ImplementationConfiguration__c, InputSchema__c, ApprovalMode__c FROM AgentTool__c WHERE Id = :toolId WITH USER_MODE diff --git a/pluto-console/main/default/classes/tests/ToolAuthoringControllerTest.cls b/pluto-console/main/default/classes/tests/ToolAuthoringControllerTest.cls index c68f211..aa2a03b 100644 --- a/pluto-console/main/default/classes/tests/ToolAuthoringControllerTest.cls +++ b/pluto-console/main/default/classes/tests/ToolAuthoringControllerTest.cls @@ -199,7 +199,10 @@ private class ToolAuthoringControllerTest { System.assertEquals('A complete tool draft is required.', refusal(null, 'UpdateRecord', null)); AgentTool__c sneaky = header(true); sneaky.InputSchema__c = HAND_WRITTEN; - System.assertEquals('The tool draft contains a field that cannot be edited here.', refusal(sneaky, 'UpdateRecord', null)); + System.assertEquals( + 'The tool draft contains a field that cannot be edited here: InputSchema__c. Refresh the page and try again.', + refusal(sneaky, 'UpdateRecord', null) + ); System.assertEquals('Choose a Tool Type.', refusal(header(true), ' ', null)); System.assertEquals('Settings must be a JSON object.', refusal(header(true), 'UpdateRecord', 'not json')); System.assertEquals('Settings must be a JSON object.', refusal(header(true), 'UpdateRecord', '["Case"]')); @@ -240,7 +243,6 @@ private class ToolAuthoringControllerTest { AgentDefinition__c = AgentKernelTestFactory.createDefinition().Id, DeveloperName__c = 'set_priority', Description__c = 'Sets a case priority.', - RequiresApproval__c = false, IsActive__c = active, SortOrder__c = 1 ); diff --git a/pluto-console/main/default/lwc/plutoRecordLayout/__tests__/shippedLayouts.test.js b/pluto-console/main/default/lwc/plutoRecordLayout/__tests__/shippedLayouts.test.js index 687b2a8..2155564 100644 --- a/pluto-console/main/default/lwc/plutoRecordLayout/__tests__/shippedLayouts.test.js +++ b/pluto-console/main/default/lwc/plutoRecordLayout/__tests__/shippedLayouts.test.js @@ -49,11 +49,11 @@ const LAYOUTS = [ 'DeveloperName__c', 'Description__c', 'AllowedSensitiveDataCategories__c', - 'RequiresApproval__c', + 'ApprovalMode__c', 'ApproverType__c', 'ApproverUser__c', 'ApproverQueue__c', - 'ApprovalTimeoutHours__c', + 'ApprovalTimeoutMinutes__c', 'ApprovalExpiryAction__c', 'ImplementationType__c', 'ToolType__c', diff --git a/pluto-console/main/default/lwc/plutoToolDetail/__tests__/plutoToolDetail.test.js b/pluto-console/main/default/lwc/plutoToolDetail/__tests__/plutoToolDetail.test.js index 5a67b4a..73b7bfe 100644 --- a/pluto-console/main/default/lwc/plutoToolDetail/__tests__/plutoToolDetail.test.js +++ b/pluto-console/main/default/lwc/plutoToolDetail/__tests__/plutoToolDetail.test.js @@ -94,22 +94,24 @@ describe('c-pluto-tool-detail', () => { const form = element.shadowRoot.querySelector('c-pluto-record-layout'); expect(form.hiddenFields).toEqual(['ToolType__c', 'ImplementationType__c', 'ImplementationConfiguration__c', 'InputSchema__c']); - expect(form.additionalFields).toEqual([{ fieldApiName: 'ApprovalRoute__c', before: 'ApprovalTimeoutHours__c' }]); + expect(form.additionalFields).toEqual([{ fieldApiName: 'ApprovalRoute__c', before: 'ApprovalTimeoutMinutes__c' }]); expect(form.fieldDependencies).toEqual([ - { controllingField: 'RequiresApproval__c', controllingValue: true, requiredFields: ['ApproverType__c'] }, + { controllingField: 'ApprovalMode__c', controllingValue: 'Always Required', showFields: ['ApproverType__c'], requiredFields: ['ApproverType__c'] }, + { + controllingField: 'ApprovalMode__c', + controllingValue: ['Always Required', 'Decided by Policy'], + showFields: ['ApprovalTimeoutMinutes__c', 'ApprovalExpiryAction__c'], + requiredFields: ['ApprovalExpiryAction__c'] + }, { controllingField: 'ApproverType__c', controllingValue: 'User', showFields: ['ApproverUser__c'], requiredFields: ['ApproverUser__c'] }, { controllingField: 'ApproverType__c', controllingValue: ['Queue', 'Anchor Record Owner'], showFields: ['ApproverQueue__c'] }, { controllingField: 'ApproverType__c', controllingValue: 'Queue', requiredFields: ['ApproverQueue__c'] }, - { - controllingField: 'ApproverType__c', - controllingValue: ['User', 'Queue', 'Anchor Record Owner', "Requester's Manager", 'Delivery Channel'], - showFields: ['ApprovalTimeoutHours__c', 'ApprovalExpiryAction__c'] - }, { controllingField: 'ApproverType__c', controllingValue: ['User', 'Queue', 'Anchor Record Owner', "Requester's Manager"], showFields: ['ApprovalRoute__c'] - } + }, + { controllingField: 'ApprovalMode__c', controllingValue: 'Decided by Policy', showFields: ['ApprovalRoute__c'] } ]); form.setFieldValue('ApproverUser__c', '005000000000000AAA'); @@ -153,17 +155,22 @@ describe('c-pluto-tool-detail', () => { await Promise.resolve(); expect(form.fieldOptions.ApproverQueue__c).toEqual([{ label: 'Queues unavailable', value: '' }]); }); - it('keeps the approver when Always Require Approval is cleared, for calls a policy sends for approval', async () => { + it('clears the settings each Approval choice does not use', async () => { const element = createElement('c-pluto-tool-detail', { is: PlutoToolDetail }); element.isNew = true; document.body.appendChild(element); await Promise.resolve(); const form = element.shadowRoot.querySelector('c-pluto-record-layout'); - const approvalFields = ['ApproverType__c', 'ApproverQueue__c', 'ApprovalRoute__c', 'ApprovalTimeoutHours__c', 'ApprovalExpiryAction__c']; - approvalFields.forEach((field) => form.setFieldValue(field, 'kept')); - form.dispatchEvent(new CustomEvent('fieldchange', { detail: { fieldName: 'RequiresApproval__c', value: false } })); - approvalFields.forEach((field) => expect(form.getFieldValue(field)).toBe('kept')); - expect(form.fieldDependencies.some((dependency) => dependency.hideFields?.includes('ApproverType__c'))).toBe(false); + const approverFields = ['ApproverType__c', 'ApproverUser__c', 'ApproverQueue__c']; + const waitFields = ['ApprovalTimeoutMinutes__c', 'ApprovalExpiryAction__c', 'ApprovalRoute__c']; + [...approverFields, ...waitFields].forEach((field) => form.setFieldValue(field, 'kept')); + + form.dispatchEvent(new CustomEvent('fieldchange', { detail: { fieldName: 'ApprovalMode__c', value: 'Decided by Policy' } })); + approverFields.forEach((field) => expect(form.getFieldValue(field)).toBeNull()); + waitFields.forEach((field) => expect(form.getFieldValue(field)).toBe('kept')); + + form.dispatchEvent(new CustomEvent('fieldchange', { detail: { fieldName: 'ApprovalMode__c', value: 'Not Required' } })); + waitFields.forEach((field) => expect(form.getFieldValue(field)).toBeNull()); expect(form.requiredFields).toContain('DeveloperName__c'); expect(element.shadowRoot.querySelector('c-tool-authoring-editor').parentElement.getAttribute('slot')).toBe('form-sections-after'); }); diff --git a/pluto-console/main/default/lwc/plutoToolDetail/plutoToolDetail.js b/pluto-console/main/default/lwc/plutoToolDetail/plutoToolDetail.js index cfbf407..efff982 100644 --- a/pluto-console/main/default/lwc/plutoToolDetail/plutoToolDetail.js +++ b/pluto-console/main/default/lwc/plutoToolDetail/plutoToolDetail.js @@ -19,17 +19,27 @@ import getApproverQueueOptions from '@salesforce/apex/PlutoImplementationDiscove // The embedded editor owns and shows the implementation type, tool type, settings, and input schema. const EDITOR_FIELDS = ['ToolType__c', 'ImplementationType__c', 'ImplementationConfiguration__c', 'InputSchema__c']; // Agent HITL adds Approval Route to the tool; it shows only where HITL is installed. -const PACKAGE_FIELDS = [{ fieldApiName: 'ApprovalRoute__c', before: 'ApprovalTimeoutHours__c' }]; +const PACKAGE_FIELDS = [{ fieldApiName: 'ApprovalRoute__c', before: 'ApprovalTimeoutMinutes__c' }]; -const APPROVER_TYPES = ['User', 'Queue', 'Anchor Record Owner', "Requester's Manager", 'Delivery Channel']; -// Approver Type stays visible without Requires Approval: an authorization policy -// may still ask for approval, and Core then uses the tool's approver. +const ALWAYS_REQUIRED = 'Always Required'; +const DECIDED_BY_POLICY = 'Decided by Policy'; +const CAN_WAIT = [ALWAYS_REQUIRED, DECIDED_BY_POLICY]; +const APPROVER_FIELDS = ['ApproverType__c', 'ApproverUser__c', 'ApproverQueue__c']; +const WAIT_FIELDS = ['ApprovalTimeoutMinutes__c', 'ApprovalExpiryAction__c', 'ApprovalRoute__c']; +// The tool names its approver only for Always Required; for Decided by Policy the policy names one. const FIELD_DEPENDENCIES = [ { - controllingField: 'RequiresApproval__c', - controllingValue: true, + controllingField: 'ApprovalMode__c', + controllingValue: ALWAYS_REQUIRED, + showFields: ['ApproverType__c'], requiredFields: ['ApproverType__c'] }, + { + controllingField: 'ApprovalMode__c', + controllingValue: CAN_WAIT, + showFields: ['ApprovalTimeoutMinutes__c', 'ApprovalExpiryAction__c'], + requiredFields: ['ApprovalExpiryAction__c'] + }, { controllingField: 'ApproverType__c', controllingValue: 'User', @@ -49,12 +59,12 @@ const FIELD_DEPENDENCIES = [ }, { controllingField: 'ApproverType__c', - controllingValue: APPROVER_TYPES, - showFields: ['ApprovalTimeoutHours__c', 'ApprovalExpiryAction__c'] + controllingValue: ['User', 'Queue', 'Anchor Record Owner', "Requester's Manager"], + showFields: ['ApprovalRoute__c'] }, { - controllingField: 'ApproverType__c', - controllingValue: ['User', 'Queue', 'Anchor Record Owner', "Requester's Manager"], + controllingField: 'ApprovalMode__c', + controllingValue: DECIDED_BY_POLICY, showFields: ['ApprovalRoute__c'] } ]; @@ -128,26 +138,29 @@ export default class PlutoToolDetail extends LightningElement { handleFieldChange(event) { const form = this.template.querySelector('c-pluto-record-layout'); - if (event.detail?.fieldName !== 'ApproverType__c') { - return; - } - const selectedType = event.detail.value; + const { fieldName, value } = event.detail || {}; const inactiveFields = []; - if (selectedType !== 'User') { - inactiveFields.push('ApproverUser__c'); - } - if (selectedType !== 'Queue' && selectedType !== 'Anchor Record Owner') { - inactiveFields.push('ApproverQueue__c'); - } - if (!selectedType || selectedType === 'Delivery Channel') { - inactiveFields.push('ApprovalRoute__c'); - } - if (!selectedType) { - inactiveFields.push('ApprovalTimeoutHours__c', 'ApprovalExpiryAction__c'); + if (fieldName === 'ApprovalMode__c') { + if (value !== ALWAYS_REQUIRED) { + inactiveFields.push(...APPROVER_FIELDS); + } + if (!CAN_WAIT.includes(value)) { + inactiveFields.push(...WAIT_FIELDS); + } + } else if (fieldName === 'ApproverType__c') { + if (value !== 'User') { + inactiveFields.push('ApproverUser__c'); + } + if (value !== 'Queue' && value !== 'Anchor Record Owner') { + inactiveFields.push('ApproverQueue__c'); + } + if (value === 'Delivery Channel') { + inactiveFields.push('ApprovalRoute__c'); + } } - inactiveFields.forEach((fieldName) => { - if (form?.getFieldValue(fieldName)) { - form.setFieldValue(fieldName, null); + inactiveFields.forEach((inactive) => { + if (form?.getFieldValue(inactive)) { + form.setFieldValue(inactive, null); } }); } diff --git a/pluto-console/main/default/lwc/toolTryModal/__tests__/toolTryModal.test.js b/pluto-console/main/default/lwc/toolTryModal/__tests__/toolTryModal.test.js index 0f924af..1011c1a 100644 --- a/pluto-console/main/default/lwc/toolTryModal/__tests__/toolTryModal.test.js +++ b/pluto-console/main/default/lwc/toolTryModal/__tests__/toolTryModal.test.js @@ -72,7 +72,7 @@ describe('c-tool-try-modal', () => { // The dry-run tagline is slotted into the header, which the Jest stub does not render. expect($(element, 'lightning-modal-header').label).toBe('Try create_case'); - expect($(element, '[data-approval-note]').textContent).toContain('skips it'); + expect($(element, '[data-approval-note]').textContent).toContain('skips approval'); expect($(element, '.form-section__heading').textContent).toBe('recordData'); const subject = $(element, '[data-path="recordData.Subject"]'); expect(subject.label).toBe('Subject'); diff --git a/pluto-console/main/default/lwc/toolTryModal/toolTryModal.html b/pluto-console/main/default/lwc/toolTryModal/toolTryModal.html index a8e090c..410f589 100644 --- a/pluto-console/main/default/lwc/toolTryModal/toolTryModal.html +++ b/pluto-console/main/default/lwc/toolTryModal/toolTryModal.html @@ -103,7 +103,9 @@

What the agent gets back