Skip to content

Follow-up: deferred review findings from PR #650 #653

Description

@philcunliffe

Deferred from the review of PR #650 ("An upstream-less ai-gateway source idles instead of throwing"), triaged non-blocking at c39f71c.

1. A partial upstream loss is still silent

compileUpstreams drops any entry missing name or base_url, per entry. PR #650 makes the all-dropped case visible via the gateway_idle_no_upstreams warning, but a partial loss is still invisible: with 2 upstreams configured and 1 valid, mergeUpstreams is non-empty, the proxy binds, listening is true, and the diagnostic only fires when listening === false.

So a user who typo'd one of two upstreams gets a working gateway that silently routes nothing for that provider. The per-entry silent drop predates #650 (only its docstring changed there) and issue #649 was specifically about the all-empty throw, so this is a pre-existing blind spot rather than a regression.

Likely shape: compare the raw configured count against the compiled route count and warn when they differ, which would subsume the current all-dropped case rather than duplicating it.

2. The "idle rather than throw" decision has no LLP

PR #650 establishes that an upstream-less gateway idles instead of failing. That is a real behavioural decision, and the review confirmed the old "at least one upstream" invariant appeared nowhere in llp/ (it lived only in proxy.js). Per CLAUDE.md's living-docs expectation this is worth a short decision LLP citing 0119/0120, so the next reader finds the rationale where they look for it rather than in a PR thread.

Worth a human's judgement, not a patch

The design trades a hard failure ([failed] / degraded) for a warning that deliberately does not flip overall, consistent with the existing gateway_port_fallback precedent. The practical consequence: external tooling that gates on overall === 'healthy' rather than reading diagnostics will not notice a fully broken gateway-only install, where before the change it would have crashed loudly. That is inherent to the chosen approach and was weighed against the LLP corpus, but it is a product-policy call rather than a code defect, so it belongs in front of a person.

Backlink: PR #650.

Metadata

Metadata

Assignees

No one assigned

    Labels

    neutral:fixDelegate this issue to neutral for an autonomous fix attempt (reproduce -> fix -> PR)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions