fix(providers): keep running threads alive when renaming providers

[Gigioxx]

What Changed

Renaming or recoloring a provider keeps its live instance and sessions, while refreshed snapshots publish the new presentation. Runtime edits, disabling, and resetting warn when that instance has running threads. Deletion always asks for confirmation.

Why

The registry compared the whole settings envelope and closed the instance scope for cosmetic edits, so renaming a provider killed in-flight work in every thread using it. Compare runtime fields separately and update presentation in place. Normalize the legacy enabled flag during hydration so a default provider's first rename preserves its instance too.

Fixes #11043.

Scope and approval

Maintainer-triaged bug: triage comment on #11043 (labels bug, accepted). Its suggested fix has three parts, and this PR does exactly those: keep the instance for presentation-only edits, warn before delete and other rebuilds when the instance has active threads, and add a registry test that a display-name-only reconcile keeps the live instance.

Verification

• Established the problem from the triage repro on main: renaming a provider with running threads tore down every live session on that instance. The new registry regression test failed before the fix.
• Manual check before the rebase, in an isolated web client: two Codex mock subprocess turns survived rename and color changes and completed through normal provider events. Canceling runtime edits or deletion preserved the provider; confirmed deletion removed it.
• After rebasing onto 0cf482b08b (Claude Opus 5.5, this session; resolved conflicts in the registry test and settings panel):
  • vp test run src/provider/Layers/ProviderInstanceRegistryLive.test.ts src/provider/Layers/CodexProvider.test.ts (apps/server): 17/17 passed.
  • vp test run src/components/settings/ProviderSettingsPanel.environment.test.tsx (apps/web): 11/11 passed.
  • tsc --noEmit in packages/contracts: no errors. The apps/server and apps/web typechecks did not finish within 20 minutes on this heavily loaded machine, so they were not completed after the rebase; CI covers them. Before the rebase, all three passed.
  • vp lint on the changed files: no errors (one existing warning). vp fmt --check: clean.
• Not checked: native desktop and mobile shells, real provider binaries, and a browser re-run after the rebase.

UI Changes

Before: Delete immediately removes the provider. After: confirmation explains the effect on running threads.

Before	After

Before recording · Rename, color, and cancellation recording

Checklist

• This PR is small and focused
• I explained what changed and why
• I included before/after screenshots for any UI changes
• I included a video for animation/interaction changes

Implemented with GPT-6 in Codex. Rebase by Claude Opus 5.5 via Claude Code.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: b4874802-60ef-43ef-98a3-bd4fadec278a

📥 Commits

Reviewing files that changed from the base of the PR and between b647041 and fc71bc7.

📒 Files selected for processing (4)

• apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts
• apps/server/src/provider/Layers/ProviderRegistry.ts
• apps/web/src/components/settings/ProviderSettingsPanel.environment.test.tsx
• apps/web/src/components/settings/ProviderSettingsPanel.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

Provider runtime configuration comparisons now distinguish runtime fields from presentation fields. Live instances retain resources after presentation edits and are replaced when runtime configuration changes or an entry is removed. Provider settings request confirmation before applicable changes.

Changes

Provider lifecycle

Layer / File(s)	Summary
Runtime configuration and legacy hydration packages/contracts/src/providerInstance.ts, apps/server/src/provider/Layers/ProviderInstanceRegistryHydration.ts	Adds shared runtime configuration equality and maps legacy enabled values to the envelope level.
Live instance reconciliation apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts, apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts	Retains live instances for presentation edits, updates presentation data dynamically, replaces instances for runtime changes, and adds lifecycle coverage.
Registry snapshot propagation apps/server/src/provider/Layers/ProviderRegistry.ts	Snapshots all registered instances, including retained instances.
Provider change confirmations apps/web/src/components/settings/ProviderSettingsPanel.tsx, apps/web/src/components/settings/ProviderSettingsPanel.environment.test.tsx	Requests confirmation for provider updates, deletion, and applicable default resets. Tests cover cancellation and asynchronous actions.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: High

Suggested reviewers: juliusmarminge, t3dotgg, stienswout

Merge Risk: ⚪ Minimal · up to fc71b

Presentation edits preserve live provider instances, while disruptive changes request confirmation. No actionable merge-blocking issue is established; merge after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to fc71b

Preserving sessions during presentation edits is appropriately separated from runtime changes. However, delayed confirmations retain an earlier settings snapshot, creating a risk that accepting an action overwrites concurrent changes, including provider disablement. No expanded cross-environment access was established.

Retained concerns

• Medium · reliability · inferred: Confirmation-dependent mutations construct whole-provider-map patches from settings captured before awaiting user approval. A concurrent disable or runtime configuration change may therefore be overwritten when a pending action is accepted, causing control drift within the selected environment. Server conflict rejection was not established.

Security review details

Security Blast Radius

• inferred — The identified concurrency risk is bounded to provider settings in the captured environment, but a whole-map patch can affect providers other than the instance named in the confirmation. The traced path does not establish cross-environment authority expansion or new privileges.

Security Findings and Attack Paths

• inferred — A pending provider action can retain an earlier enabled configuration while another settings writer disables that provider. Accepting the pending action then submits the older configuration as part of its map patch. This is a possible control rollback, not an established authorization bypass; server conflict handling remains unresolved.

Trust Boundaries and Controls

• observed — Confirmation is a client-side lifecycle guard, not the authorization boundary. Declining returns before mutation. Accepted changes retain the environment-targeted settings path, whose inspected server handler delegates to the settings service and redacts returned settings. The underlying authorization implementation was not fully inspected.

Resilience and Maintainability Implications

• inferred — Snapshot correlation checks instance ID and driver, not generation. A read from a replaced instance may temporarily reach status aggregation or persistence. This correlation limitation predates the PR, although the new retained-instance pass adds reads. Serialized synchronization, buffered registry changes, and authoritative maintenance lookup mitigate lasting effects; a worsened sensitive outcome was not established.

Hardening Proposals

• proposed — Use instance-scoped mutations with revision checks, or reject and re-confirm actions when the relevant settings changed during confirmation. This would protect concurrent disablement and runtime configuration changes without replaying unrelated provider entries.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Issue #11043 requires active work to survive provider renames and requires a warning before deletion when active threads exist. The registry now compares runtime configuration separately from presenta…
Out of Scope Changes check	✅ Passed	The changes stay within issue #11043. Registry reconciliation, snapshot refresh, legacy hydration, and focused tests implement stable provider identity and uninterrupted live work. Runtime-edit, reset…
Title check	✅ Passed	The title clearly and concisely describes the primary fix: keeping running threads alive when providers are renamed.
Description check	✅ Passed	The description explains the problem, the implementation, scope approval, focused verification, limitations, and UI evidence. It uses “What Changed” and “Why” instead of the template’s “Change” and “P…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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

[Jardo-51]

I left a use case on #11043 that this PR does not cover yet: #11043 (comment)

In short: one of my provider instances has an environment variable holding access cookies that expire. Refreshing them means editing the variable, and that edit stops every thread running on the instance, including agents that never use the cookies.

With this PR an environment edit counts as a runtime edit, so it still stops the running threads, now behind a confirmation. That is better than a silent kill, but I am left with the same two options: wait for everything to finish before editing, or restart the threads afterwards.

Would it be possible for environment edits to let running sessions finish on the environment they were started with, and apply the new values only to sessions started after the edit? A provider process cannot pick up a changed environment after spawn anyway, so stopping it gains nothing.

I understand if that is out of scope here, since it means keeping the old instance alive next to its replacement. In that case it would help to know whether it is worth a follow-up.

[macroscopeapp]

Could you add a focused ProviderRegistry test that reconciles a presentation-only edit and asserts getProviders publishes the updated name/color while retaining the instance subscription? The new snapshot-all-instances path is needed specifically for this behavior, but the added test only checks snapshots on ProviderInstanceRegistry directly. No code diff is appropriate because the fix belongs in a separate test file.

^{Posted via Macroscope — Effect Service Conventions}

[juliusmarminge]

Thanks for working on this. We merged the orchestrator V2 rewrite in #2829, and we are closing this PR as part of that transition.

This PR guards provider renames by inspecting thread.session.status and session.providerInstanceId. The V2 client read model no longer exposes that session lifecycle; refile the guard against current runtime/run state.

Sorry for the extra work this creates. If the change is still needed on V2, please rebuild it on current main, verify it there, and open a new PR linking back here. We're closing the current implementation without assuming the underlying request is resolved.