Fix supervised/local regressions - #1020
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-cloud | 1e79869 | Jun 15 2026, 02:26 AM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-marketing | 1e79869 | Commit Preview URL Branch Preview URL |
Jun 15 2026, 02:25 AM |
Cloudflare previewTorn down — the PR is closed. |
@executor-js/cli
@executor-js/config
@executor-js/execution
@executor-js/sdk
@executor-js/codemode-core
@executor-js/runtime-quickjs
@executor-js/plugin-file-secrets
@executor-js/plugin-graphql
@executor-js/plugin-keychain
@executor-js/plugin-mcp
@executor-js/plugin-onepassword
@executor-js/plugin-openapi
executor
commit: |
ff15559 to
76b8a33
Compare
Greptile SummaryThis PR fixes two categories of regressions: (1) the desktop supervised-daemon path (launchctl
Confidence Score: 4/5Safe to merge — the core regression fixes are well-tested and the logic is correct; two spots in the React connection menu deserve a follow-up. The ServerConnectionMenu hydration path introduces an async await inside a then-callback but only checks the cancellation flag before the await, leaving persistSnapshot and state setters reachable after unmount. Additionally, hasDesktopServerConnectionBridge is defined twice with subtly different predicates, which could produce inconsistent desktop-mode behaviour if the bridge APIs are ever partially present. Everything else — service install planning, launchctl fix, CLI delegation, MCP ownership — looks solid. packages/app/src/web/server-connection-menu.tsx (missing post-await cancellation check, divergent bridge helper) and packages/react/src/api/server-connection.tsx (silent no-op guard) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Desktop boots] --> B{attachToSupervisedDaemon}
B -- manifest + pid alive + reachable --> C{shouldReplaceDaemonForDesktop?}
C -- newer version OR path mismatch --> D[installSupervisedService]
C -- up to date --> E[attach: supervisedDaemon=true]
B -- no manifest --> F{supervisedServiceStatus}
F -- registered + running --> G[waitForSupervisedAttach]
F -- not running --> H{user confirmed?}
H -- yes --> D
H -- no --> I[managed spawn: daemon run --foreground]
D --> J[planServiceInstall]
J -- noop --> K[log + return]
J -- reinstall --> L[bootout then enable then bootstrap]
J -- takeover-then-install --> M[takeOverActiveLocalServer] --> L
L --> N[waitForSupervisedAttach]
N --> E
G --> E
Reviews (1): Last reviewed commit: "Fix Windows service uninstall cleanup" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| let cancelled = false; | ||
| void readStoredProfiles().then((stored) => { | ||
| if (cancelled) return; | ||
| const storedActive = getActiveExecutorServerProfile(stored); | ||
| const next = snapshotWithCurrent(stored, connection, storedActive === null); | ||
| persistSnapshot(next); | ||
| if (storedActive && storedActive.key !== connection.key) { | ||
| setServerConnection(storedActive); | ||
| } | ||
| setHydrated(true); | ||
| void (async () => { | ||
| if (cancelled) return; | ||
| const desktopBridge = hasDesktopServerConnectionBridge(); | ||
| const desktopConnection = | ||
| (await readDesktopServerConnection()?.then( | ||
| (value) => value, | ||
| () => null, | ||
| )) ?? null; | ||
| const storedActive = getActiveExecutorServerProfile(stored); | ||
| const current = desktopConnection | ||
| ? normalizeExecutorServerConnection(desktopConnection) | ||
| : connection; | ||
| const baseStored = desktopConnection ? withoutLoopbackProfiles(stored) : stored; | ||
| const shouldKeepCurrent = | ||
| desktopBridge || | ||
| storedActive === null || | ||
| (hasBearerAuth(current) && | ||
| storedActive !== null && | ||
| sameLoopbackServer(current, storedActive)); | ||
| const next = snapshotWithCurrent(baseStored, current, shouldKeepCurrent); | ||
| persistSnapshot(next); | ||
| if (desktopConnection) { | ||
| setServerConnection(desktopConnection); | ||
| } else if (!shouldKeepCurrent && storedActive && storedActive.key !== connection.key) { | ||
| setServerConnection(storedActive); | ||
| } | ||
| setHydrated(true); | ||
| })(); | ||
| }); | ||
|
|
||
| return () => { |
There was a problem hiding this comment.
The if (cancelled) return; guard fires before the await readDesktopServerConnection() call, but not after. If the component unmounts while the bridge fetch is in-flight, cancelled will be true but the code continues past the await and calls setServerConnection, setHydrated(true), and persistSnapshot(next) — the last of which writes to persistent storage with potentially stale data. In React 18 the state-setter calls are silently ignored, but the persistSnapshot write is not gated and can corrupt the stored profile snapshot with a connection computed against the unmounted component's props.
| const [connection, setConnection] = React.useState(initialConnection); | ||
| const setActiveConnection = React.useCallback((input: ExecutorServerConnectionInput): void => { | ||
| const next = normalizeExecutorServerConnection(input); | ||
| if (hasDesktopServerConnectionBridge() && next.kind !== "desktop-sidecar") return; |
There was a problem hiding this comment.
When hasDesktopServerConnectionBridge() is true and the requested connection is not desktop-sidecar, setActiveConnection returns silently — no console warning, no thrown error, no callback. Any code path that calls setActiveConnection to switch servers (e.g. stored-profile hydration logic in the menu) will appear to succeed but change nothing. Adding a log line makes regressions visible during development without affecting production behavior.
| if (hasDesktopServerConnectionBridge() && next.kind !== "desktop-sidecar") return; | |
| if (hasDesktopServerConnectionBridge() && next.kind !== "desktop-sidecar") { | |
| console.warn( | |
| "[executor] setActiveConnection: ignoring non-desktop connection in desktop context", | |
| next.kind, | |
| ); | |
| return; | |
| } |
| }; | ||
| }; | ||
|
|
||
| const hasDesktopServerConnectionBridge = (): boolean => | ||
| Boolean(desktopProfileStorageBridge()) || | ||
| typeof globalThis.window?.executor?.getServerConnection === "function"; | ||
|
|
||
| const readDesktopServerConnection = (): Promise<ExecutorServerConnectionInput | null> | null => { | ||
| const bridge = globalThis.window?.executor; | ||
| if (!bridge || typeof bridge.getServerConnection !== "function") return null; |
There was a problem hiding this comment.
hasDesktopServerConnectionBridge duplicated with divergent logic
server-connection.tsx defines the same function name as typeof globalThis.window?.executor?.getServerConnection === "function", while this file's version additionally ORs in Boolean(desktopProfileStorageBridge()) (which checks for executor.getProfiles). If only getProfiles is present (e.g. a partial bridge injection) the menu strips loopback profiles and skips the stored-active switch, but the provider's setActiveConnection guard does not fire — leaving the menu and the connection provider in inconsistent "desktop mode" states. Sharing a single helper (or at minimum aligning the checks) removes the drift risk.
#1020 moved the foreground web server behind `executor web --foreground`: plain `executor web` now opens the installed background service and prints "Executor is not running" when none exists. The release-bootstrap smoke test still invoked `executor web --port`, so it never got a served response and the v1.5.12 publish gate failed at release:check. Pass --foreground to exercise the same runForegroundSession path the test was written for.
1 participant
Fixes regressions and adds e2e coverage.