fix(cli): make dev shutdown reliable - #206
Draft
AmanVarshney01 wants to merge 1 commit into
Draft
Conversation
Summary by CodeRabbit
WalkthroughThe CLI now awaits file-watcher and attachment-service cleanup during development shutdown. A shutdown controller handles 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
commit: |
Wait for file watchers to close before reporting shutdown complete, and let a second signal force termination when graceful cleanup stalls. Cover the shutdown controller with regression tests. Signed-off-by: Aman Varshney <amanvarshney.work@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/0-framework/3-tooling/cli/src/dev/run-dev.ts`:
- Around line 303-314: Update the shutdown flow around
createDevShutdownController and the watch rebuild callback to track active
detached rebuild tasks, set a shutdown flag that prevents any pending or resumed
task from deploying, and await or cancel all tracked tasks before
attachment.stopServices completes and “[dev] stopped.” is logged.
- Around line 69-78: Update the cleanup handling in run-dev.ts around the
shutdown controller so cleanup is invoked via Promise.resolve().then(cleanup),
routing synchronous and asynchronous failures through the existing rejection
branch and ensuring done rejects. Add a regression test in
packages/0-framework/3-tooling/cli/src/dev/__tests__/run-dev.test.ts covering a
synchronously throwing cleanup and asserting shutdown.done rejects.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2a8a7fd0-0af2-4361-9f00-b66aa86d1911
📒 Files selected for processing (6)
docs/guides/running-locally.mdpackages/0-framework/3-tooling/cli/src/dev/__tests__/run-dev.test.tspackages/0-framework/3-tooling/cli/src/dev/__tests__/watch.test.tspackages/0-framework/3-tooling/cli/src/dev/run-dev.tspackages/0-framework/3-tooling/cli/src/dev/watch.tsskills/prisma-composer/SKILL.md
Comment on lines
+69
to
+78
| void cleanup().then( | ||
| () => { | ||
| state = 'stopped'; | ||
| resolve(); | ||
| }, | ||
| (error: unknown) => { | ||
| state = 'stopped'; | ||
| reject(error); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Route synchronous cleanup failures through done.
cleanup() can throw before it returns a promise. The controller has already entered stopping, but done never resolves or rejects.
packages/0-framework/3-tooling/cli/src/dev/run-dev.ts#L69-L78: invoke cleanup throughPromise.resolve().then(cleanup)so synchronous and asynchronous failures use the rejection branch.packages/0-framework/3-tooling/cli/src/dev/__tests__/run-dev.test.ts#L27-L78: add a regression test wherecleanupthrows synchronously and assert thatshutdown.donerejects.
Proposed fix
- void cleanup().then(
+ void Promise.resolve().then(cleanup).then(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| void cleanup().then( | |
| () => { | |
| state = 'stopped'; | |
| resolve(); | |
| }, | |
| (error: unknown) => { | |
| state = 'stopped'; | |
| reject(error); | |
| }, | |
| ); | |
| void Promise.resolve().then(cleanup).then( | |
| () => { | |
| state = 'stopped'; | |
| resolve(); | |
| }, | |
| (error: unknown) => { | |
| state = 'stopped'; | |
| reject(error); | |
| }, | |
| ); |
📍 Affects 2 files
packages/0-framework/3-tooling/cli/src/dev/run-dev.ts#L69-L78(this comment)packages/0-framework/3-tooling/cli/src/dev/__tests__/run-dev.test.ts#L27-L78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/0-framework/3-tooling/cli/src/dev/run-dev.ts` around lines 69 - 78,
Update the cleanup handling in run-dev.ts around the shutdown controller so
cleanup is invoked via Promise.resolve().then(cleanup), routing synchronous and
asynchronous failures through the existing rejection branch and ensuring done
rejects. Add a regression test in
packages/0-framework/3-tooling/cli/src/dev/__tests__/run-dev.test.ts covering a
synchronously throwing cleanup and asserting shutdown.done rejects.
Comment on lines
+303
to
314
| const shutdown = createDevShutdownController(async () => { | ||
| console.log("[dev] stopping — the app's services are stopping; emulators and data stay up."); | ||
| await Promise.all([ | ||
| watch.stop(), | ||
| (async () => { | ||
| for (const attachment of attachments) { | ||
| await attachment.stopServices().catch(() => undefined); | ||
| } | ||
| console.log('[dev] stopped.'); | ||
| resolve(); | ||
| })(); | ||
| }; | ||
|
|
||
| // alchemy's own library code (imported transitively while loading the | ||
| // app's config/providers) registers its own process-level SIGINT/SIGTERM | ||
| // listeners for ITS OWN in-process resource bookkeeping — irrelevant | ||
| // here, since the actual converge runs in a separate spawned `alchemy` | ||
| // child process (run-alchemy.ts), never in this one. Left in place, | ||
| // whichever of its listeners runs first can call process.exit() | ||
| // synchronously and tear this process down before the watch loop's own | ||
| // async cleanup (stopping the app's services) ever gets a turn. This is | ||
| // this process's OWN signal handling from here on: strip whatever else | ||
| // is registered and become the only listener. | ||
| process.removeAllListeners('SIGINT'); | ||
| process.removeAllListeners('SIGTERM'); | ||
| process.on('SIGINT', finish); | ||
| process.on('SIGTERM', finish); | ||
| })(), | ||
| ]); | ||
| console.log('[dev] stopped.'); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Prevent an active rebuild from deploying after shutdown starts.
The watch callback starts a detached rebuild task. Shutdown stops the watcher and services, but it does not cancel or await a rebuild that is already awaiting runPipeline. That task can resume and run a deploy after attachment.stopServices() completes.
Track active rebuild tasks and block further deploy work when shutdown begins. Wait for or cancel those tasks before logging [dev] stopped.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/0-framework/3-tooling/cli/src/dev/run-dev.ts` around lines 303 -
314, Update the shutdown flow around createDevShutdownController and the watch
rebuild callback to track active detached rebuild tasks, set a shutdown flag
that prevents any pending or resumed task from deploying, and await or cancel
all tracked tasks before attachment.stopServices completes and “[dev] stopped.”
is logged.
AmanVarshney01
marked this pull request as draft
August 7, 2026 11:43
AmanVarshney01
force-pushed
the
codex/fix-dev-shutdown
branch
from
August 7, 2026 11:43
501ee24 to
821d3c6
Compare
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
1 participant
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.
DO NOT REVIEW THIS PR!
Blocked by alchemy-run/node-utils#6 and the follow-up Alchemy dependency release.
Summary
Wait for dev watchers to close during graceful shutdown. Once the upstream fix is released, bump Alchemy and remove Composer’s process-wide signal-listener workaround.
Testing
pnpm buildpnpm typecheckpnpm lintpnpm test: 62/63 Turbo tasks passed; the integration fixture is missing its localprisma-composerbinary link