Skip to content

test(compliance): prove signal governance denial - #6590

Merged
bokelley merged 6 commits into
mainfrom
codex/signal-governance-denial
Aug 16, 2026
Merged

test(compliance): prove signal governance denial#6590
bokelley merged 6 commits into
mainfrom
codex/signal-governance-denial

Conversation

@bokelley

@bokelley bokelley commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Part of #3086.

Summary

  • gate the signal governance denial storyboard on the exact activate_signal task claim in adcp.governance_enforcement.tasks[]
  • exercise an explicit paid activation with no signed governance context
  • prove the denial caused no platform-primary outbound deployment call instead of grading a response echo
  • add a regression test and wire it into the full test command
  • consume @adcp/sdk@13.0.0-rc.25, which grades raw protocol evidence by default for schema-compliance get_products steps, and remove the seven per-step response_projection: raw workarounds

Validation

  • npm test (434 files, 6,144 tests passed; 30 skipped)
  • npm run build:compliance -- --check
  • current 3.2 storyboard matrix across all tenants
  • released 3.0.23 compatibility storyboard matrix across all tenants
  • pre-commit unit/server/typecheck suite
  • published SDK rc.25: media_buy_seller/canonical_formats (15 passed, 0 skipped, 0 N/A)

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Aug 15, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — clean additive change on an experimental compliance surface.

Checked:

  • Changeset: minor type is correct for a non-breaking additive change (adds paid activation action/pricing_option_id, tightens governance gating). Not breaking-class, so the human-ratification hard gate does not apply.
  • doc_ref fix: moves from the non-existent /signals/task-reference/* to real /signals/tasks/* pages — schema↔docs coherence improved, no drift introduced.
  • Test parity: new tests/signal-governance-storyboard.test.cjs mirrors the YAML storyboard.
  • No schema-source (static/schemas/source/**) edits, no dist artifact changes, no oneOf discriminator impact, no version hand-edit.
  • gated_paths: false; high_risk: false; no no-auto-approve team match.

Medium findings (2, non-blocking):

  • governance_denied.yaml:14 — object-valued requires_capability.contains matcher unexercised by any runner gate coverage.
  • governance_denied.yaml:141 — discovery step hard-requires optional pricing_options on positional signals[0].

Decision table: no critical/high findings (row 1 n/a); gated_paths false (row 2 n/a); no deletions (row 3 n/a); the two mediums are LLM/runner scope gaps, not data-loss/schema/infra categories (row 4 n/a); high_risk false (row 5 n/a); no prior escalation (row 6 n/a); no team gate (row 7 n/a); fewer than 3 mediums (row 8 n/a). Falls through to row 9 — approve.

Medium findings

  • static/compliance/source/specialisms/signal-marketplace/scenarios/governance_denied.yaml:14 — requires_capability uses object-valued contains untested by any runner gate coverage
  • static/compliance/source/specialisms/signal-marketplace/scenarios/governance_denied.yaml:141 — Discovery hard-requires optional pricing_options on positional signals[0]

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Aug 15, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — clean delta, no blocking or medium findings.

This PR adds an explicit paid-signal signal_spec plus a fields: [pricing_options] projection to the get_signals discovery step in the governance-denied storyboard scenario, with matching test assertions. The change is schema-valid (both fields and pricing_options are valid get-signals-request members) and directly grounds prior finding #2 — the field_present hard-require on signals[0].pricing_options[0].pricing_option_id is now justified by a request that explicitly asks for a paid marketplace signal with pricing.

Prior review (approve) flagged two mediums; both are addressed or out of the current delta:

  • Finding #2 (Discovery hard-requires optional pricing_options) — now grounded by the explicit paid-signal spec. Resolved.
  • Finding #1 (requires_capability object-valued contains) — outside this delta; not re-raised.

The .changeset/prove-signal-governance-denial.md accompanies the compliance-asset change, satisfying changeset scope. No schema-source (static/schemas/source/**) files modified, so no docs-coherence or oneOf concerns apply. package.json change is not a hand-edited version bump.

No high-risk paths, no gated paths, no author team gate, no findings. Decision table falls through to row 9.

Comment thread package.json

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Request changes

Request changes — 1 blocking finding.

Blocking findings

  • package.json:139 — Patcher removal is incomplete: the Dockerfile still invokes the deleted scripts/patch-sdk-rc15.mjs (via the removed patch:sdk-rc15/postinstall path), so the training-agent image build breaks. Reproducible runtime/build failure on a load-bearing path.

The PR otherwise cleanly bumps @adcp/sdk rc.21→rc.24 and retires the temporary storyboard patch (script, npm scripts, ledger entry, pre-push hook, patch-specific tests), relocating installed-SDK behavior tests and adding response_projection: raw to the get_products compatibility steps. Resolve the dangling Dockerfile reference to the deleted patcher and re-request review.

Note: this PR touches .agents/sdk-shim-ledger.json, a gated path, and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval is required before merge regardless of the fix.

Blocking findings

  • package.json:139 — Patcher removal is incomplete; Dockerfile still invokes the deleted scripts/patch-sdk-rc15.mjs, breaking the training-agent image build

@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Aug 16, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated paths touched without required approval.

This PR completes the coordinated removal of scripts/patch-sdk-rc15.mjs across the Dockerfile, release.yml, and two workflow trigger lists. The prior blocking finding (Dockerfile still invoking the deleted patcher) is resolved, and the reviewer found no dangling references and no new findings. On findings alone this would be a clean approve.

However, the PR modifies gated files under .agents/** and .github/workflows/** (gated_paths: true), and review_decision is CHANGES_REQUESTED, not APPROVED. Per the hard gated-paths rule (decision table row 2), the outcome cannot be approve — human/CODEOWNERS review is required to satisfy the gate.

Escalation reasons

  • Gated files require human/CODEOWNERS approval before merge; review_decision is CHANGES_REQUESTED.

Why human review

  • Modifies gated paths (.agents/sdk-shim-ledger.json, .github/workflows/migration-smoke-test.yml, release.yml, training-agent-storyboards.yml) while review_decision is CHANGES_REQUESTED — human/CODEOWNERS approval required before merge.
  • This PR touches a path under a hard, non-overridable approval gate (.agents/sdk-shim-ledger.json (modified) matches .agents/**; .github/workflows/migration-smoke-test.yml (modified) matches .github/workflows/**; .github/workflows/release.yml (modified) matches .github/workflows/**; .github/workflows/training-agent-storyboards.yml (modified) matches .github/workflows/**) and the current GitHub review decision is 'CHANGES_REQUESTED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@bokelley

Copy link
Copy Markdown
Contributor Author

@ohalushchak-exadel Ready for the gated-path human approval: all review threads are resolved, Ladon reports no remaining findings, every CI check is green, and the production Docker build was reproduced locally with the exact CI command. The gated changes only retire the now-upstreamed SDK patch and its stale workflow references.

@bokelley
bokelley dismissed aao-secretariat[bot]’s stale review August 16, 2026 05:04

The reported Dockerfile/workflow references were removed in addaa2c, the inline thread is resolved, and the exact Docker build plus full local gates pass.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated paths touched and review_decision is not APPROVED.

This PR modifies files under hard, non-overridable approval gates (.agents/** and .github/workflows/**) while the current GitHub review decision is REVIEW_REQUIRED, not APPROVED. Per decision rule row 2, this requires human/CODEOWNERS approval before merge regardless of how clean the diff is.

The reviewer found no critical/high/medium findings — the delta is a clean rc.24 → rc.25 SDK bump plus removal of the seven per-step response_projection: raw workarounds and their doc comment. Version and integrity hashes move in lockstep, no wire-shape or schema drift, and a changeset is present. The diff itself is mechanically sound, but the path-based gate stands until a human approval is recorded.

Escalation reasons

  • Modifies gated paths (.agents/sdk-shim-ledger.json, .github/workflows/migration-smoke-test.yml, .github/workflows/release.yml, .github/workflows/training-agent-storyboards.yml) while review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required before merge.

Why human review

  • Touches gated paths (.agents/sdk-shim-ledger.json, .github/workflows/migration-smoke-test.yml, .github/workflows/release.yml, .github/workflows/training-agent-storyboards.yml) under a hard approval gate while review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required before merge.
  • This PR touches a path under a hard, non-overridable approval gate (.agents/sdk-shim-ledger.json (modified) matches .agents/**; .github/workflows/migration-smoke-test.yml (modified) matches .github/workflows/**; .github/workflows/release.yml (modified) matches .github/workflows/**; .github/workflows/training-agent-storyboards.yml (modified) matches .github/workflows/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@bokelley

Copy link
Copy Markdown
Contributor Author

@ohalushchak-exadel Final head 9b6ddf29ed is ready for the gated-path approval. The SDK behavior is now upstream in adcontextprotocol/adcp-client#2549 and published as @adcp/sdk@13.0.0-rc.25; this PR removes the seven temporary response_projection: raw annotations. All 44 CI checks are green, Ladon reports no findings, every review thread is resolved, and auto-merge is armed.

@bokelley
bokelley disabled auto-merge August 16, 2026 05:11
@bokelley
bokelley merged commit 1760c31 into main Aug 16, 2026
45 checks passed
@bokelley
bokelley deleted the codex/signal-governance-denial branch August 16, 2026 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant