feat(provider): add OMP CLI provider (omp_cli)

[bigyee66]

feat(provider): add OMP CLI provider (omp_cli)

Adds a new provider adapter for the OMP CLI (omp binary), following the
existing adapter pattern. OMP is integrated as a generic TUI agent. Work item
d7348a1e-f6d6-4a24-96a9-edab9b74d8ff.

Changes (purely additive — no existing provider behaviour touched)

• providers/omp_cli.py — OmpCliProvider implementing BaseProvider:
  • env-overridable regexes (CAO_OMP_*_REGEX) so status detection / response
    extraction can be recalibrated against real omp output without a code
    change (same pattern Hermes uses).
  • binary resolved via shutil.which("omp"); optional --model override
    (constructor or agent profile).
  • fresh-spawn vs delivered-turn (IDLE vs COMPLETED) distinguished via an
    internal turn counter (mark_input_received), since the buffer alone can't
    tell them apart (mirrors cursor_cli).
  • status priority: empty→UNKNOWN, WAITING_USER_ANSWER, ERROR, PROCESSING
    (with a position guard against stale scrollback), COMPLETED/IDLE, UNKNOWN.
• providers/manager.py — register the OMP_CLI construction branch.
• services/install_service.py — explicit context-file-only placeholder
  branch (OMP has no native agent-config format yet; agent_file stays None).
• services/terminal_service.py — documents OMP's deliberate exclusion from
  both capability sets (skills via context file; native tool vocabulary not yet
  characterised).
• api/main.py — provider_binaries gains omp_cli -> omp.
• cli/commands/launch.py — add omp_cli to PROVIDERS_REQUIRING_WORKSPACE_ACCESS.
• models/provider.py — ProviderType.OMP_CLI enum (PROVIDERS /
  valid_providers auto-derived).
• test/providers/test_omp_cli_unit.py — status four-states + extraction +
  lifecycle (36 tests incl. manager).
• test/providers/test_provider_manager_unit.py — omp_cli registration
  assertion.
• test/e2e/conftest.py — require_omp fixture.
• docs/omp-cli.md — per-provider documentation.

Validation

• pytest test/providers/test_omp_cli_unit.py test/providers/test_provider_manager_unit.py → 36 passed.
• python -c import + manager build smoke test passes; PROVIDERS and
  PROVIDERS_REQUIRING_WORKSPACE_ACCESS both contain omp_cli.

Known follow-ups

Default status/extraction regexes are placeholders calibrated against
representative TUI agents. Calibration against real omp output is owed and is
designed to be done via env vars (no source edit) during test_e2e/test_unit.

Note on fork

The upstream awslabs/cli-agent-orchestrator is read-only for the contributor,
so this PR targets awslabs:main from the fork bigyee66:harness/d7348a1e.

[haofeif]

@bigyee66 Thanks for your contribution!

Can you please make sure that the new provider pass the examples/assign test ?
Supervisor agent is able to parallel assign 3x tasks to data analysts, and sequential handoff to report generator. Data analyst are ables to complete the jobs and successfully send message back to supervisor agent. supervisor agent is able to complete the reporting. Supervisor agent is not supposed to complete the jobs itself.

You can also refer to the skill https://github.com/awslabs/cli-agent-orchestrator/tree/main/skills/cao-provider for the new provider.

Also please address the copilot's reviews, and fix the CI errors and Codecov Report. Much appreciated!

[codecov-commenter]

Codecov Report

❌ Patch coverage is 82.85714% with 24 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@32db4e1). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/cli_agent_orchestrator/providers/omp_cli.py	82.83%	23 Missing ⚠️
...cli_agent_orchestrator/services/install_service.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #328   +/-   ##
=======================================
  Coverage        ?   87.48%           
=======================================
  Files           ?       95           
  Lines           ?    11590           
  Branches        ?        0           
=======================================
  Hits            ?    10139           
  Misses          ?     1451           
  Partials        ?        0           

Flag	Coverage Δ
unittests	87.48% <82.85%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Adds a new omp_cli provider integration (OMP CLI via omp binary) following the existing provider adapter pattern, plus registration, API/CLI wiring, tests, and documentation.

Changes:

• Introduce OmpCliProvider with env-overridable regex-based status detection and response extraction.
• Register omp_cli across provider manager, provider enum, API provider listing, and CLI launch workspace-access gating.
• Add unit tests/e2e fixture and provider documentation for OMP.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/cli_agent_orchestrator/providers/omp_cli.py	New provider implementation (launch, status detection, extraction, lifecycle).
src/cli_agent_orchestrator/providers/manager.py	Registers OMP_CLI provider creation branch.
src/cli_agent_orchestrator/models/provider.py	Adds ProviderType.OMP_CLI.
src/cli_agent_orchestrator/api/main.py	Adds omp_cli -> omp to provider_binaries for /agents/providers.
src/cli_agent_orchestrator/cli/commands/launch.py	Adds omp_cli to PROVIDERS_REQUIRING_WORKSPACE_ACCESS.
src/cli_agent_orchestrator/services/install_service.py	Adds explicit no-op install branch for omp_cli (context-file-only).
src/cli_agent_orchestrator/services/terminal_service.py	Documents why OMP is excluded from capability sets.
test/providers/test_omp_cli_unit.py	New unit test suite for OMP provider status/extraction/launch behavior.
test/providers/test_provider_manager_unit.py	Adds assertion that manager constructs/stores OmpCliProvider.
test/api/test_api_endpoints.py	Updates provider list expectation to include omp_cli.
test/e2e/conftest.py	Adds require_omp fixture to skip e2e tests if omp missing.
docs/omp-cli.md	New end-user/provider documentation for omp_cli.
docs/PLAN-omp-cli.md	New implementation plan document for the OMP provider.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gutosantos82]

PR Review: #328 — feat(provider): add OMP CLI provider (omp_cli)

Summary

Adds an OmpCliProvider adapter (omp binary) as a generic TUI agent, following the
established Hermes/cursor_cli pattern. The change is genuinely additive — no existing
provider behaviour is touched, command construction is safe, and the enum/manager/API
registration wiring is consistent. Three things hold up a merge: a correctness bug in
ERROR detection that will misfire on ordinary turn output, a security gap where
restricted tool policies are silently treated as unrestricted with no operator warning, and
missing README/CHANGELOG entries required by the CAO new-provider checklist.

Reviewer coverage: all five angles (correctness, security, tests, conventions, consistency)
reported.

Blocking (must fix before merge)

• [correctness] providers/omp_cli.py:326 (ERROR_PATTERN) / :538 (usage) — ERROR detection
  has no stale-buffer/position guard, unlike WAITING (:531) and PROCESSING (:546). The
  pattern is matched against the last-30-line tail with IGNORECASE|MULTILINE and wins over
  IDLE/COMPLETED. Any completed turn whose output contains a line starting Error: / ERROR:
  (test output, tool logs, the agent quoting an error) pins the terminal to ERROR even though
  omp> is present at the bottom. Concrete: ● run tests\nError: 3 tests failed\n…\nomp> →
  returns ERROR, should be COMPLETED — the supervisor concludes the worker crashed. Fix: apply
  the same "no idle prompt follows the last match" guard that WAITING/PROCESSING already use.
  (providers/ path — weighted up to blocking.)

• [conventions] README.md (provider table ~:120-131, "Valid values" lists ~:167 and ~:257) —
  the new omp_cli provider is not added to the README provider table or to either valid-values
  list. The CAO new-provider checklist requires these. Add the table row (docs link + auth) and
  append omp_cli to both lists.

• [conventions] CHANGELOG.md (:7-11, [Unreleased] > Added) — a new user-facing provider
  ships with no CHANGELOG entry (the section only lists Hermes). Add an entry, e.g.
  "add OMP CLI provider (omp_cli)".

Important (should fix)

• [security] services/terminal_service.py:127+ (exclusion) / ~:259 (warning site) — OMP is
  excluded from SOFT_ENFORCEMENT_PROVIDERS, so the launch-time warning that fires for
  soft-enforcement providers ("cannot enforce tool restrictions… treat as unrestricted") does
  not fire for omp_cli. Combined with no tool_mapping entry and no native blocking flag,
  a profile requesting restricted allowed_tools on OMP is silently treated as fully
  unrestricted — strictly weaker than the existing model, with none of the guidance
  kimi_cli/codex get. Fix: add ProviderType.OMP_CLI.value to SOFT_ENFORCEMENT_PROVIDERS so
  the warning fires, or gate OMP behind an explicit acknowledgment. (services/ path — weighted up.)

• [security] false "restrictions reach the agent via the context file" claim — repeated in
  docs/omp-cli.md:135-136, install_service.py:362-369, omp_cli.py:399 & :463-466. _write_context_file
  writes only the profile's role markdown; it never serializes allowed_tools. So the restriction
  reaches the agent through no channel at all (not even advisory) unless the author hand-types
  it into the role body. This can lull an operator into assigning a restricted role believing it is
  conveyed. Fix: correct the wording ("restrictions are NOT conveyed to OMP") or actually inject
  allowed_tools into the context/prompt. (Pairs with the enforcement-gap finding above.)

• [correctness] providers/omp_cli.py:600 (hard-coded header strip) — _extract_response
  anchors on the env-overridable ASSISTANT_HEADER_PATTERN (:331) but strips the leading marker
  with a hard-coded literal ^\s*(?:Assistant|OMP|omp)[:,]\s*. If an operator overrides the
  header env var to OMP's real marker (the entire point of the env-overridable design), extraction
  anchors correctly but does not strip the custom marker — the raw header leaks into every extracted
  response. Fix: reuse ASSISTANT_HEADER_PATTERN for the strip.

• [correctness] providers/omp_cli.py:313-317 (PROCESSING_PATTERN) — defaults match bare words
  Thinking|Working|interrupt|cancel under IGNORECASE, so ordinary prose ("I'm working on it",
  "cancel the order") matches. In get_status the position guard saves it, but _extract_response
  (:596) drops any response line containing these words — legitimate content is silently deleted
  from the extracted message. Tighten the defaults to require a spinner glyph or trailing ellipsis.

• [consistency/conventions] docs/PLAN-omp-cli.md (new, 70 lines) — a Chinese-language internal
  planning/handoff doc (requirement id, risk tables, plan→develop notes) shipped under the published
  docs/ tree, breaking the "one English docs/<provider>-cli.md" convention (already satisfied by
  docs/omp-cli.md). It also contains the non-inclusive term 白名单 ("whitelist") at :11. Remove it from
  the PR (fold anything durable into omp-cli.md); this also clears the inclusive-language hit.

• [consistency] PR description — "36 tests" vs. ~19 shipped — the body claims 36 passing tests, but
  only ~19 new test functions ship (test_omp_cli_unit.py: 18; test_provider_manager_unit.py: 1; the API
  test is a one-line count bump, not a new test). Correct the count or add the missing tests.

• [tests] placeholder-regex false confidence — test_omp_cli_unit.py:1-9 and omp_cli.py:246 admit every
  status/extraction regex is a placeholder pending calibration; the fixtures were written to match those
  placeholders, so the tests validate branch logic (priority order, guards, turn counter) but give zero
  signal about real omp output. Green CI here does not mean "works against omp." Merge only with an explicit
  "not production-ready until regexes calibrated" note (the PR body's Known-follow-ups section partly covers this).

• [tests] get_status priority ordering + WAITING guard untested — each status is asserted only in
  isolation; no test exercises coexisting triggers (e.g. approval dialog + Error: line, which the code
  resolves WAITING-before-ERROR), so the documented 7-level priority (omp_cli.py:502-511) is unverified. The
  WAITING position guard's "dialog followed by idle → not WAITING" branch has no test, even though the
  analogous PROCESSING guard does. Note: this gap is exactly why the blocking unguarded-ERROR bug slipped
  through — test_error_detected uses an error buffer with no trailing omp>, so it passes while masking the
  misfire. Add priority/coexistence tests, including one that would catch the ERROR guard.

• [tests] initialize() entirely untested — omp_cli.py:470-495 (binary probe + send_keys + wait_until_status

  • 120s timeout) has no coverage. Also untested: model-override-via-agent-profile (only explicit model= is
    covered; the load_agent_profile branch + its best-effort except at :444-450 are not).

Nits (optional)

• [correctness] providers/omp_cli.py:525 — bottom_output is assigned but never used. Dead variable.
• [correctness/conventions] providers/omp_cli.py:348 (_is_chrome_line) — defined but never called;
  _extract_response inlines its own chrome checks. Dead code copied from the Hermes template — remove
  it or wire it into extraction.
• [conventions] providers/omp_cli.py:457 & :463 — the "skill_prompt ignored" / "no native
  tool-restriction flag" cases log at logger.info; the Hermes precedent uses logger.warning for the
  identical situations. Align for cross-provider consistency.
• [correctness/consistency] docstring & docs vs. code — ERROR_PATTERN (:326), SEPARATOR_PATTERN (:337),
  and ANSI_CODE_PATTERN (:301) are not env-overridable, yet the module docstring and docs/omp-cli.md:148
  ("All detection patterns are overridable") imply they are. Reword to "all classification-prompt patterns",
  and consider making ERROR overridable for parity.
• [security] providers/omp_cli.py:306-333 & usage — env-override regexes are read at import and used
  directly with no re.compile/try-except. A malformed override raises re.error at status-poll time
  (crashes polling); a pathological pattern enables ReDoS over the agent-influenced buffer. Compile once at
  load with a fallback-and-log.
• [correctness] providers/omp_cli.py:611 — extraction runs strip_terminal_escapes (normalizes \r→\n)
  then _strip_ansi; cursor_cli deliberately avoids strip_terminal_escapes in extraction because \r→\n
  splits redraw frames, and the second _strip_ansi is redundant.
• [conventions] services/install_service.py:629 — the new elif … OMP_CLI: … pass branch is a functional
  no-op (the default context-file-only path already handles OMP). Acceptable — it's documented as an
  intentional extension point.
• [security] providers/omp_cli.py:437 (shutil.which) — resolves the first omp on PATH (PATH-hijack
  surface). Matches every existing provider and is called out in the docs — pre-existing pattern, not
  introduced.

Prior feedback (already raised — not restating)

No overlap — all findings below are net-new relative to existing human comments.

Tests

No dedicated tests review returned — this is a partial assessment reconstructed from the correctness and
consistency reviewers. The suite covers status four-states, extraction, lifecycle, manager registration, an
require_omp e2e fixture, and the API provider-count bump (10→11) — reasonable breadth for an additive
provider. Notable gaps worth adding: (1) no test for the ERROR-vs-idle case — the existing test_error_detected
uses an error buffer with no trailing omp>, so it passes while masking the blocking ERROR-guard bug; (2) no
test exercising the env-override path (the headline feature) — defaults only; (3) no test asserting the
hard-coded header-strip works against an overridden marker. The "36 tests" claim in the PR body is inaccurate
(~19 shipped).

Verdict

Request changes — one correctness bug (unguarded ERROR detection) and a silent tool-enforcement gap in core
paths, plus the required README/CHANGELOG checklist items, should land before merge. The rest are quick doc/wording
and dead-code cleanups.