chore: fix CI with new typescript setup#1194
Conversation
- add typecheck - add build - add tests check for other steps before running test
|
WalkthroughRestructures CI into discrete format-check, typecheck, build, and test jobs with manual trigger and global env; adds a shared build-config package and centralizes tsup configs; introduces StorageType across core and CLI (removing literal unions and guarding 'auto'); updates package scripts, exports, and mocks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Repo as Repository
participant GH as GitHub Actions
participant Format as FormatCheck
participant Type as Typecheck
participant Build as Build
participant Test as Test
Dev->>Repo: push or workflow_dispatch
Repo->>GH: trigger CI workflow
GH->>Format: checkout -> npm install -> npm run format-check
GH->>Type: checkout -> npm install -> npm run typecheck
GH->>Build: checkout -> npm install -> npm run build (NODE_ENV=production)
par Parallel jobs complete
Format-->>GH: status
Type-->>GH: status
Build-->>GH: status
end
GH->>Test: needs [format-check,typecheck,build] -> npm install -> npm run build:packages -> npm run test:coverage (upload artifacts)
Test-->>GH: artifacts & status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing Touches
🧪 Generate unit tests
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/cli/src/commands/list.command.ts (1)
6-20: Import StorageType from the dedicated types entry to align with new exports and avoid accidental value importMove StorageType to '@tm/core/types' and keep the core value exports on '@tm/core'. This also matches the repo’s emerging convention.
import { createTaskMasterCore, type Task, type TaskStatus, - type StorageType, type TaskMasterCore, TASK_STATUSES, OUTPUT_FORMATS, STATUS_ICONS, type OutputFormat } from '@tm/core'; +import type { StorageType } from '@tm/core/types';apps/cli/package.json (2)
16-24: Add the full test script set per repo guidelines.You currently have only test and test:watch. Please add coverage/unit/integration/e2e/ci scripts.
"scripts": { "build": "tsup", "dev": "tsup --watch", "typecheck": "tsc --noEmit", "lint": "biome check src", "format": "biome format --write src", - "test": "vitest run", - "test:watch": "vitest" + "test": "vitest run", + "test:watch": "vitest", + "test:coverage": "vitest run --coverage", + "test:unit": "vitest run -t unit", + "test:integration": "vitest run -t integration", + "test:e2e": "vitest run --dir tests/e2e", + "test:ci": "vitest run --coverage --reporter=dot" },
7-13: Align “types” with your stated DX preference (point to src).Per your prior preference for internal packages, consider pointing "types" at src to improve DX. Exports.types already points to src.
(Using your saved preference from #1178.)
- "types": "./dist/index.d.ts", + "types": "./src/index.ts",package.json (1)
13-38: Add missing test scripts to satisfy package.json guidelines.Root has test, watch, coverage, and e2e, but lacks unit, integration, and CI variants.
"scripts": { "build": "npm run build:packages && tsup", "dev": "npm run build:packages && npm link && (npm run dev:packages & tsup --watch --onSuccess 'echo Build complete && npm link')", "dev:packages": "(cd packages/tm-core && npm run dev) & (cd apps/cli && npm run dev) & wait", "dev:core": "cd packages/tm-core && npm run dev", "dev:cli": "cd apps/cli && npm run dev", "build:packages": "npm run build:core && npm run build:cli", "build:core": "cd packages/tm-core && npm run build", "build:cli": "cd apps/cli && npm run build", "typecheck": "npm run typecheck:core && npm run typecheck:cli", "typecheck:core": "cd packages/tm-core && npm run typecheck", "typecheck:cli": "cd apps/cli && npm run typecheck", "test": "node --experimental-vm-modules node_modules/.bin/jest", + "test:unit": "node --experimental-vm-modules node_modules/.bin/jest --testPathPattern=unit", + "test:integration": "node --experimental-vm-modules node_modules/.bin/jest --testPathPattern=integration", "test:fails": "node --experimental-vm-modules node_modules/.bin/jest --onlyFailures", "test:watch": "node --experimental-vm-modules node_modules/.bin/jest --watch", "test:coverage": "node --experimental-vm-modules node_modules/.bin/jest --coverage", + "test:ci": "node --experimental-vm-modules node_modules/.bin/jest --coverage --ci", "test:e2e": "./tests/e2e/run_e2e.sh", "test:e2e-report": "./tests/e2e/run_e2e.sh --analyze-log", "postpack": "chmod +x dist/task-master.js dist/mcp-server.js", "changeset": "changeset", "release": "changeset publish", "inspector": "npx @modelcontextprotocol/inspector node dist/mcp-server.js", "mcp-server": "node dist/mcp-server.js", "format-check": "biome format .", "format": "biome format . --write" },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.github/workflows/ci.yml(1 hunks)apps/cli/package.json(1 hunks)apps/cli/src/commands/list.command.ts(2 hunks)apps/cli/src/utils/ui.ts(1 hunks)package.json(2 hunks)packages/tm-core/package.json(2 hunks)packages/tm-core/src/services/task-service.ts(4 hunks)packages/tm-core/src/task-master-core.ts(2 hunks)packages/tm-core/tsconfig.json(2 hunks)packages/tm-core/tsup.config.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
package.json
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Files:
package.json
🧠 Learnings (15)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/package.json:13-64
Timestamp: 2025-09-03T12:16:15.866Z
Learning: For internal packages in the claude-task-master project, Crunchyman-ralph prefers pointing package.json "types" entries to src .ts files rather than dist .d.ts files for better developer experience (DX), as the packages are not being exported as SDKs.
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 Learning: 2025-07-18T17:06:57.833Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_services.mdc:0-0
Timestamp: 2025-07-18T17:06:57.833Z
Learning: Applies to scripts/modules/task-manager/*.js : Do not import or call anything from the old `ai-services.js`, `ai-client-factory.js`, or `ai-client-utils.js` files.
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-09-03T12:16:15.866Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/package.json:13-64
Timestamp: 2025-09-03T12:16:15.866Z
Learning: For internal packages in the claude-task-master project, Crunchyman-ralph prefers pointing package.json "types" entries to src .ts files rather than dist .d.ts files for better developer experience (DX), as the packages are not being exported as SDKs.
Applied to files:
apps/cli/src/utils/ui.tspackages/tm-core/package.json
📚 Learning: 2025-09-03T13:46:00.640Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/oauth-service.ts:20-20
Timestamp: 2025-09-03T13:46:00.640Z
Learning: For Node.js v22 and later, JSON imports should use `with { type: 'json' }` syntax, not `assert { type: 'json' }`. The `assert` syntax was removed in Node.js v22 in favor of the `with` syntax for import attributes. This applies to the eyaltoledano/claude-task-master codebase which uses Node.js v24.
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-07-18T17:16:32.622Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ui.mdc:0-0
Timestamp: 2025-07-18T17:16:32.622Z
Learning: Applies to scripts/modules/ui.js : Use chalk.blue for informational messages, chalk.green for success, chalk.yellow for warnings, chalk.red for errors, chalk.cyan for prompts/highlights, and chalk.magenta for subtask information
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-07-18T17:09:16.839Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-07-18T17:09:16.839Z
Learning: Display token breakdowns using a detailed, sectioned format, showing tokens per task, file, and prompt, and use formatting utilities like `boxen` and `chalk` for CLI output.
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-07-18T17:09:13.815Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-07-18T17:09:13.815Z
Learning: Display token breakdowns in CLI output using a detailed, sectioned format, including tasks, files, and prompt tokens, with clear formatting (e.g., using boxen and chalk).
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-07-18T17:16:32.622Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ui.mdc:0-0
Timestamp: 2025-07-18T17:16:32.622Z
Learning: Applies to scripts/modules/ui.js : Use getStatusWithColor for status display, formatDependenciesWithStatus for dependency lists, and truncate for overflowing text
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-07-31T22:07:49.716Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-31T22:07:49.716Z
Learning: Applies to scripts/modules/commands.js : Use boxen for important success messages with clear formatting, provide suggested next steps after command completion, and include ready-to-use commands for follow-up actions.
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-07-18T17:16:32.622Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ui.mdc:0-0
Timestamp: 2025-07-18T17:16:32.622Z
Learning: Applies to scripts/modules/ui.js : Use cli-table3 for table rendering, include colored bold headers, and set appropriate column widths for readability
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-07-31T22:07:49.716Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-31T22:07:49.716Z
Learning: Applies to scripts/modules/commands.js : Provide clear feedback about the operation being performed, display success or error messages after completion, and use colored output to distinguish between different message types.
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-07-18T17:16:32.622Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ui.mdc:0-0
Timestamp: 2025-07-18T17:16:32.622Z
Learning: Applies to scripts/modules/ui.js : Use consistent boxen box styles by content type (success, error, information) as shown in the examples
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-08-03T12:13:33.875Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to package.json : Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Applied to files:
package.json
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Provide functions for updating task status within the current tag context, handling both individual tasks and subtasks, and considering subtask status when updating parent tasks.
Applied to files:
packages/tm-core/src/services/task-service.ts
📚 Learning: 2025-08-03T12:13:33.875Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to jest.config.js : Create and maintain a production-ready Jest configuration in jest.config.js
Applied to files:
packages/tm-core/tsup.config.ts
🧬 Code graph analysis (3)
packages/tm-core/src/task-master-core.ts (1)
packages/tm-core/src/interfaces/configuration.interface.ts (1)
StorageType(82-82)
apps/cli/src/commands/list.command.ts (1)
packages/tm-core/src/interfaces/configuration.interface.ts (1)
StorageType(82-82)
packages/tm-core/src/services/task-service.ts (1)
packages/tm-core/src/interfaces/configuration.interface.ts (1)
StorageType(82-82)
🔇 Additional comments (23)
packages/tm-core/tsconfig.json (2)
10-10: LGTM - baseUrl addition supports module resolution.Adding
baseUrl: "."provides a foundation for module resolution, which is appropriate given the removal of path aliases.
31-31: Verified removal of path aliases. No@/*or@tm/coreimports remain in the code; the only matches are commented-out examples insrc/subpath-exports.test.ts, which don’t affect compilation.packages/tm-core/tsup.config.ts (6)
3-3: LGTM - Environment-based configuration improves development experience.The
isProductionconstant enables proper toggling between development and production builds, optimizing for debugging vs. performance as needed.
8-18: LGTM - Expanded entry points align with new package exports.The additional entry points (auth/index, config/index, services/index, logger/index, interfaces/index) correctly match the new subpath exports defined in package.json, enabling modular imports like
@tm/core/types.
26-29: LGTM - Environment-aware build optimizations.Conditionally enabling splitting, treeshaking, and minification based on
isProductionimproves development debugging while maintaining production performance.
33-33: LGTM - Supabase external dependency addition.Adding
@supabase/supabase-jsto externals is correct since it's now a runtime dependency. This prevents bundling it into the built package.
37-39: LGTM - Enhanced debugging configuration.The
sourcesContentand conditionalkeepNamesoptions provide better debugging support in development while allowing optimization in production.
42-44: LGTM - Development-optimized watch and bundle settings.Watch mode enabled for development and bundling dependencies improves the development experience with faster rebuilds and easier debugging.
packages/tm-core/package.json (2)
14-63: LGTM - Comprehensive subpath exports enable modular imports.The new subpath exports (./auth, ./storage, ./config, ./providers, ./services, ./errors, ./logger, ./types, ./interfaces, ./utils) provide a well-structured API surface. The pattern of pointing types to
.tsfiles follows the learned preference for better DX in internal packages.
79-79: LGTM - Supabase dependency addition supports expanded functionality.Adding
@supabase/supabase-js ^2.57.2as a runtime dependency is appropriate for the enhanced storage capabilities indicated by the broader PR changes..github/workflows/ci.yml (6)
12-12: LGTM - Manual trigger capability enhances CI flexibility.Adding
workflow_dispatchenables manual CI runs, which is valuable for testing and debugging.
17-19: LGTM - Standard environment variables for CI.Setting
DO_NOT_TRACK: 1andNODE_ENV: developmentprovides appropriate defaults for CI environments.
23-42: LGTM - Dedicated format-check job improves CI modularity.The standalone format-check job allows parallel execution and clear separation of concerns. Using
npm ci --prefer-offlineoptimizes dependency installation.
44-67: LGTM - Typecheck job ensures cross-package compatibility.The dedicated typecheck job properly builds packages first, then runs cross-package typecheck validation. This catches type errors early in the CI pipeline.
69-89: LGTM - Build job ensures compilation success.The dedicated build job verifies that all packages compile successfully before running tests.
91-129: LGTM - Test job depends on prerequisite checks.The test job properly depends on format-check, typecheck, and build jobs, ensuring all validations pass before running tests. The
needsdependency creates an efficient pipeline.apps/cli/src/utils/ui.ts (1)
9-9: LGTM - Import path updated to use new types export.The change from
@tm/coreto@tm/core/typescorrectly utilizes the new subpath export for types, improving import clarity and leveraging the modular package structure.packages/tm-core/src/task-master-core.ts (2)
12-15: LGTM - StorageType import enables consistent typing.Adding
StorageTypeto the import fromconfiguration.interface.tssupports the updated return type while maintaining proper type imports.
158-158: LGTM - Method signature updated to use StorageType.Updating
getStorageType()return type from'file' | 'api'toStorageTypeprovides better type consistency across the codebase. The implementation still delegates toTaskService.getStorageType(), maintaining behavior.packages/tm-core/src/services/task-service.ts (2)
7-7: Type-only import for StorageType looks goodConsistent with internal typing; no runtime impact.
27-28: Return runtime storage type, not config hint
In packages/tm-core/src/services/task-service.ts → getTaskList(), replacestorageType: config.typewith
storageType: this.getStorageType()so consumers never receive
'auto'.apps/cli/package.json (1)
28-28: Approve chalk pinning; verify workspace‐wide dedupeChalk is correctly pinned to 5.6.2 and matches the workspace policy. Unable to automatically locate other Chalk references—please manually confirm no other package.json pins a different Chalk version and dedupe across workspaces to avoid multiple copies.
package.json (1)
22-24: Root typecheck scripts: LGTM.The cross-package typecheck flow is clear and CI-friendly.
| filtered: number; | ||
| tag?: string; | ||
| storageType: 'file' | 'api'; | ||
| storageType: Exclude<StorageType, 'auto'>; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Contract mismatch risk: CLI narrows storageType to exclude 'auto' while core may return 'auto'
ListTasksResult uses Exclude<StorageType, 'auto'> but TaskService currently returns config.type (which can be 'auto'). This is masked by a type assertion in getTasks and could leak 'auto' to users or break future narrowing.
- Preferred: make core return the resolved runtime storage type (file|api), not 'auto' (see suggested change in TaskService.getStorageType()).
- Additionally, add a runtime guard in CLI after getTaskList to assert storageType !== 'auto' to future-proof.
Verification script to locate surfaces returning/using 'auto':
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Where core returns config.type (potentially 'auto')
rg -n -C2 "getStorageConfig\(\)\.type" packages/tm-core
# Check if any storage implementations expose a runtime type API
rg -n -C2 "getType\(" packages/tm-core/src/storage || true
# Find CLI casts that may mask type mismatches
rg -n -C2 "as ListTasksResult" apps/cliLength of output: 1395
Prevent ‘auto’ storageType from reaching the CLI
- In packages/tm-core/src/services/task-service.ts, refactor getStorageType() to resolve and return a concrete type (‘file’|’api’) instead of configManager.getStorageConfig().type, which may be ‘auto’.
- In apps/cli/src/commands/list.command.ts, add a runtime guard after fetching tasks to assert storageType !== ‘auto’ before casting to ListTasksResult.
🤖 Prompt for AI Agents
In apps/cli/src/commands/list.command.ts around line 41 and in
packages/tm-core/src/services/task-service.ts, prevent the 'auto' storageType
from reaching the CLI by (1) refactoring task-service.getStorageType() to
resolve the effective storage type and return a concrete union ('file' | 'api')
instead of returning configManager.getStorageConfig().type which can be 'auto',
and (2) adding a runtime guard in list.command.ts immediately after fetching
tasks that asserts storageType !== 'auto' (throw or handle error) before casting
to ListTasksResult so the CLI never operates on an 'auto' value. Ensure the
getStorageType implementation uses existing config and resolution logic to pick
'file' or 'api' deterministically and update types accordingly.
| "ajv-formats": "^3.0.1", | ||
| "boxen": "^8.0.1", | ||
| "chalk": "^5.4.1", | ||
| "chalk": "5.6.2", |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Chalk pin to 5.6.2 is fine; ensure workspace consistency.
This matches apps/cli. Keep an eye on any other packages still using a caret to avoid duplicate installs.
You can reuse the verification script in the other comment to confirm all Chalk versions are pinned and consistent.
🤖 Prompt for AI Agents
In package.json around line 73, chalk is pinned to "5.6.2" which matches
apps/cli but you need to ensure workspace-wide consistency; search all workspace
package.json files for "chalk" entries and replace any caret (^) or range
versions with the exact "5.6.2" to avoid duplicate installs, then run the
existing verification script (reuse the one referenced in the other comment) to
confirm every package.json in the repo uses the exact 5.6.2 version for chalk.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.github/workflows/ci.yml (1)
110-116: Confirm Jest runner flags and configure JUnit output
The CI step at .github/workflows/ci.yml (lines 110–116) invokes npm run test:coverage – at the repo root this maps to node … jest --coverage, so the --coverageThreshold, --detectOpenHandles and --forceExit flags are valid. However, no JUnit or test-results reporter is configured (no junit.xml found). Install/configure a reporter (e.g. jest-junit in jest.config.js or via --reporters), then add a CI step to upload the resulting junit.xml artifact.packages/tm-core/package.json (3)
3-7: Align “types” with DX preference and prevent accidental publish.Your exports map already points types to src; make the top-level consistent and mark package private if it isn’t published.
Option A (internal-only):
{ "name": "@tm/core", "version": "1.0.0", + "private": true, "description": "Core library for Task Master - TypeScript task management system", "type": "module", - "types": "./dist/index.d.ts", + "types": "./src/index.ts", "main": "./dist/index.js",Option B (published): keep "types": "./dist/index.d.ts" and add "src" to "files" so subpath types don’t break, or switch all export "types" to dist .d.ts.
9-13: Remove misleading “require” entries or ship CJS.Package is ESM-only ("type": "module"). Mapping "require" to ESM paths will break CommonJS consumers. Either drop "require" keys or produce CJS builds and point them accordingly.
Minimal fix (drop require):
"exports": { ".": { "types": "./src/index.ts", - "import": "./dist/index.js", - "require": "./dist/index.js" + "import": "./dist/index.js" }, "./auth": { "types": "./src/auth/index.ts", - "import": "./dist/auth/index.js", - "require": "./dist/auth/index.js" + "import": "./dist/auth/index.js" }, "./storage": { "types": "./src/storage/index.ts", - "import": "./dist/storage/index.js", - "require": "./dist/storage/index.js" + "import": "./dist/storage/index.js" }, "./config": { "types": "./src/config/index.ts", - "import": "./dist/config/index.js", - "require": "./dist/config/index.js" + "import": "./dist/config/index.js" }, "./providers": { "types": "./src/providers/index.ts", - "import": "./dist/providers/index.js", - "require": "./dist/providers/index.js" + "import": "./dist/providers/index.js" }, "./services": { "types": "./src/services/index.ts", - "import": "./dist/services/index.js", - "require": "./dist/services/index.js" + "import": "./dist/services/index.js" }, "./errors": { "types": "./src/errors/index.ts", - "import": "./dist/errors/index.js", - "require": "./dist/errors/index.js" + "import": "./dist/errors/index.js" }, "./logger": { "types": "./src/logger/index.ts", - "import": "./dist/logger/index.js", - "require": "./dist/logger/index.js" + "import": "./dist/logger/index.js" }, "./types": { "types": "./src/types/index.ts", - "import": "./dist/types/index.js", - "require": "./dist/types/index.js" + "import": "./dist/types/index.js" }, "./interfaces": { "types": "./src/interfaces/index.ts", - "import": "./dist/interfaces/index.js", - "require": "./dist/interfaces/index.js" + "import": "./dist/interfaces/index.js" }, "./utils": { "types": "./src/utils/index.ts", - "import": "./dist/utils/index.js", - "require": "./dist/utils/index.js" + "import": "./dist/utils/index.js" } },If you need CJS support, produce cjs via tsup (format: ['esm','cjs']) and wire "require" to dist/*.cjs.
Also applies to: 14-63
91-98: Enable tree-shaking hints.Apply:
"engines": { "node": ">=18.0.0" }, + "sideEffects": false, "files": ["dist", "README.md", "CHANGELOG.md"],
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)packages/tm-core/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/package.json:13-64
Timestamp: 2025-09-03T12:16:15.866Z
Learning: For internal packages in the claude-task-master project, Crunchyman-ralph prefers pointing package.json "types" entries to src .ts files rather than dist .d.ts files for better developer experience (DX), as the packages are not being exported as SDKs.
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to package.json : Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to scripts/modules/**/*.test.js : Test core logic independently with both data formats, mock file system operations, test tag resolution behavior, and verify migration compatibility in unit tests.
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 Learning: 2025-09-03T12:16:15.866Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/package.json:13-64
Timestamp: 2025-09-03T12:16:15.866Z
Learning: For internal packages in the claude-task-master project, Crunchyman-ralph prefers pointing package.json "types" entries to src .ts files rather than dist .d.ts files for better developer experience (DX), as the packages are not being exported as SDKs.
Applied to files:
packages/tm-core/package.json
🔇 Additional comments (2)
packages/tm-core/package.json (2)
78-81: Supabase as dep vs peer dep—decide and align with bundler externals.If tsup marks @supabase/supabase-js as external and consumers must provide it, make it a peerDependency (+ optional devDependency for local dev). If the core package bundles/owns it, leaving it in dependencies is fine—then don’t externalize it.
Proposed peer setup:
"dependencies": { - "@supabase/supabase-js": "^2.57.4", "zod": "^3.22.4" }, +"peerDependencies": { + "@supabase/supabase-js": "^2.57.4" +}, +"devDependencies": { + "@supabase/supabase-js": "^2.57.4", + ... +}Verify tsup externals and imports before switching.
9-63: Confirm TypeScript ≥ 4.7
TypeScript 4.7 (June 2022) added support for the"types"field in package.json export maps; ensure this package and all consumers use TS ≥ 4.7 so these mappings resolve correctly.
| permissions: | ||
| contents: read | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add concurrency to cancel superseded runs on the same ref.
Apply:
permissions:
contents: read
+
+concurrency:
+ group: ci-${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true📝 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.
| permissions: | |
| contents: read | |
| permissions: | |
| contents: read | |
| concurrency: | |
| group: ci-${{ github.workflow }}-${{ github.ref }} | |
| cancel-in-progress: true |
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 14 to 16, this workflow lacks a
concurrency setting to cancel superseded runs on the same ref; add a top-level
concurrency block (e.g., concurrency: group: ${{ github.workflow }}-${{
github.ref }} cancel-in-progress: true) so that when a new run for the same ref
starts the previous in-progress run is cancelled.
| env: | ||
| DO_NOT_TRACK: 1 | ||
| NODE_ENV: development | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t set NODE_ENV globally; set production only for Build.
A global NODE_ENV=development can skew builds if tsup/env gating relies on it.
Apply:
-env:
- DO_NOT_TRACK: 1
- NODE_ENV: development
+env:
+ DO_NOT_TRACK: 1And scope production to the build step:
- - name: Build
- run: npm run build
- env:
- FORCE_COLOR: 1
+ - name: Build
+ run: npm run build
+ env:
+ NODE_ENV: production
+ FORCE_COLOR: 1Also applies to: 86-90
🤖 Prompt for AI Agents
.github/workflows/ci.yml lines 17-20 (and similarly lines 86-90): the workflow
currently exports NODE_ENV=development globally which can interfere with
build-time checks; remove the global NODE_ENV entry and instead set
NODE_ENV=production only on the build job/step (e.g., add NODE_ENV=production
under the build job's env or the specific build step) so other jobs keep their
own environment and build tooling sees production during the build.
| - name: Install dependencies | ||
| run: npm install --frozen-lockfile --prefer-offline | ||
| timeout-minutes: 5 |
There was a problem hiding this comment.
Replace invalid npm flag with npm ci in all jobs.
npm doesn’t support --frozen-lockfile. Use npm ci for deterministic installs.
Apply:
- - name: Install dependencies
- run: npm install --frozen-lockfile --prefer-offline
+ - name: Install dependencies
+ run: npm ciRepeat the change in Typecheck, Build, and Test jobs.
Also applies to: 59-61, 82-84, 106-108
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 36-38 (and similarly update lines
59-61, 82-84, 106-108), replace the invalid npm install invocation that uses the
unsupported --frozen-lockfile flag with a deterministic install using npm ci;
specifically change the run command from "npm install --frozen-lockfile
--prefer-offline" to "npm ci --prefer-offline" in each job (Typecheck, Build,
Test, etc.), leaving the surrounding job configuration (timeouts, names)
unchanged.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
124-134: Make artifact upload resilient when files are missing.Prevents false-negative failures when a report isn’t generated.
Apply:
- name: Upload Test Results if: always() uses: actions/upload-artifact@v4 with: name: test-results path: | test-results coverage junit.xml retention-days: 30 + if-no-files-found: ignore
♻️ Duplicate comments (4)
.github/workflows/ci.yml (4)
14-16: Add concurrency to cancel superseded runs.Prevents wasting runners on outdated commits.
Apply:
permissions: contents: read +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true
17-20: Don’t set NODE_ENV globally; scope it per job/step.Keep DO_NOT_TRACK; drop global NODE_ENV. Build already sets production explicitly.
Apply:
env: DO_NOT_TRACK: 1 - NODE_ENV: development
31-35: Improve npm cache hits in a monorepo.Key caches per lockfile(s).
Apply:
- uses: actions/setup-node@v4 with: node-version: 20 cache: "npm" + cache-dependency-path: | + **/package-lock.json + **/pnpm-lock.yaml + **/yarn.lock(Repeat in all setup-node steps in this file.)
Also applies to: 54-58, 77-81, 102-106
36-38: Use deterministic installs: npm ci (drop unsupported flag).--frozen-lockfile isn’t an npm flag; npm ci is the right choice here.
Apply:
- - name: Install dependencies - run: npm install --frozen-lockfile --prefer-offline + - name: Install dependencies + run: npm ci timeout-minutes: 5(Repeat in Typecheck, Build, and Test jobs.)
Also applies to: 59-61, 82-84, 107-109
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/package.json:13-64
Timestamp: 2025-09-03T12:16:15.866Z
Learning: For internal packages in the claude-task-master project, Crunchyman-ralph prefers pointing package.json "types" entries to src .ts files rather than dist .d.ts files for better developer experience (DX), as the packages are not being exported as SDKs.
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to package.json : Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to scripts/modules/**/*.test.js : Test core logic independently with both data formats, mock file system operations, test tag resolution behavior, and verify migration compatibility in unit tests.
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to **/*.test.js : Do not rely on environment variables for API keys in tests; set mock environment variables in test setup.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-08-07T13:00:22.966Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1090
File: apps/extension/package.json:241-243
Timestamp: 2025-08-07T13:00:22.966Z
Learning: In monorepos, local packages should use "*" as the version constraint in package.json dependencies, as recommended by npm. This ensures the local version from within the same workspace is always used, rather than attempting to resolve from external registries. This applies to packages like task-master-ai within the eyaltoledano/claude-task-master monorepo.
Applied to files:
.github/workflows/ci.yml
⏰ 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 (2)
.github/workflows/ci.yml (2)
12-12: LGTM: Manual trigger added.workflow_dispatch is correctly declared.
96-96: LGTM: Tests gated on format/typecheck/build.The needs graph is correct.
| # Build job to ensure everything compiles | ||
| build: | ||
| name: Build | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| path: node_modules | ||
| key: ${{ runner.os }}-node-modules-${{ hashFiles('**/package-lock.json') }} | ||
| fetch-depth: 2 | ||
|
|
||
| - name: Format Check | ||
| run: npm run format-check | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
| cache: "npm" | ||
|
|
||
| - name: Install dependencies | ||
| run: npm install --frozen-lockfile --prefer-offline | ||
| timeout-minutes: 5 | ||
|
|
||
| - name: Build | ||
| run: npm run build | ||
| env: | ||
| NODE_ENV: production | ||
| FORCE_COLOR: 1 | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor: Consistent job-level timeouts.
Typecheck/Test have job timeouts; consider adding one to Build for parity.
If desired, set at job level:
build:
name: Build
+ timeout-minutes: 15
runs-on: ubuntu-latest📝 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.
| # Build job to ensure everything compiles | |
| build: | |
| name: Build | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v4 | |
| with: | |
| path: node_modules | |
| key: ${{ runner.os }}-node-modules-${{ hashFiles('**/package-lock.json') }} | |
| fetch-depth: 2 | |
| - name: Format Check | |
| run: npm run format-check | |
| - uses: actions/setup-node@v4 | |
| with: | |
| node-version: 20 | |
| cache: "npm" | |
| - name: Install dependencies | |
| run: npm install --frozen-lockfile --prefer-offline | |
| timeout-minutes: 5 | |
| - name: Build | |
| run: npm run build | |
| env: | |
| NODE_ENV: production | |
| FORCE_COLOR: 1 | |
| # Build job to ensure everything compiles | |
| build: | |
| name: Build | |
| timeout-minutes: 15 | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v4 | |
| with: | |
| fetch-depth: 2 | |
| - uses: actions/setup-node@v4 | |
| with: | |
| node-version: 20 | |
| cache: "npm" | |
| - name: Install dependencies | |
| run: npm install --frozen-lockfile --prefer-offline | |
| timeout-minutes: 5 | |
| - name: Build | |
| run: npm run build | |
| env: | |
| NODE_ENV: production | |
| FORCE_COLOR: 1 |
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 68 to 91, the Build job lacks a
job-level timeout like the Typecheck/Test jobs; add a timeout-minutes entry at
the job level (same indentation as runs-on) to match the other jobs (use the
same numeric value those jobs use, e.g., 10 or the project-standard timeout) so
the Build job will fail if it exceeds the configured duration.
| - name: Build packages (required for tests) | ||
| run: npm run build:packages | ||
| env: | ||
| NODE_ENV: production | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid re-building in Test; reuse Build artifacts.
Either download artifacts from Build or drop the Build job. Reusing artifacts shortens CI.
Apply (illustrative):
- - name: Build packages (required for tests)
- run: npm run build:packages
- env:
- NODE_ENV: production
+ - name: Download build artifacts
+ uses: actions/download-artifact@v4
+ with:
+ name: build-output
+ path: ./distAnd in the Build job, upload artifacts after building:
- name: Build
run: npm run build
env:
NODE_ENV: production
FORCE_COLOR: 1
+
+ - name: Upload build artifacts
+ uses: actions/upload-artifact@v4
+ with:
+ name: build-output
+ path: |
+ dist
+ packages/**/dist
+ retention-days: 3Adjust paths to match your actual build outputs.
📝 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.
| - name: Build packages (required for tests) | |
| run: npm run build:packages | |
| env: | |
| NODE_ENV: production | |
| # In your Test job (around lines 111–115), replace the rebuild step... | |
| - - name: Build packages (required for tests) | |
| - run: npm run build:packages | |
| - env: | |
| - name: Download build artifacts | |
| uses: actions/download-artifact@v4 | |
| with: | |
| name: build-output | |
| path: ./dist |
| - name: Build packages (required for tests) | |
| run: npm run build:packages | |
| env: | |
| NODE_ENV: production | |
| # And in your Build job, immediately after the existing “Build” step, add the upload... | |
| - name: Build | |
| run: npm run build | |
| env: | |
| NODE_ENV: production | |
| FORCE_COLOR: 1 | |
| - name: Upload build artifacts | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: build-output | |
| path: | | |
| dist | |
| packages/**/dist | |
| retention-days: 3 |
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 111-115, the Test job re-runs the build
(npm run build:packages) instead of reusing Build artifacts; change the workflow
so the Build job uploads the built package outputs using actions/upload-artifact
(specify the build output path(s) and a stable name), and in the Test job remove
the npm run build:packages step and instead add an actions/download-artifact
step to fetch the named artifact(s) into the workspace before running tests;
ensure artifact paths and names match between upload and download and remove
NODE_ENV=production from the now-removed step.
|
@claude please fix the requested changes in the PR reviews |
|
Claude finished @Crunchyman-ralph's task —— View job Fixed PR Review Issues ✅I've addressed the CodeRabbit review feedback systematically. Due to GitHub App permissions, CI workflow changes need to be applied manually. Todo List:
✅ Changes Successfully Applied:1. Package.json ImprovementsRoot
CLI
2. StorageType Type Safety 🔐
3. Import Path Modernization
4. Package Configuration Cleanup
|
|
Claude finished @Crunchyman-ralph's task —— View job Fixed PR Review Issues ✅I've addressed the CodeRabbit review feedback systematically. Due to GitHub App permissions, CI workflow changes need to be applied manually. Todo List:
✅ Changes Successfully Applied:1. Package.json ImprovementsRoot
CLI
2. StorageType Type Safety 🔐
3. Import Path Modernization
4. Package Configuration Cleanup
|
- Add missing test scripts (test:unit, test:integration, test:ci) to root and CLI package.json - Fix StorageType handling to prevent 'auto' from reaching CLI consumers - Update CLI to use modular @tm/core/types import for StorageType - Make @tm/core package private and remove misleading require entries - Add sideEffects: false for better tree-shaking - Point types to src files for better DX in internal packages - Add runtime guard in CLI to prevent 'auto' storage type leakage Co-authored-by: Ralph Khreish <Crunchyman-ralph@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/tm-core/src/services/task-service.ts (1)
112-118: Good: use getStorageType() instead of config to avoid leaking 'auto'This addresses prior feedback and centralizes resolution logic.
package.json (1)
76-76: Chalk pin replicated here — verify repo-wide consistencySame note as before; keep all packages on 5.6.2 to avoid duplicated installs.
#!/bin/bash rg -nP '"chalk"\s*:\s*"(?!5\.6\.2$)[^"]+"' -g '!**/node_modules/**' -g '!**/dist/**' | sed 's/^/Mismatch: /' || echo "All chalk pins are 5.6.2"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
apps/cli/package.json(2 hunks)apps/cli/src/commands/list.command.ts(3 hunks)package.json(2 hunks)packages/tm-core/package.json(3 hunks)packages/tm-core/src/services/task-service.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
package.json
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Files:
package.json
🧠 Learnings (11)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/package.json:13-64
Timestamp: 2025-09-03T12:16:15.866Z
Learning: For internal packages in the claude-task-master project, Crunchyman-ralph prefers pointing package.json "types" entries to src .ts files rather than dist .d.ts files for better developer experience (DX), as the packages are not being exported as SDKs.
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to package.json : Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 Learning: 2025-08-03T12:13:33.875Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to package.json : Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Applied to files:
package.jsonapps/cli/package.json
📚 Learning: 2025-09-03T12:16:15.866Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/package.json:13-64
Timestamp: 2025-09-03T12:16:15.866Z
Learning: For internal packages in the claude-task-master project, Crunchyman-ralph prefers pointing package.json "types" entries to src .ts files rather than dist .d.ts files for better developer experience (DX), as the packages are not being exported as SDKs.
Applied to files:
package.jsonapps/cli/package.jsonpackages/tm-core/package.json
📚 Learning: 2025-07-18T17:12:57.903Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to scripts/modules/**/*.test.js : Test CLI and MCP interfaces with real task data, verify end-to-end workflows across tag contexts, and test error scenarios and recovery in integration tests.
Applied to files:
package.json
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to scripts/modules/task-manager/**/*.js : Do not call AI-specific getters (like `getMainModelId`, `getMainMaxTokens`) from core logic functions in `scripts/modules/task-manager/*`; instead, pass the `role` to the unified AI service.
Applied to files:
apps/cli/src/commands/list.command.ts
📚 Learning: 2025-07-18T17:10:12.881Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:12.881Z
Learning: For CLI usage, install Taskmaster globally with `npm install -g task-master-ai` or use locally via `npx task-master-ai ...`.
Applied to files:
apps/cli/package.jsonpackages/tm-core/package.json
📚 Learning: 2025-07-31T20:49:04.638Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#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.
Applied to files:
apps/cli/package.jsonpackages/tm-core/package.json
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Use the global `task-master` CLI command instead of directly invoking `node scripts/dev.js` for all task management operations.
Applied to files:
apps/cli/package.json
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Provide functions for updating task status within the current tag context, handling both individual tasks and subtasks, and considering subtask status when updating parent tasks.
Applied to files:
packages/tm-core/src/services/task-service.ts
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Start new projects by running `task-master init` or `node scripts/dev.js parse-prd --input=<prd-file.txt>` to generate the initial tasks.json.
Applied to files:
packages/tm-core/package.json
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Extract tasks from PRD documents using AI, create them in the current tag context (defaulting to 'master'), provide clear prompts to guide AI task generation, and validate/clean up AI-generated tasks.
Applied to files:
packages/tm-core/package.json
🧬 Code graph analysis (2)
apps/cli/src/commands/list.command.ts (1)
packages/tm-core/src/interfaces/configuration.interface.ts (1)
StorageType(82-82)
packages/tm-core/src/services/task-service.ts (1)
packages/tm-core/src/interfaces/configuration.interface.ts (1)
StorageType(82-82)
🪛 GitHub Actions: CI
package.json
[error] 1-1: Step 'npm install --frozen-lockfile --prefer-offline' failed due to JSON.parse error in package.json: Expected double-quoted property name at position 285.
apps/cli/package.json
[error] 1-1: Step 'npm install --frozen-lockfile --prefer-offline' failed due to JSON.parse error in package.json: Expected double-quoted property name at position 285.
🔇 Additional comments (14)
apps/cli/package.json (4)
7-7: Types to src is aligned with repo preferencePoints "types" to ./src/index.ts as preferred for internal packages. LGTM.
22-27: Test scripts set matches policyIncludes test, test:watch, test:coverage, test:unit, test:integration, test:e2e, test:ci. Good.
8-12: Ignore outdated JSON syntax issue – apps/cli/package.json parses cleanly with jq, so there’s no trailing-comma error to fix.Likely an incorrect or invalid review comment.
32-32: ```shell
#!/usr/bin/env bash
set -euo pipefailecho "Scanning package.json for all instances of chalk..."
find . -type f -name package.json -not -path "/node_modules/" -not -path "/dist/" -print0 |
xargs -0 grep -H '"chalk"' || echo "No chalk entries found"</blockquote></details> <details> <summary>packages/tm-core/src/services/task-service.ts (2)</summary><blockquote> `7-7`: **Consistent type surface** Importing StorageType from interfaces centralizes the contract. LGTM. --- `165-171`: **Stats now include resolved storageType — consistent with list result** Looks correct. </blockquote></details> <details> <summary>package.json (2)</summary><blockquote> `22-24`: **Root typecheck orchestration is solid** Sequential core→cli typecheck matches CI gating intent. --- `26-28`: **Test scripts align with repository policy** Unit/integration/e2e/ci scripts present and CI-friendly flags used. LGTM. Also applies to: 31-31 </blockquote></details> <details> <summary>apps/cli/src/commands/list.command.ts (2)</summary><blockquote> `18-18`: **Type-only import from '@tm/core/types' is correct** Keeps runtime bundle clean and matches new subpath export. --- `41-42`: **Result type excludes 'auto' — good contract for consumers** Matches the CLI expectation. </blockquote></details> <details> <summary>packages/tm-core/package.json (4)</summary><blockquote> `4-4`: **Marking package as private is appropriate** Prevents accidental publish while using src types. --- `7-7`: **Types now point to src — consistent with team preference** Improves DX for internal consumers. --- `84-84`: **sideEffects: false** Good for tree-shaking. --- `12-53`: **Expanded subpath exports look correct; verify tsup entries cover them** Ensure tsup builds matching dist/* entry points so "import" paths resolve at runtime. ```shell #!/bin/bash set -euo pipefail cfg=$(fd -a -g "tsup.config.*" packages/tm-core | head -n1) echo "tsup config:" "$cfg" rg -n "entry|entries" "$cfg" -n -C2 || sed -n '1,200p' "$cfg" # Check that each exported subpath has a dist dir (after build this should exist) for p in auth storage config providers services errors logger types interfaces utils; do echo "Expect dist/$p/index.js to be built" done
| // Runtime guard to prevent 'auto' from reaching CLI consumers | ||
| if (result.storageType === 'auto') { | ||
| throw new Error('Internal error: unresolved storage type reached CLI. Please check TaskService.getStorageType() implementation.'); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Runtime guard prevents leaking 'auto'
Good defensive check. Consider including the raw config type in the error for quicker diagnosis.
- throw new Error('Internal error: unresolved storage type reached CLI. Please check TaskService.getStorageType() implementation.');
+ throw new Error('Internal error: unresolved storage type reached CLI. Please check TaskService.getStorageType() and storage implementations.');Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/cli/src/commands/list.command.ts around lines 176 to 179, the runtime
guard throws a generic error when result.storageType === 'auto'; change the
thrown error to include the raw config/storageType value for faster diagnosis
(e.g. append the offending value or config field), and ensure any sensitive data
is omitted or redacted before including it in the message; keep the descriptive
prefix "Internal error: unresolved storage type reached CLI" and concatenate the
raw type info for context.
| "@supabase/supabase-js": "^2.57.4", | ||
| "zod": "^3.22.4" | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
New supabase dependency
OK, but watch bundle size; consider dynamic import where feasible.
| storageType: StorageType; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider narrowing storageType to exclude 'auto' at the API boundary
If getStorageType never returns 'auto', you can type this as Exclude<StorageType, 'auto'> to encode the guarantee.
- storageType: StorageType;
+ storageType: Exclude<StorageType, 'auto'>;📝 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.
| storageType: StorageType; | |
| } | |
| storageType: Exclude<StorageType, 'auto'>; | |
| } |
🤖 Prompt for AI Agents
In packages/tm-core/src/services/task-service.ts around lines 27 to 28, the
storageType property is currently typed as StorageType but the API guarantees it
will never be 'auto'; change the type to Exclude<StorageType, 'auto'> to encode
that invariant at the boundary. Update the task-service interface/type
declaration to use Exclude<StorageType, 'auto'>, adjust any function signatures
or return types that expose storageType, and fix/annotate callers or tests that
assume StorageType to satisfy the narrower type.
| getStorageType(): StorageType { | ||
| // Prefer the runtime storage type if available to avoid exposing 'auto' | ||
| const s = this.storage as { getType?: () => 'file' | 'api' } | null; | ||
| const runtimeType = s?.getType?.(); | ||
| return (runtimeType ?? this.configManager.getStorageConfig().type) as StorageType; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure getStorageType never returns 'auto' after initialization
If a storage lacks getType(), this can fall back to config and leak 'auto'. Either (a) make getType mandatory on IStorage or (b) persist the resolved type at init and use it here; otherwise CLI may throw.
Apply this minimal guard:
getStorageType(): StorageType {
// Prefer the runtime storage type if available to avoid exposing 'auto'
const s = this.storage as { getType?: () => 'file' | 'api' } | null;
const runtimeType = s?.getType?.();
- return (runtimeType ?? this.configManager.getStorageConfig().type) as StorageType;
+ const cfgType = this.configManager.getStorageConfig().type as StorageType;
+ if (runtimeType) return runtimeType as StorageType;
+ if (cfgType !== 'auto') return cfgType;
+ // At this point we are initialized but can't resolve; fail fast rather than leaking 'auto'
+ throw new TaskMasterError('Unresolved storage type', ERROR_CODES.INTERNAL_ERROR);
}Option B (outside this hunk): persist the resolved type during initialize:
// Add to class:
private resolvedStorageType?: Exclude<StorageType, 'auto'>;
// In initialize(), after creating storage:
this.resolvedStorageType = this.storage.getType?.() as Exclude<StorageType,'auto'>;
// And prefer it in getStorageType():
if (this.resolvedStorageType) return this.resolvedStorageType;Check storages expose getType():
#!/bin/bash
rg -nP 'class\s+\w+Storage\b' packages/tm-core/src/storage -C2 || true
rg -nP '\bgetType\s*\(\s*\)\s*:\s*' packages/tm-core/src/storage -C2 || true🤖 Prompt for AI Agents
In packages/tm-core/src/services/task-service.ts around lines 338 to 343,
getStorageType can still return 'auto' if a storage instance lacks getType and
the code falls back to config; persist the resolved storage type at
initialization and use it here: add a private field (e.g. resolvedStorageType?:
Exclude<StorageType,'auto'>), after creating/initializing this.storage assign
the concrete type from this.storage.getType() (or resolve from config and
normalize to exclude 'auto') into that field, and then update getStorageType to
return resolvedStorageType when present otherwise fall back to the safe config
type; ensure the persisted value never retains 'auto' (use Exclude cast or
conditional mapping).
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/tm-core/src/services/task-service.ts (3)
116-122: Guard against uninitialized usage (lazy-init for safety)Public methods use
this.storagebut don't ensure initialization. Add a cheap guard to avoid NPEs if a caller forgetsinitialize().async getTaskList(options: GetTaskListOptions = {}): Promise<TaskListResult> { // Determine which tag to use const activeTag = this.configManager.getActiveTag(); const tag = options.tag || activeTag; + if (!this.initialized) { + await this.initialize(); + } + try {Optionally extract to a private ensureInitialized() and call it from public methods.
169-175: Stats should expose the resolved storage type onlyAlign
getTaskStatswith the list API to never return 'auto'.async getTaskStats(tag?: string): Promise<{ total: number; byStatus: Record<TaskStatus, number>; withSubtasks: number; blocked: number; - storageType: StorageType; + storageType: Exclude<StorageType, 'auto'>; }> {And ensure the computed
stats.storageTypeis fed from the guaranteed-resolved getter (see fix below).
224-259: Bug:getNextTaskcomputes completed deps from a filtered set (always empty)You filter to pending/in-progress, then derive completedIds from the same set—so it's empty. Resolve by querying done tasks as well.
- const result = await this.getTaskList({ - tag, - filter: { - status: ['pending', 'in-progress'] - } - }); + const [openResult, doneResult] = await Promise.all([ + this.getTaskList({ + tag, + filter: { status: ['pending', 'in-progress'] } + }), + this.getTaskList({ + tag, + filter: { status: 'done' } + }) + ]); - // Find tasks with no dependencies or all dependencies satisfied - const completedIds = new Set( - result.tasks.filter((t) => t.status === 'done').map((t) => t.id) - ); + // Find tasks with no dependencies or all dependencies satisfied + const completedIds = new Set(doneResult.tasks.map((t) => t.id)); - const availableTasks = result.tasks.filter((task) => { + const availableTasks = openResult.tasks.filter((task) => {
♻️ Duplicate comments (4)
package.json (1)
77-77: Chalk pin looks consistent with prior guidanceRoot now pins to 5.6.2; matches apps/cli.
#!/bin/bash # Verify all workspace chalk versions are exactly 5.6.2 rg -nP '"chalk"\s*:\s*"(?!5\.6\.2")' -g '**/package.json' || echo 'All chalk pins are 5.6.2'apps/cli/package.json (1)
32-32: Chalk pinned to 5.6.2 — consistent with rootNo action needed.
packages/tm-core/src/services/task-service.ts (1)
31-32: Narrow the API type to exclude 'auto' (after ensuring resolution)If callers must never see 'auto', encode it in the type.
- storageType: StorageType; + storageType: Exclude<StorageType, 'auto'>;Follow-up: only do this after getStorageType is guaranteed to never return 'auto' (see below).
packages/tm-core/package.json (1)
69-69: New supabase dependency — watch bundle sizeConsider lazy/dynamic import where feasible.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
apps/cli/package.json(2 hunks)apps/cli/tsup.config.ts(1 hunks)package.json(2 hunks)packages/build-config/package.json(1 hunks)packages/build-config/src/tsup.base.ts(1 hunks)packages/build-config/tsconfig.json(1 hunks)packages/build-config/tsup.config.ts(1 hunks)packages/tm-core/package.json(2 hunks)packages/tm-core/src/interfaces/configuration.interface.ts(1 hunks)packages/tm-core/src/services/task-service.ts(5 hunks)packages/tm-core/src/task-master-core.ts(2 hunks)packages/tm-core/src/types/index.ts(1 hunks)packages/tm-core/tsup.config.ts(1 hunks)tsup.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
package.json
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Files:
package.json
🧠 Learnings (14)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/package.json:13-64
Timestamp: 2025-09-03T12:16:15.866Z
Learning: For internal packages in the claude-task-master project, Crunchyman-ralph prefers pointing package.json "types" entries to src .ts files rather than dist .d.ts files for better developer experience (DX), as the packages are not being exported as SDKs.
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 Learning: 2025-09-03T12:16:15.866Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/package.json:13-64
Timestamp: 2025-09-03T12:16:15.866Z
Learning: For internal packages in the claude-task-master project, Crunchyman-ralph prefers pointing package.json "types" entries to src .ts files rather than dist .d.ts files for better developer experience (DX), as the packages are not being exported as SDKs.
Applied to files:
packages/build-config/tsconfig.jsonpackages/build-config/package.jsonpackages/tm-core/package.jsonapps/cli/package.json
📚 Learning: 2025-07-31T20:49:04.638Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#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.
Applied to files:
packages/build-config/package.jsonpackages/tm-core/package.jsonapps/cli/package.json
📚 Learning: 2025-08-03T12:13:33.875Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to jest.config.js : Create and maintain a production-ready Jest configuration in jest.config.js
Applied to files:
packages/build-config/tsup.config.ts
📚 Learning: 2025-07-18T17:06:57.833Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_services.mdc:0-0
Timestamp: 2025-07-18T17:06:57.833Z
Learning: Applies to scripts/modules/task-manager/*.js : Do not fetch AI-specific parameters (model ID, max tokens, temp) using `config-manager.js` getters for the AI call. Pass the `role` instead.
Applied to files:
packages/tm-core/src/interfaces/configuration.interface.ts
📚 Learning: 2025-07-18T17:10:02.683Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:02.683Z
Learning: Applies to .taskmaster/config.json : Store Taskmaster configuration settings (AI model selections, parameters, logging level, default subtasks/priority, project name, tag management) in `.taskmaster/config.json` in the project root. Do not configure these via environment variables.
Applied to files:
packages/tm-core/src/interfaces/configuration.interface.ts
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to scripts/modules/task-manager/**/*.js : Do not call AI-specific getters (like `getMainModelId`, `getMainMaxTokens`) from core logic functions in `scripts/modules/task-manager/*`; instead, pass the `role` to the unified AI service.
Applied to files:
packages/tm-core/src/interfaces/configuration.interface.ts
📚 Learning: 2025-07-18T17:10:12.881Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:12.881Z
Learning: Applies to .taskmaster/config.json : Store Taskmaster configuration settings (AI model selections, parameters, logging level, default subtasks/priority, project name, etc.) in the `.taskmaster/config.json` file located in the project root directory. Do not configure non-API key settings via environment variables.
Applied to files:
packages/tm-core/src/interfaces/configuration.interface.ts
📚 Learning: 2025-08-03T12:13:33.875Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to package.json : Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Applied to files:
package.jsonapps/cli/package.json
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Use the global `task-master` CLI command instead of directly invoking `node scripts/dev.js` for all task management operations.
Applied to files:
package.jsonapps/cli/package.json
📚 Learning: 2025-07-18T17:12:57.903Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to scripts/modules/**/*.test.js : Test CLI and MCP interfaces with real task data, verify end-to-end workflows across tag contexts, and test error scenarios and recovery in integration tests.
Applied to files:
package.json
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Provide functions for updating task status within the current tag context, handling both individual tasks and subtasks, and considering subtask status when updating parent tasks.
Applied to files:
packages/tm-core/src/services/task-service.ts
📚 Learning: 2025-09-03T13:46:00.640Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/oauth-service.ts:20-20
Timestamp: 2025-09-03T13:46:00.640Z
Learning: For Node.js v22 and later, JSON imports should use `with { type: 'json' }` syntax, not `assert { type: 'json' }`. The `assert` syntax was removed in Node.js v22 in favor of the `with` syntax for import attributes. This applies to the eyaltoledano/claude-task-master codebase which uses Node.js v24.
Applied to files:
apps/cli/package.json
📚 Learning: 2025-07-18T17:10:12.881Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:12.881Z
Learning: For CLI usage, install Taskmaster globally with `npm install -g task-master-ai` or use locally via `npx task-master-ai ...`.
Applied to files:
apps/cli/package.json
🧬 Code graph analysis (5)
apps/cli/tsup.config.ts (1)
packages/build-config/src/tsup.base.ts (2)
mergeConfig(123-142)cliConfig(39-58)
packages/tm-core/tsup.config.ts (1)
packages/build-config/src/tsup.base.ts (2)
mergeConfig(123-142)libraryConfig(13-34)
tsup.config.ts (1)
packages/build-config/src/tsup.base.ts (3)
mergeConfig(123-142)executableConfig(63-87)commonExternals(92-118)
packages/tm-core/src/task-master-core.ts (1)
packages/tm-core/src/types/index.ts (1)
StorageType(11-11)
packages/tm-core/src/services/task-service.ts (1)
packages/tm-core/src/types/index.ts (1)
StorageType(11-11)
⏰ 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 (18)
package.json (2)
19-21: Good: build order ensures @tm/build-config is built firstThis unblocks consumers that import its dist output.
27-33: Tests scripts set looks complete and CI-readyMatches the required script matrix.
packages/tm-core/src/interfaces/configuration.interface.ts (1)
6-10: Switch to shared StorageType import: LGTMRemoves duplication and aligns with the shared types surface.
apps/cli/package.json (2)
7-7: Pointing “types” to src is consistent with internal DX preferenceGood change for monorepo developer experience.
22-28: CLI test script matrix looks solidCovers watch, unit, integration, e2e, coverage, and CI.
packages/tm-core/src/task-master-core.ts (2)
13-18: Importing StorageType from shared types: LGTMKeeps the public API consistent.
160-160: getStorageType callers verified
Only the CLI (apps/cli/src/commands/list.command.ts) explicitly guards against ‘auto’; TaskMasterCore strips out storageType from its public stats and all other calls are internal to TaskService or storage‐factory logic. No remaining consumer exposes ‘auto’ unhandled.packages/build-config/tsup.config.ts (1)
5-23: LGTM: Sane, environment-aware base configConfig is minimal and correct for a small build-config package: dual formats, dts, dev-only sourcemaps, and minify/treeshake in prod. No blockers.
tsup.config.ts (3)
17-18: No changes required: current tsup versions support RegExp entries innoExternal(type(string | RegExp)[]).
15-16: publicDir is a valid tsup config option; keep using it
Use the built-in publicDir (or --publicDir) to copy your public/ folder during build (github.com); leverage onSuccess or a plugin only for advanced custom behavior (e.g. multiple source folders, custom mappings, watch-mode fixes) (github.com).Likely an incorrect or invalid review comment.
8-20: The need to inspectcliConfigremains; unable to locate its definition. Please verify thatcliConfig(imported from@tm/build-config) preserves shebangs and sets ESM module outputs correctly for your CLI builds.packages/tm-core/tsup.config.ts (3)
4-24: LGTM: Good adoption of shared library config and multi-entry setupEntries, env-aware optimizations, and externals look correct. DTS generation via base config is consistent.
20-21: Types field correctly points to src
packages/tm-core/package.jsonalready specifies"types": "./src/index.ts"per team DX preference.
22-23: Externals list is appropriate Only the two declared runtime deps (zod,@supabase/supabase-js) are externalized; all other imports are either Node built-ins or internal modules by design.packages/build-config/src/tsup.base.ts (1)
147-151: LGTM: env helperSimple and clear.
packages/tm-core/package.json (3)
7-7: Types point to src — matches team preferenceThis aligns with the DX preference recorded previously.
4-4: private: true — publishing intent confirmed
packages/tm-coreis marked private and itsfilesarray omitssrc, so TypeScript sources insrcwon’t be published. That’s intentional for a private package.
10-13: No internal CommonJS consumers found; dropping the “require” export is safe
Search across the repository (excluding dist and node_modules) returned norequire('@tm/core')orcreateRequireusages, so making @tm/core ESM-only will not break internal consumers.
| import { cliConfig, mergeConfig } from '@tm/build-config'; | ||
|
|
||
| export default defineConfig({ | ||
| entry: ['src/index.ts'], | ||
| format: ['esm'], | ||
| target: 'node18', | ||
| splitting: false, | ||
| sourcemap: true, | ||
| clean: true, | ||
| dts: true, | ||
| shims: true, | ||
| esbuildOptions(options) { | ||
| options.platform = 'node'; | ||
| } | ||
| }); | ||
| export default defineConfig( | ||
| mergeConfig(cliConfig, { | ||
| entry: ['src/index.ts'] | ||
| }) | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid build-order coupling on @tm/build-config dist
Importing from the package root resolves to dist by default; building CLI alone can fail if build-config isn’t prebuilt. Two fixes:
- Import sources in this config file (safe for TS config-time code), or
- Change @tm/build-config exports to use src for "import" (build-time-only module).
Option 1 (local change here):
-import { cliConfig, mergeConfig } from '@tm/build-config';
+import { cliConfig, mergeConfig } from '@tm/build-config/src/tsup.base.ts';To assess current risk, list all imports of @tm/build-config:
🏁 Script executed:
#!/bin/bash
fd -a 'tsup.config.ts' | xargs -I{} rg -n "from '@tm/build-config" {}Length of output: 154
🏁 Script executed:
#!/bin/bash
# Search across all TypeScript files for imports from '@tm/build-config'
rg -n --type ts --type tsx "from '@tm/build-config'" .Length of output: 27
🏁 Script executed:
#!/bin/bash
# List all occurrences of imports from '@tm/build-config' with file paths and line numbers
rg -n "from '@tm/build-config'" .Length of output: 231
🏁 Script executed:
#!/bin/bash
set -eux
# List top-level package directories
fd -t d packages --max-depth 1
# Find package.json files and print their package names
find packages -maxdepth 2 -type f -name package.json -exec grep -H '"name":' {} \;Length of output: 245
🏁 Script executed:
#!/bin/bash
# List files in the build-config src directory to verify tsup.base.ts exists
ls -R packages/build-config/srcLength of output: 39
Avoid build-order coupling on @tm/build-config dist
In apps/cli/tsup.config.ts replace the root import with the source path:
-import { cliConfig, mergeConfig } from '@tm/build-config';
+import { cliConfig, mergeConfig } from '@tm/build-config/src/tsup.base.ts';Mirror this change in packages/tm-core/tsup.config.ts and the root tsup.config.ts to ensure all CLI builds work without a prebuilt dist.
📝 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.
| import { cliConfig, mergeConfig } from '@tm/build-config'; | |
| export default defineConfig({ | |
| entry: ['src/index.ts'], | |
| format: ['esm'], | |
| target: 'node18', | |
| splitting: false, | |
| sourcemap: true, | |
| clean: true, | |
| dts: true, | |
| shims: true, | |
| esbuildOptions(options) { | |
| options.platform = 'node'; | |
| } | |
| }); | |
| export default defineConfig( | |
| mergeConfig(cliConfig, { | |
| entry: ['src/index.ts'] | |
| }) | |
| ); | |
| import { cliConfig, mergeConfig } from '@tm/build-config/src/tsup.base.ts'; | |
| export default defineConfig( | |
| mergeConfig(cliConfig, { | |
| entry: ['src/index.ts'] | |
| }) | |
| ); |
🤖 Prompt for AI Agents
In apps/cli/tsup.config.ts (lines 2–8) the file imports from the package root
'@tm/build-config' which causes build-order coupling on the package's dist;
change the import to point at the package source entry (e.g.
'@tm/build-config/src') so tsup uses the source files rather than a prebuilt
dist, and apply the exact same change to packages/tm-core/tsup.config.ts and the
repository root tsup.config.ts so all CLI builds reference the source path
instead of the compiled dist.
| "typecheck": "npm run typecheck:core && npm run typecheck:cli", | ||
| "typecheck:core": "cd packages/tm-core && npm run typecheck", | ||
| "typecheck:cli": "cd apps/cli && npm run typecheck", | ||
| "test": "node --experimental-vm-modules node_modules/.bin/jest", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Typecheck should include build-config too
Add a root aggregator for build-config to catch regressions early.
- "typecheck": "npm run typecheck:core && npm run typecheck:cli",
+ "typecheck": "npm run typecheck:build-config && npm run typecheck:core && npm run typecheck:cli",
+ "typecheck:build-config": "cd packages/build-config && npm run typecheck",📝 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.
| "typecheck": "npm run typecheck:core && npm run typecheck:cli", | |
| "typecheck:core": "cd packages/tm-core && npm run typecheck", | |
| "typecheck:cli": "cd apps/cli && npm run typecheck", | |
| "test": "node --experimental-vm-modules node_modules/.bin/jest", | |
| "typecheck": "npm run typecheck:build-config && npm run typecheck:core && npm run typecheck:cli", | |
| "typecheck:build-config": "cd packages/build-config && npm run typecheck", | |
| "typecheck:core": "cd packages/tm-core && npm run typecheck", | |
| "typecheck:cli": "cd apps/cli && npm run typecheck", | |
| "test": "node --experimental-vm-modules node_modules/.bin/jest", |
🤖 Prompt for AI Agents
In package.json around lines 23 to 26, the root "typecheck" aggregator doesn't
run the build-config package, so regressions there aren't caught; add a new
"typecheck:build-config" script that cd's into packages/build-config and runs
its typecheck, and update the root "typecheck" script to include "&& npm run
typecheck:build-config" (preserve ordering and existing scripts).
| "main": "./dist/tsup.base.js", | ||
| "types": "./dist/tsup.base.d.ts", | ||
| "exports": { | ||
| ".": { | ||
| "types": "./src/tsup.base.ts", | ||
| "import": "./dist/tsup.base.js", | ||
| "require": "./dist/tsup.base.cjs" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Point package “types” to src for better DX (matches repo preference)
Align with the monorepo convention to reference .ts in internal packages.
- "types": "./dist/tsup.base.d.ts",
+ "types": "./src/tsup.base.ts",📝 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.
| "main": "./dist/tsup.base.js", | |
| "types": "./dist/tsup.base.d.ts", | |
| "exports": { | |
| ".": { | |
| "types": "./src/tsup.base.ts", | |
| "import": "./dist/tsup.base.js", | |
| "require": "./dist/tsup.base.cjs" | |
| } | |
| "main": "./dist/tsup.base.js", | |
| "types": "./src/tsup.base.ts", | |
| "exports": { | |
| ".": { | |
| "types": "./src/tsup.base.ts", | |
| "import": "./dist/tsup.base.js", | |
| "require": "./dist/tsup.base.cjs" | |
| } |
🤖 Prompt for AI Agents
In packages/build-config/package.json around lines 6 to 13, the package "types"
field currently points to the built declaration file ("./dist/tsup.base.d.ts")
but should follow the monorepo convention of referencing the source .ts file for
better DX; update the top-level "types" value to "./src/tsup.base.ts" (and
ensure the exports.".".types entry stays consistent with "./src/tsup.base.ts")
so both the package-level types and the exports types point to the source .ts
file.
| "peerDependencies": { | ||
| "tsup": "^8.0.0" | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Align tsup peer range with what you develop against
Avoid version skew between peer and dev deps.
- "peerDependencies": {
- "tsup": "^8.0.0"
- }
+ "peerDependencies": {
+ "tsup": "^8.5.0"
+ }📝 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.
| "peerDependencies": { | |
| "tsup": "^8.0.0" | |
| } | |
| "peerDependencies": { | |
| "tsup": "^8.5.0" | |
| } |
🤖 Prompt for AI Agents
In packages/build-config/package.json lines 28-30, the peerDependency range for
"tsup" is out of sync with the devDependency used during development; update the
"tsup" entry in "peerDependencies" to match the exact version range specified in
this package's "devDependencies" (or use the same caret range used in
devDependencies) so peer and dev deps are aligned and avoid version skew.
| export const libraryConfig: Partial<Options> = { | ||
| format: ['cjs', 'esm'], | ||
| target: 'es2022', | ||
| // Sourcemaps only in development to reduce production bundle size | ||
| sourcemap: isDevelopment, | ||
| clean: true, | ||
| dts: true, | ||
| // Enable optimizations in production | ||
| splitting: isProduction, | ||
| treeshake: isProduction, | ||
| minify: isProduction, | ||
| bundle: true, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Align formats with package exports to avoid unused CJS output
Base library config builds CJS+ESM, but tm-core exports only ESM. Ensure consumers override to ESM-only.
Run to confirm tm-core config overrides format:
🏁 Script executed:
#!/bin/bash
fd -a 'tsup.config.ts' packages/tm-core | xargs -I {} rg -nC2 -e 'format\s*:\s*\[' {}Length of output: 0
🏁 Script executed:
#!/bin/bash
set -e
echo "### Listing all files in packages/tm-core ###"
find packages/tm-core -maxdepth 2 -type f -print
echo
echo "### Grepping for 'tsup' references in packages/tm-core ###"
grep -R "tsup" -n packages/tm-core || true
echo
echo "### Finding any tsup.config.* files in packages/tm-core ###"
find packages/tm-core -type f -name "tsup.config.*" -print
echo
echo "### Inspecting package.json for main/module/type/exports ###"
grep -E '"(main|module|type|exports)"' -n packages/tm-core/package.json || trueLength of output: 1392
🏁 Script executed:
#!/bin/bash
set -e
echo "### Searching for 'format' in tsup.config.ts ###"
grep -n "format" packages/tm-core/tsup.config.ts || true
echo
echo "### Displaying tsup.config.ts content ###"
sed -n '1,200p' packages/tm-core/tsup.config.tsLength of output: 867
Override tsup format to ESM-only in tm-core
Add format: ['esm'] to the object passed into mergeConfig in packages/tm-core/tsup.config.ts so only ESM bundles are emitted, matching tm-core’s ESM-only exports.
🤖 Prompt for AI Agents
In packages/build-config/src/tsup.base.ts around lines 13 to 24, libraryConfig
currently sets format: ['cjs', 'esm']; update the tm-core-specific tsup
configuration (packages/tm-core/tsup.config.ts) to override this by passing
format: ['esm'] into mergeConfig so tm-core emits only ESM bundles; modify the
mergeConfig call to include format: ['esm'] in the overrides (no other changes
to base config).
| "noEmit": true, | ||
| "resolveJsonModule": true, | ||
| "isolatedModules": true, | ||
| "declaration": true, | ||
| "skipLibCheck": true, | ||
| "forceConsistentCasingInFileNames": true | ||
| }, | ||
| "include": ["src/**/*"], | ||
| "exclude": ["node_modules", "dist"] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Resolve noEmit/declaration conflict and include tsup.config for typecheck
"noEmit": true + "declaration": true is contradictory. Let tsup handle d.ts and keep tsconfig for typechecking only. Also include the local tsup.config in typecheck.
- "noEmit": true,
+ "noEmit": true,
@@
- "declaration": true,
+ "declaration": false,
@@
- "include": ["src/**/*"],
+ "include": ["src/**/*", "tsup.config.ts"],📝 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.
| "noEmit": true, | |
| "resolveJsonModule": true, | |
| "isolatedModules": true, | |
| "declaration": true, | |
| "skipLibCheck": true, | |
| "forceConsistentCasingInFileNames": true | |
| }, | |
| "include": ["src/**/*"], | |
| "exclude": ["node_modules", "dist"] | |
| "noEmit": true, | |
| "resolveJsonModule": true, | |
| "isolatedModules": true, | |
| "declaration": false, | |
| "skipLibCheck": true, | |
| "forceConsistentCasingInFileNames": true | |
| }, | |
| "include": ["src/**/*", "tsup.config.ts"], | |
| "exclude": ["node_modules", "dist"] |
🤖 Prompt for AI Agents
In packages/build-config/tsconfig.json around lines 11 to 19, resolve the
conflicting settings by disabling TypeScript's declaration emit (set
"declaration" to false) so "noEmit": true remains for typechecking only, and add
the local tsup config file (e.g. "tsup.config.ts") to the "include" array so the
tsconfig typechecks it; leave generation of .d.ts to tsup.
| "./auth": { | ||
| "types": "./src/auth/index.ts", | ||
| "import": "./dist/auth/index.js" | ||
| }, | ||
| "./storage": { | ||
| "types": "./src/storage/index.ts", | ||
| "import": "./dist/storage/index.js" | ||
| }, | ||
| "./config": { | ||
| "types": "./src/config/index.ts", | ||
| "import": "./dist/config/index.js" | ||
| }, | ||
| "./providers": { | ||
| "types": "./src/providers/index.ts", | ||
| "import": "./dist/providers/index.js" | ||
| }, | ||
| "./services": { | ||
| "types": "./src/services/index.ts", | ||
| "import": "./dist/services/index.js" | ||
| }, | ||
| "./errors": { | ||
| "types": "./src/errors/index.ts", | ||
| "import": "./dist/errors/index.js" | ||
| }, | ||
| "./logger": { | ||
| "types": "./src/logger/index.ts", | ||
| "import": "./dist/logger/index.js" | ||
| }, | ||
| "./types": { | ||
| "types": "./src/types/index.ts", | ||
| "import": "./dist/types/index.js" | ||
| }, | ||
| "./interfaces": { | ||
| "types": "./src/interfaces/index.ts", | ||
| "import": "./dist/interfaces/index.js" | ||
| }, | ||
| "./utils": { | ||
| "types": "./src/utils/index.ts", | ||
| "import": "./dist/utils/index.js" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Validate subpath exports map to real src entries (and dist outputs)
Ensure each listed "types" path exists and tsup builds the matching dist JS.
🏁 Script executed:
#!/bin/bash
# Verify src type entrypoints exist
while read -r p; do
test -f "packages/tm-core/${p}" || echo "Missing: ${p}"
done <<'EOF'
src/auth/index.ts
src/storage/index.ts
src/config/index.ts
src/providers/index.ts
src/services/index.ts
src/errors/index.ts
src/logger/index.ts
src/types/index.ts
src/interfaces/index.ts
src/utils/index.ts
EOFLength of output: 0
🏁 Script executed:
#!/bin/bash
# Verify dist entrypoints exist
while read -r p; do
if [ ! -f "packages/tm-core/${p}" ]; then
echo "Missing: ${p}"
fi
done <<'EOF'
dist/auth/index.js
dist/storage/index.js
dist/config/index.js
dist/providers/index.js
dist/services/index.js
dist/errors/index.js
dist/logger/index.js
dist/types/index.js
dist/interfaces/index.js
dist/utils/index.js
EOFLength of output: 305
Missing dist entrypoints for subpath exports
The following built files are absent under packages/tm-core/dist, breaking your "import" mappings in package.json:
- dist/auth/index.js
- dist/storage/index.js
- dist/config/index.js
- dist/providers/index.js
- dist/services/index.js
- dist/errors/index.js
- dist/logger/index.js
- dist/types/index.js
- dist/interfaces/index.js
- dist/utils/index.js
Ensure your tsup build outputs these files (and that they’re committed) or adjust the exports map accordingly.
🤖 Prompt for AI Agents
In packages/tm-core/package.json around lines 14 to 53, the package export map
references dist/*.js entrypoints (e.g. dist/auth/index.js,
dist/storage/index.js, etc.) that are missing; update the build configuration to
emit these compiled files (adjust tsup/tsconfig to include each subpackage entry
and ensure dist/*/index.js are generated and committed) or modify the exports
map to point at actual built files or safe source entrypoints (e.g. point
"import" to existing files under dist or to ./src/* for ESM-only packages); then
run the build, verify each referenced dist/<sub>/index.js exists, and commit the
generated artifacts or update package.json accordingly.
| import type { | ||
| Task, | ||
| TaskFilter, | ||
| TaskStatus, | ||
| StorageType | ||
| } from '../types/index.js'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify storages expose a concrete getType()
If any storage lacks getType(), the guard above is essential. Quick repo scan:
Also applies to: 169-175, 342-348
🏁 Script executed:
#!/bin/bash
# Show storage classes and whether they implement getType()
rg -nP '(?s)^export\s+class\s+\w+Storage\b.*?\{.*?\}' packages/tm-core/src/storage -C2 || true
rg -nP 'getType\s*\(\s*\)\s*' packages/tm-core/src/storage -C2 || trueLength of output: 0
🏁 Script executed:
#!/usr/bin/env bash
# Locate any storage directories under tm-core
echo "== Storage directories =="
find packages/tm-core/src -type d | grep -i storage || echo "None found"
# List all TypeScript files under those directories
echo
echo "== Files under storage dirs =="
find packages/tm-core/src -type f -name '*.ts' | grep -i storage || echo "None found"
# For each file, show storage class declarations and getType() implementations
echo
echo "== Storage classes and getType methods =="
for file in $(find packages/tm-core/src -type f -name '*.ts' | grep -i storage); do
echo "---- $file ----"
rg -nP 'class\s+\w+Storage\b' "$file" -C2 || echo " (no storage class here)"
rg -nP 'getType\s*\(\)' "$file" -C2 || echo " (no getType() here)"
doneLength of output: 2734
Implement getType() in IStorage and all storage classes
None of the concrete storage classes define a getType(): StorageType method, causing any calls to storage.getType() (e.g., in task-service.ts lines 6–11, 169–175, 342–348) to fail at runtime. Add the getType() signature to the IStorage interface (packages/tm-core/src/interfaces/storage.interface.ts) and implement it in:
- SupabaseSessionStorage (packages/tm-core/src/auth/supabase-session-storage.ts)
- ApiStorage (packages/tm-core/src/storage/api-storage.ts)
- FileStorage (packages/tm-core/src/storage/file-storage/file-storage.ts)
- PlaceholderStorage (packages/tm-core/src/storage/index.ts)
Each implementation must return the correct StorageType enum value.
🤖 Prompt for AI Agents
In packages/tm-core/src/services/task-service.ts around lines 6–11 (and also
referenced at ~169–175 and ~342–348), calls to storage.getType() will fail
because IStorage lacks the getType() signature and concrete storages don't
implement it; add getType(): StorageType to
packages/tm-core/src/interfaces/storage.interface.ts and implement getType() in
the four storage classes: return StorageType.Supabase (or the correct enum
member for SupabaseSessionStorage) in
packages/tm-core/src/auth/supabase-session-storage.ts, return StorageType.Api in
packages/tm-core/src/storage/api-storage.ts, return StorageType.File in
packages/tm-core/src/storage/file-storage/file-storage.ts, and return
StorageType.Placeholder (or the corresponding enum member) in
packages/tm-core/src/storage/index.ts; ensure imports reference StorageType from
../types/index.js and that signatures and return types match exactly so
storage.getType() calls compile and run.
| getStorageType(): StorageType { | ||
| // Prefer the runtime storage type if available to avoid exposing 'auto' | ||
| const s = this.storage as { getType?: () => 'file' | 'api' } | null; | ||
| const runtimeType = s?.getType?.(); | ||
| return (runtimeType ?? | ||
| this.configManager.getStorageConfig().type) as StorageType; | ||
| } |
There was a problem hiding this comment.
Do not leak 'auto' from getStorageType; persist the resolved type
Ensure callers never see 'auto' post-init. Prefer the runtime storage type; if unavailable and config is 'auto', fail fast (or persist the resolved type at init).
getStorageType(): StorageType {
// Prefer the runtime storage type if available to avoid exposing 'auto'
const s = this.storage as { getType?: () => 'file' | 'api' } | null;
const runtimeType = s?.getType?.();
- return (runtimeType ??
- this.configManager.getStorageConfig().type) as StorageType;
+ const cfgType = this.configManager.getStorageConfig().type as StorageType;
+ if (runtimeType) return runtimeType as StorageType;
+ if (cfgType !== 'auto') return cfgType;
+ // Initialized but unresolved: do not leak 'auto'
+ throw new TaskMasterError('Unresolved storage type', ERROR_CODES.INTERNAL_ERROR);
}Outside this hunk (preferred): persist once during initialize.
// Add to class:
private resolvedStorageType?: Exclude<StorageType, 'auto'>;
// In initialize(), after creating/initializing storage:
this.resolvedStorageType = (this.storage.getType?.() ??
(this.configManager.getStorageConfig().type !== 'auto'
? (this.configManager.getStorageConfig().type as Exclude<StorageType,'auto'>)
: undefined)) as Exclude<StorageType,'auto'> | undefined;
// And in getStorageType():
if (this.resolvedStorageType) return this.resolvedStorageType;After this, update the exposed types to Exclude<StorageType, 'auto'> as suggested above.
🤖 Prompt for AI Agents
In packages/tm-core/src/services/task-service.ts around lines 342 to 348,
getStorageType can return the unresolved 'auto' value from config; callers must
never see 'auto' after initialization. Persist the resolved storage type during
initialize by adding a private resolvedStorageType?: Exclude<StorageType,'auto'>
set once after creating/initializing the storage (derive from storage.getType()
or the config if not 'auto'), update getStorageType to return
resolvedStorageType when present and otherwise compute/throw fast if config is
still 'auto'; update exposed types/usages to use Exclude<StorageType,'auto'>
where appropriate.
| /** | ||
| * Storage type options | ||
| * - 'file': Local file system storage | ||
| * - 'api': Remote API storage (Hamster integration) | ||
| * - 'auto': Automatically detect based on auth status | ||
| */ | ||
| export type StorageType = 'file' | 'api' | 'auto'; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Introduce StorageType: LGTM
Clear docs; union covers the needed modes.
Consider adding a type guard to centralize runtime checks (used by CLI):
// In the Type Guards section
export function isStorageType(value: unknown): value is StorageType {
return value === 'file' || value === 'api' || value === 'auto';
}🤖 Prompt for AI Agents
In packages/tm-core/src/types/index.ts around lines 5 to 12, add an exported
type guard function isStorageType(value: unknown): value is StorageType that
returns true when value strictly equals 'file', 'api', or 'auto'; place it in
the Type Guards section to centralize runtime validation so CLI and other
runtime code can call this guard instead of duplicating string checks.
Co-authored-by: Ralph Khreish <Crunchyman-ralph@users.noreply.github.com> Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: Ralph Khreish <Crunchyman-ralph@users.noreply.github.com> Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: Ralph Khreish <Crunchyman-ralph@users.noreply.github.com> Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
1 participant
What type of PR is this?
Description
Related Issues
How to Test This
# Example commands or stepsExpected result:
Contributor Checklist
npm run changesetnpm testnpm run format-check(ornpm run formatto fix)Changelog Entry
For Maintainers
Summary by CodeRabbit
New Features
Breaking Changes
Chores
Tests