Skip to content

Run your own models, and serve them on demand - #29

Merged
joelst merged 11 commits into
mainfrom
feat/byom-import
Aug 31, 2026
Merged

Run your own models, and serve them on demand#29
joelst merged 11 commits into
mainfrom
feat/byom-import

Conversation

@joelst

@joelst joelst commented Aug 31, 2026

Copy link
Copy Markdown
Owner

Why

Two things stopped Flint being useful as a local model control plane.

Any OpenAI-compatible client bounced off the endpoint. Foundry Local only answers for a model that is already resident in memory, and it exposes no HTTP route to load one — I probed ten plausible load paths and all of them 404. So a coding agent or IDE plugin did the obvious thing, read GET /v1/models, posted to a model it found there, and got 400 Model 'X' is not loaded with no way to recover. Nothing in the OpenAI protocol lets a client load a model.

You could only run models Microsoft had published. A model sitting in a folder on disk was unusable, even a well-formed ONNX export.

What changed

Serve models on demand

Flint now listens on the configured port itself and forwards to the native service. When — and only when — that exact not-loaded rejection comes back, it loads the model and replays the request once.

The proxy forwards first and inspects afterwards. Checking "is this loaded?" up front would put an SDK call on every request and would happily spend five seconds loading a multi-gigabyte model for a request that was going to be rejected for a bad route anyway. Letting Foundry answer first means it keeps doing all routing and validation, and we only ever load in response to a request it accepted.

Constraints that shape the rest of it:

  • Only the exact not-loaded 400 is retried, and only once. A retry re-executes the user's request, so it must not fire on an unrelated failure that happens to mention a model.
  • Only cached models resolve, so a stray or hostile identifier can never trigger a multi-gigabyte download. The catalog contains models that are merely downloadable; those are deliberately not indexed.
  • Loads are serialised and deduplicated, so a burst of concurrent requests for the same model produces one load, not five competing for VRAM.
  • Streaming is passed through untouched, including on the replayed request, so tokens still arrive as they are produced.

catalog.getModel() accepts only the friendly alias and throws on the variant id that /v1/models advertises, so resolution has to produce both the alias and the variant. Dropping the variant would silently load a CPU build when the client asked for the CUDA one — and the replayed request still names the original variant, so it would fail again with the same error it was meant to fix.

Fix service start, which had never worked

startService failed every time it ran after initialization with Foundry Local Core is already initialized. The native core initializes once per process, so the existing "clear the singleton and re-create with new config" strategy could not work — meaning the configured port and bind address had never actually taken effect. Flint now reads the port the service reports and proxies the configured port to it. This is the first release where the port setting does anything.

Bring your own model

inspectModelFolder validates a folder and explains why it is unusable — GGUF, missing tokenizer, interrupted download, weights genai_config.json does not point at — before anything is copied. importModelFolder stages the copy, authors the Foundry-specific inference_model.json that almost no public ONNX repo ships, then activates it with a single atomic rename and cleans up staging on any failure. linkModelFolder registers a model in place through a directory junction, so a second copy of a multi-gigabyte model is unnecessary and the source folder is never written to.

The prompt template is now visible and editable instead of an invisible guess, because a wrong template does not fail loudly — it silently mangles the conversation. Templates are validated before they are written, and rewrites are refused for catalog and linked models, whose files Flint does not own.

Models are added from the Models tab, and the list can be sorted by name, family, or last updated.

Verification

Built and ran the app, then hit it from outside exactly as an agent client would:

Scenario Result
Gateway on configured port 5272 /status reports the public port; internal port not leaked
Cold request, model not loaded 200 in 12.4 s — autoloaded and replayed
Warm request 1.3 s, no reload
Unknown model 400 in 0.05 s, no download
Streaming 42 SSE events, first at 3.0 s, last at 3.9 s — incremental
Restart × 3, keep-alive sockets held open clean rebind each time
Port conflict clear error, and the next start recovers

235 tests pass. Coverage gate raised to lines 97 / functions 94 / branches 84 / statements 95.

Running it for real found a bug the tests could not: the native service starts before the proxy binds, so a port conflict left it listening on an unadvertised port while the user was told startup had failed. Both partial-failure paths now wind the service back before throwing.

Mutation testing caught two more. The async resolver was being called without await — the tests passed because await tolerates a sync value, so the stub is now async to match production. And the 413 response for an oversize body was being destroyed by the very req.destroy() meant to stop the upload.

Risk

The endpoint path changed for every client, including Flint's own chat: traffic now crosses a proxy that did not exist before. The mitigation is that pass-through is the default and untouched behaviour — buffering only happens for small JSON posts where a replay is possible, and any other body streams through as before.

The BYOM and template modals and the sort control are verified by compile and unit tests but have not been clicked through; I can build and launch the app but not drive the window. Worth a manual pass before release. macOS is entirely unverified.

Deferred deliberately: access-log integration for proxied traffic (writeToDisk() appends synchronously and would stall the event loop), SSE token accounting, multipart/audio autoload, and unauthenticated non-loopback autoload.

joelst and others added 6 commits August 30, 2026 20:10
Replaces the stale post-0.4 idea list with a sequenced 0.5-1.0 plan grounded in
probe results rather than documentation.

Findings that changed the plan:
- The service does serve OpenAI-shaped GET /v1/models; /openai/models and
  /foundry/list are 404, so docs citing them are stale.
- Friendly-alias routing already works, so the planned alias-to-variant
  translation layer is unnecessary.
- Streaming, [DONE] and usage all work. The one real agent gap is that an
  unloaded model returns 400, which no OpenAI client can resolve itself --
  auto-load on demand is Flint's job.
- BYOM works today on SDK 1.2.4 with no new API, and directory junctions are
  traversed by the native scanner, so extra model folders need no copying and
  no writes to a foreign cache. This replaces the unsafe shared-cache idea.
- The catalog ships zero embedding models, so /v1/embeddings depends on BYOM.
- Of the top 1000 HF onnx text-generation repos, 166 ship genai_config.json but
  only 29 exceed 100 downloads/month, and 2 of 301 ship inference_model.json.
  Hence a curated validated catalog rather than a generic HF browser.

Defers tokenomics and the Flint-native tool executor past 1.0, and records the
SDK/core/CLI churn risk.
Foundry Local discovers a directory as a model when it holds genai_config.json
plus an inference_model.json carrying a Name, and no download.tmp. Of the top
1000 HuggingFace onnx text-generation repos, 166 ship genai_config.json but only
2 of 301 ship inference_model.json, because it is Foundry-specific. Flint has to
author it, so BYOM is a filesystem problem rather than an SDK one -- verified
working on SDK 1.2.4 with no new API and no upgrade.

Three sidecar commands:

- inspectModelFolder reads a candidate folder and reports why it cannot be used
  before anything is copied: GGUF (Foundry is ONNX-only), a missing tokenizer, an
  interrupted download, or weights genai_config.json does not point at. It finds
  weights nested under onnx/ or similar, since HF repos rarely put them at the
  root.
- importModelFolder stages the copy in a sibling directory, writes the metadata,
  then activates it with one atomic rename, so a crash or a failed validation can
  never leave a half-written model where the scanner would try to load it. The
  staging directory is removed on any failure. An ownership marker records that
  Flint created the directory and may delete it.
- linkModelFolder registers a model stored elsewhere via a directory junction.
  The native scanner traverses junctions, so this avoids a second copy of a
  multi-gigabyte model. The source is never written to, which is also why linking
  refuses a folder lacking inference_model.json -- it cannot add one.

The prompt template is chosen from the model's own chat_template.jinja by control
token, falling back to the architecture and flagging itself as a guess. The jinja
template wins over the architecture because a fine-tune can keep an architecture
while changing its turn markers.

Both traversal guards are mutation-tested: replacing the path check with the
classic startsWith comparison fails exactly the sibling-prefix test, and ignoring
the jinja template fails six.

The e2e suite drives the real sidecar and then asks the real SDK whether the
result is discoverable, which is the only way to prove the written files add up
to a model Foundry will load. It runs under a throwaway appName so a developer's
own cache is untouched.

Also adds byom-import.js to the coverage gate, which was not covering it, and
raises the thresholds to match what the suite now achieves (94.4 statements,
82.9 branches) rather than leaving slack.
An imported model's prompt template was an invisible guess: detection picked one from
the model's chat template or architecture, wrote it, and the user had no way to see or
correct it. A wrong template does not fail loudly -- the model loads and produces subtly
wrong output -- so the guess needed to be inspectable.

- validatePromptTemplate() enforces all four turns and the {Content} placeholder, which
  Foundry substitutes; without it the message text is dropped entirely.
- inspectModelFolder now returns the resolved template and the known presets, so the
  template can be reviewed and changed before anything is copied.
- importModelFolder accepts a promptTemplate that overrides detection.
- getModelTemplate / setModelTemplate read and rewrite an imported model's template.
  Rewrites go through a temp file and rename, so an interrupted write cannot truncate
  inference_model.json and make the model vanish from the catalog.
- Rewrites are refused unless the directory carries Flint's ownership marker: catalog
  models belong to Foundry and a linked model's files belong to the user.

Tests: 177 pass (was 147). The ownership guard was mutation-tested -- disabling it left
the original linked-model test green, because it was passing for the wrong reason, so a
test using a catalog-shaped directory was added that fails when the guard is removed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b567a7ef-7eb9-4f06-ad74-3c781f5aa481
The BYOM commands had no way in from the app. The Models tab now has an "Add model
folder" flow: pick a folder, see what was detected, review or edit the prompt template,
then either copy the folder into Flint's cache or link it in place. "Link in place" is
disabled with an explanation when the folder has no inference_model.json, because linking
never writes to the source and so cannot author the missing file. An imported model's
card gains a "Prompt template" button that reopens the same editor.

Template rules moved into sidecar/prompt-template.js, which imports nothing -- not even
Node builtins -- so the editor validates with exactly the code the sidecar enforces. A
second copy of those rules in the frontend would drift, and the failure mode is silent:
a template missing {Content} loads fine and drops the message text. byom-import.js
re-exports the module and src/lib/sdk.ts re-exports it to the UI; the production bundle
was checked to confirm it is bundled rather than externalised.

Also adds sorting of the model list by name, family, or last updated, persisted across
restarts. The logic lives in src/lib/model-sort.ts rather than the page component so it
can be tested: catalog createdAt is unix seconds and sometimes null, models with no
family sort last, and every mode falls through to alias so the list does not reshuffle
on refresh.

Cargo.lock recorded 0.4.0 while the manifest says 0.4.5; refreshed.

Tests: 188 pass (was 177). The family ordering and the alias tie-break were both
mutation-tested -- inverting the family rule fails 2 tests, removing the tie-break
fails 4.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b567a7ef-7eb9-4f06-ad74-3c781f5aa481
Foundry Local only answers for a model that is already resident, and exposes no
HTTP route to load one. Any OpenAI-compatible client that read /v1/models and
posted to a model it found there got 400 "Model is not loaded" with no way to
recover, which is the single thing that blocked third-party agent clients.

Flint now listens on the configured port and forwards to the native service. On
that exact rejection it resolves the identifier, loads the model, and replays the
request once. It forwards first and inspects afterwards, so Foundry still performs
all routing and validation and a bad request cannot trigger a large load. Only
cached models resolve, so a stray identifier cannot start a download. Loads are
serialised and deduplicated, and streaming is passed through untouched.

This also fixes service start, which failed every time with "Foundry Local Core is
already initialized". The native core initializes once per process, so the manager
can never be re-created to change its port. Flint now takes the port the service
reports and proxies to it, which makes the configured port and bind address work
for the first time.

Verified end to end against the real service: cold 200 in 15s, warm 707ms with no
reload, unknown model a clean 400. Guards mutation-tested.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b567a7ef-7eb9-4f06-ad74-3c781f5aa481
Verified the gateway end to end in the real app for the first time, which
surfaced a partial-failure state: the native service is started before the proxy
binds, so a port conflict left it listening on an unadvertised port while the
user was told the service had failed to start. The same applied when the service
never became ready. Both paths now stop the native service before throwing, so
the reported state matches reality and a retry starts clean.

Verified in the running app: gateway on the configured port 5272, /status hides
the internal port, cold request 200 in 12.4s via autoload, warm 1.3s with no
reload, unknown model 400 in 0.05s, 42 SSE events streamed incrementally.
Restart survives three cycles with keep-alive sockets held open, and a failed
bind now recovers on the next start.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b567a7ef-7eb9-4f06-ad74-3c781f5aa481
Copilot AI lite review requested due to automatic review settings August 31, 2026 03:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There is at least one concrete proxy implementation bug plus an overly-broad autoload trigger that conflicts with the PR’s stated “exact-match” retry constraint.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR turns Flint into an “OpenAI-compatible endpoint that can autoload models” by introducing a local reverse proxy (gateway) in the sidecar, and adds BYOM (bring-your-own-model) support to import/link ONNX model folders and manage prompt templates, with UI support and expanded test/coverage gating.

Changes:

  • Add a sidecar HTTP gateway that forwards to Foundry Local and, on the specific “model not loaded” rejection, loads and replays the request once.
  • Add BYOM folder inspect/import/link + prompt-template detection/validation/editing, and expose these flows in the Models UI (including model sorting).
  • Expand unit/e2e coverage to the new pure modules and raise coverage thresholds accordingly.
File summaries
File Description
vite.config.js Adds new modules to coverage include list and raises coverage thresholds.
src/routes/+page.svelte Adds model sorting persistence/UI, BYOM import/link modal UI, and prompt-template editor UI.
src/lib/sdk.ts Exposes BYOM/template IPC APIs to the UI and re-exports node-free template validation utilities.
src/lib/model-sort.ts Implements deterministic model sorting helpers (name/family/updated).
src/lib/model-sort.test.ts Adds unit tests for model sort helpers.
src/lib/ipc-contracts.ts Extends sidecar IPC contract for gateway toggle and BYOM/template commands + types.
src-tauri/Cargo.lock Bumps flint crate version entry.
sidecar/prompt-template.js Adds node-free prompt template presets, detection, and validation logic.
sidecar/model-registry.js Adds cached-only model identifier → (alias, variantId) resolution for autoload safety.
sidecar/gateway.test.ts Adds end-to-end proxy behavior tests (pass-through, autoload, streaming, failure modes).
sidecar/gateway.js Implements the reverse proxy with buffering rules, single replay, and serialized/deduped loads.
sidecar/gateway-http.test.ts Adds unit tests for gateway HTTP helpers and model registry resolution.
sidecar/gateway-http.js Adds pure helper utilities for header hygiene, error detection, buffering decisions, and /status rewrite.
sidecar/foundry-sidecar.js Integrates gateway startup/shutdown and adds BYOM/template command implementations.
sidecar/byom-import.test.ts Adds unit tests for BYOM validation/synthesis logic and template validation rules.
sidecar/byom-import.js Implements pure BYOM validation, name sanitization, cache-root guards, and inference_model synthesis.
sidecar/byom-import.e2e.test.ts Adds e2e tests that drive the real sidecar and validate discovery/editing via the real SDK scanner.
RELEASE_ROADMAP.md Updates roadmap to reflect the 0.5→1.0 plan and newly shipped gateway/BYOM work.
docs/BACKLOG.md Updates backlog with shipped items and new follow-ups (SDK matrix pinning, cache inventory, etc.).
.github/copilot-instructions.md Updates repo guidance to document the new gateway/BYOM conventions and constraints.
.changeset/byom-import.md Declares a minor release for BYOM + prompt-template editor + sorting.
.changeset/autoload-gateway.md Declares a minor release for the autoload gateway and service start fix.
Review details
  • Files reviewed: 21/22 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread sidecar/gateway.js
Comment thread sidecar/gateway-http.js
Two BYOM tests ask the real Foundry SDK to resolve an imported model. The
native core ships as a win32-x64 binary, so they can only pass on Windows;
on Linux CI FoundryLocalManager.create() threw and failed the run.

Gate those two on the binary actually existing, and split the template
rewrite test so its sidecar-level assertions -- including that Name survives
the rewrite -- still run everywhere. Verified 24/24 on Windows and, with the
guard forced off, 22 passed / 2 skipped as CI will see it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b567a7ef-7eb9-4f06-ad74-3c781f5aa481
Copilot AI review requested due to automatic review settings August 31, 2026 03:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There are a few concrete correctness/contract issues in the new gateway and BYOM UX copy that should be tightened before approval.

Review details

Suppressed comments (5)

Previously missed (3) — in code that hasn't changed since the last review.

sidecar/foundry-sidecar.js:249

  • This doc comment and waitForUpstream declaration are on the same line, which hurts readability and is likely to violate formatting/lint rules.
/** The native service reports readiness on /status; startWebService() returning does not. */async function waitForUpstream (port, deadlineMs = 20000) {

sidecar/byom-import.js:157

  • This warning says Flint will keep an existing inference_model.json, but the import path will overwrite it when a promptTemplate override is provided. The message should reflect that conditional behavior so users aren’t misled.
  if (base.has('inference_model.json')) {
    warnings.push('The folder already has inference_model.json; Flint will keep the existing file.');
  }

src/routes/+page.svelte:4772

  • The tooltip claims Flint wrote the prompt template for any local:// model, but linked models are also local:// and their templates are not owned/editable by Flint. This is misleading UI copy.
                          title="View or edit the prompt template Flint wrote for this model"

sidecar/gateway.js:107

  • The connect event handler is using a single parameter, but Node passes (req, socket, head). As written, this destroys the request object rather than the raw socket, which can leave CONNECT requests behaving inconsistently and undermines the explicit “no tunneling” guarantee.
  server.on('connect', socket => socket.destroy());
  server.on('upgrade', (_req, socket) => socket.destroy());

sidecar/gateway-http.js:75

  • isModelNotLoadedError is documented as a narrow match for the exact Foundry “model is not loaded” 400, but the implementation matches any 400 body containing both “model” and “is not loaded”. That’s broader than the PR’s stated constraint and can cause unintended retries on unrelated errors that happen to include those words.
export function isModelNotLoadedError (status, body) {
  if (status !== 400) return false;
  const text = String(body || '');
  if (!/is not loaded/i.test(text)) return false;
  // Foundry's wording: "Model 'X' is not loaded. Please load the model before ...".
  return /\bmodel\b/i.test(text);
}
  • Files reviewed: 21/22 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Co-authored-by: joelst <30506169+joelst@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 31, 2026 03:41
The matcher anchored the whole sentence, so any rewording by Foundry would
silently disable autoload and hand clients back the 400 the gateway exists to
absorb. It also discarded the body when the JSON parsed to another shape.

Match the quoted model name instead -- the one part that cannot appear by
accident -- and fall back to the raw text unless a string message is found.
This still rejects prose like "Model validation failed because it is not
loaded". Verified against a live service, whose exact wire body is now a test
case, and mutation-tested: re-anchoring fails 2 tests, loosening to a
substring fails 1.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b567a7ef-7eb9-4f06-ad74-3c781f5aa481

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The model-id registry can overwrite the “alias means unpinned” mapping when variant IDs are ${alias}:<version> (likely for BYOM), which can change autoload resolution semantics unexpectedly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/lib/sdk.ts:905

  • PromptTemplate is declared here but also declared in src/lib/ipc-contracts.ts. Duplicating the shape in two places is easy to let drift (especially since it’s used for IPC validation and UI editing). Prefer re-exporting a single shared type to keep the contract/source-of-truth consistent.
/** The four turn wrappers Foundry substitutes `{Content}` into when building a prompt. */
export interface PromptTemplate {
  system: string;
  user: string;
  assistant: string;
  • Files reviewed: 21/22 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread sidecar/model-registry.js
Comment on lines +54 to +59
const bare = stripVersion(variant.id);
if (bare === variant.id) continue;
const existing = index.get(bare);
if (!existing || compareVersions(variant.id, existing.variantId) > 0) {
index.set(bare, { alias, variantId: variant.id });
}
Copilot AI review requested due to automatic review settings August 31, 2026 03:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There are a few concrete correctness/messaging issues in sidecar/foundry-sidecar.js (notably inference_model overwrite semantics and misleading bind logging) that should be fixed before approving.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

sidecar/foundry-sidecar.js:249

  • The readiness JSDoc comment and waitForUpstream function are on the same line, which makes the comment easy to miss and harms readability (and may trip format/lint rules). Split them onto separate lines.
/** The native service reports readiness on /status; startWebService() returning does not. */async function waitForUpstream (port, deadlineMs = 20000) {

sidecar/foundry-sidecar.js:1557

  • This warning log says "Service binding", but at this point the native service chooses its own loopback port; bindAddr controls the gateway listener. The log message is misleading when binding to 0.0.0.0/lan.
      if (bindAddr !== '127.0.0.1') {
        log('warn', `Service binding to ${bindAddr} — accessible from other network interfaces`);
      }

sidecar/foundry-sidecar.js:766

  • This block overwrites inference_model.json whenever payload.promptTemplate is provided, even if the source folder already had an inference_model.json. That contradicts both the comment here ("only author … when the source did not provide one") and the inspection warning ("Flint will keep the existing file"), and it can also drop any extra fields a future Foundry inference_model.json might add.

Prefer preserving the existing JSON when present, only overriding the Name/PromptTemplate fields Flint controls when the user supplies a template override.

    // Only author inference_model.json when the source did not provide one.
    const infPath = path.join(versionDir, 'inference_model.json');
    if (!fs.existsSync(infPath) || payload.promptTemplate) {
  • Files reviewed: 21/22 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings August 31, 2026 03:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There are at least two concrete maintainability issues in changed code (a misleading gateway test and a formatting/lint-risk line in foundry-sidecar.js) that should be corrected before approval.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

sidecar/gateway.test.ts:356

  • This test name/comment contradict the behavior being asserted: it claims “does not autoload for a non-loopback caller”, but the request is loopback and the test expects called === true. This makes the suite misleading and won’t catch regressions in loopbackOnlyAutoload. Rename the test (or actually simulate a non-loopback remoteAddress).
  it('does not autoload for a non-loopback caller', async () => {
    let called = false;
    gateway = await startGateway({
      loopbackOnlyAutoload: true,
      load: async () => { called = true; },

sidecar/foundry-sidecar.js:249

  • There’s a missing newline between the JSDoc comment and async function waitForUpstream(...), which hurts readability and will likely violate formatting/lint rules.
/** The native service reports readiness on /status; startWebService() returning does not. */async function waitForUpstream (port, deadlineMs = 20000) {
  • Files reviewed: 21/22 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Foundry routes variant ids but rejects the friendly alias outright, answering
"is not loaded" even while that exact model is resident. Verified against a
live service: the exact and versionless variant ids both return 200, the alias
returns 400 no matter what is loaded.

The alias is the form Flint own integration snippets tell users to configure,
so the gateway was autoloading the model and then replaying under the same
unroutable name -- spending the memory and still returning 400. It now replays
under the variant id the loader actually chose, and caches the mapping so later
requests are rewritten before they are sent rather than paying the rejection
every time. A stale mapping self-corrects through the normal not-loaded path.

An explicit variant id is still honoured exactly as sent: asking for the CPU
build is a hardware choice, not something to substitute.

Verified end to end -- alias cold 200 in 15.8s, warm 43ms, unknown model still
a clean 400 -- and mutation-tested: replaying the original body fails 3 tests,
disabling the learned rewrite fails 1.

Also corrects the comment claiming all three identifier forms route, and
records that EPs need ensureAccelerators before any CUDA variant can load.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b567a7ef-7eb9-4f06-ad74-3c781f5aa481
Copilot AI review requested due to automatic review settings August 31, 2026 04:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There are verified startup-state bugs in sidecar/foundry-sidecar.js (stale sharedEndpoint on failure and incorrect advertised endpoint when port=0) that can misreport service availability and/or return an invalid endpoint.

Review details

Suppressed comments (5)

Previously missed (4) — in code that hasn't changed since the last review.

sidecar/foundry-sidecar.js:1563

  • If startService fails after stopping the previous gateway/service, sharedEndpoint is left pointing at the old endpoint. That can make getEndpoint/getStatus report the service as running even though startup failed.
      const useGateway = payload.gateway !== false;
      await stopGateway();

      pool.clear();
      if (manager && typeof manager.stopWebService === 'function') {

sidecar/foundry-sidecar.js:1619

  • When payload.port is 0 (ephemeral port), the gateway will bind a real port and expose it via gateway.publicPort, but sharedEndpoint is built from payload.port, producing http://127.0.0.1:0/v1 and an incorrect advertised endpoint.
      // Client-facing endpoint stays on loopback even when the gateway is bound to a wider
      // interface, so this app and the Integrations snippets always target 127.0.0.1.
      sharedEndpoint = useGateway ? `http://127.0.0.1:${payload.port}/v1` : `${nativeUrl}/v1`;
      log('info', `Service started; bind=${bindAddr}:${payload.port} `

sidecar/foundry-sidecar.js:249

  • The JSDoc comment and async function waitForUpstream… are on the same line, which is hard to read and easy to miss during review/merges; split them onto separate lines.
/** The native service reports readiness on /status; startWebService() returning does not. */async function waitForUpstream (port, deadlineMs = 20000) {

src/routes/+page.svelte:4808

  • The BYOM modal doesn't move focus into the dialog when it opens. Without initial focus inside the modal, keyboard users may be stuck behind it and the Escape key handler may never fire; add an autofocus target inside the dialog (e.g. the close button).

This issue also appears on line 4958 of the same file.

                    <div class="modal-header">
                      <h3>Add a model folder</h3>
                      <button type="button" aria-label="Close" onclick={() => { byomOpen = false; resetByom(); }}><Icon name="x" size={14} /></button>
                    </div>

src/routes/+page.svelte:4961

  • The prompt-template editor modal also opens without focusing an element inside the dialog, which can break keyboard-only navigation and make the Escape handler unreliable; add an autofocus target inside the dialog (e.g. the close button).
                    <div class="modal-header">
                      <h3>Prompt template — {templateEditAlias}</h3>
                      <button type="button" aria-label="Close" onclick={() => { templateEditAlias = null; templateEdit = null; }}><Icon name="x" size={14} /></button>
                    </div>
  • Files reviewed: 21/22 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@joelst
joelst merged commit 6dac51a into main Aug 31, 2026
10 checks passed
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.

3 participants