Skip to content

fix(training-agent): drop non-spec 'escalated' check_governance status#2354

Merged
bokelley merged 1 commit into
mainfrom
bokelley/drop-escalated-status
Apr 19, 2026
Merged

fix(training-agent): drop non-spec 'escalated' check_governance status#2354
bokelley merged 1 commit into
mainfrom
bokelley/drop-escalated-status

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

  • Narrow GovernanceCheckState.status to the AdCP v3 terminal set (approved | denied | conditions); remove the escalation field (no longer in the spec response).
  • Replace the shouldEscalate → status = 'escalated' branch in check_governance with humanReviewRequired → critical human_review finding → denied. Works identically for the two current triggers: plan.human_review_required and payloadBudget > plan.budget.reallocation_threshold.
  • Update buildCheckResponse to stop emitting the top-level escalation object (removed from check-governance-response.json).
  • Derive audit-log escalations[], drift_metrics.escalation_rate, and the spec-standard statuses.human_reviewed supplementary count from the human_review finding category.
  • Update tool description and unit tests.

Paired with the SDK cleanup in adcontextprotocol/adcp-client#592.
Partial fix for adcontextprotocol/adcp-client#589.

Why

static/schemas/source/governance/check-governance-response.json already validates status to the three-value enum, and the governance simplification changeset on this repo explicitly calls out dropping 'escalated'. The training agent was the lagging implementation: spec-compliant buyer validators reject its status: 'escalated' responses outright. Migrating to the critical human_review finding → denied shape aligns the training agent with the v3 spec without changing semantics — a human still has to approve before the buy proceeds; the signal just moves from a fourth status into the findings array.

Test plan

  • npm run typecheck clean
  • npx vitest run server/tests/unit/training-agent.test.ts — 323/323 pass
  • npx vitest run server/tests/unit/ — 1513/1513 pass (34 pre-existing skips)
  • End-to-end validation against the SDK governance e2e suite (requires the SDK PR to land first)

🤖 Generated with Claude Code

AdCP v3 governance has three terminal check_governance statuses
(approved | denied | conditions). The training agent was still emitting
a fourth 'escalated' status on human-review paths (human_review_required,
reallocation threshold exceeded), which is rejected by spec-compliant
buyer validators.

Human review is now signalled via a critical-severity human_review
finding on a denied decision. The buyer resolves review off-protocol and
re-calls check_governance with human_approval to proceed. The audit-log
summary keeps its escalations[] and escalation_rate — now derived from
checks that carry a human_review finding — and adds the spec-standard
statuses.human_reviewed supplementary count.

- GovernanceCheckState.status: drop 'escalated'; remove `escalation` field
- check_governance handler: replace shouldEscalate branch with
  humanReviewRequired → critical human_review finding → denied
- buildCheckResponse: no longer emits top-level escalation object
  (removed from check-governance-response.json schema)
- get_plan_audit_logs: derive escalations / escalation_rate /
  statuses.human_reviewed from the human_review finding category
- tool description + unit tests updated

Paired with SDK cleanup in adcontextprotocol/adcp-client#592.
Partial fix for adcontextprotocol/adcp-client#589.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 73958aa into main Apr 19, 2026
12 checks passed
@bokelley bokelley deleted the bokelley/drop-escalated-status branch April 19, 2026 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant