Skip to content

Commit cd76942

Browse files
MarkusCopilot
andcommitted
fix(workflows): address Copilot review feedback on integration auto-detect
Issue 1 (engine.py:726): Explicit --input integration=auto was rejected by enum validation before the sentinel could be resolved. Move the auto-resolution to before _coerce_input() so enum-constrained inputs also accept 'auto'. Issue 2 (engine.py:737): workflow.integration: auto in the YAML was stored raw as 'auto' in WorkflowDefinition.default_integration and leaked into StepContext, causing step dispatch to look for an 'auto' CLI. Add _resolve_workflow_integration() helper and use it at both StepContext construction sites. Issue 3 (engine.py:779): _load_project_integration() only queried the legacy 'integration' field. Newer state files may carry 'default_integration' as the canonical key. Replace the custom inner reader for integration.json with a call to the shared default_integration_key() from integration_state, which handles both fields and their precedence correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d05a622 commit cd76942

2 files changed

Lines changed: 112 additions & 18 deletions

File tree

src/specify_cli/workflows/engine.py

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@
2020
import yaml
2121

2222
from .base import RunStatus, StepContext, StepResult, StepStatus
23-
from specify_cli.integration_state import INTEGRATION_JSON as _INTEGRATION_JSON
23+
from specify_cli.integration_state import (
24+
INTEGRATION_JSON as _INTEGRATION_JSON,
25+
default_integration_key as _default_integration_key,
26+
)
2427
from specify_cli.paths import INIT_OPTIONS_FILE as _INIT_OPTIONS_FILE
2528

2629

@@ -426,7 +429,9 @@ def execute(
426429

427430
context = StepContext(
428431
inputs=resolved_inputs,
429-
default_integration=definition.default_integration,
432+
default_integration=self._resolve_workflow_integration(
433+
definition.default_integration
434+
),
430435
default_model=definition.default_model,
431436
default_options=definition.default_options,
432437
project_root=str(self.project_root),
@@ -474,7 +479,9 @@ def resume(self, run_id: str) -> RunState:
474479
context = StepContext(
475480
inputs=state.inputs,
476481
steps=state.step_results,
477-
default_integration=definition.default_integration,
482+
default_integration=self._resolve_workflow_integration(
483+
definition.default_integration
484+
),
478485
default_model=definition.default_model,
479486
default_options=definition.default_options,
480487
project_root=str(self.project_root),
@@ -713,17 +720,18 @@ def _resolve_inputs(
713720
if not isinstance(input_def, dict):
714721
continue
715722
if name in provided:
716-
resolved[name] = self._coerce_input(
717-
name, provided[name], input_def
718-
)
723+
value = provided[name]
724+
# Resolve "auto" sentinel before enum validation so workflows
725+
# with a constrained enum (e.g. enum: [claude, copilot]) don't
726+
# reject the sentinel before it can be expanded.
727+
if name == "integration" and value == "auto":
728+
value = self._resolve_default(name, value)
729+
resolved[name] = self._coerce_input(name, value, input_def)
719730
elif "default" in input_def:
720731
resolved[name] = self._resolve_default(name, input_def["default"])
721732
elif input_def.get("required", False):
722733
msg = f"Required input {name!r} not provided."
723734
raise ValueError(msg)
724-
# Also resolve "auto" sentinel when explicitly supplied by the caller
725-
if resolved.get("integration") == "auto":
726-
resolved["integration"] = self._resolve_default("integration", "auto")
727735
return resolved
728736

729737
def _resolve_default(self, name: str, default: Any) -> Any:
@@ -737,6 +745,18 @@ def _resolve_default(self, name: str, default: Any) -> Any:
737745
return self._load_project_integration()
738746
return default
739747

748+
def _resolve_workflow_integration(self, integration: str | None) -> str | None:
749+
"""Resolve the workflow-level integration sentinel.
750+
751+
If the workflow YAML sets ``workflow.integration: auto``, the string
752+
``"auto"`` is stored in ``WorkflowDefinition.default_integration``.
753+
This helper expands that sentinel to the project integration so it
754+
never leaks into ``StepContext`` or step dispatch.
755+
"""
756+
if integration == "auto":
757+
return self._resolve_default("integration", "auto")
758+
return integration
759+
740760
def _load_project_integration(self) -> str:
741761
"""Read the active integration key from project metadata.
742762
@@ -747,7 +767,8 @@ def _load_project_integration(self) -> str:
747767
contains a valid non-empty integration key.
748768
"""
749769

750-
def _read_integration(path: Path, *keys: str) -> str | None:
770+
def _read_init_options(path: Path, *keys: str) -> str | None:
771+
"""Read a key from a legacy init-options JSON file."""
751772
if not path.is_file():
752773
return None
753774
try:
@@ -760,17 +781,26 @@ def _read_integration(path: Path, *keys: str) -> str | None:
760781
value = data.get(key)
761782
if isinstance(value, str):
762783
value = value.strip()
763-
if value and value != "auto": # skip "auto" to avoid circular resolution
784+
if value and value != "auto":
764785
return value
765786
return None
766787

767-
integration = _read_integration(
768-
self.project_root / _INTEGRATION_JSON, "integration"
769-
)
770-
if integration is not None:
771-
return integration
772-
773-
integration = _read_integration(
788+
# Primary source: .specify/integration.json — use the shared normalized
789+
# reader so both "integration" and "default_integration" fields are
790+
# handled and future schema versions are respected.
791+
json_path = self.project_root / _INTEGRATION_JSON
792+
if json_path.is_file():
793+
try:
794+
data = json.loads(json_path.read_text(encoding="utf-8"))
795+
except (OSError, UnicodeDecodeError, json.JSONDecodeError):
796+
data = None
797+
if isinstance(data, dict):
798+
key = _default_integration_key(data)
799+
if key and key != "auto":
800+
return key
801+
802+
# Secondary source: .specify/init-options.json for older projects.
803+
integration = _read_init_options(
774804
self.project_root / _INIT_OPTIONS_FILE,
775805
"integration",
776806
"ai",

tests/test_workflows.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2022,3 +2022,67 @@ def test_integration_auto_integration_json_takes_priority(self, project_dir):
20222022
)
20232023
engine = WorkflowEngine(project_dir)
20242024
assert engine._load_project_integration() == "gemini"
2025+
2026+
def test_integration_explicit_auto_with_enum_constraint(self, project_dir):
2027+
"""Explicit --input integration=auto works even when enum excludes 'auto'.
2028+
2029+
Issue: enum validation in _coerce_input() ran before the auto-sentinel
2030+
was resolved, so enum: [claude, copilot] rejected the 'auto' value.
2031+
"""
2032+
from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition
2033+
2034+
(project_dir / ".specify" / "integration.json").write_text(
2035+
'{"integration": "claude"}', encoding="utf-8"
2036+
)
2037+
engine = WorkflowEngine(project_dir)
2038+
yaml_str = """
2039+
schema_version: "1.0"
2040+
workflow:
2041+
id: "enum-auto-test"
2042+
name: "Enum Auto Test"
2043+
version: "1.0.0"
2044+
inputs:
2045+
integration:
2046+
type: string
2047+
enum: ["claude", "copilot"]
2048+
default: "auto"
2049+
steps:
2050+
- id: echo
2051+
type: shell
2052+
run: "echo {{ inputs.integration }}"
2053+
"""
2054+
definition = WorkflowDefinition.from_string(yaml_str)
2055+
# Explicit "auto" must resolve to "claude" without raising on the enum
2056+
resolved = engine._resolve_inputs(definition, {"integration": "auto"})
2057+
assert resolved["integration"] == "claude"
2058+
2059+
def test_workflow_level_integration_auto_resolves_in_context(self, project_dir):
2060+
"""workflow.integration: auto is resolved before reaching StepContext.
2061+
2062+
Issue: definition.default_integration was passed raw as 'auto' into
2063+
StepContext, causing step dispatch to look for an 'auto' CLI.
2064+
"""
2065+
from specify_cli.workflows.engine import WorkflowEngine
2066+
2067+
(project_dir / ".specify" / "integration.json").write_text(
2068+
'{"integration": "opencode"}', encoding="utf-8"
2069+
)
2070+
engine = WorkflowEngine(project_dir)
2071+
resolved = engine._resolve_workflow_integration("auto")
2072+
assert resolved == "opencode"
2073+
2074+
def test_integration_auto_reads_default_integration_field(self, project_dir):
2075+
"""integration.json with 'default_integration' key is resolved correctly.
2076+
2077+
Issue: _load_project_integration() only queried the legacy 'integration'
2078+
field; state files written with the newer 'default_integration' field
2079+
(as produced by default_integration_key()) were silently ignored.
2080+
"""
2081+
from specify_cli.workflows.engine import WorkflowEngine
2082+
2083+
(project_dir / ".specify" / "integration.json").write_text(
2084+
'{"default_integration": "gemini", "integration_state_schema": 1}',
2085+
encoding="utf-8",
2086+
)
2087+
engine = WorkflowEngine(project_dir)
2088+
assert engine._load_project_integration() == "gemini"

0 commit comments

Comments
 (0)