fix: improve auth-related error handling#1477
Conversation
🦋 Changeset detectedLatest commit: ae8c8f9 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds shared Supabase authentication error utilities and re-exports, updates SupabaseAuthClient with singleton helpers plus recoverable-error detection, retry and clear-session handling for stale-session/MFA errors, and extends the CLI error handler to map/display auth errors with optional stack traces. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (error-handler)
participant Auth as SupabaseAuthClient
participant Supabase as Supabase API
User->>CLI: invoke auth action (e.g., MFA token)
CLI->>Auth: verifyOneTimeCode(token)
Auth->>Supabase: verify token / refresh session
alt Supabase returns recoverable stale-session error
Supabase-->>Auth: AuthError (recoverable)
Note right of Auth: isRecoverableStaleSessionError -> clear stored session
Auth->>Supabase: retry verifyOneTimeCode (isRetry = true)
alt retry success
Supabase-->>Auth: Session
Auth-->>CLI: Session
else retry fails
Supabase-->>Auth: AuthError
Auth-->>CLI: throws AuthenticationError
end
else Supabase returns other auth error
Supabase-->>Auth: AuthError (other)
Auth-->>CLI: throws AuthenticationError (via toAuthenticationError)
else Success
Supabase-->>Auth: Session
Auth-->>CLI: Session
end
alt CLI receives AuthenticationError or Supabase auth error
CLI->>CLI: isSupabaseAuthError / map via AUTH_ERROR_MESSAGES
CLI-->>User: user-friendly message (stack if debug/forceStack)
else Success
CLI-->>User: success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/cli/src/utils/error-handler.ts (1)
19-50: Duplicate code:isSupabaseAuthErrorandAUTH_ERROR_MESSAGESexist in both CLI and core.These utilities are duplicated in
packages/tm-core/src/modules/integration/clients/supabase-client.ts. Consider exporting them from@tm/coreand importing here to maintain a single source of truth.If keeping them separate is intentional (e.g., to avoid coupling CLI to internal Supabase client details), consider at minimum extracting to a shared error utilities module within the core package that can be imported by both.
packages/tm-core/src/modules/integration/clients/supabase-client.ts (2)
462-523: Retry logic inverifyOneTimeCodeis sound but verify recursive depth.The retry mechanism for stale session recovery is well-implemented with:
- Single retry limit via
isRetryflag- Session clearing before retry
- Proper error propagation on final failure
However, the retry logic appears in two places (lines 476-484 and 506-514) handling the same recoverable error scenario. This duplication could be consolidated.
Consider extracting the retry logic to reduce duplication:
async verifyOneTimeCode(token: string, isRetry = false): Promise<Session> { const client = this.getClient(); + const handleRecoverableError = async (error: unknown): Promise<Session | null> => { + if (!isRetry && isRecoverableStaleSessionError(error)) { + this.logger.debug( + 'MFA-expected error during token verification, clearing stale session and retrying' + ); + await this.sessionStorage.clear(); + return this.verifyOneTimeCode(token, true); + } + return null; + }; + try { // ... verification logic if (error) { - if (!isRetry && isRecoverableStaleSessionError(error)) { - this.logger.debug( - 'MFA-expected error during token verification, clearing stale session and retrying' - ); - await this.sessionStorage.clear(); - return this.verifyOneTimeCode(token, true); - } + const retryResult = await handleRecoverableError(error); + if (retryResult) return retryResult; // ... rest of error handling } } catch (error) { - if (isSupabaseAuthError(error)) { - if (!isRetry && isRecoverableStaleSessionError(error)) { - // ... same retry logic - } + if (isSupabaseAuthError(error)) { + const retryResult = await handleRecoverableError(error); + if (retryResult) return retryResult; + // ... rest of error handling } } }
76-100: The current fallback pattern already handles unmapped Supabase error codes gracefully.The
toAuthenticationErrorfunction is specifically designed to convert SupabaseAuthErrorobjects to user-friendlyAuthenticationErrors for session and credential errors. It includes a fallback (line 82): if a code isn't inAUTH_ERROR_MESSAGES, it displays${defaultMessage}: ${error.message}, so unmapped codes are already handled.Additionally,
MFA_VERIFICATION_FAILEDis intentionally created as a directAuthenticationErrorin the MFA verification flow (not viatoAuthenticationError), suggesting that application-level MFA errors are handled separately from Supabase auth errors. OTP-specific errors (otp_expired,otp_disabled) are not currently encountered in the codebase despite usingverifyOtp().Adding mappings for these codes would be beneficial only if the application needs custom user-friendly messages beyond the fallback. If this is desired, add them to
AUTH_ERROR_MESSAGES; otherwise, the current approach is sufficient.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/nine-lilies-repair.md(1 hunks)apps/cli/src/utils/error-handler.ts(3 hunks)packages/tm-core/src/modules/integration/clients/supabase-client.ts(10 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
TypeScript test files must achieve minimum code coverage thresholds: 80% lines/functions and 70% branches globally, 90% for utilities, and 85% for middleware; new features must meet or exceed these thresholds
Files:
apps/cli/src/utils/error-handler.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
**/{utils,utilities,helpers}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/{utils,utilities,helpers}/**/*.{js,ts}: Document all parameters and return values in JSDoc format, include descriptions for complex logic, and add examples for non-obvious usage
Support multiple log levels (debug, info, warn, error) with appropriate icons for different log levels and respect the configured log level
Files:
apps/cli/src/utils/error-handler.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts}: Import and use specific getters from config-manager.js (e.g., getMainProvider(), getLogLevel(), getMainMaxTokens()) to access configuration values needed for application logic
Use isApiKeySet(providerName, session) from config-manager.js to check if a provider's key is available before potentially attempting an AI call
Do not add direct console.log calls outside the logging utility - use the central log function instead
Ensure silent mode is disabled in a finally block to prevent it from staying enabled
Do not access the global silentMode variable directly - use the exported silent mode control functions instead
Do not duplicate task ID formatting logic across modules - centralize formatting utilities
Use ContextGatherer class from utils/contextGatherer.js for AI-powered commands that need project context, supporting tasks, files, custom text, and project tree context
Use FuzzyTaskSearch class from utils/fuzzyTaskSearch.js for automatic task relevance detection with configurable search parameters
Use fuzzy search to supplement user-provided task IDs and display discovered task IDs to users for transparency
Do not replace explicit user task selections with fuzzy results - fuzzy search should supplement, not replace user selections
Use readJSON and writeJSON utilities for all JSON file operations instead of raw fs.readFileSync or fs.writeFileSync
Include error handling for JSON file operations and validate JSON structure after reading
Use path.join() for cross-platform path construction and path.resolve() for absolute paths, validating paths before file operations
Support both .env files and MCP session environment for environment variable resolution with fallbacks for missing values
Prefer updating the core function to accept an outputFormat parameter and check outputFormat === 'json' before displaying UI elements
Files:
apps/cli/src/utils/error-handler.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
**/{utils,utilities}/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
Implement the silent mode control functions (enableSilentMode, disableSilentMode, isSilentMode) in utility modules and always use isSilentMode() to check current state
Files:
apps/cli/src/utils/error-handler.ts
**/{utils,utilities}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/{utils,utilities}/**/*.{js,ts}: Use try/catch blocks for all file operations and return null or a default value on failure rather than allowing exceptions to propagate unhandled
Log detailed error information using the log utility in catch blocks for file operations
Create utilities for consistent task ID handling that support different ID formats (numeric, string, dot notation)
Implement reusable task finding utilities that support both task and subtask lookups and add context to subtask results
Implement cycle detection using graph traversal by tracking visited nodes and recursion stack, returning specific information about cycles
Detect circular dependencies using DFS and validate task references before operations
Export all utility functions explicitly in logical groups and include configuration constants from utility modules
Do not use default exports in utility modules - use named exports only
Group related exports together in utility modules and avoid creating circular dependencies
Make the log function respect silent mode by skipping logging when silent mode is enabled
Files:
apps/cli/src/utils/error-handler.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import modules with
.jsextension even in TypeScript source files for ESM compatibility
Files:
apps/cli/src/utils/error-handler.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
apps/cli/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
CLI (@tm/cli) should be a thin presentation layer that calls tm-core methods and displays results; handle only CLI-specific concerns like argument parsing, output formatting, and user prompts
Files:
apps/cli/src/utils/error-handler.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1444
File: apps/cli/src/utils/auto-update/changelog.ts:103-111
Timestamp: 2025-11-25T18:32:29.828Z
Learning: The claude-task-master project uses a custom changelog format with PR numbers and author acknowledgements in the pattern `- [#PR](...) Thanks [author]! - Description`, which is parsed by the regex in apps/cli/src/utils/auto-update/changelog.ts.
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/git_workflow.mdc:0-0
Timestamp: 2025-11-24T18:00:23.032Z
Learning: Pull Request descriptions must include: Task Overview, Subtasks Completed (checklist), Implementation Details, Testing approach, Breaking Changes (if any), and Related Tasks.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1360
File: apps/cli/src/commands/models/fetchers.ts:12-64
Timestamp: 2025-10-31T20:59:04.022Z
Learning: For eyaltoledano/claude-task-master, when fetching the OpenRouter models list in apps/cli/src/commands/models/fetchers.ts, do not require an API key. Prefer an optional Authorization header if OPENROUTER_API_KEY is set, and surface a clear 401 error message if encountered.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1069
File: .changeset/fix-tag-complexity-detection.md:0-0
Timestamp: 2025-08-02T15:33:22.656Z
Learning: For changeset files (.changeset/*.md), Crunchyman-ralph prefers to ignore formatting nitpicks about blank lines between frontmatter and descriptions, as he doesn't mind having them and wants to avoid such comments in future reviews.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1105
File: scripts/modules/supported-models.json:242-254
Timestamp: 2025-08-08T11:33:15.297Z
Learning: Preference: In scripts/modules/supported-models.json, the "name" field is optional. For OpenAI entries (e.g., "gpt-5"), Crunchyman-ralph prefers omitting "name" when the id is explicit enough; avoid nitpicks requesting a "name" in such cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1200
File: src/ai-providers/custom-sdk/grok-cli/language-model.js:96-100
Timestamp: 2025-09-19T16:06:42.182Z
Learning: The user Crunchyman-ralph prefers to keep environment variable names explicit (like GROK_CLI_API_KEY) rather than supporting multiple aliases, to avoid overlap and ensure clear separation between different CLI implementations.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/subpath-exports.test.ts:6-9
Timestamp: 2025-09-03T12:45:30.724Z
Learning: The user Crunchyman-ralph prefers to avoid overly nitpicky or detailed suggestions in code reviews, especially for test coverage of minor import paths. Focus on more substantial issues rather than comprehensive coverage of all possible edge cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1217
File: apps/cli/src/index.ts:16-21
Timestamp: 2025-09-18T16:35:35.147Z
Learning: The user Crunchyman-ralph considers suggestions to export types for better ergonomics (like exporting UpdateInfo type alongside related functions) as nitpicky and prefers not to implement such suggestions.
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Provide specific error handling for common issues, include troubleshooting hints for each error type, and use consistent error formatting across all commands.
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Provide contextual error handling for common issues with specific troubleshooting hints for each error type and consistent error formatting across all commands
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Log detailed error information using the log utility in catch blocks for file operations
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Wrap async operations in try/catch blocks, display user-friendly error messages, and include detailed error information in debug mode
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Set up global error handlers for uncaught exceptions that detect and format Commander-specific errors and provide suitable guidance for fixing common errors
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Set up global error handlers for uncaught exceptions, detect and format Commander-specific errors, and provide suitable guidance for fixing common errors.
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Wrap async operations in try/catch blocks, display user-friendly error messages, and include detailed error information in debug mode.
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Provide clear feedback about operation status, display success or error messages after completion, and use colored output (chalk) to distinguish between message types
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:01:44.169Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.169Z
Learning: Applies to scripts/modules/*.js : Use structured error objects with code and message properties; include clear error messages; handle both function-specific and file system errors; log errors at appropriate severity levels
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:04:01.629Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/ui.mdc:0-0
Timestamp: 2025-11-24T18:04:01.629Z
Learning: Applies to scripts/modules/ui.js : Use chalk.blue for informational messages, chalk.green for success messages, chalk.yellow for warnings, chalk.red for errors, chalk.cyan for prompts and highlights, and chalk.magenta for subtask-related information
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T17:58:19.853Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/changeset.mdc:0-0
Timestamp: 2025-11-24T17:58:19.853Z
Learning: Provide a concise, single-line changeset summary in imperative mood (e.g., 'Add feature X', 'Fix bug Y') that describes what changed from a user/consumer perspective, not implementation details
Applied to files:
.changeset/nine-lilies-repair.md
📚 Learning: 2025-10-15T14:43:40.410Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.ts
🧬 Code graph analysis (1)
packages/tm-core/src/modules/integration/clients/supabase-client.ts (3)
packages/tm-core/src/modules/auth/types.ts (1)
AuthenticationError(126-143)packages/tm-core/src/index.ts (1)
AuthenticationError(89-89)packages/tm-core/src/modules/auth/index.ts (1)
AuthenticationError(24-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Typecheck
- GitHub Check: Build
🔇 Additional comments (7)
apps/cli/src/utils/error-handler.ts (2)
68-99: LGTM on the multi-branch error handling structure.The displayError function now properly handles three distinct error types with clean fallthrough logic:
- TaskMasterError (via
getSanitizedDetails)- AuthenticationError (clean message, no "Error:" prefix)
- Supabase auth errors (mapped to user-friendly messages)
The conditional stack trace emission and consistent use of chalk colors aligns with the coding guidelines for error formatting.
6-6: Missing.jsextension in import path.Per the coding guidelines for ESM compatibility in TypeScript files, imports should include the
.jsextension.-import { AuthenticationError } from '@tm/core'; +import { AuthenticationError } from '@tm/core';Actually, upon re-examination,
@tm/coreis a package specifier (not a relative path), so the.jsextension rule doesn't apply here.packages/tm-core/src/modules/integration/clients/supabase-client.ts (4)
16-29: LGTM on the type guard implementation.The
isSupabaseAuthErrorfunction correctly checks for the__isAuthErrormarker that Supabase auth errors use internally. The return type properly narrows toAuthError & { code?: string }.
121-138: Well-implemented singleton pattern with proper reset capability.The singleton pattern correctly prevents multiple Supabase client instances, which addresses the
refresh_token_already_usedissue from concurrent refresh attempts. TheresetInstancemethod properly nullifies both the client and instance for clean test isolation.
186-210: Good defensive handling for MFA-expected errors during initialization.The differentiated logging for recoverable stale session errors vs. other auth errors prevents noisy logs during normal MFA flows while still surfacing genuine issues.
349-374: Ensure error propagation path is consistent.In
refreshSession, the error fromclient.auth.refreshSession()is logged and then converted viatoAuthenticationError. However, the catch block also handlesisSupabaseAuthErrorerrors. This is correct since Supabase can throw errors in addition to returning them in the error field..changeset/nine-lilies-repair.md (1)
1-5: LGTM!The changeset correctly uses patch level for this error handling improvement, and the summary follows the imperative mood format as per coding guidelines.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/tm-core/src/modules/auth/utils/auth-error-utils.ts (2)
61-66: Simplify the type assertion.The
as readonly string[]cast is unnecessary since theincludesmethod onRECOVERABLE_STALE_SESSION_ERRORSalready acceptsstringarguments due to how TypeScript handles tuple types withincludes.export function isRecoverableStaleSessionError(error: unknown): boolean { if (!isSupabaseAuthError(error)) return false; - return (RECOVERABLE_STALE_SESSION_ERRORS as readonly string[]).includes( - error.code || '' - ); + return RECOVERABLE_STALE_SESSION_ERRORS.includes( + error.code as (typeof RECOVERABLE_STALE_SESSION_ERRORS)[number] + ); }Alternatively, keep the original if you prefer the explicit widening for readability.
71-95: Remove redundant type cast on line 75.The
errorparameter is already typed asAuthError, so the cast(error as AuthError).codeis unnecessary.export function toAuthenticationError( error: AuthError, defaultMessage: string ): AuthenticationError { - const code = (error as AuthError).code; + const code = error.code; const userMessage = code
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/cli/src/utils/error-handler.ts(2 hunks)packages/tm-core/src/index.ts(1 hunks)packages/tm-core/src/modules/auth/index.ts(1 hunks)packages/tm-core/src/modules/auth/utils/auth-error-utils.ts(1 hunks)packages/tm-core/src/modules/auth/utils/index.ts(1 hunks)packages/tm-core/src/modules/integration/clients/supabase-client.ts(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cli/src/utils/error-handler.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
TypeScript test files must achieve minimum code coverage thresholds: 80% lines/functions and 70% branches globally, 90% for utilities, and 85% for middleware; new features must meet or exceed these thresholds
Files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts}: Import and use specific getters from config-manager.js (e.g., getMainProvider(), getLogLevel(), getMainMaxTokens()) to access configuration values needed for application logic
Use isApiKeySet(providerName, session) from config-manager.js to check if a provider's key is available before potentially attempting an AI call
Do not add direct console.log calls outside the logging utility - use the central log function instead
Ensure silent mode is disabled in a finally block to prevent it from staying enabled
Do not access the global silentMode variable directly - use the exported silent mode control functions instead
Do not duplicate task ID formatting logic across modules - centralize formatting utilities
Use ContextGatherer class from utils/contextGatherer.js for AI-powered commands that need project context, supporting tasks, files, custom text, and project tree context
Use FuzzyTaskSearch class from utils/fuzzyTaskSearch.js for automatic task relevance detection with configurable search parameters
Use fuzzy search to supplement user-provided task IDs and display discovered task IDs to users for transparency
Do not replace explicit user task selections with fuzzy results - fuzzy search should supplement, not replace user selections
Use readJSON and writeJSON utilities for all JSON file operations instead of raw fs.readFileSync or fs.writeFileSync
Include error handling for JSON file operations and validate JSON structure after reading
Use path.join() for cross-platform path construction and path.resolve() for absolute paths, validating paths before file operations
Support both .env files and MCP session environment for environment variable resolution with fallbacks for missing values
Prefer updating the core function to accept an outputFormat parameter and check outputFormat === 'json' before displaying UI elements
Files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import modules with
.jsextension even in TypeScript source files for ESM compatibility
Files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
**/{utils,utilities,helpers}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/{utils,utilities,helpers}/**/*.{js,ts}: Document all parameters and return values in JSDoc format, include descriptions for complex logic, and add examples for non-obvious usage
Support multiple log levels (debug, info, warn, error) with appropriate icons for different log levels and respect the configured log level
Files:
packages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.ts
**/{utils,utilities}/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
Implement the silent mode control functions (enableSilentMode, disableSilentMode, isSilentMode) in utility modules and always use isSilentMode() to check current state
Files:
packages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.ts
**/{utils,utilities}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/{utils,utilities}/**/*.{js,ts}: Use try/catch blocks for all file operations and return null or a default value on failure rather than allowing exceptions to propagate unhandled
Log detailed error information using the log utility in catch blocks for file operations
Create utilities for consistent task ID handling that support different ID formats (numeric, string, dot notation)
Implement reusable task finding utilities that support both task and subtask lookups and add context to subtask results
Implement cycle detection using graph traversal by tracking visited nodes and recursion stack, returning specific information about cycles
Detect circular dependencies using DFS and validate task references before operations
Export all utility functions explicitly in logical groups and include configuration constants from utility modules
Do not use default exports in utility modules - use named exports only
Group related exports together in utility modules and avoid creating circular dependencies
Make the log function respect silent mode by skipping logging when silent mode is enabled
Files:
packages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/git_workflow.mdc:0-0
Timestamp: 2025-11-24T18:00:23.032Z
Learning: Pull Request descriptions must include: Task Overview, Subtasks Completed (checklist), Implementation Details, Testing approach, Breaking Changes (if any), and Related Tasks.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1444
File: apps/cli/src/utils/auto-update/changelog.ts:103-111
Timestamp: 2025-11-25T18:32:29.828Z
Learning: The claude-task-master project uses a custom changelog format with PR numbers and author acknowledgements in the pattern `- [#PR](...) Thanks [author]! - Description`, which is parsed by the regex in apps/cli/src/utils/auto-update/changelog.ts.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1069
File: .changeset/fix-tag-complexity-detection.md:0-0
Timestamp: 2025-08-02T15:33:22.656Z
Learning: For changeset files (.changeset/*.md), Crunchyman-ralph prefers to ignore formatting nitpicks about blank lines between frontmatter and descriptions, as he doesn't mind having them and wants to avoid such comments in future reviews.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1105
File: scripts/modules/supported-models.json:242-254
Timestamp: 2025-08-08T11:33:15.297Z
Learning: Preference: In scripts/modules/supported-models.json, the "name" field is optional. For OpenAI entries (e.g., "gpt-5"), Crunchyman-ralph prefers omitting "name" when the id is explicit enough; avoid nitpicks requesting a "name" in such cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1200
File: src/ai-providers/custom-sdk/grok-cli/language-model.js:96-100
Timestamp: 2025-09-19T16:06:42.182Z
Learning: The user Crunchyman-ralph prefers to keep environment variable names explicit (like GROK_CLI_API_KEY) rather than supporting multiple aliases, to avoid overlap and ensure clear separation between different CLI implementations.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/subpath-exports.test.ts:6-9
Timestamp: 2025-09-03T12:45:30.724Z
Learning: The user Crunchyman-ralph prefers to avoid overly nitpicky or detailed suggestions in code reviews, especially for test coverage of minor import paths. Focus on more substantial issues rather than comprehensive coverage of all possible edge cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1217
File: apps/cli/src/index.ts:16-21
Timestamp: 2025-09-18T16:35:35.147Z
Learning: The user Crunchyman-ralph considers suggestions to export types for better ergonomics (like exporting UpdateInfo type alongside related functions) as nitpicky and prefers not to implement such suggestions.
📚 Learning: 2025-10-08T19:57:00.982Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1282
File: packages/tm-core/src/utils/index.ts:16-34
Timestamp: 2025-10-08T19:57:00.982Z
Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-09-26T19:05:47.555Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1252
File: packages/ai-sdk-provider-grok-cli/package.json:11-13
Timestamp: 2025-09-26T19:05:47.555Z
Learning: In the eyaltoledano/claude-task-master repository, internal tm/ packages use a specific export pattern where the "exports" field points to TypeScript source files (./src/index.ts) while "main" points to compiled output (./dist/index.js) and "types" points to source files (./src/index.ts). This pattern is used consistently across internal packages like tm/core and tm/ai-sdk-provider-grok-cli because they are consumed directly during build-time bundling with tsdown rather than being published as separate packages.
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-12T20:05:10.138Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1389
File: packages/tm-core/src/modules/auth/config.ts:55-59
Timestamp: 2025-11-12T20:05:10.138Z
Learning: In packages/tm-core/src/modules/auth/config.ts, the DEFAULT_AUTH_CONFIG Proxy does not require enumeration support (has, ownKeys, getOwnPropertyDescriptor traps). The codebase only uses direct property access on this config object, so the basic get trap is sufficient.
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-24T22:09:45.455Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.455Z
Learning: Applies to packages/tm-core/**/*.{spec,test}.ts : In unit tests for tm/core, mock only external I/O (Supabase, APIs, filesystem) and use real internal services
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Export the registerCommands function along with setupCLI, runCLI, checkForUpdate, compareVersions, and displayUpgradeNotification functions
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Provide contextual error handling for common issues with specific troubleshooting hints for each error type and consistent error formatting across all commands
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Place utilities used primarily by the core task-master CLI logic and command modules into scripts/modules/utils.js
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Export all utility functions explicitly in logical groups and include configuration constants from utility modules
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Export the registerCommands function and keep the CLI setup code clean and maintainable.
Applied to files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Do not use default exports in utility modules - use named exports only
Applied to files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Group related exports together in utility modules and avoid creating circular dependencies
Applied to files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-24T17:59:00.056Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-11-24T17:59:00.056Z
Learning: Applies to scripts/modules/utils/{contextGatherer,fuzzyTaskSearch}.js : Export utility modules using named exports: `ContextGatherer`, `createContextGatherer` from contextGatherer.js and `FuzzyTaskSearch`, `PURPOSE_CATEGORIES`, `RELEVANCE_THRESHOLDS` from fuzzyTaskSearch.js
Applied to files:
packages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to tests/fixtures/**/*.ts : Test fixture files must export reusable test data creators and constants (createTestUser, adminUser, invalidUser, etc.) for use across unit, integration, and e2e tests to ensure consistency
Applied to files:
packages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-09-03T12:20:36.005Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/index.ts:56-57
Timestamp: 2025-09-03T12:20:36.005Z
Learning: The logger functionality in tm-core should only be available through the main package entry point (import { getLogger, createLogger, setGlobalLogger } from 'tm-core'), not as a separate subpath export like other modules such as auth or storage.
Applied to files:
packages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-10-15T14:43:40.410Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Applied to files:
packages/tm-core/src/modules/auth/utils/auth-error-utils.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
🧬 Code graph analysis (2)
packages/tm-core/src/modules/auth/utils/auth-error-utils.ts (3)
packages/tm-core/src/index.ts (4)
isSupabaseAuthError(101-101)AUTH_ERROR_MESSAGES(102-102)isRecoverableStaleSessionError(103-103)AuthenticationError(89-89)packages/tm-core/src/modules/auth/index.ts (6)
isSupabaseAuthError(44-44)AUTH_ERROR_MESSAGES(45-45)RECOVERABLE_STALE_SESSION_ERRORS(46-46)isRecoverableStaleSessionError(47-47)toAuthenticationError(48-48)AuthenticationError(24-24)packages/tm-core/src/modules/auth/utils/index.ts (5)
isSupabaseAuthError(5-5)AUTH_ERROR_MESSAGES(6-6)RECOVERABLE_STALE_SESSION_ERRORS(7-7)isRecoverableStaleSessionError(8-8)toAuthenticationError(9-9)
packages/tm-core/src/modules/integration/clients/supabase-client.ts (2)
packages/tm-core/src/index.ts (2)
isRecoverableStaleSessionError(103-103)isSupabaseAuthError(101-101)packages/tm-core/src/modules/auth/utils/auth-error-utils.ts (3)
isRecoverableStaleSessionError(61-66)isSupabaseAuthError(13-22)toAuthenticationError(71-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (13)
packages/tm-core/src/modules/auth/utils/auth-error-utils.ts (3)
13-22: LGTM!The type guard correctly checks for the
__isAuthErrorinternal flag that Supabase uses to identify auth errors. The return type properly extendsAuthErrorwith optionalcode.
29-42: LGTM!User-friendly error messages are well-crafted with actionable guidance. The documentation note about MFA flows is helpful for understanding expected behavior.
52-55: LGTM!The
as constassertion correctly narrows the array to a readonly tuple of literal types.packages/tm-core/src/modules/auth/utils/index.ts (1)
1-10: LGTM!Clean barrel export following project conventions: named exports only, logical grouping, and
.jsextension for ESM compatibility. As per coding guidelines, utility functions are explicitly exported in logical groups.packages/tm-core/src/modules/auth/index.ts (1)
41-49: LGTM!The new re-exports are properly grouped with a descriptive comment and follow the existing export patterns in this module. The exports enable the CLI to consume these utilities through the public API surface.
packages/tm-core/src/index.ts (1)
99-104: LGTM!The selective export of only
isSupabaseAuthError,AUTH_ERROR_MESSAGES, andisRecoverableStaleSessionErrorfollows the team's minimal, need-based export strategy.RECOVERABLE_STALE_SESSION_ERRORSandtoAuthenticationErrorremain internal implementation details. Based on learnings, this aligns with the preference to only add exports when functions are actually consumed by other packages.packages/tm-core/src/modules/integration/clients/supabase-client.ts (7)
14-18: LGTM!Imports correctly reference the new auth error utilities from the internal module path. This keeps the dependency within the package rather than importing from the public API.
39-56: LGTM!The singleton pattern is well-implemented to prevent "refresh_token_already_used" errors from concurrent refresh attempts. The
resetInstance()correctly clears both the static instance and the internal client reference to ensure clean state for testing.
106-128: LGTM!The error handling appropriately categorizes errors with correct log levels: recoverable MFA-related errors at debug level, non-recoverable auth errors at warn level, and unexpected errors at error level. This reduces log noise during normal MFA flows.
231-249: LGTM!Consistent error handling pattern with
initialize(). The method correctly suppresses warnings for recoverable MFA-related errors and applies appropriate log levels for different error types.
269-292: LGTM!The error handling correctly converts Supabase auth errors to user-friendly
AuthenticationErrorinstances using the new utility. The fallback for non-Supabase errors maintains backward compatibility.
372-389: LGTM!The retry mechanism is well-designed:
- The
isRetryflag prevents infinite loops by limiting to a single retry attempt- Clearing session storage before retry removes the stale session causing the error
- Returning
nullclearly signals when no retry was attempted, allowing the caller to handle the error normally
399-457: LGTM!The retry logic in
verifyOneTimeCodeis well-implemented:
- The
isRetryparameter with defaultfalseenables the single-retry pattern- The
retryFnclosure correctly captures the token and setsisRetry=true- Both error paths (from
verifyOtpresult error and catch block) consistently usehandleRecoverableError- The documentation clearly explains the MFA-related stale session recovery behavior
Crunchyman-ralph
force-pushed
the
ralph/fix/auth.error.handling
branch
from
799a3e5 to
9039a8c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/tm-core/src/modules/auth/utils/auth-error-utils.ts (1)
80-92: Consider semantic alignment of error code mapping.
invalid_credentialsanduser_not_foundare mapped toINVALID_CODE, but these are authentication failures rather than invalid verification codes. Consider whetherNOT_AUTHENTICATEDwould be more semantically accurate, or ifINVALID_CODEis intentionally overloaded for all "bad input" scenarios.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/cli/src/utils/error-handler.ts(2 hunks)packages/tm-core/src/index.ts(1 hunks)packages/tm-core/src/modules/auth/index.ts(1 hunks)packages/tm-core/src/modules/auth/utils/auth-error-utils.ts(1 hunks)packages/tm-core/src/modules/auth/utils/index.ts(1 hunks)packages/tm-core/src/modules/integration/clients/supabase-client.ts(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/tm-core/src/index.ts
- packages/tm-core/src/modules/auth/utils/index.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
TypeScript test files must achieve minimum code coverage thresholds: 80% lines/functions and 70% branches globally, 90% for utilities, and 85% for middleware; new features must meet or exceed these thresholds
Files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/integration/clients/supabase-client.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.tsapps/cli/src/utils/error-handler.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts}: Import and use specific getters from config-manager.js (e.g., getMainProvider(), getLogLevel(), getMainMaxTokens()) to access configuration values needed for application logic
Use isApiKeySet(providerName, session) from config-manager.js to check if a provider's key is available before potentially attempting an AI call
Do not add direct console.log calls outside the logging utility - use the central log function instead
Ensure silent mode is disabled in a finally block to prevent it from staying enabled
Do not access the global silentMode variable directly - use the exported silent mode control functions instead
Do not duplicate task ID formatting logic across modules - centralize formatting utilities
Use ContextGatherer class from utils/contextGatherer.js for AI-powered commands that need project context, supporting tasks, files, custom text, and project tree context
Use FuzzyTaskSearch class from utils/fuzzyTaskSearch.js for automatic task relevance detection with configurable search parameters
Use fuzzy search to supplement user-provided task IDs and display discovered task IDs to users for transparency
Do not replace explicit user task selections with fuzzy results - fuzzy search should supplement, not replace user selections
Use readJSON and writeJSON utilities for all JSON file operations instead of raw fs.readFileSync or fs.writeFileSync
Include error handling for JSON file operations and validate JSON structure after reading
Use path.join() for cross-platform path construction and path.resolve() for absolute paths, validating paths before file operations
Support both .env files and MCP session environment for environment variable resolution with fallbacks for missing values
Prefer updating the core function to accept an outputFormat parameter and check outputFormat === 'json' before displaying UI elements
Files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/integration/clients/supabase-client.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.tsapps/cli/src/utils/error-handler.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import modules with
.jsextension even in TypeScript source files for ESM compatibility
Files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/integration/clients/supabase-client.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.tsapps/cli/src/utils/error-handler.ts
**/{utils,utilities,helpers}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/{utils,utilities,helpers}/**/*.{js,ts}: Document all parameters and return values in JSDoc format, include descriptions for complex logic, and add examples for non-obvious usage
Support multiple log levels (debug, info, warn, error) with appropriate icons for different log levels and respect the configured log level
Files:
packages/tm-core/src/modules/auth/utils/auth-error-utils.tsapps/cli/src/utils/error-handler.ts
**/{utils,utilities}/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
Implement the silent mode control functions (enableSilentMode, disableSilentMode, isSilentMode) in utility modules and always use isSilentMode() to check current state
Files:
packages/tm-core/src/modules/auth/utils/auth-error-utils.tsapps/cli/src/utils/error-handler.ts
**/{utils,utilities}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/{utils,utilities}/**/*.{js,ts}: Use try/catch blocks for all file operations and return null or a default value on failure rather than allowing exceptions to propagate unhandled
Log detailed error information using the log utility in catch blocks for file operations
Create utilities for consistent task ID handling that support different ID formats (numeric, string, dot notation)
Implement reusable task finding utilities that support both task and subtask lookups and add context to subtask results
Implement cycle detection using graph traversal by tracking visited nodes and recursion stack, returning specific information about cycles
Detect circular dependencies using DFS and validate task references before operations
Export all utility functions explicitly in logical groups and include configuration constants from utility modules
Do not use default exports in utility modules - use named exports only
Group related exports together in utility modules and avoid creating circular dependencies
Make the log function respect silent mode by skipping logging when silent mode is enabled
Files:
packages/tm-core/src/modules/auth/utils/auth-error-utils.tsapps/cli/src/utils/error-handler.ts
apps/cli/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
CLI (@tm/cli) should be a thin presentation layer that calls tm-core methods and displays results; handle only CLI-specific concerns like argument parsing, output formatting, and user prompts
Files:
apps/cli/src/utils/error-handler.ts
🧠 Learnings (26)
📓 Common learnings
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/git_workflow.mdc:0-0
Timestamp: 2025-11-24T18:00:23.032Z
Learning: Pull Request descriptions must include: Task Overview, Subtasks Completed (checklist), Implementation Details, Testing approach, Breaking Changes (if any), and Related Tasks.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1444
File: apps/cli/src/utils/auto-update/changelog.ts:103-111
Timestamp: 2025-11-25T18:32:29.828Z
Learning: The claude-task-master project uses a custom changelog format with PR numbers and author acknowledgements in the pattern `- [#PR](...) Thanks [author]! - Description`, which is parsed by the regex in apps/cli/src/utils/auto-update/changelog.ts.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1069
File: .changeset/fix-tag-complexity-detection.md:0-0
Timestamp: 2025-08-02T15:33:22.656Z
Learning: For changeset files (.changeset/*.md), Crunchyman-ralph prefers to ignore formatting nitpicks about blank lines between frontmatter and descriptions, as he doesn't mind having them and wants to avoid such comments in future reviews.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1105
File: scripts/modules/supported-models.json:242-254
Timestamp: 2025-08-08T11:33:15.297Z
Learning: Preference: In scripts/modules/supported-models.json, the "name" field is optional. For OpenAI entries (e.g., "gpt-5"), Crunchyman-ralph prefers omitting "name" when the id is explicit enough; avoid nitpicks requesting a "name" in such cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1200
File: src/ai-providers/custom-sdk/grok-cli/language-model.js:96-100
Timestamp: 2025-09-19T16:06:42.182Z
Learning: The user Crunchyman-ralph prefers to keep environment variable names explicit (like GROK_CLI_API_KEY) rather than supporting multiple aliases, to avoid overlap and ensure clear separation between different CLI implementations.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/subpath-exports.test.ts:6-9
Timestamp: 2025-09-03T12:45:30.724Z
Learning: The user Crunchyman-ralph prefers to avoid overly nitpicky or detailed suggestions in code reviews, especially for test coverage of minor import paths. Focus on more substantial issues rather than comprehensive coverage of all possible edge cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1217
File: apps/cli/src/index.ts:16-21
Timestamp: 2025-09-18T16:35:35.147Z
Learning: The user Crunchyman-ralph considers suggestions to export types for better ergonomics (like exporting UpdateInfo type alongside related functions) as nitpicky and prefers not to implement such suggestions.
📚 Learning: 2025-10-08T19:57:00.982Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1282
File: packages/tm-core/src/utils/index.ts:16-34
Timestamp: 2025-10-08T19:57:00.982Z
Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Export all utility functions explicitly in logical groups and include configuration constants from utility modules
Applied to files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.ts
📚 Learning: 2025-09-26T19:05:47.555Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1252
File: packages/ai-sdk-provider-grok-cli/package.json:11-13
Timestamp: 2025-09-26T19:05:47.555Z
Learning: In the eyaltoledano/claude-task-master repository, internal tm/ packages use a specific export pattern where the "exports" field points to TypeScript source files (./src/index.ts) while "main" points to compiled output (./dist/index.js) and "types" points to source files (./src/index.ts). This pattern is used consistently across internal packages like tm/core and tm/ai-sdk-provider-grok-cli because they are consumed directly during build-time bundling with tsdown rather than being published as separate packages.
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-12T20:05:10.138Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1389
File: packages/tm-core/src/modules/auth/config.ts:55-59
Timestamp: 2025-11-12T20:05:10.138Z
Learning: In packages/tm-core/src/modules/auth/config.ts, the DEFAULT_AUTH_CONFIG Proxy does not require enumeration support (has, ownKeys, getOwnPropertyDescriptor traps). The codebase only uses direct property access on this config object, so the basic get trap is sufficient.
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Do not use default exports in utility modules - use named exports only
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Group related exports together in utility modules and avoid creating circular dependencies
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T22:09:45.455Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.455Z
Learning: Applies to packages/tm-core/**/*.{spec,test}.ts : In unit tests for tm/core, mock only external I/O (Supabase, APIs, filesystem) and use real internal services
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Export the registerCommands function along with setupCLI, runCLI, checkForUpdate, compareVersions, and displayUpgradeNotification functions
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Place utilities used primarily by the core task-master CLI logic and command modules into scripts/modules/utils.js
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Provide contextual error handling for common issues with specific troubleshooting hints for each error type and consistent error formatting across all commands
Applied to files:
packages/tm-core/src/modules/auth/index.tsapps/cli/src/utils/error-handler.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Export the registerCommands function and keep the CLI setup code clean and maintainable.
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-10-15T14:43:40.410Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Use established mocking patterns from auth.test.ts as templates: mock bcrypt with proper TypeScript typing, mock Prisma client with transaction support, and always clear mocks between tests
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.ts
📚 Learning: 2025-11-24T18:01:44.169Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.169Z
Learning: Applies to mcp-server/src/core/task-master-core.js : Update `task-master-core.js` by importing and re-exporting direct functions and adding them to the directFunctions map
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Log detailed error information using the log utility in catch blocks for file operations
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T17:59:00.056Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-11-24T17:59:00.056Z
Learning: Applies to scripts/**/*.js : Implement graceful error handling for context gathering: handle missing files with warnings, validate task IDs with helpful messages, continue processing if some context sources fail, and provide fallback behavior
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Use try/catch blocks for all file operations and return null or a default value on failure rather than allowing exceptions to propagate unhandled
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:01:06.077Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-11-24T18:01:06.077Z
Learning: Applies to mcp-server/src/core/direct-functions/*.js : Wrap core function calls and AI calls in try/catch blocks, log errors with appropriate severity and context, and return standardized error objects with code and message
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to scripts/commands/**/*.{js,ts} : Handle potential ConfigurationError if the .taskmasterconfig file is missing or invalid when accessed via getConfig
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Provide specific error handling for common issues, include troubleshooting hints for each error type, and use consistent error formatting across all commands.
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Wrap async operations in try/catch blocks, display user-friendly error messages, and include detailed error information in debug mode
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:01:44.169Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.169Z
Learning: Applies to scripts/modules/*.js : Use structured error objects with code and message properties; include clear error messages; handle both function-specific and file system errors; log errors at appropriate severity levels
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Wrap async operations in try/catch blocks, display user-friendly error messages, and include detailed error information in debug mode.
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Set up global error handlers for uncaught exceptions that detect and format Commander-specific errors and provide suitable guidance for fixing common errors
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Set up global error handlers for uncaught exceptions, detect and format Commander-specific errors, and provide suitable guidance for fixing common errors.
Applied to files:
apps/cli/src/utils/error-handler.ts
🧬 Code graph analysis (1)
packages/tm-core/src/modules/integration/clients/supabase-client.ts (1)
packages/tm-core/src/modules/auth/utils/auth-error-utils.ts (3)
isRecoverableStaleSessionError(61-66)isSupabaseAuthError(13-22)toAuthenticationError(71-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (10)
packages/tm-core/src/modules/auth/utils/auth-error-utils.ts (2)
1-22: Well-structured type guard implementation.The
isSupabaseAuthErrortype guard correctly identifies Supabase auth errors by checking the internal__isAuthErrormarker. The implementation handles null checks properly and returns a useful narrowed type.
24-42: Clear, actionable error messages.The error messages appropriately guide users to re-authenticate via
task-master login. The note about MFA flows is helpful for maintainers.packages/tm-core/src/modules/auth/index.ts (1)
41-49: LGTM!The re-exports follow the project's named export conventions and group related utilities together. These are appropriately exposed since they're consumed by the CLI error handler. Based on learnings, tm-core prefers minimal exports, and these are justified by actual cross-package usage.
apps/cli/src/utils/error-handler.ts (2)
6-10: LGTM!Clean imports from
@tm/corefor the new auth error utilities. This follows the CLI pattern of consuming core utilities for error handling.
49-70: Good layered error handling for auth errors.The cascade handles
AuthenticationError(already-converted domain errors) separately from raw Supabase errors. TheisSupabaseAuthErrorcheck with fallback toerror.messageprovides graceful degradation for unknown error codes.packages/tm-core/src/modules/integration/clients/supabase-client.ts (5)
39-56: Well-implemented singleton pattern.The singleton prevents "refresh_token_already_used" errors from concurrent refresh attempts. The
resetInstance()correctly nullifies both the static instance and the internal client reference, ensuring clean test isolation.
106-128: Good noise reduction for expected MFA errors.Suppressing warnings for recoverable stale session errors during auth flows prevents log spam while maintaining appropriate logging for genuine issues.
372-389: Clean retry helper for recoverable errors.The
handleRecoverableErrormethod properly gates retries with theisRetryflag to prevent infinite recursion. Clearing session storage before retry addresses the root cause of stale session issues.
267-292: Consistent error conversion in refreshSession.Both the
if (error)path and thecatchblock now convert Supabase errors to user-friendlyAuthenticationErrorinstances viatoAuthenticationError. The existingAuthenticationErrorcheck prevents double-wrapping.
399-457: Retry logic correctly prevents infinite loops and handles both error paths.The
isRetryparameter serves as an effective guard: the initial call hasisRetry = false, allowing a single retry ifisRecoverableStaleSessionError(error)is true. On retry,isRetry = truepreventshandleRecoverableErrorfrom triggering again. This mechanism is applied consistently in both error handling paths (lines 416–420 and 444–448), and the conditional checkif (retryResult) return retryResult;correctly propagates retry results. The approach appropriately handles Supabase errors surfaced as both return values and thrown exceptions.
Crunchyman-ralph
force-pushed
the
ralph/fix/auth.error.handling
branch
from
9039a8c to
faaf994
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cli/src/utils/error-handler.ts (1)
6-65: Auth-aware CLI error handling and stack-trace gating look solidThe new
AuthenticationErrorandisSupabaseAuthErrorbranches give nice, user-friendly messages (plus consistent optional stack traces viaisDebugMode() || forceStack) without breaking the existing TaskMasterError flow. This should make auth failures much clearer at the CLI level.If you ever want to trim duplication, you could pull the “print stack trace when debug/forced” logic into a tiny helper (e.g.,
printStackIfRequested(error, options)), but it’s not essential.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/cli/src/utils/error-handler.ts(2 hunks)packages/tm-core/src/index.ts(1 hunks)packages/tm-core/src/modules/auth/index.ts(1 hunks)packages/tm-core/src/modules/auth/utils/auth-error-utils.ts(1 hunks)packages/tm-core/src/modules/auth/utils/index.ts(1 hunks)packages/tm-core/src/modules/integration/clients/supabase-client.ts(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/tm-core/src/index.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
TypeScript test files must achieve minimum code coverage thresholds: 80% lines/functions and 70% branches globally, 90% for utilities, and 85% for middleware; new features must meet or exceed these thresholds
Files:
packages/tm-core/src/modules/auth/index.tsapps/cli/src/utils/error-handler.tspackages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts}: Import and use specific getters from config-manager.js (e.g., getMainProvider(), getLogLevel(), getMainMaxTokens()) to access configuration values needed for application logic
Use isApiKeySet(providerName, session) from config-manager.js to check if a provider's key is available before potentially attempting an AI call
Do not add direct console.log calls outside the logging utility - use the central log function instead
Ensure silent mode is disabled in a finally block to prevent it from staying enabled
Do not access the global silentMode variable directly - use the exported silent mode control functions instead
Do not duplicate task ID formatting logic across modules - centralize formatting utilities
Use ContextGatherer class from utils/contextGatherer.js for AI-powered commands that need project context, supporting tasks, files, custom text, and project tree context
Use FuzzyTaskSearch class from utils/fuzzyTaskSearch.js for automatic task relevance detection with configurable search parameters
Use fuzzy search to supplement user-provided task IDs and display discovered task IDs to users for transparency
Do not replace explicit user task selections with fuzzy results - fuzzy search should supplement, not replace user selections
Use readJSON and writeJSON utilities for all JSON file operations instead of raw fs.readFileSync or fs.writeFileSync
Include error handling for JSON file operations and validate JSON structure after reading
Use path.join() for cross-platform path construction and path.resolve() for absolute paths, validating paths before file operations
Support both .env files and MCP session environment for environment variable resolution with fallbacks for missing values
Prefer updating the core function to accept an outputFormat parameter and check outputFormat === 'json' before displaying UI elements
Files:
packages/tm-core/src/modules/auth/index.tsapps/cli/src/utils/error-handler.tspackages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import modules with
.jsextension even in TypeScript source files for ESM compatibility
Files:
packages/tm-core/src/modules/auth/index.tsapps/cli/src/utils/error-handler.tspackages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
**/{utils,utilities,helpers}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/{utils,utilities,helpers}/**/*.{js,ts}: Document all parameters and return values in JSDoc format, include descriptions for complex logic, and add examples for non-obvious usage
Support multiple log levels (debug, info, warn, error) with appropriate icons for different log levels and respect the configured log level
Files:
apps/cli/src/utils/error-handler.tspackages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.ts
**/{utils,utilities}/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
Implement the silent mode control functions (enableSilentMode, disableSilentMode, isSilentMode) in utility modules and always use isSilentMode() to check current state
Files:
apps/cli/src/utils/error-handler.tspackages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.ts
**/{utils,utilities}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/{utils,utilities}/**/*.{js,ts}: Use try/catch blocks for all file operations and return null or a default value on failure rather than allowing exceptions to propagate unhandled
Log detailed error information using the log utility in catch blocks for file operations
Create utilities for consistent task ID handling that support different ID formats (numeric, string, dot notation)
Implement reusable task finding utilities that support both task and subtask lookups and add context to subtask results
Implement cycle detection using graph traversal by tracking visited nodes and recursion stack, returning specific information about cycles
Detect circular dependencies using DFS and validate task references before operations
Export all utility functions explicitly in logical groups and include configuration constants from utility modules
Do not use default exports in utility modules - use named exports only
Group related exports together in utility modules and avoid creating circular dependencies
Make the log function respect silent mode by skipping logging when silent mode is enabled
Files:
apps/cli/src/utils/error-handler.tspackages/tm-core/src/modules/auth/utils/index.tspackages/tm-core/src/modules/auth/utils/auth-error-utils.ts
apps/cli/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
CLI (@tm/cli) should be a thin presentation layer that calls tm-core methods and displays results; handle only CLI-specific concerns like argument parsing, output formatting, and user prompts
Files:
apps/cli/src/utils/error-handler.ts
🧠 Learnings (27)
📓 Common learnings
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/git_workflow.mdc:0-0
Timestamp: 2025-11-24T18:00:23.032Z
Learning: Pull Request descriptions must include: Task Overview, Subtasks Completed (checklist), Implementation Details, Testing approach, Breaking Changes (if any), and Related Tasks.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1444
File: apps/cli/src/utils/auto-update/changelog.ts:103-111
Timestamp: 2025-11-25T18:32:29.828Z
Learning: The claude-task-master project uses a custom changelog format with PR numbers and author acknowledgements in the pattern `- [#PR](...) Thanks [author]! - Description`, which is parsed by the regex in apps/cli/src/utils/auto-update/changelog.ts.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1069
File: .changeset/fix-tag-complexity-detection.md:0-0
Timestamp: 2025-08-02T15:33:22.656Z
Learning: For changeset files (.changeset/*.md), Crunchyman-ralph prefers to ignore formatting nitpicks about blank lines between frontmatter and descriptions, as he doesn't mind having them and wants to avoid such comments in future reviews.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1105
File: scripts/modules/supported-models.json:242-254
Timestamp: 2025-08-08T11:33:15.297Z
Learning: Preference: In scripts/modules/supported-models.json, the "name" field is optional. For OpenAI entries (e.g., "gpt-5"), Crunchyman-ralph prefers omitting "name" when the id is explicit enough; avoid nitpicks requesting a "name" in such cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1200
File: src/ai-providers/custom-sdk/grok-cli/language-model.js:96-100
Timestamp: 2025-09-19T16:06:42.182Z
Learning: The user Crunchyman-ralph prefers to keep environment variable names explicit (like GROK_CLI_API_KEY) rather than supporting multiple aliases, to avoid overlap and ensure clear separation between different CLI implementations.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/subpath-exports.test.ts:6-9
Timestamp: 2025-09-03T12:45:30.724Z
Learning: The user Crunchyman-ralph prefers to avoid overly nitpicky or detailed suggestions in code reviews, especially for test coverage of minor import paths. Focus on more substantial issues rather than comprehensive coverage of all possible edge cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1217
File: apps/cli/src/index.ts:16-21
Timestamp: 2025-09-18T16:35:35.147Z
Learning: The user Crunchyman-ralph considers suggestions to export types for better ergonomics (like exporting UpdateInfo type alongside related functions) as nitpicky and prefers not to implement such suggestions.
📚 Learning: 2025-10-08T19:57:00.982Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1282
File: packages/tm-core/src/utils/index.ts:16-34
Timestamp: 2025-10-08T19:57:00.982Z
Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.
Applied to files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-09-26T19:05:47.555Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1252
File: packages/ai-sdk-provider-grok-cli/package.json:11-13
Timestamp: 2025-09-26T19:05:47.555Z
Learning: In the eyaltoledano/claude-task-master repository, internal tm/ packages use a specific export pattern where the "exports" field points to TypeScript source files (./src/index.ts) while "main" points to compiled output (./dist/index.js) and "types" points to source files (./src/index.ts). This pattern is used consistently across internal packages like tm/core and tm/ai-sdk-provider-grok-cli because they are consumed directly during build-time bundling with tsdown rather than being published as separate packages.
Applied to files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Export all utility functions explicitly in logical groups and include configuration constants from utility modules
Applied to files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-12T20:05:10.138Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1389
File: packages/tm-core/src/modules/auth/config.ts:55-59
Timestamp: 2025-11-12T20:05:10.138Z
Learning: In packages/tm-core/src/modules/auth/config.ts, the DEFAULT_AUTH_CONFIG Proxy does not require enumeration support (has, ownKeys, getOwnPropertyDescriptor traps). The codebase only uses direct property access on this config object, so the basic get trap is sufficient.
Applied to files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Do not use default exports in utility modules - use named exports only
Applied to files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-24T22:09:45.455Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.455Z
Learning: Applies to packages/tm-core/**/*.{spec,test}.ts : In unit tests for tm/core, mock only external I/O (Supabase, APIs, filesystem) and use real internal services
Applied to files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Group related exports together in utility modules and avoid creating circular dependencies
Applied to files:
packages/tm-core/src/modules/auth/index.tspackages/tm-core/src/modules/auth/utils/index.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Applies to tests/fixtures/**/*.ts : Test fixture files must export reusable test data creators and constants (createTestUser, adminUser, invalidUser, etc.) for use across unit, integration, and e2e tests to ensure consistency
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Export the registerCommands function along with setupCLI, runCLI, checkForUpdate, compareVersions, and displayUpgradeNotification functions
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Place utilities used primarily by the core task-master CLI logic and command modules into scripts/modules/utils.js
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Provide contextual error handling for common issues with specific troubleshooting hints for each error type and consistent error formatting across all commands
Applied to files:
packages/tm-core/src/modules/auth/index.tsapps/cli/src/utils/error-handler.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Export the registerCommands function and keep the CLI setup code clean and maintainable.
Applied to files:
packages/tm-core/src/modules/auth/index.ts
📚 Learning: 2025-11-24T18:01:44.169Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.169Z
Learning: Applies to mcp-server/src/core/task-master-core.js : Update `task-master-core.js` by importing and re-exporting direct functions and adding them to the directFunctions map
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Log detailed error information using the log utility in catch blocks for file operations
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Use try/catch blocks for all file operations and return null or a default value on failure rather than allowing exceptions to propagate unhandled
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T17:59:00.056Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-11-24T17:59:00.056Z
Learning: Applies to scripts/**/*.js : Implement graceful error handling for context gathering: handle missing files with warnings, validate task IDs with helpful messages, continue processing if some context sources fail, and provide fallback behavior
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:01:06.077Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-11-24T18:01:06.077Z
Learning: Applies to mcp-server/src/core/direct-functions/*.js : Wrap core function calls and AI calls in try/catch blocks, log errors with appropriate severity and context, and return standardized error objects with code and message
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to scripts/commands/**/*.{js,ts} : Handle potential ConfigurationError if the .taskmasterconfig file is missing or invalid when accessed via getConfig
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Provide specific error handling for common issues, include troubleshooting hints for each error type, and use consistent error formatting across all commands.
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Wrap async operations in try/catch blocks, display user-friendly error messages, and include detailed error information in debug mode
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Wrap async operations in try/catch blocks, display user-friendly error messages, and include detailed error information in debug mode.
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Set up global error handlers for uncaught exceptions that detect and format Commander-specific errors and provide suitable guidance for fixing common errors
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Set up global error handlers for uncaught exceptions, detect and format Commander-specific errors, and provide suitable guidance for fixing common errors.
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-11-24T18:01:44.169Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.169Z
Learning: Applies to scripts/modules/*.js : Use structured error objects with code and message properties; include clear error messages; handle both function-specific and file system errors; log errors at appropriate severity levels
Applied to files:
apps/cli/src/utils/error-handler.ts
📚 Learning: 2025-10-15T14:43:40.410Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Applied to files:
packages/tm-core/src/modules/auth/utils/auth-error-utils.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Use established mocking patterns from auth.test.ts as templates: mock bcrypt with proper TypeScript typing, mock Prisma client with transaction support, and always clear mocks between tests
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.ts
🧬 Code graph analysis (2)
apps/cli/src/utils/error-handler.ts (3)
packages/tm-core/src/index.ts (3)
AuthenticationError(89-89)isSupabaseAuthError(101-101)AUTH_ERROR_MESSAGES(102-102)packages/tm-core/src/modules/auth/index.ts (3)
AuthenticationError(24-24)isSupabaseAuthError(44-44)AUTH_ERROR_MESSAGES(45-45)packages/tm-core/src/modules/auth/utils/auth-error-utils.ts (2)
isSupabaseAuthError(13-22)AUTH_ERROR_MESSAGES(29-42)
packages/tm-core/src/modules/integration/clients/supabase-client.ts (1)
packages/tm-core/src/modules/auth/utils/auth-error-utils.ts (3)
isRecoverableStaleSessionError(61-66)isSupabaseAuthError(13-22)toAuthenticationError(71-98)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (5)
packages/tm-core/src/modules/auth/index.ts (1)
42-49: Auth error utility re-exports align with shared core/CLI usageExposing the auth error helpers via the auth module index (backed by the utils barrel) looks consistent with the minimal-but-needed export strategy and enables the CLI to consume just the shared pieces without leaking internal implementation details. Based on learnings, this targeted export surface feels appropriate.
packages/tm-core/src/modules/auth/utils/index.ts (1)
1-10: Clean barrel export for auth error utilitiesThe grouped named re-exports from
auth-error-utils.jsare tidy, avoid default exports, and match the utilities export guidelines while keeping the surface focused on a coherent set of helpers.packages/tm-core/src/modules/integration/clients/supabase-client.ts (2)
14-18: Improved classification and logging of auth/session errorsThe use of
isRecoverableStaleSessionErrorandisSupabaseAuthErrorininitializeandgetSessionnicely separates “expected during MFA / stale session cleanup” cases from true failures, reducing noisy warnings while still surfacing real expiry/unknown errors via warn/error logs. The import of the shared auth utils keeps this logic centralized and consistent.Also applies to: 105-110, 119-127, 231-234, 240-247
268-287: Refresh-session mapping and single-retry flow for stale sessions are well-structuredMapping refresh and token-verification failures through
toAuthenticationErrorgives callers consistent error codes/messages, and thehandleRecoverableErrorhelper plusisRetryflag provide a clear “retry once after clearing stale session” path without risk of infinite recursion. TheverifyOneTimeCodeflow in particular cleanly handles both{ error }results and thrown auth errors using the same helper.Also applies to: 372-458
packages/tm-core/src/modules/auth/utils/auth-error-utils.ts (1)
13-98: Centralized mapping from Supabase-style auth errors to AuthenticationError looks goodThis module cleanly encapsulates Supabase-style auth error handling:
AUTH_ERROR_MESSAGESfor user-facing strings,RECOVERABLE_STALE_SESSION_ERRORSfor stale-session classification, andtoAuthenticationErrorfor consistent internal error codes (REFRESH_FAILED/NOT_AUTHENTICATED/INVALID_CREDENTIALS). That makes the client and CLI code much simpler and keeps all auth-error specifics in one place.
Crunchyman-ralph
force-pushed
the
ralph/fix/auth.error.handling
branch
from
faaf994 to
ae8c8f9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/tm-core/src/modules/integration/clients/supabase-client.ts (1)
399-457: LGTM! Enhanced token verification with safe retry logic.The retry mechanism properly handles MFA-related stale session errors by clearing storage and retrying once. The
isRetryparameter maintains backward compatibility with its default value.The error handling has two
handleRecoverableErrorcalls (lines 416 and 444), which provides defensive coverage but creates a slightly complex flow. This is acceptable as it ensures no recoverable errors are missed, though consider consolidating if it impacts maintainability.Optional simplification:
If you find the dual retry paths add complexity, consider handling the retry at the outermost catch level only:
try { this.logger.info('Verifying authentication token...'); const { data, error } = await client.auth.verifyOtp({ token_hash: token, type: 'magiclink' }); if (error) { - const retryResult = await this.handleRecoverableError( - error, - isRetry, - retryFn - ); - if (retryResult) return retryResult; - this.logger.error('Failed to verify token:', error); throw toAuthenticationError(error, 'Failed to verify token'); } // ... rest of success path } catch (error) { if (error instanceof AuthenticationError) { + // Check for recoverable errors before re-throwing + const retryResult = await this.handleRecoverableError( + error, + isRetry, + retryFn + ); + if (retryResult) return retryResult; throw error; } // Handle raw Supabase auth errors if (isSupabaseAuthError(error)) { const retryResult = await this.handleRecoverableError( error, isRetry, retryFn ); if (retryResult) return retryResult; throw toAuthenticationError(error, 'Token verification failed'); } // ... other error handling }However, the current implementation is correct and the defensive approach may be preferred for reliability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/cli/src/utils/error-handler.ts(2 hunks)packages/tm-core/src/index.ts(1 hunks)packages/tm-core/src/modules/auth/index.ts(1 hunks)packages/tm-core/src/modules/auth/utils/auth-error-utils.ts(1 hunks)packages/tm-core/src/modules/auth/utils/index.ts(1 hunks)packages/tm-core/src/modules/integration/clients/supabase-client.ts(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/tm-core/src/modules/auth/utils/auth-error-utils.ts
- apps/cli/src/utils/error-handler.ts
- packages/tm-core/src/modules/auth/utils/index.ts
- packages/tm-core/src/modules/auth/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
TypeScript test files must achieve minimum code coverage thresholds: 80% lines/functions and 70% branches globally, 90% for utilities, and 85% for middleware; new features must meet or exceed these thresholds
Files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts}: Import and use specific getters from config-manager.js (e.g., getMainProvider(), getLogLevel(), getMainMaxTokens()) to access configuration values needed for application logic
Use isApiKeySet(providerName, session) from config-manager.js to check if a provider's key is available before potentially attempting an AI call
Do not add direct console.log calls outside the logging utility - use the central log function instead
Ensure silent mode is disabled in a finally block to prevent it from staying enabled
Do not access the global silentMode variable directly - use the exported silent mode control functions instead
Do not duplicate task ID formatting logic across modules - centralize formatting utilities
Use ContextGatherer class from utils/contextGatherer.js for AI-powered commands that need project context, supporting tasks, files, custom text, and project tree context
Use FuzzyTaskSearch class from utils/fuzzyTaskSearch.js for automatic task relevance detection with configurable search parameters
Use fuzzy search to supplement user-provided task IDs and display discovered task IDs to users for transparency
Do not replace explicit user task selections with fuzzy results - fuzzy search should supplement, not replace user selections
Use readJSON and writeJSON utilities for all JSON file operations instead of raw fs.readFileSync or fs.writeFileSync
Include error handling for JSON file operations and validate JSON structure after reading
Use path.join() for cross-platform path construction and path.resolve() for absolute paths, validating paths before file operations
Support both .env files and MCP session environment for environment variable resolution with fallbacks for missing values
Prefer updating the core function to accept an outputFormat parameter and check outputFormat === 'json' before displaying UI elements
Files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import modules with
.jsextension even in TypeScript source files for ESM compatibility
Files:
packages/tm-core/src/index.tspackages/tm-core/src/modules/integration/clients/supabase-client.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/git_workflow.mdc:0-0
Timestamp: 2025-11-24T18:00:23.032Z
Learning: Pull Request descriptions must include: Task Overview, Subtasks Completed (checklist), Implementation Details, Testing approach, Breaking Changes (if any), and Related Tasks.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1444
File: apps/cli/src/utils/auto-update/changelog.ts:103-111
Timestamp: 2025-11-25T18:32:29.828Z
Learning: The claude-task-master project uses a custom changelog format with PR numbers and author acknowledgements in the pattern `- [#PR](...) Thanks [author]! - Description`, which is parsed by the regex in apps/cli/src/utils/auto-update/changelog.ts.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1069
File: .changeset/fix-tag-complexity-detection.md:0-0
Timestamp: 2025-08-02T15:33:22.656Z
Learning: For changeset files (.changeset/*.md), Crunchyman-ralph prefers to ignore formatting nitpicks about blank lines between frontmatter and descriptions, as he doesn't mind having them and wants to avoid such comments in future reviews.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1105
File: scripts/modules/supported-models.json:242-254
Timestamp: 2025-08-08T11:33:15.297Z
Learning: Preference: In scripts/modules/supported-models.json, the "name" field is optional. For OpenAI entries (e.g., "gpt-5"), Crunchyman-ralph prefers omitting "name" when the id is explicit enough; avoid nitpicks requesting a "name" in such cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1200
File: src/ai-providers/custom-sdk/grok-cli/language-model.js:96-100
Timestamp: 2025-09-19T16:06:42.182Z
Learning: The user Crunchyman-ralph prefers to keep environment variable names explicit (like GROK_CLI_API_KEY) rather than supporting multiple aliases, to avoid overlap and ensure clear separation between different CLI implementations.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/subpath-exports.test.ts:6-9
Timestamp: 2025-09-03T12:45:30.724Z
Learning: The user Crunchyman-ralph prefers to avoid overly nitpicky or detailed suggestions in code reviews, especially for test coverage of minor import paths. Focus on more substantial issues rather than comprehensive coverage of all possible edge cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1217
File: apps/cli/src/index.ts:16-21
Timestamp: 2025-09-18T16:35:35.147Z
Learning: The user Crunchyman-ralph considers suggestions to export types for better ergonomics (like exporting UpdateInfo type alongside related functions) as nitpicky and prefers not to implement such suggestions.
📚 Learning: 2025-10-08T19:57:00.982Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1282
File: packages/tm-core/src/utils/index.ts:16-34
Timestamp: 2025-10-08T19:57:00.982Z
Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-09-26T19:05:47.555Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1252
File: packages/ai-sdk-provider-grok-cli/package.json:11-13
Timestamp: 2025-09-26T19:05:47.555Z
Learning: In the eyaltoledano/claude-task-master repository, internal tm/ packages use a specific export pattern where the "exports" field points to TypeScript source files (./src/index.ts) while "main" points to compiled output (./dist/index.js) and "types" points to source files (./src/index.ts). This pattern is used consistently across internal packages like tm/core and tm/ai-sdk-provider-grok-cli because they are consumed directly during build-time bundling with tsdown rather than being published as separate packages.
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Applies to **/{utils,utilities}/**/*.{js,ts} : Export all utility functions explicitly in logical groups and include configuration constants from utility modules
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-11-24T22:09:45.455Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.455Z
Learning: Applies to packages/tm-core/**/*.{spec,test}.ts : In unit tests for tm/core, mock only external I/O (Supabase, APIs, filesystem) and use real internal services
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-11-12T20:05:10.138Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1389
File: packages/tm-core/src/modules/auth/config.ts:55-59
Timestamp: 2025-11-12T20:05:10.138Z
Learning: In packages/tm-core/src/modules/auth/config.ts, the DEFAULT_AUTH_CONFIG Proxy does not require enumeration support (has, ownKeys, getOwnPropertyDescriptor traps). The codebase only uses direct property access on this config object, so the basic get trap is sufficient.
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-09-02T21:51:41.383Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/auth/config.ts:0-0
Timestamp: 2025-09-02T21:51:41.383Z
Learning: In packages/tm-core/src/auth/config.ts, the BASE_DOMAIN configuration intentionally does not include runtime environment variable fallbacks like TM_BASE_DOMAIN or HAMSTER_BASE_URL. The maintainers prefer to keep these capabilities "hush hush" and undocumented, using only the build-time TM_PUBLIC_BASE_DOMAIN and the default value.
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Export the registerCommands function along with setupCLI, runCLI, checkForUpdate, compareVersions, and displayUpgradeNotification functions
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-11-24T17:58:47.030Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.030Z
Learning: Applies to scripts/modules/commands.js : Provide contextual error handling for common issues with specific troubleshooting hints for each error type and consistent error formatting across all commands
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-11-24T18:04:43.972Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.972Z
Learning: Place utilities used primarily by the core task-master CLI logic and command modules into scripts/modules/utils.js
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Export the registerCommands function and keep the CLI setup code clean and maintainable.
Applied to files:
packages/tm-core/src/index.ts
📚 Learning: 2025-10-15T14:43:40.410Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
Learning: Use established mocking patterns from auth.test.ts as templates: mock bcrypt with proper TypeScript typing, mock Prisma client with transaction support, and always clear mocks between tests
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Typecheck
🔇 Additional comments (7)
packages/tm-core/src/index.ts (1)
99-104: LGTM! Well-documented public API expansion for auth error utilities.The new exports support the PR's objective to improve authentication error handling. The comment clearly indicates these utilities are shared with CLI consumers, and the exports follow the established pattern in this file.
packages/tm-core/src/modules/integration/clients/supabase-client.ts (6)
14-18: LGTM! Clean import of auth error utilities.The imports support the enhanced error handling throughout the file and follow ESM conventions with
.jsextensions.
39-56: LGTM! Proper singleton pattern implementation.The singleton pattern prevents concurrent Supabase client instances that could cause "refresh_token_already_used" errors. The
resetInstance()method properly cleans up both the singleton instance and the internal client, which is essential for test isolation.
106-127: LGTM! Appropriate error classification for MFA flows.The enhanced error handling distinguishes between expected MFA-related errors (debug level) and genuine failures (warn/error level), reducing noise in logs during normal authentication flows while preserving visibility for actual issues.
231-248: LGTM! Consistent error handling pattern.The error handling mirrors the pattern used in
initialize(), maintaining consistency across session management methods.
269-286: LGTM! User-friendly error conversion.Converting Supabase-specific errors to user-friendly messages through
toAuthenticationErrorimproves the end-user experience by providing clearer, more actionable error information.
372-389: LGTM! Safe and well-designed retry helper.The
handleRecoverableErrormethod properly guards against infinite retries with the!isRetrycheck, clears stale session state before retrying, and encapsulates the retry logic in a reusable way. The single-retry pattern is appropriate for transient MFA-related session issues.
This PR was automatically generated to update documentation based on recent changes. Original commit: fix: improve auth-related error handling (#1477)\n\n\n Co-authored-by: Claude <claude-assistant@anthropic.com>
1 participant
What type of PR is this?
Description
Related Issues
How to Test This
# Example commands or stepsExpected result:
Contributor Checklist
npm run changesetnpm testnpm run format-check(ornpm run formatto fix)Changelog Entry
For Maintainers
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.