feat: add Infrastructure view into Kaoto side bar#1365
Open
djelinek wants to merge 13 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds an Infrastructure tree view and CLI-driven infrastructure lifecycle to the Kaoto VS Code extension: JBang infra CLI wrapper, service/run/stop tasks, provider and managers for auto-refresh and reconciliation, Docker error detection, commands/menus, docs/CHANGELOG/readme updates, and unit tests for Docker error detection. ChangesInfrastructure feature (single DAG)
Sequence Diagram(s)sequenceDiagram
actor User
participant VSCode as VS Code UI
participant Cmd as StartInfraServiceCommand
participant Prov as InfrastructureProvider
participant Mgr as InfrastructureServiceManager
participant JBang as CamelInfraJBang CLI
participant Task as CamelInfraRunJBangTask
User->>VSCode: Trigger "Start Infrastructure Service"
VSCode->>Cmd: execute()
Cmd->>Prov: ensureAvailableServicesLoaded()
Prov->>Mgr: load available services
Mgr->>JBang: list() --json
JBang-->>Mgr: available services
Mgr-->>Prov: available services
Prov-->>Cmd: choices
User->>Cmd: Select service (+ optional port)
Cmd->>Mgr: getCliRunningService / check running
alt external service detected
Cmd->>User: Prompt use existing or stop & restart
opt stop & restart
Cmd->>JBang: stop(service)
JBang-->>Cmd: stop completed
Cmd->>Mgr: unregisterRunningService()
end
end
Cmd->>Task: create run task (start via JBang)
Task->>JBang: start(service, args)
JBang-->>Task: ShellExecution -> run in background
Cmd->>Mgr: registerRunningService(status: starting)
Mgr->>Prov: notify change
Prov->>VSCode: update tree view
Mgr->>JBang: ps() polling until running
JBang-->>Mgr: running details (host/port/url)
Mgr->>Mgr: updateRunningService(status: running, url/port)
Mgr->>Prov: notify change
Prov->>VSCode: show running state
sequenceDiagram
actor User
participant VSCode as VS Code UI
participant Cmd as Stop/Logs/Copy handlers
participant Mgr as InfrastructureServiceManager
participant JBang as CamelInfraJBang CLI
User->>VSCode: Click "Stop" on service
VSCode->>Cmd: stop handler
Cmd->>VSCode: confirmInfrastructureServiceStop dialog
User->>VSCode: Confirm
Cmd->>Mgr: markServiceStopping()
Cmd->>JBang: stop(service)
JBang-->>Cmd: stop completed
Cmd->>Mgr: unregisterRunningService()
Mgr->>VSCode: refresh tree
User->>VSCode: "Show Logs" → Cmd reveals terminal
User->>VSCode: "Copy URL/Port" → Cmd queries Mgr and writes clipboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
djelinek
force-pushed
the
feat/camelInfra
branch
6 times, most recently
from
527883f to
1d20b2e
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d20b2e7be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/views/providers/InfrastructureProvider.ts (1)
121-127: ⚡ Quick winAvoid restarting the auto-refresh interval on every update.
Line 124 calls
startAutoRefresh()every timeupdateAutoRefreshState()runs with running services. SincestartAutoRefresh()clears/recreates the timer, this causes unnecessary timer churn.Suggested refactor
+private autoRefreshActive = false; + private updateAutoRefreshState(): void { const hasRunningServices = this.serviceManager.getRunningServices().size > 0; - if (hasRunningServices) { + if (hasRunningServices && !this.autoRefreshActive) { this.refreshManager.startAutoRefresh(); - } else { + this.autoRefreshActive = true; + } else if (!hasRunningServices && this.autoRefreshActive) { this.refreshManager.stopAutoRefresh(); + this.autoRefreshActive = false; } this.updateContexts(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/providers/InfrastructureProvider.ts` around lines 121 - 127, The method updateAutoRefreshState currently calls refreshManager.startAutoRefresh() every time there are running services, which recreates the timer repeatedly; change it to only start the auto-refresh when it is not already active and only stop it when it is active. Locate updateAutoRefreshState(), use serviceManager.getRunningServices() as the condition, and either call an existing refreshManager.isRunning/isAutoRefreshActive method (or add a small predicate on refreshManager to expose the running state) so you call refreshManager.startAutoRefresh() only if not already running and call refreshManager.stopAutoRefresh() only if it is running.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/StartInfrastructureServiceCommand.ts`:
- Around line 46-64: In execute() and any other methods in
StartInfrastructureServiceCommand (e.g., the code paths around
handleExistingService, configureAndStartService and the blocks at the referenced
regions), ensure this.infrastructureProvider.setStartingService(false) is always
called on every early exit; wrap the main try body in a try/finally or add
explicit resets before each return so that when selectedService is falsy, when
handleExistingService(selectedService.label) returns false, or when external
stop fails, the starting flag is cleared; update the methods named execute,
handleExistingService and any places around the lines noted (including the 86-97
and 183-186 blocks) to guarantee the flag is reset whether by finally or by
placing setStartingService(false) immediately before each return.
- Around line 220-237: The startup error is being reported twice because
configureAndStartService() calls handleError() and rethrows, and execute() (the
caller around runTask.execute()) catches and calls handleError() again; remove
the duplicate handling by deleting the internal handleError() call (or the
rethrow) inside configureAndStartService() so that it simply throws errors and
lets StartInfrastructureServiceCommand.execute() / the runTask.execute() catch
block call handleError() once; update configureAndStartService(),
StartInfrastructureServiceCommand.execute(), and any related catch logic to
ensure only a single handleError(error) path is executed for startup failures.
- Around line 152-173: The current stop flow builds a single command string from
stopTask (from CamelInfraJBang().stop) and calls exec(`${command} ${args}`),
which recombines and re-interprets quoted args; change this to call
execFile(command, argsArray, ...) so arguments remain an array. Locate the stop
handling where stopTask, command and args are derived, convert args back into an
array (preserving stopTask.args items without joining), and replace exec(...)
with execFile(command, argsArray, callback) while preserving the same error
handling (reject on error using stderr || error.message and resolve on success).
In `@src/views/providers/InfrastructureProvider.ts`:
- Around line 26-29: The EventEmitter instance _onDidChangeTreeData isn't being
disposed, causing potential listener leaks; update the dispose() method to call
_onDidChangeTreeData.dispose() (or _onDidChangeTreeData?.dispose()) in addition
to iterating this.disposables and clearing the array so the emitter is cleaned
up when InfrastructureProvider.dispose() runs; locate the dispose() method and
add the emitter disposal alongside the existing disposables handling.
In `@src/views/providers/InfrastructureRefreshManager.ts`:
- Around line 25-31: The interval callback in startAutoRefresh drops the promise
from the async onRefresh, causing unhandled rejections; update startAutoRefresh
so the callback catches errors from this.onRefresh() (e.g., call
this.onRefresh().catch(err => /* handle/log error */)) or use an async IIFE with
try/catch to await this.onRefresh(), and ensure errors are logged/handled
(reference: startAutoRefresh, onRefresh, autoRefreshHandle, refreshInterval).
In `@src/views/providers/InfrastructureServiceManager.ts`:
- Around line 189-198: The code always sets changed = true when updating
runningServices, causing unnecessary refreshes; in the block that merges
currentService with cliService (references: runningServices, currentService,
cliService, and the local changed flag), compute the newService object (keeping
isExternal unchanged and setting status to 'running'), then compare each tracked
field (description, port, url, status) between currentService and newService and
only call this.runningServices.set(name, newService) and set changed = true if
at least one of those fields differs; otherwise leave runningServices and
changed untouched.
- Around line 333-345: The current executeShellExecution function (return type
Promise<string>, using CamelInfraJBang['list'] execution) builds a single
interpolated command string and calls exec; change it to use
child_process.execFile (or spawn) with a command and an array of argument
strings to avoid shell interpolation: construct args as execution.args?.map(a =>
typeof a === 'string' ? a : a.value) ?? [] and pass that array to execFile along
with a callback that rejects on error (including stderr/error.message) and
resolves with stdout || stderr; apply the same change where a command string is
built in ensureAvailableServicesLoaded so both places use execFile/spawn with
discrete arg arrays and proper Promise wrapping and error handling.
---
Nitpick comments:
In `@src/views/providers/InfrastructureProvider.ts`:
- Around line 121-127: The method updateAutoRefreshState currently calls
refreshManager.startAutoRefresh() every time there are running services, which
recreates the timer repeatedly; change it to only start the auto-refresh when it
is not already active and only stop it when it is active. Locate
updateAutoRefreshState(), use serviceManager.getRunningServices() as the
condition, and either call an existing
refreshManager.isRunning/isAutoRefreshActive method (or add a small predicate on
refreshManager to expose the running state) so you call
refreshManager.startAutoRefresh() only if not already running and call
refreshManager.stopAutoRefresh() only if it is running.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7a9a1eee-e917-4bc0-88f6-9af23391b501
📒 Files selected for processing (19)
CHANGELOG.mdREADME.mdit-tests/Util.tspackage.jsonsrc/commands/StartInfrastructureServiceCommand.tssrc/extension/ExtensionContextHandler.tssrc/extension/extension.tssrc/helpers/CamelInfraJBang.tssrc/helpers/DockerErrorDetector.tssrc/helpers/helpers.tssrc/helpers/modals.tssrc/tasks/CamelInfraRunJBangTask.tssrc/tasks/CamelInfraStopJBangTask.tssrc/test/helpers/DockerErrorDetector.test.tssrc/views/infrastructureTreeItems/InfrastructureItem.tssrc/views/providers/DeploymentsProvider.tssrc/views/providers/InfrastructureProvider.tssrc/views/providers/InfrastructureRefreshManager.tssrc/views/providers/InfrastructureServiceManager.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 1041-1043: The app currently only reads the new setting key
"kaoto.views.refresh.interval" and will ignore users' existing
"kaoto.deployments.refresh.interval" values; update the settings-loading logic
(where the code reads the refresh interval) to first check if
"kaoto.views.refresh.interval" is set and, if not, fall back to
"kaoto.deployments.refresh.interval", or perform a one-time migration copying
the old key to the new key on startup; ensure you reference both keys
("kaoto.views.refresh.interval" and "kaoto.deployments.refresh.interval") in the
change and persist the migrated value so users keep their custom interval
instead of reverting to the 5000ms default.
In `@src/commands/StartInfrastructureServiceCommand.ts`:
- Around line 193-253: The port validation in configureAndStartService is too
loose (validateInput only checks digits), so update validateInput in the
showInputBox to allow empty OR a number between 1 and 65535 (reject "0" and
>65535 with a message like "Port must be between 1 and 65535."), and also after
parsing portValue to Number (the port variable) re-check the range before
creating the CamelInfraJBang args and before calling
CamelInfraRunJBangTask.create; if out of range, set
this.infrastructureProvider.setStartingService(false) and show an error/warning
via vscode.window.showErrorMessage or showWarningMessage and return. Ensure
registerRunningService and getServiceTargetUrl are only called when port is
valid or undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3fa32848-a84c-4920-b6e7-548c86d54ab6
📒 Files selected for processing (8)
it-tests/Util.tspackage.jsonsrc/commands/StartInfrastructureServiceCommand.tssrc/helpers/CamelInfraJBang.tssrc/views/infrastructureTreeItems/InfrastructureItem.tssrc/views/providers/InfrastructureProvider.tssrc/views/providers/InfrastructureRefreshManager.tssrc/views/providers/InfrastructureServiceManager.ts
✅ Files skipped from review due to trivial changes (1)
- src/helpers/CamelInfraJBang.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- it-tests/Util.ts
- src/views/providers/InfrastructureProvider.ts
- src/views/providers/InfrastructureRefreshManager.ts
- src/views/providers/InfrastructureServiceManager.ts
djelinek
force-pushed
the
feat/camelInfra
branch
2 times, most recently
from
9d8f90e to
73e1df3
Compare
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
igarashitm
previously approved these changes
May 4, 2026
Member
igarashitm
left a comment
igarashitm
left a comment
There was a problem hiding this comment.
Just one thing noticed, the hover shows http:// for postgres, probably better to be just host:port
Otherwise LGTM 👍
Contributor
Author
thank you! You are right, I have changed URL to suggested |
|
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/views/providers/InfrastructureServiceManager.ts (1)
45-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop rebuilding
ShellExecutionintoexec()strings.Both call sites flatten an argument array into a shell string and pass it to
exec(). That reintroduces shell parsing, so quoted/default args can break on spaces and user-configured infra arguments can become injection primitives. UseexecFile()orspawn()with the original argument array in both places.Suggested fix
-import { exec } from 'child_process'; +import { execFile } from 'child_process'; @@ - const args = execution.args?.map((arg) => (typeof arg === 'string' ? arg : arg.value)).join(' ') ?? ''; + const args = execution.args?.map((arg) => (typeof arg === 'string' ? arg : arg.value)) ?? []; return await new Promise<string>((resolve, reject) => { - exec(`${command} ${args}`, (error, stdout, stderr) => { + execFile(command, args, (error, stdout, stderr) => { if (error) { reject(new Error(stderr || error.message)); return; } @@ - const args = execution.args?.map((arg) => (typeof arg === 'string' ? arg : arg.value)).join(' ') ?? ''; + const args = execution.args?.map((arg) => (typeof arg === 'string' ? arg : arg.value)) ?? []; return await new Promise<string>((resolve, reject) => { - exec(`${command} ${args}`, (error, stdout, stderr) => { + execFile(command, args, (error, stdout, stderr) => { if (error) { reject(new Error(stderr || error.message)); return; }#!/bin/bash # Verify that InfrastructureServiceManager still reconstructs ShellExecution into exec strings. rg -n -C2 'exec\(`\$\{command\}\s\$\{args\}`' src/views/providers/InfrastructureServiceManager.ts rg -n -C2 '\.map\(\(arg\).*\.join\('\'' '\''\)' src/views/providers/InfrastructureServiceManager.tsAlso applies to: 333-345
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/providers/InfrastructureServiceManager.ts` around lines 45 - 55, The code reconstructs ShellExecution into a single shell string (see CamelInfraJBang().list() usage and the variables execution, command, args) and passes it to exec(), which reintroduces shell parsing risks; change this to call execFile() or spawn() directly with the original argument array (build args as an array from execution.args preserving non-string .value entries) and invoke execFile(command, argsArray, ...) or spawn(command, argsArray, {stdio:'pipe'}) so you avoid joining into a single string and keep error/stdout handling equivalent; apply the same change to the other call-site that similarly maps execution.args into a joined string.src/commands/StartInfrastructureServiceCommand.ts (1)
163-175:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
execFile()for the external stop flow.This stop path joins
ShellExecution.argsinto a single string and hands it toexec(). That makes quoting platform-dependent again and lets shell metacharacters in service names or configured args change the command that actually runs. Keep the args as an array and callexecFile().Suggested fix
-import { exec } from 'child_process'; +import { execFile } from 'child_process'; @@ - const args = stopTask.args?.map((arg) => (typeof arg === 'string' ? arg : arg.value)).join(' ') ?? ''; + const args = stopTask.args?.map((arg) => (typeof arg === 'string' ? arg : arg.value)) ?? []; await new Promise<void>((resolve, reject) => { - exec(`${command} ${args}`, (error, stdout, stderr) => { + execFile(command, args, (error, stdout, stderr) => { if (error) { reject(new Error(stderr || error.message)); return; }#!/bin/bash # Verify that StartInfrastructureServiceCommand still reconstructs a ShellExecution into an exec string. rg -n -C2 'exec\(`\$\{command\}\s\$\{args\}`' src/commands/StartInfrastructureServiceCommand.ts rg -n -C2 '\.map\(\(arg\).*\.join\('\'' '\''\)\s*\?\?' src/commands/StartInfrastructureServiceCommand.ts🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/StartInfrastructureServiceCommand.ts` around lines 163 - 175, The current stop flow builds a single shell string from CamelInfraJBang().stop(serviceName) by joining stopTask.args into args and calling exec(`${command} ${args}`), which is unsafe; change it to call execFile with an args array instead: keep the resolution for command (const command = typeof stopTask.command === 'string' ? stopTask.command : (stopTask.command?.value ?? 'jbang')) but build argsArray = stopTask.args?.map(arg => typeof arg === 'string' ? arg : arg.value) ?? [] (do not join), then call execFile(command, argsArray, (error, stdout, stderr) => { ... }) and preserve the existing Promise resolve/reject behavior (reject with stderr || error.message). Ensure you import/use child_process.execFile if not already used and remove the string-joining/exec usage around start/stopTask.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/commands/StartInfrastructureServiceCommand.ts`:
- Around line 163-175: The current stop flow builds a single shell string from
CamelInfraJBang().stop(serviceName) by joining stopTask.args into args and
calling exec(`${command} ${args}`), which is unsafe; change it to call execFile
with an args array instead: keep the resolution for command (const command =
typeof stopTask.command === 'string' ? stopTask.command :
(stopTask.command?.value ?? 'jbang')) but build argsArray =
stopTask.args?.map(arg => typeof arg === 'string' ? arg : arg.value) ?? [] (do
not join), then call execFile(command, argsArray, (error, stdout, stderr) => {
... }) and preserve the existing Promise resolve/reject behavior (reject with
stderr || error.message). Ensure you import/use child_process.execFile if not
already used and remove the string-joining/exec usage around start/stopTask.
In `@src/views/providers/InfrastructureServiceManager.ts`:
- Around line 45-55: The code reconstructs ShellExecution into a single shell
string (see CamelInfraJBang().list() usage and the variables execution, command,
args) and passes it to exec(), which reintroduces shell parsing risks; change
this to call execFile() or spawn() directly with the original argument array
(build args as an array from execution.args preserving non-string .value
entries) and invoke execFile(command, argsArray, ...) or spawn(command,
argsArray, {stdio:'pipe'}) so you avoid joining into a single string and keep
error/stdout handling equivalent; apply the same change to the other call-site
that similarly maps execution.args into a joined string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09d5c631-68f1-49cb-acdf-43b8051915e8
📒 Files selected for processing (19)
CHANGELOG.mdREADME.mdit-tests/Util.tspackage.jsonsrc/commands/StartInfrastructureServiceCommand.tssrc/extension/ExtensionContextHandler.tssrc/extension/extension.tssrc/helpers/CamelInfraJBang.tssrc/helpers/DockerErrorDetector.tssrc/helpers/helpers.tssrc/helpers/modals.tssrc/tasks/CamelInfraRunJBangTask.tssrc/tasks/CamelInfraStopJBangTask.tssrc/test/helpers/DockerErrorDetector.test.tssrc/views/infrastructureTreeItems/InfrastructureItem.tssrc/views/providers/DeploymentsProvider.tssrc/views/providers/InfrastructureProvider.tssrc/views/providers/InfrastructureRefreshManager.tssrc/views/providers/InfrastructureServiceManager.ts
✅ Files skipped from review due to trivial changes (10)
- src/helpers/helpers.ts
- src/tasks/CamelInfraRunJBangTask.ts
- README.md
- src/helpers/DockerErrorDetector.ts
- src/test/helpers/DockerErrorDetector.test.ts
- src/views/providers/InfrastructureRefreshManager.ts
- src/tasks/CamelInfraStopJBangTask.ts
- CHANGELOG.md
- src/views/infrastructureTreeItems/InfrastructureItem.ts
- src/views/providers/InfrastructureProvider.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- it-tests/Util.ts
- src/views/providers/DeploymentsProvider.ts
- src/helpers/modals.ts
igarashitm
approved these changes
May 5, 2026
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
2 participants
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.
Screen.Recording.2026-04-28.at.17.16.30.mov
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Documentation
Tests