[codex] Fix available-actions active-buy storyboard#5379
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
…ause # Conflicts: # static/compliance/source/protocols/media-buy/scenarios/available_actions.yaml
There was a problem hiding this comment.
Approving. The fix is the right shape: drive the buy to a real terminal state before probing its action surface, instead of asserting available_actions[] semantics against a buy still stuck in pending_creatives.
The core change is sound. The negative probe at direct_decrease_wrong_status was previously testing the wrong prerequisite — it fired wrong_status only because the buy never left pending_creatives, which is creative-review noise, not action-discovery enforcement. Inverting decrease_budget to allowed_statuses: ["paused"] and driving the buy active via start_time: "asap" + an assigned creative makes the probe test what the scenario name claims: a product-supported action gated out by the buy's current status.
Things I checked
- Lifecycle wiring is complete. New
sync_creativesstep plumbsproducts[0].format_ids[0]into$context.available_actions_format_id, thencreate_media_buyassigns it viacreative_assignmentsalongsidestart_time: "asap".ad-tech-protocol-expertconfirmedasapis a validstart-timing.jsonsentinel and that asap + assigned creative underauto_approvelandsactive— matching the siblingpending_creatives_to_start.yamlpattern. - Capability gate is honest.
requires_capability: media_buy.creative_approval_mode == auto_approveis a real path (get-adcp-capabilities-response.json:266, enum["auto_approve","require_human"]) lifted verbatim frompending_creatives_to_start.yaml. Manual-review sellers gradenot_applicablerather than false-failing on a prerequisite they can't satisfy. Scenario isn't in any required-clean floor, sonot_applicabledrops nothing. - Stale prose got fixed. The
direct_decrease_wrong_statusnarrative now reads "The created buy is active" instead of "still pending_creatives." Self-consistent with the inverted assertion — no contradiction left on the wire. build-compliance.cjsversion is in scope. BothbuildTo(latestDir, version, ...)sites sit insidemain()afterconst version = getVersion(); stampinglatest/with real semver instead of the literal'latest'. No consumer keys off the literal string value.- Shim is ledgered and fail-loud.
patch-sdk-storyboard-request-builder.cjsis idempotent (sampleStart === 'asap'early-exit), backs up before patching, and hard-exits 1 if the expected window is gone. Ledger entry, lint-shim assertion, andrestoreloop over both backups are all updated in lockstep.field_value_or_absenton legacystatuscorrectly handles the 3.1 status-rename deprecation window (#4908).
Follow-ups (non-blocking — file as issues)
- Request-builder patch now runs unconditionally (
run-storyboards-matrix.sh:175). The pre-existing SDK reach-ins wereOVERLAY=1-gated; this new patch is not, so theasap-preservation behavior also applies to--latest-3.0released-bundle compat runs and to otherasapscenarios (proposal_finalize_asap_timing.yaml, etc.). The trap/restore is symmetric so nothing leaks — this is blast-radius, not corruption. PR body says the 3.0 matrix passed pre-push; if the universal application is deliberate, state that in the ledger entry. Otherwise gate it behindOVERLAY=1to match the other reach-ins.
Minor nits (non-blocking)
- Backup precedes the match guard.
patch-sdk-storyboard-request-builder.cjs:23-25writes.adcp-overlay-backupbefore checking thatlegacyWindowexists. A future SDK lacking both the legacy window and theasapmarker leaves a spurious (identical) backup on the exit-1 path.run-storyboards-matrix.sh's EXIT trap cleans it, butoverlay-compliance-cache.sh(set -euo pipefail) aborts first. Move the backup after both early-exit guards.
Two clean expert verdicts: code-reviewer — no blockers, one Medium noted above; ad-tech-protocol-expert — sound-with-caveats, and every caveat it raised is already closed by the diff it couldn't fetch.
LGTM. Follow-ups noted below.
There was a problem hiding this comment.
Approving. Drives the buy to active before the negative probes so the storyboard tests available_actions enforcement, not a pending-creative lifecycle artifact — the prerequisite and the thing under test are finally separated.
Things I checked
- The status flip is internally consistent.
decrease_budgetallowed_statusesgoes["active"]→["paused"]in the fixture (yaml L88) and theget_productsassertion (L170), and the negative-probeexpectedprose now reads "only for paused buys. The created buy is active" (L529-531). The probe firesdecrease_budgetagainst anactivebuy and correctly expectsACTION_NOT_ALLOWED. No stale assertion stranded —decrease_budgetis never asserted into buy-levelavailable_actions. - The active-buy path is wired end to end.
requires_capability: media_buy.creative_approval_mode == auto_approve+accepts_creatives+ the newsync_creativesstep +context_outputscapturingavailable_actions_format_id+$context.available_actions_format_idin the sync request all line up. Mirrors the mergedpending_creatives_to_start.yamlgate pattern. Manual-review sellers gradenot_applicablerather than false-failing on the lifecycle prerequisite. media_buy_status: active+field_value_or_absenton legacystatusis the sanctioned dual-field handling for the 3.1 deprecation window.media_buy_statusis the correct 3.1 wire field,activeis a valid enum (enums/media-buy-status.json),start_time: "asap"is spec-valid (core/start-timing.json,create_media_buy.mdxL165).build-compliance.cjs:765,782('latest'→version) flows togenerateIndexand stampslatest/index.json. No consumer compares those fields to the literal"latest"; the^3\.0\.[0-9]+$reader inrun-storyboards-matrix.shis unaffected. Aligns devlatest/with the release build, which already stamped real semver.run-storyboards-matrix.shtrap moved out of theOVERLAY==1block.restore_sdk_generated_schemais idempotent (acts only if the backup dir exists). Always-on EXIT trap now also cleans up the--latest-3.0/--compliance-dirpaths the old code leaked. Safe and an improvement.- Changeset present, empty frontmatter = no version bump — correct for a compliance-scenario + build-infra change with no published-package wire impact.
Follow-ups (non-blocking — file as issues)
active-on-create is a hard pin against a specMAY.specification.mdxL143 letsauto_approvesellers returnactive,pending_creatives, orpending_startfromcreate_media_buy(seller's choice based on platform setup time). Theasap+ assigned-creative +auto_approvecombination makesactivea reasonable deterministic reading, but a conformant seller with non-trivial setup time could legitimately returnpending_startand false-fail. Consider acceptingactive-or-pending_startand driving a flight-start transition before the negative probe.confirmed_atis not pinned.create-media-buy-response.jsonmakesactive+confirmed_at: nulla oneOf violation. The scenario assertsactivebut never assertsconfirmed_at != null, so a provisional buy could pass theactivecheck while silently violating the response oneOf. Worth afield_presentonconfirmed_atalongside theactiveassertion.
The third drift-cleanup in the lineage that ends with the SDK shim ledger swapping "SDK package" for "protocol bundle" in a single sentence — notable that the prose churn ships in the same PR as the fix, but it's accurate.
LGTM. Follow-ups noted below.
Summary
Fixes the available-actions storyboard so it first drives the buy active with
start_time: "asap", a synced creative, and an assigned creative before running pause-shaped negative probes. Also fixes dev compliancelatestmetadata to use semver and adds a temporary ledgered SDK request-builder shim so local/CI storyboard runs preserveasapuntil the upstream SDK release lands.Verification
Precommit passed unit, lint, and typecheck stages; pre-push passed version sync, current-source storyboard matrix, 3.0 compatibility storyboard matrix, and skipped docs checks because no docs changed.