fix: defer to Hermes runtime config when provider inference is ambiguous#51
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines provider resolution so the adapter defers to Hermes runtime configuration when the detected Hermes config is ambiguous or not representable by the adapter (e.g., unsupported/custom providers), and reduces false hermes_no_api_keys warnings when Hermes config already includes model.api_key.
Changes:
- Extend Hermes config detection to track
model.api_keypresence (as a boolean) and treat base URL / API mode as “runtime signals”. - Update
resolveProvider()to return"auto"(defer to Hermes) when a matching Hermes config uses an unsupported provider or omits provider while still providing runtime signals. - Add regression tests covering unsupported-provider, provider-omitted runtime config, and API-key warning suppression paths; add
npm run testscript.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/custom-provider.test.mjs | Adds regression tests for provider-resolution edge cases and API-key warning suppression. |
| src/server/test.ts | Updates environment checks to suppress false missing-key warnings and to report Hermes runtime deferral cases. |
| src/server/execute.ts | Passes additional detected runtime signals into resolveProvider() for safer provider selection. |
| src/server/detect-model.ts | Parses model.api_key presence and expands resolveProvider() logic to defer to Hermes runtime when appropriate. |
| package.json | Adds a test script that builds then runs Node’s test runner. |
| package-lock.json | Updates package version metadata to 0.3.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
alvarosanchez
left a comment
There was a problem hiding this comment.
Re-validated this PR on branch fix/hermes-config-runtime-provider-detection. Verified the current head resolves the Copilot concerns already discussed: provider-omitted messaging is explicit, Hermes config is detected once per testEnvironment run, and the regression helper now sets/restores HOME/USERPROFILE/HOMEDRIVE/HOMEPATH for portability. Local verification:
hermes-paperclip-adapter@0.3.0 test
npm run build && node --test tests/*.test.mjs
hermes-paperclip-adapter@0.3.0 build
tsc
✔ parseModelFromConfig tracks api_key presence without exposing the raw secret (0.863893ms)
✔ resolveProvider does not fall through to model inference when Hermes config provider is unsupported but matches the requested model (0.55459ms)
✔ resolveProvider also defers to Hermes runtime when the matching config omits provider but includes runtime signals (0.113644ms)
✔ resolveProvider still infers from the requested model when Hermes config is for a different model (0.928111ms)
✔ testEnvironment does not warn about missing API keys when Hermes config provides a supported provider api_key (21.096891ms)
✔ testEnvironment describes provider-omitted runtime config without inventing provider "auto" (13.815219ms)
✔ testEnvironment does not warn about missing API keys when Hermes config provides a custom provider base_url and api_key (12.350976ms)
ℹ tests 7
ℹ suites 0
ℹ pass 7
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 103.923124 passes (7/7). No additional code changes were necessary from this pass.
Summary
~/.hermes/config.yamlentry uses runtime config the adapter cannot safely representhermes_no_api_keyswarnings when the matching Hermes config already includesmodel.api_keyTest Plan
npm run testnpm run typecheck