fix(install): honor frontmatter provider with flag > frontmatter > default precedence (fixes #414)

[call-me-ram]

Summary

cao install now resolves the target provider with the same precedence the launch/assign/handoff paths already use: explicit --provider flag > profile frontmatter provider: > DEFAULT_PROVIDER. Fixes #414. Thanks @chaogebaba for the report and for validating the precedence approach.

Root cause

The Click --provider option defaulted to DEFAULT_PROVIDER (kiro_cli) and install_service.install_agent branched on that argument alone, so a profile declaring provider: codex installed silently as a kiro_cli agent (exit 0, no warning). Install was the only entry point ignoring frontmatter — launch/assign/handoff/API resolve it via resolve_provider().

Fix

• CLI --provider defaults to None; help text documents the precedence; the install summary echoes the provider the service actually resolved.
• install_agent accepts provider=None; an explicit bad provider still fails fast before any URL download or env-file mutation; a bogus frontmatter provider logs a warning (same wording as resolve_provider) and falls back to the default. InstallResult gains a provider field with the resolved winner (so the CLI summary can't print None; backward-compatible, Optional).
• InstallAgentProfileRequest.provider and the ops-MCP install_profile tool default to None (provider omitted from the request body when not explicit, same pattern as _launch_session_impl), so all three entry points behave identically. Built-in store profiles carry no frontmatter provider and keep today's default-provider behavior.

Tests

• flag wins over frontmatter; frontmatter wins when the flag is absent; both absent → DEFAULT_PROVIDER
• invalid frontmatter provider → warning logged + default used
• explicit invalid --provider → fail-fast before download (existing behavior preserved)
• built-in profile without frontmatter provider keeps the default
• API and ops-MCP entry points forward None for identical frontmatter resolution
• Full suite: 4021 passed; the one failure (test_settings_service.py::TestGetServerSettings) reproduces on a clean upstream/main checkout and is unrelated. black/isort clean.

Fixes #414

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@5270588). Learn more about missing BASE report.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #431   +/-   ##
=======================================
  Coverage        ?   88.41%           
=======================================
  Files           ?      137           
  Lines           ?    16385           
  Branches        ?        0           
=======================================
  Hits            ?    14486           
  Misses          ?     1899           
  Partials        ?        0           

Flag	Coverage Δ
unittests	88.41% <100.00%> (?)

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.

[gutosantos82]

PR #431 — fix(install): honor frontmatter provider with flag > frontmatter > default precedence

• Author: call-me-ram · Base: main · Head: 8d88a48
• Size: +266 / −14 across 9 files (5 src incl. CHANGELOG, 4 test)
• Linked issue: #414 (confirmed open; PR implements exactly the fix outline proposed and validated there)
• Mergeable: MERGEABLE · CI: pending/at head

What it does

Root cause (verified against upstream/main): cli/commands/install.py gave --provider
a Click default of DEFAULT_PROVIDER, and install_service.install_agent branched on that
argument alone — the parsed profile's provider frontmatter was never read on the install
path, unlike launch/assign/handoff which resolve it via resolve_provider(). A profile
declaring provider: codex installed silently as a kiro_cli agent with exit 0.

The fix, per entry point:

1. CLI (install.py) — --provider default becomes None; help text documents the
   precedence; the final summary line echoes result.provider (the winner the service
   actually resolved) with a safe fallback chain for older mocks.
2. Service (install_service.py) — install_agent(provider: Optional[str] = None).
   Explicit providers are still validated up front (fail-fast before URL download or
   env-file mutation — verified by test with mock_get.assert_not_called()). When None,
   after the profile parse: valid frontmatter provider wins; invalid frontmatter logs a
   warning (wording matches resolve_provider() verbatim — checked side by side) and falls
   back to DEFAULT_PROVIDER. InstallResult gains an Optional[str] provider field
   (backward-compatible).
3. REST API (api/main.py) — InstallAgentProfileRequest.provider: Optional[str] = None,
   forwarded as-is to the service.
4. ops-MCP (ops_mcp_server/server.py) — install_profile defaults provider=None and
   omits the key from the request body when not explicit (same pattern as _launch_session_impl).
   The now-unused DEFAULT_PROVIDER import is removed (verified no remaining references in
   that file).

Verification performed

• Read the full diff (489 lines, all 9 files) and the PR-head versions of the four
  touched source files.
• Ordering check: nothing reads provider between the up-front explicit validation and
  the post-parse frontmatter resolution; resolution happens before any provider-config
  materialization or agent-file write. No stale-None path exists.
• Caller audit: grep across src/ confirms install_agent() is called only from the CLI
  and the API endpoint — both updated. In-session handoff/assign use resolve_provider,
  untouched.
• Import check: Optional and DEFAULT_PROVIDER are properly imported in the CLI module;
  no leftover DEFAULT_PROVIDER usage in the ops-MCP server after the import removal.
• Ran the tests at the PR head (worktree + repo venv): all 4 touched test files —
  104 passed, 5 failed; the same 5 failures (test_install_from_local_store_writes_copilot_config
  • 4 TestInstallSkillCatalogBaking tests) reproduce identically on upstream/main, so
    they are pre-existing/environmental, not introduced by this PR. All 11 new tests pass.
• Warning-wording claim verified: the frontmatter-invalid warning is byte-identical in
  format to resolve_provider()'s.

Test coverage assessment

The precedence matrix is fully covered: flag>frontmatter, frontmatter-when-no-flag,
double-absent→default, invalid frontmatter→warn+default (with caplog assertion), explicit
invalid→fail-fast-before-download, built-in profile default, plus entry-point forwarding
tests for CLI (None passed, resolved provider echoed), API (provider=None forwarded),
and ops-MCP (key omitted from body). One help-text test is trivial but harmless.

Nits (none blocking)

• The CLI echo fallback result.provider or provider or DEFAULT_PROVIDER is slightly
  defensive (the service always sets provider on success now), but the comment explains
  it's for older mocks — acceptable.
• frontmatter provider validation intentionally happens after a URL download (warn+fallback
  is not fail); this matches the issue's agreed design.

Publish-guard assessment

Touches ops_mcp_server/server.py and services/install_service.py — the sensitive-path
regex may HOLD an auto-approve (core service surface). The change does not touch auth,
credentials, tool-permission/--yolo, providers/ status detection, or release/CI files.
Diff is 280 lines / 9 files — under the >400-line gate. Behavior change is opt-in-shaped:
callers passing an explicit provider are unaffected; only the previously-silently-wrong
default path changes, in the direction users already expected. Low blast radius.

[Copilot]

Pull request overview

Aligns cao install with the provider-resolution precedence already used by launch/assign/handoff paths: explicit --provider flag > profile frontmatter provider: > DEFAULT_PROVIDER, preventing silent installs to the wrong provider.

Changes:

• Make CLI --provider default to None, document precedence in help, and echo the resolved provider in output.
• Update install_service.install_agent() to resolve provider using flag/frontmatter/default precedence and return the resolved provider in InstallResult.
• Propagate provider=None behavior through API and ops-MCP entry points; add targeted tests and a changelog entry.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/services/test_install_service.py	Adds precedence/validation tests for provider resolution in install_service.
test/ops_mcp_server/test_server.py	Ensures ops-MCP install_profile omits provider when not explicitly provided.
test/cli/commands/test_install.py	Verifies CLI passes None when --provider is omitted and echoes resolved provider.
test/api/test_api_profiles.py	Verifies API forwards provider=None when omitted so service resolves frontmatter.
src/cli_agent_orchestrator/services/install_service.py	Implements provider resolution precedence and returns resolved provider in InstallResult.
src/cli_agent_orchestrator/ops_mcp_server/server.py	Changes MCP tool default provider behavior to omit unless explicitly set.
src/cli_agent_orchestrator/cli/commands/install.py	Sets --provider default to None, updates help text, prints resolved provider.
src/cli_agent_orchestrator/api/main.py	Makes request provider optional to enable frontmatter resolution in install path.
CHANGELOG.md	Adds Unreleased “Fixed” entry for honoring frontmatter provider on install.

---

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