From 015731f05a284ecec24eda5c34ae8524647ceed8 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sat, 18 Apr 2026 19:47:22 -0400 Subject: [PATCH 1/3] feat: drop non-spec 'escalated' governance status (#589) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 — dead under validation and misleading to consumers branching on govResult.status. Human review is now signalled via a critical-severity finding on a denied decision; the buyer resolves review off-protocol and re-calls check_governance with the human's approval. - GovernanceCheckResult.status: drop 'escalated' - TaskStatus: drop 'governance-escalated' - TaskResultFailure.status: narrows to 'failed' | 'governance-denied' - GovernanceMiddleware.checkProposed: drop 'escalated' branch - TaskExecutor.buildGovernanceResult: signature no longer takes a status - governance test-scenario validator: drop 'escalated' - governance-e2e: rewrite escalation test for denied + critical finding Paired with training-agent update in adcontextprotocol/adcp. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../governance-drop-escalated-status.md | 21 ++++++++++++++++ docs/TYPE-SUMMARY.md | 2 +- docs/getting-started.md | 2 +- docs/guides/TASKRESULT-5-MIGRATION.md | 7 +----- docs/llms.txt | 2 +- scripts/generate-agent-docs.ts | 4 ++-- src/lib/core/ConversationTypes.ts | 5 ++-- src/lib/core/GovernanceMiddleware.ts | 4 +--- src/lib/core/GovernanceTypes.ts | 2 +- src/lib/core/TaskExecutor.ts | 21 +++------------- src/lib/testing/scenarios/governance.ts | 4 ++-- test/lib/governance-e2e.test.js | 24 +++++++++++-------- 12 files changed, 50 insertions(+), 48 deletions(-) create mode 100644 .changeset/governance-drop-escalated-status.md 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 0f506b0ac..b652ee177 100644 --- a/docs/llms.txt +++ b/docs/llms.txt @@ -756,7 +756,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 305280c83..e5abba979 100644 --- a/scripts/generate-agent-docs.ts +++ b/scripts/generate-agent-docs.ts @@ -694,7 +694,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(); @@ -798,7 +798,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 fd8e9f983..1db67bece 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 @@ -302,7 +301,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 3100df553..5a3d1fe4c 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. */ @@ -107,7 +106,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. @@ -172,7 +170,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 744bf6704..f20b41da4 100644 --- a/src/lib/core/TaskExecutor.ts +++ b/src/lib/core/TaskExecutor.ts @@ -237,19 +237,6 @@ export class TaskExecutor { 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, @@ -261,7 +248,6 @@ export class TaskExecutor { if (govResult.status === 'conditions' && !govResult.conditionsApplied && isBlocking) { return this.buildGovernanceResult( - 'governance-denied', govResult, taskId, taskName, @@ -399,7 +385,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, @@ -409,8 +394,8 @@ 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: { taskId, @@ -419,7 +404,7 @@ export class TaskExecutor { responseTimeMs: Date.now() - startTime, timestamp: new Date().toISOString(), clarificationRounds: 0, - status, + 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..2915b33ed 100644 --- a/test/lib/governance-e2e.test.js +++ b/test/lib/governance-e2e.test.js @@ -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,7 +462,7 @@ 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', @@ -486,8 +486,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 +505,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 () => { From d44fafed6720659e41e73ae12d8cd5395b6f8b86 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sat, 18 Apr 2026 20:11:47 -0400 Subject: [PATCH 2/3] style: prettier format TaskExecutor governance branches Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/core/TaskExecutor.ts | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/src/lib/core/TaskExecutor.ts b/src/lib/core/TaskExecutor.ts index f20b41da4..fefa3e32f 100644 --- a/src/lib/core/TaskExecutor.ts +++ b/src/lib/core/TaskExecutor.ts @@ -236,25 +236,11 @@ export class TaskExecutor { const isBlocking = true; if (govResult.status === 'denied' && isBlocking) { - return this.buildGovernanceResult( - 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( - 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 From f2e568496b916e8f8d1255c274144e1333357672 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sat, 18 Apr 2026 20:20:18 -0400 Subject: [PATCH 3/3] test(governance-e2e): fix connectivity check + plan fixture for new test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small test-runner fixes, neither behavioral: 1. Connectivity probe sends `Accept: application/json, text/event-stream` so the MCP StreamableHTTP transport returns 200 instead of 406. The existing skip-if-unreachable logic was masking every e2e run because of this header mismatch. 2. The new human-review denial test's plan includes `reallocation_threshold` so syncPlans passes the training agent's Art 22 / Annex III invariant (introduced upstream in adcp#2310/#2338). Verified locally: new test passes against the updated training agent post-adcp#2354. Other e2e suites still fail for the same reallocation_threshold reason against a fresh training agent — that's a broader test-fixture drift worth a follow-up PR. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/lib/governance-e2e.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/lib/governance-e2e.test.js b/test/lib/governance-e2e.test.js index 2915b33ed..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' } ); @@ -467,6 +467,7 @@ describe('Governance E2E: Human-review denial', { skip: skipReason }, () => { total: 10000, currency: 'USD', authority_level: 'human_required', + reallocation_threshold: 5000, }, flight: { start: new Date().toISOString(),