Remove saved environments atomically

mwolson · mwolson · commit 5f1fe13e489a · 2026-05-09T01:08:25.000-04:00
diff --git a/apps/desktop/src/ipc/DesktopIpcHandlers.ts b/apps/desktop/src/ipc/DesktopIpcHandlers.ts
@@ -5,6 +5,7 @@ import { getClientSettings, setClientSettings } from "./methods/clientSettings.t
 import {
   getSavedEnvironmentRegistry,
   getSavedEnvironmentSecret,
+  removeSavedEnvironment,
   removeSavedEnvironmentSecret,
   setSavedEnvironmentRegistry,
   setSavedEnvironmentSecret,
@@ -52,6 +53,7 @@ export const installDesktopIpcHandlers = Effect.gen(function* () {
   yield* ipc.handle(setClientSettings);
   yield* ipc.handle(getSavedEnvironmentRegistry);
   yield* ipc.handle(setSavedEnvironmentRegistry);
+  yield* ipc.handle(removeSavedEnvironment);
   yield* ipc.handle(getSavedEnvironmentSecret);
   yield* ipc.handle(setSavedEnvironmentSecret);
   yield* ipc.handle(removeSavedEnvironmentSecret);
diff --git a/apps/desktop/src/ipc/channels.ts b/apps/desktop/src/ipc/channels.ts
@@ -16,6 +16,7 @@ export const GET_CLIENT_SETTINGS_CHANNEL = "desktop:get-client-settings";
 export const SET_CLIENT_SETTINGS_CHANNEL = "desktop:set-client-settings";
 export const GET_SAVED_ENVIRONMENT_REGISTRY_CHANNEL = "desktop:get-saved-environment-registry";
 export const SET_SAVED_ENVIRONMENT_REGISTRY_CHANNEL = "desktop:set-saved-environment-registry";
+export const REMOVE_SAVED_ENVIRONMENT_CHANNEL = "desktop:remove-saved-environment";
 export const GET_SAVED_ENVIRONMENT_SECRET_CHANNEL = "desktop:get-saved-environment-secret";
 export const SET_SAVED_ENVIRONMENT_SECRET_CHANNEL = "desktop:set-saved-environment-secret";
 export const REMOVE_SAVED_ENVIRONMENT_SECRET_CHANNEL = "desktop:remove-saved-environment-secret";
diff --git a/apps/desktop/src/ipc/methods/savedEnvironments.ts b/apps/desktop/src/ipc/methods/savedEnvironments.ts
@@ -39,6 +39,16 @@ export const setSavedEnvironmentRegistry = makeIpcMethod({
   }),
 });
 
+export const removeSavedEnvironment = makeIpcMethod({
+  channel: IpcChannels.REMOVE_SAVED_ENVIRONMENT_CHANNEL,
+  payload: EnvironmentId,
+  result: Schema.Void,
+  handler: Effect.fn("desktop.ipc.savedEnvironments.removeEnvironment")(function* (environmentId) {
+    const savedEnvironments = yield* DesktopSavedEnvironments.DesktopSavedEnvironments;
+    yield* savedEnvironments.removeEnvironment(environmentId);
+  }),
+});
+
 export const getSavedEnvironmentSecret = makeIpcMethod({
   channel: IpcChannels.GET_SAVED_ENVIRONMENT_SECRET_CHANNEL,
   payload: EnvironmentId,
diff --git a/apps/desktop/src/preload.ts b/apps/desktop/src/preload.ts
@@ -41,6 +41,8 @@ contextBridge.exposeInMainWorld("desktopBridge", {
     ipcRenderer.invoke(IpcChannels.GET_SAVED_ENVIRONMENT_REGISTRY_CHANNEL),
   setSavedEnvironmentRegistry: (records) =>
     ipcRenderer.invoke(IpcChannels.SET_SAVED_ENVIRONMENT_REGISTRY_CHANNEL, records),
+  removeSavedEnvironment: (environmentId) =>
+    ipcRenderer.invoke(IpcChannels.REMOVE_SAVED_ENVIRONMENT_CHANNEL, environmentId),
   getSavedEnvironmentSecret: (environmentId) =>
     ipcRenderer.invoke(IpcChannels.GET_SAVED_ENVIRONMENT_SECRET_CHANNEL, environmentId),
   setSavedEnvironmentSecret: (environmentId, secret) =>
diff --git a/apps/desktop/src/settings/DesktopSavedEnvironments.test.ts b/apps/desktop/src/settings/DesktopSavedEnvironments.test.ts
@@ -293,6 +293,26 @@ describe("DesktopSavedEnvironments", () => {
     ),
   );
 
+  it.effect("removes saved environment metadata and its embedded secret atomically", () =>
+    withSavedEnvironments(
+      Effect.gen(function* () {
+        const savedEnvironments = yield* DesktopSavedEnvironments.DesktopSavedEnvironments;
+        yield* savedEnvironments.setRegistry([savedRegistryRecord]);
+        yield* savedEnvironments.setSecret({
+          environmentId: savedRegistryRecord.environmentId,
+          secret: "bearer-token",
+        });
+
+        yield* savedEnvironments.removeEnvironment(savedRegistryRecord.environmentId);
+
+        assert.deepEqual(yield* savedEnvironments.getRegistry, []);
+        assert.isTrue(
+          Option.isNone(yield* savedEnvironments.getSecret(savedRegistryRecord.environmentId)),
+        );
+      }),
+    ),
+  );
+
   it.effect("treats empty saved environment documents as empty", () =>
     withSavedEnvironments(
       Effect.gen(function* () {
diff --git a/apps/desktop/src/settings/DesktopSavedEnvironments.ts b/apps/desktop/src/settings/DesktopSavedEnvironments.ts
@@ -122,6 +122,9 @@ export interface DesktopSavedEnvironmentsShape {
   readonly setRegistry: (
     records: readonly PersistedSavedEnvironmentRecord[],
   ) => Effect.Effect<void, DesktopSavedEnvironmentsWriteError>;
+  readonly removeEnvironment: (
+    environmentId: string,
+  ) => Effect.Effect<void, DesktopSavedEnvironmentsWriteError>;
   readonly getSecret: (
     environmentId: string,
   ) => Effect.Effect<Option.Option<string>, DesktopSavedEnvironmentsGetSecretError>;
@@ -303,6 +306,23 @@ export const layer = Layer.effect(
         );
         yield* writeDocument(preserveExistingSecrets(currentDocument, records));
       }),
+      removeEnvironment: Effect.fn("desktop.savedEnvironments.removeEnvironment")(
+        function* (environmentId) {
+          yield* Effect.annotateCurrentSpan({ environmentId });
+          const document = yield* readRegistryDocument(
+            fileSystem,
+            environment.savedEnvironmentRegistryPath,
+          );
+          if (!document.records.some((record) => record.environmentId === environmentId)) {
+            return;
+          }
+
+          yield* writeDocument({
+            version: document.version,
+            records: document.records.filter((record) => record.environmentId !== environmentId),
+          });
+        },
+      ),
       getSecret: Effect.fn("desktop.savedEnvironments.getSecret")(function* (environmentId) {
         yield* Effect.annotateCurrentSpan({ environmentId });
         const document = yield* readRegistryDocument(
@@ -403,6 +423,18 @@ export const layerTest = (input?: {
       return DesktopSavedEnvironments.of({
         getRegistry: Ref.get(recordsRef),
         setRegistry: (records) => Ref.set(recordsRef, records),
+        removeEnvironment: (environmentId) =>
+          Ref.update(recordsRef, (records) =>
+            records.filter((record) => record.environmentId !== environmentId),
+          ).pipe(
+            Effect.andThen(
+              Ref.update(secretsRef, (secrets) => {
+                const nextSecrets = new Map(secrets);
+                nextSecrets.delete(environmentId);
+                return nextSecrets;
+              }),
+            ),
+          ),
         getSecret: (environmentId) =>
           Ref.get(secretsRef).pipe(
             Effect.map((secrets) => Option.fromNullishOr(secrets.get(environmentId))),
diff --git a/apps/web/src/clientPersistenceStorage.ts b/apps/web/src/clientPersistenceStorage.ts
@@ -152,6 +152,14 @@ export function writeBrowserSavedEnvironmentRegistry(
   );
 }
 
+export function removeBrowserSavedEnvironment(environmentId: EnvironmentIdValue): void {
+  const document = readBrowserSavedEnvironmentRegistryDocument();
+  writeBrowserSavedEnvironmentRegistryDocument({
+    version: document.version ?? 1,
+    records: (document.records ?? []).filter((record) => record.environmentId !== environmentId),
+  });
+}
+
 export function readBrowserSavedEnvironmentSecret(
   environmentId: EnvironmentIdValue,
 ): string | null {
diff --git a/apps/web/src/components/settings/SettingsPanels.browser.tsx b/apps/web/src/components/settings/SettingsPanels.browser.tsx
@@ -352,6 +352,7 @@ const createDesktopBridgeStub = (overrides?: {
     setClientSettings: vi.fn().mockResolvedValue(undefined),
     getSavedEnvironmentRegistry: vi.fn().mockResolvedValue([]),
     setSavedEnvironmentRegistry: vi.fn().mockResolvedValue(undefined),
+    removeSavedEnvironment: vi.fn().mockResolvedValue(undefined),
     getSavedEnvironmentSecret: vi.fn().mockResolvedValue(null),
     setSavedEnvironmentSecret: vi.fn().mockResolvedValue(true),
     removeSavedEnvironmentSecret: vi.fn().mockResolvedValue(undefined),
diff --git a/apps/web/src/environments/runtime/catalog.test.ts b/apps/web/src/environments/runtime/catalog.test.ts
@@ -22,6 +22,7 @@ describe("environment runtime catalog stores", () => {
           setClientSettings: async () => undefined,
           getSavedEnvironmentRegistry: async () => [],
           setSavedEnvironmentRegistry: async () => undefined,
+          removeSavedEnvironment: async () => undefined,
           getSavedEnvironmentSecret: async () => null,
           setSavedEnvironmentSecret: async () => true,
           removeSavedEnvironmentSecret: async () => undefined,
@@ -109,6 +110,7 @@ describe("environment runtime catalog stores", () => {
               resolveRegistryRead = () => resolve([]);
             }),
           setSavedEnvironmentRegistry: async () => undefined,
+          removeSavedEnvironment: async () => undefined,
           getSavedEnvironmentSecret: async () => null,
           setSavedEnvironmentSecret: async () => true,
           removeSavedEnvironmentSecret: async () => undefined,
diff --git a/apps/web/src/environments/runtime/catalog.ts b/apps/web/src/environments/runtime/catalog.ts
@@ -244,6 +244,10 @@ export async function persistSavedEnvironmentRecord(record: SavedEnvironmentReco
   );
 }
 
+export async function removePersistedSavedEnvironment(environmentId: EnvironmentId): Promise<void> {
+  await ensureLocalApi().persistence.removeSavedEnvironment(environmentId);
+}
+
 export async function readSavedEnvironmentBearerToken(
   environmentId: EnvironmentId,
 ): Promise<string | null> {
diff --git a/apps/web/src/environments/runtime/service.addSavedEnvironment.test.ts b/apps/web/src/environments/runtime/service.addSavedEnvironment.test.ts
@@ -12,6 +12,7 @@ const mockResolveRemoteWebSocketConnectionUrl = vi.fn();
 const mockBootstrapSshBearerSession = vi.fn();
 const mockFetchSshSessionState = vi.fn();
 const mockPersistSavedEnvironmentRecord = vi.fn();
+const mockRemovePersistedSavedEnvironment = vi.fn();
 const mockWriteSavedEnvironmentBearerToken = vi.fn();
 const mockSetSavedEnvironmentRegistry = vi.fn();
 const mockGetSavedEnvironmentRecord = vi.fn((environmentId: EnvironmentId) => {
@@ -21,9 +22,21 @@ const mockReadSavedEnvironmentBearerToken = vi.fn();
 const mockRemoveSavedEnvironmentBearerToken = vi.fn();
 const mockPatchRuntime = vi.fn();
 const mockClearRuntime = vi.fn();
-const mockRegistrySetState = vi.fn((next: { byId: Record<string, Record<string, unknown>> }) => {
-  mockSavedRecords = Object.values(next.byId);
-});
+const mockRegistrySetState = vi.fn(
+  (
+    next:
+      | { byId: Record<string, Record<string, unknown>> }
+      | ((state: { byId: Record<string, Record<string, unknown>> }) => {
+          byId: Record<string, Record<string, unknown>>;
+        }),
+  ) => {
+    const current = Object.fromEntries(
+      mockSavedRecords.map((record) => [record.environmentId, record]),
+    ) as Record<string, Record<string, unknown>>;
+    const resolved = typeof next === "function" ? next({ byId: current }) : next;
+    mockSavedRecords = Object.values(resolved.byId);
+  },
+);
 const mockRemove = vi.fn((environmentId: EnvironmentId) => {
   mockSavedRecords = mockSavedRecords.filter((record) => record.environmentId !== environmentId);
 });
@@ -72,6 +85,7 @@ vi.mock("~/localApi", () => ({
   ensureLocalApi: () => ({
     persistence: {
       setSavedEnvironmentRegistry: mockSetSavedEnvironmentRegistry,
+      removeSavedEnvironment: mockRemovePersistedSavedEnvironment,
     },
   }),
 }));
@@ -82,6 +96,7 @@ vi.mock("./catalog", () => ({
   listSavedEnvironmentRecords: mockListSavedEnvironmentRecords,
   persistSavedEnvironmentRecord: mockPersistSavedEnvironmentRecord,
   readSavedEnvironmentBearerToken: mockReadSavedEnvironmentBearerToken,
+  removePersistedSavedEnvironment: mockRemovePersistedSavedEnvironment,
   removeSavedEnvironmentBearerToken: mockRemoveSavedEnvironmentBearerToken,
   toPersistedSavedEnvironmentRecord: mockToPersistedSavedEnvironmentRecord,
   useSavedEnvironmentRegistryStore: {
@@ -183,6 +198,7 @@ describe("addSavedEnvironment", () => {
       role: "owner",
     });
     mockPersistSavedEnvironmentRecord.mockResolvedValue(undefined);
+    mockRemovePersistedSavedEnvironment.mockResolvedValue(undefined);
     mockWriteSavedEnvironmentBearerToken.mockResolvedValue(false);
     mockSetSavedEnvironmentRegistry.mockResolvedValue(undefined);
     mockReadSavedEnvironmentBearerToken.mockResolvedValue(null);
@@ -397,10 +413,10 @@ describe("addSavedEnvironment", () => {
         environmentId: EnvironmentId.make("environment-2"),
       }),
     );
-    expect(mockRemove).toHaveBeenCalledWith(EnvironmentId.make("environment-1"));
-    expect(mockRemoveSavedEnvironmentBearerToken).toHaveBeenCalledWith(
+    expect(mockRemovePersistedSavedEnvironment).toHaveBeenCalledWith(
       EnvironmentId.make("environment-1"),
     );
+    expect(mockRemoveSavedEnvironmentBearerToken).not.toHaveBeenCalled();
 
     await resetEnvironmentServiceForTests();
   });
@@ -671,11 +687,12 @@ describe("addSavedEnvironment", () => {
       username: "julius",
       port: 22,
     });
-    expect(mockRemove).toHaveBeenCalledWith(EnvironmentId.make("environment-1"));
-    expect(mockRemoveSavedEnvironmentBearerToken).toHaveBeenCalledWith(
+    expect(mockRemovePersistedSavedEnvironment).toHaveBeenCalledWith(
       EnvironmentId.make("environment-1"),
     );
-    expect(mockRemove.mock.invocationCallOrder[0]).toBeLessThan(
+    expect(mockRemoveSavedEnvironmentBearerToken).not.toHaveBeenCalled();
+    expect(mockSavedRecords).toEqual([]);
+    expect(mockRemovePersistedSavedEnvironment.mock.invocationCallOrder[0]).toBeLessThan(
       mockDisconnectSshEnvironment.mock.invocationCallOrder[0] ?? Number.POSITIVE_INFINITY,
     );
 
@@ -713,7 +730,10 @@ describe("addSavedEnvironment", () => {
       username: "julius",
       port: 22,
     });
-    expect(mockRemove).toHaveBeenCalledWith(EnvironmentId.make("environment-1"));
+    expect(mockRemovePersistedSavedEnvironment).toHaveBeenCalledWith(
+      EnvironmentId.make("environment-1"),
+    );
+    expect(mockSavedRecords).toEqual([]);
 
     await resetEnvironmentServiceForTests();
   });
diff --git a/apps/web/src/environments/runtime/service.savedEnvironments.test.ts b/apps/web/src/environments/runtime/service.savedEnvironments.test.ts
@@ -89,6 +89,7 @@ vi.mock("~/localApi", () => ({
   ensureLocalApi: vi.fn(() => ({
     persistence: {
       setSavedEnvironmentRegistry: vi.fn(async () => undefined),
+      removeSavedEnvironment: vi.fn(async () => undefined),
     },
   })),
 }));
diff --git a/apps/web/src/environments/runtime/service.ts b/apps/web/src/environments/runtime/service.ts
@@ -45,6 +45,7 @@ import {
   listSavedEnvironmentRecords,
   persistSavedEnvironmentRecord,
   readSavedEnvironmentBearerToken,
+  removePersistedSavedEnvironment,
   removeSavedEnvironmentBearerToken,
   type SavedEnvironmentRecord,
   toPersistedSavedEnvironmentRecord,
@@ -1674,10 +1675,13 @@ export async function reconnectSavedEnvironment(environmentId: EnvironmentId): P
 export async function removeSavedEnvironment(environmentId: EnvironmentId): Promise<void> {
   const record = await detachSavedEnvironment(environmentId);
   disposeThreadDetailSubscriptionsForEnvironment(environmentId);
-  useSavedEnvironmentRegistryStore.getState().remove(environmentId);
+  await removePersistedSavedEnvironment(environmentId);
+  useSavedEnvironmentRegistryStore.setState((state) => {
+    const { [environmentId]: _removed, ...byId } = state.byId;
+    return { byId };
+  });
   useSavedEnvironmentRuntimeStore.getState().clear(environmentId);
   useStore.getState().removeEnvironmentState(environmentId);
-  await removeSavedEnvironmentBearerToken(environmentId);
 
   if (record?.desktopSsh && typeof window !== "undefined") {
     void window.desktopBridge?.disconnectSshEnvironment(record.desktopSsh).catch((error) => {
diff --git a/apps/web/src/localApi.test.ts b/apps/web/src/localApi.test.ts
@@ -176,6 +176,7 @@ function makeDesktopBridge(overrides: Partial<DesktopBridge> = {}): DesktopBridg
     setClientSettings: async () => undefined,
     getSavedEnvironmentRegistry: async () => [],
     setSavedEnvironmentRegistry: async () => undefined,
+    removeSavedEnvironment: async () => undefined,
     getSavedEnvironmentSecret: async () => null,
     setSavedEnvironmentSecret: async () => true,
     removeSavedEnvironmentSecret: async () => undefined,
@@ -622,6 +623,7 @@ describe("wsApi", () => {
     const setClientSettings = vi.fn().mockResolvedValue(undefined);
     const getSavedEnvironmentRegistry = vi.fn().mockResolvedValue([]);
     const setSavedEnvironmentRegistry = vi.fn().mockResolvedValue(undefined);
+    const removeSavedEnvironment = vi.fn().mockResolvedValue(undefined);
     const getSavedEnvironmentSecret = vi.fn().mockResolvedValue("bearer-token");
     const setSavedEnvironmentSecret = vi.fn().mockResolvedValue(true);
     const removeSavedEnvironmentSecret = vi.fn().mockResolvedValue(undefined);
@@ -630,6 +632,7 @@ describe("wsApi", () => {
       setClientSettings,
       getSavedEnvironmentRegistry,
       setSavedEnvironmentRegistry,
+      removeSavedEnvironment,
       getSavedEnvironmentSecret,
       setSavedEnvironmentSecret,
       removeSavedEnvironmentSecret,
@@ -642,6 +645,7 @@ describe("wsApi", () => {
     await api.persistence.setClientSettings(clientSettings);
     await api.persistence.getSavedEnvironmentRegistry();
     await api.persistence.setSavedEnvironmentRegistry([]);
+    await api.persistence.removeSavedEnvironment(EnvironmentId.make("environment-local"));
     await api.persistence.getSavedEnvironmentSecret(EnvironmentId.make("environment-local"));
     await api.persistence.setSavedEnvironmentSecret(
       EnvironmentId.make("environment-local"),
@@ -653,6 +657,7 @@ describe("wsApi", () => {
     expect(setClientSettings).toHaveBeenCalledWith(clientSettings);
     expect(getSavedEnvironmentRegistry).toHaveBeenCalledWith();
     expect(setSavedEnvironmentRegistry).toHaveBeenCalledWith([]);
+    expect(removeSavedEnvironment).toHaveBeenCalledWith("environment-local");
     expect(getSavedEnvironmentSecret).toHaveBeenCalledWith("environment-local");
     expect(setSavedEnvironmentSecret).toHaveBeenCalledWith("environment-local", "bearer-token");
     expect(removeSavedEnvironmentSecret).toHaveBeenCalledWith("environment-local");
@@ -716,5 +721,9 @@ describe("wsApi", () => {
     await expect(
       api.persistence.getSavedEnvironmentSecret(EnvironmentId.make("environment-local")),
     ).resolves.toBeNull();
+
+    await api.persistence.removeSavedEnvironment(EnvironmentId.make("environment-local"));
+
+    await expect(api.persistence.getSavedEnvironmentRegistry()).resolves.toEqual([]);
   });
 });
diff --git a/apps/web/src/localApi.ts b/apps/web/src/localApi.ts
@@ -20,6 +20,7 @@ import {
   readBrowserClientSettings,
   readBrowserSavedEnvironmentRegistry,
   readBrowserSavedEnvironmentSecret,
+  removeBrowserSavedEnvironment,
   removeBrowserSavedEnvironmentSecret,
   writeBrowserClientSettings,
   writeBrowserSavedEnvironmentRegistry,
@@ -99,6 +100,12 @@ function createBrowserLocalApi(rpcClient?: WsRpcClient): LocalApi {
         }
         writeBrowserSavedEnvironmentRegistry(records);
       },
+      removeSavedEnvironment: async (environmentId) => {
+        if (window.desktopBridge) {
+          return window.desktopBridge.removeSavedEnvironment(environmentId);
+        }
+        removeBrowserSavedEnvironment(environmentId);
+      },
       getSavedEnvironmentSecret: async (environmentId) => {
         if (window.desktopBridge) {
           return window.desktopBridge.getSavedEnvironmentSecret(environmentId);
diff --git a/packages/contracts/src/ipc.ts b/packages/contracts/src/ipc.ts
@@ -375,6 +375,7 @@ export interface DesktopBridge {
   setSavedEnvironmentRegistry: (
     records: readonly PersistedSavedEnvironmentRecord[],
   ) => Promise<void>;
+  removeSavedEnvironment: (environmentId: EnvironmentId) => Promise<void>;
   getSavedEnvironmentSecret: (environmentId: EnvironmentId) => Promise<string | null>;
   setSavedEnvironmentSecret: (environmentId: EnvironmentId, secret: string) => Promise<boolean>;
   removeSavedEnvironmentSecret: (environmentId: EnvironmentId) => Promise<void>;
@@ -452,6 +453,7 @@ export interface LocalApi {
     setSavedEnvironmentRegistry: (
       records: readonly PersistedSavedEnvironmentRecord[],
     ) => Promise<void>;
+    removeSavedEnvironment: (environmentId: EnvironmentId) => Promise<void>;
     getSavedEnvironmentSecret: (environmentId: EnvironmentId) => Promise<string | null>;
     setSavedEnvironmentSecret: (environmentId: EnvironmentId, secret: string) => Promise<boolean>;
     removeSavedEnvironmentSecret: (environmentId: EnvironmentId) => Promise<void>;
