feat(models): OpenAI-compatible /v1/* gateway as built-in Resources (#631, Phase 4 of #510) by heskew · Pull Request #1616 · HarperFast/harper

heskew · 2026-07-06T15:48:25Z

Summary

OpenAI-compatible /v1/* gateway as built-in core Resources (Phase 4 of #510): POST /v1/embeddings, POST /v1/chat/completions (streaming + non-streaming), and GET /v1/models — thin protocol-translation wrappers over scope.models. Unmodified OpenAI SDK / LangChain.js clients point baseURL at Harper's REST port and work.

resources/models/v1/ (new module): translation.ts (pure OpenAI↔internal mappers), errors.ts (OpenAI {error:{message,type,code,param}} on all error paths; 5xx messages are generic with the real error logged), embeddings.ts, chatCompletions.ts, models.ts, index.ts.
Off by default. modelsGateway: { enabled: false } ships in defaultConfig.yaml; enable with modelsGateway: { enabled: true }. Explicit-flag on an always-present key, not presence-gating (deliberate: presence-gating fails silently — see HARPER_SET_CONFIG applied after component loading (thread-timing race) and not persisted at install — env-var-configured components silently fail to register #1618).
Auth enforced in the handlers: unauthenticated → OpenAI-shaped 401 authentication_error; authorization mirrors the default Resource gate (super_user).
Streaming uses openaiStream() (Add openaiStream() formatter helper for OpenAI-compatible SSE streaming #514) through the fixed transformIterable (see below); AbortSignal propagates via ALS (resolveCallContext captures request.signal synchronously — verified in adjudication, no explicit wiring needed).
tool_choice ships the 'return' path only ('auto' orchestration = Add agent-loop orchestration / toolMode: 'auto' to scope.models #612). Model-id routing is safe upstream: @embed / models.embed no longer forwards the logical model name as the provider wire model id #1596's toBackendOpts strips the logical routing key before backend dispatch, so provider calls always use the backend's configured wire model.
openai@^6.45.0 devDep for the end-to-end SDK test.

⚠️ Where to look first — `transformIterable` fix (core serving path)

server/serverHelpers/contentTypes.ts transformIterable() called transform(step.value) on the terminal {done: true, value: undefined} step, so the text/event-stream serializer threw at the end of any finite async-iterable response body (latent until now: existing SSE is infinite subscriptions closed via .return()). Minimal guard, both sync and async paths; covered by a real-HTTP + real-eventsource unit test through the full serializeStream path.

Known CI caveat — integration test flake until #1618 lands

integrationTests/server/v1-gateway.test.ts passes or fails as a unit, run-to-run, on identical code (~50% on Linux CI; macOS reliably passes). Root cause was pinned via instrumented CI runs and is NOT in this PR: scope.options (OptionsWatcher) re-parses the on-disk config file, which races the env-var config flush at boot — so handleApplication's enabled check can read pre-env values even though componentLoader itself saw enabled: true (probe-verified on both threads of a failing run). The fix is up as the #1618 PR (OptionsWatcher composes env layers over root-config reads); once it merges this test is deterministic. Instrumentation commits were reverted (58bb6e867/2f000ea13 + reverts in history).

Resolved in review (cross-model, adjudicated)

Body-await on POST handlers (request.data is a Promise on the REST path — the integration test's 200 assertions had been masked by the CI 404 and had never actually validated); auth enforcement (above); model-id conflation (fixed upstream by #1596, with tests); 5xx message hygiene; 3 gemini inline suggestions applied earlier. The AbortSignal PR-body claim was verified TRUE by the adjudicator (ALS capture at Models.ts:151,202).

Open for review

Permission granularity: super_user (default-Resource parity) — deliberately conservative; loosen to a dedicated permission if desired.
The transformIterable guard is the one change outside the new module.

Docs

Companion PR: HarperFast/documentation#568 (synced to the enabled: true form and the 401/super_user auth behavior).

Closes #631
Tracking: #510

Generated by LLMs (Claude Sonnet 5 implementation workers + Claude Fable 5 coordination/diagnosis, via Claude Code). Cross-model reviewed: Codex leg + Harper-domain adjudication (Gemini leg failed on tooling that day — no second-model perf coverage; noted per skill protocol).

🤖 Generated with Claude Code

REST activation on bare instances (added 2026-07-17)

The gateway's resources are served by REST's middleware chain, which only activates when a component config contains a rest/REST section — a bare instance (no deployed apps) has none, leaving the endpoints registered but unservable (this was the persistent integration-test 404; never a config race). Fix: componentLoader calls the new REST.ensureStarted({ server, resources }) after the root plugin loop completes, gated on modelsGateway.enabled. Ordering is safe by construction: any user rest/REST section has already registered with its own options by that point, making the call a no-op (verified live: rest: { webSocket: false } alongside the gateway keeps the ws chain REST-free and registers the http handler exactly once). /v1/models also now dedupes logical names registered for both generative and embedding, and the streaming integration test authenticates the OpenAI SDK with a real operation token — the documented production flow (Harper JWT as the api key).

Registers three REST resources on the REST port when `modelsGateway: {}` is present in config: POST /v1/chat/completions — streaming and non-streaming chat POST /v1/embeddings — embedding endpoint GET /v1/models — enumerate registered backends Key design points: - OpenAI SDK sends `Accept: application/json` for ALL requests (incl. streaming). Harper's REST layer only dispatches `Accept: text/event-stream` as CONNECT; everything else is dispatched as the HTTP method. So `stream: true` chat requests land in `post()`, not `connect()`. The resource returns `{ body: Readable }` which REST.ts bypasses serialisation on (REST.ts:165-193). - Fixes a latent bug in `transformIterable` (contentTypes.ts): the transform function was called on the terminal `{ done: true }` step of async generators, crashing when `serialize()` received `undefined`. Guard added: skip transform on any `done: true` step. - Pure shape-mapper layer in `v1/translation.ts`; all functions are side-effect-free and fully unit-tested without a running server. - OpenAI error envelope (`{ error: { message, type, code, param } }`) built in `v1/errors.ts`; resources catch errors themselves to avoid REST.ts serialising them as RFC 9457 Problem Details. - `tool_choice: 'auto'` maps to `toolMode: 'return'`; full in-process orchestration is #612 (out of scope for this PR). - `openai` added as devDependency (^6.45.0) for future integration tests. - `listBackends(kind)` added to backendRegistry.ts for GET /v1/models. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…#631) Adds an integration test that starts a real Harper instance with: - `modelsGateway: {}` in config - A deterministic echo backend registered via the registerFromModule path (CJS fixture at integrationTests/server/fixtures/v1-gateway-test-backend.cjs) Tests all three endpoints: - GET /v1/models — model list shape - POST /v1/embeddings — single and batched input; 400 error shape - POST /v1/chat/completions — non-streaming shape; 400 error shape; streaming via the real OpenAI Node.js SDK (validates full SSE framing) The streaming test specifically exercises the SSE serving-path: the OpenAI SDK sends Accept: application/json for all requests, so stream: true lands in post() not connect(). The resource returns { body: Readable } which REST.ts bypasses serialisation on, and the SDK successfully parses the [DONE]-terminated SSE stream. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

socket-security · 2026-07-06T15:49:01Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	openai@6.45.0

View full report

gemini-code-assist

Code Review

This pull request introduces an OpenAI-compatible '/v1/*' REST gateway for Harper, adding endpoints for listing models, generating embeddings, and handling both streaming and non-streaming chat completions. It also resolves a critical bug in 'transformIterable' that caused crashes during the terminal step of streaming responses. Feedback from the review highlights opportunities to propagate the client's 'AbortSignal' to save server resources on disconnects, strengthen request body validation (such as checking for arrays in the embeddings endpoint and validating message structures), and defensively handle tool call arguments to prevent double-serialization.

claude · 2026-07-06T15:52:59Z

Reviewed the latest push (7551dae, mid-stream SSE error frames + embeddings input cap). Both address kriszyp's prior findings correctly, with boundary-case test coverage. No blockers found.

npm ci in CI was failing with "Missing: openai@6.45.0 from lock file" because package-lock.json was not updated when openai was added to devDependencies. Ran `npm install openai --package-lock-only`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Use `modelsGateway: { enabled: true }` in the integration test; an empty object `{}` is silently dropped by `flattenObject()` in `harperConfigEnvVars.ts` (no leaf paths → nothing set via HARPER_SET_CONFIG) so `componentLoader` never sees the key and skips loading the gateway - Add `|| Array.isArray(body)` guard to `V1Embeddings.post` (Gemini: arrays pass `typeof x !== 'object'` unchanged since `typeof [] === 'object'`) - Guard against pre-serialised string arguments in `toOAIToolCalls` (Gemini: passing a JSON string through `JSON.stringify` would double-encode it) - Rename inner `body` → `readable` in `V1ChatCompletions.post` streaming path (Claude bot: shadowed the outer `body` parameter at line 28) - AbortSignal thread left as informational: signal is already propagated via `resolveCallContext` / `contextStorage.getStore()?.signal` in `Models.ts` Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

server/REST.ts builds `request.data` via the streaming JSON deserializer and hands it to resource methods unawaited, so the `body` argument V1ChatCompletions.post/V1Embeddings.post received was a Promise, not the parsed object. Every real JSON POST to the gateway was reading undefined fields off a Promise and returning 400 — the integration test's 200 assertions were masked by the (separate) 404 gating bug and had never actually exercised this path. Await the body before reading any field, including `stream`. Adjudicated cross-model review finding on #1616. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Static method overrides on V1Models, V1ChatCompletions, and V1Embeddings bypass Resource's transactional() wrapper and therefore the default allowRead/allowCreate super_user gate. Add authorizeV1Request() in errors.ts that returns a well-formed OpenAI error envelope (401 for anonymous, 403 for non-super_user, null to proceed). Call it at the top of each handler before touching the body. Unit tests cover: 401 anon, 403 non-super_user, 200 super_user, and that auth is checked before body validation (prevents body deserialization on unauthorized requests). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…bled flag Previously the gateway only loaded if the user added a `modelsGateway:` key to harperdb-config.yaml (presence-gating). On CI and fast-startup environments the HARPER_SET_CONFIG env var raced component loading, causing intermittent 404s. Add `modelsGateway: { enabled: false }` to defaultConfig.yaml so the key is always present in the resolved config and componentLoader always sees it. The `handleApplication` entry point now checks `scope.options.get(['enabled'])` and returns immediately when false (same pattern as the agent component). Opt in with `modelsGateway: { enabled: true }`. The integration test already passes this explicitly; its new header comment notes the dependency on #1618 (env-config ordering) for reliable CI behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

CI's Format Check runs the lockfile prettier (3.9.3); the files were formatted with 3.8.3 which disagrees on these three. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…— REVERT BEFORE MERGE Logs per-thread: componentLoader's resolved modelsGateway config + env-var presence, handleApplication's enabled value, and (test-side) the child's harper-config.yaml block + get_configuration.modelsGateway. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Internal error strings (backend stack details, paths) don't belong in the wire response. 4xx messages are client-actionable and pass through unchanged. Adjudicated cross-model review suggestion on #1616. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…1616) — REVERT BEFORE MERGE" This reverts commit 58bb6e8.

) — REVERT BEFORE MERGE Probes the install/config path this time: createConfigFile post-env-apply + post-validate, updateConfigValue entry + pre-write, per-thread loadRootComponents config view, plus the round-1 loader/handler probes. Test-side dump intentionally NOT restored (round 1's before() awaits delayed the first request and masked the race — that run went green). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… 404 (#1616) — REVERT BEFORE MERGE" This reverts commit 2f000ea.

401 = bad/missing credentials (authentication_error); 403 = valid credentials lacking permission (permission_error) — matching authorizeV1Request's own envelope. Addresses claude-bot review feedback on #1616. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ush (#1618) OptionsWatcher re-parsed the on-disk root config, while componentLoader decides from the resolved in-memory config — which deterministically includes runtime env config (HARPER_SET_CONFIG et al.). At boot the env-applied values reach the file only after the first thread's runtime re-apply flushes them, so a component's boot-time scope.options reads (e.g. an enabled gate in handleApplication) could observe pre-env values the loader never saw. Proven via instrumented CI runs on #1616: componentLoader logged enabled:true on every thread of a failing run while the routes 404'd. OptionsWatcher now composes the env layers (composeConfigFromEnv, side-effect free, same precedence as the runtime pipeline) over every root-config read, including the install window where the file does not exist yet. Application config.yaml scopes are untouched, as is behavior when no config env vars are set. flattenObject additionally warns once per path when an env-var config value is an empty object — it flattens to nothing by design (load-bearing for restore-on-removal semantics) but the silent drop has confused users. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Per the dependency policy: the openai SDK was added as a devDependency for the /v1 gateway's unmodified-SDK acceptance test (#631) without the accompanying dependencies.md justification. Dev-only (lighter-bar per the doc's own note): ~10MB unpacked, zero transitive deps, single dynamic import in one integration test, never in the published install tree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

This reverts commit 05fe16d.

…ush (#1618) (#1726) * fix(config): scope.options sees runtime env config before the file flush (#1618) OptionsWatcher re-parsed the on-disk root config, while componentLoader decides from the resolved in-memory config — which deterministically includes runtime env config (HARPER_SET_CONFIG et al.). At boot the env-applied values reach the file only after the first thread's runtime re-apply flushes them, so a component's boot-time scope.options reads (e.g. an enabled gate in handleApplication) could observe pre-env values the loader never saw. Proven via instrumented CI runs on #1616: componentLoader logged enabled:true on every thread of a failing run while the routes 404'd. OptionsWatcher now composes the env layers (composeConfigFromEnv, side-effect free, same precedence as the runtime pipeline) over every root-config read, including the install window where the file does not exist yet. Application config.yaml scopes are untouched, as is behavior when no config env vars are set. flattenObject additionally warns once per path when an env-var config value is an empty object — it flattens to nothing by design (load-bearing for restore-on-removal semantics) but the silent drop has confused users. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(config): address domain-review findings on the env-overlay watcher - ENOENT branch no longer clobbers #rootConfig before the name check — a first read whose scope name is absent from env config falls through to the original handling and emits 'ready' (was: 'remove', which nothing consumes at boot, hanging Scope.ready). - Root-config discrimination is now an exact match against the resolved root-config path (lazy configUtils require), covering the legacy harperdb-config.yaml name and excluding components that ship a root-named file; filename fallback (both names) when configUtils isn't initialized. - composeConfigFromEnv failures in the ENOENT branch route to 'error' instead of rejecting unhandled. - Tests: name-absent ENOENT emits ready (regression guard); legacy filename gets the overlay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(config): explicit isRootConfig override; filename-only heuristic The exact-path check against getConfigFilePath() misclassified every watcher in environments where configUtils resolves a real instance (the unit harness boots one), sending the overlay tests into a ready-hang and timing out unit CI. The heuristic is now filename-only (current + legacy root names) with an explicit isRootConfig constructor argument for callers that know root-ness authoritatively. Known limitation documented: a component shipping its own harper-config.yaml is misclassified as root — harmless unless config env vars are set and keys collide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(config): keep env-config specifics in the config layer, not OptionsWatcher OptionsWatcher had absorbed config-domain knowledge — the three env-var names, the root config filenames, and the compose semantics. Move all of it behind two config-layer functions: overlayRootEnvConfig(base) (no-op when no config env vars; composes otherwise; handles the empty/absent base) and isRootConfigFilename(). OptionsWatcher now only expresses 'this is the root config, overlay env onto it', which is the minimal knowledge a watcher needs. Also moots the gemini review note about the inline object guard — that guard now lives inside overlayRootEnvConfig. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(config): preserve env-derived scope config on a transient ENOENT (gemini review) Env config is file-independent, but the ENOENT read path discarded it: when the scope's name was in the composed env config and #scopedConfig was already set, the branch fell through to #resetConfig()+emit('remove'), wiping active config on a transient/unreadable-file read. Now, when the scope IS in the composed env config, an ENOENT preserves it — first read emits ready, a later read merges — and only falls through to reset when the scope is absent from env config. Real deletion is unaffected (chokidar 'unlink' -> #handleUnlink). Addresses the gemini [high] review note on #1726. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(config): thread the loader's authoritative isRoot into OptionsWatcher (kris review) The isRootConfig constructor escape hatch had no caller: componentLoader had the authoritative isRoot two frames up and never threaded it. Now the Scope construction passes isRoot through to OptionsWatcher, so real component loads use the loader's signal — an app component that ships a root-named config file no longer gets the env overlay. The filename heuristic remains as fallback for direct constructions only (tests, ad-hoc callers); doc updated accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(config): preserve base-file empty-object scopes; env overlay on unlink (kris review) Two env-layer-semantics leaks onto the base layer, from the #1726 re-review: - composeConfigFromEnv routed the base file through the same flattenObject drop path as env layers, so 'componentName: {}' in the file vanished the moment any config env var was set — firing 'remove' for a scope that was present pre-PR. Base empty objects are user content: restore the ones composition didn't otherwise populate (env replacements still win); env layers keep their no-overrides drop semantics, and the drop warning no longer fires for the base file. - #handleUnlink reset unconditionally to DEFAULT_CONFIG, discarding HARPER_SET_CONFIG-defined scopes on a runtime config-file deletion — the same env-independence the ENOENT read path preserves. Both paths now share #applyEnvOnlyConfig (first application → ready, already configured → merge, malformed env JSON → 'error'). Tests: base empty-object survival (compose + watcher level, incl. nested, env-replaced, and env-populated cases), env-layer {} drop semantics pinned, and unlink-with-env-scope (no remove; exercises the shared merge branch). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdbdepRFuyUWoEmNN1pSVQ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

# Conflicts: # server/serverHelpers/contentTypes.ts

… ids The gateway registered its v1/* resources into the global registry, but REST's middleware chain only activates when some component config contains a rest/REST key. On a bare instance (no deployed apps) nothing provides one, so the resources were registered but unservable and every /v1/* request fell through to unhandled -> 404 (the "CI-only" integration failure; it was never a config race, and reproduces anywhere Harper runs with no apps). - Split the started-guarded chain registration out of REST.handleApplication into an exported REST.ensureStarted(scope); handleApplication delegates to it. ensureStarted does not adopt the caller's config section as REST's http options, so the gateway (or any core plugin registering REST-served resources) can activate serving without clobbering rest config semantics. - modelsGateway.handleApplication calls ensureStarted after registering its resources. - /v1/models: dedupe logical names registered for both generative and embedding (previously `default` appeared twice); OpenAI model ids are unique. - Refresh the stale race-theory NOTE in the integration test; the suite now also covers the bare-instance activation path. - package-lock: regenerated after the main merge (naive git lockfile merge). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PgCUUCjEqChDgquKJhaNZr

…oken for the SDK - The 200-assertions interpolated `await res.text()` into the assert message, which evaluates unconditionally — consuming the body on SUCCESS and making the following res.json() throw "Body is unusable". Read the text once and JSON.parse it. (CI showed the endpoints themselves returning 200: the batched-input test, without the template, passed.) - The streaming test assumed AUTHENTICATION_AUTHORIZELOCAL lets any apiKey through; it only covers credential-less requests — a present-but-invalid Bearer is 401 "invalid token" (verified against a live instance). Mint an operation token via create_authentication_tokens and use it as the SDK apiKey, which is also the documented production flow. - Revert package-lock.json to the merge result (897e987): the locally regenerated lockfile dropped platform-conditional optionals (bufferutil, utf-8-validate) and drifted react-native pins, so CI `npm ci` rejected it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PgCUUCjEqChDgquKJhaNZr

…root plugin load The prior shape (gateway calls REST.ensureStarted from its own handleApplication) was order-dependent: with both modelsGateway.enabled and a user rest: section, whichever loaded first won, and a gateway-first load registered REST's chain with empty options — silently dropping e.g. webSocket: false (claude review, server/REST.ts:349 thread). componentLoader now calls REST.ensureStarted({ server, resources }) after the root plugin loop completes, gated on modelsGateway.enabled: by then any rest/REST section — whatever its key order — has already registered with the user's options, making the call a no-op. ensureStarted takes {server, resources} structurally so a Scope still satisfies it. handleApplication hardens getAll() with ?? {} (section values like `rest: true` scope to undefined). Note for the record: an in-memory `config.rest = true` synthesis was tried and rejected — OptionsWatcher only emits `ready` for sections present in the config file (or env overlay), so a synthesized key hangs boot on scope.ready. Verified against live instances (dist build, HARPER_SET_CONFIG like the integration harness): - bare instance: /v1/models 200 - with rest: {webSocket: false}: http chain "authentication → rest" exactly once, websocket chain has no rest entry (option honored), /v1/models 200 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PgCUUCjEqChDgquKJhaNZr

kriszyp

Review: OpenAI-compatible `/v1/*` gateway

Clean, well-scoped addition: three REST resources (/v1/models, /v1/embeddings, /v1/chat/completions) that translate OpenAI wire shapes to Harper's internal models facade. Default-off, super_user-gated, zero cost on the default path. The REST-activation ordering bug (gateway racing a user rest: config section) was found and fixed correctly in the final commit (84b04071f) — confirmed by reading server/REST.ts and components/componentLoader.ts directly, not just the commit message.

Test coverage is strong on the pure translation/auth logic, and the integration suite proves the real OpenAI SDK round-trips against a live instance, including streaming. The one real gap is mid-stream error handling on /v1/chat/completions (stream: true) — see the inline comment below.

Security: all three endpoints gate on super_user, matching existing precedent (Resource.ts:459-471, REST.ts:161, Table.ts:5540); authorizeV1Request runs before any body parsing in all three handlers; openai devDependency is dev-only with zero hard transitive deps and never reaches the production install tree.

Perf: confirmed zero default-path cost — handleApplication returns immediately when modelsGateway.enabled is falsy, and componentLoader.ts's REST.ensureStarted call is gated the same way.

One nit not called out inline: 05fe16d23 added a dependencies.md write-up for the new openai devDependency, and the very next commit (5858cc220) reverted it with a bare "Revert ..." and no stated rationale — worth a one-line confirmation that the omission is intentional (and if devDependencies are meant to be exempt from the doc, maybe say so there so this doesn't get flagged again).

Verdict: COMMENTS — nothing blocking; the streaming-error gap below is the one item worth a follow-up (either in this PR or as a tracked issue).

— Claude Sonnet 5

kriszyp

Apologies if this is a duplicate comment, testing another form of reviews.

Bring the gateway branch current with main (carries the updated .github/workflows/claude-review.yml so the review workflow validates).

@kriszyp

…iew) Addresses @kriszyp's review on #1616: - openaiStream() now wraps its token-pull loop in try/catch: a mid-stream backend error (Models#wrapStream re-throws) is converted into a final OpenAI-shaped `data: {error}` SSE frame instead of an abrupt socket close, so streaming clients get the same error semantics as the non-streaming path. chatCompletions passes a formatError derived from toOpenAIError; the generic formatter stays decoupled from the v1 layer. - /v1/embeddings caps batched input at 2048 items (OpenAI's own limit), returning a 400 OpenAI-shape error above it. The third finding (embed() usage always zero) is an upstream Models facade gap — tracked in #1882, out of scope here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXx78W5LxbvxEdSyrB1vvn

heskew · 2026-07-21T04:38:52Z

CI status note

Two red checks remain, neither caused by this PR's changes:

1. Next.js adapter integration (Node 20/22/24) — a pre-existing tsc error in resources/DatabaseTransaction.ts, unrelated to the gateway:

resources/DatabaseTransaction.ts: error TS2345: Argument of type 'Promise<number | void>'
is not assignable to parameter of type 'Promise<void>'.

It originates from #1688 (commit-latency analytics) and is present on main and this branch identically — this PR doesn't touch that file. It only surfaced here because merging main in tripped the Next.js job's path filter, so its strict npm run build (tsc) actually ran. The main test workflow doesn't fail on it because that step is continue-on-error: true ("we currently have type errors so just ignore that"); the Next.js adapter build has no such guard.

Fix already exists: #1853 (fix/commit-latency-promise-type) — corrects the recorder's param to the honest Promise<number | void> and drops the as Promise<void> cast. Verified locally: applying #1853's two edits on top of this branch's merged tree builds clean (exit 0, 0 TS errors). Plan: land #1853 to main, then re-merge main here to inherit it.

2. Integration Tests 3/6 (Node.js v26) — a v26-isolated blob-lifecycle flake (Invalid column family specified in write batch, ~100s hang). Shard 3/6 passed on v22, v24, Bun, and uWS; unrelated to this PR. Should clear on re-run.

This branch now also carries: a merge of current main, and fixes for the three review findings above (mid-stream SSE error frames + embeddings input cap; the embed-usage facade gap is tracked separately in #1882).

kriszyp

I'll let your model assess if these are worth fixing.

kriszyp · 2026-07-21T12:03:52Z

+					if (!incoming.id) continue;
+					const existing = toolAssembly.get(incoming.id) ?? { index: toolAssembly.size, arguments: {} };
+					if (incoming.name) existing.name = incoming.name;
+					if (incoming.arguments) existing.arguments = { ...existing.arguments, ...incoming.arguments };


This accumulator predates the gateway, but the gateway now makes it a public HTTP path and this hunk moves it under the new error boundary without bounding it. It copies every previously accumulated argument property on each partial delta (O(n²) as fields grow), while distinct IDs grow toolAssembly without a cap. Could we mutate with Object.assign and enforce per-stream call-count/argument-size caps, terminating through the sanitized error-frame path on overflow?

kriszyp · 2026-07-21T12:03:52Z

+ * may invoke tools itself.
+ */
+export function toGenerateOpts(body: OAIChatRequest): GenerateOpts {
+	const opts: GenerateOpts = { toolMode: 'return' };


tool_choice is advertised above but never read: none, required, and a named function all produce the same {toolMode:'return'} options, while toGenerateInput still forwards every tool. Concretely, tool_choice:'none' may return a tool call, and required/named selection are not enforced. Please implement the supported modes (at least omit tools for none) or return a 400 OpenAI error for choices the internal contract cannot represent instead of silently weakening them.

kriszyp · 2026-07-21T12:03:52Z

+		if (rf.type === 'json_object') {
+			opts.responseFormat = 'json';
+		} else if (rf.type === 'json_schema' && rf.json_schema) {
+			opts.responseFormat = { schema: rf.json_schema as object };


The OpenAI wire value here is a wrapper like {name, strict, schema: <JSON Schema>}, while Harper's GenerateOpts.responseFormat expects {schema: <JSON Schema>}. Assigning the whole wrapper means the OpenAI backend wraps it again and sends the metadata object where the provider expects the actual schema. Please validate/extract rf.json_schema.schema, and update the test to use the real wrapper shape rather than a bare schema under json_schema.

kriszyp · 2026-07-21T12:03:52Z

+		}
+
+		const model = typeof req.model === 'string' ? req.model : 'default';
+		const messages = translateMessages(req.messages);


These translations happen before the try and only the outer messages array was validated. messages:[null], tool_calls:[{}], and tools:[{}] each throw a TypeError here/inside the mapper, so REST returns an RFC 9457 500 instead of a 400 OpenAI envelope; a rejected JSON body Promise also escapes before try. Please validate the nested wire shapes and parse failure, return badRequest() for client errors, and keep unexpected mapper failures inside the generic toOpenAIError() boundary.

kriszyp · 2026-07-21T12:03:52Z

 			if (request.isWebSocket) return;
 			return http(request, nextHandler);
 		},
 		{ after: 'authentication', ...(httpOptions as any) }


Keeping REST after authentication is correct, but it means invalid/expired credentials are short-circuited by security/auth.ts before any /v1 Resource runs. That path serializes {error: err.message}, not OpenAI's {error:{message,type,code,param}}; the integration test mints a valid token and does not cover this common SDK failure path. Could the gateway add a /v1-scoped wrapper/hook that normalizes auth 401 responses, plus an end-to-end invalid Bearer assertion, without weakening this ordering?

kriszyp · 2026-07-21T12:03:52Z

+ *
+ * SSE serving-path note: the OpenAI SDK sends `Accept: application/json` for
+ * ALL requests including streaming ones (`client.ts:1160` in the SDK source).
+ * Harper's REST layer dispatches `Accept: text/event-stream` as CONNECT, and


This documents a compatibility hole rather than handling it: a client that explicitly sends Accept: text/event-stream with POST {stream:true} is rewritten to CONNECT by REST, and this Resource has no connect(), so it receives method-not-allowed. The added SDK happens to send application/json, but other valid SSE clients need to work too. Please delegate connect() to the same parsed-body implementation or adjust REST dispatch for POST, and add a test with exact Accept: text/event-stream.

kriszyp · 2026-07-21T12:03:52Z

 import * as roles from '../resources/roles.ts';
 import * as jsHandler from '../resources/jsResource.ts';
 import * as login from '../resources/login.ts';
+import * as modelsGateway from '../resources/models/v1/index.ts';


Because this is a static import, the full gateway module graph is evaluated in the main process and every worker even though modelsGateway.enabled defaults to false. For an opt-in built-in, could this use the existing lazy trusted-plugin pattern (or a conditional import once enabled) so disabled installations have zero gateway startup/module-load cost?

heskew and others added 2 commits July 6, 2026 08:40

gemini-code-assist Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread resources/models/v1/chatCompletions.ts Outdated

Comment thread resources/models/v1/embeddings.ts Outdated

Comment thread resources/models/v1/translation.ts

claude Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread resources/models/v1/chatCompletions.ts Outdated

heskew requested a review from kriszyp July 6, 2026 17:21

heskew and others added 2 commits July 6, 2026 10:24

heskew and others added 2 commits July 6, 2026 11:51

claude Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread resources/models/v1/index.ts Outdated

heskew and others added 2 commits July 6, 2026 16:26

Merge branch 'main' into feat/models-v1-gateway

732b708

claude Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread resources/models/v1/errors.ts

heskew and others added 2 commits July 8, 2026 15:01

style: format v1 gateway files with prettier 3.9.3 (lockfile version)

2597cc2

CI's Format Check runs the lockfile prettier (3.9.3); the files were formatted with 3.8.3 which disagrees on these three. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

claude Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread resources/models/v1/index.ts Outdated

claude Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread components/componentLoader.ts Outdated

heskew and others added 3 commits July 8, 2026 15:12

Revert "debug: TEMP instrumentation to pin CI-only modelsGateway 404 (#…

1d7dc8e

…1616) — REVERT BEFORE MERGE" This reverts commit 58bb6e8.

claude Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread resources/models/v1/index.ts Outdated

Revert "debug: TEMP round-2 instrumentation for CI-only modelsGateway…

a5f2f41

… 404 (#1616) — REVERT BEFORE MERGE" This reverts commit 2f000ea.

claude Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread resources/models/v1/errors.ts

heskew mentioned this pull request Jul 8, 2026

fix(config): scope.options sees runtime env config before the file flush (#1618) #1726

Merged

heskew and others added 2 commits July 11, 2026 17:06

Revert "docs: add openai devDependency entry to dependencies.md"

5858cc2

This reverts commit 05fe16d.

heskew and others added 2 commits July 17, 2026 15:28

Merge remote-tracking branch 'origin/main' into feat/models-v1-gateway

897e987

# Conflicts: # server/serverHelpers/contentTypes.ts

claude Bot reviewed Jul 18, 2026

View reviewed changes

Comment thread server/REST.ts

heskew mentioned this pull request Jul 18, 2026

test(models): LangChain.js e2e acceptance for the /v1 gateway + embeddings encoding_format (#631) #1856

Open

heskew marked this pull request as ready for review July 18, 2026 01:07

heskew mentioned this pull request Jul 18, 2026

[Models] /v1 gateway: input-validation hardening — 400-vs-500 envelope consistency, json_schema nesting, 'developer' role #1857

Open

kriszyp reviewed Jul 20, 2026

View reviewed changes

Comment thread resources/models/v1/chatCompletions.ts Outdated

Comment thread resources/models/v1/embeddings.ts

Comment thread resources/models/v1/embeddings.ts

kriszyp reviewed Jul 20, 2026

View reviewed changes

Merge branch 'main' into feat/models-v1-gateway

96e875f

Bring the gateway branch current with main (carries the updated .github/workflows/claude-review.yml so the review workflow validates).

heskew mentioned this pull request Jul 21, 2026

[Models] Models#embed() discards backend token usage — /v1/embeddings usage always reports zero #1882

Open

kriszyp requested changes Jul 21, 2026

View reviewed changes

Uh oh!

Conversation

heskew commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

⚠️ Where to look first — transformIterable fix (core serving path)

Known CI caveat — integration test flake until #1618 lands

Resolved in review (cross-model, adjudicated)

Open for review

Docs

REST activation on bare instances (added 2026-07-17)

Uh oh!

socket-security Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kriszyp left a comment

Choose a reason for hiding this comment

Review: OpenAI-compatible /v1/* gateway

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kriszyp left a comment

Choose a reason for hiding this comment

Uh oh!

heskew commented Jul 21, 2026

CI status note

Uh oh!

kriszyp left a comment

Choose a reason for hiding this comment

Uh oh!

kriszyp Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

kriszyp Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

kriszyp Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

kriszyp Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

kriszyp Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

kriszyp Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

kriszyp Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

heskew commented Jul 6, 2026 •

edited

Loading

⚠️ Where to look first — `transformIterable` fix (core serving path)

socket-security Bot commented Jul 6, 2026 •

edited

Loading

claude Bot commented Jul 6, 2026 •

edited

Loading

Review: OpenAI-compatible `/v1/*` gateway