Skip to content

feat(providers): add omp, a keyless provider backed by a local Oh My Pi install - #2194

Open
oldschoola wants to merge 6 commits into
repowise-dev:mainfrom
oldschoola:feat/omp-provider
Open

feat(providers): add omp, a keyless provider backed by a local Oh My Pi install#2194
oldschoola wants to merge 6 commits into
repowise-dev:mainfrom
oldschoola:feat/omp-provider

Conversation

@oldschoola

Copy link
Copy Markdown
Contributor

What

Adds omp, a keyless LLM provider backed by a local Oh My Pi install.

repowise init --provider omp --yes    # no API key anywhere

Oh My Pi already holds credentials for whatever it is configured against — an OAuth subscription, a provider account, or a key in its own config — so repowise never sees one. It joins claude_cli, codex_cli and opencode in KEYLESS_PROVIDERS, and omp/* is priced at $0.00 because Oh My Pi bills against its own account rather than a repowise key. The cost Oh My Pi reports is still recorded in the usage record for auditing.

How

Generation runs:

omp -p --mode json --no-session --no-extensions --no-skills --no-rules --no-tools \
    --config <scratch>/omp-config.yml --system-prompt <scratch>/system-prompt.md

The page prompt goes on stdin (print mode reads non-TTY stdin as the initial message), and the JSONL event stream on stdout is parsed for the assistant message_end — which carries the answer text, token usage, stop reason, and the model actually routed to. A non-zero exit is a failure, with Oh My Pi's own reason on stderr.

Oh My Pi also exposes a bidirectional JSON-RPC transport (--mode rpc). It is the richer surface, but nothing it adds — request/response correlation, mid-turn steering, host-owned tools — applies to a one-shot completion, so print mode keeps this provider the same shape as its three siblings (611 lines, against 451/470/522) instead of carrying a bespoke framing client that would have to track the RPC protocol's own versioning.

Isolation

The CLI otherwise brings a whole agent session with it, so each call runs in a temporary scratch directory (absent from REPO_PATH_PROVIDERS for the same reason claude_cli is — repo context files would bias every page and cost tokens), with tools, extensions, skills and rules off, and --no-session so a 68-page run leaves no resumable sessions behind.

A generated --config overlay turns off four global settings a docs run has no business inheriting:

Key Why
memory, autolearn add write-capable tools (learn, manage_skill) — a docs run must not write to the user's memory store
advisor otherwise every finished page spends a second model call reviewing it
tools.approvalMode: always-ask MCP servers from the user's own config still load; an inherited yolo would let the model run a mutating MCP tool against their machine unprompted

That last one is measured, not assumed. The enum is a permissiveness ladder rather than a gate selector — write auto-approves writes — so always-ask is the only value that gates anything. Headless has no UI to satisfy it, which is exactly what makes it safe: Oh My Pi fails the call with Tool "x" requires approval but no interactive UI available, the model gets a tool error, and the turn still finishes with prose. Verified both ways against the real binary.

Model selection and reasoning

omp/default leaves the choice to Oh My Pi (the zero-config path). A specific model is an Oh My Pi selector — omp/anthropic/claude-sonnet-4-5, or a role like omp/@slow. The interactive picker is populated from omp models --json, and a model the catalog reports as non-reasoning is offered auto only.

Oh My Pi accepts every reasoning level repowise names, which makes this the fullest coverage of any CLI-backed provider: auto omits the flag so the configured thinking level survives, off/none map to --thinking off, and the rest pass through unchanged.

Known consideration

MCP servers configured in the user's own Oh My Pi config still load, and their tool schemas ride along on each request. Oh My Pi has no switch to suppress them for a headless run, so this provider does not pretend to remove them — it disables everything it actually can. The schemas are prompt-cached, so the cost is roughly one cache write per run rather than per page. Documented in docs/agent/OMP.md.

Testing

  • tests/unit/test_providers/test_omp_provider.py — 24 cases. The parser tests are the load-bearing ones: a role: "user" message_end must not leak into the page, text must accumulate across assistant messages rather than overwrite, a stray non-JSON line on stdout must not abort a page, and a missing usage must mark the result estimated. Non-vacuity was checked by mutating the parser in memory and confirming each test fails.
  • Verified against the real binary, pinned to ccs/claude-opus-5: correct routing, --thinking high honoured, and a 400 KB prompt delivered via stdin (argv would have hit MAX_ARG_STRLEN).
  • A real reasoning turn returns content = [thinking, text]; the parser filters to type == "text", so reasoning never lands in a wiki page.
  • Failure paths exercised: non-zero exit surfaces omp's stderr, clean exit with no assistant text raises rather than writing an empty page.
  • Full unit suite: 17,784 passed. The 4 failures on this branch reproduce identically on untouched main (banner width, a DST offset, and two provider-prompt cases) — confirmed by stashing.
  • ruff check, ruff format --check on every touched file, mypy, and generate-types-check all clean.

Drive-by

Second commit fixes the Adding a new LLM provider checklist in CONTRIBUTING.md, found stale while working through it: it pointed at run-config-form.tsx, which does not exist in this repo, and omitted the pricing pair (_COST_TABLE_PREFIX + the _lookup_cost guard) and is_local_model — miss those and a subscription provider is silently billed at another model's rate. Also names the two drift gates that fail the moment a provider lands in _BUILTIN_PROVIDERS. Several provider enumerations in the docs were missing claude_cli; corrected alongside the omp entries.

…Pi install

Oh My Pi holds its own credentials, so `repowise init --provider omp` needs no
API key at all. It joins claude_cli, codex_cli and opencode in KEYLESS_PROVIDERS
and is priced at $0.00, because Oh My Pi bills against its own account rather
than a repowise key.

Generation runs `omp -p --mode json` and parses the JSONL event stream, reading
the assistant `message_end` for the answer, the token usage, the stop reason and
the model actually routed to. Oh My Pi also exposes a bidirectional JSON-RPC
transport, but nothing it adds -- request correlation, mid-turn steering,
host-owned tools -- applies to a one-shot completion, so print mode keeps this
provider the same shape as its three siblings.

The prompt goes on stdin rather than argv: a rendered page prompt can exceed the
per-argument length limit.

Isolation, because the CLI otherwise brings a whole agent session with it. Each
call runs in a temporary scratch directory with tools, extensions, skills and
rules off, `--no-session` so a 68-page run leaves no resumable sessions behind,
and a generated config overlay that turns off memory, auto-learn and the advisor
-- the advisor would otherwise spend a second model call reviewing every page.
The overlay also pins `tools.approvalMode` to `always-ask`: MCP servers from the
user's own config still load, and an inherited `yolo` would let the model run a
mutating MCP tool against their machine unprompted. Headless has no UI to answer
an approval, which is what makes that value safe rather than blocking.

Model selection is an Oh My Pi selector (`omp/anthropic/claude-sonnet-4-5`, or a
role like `omp/@slow`); `omp/default` leaves the choice to Oh My Pi, which is the
zero-config path. The interactive picker is populated from `omp models --json`,
and a model the catalog reports as non-reasoning is offered `auto` only.
Found while adding omp. The checklist pointed at `run-config-form.tsx`, which
does not exist in this repo, and omitted three sites a new provider genuinely
needs: `_PROVIDER_NOTES` and `_LOCAL_PROVIDER_SETUP` in provider_selection.py,
the zero-cost prefix table and `_lookup_cost` guard in pricing.py, and
`is_local_model` in cost_tracker.py. Miss the pricing pair and a subscription
provider is silently billed at another model's rate.

Also names the two drift gates that fail the moment a provider lands in
`_BUILTIN_PROVIDERS` without a deliberate decision beside it: the builtin count
in tests/providers/test_registry.py and the per-provider interactive budget in
tests/unit/server/mcp/test_answer_synthesis_timeout.py.
A clean exit with no prose and an empty stderr is what an install that was
never signed into looks like, and for a provider whose whole premise is "no API
key" that is the likeliest first failure. Reporting "the event stream carried no
assistant text" gave the user nothing to act on.
@RaghavChamadiya

Copy link
Copy Markdown
Member

Thanks @oldschoola. Four PRs in a day, each with a written-out mechanism, is an unusual way to arrive and a welcome one. This one I want to take more slowly than the other three, and I want to be clear that is about the shape of the change rather than its quality.

What is right, and worth saying because it is the part people get wrong: running in a temporary scratch directory and staying out of REPO_PATH_PROVIDERS for the same reason claude_cli does, --no-session so a 68-page run leaves nothing resumable behind, and the config overlay switching off memory and autolearn specifically because they add write-capable tools. A docs run silently writing into someone's memory store would have been a genuinely bad surprise, and you closed it before anyone hit it. Choosing print mode over --mode rpc with the reasoning stated is the right call and the right way to record it.

One thing to fix regardless of what happens to the rest, and it connects to a report that came in the day before yours. _load_omp_model_catalog runs:

completed = subprocess.run(
    [omp_cmd, "models", "--json", "--no-extensions"],
    capture_output=True, check=False, text=True,
    timeout=_CATALOG_TIMEOUT_SECONDS,
)
except (OSError, subprocess.SubprocessError):

text=True with no encoding decodes with the process locale, so on a default Windows install any non-ASCII byte in that JSON raises UnicodeDecodeError. That is a ValueError, so neither OSError nor SubprocessError catches it and the exception escapes past the return None degradation you wrote for every other failure. This is #2186 exactly, filed against codex_cli.py's identically-shaped catalog function. encoding="utf-8", errors="replace" on that call. Your generation path is already correct, .decode("utf-8", errors="replace") at omp.py:563-564, so this is the one site.

On the PR as a whole. A new provider is 24 files and a docs/agent/OMP.md, and the thing I cannot do from a diff is the thing that decides it: run it against a real Oh My Pi install and see what a 68-page generation actually does. Merging a provider nobody here has exercised means the first person to find out it drifted is a user. It also carries an ongoing cost that is not in the diff, since omp models --json and the print-mode event stream are both someone else's interface, and claude_cli / codex_cli / opencode have each needed maintenance when their CLI moved.

So two questions rather than a verdict, and your answers decide it rather than my reading of the code:

  1. Are you in a position to stay on it when Oh My Pi's output changes? An unmaintained provider is worse than none, because it fails at generation time on someone else's repo.
  2. How stable is that print-mode JSONL contract? If message_end and its usage fields are documented and versioned, that is a very different risk from parsing whatever the current build happens to print.

Nothing else here is blocking and I am not asking for code changes beyond the encoding one. #2196 is merged, and #2197, #2198 and #2199 are queued behind CI.

`text=True` alone decodes with the process locale and with `errors="strict"`,
so one non-ASCII byte in `omp models --json` -- a model's display name is
enough -- raises UnicodeDecodeError. That is a ValueError, caught by neither
OSError nor SubprocessError, so it escaped the degradation this loader exists
to provide and took the provider picker down with it instead of falling back to
the default option. Same defect as repowise-dev#2186 against codex_cli's catalog loader.

ValueError is deliberately not added to the handler. With the encoding pinned
the decode can no longer raise, so catching it could only swallow a future
regression that removed the encoding again -- turning this exact loud failure
back into a silent empty catalog.

The test drives a real child process emitting a lone 0xe9, because the bug is
in how `subprocess.run` decodes the child's bytes: a mocked `run` cannot
exercise it. Stripping the encoding argument fails that test.

The generation path was already explicit (`.decode("utf-8", errors="replace")`),
so this was the one site in this provider.
Print mode exits 0 when Oh My Pi gives up on a turn. That is deliberate --
the outcome is carried in the stream's stop reason rather than the exit
status (oh-my-pi#7635, where a deadline abort exited 0 with
`stopReason: "aborted"`). Checking only the exit code therefore accepted
whatever prose had arrived before the abort and filed that fragment as a
finished wiki page.

The parser already read the stop reason for attribution; it now also keeps
`errorMessage`, and generation fails on an aborted turn with the reason Oh My
Pi gave.

Reading `message_end` rather than `agent_end` matters here for a second
reason found in the same issue: the record that truncated under backpressure
was the terminal `agent_end`, while "the preceding message_end and turn_end
records were complete".
@oldschoola

Copy link
Copy Markdown
Contributor Author

Thanks — that's a more careful read than the diff deserved, and the encoding bug is a real one I'd have shipped.

The encoding fix

Done, in the history_scan.py form: encoding="utf-8", errors="replace".

Worth recording that I got it wrong first. My initial attempt also added ValueError to the handler, with a test that patched subprocess.run into raising UnicodeDecodeError. That's circular — with the encoding pinned the decode can no longer raise, so the test only passed because of the catch. Worse, that catch could only ever swallow a future regression that removed the encoding again, turning this exact loud failure back into a silent empty catalog. So the handler stays as narrow as it was.

The test drives a real child process emitting a lone 0xe9, because a mocked run cannot exercise this: the bug is in how run decodes the child's bytes, not in anything the provider does with them. Stripping encoding= fails it on Linux too — the default locale here is UTF-8 with errors="strict", so CI catches the regression without a cp1252 runner.

I've commented on #2186 with those two notes. Not picking up the 44-site sweep — @JolleNo10 reported it and asked for it, and it's rightly theirs.

One change beyond what you asked for

You said you weren't asking for code changes beyond the encoding one, so this is me flagging rather than sneaking it in. While verifying your #2186 link I read oh-my-pi#7635, which says plainly:

JSON-mode deadline aborts currently exit 0 … that's by design (the stream carries stopReason: "aborted"; consumers inspect it rather than the exit code)

My provider only failed on a non-zero exit. So an aborted turn returned whatever prose had arrived before the abort and filed that fragment as a finished page. I traced where it would land: normalize_stop_reason has no "aborted" alias, so stop_reason is None, and page_generator/validation.py only checks == "max_tokens" — nothing downstream flags it. Silent half-page.

Generation now fails on an aborted turn with the reason Oh My Pi gave. It reads the raw stream value, so max_tokens still passes through as the legitimate completion boundary it is. If you'd rather that live in the generation layer as a provider-neutral rule — which is arguably the better home, since the siblings report stop reasons and let the caller decide — say so and I'll pull it out of this PR and raise it separately.

2. How stable is the print-mode contract

Better than I'd have claimed before checking, and the distinction is narrower than "documented vs not".

Pi documents the wire format as a named type, JsonAgentSessionEvent (docs), with a versioned session header ({"type":"session","version":3,…}) and this line: "message_end contains the final authoritative message." The doc's own consumption example is jq -c 'select(.type == "message_end")', which is exactly what _parse_events does. That's upstream Pi's documentation rather than oh-my-pi's own — the fork doesn't re-document it — so the honest framing is: the format is specified upstream, and I confirmed empirically that omp -p --mode json emits that shape.

What RPC adds over it is transport negotiation (negotiate_protocol, v1/v2 chunk framing). The event shapes are the same typed source in both.

Two things from the docs shaped the parser rather than just reassuring me about it:

  • message_update is delta-only and its usage "may remain zero when a provider only reports usage at completion" — so usage is read from message_end, and a message with no usage degrades to estimated=True rather than reporting zeros as fact.
  • #7635's truncation was in the terminal agent_end, while "the preceding message_end and turn_end records were complete". Reading message_end and never depending on agent_end means that class of bug costs nothing here; a truncated trailing line is skipped as unparseable rather than losing the answer.

That's the general shape of the drift answer: unknown event types are ignored, unparseable lines are skipped, missing usage degrades, and a missing message_end is a loud failure rather than an empty page. Drift should degrade or shout, not corrupt. Each of those is a test, and I checked they're not vacuous by mutating the parser and confirming each one fails.

1. Will I stay on it

Yes, and the honest reason is self-interest rather than goodwill: omp is my daily driver, which is why I wrote this at all. Drift lands in my own runs before it reaches anyone else's — that's how both of the bugs above surfaced today.

What I can't promise is indefinite availability, so the thing that matters is what happens if I do go quiet. Concretely: the contract is pinned by tests rather than by my attention, so a shape change fails CI rather than a user's wiki; failures are loud (ProviderError with Oh My Pi's own stderr) rather than silent; and the provider is 620 lines of one file with no shared surface, so ripping it out is a delete rather than an unpick. If it does rot, I'd rather you delete it than carry it.

Also

I've merged main in — the branch was 17 commits behind and the bot was right that it read as removing _interactive_gate from #2106. Clean merge, _interactive_gate intact, 506 tests green on the merged branch, ruff/mypy clean.

And thanks for merging #2197/#2198/#2199. #2203 is the last of mine and the least urgent — it's rebased onto #2155, which made it considerably smaller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants