Conversation
This was referenced May 1, 2026
Closed
…onger cascades The F6 deferred-cascade fix (6.1.0) still fired the cascade when a phase had only one stateful step returning not_applicable and no peer substitute existed. This collapsed 1/9/0 on sales_social for citrusad, amazon, criteo, and google — adapters that manage account state implicitly and have no list_accounts peer in their account_setup phase. Cascade now only promotes to a hard cascade when OTHER stateful peers existed in the phase (peer-substitute path was available but failed to establish state). Sole-stateful-step not_applicable phases are treated as "platform uses a different account model" — state established implicitly. Closes #1144 https://claude.ai/code/session_011LYCJuRwJHHFqzX2yxcAfi
10dab75 to
d464202
Compare
Contributor
Author
Admin merge rationaleSame `Test & Build` failures as every recent main run: `storyboard adcp#3083` + `a2a_context_continuity`. Both pre-existing, tracked at #1178. All other CI lanes pass. |
Contributor
Author
|
Acknowledged — pre-existing Generated by Claude Code |
Contributor
Author
|
Understood — #1178 has the tracking surface for those pre-existing failures. Nothing to do here. Generated by Claude Code Generated by Claude Code |
Contributor
Author
|
Acknowledged — #1178 noted as the tracking surface for the pre-existing failures. No further action on this PR. Generated by Claude Code Generated by Claude Code |
This was referenced May 1, 2026
bokelley
added a commit
that referenced
this pull request
May 4, 2026
…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>
bokelley
added a commit
that referenced
this pull request
May 4, 2026
…ade (#1545) * fix(storyboard): sole stateful step missing_tool no longer trips cascade 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> * refactor(storyboard): simplify hasStatefulPeers + flatten branch 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> * feat(storyboard): surface sole-stateful exemption + parity invariant 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> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
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>
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
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 #1144
Summary
The F6 cascade-skip fix (6.1.0) deferred
not_applicablecascade decisions to phase end, checking if any peer stateful step established substitute state. This worked for snap (sync_accounts: not_applicable+list_accounts: passes) but still cascaded for adapters with a single stateful step in the phase and no peer-substitute — citrusad, amazon, criteo, and google all showed1/9/0onsales_social, every step aftersync_accounts: not_applicablecollapsing toprerequisite_failed.Root cause: The phase-end condition
phasePendingNotApplicable && !phaseEstablishedStatefulState && !statefulFailedcould not distinguish "peer existed but failed to establish state" from "no peer ever existed because the platform manages accounts implicitly." Both cases produced the same cascade.Fix: Compute
phaseStatefulStepIdsat phase initialization (phase.steps.filter(s => s.stateful)). At phase end, cascade only fires whenphaseStatefulStepIds.some(id => id !== phasePendingNotApplicable.stepId)— i.e., there were other stateful steps that could have served as substitutes but didn't. When thenot_applicablestep is the sole stateful step in the phase, no cascade fires: the platform is legitimately using a different account model, and downstream steps should run.Protocol basis:
not_applicablemeans "this path doesn't apply to this agent." When no peer-substitute path exists in the storyboard phase, there is nothing for the cascade to protect — the platform's account state is managed implicitly, and downstream assertions are independently valid.What was tested
npx tsc --project tsconfig.lib.json— pre-existingTS2688/TS5107only (confirmed onmainbefore this branch)node --test test/lib/storyboard-cascade-skip-on-skip.test.js— 11/11 pass (was 9/11 before fix; 2 tests updated to reflect corrected behavior)npm run test:lib— 166 fail / 18 cancelled (baseline onmain: 168 fail / 18 cancelled — net improvement, no regressions introduced)Test changes
phaseStatefulStepIdsfilter, so the sole-stateful-step rule appliesPre-PR review
phase.stepsis loader-normalized before runner receives it (not reachable undefined in practice); nits fixed (comment "lazily" → "eagerly", test title clarified)not_applicableon sole stateful step is semantically equivalent to "platform doesn't use this sync pathway";patchchangeset classification correctSession: https://claude.ai/code/session_011LYCJuRwJHHFqzX2yxcAfi
Generated by Claude Code