fix: Ensure FileStorage.close() is called in export operations#1586
fix: Ensure FileStorage.close() is called in export operations#1586bjcoombs wants to merge 6 commits intoeyaltoledano:nextfrom
Conversation
Wraps FileStorage usage in try/finally blocks to ensure close() is always called, even when errors occur. This prevents resource leaks in export operations. Fixes 6 locations: - 4 in apps/cli/src/commands/export.command.ts - 2 in packages/tm-core/src/modules/integration/services/export.service.ts Includes tests verifying close() is called on both success and error paths. Closes eyaltoledano#1585
🦋 Changeset detectedLatest commit: d571bd1 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on February 9. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughWraps FileStorage usage in try/finally blocks across CLI export command and core export service to ensure FileStorage.close() is always called; adds tests asserting cleanup behavior and call ordering. Changes
Sequence Diagram(s)(Skipped — changes are resource-cleanup focused and do not introduce a new multi-component sequential feature warranting a diagram.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cli/src/commands/export.command.ts (1)
723-790: Ensure FileStorage is closed ifinitialize()fails and always stop the spinner.
initialize()runs before the try/finally, so an init failure skipsclose()and can leave the spinner running. Move initialization and spinner stop into the guarded block.🐛 Suggested fix
const fileStorage = new FileStorage(projectRoot); -await fileStorage.initialize(); let results: ExportResult[]; try { + await fileStorage.initialize(); const exportPromises: Promise<ExportResult>[] = tags.map(async (tag) => { try { // Load tasks from LOCAL FileStorage to count parent tasks vs subtasks const tasks = await fileStorage.loadTasks(tag); @@ // Wait for all exports to complete results = await Promise.all(exportPromises); - spinner.stop(); } finally { + spinner.stop(); await fileStorage.close(); }
🤖 Fix all issues with AI agents
In `@packages/tm-core/src/modules/integration/services/export.service.spec.ts`:
- Around line 8-26: The file fails CI due to Biome formatting issues; run the
project formatter (e.g., `biome format .`) and commit the reformatted file so
the mock definitions (mockInitialize, mockLoadTasks, mockClose) and classes
(MockFileStorage, MockAuthDomain) follow the project's style rules; ensure
trailing commas, spacing, and indenting for the hoisted vi.hoisted block are
corrected and then push the updated file.
🧹 Nitpick comments (2)
packages/tm-core/src/modules/integration/services/export.service.ts (1)
418-447: Preventclose()from masking the primary failure.If
initialize()/loadTasks()throws, aclose()error would replace the original exception. Consider guarding close errors to preserve the root cause.♻️ Suggested tweak
} finally { - await fileStorage.close(); + try { + await fileStorage.close(); + } catch { + // Optional: log via existing logger if available + } }packages/tm-core/src/modules/integration/services/export.service.spec.ts (1)
119-129: Restore stubbedfetchbetween tests to avoid leakage.Overwriting
global.fetchcan leak across tests. Prefervi.stubGlobal('fetch', ...)andvi.unstubAllGlobals()(or restore the original) for isolation.🔧 Example adjustment
- global.fetch = vi.fn().mockResolvedValue({ + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: () => Promise.resolve({ successCount: 1, totalTasks: 1, failedCount: 0, results: [] }) - }); + }));You can add this near
beforeEach:afterEach(() => { vi.unstubAllGlobals(); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/fix-filestorage-cleanup.mdapps/cli/src/commands/export.command.tspackages/tm-core/src/modules/integration/services/export.service.spec.tspackages/tm-core/src/modules/integration/services/export.service.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
TypeScript test files must achieve minimum code coverage thresholds: 80% lines/functions and 70% branches globally, 90% for utilities, and 85% for middleware; new features must meet or exceed these thresholds
Files:
packages/tm-core/src/modules/integration/services/export.service.tspackages/tm-core/src/modules/integration/services/export.service.spec.tsapps/cli/src/commands/export.command.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts}: Import and use specific getters from config-manager.js (e.g., getMainProvider(), getLogLevel(), getMainMaxTokens()) to access configuration values needed for application logic
Use isApiKeySet(providerName, session) from config-manager.js to check if a provider's key is available before potentially attempting an AI call
Do not add direct console.log calls outside the logging utility - use the central log function instead
Ensure silent mode is disabled in a finally block to prevent it from staying enabled
Do not access the global silentMode variable directly - use the exported silent mode control functions instead
Do not duplicate task ID formatting logic across modules - centralize formatting utilities
Use ContextGatherer class from utils/contextGatherer.js for AI-powered commands that need project context, supporting tasks, files, custom text, and project tree context
Use FuzzyTaskSearch class from utils/fuzzyTaskSearch.js for automatic task relevance detection with configurable search parameters
Use fuzzy search to supplement user-provided task IDs and display discovered task IDs to users for transparency
Do not replace explicit user task selections with fuzzy results - fuzzy search should supplement, not replace user selections
Use readJSON and writeJSON utilities for all JSON file operations instead of raw fs.readFileSync or fs.writeFileSync
Include error handling for JSON file operations and validate JSON structure after reading
Use path.join() for cross-platform path construction and path.resolve() for absolute paths, validating paths before file operations
Support both .env files and MCP session environment for environment variable resolution with fallbacks for missing values
Prefer updating the core function to accept an outputFormat parameter and check outputFormat === 'json' before displaying UI elements
Files:
packages/tm-core/src/modules/integration/services/export.service.tspackages/tm-core/src/modules/integration/services/export.service.spec.tsapps/cli/src/commands/export.command.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import modules with
.jsextension even in TypeScript source files for ESM compatibility
Files:
packages/tm-core/src/modules/integration/services/export.service.tspackages/tm-core/src/modules/integration/services/export.service.spec.tsapps/cli/src/commands/export.command.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place package and app test files in
packages/<package-name>/src/<module>/<file>.spec.tsorapps/<app-name>/src/<module>/<file>.spec.tsalongside source files
Files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
**/*.{spec,test}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{spec,test}.ts: Always use.tsfor TypeScript tests, never.js
NEVER use async/await in test functions unless testing actual asynchronous operations; use synchronous top-level imports instead of dynamicawait import()
Files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
packages/tm-core/**/*.{spec,test}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In unit tests for
@tm/core, mock only external I/O (Supabase, APIs, filesystem) and use real internal services
Files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
apps/cli/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
CLI (
@tm/cli) should be a thin presentation layer that calls tm-core methods and displays results; handle only CLI-specific concerns like argument parsing, output formatting, and user prompts
Files:
apps/cli/src/commands/export.command.ts
🧠 Learnings (39)
📚 Learning: 2025-10-08T19:57:00.982Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1282
File: packages/tm-core/src/utils/index.ts:16-34
Timestamp: 2025-10-08T19:57:00.982Z
Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-09-26T19:05:47.555Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1252
File: packages/ai-sdk-provider-grok-cli/package.json:11-13
Timestamp: 2025-09-26T19:05:47.555Z
Learning: In the eyaltoledano/claude-task-master repository, internal tm/ packages use a specific export pattern where the "exports" field points to TypeScript source files (./src/index.ts) while "main" points to compiled output (./dist/index.js) and "types" points to source files (./src/index.ts). This pattern is used consistently across internal packages like tm/core and tm/ai-sdk-provider-grok-cli because they are consumed directly during build-time bundling with tsdown rather than being published as separate packages.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:01:44.169Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.169Z
Learning: Applies to mcp-server/src/core/task-master-core.js : Update `task-master-core.js` by importing and re-exporting direct functions and adding them to the directFunctions map
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to scripts/modules/**/*.{js,ts} : Implement complete migration functions for tagged task lists that handle configuration, state file creation, and migration status tracking
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T22:09:45.455Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.455Z
Learning: Applies to packages/tm-core/**/*.{spec,test}.ts : In unit tests for tm/core, mock only external I/O (Supabase, APIs, filesystem) and use real internal services
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tspackages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Use try/catch blocks for all file operations and return null or a default value on failure rather than allowing exceptions to propagate unhandled
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.ts
📚 Learning: 2025-11-24T18:02:36.388Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-11-24T18:02:36.388Z
Learning: Applies to scripts/modules/task-manager.js : Format task files with consistent structure including task metadata (ID, title, status), dependencies with status indicators, and tag context information in the file header
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to scripts/modules/**/*.{js,ts} : Implement silent migration for tasks.json files that transforms old format to tagged format, marking global flag and performing complete migration
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T17:59:00.056Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-11-24T17:59:00.056Z
Learning: Applies to scripts/modules/utils/{contextGatherer,fuzzyTaskSearch}.js : Export utility modules using named exports: `ContextGatherer`, `createContextGatherer` from contextGatherer.js and `FuzzyTaskSearch`, `PURPOSE_CATEGORIES`, `RELEVANCE_THRESHOLDS` from fuzzyTaskSearch.js
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.ts
📚 Learning: 2025-11-24T17:58:07.992Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/architecture.mdc:0-0
Timestamp: 2025-11-24T17:58:07.992Z
Learning: Applies to scripts/modules/task-manager.js : task-manager.js should handle reading/writing tasks.json with tagged task lists support, implement CRUD operations, delegate AI interactions to ai-services-unified.js layer, and access non-AI configuration via config-manager.js getters
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.ts
📚 Learning: 2025-11-24T18:02:04.644Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tags.mdc:0-0
Timestamp: 2025-11-24T18:02:04.644Z
Learning: Applies to scripts/modules/task-manager/**/*.{js,mjs} : Use `readJSON(tasksPath, projectRoot, tag)` and `writeJSON(tasksPath, data, projectRoot, tag)` for all task data access
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:01:44.169Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.169Z
Learning: Applies to scripts/modules/*.js : Design core logic to work with both legacy (flat tasks array) and tagged task data formats; use tag resolution functions (getTasksForTag, setTasksForTag) for task data access; support silent migration during feature usage
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:01:44.169Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.169Z
Learning: Applies to scripts/modules/*.js : Process large task lists efficiently; minimize file I/O operations per feature execution; cache tag resolution results when appropriate; avoid keeping all tag data in memory simultaneously
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:02:36.388Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-11-24T18:02:36.388Z
Learning: Applies to scripts/modules/task-manager.js : Extract tasks from PRD documents using AI within the current tag context (defaulting to "master"), providing clear prompts to guide AI task generation and validating/cleaning up AI-generated tasks
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:02:36.388Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-11-24T18:02:36.388Z
Learning: Applies to scripts/modules/task-manager.js : Filter and display tasks by status within the current tag context, handling subtask display in lists and using consistent table formats
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.ts
📚 Learning: 2025-11-24T18:02:04.644Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tags.mdc:0-0
Timestamp: 2025-11-24T18:02:04.644Z
Learning: Every command that reads or writes tasks.json must be tag-aware and support the tagged task lists system
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:02:04.644Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tags.mdc:0-0
Timestamp: 2025-11-24T18:02:04.644Z
Learning: Applies to scripts/modules/task-manager/**/*.{js,mjs} : Pass context object `{ projectRoot, tag }` to all core functions that read or write tasks
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to scripts/modules/**/*.{js,ts} : Implement tag resolution functions (getTasksForTag, setTasksForTag, getCurrentTag) that provide backward compatibility with legacy format and default to master tag
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T22:09:45.455Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.455Z
Learning: Applies to **/tests/integration/**/*.{spec,test}.ts : In integration tests, use real tm-core and mock only external boundaries (APIs, DB, filesystem)
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:46.713Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.713Z
Learning: Applies to **/*.test.js : For task file operations in tests: use test-specific file paths (e.g., 'test-tasks.json'); mock `readJSON` and `writeJSON` to avoid real file system interactions; verify file operations use correct paths; use different paths for each test; verify modifications on in-memory task objects passed to `writeJSON`.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to tests/fixtures/**/*.ts : Test fixture files must export reusable test data creators and constants (createTestUser, adminUser, invalidUser, etc.) for use across unit, integration, and e2e tests to ensure consistency
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to **/*.integration.test.ts : Integration tests must use supertest for API endpoint testing, verify database state changes after operations, clean test data before each test, and include full request/response validation with expected HTTP status codes
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to tests/e2e/**/*.test.ts : End-to-end tests must test complete user workflows across multiple API endpoints in sequence, verify state changes between workflow steps, use extended timeouts (30000ms), and validate final outcomes without mocking business logic
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:46.713Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.713Z
Learning: Applies to **/*.test.js : Do not import or instantiate real AI service clients. Create fully mocked versions that return predictable responses. Mock the entire module with controlled behavior.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:01:44.169Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.169Z
Learning: Applies to **/*.test.{js,ts} : Test new features with both legacy and tagged task data formats; verify tag resolution behavior; test migration compatibility; ensure pre-migration projects are handled correctly
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.tsapps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to tests/teardown.ts : Global teardown file must be created to handle cleanup after all tests complete and prevent worker process leaks
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T17:58:07.992Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/architecture.mdc:0-0
Timestamp: 2025-11-24T17:58:07.992Z
Learning: Each module should have well-defined exports that can be mocked in tests
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:46.713Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.713Z
Learning: Applies to **/*.test.js : Mock file system operations using `mock-fs` library. Mock API calls by providing jest.fn() implementations that return expected structures. Mock environment variables in test setup.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Use established mocking patterns from auth.test.ts as templates: mock bcrypt with proper TypeScript typing, mock Prisma client with transaction support, and always clear mocks between tests
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:46.713Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.713Z
Learning: Applies to **/*.test.js : For modules with initialization-dependent functions in tests: create test-specific implementations that initialize all variables correctly; use factory functions in mocks to ensure proper initialization order; be careful with how you mock functions that depend on module state.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to **/*.test.ts : Unit tests must follow the describe/it pattern, use beforeEach for mock setup with jest.clearAllMocks(), include specific assertions with expect(), and test both success and error scenarios including edge cases
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Export all utility functions explicitly in logical groups and include configuration constants from utility modules
Applied to files:
apps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Group related exports together in utility modules and avoid creating circular dependencies
Applied to files:
apps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Do not use default exports in utility modules - use named exports only
Applied to files:
apps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:02:04.644Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tags.mdc:0-0
Timestamp: 2025-11-24T18:02:04.644Z
Learning: Applies to scripts/modules/task-manager/**/*.{js,mjs} : Core task functions must accept a context parameter with `{ projectRoot, tag }` properties
Applied to files:
apps/cli/src/commands/export.command.ts
📚 Learning: 2025-07-18T05:38:17.352Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 943
File: scripts/modules/task-manager/move-task.js:24-24
Timestamp: 2025-07-18T05:38:17.352Z
Learning: In the Claude Task Master system, core task-manager functions are designed with fallback mechanisms for missing projectRoot parameters using the pattern `const projectRoot = providedProjectRoot || findProjectRoot();`. The readJSON and writeJSON functions have default parameters (projectRoot = null, tag = null) and handle missing parameters gracefully. Adding strict validation to these core functions would break the established flexible architecture pattern.
Applied to files:
apps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:00:06.827Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-11-24T18:00:06.827Z
Learning: Initialize projects using `task-master init` or `task-master parse-prd` to generate initial tasks.json with tagged structure
Applied to files:
apps/cli/src/commands/export.command.ts
📚 Learning: 2025-07-18T17:09:40.548Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:40.548Z
Learning: Applies to scripts/modules/dependency-manager.js : Check for and remove references to non-existent tasks during cleanup
Applied to files:
apps/cli/src/commands/export.command.ts
📚 Learning: 2025-11-24T18:02:36.388Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-11-24T18:02:36.388Z
Learning: Applies to scripts/modules/task-manager.js : Calculate and display task completion statistics for the current tag context including total tasks, completed tasks, completion percentage, and subtask completion counts using visual progress indicators
Applied to files:
apps/cli/src/commands/export.command.ts
🧬 Code graph analysis (1)
packages/tm-core/src/modules/integration/services/export.service.ts (1)
packages/tm-core/src/common/types/index.ts (1)
Task(131-159)
🪛 GitHub Actions: CI
packages/tm-core/src/modules/integration/services/export.service.spec.ts
[error] 1-39: Format check failed. The formatter would have reformatted the file. Run 'biome format .' to fix formatting issues. Differences shown in the log (mock hoisting block and return object formatting).
.changeset/fix-filestorage-cleanup.md
[error] 1-1: Changeset validation failed: Invalid package "@tm/cli". Only these packages are allowed: task-master-ai, extension.
[error] 1-1: Changeset validation failed: Invalid package "@tm/core". Only these packages are allowed: task-master-ai, extension.
🔇 Additional comments (5)
packages/tm-core/src/modules/integration/services/export.service.ts (1)
855-867: Good cleanup coverage for brief generation.Wrapping initialize/load in try/finally ensures FileStorage is always closed.
apps/cli/src/commands/export.command.ts (3)
235-242: Cleanup looks solid in tag discovery flow.The try/finally around
getTagsWithStats()ensures FileStorage is always closed.
397-405: Cleanup looks solid in standard export.Wrapping initialize/load in try/finally covers success and failure paths.
508-515: Cleanup looks solid in interactive export.FileStorage is now reliably closed even if load fails.
.changeset/fix-filestorage-cleanup.md (1)
1-3: Fix changeset package names to pass validation.CI rejects
@tm/cliand@tm/core; onlytask-master-aiandextensionare allowed. Update the frontmatter to permitted package names (or adjust the changeset config) so validation passes.⛔ Skipped due to learnings
Learnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 1002 File: .changeset/puny-friends-give.md:2-3 Timestamp: 2025-07-17T21:33:57.585Z Learning: In the eyaltoledano/claude-task-master repository, the MCP server code in mcp-server/src/ is part of the main "task-master-ai" package, not a separate "mcp-server" package. When creating changesets for MCP server changes, use "task-master-ai" as the package name.Learnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 1232 File: packages/build-config/package.json:14-15 Timestamp: 2025-09-22T19:45:13.323Z Learning: In the eyaltoledano/claude-task-master repository, Crunchyman-ralph intentionally omits version fields from internal packages (like tm/build-config) to prevent changesets from releasing new versions for these packages. This is the desired behavior for internal tooling packages that should not be published or versioned independently.Learnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 1252 File: tsconfig.json:22-28 Timestamp: 2025-09-26T19:10:32.906Z Learning: In the eyaltoledano/claude-task-master repository, all internal tm/ package path mappings in tsconfig.json consistently point to TypeScript source files (e.g., "./packages/*/src/index.ts") rather than built JavaScript. This is intentional architecture because tsdown bundles internal packages directly from source during build time, eliminating the need for separate compilation of internal packages.Learnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 1232 File: packages/tm-core/package.json:50-51 Timestamp: 2025-09-22T19:45:04.337Z Learning: In the eyaltoledano/claude-task-master project, Crunchyman-ralph intentionally omits version fields from internal/private packages in package.json files to prevent changesets from releasing new versions of these packages while still allowing them to be processed for dependency updates. The changesets warnings about missing versions are acceptable as they don't break the process and achieve the desired behavior of only releasing public packages.Learnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 1252 File: package.json:130-132 Timestamp: 2025-09-26T19:03:33.225Z Learning: In the eyaltoledano/claude-task-master repository, all packages with tm prefix are internal packages that are part of the monorepo structure and are not published externally.Learnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 1095 File: .github/scripts/check-pre-release-mode.mjs:33-34 Timestamp: 2025-08-06T21:16:02.300Z Learning: In the claude-task-master project, the .changeset directory is guaranteed to exist as part of the fundamental project structure, so validation checks for its existence are unnecessary.Learnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 1282 File: packages/tm-core/src/utils/index.ts:16-34 Timestamp: 2025-10-08T19:57:00.982Z Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.Learnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 1252 File: packages/ai-sdk-provider-grok-cli/package.json:11-13 Timestamp: 2025-09-26T19:05:47.555Z Learning: In the eyaltoledano/claude-task-master repository, internal tm/ packages use a specific export pattern where the "exports" field points to TypeScript source files (./src/index.ts) while "main" points to compiled output (./dist/index.js) and "types" points to source files (./src/index.ts). This pattern is used consistently across internal packages like tm/core and tm/ai-sdk-provider-grok-cli because they are consumed directly during build-time bundling with tsdown rather than being published as separate packages.Learnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 1252 File: package.json:130-132 Timestamp: 2025-09-26T19:03:33.225Z Learning: In the eyaltoledano/claude-task-master repository, packages are bundled using tsdown during the build process, which means dependencies imported by the source code (including tm internal packages like tm/ai-sdk-provider-grok-cli) are included in the final bundle and don't need to be available as separate runtime dependencies, so they should remain as devDependencies rather than being moved to dependencies.Learnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 997 File: apps/extension/package.publish.json:2-8 Timestamp: 2025-07-31T20:49:04.638Z Learning: In the eyaltoledano/claude-task-master repository, the VS Code extension uses a 3-file packaging system where package.json (with name "extension") is for development within the monorepo, while package.publish.json (with name "task-master-hamster") contains the clean manifest for VS Code marketplace publishing. The different names are intentional and serve distinct purposes in the build and publishing workflow.Learnt from: CR Repo: eyaltoledano/claude-task-master PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-24T22:09:45.455Z Learning: Applies to apps/cli/src/**/*.{ts,tsx} : CLI (tm/cli) should be a thin presentation layer that calls tm-core methods and displays results; handle only CLI-specific concerns like argument parsing, output formatting, and user promptsLearnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 1178 File: apps/cli/src/commands/auth.command.ts:222-224 Timestamp: 2025-09-03T12:15:03.208Z Learning: The CLI can be invoked using both "task-master" and "tm" as aliases - both forms are valid and acceptable in help text, documentation, and examples.Learnt from: CR Repo: eyaltoledano/claude-task-master PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-24T22:09:45.455Z Learning: Applies to apps/cli/**/*.{spec,test}.ts : In unit tests for apps/cli, mock tm-core responses but use real Commander/chalk/inquirer/other npm packages to test display logicLearnt from: Crunchyman-ralph Repo: eyaltoledano/claude-task-master PR: 1252 File: packages/ai-sdk-provider-grok-cli/package.json:21-35 Timestamp: 2025-09-26T19:07:10.485Z Learning: In the eyaltoledano/claude-task-master repository, the tsdown build configuration uses `noExternal: [/^tm\//]` which means internal tm/ packages are bundled into the final output while external npm dependencies remain external and are resolved from the root package.json dependencies at runtime. This eliminates the need for peer dependencies in internal packages since the root package.json already provides the required external dependencies.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
packages/tm-core/src/modules/integration/services/export.service.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/tm-core/src/modules/integration/services/export.service.spec.ts`:
- Around line 131-142: The test currently assigns global.fetch directly
(global.fetch = vi.fn()...) which can leak into other tests; replace those
direct assignments with vi.stubGlobal('fetch', vi.fn().mockResolvedValue(...))
for each mocked response (both the block around startLine 131-142 and the
similar block at 205-226) and add a cleanup hook (e.g., afterEach(() => {
vi.unstubAllGlobals(); })) to the test suite so the stubbed globals are restored
between tests; update any test-specific mock setups (the mockedResolvedValue
payload) but keep the same shape used in the existing mocks.
🧹 Nitpick comments (1)
packages/tm-core/src/modules/integration/services/export.service.spec.ts (1)
45-47: Prefer realAuthDomainin tm-core unit tests.Guidelines say to mock only external I/O;
AuthDomainis an internal service. Consider using the realAuthDomainand stub its external boundaries instead. Based on learnings, please align with tm-core unit test rules.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/fix-filestorage-cleanup.mdpackages/tm-core/src/modules/integration/services/export.service.spec.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
TypeScript test files must achieve minimum code coverage thresholds: 80% lines/functions and 70% branches globally, 90% for utilities, and 85% for middleware; new features must meet or exceed these thresholds
Files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts}: Import and use specific getters from config-manager.js (e.g., getMainProvider(), getLogLevel(), getMainMaxTokens()) to access configuration values needed for application logic
Use isApiKeySet(providerName, session) from config-manager.js to check if a provider's key is available before potentially attempting an AI call
Do not add direct console.log calls outside the logging utility - use the central log function instead
Ensure silent mode is disabled in a finally block to prevent it from staying enabled
Do not access the global silentMode variable directly - use the exported silent mode control functions instead
Do not duplicate task ID formatting logic across modules - centralize formatting utilities
Use ContextGatherer class from utils/contextGatherer.js for AI-powered commands that need project context, supporting tasks, files, custom text, and project tree context
Use FuzzyTaskSearch class from utils/fuzzyTaskSearch.js for automatic task relevance detection with configurable search parameters
Use fuzzy search to supplement user-provided task IDs and display discovered task IDs to users for transparency
Do not replace explicit user task selections with fuzzy results - fuzzy search should supplement, not replace user selections
Use readJSON and writeJSON utilities for all JSON file operations instead of raw fs.readFileSync or fs.writeFileSync
Include error handling for JSON file operations and validate JSON structure after reading
Use path.join() for cross-platform path construction and path.resolve() for absolute paths, validating paths before file operations
Support both .env files and MCP session environment for environment variable resolution with fallbacks for missing values
Prefer updating the core function to accept an outputFormat parameter and check outputFormat === 'json' before displaying UI elements
Files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place package and app test files in
packages/<package-name>/src/<module>/<file>.spec.tsorapps/<app-name>/src/<module>/<file>.spec.tsalongside source files
Files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
**/*.{spec,test}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{spec,test}.ts: Always use.tsfor TypeScript tests, never.js
NEVER use async/await in test functions unless testing actual asynchronous operations; use synchronous top-level imports instead of dynamicawait import()
Files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import modules with
.jsextension even in TypeScript source files for ESM compatibility
Files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
packages/tm-core/**/*.{spec,test}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In unit tests for
@tm/core, mock only external I/O (Supabase, APIs, filesystem) and use real internal services
Files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
🧠 Learnings (17)
📚 Learning: 2025-11-24T22:09:45.455Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.455Z
Learning: Applies to packages/tm-core/**/*.{spec,test}.ts : In unit tests for tm/core, mock only external I/O (Supabase, APIs, filesystem) and use real internal services
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T22:09:45.455Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.455Z
Learning: Applies to **/tests/integration/**/*.{spec,test}.ts : In integration tests, use real tm-core and mock only external boundaries (APIs, DB, filesystem)
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:46.713Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.713Z
Learning: Applies to **/*.test.js : For task file operations in tests: use test-specific file paths (e.g., 'test-tasks.json'); mock `readJSON` and `writeJSON` to avoid real file system interactions; verify file operations use correct paths; use different paths for each test; verify modifications on in-memory task objects passed to `writeJSON`.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to tests/fixtures/**/*.ts : Test fixture files must export reusable test data creators and constants (createTestUser, adminUser, invalidUser, etc.) for use across unit, integration, and e2e tests to ensure consistency
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to **/*.integration.test.ts : Integration tests must use supertest for API endpoint testing, verify database state changes after operations, clean test data before each test, and include full request/response validation with expected HTTP status codes
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:01:44.169Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.169Z
Learning: Applies to **/*.test.{js,ts} : Test new features with both legacy and tagged task data formats; verify tag resolution behavior; test migration compatibility; ensure pre-migration projects are handled correctly
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to tests/e2e/**/*.test.ts : End-to-end tests must test complete user workflows across multiple API endpoints in sequence, verify state changes between workflow steps, use extended timeouts (30000ms), and validate final outcomes without mocking business logic
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:46.713Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.713Z
Learning: Applies to **/*.test.js : Do not import or instantiate real AI service clients. Create fully mocked versions that return predictable responses. Mock the entire module with controlled behavior.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to tests/teardown.ts : Global teardown file must be created to handle cleanup after all tests complete and prevent worker process leaks
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to src/**/*.ts : Source code must have corresponding test files either colocated (*.test.ts) or in tests/unit/ directory following established patterns from src/utils/auth.test.ts with proper mocking for external dependencies
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to **/*.test.ts : Unit tests must follow the describe/it pattern, use beforeEach for mock setup with jest.clearAllMocks(), include specific assertions with expect(), and test both success and error scenarios including edge cases
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T17:57:14.743Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-11-24T17:57:14.743Z
Learning: Applies to tests/unit/ai-providers/*.test.js : Create unit tests in `tests/unit/ai-providers/<provider-name>.test.js` that mock the provider's AI SDK module and test each exported function for correct client instantiation, parameter passing, result handling, and error handling
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:46.713Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.713Z
Learning: Applies to **/*.test.js : When testing UI functions: mock console output and verify correct formatting; test conditional output logic; use `toContain()` or `toMatch()` rather than exact `toBe()` for strings with emojis or formatting; create separate tests for different behavior modes; test structure of formatted output rather than exact string matching.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:46.713Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.713Z
Learning: Applies to **/*.test.js : For modules with initialization-dependent functions in tests: create test-specific implementations that initialize all variables correctly; use factory functions in mocks to ensure proper initialization order; be careful with how you mock functions that depend on module state.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T17:58:07.992Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/architecture.mdc:0-0
Timestamp: 2025-11-24T17:58:07.992Z
Learning: Each module should have well-defined exports that can be mocked in tests
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:46.713Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.713Z
Learning: Applies to **/*.test.js : Mock file system operations using `mock-fs` library. Mock API calls by providing jest.fn() implementations that return expected structures. Mock environment variables in test setup.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Use established mocking patterns from auth.test.ts as templates: mock bcrypt with proper TypeScript typing, mock Prisma client with transaction support, and always clear mocks between tests
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.spec.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). (1)
- GitHub Check: Test
🔇 Additional comments (1)
.changeset/fix-filestorage-cleanup.md (1)
1-5: Looks good — clear patch note.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
packages/tm-core/src/modules/integration/services/export.service.spec.ts
Show resolved
Hide resolved
- Use vi.stubGlobal/unstubAllGlobals for fetch mocking to prevent test leakage - Move initialize() inside try block in performExportAll for proper cleanup
Crunchyman-ralph
left a comment
Crunchyman-ralph
left a comment
There was a problem hiding this comment.
Please rebase based on next, remember, when creating PRs, always point them to next.
Regarding your proposed fixed, it feels like there's a lot of boilerplate to using the filestorage, there has to be a better way, we have built this in a way to abstract complexity away, we can't allow ourselves to add more boilerplate complexity.
Maybe you can have tmcore do the heavy lifting with the closing so that other parts of the app don't have to worry about it ? inside the tm-core facade or somewhere else, as like a final shutdown of resources, instead of doing it at each operation ?
Don't forget to run npm run format.
| "task-master-ai": patch | ||
| --- | ||
|
|
||
| Fix FileStorage resource leaks by ensuring close() is called in try/finally blocks |
There was a problem hiding this comment.
I don't know if close() actually does anything, but hey, why not!
2 participants
Summary
close()is called on both success and error pathsChanges Made
apps/cli/src/commands/export.command.ts (4 locations):
listAndSelectTag()- wraps FileStorage usage with try/finallyhandleTagSelection()- wraps FileStorage usage with try/finallyperformExportToTag()- wraps FileStorage usage with try/finallyperformExportAll()- wraps Promise.all with try/finallypackages/tm-core/src/modules/integration/services/export.service.ts (2 locations):
exportTasks()- wraps FileStorage usage with try/finallygenerateBriefFromTasks()- wraps FileStorage usage with try/finallypackages/tm-core/src/modules/integration/services/export.service.spec.ts (new):
close()is called after successful operationsclose()is called even whenloadTasks()throwsclose()is called even wheninitialize()throwsTest plan
Closes #1585
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.