Skip to content

fix(storyboard): honor explicit tool gates before cascade - #3524

Open
sangilish wants to merge 1 commit into
adcontextprotocol:mainfrom
sangilish:fix/explicit-tool-gates-before-cascade
Open

sangilish wants to merge 1 commit into
adcontextprotocol:mainfrom
sangilish:fix/explicit-tool-gates-before-cascade

Conversation

@sangilish

Copy link
Copy Markdown
Contributor

Summary

  • Honor explicit requires_tool gates before a stateful cascade in ordinary single-agent execution, matching the routed path.
  • Keep controller-specific skip reasons and the existing called-tool advertisement check.
  • Add nine focused regression/control cases for same-phase and downstream consumers, supported execution, real failures, non-stateful parity, controller diagnostics, and zero-coverage protection.

Refs adcontextprotocol/adcp#8506.

Boundary

This is not a blanket conversion of missing-tool cascades into passing evidence. A genuinely failed prerequisite still fails its consumers when their declared tools are available, an earlier real failure still fails the storyboard, and an entirely tool-gated run cannot pass without executed coverage.

The paired storyboard source change must declare sync_creatives on the dependent provenance audit reads. The runner change alone does not repair the existing controller-only declarations. This also does not claim to resolve the broader fixture, hosted-diagnostic and coverage issues in adcontextprotocol/adcp#8576.

Validation

  • npm run build:lib
  • npm run typecheck
  • Nine focused cascade, skip-counting, requires, capability and routed suites: 349 tests passed.
  • Prettier check passed.
  • ESLint: zero errors; 24 warnings, also present on the unmodified runner at the base revision.
  • Changeset status, PR title validation and git diff --check passed.

The full monorepo suite and hosted grader were not run. Includes an SDK patch changeset; no protocol wire shape or generated release artifact is changed.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-30T03:18:56.771444Z
We couldn’t complete this review. Comment @codex review to try again.
ba0c26b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — clean patch with no blocking findings.

Checked: drops the routingContext && guard at runner.ts:6066 so a cascade-tripped stateful step honors its explicit requires_tool gate in single-agent runs, matching the routed and non-cascade paths (runner.ts:8668, same skip-reason mapping). The missingReason change preserves the controller-specific reason via step.requires_tool === missingTool. Boundaries hold — real failures still cascade when the declared tool is advertised, earlier real failures still fail the run, and zero executed coverage still fails. Nine regression tests cover same-phase/downstream/supported/failure/non-stateful/controller/zero-coverage branches. Changeset is a patch and appropriate — no wire or export change.

No critical/high/medium findings. gated_paths is false, high_risk is false, no author team gate. review_decision is REVIEW_REQUIRED but the gated-paths hard gate (row 2) only fires when gated_paths is true, which it is not. Falls through to row 9.

This branch has not been deployed

No deployments
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.

1 participant