diff --git a/CHANGELOG.md b/CHANGELOG.md index 376d662dd5..94e1ecb2f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -143,6 +143,7 @@ ### πŸ“ Maintenance +- **docs (architecture):** add `docs/architecture/ROUTER_BACKENDS.md` β€” an ADR pinning down how the routing engines (`ts` native, `bifrost`, `cliproxy`, `9router`, VibeProxy-compatible) relate to each other along two orthogonal axes (lifecycle: in-process / supervised / external vs. relay selection backend), answering the architecture questions raised in [#5603](https://github.com/diegosouzapw/OmniRoute/issues/5603) (backend interface model, why CLIProxy spawns a process, feature-flag swapping, actionable route-contract errors). The typed router-backend registry the ADR describes lands separately via [#5868](https://github.com/diegosouzapw/OmniRoute/pull/5868). ([#5891](https://github.com/diegosouzapw/OmniRoute/pull/5891)) - **tests (autoCombo):** stabilize the `getTaskFitnessWithSource identifies fitness_table as source for known models` unit test, which flaked whenever the models.dev capabilities DB was populated in CI: the fixture model `gpt-4o` is a real models.dev catalog id, so the fitness resolution chain returned `models_dev_tier` instead of the expected static `fitness_table` source. The fixture now uses `claude-sonnet` (a shortened alias absent from the models.dev catalog, matching the sibling resolution-chain test), which deterministically falls through to the static table β€” the exact `source` and score assertions are preserved (`0.95` = `FITNESS_TABLE.coding["claude-sonnet"]`). ([#5890](https://github.com/diegosouzapw/OmniRoute/pull/5890)) β€” thanks @KooshaPari - **oauth (dead-code removal):** delete the superseded legacy OAuth **service-class** hierarchy under `src/lib/oauth/services/`. The live OAuth flow runs through `src/lib/oauth/providers.ts` + `src/lib/oauth/providers/` (wired into the generic `oauth/[provider]/[action]` route); the old per-provider `class *Service extends OAuthService` implementations plus their barrel had **zero** production or test references. Removed `oauth.ts` (base class), `openai.ts`, `github.ts`, `claude.ts`, `codex.ts`, `antigravity.ts`, `qwen.ts`, `qoder.ts`, and the `index.ts` barrel (βˆ’1559 LOC). Kept the three still-live files that routes import **directly** by path: `kiro.ts` (Kiro import/exchange routes), `cursor.ts` (Cursor import route), and `codexImport.ts` (utility fns for the Codex bulk-import route). Proven safe by `typecheck:core` staying green (any live reference would fail the build) + a filesystem guard `tests/unit/oauth-legacy-services-removed.test.ts` pinning the removal against re-introduction. Salvage of the closed PR [#5039](https://github.com/diegosouzapw/OmniRoute/pull/5039). gaps v3.8.42 β€” T10 (5.7). diff --git a/docs/architecture/ROUTER_BACKENDS.md b/docs/architecture/ROUTER_BACKENDS.md new file mode 100644 index 0000000000..1ce0cdd67d --- /dev/null +++ b/docs/architecture/ROUTER_BACKENDS.md @@ -0,0 +1,141 @@ +# Router Backends & Embedded Services β€” architecture contract (ADR) + +> **Status:** Accepted Β· **Context:** [#5670](https://github.com/diegosouzapw/OmniRoute/issues/5670), +> [#5603](https://github.com/diegosouzapw/OmniRoute/issues/5603) Β· **Contract:** `src/domain/routing/routerBackends.ts` + +This ADR pins down how `ts` (native), `bifrost`, `cliproxy`, `9router`, and +VibeProxy-compatible engines relate to each other, so contributors stop +conflating two things that are architecturally distinct. It documents the typed +registry introduced by the router-backend-registry work as the single source of +truth for that model. + +## The core distinction β€” two orthogonal axes + +An engine's role is described by **two independent axes**, encoded together in the +registry's `RouterBackendDefinition`: + +1. **Lifecycle** (`RouterBackendLifecycle`) β€” _how the engine runs_: + - `in-process` β€” runs inside the OmniRoute Node process (the native TS pipeline). + - `supervised` β€” a local child process OmniRoute installs/starts/stops/health-checks + via `ServiceSupervisor`, then consumes as a provider connection. + - `external` β€” an HTTP endpoint OmniRoute dispatches to but does **not** manage + (configured by an env base URL). + - `disabled` β€” registered but not selectable. +2. **Selection axis** (relay routing backend) β€” _whether the relay dispatches to it_: + `RelayRoutingBackend = "ts" | "bifrost" | "auto"` in + `src/app/api/v1/relay/chat/completions/routingBackend.ts`. + +The mistake to avoid: treating "embedded service" and "routing backend" as one +list. They are not. A `supervised` engine (9router/cliproxy) is a **provider +connection consumed by the native pipeline**, not an alternate relay dispatch +backend. `bifrost` is the reverse β€” a relay dispatch backend that (historically) +was `external`-only. + +## The registry β€” single source of truth + +`src/domain/routing/routerBackends.ts` declares every engine once, with its +lifecycle, capabilities, service identity, default port, health config, and +telemetry support. Consumers look engines up via `getRouterBackend(id)`, +`listRouterBackends()`, and `listRouterBackendsByCapability(cap)` instead of +special-casing each sidecar. + +| Backend | Lifecycle | Service (axis A) | Relay backend (axis B) | Health | Default port | +| ----------- | ------------ | ---------------- | ---------------------- | ------------- | ------------ | +| `ts` | `in-process` | β€” | `ts` (native) | β€” | β€” | +| `bifrost` | `external`ΒΉ | β€”ΒΉ | `bifrost` / `auto` | `/health` | β€” | +| `cliproxy` | `supervised` | `cliproxy` | β€” (provider) | `/v1/models` | 8317 | +| `9router` | `supervised` | `9router` | β€” (provider) | `/api/health` | 20130 | +| `vibeproxy` | `external` | β€” | β€” (provider adapter) | `/v1/models` | β€” | + +ΒΉ Bifrost's promotion to a `supervised` embedded service (installable/startable +from `/api/services/bifrost/`) is tracked in +[#5817](https://github.com/diegosouzapw/OmniRoute/pull/5817); until it merges, +Bifrost is `external`-only (reachable solely via `BIFROST_BASE_URL`). + +`capabilities` (`chat`, `responses`, `streaming`, `tools`, `vision`, +`oauth-backed`, `dashboard-embed`, `model-sync`, `native-hot-path`) let callers +filter by what an engine can actually do rather than hard-coding per-id branches. + +## Axis A β€” embedded services (supervised process side) + +- **Registry of supervised processes:** `src/lib/services/bootstrap.ts` `SERVICES[]` + (today: `9router`, `cliproxy`). +- **Lifecycle owner:** `src/lib/services/ServiceSupervisor.ts` β€” `start()` spawns the + child, gates on `waitForHealthy()`, taps stdout/stderr into a ring buffer; + `stop()` SIGTERMβ†’SIGKILL; all serialized under a lock. +- **State union** (`src/lib/services/types.ts`): + `not_installed | stopped | starting | running | stopping | error`, plus an + orthogonal `HealthState = healthy | unhealthy | unknown`. +- **Why a separate process (not an in-proc SDK)?** Process isolation is what makes + install/start/stop/health/logs independently controllable per sidecar and lets the + loopback spawn-guard apply. Modeling an in-proc adapter is future work β€” the + `native-hot-path` capability flag is where that would be expressed. + +### Lifecycle route contract (`/api/services//…`) + +Status codes are **state/verb/path-specific by design** β€” this is the contract, not +inconsistency: + +| Call | Condition | Status | +| ---------------------------- | ------------------------------- | ------------------------------------ | +| `POST .../start` | service `not_installed` | **409** (precondition) | +| `POST .../stop` | already stopped | **200** (idempotent no-op) | +| `GET .../status` | OK | **200** (`live ?? row ?? "unknown"`) | +| `POST .../start` | spawn failure | **503** (transient) | +| `GET .../status`, `.../stop` | uncaught error | **500** | +| `GET /api/services//logs` | unknown tool `` | **404** `Service '' not found` | +| `GET .../status?reveal=key` | missing `X-Reveal-Confirm: yes` | **403** (9router only) | +| **any** `/api/services/*` | caller not loopback/private-LAN | **403 LOCAL_ONLY** | + +All error bodies are shaped by `createErrorResponse()` β†’ +`{ error: { message, type }, requestId }`, where `type` is derived from the status +(`500β†’server_error`, `404β†’not_found`, `409β†’conflict`, else `invalid_request`) and is +the machine-actionable discriminator. Messages are pre-sanitized +(`sanitizeErrorMessage()`, Hard Rule #12). + +**The loopback guard** is the most common source of a `403`: `/api/services/` is in +`LOCAL_ONLY_API_PREFIXES` (`src/server/authz/routeGuard.ts`) and +`src/server/authz/policies/management.ts` rejects any non-loopback / non-private-LAN +caller **before auth**, because these routes spawn child processes (Hard Rules 15 +and 17). Reaching them through a public tunnel is `403` by design. + +## Axis B β€” relay routing backend (dispatch side) + +Only the relay proxy path `/api/v1/relay/chat/completions` selects a dispatch +backend; the main `/api/v1/chat/completions` surface never consults +`routingBackend.ts`. + +- **Selection** (`resolveRelayRoutingBackend`): a single global env toggle β€” + `OMNIROUTE_RELAY_BACKEND` / `RELAY_ROUTING_BACKEND` ∈ {`ts`, `bifrost`, `auto`}. + If unset, `auto` when Bifrost is configured+enabled, else `ts`. +- **Behavior:** + - `bifrost` (forced): Bifrost failure β†’ hard `502`, no fallback. + - `auto`: try Bifrost, on failure/cooldown silently fall through to native. + - `ts` / post-fallback: the native `open-sse` translator/executor pipeline. +- **Cooldown:** per-`baseUrl` failure cooldown in `bifrostCooldown.ts`. + +Selection is **all-or-nothing at the relay level today** β€” there is no per-provider +or per-request engine swap on `release/v3.8.43`. The per-request gate is being added +by the sidecar-manifest work +([#5869](https://github.com/diegosouzapw/OmniRoute/pull/5869) manifest + +[#5870](https://github.com/diegosouzapw/OmniRoute/pull/5870) `shouldTryBifrostForRequest`), +which lets `auto` route only manifest-eligible providers through Bifrost. + +## Dashboard integration + +The services dashboard polls `GET /api/services//status` every 5s via +`src/app/(dashboard)/dashboard/providers/services/hooks/useServiceStatus.ts`, +returning `{ tool, state, pid, port, health, installedVersion, latestVersion, +updateAvailable, autoStart, … }`. There is no shared availability-context provider β€” +each component calls the hook per tool. On `!res.ok` the hook currently surfaces a +bare `HTTP `; mapping the `error.type` field to a human explanation is a +tracked UX improvement, not a contract change. + +## Consequences + +- New engines register once in `ROUTER_BACKENDS`; consumers gain them via capability + queries without new per-id branches. +- "Is this a service or a routing backend?" is answered by the `lifecycle` field, not + by which list an id happens to appear in. +- The Bifrost supervision (#5817) and native hot-path migration (#5670) build on this + shared contract instead of special-casing each sidecar.