fix(copilot): expose only account-available models

[hgaol]

Title

fix(copilot): expose only account-available models

Description

Summary

GitHub Copilot model discovery could expose models the connected account could not use.

This fixes both sources:

1. Provider response filtering

   • Policy-free models remain valid.
   • A present policy must be exactly enabled.
   • disabled, unconfigured, unknown, and malformed policies are excluded.
   • Existing picker, tool-call, and supported-wire requirements remain unchanged.

2. Catalog persistence

   • Copilot's filtered /models response is treated as the account's authoritative usable catalog.
   • The first account-scoped discovery replaces bootstrap fallback IDs with the available account models.
   • Later refreshes remove withdrawn IDs while preserving still-available user selections.
   • Newly introduced models remain opt-in after the initial discovery.
   • If an unavailable default must be replaced, reconciliation selects an available model.

The generic reconciliation behavior for other providers is unchanged: their model lists may be partial, so omitted user-selected IDs remain preserved.

Fixes#4697

Root cause

toGitHubCopilotModelInfo rejected only:

policy?.state==='disabled'

That admitted unconfigured models even though Maka cannot accept their policy terms.

Separately, reconcileConnectionAfterModelFetch intentionally preserved every previously enabled model. Interactive Copilot connections start with fallback IDs, so models absent from the account response were added back into the projected catalog.

Verification

The regressions fail without this change and pass with it:

• Provider-contract fixture with:
  • policy-free model;
  • enabled model;
  • disabled model;
  • unconfigured model;
  • hidden-picker model;
  • tool-incompatible model;
  • unsupported endpoint.
• Core authoritative-reconciliation tests:
  • initial fallback replacement;
  • withdrawn model removal;
  • still-available user choice preservation;
  • newly introduced model remains opt-in.
• Runtime Policy integration test proving unavailable bootstrap IDs are removed from persisted Copilot state.

Checks run:

• Provider conformance matrix: 126 passed
• @maka/core: 782 passed
• @maka/storage: 1107 tests, 0 failures
• @maka/runtime: 3176 tests, 0 failures
• Full workspace build: passed
• Full workspace typecheck: passed
• Lint: passed
• Format check: passed
• ASF source-header check: passed

The focused regressions and full typecheck/lint/format were rerun after merging the latest main.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: pi with an OpenAI model assisted diagnosis, primary-source comparison against GitHub's Copilot client, implementation, test creation, and verification. The human contributor remains responsible for the change.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — GitHub Copilot model lists now omit models that are unavailable to the connected account.
• No

[Astro-Han]

Reviewed exact head dba7211ec39a99c12481a77a497ed6f9d68b8a29. I found no P0–P3 issues.

What the diff actually does (7 files, +139/−17, two commits):

1. packages/runtime/src/model-fetcher.ts — the Copilot discovery filter now admits a model only when policy is absent or exactly { state: 'enabled' } (isGitHubCopilotModelPolicyEnabled). unconfigured, disabled, malformed, and unknown states are excluded, failing closed. The existing picker, tool-call, and supported-endpoint requirements are unchanged.
2. packages/core/src/llm-connections.ts — reconcileConnectionAfterModelFetch gains an authoritative option: the first account-scoped fetch replaces bootstrap fallback ids with the fetched catalog, and later refreshes keep only user-enabled ids still present in the catalog, so newly introduced models stay opt-in.
3. packages/storage/src/runtime-policy/connection-catalog-document.ts — the single production reconcile call site sets authoritative only for github-copilot, so generic providers keep the existing preserve-everything behavior.
4. scripts/release-cli-package.mjs — the production dependency audit retries once on transient 5xx and transport errors; real vulnerability reports still fail closed.

What I checked:

• I read the full diff plus the surrounding reconcile logic, including alias mapping (supersededModelId) and the hasModelInventory derivation (previous.models.length > 0; the catalog codec forbids a non-empty models array before discovery, so the first Copilot fetch always takes the replace-bootstrap branch).
• Red-green per layer: with each changed file reverted to the base commit, the new core reconciliation test, the provider conformance matrix, and the storage runtime-policy integration test fail; with the PR applied all three pass. Local results on this head: core 14/14, provider contract matrix 126/126, storage runtime-policy stores 69/69.
• Hosted checks on this exact head are green: test, four platform addon builds, immutable tarball, four Validate-installed-CLI jobs, and Qualify released State Roots. The Validate-installed-CLI Eval job shows as skipped in a non-required matrix cell.
• No database schema change; the reconcile option is call-time only and never persisted.

Two observations, neither blocking:

• When every user-enabled model has been withdrawn but the account catalog is still non-empty, the reconciler force-enables the first catalog entry and moves the default to it (packages/core/src/llm-connections.ts:507, branch at :498). That sits in tension with the adjacent comment that newly introduced models stay opt-in (:502); the counter-argument is that it keeps the connection usable instead of leaving zero enabled models. That trade-off deserves a deliberate product decision at some point.
• An account whose entire catalog is policy-gated now fetches zero models, so a refresh (and onboarding verify) fails with the generic invalid_response error class (packages/runtime-host/src/server/connection-effect-coordinator.ts:158, :195). Failing closed is the right direction; the error just does not tell the user it is a policy or entitlement issue.

Housekeeping: the PR body declares a substantive generative-tool contribution, but neither commit carries a Generated-by: trailer. With the repository's squash-merge message setting, such a trailer would be lost at merge time regardless, which is worth noting for whoever merges.

This is not an approval; the merge decision stays with the human reviewers.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[hgaol]

Reviewed exact head dba7211ec39a99c12481a77a497ed6f9d68b8a29. I found no P0–P3 issues.

What the diff actually does (7 files, +139/−17, two commits):

1. packages/runtime/src/model-fetcher.ts — the Copilot discovery filter now admits a model only when policy is absent or exactly { state: 'enabled' } (isGitHubCopilotModelPolicyEnabled). unconfigured, disabled, malformed, and unknown states are excluded, failing closed. The existing picker, tool-call, and supported-endpoint requirements are unchanged.
2. packages/core/src/llm-connections.ts — reconcileConnectionAfterModelFetch gains an authoritative option: the first account-scoped fetch replaces bootstrap fallback ids with the fetched catalog, and later refreshes keep only user-enabled ids still present in the catalog, so newly introduced models stay opt-in.
3. packages/storage/src/runtime-policy/connection-catalog-document.ts — the single production reconcile call site sets authoritative only for github-copilot, so generic providers keep the existing preserve-everything behavior.
4. scripts/release-cli-package.mjs — the production dependency audit retries once on transient 5xx and transport errors; real vulnerability reports still fail closed.

What I checked:

• I read the full diff plus the surrounding reconcile logic, including alias mapping (supersededModelId) and the hasModelInventory derivation (previous.models.length > 0; the catalog codec forbids a non-empty models array before discovery, so the first Copilot fetch always takes the replace-bootstrap branch).
• Red-green per layer: with each changed file reverted to the base commit, the new core reconciliation test, the provider conformance matrix, and the storage runtime-policy integration test fail; with the PR applied all three pass. Local results on this head: core 14/14, provider contract matrix 126/126, storage runtime-policy stores 69/69.
• Hosted checks on this exact head are green: test, four platform addon builds, immutable tarball, four Validate-installed-CLI jobs, and Qualify released State Roots. The Validate-installed-CLI Eval job shows as skipped in a non-required matrix cell.
• No database schema change; the reconcile option is call-time only and never persisted.

Two observations, neither blocking:

• When every user-enabled model has been withdrawn but the account catalog is still non-empty, the reconciler force-enables the first catalog entry and moves the default to it (packages/core/src/llm-connections.ts:507, branch at :498). That sits in tension with the adjacent comment that newly introduced models stay opt-in (:502); the counter-argument is that it keeps the connection usable instead of leaving zero enabled models. That trade-off deserves a deliberate product decision at some point.
• An account whose entire catalog is policy-gated now fetches zero models, so a refresh (and onboarding verify) fails with the generic invalid_response error class (packages/runtime-host/src/server/connection-effect-coordinator.ts:158, :195). Failing closed is the right direction; the error just does not tell the user it is a policy or entitlement issue.

Housekeeping: the PR body declares a substantive generative-tool contribution, but neither commit carries a Generated-by: trailer. With the repository's squash-merge message setting, such a trailer would be lost at merge time regardless, which is worth noting for whoever merges.

This is not an approval; the merge decision stays with the human reviewers.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

Resolved and added generated-by

[Astro-Han]

Head update note: this PR moved from dba7211ec3 to 5f0997ff53 after the review below was posted. The new head shares both the tree and the merge-base (26f31deafa) with the reviewed head — the pushed commit only changed the parent, not the content — so every conclusion in the earlier review applies verbatim to 5f0997ff53.

One unrelated but concrete reason to rebase before merging: main has advanced ~71 commits past the merge-base, and three of the seven files this PR touches were modified in the meantime (packages/core/src/llm-connections.ts — most recently via #4526 — and scripts/release-cli-package.mjs, three commits). That exposure is identical for both heads and does not change the review conclusions, but the overlap is worth resolving in a rebase.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Astro-Han]

Head update note: the PR head advanced to f06bf036 via a merge of upstream/main into this branch (the two author commits are unchanged ancestors). The effective diff against the current base still touches the same seven files, and its content lines are byte-identical to the previously reviewed diff — only hunk offsets shifted with the base — so the conclusions in the earlier review and the head-update note above apply verbatim to f06bf036.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Astro-Han]

Re-reviewed at f06bf03 (a merge of main into the branch; the seven files are still +139/−17 against the merged base) against main (a5a99a633), clean merge.

The Copilot half stands. /models already carries policy.state and the fetcher already dropped disabled (model-fetcher.ts:526), so treating unconfigured the same is the producer's call; the reconcile change is limited to github-copilot, whose fallbackModels are what kept bootstrap ids reappearing after discovery; a failed or empty fetch returns failed before anything is written (connection-effect-coordinator.ts:155), so users never lose their current models. Generated-by trailers are present now. Two items from last time are unchanged and stay P3: forcing the first catalogue entry on when nothing is enabled (llm-connections.ts:505-507), and a whole catalogue blocked by policy reporting as invalid_response rather than as an authorization problem.

P2 (path ②): the second commit, the npm audit retry in scripts/release-cli-package.mjs:300-325, does not do what it says and does not belong here. When the registry is down, npm audit --json prints {"error":{"code":"E503","summary":…}}; there is no top-level statusCode or message, and audit.error?.code is only set when the spawn itself fails. So transient is never true on the case the commit targets, and the warning prints undefined. The repository already has the audit authority with an unavailable outcome and --allow-unavailable (scripts/audit-shipped-dependencies.mjs, which parses report.error.summary); this adds a second transience classifier on the older --omit=dev path. Drop the commit from this PR. If the release path needs to tolerate a registry outage, that is a change to the existing authority in its own PR.

P3:

• connection-catalog-document.ts:473 hard-codes previous.providerType === 'github-copilot' at the call site, next to modelIdAliasesForProvider, which is the existing seam for provider facts. A providerReportsCompleteModelCatalog(providerType) in model-metadata keeps the fact in one place.
• The removal is permanent: a model an org sets to unconfigured for a week comes back into models but not into the enabled set, and a changed default is not restored. Worth saying in the body as the accepted trade, or remembering "was enabled, now absent" ids.
• #4697 is your own issue with a source-level repro; a real /models response from an account with unconfigured entries in the body would settle the semantics.

Evidence boundary: static read; no live Copilot response was inspected; the npm audit error shape is from npm's documented JSON output and the repository's own parser, not from a reproduced outage.

AI-assisted review: drafted with Maka; I verified the retry predicate, the audit authority's unavailable path and the reconcile scope myself.