Skip to content

feat: comply_test_controller on training agent#1695

Merged
bokelley merged 7 commits into
mainfrom
bokelley/triage-and-pr-1694
Mar 25, 2026
Merged

feat: comply_test_controller on training agent#1695
bokelley merged 7 commits into
mainfrom
bokelley/triage-and-pr-1694

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

  • Implement comply_test_controller tool on the training agent, enabling deterministic lifecycle testing for the @adcp/client comply engine
  • Supports 6 scenarios: force_creative_status, force_account_status, force_media_buy_status, force_session_status, simulate_delivery, simulate_budget_spend
  • All transitions enforce the same state machine rules as production — invalid transitions return INVALID_TRANSITION, unknown entities return NOT_FOUND
  • Simulated delivery data surfaces through get_media_buy_delivery
  • Fix misleading SISessionStatus description in si-initiate-session-response.json

Security

  • Sandbox-gated: requires account.sandbox: true (returns FORBIDDEN otherwise)
  • Session-isolated: no cross-session state mutation
  • Validate-before-mutate: all state checks happen before writes
  • Input truncation in error messages

Test plan

  • 32 unit tests covering all scenarios, valid/invalid transitions, idempotency, NOT_FOUND, terminal states, delivery/budget simulation
  • 3 sandbox gating tests (no sandbox, sandbox: false, no account)
  • 204 existing training agent tests still pass
  • Full pre-commit hook suite passes (schemas, types, lint, all test suites)

Closes #1694
Closes #1693

🤖 Generated with Claude Code

bokelley and others added 4 commits March 26, 2026 06:36
Add comply_test_controller tool to the training agent, enabling
deterministic lifecycle testing for the @adcp/client comply engine.

Supports: force_creative_status, force_account_status,
force_media_buy_status, force_session_status, simulate_delivery,
simulate_budget_spend. All transitions enforce the same state machine
rules as production. Simulated delivery data surfaces through
get_media_buy_delivery.

Also fixes #1693: SISessionStatus description in
si-initiate-session-response.json was misleading.

Closes #1694
Closes #1693

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Validate before mutating state in handleForceSessionStatus
- Add sandbox gating (account.sandbox required)
- Remove cross-session media buy lookup (session isolation)
- Truncate scenario value in error messages
- Shorten tool description (agent discovers params via list_scenarios)
- Clean up verbose test comments
- Add sandbox gating tests

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread server/src/training-agent/comply-test-controller.ts Fixed

@bokelley bokelley left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Implementation looks solid — state machine tables are correct, sandbox gating works, error codes match the spec, and test coverage is thorough. A few things:

1. archived is not terminal in the creative transition table

The table has archived: ['approved'] which means archived creatives can be unarchived. This is fine if that's the intended spec behavior, but worth calling out: the @adcp/client deterministic creative scenario currently tests archived → processing as the invalid-transition case, which works since that specific path isn't in the table. But the scenario description says "archived is terminal" — it's not terminal here, just restricted. We'll align the client-side scenario language.

2. force_account_status changes status but nothing gates on it

getAccountStatus is exported but I don't see it wired into create_media_buy or any other handler to enforce operation gates. So if comply calls force_account_status(suspended) then tries create_media_buy, the media buy will succeed because handleCreateMediaBuy doesn't check the account status.

This means the testAccountStateMachine scenario in @adcp/client (which verifies create_media_buy is blocked when suspended) will fail against this training agent. Either:

  • Wire getAccountStatus into handleCreateMediaBuy to return an error when suspended/payment_required/closed
  • Or we skip the operation-gate test in the client until this is wired

The status tracking itself is correct — it's just the enforcement that's missing.

3. SI sessions: unknown session IDs are implicitly active

handleForceSessionStatus treats unknown session IDs as implicitly active rather than returning NOT_FOUND. This is pragmatic for testing (comply creates a session via si_initiate_session, then the controller needs to find it) but means NOT_FOUND is never returned for sessions. Not a blocker — just noting the asymmetry with creative/media buy which do return NOT_FOUND.

4. simulate_delivery integration is clean

The getDeliverySimulation export wired into handleGetMediaBuyDelivery is exactly right — additive simulation data flows into the real delivery response. This is the pattern the spec calls for.

Overall this is ready to merge with just #2 as a follow-up (or in this PR if it's easy to add the account status check to handleCreateMediaBuy).

bokelley and others added 2 commits March 26, 2026 06:55
- Wire getAccountStatus into handleCreateMediaBuy to block operations
  when account is suspended/payment_required/closed/rejected
- Remove unused CREATIVE_TERMINAL variable (CodeQL finding)
- Add test: create_media_buy blocked when account is suspended

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Protocol fixes from ad-tech-protocol-expert review:
- Add approved → pending_review creative transition (seller re-review)
- Remove invalid payment_required → suspended/closed transitions
- Add FORBIDDEN and INVALID_STATE to response schema error enum

Agentic product fixes from agentic-product-architect review:
- list_scenarios now returns per-scenario parameter schemas (required
  params, optional params, valid statuses, notes) instead of just names
- Improved tool description with negative guidance and sandbox
  requirement
- Improved params description to reference list_scenarios

Code review fix:
- simulate_delivery rejects terminal media buys (INVALID_STATE)
- Tests for approved→pending_review, rejection_reason enforcement,
  terminal delivery rejection

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread server/src/training-agent/comply-test-controller.ts Fixed
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 9ae4fdc into main Mar 25, 2026
11 checks passed
@bokelley bokelley deleted the bokelley/triage-and-pr-1694 branch March 25, 2026 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: implement comply_test_controller on training agent SISessionStatus description is misleading

1 participant