feat: Implement shared IDE extension core module#30
Merged
Patrick-Ehimen merged 2 commits intomainfrom Oct 25, 2025
Merged
Conversation
…ive extension core architecture with modular design - Implemented ExtensionCore class orchestrating all extension functionality - Built CommandRegistry with command registration, validation, and execution - Created ProgressStreamer for real-time progress updates with event streaming - Implemented WorkspaceContextProvider with file watching and Git integration - Added AICommandHandler with built-in Lighthouse commands (upload/download/datasets) - Built ConfigurationManager with persistent settings and change notifications - Created comprehensive event system with ExtensionEventEmitter - Added extensive unit tests covering all core functionality (20 tests passing) - Integrated with existing SDK wrapper and shared utilities - Proper TypeScript types and interfaces for all components
Reviewer's GuideImplements a shared IDE extension core module by composing an orchestrator, command registry, workspace context provider, AI command handler, progress streaming, and configuration management into a cohesive TypeScript package with event-driven architecture. Sequence diagram for AI command handling and progress streamingsequenceDiagram
participant AICommandHandlerImpl
participant ProgressStreamerImpl
participant WorkspaceContextProviderImpl
participant LighthouseAISDK
participant ExtensionEventEmitter
actor IDEUser
IDEUser->>AICommandHandlerImpl: Send AI command (e.g. upload file)
AICommandHandlerImpl->>ProgressStreamerImpl: startProgress(operationId, title)
AICommandHandlerImpl->>LighthouseAISDK: uploadFile(filePath)
LighthouseAISDK-->>AICommandHandlerImpl: result
AICommandHandlerImpl->>ProgressStreamerImpl: complete(result)
AICommandHandlerImpl->>ExtensionEventEmitter: emit("ai.command.completed", ...)
AICommandHandlerImpl-->>IDEUser: Return AICommandResult
Class diagram for the new shared IDE extension core moduleclassDiagram
class ExtensionCoreImpl {
-_initialized: boolean
-_commandRegistry: CommandRegistry
-_workspaceContextProvider: WorkspaceContextProvider
-_aiCommandHandler: AICommandHandler
-_progressStreamer: ProgressStreamer
-_configurationManager: ConfigurationManager
-_eventEmitter: ExtensionEventEmitter
-_logger: Logger
+initialize()
+dispose()
+getCommandRegistry()
+getWorkspaceContextProvider()
+getAICommandHandler()
+getProgressStreamer()
+getConfigurationManager()
+isInitialized()
}
class CommandRegistryImpl {
-_commands: Map
-_logger: Logger
-_eventEmitter: ExtensionEventEmitter
+registerCommand(definition)
+unregisterCommand(commandId)
+executeCommand(commandId, ...args)
+getCommands()
+hasCommand(commandId)
}
class WorkspaceContextProviderImpl {
-_workspacePath: string
-_cachedContext: WorkspaceContext | null
-_cacheTimestamp: number
-_cacheTtl: number
-_fileWatcher: FileWatcherImpl
-_gitIntegration: GitIntegrationImpl
-_eventEmitter: ExtensionEventEmitter
-_configurationManager: ConfigurationManager
-_logger: Logger
+initialize()
+dispose()
+getContext()
+refreshContext()
+watchWorkspace(callback)
+getWorkspaceFiles()
+getLighthouseFiles()
+getActiveDatasets()
}
class AICommandHandlerImpl {
-_handlers: Map
-_workspaceContextProvider: WorkspaceContextProvider
-_progressStreamer: ProgressStreamer
-_eventEmitter: ExtensionEventEmitter
-_lighthouseSDK: LighthouseAISDK
-_logger: Logger
+initialize()
+dispose()
+handleCommand(command)
+registerHandler(commandType, handler)
+unregisterHandler(commandType)
+getAvailableCommands()
}
class ProgressStreamerImpl {
-_streams: Map
-_eventEmitter: ExtensionEventEmitter
-_logger: Logger
+startProgress(operationId, title)
+getProgress(operationId)
+stopProgress(operationId)
+getActiveStreams()
+clearAllStreams()
}
class ConfigurationManagerImpl {
-_configuration: ExtensionConfiguration
-_settingsProvider: SettingsProviderImpl
-_watchers: Set
-_logger: Logger
+initialize()
+dispose()
+getConfiguration()
+updateConfiguration(config)
+watchConfiguration(callback)
+resetConfiguration()
}
class ExtensionEventEmitter {
+emit(eventType, event)
+on(eventType, listener)
+once(eventType, listener)
+getListenerCount(eventType)
+getEventNames()
}
class FileWatcherImpl {
-_workspacePath: string
-_watchers: Map
-_callbacks: Set
-_logger: Logger
-_debounceMap: Map
-_debounceDelay: number
+initialize()
+dispose()
+onFileChanged(callback)
+offFileChanged(callback)
}
class GitIntegrationImpl {
-_workspacePath: string
-_logger: Logger
-_isGitRepo: boolean
+initialize()
+dispose()
+getGitInfo()
}
class SettingsProviderImpl {
-_settingsPath: string
-_logger: Logger
+loadConfiguration()
+saveConfiguration(config)
}
class ProgressStreamImpl {
-_operationId: string
-_title: string
-_currentProgress: ProgressUpdate
-_active: boolean
-_eventEmitter: ExtensionEventEmitter
-_logger: Logger
+update(progress)
+complete(result)
+fail(error)
+cancel()
+getCurrentProgress()
+isActive()
}
class AIContextManagerImpl {
-_currentContext: AIContext | null
-_logger: Logger
+getCurrentContext()
+setContext(context)
+updateSession(sessionUpdate)
+updatePreferences(preferences)
+addHistoryEntry(entry)
+clearContext()
}
class AISessionManagerImpl {
-_activeSessions: Map
-_logger: Logger
+createSession(agentId, context)
+getSession(sessionId)
+updateSessionActivity(sessionId)
+endSession(sessionId)
+getActiveSessions()
+cleanupOldSessions(maxAge)
}
ExtensionCoreImpl --> CommandRegistryImpl
ExtensionCoreImpl --> WorkspaceContextProviderImpl
ExtensionCoreImpl --> AICommandHandlerImpl
ExtensionCoreImpl --> ProgressStreamerImpl
ExtensionCoreImpl --> ConfigurationManagerImpl
ExtensionCoreImpl --> ExtensionEventEmitter
WorkspaceContextProviderImpl --> FileWatcherImpl
WorkspaceContextProviderImpl --> GitIntegrationImpl
WorkspaceContextProviderImpl --> ExtensionEventEmitter
WorkspaceContextProviderImpl --> ConfigurationManagerImpl
AICommandHandlerImpl --> WorkspaceContextProviderImpl
AICommandHandlerImpl --> ProgressStreamerImpl
AICommandHandlerImpl --> ExtensionEventEmitter
ProgressStreamerImpl --> ProgressStreamImpl
ProgressStreamerImpl --> ExtensionEventEmitter
ConfigurationManagerImpl --> SettingsProviderImpl
CommandRegistryImpl --> ExtensionEventEmitter
FileWatcherImpl --> Logger
GitIntegrationImpl --> Logger
SettingsProviderImpl --> Logger
ProgressStreamImpl --> ExtensionEventEmitter
ProgressStreamImpl --> Logger
AIContextManagerImpl --> Logger
AISessionManagerImpl --> Logger
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- ProgressStreamerImpl starts a setInterval for cleanup but never clears it on dispose—consider adding a dispose method to clear that timer to avoid orphaned background tasks.
- GitIntegrationImpl uses execSync for all Git commands, which will block the event loop—switch to an async child_process API or a promise‐based Git library to keep operations non-blocking.
- FileWatcherImpl relies on fs.watch with recursive option and custom ignore logic—consider adopting a battle-tested watcher like chokidar to improve cross-platform reliability and more flexible ignore patterns.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- ProgressStreamerImpl starts a setInterval for cleanup but never clears it on dispose—consider adding a dispose method to clear that timer to avoid orphaned background tasks.
- GitIntegrationImpl uses execSync for all Git commands, which will block the event loop—switch to an async child_process API or a promise‐based Git library to keep operations non-blocking.
- FileWatcherImpl relies on fs.watch with recursive option and custom ignore logic—consider adopting a battle-tested watcher like chokidar to improve cross-platform reliability and more flexible ignore patterns.
## Individual Comments
### Comment 1
<location> `packages/extension-core/src/workspace/context-provider.ts:672-678` </location>
<code_context>
+ const frontendFrameworks = frameworks.filter((f) => f.type === "frontend");
+ const backendFrameworks = frameworks.filter((f) => f.type === "backend");
+
+ if (frontendFrameworks.length > 0 && backendFrameworks.length > 0) {
+ return "web_app";
+ } else if (frontendFrameworks.length > 0) {
+ return "web_app";
</code_context>
<issue_to_address>
**suggestion:** Workspace type detection may be too broad.
Current logic does not differentiate between full-stack and frontend-only apps. Please update type detection to provide more specific classifications.
```suggestion
if (frontendFrameworks.length > 0 && backendFrameworks.length > 0) {
return "web_fullstack_app";
} else if (frontendFrameworks.length > 0) {
return "web_frontend_app";
} else if (backendFrameworks.length > 0) {
return "web_backend_app";
}
```
</issue_to_address>
### Comment 2
<location> `packages/extension-core/src/core/progress-streamer.ts:191` </location>
<code_context>
+ this._logger = new Logger({ level: "info", component: "ProgressStreamer" });
+
+ // Clean up completed streams periodically
+ setInterval(() => this._cleanupCompletedStreams(), 60000); // Every minute
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Unmanaged setInterval may cause resource leaks.
Store the interval ID and clear it in a dispose method to prevent leaks when ProgressStreamerImpl is disposed and recreated.
</issue_to_address>
### Comment 3
<location> `packages/extension-core/src/core/command-registry.ts:220-222` </location>
<code_context>
+ ): void {
+ const requiredParams = parameters.filter((p) => p.required);
+
+ if (args.length < requiredParams.length) {
+ throw new Error(
+ `Command ${commandId} requires ${requiredParams.length} arguments, got ${args.length}`,
+ );
+ }
</code_context>
<issue_to_address>
**issue:** Argument validation may not handle optional parameters correctly.
Validating only by required parameter count may fail if optional parameters come before required ones. Please update the logic to account for parameter order and required status.
</issue_to_address>
### Comment 4
<location> `packages/extension-core/src/events/event-emitter.ts:27` </location>
<code_context>
+ /**
+ * Emit an extension event
+ */
+ emit(eventType: string, event: ExtensionEvent | any): boolean {
+ this._logger.debug(`Emitting event: ${eventType}`, event);
+ return super.emit(eventType, event);
</code_context>
<issue_to_address>
**suggestion:** Event type is not strictly validated.
Validate eventType against the predefined set of allowed event types to prevent typos and incorrect usage.
Suggested implementation:
```typescript
/**
* Allowed event types for extension events
*/
private static readonly ALLOWED_EVENT_TYPES = [
"extensionLoaded",
"extensionUnloaded",
"extensionError",
// Add other allowed event types here
];
/**
* Emit an extension event
*/
emit(eventType: string, event: ExtensionEvent | any): boolean {
if (!ExtensionEventEmitter.ALLOWED_EVENT_TYPES.includes(eventType)) {
this._logger.error(`Invalid event type: ${eventType}. Allowed types: ${ExtensionEventEmitter.ALLOWED_EVENT_TYPES.join(", ")}`);
return false;
}
this._logger.debug(`Emitting event: ${eventType}`, event);
return super.emit(eventType, event);
}
```
- You should update the `ALLOWED_EVENT_TYPES` array to include all valid event types used in your codebase.
- If event types are defined elsewhere (e.g., as an enum or constant), consider importing and using that definition for consistency.
</issue_to_address>
### Comment 5
<location> `packages/extension-core/__tests__/command-registry.test.ts:5` </location>
<code_context>
+ * Command registry tests
+ * @fileoverview Tests for the command registry functionality
+ */
+
+import { CommandRegistryImpl } from "../src/core/command-registry";
+import { ExtensionEventEmitter } from "../src/events/event-emitter";
</code_context>
<issue_to_address>
**suggestion (testing):** Missing test coverage for command argument validation and error handling.
Add tests for argument validation failures (missing, incorrect types, custom validator errors) and error handling during command execution (disabled or unregistered commands).
</issue_to_address>
### Comment 6
<location> `packages/extension-core/__tests__/extension-core.test.ts:5` </location>
<code_context>
+ * Command registry tests
+ * @fileoverview Tests for the command registry functionality
+ */
+
+import { CommandRegistryImpl } from "../src/core/command-registry";
+import { ExtensionEventEmitter } from "../src/events/event-emitter";
</code_context>
<issue_to_address>
**suggestion (testing):** Missing tests for error scenarios during initialization and disposal.
Add tests that cover error handling during ExtensionCore initialization and disposal, ensuring proper logging and propagation.
</issue_to_address>
### Comment 7
<location> `packages/extension-core/__tests__/progress-streamer.test.ts:5` </location>
<code_context>
+ * Command registry tests
+ * @fileoverview Tests for the command registry functionality
+ */
+
+import { CommandRegistryImpl } from "../src/core/command-registry";
+import { ExtensionEventEmitter } from "../src/events/event-emitter";
</code_context>
<issue_to_address>
**suggestion (testing):** Missing tests for cleanup and edge cases in progress streaming.
Add tests to cover cleanup of completed, failed, and cancelled progress streams, ensuring they are removed as expected. Also include cases for starting streams with duplicate operationIds and stopping streams that do not exist.
</issue_to_address>
### Comment 8
<location> `packages/extension-core/src/core/command-registry.ts:236-240` </location>
<code_context>
if (arg !== undefined && arg !== null && param.validator) {
if (!param.validator(arg)) {
throw new Error(`Command ${commandId} parameter ${param.name} validation failed`);
}
}
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge nested if conditions ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/merge-nested-ifs))
```suggestion
if (arg !== undefined && arg !== null && param.validator && !param.validator(arg)) {
throw new Error(`Command ${commandId} parameter ${param.name} validation failed`);
}
```
<br/><details><summary>Explanation</summary>Reading deeply nested conditional code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two `if` conditions can be combined using
`and` is an easy win.
</details>
</issue_to_address>
### Comment 9
<location> `packages/extension-core/src/core/extension-core.ts:204-210` </location>
<code_context>
const status = {
initialized: this._initialized,
activeStreams: this._progressStreamer.getActiveStreams().length,
registeredCommands: this._commandRegistry.getCommands().length,
workspaceContext: await this._workspaceContextProvider.getContext(),
};
return status;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/inline-immediately-returned-variable))
```suggestion
return {
initialized: this._initialized,
activeStreams: this._progressStreamer.getActiveStreams().length,
registeredCommands: this._commandRegistry.getCommands().length,
workspaceContext: await this._workspaceContextProvider.getContext(),
};
```
<br/><details><summary>Explanation</summary>Something that we often see in people's code is assigning to a result variable
and then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
</details>
</issue_to_address>
### Comment 10
<location> `packages/extension-core/src/workspace/context-provider.ts:199-200` </location>
<code_context>
const files = await this._scanDirectory(this._workspacePath);
return files;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/inline-immediately-returned-variable))
```suggestion
return await this._scanDirectory(this._workspacePath);
```
<br/><details><summary>Explanation</summary>Something that we often see in people's code is assigning to a result variable
and then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+672
to
+678
| if (frontendFrameworks.length > 0 && backendFrameworks.length > 0) { | ||
| return "web_app"; | ||
| } else if (frontendFrameworks.length > 0) { | ||
| return "web_app"; | ||
| } else if (backendFrameworks.length > 0) { | ||
| return "web_app"; | ||
| } |
There was a problem hiding this comment.
suggestion: Workspace type detection may be too broad.
Current logic does not differentiate between full-stack and frontend-only apps. Please update type detection to provide more specific classifications.
Suggested change
| if (frontendFrameworks.length > 0 && backendFrameworks.length > 0) { | |
| return "web_app"; | |
| } else if (frontendFrameworks.length > 0) { | |
| return "web_app"; | |
| } else if (backendFrameworks.length > 0) { | |
| return "web_app"; | |
| } | |
| if (frontendFrameworks.length > 0 && backendFrameworks.length > 0) { | |
| return "web_fullstack_app"; | |
| } else if (frontendFrameworks.length > 0) { | |
| return "web_frontend_app"; | |
| } else if (backendFrameworks.length > 0) { | |
| return "web_backend_app"; | |
| } |
| this._logger = new Logger({ level: "info", component: "ProgressStreamer" }); | ||
|
|
||
| // Clean up completed streams periodically | ||
| setInterval(() => this._cleanupCompletedStreams(), 60000); // Every minute |
There was a problem hiding this comment.
issue (bug_risk): Unmanaged setInterval may cause resource leaks.
Store the interval ID and clear it in a dispose method to prevent leaks when ProgressStreamerImpl is disposed and recreated.
Comment on lines
+220
to
+222
| if (args.length < requiredParams.length) { | ||
| throw new Error( | ||
| `Command ${commandId} requires ${requiredParams.length} arguments, got ${args.length}`, |
There was a problem hiding this comment.
issue: Argument validation may not handle optional parameters correctly.
Validating only by required parameter count may fail if optional parameters come before required ones. Please update the logic to account for parameter order and required status.
| /** | ||
| * Emit an extension event | ||
| */ | ||
| emit(eventType: string, event: ExtensionEvent | any): boolean { |
There was a problem hiding this comment.
suggestion: Event type is not strictly validated.
Validate eventType against the predefined set of allowed event types to prevent typos and incorrect usage.
Suggested implementation:
/**
* Allowed event types for extension events
*/
private static readonly ALLOWED_EVENT_TYPES = [
"extensionLoaded",
"extensionUnloaded",
"extensionError",
// Add other allowed event types here
];
/**
* Emit an extension event
*/
emit(eventType: string, event: ExtensionEvent | any): boolean {
if (!ExtensionEventEmitter.ALLOWED_EVENT_TYPES.includes(eventType)) {
this._logger.error(`Invalid event type: ${eventType}. Allowed types: ${ExtensionEventEmitter.ALLOWED_EVENT_TYPES.join(", ")}`);
return false;
}
this._logger.debug(`Emitting event: ${eventType}`, event);
return super.emit(eventType, event);
}- You should update the
ALLOWED_EVENT_TYPESarray to include all valid event types used in your codebase. - If event types are defined elsewhere (e.g., as an enum or constant), consider importing and using that definition for consistency.
| * Extension core tests | ||
| * @fileoverview Tests for the main extension core functionality | ||
| */ | ||
|
|
There was a problem hiding this comment.
suggestion (testing): Missing tests for error scenarios during initialization and disposal.
Add tests that cover error handling during ExtensionCore initialization and disposal, ensuring proper logging and propagation.
Comment on lines
+204
to
+210
| const status = { | ||
| initialized: this._initialized, | ||
| activeStreams: this._progressStreamer.getActiveStreams().length, | ||
| registeredCommands: this._commandRegistry.getCommands().length, | ||
| workspaceContext: await this._workspaceContextProvider.getContext(), | ||
| }; | ||
| return status; |
There was a problem hiding this comment.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
Suggested change
| const status = { | |
| initialized: this._initialized, | |
| activeStreams: this._progressStreamer.getActiveStreams().length, | |
| registeredCommands: this._commandRegistry.getCommands().length, | |
| workspaceContext: await this._workspaceContextProvider.getContext(), | |
| }; | |
| return status; | |
| return { | |
| initialized: this._initialized, | |
| activeStreams: this._progressStreamer.getActiveStreams().length, | |
| registeredCommands: this._commandRegistry.getCommands().length, | |
| workspaceContext: await this._workspaceContextProvider.getContext(), | |
| }; | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
Comment on lines
+199
to
+200
| const files = await this._scanDirectory(this._workspacePath); | ||
| return files; |
There was a problem hiding this comment.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
Suggested change
| const files = await this._scanDirectory(this._workspacePath); | |
| return files; | |
| return await this._scanDirectory(this._workspacePath); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Implements the shared IDE extension core module that provides common business logic, workspace context management, AI command handling, and progress streaming for both VSCode and Cursor extensions.
Key Features
Structure
Usage
Built-in AI Commands
lighthouse.upload.file- Upload files with encryptionlighthouse.download.file- Download by CIDlighthouse.create.dataset- Dataset managementlighthouse.workspace.context- Get workspace infolighthouse.list.datasets- List active datasetsSummary by Sourcery
Implement a shared IDE extension core module with orchestrator, workspace context, AI commands, progress tracking, configuration management, and an event bus
New Features:
Build:
Tests: