Skip to content

fix(compliance): emit stripped field notices#2257

Merged
bokelley merged 1 commit into
backport-2114-adcp-3.0from
fix/input-schema-strip-3-0
Jun 18, 2026
Merged

fix(compliance): emit stripped field notices#2257
bokelley merged 1 commit into
backport-2114-adcp-3.0from
fix/input-schema-strip-3-0

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Backports the structured input_schema_field_stripped compliance notice fix from main to the AdCP 3.0 / 7.11.x release branch.

Why

adcontextprotocol/adcp#5495 remains open because @adcp/sdk@7.11.5 only prints stripped input-schema fields as console warnings. CI JSON reports still miss the signal, so storyboard scenarios can appear to pass without exercising fields the runner sent.

Changes

  • Emits structured debug_logs entries when SingleAgentClient strips fields missing from an agent tool inputSchema.
  • Preserves pre-polling strip logs when an async storyboard step is replaced by waitForCompletion output.
  • Promotes strip logs to step-level notices and aggregates them into StoryboardResult.notices.
  • Adds regression coverage for sync and async storyboard paths.
  • Syncs generated src/lib/version.ts with the current 7.11.5 package version, matching the branch build output.

Validation

  • npm run build
  • NODE_ENV=test node --test-timeout=60000 --test-force-exit --test-name-pattern "input_schema_field_stripped|v3 partial-schema field stripping" --test test/lib/request-validation.test.js test/lib/storyboard-notices.test.js
  • echo "fix(compliance): emit stripped field notices" | npx commitlint --config commitlint.config.js
  • npx commitlint --from origin/backport-2114-adcp-3.0 --to HEAD --verbose
  • npm pack --silent --pack-destination .tmp-pack, then tarball grep confirmed input_schema_field_stripped appears in packed dist/lib/core/SingleAgentClient.js and dist/lib/testing/storyboard/runner.js.

* fix(compliance): emit stripped field notices

* fix(compliance): preserve strip notices through polling
@bokelley bokelley marked this pull request as ready for review June 18, 2026 08:51
@bokelley bokelley merged commit e0cf1e5 into backport-2114-adcp-3.0 Jun 18, 2026
6 checks passed
@bokelley bokelley deleted the fix/input-schema-strip-3-0 branch June 18, 2026 08:51

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Clean backport of the input_schema_field_stripped surfacing fix from main (b4defa56) to the 7.11.x / AdCP 3.0 branch. Right shape: it promotes a console.warn-only signal to structured debug_logs + storyboard notices without touching the strip behavior itself, so the runner becomes a witness to what it already does rather than a translator of it.

Things I checked

  • Backport parity. All three source files match the merged main fix; main is at 9.0.0-beta.31/3.1.0-rc.14, this branch carries the 7.11.5 generated version.ts. The version.ts bump is the branch's own build output, not a hand-edit racing changesets — package.json is untouched and a patch changeset is present. Not MUST FIX #6.
  • Witness, not translator. collectInputSchemaFieldStripNotices (runner.ts:1444) rebuilds the message from structured details.task/details.fields, not by re-parsing the warning string. No fabricated fields, no silent normalization.
  • executeStep return-site coverage. inputSchemaStripNotices is computed post-dispatch and attached to the three reachable post-dispatch returns — force_scenario_unsupported skip (runner.ts:3595), unsupported/unknown-tool skip (3624), and the main success return. The earlier returns (≤3582) fire before any task dispatch, so there's nothing to carry. No silent drop.
  • Pre-polling preservation. task-map.ts:135 snapshots prePollingDebugLogs before the waitForCompletion replacement and merges only when replacedByPolling. code-reviewer confirmed pollTaskCompletion builds fresh TaskResults with no carried debug_logs, so the merge can't double-count; the seen set is belt-and-suspenders on top.
  • Pass/fail isolation. overall_passed is failedCount === 0 && requiredPhasesPassed && !assertionsFailed (runner.ts:2613) — no notice term. An info notice provably can't flip a passing run, and both new tests assert exactly that.
  • Param threading. debugLogs? is optional (no caller breakage); new positional storyboardId on the module-private executeStep is updated at both internal callers (runner.ts:2167, 2960). No public surface removed or renamed.

Follow-ups (non-blocking — file as issues)

  • Storyboard-level notice rollup is lossy for variable payloads. mergeRunnerNotices (runner.ts:~1483) dedups by code alone, keeping the first notice's message and only unioning storyboard_ids. Fine for the static-message codes (request_signing.required, etc.), but input_schema_field_stripped encodes per-task/per-field detail in its message — two steps stripping different fields on different tasks collapse to the first step's message at StoryboardResult.notices. Full witness survives at step.notices, so it's a partial rollup, not data loss. Either merge by code + task for this notice or fold the distinct field sets into the aggregated message. (code-reviewer: Medium.)
  • Severity direction. ad-tech-protocol-expert (sound-with-caveats): info is the correct safe default for a backport that must not break passing runs, but it collapses two different events — buyer sent an unsupported field (info is right) vs. an agent that under-declares its inputSchema and gets a real field silently dropped (a genuine interop defect). Confirm adcp#5495 tracks the open strip-vs-pass-through design question rather than treating "we emit a notice now" as closure. Not this PR's job to resolve.

Minor nits (non-blocking)

  1. Changeset type. New NoticeCode enum value + new optional notices? on StoryboardStepResult is additive public-type surface, which leans minor by the strict rule. patch is defensible here — it's diagnostic-only and restores intended visibility — but worth a glance next time the rule bites.
  2. docs_url is an issue, not a spec anchor. https://github.com/adcontextprotocol/adcp/issues/5495 is weaker than a published spec section; fine for now, upgrade for GA.
  3. any[] on inputSchemaStripLogs (SingleAgentClient.ts:1267, 2274) matches the adjacent v25DriftLogs: any[] convention — consistent, but a DebugLogEntry type would be stronger.

LGTM. Follow-ups noted below.

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. Clean backport of b4defa5 from main. Follow-ups noted below.

The fix is the right shape: stripped-field signals already existed via console.warn, so this is witness, not translator — surfaces what is happening into structured debug_logs and runner notices so compliance JSON output stops hiding the signal. Pre-polling preservation in task-map.ts is the load-bearing piece — waitForCompletion replacement was the path that silently dropped them.

Things I checked:

  • Diff equivalence vs upstream b4defa5 on main — same fix; only delta is src/lib/version.ts LIBRARY_VERSION 7.11.2 -> 7.11.5, which is the auto-generated file (header @generated by scripts/sync-schemas.ts) syncing to the branch package.json (already at 7.11.5). Not a manual package.json edit.
  • Changeset present (patch) and type is correct — additive notice surfacing, no API breakage.
  • collectInputSchemaFieldStripNotices at src/lib/testing/storyboard/runner.ts:1444 — defensive type-narrowing on debug_logs is correct, fields filtered to strings, empty-fields entries dropped.
  • replacedByPolling flag in src/lib/testing/storyboard/task-map.ts:135-150 — pre-polling logs only merged when polling actually replaced the result. Happy path (sync completion) is unchanged.
  • New NoticeCode input_schema_field_stripped added to the union in types.ts:1636 — StoryboardStepResult.notices? is optional and will not break existing readers.
  • No src/lib/protocols/** changes, no adapter/wire behavior changes, no fabrication. The stripping behavior itself is unchanged.
  • Tests cover sync and async polling paths; request-validation.test.js asserts the structured debug_logs shape (code, task, fields).

Follow-ups (non-blocking — file as issues):

  • Multi-pass notice aggregation (runner.ts:2779). Pre-existing: noticesDedup keeps the first pass storyboard_ids and drops the rest. For this PR it is a no-op (multi-pass runs one storyboard.id), but the comment "notices are identical across passes" was written for capability-derived notices and is now load-bearing in a way it was not before. Worth a follow-up to switch to mergeRunnerNotices(passResults.flatMap(r => r.notices)) so step-derived notices accumulate cleanly if multi-pass ever spans storyboards.
  • debug_logs duplication in task-map.ts:151. The spread [...prePollingDebugLogs, ...debugLogs] will double-list any log the completion frame echoes back. The notice collector dedups by (task, fields) so the strip notice is safe, but other adopters iterating raw debug_logs would see duplicates. Cheap fix: dedup by (type, message, timestamp).
  • Async test does not exercise the merge. test/lib/storyboard-notices.test.js async case verifies the pre-polling log survives, but the polled result has no competing debug_logs. A regression that silently drops prePollingDebugLogs would pass the test. Add a strip log on the completion frame and assert exactly one notice.

Minor nits (non-blocking):

  1. Dedup key is order-sensitive. Joining task and fields produces different keys for [a,b] vs [b,a]. stripped is populated by Object.entries(adapted) iteration order so it is stable in practice, but sorting fields before joining is a one-liner that removes the assumption.
  2. debugLogs?: any[] out-param at SingleAgentClient.ts:1332. Matches the surrounding v25DriftLogs pattern, so consistent — but a typed DebugLogEntry[] (or returning { adapted, debugLogs }) would tighten the surface. Pre-existing shape, not blocking.
  3. Notice message could name the remediation path. docs_url points at adcp#5495, but the message itself does not tell the adopter what to fix beyond "fix the tool schema declaration." Naming inputSchema explicitly would close the loop.

Approving.

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.

2 participants