fix(decisioning): Tier 2 expert-review fix-pack#372
Merged
Conversation
5 tasks
bokelley
added a commit
that referenced
this pull request
May 3, 2026
) (#393) * fix(decisioning): rename Tier 2 codes to spec-conformant PERMISSION_DENIED + parity (#375) The Tier 2 commercial-identity gate previously raised four error codes absent from the AdCP spec's `error-code.json` 51-entry vocabulary: `AGENT_SUSPENDED`, `AGENT_BLOCKED`, `REQUEST_AUTH_UNRECOGNIZED_AGENT`, `INVALID_BILLING_MODEL`. The cross-tenant onboarding-oracle clamp in the spec requires the unrecognized-agent path and the recognized-but- denied path to be observably indistinguishable to an external attacker — distinct codes per status leak which `agent_url`s are onboarded with which sellers, enabling enumeration of commercial relationships. Code changes: * `_resolve_buyer_agent` raises `PERMISSION_DENIED` on all four denial paths. Recognized-but-denied paths (suspended / blocked) carry `details.scope="agent"` + `details.status`; unrecognized paths (registry miss, no credential, unknown status) OMIT `details` per the spec's omit-on-unestablished-identity rule. * All four paths share a single `_denied_message` constant so `error.message` is not a side channel. * `validate_billing_for_agent` raises `BILLING_NOT_PERMITTED_FOR_AGENT` with `details.rejected_billing` (required) and an optional `details.suggested_billing` (the alphabetically-first permitted mode). The full `permitted_billing` subset is no longer leaked — surfacing it on every rejection let a misconfigured buyer probe and exfiltrate the billing matrix. Recovery semantic: `PERMISSION_DENIED` is treated as `terminal` for the commercial-identity gate (resolution path is operator-onboarding, not request-side correction). This overrides the spec's `enumMetadata` default of `correctable` for the code; the override is documented in `_resolve_buyer_agent`'s docstring. Spec status of `BILLING_NOT_PERMITTED_FOR_AGENT`: this code is not in the 51-entry standard enum and lacks the `X_` vendor prefix required by `vendor-error-codes.json`. The user-facing issue (#375) specifies this code; raising the spec conflict in the PR body for sign-off. Tests: * `tests/test_tier2_spec_conformance.py` — pins the wire shape: - All four removed codes are no longer raised. - Recognized-but-denied paths carry `scope` + `status` in details. - Unrecognized paths omit `details`. - Billing path carries `rejected_billing` + optional `suggested_billing`; never leaks `permitted_billing` or `agent_url`. * Existing `tests/test_decisioning_buyer_agent_dispatch.py` and `tests/test_buyer_agent_registry.py` updated to the new wire shape. Deferred to follow-up: The latency / headers / side-effects / observability parity contract between the four denial paths is a larger dispatch-path refactor (single emit point, deliberate latency padding, identical audit / metric side-effects) that does not fit in this PR. Tracking issue to follow. The eager-raise pattern in `_resolve_buyer_agent` still completes the unrecognized path on a different code path than the recognized one — this PR closes the wire-code mismatch only. Backwards compatibility: this is a behavior change for adopters who match on the old codes. The codes were just introduced in PRs #364 / #372, so blast radius is small. Adopters need to migrate to matching on `code == "PERMISSION_DENIED"` + reading `details.scope` and `details.status` to discriminate recognized-but-denied paths. Closes #375 (rename portion). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(decisioning): recovery=correctable per spec enumMetadata (PR #393 fix-pack) Flip wire-level recovery from terminal to correctable on the four PERMISSION_DENIED denial paths in _resolve_buyer_agent (handler.py) and on BILLING_NOT_PERMITTED_FOR_AGENT in validate_billing_for_agent (registry.py) to match the spec's enumMetadata defaults. The details.scope == "agent" discriminator (when present) is the signal callers surface to a human operator rather than auto-retry — that semantic stays in details, not in the recovery hint. Also annotates docs/proposals/v3-identity-bundle-design.md with a status note pointing readers to the test suite for current behavior; the proposal still uses the pre-rename code names but is preserved as a historical doc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix-pack from the 4-expert holistic review of the merged Tier 2 v3-identity bundle, plus the IndeterminateDatatype SQL fix that was lost during the #361 rebase.
Eight items addressed:
Plus: `PgTaskRegistry._sql_get` uses `%s::text IS NULL` to fix IndeterminateDatatype on cross-tenant probes (was lost during the #361 rebase).
Not in this PR (validated as non-issues):
Test plan
🤖 Generated with Claude Code