Skip to content

feat(workflows): add per-step integration configuration - #4425

Open
philo-x wants to merge 4 commits into
github:mainfrom
philo-x:feat/4275-per-step-integration-config
Open

feat(workflows): add per-step integration configuration#4425
philo-x wants to merge 4 commits into
github:mainfrom
philo-x:feat/4275-per-step-integration-config

Conversation

@philo-x

@philo-x philo-x commented Sep 3, 2026

Copy link
Copy Markdown

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

Summary

  • add optional per-step integration_args and integration_options to workflow command steps
  • resolve, validate, isolate, and persist runtime integration configuration for deterministic resume behavior
  • add Docker Agent support for per-step agent references and validated agent and safety options, using the command-step model field for model selection while retaining the legacy extra-args path for steps without per-step integration_args
  • document the workflow syntax and audit all built-in execution signatures for compatibility

Closes #4275

Testing

  • .venv/bin/python -m pytest tests/integrations/test_base.py tests/integrations/test_integration_docker_agent.py tests/test_workflows.py -q — 1121 passed on b4bcd5f7 after syncing main
  • Ruff 0.15.0 targeted checks (E9,F63,F7,F82,S602,S604,S605)
  • git diff --check
  • Earlier full suite result (not rerun for this precedence follow-up): 7523 passed, 195 skipped, with 3 failures in untouched areas (PowerShell launcher availability and two bundler reference tests)

AI Disclosure

  • I did use AI assistance

Implementation, tests, documentation, and this PR description were authored by OpenAI Codex (model: GPT-5, autonomous) on behalf of @philo-x.

Precedence follow-up, regression tests, documentation updates, and main synchronization authored by OpenAI Codex (model: GPT-6, autonomous) on behalf of @philo-x.

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Combining per-step Docker Agent arguments with the documented legacy environment value can emit two conflicting agent references.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds per-step runtime configuration for workflow command integrations, including deterministic resume support and Docker Agent-specific handling.

Changes:

  • Resolves, validates, persists, and dispatches per-step integration arguments/options.
  • Adds Docker Agent runtime options and agent references.
  • Updates compatibility tests and workflow documentation.
File summaries
File Description
workflows/README.md Documents runtime configuration syntax.
tests/test_workflows.py Tests resolution, isolation, validation, and resume.
tests/integrations/test_integration_docker_agent.py Tests Docker Agent argv and validation.
tests/integrations/test_base.py Verifies base behavior and signatures.
src/specify_cli/workflows/steps/command/__init__.py Resolves and dispatches runtime configuration.
src/specify_cli/workflows/engine.py Persists configuration for resume.
src/specify_cli/workflows/base.py Marks resumed execution contexts.
src/specify_cli/integrations/base.py Extends integration runtime APIs.
src/specify_cli/integrations/docker_agent/__init__.py Implements Docker Agent configuration support.
src/specify_cli/integrations/copilot/__init__.py Updates custom dispatch signatures.
src/specify_cli/integrations/agy/__init__.py Updates execution signature.
src/specify_cli/integrations/codex/__init__.py Updates execution signature.
src/specify_cli/integrations/cursor_agent/__init__.py Updates execution signature.
src/specify_cli/integrations/devin/__init__.py Updates execution signature.
src/specify_cli/integrations/droid/__init__.py Updates execution signature.
src/specify_cli/integrations/dsh/__init__.py Updates execution signature.
src/specify_cli/integrations/goose/__init__.py Updates execution signature.
src/specify_cli/integrations/grok/__init__.py Updates execution signature.
src/specify_cli/integrations/hermes/__init__.py Updates execution signature.
src/specify_cli/integrations/muse/__init__.py Updates execution signature.
src/specify_cli/integrations/omp/__init__.py Updates execution signature.
src/specify_cli/integrations/opencode/__init__.py Updates execution signature.
src/specify_cli/integrations/rovodev/__init__.py Updates execution signature.
Review details
  • Files reviewed: 23/23 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/specify_cli/integrations/docker_agent/__init__.py Outdated

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address Copilot feedback

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)
@philo-x

philo-x commented Sep 4, 2026

Copy link
Copy Markdown
Author

Posted on behalf of @philo-x by OpenAI Codex (model: GPT-5).

Addressed the review feedback in 83b6b9a2f743bf505f0de59ea0618ebb6bbfa9e4:

  • replaced the legacy Docker Agent config reference when per-step integration_args is present while preserving legacy flags
  • made the command-step model field the single Docker Agent model source
  • re-resolved the complete dispatch configuration consistently on resume
  • enforced runtime configuration validation for direct execution-argument builders
  • added regression coverage and updated the workflow reference documentation

Validation: 1,111 targeted tests pass; Ruff 0.15.0 and git diff --check pass. The broader integration suite reports 2,892 passed and 5 skipped, with only the pre-existing PowerShell launcher availability failure remaining.

@philo-x
philo-x requested a review from mnriem September 4, 2026 13:18
@mnriem
mnriem requested a balanced review from Copilot September 8, 2026 21:56
@mnriem mnriem added author-awaiting Waiting on author response triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review labels Sep 8, 2026
@mnriem

mnriem commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Thanks — and good that this rides on the agreed #4275 design, with disclosure. One merge-blocker to resolve from the review: when the legacy Docker-Agent env var has its documented shape (e.g. ./default.yaml --hide-tool-results) and a step supplies integration_args, the current code appends both agent references, emitting two conflicting ones. Since preserving the legacy path is an explicit goal of this PR, please define the precedence (I'd expect per-step integration_args to win and the legacy value to be ignored when both are present) and add a test for that collision. Re-request review once that's in.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation satisfies the linked acceptance criteria with comprehensive compatibility, isolation, validation, persistence, and resume coverage.

Review details
  • Files reviewed: 24/24 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Assisted-by: OpenAI Codex (model: GPT-6, autonomous)
Assisted-by: OpenAI Codex (model: GPT-6, autonomous)
@philo-x

philo-x commented Sep 9, 2026

Copy link
Copy Markdown
Author

Posted on behalf of @philo-x by OpenAI Codex (model: GPT-6, autonomous).

Thanks @mnriem. The stricter precedence is implemented in 88d6abd: when per-step integration_args supplies an agent reference, the entire SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS value is ignored for that step, including malformed quoting. The legacy environment behavior remains unchanged when no per-step agent reference is supplied.

For completeness, the previous head (83b6b9a2) already replaced the legacy agent reference with the per-step one, but still inherited any trailing legacy flags. This follow-up makes the precedence stricter by ignoring the legacy value entirely whenever a per-step agent reference is present.

Regression coverage now includes legacy agent-plus-flags, flags-only, and malformed quoting. Both workflow guides document the precedence, and the PR description identifies the command-step model field as the model source.

Synced with main at 0c8e31ff; final head is b4bcd5f. Validation on that head: 1,121 targeted tests passed, Ruff 0.15.0 targeted checks passed, and git diff --check passed. CI workflows are currently awaiting maintainer approval.

@mnriem, this is ready for another review. Thanks again.

@mnriem
mnriem requested a balanced review from Copilot September 9, 2026 16:20
@mnriem

mnriem commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks — that's exactly the precedence I wanted: when a per-step integration_args agent reference is present, ignore the legacy SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS entirely (including malformed quoting), and the regression coverage for legacy-agent-plus-flags / flags-only / malformed quoting nails the cases down. Re-requesting review/CI on 88d6abd; once it's green I'll merge (closes #4275).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation matches the linked requirements and includes comprehensive compatibility and regression coverage.

Review details
  • Files reviewed: 24/24 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author-awaiting Waiting on author response triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Per step integration configuration in workflows

4 participants