diff --git a/.changeset/governance-drop-escalated-status.md b/.changeset/governance-drop-escalated-status.md new file mode 100644 index 000000000..1400c216c --- /dev/null +++ b/.changeset/governance-drop-escalated-status.md @@ -0,0 +1,21 @@ +--- +'@adcp/client': minor +--- + +Governance: remove non-spec `'escalated'` status to align with AdCP v3. + +AdCP v3 governance has three terminal `check_governance` statuses: `'approved' | 'denied' | 'conditions'`. `CheckGovernanceResponseSchema` already validates to this set, but the SDK core carried `'escalated'` as a fourth status from a pre-v3 model. Spec-compliant governance agents cannot emit it, so the code path was dead under validation and misrepresented the protocol to consumers branching on `govResult.status`. + +Human review is modelled in v3 as a workflow on `denied` (governance agent denies with a critical-severity finding that says human review is required; the buyer resolves review off-protocol and calls `check_governance` again with the human's approval), not as a fourth terminal status. + +Changes: +- `GovernanceCheckResult.status` narrows to `'approved' | 'denied' | 'conditions'`. +- `TaskStatus` drops `'governance-escalated'`; failing governance checks surface as `'governance-denied'`. +- `TaskResultFailure.status` narrows to `'failed' | 'governance-denied'`. +- `GovernanceMiddleware` drops the `'escalated'` branch in `checkProposed`. +- `TaskExecutor.buildGovernanceResult` signature no longer takes a status parameter. +- Test-scenario validator for `check_governance` rejects `'escalated'` as an unexpected status. + +Migration: if you branch on `result.status === 'governance-escalated'` or `govResult.status === 'escalated'`, fold those branches into the `'governance-denied'` / `'denied'` paths. Inspect `governance.findings` for human-review signals if you need to distinguish the reason. + +Fixes #589. diff --git a/docs/TYPE-SUMMARY.md b/docs/TYPE-SUMMARY.md index 0bdf56071..83a0bae2b 100644 --- a/docs/TYPE-SUMMARY.md +++ b/docs/TYPE-SUMMARY.md @@ -21,7 +21,7 @@ interface AgentConfig { interface TaskResult { success: boolean; status: 'completed' | 'deferred' | 'submitted' | 'input-required' - | 'working' | 'governance-denied' | 'governance-escalated'; + | 'working' | 'governance-denied'; data?: T; error?: string; deferred?: DeferredContinuation; diff --git a/docs/getting-started.md b/docs/getting-started.md index e3e94a1b5..212559454 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -204,7 +204,7 @@ if (result.success && result.status === 'completed') { } if (!result.success) { result.error // string (always present) - result.status // 'failed' | 'governance-denied' | 'governance-escalated' + result.status // 'failed' | 'governance-denied' } ``` diff --git a/docs/guides/TASKRESULT-5-MIGRATION.md b/docs/guides/TASKRESULT-5-MIGRATION.md index fc901c137..fcbb9cfa3 100644 --- a/docs/guides/TASKRESULT-5-MIGRATION.md +++ b/docs/guides/TASKRESULT-5-MIGRATION.md @@ -13,7 +13,7 @@ This page covers the four migration patterns every consumer hits. type TaskResult = | { success: true; status: 'completed'; data: T } | { success: true; status: 'working' | 'submitted' | 'input-required' | 'deferred'; data?: T } - | { success: false; status: 'failed' | 'governance-denied' | 'governance-escalated'; data?: T; + | { success: false; status: 'failed' | 'governance-denied'; data?: T; error: string; adcpError?: { code: string; recovery?: string; retryAfterMs?: number; /* ... */ }; correlationId?: string }; @@ -113,9 +113,6 @@ if (!result.success) { case 'governance-denied': notifyApprover(result.adcpError); break; - case 'governance-escalated': - trackPending(result.correlationId); - break; } return; } @@ -156,7 +153,6 @@ switch (result.status) { case 'failed': case 'governance-denied': - case 'governance-escalated': // see Pattern 2 return; } @@ -182,7 +178,6 @@ switch (result.status) { case 'deferred': ... case 'failed': ... case 'governance-denied': ... - case 'governance-escalated': ... default: assertNever(result); // TS errors if you miss a status } ``` diff --git a/docs/llms.txt b/docs/llms.txt index 38308962b..455f87c1c 100644 --- a/docs/llms.txt +++ b/docs/llms.txt @@ -798,7 +798,7 @@ Every tool call returns a `TaskResult` with one of these statuses: - `submitted` — Long-running. Poll via `result.submitted.waitForCompletion()` or use webhooks. - `working` — In progress (intermediate, usually not seen by callers). - `deferred` — Requires human decision. Token in `result.deferred.token`. -- `governance-denied` / `governance-escalated` — Blocked or flagged by governance middleware. +- `governance-denied` — Blocked by governance middleware. **Deep dive:** docs/guides/ASYNC-DEVELOPER-GUIDE.md, docs/guides/ASYNC-API-REFERENCE.md diff --git a/scripts/generate-agent-docs.ts b/scripts/generate-agent-docs.ts index 4c9238983..a00511f70 100644 --- a/scripts/generate-agent-docs.ts +++ b/scripts/generate-agent-docs.ts @@ -744,7 +744,7 @@ function generateLlmsTxt( ln(`- \`submitted\` — Long-running. Poll via \`result.submitted.waitForCompletion()\` or use webhooks.`); ln(`- \`working\` — In progress (intermediate, usually not seen by callers).`); ln(`- \`deferred\` — Requires human decision. Token in \`result.deferred.token\`.`); - ln(`- \`governance-denied\` / \`governance-escalated\` — Blocked or flagged by governance middleware.`); + ln(`- \`governance-denied\` — Blocked by governance middleware.`); ln(); ln(`**Deep dive:** docs/guides/ASYNC-DEVELOPER-GUIDE.md, docs/guides/ASYNC-API-REFERENCE.md`); ln(); @@ -848,7 +848,7 @@ function generateTypeSummary(index: SchemaIndex, tools: ToolInfo[]): string { ln(`interface TaskResult {`); ln(` success: boolean;`); ln(` status: 'completed' | 'deferred' | 'submitted' | 'input-required'`); - ln(` | 'working' | 'governance-denied' | 'governance-escalated';`); + ln(` | 'working' | 'governance-denied';`); ln(` data?: T;`); ln(` error?: string;`); ln(` deferred?: DeferredContinuation;`); diff --git a/src/lib/core/ConversationTypes.ts b/src/lib/core/ConversationTypes.ts index a976bb341..63072f8aa 100644 --- a/src/lib/core/ConversationTypes.ts +++ b/src/lib/core/ConversationTypes.ts @@ -115,8 +115,7 @@ export type TaskStatus = | 'deferred' | 'aborted' | 'submitted' - | 'governance-denied' - | 'governance-escalated'; + | 'governance-denied'; /** * Options for task execution @@ -324,7 +323,7 @@ export interface TaskResultIntermediate extends TaskResultBase { /** Task failed — `error` is always present. */ export interface TaskResultFailure extends TaskResultBase { success: false; - status: 'failed' | 'governance-denied' | 'governance-escalated'; + status: 'failed' | 'governance-denied'; /** Response payload with structured error details (adcp_error, context, ext) */ data?: T; /** Human-readable error message */ diff --git a/src/lib/core/GovernanceMiddleware.ts b/src/lib/core/GovernanceMiddleware.ts index 432bdba25..c90e44fbd 100644 --- a/src/lib/core/GovernanceMiddleware.ts +++ b/src/lib/core/GovernanceMiddleware.ts @@ -6,7 +6,6 @@ * - approved: proceed with execution * - denied: return denial to caller * - conditions: auto-apply machine-actionable conditions and re-check - * - escalated: return escalation continuation to caller * * After execution, reports the outcome back to the governance agent. */ @@ -152,7 +151,6 @@ export class GovernanceMiddleware { * Returns the governance result. The caller decides how to handle each status: * - approved: proceed with execution (params may be modified by conditions) * - denied: do not execute - * - escalated: do not execute, return continuation to caller * * When conditions are returned with required_value, this method auto-applies * them and re-checks, up to maxConditionsIterations. @@ -217,7 +215,7 @@ export class GovernanceMiddleware { return { result: checkResult, params: currentParams }; } - if (checkResult.status === 'denied' || checkResult.status === 'escalated') { + if (checkResult.status === 'denied') { return { result: checkResult, params: currentParams }; } diff --git a/src/lib/core/GovernanceTypes.ts b/src/lib/core/GovernanceTypes.ts index a1c0ce117..35e9e5f73 100644 --- a/src/lib/core/GovernanceTypes.ts +++ b/src/lib/core/GovernanceTypes.ts @@ -111,7 +111,7 @@ export interface GovernanceEscalation { */ export interface GovernanceCheckResult { checkId: string; - status: 'approved' | 'denied' | 'conditions' | 'escalated'; + status: 'approved' | 'denied' | 'conditions'; explanation: string; findings?: GovernanceFinding[]; conditions?: GovernanceCondition[]; diff --git a/src/lib/core/TaskExecutor.ts b/src/lib/core/TaskExecutor.ts index 0bc03561f..8af84ed58 100644 --- a/src/lib/core/TaskExecutor.ts +++ b/src/lib/core/TaskExecutor.ts @@ -297,39 +297,11 @@ export class TaskExecutor { const isBlocking = true; if (govResult.status === 'denied' && isBlocking) { - return this.buildGovernanceResult( - 'governance-denied', - govResult, - taskId, - taskName, - agent, - startTime, - debugLogs - ); - } - - if (govResult.status === 'escalated' && isBlocking) { - return this.buildGovernanceResult( - 'governance-escalated', - govResult, - taskId, - taskName, - agent, - startTime, - debugLogs - ); + return this.buildGovernanceResult(govResult, taskId, taskName, agent, startTime, debugLogs); } if (govResult.status === 'conditions' && !govResult.conditionsApplied && isBlocking) { - return this.buildGovernanceResult( - 'governance-denied', - govResult, - taskId, - taskName, - agent, - startTime, - debugLogs - ); + return this.buildGovernanceResult(govResult, taskId, taskName, agent, startTime, debugLogs); } // Approved, or non-blocking mode (advisory/audit) allows execution to proceed @@ -460,7 +432,6 @@ export class TaskExecutor { * Handle agent response based on ADCP status (PR #78) */ private buildGovernanceResult( - status: 'governance-denied' | 'governance-escalated', govResult: GovernanceCheckResult, taskId: string, taskName: string, @@ -470,10 +441,10 @@ export class TaskExecutor { ): TaskResult { return { success: false, - status, - error: govResult.explanation || `Governance ${status}`, + status: 'governance-denied', + error: govResult.explanation || 'Governance governance-denied', governance: govResult, - metadata: this.buildMetadata({ taskId, taskName, agent, startTime, status }), + metadata: this.buildMetadata({ taskId, taskName, agent, startTime, status: 'governance-denied' }), conversation: [], debug_logs: debugLogs, }; diff --git a/src/lib/testing/scenarios/governance.ts b/src/lib/testing/scenarios/governance.ts index ac4a43d13..53ed0755c 100644 --- a/src/lib/testing/scenarios/governance.ts +++ b/src/lib/testing/scenarios/governance.ts @@ -938,8 +938,8 @@ export async function testCampaignGovernance( ); step.observation_data = { governance_context: data.governance_context || null }; - // Any status is valid — we're testing the protocol, not the policy - if (!['approved', 'denied', 'conditions', 'escalated'].includes(status)) { + // Only spec-valid statuses are accepted — we're testing the protocol, not the policy + if (!['approved', 'denied', 'conditions'].includes(status)) { step.passed = false; step.error = `Unexpected governance status: ${status}`; } diff --git a/test/lib/governance-e2e.test.js b/test/lib/governance-e2e.test.js index 30c4395dd..2969a61ef 100644 --- a/test/lib/governance-e2e.test.js +++ b/test/lib/governance-e2e.test.js @@ -27,7 +27,7 @@ try { process.execPath, [ '-e', - `fetch('${AGENT_URL}',{method:'POST',headers:{'Content-Type':'application/json'},body:'{"jsonrpc":"2.0","method":"ping","id":0}',signal:AbortSignal.timeout(2000)}).then(r=>process.exit(r.ok||r.status===400?0:1)).catch(()=>process.exit(1))`, + `fetch('${AGENT_URL}',{method:'POST',headers:{'Content-Type':'application/json','Accept':'application/json, text/event-stream'},body:'{"jsonrpc":"2.0","method":"ping","id":0}',signal:AbortSignal.timeout(2000)}).then(r=>process.exit(r.ok||r.status===400?0:1)).catch(()=>process.exit(1))`, ], { timeout: 5000, stdio: 'ignore' } ); @@ -446,9 +446,9 @@ describe('Governance E2E: Capabilities discovery', { skip: skipReason }, () => { }); }); -describe('Governance E2E: Escalation flow', { skip: skipReason }, () => { - const planId = `e2e-escalation-${Date.now()}`; - const campaignRef = `e2e-escalation-campaign-${Date.now()}`; +describe('Governance E2E: Human-review denial', { skip: skipReason }, () => { + const planId = `e2e-human-review-${Date.now()}`; + const campaignRef = `e2e-human-review-campaign-${Date.now()}`; let client; let governanceAgent; @@ -462,11 +462,12 @@ describe('Governance E2E: Escalation flow', { skip: skipReason }, () => { { plan_id: planId, brand: { domain: 'test.example' }, - objectives: 'Escalation flow test', + objectives: 'Human-review denial test', budget: { total: 10000, currency: 'USD', authority_level: 'human_required', + reallocation_threshold: 5000, }, flight: { start: new Date().toISOString(), @@ -486,8 +487,10 @@ describe('Governance E2E: Escalation flow', { skip: skipReason }, () => { assert.ok(syncResult.success, `syncPlans failed: ${syncResult.error}`); }); - it('should escalate when budget exceeds 50% with human_required authority', async () => { - // Budget of $6000 > 50% of $10000 plan with human_required authority + // AdCP v3 has three terminal statuses (approved|denied|conditions). Human review is + // signalled via a critical-severity finding on a denied decision; the buyer resolves + // review off-protocol and calls check_governance again with the human's approval. + it('should deny with critical-severity finding when budget exceeds 50% under human_required authority', async () => { const result = await client.executeTask('check_governance', { plan_id: planId, buyer_campaign_ref: campaignRef, @@ -503,10 +506,12 @@ describe('Governance E2E: Escalation flow', { skip: skipReason }, () => { assert.ok(result.success, `check_governance failed: ${result.error}`); const data = result.data; - assert.equal(data.status, 'escalated', 'Expected escalated status'); - assert.ok(data.escalation, 'Expected escalation details'); - assert.equal(data.escalation.requires_human, true, 'Expected requires_human'); - assert.ok(data.escalation.reason, 'Expected escalation reason'); + assert.equal(data.status, 'denied', 'Expected denied status'); + assert.ok(Array.isArray(data.findings) && data.findings.length > 0, 'Expected findings'); + assert.ok( + data.findings.some(f => f.severity === 'critical'), + 'Expected at least one critical-severity finding' + ); }); it('should approve small budget with human_required authority (under 50%)', async () => {