feat: add OMO Slim as an optional orchestration plugin#722
Conversation
Mutually exclusive with enable-omo. Validates preset against the openai/opencode-go allowlist and fails fast when both plugins are enabled.
installOmoSlim runs the pinned slim installer with the validated preset. buildCIConfig gains a slim mode that registers the slim plugin, pins default_agent to orchestrator, guards against dual-plugin configs, and version-gates orchestrator support.
A workflow_dispatch-gated job exercises enable-omo-slim end to end and asserts the slim config assembly. README documents the new inputs and the mutual-exclusivity contract with oMo.
Reject unverified slim versions before running the installer, fall back to a non-slim config when the slim install fails so a missing plugin is never pinned as default_agent, strip the legacy plugins key in slim mode, and source the default version from the shared constant.
fro-bot
left a comment
There was a problem hiding this comment.
Verdict: PASS
Solid, well-tested addition. The slim-mode path is correctly isolated from the existing oMo and disabled paths, mutual-exclusion is enforced early, and the fail-closed degradation (failed install → non-slim build config rather than a pinned-but-missing orchestrator) is handled cleanly via the slimReady reassignment in setup.ts:181. 126 tests in the touched suites pass locally. Nothing here blocks merge.
Blocking issues
None
Non-blocking concerns
-
verifyOmoSlimInstallationis dead code with a latent bug (src/services/setup/omo-slim.ts:58-72). It is never called fromsetup.ts(only from its own test). Worse, if it is wired up later it will always returnfalse:execAdapter.exec('ls', ['-la', '~/.config/opencode/oh-my-opencode-slim.json'])passes~as a literal argv element — no shell tilde expansion happens with arg arrays, solslooks for a literal./~/.config/...path. Either remove the function until it is needed, or expand the home dir explicitly (e.g.join(homedir(), '.config/opencode/oh-my-opencode-slim.json')) and assert on exit code rather than substring-matching'No such file'(which is locale/ls-implementation dependent). The verification gap is acceptable for now because the slim install already gates on installer exit code. -
Redundant double version gate. The R19 verified-version check runs twice: once eagerly in
setup.ts:142(isOmoSlimVersionVerified) and again insidebuildCIConfig(ci-config.ts:138). SinceomoSlimVersionis currently hardwired toDEFAULT_OMO_SLIM_VERSIONinbootstrap.ts:51(there is noomo-slim-versionaction input despite the plan referencing one), neither gate can fail at runtime today — both are effectively dead in production. This is fine as forward-looking defense, but the two error strings differ slightly (setup.tshardcodesknown-good: 1.1.1whileci-config.tsderives it from the list); deriving both fromOMO_SLIM_ORCHESTRATOR_VERIFIED_VERSIONSwould prevent them drifting when Renovate bumps the version. -
oMo silently stripped in slim mode rather than rejected. The dual-plugin guard (
ci-config.ts:130-135) only fires when a single user-supplied config contains both plugin families. Whenenable-omo-slim: trueand the user config already lists anoh-my-openagentplugin, slim mode strips it viastripOmoPluginswith no warning. The CLI-input mutual-exclusion (inputs.ts:308) covers the common case, but a config-injected oMo plugin disappears silently. Consider alogger.warningwhenstripOmoPluginsremoves entries in slim mode, mirroring the disabled-moderewrittenFieldswarning. -
Version-detection regex is best-effort.
omo-slim.ts:46parses the installed version from stdout via/oh-my-opencode-slim@(\d+\.\d+\.\d+)/iand silently falls back to the requested version on no match. That is reasonable, but the reportedversioninOmoInstallResultis then advisory only — fine since nothing downstream depends on the detected value (the config usesomoSlimVersion, the input).
Missing tests
None that block. Coverage is strong: preset argv assembly (both presets), exit-code failure, exception path, no --skip-auth, slim-mode config assembly, unverified-version rejection in both buildCIConfig and the early setup.ts gate (fix B), mutual-exclusion in inputs.test.ts, and the failed-install degradation. Two gaps worth a follow-up (non-blocking): (a) no test exercises buildCIConfig slim mode when the user config already contains an oh-my-openagent plugin (the silent-strip path in concern #3); (b) no test for the verifyOmoSlimInstallation tilde-path bug — current tests mock getExecOutput and substring-match, so they pass without catching the real-world expansion failure.
Risk assessment (LOW): All new behavior is gated behind enable-omo-slim (default false) and is mutually exclusive with the existing oMo path, so the blast radius for existing users is zero. The shared touch point — buildCIConfig and the setup.ts orchestration — adds new branches without altering the enabled/disabled oMo paths (verified by the unchanged existing assertions). No security exposure: no new secret handling, no --skip-auth, no telemetry env vars, and the installer runs ephemerally via bunx with a pinned, version-gated specifier. Main residual risk is operational (the live bunx install in the workflow_dispatch-only smoke job), not a regression risk to merged code.
Run Summary
| Field | Value |
|---|---|
| Event | pull_request |
| Repository | fro-bot/agent |
| Run ID | 26736557823 |
| Cache | hit |
| Session | ses_17e62e07bffeCDM1nnfBz6x2oQ |
Slim config assembly is covered by unit tests; a workflow_dispatch-only job only adds a perpetually-skipped check to every PR. Dispatch fro-bot.yaml for an on-demand live install smoke when needed.
fro-bot
left a comment
There was a problem hiding this comment.
Verdict: PASS
Re-review after the latest push. The only change since my prior review (commit 3f63c17) is the removal of the workflow_dispatch-gated test-action-omo-slim CI job. The implementation code (omo-slim.ts, ci-config.ts, setup.ts, inputs.ts) is unchanged. Dropping the dispatch-only job is a sound call: it was perpetually skipped on every PR (so it added a noisy required-check candidate without ever running), and slim config assembly is fully covered by unit tests. On-demand live install smoke remains available via fro-bot.yaml dispatch. This directly resolves the lone operational note from my prior review. 172 tests across the touched suites pass locally.
Blocking issues
None
Non-blocking concerns
The four items from my prior review remain open and remain non-blocking. Carried forward for visibility:
-
verifyOmoSlimInstallationis dead code with a latent bug (src/services/setup/omo-slim.ts:58-72). Never called fromsetup.ts. If wired up later it will always returnfalse:'~'is passed as a literal argv element tols(no shell tilde expansion with arg arrays). Either remove it until needed, or usejoin(homedir(), ...)and assert on exit code rather than substring-matching'No such file'. -
Redundant double version gate (
setup.ts:142andci-config.ts:138). Both currently unreachable sinceomoSlimVersionis hardwired toDEFAULT_OMO_SLIM_VERSIONinbootstrap.ts:51(noomo-slim-versioninput). The two error strings differ (setup.tshardcodesknown-good: 1.1.1;ci-config.tsderives from the list) — derive both fromOMO_SLIM_ORCHESTRATOR_VERIFIED_VERSIONSto prevent drift on Renovate bumps. -
oMo silently stripped in slim mode rather than rejected (
ci-config.ts:130-135). The dual-plugin guard only fires when one user config contains both families. A config-injectedoh-my-openagentplugin is stripped silently in slim mode. Consider alogger.warningmirroring the disabled-moderewrittenFieldswarning. -
Version-detection regex is best-effort (
omo-slim.ts:46). Falls back to the requested version on no match; advisory only since downstream uses the input version. Acceptable.
Missing tests
None that block. Coverage remains strong (preset argv for both presets, exit-code failure, exception path, no --skip-auth, slim-mode config assembly, unverified-version rejection in both gate locations, mutual-exclusion, failed-install degradation). Two follow-up gaps (non-blocking, unchanged): (a) no test for buildCIConfig slim mode when the user config already contains an oh-my-openagent plugin (the silent-strip path in concern #3); (b) no test catching the verifyOmoSlimInstallation tilde-path bug (current tests mock getExecOutput and substring-match, so they pass without exercising real expansion).
Risk assessment (LOW): Removing a perpetually-skipped, workflow_dispatch-only job has zero regression risk to merged code or to PR gating — it only deletes a check that never executed on PRs. All slim behavior remains gated behind enable-omo-slim (default false) and mutually exclusive with oMo, so blast radius for existing users is zero. No security exposure: no new secret handling, no --skip-auth, no telemetry env vars, ephemeral pinned bunx install. The only loss is the automated end-to-end live-install assertion, which is mitigated by full unit coverage of config assembly plus the retained on-demand fro-bot.yaml dispatch path.
Run Summary
| Field | Value |
|---|---|
| Event | pull_request |
| Repository | fro-bot/agent |
| Run ID | 26736854151 |
| Cache | hit |
| Session | ses_17e62e07bffeCDM1nnfBz6x2oQ |
…l-dependency # Conflicts: # dist/artifact-B_zc1jSr.js # dist/artifact-C7LCxgbI.js # dist/artifact-CpTI6Kyo.js # dist/main.js # dist/post.js
fro-bot
left a comment
There was a problem hiding this comment.
Verdict: PASS
Approving. This is the third pass on PR #722; my two prior reviews (PASS, comment-only) covered the implementation in full. The only change since then is merge commit 3bbf5ee pulling origin/main into the branch. That merge resolved conflicts only in dist/ build artifacts (dist/main.js, dist/post.js, the renamed artifact-*.js) — no source-level slim files changed. Verified at HEAD: omo-slim.ts, ci-config.ts, setup.ts, inputs.ts, and bootstrap.ts:51 (omoSlimVersion: DEFAULT_OMO_SLIM_VERSION) are identical to the previously reviewed state. Full test run is green: 425 tests pass across the setup, harness-config, and agent suites. No new regression surface introduced by the merge.
Blocking issues
None
Non-blocking concerns
Unchanged from prior reviews — all four remain open and non-blocking. Carried forward for visibility:
-
verifyOmoSlimInstallationis dead code with a latent bug (src/services/setup/omo-slim.ts:58-72). Never called fromsetup.ts. If wired up later it always returnsfalse:'~'is passed as a literal argv element tols(no shell tilde expansion with arg arrays). Remove until needed, or usejoin(homedir(), ...)and assert on exit code rather than substring-matching'No such file'. -
Redundant double version gate (
setup.ts:142andci-config.ts:138). Both unreachable today sinceomoSlimVersionis hardwired toDEFAULT_OMO_SLIM_VERSION(noomo-slim-versioninput). Error strings differ —setup.tshardcodesknown-good: 1.1.1whileci-config.tsderives from the list. Derive both fromOMO_SLIM_ORCHESTRATOR_VERIFIED_VERSIONSto prevent drift on Renovate bumps. -
oMo silently stripped in slim mode (
ci-config.ts:130-135). The dual-plugin guard only fires when one user config contains both families; a config-injectedoh-my-openagentplugin is stripped silently in slim mode. Consider alogger.warningmirroring the disabled-moderewrittenFieldswarning. -
Version-detection regex is best-effort (
omo-slim.ts:46). Falls back to the requested version on no match; advisory only since downstream uses the input version. Acceptable.
Missing tests
None that block. Coverage remains strong (preset argv for both presets, exit-code failure, exception path, no --skip-auth, slim-mode config assembly, unverified-version rejection in both gate locations, mutual-exclusion, failed-install degradation). Two non-blocking follow-up gaps, unchanged: (a) no test for buildCIConfig slim mode when the user config already contains an oh-my-openagent plugin (the silent-strip path in concern #3); (b) no test catching the verifyOmoSlimInstallation tilde-path bug (current tests mock getExecOutput and substring-match).
Risk assessment (LOW): The merge from origin/main touched only dist/ artifacts at the conflict level and introduced no source-level slim changes, so it adds no new regression surface. All slim behavior remains gated behind enable-omo-slim (default false) and mutually exclusive with oMo — blast radius for existing users is zero. No security exposure: no new secret handling, no --skip-auth, no telemetry env vars, ephemeral pinned bunx install with a version-gated specifier. 425 tests green confirm the merge did not disturb existing behavior. One verification worth the author's glance before merge: confirm dist/ was rebuilt from the post-merge source (the conflict resolution should reflect current src/), since dist/ is the shipped artifact for this action.
Run Summary
| Field | Value |
|---|---|
| Event | pull_request |
| Repository | fro-bot/agent |
| Run ID | 26750642037 |
| Cache | hit |
| Session | ses_17e62e07bffeCDM1nnfBz6x2oQ |
Summary
Adds OMO Slim (
oh-my-opencode-slim) as an optional orchestration plugin, alongside the existing Oh My OpenAgent (oMo) integration. OMO Slim is selected with a single preset name rather than oMo's provider list.What's included
enable-omo-slimaction input (defaultfalse), mutually exclusive withenable-omo— enabling both fails fast.omo-slim-presetinput (openai|opencode-go, defaultopenai), validated to the allowlist before reaching the installer.DEFAULT_OMO_SLIM_VERSIONconstant with Renovate tracking, mirroring the oMo/Bun/Systematic version-pinning pattern.bunx oh-my-opencode-slim@<version> install --no-tui --reset --preset=<preset>and assembles the OpenCode config in slim mode: registers the slim plugin, pinsdefault_agenttoorchestrator, and keeps@fro.bot/systematic.workflow_dispatch-gated CI job exercises slim mode end to end and asserts the generated config.The gateway environment variable for selecting slim presets in workspace containers is deferred until the workspace image runs OpenCode.