fix(settings): Providers screen fetches models through /api/models, not the gateway (follow-up to #17) - #28
Closed
lightcloud00 wants to merge 1 commit into
Conversation
…rectly The Providers screen was the only client-side model consumer still calling the Hermes gateway from the browser: const HERMES_API_URL = process.env.HERMES_API_URL || 'http://127.0.0.1:8642' const response = await fetch(`${HERMES_API_URL}/v1/models`) That request carries no Authorization header, so the gateway returns 401 whenever API_SERVER_KEY is set and the Providers model list stays empty. It is also cross-origin, and in a container or remote deploy 127.0.0.1:8642 resolves on the viewer's machine rather than the gateway (vite inlines HERMES_API_URL at build time). Route it through Studio's own /api/models instead, as hermes-onboarding, settings-dialog, chat-screen, conductor-settings and routes/settings already do. That route authenticates the caller via the hermes-auth session cookie, attaches the gateway bearer server-side, and performs the same id/name/provider normalization this screen was doing by hand — so the hand-rolled normalization is deleted. Refs JPeetz#17
|
@lightcloud00 is attempting to deploy a commit to the Joerg Peetz's projects Team on Vercel. A member of the Team first needs to authorize it. |
Author
|
Closing this — we're continuing this work on our own repo (lightcloud00/Hermes-Studio) rather than upstream. Nothing here is abandoned; the branch and all its commits are preserved there. Thanks for the project, and apologies for the churn in your PR queue. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Not a duplicate of #19 — it's the other half
#19 by @coolswood fixes all eight files named in #17 and should merge first. This PR touches none of them. Its files are
hermes-jobs.ts,hermes-jobs.$jobId.ts,models.ts,hermes-runs.ts,hermes-runs.$runId.events.ts,approvals.$approvalId.approve.ts,approvals.$approvalId.deny.ts,hermes-proxy/$.ts. This PR changes one file:src/screens/settings/providers-screen.tsx.#17's symptom list includes "Model dropdown empty". #19 fixes the outbound leg — it puts the bearer on
/api/models's call to the gateway — which repairs the model pickers insettings-dialog,chat-screen,conductor-settings,hermes-onboardingandroutes/settings/index.But the Providers screen never calls
/api/models. It calls the gateway itself, from the browser:So after #19 merges, that one screen is still empty. This PR closes that gap. The two changes are complementary — neither fixes this screen alone.
Three separate failures in that one call
Authorizationheader. The gateway returns 401 wheneverAPI_SERVER_KEYis set.127.0.0.1:8642is the wrong machine.vite.config.tsinlinesHERMES_API_URLat build time, so a container build bakes in a container-internal address that resolves on the viewer's laptop.Verified live against a gateway with
API_SERVER_KEYset. On the base commit, the browser produces:Failure 2 firing before failure 1 even gets a chance.
The fix
Point it at
/api/models, as every other model consumer already does —hermes-onboarding.tsx:180,settings-dialog.tsx:1386,routes/settings/index.tsx:278,chat-screen.tsx:859,conductor-settings.tsx:165. Five to one; this screen was the outlier.That route already gates on
isAuthenticated(models.ts:131) so the browser call is authorized by thehermes-authsession cookie, and itsnormalizeHermesModel(models.ts:83-110) is field-for-field equivalent to the normalization this screen was doing by hand — sameid → name → modelprecedence, samename → display_name → label → id, sameprovider → owned_by → id.split('/')[0] → 'hermes-agent'. So the ~85 hand-rolled lines just go away;fetchModels()already declared the/api/modelsreturn shape.After: exactly one same-origin
GET /api/models, and no browser request to:8642from this screen.Two intentional behaviour deltas: the screen gains the auth-store models (Anthropic/OpenAI/xAI) the other five screens already show — consistency, not regression — and loses an unused bare-array
/v1/modelsbranch that no other consumer supports.Verification
Against a live gateway that 401s unauthenticated and 200s with the key:
GET :8642/v1/models→ CORS403+ERR_FAILED, list emptyGET /api/models→ served by Studio; zero browser requests to:8642from this screenGET /api/models → 200with the model list populatedThat last row is the point: this PR and #19 together fix the screen. I applied #19's
authHeaders()change tomodels.tslocally just to confirm it, and it is not part of this diff.tsc --noEmitclean (the deletion removes three symbols —HERMES_API_URL,HermesCatalogEntry,isHermesCatalogEntry, none referenced elsewhere). 190 tests pass. Lint is unchanged from the base commit — the one finding in this file exists identically before and after, shifted by the deletion.No test covers
fetchModelsupstream; adding one would need jsdom + MSW for a 13-line function, so I've left it — flagging that rather than implying coverage.Adjacent, deliberately not touched
chat-composer.tsx:121,244has the identical browser→gateway defect. fix: Hermes Studio compat with gateway v0.20 api_server #24 already fixes it by repointing to/api/hermes-proxy/v1/models, so I've left it alone. After this PR and fix: Hermes Studio compat with gateway v0.20 api_server #24, no client file fetches the gateway directly.vite.config.tsstill inlinesHERMES_API_TOKENinto client bundles. Nothing references it today so nothing actually leaks, but it means any future client file that typesprocess.env.HERMES_API_TOKENsilently ships the gateway key to every browser. Worth removing; separate two-line PR, and it would collide with fix: Hermes Studio compat with gateway v0.20 api_server #24'svite.config.tsrewrite if bundled here.Conflicts
None. #24 edits this same file but in different regions (imports, the config path, the render guard) — nearest hunk ends ~25 lines above the first line this PR touches. #19 and #22 don't touch it at all.
Refs #17rather than a closing keyword — #19's merge is what should close #17.🤖 Generated with Claude Code