Ralph/implement.tm.list.remote#1185
Conversation
|
|
Caution Review failedThe pull request is closed. WalkthroughIntroduces a new CLI ContextCommand for managing org/brief context; extends AuthManager with session restoration, PKCE/auth flows, user-context persistence, and org/brief/task fetch APIs; adds OrganizationService, repositories, TaskMapper, and Supabase-backed ApiStorage. Removes “auto” storage type, adjusts configs/build, adds database types, and updates exports/registrations/tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant CLI as ContextCommand
participant Auth as AuthManager
participant OrgSvc as OrganizationService
participant SB as Supabase
User->>CLI: context org
CLI->>Auth: ensure authenticated
Auth->>SB: initialize/refresh session
Auth-->>CLI: session OK
CLI->>Auth: getOrganizations()
Auth->>OrgSvc: getOrganizations()
OrgSvc->>SB: query user_accounts/accounts
OrgSvc-->>Auth: organizations[]
Auth-->>CLI: organizations[]
CLI->>User: prompt select org
User-->>CLI: chosen org
CLI->>Auth: updateContext({ orgId, orgName, briefId: undefined })
Auth-->>CLI: updated context
CLI-->>User: success message
sequenceDiagram
autonumber
participant User as User
participant CLI as OAuthService
participant SB as SupabaseAuthClient
participant Store as CredentialStore
User->>CLI: start auth (PKCE)
CLI->>SB: signInWithPKCE()
SB-->>CLI: auth URL
CLI->>User: open URL
User-->>CLI: redirect with code
CLI->>SB: exchangeCodeForSession(code)
SB-->>CLI: Session{access_token, refresh_token, expires_at, user}
CLI->>Store: persist AuthCredentials
CLI-->>User: authenticated
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (31)
✨ Finishing Touches
🧪 Generate unit tests
Comment |
Crunchyman-ralph
force-pushed
the
ralph/implement.tm.list.remote
branch
from
233d1bf to
1c2c253
Compare
Crunchyman-ralph
changed the base branch from
ralph/implement.auth.ham.tweaks
to
next-typescript
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 50
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tm-core/src/auth/auth-manager.ts (1)
150-161: Reset cached OrganizationService on logout.After sign-out, keep the instance from being reused with a cleared/changed session.
// Always clear local credentials (removes auth.json file) this.credentialStore.clearCredentials(); + // Reset cached service to avoid using a stale client after logout + this.organizationService = undefined;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (18)
apps/cli/src/commands/context.command.ts(1 hunks)apps/cli/src/index.ts(1 hunks)packages/tm-core/src/auth/auth-manager.ts(4 hunks)packages/tm-core/src/auth/index.ts(1 hunks)packages/tm-core/src/auth/oauth-service.ts(2 hunks)packages/tm-core/src/auth/supabase-session-storage.ts(1 hunks)packages/tm-core/src/auth/types.ts(2 hunks)packages/tm-core/src/clients/supabase-client.ts(4 hunks)packages/tm-core/src/mappers/TaskMapper.ts(1 hunks)packages/tm-core/src/repositories/supabase-task-repository.ts(1 hunks)packages/tm-core/src/repositories/task-repository.interface.ts(1 hunks)packages/tm-core/src/services/index.ts(1 hunks)packages/tm-core/src/services/organization.service.ts(1 hunks)packages/tm-core/src/storage/api-storage.ts(21 hunks)packages/tm-core/src/storage/storage-factory.ts(2 hunks)packages/tm-core/src/types/database.types.ts(1 hunks)packages/tm-core/src/types/index.ts(1 hunks)scripts/modules/commands.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
scripts/modules/commands.js
📄 CodeRabbit inference engine (.cursor/rules/ai_services.mdc)
scripts/modules/commands.js: Centralize all LLM calls throughgenerateTextServiceorgenerateObjectService.
Do not import or call anything from the oldai-services.js,ai-client-factory.js, orai-client-utils.jsfiles.
Do not fetch AI-specific parameters (model ID, max tokens, temp) usingconfig-manager.jsgetters for the AI call. Pass theroleinstead.
Do not implement fallback or retry logic outsideai-services-unified.js.
Do not handle API key resolution outside the service layer (it usesutils.jsinternally).
Determine the appropriaterole(main,research,fallback) in your core logic and pass it to the service.
Pass thesessionobject (received in thecontextparameter, especially from direct function wrappers) to the service call when in MCP context.
UsegenerateTextServiceand implement robust manual JSON parsing (with Zod validation after parsing) when structured output is needed, asgenerateObjectServicehas shown unreliability with some providers/schemas.
Be aware of potential reliability issues withgenerateObjectServiceacross different providers and complex schemas. PrefergenerateTextService+ manual parsing as a more robust alternative for structured data needs.
scripts/modules/commands.js: All new user-facing commands should be added to 'scripts/modules/commands.js'.
Use consistent patterns for option naming and help text in CLI commands.
Follow the Commander.js model for subcommand structure in CLI commands.
When using callbacks (like in Commander.js commands), define them separately to allow testing the callback logic independently.
Add help text to the command definition and update 'dev_workflow.mdc' with command reference when adding a new feature.
Follow the established pattern in 'commands.js' for CLI command implementation, using Commander.js for argument parsing, including comprehensive help text and examples, and supporting tagged task context awareness.
Provide clear error messages for common failu...
Files:
scripts/modules/commands.js
scripts/modules/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Each module in scripts/modules/ should be focused on a single responsibility, following the modular architecture (e.g., commands.js for CLI command handling, task-manager.js for task data and core logic, dependency-manager.js for dependency management, ui.js for CLI output formatting, ai-services-unified.js for AI service integration, config-manager.js for configuration management, utils.js for utility functions).
scripts/modules/*.js: Export all core functions, helper functions, and utility methods needed by your new function or command from their respective modules. Explicitly review the module's export block to ensure every required dependency is included.
Pass all required parameters to functions you call within your implementation and verify that direct function parameters match their core function counterparts.
Use consistent file naming conventions: 'task_${id.toString().padStart(3, '0')}.txt', use path.join for composing file paths, and use appropriate file extensions (.txt for tasks, .json for data).
Use structured error objects with code and message properties, include clear error messages, and handle both function-specific and file system errors.
Import all silent mode utilities together from 'scripts/modules/utils.js' and always use isSilentMode() to check global silent mode status. Wrap core function calls within direct functions using enableSilentMode() and disableSilentMode() in a try/finally block if the core function might produce console output.
Core functions should check outputFormat === 'text' before displaying UI elements and use internal logging that respects silent mode.
Design functions to accept dependencies as parameters (dependency injection) and avoid hard-coded dependencies that are difficult to mock.
Keep pure logic separate from I/O operations or UI rendering to allow testing the logic without mocking complex dependencies.
When implementing core logic for new features, do so in 'scripts/modules/' before CLI or MCP interfaces, and d...
Files:
scripts/modules/commands.js
scripts/modules/**
📄 CodeRabbit inference engine (.cursor/rules/dev_workflow.mdc)
When using the MCP server, restart it if core logic in
scripts/modulesor MCP tool/direct function definitions change.
Files:
scripts/modules/commands.js
scripts/modules/*
📄 CodeRabbit inference engine (.cursor/rules/tags.mdc)
scripts/modules/*: Every command that reads or writes tasks.json must be tag-aware
All command files must import getCurrentTag from utils.js
Every CLI command that operates on tasks must include the --tag CLI option
All commands must resolve the tag using the pattern: options.tag || getCurrentTag(projectRoot) || 'master'
All commands must find projectRoot with error handling before proceeding
All commands must pass { projectRoot, tag } as context to core functions
MCP direct functions must accept and use a context object containing projectRoot and tag, and pass them to core functions
Do not hard-code tag resolution (e.g., const tag = options.tag || 'master';); always use getCurrentTag
Do not omit the --tag CLI option in commands that operate on tasks
Do not omit the context parameter when calling core functions from commands
Do not call readJSON or writeJSON without passing projectRoot and tag
Files:
scripts/modules/commands.js
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
**/*.js: Declare and initialize global variables at the top of modules to avoid hoisting issues.
Use proper function declarations to avoid hoisting issues and initialize variables before they are referenced.
Do not reference variables before their declaration in module scope.
Use dynamic imports (import()) to avoid initialization order issues in modules.
Files:
scripts/modules/commands.js
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/git_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:31.810Z
Learning: Pull Request descriptions must use the provided template, including Task Overview, Subtasks Completed, Implementation Details, Testing, Breaking Changes, and Related Tasks
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 Learning: 2025-07-18T17:09:13.815Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-07-18T17:09:13.815Z
Learning: Applies to scripts/modules/utils/contextGatherer.js : Export `ContextGatherer` and `createContextGatherer` from the context gatherer module.
Applied to files:
apps/cli/src/index.ts
📚 Learning: 2025-07-31T22:07:49.716Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-31T22:07:49.716Z
Learning: Applies to scripts/modules/commands.js : For AI-powered commands that benefit from project context, use the ContextGatherer utility for multi-source context extraction, support task IDs, file paths, custom context, and project tree, implement fuzzy search for automatic task discovery, and display detailed token breakdown for transparency.
Applied to files:
apps/cli/src/index.tsapps/cli/src/commands/context.command.tsscripts/modules/commands.js
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to scripts/modules/utils/contextGatherer.js : Use `ContextGatherer` in `scripts/modules/utils/contextGatherer.js` for AI-powered commands that need project context, supporting tasks, files, custom text, project tree context, token counting, and multiple output formats.
Applied to files:
apps/cli/src/index.tsapps/cli/src/commands/context.command.tsscripts/modules/commands.js
📚 Learning: 2025-07-18T17:09:16.839Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-07-18T17:09:16.839Z
Learning: Applies to scripts/modules/utils/contextGatherer.js : Use the `ContextGatherer` class in `scripts/modules/utils/contextGatherer.js` as the centralized utility for extracting context from multiple sources in Task Master commands.
Applied to files:
apps/cli/src/index.tsapps/cli/src/commands/context.command.tsscripts/modules/commands.js
📚 Learning: 2025-07-18T17:09:13.815Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-07-18T17:09:13.815Z
Learning: Applies to scripts/modules/utils/contextGatherer.js : Use the `ContextGatherer` class from `scripts/modules/utils/contextGatherer.js` as the centralized utility for extracting context from multiple sources in Task Master commands.
Applied to files:
apps/cli/src/index.tsapps/cli/src/commands/context.command.tsscripts/modules/commands.js
📚 Learning: 2025-07-18T17:12:57.903Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to scripts/modules/commands.js : Follow the established pattern in 'commands.js' for CLI command implementation, using Commander.js for argument parsing, including comprehensive help text and examples, and supporting tagged task context awareness.
Applied to files:
apps/cli/src/index.tsapps/cli/src/commands/context.command.tsscripts/modules/commands.js
📚 Learning: 2025-07-31T22:07:49.716Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-31T22:07:49.716Z
Learning: Applies to scripts/modules/commands.js : Export the registerCommands function and keep the CLI setup code clean and maintainable.
Applied to files:
apps/cli/src/index.tsscripts/modules/commands.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Use consistent formatting for task files, include all task properties in text files, and format dependencies with status indicators.
Applied to files:
packages/tm-core/src/mappers/TaskMapper.ts
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Tasks must be organized into separate contexts (tags) within tasks.json, using the tagged format: {"master": {"tasks": [...]}, "feature-branch": {"tasks": [...]}}. Legacy format {"tasks": [...]} must be silently migrated to the tagged format on first use.
Applied to files:
packages/tm-core/src/types/index.tspackages/tm-core/src/storage/api-storage.ts
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to scripts/modules/utils.js : Use tagged task system aware functions for task finding and manipulation, handle both task and subtask operations, and validate task IDs before operations.
Applied to files:
packages/tm-core/src/types/index.ts
📚 Learning: 2025-07-18T17:10:12.881Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:12.881Z
Learning: Applies to tasks.json : Use the `tasks.json` file (generated by Taskmaster) to store the project's task list, including tags and task structures.
Applied to files:
packages/tm-core/src/types/index.tspackages/tm-core/src/storage/api-storage.ts
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Tasks must be accessed and updated through the tag resolution layer using getTasksForTag(data, tagName) and setTasksForTag(data, tagName, tasks); direct manipulation of the tagged structure in core functions is not allowed.
Applied to files:
packages/tm-core/src/types/index.tspackages/tm-core/src/storage/api-storage.ts
📚 Learning: 2025-07-18T17:13:30.188Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tags.mdc:0-0
Timestamp: 2025-07-18T17:13:30.188Z
Learning: Applies to scripts/modules/* : Every command that reads or writes tasks.json must be tag-aware
Applied to files:
packages/tm-core/src/types/index.tsscripts/modules/commands.js
📚 Learning: 2025-07-18T17:10:12.881Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:12.881Z
Learning: Organize tasks into tags (contexts) such as 'master', 'feature-branch', or 'v2.0' in Taskmaster to isolate workstreams and prevent conflicts.
Applied to files:
packages/tm-core/src/types/index.ts
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Provide functions for updating task status within the current tag context, handling both individual tasks and subtasks, and considering subtask status when updating parent tasks.
Applied to files:
packages/tm-core/src/storage/api-storage.ts
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : The default tag 'master' must be used for all existing and new tasks unless otherwise specified.
Applied to files:
packages/tm-core/src/storage/api-storage.ts
📚 Learning: 2025-07-18T17:12:57.903Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to scripts/modules/commands.js : All new user-facing commands should be added to 'scripts/modules/commands.js'.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:12:57.903Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to scripts/modules/commands.js : Add help text to the command definition and update 'dev_workflow.mdc' with command reference when adding a new feature.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Extract tasks from PRD documents using AI, create them in the current tag context (defaulting to 'master'), provide clear prompts to guide AI task generation, and validate/clean up AI-generated tasks.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Start new projects by running `task-master init` or `node scripts/dev.js parse-prd --input=<prd-file.txt>` to generate the initial tasks.json.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-20T01:35:05.831Z
Learnt from: rtmcrc
PR: eyaltoledano/claude-task-master#933
File: scripts/modules/task-manager/parse-prd.js:226-226
Timestamp: 2025-07-20T01:35:05.831Z
Learning: The parsePRD function in scripts/modules/task-manager/parse-prd.js has a different parameter structure than other task-manager functions - it uses `options` parameter instead of `context` parameter because it generates tasks from PRD documents rather than operating on existing tasks.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:13:30.188Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tags.mdc:0-0
Timestamp: 2025-07-18T17:13:30.188Z
Learning: Applies to scripts/modules/task-manager/* : All core functions in scripts/modules/task-manager/ must use readJSON(tasksPath, projectRoot, tag) and writeJSON(tasksPath, data, projectRoot, tag)
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to mcp-server/src/core/utils/path-utils.js : Call `findTasksJsonPath(args, log)` from within direct function wrappers in `mcp-server/src/core/direct-functions/` to get the absolute path to the relevant `tasks.json`.
Applied to files:
scripts/modules/commands.js
🧬 Code graph analysis (12)
packages/tm-core/src/auth/supabase-session-storage.ts (1)
packages/tm-core/src/auth/types.ts (1)
AuthCredentials(5-14)
packages/tm-core/src/repositories/task-repository.interface.ts (1)
packages/tm-core/src/types/index.ts (1)
TaskTag(102-106)
packages/tm-core/src/mappers/TaskMapper.ts (2)
packages/tm-core/src/types/database.types.ts (2)
Tables(413-436)Database(9-411)packages/tm-core/src/types/index.ts (2)
Task(48-67)Subtask(72-76)
packages/tm-core/src/services/organization.service.ts (1)
packages/tm-core/src/types/database.types.ts (1)
Database(9-411)
packages/tm-core/src/auth/oauth-service.ts (2)
packages/tm-core/src/auth/types.ts (1)
AuthCredentials(5-14)packages/tm-core/src/auth/auth-manager.ts (1)
refreshToken(104-145)
packages/tm-core/src/repositories/supabase-task-repository.ts (2)
packages/tm-core/src/types/database.types.ts (1)
Database(9-411)packages/tm-core/src/mappers/TaskMapper.ts (1)
TaskMapper(7-170)
apps/cli/src/commands/context.command.ts (2)
packages/tm-core/src/auth/types.ts (2)
UserContext(16-22)AuthenticationError(88-100)packages/tm-core/src/auth/auth-manager.ts (1)
AuthManager(26-279)
packages/tm-core/src/storage/storage-factory.ts (2)
packages/tm-core/src/clients/supabase-client.ts (1)
SupabaseAuthClient(16-312)packages/tm-core/src/storage/api-storage.ts (1)
ApiStorage(37-728)
packages/tm-core/src/auth/auth-manager.ts (4)
packages/tm-core/src/services/organization.service.ts (4)
OrganizationService(56-363)Organization(14-18)Brief(23-30)RemoteTask(35-51)packages/tm-core/src/auth/types.ts (4)
AuthConfig(39-43)AuthCredentials(5-14)AuthenticationError(88-100)UserContext(16-22)packages/tm-core/src/auth/credential-store.ts (1)
CredentialStore(21-241)packages/tm-core/src/clients/supabase-client.ts (1)
SupabaseAuthClient(16-312)
packages/tm-core/src/clients/supabase-client.ts (4)
packages/tm-core/src/auth/supabase-session-storage.ts (1)
SupabaseSessionStorage(16-155)packages/tm-core/src/logger/factory.ts (1)
getLogger(23-44)packages/tm-core/src/auth/credential-store.ts (1)
CredentialStore(21-241)packages/tm-core/src/auth/types.ts (1)
AuthenticationError(88-100)
packages/tm-core/src/storage/api-storage.ts (4)
packages/tm-core/src/repositories/task-repository.interface.ts (1)
TaskRepository(3-36)packages/tm-core/src/types/index.ts (3)
TaskTag(102-106)Task(48-67)TaskMetadata(81-89)packages/tm-core/src/repositories/supabase-task-repository.ts (1)
SupabaseTaskRepository(7-110)packages/tm-core/src/auth/auth-manager.ts (1)
AuthManager(26-279)
scripts/modules/commands.js (2)
apps/cli/src/commands/context.command.ts (1)
ContextCommand(31-570)apps/cli/src/index.ts (1)
ContextCommand(9-9)
🔇 Additional comments (20)
packages/tm-core/src/types/database.types.ts (2)
219-244: Ensure audit fields on tasks writes
Confirm that every insert/update into thetaskstable sets bothupdated_byandupdated_at(as they’re non-nullable in the Row definition), either in the application’s repository/service layer or via database defaults/triggers.
404-406: Ensure exhaustive, explicit mappings in TaskMapper
- Verify
mapStatus()covers all DB values ('todo','in_progress','done'), maps them to the corresponding domain statuses, and throws or logs on unknown inputs- Verify
mapPriority()covers all DB values ('low','medium','high','urgent'), including a documented'urgent'↔'critical'mapping, and guards unknown prioritiesapps/cli/src/index.ts (1)
9-9: Re-export looks good; verify downstream usage compilesContextCommand is now part of the public CLI surface alongside Auth/List. No issues spotted.
packages/tm-core/src/auth/index.ts (1)
9-13: Public type surface expansion looks fineRe-exporting Organization/Brief/RemoteTask and CliData/UserContext from here is consistent with the new auth/session workflow.
Also applies to: 19-21
scripts/modules/commands.js (2)
19-19: Importing ContextCommand from @tm/cli is correctMatches the new CLI module wiring.
1748-1751: Registration placement is goodContextCommand registered alongside List/Auth; no conflicts spotted.
packages/tm-core/src/auth/oauth-service.ts (2)
185-186: CLI sign-in endpoint change acknowledgedSwitch to /auth/cli/sign-in with base64-encoded cli payload is fine assuming server side expects the cli param.
If the server still supports the legacy path, consider a fallback to reduce breakage during rollout.
275-315: PKCE and legacy flows look correctState check present, Supabase exchange and setSession usage are appropriate. Minimal risk noted after the timeout fix.
Also applies to: 325-339
packages/tm-core/src/auth/supabase-session-storage.ts (1)
9-11: Check custom implementation against v2.57.0’s SupportedStorage
Ensure yoursupabase-session-storage.tsmethods exactly match the official interface in @supabase/supabase-js@2.57.0 (e.g. promise-basedgetItem,setItem,removeItem, plus optionalisServer). If you see any missing or extra sync signatures, align them to:type SupportedStorage = PromisifyMethods<Pick<Storage, "getItem" | "setItem" | "removeItem">> & { isServer?: boolean };– adjust your class to return
Promise<string | null>,Promise<void>, etc., instead of sync types.packages/tm-core/src/repositories/supabase-task-repository.ts (1)
21-37: Query looks good and orders are explicitThe relational select and multi-column ordering are correct for building a stable backlog view.
apps/cli/src/commands/context.command.ts (1)
115-116: Prefer Commander’s error handling over process.exit for testabilityDirect
process.exit(1)makes the command hard to test and compose. Usethis.exitOverride()at program level or throw errors and let the CLI’s central handler exit.Also applies to: 197-199, 203-206, 207-209, 275-277, 285-286, 291-294, 295-297, 381-383, 388-391, 392-394, 428-430, 435-438, 439-441
⛔ Skipped due to learnings
Learnt from: joedanz PR: eyaltoledano/claude-task-master#748 File: scripts/modules/task-manager/parse-prd.js:726-733 Timestamp: 2025-07-21T17:51:07.239Z Learning: In CLI contexts within task-manager modules like scripts/modules/task-manager/parse-prd.js, using process.exit(1) for validation failures and error conditions is correct and preferred over throwing errors, as it provides immediate termination with appropriate exit codes for scripting. The code should distinguish between MCP contexts (throw errors) and CLI contexts (use process.exit).Learnt from: CR PR: eyaltoledano/claude-task-master#0 File: .cursor/rules/commands.mdc:0-0 Timestamp: 2025-07-31T22:07:49.716Z Learning: Applies to scripts/modules/commands.js : Check that required parameters are provided, provide clear error messages when parameters are missing, and use early returns with process.exit(1) for validation failures.Learnt from: CR PR: eyaltoledano/claude-task-master#0 File: .cursor/rules/commands.mdc:0-0 Timestamp: 2025-07-18T17:08:48.695Z Learning: Applies to scripts/modules/commands.js : Check that required parameters are provided, provide clear error messages when parameters are missing, and use early returns with process.exit(1) for validation failures.Learnt from: CR PR: eyaltoledano/claude-task-master#0 File: .cursor/rules/new_features.mdc:0-0 Timestamp: 2025-07-18T17:12:57.903Z Learning: Applies to scripts/modules/commands.js : Provide clear error messages for common failures, handle tagged system migration errors gracefully, include suggestions for resolution, and exit with appropriate codes for scripting in CLI commands.Learnt from: CR PR: eyaltoledano/claude-task-master#0 File: .cursor/rules/tests.mdc:0-0 Timestamp: 2025-07-18T17:16:13.793Z Learning: Applies to tests/{unit,integration,e2e}/**/*.test.js : When testing CLI commands built with Commander.js, test the command action handlers directly rather than trying to mock the entire Commander.js chain.Learnt from: CR PR: eyaltoledano/claude-task-master#0 File: .cursor/rules/commands.mdc:0-0 Timestamp: 2025-07-31T22:07:49.716Z Learning: Applies to scripts/modules/commands.js : Provide specific error handling for common issues, include troubleshooting hints for each error type, and use consistent error formatting across all commands.Learnt from: CR PR: eyaltoledano/claude-task-master#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.Learnt from: CR PR: eyaltoledano/claude-task-master#0 File: .cursor/rules/commands.mdc:0-0 Timestamp: 2025-07-31T22:07:49.716Z Learning: Applies to scripts/modules/commands.js : Set up global error handlers for uncaught exceptions, detect and format Commander-specific errors, and provide suitable guidance for fixing common errors.Learnt from: CR PR: eyaltoledano/claude-task-master#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.Learnt from: CR PR: eyaltoledano/claude-task-master#0 File: .cursor/rules/tests.mdc:0-0 Timestamp: 2025-07-18T17:16:13.793Z Learning: Applies to tests/{unit,integration,e2e}/**/*.test.js : Do not try to use the real action implementation without proper mocking, and do not mock Commander partially—either mock it completely or test the action directly.packages/tm-core/src/auth/auth-manager.ts (5)
101-145: Session-backed refresh and context preservation look correct.
171-176: Context getter is straightforward.
207-216: Clear-context flow looks good.
243-278: Thin wrappers over OrganizationService look good.
222-238: Outdated refactor suggestions—no action required
The codebase no longer contains anyas anycasts, and there is nocredentialStoreproperty to guard against; the proposed changes aren’t applicable.Likely an incorrect or invalid review comment.
packages/tm-core/src/services/organization.service.ts (1)
271-282: Access validation helpers look fine as existence checks.Also applies to: 287-295
packages/tm-core/src/clients/supabase-client.ts (3)
199-234: Refresh flow and error wrapping look solid.
260-279: Sign-out clears storage and logs appropriately.
284-311: Session setter is correct and guarded.
| return { | ||
| success: true, | ||
| action: 'select-brief', | ||
| context: this.authManager.getContext() || undefined, | ||
| message: `Selected brief: ${selectedBrief.name}` | ||
| }; |
There was a problem hiding this comment.
Fix brief message: selectedBrief.name is likely undefined
You construct briefName but the success message uses selectedBrief.name, which may not exist on the Brief object. Use briefName for consistency.
- return {
- success: true,
- action: 'select-brief',
- context: this.authManager.getContext() || undefined,
- message: `Selected brief: ${selectedBrief.name}`
- };
+ return {
+ success: true,
+ action: 'select-brief',
+ context: this.authManager.getContext() || undefined,
+ message: `Selected brief: ${briefName}`
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return { | |
| success: true, | |
| action: 'select-brief', | |
| context: this.authManager.getContext() || undefined, | |
| message: `Selected brief: ${selectedBrief.name}` | |
| }; | |
| return { | |
| success: true, | |
| action: 'select-brief', | |
| context: this.authManager.getContext() || undefined, | |
| message: `Selected brief: ${briefName}` | |
| }; |
🤖 Prompt for AI Agents
In apps/cli/src/commands/context.command.ts around lines 346 to 351, the success
message references selectedBrief.name which may be undefined; replace that with
the constructed briefName variable (or ensure briefName is used consistently) so
the returned message reads the selected brief label from briefName; update the
action return to use briefName for message interpolation and keep the rest of
the object unchanged.
| /** | ||
| * Execute set context with options | ||
| */ | ||
| private async executeSet(options: any): Promise<void> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type the options object instead of any
Avoid any to preserve type safety.
- private async executeSet(options: any): Promise<void> {
+ private async executeSet(options: { org?: string; orgName?: string; brief?: string; briefName?: string }): Promise<void> {- private async setContext(options: any): Promise<ContextResult> {
+ private async setContext(options: { org?: string; orgName?: string; brief?: string; briefName?: string }): Promise<ContextResult> {Also applies to: 447-447
| private constructor(config?: Partial<AuthConfig>) { | ||
| this.credentialStore = new CredentialStore(config); | ||
| this.supabaseClient = new SupabaseAuthClient(); | ||
| this.oauthService = new OAuthService(this.credentialStore, config); | ||
|
|
||
| // Initialize Supabase client with session restoration | ||
| this.initializeSupabaseSession(); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid floating async call in constructor.
Fire-and-forget async call can trip linters and mask errors. Explicitly mark it as intentionally unawaited.
- // Initialize Supabase client with session restoration
- this.initializeSupabaseSession();
+ // Initialize Supabase client with session restoration (fire-and-forget)
+ void this.initializeSupabaseSession();Outside this hunk, consider adding a class logger to avoid repeated factory calls:
// near other fields
private logger = getLogger('AuthManager');🤖 Prompt for AI Agents
In packages/tm-core/src/auth/auth-manager.ts around lines 33 to 41, the
constructor makes a fire-and-forget async call to initializeSupabaseSession
which can hide errors and trip linters; change the call to be explicitly
unawaited by prefixing it with void (i.e., use the void operator) or move
session initialization into an async init method that callers await, and ensure
any thrown errors are caught and logged; additionally, add a class-level logger
field (e.g., private logger = getLogger('AuthManager')) and use it to report
initialization errors instead of repeated factory calls.
| async updateContext(context: Partial<UserContext>): Promise<void> { | ||
| const credentials = this.getCredentials(); | ||
| if (!credentials) { | ||
| throw new AuthenticationError('Not authenticated', 'NOT_AUTHENTICATED'); | ||
| } | ||
|
|
||
| // Merge with existing context | ||
| const existingContext = credentials.selectedContext || {}; | ||
| const newContext: UserContext = { | ||
| ...existingContext, | ||
| ...context, | ||
| updatedAt: new Date().toISOString() | ||
| }; | ||
|
|
||
| // Save updated credentials with new context | ||
| const updatedCredentials: AuthCredentials = { | ||
| ...credentials, | ||
| selectedContext: newContext | ||
| }; | ||
|
|
||
| this.credentialStore.saveCredentials(updatedCredentials); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate org/brief before persisting context.
Prevent saving invalid IDs by validating access via OrganizationService before writing.
Example (outside this hunk):
const service = await this.getOrganizationService();
if (context.orgId) {
const ok = await service.validateOrgAccess(context.orgId);
if (!ok) throw new AuthenticationError('Invalid orgId', 'NOT_AUTHORIZED');
}
if (context.briefId) {
const ok = await service.validateBriefAccess(context.briefId);
if (!ok) throw new AuthenticationError('Invalid briefId', 'NOT_AUTHORIZED');
}🤖 Prompt for AI Agents
In packages/tm-core/src/auth/auth-manager.ts around lines 181 to 202, the method
updateContext currently persists orgId/briefId without verifying access; before
saving, obtain the OrganizationService via this.getOrganizationService() and, if
context.orgId is present, call service.validateOrgAccess(context.orgId) and
throw new AuthenticationError('Invalid orgId','NOT_AUTHORIZED') when it returns
false; similarly, if context.briefId is present call
service.validateBriefAccess(context.briefId) and throw new
AuthenticationError('Invalid briefId','NOT_AUTHORIZED') on failure; perform
these checks prior to merging/saving credentials so only validated IDs are
persisted.
| export { AuthManager } from './auth-manager.js'; | ||
| export { CredentialStore } from './credential-store.js'; | ||
| export { OAuthService } from './oauth-service.js'; | ||
| export { SupabaseSessionStorage } from './supabase-session-storage'; |
There was a problem hiding this comment.
Fix ESM path: missing .js extension will break Node ESM resolution
Other exports in this file include the .js suffix; this one should too.
Apply this diff:
-export { SupabaseSessionStorage } from './supabase-session-storage';
+export { SupabaseSessionStorage } from './supabase-session-storage.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export { SupabaseSessionStorage } from './supabase-session-storage'; | |
| export { SupabaseSessionStorage } from './supabase-session-storage.js'; |
🤖 Prompt for AI Agents
In packages/tm-core/src/auth/index.ts around line 8, the named export for
SupabaseSessionStorage is missing the .js extension which breaks Node ESM
resolution; update the export to reference './supabase-session-storage.js'
(i.e., add the .js suffix) so it matches the other exports and resolves
correctly under ESM.
| const tasks = await this.repository.getTasks(this.projectId); | ||
| if (tasks.length > 0) { | ||
| await this.repository.bulkDeleteTasks( | ||
| this.projectId, | ||
| tasks.map((t) => t.id) | ||
| ); | ||
| } | ||
|
|
||
| if (!response.success) { | ||
| throw new Error(response.error || 'Failed to clear data'); | ||
| // Delete all tags | ||
| const tags = await this.repository.getTags(this.projectId); | ||
| for (const tag of tags) { | ||
| await this.repository.deleteTag(this.projectId, tag.name); | ||
| } | ||
|
|
||
| // Clear cache | ||
| this.tagsCache.clear(); | ||
| } catch (error) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
clear() assumes bulk/tag ops exist; add safe fallbacks
When bulk delete or tag APIs aren’t implemented, fall back to per-task deletes and cache-only tag clear.
- const tasks = await this.repository.getTasks(this.projectId);
- if (tasks.length > 0) {
- await this.repository.bulkDeleteTasks(
- this.projectId,
- tasks.map((t) => t.id)
- );
- }
+ const tasks = await this.repository.getTasks(this.projectId);
+ if (tasks.length > 0) {
+ if (typeof (this.repository as any).bulkDeleteTasks === 'function') {
+ await (this.repository as any).bulkDeleteTasks(
+ this.projectId,
+ tasks.map((t) => t.id)
+ );
+ } else {
+ for (const t of tasks) {
+ await this.repository.deleteTask(this.projectId, t.id);
+ }
+ }
+ }
- const tags = await this.repository.getTags(this.projectId);
- for (const tag of tags) {
- await this.repository.deleteTag(this.projectId, tag.name);
- }
+ if (this.tagOpsAvailable) {
+ const tags = await (this.repository as any).getTags(this.projectId);
+ for (const tag of tags) {
+ await (this.repository as any).deleteTag(this.projectId, tag.name);
+ }
+ }
// Clear cache
this.tagsCache.clear();| import { ERROR_CODES, TaskMasterError } from '../errors/task-master-error.js'; | ||
| import { AuthManager } from '../auth/auth-manager.js'; | ||
| import { getLogger } from '../logger/index.js'; | ||
| import { SupabaseAuthClient } from '../clients/supabase-client.js'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Supabase introduced; reconcile with legacy Hamster config paths
Good to see SupabaseAuthClient, but the file still validates and prefers apiEndpoint/apiAccessToken (“Hamster”) in multiple branches. This will confuse users and can route into dead validation paths since createApiStorage no longer uses those fields.
Would you like me to open a follow-up PR to:
- replace isHamsterAvailable/validateStorageConfig/detectOptimalStorage messaging with Supabase-based detection, and
- deprecate apiEndpoint/apiAccessToken in favor of session-based auth?
🏁 Script executed:
#!/bin/bash
set -eux
# Print the first 200 lines of storage-factory.ts to inspect imports and initial logic
sed -n '1,200p' packages/tm-core/src/storage/storage-factory.ts
# Locate legacy Hamster detection/util methods in this file
grep -R -n "isHamsterAvailable" packages/tm-core/src/storage/storage-factory.ts || true
grep -R -n "validateStorageConfig" packages/tm-core/src/storage/storage-factory.ts || true
grep -R -n "detectOptimalStorage" packages/tm-core/src/storage/storage-factory.ts || true
# Show createApiStorage implementation to confirm which config fields it uses
grep -R -n "function createApiStorage" -n packages/tm-core/src/storage/storage-factory.ts -A20 || trueLength of output: 7165
Refactor StorageFactory to remove Hamster (apiEndpoint/apiAccessToken) logic in favor of Supabase session-based auth
- Update isHamsterAvailable, validateStorageConfig and detectOptimalStorage to rely on SupabaseAuthClient session detection
- Deprecate config.storage.apiEndpoint and apiAccessToken in favor of AuthManager session tokens
🤖 Prompt for AI Agents
In packages/tm-core/src/storage/storage-factory.ts around line 16, refactor
StorageFactory to remove the old "Hamster" (apiEndpoint/apiAccessToken) logic
and instead rely on SupabaseAuthClient session-based auth: replace any uses of
config.storage.apiEndpoint and apiAccessToken with calls to SupabaseAuthClient
methods that detect an active session (e.g., hasSession()/getSession()), update
isHamsterAvailable to return true only when SupabaseAuthClient reports a valid
session, change validateStorageConfig to validate presence of an active Supabase
session and mark apiEndpoint/apiAccessToken as deprecated (remove their runtime
checks), and update detectOptimalStorage to prefer Supabase-backed storage when
a session exists; also update types/comments and add a short
migration/deprecation note where config fields were removed.
| export type Json = | ||
| | string | ||
| | number | ||
| | boolean | ||
| | null | ||
| | { [key: string]: Json | undefined } | ||
| | Json[]; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mark file as generated to prevent drift
If this is Supabase-generated, add a header to discourage manual edits and ease regen diffs.
+/** THIS FILE IS AUTO-GENERATED. DO NOT EDIT MANUALLY.
+ * Source: Supabase schema types
+ */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type Json = | |
| | string | |
| | number | |
| | boolean | |
| | null | |
| | { [key: string]: Json | undefined } | |
| | Json[]; | |
| /** THIS FILE IS AUTO-GENERATED. DO NOT EDIT MANUALLY. | |
| * Source: Supabase schema types | |
| */ | |
| export type Json = | |
| | string | |
| | number | |
| | boolean | |
| | null | |
| | { [key: string]: Json | undefined } | |
| | Json[]; |
🤖 Prompt for AI Agents
In packages/tm-core/src/types/database.types.ts around lines 1 to 8, this file
appears to be Supabase-generated and should be marked as generated to prevent
accidental manual edits and reduce churn during regenerations; add a standard
generated-file header comment at the very top (e.g., "// This file is generated
by <tool> - DO NOT EDIT MANUALLY." optionally including generation
command/source and timestamp), and ensure the header is consistent with other
generated files in the repo so tooling and reviewers recognize it.
| /** | ||
| * Task tag for organizing tasks | ||
| */ | ||
| export interface TaskTag { | ||
| name: string; | ||
| tasks: string[]; // Task IDs belonging to this tag | ||
| metadata: Record<string, any>; | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Tighten TaskTag typing, add a guard, and prevent runtime import pitfalls
- Use safer metadata typing and immutable task IDs.
- Consider a stable identifier (id/slug) distinct from display name.
Apply:
-export interface TaskTag {
- name: string;
- tasks: string[]; // Task IDs belonging to this tag
- metadata: Record<string, any>;
-}
+export interface TaskTag {
+ /** Human-friendly label; not guaranteed unique if you later add slugs */
+ name: string;
+ /** Task IDs belonging to this tag */
+ tasks: readonly string[];
+ /** Arbitrary structured data; avoid `any` to preserve type-safety */
+ metadata: Record<string, unknown>;
+}Optional (outside this hunk): add a runtime guard to match your existing pattern of type guards in this file.
export function isTaskTag(obj: unknown): obj is TaskTag {
if (!obj || typeof obj !== 'object') return false;
const t = obj as Record<string, unknown>;
return (
typeof t.name === 'string' &&
Array.isArray(t.tasks) &&
t.tasks.every(id => typeof id === 'string')
);
}Also, ensure downstream imports use type-only syntax to avoid ESM runtime import of a type:
- Replace
import { TaskTag } from '../types/index.js'withimport type { TaskTag } from '../types/index.js'wherever applicable.
Per your Taskmaster tag rules (retrieved learnings), ensure repository/storage operations are tag-aware and go through the tag resolution layer when reading/writing tasks associated with TaskTag.
🏁 Script executed:
#!/bin/bash
# Find non type-only imports of TaskTag to prevent runtime import errors under verbatimModuleSyntax.
rg -nP "import\s*\{\s*TaskTag\s*\}\s*from\s*['\"][^'\"]+index\.js['\"]"Length of output: 0
Tighten TaskTag typing and add a runtime guard
- In packages/tm-core/src/types/index.ts, change
•tasks: string[];→tasks: readonly string[];
•metadata: Record<string, any>;→metadata: Record<string, unknown>; - Add this in the same file to validate at runtime:
export function isTaskTag(obj: unknown): obj is TaskTag {
if (!obj || typeof obj !== 'object') return false;
const t = obj as Record<string, unknown>;
return (
typeof t.name === 'string' &&
Array.isArray(t.tasks) &&
t.tasks.every(id => typeof id === 'string')
);
}- No non-
typeimports ofTaskTagwere found; wherever you consume it, use
import type { TaskTag } from '…/types';
🤖 Prompt for AI Agents
In packages/tm-core/src/types/index.ts around lines 99 to 107, tighten the
TaskTag type and add a runtime guard: change tasks from mutable to readonly
(tasks: readonly string[]) and metadata to Record<string, unknown>, and add an
exported isTaskTag(obj: unknown): obj is TaskTag function that checks obj is an
object, verifies typeof name === 'string', Array.isArray(tasks) and every task
id is typeof 'string'; also ensure consumers only import TaskTag as a type
(import type { TaskTag } ...) where applicable.
| import { | ||
| createClient, | ||
| SupabaseClient as SupabaseJSClient, | ||
| User, | ||
| Session | ||
| } from '@supabase/supabase-js'; | ||
| import { AuthenticationError } from '../auth/types.js'; | ||
| import { getLogger } from '../logger/index.js'; | ||
| import { SupabaseSessionStorage } from '../auth/supabase-session-storage'; | ||
| import { CredentialStore } from '../auth/credential-store'; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return a typed Supabase client to propagate Database types.
Typing the client removes the downstream any-cast in AuthManager.
import {
createClient,
- SupabaseClient as SupabaseJSClient,
+ SupabaseClient as SupabaseJSClient,
User,
Session
} from '@supabase/supabase-js';
import { AuthenticationError } from '../auth/types.js';
import { getLogger } from '../logger/index.js';
+import { Database } from '../types/database.types.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { | |
| createClient, | |
| SupabaseClient as SupabaseJSClient, | |
| User, | |
| Session | |
| } from '@supabase/supabase-js'; | |
| import { AuthenticationError } from '../auth/types.js'; | |
| import { getLogger } from '../logger/index.js'; | |
| import { SupabaseSessionStorage } from '../auth/supabase-session-storage'; | |
| import { CredentialStore } from '../auth/credential-store'; | |
| import { | |
| createClient, | |
| SupabaseClient as SupabaseJSClient, | |
| User, | |
| Session | |
| } from '@supabase/supabase-js'; | |
| import { AuthenticationError } from '../auth/types.js'; | |
| import { getLogger } from '../logger/index.js'; | |
| import { Database } from '../types/database.types.js'; | |
| import { SupabaseSessionStorage } from '../auth/supabase-session-storage'; | |
| import { CredentialStore } from '../auth/credential-store'; |
🤖 Prompt for AI Agents
In packages/tm-core/src/clients/supabase-client.ts around lines 5 to 15, the
Supabase client is untyped which forces downstream any-casts; import your
project Database types (e.g. a Database interface/type that describes your
schema), use that type as the generic parameter when calling createClient
(createClient<Database>(...)), and update the local Supabase client type alias
to SupabaseJSClient<Database> so the returned client is strongly typed and
database types propagate through AuthManager and other consumers.
| import { SupabaseSessionStorage } from '../auth/supabase-session-storage'; | ||
| import { CredentialStore } from '../auth/credential-store'; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix ESM import paths (.js extensions).
Without .js, NodeNext/ESM resolution can fail at runtime.
-import { SupabaseSessionStorage } from '../auth/supabase-session-storage';
-import { CredentialStore } from '../auth/credential-store';
+import { SupabaseSessionStorage } from '../auth/supabase-session-storage.js';
+import { CredentialStore } from '../auth/credential-store.js';🏁 Script executed:
#!/bin/bash
# Check for missing .js in local ESM imports
rg -nP "from\\s+['\"]\\.\\./[^'\".]+['\"]" -g 'packages/**/*.ts' -SLength of output: 440
🏁 Script executed:
#!/bin/bash
# Find TS config files
fd -e json tsconfig*.json
# Display root tsconfig.json if it exists
if [ -f tsconfig.json ]; then
echo "=== tsconfig.json ==="
cat tsconfig.json
else
echo "No root tsconfig.json"
fi
# Display tm-core tsconfig.json if it exists
if [ -f packages/tm-core/tsconfig.json ]; then
echo "=== packages/tm-core/tsconfig.json ==="
cat packages/tm-core/tsconfig.json
else
echo "No packages/tm-core/tsconfig.json"
fi
# Display root package.json
if [ -f package.json ]; then
echo "=== package.json ==="
cat package.json
else
echo "No root package.json"
fi
# Display tm-core package.json
if [ -f packages/tm-core/package.json ]; then
echo "=== packages/tm-core/package.json ==="
cat packages/tm-core/package.json
else
echo "No packages/tm-core/package.json"
fi
# Check for ESM declaration in any package.json
grep -R '"type"\s*:\s*"module"' -n .Length of output: 9743
Add .js extensions to local ESM imports. Node.js ESM default resolution (“explicit” mode) requires full file extensions in import specifiers; omission leads to ERR_MODULE_NOT_FOUND at runtime (nodejs.org, stackoverflow.com)
packages/tm-core/src/clients/supabase-client.ts lines 13–14
-import { SupabaseSessionStorage } from '../auth/supabase-session-storage';
-import { CredentialStore } from '../auth/credential-store';
+import { SupabaseSessionStorage } from '../auth/supabase-session-storage.js';
+import { CredentialStore } from '../auth/credential-store.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { SupabaseSessionStorage } from '../auth/supabase-session-storage'; | |
| import { CredentialStore } from '../auth/credential-store'; | |
| import { SupabaseSessionStorage } from '../auth/supabase-session-storage.js'; | |
| import { CredentialStore } from '../auth/credential-store.js'; |
🤖 Prompt for AI Agents
In packages/tm-core/src/clients/supabase-client.ts around lines 13 to 15, the
local ESM imports omit file extensions which breaks Node.js explicit ESM
resolution; update the import specifiers to include the .js extension (e.g.,
'../auth/supabase-session-storage.js' and '../auth/credential-store.js') so the
files resolve at runtime, and ensure any corresponding TypeScript build/emit
preserves or maps these extensions correctly.
| private client: SupabaseJSClient | null = null; | ||
| private sessionStorage: SupabaseSessionStorage; | ||
| private logger = getLogger('SupabaseAuthClient'); | ||
|
|
||
| constructor() { | ||
| const credentialStore = new CredentialStore(); | ||
| this.sessionStorage = new SupabaseSessionStorage(credentialStore); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Propagate Database generic through fields and method.
-export class SupabaseAuthClient {
- private client: SupabaseJSClient | null = null;
+export class SupabaseAuthClient {
+ private client: SupabaseJSClient<Database> | null = null;
private sessionStorage: SupabaseSessionStorage;
private logger = getLogger('SupabaseAuthClient');
constructor() {
const credentialStore = new CredentialStore();
this.sessionStorage = new SupabaseSessionStorage(credentialStore);
}
/**
* Get Supabase client with proper session management
*/
- getClient(): SupabaseJSClient {
+ getClient(): SupabaseJSClient<Database> {Also applies to: 29-31
🤖 Prompt for AI Agents
In packages/tm-core/src/clients/supabase-client.ts around lines 17-24 (and also
29-31), the Supabase client and related fields are untyped for the DB schema;
update the class to propagate the Database generic everywhere: make the class or
client fields use SupabaseJSClient<Database> (or SupabaseClient<Database> per
your import), change private client: SupabaseJSClient | null to private client:
SupabaseJSClient<Database> | null, update sessionStorage and any methods on
lines 29-31 to accept/return the same generic-typed client or storage types
(e.g., SupabaseSessionStorage<Database> if applicable), and adjust
constructor/initialization types accordingly so the Database generic flows
through all fields and method signatures.
| // Create client with custom storage adapter (similar to React Native AsyncStorage) | ||
| this.client = createClient(supabaseUrl, supabaseAnonKey, { | ||
| auth: { | ||
| storage: this.sessionStorage, | ||
| autoRefreshToken: true, | ||
| persistSession: false, // We handle persistence ourselves | ||
| persistSession: true, | ||
| detectSessionInUrl: false | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Instantiate client with Database generic.
- this.client = createClient(supabaseUrl, supabaseAnonKey, {
+ this.client = createClient<Database>(supabaseUrl, supabaseAnonKey, {
auth: {
storage: this.sessionStorage,
autoRefreshToken: true,
persistSession: true,
detectSessionInUrl: false
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Create client with custom storage adapter (similar to React Native AsyncStorage) | |
| this.client = createClient(supabaseUrl, supabaseAnonKey, { | |
| auth: { | |
| storage: this.sessionStorage, | |
| autoRefreshToken: true, | |
| persistSession: false, // We handle persistence ourselves | |
| persistSession: true, | |
| detectSessionInUrl: false | |
| } | |
| }); | |
| // Create client with custom storage adapter (similar to React Native AsyncStorage) | |
| this.client = createClient<Database>(supabaseUrl, supabaseAnonKey, { | |
| auth: { | |
| storage: this.sessionStorage, | |
| autoRefreshToken: true, | |
| persistSession: true, | |
| detectSessionInUrl: false | |
| } | |
| }); |
🤖 Prompt for AI Agents
In packages/tm-core/src/clients/supabase-client.ts around lines 42 to 50, the
Supabase client is created without the Database generic which loses typed
table/row definitions; import or reference your Database type (e.g., from your
db schema types) and call createClient with that generic
(createClient<Database>(...)) so the client is strongly typed for queries and
storage; update the createClient call to include the generic and ensure the
Database type is imported/available in this file.
| * Sign in with PKCE flow (for CLI auth) | ||
| */ | ||
| async refreshSession(refreshToken: string): Promise<{ | ||
| token: string; | ||
| refreshToken?: string; | ||
| expiresAt?: string; | ||
| }> { | ||
| try { | ||
| const client = this.getClient(); | ||
| async signInWithPKCE(): Promise<{ url: string; codeVerifier: string }> { | ||
| const client = this.getClient(); | ||
|
|
||
| this.logger.info('Refreshing session...'); | ||
|
|
||
| // Set the session with refresh token | ||
| const { data, error } = await client.auth.refreshSession({ | ||
| refresh_token: refreshToken | ||
| try { | ||
| // Generate PKCE challenge | ||
| const { data, error } = await client.auth.signInWithOAuth({ | ||
| provider: 'github', | ||
| options: { | ||
| redirectTo: | ||
| process.env.TM_AUTH_CALLBACK_URL || | ||
| 'http://localhost:3421/auth/callback', | ||
| scopes: 'email' | ||
| } | ||
| }); | ||
|
|
||
| if (error) { | ||
| this.logger.error('Failed to refresh session:', error); | ||
| throw new AuthenticationError( | ||
| `Failed to refresh session: ${error.message}`, | ||
| 'REFRESH_FAILED' | ||
| `Failed to initiate PKCE flow: ${error.message}`, | ||
| 'PKCE_INIT_FAILED' | ||
| ); | ||
| } | ||
|
|
||
| if (!data.session) { | ||
| if (!data?.url) { | ||
| throw new AuthenticationError( | ||
| 'No session data returned', | ||
| 'No authorization URL returned', | ||
| 'INVALID_RESPONSE' | ||
| ); | ||
| } | ||
|
|
||
| this.logger.info('Successfully refreshed session'); | ||
|
|
||
| // Extract code_verifier from the URL or generate it | ||
| // Note: Supabase handles PKCE internally, we just need to handle the callback | ||
| return { | ||
| token: data.session.access_token, | ||
| refreshToken: data.session.refresh_token, | ||
| expiresAt: data.session.expires_at | ||
| ? new Date(data.session.expires_at * 1000).toISOString() | ||
| : undefined | ||
| url: data.url, | ||
| codeVerifier: '' // Supabase manages this internally | ||
| }; | ||
| } catch (error) { | ||
| if (error instanceof AuthenticationError) { | ||
| throw error; | ||
| } | ||
|
|
||
| throw new AuthenticationError( | ||
| `Failed to start PKCE flow: ${(error as Error).message}`, | ||
| 'PKCE_FAILED' | ||
| ); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
PKCE API shape: drop unused codeVerifier from return.
Supabase manages PKCE internally; returning a dummy field is confusing. Consider returning only the authorization URL.
🤖 Prompt for AI Agents
In packages/tm-core/src/clients/supabase-client.ts around lines 86 to 134, the
signInWithPKCE method returns a misleading empty codeVerifier field even though
Supabase manages PKCE internally; remove the unused codeVerifier from the
returned object and update the method signature/Promise return type to only
return the authorization URL (e.g., Promise<{ url: string }>) so callers won’t
receive or rely on a dummy value; ensure any references to codeVerifier in this
method are deleted and update any consuming code/types to match the new return
shape.
| /** | ||
| * Brief data structure | ||
| */ | ||
| export interface Brief { | ||
| id: string; | ||
| accountId: string; | ||
| documentId: string; | ||
| status: string; | ||
| createdAt: string; | ||
| updatedAt: string; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Brief status should use DB enum type.
Align with generated types for stronger guarantees.
export interface Brief {
id: string;
accountId: string;
documentId: string;
- status: string;
+ status: Database['public']['Enums']['brief_status'];
createdAt: string;
updatedAt: string;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Brief data structure | |
| */ | |
| export interface Brief { | |
| id: string; | |
| accountId: string; | |
| documentId: string; | |
| status: string; | |
| createdAt: string; | |
| updatedAt: string; | |
| } | |
| export interface Brief { | |
| id: string; | |
| accountId: string; | |
| documentId: string; | |
| status: Database['public']['Enums']['brief_status']; | |
| createdAt: string; | |
| updatedAt: string; | |
| } |
🤖 Prompt for AI Agents
In packages/tm-core/src/services/organization.service.ts around lines 20 to 31,
the Brief interface uses status: string but should use the generated DB enum
type for stronger typing; import the appropriate generated enum (e.g.,
BriefStatus or the exact enum name from your prisma/generated types) at the top
of the file and replace status: string with status: ThatEnumType, and update any
code that constructs or asserts Brief objects to use the enum values instead of
raw strings.
| export interface RemoteTask { | ||
| id: string; | ||
| briefId: string; | ||
| documentId: string; | ||
| position: number | null; | ||
| subtaskPosition: number | null; | ||
| status: string; | ||
| createdAt: string; | ||
| updatedAt: string; | ||
| // Document details from join | ||
| document?: { | ||
| id: string; | ||
| document_name: string; | ||
| title: string; | ||
| description: string; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
RemoteTask interface mismatches DB schema.
- brief_id/document_id can be null.
- position/subtask_position are non-null numbers in schema.
- status is an enum.
- document.description can be null.
export interface RemoteTask {
id: string;
- briefId: string;
- documentId: string;
- position: number | null;
- subtaskPosition: number | null;
- status: string;
+ briefId: string | null;
+ documentId: string | null;
+ position: number;
+ subtaskPosition: number;
+ status: Database['public']['Enums']['task_status'];
createdAt: string;
updatedAt: string;
// Document details from join
document?: {
id: string;
document_name: string;
title: string;
- description: string;
+ description: string | null;
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface RemoteTask { | |
| id: string; | |
| briefId: string; | |
| documentId: string; | |
| position: number | null; | |
| subtaskPosition: number | null; | |
| status: string; | |
| createdAt: string; | |
| updatedAt: string; | |
| // Document details from join | |
| document?: { | |
| id: string; | |
| document_name: string; | |
| title: string; | |
| description: string; | |
| }; | |
| } | |
| export interface RemoteTask { | |
| id: string; | |
| briefId: string | null; | |
| documentId: string | null; | |
| position: number; | |
| subtaskPosition: number; | |
| status: Database['public']['Enums']['task_status']; | |
| createdAt: string; | |
| updatedAt: string; | |
| // Document details from join | |
| document?: { | |
| id: string; | |
| document_name: string; | |
| title: string; | |
| description: string | null; | |
| }; | |
| } |
🤖 Prompt for AI Agents
In packages/tm-core/src/services/organization.service.ts around lines 35 to 51,
the RemoteTask interface does not match the DB schema: update briefId and
documentId to allow null (string | null), change position and subtaskPosition to
be non-nullable numbers (number), tighten status to the enum/type used by the DB
(replace string with the appropriate union or exported enum type), and mark
document.description as nullable (string | null); ensure any imports or exported
types used for the status enum are referenced and that optional/nullable
semantics align with how tasks are returned from the DB.
| async getOrganizations(): Promise<Organization[]> { | ||
| try { | ||
| // The user is already authenticated via the Authorization header | ||
| // Query the user_accounts view/table (filtered by RLS for current user) | ||
| const { data, error } = await this.supabaseClient | ||
| .from('user_accounts') | ||
| .select(` | ||
| id, | ||
| name, | ||
| slug | ||
| `); | ||
|
|
||
| if (error) { | ||
| throw new TaskMasterError( | ||
| `Failed to fetch organizations: ${error.message}`, | ||
| ERROR_CODES.API_ERROR, | ||
| { operation: 'getOrganizations' }, | ||
| error | ||
| ); | ||
| } | ||
|
|
||
| if (!data || data.length === 0) { | ||
| this.logger.debug('No organizations found for user'); | ||
| return []; | ||
| } | ||
|
|
||
| // Map to our Organization interface | ||
| return data.map((org) => ({ | ||
| id: org.id ?? '', | ||
| name: org.name ?? '', | ||
| slug: org.slug ?? org.id ?? '' // Use ID as fallback if slug is null | ||
| })); | ||
| } catch (error) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid emitting empty-string IDs; filter or hard-fail on nulls.
Returning id: '' hides data issues and can propagate bugs.
- return data.map((org) => ({
- id: org.id ?? '',
- name: org.name ?? '',
- slug: org.slug ?? org.id ?? '' // Use ID as fallback if slug is null
- }));
+ const rows = data.filter((o) => o.id); // ensure non-null
+ return rows.map((org) => ({
+ id: org.id as string,
+ name: org.name ?? '',
+ slug: org.slug ?? (org.id as string)
+ }));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async getOrganizations(): Promise<Organization[]> { | |
| try { | |
| // The user is already authenticated via the Authorization header | |
| // Query the user_accounts view/table (filtered by RLS for current user) | |
| const { data, error } = await this.supabaseClient | |
| .from('user_accounts') | |
| .select(` | |
| id, | |
| name, | |
| slug | |
| `); | |
| if (error) { | |
| throw new TaskMasterError( | |
| `Failed to fetch organizations: ${error.message}`, | |
| ERROR_CODES.API_ERROR, | |
| { operation: 'getOrganizations' }, | |
| error | |
| ); | |
| } | |
| if (!data || data.length === 0) { | |
| this.logger.debug('No organizations found for user'); | |
| return []; | |
| } | |
| // Map to our Organization interface | |
| return data.map((org) => ({ | |
| id: org.id ?? '', | |
| name: org.name ?? '', | |
| slug: org.slug ?? org.id ?? '' // Use ID as fallback if slug is null | |
| })); | |
| } catch (error) { | |
| async getOrganizations(): Promise<Organization[]> { | |
| try { | |
| // The user is already authenticated via the Authorization header | |
| // Query the user_accounts view/table (filtered by RLS for current user) | |
| const { data, error } = await this.supabaseClient | |
| .from('user_accounts') | |
| .select(` | |
| id, | |
| name, | |
| slug | |
| `); | |
| if (error) { | |
| throw new TaskMasterError( | |
| `Failed to fetch organizations: ${error.message}`, | |
| ERROR_CODES.API_ERROR, | |
| { operation: 'getOrganizations' }, | |
| error | |
| ); | |
| } | |
| if (!data || data.length === 0) { | |
| this.logger.debug('No organizations found for user'); | |
| return []; | |
| } | |
| // Filter out any rows lacking a valid ID, then map to our Organization interface | |
| const rows = data.filter((o) => o.id); // ensure non-null | |
| return rows.map((org) => ({ | |
| id: org.id as string, | |
| name: org.name ?? '', | |
| slug: org.slug ?? (org.id as string) | |
| })); | |
| } catch (error) { | |
| // existing catch logic... | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/tm-core/src/services/organization.service.ts around lines 64 to 96,
avoid returning organizations with id set to an empty string; validate the
fetched data for missing/null ids and fail fast or filter them out. After
receiving data, check each record and if any has id == null/undefined either
throw a TaskMasterError with context (operation: 'getOrganizations') so the
issue is surfaced, or remove those records and this.logger.warn about discarded
entries; then map remaining items using id as a required field (no empty-string
fallback) and only use slug || id when id is present.
| if (!data || data.length === 0) { | ||
| this.logger.debug(`No briefs found for organization ${orgId}`); | ||
| return []; | ||
| } | ||
|
|
||
| // Map to our Brief interface | ||
| return data.map((brief: any) => ({ | ||
| id: brief.id, | ||
| accountId: brief.account_id, | ||
| documentId: brief.document_id, | ||
| status: brief.status, | ||
| createdAt: brief.created_at, | ||
| updatedAt: brief.updated_at | ||
| })); | ||
| } catch (error) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove any and strongly type brief rows.
- // Map to our Brief interface
- return data.map((brief: any) => ({
- id: brief.id,
- accountId: brief.account_id,
- documentId: brief.document_id,
- status: brief.status,
- createdAt: brief.created_at,
- updatedAt: brief.updated_at
- }));
+ const rows = data as Database['public']['Tables']['brief']['Row'][];
+ return rows.map((b) => ({
+ id: b.id,
+ accountId: b.account_id,
+ documentId: b.document_id,
+ status: b.status,
+ createdAt: b.created_at,
+ updatedAt: b.updated_at
+ }));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!data || data.length === 0) { | |
| this.logger.debug(`No briefs found for organization ${orgId}`); | |
| return []; | |
| } | |
| // Map to our Brief interface | |
| return data.map((brief: any) => ({ | |
| id: brief.id, | |
| accountId: brief.account_id, | |
| documentId: brief.document_id, | |
| status: brief.status, | |
| createdAt: brief.created_at, | |
| updatedAt: brief.updated_at | |
| })); | |
| } catch (error) { | |
| if (!data || data.length === 0) { | |
| this.logger.debug(`No briefs found for organization ${orgId}`); | |
| return []; | |
| } | |
| const rows = data as Database['public']['Tables']['brief']['Row'][]; | |
| return rows.map((b) => ({ | |
| id: b.id, | |
| accountId: b.account_id, | |
| documentId: b.document_id, | |
| status: b.status, | |
| createdAt: b.created_at, | |
| updatedAt: b.updated_at | |
| })); | |
| } catch (error) { |
🤖 Prompt for AI Agents
In packages/tm-core/src/services/organization.service.ts around lines 187 to
201, the mapping currently uses an untyped "any" for brief rows; replace this
with a concrete type/interface that models the DB row (e.g., fields id,
account_id, document_id, status, created_at, updated_at), update the map
signature to use that type instead of any, and adjust any field access if names
differ so the returned objects are strongly typed to the Brief interface.
| .from('tasks') | ||
| .select(` | ||
| *, | ||
| document:document_id ( | ||
| id, | ||
| document_name, | ||
| title, | ||
| description | ||
| ) | ||
| `) | ||
| .eq('brief_id', briefId) | ||
| .order('position', { ascending: true }) | ||
| .order('subtask_position', { ascending: true }) | ||
| .order('created_at', { ascending: true }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
*Avoid SELECT ; fetch only required task fields.
Reduces payload and improves performance, especially with JSON columns.
- .select(`
- *,
- document:document_id (
- id,
- document_name,
- title,
- description
- )
- `)
+ .select(`
+ id, brief_id, document_id, position, subtask_position, status, created_at, updated_at,
+ document:document_id ( id, document_name, title, description )
+ `)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .from('tasks') | |
| .select(` | |
| *, | |
| document:document_id ( | |
| id, | |
| document_name, | |
| title, | |
| description | |
| ) | |
| `) | |
| .eq('brief_id', briefId) | |
| .order('position', { ascending: true }) | |
| .order('subtask_position', { ascending: true }) | |
| .order('created_at', { ascending: true }); | |
| .from('tasks') | |
| .select(` | |
| id, brief_id, document_id, position, subtask_position, status, created_at, updated_at, | |
| document:document_id ( id, document_name, title, description ) | |
| `) | |
| .eq('brief_id', briefId) | |
| .order('position', { ascending: true }) | |
| .order('subtask_position', { ascending: true }) | |
| .order('created_at', { ascending: true }); |
🤖 Prompt for AI Agents
In packages/tm-core/src/services/organization.service.ts around lines 303 to
317, the query currently uses SELECT * for tasks which pulls all columns
(including potentially large JSON columns); replace the wildcard with an
explicit list of only the task fields your caller needs (for example id,
brief_id, title, description, position, subtask_position, created_at — or the
exact minimal set used by the consumer), keep the existing document:document_id
subselect as-is, and update the select(...) string to enumerate those task
columns so the payload and JSON data transferred are minimized and performance
is improved.
b9e3eec to
cf1687b
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.changeset/pre.json (1)
9-15: Validate changeset coverage for user-facing workThis PR appears to add CLI/context/auth/repo features; verify each user-facing change has a corresponding changeset entry and correct semver bump. If any are missing, add a changeset before exiting pre.
I can draft the missing changesets if you confirm package impact and bump levels.
.github/workflows/claude-docs-updater.yml (2)
91-108: Fix commit message quoting (breaks when commit_message has quotes/newlines)Interpolating a JSON-encoded string into a double-quoted
-mbreaks the shell. Use a heredoc and decode JSON.- - name: Check if changes were made + - name: Check if changes were made id: check-changes run: | if git diff --quiet; then echo "has_changes=false" >> $GITHUB_OUTPUT else echo "has_changes=true" >> $GITHUB_OUTPUT git add -A git config --local user.email "github-actions[bot]@users.noreply.github.com" git config --local user.name "github-actions[bot]" - git commit -m "docs: auto-update documentation based on changes in next branch - - This PR was automatically generated to update documentation based on recent changes. - - Original commit: ${{ inputs.commit_message }} - - Co-authored-by: Claude <claude-assistant@anthropic.com>" + COMMIT_MSG=$(cat <<'MSG' +docs: auto-update documentation based on changes in next branch + +This PR was automatically generated to update documentation based on recent changes. + +Original commit: +${{ fromJSON(inputs.commit_message) }} + +Co-authored-by: Claude <claude-assistant@anthropic.com> +MSG +) + git commit -m "$COMMIT_MSG" fi
110-146: Use heredoc for PR body and decode JSON; current quoting can break gh CLIThe double-quoted body will break with quotes/newlines from inputs. Switch to heredoc and decode.
- gh pr create \ + gh pr create \ --title "docs: update documentation for recent changes" \ - --body "## 📚 Documentation Update -... - **Commit:** ${{ inputs.commit_sha }} - **Message:** ${{ inputs.commit_message }} - - ### Changed Files in Original Commit - ``` - ${{ inputs.changed_files }} - ``` -... - *This PR was automatically generated by Claude Code GitHub Action*" \ + --body-file <(cat <<'PRBODY' +## 📚 Documentation Update + +This PR automatically updates documentation based on recent changes merged to the `next` branch. + +### Original Changes +**Commit:** ${{ inputs.commit_sha }} +**Message:** ${{ fromJSON(inputs.commit_message) }} + +### Changed Files in Original Commit +``` +${{ fromJSON(inputs.changed_files) }} +``` + +### Documentation Updates +This PR includes documentation updates to reflect the changes above. Please review to ensure: +- [ ] Documentation accurately reflects the changes +- [ ] Examples are correct and working +- [ ] No important details are missing +- [ ] Style is consistent with existing documentation + +--- +*This PR was automatically generated by Claude Code GitHub Action* +PRBODY +) \ --base next \ --head ${{ steps.create-branch.outputs.branch_name }} \ --label "documentation" \ --label "automated"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
.changeset/pre.json(1 hunks).github/workflows/claude-docs-trigger.yml(1 hunks).github/workflows/claude-docs-updater.yml(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*
📄 CodeRabbit inference engine (.cursor/rules/new_features.mdc)
Create appropriate changesets for new features, use semantic versioning, include tagged system information in release notes, and document breaking changes if any.
Files:
.changeset/pre.json
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/git_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:31.810Z
Learning: Pull Request descriptions must use the provided template, including Task Overview, Subtasks Completed, Implementation Details, Testing, Breaking Changes, and Related Tasks
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
🪛 YAMLlint (1.37.1)
.github/workflows/claude-docs-trigger.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[warning] 24-24: too few spaces before comment: expected 2
(comments)
[error] 57-57: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/claude-docs-updater.yml
[warning] 35-35: too few spaces before comment: expected 2
(comments)
[error] 127-127: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.changeset/pre.json (1)
2-2: Confirm intent to exit pre-release modeSwitching
"mode": "exit"finalizes the pre cycle. Ensure this aligns with release timing and CI/publish steps that consume Changesets pre-state.
| jobs: | ||
| trigger-docs-update: | ||
| # Only run if changes were merged (not direct pushes from bots) | ||
| if: github.actor != 'github-actions[bot]' && github.actor != 'dependabot[bot]' | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| actions: write | ||
| steps: |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Confirm permissions/token can dispatch workflows
GITHUB_TOKEN sometimes cannot trigger workflow_dispatch in the same repo, depending on org settings. If you see “Resource not accessible by integration,” use a PAT with workflow scope (e.g., GH_TOKEN secret) or enable “Allow GitHub Actions to create and approve pull requests” in repo settings.
I can provide a minimal test workflow to validate dispatch with the current token.
🤖 Prompt for AI Agents
.github/workflows/claude-docs-trigger.yml lines 12-20 — the current workflow may
fail to dispatch other workflows because GITHUB_TOKEN can be restricted by org
settings; replace usage of the default GITHUB_TOKEN with a repository secret
containing a Personal Access Token (PAT) that has the "workflow" scope (e.g.,
GH_TOKEN) and update the workflow to use that secret for any dispatch API calls
and ensure job permissions include workflows: write (or actions: write if
dispatch is via actions API) instead of only contents: read; alternatively, if
you prefer not to use a PAT, enable "Allow GitHub Actions to create and approve
pull requests" / permit GitHub Actions to create workflow dispatches in the
repo/org settings; finally, add a minimal test workflow that attempts a
workflow_dispatch using the chosen token to verify the token/permissions work
and fail fast if dispatch returns "Resource not accessible by integration."
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 2 # Need previous commit for comparison |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Pad inline comment for YAML lint
Minor formatting.
- fetch-depth: 2 # Need previous commit for comparison
+ fetch-depth: 2 # Need previous commit for comparison📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fetch-depth: 2 # Need previous commit for comparison | |
| fetch-depth: 2 # Need previous commit for comparison |
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 24-24: too few spaces before comment: expected 2
(comments)
🤖 Prompt for AI Agents
In .github/workflows/claude-docs-trigger.yml at line 24, the inline comment on
the fetch-depth setting needs padding for YAML linting; add one extra space
between the value and the '#' so the line reads with two spaces before the
comment (e.g., "fetch-depth: 2 # Need previous commit for comparison"), then
save and re-run the linter/CI.
| - name: Get changed files | ||
| id: changed-files | ||
| run: | | ||
| echo "Changed files in this push:" | ||
| git diff --name-only HEAD^ HEAD | tee changed_files.txt | ||
|
|
||
| # Store changed files for Claude to analyze (escaped for JSON) | ||
| CHANGED_FILES=$(git diff --name-only HEAD^ HEAD | jq -Rs .) | ||
| echo "changed_files=$CHANGED_FILES" >> $GITHUB_OUTPUT | ||
|
|
||
| # Get the commit message (escaped for JSON) | ||
| COMMIT_MSG=$(git log -1 --pretty=%B | jq -Rs .) | ||
| echo "commit_message=$COMMIT_MSG" >> $GITHUB_OUTPUT | ||
|
|
||
| # Get diff for documentation context (escaped for JSON) | ||
| COMMIT_DIFF=$(git diff HEAD^ HEAD --stat | jq -Rs .) | ||
| echo "commit_diff=$COMMIT_DIFF" >> $GITHUB_OUTPUT | ||
|
|
||
| # Get commit SHA | ||
| echo "commit_sha=${{ github.sha }}" >> $GITHUB_OUTPUT | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Compare against the correct base for multi-commit pushes
HEAD^ only captures the last commit. Use github.event.before to include all files in the push.
- echo "Changed files in this push:"
- git diff --name-only HEAD^ HEAD | tee changed_files.txt
+ echo "Changed files in this push:"
+ git diff --name-only ${{ github.event.before }} ${{ github.sha }} | tee changed_files.txt
...
- CHANGED_FILES=$(git diff --name-only HEAD^ HEAD | jq -Rs .)
+ CHANGED_FILES=$(git diff --name-only ${{ github.event.before }} ${{ github.sha }} | jq -Rs .)
...
- COMMIT_DIFF=$(git diff HEAD^ HEAD --stat | jq -Rs .)
+ COMMIT_DIFF=$(git diff ${{ github.event.before }} ${{ github.sha }} --stat | jq -Rs .)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Get changed files | |
| id: changed-files | |
| run: | | |
| echo "Changed files in this push:" | |
| git diff --name-only HEAD^ HEAD | tee changed_files.txt | |
| # Store changed files for Claude to analyze (escaped for JSON) | |
| CHANGED_FILES=$(git diff --name-only HEAD^ HEAD | jq -Rs .) | |
| echo "changed_files=$CHANGED_FILES" >> $GITHUB_OUTPUT | |
| # Get the commit message (escaped for JSON) | |
| COMMIT_MSG=$(git log -1 --pretty=%B | jq -Rs .) | |
| echo "commit_message=$COMMIT_MSG" >> $GITHUB_OUTPUT | |
| # Get diff for documentation context (escaped for JSON) | |
| COMMIT_DIFF=$(git diff HEAD^ HEAD --stat | jq -Rs .) | |
| echo "commit_diff=$COMMIT_DIFF" >> $GITHUB_OUTPUT | |
| # Get commit SHA | |
| echo "commit_sha=${{ github.sha }}" >> $GITHUB_OUTPUT | |
| - name: Get changed files | |
| id: changed-files | |
| run: | | |
| echo "Changed files in this push:" | |
| git diff --name-only ${{ github.event.before }} ${{ github.sha }} | tee changed_files.txt | |
| # Store changed files for Claude to analyze (escaped for JSON) | |
| CHANGED_FILES=$(git diff --name-only ${{ github.event.before }} ${{ github.sha }} | jq -Rs .) | |
| echo "changed_files=$CHANGED_FILES" >> $GITHUB_OUTPUT | |
| # Get the commit message (escaped for JSON) | |
| COMMIT_MSG=$(git log -1 --pretty=%B | jq -Rs .) | |
| echo "commit_message=$COMMIT_MSG" >> $GITHUB_OUTPUT | |
| # Get diff for documentation context (escaped for JSON) | |
| COMMIT_DIFF=$(git diff ${{ github.event.before }} ${{ github.sha }} --stat | jq -Rs .) | |
| echo "commit_diff=$COMMIT_DIFF" >> $GITHUB_OUTPUT | |
| # Get commit SHA | |
| echo "commit_sha=${{ github.sha }}" >> $GITHUB_OUTPUT |
🤖 Prompt for AI Agents
.github/workflows/claude-docs-trigger.yml around lines 26 to 46: the script uses
HEAD^ which only captures the last commit; change the git ranges to compare
github.event.before with github.sha so all commits in the push are included.
Replace occurrences of git diff --name-only HEAD^ HEAD and git diff HEAD^ HEAD
--stat with git diff --name-only ${{ github.event.before }} ${{ github.sha }}
and git diff ${{ github.event.before }} ${{ github.sha }} --stat respectively,
and similarly compute CHANGED_FILES and COMMIT_DIFF from that range; also handle
the special case where github.event.before is all zeros (new branch) by diffing
from the empty tree or using git diff --name-only ${{ github.sha }} to capture
added files.
| - name: Trigger Claude workflow | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| # Trigger the Claude docs updater workflow with the change information | ||
| gh workflow run claude-docs-updater.yml \ | ||
| --ref next \ | ||
| -f commit_sha="${{ steps.changed-files.outputs.commit_sha }}" \ | ||
| -f commit_message=${{ steps.changed-files.outputs.commit_message }} \ | ||
| -f changed_files=${{ steps.changed-files.outputs.changed_files }} \ | ||
| -f commit_diff=${{ steps.changed-files.outputs.commit_diff }} No newline at end of file |
There was a problem hiding this comment.
Quote inputs passed to gh; preserve whitespace and specials
Unquoted JSON/text can break argument parsing. Wrap all -f values in quotes.
- gh workflow run claude-docs-updater.yml \
+ gh workflow run claude-docs-updater.yml \
--ref next \
- -f commit_sha="${{ steps.changed-files.outputs.commit_sha }}" \
- -f commit_message=${{ steps.changed-files.outputs.commit_message }} \
- -f changed_files=${{ steps.changed-files.outputs.changed_files }} \
- -f commit_diff=${{ steps.changed-files.outputs.commit_diff }}
+ -f commit_sha="${{ steps.changed-files.outputs.commit_sha }}" \
+ -f commit_message="${{ steps.changed-files.outputs.commit_message }}" \
+ -f changed_files="${{ steps.changed-files.outputs.changed_files }}" \
+ -f commit_diff="${{ steps.changed-files.outputs.commit_diff }}"🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 57-57: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
.github/workflows/claude-docs-trigger.yml around lines 47 to 57: the gh workflow
run -f arguments are unquoted which can break parsing for JSON/text with spaces
or special chars; update each -f value to be quoted (e.g. -f commit_message="${{
steps.changed-files.outputs.commit_message }}" and similarly quote commit_sha,
changed_files, commit_diff) so whitespace and special characters are preserved
when passed to the gh CLI.
| -f commit_sha="${{ steps.changed-files.outputs.commit_sha }}" \ | ||
| -f commit_message=${{ steps.changed-files.outputs.commit_message }} \ | ||
| -f changed_files=${{ steps.changed-files.outputs.changed_files }} \ | ||
| -f commit_diff=${{ steps.changed-files.outputs.commit_diff }} No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add newline at EOF
Satisfies linters and POSIX tools.
- -f commit_diff=${{ steps.changed-files.outputs.commit_diff }}
+ -f commit_diff=${{ steps.changed-files.outputs.commit_diff }}
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -f commit_diff=${{ steps.changed-files.outputs.commit_diff }} | |
| -f commit_diff=${{ steps.changed-files.outputs.commit_diff }} | |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 57-57: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
.github/workflows/claude-docs-trigger.yml around line 57: the file is missing a
trailing newline at EOF which fails linters and some POSIX tools; open the file
and add a single newline character after the last line (after the "-f
commit_diff=${{ steps.changed-files.outputs.commit_diff }}" line), save the file
so it ends with a newline, and commit the change.
| ref: next | ||
| fetch-depth: 0 # Need full history to checkout specific commit |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
YAML lint nits
Two-space pad before inline comments is standard; apply here.
- ref: next
- fetch-depth: 0 # Need full history to checkout specific commit
+ ref: next
+ fetch-depth: 0 # Need full history to checkout specific commit📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ref: next | |
| fetch-depth: 0 # Need full history to checkout specific commit | |
| ref: next | |
| fetch-depth: 0 # Need full history to checkout specific commit |
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 35-35: too few spaces before comment: expected 2
(comments)
🤖 Prompt for AI Agents
.github/workflows/claude-docs-updater.yml lines 34-35: the inline comment on
line 35 uses a single space before the '#' — update the line to use two spaces
before the inline comment (i.e., add one more space between "0" and "#") so the
file follows the two-space padding YAML lint convention.
| - Commit: ${{ inputs.commit_message }} | ||
| - Changed files: | ||
| ${{ steps.changed-files.outputs.changed_files }} | ||
| ${{ inputs.changed_files }} | ||
|
|
||
| - Changes summary: | ||
| ${{ steps.changed-files.outputs.commit_diff }} | ||
| ${{ inputs.commit_diff }} | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Decode JSON inputs before using in prompts
inputs.commit_message/changed_files/commit_diff are JSON-escaped by the trigger. Use fromJSON(...) so Claude sees clean text.
- - Commit: ${{ inputs.commit_message }}
+ - Commit: ${{ fromJSON(inputs.commit_message) }}
...
- ${{ inputs.changed_files }}
+ ${{ fromJSON(inputs.changed_files) }}
...
- ${{ inputs.commit_diff }}
+ ${{ fromJSON(inputs.commit_diff) }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Commit: ${{ inputs.commit_message }} | |
| - Changed files: | |
| ${{ steps.changed-files.outputs.changed_files }} | |
| ${{ inputs.changed_files }} | |
| - Changes summary: | |
| ${{ steps.changed-files.outputs.commit_diff }} | |
| ${{ inputs.commit_diff }} | |
| - Commit: ${{ fromJSON(inputs.commit_message) }} | |
| - Changed files: | |
| ${{ fromJSON(inputs.changed_files) }} | |
| - Changes summary: | |
| ${{ fromJSON(inputs.commit_diff) }} |
🤖 Prompt for AI Agents
.github/workflows/claude-docs-updater.yml around lines 63-69: the workflow is
using inputs.commit_message, inputs.changed_files and inputs.commit_diff
directly but they are JSON-escaped by the trigger; decode them before embedding
into prompts by wrapping each with fromJSON(...) (e.g. use ${{
fromJSON(inputs.commit_message) }}, ${{ fromJSON(inputs.changed_files) }}, ${{
fromJSON(inputs.commit_diff) }}) or assign decoded values to intermediate vars
and use those so Claude receives clean, unescaped text.
- add typescript - add npm workspaces
- add typescript - add npm workspaces
- add typescript - add npm workspaces
Crunchyman-ralph
force-pushed
the
ralph/implement.tm.list.remote
branch
from
ba190d2 to
d7afa8d
Compare
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
Improvements
Changes
Chores