fix(client): remove account_from_brand fabrication shim in normalizeRequestParams#1683
Merged
Merged
Conversation
…equestParams (#1676) Replaces the deprecated v2 compat shim that silently set account.operator = brand.domain with a ValidationError when create_media_buy omits account. https://claude.ai/code/session_01WYQpHoctUsoEBa2wuSpnY4
- Qualify sync_accounts as implicit-account sellers only in error message - Explicitly document both widened guard cases in changeset https://claude.ai/code/session_01WYQpHoctUsoEBa2wuSpnY4
This was referenced May 11, 2026
Closed
Contributor
Author
|
Acknowledged — merge dependency on #1684 (visibility keystone) noted. This PR will stay draft until #1684 lands; #1681 and this can ship in either order after that. Generated by Claude Code |
…ove-account-from-brand-shim # Conflicts: # src/lib/version.ts
This was referenced May 11, 2026
bokelley
added a commit
that referenced
this pull request
May 11, 2026
…lidationError import Adds eight regression tests for the new issues[] field landing in #1694: - L3 structuredContent path forwards well-formed issues with pointer/message/keyword - Malformed items (non-string fields, missing keys) are dropped - All-malformed input → field absent, never [] - Wire field absent → undefined - Wire field non-array → undefined - details + issues are orthogonal (both can co-exist) - schemaPath preserved when present - L3 path drops bad items by the same rule Also drops a duplicate `import { ValidationError }` in `request-normalizer.ts` introduced by the squash-merge collision of #1681 + #1683 — both PRs added the import from slightly different paths and the squash produced two identical imports, breaking the build on main. Fixed here so #1699 builds clean; the fix lands on main when this PR merges. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
May 11, 2026
Two reviewer-flagged gaps:
1. `test/lib/storyboard-account-invariant.test.js` gains a loader test
asserting `omit_account: true` without `expect_error: true` throws at
parse time (was previously enforced in code but uncovered by tests).
2. `test/request-normalizer-package-params.test.js` adds `account: {
account_id: 'test-acc' }` to both fixtures. After #1683 landed the
account-required throw runs before normalizePackageParams, so the
package-shape gates (#1681) need an account in the fixture to be the
gate under test.
bokelley
added a commit
that referenced
this pull request
May 11, 2026
bokelley
added a commit
that referenced
this pull request
May 11, 2026
…ErrorInfo (#1699) * feat(client): add AdcpValidationIssue type and issues[] field on AdcpErrorInfo Adds `AdcpValidationIssue` (pointer/message/keyword/schemaPath?) and `issues?: AdcpValidationIssue[]` to `AdcpErrorInfo` and `ExtractedAdcpError`, populated by `extractAdcpErrorInfo` and `buildExtracted` from the seller's VALIDATION_ERROR envelope. Previously the spec's `issues[]` array landed in the free-form `details` field; consumers had to read `details.validation_errors` as a convention. Now it surfaces as a typed field that LLM self-correction loops can read directly via `failure.adcp_error.issues[].pointer` and `.keyword`. Closes #1694. https://claude.ai/code/session_01RxPkjDrwRRT8TNW7U4ShHQ * test(error-extraction): cover issues[] forwarding + drop duplicate ValidationError import Adds eight regression tests for the new issues[] field landing in #1694: - L3 structuredContent path forwards well-formed issues with pointer/message/keyword - Malformed items (non-string fields, missing keys) are dropped - All-malformed input → field absent, never [] - Wire field absent → undefined - Wire field non-array → undefined - details + issues are orthogonal (both can co-exist) - schemaPath preserved when present - L3 path drops bad items by the same rule Also drops a duplicate `import { ValidationError }` in `request-normalizer.ts` introduced by the squash-merge collision of #1681 + #1683 — both PRs added the import from slightly different paths and the squash produced two identical imports, breaking the build on main. Fixed here so #1699 builds clean; the fix lands on main when this PR merges. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style(error-extraction): prettier — single-line filter callback Pure formatting fix flagged by CI. * fix(test): pass account in pre-3.0 package shape tests Same fix as on #1698's branch — after #1683 landed, create_media_buy requires account; the package-shape validators (#1681) need an account fixture to be the gate under test. --------- Co-authored-by: Claude <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
May 11, 2026
…ible via ComplianceResult (#1698) * docs(AdcpErrorInfo): warn sellers that message/details are grader-visible `adcp_error.message` and `.details` from seller responses are forwarded into `ComplianceResult.failures[].adcp_error` (landed in PR #1684) — a grader-visible archive surface that outlives the request. Three touches: - JSDoc on `AdcpErrorInfo.message` and `.details` in ConversationTypes.ts with an explicit seller-side warning (grader-visible, don't embed tokens or internal IDs) - New subsection §4 "Compliance failure envelopes (`adcp_error`)" in `docs/guides/CTX-METADATA-SAFETY.md` covering what flows where, what to avoid, and cross-linking to pickSafeDetails - Two-sentence addition in `skills/build-decisioning-platform/advanced/ REFERENCE.md` pickSafeDetails section noting the compliance-record leak class alongside the live-buyer-response leak class Closes #1697 https://claude.ai/code/session_011YZtF1KAFTD54e85so2KjN * fix(docs): correct relative path in REFERENCE.md cross-link Three ../ hops not four from skills/build-decisioning-platform/advanced/ to repo root. https://claude.ai/code/session_011YZtF1KAFTD54e85so2KjN * fix: drop duplicate ValidationError import in request-normalizer The squash-merge of #1681 (`from '../errors/index'`) and #1683 (`from '../errors'`) into main produced two identical-name imports. TS2300 breaks the build on every PR branched off the post-merge main. Single import resolves both. * fix(test): pass account in pre-3.0 package shape tests After #1683 landed, `normalizeRequestParams('create_media_buy', …)` requires an explicit account. Add `account: { account_id: 'test-acc' }` to the two test fixtures so the package-shape validators (#1681) are still the gate under test. --------- Co-authored-by: Claude <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
May 11, 2026
…applicable in ComplianceResult (#1701) * feat(conform): split storyboards_missing_tools from storyboards_not_applicable (#1695) Adds ComplianceResult.storyboards_missing_tools to distinguish storyboards filtered because the agent declared the protocol but a required tool was absent from storyboards_not_applicable (version-gated, protocol not declared). Also fixes pre-existing duplicate ValidationError import in request-normalizer.ts. Closes #1695 https://claude.ai/code/session_01J4JR1oK6rvbGZstZxZL4yG * feat(conform): update complyImpl to route required_tools stubs to storyboards_missing_tools Split the notApplicable array into two: version-gated entries remain in notApplicable (→ storyboards_not_applicable) and required_tools-filtered entries go to missingToolStoryboards (→ storyboards_missing_tools). Part of #1695 https://claude.ai/code/session_01J4JR1oK6rvbGZstZxZL4yG * fix(comply): restore escape sequences in sanitizeAgentText and separator widths Revert two incidental prettier-formatting changes that crept in during the previous commit: 1. sanitizeAgentText regex: prettier had converted explicit Unicode escape sequences ( --) into embedded literal bidi control characters. Reverted to escape sequences — the function strips these characters and having them appear as invisible literals in the source makes security review much harder. 2. Section separator comments: prettier trimmed the ─────── bars from 60 to 56 characters. Restored original width to preserve existing style. * fix(comply): restore escape sequences in sanitizeAgentText and separator widths Revert two incidental prettier-formatting changes from the previous commit: 1. sanitizeAgentText regex: prettier had converted explicit Unicode escape sequences ( --) into embedded literal bidi control characters. Reverted to named escape sequences — the function strips these chars; having them embedded invisibly in security-sensitive source makes code review much harder. 2. Section separator comments: prettier trimmed the ─── bars from 60 to 56 characters. Restored original width. * test: encoding verification * fix(comply): restore \uXXXX escape sequences in sanitizeAgentText regex Revert incidental prettier change that converted Unicode escape sequences to embedded literal bidi control characters in the sanitizeAgentText regex. The function strips these characters; having them appear as invisible literals in security-sensitive source makes code review much harder. Also restores the section separator comments from 56 to 60 dashes (also prettified in the same pass). * chore: remove accidental test artifact * fix(conform): bump changeset to major, document skip_causes for tool names * fix(test): pass account in pre-3.0 package shape tests Same fix as on #1698/#1699/#1700 — after #1683 landed, create_media_buy requires account. Add `account: { account_id: 'test-acc' }` to both fixtures so the package-shape gates (#1681) remain the gate under test.
bokelley
added a commit
that referenced
this pull request
May 11, 2026
…1696) (#1700) * feat(conformance): add omit_account escape hatch to StoryboardStep (#1696) Follow-up to PR #1683 which removed the account_from_brand shim. Storyboard schema_validation steps that deliberately test seller-side missing-account rejection can no longer reach the seller because the SDK's client-side ValidationError short-circuits before the wire call. Mirrors the existing omit_idempotency_key pattern: a step-level boolean that suppresses account injection in applyBrandInvariant (both synthetic-construction and natural-key-merge branches) and the normalizeRequestParams/validateRequest account-required checks (via skipAccountValidation on TaskOptions). For non-A2A runs, also triggers the raw-probe defense-in-depth path. https://claude.ai/code/session_01AW5JtCv7bVCVj17PHspMY8 * style: fix prettier formatting in storyboard-account-invariant test https://claude.ai/code/session_01AW5JtCv7bVCVj17PHspMY8 * fix(normalizer): remove duplicate ValidationError import in request-normalizer https://claude.ai/code/session_01AW5JtCv7bVCVj17PHspMY8 * fix(conformance): address pre-PR review feedback on omit_account escape hatch - Clarify in SingleAgentClient comments that skipAccountValidation bypasses the entire Zod schema parse (matching skipIdempotencyAutoInject behavior), not just the account field — both sites updated - Add loader co-requirement: omit_account: true now requires expect_error: true (loader throws at parse time; accountless create_media_buy always fails) - Extend omit_account JSDoc with: no-op note for non-create_media_buy tasks, request-builder caveat for future update_media_buy use, expect_error pairing rule - Add ordering-anchor comment in runner.ts noting testsMissingAccount must remain after applyBrandInvariant call https://claude.ai/code/session_01AW5JtCv7bVCVj17PHspMY8 * chore: update generated wire-spec-fields timestamp and package-lock Auto-generated by build:lib (timestamp only change); package-lock refreshed by npm install run during CI setup. https://claude.ai/code/session_01AW5JtCv7bVCVj17PHspMY8 * test: cover omit_account loader co-req + fix package-params fixtures Two reviewer-flagged gaps: 1. `test/lib/storyboard-account-invariant.test.js` gains a loader test asserting `omit_account: true` without `expect_error: true` throws at parse time (was previously enforced in code but uncovered by tests). 2. `test/request-normalizer-package-params.test.js` adds `account: { account_id: 'test-acc' }` to both fixtures. After #1683 landed the account-required throw runs before normalizePackageParams, so the package-shape gates (#1681) need an account in the fixture to be the gate under test. --------- Co-authored-by: Claude <noreply@anthropic.com>
Merged
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
2 participants
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.
Closes #1676
Summary
normalizeRequestParamscontained a v2 backward-compat shim that silently setaccount.operator = brand.domainwhen acreate_media_buycall omittedaccount. This PR removes it and throwsValidationErrorinstead.Why the shim was wrong:
account.operatoridentifies the billing account's operator (e.g.pinnacle-media.com), not the brand's own domain — the fabricated value was semantically correct only for direct-buy topologies and wrong for any agency/DSP-intermediated call.accountis a caller bug, not a back-compat case. The normalizer was masking it.create_media_buyaccount semantics.Behavior change (two cases):
create_media_buywithbrand.domainbut noaccount— previously fabricated{ brand, operator: brand.domain }; now throwsValidationError(field='account').create_media_buywith neitheraccountnorbrand— previously fell through to seller-side schema rejection; now throwsValidationErrorclient-side.Note on
executeAndHandlevsexecuteTask:executeTask(generic method) wraps all errors in aTaskResult.failedreturn value.executeAndHandle(used by typed methods likecreateMediaBuy) does not —ValidationErrorpropagates as a thrown exception from typed methods, consistent with howthis.validateRequestalready throws from that path. This pre-existing asymmetry is not addressed in this PR; it's documented here as a known limitation. Callers of typed methods shouldtry/catchfor pre-flight validation errors.Known follow-up: The
schema_validationstoryboard step that deliberately sendscreate_media_buywithoutaccount(to test seller error handling) needs anomit_accountescape hatch analogous toomit_idempotency_key— otherwise it tests client-side error behavior rather than seller-side. That storyboard path was already broken by the old shim (shim was fabricating account, seller never saw the missing-account case). Follow-up needed in storyboard runner.What was tested
npm run format:check— passednpm run typecheck— pre-existing errors unrelated to this diff (TS2688: @types/node,TS5107: moduleResolution=node10 deprecated); both present onmainbefore this changedist/pre-seeded with compiled changes; full build requiresnpm install(CI will verify)test/lib/request-normalizer.test.jsadded with 7 cases covering throw + no-throw paths; full test suite requiresnpm install(CI will verify)Pre-PR review:
executeAndHandleno-catch (pre-existing, documented above) and missing tests (tests are attest/lib/request-normalizer.test.js)operator = brand.domainis precisely wrong perAccountReferencediscriminated union; condition widening is correct (explicitly documented in changeset); qualifiedsync_accountsguidance to implicit-account sellers only in error messageNits (not fixed):
ValidationErrorconstraintparameter receives a recovery sentence rather than a pure constraint description — pre-existing pattern issue in the error class, out of scope for this PR.Session: https://claude.ai/code/session_01WYQpHoctUsoEBa2wuSpnY4
Generated by Claude Code