Skip to content

fix(server): pre-populate storyboard-declared test kits on hosted storyboard runs - #6756

Open
fgranata wants to merge 1 commit into
adcontextprotocol:mainfrom
fgranata:server-test-kit-prepopulate
Open

fix(server): pre-populate storyboard-declared test kits on hosted storyboard runs#6756
fgranata wants to merge 1 commit into
adcontextprotocol:mainfrom
fgranata:server-test-kit-prepopulate

Conversation

@fgranata

Copy link
Copy Markdown
Contributor

Problem

Server-side half of #6735 (P0), built to the triage brief and the maintainer go-ahead in the issue thread.

withHostedAuthTestKit substitutes the graded run's own bearer into test_kit.auth.api_key when no kit is configured — so on hosted runs, from_test_kit steps authenticate as the seller's normal principal instead of the storyboard-declared kit credential. comply_controller_mode_gate then fails deterministically against conformant sellers (NOT_FOUND instead of the contracted FORBIDDEN; 7 of 8 hosted runs in the issue's audit-log evidence).

Change (per the brief)

  • Resolve the declared kit via the existing getTestKitForStoryboard and pre-populate options.test_kit at all four withHostedStoryboardRunOptions call sites: registry-api.ts step run, run-storyboard-via-ws.ts conformance-socket run, and both Addie member-tools.ts sites (run_storyboard, run_storyboard_step).
  • The existing !nextAuth.api_key guard in withHostedAuthTestKit then correctly no-ops the bearer substitution when the storyboard carried its own kit. Storyboards that declare no kit keep the substitution — its legitimate purpose.
  • withHostedComplianceRunOptions (suite-wide comply()) untouched, per the scope guard in the brief.
  • Docs: storyboard-troubleshooting.mdx now states that prerequisites.test_kit is a runner loading directive, not decoration.

Testing

  • server/tests/unit/storyboards.test.ts (47/47): new cases — getTestKitForStoryboard('comply_controller_mode_gate') resolves the kit with its auth.api_key, and the substitution guard holds over a pre-populated declared kit (run bearer does not clobber it).
  • npm run typecheck clean.
  • Note on the precommit hook: the full unit sweep fails locally only on the network-dependent validator.test.ts / training-agent-webhook-fetch.test.ts (DNS/public-hostname fetches unavailable in my sandbox); untouched by this diff — CI is authoritative.

Changeset

The triage brief asked for a patch changeset with a 3.1.x cherry-pick, but the repo's push policy rejects changesets for server-only changes ("Protocol changesets are only allowed when the PR also changes protocol schemas, compliance assets, normative reference docs…"). Deferring to the policy — happy to re-add if you classify this as compliance-assets-adjacent, and the 3.1.x cherry-pick works either way.

Pairs with the runner half: adcontextprotocol/adcp-client#2643.

🤖 Generated with Claude Code

…ryboard runs

The hosted engine's withHostedAuthTestKit substituted the graded run's
own bearer into test_kit.auth.api_key, so from_test_kit steps ran as
the seller's normal principal instead of the storyboard-declared kit
credential - comply_controller_mode_gate failed deterministically
against conformant sellers (NOT_FOUND instead of the contracted
FORBIDDEN; adcp#6735, 7-of-8 hosted runs in the issue's audit-log
evidence).

Per the triage brief: resolve the declared kit via
getTestKitForStoryboard and pre-populate options.test_kit at all four
withHostedStoryboardRunOptions call sites (registry-api step run,
conformance-socket run, Addie run_storyboard + run_storyboard_step).
The existing !nextAuth.api_key guard in withHostedAuthTestKit then
correctly no-ops the bearer substitution when the storyboard carried
its own kit; storyboards that declare no kit keep the substitution.
withHostedComplianceRunOptions (suite-wide comply()) untouched.

Tests: getTestKitForStoryboard resolves the mode-gate kit WITH its
credential; the substitution guard holds over a pre-populated declared
kit. Docs: prerequisites.test_kit documented as a runner loading
directive (storyboard-troubleshooting).

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — server-only auth fix, no blocking or medium findings.

What I checked:

  • Scope is server-only (server/** + one troubleshooting doc). No touch to static/schemas/source/**, docs/reference/**, dist/**, task definitions, or any published protocol surface — so no changeset is required, and no schema↔docs / oneOf / immutable-artifact concerns apply.
  • The fix pre-populates the storyboard-declared test kit into options.test_kit at all four hosted withHostedStoryboardRunOptions call sites, so from_test_kit steps authenticate with the storyboard's declared credential rather than the seller's run bearer. Mechanism relies on the existing !nextAuth.api_key guard in withHostedAuthTestKit, which no-ops the substitution — that load-bearing guard is covered by a test.
  • No REST self-report-score route (certification remains Addie-gated). No production migration changes.

High-risk flag: true only because server/src/addie/** and server/src/routes/** were modified. Both are (modified), not deleted, and the reviewer surfaced no medium-or-higher concerns — presumed contract-preserving. Not escalation-worthy on the flag alone.

Gate check: gated_paths is false, so row 2 does not apply despite review_decision: REVIEW_REQUIRED. No no-auto-approve team match. No prior decision. Zero findings → rows 1–8 do not fire → row 9 approve.

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.

1 participant