-
Notifications
You must be signed in to change notification settings - Fork 10
feat(models): OpenAI-compatible /v1/* gateway as built-in Resources (#631, Phase 4 of #510) #1616
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
heskew
wants to merge
43
commits into
main
Choose a base branch
from
feat/models-v1-gateway
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
f785a85
feat(models): add OpenAI-compatible /v1/* REST gateway (#631)
heskew b4c6647
test(models): integration test for /v1/* gateway with real OpenAI SDK…
heskew 82b4443
fix(deps): regenerate package-lock.json with openai resolved
heskew 0eda027
fix(models): resolve gateway 404s and address bot review comments
heskew 5690be5
fix(models): await the /v1/* request body before reading fields
heskew 469c5e1
fix(models): require super_user auth on all /v1/* gateway handlers
heskew 89f2e11
fix(models): guarantee modelsGateway config key; default-OFF with ena…
heskew 732b708
Merge branch 'main' into feat/models-v1-gateway
kriszyp 2597cc2
style: format v1 gateway files with prettier 3.9.3 (lockfile version)
heskew 58bb6e8
debug: TEMP instrumentation to pin CI-only modelsGateway 404 (#1616) …
heskew 6a0deeb
fix(models): keep /v1 5xx error messages generic; log the real error
heskew 1d7dc8e
Revert "debug: TEMP instrumentation to pin CI-only modelsGateway 404 …
heskew 2f000ea
debug: TEMP round-2 instrumentation for CI-only modelsGateway 404 (#1…
heskew a5f2f41
Revert "debug: TEMP round-2 instrumentation for CI-only modelsGateway…
heskew 0cdd0f3
fix(models): map /v1 403 errors to permission_error per OpenAI semantics
heskew 05fe16d
docs: add openai devDependency entry to dependencies.md
heskew 5858cc2
Revert "docs: add openai devDependency entry to dependencies.md"
heskew 897e987
Merge remote-tracking branch 'origin/main' into feat/models-v1-gateway
heskew 69a9807
fix(models): activate REST serving from the /v1 gateway; dedupe model…
heskew 1c5cc32
test(models): fix v1-gateway self-sabotaging assertions; use a real t…
heskew 84b0407
fix(models): make REST activation order-independent — activate after …
heskew 96e875f
Merge branch 'main' into feat/models-v1-gateway
heskew 7551dae
feat(models): mid-stream SSE error frames + embeddings input cap (rev…
heskew 87c90ab
Merge branch 'main' into feat/models-v1-gateway
heskew 6e8cbc7
refactor(models): make the /v1 gateway a clean plugin — remove core s…
heskew 1aab4a9
fix(models): honor tool_choice, unwrap json_schema, bound tool assemb…
heskew 7b150b8
fix(models): null-prototype tool-argument accumulator; reassemble SSE…
heskew 702d153
fix(models): address cross-model review — registry specifier, symmetr…
heskew da1d845
docs(models): correct v1-gateway test header — REST is configured, no…
heskew 42c9bb5
fix(models): zero-cost disabled /v1 gateway; register modelsGateway_e…
heskew 2324cc6
test(models): mirror the malformed-JSON reject-path case on /v1/embed…
heskew fcab43d
test(models): mixed-app regression test for the /v1 gateway's REST be…
heskew bc31ac7
fix(deps): restore production-native lockfile metadata lost in regene…
heskew cfb3b5c
fix(components): request restart when a component's config block is r…
heskew f43b7ac
fix(models): reserve /v1 gateway paths against silent app-Resource re…
heskew 75da150
fix(models): explicit protocol visibility for /v1 routes; getMatch ho…
heskew 32016ee
fix(models): validate top-level chat fields instead of coercing or ig…
heskew ca162e9
fix(models): report capability mismatches as OpenAI 400s, not sanitiz…
heskew 37b3359
fix(models): carry OpenAI 'developer' role as 'system'; reject unknow…
heskew edccb0d
fix(models): report real embedding token usage on /v1/embeddings
heskew 88af257
fix(models): cumulative serialized budget for stream tool-call assembly
heskew 6cdc401
fix(models): charge JSON syntax overhead in the stream assembly budget
heskew 2251931
test(components): reset the restart buffer at requestRestart.test entry
heskew File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
66 changes: 66 additions & 0 deletions
66
integrationTests/server/fixtures/v1-gateway-test-backend.cjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| 'use strict'; | ||
|
|
||
| /** | ||
| * Minimal test backend for the /v1/* gateway integration test (#631). | ||
| * | ||
| * Implements a simple echo backend inline — no imports from the Harper dist | ||
| * directory. Loaded by `bootstrapModels()` via a `backend: <absolute-path>` | ||
| * entry in the test config. Uses `global.models.registerBackend()` (which is | ||
| * already populated when this register function is invoked). | ||
| */ | ||
|
|
||
| function textFromInput(input) { | ||
| if (typeof input === 'string') return input; | ||
| const messages = Array.isArray(input) ? input : input.messages; | ||
| return messages.map((m) => m.content || '').join(' '); | ||
| } | ||
|
|
||
| const echoGenerative = { | ||
| name: 'integration-test-echo', | ||
| capabilities: () => ({ embed: false, generate: true, stream: true, tools: false, adapters: false }), | ||
| async generate(input) { | ||
| const text = textFromInput(input); | ||
| const content = `[echo]: ${text}`; | ||
| return { | ||
| status: 'completed', | ||
| output: { content, finishReason: 'stop' }, | ||
| usage: { promptTokens: text.length, completionTokens: content.length }, | ||
| }; | ||
| }, | ||
| async *generateStream(input) { | ||
| const text = textFromInput(input); | ||
| const words = `[echo stream]: ${text}`.split(' '); | ||
| for (const word of words) { | ||
| yield { deltaContent: word + ' ' }; | ||
| } | ||
| yield { finishReason: 'stop' }; | ||
| }, | ||
| }; | ||
|
|
||
| const echoEmbedding = { | ||
| name: 'integration-test-echo-embed', | ||
| capabilities: () => ({ embed: true, generate: false, stream: false, tools: false, adapters: false }), | ||
| async embed(input) { | ||
| const inputs = Array.isArray(input) ? input : [input]; | ||
| return { | ||
| status: 'completed', | ||
| output: inputs.map(() => new Float32Array([0.1, 0.2, 0.3, 0.4])), | ||
| usage: { embeddingTokens: inputs.length }, | ||
| }; | ||
| }, | ||
| }; | ||
|
|
||
| /** | ||
| * Called by `registerFromModule` in bootstrap.ts. `global.models` is | ||
| * available at call time (Models singleton is set before bootstrapModels runs). | ||
| * @param {{ logicalName: string, kind: 'embedding' | 'generative' }} args | ||
| */ | ||
| exports.register = function ({ logicalName, kind }) { | ||
| const models = global.models; | ||
| if (!models) throw new Error('global.models is not set — bootstrap order violation'); | ||
| if (kind === 'generative') { | ||
| models.registerBackend('generative', logicalName, echoGenerative); | ||
| } else if (kind === 'embedding') { | ||
| models.registerBackend('embedding', logicalName, echoEmbedding); | ||
| } | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # Fixture for the /v1 route-reservation test: an app that (wrongly) tries to claim | ||
| # the gateway's fixed `v1/models` path with its own custom Resource. The gateway's | ||
| # reservedPath marking must turn this into a loud conflict rather than a silent | ||
| # replacement of the endpoint and its super_user gate. | ||
| rest: true | ||
| jsResource: | ||
| files: resources.js |
19 changes: 19 additions & 0 deletions
19
integrationTests/server/v1-gateway-collision-app/resources.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // An app Resource that tries to claim the gateway's fixed `v1/models` route. | ||
| // Before path reservation, this silently replaced the gateway endpoint (both are | ||
| // non-table Resources, so Resources.set saw no conflict). Now it must produce a | ||
| // loud conflict (ErrorResource → 500 + logged error), never this payload. | ||
| class Imposter extends Resource { | ||
| static path = 'v1/models'; | ||
| get() { | ||
| return { imposter: true }; | ||
| } | ||
| } | ||
|
|
||
| // A legitimately-named app resource, proving the app itself still loads and serves. | ||
| export class Legit extends Resource { | ||
| get() { | ||
| return { legit: true }; | ||
| } | ||
| } | ||
|
|
||
| export { Imposter }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,202 @@ | ||
| /** | ||
| * Mixed-app regression test for the `/v1/*` gateway (#631, PR #1616 review). | ||
| * | ||
| * An earlier revision made componentLoader start REST on the gateway's behalf | ||
| * whenever `modelsGateway` was enabled: | ||
| * | ||
| * if (isRoot && resources.isWorker && config.modelsGateway?.enabled) { | ||
| * REST.ensureStarted({ server, resources }); | ||
| * } | ||
| * | ||
| * Root components load before application directories, and REST serves the | ||
| * shared `Resources` registry, so that force-start exposed every REST-exportable | ||
| * entry — including apps that had deliberately omitted `rest`. It also set | ||
| * `REST.started` before a later app could contribute its own port/host/urlPath/ | ||
| * WebSocket options. | ||
| * | ||
| * The force-start is gone; the gateway is a plain plugin that requires REST to | ||
| * already be configured. This pins that. | ||
| * | ||
| * IMPORTANT — what this test can and cannot show. Once a root `rest` section | ||
| * exists, REST serves the shared registry and an app's `@export`ed table is | ||
| * reachable whether or not that app declared `rest` itself. That is existing | ||
| * Harper behavior and is not what this test is about. The gateway-specific | ||
| * regression is only observable with NO `rest` section anywhere: previously, | ||
| * enabling the gateway alone was enough to stand REST up and expose the app. | ||
| * So this suite deliberately configures no `rest` at all. | ||
| * | ||
| * Expected now: enabling the gateway starts nothing, so the app stays off the | ||
| * wire. `/v1/*` is unserved too — that is the documented trade-off in | ||
| * resources/models/v1/index.ts, which logs a warning rather than forcing REST. | ||
| */ | ||
| import { suite, test, before, after } from 'node:test'; | ||
| import assert from 'node:assert'; | ||
| import { resolve as resolvePath } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import { setupHarperWithFixture, teardownHarper, type ContextWithHarper } from '@harperfast/integration-testing'; | ||
|
|
||
| const __dirname = fileURLToPath(new URL('.', import.meta.url)); | ||
| const FIXTURE_PATH = resolvePath(__dirname, 'v1-gateway-mixed-app'); | ||
| const ECHO_BACKEND_PATH = resolvePath(__dirname, 'fixtures/v1-gateway-test-backend.cjs'); | ||
|
|
||
| function authHeader(ctx: ContextWithHarper): string { | ||
| return `Basic ${Buffer.from(`${ctx.harper.admin.username}:${ctx.harper.admin.password}`).toString('base64')}`; | ||
| } | ||
|
|
||
| /** | ||
| * With no REST server expected, a request may be refused outright rather than | ||
| * answered. Both outcomes mean "not served"; only a 200 is a regression. | ||
| */ | ||
| async function statusOrRefused(ctx: ContextWithHarper, path: string): Promise<number | 'refused'> { | ||
| try { | ||
| const res = await fetch(`${ctx.harper.httpURL}${path}`, { | ||
| headers: { Authorization: authHeader(ctx) }, | ||
| signal: AbortSignal.timeout(5_000), | ||
| }); | ||
| return res.status; | ||
| } catch { | ||
| return 'refused'; | ||
| } | ||
| } | ||
|
|
||
| suite('/v1 gateway does not stand REST up on another component behalf', (ctx: ContextWithHarper) => { | ||
| before(async () => { | ||
| await setupHarperWithFixture(ctx, FIXTURE_PATH, { | ||
| config: { | ||
| // Deliberately NO `rest` section — see the header. This is the only | ||
| // configuration in which the gateway's own effect on REST is observable. | ||
| modelsGateway: { enabled: true }, | ||
| models: { | ||
| generative: { default: { backend: ECHO_BACKEND_PATH } }, | ||
| embedding: { default: { backend: ECHO_BACKEND_PATH } }, | ||
| }, | ||
| }, | ||
| env: {}, | ||
| }); | ||
| }); | ||
|
|
||
| after(async () => { | ||
| await teardownHarper(ctx); | ||
| }); | ||
|
|
||
| test('an app that omitted `rest` is not exposed by enabling the gateway', async () => { | ||
| const status = await statusOrRefused(ctx, '/MixedAppPrivate/'); | ||
| assert.notEqual( | ||
| status, | ||
| 200, | ||
| 'enabling modelsGateway must not start REST, which would expose an app that ' + | ||
| 'declared no `rest` section of its own' | ||
| ); | ||
| }); | ||
|
|
||
| test('the gateway does not serve itself either, rather than forcing REST', async () => { | ||
| // The other half of the same property: the gateway declines to start REST for | ||
| // its own benefit too. resources/models/v1/index.ts warns about exactly this. | ||
| const status = await statusOrRefused(ctx, '/v1/models'); | ||
| assert.notEqual(status, 200, 'the gateway must not force REST to start for its own resources'); | ||
| }); | ||
| }); | ||
|
|
||
| /** | ||
| * Control for the suite above. | ||
| * | ||
| * Without this, the primary assertions could pass vacuously — a typo'd path or a | ||
| * fixture that never deployed would also "not be reachable". Here the identical | ||
| * fixture and URL are exercised with a root `rest` section present, which is the | ||
| * state the removed force-start effectively created. The table becomes reachable, | ||
| * so `notEqual(status, 200)` above is a real constraint and not an artifact. | ||
| * | ||
| * This also documents current Harper behavior: once REST is running it serves the | ||
| * shared `Resources` registry, so an `@export`ed table is reachable even though | ||
| * this app declares no `rest` of its own. Whether that ought to be per-app scoped | ||
| * is a separate question (#1931) — this test only pins that enabling the gateway | ||
| * is not what causes it. | ||
| */ | ||
| suite('control: the same table IS reachable once REST is configured', (ctx: ContextWithHarper) => { | ||
| before(async () => { | ||
| await setupHarperWithFixture(ctx, FIXTURE_PATH, { | ||
| config: { | ||
| // `webSocket` is a leaf value because a plain empty object is dropped by | ||
| // flattenObject() in harperConfigEnvVars.ts. | ||
| rest: { webSocket: true }, | ||
| modelsGateway: { enabled: true }, | ||
| models: { | ||
| generative: { default: { backend: ECHO_BACKEND_PATH } }, | ||
| embedding: { default: { backend: ECHO_BACKEND_PATH } }, | ||
| }, | ||
| }, | ||
| env: {}, | ||
| }); | ||
| }); | ||
|
|
||
| after(async () => { | ||
| await teardownHarper(ctx); | ||
| }); | ||
|
|
||
| test('proves the probe URL is live when REST is running', async () => { | ||
| const status = await statusOrRefused(ctx, '/MixedAppPrivate/'); | ||
| assert.equal(status, 200, 'if this stops returning 200 the regression guard above has gone vacuous'); | ||
| }); | ||
| }); | ||
|
|
||
| /** | ||
| * Route reservation (#1616 review): the gateway's fixed routes are non-table | ||
| * Resources, and `Resources.set` compares databaseName/tableName (both undefined) | ||
| * for conflict identity — so before `reservedPath`, an app registering its own | ||
| * Resource at `v1/models` silently replaced the gateway endpoint and its | ||
| * super_user gate, with no startup error. Now it must be a loud conflict: the | ||
| * path serves an ErrorResource (500), never the app's payload, and the rest of | ||
| * the gateway keeps working. | ||
| */ | ||
| suite('an app claiming a reserved /v1 route is a loud conflict, not a silent takeover', (ctx: ContextWithHarper) => { | ||
| before(async () => { | ||
| await setupHarperWithFixture(ctx, resolvePath(__dirname, 'v1-gateway-collision-app'), { | ||
| config: { | ||
| rest: { webSocket: true }, | ||
| modelsGateway: { enabled: true }, | ||
| models: { | ||
| generative: { default: { backend: ECHO_BACKEND_PATH } }, | ||
| embedding: { default: { backend: ECHO_BACKEND_PATH } }, | ||
| }, | ||
| }, | ||
| env: {}, | ||
| }); | ||
| }); | ||
|
|
||
| after(async () => { | ||
| await teardownHarper(ctx); | ||
| }); | ||
|
|
||
| test('the contested path never serves the imposter payload', async () => { | ||
| const res = await fetch(`${ctx.harper.httpURL}/v1/models`, { | ||
| headers: { Authorization: authHeader(ctx) }, | ||
| signal: AbortSignal.timeout(5_000), | ||
| }); | ||
| const text = await res.text(); | ||
| assert.ok( | ||
| !text.includes('imposter'), | ||
| `app resource must not take over a reserved route, got: ${text.slice(0, 200)}` | ||
| ); | ||
| // Loud conflict: the reserved path serves the conflict ErrorResource (500), | ||
| // which is the same behavior table-path conflicts have always had. | ||
| assert.equal(res.status, 500, `expected the conflict ErrorResource, got ${res.status}: ${text.slice(0, 200)}`); | ||
| }); | ||
|
|
||
| test('the rest of the gateway still serves (conflict is contained to the contested path)', async () => { | ||
| const res = await fetch(`${ctx.harper.httpURL}/v1/embeddings`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json', 'Accept': 'application/json', 'Authorization': authHeader(ctx) }, | ||
| body: JSON.stringify({ model: 'default', input: 'hello' }), | ||
| signal: AbortSignal.timeout(5_000), | ||
| }); | ||
| assert.equal(res.status, 200, 'uncontested gateway routes must be unaffected'); | ||
| }); | ||
|
|
||
| test('the app itself still loads and serves its legitimately-named resource', async () => { | ||
| const res = await fetch(`${ctx.harper.httpURL}/Legit/`, { | ||
| headers: { Authorization: authHeader(ctx) }, | ||
| signal: AbortSignal.timeout(5_000), | ||
| }); | ||
| assert.equal(res.status, 200, 'the collision must not break the rest of the app'); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # Deliberately declares NO `rest` section. | ||
| # | ||
| # This app exports a table, so it has a REST-exportable entry in the shared | ||
| # Resources registry — but by omitting `rest` it opts out of being served over | ||
| # REST. Enabling the /v1 gateway must not change that. | ||
| graphqlSchema: | ||
| files: '*.graphql' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # An @export table in an app that does not declare `rest`. | ||
| # | ||
| # @export puts it in the shared Resources registry; the missing `rest` section is | ||
| # what should keep it off the wire. If enabling modelsGateway ever starts REST on | ||
| # this app's behalf, this table becomes reachable and the regression test fails. | ||
| type MixedAppPrivate @table @export { | ||
| id: ID @primaryKey | ||
| value: String | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.