test(seller-agent): add MCP-level tool tests and cover force_account_status

[bokelley]

Policy-compliant relay of external contribution #475 so repository-default CodeQL can run. The original contributor remains the commit author; maintainer and Ladon reviews plus CI passed on #475.

Summary

Closes #140.

reference/seller-agent already had strong, honest state-machine coverage for most of the issue's acceptance bullets — added incrementally after #138 merged (see #195, #245, and follow-on work). Before writing anything I read cmd/seller-agent/main_test.go in full to avoid duplicating that work, and cross-checked it line-by-line against every bullet in the issue:

• create media buy with and without creative assignments — already covered (TestCreateMediaBuy_WithoutCreatives, TestCreateMediaBuy_WithCreatives)
• pending_creatives -> active via sync_creatives and update_media_buy — already covered
• cancellation and double-cancel — already covered (TestCancellation, TestDoubleCancellation)
• list_creatives filtering by creative ID and format ID — already covered
• delivery simulation/reporting — already covered
• custom comply_test_controller scenarios (seed_product, seed_pricing_option, force_create_media_buy_arm) — already covered

Two real gaps remained, which this PR fixes:

1. force_account_status had zero test coverage. It's one of the seller's own TestControllerStore methods (b.forceAccountStatus), wired into comply_test_controller, and nothing exercised it.
2. Every existing test calls backend methods directly (b.createMediaBuy(...), b.updateMediaBuy(...), etc.), which exercises the state machine but bypasses the actual adcp.Register / adcp.AddTool / adcp.RegisterTestController wiring main() uses to serve requests. A regression in tool names, or in how a request/response gets JSON-mapped onto the wire, would compile fine and pass every existing test — the npm storyboard runner would be the only thing to catch it, which is exactly what the issue says shouldn't be the only safety net.

What's added

• Refactor (no behavior change): extracted newServer(b *backend) *mcp.Server out of main() so tests can stand up the exact server main() runs, over an in-memory MCP transport. Verified with git diff -b that this is a pure whitespace/extraction diff — no logic changed.
• main_mcp_test.go (new): 10 tests that call the real registered tools by name with JSON arguments (the way an MCP client, including the storyboard runner, would), covering every bullet in the issue's acceptance list end-to-end: create_media_buy with/without creative assignments, pending_creatives -> active via sync_creatives and update_media_buy, cancel + double-cancel (asserting the specific NOT_CANCELLABLE code, not just "is an error"), list_creatives filtering by creative_id/format_id, delivery simulation+reporting via comply_test_controller (simulate_delivery) + get_media_buy_delivery, and the seed_product/force_create_media_buy_arm custom scenarios through the wire.
• main_test.go: 2 new unit tests closing the one real gap — TestForceAccountStatus and TestForceAccountStatus_NotFound.

Verification that these tests catch real regressions, not just compile

Per this project's own bar for proving a test is load-bearing, I broke three separate pieces of real logic, confirmed the new tests failed for the right reason, then restored the fix and reconfirmed everything passes:

1. Commented out acct.Status = status in forceAccountStatus → TestForceAccountStatus failed (forceAccountStatus did not persist new status, got active).
2. Disabled the double-cancel guard in updateMediaBuy → TestMCP_CancelAndDoubleCancel failed (want NOT_CANCELLABLE on double-cancel, got "INVALID_TRANSITION") — and notably, the pre-existing TestDoubleCancellation kept passing, because it only asserts IsError, not the specific code. This is a concrete example of the MCP-level tests catching something the existing suite didn't.
3. Zeroed out the spend term in simulateDelivery's weightedSpend call → both the new TestMCP_DeliverySimulationAndReporting and the pre-existing TestDeliveryReporting_SimulateDelivery/TestDeliveryReporting_SimulateDeliveryWeightsSpendByBudget correctly failed.

Acceptance criteria

• cd reference/seller-agent && go test ./... — 44 tests, all passing, exercising every state transition in the issue (create/no-create, activation via both paths, cancel/double-cancel, filtering, delivery, and all three custom scenarios) without the npm storyboard runner.
• Storyboard harness (scripts/ci/run_storyboard_reference_seller.sh from [codex] Add Go reference seller storyboard harness #138) is untouched and remains the end-to-end compliance check.

Validation

cd reference/seller-agent && go build ./... && go vet ./... && go test ./... -v
go build ./...   # repo root
go vet ./...     # repo root

All green. git status is clean — this PR touches exactly reference/seller-agent/cmd/seller-agent/{main.go,main_test.go,main_mcp_test.go}.

Note on scope

I did not add a general integration test for get_media_buys account-scoping, sync_accounts, or the standard force_media_buy_status/force_creative_status scenarios beyond what already existed, since those are outside the issue's explicit bullet list and already have direct or indirect coverage (TestForceMediaBuyStatus*, TestImpairmentsRequireMaterialCreativeImpactAndNewReopenID exercising forceCreativeStatus). Happy to extend if maintainers want broader coverage in the same pass.

[aao-secretariat]

Ladon verdict: Approve

Approve — no blocking or medium findings.

Checked:

• targeting/contextagent/handler.go — validator detail now routed to structured slog.Warn; response returns generic "invalid request" instead of echoing err.Error(), closing an info-leak/observability gap. Response shape and HTTP status unchanged, so no breaking-wire-change marker needed; commit uses fix(targeting).
• reference/seller-agent/cmd/seller-agent/main.go — newServer extraction is a behavior-preserving refactor, covered by new MCP-level and force_account_status tests.
• No schema/types_gen.go, tmproto signing/verify, identity-agent TEE, or protocol-managed skills surfaces touched.

Decision-table application: no critical/high/medium findings. gated_paths is false (row 2 N/A). high_risk is true only via (modified)/(added) entries with no medium finding (rows 3–5 N/A). No prior decision, no no-auto-approve team match. Falls through to row 9 → approve. (review_decision: REVIEW_REQUIRED does not block since gated_paths is false and no team gate applies.)

[aao-secretariat]

Ladon verdict: Approve

Approve — clean delta pass, no blocking or medium findings.

Checked:

• contextagent validation path now returns a generic "invalid request" response instead of echoing tmproto.ValidateContextRequest's err.Error(), closing an info-leak of field-shaped detail. Real error is logged server-side via a new logValidationFailure helper.
• The helper verbatim-mirrors the identityagent and router implementations (same SafeRequestIDForEcho guard and request_id_valid=false elision), so behavior is consistent across agents.
• Two new tests pin the generic-response and unsafe-request_id-not-echoed invariants.

High-risk flag is true only because the change touches targeting/** (one modified handler, one added test file) — the reviewer surfaced no medium-or-higher concerns on the modified file, so the modification is presumed safe. No deletions. gated_paths is false. No no-auto-approve team match. This is a net improvement over the prior approve with no new findings. Falls through to row 9.

[aao-secretariat]

Ladon verdict: Approve

Approve — test-only refactor with no findings.

This PR touches three files, all in reference/seller-agent/cmd/seller-agent/: main.go, main_mcp_test.go, and main_test.go. The change extracts main()'s server-wiring closure verbatim into a newServer(b) helper (behavior-preserving: same Register/AddTool calls, same ADCP_SANDBOX guard, same shared backend per connection) and adds MCP in-memory-transport integration tests plus a TestForceAccountStatus unit test.

No sensitive surfaces touched: no wire shapes, no adcp/schemas/** or adcp/types_gen.go, no tmproto signing/verification, no identity-agent TEE boundary, no protocol-managed skills. Schema↔types coherence and TMP semantics are unaffected.

Decision-table walk: no critical/high findings (row 1 n/a); gated_paths is false and review_decision is APPROVED anyway (row 2 n/a); high_risk is false (rows 3/5 n/a); no medium findings (rows 4/8 n/a); no no-auto-approve team match (row 7 n/a); prior decision was approve (row 6 n/a). Falls through to row 9 → approve.

The reviewer returned a clean pass with zero findings.