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 (9843 passed; 11 failures, all in the *_python_parity shell-script tests, which this PR does not touch)
Tested with a sample project (if applicable): specify init sample --integration omp generates .omp/commands/*

AI Disclosure

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

AI disclosure: Claude Code (model: Claude Sonnet 5.5, autonomous) handled review feedback: it added the -- option-terminator rejection and its tests (6234eee), ran the checks above, and updated this description. Authorship of the earlier commits is not covered by this note; the contributor should add it.

🤖 Generated with Claude Code

- 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.

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

Please address Copilot feedback

A bare '--' passed validation and, being inserted before the generated --model, --mode json and prompt, made OMP treat them as positional messages. Reject it in validate_runtime_config and add regression tests. Assisted-by: Claude Code (model: Claude Sonnet 5.5, autonomous) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

kevinnguyenhoang91 · 2026-10-30T12:50:46Z

Review round summary (fix commit 6234eee):

Missing tests for integration_args/options (thread r4216493726): already addressed in da070fe. Positive argv and negative cases (model, unknown/empty values, malformed args, invalid thinking) are covered. The thread was already resolved, so no change this round.
Reject -- in integration_args (thread r4218591436): fixed in 6234eee, with regression tests.
Earlier Copilot notes (reject --model in per-step args; document OMP options) were handled in 1362568.

AI-generated reply posted on behalf of @kevinnguyenhoang91 by Claude Code (model: Claude Sonnet 5.5, autonomous). The agent assessed the review comments and wrote the fix and tests; a human has not line-by-line reviewed them.

Copilot

🔵 Needs a closer look

Human assessment is needed because OMP’s streaming argument parsing can treat the prompt as a session selector rather than execute it when --resume is supplied.

No open issues 🎉

1 resolved since last review

Reject integration_args containing option terminator '--'

_{Review effort: Balanced}

Upstream moved per-agent docs to docs/integrations/; relocate the OMP runtime options section to docs/integrations/omp.md and keep both sides of the workflows.md change. Assisted-by: Claude Code (model: Claude Sonnet 5.5, autonomous) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot

🟢 Approval recommended

No active review issues remain for the per-step runtime configuration changes.

No open issues 🎉

_{Review effort: Balanced}

OMP normalizes '--=' to the '--' terminator, and bare value-taking flags (--system-prompt, --resume) consume generated flags or the prompt. Accept only known valueless flags and self-contained --name=value tokens, and add regression tests that replay OMP's parsing over the generated argv. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

kevinnguyenhoang91 · 2026-11-03T02:14:20Z

@mnriem Fixed in 818ce72. I read OMP's args.ts/flag-tables.ts and confirmed all three cases: --= is normalized to -- (end of options), string flags like --system-prompt consume the next token even when flag-looking, and --resume consumes the next non-dash token (the prompt when nothing generated follows).

integration_args now accepts only (a) known valueless flags (e.g. --no-session) and (b) self-contained --name=value tokens. Bare value-taking flags, --= forms, positional text, @file, short flags and --help/--version/--print are rejected before launch. This is stricter than before (bare --verbose / --models a,b no longer pass), because an allowlist can't tell which bare flags take values; docs in docs/integrations/omp.md are updated.

Regression tests replay OMP's parseArgs semantics over the generated argv and assert the prompt, model, thinking and mode survive; they fail without the fix (11 failures) and pass with it. Caveat: omp isn't installed in my environment, so the replay is a Python port of the parser, not the real CLI. One unrelated shell-script parity test (test_check_prerequisites_python_parity) failed in my broader run.

AI-generated reply posted on behalf of @kevinnguyenhoang91 by Claude Code (model: Claude Sonnet 5.5, autonomous). The agent made the change and tests; a human has not line-by-line reviewed them.

Copilot

🔵 Needs a closer look

Human assessment is needed because inline reserved flags can bypass validation and cause a workflow to report success without executing the prompt.

No open issues 🎉

_{Review effort: Balanced (auto)}

…at/enable_integration_args_options_omp

Copilot

🟡 Changes recommended

Equals-form --help and --version arguments bypass validation, causing OMP to display information and exit instead of executing the workflow prompt.

1 open issue

Reject equals-form help and version flags · New

_{Review effort: Balanced (auto)}

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot

🔵 Needs a closer look

Human assessment is needed to evaluate critical workflow-prompt bypasses and unexpected early exits caused by accepted equals-form flags.

No open issues 🎉

1 resolved since last review

Reject equals-form help and version flags

_{Review effort: Balanced (auto)}

Copilot

🟡 Changes recommended

Critical OMP argument-handling gaps let accepted flags bypass workflow execution, write unintended files, or enter an RPC loop that can wait indefinitely.

2 open issues

_{Review effort: Balanced (auto)}

mnriem · 2026-11-03T15:59:16Z

Please address Copilot feedback

…SON is off Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot

🟢 Approval recommended

No active review issues remain for the per-step Oh My Pi runtime configuration changes.

No open issues 🎉

2 resolved since last review

_{Review effort: Balanced (auto)}

Copilot

🟡 Changes recommended

Validation accepts --yolo=false and --auto-approve=false, but OMP ignores their values and enables automatic approval, allowing tool execution without expected confirmation.

1 open issue

Reject inline false values for auto-approval flags · New

_{Review effort: Balanced (auto)}

mnriem · 2026-11-04T18:49:29Z

Please address Copilot feedback

OMP's parseArgs splits `--name=value` at `=`, matches only the flag name for booleans, and drops the inline value, so `--yolo=false` and `--auto-approve=false` still enable automatic approval. Reject the equals-form of every known valueless flag; bare forms stay allowed. Constraint: OMP parser (packages/coding-agent/src/cli/args.ts) ignores inline values on boolean flags Rejected: Block only --yolo/--auto-approve | every boolean flag has the same misleading =false semantics Confidence: high Scope-risk: narrow Assisted-by: Claude Code (model: Claude Opus 5.5, autonomous) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kevinnguyenhoang91 · 2026-11-08T06:44:34Z

Review round summary (fix commit b38d29e):

--yolo=false / --auto-approve=false enable auto-approval (thread r4471659765): fixed. OMP ignores inline values on boolean flags, so integration_args now rejects the --flag=value form of every known valueless flag. Bare forms are unchanged. New tests: rejection of the false/empty/0 forms and acceptance of the bare approval flags. tests/integrations/test_integration_omp.py and tests/workflows pass (687 tests).

AI-generated summary posted on behalf of @kevinnguyenhoang91 by Claude Code (model: Claude Opus 5.5, autonomous). The agent made the change; a human has not line-by-line reviewed it.

Copilot

🟢 Approval recommended

No active review issues are reported for the per-step OMP runtime configuration change.

No open issues 🎉

1 resolved since last review

Reject inline false values for auto-approval flags

_{Review effort: Balanced (auto)}

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 Resolved

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 Resolved

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

Closed

Copilot AI balanced review requested due to automatic review settings October 30, 2026 12:50

Copilot started reviewing on behalf of kevinnguyenhoang91 October 30, 2026 12:50

View session

Copilot AI reviewed Oct 30, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 30, 2026 13:05

Copilot started reviewing on behalf of kevinnguyenhoang91 October 30, 2026 13:06

View session

Copilot AI reviewed Oct 30, 2026

View reviewed changes

Copilot AI auto review requested due to automatic review settings November 3, 2026 02:14

Copilot started reviewing on behalf of kevinnguyenhoang91 November 3, 2026 02:15

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into kevinnguyenhoang91/fe…

4c865fe

…at/enable_integration_args_options_omp

Copilot AI auto review requested due to automatic review settings November 3, 2026 02:27

Copilot started reviewing on behalf of kevinnguyenhoang91 November 3, 2026 02:28

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

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

fix(omp): reject equals-form help/version/print in integration_args

b236d49

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot AI auto review requested due to automatic review settings November 3, 2026 12:01

Copilot started reviewing on behalf of kevinnguyenhoang91 November 3, 2026 12:02

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

mnriem requested an auto review from Copilot November 3, 2026 15:44

Copilot started reviewing on behalf of mnriem November 3, 2026 15:44

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

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

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

fix(omp): reserve --export/--alias/--mode and emit --mode text when J…

0042975

…SON is off Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot AI auto review requested due to automatic review settings November 4, 2026 03:07

Copilot started reviewing on behalf of kevinnguyenhoang91 November 4, 2026 03:08

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

mnriem requested an auto review from Copilot November 4, 2026 18:31

Copilot started reviewing on behalf of mnriem November 4, 2026 18:31

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

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

Copilot AI auto review requested due to automatic review settings November 8, 2026 06:44

Copilot started reviewing on behalf of kevinnguyenhoang91 November 8, 2026 06:44

View session

Copilot AI reviewed Nov 8, 2026

View reviewed changes

Conversation

kevinnguyenhoang91 commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Uh oh!

mnriem commented Oct 8, 2026

Uh oh!

kevinnguyenhoang91 commented Oct 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

kevinnguyenhoang91 commented Nov 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

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!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

mnriem commented Nov 3, 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!

Uh oh!

mnriem commented Nov 4, 2026

Uh oh!

kevinnguyenhoang91 commented Nov 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

This branch has not been deployed

kevinnguyenhoang91 commented Oct 8, 2026 •

edited

Loading