fix(storyboard): sole stateful step missing_tool no longer trips cascade#1545
Merged
Conversation
Surfaced by adcp-client-python#550 (ProposalManager v1.5). A proposal-mode adopter doesn't advertise sync_accounts because account state materializes on the first get_products call. With sync_accounts as the SOLE stateful step in the storyboard's setup phase, the runner pre-fix tripped the cross-phase cascade on its missing_tool skip and collapsed every downstream stateful phase (refine_proposal, finalize_proposal, accept_proposal) to prerequisite_failed — masking real coverage on the framework paths the adopter does implement. Extends the sole-stateful-step exemption from #1146 (originally scoped to not_applicable) to hard-missing skip reasons. When missing_tool or missing_test_controller lands on the only stateful step in a phase and nothing declares peer_substitutes_for it, no peer could have established substitute state — so the runner treats this as "platform legitimately doesn't use this pathway" and lets downstream phases run on their own merits. Multi-stateful-step phases preserve existing behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per code-reviewer feedback on #1545: the skipping step is always stateful and always in phaseStatefulStepIds, so length > 1 ⇔ a stateful peer exists. Cheaper, self-documenting, and removes the empty-comment-only branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…test Two follow-ups on the cascade exemption (#1545): 1. **Diagnostic emission.** When the sole-stateful-step exemption fires (not_applicable / missing_tool / missing_test_controller on the only stateful step in a phase, no provides_state_for rescue), the skipping step's `skip.detail` is annotated with a greppable marker: "Sole stateful step exemption applied for phase '<phase_id>': ..." Adopters reading per-step output now see the runner's decision explicitly rather than inferring it from the absence of cascade-skips. 2. **Parity invariant test.** Adds a parameterized test that asserts identical cascade outcomes (downstream runs + exemption marker on skip detail) for every skip reason participating in the family. The bug behind #1545 was an asymmetric blind spot — #1146 fixed not_applicable, missing_tool was forgotten. This loop catches the next time the family grows and the wiring is forgotten. Spec coordination: adcontextprotocol/adcp#4053 to lift the exemption rule into runner-output-contract.yaml so adcp-client-python and Addie converge on the same behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
bokelley
added a commit
that referenced
this pull request
May 8, 2026
…r_shape × topology) matrix (#1548) (#1587) * chore(framework): narrow typed handler ctx fields to match dispatch guarantees (#1384) Sweep follow-up to #1327 for typed handler entrypoints in `src/lib/server/decisioning/`. Audited each ctx field surfaced via `RequestContext` against the dispatch reality in `runtime/to-context.ts` + `runtime/from-platform.ts`. Found one mismatch: both `CreativeBuilderPlatform` and `CreativeAdServerPlatform` declared `listCreativeFormats` twice — once correctly with `NoAccountCtx<TCtxMeta>` and once with `Ctx<TCtxMeta>` (claiming `account` non-optional). TS overload resolution preserved the narrow at the implementation site, so adopters were not silently exposed to a runtime crash, but the duplicate was a footgun for adopters reading the interface and a tripwire for future refactors that might delete the "wrong" one. Removed the `Ctx`-typed declarations on both interfaces, folded the precedence note into the surviving `NoAccountCtx` declaration on `CreativeBuilderPlatform`, and added regression locks in `decisioning.type-checks.ts` mirroring the `previewCreative` pattern from #1327. The rest of the audit (`ctx.agent`, `ctx.ctxMetadata`, `ctx.recipes`, `ctx.handoffToTask`, `ctx.state.*`, `ctx.resolve.*`) all match dispatch guarantees. `ctx.authInfo`, `ctx.emitWebhook`, and `ctx.publishStatusChange` are not on `RequestContext` (legacy `HandlerContext` or module-level exports) — out of scope. * test(storyboard): refactor cascade-skip-on-skip to (skip_reason × peer_shape × topology) matrix (#1548) Refactors test/lib/storyboard-cascade-skip-on-skip.test.js from prose-driven scenarios (~2000 lines, 35 named tests) to a fixture-table matrix keyed by the three orthogonal dimensions of cascade behavior: skip_reason ∈ { missing_tool, missing_test_controller, not_applicable, real_failure } peer_shape ∈ { sole_stateful, peer_passes, peer_fails, peer_substitute_declared, peer_substitute_missing } phase_topology ∈ { same_phase, downstream_phase } PR #1545 fixed an asymmetric blind spot in cascade behavior — the sole-stateful-step exemption applied to `not_applicable` (#1146) but not to `missing_tool`. The parallel review traced the cause to test architecture: prose-driven scenarios make empty cells in the implicit cube invisible until production hits one. This refactor surfaces those cells structurally. Each row is one fixture run end-to-end. A coverage-holes assertion at the bottom of the file partitions the 40-cell cube into covered (11), structurally impossible (4), tracked-elsewhere (25), and empty (0). Adding a new axis value or dimension fails the assertion until the cube is repartitioned — so the next asymmetric blind spot lands as a visible coverage gap, not a production bug. Preserves all 35 historical assertions (24 matrix rows + 1 control row + 9 parse-time validation rows + 1 parity invariant). Net line shrink ~41% (2143 → 1256) from extracting the repeated fakeAgent setup + storyboard scaffold into helpers. Test-only change. No runner behavior change. Closes #1548. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(storyboard): resolve cascade-skip matrix review blockers Two follow-ups from PR #1587 review: 1. The four `real_failure × peer_{passes,fails} × {same,downstream}_phase` cells previously cited a non-existent `storyboard-runner.test.js`. They are now first-class MATRIX_ROWS, exercising the within-phase cascade immediately-trips behavior (peer that runs after a real failure is itself cascade-skipped) and the cross-phase carry-through. Total rows grow from 36 to 40. 2. The vague `tracked_in_issue_1548_followup` markers (eleven cells) are retargeted at #1589, filed today, which lists the open contract questions per cell (substitute-rescue applicability for `not_applicable`, real-failure-wins ordering for `missing_tool` peers, and same/downstream asymmetry mirrors). `MATRIX_REPORT=1` now shows: covered=15, impossible=4, tracked=21, empty=0. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
6 tasks
bokelley
added a commit
that referenced
this pull request
May 8, 2026
…ller_adapter_proposal_mode (#1549) (#1594) Wires comply_test_controller into the proposal-mode reference adapter (seed.media_buy / force.media_buy_status / simulate.delivery, gated by the framework's mode==='sandbox' admission per the other reference sellers) and adds invariant assertions to the gate test that verify the SDK behavior PR #1545 introduced: sync_accounts skip carries the sole-stateful exemption marker AND none of the four downstream phases (brief_with_proposals, refine_proposal, finalize_proposal, accept_proposal) are cascade-skipped from the setup-phase skip. Closes the example-tier coverage gap nodejs-testing-expert flagged on PR #1545. The runner-level invariant test in the cascade-skip-on-skip matrix already covers the unit layer; this adds the end-to-end mirror of how proposal-mode adopters (adcp-client-python#550) actually exercise the SDK. The proposal lifecycle past brief_with_proposals stops on the balanced_reach_q2 placeholder gap tracked at adcp#4086 + draft PR #4088. The gate test acknowledges the failure via EXPECTED_FAILURES so the issue-#1549 invariants run against a clean grader output, and the defensive stale-allowlist check flips the gate red the moment that spec PR lands. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
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
Surfaced by adcp-client-python#550 (ProposalManager v1.5). A proposal-mode seller doesn't advertise
sync_accountsbecause account state materializes on the firstget_productscall. Withsync_accountsas the sole stateful step in the storyboard'ssetupphase, the runner pre-fix tripped the cross-phase cascade on itsmissing_toolskip and collapsed every downstream stateful phase (refine_proposal,finalize_proposal,accept_proposal) toprerequisite_failed— masking real coverage on the framework paths the adopter does implement.Fix
Extends the sole-stateful-step exemption from #1146 (originally scoped to
not_applicable) to hard-missing skip reasons. Whenmissing_toolormissing_test_controllerlands on the only stateful step in a phase and nothing declarespeer_substitutes_forit, no peer could have established substitute state — so the runner treats it as "platform legitimately doesn't use this pathway" and lets downstream phases run on their own merits.Multi-stateful-step phases preserve existing behavior: a
missing_toolskip with at least one stateful peer (and no rescue declaration) still trips the cascade.Diagnostic emission
When the exemption fires, the skipping step's
skip.detailis annotated with a greppable marker:Adopters reading per-step output now see the runner's decision explicitly rather than inferring it from the absence of cascade-skips.
Parity invariant test
A new parameterized test asserts identical cascade outcomes (downstream runs + exemption marker present) for every skip reason participating in the exemption family. Bug behind this PR was an asymmetric blind spot — #1146 fixed
not_applicable,missing_toolwas forgotten. The invariant loop catches the next time the family grows and the wiring is forgotten.Why this is the right call
ACCOUNT_NOT_FOUND,NOT_AUTHORIZED, etc.) — strictly more informative thanprerequisite_failedeverywhere.not_applicable. Both reasons mean "this pathway isn't supported." The asymmetry between them was an artifact of fix(conformance): sole-stateful-step not_applicable no longer cascades #1146 only addressing the cases the original report surfaced.Cross-runner spec coordination
Filed adcontextprotocol/adcp#4053 to lift the sole-stateful-step exemption (now covering
not_applicable+missing_tool+missing_test_controller) intorunner-output-contract.yamlas normative. Without spec language, the same proposal-mode adopter could pass against the TS runner and fail against the Python runner on the same storyboard.Test plan
sole stateful step missing_tool → no cascade (adcp-client-python#550)— verifies a proposal-mode-shaped storyboard runs downstream phases when sync_accounts skips with missing_tool.missing_toolandnot_applicable.tsc --noEmitclean.prettier --checkclean.Expert review
Reviewed in parallel by
ad-tech-protocol-expert,code-reviewer, andnodejs-testing-expert:hasStatefulPeerssimplified tolength > 1, branch flattened).🤖 Generated with Claude Code