diff --git a/docs/reference/workflows.md b/docs/reference/workflows.md index 93377f4bfb..8013f26e08 100644 --- a/docs/reference/workflows.md +++ b/docs/reference/workflows.md @@ -588,6 +588,7 @@ specify workflow run speckit -i spec="Build a kanban board with drag-and-drop ta | `command` | Invoke a Spec Kit command (e.g., `speckit.plan`) | | `prompt` | Send an arbitrary prompt to the AI coding agent | | `shell` | Execute a shell command and capture output | +| `github` | Add an explicit issue or pull-request label | | `init` | Bootstrap a project (like `specify init`) | | `slot` | Named workflow slot; skipped when unfilled | | `gate` | Pause for human approval before continuing | @@ -769,6 +770,40 @@ events. > **Security note:** a `shell` step runs a local command with **your** privileges. There is no capability sandbox — `requires` is an advisory pre-condition block (spec-kit version, integrations), not a runtime gate, so it does **not** restrict what a step can do. In particular there is no `requires.permissions` capability gate: it is rejected by validation precisely because it would imply a sandbox that does not exist. Review any catalog or downloaded workflow before running it, and use a `gate` step to require explicit approval before sensitive or destructive shell commands. +### GitHub step + +`type: github` uses the installed `gh` CLI and its active authentication. +Specify the GitHub repository as `owner/repo`; no Git remote or source checkout +is required. Requests go to GitHub's `api.github.com` REST endpoint for that +repository. The active token needs permission to read the issue or pull request +and to write its labels (for example, `issues: write` for issues or +`pull-requests: write` for PRs when using `GITHUB_TOKEN` in Actions). The step +uses only the existing token: it does not request or elevate permissions. It +does not inspect issue contents for instructions or make a maintainer decision. + +```yaml +- id: mark-ready + type: github + operation: add-label + repository: owner/repo + target: issue + number: "{{ inputs.issue_number }}" + label: ready-for-review +``` + +`target` is `issue` or `pull_request`; `number` must resolve to a positive +integer. `label` is one explicit label name (up to 50 characters) and must +already exist in the repository. `repository`, `number`, and `label` may use +workflow expressions. The repository must resolve to a single `owner/repo` +name, not a URL or Git remote. The step checks the target type and its existing +labels, skips the write when the label is already present, and verifies GitHub's +response after adding it. +The output includes `repository`, `target`, `number`, `label`, and `added` +(`true` only when a label was added). Repeated execution and fan-out items +are safe to retry: an already-present label is left unchanged. This first +version does not post comments, restore artifacts, check out PRs, or infer +which label a maintainer would choose. + ### Custom step packages Custom step types are installed with `specify workflow step`. A step is a diff --git a/src/specify_cli/workflows/__init__.py b/src/specify_cli/workflows/__init__.py index dd52c2f7f2..2f457371ad 100644 --- a/src/specify_cli/workflows/__init__.py +++ b/src/specify_cli/workflows/__init__.py @@ -49,6 +49,7 @@ def _register_builtin_steps() -> None: from .step.fan_in import FanInStep from .step.fan_out import FanOutStep from .step.gate import GateStep + from .step.github import GitHubStep from .step.workflow import WorkflowStep from .step.if_then import IfThenStep from .step.init import InitStep @@ -63,6 +64,7 @@ def _register_builtin_steps() -> None: _register_step(FanInStep()) _register_step(FanOutStep()) _register_step(GateStep()) + _register_step(GitHubStep()) _register_step(WorkflowStep()) _register_step(IfThenStep()) _register_step(InitStep()) diff --git a/src/specify_cli/workflows/step/github/__init__.py b/src/specify_cli/workflows/step/github/__init__.py new file mode 100644 index 0000000000..47b498fc52 --- /dev/null +++ b/src/specify_cli/workflows/step/github/__init__.py @@ -0,0 +1,186 @@ +"""GitHub workflow step: add an explicit label to an issue or pull request.""" + +from __future__ import annotations + +import json +import re +import subprocess +from pathlib import Path +from typing import Any + +from specify_cli.workflows.base import StepBase, StepContext, StepResult, StepStatus +from specify_cli.workflows.expressions import evaluate_expression + +_NUMBER = re.compile(r"[1-9][0-9]*\Z") +_REPOSITORY = re.compile(r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+\Z") + + +def _run(args: list[str], root: Path, *, input_text: str | None = None) -> str: + try: + result = subprocess.run( + args, + cwd=root, + input=input_text, + capture_output=True, + text=True, + timeout=60, + check=False, + ) + except (OSError, subprocess.TimeoutExpired) as exc: + raise ValueError(f"{args[0]} request failed: {exc}") from exc + if result.returncode: + # CLI stderr may contain authentication details; never include it in run state. + raise ValueError(f"{args[0]} request failed (exit code {result.returncode})") + return result.stdout.strip() + + +def _api(root: Path, endpoint: str, *, labels: list[str] | None = None) -> Any: + method = "POST" if labels is not None else "GET" + args = ["gh", "api", "--method", method, endpoint] + if labels is not None: + args.extend(["--input", "-"]) + raw = _run( + args, + root, + input_text=json.dumps({"labels": labels}) if labels is not None else None, + ) + try: + return json.loads(raw) + except json.JSONDecodeError as exc: + raise ValueError(f"GitHub returned invalid JSON for {endpoint}") from exc + + +def _resolved(value: Any, context: StepContext) -> Any: + if isinstance(value, str) and "{{" in value: + return evaluate_expression(value, context) + return value + + +def _number(value: Any) -> int: + if isinstance(value, bool) or not ( + (isinstance(value, int) and value > 0) + or (isinstance(value, str) and _NUMBER.fullmatch(value)) + ): + raise ValueError("'number' must be a positive integer") + return int(value) + + +def _repository(value: Any) -> str: + if ( + not isinstance(value, str) + or not _REPOSITORY.fullmatch(value) + or any(part in (".", "..") for part in value.split("/")) + ): + raise ValueError("'repository' must be a GitHub owner/repo name") + return value + + +def _label(value: Any) -> str: + if ( + not isinstance(value, str) + or not value.strip() + or len(value) > 50 + or any(ord(char) < 32 or ord(char) == 127 for char in value) + ): + raise ValueError( + "'label' must be a non-empty label name of at most 50 characters without control characters" + ) + return value + + +def _label_names(value: Any) -> set[str]: + if not isinstance(value, list) or not all( + isinstance(entry, dict) and isinstance(entry.get("name"), str) + for entry in value + ): + raise ValueError("GitHub returned an invalid label list") + return {entry["name"].casefold() for entry in value} + + +class GitHubStep(StepBase): + """Add only the label explicitly specified by a workflow author.""" + + type_key = "github" + + def validate(self, config: dict[str, Any]) -> list[str]: + errors = super().validate(config) + if config.get("operation") != "add-label": + errors.append("GitHub step 'operation' must be add-label.") + if config.get("target") not in ("issue", "pull_request"): + errors.append("GitHub step 'target' must be issue or pull_request.") + for key in config.keys() - { + "id", + "type", + "operation", + "target", + "repository", + "number", + "label", + "continue_on_error", + }: + errors.append(f"GitHub step has unsupported field {key!r}.") + if "number" not in config: + errors.append("GitHub step requires 'number'.") + elif not (isinstance(config["number"], str) and "{{" in config["number"]): + try: + _number(config["number"]) + except ValueError as exc: + errors.append(str(exc)) + if "repository" not in config: + errors.append("GitHub step requires 'repository'.") + elif not ( + isinstance(config["repository"], str) and "{{" in config["repository"] + ): + try: + _repository(config["repository"]) + except ValueError as exc: + errors.append(str(exc)) + if "label" not in config: + errors.append("GitHub step requires 'label'.") + elif not (isinstance(config["label"], str) and "{{" in config["label"]): + try: + _label(config["label"]) + except ValueError as exc: + errors.append(str(exc)) + return errors + + def execute(self, config: dict[str, Any], context: StepContext) -> StepResult: + try: + errors = self.validate(config) + if errors: + raise ValueError("; ".join(errors)) + number = _number(_resolved(config["number"], context)) + label = _label(_resolved(config["label"], context)) + repository = _repository(_resolved(config["repository"], context)) + root = Path(context.project_root or ".").resolve() + url = f"https://api.github.com/repos/{repository}" + target = config["target"] + kind = "pulls" if target == "pull_request" else "issues" + resource = _api(root, f"{url}/{kind}/{number}") + if ( + not isinstance(resource, dict) + or resource.get("number") != number + or (target == "issue" and "pull_request" in resource) + ): + raise ValueError( + f"GitHub {target} #{number} does not match the requested target" + ) + existing = _label_names(resource.get("labels")) + output = { + "repository": repository, + "target": target, + "number": number, + "label": label, + "added": False, + } + if label.casefold() in existing: + return StepResult(output=output) + labels = _api(root, f"{url}/issues/{number}/labels", labels=[label]) + if label.casefold() not in _label_names(labels): + raise ValueError( + f"GitHub did not apply label {label!r} to {target} #{number}" + ) + output["added"] = True + return StepResult(output=output) + except (ValueError, OSError) as exc: + return StepResult(status=StepStatus.FAILED, error=f"GitHub step: {exc}") diff --git a/tests/test_workflows.py b/tests/test_workflows.py index a4ea5d1786..1c1229768a 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -4,7 +4,7 @@ - Step registry & auto-discovery - Base classes (StepBase, StepContext, StepResult) - Expression engine -- All 12 built-in step types +- All 14 built-in step types - Workflow definition loading & validation - Workflow engine execution & state persistence - Workflow catalog & registry @@ -106,7 +106,7 @@ def test_all_step_types_registered(self): expected = { "command", "shell", "prompt", "gate", "if", "switch", - "while", "do-while", "fan-out", "fan-in", "init", "slot", + "while", "do-while", "fan-out", "fan-in", "init", "slot", "github", "workflow", } assert expected.issubset(set(STEP_REGISTRY.keys())) diff --git a/tests/workflows/test_github_step.py b/tests/workflows/test_github_step.py new file mode 100644 index 0000000000..466ac1174e --- /dev/null +++ b/tests/workflows/test_github_step.py @@ -0,0 +1,314 @@ +"""Tests for the opt-in-by-use built-in GitHub label step (no live API calls).""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from specify_cli.workflows import BUILTIN_STEP_TYPES, get_step_type +from specify_cli.workflows.base import RunStatus, StepContext, StepStatus +from specify_cli.workflows.engine import WorkflowDefinition, WorkflowEngine +from specify_cli.workflows.step import github + + +@pytest.fixture +def context(tmp_path: Path) -> StepContext: + return StepContext( + inputs={"number": 12}, project_root=str(tmp_path), run_id="run-1" + ) + + +@pytest.fixture +def api(monkeypatch): + stored: list[str] = [] + calls: list[tuple[str, str, list[str] | None]] = [] + + def fake_api(root, endpoint, *, labels: list[str] | None = None): + calls.append(("POST" if labels is not None else "GET", endpoint, labels)) + if endpoint.endswith("/issues/12"): + return {"number": 12, "labels": [{"name": name} for name in stored]} + if endpoint.endswith("/pulls/12"): + return {"number": 12, "labels": [{"name": name} for name in stored]} + if endpoint.endswith("/issues/12/labels") and labels is not None: + for name in labels: + if name.casefold() not in {existing.casefold() for existing in stored}: + stored.append(name) + return [{"name": name} for name in stored] + raise AssertionError(f"Unexpected API call: {endpoint}") + + monkeypatch.setattr(github, "_api", fake_api) + return stored, calls + + +def config(**overrides): + definition = { + "id": "label", + "type": "github", + "operation": "add-label", + "repository": "owner/repo", + "target": "issue", + "number": "{{ inputs.number }}", + "label": "ready-for-review", + } + definition.update(overrides) + return definition + + +def test_builtin_registered_and_accepts_only_add_label(): + step = get_step_type("github") + assert step is not None + assert "github" in BUILTIN_STEP_TYPES + assert step.validate(config()) == [] + assert step.validate(config(target="pull_request")) == [] + for operation in ("comment", "fetch-artifact", "checkout-pr", "remove-label", None): + assert step.validate(config(operation=operation)) + assert step.validate(config(body="Not supported")) + + +def test_issue_label_added_once_and_retry_skips_write(context, api): + labels, calls = api + step = get_step_type("github") + first = step.execute(config(), context) + retry = step.execute(config(), context) + assert first.status == retry.status == StepStatus.COMPLETED + assert first.output == { + "repository": "owner/repo", + "target": "issue", + "number": 12, + "label": "ready-for-review", + "added": True, + } + assert retry.output == {**first.output, "added": False} + assert labels == ["ready-for-review"] + assert calls[0][1] == "https://api.github.com/repos/owner/repo/issues/12" + assert len([method for method, _, _ in calls if method == "POST"]) == 1 + + +def test_pull_request_uses_issue_labels_endpoint(context, api): + labels, calls = api + result = get_step_type("github").execute(config(target="pull_request"), context) + assert result.status == StepStatus.COMPLETED + assert result.output["target"] == "pull_request" + assert labels == ["ready-for-review"] + assert calls[0][1].endswith("/pulls/12") + assert calls[1][1].endswith("/issues/12/labels") + + +def test_existing_label_is_case_insensitive_and_skips_post(context, api): + labels, calls = api + labels.append("Ready-For-Review") + result = get_step_type("github").execute(config(), context) + assert result.status == StepStatus.COMPLETED + assert result.output["added"] is False + assert len(calls) == 1 + + +def test_repository_expression_selects_api_target_without_git_origin(context, api): + labels, calls = api + context.inputs["repository"] = "other/project" + context.project_root = None + result = get_step_type("github").execute( + config(repository="{{ inputs.repository }}"), context + ) + assert result.status == StepStatus.COMPLETED + assert result.output["repository"] == "other/project" + assert labels == ["ready-for-review"] + assert [endpoint for _, endpoint, _ in calls] == [ + "https://api.github.com/repos/other/project/issues/12", + "https://api.github.com/repos/other/project/issues/12/labels", + ] + + +def test_api_requests_need_no_git_remote(context, monkeypatch): + calls = [] + + def fake_run(args, root, *, input_text=None): + assert args[:2] == ["gh", "api"] + calls.append((args, input_text)) + if args[2:4] == ["--method", "GET"]: + return '{"number": 12, "labels": []}' + return '[{"name": "ready-for-review"}]' + + monkeypatch.setattr(github, "_run", fake_run) + result = get_step_type("github").execute(config(), context) + assert result.status == StepStatus.COMPLETED + assert result.output["added"] is True + assert calls[0][0][-1] == "https://api.github.com/repos/owner/repo/issues/12" + assert calls[1][0][4] == "https://api.github.com/repos/owner/repo/issues/12/labels" + assert json.loads(calls[1][1]) == {"labels": ["ready-for-review"]} + + +@pytest.mark.parametrize( + "repository", + [ + None, + "", + " ", + "owner", + "owner/", + "/repo", + "owner/repo/extra", + "owner/..", + "./repo", + "owner/repo?x=1", + "https://github.com/owner/repo", + "git@github.com:owner/repo", + "owner/repo\nx", + ["owner", "repo"], + ], +) +def test_invalid_repository_fails_before_any_api_call(context, api, repository): + _, calls = api + step = get_step_type("github") + assert step.validate(config(repository=repository)) + result = step.execute(config(repository=repository), context) + assert result.status == StepStatus.FAILED + assert "repository" in result.error + assert calls == [] + + +def test_invalid_resolved_repository_fails_before_api_call(context, api): + _, calls = api + context.inputs["repository"] = "other/repo/extra" + step = get_step_type("github") + definition = config(repository="{{ inputs.repository }}") + assert step.validate(definition) == [] + result = step.execute(definition, context) + assert result.status == StepStatus.FAILED + assert "repository" in result.error + assert calls == [] + + +@pytest.mark.parametrize("number", [0, -1, True, None, "1; echo bad", ""]) +def test_invalid_number_fails_before_any_api_call(context, api, number): + _, calls = api + result = get_step_type("github").execute(config(number=number), context) + assert result.status == StepStatus.FAILED + assert "number" in result.error + assert calls == [] + + +@pytest.mark.parametrize( + "label", [None, "", " ", "bad\nlabel", "bad\x7flabel", "a" * 51, ["ready"]] +) +def test_invalid_label_fails_before_any_api_call(context, api, label): + _, calls = api + result = get_step_type("github").execute(config(label=label), context) + assert result.status == StepStatus.FAILED + assert "label" in result.error + assert calls == [] + + +def test_missing_fields_and_unsupported_operations_fail(context, api): + _, calls = api + step = get_step_type("github") + for broken in ( + config(target="repository"), + config(operation="comment"), + {key: value for key, value in config().items() if key != "label"}, + {key: value for key, value in config().items() if key != "number"}, + {key: value for key, value in config().items() if key != "repository"}, + config(body="unexpected"), + ): + assert step.validate(broken) + assert step.execute(broken, context).status == StepStatus.FAILED + assert calls == [] + + +def test_wrong_issue_target_and_malformed_labels_fail(context, api, monkeypatch): + original_api = github._api + step = get_step_type("github") + + def wrong_target(root, endpoint, **kwargs): + if endpoint.endswith("/issues/12"): + return {"number": 12, "pull_request": {}, "labels": []} + return original_api(root, endpoint, **kwargs) + + monkeypatch.setattr(github, "_api", wrong_target) + result = step.execute(config(), context) + assert result.status == StepStatus.FAILED + assert "does not match" in result.error + + monkeypatch.setattr( + github, + "_api", + lambda root, endpoint, **kwargs: {"number": 12, "labels": "invalid"}, + ) + result = step.execute(config(), context) + assert result.status == StepStatus.FAILED + assert "invalid label list" in result.error + + +def test_api_failure_or_missing_posted_label_is_not_success(context, api, monkeypatch): + step = get_step_type("github") + original_api = github._api + + def no_label(root, endpoint, **kwargs): + if kwargs.get("labels") is not None: + return [] + return original_api(root, endpoint, **kwargs) + + monkeypatch.setattr(github, "_api", no_label) + result = step.execute(config(), context) + assert result.status == StepStatus.FAILED + assert "did not apply label" in result.error + monkeypatch.setattr( + github, + "_api", + lambda *args, **kwargs: (_ for _ in ()).throw(ValueError("GitHub API failed")), + ) + result = step.execute(config(), context) + assert result.status == StepStatus.FAILED + assert "GitHub API failed" in result.error + assert result.output == {} + + +def test_api_post_uses_json_stdin_and_does_not_put_label_in_arguments( + context, monkeypatch +): + observed = [] + + def fake_run(args, root, *, input_text=None): + observed.append((args, input_text)) + return '[{"name":"ready"}]' + + monkeypatch.setattr(github, "_run", fake_run) + github._api( + Path(context.project_root), + "https://api.github.com/repos/o/r/issues/12/labels", + labels=["ready"], + ) + assert json.loads(observed[0][1]) == {"labels": ["ready"]} + assert "ready" not in observed[0][0] + + +def test_engine_executes_label_yaml_and_fan_out_with_retries(context, api): + labels, calls = api + workflow = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: label-items + name: Label items + version: "1.0.0" +steps: + - id: labels + type: fan-out + items: "{{ ['ready', 'needs-review'] }}" + max_concurrency: 2 + step: + id: add-label + type: github + operation: add-label + repository: owner/repo + target: issue + number: 12 + label: "{{ item }}" +""") + engine = WorkflowEngine(Path(context.project_root)) + assert engine.validate(workflow) == [] + assert engine.execute(workflow, run_id="labels-run").status == RunStatus.COMPLETED + assert set(labels) == {"ready", "needs-review"} + assert engine.execute(workflow, run_id="labels-run").status == RunStatus.COMPLETED + assert len([method for method, _, _ in calls if method == "POST"]) == 2 diff --git a/workflows/ARCHITECTURE.md b/workflows/ARCHITECTURE.md index 6382e0c35e..c0fc391485 100644 --- a/workflows/ARCHITECTURE.md +++ b/workflows/ARCHITECTURE.md @@ -152,13 +152,14 @@ as the new format. ## Step Types -The engine ships with 13 built-in step types, each in its own subpackage under `src/specify_cli/workflows/step/`: +The engine ships with 14 built-in step types, each in its own subpackage under `src/specify_cli/workflows/step/`: | Type Key | Class | Purpose | Returns `next_steps`? | |----------|-------|---------|-----------------------| | `command` | `CommandStep` | Invoke an installed Spec Kit command via integration CLI | No | | `prompt` | `PromptStep` | Send an arbitrary inline prompt to integration CLI | No | | `shell` | `ShellStep` | Run a shell command, capture output | No | +| `github` | `GitHubStep` | Add an explicit issue or pull-request label | No | | `init` | `InitStep` | Bootstrap a project (equivalent to `specify init`) | No | | `slot` | `SlotStep` | Named workflow slot; skipped when unfilled | No | | `gate` | `GateStep` | Interactive human review/approval | No (pauses in CI) | diff --git a/workflows/README.md b/workflows/README.md index c952b4ad4b..642aa6ebbe 100644 --- a/workflows/README.md +++ b/workflows/README.md @@ -89,7 +89,7 @@ The bundled `speckit` workflow only declares `spec` (and optional ## Step Types -Workflows support 13 built-in step types, including `workflow` for calling an +Workflows support 14 built-in step types, including `workflow` for calling an installed workflow with private inputs and declared outputs. See [workflow composition and resume](../docs/reference/workflows.md#workflow-composition) for the scope and execution identity contracts. @@ -179,6 +179,14 @@ killed and the step fails; it must be a positive number and defaults to `300` (five minutes) when omitted. Raise it for long-running gates such as full builds, linter aggregators, or integration-test targets. +### GitHub Steps + +The built-in `github` step adds an explicitly configured issue or pull-request +label when a workflow uses `type: github`. It does not install or invoke the +opt-in GitHub agent-command extension. See the +[GitHub step reference](../docs/reference/workflows.md#github-step) for +configuration, credentials, and retry behavior. + ### Init Steps Bootstrap a project the same way `specify init` does — scaffolding