fix(dev): CLI now properly cleans up the store dir on dev CLI exit #2744
Conversation
|
WalkthroughThe changes update the temporary directory management system by adding an optional parameter to the Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ericallam
force-pushed
the
ea-branch-106
branch
from
2337528 to
249cdf7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/cli-v3/src/utilities/tempDirectories.ts (1)
68-82: Consider guarding against multiple exit handler registrations.Each call to
getStoreDirregisters a new exit handler. If the function is called multiple times (e.g., across multiple dev sessions in the same process), multiple handlers will be registered for the same directory. Whilefs.rmSyncwithforce: trueis idempotent and the current usage pattern calls this once per session, you could add a guard to prevent duplicate registrations.Optional enhancement using a module-level flag:
+let storeExitHandlerRegistered = false; + export function getStoreDir(projectRoot: string | undefined, keep: boolean = false): string { projectRoot ??= process.cwd(); const storeDir = path.join(projectRoot, ".trigger", "tmp", "store"); fs.mkdirSync(storeDir, { recursive: true }); // Register exit handler to clean up the store directory - if (!keep && !process.env.KEEP_TMP_DIRS) { + if (!keep && !process.env.KEEP_TMP_DIRS && !storeExitHandlerRegistered) { + storeExitHandlerRegistered = true; onExit(() => { try { fs.rmSync(storeDir, { recursive: true, force: true }); } catch (e) { // This sometimes fails on Windows with EBUSY } }); } return storeDir; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/strange-beds-poke.md(1 hunks)packages/cli-v3/src/dev/devSession.ts(1 hunks)packages/cli-v3/src/utilities/tempDirectories.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
packages/cli-v3/src/dev/devSession.tspackages/cli-v3/src/utilities/tempDirectories.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
packages/cli-v3/src/dev/devSession.tspackages/cli-v3/src/utilities/tempDirectories.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
packages/cli-v3/src/dev/devSession.tspackages/cli-v3/src/utilities/tempDirectories.ts
🧠 Learnings (11)
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Run `npx trigger.devlatest dev` to start the Trigger.dev development server
Applied to files:
.changeset/strange-beds-poke.md
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Run `npx trigger.devlatest init` to initialize a Trigger.dev project
Applied to files:
.changeset/strange-beds-poke.md
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Configure build process in trigger.config.ts using `build` object with external packages, extensions, and JSX settings
Applied to files:
packages/cli-v3/src/dev/devSession.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Use build extensions in trigger.config.ts (additionalFiles, additionalPackages, aptGet, prismaExtension, etc.) to customize the build
Applied to files:
packages/cli-v3/src/dev/devSession.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Configure Trigger.dev project in `trigger.config.ts` using `defineConfig()` with project ref and task directories
Applied to files:
packages/cli-v3/src/dev/devSession.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks
Applied to files:
packages/cli-v3/src/dev/devSession.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use the `task()` function from `trigger.dev/sdk/v3` to define tasks with id and run properties
Applied to files:
packages/cli-v3/src/dev/devSession.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to packages/trigger-sdk/**/*.{ts,tsx} : In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
Applied to files:
packages/cli-v3/src/dev/devSession.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from `trigger.dev/core` in the webapp, use subpath exports from the package.json instead of importing from the root path
Applied to files:
packages/cli-v3/src/dev/devSession.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: The SDK at packages/trigger-sdk is an isomorphic TypeScript SDK
Applied to files:
packages/cli-v3/src/dev/devSession.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use logger methods (debug, log, info, warn, error) from `trigger.dev/sdk/v3` for structured logging in tasks
Applied to files:
packages/cli-v3/src/dev/devSession.ts
🧬 Code graph analysis (1)
packages/cli-v3/src/dev/devSession.ts (1)
packages/cli-v3/src/utilities/tempDirectories.ts (1)
getStoreDir(68-85)
🪛 LanguageTool
.changeset/strange-beds-poke.md
[style] ~5-~5: Consider an alternative adverb to strengthen your wording.
Context: ...gger.dev": patch --- fix(dev): CLI now properly cleans up the store dir on dev CLI exit...
(PROPERLY_THOROUGHLY)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
.changeset/strange-beds-poke.md (1)
1-5: LGTM!The changeset correctly documents the patch-level fix for store directory cleanup on dev CLI exit.
packages/cli-v3/src/dev/devSession.ts (1)
61-61: LGTM!The
keepTmpFilesparameter is correctly passed togetStoreDir, enabling control over temporary directory cleanup behavior.packages/cli-v3/src/utilities/tempDirectories.ts (1)
66-66: LGTM!The documentation correctly describes the new automatic cleanup behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli-v3/src/utilities/tempDirectories.ts (1)
62-67: Doc comment should reflect that cleanup is conditional.The comment states the store directory is "Automatically cleaned up when the process exits", but the implementation at line 74 shows cleanup only happens when both
keepis false ANDKEEP_TMP_DIRSis not set. Update the doc comment:- * Automatically cleaned up when the process exits. + * Automatically cleaned up when the process exits, unless KEEP_TMP_DIRS is set + * or the caller passes `keep = true`.
🧹 Nitpick comments (2)
packages/cli-v3/src/utilities/tempDirectories.ts (2)
9-12: Prefertypealias overinterfaceforEphemeralDirectory.This is pre-existing, but TS guidelines here favor
typealiases overinterface. When you next touch this, consider:-export interface EphemeralDirectory { +export type EphemeralDirectory = { path: string; remove(): void; -} +};
73-82: Exit-handler registration is fine, but consider deduping and sharing safermlogic.Each
getStoreDircall (withkeep === falseand noKEEP_TMP_DIRS) registers a newonExithandler, even if the store dir path is identical. This is safe but can add redundant listeners. Also, the “rmSync with EBUSY guard” logic is now duplicated in three places in this file.If this path is hit often in a long-lived process, consider:
- Tracking a
Set<string>of store dirs to ensure you only register one handler per dir, and/or- Extracting a small
safeRemoveDir(dir: string)helper reused bygetTmpDir,clearTmpDirs, and this exit handler.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/cli-v3/src/dev/devSession.ts(1 hunks)packages/cli-v3/src/utilities/tempDirectories.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli-v3/src/dev/devSession.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
packages/cli-v3/src/utilities/tempDirectories.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
packages/cli-v3/src/utilities/tempDirectories.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
packages/cli-v3/src/utilities/tempDirectories.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (1)
packages/cli-v3/src/utilities/tempDirectories.ts (1)
68-68: Verify call sites to ensure the default parameter change doesn't break existing persistence requirements.The prior behavior is unclear in the current repository context, so manual verification is needed. The suggestion to audit call sites with
rg -n "getStoreDir\(" packages/cli-v3remains valid. Specifically:
- Check each call site to determine if it explicitly passes
keep=trueor relies on the prior default behavior- Verify that dev/test code paths handle the new auto-cleanup default correctly
- If any production code relies on persistence without explicitly passing
keep=true, either update call sites or reconsider the default valueConsider keeping the default as
keep=truefor backward compatibility unless dev session cleanup is explicitly opt-in elsewhere.
No description provided.