fix(copilot): hide unavailable and unconfigured models - #4753
fix(copilot): hide unavailable and unconfigured models#4753seekskyworld wants to merge 3 commits into
Conversation
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed exact head db68937eaac43b109e788590a255ca8ae3e20a3e. The Copilot-only filtering is directionally correct, but two reachable cases still break the authoritative-catalog contract: an all-unavailable response cannot be committed, and a nullable policy crashes discovery. The required test check is also currently failing because the two changed test files are not Biome-formatted.
Local verification: clean npm 11.19 install, full build:test, Runtime 3,196 passed / 13 skipped, Storage 1,110 passed / 10 skipped, focused Runtime Host 29/29, focused Core 20/20, lint, ASF headers, diff check, and a clean patch-preserving merge onto current main (7743890729b26a7e95f653af8b93ccbe42ad480f). I did not call the live Copilot service; provider behavior was exercised through deterministic local HTTP fixtures.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
|
The two inline findings are addressed in commit
Added Host, storage, protocol, and provider-contract regressions. Validation: Runtime provider matrix 139/139, storage focused tests 9/9, Host protocol 4/4, Host coordinator 30/30, Biome, |
d0f90d1 to
5cca2f9
Compare
|
Rebased the PR onto current The new hosted CI run |
5cca2f9 to
7b1e03d
Compare
Signed-off-by: seekskyworld <djh1813553759@gmail.com> Generated-by: Codex
Signed-off-by: seekskyworld <djh1813553759@gmail.com>
Signed-off-by: seekskyworld <djh1813553759@gmail.com>
7b1e03d to
0aab913
Compare
|
Rebased onto current |
|
The hosted |
hqhq1025
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 0aab913765d8c7452e816d85a6fa3e4e63e7cf2c. The two previously reported defects are fixed: empty filtered Copilot catalogs can now commit, and nullable/malformed policy values no longer crash discovery. One first-login correctness issue remains: a valid non-empty account catalog can be persisted with no enabled model and no default. The branch is also currently conflicting with main, where #4697 has already been closed by #4698, so it needs to be reconciled with that implementation before merge.\n\nVerification: clean dependency install and patch application; build:test; Runtime 3,310 passed / 13 skipped; Storage 1,188 passed / 10 skipped; focused Runtime Host 115/115; focused Runtime/Storage suites; typecheck, lint, format, ASF headers, and diff check. The full Runtime Host run had 1 failure in the managed Bash sandbox assertion; the same failure reproduces on the exact base 00f2c9e0, so it is not attributable to this PR. I did not call the live Copilot service; the account-catalog behavior was exercised through the real Runtime -> Host coordinator -> file-backed Storage path with a deterministic HTTP response.\n\n> Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| const isGitHubCopilot = previous.providerType === 'github-copilot'; | ||
| const liveModelIds = isGitHubCopilot ? new Set(result.models.map(({ id }) => id)) : undefined; | ||
| const enabledModelIds = isGitHubCopilot | ||
| ? reconciled.enabledModelIds.filter((modelId) => liveModelIds?.has(modelId)) |
There was a problem hiding this comment.
[P1] Seed the first authoritative catalog instead of emptying a new connection. A newly created interactive Copilot connection starts with shipped fallback IDs in enabledModelIds but models: []. When its first valid /models response contains only account models outside that fallback list, the generic reconciler preserves the fallback selection and this filter removes all of it. Through the real OAuth-authority -> Host coordinator -> file-backed Storage path, a one-model response committed with modelCount: 1 and models: [account-only-model], but persisted enabledModelIds: [] and defaultTarget: null. synchronizeRuntimeHostAccountConnectionById then has no enabledModelIds[0] to select, so login reports success while the connection cannot start a session until manually edited. Distinguish the first account-scoped fetch (previous.models.length === 0) from later refreshes: seed the live catalog on the first fetch, while preserving intersection-only behavior once an inventory already exists.
Summary
Fixes #4697
GitHub Copilot model discovery now treats the account
/modelsresponse as authoritative: policy-free andenabledmodels remain visible,disabledandunconfiguredmodels are excluded, and refreshes drop bootstrap fallback ids that are no longer in the live catalog without auto-selecting a replacement.Verification
npm exec -- tsx --test packages/runtime/src/__tests__/provider-contract-overrides.tsnpm exec -- tsx --test packages/storage/src/__tests__/runtime-policy-model-facts.test.tsAI use
Tool(s) and scope:
Checklist
Does this PR entail a change in behavior?