From 99b9b43d4f942f95294531673341948316fd52e1 Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Mon, 6 Jul 2026 14:11:14 +0300 Subject: [PATCH 1/2] fix(tasks): survive workspace-server respawns during task creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workspace-server child respawns on a new random port and secret after a crash, but the Electron main process built its tRPC client once at boot and bound it as a DI constant. After any respawn, every main-process call kept hitting the dead port with "fetch failed" for the rest of the session — surfacing as a "Failed to detect repository: fetch failed" toast that aborted worktree task creation. Three layers of fix: - Repo detection in TaskCreationSaga is now best-effort: it only fills the org/repo metadata tag, so a transport failure degrades to an untagged task instead of aborting the saga. - New createReconnectingWorkspaceClient re-reads the current connection on every call chain and rebuilds the underlying client when the port/secret changes; the main process now uses it for all workspace-client bindings. - The two long-lived main-process SSE subscriptions (file watcher bridge, auth connectivity adapter) resubscribe when the server reports ready again, instead of retrying the dead port forever. Generated-By: PostHog Code Task-Id: d333d9be-05ed-410e-803f-8f0b304d5753 --- apps/code/src/main/index.ts | 37 +++++++++- .../src/main/services/auth/port-adapters.ts | 22 +++++- .../src/task-detail/taskCreationSaga.test.ts | 19 +++++ .../core/src/task-detail/taskCreationSaga.ts | 12 ++- packages/workspace-client/package.json | 4 +- packages/workspace-client/src/client.test.ts | 73 +++++++++++++++++++ packages/workspace-client/src/client.ts | 41 +++++++++++ packages/workspace-client/vitest.config.ts | 12 +++ pnpm-lock.yaml | 3 + 9 files changed, 216 insertions(+), 7 deletions(-) create mode 100644 packages/workspace-client/src/client.test.ts create mode 100644 packages/workspace-client/vitest.config.ts diff --git a/apps/code/src/main/index.ts b/apps/code/src/main/index.ts index 71ec77ceaa..db6099f1b5 100644 --- a/apps/code/src/main/index.ts +++ b/apps/code/src/main/index.ts @@ -2,7 +2,7 @@ import "reflect-metadata"; import os from "node:os"; import { TypedEventEmitter } from "@posthog/shared"; import type { WorkspaceClient } from "@posthog/workspace-client/client"; -import { createWorkspaceClient } from "@posthog/workspace-client/client"; +import { createReconnectingWorkspaceClient } from "@posthog/workspace-client/client"; import type { FileWatcherEvent } from "@posthog/workspace-client/types"; import { app, BrowserWindow, dialog, session } from "electron"; import log from "electron-log/main"; @@ -80,7 +80,11 @@ import { focusSessionStore, focusWorktreePaths, } from "./services/focus/desktop-adapters"; -import type { WorkspaceServerService } from "./services/workspace-server/service"; +import { + WorkspaceServerEvent, + type WorkspaceServerService, + WorkspaceServerStatus, +} from "./services/workspace-server/service"; import { collectMemorySnapshot, flattenMemorySnapshot, @@ -128,6 +132,19 @@ export class FileWatcherBridge extends TypedEventEmitter { const wsServer = container.get( WORKSPACE_SERVER_SERVICE, ); - const connection = await wsServer.start(); - const workspaceClient = createWorkspaceClient(connection); + await wsServer.start(); + // The workspace-server child respawns on a new port/secret after a crash; + // a reconnecting client follows the current connection so main-process + // callers don't keep hitting the dead port for the rest of the session. + const workspaceClient = createReconnectingWorkspaceClient(() => + wsServer.getConnection(), + ); container.bind(WORKSPACE_CLIENT).toConstantValue(workspaceClient); container.bind(GIT_WORKSPACE_CLIENT).toConstantValue(workspaceClient); container.bind(CONNECTIVITY_CLIENT).toConstantValue(workspaceClient); container.bind(ENVIRONMENT_CLIENT).toConstantValue(workspaceClient); const fileWatcherBridge = new FileWatcherBridge(workspaceClient); + // Re-establish live watches after a workspace-server respawn — the old SSE + // subscriptions keep retrying the dead port and never recover on their own. + wsServer.on(WorkspaceServerEvent.StatusChanged, ({ status }) => { + if (status === WorkspaceServerStatus.Ready) { + fileWatcherBridge.resubscribeAll(); + } + }); container.bind(FILE_WATCHER_SERVICE).toConstantValue(fileWatcherBridge); container.bind(FILE_WATCHER_CONTROL).toConstantValue(fileWatcherBridge); container.bind(FOCUS_WORKSPACE_CLIENT).toConstantValue(workspaceClient); diff --git a/apps/code/src/main/services/auth/port-adapters.ts b/apps/code/src/main/services/auth/port-adapters.ts index 9747fb9423..522406d0d1 100644 --- a/apps/code/src/main/services/auth/port-adapters.ts +++ b/apps/code/src/main/services/auth/port-adapters.ts @@ -26,8 +26,14 @@ import { AUTH_PREFERENCE_REPOSITORY, AUTH_SESSION_REPOSITORY, WORKSPACE_CLIENT, + WORKSPACE_SERVER_SERVICE, } from "../../di/tokens"; import { decrypt, encrypt } from "../../utils/encryption"; +import { + WorkspaceServerEvent, + type WorkspaceServerService, + WorkspaceServerStatus, +} from "../workspace-server/service"; @injectable() export class TokenCipherPortAdapter implements IAuthTokenCipher { @@ -149,12 +155,26 @@ export class AuthPreferencePortAdapter implements IAuthPreferenceStore { export class ConnectivityPortAdapter implements IAuthConnectivity { private isOnline = true; private readonly handlers = new Set<(status: ConnectivityStatus) => void>(); + private sub: { unsubscribe: () => void } | null = null; constructor( @inject(WORKSPACE_CLIENT) private readonly workspace: WorkspaceClient, + @inject(WORKSPACE_SERVER_SERVICE) + workspaceServer: WorkspaceServerService, ) { - this.workspace.connectivity.onStatusChange.subscribe(undefined, { + this.subscribe(); + // The workspace-server child respawns on a new port after a crash; the + // old SSE subscription keeps retrying the dead port forever, so re- + // establish it against the current connection once the server is healthy. + workspaceServer.on(WorkspaceServerEvent.StatusChanged, ({ status }) => { + if (status === WorkspaceServerStatus.Ready) this.subscribe(); + }); + } + + private subscribe(): void { + this.sub?.unsubscribe(); + this.sub = this.workspace.connectivity.onStatusChange.subscribe(undefined, { onData: (status) => { this.isOnline = status.isOnline; for (const handler of this.handlers) { diff --git a/packages/core/src/task-detail/taskCreationSaga.test.ts b/packages/core/src/task-detail/taskCreationSaga.test.ts index e841a39d2b..86a1a4dce4 100644 --- a/packages/core/src/task-detail/taskCreationSaga.test.ts +++ b/packages/core/src/task-detail/taskCreationSaga.test.ts @@ -776,6 +776,25 @@ describe("TaskCreationSaga", () => { ); }); + it("creates the task without a repository when repo detection fails", async () => { + const createTaskMock = vi.fn().mockResolvedValue(createTask()); + mockHost.addFolder.mockResolvedValue({ id: "folder-1", path: "/repo" }); + mockHost.detectRepo.mockRejectedValue(new TypeError("fetch failed")); + + const saga = makeSaga({ createTask: createTaskMock }); + + const result = await saga.run({ + content: "Ship the fix", + repoPath: "/repo", + workspaceMode: "worktree", + }); + + expect(result.success).toBe(true); + expect(createTaskMock).toHaveBeenCalledWith( + expect.objectContaining({ repository: undefined }), + ); + }); + it("rolls back the import snapshot and tracking row when a later step fails", async () => { const createdTask = createTask(); const createTaskMock = vi.fn().mockResolvedValue(createdTask); diff --git a/packages/core/src/task-detail/taskCreationSaga.ts b/packages/core/src/task-detail/taskCreationSaga.ts index e44637f981..b771155e3c 100644 --- a/packages/core/src/task-detail/taskCreationSaga.ts +++ b/packages/core/src/task-detail/taskCreationSaga.ts @@ -600,8 +600,18 @@ export class TaskCreationSaga extends Saga< const repoPathForDetection = input.repoPath; if (!repository && repoPathForDetection) { + // Detection only fills the org/repo metadata on the task; a transient + // failure (e.g. the workspace-server transport dropping mid-call) must + // not abort task creation, so degrade to an untagged task instead. const detected = await this.readOnlyStep("repo_detection", () => - this.deps.host.detectRepo({ directoryPath: repoPathForDetection }), + this.deps.host + .detectRepo({ directoryPath: repoPathForDetection }) + .catch((error) => { + this.log.warn("Repo detection failed; creating task without one", { + error, + }); + return null; + }), ); if (detected) { repository = `${detected.organization}/${detected.repository}`; diff --git a/packages/workspace-client/package.json b/packages/workspace-client/package.json index fc9bec9beb..ec95c227c4 100644 --- a/packages/workspace-client/package.json +++ b/packages/workspace-client/package.json @@ -12,6 +12,7 @@ }, "scripts": { "typecheck": "tsc --noEmit", + "test": "vitest run", "clean": "node ../../scripts/rimraf.mjs .turbo" }, "dependencies": { @@ -30,6 +31,7 @@ "@trpc/tanstack-react-query": "catalog:", "@types/react": "catalog:", "react": "catalog:", - "typescript": "catalog:" + "typescript": "catalog:", + "vitest": "^4.1.8" } } diff --git a/packages/workspace-client/src/client.test.ts b/packages/workspace-client/src/client.test.ts new file mode 100644 index 0000000000..57dc071c8d --- /dev/null +++ b/packages/workspace-client/src/client.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it, vi } from "vitest"; +import { + createReconnectingWorkspaceClient, + type WorkspaceClient, + type WorkspaceConnection, +} from "./client"; + +const connA: WorkspaceConnection = { + url: "http://127.0.0.1:1111", + secret: "secret-a", +}; +const connB: WorkspaceConnection = { + url: "http://127.0.0.1:2222", + secret: "secret-b", +}; + +function makeFakeClient(name: string): WorkspaceClient { + return { name } as unknown as WorkspaceClient; +} + +describe("createReconnectingWorkspaceClient", () => { + it("builds one client per connection and reuses it across accesses", () => { + const buildClient = vi.fn(() => makeFakeClient("a")); + const client = createReconnectingWorkspaceClient(() => connA, buildClient); + + expect(Reflect.get(client, "name")).toBe("a"); + expect(Reflect.get(client, "name")).toBe("a"); + expect(buildClient).toHaveBeenCalledTimes(1); + expect(buildClient).toHaveBeenCalledWith(connA); + }); + + it("rebuilds the client when the connection changes (server respawn)", () => { + let current = connA; + const buildClient = vi + .fn() + .mockReturnValueOnce(makeFakeClient("a")) + .mockReturnValueOnce(makeFakeClient("b")); + const client = createReconnectingWorkspaceClient( + () => current, + buildClient, + ); + + expect(Reflect.get(client, "name")).toBe("a"); + current = connB; + expect(Reflect.get(client, "name")).toBe("b"); + expect(buildClient).toHaveBeenCalledTimes(2); + expect(buildClient).toHaveBeenLastCalledWith(connB); + }); + + it("keeps using the last known client while the server is down", () => { + let current: WorkspaceConnection | null = connA; + const buildClient = vi.fn(() => makeFakeClient("a")); + const client = createReconnectingWorkspaceClient( + () => current, + buildClient, + ); + + expect(Reflect.get(client, "name")).toBe("a"); + current = null; + expect(Reflect.get(client, "name")).toBe("a"); + expect(buildClient).toHaveBeenCalledTimes(1); + }); + + it("throws when accessed before any connection is established", () => { + const buildClient = vi.fn(); + const client = createReconnectingWorkspaceClient(() => null, buildClient); + + expect(() => Reflect.get(client, "name")).toThrow( + "workspace-server connection is not established yet", + ); + expect(buildClient).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/workspace-client/src/client.ts b/packages/workspace-client/src/client.ts index 79b518f93e..39e0c44597 100644 --- a/packages/workspace-client/src/client.ts +++ b/packages/workspace-client/src/client.ts @@ -16,6 +16,47 @@ export interface WorkspaceConnection { export type WorkspaceClient = ReturnType; +/** + * A workspace client that follows the host's current connection. The + * workspace-server child can crash and be respawned on a new port with a new + * secret; a client built once at boot would keep calling the dead port + * ("fetch failed") for the rest of the app session. This proxy re-reads the + * connection at the start of every call chain and rebuilds the underlying + * client whenever it changes. While the server is down (`getConnection` + * returns null) the last known client is used, so calls fail fast against the + * old port instead of hanging. + */ +export function createReconnectingWorkspaceClient( + getConnection: () => WorkspaceConnection | null, + buildClient: ( + connection: WorkspaceConnection, + ) => WorkspaceClient = createWorkspaceClient, +): WorkspaceClient { + let built: { + connection: WorkspaceConnection; + client: WorkspaceClient; + } | null = null; + + const resolve = (): WorkspaceClient => { + const connection = getConnection() ?? built?.connection; + if (!connection) { + throw new Error("workspace-server connection is not established yet"); + } + if ( + !built || + built.connection.url !== connection.url || + built.connection.secret !== connection.secret + ) { + built = { connection, client: buildClient(connection) }; + } + return built.client; + }; + + return new Proxy({} as WorkspaceClient, { + get: (_target, prop) => Reflect.get(resolve() as object, prop), + }); +} + export function createWorkspaceClient(connection: WorkspaceConnection) { const url = `${connection.url.replace(/\/$/, "")}/trpc`; const headers = { [SECRET_HEADER]: connection.secret }; diff --git a/packages/workspace-client/vitest.config.ts b/packages/workspace-client/vitest.config.ts new file mode 100644 index 0000000000..407c36b953 --- /dev/null +++ b/packages/workspace-client/vitest.config.ts @@ -0,0 +1,12 @@ +import { defineConfig } from "vitest/config"; +import { trunkTestOptions } from "../../vitest.config.base"; + +export default defineConfig({ + test: { + globals: true, + ...trunkTestOptions, + environment: "node", + include: ["src/**/*.test.ts"], + exclude: ["**/node_modules/**", "**/dist/**"], + }, +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1cb2f3bb09..295de08202 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1419,6 +1419,9 @@ importers: typescript: specifier: 'catalog:' version: 5.9.3 + vitest: + specifier: ^4.1.8 + version: 4.1.8(@opentelemetry/api@1.9.1)(@types/node@25.2.0)(@vitest/ui@4.1.8)(jsdom@26.1.0)(msw@2.12.8(@types/node@25.2.0)(typescript@5.9.3))(vite@7.3.5(@types/node@25.2.0)(jiti@2.7.0)(lightningcss@1.32.0)(terser@5.46.0)(tsx@4.22.4)(yaml@2.9.0)) packages/workspace-server: dependencies: From 5201c288667cdde02d756973e7fcae4c2c4eb390 Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Mon, 6 Jul 2026 14:29:39 +0300 Subject: [PATCH 2/2] fix(tasks): address review feedback on reconnecting client Two Greptile P2s: add has/getPrototypeOf traps so the reconnecting proxy stays transparent to callers that probe the object, and give ConnectivityPortAdapter a dispose() that detaches its StatusChanged listener and SSE subscription so a torn-down instance can't keep resubscribing. Generated-By: PostHog Code Task-Id: d333d9be-05ed-410e-803f-8f0b304d5753 --- .../src/main/services/auth/port-adapters.ts | 25 ++++++++++++++++--- packages/workspace-client/src/client.ts | 2 ++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/apps/code/src/main/services/auth/port-adapters.ts b/apps/code/src/main/services/auth/port-adapters.ts index 522406d0d1..c354a9e5d5 100644 --- a/apps/code/src/main/services/auth/port-adapters.ts +++ b/apps/code/src/main/services/auth/port-adapters.ts @@ -156,20 +156,37 @@ export class ConnectivityPortAdapter implements IAuthConnectivity { private isOnline = true; private readonly handlers = new Set<(status: ConnectivityStatus) => void>(); private sub: { unsubscribe: () => void } | null = null; + private readonly onServerStatusChanged = ({ + status, + }: { + status: WorkspaceServerStatus; + }): void => { + if (status === WorkspaceServerStatus.Ready) this.subscribe(); + }; constructor( @inject(WORKSPACE_CLIENT) private readonly workspace: WorkspaceClient, @inject(WORKSPACE_SERVER_SERVICE) - workspaceServer: WorkspaceServerService, + private readonly workspaceServer: WorkspaceServerService, ) { this.subscribe(); // The workspace-server child respawns on a new port after a crash; the // old SSE subscription keeps retrying the dead port forever, so re- // establish it against the current connection once the server is healthy. - workspaceServer.on(WorkspaceServerEvent.StatusChanged, ({ status }) => { - if (status === WorkspaceServerStatus.Ready) this.subscribe(); - }); + this.workspaceServer.on( + WorkspaceServerEvent.StatusChanged, + this.onServerStatusChanged, + ); + } + + dispose(): void { + this.workspaceServer.off( + WorkspaceServerEvent.StatusChanged, + this.onServerStatusChanged, + ); + this.sub?.unsubscribe(); + this.sub = null; } private subscribe(): void { diff --git a/packages/workspace-client/src/client.ts b/packages/workspace-client/src/client.ts index 39e0c44597..186a7c8dda 100644 --- a/packages/workspace-client/src/client.ts +++ b/packages/workspace-client/src/client.ts @@ -54,6 +54,8 @@ export function createReconnectingWorkspaceClient( return new Proxy({} as WorkspaceClient, { get: (_target, prop) => Reflect.get(resolve() as object, prop), + has: (_target, prop) => Reflect.has(resolve() as object, prop), + getPrototypeOf: () => Reflect.getPrototypeOf(resolve() as object), }); }