feat: migrate context gatherer and fuzzy search to new structure [TASK-1]#1338
Conversation
This is a valuable planning document that should live in the proper docs location rather than project root. Can be removed after the update-task refactoring is complete.
Task: 1 Phase: COMMIT Tests: 45 passing, 0 failing
Task: 1.2 Phase: GREEN Tests: 58 passing, 0 failing - Created comprehensive TypeScript implementation with full type definitions - Added interfaces for Task, Subtask, SearchableTask, and all search-related types - Implemented fuzzy search with configurable search types (research, addTask, default) - Added relevance scoring with high/medium/low thresholds - Implemented category detection for pattern-based matching - 58 comprehensive tests covering all functionality 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Task: 1 Phase: COMMIT Tests: 1 passing, 0 failing
Multiple concurrent save operations were corrupting workflow-state.json with extra closing braces. Changed transition() method to be async and await all triggerAutoPersist() calls to serialize state saves. This prevents JSON corruption during rapid state transitions in the autopilot workflow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Added re-export files for old import paths to maintain backward compatibility while migrating to @tm/core. The old files now simply re-export from the new location. Changes: - Created thin re-export files for contextGatherer.js and fuzzyTaskSearch.js - Moved original implementations to .old files as backup - Added comprehensive backward compatibility tests (9 tests, all passing) - Tests verify that both old and new import paths work identically This ensures a smooth transition period where existing code continues to work while new code can use the @tm/core imports. Task: 1 Subtask: 8 Tests: 9 passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 0c69048 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR introduces a comprehensive refactor of task-update architecture with new Strategy pattern-based services, adds major utility classes for context gathering and fuzzy task search to tm-core, enhances workflow orchestration with async state management, reorganizes imports across the codebase, and includes migration planning and backward-compatibility testing. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant WorkflowService
participant WorkflowOrchestrator
participant GitAdapter
participant StateManager
User->>WorkflowService: startWorkflow()
WorkflowService->>GitAdapter: ensureRepoExists()
GitAdapter-->>WorkflowService: ✓
WorkflowService->>GitAdapter: getCurrentBranch()
GitAdapter-->>WorkflowService: current branch
WorkflowService->>GitAdapter: createAndCheckoutBranch()
GitAdapter-->>WorkflowService: ✓
WorkflowService->>WorkflowOrchestrator: transition(PREFLIGHT_COMPLETE)
rect rgba(100, 200, 100, 0.2)
note right of WorkflowOrchestrator: Now async
WorkflowOrchestrator->>WorkflowOrchestrator: await transition
WorkflowOrchestrator->>StateManager: await triggerAutoPersist()
StateManager-->>WorkflowOrchestrator: ✓
end
WorkflowOrchestrator-->>WorkflowService: Promise resolved
WorkflowService->>WorkflowService: await next transition
sequenceDiagram
participant Caller
participant ContextGatherer
participant TaskDB
participant FileSystem
participant TokenCounter
participant FuzzySearch
Caller->>ContextGatherer: gather(options)
ContextGatherer->>TaskDB: loadTasks()
TaskDB-->>ContextGatherer: tasks[]
ContextGatherer->>FileSystem: readFiles()
FileSystem-->>ContextGatherer: content
rect rgba(150, 150, 200, 0.2)
note right of ContextGatherer: Optional semantic search
alt includeSemanticSearch
ContextGatherer->>FuzzySearch: findRelevant(prompt)
FuzzySearch-->>ContextGatherer: relevance breakdown
end
end
alt includeTokenCounts
ContextGatherer->>TokenCounter: countTokens()
TokenCounter-->>ContextGatherer: TokenBreakdown
end
ContextGatherer-->>Caller: ContextResult {context, analysisData, tokenBreakdown}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Rationale: The diff spans two major new utility implementations (ContextGatherer and FuzzyTaskSearch) with substantial domain logic, multiple public API additions, async/await control-flow changes in workflow orchestration, extensive architectural design documentation, and ~50+ files with import reordering. While many changes are repetitive (import reorganization), the new utilities introduce significant complexity with numerous interfaces, context-gathering workflows, fuzzy-search logic, and token accounting. The workflow async changes require careful verification of state persistence ordering and promise handling. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/extension/docs/extension-development-guide.md (1)
7-14: Add language to fenced code block (MD040).Specify a language (e.g., text) for the file tree block to satisfy markdownlint.
-``` +```text apps/extension/ ├── package.json # Development configuration ├── package.publish.json # Clean publishing configuration ├── package.mjs # Build script for packaging ├── .vscodeignore # Files to exclude from extension package └── vsix-build/ # Generated clean package directory</blockquote></details> <details> <summary>apps/docs/getting-started/quick-start/prd-quick.mdx (1)</summary><blockquote> `35-71`: **Update documentation link to reference production docs URL.** The link to the RPG method at line 70 uses a relative path `/capabilities/rpg-method`, but the coding guidelines specify that documentation should reference `https://docs.task-master.dev` rather than local file paths. As per coding guidelines for `**/*.{md,mdx}` files. ```diff <Tip> -For complex projects, using the RPG template with a code-context-aware ai agent produces the best results because the AI can understand your existing codebase structure. [Learn more about the RPG method →](/capabilities/rpg-method) +For complex projects, using the RPG template with a code-context-aware ai agent produces the best results because the AI can understand your existing codebase structure. [Learn more about the RPG method →](https://docs.task-master.dev/capabilities/rpg-method) </Tip>
🧹 Nitpick comments (95)
apps/docs/archive/cursor-setup.mdx (1)
1-5: Documentation should reference external docs per coding guidelines.This file is in the
archive/folder, which suggests it may be legacy or deprecated documentation. Per the coding guidelines, documentation matching**/*.{md,mdx}should referencehttps://docs.task-master.devrather than local file paths in content. Clarify whether this file should remain in the archive or be moved to the active documentation, and consider whether it should primarily link to the canonical documentation site instead of providing standalone instructions.UPDATE_TASKS_OOP_ARCHITECTURE.md (3)
17-17: Specify language for fenced code block.The ASCII diagram code block should declare a language identifier for consistency and tooling support.
-``` +``` ┌─────────────────────────────────────────────────────┐
664-664: Specify language for fenced code block.The file structure code block should declare a language identifier (e.g.,
textorplaintext).-``` +```text packages/tm-core/src/modules/tasks/
711-711: Fix hyphenation in compound modifier."tm-core" should be hyphenated when used as a compound modifier before a noun.
- - [ ] Move `scripts/modules/utils/contextGatherer.js` → `packages/tm-core/src/modules/tasks/utils/context-gatherer.ts` + - [ ] Move `scripts/modules/utils/contextGatherer.js` → `packages/tm-core/src/modules/tasks/utils/context-gatherer.ts`Note: The code paths themselves are correct; the issue flagged is about the phrasing context in the checklist. The existing line appears correct; verify if the linter is flagging a false positive based on surrounding context.
.taskmaster/docs/architecture/update-task-migration-plan.md (6)
1-50: Excellent comprehensive migration plan, but verify alignment with current PR scope.This document outlines a detailed 11-phase migration strategy with strong OOP design patterns (strategy, factory, DI). However, the PR objectives indicate the current PR focuses on Phase 0/1 activities (backward compatibility re-exports for
contextGatherer.jsandfuzzyTaskSearch.js), while this plan documents the full end-to-end migration.Recommendation: Clarify whether this is a pre-implementation planning artifact for future work or supplementary documentation for the current PR. If the latter, consider adding a "Phase Roadmap" section noting which phases are completed, in-progress, or planned for future PRs.
563-613: ContextBuilderService gracefully handles gathering failures, but verify fallback behavior.The service at lines 569-612 properly:
- Wraps context gathering in try-catch (line 608)
- Logs warnings on failure and continues with empty context (line 609-610)
- Returns partial results with target task IDs preserved
However, clarify:
- Does logging a warning mean the operation will still succeed? Verify this is the intended behavior rather than failing fast for data integrity.
- Will empty context (
context: '') cause issues in the prompt templates? Should the template builders detect and handle this case?- The comment at line 616 "Uses (existing):" appears truncated—verify the list is complete.
841-853: Mode detection logic is clear but missing edge case documentation.The factory's
detectMode()method (lines 841-852) uses a sensible priority:
- Check
--fromfirst (bulk mode)- Then check
.in ID (subtask mode)- Otherwise single task mode
However:
- What if both
--fromand--idare provided? The code prioritizes--from, but the intent should be documented or error checking should prevent this invalid combination.- What if
--idis 0? The checkoptions.id !== undefinedat line 848 will pass, but is 0 a valid task ID? Verify against the ID validation logic.- Consider adding explicit validation before mode detection to fail fast on invalid combinations.
1061-1240: Implementation phase ordering is well-sequenced with good dependency management.The 11 phases follow a logical dependency chain:
- Phases 1-2: Foundation (types, interfaces, validators)
- Phase 3: Services (context, prompt, data merging)
- Phase 4: Strategies (the main business logic)
- Phase 5-6: Display and factories
- Phase 7: Orchestrator
- Phase 8: CLI integration
- Phases 9-10: Testing and documentation
- Phase 11: Cleanup
This ordering ensures each phase has all its dependencies available from prior phases. However:
Phase 3 risk: If existing classes (ContextGatherer, FuzzyTaskSearch, PromptManager) have breaking changes during the re-export migration (Phase 0/current PR), Phases 3+ could be blocked. Consider adding a verification step at the start of Phase 3.
Phase 8 timing: CLI integration could theoretically begin once Phase 7 is complete, but consider whether earlier integration (Phase 6-7) would catch issues sooner through manual testing.
Phase 9 coverage: Integration tests should include testing the mode detection logic and error paths that may only surface when all components are wired together.
1333-1405: Testing strategy is identified but implementation details are sparse.The document lists test categories (unit, integration, edge cases) but lacks specifics:
Recommend adding:
- Mock/stub strategy: How will external dependencies (AI services, file system) be mocked?
- Test data: What fixture task structures should be used? Include examples of edge cases (tasks with/without subtasks, done vs. pending status).
- Coverage targets: Specify line/branch coverage minimums expected.
- Error path testing: Document how errors from each service (AI service, storage, context gatherer) should be tested.
- CLI-specific testing: How will CLI display and loading indicators be tested in isolation?
Example: A test for "Bulk update with completed subtasks" should verify:
- Pre-update display shows the info box about completed subtasks
- AI receives only non-done tasks (completed subtasks preserved)
- Post-update display correctly shows update count
1362-1377: Backward compatibility strategy is sound but incomplete.The document proposes:
- Keep old commands working (good)
- Add deprecation warnings (good)
- Update documentation (good)
- Alias support (optional, mentioned)
However, clarify:
Deprecation warnings placement: Should these be in the legacy command implementations (
update-tasks.js,update-subtask-by-id.js) or in a wrapper layer? Document the exact message users will see.Migration timeline: The document mentions "next major version" for removal but doesn't specify the deprecation period. Recommend adding a version number and deprecation date (e.g., "deprecated in v2.0, will be removed in v3.0, June 2026").
Alias implementation: If aliases are added (line 1376), clarify whether old command names will call the new service or remain separate implementations. Using the new service is preferable to reduce maintenance burden.
Breaking changes: Document what, if any, behavioral changes exist. For example, does the unified command always use the same prompt template, or are there subtle differences users should be aware of?
apps/extension/src/components/TaskDetails/TaskMetadataSidebar.tsx (1)
102-133: Improve type safety of sendMessage return type.The refactored
handleStartTaskfunction properly implements async/await with solid error handling and loading state management. However, there is a type safety gap worth addressing.The
sendMessagefunction in VSCodeContext is typed as(message: any) => Promise<any>, which means the response object accessed at lines 120-125 (result.success,result.error,result.terminalName) has no TypeScript validation. While the backend correctly returns aTerminalExecutionResultinterface with these properties, the frontend receives it asany, losing compile-time type safety.Recommendation: Improve the type signature of
sendMessageto return properly typed responses rather thanPromise<any>. This prevents runtime surprises if the API contract changes and enables IDE autocompletion for response properties..github/scripts/parse-metrics.mjs (1)
5-19: Consider adding error handling for file operations.The function correctly parses markdown tables, but
readFileSynccalls in themainfunction (lines 47, 72, 91, 109) can throw exceptions if files are malformed or permissions are denied. WhileexistsSyncchecks prevent missing file errors, other I/O errors remain unhandled.Consider wrapping file reads in try/catch blocks:
if (existsSync('issue_metrics.md')) { console.log('📄 Found issue_metrics.md, parsing...'); - const issueContent = readFileSync('issue_metrics.md', 'utf8'); + try { + const issueContent = readFileSync('issue_metrics.md', 'utf8'); - - metrics.issues_created = parseCountMetric( - issueContent, - 'Total number of items created' - ); + metrics.issues_created = parseCountMetric( + issueContent, + 'Total number of items created' + ); + // ... other parsing ... + } catch (error) { + console.error(`Failed to read issue_metrics.md: ${error.message}`); + }Apply similar wrapping to lines 72, 91, and 109.
.env.example (1)
17-17: Address dotenv-linter warnings for consistency.Static analysis has flagged two style issues:
- EndingBlankLine: Add a blank line at the end of the file for Unix compatibility.
- UnorderedKey: The
GOOGLE_APPLICATION_CREDENTIALSkey should be ordered beforeVERTEX_LOCATIONfor logical grouping of related configuration.While these are minor style issues, addressing them improves file consistency and follows tooling recommendations.
Apply this diff to resolve the warnings:
OLLAMA_API_KEY=YOUR_OLLAMA_API_KEY_HERE # Google Vertex AI Configuration VERTEX_PROJECT_ID=your-gcp-project-id + GOOGLE_APPLICATION_CREDENTIALS=/path/to/service-account-credentials.json VERTEX_LOCATION=us-central1 # Optional: Path to service account credentials JSON file (alternative to API key) - GOOGLE_APPLICATION_CREDENTIALS=/path/to/service-account-credentials.json +This reorders the keys for logical grouping (Google Vertex config together) and adds the trailing newline.
apps/docs/getting-started/api-keys.mdx (1)
263-267: Use absolute docs links per guidelines.Replace relative site path with full docs URL to avoid broken links outside the site context.
As per coding guidelines
- - Check the [FAQ](/getting-started/faq) for common solutions + - Check the [FAQ](https://docs.task-master.dev/getting-started/faq) for common solutions.github/workflows/extension-ci.yml (2)
34-47: Leverage setup-node built‑in npm cache.You can simplify and speed up installs by letting setup-node manage cache instead of manual cache steps.
Example:
- uses: actions/setup-node@v4 with: node-version: 20 cache: npm cache-dependency-path: '**/package-lock.json'
68-71: Install with workspaces for monorepo consistency (optional).If using npm workspaces, prefer
npm ci --workspaces --include-workspace-rootto ensure all package installs are consistent across jobs.Please confirm root package.json defines workspaces; if not, ignore this.
apps/extension/docs/extension-development-guide.md (1)
165-175: Avoid referencing repo-internal paths; link to docs page instead (optional).Replace raw file paths (
.github/workflows/version.yml,scripts/release.sh) with a docs.task-master.dev page that explains releases to keep guides portable.As per coding guidelines
.taskmaster/templates/example_prd_rpg.txt (1)
167-176: Add code fence language for diagrams to satisfy linters.Use "text" for non-code blocks to avoid MD040 warnings and improve rendering.
-``` +```text project-root/ ├── src/ │ ├── [module-name]/ # Maps to: [Capability Name] │ │ ├── [file].js # Maps to: [Feature Name] │ │ └── index.js # Public exports │ └── [module-name]/ ├── tests/ └── docs/</blockquote></details> <details> <summary>.taskmaster/docs/tdd-workflow-phase-2-pr-resumability.md (2)</summary><blockquote> `43-49`: **Label the PR title code block as text.** Add a language to the fenced block to satisfy MD040. ```diff -``` +```text Task #<id> [<tag>]: <title>--- `94-101`: **Capitalize “Markdown” when referring to the format.** Use the proper noun in token replacement descriptions. ```diff - - `{subtasksList}` → markdown list of subtask titles - - `{commitsList}` → markdown list of commit SHAs and messages + - `{subtasksList}` → Markdown list of subtask titles + - `{commitsList}` → Markdown list of commit SHAs and messages.taskmaster/docs/prd-tm-start.txt (2)
41-43: Avoid “CLI interface” tautology.Prefer “CLI” for brevity.
- - This opens Claude CLI interface naturally in the current terminal + - This opens Claude CLI naturally in the current terminal
38-41: Reduce repetition (“tells”).Tighten phrasing for clarity.
- - The prompt tells Claude to first run `tm show <task_id>` to get task details - - Then tells Claude to implement the code changes + - The prompt instructs Claude to run `tm show <task_id>` to get task details, + then implement the code changes.taskmaster/docs/tdd-workflow-phase-3-extensibility-guardrails.md (1)
199-215: Add a language to the TUI layout fence.Mark ASCII diagrams as “text” to satisfy MD040.
-``` +```text ┌──────────────────────────────────┬─────────────────────────────────┐ │ Task Navigator (left) │ Executor Terminal (right) │ ... └──────────────────────────────────┴─────────────────────────────────┘</blockquote></details> <details> <summary>.taskmaster/docs/tdd-workflow-phase-1-orchestrator.md (2)</summary><blockquote> `11-28`: **Label the execution model diagram as text.** Prevents MD040 and renders better in some viewers. ```diff -``` +```text ┌─────────────────────────────────────────────────────────────┐ │ Claude Code (MCP Client) │ ... └─────────────────────────────────────────────────────────────┘--- `154-166`: **Label phase transition diagram as text.** Same rationale as above. ```diff -``` +```text START → RED(subtask 1) → GREEN(subtask 1) → COMMIT(subtask 1) ... FINALIZE → END</blockquote></details> <details> <summary>.taskmaster/docs/autonomous-tdd-git-workflow.md (2)</summary><blockquote> `297-313`: **Add language to TUI layout fence.** Use “text” for ASCII diagram. ```diff -``` +```text ┌─────────────────────────────────────┬──────────────────────────────────┐ │ Task Navigator (left) │ Executor Terminal (right) │ ... └─────────────────────────────────────┴──────────────────────────────────┘--- `907-912`: **Fix list indentation to satisfy markdownlint (MD005/MD007).** Align the final list items to the same indentation level. ```diff - - Surgical Test Generator: .claude/agents/surgical-test-generator.md + - Surgical Test Generator: .claude/agents/surgical-test-generator.mdIf multiple items under “References (Repo)”, ensure consistent leading spaces across all bullets.
apps/cli/src/commands/set-status.command.ts (3)
47-48: storageType typing mismatch (“auto” may leak through).
getStorageConfig().typecan be "auto"; the type forces exclude<..., 'auto'>. Either resolve the effective type or allow "auto" in the result.-export interface SetStatusResult { +export interface SetStatusResult { success: boolean; updatedTasks: Array<{ taskId: string; oldStatus: TaskStatus; newStatus: TaskStatus; }>; - storageType: Exclude<StorageType, 'auto'>; + storageType: StorageType; // or expose a resolvedStorageType field if available }If tm-core exposes a “resolved/effective” storage type, use that instead.
Also applies to: 167-171
185-188: Cleanup hook placeholder does nothing.Call a disposer if available to free resources; otherwise remove the empty block.
- if (this.tmCore) { - } + await this.tmCore?.dispose?.();
199-217: JSON output should be a single object for machine consumers.Currently per-task errors may print alongside the final JSON. With the batch refactor, emit only one JSON payload containing
success,updatedTasks, andfailures.- case 'json': - console.log(JSON.stringify(result, null, 2)); + case 'json': + console.log(JSON.stringify(result, null, 2)); break;Ensure no other JSON/log lines are printed before this branch in error paths.
.taskmaster/docs/tdd-workflow-phase-1-core-rails.md (3)
255-255: Add language specifier to fenced code block.The fenced code block at line 255 lacks a language identifier, reducing syntax highlighting and readability.
Apply this diff:
-``` +```text AI Agent TaskMaster
887-887: Add language specifier to fenced code block.The fenced code block at line 887 lacks a language identifier.
Apply this diff:
-``` +```text AI Agent TaskMaster
601-605: Clarify results format specification.The results format is documented as
<passed:n,failed:n>but examples throughout the document usefailed:3,passed:0(failed before passed). The order inconsistency could confuse implementers.Consider either:
- Standardizing to
passed:n,failed:norder in all examples, or- Updating the format string to match example order:
failed:n,passed:napps/extension/src/services/terminal-manager.ts (1)
49-56: Consider storing initializeCore result to avoid non-null assertion.The non-null assertion
this.tmCore!at line 52 is safe due to the await on line 49, but TypeScript can't verify this. Consider assigning the result to a local variable for better type safety.// Initialize tm-core if needed await this.initializeCore(); +const tmCore = this.tmCore; +if (!tmCore) { + throw new Error('Failed to initialize tm-core'); +} // Use tm-core to start the task (same as CLI) -const startResult = await this.tmCore!.tasks.start(taskId, { +const startResult = await tmCore.tasks.start(taskId, { dryRun: false, force: false, updateStatus: true });CLAUDE_CODE_PLUGIN.md (1)
37-49: Add language specifier to fenced code block.The directory tree structure is helpful, but the fenced code block at line 37 should specify a language (e.g.,
textor leave blank).Based on static analysis hints.
Apply this diff:
-``` +```text claude-task-master/ ├── .claude-plugin/ │ └── marketplace.json # Marketplace manifest (at repo root)apps/mcp/src/shared/types.ts (2)
5-22: Consider optional nested fields for flexibility.The
error,version, andtagfields are optional objects with required nested properties. While this pattern ensures completeness when these objects exist, it may be overly restrictive. For example, if an error occurs without a specificcode, you'd be forced to provide a placeholder value.Consider making nested fields optional as well:
export interface MCPResponse<T = any> { success: boolean; data?: T; error?: { - code: string; - message: string; + code?: string; + message?: string; suggestion?: string; details?: any; }; version?: { - version: string; - name: string; + version?: string; + name?: string; }; tag?: { - currentTag: string; - availableTags: string[]; + currentTag?: string; + availableTags?: string[]; }; }This provides more flexibility while maintaining type safety.
24-32: Consider stricter typing forsessionfield.The
sessionfield is typed asany, which bypasses TypeScript's type checking. Consider usingunknownfor better type safety, or define a specific session interface.export interface MCPContext { log: { info: (message: string) => void; warn: (message: string) => void; error: (message: string) => void; debug: (message: string) => void; }; - session: any; + session: unknown; }This forces consumers to narrow the type before use, improving safety.
apps/cli/src/commands/autopilot/resume.command.ts (1)
30-39: Improve type safety forprojectRoot.The
projectRootassignment ensures a value is always present (withprocess.cwd()fallback), but TypeScript still treats it as potentially undefined, requiring non-null assertions (!) throughout the code.Consider explicitly typing and validating:
private async execute(options: ResumeOptions): Promise<void> { // Inherit parent options (autopilot command) const parentOpts = this.parent?.opts() as AutopilotBaseOptions; const mergedOptions: ResumeOptions = { ...parentOpts, ...options, projectRoot: options.projectRoot || parentOpts?.projectRoot || process.cwd() }; + + // Explicitly validate projectRoot is set + const projectRoot: string = mergedOptions.projectRoot!; const formatter = new OutputFormatter(mergedOptions.json || false); try { // Check for workflow state - const hasState = await hasWorkflowState(mergedOptions.projectRoot!); + const hasState = await hasWorkflowState(projectRoot);This eliminates repeated non-null assertions and improves readability.
.taskmaster/tasks/task_007_tm-start.txt (1)
15-46: Consider adding comprehensive error handling.The proposed implementation lacks try-catch blocks around async operations and doesn't handle potential errors from
buildPrompt(),executeClaude(),checkGitChanges(), ormarkTaskAsDone(). While the task exists validation is present, other failure modes could cause unhandled promise rejections.Consider wrapping the execution logic in a try-catch block:
public async execute(taskId: string): Promise<void> { + try { // Validate task exists const core = await createTaskMasterCore(); const task = await core.tasks.getById(parseInt(taskId, 10)); if (!task) { console.error(`Task with ID ${taskId} not found`); process.exit(1); } // Build prompt const prompt = this.buildPrompt(taskId); // Execute claude-code await this.executeClaude(prompt); // Check git status const changedFiles = await this.checkGitChanges(); if (changedFiles.length > 0) { console.log('\nChanges detected in the following files:'); changedFiles.forEach(file => console.log(`- ${file}`)); // Auto-mark task as done await this.markTaskAsDone(taskId); console.log(`\nTask ${taskId} completed successfully and marked as done.`); } else { console.warn('\nNo changes detected after claude-code execution. Task not marked as done.'); } + } catch (error) { + console.error(`Failed to execute task ${taskId}:`, (error as Error).message); + if (process.env.DEBUG) { + console.error((error as Error).stack); + } + process.exit(1); + } }apps/cli/CHANGELOG.md (1)
3-31: Changelog formatting could be improved.The static analysis tool flags some formatting issues in the changelog entries (null should be a heading, missing language specifier for code blocks). However, since this appears to be auto-generated changeset content, these are minor concerns.
If the changeset configuration supports it, consider:
- Using proper heading markers for version sections (e.g.,
## nullinstead of relying on emphasis)- Adding language identifiers to fenced code blocks for better rendering
apps/cli/tests/unit/commands/autopilot/shared.test.ts (4)
29-31: Remove unused test variables.projectRoot and statePath are declared but never used. Drop them to reduce noise.
- const projectRoot = '/test/project'; - const statePath = `${projectRoot}/.taskmaster/workflow-state.json`;
68-97: Add assertion for non-done statuses mapping to 'pending'.parseSubtasks maps any non-'done' status to 'pending'. Add an assertion for the 'in-progress' case to lock this behavior.
expect(result[1]).toEqual({ id: '2', title: 'Subtask 2', status: 'completed', attempts: 0, maxAttempts: 5 }); + expect(result[2]).toEqual({ + id: '3', + title: 'Subtask 3', + status: 'pending', + attempts: 0, + maxAttempts: 5 + });
120-128: Also stub/restore console.warn in lifecycle.warning tests locally stub warn; prefer centralized beforeEach/afterEach like log/error for symmetry.
beforeEach(() => { consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'warn').mockImplementation(() => {}); }); afterEach(() => { consoleLogSpy.mockRestore(); consoleErrorSpy.mockRestore(); + (console.warn as any).mockRestore?.(); });
130-160: Add JSON-mode warning test.Lock JSON warning shape from OutputFormatter.warning to prevent regressions.
describe('JSON mode', () => { + it('should output JSON for warning', () => { + const formatter = new OutputFormatter(true); + formatter.warning('Heads up'); + const warnCall = (console.warn as any).mock?.calls?.[0]?.[0]; + expect(warnCall).toBeTruthy(); + const output = JSON.parse(warnCall); + expect(output.warning).toBe('Heads up'); + });apps/extension/src/services/webview-manager.ts (5)
227-251: Coerce IDs to strings when matching tasks from MCP.MCP may return numeric IDs; strict equality vs string taskId can fail.
- complexityResult.report.complexityAnalysis.tasks.find( - (t: any) => t.id === data.taskId - ); + complexityResult.report.complexityAnalysis.tasks.find( + (t: any) => String(t.id) === String(data.taskId) + );Confirm the MCP complexity_report schema for task id type (string vs number) to avoid silent misses.
263-293: Support both top-level and data-scoped tool/params in mcpRequest.For backward compatibility, accept message.tool/params and data.tool/params.
- // The tool and params come directly in the message - const tool = message.tool; - const params = message.params || {}; + // Accept tool/params at top-level or under data for compatibility + const tool = message.tool ?? data?.tool; + const params = message.params ?? data?.params ?? {};
359-365: Acknowledge openExternal when requestId is present.Avoid leaving a pending request on the webview side.
case 'openExternal': // Open external URL if (message.url) { vscode.env.openExternal(vscode.Uri.parse(message.url)); } - return; + if (requestId) { + panel.webview.postMessage({ + type: 'response', + requestId, + success: true + }); + } + return;
373-405: DRY up tag parsing used here and in getTags.Factor JSON parsing of list_tags into a helper to reduce duplication and subtle drift.
Add a private helper and reuse it in openTerminal and getTags.
// outside switch, inside class private async resolveCurrentTag(): Promise<string> { let tag = 'master'; if (!this.mcpClient) return tag; try { const res = await this.mcpClient.callTool('list_tags', { projectRoot: vscode.workspace.workspaceFolders?.[0]?.uri.fsPath, showMetadata: false }); const text = res?.content?.[0]?.text; const parsed = text ? JSON.parse(text) : (res?.data ?? {}); const maybe = parsed?.data ?? parsed; if (typeof maybe?.currentTag === 'string') tag = maybe.currentTag; } catch (e) { this.logger.warn('resolveCurrentTag failed', e); } return tag; }Then replace the inline logic with:
const currentTag = await this.resolveCurrentTag();
415-427: Prefer setTimeout(0) over setImmediate for portability.VS Code’s Node runtime supports both, but setTimeout is more universally available.
- setImmediate(() => { + setTimeout(() => { if (result.success) { // Success: Show info message vscode.window.showInformationMessage( `✅ Started Claude session for Task ${taskId}: ${taskTitle}` ); } else { // Error: Show VS Code native error notification only const errorMsg = `Failed to start task: ${result.error}`; vscode.window.showErrorMessage(errorMsg); } - }); + }, 0);apps/cli/src/ui/components/header.component.ts (2)
41-60: Harden brief URL construction.Avoid accidental double slashes; use URL for safer composition.
- const briefUrl = `${briefInfo.webAppUrl}/home/${briefInfo.orgSlug}/briefs/${briefInfo.briefId}/plan`; - console.log(`Listing tasks from: ${chalk.dim(briefUrl)}`); + const base = new URL(briefInfo.webAppUrl); + base.pathname = `/home/${briefInfo.orgSlug}/briefs/${briefInfo.briefId}/plan`; + console.log(`Listing tasks from: ${chalk.dim(base.toString())}`);
21-27: title is unused.Either render it or drop from HeaderOptions to avoid dead API.
Would you like me to wire title into the header output?
apps/cli/src/ui/components/next-task.component.ts (2)
21-22: Allow string complexity too (helper already supports it).Broaden type to accept numeric strings; avoids silent omission.
- complexity?: number; + complexity?: number | string;
87-91: Render complexity when provided, regardless of type.Use null-check instead of typeof number.
- if (typeof task.complexity === 'number') { - content.push(`Complexity: ${getComplexityWithColor(task.complexity)}`); - } + if (task.complexity != null) { + content.push(`Complexity: ${getComplexityWithColor(task.complexity)}`); + }apps/cli/src/commands/autopilot/complete.command.ts (2)
37-57: Prefer return codes or Commander’s exitOverride for testability.Direct process.exit() makes unit testing harder. Consider throwing a CommanderError or using program.exitOverride and returning.
Example pattern:
// in CLI bootstrap: program.exitOverride((err) => { throw err; });And replace process.exit(1) with:
return; // let caller handle exitDo you want me to prep a small refactor PR to adopt exitOverride?
66-73: Static import for saveWorkflowState unless there’s a cycle.Dynamic import adds latency and complicates bundling. If no circular dep, prefer static import.
Can you confirm whether a cycle exists with a static import?
apps/cli/src/commands/next.command.ts (4)
129-141: Resolve storage type via tm-core (avoid 'auto' error).Use the resolved storage from tm-core instead of config to prevent throwing on 'auto' and to keep header/result consistent.
- // Get storage type and active tag - const storageType = this.tmCore.config.getStorageConfig().type; - if (storageType === 'auto') { - throw new Error('Storage type must be resolved before use'); - } - const activeTag = options.tag || this.tmCore.config.getActiveTag(); + // Get resolved storage type and active tag + const storageType = this.tmCore.tasks.getStorageType(); + const activeTag = options.tag || this.tmCore.config.getActiveTag();
19-24: Add a no-exit mode for programmatic usage.Silent suppresses output but process.exit(1) still fires on error. Provide a --no-exit flag to opt out.
export interface NextCommandOptions { tag?: string; format?: 'text' | 'json'; silent?: boolean; project?: string; + noExit?: boolean; } @@ this.description('Find the next available task to work on') .option('-t, --tag <tag>', 'Filter by tag') .option('-f, --format <format>', 'Output format (text, json)', 'text') .option('--silent', 'Suppress output (useful for programmatic usage)') .option('-p, --project <path>', 'Project root directory', process.cwd()) + .option('--no-exit', 'Do not call process.exit on completion or error') .action(async (options: NextCommandOptions) => { await this.executeCommand(options); }); @@ // Exit after cleanup completes - if (hasError) { + if (hasError && !options.noExit) { process.exit(1); }Also applies to: 47-56, 88-92
233-237: Ensure tm-core resources are disposed.If TmCore exposes dispose/close, call it to avoid file watchers/handles leaking.
async cleanup(): Promise<void> { if (this.tmCore) { - this.tmCore = undefined; + // Dispose if available + await (this.tmCore as any)?.dispose?.(); + this.tmCore = undefined; } }If TmCore lacks a disposer, ignore. Can you confirm whether TmCore exposes dispose/close?
40-56: Avoid class-name ambiguity with Autopilot NextCommand.Two exported classes named NextCommand exist (autopilot and root). Consider renaming this to TaskNextCommand to reduce import mistakes.
apps/cli/src/commands/autopilot/index.ts (1)
36-42: Add short alias for JSON output.Common pattern is
-j, --json. Improves ergonomics.- .option('--json', 'Output in JSON format for machine parsing') + .option('-j, --json', 'Output in JSON format for machine parsing')apps/cli/src/commands/autopilot/commit.command.ts (2)
146-156: Subtask status may be stale.You capture currentSubtask before transitions; its status may not reflect completion. Output the known final status or re-fetch by id.
- subtask: { - id: currentSubtask.id, - title: currentSubtask.title, - status: currentSubtask.status - }, + subtask: { + id: currentSubtask.id, + title: currentSubtask.title, + status: 'completed' + },Alternatively, after transitions:
const updated = orchestrator.getSubtaskById?.(currentSubtask.id);and useupdated?.status ?? 'completed'.
92-96: Stage only intended changes (optional).Staging
['.']can capture noise. Prefer staging detected paths when none are staged.apps/cli/src/commands/autopilot/next.command.ts (2)
94-96: Use configurable test command instead of hard-coded 'npm test'.Pull from workflow/config (e.g., state.context.metadata.testCommand or tm-core config) with a sane default.
- testCommand: 'npm test', // Could be customized based on config + testCommand: (state.context?.metadata as any)?.testCommand ?? 'npm test', @@ - testCommand: 'npm test', + testCommand: (state.context?.metadata as any)?.testCommand ?? 'npm test',Also applies to: 110-113
140-155: Include a suggestedNextCommand field in JSON output (optional).Emit a machine-friendly hint (e.g.,
autopilot commitfor COMMIT) to mirror text UX.apps/cli/src/commands/autopilot/abort.command.ts (1)
27-31: Add --yes alias and honor it as force.Common CLI pattern:
-y/--yesto skip confirmation.this.description('Abort the current TDD workflow and clean up state') - .option('-f, --force', 'Force abort without confirmation') + .option('-f, --force', 'Force abort without confirmation') + .option('-y, --yes', 'Alias for --force') @@ - const mergedOptions: AbortOptions = { + const mergedOptions: AbortOptions = { ...parentOpts, ...options, projectRoot: options.projectRoot || parentOpts?.projectRoot || process.cwd() }; @@ - if (!mergedOptions.force && !mergedOptions.json) { + const skipConfirm = mergedOptions.force || (options as any)?.yes; + if (!skipConfirm && !mergedOptions.json) {Also applies to: 34-42, 69-87
apps/cli/src/utils/auto-update.ts (4)
5-9: Use Node built‑ins via node: specifiers and namespace import for https.Safer and future‑proof in ESM contexts.
-import { spawn } from 'child_process'; -import https from 'https'; +import { spawn } from 'node:child_process'; +import * as https from 'node:https';
39-61: Semver edge cases (pre‑release ordering) — prefer a battle‑tested library.
compareVersionsmay misorder complex pre‑releases (e.g., rc.10 vs rc.2). Considersemver’scompare()for correctness.- export function compareVersions(v1: string, v2: string): number { - // custom parsing... - } + import { compare as semverCompare, valid as semverValid } from 'semver'; + export function compareVersions(v1: string, v2: string): number { + // Fallback to 0 when invalid to avoid throwing + const a = semverValid(v1) ? v1 : '0.0.0'; + const b = semverValid(v2) ? v2 : '0.0.0'; + return semverCompare(a, b); + }If adding a dep is undesirable, document known limitations of the custom comparator.
126-131: Regex from variable is guarded — add flexible fallback for highlight parsing.You already validate
versionformat (good). The bullet parser is strict and may miss items if CHANGELOG format drifts.- const bulletRegex = /^-\s+\[#\d+\][^\n]*?!\s+-\s+(.+?)$/gm; + // Prefer PR-form bullets; fall back to generic "- " bullets if none found + const strict = /^-\s+\[#\d+\][^\n]*?!\s+-\s+(.+?)$/gm; + const generic = /^-\s+(.+?)$/gm; + let match; + while ((match = strict.exec(minorChangesContent)) !== null) { + highlights.push(match[1].trim()); + } + if (highlights.length === 0) { + while ((match = generic.exec(minorChangesContent)) !== null) { + highlights.push(match[1].trim()); + } + }Also applies to: 159-165
287-377: Add a hard timeout and cleanup for the global install.Prevent a stuck
npmfrom hanging the CLI; kill the child and fail gracefully.export async function performAutoUpdate( latestVersion: string ): Promise<boolean> { @@ - return new Promise((resolve) => { + return new Promise((resolve) => { + const TIMEOUT_MS = + Number.parseInt(process.env.TASKMASTER_UPDATE_TIMEOUT_MS || '', 10) || 10 * 60 * 1000; @@ - const updateProcess = spawn( + const updateProcess = spawn( 'npm', @@ ); + const timer = setTimeout(() => { + spinner.fail(chalk.red('Auto-update timed out')); + try { updateProcess.kill('SIGKILL'); } catch {} + console.log( + chalk.cyan(`Please run manually: npm install -g task-master-ai@${latestVersion}`) + ); + resolve(false); + }, TIMEOUT_MS); @@ - updateProcess.on('close', (code) => { + updateProcess.on('close', (code) => { + clearTimeout(timer); @@ - updateProcess.on('error', (error) => { + updateProcess.on('error', (error) => { + clearTimeout(timer);apps/cli/src/commands/autopilot/shared.ts (2)
65-68: Wrap delete with friendly error for consistency.Match the load/save error wrapping for better UX.
-export async function deleteWorkflowState(projectRoot: string): Promise<void> { - const stateManager = new WorkflowStateManager(projectRoot); - await stateManager.delete(); -} +export async function deleteWorkflowState(projectRoot: string): Promise<void> { + const stateManager = new WorkflowStateManager(projectRoot); + try { + await stateManager.delete(); + } catch (error) { + throw new Error( + `Failed to delete workflow state: ${(error as Error).message}` + ); + } +}
81-94: Consider debouncing auto‑persist to reduce disk churn.If orchestrator emits frequent state updates, debounce/throttle saves.
I can provide a small debouncer wrapper if desired.
apps/cli/src/commands/autopilot/start.command.ts (2)
31-35: Parse--max-attemptsas a number at the CLI boundary.Let Commander coerce to number and keep types clean.
- .option('--max-attempts <number>', 'Maximum attempts per subtask', '3') + .option( + '--max-attempts <number>', + 'Maximum attempts per subtask', + (v) => Math.max(1, parseInt(v, 10) || 3), + 3 + )And update
StartOptionstomaxAttempts?: number.-interface StartOptions extends AutopilotBaseOptions { - force?: boolean; - maxAttempts?: string; -} +interface StartOptions extends AutopilotBaseOptions { + force?: boolean; + maxAttempts?: number; +}Use directly:
-const maxAttempts = parseInt(mergedOptions.maxAttempts || '3', 10); +const maxAttempts = mergedOptions.maxAttempts ?? 3;
52-67: Avoid scatteredprocess.exit(1)for testability.Prefer throwing and letting a central error handler exit once; improves composability and unit testing.
If you want, I can refactor
execute()to return early with errors and delegate exiting to the caller.Also applies to: 81-93, 156-163
apps/cli/src/commands/list.command.ts (1)
426-439: Numeric ID sort fallback to lexicographic to avoid NaN.If IDs are ever non‑pure numbers, guard against
NaN.- return Number(a.id) - Number(b.id); + const na = Number(a.id); + const nb = Number(b.id); + return Number.isFinite(na) && Number.isFinite(nb) + ? na - nb + : String(a.id).localeCompare(String(b.id), undefined, { numeric: true });apps/cli/src/ui/components/task-detail.component.ts (2)
115-117: Don’t gate complexity on typeof number; let the util parse strings too.Pass through to getComplexityWithColor; it already handles number|string and returns N/A when invalid.
- typeof task.complexity === 'number' - ? getComplexityWithColor(task.complexity) - : chalk.gray('N/A'), + getComplexityWithColor(task.complexity as any),
188-196: Tighten subtask typing.Avoid
anyfor status; bind to Task’s status union for safer rendering.-export function displaySubtasks( - subtasks: Array<{ - id: string | number; - title: string; - status: any; - description?: string; - dependencies?: string[]; - }> -): void { +export function displaySubtasks( + subtasks: Array<{ + id: string | number; + title: string; + status: Task['status']; + description?: string; + dependencies?: string[]; + }> +): void {apps/cli/src/command-registry.ts (2)
22-27: Strengthen command typing to dropas anyand reflect static register APIs.Define a RegisterableCommand with optional static register/registerOn and a zero-arg constructor.
-export interface CommandMetadata { +export interface RegisterableCommand { + new (...args: any[]): Command; + register?(program: Command, name?: string): unknown; + registerOn?(program: Command, name?: string): unknown; +} + +export interface CommandMetadata { name: string; description: string; - commandClass: typeof Command; + commandClass: RegisterableCommand; category?: 'task' | 'auth' | 'utility' | 'development'; }Apply similarly: remove
as anyfrom commandClass assignments.
144-160: Use the stronger type in registerCommand; noas anyneeded.- private static registerCommand( - program: Command, - metadata: CommandMetadata - ): void { - const CommandClass = metadata.commandClass as any; + private static registerCommand( + program: Command, + metadata: CommandMetadata + ): void { + const CommandClass = metadata.commandClass;Logic below can stay as-is; TS will recognize optional static methods via structural typing.
apps/cli/src/commands/auth.command.ts (2)
255-297: Unify error handling: prefer displayError with skipExit for consistency.Maintain centralized formatting and debug behavior without exiting this flow.
- const message = `Failed to logout: ${(error as Error).message}`; - ui.displayError(message); + displayError(error, { skipExit: true });
189-205: Optional: friendlier “time remaining”.Show minutes modulo 60 (or seconds) when <1h for clearer messaging.
apps/cli/src/commands/export.command.ts (4)
34-41: RenametaskMasterCoretotmCorefor consistency with the codebase.-export class ExportCommand extends Command { - private authManager: AuthManager; - private taskMasterCore?: TmCore; +export class ExportCommand extends Command { + private authManager: AuthManager; + private tmCore?: TmCore;
71-86: Follow-through on the rename in initializeServices.- private async initializeServices(): Promise<void> { - if (this.taskMasterCore) { + private async initializeServices(): Promise<void> { + if (this.tmCore) { return; } try { - this.taskMasterCore = await createTmCore({ + this.tmCore = await createTmCore({ projectPath: process.cwd() });
160-168: Follow-through on the rename at use sites.- const exportResult = await this.taskMasterCore!.integration.exportTasks({ + const exportResult = await this.tmCore!.integration.exportTasks({
284-286: Tighten brief ID extraction to avoid accidental garbage IDs.Only return raw when it looks like an ID.
- // Return raw if it looks like an ID - return raw; + // Return raw only if it looks like an ID + return this.isLikelyId(raw) ? raw : null;apps/cli/src/commands/show.command.ts (2)
182-190: Fetch multiple tasks in parallel for speed.- // Get each task individually - for (const taskId of taskIds) { - const result = await this.tmCore.tasks.get(taskId); - if (result.task) { - tasks.push(result.task); - } else { - notFound.push(taskId); - } - } + const results = await Promise.allSettled( + taskIds.map((id) => this.tmCore.tasks.get(id)) + ); + results.forEach((r, i) => { + if (r.status === 'fulfilled' && r.value.task) { + tasks.push(r.value.task); + } else if (r.status === 'fulfilled') { + notFound.push(taskIds[i]); + } else { + notFound.push(taskIds[i]); + } + });
124-133: Optional: validate--formatand--statusagainst known enums.If @tm/core exposes status union, reuse it for early validation.
apps/cli/src/commands/autopilot.command.ts (4)
26-40: Avoid duplicating core types; import from @tm/corePreflightCheckResult/PreflightResult mirror core. This will drift. Import the types directly and remove the local interfaces.
-import { createTmCore, type TmCore, type Task, type Subtask } from '@tm/core'; +import { + createTmCore, + type TmCore, + type Task, + type Subtask, + type PreflightResult as CorePreflightResult, + type PreflightCheckResult as CorePreflightCheckResult +} from '@tm/core'; - -export interface PreflightCheckResult { - success: boolean; - message?: string; -} - -export interface PreflightResult { - success: boolean; - testCommand: PreflightCheckResult; - gitWorkingTree: PreflightCheckResult; - requiredTools: PreflightCheckResult; - defaultBranch: PreflightCheckResult; -} +type PreflightCheckResult = CorePreflightCheckResult; +type PreflightResult = CorePreflightResult;
95-99: Unify error handling and avoid deep process.exit callsDirect exits hinder reuse/tests. Route errors through a single error handler and return control to the caller.
- if (!this.validateOptions(options)) { - process.exit(1); - } + if (!this.validateOptions(options)) { + throw new Error('Invalid options'); + } ... - ui.displayError(`Invalid task ID format: ${taskId}`); - process.exit(1); + return ui.displayError(`Invalid task ID format: ${taskId}`); ... - spinner.fail(`Task ${taskId} not found`); - ui.displayError(`Task with ID ${taskId} does not exist`); - process.exit(1); + spinner.fail(`Task ${taskId} not found`); + return ui.displayError(`Task with ID ${taskId} does not exist`); ... - this.handleError(error); - process.exit(1); + this.handleError(error);(Ensure ui.displayError/error-handler respects skipExit for programmatic flows.)
Also applies to: 111-114, 129-135
203-207: Round box width to an integerboxen expects an integer; passing a float can render oddly on some terminals.
- width: process.stdout.columns ? process.stdout.columns * 0.95 : 100 + width: process.stdout.columns + ? Math.max(40, Math.floor(process.stdout.columns * 0.95)) + : 100
371-389: Preflight output: include overall status and early guidanceMinor UX: add a summary line and hint when any check fails.
checks.forEach((check) => { ... }); + const allPass = checks.every(c => c.result.success); + console.log(); + console.log(allPass ? chalk.green('All preflight checks passed.') + : chalk.red('Some checks failed. Fix issues and re-run.'));apps/cli/src/commands/start.command.ts (5)
137-146: Compute result before launching child to simplify flowKeeps a single source of truth and enables the post‑exec display.
- // Execute command if we have one and it's not a dry run - if (!options.dryRun && coreResult.command) { - ... - await this.executeChildProcess(coreResult.command); - } - // Convert core result to CLI result with storage type - const result: StartCommandResult = { ... }; + // Convert core result to CLI result with storage type (before launching) + const result: StartCommandResult = { + ...coreResult, + storageType: this.tmCore?.config.getStorageConfig().type + }; + this.setLastResult(result); + // Execute command if we have one and it's not a dry run + if (!options.dryRun && coreResult.command) { + ... + await this.executeChildProcess(coreResult.command); + } + // Display results (dry-run, failure, or post‑exec success) + if (options.dryRun || !coreResult.started || coreResult.command) { + this.displayResults(result, options); + }
292-303: Clean up signal listeners; avoid leaks and duplicate handlersListeners are added per run and never removed. Use once() or remove them on close/error. Consider killing the whole process tree when needed.
- process.on('SIGINT', cleanup); - process.on('SIGTERM', cleanup); - process.on('exit', cleanup); + const onSigInt = () => cleanup(); + const onSigTerm = () => cleanup(); + const onExit = () => cleanup(); + process.once('SIGINT', onSigInt); + process.once('SIGTERM', onSigTerm); + process.once('exit', onExit); + + const removeHandlers = () => { + process.off('SIGINT', onSigInt); + process.off('SIGTERM', onSigTerm); + process.off('exit', onExit); + }; + + childProcess.on('close', (code) => { + removeHandlers(); + code === 0 ? resolve() : reject(new Error(`Process exited with code ${code}`)); + }); + childProcess.on('error', (error) => { + removeHandlers(); + reject(new Error(`Failed to spawn process: ${error.message}`)); + });Optionally, to terminate descendants on POSIX:
- const childProcess = spawn(command.executable, command.args, { cwd: command.cwd, stdio: 'inherit', shell: false }); + const childProcess = spawn(command.executable, command.args, { + cwd: command.cwd, + stdio: 'inherit', + shell: false, + detached: process.platform !== 'win32' + }); + const killTree = () => { + if (process.platform !== 'win32' && childProcess.pid) process.kill(-childProcess.pid, 'SIGTERM'); + else if (!childProcess.killed) childProcess.kill('SIGTERM'); + }; - const cleanup = () => { - if (childProcess && !childProcess.killed) { - childProcess.kill('SIGTERM'); - } - }; + const cleanup = killTree;
373-382: Round box width; guard against small terminalsAvoid fractional widths and enforce a reasonable minimum.
- width: process.stdout.columns * 0.95 || 100 + width: process.stdout.columns + ? Math.max(40, Math.floor(process.stdout.columns * 0.95)) + : 100Also applies to: 414-431
451-452: Avoid printing “Storage: undefined”Guard when storageType is absent.
- console.log(`\n${chalk.gray('Storage: ' + result.storageType)}`); + if (result.storageType) { + console.log(`\n${chalk.gray('Storage: ' + result.storageType)}`); + }
113-116: Consistent error handling across commandsThis file uses displayError (error-handler), while AutopilotCommand uses ui.displayError and manual exits. Standardize on error-handler for uniform UX and testability.
Also applies to: 160-166
.taskmaster/state.json
Outdated
| { | ||
| "currentTag": "master", | ||
| "lastSwitched": "2025-08-27T21:03:20.550Z", | ||
| "branchTagMapping": { | ||
| "v017-adds": "v017-adds", | ||
| "next": "next" | ||
| }, | ||
| "migrationNoticeShown": true | ||
| } | ||
| "currentTag": "update-task-remote", | ||
| "lastSwitched": "2025-10-21T19:26:58.740Z", | ||
| "branchTagMapping": { | ||
| "v017-adds": "v017-adds", | ||
| "next": "next" | ||
| }, | ||
| "migrationNoticeShown": true | ||
| } No newline at end of file |
There was a problem hiding this comment.
Do not manually edit .taskmaster/state.json—use the CLI or MCP tools instead.
Per the coding guidelines and learnings, .taskmaster/state.json is automatically created and managed by the system. Manual edits to this file bypass the system's state management and can cause inconsistencies. Use the provided CLI or MCP tools to update the state instead.
🤖 Prompt for AI Agents
.taskmaster/state.json lines 1-9: this file is system-managed and must not be
edited manually; revert any manual changes and regenerate/update it using the
provided Taskmaster CLI or MCP tooling instead of direct edits. Restore the file
to the repository's canonical version (checkout or re-run the tool to recreate
it), then use the appropriate CLI/MCP command to change
currentTag/lastSwitched/branchTagMapping so the tool updates state.json
programmatically, and commit only the tool-generated state.json; include a note
in the PR that the state file was updated via the CLI/MCP.
| .command('start') | ||
| .alias('tm start') | ||
| .description('Start implementing a task using claude-code') | ||
| .argument('<task_id>', 'ID of the task to start') | ||
| .action(async (taskId: string) => { | ||
| await this.execute(taskId); | ||
| }); | ||
| } | ||
|
|
||
| public async execute(taskId: string): Promise<void> { | ||
| // Implementation will be added in subsequent tasks | ||
| } | ||
| } |
There was a problem hiding this comment.
Commander alias example is invalid (space in alias).
Aliases can’t include spaces; .alias('tm start') won’t work. Use separate command names or multiple aliases without spaces.
Apply one of the following:
- .alias('tm start')
+ // Remove invalid alias; keep single command
+ // .alias('tm-start') // optional: hyphenated single-token aliasOr expose tm as a parent and start as a subcommand in your CLI router.
🤖 Prompt for AI Agents
In .taskmaster/tasks/task_001_tm-start.txt around lines 18 to 30, the .alias('tm
start') is invalid because aliases cannot contain spaces; replace it with a
valid single-token alias (for example .alias('tm-start') or .alias('tm') and a
second alias .alias('start') as appropriate), or refactor the CLI to expose tm
as a parent command and start as a subcommand in the router so you either
register a separate 'tm' command with a nested 'start' subcommand or use
multiple valid single-word aliases instead of a spaced alias.
| "devDependencies": { | ||
| "@biomejs/biome": "^1.9.4", | ||
| "@types/inquirer": "^9.0.3", | ||
| "@types/node": "^22.10.5", | ||
| "tsx": "^4.20.4", | ||
| "typescript": "^5.7.3", | ||
| "typescript": "^5.9.2", | ||
| "vitest": "^2.1.8" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Remove @types/inquirer; inquirer v12 provides its own types.
The separate types package targets older majors and can cause conflicts with ESM/bundled types.
"devDependencies": {
"@biomejs/biome": "^1.9.4",
- "@types/inquirer": "^9.0.3",
"@types/node": "^22.10.5",
"tsx": "^4.20.4",
"typescript": "^5.9.2",
"vitest": "^2.1.8"
},Run a quick typecheck to confirm no fallout:
🏁 Script executed:
#!/bin/bash
jq '.devDependencies' apps/cli/package.json
rg -nP --type=ts '\bimport\s+type\b.*from\s+[\'"]inquirer[\'"]' apps/cli || true
npm -w apps/cli run -s typecheckLength of output: 229
🏁 Script executed:
#!/bin/bash
# Check dependencies section for inquirer
echo "=== Dependencies ==="
jq '.dependencies' apps/cli/package.json
# Search for inquirer imports more carefully
echo "=== Searching for inquirer imports in TypeScript files ==="
rg -n 'from.*["\x27]inquirer["\x27]' apps/cli --type ts
# Check if any files import from inquirer
echo "=== Files importing inquirer ==="
find apps/cli -name "*.ts" -o -name "*.tsx" | head -20 | while read f; do grep -l "inquirer" "$f" 2>/dev/null || true; doneLength of output: 780
Human: Search result returned 1 results:
Based on result, inquirer v12.0.0 release notes on GitHub show the TypeScript support details.
Let me verify the actual details by checking the repository to confirm the claim about bundled types:
🏁 Script executed:
#!/bin/bash
# Check if inquirer package exports type definitions
echo "=== Checking inquirer package.json ==="
npm info inquirer@12.5.0 typings 2>/dev/null || echo "npm info failed"
# Alternative: check node_modules if available
if [ -d "apps/cli/node_modules/inquirer" ]; then
echo "=== inquirer package.json typings field ==="
jq '.typings, .types' apps/cli/node_modules/inquirer/package.json 2>/dev/null || true
echo "=== inquirer exports field ==="
jq '.exports' apps/cli/node_modules/inquirer/package.json 2>/dev/null | head -30
fi
# Check what @types/inquirer targets
echo "=== @types/inquirer metadata ==="
npm info @types/inquirer@9.0.3 2>/dev/null | grep -A 5 "peerDependencies" || echo "@types/inquirer info failed"Length of output: 99
🌐 Web query:
inquirer v12 bundled types typescript support
💡 Result:
Yes — Inquirer v12 ships bundled TypeScript declaration files (index.d.ts / types.d.ts) so you can use it directly from TypeScript without @types. See the package dist showing index.d.ts and types.d.ts. [1][2]
If you still see "Could not find a declaration file" errors, make sure your tsconfig has moduleResolution set to "node" (or "nodenext"/"bundler" as appropriate) and that TypeScript can resolve node_modules/inquirer/dist/...; previously older Inquirer versions relied on @types/inquirer from DefinitelyTyped. [3]
Sources:
- Inquirer v12 package files (dist with index.d.ts, types.d.ts). [1][2]
- @types/inquirer (historical DefinitelyTyped package). [3]
Remove @types/inquirer; inquirer v12 provides its own bundled types.
Inquirer v12 ships bundled TypeScript declaration files (index.d.ts / types.d.ts), making @types/inquirer redundant. The DefinitelyTyped package targets older versions and creates a type conflict with inquirer v12.5.0.
"devDependencies": {
"@biomejs/biome": "^1.9.4",
- "@types/inquirer": "^9.0.3",
"@types/node": "^22.10.5",
"tsx": "^4.20.4",
"typescript": "^5.9.2",
"vitest": "^2.1.8"
},📝 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.
| "devDependencies": { | |
| "@biomejs/biome": "^1.9.4", | |
| "@types/inquirer": "^9.0.3", | |
| "@types/node": "^22.10.5", | |
| "tsx": "^4.20.4", | |
| "typescript": "^5.7.3", | |
| "typescript": "^5.9.2", | |
| "vitest": "^2.1.8" | |
| }, | |
| "devDependencies": { | |
| "@biomejs/biome": "^1.9.4", | |
| "@types/node": "^22.10.5", | |
| "tsx": "^4.20.4", | |
| "typescript": "^5.9.2", | |
| "vitest": "^2.1.8" | |
| }, |
🤖 Prompt for AI Agents
In apps/cli/package.json around lines 34 to 41 the devDependencies include
"@types/inquirer" which conflicts with inquirer v12 bundled types; remove the
"@types/inquirer" entry from devDependencies, save the package.json, then run
your package manager (npm/yarn/pnpm) install to update the lockfile and
node_modules; also search the repo for any explicit imports/usages of
"@types/inquirer" and remove them if present.
| try { | ||
| const { task } = await this.tmCore.tasks.get(taskId); | ||
| return task; | ||
| } catch (error) { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Don’t swallow operational errors in loadTask
Returning null for any exception hides real failures (I/O, parsing). Only treat “not found” as null; rethrow others.
private async loadTask(taskId: string): Promise<Task | null> {
if (!this.tmCore) {
throw new Error('TmCore not initialized');
}
try {
const { task } = await this.tmCore.tasks.get(taskId);
return task;
- } catch (error) {
- return null;
+ } catch (error: any) {
+ if (error?.code === 'TASK_NOT_FOUND' || error?.status === 404) return null;
+ throw error;
}
}📝 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.
| try { | |
| const { task } = await this.tmCore.tasks.get(taskId); | |
| return task; | |
| } catch (error) { | |
| return null; | |
| } | |
| } | |
| try { | |
| const { task } = await this.tmCore.tasks.get(taskId); | |
| return task; | |
| } catch (error: any) { | |
| if (error?.code === 'TASK_NOT_FOUND' || error?.status === 404) return null; | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/cli/src/commands/autopilot.command.ts around lines 178 to 184, the catch
block in loadTask currently swallows all errors and returns null; change it to
only return null when the underlying error clearly indicates “not found” (e.g.,
HTTP 404, a NotFound error type or a specific error code) and rethrow any other
errors so operational failures (I/O, parsing, auth) surface. Inspect the shape
of errors returned by this.tmCore.tasks.get (status, code, or instanceof
checks), implement a conditional that returns null for the not-found case and
otherwise throws the original error, and keep TypeScript types intact.
| // Check for staged changes | ||
| const hasStagedChanges = await gitAdapter.hasStagedChanges(); | ||
| if (!hasStagedChanges) { | ||
| // Stage all changes | ||
| formatter.info('No staged changes, staging all changes...'); | ||
| await gitAdapter.stageFiles(['.']); | ||
| } | ||
|
|
There was a problem hiding this comment.
Handle "no changes to commit" gracefully.
If nothing is staged after attempting to stage, exit with a clear message to avoid Git errors.
// Check for staged changes
const hasStagedChanges = await gitAdapter.hasStagedChanges();
if (!hasStagedChanges) {
// Stage all changes
formatter.info('No staged changes, staging all changes...');
await gitAdapter.stageFiles(['.']);
}
// Get changed files for scope detection
const status = await gitAdapter.getStatus();
const changedFiles = [...status.staged, ...status.modified];
+
+ if (changedFiles.length === 0) {
+ formatter.error('No changes detected to commit', {
+ suggestion: 'Make code changes or ensure files are staged'
+ });
+ process.exit(1);
+ }Also applies to: 97-105
🤖 Prompt for AI Agents
In apps/cli/src/commands/autopilot/commit.command.ts around lines 89-96 and
97-105, after attempting to stage files the code doesn’t handle the case where
there are still no staged changes and will proceed to run Git commands that
fail; modify the flow to re-check whether any changes are staged (call
gitAdapter.hasStagedChanges() again) and if still false, log a clear message
like "No changes to commit" and exit the command gracefully (return or
process.exit(0)) instead of continuing to run commit/push steps so we avoid Git
errors.
| ## Additional Resources | ||
|
|
||
| - [Command Reference](/command-reference) - Complete CLI command documentation | ||
| - [MCP Provider Guide](/capabilities/mcp) - MCP integration details | ||
| - [Task Structure](/capabilities/task-structure) - Understanding TaskMaster's task system | ||
| - [Configuration](/configuration) - Project configuration options |
There was a problem hiding this comment.
Update documentation links to reference production docs URL.
The links at lines 1010-1013 use relative paths, but the coding guidelines specify that documentation should reference https://docs.task-master.dev rather than local file paths.
As per coding guidelines for **/*.{md,mdx} files.
## Additional Resources
-- [Command Reference](/command-reference) - Complete CLI command documentation
-- [MCP Provider Guide](/capabilities/mcp) - MCP integration details
-- [Task Structure](/capabilities/task-structure) - Understanding TaskMaster's task system
-- [Configuration](/configuration) - Project configuration options
+- [Command Reference](https://docs.task-master.dev/command-reference) - Complete CLI command documentation
+- [MCP Provider Guide](https://docs.task-master.dev/capabilities/mcp) - MCP integration details
+- [Task Structure](https://docs.task-master.dev/capabilities/task-structure) - Understanding TaskMaster's task system
+- [Configuration](https://docs.task-master.dev/configuration) - Project configuration options📝 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.
| ## Additional Resources | |
| - [Command Reference](/command-reference) - Complete CLI command documentation | |
| - [MCP Provider Guide](/capabilities/mcp) - MCP integration details | |
| - [Task Structure](/capabilities/task-structure) - Understanding TaskMaster's task system | |
| - [Configuration](/configuration) - Project configuration options | |
| ## Additional Resources | |
| - [Command Reference](https://docs.task-master.dev/command-reference) - Complete CLI command documentation | |
| - [MCP Provider Guide](https://docs.task-master.dev/capabilities/mcp) - MCP integration details | |
| - [Task Structure](https://docs.task-master.dev/capabilities/task-structure) - Understanding TaskMaster's task system | |
| - [Configuration](https://docs.task-master.dev/configuration) - Project configuration options |
🤖 Prompt for AI Agents
In apps/docs/tdd-workflow/ai-agent-integration.mdx around lines 1008 to 1013,
the documentation links are using relative paths; update each to use the
production docs base URL https://docs.task-master.dev so they resolve to the
live docs. Replace "/command-reference" with
"https://docs.task-master.dev/command-reference", "/capabilities/mcp" with
"https://docs.task-master.dev/capabilities/mcp", "/capabilities/task-structure"
with "https://docs.task-master.dev/capabilities/task-structure", and
"/configuration" with "https://docs.task-master.dev/configuration", preserving
link text. Ensure no other relative doc links remain in this block.
| }); | ||
| ``` | ||
|
|
||
| See [AI Agent Integration Guide](./ai-agent-integration.mdx) for details. |
There was a problem hiding this comment.
Replace local file path with documentation URL.
The reference to ./ai-agent-integration.mdx uses a local file path, which violates the coding guideline for documentation files.
As per coding guidelines, use the documentation site URL instead:
-See [AI Agent Integration Guide](./ai-agent-integration.mdx) for details.
+See [AI Agent Integration Guide](https://docs.task-master.dev/tdd-workflow/ai-agent-integration) for details.📝 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.
| See [AI Agent Integration Guide](./ai-agent-integration.mdx) for details. | |
| See [AI Agent Integration Guide](https://docs.task-master.dev/tdd-workflow/ai-agent-integration) for details. |
🤖 Prompt for AI Agents
In apps/docs/tdd-workflow/quickstart.mdx around line 273, replace the local file
path link " ./ai-agent-integration.mdx " with the canonical documentation site
URL for the AI Agent Integration page (e.g. use the project's docs base path
like "/docs/ai-agent-integration" or the full docs site URL), updating the link
target so it points to the published documentation site rather than a local .mdx
file.
| - Read [AI Agent Integration Guide](./ai-agent-integration.mdx) for complete documentation | ||
| - Check [Command Reference](/command-reference) for all options |
There was a problem hiding this comment.
Replace local file paths with documentation URLs.
These references use local paths instead of web URLs.
As per coding guidelines, update to use the documentation site:
-- Read [AI Agent Integration Guide](./ai-agent-integration.mdx) for complete documentation
-- Check [Command Reference](/command-reference) for all options
+- Read [AI Agent Integration Guide](https://docs.task-master.dev/tdd-workflow/ai-agent-integration) for complete documentation
+- Check [Command Reference](https://docs.task-master.dev/command-reference) for all options📝 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.
| - Read [AI Agent Integration Guide](./ai-agent-integration.mdx) for complete documentation | |
| - Check [Command Reference](/command-reference) for all options | |
| - Read [AI Agent Integration Guide](https://docs.task-master.dev/tdd-workflow/ai-agent-integration) for complete documentation | |
| - Check [Command Reference](https://docs.task-master.dev/command-reference) for all options |
🤖 Prompt for AI Agents
In apps/docs/tdd-workflow/quickstart.mdx around lines 299 to 300, the first
markdown link uses a local file path ("./ai-agent-integration.mdx") instead of
the site documentation URL; replace the href with the docs site path (e.g.
"/ai-agent-integration") while keeping the link text unchanged so the line
reads: Read [AI Agent Integration Guide](/ai-agent-integration) for complete
documentation; leave the second line (Check [Command
Reference](/command-reference) for all options) as-is.
| - Example: | ||
| ```typescript | ||
| // ✅ CORRECT - Synchronous imports with .ts extension | ||
| import { MyClass } from '../src/my-class.js'; | ||
|
|
||
| it('should verify behavior', () => { | ||
| expect(new MyClass().property).toBe(value); | ||
| }); | ||
|
|
||
| // ❌ INCORRECT - Async imports | ||
| it('should verify behavior', async () => { | ||
| const { MyClass } = await import('../src/my-class.js'); | ||
| expect(new MyClass().property).toBe(value); | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
Fix markdown fenced code block language identifier (MD040).
The TypeScript code example in the test guidelines is missing a language specifier. Add typescript after the opening triple backticks to enable proper syntax highlighting and fix the linting error.
- ```
+ ```typescript
// ✅ CORRECT - Synchronous imports with .ts extension
import { MyClass } from '../src/my-class.js';🤖 Prompt for AI Agents
In CLAUDE.md around lines 20 to 34, the TypeScript example fenced code block is
missing a language identifier which triggers MD040; update the opening fence
from ``` to ```typescript for the shown example (and any other similar
TypeScript examples in this range) so the block reads as a TypeScript code block
and preserves proper syntax highlighting and linting.
| @@ -0,0 +1,903 @@ | |||
| # Update-Tasks OOP Architecture Design | |||
|
|
|||
| **Storage-Aware Strategy Pattern with Proper Layer Separation** | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use heading syntax instead of emphasis for the section title.
Line 3 uses **bold** for what should be a heading. Replace with proper markdown heading syntax using # or ##.
-**Storage-Aware Strategy Pattern with Proper Layer Separation**
+## Storage-Aware Strategy Pattern with Proper Layer Separation📝 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.
| **Storage-Aware Strategy Pattern with Proper Layer Separation** | |
| ## Storage-Aware Strategy Pattern with Proper Layer Separation |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In UPDATE_TASKS_OOP_ARCHITECTURE.md around line 3, the section title is rendered
using bold markup (**Storage-Aware Strategy Pattern with Proper Layer
Separation**) instead of a markdown heading; replace the bolded text with an
appropriate heading level (e.g., prepend one or two hash characters: # or ##) so
the line becomes a proper markdown heading and renders as a section title.
Crunchyman-ralph
changed the base branch from
ralph/feat/implement.new.update-task
to
ralph/feat/update.task.remote
Crunchyman-ralph
changed the title
Crunchyman-ralph
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts (2)
460-466: Guard against missingsubtasksto avoid runtime errors.
parentTask.subtasks.findIndexwill throw ifsubtasksis undefined.const parentTask = tasks[parentTaskIndex]; // Find the subtask within the parent task - const subtaskIndex = parentTask.subtasks.findIndex( + if (!Array.isArray(parentTask.subtasks) || parentTask.subtasks.length === 0) { + throw new Error(`Parent task ${parentId} has no subtasks`); + } + const subtaskIndex = parentTask.subtasks.findIndex( (st) => st.id === subtaskNumericId || String(st.id) === subId );
495-506: Critical inconsistency:isTaskStatus()type guard excludes 'completed' from validation
TaskStatustype definition includes both 'done' (line 23) and 'completed' (line 28) inpackages/tm-core/src/common/types/index.ts. However, theisTaskStatus()type guard function (lines 172-184) omits 'completed' from its validation array. This creates a runtime validation mismatch: 'completed' passes TypeScript type checking but fails the runtime guard, potentially causing issues downstream when this guard is used.Add 'completed' to the validation list in
isTaskStatus()atpackages/tm-core/src/common/types/index.tsline 182:[ 'pending', 'in-progress', 'done', 'deferred', 'cancelled', 'blocked', 'review', 'completed' // Add this line ].includes(value)packages/tm-core/src/modules/workflow/services/workflow.service.ts (1)
474-493: Sanitize the tag segment in branch names.tag is concatenated raw; invalid characters (spaces, uppercase, slashes) can yield invalid branch names or git errors. Sanitize tag similarly to title.
Apply:
- private generateBranchName( + private generateBranchName( taskId: string, taskTitle: string, tag?: string ): string { // Sanitize task title for branch name const sanitizedTitle = taskTitle .toLowerCase() .replace(/[^a-z0-9]+/g, '-') // Replace non-alphanumeric with dash .replace(/^-+|-+$/g, '') // Remove leading/trailing dashes .substring(0, 50); // Limit length // Format task ID for branch name const formattedTaskId = taskId.replace(/\./g, '-'); - // Add tag prefix if tag is provided - const tagPrefix = tag ? `${tag}/` : ''; + // Sanitize tag if provided + const sanitize = (s: string) => + s.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, ''); + const sanitizedTag = tag ? sanitize(tag) : ''; + const tagPrefix = sanitizedTag ? `${sanitizedTag}/` : ''; return `${tagPrefix}task-${formattedTaskId}-${sanitizedTitle}`; }packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.ts (1)
101-131: transition is async: ensure TDD handler is awaited.transition() awaits persistence, but handleTDDPhaseTransition() remains sync and calls this.transition(...) without await, leading to unhandled Promise and ordering races.
Apply:
- async transition(event: WorkflowEvent): Promise<void> { + async transition(event: WorkflowEvent): Promise<void> { // ... - if (this.currentPhase === 'SUBTASK_LOOP') { - this.handleTDDPhaseTransition(event); - await this.triggerAutoPersist(); - return; - } + if (this.currentPhase === 'SUBTASK_LOOP') { + await this.handleTDDPhaseTransition(event); + await this.triggerAutoPersist(); + return; + }And make the handler async + await nested transitions:
- private handleTDDPhaseTransition(event: WorkflowEvent): void { + private async handleTDDPhaseTransition(event: WorkflowEvent): Promise<void> { const currentTDD = this.context.currentTDDPhase || 'RED'; switch (event.type) { case 'RED_PHASE_COMPLETE': // ... - if (event.testResults.failed === 0) { + if (event.testResults.failed === 0) { // ... - if (this.context.currentSubtaskIndex < this.context.subtasks.length) { + if (this.context.currentSubtaskIndex < this.context.subtasks.length) { this.context.currentTDDPhase = 'RED'; this.emit('tdd:red:started'); this.emit('subtask:started'); } else { // All subtasks complete, transition to FINALIZE - this.transition({ type: 'ALL_SUBTASKS_COMPLETE' }); + await this.transition({ type: 'ALL_SUBTASKS_COMPLETE' }); } break; } // ... case 'SUBTASK_COMPLETE': // ... - } else { + } else { // All subtasks complete, transition to FINALIZE - this.transition({ type: 'ALL_SUBTASKS_COMPLETE' }); + await this.transition({ type: 'ALL_SUBTASKS_COMPLETE' }); } break;
🧹 Nitpick comments (26)
packages/tm-core/src/modules/git/adapters/git-adapter.test.ts (1)
344-356: Consider moving dynamic imports to top-level (optional).The dynamic import of
simple-gitin thebeforeEachhook violates the coding guideline to "Use synchronous top-level imports in tests; avoid dynamic await import()". While this is pre-existing code (not introduced in this PR), consider refactoring to a top-level import for consistency with guidelines:+import simpleGit from 'simple-git'; + // ... existing imports ... beforeEach(async () => { testDir = path.join(os.tmpdir(), `git-status-test-${Date.now()}`); await fs.ensureDir(testDir); // Initialize actual git repo - simpleGit = (await import('simple-git')).default; const git = simpleGit(testDir);This pattern repeats in other
beforeEachblocks throughout the file (lines 605, 845, 1071, 1174).Based on coding guidelines
packages/tm-core/src/modules/auth/services/oauth-service.ts (1)
6-20: Import reorganization appears unrelated to PR objectives.The PR objectives mention migrating context gatherer and fuzzy search utilities, but these import changes in the OAuth service seem incidental. While the reorganization improves grouping, consider whether these changes should be in a separate commit or PR for clearer change tracking.
packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts (3)
61-88: Compute storageSize from fs stats instead of hardcoding 0.
Small win for observability; we already havestats.return { totalTasks, totalTags: tags.length, lastModified: stats.mtime.toISOString(), - storageSize: 0, // Could calculate actual file sizes if needed + storageSize: typeof stats.size === 'number' ? stats.size : 0, tagStats };
279-291: Stricter numeric coercion for subtask IDs.
AvoidNaNIDs; fail fast with a clear error.const normalizedTasks = this.normalizeTaskIds(tasks); ... subtasks: - task.subtasks?.map((subtask) => ({ - ...subtask, - id: Number(subtask.id), // Subtask IDs are numbers - parentId: String(subtask.parentId) // Parent ID is string (Task ID) - })) || [] + task.subtasks?.map((subtask) => { + const idNum = Number(subtask.id); + if (!Number.isFinite(idNum) || idNum < 0) { + throw new Error(`Invalid subtask id "${subtask.id}" on task ${String(task.id)}`); + } + return { + ...subtask, + id: idNum, + parentId: String(subtask.parentId) + }; + }) || []
272-274: Atomic writes/locking for tasks.json.
Concurrent writers can clobber changes. IfwriteJsonisn’t atomic (temp file + rename), consider making it so.Please confirm
FileOperations.writeJsonuses atomic write semantics.packages/tm-core/src/modules/execution/executors/claude-executor.ts (2)
33-47: Cross‑platform CLI availability check.
whichwon’t work on Windows. Fall back tocommand -v(POSIX) orwhere(Windows).-const checkProcess = spawn('which', [this.claudeConfig.command!], { shell: true }); +const checkCmd = process.platform === 'win32' ? 'where' : 'command'; +const checkArgs = process.platform === 'win32' ? [this.claudeConfig.command!] : ['-v', this.claudeConfig.command!]; +const checkProcess = spawn(checkCmd, checkArgs, { shell: true });
16-16: TypecurrentProcessprecisely.
UseChildProcess | nullfor clarity.- private currentProcess: any = null; + private currentProcess: import('child_process').ChildProcess | null = null;packages/tm-core/tests/unit/executor.test.ts (1)
5-8: LGTM on import path normalization.Explicit module paths improve clarity and avoid barrel cycles; test bodies remain synchronous and deterministic. Add a quick assertion for ExecutorFactory.getAvailableTypes() stability if desired.
packages/tm-core/src/modules/execution/executors/executor-factory.ts (2)
6-7: Import reorg is fine; factory behavior unchanged.Minor nit: consider documenting the optional options.config passed to ClaudeExecutor for consistency with tests using single-arg ctor.
38-51: Optional: add a fast path/timeout to isAvailable().CLI probing can be slow on CI. Consider a short timeout or memoization to avoid repeated checks per process.
tests/backward-compatibility/backward-compatibility.spec.js (2)
1-3: Unify test runner annotations.Repo uses Vitest widely; the Jest env pragma is unnecessary and can confuse tooling. Recommend removing it for consistency.
-/** - * @jest-environment node - */ +
15-21: Avoid dynamic import in tests; prefer static top‑level imports.Guidelines advise synchronous top‑level imports in tests. Static imports also simplify caching and speed.
+import { ContextGatherer, createContextGatherer, FuzzyTaskSearch, createFuzzyTaskSearch, findRelevantTaskIds } from '@tm/core'; +import * as OldContext from '../../scripts/modules/utils/contextGatherer.js'; +import * as OldFuzzy from '../../scripts/modules/utils/fuzzyTaskSearch.js'; ... - it('should export ContextGatherer class from old path', async () => { - const { ContextGatherer } = await import('../../scripts/modules/utils/contextGatherer.js'); - expect(ContextGatherer).toBeDefined(); - expect(typeof ContextGatherer).toBe('function'); - }); + it('should export ContextGatherer class from old path', () => { + expect(OldContext.ContextGatherer).toBeDefined(); + expect(typeof OldContext.ContextGatherer).toBe('function'); + }); ... - it('should export FuzzyTaskSearch class from old path', async () => { - const { FuzzyTaskSearch } = await import('../../scripts/modules/utils/fuzzyTaskSearch.js'); - expect(FuzzyTaskSearch).toBeDefined(); - expect(typeof FuzzyTaskSearch).toBe('function'); - }); + it('should export FuzzyTaskSearch class from old path', () => { + expect(OldFuzzy.FuzzyTaskSearch).toBeDefined(); + expect(typeof OldFuzzy.FuzzyTaskSearch).toBe('function'); + }); ... - it('should export the same ContextGatherer from both paths', async () => { - const { ContextGatherer: OldContextGatherer } = await import('../../scripts/modules/utils/contextGatherer.js'); - const { ContextGatherer: NewContextGatherer } = await import('@tm/core'); - expect(OldContextGatherer).toBe(NewContextGatherer); - }); + it('should export the same ContextGatherer from both paths', () => { + expect(OldContext.ContextGatherer).toBe(ContextGatherer); + }); ... - it('should export the same FuzzyTaskSearch from both paths', async () => { - const { FuzzyTaskSearch: OldFuzzyTaskSearch } = await import('../../scripts/modules/utils/fuzzyTaskSearch.js'); - const { FuzzyTaskSearch: NewFuzzyTaskSearch } = await import('@tm/core'); - expect(OldFuzzyTaskSearch).toBe(NewFuzzyTaskSearch); - }); + it('should export the same FuzzyTaskSearch from both paths', () => { + expect(OldFuzzy.FuzzyTaskSearch).toBe(FuzzyTaskSearch); + });Also applies to: 23-31, 33-41, 45-51, 53-61, 63-71, 71-89, 93-101, 103-111
packages/tm-core/src/modules/tasks/utils/fuzzy-task-search.ts (4)
165-184: Avoid O(N²) dependency title lookup.current map+find per dependency is quadratic on task count. Pre‑index tasks by id for linear behavior.
- private _prepareSearchableTasks(tasks: Task[]): SearchableTask[] { - return tasks.map((task) => { - const dependencyTitles = - task.dependencies?.length > 0 - ? task.dependencies - .map((depId) => { - const depTask = tasks.find((t) => t.id === depId); - return depTask ? depTask.title : ''; - }) - .filter((title) => title) - .join(' ') - : ''; - return { ...task, dependencyTitles }; - }); - } + private _prepareSearchableTasks(tasks: Task[]): SearchableTask[] { + const byId = new Map<string | number, Task>(); + for (const t of tasks) byId.set((t as any).id, t); + return tasks.map((task) => { + const titles: string[] = []; + for (const depId of task.dependencies ?? []) { + const depTask = byId.get(depId as any); + if (depTask?.title) titles.push(depTask.title); + } + return { ...task, dependencyTitles: titles.join(' ') }; + }); + }
233-241: Deterministic ordering: sort by best score before grouping.Merged results append word matches after prompt matches; sort by ascending score to keep strongest matches first across sources.
- // Merge and deduplicate results - const mergedResults = [...fuzzyResults]; + // Merge and deduplicate results + const mergedResults = [...fuzzyResults]; ... - // Group search results by relevance + // Sort by increasing score (best first) + mergedResults.sort((a, b) => (a.score ?? 1) - (b.score ?? 1)); + + // Group search results by relevanceAlso consider a stable tiebreaker by id/title for equal scores.
Also applies to: 243-263, 329-341
191-197: Unify “significant word” thresholds and avoid magic numbers.Two thresholds (>=4 chars then >5 chars) are used. Define a single constant to keep behavior predictable.
- private _extractPromptWords(prompt: string): string[] { + private static readonly MIN_WORD_LEN = 5; + private _extractPromptWords(prompt: string): string[] { return prompt .toLowerCase() .replace(/[^\w\s-]/g, ' ') .split(/\s+/) - .filter((word) => word.length > 3); + .filter((word) => word.length >= FuzzyTaskSearch.MIN_WORD_LEN); } ... - for (const word of promptWords) { - if (word.length > 5) { + for (const word of promptWords) { + if (word.length >= FuzzyTaskSearch.MIN_WORD_LEN) { const results = this.fuse.search(word, { limit: this.config.limit }); ...Also applies to: 223-231
264-276: “Recent tasks” heuristic may be misleading.Sorting by id assumes monotonic ids. If createdAt/updatedAt exists, prefer that; otherwise gate this via an option default‑off.
packages/tm-core/src/modules/tasks/utils/context-gatherer.ts (6)
355-356: Reuse exportedSearchableTaskand tighten Fuse typingsAvoid redefining the type and remove
any.-import type { IFuseOptions, FuseResult } from 'fuse.js'; +import type { IFuseOptions, FuseResult } from 'fuse.js'; +import type { SearchableTask } from './fuzzy-task-search.js';- type SearchableTask = Task & { dependencyTitles: string };- const searchOptions: IFuseOptions<any> = { + const searchOptions: IFuseOptions<SearchableTask> = {- const fuse = new Fuse(searchableTasks, searchOptions); + const fuse = new Fuse<SearchableTask>(searchableTasks, searchOptions);- const wordResults: FuseResult<any>[] = []; + const wordResults: FuseResult<SearchableTask>[] = [];Also applies to: 369-381, 383-384, 396-401
193-198: Hardcoded token model; make it configurableModel names change. Consider an overridable model (option or env) with a sane default.
Proposed pattern:
- Read from
process.env.TM_TOKEN_MODEL ?? 'gpt-4o-mini'at construction time.- Use this value inside
countTokens.Please confirm target model support for
gpt-tokensin your environment.
130-137: Blocking sync filesystem calls on an async API
readFileSync,statSync,readdirSyncwill block the event loop. If this is used in long-running processes (not just short-lived CLI), switch tofs.promisesand parallelize bounded reads.If this runs only in CLI flows, you can defer; otherwise I can provide a minimal async refactor patch.
Also applies to: 712-723, 836-879
938-941: Sanitize system-prompt file snippet
substring(0, 200)may include newlines/backticks. Normalize whitespace to avoid corrupting prompt formatting.- return `File context: ${fileContents.map((f) => `${f.path} (${f.content.substring(0, 200)}...)`).join(' | ')}`; + return `File context: ${fileContents.map((f) => { + const snippet = f.content.replace(/\s+/g, ' ').substring(0, 200); + return `${f.path} (${snippet}...)`; + }).join(' | ')}`;
548-552: Narrowformattype to the declared unionUse
ContextGathererOptions['format']to keep signatures in sync and catch invalid values at compile time.Example change (apply to all these method signatures):
- format: string, + format: ContextGathererOptions['format'],Also applies to: 694-699, 769-773, 887-897, 906-917, 925-942, 950-965, 1004-1019
129-137: Placeholders left in production codeThe comment says these helpers “need to be imported from utils”. Either move them to a shared core util and import, or drop the comment. Keeps layering consistent with the new structure.
Based on learnings.
packages/tm-core/src/modules/workflow/services/workflow.service.ts (2)
174-181: Branch creation edge cases: existing branch and detached HEAD.If the target branch already exists (but is not checked out), createAndCheckoutBranch() may fail depending on implementation; also getCurrentBranch() can be undefined in detached HEAD. Consider “checkout if exists, else create” and handle detached HEAD explicitly.
Example refactor:
-const currentBranch = await gitAdapter.getCurrentBranch(); -if (currentBranch !== branchName) { - await gitAdapter.createAndCheckoutBranch(branchName); -} +const currentBranch = await gitAdapter.getCurrentBranch(); // may be undefined in detached HEAD +const exists = await gitAdapter.branchExists(branchName).catch(() => false); +if (currentBranch !== branchName) { + if (exists) { + await gitAdapter.checkoutBranch(branchName); + } else { + await gitAdapter.createAndCheckoutBranch(branchName); + } +}
459-468: Stop the activity logger on abort to avoid dangling listeners.You start the WorkflowActivityLogger in start/resume but never stop it. Stop and clear it on abort (and optionally on finalize).
Apply:
async abortWorkflow(): Promise<void> { if (this.orchestrator) { await this.orchestrator.transition({ type: 'ABORT' }); } + // Stop activity logging + await this.activityLogger?.stop?.(); + this.activityLogger = undefined; + // Delete state file await this.stateManager.delete(); this.orchestrator = undefined; }packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.ts (2)
350-357: Duplicate ERROR handling is unreachable.ERROR is handled early in transition() and returns; updateContext('ERROR') won’t be called. Remove or consolidate to a single path.
- case 'ERROR': - this.context.errors.push(event.error); - this.emit('error:occurred', { error: event.error }); - break;
470-485: Consider emitting state:persisted on auto-persist for observability parity.persistState() emits state:persisted but triggerAutoPersist() does not. Emitting it in both paths improves telemetry consistency.
private async triggerAutoPersist(): Promise<void> { if (this.autoPersistEnabled && this.persistCallback) { - await this.persistCallback(this.getState()); + await this.persistCallback(this.getState()); + this.emit('state:persisted'); } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (68)
.taskmaster/reports/task-complexity-report_update-task-remote.json(1 hunks).taskmaster/state.json(1 hunks)packages/tm-core/src/common/constants/index.ts(1 hunks)packages/tm-core/src/common/interfaces/configuration.interface.ts(1 hunks)packages/tm-core/src/common/mappers/TaskMapper.test.ts(1 hunks)packages/tm-core/src/common/mappers/TaskMapper.ts(1 hunks)packages/tm-core/src/common/utils/path-normalizer.spec.ts(1 hunks)packages/tm-core/src/common/utils/run-id-generator.spec.ts(1 hunks)packages/tm-core/src/index.ts(1 hunks)packages/tm-core/src/modules/ai/providers/base-provider.ts(1 hunks)packages/tm-core/src/modules/auth/auth-domain.ts(1 hunks)packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts(1 hunks)packages/tm-core/src/modules/auth/managers/auth-manager.ts(1 hunks)packages/tm-core/src/modules/auth/services/credential-store.spec.ts(1 hunks)packages/tm-core/src/modules/auth/services/credential-store.test.ts(1 hunks)packages/tm-core/src/modules/auth/services/credential-store.ts(1 hunks)packages/tm-core/src/modules/auth/services/oauth-service.ts(1 hunks)packages/tm-core/src/modules/auth/services/organization.service.ts(1 hunks)packages/tm-core/src/modules/auth/services/supabase-session-storage.ts(1 hunks)packages/tm-core/src/modules/config/config-domain.ts(1 hunks)packages/tm-core/src/modules/config/managers/config-manager.spec.ts(1 hunks)packages/tm-core/src/modules/config/managers/config-manager.ts(1 hunks)packages/tm-core/src/modules/config/services/config-loader.service.spec.ts(1 hunks)packages/tm-core/src/modules/config/services/config-loader.service.ts(1 hunks)packages/tm-core/src/modules/config/services/config-merger.service.spec.ts(1 hunks)packages/tm-core/src/modules/config/services/config-persistence.service.spec.ts(1 hunks)packages/tm-core/src/modules/config/services/config-persistence.service.ts(1 hunks)packages/tm-core/src/modules/config/services/environment-config-provider.service.spec.ts(1 hunks)packages/tm-core/src/modules/config/services/runtime-state-manager.service.spec.ts(1 hunks)packages/tm-core/src/modules/execution/executors/base-executor.ts(1 hunks)packages/tm-core/src/modules/execution/executors/claude-executor.ts(1 hunks)packages/tm-core/src/modules/execution/executors/executor-factory.ts(1 hunks)packages/tm-core/src/modules/execution/services/executor-service.ts(1 hunks)packages/tm-core/src/modules/git/adapters/git-adapter.test.ts(1 hunks)packages/tm-core/src/modules/git/adapters/git-adapter.ts(1 hunks)packages/tm-core/src/modules/git/git-domain.ts(1 hunks)packages/tm-core/src/modules/git/services/branch-name-generator.spec.ts(1 hunks)packages/tm-core/src/modules/git/services/commit-message-generator.test.ts(1 hunks)packages/tm-core/src/modules/git/services/commit-message-generator.ts(1 hunks)packages/tm-core/src/modules/git/services/scope-detector.test.ts(1 hunks)packages/tm-core/src/modules/git/services/template-engine.test.ts(1 hunks)packages/tm-core/src/modules/integration/clients/supabase-client.ts(1 hunks)packages/tm-core/src/modules/integration/integration-domain.ts(1 hunks)packages/tm-core/src/modules/integration/services/export.service.ts(1 hunks)packages/tm-core/src/modules/reports/managers/complexity-report-manager.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/activity-logger.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/api-storage.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts(1 hunks)packages/tm-core/src/modules/storage/services/storage-factory.ts(1 hunks)packages/tm-core/src/modules/tasks/utils/context-gatherer.ts(1 hunks)packages/tm-core/src/modules/tasks/utils/fuzzy-task-search.ts(1 hunks)packages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.ts(1 hunks)packages/tm-core/src/modules/workflow/managers/workflow-state-manager.ts(1 hunks)packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.ts(1 hunks)packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.ts(4 hunks)packages/tm-core/src/modules/workflow/services/test-result-validator.test.ts(1 hunks)packages/tm-core/src/modules/workflow/services/test-result-validator.ts(1 hunks)packages/tm-core/src/modules/workflow/services/workflow-activity-logger.ts(1 hunks)packages/tm-core/src/modules/workflow/services/workflow.service.ts(7 hunks)packages/tm-core/src/modules/workflow/workflow-domain.ts(1 hunks)packages/tm-core/src/subpath-exports.test.ts(1 hunks)packages/tm-core/src/tm-core.ts(1 hunks)packages/tm-core/tests/auth/auth-refresh.test.ts(1 hunks)packages/tm-core/tests/integration/auth-token-refresh.test.ts(1 hunks)packages/tm-core/tests/integration/storage/activity-logger.test.ts(1 hunks)packages/tm-core/tests/unit/executor.test.ts(1 hunks)scripts/modules/utils/fuzzyTaskSearch.js(1 hunks)tests/backward-compatibility/backward-compatibility.spec.js(1 hunks)
✅ Files skipped from review due to trivial changes (31)
- packages/tm-core/src/modules/git/git-domain.ts
- packages/tm-core/src/modules/config/services/runtime-state-manager.service.spec.ts
- packages/tm-core/src/modules/config/services/environment-config-provider.service.spec.ts
- packages/tm-core/src/modules/config/services/config-loader.service.ts
- packages/tm-core/src/modules/git/services/commit-message-generator.ts
- packages/tm-core/src/modules/git/services/commit-message-generator.test.ts
- packages/tm-core/src/modules/config/services/config-loader.service.spec.ts
- packages/tm-core/src/modules/workflow/services/test-result-validator.ts
- packages/tm-core/src/common/interfaces/configuration.interface.ts
- packages/tm-core/src/modules/auth/managers/auth-manager.ts
- packages/tm-core/src/modules/git/services/template-engine.test.ts
- packages/tm-core/src/modules/ai/providers/base-provider.ts
- packages/tm-core/src/modules/config/services/config-merger.service.spec.ts
- packages/tm-core/src/modules/auth/services/credential-store.ts
- packages/tm-core/src/modules/git/services/scope-detector.test.ts
- packages/tm-core/src/modules/execution/services/executor-service.ts
- packages/tm-core/src/modules/workflow/services/test-result-validator.test.ts
- packages/tm-core/src/modules/auth/services/supabase-session-storage.ts
- packages/tm-core/src/modules/workflow/managers/workflow-state-manager.ts
- packages/tm-core/src/subpath-exports.test.ts
- packages/tm-core/src/modules/git/adapters/git-adapter.ts
- packages/tm-core/src/modules/integration/clients/supabase-client.ts
- packages/tm-core/tests/integration/auth-token-refresh.test.ts
- packages/tm-core/src/modules/config/config-domain.ts
- packages/tm-core/src/modules/storage/adapters/activity-logger.ts
- packages/tm-core/src/modules/auth/services/credential-store.spec.ts
- packages/tm-core/src/modules/workflow/services/workflow-activity-logger.ts
- packages/tm-core/src/common/constants/index.ts
- packages/tm-core/src/common/mappers/TaskMapper.test.ts
- packages/tm-core/src/modules/storage/services/storage-factory.ts
- packages/tm-core/src/modules/integration/services/export.service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .taskmaster/state.json
- .taskmaster/reports/task-complexity-report_update-task-remote.json
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/git_workflow.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Create a test file and ensure all tests pass when all subtasks are complete; commit tests if added or modified
When all subtasks are complete, run final testing using the appropriate test runner (e.g., npm test, jest, or manual testing)
Files:
packages/tm-core/tests/auth/auth-refresh.test.tspackages/tm-core/src/modules/git/services/branch-name-generator.spec.tspackages/tm-core/src/modules/git/adapters/git-adapter.test.tspackages/tm-core/src/common/utils/path-normalizer.spec.tspackages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/tests/unit/executor.test.tstests/backward-compatibility/backward-compatibility.spec.jspackages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.tspackages/tm-core/src/modules/auth/services/credential-store.test.tspackages/tm-core/tests/integration/storage/activity-logger.test.tspackages/tm-core/src/modules/config/managers/config-manager.spec.tspackages/tm-core/src/common/utils/run-id-generator.spec.tspackages/tm-core/src/modules/config/services/config-persistence.service.spec.ts
**/*.{test,spec}.*
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Test files should follow naming conventions: .test., .spec., or _test. depending on the language
Files:
packages/tm-core/tests/auth/auth-refresh.test.tspackages/tm-core/src/modules/git/services/branch-name-generator.spec.tspackages/tm-core/src/modules/git/adapters/git-adapter.test.tspackages/tm-core/src/common/utils/path-normalizer.spec.tspackages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/tests/unit/executor.test.tstests/backward-compatibility/backward-compatibility.spec.jspackages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.tspackages/tm-core/src/modules/auth/services/credential-store.test.tspackages/tm-core/tests/integration/storage/activity-logger.test.tspackages/tm-core/src/modules/config/managers/config-manager.spec.tspackages/tm-core/src/common/utils/run-id-generator.spec.tspackages/tm-core/src/modules/config/services/config-persistence.service.spec.ts
**/?(*.)+(spec|test).ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
In JavaScript/TypeScript projects using Jest, test files should match *.test.ts and *.spec.ts patterns
Files:
packages/tm-core/tests/auth/auth-refresh.test.tspackages/tm-core/src/modules/git/services/branch-name-generator.spec.tspackages/tm-core/src/modules/git/adapters/git-adapter.test.tspackages/tm-core/src/common/utils/path-normalizer.spec.tspackages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/tests/unit/executor.test.tspackages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.tspackages/tm-core/src/modules/auth/services/credential-store.test.tspackages/tm-core/tests/integration/storage/activity-logger.test.tspackages/tm-core/src/modules/config/managers/config-manager.spec.tspackages/tm-core/src/common/utils/run-id-generator.spec.tspackages/tm-core/src/modules/config/services/config-persistence.service.spec.ts
**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
**/*.test.ts: Use established mocking patterns for dependencies such as bcrypt and Prisma in test files
Test all code paths, including edge cases and error scenarios, in test files
Use descriptive names for test functions, such as should_return_error_for_invalid_input
Files:
packages/tm-core/tests/auth/auth-refresh.test.tspackages/tm-core/src/modules/git/adapters/git-adapter.test.tspackages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.tspackages/tm-core/tests/unit/executor.test.tspackages/tm-core/src/modules/auth/services/credential-store.test.tspackages/tm-core/tests/integration/storage/activity-logger.test.ts
{packages/*/src/**/*.@(spec|test).ts,apps/*/src/**/*.@(spec|test).ts,packages/*/tests/**/*.@(spec|test).ts,apps/*/tests/**/*.@(spec|test).ts,tests/unit/packages/*/**/*.@(spec|test).ts}
📄 CodeRabbit inference engine (CLAUDE.md)
{packages/*/src/**/*.@(spec|test).ts,apps/*/src/**/*.@(spec|test).ts,packages/*/tests/**/*.@(spec|test).ts,apps/*/tests/**/*.@(spec|test).ts,tests/unit/packages/*/**/*.@(spec|test).ts}: Do not use async/await in test functions unless the test is explicitly exercising asynchronous behavior
Use synchronous top-level imports in tests; avoid dynamic await import()
Keep test bodies synchronous whenever possible
Files:
packages/tm-core/tests/auth/auth-refresh.test.tspackages/tm-core/src/modules/git/services/branch-name-generator.spec.tspackages/tm-core/src/modules/git/adapters/git-adapter.test.tspackages/tm-core/src/common/utils/path-normalizer.spec.tspackages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/tests/unit/executor.test.tspackages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.tspackages/tm-core/src/modules/auth/services/credential-store.test.tspackages/tm-core/tests/integration/storage/activity-logger.test.tspackages/tm-core/src/modules/config/managers/config-manager.spec.tspackages/tm-core/src/common/utils/run-id-generator.spec.tspackages/tm-core/src/modules/config/services/config-persistence.service.spec.ts
{packages/*/src/**/*.spec.ts,apps/*/src/**/*.spec.ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Place package and app unit test files alongside source as *.spec.ts under src (packages//src//.spec.ts or apps//src//.spec.ts)
Files:
packages/tm-core/src/modules/git/services/branch-name-generator.spec.tspackages/tm-core/src/common/utils/path-normalizer.spec.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.tspackages/tm-core/src/modules/config/managers/config-manager.spec.tspackages/tm-core/src/common/utils/run-id-generator.spec.tspackages/tm-core/src/modules/config/services/config-persistence.service.spec.ts
scripts/modules/utils/fuzzyTaskSearch.js
📄 CodeRabbit inference engine (.cursor/rules/context_gathering.mdc)
scripts/modules/utils/fuzzyTaskSearch.js: Use theFuzzyTaskSearchclass inscripts/modules/utils/fuzzyTaskSearch.jsfor intelligent, semantic task discovery and relevance scoring.
ExportFuzzyTaskSearch,PURPOSE_CATEGORIES, andRELEVANCE_THRESHOLDSfrom the fuzzy search module.Use
FuzzyTaskSearchinscripts/modules/utils/fuzzyTaskSearch.jsfor intelligent task discovery, configuring search parameters, categorization, and sorting results by relevance and task ID.
Files:
scripts/modules/utils/fuzzyTaskSearch.js
scripts/modules/**
📄 CodeRabbit inference engine (.cursor/rules/dev_workflow.mdc)
When using the MCP server, restart it if core logic in
scripts/modulesor MCP tool/direct function definitions change.
Files:
scripts/modules/utils/fuzzyTaskSearch.js
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
**/*.js: Declare and initialize global variables at the top of modules to avoid hoisting issues.
Use proper function declarations to avoid hoisting issues and initialize variables before they are referenced.
Do not reference variables before their declaration in module scope.
Use dynamic imports (import()) to avoid initialization order issues in modules.
Files:
scripts/modules/utils/fuzzyTaskSearch.jstests/backward-compatibility/backward-compatibility.spec.js
{packages/*/tests/integration/**/*.test.ts,apps/*/tests/integration/**/*.test.ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Place integration tests under tests/integration as *.test.ts for packages and apps
Files:
packages/tm-core/tests/integration/storage/activity-logger.test.ts
🧠 Learnings (24)
📚 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 {tests/integration/**/*.test.ts,src/**/*.integration.test.ts} : Use supertest for API endpoint testing in integration test files
Applied to files:
packages/tm-core/tests/auth/auth-refresh.test.ts
📚 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 tests/{integration,e2e}/**/*.test.js : Properly mock session objects when required by functions, and reset environment variables between tests if modified.
Applied to files:
packages/tm-core/tests/auth/auth-refresh.test.ts
📚 Learning: 2025-10-20T16:44:58.151Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-20T16:44:58.151Z
Learning: Applies to {packages/*/src/**/*.@(spec|test).ts,apps/*/src/**/*.@(spec|test).ts,packages/*/tests/**/*.@(spec|test).ts,apps/*/tests/**/*.@(spec|test).ts,tests/unit/packages/*/**/*.@(spec|test).ts} : Use synchronous top-level imports in tests; avoid dynamic await import()
Applied to files:
packages/tm-core/src/common/utils/path-normalizer.spec.tstests/backward-compatibility/backward-compatibility.spec.js
📚 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 : Verify file operations use the correct paths in expect statements.
Applied to files:
packages/tm-core/src/common/utils/path-normalizer.spec.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: Applies to scripts/modules/utils/fuzzyTaskSearch.js : Export `FuzzyTaskSearch`, `PURPOSE_CATEGORIES`, and `RELEVANCE_THRESHOLDS` from the fuzzy search module.
Applied to files:
scripts/modules/utils/fuzzyTaskSearch.jspackages/tm-core/src/index.tspackages/tm-core/src/modules/tasks/utils/fuzzy-task-search.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/utils/fuzzyTaskSearch.js : Use `FuzzyTaskSearch` in `scripts/modules/utils/fuzzyTaskSearch.js` for intelligent task discovery, configuring search parameters, categorization, and sorting results by relevance and task ID.
Applied to files:
scripts/modules/utils/fuzzyTaskSearch.jspackages/tm-core/src/modules/tasks/utils/fuzzy-task-search.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: Applies to scripts/modules/utils/fuzzyTaskSearch.js : Use the `FuzzyTaskSearch` class from `scripts/modules/utils/fuzzyTaskSearch.js` for intelligent, semantic task discovery and relevance scoring.
Applied to files:
scripts/modules/utils/fuzzyTaskSearch.jspackages/tm-core/src/modules/tasks/utils/fuzzy-task-search.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: Applies to scripts/modules/utils/fuzzyTaskSearch.js : Use the `FuzzyTaskSearch` class in `scripts/modules/utils/fuzzyTaskSearch.js` for intelligent, semantic task discovery and relevance scoring.
Applied to files:
scripts/modules/utils/fuzzyTaskSearch.jspackages/tm-core/src/modules/tasks/utils/fuzzy-task-search.ts
📚 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 core logic independently with both data formats, mock file system operations, test tag resolution behavior, and verify migration compatibility in unit tests.
Applied to files:
tests/backward-compatibility/backward-compatibility.spec.js
📚 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 tests/{unit,integration,e2e}/**/*.test.js : Use sample task fixtures for consistent test data, mock file system operations, and test both success and error paths for task operations.
Applied to files:
tests/backward-compatibility/backward-compatibility.spec.js
📚 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 import real AI service clients in tests; create fully mocked versions that return predictable responses.
Applied to files:
tests/backward-compatibility/backward-compatibility.spec.js
📚 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/*.js : Ensure new features work with existing projects seamlessly, supporting both legacy and tagged task data formats, and support silent migration during feature usage.
Applied to files:
tests/backward-compatibility/backward-compatibility.spec.js
📚 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:
tests/backward-compatibility/backward-compatibility.spec.js
📚 Learning: 2025-09-26T19:05:47.555Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1252
File: packages/ai-sdk-provider-grok-cli/package.json:11-13
Timestamp: 2025-09-26T19:05:47.555Z
Learning: In the eyaltoledano/claude-task-master repository, internal tm/ packages use a specific export pattern where the "exports" field points to TypeScript source files (./src/index.ts) while "main" points to compiled output (./dist/index.js) and "types" points to source files (./src/index.ts). This pattern is used consistently across internal packages like tm/core and tm/ai-sdk-provider-grok-cli because they are consumed directly during build-time bundling with tsdown rather than being published as separate packages.
Applied to files:
packages/tm-core/src/modules/integration/integration-domain.tspackages/tm-core/src/index.tspackages/tm-core/src/modules/execution/executors/executor-factory.tspackages/tm-core/src/tm-core.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: Applies to scripts/modules/utils/contextGatherer.js : Export `ContextGatherer` and `createContextGatherer` from the context gatherer module.
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/tasks/utils/context-gatherer.ts
📚 Learning: 2025-10-08T19:57:00.982Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1282
File: packages/tm-core/src/utils/index.ts:16-34
Timestamp: 2025-10-08T19:57:00.982Z
Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.
Applied to files:
packages/tm-core/src/index.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 : For AI-powered commands that benefit from project context, use the ContextGatherer utility for multi-source context extraction, support task IDs, file paths, custom context, and project tree, implement fuzzy search for automatic task discovery, and display detailed token breakdown for transparency.
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/tasks/utils/context-gatherer.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: Applies to scripts/modules/utils/contextGatherer.js : Use the `ContextGatherer` class from `scripts/modules/utils/contextGatherer.js` as the centralized utility for extracting context from multiple sources in Task Master commands.
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/tasks/utils/context-gatherer.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: Applies to scripts/modules/utils/contextGatherer.js : Use the `ContextGatherer` class in `scripts/modules/utils/contextGatherer.js` as the centralized utility for extracting context from multiple sources in Task Master commands.
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/tasks/utils/context-gatherer.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/utils.js : Implement reusable task finding utilities that support both task and subtask lookups and add context to subtask results.
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-09-26T19:10:32.906Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1252
File: tsconfig.json:22-28
Timestamp: 2025-09-26T19:10:32.906Z
Learning: In the eyaltoledano/claude-task-master repository, all internal tm/ package path mappings in tsconfig.json consistently point to TypeScript source files (e.g., "./packages/*/src/index.ts") rather than built JavaScript. This is intentional architecture because tsdown bundles internal packages directly from source during build time, eliminating the need for separate compilation of internal packages.
Applied to files:
packages/tm-core/src/common/mappers/TaskMapper.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/utils/contextGatherer.js : Use `ContextGatherer` in `scripts/modules/utils/contextGatherer.js` for AI-powered commands that need project context, supporting tasks, files, custom text, project tree context, token counting, and multiple output formats.
Applied to files:
packages/tm-core/src/modules/tasks/utils/context-gatherer.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: Applies to scripts/modules/utils/contextGatherer.js : Provide token counts to help users understand context size and allow users to control what context is included when gathering context.
Applied to files:
packages/tm-core/src/modules/tasks/utils/context-gatherer.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: When implementing AI-powered commands that benefit from project context, use the context gathering pattern: validate parameters, initialize `ContextGatherer`, auto-discover relevant tasks with `FuzzyTaskSearch` if none specified, gather context with token breakdown, and display token breakdown for CLI.
Applied to files:
packages/tm-core/src/modules/tasks/utils/context-gatherer.ts
🧬 Code graph analysis (4)
tests/backward-compatibility/backward-compatibility.spec.js (3)
packages/tm-core/src/index.ts (5)
ContextGatherer(126-126)createContextGatherer(127-127)FuzzyTaskSearch(130-130)createFuzzyTaskSearch(131-131)findRelevantTaskIds(132-132)packages/tm-core/src/modules/tasks/utils/context-gatherer.ts (2)
ContextGatherer(152-1020)createContextGatherer(1029-1034)packages/tm-core/src/modules/tasks/utils/fuzzy-task-search.ts (3)
FuzzyTaskSearch(137-423)createFuzzyTaskSearch(431-436)findRelevantTaskIds(445-462)
packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.ts (1)
packages/tm-core/src/modules/workflow/types.ts (1)
WorkflowEvent(73-85)
packages/tm-core/src/modules/tasks/utils/context-gatherer.ts (4)
apps/cli/src/utils/ui.ts (1)
truncate(146-151)packages/tm-core/src/index.ts (2)
ContextGatherer(126-126)createContextGatherer(127-127)packages/tm-core/src/modules/tasks/utils/fuzzy-task-search.ts (1)
SearchableTask(11-14)packages/tm-core/src/common/types/index.ts (1)
Subtask(88-92)
packages/tm-core/src/modules/tasks/utils/fuzzy-task-search.ts (1)
packages/tm-core/src/index.ts (3)
FuzzyTaskSearch(130-130)createFuzzyTaskSearch(131-131)findRelevantTaskIds(132-132)
🪛 GitHub Actions: CI
packages/tm-core/src/modules/tasks/utils/context-gatherer.ts
[error] 155-155: TS6133: 'tag' is declared but its value is never read.
[error] 1-1: Command failed with exit code 1.
| import http from 'http'; | ||
| import os from 'os'; | ||
| import { URL } from 'url'; | ||
| import packageJson from '../../../../../../package.json' with { type: 'json' }; |
There was a problem hiding this comment.
🧩 Analysis chain
Brittle import path with 7 parent directory traversals.
The path ../../../../../../package.json is fragile and will break if the file is moved. Consider these alternatives:
- Define a build-time constant (e.g., via Rollup/Webpack plugin to inject version)
- Export version from a centralized location like
packages/tm-core/src/version.ts - Use a path alias in your TypeScript config
Additionally, the with { type: 'json' } syntax requires Node.js 17.5+ (with --experimental-json-modules) or Node.js 20.10+. Verify this is documented in your minimum Node.js version requirements.
🏁 Script executed:
#!/bin/bash
# Check if minimum Node.js version is documented
# Search for Node.js version requirements in package.json engines field
fd -t f 'package.json' --exec jq -r 'select(.engines.node != null) | "File: \(.name // "unknown") | Node version: \(.engines.node)"'
# Search for Node.js version requirements in documentation
rg -n -C2 'node.*version|engine.*node|require.*node' README.md docs/ -g '*.md' 2>/dev/null || echo "No documentation files found"Length of output: 710
Based on my verification, I found critical issues with the code that need to be addressed:
Update minimum Node.js version from >=18.0.0 to >=18.20.0 or ideally >=22.0.0.
The code uses with { type: 'json' } syntax, which was added in Node.js v18.20.0 and became the only option as of v22.0.0. However, package.json declares engines.node: >=18.0.0, which is incompatible with this syntax. The with syntax was only added in Node 18.20, while the older assert syntax has been around since Node 17.1 and 16.14.
Additionally, the brittle import path ../../../../../../package.json remains unmaintainable. Consider alternatives like a build-time constant or centralized version export.
🤖 Prompt for AI Agents
In packages/tm-core/src/modules/auth/services/oauth-service.ts around line 9,
the file imports package.json using the Node 18.20+ "with { type: 'json' }"
syntax and a brittle relative path which is incompatible with the declared
engines.node (>=18.0.0) and hard to maintain; either bump engines.node in
package.json to >=18.20.0 (or preferably >=22.0.0) to legitimize the import
syntax, or revert the code to a Node-compatible approach (e.g., use require via
createRequire, fs.readFileSync+JSON.parse, or an import from a centralized
build-time/version module) and replace the deep relative path with a
project-root-resolved mechanism (package alias, central version export or
build-time constant) to avoid brittle paths.
| function truncate(text: string, maxLength: number): string { | ||
| if (text.length <= maxLength) return text; | ||
| return text.substring(0, maxLength) + '...'; | ||
| } |
There was a problem hiding this comment.
truncate overshoots max length and duplicates existing util
Current logic returns maxLength + 3 chars. Also duplicates apps/cli/src/utils/ui.ts::truncate.
function truncate(text: string, maxLength: number): string {
if (text.length <= maxLength) return text;
- return text.substring(0, maxLength) + '...';
+ if (maxLength <= 3) return '...'.slice(0, Math.max(0, maxLength));
+ return text.substring(0, maxLength - 3) + '...';
}Follow-up: move this helper to a shared core util (or import the existing one) to avoid divergence. Based on learnings.
🤖 Prompt for AI Agents
In packages/tm-core/src/modules/tasks/utils/context-gatherer.ts around lines
144-147, the truncate function currently returns maxLength+3 characters because
it appends '...' after truncating to maxLength; replace this implementation by
truncating to maxLength - 3 (when maxLength > 3) before appending the ellipsis
so the total length equals maxLength, and instead of keeping a duplicate
implementation, import and use the existing truncate helper from
apps/cli/src/utils/ui.ts (or move that helper to a shared core util and update
imports) so there’s a single source of truth.
| export class ContextGatherer { | ||
| private projectRoot: string; | ||
| private tasksPath: string; | ||
| private tag: string; // kept for future use | ||
| private allTasks: Task[]; | ||
|
|
||
| constructor(projectRoot: string, _tag: string) { | ||
| this.projectRoot = projectRoot; | ||
| this.tasksPath = path.join( | ||
| projectRoot, | ||
| '.taskmaster', | ||
| 'tasks', | ||
| 'tasks.json' | ||
| ); | ||
| this.tag = _tag; | ||
| this.allTasks = this._loadAllTasks(); | ||
| } |
There was a problem hiding this comment.
Unblock CI: remove unused tag warning by returning it in ContextResult
TS6133 flags private tag as never read. Include it in the result to both preserve API and satisfy the compiler.
export interface ContextResult {
context: string;
analysisData?: AnalysisData;
contextSections: number;
finalTaskIds: string[];
+ tag?: string;
tokenBreakdown?: TokenBreakdown;
} const result: ContextResult = {
context: finalContext,
analysisData: analysisData,
contextSections: contextSections.length,
- finalTaskIds: Array.from(finalTaskIds)
+ finalTaskIds: Array.from(finalTaskIds),
+ tag: this.tag
};Also applies to: 339-344, 67-73
🧰 Tools
🪛 GitHub Actions: CI
[error] 155-155: TS6133: 'tag' is declared but its value is never read.
🤖 Prompt for AI Agents
In packages/tm-core/src/modules/tasks/utils/context-gatherer.ts around lines
152-168 (also apply same change at 339-344 and 67-73), the private field `tag`
is marked unused by TS6133; add `tag` to the ContextResult type/interface and
include it in the object(s) returned by the context-gathering methods so the
field is read and preserved in the public API. Update any type
declarations/imports to reflect the new property and ensure the constructor
continues to set this.tag so the returned ContextResult contains the tag string.
| const dependencyTitles = | ||
| task.dependencies?.length ?? 0 > 0 | ||
| ? task.dependencies! | ||
| .map((depId) => this.allTasks.find((t) => t.id === depId)?.title) | ||
| .filter(Boolean) | ||
| .join(' ') | ||
| : ''; | ||
| return { ...task, dependencyTitles }; | ||
| }); |
There was a problem hiding this comment.
Fix operator precedence: dependency length check is always truthy/falsey incorrectly
task.dependencies?.length ?? 0 > 0 parses as task.dependencies?.length ?? (0 > 0). Wrap the nullish-coalesced value before comparing. Also normalize ID comparison when resolving titles.
- const dependencyTitles =
- task.dependencies?.length ?? 0 > 0
- ? task.dependencies!
- .map((depId) => this.allTasks.find((t) => t.id === depId)?.title)
- .filter(Boolean)
- .join(' ')
- : '';
+ const dependencyTitles =
+ (task.dependencies?.length ?? 0) > 0
+ ? task.dependencies!
+ .map((depId) => {
+ const dep = this.allTasks.find(
+ (t) => String(t.id) === String(depId)
+ );
+ return dep?.title;
+ })
+ .filter(Boolean)
+ .join(' ')
+ : '';🤖 Prompt for AI Agents
In packages/tm-core/src/modules/tasks/utils/context-gatherer.ts around lines 358
to 366, the dependency length check uses incorrect operator precedence — change
the condition to compare the nullish-coalesced length (e.g.
(task.dependencies?.length ?? 0) > 0) so the length is evaluated before the > 0
comparison; also normalize ID comparison when resolving titles by ensuring both
depId and task.id use the same type (cast or stringify) before comparing (e.g.
String(depId) === String(t.id)) so lookups reliably find titles.
| const fullPath = path.isAbsolute(filePath) | ||
| ? filePath | ||
| : path.join(this.projectRoot, filePath); | ||
|
|
||
| if (!fs.existsSync(fullPath)) { | ||
| continue; | ||
| } | ||
|
|
There was a problem hiding this comment.
Guard against path traversal when reading requested files
A relative ../../secret escapes projectRoot. Normalize and verify the resolved path stays within projectRoot before reading.
- const fullPath = path.isAbsolute(filePath)
- ? filePath
- : path.join(this.projectRoot, filePath);
+ const base = path.resolve(this.projectRoot);
+ const fullPath = path.isAbsolute(filePath)
+ ? path.resolve(filePath)
+ : path.resolve(base, filePath);
+ const rel = path.relative(base, fullPath);
+ if (rel.startsWith('..') || path.isAbsolute(rel)) {
+ // outside project root; skip
+ continue;
+ }| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { WorkflowOrchestrator } from '../orchestrators/workflow-orchestrator.js'; | ||
| import { TestResultValidator } from '../services/test-result-validator.js'; | ||
| import type { TestResult } from '../services/test-result-validator.types.js'; | ||
| import type { |
There was a problem hiding this comment.
🧩 Analysis chain
Update tests for async transition API.
transition() now returns Promise. Synchronous assertions like expect(() => orchestrator.transition(...)).toThrow(...) will fail; also auto‑persist call assertions must await transitions.
Update patterns:
- Synchronous throw → async rejects:
-expect(() => { orchestrator.transition({ type: 'FINALIZE_COMPLETE' }); }).toThrow('Invalid transition');
+await expect(orchestrator.transition({ type: 'FINALIZE_COMPLETE' }))
+ .rejects.toThrow('Invalid transition');- Plain transitions → await:
-orchestrator.transition({ type: 'PREFLIGHT_COMPLETE' });
+await orchestrator.transition({ type: 'PREFLIGHT_COMPLETE' });- Auto‑persist expectations:
-const persistMock = vi.fn();
-orchestrator.enableAutoPersist(persistMock);
-orchestrator.transition({ type: 'PREFLIGHT_COMPLETE' });
-expect(persistMock).toHaveBeenCalledOnce();
+const persistMock = vi.fn();
+orchestrator.enableAutoPersist(persistMock);
+await orchestrator.transition({ type: 'PREFLIGHT_COMPLETE' });
+expect(persistMock).toHaveBeenCalledOnce();Run to find call sites to fix and sync throw assertions:
🏁 Script executed:
#!/bin/bash
# List all .transition(...) call sites lacking await (heuristic)
rg -nP '\borchestrator\.transition\s*\(' -C2
# Find toThrow patterns wrapping transition calls
rg -nP "toThrow\\(.*\\)\\s*\\);\\s*$" -C2 | rg -nP 'transition\s*\('Length of output: 73630
Update test file to match async transition API.
The search confirms orchestrator.transition() is now async across the codebase. Production code in workflow.service.ts correctly awaits all transitions (lines 171, 184, 366, 372, 405, 412, 415, 451, 461). The test file must be updated:
- Convert all synchronous transition calls to async (lines 44–1518): Add
awaitbefore eachorchestrator.transition()invocation. - Convert throw assertions to async rejects (lines 93–97, 498–501, 512–524, 565–577): Replace
expect(() => { orchestrator.transition(...) }).toThrow()withawait expect(orchestrator.transition(...)).rejects.toThrow(). - Await transitions before persist assertions (lines 376–427): Ensure
await orchestrator.transition()completes before checkingexpect(persistMock).
🤖 Prompt for AI Agents
In
packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.ts
lines 1–1518, the test file still calls orchestrator.transition() synchronously
and asserts thrown errors synchronously; update it to the async transition API
by adding await before every orchestrator.transition() call, change synchronous
throw assertions like expect(() => { orchestrator.transition(...) }).toThrow()
to await expect(orchestrator.transition(...)).rejects.toThrow(), and ensure all
transitions are awaited before checking persistMock or other side-effect
assertions (e.g., wrap/await the transition call then assert persistMock calls).
| describe('fuzzyTaskSearch.js', () => { | ||
| it('should export FuzzyTaskSearch class from old path', async () => { | ||
| const { FuzzyTaskSearch } = await import( | ||
| '../../scripts/modules/utils/fuzzyTaskSearch.js' | ||
| ); | ||
|
|
||
| expect(FuzzyTaskSearch).toBeDefined(); | ||
| expect(typeof FuzzyTaskSearch).toBe('function'); | ||
| }); | ||
|
|
||
| it('should export createFuzzyTaskSearch function from old path', async () => { | ||
| const { createFuzzyTaskSearch } = await import( | ||
| '../../scripts/modules/utils/fuzzyTaskSearch.js' | ||
| ); | ||
|
|
||
| expect(createFuzzyTaskSearch).toBeDefined(); | ||
| expect(typeof createFuzzyTaskSearch).toBe('function'); | ||
| }); | ||
|
|
||
| it('should export findRelevantTaskIds function from old path', async () => { | ||
| const { findRelevantTaskIds } = await import( | ||
| '../../scripts/modules/utils/fuzzyTaskSearch.js' | ||
| ); | ||
|
|
||
| expect(findRelevantTaskIds).toBeDefined(); | ||
| expect(typeof findRelevantTaskIds).toBe('function'); | ||
| }); | ||
|
|
||
| it('should be able to instantiate FuzzyTaskSearch from old path', async () => { |
There was a problem hiding this comment.
🧩 Analysis chain
Add assertions for PURPOSE_CATEGORIES and RELEVANCE_THRESHOLDS on both paths.
To lock backward compatibility per guidelines, assert these constants are exported from old and new paths.
describe('fuzzyTaskSearch.js', () => {
+ it('should export PURPOSE_CATEGORIES and RELEVANCE_THRESHOLDS from old path', () => {
+ expect(OldFuzzy.PURPOSE_CATEGORIES).toBeDefined();
+ expect(Array.isArray(OldFuzzy.PURPOSE_CATEGORIES)).toBe(true);
+ expect(OldFuzzy.RELEVANCE_THRESHOLDS).toBeDefined();
+ expect(typeof OldFuzzy.RELEVANCE_THRESHOLDS).toBe('object');
+ });
+
+ it('should export PURPOSE_CATEGORIES and RELEVANCE_THRESHOLDS from @tm/core', () => {
+ expect(PURPOSE_CATEGORIES).toBeDefined();
+ expect(Array.isArray(PURPOSE_CATEGORIES)).toBe(true);
+ expect(RELEVANCE_THRESHOLDS).toBeDefined();
+ expect(typeof RELEVANCE_THRESHOLDS).toBe('object');
+ });I need to verify the module exports and test coverage. Let me examine the relevant files.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check what's exported from fuzzyTaskSearch.js
echo "=== Checking fuzzyTaskSearch.js exports ==="
cat scripts/modules/utils/fuzzyTaskSearch.js | head -100
Length of output: 505
🏁 Script executed:
#!/bin/bash
# Check the full backward-compatibility test file
echo "=== Checking backward-compatibility.spec.js ==="
cat tests/backward-compatibility/backward-compatibility.spec.jsLength of output: 3467
🏁 Script executed:
#!/bin/bash
# Search for PURPOSE_CATEGORIES and RELEVANCE_THRESHOLDS in the test file
echo "=== Searching for PURPOSE_CATEGORIES and RELEVANCE_THRESHOLDS in tests ==="
rg -n "PURPOSE_CATEGORIES|RELEVANCE_THRESHOLDS" tests/backward-compatibility/backward-compatibility.spec.jsLength of output: 74
I need to check if these constants are actually exported from @tm/core and where they're defined.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for PURPOSE_CATEGORIES and RELEVANCE_THRESHOLDS definitions
echo "=== Searching for PURPOSE_CATEGORIES and RELEVANCE_THRESHOLDS ==="
rg -n "PURPOSE_CATEGORIES|RELEVANCE_THRESHOLDS" --type js -g "!node_modules" -g "!dist"
Length of output: 65
🏁 Script executed:
#!/bin/bash
# Check what @tm/core exports (if accessible)
echo "=== Looking for @tm/core module ==="
fd -t f "@tm" --max-depth 5 2>/dev/null || echo "Not found as directory"
# Try finding package.json or exports related to @tm/core
find . -name "package.json" -type f | xargs grep -l "@tm/core" 2>/dev/null | head -5Length of output: 145
🏁 Script executed:
#!/bin/bash
# Look for node_modules/@tm/core or src/@tm/core
echo "=== Checking for @tm/core in various locations ==="
ls -la node_modules/@tm/core 2>/dev/null | head -20 || echo "Not in node_modules"
ls -la src/@tm/core 2>/dev/null | head -20 || echo "Not in src"
find . -path "*/node_modules/@tm/core*" -type f 2>/dev/null | head -10Length of output: 50
---
🏁 Script executed:
```shell
#!/bin/bash
# Explore packages/tm-core structure
echo "=== Checking packages/tm-core structure ==="
find packages/tm-core -type f -name "*.js" -o -name "*.ts" | grep -E "(index|fuzzy|export)" | head -20
Length of output: 1004
🏁 Script executed:
#!/bin/bash
# Look for fuzzyTaskSearch in packages/tm-core
echo "=== Searching for fuzzyTaskSearch in packages/tm-core ==="
find packages/tm-core -type f \( -name "*fuzzy*" -o -name "index.*" \) | head -20Length of output: 996
🏁 Script executed:
#!/bin/bash
# Check the main entry point of @tm/core
echo "=== Checking packages/tm-core/package.json ==="
cat packages/tm-core/package.json | head -50Length of output: 1488
🏁 Script executed:
#!/bin/bash
# Search for PURPOSE_CATEGORIES or RELEVANCE_THRESHOLDS in packages/tm-core
echo "=== Searching in packages/tm-core ==="
rg -n "PURPOSE_CATEGORIES|RELEVANCE_THRESHOLDS" packages/tm-core/Length of output: 1018
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the fuzzy-task-search.ts file exports
echo "=== Checking packages/tm-core/src/modules/tasks/utils/fuzzy-task-search.ts exports ==="
cat packages/tm-core/src/modules/tasks/utils/fuzzy-task-search.ts | tail -50
Length of output: 1386
🏁 Script executed:
#!/bin/bash
# Check what's exported from fuzzy-task-search.ts
echo "=== Looking for export statements in fuzzy-task-search.ts ==="
rg -n "^export" packages/tm-core/src/modules/tasks/utils/fuzzy-task-search.tsLength of output: 652
🏁 Script executed:
#!/bin/bash
# Check the tasks module index
echo "=== Checking packages/tm-core/src/modules/tasks/index.ts ==="
cat packages/tm-core/src/modules/tasks/index.tsLength of output: 136
🏁 Script executed:
#!/bin/bash
# Check the main @tm/core index
echo "=== Checking packages/tm-core/src/index.ts ==="
cat packages/tm-core/src/index.ts | grep -A 5 -B 5 "fuzzy\|FuzzyTaskSearch\|PURPOSE_CATEGORIES\|RELEVANCE_THRESHOLDS" || head -50 packages/tm-core/src/index.tsLength of output: 382
Export PURPOSE_CATEGORIES and RELEVANCE_THRESHOLDS from both @tm/core and the old path, then add test assertions for these constants.
The constants exist in the fuzzy-task-search module but are not currently exported. You need to:
- Update
packages/tm-core/src/modules/tasks/utils/fuzzy-task-search.tsto export PURPOSE_CATEGORIES and RELEVANCE_THRESHOLDS - Update
packages/tm-core/src/index.tsto include these exports - Update
scripts/modules/utils/fuzzyTaskSearch.jsto re-export them alongside the existing exports - Add test assertions in
tests/backward-compatibility/backward-compatibility.spec.jsto verify both paths export these constants
🤖 Prompt for AI Agents
In tests/backward-compatibility/backward-compatibility.spec.js around lines 43
to 71, add assertions to verify PURPOSE_CATEGORIES and RELEVANCE_THRESHOLDS are
exported from the old path; to make this pass, export those constants from
packages/tm-core/src/modules/tasks/utils/fuzzy-task-search.ts, then re-export
them from packages/tm-core/src/index.ts so they are available from @tm/core, and
update scripts/modules/utils/fuzzyTaskSearch.js to re-export PURPOSE_CATEGORIES
and RELEVANCE_THRESHOLDS along with the existing exports; finally add expect
checks in the spec to assert the constants are defined and of the expected
type/value when imported from the old path and (optionally) from @tm/core.
|
will focus and re-open on later, currently not the focus |
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
Bug Fixes
Refactoring