feat(cert): pin certification modules to training-agent tenants#3930
Merged
feat(cert): pin certification modules to training-agent tenants#3930
Conversation
Each certification module now declares the training-agent tenants its lessons exercise. Sage emits deterministic per-tenant URLs at start / get / context-build time instead of the legacy single-URL alias that pre-dated the multi-tenant migration in #3713. Closes the wrong-tenant footgun for cert work — a learner on a signals module no longer gets pointed at /sales/mcp, finds get_signals missing, and hits Unknown tool. Schema: `certification_modules.tenant_ids TEXT[]` (ordered — index 0 is primary). NULL means "no pinning — fall back to discovery extension" (safe default for future modules). Migration 464 backfills all 20 seeded modules; multi-tenant ones (C1, C2, C3, C4, D2, S5, A3) declare the full set with primary first. Plumbing: `tenantUrlsForModule()` in training-agent/config.ts resolves ids → URLs at the prompt boundary; `formatTenantBlock()` in certification-tools.ts collapses single-tenant to a one-liner and emits primary + sibling list for multi-tenant modules. Three injection sites updated: buildCertificationContext (unions tenant_ids across active modules), start_certification_module, get_certification_module. Lays groundwork for the persona harness in #3712 — assertions become "for module M, did Sage steer the persona to a tenant in M.tenant_ids?" rather than "did the LLM correctly infer from the discovery extension?". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
Address feedback from code-reviewer, adtech-product-expert, education-expert, and prompt-engineer on #3930. Substantive changes: 1. NULL out A3, C3, S5 in migration 464. The training agent has no tenant that serves si_* tools — verified empirically against the local stack (every tenant + the legacy /mcp returned zero si_* in tools/list). Pinning these to a sibling would ship a confidently-wrong URL into Sage's prompt. Tracked as #3940 (add an si tenant + repin). 2. B3 reduced to [sales]. Publisher learners shouldn't be pointed at /signals/mcp — signals is a buy-side discovery surface they consume on their own sales agent, not a tenant they operate. 3. C1 drops governance. Per migration 288, governance lessons live in C2; pinning here was speculative and added URL noise with no matching curriculum content. 4. Multi-tenant prompt block rewritten. Tagged "Internal — do not narrate to the learner" with an explicit error-driven trigger ("on unknown tool → GET /.well-known/adagents.json → switch sibling → retry"). Without the tag Sage paraphrases the URL list into the conversation; without the trigger she treats the discovery extension as docs prose, not a procedure. 5. buildCertificationContext caches the active-modules fetch in a moduleCache Map and reuses it in the per-module loop (was double-fetching on every Sage prompt build). Module ids normalized once at the boundary — no more toUpperCase mismatch between the union loop and the per-module loop. 6. Migration 464 backfill is now `UPDATE ... WHERE tenant_ids IS NULL` so a stale DB with hand-edited rows survives a re-run intact. Tests updated: unit tests assert the new "Internal — do not narrate" framing + explicit error trigger; integration tests assert SI modules are NULL, B3 doesn't touch signals, C1 doesn't touch governance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Expert review pass complete (code-reviewer, adtech-product-expert, education-expert, prompt-engineer). Pushed Substantive changes:
Tests (16 passing): unit tests assert new "Internal — do not narrate" framing + explicit error trigger; integration tests assert SI modules are NULL, B3 doesn't touch signals, C1 doesn't touch governance. Verified end-to-end in docker — local stack with the revised mapping emits |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Each certification module now declares the training-agent tenants its lessons exercise. Sage hands learners deterministic per-tenant URLs (
/signals/mcpfor B3,/brand/mcp+/governance/mcpfor C2, etc.) instead of the legacy single-URL alias that pre-dated the multi-tenant migration in #3713.Closes the wrong-tenant footgun for cert work: a learner on a signals module no longer gets pointed at
/sales/mcp, findsget_signalsmissing, and hitsUnknown tool.Why
Before this PR, three injection sites in
certification-tools.tshardcoded${trainingAgentUrl}/mcp(the legacy alias) for every module's demos:buildCertificationContextline 498 — fires at the top of every Sage turn for any in-progress moduleget_certification_moduleline 1367 — module previewstart_certification_moduleline 1490-1494 — module entry pointPUBLIC_TEST_AGENT_URLS(the per-tenant map shipped in #3713) was referenced in exactly two lines — both for URL recognition during auth resolution, never for steering. Cert content had zero references to per-tenant URLs.How
Schema (migration 464):
certification_modules.tenant_ids TEXT[]. Order is significant — index 0 is primary. NULL means "no pinning — fall back to discovery extension" (today's behavior; safe default for future modules).Backfill: all 20 seeded modules. Single-tenant modules (B3, S3, S4) declare one id; multi-tenant ones (A3 tour, C1/C2/C3/C4 buyer flows, D2, S5) declare an ordered list.
Plumbing:
tenantUrlsForModule()inserver/src/training-agent/config.ts— resolves ids → URLs at the prompt boundary. Decoupled from canonical hostname (https://test-agent.adcontextprotocol.orglives in one place).formatTenantBlock()incertification-tools.ts— collapses single-tenant modules toagent_url: "..."and emits primary + sibling list for multi-tenant modules with explicit "use primary by default; switch when needed; the discovery extension is the documented escape hatch" guidance.buildCertificationContextunionstenant_idsacross active modules so a learner with two specialisms in flight gets a single deterministic source of truth.Mapping (full table)
[sales][sales][sales,signals,governance,creative,brand][sales][sales,creative][signals,sales][sales][sales,signals,governance][brand,governance][creative,brand,creative-builder][sales,signals,brand][sales][sales,governance][sales][sales,signals][sales][creative,creative-builder][signals][governance][brand,creative,governance]Lays groundwork for #3712
The persona harness was going to need to assert "the LLM correctly picked the right tenant from the discovery extension." With pinning in place, the assertion is "for module M, did Sage steer the persona to a tenant in
M.tenant_ids[], primary first?" — a deterministic check, not an LLM-quality measurement.Tests
tenantUrlsForModule+formatTenantBlock(single, multi, empty, hyphenated ids, trailing slash)npm run typecheck+ new tests passTest plan
start_certification_module S4over MCP emits/governance/mcp(not/mcp)🤖 Generated with Claude Code