feat: allow multiple apps to run concurrently with garbage collection#2825
feat: allow multiple apps to run concurrently with garbage collection#2825wwwillchen-bot wants to merge 3 commits intodyad-sh:mainfrom
Conversation
Previously, Dyad only kept one app running at a time - switching to a different app would immediately stop the previous one. This caused friction for users working on multiple apps. Now multiple apps can run simultaneously. A garbage collector runs every minute and stops apps that: 1. Haven't been viewed in the last 10 minutes 2. Are not the currently selected app Changes: - Add `lastViewedAt` timestamp to RunningAppInfo - Create garbage collection system in process_manager.ts - Add IPC handlers for tracking selected app and getting running apps - Modify PreviewPanel to not stop previous app when switching - Clean up all running apps on app quit Closes dyad-sh#2819 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
wwwillchen-bot
temporarily deployed
to
ai-bots
GitHub Actions
Inactive
wwwillchen-bot
had a problem deploying
to
ai-bots
GitHub Actions
Failure
|
@BugBot run |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's multitasking capabilities by allowing users to run multiple Dyad apps simultaneously without interruption when switching between them. It introduces a robust garbage collection system to manage resources efficiently by automatically terminating idle apps after a set period, while ensuring the currently active app remains untouched. Additionally, it improves application shutdown behavior by ensuring all background processes are properly terminated. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Greptile SummaryEnabled multiple Dyad apps to run concurrently by removing automatic app stopping on switch, implementing a garbage collector that stops apps idle for 10+ minutes (excluding currently selected app), and adding proper cleanup on application quit. Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant PreviewPanel
participant Backend
participant GC as Garbage Collector
participant RunningApps as Running Apps Map
Note over Backend,GC: App Startup
Backend->>GC: startAppGarbageCollection()
GC->>GC: Schedule next run in 1 min
Note over User,RunningApps: User switches to App 1
User->>PreviewPanel: Select App 1
PreviewPanel->>Backend: selectAppForPreview(appId: 1)
Backend->>RunningApps: setCurrentlySelectedAppId(1)
Backend->>RunningApps: updateAppLastViewed(1)
PreviewPanel->>Backend: runApp(1)
Backend->>RunningApps: Add app 1 with lastViewedAt=now
Note over User,RunningApps: User switches to App 2 (App 1 keeps running)
User->>PreviewPanel: Select App 2
PreviewPanel->>Backend: selectAppForPreview(appId: 2)
Backend->>RunningApps: setCurrentlySelectedAppId(2)
Backend->>RunningApps: updateAppLastViewed(2)
PreviewPanel->>Backend: runApp(2)
Backend->>RunningApps: Add app 2 with lastViewedAt=now
Note over GC,RunningApps: After 10+ minutes of App 1 being idle
GC->>GC: garbageCollectIdleApps()
GC->>RunningApps: Check all apps
RunningApps-->>GC: App 1: idle 10+ min, not selected
RunningApps-->>GC: App 2: selected (protected)
GC->>RunningApps: stopAppByInfo(app 1)
GC->>GC: Schedule next run in 1 min
Note over User,Backend: User quits application
User->>Backend: Quit
Backend->>GC: stopAppGarbageCollection()
Backend->>RunningApps: stopAllAppsSync()
RunningApps->>RunningApps: Send SIGTERM to all apps
Last reviewed commit: 06a9a08 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
🎭 Playwright Test Results❌ Some tests failed
Summary: 233 passed, 3 failed, 5 flaky, 6 skipped Failed Tests🍎 macOS
📋 Re-run Failing Tests (macOS)Copy and paste to re-run all failing spec files locally: npm run e2e \
e2e-tests/fix_error.spec.ts \
e2e-tests/refresh.spec.ts \
e2e-tests/select_component.spec.ts
|
- Fix async will-quit handler: make synchronous to prevent orphaned processes - Switch GC from setInterval to recursive setTimeout to prevent overlapping runs - Use electron-log instead of console.log in process_manager for consistency - Store and re-emit proxy URL for already-running apps to preserve preview - Add cancellation pattern to notifyAppSelected to prevent race conditions - Add stopAllAppsSync for synchronous cleanup during app quit Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
✅ Claude Code completed successfullySummary
Review Comments Addressed
CI Checks
Details of code changesCritical fix — async GC timer fix (Thread 1): Logging consistency (Thread 2): Preview URL preservation (Thread 4): Race condition fix (Thread 5): |
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/ipc/utils/process_manager.ts">
<violation number="1" location="src/ipc/utils/process_manager.ts:301">
P1: `spawn("docker", ...)` is used without an `error` listener in `stopAllAppsSync`, which can raise an unhandled `error` event and crash the app on quit if Docker is unavailable.</violation>
</file>
<file name="src/ipc/handlers/app_handlers.ts">
<violation number="1" location="src/ipc/handlers/app_handlers.ts:1023">
P1: Re-emitting a cached proxy URL for already-running apps can emit stale/dead URLs after another app start terminates the previous global proxy worker.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (appInfo.isDocker) { | ||
| const containerName = appInfo.containerName || `dyad-app-${appId}`; | ||
| // Fire-and-forget: spawn docker stop without awaiting | ||
| spawn("docker", ["stop", containerName], { stdio: "ignore" }); |
There was a problem hiding this comment.
P1: spawn("docker", ...) is used without an error listener in stopAllAppsSync, which can raise an unhandled error event and crash the app on quit if Docker is unavailable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ipc/utils/process_manager.ts, line 301:
<comment>`spawn("docker", ...)` is used without an `error` listener in `stopAllAppsSync`, which can raise an unhandled `error` event and crash the app on quit if Docker is unavailable.</comment>
<file context>
@@ -217,51 +222,95 @@ export async function garbageCollectIdleApps(): Promise<void> {
+ if (appInfo.isDocker) {
+ const containerName = appInfo.containerName || `dyad-app-${appId}`;
+ // Fire-and-forget: spawn docker stop without awaiting
+ spawn("docker", ["stop", containerName], { stdio: "ignore" });
+ logger.info(`Sent docker stop for app ${appId} (${containerName})`);
+ } else if (appInfo.process.pid) {
</file context>
| spawn("docker", ["stop", containerName], { stdio: "ignore" }); | |
| const stop = spawn("docker", ["stop", containerName], { stdio: "ignore" }); | |
| stop.on("error", (err) => { | |
| logger.warn(`Failed to stop docker container for app ${appId} (${containerName}): ${err.message}`); | |
| }); |
| logger.debug(`App ${appId} is already running.`); | ||
| // Re-emit the proxy URL so the frontend can restore the preview | ||
| const appInfo = runningApps.get(appId); | ||
| if (appInfo?.proxyUrl && appInfo?.originalUrl) { |
There was a problem hiding this comment.
P1: Re-emitting a cached proxy URL for already-running apps can emit stale/dead URLs after another app start terminates the previous global proxy worker.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ipc/handlers/app_handlers.ts, line 1023:
<comment>Re-emitting a cached proxy URL for already-running apps can emit stale/dead URLs after another app start terminates the previous global proxy worker.</comment>
<file context>
@@ -1012,6 +1018,15 @@ export function registerAppHandlers() {
logger.debug(`App ${appId} is already running.`);
+ // Re-emit the proxy URL so the frontend can restore the preview
+ const appInfo = runningApps.get(appId);
+ if (appInfo?.proxyUrl && appInfo?.originalUrl) {
+ safeSend(event.sender, "app:output", {
+ type: "stdout",
</file context>
There was a problem hiding this comment.
🔴 Singleton proxyWorker is terminated when a second app starts, breaking the first app's preview
With the new concurrent app model, multiple apps can run simultaneously. However, proxyWorker is a single module-level variable. When executeApp is called for any app, it unconditionally terminates the existing proxy worker at lines 177-179. This means starting App B kills App A's proxy, silently breaking App A's preview iframe.
Root cause and impact
The proxy worker forwards requests from the preview iframe to each app's local dev server. It is started per-app in listenToProcess at src/ipc/handlers/app_handlers.ts:349 when a localhost URL is detected in stdout:
proxyWorker = await startProxy(urlMatch[1], { ... });But proxyWorker is a single let variable (src/ipc/handlers/app_handlers.ts:156). When executeApp is called for the second app, lines 177-179 terminate the previous worker:
if (proxyWorker) {
proxyWorker.terminate();
proxyWorker = null;
}Previously this was fine because only one app ran at a time. Now with concurrent apps, this terminates App A's proxy when App B starts, making App A's preview unreachable. The proxyUrl/originalUrl stored in RunningAppInfo at src/ipc/handlers/app_handlers.ts:352-355 will reference a dead proxy, and re-emitting these URLs on app switch (src/ipc/handlers/app_handlers.ts:1023-1029) will point the iframe at a terminated worker.
Impact: The core feature of this PR (keeping multiple apps running) is undermined — only the most recently started app will have a working preview.
(Refers to lines 177-179)
Prompt for agents
The proxyWorker variable in src/ipc/handlers/app_handlers.ts (line 156) is a single module-level variable, but now multiple apps run concurrently and each needs its own proxy worker. To fix this:
1. Remove the singleton `let proxyWorker: Worker | null = null;` at line 156.
2. Remove the termination of proxyWorker in executeApp (lines 177-179).
3. Store the proxy Worker reference inside `RunningAppInfo` (in src/ipc/utils/process_manager.ts), alongside the existing `proxyUrl` and `originalUrl` fields.
4. When starting a new proxy in `listenToProcess` (line 349), store the worker in the app's `RunningAppInfo` entry rather than the module-level variable.
5. When stopping an app via `stopAppByInfo` in src/ipc/utils/process_manager.ts, also terminate that app's proxy worker if it exists.
6. Consider whether the proxy worker for an already-running app should be re-used (it currently would only be started once since the URL match only fires on process stdout).
Was this helpful? React with 👍 or 👎 to provide feedback.
| for (const appId of appsToStop) { | ||
| const appInfo = runningApps.get(appId); | ||
| if (appInfo) { | ||
| logger.info(`Garbage collecting idle app ${appId}`); | ||
| try { | ||
| await stopAppByInfo(appId, appInfo); | ||
| } catch (error) { | ||
| logger.error(`Failed to garbage collect app ${appId}:`, error); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Garbage collector calls stopAppByInfo without withLock, racing with lock-protected app operations
The garbageCollectIdleApps function stops apps by calling stopAppByInfo directly, without acquiring the per-app lock via withLock. Meanwhile, runApp, stopApp, and restartApp handlers all protect their operations with withLock(appId, ...). This creates a race condition where GC and a user-initiated operation can operate on the same app concurrently.
Race condition scenario
Consider the following interleaving:
- GC reads
appInfofromrunningAppsfor app 1 and callsstopAppByInfo(1, appInfo)atsrc/ipc/utils/process_manager.ts:227 stopAppByInfocallsawait killProcess(appInfo.process)atsrc/ipc/utils/process_manager.ts:131, which awaits the process close/timeout- During that
await, the user triggersrestartAppfor app 1, which acquires the lock atsrc/ipc/handlers/app_handlers.ts:1122 restartAppstops the old process, starts a new one, and adds the new entry torunningAppsrestartAppreleases the lock- GC's
killProcessresolves;stopAppByInfothen executesrunningApps.delete(appId)atsrc/ipc/utils/process_manager.ts:133 - This deletes the new app entry that
restartAppjust created
Impact: The freshly started app is removed from runningApps even though its process is still alive. The app becomes invisible to the system — it won't be tracked, can't be stopped via the UI, and its port won't be cleaned up properly. The orphaned process will continue running until the user quits Dyad.
Prompt for agents
In src/ipc/utils/process_manager.ts, the garbageCollectIdleApps function (lines 201-239) calls stopAppByInfo directly without acquiring the per-app lock. This races with lock-protected handlers (runApp, stopApp, restartApp) in src/ipc/handlers/app_handlers.ts.
To fix this, either:
Option A (recommended): Import and use withLock from src/ipc/utils/lock_utils.ts in the GC loop. Change the stop loop (lines 222-231) to acquire the lock for each app before stopping it:
for (const appId of appsToStop) {
try {
await withLock(appId, async () => {
const appInfo = runningApps.get(appId);
if (appInfo) {
// Re-check idle time under lock in case the app was viewed/restarted while waiting
const now = Date.now();
if (appId !== currentlySelectedAppId && (now - appInfo.lastViewedAt) >= IDLE_TIMEOUT_MS) {
await stopAppByInfo(appId, appInfo);
}
}
});
} catch (error) {
logger.error(`Failed to garbage collect app ${appId}:`, error);
}
}
Option B: Use removeAppIfCurrentProcess instead of the unconditional runningApps.delete in stopAppByInfo, so that if the process was replaced, the new entry won't be deleted. However, this requires changing stopAppByInfo's semantics which may affect other callers.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06a9a081e2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (appInfo?.proxyUrl && appInfo?.originalUrl) { | ||
| safeSend(event.sender, "app:output", { | ||
| type: "stdout", | ||
| message: `[dyad-proxy-server]started=[${appInfo.proxyUrl}] original=[${appInfo.originalUrl}]`, | ||
| appId, |
There was a problem hiding this comment.
This branch re-emits a cached proxy URL for already-running apps, but after enabling concurrent app processes that URL can be stale: executeApp still unconditionally terminates the singleton proxyWorker before starting another app (src/ipc/handlers/app_handlers.ts:177-179). In the flow “run A, run B, switch back to A”, app A stays in runningApps but its proxy worker has been killed, so this message restores a dead URL and the preview fails to load. Please track proxy workers per app (or stop terminating a global worker) before treating cached URLs as reusable.
Useful? React with 👍 / 👎.
| if (appId !== null) { | ||
| updateAppLastViewed(appId); | ||
| } |
There was a problem hiding this comment.
setCurrentlySelectedAppId only refreshes lastViewedAt for the newly selected app, so the app being deselected keeps an old timestamp. Because garbage collection compares now - lastViewedAt, an app that was open for more than 10 minutes can be collected almost immediately after switching away (next GC tick) even though it was just viewed. Update the previous selection's lastViewedAt on selection change (or on deselection) so “idle for 10+ minutes” is measured from when viewing actually stopped.
Useful? React with 👍 / 👎.
2 participants
Summary
lastViewedAttimestamp on running apps to determine eligibility for GCTest plan
Closes #2819
🤖 Generated with Claude Code