feat: send editor context (visible files, open tabs, shell, timezone) to CLI backend#322
feat: send editor context (visible files, open tabs, shell, timezone) to CLI backend#322markijbema wants to merge 2 commits intodevfrom
Conversation
![kiloconnect[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| return undefined | ||
| } | ||
| const relative = path.relative(workspaceDir, fsPath) | ||
| if (relative.startsWith("..")) { |
There was a problem hiding this comment.
WARNING: toRelative() may mis-handle files on Windows when paths are on different drive letters
path.relative(workspaceDir, fsPath) can return an absolute path (e.g. D:\...) when workspaceDir is on C: and the file is on D:. In that case relative.startsWith('..') is false, so this code would incorrectly treat an out-of-workspace file as workspace-relative and send an absolute path to the backend. Consider also rejecting path.isAbsolute(relative) (and/or using path.resolve(workspaceDir, relative) + prefix check) before returning the value.
|
|
||
| parts.push({ type: "text", text }) | ||
|
|
||
| const editorContext = this.gatherEditorContext() |
There was a problem hiding this comment.
SUGGESTION: Avoid sending an empty editorContext object
gatherEditorContext() can return {} when no fields are available, but options?.editorContext is truthy even when empty so the payload will always include editorContext: {}. If the backend schema expects editorContext to be omitted when empty, this could cause validation/behavior differences. Consider only setting body.editorContext when it has at least one key.
Code Review SummaryStatus: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (5 files)
|
There was a problem hiding this comment.
Pull request overview
Adds support for sending VS Code “editor context” metadata alongside chat messages so the CLI backend can build more accurate environment/system prompts.
Changes:
- Introduces an
EditorContexttype for editor-derived context fields (visible files, open tabs, active file, shell, timezone). - Extends the CLI backend HTTP client
sendMessage()payload to optionally includeeditorContext. - Implements
gatherEditorContext()inKiloProviderto collect and forward editor context; adds a design doc describing phased context expansion.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/kilo-vscode/src/services/cli-backend/types.ts | Adds the EditorContext interface for message-adjacent editor context data. |
| packages/kilo-vscode/src/services/cli-backend/http-client.ts | Extends sendMessage() to include an editorContext field in the POST payload. |
| packages/kilo-vscode/src/KiloProvider.ts | Collects editor state (visible files/tabs/active file/shell/timezone) and passes it to the backend on message send. |
| packages/kilo-vscode/docs/vscode-context-plan.md | Adds a phased design plan and proposed protocol shape for VS Code context. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** VS Code editor context sent alongside messages to the CLI backend */ | ||
| export interface EditorContext { | ||
| /** Workspace-relative paths of currently visible editors */ | ||
| visibleFiles?: string[] | ||
| /** Workspace-relative paths of open tabs */ | ||
| openTabs?: string[] | ||
| /** Workspace-relative path of the active editor file */ | ||
| activeFile?: string | ||
| /** User's default shell (from vscode.env.shell) */ | ||
| shell?: string | ||
| /** User's timezone (e.g. "Europe/Amsterdam") */ | ||
| timezone?: string | ||
| } |
There was a problem hiding this comment.
PR description says the EditorContext shape includes cwd, but this interface does not define it. Either add a cwd?: string field (and populate it) or update the PR description/docs to reflect that workspace cwd is provided only via the existing x-opencode-directory header.
| // NEW: Editor context from VS Code | ||
| editorContext?: { | ||
| visibleFiles?: string[] // workspace-relative paths | ||
| openTabs?: string[] // workspace-relative paths | ||
| activeFile?: string // workspace-relative path (replaces file:// part) | ||
| shell?: string // user's default shell | ||
| timezone?: string // user's timezone | ||
| diagnostics?: DiagnosticInfo[] // workspace diagnostics | ||
| } |
There was a problem hiding this comment.
The protocol example says activeFile “replaces file:// part”, but this PR explicitly preserves the existing file:// part injection for backward compatibility and also sends editorContext.activeFile. This doc section should be updated to avoid implying the file-part will be removed/absent when activeFile is present (or explicitly mark that as a later migration step).
| if (relative.startsWith("..")) { | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
toRelative() uses path.relative(workspaceDir, fsPath) and only checks relative.startsWith(".."). On Windows, if fsPath is on a different drive than workspaceDir, path.relative can return an absolute path (e.g. D:\...), which will not start with .. and will incorrectly be treated as in-workspace. Also, startsWith("..") will incorrectly exclude legitimate in-workspace paths like ..foo. Consider using vscode.workspace.getWorkspaceFolder(uri) / vscode.workspace.asRelativePath(uri, false) to gate inclusion, or at least additionally reject path.isAbsolute(relative) and check relative === ".." || relative.startsWith(..${path.sep}).
| if (relative.startsWith("..")) { | |
| return undefined | |
| } | |
| // Exclude paths that are not within the workspace: | |
| // - On Windows, path.relative can return an absolute path for different drives. | |
| // - Only treat ".." or ".." + path.sep + ... as outside the workspace, not names like "..foo". | |
| if (path.isAbsolute(relative)) { | |
| return undefined | |
| } | |
| if (relative === ".." || relative.startsWith(`..${path.sep}`)) { | |
| return undefined | |
| } |
| } | ||
| } | ||
| } | ||
| const openTabs = [...openTabSet] |
There was a problem hiding this comment.
openTabs is currently unbounded (const openTabs = [...openTabSet]). In large workspaces/users with many tabs, this can noticeably increase the per-message payload size sent to the CLI backend. The design doc in this PR mentions a default cap (e.g. maxOpenTabsContext); consider enforcing a reasonable max (and possibly a stable ordering) before sending.
| const openTabs = [...openTabSet] | |
| const maxOpenTabsContext = 50 | |
| const openTabs = [...openTabSet].sort().slice(0, maxOpenTabsContext) |
| envLines.push(` Active file: ${editorContext.activeFile}`) | ||
| } | ||
| if (editorContext?.visibleFiles?.length) { | ||
| envLines.push(` Visible files: ${editorContext.visibleFiles.join(", ")}`) |
There was a problem hiding this comment.
WARNING: editorContext values should be sanitized/capped before embedding in the system prompt
visibleFiles/openTabs are joined directly into the <env> block. On POSIX, filenames can contain newlines (and other control chars), which can break the tag structure and create prompt-injection-y formatting. Independently, large tab sets can bloat the system prompt and increase token usage. Consider normalizing these strings (e.g., replace \r/\n) and applying a max count / max character budget before join(", ").
1 participant
Summary
Send editor context information (visible files, open tabs, active file, shell, timezone) alongside chat messages to the CLI backend.
Changes
EditorContextinterface added tosrc/services/cli-backend/types.ts— defines the shape of editor context data (visible files, open tabs, active file, shell, timezone, cwd)sendMessage()inhttp-client.tsextended to accept and forward an optionaleditorContextparameter to the CLI backendgatherEditorContext()inKiloProvider.ts— new method that collects:Intl.DateTimeFormat)file://URI injection into the message body is preserved for backward compatibilitydocs/vscode-context-plan.mddesign documentContext
Implements the extension-side pieces of Phase 1 + Phase 2 from
docs/vscode-context-plan.md. The CLI backend side (consuming the context) is tracked separately.