fix(tasks): survive workspace-server respawns during task creation#3171
fix(tasks): survive workspace-server respawns during task creation#3171richardsolomou wants to merge 2 commits into
Conversation
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
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "fix(tasks): survive workspace-server res..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return new Proxy({} as WorkspaceClient, { | ||
| get: (_target, prop) => Reflect.get(resolve() as object, prop), | ||
| }); |
There was a problem hiding this comment.
get
The reconnecting proxy intercepts only get, so any code that uses in, Object.keys(), Object.getOwnPropertyDescriptor(), or getPrototypeOf() against the wrapped client will see the empty {} target instead of the real tRPC client. This isn't a problem today — tRPC clients don't rely on those traps — but it makes the wrapper silently wrong for callers that might probe the object. Adding has and getPrototypeOf traps would make it a complete transparent proxy.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| workspaceServer.on(WorkspaceServerEvent.StatusChanged, ({ status }) => { | ||
| if (status === WorkspaceServerStatus.Ready) this.subscribe(); | ||
| }); |
There was a problem hiding this comment.
StatusChanged listener is never removed
workspaceServer.on(...) is called in the constructor but there is no matching off() or disposal path. The handler closes over this, so WorkspaceServerService holds a permanent reference back to ConnectivityPortAdapter. For singleton DI services in a single-session Electron app this doesn't cause a leak, but there is no way to stop the resubscription logic if the adapter is ever torn down — future StatusChanged.Ready events would still invoke this.subscribe() on the stale instance. Storing the unsubscribe handle and exposing a dispose() method (or at minimum a Symbol.dispose implementation) would make this symmetric with the sub tracking already done for the tRPC subscription.
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
1 participant
Problem
Starting a worktree task would sometimes fail with a "Failed to detect repository: fetch failed" toast, even though repo detection is only optional metadata.
Root cause: the workspace-server child respawns on a new random port and secret after a crash (and
serve.tshas nouncaughtException/unhandledRejectionhandlers, so any stray rejection kills it). The renderer rebuilds its client on reconnect, 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 — repo detection during task creation, file watching, connectivity — kept hitting the dead port withfetch faileduntil the app was relaunched.Changes
packages/core— repo detection inTaskCreationSagais now best-effort: it only fills theorg/repometadata tag on the task, so a transport failure logs a warning and creates the task untagged instead of aborting the saga and toasting.packages/workspace-client— newcreateReconnectingWorkspaceClient(getConnection): a proxy that re-reads the current connection at the start of every call chain and rebuilds the underlying client when the port/secret changes. While the server is down it falls back to the last known client so calls fail fast instead of hanging.apps/codemain — all main-process workspace-client bindings now use the reconnecting client. The two long-lived SSE subscriptions (FileWatcherBridge,ConnectivityPortAdapter) resubscribe whenWorkspaceServerServicereports ready again, since the old streams retry the dead port forever.How did you test this?
TaskCreationSagacreates the task without a repository whendetectReporejects withfetch failed(21 saga tests pass).@posthog/workspace-clienttest suite (vitest added to the package) covering the reconnecting client: build-once caching, rebuild on connection change, fallback while the server is down, error before first connection.TRPCClientError: fetch failed, matching the toast verbatim.apps/codeunit tests pass;@posthog/core,@posthog/workspace-client, andapps/codetypecheck clean; Biome clean;check-host-boundaries.mjsreports no new violations.Automatic notifications