docs(proposals): salesagent side-car experiment plan (full GAM)

[bokelley]

Summary

Experiment plan that falsifies #502's two-platform composition against salesagent + live GAM. Side-car runtime in a salesagent worktree, points at salesagent's existing Postgres, drives a sandbox GAM Network ID. Admin UI untouched.

Binary exit criterion: the AdCP `media_buy_seller` storyboard passes against the experiment tenant. If it does, the architecture holds and we have a working `ProposalManager` shape that emerged from real adopter code. If it doesn't, we learn precisely where #502 breaks before the architecture solidifies.

In scope (deliberately ambitious)

• Full GAM upstream — real orders/line items in sandbox. Mock-mode rejected because it would only test wire shape, not whether the SDK can host salesagent's actual upstream complexity.
• HITL approval lifecycle — `compose_method` + `ShortCircuit` against salesagent's actual `WorkflowStep` flow. Grounded in real code (`google_ad_manager.py:571` gate, `media_buy_create.py:458` resumption). Salesagent's `WorkflowStep` table IS its task registry; SDK `TaskRegistry` doesn't participate.
• Webhook delivery via SDK F12 auto-emit — `protocol_webhook_service.py` disabled per-tenant; `WebhookSender` configured on `serve(...)`. Validates the §3.14 claim that adopters delete their webhook plumbing.

Out of scope

Other adapters, other tenants, refine flow / proposal lifecycle (`finalize`, `expires_at`), push reporting, creative agent.

Why now / why GAM live

GAM is where ~99% of salesagent's deployed value lives. The dynamic-product assembly, recipe shape, lifecycle state graph, HITL plumbing — all of it is GAM-shaped today. If our architecture survives contact with GAM, it survives. Mock conformance was never going to tell us.

Wrap, don't port

`media_buy_create.py` is 3,930 LOC; `dynamic_products.py` is 505. The discipline is to wrap the existing tool bodies, not re-port them. Isolates the experiment to the seam — where wire requests become salesagent-shaped calls and back. We learn whether the SDK abstractions can host salesagent's existing logic without re-porting it.

Five questions the experiment answers

1. Does `dynamic_products.py` factor onto `ProposalManager.get_products` (target: <300 LOC of glue)?
2. Does the recipe carry GAM's `implementation_config` without escape hatches?
3. What hydration model does `create_media_buy` need (session cache, DB row, fresh lookup)?
4. What's the right shape for the HITL resumption marker — typed `ctx.resumption_token` vs. adopter-side sentinel?
5. Does F12 webhook auto-emit hold up under real load without adapter participation?

Refs

• #502 — the architecture this experiment falsifies
• #489 — the migration narrative this validates against real code
• `prebid/salesagent` — the adopter codebase the experiment runs inside

Test plan

• Reviewers sanity-check the GAM-vs-mock decision and the wrap-don't-port discipline
• Reviewers flag any storyboard surface that exercises a deferred feature (refine, push reporting) on the happy path
• Reviewers confirm the salesagent file:line citations for HITL are accurate and the SDK-mapping reads cleanly
• Decide whether to start the experiment immediately or wait for adversarial review on this plan first

🤖 Generated with Claude Code

[bokelley]

Read through and verified the salesagent citations against current main (file:line refs all check out, LOC counts match). Here's pushback on execution risks and a few suggested changes.

Gotchas

1. Wrap target is wrong. The plan wraps google_ad_manager.py. But media_buy_create.py (3,930 LOC) does a lot of non-GAM work — buyer/principal resolution, tenant config, currency validation, signal lookup, audit logs, workflow rows, webhook scheduling. Salesagent already established _impl functions as the transport-agnostic seam (CLAUDE.md Pattern #5). The natural wrap is _create_media_buy_impl, not the adapter. Wrapping the adapter forces the side-car to re-implement everything else from scratch — that's a port disguised as a wrap, and the "<300 LOC of glue" target evaporates instantly.

2. "Same process, two runtimes" is the bigger of two unspecified shims. "Tiny dispatcher consults Principal.tenant_id" hides the real problem: both runtimes own MCP+A2A transport mounts, FastMCP tool registries, identity resolution, and (per salesagent's structural guards) ResolvedIdentity construction. Routing has to happen before either runtime's transport layer claims the request. Separate process on a separate port with nginx-level routing by tenant header is cleaner, easier to delete, and avoids dual-import-time conflicts. The plan rules this out implicitly without justifying it.

3. ~25 structural guards will fire on src/sdk_runtime/. Salesagent has AST-enforced guards on make quality: no raw select() outside repos, no get_db_session() in _impl, transport-agnostic _impl, schema inheritance from adcp library, repository pattern. Either the side-car lives outside src/ (and is therefore not really inside salesagent) or every guard needs an allowlist entry. Workstream needs a step 0 for this.

4. Two writers, one DB is not fine just because it's a test tenant.

• SQLAlchemy models defined in both salesagent (models.py, 2,113 LOC) and adcp-client-python — cascade/relationship semantics must agree exactly.
• Single alembic head is a structural guard; SDK runtime can't bring its own migrations.
• Workflow rows written by the SDK's before hook must be re-readable by admin UI's approve_workflow_step and re-callable into the SDK runtime — a cross-runtime contract on a shared row, not a tenant-isolated boundary.

5. The HITL marker decision is N=1, and salesagent's pattern is unrepresentative. Salesagent uses DB-persisted re-call with a sentinel, not paused-coroutine resumption. The experiment can answer "does the SDK seam accommodate salesagent's shape" — it cannot answer "what's the right Protocol seam." Risk: typed ctx.resumption_token gets baked into the spec because it felt natural in one codebase, then the next adopter with TaskRegistry-style coroutine resumption finds it awkward. Decouple this — it's a Protocol design call informed by, not settled inside, the experiment.

6. _already_approved sentinel via setattr on a Pydantic model. With extra=\"forbid\" in dev (salesagent Pattern #7), worth confirming this still works under the SDK's request projection — the new runtime presumably re-validates, which would strip the sentinel. Test before relying on it.

7. Webhook compatibility test is shallow. Disabling protocol_webhook_service.py and turning on WebhookSender tells you whether something fires. It doesn't tell you whether existing buyers, who trust salesagent's HMAC scheme via webhook_authenticator.py, will accept signatures from SDK F12. Risks signature scheme drift being declared "validated" when it wasn't tested against a real subscribed buyer.

8. The "<300 LOC glue" target for GAMProposalManager is unfounded. dynamic_products.py is 505 LOC plus AI services in src/services/ai/. The wrapper must project request, hydrate Product rows, pass to assembly, project response with implementation_config. No basis for the 300 number; will feel like a moving goalpost when reality lands at 700.

9. Single exit criterion lets the experiment lie. "Storyboard passes" is satisfiable with wrappers that grew enormous. Need a co-equal budget criterion: no extra: dict[str, Any] escape hatches in recipe; combined glue under N LOC; zero structural-guard allowlist additions. Otherwise the architectural hypothesis is unfalsifiable even when the storyboard "passes."

10. Pin SHAs in step 0. adcp-client-python from main will rebase. Storyboard version isn't pinned. GAM sandbox network config isn't pinned. Step 1 should be "freeze: adcp-client-python@<sha>, storyboard <tag>, GAM Network ID <id> in experiment README."

Better way (smallest changes I'd push for)

1. Wrap _impl not adapter. The seam already exists.
2. Separate process + tenant-header routing at nginx, not in-process dispatch. Killable. Independently restartable. No model/import collisions.
3. Add a budget exit criterion alongside "storyboard passes." Glue LOC, recipe escape hatches, guard-allowlist deltas.
4. Lift HITL marker decision out of the experiment. Run it as a Protocol design RFC informed by the experiment, not settled by it.
5. Step 0: pin SDK SHA, storyboard tag, GAM sandbox config; resolve structural-guard story for src/sdk_runtime/ (or move it).
6. Webhook test against a real subscribed buyer, not just "did something fire." Otherwise §3.14 isn't validated.

The instincts are right (sidecar, wrap-don't-port, GAM-live, binary criterion). Execution risks: wrap target is too low in the stack, in-process routing is harder than billed, and the architectural hypothesis is over-coupled to one adopter's HITL idiom.

[bokelley]

Plan is well-structured and the salesagent file:line citations check out (spot-checked google_ad_manager.py:570-571, media_buy_create.py:458 + literal setattr(request, "_already_approved", True) at line 529, models.py:256 for implementation_config, admin/blueprints/workflows.py:155 for approve_workflow_step calling into execute_approved_media_buy). The "wrap don't port" discipline and the five forced-choice questions are the right framing.

The headline design risk: the experiment quietly conflates "does the SDK shape host salesagent's logic" with "is the SDK shape correct." A wrap that ports cleanly proves the seam is flexible enough, not that it's right — especially on the proposal side, where I control the wrap, the seam, and the success criterion.

Gotchas / what won't work cleanly

1. _already_approved is one of three approval re-entry surfaces, not one. The plan focuses on create_media_buy, but salesagent gates add_creative_assets and update_media_buy too, and creative approval has its own re-entry path through order_approval_service.py and background_approval_service.py. If compose_method only models the create path cleanly, the experiment passes a happy-path storyboard and still misses ⅔ of the actual seam. Either exercise creative approval too, or scope it out explicitly.

2. raw_request on MediaBuy is the resumption substrate, and it lives in the salesagent DB. The plan acknowledges salesagent's WorkflowStep table is its task registry, but raw_request is the payload — and the SDK has no concept of it. The "typed ctx.resumption_token" option only works if the SDK can carry the equivalent of raw_request through dispatch, or if the adopter rebuilds the request before calling back in. The plan is hand-wavy here ("attach a resumption marker") and this is exactly where shape decisions get made.

3. implementation_config may be too late-bound for the recipe model in docs(proposals): product architecture — layered model + two-platform composition #502. PR docs(proposals): product architecture — layered model + two-platform composition #502 says the recipe "lives in framework session cache against proposal_id" during refine, then "framework persists it" on finalize. Salesagent's implementation_config is on the Product row (models.py:256), not on the proposal. Dynamic products generate signal-driven variant Product rows at brief time (dynamic_products.py) — those are new Product rows the framework doesn't know about. The recipe-as-framework-managed-state model probably has to allow proposal-time recipe assembly, not just lookup. Worth surfacing as Q1.5.

4. F12 webhook signature parity is a real risk, not a small one. Salesagent has its own webhook_authenticator.py and webhook_verification.py. If signing semantics diverge (header names, canonicalization, key rotation), the experiment hits non-determinism that's not a SDK design issue — it'll burn cycles. Do the signing dry-run on day 1, before the storyboard. Surface it as workstream step 0.

5. "<300 LOC of glue" for dynamic_products.py is a vibe number, not a constraint. That file already calls a SignalsAgentRegistry, hits Postgres, mutates Product rows with TTLs (expires_at), hashes for dedup. If the wrap hits 600 LOC, what does the experiment conclude? Set the threshold so it can actually fail.

6. "Two writers, one DB — controlled" is fine if the experiment tenant is genuinely isolated. Salesagent's background services (background_sync_service, background_approval_service, delivery_webhook_scheduler, protocol_webhook_service) run cross-tenant. Disabling per-tenant is doable but easy to miss. Enumerate the disable list in the plan, not a footnote.

7. The media_buy_seller storyboard is a moving target. Pin the commit SHA, not just the version — storyboards have changed materially across 6.7→6.9 and there's more in flight for 6.10.

8. The "binary exit criterion" is too binary. A passing storyboard against a hand-wrapped adapter, where I control the wrap shape, the seam, and the success criterion, isn't falsifying much. More honest exit: storyboard passes AND at least one of the five questions has an answer that contradicts a docs(proposals): product architecture — layered model + two-platform composition #502 prior. Otherwise the experiment will always confirm.

What to be careful about

• One author wearing three hats: SDK author, adopter (salesagent), and reviewer. The "force a choice between typed vs untyped resumption marker" framing is only honest if I commit upfront to which signal would tell me each shape is wrong. Otherwise it's tempting to pick whichever felt natural after the fact and call it "what the experiment showed."
• Don't let the experiment ship the ProposalManager Protocol. Next-steps says factor it in a separate PR — hold that line. Running in a worktree against forked main is qualitatively different from declaring a Protocol shape other adopters inherit. The experiment is one data point, not the spec.
• PR docs: salesagent → adcp Python SDK migration guide #489 §3.3 is already partially baked. It maps HITL to compose_method + ShortCircuit. If the experiment finds compose_method is the wrong primitive, docs: salesagent → adcp Python SDK migration guide #489 has to revise too — name that in the exit criteria so it doesn't get forgotten.

Is there a better way

Two cheaper falsifiers worth running first:

1. Falsify on dynamic_products.py in isolation, before standing up the side-car. Port just that 505-LOC file to a ProposalManager.get_products wrapper as a unit — no GAM, no admin UI, no DB rewiring. ~1 day. If the recipe shape can't carry signal-driven variants without escape hatches, Q1+Q2 are answered without touching the runtime, GAM credentials, or webhook signing. This is the cheapest available falsification of docs(proposals): product architecture — layered model + two-platform composition #502, and it gates whether the side-car experiment is worth running at all.

2. Skip the live runtime; run as a contract test. Drive ProposalManager + DecisioningPlatform against a recorded dynamic_products output and a recorded GAM SOAP transcript. Lose live-GAM realism, gain repeatability + CI integration + the ability to A/B multiple SDK shapes. The current plan is one-shot — once we wire it up we won't run it again as the SDK evolves.

(I considered "do Kevel first, not GAM" but Kevel isn't actually wired to any real tenants today, so it doesn't give us a falsifier with adopter-shape complexity. GAM is the only live target with the surface to be informative — which is also why it's the only one that can really break.)

Three concrete adds before merging the proposal doc

1. Make the exit criterion non-tautological — require a contradicting finding on ≥1 of the five questions, not just a passing storyboard.
2. Either scope creative-approval and update_media_buy re-entry in, or scope them out explicitly. Right now they're invisible.
3. Run the dynamic_products falsification (option 1 above) first. It's the cheapest experiment that can actually kill docs(proposals): product architecture — layered model + two-platform composition #502's recipe model, and it gates whether the side-car experiment is worth doing.

[bokelley]

Thanks for the detailed self-review — the concerns are well-founded and several are blockers for the plan's validity. Since this is your branch I can't push fixup commits directly; flagging what should change before the plan is acted on.

Blockers (plan is misleading or wrong without addressing these):

#1 — Wrap target. Agreed. The plan says "wraps salesagent's existing google_ad_manager.py adapter" but the _impl seam you're pointing at (_create_media_buy_impl) is the right isolation point. Wrapping the full adapter without first landing the _impl split in salesagent effectively makes this a port, not a wrap — and the "<300 LOC of glue" target becomes meaningless. Workstream step 0 should be: identify (or introduce) the transport-agnostic _impl functions that become the actual wrap targets before writing any SDK runtime code.

#2 — Dual transport mounts. The plan doesn't address how routing happens before FastMCP and A2A transport layers claim the request. "Tiny dispatcher consults Principal.tenant_id" buries the hard part. The plan should either (a) specify the exact hook point (ASGI middleware before any framework router? a request-ID header the dispatcher reads at process boundary?), or (b) explicitly choose the separate-process option and justify skipping it. Right now it implicitly rules out separate-process without saying why.

#3 — Structural guards. A "step 0" item is missing: enumerate which of salesagent's AST guards fire on src/sdk_runtime/ code and whether allowlist entries or location changes are needed. This is a concrete prereq, not a risk to manage mid-experiment.

#4 — Cross-runtime execute_approved_media_buy contract. The plan correctly describes the HITL re-entry shape but underplays the shared-row dependency. When execute_approved_media_buy reconstructs the request from raw_request and calls back into the SDK runtime, it's reading a row written by the SDK's before hook — that's a cross-runtime contract on a shared schema row. The "controlled" risk framing is fine for the test tenant, but the plan should acknowledge this explicitly so reviewers don't assume it's isolated.

Should be updated before acting on the plan:

• feat: complete Python AdCP SDK with typed API and auto-generated types #5 (HITL marker N=1): Add an explicit note to the findings-doc step: the experiment informs but does not settle the Protocol seam for ctx.resumption_token. The spec decision belongs in a separate doc after the experiment — not inherited from what felt natural in salesagent.

• revert: restore correct PYPY_API_TOKEN secret name #7 (Webhook test shallowness): Workstream step 6 should add "verify against a subscribed test buyer that validates salesagent's existing HMAC scheme" — otherwise the test only confirms something fires, not that buyer signature verification passes.

• fix: use correct PYPY_API_TOKEN secret for PyPI publishing #8 (<300 LOC target): Replace the absolute target with a ratio goal (e.g., "glue LOC ≤ 60% of dynamic_products.py body") or drop the number and just instrument it. The current target has no basis given the AI services in src/services/ai/.

Lower priority (valid, can be tracked as risks):

• fix: correct PyPI API token secret name #6 (_already_approved with extra="forbid"): Worth adding to Risks as a concrete item: "Validate that setattr(request, '_already_approved', True) survives SDK re-validation under extra='forbid'; if not, prototype typed marker first."

• chore(main): release 1.0.1 #9 (truncated): Didn't receive the full text — if the concern is that a binary storyboard-passes criterion conflates "architecture works" with "we worked around it," that's worth a sentence in the exit criteria section: "Passes counts only if the wraps stayed thin (no inline re-porting). Document any case where the wrap grew into a port as a failure mode even if the storyboard passed."

I'd suggest revising the plan to incorporate at least the four blockers above before starting workstream step 1.

---

Claude Code — cannot push to non-claude/* branches; flagging for your update.

---

Generated by Claude Code

[bokelley]

Good second take — it's complementary to the first review (execution risks) and adds the epistemological layer: can the experiment falsify anything it doesn't already control?

What's genuinely new vs. what was already flagged:

• Q1.5 on implementation_config (point 3): This is new and worth adding as an explicit question. The prior response flagged the cross-runtime contract on shared rows; this sharpens a different edge — docs(proposals): product architecture — layered model + two-platform composition #502's recipe is assumed to live on the proposal (session cache → finalize → persist), but salesagent's implementation_config lives on the Product row, and signal-driven dynamic_products.py generates new Product rows at brief time that the framework doesn't track. If "framework persists the recipe" assumes lookup-by-proposal-id, it breaks at dynamic-product assembly time. That's worth naming as Q1.5 in "What we expect to learn."

• raw_request as payload substrate (point 2): The plan's HITL section says "attach a resumption marker" without grappling with the fact that execute_approved_media_buy reconstructs the full request from raw_request — a DB-persisted JSON blob, not a token. A typed ctx.resumption_token doesn't carry that. Either the adopter rebuilds the request independently before calling back in (from what source?), or the SDK carries more than a marker. The plan is hand-wavy exactly where the shape decision lands.

The three concrete asks are right. The prior response covered the execution blockers; these are the epistemological ones. The "contradicting finding required" framing for the exit criterion is the sharpest version — a storyboard that passes without challenging any #502 prior is unfalsifying by construction, regardless of whether the wraps stayed thin.

I can't push to bokelley/salesagent-gam-experiment, so these are yours to apply. Together the two review threads make a complete revision checklist.

---

Claude Code — PR-feedback mode, non-claude/* branch.

---

Generated by Claude Code