perf(startup): stop unnecessary engine disposes, unblock first load on restricted networks, add startup benchmark - #3118
Open
benjaminshafii wants to merge 7 commits into
Open
perf(startup): stop unnecessary engine disposes, unblock first load on restricted networks, add startup benchmark#3118benjaminshafii wants to merge 7 commits into
benjaminshafii wants to merge 7 commits into
Conversation
…mark harness Adds NDJSON startup tracing to the Electron main process, the in-process OpenWork server, and the MCP sync/cloud-reconcile paths. There was previously no timing instrumentation at all in the main process, so cold-start work could not be measured. - apps/desktop/electron/startup-trace.mjs + apps/server/src/startup-trace.ts: no-op when OPENWORK_STARTUP_TRACE is unset, buffered async flush with a synchronous flush on exit/SIGTERM/SIGINT, and never throws. - Instruments boot phases, opencode spawn/listen, openwork server listen, per-entry MCP POST registration, and the cloud MCP poll ladder. - scripts/bench-cold-start.mjs launches the app repeatedly and reports min/median/p95/max for window, openworkServer, opencode, mcpSynced and cloudConnected, with --compare for before/after deltas. Measurement only: no behavior change.
# Conflicts: # apps/desktop/electron/runtime.mjs # apps/server/src/embedded.ts
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Users hit
Error("Not connected")when the agent calls anopenwork-cloudMCP tool mid-conversation. Root cause, verified against the shipped opencode 1.17.11 binary:MCP.convertToolcaptures the MCPClientobject in each tool'sexecuteclosure at turn start. If that client is closed mid-turn, the captured client has_transport === undefinedand the SDK throws"Not connected".POST /instance/disposecloses every MCP client and SIGTERMs every local/stdio MCP child. Most of the disposes that cause this are self-inflicted. We cannot patch the pinned engine, so this PR stops closing those clients unnecessarily, and adds the measurement tooling that made the problem visible.Guiding principle: if the user is logged in, cloud capabilities are loaded and stay loaded. No new pre-send gate — see "Rejected" below.
What changed
Stop disposing the engine unnecessarily
syncRuntimeMcpToOpencodeEngine/disconnectMcpFromOpencodeEngine), but also emitted a reload event and the renderer calledmarkReloadRequired("mcp")— coalescing into a full dispose ~1.5 s later. The reload event is now emitted only when the surgical sync actually failed (real inspection ofEngineMcpSyncResult), and the three redundant renderer calls are gone.resolveWorkspace()could fire a fire-and-forget dispose from any workspace-scoped API request (viarepairCommands), errors swallowed.repairCommandsstill runs unchanged; it now emits a gated"commands"reload event instead, so the change still reaches the engine but through the reload coordinator's idle gate.refreshProviders({ dispose: true })on every 2 s poll iteration for up to 15 s (up to 8 disposes for one OAuth). Polling is now non-disposing; the engine is disposed at most once, after the provider is detected connected. The rawinstance.dispose()fallback — which skipped MCP re-registration and silently dropped non-primary-workspace MCPs — is now a genuine last resort.First-load / air-gap correctness
ArchitectureMismatchGaterenderednullfor the entire app tree while Electron main did an un-timeoutedfetchto GitHub releases over Node undici (no OS trust, no system proxy). On a stalled corporate proxy that blanks the window for minutes. Now:net.fetch(OS trust + proxy) with a 5 s abort, plus a bounded wait in the gate. Fail-open preserved.opencode-chrome-devtoolswas a bare npm specifier in the enginepluginarray — a runtime npm install on first launch, which cannot work air-gapped. It is now bundled to a local file like the other five plugins, pinned to 1.0.4, with a fallback to the old specifier if the bundled file is missing.Measurement (new)
startup-tracemodules for Electron main + the in-process server: NDJSON, no-op unlessOPENWORK_STARTUP_TRACEis set, buffered with a sync flush on exit/SIGTERM/SIGINT, and never throws. There was previously no timing instrumentation at all in the main process.scripts/bench-cold-start.mjslaunches the app repeatedly and reports min/median/p95/max for window / openworkServer / opencode / mcpSynced / cloudConnected, with--home fresh|shared(first-launch-ever vs relaunch) and--comparethat refuses mismatched profiles.Measured (Daytona Linux, medians)
Cold start, seeded with 3 MCPs, first-launch-ever (
--home fresh):The latency numbers did not move, and I am not claiming they did. Bundling the plugin removes a network dependency, not wall time. Isolating the engine with its real generated config showed why:
GET /mcppluginarray removedOPENWORK_SERVER_URLunset~4.1 s of first-launch cost is plugin loading, essentially all of it opencode npm-installing 28 packages into
~/.config/opencode/node_moduleson a freshHOME. Withplugin: []it installs nothing. Removing chrome-devtools' own install (verified: no~/.cache/opencode/packagesentry created) is dwarfed by that shared install, which is also the real air-gap blocker. That is the next lever and it is not attempted here — the engine installing its own version-matched@opencode-ai/pluginis plausibly safer than us bundling a mismatched copy.Verification
pnpm --filter openwork-server test→ 531 pass, 0 failpnpm --filter @openwork/app test→ 424 pass, 0 failpnpm --filter @openwork/desktop test→ 113 pass, 0 failpnpm --filter openwork-server typecheck,pnpm typecheck,pnpm --filter openwork-server build→ passNew tests: the three MCP routes emit no reload event when surgical sync succeeds and do emit one when it fails;
resolveWorkspacerecords a"commands"event and issues no/instance/dispose(asserted against a mock engine that records request paths); provider OAuth polling triggers at most one reload.No tool regression from bundling. Same engine, same dist, only
plugin[0]differing —/experimental/tool/idsreturns a byte-identical 27-tool list including all 8browser_*tools, and theafterrun creates no npm cache entry.fraimz
core-flowon this branch: frames 1–3 pass (boots to a usable session surface, creates a task, sends a message and gets a response). Frame 4 ("closes and reopens; session survives") fails.I ran the identical flow on unmodified
devin the same sandbox as a control, and it fails on the same frame with the same error — so this is pre-existing, not introduced here. It may be environment-specific (Daytona, dev mode, free model) or a real persistence bug; either way it deserves its own investigation.evals/results/2026-07-25T13-08-17-915Z/fraimz.htmlevals/results/2026-07-25T13-15-37-826Z/fraimz.htmlBoth preserved on the artifacts volume under
validation/cold-start-mcp/.Rejected: re-enabling the pre-send Cloud MCP gate
session-route.tsxstill carriesskipGate: true. Re-enabling it would be actively harmful:assessCloudMcpSubmissionReadinessrequirestools.direct.checked === true, i.e. a successful server-side probe — whichcloud-mcp-health.tsexplicitly declares non-authoritative precisely because it fails on corporate TLS interception, where the engine's own MCP connection works fine. That would block every message in exactly the air-gapped/intercepted fleets we need to support. Fixing the causes is the right move; the gate stays off.Follow-ups (evidence in hand, not done here)
"Not connected"instead of dead-ending.attributeChatToolErrorhas no branch for"Not connected"/"Connection closed", so the tool card renders a bare error with no reconnect or retry. A targeted, non-destructivePOST /workspace/:id/mcp/openwork-cloud/engine-refreshalready exists and is currently only reachable from a Settings button.OPENCODE_CONFIG; the first POST forces full engine init and posts 2–3 cost ~577 ms each of pure waste. OneGET /mcpthen POST only what is missing.runtime.mjsuses onlytls.getCACertificates("system");apps/installer/src/system-ca.tsadditionally enumerates Windows cert stores and macOS keychains because the runtime returns a partial root set on inspected networks.fetchcalls bypass OS trust (agent-context-cloud-probe.ts,agent-context-diagnostics.ts), andno-bare-fetch.test.ts's regex misses?? fetch.Skill.state/Command.stateare memoized per instance with no update endpoint, so skipping the reload would silently stop new skills appearing. Needs engine-side granular invalidation.