mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
feat: bind gate verdict to workflow input via verdict_input (#3725)
* feat: bind gate verdict to workflow input via verdict_input Add an optional `verdict_input` field to gate steps that lets an external system supply a verdict through a declared workflow input instead of an interactive TTY prompt. When the referenced input carries a non-empty string value that matches one of the gate's `options` (case-insensitive), the gate auto-decides, records the matched spelling in `output.choice`, and applies the existing `on_reject` / abort / skip / retry semantics. If the value is present but does not match an option, or is a non-string, the gate fails immediately with a clear error message. When the input is absent, null, or empty, the gate falls back to today's TTY-prompt-or-pause behaviour unchanged. The engine now persists `result.error` alongside each step's status and output so that failed-step error messages survive across runs. The CLI (`workflow run` and `workflow resume`) surfaces these persisted errors after a failed or aborted run. `validate_workflow` cross-references `verdict_input` against the workflow's declared inputs block and reports an error for undeclared names, consistent with the existing `wait_for` id cross-check. Closes discussion: https://github.com/github/spec-kit/discussions/3717 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fix workflow JSON error payloads Include persisted step errors in _workflow_run_payload so workflow run/resume/status --json all surface failure reasons consistently. Add JSON-path tests for failed and successful runs. Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(workflows): reject verdict inputs in fan-out Fan-out items share workflow inputs and cannot safely consume a bound gate verdict. Reject verdict_input bindings during validation and at runtime while preserving unbound gates. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: update workflow command handling Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Markus <markus@example.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -308,7 +308,15 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]:
|
||||
errors.append("Workflow has no steps defined.")
|
||||
|
||||
seen_ids: set[str] = set()
|
||||
_validate_steps(definition.steps, seen_ids, errors)
|
||||
# ``input_names`` is the set of declared workflow input names — used by
|
||||
# ``_validate_steps`` to cross-reference gate ``verdict_input`` bindings.
|
||||
# ``None`` means the inputs block itself is malformed (already reported
|
||||
# above); the cross-check is then disabled so one authoring mistake does
|
||||
# not cascade into N spurious "undeclared" errors.
|
||||
input_names: set[str] | None = (
|
||||
set(definition.inputs) if isinstance(definition.inputs, dict) else None
|
||||
)
|
||||
_validate_steps(definition.steps, seen_ids, errors, input_names)
|
||||
|
||||
return errors
|
||||
|
||||
@@ -317,8 +325,16 @@ def _validate_steps(
|
||||
steps: list[dict[str, Any]],
|
||||
seen_ids: set[str],
|
||||
errors: list[str],
|
||||
input_names: set[str] | None = None,
|
||||
inside_fan_out: bool = False,
|
||||
) -> None:
|
||||
"""Recursively validate a list of steps."""
|
||||
"""Recursively validate a list of steps.
|
||||
|
||||
``input_names`` is the set of declared workflow input names (or ``None``
|
||||
when the inputs block is malformed). ``inside_fan_out`` is threaded
|
||||
through nested control-flow steps so gate verdict bindings can be rejected
|
||||
anywhere inside a fan-out template.
|
||||
"""
|
||||
from . import STEP_REGISTRY
|
||||
|
||||
for step_config in steps:
|
||||
@@ -411,30 +427,73 @@ def _validate_steps(
|
||||
f"unknown or not-yet-declared step id {wid!r}."
|
||||
)
|
||||
|
||||
# Gate verdict_input: fan-out items cannot bind shared workflow inputs
|
||||
# as per-item verdicts. Outside fan-out, the binding must reference a
|
||||
# declared workflow input because ``_resolve_inputs`` drops undeclared
|
||||
# names at both initial run and resume. Only check a non-empty string;
|
||||
# malformed shapes are already reported by ``GateStep.validate()``.
|
||||
if step_type == "gate":
|
||||
verdict_input = step_config.get("verdict_input")
|
||||
if isinstance(verdict_input, str) and verdict_input:
|
||||
if inside_fan_out:
|
||||
errors.append(
|
||||
f"Gate step {step_id!r}: 'verdict_input' is not "
|
||||
"supported inside fan-out templates."
|
||||
)
|
||||
elif input_names is not None and verdict_input not in input_names:
|
||||
errors.append(
|
||||
f"Gate step {step_id!r}: 'verdict_input' references "
|
||||
f"undeclared input {verdict_input!r}."
|
||||
)
|
||||
|
||||
# Recursively validate nested steps
|
||||
for nested_key in ("then", "else", "steps"):
|
||||
nested = step_config.get(nested_key)
|
||||
if isinstance(nested, list):
|
||||
_validate_steps(nested, seen_ids, errors)
|
||||
_validate_steps(
|
||||
nested,
|
||||
seen_ids,
|
||||
errors,
|
||||
input_names,
|
||||
inside_fan_out=inside_fan_out,
|
||||
)
|
||||
|
||||
# Validate switch cases
|
||||
cases = step_config.get("cases")
|
||||
if isinstance(cases, dict):
|
||||
for _case_key, case_steps in cases.items():
|
||||
if isinstance(case_steps, list):
|
||||
_validate_steps(case_steps, seen_ids, errors)
|
||||
_validate_steps(
|
||||
case_steps,
|
||||
seen_ids,
|
||||
errors,
|
||||
input_names,
|
||||
inside_fan_out=inside_fan_out,
|
||||
)
|
||||
|
||||
# Validate switch default
|
||||
default = step_config.get("default")
|
||||
if isinstance(default, list):
|
||||
_validate_steps(default, seen_ids, errors)
|
||||
_validate_steps(
|
||||
default,
|
||||
seen_ids,
|
||||
errors,
|
||||
input_names,
|
||||
inside_fan_out=inside_fan_out,
|
||||
)
|
||||
|
||||
# Validate fan-out nested step (template — not added to seen_ids
|
||||
# since the engine generates parentId:templateId:index at runtime)
|
||||
fan_step = step_config.get("step")
|
||||
if isinstance(fan_step, dict):
|
||||
fan_errors: list[str] = []
|
||||
_validate_steps([fan_step], set(), fan_errors)
|
||||
_validate_steps(
|
||||
[fan_step],
|
||||
set(),
|
||||
fan_errors,
|
||||
input_names,
|
||||
inside_fan_out=True,
|
||||
)
|
||||
errors.extend(fan_errors)
|
||||
|
||||
|
||||
@@ -560,6 +619,7 @@ class RunState:
|
||||
self.created_at = datetime.now(timezone.utc).isoformat()
|
||||
self.updated_at = self.created_at
|
||||
self.log_entries: list[dict[str, Any]] = []
|
||||
self.error: str | None = None
|
||||
|
||||
@property
|
||||
def runs_dir(self) -> Path:
|
||||
@@ -614,6 +674,7 @@ class RunState:
|
||||
"workflow_dir": self.workflow_dir,
|
||||
"created_at": self.created_at,
|
||||
"updated_at": self.updated_at,
|
||||
"error": self.error,
|
||||
}
|
||||
self._atomic_write_json(runs_dir / "state.json", state_data)
|
||||
self._atomic_write_json(runs_dir / "inputs.json", {"inputs": self.inputs})
|
||||
@@ -707,6 +768,7 @@ class RunState:
|
||||
state.workflow_dir = state_data.get("workflow_dir")
|
||||
state.created_at = state_data.get("created_at", "")
|
||||
state.updated_at = state_data.get("updated_at", "")
|
||||
state.error = state_data.get("error")
|
||||
|
||||
inputs_path = runs_dir / "inputs.json"
|
||||
if inputs_path.exists():
|
||||
@@ -901,6 +963,7 @@ class WorkflowEngine:
|
||||
return state
|
||||
except Exception as exc:
|
||||
state.status = RunStatus.FAILED
|
||||
state.error = str(exc)
|
||||
state.append_log({"event": "workflow_failed", "error": str(exc)})
|
||||
state.save()
|
||||
raise
|
||||
@@ -959,6 +1022,7 @@ class WorkflowEngine:
|
||||
|
||||
from . import STEP_REGISTRY
|
||||
|
||||
state.error = None
|
||||
state.status = RunStatus.RUNNING
|
||||
state.save()
|
||||
|
||||
@@ -979,6 +1043,7 @@ class WorkflowEngine:
|
||||
return state
|
||||
except Exception as exc:
|
||||
state.status = RunStatus.FAILED
|
||||
state.error = str(exc)
|
||||
state.append_log({"event": "resume_failed", "error": str(exc)})
|
||||
state.save()
|
||||
raise
|
||||
@@ -1038,6 +1103,7 @@ class WorkflowEngine:
|
||||
step_impl = registry.get(step_type)
|
||||
if not step_impl:
|
||||
state.status = RunStatus.FAILED
|
||||
state.error = f"Unknown step type: {step_type!r}"
|
||||
state.append_log(
|
||||
{
|
||||
"event": "step_failed",
|
||||
@@ -1065,6 +1131,7 @@ class WorkflowEngine:
|
||||
or step_config.get("input", {}),
|
||||
"output": result.output,
|
||||
"status": result.status.value,
|
||||
"error": result.error,
|
||||
}
|
||||
self._record_result(context, state, step_id, step_data)
|
||||
|
||||
@@ -1090,6 +1157,7 @@ class WorkflowEngine:
|
||||
# is for transient/expected step failures only.
|
||||
if result.output.get("aborted"):
|
||||
state.status = RunStatus.ABORTED
|
||||
state.error = result.error
|
||||
state.append_log(
|
||||
{
|
||||
"event": "workflow_aborted",
|
||||
@@ -1132,6 +1200,7 @@ class WorkflowEngine:
|
||||
continue
|
||||
|
||||
state.status = RunStatus.FAILED
|
||||
state.error = result.error
|
||||
state.append_log(
|
||||
{
|
||||
"event": "step_failed",
|
||||
@@ -1305,11 +1374,18 @@ class WorkflowEngine:
|
||||
# Sequential path — identical to the historical behavior.
|
||||
if workers <= 1:
|
||||
results: list[Any] = []
|
||||
for item_idx, item_val in enumerate(items):
|
||||
context.item = item_val
|
||||
results.append(run_item(item_idx, context))
|
||||
if state.status in halting:
|
||||
break
|
||||
previous_item = context.item
|
||||
previous_inside_fan_out = context.inside_fan_out
|
||||
context.inside_fan_out = True
|
||||
try:
|
||||
for item_idx, item_val in enumerate(items):
|
||||
context.item = item_val
|
||||
results.append(run_item(item_idx, context))
|
||||
if state.status in halting:
|
||||
break
|
||||
finally:
|
||||
context.item = previous_item
|
||||
context.inside_fan_out = previous_inside_fan_out
|
||||
return results
|
||||
|
||||
# Concurrent path — bounded sliding window; results assembled in item order.
|
||||
@@ -1320,7 +1396,14 @@ class WorkflowEngine:
|
||||
# Each item runs against its own context copy so context.item is not
|
||||
# clobbered across threads; the shared steps dict is written only on the
|
||||
# disjoint parentId:templateId:index key (GIL-safe on distinct keys).
|
||||
return run_item(idx, dataclasses.replace(context, item=items[idx]))
|
||||
return run_item(
|
||||
idx,
|
||||
dataclasses.replace(
|
||||
context,
|
||||
item=items[idx],
|
||||
inside_fan_out=True,
|
||||
),
|
||||
)
|
||||
|
||||
def item_halt_status(idx: int) -> RunStatus | None:
|
||||
# If THIS item's own execution halted the run, return the resulting run
|
||||
@@ -1401,6 +1484,16 @@ class WorkflowEngine:
|
||||
# pool joined; restore the halting item's own outcome so the final run
|
||||
# status matches the sequential semantics.
|
||||
state.status = halted_status
|
||||
# Restore the halting item's error so it matches the terminal
|
||||
# status — a concurrent item may have overwritten state.error
|
||||
# before the pool joined. Assign unconditionally when a record
|
||||
# exists (even when the halting item's own error is falsy) so a
|
||||
# third-party step returning FAILED with no message never inherits
|
||||
# an unrelated concurrent item's error; this mirrors the sequential
|
||||
# path, which sets state.error = result.error verbatim.
|
||||
halt_rec = context.steps.get(item_id(halted_at))
|
||||
if isinstance(halt_rec, dict):
|
||||
state.error = halt_rec.get("error")
|
||||
return slots[: halted_at + 1]
|
||||
return slots[:collected]
|
||||
|
||||
|
||||
Reference in New Issue
Block a user