From 393ac6b7101cf2a02729b5b395b9fe98ccbd10d8 Mon Sep 17 00:00:00 2001 From: doquanghuy Date: Fri, 29 May 2026 21:47:00 +0700 Subject: [PATCH] =?UTF-8?q?docs(workflows):=20clarify=20on=5Freject:skip?= =?UTF-8?q?=20semantics=20=E2=80=94=20engine=20returns=20COMPLETED,=20not?= =?UTF-8?q?=20auto-skip?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot finding on b8982a7: The README example's gate message said "reject to skip the rest of this branch", and the explanatory paragraph claimed [approve, reject] map to "continue" vs "skip the rest of this branch". The engine does not implement automatic branch-skipping. `on_reject: skip` returns `StepStatus.COMPLETED` (gate/__init__.py:65-66); the next sibling step runs unconditionally unless the author wires a downstream `if` reading `{{ steps..output.choice }}`. Two fixes: 1. Restructured the YAML example so it actually demonstrates the manual-branching pattern: added a `recover` if-step after the gate that conditions on `steps.review.output.choice == 'approve'`. Now the example shows the real workflow author's responsibility instead of implying the engine does it. 2. Replaced the trailing paragraph with three precise notes: - both gate options return COMPLETED; `on_reject: skip` controls abort behaviour only, not sibling-skipping - all three `on_reject` values enumerated with their actual engine semantics (FAILED+aborted / COMPLETED / PAUSED) - the original retry-loop guidance retained as the third bullet Updated the gate message in the example to match — "reject to leave the failure recorded and move on" instead of "reject to skip the rest of this branch". Audited the whole PR diff for the same overclaim: no other instance. Engine semantics, validation, and test bodies are unchanged. Docs-only. 161/161 tests/test_workflows.py pass locally. Co-Authored-By: Claude Opus 4.7 (1M context) --- workflows/README.md | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/workflows/README.md b/workflows/README.md index b0b7050fe..6282045a1 100644 --- a/workflows/README.md +++ b/workflows/README.md @@ -242,18 +242,36 @@ remains available on `steps..output.exit_code` so downstream then: - id: review type: gate - message: "Step failed (exit {{ steps.heavy-thing.output.exit_code }}). Approve to continue, or reject to skip the rest of this branch." + message: "Step failed (exit {{ steps.heavy-thing.output.exit_code }}). Approve to run the recovery path, or reject to leave the failure recorded and move on." on_reject: skip + - id: recover + type: if + condition: "{{ steps.review.output.choice == 'approve' }}" + then: + - id: rerun + command: speckit.recovery else: - id: next-thing command: speckit.next-thing ``` -The gate's default `options: [approve, reject]` map directly to "continue -the run" vs. "skip the rest of this branch" — gates do not automatically -re-run the failed step. To express a retry path, either define custom -gate options and branch on the choice downstream, or wrap the failing -step in your own loop. +A few things worth knowing about that example: + +- Both gate options (`approve`, `reject`) return `StepStatus.COMPLETED`; + `on_reject: skip` controls only whether the engine aborts on reject + (it doesn't, with `skip`) — it does **not** auto-skip subsequent + sibling steps in the `then:` list. Downstream branching is the + workflow author's responsibility: read + `{{ steps..output.choice }}` in a follow-up `if`, `switch`, + or expression, as the `recover` step above does. +- `on_reject` has three values: `abort` (default — reject → `FAILED` + with `output.aborted = True`, halts the run), `skip` (reject → + `COMPLETED`, author handles branching as shown), and `retry` + (reject → `PAUSED` so the next `specify workflow resume` re-runs + the gate). +- Gates do not automatically re-run the failed step. To express a + retry path, either define custom gate options and branch on the + choice downstream, or wrap the failing step in your own loop. **Notes:**