fix(workflows): auto-detect project integration instead of hardcoding copilot default#2408
fix(workflows): auto-detect project integration instead of hardcoding copilot default#2408markuswondrak wants to merge 23 commits intogithub:mainfrom
Conversation
… copilot (github#2406) The workflow engine hardcoded 'copilot' as the default integration input, ignoring the project's configured integration in .specify/integration.json. This caused workflows to fail when a project was initialized with a different integration (e.g. opencode) but the copilot CLI happened to be installed. Changes: - Change workflow.yml integration default from 'copilot' to 'auto' - Add _resolve_integration_auto() to WorkflowEngine that reads .specify/integration.json and falls back to 'copilot' if absent - Post-process resolved inputs to replace 'auto' with detected value - Add 4 regression tests for auto-detection scenarios Fixes github#2406 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the built-in speckit workflow and workflow engine to avoid hardcoding "copilot" as the default integration by introducing an "auto" integration mode that resolves from project configuration.
Changes:
- Set
workflows/speckit/workflow.ymlintegration input default to"auto"and updated the prompt to document it. - Added
WorkflowEngine._resolve_integration_auto()and post-processing in_resolve_inputs()to read.specify/integration.json, with fallback to"copilot". - Added regression tests for auto-detection, fallback behavior, explicit override precedence, and empty JSON handling; added an Unreleased changelog entry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| workflows/speckit/workflow.yml | Switches workflow input default integration from "copilot" to "auto" so runs honor project configuration. |
| src/specify_cli/workflows/engine.py | Implements "auto" integration resolution by reading .specify/integration.json with a backward-compatible fallback. |
| tests/test_workflows.py | Adds regression tests to ensure "auto" resolves correctly and preserves override behavior. |
| CHANGELOG.md | Notes the fix in the Unreleased section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mnriem
left a comment
There was a problem hiding this comment.
Thank you for looking into this. Please address Copilot feedback. If not applicable, please explain why
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 4
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback
- Add tests for JSONDecodeError and OSError fallback paths in _resolve_integration_auto() - Centralize INTEGRATION_JSON constant in workflows/constants.py (zero-dependency module) - Change changelog reference to (Fixes github#2406) for auto-close semantics - Remove static requires.integrations.any from speckit workflow (incomplete with 'auto') - Add opencode to integration prompt examples
|
Thanks for the thorough review! I've addressed all four findings:
|
mnriem
left a comment
There was a problem hiding this comment.
Please resolve conflicts
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/6 changed files
- Comments generated: 3
|
Please revert changes to CHANGELOG.md as it is now auto-generated (still need to update docs to no longer require it) and resolve the other conflict |
- CHANGELOG.md: take released [0.8.3] block from origin, discard auto-generated [Unreleased] section that shouldn't have been committed - __init__.py: add devin_skill_mode from origin, preserve multi-line native_skill_mode format from HEAD Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- engine.py: strip whitespace from integration value in _resolve_integration_auto(); whitespace-only strings like ' ' now fall back to 'copilot' instead of being returned as-is - __init__.py: define INTEGRATION_JSON locally instead of importing from workflows.constants; avoids executing the workflows package __init__ (which registers all step types) during CLI startup for commands that don't use workflows - tests: add test for whitespace-only integration value fallback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Move INTEGRATION_JSON from specify_cli.workflows.constants to specify_cli.constants to avoid importing the workflows package when the main CLI module loads. Update imports in __init__.py and workflows/engine.py accordingly, and remove the now-empty workflows/constants.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
src/specify_cli/workflows/engine.py:747
- The docstring says the method falls back when the file “does not contain a valid” key, but the implementation only checks for a non-empty string (and not a known integration key). Either validate the value against the registered integrations (and fall back if unknown) or adjust the docstring wording to match the current behavior.
def _load_project_integration(self) -> str:
"""Read the active integration key from ``.specify/integration.json``.
Returns the stored integration string, or ``"copilot"`` when the file is
missing, unreadable, or does not contain a valid non-empty key.
The ``"copilot"`` fallback preserves backwards compatibility for projects
that predate the introduction of ``.specify/integration.json``.
"""
- Files reviewed: 3/3 changed files
- Comments generated: 2
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback. If not applicable, please explain why
… auto-detect integration Agent-Logs-Url: https://github.com/markuswondrak/spec-kit/sessions/54bfb375-ba13-4531-b9b0-4140ea6b0edc Co-authored-by: markuswondrak <245696895+markuswondrak@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use SPECIFY_DIR-derived constants for integration.json and init-options.json. Co-authored-by: Cursor <cursoragent@cursor.com>
Sync fork branch with latest changes from the original repository. Co-authored-by: Cursor <cursoragent@cursor.com>
…keep PR branch version
fix(workflows): centralize path constants and add init-options.json fallback for auto-detect
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- engine.py: Import INTEGRATION_JSON from integration_state (single source of truth) - engine.py: _load_project_integration() uses default_integration_key() to properly resolve default_integration -> integration key resolution - engine.py: _resolve_inputs() resolves 'auto' sentinel before enum validation so workflows with enum constraints on integration input work correctly - engine.py: StepContext creation resolves 'auto' in default_integration so context always carries a real integration key - paths.py: Remove duplicate INTEGRATION_JSON constant - workflow.yml: Bump speckit_version to >=0.8.4 (older CLIs don't support 'auto') - tests: Add 5 regression tests for the fixes Fixes review comments from copilot-pull-request-reviewer[bot] on PR github#2408
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ithub#2408) Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _read_integration(path: Path, *keys: str) -> str | None: | ||
| if not path.is_file(): | ||
| return None | ||
| try: | ||
| data = json.loads(path.read_text(encoding="utf-8")) | ||
| except (OSError, UnicodeDecodeError, json.JSONDecodeError): | ||
| return None | ||
| if not isinstance(data, dict): | ||
| return None | ||
| for key in keys: | ||
| value = data.get(key) | ||
| if isinstance(value, str): | ||
| value = value.strip() | ||
| if value and value != "auto": # skip "auto" to avoid circular resolution | ||
| return value | ||
| return None | ||
|
|
||
| integration = _read_integration( | ||
| self.project_root / _INTEGRATION_JSON, "integration" | ||
| ) | ||
| if integration is not None: | ||
| return integration | ||
|
|
||
| integration = _read_integration( | ||
| self.project_root / _INIT_OPTIONS_FILE, | ||
| "integration", | ||
| "ai", | ||
| ) | ||
| if integration is not None: | ||
| return integration |
| if name in provided: | ||
| resolved[name] = self._coerce_input( | ||
| name, provided[name], input_def | ||
| ) | ||
| elif "default" in input_def: | ||
| resolved[name] = input_def["default"] | ||
| resolved[name] = self._resolve_default(name, input_def["default"]) | ||
| elif input_def.get("required", False): | ||
| msg = f"Required input {name!r} not provided." | ||
| raise ValueError(msg) | ||
| # Also resolve "auto" sentinel when explicitly supplied by the caller | ||
| if resolved.get("integration") == "auto": | ||
| resolved["integration"] = self._resolve_default("integration", "auto") |
| type: string | ||
| default: "copilot" | ||
| prompt: "Integration to use (e.g. claude, copilot, gemini)" | ||
| default: "auto" |
Description
Fixes #2406
The built-in
speckitworkflow used"copilot"as the defaultintegrationinput. In projects initialized with a different integration such asopencode, workflows could dispatch to the wrong agent and fail with"No such agent: speckit.specify"when thecopilotCLI happened to be installed locally.Changes
workflows/speckit/workflow.ymlintegrationinput default from"copilot"to"auto"opencodeand the explicitautooptionrequires.integrations.anyallowlistsrc/specify_cli/workflows/engine.py_resolve_default()and_load_project_integration()to read.specify/integration.json"copilot"fallback for missing, unreadable, invalid, or empty config_resolve_inputs()so both defaulted and explicit--input integration=autovalues resolve through the same pathtests/test_workflows.pyintegration=auto, explicit override precedence, missing file fallback, invalid JSON fallback, OSError fallback, and whitespace-only valuesHow it works
integrationto"auto""auto", the engine reads.specify/integration.jsonand uses the stored integration key"copilot"for backwards compatibilityTesting
uvx ruff check src/ tests/test_workflows.pyuv run pytest tests/test_workflows.pyuv run pytest tests/test_workflows.py -k TestIntegrationAutoDetect -vAI Disclosure