Skip to content

fix(handlers): sync MEDIA_BUY_STATE_MACHINE with spec v3 enum#289

Open
bokelley wants to merge 2 commits intomainfrom
claude/issue-288-state-machine-pending-activation
Open

fix(handlers): sync MEDIA_BUY_STATE_MACHINE with spec v3 enum#289
bokelley wants to merge 2 commits intomainfrom
claude/issue-288-state-machine-pending-activation

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #288

Summary

Issue #288 identified schema drift between the SDK and AdCP v3 spec. Most reported drift was already resolved in v4.0.0 (the MediaBuyStatus generated enum and AggregatedTotals fields are correct). One residual bug remained: MEDIA_BUY_STATE_MACHINE in src/adcp/server/helpers.py still had "pending_activation" as its pre-flight key — a status that no longer exists in enums/media-buy-status.json. This caused valid_actions_for_status("pending_start") to return [] instead of the correct pre-flight action list, and valid_actions_for_status("pending_activation") to return a non-empty list for a spec-invalid status.

Changes:

  • Replace "pending_activation" key with "pending_start" (same pre-flight actions) and "pending_creatives" (adds sync_creatives — the buyer must attach creatives before the buy can serve)
  • Expand valid_actions_for_status docstring to enumerate all valid status strings and document the [] fallback
  • Update tests/test_server_helpers.py: rename test_pending_activation_allows_canceltest_pending_start_allows_cancel, add test_pending_creatives_allows_sync_creatives
  • Fix stale pending_activation → active webhook transition example in skills/build-seller-agent/SKILL.mdpending_creatives → pending_start → active

What was tested

  • pytest tests/test_server_helpers.py — 43 passed
  • ruff check src/adcp/server/helpers.py — clean
  • mypy src/adcp/server/helpers.py — no issues

Pre-PR review

  • code-reviewer: approved — no blockers; nit: is_terminal_status hard-codes its terminal set independently of MEDIA_BUY_STATE_MACHINE (same drift risk pattern, currently consistent); nit: test_pending_start_allows_cancel could add assert "sync_creatives" not in actions negative assertion
  • dx-expert: approved — no blockers; nit: add intent comment in pending_start explaining sync_creatives is intentionally absent (creatives locked once flight-date-pending); same negative assertion nit as above

Nits surfaced (not fixed — reviewer discretion)

  • is_terminal_status hard-codes ("completed", "rejected", "canceled") separately from the state machine; currently consistent but divergence risk on future spec updates
  • test_pending_start_allows_cancel could assert "sync_creatives" not in actions to make the pending_creatives vs pending_start distinction explicit
  • pending_start action block could carry a one-line comment: # sync_creatives intentionally absent — creatives are locked once flight-date-pending

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_01Q4gEoNmiYsfqnfFSR71JLg


Generated by Claude Code

`pending_activation` was removed from `enums/media-buy-status.json` in
the AdCP v3 spec; `pending_start` and `pending_creatives` are the
correct replacements. The state machine dict and its test were still
using the stale key, causing `valid_actions_for_status("pending_start")`
to return `[]` and `valid_actions_for_status("pending_activation")` to
return a non-empty list for a status that no longer exists.

Also updates the `valid_actions_for_status` docstring to enumerate valid
status strings, and corrects the stale `pending_activation → active`
webhook example in `skills/build-seller-agent/SKILL.md`.

Closes #288

https://claude.ai/code/session_01Q4gEoNmiYsfqnfFSR71JLg
Acting on python-expert review of this PR:

- Add ``test_pending_activation_is_not_recognized`` so a copy-paste
  from old SDK code or pre-3.0 spec docs surfaces as an empty action
  list (the documented contract) and never accidentally re-matches
  via a future stale entry.
- Add ``test_state_machine_keys_match_spec_enum`` asserting the dict
  contains exactly the seven canonical statuses from
  ``enums/media-buy-status.json`` (pending_creatives, pending_start,
  active, paused, completed, rejected, canceled). Guards against
  future silent drift between the spec enum and the dispatcher
  table — the regression that produced this bug originally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python SDK (adcp 3.10.0) schema drift: MediaBuyStatus, AggregatedTotals, UpdateMediaBuySuccess out of sync with v3 schemas

2 participants