fix: stabilize module registry integration

[buremba]

Summary

• add workspace path aliases to tsconfig and include the new module sources
• expose GitHub module helpers through the registry and update dispatcher/worker consumers to use typed accessors
• clean up orchestrator env handling and satisfy lint rules in integration mocks

Testing

• bun run lint
• bun run typecheck
• bun test (fails: database connection refused in integration tests)

---

https://chatgpt.com/codex/tasks/task_e_68e158f37e308325802a20953aad3686

[claude]

Claude finished @buremba's task —— View job

---

PR Review: fix: stabilize module registry integration

Review Tasks:

• Analyze module registry integration changes
• Review TypeScript configuration updates
• Examine GitHub module refactoring
• Check error handling and type safety
• Assess security implications
• Review test coverage and integration issues
• Provide comprehensive feedback

Review Feedback

🎯 Overall Assessment

This is a well-architected refactoring that successfully decouples GitHub-specific functionality from core packages into a modular system. The changes demonstrate good separation of concerns and establish a solid foundation for future module additions.

✅ Strengths

1. Clean Module Architecture - The module registry pattern with typed interfaces (HomeTabModule, WorkerModule, etc.) provides excellent extensibility
2. TypeScript Configuration - Added workspace path aliases improve import clarity and maintainability
3. Proper Error Handling - Good use of structured error classes like GitHubRepositoryError extending BaseError
4. Security Conscious - GitHub tokens are properly handled through environment variables and module configuration

⚠️ Issues & Recommendations

Critical Issues

1. Type Safety Gaps in Module Registry (modules/index.ts:63)

   getModule<T extends ModuleInterface>(name: string): T | undefined {
     return this.modules.get(name) as T;
   }

   Issue: Unsafe type assertion without runtime validation
   Fix: Add runtime type checking or use type guards

2. Missing Error Boundaries (packages/dispatcher/src/slack/handlers/action-handler.ts:40-53)

   for (const module of dispatcherModules) {
     if (module.handleAction) {
       const moduleHandled = await module.handleAction(actionId, userId, {

   Issue: Module failures could crash the entire action handler
   Fix: Wrap in try-catch with fallback handling

Security Concerns

3. GitHub Token Exposure Risk (modules/github/workspace.ts:80)

   await execAsync(
     `echo "${this.config.githubToken}" | gh auth login --with-token`,

   Issue: Token passed via command line arguments (visible in process list)
   Fix: Use environment variables or stdin for token passing

4. Command Injection Potential (modules/github/workspace.ts:285)

   await execAsync(`git commit -m "${message}"`, { cwd: userDirectory });

   Issue: Commit message not properly escaped
   Fix: Use parameterized execution or proper escaping

Performance & Reliability

5. Module Auto-Registration Side Effects (modules/index.ts:30-36)

   private autoRegisterModules(): void {
     const gitHubModule = new GitHubModule();
     if (!this.modules.has(gitHubModule.name)) {
       this.register(gitHubModule);
     }
   }

   Issue: Creates instances even if module won't be enabled
   Fix: Check isEnabled() before instantiation

6. Workspace Race Conditions (packages/worker/src/workspace-manager.ts:47-50)

   const userDirectory = join(
     this.config.baseDirectory,
     threadId.replace(/[^a-zA-Z0-9.-]/g, "_")
   );

   Issue: Multiple workers could conflict on same thread directory
   Fix: Add file locking or unique session identifiers

Code Quality

7. Inconsistent Error Handling (packages/worker/src/module-integration.ts:17-23)

   } catch (error) {
     console.error(
       `Failed to execute onSessionStart for module ${module.name}:`,
       error
     );
   }

   Issue: Swallows errors instead of propagating critical failures
   Fix: Differentiate between recoverable and critical errors

8. Missing Input Validation (modules/github/index.ts:274-277)

   private extractRepoName(url: string): string {
     const match = url.match(/github\.com[:/]([^/]+)\/([^/]+?)(?:\.git)?$/);
     return match ? `${match[1]}/${match[2]}` : url;
   }

   Issue: No validation for malformed URLs
   Fix: Add proper URL validation and error handling

🧪 Test Coverage Issues

9. Integration Test Failures - Database connection refused in integration tests
   Recommendation: Mock database connections or use test containers for reliable CI

10. Missing Module Integration Tests - No tests verify module loading/unloading behavior
    Recommendation: Add tests for module registry lifecycle

📈 Performance Considerations

11. Memory Usage - Module registry keeps all modules in memory
    Recommendation: Consider lazy loading for resource-intensive modules

12. Git Operations - Multiple git commands could be batched
    Recommendation: Optimize git operations in workspace.ts

🔧 Minor Issues

• Unused Imports: Some files have unused imports that should be cleaned up
• Inconsistent Logging: Mix of console.error and structured logging
• Magic Numbers: Timeout values should be configurable constants

🎯 Recommendations for Follow-up

1. Add comprehensive module integration tests
2. Implement module health checks and graceful degradation
3. Add configuration validation for module settings
4. Consider implementing module dependency management
5. Add metrics/monitoring for module performance

---

[cursor]

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[cursor]

Bug: Log Before Null Check Reveals Undefined User

The userId is logged before its null check. This means the log message can show "user: undefined" even when the function returns early due to a missing userId.

 

[cursor]

Bug: GitHub Module Missing Causes Runtime Error

In updateAppHome, gitHubModule.getUserInfo(userId) is called directly. If the GitHub module isn't registered, gitHubModule is undefined, which causes a runtime error.