feat(omp): support integration args and options in build_exec_args by kevinnguyenhoang91 · Pull Request #4876 · github/spec-kit

kevinnguyenhoang91 · 2026-10-08T08:01:09Z

Pass per-step integration_args straight through to the omp CLI.
Map the named options profile, thinking, and tools to their flags.
Add validate_runtime_config to reject unknown options, empty values, and invalid thinking levels before the command runs.
Reject integration_options.model so model selection stays on the command-step 'model' field.

Description

Testing

Tested locally with uv run specify --help
Ran existing tests with uv sync && uv run pytest
Tested with a sample project (if applicable)

AI Disclosure

I did not use AI assistance for this contribution
I did use AI assistance (fill in the disclosure below)

AI disclosure: N/A

- Pass per-step integration_args straight through to the omp CLI. - Map the named options profile, thinking, and tools to their flags. - Add validate_runtime_config to reject unknown options, empty values, and invalid thinking levels before the command runs. - Reject integration_options.model so model selection stays on the command-step 'model' field.

Copilot

🟡 Changes recommended

The new deterministic argument-building and validation behavior lacks required positive and negative tests.

1 open finding

Missing tests for integration_args validation and named options · New

What changed in this PR

Adds OMP runtime argument and option support with upfront validation.

Changes:

Passes per-step arguments to OMP.
Maps profile, thinking, and tools options to CLI flags.
Validates runtime configuration values.

File	Description
`src/specify_cli/integrations/omp/__init__.py`	Builds and validates OMP runtime arguments.

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

- Verify integration_args and integration_options are passed to omp and ordered after SPECKIT_INTEGRATION_OMP_EXTRA_ARGS. - Accept every documented thinking level and valid runtime configs. - Reject malformed args, the reserved model option, and invalid, unknown or non-string option keys and values.

Copilot

🔵 Needs a closer look

Raw arguments bypass model-field enforcement, and the new public options remain undocumented.

0 open findings

1 resolved since last review

Missing tests for integration_args validation and named options

Previously missed (2)

In code that hasn't changed since last review

Reject model flags in per-step integration_args

src/specify_cli/integrations/omp/__init__.py:57

integration_args can still select a model (for example, ['--model', 'gpt-5']), so a step can run a model while its top-level model field—and recorded output—remain unset. This defeats the stated invariant enforced for integration_options.model; reject OMP's model flag forms in per-step raw args as well, with a regression test.

Document OMP runtime options in the integrations reference

src/specify_cli/integrations/omp/__init__.py:33

These new public runtime options are not documented in the supported-integrations reference, and the workflow reference still describes only Docker Agent's named options. design/integration.md:65-66 requires agent-specific options to be represented in docs/reference/integrations.md; document OMP's profile, thinking, and tools options (including accepted thinking values and argument precedence) so workflow authors can discover and use this API.

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

kevinnguyenhoang91 · 2026-10-08T08:26:21Z

Fixing:

@copilot-pull-request-reviewer: review summary — 🟡 Changes recommended
@copilot-pull-request-reviewer: review summary — 🔵 Needs a closer look

Will be in the next commit.

Model selection must go through the command-step `model` field, so a `--model` flag in `integration_args` now fails validation before OMP is launched. This matches the existing rejection of `integration_options.model` and avoids two competing model sources on the command line. - Reject `--model` and `--model=...` in `integration_args`; `--models` (Ctrl+P cycling scope) stays allowed. - Document `profile`, `thinking`, and `tools` options, `integration_args` passthrough, and argument ordering in the integrations reference. - Link the OMP options from the workflows per-step configuration section. - Add tests for rejected `--model` forms and the allowed `--models` flag.

Copilot

🟢 Approval recommended

The implementation matches the documented behavior and includes appropriate positive and negative coverage.

0 open findings

1 resolved since last review

Missing tests for integration_args validation and named options

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Copilot

🟡 Changes recommended

An accepted -- argument prevents generated options and the intended prompt from being parsed correctly.

1 open finding

Reject integration_args containing option terminator '--' · New

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

        self.validate_runtime_config(integration_args, integration_options)
        args = [self._resolve_executable(), "--print"]
        self._apply_extra_args_env_var(args)
+        args.extend(integration_args or ())


mnriem · 2026-10-08T12:19:04Z

Please address Copilot feedback

Copilot AI balanced review requested due to automatic review settings October 8, 2026 08:01

kevinnguyenhoang91 requested a review from mnriem as a code owner October 8, 2026 08:01

Copilot started reviewing on behalf of kevinnguyenhoang91 October 8, 2026 08:01 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

Comment thread src/specify_cli/integrations/omp/__init__.py

Copilot AI balanced review requested due to automatic review settings October 8, 2026 08:21

Copilot started reviewing on behalf of kevinnguyenhoang91 October 8, 2026 08:22 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 8, 2026 08:30

Copilot started reviewing on behalf of kevinnguyenhoang91 October 8, 2026 08:30 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

mnriem requested a balanced review from Copilot October 8, 2026 11:50

Copilot started reviewing on behalf of mnriem October 8, 2026 11:51 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

Comment thread src/specify_cli/integrations/omp/__init__.py

self.validate_runtime_config(integration_args, integration_options)

args = [self._resolve_executable(), "--print"]

self._apply_extra_args_env_var(args)

args.extend(integration_args or ())

mnriem added the triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review label Oct 8, 2026

This was referenced Oct 9, 2026

[Controlled test; do not merge] feat(omp): support integration args and options in build_exec_args KSchlobohm/spec-kit#66

Closed

[Controlled test; do not merge] feat(omp): support integration args and options in build_exec_args KSchlobohm/spec-kit#67

Draft

Navigation Menu

Repository navigation

feat(omp): support integration args and options in build_exec_args - #4876

feat(omp): support integration args and options in build_exec_args#4876
kevinnguyenhoang91 wants to merge 3 commits into
github:mainfrom
kevinnguyenhoang91:kevinnguyenhoang91/feat/enable_integration_args_options_omp

kevinnguyenhoang91 commented Oct 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

kevinnguyenhoang91 commented Oct 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

mnriem commented Oct 8, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

kevinnguyenhoang91 commented Oct 8, 2026

Description

Testing

AI Disclosure

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

kevinnguyenhoang91 commented Oct 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

mnriem commented Oct 8, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants