Conversation
|
WalkthroughAdds a new workflow engine package and integrates it into core and CLI. Introduces workflow CLI commands (start, list, stop, status) and a command registry. Removes legacy registerOn methods. Updates build/typecheck scripts and core build config. Adds UI helpers (with duplicate definitions). Adjusts scripts to use centralized registration. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (WorkflowStartCommand)
participant Core as TaskMasterCore
participant WS as WorkflowService
participant TEM as TaskExecutionManager
participant WT as WorktreeManager
participant PS as ProcessSandbox
participant SM as WorkflowStateManager
User->>CLI: workflow start <taskId> [options]
CLI->>Core: initialize(projectRoot, workflow config)
Core->>WS: initialize()
WS->>TEM: initialize()
TEM->>SM: loadState()
CLI->>Core: get task by id
CLI->>WS: start(taskId, {branch, timeout, env})
WS->>TEM: startTaskExecution(task, options)
TEM->>WT: createWorktree(taskId, branch)
TEM->>PS: startProcess(workflowId, taskId, prompt, {cwd, timeout, env})
PS-->>TEM: process.started(pid)
TEM->>SM: registerWorkflow(context)
TEM-->>WS: workflowId
WS-->>CLI: workflowId
CLI-->>User: Print workflowId, next steps
sequenceDiagram
autonumber
actor User
participant CLI as CLI (WorkflowStatusCommand)
participant TEM as TaskExecutionManager
participant SM as WorkflowStateManager
User->>CLI: workflow status <workflowId> [--watch]
CLI->>TEM: getWorkflowStatus(workflowId)
TEM->>SM: getWorkflow(workflowId)
SM-->>TEM: context
TEM-->>CLI: context
alt watch mode
loop every 2s until final
CLI->>TEM: getWorkflowStatus()
TEM->>SM: getWorkflow()
SM-->>TEM: context
TEM-->>CLI: context
CLI-->>User: refresh display
end
else once
CLI-->>User: print status
end
sequenceDiagram
autonumber
participant Main as scripts/modules/commands.js
participant CLI as @tm/cli
participant Reg as registerAllCommands
participant Cmds as Command Classes
Main->>CLI: import registerAllCommands
Main->>Reg: registerAllCommands(program)
Reg->>Cmds: iterate [ListTasks, Auth, Workflow]
Cmds-->>Reg: static register(program)
Reg-->>Main: commands attached to program
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 95
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/cli/src/utils/ui.ts (1)
1-385: Fix Biome formatting to unblock CI.CI reports Biome format issues for this file. Run: npm run format.
package.json (1)
24-26: Add workflow typecheck to the aggregate script.New package isn’t included in the root typecheck.
- "typecheck": "npm run typecheck:core && npm run typecheck:cli", + "typecheck": "npm run typecheck:core && npm run typecheck:workflow && npm run typecheck:cli", "typecheck:core": "cd packages/tm-core && npm run typecheck", + "typecheck:workflow": "cd packages/workflow-engine && npm run typecheck", "typecheck:cli": "cd apps/cli && npm run typecheck",packages/tm-core/package.json (1)
10-13: Consider adding "require" condition for the root export.If any CJS consumers import "@tm/core", missing "require" under "." with an exports map can break resolution. Add it if CJS usage exists.
".": { "types": "./src/index.ts", - "import": "./dist/index.js" + "import": "./dist/index.js", + "require": "./dist/index.js" },packages/workflow-engine/src/process/process-sandbox.ts (1)
1-379: CI: Biome format check failing — fix biome.jsonbiome.json (repo root) contains unknown keys causing Biome to error: files.ignore (line 3), linter.include (line 28), linter.ignore (line 29). Update/remove those keys to match Biome's config schema (e.g., use supported keys like "includes" or the appropriate scanner ignore settings), then run npm run format / npm run format-check.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
apps/cli/package.json(1 hunks)apps/cli/src/commands/auth.command.ts(0 hunks)apps/cli/src/commands/index.ts(1 hunks)apps/cli/src/commands/list.command.ts(0 hunks)apps/cli/src/commands/workflow.command.ts(1 hunks)apps/cli/src/commands/workflow/index.ts(1 hunks)apps/cli/src/commands/workflow/workflow-list.command.ts(1 hunks)apps/cli/src/commands/workflow/workflow-start.command.ts(1 hunks)apps/cli/src/commands/workflow/workflow-status.command.ts(1 hunks)apps/cli/src/commands/workflow/workflow-stop.command.ts(1 hunks)apps/cli/src/index.ts(1 hunks)apps/cli/src/utils/ui.ts(1 hunks)package.json(1 hunks)packages/tm-core/package.json(2 hunks)packages/tm-core/src/index.ts(1 hunks)packages/tm-core/src/task-master-core.ts(5 hunks)packages/tm-core/src/workflow/index.ts(1 hunks)packages/tm-core/src/workflow/workflow-service.ts(1 hunks)packages/tm-core/tsup.config.ts(1 hunks)packages/workflow-engine/README.md(1 hunks)packages/workflow-engine/package.json(1 hunks)packages/workflow-engine/src/errors/index.ts(1 hunks)packages/workflow-engine/src/errors/workflow.errors.ts(1 hunks)packages/workflow-engine/src/index.ts(1 hunks)packages/workflow-engine/src/process/index.ts(1 hunks)packages/workflow-engine/src/process/process-sandbox.ts(1 hunks)packages/workflow-engine/src/state/index.ts(1 hunks)packages/workflow-engine/src/state/workflow-state-manager.ts(1 hunks)packages/workflow-engine/src/task-execution/index.ts(1 hunks)packages/workflow-engine/src/task-execution/task-execution-manager.ts(1 hunks)packages/workflow-engine/src/types/index.ts(1 hunks)packages/workflow-engine/src/types/workflow.types.ts(1 hunks)packages/workflow-engine/src/worktree/index.ts(1 hunks)packages/workflow-engine/src/worktree/worktree-manager.ts(1 hunks)packages/workflow-engine/tsconfig.json(1 hunks)packages/workflow-engine/tsup.config.ts(1 hunks)packages/workflow-engine/vitest.config.ts(1 hunks)scripts/modules/commands.js(2 hunks)
💤 Files with no reviewable changes (2)
- apps/cli/src/commands/auth.command.ts
- apps/cli/src/commands/list.command.ts
🧰 Additional context used
📓 Path-based instructions (6)
scripts/modules/commands.js
📄 CodeRabbit inference engine (.cursor/rules/ai_services.mdc)
scripts/modules/commands.js: Centralize all LLM calls throughgenerateTextServiceorgenerateObjectService.
Do not import or call anything from the oldai-services.js,ai-client-factory.js, orai-client-utils.jsfiles.
Do not fetch AI-specific parameters (model ID, max tokens, temp) usingconfig-manager.jsgetters for the AI call. Pass theroleinstead.
Do not implement fallback or retry logic outsideai-services-unified.js.
Do not handle API key resolution outside the service layer (it usesutils.jsinternally).
Determine the appropriaterole(main,research,fallback) in your core logic and pass it to the service.
Pass thesessionobject (received in thecontextparameter, especially from direct function wrappers) to the service call when in MCP context.
UsegenerateTextServiceand implement robust manual JSON parsing (with Zod validation after parsing) when structured output is needed, asgenerateObjectServicehas shown unreliability with some providers/schemas.
Be aware of potential reliability issues withgenerateObjectServiceacross different providers and complex schemas. PrefergenerateTextService+ manual parsing as a more robust alternative for structured data needs.
scripts/modules/commands.js: All new user-facing commands should be added to 'scripts/modules/commands.js'.
Use consistent patterns for option naming and help text in CLI commands.
Follow the Commander.js model for subcommand structure in CLI commands.
When using callbacks (like in Commander.js commands), define them separately to allow testing the callback logic independently.
Add help text to the command definition and update 'dev_workflow.mdc' with command reference when adding a new feature.
Follow the established pattern in 'commands.js' for CLI command implementation, using Commander.js for argument parsing, including comprehensive help text and examples, and supporting tagged task context awareness.
Provide clear error messages for common failu...
Files:
scripts/modules/commands.js
scripts/modules/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Each module in scripts/modules/ should be focused on a single responsibility, following the modular architecture (e.g., commands.js for CLI command handling, task-manager.js for task data and core logic, dependency-manager.js for dependency management, ui.js for CLI output formatting, ai-services-unified.js for AI service integration, config-manager.js for configuration management, utils.js for utility functions).
scripts/modules/*.js: Export all core functions, helper functions, and utility methods needed by your new function or command from their respective modules. Explicitly review the module's export block to ensure every required dependency is included.
Pass all required parameters to functions you call within your implementation and verify that direct function parameters match their core function counterparts.
Use consistent file naming conventions: 'task_${id.toString().padStart(3, '0')}.txt', use path.join for composing file paths, and use appropriate file extensions (.txt for tasks, .json for data).
Use structured error objects with code and message properties, include clear error messages, and handle both function-specific and file system errors.
Import all silent mode utilities together from 'scripts/modules/utils.js' and always use isSilentMode() to check global silent mode status. Wrap core function calls within direct functions using enableSilentMode() and disableSilentMode() in a try/finally block if the core function might produce console output.
Core functions should check outputFormat === 'text' before displaying UI elements and use internal logging that respects silent mode.
Design functions to accept dependencies as parameters (dependency injection) and avoid hard-coded dependencies that are difficult to mock.
Keep pure logic separate from I/O operations or UI rendering to allow testing the logic without mocking complex dependencies.
When implementing core logic for new features, do so in 'scripts/modules/' before CLI or MCP interfaces, and d...
Files:
scripts/modules/commands.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/commands.js
scripts/modules/*
📄 CodeRabbit inference engine (.cursor/rules/tags.mdc)
scripts/modules/*: Every command that reads or writes tasks.json must be tag-aware
All command files must import getCurrentTag from utils.js
Every CLI command that operates on tasks must include the --tag CLI option
All commands must resolve the tag using the pattern: options.tag || getCurrentTag(projectRoot) || 'master'
All commands must find projectRoot with error handling before proceeding
All commands must pass { projectRoot, tag } as context to core functions
MCP direct functions must accept and use a context object containing projectRoot and tag, and pass them to core functions
Do not hard-code tag resolution (e.g., const tag = options.tag || 'master';); always use getCurrentTag
Do not omit the --tag CLI option in commands that operate on tasks
Do not omit the context parameter when calling core functions from commands
Do not call readJSON or writeJSON without passing projectRoot and tag
Files:
scripts/modules/commands.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/commands.js
package.json
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Files:
package.json
🧠 Learnings (33)
📓 Common learnings
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/AGENTS.md:0-0
Timestamp: 2025-09-01T09:55:15.061Z
Learning: Applies to assets/.claude/commands/taskmaster-next.md : Create .claude/commands/taskmaster-next.md implementing the 4-step workflow to find and show the next task
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/AGENTS.md:0-0
Timestamp: 2025-09-01T09:55:15.061Z
Learning: Applies to assets/.claude/commands/taskmaster-complete.md : Create .claude/commands/taskmaster-complete.md implementing the 5-step task completion workflow
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T12:30:23.843Z
Learning: Import Task Master's development workflow commands and guidelines; treat the contents of ./.taskmaster/CLAUDE.md as if included in the main CLAUDE.md
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 Learning: 2025-07-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 : Export the registerCommands function and keep the CLI setup code clean and maintainable.
Applied to files:
apps/cli/src/commands/index.tsapps/cli/src/commands/workflow/index.tsapps/cli/src/index.tsscripts/modules/commands.js
📚 Learning: 2025-08-03T12:13:33.875Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to jest.config.js : Create and maintain a production-ready Jest configuration in jest.config.js
Applied to files:
packages/workflow-engine/vitest.config.ts
📚 Learning: 2025-08-11T12:30:23.843Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T12:30:23.843Z
Learning: Import Task Master's development workflow commands and guidelines; treat the contents of ./.taskmaster/CLAUDE.md as if included in the main CLAUDE.md
Applied to files:
packages/workflow-engine/README.md
📚 Learning: 2025-07-18T17:10:53.657Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/glossary.mdc:0-0
Timestamp: 2025-07-18T17:10:53.657Z
Learning: Guide for using Task Master to manage task-driven development workflows with tagged task lists support (dev_workflow.mdc).
Applied to files:
packages/workflow-engine/README.md
📚 Learning: 2025-07-18T17:10:53.657Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/glossary.mdc:0-0
Timestamp: 2025-07-18T17:10:53.657Z
Learning: Describes the high-level architecture of the Task Master CLI application, including the new tagged task lists system (architecture.mdc).
Applied to files:
packages/workflow-engine/README.md
📚 Learning: 2025-09-01T09:55:15.061Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/AGENTS.md:0-0
Timestamp: 2025-09-01T09:55:15.061Z
Learning: Applies to assets/.claude/commands/taskmaster-complete.md : Create .claude/commands/taskmaster-complete.md implementing the 5-step task completion workflow
Applied to files:
packages/workflow-engine/README.mdapps/cli/src/commands/workflow/workflow-start.command.ts
📚 Learning: 2025-07-18T17:10:53.657Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/glossary.mdc:0-0
Timestamp: 2025-07-18T17:10:53.657Z
Learning: Guidelines for integrating new features into the Task Master CLI with tagged system considerations (new_features.mdc).
Applied to files:
packages/workflow-engine/README.md
📚 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/task-manager.js : Features that create, read, update, or delete tasks belong in 'scripts/modules/task-manager.js'.
Applied to files:
packages/workflow-engine/src/task-execution/index.ts
📚 Learning: 2025-07-18T17:07:39.336Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/architecture.mdc:0-0
Timestamp: 2025-07-18T17:07:39.336Z
Learning: Applies to scripts/modules/task-manager/*.js : Files in scripts/modules/task-manager/ should each handle a specific action related to task management (e.g., add-task.js, expand-task.js), supporting the tagged task lists system and backward compatibility.
Applied to files:
packages/workflow-engine/src/task-execution/index.ts
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Use consistent formatting for task files, include all task properties in text files, and format dependencies with status indicators.
Applied to files:
packages/workflow-engine/src/task-execution/index.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:
packages/workflow-engine/src/task-execution/index.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/dependency-manager.js : Features that handle task relationships belong in 'scripts/modules/dependency-manager.js'.
Applied to files:
packages/workflow-engine/src/task-execution/index.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 : Use tagged task system aware functions for task finding and manipulation, handle both task and subtask operations, and validate task IDs before operations.
Applied to files:
packages/workflow-engine/src/task-execution/index.ts
📚 Learning: 2025-09-03T12:20:36.005Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/index.ts:56-57
Timestamp: 2025-09-03T12:20:36.005Z
Learning: The logger functionality in tm-core should only be available through the main package entry point (import { getLogger, createLogger, setGlobalLogger } from 'tm-core'), not as a separate subpath export like other modules such as auth or storage.
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-07-31T20:49:04.638Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#997
File: apps/extension/package.publish.json:2-8
Timestamp: 2025-07-31T20:49:04.638Z
Learning: In the eyaltoledano/claude-task-master repository, the VS Code extension uses a 3-file packaging system where package.json (with name "extension") is for development within the monorepo, while package.publish.json (with name "task-master-hamster") contains the clean manifest for VS Code marketplace publishing. The different names are intentional and serve distinct purposes in the build and publishing workflow.
Applied to files:
packages/workflow-engine/package.json
📚 Learning: 2025-09-03T12:16:15.866Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/package.json:13-64
Timestamp: 2025-09-03T12:16:15.866Z
Learning: For internal packages in the claude-task-master project, Crunchyman-ralph prefers pointing package.json "types" entries to src .ts files rather than dist .d.ts files for better developer experience (DX), as the packages are not being exported as SDKs.
Applied to files:
packages/workflow-engine/package.jsonpackages/tm-core/package.json
📚 Learning: 2025-08-07T13:00:22.966Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1090
File: apps/extension/package.json:241-243
Timestamp: 2025-08-07T13:00:22.966Z
Learning: In monorepos, local packages should use "*" as the version constraint in package.json dependencies, as recommended by npm. This ensures the local version from within the same workspace is always used, rather than attempting to resolve from external registries. This applies to packages like task-master-ai within the eyaltoledano/claude-task-master monorepo.
Applied to files:
packages/workflow-engine/package.json
📚 Learning: 2025-07-18T17:12:57.903Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to scripts/modules/commands.js : Add help text to the command definition and update 'dev_workflow.mdc' with command reference when adding a new feature.
Applied to files:
apps/cli/src/commands/workflow.command.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,mcp-server/src/core/utils/path-utils.js,mcp-server/src/tools/utils.js} : Keep utilities relevant to their location, export all utility functions in a single statement per file, group related exports together, export configuration constants, do not use default exports, and do not create circular dependencies.
Applied to files:
packages/tm-core/package.json
📚 Learning: 2025-09-02T21:51:41.383Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:0-0
Timestamp: 2025-09-02T21:51:41.383Z
Learning: In packages/tm-core/src/auth/config.ts, the BASE_DOMAIN configuration intentionally does not include runtime environment variable fallbacks like TM_BASE_DOMAIN or HAMSTER_BASE_URL. The maintainers prefer to keep these capabilities "hush hush" and undocumented, using only the build-time TM_PUBLIC_BASE_DOMAIN and the default value.
Applied to files:
packages/tm-core/tsup.config.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/commands.js : Follow the established pattern in 'commands.js' for CLI command implementation, using Commander.js for argument parsing, including comprehensive help text and examples, and supporting tagged task context awareness.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:06:57.833Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_services.mdc:0-0
Timestamp: 2025-07-18T17:06:57.833Z
Learning: Applies to scripts/modules/task-manager/*.js : Do not import or call anything from the old `ai-services.js`, `ai-client-factory.js`, or `ai-client-utils.js` files.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:06:57.833Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_services.mdc:0-0
Timestamp: 2025-07-18T17:06:57.833Z
Learning: Applies to scripts/modules/commands.js : Do not import or call anything from the old `ai-services.js`, `ai-client-factory.js`, or `ai-client-utils.js` files.
Applied to files:
scripts/modules/commands.js
📚 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 : Place utilities used primarily by the core `task-master` CLI logic and command modules (`scripts/modules/*`) into `scripts/modules/utils.js`.
Applied to files:
scripts/modules/commands.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/commands.js : All new user-facing commands should be added to 'scripts/modules/commands.js'.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Use the global `task-master` CLI command instead of directly invoking `node scripts/dev.js` for all task management operations.
Applied to files:
scripts/modules/commands.js
📚 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:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Start new projects by running `task-master init` or `node scripts/dev.js parse-prd --input=<prd-file.txt>` to generate the initial tasks.json.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Extract tasks from PRD documents using AI, create them in the current tag context (defaulting to 'master'), provide clear prompts to guide AI task generation, and validate/clean up AI-generated tasks.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-20T01:35:05.831Z
Learnt from: rtmcrc
PR: eyaltoledano/claude-task-master#933
File: scripts/modules/task-manager/parse-prd.js:226-226
Timestamp: 2025-07-20T01:35:05.831Z
Learning: The parsePRD function in scripts/modules/task-manager/parse-prd.js has a different parameter structure than other task-manager functions - it uses `options` parameter instead of `context` parameter because it generates tasks from PRD documents rather than operating on existing tasks.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:16:32.622Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ui.mdc:0-0
Timestamp: 2025-07-18T17:16:32.622Z
Learning: Applies to scripts/modules/ui.js : Use cli-table3 for table rendering, include colored bold headers, and set appropriate column widths for readability
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-08-03T12:13:33.875Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to package.json : Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Applied to files:
package.json
🧬 Code graph analysis (15)
apps/cli/src/commands/index.ts (4)
apps/cli/src/index.ts (4)
registerAllCommands(13-13)ListTasksCommand(7-7)AuthCommand(8-8)WorkflowCommand(9-9)apps/cli/src/commands/list.command.ts (1)
ListTasksCommand(48-324)apps/cli/src/commands/auth.command.ts (1)
AuthCommand(32-503)apps/cli/src/commands/workflow.command.ts (1)
WorkflowCommand(17-56)
packages/workflow-engine/src/process/process-sandbox.ts (3)
packages/workflow-engine/src/types/workflow.types.ts (3)
WorkflowProcess(80-95)WorkflowEventType(65-78)WorkflowEvent(56-63)packages/workflow-engine/src/errors/workflow.errors.ts (1)
ProcessError(26-31)packages/workflow-engine/src/task-execution/task-execution-manager.ts (1)
cleanup(342-351)
packages/workflow-engine/src/types/workflow.types.ts (1)
packages/tm-core/src/workflow/index.ts (7)
WorkflowExecutionContext(10-10)WorkflowStatus(11-11)WorkflowEvent(12-12)WorkflowEventType(13-13)WorkflowProcess(14-14)ProcessStatus(15-15)WorktreeInfo(16-16)
apps/cli/src/commands/workflow/workflow-list.command.ts (3)
packages/workflow-engine/src/index.ts (1)
TaskExecutionManager(19-19)packages/workflow-engine/src/task-execution/task-execution-manager.ts (2)
TaskExecutionManager(43-433)TaskExecutionManagerConfig(34-37)packages/workflow-engine/src/types/workflow.types.ts (1)
WorkflowExecutionContext(19-44)
packages/workflow-engine/src/task-execution/task-execution-manager.ts (5)
packages/workflow-engine/src/types/workflow.types.ts (4)
WorkflowConfig(6-17)WorkflowEvent(56-63)WorkflowExecutionContext(19-44)WorkflowStatus(46-54)packages/workflow-engine/src/worktree/worktree-manager.ts (2)
WorktreeManager(25-351)WorktreeManagerConfig(12-19)packages/workflow-engine/src/process/process-sandbox.ts (2)
ProcessSandbox(41-378)ProcessSandboxConfig(15-24)packages/workflow-engine/src/state/workflow-state-manager.ts (2)
WorkflowStateManager(47-320)WorkflowStateConfig(15-20)packages/workflow-engine/src/errors/workflow.errors.ts (3)
MaxConcurrentWorkflowsError(51-59)WorkflowError(6-17)WorkflowNotFoundError(44-49)
apps/cli/src/commands/workflow.command.ts (4)
apps/cli/src/commands/workflow/workflow-start.command.ts (1)
WorkflowStartCommand(32-239)apps/cli/src/commands/workflow/workflow-list.command.ts (1)
WorkflowListCommand(28-253)apps/cli/src/commands/workflow/workflow-stop.command.ts (1)
WorkflowStopCommand(26-260)apps/cli/src/commands/workflow/workflow-status.command.ts (1)
WorkflowStatusCommand(26-339)
packages/tm-core/src/task-master-core.ts (1)
packages/tm-core/src/workflow/workflow-service.ts (2)
WorkflowServiceConfig(14-27)WorkflowService(32-218)
packages/workflow-engine/src/worktree/worktree-manager.ts (3)
packages/workflow-engine/src/types/workflow.types.ts (1)
WorktreeInfo(104-119)packages/workflow-engine/src/errors/workflow.errors.ts (1)
WorktreeError(19-24)packages/workflow-engine/src/task-execution/task-execution-manager.ts (1)
cleanup(342-351)
packages/tm-core/src/workflow/workflow-service.ts (4)
packages/workflow-engine/src/index.ts (1)
TaskExecutionManager(19-19)packages/workflow-engine/src/task-execution/task-execution-manager.ts (2)
TaskExecutionManager(43-433)TaskExecutionManagerConfig(34-37)packages/tm-core/src/task-master-core.ts (1)
getTask(150-152)packages/workflow-engine/src/types/workflow.types.ts (1)
WorkflowExecutionContext(19-44)
scripts/modules/commands.js (2)
apps/cli/src/commands/index.ts (1)
registerAllCommands(21-38)apps/cli/src/index.ts (1)
registerAllCommands(13-13)
apps/cli/src/utils/ui.ts (3)
scripts/modules/commands.js (1)
message(5329-5338)tests/unit/ui.test.js (1)
chalk(17-17)scripts/modules/task-manager/tag-management.js (1)
headers(612-612)
apps/cli/src/commands/workflow/workflow-start.command.ts (4)
packages/tm-core/src/index.ts (2)
TaskMasterCore(8-8)createTaskMasterCore(9-9)packages/tm-core/src/task-master-core.ts (3)
TaskMasterCore(43-223)workflow(210-212)createTaskMasterCore(228-232)packages/workflow-engine/src/index.ts (1)
TaskExecutionManager(19-19)packages/workflow-engine/src/task-execution/task-execution-manager.ts (2)
TaskExecutionManager(43-433)TaskExecutionManagerConfig(34-37)
apps/cli/src/commands/workflow/workflow-stop.command.ts (2)
packages/workflow-engine/src/index.ts (1)
TaskExecutionManager(19-19)packages/workflow-engine/src/task-execution/task-execution-manager.ts (2)
TaskExecutionManager(43-433)TaskExecutionManagerConfig(34-37)
apps/cli/src/commands/workflow/workflow-status.command.ts (2)
packages/workflow-engine/src/index.ts (1)
TaskExecutionManager(19-19)packages/workflow-engine/src/task-execution/task-execution-manager.ts (2)
TaskExecutionManager(43-433)TaskExecutionManagerConfig(34-37)
packages/workflow-engine/src/state/workflow-state-manager.ts (2)
packages/workflow-engine/src/types/workflow.types.ts (3)
WorkflowStatus(46-54)WorkflowExecutionContext(19-44)WorkflowEvent(56-63)packages/workflow-engine/src/errors/workflow.errors.ts (1)
WorkflowError(6-17)
🪛 GitHub Actions: CI
packages/workflow-engine/src/index.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/workflow-engine/src/worktree/index.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/workflow-engine/tsconfig.json
[error] 1-1: Biome format check failed. Detected formatting differences in this file. Run 'biome format .' to fix. (Triggered by 'npm run format-check').
packages/workflow-engine/src/types/index.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/workflow-engine/src/process/index.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/workflow-engine/src/types/workflow.types.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/workflow-engine/src/state/index.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
apps/cli/src/commands/workflow/index.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/workflow-engine/src/task-execution/index.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/tm-core/src/index.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
apps/cli/src/commands/workflow/workflow-list.command.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/workflow-engine/src/errors/workflow.errors.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/tm-core/src/workflow/index.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/workflow-engine/src/errors/index.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/tm-core/src/workflow/workflow-service.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
apps/cli/src/utils/ui.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
apps/cli/src/commands/workflow/workflow-start.command.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
apps/cli/src/commands/workflow/workflow-status.command.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
packages/workflow-engine/src/state/workflow-state-manager.ts
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🪛 markdownlint-cli2 (0.17.2)
packages/workflow-engine/README.md
17-17: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
319-319: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
328-328: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
337-337: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
371-371: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (22)
packages/workflow-engine/vitest.config.ts (2)
1-19: Vitest config looks good.Node env, globals, and coverage reporters are reasonable defaults.
1-19: No action needed — Vitest scripts present.
packages/workflow-engine/package.json already defines "test": "vitest" and "test:watch": "vitest --watch".packages/workflow-engine/tsup.config.ts (1)
4-10: Do not add 'src/workflow/index.ts' — file is missing.
packages/workflow-engine/src/workflow/index.ts is not present; either create that file if you intend to publish it, or do not include it in packages/workflow-engine/tsup.config.ts.Likely an incorrect or invalid review comment.
package.json (1)
14-43: Scripts update aligns with testing guidelines.test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci are present.
packages/tm-core/tsup.config.ts (1)
17-49: Overall config changes LGTM.Build-time TM_PUBLIC_* injection, entries (including workflow), and dual formats are solid.
packages/workflow-engine/src/task-execution/index.ts (1)
1-6: Run Biome formatter and confirm TS moduleResolution
- packages/workflow-engine/src/task-execution/index.ts already ends with a newline; no EOF-newline change required — run Biome across the repo to pick up any remaining formatting failures.
- tsconfig: root tsconfig.json and packages/build-config/tsconfig.json (and others) set compilerOptions.moduleResolution = "bundler" (module = "ESNext"), but packages/workflow-engine/tsconfig.json does not set these keys — confirm it extends a base tsconfig that provides "bundler" or explicitly add compilerOptions.moduleResolution = "bundler" (or "nodenext") so the .js import extension resolves correctly.
apps/cli/src/index.ts (2)
9-9: No change required — named export present.
apps/cli/src/commands/workflow.command.ts declaresexport class WorkflowCommand ...and alsoexport default WorkflowCommand, so the currentexport { WorkflowCommand } from './commands/workflow.command.js';is valid.Likely an incorrect or invalid review comment.
12-14: Registry export is pure — no top-level registration on import
apps/cli/src/commands/index.ts exports registerAllCommands(program: Command) and contains no top-level program.command(...) calls; registration occurs inside the exported function.packages/tm-core/src/task-master-core.ts (4)
19-22: Workflow integration API surface LGTM.Importing WorkflowService/Config and exposing partial options is consistent and non-breaking.
Also applies to: 30-31
46-47: Member initialization is sound.Private member + factory create() ensures proper init before use.
Also applies to: 69-70
207-213: Accessor reads well.Getter naming and visibility are fine.
219-222: Dispose workflow service on close LGTM.Cleans up listeners and engine; matches lifecycle expectations.
packages/tm-core/package.json (3)
52-54: Utils subpath export addition LGTM.Providing a require target for utils improves interop.
55-60: Workflow subpath export LGTM.Clear pathing for types/import/require.
76-79: Verify dependency intent & pinning
- "@tm/workflow-engine" is set to "" in packages/tm-core/package.json — replace with a workspace range (e.g. "workspace:") or an explicit semver to avoid registry drift/failures.
- "chalk" is actually used (packages/tm-core/src/logger/logger.ts:5 — import chalk from 'chalk') — do not remove unless you also remove/replace the usage.
- "@supabase/supabase-js" (^2.57.0) is imported in packages/tm-core/src/auth/supabase-session-storage.ts, src/storage/api-storage.ts, src/services/organization.service.ts, src/repositories/supabase-task-repository.ts, src/clients/supabase-client.ts — confirm ^2.57.0 is compatible with these auth/storage/client codepaths and run tests.
apps/cli/src/commands/workflow.command.ts (1)
50-54: Registration API LGTM.Static register pattern aligns with the registry.
apps/cli/src/commands/workflow/workflow-stop.command.ts (1)
147-155: Ensure spinner is stopped in all cases.If
ui.displaySpinnerreturns a stop/dispose function, use try/finally to avoid a stuck spinner on failure.If applicable, update like:
- ui.displaySpinner('Stopping workflow and cleaning up resources...'); - - await this.workflowManager!.stopTaskExecution( - idToStop, - options.force - ); + const stop = ui.displaySpinner('Stopping workflow and cleaning up resources...'); + try { + await this.workflowManager!.stopTaskExecution(idToStop, options.force); + } finally { + stop?.(); + }And similarly for "Stopping all workflows...".
Also applies to: 198-201
apps/cli/src/commands/workflow/workflow-list.command.ts (1)
1-5: CI failing on Biome format.Run npm run format locally to unblock CI. If issues persist, I can auto-apply fixes.
apps/cli/src/commands/workflow/workflow-start.command.ts (1)
141-148: Confirm timeout units.defaultTimeout in engine vs per-run timeout here: both appear to be minutes. Verify engine expects minutes to avoid premature/overlong timeouts.
packages/workflow-engine/package.json (2)
8-29: TS export types mapping: ensure tsconfig uses NodeNext/Node16.Using "exports" with "types" entries requires TS NodeNext/Node16 moduleResolution. Verify repo tsconfig; otherwise types for subpaths may not resolve.
37-39: No runtime circular dependency detected — only type-only imports are used.workflow-engine only imports types from @tm/core (
import type { Task } from '@tm/core'in packages/workflow-engine/src/task-execution/task-execution-manager.ts:130). tm-core's references to @tm/workflow-engine are type-only (packages/tm-core/src/workflow/index.ts:85; packages/tm-core/src/workflow/workflow-service.ts:93).packages/workflow-engine/src/worktree/worktree-manager.ts (1)
6-8: Keep 'node:' scheme imports (match repo style)Repo predominantly uses 'node:' scheme imports (30 occurrences) vs bare core imports (10); leave packages/workflow-engine/src/worktree/worktree-manager.ts lines 6–8 unchanged.
Likely an incorrect or invalid review comment.
| }, | ||
| "dependencies": { | ||
| "@tm/core": "*", | ||
| "@tm/workflow-engine": "*", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use workspace protocol for internal dep instead of "*"
"" makes builds non-reproducible. If you’re using pnpm/yarn workspaces, prefer workspace: for local linking.
Apply:
- "@tm/workflow-engine": "*",
+ "@tm/workflow-engine": "workspace:*",📝 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.
| "@tm/workflow-engine": "*", | |
| "@tm/workflow-engine": "workspace:*", |
🤖 Prompt for AI Agents
In apps/cli/package.json around line 31, the dependency version uses "*" which
makes builds non-reproducible; change the version to the workspace protocol by
replacing "*" with "workspace:*" for the internal package "@tm/workflow-engine".
Update the package.json dependency entry accordingly and run your package
manager install (pnpm/yarn) to verify the workspace link resolves.
| export function registerAllCommands(program: Command): void { | ||
| // Get all exports from this module | ||
| const commands = [ | ||
| ListTasksCommand, | ||
| AuthCommand, | ||
| WorkflowCommand | ||
| // Add new commands here as they're imported above | ||
| ]; | ||
|
|
||
| commands.forEach((CommandClass) => { | ||
| if ( | ||
| 'register' in CommandClass && | ||
| typeof CommandClass.register === 'function' | ||
| ) { | ||
| CommandClass.register(program); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Registry mechanism LGTM; tweak comment for clarity.
The comment says “Get all exports from this module” but the list is explicit. Consider rewording to avoid confusion.
- // Get all exports from this module
+ // Register explicitly listed command classes📝 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.
| export function registerAllCommands(program: Command): void { | |
| // Get all exports from this module | |
| const commands = [ | |
| ListTasksCommand, | |
| AuthCommand, | |
| WorkflowCommand | |
| // Add new commands here as they're imported above | |
| ]; | |
| commands.forEach((CommandClass) => { | |
| if ( | |
| 'register' in CommandClass && | |
| typeof CommandClass.register === 'function' | |
| ) { | |
| CommandClass.register(program); | |
| } | |
| }); | |
| } | |
| export function registerAllCommands(program: Command): void { | |
| // Register explicitly listed command classes | |
| const commands = [ | |
| ListTasksCommand, | |
| AuthCommand, | |
| WorkflowCommand | |
| // Add new commands here as they're imported above | |
| ]; | |
| commands.forEach((CommandClass) => { | |
| if ( | |
| 'register' in CommandClass && | |
| typeof CommandClass.register === 'function' | |
| ) { | |
| CommandClass.register(program); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/cli/src/commands/index.ts around lines 21 to 38, the inline comment "Get
all exports from this module" is misleading because the commands array is
explicit; change the comment to accurately reflect that this is an explicit list
of imported commands (e.g., "List of command classes to register" or "Explicitly
registered commands") so readers won't expect dynamic export collection; update
the comment text accordingly.
| private addSubcommands(): void { | ||
| // Start workflow | ||
| WorkflowStartCommand.register(this); | ||
|
|
||
| // List workflows | ||
| WorkflowListCommand.register(this); | ||
|
|
||
| // Stop workflow | ||
| WorkflowStopCommand.register(this); | ||
|
|
||
| // Show workflow status | ||
| WorkflowStatusCommand.register(this); | ||
|
|
||
| // Alias commands for convenience | ||
| this.addCommand(new WorkflowStartCommand('run')); // tm workflow run <task-id> | ||
| this.addCommand(new WorkflowStopCommand('kill')); // tm workflow kill <workflow-id> | ||
| this.addCommand(new WorkflowStatusCommand('info')); // tm workflow info <workflow-id> | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Route subcommands through tm-core WorkflowService (avoid direct engine usage).
Start/list/stop/status currently instantiate TaskExecutionManager directly (see subcommand files). Prefer tmCore.workflow to keep a single abstraction and future-proof engine internals.
I can provide diffs for workflow-*.command.ts to use:
- const core = await createTaskMasterCore({ projectPath: projectRoot });
- const workflow = core.workflow;
- Replace engine calls (startTaskExecution/stopTaskExecution/etc.) with workflow.start()/stop()/getStatus()/list().
Would you like me to generate those changes?
Fix Chalk usage in subcommands.
workflow-list.command.ts and workflow-status.command.ts use chalk.orange which doesn’t exist in Chalk v5. Use chalk.hex('#FFA500') or chalk.keyword('orange').
Example:
- paused: { icon: '⏸️', color: chalk.orange },
+ paused: { icon: '⏸️', color: chalk.hex('#FFA500') },Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/cli/src/commands/workflow.command.ts around lines 28 to 45, the review
flags Chalk v5 usage in related subcommand files: update
workflow-list.command.ts and workflow-status.command.ts to stop using the
nonexistent chalk.orange; replace any chalk.orange occurrences with
chalk.hex('#FFA500') or chalk.keyword('orange') (choose one consistent style),
import chalk if missing, and run a quick build/lint to ensure types match and no
remaining chalk.orange usages remain.
| /** | ||
| * @fileoverview Workflow Commands | ||
| * Exports for all workflow-related CLI commands | ||
| */ | ||
|
|
||
| export * from './workflow-start.command.js'; | ||
| export * from './workflow-list.command.js'; | ||
| export * from './workflow-stop.command.js'; | ||
| export * from './workflow-status.command.js'; No newline at end of file |
There was a problem hiding this comment.
Format to satisfy Biome.
Run the formatter; CI shows a Biome format failure on this file.
🧰 Tools
🪛 GitHub Actions: CI
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🤖 Prompt for AI Agents
In apps/cli/src/commands/workflow/index.ts lines 1-9 the file fails Biome
formatting in CI; run the project formatter to fix whitespace/format issues.
Update the file by running the project's Biome formatter (e.g., biome format) or
the repo's formatting npm script (npm run format) so the file is reformatted to
match the code style, then stage and commit the formatted file.
|
|
||
| import { Command } from 'commander'; | ||
| import chalk from 'chalk'; | ||
| import path from 'node:path'; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use bare Node core import to match maintainer preference.
Per team preference, switch from node: scheme to bare module name.
-import path from 'node:path';
+import path from 'path';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import path from 'node:path'; | |
| import path from 'path'; |
🧰 Tools
🪛 GitHub Actions: CI
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🤖 Prompt for AI Agents
In apps/cli/src/commands/workflow/workflow-list.command.ts around line 8, the
import uses the node: scheme ("node:path"); change it to the bare core module
name ("path") to match project convention — update the import declaration
accordingly and run a quick lint/tsc to ensure no other references rely on the
node: prefix.
| { | ||
| "extends": "../../tsconfig.json", | ||
| "compilerOptions": { | ||
| "outDir": "dist", | ||
| "rootDir": "src", | ||
| "declaration": true, | ||
| "declarationMap": true, | ||
| "sourceMap": true | ||
| }, | ||
| "include": [ | ||
| "src/**/*" | ||
| ], | ||
| "exclude": [ | ||
| "dist", | ||
| "node_modules", | ||
| "**/*.test.ts", | ||
| "**/*.spec.ts" | ||
| ] | ||
| } No newline at end of file |
There was a problem hiding this comment.
Fix Biome formatting to unblock CI.
CI fails on this file. Run: npm run format.
🧰 Tools
🪛 GitHub Actions: CI
[error] 1-1: Biome format check failed. Detected formatting differences in this file. Run 'biome format .' to fix. (Triggered by 'npm run format-check').
🤖 Prompt for AI Agents
packages/workflow-engine/tsconfig.json lines 1-19: the file is misformatted
causing CI to fail; run the project's formatter to apply Biome formatting rules
(execute npm run format from the repo root), then stage and commit the
reformatted tsconfig.json so CI picks up the corrected formatting.
| "compilerOptions": { | ||
| "outDir": "dist", | ||
| "rootDir": "src", | ||
| "declaration": true, | ||
| "declarationMap": true, | ||
| "sourceMap": true | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optional: add tsbuildinfo and composite for faster, safer incremental builds.
This keeps TS build artifacts package-local and enables project references.
"compilerOptions": {
"outDir": "dist",
"rootDir": "src",
"declaration": true,
"declarationMap": true,
- "sourceMap": true
+ "sourceMap": true,
+ "composite": true,
+ "tsBuildInfoFile": "dist/.tsbuildinfo"
},📝 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.
| "compilerOptions": { | |
| "outDir": "dist", | |
| "rootDir": "src", | |
| "declaration": true, | |
| "declarationMap": true, | |
| "sourceMap": true | |
| }, | |
| "compilerOptions": { | |
| "outDir": "dist", | |
| "rootDir": "src", | |
| "declaration": true, | |
| "declarationMap": true, | |
| "sourceMap": true, | |
| "composite": true, | |
| "tsBuildInfoFile": "dist/.tsbuildinfo" | |
| }, |
🤖 Prompt for AI Agents
In packages/workflow-engine/tsconfig.json around lines 3 to 9, enable
project-reference friendly incremental builds by setting "composite": true and
"incremental": true in compilerOptions and add a package-local tsBuildInfo file
(e.g. "tsBuildInfoFile": "./.tsbuildinfo"); ensure "declaration" and
"declarationMap" remain enabled (required by composite) so the project can be
safely used in project references and produce local build artifacts.
| format: ['esm'], | ||
| dts: true, | ||
| sourcemap: true, | ||
| clean: true, | ||
| splitting: false, | ||
| treeshake: true | ||
| }); No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider externalizing node_modules to keep the library lean.
Prevents bundling deps and aligns with tm-core tsup config.
treeshake: true
-});
+ ,
+ external: [/^[^./]/]
+});📝 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.
| format: ['esm'], | |
| dts: true, | |
| sourcemap: true, | |
| clean: true, | |
| splitting: false, | |
| treeshake: true | |
| }); | |
| format: ['esm'], | |
| dts: true, | |
| sourcemap: true, | |
| clean: true, | |
| splitting: false, | |
| treeshake: true, | |
| external: [/^[^./]/] | |
| }); |
🤖 Prompt for AI Agents
In packages/workflow-engine/tsup.config.ts around lines 11 to 17, the config
currently bundles node_modules which inflates the library; modify the tsup
config to externalize dependencies (and peerDependencies) from package.json so
node_modules are not bundled—read package.json deps/peerDeps and set tsup's
external option (or use an array of package names) to exclude them, keeping
existing options (format, dts, sourcemap, clean, splitting, treeshake) intact.
| // Import command registry from @tm/cli | ||
| import { registerAllCommands } from '@tm/cli'; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid top-level import of @tm/cli; use dynamic import to prevent init-order/circular issues.
Guidelines recommend dynamic imports in modules. Also ensures graceful behavior if @tm/cli isn’t present.
-import { registerAllCommands } from '@tm/cli';
+// Defer loading the CLI registry to avoid init-order issues
+let __registerAllCommands;
+async function __loadCliRegistry() {
+ if (!__registerAllCommands) {
+ __registerAllCommands = (await import('@tm/cli')).registerAllCommands;
+ }
+ return __registerAllCommands;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Import command registry from @tm/cli | |
| import { registerAllCommands } from '@tm/cli'; | |
| // Import command registry from @tm/cli | |
| // Defer loading the CLI registry to avoid init-order issues | |
| let __registerAllCommands; | |
| async function __loadCliRegistry() { | |
| if (!__registerAllCommands) { | |
| __registerAllCommands = (await import('@tm/cli')).registerAllCommands; | |
| } | |
| return __registerAllCommands; | |
| } |
🤖 Prompt for AI Agents
In scripts/modules/commands.js around lines 18 to 20, replace the top-level
static import of @tm/cli with a dynamic import to avoid init-order and circular
dependency issues: remove the static import and instead load the module at
runtime using import('@tm/cli') inside an async function or initialization path,
wrap the dynamic import in try/catch to handle the module not being present and
log or no-op gracefully, and then call registerAllCommands from the resolved
module (or export an async initializer that performs this) so nothing depends on
@tm/cli at module-evaluation time.
💡 Verification agent
🧩 Analysis chain
Verify dependency and export wiring for @tm/cli.
Ensure the package that ships scripts/modules declares @tm/cli as a dependency and that apps/cli/src/index.ts re-exports registerAllCommands.
Also applies to: 1740-1743
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check that the root package (hosting scripts/) depends on @tm/cli
jq -r '.name,.dependencies["@tm/cli"],.devDependencies["@tm/cli"]' package.json
# Verify export exists
rg -n "export\\s+\\{?\\s*registerAllCommands\\s*\\}?\\s+from" apps/cli/src/index.ts || rg -n "export\\s+(const|function)\\s+registerAllCommands" apps/cli/src/index.tsLength of output: 86
Add @tm/cli to package.json dependencies
Root package "task-master-ai" does not list @tm/cli in package.json, while apps/cli/src/index.ts already re-exports registerAllCommands (apps/cli/src/index.ts:13). Add @tm/cli to the package that ships scripts/modules (root package.json or as a workspace dependency) so the import resolves at build/publish time.
🤖 Prompt for AI Agents
In scripts/modules/commands.js around lines 18 to 20, the import from '@tm/cli'
will fail because the root package.json for task-master-ai does not list @tm/cli
as a dependency; add "@tm/cli" to the package.json dependencies (or as a
workspace dependency) for the package that ships scripts/modules, pinning the
version to the local workspace package (e.g. "workspace:*" or the matching
version from apps/cli) or an appropriate published version, then run your
package manager install to update lockfiles and verify the import resolves at
build/publish time.
| // Register all commands from @tm/cli using the command registry | ||
| // This automatically registers ListTasksCommand, AuthCommand, and any future commands | ||
| registerAllCommands(programInstance); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Guard registry call and surface actionable error if @tm/cli is missing.
Prevents runtime crash with a clearer message.
-// This automatically registers ListTasksCommand, AuthCommand, and any future commands
-registerAllCommands(programInstance);
+// This automatically registers ListTasksCommand, AuthCommand, and any future commands
+try {
+ const registerAllCommands = await __loadCliRegistry();
+ registerAllCommands(programInstance);
+} catch (e) {
+ console.error(chalk.red('Failed to load @tm/cli command registry.'));
+ console.error(chalk.yellow('Ensure @tm/cli is installed and exported correctly.'));
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/modules/commands.js around lines 1740 to 1743, the call
registerAllCommands(programInstance) can throw if the @tm/cli module (or the
registry exporter) is missing; wrap the import/registry call in a try/catch and
detect the specific module-not-found error (or absence of registerAllCommands),
then log or throw an actionable error message instructing the operator to
install @tm/cli (e.g., "Missing dependency '@tm/cli'. Run `npm install @tm/cli`
or check your build."), and bail gracefully (return/exit) instead of allowing an
uncaught runtime crash. Ensure the guard handles both require/import failure and
a falsy/missing export before invoking registerAllCommands.
| private getStatusDisplay(status: string): string { | ||
| const statusMap = { | ||
| pending: { icon: '⏳', color: chalk.yellow }, | ||
| initializing: { icon: '🔄', color: chalk.blue }, | ||
| running: { icon: '🚀', color: chalk.green }, | ||
| paused: { icon: '⏸️', color: chalk.orange }, | ||
| completed: { icon: '✅', color: chalk.green }, | ||
| failed: { icon: '❌', color: chalk.red }, | ||
| cancelled: { icon: '🛑', color: chalk.gray }, | ||
| timeout: { icon: '⏰', color: chalk.red } | ||
| }; | ||
|
|
||
| const statusInfo = statusMap[status as keyof typeof statusMap] || { icon: '❓', color: chalk.white }; | ||
| return `${statusInfo.icon} ${statusInfo.color(status)}`; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix invalid Chalk color (chalk.orange is not a function).
Chalk has no orange method; this will throw at runtime. Use hex or reuse yellow.
private getStatusDisplay(status: string): string {
const statusMap = {
pending: { icon: '⏳', color: chalk.yellow },
initializing: { icon: '🔄', color: chalk.blue },
running: { icon: '🚀', color: chalk.green },
- paused: { icon: '⏸️', color: chalk.orange },
+ paused: { icon: '⏸️', color: chalk.hex('#FFA500') },
completed: { icon: '✅', color: chalk.green },
failed: { icon: '❌', color: chalk.red },
cancelled: { icon: '🛑', color: chalk.gray },
timeout: { icon: '⏰', color: chalk.red }
};📝 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.
| private getStatusDisplay(status: string): string { | |
| const statusMap = { | |
| pending: { icon: '⏳', color: chalk.yellow }, | |
| initializing: { icon: '🔄', color: chalk.blue }, | |
| running: { icon: '🚀', color: chalk.green }, | |
| paused: { icon: '⏸️', color: chalk.orange }, | |
| completed: { icon: '✅', color: chalk.green }, | |
| failed: { icon: '❌', color: chalk.red }, | |
| cancelled: { icon: '🛑', color: chalk.gray }, | |
| timeout: { icon: '⏰', color: chalk.red } | |
| }; | |
| const statusInfo = statusMap[status as keyof typeof statusMap] || { icon: '❓', color: chalk.white }; | |
| return `${statusInfo.icon} ${statusInfo.color(status)}`; | |
| } | |
| private getStatusDisplay(status: string): string { | |
| const statusMap = { | |
| pending: { icon: '⏳', color: chalk.yellow }, | |
| initializing: { icon: '🔄', color: chalk.blue }, | |
| running: { icon: '🚀', color: chalk.green }, | |
| paused: { icon: '⏸️', color: chalk.hex('#FFA500') }, | |
| completed: { icon: '✅', color: chalk.green }, | |
| failed: { icon: '❌', color: chalk.red }, | |
| cancelled: { icon: '🛑', color: chalk.gray }, | |
| timeout: { icon: '⏰', color: chalk.red } | |
| }; | |
| const statusInfo = statusMap[status as keyof typeof statusMap] || { icon: '❓', color: chalk.white }; | |
| return `${statusInfo.icon} ${statusInfo.color(status)}`; | |
| } |
🧰 Tools
🪛 GitHub Actions: CI
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🤖 Prompt for AI Agents
In apps/cli/src/commands/workflow/workflow-list.command.ts around lines 202 to
216, the status map uses chalk.orange which doesn't exist and will throw at
runtime; replace chalk.orange with a valid Chalk call (e.g.,
chalk.keyword('orange') or chalk.hex('#FFA500')) or use an existing color like
chalk.yellow, updating the paused entry to use that function so the returned
status string calls a real Chalk color function.
| private formatDuration(start: Date, end: Date): string { | ||
| const diff = end.getTime() - start.getTime(); | ||
| const minutes = Math.floor(diff / (1000 * 60)); | ||
| const hours = Math.floor(minutes / 60); | ||
|
|
||
| if (hours > 0) { | ||
| return `${hours}h ${minutes % 60}m`; | ||
| } else if (minutes > 0) { | ||
| return `${minutes}m`; | ||
| } else { | ||
| return '<1m'; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Defensive date handling to avoid crashes if dates are strings.
State may revive dates as strings. Coerce to Date before math.
- private formatDuration(start: Date, end: Date): string {
- const diff = end.getTime() - start.getTime();
+ private formatDuration(start: Date | string, end: Date | string): string {
+ const s = start instanceof Date ? start : new Date(start);
+ const e = end instanceof Date ? end : new Date(end);
+ const diff = e.getTime() - s.getTime();📝 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.
| private formatDuration(start: Date, end: Date): string { | |
| const diff = end.getTime() - start.getTime(); | |
| const minutes = Math.floor(diff / (1000 * 60)); | |
| const hours = Math.floor(minutes / 60); | |
| if (hours > 0) { | |
| return `${hours}h ${minutes % 60}m`; | |
| } else if (minutes > 0) { | |
| return `${minutes}m`; | |
| } else { | |
| return '<1m'; | |
| } | |
| } | |
| private formatDuration(start: Date | string, end: Date | string): string { | |
| const s = start instanceof Date ? start : new Date(start); | |
| const e = end instanceof Date ? end : new Date(end); | |
| const diff = e.getTime() - s.getTime(); | |
| const minutes = Math.floor(diff / (1000 * 60)); | |
| const hours = Math.floor(minutes / 60); | |
| if (hours > 0) { | |
| return `${hours}h ${minutes % 60}m`; | |
| } else if (minutes > 0) { | |
| return `${minutes}m`; | |
| } else { | |
| return '<1m'; | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: CI
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🤖 Prompt for AI Agents
In apps/cli/src/commands/workflow/workflow-list.command.ts around lines 228 to
240, the formatDuration function assumes start and end are Date objects and will
crash if they are strings; convert/coerce start and end to Date instances (e.g.,
new Date(start)) before doing getTime(), validate that resulting Dates are valid
(isNaN(date.getTime()) check) and handle invalid/missing dates by returning a
safe fallback like '<invalid>' or '<unknown>' so the function never throws.
|
|
||
| import { Command } from 'commander'; | ||
| import chalk from 'chalk'; | ||
| import path from 'node:path'; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use bare Node core import to match maintainer preference.
Align with team preference.
-import path from 'node:path';
+import path from 'path';🧰 Tools
🪛 GitHub Actions: CI
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🤖 Prompt for AI Agents
In apps/cli/src/commands/workflow/workflow-start.command.ts around line 8, the
import uses the prefixed core module "node:path"; change it to the bare Node
core import "path" to match project style and maintainer preference (replace
"import path from 'node:path';" with "import path from 'path';").
| export interface WorkflowStartOptions { | ||
| project?: string; | ||
| branch?: string; | ||
| timeout?: number; | ||
| worktreeBase?: string; | ||
| claude?: string; | ||
| debug?: boolean; | ||
| env?: string; | ||
| } |
There was a problem hiding this comment.
Timeout option typing is inconsistent with Commander.
Commander provides strings; interface says number. Fix typing to string and keep parseInt at use sites.
export interface WorkflowStartOptions {
project?: string;
branch?: string;
- timeout?: number;
+ timeout?: string;
worktreeBase?: string;
claude?: string;
debug?: boolean;
env?: string;
}Also applies to: 43-44, 95-97, 141-148
🧰 Tools
🪛 GitHub Actions: CI
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🤖 Prompt for AI Agents
In apps/cli/src/commands/workflow/workflow-start.command.ts around lines 19-27
(and also adjust usages at 43-44, 95-97, 141-148), the
WorkflowStartOptions.timeout is declared as number but Commander passes option
values as strings; change the timeout type to string in the interface and at all
related option declarations/usages, and update code sites that need numeric
timeout to call parseInt (or Number) where the value is consumed so runtime
behavior remains the same.
| ui.displaySpinner('Creating worktree and starting Claude Code process...'); | ||
|
|
||
| const workflowId = await this.workflowManager!.startTaskExecution(task, { | ||
| branchName: options.branch, | ||
| timeout: parseInt(options.timeout || '60'), | ||
| env | ||
| }); | ||
|
|
||
| const workflow = this.workflowManager!.getWorkflowStatus(workflowId); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Ensure spinner stops on both success and error paths.
If ui.displaySpinner() starts a persistent spinner, make sure it is stopped/cleared before printing success lines and in the catch block.
I can adapt this to a withSpinner helper if you share ui utils.
Also applies to: 101-107
🧰 Tools
🪛 GitHub Actions: CI
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🤖 Prompt for AI Agents
In apps/cli/src/commands/workflow/workflow-start.command.ts around lines 91-99
(and similarly 101-107), the persistent spinner started by
ui.displaySpinner(...) is not guaranteed to be stopped on either the success or
error path; wrap the workflow start and subsequent status handling in a
try/finally (or use a withSpinner helper) so that ui.stopSpinner() (or the
appropriate ui.clearSpinner()) is called in the finally block before any console
output, and also call it in the catch path if you keep separate error handling,
ensuring the spinner is stopped prior to printing success or error messages.
| for (const workflow of workflows) { | ||
| const isStale = | ||
| workflow.status === 'running' && | ||
| !this.processSandbox.isProcessRunning(`workflow-${workflow.taskId}`); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix incorrect workflow ID in stale workflow detection
The isProcessRunning check uses workflow-${workflow.taskId} but the actual workflow IDs are generated differently by generateWorkflowId() in the state manager (format: workflow-${taskId}-${timestamp}-${random}).
Let me verify the workflow ID format:
Based on the state manager implementation, the fix should be:
const isStale =
workflow.status === 'running' &&
- !this.processSandbox.isProcessRunning(`workflow-${workflow.taskId}`);
+ !this.processSandbox.isProcessRunning(workflowId);You'll also need to iterate over workflows with their IDs. Consider refactoring the state manager to return a Map or array of tuples instead of just the contexts.
🏁 Script executed:
#!/bin/bash
# Check the generateWorkflowId implementation in the state manager
ast-grep --pattern 'generateWorkflowId($_) {
$$$
}'Length of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo ">>> Searching for generateWorkflowId occurrences"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore -S "generateWorkflowId" -C3 || true
rg -n --hidden --no-ignore -S "generateWorkflowId\(" -C3 || true
else
grep -RIn "generateWorkflowId" . || true
fi
echo
echo ">>> Searching for workflowId variable/usages"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore -S "workflowId\b" -C3 || true
else
grep -RIn "workflowId" . || true
fi
echo
echo ">>> Searching for isProcessRunning calls"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore -S "isProcessRunning\(" -C3 || true
else
grep -RIn "isProcessRunning(" . || true
fi
echo
echo ">>> Searching for 'workflow-' string/template usages (shows nearby context)"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore -S "workflow-" -C1 --max-columns=200 || true
else
grep -RIn --line-number "workflow-" . || true
fi
echo
echo ">>> Attempting to locate and print task-execution-manager file (first match)"
if command -v rg >/dev/null 2>&1; then
file=$(rg --files | rg -i "task-execution-manager" | head -n1 || true)
else
file=$(find . -type f -iname "*task-execution-manager*.ts" | head -n1 || true)
fi
if [ -n "$file" ]; then
echo "Found: $file"
echo "----- $file (lines ~360-410) -----"
sed -n '360,410p' "$file" || true
else
echo "task-execution-manager file not found by name search"
fiLength of output: 103989
Fix stale-workflow detection — use actual workflow IDs (not workflow-${taskId})
generateWorkflowId() returns workflow-${taskId}-${timestamp}-${random} (packages/workflow-engine/src/state/workflow-state-manager.ts:315-318). cleanupStaleWorkflows builds workflow-${workflow.taskId} which will not match active process keys; this will miss/incorrectly mark running processes.
Suggested change (still relevant):
const isStale =
workflow.status === 'running' &&
- !this.processSandbox.isProcessRunning(`workflow-${workflow.taskId}`);
+ !this.processSandbox.isProcessRunning(workflowId);Actions required:
- Iterate workflows with their IDs (don’t rebuild IDs from taskId). Update the loop in packages/workflow-engine/src/task-execution/task-execution-manager.ts (around lines 390–401) to iterate [workflowId, workflow] so you can call isProcessRunning(workflowId).
- Update the subsequent updateWorkflowStatus call to use workflowId (not
workflow-${workflow.taskId}). - Preferably refactor the state manager API to return entries (Map or array of [workflowId, context]) or add a helper (e.g., listWorkflowEntries()) to avoid reconstructing IDs.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/workflow-engine/src/task-execution/task-execution-manager.ts around
lines 390 to 401, the cleanup loop reconstructs process IDs as
`workflow-${workflow.taskId}` which doesn't match actual generated workflow IDs;
change the iteration to use workflow entries (iterate [workflowId, workflow]) so
you can call this.processSandbox.isProcessRunning(workflowId) directly, and pass
workflowId (not the reconstructed string) into updateWorkflowStatus; optionally
adjust the state manager to return Map/entries or add a helper like
listWorkflowEntries() so callers receive [workflowId, context] pairs instead of
rebuilding IDs.
| `workflow-${workflow.taskId}`, | ||
| 'failed' |
There was a problem hiding this comment.
Pass correct workflow ID to updateWorkflowStatus
Similar to the issue above, this uses an incorrect workflow ID format.
The workflow ID should be obtained from the state manager's internal map. You need to modify the iteration to get both the workflow ID and context:
- await this.stateManager.updateWorkflowStatus(
- `workflow-${workflow.taskId}`,
- 'failed'
- );
+ // Need to get the actual workflow ID from state manager
+ // This requires enhancing the state manager APIConsider adding a method to the state manager like listWorkflowsWithIds() that returns an array of { id: string, context: WorkflowExecutionContext }.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
packages/workflow-engine/src/task-execution/task-execution-manager.ts around
lines 400-401: the code currently passes a fabricated workflow ID
(`workflow-${workflow.taskId}`) to updateWorkflowStatus; instead iterate the
state manager map to obtain both the real workflow ID and its
WorkflowExecutionContext (e.g., add or use a method like
stateManager.listWorkflowsWithIds() that returns {id, context}[]), then call
updateWorkflowStatus with the real id and 'failed'; implement
listWorkflowsWithIds() on the state manager if missing and replace the current
iteration to destructure { id, context } so the correct id is used.
| /** | ||
| * @fileoverview Workflow Engine Types | ||
| * Core types for workflow execution system | ||
| */ | ||
|
|
||
| export interface WorkflowConfig { | ||
| /** Maximum number of concurrent workflows */ | ||
| maxConcurrent: number; | ||
| /** Default timeout for workflow execution (minutes) */ | ||
| defaultTimeout: number; | ||
| /** Base directory for worktrees */ | ||
| worktreeBase: string; | ||
| /** Claude Code executable path */ | ||
| claudeExecutable: string; | ||
| /** Enable debug logging */ | ||
| debug: boolean; | ||
| } | ||
|
|
||
| export interface WorkflowExecutionContext { | ||
| /** Task ID being executed */ | ||
| taskId: string; | ||
| /** Task title for display */ | ||
| taskTitle: string; | ||
| /** Full task description */ | ||
| taskDescription: string; | ||
| /** Task implementation details */ | ||
| taskDetails?: string; | ||
| /** Project root path */ | ||
| projectRoot: string; | ||
| /** Worktree path */ | ||
| worktreePath: string; | ||
| /** Branch name for this workflow */ | ||
| branchName: string; | ||
| /** Process ID of running Claude Code */ | ||
| processId?: number; | ||
| /** Workflow start time */ | ||
| startedAt: Date; | ||
| /** Workflow status */ | ||
| status: WorkflowStatus; | ||
| /** Last activity timestamp */ | ||
| lastActivity: Date; | ||
| /** Execution metadata */ | ||
| metadata?: Record<string, any>; | ||
| } | ||
|
|
||
| export type WorkflowStatus = | ||
| | 'pending' // Created but not started | ||
| | 'initializing' // Setting up worktree/process | ||
| | 'running' // Active execution | ||
| | 'paused' // Temporarily stopped | ||
| | 'completed' // Successfully finished | ||
| | 'failed' // Error occurred | ||
| | 'cancelled' // User cancelled | ||
| | 'timeout'; // Exceeded time limit | ||
|
|
||
| export interface WorkflowEvent { | ||
| type: WorkflowEventType; | ||
| workflowId: string; | ||
| taskId: string; | ||
| timestamp: Date; | ||
| data?: any; | ||
| error?: Error; | ||
| } | ||
|
|
||
| export type WorkflowEventType = | ||
| | 'workflow.created' | ||
| | 'workflow.started' | ||
| | 'workflow.paused' | ||
| | 'workflow.resumed' | ||
| | 'workflow.completed' | ||
| | 'workflow.failed' | ||
| | 'workflow.cancelled' | ||
| | 'worktree.created' | ||
| | 'worktree.deleted' | ||
| | 'process.started' | ||
| | 'process.stopped' | ||
| | 'process.output' | ||
| | 'process.error'; | ||
|
|
||
| export interface WorkflowProcess { | ||
| /** Process ID */ | ||
| pid: number; | ||
| /** Command that was executed */ | ||
| command: string; | ||
| /** Command arguments */ | ||
| args: string[]; | ||
| /** Working directory */ | ||
| cwd: string; | ||
| /** Environment variables */ | ||
| env?: Record<string, string>; | ||
| /** Process start time */ | ||
| startedAt: Date; | ||
| /** Process status */ | ||
| status: ProcessStatus; | ||
| } | ||
|
|
||
| export type ProcessStatus = | ||
| | 'starting' | ||
| | 'running' | ||
| | 'stopped' | ||
| | 'crashed' | ||
| | 'killed'; | ||
|
|
||
| export interface WorktreeInfo { | ||
| /** Worktree path */ | ||
| path: string; | ||
| /** Branch name */ | ||
| branch: string; | ||
| /** Creation timestamp */ | ||
| createdAt: Date; | ||
| /** Associated task ID */ | ||
| taskId: string; | ||
| /** Git commit hash */ | ||
| commit?: string; | ||
| /** Worktree lock status */ | ||
| locked: boolean; | ||
| /** Lock reason if applicable */ | ||
| lockReason?: string; | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Type source of truth.
Confirm tm-core re-exports these types rather than defining divergent copies to avoid drift.
If tm-core also defines these, consider re-exporting from a single package (e.g., workflow-engine) and updating imports.
🧰 Tools
🪛 GitHub Actions: CI
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🤖 Prompt for AI Agents
In packages/workflow-engine/src/types/workflow.types.ts lines 1-119, the PR
reviewer asks you to ensure these types are the single source of truth and not
duplicated in tm-core; verify whether tm-core currently defines identical types
and if so remove its duplicate definitions and have tm-core re-export these
types from workflow-engine (or move types into a shared package and update
imports accordingly). Update tm-core imports to re-export or import from the
chosen canonical package, run TypeScript build to catch any path/type errors,
and adjust any import paths across the repo so all references use the single
exported type definitions.
💡 Verification agent
🧩 Analysis chain
CI: Biome format check is failing.
Run formatting to satisfy CI.
🏁 Script executed:
#!/bin/bash
npm run format
npm run format-checkLength of output: 2977
CI: Biome format check failing — invalid biome.json keys
Running npm run format fails: biome.json contains unknown keys (files.ignore at line 3; linter.include and linter.ignore at lines 28–29). Fix biome.json to use the Biome schema (e.g., replace include with includes and remove/replace unsupported ignore entries) and re-run formatting.
🧰 Tools
🪛 GitHub Actions: CI
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🤖 Prompt for AI Agents
In packages/workflow-engine/src/types/workflow.types.ts lines 1–119: CI fails
due to invalid keys in the repo biome.json (files.ignore and
linter.include/linter.ignore) — open the project's biome.json and fix the schema
to match Biome's expected keys: replace any top-level or nested "include" with
"includes", remove unsupported "files.ignore" and "linter.ignore" entries (or
convert them to the supported "files.excludes" / "linter.excludes" key names if
appropriate), and ensure linter configuration uses the correct "linter" object
keys per Biome docs; save and re-run npm run format to confirm the
formatter/linter passes.
| export type WorkflowStatus = | ||
| | 'pending' // Created but not started | ||
| | 'initializing' // Setting up worktree/process | ||
| | 'running' // Active execution | ||
| | 'paused' // Temporarily stopped | ||
| | 'completed' // Successfully finished | ||
| | 'failed' // Error occurred | ||
| | 'cancelled' // User cancelled | ||
| | 'timeout'; // Exceeded time limit | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Export active workflow status set for consistent reuse.
Avoid scattering the active-status list across modules.
export type WorkflowStatus =
| 'pending' // Created but not started
| 'initializing' // Setting up worktree/process
| 'running' // Active execution
| 'paused' // Temporarily stopped
| 'completed' // Successfully finished
| 'failed' // Error occurred
| 'cancelled' // User cancelled
| 'timeout'; // Exceeded time limit
+
+export const ACTIVE_WORKFLOW_STATUSES = ['pending','initializing','running','paused'] as const;
+export type ActiveWorkflowStatus = typeof ACTIVE_WORKFLOW_STATUSES[number];🧰 Tools
🪛 GitHub Actions: CI
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🤖 Prompt for AI Agents
In packages/workflow-engine/src/types/workflow.types.ts around lines 46–55, add
and export a single source-of-truth constant (e.g. ACTIVE_WORKFLOW_STATUSES)
containing the set of statuses considered "active" (pending, initializing,
running, paused) so other modules can import it instead of duplicating the list;
make it a readonly array/tuple and export it from this file for consistent reuse
across the codebase.
| cwd: string; | ||
| /** Environment variables */ | ||
| env?: Record<string, string>; | ||
| /** Process start time */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix env type to match Node’s ProcessEnv.
process.env values can be string | undefined; current type is too narrow.
- env?: Record<string, string>;
+ env?: Record<string, string | undefined>;📝 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.
| cwd: string; | |
| /** Environment variables */ | |
| env?: Record<string, string>; | |
| /** Process start time */ | |
| cwd: string; | |
| /** Environment variables */ | |
| env?: Record<string, string | undefined>; | |
| /** Process start time */ |
🧰 Tools
🪛 GitHub Actions: CI
[error] Biome format check failed: code is not formatted according to Biome rules. Step: 'npm run format-check'.
🤖 Prompt for AI Agents
In packages/workflow-engine/src/types/workflow.types.ts around lines 88 to 91,
the env property is currently typed as Record<string, string> which is too
narrow compared to Node’s process.env; update the type to NodeJS.ProcessEnv (or
Record<string, string | undefined>) so environment variable values may be string
or undefined, and adjust any imports/tsconfig types if needed to ensure NodeJS
namespace is available.
| // Store process references | ||
| this.activeProcesses.set(workflowId, workflowProcess); | ||
| this.childProcesses.set(workflowId, childProcess); | ||
|
|
||
| // Setup process event handlers |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Persist taskId mapping at spawn time.
// Store process references
this.activeProcesses.set(workflowId, workflowProcess);
this.childProcesses.set(workflowId, childProcess);
+this.workflowTaskIds.set(workflowId, taskId);📝 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.
| // Store process references | |
| this.activeProcesses.set(workflowId, workflowProcess); | |
| this.childProcesses.set(workflowId, childProcess); | |
| // Setup process event handlers | |
| // Store process references | |
| this.activeProcesses.set(workflowId, workflowProcess); | |
| this.childProcesses.set(workflowId, childProcess); | |
| this.workflowTaskIds.set(workflowId, taskId); | |
| // Setup process event handlers |
🤖 Prompt for AI Agents
In packages/workflow-engine/src/process/process-sandbox.ts around lines 103 to
107, persist the mapping between the spawned process's taskId and the workflowId
at spawn time. After setting this.activeProcesses and this.childProcesses, add
logic to extract the taskId(s) from the workflowProcess/childProcess and store
taskId -> workflowId in the existing in-memory map (create
this.taskIdToWorkflowId if it doesn't exist). Ensure the mapping is set
synchronously at spawn so later task lookups can resolve the correct workflowId.
| async stopProcess(workflowId: string, force = false): Promise<void> { | ||
| const process = this.activeProcesses.get(workflowId); | ||
| const childProcess = this.childProcesses.get(workflowId); | ||
|
|
||
| if (!process || !childProcess) { | ||
| throw new ProcessError( | ||
| `No running process found for workflow ${workflowId}` | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| // Clear timeout | ||
| const timeout = this.timeouts.get(workflowId); | ||
| if (timeout) { | ||
| clearTimeout(timeout); | ||
| this.timeouts.delete(workflowId); | ||
| } | ||
|
|
||
| // Kill the process | ||
| if (force) { | ||
| childProcess.kill('SIGKILL'); | ||
| } else { | ||
| childProcess.kill('SIGTERM'); | ||
|
|
||
| // Give it 5 seconds to gracefully exit, then force kill | ||
| setTimeout(() => { | ||
| if (!childProcess.killed) { | ||
| childProcess.kill('SIGKILL'); | ||
| } | ||
| }, 5000); | ||
| } | ||
|
|
||
| process.status = 'stopped'; | ||
|
|
||
| // Emit process stopped event | ||
| this.emitEvent('process.stopped', workflowId, process.pid.toString(), { | ||
| pid: process.pid, | ||
| forced: force | ||
| }); | ||
| } catch (error) { |
There was a problem hiding this comment.
Wrong taskId in process.stopped event and duplicate emissions; also return before actual stop.
- You pass pid as taskId (
process.pid.toString()), which breaks consumers. - stopProcess emits process.stopped and the exit handler emits it again.
- Doesn’t wait for actual process exit.
Apply this refactor:
-async stopProcess(workflowId: string, force = false): Promise<void> {
- const process = this.activeProcesses.get(workflowId);
+async stopProcess(workflowId: string, force = false): Promise<void> {
+ const wfProcess = this.activeProcesses.get(workflowId);
const childProcess = this.childProcesses.get(workflowId);
-
- if (!process || !childProcess) {
+ if (!wfProcess || !childProcess) {
throw new ProcessError(
`No running process found for workflow ${workflowId}`
);
}
try {
@@
- if (force) {
+ if (force) {
childProcess.kill('SIGKILL');
} else {
childProcess.kill('SIGTERM');
// Give it 5 seconds to gracefully exit, then force kill
setTimeout(() => {
if (!childProcess.killed) {
childProcess.kill('SIGKILL');
}
}, 5000);
}
-
- process.status = 'stopped';
-
- // Emit process stopped event
- this.emitEvent('process.stopped', workflowId, process.pid.toString(), {
- pid: process.pid,
- forced: force
- });
+ // Wait for actual exit to avoid duplicate emissions and ensure cleanup
+ await new Promise<void>((resolve) => childProcess.once('exit', () => resolve()));
} catch (error) {
throw new ProcessError(
`Failed to stop process for workflow ${workflowId}`,
- process.pid,
+ wfProcess.pid,
error as Error
);
}
}Additionally, store and use the real taskId (see next comments).
📝 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.
| async stopProcess(workflowId: string, force = false): Promise<void> { | |
| const process = this.activeProcesses.get(workflowId); | |
| const childProcess = this.childProcesses.get(workflowId); | |
| if (!process || !childProcess) { | |
| throw new ProcessError( | |
| `No running process found for workflow ${workflowId}` | |
| ); | |
| } | |
| try { | |
| // Clear timeout | |
| const timeout = this.timeouts.get(workflowId); | |
| if (timeout) { | |
| clearTimeout(timeout); | |
| this.timeouts.delete(workflowId); | |
| } | |
| // Kill the process | |
| if (force) { | |
| childProcess.kill('SIGKILL'); | |
| } else { | |
| childProcess.kill('SIGTERM'); | |
| // Give it 5 seconds to gracefully exit, then force kill | |
| setTimeout(() => { | |
| if (!childProcess.killed) { | |
| childProcess.kill('SIGKILL'); | |
| } | |
| }, 5000); | |
| } | |
| process.status = 'stopped'; | |
| // Emit process stopped event | |
| this.emitEvent('process.stopped', workflowId, process.pid.toString(), { | |
| pid: process.pid, | |
| forced: force | |
| }); | |
| } catch (error) { | |
| async stopProcess(workflowId: string, force = false): Promise<void> { | |
| const wfProcess = this.activeProcesses.get(workflowId); | |
| const childProcess = this.childProcesses.get(workflowId); | |
| if (!wfProcess || !childProcess) { | |
| throw new ProcessError( | |
| `No running process found for workflow ${workflowId}` | |
| ); | |
| } | |
| try { | |
| // Clear timeout | |
| const timeout = this.timeouts.get(workflowId); | |
| if (timeout) { | |
| clearTimeout(timeout); | |
| this.timeouts.delete(workflowId); | |
| } | |
| // Kill the process | |
| if (force) { | |
| childProcess.kill('SIGKILL'); | |
| } else { | |
| childProcess.kill('SIGTERM'); | |
| // Give it 5 seconds to gracefully exit, then force kill | |
| setTimeout(() => { | |
| if (!childProcess.killed) { | |
| childProcess.kill('SIGKILL'); | |
| } | |
| }, 5000); | |
| } | |
| // Wait for actual exit to avoid duplicate emissions and ensure cleanup | |
| await new Promise<void>((resolve) => childProcess.once('exit', () => resolve())); | |
| } catch (error) { | |
| throw new ProcessError( | |
| `Failed to stop process for workflow ${workflowId}`, | |
| wfProcess.pid, | |
| error as Error | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/workflow-engine/src/process/process-sandbox.ts around lines 136 to
175, stopProcess currently emits a process.stopped event with the pid as the
taskId, duplicates the event because the child exit handler also emits it, and
returns before the child actually exits; to fix: use the real taskId (retrieve
it from the stored process object or where taskId is tracked) instead of
process.pid.toString() when emitting, remove or guard the local emit so only the
child's 'exit'/'close' handler emits process.stopped (or set a flag to prevent
double-emission), and make stopProcess return a Promise that awaits the child's
exit/close event (with a timeout fallback) before marking status stopped and
resolving; also ensure timeouts and listeners are cleaned up after exit to avoid
leaks.
| try { | ||
| childProcess.stdin?.write(input); | ||
| childProcess.stdin?.write('\n'); | ||
| } catch (error) { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Guard stdin writes.
Avoid writing to closed or non-writable streams.
- childProcess.stdin?.write(input);
- childProcess.stdin?.write('\n');
+ if (!childProcess.stdin || !childProcess.stdin.writable) {
+ throw new ProcessError('Process stdin is not writable', childProcess.pid);
+ }
+ childProcess.stdin.write(input + '\n');📝 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 { | |
| childProcess.stdin?.write(input); | |
| childProcess.stdin?.write('\n'); | |
| } catch (error) { | |
| try { | |
| if (!childProcess.stdin || !childProcess.stdin.writable) { | |
| throw new ProcessError('Process stdin is not writable', childProcess.pid); | |
| } | |
| childProcess.stdin.write(input + '\n'); | |
| } catch (error) { |
🤖 Prompt for AI Agents
In packages/workflow-engine/src/process/process-sandbox.ts around lines 195 to
198, the code writes to childProcess.stdin without verifying the stream is
present and writable; update the block to first check that childProcess.stdin
exists and is writable (e.g., childProcess.stdin && childProcess.stdin.writable
&& !childProcess.stdin.destroyed && !childProcess.stdin.writableEnded) before
calling write, and only perform the writes when those checks pass; keep the
try/catch around the write and otherwise skip or log a warning when stdin is
closed/non-writable.
| private setupProcessHandlers( | ||
| workflowId: string, | ||
| taskId: string, | ||
| childProcess: ChildProcess | ||
| ): void { | ||
| const process = this.activeProcesses.get(workflowId); | ||
| if (!process) return; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid shadowing Node’s global process.
Rename local var to reduce confusion and accidental misuse.
-const process = this.activeProcesses.get(workflowId);
-if (!process) return;
+const wfProcess = this.activeProcesses.get(workflowId);
+if (!wfProcess) return;Apply the rename in this method.
📝 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.
| private setupProcessHandlers( | |
| workflowId: string, | |
| taskId: string, | |
| childProcess: ChildProcess | |
| ): void { | |
| const process = this.activeProcesses.get(workflowId); | |
| if (!process) return; | |
| private setupProcessHandlers( | |
| workflowId: string, | |
| taskId: string, | |
| childProcess: ChildProcess | |
| ): void { | |
| const wfProcess = this.activeProcesses.get(workflowId); | |
| if (!wfProcess) return; |
🤖 Prompt for AI Agents
In packages/workflow-engine/src/process/process-sandbox.ts around lines 252 to
259, the local variable named "process" shadows Node's global process; rename
this local variable (for example to "proc" or "activeProcess") and update every
reference inside this setupProcessHandlers method to use the new name so the
global process object is not accidentally shadowed or misused.
| childProcess.on('exit', (code, signal) => { | ||
| process.status = code === 0 ? 'stopped' : 'crashed'; | ||
|
|
||
| this.emitEvent('process.stopped', workflowId, taskId, { | ||
| pid: process.pid, | ||
| exitCode: code, | ||
| signal | ||
| }); | ||
|
|
||
| // Cleanup | ||
| this.activeProcesses.delete(workflowId); | ||
| this.childProcesses.delete(workflowId); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use correct taskId on exit and apply rename.
- childProcess.on('exit', (code, signal) => {
- process.status = code === 0 ? 'stopped' : 'crashed';
+ childProcess.on('exit', (code, signal) => {
+ wfProcess.status = code === 0 ? 'stopped' : 'crashed';
- this.emitEvent('process.stopped', workflowId, taskId, {
- pid: process.pid,
+ const tId = this.workflowTaskIds.get(workflowId) ?? taskId;
+ this.emitEvent('process.stopped', workflowId, tId, {
+ pid: wfProcess.pid,
exitCode: code,
signal
});
// Cleanup
this.activeProcesses.delete(workflowId);
this.childProcesses.delete(workflowId);
+ this.workflowTaskIds.delete(workflowId);📝 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.
| childProcess.on('exit', (code, signal) => { | |
| process.status = code === 0 ? 'stopped' : 'crashed'; | |
| this.emitEvent('process.stopped', workflowId, taskId, { | |
| pid: process.pid, | |
| exitCode: code, | |
| signal | |
| }); | |
| // Cleanup | |
| this.activeProcesses.delete(workflowId); | |
| this.childProcesses.delete(workflowId); | |
| childProcess.on('exit', (code, signal) => { | |
| wfProcess.status = code === 0 ? 'stopped' : 'crashed'; | |
| const tId = this.workflowTaskIds.get(workflowId) ?? taskId; | |
| this.emitEvent('process.stopped', workflowId, tId, { | |
| pid: wfProcess.pid, | |
| exitCode: code, | |
| signal | |
| }); | |
| // Cleanup | |
| this.activeProcesses.delete(workflowId); | |
| this.childProcesses.delete(workflowId); | |
| this.workflowTaskIds.delete(workflowId); |
🤖 Prompt for AI Agents
In packages/workflow-engine/src/process/process-sandbox.ts around lines 287-299,
the exit handler is using the wrong taskId and the local variable name `process`
shadows the global process; rename the local variable (e.g., to `proc`) and use
the correct task id from the proc object when emitting the event and updating
status. Specifically, change `process` -> `proc` in the handler, set
`proc.status = code === 0 ? 'stopped' : 'crashed'`, call
`this.emitEvent('process.stopped', workflowId, proc.taskId, { pid: proc.pid,
exitCode: code, signal })`, and ensure the same `proc` identifier is used for
cleanup and any other references in this block.
| private async executeGitCommand( | ||
| args: string[], | ||
| options: { cwd: string } | ||
| ): Promise<string> { | ||
| return new Promise((resolve, reject) => { | ||
| const git = spawn('git', args, { | ||
| cwd: options.cwd, | ||
| stdio: ['ignore', 'pipe', 'pipe'] | ||
| }); | ||
|
|
||
| let stdout = ''; | ||
| let stderr = ''; | ||
|
|
||
| git.stdout.on('data', (data) => { | ||
| stdout += data.toString(); | ||
| }); | ||
|
|
||
| git.stderr.on('data', (data) => { | ||
| stderr += data.toString(); | ||
| }); | ||
|
|
||
| git.on('close', (code) => { | ||
| if (code === 0) { | ||
| resolve(stdout); | ||
| } else { | ||
| reject(new Error(`Git command failed (${code}): ${stderr || stdout}`)); | ||
| } | ||
| }); | ||
|
|
||
| git.on('error', (error) => { | ||
| reject(error); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeouts to git calls to avoid indefinite hangs.
Long-running git commands can stall the engine. Introduce an optional timeout.
- private async executeGitCommand(
- args: string[],
- options: { cwd: string }
- ): Promise<string> {
+ private async executeGitCommand(
+ args: string[],
+ options: { cwd: string; timeoutMs?: number }
+ ): Promise<string> {
return new Promise((resolve, reject) => {
const git = spawn('git', args, {
cwd: options.cwd,
stdio: ['ignore', 'pipe', 'pipe']
});
let stdout = '';
let stderr = '';
+ let killedByTimeout = false;
+ const timer =
+ options.timeoutMs && options.timeoutMs > 0
+ ? setTimeout(() => {
+ killedByTimeout = true;
+ git.kill('SIGKILL');
+ }, options.timeoutMs)
+ : undefined;
git.stdout.on('data', (data) => {
stdout += data.toString();
});
git.stderr.on('data', (data) => {
stderr += data.toString();
});
git.on('close', (code) => {
+ if (timer) clearTimeout(timer);
if (code === 0) {
resolve(stdout);
} else {
- reject(new Error(`Git command failed (${code}): ${stderr || stdout}`));
+ const reason = killedByTimeout ? 'timed out' : `exit code ${code}`;
+ reject(new Error(`Git command ${reason}: ${stderr || stdout}`));
}
});
git.on('error', (error) => {
+ if (timer) clearTimeout(timer);
reject(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.
| private async executeGitCommand( | |
| args: string[], | |
| options: { cwd: string } | |
| ): Promise<string> { | |
| return new Promise((resolve, reject) => { | |
| const git = spawn('git', args, { | |
| cwd: options.cwd, | |
| stdio: ['ignore', 'pipe', 'pipe'] | |
| }); | |
| let stdout = ''; | |
| let stderr = ''; | |
| git.stdout.on('data', (data) => { | |
| stdout += data.toString(); | |
| }); | |
| git.stderr.on('data', (data) => { | |
| stderr += data.toString(); | |
| }); | |
| git.on('close', (code) => { | |
| if (code === 0) { | |
| resolve(stdout); | |
| } else { | |
| reject(new Error(`Git command failed (${code}): ${stderr || stdout}`)); | |
| } | |
| }); | |
| git.on('error', (error) => { | |
| reject(error); | |
| }); | |
| }); | |
| } | |
| private async executeGitCommand( | |
| args: string[], | |
| options: { cwd: string; timeoutMs?: number } | |
| ): Promise<string> { | |
| return new Promise((resolve, reject) => { | |
| const git = spawn('git', args, { | |
| cwd: options.cwd, | |
| stdio: ['ignore', 'pipe', 'pipe'] | |
| }); | |
| let stdout = ''; | |
| let stderr = ''; | |
| let killedByTimeout = false; | |
| const timer = | |
| options.timeoutMs && options.timeoutMs > 0 | |
| ? setTimeout(() => { | |
| killedByTimeout = true; | |
| git.kill('SIGKILL'); | |
| }, options.timeoutMs) | |
| : undefined; | |
| git.stdout.on('data', (data) => { | |
| stdout += data.toString(); | |
| }); | |
| git.stderr.on('data', (data) => { | |
| stderr += data.toString(); | |
| }); | |
| git.on('close', (code) => { | |
| if (timer) clearTimeout(timer); | |
| if (code === 0) { | |
| resolve(stdout); | |
| } else { | |
| const reason = killedByTimeout ? 'timed out' : `exit code ${code}`; | |
| reject(new Error(`Git command ${reason}: ${stderr || stdout}`)); | |
| } | |
| }); | |
| git.on('error', (error) => { | |
| if (timer) clearTimeout(timer); | |
| reject(error); | |
| }); | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/workflow-engine/src/worktree/worktree-manager.ts around lines 266 to
299, the executeGitCommand currently can hang indefinitely; add an optional
timeout parameter (e.g. timeoutMs?: number) to the function signature and
implement a timer that, when reached, kills the spawned git process, removes
listeners, clears buffers, and rejects the Promise with a descriptive timeout
Error; ensure the timer is cleared on normal close/error, that process.kill is
used safely (check if process is still running), and listeners for
stdout/stderr/close/error are cleaned up to avoid leaks.
| private async pathExists(path: string): Promise<boolean> { | ||
| try { | ||
| await fs.access(path); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor: avoid shadowing imported 'path' name.
Parameter name path hides the imported path module in this scope. Rename for clarity.
- private async pathExists(path: string): Promise<boolean> {
+ private async pathExists(filePath: string): Promise<boolean> {
try {
- await fs.access(path);
+ await fs.access(filePath);
return true;
} catch {
return false;
}
}📝 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.
| private async pathExists(path: string): Promise<boolean> { | |
| try { | |
| await fs.access(path); | |
| return true; | |
| } catch { | |
| return false; | |
| } | |
| } | |
| private async pathExists(filePath: string): Promise<boolean> { | |
| try { | |
| await fs.access(filePath); | |
| return true; | |
| } catch { | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/workflow-engine/src/worktree/worktree-manager.ts around lines 311 to
318, the method parameter named "path" shadows the imported path module; rename
the parameter (e.g., to "targetPath" or "filePath") and update the method
signature and all internal references within this function accordingly to avoid
the name collision while keeping the return type Promise<boolean> and behavior
unchanged.
| private completeWorktreeInfo(partial: Partial<WorktreeInfo>): WorktreeInfo { | ||
| const branch = partial.branch || 'unknown'; | ||
| const taskIdMatch = branch.match(/^task\/(.+?)-/); | ||
|
|
||
| return { | ||
| path: partial.path || '', | ||
| branch, | ||
| createdAt: partial.createdAt || new Date(), | ||
| taskId: taskIdMatch?.[1] || partial.taskId || 'unknown', | ||
| commit: partial.commit, | ||
| locked: partial.locked || false, | ||
| lockReason: partial.lockReason | ||
| }; |
There was a problem hiding this comment.
Task ID parsing is incorrect for IDs containing dashes.
Regex /^task/(.+?)-/ captures only up to the first dash. With taskId “ABC-123”, you’ll derive “ABC”. This breaks cleanup and lookups.
Apply this diff to derive taskId robustly (prefer last - suffix; fallback to directory name task-):
- private completeWorktreeInfo(partial: Partial<WorktreeInfo>): WorktreeInfo {
- const branch = partial.branch || 'unknown';
- const taskIdMatch = branch.match(/^task\/(.+?)-/);
-
- return {
+ private completeWorktreeInfo(partial: Partial<WorktreeInfo>): WorktreeInfo {
+ const branch = partial.branch || 'unknown';
+ const taskId =
+ (/^task\/(.+)-\d+$/.test(branch)
+ ? branch.replace(/^task\/(.+)-\d+$/, '$1')
+ : (() => {
+ const base = path.basename(partial.path || '');
+ const m = base.match(/^task-(.+)$/);
+ return m?.[1];
+ })()) || partial.taskId || 'unknown';
+ return {
path: partial.path || '',
branch,
createdAt: partial.createdAt || new Date(),
- taskId: taskIdMatch?.[1] || partial.taskId || 'unknown',
+ taskId,
commit: partial.commit,
locked: partial.locked || false,
lockReason: partial.lockReason
};📝 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.
| private completeWorktreeInfo(partial: Partial<WorktreeInfo>): WorktreeInfo { | |
| const branch = partial.branch || 'unknown'; | |
| const taskIdMatch = branch.match(/^task\/(.+?)-/); | |
| return { | |
| path: partial.path || '', | |
| branch, | |
| createdAt: partial.createdAt || new Date(), | |
| taskId: taskIdMatch?.[1] || partial.taskId || 'unknown', | |
| commit: partial.commit, | |
| locked: partial.locked || false, | |
| lockReason: partial.lockReason | |
| }; | |
| private completeWorktreeInfo(partial: Partial<WorktreeInfo>): WorktreeInfo { | |
| const branch = partial.branch || 'unknown'; | |
| const taskId = | |
| (/^task\/(.+)-\d+$/.test(branch) | |
| ? branch.replace(/^task\/(.+)-\d+$/, '$1') | |
| : (() => { | |
| const base = path.basename(partial.path || ''); | |
| const m = base.match(/^task-(.+)$/); | |
| return m?.[1]; | |
| })()) || partial.taskId || 'unknown'; | |
| return { | |
| path: partial.path || '', | |
| branch, | |
| createdAt: partial.createdAt || new Date(), | |
| taskId, | |
| commit: partial.commit, | |
| locked: partial.locked || false, | |
| lockReason: partial.lockReason | |
| }; | |
| } |
🤖 Prompt for AI Agents
packages/workflow-engine/src/worktree/worktree-manager.ts around lines 323 to
335: the current regex /^task\/(.+?)-/ stops at the first dash and truncates IDs
like "ABC-123"; change the parsing to first try matching a final timestamp-style
suffix and capture everything before it (e.g. /^task\/(.+)-\d+$/ to get
"ABC-123" from "task/ABC-123-161800"), if that fails then fallback to extracting
the ID after a "task-" directory token (e.g. find "task-" and take the remainder
up to a slash or end), and only then fall back to partial.taskId or 'unknown';
update the assignment logic to prefer these new parsed values and keep existing
fallbacks for commit/locked/lockReason.
| private setupCleanupHandlers(): void { | ||
| const cleanup = () => { | ||
| console.log('Cleaning up worktrees...'); | ||
| this.cleanupAll(true).catch(console.error); | ||
| }; | ||
|
|
||
| process.on('SIGINT', cleanup); | ||
| process.on('SIGTERM', cleanup); | ||
| process.on('exit', cleanup); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid async work in 'exit' and prevent double cleanup; use once + beforeExit.
process 'exit' won’t await async work; repeated signals can trigger duplicate cleanup. Add a reentrancy guard and switch to once + beforeExit.
- private config: WorktreeManagerConfig;
- private activeWorktrees = new Map<string, WorktreeInfo>();
+ private config: WorktreeManagerConfig;
+ private activeWorktrees = new Map<string, WorktreeInfo>();
+ private cleanupTriggered = false;- private setupCleanupHandlers(): void {
- const cleanup = () => {
- console.log('Cleaning up worktrees...');
- this.cleanupAll(true).catch(console.error);
- };
-
- process.on('SIGINT', cleanup);
- process.on('SIGTERM', cleanup);
- process.on('exit', cleanup);
- }
+ private setupCleanupHandlers(): void {
+ const cleanup = async () => {
+ if (this.cleanupTriggered) return;
+ this.cleanupTriggered = true;
+ console.log('Cleaning up worktrees...');
+ try {
+ await this.cleanupAll(true);
+ } catch (err) {
+ console.error(err);
+ }
+ };
+ process.once('SIGINT', cleanup);
+ process.once('SIGTERM', cleanup);
+ process.once('beforeExit', cleanup);
+ }Also applies to: 26-28
🤖 Prompt for AI Agents
In packages/workflow-engine/src/worktree/worktree-manager.ts around lines
341-350 (and also apply the same change to the similar handlers at lines 26-28),
the cleanup handler registers async work on process 'exit' and uses process.on
which allows duplicate invocations; change to use process.once for signals and
'beforeExit' instead of 'exit', and add a reentrancy guard (e.g., an internal
boolean like isCleaningUp) to ensure cleanupAll is only invoked once; for the
'beforeExit' handler ensure you await or call cleanupAll properly (since
beforeExit can be async) and keep the 'exit' handler only for synchronous
finalization or to skip async work entirely.
| console.log('Cleaning up worktrees...'); | ||
| this.cleanupAll(true).catch(console.error); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer a pluggable logger over console. in library code.*
Wire a logger (or accept one via config) so callers control verbosity and sinks.
🤖 Prompt for AI Agents
In packages/workflow-engine/src/worktree/worktree-manager.ts around lines 343 to
344, the code uses console.log and console.error directly which hardcodes
logging; refactor to accept a pluggable logger (e.g., via constructor or config)
and replace console.log with logger.info/debug and console.error with
logger.error, ensuring the logger has the minimal methods used or create a small
adapter that falls back to console; update class initialization to store the
logger and update calls to use it, and add tests or default behavior so existing
callers without a logger get a sensible default.
|
went with tdd workflow instead #1289 |
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