fix(web): hide deleted providers with prototype keys

[Lucenx9]

What Changed

• Check provider instance settings maps for own properties before applying them.
• Treat removed custom and fork-provider snapshots as disabled while keeping valid prototype-named instances configurable.
• Add regression coverage for inherited keys and the built-in legacy fallback.

Why

Plain object lookups can resolve inherited values such as Object.prototype.constructor. A deleted provider with that instance ID was therefore treated as configured and could remain visible until the next provider snapshot.

Own-property checks match the settings invariant without restricting valid provider slugs.

UI Changes

No visual design changes. This only removes stale deleted provider entries during settings reconciliation.

Checklist

• This PR is small and focused
• I explained what changed and why
• I included before/after screenshots for any UI changes (not applicable: no visual changes)
• I included a video for animation/interaction changes (not applicable)

Built with GPT-5.6 Sol in the Codex harness through T3 Code.

---

Note

Low Risk
Narrow change to client-side settings overlay for provider picker visibility; no auth or server persistence changes.

Overview
Fixes stale provider entries staying visible in the picker when a deleted instance ID collides with inherited object keys (e.g. constructor, toString).

applyProviderInstanceSettings now uses Object.hasOwn for both providerInstances and legacy providers before reading settings. Removed custom instances (including fork defaults with no legacy row) are forced disabled when absent from settings; legacy providers.*.enabled applies only to built-in defaults that actually have an own legacy entry. Explicitly configured instances with prototype-like names still honor their settings.

Regression tests cover inherited-key deletion, valid prototype-named instances, fork-driver defaults, and built-in legacy fallback.

^{Reviewed by Cursor Bugbot for commit bdeb3b5. Bugbot is set up for automated code reviews on this repo. Configure here.}

Note

Fix deleted providers with prototype keys in applyProviderInstanceSettings

• applyProviderInstanceSettings in providerInstances.ts now uses Object.hasOwn before reading providerInstances and legacyProviders, so prototype members like constructor or toString are no longer treated as configured instances or legacy provider entries.
• Default instances without an explicit config or a real legacy entry now default to disabled instead of preserving the streamed enabled state.
• Risk: default instances absent from both providerInstances and legacyProviders will now be disabled rather than kept enabled; reviewers should check providerInstances.ts defaulting logic for any callers relying on the old pass-through behavior.

^{Macroscope summarized bdeb3b5.}

Summary by CodeRabbit

• Bug Fixes
  • Corrected provider instance enablement when custom instances use reserved names such as constructor or toString.
  • Removed custom and forked default instances are now correctly treated as disabled while preserving default status.
  • Legacy provider settings that disable built-in instances are now honored consistently.
  • Default instances without legacy configuration no longer retain an unintended enabled state.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 36e7885e-e141-4e8a-8128-6d3157ce0614

📥 Commits

Reviewing files that changed from the base of the PR and between a3a8cbd and bdeb3b5.

📒 Files selected for processing (2)

• apps/web/src/providerInstances.test.ts
• apps/web/src/providerInstances.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

applyProviderInstanceSettings now uses own-property checks for instance and legacy provider settings. Missing default-instance settings resolve to disabled. Tests cover prototype-key IDs, removed instances, default flags, and legacy settings.

Changes

Provider instance settings

Layer / File(s)	Summary
Guard provider instance settings lookups apps/web/src/providerInstances.ts	applyProviderInstanceSettings uses Object.hasOwn for instance and legacy provider lookups. Default instances without legacy entries resolve to disabled.
Validate instance resolution edge cases apps/web/src/providerInstances.test.ts	Tests cover constructor and toString IDs, removed instances, preserved isDefault flags, and legacy providers settings.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to bdeb3

This narrowly fixes stale deleted provider entries during settings reconciliation while preserving valid configured names; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: juliusmarminge, vitalyiegorov

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main fix: hiding deleted providers whose IDs match prototype keys.
Description check	✅ Passed	The description explains what changed, why the change is needed, the lack of UI changes, and includes completed checklist items. It also documents the regression coverage and behavior changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[Lucenx9]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[macroscopeapp]

Approvability

Verdict: Approved at bdeb3b5

Macroscope's review found this PR approvable — This is a narrowly scoped web bug fix that hardens provider settings lookups against inherited prototype keys and adds focused regression tests. Its runtime impact is limited to provider visibility and enablement reconciliation, with no schema, deployment, security, billing, or authentication changes.

^{You can add or adjust custom eligibility rules. Learn more.}

[Lucenx9]

Review: verified correct ✅

Reviewed the diff locally at bdeb3b5 and traced every production call path. Approving the fix logic; one non-blocking note below.

The bug is real

settings.providerInstances?.[entry.instanceId] resolved inherited keys like Object.prototype.constructor, so a deleted custom instance named after a prototype member stayed visible until the next provider snapshot reconciled it. Same failure class existed on the legacy providers lookup for fork-driver default instances (legacyProviders[entry.driverKind] where driverKind === "constructor").

The fix is right

• Object.hasOwn correctly guards both maps without restricting valid instance slugs — a custom instance genuinely named constructor still resolves its own settings row.
• The tightened default branch (entry.isDefault && legacyProvider ? ... : false) matches the documented invariant: only built-in defaults own a legacy providers.* entry, so absence there means a stale snapshot and "disabled" is correct, including for fork drivers that exist solely through providerInstances.
• No regression risk for built-ins: their legacy keys always exist as own properties after schema decode (withDecodingDefault({})), so the new hasOwn check always passes for them.

Call-site safety checked

All four production callers pass the full decoded settings object, never a hand-built partial:

• ChatView.tsx:2788
• ChatComposer.tsx:824
• SettingsPanels.tsx:1878
• ProjectSettingsPanel.tsx:423

The atom fallback (primaryServerConfigAtom?.settings ?? DEFAULT_SERVER_SETTINGS, state/server.ts:74) guarantees both maps are present at runtime, so Object.hasOwn(undefined, ...) cannot throw.

Non-blocking note

The old ?. tolerated caller-built partials missing the settings fields entirely; the new code would throw on those. The types already forbid this shape and no caller constructs one, so no change needed — just worth knowing if a future caller passes an ad-hoc object.

Verification

• vitest run apps/web/src/providerInstances.test.ts → 34/34 pass
• tsc --noEmit -p apps/web → clean
• Web-only module (apps/web/src); mobile surface untouched, correctly
• CI green: Check, Test ×3, Macroscope, Bugbot

---

Reviewed with GLM 5.3 Flash (Z.ai) in the opencode harness.