Skip to content

recipes: denied approval gates can be bypassed by resume; renderer, README, and examples contradict the documented post-stage gate semantics #383

Description

@Joi

Filing here because the code lives in amplifier-bundle-recipes, where issues are disabled (per SUPPORT.md).

All references are to amplifier-bundle-recipes at 4c96b0b.

1. Bug: denial is not terminal — a second resume continues past a denied gate

_deny_stage records DENIED but then clears the pending approval (modules/tool-recipes/amplifier_module_tool_recipes/__init__.py:783-802). _resume_recipe (473–528) performs no denied-status check before re-invoking the executor, and the executor's DENIED check sits inside if pending (1107–1139) — which denial has just emptied. Since the staged state was saved with the next stage as the resume target before the approval was created (executor.py, the "Stage completed - check for approval gate" block at line 1353), a denied recipe that is resumed again continues from the saved next-stage index. Timeout denial has the same escape via session.py:446-450.

This contradicts the deny path's own "Recipe execution will not continue" message (__init__.py:811). Expected: _resume_recipe refuses to continue past a DENIED gate.

Repro (three stages so the denied gate has a following stage):

name: deny-bypass-repro
stages:
  - name: one
    steps:
      - id: s1
        type: bash
        command: "echo one"
  - name: two
    approval:
      required: true
      prompt: "Continue past stage two?"
    steps:
      - id: s2
        type: bash
        command: "echo two"
  - name: three
    steps:
      - id: s3
        type: bash
        command: "touch /tmp/should-not-exist-after-deny"

Run staged, DENY the gate at stage two, then invoke resume on the same session id: stage three runs and the file appears.

2. Consistency: three artifacts teach pre-stage gating the runtime does not have

RECIPE_SCHEMA.md is clear that gates are post-stage ("Stage completes → Recipe pauses", lines 365, 455, 468), and the executor matches it. But:

  • The diagram renderer draws the opposite. recipe_to_dot.py:552-600 inserts the gate node BEFORE its named stage, and test_recipe_to_dot.py:130-132 asserts start -> gate_stage_one. A generated diagram tells the author destructive stage work is pre-approved when it is not.
  • The root README documents step-level requires_approval / approval_message fields (README.md:197-209) that do not exist on Step (models.py:217-295); unknown keys reach Step(**step_data_copy) (models.py:675), so the example cannot parse. agents/result-validator.md:125-137 repeats the same nonexistent fields.
  • A shipped example promises deny-safety it does not have. examples/context-intelligence/synthesis/action-executor.yaml:327 says "If you DENY: No changes will be made", but that approval sits on the stage whose own (dry_run-conditional) steps create backups and edit files before the pause — with dry_run=false, the mutations precede the gate. examples/dependency-upgrade-staged-recipe.yaml:89-123 has the same prompt-versus-runtime ordering mismatch ("Approve to validate" on the stage that already validated).

The supported way to gate a stage's work is to put the approval on its predecessor (RECIPE_SCHEMA.md:323 shows exactly this). The renderer, README, and examples should say and show that — today they teach recipe authors the unsafe reading, and an author who follows them writes a consent-before-mutation contract the runtime cannot honor.

3. Suggestion, not a defect claim

Post-stage-only placement has two structural gaps: no gate before the first stage, and no gate before an arbitrary step. If pre-stage (or step-level) placement is ever added, the README's imagined requires_approval is roughly the right shape. Until then, tightening the three artifacts above to match the schema would prevent the misreading. There are also no tests covering stage-gate ordering or resume-after-denial; both are cheap to assert.

Happy to open a PR for the denied-resume check plus the renderer/README/example corrections and the missing tests — but item 1 changes resume behavior, so I wanted your call first.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions