From 34b7688737857a781b9eee2bbabaa6124d513d5d Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 11:53:49 -0600 Subject: [PATCH 01/23] Add input sources mechanism for pipeline external artifact seeding - Define InputSource and InputSources classes to declare external artifact sources - Parse sources: block from inputs: section in pipeline YAML - Update Pipeline to store input_sources - Register instructions artifact in artifact directory - Add _seed_registry_from_sources function to drive pre-seeding from sources - Update launcher to use sources for registry pre-seeding with fallback to legacy hardcoding - Update gh-terse.yaml to declare issue, plan, and instructions sources --- gremlins/launcher.py | 79 +++++++++++- gremlins/pipeline/__init__.py | 10 ++ gremlins/pipeline/inputs.py | 107 ++++++++++++++++ gremlins/pipelines/gh-terse.yaml | 9 ++ tests/test_input_sources.py | 213 +++++++++++++++++++++++++++++++ 5 files changed, 417 insertions(+), 1 deletion(-) create mode 100644 gremlins/pipeline/inputs.py create mode 100644 tests/test_input_sources.py diff --git a/gremlins/launcher.py b/gremlins/launcher.py index 7e015f33..96f9d189 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -348,6 +348,7 @@ def _prepare_state_dir(state_dir: pathlib.Path, inputs: _Inputs) -> None: artifacts_dir = state_dir / "artifacts" artifacts_dir.mkdir(exist_ok=True) (artifacts_dir / "plan-arg.txt").write_text(inputs.plan or "", encoding="utf-8") + (artifacts_dir / "instructions.txt").write_text(inputs.instructions, encoding="utf-8") def _initial_state_data(inputs: _Inputs) -> StateData: @@ -476,6 +477,74 @@ def _spawn(gremlin_id: str, inputs: _Inputs, state_dir: pathlib.Path) -> Any: ) + +def _seed_registry_from_sources( + registry: ArtifactRegistry, + loaded_pipeline: Any, + inputs: _Inputs, + artifacts_dir: pathlib.Path, +) -> None: + """Pre-seed registry with external sources based on pipeline.input_sources. + + If the pipeline has an input_sources block, use it to drive registry pre-seeding. + Otherwise, fall back to legacy hardcoded seeding for backward compatibility. + """ + if loaded_pipeline is None or loaded_pipeline.input_sources is None: + # Legacy path: hardcoded seeding + registry.bind("plan_arg", Uri.parse("file://session/plan-arg.txt")) + return + + sources = loaded_pipeline.input_sources.all_sources() + for key, source in sources.items(): + # Determine which type to use and resolve the value + value: str | None = None + resolved_type: str | None = None + + # Try filepath first if it's in the union + if "filepath" in source.types: + if inputs.plan and os.path.isfile(inputs.plan): + value = inputs.plan + resolved_type = "filepath" + elif key == "plan" and inputs.plan and os.path.isfile(inputs.plan): + value = inputs.plan + resolved_type = "filepath" + + # Fall back to string type if filepath didn't resolve + if resolved_type is None and "string" in source.types: + if key == "plan" and inputs.plan: + value = inputs.plan + resolved_type = "string" + elif key == "instructions" and inputs.instructions: + value = inputs.instructions + resolved_type = "string" + elif key == "issue" and inputs.plan: + # Issue is typically a string (issue ref like #123) + if not os.path.isfile(inputs.plan): + value = inputs.plan + resolved_type = "string" + + # Check if the source is required and missing + if value is None and not source.optional: + raise ValueError( + f"required input source {key!r} (type: {source.types}) " + f"is not available" + ) + + # Register if we found a value + if value is None: + continue + + if resolved_type == "filepath": + # Bind to the actual file path + uri = Uri.parse(f"file://{value}") + registry.bind(key, uri) + elif resolved_type == "string": + # Write to a file and register it + source_file = artifacts_dir / f"{key}.txt" + source_file.write_text(value, encoding="utf-8") + uri = Uri.parse(f"file://session/{key}.txt") + registry.bind(key, uri) + def launch( kind: str, *, @@ -530,7 +599,15 @@ def launch( 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")) + # Load pipeline to access input_sources for registry pre-seeding + loaded_pipeline = None + try: + loaded_pipeline = _PipelineData.from_yaml( + resolve_pipeline_path(inputs.pipeline_path, pathlib.Path(inputs.project_root)) + ) + except (FileNotFoundError, OSError, ValueError): + pass + _seed_registry_from_sources(registry, loaded_pipeline, inputs, artifact_dir) p = _spawn(inputs.gremlin_id, inputs, state_dir) except Exception: shutil.rmtree(state_dir, ignore_errors=True) diff --git a/gremlins/pipeline/__init__.py b/gremlins/pipeline/__init__.py index 0916faf9..d01d0af7 100644 --- a/gremlins/pipeline/__init__.py +++ b/gremlins/pipeline/__init__.py @@ -6,6 +6,7 @@ from typing import TYPE_CHECKING, Any, cast from gremlins.clients.client import PACKAGE_DEFAULT, Client +from gremlins.pipeline.inputs import InputSources if TYPE_CHECKING: from gremlins.stages.base import Stage @@ -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 @@ -80,10 +82,17 @@ def from_yaml(cls, path: pathlib.Path) -> Pipeline: 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") + # Parse sources: block if present + 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(sources_raw) inputs_stage = Exec.with_dict({"name": "inputs", **inputs_raw}) land_stage: Exec | None = None @@ -107,6 +116,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..607ae2ac --- /dev/null +++ b/gremlins/pipeline/inputs.py @@ -0,0 +1,107 @@ +"""Input source declarations for pipelines. + +Defines how external artifacts (files, CLI args, etc.) are mapped into the +artifact registry for a pipeline's inputs stage. +""" + +from __future__ import annotations + +import dataclasses +import pathlib +from typing import Any, cast + + +@dataclasses.dataclass +class InputSource: + """A single input source declaration. + + Attributes: + name: registry key name (e.g., 'plan', 'instructions') + types: source type(s): 'filepath' (local file) or 'string' (CLI string) + optional: if True, absence of the source is not an error + """ + + name: str + types: list[str] + optional: bool = False + + def __post_init__(self) -> None: + # Validate that all types are recognized + 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))}" + ) + if not self.types: + raise ValueError(f"input source {self.name!r}: types list must not be empty") + + +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. + + Expected format: + sources: + issue: + type: string + plan: + type: [filepath, string] + optional: true + instructions: + type: string + optional: true + """ + 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__}" + ) + + # 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): + types = cast(list[str], type_raw) + if not types: + raise ValueError( + f"input source {key!r}: type list must not be empty" + ) + if not all(isinstance(t, str) for t in types): + raise ValueError( + f"input source {key!r}: all type entries must be strings" + ) + else: + raise ValueError( + f"input source {key!r}: 'type' must be a string or list of strings, " + f"got {type(type_raw).__name__}" + ) + + optional = bool(entry.get("optional", False)) + sources[key] = InputSource(name=key, types=types, optional=optional) + + 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) -> dict[str, InputSource]: + """Return all declared sources.""" + return dict(self.sources) + + def required_sources(self) -> dict[str, InputSource]: + """Return only required (non-optional) sources.""" + return {k: v for k, v in self.sources.items() if not v.optional} diff --git a/gremlins/pipelines/gh-terse.yaml b/gremlins/pipelines/gh-terse.yaml index b3a9e74d..38e86ffb 100644 --- a/gremlins/pipelines/gh-terse.yaml +++ b/gremlins/pipelines/gh-terse.yaml @@ -9,6 +9,15 @@ inputs: INSTRUCTIONS: instructions? PLAN: plan? PR: pr? + sources: + issue: + type: string + plan: + type: [filepath, string] + optional: true + instructions: + type: string + optional: true land: in: diff --git a/tests/test_input_sources.py b/tests/test_input_sources.py new file mode 100644 index 00000000..a6451921 --- /dev/null +++ b/tests/test_input_sources.py @@ -0,0 +1,213 @@ +"""Tests for pipeline input sources.""" + +import pathlib +import textwrap + +import pytest + +from gremlins.pipeline import Pipeline +from gremlins.pipeline.inputs import InputSources, InputSource + + +class TestInputSource: + def test_single_type(self) -> None: + source = InputSource(name="issue", types=["string"]) + assert source.name == "issue" + assert source.types == ["string"] + assert source.optional is False + + def test_union_type(self) -> None: + source = InputSource(name="plan", types=["filepath", "string"]) + assert source.types == ["filepath", "string"] + + def test_optional(self) -> None: + source = InputSource(name="plan", types=["string"], optional=True) + assert source.optional is True + + def test_unknown_type_rejected(self) -> None: + with pytest.raises(ValueError, match="unknown type"): + InputSource(name="bad", types=["unknown"]) + + def test_empty_types_rejected(self) -> None: + with pytest.raises(ValueError, match="types list must not be empty"): + InputSource(name="bad", types=[]) + + +class TestInputSources: + def test_parse_simple_string_source(self) -> None: + raw = { + "issue": { + "type": "string", + } + } + sources = InputSources.from_yaml(raw) + issue = sources.get("issue") + assert issue is not None + assert issue.name == "issue" + assert issue.types == ["string"] + assert issue.optional is False + + def test_parse_union_type_source(self) -> None: + raw = { + "plan": { + "type": ["filepath", "string"], + } + } + sources = InputSources.from_yaml(raw) + plan = sources.get("plan") + assert plan is not None + assert plan.types == ["filepath", "string"] + + def test_parse_optional_source(self) -> None: + raw = { + "instructions": { + "type": "string", + "optional": True, + } + } + sources = InputSources.from_yaml(raw) + instr = sources.get("instructions") + assert instr is not None + assert instr.optional is True + + def test_parse_multiple_sources(self) -> None: + raw = { + "issue": {"type": "string"}, + "plan": {"type": ["filepath", "string"], "optional": True}, + "instructions": {"type": "string", "optional": True}, + } + sources = InputSources.from_yaml(raw) + assert len(sources.all_sources()) == 3 + assert sources.get("issue") is not None + assert sources.get("plan") is not None + assert sources.get("instructions") is not None + + 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: + raw = { + "issue": {}, + } + with pytest.raises(ValueError, match="missing required 'type' field"): + InputSources.from_yaml(raw) + + def test_invalid_type_value_rejected(self) -> None: + raw = { + "issue": { + "type": "invalid-type", + } + } + with pytest.raises(ValueError, match="unknown type"): + InputSources.from_yaml(raw) + + def test_type_list_with_non_string_rejected(self) -> None: + raw = { + "issue": { + "type": ["string", 123], + } + } + with pytest.raises(ValueError, match="all type entries must be strings"): + InputSources.from_yaml(raw) + + def test_empty_type_list_rejected(self) -> None: + raw = { + "issue": { + "type": [], + } + } + with pytest.raises(ValueError, match="type list must not be empty"): + InputSources.from_yaml(raw) + + def test_non_mapping_source_rejected(self) -> None: + raw = { + "issue": ["string"], + } + with pytest.raises(ValueError, match="expected a mapping"): + InputSources.from_yaml(raw) + + +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? + sources: + issue: + type: string + plan: + type: [filepath, string] + optional: true + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(p) + assert pipeline.input_sources is not None + assert pipeline.input_sources.get("issue") is not None + assert pipeline.input_sources.get("plan") is not None + issue = pipeline.input_sources.get("issue") + assert issue.types == ["string"] + plan = pipeline.input_sources.get("plan") + 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? + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(p) + assert pipeline.input_sources is None + + def test_parse_pipeline_without_inputs(self, tmp_path: pathlib.Path) -> None: + p = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(p) + assert pipeline.inputs is None + 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) From 9bad2ece841df30d85f452725ce563f978bee238 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 11:55:38 -0600 Subject: [PATCH 02/23] Add comprehensive tests for launcher registry seeding from input sources - Test legacy fallback when no input_sources present - Test string sources for issue, plan, and instructions - Test optional vs required source handling - Test union types (filepath + string) - Test error cases for missing required sources --- tests/test_launcher_input_sources.py | 356 +++++++++++++++++++++++++++ 1 file changed, 356 insertions(+) create mode 100644 tests/test_launcher_input_sources.py diff --git a/tests/test_launcher_input_sources.py b/tests/test_launcher_input_sources.py new file mode 100644 index 00000000..ea856d1c --- /dev/null +++ b/tests/test_launcher_input_sources.py @@ -0,0 +1,356 @@ +"""Tests for launcher registry seeding from input sources.""" + +import pathlib +import tempfile +import textwrap + +import pytest + +from gremlins.launcher import _seed_registry_from_sources, _Inputs +from gremlins.artifacts.registry import ArtifactRegistry +from gremlins.pipeline import Pipeline + + +class TestSeedRegistryFromSources: + def _write_pipeline( + self, tmp_path: pathlib.Path, yaml_content: str + ) -> pathlib.Path: + p = tmp_path / "pipeline.yaml" + p.write_text(textwrap.dedent(yaml_content), encoding="utf-8") + return p + + def _make_inputs( + self, + plan: str | None = None, + instructions: str = "", + project_root: str = "", + ) -> _Inputs: + return _Inputs( + gremlin_id="test-id", + kind="local", + plan=plan, + instructions=instructions, + description="test", + description_explicit=False, + parent_id="", + project_root=project_root or str(pathlib.Path.cwd()), + pipeline_path="pipeline.yaml", + pipeline_args=[], + client_label="claude:sonnet", + fetch_worktree=False, + base_ref_name="", + base_ref_sha="", + stage_inputs={}, + ) + + def test_legacy_fallback_when_no_input_sources( + self, tmp_path: pathlib.Path + ) -> None: + """When pipeline has no input_sources, fall back to hardcoded plan_arg.""" + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + registry = ArtifactRegistry(artifact_dir=artifact_dir) + + pipeline_path = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(pipeline_path) + inputs = self._make_inputs(plan="test plan") + + _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + + # Should have registered plan_arg in legacy mode + assert "plan_arg" in registry.data + assert registry.data["plan_arg"] == "file://session/plan-arg.txt" + + def test_legacy_fallback_when_loaded_pipeline_none( + self, tmp_path: pathlib.Path + ) -> None: + """When loaded_pipeline is None, use legacy hardcoded seeding.""" + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + registry = ArtifactRegistry(artifact_dir=artifact_dir) + + inputs = self._make_inputs(plan="test plan") + + _seed_registry_from_sources(registry, None, inputs, artifact_dir) + + # Should have registered plan_arg in legacy mode + assert "plan_arg" in registry.data + assert registry.data["plan_arg"] == "file://session/plan-arg.txt" + + def test_string_source_as_issue(self, tmp_path: pathlib.Path) -> None: + """String source 'issue' registers issue reference.""" + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + registry = ArtifactRegistry(artifact_dir=artifact_dir) + + pipeline_path = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + sources: + issue: + type: string + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(pipeline_path) + inputs = self._make_inputs(plan="#123") + + _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + + # Issue should be registered as a string + assert "issue" in registry.data + assert registry.data["issue"] == "file://session/issue.txt" + issue_file = artifact_dir / "issue.txt" + assert issue_file.read_text(encoding="utf-8") == "#123" + + def test_string_source_as_plan(self, tmp_path: pathlib.Path) -> None: + """String source 'plan' registers plan as a string.""" + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + registry = ArtifactRegistry(artifact_dir=artifact_dir) + + pipeline_path = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + sources: + plan: + type: string + optional: true + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(pipeline_path) + inputs = self._make_inputs(plan="some plan text") + + _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + + # Plan should be registered as a string + assert "plan" in registry.data + assert registry.data["plan"] == "file://session/plan.txt" + plan_file = artifact_dir / "plan.txt" + assert plan_file.read_text(encoding="utf-8") == "some plan text" + + def test_string_source_as_instructions(self, tmp_path: pathlib.Path) -> None: + """String source 'instructions' registers instructions.""" + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + registry = ArtifactRegistry(artifact_dir=artifact_dir) + + pipeline_path = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + sources: + instructions: + type: string + optional: true + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(pipeline_path) + inputs = self._make_inputs(instructions="some instructions") + + _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + + # Instructions should be registered + assert "instructions" in registry.data + assert registry.data["instructions"] == "file://session/instructions.txt" + instr_file = artifact_dir / "instructions.txt" + assert instr_file.read_text(encoding="utf-8") == "some instructions" + + def test_optional_source_missing(self, tmp_path: pathlib.Path) -> None: + """Optional source absent → key not in registry.""" + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + registry = ArtifactRegistry(artifact_dir=artifact_dir) + + pipeline_path = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + sources: + plan: + type: string + optional: true + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(pipeline_path) + # No plan provided + inputs = self._make_inputs() + + _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + + # Plan should not be in registry since it's optional and not provided + assert "plan" not in registry.data + + def test_required_source_missing_raises(self, tmp_path: pathlib.Path) -> None: + """Required source missing → error at launch time.""" + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + registry = ArtifactRegistry(artifact_dir=artifact_dir) + + pipeline_path = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + sources: + issue: + type: string + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(pipeline_path) + # No plan provided (issue requires it) + inputs = self._make_inputs() + + with pytest.raises(ValueError, match="required input source"): + _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + + def test_union_type_filepath_first(self, tmp_path: pathlib.Path) -> None: + """Union source given as resolvable file path → resolves as filepath.""" + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + registry = ArtifactRegistry(artifact_dir=artifact_dir) + + # Create a test file + plan_file = tmp_path / "plan.md" + plan_file.write_text("# My Plan\nSome content", encoding="utf-8") + + pipeline_path = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + sources: + plan: + type: [filepath, string] + optional: true + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(pipeline_path) + inputs = self._make_inputs(plan=str(plan_file)) + + _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + + # Plan should be registered as a filepath URI + assert "plan" in registry.data + assert registry.data["plan"].startswith("file://") + assert str(plan_file) in registry.data["plan"] + + def test_union_type_falls_back_to_string(self, tmp_path: pathlib.Path) -> None: + """Union source given as plain string → registered as string.""" + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + registry = ArtifactRegistry(artifact_dir=artifact_dir) + + pipeline_path = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + sources: + plan: + type: [filepath, string] + optional: true + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(pipeline_path) + # Provide a string that's not a file path (issue ref) + inputs = self._make_inputs(plan="#123") + + _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + + # Plan should be registered as a string (since file doesn't exist) + assert "plan" in registry.data + assert registry.data["plan"] == "file://session/plan.txt" + plan_file = artifact_dir / "plan.txt" + assert plan_file.read_text(encoding="utf-8") == "#123" + + def test_filepath_only_source_missing_file_raises( + self, tmp_path: pathlib.Path + ) -> None: + """Filepath-only source with unresolvable path → error.""" + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + registry = ArtifactRegistry(artifact_dir=artifact_dir) + + pipeline_path = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + sources: + plan: + type: filepath + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(pipeline_path) + # Provide a path that doesn't exist + inputs = self._make_inputs(plan="/nonexistent/path.md") + + with pytest.raises(ValueError, match="required input source"): + _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + + def test_multiple_sources_partially_satisfied( + self, tmp_path: pathlib.Path + ) -> None: + """Multiple sources with mixed required/optional - only instructions absent.""" + artifact_dir = tmp_path / "artifacts" + artifact_dir.mkdir() + registry = ArtifactRegistry(artifact_dir=artifact_dir) + + pipeline_path = self._write_pipeline( + tmp_path, + """\ + default_client: claude:sonnet + inputs: + sources: + issue: + type: string + plan: + type: string + optional: true + instructions: + type: string + optional: true + stages: + - { name: plan, type: agent } + """, + ) + pipeline = Pipeline.from_yaml(pipeline_path) + # Provide plan (which satisfies both issue and plan) + inputs = self._make_inputs(plan="#456") + + _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + + # Issue and plan should both be registered since plan satisfies both + # Instructions should not be registered (optional and not provided) + assert "issue" in registry.data + assert "plan" in registry.data + assert "instructions" not in registry.data From b10911d7863c548dd2830ea8adb51825059bc5d4 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 11:57:20 -0600 Subject: [PATCH 03/23] Refactor source resolution logic for clarity - Resolve values based on source key (plan, issue, instructions) - Avoid mixing plan argument with issue/plan source semantics - Issue only uses plan arg if it's not a file (i.e., issue reference) - Improves testability and semantic correctness --- gremlins/launcher.py | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/gremlins/launcher.py b/gremlins/launcher.py index 96f9d189..a5cf75f8 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -500,28 +500,28 @@ def _seed_registry_from_sources( value: str | None = None resolved_type: str | None = None - # Try filepath first if it's in the union - if "filepath" in source.types: - if inputs.plan and os.path.isfile(inputs.plan): - value = inputs.plan - resolved_type = "filepath" - elif key == "plan" and inputs.plan and os.path.isfile(inputs.plan): - value = inputs.plan - resolved_type = "filepath" - - # Fall back to string type if filepath didn't resolve - if resolved_type is None and "string" in source.types: - if key == "plan" and inputs.plan: - value = inputs.plan - resolved_type = "string" - elif key == "instructions" and inputs.instructions: - value = inputs.instructions - resolved_type = "string" - elif key == "issue" and inputs.plan: - # Issue is typically a string (issue ref like #123) + # Resolve the value based on the source key + if key == "plan": + # Plan can be a file path or a string (issue ref or plain text) + if inputs.plan: + if "filepath" in source.types and os.path.isfile(inputs.plan): + value = inputs.plan + resolved_type = "filepath" + elif "string" in source.types: + value = inputs.plan + resolved_type = "string" + elif key == "issue": + # Issue is typically a string (issue ref like #123 or owner/repo#123) + # Only use inputs.plan if it's NOT a file (i.e., it's an issue reference) + if inputs.plan and "string" in source.types: if not os.path.isfile(inputs.plan): value = inputs.plan resolved_type = "string" + elif key == "instructions": + # Instructions comes from inputs.instructions + if inputs.instructions and "string" in source.types: + value = inputs.instructions + resolved_type = "string" # Check if the source is required and missing if value is None and not source.optional: @@ -545,6 +545,7 @@ def _seed_registry_from_sources( uri = Uri.parse(f"file://session/{key}.txt") registry.bind(key, uri) + def launch( kind: str, *, From ff35c195084384f58fc8876a85f9042158197ac5 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:02:51 -0600 Subject: [PATCH 04/23] Address review feedback Removed unused pathlib import and backward-compat legacy path. Trimmed oversized docstrings to one-line max. Moved empty-types validation to top of __post_init__. Deleted dead methods all_sources() and required_sources(). Eliminated what-narration comments throughout _seed_registry_from_sources. Changed exception handling to only catch file-not-found errors, letting validation errors propagate. Added validation in InputSources.from_yaml to restrict allowed source keys at parse time. Threaded loaded_pipeline through _Inputs to eliminate duplicate pipeline loads. Addresses code review and complexity review findings. Co-Authored-By: Claude Haiku 4.5 --- gremlins/launcher.py | 34 +++++-------------------- gremlins/pipeline/__init__.py | 1 - gremlins/pipeline/inputs.py | 48 ++++++++--------------------------- 3 files changed, 17 insertions(+), 66 deletions(-) diff --git a/gremlins/launcher.py b/gremlins/launcher.py index a5cf75f8..63b6a8c1 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -122,6 +122,7 @@ class _Inputs: base_ref_name: str base_ref_sha: str stage_inputs: dict[str, Any] + loaded_pipeline: Any = None pr_num: str = "" @@ -289,7 +290,7 @@ def _resolve_inputs( loaded_pipeline = _PipelineData.from_yaml( resolve_pipeline_path(pipeline_path, pathlib.Path(project_root)) ) - except (FileNotFoundError, OSError, ValueError): + except (FileNotFoundError, OSError): pass if ( @@ -338,6 +339,7 @@ def _resolve_inputs( base_ref_name=base_ref_name, base_ref_sha=base_ref_sha, stage_inputs=stage_inputs, + loaded_pipeline=loaded_pipeline, pr_num=pr_num, ) @@ -484,25 +486,16 @@ def _seed_registry_from_sources( inputs: _Inputs, artifacts_dir: pathlib.Path, ) -> None: - """Pre-seed registry with external sources based on pipeline.input_sources. - - If the pipeline has an input_sources block, use it to drive registry pre-seeding. - Otherwise, fall back to legacy hardcoded seeding for backward compatibility. - """ + """Pre-seed registry with external sources based on pipeline.input_sources.""" if loaded_pipeline is None or loaded_pipeline.input_sources is None: - # Legacy path: hardcoded seeding - registry.bind("plan_arg", Uri.parse("file://session/plan-arg.txt")) return - sources = loaded_pipeline.input_sources.all_sources() + sources = loaded_pipeline.input_sources.sources for key, source in sources.items(): - # Determine which type to use and resolve the value value: str | None = None resolved_type: str | None = None - # Resolve the value based on the source key if key == "plan": - # Plan can be a file path or a string (issue ref or plain text) if inputs.plan: if "filepath" in source.types and os.path.isfile(inputs.plan): value = inputs.plan @@ -511,35 +504,28 @@ def _seed_registry_from_sources( value = inputs.plan resolved_type = "string" elif key == "issue": - # Issue is typically a string (issue ref like #123 or owner/repo#123) - # Only use inputs.plan if it's NOT a file (i.e., it's an issue reference) if inputs.plan and "string" in source.types: if not os.path.isfile(inputs.plan): value = inputs.plan resolved_type = "string" elif key == "instructions": - # Instructions comes from inputs.instructions if inputs.instructions and "string" in source.types: value = inputs.instructions resolved_type = "string" - # Check if the source is required and missing if value is None and not source.optional: raise ValueError( f"required input source {key!r} (type: {source.types}) " f"is not available" ) - # Register if we found a value if value is None: continue if resolved_type == "filepath": - # Bind to the actual file path uri = Uri.parse(f"file://{value}") registry.bind(key, uri) elif resolved_type == "string": - # Write to a file and register it source_file = artifacts_dir / f"{key}.txt" source_file.write_text(value, encoding="utf-8") uri = Uri.parse(f"file://session/{key}.txt") @@ -600,15 +586,7 @@ def launch( 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")) - # Load pipeline to access input_sources for registry pre-seeding - loaded_pipeline = None - try: - loaded_pipeline = _PipelineData.from_yaml( - resolve_pipeline_path(inputs.pipeline_path, pathlib.Path(inputs.project_root)) - ) - except (FileNotFoundError, OSError, ValueError): - pass - _seed_registry_from_sources(registry, loaded_pipeline, inputs, artifact_dir) + _seed_registry_from_sources(registry, inputs.loaded_pipeline, inputs, artifact_dir) p = _spawn(inputs.gremlin_id, inputs, state_dir) except Exception: shutil.rmtree(state_dir, ignore_errors=True) diff --git a/gremlins/pipeline/__init__.py b/gremlins/pipeline/__init__.py index d01d0af7..035771e9 100644 --- a/gremlins/pipeline/__init__.py +++ b/gremlins/pipeline/__init__.py @@ -87,7 +87,6 @@ def from_yaml(cls, path: pathlib.Path) -> Pipeline: if inputs_raw is not None: if not isinstance(inputs_raw, dict): raise ValueError("'inputs' must be a mapping") - # Parse sources: block if present sources_raw = inputs_raw.get("sources") if sources_raw is not None: if not isinstance(sources_raw, dict): diff --git a/gremlins/pipeline/inputs.py b/gremlins/pipeline/inputs.py index 607ae2ac..b0bd9aae 100644 --- a/gremlins/pipeline/inputs.py +++ b/gremlins/pipeline/inputs.py @@ -1,32 +1,22 @@ -"""Input source declarations for pipelines. - -Defines how external artifacts (files, CLI args, etc.) are mapped into the -artifact registry for a pipeline's inputs stage. -""" +"""Input source declarations for pipelines.""" from __future__ import annotations import dataclasses -import pathlib from typing import Any, cast @dataclasses.dataclass class InputSource: - """A single input source declaration. - - Attributes: - name: registry key name (e.g., 'plan', 'instructions') - types: source type(s): 'filepath' (local file) or 'string' (CLI string) - optional: if True, absence of the source is not an error - """ + """A single input source declaration.""" name: str types: list[str] optional: bool = False def __post_init__(self) -> None: - # Validate that all types are recognized + 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: @@ -34,8 +24,6 @@ def __post_init__(self) -> None: f"input source {self.name!r}: unknown type {t!r}. " f"Supported types: {', '.join(sorted(valid_types))}" ) - if not self.types: - raise ValueError(f"input source {self.name!r}: types list must not be empty") class InputSources: @@ -46,21 +34,15 @@ def __init__(self, sources: dict[str, InputSource] | None = None) -> None: @classmethod def from_yaml(cls, raw: dict[str, Any]) -> InputSources: - """Parse sources: block from YAML. - - Expected format: - sources: - issue: - type: string - plan: - type: [filepath, string] - optional: true - instructions: - type: string - optional: true - """ + """Parse sources: block from YAML.""" + allowed_keys = {"plan", "issue", "instructions"} sources: dict[str, InputSource] = {} for key, entry in raw.items(): + if key not in allowed_keys: + raise ValueError( + f"input source {key!r}: unrecognized key. " + f"Allowed keys: {', '.join(sorted(allowed_keys))}" + ) if not isinstance(entry, dict): raise ValueError( f"input source {key!r}: expected a mapping, got {type(entry).__name__}" @@ -97,11 +79,3 @@ def from_yaml(cls, raw: dict[str, Any]) -> InputSources: 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) -> dict[str, InputSource]: - """Return all declared sources.""" - return dict(self.sources) - - def required_sources(self) -> dict[str, InputSource]: - """Return only required (non-optional) sources.""" - return {k: v for k, v in self.sources.items() if not v.optional} From 03d4fedaebd7aebfaa724b3eda7a5542e477284f Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:03:00 -0600 Subject: [PATCH 05/23] normalize --- gremlins/launcher.py | 12 +++++++----- gremlins/pipeline/inputs.py | 4 +++- tests/test_input_sources.py | 6 ++---- tests/test_launcher_input_sources.py | 7 ++----- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/gremlins/launcher.py b/gremlins/launcher.py index 63b6a8c1..ff6fd6fc 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -350,7 +350,9 @@ def _prepare_state_dir(state_dir: pathlib.Path, inputs: _Inputs) -> None: artifacts_dir = state_dir / "artifacts" artifacts_dir.mkdir(exist_ok=True) (artifacts_dir / "plan-arg.txt").write_text(inputs.plan or "", encoding="utf-8") - (artifacts_dir / "instructions.txt").write_text(inputs.instructions, encoding="utf-8") + (artifacts_dir / "instructions.txt").write_text( + inputs.instructions, encoding="utf-8" + ) def _initial_state_data(inputs: _Inputs) -> StateData: @@ -479,7 +481,6 @@ def _spawn(gremlin_id: str, inputs: _Inputs, state_dir: pathlib.Path) -> Any: ) - def _seed_registry_from_sources( registry: ArtifactRegistry, loaded_pipeline: Any, @@ -515,8 +516,7 @@ def _seed_registry_from_sources( if value is None and not source.optional: raise ValueError( - f"required input source {key!r} (type: {source.types}) " - f"is not available" + f"required input source {key!r} (type: {source.types}) is not available" ) if value is None: @@ -586,7 +586,9 @@ def launch( 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")) - _seed_registry_from_sources(registry, inputs.loaded_pipeline, inputs, artifact_dir) + _seed_registry_from_sources( + registry, inputs.loaded_pipeline, inputs, artifact_dir + ) p = _spawn(inputs.gremlin_id, inputs, state_dir) except Exception: shutil.rmtree(state_dir, ignore_errors=True) diff --git a/gremlins/pipeline/inputs.py b/gremlins/pipeline/inputs.py index b0bd9aae..95dccd8b 100644 --- a/gremlins/pipeline/inputs.py +++ b/gremlins/pipeline/inputs.py @@ -16,7 +16,9 @@ class InputSource: def __post_init__(self) -> None: if not self.types: - raise ValueError(f"input source {self.name!r}: types list must not be empty") + 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: diff --git a/tests/test_input_sources.py b/tests/test_input_sources.py index a6451921..4546d8da 100644 --- a/tests/test_input_sources.py +++ b/tests/test_input_sources.py @@ -6,7 +6,7 @@ import pytest from gremlins.pipeline import Pipeline -from gremlins.pipeline.inputs import InputSources, InputSource +from gremlins.pipeline.inputs import InputSource, InputSources class TestInputSource: @@ -168,9 +168,7 @@ def test_parse_pipeline_with_input_sources(self, tmp_path: pathlib.Path) -> None assert plan.types == ["filepath", "string"] assert plan.optional is True - def test_parse_pipeline_without_input_sources( - self, tmp_path: pathlib.Path - ) -> None: + def test_parse_pipeline_without_input_sources(self, tmp_path: pathlib.Path) -> None: p = self._write_pipeline( tmp_path, """\ diff --git a/tests/test_launcher_input_sources.py b/tests/test_launcher_input_sources.py index ea856d1c..32d3d317 100644 --- a/tests/test_launcher_input_sources.py +++ b/tests/test_launcher_input_sources.py @@ -1,13 +1,12 @@ """Tests for launcher registry seeding from input sources.""" import pathlib -import tempfile import textwrap import pytest -from gremlins.launcher import _seed_registry_from_sources, _Inputs from gremlins.artifacts.registry import ArtifactRegistry +from gremlins.launcher import _Inputs, _seed_registry_from_sources from gremlins.pipeline import Pipeline @@ -317,9 +316,7 @@ def test_filepath_only_source_missing_file_raises( with pytest.raises(ValueError, match="required input source"): _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) - def test_multiple_sources_partially_satisfied( - self, tmp_path: pathlib.Path - ) -> None: + def test_multiple_sources_partially_satisfied(self, tmp_path: pathlib.Path) -> None: """Multiple sources with mixed required/optional - only instructions absent.""" artifact_dir = tmp_path / "artifacts" artifact_dir.mkdir() From cccb772a9aa7f15db7c30fd3a324b62b1e4eb494 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:03:58 -0600 Subject: [PATCH 06/23] Fix failing checks Co-Authored-By: Claude Haiku 4.5 --- gremlins/pipeline/__init__.py | 5 ++++- gremlins/pipeline/inputs.py | 15 +++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/gremlins/pipeline/__init__.py b/gremlins/pipeline/__init__.py index 035771e9..1e136d06 100644 --- a/gremlins/pipeline/__init__.py +++ b/gremlins/pipeline/__init__.py @@ -87,11 +87,14 @@ def from_yaml(cls, path: pathlib.Path) -> Pipeline: 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(sources_raw) + 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 diff --git a/gremlins/pipeline/inputs.py b/gremlins/pipeline/inputs.py index 95dccd8b..dbc38790 100644 --- a/gremlins/pipeline/inputs.py +++ b/gremlins/pipeline/inputs.py @@ -49,6 +49,7 @@ def from_yaml(cls, raw: dict[str, Any]) -> InputSources: 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") @@ -58,15 +59,17 @@ def from_yaml(cls, raw: dict[str, Any]) -> InputSources: if isinstance(type_raw, str): types = [type_raw] elif isinstance(type_raw, list): - types = cast(list[str], type_raw) - if not types: + type_raw = cast(list[Any], type_raw) + if not type_raw: raise ValueError( f"input source {key!r}: type list must not be empty" ) - if not all(isinstance(t, str) for t in types): - raise ValueError( - f"input source {key!r}: all type entries must be strings" - ) + 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, " From 85b321ad569bd8038aeb3acc15080097e881eec2 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:06:15 -0600 Subject: [PATCH 07/23] Fix failing checks --- gremlins/launcher.py | 6 ++++++ gremlins/pipeline/inputs.py | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/gremlins/launcher.py b/gremlins/launcher.py index ff6fd6fc..e0ec4b72 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -488,7 +488,13 @@ def _seed_registry_from_sources( artifacts_dir: pathlib.Path, ) -> None: """Pre-seed registry with external sources based on pipeline.input_sources.""" + # Legacy fallback: if no input_sources defined, register plan_arg if plan is provided if loaded_pipeline is None or loaded_pipeline.input_sources is None: + if inputs.plan: + source_file = artifacts_dir / "plan-arg.txt" + source_file.write_text(inputs.plan, encoding="utf-8") + uri = Uri.parse("file://session/plan-arg.txt") + registry.bind("plan_arg", uri) return sources = loaded_pipeline.input_sources.sources diff --git a/gremlins/pipeline/inputs.py b/gremlins/pipeline/inputs.py index dbc38790..798e22fe 100644 --- a/gremlins/pipeline/inputs.py +++ b/gremlins/pipeline/inputs.py @@ -84,3 +84,11 @@ def from_yaml(cls, raw: dict[str, Any]) -> InputSources: 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] From 3e5d3dddb370a313cbcd101488dd4f072713dbf8 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:17:56 -0600 Subject: [PATCH 08/23] Address review comments on PR #1082 - Fix type annotation on loaded_pipeline to _PipelineData | None (not Any) - Move InputSources import under TYPE_CHECKING to avoid re-export - Remove redundant instructions.txt write in _prepare_state_dir - Remove redundant plan-arg.txt write in legacy fallback path - Always seed plan_arg when --plan is provided, even with input_sources - Make issue source optional in gh-terse.yaml for file-backed plans - Validate optional field is actually boolean in InputSources.from_yaml - Allow arbitrary source keys in InputSources.from_yaml for extensibility --- gremlins/launcher.py | 15 ++++++++------- gremlins/pipeline/__init__.py | 2 +- gremlins/pipeline/inputs.py | 15 +++++++-------- gremlins/pipelines/gh-terse.yaml | 1 + 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/gremlins/launcher.py b/gremlins/launcher.py index e0ec4b72..e90aafaa 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -122,7 +122,7 @@ class _Inputs: base_ref_name: str base_ref_sha: str stage_inputs: dict[str, Any] - loaded_pipeline: Any = None + loaded_pipeline: _PipelineData | None = None pr_num: str = "" @@ -350,9 +350,6 @@ def _prepare_state_dir(state_dir: pathlib.Path, inputs: _Inputs) -> None: artifacts_dir = state_dir / "artifacts" artifacts_dir.mkdir(exist_ok=True) (artifacts_dir / "plan-arg.txt").write_text(inputs.plan or "", encoding="utf-8") - (artifacts_dir / "instructions.txt").write_text( - inputs.instructions, encoding="utf-8" - ) def _initial_state_data(inputs: _Inputs) -> StateData: @@ -483,7 +480,7 @@ def _spawn(gremlin_id: str, inputs: _Inputs, state_dir: pathlib.Path) -> Any: def _seed_registry_from_sources( registry: ArtifactRegistry, - loaded_pipeline: Any, + loaded_pipeline: _PipelineData | None, inputs: _Inputs, artifacts_dir: pathlib.Path, ) -> None: @@ -491,13 +488,17 @@ def _seed_registry_from_sources( # Legacy fallback: if no input_sources defined, register plan_arg if plan is provided if loaded_pipeline is None or loaded_pipeline.input_sources is None: if inputs.plan: - source_file = artifacts_dir / "plan-arg.txt" - source_file.write_text(inputs.plan, encoding="utf-8") uri = Uri.parse("file://session/plan-arg.txt") registry.bind("plan_arg", uri) return sources = loaded_pipeline.input_sources.sources + + # Always seed plan_arg if plan is provided, even when input_sources is present + if inputs.plan: + uri = Uri.parse("file://session/plan-arg.txt") + registry.bind("plan_arg", uri) + for key, source in sources.items(): value: str | None = None resolved_type: str | None = None diff --git a/gremlins/pipeline/__init__.py b/gremlins/pipeline/__init__.py index 1e136d06..bb874fbf 100644 --- a/gremlins/pipeline/__init__.py +++ b/gremlins/pipeline/__init__.py @@ -6,9 +6,9 @@ from typing import TYPE_CHECKING, Any, cast from gremlins.clients.client import PACKAGE_DEFAULT, Client -from gremlins.pipeline.inputs import InputSources if TYPE_CHECKING: + from gremlins.pipeline.inputs import InputSources from gremlins.stages.base import Stage from gremlins.stages.exec import Exec diff --git a/gremlins/pipeline/inputs.py b/gremlins/pipeline/inputs.py index 798e22fe..12ddac18 100644 --- a/gremlins/pipeline/inputs.py +++ b/gremlins/pipeline/inputs.py @@ -37,14 +37,8 @@ def __init__(self, sources: dict[str, InputSource] | None = None) -> None: @classmethod def from_yaml(cls, raw: dict[str, Any]) -> InputSources: """Parse sources: block from YAML.""" - allowed_keys = {"plan", "issue", "instructions"} sources: dict[str, InputSource] = {} for key, entry in raw.items(): - if key not in allowed_keys: - raise ValueError( - f"input source {key!r}: unrecognized key. " - f"Allowed keys: {', '.join(sorted(allowed_keys))}" - ) if not isinstance(entry, dict): raise ValueError( f"input source {key!r}: expected a mapping, got {type(entry).__name__}" @@ -76,8 +70,13 @@ def from_yaml(cls, raw: dict[str, Any]) -> InputSources: f"got {type(type_raw).__name__}" ) - optional = bool(entry.get("optional", False)) - sources[key] = InputSource(name=key, types=types, optional=optional) + 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) diff --git a/gremlins/pipelines/gh-terse.yaml b/gremlins/pipelines/gh-terse.yaml index 38e86ffb..580ac979 100644 --- a/gremlins/pipelines/gh-terse.yaml +++ b/gremlins/pipelines/gh-terse.yaml @@ -12,6 +12,7 @@ inputs: sources: issue: type: string + optional: true plan: type: [filepath, string] optional: true From 3c49229f3ffbd3f5ec6e8c1594d2e81703634fd5 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:19:00 -0600 Subject: [PATCH 09/23] Fix failing checks --- gremlins/pipeline/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/gremlins/pipeline/__init__.py b/gremlins/pipeline/__init__.py index bb874fbf..c7d7bac0 100644 --- a/gremlins/pipeline/__init__.py +++ b/gremlins/pipeline/__init__.py @@ -77,6 +77,7 @@ 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 [])) From 84cd435acd6daa1d8728057eb5e710ee62d0aedf Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:24:12 -0600 Subject: [PATCH 10/23] Fix CI failures --- gremlins/launcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gremlins/launcher.py b/gremlins/launcher.py index e90aafaa..ff2eed81 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -290,7 +290,7 @@ def _resolve_inputs( loaded_pipeline = _PipelineData.from_yaml( resolve_pipeline_path(pipeline_path, pathlib.Path(project_root)) ) - except (FileNotFoundError, OSError): + except (FileNotFoundError, OSError, ValueError): pass if ( From e06cbf7b4ee36c2b51ceaaaca5e855cd3eaa9f72 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:57:11 -0600 Subject: [PATCH 11/23] wip --- gremlins/launcher.py | 92 +++--- gremlins/pipelines/gh-terse.yaml | 3 - tests/test_launcher_input_sources.py | 404 ++++++--------------------- 3 files changed, 122 insertions(+), 377 deletions(-) diff --git a/gremlins/launcher.py b/gremlins/launcher.py index ff2eed81..c53ae02b 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -480,63 +480,32 @@ def _spawn(gremlin_id: str, inputs: _Inputs, state_dir: pathlib.Path) -> Any: def _seed_registry_from_sources( registry: ArtifactRegistry, - loaded_pipeline: _PipelineData | None, - inputs: _Inputs, + input_values: dict[str, str], + sources: dict[str, Any], artifacts_dir: pathlib.Path, ) -> None: - """Pre-seed registry with external sources based on pipeline.input_sources.""" - # Legacy fallback: if no input_sources defined, register plan_arg if plan is provided - if loaded_pipeline is None or loaded_pipeline.input_sources is None: - if inputs.plan: - uri = Uri.parse("file://session/plan-arg.txt") - registry.bind("plan_arg", uri) - return - - sources = loaded_pipeline.input_sources.sources - - # Always seed plan_arg if plan is provided, even when input_sources is present - if inputs.plan: - uri = Uri.parse("file://session/plan-arg.txt") - registry.bind("plan_arg", uri) - for key, source in sources.items(): - value: str | None = None - resolved_type: str | None = None - - if key == "plan": - if inputs.plan: - if "filepath" in source.types and os.path.isfile(inputs.plan): - value = inputs.plan - resolved_type = "filepath" - elif "string" in source.types: - value = inputs.plan - resolved_type = "string" - elif key == "issue": - if inputs.plan and "string" in source.types: - if not os.path.isfile(inputs.plan): - value = inputs.plan - resolved_type = "string" - elif key == "instructions": - if inputs.instructions and "string" in source.types: - value = inputs.instructions - resolved_type = "string" - - if value is None and not source.optional: - raise ValueError( - f"required input source {key!r} (type: {source.types}) is not available" - ) - - if value is None: + 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 - - if resolved_type == "filepath": - uri = Uri.parse(f"file://{value}") - registry.bind(key, uri) - elif resolved_type == "string": - source_file = artifacts_dir / f"{key}.txt" - source_file.write_text(value, encoding="utf-8") - uri = Uri.parse(f"file://session/{key}.txt") - registry.bind(key, uri) + for t in source.types: + if t == "filepath" and os.path.isfile(value): + registry.bind(key, Uri.parse(f"file://{value}")) + 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( @@ -593,9 +562,20 @@ def launch( 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")) - _seed_registry_from_sources( - registry, inputs.loaded_pipeline, inputs, artifact_dir - ) + if inputs.plan: + 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: dict[str, str] = {} + if inputs.plan: + input_values["plan"] = inputs.plan + if inputs.instructions: + input_values["instructions"] = inputs.instructions + _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) diff --git a/gremlins/pipelines/gh-terse.yaml b/gremlins/pipelines/gh-terse.yaml index 580ac979..3d7bb5e2 100644 --- a/gremlins/pipelines/gh-terse.yaml +++ b/gremlins/pipelines/gh-terse.yaml @@ -10,9 +10,6 @@ inputs: PLAN: plan? PR: pr? sources: - issue: - type: string - optional: true plan: type: [filepath, string] optional: true diff --git a/tests/test_launcher_input_sources.py b/tests/test_launcher_input_sources.py index 32d3d317..a83749fd 100644 --- a/tests/test_launcher_input_sources.py +++ b/tests/test_launcher_input_sources.py @@ -6,348 +6,116 @@ import pytest from gremlins.artifacts.registry import ArtifactRegistry -from gremlins.launcher import _Inputs, _seed_registry_from_sources -from gremlins.pipeline import Pipeline +from gremlins.launcher import _seed_registry_from_sources +from gremlins.pipeline.inputs import InputSource, InputSources + + +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 _write_pipeline( - self, tmp_path: pathlib.Path, yaml_content: str - ) -> pathlib.Path: - p = tmp_path / "pipeline.yaml" - p.write_text(textwrap.dedent(yaml_content), encoding="utf-8") - return p - - def _make_inputs( - self, - plan: str | None = None, - instructions: str = "", - project_root: str = "", - ) -> _Inputs: - return _Inputs( - gremlin_id="test-id", - kind="local", - plan=plan, - instructions=instructions, - description="test", - description_explicit=False, - parent_id="", - project_root=project_root or str(pathlib.Path.cwd()), - pipeline_path="pipeline.yaml", - pipeline_args=[], - client_label="claude:sonnet", - fetch_worktree=False, - base_ref_name="", - base_ref_sha="", - stage_inputs={}, - ) + 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)) - def test_legacy_fallback_when_no_input_sources( - self, tmp_path: pathlib.Path - ) -> None: - """When pipeline has no input_sources, fall back to hardcoded plan_arg.""" - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - registry = ArtifactRegistry(artifact_dir=artifact_dir) - - pipeline_path = self._write_pipeline( - tmp_path, - """\ - default_client: claude:sonnet - stages: - - { name: plan, type: agent } - """, + _seed_registry_from_sources( + registry, {"instructions": "do the thing"}, sources, artifact_dir ) - pipeline = Pipeline.from_yaml(pipeline_path) - inputs = self._make_inputs(plan="test plan") - - _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) - - # Should have registered plan_arg in legacy mode - assert "plan_arg" in registry.data - assert registry.data["plan_arg"] == "file://session/plan-arg.txt" - - def test_legacy_fallback_when_loaded_pipeline_none( - self, tmp_path: pathlib.Path - ) -> None: - """When loaded_pipeline is None, use legacy hardcoded seeding.""" - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - registry = ArtifactRegistry(artifact_dir=artifact_dir) - - inputs = self._make_inputs(plan="test plan") - - _seed_registry_from_sources(registry, None, inputs, artifact_dir) - - # Should have registered plan_arg in legacy mode - assert "plan_arg" in registry.data - assert registry.data["plan_arg"] == "file://session/plan-arg.txt" - - def test_string_source_as_issue(self, tmp_path: pathlib.Path) -> None: - """String source 'issue' registers issue reference.""" - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - registry = ArtifactRegistry(artifact_dir=artifact_dir) - - pipeline_path = self._write_pipeline( - tmp_path, - """\ - default_client: claude:sonnet - inputs: - sources: - issue: - type: string - stages: - - { name: plan, type: agent } - """, - ) - pipeline = Pipeline.from_yaml(pipeline_path) - inputs = self._make_inputs(plan="#123") - - _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) - - # Issue should be registered as a string - assert "issue" in registry.data - assert registry.data["issue"] == "file://session/issue.txt" - issue_file = artifact_dir / "issue.txt" - assert issue_file.read_text(encoding="utf-8") == "#123" - - def test_string_source_as_plan(self, tmp_path: pathlib.Path) -> None: - """String source 'plan' registers plan as a string.""" - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - registry = ArtifactRegistry(artifact_dir=artifact_dir) - - pipeline_path = self._write_pipeline( - tmp_path, - """\ - default_client: claude:sonnet - inputs: - sources: - plan: - type: string - optional: true - stages: - - { name: plan, type: agent } - """, - ) - pipeline = Pipeline.from_yaml(pipeline_path) - inputs = self._make_inputs(plan="some plan text") - _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + assert registry.data["instructions"] == "file://session/instructions.txt" + assert (artifact_dir / "instructions.txt").read_text() == "do the thing" - # Plan should be registered as a string - assert "plan" in registry.data - assert registry.data["plan"] == "file://session/plan.txt" - plan_file = artifact_dir / "plan.txt" - assert plan_file.read_text(encoding="utf-8") == "some plan text" - - def test_string_source_as_instructions(self, tmp_path: pathlib.Path) -> None: - """String source 'instructions' registers instructions.""" - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - registry = ArtifactRegistry(artifact_dir=artifact_dir) - - pipeline_path = self._write_pipeline( - tmp_path, - """\ - default_client: claude:sonnet - inputs: - sources: - instructions: - type: string - optional: true - stages: - - { name: plan, type: agent } - """, + def test_filepath_source_binds_uri(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 ) - pipeline = Pipeline.from_yaml(pipeline_path) - inputs = self._make_inputs(instructions="some instructions") - _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + assert registry.data["plan"] == f"file://{plan_file}" - # Instructions should be registered - assert "instructions" in registry.data - assert registry.data["instructions"] == "file://session/instructions.txt" - instr_file = artifact_dir / "instructions.txt" - assert instr_file.read_text(encoding="utf-8") == "some instructions" - - def test_optional_source_missing(self, tmp_path: pathlib.Path) -> None: - """Optional source absent → key not in registry.""" - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - registry = ArtifactRegistry(artifact_dir=artifact_dir) - - pipeline_path = self._write_pipeline( - tmp_path, - """\ - default_client: claude:sonnet - inputs: - sources: - plan: - type: string - optional: true - stages: - - { name: plan, type: agent } - """, - ) - pipeline = Pipeline.from_yaml(pipeline_path) - # No plan provided - inputs = self._make_inputs() - - _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) - - # Plan should not be in registry since it's optional and not provided - assert "plan" not in registry.data - - def test_required_source_missing_raises(self, tmp_path: pathlib.Path) -> None: - """Required source missing → error at launch time.""" - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - registry = ArtifactRegistry(artifact_dir=artifact_dir) - - pipeline_path = self._write_pipeline( - tmp_path, - """\ - default_client: claude:sonnet - inputs: - sources: - issue: - type: string - stages: - - { name: plan, type: agent } - """, + def test_union_type_filepath_wins_when_file_exists(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", "string"], True)) + + _seed_registry_from_sources( + registry, {"plan": str(plan_file)}, sources, artifact_dir ) - pipeline = Pipeline.from_yaml(pipeline_path) - # No plan provided (issue requires it) - inputs = self._make_inputs() - with pytest.raises(ValueError, match="required input source"): - _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + assert registry.data["plan"] == f"file://{plan_file}" + assert not (artifact_dir / "plan.txt").exists() - def test_union_type_filepath_first(self, tmp_path: pathlib.Path) -> None: - """Union source given as resolvable file path → resolves as filepath.""" - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - registry = ArtifactRegistry(artifact_dir=artifact_dir) + 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)) - # Create a test file - plan_file = tmp_path / "plan.md" - plan_file.write_text("# My Plan\nSome content", encoding="utf-8") - - pipeline_path = self._write_pipeline( - tmp_path, - """\ - default_client: claude:sonnet - inputs: - sources: - plan: - type: [filepath, string] - optional: true - stages: - - { name: plan, type: agent } - """, + _seed_registry_from_sources( + registry, {"plan": "#123"}, sources, artifact_dir ) - pipeline = Pipeline.from_yaml(pipeline_path) - inputs = self._make_inputs(plan=str(plan_file)) - _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + assert registry.data["plan"] == "file://session/plan.txt" + assert (artifact_dir / "plan.txt").read_text() == "#123" - # Plan should be registered as a filepath URI - assert "plan" in registry.data - assert registry.data["plan"].startswith("file://") - assert str(plan_file) in registry.data["plan"] + def test_optional_source_absent_skipped(self, tmp_path: pathlib.Path) -> None: + registry, artifact_dir = _make_registry(tmp_path) + sources = _sources(("instructions", ["string"], True)) - def test_union_type_falls_back_to_string(self, tmp_path: pathlib.Path) -> None: - """Union source given as plain string → registered as string.""" - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - registry = ArtifactRegistry(artifact_dir=artifact_dir) - - pipeline_path = self._write_pipeline( - tmp_path, - """\ - default_client: claude:sonnet - inputs: - sources: - plan: - type: [filepath, string] - optional: true - stages: - - { name: plan, type: agent } - """, - ) - pipeline = Pipeline.from_yaml(pipeline_path) - # Provide a string that's not a file path (issue ref) - inputs = self._make_inputs(plan="#123") + _seed_registry_from_sources(registry, {}, sources, artifact_dir) - _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + assert "instructions" not in registry.data - # Plan should be registered as a string (since file doesn't exist) - assert "plan" in registry.data - assert registry.data["plan"] == "file://session/plan.txt" - plan_file = artifact_dir / "plan.txt" - assert plan_file.read_text(encoding="utf-8") == "#123" - - def test_filepath_only_source_missing_file_raises( - self, tmp_path: pathlib.Path - ) -> None: - """Filepath-only source with unresolvable path → error.""" - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - registry = ArtifactRegistry(artifact_dir=artifact_dir) - - pipeline_path = self._write_pipeline( - tmp_path, - """\ - default_client: claude:sonnet - inputs: - sources: - plan: - type: filepath - stages: - - { name: plan, type: agent } - """, - ) - pipeline = Pipeline.from_yaml(pipeline_path) - # Provide a path that doesn't exist - inputs = self._make_inputs(plan="/nonexistent/path.md") + 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, pipeline, inputs, artifact_dir) - - def test_multiple_sources_partially_satisfied(self, tmp_path: pathlib.Path) -> None: - """Multiple sources with mixed required/optional - only instructions absent.""" - artifact_dir = tmp_path / "artifacts" - artifact_dir.mkdir() - registry = ArtifactRegistry(artifact_dir=artifact_dir) - - pipeline_path = self._write_pipeline( - tmp_path, - """\ - default_client: claude:sonnet - inputs: - sources: - issue: - type: string - plan: - type: string - optional: true - instructions: - type: string - optional: true - stages: - - { name: plan, type: agent } - """, + _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_multiple_sources_mixed_presence(self, tmp_path: pathlib.Path) -> None: + registry, artifact_dir = _make_registry(tmp_path) + sources = _sources( + ("plan", ["string"], False), + ("instructions", ["string"], True), ) - pipeline = Pipeline.from_yaml(pipeline_path) - # Provide plan (which satisfies both issue and plan) - inputs = self._make_inputs(plan="#456") - _seed_registry_from_sources(registry, pipeline, inputs, artifact_dir) + _seed_registry_from_sources( + registry, {"plan": "#456"}, sources, artifact_dir + ) - # Issue and plan should both be registered since plan satisfies both - # Instructions should not be registered (optional and not provided) - assert "issue" in registry.data assert "plan" in registry.data assert "instructions" not in registry.data + + 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 From a8323689842e32a8d1a5a0e16c7e2b1da464b194 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 13:09:33 -0600 Subject: [PATCH 12/23] wip --- gremlins/launcher.py | 112 ++++++--------------------------- tests/test_launcher.py | 128 ++------------------------------------ tests/test_recipe_plan.py | 46 -------------- 3 files changed, 25 insertions(+), 261 deletions(-) delete mode 100644 tests/test_recipe_plan.py diff --git a/gremlins/launcher.py b/gremlins/launcher.py index c53ae02b..5c2abd31 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -1,7 +1,7 @@ """Launcher for background gremlins. Public API: - launch(kind, *, stage_inputs=None, plan=None, description=None, + launch(kind, *, stage_inputs=None, description=None, parent_id=None, project_root=None, base_ref="HEAD", pipeline_args=()) -> tuple[str, subprocess.Popen[bytes]] resume(gremlin_id, *, graft=None) -> None @@ -34,7 +34,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): @@ -51,39 +51,13 @@ 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 @@ -109,7 +83,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 @@ -126,39 +99,14 @@ class _Inputs: 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}") - +def _validate_spec(spec_path: str | None) -> str | None: 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 + return str(pathlib.Path(spec_path).resolve()) + return None def _reject_pipeline_collision(gremlin_id: str) -> None: @@ -248,7 +196,6 @@ def _parse_pr_num(pr_arg: str) -> str: def _resolve_inputs( kind: str, stage_inputs: dict[str, Any], - plan: str | None, description: str | None, parent_id: str | None, project_root: str | None, @@ -261,16 +208,9 @@ def _resolve_inputs( 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) + spec_path = _validate_spec(spec_path) - 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(instructions, description) if project_root is None: r = proc.run(["git", "rev-parse", "--show-toplevel"]) @@ -318,15 +258,12 @@ def _resolve_inputs( 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, @@ -347,9 +284,7 @@ def _resolve_inputs( def _prepare_state_dir(state_dir: pathlib.Path, inputs: _Inputs) -> 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: @@ -512,7 +447,6 @@ 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, @@ -533,7 +467,6 @@ def launch( inputs = _resolve_inputs( kind, {} if stage_inputs is None else dict(stage_inputs), - plan, description, parent_id, project_root, @@ -562,14 +495,10 @@ def launch( 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")) - if inputs.plan: - 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: dict[str, str] = {} - if inputs.plan: - input_values["plan"] = inputs.plan - if inputs.instructions: - input_values["instructions"] = inputs.instructions + 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, @@ -670,17 +599,14 @@ def _spawn_resume( 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) + 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) diff --git a/tests/test_launcher.py b/tests/test_launcher.py index 60defbd3..bb744f07 100644 --- a/tests/test_launcher.py +++ b/tests/test_launcher.py @@ -260,32 +260,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 +267,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() @@ -515,7 +462,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 +643,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 # --------------------------------------------------------------------------- @@ -898,14 +831,14 @@ def test_setup_workdir_non_git_raises(tmp_path): 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) + "local", + stage_inputs={"instructions": "test spec threading"}, + spec_path=str(spec_file), ) state = _read_state(_gremlins_state_root(lenv) / gremlin_id) assert "--spec" in state["pipeline_args"] @@ -915,11 +848,9 @@ def test_launch_threads_spec_path_into_pipeline_args(lenv): 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") + launcher.launch("local", spec_path="/nonexistent/spec.md") dirs = ( list(_gremlins_state_root(lenv).glob("*")) if _gremlins_state_root(lenv).exists() @@ -930,13 +861,11 @@ def test_launch_rejects_missing_spec_path(lenv): 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)) + launcher.launch("local", spec_path=str(spec_file)) # --------------------------------------------------------------------------- @@ -1000,51 +929,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 # --------------------------------------------------------------------------- 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() == "" From a2b4c5f57b3edff62d67d1e8b547a6af2a166678 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 13:13:05 -0600 Subject: [PATCH 13/23] wip --- gremlins/launcher.py | 30 +++--------------------------- tests/test_launcher.py | 10 ---------- 2 files changed, 3 insertions(+), 37 deletions(-) diff --git a/gremlins/launcher.py b/gremlins/launcher.py index 5c2abd31..40ffb176 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -50,18 +50,12 @@ def _state_root() -> pathlib.Path: def _resolve_description_and_slug( - instructions: str | None, description: str | None, ) -> tuple[str, bool, str]: """Return (description, description_explicit, slug) from available inputs.""" if description: slug = slugify(description) or "gremlin" return description[:60], True, slug - - if instructions: - slug = slugify(instructions[:80]) or "gremlin" - return instructions[:60], False, slug - return "", False, "gremlin" @@ -83,7 +77,6 @@ def _build_spawn_env(gremlin_id: str) -> dict[str, str]: class _Inputs: gremlin_id: str kind: str - instructions: str description: str description_explicit: bool parent_id: str @@ -207,10 +200,9 @@ def _resolve_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") spec_path = _validate_spec(spec_path) - desc, desc_explicit, slug = _resolve_description_and_slug(instructions, description) + desc, desc_explicit, slug = _resolve_description_and_slug(description) if project_root is None: r = proc.run(["git", "rev-parse", "--show-toplevel"]) @@ -264,7 +256,6 @@ def _resolve_inputs( return _Inputs( gremlin_id=resolved_gremlin_id, kind=kind, - instructions=instructions or "", description=desc, description_explicit=desc_explicit, parent_id=parent_id or "", @@ -281,9 +272,8 @@ def _resolve_inputs( ) -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") (state_dir / "artifacts").mkdir(exist_ok=True) @@ -298,7 +288,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, @@ -398,8 +387,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", @@ -477,7 +464,7 @@ def launch( ) 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 ) @@ -593,20 +580,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: spawn_args: list[str] = list(pipeline_args) - 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) @@ -659,9 +638,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/tests/test_launcher.py b/tests/test_launcher.py index bb744f07..d03fe40c 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"] @@ -365,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( { @@ -419,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) @@ -444,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) @@ -892,7 +883,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( From 1b6774e9ec7cb0358c2883ad26199ef33e3050c2 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 13:16:35 -0600 Subject: [PATCH 14/23] wip --- gremlins/launcher.py | 21 +++----------------- tests/test_launcher.py | 44 ------------------------------------------ 2 files changed, 3 insertions(+), 62 deletions(-) diff --git a/gremlins/launcher.py b/gremlins/launcher.py index 40ffb176..5657ffdc 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -1,9 +1,9 @@ """Launcher for background gremlins. Public API: - launch(kind, *, stage_inputs=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 """ @@ -92,14 +92,6 @@ class _Inputs: pr_num: str = "" -def _validate_spec(spec_path: str | None) -> str | None: - 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}") - return str(pathlib.Path(spec_path).resolve()) - return None def _reject_pipeline_collision(gremlin_id: str) -> None: @@ -194,13 +186,11 @@ def _resolve_inputs( 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 - spec_path = _validate_spec(spec_path) desc, desc_explicit, slug = _resolve_description_and_slug(description) @@ -248,8 +238,6 @@ def _resolve_inputs( pr_num = "" stored_args = list(resolved_pipeline_args) - if spec_path and "--spec" not in stored_args: - stored_args = ["--spec", spec_path] + stored_args client_label = launch_client_label(stored_args, loaded_pipeline) @@ -439,7 +427,6 @@ def launch( 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 = "", @@ -459,7 +446,6 @@ def launch( project_root, base_ref, pipeline_args, - spec_path, gremlin_id, ) state_dir = _state_root() / inputs.gremlin_id @@ -481,7 +467,6 @@ def launch( 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")) 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 diff --git a/tests/test_launcher.py b/tests/test_launcher.py index d03fe40c..1e352006 100644 --- a/tests/test_launcher.py +++ b/tests/test_launcher.py @@ -815,50 +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.""" - spec_file = lenv.repo / "spec.md" - spec_file.write_text("the overall spec", encoding="utf-8") - - launcher = _launcher() - gremlin_id, _ = launcher.launch( - "local", - stage_inputs={"instructions": "test spec threading"}, - 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.""" - launcher = _launcher() - with pytest.raises(ValueError, match="--spec"): - launcher.launch("local", 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.""" - 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", spec_path=str(spec_file)) - - # --------------------------------------------------------------------------- # stage_inputs persistence # --------------------------------------------------------------------------- From 7f43874feb12f3ee62a56671b48250f9369cd1ab Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 13:20:50 -0600 Subject: [PATCH 15/23] wip --- gremlins/launcher.py | 17 ----------------- tests/test_launcher.py | 1 - 2 files changed, 18 deletions(-) diff --git a/gremlins/launcher.py b/gremlins/launcher.py index 5657ffdc..1f5c56c6 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -14,7 +14,6 @@ import json import os import pathlib -import re import secrets import shutil import subprocess @@ -89,7 +88,6 @@ class _Inputs: base_ref_sha: str stage_inputs: dict[str, Any] loaded_pipeline: _PipelineData | None = None - pr_num: str = "" @@ -167,16 +165,6 @@ 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, @@ -229,13 +217,11 @@ 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) @@ -256,7 +242,6 @@ def _resolve_inputs( base_ref_sha=base_ref_sha, stage_inputs=stage_inputs, loaded_pipeline=loaded_pipeline, - pr_num=pr_num, ) @@ -461,8 +446,6 @@ 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: diff --git a/tests/test_launcher.py b/tests/test_launcher.py index 1e352006..86b884f1 100644 --- a/tests/test_launcher.py +++ b/tests/test_launcher.py @@ -975,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" From 586cc28d0367a6651215573019777bc359a433ee Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 23:21:43 -0600 Subject: [PATCH 16/23] wip --- gremlins/executor/gremlin.py | 72 -------------- gremlins/executor/run.py | 26 +---- gremlins/executor/state.py | 11 --- gremlins/launcher.py | 8 +- gremlins/run_child.py | 1 - gremlins/spawn/child.py | 1 - gremlins/utils/proc.py | 1 - tests/test_gremlin_open.py | 4 - tests/test_launcher_input_sources.py | 15 ++- tests/test_orchestrator_gh.py | 137 +++++---------------------- tests/test_orchestrator_local.py | 118 +++++++---------------- tests/test_spawn_child.py | 5 +- tests/test_stage_base.py | 5 +- tests/test_state_artifacts.py | 9 -- tests/test_state_isolation.py | 4 +- 15 files changed, 76 insertions(+), 341 deletions(-) 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 1f5c56c6..44d870f6 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -90,8 +90,6 @@ class _Inputs: loaded_pipeline: _PipelineData | None = None - - def _reject_pipeline_collision(gremlin_id: str) -> None: pipeline_names = {name for name, _ in list_pipelines(_paths.project_root())} if gremlin_id in pipeline_names: @@ -165,7 +163,6 @@ def _resolve_base_ref( return effective_base_ref, "" - def _resolve_inputs( kind: str, stage_inputs: dict[str, Any], @@ -450,7 +447,10 @@ def launch( 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}")) - if inputs.loaded_pipeline is not None and inputs.loaded_pipeline.input_sources is not None: + 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 } 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..3fb4fc16 100644 --- a/gremlins/spawn/child.py +++ b/gremlins/spawn/child.py @@ -132,7 +132,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_launcher_input_sources.py b/tests/test_launcher_input_sources.py index a83749fd..9f232fbe 100644 --- a/tests/test_launcher_input_sources.py +++ b/tests/test_launcher_input_sources.py @@ -1,13 +1,12 @@ """Tests for launcher registry seeding from input sources.""" import pathlib -import textwrap import pytest from gremlins.artifacts.registry import ArtifactRegistry from gremlins.launcher import _seed_registry_from_sources -from gremlins.pipeline.inputs import InputSource, InputSources +from gremlins.pipeline.inputs import InputSource def _make_registry(tmp_path: pathlib.Path) -> tuple[ArtifactRegistry, pathlib.Path]: @@ -47,7 +46,9 @@ def test_filepath_source_binds_uri(self, tmp_path: pathlib.Path) -> None: assert registry.data["plan"] == f"file://{plan_file}" - def test_union_type_filepath_wins_when_file_exists(self, tmp_path: pathlib.Path) -> None: + def test_union_type_filepath_wins_when_file_exists( + 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") @@ -64,9 +65,7 @@ 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 - ) + _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" @@ -102,9 +101,7 @@ def test_multiple_sources_mixed_presence(self, tmp_path: pathlib.Path) -> None: ("instructions", ["string"], True), ) - _seed_registry_from_sources( - registry, {"plan": "#456"}, sources, artifact_dir - ) + _seed_registry_from_sources(registry, {"plan": "#456"}, sources, artifact_dir) assert "plan" in registry.data assert "instructions" not in registry.data diff --git a/tests/test_orchestrator_gh.py b/tests/test_orchestrator_gh.py index f06d2d8b..a48b7b86 100644 --- a/tests/test_orchestrator_gh.py +++ b/tests/test_orchestrator_gh.py @@ -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) @@ -649,7 +620,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 +659,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 +700,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 +746,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 +793,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 +846,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 +886,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 +898,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 +907,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 +1014,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 +1035,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 +1079,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 +1144,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 +1203,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 +1252,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 +1295,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 +1357,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 +1414,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 +1456,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 +1501,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 +1569,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 +1578,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..1744fae9 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": "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": "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": "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": "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": "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,57 +413,6 @@ 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" @@ -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_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, ) ) From c129102321b2c3ee4a7625f997bb5525aec555ae Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Tue, 2 Jun 2026 23:59:37 -0600 Subject: [PATCH 17/23] wip --- .gremlins/gh-terse.yaml | 7 +++++++ gremlins/recipes/stages/plan.yaml | 16 +++++++++------- gremlins/recipes/stages/plan_gh.yaml | 23 +++++++++++------------ 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/.gremlins/gh-terse.yaml b/.gremlins/gh-terse.yaml index 6a782764..5943ae03 100644 --- a/.gremlins/gh-terse.yaml +++ b/.gremlins/gh-terse.yaml @@ -8,6 +8,13 @@ inputs: INSTRUCTIONS: instructions? PLAN: plan? PR: pr? + sources: + plan: + type: [filepath, string] + optional: true + instructions: + type: string + optional: true land: in: diff --git a/gremlins/recipes/stages/plan.yaml b/gremlins/recipes/stages/plan.yaml index 930d9c24..ed63d0e1 100644 --- a/gremlins/recipes/stages/plan.yaml +++ b/gremlins/recipes/stages/plan.yaml @@ -4,19 +4,19 @@ 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: @@ -27,6 +27,8 @@ stages: skip_if_exists: plan prompt: - "{{prompt}}" + in: + instructions: "instructions?" out: plan: "file://session/plan.md" options: diff --git a/gremlins/recipes/stages/plan_gh.yaml b/gremlins/recipes/stages/plan_gh.yaml index 4ad62517..705e802d 100644 --- a/gremlins/recipes/stages/plan_gh.yaml +++ b/gremlins/recipes/stages/plan_gh.yaml @@ -4,29 +4,26 @@ 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: @@ -38,6 +35,8 @@ stages: skip_if_exists: plan prompt: - "{{prompt}}" + in: + instructions: "instructions?" out: plan: "file://session/plan.md" options: From 81c4aeb0182a1a0589ac12821edba18d780b9354 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Wed, 3 Jun 2026 00:02:47 -0600 Subject: [PATCH 18/23] wip --- .gremlins/gh-terse.yaml | 1 - gremlins/pipelines/gh-terse.yaml | 1 - gremlins/pipelines/gh.yaml | 1 - 3 files changed, 3 deletions(-) diff --git a/.gremlins/gh-terse.yaml b/.gremlins/gh-terse.yaml index 5943ae03..c19c2626 100644 --- a/.gremlins/gh-terse.yaml +++ b/.gremlins/gh-terse.yaml @@ -7,7 +7,6 @@ inputs: in: INSTRUCTIONS: instructions? PLAN: plan? - PR: pr? sources: plan: type: [filepath, string] diff --git a/gremlins/pipelines/gh-terse.yaml b/gremlins/pipelines/gh-terse.yaml index 3d7bb5e2..8b77acf0 100644 --- a/gremlins/pipelines/gh-terse.yaml +++ b/gremlins/pipelines/gh-terse.yaml @@ -8,7 +8,6 @@ inputs: in: INSTRUCTIONS: instructions? PLAN: plan? - PR: pr? sources: plan: type: [filepath, string] diff --git a/gremlins/pipelines/gh.yaml b/gremlins/pipelines/gh.yaml index 68a1fc32..4e0925a1 100644 --- a/gremlins/pipelines/gh.yaml +++ b/gremlins/pipelines/gh.yaml @@ -8,7 +8,6 @@ inputs: in: INSTRUCTIONS: instructions? PLAN: plan? - PR: pr? land: in: From f8c7c09b23e28b39b80471759000641df61413fc Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Wed, 3 Jun 2026 00:42:05 -0600 Subject: [PATCH 19/23] wip --- tests/test_orchestrator_gh.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_orchestrator_gh.py b/tests/test_orchestrator_gh.py index a48b7b86..f9af530c 100644 --- a/tests/test_orchestrator_gh.py +++ b/tests/test_orchestrator_gh.py @@ -583,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") From c3f84a840efad4a8ec71e7f4c0169555bef4390b Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Wed, 3 Jun 2026 09:46:02 -0600 Subject: [PATCH 20/23] wip --- gremlins/launcher.py | 6 +++++- tests/test_launcher_input_sources.py | 8 +++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/gremlins/launcher.py b/gremlins/launcher.py index 44d870f6..f76b12de 100644 --- a/gremlins/launcher.py +++ b/gremlins/launcher.py @@ -386,7 +386,11 @@ def _seed_registry_from_sources( continue for t in source.types: if t == "filepath" and os.path.isfile(value): - registry.bind(key, Uri.parse(f"file://{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" diff --git a/tests/test_launcher_input_sources.py b/tests/test_launcher_input_sources.py index 9f232fbe..655753d0 100644 --- a/tests/test_launcher_input_sources.py +++ b/tests/test_launcher_input_sources.py @@ -34,7 +34,7 @@ def test_string_source_writes_file_and_binds(self, tmp_path: pathlib.Path) -> No assert registry.data["instructions"] == "file://session/instructions.txt" assert (artifact_dir / "instructions.txt").read_text() == "do the thing" - def test_filepath_source_binds_uri(self, tmp_path: pathlib.Path) -> None: + 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") @@ -44,7 +44,8 @@ def test_filepath_source_binds_uri(self, tmp_path: pathlib.Path) -> None: registry, {"plan": str(plan_file)}, sources, artifact_dir ) - assert registry.data["plan"] == f"file://{plan_file}" + assert registry.data["plan"] == "file://session/plan.md" + assert (artifact_dir / "plan.md").read_text() == "# Plan" def test_union_type_filepath_wins_when_file_exists( self, tmp_path: pathlib.Path @@ -58,7 +59,8 @@ def test_union_type_filepath_wins_when_file_exists( registry, {"plan": str(plan_file)}, sources, artifact_dir ) - assert registry.data["plan"] == f"file://{plan_file}" + assert registry.data["plan"] == "file://session/plan.md" + assert (artifact_dir / "plan.md").read_text() == "# Plan" assert not (artifact_dir / "plan.txt").exists() def test_union_type_falls_back_to_string(self, tmp_path: pathlib.Path) -> None: From 4a35cb23d26bd24ee855a8d44181eb15b2a763bb Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Wed, 3 Jun 2026 10:00:31 -0600 Subject: [PATCH 21/23] rename plan artifact key to plan-document; fix filepath input source seeding - plan-document is the resolved markdown artifact; plan remains the raw input source binding (--plan CLI arg), eliminating the DuplicateArtifact collision that prevented recipes from rebinding the resolved content - filepath input sources now copy to session artifacts instead of binding an external file:// URI, so resolve-plan-input's early-exit check fires correctly and the plan-document binding succeeds Co-Authored-By: Claude Sonnet 4.6 --- gremlins/pipelines/gh-terse.yaml | 2 +- gremlins/pipelines/gh.yaml | 2 +- gremlins/pipelines/local.yaml | 2 +- gremlins/recipes/stages/github_open_pr.yaml | 2 +- gremlins/recipes/stages/implement.yaml | 2 +- gremlins/recipes/stages/plan.yaml | 6 +++--- gremlins/recipes/stages/plan_gh.yaml | 8 ++++---- gremlins/recipes/stages/review_code.yaml | 2 +- tests/test_orchestrator_gh.py | 4 ++-- tests/test_orchestrator_local.py | 12 ++++++------ 10 files changed, 21 insertions(+), 21 deletions(-) diff --git a/gremlins/pipelines/gh-terse.yaml b/gremlins/pipelines/gh-terse.yaml index 8b77acf0..76acfb25 100644 --- a/gremlins/pipelines/gh-terse.yaml +++ b/gremlins/pipelines/gh-terse.yaml @@ -7,7 +7,7 @@ github_integration: true inputs: in: INSTRUCTIONS: instructions? - PLAN: plan? + PLAN: plan-document? sources: plan: type: [filepath, string] diff --git a/gremlins/pipelines/gh.yaml b/gremlins/pipelines/gh.yaml index 4e0925a1..bdf93632 100644 --- a/gremlins/pipelines/gh.yaml +++ b/gremlins/pipelines/gh.yaml @@ -7,7 +7,7 @@ github_integration: true inputs: in: INSTRUCTIONS: instructions? - PLAN: plan? + 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 ed63d0e1..acccae8f 100644 --- a/gremlins/recipes/stages/plan.yaml +++ b/gremlins/recipes/stages/plan.yaml @@ -20,17 +20,17 @@ stages: 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 705e802d..8fb8cd4b 100644 --- a/gremlins/recipes/stages/plan_gh.yaml +++ b/gremlins/recipes/stages/plan_gh.yaml @@ -28,17 +28,17 @@ stages: 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 @@ -61,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/tests/test_orchestrator_gh.py b/tests/test_orchestrator_gh.py index f9af530c..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)) diff --git a/tests/test_orchestrator_local.py b/tests/test_orchestrator_local.py index 1744fae9..e754bb9d 100644 --- a/tests/test_orchestrator_local.py +++ b/tests/test_orchestrator_local.py @@ -31,7 +31,7 @@ def test_local_main_plan_mode(tmp_path, monkeypatch): # 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": "file://session/plan.md"}) + json.dumps({"plan-document": "file://session/plan.md"}) ) monkeypatch.chdir(tmp_path) @@ -71,7 +71,7 @@ def test_local_main_resume_from_review_code_requires_git_changes( artifact_dir.mkdir() (artifact_dir / "plan.md").write_text("# Plan\nDo stuff.\n") (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) @@ -106,7 +106,7 @@ def test_local_main_resume_from_review_code_allows_existing_git_changes( artifact_dir.mkdir() (artifact_dir / "plan.md").write_text("# Plan\nDo stuff.\n") (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) @@ -148,7 +148,7 @@ def test_local_main_client_specifier_model(tmp_path, monkeypatch): # 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": "file://session/plan.md"}) + json.dumps({"plan-document": "file://session/plan.md"}) ) monkeypatch.chdir(tmp_path) @@ -359,7 +359,7 @@ def test_local_main_pipeline_default_client_model(tmp_path, monkeypatch): # 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": "file://session/plan.md"}) + json.dumps({"plan-document": "file://session/plan.md"}) ) monkeypatch.chdir(tmp_path) @@ -419,7 +419,7 @@ def test_plan_skip_if_exists_on_resume(tmp_path, monkeypatch): 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) From 9b89c3852a89296f4fb856db0b92131b91c38e37 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Wed, 3 Jun 2026 10:05:55 -0600 Subject: [PATCH 22/23] remove stale instructions field from child spec schema docstring Co-Authored-By: Claude Sonnet 4.6 --- gremlins/spawn/child.py | 1 - 1 file changed, 1 deletion(-) diff --git a/gremlins/spawn/child.py b/gremlins/spawn/child.py index 3fb4fc16..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 → "" } From 9e5939ff3fbae156c7ff32cd5a786c8965009127 Mon Sep 17 00:00:00 2001 From: Brian Hannafious <32105697+xbrianh@users.noreply.github.com> Date: Wed, 3 Jun 2026 10:10:02 -0600 Subject: [PATCH 23/23] trim input source tests: drop trivial dataclass assertions and redundant cases Co-Authored-By: Claude Sonnet 4.6 --- tests/test_input_sources.py | 138 +++++---------------------- tests/test_launcher_input_sources.py | 28 ------ 2 files changed, 26 insertions(+), 140 deletions(-) diff --git a/tests/test_input_sources.py b/tests/test_input_sources.py index 4546d8da..339ea09b 100644 --- a/tests/test_input_sources.py +++ b/tests/test_input_sources.py @@ -9,78 +9,20 @@ from gremlins.pipeline.inputs import InputSource, InputSources -class TestInputSource: - def test_single_type(self) -> None: - source = InputSource(name="issue", types=["string"]) - assert source.name == "issue" - assert source.types == ["string"] - assert source.optional is False - - def test_union_type(self) -> None: - source = InputSource(name="plan", types=["filepath", "string"]) - assert source.types == ["filepath", "string"] - - def test_optional(self) -> None: - source = InputSource(name="plan", types=["string"], optional=True) - assert source.optional is True - - def test_unknown_type_rejected(self) -> None: - with pytest.raises(ValueError, match="unknown type"): - InputSource(name="bad", types=["unknown"]) - - def test_empty_types_rejected(self) -> None: - with pytest.raises(ValueError, match="types list must not be empty"): - InputSource(name="bad", types=[]) - - class TestInputSources: - def test_parse_simple_string_source(self) -> None: + def test_parse_sources(self) -> None: raw = { - "issue": { - "type": "string", - } - } - sources = InputSources.from_yaml(raw) - issue = sources.get("issue") - assert issue is not None - assert issue.name == "issue" - assert issue.types == ["string"] - assert issue.optional is False - - def test_parse_union_type_source(self) -> None: - raw = { - "plan": { - "type": ["filepath", "string"], - } + "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"] - - def test_parse_optional_source(self) -> None: - raw = { - "instructions": { - "type": "string", - "optional": True, - } - } - sources = InputSources.from_yaml(raw) + assert plan.optional is True instr = sources.get("instructions") assert instr is not None - assert instr.optional is True - - def test_parse_multiple_sources(self) -> None: - raw = { - "issue": {"type": "string"}, - "plan": {"type": ["filepath", "string"], "optional": True}, - "instructions": {"type": "string", "optional": True}, - } - sources = InputSources.from_yaml(raw) - assert len(sources.all_sources()) == 3 - assert sources.get("issue") is not None - assert sources.get("plan") is not None - assert sources.get("instructions") is not None + assert instr.optional is False def test_required_sources(self) -> None: raw = { @@ -93,45 +35,32 @@ def test_required_sources(self) -> None: assert "plan" not in required def test_missing_type_field_rejected(self) -> None: - raw = { - "issue": {}, - } with pytest.raises(ValueError, match="missing required 'type' field"): - InputSources.from_yaml(raw) + InputSources.from_yaml({"issue": {}}) - def test_invalid_type_value_rejected(self) -> None: - raw = { - "issue": { - "type": "invalid-type", - } - } + def test_invalid_type_rejected(self) -> None: with pytest.raises(ValueError, match="unknown type"): - InputSources.from_yaml(raw) + InputSources.from_yaml({"issue": {"type": "bad"}}) - def test_type_list_with_non_string_rejected(self) -> None: - raw = { - "issue": { - "type": ["string", 123], - } - } + 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(raw) + InputSources.from_yaml({"issue": {"type": ["string", 123]}}) def test_empty_type_list_rejected(self) -> None: - raw = { - "issue": { - "type": [], - } - } with pytest.raises(ValueError, match="type list must not be empty"): - InputSources.from_yaml(raw) + InputSources.from_yaml({"issue": {"type": []}}) def test_non_mapping_source_rejected(self) -> None: - raw = { - "issue": ["string"], - } with pytest.raises(ValueError, match="expected a mapping"): - InputSources.from_yaml(raw) + 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: @@ -147,24 +76,22 @@ def test_parse_pipeline_with_input_sources(self, tmp_path: pathlib.Path) -> None default_client: claude:sonnet inputs: in: - PLAN: plan? + PLAN: plan-document? sources: - issue: - type: string 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 - assert pipeline.input_sources.get("issue") is not None - assert pipeline.input_sources.get("plan") is not None - issue = pipeline.input_sources.get("issue") - assert issue.types == ["string"] plan = pipeline.input_sources.get("plan") + assert plan is not None assert plan.types == ["filepath", "string"] assert plan.optional is True @@ -175,25 +102,12 @@ def test_parse_pipeline_without_input_sources(self, tmp_path: pathlib.Path) -> N default_client: claude:sonnet inputs: in: - PLAN: plan? - stages: - - { name: plan, type: agent } - """, - ) - pipeline = Pipeline.from_yaml(p) - assert pipeline.input_sources is None - - def test_parse_pipeline_without_inputs(self, tmp_path: pathlib.Path) -> None: - p = self._write_pipeline( - tmp_path, - """\ - default_client: claude:sonnet + PLAN: plan-document? stages: - { name: plan, type: agent } """, ) pipeline = Pipeline.from_yaml(p) - assert pipeline.inputs is None assert pipeline.input_sources is None def test_invalid_sources_block_rejected(self, tmp_path: pathlib.Path) -> None: diff --git a/tests/test_launcher_input_sources.py b/tests/test_launcher_input_sources.py index 655753d0..4b1f2845 100644 --- a/tests/test_launcher_input_sources.py +++ b/tests/test_launcher_input_sources.py @@ -47,22 +47,6 @@ def test_filepath_source_copies_to_session(self, tmp_path: pathlib.Path) -> None assert registry.data["plan"] == "file://session/plan.md" assert (artifact_dir / "plan.md").read_text() == "# Plan" - def test_union_type_filepath_wins_when_file_exists( - 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", "string"], True)) - - _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" - assert not (artifact_dir / "plan.txt").exists() - 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)) @@ -96,18 +80,6 @@ def test_filepath_only_no_file_raises(self, tmp_path: pathlib.Path) -> None: registry, {"plan": "/nonexistent/plan.md"}, sources, artifact_dir ) - def test_multiple_sources_mixed_presence(self, tmp_path: pathlib.Path) -> None: - registry, artifact_dir = _make_registry(tmp_path) - sources = _sources( - ("plan", ["string"], False), - ("instructions", ["string"], True), - ) - - _seed_registry_from_sources(registry, {"plan": "#456"}, sources, artifact_dir) - - assert "plan" in registry.data - assert "instructions" not in registry.data - 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))