[codex] Clarify sandbox runner coverage#5045
Conversation
There was a problem hiding this comment.
Coverage-vs-failure separation is the right shape — selection_result and skip_result finally encode the distinction Scope3-style profiles have been asking for, and the compliance_testing.scenarios enum alignment closes a real drift against the canonical controller request. Two version-coordination gaps want fixing in this PR, not after merge.
Things I checked
- Schema enum addition at
static/schemas/source/protocol/get-adcp-capabilities-response.json:1504-1522is additive; both new values (query_upstream_traffic,force_upstream_unavailable) already live in the canonical request enum atstatic/schemas/source/compliance/comply-test-controller-request.json:481-482. Thelist_scenariosexclusion is justified — discovery op, not a capability. - Sandbox-vs-Live rewrite in
docs/building/verification/aao-verified.mdxis internally consistent. Remaining(Live)strings (lines 57, 70, 79) are historical-context prose, correctly preserved. - New
selection_resultblock atstatic/compliance/source/universal/runner-output-contract.yaml:879-916is consistent with the new doc vocabulary inhow-grading-works.mdx,validate-your-agent.mdx,get-test-ready.mdx. - Lint path migration at
scripts/lint-universal-storyboard-doc-parity.cjsmatches the new docs location. ad-tech-protocol-expert: sound with caveats — version + changeset level are publishable-spec drift.
Would flip to approve once these land
- Bump
runner-output-contract.yamlfrom2.3.0to2.4.0. Line 29 stays at"2.3.0"butrun_summary.required_fieldsat line 1143 now mandatessteps_not_selected. Same version string, two different normative shapes — a runner emitting valid 2.3.0 output today becomes non-conforming against this same advertised version. Precedent: #4796 (2.1→2.2) and #4967 (2.2→2.3) both bumped for additions of comparable scope. - Declare a level on
.changeset/live-sandbox-coverage-gaps.md. Frontmatter is empty (---\n---), sochangesets versionwill not surface this change in CHANGELOG even thoughstatic/schemas/source/**andstatic/compliance/source/**both changed. Precedent:3216-storyboard-schema-generate.md(single schema field addition) →"adcontextprotocol": patch. The empty-frontmatter precedent (2487-image-size-cap-for-c2pa-manifest.md) was a pure build hook, not a wire-touching schema change.patchat minimum;minoris defensible.
Follow-ups (non-blocking)
- Test deletions in
server/tests/unit/training-agent.test.ts—code-reviewerflagged thecompliance_testing.scenariosadvertisement assertion and the cross-machineseed_measurement_catalogdurability test were removed wholesale. The schema just expanded the canonical enum; restoring the advertisement assertion against the new set would catch a future re-narrowing ofhandleGetAdcpCapabilities. - SDK internal path in
server/tests/manual/run-storyboards.ts:69-80— prewarm reaches into@adcp/sdk/dist/lib/validation/schema-loader.jsviacreateRequire+dirname(require.resolve). Bypasses the packageexportsmap; the next SDK reshuffle (already happening per the beta.12→beta.13 churn this PR is reacting to) will throw and abort the 3.0-compat lane. Wrap intry/catchor push the helper into the SDK's published surface. - Doc coverage on selection reasons — the contract defines four reasons (
run_mode_excluded,explicit_scope_excluded,version_excluded,profile_excluded); the verification docs only exemplifyrun_mode_excluded. One worked example ofversion_excludedinhow-grading-works.mdxwould help dashboard implementers. - Workflow
git archiveofdist/schemas/${VERSION}at.github/workflows/training-agent-storyboards.yml:200-202— the resulting${TMP_DIR}/schemas/cache/${VERSION}directory isn't consumed byrun-storyboards.tsor env. The real stage runs viastage-sdk-schema-bundle.sh. Either drop the lines or wire an env var.
Two-line fix for items 1 and 2 unblocks merge.
There was a problem hiding this comment.
LGTM. Follow-ups noted below. The not-selected/skipped split is the right shape — collapsing them was masking the seller-correct case (production correctly omits comply_test_controller, so the run is zero-failure partial, not skipped-for-cause).
Things I checked
- Schema change at
static/schemas/source/protocol/get-adcp-capabilities-response.json:1497,1501is description-only. The 17-valuescenarios[]enum is byte-identical to the canonical controller enum atstatic/schemas/source/compliance/comply-test-controller-request.json:464-483minuslist_scenarios, exactly as the new description now claims. No wire shape change. No MUST escalation — the new prose is a SHOULD. - New universal storyboard rows in
compliance-catalog.mdxandconformance.mdxresolve to real YAMLs:comply-controller-mode-gate.yaml(id: comply_controller_mode_gate) andversion-negotiation.yaml(id: version_negotiation). Doc-parity lint was simultaneously moved todocs/building/verification/, fixture layout in the test matches. buildCreativeCompletedwrapper atserver/src/training-agent/task-handlers.ts:4623-4751is schema-conformant on the error envelope:BuildCreativeError'snotclause only forbidscreative_manifest/creative_manifestsandstatus: \"submitted\", whileprotocol-envelope.jsonrequiresstatusto be present.status: 'completed'on a terminal error envelope matches existing precedent inhandleSyncCreatives. Happy path is unchanged.productForThreeZeroStoryboardCompat(task-handlers.ts:244-254) cleanly stripsproduct_card/product_card_detailedfor the 3.0 compat overlay before they hit the response, since the 3.0 Product schema does not carry them.lastGetProductsContextalso stores the stripped shape so refine() does not bleed 3.1 fields into a 3.0 session.PINNED_VERSIONatscripts/overlay-compliance-cache.sh:41is consumed downstream (line 56 default, line 149 guard). Not dead code.- Changeset (
live-sandbox-coverage-gaps.md) has empty frontmatter — matches the precedent at3570-version-negotiation-docs-and-storyboard.mdfor docs/compliance-only changes that ride alongside a separate SDK bump.
Follow-ups (non-blocking — file as issues)
steps_not_selectedis a new REQUIRED field onrun_summary(static/compliance/source/universal/runner-output-contract.yaml:1143). For pre-beta.13 SDK consumers, that field is absent and a strict validator will reject the older shape. The contract change is correct, but the changeset prose downsells it as "docs(compliance)" — when 8.1 leaves beta, this is worth a one-line migration note for anyone parsing run_summary outside the SDK.selection_result.reasonoverlap.run_mode_excludedandprofile_excludedcan both fit a Sandbox-only run excluding live-only probes.aao-verified.mdx:79-82impliesrun_mode_excludedis canonical for the sandbox/live split andprofile_excludedis reserved for additional named-profile rules — worth a single sentence inrunner-output-contract.yaml:1126-1140saying so explicitly, so two conforming runners do not classify the same exclusion under different reasons.prewarmAsyncVariantSchemasreaches into@adcp/sdk/dist/lib/validation/schema-loader.js(server/tests/manual/run-storyboards.ts:60-73). Internal SDK path. Will fail loud (MODULE_NOT_FOUND) if reorganized, which is acceptable here, but a two-line comment naming the SDK surface this depends on saves the next bump a debugging round.
Minor nits (non-blocking)
- "Naming history" table still shows the old
(Live)qualifier.docs/building/verification/aao-verified.mdx:69-71readsAAO Verified (Live)andAdCP Conformant + AAO Verified→AAO Verified (Spec + Live). Every other reference on the page now says(Sandbox). Last(Live)holdout — notable in a PR whose entire point is to retire that framing. - AAO vs AgenticAdvertising.org is mixed on the same concept.
aao-verified.mdx:273says "AAO compliance heartbeat" and "AAO membership";aao-verified.mdx:285says "AgenticAdvertising.org compliance heartbeat". Pick one rule (e.g., "AAO" stays as the badge brand prefix, "AgenticAdvertising.org" only for registry/runner host references) and normalize the file in a follow-up sweep.
Approving on the strength of the schema/contract coherence and the three-layer mental model (runner evidence → verification profile → buyer profile) landing consistently across how-grading-works.mdx, validate-your-agent.mdx, get-test-ready.mdx, and aao-verified.mdx.
Summary
@adcp/sdk@8.1.0-beta.13, which contains the runner-side split fornot_selectedvsskippedsteps_not_selected,not_selected_by_reason, and skip breakdown fieldsWhy
The review thread was asking for two different concepts to stop collapsing into one bucket:
This PR reflects that distinction in the published docs and schema surface. A Scope3-style buyer profile can now reason about selected coverage separately from unimplemented or unavailable functionality.
Upstream runner issue: adcp-client#2012. It is closed as shipped in beta.13.
Notes
--no-sandboxis not introduced as the recommendation for sandbox testing; it remains documented only as the production/live-path knob.Checks
npm run typechecknpx vitest run server/tests/unit/training-agent.test.ts --pool=threadsnpm run test:docs-navnpm run test:storyboard-doc-paritynpm run test:schemasnpm run test:unitnpm run test:test-dynamic-importsnpm run test:callapi-state-changebash scripts/run-storyboards-matrix.shbash scripts/run-storyboards-3-0-compat.sh