fix(connect): honor upstream model restrictions - #255
Open
andya1lan wants to merge 3 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the DEVIN_CONNECT model discovery + routing pipeline to treat a successful non-empty upstream Connect catalog as authoritative (instead of snapshot ∪ live), preventing stale snapshot-only selectors/aliases from remaining discoverable or routable after upstream restrictions omit them.
Changes:
- Make selector existence/alias validity depend on the authoritative live catalog after first successful sync (snapshot becomes cold-start/failed-sync fallback).
- Update
/v1/modelsand Dashboard Connect views to (a) synthesize live-only selectors, (b) dedupe rows by resolved upstream selector, and (c) suppress paid-downgrade warnings during read-only discovery. - Enforce per-account routing boundaries by requiring a paid account’s own live catalog to contain the selector (mixed pools still discover the union).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/mutations/dashboard-connect-parity.json | Updates mutation anchors/expectations to match the revised Dashboard + models reachability/dedupe behavior. |
| test/models-live-catalog.test.js | Adds coverage for deduping live canonical selectors already represented by aliases. |
| test/devin-connect-models.test.js | Adds coverage for retiring snapshot-only aliases once live catalog proves the target is absent. |
| test/dashboard-models-connect-parity.test.js | Adds parity coverage for Dashboard inclusion of live-only selectors and dedupe vs alias-backed selectors; adds deterministic offline seams. |
| test/connect-discovery-rebuild.test.js | Adds coverage for “live is authoritative” and suppressing downgrade warnings during discovery. |
| test/connect-catalog-delatch.test.js | Adds coverage that routing stays within each account’s own live catalog contribution. |
| src/handlers/models.js | Makes /v1/models Connect discovery authoritative, deduped by resolved selector, and live-catalog-synthesized. |
| src/devin-connect-models.js | Introduces authoritative-live selector existence, validates aliases against it, and adds warnOnFallback control to suppress discovery-time warnings. |
| src/dashboard/api.js | Mirrors /v1/models live-only selector synthesis and dedupe in the Dashboard Connect models view. |
| src/auth.js | Enforces per-account routing: paid tier is necessary but not sufficient; selector must also be in that account’s live catalog once available. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Author
|
Fixed copilot's comment, should be able for review. |
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.
Summary
/v1/modelsby resolved upstream selector.Why a follow-up is needed after #234
#234 correctly separated the Connect and Cascade namespaces and made the Dashboard and
/v1/modelsshare the same reachability predicate.However, selector existence was still defined as:
That works when the upstream catalog only adds selectors. It does not work when upstream restrictions cause a non-empty live catalog to omit selectors.
In that case, entries retained by the frozen snapshot remain classified as known even though the upstream catalog no longer exposes them. This can cause:
/v1/modelsto advertise selectors omitted by upstream restrictions;The views introduced by #234 therefore agree with each other, but can still agree on stale catalog data.
Fix
After a successful non-empty catalog synchronization:
Empty responses never replace the last known-good catalog.
Verification
/v1/modelscontains the live selector set plus the universal free fallback./v1/models./v1/modelsproduces no paid-request downgrade warnings.Follow-up to #234.