Skip to content

compliance(storyboards): contradiction lint + contributes shorthand (#2634)#2661

Merged
bokelley merged 4 commits into
mainfrom
bokelley/contradiction-lint
Apr 21, 2026
Merged

compliance(storyboards): contradiction lint + contributes shorthand (#2634)#2661
bokelley merged 4 commits into
mainfrom
bokelley/contradiction-lint

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

  • Closes #2634 rules 1 (cross-storyboard contradictions) and 2 (orphan contributions, peer completeness).
  • Dogfoods the contributes: true shorthand from adcp-client#693 (ships in @adcp/client 5.8.0). Migrates the two branch-set phases end-to-end.
  • Rule 3 (field-entity-context) filed as Storyboard field-entity-context lint (rule 3 from #2634) #2660 — needs an upstream schema-annotation pass before it's tractable.

Contradiction lint

scripts/lint-storyboard-contradictions.cjs groups every step-with-assertions across all storyboards by:

  • task
  • Request fingerprint — canonical JSON of sample_request; strips idempotency_key + context.correlation_id; normalizes \$generate:*, \$context.*, {{prior_step.*}}, {{runner.*}} substitutions. Fixed a subtle bug in the first pass where JSON.stringify(value, arrayOfKeys) silently dropped nested fields because the key-filter applies at every depth.
  • State-path fingerprint — ordered (task, request_fp) of prior mutating steps in the same phase + all earlier non-optional phases. The classic double-cancel pattern (first cancel → success, second cancel → NOT_CANCELLABLE) now has distinct state paths and doesn't collide.
  • Env fingerprintdoc.id, comply_scenario, auth override, prerequisites.controller_seeding. Independent storyboard suites don't false-positive; two steps in the same suite with different scenarios are legitimately independent.

Flags groups whose outcomes disagree in a way no conformant agent can satisfy:

  • success ↔ error (non-empty codes)
  • disjoint error-code sets

Branch-set peers (same storyboard, same branch_set.id) are exempt by design — any_of semantics intentionally asserts mutually exclusive outcomes.

Contributes shorthand

@adcp/client 5.8.0 resolves contributes: true inside a branch_set: phase to the enclosing phase's branch_set.id. Three new branch-set lint rules mirror the runner's loader so authors see the violation at build time:

  • contributes_both — step can't declare both contributes and contributes_to
  • contributes_outside_branch_setcontributes: true needs a branch_set to resolve to
  • contributes_bad_type — must be boolean

Migrated universal/schema-validation.yaml (past_start_handled) and universal/security.yaml (auth_mechanism_verified) to the shorthand.

Orphan-contribution rule

A fourth new branch-set rule: orphan_contribution — any contributes_to / contributes: true whose resolved flag has no matching assert_contribution any_of anywhere in the same storyboard is dead code. Complements no_assertion and peer_not_declared from #2646.

Test plan

  • npm run test:storyboard-contradictions — 14 tests (source guard + fingerprint / outcome / state / env / peer-exemption coverage)
  • npm run test:storyboard-branch-sets — 14 tests (4 new for the contributes + orphan rules)
  • npm run build:compliance — all three lints run in sequence (scoping, branch-sets, contradictions)
  • Precommit (npm run test:unit + npm run typecheck) — 631 tests pass
  • Planted a synthetic contradiction; lint catches it and prints location + outcome shapes

Follow-up: #2660 (rule 3 field-entity-context lint, blocked on schema annotation work).

🤖 Generated with Claude Code

bokelley and others added 2 commits April 21, 2026 10:47
…2634)

Closes #2634 rules 1 and 2. Rule 3 (field-entity-context) is filed as
#2660 — requires an upstream schema-annotation pass before the lint is
tractable.

Contradiction lint (scripts/lint-storyboard-contradictions.cjs):
- Groups every step-with-assertions across all storyboards by
  (task, canonicalized request fingerprint, prior-state fingerprint,
  env fingerprint) and flags groups whose outcomes disagree in a way
  no conformant agent can satisfy (success vs error, or disjoint
  error-code sets).
- Fingerprinting strips idempotency_key and context.correlation_id,
  normalizes $generate / $context / {{prior_step}} / {{runner}}
  substitutions, and hashes a stable stringification (fixed a bug
  where JSON.stringify with an array replacer silently dropped nested
  fields).
- State path is computed from prior mutating steps in the same phase
  and earlier non-optional phases, so double-cancel (first success +
  second NOT_CANCELLABLE) no longer collides.
- Env fingerprint includes doc.id so independent storyboard files
  don't false-positive across controller-seeded state boundaries.
- Branch-set peers (same storyboard, same branch_set.id) are exempt
  by design — any_of intentionally asserts mutually exclusive
  outcomes.
- Wired into build:compliance and npm test; 14 synthetic-fixture
  tests cover fingerprint, outcome classification, state
  discrimination, env scoping, and peer exemption.

Contributes shorthand (adcp-client#693, @adcp/client 5.8.0):
- Schema accepts `contributes: true` inside a branch_set phase; the
  runner's loader resolves it to phase.branch_set.id.
- Extended lint-storyboard-branch-sets with three new rules mirroring
  the loader's checks: contributes_both, contributes_outside_branch_set,
  contributes_bad_type. Fail-fast at build time instead of storyboard-
  load time.
- Added orphan_contribution: any contributes_to / contributes: true
  whose resolved flag is never consumed by an assert_contribution
  any_of is dead code. Complements no_assertion and peer_not_declared.
- Migrated schema-validation.yaml (past_start_handled) and
  security.yaml (auth_mechanism_verified) to the shorthand end-to-end,
  dogfooding the spec + runner + lint stack.

Bumps @adcp/client to ^5.8.0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lints

@adcp/client 5.8.0 introduced stricter response-schema validation that
regresses ~9 unrelated storyboards (list_creative_formats formats[16..25]
oneOf mismatches in assets[0], validate_property_delivery additional-
properties). Training Agent Storyboards CI drops from 35/279 to 27/271
on the legacy matrix and 21/237 to 19/226 on framework — below both
regression floors.

The adoption is independent of this PR's goal (#2634 contradiction lint).
Unbundle to keep CI green and file follow-up for the 5.8.0 upgrade.

Changes:
- Pin @adcp/client to ^5.6.0 (matches main).
- Revert `contributes: true` shorthand migration in schema-validation.yaml
  and security.yaml back to `contributes_to: <flag>`.
- Schema doc notes the shorthand exists in 5.8.0+ but this repo stays on
  the string form until the runner upgrade is unblocked.

Kept: contradiction lint, orphan_contribution rule, defensive branch-set
lint rules for the shorthand (contributes_both,
contributes_outside_branch_set, contributes_bad_type). These are static
checks that don't require 5.8.0 at runtime — they protect authors who
start using the shorthand before the runner rolls forward.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n-lint

# Conflicts:
#	package.json
#	scripts/build-compliance.cjs
Two fixes from post-merge expert review of #2661.

1. Contradiction fingerprint now handles Date objects. `yaml.load` parses
   unquoted ISO timestamps (YAML 1.1) into JS Dates; `stableStringify`
   was emitting `{}` for them, so two steps with different `start_time`
   values would silently collide. Added a Date branch in
   `normalizeRequestValue` that converts to ISO string. New regression
   test locks in the behavior.

2. Changeset reflects the landed state. Rule 3 (field-entity-context)
   was filed as #2660 during this PR and shipped separately via #2668's
   context-entity lint; the changeset's previous language treating it
   as an open follow-up was stale.

Protocol follow-ups filed (not in this PR):
- #2669: derive MUTATING_TASKS from request-schema annotations
- #2670: include test_kit + top-level fixtures in env fingerprint; plan
  sb=doc.id removal
- #2671: resolve requires_scenarios before flagging orphan_contribution

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit b54c681 into main Apr 21, 2026
15 checks passed
bokelley added a commit that referenced this pull request Apr 21, 2026
… fingerprint (#2670 pt2) (#2714)

* compliance(storyboards): drop sb=<doc.id> from contradiction lint env 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>

* compliance(storyboards): resolved-auth fingerprint shape (#2708 + #2711) (#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>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

Storyboard contradiction lint — catch peer-storyboard contradictions pre-merge

1 participant