diff --git a/docs/integrations/omp.md b/docs/integrations/omp.md index b32a3fc6f4..339131732e 100644 --- a/docs/integrations/omp.md +++ b/docs/integrations/omp.md @@ -28,4 +28,56 @@ Invoke them in the agent's chat using dotted notation, for example Workflow steps use `omp --print `; `--print` enables one-shot mode and the prompt is positional. The adapter forwards a configured `--model` and uses `--mode json`, not `--output-format`, for structured output. -There are no integration-specific setup options. +There are no integration-specific setup options; see Runtime Options below. + +## Runtime Options + +Command steps can pass per-step runtime configuration to `omp --print` (see +[Per-Step Integration Configuration](../reference/workflows.md#per-step-integration-configuration)): + +| `integration_options` key | OMP flag | Value | +| ------------------------- | ------------ | ------------------------------------------------------------------------ | +| `profile` | `--profile` | Isolated OMP profile name | +| `thinking` | `--thinking` | One of `off`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `auto` | +| `tools` | `--tools` | Comma-separated list of tools to enable | + +```yaml +- id: plan-with-omp + type: command + command: speckit.plan + integration: omp + integration_args: + - "--no-session" + integration_options: + profile: work + thinking: high + tools: "read,bash" + model: "gpt-5" +``` + +Option values must be non-empty strings; unknown keys and unsupported +`thinking` values fail before OMP is launched. Select the model with the step's +top-level `model` field: `integration_options.model` and a `--model` flag in +`integration_args` are both rejected. + +`integration_args` is passed to OMP as raw flags, but OMP's parser lets some +tokens consume or reorder the generated options and prompt, so each value must +be one of: + +- a known valueless flag such as `--no-session`, `--no-title` or `--yolo`; or +- a self-contained `--name=value` token such as `--system-prompt=...` or + `--resume=`. + +Bare value-taking flags (`--system-prompt`, `--resume`), the `--` terminator +and `--=` forms (which OMP normalizes to `--`), positional text, `@file` +references, short flags, and `--help`/`--version`/`--print`/`--export`/`--alias`/`--mode` fail before OMP is +launched. + +Arguments are passed to `omp --print` in this order, so later sources follow +earlier ones on the command line: + +1. `SPECKIT_INTEGRATION_OMP_EXTRA_ARGS` +2. `integration_args` +3. `integration_options` flags +4. `--model` from the step's `model` field +5. `--mode json` (or `--mode text` when JSON output is off), then the prompt diff --git a/src/specify_cli/integrations/omp/__init__.py b/src/specify_cli/integrations/omp/__init__.py index b2088593d2..4de0e9e1ed 100644 --- a/src/specify_cli/integrations/omp/__init__.py +++ b/src/specify_cli/integrations/omp/__init__.py @@ -1,6 +1,8 @@ """Oh My Pi (omp) coding agent integration.""" from __future__ import annotations + +import re from pathlib import Path from collections.abc import Mapping, Sequence @@ -26,6 +28,39 @@ class OmpIntegration(MarkdownIntegration): } multi_install_safe = True + _RUNTIME_OPTION_FLAGS = { + "profile": "--profile", + "thinking": "--thinking", + "tools": "--tools", + } + _THINKING_LEVELS = { + "off", "minimal", "low", "medium", "high", "xhigh", "max", "auto", + } + + # OMP's parser (args.ts / flag-tables.ts in can1357/oh-my-pi) is not + # POSIX-strict: value-taking flags consume the *next* token, even one that + # looks like a flag, and `--=...` is normalized to the `--` terminator. + # To keep generated flags and the prompt intact, ``integration_args`` is + # limited to (a) known valueless flags and (b) self-contained + # ``--name=value`` tokens. + _VALUELESS_FLAGS = frozenset({ + "--allow-home", "--continue", "--from-claude", "--from-codex", + "--no-session", "--no-tools", "--no-lsp", "--no-pty", + "--hide-thinking", "--advisor", "--wait-async", "--external-thinking", + "--prewalk", "--no-prewalk", "--plan-yolo", "--print-thoughts", + "--no-extensions", "--no-skills", "--no-rules", "--no-context-files", + "--no-title", "--no-ui", "--auto-approve", "--yolo", + }) + # OMP splits ``--name=value`` at ``=`` and enables these flags regardless + # of the inline value (``--help=false`` still prints help and exits). + # ``--export``/``--alias`` replace prompt execution (the prompt becomes an + # output filename, or alias installation returns early) and ``--mode`` is + # owned by the adapter, so they are rejected in equals-form too. + _RESERVED_FLAG_NAMES = frozenset({ + "--help", "--version", "--print", "--export", "--alias", "--mode", + }) + _EQUALS_FLAG_RE = re.compile(r"^--[A-Za-z][A-Za-z0-9-]*=") + def build_exec_args( self, prompt: str, @@ -45,9 +80,80 @@ def build_exec_args( 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 ()) + for option, value in (integration_options or {}).items(): + args.extend([self._RUNTIME_OPTION_FLAGS[option], value]) if model: args.extend(["--model", model]) - if output_json: - args.extend(["--mode", "json"]) + args.extend(["--mode", "json" if output_json else "text"]) args.append(prompt) return args + + def validate_runtime_config( + self, + integration_args: Sequence[str] | None = None, + integration_options: Mapping[str, Any] | None = None, + ) -> None: + """Validate OMP per-step CLI flags and named options.""" + if not all( + isinstance(value, str) and value.strip() + for value in integration_args or () + ): + raise ValueError("OMP 'integration_args' values must be non-empty strings.") + if "--" in (integration_args or ()): + raise ValueError( + "OMP 'integration_args' must not contain the '--' option " + "terminator; it would turn generated options and the prompt " + "into positional arguments." + ) + if any( + value == "--model" or value.startswith("--model=") + for value in integration_args or () + ): + raise ValueError( + "OMP model selection must use the command-step 'model' field, " + "not a '--model' flag in 'integration_args'." + ) + + for value in integration_args or (): + if ( + value in self._VALUELESS_FLAGS + or self._EQUALS_FLAG_RE.match(value) + and value.partition("=")[0] not in self._RESERVED_FLAG_NAMES + ): + continue + raise ValueError( + f"OMP 'integration_args' value {value!r} is not allowed. Use a " + "known valueless flag (for example '--no-session') or pass " + "flag values inline as '--name=value'; bare value-taking " + "flags, positional text, '@file' references and '--=' forms " + "can consume or reorder the generated options and prompt." + ) + + options = integration_options or {} + if not all(isinstance(name, str) for name in options): + raise ValueError("OMP 'integration_options' keys must be strings.") + if "model" in options: + raise ValueError( + "OMP model selection must use the command-step 'model' field, " + "not 'integration_options.model'." + ) + unknown = sorted(set(options) - self._RUNTIME_OPTION_FLAGS.keys()) + if unknown: + names = ", ".join(repr(name) for name in unknown) + allowed = ", ".join(sorted(self._RUNTIME_OPTION_FLAGS)) + raise ValueError( + f"OMP received unknown integration option(s): {names}. " + f"Supported options: {allowed}." + ) + for name, value in options.items(): + if not isinstance(value, str) or not value.strip(): + raise ValueError( + f"OMP integration option {name!r} must be a non-empty string." + ) + thinking = options.get("thinking") + if thinking is not None and thinking not in self._THINKING_LEVELS: + allowed = ", ".join(sorted(self._THINKING_LEVELS)) + raise ValueError( + f"OMP integration option 'thinking' must be one of: {allowed}." + ) diff --git a/tests/integrations/test_integration_omp.py b/tests/integrations/test_integration_omp.py index 07dd57336b..28e4b2f42a 100644 --- a/tests/integrations/test_integration_omp.py +++ b/tests/integrations/test_integration_omp.py @@ -1,5 +1,7 @@ """Tests for OmpIntegration.""" +import pytest + from specify_cli.integrations import get_integration from .test_integration_base_markdown import MarkdownIntegrationTests @@ -34,3 +36,264 @@ def test_build_exec_args_uses_omp_json_mode(self): "json", "/speckit.specify Build auth", ] + + def test_build_exec_args_passes_integration_args_and_options(self, monkeypatch): + monkeypatch.delenv("SPECKIT_INTEGRATION_OMP_EXECUTABLE", raising=False) + monkeypatch.delenv("SPECKIT_INTEGRATION_OMP_EXTRA_ARGS", raising=False) + + args = get_integration(self.KEY).build_exec_args( + "/speckit.plan", + model="gpt-5", + integration_args=["--no-session", "--yolo"], + integration_options={ + "profile": "work", + "thinking": "high", + "tools": "read,bash", + }, + ) + + assert args == [ + "omp", + "--print", + "--no-session", + "--yolo", + "--profile", + "work", + "--thinking", + "high", + "--tools", + "read,bash", + "--model", + "gpt-5", + "--mode", + "json", + "/speckit.plan", + ] + + def test_build_exec_args_places_integration_args_after_env_extra_args( + self, monkeypatch + ): + monkeypatch.delenv("SPECKIT_INTEGRATION_OMP_EXECUTABLE", raising=False) + monkeypatch.setenv("SPECKIT_INTEGRATION_OMP_EXTRA_ARGS", "--from-env") + + args = get_integration(self.KEY).build_exec_args( + "prompt", + output_json=False, + integration_args=["--no-title"], + integration_options={"thinking": "low"}, + ) + + assert args == [ + "omp", + "--print", + "--from-env", + "--no-title", + "--thinking", + "low", + "--mode", + "text", + "prompt", + ] + + @pytest.mark.parametrize( + "thinking", + ["off", "minimal", "low", "medium", "high", "xhigh", "max", "auto"], + ) + def test_validate_runtime_config_accepts_thinking_levels(self, thinking): + get_integration(self.KEY).validate_runtime_config( + None, {"thinking": thinking} + ) + + @pytest.mark.parametrize( + ("integration_args", "integration_options"), + [(None, None), ([], {}), (["--no-session"], {"profile": "work"})], + ) + def test_validate_runtime_config_accepts_valid_config( + self, integration_args, integration_options + ): + get_integration(self.KEY).validate_runtime_config( + integration_args, integration_options + ) + + @pytest.mark.parametrize("integration_args", [[""], [" "], [42], ["--ok", None]]) + def test_integration_args_reject_malformed_values(self, integration_args): + with pytest.raises(ValueError, match="non-empty strings"): + get_integration(self.KEY).build_exec_args( + "prompt", + integration_args=integration_args, + ) + + @pytest.mark.parametrize( + "integration_args", + [["--model", "gpt-5"], ["--model=gpt-5"], ["--no-session", "--model", "gpt-5"]], + ) + def test_integration_args_model_flag_is_rejected(self, integration_args): + with pytest.raises(ValueError, match="command-step 'model' field"): + get_integration(self.KEY).build_exec_args( + "prompt", + integration_args=integration_args, + ) + + @pytest.mark.parametrize( + "integration_args", [["--"], ["--no-session", "--"], ["--", "extra"]] + ) + def test_integration_args_option_terminator_is_rejected(self, integration_args): + with pytest.raises(ValueError, match="option terminator"): + get_integration(self.KEY).build_exec_args( + "prompt", + integration_args=integration_args, + ) + + @pytest.mark.parametrize( + "integration_args", + [ + ["--="], + ["--=x"], + ["--system-prompt"], + ["--resume"], + ["--models", "a,b"], + ["positional"], + ["@file.md"], + ["-"], + ["-r"], + ["--help"], + ["--print"], + ["--help=false"], + ["--version=false"], + ["--print=false"], + ["--help="], + ["--export=session.jsonl"], + ["--alias=omp-work"], + ["--mode=rpc"], + ], + ) + def test_integration_args_unsafe_tokens_are_rejected(self, integration_args): + with pytest.raises(ValueError, match="not allowed"): + get_integration(self.KEY).build_exec_args( + "prompt", integration_args=integration_args + ) + + @pytest.mark.parametrize( + "integration_args", + [ + ["--no-session", "--yolo"], + ["--system-prompt=be brief"], + ["--resume=abc123"], + ["--models=a,b", "--no-title"], + ], + ) + @pytest.mark.parametrize("output_json", [True, False]) + def test_integration_args_preserve_omp_parsing( + self, monkeypatch, integration_args, output_json + ): + """Replay OMP's parseArgs semantics over the generated argv. + + Mirrors args.ts/flag-tables.ts (can1357/oh-my-pi): ``--x=v`` is split + into ``--x v``, string flags consume the next token even if + flag-looking, optional-value flags consume the next non-dash token, + and ``--`` ends option parsing. + """ + monkeypatch.delenv("SPECKIT_INTEGRATION_OMP_EXECUTABLE", raising=False) + monkeypatch.delenv("SPECKIT_INTEGRATION_OMP_EXTRA_ARGS", raising=False) + argv = get_integration(self.KEY).build_exec_args( + "the prompt", + model="gpt-5", + output_json=output_json, + integration_args=integration_args, + integration_options={"thinking": "high"}, + ) + parsed = _parse_omp_args(argv[1:]) + assert parsed["messages"] == ["the prompt"] + assert parsed["model"] == "gpt-5" + assert parsed["thinking"] == "high" + assert parsed["print"] is True + assert parsed.get("mode") == ("json" if output_json else "text") + + def test_replayed_parser_shows_why_bare_flags_are_unsafe(self): + # Sanity-check the model against the cases from the PR review. + assert _parse_omp_args(["--print", "--=", "--model", "m", "p"])[ + "messages" + ] == ["--model", "m", "p"] + assert _parse_omp_args(["--system-prompt", "--mode", "json", "p"])[ + "system-prompt" + ] == "--mode" + assert _parse_omp_args(["--resume", "p"])["messages"] == [] + + def test_integration_args_allow_models_cycling_flag(self): + # `--models` only scopes Ctrl+P cycling; it does not select the model. + get_integration(self.KEY).validate_runtime_config( + ["--models=claude-sonnet,gpt-5"], None + ) + + def test_integration_options_model_is_rejected(self): + with pytest.raises(ValueError, match="command-step 'model' field"): + get_integration(self.KEY).build_exec_args( + "prompt", + integration_options={"model": "gpt-5"}, + ) + + @pytest.mark.parametrize( + ("options", "message"), + [ + ({1: "value"}, "keys must be strings"), + ({"unknown": "value"}, r"unknown integration option\(s\): 'unknown'"), + ({"profile": ""}, "'profile' must be a non-empty string"), + ({"tools": " "}, "'tools' must be a non-empty string"), + ({"profile": 42}, "'profile' must be a non-empty string"), + ({"thinking": "extreme"}, "'thinking' must be one of"), + ], + ) + def test_integration_options_are_validated(self, options, message): + with pytest.raises(ValueError, match=message): + get_integration(self.KEY).build_exec_args( + "prompt", + integration_options=options, + ) + + +_STRING_FLAGS = { + "--system-prompt", "--model", "--mode", "--thinking", "--tools", + "--models", "--profile", "--alias", "--append-system-prompt", "--cwd", +} +_OPTIONAL_FLAGS = {"--resume", "-r", "--restore", "--session"} + + +def _parse_omp_args(argv): + """Minimal replay of OMP's ``parseArgs`` for the flags this adapter emits.""" + args = list(argv) + result = {"messages": [], "print": False} + separator = False + i = 0 + while i < len(args): + arg = args[i] + if separator: + result["messages"].append(arg) + i += 1 + continue + eq_index = -1 + if arg.startswith("--") and "=" in arg: + name, _, value = arg.partition("=") + arg = name + args.insert(i + 1, value) + eq_index = i + 1 + start = i + if arg in _STRING_FLAGS: + if i + 1 < len(args): + i += 1 + result[arg[2:]] = args[i] + elif arg in _OPTIONAL_FLAGS: + nxt = args[i + 1] if i + 1 < len(args) else None + if nxt is not None and not nxt.startswith("-") and nxt != "": + i += 1 + result[arg.lstrip("-")] = args[i] + elif arg in ("--print", "-p"): + result["print"] = True + elif arg == "--": + separator = True + elif arg.startswith("@") or not arg.startswith("-") or arg == "-": + if not arg.startswith("@"): + result["messages"].append(arg) + if eq_index != -1 and i == start: + del args[eq_index] + i += 1 + return result