An upstream-less ai-gateway source idles instead of throwing (#649) - #650
An upstream-less ai-gateway source idles instead of throwing (#649)#650philcunliffe wants to merge 3 commits into
Conversation
A picker run selecting `hermes` and nothing else composed
`@hypaware/ai-gateway` with `{ upstreams: [] }` and then failed at source
start with "ai-gateway: at least one upstream must be configured before
start", so a reachable first-run choice produced a broken install.
The gateway plugin does two separable jobs: at activation it contributes
the `ai_gateway_messages` dataset and the shared
`ai_gateway.projected_exchange` materializer, and at source start it runs
the proxy. `@hypaware/hermes` wants the first alone. It is "never
modified, configured, or proxied" (LLP 0119) so it contributes no
`gateway_upstream`, yet the materializer is a hard `requires.plugins`
dependency (LLP 0120) so its picker row must still compose the gateway
plugin.
So the source now treats an empty compiled routing table as idle: no
listener bound, `state.listen` left unset (`localEndpoint()` keeps
throwing rather than handing an attach a URL nothing is listening on),
`status()` reporting `listening: false` with a reason. A `reload()` that
brings an upstream binds normally, so idling is recoverable.
Co-Authored-By: Claude <noreply@anthropic.com>
…us (#650 review) Letting an upstream-less `ai-gateway` source idle rather than fail its start is right for the config that wants it (hermes composes the plugin for its materializer alone, LLP 0120), but the same path swallowed a real misconfiguration. A gateway-only install whose TOML says `url =` where `base_url` was meant has every entry dropped by `compileUpstreams` without complaint, and `diagnoseV1Config`'s `gateway_missing_*_upstream` check does not fire for that shape. On master that install read `[failed]` / `degraded`; on this branch it read `[started]` / `healthy` while the user's client got ECONNREFUSED, with the reason living only in `status.message`, which the daemon snapshot never captures. `hyp status` now emits a non-degrading `gateway_idle_no_upstreams` warning when the gateway reports `listening: false` and a non-empty `details.upstreams`. That field is the raw configured names, pre-compile, so it discriminates exactly: a hermes-only install publishes `[]` and stays quiet and healthy, a dropped or misspelled upstream publishes `['anthropic']` and warns, naming the entry and the field that drops it. Also: `aigw.idle_no_upstreams` logs at `warn` with the configured count and names when the config listed upstreams and compiled to none, staying at `info` for the genuine hermes-only case; and `status()` reads the context `reload()` handed it rather than the boot-time closure, so `details.upstreams` describes the config in force (pre-existing, but the new diagnostic reads that field). Tests: the diagnostic fires for the dropped-upstream shape and not for the hermes-only one, and never degrades `overall`; a reload that removes every upstream tears a live listener down and idles without error, which ends capture for already-attached clients and is now pinned deliberately. Co-Authored-By: Claude <noreply@anthropic.com>
Neutral review round 1 -
|
| master | this PR before the fix | |
|---|---|---|
| source | failed |
started |
| daemon | degraded |
healthy |
hyp status |
[failed] + recent-errors bump |
clean, recent errors 0 |
The user's client gets ECONNREFUSED with nothing in HypAware saying why. That cuts against LLP 0114#fallback-is-visible, which is annotated three lines above the change.
Fixed with a non-degrading warning diagnostic gateway_idle_no_upstreams, gated on details.listening === false and a non-empty raw configured-upstream list. The discrimination was verified against the real source before being relied on:
typo'd entry -> upstreams: ["anthropic"], listening: false -> warns
hermes-only -> upstreams: [], listening: false -> quiet
Raw configured names survive compileUpstreams dropping the entry, so the two shapes separate exactly. Overall health stays healthy for hermes, asserted.
Two LOWs fixed alongside: aigw.idle_no_upstreams now logs at warn with the configured count and names when the config listed upstreams but the compiled table came out empty (info retained for the genuine hermes case), and the reverse reload() direction is now pinned, since a reload that removes upstreams tears down a live listener and silently ends capture for attached clients. A pre-existing staleness bug was also adopted: status() read the closure ctx rather than the reloaded one, which the new diagnostic depends on; reverting that one line now fails a test.
Verification
The new status-gateway-idle.test.js fails first: with the fix stashed, the two positive assertions fail while all three negative controls pass, which is the right shape. npm test 3584 pass / 0 fail / 1 skip, tsc clean, status_diagnostics, gateway_claude_capture and gateway_codex_capture all ok. The regression test composes a hermes-only config through the real composePickerConfig over the real bundled manifests and starts the real source, so a manifest edit reintroducing the shape fails it.
client_attach_idempotent fails, and is pre-existing: it fails identically on clean origin/master with the same assertion. Not from this PR.
review 2) The `gateway_idle_no_upstreams` warning gated on `details.upstreams`, the raw configured names, so it could not see the config it most needed to see. `compileUpstreams` drops an entry for a missing `name` exactly as silently as for a missing `base_url`, and a nameless entry contributes no name at all, so `provider = "anthropic", base_url = "..."` published `upstreams: []` and read as hermes-only: `healthy`, no diagnostic, ECONNREFUSED at the client. The repo's own `diagnoseV1Config` affirms that config, because it matches an upstream by its `provider` field. The gateway source already computed the count (its idle log is a `warn` on the strength of it); it just never published it. It now does, as `details.upstreams_configured`, and core gates on the count instead of the names. The names still ride along and still make the message concrete when they exist; a status file from a build that predates the count falls back to counting the names, so an older daemon's dropped `base_url` stays visible. Hermes-only (0 configured) stays quiet and healthy, and the diagnostic remains non-degrading. The repair lines pointed at `hyp config validate`, which prints `config ok` and exits 0 for exactly this config: `@hypaware/ai-gateway` registers no config section, so nothing checks upstream shape. Replaced with the edit that fixes it, naming the config file and both required keys, followed by the restart that makes it take effect. Co-Authored-By: Claude <noreply@anthropic.com>
Neutral review round 2 -
|
| shape | warns | overall |
|---|---|---|
hermes-only, no upstreams key |
no | healthy |
upstreams: [] |
no | healthy |
missing base_url |
yes (named) | healthy |
missing name |
yes (count-based) | healthy |
name: "" |
yes | healthy |
non-array upstreams |
no, no crash | healthy |
| listening gateway | no | healthy |
| stopped daemon, stale snapshot | no | healthy |
Also confirmed clean in round 2: degradingKinds is untouched so overall cannot flip; status consumers are kind-agnostic; the activeCtx change was exercised across three reloads with traffic and leaves no stale or double-applied state; and gatewaySourceRawDetails was diffed against master's gatewaySourceDetails across 17 probes with identical output.
New tests fail first: on 693deb1 the run was 7 pass / 3 fail. After: npm test 3591 pass / 0 fail / 1 skip, tsc clean, status_diagnostics and gateway_claude_capture ok.
Note on client_attach_idempotent: it fails on origin/master too, so it is not from this PR, but round 2 narrowed it usefully: it passes at 1fbfa0d (v1.19.0) and fails at 7116f95. That is a real regression with a bisect range, filed separately.
Triaged at
|
The bug, re-confirmed against the real modules
A picker run selecting
hermesand nothing else composes@hypaware/ai-gatewaywith{ upstreams: [] }, and starting that source rejects with:Confirmed two ways before touching anything:
startProxy({ upstreams: [], ... })throws directly, and the two new tests below fail on master with exactly that message.Which direction, and why the other two are wrong
The issue offered three. Two of them are ruled out by Accepted LLPs, so this was settled by the corpus rather than by taste.
@hypaware/ai-gatewaydoes two separable jobs. At activation it contributes theai_gateway_messagesdataset and the sharedai_gateway.projected_exchangematerializer. At source start it runs the proxy. Hermes wants the first and only the first:gateway_upstreamis correct by design, not an oversight.requires.plugins: [\"@hypaware/ai-gateway\"]in its manifest; the materializer is a hard dependency."Grepping
hermes/src/confirms the code matches: it touches the gateway only as the source of the materializer and theai_gateway_messagesdataset, never a proxy, an upstream preset, a gateway client, or gateway state.Direction 1 (drop
requires_gatewayfrom the hermes manifest) is wrong, and not merely suboptimal. Composing a hermes-only config without@hypaware/ai-gatewayleaves hermes's hard dependency unsatisfied. Run against the realresolveDependenciesand the real bundled manifests:Hermes is eliminated and never activates. That trades a loud start failure for an install that silently captures nothing, and it contradicts LLP 0120.
Direction 2 (omit the gateway when no picked row contributes an upstream) fails identically, for the same reason and by the same evidence: hermes needs the plugin present but contributes no upstream, so that rule removes exactly the plugin hermes depends on. It would also mis-handle presets, since adapter plugins add upstreams at activation, after composition has already run.
Direction 3 is the only one consistent with both Accepted docs, so that is what this PR does. Nothing in
llp/settles "the gateway must have at least one upstream"; that invariant lived only inproxy.jsand was never a recorded decision, which is why no doc had to change here. This restores the behavior LLPs 0119 and 0120 already imply rather than altering settled design, so no new LLP is minted. If the team wants the idle-gateway state recorded as a decision in its own right, that would be a follow-up doc.The fix
The decision moved to the source layer, which is the layer that knows an upstream-less gateway is a legitimate config.
startProxykeeps its invariant: binding a listener that can route nothing is still a bug at that level.launchListenernow compiles the routing table first (config upstreams merged with adapter presets) and, if it is empty, binds nothing and returnsundefined:state.listenstays unset, solocalEndpoint()keeps throwing rather than handing an attach a URL nothing is listening on. Callers already handle that throw, andgatewaySourceDetailsindaemon/status.jsalready reports "no reachable gateway" when no port is recorded, so no downstream reader needed changing.status()reportsstate: 'ready'(an idle gateway is not an error) withlistening: falseand a message saying why.reload()re-runs the same path, so idling is recoverable: adding an upstream binds a listener with no daemon restart.Also corrected a doc comment in
config.jsthat claimed missingupstreamsis "rejected loudly", which was untrue before this change and actively misleading after it.Regression tests
Both are in
test/plugins/ai-gateway-source.test.jsand both fail on master (verified by revertingsource.jsto master with the tests in place, giving# pass 4 / # fail 2, each failing withai-gateway: at least one upstream must be configured before start) and pass after:the gateway source a hermes-only picker run composes starts, idlecomposes the hermes-only config through the realcomposePickerConfigover the real bundled manifests, then starts the real gateway source with the slice it produced. This ties picker composition to source start, so a manifest edit that reintroduces the shape fails here rather than drifting.an idle gateway binds once a reload brings an upstreamproves idle is not a dead end, and covers the newproxy?.stop()path.Checks
node scripts/run-tests.js: 3576 passed, 0 failed, 1 skipped (3577 total)npx tsc -p tsconfig.json --noEmit: cleannpm run smoke -- walkthrough_picker_to_first_query: ok (picker composition touched)gateway_claude_capture,gateway_codex_capture,status_diagnostics: okclient_attach_idempotent: fails, but pre-existing on clean master and unrelated (an attach error-message assertion); verified by stashing this change and re-running.Fixes #649