diff --git a/.gremlins/gh-terse.yaml b/.gremlins/gh-terse.yaml index 6a782764..c19c2626 100644 --- a/.gremlins/gh-terse.yaml +++ b/.gremlins/gh-terse.yaml @@ -7,7 +7,13 @@ inputs: in: INSTRUCTIONS: instructions? PLAN: plan? - PR: pr? + sources: + plan: + type: [filepath, string] + optional: true + instructions: + type: string + optional: true land: in: diff --git a/gremlins/executor/gremlin.py b/gremlins/executor/gremlin.py index 69124deb..16f60f30 100644 --- a/gremlins/executor/gremlin.py +++ b/gremlins/executor/gremlin.py @@ -2,14 +2,12 @@ from __future__ import annotations -import argparse import asyncio import dataclasses import json import logging import os import pathlib -import re import shutil from collections.abc import Awaitable, Callable, Sequence from typing import TYPE_CHECKING, Any, cast @@ -114,9 +112,6 @@ def __init__( worktree_dir: pathlib.Path | None = None, worktree_parent: pathlib.Path | None = None, resume_from: str | None = None, - instructions: str = "", - spec: str | None = None, - plan: str | None = None, repo: str = "", state_file: pathlib.Path | None = None, project_root: str = "", @@ -147,9 +142,6 @@ def __init__( self.worktree_dir = worktree_dir self.worktree_parent = worktree_parent self.resume_from = resume_from - self.instructions = instructions - self.spec = spec - self.plan = plan self.repo = repo self.state_file = state_file self.project_root = project_root @@ -254,7 +246,6 @@ async def fork( pipeline_data=effective_pipeline, repo=state.repo, cwd=child_cwd, - instructions=state.instructions, worktree=child_worktree, worktree_parent=state.worktree_parent, artifacts=child_registry, @@ -288,12 +279,6 @@ def _set_gremlin_recursive(self, stage: StageProtocol) -> None: def _collect_stages( self, stages: Sequence[StageProtocol] ) -> list[tuple[str, Callable[[], Awaitable[Any]]]]: - args = argparse.Namespace( - plan=self.plan, - resume_from=self.resume_from, - spec=self.spec, - instructions=[self.instructions] if self.instructions else [], - ) cwd = ( str(self.worktree_dir) if self.worktree_dir is not None @@ -307,11 +292,9 @@ def _collect_stages( data=StateData(gremlin_id=self.gremlin_id, state_file=self.state_file), client=stage_client, artifact_dir=self.artifact_dir, - args=args, pipeline_data=self.pipeline_data, repo=self.repo, cwd=cwd, - instructions=self.instructions, worktree=self.worktree_dir, worktree_parent=self.worktree_parent, artifacts=self.registry, @@ -374,7 +357,6 @@ def open(cls, gremlin_id: str) -> Gremlin: pipeline_args = cast(list[str], state_raw.get("pipeline_args") or []) pipeline_path = cast(str, state_raw.get("pipeline_path") or "") worktree_dir_str = cast(str, state_raw.get("workdir") or "") - instructions = cast(str, state_raw.get("instructions") or "") # Resolve pipeline (hermetic check first, then fallback) hermetic = state_dir / "pipeline.yaml" @@ -426,7 +408,6 @@ def open(cls, gremlin_id: str) -> Gremlin: gremlin_id=gremlin_id, pipeline_data=pipeline, worktree_dir=worktree_dir, - instructions=instructions, project_root=project_root, pipeline_path=pipeline_path, pipeline_args=pipeline_args, @@ -441,10 +422,7 @@ def initialize_with_runtime( project_dir: pathlib.Path, pipeline_ref: str, worktree_parent: pathlib.Path | None = None, - instructions: str = "", resume_from: str | None = None, - plan: str | None = None, - spec: str | None = None, project_root: str = "", base_ref_sha: str = "", base_ref: str = "", @@ -486,9 +464,6 @@ def initialize_with_runtime( worktree_dir=worktree_dir, worktree_parent=worktree_parent, resume_from=resume_from, - instructions=instructions, - spec=spec, - plan=plan, project_root=project_root, base_ref_sha=base_ref_sha, base_ref=base_ref, @@ -500,7 +475,6 @@ def initialize_with_runtime( self.state_dir, self.artifact_dir, self.gremlin_id, - instructions=self.instructions or "", ) worktree_created: str | None = None @@ -522,23 +496,6 @@ def initialize_with_runtime( setup_kind="worktree-detached", ) - if self.spec: - spec_file = self.artifact_dir / "spec.md" - if not spec_file.exists(): - spec_src = pathlib.Path(self.spec) - if not spec_src.is_file(): - raise ValueError(f"--spec: file not found: {self.spec}") - if spec_src.stat().st_size == 0: - raise ValueError(f"--spec: file is empty: {self.spec}") - shutil.copyfile(spec_src, spec_file) - - if self.plan and not self.pipeline_data.github_integration: - plan_file = self.artifact_dir / "plan.md" - if not plan_file.exists(): - src = pathlib.Path(self.plan) - if src.is_file(): - shutil.copyfile(src, plan_file) - if self.worktree_dir is not None: os.chdir(self.worktree_dir) @@ -555,35 +512,6 @@ def initialize_with_runtime( sha = _git_mod.head_sha(cwd=self.worktree_dir) if sha: self.registry.bind("base_sha", Uri.parse(f"git://commit/{sha}")) - # When --plan is a GH issue ref on a github_integration pipeline, - # the opaque issue URI is what compose-pr's plan.uri? needs. - plan_issue_uri: str | None = None - if self.pipeline_data.github_integration and self.plan: - m = re.match( - r"^(?:[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)?#([0-9]+)$", self.plan - ) - if m: - plan_issue_uri = f"gh://issue/{m.group(1)}" - - if not self.registry.produced("plan"): - if (self.artifact_dir / "plan.md").exists(): - if not self.pipeline_data.github_integration: - self.registry.bind("plan", Uri.parse("file://session/plan.md")) - elif plan_issue_uri is not None: - self.registry.bind("plan", Uri.parse(plan_issue_uri)) - elif self.registry.produced("plan-issue-number"): - n = str(self.registry.read("plan-issue-number")).strip() - self.registry.bind("plan", Uri.parse(f"gh://issue/{n}")) - # else: github_integration with no issue ref/number yet — - # publish-as-issue will bind plan (avoid DuplicateArtifact). - elif ( - plan_issue_uri is not None - and self.registry.resolve("plan").scheme == "file" - ): - # resume: upgrade an existing file:// plan to the issue URI so - # compose-pr resolves it even when the plan stage was skipped. - self.registry.unbind("plan") - self.registry.bind("plan", Uri.parse(plan_issue_uri)) except Exception: if worktree_created: _git_mod.remove_worktree(self.project_root, worktree_created) diff --git a/gremlins/executor/run.py b/gremlins/executor/run.py index 3da0c76b..bc84015f 100644 --- a/gremlins/executor/run.py +++ b/gremlins/executor/run.py @@ -135,22 +135,9 @@ def _atexit_log() -> None: def _parse_args(argv: list[str]) -> argparse.Namespace: parser = argparse.ArgumentParser(add_help=False) - parser.add_argument("--plan", dest="plan", default=None) - parser.add_argument("--spec", default=None) parser.add_argument("--client", dest="client", default=None) parser.add_argument("--resume-from", dest="resume_from", default=None) - parser.add_argument("instructions", nargs="*") - args = parser.parse_args(argv) - if args.plan and args.instructions: - die("--plan and positional instructions are mutually exclusive") - if ( - not args.plan - and not args.instructions - and not args.resume_from - and not os.environ.get("GREMLINS_RESUME_FROM") - ): - die("one of --plan or positional instructions is required") - return args + return parser.parse_args(argv) def _unique_clients(stages: Sequence[StageProtocol]) -> list[Client]: @@ -210,9 +197,6 @@ async def run_pipeline( worktree_dir = pathlib.Path(_workdir) if _workdir else None project_root = str(state_json.get("project_root") or "") stage_inputs: dict[str, Any] = dict(state_json.get("stage_inputs") or {}) - instructions: str = str( - stage_inputs.get("instructions") or " ".join(args.instructions or []) - ) # base_ref_sha and base_ref are bound in registry.json at launch time _registry_path = state_dir / "registry.json" @@ -257,10 +241,7 @@ async def run_pipeline( if project_root else paths.project_root(), pipeline_ref=str(pipeline_path), - instructions=instructions, resume_from=resume_from, - spec=args.spec, - plan=args.plan, worktree_dir=worktree_dir, project_root=project_root, base_ref_sha=base_ref_sha, @@ -307,8 +288,6 @@ async def run_pipeline( if shutil.which("claude") is None: die("claude not found on PATH") - plan_file = artifact_dir / "plan.md" - if not gh and resume_from: _expanded_stage_names = [s.name for s in gremlin.stages] @@ -323,9 +302,6 @@ def _name_idx(stage_name: str) -> int: if resume_from in _expanded_stage_names else 0 ) - if start_idx >= _name_idx("implement"): - if not plan_file.exists() or plan_file.stat().st_size == 0: - die(f"--resume-from {resume_from} requires existing {plan_file}") if start_idx >= _name_idx("review-code"): if not has_dirty_worktree() and not has_commits(): die( diff --git a/gremlins/executor/state.py b/gremlins/executor/state.py index 7ce65538..c65ddc7c 100644 --- a/gremlins/executor/state.py +++ b/gremlins/executor/state.py @@ -115,7 +115,6 @@ class StateData: worktree_base: str = "" status: str = "" started_at: str = "" - instructions: str = "" description: str = "" description_explicit: bool = False parent_id: str = "" @@ -147,7 +146,6 @@ def load(cls, gremlin_id: str | None) -> StateData: worktree_base=sd.get("worktree_base") or "", status=sd.get("status") or "", started_at=sd.get("started_at") or "", - instructions=sd.get("instructions") or "", description=sd.get("description") or "", description_explicit=bool(sd.get("description_explicit")), parent_id=sd.get("parent_id") or "", @@ -178,7 +176,6 @@ def persist(self, state_dir: pathlib.Path) -> None: "worktree_base": self.worktree_base, "status": self.status, "started_at": self.started_at, - "instructions": self.instructions, "description": self.description, "description_explicit": self.description_explicit, "parent_id": self.parent_id, @@ -494,7 +491,6 @@ class State: cwd: str = "" args: argparse.Namespace = dataclasses.field(default_factory=argparse.Namespace) pipeline_data: Pipeline | None = None - instructions: str = "" current_scope: list[StageProtocol] = dataclasses.field(default_factory=_stage_list) child_key: str | None = None parent_stage: str = "" @@ -507,7 +503,6 @@ class State: "name", "model", "artifact_dir", - "instructions", "repo", "cwd", "base_ref", @@ -521,7 +516,6 @@ def framework_subs(self, stage: StageProtocol) -> dict[str, str]: "name": stage.name, "model": self.client.model, "artifact_dir": str(self.artifact_dir), - "instructions": self.instructions, "repo": self.repo, "cwd": self.cwd, "base_ref": self.base_ref, @@ -533,12 +527,9 @@ def setup_dirs( state_dir: pathlib.Path, artifact_dir: pathlib.Path, gremlin_id: str | None, - *, - instructions: str = "", ) -> None: state_dir.mkdir(parents=True, exist_ok=True) artifact_dir.mkdir(parents=True, exist_ok=True) - (state_dir / "instructions.txt").write_text(instructions, encoding="utf-8") sf = state_dir / "state.json" if gremlin_id and not sf.exists(): write_state(state_dir, {"id": gremlin_id}) @@ -624,7 +615,6 @@ def build_state( pipeline_data: Pipeline | None = None, repo: str = "", cwd: str = "", - instructions: str = "", worktree: pathlib.Path | None = None, worktree_parent: pathlib.Path | None = None, artifacts: ArtifactRegistry | None = None, @@ -643,7 +633,6 @@ def build_state( or (str(worktree) if worktree is not None else str(_paths.project_root())), args=args if args is not None else argparse.Namespace(), pipeline_data=pipeline_data, - instructions=instructions, worktree=worktree, worktree_parent=worktree_parent, child_key=child_key, diff --git a/gremlins/launcher.py b/gremlins/launcher.py index 7e015f33..f76b12de 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -1,9 +1,9 @@ """Launcher for background gremlins. Public API: - launch(kind, *, stage_inputs=None, plan=None, description=None, - parent_id=None, project_root=None, base_ref="HEAD", - pipeline_args=()) -> tuple[str, subprocess.Popen[bytes]] + launch(kind, *, stage_inputs=None, description=None, parent_id=None, + project_root=None, base_ref=None, pipeline_args=(), + gremlin_id=None) -> tuple[str, subprocess.Popen[bytes]] resume(gremlin_id, *, graft=None) -> None """ @@ -14,7 +14,6 @@ import json import os import pathlib -import re import secrets import shutil import subprocess @@ -34,7 +33,7 @@ from gremlins.utils.spawn_logged_process import ( spawn_logged_process as _spawn_logged_process, ) -from gremlins.utils.text import read_markdown_title, slugify +from gremlins.utils.text import slugify class GremlinAlreadyRunning(RuntimeError): @@ -50,44 +49,12 @@ def _state_root() -> pathlib.Path: def _resolve_description_and_slug( - instructions: str | None, - plan: str | None, description: str | None, - *, - issue_title: str = "", ) -> tuple[str, bool, str]: - """Return (description, description_explicit, slug) from available inputs. - - ``issue_title`` is an optional pre-fetched title for an issue-ref ``plan`` - so callers that already resolved the issue (e.g. boss pipeline) don't trigger - a second ``gh`` round-trip here. - """ + """Return (description, description_explicit, slug) from available inputs.""" if description: slug = slugify(description) or "gremlin" return description[:60], True, slug - - if plan and os.path.isfile(plan): - h1 = read_markdown_title(plan) - if h1: - slug = slugify(h1) or "gremlin" - return h1[:60], False, slug - base = os.path.splitext(os.path.basename(plan))[0] - slug = slugify(base) or "gremlin" - return "", False, slug - - if plan: - # Non-file plan arg (issue ref); derive slug from the ref string directly. - title = issue_title - if title: - slug = slugify(title) or "gremlin" - return title[:60], False, slug - slug = slugify(plan) or "gremlin" - return "", False, slug - - if instructions: - slug = slugify(instructions[:80]) or "gremlin" - return instructions[:60], False, slug - return "", False, "gremlin" @@ -109,8 +76,6 @@ def _build_spawn_env(gremlin_id: str) -> dict[str, str]: class _Inputs: gremlin_id: str kind: str - plan: str | None - instructions: str description: str description_explicit: bool parent_id: str @@ -122,42 +87,7 @@ class _Inputs: base_ref_name: str base_ref_sha: str stage_inputs: dict[str, Any] - pr_num: str = "" - - -def _validate_plan_args( - plan: str | None, - instructions: str | None, - spec_path: str | None, -) -> tuple[str | None, str | None]: - if plan and instructions: - raise ValueError("--plan and instructions are mutually exclusive") - - if plan and os.path.isfile(plan) and os.path.getsize(plan) == 0: - raise ValueError(f"--plan: file is empty: {plan}") - - if spec_path is not None: - if not os.path.isfile(spec_path): - raise ValueError(f"--spec: file not found: {spec_path}") - if os.path.getsize(spec_path) == 0: - raise ValueError(f"--spec: file is empty: {spec_path}") - spec_path = str(pathlib.Path(spec_path).resolve()) - - if plan and os.path.isfile(plan): - plan = str(pathlib.Path(plan).resolve()) - - if plan and not os.path.isfile(plan): - if os.sep in plan or plan.endswith(".md"): - raise ValueError(f"--plan: file not found: {plan}") - _is_issue_ref = re.match(r"^#([0-9]+)$", plan) or re.match( - r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+#[0-9]+$", plan - ) - if not _is_issue_ref: - raise ValueError( - f"--plan: not a file or recognized issue ref (use #N or owner/repo#N): {plan}" - ) - - return plan, spec_path + loaded_pipeline: _PipelineData | None = None def _reject_pipeline_collision(gremlin_id: str) -> None: @@ -233,43 +163,21 @@ def _resolve_base_ref( return effective_base_ref, "" -def _parse_pr_num(pr_arg: str) -> str: - """Extract a PR number string from a --pr arg (number, #N, or .../pull/N URL).""" - m = re.search(r"/pull/(\d+)(?:[/#?]|$)", pr_arg) - if m: - return m.group(1) - stripped = pr_arg.strip().lstrip("#") - if re.fullmatch(r"\d+", stripped): - return stripped - raise ValueError(f"cannot parse PR number from arg: {pr_arg!r}") - - def _resolve_inputs( kind: str, stage_inputs: dict[str, Any], - plan: str | None, description: str | None, parent_id: str | None, project_root: str | None, base_ref: str | None, pipeline_args: tuple[str, ...], - spec_path: str | None, gremlin_id: str | None, ) -> _Inputs: from gremlins.cli.pipeline_args import launch_client_label, resolve_pipeline pr = stage_inputs.pop("pr", None) or None - instructions: str | None = stage_inputs.get("instructions") - if plan is None: - plan = stage_inputs.pop("plan", None) - plan, spec_path = _validate_plan_args(plan, instructions, spec_path) - - desc, desc_explicit, slug = _resolve_description_and_slug( - instructions, - plan, - description, - ) + desc, desc_explicit, slug = _resolve_description_and_slug(description) if project_root is None: r = proc.run(["git", "rev-parse", "--show-toplevel"]) @@ -306,27 +214,19 @@ def _resolve_inputs( base_ref_name = "" base_ref_sha = pr_ref fetch_worktree = True - pr_num = _parse_pr_num(pr) else: base_ref_name, base_ref_sha = _resolve_base_ref( base_ref, project_root, loaded_pipeline ) fetch_worktree = False - pr_num = "" stored_args = list(resolved_pipeline_args) - if spec_path and "--spec" not in stored_args: - stored_args = ["--spec", spec_path] + stored_args - if plan and "--plan" not in stored_args: - stored_args = ["--plan", plan] + stored_args client_label = launch_client_label(stored_args, loaded_pipeline) return _Inputs( gremlin_id=resolved_gremlin_id, kind=kind, - plan=plan, - instructions=instructions or "", description=desc, description_explicit=desc_explicit, parent_id=parent_id or "", @@ -338,16 +238,13 @@ def _resolve_inputs( base_ref_name=base_ref_name, base_ref_sha=base_ref_sha, stage_inputs=stage_inputs, - pr_num=pr_num, + loaded_pipeline=loaded_pipeline, ) -def _prepare_state_dir(state_dir: pathlib.Path, inputs: _Inputs) -> None: +def _prepare_state_dir(state_dir: pathlib.Path) -> None: state_dir.mkdir(parents=True, exist_ok=True) - (state_dir / "instructions.txt").write_text(inputs.instructions, encoding="utf-8") - artifacts_dir = state_dir / "artifacts" - artifacts_dir.mkdir(exist_ok=True) - (artifacts_dir / "plan-arg.txt").write_text(inputs.plan or "", encoding="utf-8") + (state_dir / "artifacts").mkdir(exist_ok=True) def _initial_state_data(inputs: _Inputs) -> StateData: @@ -361,7 +258,6 @@ def _initial_state_data(inputs: _Inputs) -> StateData: worktree_base="", status="running", started_at=now_iso, - instructions=inputs.instructions[:200], description=inputs.description, description_explicit=inputs.description_explicit, parent_id=inputs.parent_id, @@ -461,8 +357,6 @@ def _persist_expanded_pipeline(state_dir: pathlib.Path, pipeline_path: str) -> s def _spawn(gremlin_id: str, inputs: _Inputs, state_dir: pathlib.Path) -> Any: spawn_args = list(inputs.pipeline_args) - if inputs.instructions: - spawn_args.append(inputs.instructions) cmd = [ sys.executable, "-m", @@ -476,17 +370,49 @@ def _spawn(gremlin_id: str, inputs: _Inputs, state_dir: pathlib.Path) -> Any: ) +def _seed_registry_from_sources( + registry: ArtifactRegistry, + input_values: dict[str, str], + sources: dict[str, Any], + artifacts_dir: pathlib.Path, +) -> None: + for key, source in sources.items(): + value = input_values.get(key) or None + if not value: + if not source.optional: + raise ValueError( + f"required input source {key!r} (type: {source.types}) is not available" + ) + continue + for t in source.types: + if t == "filepath" and os.path.isfile(value): + src = pathlib.Path(value) + ext = src.suffix or ".txt" + dest = artifacts_dir / f"{key}{ext}" + dest.write_bytes(src.read_bytes()) + registry.bind(key, Uri.parse(f"file://session/{key}{ext}")) + break + elif t == "string": + dest = artifacts_dir / f"{key}.txt" + dest.write_text(value, encoding="utf-8") + registry.bind(key, Uri.parse(f"file://session/{key}.txt")) + break + else: + if not source.optional: + raise ValueError( + f"required input source {key!r} (type: {source.types}) could not be resolved" + ) + + def launch( kind: str, *, stage_inputs: dict[str, Any] | None = None, - plan: str | None = None, description: str | None = None, parent_id: str | None = None, project_root: str | None = None, base_ref: str | None = None, pipeline_args: tuple[str, ...] = (), - spec_path: str | None = None, gremlin_id: str | None = None, bypass: bool = False, permissions_file: str = "", @@ -501,18 +427,16 @@ def launch( inputs = _resolve_inputs( kind, {} if stage_inputs is None else dict(stage_inputs), - plan, description, parent_id, project_root, base_ref, pipeline_args, - spec_path, gremlin_id, ) state_dir = _state_root() / inputs.gremlin_id try: - _prepare_state_dir(state_dir, inputs) + _prepare_state_dir(state_dir) inputs.pipeline_path = _persist_expanded_pipeline( state_dir, inputs.pipeline_path ) @@ -523,14 +447,23 @@ def launch( artifact_dir = state_dir / "artifacts" artifact_dir.mkdir(parents=True, exist_ok=True) registry = ArtifactRegistry(artifact_dir=artifact_dir) - if inputs.pr_num: - registry.bind("pr", Uri.parse(f"gh://pr/{inputs.pr_num}")) if inputs.base_ref_sha: registry.bind("base_sha", Uri.parse(f"git://commit/{inputs.base_ref_sha}")) if inputs.base_ref_name: registry.bind("base_ref", Uri.parse(f"git://ref/{inputs.base_ref_name}")) - registry.bind("spec", Uri.parse("file://session/spec.md")) - registry.bind("plan_arg", Uri.parse("file://session/plan-arg.txt")) + if ( + inputs.loaded_pipeline is not None + and inputs.loaded_pipeline.input_sources is not None + ): + input_values = { + k: v for k, v in inputs.stage_inputs.items() if isinstance(v, str) and v + } + _seed_registry_from_sources( + registry, + input_values, + inputs.loaded_pipeline.input_sources.sources, + artifact_dir, + ) p = _spawn(inputs.gremlin_id, inputs, state_dir) except Exception: shutil.rmtree(state_dir, ignore_errors=True) @@ -619,23 +552,12 @@ def _patch_state_for_resume( def _spawn_resume( gremlin_id: str, state_dir: pathlib.Path, - state: dict[str, Any], pipeline_path: str, pipeline_args: list[str], stage: str, project_root: str, ) -> Any: - has_plan = any(a == "--plan" or str(a).startswith("--plan=") for a in pipeline_args) - spawn_args: list[str] = list(pipeline_args) - if not has_plan: - instr_file = state_dir / "instructions.txt" - if instr_file.is_file(): - instructions = instr_file.read_text(encoding="utf-8") - else: - instructions = str(state.get("instructions") or "") - if instructions: - spawn_args.append(instructions) env = _build_spawn_env(gremlin_id) @@ -688,9 +610,6 @@ def resume(gremlin_id: str, *, graft: str | None = None) -> None: p = _spawn_resume( gremlin_id, gremlin.state_dir, - { - "instructions": state_data.instructions, - }, gremlin.pipeline_path, gremlin.pipeline_args, stage, diff --git a/gremlins/pipeline/__init__.py b/gremlins/pipeline/__init__.py index 0916faf9..c7d7bac0 100644 --- a/gremlins/pipeline/__init__.py +++ b/gremlins/pipeline/__init__.py @@ -8,6 +8,7 @@ from gremlins.clients.client import PACKAGE_DEFAULT, Client if TYPE_CHECKING: + from gremlins.pipeline.inputs import InputSources from gremlins.stages.base import Stage from gremlins.stages.exec import Exec @@ -31,6 +32,7 @@ class Pipeline: default_client: Client | None = None base_ref: str = "current" inputs: Exec | None = None + input_sources: InputSources | None = None land: Exec | None = None github_integration: bool = False @@ -75,15 +77,25 @@ def from_yaml(cls, path: pathlib.Path) -> Pipeline: github_integration = bool(raw.get("github_integration", False)) + from gremlins.pipeline.inputs import InputSources from gremlins.stages.exec import Exec stages = parse_stages(cast(list[dict[str, Any]], raw.get("stages") or [])) inputs_stage: Exec | None = None + input_sources: InputSources | None = None inputs_raw = raw.get("inputs") if inputs_raw is not None: if not isinstance(inputs_raw, dict): raise ValueError("'inputs' must be a mapping") + inputs_raw = cast(dict[str, Any], inputs_raw) + sources_raw = inputs_raw.get("sources") + if sources_raw is not None: + if not isinstance(sources_raw, dict): + raise ValueError("'inputs.sources' must be a mapping") + input_sources = InputSources.from_yaml( + cast(dict[str, Any], sources_raw) + ) inputs_stage = Exec.with_dict({"name": "inputs", **inputs_raw}) land_stage: Exec | None = None @@ -107,6 +119,7 @@ def from_yaml(cls, path: pathlib.Path) -> Pipeline: default_client=default_client, base_ref=pipeline_base_ref, inputs=inputs_stage, + input_sources=input_sources, land=land_stage, github_integration=github_integration, ) diff --git a/gremlins/pipeline/inputs.py b/gremlins/pipeline/inputs.py new file mode 100644 index 00000000..12ddac18 --- /dev/null +++ b/gremlins/pipeline/inputs.py @@ -0,0 +1,93 @@ +"""Input source declarations for pipelines.""" + +from __future__ import annotations + +import dataclasses +from typing import Any, cast + + +@dataclasses.dataclass +class InputSource: + """A single input source declaration.""" + + name: str + types: list[str] + optional: bool = False + + def __post_init__(self) -> None: + if not self.types: + raise ValueError( + f"input source {self.name!r}: types list must not be empty" + ) + valid_types = {"filepath", "string"} + for t in self.types: + if t not in valid_types: + raise ValueError( + f"input source {self.name!r}: unknown type {t!r}. " + f"Supported types: {', '.join(sorted(valid_types))}" + ) + + +class InputSources: + """Container for input source declarations from a pipeline's inputs: block.""" + + def __init__(self, sources: dict[str, InputSource] | None = None) -> None: + self.sources = sources or {} + + @classmethod + def from_yaml(cls, raw: dict[str, Any]) -> InputSources: + """Parse sources: block from YAML.""" + sources: dict[str, InputSource] = {} + for key, entry in raw.items(): + if not isinstance(entry, dict): + raise ValueError( + f"input source {key!r}: expected a mapping, got {type(entry).__name__}" + ) + entry = cast(dict[str, Any], entry) + + # Parse type field: can be a string or list of strings + type_raw = entry.get("type") + if type_raw is None: + raise ValueError(f"input source {key!r}: missing required 'type' field") + + if isinstance(type_raw, str): + types = [type_raw] + elif isinstance(type_raw, list): + type_raw = cast(list[Any], type_raw) + if not type_raw: + raise ValueError( + f"input source {key!r}: type list must not be empty" + ) + for t in type_raw: + if not isinstance(t, str): + raise ValueError( + f"input source {key!r}: all type entries must be strings" + ) + types = cast(list[str], type_raw) + else: + raise ValueError( + f"input source {key!r}: 'type' must be a string or list of strings, " + f"got {type(type_raw).__name__}" + ) + + optional_raw = entry.get("optional", False) + if not isinstance(optional_raw, bool): + raise ValueError( + f"input source {key!r}: 'optional' must be a boolean, " + f"got {type(optional_raw).__name__}" + ) + sources[key] = InputSource(name=key, types=types, optional=optional_raw) + + return cls(sources) + + def get(self, key: str) -> InputSource | None: + """Retrieve a source by name, or None if not defined.""" + return self.sources.get(key) + + def all_sources(self) -> list[str]: + """Return list of all source names.""" + return list(self.sources.keys()) + + def required_sources(self) -> list[str]: + """Return list of required source names (optional=False).""" + return [name for name, src in self.sources.items() if not src.optional] diff --git a/gremlins/pipelines/gh-terse.yaml b/gremlins/pipelines/gh-terse.yaml index b3a9e74d..76acfb25 100644 --- a/gremlins/pipelines/gh-terse.yaml +++ b/gremlins/pipelines/gh-terse.yaml @@ -7,8 +7,14 @@ github_integration: true inputs: in: INSTRUCTIONS: instructions? - PLAN: plan? - PR: pr? + PLAN: plan-document? + sources: + plan: + type: [filepath, string] + optional: true + instructions: + type: string + optional: true land: in: diff --git a/gremlins/pipelines/gh.yaml b/gremlins/pipelines/gh.yaml index 68a1fc32..bdf93632 100644 --- a/gremlins/pipelines/gh.yaml +++ b/gremlins/pipelines/gh.yaml @@ -7,8 +7,7 @@ github_integration: true inputs: in: INSTRUCTIONS: instructions? - PLAN: plan? - PR: pr? + PLAN: plan-document? land: in: diff --git a/gremlins/pipelines/local.yaml b/gremlins/pipelines/local.yaml index 98d1e871..75c259fd 100644 --- a/gremlins/pipelines/local.yaml +++ b/gremlins/pipelines/local.yaml @@ -5,7 +5,7 @@ base_ref: current inputs: in: INSTRUCTIONS: instructions? - PLAN: plan? + PLAN: plan-document? prompts: code-style: gremlins:code_style.md diff --git a/gremlins/recipes/stages/github_open_pr.yaml b/gremlins/recipes/stages/github_open_pr.yaml index 24736c0b..ef8eb047 100644 --- a/gremlins/recipes/stages/github_open_pr.yaml +++ b/gremlins/recipes/stages/github_open_pr.yaml @@ -13,7 +13,7 @@ stages: type: agent in: base_ref_to_open_pr: base_ref_to_open_pr - plan_uri: plan.uri? + plan_uri: plan-document.uri? out: pr_title: "file://session/pr-title.txt" pr_body: "file://session/pr-body.md" diff --git a/gremlins/recipes/stages/implement.yaml b/gremlins/recipes/stages/implement.yaml index 38549050..cc0d0493 100644 --- a/gremlins/recipes/stages/implement.yaml +++ b/gremlins/recipes/stages/implement.yaml @@ -3,7 +3,7 @@ required-prompt: true stages: - name: implement type: agent - in: { spec: spec, plan: plan } + in: { spec: spec, plan: plan-document } prompt: - "{{prompt}}" options: diff --git a/gremlins/recipes/stages/plan.yaml b/gremlins/recipes/stages/plan.yaml index 930d9c24..acccae8f 100644 --- a/gremlins/recipes/stages/plan.yaml +++ b/gremlins/recipes/stages/plan.yaml @@ -4,31 +4,33 @@ stages: - name: resolve-plan-input type: exec in: - plan_arg: "plan_arg?" + plan: "plan?" options: cmds: - | if [ -s "{artifact_dir}/plan.md" ]; then exit 0; fi - if [ -z "$plan_arg" ]; then exit 0; fi - if [ -f "$plan_arg" ]; then - cp "$plan_arg" "{artifact_dir}/plan.md" - elif echo "$plan_arg" | grep -qE '^([^/]+/[^#]+)?#[0-9]+$'; then - gh issue view "$plan_arg" --json body --jq .body \ + if [ -z "$plan" ]; then exit 0; fi + if [ -f "$plan" ]; then + cp "$plan" "{artifact_dir}/plan.md" + elif echo "$plan" | grep -qE '^([^/]+/[^#]+)?#[0-9]+$'; then + gh issue view "$plan" --json body --jq .body \ > "{artifact_dir}/plan.md" else - printf 'resolve-plan-input: unrecognized plan_arg: %s\n' "$plan_arg" >&2 + printf 'resolve-plan-input: unrecognized plan: %s\n' "$plan" >&2 exit 1 fi out: - plan?: "file://session/plan.md" + plan-document?: "file://session/plan.md" - name: plan type: agent - skip_if_exists: plan + skip_if_exists: plan-document prompt: - "{{prompt}}" + in: + instructions: "instructions?" out: - plan: "file://session/plan.md" + plan-document: "file://session/plan.md" options: idle_timeout: 600 diff --git a/gremlins/recipes/stages/plan_gh.yaml b/gremlins/recipes/stages/plan_gh.yaml index 4ad62517..8fb8cd4b 100644 --- a/gremlins/recipes/stages/plan_gh.yaml +++ b/gremlins/recipes/stages/plan_gh.yaml @@ -4,42 +4,41 @@ stages: - name: resolve-plan-input type: exec in: - plan_arg: "plan_arg?" + plan: "plan?" options: cmds: - | if [ -s "{artifact_dir}/plan.md" ]; then exit 0; fi - if [ -z "$plan_arg" ]; then - printf 'resolve-plan-input: no plan_arg provided and plan.md does not exist\n' >&2 - exit 1 - fi - if [ -f "$plan_arg" ]; then - cp "$plan_arg" "{artifact_dir}/plan.md" - elif echo "$plan_arg" | grep -qE '^#[0-9]+$'; then - gh issue view "$plan_arg" --json body --jq .body \ + if [ -z "$plan" ]; then exit 0; fi + if [ -f "$plan" ]; then + cp "$plan" "{artifact_dir}/plan.md" + elif echo "$plan" | grep -qE '^#[0-9]+$'; then + gh issue view "$plan" --json body --jq .body \ > "{artifact_dir}/plan.md" if ! head -1 "{artifact_dir}/plan.md" | grep -q '^# '; then - title=$(gh issue view "$plan_arg" --json title --jq .title) + title=$(gh issue view "$plan" --json title --jq .title) { printf '# %s\n\n' "$title"; cat "{artifact_dir}/plan.md"; } > "{artifact_dir}/plan.md.tmp" mv "{artifact_dir}/plan.md.tmp" "{artifact_dir}/plan.md" fi - gh issue view "$plan_arg" --json number --jq .number \ + gh issue view "$plan" --json number --jq .number \ > "{artifact_dir}/plan-issue-number.txt" else - printf 'resolve-plan-input: unrecognized plan_arg: %s\n' "$plan_arg" >&2 + printf 'resolve-plan-input: unrecognized plan: %s\n' "$plan" >&2 exit 1 fi out: plan-issue-number?: "file://session/plan-issue-number.txt" - plan?: "gh://issue/{read:plan-issue-number}" # must follow plan-issue-number? — {read:plan-issue-number} interpolates it + plan-document?: "gh://issue/{read:plan-issue-number}" # must follow plan-issue-number? — {read:plan-issue-number} interpolates it - name: plan type: agent - skip_if_exists: plan + skip_if_exists: plan-document prompt: - "{{prompt}}" + in: + instructions: "instructions?" out: - plan: "file://session/plan.md" + plan-document: "file://session/plan.md" options: idle_timeout: 600 @@ -62,7 +61,7 @@ stages: printf '%s' "${url##*/}" > "{artifact_dir}/plan-issue-number.txt" out: plan-issue-number: "file://session/plan-issue-number.txt" - plan?: "gh://issue/{read:plan-issue-number}" + plan-document?: "gh://issue/{read:plan-issue-number}" - name: update-description type: exec diff --git a/gremlins/recipes/stages/review_code.yaml b/gremlins/recipes/stages/review_code.yaml index 97921e19..98bf4f3d 100644 --- a/gremlins/recipes/stages/review_code.yaml +++ b/gremlins/recipes/stages/review_code.yaml @@ -1,7 +1,7 @@ stages: - name: review-code type: agent - in: { plan: plan } + in: { plan: plan-document } out: { "{name}": "file://session/{name}-{model}.md" } options: {} prompt: diff --git a/gremlins/run_child.py b/gremlins/run_child.py index aa016941..78f8dae0 100644 --- a/gremlins/run_child.py +++ b/gremlins/run_child.py @@ -117,7 +117,6 @@ def _build_state(spec: dict[str, Any]) -> State: artifact_dir=artifact_dir, pipeline_data=pipeline_data, repo=str(spec.get("repo") or ""), - instructions=str(spec.get("instructions") or ""), child_key=spec.get("child_key") or None, parent_stage=str(spec.get("parent_stage") or ""), worktree=worktree, diff --git a/gremlins/spawn/child.py b/gremlins/spawn/child.py index f390dc64..36bf86bf 100644 --- a/gremlins/spawn/child.py +++ b/gremlins/spawn/child.py @@ -14,7 +14,6 @@ "attempt": attempt id for this child (overrides state.json) "parent_stage": parent stage name for sub-stage tracking "repo": "owner/repo" for gh API calls (from parent) - "instructions": freeform instructions forwarded from parent "base_ref": base branch/ref name for prompt template substitution (e.g. "main"); omitted or null → "" } @@ -132,7 +131,6 @@ def _build_state(spec: dict[str, Any]) -> State: worktree=worktree, worktree_parent=worktree_parent, repo=str(spec.get("repo") or ""), - instructions=str(spec.get("instructions") or ""), base_ref=str(spec.get("base_ref") or ""), ) diff --git a/gremlins/utils/proc.py b/gremlins/utils/proc.py index 566be376..f536b831 100644 --- a/gremlins/utils/proc.py +++ b/gremlins/utils/proc.py @@ -298,7 +298,6 @@ def _build_child_spec_dict( "attempt": attempt, "parent_stage": child_st.parent_stage, "repo": child_st.repo, - "instructions": child_st.instructions, "base_ref": child_st.base_ref, } diff --git a/tests/test_gremlin_open.py b/tests/test_gremlin_open.py index 7b0fb7ca..ee3ac00f 100644 --- a/tests/test_gremlin_open.py +++ b/tests/test_gremlin_open.py @@ -77,7 +77,6 @@ def test_gremlin_open_valid_state(sandbox, project_dir, pipeline_yaml): "project_root": str(project_dir), "pipeline_path": str(pipeline_yaml), "pipeline_args": [], - "instructions": "test instructions", "workdir": "/tmp/worktree", } (state_dir / "state.json").write_text(json.dumps(state_data), encoding="utf-8") @@ -86,7 +85,6 @@ def test_gremlin_open_valid_state(sandbox, project_dir, pipeline_yaml): assert gremlin.gremlin_id == gremlin_id assert gremlin.project_root == str(project_dir) - assert gremlin.instructions == "test instructions" assert gremlin.worktree_dir == pathlib.Path("/tmp/worktree") assert gremlin.pipeline_data is not None @@ -155,7 +153,6 @@ def test_gremlin_open_with_hermetic_pipeline(sandbox, project_dir): "project_root": str(project_dir), "pipeline_path": "/some/other/path.yaml", "pipeline_args": [], - "instructions": "", } (state_dir / "state.json").write_text(json.dumps(state_data), encoding="utf-8") @@ -178,7 +175,6 @@ def test_gremlin_open_filters_pipeline_args(sandbox, project_dir, pipeline_yaml) "project_root": str(project_dir), "pipeline_path": str(pipeline_yaml), "pipeline_args": ["--pipeline", str(pipeline_yaml), "--other", "val"], - "instructions": "", } (state_dir / "state.json").write_text(json.dumps(state_data), encoding="utf-8") diff --git a/tests/test_input_sources.py b/tests/test_input_sources.py new file mode 100644 index 00000000..339ea09b --- /dev/null +++ b/tests/test_input_sources.py @@ -0,0 +1,125 @@ +"""Tests for pipeline input sources.""" + +import pathlib +import textwrap + +import pytest + +from gremlins.pipeline import Pipeline +from gremlins.pipeline.inputs import InputSource, InputSources + + +class TestInputSources: + def test_parse_sources(self) -> None: + raw = { + "plan": {"type": ["filepath", "string"], "optional": True}, + "instructions": {"type": "string"}, + } + sources = InputSources.from_yaml(raw) + plan = sources.get("plan") + assert plan is not None + assert plan.types == ["filepath", "string"] + assert plan.optional is True + instr = sources.get("instructions") + assert instr is not None + assert instr.optional is False + + def test_required_sources(self) -> None: + raw = { + "issue": {"type": "string"}, + "plan": {"type": "string", "optional": True}, + } + sources = InputSources.from_yaml(raw) + required = sources.required_sources() + assert "issue" in required + assert "plan" not in required + + def test_missing_type_field_rejected(self) -> None: + with pytest.raises(ValueError, match="missing required 'type' field"): + InputSources.from_yaml({"issue": {}}) + + def test_invalid_type_rejected(self) -> None: + with pytest.raises(ValueError, match="unknown type"): + InputSources.from_yaml({"issue": {"type": "bad"}}) + + def test_non_boolean_optional_rejected(self) -> None: + with pytest.raises(ValueError, match="'optional' must be a boolean"): + InputSources.from_yaml({"issue": {"type": "string", "optional": "yes"}}) + + def test_type_list_non_string_rejected(self) -> None: + with pytest.raises(ValueError, match="all type entries must be strings"): + InputSources.from_yaml({"issue": {"type": ["string", 123]}}) + + def test_empty_type_list_rejected(self) -> None: + with pytest.raises(ValueError, match="type list must not be empty"): + InputSources.from_yaml({"issue": {"type": []}}) + + def test_non_mapping_source_rejected(self) -> None: + with pytest.raises(ValueError, match="expected a mapping"): + InputSources.from_yaml({"issue": ["string"]}) + + def test_unknown_type_on_inputsource_rejected(self) -> None: + with pytest.raises(ValueError, match="unknown type"): + InputSource(name="bad", types=["unknown"]) + + +class TestPipelineInputSources: + def _write_pipeline(self, tmp_path: pathlib.Path, content: str) -> pathlib.Path: + p = tmp_path / "pipeline.yaml" + p.write_text(textwrap.dedent(content), encoding="utf-8") + return p + + def test_parse_pipeline_with_input_sources(self, tmp_path: pathlib.Path) -> None: + p = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + in: + PLAN: plan-document? + sources: + plan: + type: [filepath, string] + optional: true + instructions: + type: string + optional: true + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(p) + assert pipeline.input_sources is not None + plan = pipeline.input_sources.get("plan") + assert plan is not None + assert plan.types == ["filepath", "string"] + assert plan.optional is True + + def test_parse_pipeline_without_input_sources(self, tmp_path: pathlib.Path) -> None: + p = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + in: + PLAN: plan-document? + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(p) + assert pipeline.input_sources is None + + def test_invalid_sources_block_rejected(self, tmp_path: pathlib.Path) -> None: + p = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + sources: ["not-a-mapping"] + stages: + - { name: plan, type: agent } + """, + ) + with pytest.raises(ValueError, match="'inputs.sources' must be a mapping"): + Pipeline.from_yaml(p) diff --git a/tests/test_launcher.py b/tests/test_launcher.py index 60defbd3..86b884f1 100644 --- a/tests/test_launcher.py +++ b/tests/test_launcher.py @@ -115,17 +115,11 @@ def test_launch_creates_state_layout(lenv): f"pipeline did not finish; log:\n{(state_dir / 'log').read_text(errors='replace')[-2000:]}" ) - assert (state_dir / "instructions.txt").exists() - assert (state_dir / "instructions.txt").read_text( - encoding="utf-8" - ) == "test instructions" - state = _read_state(state_dir) assert state["id"] == gremlin_id assert state["kind"] == "local" assert state["setup_kind"] == "worktree-detached" assert state["pipeline_path"].endswith(".yaml") - assert "test instructions" in state["instructions"] assert "workdir" in state and state["workdir"] @@ -260,32 +254,6 @@ def test_launch_ghgremlin_persists_cli_client_override(lenv_with_gh): assert state["client"] == "copilot:gpt-5.4" -def test_launch_plan_normalized_to_absolute(lenv): - """A relative --plan path is resolved to absolute in state.json.""" - plan_file = lenv.repo / "my-plan.md" - plan_file.write_text("# My Plan Heading\n\nBody.\n", encoding="utf-8") - launcher = _launcher() - gremlin_id, _ = launcher.launch("local", plan=str(plan_file.name)) - state = _read_state(_gremlins_state_root(lenv) / gremlin_id) - idx = state["pipeline_args"].index("--plan") - persisted = state["pipeline_args"][idx + 1] - assert os.path.isabs(persisted), f"expected absolute path, got: {persisted!r}" - assert pathlib.Path(persisted).name == "my-plan.md" - assert state["description"].startswith("My Plan Heading") - - -def test_launch_h1_as_description(lenv): - """H1 from --plan file becomes the gremlin description.""" - plan_file = lenv.repo / "plan-with-h1.md" - plan_file.write_text( - "# Hello World Feature\n\n## Tasks\n- [ ] Do it\n", encoding="utf-8" - ) - launcher = _launcher() - gremlin_id, _ = launcher.launch("local", plan=str(plan_file)) - state = _read_state(_gremlins_state_root(lenv) / gremlin_id) - assert state["description"].startswith("Hello World Feature") - - def test_launch_invalid_pipeline_name_raises(lenv): """launch() raises FileNotFoundError for an unresolvable pipeline name.""" launcher = _launcher() @@ -293,33 +261,6 @@ def test_launch_invalid_pipeline_name_raises(lenv): launcher.launch("notapipeline", stage_inputs={"instructions": "test"}) -def test_launch_plan_and_instructions_mutex(lenv): - """launch() raises ValueError when both plan and instructions are given.""" - plan_file = lenv.repo / "plan.md" - plan_file.write_text("# X\n", encoding="utf-8") - launcher = _launcher() - with pytest.raises(ValueError, match="mutually exclusive"): - launcher.launch( - "local", plan=str(plan_file), stage_inputs={"instructions": "extra"} - ) - - -def test_launch_empty_plan_file_rejected(lenv): - """localgremlin rejects an empty --plan file before creating state.""" - empty = lenv.repo / "empty-plan.md" - empty.write_text("", encoding="utf-8") - launcher = _launcher() - with pytest.raises(ValueError, match="empty"): - launcher.launch("local", plan=str(empty)) - # No state dir should have been created - dirs = ( - list((_gremlins_state_root(lenv)).glob("*")) - if _gremlins_state_root(lenv).exists() - else [] - ) - assert dirs == [], f"empty-plan failure must not create state: {dirs}" - - def test_launch_spawned_process_detached(lenv): """The spawned pipeline has a different process group than the parent.""" launcher = _launcher() @@ -418,7 +359,6 @@ def test_resume_uses_persisted_client_label(lenv, monkeypatch): state_dir.mkdir(parents=True) (state_dir / "log").write_text("", encoding="utf-8") (state_dir / "finished").touch() - (state_dir / "instructions.txt").write_text("test resume refresh", encoding="utf-8") (state_dir / "state.json").write_text( json.dumps( { @@ -472,7 +412,6 @@ def test_resume_refuses_running_gremlin(lenv): "pipeline_args": [], } (state_dir / "state.json").write_text(json.dumps(state), encoding="utf-8") - (state_dir / "instructions.txt").write_text("foo", encoding="utf-8") with pytest.raises(RuntimeError, match="still running"): launcher.resume(gremlin_id) @@ -497,7 +436,6 @@ def test_resume_refuses_finished_success(lenv): } (state_dir / "state.json").write_text(json.dumps(state), encoding="utf-8") (state_dir / "finished").touch() - (state_dir / "instructions.txt").write_text("foo", encoding="utf-8") with pytest.raises(RuntimeError, match="finished successfully"): launcher.resume(gremlin_id) @@ -515,7 +453,7 @@ def test_run_pipeline_writes_terminal_state_on_success(lenv, monkeypatch): "# Test Plan\n\n## Tasks\n- [ ] Touch a file\n", encoding="utf-8" ) launcher = _launcher() - gremlin_id, _ = launcher.launch("local", plan=str(plan_file)) + gremlin_id, _ = launcher.launch("local", stage_inputs={"plan": str(plan_file)}) state_dir = _gremlins_state_root(lenv) / gremlin_id assert _wait_for_finished(state_dir, timeout=120), ( f"pipeline did not finish; log:\n{(state_dir / 'log').read_text(errors='replace')[-2000:]}" @@ -696,20 +634,6 @@ def test_launch_ghgremlin_state_layout(lenv_with_gh): assert workdir.is_dir(), f"worktree directory should exist: {workdir}" -def test_launch_gh_plan_issue_ref_not_snapshotted(lenv_with_gh): - """gh pipeline with --plan keeps the raw ref without snapshotting.""" - lenv = lenv_with_gh - launcher = _launcher() - gremlin_id, _ = launcher.launch("gh", plan="#42") - state_dir = _gremlins_state_root(lenv) / gremlin_id - state = _read_state(state_dir) - - idx = state["pipeline_args"].index("--plan") - persisted = state["pipeline_args"][idx + 1] - assert persisted == "#42", f"expected raw issue ref '#42', got: {persisted!r}" - assert not (state_dir / "plan-from-issue.md").exists() - - # --------------------------------------------------------------------------- # PYTHONSAFEPATH worktree-rename regression # --------------------------------------------------------------------------- @@ -891,54 +815,6 @@ def test_setup_workdir_non_git_raises(tmp_path): assert "is not a git repository" in exc_info.value.stderr -# --------------------------------------------------------------------------- -# spec_path forwarding -# --------------------------------------------------------------------------- - - -def test_launch_threads_spec_path_into_pipeline_args(lenv): - """launch(spec_path=) puts --spec into state.json pipeline_args.""" - plan_file = lenv.repo / "plan.md" - plan_file.write_text("# Plan\n\nDo stuff.\n", encoding="utf-8") - spec_file = lenv.repo / "spec.md" - spec_file.write_text("the overall spec", encoding="utf-8") - - launcher = _launcher() - gremlin_id, _ = launcher.launch( - "local", plan=str(plan_file), spec_path=str(spec_file) - ) - state = _read_state(_gremlins_state_root(lenv) / gremlin_id) - assert "--spec" in state["pipeline_args"] - idx = state["pipeline_args"].index("--spec") - assert state["pipeline_args"][idx + 1] == str(spec_file.resolve()) - - -def test_launch_rejects_missing_spec_path(lenv): - """spec_path that doesn't exist raises ValueError before any state-dir setup.""" - plan_file = lenv.repo / "plan.md" - plan_file.write_text("# Plan\n", encoding="utf-8") - launcher = _launcher() - with pytest.raises(ValueError, match="--spec"): - launcher.launch("local", plan=str(plan_file), spec_path="/nonexistent/spec.md") - dirs = ( - list(_gremlins_state_root(lenv).glob("*")) - if _gremlins_state_root(lenv).exists() - else [] - ) - assert dirs == [], f"missing-spec failure must not create state: {dirs}" - - -def test_launch_rejects_empty_spec_path(lenv): - """spec_path pointing to an empty file raises ValueError.""" - plan_file = lenv.repo / "plan.md" - plan_file.write_text("# Plan\n", encoding="utf-8") - spec_file = lenv.repo / "empty-spec.md" - spec_file.write_text("", encoding="utf-8") - launcher = _launcher() - with pytest.raises(ValueError, match="--spec"): - launcher.launch("local", plan=str(plan_file), spec_path=str(spec_file)) - - # --------------------------------------------------------------------------- # stage_inputs persistence # --------------------------------------------------------------------------- @@ -963,7 +839,6 @@ def test_stage_inputs_survives_resume(lenv, monkeypatch): state_dir.mkdir(parents=True) (state_dir / "log").write_text("", encoding="utf-8") (state_dir / "finished").touch() - (state_dir / "instructions.txt").write_text("do the thing", encoding="utf-8") saved_stage_inputs = {"instructions": "do the thing", "flag": "value"} pipeline_yaml = lenv.repo / "pipeline.yaml" pipeline_yaml.write_text( @@ -1000,51 +875,6 @@ class _Proc: assert post_state["stage_inputs"] == saved_stage_inputs -# --------------------------------------------------------------------------- -# Boss pipeline + issue-ref plan materializes plan.md -# --------------------------------------------------------------------------- - - -def test_launch_boss_plan_issue_ref_materializes_plan_md(lenv, monkeypatch): - """Boss + --plan #N writes the issue ref to artifacts/plan-arg.txt before chain runs. - - The actual issue body is fetched at runtime by resolve-plan-input, not at launch time. - """ - launcher = _launcher() - - monkeypatch.setattr( - launcher, "_spawn_logged_process", lambda *args, **kwargs: _FakeProc() - ) - - gremlin_id, _ = launcher.launch("boss", plan="#317", project_root=str(lenv.repo)) - state_dir = _gremlins_state_root(lenv) / gremlin_id - plan_arg_txt = state_dir / "artifacts" / "plan-arg.txt" - - assert plan_arg_txt.exists(), f"plan-arg.txt not found at {plan_arg_txt}" - assert plan_arg_txt.read_text(encoding="utf-8").strip() == "#317" - - -def test_launch_plan_issue_ref_writes_issue_url_and_num(lenv, monkeypatch): - """Boss + --plan #N stores the issue ref in plan-arg.txt for the recipe to resolve.""" - launcher = _launcher() - - monkeypatch.setattr( - launcher, "_spawn_logged_process", lambda *args, **kwargs: _FakeProc() - ) - - gremlin_id, _ = launcher.launch("boss", plan="#378", project_root=str(lenv.repo)) - - state_dir = _gremlins_state_root(lenv) / gremlin_id - plan_arg_txt = state_dir / "artifacts" / "plan-arg.txt" - assert plan_arg_txt.exists(), f"plan-arg.txt not found at {plan_arg_txt}" - assert plan_arg_txt.read_text(encoding="utf-8").strip() == "#378" - - registry_data = json.loads( - (_gremlins_state_root(lenv) / gremlin_id / "registry.json").read_text() - ) - assert registry_data.get("plan_arg") == "file://session/plan-arg.txt" - - # --------------------------------------------------------------------------- # --gremlin-id explicit id # --------------------------------------------------------------------------- @@ -1145,5 +975,4 @@ def test_launch_pr_kwarg_sets_state_fields(lenv, monkeypatch): registry_path = _gremlins_state_root(lenv) / gremlin_id / "registry.json" assert registry_path.exists(), "registry.json should have been written" registry_data = json.loads(registry_path.read_text()) - assert registry_data.get("pr") == "gh://pr/697" assert registry_data.get("base_sha") == "git://commit/pull/697/head" diff --git a/tests/test_launcher_input_sources.py b/tests/test_launcher_input_sources.py new file mode 100644 index 00000000..4b1f2845 --- /dev/null +++ b/tests/test_launcher_input_sources.py @@ -0,0 +1,92 @@ +"""Tests for launcher registry seeding from input sources.""" + +import pathlib + +import pytest + +from gremlins.artifacts.registry import ArtifactRegistry +from gremlins.launcher import _seed_registry_from_sources +from gremlins.pipeline.inputs import InputSource + + +def _make_registry(tmp_path: pathlib.Path) -> tuple[ArtifactRegistry, pathlib.Path]: + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + return ArtifactRegistry(artifact_dir=artifact_dir), artifact_dir + + +def _sources(*items: tuple[str, list[str], bool]) -> dict[str, InputSource]: + return { + name: InputSource(name=name, types=types, optional=optional) + for name, types, optional in items + } + + +class TestSeedRegistryFromSources: + def test_string_source_writes_file_and_binds(self, tmp_path: pathlib.Path) -> None: + registry, artifact_dir = _make_registry(tmp_path) + sources = _sources(("instructions", ["string"], False)) + + _seed_registry_from_sources( + registry, {"instructions": "do the thing"}, sources, artifact_dir + ) + + assert registry.data["instructions"] == "file://session/instructions.txt" + assert (artifact_dir / "instructions.txt").read_text() == "do the thing" + + def test_filepath_source_copies_to_session(self, tmp_path: pathlib.Path) -> None: + registry, artifact_dir = _make_registry(tmp_path) + plan_file = tmp_path / "plan.md" + plan_file.write_text("# Plan", encoding="utf-8") + sources = _sources(("plan", ["filepath"], False)) + + _seed_registry_from_sources( + registry, {"plan": str(plan_file)}, sources, artifact_dir + ) + + assert registry.data["plan"] == "file://session/plan.md" + assert (artifact_dir / "plan.md").read_text() == "# Plan" + + def test_union_type_falls_back_to_string(self, tmp_path: pathlib.Path) -> None: + registry, artifact_dir = _make_registry(tmp_path) + sources = _sources(("plan", ["filepath", "string"], True)) + + _seed_registry_from_sources(registry, {"plan": "#123"}, sources, artifact_dir) + + assert registry.data["plan"] == "file://session/plan.txt" + assert (artifact_dir / "plan.txt").read_text() == "#123" + + def test_optional_source_absent_skipped(self, tmp_path: pathlib.Path) -> None: + registry, artifact_dir = _make_registry(tmp_path) + sources = _sources(("instructions", ["string"], True)) + + _seed_registry_from_sources(registry, {}, sources, artifact_dir) + + assert "instructions" not in registry.data + + def test_required_source_absent_raises(self, tmp_path: pathlib.Path) -> None: + registry, artifact_dir = _make_registry(tmp_path) + sources = _sources(("plan", ["string"], False)) + + with pytest.raises(ValueError, match="required input source"): + _seed_registry_from_sources(registry, {}, sources, artifact_dir) + + def test_filepath_only_no_file_raises(self, tmp_path: pathlib.Path) -> None: + registry, artifact_dir = _make_registry(tmp_path) + sources = _sources(("plan", ["filepath"], False)) + + with pytest.raises(ValueError, match="required input source"): + _seed_registry_from_sources( + registry, {"plan": "/nonexistent/plan.md"}, sources, artifact_dir + ) + + def test_unknown_key_in_input_values_ignored(self, tmp_path: pathlib.Path) -> None: + registry, artifact_dir = _make_registry(tmp_path) + sources = _sources(("plan", ["string"], True)) + + _seed_registry_from_sources( + registry, {"plan": "ref", "extra": "ignored"}, sources, artifact_dir + ) + + assert "plan" in registry.data + assert "extra" not in registry.data diff --git a/tests/test_orchestrator_gh.py b/tests/test_orchestrator_gh.py index f06d2d8b..6bc3065c 100644 --- a/tests/test_orchestrator_gh.py +++ b/tests/test_orchestrator_gh.py @@ -182,7 +182,7 @@ def _patch_common( # this keeps publish-as-issue's bind idempotent (same URI, no DuplicateArtifact). registry_data: dict = { "spec": "file://session/spec.md", - "plan": "gh://issue/42", + "plan-document": "gh://issue/42", "plan-draft": "file://session/plan.md", "pr-url": "file://session/pr-url.txt", "pr-branch": "file://session/pr-branch.txt", @@ -271,7 +271,7 @@ def _prepare_for_plan_stage(tmp_path: pathlib.Path) -> None: """Remove plan so skip_if_exists does not skip the plan stage.""" reg_path = tmp_path / "registry.json" reg = json.loads(reg_path.read_text()) - reg.pop("plan", None) + reg.pop("plan-document", None) reg_path.write_text(json.dumps(reg)) @@ -338,34 +338,9 @@ def fake_run(cmd, *args, **kwargs): # --------------------------------------------------------------------------- -# _parse_gh_args — arg parsing unit tests -# --------------------------------------------------------------------------- - - -def test_parse_instructions(): - args = _parse_gh_args(["add a login page"]) - # A single quoted string arrives as one element in argv - assert args.instructions == ["add a login page"] - assert args.plan is None - assert args.resume_from is None - - -def test_parse_plan_source(): - args = _parse_gh_args(["--plan", "#42"]) - assert args.plan == "#42" - assert args.instructions == [] - - -def test_parse_resume_from_commit(capsys): - args = _parse_gh_args(["--plan", "#42", "--resume-from", "commit"]) +def test_parse_resume_from_commit(): + args = _parse_gh_args(["--resume-from", "commit"]) assert args.resume_from == "commit" - captured = capsys.readouterr() - assert "rewinding" not in captured.err - - -def test_parse_plan_and_instructions_mutual_exclusion(): - with pytest.raises(SystemExit): - _parse_gh_args(["--plan", "#42", "also some instructions"]) def test_gh_pipeline_stage_names(tmp_path): @@ -473,7 +448,7 @@ def test_plan_mode_skips_plan_stage(tmp_path, monkeypatch): ) result = asyncio.run( - run_pipeline(_gh_pipeline_path(tmp_path), argv=["--plan", "#42"], client=client) + run_pipeline(_gh_pipeline_path(tmp_path), argv=[], client=client) ) assert result == 0 @@ -511,9 +486,7 @@ def test_plan_skip_if_exists_on_resume(tmp_path, monkeypatch): ) result = asyncio.run( - run_pipeline( - _gh_pipeline_path(tmp_path), argv=["add foo feature"], client=client - ) + run_pipeline(_gh_pipeline_path(tmp_path), argv=[], client=client) ) assert result == 0 labels = [c.label for c in client.calls] @@ -569,9 +542,7 @@ async def _recording_shell(cmd, **kwargs): ) result = asyncio.run( - run_pipeline( - _gh_pipeline_path(tmp_path), argv=["add foo feature"], client=client - ) + run_pipeline(_gh_pipeline_path(tmp_path), argv=[], client=client) ) assert result == 0 assert not any("gh issue create" in cmd for cmd in shell_cmds) @@ -612,10 +583,11 @@ def test_plan_no_h1_issue_body(tmp_path, monkeypatch): # Clear plan.md so resolve-plan-input actually fetches the issue (tests H1 prepend logic). (artifact_dir / "plan.md").write_text("", encoding="utf-8") - # Wire up plan_arg so resolve-plan-input doesn't exit at the [ -z ] guard + # Seed plan as a file URI containing the raw issue ref so resolve-plan-input + # gets $plan="#42" (matching what inputs.sources would produce at launch time). registry_path = tmp_path / "registry.json" reg = json.loads(registry_path.read_text()) - reg["plan_arg"] = "file://session/plan-arg.txt" + reg["plan"] = "file://session/plan-arg.txt" registry_path.write_text(json.dumps(reg)) (artifact_dir / "plan-arg.txt").write_text("#42", encoding="utf-8") @@ -649,7 +621,7 @@ async def _shell(cmd, *, cwd=None, env=None, timeout=None): ) result = asyncio.run( - run_pipeline(_gh_pipeline_path(tmp_path), argv=["--plan", "#42"], client=client) + run_pipeline(_gh_pipeline_path(tmp_path), argv=[], client=client) ) assert result == 0 plan_content = (artifact_dir / "plan.md").read_text(encoding="utf-8") @@ -688,14 +660,11 @@ def test_plan_stage_uses_bundled_prompt_not_slash_command(tmp_path, monkeypatch) ) result = asyncio.run( - run_pipeline( - _gh_pipeline_path(tmp_path), argv=["add foo feature"], client=client - ) + run_pipeline(_gh_pipeline_path(tmp_path), argv=[], client=client) ) assert result == 0 plan_call = next(c for c in client.calls if c.label == "plan") - assert "add foo feature" in plan_call.prompt assert not plan_call.prompt.startswith("/ghplan") assert "/ghplan" not in plan_call.prompt @@ -732,7 +701,7 @@ def test_model_forwarded_to_all_stages(tmp_path, monkeypatch): result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#42", "--client", "claude:claude-opus-4-7"], + argv=["--client", "claude:claude-opus-4-7"], client=client, ) ) @@ -778,7 +747,7 @@ def test_gh_main_defaults_model_to_sonnet(tmp_path, monkeypatch): # Invoke with NO --model. result = asyncio.run( - run_pipeline(_gh_pipeline_path(tmp_path), argv=["--plan", "#42"], client=client) + run_pipeline(_gh_pipeline_path(tmp_path), argv=[], client=client) ) assert result == 0 @@ -825,7 +794,7 @@ def test_gh_main_client_specifier_model(tmp_path, monkeypatch): result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#42", "--client", "copilot:gpt-4o"], + argv=["--client", "copilot:gpt-4o"], client=client, ) ) @@ -878,7 +847,7 @@ def test_resume_from_implement(tmp_path, monkeypatch): result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#99", "--resume-from", "implement"], + argv=["--resume-from", "implement"], client=client, ) ) @@ -918,7 +887,7 @@ def test_resume_from_github_review_pull_request(tmp_path, monkeypatch): result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#5", "--resume-from", "github-review-pull-request"], + argv=["--resume-from", "github-review-pull-request"], client=client, ) ) @@ -930,7 +899,7 @@ def test_resume_from_github_review_pull_request(tmp_path, monkeypatch): def test_plan_file_path_includes_plan_title_cost_in_total(tmp_path, monkeypatch): - """gh_main with --plan aggregates plan-title's cost into the persisted total_cost_usd. + """Plan stage cost is aggregated into the persisted total_cost_usd. Regression guard for #157 (missing plan-title cost) and #164 (plan-title moved to stream-json mode for cost capture). Reads total_cost_usd from the @@ -939,9 +908,6 @@ def test_plan_file_path_includes_plan_title_cost_in_total(tmp_path, monkeypatch) _init_git_repo(tmp_path) monkeypatch.chdir(tmp_path) - plan_file = tmp_path / "my-plan.md" - plan_file.write_text("# Feature\nDo the thing.\n") - artifact_dir, state_file = _patch_common(monkeypatch, tmp_path) _prepare_for_plan_stage(tmp_path) @@ -1049,9 +1015,7 @@ async def fake_gh_run_async(cmd, *args, **kwargs): fixtures=fixtures, ) result = asyncio.run( - run_pipeline( - _gh_pipeline_path(tmp_path), argv=["--plan", str(plan_file)], client=client - ) + run_pipeline(_gh_pipeline_path(tmp_path), argv=[], client=client) ) assert result == 0 @@ -1072,8 +1036,8 @@ async def fake_gh_run_async(cmd, *args, **kwargs): ) -def test_parse_resume_from_open_pr(capsys): - args = _parse_gh_args(["--plan", "#42", "--resume-from", "open-pr"]) +def test_parse_resume_from_open_pr(): + args = _parse_gh_args(["--resume-from", "open-pr"]) assert args.resume_from == "open-pr" @@ -1116,7 +1080,7 @@ def test_resume_from_open_pr(tmp_path, monkeypatch): result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#42", "--resume-from", "open-pr"], + argv=["--resume-from", "open-pr"], client=client, ) ) @@ -1181,7 +1145,7 @@ async def record_loop(self, state): result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#42", "--client", "claude:claude-opus-4-7"], + argv=["--client", "claude:claude-opus-4-7"], client=client, ) ) @@ -1240,7 +1204,7 @@ async def record_loops(self, state): result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#42", "--client", "claude:claude-opus-4-7"], + argv=["--client", "claude:claude-opus-4-7"], client=client, ) ) @@ -1289,7 +1253,7 @@ def test_github_wait_ci_stage_ordering(tmp_path, monkeypatch): ) result = asyncio.run( - run_pipeline(_gh_pipeline_path(tmp_path), argv=["--plan", "#42"], client=client) + run_pipeline(_gh_pipeline_path(tmp_path), argv=[], client=client) ) assert result == 0 @@ -1332,7 +1296,7 @@ async def track_loops(self, state): result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#5", "--resume-from", "ci-gate"], + argv=["--resume-from", "ci-gate"], client=client, ) ) @@ -1394,7 +1358,7 @@ async def record_verify(self, state): result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#42", "--client", "claude:claude-opus-4-7"], + argv=["--client", "claude:claude-opus-4-7"], client=client, ) ) @@ -1451,7 +1415,7 @@ def test_resume_from_verify(tmp_path, monkeypatch): result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#5", "--resume-from", "verify"], + argv=["--resume-from", "verify"], client=client, ) ) @@ -1493,7 +1457,7 @@ def test_gh_main_writes_stage_to_state(tmp_path, monkeypatch): result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#42"], + argv=[], gremlin_id=gremlin_id, client=client, ) @@ -1538,7 +1502,7 @@ def test_gh_main_state_client_tracks_effective_model( result = asyncio.run( run_pipeline( _gh_pipeline_path(tmp_path), - argv=["--plan", "#42", "--client", "copilot:gpt-5.4"], + argv=["--client", "copilot:gpt-5.4"], gremlin_id=gremlin_id, client=client, ) @@ -1606,7 +1570,7 @@ def _from_yaml_copilot_default(path): ) result = asyncio.run( - run_pipeline(_gh_pipeline_path(tmp_path), argv=["--plan", "#42"], client=client) + run_pipeline(_gh_pipeline_path(tmp_path), argv=[], client=client) ) assert result == 0 @@ -1615,51 +1579,3 @@ def _from_yaml_copilot_default(path): assert not bad, ( f"{len(bad)} stage(s) used wrong model: {[(c.label, c.model) for c in bad]}" ) - - -# --------------------------------------------------------------------------- -# stage_inputs wiring: gh pipeline reads instructions from state.json -# --------------------------------------------------------------------------- - - -def test_gh_stage_inputs_instructions_reach_plan(tmp_path, monkeypatch): - """stage_inputs["instructions"] from state.json is passed to plan.Plan, and - takes precedence over the CLI positional argument.""" - _init_git_repo(tmp_path) - monkeypatch.chdir(tmp_path) - - artifact_dir, state_file = _patch_common( - monkeypatch, - tmp_path, - state_data={"stage_inputs": {"instructions": "instr from state"}}, - ) - _prepare_for_plan_stage(tmp_path) - - monkeypatch.setattr(subprocess, "run", _make_gh_subprocess()) - monkeypatch.setattr( - "gremlins.stages.loop.LoopStage.run", _async(lambda self, pipe: None) - ) - - client = _CommittingClient( - git_dir=tmp_path, - artifact_dir=artifact_dir, - fixtures={ - "plan": _issue_events(), - "implement": IMPL_EVENTS, - "commit": IMPL_EVENTS, - "compose-pr": MINIMAL_EVENTS, - "github-review-pull-request": MINIMAL_EVENTS, - "github-address-pull-request-reviews": MINIMAL_EVENTS, - }, - ) - - result = asyncio.run( - run_pipeline( - _gh_pipeline_path(tmp_path), argv=["instr from cli"], client=client - ) - ) - assert result == 0 - - plan_call = next(c for c in client.calls if c.label == "plan") - assert "instr from state" in plan_call.prompt - assert "instr from cli" not in plan_call.prompt diff --git a/tests/test_orchestrator_local.py b/tests/test_orchestrator_local.py index 08c97750..e754bb9d 100644 --- a/tests/test_orchestrator_local.py +++ b/tests/test_orchestrator_local.py @@ -28,8 +28,11 @@ def _local_pipeline_path(cwd): def test_local_main_plan_mode(tmp_path, monkeypatch): artifact_dir = tmp_path / "artifacts" artifact_dir.mkdir() - plan_file = tmp_path / "plan.md" - plan_file.write_text("# Plan\nDo stuff.\n") + # Pre-seed plan artifact so the plan stage is skipped (skip_if_exists) + (artifact_dir / "plan.md").write_text("# Plan\nDo stuff.\n") + (tmp_path / "registry.json").write_text( + json.dumps({"plan-document": "file://session/plan.md"}) + ) monkeypatch.chdir(tmp_path) _common_patches(monkeypatch) @@ -48,7 +51,7 @@ def test_local_main_plan_mode(tmp_path, monkeypatch): result = asyncio.run( run_pipeline( _local_pipeline_path(tmp_path), - argv=["--plan", str(plan_file)], + argv=[], client=client, ) ) @@ -66,8 +69,10 @@ def test_local_main_resume_from_review_code_requires_git_changes( ): artifact_dir = tmp_path / "artifacts" artifact_dir.mkdir() - plan_file = tmp_path / "plan.md" - plan_file.write_text("# Plan\nDo stuff.\n") + (artifact_dir / "plan.md").write_text("# Plan\nDo stuff.\n") + (tmp_path / "registry.json").write_text( + json.dumps({"plan-document": "file://session/plan.md"}) + ) monkeypatch.chdir(tmp_path) _common_patches(monkeypatch) @@ -83,7 +88,7 @@ def test_local_main_resume_from_review_code_requires_git_changes( asyncio.run( run_pipeline( _local_pipeline_path(tmp_path), - argv=["--plan", str(plan_file), "--resume-from", "review-code"], + argv=["--resume-from", "review-code"], client=FakeClaudeClient(fixtures={}), ) ) @@ -99,8 +104,10 @@ def test_local_main_resume_from_review_code_allows_existing_git_changes( ): artifact_dir = tmp_path / "artifacts" artifact_dir.mkdir() - plan_file = tmp_path / "plan.md" - plan_file.write_text("# Plan\nDo stuff.\n") + (artifact_dir / "plan.md").write_text("# Plan\nDo stuff.\n") + (tmp_path / "registry.json").write_text( + json.dumps({"plan-document": "file://session/plan.md"}) + ) monkeypatch.chdir(tmp_path) _common_patches(monkeypatch) @@ -122,7 +129,7 @@ def test_local_main_resume_from_review_code_allows_existing_git_changes( result = asyncio.run( run_pipeline( _local_pipeline_path(tmp_path), - argv=["--plan", str(plan_file), "--resume-from", "review-code"], + argv=["--resume-from", "review-code"], client=client, ) ) @@ -138,8 +145,11 @@ def test_local_main_client_specifier_model(tmp_path, monkeypatch): """Model from --client provider:model flows into stage run() calls.""" artifact_dir = tmp_path / "artifacts" artifact_dir.mkdir() - plan_file = tmp_path / "plan.md" - plan_file.write_text("# Plan\nDo stuff.\n") + # Pre-seed plan artifact so plan stage is skipped + (artifact_dir / "plan.md").write_text("# Plan\nDo stuff.\n") + (tmp_path / "registry.json").write_text( + json.dumps({"plan-document": "file://session/plan.md"}) + ) monkeypatch.chdir(tmp_path) _common_patches(monkeypatch) @@ -158,7 +168,7 @@ def test_local_main_client_specifier_model(tmp_path, monkeypatch): result = asyncio.run( run_pipeline( _local_pipeline_path(tmp_path), - argv=["--plan", str(plan_file), "--client", "copilot:gpt-4o"], + argv=["--client", "copilot:gpt-4o"], client=client, ) ) @@ -192,8 +202,6 @@ def test_local_main_writes_stage_to_state(tmp_path, monkeypatch, make_state_dir) artifact_dir = tmp_path / "artifacts" artifact_dir.mkdir() - plan_file = tmp_path / "plan.md" - plan_file.write_text("# Plan\nDo stuff.\n") monkeypatch.chdir(tmp_path) _common_patches(monkeypatch) @@ -213,7 +221,7 @@ def test_local_main_writes_stage_to_state(tmp_path, monkeypatch, make_state_dir) result = asyncio.run( run_pipeline( _local_pipeline_path(tmp_path), - argv=["--plan", str(plan_file)], + argv=[], client=client, gremlin_id=gremlin_id, ) @@ -230,8 +238,6 @@ def test_local_main_env_file_vars_reach_verify(tmp_path, monkeypatch): artifact_dir = tmp_path / "artifacts" artifact_dir.mkdir() - plan_file = tmp_path / "plan.md" - plan_file.write_text("# Plan\nDo stuff.\n") dot_gremlins = tmp_path / ".gremlins" dot_gremlins.mkdir() @@ -267,7 +273,7 @@ async def _capturing_shell(cmd, env=None, **kwargs): result = asyncio.run( run_pipeline( _local_pipeline_path(tmp_path), - argv=["--plan", str(plan_file)], + argv=[], client=client, ) ) @@ -289,8 +295,6 @@ def test_local_main_env_file_sourced_with_overlay_dir_set(tmp_path, monkeypatch) proj_dir = tmp_path / "proj" proj_dir.mkdir() - plan_file = proj_dir / "plan.md" - plan_file.write_text("# Plan\nDo stuff.\n") (proj_dir / ".gremlins").mkdir() (proj_dir / ".gremlins" / "env").write_text( "export GREMLIN_ENV_TEST_SENTINEL=from_env_file\n" @@ -333,7 +337,7 @@ async def _capturing_shell(cmd, env=None, **kwargs): result = asyncio.run( run_pipeline( _local_pipeline_path(proj_dir), - argv=["--plan", str(plan_file)], + argv=[], client=client, ) ) @@ -352,8 +356,11 @@ def test_local_main_pipeline_default_client_model(tmp_path, monkeypatch): """ artifact_dir = tmp_path / "artifacts" artifact_dir.mkdir() - plan_file = tmp_path / "plan.md" - plan_file.write_text("# Plan\nDo stuff.\n") + # Pre-seed plan artifact so plan stage is skipped + (artifact_dir / "plan.md").write_text("# Plan\nDo stuff.\n") + (tmp_path / "registry.json").write_text( + json.dumps({"plan-document": "file://session/plan.md"}) + ) monkeypatch.chdir(tmp_path) _common_patches(monkeypatch) @@ -395,7 +402,7 @@ def _from_yaml_copilot_default(path): result = asyncio.run( run_pipeline( _local_pipeline_path(tmp_path), - argv=["--plan", str(plan_file)], + argv=[], client=client, ) ) @@ -406,64 +413,13 @@ def _from_yaml_copilot_default(path): assert client.calls[1].model == "gpt-5.4" -# --------------------------------------------------------------------------- -# stage_inputs wiring: local pipeline reads instructions from state.json -# --------------------------------------------------------------------------- - - -def test_local_stage_inputs_instructions_reach_plan( - tmp_path, monkeypatch, make_state_dir -): - """stage_inputs["instructions"] from state.json is passed to the plan agent stage - and takes precedence over the CLI positional argument.""" - gremlin_id = "test-si-local" - state_dir = make_state_dir(gremlin_id) - - sf = state_dir / "state.json" - state = json.loads(sf.read_text()) - state["stage_inputs"] = {"instructions": "instr from state"} - sf.write_text(json.dumps(state)) - - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - - monkeypatch.chdir(tmp_path) - _common_patches(monkeypatch) - monkeypatch.setattr( - "gremlins.executor.run.resolve_artifact_dir", - lambda gremlin_id=None: artifact_dir, - ) - - client = _ReviewCreatingClient( - fixtures={ - "plan": MINIMAL_EVENTS, - "implement": MINIMAL_EVENTS, - **{lbl: MINIMAL_EVENTS for lbl in _REVIEW_LABELS}, - "address-code": MINIMAL_EVENTS, - } - ) - - result = asyncio.run( - run_pipeline( - _local_pipeline_path(tmp_path), - argv=["instr from cli"], - client=client, - gremlin_id=gremlin_id, - ) - ) - - assert result == 0 - plan_call = next(c for c in client.calls if c.label == "plan") - assert "instr from state" in plan_call.prompt - - def test_plan_skip_if_exists_on_resume(tmp_path, monkeypatch): """Resume: plan stage is skipped when plan artifact is already verified.""" artifact_dir = tmp_path / "artifacts" artifact_dir.mkdir() (artifact_dir / "plan.md").write_text("# Plan\nDo stuff.\n", encoding="utf-8") (tmp_path / "registry.json").write_text( - json.dumps({"plan": "file://session/plan.md"}) + json.dumps({"plan-document": "file://session/plan.md"}) ) monkeypatch.chdir(tmp_path) @@ -483,7 +439,7 @@ def test_plan_skip_if_exists_on_resume(tmp_path, monkeypatch): result = asyncio.run( run_pipeline( _local_pipeline_path(tmp_path), - argv=["implement this feature"], + argv=[], client=client, ) ) @@ -495,8 +451,6 @@ def test_plan_skip_if_exists_on_resume(tmp_path, monkeypatch): def test_startup_fails_in_non_git_dir(tmp_path, monkeypatch, capsys): """gremlins exits with a clear error when cwd is not a git repository.""" - plan_file = tmp_path / "plan.md" - plan_file.write_text("# Plan\nDo stuff.\n") monkeypatch.chdir(tmp_path) monkeypatch.setattr( shutil, "which", lambda n: f"/fake/{n}" if n in ("claude", "git") else None @@ -509,7 +463,7 @@ def test_startup_fails_in_non_git_dir(tmp_path, monkeypatch, capsys): asyncio.run( run_pipeline( _local_pipeline_path(tmp_path), - argv=["--plan", str(plan_file)], + argv=[], client=FakeClaudeClient(fixtures={}), ) ) @@ -518,8 +472,6 @@ def test_startup_fails_in_non_git_dir(tmp_path, monkeypatch, capsys): def test_claude_probe_conditional_on_provider(tmp_path, monkeypatch, capsys): """claude missing errors only for claude provider.""" - plan_file = tmp_path / "plan.md" - plan_file.write_text("# Plan\nDo stuff.\n") monkeypatch.chdir(tmp_path) monkeypatch.setattr( shutil, @@ -549,7 +501,7 @@ async def _test_run_pipeline(pipeline_path, *, argv, gremlin_id=None, client=Non result = asyncio.run( gremlins.executor.run.run_pipeline( _local_pipeline_path(tmp_path), - argv=["--plan", str(plan_file)], + argv=[], client=Client("copilot", "gpt-4o"), ) ) @@ -560,7 +512,7 @@ async def _test_run_pipeline(pipeline_path, *, argv, gremlin_id=None, client=Non asyncio.run( gremlins.executor.run.run_pipeline( _local_pipeline_path(tmp_path), - argv=["--plan", str(plan_file)], + argv=[], client=Client("claude", "sonnet"), ) ) diff --git a/tests/test_recipe_plan.py b/tests/test_recipe_plan.py deleted file mode 100644 index ab764eeb..00000000 --- a/tests/test_recipe_plan.py +++ /dev/null @@ -1,46 +0,0 @@ -"""Tests for plan_arg launcher binding (prereq for plan recipe).""" - -from __future__ import annotations - -import pathlib - -import gremlins.launcher as _launcher - - -def _make_inputs(*, plan: str | None = None) -> _launcher._Inputs: # type: ignore[reportPrivateUsage] - return _launcher._Inputs( # type: ignore[reportPrivateUsage] - gremlin_id="test-abc123", - kind="local", - plan=plan, - instructions="", - description="", - description_explicit=False, - parent_id="", - project_root="/tmp", - pipeline_path="local", - pipeline_args=[], - client_label="claude:sonnet", - fetch_worktree=False, - base_ref_name="main", - base_ref_sha="", - stage_inputs={}, - pr_num="", - ) - - -def test_prepare_state_dir_writes_plan_arg_file(tmp_path: pathlib.Path) -> None: - inputs = _make_inputs(plan="#42") - _launcher._prepare_state_dir(tmp_path, inputs) # type: ignore[attr-defined] - plan_arg_file = tmp_path / "artifacts" / "plan-arg.txt" - assert plan_arg_file.exists() - assert plan_arg_file.read_text() == "#42" - - -def test_prepare_state_dir_writes_empty_plan_arg_when_no_plan( - tmp_path: pathlib.Path, -) -> None: - inputs = _make_inputs(plan=None) - _launcher._prepare_state_dir(tmp_path, inputs) # type: ignore[attr-defined] - plan_arg_file = tmp_path / "artifacts" / "plan-arg.txt" - assert plan_arg_file.exists() - assert plan_arg_file.read_text() == "" diff --git a/tests/test_spawn_child.py b/tests/test_spawn_child.py index acfec43b..f81e1e81 100644 --- a/tests/test_spawn_child.py +++ b/tests/test_spawn_child.py @@ -115,7 +115,7 @@ def test_load_spec_invalid_json(tmp_path: pathlib.Path) -> None: _rc._load_spec(p) -def test_build_state_propagates_repo_and_instructions( +def test_build_state_propagates_repo( tmp_path: pathlib.Path, ) -> None: state = _rc._build_state( @@ -123,11 +123,9 @@ def test_build_state_propagates_repo_and_instructions( "client": "fake:fake", "artifact_dir": str(tmp_path / "artifacts"), "repo": "owner/myrepo", - "instructions": "do the thing", } ) assert state.repo == "owner/myrepo" - assert state.instructions == "do the thing" def test_build_state_repo_defaults_to_empty(tmp_path: pathlib.Path) -> None: @@ -138,7 +136,6 @@ def test_build_state_repo_defaults_to_empty(tmp_path: pathlib.Path) -> None: } ) assert state.repo == "" - assert state.instructions == "" def test_build_state_missing_client() -> None: diff --git a/tests/test_stage_base.py b/tests/test_stage_base.py index 7fafbf20..8d1c40fa 100644 --- a/tests/test_stage_base.py +++ b/tests/test_stage_base.py @@ -94,7 +94,6 @@ def _subs_state() -> State: pipeline_data=_PIPELINE, repo="owner/proj", cwd="/work", - instructions="do the thing", base_ref="trunk", ) @@ -102,9 +101,9 @@ def _subs_state() -> State: def test_substitute_vars_renders_shared_framework_keys() -> None: stage = _SimpleStage("st", [], {}) state = _subs_state() - text = "{name} {model} {artifact_dir} {instructions} {repo} {cwd} {base_ref}" + text = "{name} {model} {artifact_dir} {repo} {cwd} {base_ref}" assert stage.substitute_vars(text, state) == ( - "st fake /tmp/sess do the thing owner/proj /work trunk" + "st fake /tmp/sess owner/proj /work trunk" ) diff --git a/tests/test_state_artifacts.py b/tests/test_state_artifacts.py index cff5a50f..e90e659b 100644 --- a/tests/test_state_artifacts.py +++ b/tests/test_state_artifacts.py @@ -21,15 +21,6 @@ def test_setup_dirs_creates_directories(tmp_path): assert artifact_dir.is_dir() -def test_setup_dirs_writes_instructions(tmp_path): - state_dir = tmp_path / "state" / "gr-1" - artifact_dir = state_dir / "artifacts" - state_mod.State.setup_dirs( - state_dir, artifact_dir, gremlin_id=None, instructions="do x" - ) - assert (state_dir / "instructions.txt").read_text() == "do x" - - def test_setup_dirs_no_gremlin_id_skips_state_json(tmp_path): state_dir = tmp_path / "state" / "gr-1" artifact_dir = state_dir / "artifacts" diff --git a/tests/test_state_isolation.py b/tests/test_state_isolation.py index f23404ba..7154cbcd 100644 --- a/tests/test_state_isolation.py +++ b/tests/test_state_isolation.py @@ -136,8 +136,6 @@ def test_local_main_does_not_clobber_external_state(tmp_path, monkeypatch, sandb artifact_dir = tmp_path / "session-local" artifact_dir.mkdir() - plan_file = tmp_path / "plan.md" - plan_file.write_text("# Plan\nDo stuff.\n") _common_patches(monkeypatch) monkeypatch.setattr( @@ -156,7 +154,7 @@ def test_local_main_does_not_clobber_external_state(tmp_path, monkeypatch, sandb asyncio.run( run_pipeline( resolve_pipeline_path("local", tmp_path), - argv=["--plan", str(plan_file)], + argv=[], client=client, ) )