feat: add skeleton for container extension#691
Conversation
benoitf
requested review from
axel7083 and
jeffmaury
and removed request for
a team
📝 WalkthroughWalkthroughAdds a new container extension with an API package ( Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant extension.ts
participant ContainerExtension
participant InversifyBinding
participant InversifyContainer
participant ContainerEngineManager
Caller->>extension.ts: activate(extensionContext)
extension.ts->>ContainerExtension: new ContainerExtension(extensionContext)
extension.ts->>ContainerExtension: activate()
ContainerExtension->>InversifyBinding: initBindings()
InversifyBinding->>InversifyContainer: create Container & bind symbols
InversifyBinding->>InversifyContainer: load(managersModule)
InversifyBinding->>InversifyContainer: getAsync(ContainerEngineManager)
InversifyContainer->>ContainerEngineManager: init()
ContainerEngineManager-->>InversifyBinding: initialized
InversifyBinding-->>ContainerExtension: container
ContainerExtension->>ContainerEngineManager: exports()
ContainerEngineManager-->>ContainerExtension: ContainerExtensionAPI
ContainerExtension-->>extension.ts: ContainerExtensionAPI
extension.ts-->>Caller: ContainerExtensionAPI
Caller->>extension.ts: deactivate()
extension.ts->>ContainerExtension: deactivate()
ContainerExtension->>InversifyBinding: dispose()
InversifyBinding->>InversifyContainer: unbindAll() / clear
ContainerEngineManager->>ContainerEngineManager: dispose()
ContainerExtension-->>extension.ts: ✓
extension.ts-->>Caller: ✓
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.18.1)extensions/container/packages/extension/README.mdmarkdownlint-cli2 v0.18.1 (markdownlint v0.38.0) ... [truncated 1164 characters] ... 4:11) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
extensions/container/packages/extension/scripts/build.js (2)
30-36: Add error handling for cleanup operations.The synchronous
rmSynccalls could fail due to permission issues or locked files. Consider wrapping them in try-catch blocks for more graceful error handling.Apply this diff to add error handling:
// remove the .cdix file before zipping if (fs.existsSync(destFile)) { - fs.rmSync(destFile); + try { + fs.rmSync(destFile); + } catch (error) { + console.error('Failed to remove existing .cdix file:', error); + process.exit(1); + } } // remove the builtin folder before zipping if (fs.existsSync(builtinDirectory)) { - fs.rmSync(builtinDirectory, { recursive: true, force: true }); + try { + fs.rmSync(builtinDirectory, { recursive: true, force: true }); + } catch (error) { + console.error('Failed to remove existing builtin directory:', error); + process.exit(1); + } }
42-51: Consider consistent async/sync patterns and add success logging.The unzip logic uses async operations while earlier steps (cleanup, zip creation) use synchronous operations. For consistency and better user experience, consider making all operations async or adding logging to indicate success.
Consider adding success logging after extraction:
mkdirp(unzippedDirectory) .then(() => { const unzip = new AdmZip(destFile); unzip.extractAllTo(unzippedDirectory); + console.log(`Successfully packaged extension to ${destFile}`); + console.log(`Extracted builtin copy to ${unzippedDirectory}`); }) .catch(error => { console.error('Failed to create builtin directory or extract archive:', error); process.exit(1); });extensions/container/packages/api/src/container-extension-api.d.ts (1)
19-20: Consider using a type alias for the empty API placeholder.For an empty type, a type alias is more idiomatic than an empty interface. Since this is a skeleton, the current approach is acceptable.
Apply this diff if you'd like to align with TypeScript best practices:
-// eslint-disable-next-line @typescript-eslint/no-empty-object-type -export interface ContainerExtensionAPI {} +export type ContainerExtensionAPI = Record<string, never>;extensions/container/packages/extension/tsconfig.json (1)
4-10: Update moduleResolution to align with ESNext modules.The
moduleResolution: "Node"setting uses the legacy resolution algorithm. Formodule: "esnext", consider using"Bundler","Node16", or"NodeNext"to ensure proper module resolution.Apply this diff to use the Bundler strategy (appropriate for Vite):
"compilerOptions": { "strict": true, "module": "esnext", "lib": ["ES2024"], "sourceMap": true, "rootDir": "src", "outDir": "dist", "target": "esnext", - "moduleResolution": "Node", + "moduleResolution": "Bundler", "allowSyntheticDefaultImports": true,extensions/container/packages/extension/src/container-extension.ts (2)
55-60: Consider using telemetryLogger for error logging.The error is logged using
console.error, but atelemetryLoggerwas already created at line 37. For consistency with the extension's observability approach, consider using the telemetry logger instead. Additionally, the error message "Error while creating the container provider manager" could be more precise—it's actually an error while resolving the manager from the DI container.Apply this diff to use the telemetry logger:
async activate(): Promise<ContainerExtensionAPI> { const telemetryLogger = env.createTelemetryLogger(); const containerProvider = provider.createProvider({ name: 'Container', status: 'unknown', id: 'container', images: { icon: './icon.png', logo: { dark: './icon.png', light: './icon.png', }, }, }); this.#inversifyBinding = new InversifyBinding(containerProvider, this.#extensionContext, telemetryLogger); this.#container = await this.#inversifyBinding.initBindings(); try { this.#containerEngineManager = await this.getContainer()?.getAsync(ContainerEngineManager); } catch (e) { - console.error('Error while creating the container provider manager', e); + telemetryLogger.logError('Error while resolving ContainerEngineManager from DI container', e); throw e; }
74-78: Consider clearing all references after disposal.The deactivate method properly disposes of resources and clears the
#containerEngineManagerreference. For consistency and to ensure complete cleanup, consider also clearing the#inversifyBindingand#containerreferences after disposal.Apply this diff:
async deactivate(): Promise<void> { this.#containerEngineManager?.dispose(); this.#containerEngineManager = undefined; await this.#inversifyBinding?.dispose(); + this.#inversifyBinding = undefined; + this.#container = undefined; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
extensions/container/packages/extension/icon.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
extensions/container/packages/api/package.json(1 hunks)extensions/container/packages/api/src/container-extension-api.d.ts(1 hunks)extensions/container/packages/api/tsconfig.json(1 hunks)extensions/container/packages/api/vite.config.js(1 hunks)extensions/container/packages/extension/README.md(1 hunks)extensions/container/packages/extension/package.json(1 hunks)extensions/container/packages/extension/scripts/build.js(1 hunks)extensions/container/packages/extension/src/container-extension.spec.ts(1 hunks)extensions/container/packages/extension/src/container-extension.ts(1 hunks)extensions/container/packages/extension/src/extension.spec.ts(1 hunks)extensions/container/packages/extension/src/extension.ts(1 hunks)extensions/container/packages/extension/src/inject/inversify-binding.spec.ts(1 hunks)extensions/container/packages/extension/src/inject/inversify-binding.ts(1 hunks)extensions/container/packages/extension/src/inject/symbol.ts(1 hunks)extensions/container/packages/extension/src/manager/_manager-module.ts(1 hunks)extensions/container/packages/extension/src/manager/container-engine-manager.ts(1 hunks)extensions/container/packages/extension/tsconfig.json(1 hunks)extensions/container/packages/extension/types/template.d.ts(1 hunks)extensions/container/packages/extension/vite.config.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{packages,extensions}/**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Co-locate unit tests with source files and name them *.spec.ts
Files:
extensions/container/packages/extension/src/container-extension.spec.tsextensions/container/packages/extension/src/extension.spec.tsextensions/container/packages/extension/src/inject/inversify-binding.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate unit tests alongside source files using the *.spec.ts naming convention
Files:
extensions/container/packages/extension/src/container-extension.spec.tsextensions/container/packages/extension/src/extension.spec.tsextensions/container/packages/extension/src/inject/inversify-binding.spec.ts
🧠 Learnings (20)
📓 Common learnings
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 678
File: extensions/container/packages/api/vite.config.js:34-40
Timestamp: 2025-11-05T16:31:22.355Z
Learning: In the kortex repository, the `kortex-app/container-extension-api` package at `extensions/container/packages/api/` is a declaration-only package that ships pure TypeScript declaration files (.d.ts) without any compiled JavaScript. It intentionally has an empty build script and does not require library build configuration in vite.config.js.
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/main/**/*.ts : Container operations must include an engineId parameter when invoking ContainerProviderRegistry actions
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to packages/main/src/plugin/index.ts : Register all major services as singletons in the Inversify DI container during initialization in the main process plugin entrypoint
📚 Learning: 2025-11-05T16:31:22.355Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 678
File: extensions/container/packages/api/vite.config.js:34-40
Timestamp: 2025-11-05T16:31:22.355Z
Learning: In the kortex repository, the `kortex-app/container-extension-api` package at `extensions/container/packages/api/` is a declaration-only package that ships pure TypeScript declaration files (.d.ts) without any compiled JavaScript. It intentionally has an empty build script and does not require library build configuration in vite.config.js.
Applied to files:
extensions/container/packages/api/src/container-extension-api.d.tsextensions/container/packages/extension/README.mdextensions/container/packages/extension/types/template.d.tsextensions/container/packages/extension/tsconfig.jsonextensions/container/packages/extension/src/inject/symbol.tsextensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/container-extension.spec.tsextensions/container/packages/extension/src/manager/_manager-module.tsextensions/container/packages/api/package.jsonextensions/container/packages/api/vite.config.jsextensions/container/packages/extension/src/extension.tsextensions/container/packages/api/tsconfig.jsonextensions/container/packages/extension/scripts/build.jsextensions/container/packages/extension/src/manager/container-engine-manager.tsextensions/container/packages/extension/src/extension.spec.tsextensions/container/packages/extension/vite.config.jsextensions/container/packages/extension/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/src/**/*.ts : Extensions should use kortex-app/api for provider registration, configuration, and contributions
Applied to files:
extensions/container/packages/api/src/container-extension-api.d.tsextensions/container/packages/extension/README.mdextensions/container/packages/extension/tsconfig.jsonextensions/container/packages/extension/src/inject/symbol.tsextensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/inject/inversify-binding.tsextensions/container/packages/extension/src/container-extension.spec.tsextensions/container/packages/extension/src/manager/_manager-module.tsextensions/container/packages/api/package.jsonextensions/container/packages/extension/src/extension.tsextensions/container/packages/extension/src/manager/container-engine-manager.tsextensions/container/packages/extension/vite.config.jsextensions/container/packages/extension/package.json
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Extension configuration properties must be declared under contributes.configuration.properties in package.json
Applied to files:
extensions/container/packages/extension/tsconfig.jsonextensions/container/packages/api/package.jsonextensions/container/packages/api/vite.config.jsextensions/container/packages/extension/vite.config.jsextensions/container/packages/extension/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/package.json : Define configuration under contributes.configuration.properties in extension package.json
Applied to files:
extensions/container/packages/extension/tsconfig.jsonextensions/container/packages/api/package.jsonextensions/container/packages/api/vite.config.jsextensions/container/packages/extension/vite.config.jsextensions/container/packages/extension/package.json
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to {packages,extensions}/**/*.spec.ts : Co-locate unit tests with source files and name them *.spec.ts
Applied to files:
extensions/container/packages/extension/tsconfig.jsonextensions/container/packages/extension/src/container-extension.spec.tsextensions/container/packages/api/tsconfig.jsonextensions/container/packages/extension/src/extension.spec.tsextensions/container/packages/extension/src/inject/inversify-binding.spec.ts
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Each extension’s package.json must set "main" to "./dist/extension.js"
Applied to files:
extensions/container/packages/extension/tsconfig.jsonextensions/container/packages/api/package.jsonextensions/container/packages/extension/scripts/build.jsextensions/container/packages/extension/vite.config.jsextensions/container/packages/extension/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/package.json : Set package.json main to ./dist/extension.js for extensions
Applied to files:
extensions/container/packages/extension/tsconfig.jsonextensions/container/packages/api/package.jsonextensions/container/packages/extension/scripts/build.jsextensions/container/packages/extension/vite.config.jsextensions/container/packages/extension/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to **/*.spec.ts : Co-locate unit tests alongside source files using the *.spec.ts naming convention
Applied to files:
extensions/container/packages/extension/tsconfig.jsonextensions/container/packages/api/tsconfig.jsonextensions/container/packages/extension/src/extension.spec.tsextensions/container/packages/extension/src/inject/inversify-binding.spec.ts
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/{preload,preload-webview}/**/*.{ts,js} : Place Preload scripts under packages/preload and packages/preload-webview
Applied to files:
extensions/container/packages/extension/tsconfig.json
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/src/extension.ts : Extensions must export a standard activation API with an activate() function in src/extension.ts
Applied to files:
extensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/container-extension.spec.tsextensions/container/packages/extension/src/extension.tsextensions/container/packages/extension/src/extension.spec.ts
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/src/extension.ts : Extension entry src/extension.ts must export an activate() function
Applied to files:
extensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/container-extension.spec.tsextensions/container/packages/extension/src/extension.tsextensions/container/packages/extension/src/extension.spec.ts
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/main/**/*.ts : Container operations must include an engineId parameter when invoking ContainerProviderRegistry actions
Applied to files:
extensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/container-extension.spec.tsextensions/container/packages/extension/src/manager/container-engine-manager.tsextensions/container/packages/extension/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/main/src/plugin/index.ts : Register all major services as singletons in the Inversify DI container during initialization
Applied to files:
extensions/container/packages/extension/src/inject/inversify-binding.tsextensions/container/packages/extension/src/manager/_manager-module.tsextensions/container/packages/extension/src/inject/inversify-binding.spec.ts
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to packages/main/src/plugin/index.ts : Register all major services as singletons in the Inversify DI container during initialization in the main process plugin entrypoint
Applied to files:
extensions/container/packages/extension/src/inject/inversify-binding.tsextensions/container/packages/extension/src/manager/_manager-module.tsextensions/container/packages/extension/src/inject/inversify-binding.spec.ts
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Each extension must declare Kortex compatibility via engines.kortex in package.json
Applied to files:
extensions/container/packages/api/package.jsonextensions/container/packages/extension/src/manager/container-engine-manager.tsextensions/container/packages/extension/vite.config.jsextensions/container/packages/extension/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/package.json : Each extension must declare engines.kortex version compatibility in package.json
Applied to files:
extensions/container/packages/api/package.jsonextensions/container/packages/extension/src/manager/container-engine-manager.tsextensions/container/packages/extension/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to vitest.config.js : Keep root vitest.config.js as the authoritative workspace test configuration
Applied to files:
extensions/container/packages/api/vite.config.jsextensions/container/packages/extension/vite.config.js
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to vitest.config.js : Define and maintain workspace test projects in the root vitest.config.js
Applied to files:
extensions/container/packages/api/vite.config.jsextensions/container/packages/extension/vite.config.js
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/dist/extension.js : Built extensions must output dist/extension.js (Vite build result)
Applied to files:
extensions/container/packages/api/vite.config.jsextensions/container/packages/extension/scripts/build.jsextensions/container/packages/extension/vite.config.js
🧬 Code graph analysis (10)
extensions/container/packages/extension/src/container-extension.ts (5)
packages/extension-api/src/extension-api.d.ts (1)
ExtensionContext(183-203)extensions/container/packages/extension/src/inject/inversify-binding.ts (1)
InversifyBinding(26-58)extensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
ContainerEngineManager(22-30)extensions/container/packages/api/src/container-extension-api.d.ts (1)
ContainerExtensionAPI(20-20)__mocks__/@kortex-app/api.js (2)
env(31-33)provider(27-29)
extensions/container/packages/extension/src/inject/inversify-binding.ts (3)
packages/extension-api/src/extension-api.d.ts (2)
ExtensionContext(183-203)TelemetryLogger(4676-4726)extensions/container/packages/extension/src/inject/symbol.ts (3)
ExtensionContextSymbol(22-22)TelemetryLoggerSymbol(23-23)ContainersProvider(20-20)extensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
ContainerEngineManager(22-30)
extensions/container/packages/extension/src/container-extension.spec.ts (3)
extensions/container/packages/extension/src/container-extension.ts (1)
ContainerExtension(25-79)packages/extension-api/src/extension-api.d.ts (1)
ExtensionContext(183-203)extensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
ContainerEngineManager(22-30)
extensions/container/packages/extension/src/manager/_manager-module.ts (1)
extensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
ContainerEngineManager(22-30)
extensions/container/packages/api/vite.config.js (2)
extensions/container/packages/extension/vite.config.js (1)
PACKAGE_ROOT(22-22)eslint.config.mjs (1)
__dirname(40-40)
extensions/container/packages/extension/src/extension.ts (3)
extensions/container/packages/extension/src/container-extension.ts (3)
ContainerExtension(25-79)activate(36-68)deactivate(74-78)packages/extension-api/src/extension-api.d.ts (1)
ExtensionContext(183-203)extensions/container/packages/api/src/container-extension-api.d.ts (1)
ContainerExtensionAPI(20-20)
extensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
extensions/container/packages/api/src/container-extension-api.d.ts (1)
ContainerExtensionAPI(20-20)
extensions/container/packages/extension/src/extension.spec.ts (3)
packages/extension-api/src/extension-api.d.ts (1)
ExtensionContext(183-203)extensions/container/packages/extension/src/container-extension.ts (3)
activate(36-68)ContainerExtension(25-79)deactivate(74-78)extensions/container/packages/extension/src/extension.ts (2)
activate(25-28)deactivate(30-33)
extensions/container/packages/extension/src/inject/inversify-binding.spec.ts (4)
extensions/container/packages/extension/src/inject/inversify-binding.ts (1)
InversifyBinding(26-58)extensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
ContainerEngineManager(22-30)extensions/container/packages/extension/src/inject/symbol.ts (3)
ExtensionContextSymbol(22-22)TelemetryLoggerSymbol(23-23)ContainersProvider(20-20)extensions/container/packages/extension/src/manager/_manager-module.ts (1)
managersModule(27-27)
extensions/container/packages/extension/vite.config.js (2)
extensions/container/packages/api/vite.config.js (1)
PACKAGE_ROOT(21-21)eslint.config.mjs (1)
__dirname(40-40)
🪛 Biome (2.1.2)
extensions/container/packages/api/src/container-extension-api.d.ts
[error] 20-20: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: smoke e2e tests (production)
- GitHub Check: smoke e2e tests (development)
- GitHub Check: typecheck
- GitHub Check: linter, formatters
- GitHub Check: Windows
- GitHub Check: Linux
- GitHub Check: macOS
🔇 Additional comments (25)
extensions/container/packages/extension/scripts/build.js (2)
20-24: LGTM!Dependencies are appropriate for the packaging task.
26-28: LGTM!Path resolution correctly uses package.json name for consistent output naming.
extensions/container/packages/extension/src/manager/_manager-module.ts (1)
23-25: LGTM!The Inversify module binding follows best practices by registering ContainerEngineManager as a singleton, aligning with the pattern of registering major services as singletons in the DI container.
extensions/container/packages/extension/src/extension.spec.ts (1)
37-54: LGTM!The tests appropriately verify the extension lifecycle (activate/deactivate) and follow the co-location naming convention (*.spec.ts) as per coding guidelines.
extensions/container/packages/extension/package.json (1)
1-36: Extension manifest structure is correct for a skeleton.The
mainfield correctly points to./dist/extension.jsandengines.kortexdeclares version compatibility as expected. As noted in the PR description, this is a skeleton extension not yet included in the main build.Note: When the extension moves beyond skeleton status, you may want to add a
contributessection if configuration properties or other contributions are needed. As per coding guidelines, extension configuration properties should be declared undercontributes.configuration.propertiesin package.json.extensions/container/packages/extension/src/container-extension.spec.ts (4)
26-27: LGTM! Modern mock syntax.The use of
vi.mock(import('...'))with dynamic imports is correct for Vitest. This ensures proper ESM mocking.
45-48: LGTM! Activation test covers the happy path.The test correctly verifies that
ContainerEngineManager.prototype.initis called during activation.
50-57: LGTM! Error handling test is well-structured.The test properly mocks a failure scenario during container creation and verifies that the error propagates correctly.
59-63: LGTM! Deactivation test verifies cleanup.The test ensures that
disposeis called on the manager during deactivation, which is essential for proper resource cleanup.extensions/container/packages/extension/src/inject/symbol.ts (1)
19-23: LGTM! Symbol definitions are appropriate.The use of
Symbol.for()forExtensionContextSymbolandTelemetryLoggerSymbolenables cross-module symbol sharing, whileContainersProvideruses a local symbol for extension-specific binding. This is a reasonable design choice.extensions/container/packages/extension/src/inject/inversify-binding.spec.ts (2)
46-59: LGTM! Comprehensive initialization test.The test correctly verifies that all three symbols are bound, the managers module is loaded, and the ContainerEngineManager is resolved during initialization.
61-69: LGTM! Disposal test verifies cleanup.The test ensures that
unbindAllis called on the container during disposal, which properly releases all DI bindings.extensions/container/packages/extension/src/extension.ts (2)
25-28: LGTM! Standard extension activation pattern.The lazy initialization using
??=and delegation to the ContainerExtension class is clean and follows the expected extension lifecycle pattern. Based on learnings.
30-33: LGTM! Proper cleanup in deactivate.The deactivation properly calls the extension's deactivate method and clears the singleton instance, ensuring proper cleanup on extension shutdown.
extensions/container/packages/extension/src/inject/inversify-binding.ts (2)
39-51: LGTM! Proper DI initialization with eager resolution.The binding initialization correctly:
- Creates the container and binds all necessary symbols
- Loads the managers module
- Eagerly resolves ContainerEngineManager to validate the DI configuration
The eager resolution on Line 49 is a good practice for catching DI configuration errors early.
53-57: LGTM! Proper container disposal.The disposal logic correctly unbinds all container bindings when the container exists, ensuring proper cleanup of DI resources.
extensions/container/packages/extension/types/template.d.ts (1)
19-22: LGTM! Standard PNG module declaration.This declaration enables importing PNG files as strings in TypeScript, which is necessary for the extension's icon references. The
export =syntax is appropriate for this use case.extensions/container/packages/extension/vite.config.js (2)
28-56: LGTM! Proper extension build configuration.The Vite configuration correctly:
- Sets entry to
src/extension.ts(as per learnings)- Uses CJS format for extension compatibility
- Externalizes
@kortex-app/apiand Node.js builtins appropriately- Outputs
extension.jsin the dist directoryBased on learnings.
57-64: LGTM! Test configuration with proper mocking.The test configuration correctly:
- Uses Node environment for extension tests
- Includes the standard test file patterns
- Provides a mock alias for
@kortex-app/apithat correctly navigates up to the repository root's__mocks__directoryextensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
21-30: LGTM! Skeleton implementation with proper lifecycle structure.The ContainerEngineManager provides a clear skeleton with the expected lifecycle methods (
init,dispose) and API surface (exports). The placeholder implementations are appropriate for a skeleton extension as noted in the PR description.extensions/container/packages/api/package.json (2)
15-21: Empty scripts are correct for declaration-only package.The empty
build,prepare, andwatchscripts are intentional for this declaration-only API package that ships pure TypeScript declaration files without compiled JavaScript.Based on learnings.
1-14: No issue found. API packages intentionally lackengines.kortex.The verification shows a clear pattern: main extension packages have
engines.kortexdefined, but API/utility packages do not. The container extension's API package atextensions/container/packages/api/is correctly configured as a declaration-only package withoutengines.kortex. This is consistent with all other extensions in the repository—only the main extension entry points (e.g.,extensions/container/packages/extension/) require theengines.kortexfield. The absence of this field in the API package is expected and correct.extensions/container/packages/extension/src/container-extension.ts (3)
18-24: LGTM!All imports are necessary and correctly reference the required dependencies for the extension lifecycle implementation.
25-34: LGTM!The class structure is clean with appropriately scoped private fields and a minimal constructor that stores the extension context for later use during activation.
70-72: LGTM!The protected accessor enables testability while maintaining encapsulation of the internal DI container.
| const zip = new AdmZip(); | ||
| zip.addLocalFolder(path.resolve(__dirname, '../')); | ||
| zip.writeZip(destFile); |
There was a problem hiding this comment.
Critical: Indiscriminate packaging includes development artifacts and dependencies.
The script zips the entire parent directory without filtering, which will include:
node_modules/(potentially hundreds of MB if present)- Build scripts and source files
- Test files and configuration
- Any temporary files or development artifacts
This creates bloated .cdix archives with unnecessary and potentially sensitive files. Extensions should only ship runtime-necessary files.
Apply this diff to package only required runtime files:
const zip = new AdmZip();
-zip.addLocalFolder(path.resolve(__dirname, '../'));
+// Only package runtime-necessary files
+const extensionRoot = path.resolve(__dirname, '../');
+const distPath = path.join(extensionRoot, 'dist');
+const packageJsonPath = path.join(extensionRoot, 'package.json');
+
+// Validate that the build output exists
+if (!fs.existsSync(distPath) || !fs.existsSync(path.join(distPath, 'extension.js'))) {
+ console.error('Build output not found. Run build before packaging.');
+ process.exit(1);
+}
+
+// Add only necessary files
+zip.addLocalFolder(distPath, 'dist');
+zip.addLocalFile(packageJsonPath);
+
+// Optionally add LICENSE, README, etc. if they exist
+const optionalFiles = ['LICENSE', 'README.md', 'CHANGELOG.md'];
+optionalFiles.forEach(file => {
+ const filePath = path.join(extensionRoot, file);
+ if (fs.existsSync(filePath)) {
+ zip.addLocalFile(filePath);
+ }
+});
+
zip.writeZip(destFile);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In extensions/container/packages/extension/scripts/build.js around lines 38-40,
the current zip.addLocalFolder(path.resolve(__dirname, '../')) indiscriminately
packages the entire parent directory (including node_modules, tests, configs and
dev artefacts); replace this with a targeted packaging strategy that only
includes runtime files: build a list/array of allowed runtime paths (e.g.,
package.json, package-lock.json, dist/ or lib/, bin/, assets/, README, LICENSE),
use glob patterns or AdmZip.addLocalFile/addLocalFolder per-entry to add only
those, and ensure node_modules is either omitted or recreated as production-only
(npm ci --production or npm prune --production) before adding; also respect
package.json "files" if present and explicitly exclude dotfiles, test, and
config directories to keep the archive minimal and free of dev artifacts.
There was a problem hiding this comment.
ignoring for now as all build scripts are the same in each extension
There was a problem hiding this comment.
@benoitf, understood! Since this pattern is consistent across all extensions, addressing it would indeed require a broader refactoring effort beyond the scope of this PR.
Would you like me to create an issue to track improving the build scripts across all extensions to package only runtime-necessary files? This would help ensure the .cdix archives remain lean and free of development artifacts in the future.
🧠 Learnings used
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/dist/extension.js : Built extensions must output dist/extension.js (Vite build result)
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/package.json : Set package.json main to ./dist/extension.js for extensions
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Each extension’s package.json must set "main" to "./dist/extension.js"
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/{preload,preload-webview}/**/*.{ts,js} : Place Preload scripts under packages/preload and packages/preload-webview
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 678
File: extensions/container/packages/api/vite.config.js:34-40
Timestamp: 2025-11-05T16:31:22.355Z
Learning: In the kortex repository, the `kortex-app/container-extension-api` package at `extensions/container/packages/api/` is a declaration-only package that ships pure TypeScript declaration files (.d.ts) without any compiled JavaScript. It intentionally has an empty build script and does not require library build configuration in vite.config.js.
simonrey1
left a comment
simonrey1
left a comment
There was a problem hiding this comment.
LGTM, I was able to start and stop the extension without visible issues in the rest of app
benoitf
force-pushed
the
skeleton-container-extension
branch
from
d47bf81 to
a2a753f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
extensions/container/packages/extension/src/inject/inversify-binding.ts (2)
39-51: Consider guarding against multiple initialization calls.The
initBindings()method creates a new container instance each time it's called without checking if one already exists. While the current usage appears to call it only once, adding a guard would prevent potential resource leaks if accidentally called multiple times.Consider applying this diff:
public async initBindings(): Promise<Container> { + if (this.#container) { + throw new Error('Container already initialized'); + } this.#container = new Container();Note: The eager resolution of
ContainerEngineManagerat line 49 ensures initialization but the instance is resolved again later by the caller. This is acceptable for singletons but could be noted in a comment.
53-57: Consider clearing the container reference after disposal.After calling
unbindAll(), the container reference remains set, which could lead to confusion or allow multiple disposal attempts. Consider setting#containertoundefinedafter cleanup.Apply this diff:
async dispose(): Promise<void> { if (this.#container) { await this.#container.unbindAll(); + this.#container = undefined; } }extensions/container/packages/extension/src/container-extension.ts (2)
52-63: Consider removing optional chaining for clarity.At line 56,
getContainer()is called immediately after assigning#containerat line 53, so the container should always be defined. The optional chaining (?.) is defensive but unnecessary here and may mask issues. Consider using a direct call or adding an assertion.Apply this diff for clarity:
- try { - this.#containerEngineManager = await this.getContainer()?.getAsync(ContainerEngineManager); - } catch (e) { - console.error('Error while creating the container provider manager', e); - throw e; - } - if (!this.#containerEngineManager) { - throw new Error('ContainerEngineManager not found in container'); - } + if (!this.#container) { + throw new Error('Container not initialized'); + } + try { + this.#containerEngineManager = await this.#container.getAsync(ContainerEngineManager); + } catch (e) { + console.error('Error while creating the container provider manager', e); + throw e; + }
74-78: Consider consistent cleanup in deactivate().The deactivation logic correctly disposes of the manager and bindings. However, for consistency with setting
#containerEngineManagertoundefinedat line 76, consider also clearing the#inversifyBindingand#containerreferences after disposal.Apply this diff for consistency:
async deactivate(): Promise<void> { this.#containerEngineManager?.dispose(); this.#containerEngineManager = undefined; await this.#inversifyBinding?.dispose(); + this.#inversifyBinding = undefined; + this.#container = undefined; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
extensions/container/packages/extension/icon.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
extensions/container/packages/api/package.json(1 hunks)extensions/container/packages/api/src/container-extension-api.d.ts(1 hunks)extensions/container/packages/api/tsconfig.json(1 hunks)extensions/container/packages/api/vite.config.js(1 hunks)extensions/container/packages/extension/README.md(1 hunks)extensions/container/packages/extension/package.json(1 hunks)extensions/container/packages/extension/scripts/build.js(1 hunks)extensions/container/packages/extension/src/container-extension.spec.ts(1 hunks)extensions/container/packages/extension/src/container-extension.ts(1 hunks)extensions/container/packages/extension/src/extension.spec.ts(1 hunks)extensions/container/packages/extension/src/extension.ts(1 hunks)extensions/container/packages/extension/src/inject/inversify-binding.spec.ts(1 hunks)extensions/container/packages/extension/src/inject/inversify-binding.ts(1 hunks)extensions/container/packages/extension/src/inject/symbol.ts(1 hunks)extensions/container/packages/extension/src/manager/_manager-module.ts(1 hunks)extensions/container/packages/extension/src/manager/container-engine-manager.ts(1 hunks)extensions/container/packages/extension/tsconfig.json(1 hunks)extensions/container/packages/extension/types/template.d.ts(1 hunks)extensions/container/packages/extension/vite.config.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- extensions/container/packages/extension/src/inject/symbol.ts
- extensions/container/packages/extension/src/container-extension.spec.ts
- extensions/container/packages/api/vite.config.js
- extensions/container/packages/extension/src/extension.ts
- extensions/container/packages/extension/tsconfig.json
- extensions/container/packages/extension/vite.config.js
- extensions/container/packages/extension/src/manager/container-engine-manager.ts
- extensions/container/packages/extension/scripts/build.js
🧰 Additional context used
📓 Path-based instructions (2)
{packages,extensions}/**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Co-locate unit tests with source files and name them *.spec.ts
Files:
extensions/container/packages/extension/src/inject/inversify-binding.spec.tsextensions/container/packages/extension/src/extension.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate unit tests alongside source files using the *.spec.ts naming convention
Files:
extensions/container/packages/extension/src/inject/inversify-binding.spec.tsextensions/container/packages/extension/src/extension.spec.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 678
File: extensions/container/packages/api/vite.config.js:34-40
Timestamp: 2025-11-05T16:31:22.355Z
Learning: In the kortex repository, the `kortex-app/container-extension-api` package at `extensions/container/packages/api/` is a declaration-only package that ships pure TypeScript declaration files (.d.ts) without any compiled JavaScript. It intentionally has an empty build script and does not require library build configuration in vite.config.js.
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/main/**/*.ts : Container operations must include an engineId parameter when invoking ContainerProviderRegistry actions
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/src/**/*.ts : Extensions should use kortex-app/api for provider registration, configuration, and contributions
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to packages/main/src/plugin/index.ts : Register all major services as singletons in the Inversify DI container during initialization in the main process plugin entrypoint
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/main/src/plugin/index.ts : Register all major services as singletons in the Inversify DI container during initialization
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/src/extension.ts : Extensions must export a standard activation API with an activate() function in src/extension.ts
📚 Learning: 2025-11-05T16:31:22.355Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 678
File: extensions/container/packages/api/vite.config.js:34-40
Timestamp: 2025-11-05T16:31:22.355Z
Learning: In the kortex repository, the `kortex-app/container-extension-api` package at `extensions/container/packages/api/` is a declaration-only package that ships pure TypeScript declaration files (.d.ts) without any compiled JavaScript. It intentionally has an empty build script and does not require library build configuration in vite.config.js.
Applied to files:
extensions/container/packages/extension/README.mdextensions/container/packages/api/tsconfig.jsonextensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/package.jsonextensions/container/packages/api/src/container-extension-api.d.tsextensions/container/packages/extension/src/extension.spec.tsextensions/container/packages/extension/types/template.d.tsextensions/container/packages/extension/src/manager/_manager-module.tsextensions/container/packages/api/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/src/**/*.ts : Extensions should use kortex-app/api for provider registration, configuration, and contributions
Applied to files:
extensions/container/packages/extension/README.mdextensions/container/packages/api/tsconfig.jsonextensions/container/packages/extension/src/inject/inversify-binding.tsextensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/package.jsonextensions/container/packages/api/src/container-extension-api.d.tsextensions/container/packages/api/package.json
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to {packages,extensions}/**/*.spec.ts : Co-locate unit tests with source files and name them *.spec.ts
Applied to files:
extensions/container/packages/api/tsconfig.jsonextensions/container/packages/extension/src/inject/inversify-binding.spec.tsextensions/container/packages/extension/src/extension.spec.ts
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Extension configuration properties must be declared under contributes.configuration.properties in package.json
Applied to files:
extensions/container/packages/api/tsconfig.jsonextensions/container/packages/extension/package.jsonextensions/container/packages/api/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/package.json : Define configuration under contributes.configuration.properties in extension package.json
Applied to files:
extensions/container/packages/api/tsconfig.jsonextensions/container/packages/extension/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to **/*.spec.ts : Co-locate unit tests alongside source files using the *.spec.ts naming convention
Applied to files:
extensions/container/packages/api/tsconfig.jsonextensions/container/packages/extension/src/inject/inversify-binding.spec.tsextensions/container/packages/extension/src/extension.spec.ts
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/main/src/plugin/index.ts : Register all major services as singletons in the Inversify DI container during initialization
Applied to files:
extensions/container/packages/extension/src/inject/inversify-binding.tsextensions/container/packages/extension/src/inject/inversify-binding.spec.tsextensions/container/packages/extension/src/manager/_manager-module.ts
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to packages/main/src/plugin/index.ts : Register all major services as singletons in the Inversify DI container during initialization in the main process plugin entrypoint
Applied to files:
extensions/container/packages/extension/src/inject/inversify-binding.tsextensions/container/packages/extension/src/inject/inversify-binding.spec.tsextensions/container/packages/extension/src/manager/_manager-module.ts
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/src/extension.ts : Extensions must export a standard activation API with an activate() function in src/extension.ts
Applied to files:
extensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/extension.spec.ts
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/src/extension.ts : Extension entry src/extension.ts must export an activate() function
Applied to files:
extensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/extension.spec.ts
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/main/**/*.ts : Container operations must include an engineId parameter when invoking ContainerProviderRegistry actions
Applied to files:
extensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/package.json : Set package.json main to ./dist/extension.js for extensions
Applied to files:
extensions/container/packages/extension/package.jsonextensions/container/packages/api/package.json
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Each extension’s package.json must set "main" to "./dist/extension.js"
Applied to files:
extensions/container/packages/extension/package.jsonextensions/container/packages/api/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/package.json : Each extension must declare engines.kortex version compatibility in package.json
Applied to files:
extensions/container/packages/extension/package.jsonextensions/container/packages/api/package.json
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Each extension must declare Kortex compatibility via engines.kortex in package.json
Applied to files:
extensions/container/packages/extension/package.jsonextensions/container/packages/api/package.json
🧬 Code graph analysis (5)
extensions/container/packages/extension/src/inject/inversify-binding.ts (4)
packages/extension-api/src/extension-api.d.ts (2)
ExtensionContext(183-203)TelemetryLogger(4676-4726)__mocks__/@kortex-app/api.js (1)
provider(27-29)extensions/container/packages/extension/src/inject/symbol.ts (3)
ExtensionContextSymbol(22-22)TelemetryLoggerSymbol(23-23)ContainersProvider(20-20)extensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
ContainerEngineManager(22-30)
extensions/container/packages/extension/src/inject/inversify-binding.spec.ts (3)
extensions/container/packages/extension/src/inject/inversify-binding.ts (1)
InversifyBinding(26-58)extensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
ContainerEngineManager(22-30)extensions/container/packages/extension/src/inject/symbol.ts (3)
ExtensionContextSymbol(22-22)TelemetryLoggerSymbol(23-23)ContainersProvider(20-20)
extensions/container/packages/extension/src/container-extension.ts (5)
packages/extension-api/src/extension-api.d.ts (1)
ExtensionContext(183-203)extensions/container/packages/extension/src/inject/inversify-binding.ts (1)
InversifyBinding(26-58)extensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
ContainerEngineManager(22-30)extensions/container/packages/api/src/container-extension-api.d.ts (1)
ContainerExtensionAPI(20-20)__mocks__/@kortex-app/api.js (2)
env(31-33)provider(27-29)
extensions/container/packages/extension/src/extension.spec.ts (3)
packages/extension-api/src/extension-api.d.ts (1)
ExtensionContext(183-203)extensions/container/packages/extension/src/container-extension.ts (3)
activate(36-68)ContainerExtension(25-79)deactivate(74-78)extensions/container/packages/extension/src/extension.ts (2)
activate(25-28)deactivate(30-33)
extensions/container/packages/extension/src/manager/_manager-module.ts (1)
extensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
ContainerEngineManager(22-30)
🪛 Biome (2.1.2)
extensions/container/packages/api/src/container-extension-api.d.ts
[error] 20-20: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: smoke e2e tests (production)
- GitHub Check: smoke e2e tests (development)
- GitHub Check: typecheck
- GitHub Check: linter, formatters
- GitHub Check: macOS
- GitHub Check: Linux
- GitHub Check: Windows
🔇 Additional comments (13)
extensions/container/packages/extension/types/template.d.ts (1)
19-22: LGTM! Standard PNG import declaration.The module declaration correctly enables PNG files to be imported as strings, which is a standard pattern for asset handling in TypeScript projects.
extensions/container/packages/api/src/container-extension-api.d.ts (1)
19-20: LGTM! Intentionally empty skeleton API.The empty interface serves as a placeholder for the API surface. While Biome suggests using a type alias, keeping it as an interface allows for future declaration merging when the API is implemented.
extensions/container/packages/extension/README.md (1)
1-11: LGTM! Clear and concise documentation.The README appropriately describes the extension's purpose, API location, and runtime requirements.
extensions/container/packages/extension/src/extension.spec.ts (1)
37-54: LGTM! Tests correctly verify extension lifecycle.The tests appropriately verify that:
- The
activate()function initializes and activates the ContainerExtension- The
deactivate()function properly cleans up by calling ContainerExtension's deactivate methodThe mock setup and assertions are correct for this skeleton implementation.
extensions/container/packages/extension/src/manager/_manager-module.ts (1)
23-25: LGTM! Correct DI binding pattern.The ContainerEngineManager is appropriately registered as a singleton using Inversify's ContainerModule, aligning with the project's DI patterns. Based on learnings.
extensions/container/packages/api/tsconfig.json (1)
1-13: LGTM! Configuration aligns with declaration-only package.The TypeScript configuration correctly:
- Extends the strictest preset for maximum type safety
- Includes only
.d.tsdeclaration files- Uses appropriate settings for a Node.js-targeted declaration package
This aligns with the learning that the container-extension-api package ships pure TypeScript declarations. Based on learnings.
extensions/container/packages/extension/package.json (1)
1-31: LGTM! Package manifest follows extension conventions.The package.json correctly:
- Sets
mainto./dist/extension.jsper conventions- Declares Kortex engine compatibility
- Uses workspace references for monorepo packages
- Includes appropriate build and test scripts
- Lists inversify as a runtime dependency for DI support
The previous dependency version concerns were appropriately addressed by the author. Based on learnings.
extensions/container/packages/extension/src/inject/inversify-binding.spec.ts (1)
46-69: LGTM! Tests correctly verify DI binding lifecycle.The tests appropriately verify:
- Initialization: All required symbols (ExtensionContextSymbol, TelemetryLoggerSymbol, ContainersProvider) are bound and managersModule is loaded
- Disposal: The container is properly cleaned up via unbindAll
The mock setup correctly isolates the Inversify container behavior for testing. Based on learnings.
extensions/container/packages/api/package.json (1)
1-25: LGTM! Declaration-only API package correctly configured.The package is properly set up as a declaration-only package with the
typesfield pointing to the TypeScript declarations and empty build scripts, consistent with the established pattern for API packages in this repository. The configuration aligns with the expected structure for shipping pure TypeScript declaration files.Based on learnings.
extensions/container/packages/extension/src/inject/inversify-binding.ts (1)
26-37: LGTM! Constructor properly stores dependencies.The constructor correctly stores the Provider, ExtensionContext, and TelemetryLogger for use during container initialization.
extensions/container/packages/extension/src/container-extension.ts (3)
25-34: LGTM! Constructor properly initializes state.The constructor correctly stores the ExtensionContext and initializes manager-related fields to undefined, which will be set during activation.
36-50: LGTM! Provider creation follows standard pattern.The telemetry logger and container provider are correctly created during activation. The provider configuration looks appropriate for a container extension skeleton, with the initial status set to 'unknown'.
65-68: LGTM! Manager initialization follows standard pattern.The ContainerEngineManager is properly initialized and its exports are returned as the extension API, following the standard extension activation pattern.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
extensions/container/packages/extension/src/container-extension.ts(1 hunks)extensions/container/packages/extension/src/inject/inversify-binding.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extensions/container/packages/extension/src/container-extension.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 678
File: extensions/container/packages/api/vite.config.js:34-40
Timestamp: 2025-11-05T16:31:22.355Z
Learning: In the kortex repository, the `kortex-app/container-extension-api` package at `extensions/container/packages/api/` is a declaration-only package that ships pure TypeScript declaration files (.d.ts) without any compiled JavaScript. It intentionally has an empty build script and does not require library build configuration in vite.config.js.
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/src/**/*.ts : Extensions should use kortex-app/api for provider registration, configuration, and contributions
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Each extension must declare Kortex compatibility via engines.kortex in package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/main/src/plugin/index.ts : Register all major services as singletons in the Inversify DI container during initialization
Applied to files:
extensions/container/packages/extension/src/inject/inversify-binding.ts
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to packages/main/src/plugin/index.ts : Register all major services as singletons in the Inversify DI container during initialization in the main process plugin entrypoint
Applied to files:
extensions/container/packages/extension/src/inject/inversify-binding.ts
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/src/**/*.ts : Extensions should use kortex-app/api for provider registration, configuration, and contributions
Applied to files:
extensions/container/packages/extension/src/inject/inversify-binding.ts
🧬 Code graph analysis (1)
extensions/container/packages/extension/src/inject/inversify-binding.ts (5)
packages/extension-api/src/extension-api.d.ts (2)
ExtensionContext(183-203)TelemetryLogger(4676-4726)__mocks__/@kortex-app/api.js (1)
provider(27-29)extensions/container/packages/extension/src/inject/symbol.ts (3)
ExtensionContextSymbol(22-22)TelemetryLoggerSymbol(23-23)ContainersProvider(20-20)extensions/container/packages/extension/src/manager/_manager-module.ts (1)
managersModule(27-27)extensions/container/packages/extension/src/manager/container-engine-manager.ts (1)
ContainerEngineManager(22-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: smoke e2e tests (production)
- GitHub Check: smoke e2e tests (development)
- GitHub Check: Linux
- GitHub Check: macOS
- GitHub Check: typecheck
- GitHub Check: linter, formatters
- GitHub Check: Windows
🔇 Additional comments (2)
extensions/container/packages/extension/src/inject/inversify-binding.ts (2)
26-37: LGTM!Clean encapsulation with private fields and straightforward dependency injection in the constructor.
39-54: Dead binding detected:ContainersProvideris bound but never used.The
ContainersProvideris bound at line 47 but is not injected or retrieved anywhere in the codebase—not bymanagersModule,ContainerEngineManager, or any other service. Clarify whether this binding is:
- Intentional for future use (consider adding a comment)
- Dead code and should be removed
Additionally, the partial initialization concern remains valid: if
load()orgetAsync()fails after line 43,this.#containeris set, preventing retry attempts on the nextinitBindings()call.
benoitf
force-pushed
the
skeleton-container-extension
branch
from
b3a4f0d to
1d89628
Compare
|
rebased on top of the main branch |
axel7083
left a comment
axel7083
left a comment
There was a problem hiding this comment.
LGTM on windows, the extension is not failing ;p
extensions/container/packages/extension/src/manager/container-engine-manager.ts
Show resolved
Hide resolved
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
benoitf
force-pushed
the
skeleton-container-extension
branch
from
1d89628 to
7747983
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
extensions/container/packages/extension/icon.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
extensions/container/packages/api/package.json(1 hunks)extensions/container/packages/api/src/container-extension-api.d.ts(1 hunks)extensions/container/packages/api/tsconfig.json(1 hunks)extensions/container/packages/extension/README.md(1 hunks)extensions/container/packages/extension/package.json(1 hunks)extensions/container/packages/extension/scripts/build.js(1 hunks)extensions/container/packages/extension/src/container-extension.spec.ts(1 hunks)extensions/container/packages/extension/src/container-extension.ts(1 hunks)extensions/container/packages/extension/src/extension.spec.ts(1 hunks)extensions/container/packages/extension/src/extension.ts(1 hunks)extensions/container/packages/extension/src/inject/inversify-binding.spec.ts(1 hunks)extensions/container/packages/extension/src/inject/inversify-binding.ts(1 hunks)extensions/container/packages/extension/src/inject/symbol.ts(1 hunks)extensions/container/packages/extension/src/manager/_manager-module.ts(1 hunks)extensions/container/packages/extension/src/manager/container-engine-manager.ts(1 hunks)extensions/container/packages/extension/tsconfig.json(1 hunks)extensions/container/packages/extension/types/template.d.ts(1 hunks)extensions/container/packages/extension/vite.config.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- extensions/container/packages/api/tsconfig.json
- extensions/container/packages/extension/src/inject/inversify-binding.spec.ts
- extensions/container/packages/extension/src/manager/container-engine-manager.ts
- extensions/container/packages/extension/package.json
- extensions/container/packages/extension/types/template.d.ts
- extensions/container/packages/extension/scripts/build.js
- extensions/container/packages/extension/src/extension.spec.ts
- extensions/container/packages/extension/src/manager/_manager-module.ts
- extensions/container/packages/extension/README.md
- extensions/container/packages/extension/vite.config.js
🧰 Additional context used
📓 Path-based instructions (2)
{packages,extensions}/**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Co-locate unit tests with source files and name them *.spec.ts
Files:
extensions/container/packages/extension/src/container-extension.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate unit tests alongside source files using the *.spec.ts naming convention
Files:
extensions/container/packages/extension/src/container-extension.spec.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 678
File: extensions/container/packages/api/vite.config.js:34-40
Timestamp: 2025-11-05T16:31:22.355Z
Learning: In the kortex repository, the `kortex-app/container-extension-api` package at `extensions/container/packages/api/` is a declaration-only package that ships pure TypeScript declaration files (.d.ts) without any compiled JavaScript. It intentionally has an empty build script and does not require library build configuration in vite.config.js.
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/src/**/*.ts : Extensions should use kortex-app/api for provider registration, configuration, and contributions
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Each extension must declare Kortex compatibility via engines.kortex in package.json
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/src/extension.ts : Extensions must export a standard activation API with an activate() function in src/extension.ts
Applied to files:
extensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/extension.tsextensions/container/packages/extension/src/container-extension.spec.ts
📚 Learning: 2025-11-05T16:31:22.355Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 678
File: extensions/container/packages/api/vite.config.js:34-40
Timestamp: 2025-11-05T16:31:22.355Z
Learning: In the kortex repository, the `kortex-app/container-extension-api` package at `extensions/container/packages/api/` is a declaration-only package that ships pure TypeScript declaration files (.d.ts) without any compiled JavaScript. It intentionally has an empty build script and does not require library build configuration in vite.config.js.
Applied to files:
extensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/extension.tsextensions/container/packages/extension/src/container-extension.spec.tsextensions/container/packages/extension/src/inject/inversify-binding.tsextensions/container/packages/api/package.jsonextensions/container/packages/extension/tsconfig.jsonextensions/container/packages/api/src/container-extension-api.d.ts
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/src/extension.ts : Extension entry src/extension.ts must export an activate() function
Applied to files:
extensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/extension.tsextensions/container/packages/extension/src/container-extension.spec.ts
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/src/**/*.ts : Extensions should use kortex-app/api for provider registration, configuration, and contributions
Applied to files:
extensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/extension.tsextensions/container/packages/extension/src/container-extension.spec.tsextensions/container/packages/extension/src/inject/inversify-binding.tsextensions/container/packages/api/package.jsonextensions/container/packages/extension/tsconfig.jsonextensions/container/packages/api/src/container-extension-api.d.ts
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/main/**/*.ts : Container operations must include an engineId parameter when invoking ContainerProviderRegistry actions
Applied to files:
extensions/container/packages/extension/src/container-extension.tsextensions/container/packages/extension/src/container-extension.spec.ts
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to {packages,extensions}/**/*.spec.ts : Co-locate unit tests with source files and name them *.spec.ts
Applied to files:
extensions/container/packages/extension/src/container-extension.spec.tsextensions/container/packages/extension/tsconfig.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/main/src/plugin/index.ts : Register all major services as singletons in the Inversify DI container during initialization
Applied to files:
extensions/container/packages/extension/src/inject/inversify-binding.ts
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to packages/main/src/plugin/index.ts : Register all major services as singletons in the Inversify DI container during initialization in the main process plugin entrypoint
Applied to files:
extensions/container/packages/extension/src/inject/inversify-binding.ts
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Each extension must declare Kortex compatibility via engines.kortex in package.json
Applied to files:
extensions/container/packages/api/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/package.json : Each extension must declare engines.kortex version compatibility in package.json
Applied to files:
extensions/container/packages/api/package.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/package.json : Set package.json main to ./dist/extension.js for extensions
Applied to files:
extensions/container/packages/api/package.jsonextensions/container/packages/extension/tsconfig.json
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Each extension’s package.json must set "main" to "./dist/extension.js"
Applied to files:
extensions/container/packages/api/package.jsonextensions/container/packages/extension/tsconfig.json
📚 Learning: 2025-10-24T07:15:59.216Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-24T07:15:59.216Z
Learning: Applies to extensions/*/package.json : Extension configuration properties must be declared under contributes.configuration.properties in package.json
Applied to files:
extensions/container/packages/api/package.jsonextensions/container/packages/extension/tsconfig.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to extensions/*/package.json : Define configuration under contributes.configuration.properties in extension package.json
Applied to files:
extensions/container/packages/extension/tsconfig.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to **/*.spec.ts : Co-locate unit tests alongside source files using the *.spec.ts naming convention
Applied to files:
extensions/container/packages/extension/tsconfig.json
📚 Learning: 2025-10-24T07:16:48.460Z
Learnt from: CR
Repo: kortex-hub/kortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-24T07:16:48.460Z
Learning: Applies to packages/{preload,preload-webview}/**/*.{ts,js} : Place Preload scripts under packages/preload and packages/preload-webview
Applied to files:
extensions/container/packages/extension/tsconfig.json
🧬 Code graph analysis (4)
extensions/container/packages/extension/src/container-extension.ts (4)
packages/extension-api/src/extension-api.d.ts (1)
ExtensionContext(183-203)extensions/container/packages/extension/src/inject/inversify-binding.ts (1)
InversifyBinding(26-62)extensions/container/packages/api/src/container-extension-api.d.ts (1)
ContainerExtensionAPI(20-20)__mocks__/@kortex-app/api.js (2)
env(31-33)provider(27-29)
extensions/container/packages/extension/src/extension.ts (3)
extensions/container/packages/extension/src/container-extension.ts (3)
ContainerExtension(25-81)activate(36-68)deactivate(74-80)packages/extension-api/src/extension-api.d.ts (1)
ExtensionContext(183-203)extensions/container/packages/api/src/container-extension-api.d.ts (1)
ContainerExtensionAPI(20-20)
extensions/container/packages/extension/src/container-extension.spec.ts (2)
extensions/container/packages/extension/src/container-extension.ts (1)
ContainerExtension(25-81)packages/extension-api/src/extension-api.d.ts (1)
ExtensionContext(183-203)
extensions/container/packages/extension/src/inject/inversify-binding.ts (2)
packages/extension-api/src/extension-api.d.ts (2)
ExtensionContext(183-203)TelemetryLogger(4676-4726)extensions/container/packages/extension/src/inject/symbol.ts (3)
ExtensionContextSymbol(22-22)TelemetryLoggerSymbol(23-23)ContainersProvider(20-20)
🪛 Biome (2.1.2)
extensions/container/packages/api/src/container-extension-api.d.ts
[error] 20-20: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: typecheck
- GitHub Check: linter, formatters
- GitHub Check: smoke e2e tests (development)
- GitHub Check: smoke e2e tests (production)
- GitHub Check: Windows
- GitHub Check: macOS
- GitHub Check: Linux
🔇 Additional comments (8)
extensions/container/packages/api/src/container-extension-api.d.ts (1)
19-20: LGTM! Empty interface is appropriate for a skeleton API.The empty interface with the ESLint directive is the correct approach for a placeholder API surface. Unlike a type alias (as suggested by static analysis), an interface supports declaration merging, allowing future extension packages to augment this API without breaking changes.
extensions/container/packages/extension/tsconfig.json (1)
1-23: LGTM! TypeScript configuration is appropriate for Inversify-based DI.The configuration correctly enables experimental decorators and decorator metadata (required for Inversify), sets up path aliases, and uses strict type checking with appropriate relaxations for DI patterns.
extensions/container/packages/extension/src/container-extension.spec.ts (1)
45-48: LGTM! Tests appropriately cover the extension lifecycle.The test suite covers key scenarios: successful activation, error handling during initialization, and proper cleanup on deactivation. The use of a test helper class to access protected methods is a clean testing pattern.
extensions/container/packages/extension/src/inject/symbol.ts (1)
19-23: LGTM! Symbol usage pattern is appropriate for DI tokens.The use of
Symbol.for()forExtensionContextandTelemetryLoggercreates global symbols suitable for host-provided dependencies, whileSymbol()forContainersProvidercreates a unique token for extension-specific bindings. This pattern correctly distinguishes between shared and local DI tokens.extensions/container/packages/extension/src/extension.ts (1)
25-28: LGTM! Standard extension lifecycle implementation.The entry point correctly implements the required
activate()function, uses lazy singleton initialization with nullish coalescing, and returns theContainerExtensionAPItype. The lifecycle management is clean and follows Kortex extension patterns.Based on learnings.
extensions/container/packages/extension/src/inject/inversify-binding.ts (1)
39-54: LGTM! Clean DI container setup with proper initialization.The binding initialization correctly:
- Guards against re-initialization
- Binds host-provided dependencies (context, telemetry, provider) to DI tokens
- Loads the manager module
- Eagerly resolves
ContainerEngineManagerto ensure proper initialization orderThe separation of concerns is clean: this class manages the DI container lifecycle while
ContainerExtensionmanages the manager lifecycle.extensions/container/packages/extension/src/container-extension.ts (2)
36-68: LGTM! Well-structured extension activation with proper error handling.The activation flow is logical and robust:
- Creates telemetry logger and provider
- Initializes DI container with bindings
- Resolves
ContainerEngineManagerwith try-catch error handling- Initializes the manager
- Returns the public API
The error handling around service resolution (Lines 55-63) is particularly good, providing clear error messages for debugging.
74-80: LGTM! Proper cleanup order in deactivation.The deactivation correctly disposes resources in reverse order: manager disposal first, then DI container cleanup, followed by clearing all references. This ensures proper resource cleanup and prevents memory leaks.
* feat: add skeleton for container extension Signed-off-by: Florent Benoit <fbenoit@redhat.com>
* feat: add skeleton for container extension Signed-off-by: Florent Benoit <fbenoit@redhat.com>
4 participants
note: there is no package.json scripts to build this extension when performing the main build so this extension is not included in the binary as it's still a skeleton. (Only visible for pnpm watch)