compliance(storyboards): drop sb=<doc.id> from contradiction lint env fingerprint (#2670 pt2)#2714
Merged
Merged
Conversation
… fingerprint (#2670 part 2) Including the storyboard id was conservative-by-design during the lint's initial rollout (#2661) but suppressed the exact class of bug the lint was built to catch — cross-storyboard contradictions (#2627, #2628, #2629). Prerequisites for the removal are now in place: #2679 added test_kit + top-level fixtures precision, PR #2710 (from #2684) audited principal identity and added role= as the forward guard, and #2708 tracks the remaining auth= structural gap. Experiment on the current 56-storyboard suite: removing sb= surfaces 10 multi-member groups (7 spanning ≥2 files), including 25 files agreeing that get_adcp_capabilities returns success on acme-outdoor and 6 doing the same on nova-motors. Zero contradictions — the suite is genuinely consistent and the lint now exercises cross-storyboard checks as originally intended. Inverts one test (no-contradiction-when-ids-differ → contradictions- surface-when-ids-differ-but-env-matches) to pin the new behavior, adds a complementary negative test (different test_kit + different ids → no contradiction) to pin test_kit= as a cross-file separator now that sb= is gone, and updates the `test_kit discriminates env` comment to note test_kit= is its sole discriminator post-removal. Protocol review surfaced a residual hole that sb= removal exposes: steps inheriting the transport's default auth emit no auth= component, so two storyboards with different transport defaults could collide. Out of scope here; tracked as #2711. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
…) (#2723) Extracts auth emission into a describeStepAuth() helper that always returns a stable token. Fixes two structural gaps that removing sb=<doc.id> (#2670 part 2) exposed: #2711 — inherited-default steps (336 of 340 in the current suite) used to emit no auth= component, so they could collide silently with explicit steps authored against divergent transport defaults. Now emits auth=kit_default. #2708 — auth.from_test_kit was read as boolean regardless of whether it was true or a string path, collapsing distinct principals in multi-principal kits. Now emits auth=<type>:from_test_kit:<path> for string form. Literal auth.value is sha1-8hex hashed so different literals don't collide (defense-in-depth; #2720 tracks flagging literals as a code smell). Zero grouping change on the current suite — no storyboard uses string-form from_test_kit or literal value; all 336 inheriting steps get the new kit_default token uniformly. Tests: - describeStepAuth shape-matrix unit test, every branch including defensive fallbacks. Literal hashes pinned to precomputed sha1 values for stability. - Inherited-default cross-storyboard test asserting kit_default emission and contradiction surface when outcomes disagree. - Multi-principal forward-guard test pinning that from_test_kit path selection discriminates, with direct fingerprintEnv comparison asserting the path token appears verbatim. Protocol review surfaced one structural question deferred as #2721: runner test kits (webhook-receiver, substitution-observer, signed-requests) have no auth: block — if a storyboard ever composes runner + primary kit, kit_default becomes ambiguous. No storyboard does that today. 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
Closes the #2670 planned evolution (part 2): removes
sb=<doc.id>from the contradiction lint's env fingerprint so cross-storyboard contradictions actually surface. Builds on PR #2710 (#2684,role=forward guard) which was the prerequisite audit.Why now
Including the storyboard id in the fingerprint was conservative-by-design during the lint's initial rollout (#2661) but suppressed the exact class of bug the lint was built to catch — #2627, #2628, #2629 all shared the shape "two locally-valid storyboards encoding disagreeing required responses for the same request." The richer env fingerprint is now sufficient:
test_kit+ top-levelfixturesrole=<doc.caller.role>auth=identity-vs-strategy gap (out of scope here)sb=removal exposes (out of scope, deferred)Experiment result
On the current 56-storyboard suite, after removing
sb=:get_adcp_capabilities → successagreed across 25 files onacme-outdoor.yamlget_adcp_capabilities → successagreed across 6 files onnova-motors.yamlsync_accounts → successacross 8 files;get_productspairs across scenarios;list_creative_formatsacross creative-generative + creative-templateChanges
scripts/lint-storyboard-contradictions.cjs— removed thesb=<doc.id>push fromfingerprintEnv; removed thesbentry from the Components JSDoc; added a trailing "Note onsb=<doc.id>" paragraph explaining the removal and cross-referencing Contradiction lint env: include test_kit + top-level fixtures, plan sb=doc.id removal #2670 / Contradiction lint env fingerprint: evaluate adding principal identity before sb= removal #2684 / Contradiction lint env: auth= component hashes strategy, not principal identity #2708 so future readers aren't surprised by the absence.tests/lint-storyboard-contradictions.test.cjs— inverted one existing test to pin the new surfaceable behavior (length === 1, both files inmembers, outcome pair =['error', 'success']); added a complementary negative test (different test_kit + different ids → no contradiction); updated thetest_kit discriminates envcomment to note it's the sole discriminator post-removal..changeset/env-fingerprint-drop-sb-doc-id.md— descriptive changeset with the audit numbers and the Contradiction lint env: absent step.auth contributes nothing, transport-default inheritance can collide #2711 deferred-scope pointer.Expert review
Three rounds pre-push:
c.members(full group, stable) instead ofc.mismatch(picked pair, brittle under group-size changes); added an outcome-pair sort assertion to pin the kind of disagreement not just its existence; added the complementary negative test they recommended; updated thetest_kit discriminates envcomment so a future reader doesn't delete it as redundant.fingerprintEnvgates its auth emission onif (step.auth)— steps inheriting the transport's default auth contribute nothing, so two storyboards with different transport defaults can collide. Filed as #2711 with a concrete resolve-then-emit proposal; out of scope for this PR since no storyboard in the current suite exercises the case.Test plan
npm run test:storyboard-contradictions— 27/27 pass (1 inverted, 1 new)npm run build:compliance— all four storyboard lints green, 56 storyboards buildnpm run test:unit— 631/631 pass (precommit)npm run typecheck— clean (precommit).context/try-sb-removal.cjs) confirmed zero new contradictions + 7 cross-file groups before the source edit landed🤖 Generated with Claude Code