Skip to content

feat(conformance): strengthen 3.2 reference agent coverage - #6883

Open
bokelley wants to merge 12 commits into
mainfrom
polish-3-2-agent-stories
Open

feat(conformance): strengthen 3.2 reference agent coverage#6883
bokelley wants to merge 12 commits into
mainfrom
polish-3-2-agent-stories

Conversation

@bokelley

@bokelley bokelley commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • make the 3.2 reference sales agent execute targeting-aware discovery and advertise the geo overlay its default products support
  • implement advanced delivery reporting and audience-activation discovery, with capability-gated end-to-end storyboards and ratcheted CI floors
  • upgrade the reference agent to @adcp/sdk@14.0.0-beta.12, regression-gate its account-scoping fixes, and enable functional storyboard request signing
  • bound bundled creative validators so generated schemas remain tractable
  • refresh the public boundaries/limitations audit to distinguish deliberate protocol boundaries, deferred work, and named implementation gaps

Why

The reference agent should execute every capability it claims and pass every in-scope storyboard that is not explicitly quarantined. The public limitations page should also describe current 3.2 behavior rather than preserving resolved 3.0 gaps as if they were still missing.

SDK beta 12 restores authored account scope for wholesale product feeds, so that regression is now required-clean. It also makes functional request signing available to the reference runner, addressing the previously documented signing limitation. Wholesale signal-feed scope and end-to-end webhook emission remain narrowly documented and quarantined because beta 12 does not resolve them.

Validation

  • npm run precommit: 1,056 repository tests passed
  • server unit suite: 7,020 passed, 30 skipped across 488 files
  • focused delivery, signing, sharding, schema, runtime, and certification suites passed
  • frozen 3.0.26 compatibility matrix: all seven tenant floors passed; sales replay was 39/39 clean with 219 passed steps and zero failures
  • storyboard schema/scoping/context/validation lints
  • schema and compliance builds
  • A2A profile, targeting-aware discovery, and audience-activation contract tests
  • changeset scope, documentation navigation, and status checks

Two post-merge pre-push matrix attempts passed all non-sales tenant floors. Sales was blocked once by an RSS-limit kill and once by a local fetch failed; both affected storyboards passed immediately in isolation (9/9 and 39/39), and neither run exposed a deterministic assertion regression.

Comment on lines +2677 to +2682
import {
buildCatalog,
buildShowsForProducts,
buildProposals,
TRAINING_AUDIENCE_ACTIVATION_METHODS,
} from './product-factory.js';
"$schema": "http://json-schema.org/draft-07/schema#",
"$id": "/schemas/core/assets/asset-union.json",
"title": "AssetVariant",
"x-adcp-hoist": true,

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.

Medium: Duplicate x-adcp-hoist key. This object already declares "x-adcp-hoist": true two lines down (after description), so this added line makes it appear twice in the same JSON object. JSON.parse collapses to the last value (both true, so the build behaves), but a duplicate key in published schema source is unintended and trips strict JSON linters. Drop this line and keep the existing one.

@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 review approval.

This PR strengthens the 3.2 reference sales agent, adds two capability-gated compliance scenarios, ratchets sales CI floors, and refreshes boundaries/limitations docs. Changeset is present (patch), and schema↔docs coherence was verified (pharmaceutical_advertising enforcement in sync-plans-request.json matches the four-category doc claim).

Why escalate (decision table row 2): gated_paths: true — the diff modifies .github/workflows/training-agent-storyboards.yml and static/schemas/source/core/assets/asset-union.json, both under hard, deterministic approval gates. review_decision is REVIEW_REQUIRED (not APPROVED), so the gate is NOT satisfied. Human/CODEOWNERS review is required before merge.

Medium findings (non-blocking):

  • static/schemas/source/core/assets/asset-union.json:5 — Duplicate x-adcp-hoist key introduced. Worth cleaning up before merge, but not itself a blocker.

No critical/high findings surfaced; the single medium is not in a data-loss/schema/infra category that would independently escalate, but the gated-paths gate stands on its own.

Escalation reasons:

  • .github/workflows/training-agent-storyboards.yml (gated path, .github/workflows/**) modified — requires human/CODEOWNERS review.
  • static/schemas/source/core/assets/asset-union.json (gated path, static/schemas/source/**) modified — requires human/CODEOWNERS review.

Medium findings

  • static/schemas/source/core/assets/asset-union.json:5 — Duplicate x-adcp-hoist key in asset-union.json

Why human review

  • Modifies .github/workflows/training-agent-storyboards.yml — gated path (.github/workflows/**) with review_decision=REVIEW_REQUIRED; human/CODEOWNERS approval required.
  • Modifies static/schemas/source/core/assets/asset-union.json — gated path (static/schemas/source/**) with review_decision=REVIEW_REQUIRED; human/CODEOWNERS approval required.
  • Medium: duplicate x-adcp-hoist key in asset-union.json:5 should be resolved before merge.
  • This PR touches a path under a hard, non-overridable approval gate (.github/workflows/training-agent-storyboards.yml (modified) matches .github/workflows/**; static/schemas/source/core/assets/asset-union.json (modified) matches static/schemas/source/**) 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.

@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Aug 25, 2026
domain: "acmeoutdoor.example"
account:
brand:
domain: "acmeoutdoor.example"

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.

Medium: Broken indentation in the create_media_buy sample_request. domain sits at 12 spaces — the same level as brand: on the prior line — so account.brand resolves to null and domain/operator become direct keys of account. Every other account block in this file nests as account.brand.domain (14 spaces); this one alone diverges, sending a null-brand account on the wire. The domain-rename edit dropped two spaces here.

Suggested change
domain: "acmeoutdoor.example"
domain: "acmeoutdoor.example"

@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 path awaiting human/CODEOWNERS approval (review_decision ≠ APPROVED).

Gated paths (hard approval gate, row 2):

  • .github/workflows/training-agent-storyboards.yml (modified) — matches .github/workflows/**
  • static/schemas/source/core/assets/asset-union.json (modified) — matches static/schemas/source/**

gated_paths: true and review_decision is unknown/none (not APPROVED). This is a hard, deterministic gate — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

Medium findings (informational, not blocking on their own):

  • static/compliance/source/protocols/media-buy/scenarios/advanced_delivery_reporting.yaml:150 — Broken YAML nesting: the domain rename dropped two spaces, collapsing account.brand.domain into a null account.brand plus a stray account.domain in the create_media_buy sample_request. Should be fixed before merge.

Iteration note: The prior asset-union.json duplicate x-adcp-hoist key finding is no longer present in this delta — acknowledged as addressed. The SDK bump (14.0.0-beta.8→beta.11) is propagated coherently across changesets, docs, package.json, and the certification module; wire pin stays 3.2-beta.6 with no schema/docs drift. This is a net improvement over the prior pass. The remaining escalation is purely the unsatisfied gated-path approval gate plus one new medium YAML defect worth a human eye.

Medium findings

  • static/compliance/source/protocols/media-buy/scenarios/advanced_delivery_reporting.yaml:150 — Broken YAML nesting: account.brand null in create_media_buy sample_request

Why human review

  • Modifies .github/workflows/training-agent-storyboards.yml — gated path (.github/workflows/**) with review_decision not APPROVED; human/CODEOWNERS approval required.
  • Modifies static/schemas/source/core/assets/asset-union.json — gated path (static/schemas/source/**) with review_decision not APPROVED; human/CODEOWNERS approval required.
  • New medium finding: broken YAML nesting in advanced_delivery_reporting.yaml:150 (null account.brand) — worth resolving before merge.
  • This PR touches a path under a hard, non-overridable approval gate (.github/workflows/training-agent-storyboards.yml (modified) matches .github/workflows/**; static/schemas/source/core/assets/asset-union.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'unknown', 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.

# Conflicts:
#	package-lock.json
#	package.json
#	scripts/run-storyboards-matrix.sh
#	server/src/training-agent/task-handlers.ts

@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 an APPROVED review.

This PR modifies files under hard, deterministic approval gates and the current GitHub review decision is not APPROVED, so Ladon cannot auto-approve regardless of how clean the diff is (decision table row 2).

Escalation reasons

  • Modifies .github/workflows/training-agent-storyboards.yml — gated path (.github/workflows/**); requires human/CODEOWNERS approval.
  • Modifies static/schemas/source/core/assets/asset-union.json — gated path (static/schemas/source/**); requires human/CODEOWNERS approval.

Medium findings (carried forward)

  • static/compliance/source/protocols/media-buy/scenarios/advanced_delivery_reporting.yaml:150 — Broken YAML nesting: account.brand null in create_media_buy sample_request. Not in this delta (a merge of main), so not re-raised by the reviewer, but still unresolved and worth fixing before merge.

Notes for reviewers

  • The current delta (e5f47cfe19f481) is a merge of main, not net-new work. The reviewer confirmed the code that appears to be introduced (read-only tool loop, Google tool-calling adapter, viewed-seconds schema + semantic validator, functional signing contract) is already-merged main code, reviewed for safety and found solid and well-tested.
  • No new PR-authored defects surface from the delta; no critical/high findings.

Once a human/CODEOWNERS approval is recorded (review_decision = APPROVED), the gate lifts and this can fall through to a normal approve on a clean diff — pending the advanced_delivery_reporting.yaml:150 fix.

Why human review

  • Modifies .github/workflows/training-agent-storyboards.yml — gated path (.github/workflows/**) with review_decision not APPROVED; human/CODEOWNERS approval required.
  • Modifies static/schemas/source/core/assets/asset-union.json — gated path (static/schemas/source/**) with review_decision not APPROVED; human/CODEOWNERS approval required.
  • Carried-forward medium: broken YAML nesting in advanced_delivery_reporting.yaml:150 (null account.brand) — worth resolving before merge.
  • This PR touches a path under a hard, non-overridable approval gate (.github/workflows/training-agent-storyboards.yml (modified) matches .github/workflows/**; static/schemas/source/core/assets/asset-union.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'unknown', 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.

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