History

fix(web): resolve sidebar provider icons from the thread's own environment (#7292)

Vitaly Iegorov committed 2 months ago

Commit f3fcfe11 parent 18f6d033 files · +96 −11Browse files

Unified · Split

3 files
  1. apps/web/src/components/Sidebar.tsx +20 −11
  2. apps/web/src/providerInstances.test.ts +49 −0
  3. apps/web/src/providerInstances.ts +27 −0
apps/web/src/components/Sidebar.tsx+20−11
@@ -165,11 +165,10 @@ import { ProjectFavicon } from "./ProjectFavicon"; · 164 unmodified lines
165import { ProviderInstanceIcon } from "./chat/ProviderInstanceIcon";165import { ProviderInstanceIcon } from "./chat/ProviderInstanceIcon";
166import { getTriggerDisplayModelLabel } from "./chat/providerIconUtils";166import { getTriggerDisplayModelLabel } from "./chat/providerIconUtils";
167import {167import {
168 deriveProviderInstanceEntries,168 deriveProviderEntriesByEnvironment,
169 shouldShowInstanceBadge,169 shouldShowInstanceBadge,
170 type ProviderInstanceEntry,170 type ProviderInstanceEntry,
171} from "../providerInstances";171} from "../providerInstances";
172import { primaryServerProvidersAtom } from "../state/server";
173import { useThreadRunningTerminalIds } from "../state/terminalSessions";172import { useThreadRunningTerminalIds } from "../state/terminalSessions";
174import { stackedThreadToast, toastManager } from "./ui/toast";173import { stackedThreadToast, toastManager } from "./ui/toast";
175import { Button } from "./ui/button";174import { Button } from "./ui/button";
@@ -247,6 +246,8 @@ function WorkingDuration(props: { startedAt: string | null }) { · 71 unmodified lines
247 );246 );
248}247}
249 248
249const EMPTY_PROVIDER_ENTRIES: ReadonlyMap<string, ProviderInstanceEntry> = new Map();
250
250function terminalProcessLabel(count: number): string {251function terminalProcessLabel(count: number): string {
251 return `${count} terminal ${count === 1 ? "process" : "processes"} running`;252 return `${count} terminal ${count === 1 ? "process" : "processes"} running`;
252}253}
@@ -1862,15 +1863,18 @@ export default function Sidebar() { · 1609 unmodified lines
1862 () => sortLogicalProjectsForSidebar(unsortedProjectGroups, threads, sidebarProjectSortOrder),1863 () => sortLogicalProjectsForSidebar(unsortedProjectGroups, threads, sidebarProjectSortOrder),
1863 [sidebarProjectSortOrder, threads, unsortedProjectGroups],1864 [sidebarProjectSortOrder, threads, unsortedProjectGroups],
1864 );1865 );
1865 const serverProviders = useAtomValue(primaryServerProvidersAtom);1866 const serverConfigs = useAtomValue(environmentServerConfigsAtom);
1866 const providerEntryByInstanceId = useMemo(1867 // Threads on non-primary environments (T3 Connect, hosted) resolve their
1868 // provider entry from their own environment's config: default instance ids
1869 // are driver slugs, so a flat map would collide across environments.
1870 const providerEntriesByEnvironment = useMemo(
1867 () =>1871 () =>
1868 new Map(1872 deriveProviderEntriesByEnvironment(
1869 deriveProviderInstanceEntries(serverProviders).map(1873 [...serverConfigs].map(
1870 (entry) => [entry.instanceId as string, entry] as const,1874 ([environmentId, config]) => [environmentId, config.providers] as const,
1871 ),1875 ),
1872 ),1876 ),
1873 [serverProviders],1877 [serverConfigs],
1874 );1878 );
1875 const projectCwdByKey = useMemo(1879 const projectCwdByKey = useMemo(
1876 () =>1880 () =>
@@ -1991,7 +1995,6 @@ export default function Sidebar() { · 114 unmodified lines
1991 // the partition works directly off live shells: no archived-snapshot1995 // the partition works directly off live shells: no archived-snapshot
1992 // merging, no optimistic holds. Archived threads remain hidden here —1996 // merging, no optimistic holds. Archived threads remain hidden here —
1993 // archive keeps its original "remove from sidebar" meaning.1997 // archive keeps its original "remove from sidebar" meaning.
1994 const serverConfigs = useAtomValue(environmentServerConfigsAtom);
1995 const {1998 const {
1996 pinnedThreads,1999 pinnedThreads,
1997 reorderablePinnedKeys,2000 reorderablePinnedKeys,
@@ -3598,7 +3601,10 @@ export default function Sidebar() { · 1600 unmodified lines
3598 ) ?? null3601 ) ?? null
3599 }3602 }
3600 environmentLabel={environmentLabelById.get(thread.environmentId) ?? null}3603 environmentLabel={environmentLabelById.get(thread.environmentId) ?? null}
3601 providerEntryByInstanceId={providerEntryByInstanceId}3604 providerEntryByInstanceId={
3605 providerEntriesByEnvironment.get(thread.environmentId) ??
3606 EMPTY_PROVIDER_ENTRIES
3607 }
3602 isHighlighted={activeSearchResultIndex === index}3608 isHighlighted={activeSearchResultIndex === index}
3603 isRouteActive={routeThreadKey === threadKey}3609 isRouteActive={routeThreadKey === threadKey}
3604 resultId={`sidebar-thread-search-result-${index}`}3610 resultId={`sidebar-thread-search-result-${index}`}
@@ -3707,7 +3713,10 @@ export default function Sidebar() { · 102 unmodified lines
3707 `${thread.environmentId}:${thread.projectId}`,3713 `${thread.environmentId}:${thread.projectId}`,
3708 ) ?? null3714 ) ?? null
3709 }3715 }
3710 providerEntryByInstanceId={providerEntryByInstanceId}3716 providerEntryByInstanceId={
3717 providerEntriesByEnvironment.get(thread.environmentId) ??
3718 EMPTY_PROVIDER_ENTRIES
3719 }
3711 timestampFormat={timestampFormat}3720 timestampFormat={timestampFormat}
3712 onThreadClick={handleThreadClick}3721 onThreadClick={handleThreadClick}
3713 onThreadActivate={navigateToThread}3722 onThreadActivate={navigateToThread}
apps/web/src/providerInstances.test.ts+49−0
@@ -2,6 +2,7 @@ import { ProviderDriverKind, ProviderInstanceId, type ServerProvider } from "@t3 · 1 unmodified line
2import { describe, expect, it } from "vite-plus/test";2import { describe, expect, it } from "vite-plus/test";
3import {3import {
4 applyProviderInstanceSettings,4 applyProviderInstanceSettings,
5 deriveProviderEntriesByEnvironment,
5 deriveProviderInstanceEntries,6 deriveProviderInstanceEntries,
6 getDefaultProviderInstanceModel,7 getDefaultProviderInstanceModel,
7 isProviderInstancePickerReady,8 isProviderInstancePickerReady,
@@ -17,6 +18,7 @@ function provider(input: { · 9 unmodified lines
17 enabled?: boolean;18 enabled?: boolean;
18 availability?: ServerProvider["availability"];19 availability?: ServerProvider["availability"];
19 displayName?: string;20 displayName?: string;
21 accentColor?: string;
20 status?: ServerProvider["status"];22 status?: ServerProvider["status"];
21 models?: ServerProvider["models"];23 models?: ServerProvider["models"];
22}): ServerProvider {24}): ServerProvider {
@@ -24,6 +26,7 @@ function provider(input: { · 1 unmodified line
24 instanceId: ProviderInstanceId.make(input.instanceId),26 instanceId: ProviderInstanceId.make(input.instanceId),
25 driver: input.provider,27 driver: input.provider,
26 ...(input.displayName ? { displayName: input.displayName } : {}),28 ...(input.displayName ? { displayName: input.displayName } : {}),
29 ...(input.accentColor ? { accentColor: input.accentColor } : {}),
27 enabled: input.enabled ?? true,30 enabled: input.enabled ?? true,
28 installed: true,31 installed: true,
29 version: null,32 version: null,
@@ -132,6 +135,52 @@ describe("deriveProviderInstanceEntries", () => { · 102 unmodified lines
132 });135 });
133});136});
134 137
138describe("deriveProviderEntriesByEnvironment", () => {
139 it("keeps same-id default instances distinct per environment", () => {
140 const byEnvironment = deriveProviderEntriesByEnvironment([
141 [
142 "local",
143 [
144 provider({
145 provider: ProviderDriverKind.make("claude"),
146 instanceId: "claude",
147 displayName: "Claude Local",
148 accentColor: "#112233",
149 }),
150 ],
151 ],
152 [
153 "remote",
154 [
155 provider({
156 provider: ProviderDriverKind.make("claude"),
157 instanceId: "claude",
158 displayName: "Claude Remote",
159 accentColor: "#445566",
160 }),
161 ],
162 ],
163 ]);
164
165 expect(byEnvironment.get("local")?.get("claude")?.displayName).toBe("Claude Local");
166 expect(byEnvironment.get("local")?.get("claude")?.accentColor).toBe("#112233");
167 expect(byEnvironment.get("remote")?.get("claude")?.displayName).toBe("Claude Remote");
168 expect(byEnvironment.get("remote")?.get("claude")?.accentColor).toBe("#445566");
169 });
170
171 it("never falls back to another environment's instances", () => {
172 const byEnvironment = deriveProviderEntriesByEnvironment([
173 ["local", [provider({ provider: ProviderDriverKind.make("codex"), instanceId: "codex" })]],
174 ["empty", []],
175 ]);
176
177 expect(byEnvironment.get("empty")?.get("codex")).toBeUndefined();
178 // Every environment gets its own bucket, so an absent lookup is a real
179 // "this environment has no such instance", not a missing key.
180 expect(byEnvironment.get("empty")?.size).toBe(0);
181 });
182});
183
135describe("resolveSelectableProviderInstance", () => {184describe("resolveSelectableProviderInstance", () => {
136 it("returns the requested instance when it is enabled and available", () => {185 it("returns the requested instance when it is enabled and available", () => {
137 const requested = ProviderInstanceId.make("claude_work");186 const requested = ProviderInstanceId.make("claude_work");
apps/web/src/providerInstances.ts+27−0
@@ -200,6 +200,33 @@ export function deriveProviderInstanceEntries( · 199 unmodified lines
200 });200 });
201}201}
202 202
203/**
204 * Project several environments' `ServerProvider[]` into a nested
205 * `environmentId → instanceId → entry` lookup.
206 *
207 * Instance ids are per-environment routing keys, and `defaultInstanceIdForDriver`
208 * makes the default id literally the driver slug, so every environment running
209 * the same driver reports the same id. Flattening across environments would
210 * clobber entries and mis-resolve accent colors; lookups must stay scoped to
211 * the thread's own environment.
212 */
213export function deriveProviderEntriesByEnvironment(
214 providersByEnvironment: Iterable<readonly [string, ReadonlyArray<ServerProvider>]>,
215): ReadonlyMap<string, ReadonlyMap<string, ProviderInstanceEntry>> {
216 const byEnvironment = new Map<string, ReadonlyMap<string, ProviderInstanceEntry>>();
217 for (const [environmentId, providers] of providersByEnvironment) {
218 byEnvironment.set(
219 environmentId,
220 new Map(
221 deriveProviderInstanceEntries(providers).map(
222 (entry) => [entry.instanceId as string, entry] as const,
223 ),
224 ),
225 );
226 }
227 return byEnvironment;
228}
229
203/**230/**
204 * Overlay the current settings configuration onto streamed provider snapshots.231 * Overlay the current settings configuration onto streamed provider snapshots.
205 * Provider probes can briefly retain their previous `enabled` value after a232 * Provider probes can briefly retain their previous `enabled` value after a