feat: Hamster Integration - Cloud Storage, Sentry Telemetry, and Major CLI UX Improvements

[eyaltoledano]

Overview

This PR introduces comprehensive Hamster integration for Task Master, enabling cloud-based team collaboration alongside the existing local workflow. It includes major UX improvements to the CLI initialization flow, Sentry telemetry integration for AI operations, and numerous quality-of-life enhancements for both Hamster and local users.

---

🎯 Major Features

1. Storage Selection & OAuth Integration

The foundation for Hamster cloud integration.

• Interactive storage selection during tm init with two options:
  • Local Taskmaster (Solo): File-based task management in your repo
  • Hamster Studio (Multiplayer): Cloud-based team collaboration with OAuth
• OAuth flow integration: Selecting Hamster triggers authentication
• Storage configuration: Saves storageType in .taskmaster/config.json
• Graceful fallback: Authentication errors fall back to local storage
• Anonymous tracking ID: Generates taskmaster_id for telemetry (privacy-safe)

Files: scripts/init.js, .cursor/rules/hamster.mdc, .claude/hamster.md

---

2. Sentry Telemetry Integration

Comprehensive error tracking and AI usage telemetry.

• New src/telemetry/sentry.js with Vercel AI SDK integration
• Instrumented ALL AI operations: generateText, streamText, generateObject, streamObject
• MCP Server wrapping: Sentry.wrapMcpServerWithSentry() for tool tracking
• Enhanced metadata tracking:
  • command: Filter by operation (e.g., add-task, update-task)
  • outputType: cli vs mcp for context-specific debugging
  • tag: Task tag/brief being operated on
  • projectHash: 8-char SHA256 hash for privacy-safe project grouping
  • userId, briefId: Hamster user and brief context (when authenticated)
• Privacy control: anonymousTelemetry config option (default: true)
• DSN hardcoded internally: Not user-configurable

Files: src/telemetry/sentry.js, src/ai-providers/base-provider.js, scripts/modules/ai-services-unified.js, mcp-server/src/index.js, scripts/dev.js

---

3. CLI UX Improvements

Simpler commands, better help, progress indicators.

Positional Argument Syntax (Backward Compatible)

• tm update-task 1 Added details (no quotes needed for multi-word prompts)
• tm status 1 done (new alias) or tm set-status 1,1.1 in-progress
• tm list done, tm list in-progress, tm list all (with --with-subtasks)

Hamster-Aware Help

• Context-specific help when connected to Hamster (auto-detected)
• Beautiful boxed section headers with rounded borders
• Shows Brief: [name] instead of tag: [name] when connected
• Dynamic width calculation (90% of terminal, min 80, max 120)

Progress Indicators

• Loading spinner for update-task when connected to Hamster
• Shows Updating task X on Hamster... during AI processing

New CLI Aliases

• Added hamster and ham as global aliases alongside tm and taskmaster

Files: scripts/modules/commands.js, scripts/modules/ui.js, apps/cli/src/commands/*.ts

---

4. Init Flow UX Overhaul

Major improvements to the tm init experience.

Reordered Prompts (All Before Confirmation)

1. Storage selection (Local vs Hamster)
2. Shell aliases (tm/hamster/ham)
3. Git prompts (local storage only)
4. AI IDE rules setup (opt-in, defaults to No)
5. Response language
6. Confirmation with ALL settings
7. Project creation

Smart Defaults

• AI IDE rules: Changed from opt-out to opt-in (default: No)
• AI model setup: Skipped for Hamster (managed on backend)
• Git prompts: Skipped for Hamster (tasks in cloud, not Git)

Reduced Verbosity

• Changed 15+ log statements from info to debug level
• Cleaner output for new users (use --log-level debug for details)

Customized Getting Started Messages

• Hamster: Brief creation, context connection, CLI commands
• Local: AI config, PRD creation, task management

ASCII Consistency

• Replaced all emoji icons with ASCII: ✓ ✗ ! → •
• Consistent styling throughout

Files: scripts/init.js, assets/hamster-art.txt

---

5. Brief Selection UI

Interactive table navigation for Hamster briefs.

• Unified table selection: Navigate with arrow keys, select with Enter
• ESC key support to cancel selection
• Truncated names with ellipsis to prevent overflow
• tm briefs command: View and switch between briefs

Files: apps/cli/src/commands/briefs.command.ts, packages/tm-bridge/src/tags-bridge.ts

---

6. Research Streaming Enhancement

Real-time AI reasoning with clean output.

• Stream reasoning in real-time: Muted gray text for <think> content
• Filter <think> tags: Show content during streaming, strip from final output
• Markdown rendering: Beautiful formatted result in bordered box
• Telemetry fix: Preserve ALL telemetry fields when updating after stream

Files: scripts/modules/task-manager/research.js, src/ai-providers/base-provider.js

---

7. Error Formatter

Professional error handling with actionable hints.

• New scripts/modules/error-formatter.js
• Error categorization: auth, validation, network, API, file system, task, permission, timeout
• Context-specific hints: Actionable suggestions for each error type
• Sensitive data sanitization: API keys, tokens, file paths, emails
• Debug mode support: Stack traces when enabled
• UI utilities: displayFormattedError, displayWarning, displayInfo, displaySuccess

Files: scripts/modules/error-formatter.js, scripts/modules/commands.js

---

📁 Files Changed Summary

Category	Files
Init Flow	scripts/init.js (829+ lines changed)
CLI Commands	scripts/modules/commands.js, apps/cli/src/commands/*.ts
Telemetry	src/telemetry/sentry.js (new), src/ai-providers/base-provider.js
AI Services	scripts/modules/ai-services-unified.js, research.js
Error Handling	scripts/modules/error-formatter.js (new)
UI	scripts/modules/ui.js, apps/cli/src/ui/components/*.ts
Bridge	packages/tm-bridge/src/*.ts
Assets	assets/hamster-art.txt (new), .taskmaster/templates/*
Rules/Docs	.cursor/rules/hamster.mdc, .cursor/rules/taskmaster/*
Config	.taskmaster/config.json, package.json, package-lock.json

---

🔄 Breaking Changes

None. All changes are backward compatible:

• Flag syntax still works (--id, --status, etc.)
• Local storage workflow unchanged
• Existing projects work without modification

---

✅ Testing Notes

• Test tm init with Local storage selection
• Test tm init with Hamster storage selection (OAuth flow)
• Test positional syntax: tm update-task 1 test prompt
• Test positional syntax: tm status 1 done
• Test positional syntax: tm list done
• Test Hamster help display when connected
• Test tm briefs command and selection
• Test research streaming with <think> tags
• Verify Sentry telemetry captures AI operations
• Test error formatter with various error types

---

📋 Commits (31)

Click to expand commit list

1. feat(task-66): Add storage selection UI in CLI initialization flow
2. refactor(task-66): Rename tm_uuid to taskmaster_id
3. feat(task-60): Complete storage selection and OAuth integration in init
4. fix(task-66): Skip Git prompts when cloud storage is selected
5. feat(init): Skip AI model configuration for local storage
6. feat(errors): Add comprehensive error message formatter
7. feat(errors): Integrate error formatter into CLI command handlers
8. feat(init): Add ASCII hamster art for cloud storage success message
9. fix(init): Correct AI model setup logic and improve hamster ASCII art
10. refactor(init): Update messaging from 'cloud sync' to 'Hamster Studio'
11. fix(errors): Remove non-existent getBoxWidth import
12. feat(cli): Improve Hamster help width and add list command options
13. refactor(cli): Adjust Hamster help alignment and remove hash symbols
14. feat(cli): Improve Hamster help display and context awareness
15. feat(cli): Add simpler positional syntax and Hamster-aware UI improvements
16. feat: add progress indicators and UX improvements for Hamster workflow
17. feat(telemetry): Add Sentry integration with AI and MCP instrumentation
18. feat(cli): Add 'hamster' and 'ham' as CLI aliases
19. feat(telemetry): Add enhanced metadata tracking to Sentry AI spans
20. feat(telemetry): Add Hamster user and brief context to Sentry metadata
21. feat(research): Stream reasoning with <think> tag filtering
22. feat(init): Add high-fidelity hamster pixel art for cloud storage welcome
23. feat(init): Major UX improvements for Hamster and local workflows
24. fix(init): Improve prompt flow and use ASCII icons consistently
25. fix(init): Make createProjectStructure async to support dynamic imports
26. fix(init): Default AI IDE rules setup to No instead of Yes
27. feat(init): Refine workflow selection copy to emphasize planning-first
28. fix(init): Fix setResponseLanguage import and refine workflow copy
29. fix(init): Update Workflow section title and subtitle for Hamster users
30. feat(briefs): Add integrated table selection for brief switching
31. feat(init): improve init flow and fix rebase artifacts

---

Related Tasks

• TAS-60: CLI initialization can present storage options and trigger authentication
• TAS-66: Build storage option selection UI in CLI initialization flow
• TAS-69: Build error message formatter with context-specific hints
• TAS-76: Integrate error formatter into command handlers
• TAS-82: Add progress indicators and UX improvements for Hamster workflow
• TAS-89: Add Sentry telemetry integration

There is more coming. This PR is not ready for merge yet.

---

Note

Integrates Hamster cloud workflow (storage selection + OAuth, briefs/export/PRD, invitations) with Sentry-instrumented AI ops, and ships a major CLI/init UX revamp (positional syntax, Hamster-aware help, streaming research, rich error formatting).

• Hamster Cloud Integration:
  • tm init: storage selection (local vs cloud) with OAuth; writes config and falls back gracefully.
  • Briefs: list/select (tm briefs), generate from tasks/PRD, auto-set context, and send team invitations.
  • Auth improvements: tm login/logout, header control flags; display-ID normalization (e.g., ham31 → HAM-31).
• AI Telemetry (Sentry):
  • New src/telemetry/sentry.js; instruments generateText/streamText/generateObject/streamObject with rich metadata (command/outputType/tag/projectHash/userId/briefId); opt-out via anonymousTelemetry.
• CLI & Init UX:
  • Positional syntax (update-task, status, list), new aliases (hamster, ham), Hamster-aware help, TUI entry (tui/repl).
  • Init flow overhaul: reordered prompts, rules setup opt-in, skip local model setup for cloud, tailored onboarding, ASCII-only output.
  • Spinners/progress (e.g., remote updates) and broader error surfacing.
• Research & Rendering:
  • Stream reasoning with <think> filtering; final output as formatted Markdown; render tiptap HTML via turndown + marked.
• Error Handling:
  • New scripts/modules/error-formatter.js with categorization, hints, and sanitization; integrated in command handlers.
• Core/Services:
  • Add normalizeDisplayId; new prompts module (types/services/constants) and exports.
  • Integration domain/service: generate brief from tasks/PRD, brief status polling, team invitations; dependency cleanup on export.
  • Auth constants (AUTH_TIMEOUT_MS, MFA_MAX_ATTEMPTS) and domain helpers.
• Bridge/UX Tweaks:
  • tags-bridge table skip option; refined update-bridge spinner messaging.
• Dependencies:
  • Add @sentry/node, turndown, @types/turndown.

^{Written by Cursor Bugbot for commit 519c3a6. This will update automatically on new commits. Configure here.}

Summary by CodeRabbit

• New Features

  • Hamster collaboration: export tasks to briefs, parse PRDs to briefs, invite teammates, and auto-set brief context
  • New auth commands: tm login / tm logout with OAuth flows and MFA support

• UX Improvements

  • Simpler positional CLI syntax, richer TTY interactions (spinners, progress, clearer messages)
  • Clickable links and improved HTML/Markdown rendering for task descriptions

• Telemetry

  • Sentry-based AI/error telemetry with anonymous opt-out (default: enabled)

• Documentation

  • Many new Hamster guides, workflows, templates, and research notes

_{✏️ Tip: You can customize this high-level summary in your review settings.}

---

Update Nov 27

[3 tools called]

Here's a comprehensive summary for your PR #1452 update:

---

📝 PR Update: What's New Since Last Update

🚀 Major Feature: Complete Export Command Redesign

tm export - Generate Briefs from Tasks

• NEW ARCHITECTURE: Export now creates new briefs from tasks atomically (instead of exporting to existing briefs)
• Multi-select tags with parallel export support (Promise.all)
• Track exported tags in state.json to prevent duplicate exports
• Auto-set CLI context to newly created brief after export
• AI-generated brief titles and descriptions from task content

Interactive UX Flow:

1. Check if brief already in context → prompt for different local tag
2. Show tag selection with task counts, mark already-exported as disabled
3. Pre-select current tag
4. Show value prop message after selection
5. Preview tasks with warnings
6. Export with optional team invitations
7. Display results and auto-set context

👥 Team Invitations During Export

• -I, --invite flag or interactive prompt
• Collect emails before API call (max 10, validated)
• Invitations sent atomically with brief creation
• Display invitation results (sent/already_member/error)
• Show invite URL if user declines

🔐 Authentication Improvements

• tm login / tm logout: New aliases for tm auth login / tm auth logout
• --no-header flag: Added to all tm auth and tm context commands
• Auth Guard Utility: Reusable ensureAuthenticated() with full MFA/OTP support
• Fixed AuthManager method calls and MFA retry logic

🆔 Display ID Shortcuts

tm show ham31      # Works! (normalizes to HAM-31)
tm show HAM31      # Works!
tm set-status --id=ham31.1 --status=done  # Subtasks too!

📋 Rendering Improvements

• HTML/Markdown Rendering: Task descriptions from tiptap now render properly using turndown + marked
• Applied to: task details, implementation details, test strategy, next task

🛠 Bug Fixes

• Strip invalid dependencies before API call (prevents "non-existent task" errors)
• Use FileStorage directly for accurate task/subtask counts
• Fix briefs command to ensure org is selected first
• Read local tags from file storage, not API (when brief in context)
• Fix duplicate exports in tm-core index.ts
• Resolve all TypeScript errors in login, briefs, list, next commands

📊 UX Polish

• Clean value prop box (no emojis, per preference)
• Compact task selection with status icons
• Full-width tables
• Show subtask counts: "11 tasks + 55 subtasks (66 total)"
• Show actual warning messages, not just count

---

Files Changed: ~20+ files across apps/cli, packages/tm-core
New Dependencies: turndown for HTML→Markdown conversion

[coderabbitai]

Actionable comments posted: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (16)

mcp-server/src/core/direct-functions/fix-dependencies.js (1)

54-73: Use try/finally pattern for silent mode as per coding guidelines.

The current implementation calls disableSilentMode() in both the success path (line 61) and catch block (line 73), but this doesn't follow the required pattern. According to the coding guidelines: "Use try/finally block pattern when wrapping core function calls with enableSilentMode/disableSilentMode to ensure silent mode is always restored, even if errors occur."

As per coding guidelines, apply this diff to use the try/finally pattern:

-		// Enable silent mode to prevent console logs from interfering with JSON response
-		enableSilentMode();
-
-		const options = { projectRoot, tag };
-		// Call the original command function using the provided path and proper context
-		await fixDependenciesCommand(tasksPath, options);
-
-		// Restore normal logging
-		disableSilentMode();
-
-		return {
-			success: true,
-			data: {
-				message: 'Dependencies fixed successfully',
-				tasksPath,
-				tag: tag || 'master'
-			}
-		};
+		// Enable silent mode to prevent console logs from interfering with JSON response
+		try {
+			enableSilentMode();
+			const options = { projectRoot, tag };
+			// Call the original command function using the provided path and proper context
+			await fixDependenciesCommand(tasksPath, options);
+
+			return {
+				success: true,
+				data: {
+					message: 'Dependencies fixed successfully',
+					tasksPath,
+					tag: tag || 'master'
+				}
+			};
+		} finally {
+			// Restore normal logging
+			disableSilentMode();
+		}
 	} catch (error) {
-		// Make sure to restore normal logging even if there's an error
-		disableSilentMode();
-
 		log.error(`Error fixing dependencies: ${error.message}`);
 		return {
 			success: false,

scripts/modules/task-manager/update-subtask-by-id.js (1)

317-331: Remove development comment artifact.

Line 321 contains a development comment // <--- ADD THIS LINE: Store for display that should be removed before merging.

Apply this diff:

 			const timestamp = new Date().toISOString();
 			const formattedBlock = `<info added on ${timestamp}>\n${generatedContentString.trim()}\n</info added on ${timestamp}>`;
-			newlyAddedSnippet = formattedBlock; // <--- ADD THIS LINE: Store for display
+			newlyAddedSnippet = formattedBlock;
 
 			subtask.details =

scripts/modules/task-manager/expand-task.js (1)

67-67: Return structure mismatch between remote and local expansion requires fixing.

Verification confirms the original concern. The remote expansion and local expansion return incompatible structures:

Remote expansion returns:

{
  success: boolean,
  taskId: string | number,
  message: string,
  telemetryData: null,
  tagInfo: null
}

Local expansion returns:

{
  task,
  telemetryData,
  tagInfo
}

At line 90, expandTask returns remoteResult directly without transformation, but callers expecting the local expansion structure will fail. The fallback to local expansion (line 92+) returns a completely different shape, creating inconsistent API behavior.

The truthy check at line 89 is correct (tryExpandViaRemote returns null when not handling the request), and error propagation is correct (errors throw and are caught by the outer try-catch at line 382). However, the return structure incompatibility must be resolved:

Either:

1. Transform remoteResult to match local structure before returning at line 90, or
2. Normalize both to return the same structure

scripts/modules/task-manager/expand-all-tasks.js (1)

189-197: Replace direct console.log / console.error with the central logger

Direct console.log / console.error calls here bypass the central logging utility and silent-mode handling, which conflicts with the logging guidelines (**/*.{js,ts}).

You can route both the summary box and debug stack through logger instead:

-    if (outputFormat === 'text') {
-      const summaryContent =
-        `${chalk.white.bold('Expansion Summary:')}\n\n` +
-        `${chalk.cyan('-')} Attempted: ${chalk.bold(tasksToExpandCount)}\n` +
-        `${chalk.green('-')} Expanded:  ${chalk.bold(expandedCount)}\n` +
-        // Skipped count is always 0 now due to pre-filtering
-        `${chalk.gray('-')} Skipped:   ${chalk.bold(0)}\n` +
-        `${chalk.red('-')} Failed:    ${chalk.bold(failedCount)}`;
-
-      console.log(
-        boxen(summaryContent, {
-          padding: 1,
-          margin: { top: 1 },
-          borderColor: failedCount > 0 ? 'red' : 'green', // Red if failures, green otherwise
-          borderStyle: 'round'
-        })
-      );
-    }
+    if (outputFormat === 'text') {
+      const summaryContent =
+        `${chalk.white.bold('Expansion Summary:')}\n\n` +
+        `${chalk.cyan('-')} Attempted: ${chalk.bold(tasksToExpandCount)}\n` +
+        `${chalk.green('-')} Expanded:  ${chalk.bold(expandedCount)}\n` +
+        // Skipped count is always 0 now due to pre-filtering
+        `${chalk.gray('-')} Skipped:   ${chalk.bold(0)}\n` +
+        `${chalk.red('-')} Failed:    ${chalk.bold(failedCount)}`;
+
+      logger.info(
+        boxen(summaryContent, {
+          padding: 1,
+          margin: { top: 1 },
+          borderColor: failedCount > 0 ? 'red' : 'green', // Red if failures, green otherwise
+          borderStyle: 'round'
+        })
+      );
+    }
@@
-    if (!isMCPCall && getDebugFlag(session)) {
-      console.error(error); // Log full stack in debug CLI mode
-    }
+    if (!isMCPCall && getDebugFlag(session)) {
+      // Log full stack in debug CLI mode via central logger
+      logger.error(error.stack || error.message);
+    }

This keeps behavior but centralizes all terminal output through the logging layer and honors silent mode.
As per coding guidelines, console usage should go through the logging utility.

Also applies to: 216-218

mcp-server/src/core/direct-functions/models.js (1)

60-115: Propagate commandName and outputType: 'mcp' into all core model calls

Per the MCP direct-function guidelines, the context passed into core functions should explicitly include commandName and outputType: 'mcp' (alongside session, mcpLog, and projectRoot) so that telemetry, logging, and output suppression behave correctly for MCP calls, instead of relying on CLI defaults.

Right now getAvailableModelsList, setModel, and getModelConfiguration only receive { session, mcpLog, projectRoot }, which may cause them to treat these calls as generic/CLI invocations.

Consider refactoring to use a shared base context that includes the required fields, and pass it consistently everywhere, e.g.:

 export async function modelsDirect(args, log, context = {}) {
   const { session } = context;
-  const { projectRoot } = args; // Extract projectRoot from args
+  const { projectRoot } = args; // Extract projectRoot from args
+
+  const baseContext = {
+    session,
+    mcpLog: createLogWrapper(log),
+    projectRoot,
+    commandName: 'models',
+    outputType: 'mcp'
+  };
-
-  // Create a logger wrapper that the core functions can use
-  const mcpLog = createLogWrapper(log);
+  const { mcpLog } = baseContext;
@@
-      if (args.listAvailableModels === true) {
-        return await getAvailableModelsList({
-          session,
-          mcpLog,
-          projectRoot
-        });
-      }
+      if (args.listAvailableModels === true) {
+        return await getAvailableModelsList(baseContext);
+      }
@@
-      const modelContext = { session, mcpLog, projectRoot };
-      const modelSetResult = await handleModelSetting(args, modelContext);
+      const modelSetResult = await handleModelSetting(args, baseContext);
@@
-      return await getModelConfiguration({
-        session,
-        mcpLog,
-        projectRoot
-      });
+      return await getModelConfiguration(baseContext);

…and update handleModelSetting to expect that extended context.

This keeps the direct function aligned with the standardized MCP context contract for core functions.

As per coding guidelines, direct wrappers should always pass commandName and outputType: 'mcp' in their context.

mcp-server/src/tools/set-task-status.js (2)

37-41: Fix minor typo in status parameter description

The description string for status is missing a closing parenthesis and has the period inside the list, which is slightly off in help/metadata output.

Current:

.describe(
  "New status to set (e.g., 'pending', 'done', 'in-progress', 'review', 'deferred', 'cancelled'."
)

Suggested:

-                .describe(
-                    "New status to set (e.g., 'pending', 'done', 'in-progress', 'review', 'deferred', 'cancelled'."
-                ),
+                .describe(
+                    "New status to set (e.g., 'pending', 'done', 'in-progress', 'review', 'deferred', 'cancelled')."
+                ),

---

31-53: Make projectRoot optional in the Zod schema to enable the withNormalizedProjectRoot fallback mechanism

The withNormalizedProjectRoot HOF is designed with a precedence chain to derive projectRoot from environment variables, session context, or the provided argument. However, the current Zod schema requires projectRoot to be explicitly provided, which prevents the HOF's fallback logic from being invoked when the argument is omitted. This is inconsistent with the tool's design and prevents clients from relying on session-based or environment-based project root resolution.

Suggested change:

-            projectRoot: z
-                .string()
-                .describe('The directory of the project. Must be an absolute path.'),
+            projectRoot: z
+                .string()
+                .optional()
+                .describe(
+                    'Optional project root. If omitted, the tool will infer and normalize it from the MCP session/context.'
+                ),

Consistency note: This change applies to all legacy MCP tools in mcp-server/src/tools/*.js. A codebase-wide audit found that all examined tools (add-task.js, add-subtask.js, analyze.js, etc.) currently define projectRoot as required, but they should all be updated to optional to align with the withNormalizedProjectRoot HOF's intended design.

scripts/modules/task-manager/list-tasks.js (1)

613-657: Subtask dependency detection via < 100 is brittle and can misclassify IDs

The subtask dependency formatting logic treats any numeric depId < 100 as a subtask ID (lines 619-623), otherwise as a top‑level task ID (lines 640-655). This heuristic is fragile:

• If you ever have top‑level tasks with IDs below 100 (which is common), they’ll be misinterpreted as subtasks of the current task when rendering dependencies.
• The threshold 100 is a magic number with no explicit contract in the data model and may surprise future maintainers.

Consider replacing this with a more explicit scheme, for example:

• Distinguish task vs subtask dependencies structurally in the data (e.g., storing subtasks deps as "<parentId>.<subId>" or { type: 'subtask', parentId, subtaskId }), or
• Use a flag on the dependency entry rather than a numeric range heuristic.

At minimum, a TODO/comment documenting the assumption and its implications would help, but a more explicit representation would avoid subtle misrendering bugs as the ID space grows.

mcp-server/src/tools/expand-task.js (1)

57-57: Avoid logging full args (includes prompt and potentially sensitive data)

JSON.stringify(args) will log the entire prompt and other user-provided context, which conflicts with the guidance to log sanitized arguments and avoid sensitive payloads in MCP logs.

Recommend logging a minimal, non-sensitive subset instead:

-			try {
-				log.info(`Starting expand-task with args: ${JSON.stringify(args)}`);
+			try {
+				const { id, num, research, file, projectRoot, tag } = args;
+				log.info(
+					`Starting expand-task with args: ${JSON.stringify({
+						id,
+						num,
+						research,
+						file,
+						projectRoot,
+						tag
+					})}`
+				);

This keeps observability while stripping out the free-form prompt and other potentially sensitive fields.

mcp-server/src/core/direct-functions/scope-up.js (2)

20-21: Have the direct function resolve tasksJsonPath via findTasksJsonPath instead of requiring it

Per the MCP rules, direct functions should resolve tasks.json themselves using findTasksJsonPath(args, log) rather than requiring tasksJsonPath as a mandatory argument. Currently you treat tasksJsonPath as required and error out if it’s missing, which diverges from the shared pattern and duplicates responsibility across tools and direct functions.

Recommend:

• Import findTasksJsonPath from the core utils.
• Treat args.tasksJsonPath as an optional override (mainly for tests), but default to await findTasksJsonPath(args, log).
• Drop the explicit MISSING_ARGUMENT error for tasksJsonPath and let findTasksJsonPath handle file/path-related errors with the standard error codes.

Sketch:

-import { scopeUpTask } from '../../../../scripts/modules/task-manager.js';
-import {
-	disableSilentMode,
-	enableSilentMode
-} from '../../../../scripts/modules/utils.js';
+import { scopeUpTask } from '../../../../scripts/modules/task-manager.js';
+import {
+	disableSilentMode,
+	enableSilentMode
+} from '../../../../scripts/modules/utils.js';
+import { findTasksJsonPath } from '../utils/path-utils.js';
@@
-	const {
-		tasksJsonPath,
+	const {
+		tasksJsonPath,
@@
-		// Check if tasksJsonPath was provided
-		if (!tasksJsonPath) {
-			log.error('scopeUpDirect called without tasksJsonPath');
-			return {
-				success: false,
-				error: {
-					code: 'MISSING_ARGUMENT',
-					message: 'tasksJsonPath is required'
-				}
-			};
-		}
+		// Resolve tasks.json path (allow explicit override, but default to standard resolution)
+		const resolvedTasksJsonPath =
+			tasksJsonPath || (await findTasksJsonPath({ projectRoot, tag }, log));
@@
-		const result = await scopeUpTask(
-			tasksJsonPath,
+		const result = await scopeUpTask(
+			resolvedTasksJsonPath,

This keeps the behavior flexible while aligning with the shared MCP path-resolution contract. Based on learnings and MCP direct-function guidelines.

Also applies to: 31-32, 49-59, 82-85

---

62-76: Validate parsed task IDs and fail fast on invalid input

id is documented as a comma-separated list of IDs but there’s no validation after parseInt. Inputs like id="123,abc" will yield [123, NaN], which then get passed to scopeUpTask. That pushes input validation down into core logic (if it exists) and can make error messages less clear.

Suggest:

• Validate that all parsed values are finite numbers.
• If any are invalid, return a standardized input-validation error with a clear message.

Example:

-		// Parse task IDs - convert to numbers as expected by scopeUpTask
-		const taskIds = id.split(',').map((taskId) => parseInt(taskId.trim(), 10));
+		// Parse and validate task IDs - convert to numbers as expected by scopeUpTask
+		const taskIds = id
+			.split(',')
+			.map((taskId) => Number.parseInt(taskId.trim(), 10));
+
+		if (!taskIds.length || taskIds.some((taskId) => !Number.isFinite(taskId))) {
+			return {
+				success: false,
+				error: {
+					code: 'INPUT_VALIDATION_ERROR',
+					message:
+						'The id parameter must contain one or more numeric task IDs (comma-separated).'
+				}
+			};
+		}

This keeps the direct function self-contained and surfaces bad input early. As per coding guidelines, input validation errors should use the shared INPUT_VALIDATION_ERROR code.

scripts/modules/task-manager/remove-task.js (1)

132-167: Based on my verification, the review comment is correct. I found a critical inconsistency:

Key Findings:

1. Schema Mismatch: scope-adjustment.js defines subtask dependencies as z.array(z.string()) while parse-prd-config.js defines main task dependencies as z.array(z.number())
2. Normalization Pattern Exists: move-task.js includes a normalizeDependency() function (lines 73-98) that converts string dependencies to numbers before comparison
3. Remove-task.js Lacks Normalization: While allRemovedIds correctly stores numbers for main IDs and strings for dot-notation, the filter on line 24 performs a direct Set.has(depId) check without normalizing depId first
4. Stale Dependencies Risk: If any main task dependency is stored as a string (e.g., "5" instead of 5) due to edge cases or migration issues, the filter will fail to remove it when that task is deleted

The fix should follow the pattern demonstrated in move-task.js by normalizing the dependency ID before checking if it exists in the removed set.

---

Normalize dependency IDs before removing stale references to prevent missed cleanup

While allRemovedIds correctly stores numeric IDs for main tasks and string IDs for subtasks (e.g., "5.2"), the filter on line 24 performs a direct !allRemovedIds.has(depId) check without normalization. If a main task dependency is stored as a string (e.g., "5" instead of 5), the Set lookup will fail and stale dependencies will remain.

The codebase already demonstrates the correct pattern in move-task.js (lines 73-98) with the normalizeDependency() function. Apply similar normalization here before the Set comparison:

task.dependencies = task.dependencies.filter((depId) => {
  const normalizedId = typeof depId === 'string' ? parseInt(depId, 10) : depId;
  return !allRemovedIds.has(normalizedId);
});

This ensures stale dependencies are removed regardless of whether they're stored as strings or numbers.

mcp-server/src/tools/scope-up.js (1)

44-46: Based on my verification, I have confirmed the review comment is accurate. The evidence is clear:

1. Coding Guidelines Explicitly State: Both architecture.mdc (line 298) and mcp.mdc (line 220) state that projectRoot should be defined as z.string().optional().describe(...)

2. withNormalizedProjectRoot HOF Design: The HOF (lines 718-730 of utils.js) has a complete fallback chain:

   • Environment variable (TASK_MASTER_PROJECT_ROOT)
   • args.projectRoot (if provided)
   • Session-based resolution
   • Only errors if none of the above exist

3. Current Implementation Inconsistency:

   • scope-up.js (lines 44-46) defines projectRoot as required (no .optional())
   • The code comment at line 62 acknowledges the HOF's guarantee
   • ALL examined MCP tools (add-task.js, set-task-status.js, scope-down.js, etc.) have the same issue

4. Clear Discrepancy: The tools require projectRoot in their Zod schemas, but the HOF is specifically designed to handle optional projectRoot with intelligent fallback resolution.

---

Update projectRoot parameter to be optional per coding guidelines.

The projectRoot parameter in the zod schema should be defined as z.string().optional() since withNormalizedProjectRoot provides fallback resolution through environment variables and session context. This aligns with the MCP architecture guidelines (.cursor/rules/architecture.mdc:298 and .cursor/rules/mcp.mdc:220) which explicitly state: "Define parameters, making projectRoot optional: projectRoot: z.string().optional().describe(...)"

projectRoot: z
  .string()
  .optional()
  .describe('The directory of the project. Must be an absolute path.'),

mcp-server/src/tools/remove-task.js (1)

6-92: confirm parameter is exposed but not used in remove_task direct call

The new imports from @tm/mcp, z, and resolveTag are wired correctly, and the rest of the tool follows the expected MCP pattern.

However, the parameters schema exposes a confirm?: boolean flag:

confirm: z
  .boolean()
  .optional()
  .describe('Whether to skip confirmation prompt (default: false)'),

but confirm is never passed into removeTaskDirect, so setting it has no effect on behavior. To avoid surprising consumers:

• Either pass it through to the direct function:

-        const result = await removeTaskDirect(
-          {
-            tasksJsonPath: tasksJsonPath,
-            id: args.id,
-            projectRoot: args.projectRoot,
-            tag: resolvedTag
-          },
+        const result = await removeTaskDirect(
+          {
+            tasksJsonPath: tasksJsonPath,
+            id: args.id,
+            confirm: args.confirm,
+            projectRoot: args.projectRoot,
+            tag: resolvedTag
+          },

(if removeTaskDirect supports this),

• Or, if confirmation is intentionally handled elsewhere for MCP flows, remove confirm from the schema/description to avoid a dead parameter.

scripts/modules/task-manager/tag-management.js (1)

1579-1592: Remove unreachable code after early return.

Lines 1583-1588 are unreachable due to the return null; at line 1580. The comment indicates git workflow was intentionally removed, but the dead code should be deleted for clarity.

 	// Git workflow has been removed - return null to disable auto-switching
 	return null;
-
-	// Perform auto-switch
-	return await autoSwitchTagForBranch(
-		tasksPath,
-		{ createIfMissing: true, copyFromCurrent: false },
-		context,
-		'json'
-	);
 } catch (error) {

scripts/modules/task-manager/update-task-by-id.js (1)

433-436: I'll help you verify this review comment about task ID preservation. Let me start by examining the file structure and understanding the context around the code being reviewed.
<function_calls>

First, let's locate and examine the update-task-by-id.js file

fd "update-task-by-id.js" --type f


</function_calls>

<function_calls>

Get the file size to ensure we can read it

wc -l scripts/modules/task-manager/update-task-by-id.js


</function_calls>

<function_calls>

Read the full file to understand context

cat -n scripts/modules/task-manager/update-task-by-id.js


</function_calls>

<function_calls>

Now let's trace how updateTaskById is called - search for invocations

rg -n "updateTaskById" --type js -C 3


</function_calls>

<function_calls>

Let's check how tasks are read from file and what their ID type is

First, find the readJSON or task reading logic

rg -n "readJSON" scripts/modules/task-manager/ --type js -B 2 -A 2


</function_calls>

<function_calls>

Check the JSON files to understand the actual ID format

fd "tasks*.json" --type f | head -5


</function_calls>

<function_calls>

Let's look at a sample tasks.json to see ID format

find . -name "tasks.json" -o -name "tasks*.json" 2>/dev/null | head -1 | xargs head -50


</function_calls>

<function_calls>

Let's check the utilities to understand ID handling

cat scripts/modules/utils.js | head -100


</function_calls>

<function_calls>

Check how tasks are read and ID types are handled

cat scripts/modules/task-manager/update-task-by-id.js | sed -n '1,420p' | tail -100


</function_calls>

<function_calls>

Let's see the actual readJSON and writeJSON implementations

rg -n "function readJSON|function writeJSON|exports.readJSON|exports.writeJSON" scripts/modules/ --type js -A 10


</function_calls>

<function_calls>

Check the readJSON utility more carefully

fd ".js" scripts/modules -x grep -l "readJSON" {} ; | head -3 | xargs grep -n "function readJSON|readJSON.="


</function_calls>

<function_calls>

Let's look at the JSON utility file

cat scripts/modules/utils.js | grep -n "readJSON" -A 20


</function_calls>

<function_calls>

Check what the actual JSON file structure looks like

Let's also look at where taskToUpdate comes from in update-task-by-id.js

sed -n '400,415p' scripts/modules/task-manager/update-task-by-id.js


</function_calls>

<function_calls>

Now let's check the critical section - how taskToUpdate is assigned

sed -n '400,410p' scripts/modules/task-manager/update-task-by-id.js


</function_calls>

<function_calls>

Let's find the findTaskById function to see how tasks are looked up

rg -n "function findTaskById|findTaskById.*=" scripts/modules/ --type js -A 20


</function_calls>

<function_calls>

Search for findTaskById implementation

rg -n "findTaskById" scripts/modules/task-manager/ --type js -A 15


</function_calls>

<function_calls>

Let's look at how tasks are compared - search for strict equality with task IDs

rg -n "task.id\s*===" scripts/modules/task-manager/ --type js -C 2


</function_calls>

<function_calls>

Check the specific lines 433-436 more carefully and surrounding context

sed -n '425,445p' scripts/modules/task-manager/update-task-by-id.js


</function_calls>

<function_calls>

Check if tasks.json files actually exist and their content

find . -path ./node_modules -prune -o -name "tasks.json" -type f -print 2>/dev/null | head -2


</function_calls>

<function_calls>

Let's examine the entire file around that section with more context

sed -n '390,450p' scripts/modules/task-manager/update-task-by-id.js


</function_calls>

<function_calls>

Let's trace the complete flow - check how tasks are initially parsed from JSON

Look for any JSON.parse or similar that might affect ID type

rg -n "JSON.parse" scripts/modules/task-manager/ --type js -B 2 -A 2


</function_calls>

<function_calls>

Check the entire findTaskById implementation more carefully

rg -n "findTaskById" scripts/modules -A 25 --type js | head -40


</function_calls>

<function_calls>

Look at the core logic - findTaskById from task-manager

cat scripts/modules/task-manager/find-task-by-id.js


</function_calls>

<function_calls>

Let's check the learnings about task IDs to understand the intended behavior

The learnings mention numeric IDs for file storage and dot notation for subtasks

echo "=== Checking for ID format specifications ==="

Search for any ID format documentation or rules

rg -n "numeric.*id|id.*numeric" scripts/ -i --type js | head -20


</function_calls>

<function_calls>

Let's check if there are any test files that show expected behavior

fd ".test..js" scripts/modules/task-manager/ --type f


</function_calls>

<function_calls>

Let's also check the learnings about task ID handling

They mention using numeric IDs for direct task references

Check the coding guidelines to understand ID type handling

rg -n "numeric.*id|task.*id.*type|string.*number" .cursor/rules/ --type md 2>/dev/null | head -20


</function_calls>

<function_calls>

Let's examine how update-task-by-id is actually called to see the taskId parameter type

Search for CLI command that invokes this

rg -n "update-task-by-id|updateTaskById" scripts/modules/ --type js -B 5 | grep -E "(parseInt|Number|String|taskId.*=)" | head -20


</function_calls>

<function_calls>

Check the CLI entry point for update-task command

fd "update-task.js" scripts/commands/ --type f


</function_calls>

<function_calls>

Read the update-task CLI command

cat scripts/commands/update-task.js


</function_calls>

<function_calls>

Let's check specifically the line where taskId is extracted and used

cat scripts/commands/update-task.js | grep -n "taskId|parseInt|argv." | head -20


</function_calls>

<function_calls>

Let's look at the complete update-task.js with line numbers

cat -n scripts/commands/update-task.js


</function_calls>

<function_calls>

Now let's check the findTaskById function to see how it compares IDs

cat -n scripts/modules/task-manager/find-task-by-id.js


</function_calls>

<function_calls>

Let's trace what type taskToUpdate.id actually is - check if tasks maintain numeric IDs

Let's look at how tasks are read and their structure

cat scripts/modules/task-manager/read-tasks.js


</function_calls>

<function_calls>

Check if there's any ID type coercion happening when reading tasks

rg -n "id.*=" scripts/modules/task-manager/read-tasks.js --type js -C 2


</function_calls>

<function_calls>

Let's understand better - check if JSON.parse preserves numeric IDs

Create a test to verify

node << 'EOF'
const testJson = '{"id": 1, "title": "test"}';
const parsed = JSON.parse(testJson);
console.log("Type of id:", typeof parsed.id);
console.log("Value of id:", parsed.id);
console.log("Strict equality (1 === parsed.id):", 1 === parsed.id);
console.log("String comparison ('1' === parsed.id):", '1' === parsed.id);

const stringIdJson = '{"id": "1", "title": "test"}';
const parsed2 = JSON.parse(stringIdJson);
console.log("\nWith string ID in JSON:");
console.log("Type of id:", typeof parsed2.id);
console.log("String equality ('1' === parsed2.id):", '1' === parsed2.id);
EOF


</function_calls>

<function_calls>

Let's find ALL callers of updateTaskById to see if any pass string IDs

rg -n "updateTaskById(" scripts/ --type js -B 5 | grep -E "(updateTaskById(|taskId|parseInt|argv)"


</function_calls>

<function_calls>

More thorough search for updateTaskById calls

rg -n "updateTaskById" scripts/ --type js | grep -v "^Binary"


</function_calls>

<function_calls>

Let's check the task-manager index to see what's exported

cat scripts/modules/task-manager/index.js


</function_calls>

<function_calls>

Let's look for how the module is used by checking imports

rg -n "from.*update-task|require.*update-task" scripts/ --type js -C 2


</function_calls>

<function_calls>

Now let's check what type the AI service returns for task IDs

Look at the AI service integration in update-task-by-id.js

sed -n '350,380p' scripts/modules/task-manager/update-task-by-id.js


</function_calls>

<function_calls>

Let's see the AI call and what it returns

sed -n '370,410p' scripts/modules/task-manager/update-task-by-id.js


</function_calls>

<function_calls>

Let's understand the full flow - what does the AI return?

Check the prompt or schema that's sent to AI

sed -n '310,360p' scripts/modules/task-manager/update-task-by-id.js


</function_calls>

<function_calls>

Let's see the beginning of the function to understand the full context

sed -n '1,50p' scripts/modules/task-manager/update-task-by-id.js


</function_calls>

<function_calls>

Now let's create a comprehensive test to verify the type issue

node << 'EOF'
// Simulate the scenario described in the review

// Scenario 1: All numeric IDs (correct case)
const taskToUpdate_numeric = { id: 1, title: "Test" };
const taskId_numeric = 1; // from parseInt
const updatedTask_from_ai_numeric = { id: 1, title: "Updated" };

console.log("=== Scenario 1: All numeric IDs ===");
console.log("taskId (input):", taskId_numeric, typeof taskId_numeric);
console.log("taskToUpdate.id (from file):", taskToUpdate_numeric.id, typeof taskToUpdate_numeric.id);
console.log("updatedTask.id (from AI):", updatedTask_from_ai_numeric.id, typeof updatedTask_from_ai_numeric.id);
console.log("Condition (updatedTask.id !== taskId):", updatedTask_from_ai_numeric.id !== taskId_numeric);
console.log("Would restore? No (they're equal)\n");

// Scenario 2: AI returns string ID
const taskToUpdate_numeric2 = { id: 1, title: "Test" };
const taskId_numeric2 = 1; // from parseInt
const updatedTask_from_ai_string = { id: "1", title: "Updated" };

console.log("=== Scenario 2: AI returns string ID ===");
console.log("taskId (input):", taskId_numeric2, typeof taskId_numeric2);
console.log("taskToUpdate.id (from file):", taskToUpdate_numeric2.id, typeof taskToUpdate_numeric2.id);
console.log("updatedTask.id (from AI):", updatedTask_from_ai_string.id, typeof updatedTask_from_ai_string.id);
console.log("Condition (updatedTask.id !== taskId):", updatedTask_from_ai_string.id !== taskId_numeric2);
console.log("Would restore with current code? Yes (string '1' !== number 1)");
console.log("Would restore with proposed code? Yes (string '1' !== number 1)");
console.log("Result with current code: updatedTask.id = 1 (numeric) ✓");
console.log("Result with proposed code: updatedTask.id = 1 (numeric) ✓\n");

// Scenario 3: What if taskId is somehow a string?
const taskToUpdate_numeric3 = { id: 1, title: "Test" };
const taskId_string = "1"; // hypothetical - if not parsed
const updatedTask_from_ai_string2 = { id: "1", title: "Updated" };

console.log("=== Scenario 3: taskId is string (hypothetical) ===");
console.log("taskId (input):", taskId_string, typeof taskId_string);
console.log("taskToUpdate.id (from file):", taskToUpdate_numeric3.id, typeof taskToUpdate_numeric3.id);
console.log("updatedTask.id (from AI):", updatedTask_from_ai_string2.id, typeof updatedTask_from_ai_string2.id);
console.log("Condition (updatedTask.id !== taskId):", updatedTask_from_ai_string2.id !== taskId_string);
console.log("Would restore with current code? No (both are string '1')");
console.log("Result with current code: updatedTask.id stays '1' (STRING) ✗ - TYPE DRIFT!");
console.log("Would restore with proposed code? Yes (string '1' !== number 1)");
console.log("Result with proposed code: updatedTask.id = 1 (numeric) ✓");
EOF


</function_calls>

<function_calls>

Let's do a more thorough search for all references to updateTaskById

including in test files, other modules, and documentation

find scripts -name "*.js" -type f -exec grep -l "updateTaskById" {} ; 2>/dev/null


</function_calls>

<function_calls>

Now let's check each file that references updateTaskById

to see if it's imported/called

for file in $(find scripts -name "*.js" -type f -exec grep -l "updateTaskById" {} ;); do
echo "=== $file ==="
grep -n "updateTaskById" "$file" | head -5
done


</function_calls>

<function_calls>

Let's check if there are any other commands or modules that might call updateTaskById

grep -r "updateTaskById" scripts/ --include="*.js" -n | grep -v "update-task-by-id.js" | head -20


</function_calls>

<function_calls>

Let's check the learnings again to understand if there's guidance on ID type handling

The learnings mention numeric IDs for direct task references

Let's look at what parseTaskId does

grep -n "parseTaskId" scripts/modules/utils.js -A 15


</function_calls>

<function_calls>

Let's look for test fixtures or sample tasks.json files

find . -name ".json" -path "/test*" -o -name ".json" -path "/fixture*" | head -10


</function_calls>

<function_calls>

Let's also check the rules directory for guidance on ID handling

ls -la .cursor/rules/


</function_calls>

<function_calls>

Check the rules for ID handling

cat .cursor/rules/tasks.mdc


</function_calls>

<function_calls>

Check dependencies rules for ID format

cat .cursor/rules/dependencies.mdc


</function_calls>

<function_calls>

Let's check if there are any other commands or imports that might call this internally

Check the task-manager module structure

ls -la scripts/modules/task-manager/


</function_calls>

<function_calls>

Let's check if any core task-manager functions call updateTaskById

grep -r "updateTaskById" scripts/modules/task-manager/ -n


</function_calls>

<function_calls>

Let's look at what other code might programmatically call update functions

Check if there's batch update or script execution

grep -r ".id.*!==" scripts/modules/task-manager/ --type js -C 2 | head -40


</function_calls>

<function_calls>

Let's also check the AI integration - what schema/format does AI expect for task IDs?

Look at the system prompt or schema

sed -n '200,250p' scripts/modules/task-manager/update-task-by-id.js


</function_calls>

<function_calls>

Let's see if there's any type coercion or conversion happening with the AI response

sed -n '410,445p' scripts/modules/task-manager/update-task-by-id.js


</function_calls>

Based on my thorough verification, let me examine one more critical aspect - whether there are any edge cases or ongoing maintenance concerns:
<function_calls>

Final verification: Check if there are any future plans to call updateTaskById from other places

or if there are internal task processing workflows

grep -r "updateTask" scripts/ --type js | grep -v test | grep -v "update-task" | head -10


</function_calls>

<function_calls>

Let's verify the exact current code at those lines one more time

sed -n '430,440p' scripts/modules/task-manager/update-task-by-id.js | cat -n


</function_calls>

Based on my comprehensive verification, I can now generate the rewritten review comment:

---

Use canonical task ID from file storage for ID preservation check to ensure type consistency

The code compares updatedTask.id against taskId (the input parameter), which could theoretically be a string if someone adds a new caller that doesn't parse the ID to numeric. Since the coding rules require numeric IDs for file storage and taskToUpdate.id is guaranteed to be numeric (from the stored task), comparing against taskToUpdate.id instead provides type safety and prevents potential type drift.

-			// Preserve ID if AI changed it
-			if (updatedTask.id !== taskId) {
-				report('warn', `AI changed task ID. Restoring original ID ${taskId}.`);
-				updatedTask.id = taskId;
-			}
+			// Preserve ID if AI changed it (use canonical numeric ID from stored task)
+			if (updatedTask.id !== taskToUpdate.id) {
+				report(
+					'warn',
+					`AI changed task ID. Restoring original ID ${taskToUpdate.id}.`
+				);
+				updatedTask.id = taskToUpdate.id;
+			}

This ensures the restored ID is always numeric (per coding guidelines) regardless of how taskId is passed in.

♻️ Duplicate comments (11)

apps/cli/src/commands/auth.command.ts (1)

98-129: --no-header options declared but not captured in logout/status/refresh actions.

The logout, status, and refresh subcommands define --no-header but the action callbacks don't capture the options parameter. This was flagged in a previous review and remains unaddressed.

apps/cli/src/export/export-validator.ts (1)

15-60: Avoid returning an undefined taskId from validateTask

When task.id is missing, you correctly add "Task ID is required" to errors, but the returned taskId is still undefined. Downstream, validateTasks formats messages as Task ${result.taskId}: ${error}, which will produce "Task undefined: Task ID is required" and may confuse consumers or violate the expected taskId type.

Consider returning a stable sentinel string in this case:

  return {
-		taskId: task.id,
+		taskId: task.id ?? '[missing-task-id]',
    isValid: errors.length === 0,
    errors,
    warnings
  };

This keeps valid IDs intact while making missing IDs explicit and non-undefined.

Also applies to: 73-80

packages/tm-core/src/modules/integration/services/export.service.ts (1)

1016-1024: Path handling improved but consider using path.basename for clarity.

The regex split(/[/\\]/) handles both Unix and Windows separators, which addresses the cross-platform concern. However, using path.basename() would be more idiomatic and clearer:

+import path from 'path';
+
 private getProjectName(): string | undefined {
   try {
     const projectRoot = this.configManager.getProjectRoot();
-    return projectRoot.split(/[/\\]/).pop() || undefined;
+    return path.basename(projectRoot) || undefined;
   } catch {
     return undefined;
   }
 }

apps/cli/src/commands/briefs.command.ts (3)

485-522: ESC cancellation pattern has unreachable code path.

The cancelled flag check at lines 519-522 is unreachable when ESC is pressed because the process.stdin.emit('keypress', '', { name: 'c', ctrl: true }) causes inquirer to throw, bypassing the check entirely.

Consider handling the Ctrl+C error in the catch block to detect ESC-triggered cancellation:

 } catch (error) {
-  // If user cancels (Ctrl+C), inquirer throws - handle gracefully
-  if ((error as any).isTtyError) {
+  // If user cancels (Ctrl+C or ESC), inquirer throws - handle gracefully
+  if ((error as any).isTtyError || cancelled) {
     // Not a TTY, skip interactive prompt
     return;
   }
+  // Check if this was a user cancellation (Ctrl+C or ESC)
+  const errorMessage = (error as Error).message || '';
+  if (errorMessage.includes('User force closed') || cancelled) {
+    return; // Silent exit on user cancellation
+  }

---

314-314: Use ES module import instead of require().

This was flagged in a previous review but remains unaddressed. Using require() in TypeScript breaks ESM consistency.

Add at the top of the file with other imports:

import Table from 'cli-table3';

Then remove line 314:

-const Table = require('cli-table3');

---

211-216: Sort implementation doesn't use updatedAt as documented.

The comment says "Sort: current first, then by updatedAt" but the comparison returns 0 for all non-current briefs, leaving them in arbitrary order.

 // Sort: current first, then by updatedAt
 tags.sort((a, b) => {
   if (a.isCurrent) return -1;
   if (b.isCurrent) return 1;
-  return 0;
+  // Sort by updatedAt descending (most recent first)
+  const aTime = a.updatedAt ? new Date(a.updatedAt).getTime() : 0;
+  const bTime = b.updatedAt ? new Date(b.updatedAt).getTime() : 0;
+  return bTime - aTime;
 });

apps/cli/src/commands/export.command.ts (5)

583-596: Task selection still not used in export.

The TODO comment at line 583 acknowledges that selected tasks are ignored and all tasks from the tag are exported instead. This issue was flagged in previous reviews and remains unresolved, creating a misleading UX.

The interactive task selection (lines 513-530) and preview (lines 548-560) give users the impression they can control which tasks are exported, but line 586 exports all tasks from the tag regardless of selection. Either:

1. Implement selective export by passing selectionResult.selectedTasks to the export API, or
2. Remove the task selection UI and explain that entire tags are exported atomically

---

703-746: Race condition in parallel export tracking.

Multiple tags are exported in parallel via Promise.all (line 750), and each successful export calls trackExportedTag (line 726) which performs a non-atomic read-modify-write on state.json. If two exports complete simultaneously, one may overwrite the other's tracking data.

Apply this diff to serialize state writes:

+		// Mutex for serializing state.json writes
+		const stateWriteLock = { locked: false, queue: [] };
+		const acquireLock = () => new Promise(resolve => {
+			if (!stateWriteLock.locked) {
+				stateWriteLock.locked = true;
+				resolve();
+			} else {
+				stateWriteLock.queue.push(resolve);
+			}
+		});
+		const releaseLock = () => {
+			if (stateWriteLock.queue.length > 0) {
+				const next = stateWriteLock.queue.shift();
+				next();
+			} else {
+				stateWriteLock.locked = false;
+			}
+		};
+
 		const exportPromises: Promise<ExportResult>[] = tags.map(async (tag) => {
 			try {
 				// ... export logic ...
 				if (result.success && result.brief) {
-					await this.trackExportedTag(tag, result.brief.id, result.brief.url);
+					await acquireLock();
+					try {
+						await this.trackExportedTag(tag, result.brief.id, result.brief.url);
+					} finally {
+						releaseLock();
+					}
 					return { ... };
 				}
 			} catch { ... }
 		});

---

1099-1127: State file directory may not exist.

The trackExportedTag method writes to state.json but doesn't ensure the .taskmaster directory exists. This was flagged in previous reviews and remains unaddressed.

Apply this diff to ensure directory exists:

 	private async trackExportedTag(
 		tagName: string,
 		briefId: string,
 		briefUrl: string
 	): Promise<void> {
 		const projectRoot = getProjectRoot();
 		if (!projectRoot) return;

+		const stateDir = path.join(projectRoot, '.taskmaster');
 		const statePath = path.join(projectRoot, '.taskmaster', 'state.json');

 		try {
 			let state: Record<string, any> = {};

 			try {
 				const stateData = await fs.readFile(statePath, 'utf-8');
 				state = JSON.parse(stateData);
 			} catch {
 				// State file doesn't exist, create new
+				await fs.mkdir(stateDir, { recursive: true });
 			}

---

962-985: Type-safe context setting needed.

The unsafe cast (this.taskMasterCore.auth as any).authManager at line 967 was flagged in previous reviews and marked as addressed in commit c66ead6, but the code still shows the unsafe pattern.

As per coding guidelines, CLI should use public APIs from tm-core. Consider one of these approaches:

1. Add a public setContextFromBriefUrl(url: string) method to TmCore
2. Expose authManager as a typed property on the auth interface
3. Move the selectBriefFromInput logic into tm-core as a public method

This ensures type safety and respects encapsulation.

---

1157-1169: ExportTagCommand implementation creates nested command instances.

The ExportTagCommand action creates a new ExportCommand and calls parseAsync with synthetic argv (lines 1159-1168). This pattern was flagged in previous reviews and marked as addressed in commit c66ead6, but the problematic code remains.

This approach has issues:

1. parseAsync() is for CLI parsing, not programmatic execution
2. The new instance's state (like lastResult) is not accessible
3. Nested command instances may not behave correctly

Apply this diff to properly delegate:

 		this.action(async (tag: string, options: any) => {
-			// Create and execute ExportCommand with tag option
 			const exportCmd = new ExportCommand();
-			// Call the private method via the public action
-			await exportCmd.parseAsync([
-				'node',
-				'export',
-				'--tag',
-				tag,
-				...(options.title ? ['--title', options.title] : []),
-				...(options.description ? ['--description', options.description] : [])
-			]);
+			// Directly execute with merged options
+			await exportCmd['executeExport']({
+				tag,
+				...options
+			});
 		});

Or better yet, extract the export logic into a reusable public method that both commands can call.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add language specifications to all fenced code blocks.

Per markdownlint (MD040), all fenced code blocks must specify a language identifier for proper syntax highlighting and rendering. Apply these fixes across all code blocks:

- ```
+ ```bash
  /go:ham [task-id]
- ```
+ ```

- ```bash
+ ```bash
  tm show $ARGUMENTS
- ```
+ ```

- ```bash
+ ```bash
  tm show <task-id>
- ```
+ ```

- ```bash
+ ```bash
  tm set-status --id=<subtask-id> --status=in-progress
- ```
+ ```

- ```
+ ```text
  User: "/go:ham 1.2"
  
  1. Claude runs: tm show 1.2
     → Displays full task details
  2. Claude analyzes the task and creates an implementation plan
  3. Claude marks task in-progress: tm set-status --id=1.2 --status=in-progress
  4. Claude begins implementation following the task requirements
  5. Upon completion, Claude runs: tm set-status --id=1.2 --status=done
  6. Claude automatically identifies next task with tm list
- ```
+ ```

- ```
+ ```text
  User: "/go:ham"
  
  1. Claude runs: tm list
  2. Claude identifies next available task (e.g., 1.2)
  3. Claude runs: tm show 1.2
     → Displays full task details
  4. Claude analyzes the task and creates an implementation plan
  5. Claude marks task in-progress: tm set-status --id=1.2 --status=in-progress
  6. Claude begins implementation following the task requirements
  7. Upon completion, Claude runs: tm set-status --id=1.2 --status=done
  8. Claude automatically identifies next task with tm list
- ```
+ ```

Also applies to: 22-24, 40-42, 61-63, 91-101, 104-116

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

7-7: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

In .claude/commands/go/ham.md around lines 7 to 9 (and also apply to ranges
22-24, 40-42, 61-63, 91-101, 104-116), several fenced code blocks lack language
identifiers; update each fenced block to include the appropriate language tag
(e.g., ```bash for shell commands, ```text for plain dialogue) so every
triple-backtick block specifies a language identifier, preserving existing
content and indentation.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Clarify polling timeout behavior and surface persistent failures more explicitly

The polling loop treats a 2‑minute timeout or unknown/non‑generating initial status the same as success, then calls spinner.succeed('Brief created!') even if tasks may still be generating or getBrief has repeatedly failed. This can mislead users into thinking task generation is complete when it may not be.

Consider:

• Explicitly handling the timeout case (e.g., a different message indicating generation is still in progress and they should check Hamster).
• Surfacing repeated polling errors once (instead of fully swallowing them) so users see that background status checks are failing.

🤖 Prompt for AI Agents

In apps/cli/src/hamster/parse-prd-to-hamster.ts around lines 140 to 181, the
polling loop currently treats a timeout or repeated polling errors as success
and always calls spinner.succeed('Brief created!'); change the logic so that (1)
you track consecutive polling exceptions (e.g., count and after N failures show
spinner.warn or spinner.fail and return a failure/action:error result so the
user knows status checks are failing), (2) after the loop completes check the
final briefStatus/newStatus and if it is not 'ready' or 'complete' (including
the case of hitting the maxWait timeout) call spinner.warn (or spinner.fail for
explicit 'failed' state) and return a non-success result with a clear message
that generation is still in progress or timed out and instruct the user to check
Hamster, and (3) only call spinner.succeed('Brief created!') and return success
when the briefStatus/newStatus is explicitly 'ready' or 'complete'. Ensure
taskCount is still surfaced in progress messages while polling.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Invite flow currently ignores collected emails and reports success anyway

promptAndSendInvites validates and collects email addresses but never uses _core or _briefId, and does not call any integration/API to actually send invites. Yet it always shows a “Sending invitations...” spinner and then spinner.succeed(...), which implies real invitations were sent.

This is misleading UX and silently discards user input. Until the integration domain exposes a real invite API, consider either:

• Wiring this helper up to the real invite call, passing emailList and briefId, or
• Clearly marking the flow as not yet implemented (and not asking for emails), or at minimum showing a neutral “Invite flow coming soon” message instead of a success spinner.

🤖 Prompt for AI Agents

In apps/cli/src/hamster/parse-prd-to-hamster.ts around lines 247–296, the
promptAndSendInvites function collects and validates emails but never uses _core
or _briefId and always reports success; update the function to either (A) call
the real invite API on the integration/core with the collected emailList and
briefId, awaiting the result, showing the spinner while the API call is in
progress, succeeding only on a successful response and failing with spinner.fail
plus the API error message on error, or (B) if the invite API is not yet
available, stop soliciting emails and replace the spinner/success flow with a
clear “Invite flow coming soon” informational message (or show the prompt but
display a neutral “not implemented” response) so user input is not silently
discarded; ensure emailList is passed to the API, errors are logged/propagated,
and the UI reflects real outcome.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Incomplete refactoring: remove shadowing local import.

The top-level path import is correct, but line 69 still contains const path = require('path'); which shadows this module-level import. This local require should be removed to complete the refactoring.

Apply this diff to remove the shadowing import at line 69:

 		// Test if the target directory and .taskmaster exist
 		const fs = require('fs');
-		const path = require('path');
 		try {

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In apps/extension/src/utils/mcpClient.ts around lines 1 to 69, there is a
leftover local require "const path = require('path');" at line 69 that shadows
the top-level TypeScript import "import * as path from 'path';"; remove that
local require so the file uses the module-level path import, adjust any nearby
references if necessary, and run the linter/TS compile to ensure there are no
unused variable warnings after removal.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

New Kanban imports are wired correctly; clean up unused useEffect and add .js extensions to relative imports

The added imports (React Query, icons, constants, hooks, and UI components) match their usage later in the component (HEADER_HEIGHT, kanbanStatuses, taskKeys, EmptyState, PollingStatus, TagDropdown, TaskCard, TaskEditModal), so the wiring itself looks good.

Two small follow‑ups:

• useEffect is imported but not used anywhere in this file:

import React, { useState, useCallback, useEffect } from 'react';

This can trigger lint/TS warnings; drop useEffect from the import list if it’s no longer needed.

• The TS/TSX guideline for this repo is to include .js extensions on relative imports for ESM compatibility. The newly added/updated imports like:

import { HEADER_HEIGHT, kanbanStatuses } from '../constants';
import {
  taskKeys,
  useTasks,
  useUpdateTask,
  useUpdateTaskStatus
} from '../hooks/useTaskQueries';
import { EmptyState } from './EmptyState';
import { PollingStatus } from './PollingStatus';
import { TagDropdown } from './TagDropdown';
import { TaskCard } from './TaskCard';
import { TaskEditModal } from './TaskEditModal';

should be converted to their .js forms (e.g., ../constants.js, ../hooks/useTaskQueries.js, ./EmptyState.js, etc.) to match that convention.

As per TypeScript/ESM import guidelines for this repo.

🤖 Prompt for AI Agents

In apps/extension/src/webview/components/TaskMasterKanban.tsx around lines 5 to
28, remove the unused useEffect from the React import list (change React, {
useState, useCallback, useEffect } to React, { useState, useCallback }) to avoid
lint/TS warnings, and update all relative imports in this block to include .js
extensions (e.g., ../constants -> ../constants.js, ../hooks/useTaskQueries ->
../hooks/useTaskQueries.js, ./EmptyState -> ./EmptyState.js, ./PollingStatus ->
./PollingStatus.js, ./TagDropdown -> ./TagDropdown.js, ./TaskCard ->
./TaskCard.js, ./TaskEditModal -> ./TaskEditModal.js) so they follow the repo’s
TypeScript/ESM import convention.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fix silent mode leak on argument validation in rulesDirect

rulesDirect calls enableSilentMode() at the top (Line 37), but if required args are missing (the !action || !Array.isArray(profiles) || … check at Lines 40–45), the function returns immediately (Lines 46–53) without ever calling disableSilentMode(). That leaves silent mode enabled for all subsequent operations in this process, which breaks the “silent mode must always be restored” guarantee.

Other branches manually call disableSilentMode() before returning, but this early-return path does not, and duplicating disables across branches makes it easy to miss cases like this.

You can resolve this by:

• Having a single disableSilentMode() in a finally block.
• Removing the per-branch disableSilentMode() calls.

For example:

 export async function rulesDirect(args, log, context = {}) {
-	enableSilentMode();
-	try {
+	enableSilentMode();
+	try {
 		const { action, profiles, projectRoot, yes, force } = args;
 		if (
 			!action ||
 			!Array.isArray(profiles) ||
 			profiles.length === 0 ||
 			!projectRoot
 		) {
 			return {
 				success: false,
 				error: {
 					code: 'MISSING_ARGUMENT',
 					message: 'action, profiles, and projectRoot are required.'
 				}
 			};
 		}
@@
 			if (withNotices.length > 0) {
 				summary += ` Notices: ${withNotices.map((r) => `${r.profileName} - ${r.notice}`).join('; ')}.`;
 			}
-			disableSilentMode();
 			return {
 				success: errors.length === 0,
 				data: { summary, results: removalResults }
 			};
 		} else if (action === RULES_ACTIONS.ADD) {
@@
 			if (errors.length > 0) {
 				summary += errors
 					.map((r) => ` Error adding ${r.profileName}: ${r.error}`)
 					.join(' ');
 			}
-			disableSilentMode();
 			return {
 				success: errors.length === 0,
 				data: { summary, results: addResults }
 			};
 		} else {
-			disableSilentMode();
 			return {
 				success: false,
 				error: {
 					code: 'INVALID_ACTION',
 					message: `Unknown action. Use "${RULES_ACTIONS.ADD}" or "${RULES_ACTIONS.REMOVE}".`
 				}
 			};
 		}
 	} catch (error) {
-		disableSilentMode();
 		log.error(`[rulesDirect] Error: ${error.message}`);
 		return {
 			success: false,
 			error: {
 				code: error.code || 'RULES_ERROR',
 				message: error.message
 			}
 		};
+	} finally {
+		disableSilentMode();
 	}
 }

This ensures silent mode is always restored, including when arguments are invalid or an unexpected error occurs.

As per coding guidelines, direct functions must restore silent mode in a finally block to avoid leaking global state between invocations.

Also applies to: 36-209

🤖 Prompt for AI Agents

In mcp-server/src/core/direct-functions/rules.js around lines 36 to 209,
enableSilentMode() is called but the early-return argument-validation path
(lines ~40–53) and some other branches return without calling
disableSilentMode(), leaking silent mode; wrap the function body (from
immediately after enableSilentMode()) in a try block and add a single finally
block that calls disableSilentMode(), and remove the existing per-branch
disableSilentMode() calls so silent mode is always restored even on validation
failures or exceptions.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Use a try/finally block for silent mode and include createLogWrapper inside it

Right now enableSilentMode() is called before the try, and createLogWrapper(log) is also outside the try. If either of these throws, silent mode will stay enabled globally with no disableSilentMode() in a finally block, which violates the MCP/silent-mode guidelines and can mute subsequent logs. You’re also manually calling disableSilentMode() in several return paths instead of centralizing it.

Refactor to the standard pattern:

• Call enableSilentMode() just before the try.
• Move createLogWrapper(log) inside the try.
• Remove the scattered disableSilentMode() calls and restore silent mode in a single finally block.

For example:

 export async function scopeUpDirect(args, log, context = {}) {
-	// Destructure expected args
-	const {
-		tasksJsonPath,
-		id,
-		strength = 'regular',
-		prompt: customPrompt,
-		research = false,
-		projectRoot,
-		tag
-	} = args;
-	const { session } = context; // Destructure session from context
-
-	// Enable silent mode to prevent console logs from interfering with JSON response
-	enableSilentMode();
-
-	// Create logger wrapper using the utility
-	const mcpLog = createLogWrapper(log);
-
-	try {
+	const {
+		tasksJsonPath,
+		id,
+		strength = 'regular',
+		prompt: customPrompt,
+		research = false,
+		projectRoot,
+		tag
+	} = args;
+	const { session } = context;
+
+	// Enable silent mode to prevent console logs from interfering with JSON response
+	enableSilentMode();
+
+	try {
+		const mcpLog = createLogWrapper(log);
@@
-			log.error('scopeUpDirect called without tasksJsonPath');
-			disableSilentMode(); // Disable before returning
+			log.error('scopeUpDirect called without tasksJsonPath');
 			return {
@@
-			log.error('Missing required parameter: id');
-			disableSilentMode();
+			log.error('Missing required parameter: id');
 			return {
@@
-		// Restore normal logging
-		disableSilentMode();
-
 		return {
@@
-	} catch (error) {
-		// Make sure to restore normal logging even if there's an error
-		disableSilentMode();
-
+	} catch (error) {
 		log.error(`Error in scopeUpDirect: ${error.message}`);
 		return {
 			success: false,
 			error: {
 				code: error.code || 'SCOPE_UP_ERROR',
 				message: error.message
 			}
 		};
+	} finally {
+		// Always restore normal logging
+		disableSilentMode();
 	}
 }

This keeps behavior the same in normal paths while guaranteeing silent mode is always restored, even on unexpected sync errors. As per coding guidelines and learnings, silent mode restoration must be done via finally.

Also applies to: 41-52, 99-113

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find and check package.json for gpt-tokens version
find . -name "package.json" -type f | head -5

Repository: eyaltoledano/claude-task-master

Length of output: 226

---

🏁 Script executed:

# Check gpt-tokens in package.json
cat package.json | grep -A 2 -B 2 "gpt-tokens"

Repository: eyaltoledano/claude-task-master

Length of output: 202

---

🏁 Script executed:

# Read contextGatherer.js around the import and countTokens implementation
head -100 scripts/modules/utils/contextGatherer.js | cat -n

Repository: eyaltoledano/claude-task-master

Length of output: 3700

---

🏁 Script executed:

# Search for console.warn usage in contextGatherer.js
rg "console\." scripts/modules/utils/contextGatherer.js -n -C 2

Repository: eyaltoledano/claude-task-master

Length of output: 675

---

🏁 Script executed:

# Read the end of contextGatherer.js to check for default export
tail -50 scripts/modules/utils/contextGatherer.js | cat -n

Repository: eyaltoledano/claude-task-master

Length of output: 1604

---

🌐 Web query:

gpt-tokens npm package 1.3.14 API documentation encode function

💡 Result:

Short answer: gpt-tokens v1.3.14 does not publish a standalone encode(text) API like tokenizer libraries do. It's a token-usage / cost calculator (class GPTTokens) that depends on tokenizer libraries (js-tiktoken / openai-chat-tokens) to perform encoding. See the package README and npm listing. [1][2]

Sources:

• Package README / GitHub (gpt-tokens, v1.3.14 release info). [1]
• npm package page for gpt-tokens (v1.3.14). [2]

If you want, I can:

• locate the exact method in the repo that performs encoding (and paste its signature / example), or
• show how to call js-tiktoken / openai-chat-tokens directly to get an encode(text) -> number[] function. Which would you prefer?

---

🏁 Script executed:

# Check for log utility in utils
ls -la scripts/modules/utils/ | grep -E "\.js$"

Repository: eyaltoledano/claude-task-master

Length of output: 275

---

🏁 Script executed:

# Look for log function or logger in utils.js
rg "export.*log\|function log" scripts/modules/utils/utils.js -A 2

Repository: eyaltoledano/claude-task-master

Length of output: 145

---

🏁 Script executed:

# Check if contextGatherer is tested and if tests would catch the encode issue
find . -name "*contextGatherer*test*" -o -name "*contextGatherer*.spec.js" 2>/dev/null

Repository: eyaltoledano/claude-task-master

Length of output: 57

---

🏁 Script executed:

# Check scripts/modules/utils.js (parent directory)
head -100 scripts/modules/utils.js | cat -n

Repository: eyaltoledano/claude-task-master

Length of output: 4178

---

🏁 Script executed:

# Search for log function in scripts/modules/
rg "export.*log\|function log" scripts/modules/ -A 2 | head -50

Repository: eyaltoledano/claude-task-master

Length of output: 57

---

🏁 Script executed:

# Check where log is imported in other files in scripts/modules
rg "import.*log\|from.*log" scripts/modules/ -B 2 -A 2 | head -60

Repository: eyaltoledano/claude-task-master

Length of output: 57

---

🏁 Script executed:

# Search for log function export in scripts/modules/utils.js
rg "export.*log|^function log|^const log" scripts/modules/utils.js -A 3

Repository: eyaltoledano/claude-task-master

Length of output: 186

---

🏁 Script executed:

# Check how other files in scripts/modules import and use log
rg "import.*log.*from.*utils" scripts/modules/ -A 3 | head -40

Repository: eyaltoledano/claude-task-master

Length of output: 2278

---

The review comment is accurate. The gpt-tokens package version 1.3.14 does not export an encode function, confirming that token counting is broken and always falls back to character-based estimation.

I've verified the following:

1. gpt-tokens API: Package v1.3.14 is a token-usage cost calculator, not a tokenizer library. It does not export a standalone encode() function, so const { encode } = pkg results in encode === undefined.

2. Token counting failure: The countTokens() method at line 59 calls encode(text).length, which will throw a TypeError every time, causing the code to silently use the fallback character-length/4 estimation for all token counts.

3. Logging violations: The file uses console.warn directly at lines 42-44, 564-565, 690-692, and 732-734, which violates the coding guideline requiring centralized logging via the log utility (which is available and imported by other modules in the codebase).

4. Default export: Line 50 exports export default ContextGatherer;, which violates the guideline: "Do not use default exports in utility modules - use named exports only."

🤖 Prompt for AI Agents

In scripts/modules/utils/contextGatherer.js around lines 10 to 18 (and noting
related uses at ~42-44, 50, 56-60, 564-565, 690-692, 732-734), the file imports
and destructures encode from the gpt-tokens package which in v1.3.14 does not
provide an encode() function (breaking token counting), uses console.warn
directly (violates centralized logging), and uses a default export (violates the
named-export guideline); fix by replacing the broken tokenizer usage with a
proper tokenizer library (e.g., add and use @dqbd/tiktoken: create an encoder
via getEncoding or a relevant async factory and call encoder.encode(text).length
in countTokens with a safe fallback to Math.ceil(text.length/4) if encoder setup
fails), remove the const { encode } = pkg import, replace all console.warn calls
with the centralized log utility (import log from the shared logger module and
call log.warn(...)), and change export default ContextGatherer to a named export
(export const ContextGatherer = ... or export { ContextGatherer }) and update
any imports accordingly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# First, find the file and check its size
find . -name "parse-prd-tracker.js" -type f

# Get basic info about the file
if [ -f "src/progress/parse-prd-tracker.js" ]; then
  wc -l "src/progress/parse-prd-tracker.js"
fi

Repository: eyaltoledano/claude-task-master

Length of output: 148

---

🏁 Script executed:

#!/bin/bash

# Read the first 30 lines to see imports and constant definitions
head -30 "src/progress/parse-prd-tracker.js"

Repository: eyaltoledano/claude-task-master

Length of output: 896

---

🏁 Script executed:

#!/bin/bash

# Search for PRIORITY_COLORS usage in the entire file
echo "=== Searching for PRIORITY_COLORS in parse-prd-tracker.js ==="
rg -n "PRIORITY_COLORS" "src/progress/parse-prd-tracker.js"

echo ""
echo "=== Searching for PRIORITY_COLORS in entire codebase ==="
rg -n "PRIORITY_COLORS" --type=js

echo ""
echo "=== Searching for getPriorityColors usage in codebase ==="
rg -n "getPriorityColors" --type=js

Repository: eyaltoledano/claude-task-master

Length of output: 996

---

Remove unused import and constant: getPriorityColors and PRIORITY_COLORS.

Verification confirms that PRIORITY_COLORS is defined on line 19 via getPriorityColors() but is never referenced anywhere in src/progress/parse-prd-tracker.js. While a different PRIORITY_COLORS object exists and is actively used in src/ui/parse-prd.js, the one defined in this file appears to be dead code—likely from an incomplete refactoring.

Since it's not used within the file and has no external dependents, both the import and the constant should be removed to keep the code clean.

🤖 Prompt for AI Agents

In src/progress/parse-prd-tracker.js around lines 4 and 19, the file imports
getPriorityColors (line 4) and defines PRIORITY_COLORS = getPriorityColors()
(line 19) but that constant is never used; remove the unused import and the
PRIORITY_COLORS constant declaration to clean up dead code and ensure no other
code in this file references them before committing.

[coderabbitai]

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

scripts/modules/commands.js (1)

3518-3569: Init aliases options vs behavior and flag naming

The init command adds:

.option('--aliases', 'Add shell aliases (tm, taskmaster, hamster, ham)')
.option('--no-aliases', 'Skip shell aliases (tm, taskmaster, hamster, ham)')

but initializeProject(cmdOptions) doesn’t look at these flags; createProjectStructure is always called with addAliases = true on the init side. That means:

• --no-aliases currently has no effect (aliases are still attempted).
• --aliases doesn’t do anything that isn’t already the default.

Combined with the comment in init.js (“Always add aliases”), this is misleading.

Once you wire addAliases through initializeProject as suggested in the init.js comment, these flags will behave correctly. Before release, you may also want to reconsider --no-aliases in light of the repo guideline preferring positive --skip-* flags (e.g., --skip-aliases) to avoid negated boolean options.

♻️ Duplicate comments (9)

apps/cli/src/utils/auth-guard.ts (1)

156-158: Fix Biome formatting issue.

The CI pipeline reports a Biome format check failure at these lines. Run biome format . to fix the code style.

#!/bin/bash
# Fix formatting issues
biome format apps/cli/src/utils/auth-guard.ts --write

apps/cli/src/commands/auth.command.ts (3)

98-104: Capture and use the --no-header option in logout action.

The --no-header option is declared but the action handler doesn't capture or pass it to executeLogout. Users passing --no-header will see the option accepted but it will have no effect.

Apply this diff:

 private addLogoutCommand(): void {
 	this.command('logout')
 		.description('Logout and clear credentials')
 		.option('--no-header', 'Suppress the Task Master header banner')
-		.action(async () => {
-			await this.executeLogout();
+		.action(async (options?: { header?: boolean }) => {
+			await this.executeLogout(options?.header !== false);
 		});
 }

Then update executeLogout and downstream methods to accept and honor the showHeader parameter.

---

110-116: Capture and use the --no-header option in status action.

The --no-header option is declared but not captured or forwarded in the action handler.

Apply this diff:

 private addStatusCommand(): void {
 	this.command('status')
 		.description('Display authentication status')
 		.option('--no-header', 'Suppress the Task Master header banner')
-		.action(async () => {
-			await this.executeStatus();
+		.action(async (options?: { header?: boolean }) => {
+			await this.executeStatus(options?.header !== false);
 		});
 }

Then update executeStatus and displayStatus to accept and honor the showHeader parameter.

---

122-128: Capture and use the --no-header option in refresh action.

The --no-header option is declared but not captured or forwarded in the action handler.

Apply this diff:

 private addRefreshCommand(): void {
 	this.command('refresh')
 		.description('Refresh authentication token')
 		.option('--no-header', 'Suppress the Task Master header banner')
-		.action(async () => {
-			await this.executeRefresh();
+		.action(async (options?: { header?: boolean }) => {
+			await this.executeRefresh(options?.header !== false);
 		});
 }

Then update executeRefresh and refreshToken to accept and honor the showHeader parameter.

scripts/init.js (1)

767-805: updateStorageConfig should use shared JSON utilities and drops authCredentials

This helper still manually does fs.readFileSync/JSON.parse and fs.writeFileSync against config.json, and the authCredentials parameter is unused. Repo guidelines ask to route JSON I/O through readJSON/writeJSON from utils.js for consistent behavior and error handling, and an unused parameter is confusing for future maintainers.

A small refactor would align it:

-import fs from 'fs';
+import fs from 'fs';
@@
-import { isSilentMode } from './modules/utils.js';
+import { isSilentMode, readJSON, writeJSON } from './modules/utils.js';
@@
-function updateStorageConfig(configPath, selectedStorage, authCredentials) {
+function updateStorageConfig(configPath, selectedStorage /*, authCredentials */) {
   try {
@@
-    const configContent = fs.readFileSync(configPath, 'utf8');
-    const config = JSON.parse(configContent);
+    const config = readJSON(configPath);
@@
-    fs.writeFileSync(configPath, JSON.stringify(config, null, 2));
+    writeJSON(configPath, config);

(or equivalent, depending on existing utility signatures).

scripts/modules/commands.js (4)

1158-1317: update-task ID validation blocks subtasks and append mode is forced silently for Hamster

Two issues in this command:

1. ID validation vs docs and subtask IDs

You now only accept:

const validId =
  /^\d+$/.test(taskId) ||        // integer
  /^[a-z]+-\d+$/i.test(taskId);  // label-number

but:

• The argument help still advertises Task ID to update (e.g., 1, 1.1, TAS-123).
• Subtask IDs with dots (e.g. 1.2, HAM-31.1) are rejected, even though other parts of the system support dot notation and the PR description explicitly calls out subtask addressing.

If updateTaskById and ID-normalization utilities already understand subtask IDs, this will unnecessarily block valid inputs.

A minimal tweak that matches the help text:

- const validId =
-   /^\d+$/.test(taskId) || // plain positive integer
-   /^[a-z]+-\d+$/i.test(taskId); // label-number format (e.g., ham-123)
+ const validId =
+   /^\d+(\.\d+)?$/.test(taskId) || // e.g. "1" or "1.2"
+   /^[a-z]+-\d+(\.\d+)?$/i.test(taskId); // e.g. "HAM-31" or "HAM-31.1"

and update the error message to mention subtask formats explicitly.

2. Append mode is forced when connected to Hamster without explanation

This block:

// Force append mode when connected to Hamster
const shouldAppend = isConnectedToHamster()
  ? true
  : options.append || false;

silently overrides the user’s --append choice (or default) whenever Hamster is connected. That behavior was flagged previously as confusing.

If Hamster truly requires append mode, make it explicit:

- const shouldAppend = isConnectedToHamster()
-   ? true
-   : options.append || false;
+ const connectedToHamster = isConnectedToHamster();
+ const shouldAppend = connectedToHamster ? true : options.append || false;
+
+ if (connectedToHamster && !options.append) {
+   console.log(
+     chalk.blue(
+       'Note: When connected to Hamster, updates are always appended to task details.'
+     )
+   );
+ }

Alternatively, respect the user flag and just document the Hamster caveat in the --append description.

---

260-390: Hamster PRD path handling: undefined prdPath and missing file validation

handleParsePrdToHamster(prdPath) assumes a valid path and immediately does fs.readFileSync(prdPath, 'utf-8'), but the parse-prd action still calls it with the positional file:

const initOptions = { prdPath: file || options.input || true, ... };
// ...
const collaborationChoice = await promptHamsterCollaboration();
if (collaborationChoice === 'hamster') {
  await handleParsePrdToHamster(file);
}

If the user only provides --input, file is undefined, so prdPath becomes undefined and readFileSync will throw a low-level error. There’s also no explicit existence check or path echo, so failures are opaque. This was already raised in an earlier review and is still present.

You can fix both issues by:

1. Passing the resolved PRD path from taskMaster:

-      if (collaborationChoice === 'hamster') {
-        // User chose Hamster - send PRD to Hamster for brief creation
-        await handleParsePrdToHamster(file);
+      if (collaborationChoice === 'hamster') {
+        // User chose Hamster - send the resolved PRD path to Hamster
+        await handleParsePrdToHamster(taskMaster.getPrdPath());
         return;
       }

2. Guarding and validating inside handleParsePrdToHamster:

 async function handleParsePrdToHamster(prdPath) {
@@
-    // Read PRD file content
-    const prdContent = fs.readFileSync(prdPath, 'utf-8');
+    if (!prdPath) {
+      console.error(chalk.red('\n  PRD path is required to create a Hamster brief.\n'));
+      return;
+    }
+    if (!fs.existsSync(prdPath)) {
+      console.error(chalk.red(`\n  PRD file not found at: ${prdPath}\n`));
+      return;
+    }
+    const prdContent = fs.readFileSync(prdPath, 'utf-8');

This keeps both positional and --input flows working and surfaces clear errors when the PRD path is bad.

Also applies to: 972-977

---

179-253: Hamster collaboration prompt is mandatory and not connection-aware

promptHamsterCollaboration() always runs an inquirer prompt (unless stdin is non-TTY) and its docstring claims it is “only shown to users who are not already connected to Hamster,” but:

• It never checks isConnectedToHamster().
• The parse-prd action unconditionally calls it for every invocation.

This has two implications:

• There’s no way to script parse-prd non-interactively into Hamster or “local only” mode (previously flagged: missing --local / --hamster / --no-prompt style options).
• Users already connected to Hamster still see the full collaboration prompt every time, contrary to the comment and expected ergonomics.

Consider:

• Adding flags to bypass the prompt and choose the path directly (e.g., --local, --hamster, or a generic --no-collab-prompt), and
• Short‑circuiting when already connected:

 async function promptHamsterCollaboration() {
-  // Skip prompt in non-interactive mode only
-  if (!process.stdin.isTTY) {
-    return 'local';
-  }
+  // Skip prompt in non-interactive or Hamster-connected mode
+  if (!process.stdin.isTTY || isConnectedToHamster()) {
+    return isConnectedToHamster() ? 'hamster' : 'local';
+  }

and then honoring explicit CLI flags in the parse-prd action before falling back to the prompt.

Also applies to: 972-979

---

5127-5234: launchREPL uses undefined loadTUI, breaking tm tui / tm repl and default no-arg invocation

The new TUI/REPL integration adds:

programInstance
  .command('tui')
  .alias('repl')
  .action(async () => {
    await launchREPL();
  });

async function launchREPL() {
  const React = await import('react');
  const tui = await loadTUI();
  // ...
}

but there is no loadTUI function defined or imported in this module. This will throw a ReferenceError whenever:

• The user runs tm tui or tm repl, or
• runCLI is invoked with no arguments (argv.length <= 2), which now calls launchREPL() by default.

To keep TUI optional and avoid hard dependency failures, add a small helper that dynamically loads the TUI package and falls back gracefully when it’s not installed, then use it in launchREPL:

-import {
-  AuthDomain,
-  AuthManager,
-  CUSTOM_PROVIDERS,
-  createTmCore
-} from '@tm/core';
+import {
+  AuthDomain,
+  AuthManager,
+  CUSTOM_PROVIDERS,
+  createTmCore
+} from '@tm/core';
+
+/**
+ * Best-effort dynamic loader for the optional TUI package.
+ * Returns null if @tm/tui is not installed or fails to load.
+ */
+async function loadTUI() {
+  try {
+    return await import('@tm/tui');
+  } catch (error) {
+    if (process.env.DEBUG === '1') {
+      console.error('Failed to load TUI package (@tm/tui):', error);
+    }
+    return null;
+  }
+}
@@
 async function launchREPL() {
   const React = await import('react');
   const tui = await loadTUI();

With this in place, the existing fallback path (TUI mode not available. Install @tm/tui...) will work instead of crashing when the package isn’t present.

🧹 Nitpick comments (2)

scripts/init.js (1)

487-509: Address Biome formatting warnings for this file

CI reports Formatter would have reformatted this file around these lines. To keep the codebase consistent with the agreed style, please run:

biome format scripts/init.js
# or
biome format .

and commit the resulting formatting changes.

scripts/modules/commands.js (1)

356-359: Address Biome formatting warnings for commands.js

CI reports Formatter would have reformatted this file around these lines. To keep the CLI module consistent with the repository formatting rules, please run:

biome format scripts/modules/commands.js
# or
biome format .

and commit the changes.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get file size first
wc -l scripts/init.js

# Read the problematic sections
echo "=== Lines 360-395 (around 371-384) ==="
sed -n '360,395p' scripts/init.js

echo "=== Lines 700-730 (around 712-723) ==="
sed -n '700,730p' scripts/init.js

Repository: eyaltoledano/claude-task-master

Length of output: 2008

---

🏁 Script executed:

#!/bin/bash
# Search for the initializeProject function definition
echo "=== initializeProject function ==="
rg -n -A 30 'async function initializeProject' scripts/init.js

# Search for createProjectStructure calls
echo "=== createProjectStructure calls ==="
rg -n 'createProjectStructure' scripts/init.js

# Search for --aliases flag definition
echo "=== --aliases flag definition ==="
rg -n -B 5 -A 5 '\.option.*aliases' scripts/init.js

Repository: eyaltoledano/claude-task-master

Length of output: 1535

---

🏁 Script executed:

#!/bin/bash
# Search for init command definition in commands.js
echo "=== Searching for init command definition ==="
rg -n -B 5 -A 30 "\.command.*'init'" scripts/modules/commands.js

# Also search for .option calls in init command context
echo "=== Searching for .option in commands.js ==="
rg -n -B 2 -A 2 "\.option.*alias" scripts/modules/commands.js

Repository: eyaltoledano/claude-task-master

Length of output: 2404

---

🏁 Script executed:

#!/bin/bash
# Look at createProjectStructure function signature
echo "=== createProjectStructure function signature ==="
sed -n '894,920p' scripts/init.js

# Search for all options being used in options object
echo "=== Check if options.aliases is referenced anywhere ==="
rg -n 'options\.aliases' scripts/init.js

# Check createProjectStructure implementation to see how it uses addAliases param
echo "=== createProjectStructure implementation (first 50 lines) ==="
sed -n '894,944p' scripts/init.js

Repository: eyaltoledano/claude-task-master

Length of output: 2611

---

🏁 Script executed:

#!/bin/bash
# Check how the init command action handler processes and passes options
echo "=== Init command action handler (lines 3538-3570) ==="
sed -n '3538,3570p' scripts/modules/commands.js

# Check if initializeProject is called with cmdOptions
echo "=== Search for initializeProject call in commands.js ==="
rg -n -A 10 'initializeProject' scripts/modules/commands.js | head -40

Repository: eyaltoledano/claude-task-master

Length of output: 2128

---

CLI flag --aliases / --no-aliases are ignored; aliases are always added regardless of user intent

The flags ARE defined in the init command (scripts/modules/commands.js lines 3532–3533), and the full cmdOptions object with parsed flags is passed through to initializeProject(). However, both calls to createProjectStructure() hardcode true for the addAliases parameter:

• Line 375 (non-interactive path): await createProjectStructure(true, ...)
• Line 714 (interactive path): await createProjectStructure(true, ...)

This bypasses the user-provided flags entirely. When a user runs task-master init --no-aliases, the aliases are still added.

The fix mirrors the existing pattern for --git / --no-git (lines 301–305 in initializeProject):

 async function initializeProject(options = {}) {
+  // Handle boolean aliases flags
+  let addAliases = true; // Default
+  if (options.aliases === true) {
+    addAliases = true; // --aliases flag provided
+  } else if (options.noAliases === true || options.aliases === false) {
+    addAliases = false; // --no-aliases flag provided
+  }
+
   // ... existing code ...
   
   await createProjectStructure(
-    true, // Always add aliases
+    addAliases,
     initGit,

Apply the same change at line 714 in the interactive path.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In scripts/init.js around lines 371–384 (and also update the interactive-path
call at ~line 714), the call to createProjectStructure currently hardcodes the
first argument to true which ignores the user's --aliases / --no-aliases flag;
change these calls to pass a computed addAliases value that respects the parsed
options (e.g. derive const addAliases = (options && typeof options.aliases !==
'undefined') ? options.aliases : true) and replace the hardcoded true with
addAliases in both the non-interactive and interactive createProjectStructure
invocations so the CLI flag is honored.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

scripts/modules/commands.js (1)

2961-3087: remove-subtask help text still advertises --skip-generate while the option is --generate

The remove-subtask command defines:

.option('--generate', 'Regenerate task files after removing subtask')

but showRemoveSubtaskHelp() continues to document a --skip-generate flag:

'  --skip-generate     Skip regenerating task files\n\n'

This mismatch can easily confuse users about which flag actually exists.

Recommend aligning the help text with the current option surface, e.g.:

- '  --skip-generate     Skip regenerating task files\n\n' +
+ '  --generate          Regenerate task files after removing subtask\n\n' +

(or reintroduce a --skip-generate option if that’s intentionally the supported behavior and deprecate --generate).

Also applies to: 3095-3118

♻️ Duplicate comments (9)

apps/cli/src/commands/login.command.ts (1)

140-185: Refactor duplicated OAuth countdown logic into shared utility

The AUTH_TIMEOUT_MS + startCountdown/stopCountdown implementation here duplicates the countdown logic already present in the auth flows (e.g., auth-guard.ts, auth.command.ts). Centralizing this into a shared helper (such as a createAuthCountdown utility) would reduce duplication and keep countdown behavior consistent if it changes in the future.

Once the shared helper exists, this block could be replaced with something like:

const AUTH_TIMEOUT_MS = 10 * 60 * 1000;
const countdown = createAuthCountdown(AUTH_TIMEOUT_MS);

await this.authManager.authenticateWithOAuth({
  timeout: AUTH_TIMEOUT_MS,
  onWaitingForAuth: () => {
    // ...
    countdown.start();
  },
  onSuccess: () => countdown.stop(true),
  onError: () => countdown.stop(false),
});

// in finally
countdown.cleanup();

This keeps LoginCommand focused on wiring while reusing the same countdown behavior as other auth surfaces.

scripts/init.js (2)

758-810: Refactor updateStorageConfig to use shared JSON utilities; authCredentials currently unused

Functionally this works, but two issues remain:

1. It uses fs.readFileSync / JSON.parse / fs.writeFileSync directly instead of the shared readJSON / writeJSON utilities mandated for scripts, which centralize error handling and future behavior changes.
2. The authCredentials parameter is not used; if you don’t plan to persist anything derived from it, it should either be removed or explicitly documented as “reserved for future use.”

Suggested direction (pseudo‑diff, adapt to actual utility signatures and imports from ./modules/utils.js as per coding guidelines):

-import fs from 'fs';
+import fs from 'fs';
+import { readJSON, writeJSON } from './modules/utils.js';
@@
-function updateStorageConfig(configPath, selectedStorage, authCredentials) {
+function updateStorageConfig(configPath, selectedStorage /*, authCredentials */) {
 	try {
 		if (!fs.existsSync(configPath)) {
@@
-		const configContent = fs.readFileSync(configPath, 'utf8');
-		const config = JSON.parse(configContent);
+		const config = readJSON(configPath);
@@
-		fs.writeFileSync(configPath, JSON.stringify(config, null, 2));
+		writeJSON(configPath, config);

This keeps behavior the same while complying with the repo’s JSON‑access conventions.
Based on learnings, JSON I/O in scripts should consistently go through readJSON/writeJSON.

---

371-384: Honor --aliases / --no-aliases flags instead of hardcoding addAliases = true

Both non‑interactive and interactive paths still call createProjectStructure(true, ...), which ignores the user’s --aliases / --no-aliases intent. The init command already defines these flags and passes options through, so this effectively makes them no-ops. This was flagged previously and remains unresolved.

Consider deriving addAliases once from options at the top of initializeProject and using it in both calls:

 async function initializeProject(options = {}) {
+	// Handle aliases flags (default: add aliases)
+	let addAliases = true;
+	if (options.aliases === true) {
+		addAliases = true; // --aliases
+	} else if (options.noAliases === true || options.aliases === false) {
+		addAliases = false; // --no-aliases
+	}
@@ -371,7 +379,7 @@
-		await createProjectStructure(
-			true, // Always add aliases
+		await createProjectStructure(
+			addAliases,
 			initGit,
@@ -717,7 +725,7 @@
-			await createProjectStructure(
-				true, // Always add aliases
+			await createProjectStructure(
+				addAliases,
 				initGitPrompted,

(Adjust the exact noAliases property name if Commander maps it differently in commands.js.)
This will finally make the alias flags effective while preserving the default behavior of adding aliases when no flag is provided.

Also applies to: 717-727

scripts/modules/commands.js (6)

1185-1222: Forced append mode for Hamster in update-task is still silent and can surprise users

update-task now:

• Accepts richer ID formats (ham/tas-style) and positional arguments, which is good.
• Still forces append mode when isConnectedToHamster() is true:

const shouldAppend = isConnectedToHamster()
  ? true
  : options.append || false;

This silently overrides a user’s intent when they don’t pass --append, which is confusing—especially for users used to local behavior. This is the same issue called out previously.

Please either:

1. Emit a clear, one-time info message when append is being forced for Hamster (and ideally mention it in the --append help text), or
2. Respect the user’s flag and not force append, documenting any Hamster-specific constraints instead.

Also applies to: 1224-1242, 1244-1258, 1309-1322

---

184-256: parse-prd Hamster prompt is mandatory and ignores CLI flags or existing Hamster state

parse-prd always calls promptHamsterCollaboration() and has no --local / --hamster flags or other non-interactive escape hatch, even when a user is already connected to Hamster. This both reintroduces the concern from the earlier review and contradicts the comment that the prompt is “only shown to users who are not already connected to Hamster”. In addition, it forces an interactive choice in scripted/automated flows.

Consider:

• Adding explicit flags (e.g. --local, --hamster) to bypass the prompt; and
• Short‑circuiting the prompt when isConnectedToHamster() is already true (defaulting to Hamster or local as you decide), so the comment matches behavior.

Also applies to: 911-983

---

263-395: handleParsePrdToHamster still breaks for --input and missing PRD files

The Hamster PRD path handling is still fragile:

• parse-prd passes file into handleParsePrdToHamster(file), but when users call task-master parse-prd --input=requirements.txt (no positional arg), file is undefined.
• handleParsePrdToHamster(prdPath) then calls fs.readFileSync(prdPath, 'utf-8') with prdPath potentially undefined or pointing at a non-existent file, which results in a low-level error instead of a clear CLI message.
• There is still no existence check or explicit error for a missing PRD path, nor is the resolved path from taskMaster used, despite the previous review note.

This is a user-visible runtime bug for common --input usage and should be fixed before merge.

Suggested direction:

 async function handleParsePrdToHamster(prdPath) {
+  if (!prdPath) {
+    console.error(chalk.red('\n  PRD path is required to create a Hamster brief.\n'));
+    return;
+  }
@@
-    // Read PRD file content
-    const prdContent = fs.readFileSync(prdPath, 'utf-8');
+    // Read PRD file content
+    if (!fs.existsSync(prdPath)) {
+      console.error(chalk.red(`\n  PRD file not found at: ${prdPath}\n`));
+      return;
+    }
+    const prdContent = fs.readFileSync(prdPath, 'utf-8');

And in the parse-prd action, pass the resolved PRD path instead of the raw file arg:

-      if (collaborationChoice === 'hamster') {
-        // User chose Hamster - send PRD to Hamster for brief creation
-        await handleParsePrdToHamster(file);
+      if (collaborationChoice === 'hamster') {
+        // User chose Hamster - send the resolved PRD path to Hamster
+        await handleParsePrdToHamster(taskMaster.getPrdPath());
         return;
       }

Also applies to: 387-392

---

977-983: Hamster PRD branch should use resolved PRD path, not raw file argument

In addition to the missing guards in handleParsePrdToHamster, this call site still passes file instead of the resolved PRD path from taskMaster, so --input-only usage breaks the Hamster path. The branch should delegate taskMaster.getPrdPath() (which already encapsulates positional vs --input resolution).

See concrete diff in the earlier comment on handleParsePrdToHamster.

---

5132-5139: TUI/REPL entry uses undefined loadTUI – CLI will crash on tui/repl and with no args

launchREPL() calls await loadTUI() but there is no loadTUI defined or imported anywhere in this module. This is a hard runtime error:

• tm tui / tm repl will throw ReferenceError: loadTUI is not defined.
• runCLI() now calls launchREPL() whenever argv.length <= 2, so just running task-master without arguments will also crash instead of showing the TUI/help fallback.

This was highlighted in a previous review and remains unresolved.

You likely want a small local helper that dynamically imports the optional TUI package and returns null on failure, so the existing fallback path works:

@@
-import { registerCommands, setupCLI, runCLI, launchREPL };
+// Best-effort dynamic loader for optional TUI package
+async function loadTUI() {
+  try {
+    // Adjust specifier if your TUI module lives elsewhere
+    return await import('@tm/tui');
+  } catch (error) {
+    if (process.env.DEBUG === '1') {
+      console.error('Failed to load TUI package (@tm/tui):', error);
+    }
+    return null;
+  }
+}
+
+export { registerCommands, setupCLI, runCLI, launchREPL };

(Place loadTUI above launchREPL so it is in scope.)

Until this is fixed, the new TUI/REPL entry points (and the no-args CLI path) are effectively broken.

Also applies to: 5147-5239, 5283-5288, 5443-5443

---

5283-5295: Defaulting to REPL/TUI on no args is nice, but currently blocked by missing loadTUI

The new behavior in runCLI:

if (argv.length <= 2) {
  await launchREPL();
  return;
}

gives a good “just type tm to get an interactive experience” UX. Once loadTUI is defined and the TUI import is robust, this will be a strong default. For now, note that this path is part of the loadTUI critical issue described earlier.

🧹 Nitpick comments (6)

apps/cli/src/commands/login.command.ts (1)

192-203: Route user-facing output through the central logging/UI utilities instead of console.log

There are several direct console.log calls for user messages (URL display, browser-open fallback, auth status hints, MFA prompts). Project guidelines call for using the central logging/UI layer instead of raw console.log, even in CLI commands.

Where possible, please replace these with the existing UI/logging abstractions (e.g., the same utilities used for displayInfo, displaySuccess, displayError) so formatting, verbosity, and future telemetry hooks remain consistent across commands.

As per coding guidelines, avoid direct console.log in new code.

Also applies to: 206-214, 264-264, 298-300

scripts/init.js (3)

385-737: Interactive init flow + promptQuestion helper are well-structured; verify silent/MCP behavior

The storage selection, Hamster OAuth flow, Git prompts, AI IDE rules prompt, language prompt, and the new promptQuestion helper are nicely organized and align with the init design goals. The countdown + spinner wiring around authenticateWithOAuth also looks sound.

One thing to double‑check: in MCP / silent mode, initializeProject can still enter the interactive branch if options.yes, options.name, and options.description are unset, leading to readline prompts despite isSilentMode() being true. promptStorageSelection() short‑circuits to 'local' under silent mode, but the subsequent promptQuestion calls will still block on stdin.

You may want to either:

• Ensure commands.js always passes yes: true (or otherwise sets skipPrompts) when running under silent/MCP, or
• Add an early guard here such as if (isSilentMode()) { /* treat as non-interactive defaults */ } to avoid ever entering the interactive prompt path in silent mode.

This would fully align init with the project’s silent‑mode guidelines.

Also applies to: 740-756

---

812-896: Storage selection UX is good; consider using an Inquirer separator instead of a literal newline choice

The promptStorageSelection() flow looks great overall: it respects silent mode (defaulting to local), gives a clear narrative comparison between Solo (Taskmaster) and Together (Hamster), and safely falls back to local on prompt cancellation.

One small UX nit: the '\n' entry in choices becomes a real selectable option whose value is the string '\n'. If a user strikes Enter on it, storageType will be '\n', which will then be treated as “not cloud” and effectively mapped to local storage.

To make the intent explicit and avoid a “blank” selectable choice, you can swap that for an Inquirer separator:

-				choices: [
-					'\n',
+				choices: [
+					new inquirer.Separator(),
 					{

This keeps the visual spacing while ensuring only valid storage types can be selected.

---

898-964: createProjectStructure: storage/model/rules/language flows mostly solid; a few small polish items

The extended createProjectStructure signature and behavior (storage‑aware config, rules setup, response language, Hamster‑vs‑local model configuration, and tailored “getting started” guidance) align well with the PR goals. A few minor points to consider:

1. Respect silent mode for Hamster model info box
   In the model configuration block, the Hamster “AI Models Managed by Hamster” box (selectedStorage === 'cloud' && !dryRun) does not check isSilentMode(), so it will print even in MCP/silent mode whereas the local models setup is gated by !isSilentMode(). You might want to add !isSilentMode() to that branch as well to keep silent mode truly non‑verbose.

2. Robustness when reading hamster-art.txt
   The Hamster welcome path calls readAsset('hamster-art.txt', 'utf8') without the assetExists check or a try/catch used elsewhere in this file. If the asset is missing or mis‑packaged, init will crash instead of degrading gracefully. Mirroring the copyTemplateFile pattern (check assetExists or wrap in try/catch and log a warning) would make this more robust.

3. Alias DRY‑RUN log message is now slightly stale
   The aliases list now includes hamster and ham in addition to tm and taskmaster, but the DRY RUN log still says DRY RUN: Would add shell aliases (tm, taskmaster). It’s minor, but updating the message (or deriving it from the same alias array) would keep UX accurate.

For example:

-	} else if (addAliases && dryRun) {
-		log('debug', 'DRY RUN: Would add shell aliases (tm, taskmaster)');
-	}
+	} else if (addAliases && dryRun) {
+		log('debug', 'DRY RUN: Would add shell aliases (tm, taskmaster, hamster, ham)');
+	}

Overall, the new storage‑aware setup and tailored success/getting‑started messaging look good; these tweaks would mainly tighten edge‑case behavior and polish the UX.

Also applies to: 1046-1180, 1183-1192, 1194-1307

scripts/modules/commands.js (2)

2371-2431: clear-subtasks uses tag-aware readJSON correctly; consider basic error guard

clear-subtasks now:

• Resolves tag via taskMaster.getCurrentTag() and displays it.
• Uses readJSON(tasksPath, projectRoot, tag) to derive all IDs when --all is passed, then calls clearSubtasks() with { projectRoot, tag }.

The plumbing is good. As a future improvement, you might consider wrapping the readJSON call in a try/catch (similar to the research command) to produce a clearer error when the tasks file is missing or malformed, instead of letting a raw JSON/fs error bubble up.

---

3523-3574: init aliases flags conflict with “no --no-*” guideline

The new init options:

.option('--aliases', 'Add shell aliases (tm, taskmaster, hamster, ham)')
.option('--no-aliases', 'Skip shell aliases (tm, taskmaster, hamster, ham)')

add another --no-* negated flag alongside the existing --no-git / --no-git-tasks. Project guidelines prefer positive --skip-* flags to avoid double negatives.

Consider, when you next touch this command, migrating toward something like:

• --aliases (default on) and --skip-aliases (or --no-shell-aliases) instead of --no-aliases.

Given established usage of --no-git, I’d treat this as a consistency/cleanup refactor rather than a blocker.

[Crunchyman-ralph]

254 files viewed and reviewed, please address my current comments, part 2 review coming soon

[Crunchyman-ralph]

remove from gitignore please

[Crunchyman-ralph]

lol,

Seems like a duplicate of assets/hamster-art.txt, please remove

[Crunchyman-ralph]

lets not play around with the defaults here, there is only one required change here, and it is:
anonymousTelemetry: true

[Crunchyman-ralph]

Maybe you would rather move this type of info to ~/.taskmaster/<workspace-dir/state.json ? instead of in the project's git ?

[Crunchyman-ralph]

should be .md not .txt

[Crunchyman-ralph]

apps/cli is a thin interface layer, all this logic should live in tm-core

[Crunchyman-ralph]

does this work ? it just looks like its calling itself without really doing anything, shouldn't it call verifyMFA at some point ?

[Crunchyman-ralph]

don't understand what this does here, getApiBaseUrl does nothing vs getWebAppUrl, why the rename ?

[Crunchyman-ralph]

move to devDependencies

[Crunchyman-ralph]

does this add the alias ham and hamster ?

[Crunchyman-ralph]

part 2 review done, now I have the 18 files left that are the chunkiest, part 3 coming after a break

[Crunchyman-ralph]

I can totally see this being a part of tm-core as well, with the other files related to the export service.

[Crunchyman-ralph]

this is a perfect example of what should live in the apps/cli, its interactive, contains essential terminal ui components.

[Crunchyman-ralph]

why remove this ? just make it better and use it in displayAsciiBanner

[Crunchyman-ralph]

I believe we already have a utility function for this.

[Crunchyman-ralph]

rename to turnDown instead of TurnDownService,

Suggested change

	const turndownService = new TurndownService({
	headingStyle: 'atx',
	codeBlockStyle: 'fenced',
	bulletListMarker: '-'
	});
	const turndownService = new Turndown({
	headingStyle: 'atx',
	codeBlockStyle: 'fenced',
	bulletListMarker: '-'
	});

[Crunchyman-ralph]

Suggested change

	import TurndownService from 'turndown';
	import Turndown from 'turndown';

[Crunchyman-ralph]

bad file name for this function, I'd rather just put it in its own file

[Crunchyman-ralph]

Most detailed review going over the chunkiest parts of this PR, have some very deatiled reviews that I put into expandable comments, please go over them, the easiest thing to do here is just grab it and give each of the collapsible comments to 1 AI agent and let them handle it.

I notice a lot of code duplication, so those are the most important ones

[Crunchyman-ralph]

if removing "tryListTagsViaRemote" I think this is the only place where it is used, hence you could maybe remove it, but check twice where it is removed before removing it.

[Crunchyman-ralph]

watch out for those dynamic imports.

[Crunchyman-ralph]

use the utility function for terminalWidth

[Crunchyman-ralph]

also looks like a very useful and extractable utility that can be used by many of our views that use tables.

[Crunchyman-ralph]

no need for dynamic imports here, just import it at the top of the file

[Crunchyman-ralph]

Detailed summary of requested changes to give to AI

# Refactoring Plan: briefs.command.ts

File Overview

The briefs.command.ts file (732 lines) implements a CLI command for managing briefs (tags) in API storage mode. It provides:

• List briefs with interactive table selection
• Select briefs by ID, URL, or interactively
• Create briefs (redirect to web UI)

The command is well-structured as a class extending Commander's Command class, with proper separation of subcommands (list, select, create).

---

Current Issues

Issue 1: Dynamic require() Import (Line 314)

Severity: High

const Table = require('cli-table3');

This violates the codebase guidelines which require static top-level imports. The same import pattern is already used correctly elsewhere in the codebase:

• apps/cli/src/ui/display/tables.ts uses import Table from 'cli-table3';
• apps/cli/src/ui/components/task-detail.component.ts uses import Table from 'cli-table3';

Impact: Makes the code harder to analyze statically, prevents tree-shaking, and inconsistent with the rest of the codebase.

---

Issue 2: Duplicated Date Formatting Logic (Lines 342-347, 407-413)

Severity: Medium

The same date formatting pattern appears twice:

const updatedDate = tag.updatedAt
    ? new Date(tag.updatedAt).toLocaleDateString('en-US', {
            month: 'short',
            day: 'numeric',
            year: 'numeric'
        })
    : chalk.gray('N/A');

Additionally, apps/cli/src/utils/brief-selection.ts uses formatRelativeTime from @tm/core for similar display purposes. The inconsistency in date formatting approaches across the CLI creates a fragmented UX.

Impact: Code duplication, inconsistent date formatting across the CLI.

---

Issue 3: Duplicated Organization Selection Logic

Severity: Medium

The ensureOrgSelected() method (lines 252-308) duplicates logic found in context.command.ts's selectOrganization() method. Both methods:

1. Check if org is already selected
2. Fetch organizations from API
3. Auto-select if only one org exists
4. Prompt user to select from multiple orgs
5. Update context with selected org

The briefs command has ~56 lines of org selection logic that could leverage the existing implementation.

Impact: Code duplication, maintenance burden, potential for behavior drift between commands.

---

Issue 4: Large promptBriefSelection Method (Lines 432-563)

Severity: Medium

This method is 131 lines and handles multiple concerns:

1. Column width calculations
2. Table header creation
3. Choice building with separators
4. ESC key handling with raw keypress events
5. User prompt with inquirer
6. Context update after selection

The ESC key handling pattern (lines 485-517) with readline.emitKeypressEvents is complex and could be error-prone.

Impact: Long method with multiple responsibilities, hard to test individual pieces.

---

Issue 5: Inconsistent Authentication Checking

Severity: Low

The command uses two different patterns for auth checking:

• executeList() and executeCreate() use this.checkAuth() which shows a friendly card box
• executeSelect() and executeSelectFromUrl() use inline authManager.hasValidSession() with ui.displayError()

// Pattern 1 (in executeList/executeCreate):
if (!(await this.checkAuth())) {
    process.exit(1);
}

// Pattern 2 (in executeSelect/executeSelectFromUrl):
const hasSession = await this.authManager.hasValidSession();
if (!hasSession) {
    ui.displayError('Not authenticated. Run "tm auth login" first.');
    process.exit(1);
}

Impact: Inconsistent user experience, code duplication.

---

Issue 6: Unused Parameter (Line 524)

Severity: Low

private async executeSelectFromUrl(input: string): Promise<void> {

The method initializes tmCore but the _showHeader parameter pattern used in similar methods in context.command.ts is not present here, suggesting the API was designed for consistency but isn't being used.

---

Issue 7: formatBriefAsTableRow Helper (Lines 364-427)

Severity: Low

This 63-line private method handles complex string formatting with visual length calculations. While functional, it:

1. Mixes presentation logic (column widths, truncation)
2. Has no unit test coverage (private method)
3. Could be extracted to a testable utility in ui/formatters/

---

Existing Utilities to Reuse

1. cli-table3 Import Pattern

Location: apps/cli/src/ui/display/tables.ts

import Table from 'cli-table3';

Already imported at top-level in the tables utility.

2. formatRelativeTime from @tm/core

Location: packages/tm-core/src/utils/time.utils.ts

import { formatRelativeTime } from '@tm/core';

Already used in brief-selection.ts for consistent relative time display.

3. selectBriefInteractive and selectBriefFromInput

Location: apps/cli/src/utils/brief-selection.ts

These utilities are already correctly used for the select subcommand but NOT for the interactive selection in executeList(). The promptBriefSelection method in the command duplicates similar functionality.

4. checkAuthentication helper

Location: apps/cli/src/utils/auth-helpers.ts

Already used in some places but not consistently across all auth checks.

5. createTaskTable for table display

Location: apps/cli/src/ui/display/tables.ts

While not directly applicable (task vs brief), the pattern and table creation logic could inform a createBriefTable utility.

---

Refactoring Recommendations

Recommendation 1: Convert Dynamic Import to Static Import

Priority: High
Complexity: Simple
Risk: Low

Replace:

// Line 314 (inside displayBriefsTable)
const Table = require('cli-table3');

With (at top of file):

import Table from 'cli-table3';

Expected Benefits:

• Consistent with codebase patterns
• Enables static analysis and tree-shaking
• Follows ESM best practices

---

Recommendation 2: Extract Date Formatting Utility

Priority: Medium
Complexity: Simple
Risk: Low

Create a new utility function in apps/cli/src/ui/formatters/date-formatters.ts:

/**
 * Format a date for brief/table display
 * Returns "MMM D, YYYY" format (e.g., "Nov 28, 2025") or fallback
 */
export function formatBriefDate(
    date: string | Date | undefined | null,
    fallback: string = 'N/A'
): string {
    if (!date) return fallback;

    const dateObj = typeof date === 'string' ? new Date(date) : date;
    return dateObj.toLocaleDateString('en-US', {
        month: 'short',
        day: 'numeric',
        year: 'numeric'
    });
}

Then use consistently across briefs.command.ts and context.command.ts.

Alternative: Use formatRelativeTime from @tm/core for consistency with brief-selection.ts.

Expected Benefits:

• Single source of truth for date formatting
• Consistent UX across commands
• Easier to maintain and update

---

Recommendation 3: Extract Organization Selection Utility

Priority: Medium
Complexity: Moderate
Risk: Medium

Create a shared utility in apps/cli/src/utils/org-selection.ts:

export interface OrgSelectionResult {
    success: boolean;
    orgId?: string;
    orgName?: string;
    orgSlug?: string;
    message?: string;
}

/**
 * Ensure an organization is selected, prompting if necessary
 * Returns the selected org ID or null if selection failed/cancelled
 */
export async function ensureOrgSelected(
    authManager: AuthManager,
    options?: { autoSelectSingle?: boolean }
): Promise<OrgSelectionResult>;

/**
 * Interactive organization selection
 */
export async function selectOrgInteractive(
    authManager: AuthManager
): Promise<OrgSelectionResult>;

Then update both briefs.command.ts and context.command.ts to use this shared utility.

Expected Benefits:

• DRY principle - single implementation
• Consistent behavior across commands
• Easier testing of org selection logic

---

Recommendation 4: Standardize Authentication Checking

Priority: Medium
Complexity: Simple
Risk: Low

Replace all inline auth checks with the existing checkAuthentication helper:

// Instead of:
const hasSession = await this.authManager.hasValidSession();
if (!hasSession) {
    ui.displayError('Not authenticated. Run "tm auth login" first.');
    process.exit(1);
}

// Use:
if (!(await this.checkAuth())) {
    process.exit(1);
}

Or for more flexibility, use ensureAuthenticated from auth-guard.ts which provides OAuth flow triggering.

Expected Benefits:

• Consistent authentication UX
• Friendly error messages with suggested actions
• Reduced code duplication

---

Recommendation 5: Simplify promptBriefSelection Method

Priority: Low
Complexity: Moderate
Risk: Medium

Consider refactoring the promptBriefSelection method by:

1. Extract table row formatting to ui/formatters/brief-formatters.ts:

export function formatBriefTableRow(
    brief: TagInfo,
    colWidths: BriefColumnWidths
): string;

2. Extract ESC key handling to a reusable utility:

export function withEscapeCancel<T>(
    prompt: () => Promise<T>
): Promise<T | null>;

3. Consider using existing selectBriefInteractive from brief-selection.ts which already provides a different but functional brief selection UI.

Alternative Approach: Given that selectBriefInteractive already exists with a search-based interface, evaluate whether the table-based selection in promptBriefSelection provides enough additional value to justify maintaining two similar selection mechanisms.

Expected Benefits:

• Smaller, focused methods
• Testable formatting logic
• Reusable ESC handling pattern

---

Recommendation 6: Consider Consolidating Brief Selection UI

Priority: Low
Complexity: High
Risk: Medium

There are currently TWO brief selection interfaces:

1. selectBriefInteractive in brief-selection.ts - Search-based with status grouping
2. promptBriefSelection in briefs.command.ts - Table-based with inline selection

Consider whether both are necessary. If the table view in executeList is primarily for viewing (not selecting), the interactive selection should use the shared selectBriefInteractive utility.

Expected Benefits:

• Consistent selection UX
• Less code to maintain
• Single source of truth for brief selection logic

---

Summary Table

Issue	Priority	Complexity	Risk	Lines Affected
Dynamic require()	High	Simple	Low	1 line
Date formatting duplication	Medium	Simple	Low	~10 lines
Org selection duplication	Medium	Moderate	Medium	~56 lines
Auth checking inconsistency	Medium	Simple	Low	~8 lines
Long promptBriefSelection	Low	Moderate	Medium	~131 lines
formatBriefAsTableRow extraction	Low	Moderate	Low	~63 lines
Consolidate brief selection UI	Low	High	Medium	Architecture decision

---

Implementation Order

1. Phase 1 (Quick Wins):

   • Fix dynamic import (Recommendation 1)
   • Standardize auth checking (Recommendation 4)

2. Phase 2 (DRY Improvements):

   • Extract date formatting utility (Recommendation 2)
   • Extract org selection utility (Recommendation 3)

3. Phase 3 (Architecture):

   • Simplify promptBriefSelection (Recommendation 5)
   • Evaluate brief selection consolidation (Recommendation 6)

---

Notes

• The file follows good practices overall with proper TypeScript types, JSDoc comments, and clear separation of concerns
• Business logic is appropriately delegated to @tm/core (URL parsing, brief resolution)
• The command properly uses existing utilities like selectBriefFromInput and selectBriefInteractive for the select subcommand
• The architecture guidelines from CLAUDE.md are generally well-followed; no significant business logic violations were found

[Crunchyman-ralph]

Give AI

Refactoring Plan: export.service.ts

File: /packages/tm-core/src/modules/integration/services/export.service.ts
Lines: 1569
Date: 2025-11-28

---

1. File Overview

The ExportService handles exporting Task Master tasks to external systems (Hamster briefs). It provides functionality for:

• Exporting tasks to an existing brief (exportTasks)
• Generating a new brief from local tasks (generateBriefFromTasks)
• Generating a new brief from PRD content (generateBriefFromPrd)
• Polling brief generation status (getBriefStatus)
• Sending team invitations (sendTeamInvitations)

The service transforms local Task Master task formats into Hamster-compatible import formats and makes API calls to the Hamster backend.

---

2. Current Issues

Issue 2.1: Repeated AuthDomain Instantiation

Severity: High
Lines: 664, 1041, 1226, 1361, 1485

The file creates new AuthDomain() five separate times inside private methods:

• performExport() (line 664)
• callGenerateBriefEndpoint() (line 1041)
• callGenerateBriefFromPrdEndpoint() (line 1226)
• getBriefStatus() (line 1361)
• sendTeamInvitations() (line 1485)

Problem: Creates unnecessary object instances and violates DRY. The AuthDomain uses AuthManager.getInstance() internally, so this works but is wasteful and inconsistent.

Issue 2.2: Duplicated API Request Boilerplate

Severity: High
Lines: 1090-1159, 1272-1339, 1410-1458, 1513-1566

Four methods contain nearly identical patterns for:

1. Getting API base URL
2. Checking authentication
3. Making fetch request with auth headers
4. Checking content-type before JSON parsing
5. Extracting error messages from response
6. Returning success/error result objects

Example pattern repeated 4+ times:

const contentType = response.headers.get('content-type') || '';
if (!contentType.includes('application/json')) {
    const text = await response.text();
    return {
        success: false,
        error: {
            code: 'API_ERROR',
            message: `API returned non-JSON response...`
        }
    };
}

Issue 2.3: Duplicated Task Transformation Logic

Severity: High
Lines: 530-609 vs 886-958

Two methods contain almost identical logic for transforming tasks:

• transformTasksForBulkImport() - returns any[]
• transformTasksForImport() - returns ImportTask[]

Both methods:

1. Build valid ID sets identically
2. Filter dependencies identically
3. Process subtasks with the same logic
4. Call enrichDescription() the same way

The only differences are:

• Property names (parentExternalId vs parentId)
• Additional fields in bulk import (testStrategy, complexity on root)
• Return types

Issue 2.4: Duplicated Status/Priority Mapping

Severity: Medium
Lines: 642-653 vs 963-978

Two nearly identical status mapping functions:

• mapStatusForAPI() - returns string
• mapStatusForImport() - returns typed union

Both contain the same switch logic but with slight differences in output types. This is also duplicated from TaskMapper.mapStatus() in common/mappers/TaskMapper.ts.

Issue 2.5: Existing ApiClient Not Used

Severity: High
Lines: 692-708, 1091-1099, etc.

An ApiClient utility exists at packages/tm-core/src/modules/storage/utils/api-client.ts that:

• Handles authentication headers
• Provides proper error handling with TaskMasterError
• Has convenience methods (get, post, etc.)

But ExportService manually builds all fetch requests instead of using this existing utility.

Issue 2.6: Error Message Extraction Duplication

Severity: Medium
Lines: 465-466, 1149-1150, 1329-1330, 1449-1450, 1557-1558

Same error extraction pattern repeated 5 times:

const errorMessage = error instanceof Error ? error.message : String(error);

Issue 2.7: Large Type Definition Block

Severity: Medium
Lines: 18-365 (347 lines)

The file has 347 lines of type definitions before any class implementation. These should be extracted to a separate types file for:

• Better organization
• Reusability across other services
• Cleaner service file

Types that should be extracted:

• TaskImportResult, BulkTasksResponse
• ImportTask, GenerateBriefOptions
• InvitationResult
• GenerateBriefFromPrdOptions, GenerateBriefFromPrdResponse, GenerateBriefFromPrdResult
• BriefGenerationProgress, BriefStatusResponse, BriefStatusResult
• SendTeamInvitationsResponse, SendTeamInvitationsResult
• GenerateBriefResponse, GenerateBriefResult
• ExportTasksOptions, ExportResult
• Brief

Issue 2.8: Validation Logic Duplication

Severity: Medium
Lines: 381-388, 810-816, 1181-1188, 1349-1358, 1473-1481

Authentication check pattern repeated:

const isAuthenticated = await this.authManager.hasValidSession();
if (!isAuthenticated) {
    throw new TaskMasterError(
        'Authentication required...',
        ERROR_CODES.AUTHENTICATION_ERROR
    );
}

Issue 2.9: Organization Fallback Logic Duplication

Severity: Medium
Lines: 823-837 vs 1193-1207

Identical pattern for getting organization ID with fallback:

if (!orgId) {
    const organizations = await this.authManager.getOrganizations();
    if (organizations.length === 0) {
        return { success: false, error: { code: 'NO_ORGANIZATIONS', ... } };
    }
    orgId = organizations[0].id;
}

Issue 2.10: Brief ID Extraction - Complex Helper

Severity: Low
Lines: 736-799

The extractBriefId() and related helper methods (parseUrl, fromParts, isLikelyId) are useful but could be extracted to a shared utility for reuse.

---

3. Existing Utilities to Reuse

3.1: ApiClient (packages/tm-core/src/modules/storage/utils/api-client.ts)

• Handles auth headers automatically
• Provides proper error wrapping with TaskMasterError
• Has get(), post(), patch(), put(), delete() convenience methods
• Handles non-JSON error responses

3.2: TaskMapper (packages/tm-core/src/common/mappers/TaskMapper.ts)

• Already has mapStatus() for database-to-internal conversion
• Could be extended with inverse mappings for export

3.3: TaskMasterError (packages/tm-core/src/common/errors/task-master-error.ts)

• Rich error class with codes, context, and chaining
• Already imported but underutilized in catch blocks

3.4: AuthDomain (packages/tm-core/src/modules/auth/auth-domain.ts)

• Should be injected once at construction time, not instantiated repeatedly

---

4. Refactoring Recommendations

Recommendation 4.1: Extract Types to Dedicated File

Priority: High
Complexity: Simple
Risk: Low

Action: Create /packages/tm-core/src/modules/integration/types/export.types.ts

Move all interface/type definitions (lines 18-365) to this new file. Keep only the service class in export.service.ts.

Benefits:

• Service file drops from 1569 to ~1200 lines
• Types become reusable
• Better separation of concerns

Recommendation 4.2: Create Shared HTTP Client for Export

Priority: High
Complexity: Moderate
Risk: Low

Action: Either use the existing ApiClient or create a thin wrapper for export-specific needs.

// Option A: Use existing ApiClient
private apiClient: ApiClient;

constructor(configManager: ConfigManager, authManager: AuthManager) {
    this.configManager = configManager;
    this.authManager = authManager;

    const baseUrl = new AuthDomain().getApiBaseUrl();
    if (baseUrl) {
        this.apiClient = new ApiClient({
            baseUrl,
            authManager: authManager as any // Type mismatch needs resolution
        });
    }
}

// Option B: Create export-specific helper
private async makeApiRequest<T>(endpoint: string, options: RequestInit): Promise<T>

Benefits:

• Eliminates ~100 lines of duplicated fetch code
• Consistent error handling
• Single place to modify API behavior

Recommendation 4.3: Unify Task Transformation Methods

Priority: High
Complexity: Moderate
Risk: Medium

Action: Create a single transformation method with options:

private transformTasks(
    tasks: Task[],
    format: 'bulk-import' | 'import'
): ImportTask[] {
    // Shared logic here
}

Or better, extract to a dedicated transformer class:

// /packages/tm-core/src/modules/integration/mappers/task-export-mapper.ts
export class TaskExportMapper {
    static toImportFormat(tasks: Task[]): ImportTask[] { ... }
    static toBulkImportFormat(tasks: Task[]): BulkImportTask[] { ... }

    // Shared helpers
    private static buildValidIdSet(tasks: Task[]): Set<string> { ... }
    private static filterValidDependencies(deps: string[], validIds: Set<string>): string[] { ... }
    private static enrichDescription(task: Task | Subtask): string { ... }
    static mapStatus(status?: string): 'todo' | 'in_progress' | 'done' | 'blocked' { ... }
    static mapPriority(priority?: string): 'low' | 'medium' | 'high' | 'urgent' { ... }
}

Benefits:

• Eliminates ~150 lines of duplicated transformation code
• Single source of truth for mapping logic
• Easier to test in isolation

Recommendation 4.4: Inject AuthDomain via Constructor

Priority: High
Complexity: Simple
Risk: Low

Action: Accept AuthDomain instance in constructor instead of creating it repeatedly.

export class ExportService {
    private configManager: ConfigManager;
    private authManager: AuthManager;
    private authDomain: AuthDomain;

    constructor(
        configManager: ConfigManager,
        authManager: AuthManager,
        authDomain?: AuthDomain
    ) {
        this.configManager = configManager;
        this.authManager = authManager;
        this.authDomain = authDomain ?? new AuthDomain();
    }
}

Benefits:

• Single AuthDomain instance
• Better testability (can inject mocks)
• Cleaner private methods

Recommendation 4.5: Extract Auth Validation Helper

Priority: Medium
Complexity: Simple
Risk: Low

Action: Create private method for auth check:

private async requireAuth(): Promise<void> {
    const isAuthenticated = await this.authManager.hasValidSession();
    if (!isAuthenticated) {
        throw new TaskMasterError(
            'Authentication required',
            ERROR_CODES.AUTHENTICATION_ERROR
        );
    }
}

private async getRequiredOrgId(providedOrgId?: string): Promise<string> {
    const context = await this.authManager.getContext();
    let orgId = providedOrgId || context?.orgId;

    if (!orgId) {
        const organizations = await this.authManager.getOrganizations();
        if (organizations.length === 0) {
            throw new TaskMasterError(
                'No organizations available',
                ERROR_CODES.MISSING_CONFIGURATION
            );
        }
        orgId = organizations[0].id;
    }

    return orgId;
}

Benefits:

• Eliminates 5 instances of auth check boilerplate
• Consistent error messages
• Easier to modify auth behavior

Recommendation 4.6: Extract Error Utilities

Priority: Low
Complexity: Simple
Risk: Low

Action: Add utility function to common/utils or use existing pattern:

// Already exists in codebase - just use consistently
const errorMessage = error instanceof Error ? error.message : String(error);

// Or create typed helper
function getErrorMessage(error: unknown): string {
    return error instanceof Error ? error.message : String(error);
}

Benefits:

• Minor code reduction
• Consistent error handling

Recommendation 4.7: Extract Brief ID Parser

Priority: Low
Complexity: Simple
Risk: Low

Action: Move extractBriefId(), isLikelyId(), and related helpers to /packages/tm-core/src/common/utils/brief-id-parser.ts

export function extractBriefId(input: string): string | null { ... }
export function isValidBriefId(value: string): boolean { ... }

Benefits:

• Reusable across other services
• Easier to unit test
• Cleaner service class

---

5. Implementation Order

1. Phase 1 - Type Extraction (Low risk, high impact)

   • Create export.types.ts
   • Move all interfaces/types
   • Update imports

2. Phase 2 - AuthDomain Injection (Low risk)

   • Modify constructor
   • Update all method usages
   • Add to dependency injection chain

3. Phase 3 - Extract Helpers (Low risk)

   • requireAuth() method
   • getRequiredOrgId() method
   • Brief ID parser utilities

4. Phase 4 - Task Transformation Unification (Medium risk)

   • Create TaskExportMapper class
   • Migrate both transform methods
   • Add comprehensive tests before/after

5. Phase 5 - HTTP Client Integration (Medium risk)

   • Evaluate using existing ApiClient
   • Create wrapper if needed
   • Migrate all fetch calls

---

6. Expected Outcomes

Metric	Before	After (Estimated)
Lines of Code	1569	~900-1000
Type Definition Lines	347	0 (moved)
Duplicated API Call Blocks	4	0-1
Duplicated Transform Methods	2	1
AuthDomain Instantiations	5	1
Test Coverage Potential	Difficult	Improved

---

7. Files to Create/Modify

New Files:

• /packages/tm-core/src/modules/integration/types/export.types.ts
• /packages/tm-core/src/modules/integration/mappers/task-export-mapper.ts
• /packages/tm-core/src/common/utils/brief-id-parser.ts (optional)

Modified Files:

• /packages/tm-core/src/modules/integration/services/export.service.ts
• /packages/tm-core/src/modules/integration/index.ts (re-exports)

---

8. Testing Considerations

Before refactoring:

1. Ensure existing integration tests cover all public methods
2. Add unit tests for transformation logic
3. Document current API response formats

After refactoring:

1. All existing tests should pass unchanged
2. Add unit tests for extracted utilities
3. Add unit tests for TaskExportMapper

[Crunchyman-ralph]

Give AI

Refactoring Plan: parse-prd-to-hamster.ts

File Overview

Location: /apps/cli/src/hamster/parse-prd-to-hamster.ts

This file provides the parsePrdToHamster function that:

1. Authenticates the user via OAuth
2. Reads a PRD file from disk
3. Creates a brief on Hamster from the PRD content using tm-core
4. Polls for task generation completion
5. Prompts for team invitations
6. Sets the CLI context to the new brief

The file is approximately 330 lines and serves as a CLI presentation layer function.

---

Current Issues

1. Code Duplication with export.command.ts

Severity: High

Several patterns are duplicated between parse-prd-to-hamster.ts and export.command.ts:

a) showInviteUrl function (lines 301-311)

Both files have nearly identical implementations:

// parse-prd-to-hamster.ts (lines 302-310)
const urlMatch = briefUrl.match(/^(https?:\/\/[^/]+)\/home\/([^/]+)\/briefs\//);
if (urlMatch) {
    const [, baseUrl, orgSlug] = urlMatch;
    const membersUrl = `${baseUrl}/home/${orgSlug}/members`;
    // ...
}

// export.command.ts (lines 946-956)
// Nearly identical code

b) setContextToBrief function (lines 316-328)

Both files have identical implementations using selectBriefFromInput:

// parse-prd-to-hamster.ts (lines 316-328)
async function setContextToBrief(core: TmCore, briefUrl: string): Promise<void> {
    const authManager = (core.auth as any).authManager;
    await selectBriefFromInput(authManager, briefUrl, core);
}

// export.command.ts (lines 962-985)
// Same pattern with more error handling

c) Email validation and invitation prompts (lines 250-296)

Both files have nearly identical email validation logic:

// parse-prd-to-hamster.ts (lines 261-273)
const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
const invalid = emailList.filter((e) => !emailRegex.test(e));

// export.command.ts (lines 882-912)
// Same validation pattern

2. Polling Logic Belongs in tm-core

Severity: Medium

The brief status polling logic (lines 140-179) is business logic that should be in tm-core:

// Lines 140-179 - This is business logic, not presentation
while (briefStatus === 'generating' && Date.now() - startTime < maxWait) {
    await new Promise((resolve) => setTimeout(resolve, pollInterval));
    const briefInfo = await taskMasterCore.auth.getBrief(briefId);
    // ... status checking logic
}

This polling pattern:

• Uses magic numbers (120000ms timeout, 2000ms interval)
• Contains status transition logic (generating -> ready/complete/failed)
• Accesses plan_generation_status with type casting (briefInfo as any)

The IntegrationDomain already has getBriefStatus() method but it's not being used. The polling should be encapsulated in tm-core.

3. Type Casting / Unsafe Type Access

Severity: Medium

Several places use unsafe type casting:

// Line 155-157
const newStatus = (briefInfo as any).plan_generation_status as string;
taskCount = (briefInfo as any).taskCount || 0;

// Line 321
const authManager = (core.auth as any).authManager;

This indicates either:

• Missing type definitions in tm-core
• Architecture issue where internal properties need to be exposed

4. Unused Function Parameters

Severity: Low

The promptAndSendInvites function has unused parameters:

// Lines 250-253
async function promptAndSendInvites(
    _core: TmCore,      // Unused (prefixed with _)
    _briefId: string    // Unused (prefixed with _)
): Promise<void> {

The function only validates emails but doesn't actually send invitations. The comment on line 289-290 confirms this is incomplete:

// Note: We'd need to add an invite method to the integration domain
// For now, just show success - invites were sent with the initial request

5. Missing Integration with IntegrationDomain

Severity: Medium

The file doesn't use sendTeamInvitations from IntegrationDomain, even though it exists. This leads to:

• Incomplete functionality (invitations not actually sent)
• Duplicated patterns when proper integration would be cleaner

---

Existing Utilities to Reuse

1. From apps/cli/src/utils/

Utility	File	Current Usage	Opportunity
selectBriefFromInput	brief-selection.ts	Used	Could extract setContextToBrief there
displayError	error-handler.ts	Used	Already using
ensureAuthenticated	auth-guard.ts	Used	Already using
getProjectRoot	project-root.ts	Used	Already using

2. From packages/tm-core/

Utility	Location	Opportunity
BriefUrlParser	modules/briefs/utils/url-parser.ts	Use for URL parsing in showInviteUrl
getBriefStatus	IntegrationDomain	Use for polling logic
sendTeamInvitations	IntegrationDomain	Use for actual invitation sending

3. Missing Shared Utilities to Create

Utility	Description	Used By
validateEmails	Email validation + parsing	parse-prd-to-hamster.ts, export.command.ts
showInviteUrl	Extract org members URL from brief URL	parse-prd-to-hamster.ts, export.command.ts, scripts/modules/commands.js
setContextToBrief	Set CLI context to a brief	parse-prd-to-hamster.ts, export.command.ts
pollBriefGeneration	Poll brief status until ready (in tm-core)	parse-prd-to-hamster.ts

---

Refactoring Recommendations

1. Extract Shared UI Utilities

Priority: High
Complexity: Simple
Risk: Low

Create a new file apps/cli/src/utils/brief-utils.ts with shared functions:

// apps/cli/src/utils/brief-utils.ts

import chalk from 'chalk';
import { createUrlLink } from '../ui/index.js';
import { selectBriefFromInput } from './brief-selection.js';

/**
 * Extract members invite URL from brief URL
 */
export function getInviteUrl(briefUrl: string): string | null {
    const urlMatch = briefUrl.match(/^(https?:\/\/[^/]+)\/home\/([^/]+)\/briefs\//);
    if (urlMatch) {
        const [, baseUrl, orgSlug] = urlMatch;
        return `${baseUrl}/home/${orgSlug}/members`;
    }
    return null;
}

/**
 * Display invite URL for team members
 */
export function showInviteUrl(briefUrl: string): void {
    const membersUrl = getInviteUrl(briefUrl);
    if (membersUrl) {
        console.log(chalk.gray('  Invite teammates: ') + createUrlLink(membersUrl));
        console.log('');
    }
}

/**
 * Set CLI context to a brief (silently fails if unable)
 */
export async function setContextToBrief(
    tmCore: any,
    briefUrl: string
): Promise<void> {
    try {
        const authManager = (tmCore.auth as any).authManager;
        if (!authManager) return;
        await selectBriefFromInput(authManager, briefUrl, tmCore);
    } catch {
        // Silently fail - context setting is nice-to-have
    }
}

Implementation Steps:

1. Create apps/cli/src/utils/brief-utils.ts
2. Update apps/cli/src/utils/index.ts to export the new utilities
3. Update parse-prd-to-hamster.ts to import from brief-utils.ts
4. Update export.command.ts to import from brief-utils.ts
5. Remove duplicated functions from both files

2. Extract Email Validation Utility

Priority: Medium
Complexity: Simple
Risk: Low

Create a shared email validation utility:

// apps/cli/src/utils/email-utils.ts

const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
const MAX_INVITES = 10;

export interface EmailValidationResult {
    valid: string[];
    invalid: string[];
}

/**
 * Parse and validate comma-separated email addresses
 */
export function parseEmails(input: string, maxCount = MAX_INVITES): EmailValidationResult {
    if (!input.trim()) {
        return { valid: [], invalid: [] };
    }

    const emails = input.split(',').map(e => e.trim()).filter(Boolean);
    const truncated = emails.slice(0, maxCount);

    return {
        valid: truncated.filter(e => EMAIL_REGEX.test(e)),
        invalid: truncated.filter(e => !EMAIL_REGEX.test(e))
    };
}

/**
 * Inquirer validator for email input
 */
export function validateEmailInput(input: string): string | true {
    if (!input.trim()) return true;

    const { invalid } = parseEmails(input);
    if (invalid.length > 0) {
        return `Invalid email format: ${invalid.join(', ')}`;
    }

    const emailList = input.split(',').map(e => e.trim()).filter(Boolean);
    if (emailList.length > MAX_INVITES) {
        return `Maximum ${MAX_INVITES} email addresses allowed`;
    }

    return true;
}

Implementation Steps:

1. Create apps/cli/src/utils/email-utils.ts
2. Update apps/cli/src/utils/index.ts to export
3. Replace duplicated validation in both files

3. Move Polling Logic to tm-core

Priority: Medium
Complexity: Moderate
Risk: Medium

Add a polling method to IntegrationDomain:

// packages/tm-core/src/modules/integration/integration-domain.ts

interface PollBriefGenerationOptions {
    maxWaitMs?: number;
    pollIntervalMs?: number;
    onProgress?: (status: string, taskCount: number) => void;
}

interface PollBriefGenerationResult {
    success: boolean;
    status: 'ready' | 'complete' | 'failed' | 'timeout';
    taskCount: number;
    error?: string;
}

/**
 * Poll for brief generation completion
 */
async pollBriefGeneration(
    briefId: string,
    options?: PollBriefGenerationOptions
): Promise<PollBriefGenerationResult> {
    const maxWait = options?.maxWaitMs ?? 120000;
    const pollInterval = options?.pollIntervalMs ?? 2000;
    // ... implementation
}

Implementation Steps:

1. Add pollBriefGeneration method to IntegrationDomain
2. Add proper types for brief status (avoid any casts)
3. Update parse-prd-to-hamster.ts to use the new method
4. Move magic numbers to constants

4. Fix Invitation Functionality

Priority: Medium
Complexity: Moderate
Risk: Low

The promptAndSendInvites function is incomplete. Options:

Option A: Actually implement invitation sending using IntegrationDomain.sendTeamInvitations:

async function promptAndSendInvites(
    core: TmCore,
    briefUrl: string
): Promise<void> {
    const emails = await promptForEmails();
    if (emails.length === 0) return;

    const orgSlug = getOrgSlugFromUrl(briefUrl);
    if (!orgSlug) return;

    const result = await core.integration.sendTeamInvitations(orgSlug, emails);
    // ... display results
}

Option B: Remove the unused parameters and document the limitation:

async function promptAndSendInvites(): Promise<string[]> {
    // Returns validated emails for display purposes
    // Actual sending happens via brief creation
}

Recommended: Option A - implement proper invitation sending.

5. Add Proper Types for Brief Status

Priority: Low
Complexity: Simple
Risk: Low

Add explicit types to avoid as any casts:

// In tm-core types
interface BriefWithStatus extends Brief {
    plan_generation_status?: 'generating' | 'pending_plan' | 'ready' | 'complete' | 'failed';
    taskCount?: number;
}

Implementation Steps:

1. Update Brief type in tm-core to include optional status fields
2. Update AuthDomain.getBrief() return type
3. Remove type casts in parse-prd-to-hamster.ts

6. Expose AuthManager Properly (Architecture)

Priority: Low
Complexity: Moderate
Risk: Medium

The pattern (core.auth as any).authManager indicates an architecture issue. Options:

Option A: Add a proper getter in AuthDomain:

// In AuthDomain
public getAuthManager(): AuthManager {
    return this.authManager;
}

Option B: Move setContextToBrief functionality into AuthDomain:

// In AuthDomain
async setContextFromBrief(briefUrl: string): Promise<void> {
    // Use internal authManager
}

Recommended: Option B - keeps presentation layer clean.

---

Summary Table

Issue	Priority	Complexity	Risk	Effort
Extract shared UI utilities	High	Simple	Low	1-2 hours
Extract email validation	Medium	Simple	Low	30 min
Move polling to tm-core	Medium	Moderate	Medium	2-3 hours
Fix invitation functionality	Medium	Moderate	Low	1-2 hours
Add proper brief status types	Low	Simple	Low	30 min
Expose AuthManager properly	Low	Moderate	Medium	1-2 hours

---

Recommended Implementation Order

1. Extract shared UI utilities - Quick win, reduces duplication immediately
2. Extract email validation - Simple extraction, no behavior change
3. Add proper brief status types - Improves type safety
4. Fix invitation functionality - Completes incomplete feature
5. Move polling to tm-core - Architectural improvement
6. Expose AuthManager properly - Long-term architecture fix

---

Notes

• No dynamic imports found - all imports are proper static ES6 imports
• The file follows the architecture guidelines (presentation layer calling tm-core)
• The main function parsePrdToHamster is well-structured with clear phases
• Error handling is consistent using displayError utility
• The file is not overly complex (330 lines), but contains duplicated patterns

[Crunchyman-ralph]

Give AI

Refactoring Analysis: TriggerEvaluator

File: /packages/tm-core/src/modules/prompts/services/trigger-evaluator.ts
Analysis Date: 2025-11-28
Lines of Code: ~295 (excluding comments)

---

1. File Overview

The TriggerEvaluator class is responsible for evaluating trigger conditions to determine if upgrade prompts should be shown to users. It:

• Evaluates multiple trigger types (task_count, tags_used, list_count, etc.)
• Checks cooldown periods and dismissed states
• Returns structured results with reasons for decisions
• Prioritizes triggers and returns the highest-priority matching trigger

The file is well-structured and follows good separation of concerns. It is part of the prompts module within tm-core.

---

2. Current Issues

2.1 Code Duplication in evaluateThreshold Method (Medium)

Location: Lines 183-272

The evaluateThreshold method contains repetitive patterns for each trigger type. Each case follows the same structure:

case 'task_count':
    const taskMet = metrics.totalTaskCount >= trigger.threshold;
    return {
        met: taskMet,
        reason: taskMet
            ? `Task count ${metrics.totalTaskCount} >= ${trigger.threshold}`
            : `Task count ${metrics.totalTaskCount} < ${trigger.threshold}`
    };

This pattern is repeated 5 times for numeric threshold comparisons (task_count, tags_used, list_count, dependencies_complex, days_active), creating maintenance burden and potential for inconsistencies.

2.2 Switch Statement Scalability (Low)

Location: Lines 189-271

The large switch statement with 8 cases makes adding new trigger types require modifying the core evaluation logic. While currently manageable, this could become problematic as more trigger types are added.

2.3 Duplicated Data Fetching (Low)

Location: Lines 60-61 and Lines 101-102

Both evaluate() and evaluateTriggerType() methods independently fetch metrics and daysActive:

// In evaluate()
const metrics = await this.stateManager.getMetrics();
const daysActive = await this.stateManager.getDaysActive();

// In evaluateTriggerType()
const metrics = await this.stateManager.getMetrics();
const daysActive = await this.stateManager.getDaysActive();

While this is not a critical issue (the stateManager caches), it could be refactored for clarity.

2.4 Time Calculation Pattern Duplication (Low)

Location: Indirectly related - PromptStateManager at lines 238, 263

The pattern 24 * 60 * 60 * 1000 for milliseconds-per-day appears in prompt-state-manager.ts. Consider creating a shared constant or utility.

2.5 Missing Logger (Low)

Location: Throughout file

The TriggerEvaluator class does not use the shared logger (getLogger), unlike its sibling classes PromptStateManager and PromptService. Adding logging would improve debuggability.

---

3. Existing Utilities to Reuse

3.1 Logger Factory

Location: packages/tm-core/src/common/logger/index.ts

import { getLogger } from '../../../common/logger/index.js';

Both PromptStateManager and PromptService use this pattern. TriggerEvaluator should follow suit for consistency.

3.2 Time Utilities

Location: packages/tm-core/src/utils/time.utils.ts

Contains formatRelativeTime using date-fns. While not directly applicable, date-fns could be leveraged for day calculations if more complex time operations are needed.

3.3 Error Handling Pattern

Location: packages/tm-core/src/common/errors/task-master-error.ts

The TaskMasterError class with ERROR_CODES is available for structured error handling. Currently, the trigger evaluator doesn't throw errors but this could be useful for validation errors.

---

4. Refactoring Recommendations

4.1 Extract Numeric Threshold Evaluation Helper

Priority: Medium
Complexity: Simple
Risk: Low

Create a helper function to reduce duplication in numeric threshold comparisons:

/**
 * Helper type for threshold evaluation result
 */
interface ThresholdResult {
    met: boolean;
    reason: string;
}

/**
 * Evaluate a numeric threshold comparison
 */
private evaluateNumericThreshold(
    label: string,
    currentValue: number,
    threshold: number
): ThresholdResult {
    const met = currentValue >= threshold;
    return {
        met,
        reason: met
            ? `${label} ${currentValue} >= ${threshold}`
            : `${label} ${currentValue} < ${threshold}`
    };
}

Then refactor the switch cases:

case 'task_count':
    return this.evaluateNumericThreshold(
        'Task count',
        metrics.totalTaskCount,
        trigger.threshold
    );

case 'tags_used':
    return this.evaluateNumericThreshold(
        'Tag count',
        metrics.tagCount,
        trigger.threshold
    );

Benefits:

• Reduces ~40 lines of duplicated code
• Single place to modify threshold comparison logic
• Consistent message formatting

4.2 Add Logger Instance

Priority: Low
Complexity: Simple
Risk: Low

Add logging capability for debugging trigger evaluations:

import { getLogger } from '../../../common/logger/index.js';

export class TriggerEvaluator {
    private readonly logger = getLogger('TriggerEvaluator');
    private readonly stateManager: PromptStateManager;
    private readonly config: UpgradePromptConfig;

    // ... rest of class

    async evaluate(context: TriggerContext = {}): Promise<TriggerEvaluationResult> {
        this.logger.debug('Evaluating triggers', { context });

        if (!this.config.enabled) {
            this.logger.debug('Prompts are disabled');
            return {
                shouldShow: false,
                reason: 'Prompts are disabled'
            };
        }
        // ... rest of method
    }
}

Benefits:

• Consistent with other services in the module
• Easier debugging of trigger evaluation logic
• Better observability

4.3 Consider Strategy Pattern for Trigger Evaluation (Future)

Priority: Low
Complexity: Moderate
Risk: Medium

If many more trigger types are expected, consider refactoring to a strategy pattern. However, with only 8 trigger types currently, this may be over-engineering.

Current approach is acceptable because:

• 8 cases is manageable
• All logic is in one place for easy review
• Switch statement is type-safe with TypeScript

When to reconsider:

• If trigger types grow beyond 12-15
• If trigger evaluation logic becomes more complex per type
• If external plugins need to define custom triggers

4.4 Extract Shared Time Constant

Priority: Low
Complexity: Simple
Risk: Low

Create a shared constant for day-to-milliseconds conversion:

// In packages/tm-core/src/common/constants/time.ts
export const MS_PER_DAY = 24 * 60 * 60 * 1000;
export const MS_PER_HOUR = 60 * 60 * 1000;
export const MS_PER_MINUTE = 60 * 1000;

This would benefit both PromptStateManager (lines 238, 263) and any future time calculations.

4.5 Consider Adding Unit Tests

Priority: Medium
Complexity: Moderate
Risk: Low

No test file exists for the prompts module (packages/tm-core/src/modules/prompts/**/*.spec.ts returns no results). Adding tests would:

• Ensure threshold logic works correctly
• Document expected behavior
• Enable confident refactoring

Suggested test cases:

• Each trigger type evaluation
• Cooldown period calculations
• Dismissed state handling
• Priority ordering
• Edge cases (zero metrics, missing context)

---

5. Summary

Issue	Priority	Effort	Risk	Recommendation
Duplicated numeric threshold logic	Medium	Simple	Low	Extract helper method
Missing logger	Low	Simple	Low	Add logger instance
No unit tests	Medium	Moderate	Low	Add comprehensive tests
Time constant duplication	Low	Simple	Low	Create shared constant
Switch statement scalability	Low	N/A	N/A	Monitor, refactor if >12 types

---

6. No Issues Found

The following were checked and found to be acceptable:

• No dynamic imports: All imports are top-level static imports
• No require() statements: Using ES module imports throughout
• Architecture compliance: Business logic correctly placed in tm-core, not in presentation layers
• Error handling: Appropriate for non-throwing evaluation logic
• Type safety: Proper TypeScript types for all public interfaces
• File size: 295 lines is well within acceptable limits

---

7. Action Items

1. Immediate (Low effort, high value):

   • Add logger instance for debugging
   • Extract evaluateNumericThreshold helper method

2. Short-term:

   • Add unit tests for trigger evaluation logic
   • Create shared time constants in common/constants

3. Long-term (if needed):

   • Monitor trigger type growth; refactor to strategy pattern if >12 types

[Crunchyman-ralph]

Give AI

Refactoring Analysis: export.command.ts

File Overview

The export.command.ts file (1181 lines) implements the CLI export command for Task Master. It handles exporting local tasks to Hamster by creating new briefs. The command supports:

• Interactive tag selection for export
• Multi-tag batch exports
• Team invitation workflows
• Tracking of exported tags in state.json
• Auto-context switching after export

Location: /apps/cli/src/commands/export.command.ts

---

Current Issues

1. Duplicated getStatusIcon Function [High Priority]

Problem: The getStatusIcon method at lines 990-1002 duplicates logic that already exists in:

• /apps/cli/src/export/task-selector.ts (lines 118-131) - nearly identical function
• /apps/cli/src/ui/formatters/dependency-formatters.ts (lines 29-35) - similar status icon logic

// export.command.ts line 990
private getStatusIcon(status?: string): string {
    switch (status) {
        case 'done':
            return chalk.green('●');
        case 'in-progress':
        case 'in_progress':
            return chalk.yellow('◐');
        case 'blocked':
            return chalk.red('⊘');
        default:
            return chalk.gray('○');
    }
}

Impact: Three different implementations of status icons exist, with slight variations. This violates DRY and creates maintenance burden.

---

2. Repeated FileStorage Initialization Pattern [High Priority]

Problem: The pattern of checking projectRoot, creating FileStorage, and calling initialize() is repeated 4 times:

• Lines 232-235 (executeInteractiveTagSelection)
• Lines 373-376 (executeStandardExport)
• Lines 478-481 (executeInteractiveExport)
• Lines 699-701 (executeExportMultipleTags)

// This pattern appears 4 times:
const projectRoot = getProjectRoot();
if (!projectRoot) {
    console.log(chalk.yellow('\nNo project root found.\n'));
    this.lastResult = { success: false, action: 'cancelled', message: 'No project root' };
    return;
}
const fileStorage = new FileStorage(projectRoot);
await fileStorage.initialize();

Impact: ~50 lines of duplicated boilerplate across the file.

---

3. State.json Manipulation Should Be in tm-core [High Priority - Architecture Violation]

Problem: The methods getExportedTags (lines 1021-1034) and trackExportedTag (lines 1089-1127) directly read/write to .taskmaster/state.json. This is business logic that belongs in tm-core.

Evidence: tm-core already has RuntimeStateManager at /packages/tm-core/src/modules/config/services/runtime-state-manager.service.ts which handles state.json operations, including a metadata field designed for exactly this purpose.

// Current violation in CLI layer:
private async trackExportedTag(tagName: string, briefId: string, briefUrl: string): Promise<void> {
    const statePath = path.join(projectRoot, '.taskmaster', 'state.json');
    // Direct file manipulation...
}

Impact: Duplicates file I/O logic, bypasses tm-core's state management, makes testing difficult.

---

4. Similar Code Blocks in Export Methods [Medium Priority]

Problem: executeStandardExport (lines 357-458) and executeInteractiveExport (lines 463-641) share significant code:

• Both have identical spinner/result handling
• Both call the same generateBriefFromTasks API
• Both have identical success/error display logic
• Both track exported tags and set context identically

The only meaningful differences are:

• Interactive has task selection UI
• Interactive has preview confirmation

Impact: ~100 lines of near-duplicate code between these two methods.

---

5. Repeated Invitation Prompt Pattern [Medium Priority]

Problem: The invitation email prompt pattern appears 3 times with minor variations:

• Lines 393-396 (executeStandardExport)
• Lines 563-578 (executeInteractiveExport)
• Lines 659-675 (executeExportMultipleTags)

// Repeated 3 times with slight message variations:
const { wantsToInvite } = await inquirer.prompt<{ wantsToInvite: boolean }>([{
    type: 'confirm',
    name: 'wantsToInvite',
    message: 'Do you want to invite teammates...?',
    default: false
}]);
if (wantsToInvite) {
    inviteEmails = await this.promptForInviteEmails();
}

---

6. Result Setting Inconsistency [Low Priority]

Problem: The lastResult is set inconsistently across methods:

• Some paths set it before returning
• Some don't set it at all on error paths
• The action field has inconsistent values ('cancelled' vs 'validate')

---

7. Large File Size (Evaluation Threshold) [Medium Priority]

Problem: At 1181 lines, the file is above the evaluation threshold (1000 lines) for a single command file. This is not critical but indicates potential for better organization.

Comparison: Other command files are smaller:

• list.command.ts - focused on listing only
• briefs.command.ts - ~731 lines but handles multiple subcommands
• context.command.ts - focused on context management

---

Existing Utilities to Reuse

1. RuntimeStateManager (tm-core)

Location: /packages/tm-core/src/modules/config/services/runtime-state-manager.service.ts
Use for: Replace direct state.json manipulation with updateMetadata() method.

// RuntimeStateManager already supports metadata:
async updateMetadata(metadata: Record<string, unknown>): Promise<void>

2. STATUS_ICONS (tm-core)

Location: tm-core exports STATUS_ICONS constant
Use for: Replace duplicated getStatusIcon implementations.

3. displayCommandHeader (utils)

Location: /apps/cli/src/utils/display-helpers.ts
Use for: Standardized header display (already imported but not used).

4. Invitation Result Display Pattern

Location: The displayInvitationResults method in this file is well-structured and could be extracted to a shared UI utility.

---

Refactoring Recommendations

Recommendation 1: Extract Status Icon Utility [High Priority]

Approach: Create a single source of truth for task status icons in the CLI UI layer.

Implementation Steps:

1. Add getTaskStatusIcon function to /apps/cli/src/ui/formatters/status-formatters.ts
2. Export from /apps/cli/src/ui/index.ts
3. Replace all 3 implementations with the shared utility

Complexity: Simple
Risk: Low
Files Affected:

• apps/cli/src/ui/formatters/status-formatters.ts
• apps/cli/src/export/task-selector.ts
• apps/cli/src/commands/export.command.ts

Example:

// apps/cli/src/ui/formatters/status-formatters.ts
export function getTaskStatusIcon(status?: string, style: 'compact' | 'bullet' = 'bullet'): string {
    const normalizedStatus = status?.toLowerCase().replace('_', '-');
    switch (normalizedStatus) {
        case 'done':
        case 'completed':
            return style === 'compact' ? chalk.green('✓') : chalk.green('●');
        case 'in-progress':
            return style === 'compact' ? chalk.yellow('◐') : chalk.yellow('●');
        case 'blocked':
            return style === 'compact' ? chalk.red('✗') : chalk.red('⊘');
        default:
            return chalk.gray('○');
    }
}

---

Recommendation 2: Move State Tracking to tm-core [High Priority]

Approach: Move getExportedTags and trackExportedTag logic to tm-core, either in RuntimeStateManager or a new ExportStateService.

Implementation Steps:

1. Add getExportedTags() and trackExportedTag() methods to RuntimeStateManager
2. Expose through TmCore facade (e.g., tmCore.config.getExportedTags())
3. Replace direct file manipulation in export.command.ts with tm-core calls

Complexity: Moderate
Risk: Low
Files Affected:

• packages/tm-core/src/modules/config/services/runtime-state-manager.service.ts
• apps/cli/src/commands/export.command.ts

Example:

// In RuntimeStateManager:
async getExportedTags(): Promise<Record<string, ExportedTagInfo>> {
    const state = await this.loadState();
    return (state.metadata?.exportedTags as Record<string, ExportedTagInfo>) || {};
}

async trackExportedTag(tagName: string, briefId: string, briefUrl: string): Promise<void> {
    const exportedTags = await this.getExportedTags();
    exportedTags[tagName] = { briefId, briefUrl, exportedAt: new Date().toISOString() };
    await this.updateMetadata({ exportedTags });
}

---

Recommendation 3: Extract FileStorage Initialization Helper [Medium Priority]

Approach: Create a helper that handles the repeated projectRoot + FileStorage initialization pattern.

Implementation Steps:

1. Create getInitializedFileStorage() in /apps/cli/src/utils/storage-helpers.ts
2. Return { storage: FileStorage, projectRoot: string } or throw/return null pattern
3. Replace 4 instances in export.command.ts

Complexity: Simple
Risk: Low
Files Affected:

• New file: apps/cli/src/utils/storage-helpers.ts
• apps/cli/src/commands/export.command.ts

Example:

// apps/cli/src/utils/storage-helpers.ts
export async function getInitializedFileStorage(): Promise<{
    storage: FileStorage;
    projectRoot: string;
} | null> {
    const projectRoot = getProjectRoot();
    if (!projectRoot) {
        console.log(chalk.yellow('\nNo project root found.\n'));
        return null;
    }
    const storage = new FileStorage(projectRoot);
    await storage.initialize();
    return { storage, projectRoot };
}

---

Recommendation 4: Extract Common Export Result Handler [Medium Priority]

Approach: Create a helper function that handles the common success/failure display and tracking after export.

Implementation Steps:

1. Extract common post-export logic into a private method handleExportResult
2. Include: success display, invitation sending, invite URL display, context setting, tag tracking, metrics

Complexity: Moderate
Risk: Low
Files Affected:

• apps/cli/src/commands/export.command.ts (internal refactor)

Example:

private async handleExportResult(
    result: GenerateBriefResult,
    options: { tag?: string; inviteEmails: string[]; spinner?: Ora }
): Promise<boolean> {
    if (!result.success || !result.brief) {
        options.spinner?.fail('Export failed');
        const errorMsg = result.error?.message || 'Unknown error occurred';
        console.error(chalk.red(`\n${errorMsg}`));
        return false;
    }

    options.spinner?.succeed('Export complete');
    this.displaySuccessResult(result);

    if (options.inviteEmails.length > 0) {
        await this.sendInvitationsForBrief(result.brief.url, options.inviteEmails);
    }

    this.showInviteUrl(result.brief.url);
    await this.setContextToBrief(result.brief.url);
    await this.trackExportedTag(options.tag || 'master', result.brief.id, result.brief.url);
    await this.promptService?.recordAction('export_attempt', 'accepted');

    return true;
}

---

Recommendation 5: Extract Invitation Prompt Helper [Low Priority]

Approach: Create a helper for the "want to invite" confirmation + email collection flow.

Implementation Steps:

1. Create promptForTeamInvitations() function that combines the confirm + email prompts
2. Returns string[] (empty if user declines)

Complexity: Simple
Risk: Low

Example:

async function promptForTeamInvitations(
    message = 'Do you want to invite teammates to collaborate on these tasks together?'
): Promise<string[]> {
    const { wantsToInvite } = await inquirer.prompt([{
        type: 'confirm',
        name: 'wantsToInvite',
        message,
        default: false
    }]);

    if (!wantsToInvite) return [];
    return promptForInviteEmails();
}

---

Recommendation 6: Consider Splitting Multi-Tag Export [Low Priority]

Approach: If file continues to grow, consider extracting executeExportMultipleTags and related logic to a separate file.

Implementation Steps:

1. Create /apps/cli/src/commands/export/multi-tag-export.ts
2. Move executeExportMultipleTags and supporting types
3. Import and call from main ExportCommand

Complexity: Moderate
Risk: Low
Defer: This can be deferred unless other changes push the file significantly larger.

---

Priority Summary

Priority	Issue	Effort	Impact
High	Duplicated getStatusIcon	Simple	Eliminates 3 duplicates
High	State tracking in CLI (architecture)	Moderate	Proper separation of concerns
Medium	Repeated FileStorage init	Simple	~50 lines removed
Medium	Similar export methods	Moderate	~100 lines consolidated
Medium	File size (1181 lines)	-	Addressed by above
Low	Invitation prompt pattern	Simple	Minor DRY improvement
Low	Result setting inconsistency	Simple	Better error handling

---

Recommended Order of Implementation

1. First: Extract status icon utility (quick win, high impact)
2. Second: Move state tracking to tm-core (architecture fix)
3. Third: Extract FileStorage initialization helper
4. Fourth: Consolidate export result handling
5. Optional: Invitation prompt helper and file splitting if needed

---

Recommendation 7: Consolidate ExportTagCommand into ExportCommand [Medium Priority]

Analysis

Current State: Two separate command classes exist:

• ExportCommand (lines 70-1136) - Main export command with --tag <tag> option
• ExportTagCommand (lines 1143-1180) - Alias class that transforms tm export-tag <tag> into tm export --tag <tag>

Pattern Comparison with Codebase:

The codebase uses Commander.js .alias() for simple name aliases:

// list.command.ts - Uses .alias() for 'ls'
this.description('List tasks with optional filtering').alias('ls')

// briefs.command.ts - Uses .alias() for 'brief'
this.description('Manage briefs (API storage only)');
this.alias('brief');

// set-status.command.ts - Uses .alias() for 'status'
this.description('Update the status of one or more tasks').alias('status')

// autopilot/index.ts - Uses .alias() for 'ap'
this.description('AI agent orchestration...').alias('ap')

Key Difference: ExportTagCommand is NOT a simple alias. It provides:

1. A positional argument (<tag>) instead of a flag (--tag <tag>)
2. Slightly different UX: tm export-tag myTag vs tm export --tag myTag

Problems with Current Implementation:

1. Indirection complexity: Creates a new ExportCommand instance and calls parseAsync() - unusual pattern
2. Option duplication: --title and --description options are duplicated in both classes
3. Incomplete option parity: ExportTagCommand is missing the -I, --invite option that ExportCommand has
4. Extra class registration: Requires separate import, registration in command-registry.ts, and documentation
5. Maintenance burden: Changes to export options must be synchronized across both classes

Recommendation: Consolidate into ExportCommand

Verdict: YES, consolidate. The export-tag command provides marginal UX benefit (saves typing --tag) while adding maintenance complexity and risking option drift.

Approach: Use Commander.js native patterns to support both syntaxes within a single ExportCommand:

// Option A: Use .alias() with optional argument
// Supports: tm export [tag], tm export --tag <tag>, tm exp <tag>
export class ExportCommand extends Command {
    constructor(name?: string) {
        super(name || 'export');

        this.alias('exp')  // Short alias
            .description('Export tasks to Hamster by creating a new brief')
            .argument('[tag]', 'Tag to export (alternative to --tag)')
            .option('--tag <tag>', 'Export tasks from a specific tag')
            .option('--title <title>', 'Specify a title for the generated brief')
            .option('--description <description>', 'Specify a description')
            .option('-I, --invite', 'Prompt for email addresses to invite collaborators')
            .action(async (tag?: string, options?: any) => {
                // Merge positional tag with --tag option (positional takes precedence)
                const effectiveTag = tag || options?.tag;
                await this.executeExport({ ...options, tag: effectiveTag });
            });
    }
}

Alternative Approach: If the export-tag command name must be preserved for backward compatibility:

// Option B: Register same command with different name
// In command-registry.ts or program setup:
const exportCmd = new ExportCommand('export');
program.addCommand(exportCmd);

const exportTagAlias = new ExportCommand('export-tag');
exportTagAlias.description('Export a specific tag to Hamster (alias for: tm export [tag])');
program.addCommand(exportTagAlias);

Implementation Steps

Phase 1: Add positional argument to ExportCommand

1. Add optional [tag] positional argument to ExportCommand
2. Merge positional argument with --tag option in action handler
3. Update help text to document both syntaxes

Phase 2: Remove ExportTagCommand

1. Remove ExportTagCommand class from export.command.ts
2. Remove export from index.ts if present
3. Remove from command-registry.ts
4. Update any tests referencing ExportTagCommand

Phase 3: (Optional) Add backward-compatible alias
If tm export-tag must still work:

1. Add .alias('export-tag') to ExportCommand
2. Or register ExportCommand twice with different names (less elegant)

Migration Path

Before:

tm export-tag myTag                    # Works via ExportTagCommand
tm export --tag myTag                  # Works via ExportCommand
tm export                              # Interactive mode

After (recommended):

tm export myTag                        # Positional argument (new, preferred)
tm export --tag myTag                  # Still works (option syntax)
tm export                              # Interactive mode (unchanged)
tm exp myTag                           # Short alias (optional addition)

Complexity and Risk

Complexity: Simple
Risk: Low - positional arguments in Commander.js are well-supported
Breaking Change Risk: Medium if export-tag command is removed without alias

Benefits

1. Eliminates 38 lines of duplicated command class
2. Removes option drift risk - single source of truth for options
3. Simpler codebase - one class instead of two
4. Better UX - tm export myTag is cleaner than tm export-tag myTag
5. Follows codebase patterns - consistent with how other commands use .alias()

Decision Matrix

Factor	Keep Separate	Consolidate
Maintenance	Harder (sync options)	Easier (one class)
Codebase consistency	Unique pattern	Matches other commands
UX improvement	None	Cleaner positional syntax
Breaking changes	None	Possible if export-tag removed
Lines of code	+38 lines	-38 lines

Recommendation: Consolidate with backward-compatible alias.

---

Updated Priority Summary

Priority	Issue	Effort	Impact
High	Duplicated getStatusIcon	Simple	Eliminates 3 duplicates
High	State tracking in CLI (architecture)	Moderate	Proper separation of concerns
Medium	Repeated FileStorage init	Simple	~50 lines removed
Medium	Similar export methods	Moderate	~100 lines consolidated
Medium	ExportTagCommand consolidation	Simple	38 lines removed, better UX
Medium	File size (1181 lines)	-	Addressed by above
Low	Invitation prompt pattern	Simple	Minor DRY improvement
Low	Result setting inconsistency	Simple	Better error handling

---

Notes

• All changes should maintain backward compatibility with existing CLI behavior
• Consider adding unit tests when extracting utilities to tm-core
• If export-tag command must be preserved, use .alias('export-tag') on the consolidated ExportCommand

[cursor]

Bug: CLI --no-header option added but not captured in subcommands

The --no-header option is added to the org, brief, clear, and set subcommands but the action callbacks don't capture the options parameter. In Commander.js, when a command has both .argument() and .option(), the action receives arguments first, then options. The current action handlers only accept the argument (e.g., async (orgId?: string) => {...}), so the options object containing header is never received. Users can pass --no-header without error and see it in help text, but the flag has no effect. The action signature needs to include the options parameter to access the flag value.

Additional Locations (2)

• apps/cli/src/commands/context.command.ts#L96-L100
• apps/cli/src/commands/context.command.ts#L108-L112

 

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (4)

apps/cli/src/utils/auth-guard.ts (1)

222-229: Use UI helpers for consistent error display.

Lines 224 and 226 use raw console.log(chalk.*) instead of the UI helpers available in ./ui.js. This was flagged in a previous review for consistency.

 if (!result.authenticated) {
   if (result.cancelled) {
-    console.log(chalk.yellow('\nOperation cancelled.\n'));
+    ui.displayWarning('Operation cancelled.');
   } else if (result.error) {
-    console.log(chalk.red(`\nAuthentication failed: ${result.error}\n`));
+    ui.displayError(`Authentication failed: ${result.error}`);
   }
   process.exit(1);
 }

Add import * as ui from './ui.js'; at the top.

apps/cli/src/commands/auth.command.ts (1)

105-136: --no-header option declared but still not used in logout/status/refresh.

The option is declared and captured in _options (lines 109, 121, 133), but:

1. The underscore prefix indicates the parameter is intentionally unused
2. The execute methods (executeLogout, executeStatus, executeRefresh) don't accept a showHeader parameter

If header suppression is intended for these commands, forward the option:

-.action(async (_options?: { header?: boolean }) => {
-  await this.executeLogout();
+.action(async (options?: { header?: boolean }) => {
+  await this.executeLogout(options?.header !== false);
 });

And update the execute method signatures accordingly. Otherwise, remove the --no-header option from these commands to avoid user confusion.

scripts/init.js (2)

378-379: CLI flag --aliases / --no-aliases still ignored.

The createProjectStructure calls still hardcode true for the addAliases parameter at lines 378-379 and 713-714, bypassing user-provided flags entirely. This was flagged in a previous review.

Apply a fix similar to the --git / --no-git handling pattern at lines 303-308:

+  // Handle boolean aliases flags
+  let addAliases = true; // Default
+  if (options.aliases === false || options.noAliases === true) {
+    addAliases = false;
+  }
+
   await createProjectStructure(
-    true, // Always add aliases
+    addAliases,
     initGit,

Apply the same at line 713.

---

766-804: Use readJSON/writeJSON utilities instead of raw fs operations.

Per the coding guidelines, config JSON reads/writes should go through the shared readJSON/writeJSON utilities for consistent error handling. The authCredentials parameter is also unused.

-function updateStorageConfig(configPath, selectedStorage, authCredentials) {
+function updateStorageConfig(configPath, selectedStorage /*, authCredentials */) {
   try {
     if (!fs.existsSync(configPath)) {
       log('warn', 'Config file does not exist, skipping storage configuration');
       return;
     }
 
-    const configContent = fs.readFileSync(configPath, 'utf8');
-    const config = JSON.parse(configContent);
+    const config = readJSON(configPath);
+    if (!config) {
+      log('warn', 'Could not read config file');
+      return;
+    }
     // ... rest of config updates
-    fs.writeFileSync(configPath, JSON.stringify(config, null, 2));
+    writeJSON(configPath, config);

Import readJSON and writeJSON from ./modules/utils.js.

🧹 Nitpick comments (1)

scripts/init.js (1)

436-476: Consider extracting countdown timer to shared utility.

This countdown timer implementation (lines 436-476) duplicates the AuthCountdownTimer class now available in apps/cli/src/utils/auth-ui.ts. While this file is in the legacy scripts/ layer, consider importing or extracting a shared countdown utility to reduce maintenance burden when behavior needs to change.

For now, the implementation is functional and includes proper cleanup in stopCountdown.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify AUTH_TIMEOUT_MS duplication in codebase
rg -n "AUTH_TIMEOUT_MS\s*=" --type=js --type=ts

Repository: eyaltoledano/claude-task-master

Length of output: 245

---

🏁 Script executed:

#!/bin/bash
# Check imports in commands.js and context around line 294
head -30 scripts/modules/commands.js | grep -i "import\|require\|auth"
echo "---"
sed -n '290,300p' scripts/modules/commands.js

Repository: eyaltoledano/claude-task-master

Length of output: 664

---

🏁 Script executed:

# Current state
scripts/modules/commands.js:294: const AUTH_TIMEOUT_MS = 10 * 60 * 1000;
packages/tm-core/src/modules/auth/constants.ts:9: export const AUTH_TIMEOUT_MS = 10 * 60 * 1000;

Repository: eyaltoledano/claude-task-master

Length of output: 251

---

Consolidate AUTH_TIMEOUT_MS to single source of truth in constants.ts; remove local redefinition in commands.js.

The duplication is confirmed. The constant AUTH_TIMEOUT_MS is defined locally in scripts/modules/commands.js:294 but is already exported from packages/tm-core/src/modules/auth/constants.ts:9. The commands.js file should import and use the exported constant instead of redefining it locally to eliminate maintenance risk and ensure a single source of truth.

🤖 Prompt for AI Agents

In packages/tm-core/src/modules/auth/constants.ts around lines 5 to 14,
AUTH_TIMEOUT_MS is defined but also redefined in scripts/modules/commands.js at
line ~294; remove the local definition in commands.js and import AUTH_TIMEOUT_MS
from "packages/tm-core/src/modules/auth/constants" (use the correct relative
path from scripts/modules/commands.js), then replace all local references with
the imported constant so the file uses the single exported source of truth and
no duplicate constant remains.

[Crunchyman-ralph]

1 more nit, and I'm worried lots of other parts of my comments didn't make it to your recent commit.

[Crunchyman-ralph]

ui-related even utils, should go in apps/cli/src/ui