Skip to content

fix: handle chunked CLI startup port output in VS Code extension - #7650

Closed
LuisCharro wants to merge 3 commits into
Kilo-Org:mainfrom
LuisCharro:fix/vscode-server-startup-port
Closed

fix: handle chunked CLI startup port output in VS Code extension#7650
LuisCharro wants to merge 3 commits into
Kilo-Org:mainfrom
LuisCharro:fix/vscode-server-startup-port

Conversation

@LuisCharro

Copy link
Copy Markdown

Fixes #7649

Summary

  • buffer CLI stdout while waiting for the startup port announcement
  • parse the port across chunk boundaries instead of treating each chunk as standalone
  • add unit coverage for split startup output and split port digits

Testing

  • not run locally in this shell (bun is unavailable here)

* Does NOT kill the server — that's the connection service's job.
*/
dispose(): void {
this.disposed = true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: dispose() can still leak connection listeners when initialization is mid-flight

If a tab/panel is closed while initializeConnection() is still awaiting this.connectionService.connect(...), dispose() runs before unsubscribeEvent / unsubscribeState have been assigned. The remainder of doInitializeConnection() will then register fresh listeners against KiloConnectionService, and there is no later cleanup path for this disposed provider. Add a this.disposed/this.webview guard after the awaited connection step before subscribing so closed webviews do not stay registered forever.

@kilo-code-bot

kilo-code-bot Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

WARNING

File Line Issue
packages/kilo-vscode/src/KiloProvider.ts 2636 dispose() does not stop an in-flight initializeConnection() from registering new listeners after the provider has already been torn down.
Other Observations (not in diff)

No additional issues found outside the diff.

Files Reviewed (6 files)
  • packages/kilo-vscode/src/KiloProvider.ts - 1 issue
  • packages/kilo-vscode/src/__tests__/KiloProvider.spec.ts - 0 issues
  • packages/kilo-vscode/src/services/cli-backend/server-manager.ts - 0 issues
  • packages/kilo-vscode/src/services/cli-backend/server-utils.ts - 0 issues
  • packages/kilo-vscode/tests/unit/server-manager-utils.test.ts - 0 issues
  • packages/kilo-vscode/webview-ui/src/context/session.tsx - 0 issues

Reviewed by gpt-5.4-20260305 · 731,627 tokens

@johnnyeric

johnnyeric commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Closing as stale, as the review feedback was not addressed. Feel free to reopen or submit a follow-up PR with those changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VS Code extension can miss CLI startup port when stdout splits across chunks

2 participants