spec + compliance: pull forward three substitution-safety follow-ups to 3.0 GA (#2650, #2654, #2655)#2656
Merged
Conversation
#2650, #2654, #2655) Three follow-ups from the #2647 review cycle, originally milestoned 3.1, evaluated by expert reviewers (protocol + security + DX + code- reviewer) as pre-GA-appropriate: #2650 — Unicode NFC normalization pinned The #2620 rule did not pin a normalization form. Two implementations satisfying the unreserved-whitelist rule produced different bytes for the same visual string (café NFC = %C3%A9 vs NFD = e%CC%81). Shipping 3.0 without NFC would lock in interop breakage on any source path that emits NFD (macOS HFS+, Python defaults, mixed feeds). Spec: one normative paragraph in universal-macros.mdx. NFKC/NFKD explicitly not acceptable substitutes (compatibility folding mutates fullwidth/halfwidth variants in Japanese/Korean retailer catalogs). Fixture: added nfc-normalization-before-encoding vector. All 8 vectors verified under NFC-then-strict-encoding. #2654 — PHASE_TEMPLATE block Two consumers of substitution_observer_runner now exist with ~150 LOC near-identical phases. Added advisory phase_template: block to the observer contract YAML with <<PLACEHOLDER>> markers for the five fields that vary across specialisms. Block is YAML comment, not runtime-enforced — prevents silent drift on load-bearing fields (require_every_binding_observed: true, fixture-lookup binding shape, requires_contract) before the third consumer. #2655 — vector_name authoring-time lint New scripts/lint-substitution-vector-names.cjs (~120 LOC, mirrors lint-error-codes.cjs). Walks storyboards for task: expect_substitution_safe, extracts catalog_bindings[].vector_name, asserts each is canonical in static/test-vectors/catalog-macro-substitution.json. Cross-checks contract's canonical_vector_names against fixture — drift fails. Wired into aggregate test pipeline as test:substitution-vector-names. Caught a synthetic typo (reserved-character-break0ut) in sanity-test. Deferred: #2651 (sales-social observation hook) — PM review suggested a narrower pre-GA attestation approach as separate work; not bundled here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Schema Link Check ResultsCommit:
|
Security (BLOCKER fix): - Added nfc-normalization-before-encoding to the observer contract's canonical_vector_names list. Without this, the new fixture vector was present but the contract under-advertised coverage — the new lint script's contract-fixture drift check would flag it (as warn, not error, but the security reviewer caught the over-broad scope mismatch anyway). DX (#2654 PHASE_TEMPLATE strengthened): - Expanded placeholder list from 5 to 11: added BRAND_DOMAIN, OPERATOR_DOMAIN, PHASE_ID, BUILD_SCHEMA_REF, BUILD_RESPONSE_SCHEMA_REF, BUILD_COMPLY_SCENARIO, MESSAGE_BRIEF, IDEMPOTENCY_PREFIX. Fixed TARGET_FORMAT_ID to explicitly be an object shape (was ambiguous). Added account block, quality: draft, idempotency_key, correlation_id fields that appeared in both real consumers but were missing from the template. - Annotated the 3-vs-5 vector scope choice: "Minimum 3 vectors (canonical baseline). Specialisms with higher risk profile (generative, user-text-in-copy) SHOULD add crlf + bidi; template- shaped substitution specialisms MAY stay at 3." nfc_normalization and url_scheme_injection noted as opt-in. - Strengthened the require_every_binding_observed: true warning with explicit #2647 security reference and concrete bypass example ("a seller that emits `?sku=` (empty value) instead of substituting the macro passes the test with zero observed bindings"). Code-reviewer (lint polish): - Added 12-line maintainer comment above checkContractFixtureSync explaining the asymmetric severity (over-claim vs under-advertise) so future maintainers don't "fix" the error/warn split. - Added opt-in override_reason field: bindings with raw_value / expected_encoded overrides PLUS an override_reason: "..." field suppress the warning entirely. Without override_reason, warn still fires with an actionable message pointing to the suppression mechanism. Prevents training reviewers to ignore the warn. All validated: test:substitution-vector-names clean (0 errors), test:storyboard-scoping clean, test:error-codes clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 21, 2026
bokelley
added a commit
that referenced
this pull request
Apr 26, 2026
…emplate (#2654) (#2730) Closes the last acceptance item on #2654. The phase_template: block in static/compliance/source/test-kits/substitution-observer-runner.yaml and the lint:substitution-vector-names drift-guard (#2655) already shipped in PR #2656. This adds a "Adding a catalog-substitution-safety phase to a new specialism" section to storyboard-authoring.md so authors reaching for a fourth consumer find the template before they clone from a sibling. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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
Three follow-ups from the #2647 review cycle, originally milestoned 3.1.0, evaluated by expert reviewers as pre-GA-appropriate and bundled here.
PHASE_TEMPLATEblock for repeated substitution-safety phases (compliance, docs-only)vector_namereferences (compliance, new script)#2650 — NFC normalization pinned
The #2620 rule did not pin a normalization form. Two implementations that both satisfy the unreserved-whitelist encoding rule produce different bytes for the same visual string (
caféNFC =%C3%A9vscafe\u0301NFD =e%CC%81). Shipping 3.0 without this rule ships an interop hazard on any source path that emits NFD — macOS HFS+ decomposes filenames to NFD, Pythonunicodedatadefaults don't normalize, mixed-source feeds routinely combine both forms.Changes:
docs/creative/universal-macros.mdx#substitution-safety-catalog-item-macros: one normative paragraph added ahead of the existing percent-encoding rule. NFKC / NFKD explicitly called out as not acceptable (compatibility folding silently mutates fullwidth/halfwidth variants in Japanese/Korean retailer catalogs).static/test-vectors/catalog-macro-substitution.json: addednfc-normalization-before-encodingvector — valuecafe\u0301-amsterdam(NFD) → expectedcaf%C3%A9-amsterdam. Fixture header updated. All 8 vectors verified: NFC-normalize → strict-RFC-3986-encode reproduces eachexpectedbyte-for-byte.Protocol reviewer verdict: "pull-forward. NFC is the right form. Real interop risk, not theoretical. Minimum scope ~15 min."
#2654 — PHASE_TEMPLATE block in observer contract
Two consumers of
substitution_observer_runnernow exist (sales-catalog-driven, creative-generative) with ~150 LOC near-identical three-step phases. Before the third consumer copies-and-drifts on load-bearing fields (require_every_binding_observed: true, fixture-lookup binding shape,requires_contract), added an advisoryphase_template:block tostatic/compliance/source/test-kits/substitution-observer-runner.yaml.Block is a YAML comment with
<<PLACEHOLDER>>markers for the five fields that vary across specialisms: specialism slug, domain, catalog_id prefix, correlation_id prefix, template URL. Not runtime-enforced — deviation is legitimate; the value is that starting from the template avoids silent drift.DX reviewer verdict: "pull-forward option 1 only. Zero schema change, zero runner change, 30-60 min work. Options 2/3 (schema
include:, runner macro) stay 3.1."#2655 — vector_name lint
New lint at
scripts/lint-substitution-vector-names.cjs(120 LOC, mirrorsscripts/lint-error-codes.cjs):static/compliance/source/fortask: expect_substitution_safesteps.catalog_bindings[].vector_name, asserts the name is canonical instatic/test-vectors/catalog-macro-substitution.json.attacker_value_catalog.canonical_vector_namesagainst the fixture — drift between contract-declared names and fixture-present names fails the lint.raw_value/expected_encodedoverrides grade as warnings (custom vectors are opt-in per the contract).Wired into the aggregate
testpipeline asnpm run test:substitution-vector-names. Sanity-tested against a synthetic typo (reserved-character-break0ut): caught at build time as expected.Code reviewer verdict: "pull-forward. 60-90 min / ~120 LOC. Low risk (narrow predicate). Catches typos at build time rather than confusing 'vector not found' runner failures for third-party authors."
Out of scope
include:directive, first-class phase_type macro) — stay 3.1 per DX review.Test plan
npm run test:substitution-vector-names— clean (0 errors, 2 pre-existing YAML warnings shared with other lints)npm run test:storyboard-scoping— cleannpm run test:error-codes— clean (pre-existing YAML warning unrelated)npm run typecheckpassesReview lineage
All three items reviewed in the pull-forward evaluation round:
🤖 Generated with Claude Code