A partially dropped gateway upstream is visible in status, and LLP 0193 records why the gateway idles (#653) - #658
A partially dropped gateway upstream is visible in status, and LLP 0193 records why the gateway idles (#653)#658philcunliffe wants to merge 6 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 <[email protected]>
…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 <[email protected]>
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 <[email protected]>
`compileUpstreams` drops an upstream entry missing `name` or `base_url`, per entry and silently. #650 made the all-dropped case visible; a partial loss stayed invisible, because the proxy binds, `listening` is never set, and the diagnostic only fired when it was `false`. A user who typo'd one of two upstreams got a working gateway that routed nothing for that provider. The gateway source now publishes `upstreams_dropped` (raw configured entries minus compiled routes) and the names it can attribute, and warns at boot when the routing table is smaller than the config asked for. `hyp status` reads the difference: one comparison, two mutually exclusive kinds - `gateway_idle_no_upstreams` when nothing survived, `gateway_upstreams_dropped` when some did. Both non-degrading, following the `gateway_port_fallback` precedent. Also mints LLP 0193, recording the "idle rather than throw" decision #650 established (which lived only in `proxy.js` and a PR thread) and its accepted visibility tradeoff. Co-Authored-By: Claude <[email protected]>
PR #643 (branch openclaw-cli-backend-denylist) is open and older, and it already claims both 0193 and 0194. Master's highest is 0192, so scanning only master made 0193 look free. The minting rule takes the next number past the highest across the default branch and every branch, so this PR yields and moves to 0195, the first number free everywhere. Mechanical: the file, its title line, and the four @refs that point at it. No prose or decision content changes. Co-Authored-By: Claude <[email protected]>
…names The non-idle status message claimed traffic for a dropped upstream is "not proxied and nothing is captured" unconditionally, but mergeUpstreams backfills a registered adapter preset (e.g. @hypaware/claude's `anthropic`) whenever its name is not already in the compiled config table, so a typo'd override can still route through the preset's default endpoint. Reword to hedge on that case, and fix the message's hardcoded plural to match the idle branch's configured/upstream(s) treatment (reachable at dropped === configured === 1 exactly via the preset-backfill path). Also dedupe readConfiguredUpstreams' droppedNames, which could print a name twice when two same-named config entries both fail to compile, and split the LLP 0195 @ref on gateway_upstreams_dropped so the "does not flip overall" half cites #consequences instead of #visible-when-unintended, which never says that. Co-Authored-By: Claude <[email protected]>
Review round 1 -
|
Review round 2 -
|
philcunliffe
added
the
neutral:approved
- Renumber llp/0195-setup-ends-on-a-question -> 0198: 0195 collides with PR #658's fix/issue-653 doc, 0196/0197 are taken by PR #668. Updates all 28 @ref sites plus the LLP 0135 Extended-by block. - ask.js askableClients: a successful status probe reporting zero attached clients is evidence of detachment, not grounds to fall back to every launchable client on PATH. Only a thrown probe now falls through. - ask.js --list: launchable claim now reflects actual resolvability instead of being hardcoded true. - ask.js docblock/comment: four suggested prompts, not five. - Add test/core/commands/ask.test.js covering askableClients' probe success/throw branches and the no-launcher/--list exit-code contract. - Add two firstLookHadRows cases (slow, absent/error) to wizard/index.test.js pinning the two mutants the review found. - Add a no-preauth assertion to client-launch-manifest.test.js: bundled launch args never carry --allowedTools or --dangerously-skip-permissions. Co-Authored-By: Claude <[email protected]>
1 participant
Stacks on #650
This branch is built on
fix/issue-649(PR #650) and must merge after it. Its first two commits are #650's; only1c83444is new here. #650 introducedlistening: falseanddetails.upstreams_configured, and item 1 below is a direct extension of exactly that machinery, so building standalone on master would have meant either duplicating #650 or conflicting with it. It also would have made half the required test coverage unwritable: on master a fully dropped upstream set throws at source start, so "a full loss still warns" has nothing to warn about. Once #650 merges, this PR's diff collapses to the single commit.Item 3 of the issue (should
overallflip for a fully broken gateway?) is not touched: it is reserved for a human. See the note at the bottom, which this work sharpens rather than answers.1. A partial upstream loss is now visible
compileUpstreamsdrops any entry missingnameorbase_url, per entry and silently. With two upstreams configured and one valid the routing table is non-empty, the proxy binds,listeningis never set, and #650's diagnostic (which gates onlistening === false) never fires. The user gets a gateway that works for one provider and silently routes nothing for the other. Nothing else catches it either:@hypaware/ai-gatewayregisters no config section sohyp config validateprintsconfig ok, anddiagnoseV1Config'sgateway_missing_*_upstreamcheck matches onprovider, which a broken entry still has.Source side.
readConfiguredUpstreamsnow runs the realcompileUpstreamsover the raw entries and reports the difference, rather than re-deriving the "does this entry survive?" predicate (a second copy would drift from the table the proxy is actually built with).status().detailsgainsupstreams_dropped(always, including as0, so core can tell a healthy gateway from a status file written by an older build by the field's presence) andupstreams_dropped_nameswhen any dropped entry had a usable name.launchListeneralso logsaigw.upstreams_droppedatwarnon the bound path, because the daemon log is where an operator looks first when one provider's rows never arrive.Status side. One check,
gatewayDroppedUpstreams, replacesgatewayIdleWithConfiguredUpstreams: it reads configured-vs-dropped once and covers both shapes of the fault.Subsumed the detection, kept the kinds distinct
The issue asked me to evaluate subsuming the all-dropped case into a single count comparison. I subsumed the detection and kept two
kinds, chosen by whether the gateway is bound:gateway_idle_no_upstreams(unchanged, from An upstream-less ai-gateway source idles instead of throwing (#649) #650) - nothing survived, no listener at all, every client gets connection refused.gateway_upstreams_dropped(new) - the proxy is bound and looks entirely healthy; traffic for the dropped upstream is unrouted and uncaptured.There is now exactly one place that decides whether upstreams were lost, so the duplication the issue was worried about does not exist. But collapsing to a single kind would have cost real specificity in both directions. "The gateway is running but listening on nothing ... clients will get connection refused" is true only of the total loss and is the sentence an operator staring at a dead gateway needs; it is actively wrong for the partial case, where the gateway is up. And consumers gate on
kind: calling a listening gatewayidlewould make the kind a lie about the one thing its name asserts, and would merge "refuses every connection" with "quietly misroutes one provider" into a single alert. The two branches are mutually exclusive by construction (a gateway is either bound or it is not), so they can never double-report.Back-compat: a status file written before
upstreams_droppedexisted can still answer the all-dropped question (idle plus a configured count), and it is deliberately not used to guess at a partial loss - guessing there would warn about every install whose adapter presets outnumber its config entries. Such a file starts reporting properly the moment the daemon restarts on this build.2. LLP 0193
llp/0193-upstream-less-gateway-idles.decision.md(Decision, Accepted) records what #650 established and had nowhere to live. It cites LLP 0119 and 0120 as the pair that rules out the alternatives (hermes needs the gateway plugin for the shared materializer, and must never be proxied, so it composes the plugin while contributing no upstream), states that the "at least one upstream" invariant moves to the source rather than disappearing (startProxystill keeps it: binding a listener that can route nothing is still a bug at that layer), and records the visibility half above under#visible-when-unintended.Its Consequences section records the accepted tradeoff explicitly, including that the warning is non-degrading and what that costs. Two
@refs point at it:launchListener(#idle-not-throw) and the status diagnostic (#visible-when-unintended).Regression tests
All fail before the change. Reverting the four non-test files with the tests in place gives 7 failing (
# pass 22 / # fail 7); all 29 pass after.test/core/status-gateway-idle.test.jsdrives the real gateway source and the realcompileUpstreams, feeds the details it publishes intocollectHypAwareStatus, and asserts:gateway_upstreams_dropped, message counts the loss and namesopenaibut not the healthyanthropic,overallstayshealthy, andgateway_idle_no_upstreamsdoes not also fire.nameat all -> still warns, off the count.gateway_idle_no_upstreams, keeping its own "listening on nothing" / "connection refused" wording, andgateway_upstreams_droppeddoes not fire.upstreams: []) -> silent and healthy, through the same comparison.upstreams_dropped-> does not guess at a partial loss.test/plugins/ai-gateway-source.test.jscovers the boot log and the published details directly: theaigw.upstreams_droppedwarn and its attributes on a partial drop, and that a fully valid config logs nothing and reportsupstreams_dropped: 0with no empty name list riding along in every healthy status file.Health is not degraded by either warning, and that is asserted in three places.
Checks
node scripts/run-tests.js: 3599 passed, 0 failed, 1 skipped (3600 total)npx tsc -p tsconfig.json --noEmit: cleannpm run smoke -- status_diagnostics: okclient_attach_idempotentis failing on master pending client_attach_idempotent: the capability gate reports the enablement layer (LLP 0174) #654; unrelated and untouched here.For the human, not decided here (issue item 3)
The reserved question was whether
overall === 'healthy'should stay unflipped for a fully broken gateway. This work sharpens it rather than answering it: there are now two non-degrading gateway warnings, and they are not equally severe.gateway_upstreams_droppedon a gateway routing three providers and missing a fourth is clearly a working install and clearly should not degrade.gateway_idle_no_upstreamson a gateway-only install is a captureless one. If the answer is ever "flip it", the split above is the seam to flip along - the kinds are already distinct, andgateway_upstreams_droppedshould almost certainly stay non-degrading either way.Fixes #653