diff --git a/.changeset/agent-manager-browser-context.md b/.changeset/agent-manager-browser-context.md index 9236086d37..2362ea4ca8 100644 --- a/.changeset/agent-manager-browser-context.md +++ b/.changeset/agent-manager-browser-context.md @@ -3,4 +3,4 @@ "kilo-code": minor --- -Open and interact with local applications in Agent Manager through a session-scoped browser. +Inspect local applications in Agent Manager with embedded developer tools and review-style element feedback for precise frontend changes. diff --git a/packages/kilo-vscode/src/KiloProvider.ts b/packages/kilo-vscode/src/KiloProvider.ts index c5925b92ed..4402de6e19 100644 --- a/packages/kilo-vscode/src/KiloProvider.ts +++ b/packages/kilo-vscode/src/KiloProvider.ts @@ -142,7 +142,8 @@ import { } from "./kilo-provider/handlers/question" import { fetchAndSendPendingSuggestions } from "./kilo-provider/handlers/suggestion" import { nativeTitle } from "./kilo-provider/native-tab-title" -import { parseReview, reviewMetadata, type ReviewMessageData } from "./shared/review-comments" +import { type ReviewMessageData } from "./shared/review-comments" +import { feedbackMetadata, parseFeedback, type BrowserFeedbackData } from "./shared/browser-feedback" import { completesWithoutStatus } from "./kilo-provider/command-completion" import { KiloProviderMemory } from "./kilo-provider/memory" @@ -202,6 +203,29 @@ type TypedWebviewMessage = { type: string value?: unknown } + +type WebviewMessage = Parameters[0]>[0] + +function feedbackMessage(message: { text: string; review?: unknown; browserFeedback?: unknown }) { + return parseFeedback({ review: message.review, browserFeedback: message.browserFeedback }, message.text) +} + +type SendWebviewMessage = { + type: "sendMessage" + text: string + messageID?: unknown + sessionID?: string + draftID?: unknown + providerID?: string + modelID?: string + agent?: string + variant?: string + files?: unknown + review?: unknown + browserFeedback?: unknown + agentManagerContext?: unknown + contextDirectory?: unknown +} type SandboxSupportClient = { support: ( parameters: { directory?: string }, @@ -1096,21 +1120,7 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper this.readyResolvers.splice(0).forEach((r) => r()) break case "sendMessage": { - const msg = message as typeof message & ContextMessage - await this.handleSendMessage( - message.text, - typeof message.messageID === "string" ? message.messageID : undefined, - message.sessionID, - typeof message.draftID === "string" ? message.draftID : undefined, - message.providerID, - message.modelID, - message.agent, - message.variant, - parseMessageFiles(message.files), - parseReview(message.review, message.text), - typeof message.agentManagerContext === "string" ? message.agentManagerContext : undefined, - typeof msg.contextDirectory === "string" ? msg.contextDirectory : undefined, - ) + await this.sendWebviewMessage(message as SendWebviewMessage) break } case "sendCommand": { @@ -1462,6 +1472,7 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper break case "importAndSend": { const files = parseMessageFiles(message.files) + const feedback = feedbackMessage(message) void handleImportAndSend( this.cloudSessionCtx, message.cloudSessionId, @@ -1472,9 +1483,10 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper message.agent, message.variant, files, - parseReview(message.review, message.text), + feedback?.review, typeof message.command === "string" ? message.command : undefined, typeof message.commandArgs === "string" ? message.commandArgs : undefined, + feedback?.browserFeedback, ) break } @@ -1552,6 +1564,25 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper this.webviewMessageDisposable = watchWorkStyleConfig((msg) => this.postMessage(msg), this.webviewMessageDisposable) } + private async sendWebviewMessage(message: SendWebviewMessage): Promise { + const feedback = feedbackMessage(message) + await this.handleSendMessage( + message.text, + typeof message.messageID === "string" ? message.messageID : undefined, + message.sessionID, + typeof message.draftID === "string" ? message.draftID : undefined, + message.providerID, + message.modelID, + message.agent, + message.variant, + parseMessageFiles(message.files), + feedback?.review, + typeof message.agentManagerContext === "string" ? message.agentManagerContext : undefined, + typeof message.contextDirectory === "string" ? message.contextDirectory : undefined, + feedback?.browserFeedback, + ) + } + private async handleProfileDataMessage(message: TypedWebviewMessage): Promise { if (message.type === "refreshProfile") { await handleRefreshProfile(this.authCtx) @@ -3987,6 +4018,7 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper review?: ReviewMessageData, context?: string, contextDirectory?: string, + browserFeedback?: BrowserFeedbackData, ): Promise { if (!this.client) { this.postMessage({ @@ -3998,6 +4030,7 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper messageID, files, review, + browserFeedback, }) return } @@ -4019,7 +4052,7 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper parts.push({ type: "file", mime: f.mime, url: f.url, filename: f.filename, source: f.source }) } } - parts.push({ type: "text", text, metadata: review ? reviewMetadata(review) : undefined }) + parts.push({ type: "text", text, metadata: feedbackMetadata(review, browserFeedback) }) const editorContext = await this.gatherEditorContext(dir) if (draftID && this.closedDrafts.delete(draftID)) { @@ -4061,6 +4094,7 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper messageID, files, review, + browserFeedback, }) } } diff --git a/packages/kilo-vscode/src/agent-manager/browser-message.ts b/packages/kilo-vscode/src/agent-manager/browser-message.ts index 930cb43850..282ae0d187 100644 --- a/packages/kilo-vscode/src/agent-manager/browser-message.ts +++ b/packages/kilo-vscode/src/agent-manager/browser-message.ts @@ -114,7 +114,7 @@ function action( return true } void deps.browser - .inspect(m.sessionId, scope.project, point) + .inspect(m.sessionId, scope.project, point, m.hover !== true) .then((inspection) => deps.post({ type: "agentManager.browserInspection", diff --git a/packages/kilo-vscode/src/agent-manager/types.ts b/packages/kilo-vscode/src/agent-manager/types.ts index 4f78df5de4..c6ad2c5487 100644 --- a/packages/kilo-vscode/src/agent-manager/types.ts +++ b/packages/kilo-vscode/src/agent-manager/types.ts @@ -20,6 +20,7 @@ import type { ProjectSnapshot } from "./project/contexts" import type { SidebarTarget } from "./project/route" import type { TerminalDestination } from "./terminal-destination" import type { ScriptTerminalView } from "./ScriptTerminalManager" +import type { BrowserFeedbackData } from "../shared/browser-feedback" export type { TerminalFont } export type { ProjectSnapshot } @@ -314,6 +315,7 @@ interface SendInitialMessage { agent?: string variant?: string files?: Array<{ mime: string; url: string }> + browserFeedback?: BrowserFeedbackData } interface BranchesMessage { @@ -469,7 +471,12 @@ interface BrowserInspectionMessage { sessionId: string url?: string title?: string - element?: BrowserElement + element?: BrowserElement & { + hierarchy?: string[] + html?: string + styles?: { color?: string; backgroundColor?: string } + source?: { file: string; line?: number; column?: number } + } logs: string[] hover?: boolean } @@ -947,6 +954,7 @@ interface SendMessageIn { files?: Array<{ mime: string; url: string; filename?: string; source?: FileSourceIn }> agentManagerContext?: string contextDirectory?: string + browserFeedback?: BrowserFeedbackData } interface SendCommandIn { diff --git a/packages/kilo-vscode/src/kilo-provider/handlers/cloud-session.ts b/packages/kilo-vscode/src/kilo-provider/handlers/cloud-session.ts index e978f40576..f64652dfe4 100644 --- a/packages/kilo-vscode/src/kilo-provider/handlers/cloud-session.ts +++ b/packages/kilo-vscode/src/kilo-provider/handlers/cloud-session.ts @@ -9,7 +9,8 @@ import type { KiloClient, Session, TextPartInput, FilePartInput } from "@kilocod import type { CloudSessionData, EditorContext } from "../../services/cli-backend/types" import { getErrorMessage, sessionToWebview, mapCloudSessionMessageToWebviewMessage } from "../../kilo-provider-utils" import type { MessageFile } from "../message-files" -import { reviewMetadata, type ReviewMessageData } from "../../shared/review-comments" +import { type ReviewMessageData } from "../../shared/review-comments" +import { feedbackMetadata, type BrowserFeedbackData } from "../../shared/browser-feedback" import { completesWithoutStatus } from "../command-completion" const TIMEOUT = 30_000 @@ -124,6 +125,7 @@ export async function handleImportAndSend( review?: ReviewMessageData, command?: string, commandArgs?: string, + browserFeedback?: BrowserFeedbackData, ): Promise { if (!ctx.client) { ctx.postMessage({ @@ -216,7 +218,7 @@ export async function handleImportAndSend( parts.push({ type: "file", mime: f.mime, url: f.url, filename: f.filename, source: f.source }) } } - parts.push({ type: "text", text, metadata: review ? reviewMetadata(review) : undefined }) + parts.push({ type: "text", text, metadata: feedbackMetadata(review, browserFeedback) }) const editorContext = await ctx.gatherEditorContext() await client.session.promptAsync( @@ -247,6 +249,7 @@ export async function handleImportAndSend( messageID, files, review: command ? undefined : review, + browserFeedback: command ? undefined : browserFeedback, }) } } diff --git a/packages/kilo-vscode/src/services/browser-automation/browser-broker.ts b/packages/kilo-vscode/src/services/browser-automation/browser-broker.ts index ba10e16cab..42d4cf2800 100644 --- a/packages/kilo-vscode/src/services/browser-automation/browser-broker.ts +++ b/packages/kilo-vscode/src/services/browser-automation/browser-broker.ts @@ -1,9 +1,11 @@ import { randomBytes, randomUUID, timingSafeEqual } from "node:crypto" import { createServer, type IncomingMessage, type Server, type ServerResponse } from "node:http" +import type { Socket } from "node:net" import { URL } from "node:url" import { stripVTControlCharacters } from "node:util" import { chromium, type BrowserContext, type Page } from "playwright-core" import { BrowserDevtools } from "./browser-devtools" +import { capture as element, locate } from "./browser-element" export type BrowserStatus = "starting" | "ready" | "loading" | "error" | "closed" @@ -37,6 +39,10 @@ export interface BrowserElement { text?: string selector?: string rect?: { x: number; y: number; width: number; height: number } + hierarchy?: string[] + html?: string + styles?: { color?: string; backgroundColor?: string } + source?: { file: string; line?: number; column?: number } } export interface BrowserInspection { @@ -183,6 +189,7 @@ export class BrowserBroker { private readonly token = randomBytes(32).toString("hex") private owner: ((route: BrowserRoute) => BrowserRoute | undefined) | undefined private server: Server | undefined + private readonly sockets = new Set() private port: number | undefined private debugging: number | undefined private tools: BrowserDevtools | undefined @@ -201,6 +208,10 @@ export class BrowserBroker { this.server = createServer((req, res) => { void this.handle(req, res) }) + this.server.on("connection", (socket) => { + this.sockets.add(socket) + socket.once("close", () => this.sockets.delete(socket)) + }) this.server.once("error", reject) this.server.listen(0, "127.0.0.1", () => { const server = this.server @@ -359,47 +370,19 @@ export class BrowserBroker { sessionId: string, projectId: string | undefined, position: { x: number; y: number; width: number; height: number }, + detail = true, ): Promise { return this.serial(this.key(sessionId, projectId), async () => { this.available() const entry = this.require(sessionId, undefined, projectId) await this.point(entry, position) - const element = await entry.page.evaluate(({ x, y }) => { - const root = document.documentElement - if (root.scrollHeight > innerHeight && root.clientWidth === innerWidth) { - root.style.setProperty("scrollbar-gutter", "stable") - } - const node = document.elementFromPoint(x * innerWidth, y * innerHeight) - if (!(node instanceof Element)) return undefined - const tag = node.tagName.toLowerCase() - const id = node.id || undefined - const classes = node.getAttribute("class")?.trim().slice(0, 180) || undefined - const text = (node.textContent ?? "").replace(/\s+/g, " ").trim().slice(0, 400) || undefined - const selector = id ? `#${CSS.escape(id)}` : tag - const rect = node.getBoundingClientRect() - const left = Math.max(0, Math.min(innerWidth, rect.left)) - const top = Math.max(0, Math.min(innerHeight, rect.top)) - const right = Math.max(left, Math.min(innerWidth, rect.right)) - const bottom = Math.max(top, Math.min(innerHeight, rect.bottom)) - return { - tag, - id, - classes, - text, - selector, - rect: { - x: left / innerWidth, - y: top / innerHeight, - width: (right - left) / innerWidth, - height: (bottom - top) / innerHeight, - }, - } - }, position) + const selected: BrowserElement | undefined = await entry.page.evaluate(element, { ...position, detail }) + if (selected?.source) selected.source = await locate(entry.route.directory, selected.source) await this.update(entry) return { url: entry.state.url, title: entry.state.title, - element, + element: selected, logs: [...(entry.state.logs ?? [])], } }) @@ -469,6 +452,9 @@ export class BrowserBroker { await new Promise((resolve) => { if (!this.server) return resolve() this.server.close(() => resolve()) + for (const socket of this.sockets) socket.destroy() + this.sockets.clear() + if (!this.server.listening) resolve() }) this.server = undefined this.port = undefined @@ -749,9 +735,10 @@ export class BrowserBroker { private authorized(req: IncomingMessage): boolean { const value = req.headers.authorization - const expected = `Bearer ${this.token}` - if (typeof value !== "string" || value.length !== expected.length) return false - return timingSafeEqual(Buffer.from(value), Buffer.from(expected)) + if (typeof value !== "string") return false + const actual = Buffer.from(value) + const expected = Buffer.from(`Bearer ${this.token}`) + return actual.byteLength === expected.byteLength && timingSafeEqual(actual, expected) } private status(req: IncomingMessage, res: ServerResponse, route: URL): boolean { diff --git a/packages/kilo-vscode/src/services/browser-automation/browser-devtools.ts b/packages/kilo-vscode/src/services/browser-automation/browser-devtools.ts index 03a673666f..83b08545c3 100644 --- a/packages/kilo-vscode/src/services/browser-automation/browser-devtools.ts +++ b/packages/kilo-vscode/src/services/browser-automation/browser-devtools.ts @@ -1,5 +1,5 @@ import { randomBytes, timingSafeEqual } from "node:crypto" -import type { IncomingMessage, Server, ServerResponse } from "node:http" +import { request, type IncomingHttpHeaders, type IncomingMessage, type Server, type ServerResponse } from "node:http" import type { Duplex } from "node:stream" import { URL } from "node:url" import WebSocket, { WebSocketServer, type RawData } from "ws" @@ -22,7 +22,9 @@ const LIFETIME = 15 * 60 * 1000 const PAYLOAD = 16 * 1024 * 1024 function equal(left: string, right: string): boolean { - return left.length === right.length && timingSafeEqual(Buffer.from(left), Buffer.from(right)) + const actual = Buffer.from(left) + const expected = Buffer.from(right) + return actual.byteLength === expected.byteLength && timingSafeEqual(actual, expected) } function reject(socket: Duplex, status: number, reason: string): void { @@ -30,6 +32,30 @@ function reject(socket: Duplex, status: number, reason: string): void { socket.destroy() } +function resource(port: number, path: string): Promise<{ status: number; headers: IncomingHttpHeaders; body: Buffer }> { + return new Promise((resolve, reject) => { + const req = request({ hostname: "127.0.0.1", port, path, method: "GET" }, (response) => { + const chunks: Buffer[] = [] + let size = 0 + response.on("data", (chunk: Buffer) => { + size += chunk.byteLength + if (size > PAYLOAD) { + response.destroy(new Error("Browser developer tools asset exceeds the size limit")) + return + } + chunks.push(chunk) + }) + response.once("error", reject) + response.once("end", () => + resolve({ status: response.statusCode ?? 502, headers: response.headers, body: Buffer.concat(chunks) }), + ) + }) + req.setTimeout(10_000, () => req.destroy(new Error("Browser developer tools asset request timed out"))) + req.once("error", reject) + req.end() + }) +} + function inspect(data: RawData): Message | undefined { const value = Array.isArray(data) ? Buffer.concat(data).toString("utf8") @@ -104,13 +130,11 @@ export class BrowserDevtools { res.end(script) return true } - const url = new URL(`/devtools/${scope.path}`, `http://127.0.0.1:${scope.target.port}`) - url.search = route.search try { - const response = await fetch(url, { redirect: "manual" }) - const data = Buffer.from(await response.arrayBuffer()) + const response = await resource(scope.target.port, `/devtools/${scope.path}${route.search}`) + const data = response.body const body = - scope.path === "inspector.html" && response.ok + scope.path === "inspector.html" && response.status === 200 ? Buffer.from( data .toString("utf8") @@ -126,8 +150,8 @@ export class BrowserDevtools { "referrer-policy": "no-referrer", } for (const name of ["content-type", "content-security-policy"]) { - const value = response.headers.get(name) - if (value) headers[name] = value + const value = response.headers[name] + if (typeof value === "string") headers[name] = value } res.writeHead(response.status, headers) res.end(body) diff --git a/packages/kilo-vscode/src/services/browser-automation/browser-element.ts b/packages/kilo-vscode/src/services/browser-automation/browser-element.ts new file mode 100644 index 0000000000..4dadcc25df --- /dev/null +++ b/packages/kilo-vscode/src/services/browser-automation/browser-element.ts @@ -0,0 +1,208 @@ +import { readFile, realpath, stat } from "node:fs/promises" +import path from "node:path" + +function integer(value: number | undefined) { + return typeof value === "number" && Number.isInteger(value) && value > 0 ? value : undefined +} + +async function coordinates(file: string, source: { line?: number; column?: number }, size: number) { + const line = integer(source.line) + const text = line && size <= 1_048_576 ? await readFile(file, "utf8").catch(() => undefined) : undefined + const lines = text?.split("\n") + const valid = line && lines && line <= lines.length ? line : undefined + const column = integer(source.column) + return { + line: valid, + column: valid && column && column <= (lines?.[valid - 1]?.length ?? 0) + 1 ? column : undefined, + } +} + +export async function locate(directory: string, source?: { file: string; line?: number; column?: number }) { + if (!source?.file || source.file.includes("\0") || /^[a-z][a-z0-9+.-]*:\/\//i.test(source.file)) return undefined + if (!/\.(?:[cm]?[jt]sx?|vue|svelte|html?|css|scss|sass|less)$/i.test(source.file)) return undefined + const candidate = path.resolve(directory, source.file) + const [root, file] = await Promise.all([ + realpath(directory).catch(() => undefined), + realpath(candidate).catch(() => undefined), + ]) + if (!root || !file) return undefined + const relative = path.relative(root, file) + if (!relative || path.isAbsolute(relative) || relative.split(path.sep).includes("..")) return undefined + if (relative.split(path.sep).some((part) => part === "node_modules" || part === ".git")) return undefined + const info = await stat(file).catch(() => undefined) + if (!info?.isFile()) return undefined + return { file: relative.split(path.sep).join("/"), ...(await coordinates(file, source, info.size)) } +} + +export function capture(position: { x: number; y: number; detail?: boolean }) { + const root = document.documentElement + if (root.scrollHeight > innerHeight && root.clientWidth === innerWidth) { + root.style.setProperty("scrollbar-gutter", "stable") + } + const node = document.elementFromPoint(position.x * innerWidth, position.y * innerHeight) + if (!(node instanceof Element)) return undefined + + const label = (element: Element) => { + const tag = element.tagName.toLowerCase() + if (element.id) return `${tag}#${CSS.escape(element.id.slice(0, 120))}` + const classes = [...element.classList].slice(0, 3).map((item) => `.${CSS.escape(item.slice(0, 60))}`) + return `${tag}${classes.join("")}` + } + const locator = (element: Element) => { + if (element.id && element.id.length <= 180) { + const selector = `#${CSS.escape(element.id)}` + if (document.querySelectorAll(selector).length === 1) return selector + } + for (const name of ["data-testid", "data-test", "data-cy", "aria-label"]) { + const value = element.getAttribute(name) + if (!value || value.length > 160) continue + const selector = `${element.tagName.toLowerCase()}[${name}="${CSS.escape(value)}"]` + if (document.querySelectorAll(selector).length === 1) return selector + } + return undefined + } + const selector = () => { + const direct = locator(node) + if (direct) return direct + const path: string[] = [] + let current: Element | null = node + while (current && path.length < 128) { + const anchor = locator(current) + if (anchor) { + path.unshift(anchor) + const value = path.join(" > ") + return value.length <= 2048 && document.querySelectorAll(value).length === 1 ? value : undefined + } + const tag = current.tagName.toLowerCase() + const siblings: Element[] = current.parentElement + ? [...current.parentElement.children].filter((item) => item.tagName === current?.tagName) + : [] + path.unshift(`${tag}${siblings.length > 1 ? `:nth-of-type(${siblings.indexOf(current) + 1})` : ""}`) + const value = path.join(" > ") + if (value.length > 2048) return undefined + if (document.querySelectorAll(value).length === 1) return value + current = current.parentElement + } + return undefined + } + const identity = selector() + if (!identity) return undefined + const rect = node.getBoundingClientRect() + const left = Math.max(0, Math.min(innerWidth, rect.left)) + const top = Math.max(0, Math.min(innerHeight, rect.top)) + const right = Math.max(left, Math.min(innerWidth, rect.right)) + const bottom = Math.max(top, Math.min(innerHeight, rect.bottom)) + const result = { + tag: node.tagName.toLowerCase(), + id: node.id.slice(0, 120) || undefined, + classes: node.getAttribute("class")?.trim().slice(0, 180) || undefined, + selector: identity, + rect: { + x: left / innerWidth, + y: top / innerHeight, + width: (right - left) / innerWidth, + height: (bottom - top) / innerHeight, + }, + } + if (position.detail === false) return result + + const ancestry: Element[] = [] + let parent: Element | null = node + while (parent && ancestry.length < 6) { + ancestry.unshift(parent) + parent = parent.parentElement + } + const hidden = (element: Element) => { + if ( + element.matches( + "script,style,noscript,template,input,textarea,select,[hidden],[aria-hidden=true],[contenteditable]:not([contenteditable=false])", + ) + ) { + return true + } + const style = getComputedStyle(element) + return ( + style.display === "none" || + style.visibility === "hidden" || + style.visibility === "collapse" || + style.opacity === "0" || + style.contentVisibility === "hidden" + ) + } + const content = () => { + const editable = node.closest("input,textarea,select,[contenteditable]:not([contenteditable=false])") + if (editable) return editable.getAttribute("aria-label")?.slice(0, 180) + const pending: Node[] = [] + let current: Node | null = node.firstChild + let visits = 0 + let value = "" + while (current && visits++ < 256 && value.length < 180) { + if (current instanceof Element && !hidden(current) && current.firstChild) { + if (current.nextSibling) pending.push(current.nextSibling) + current = current.firstChild + continue + } + if (current.nodeType === 3) value += (current.textContent ?? "").replace(/\s+/g, " ").slice(0, 180 - value.length) + current = current.nextSibling ?? pending.pop() ?? null + } + return value.trim() || undefined + } + const text = content() + const snippet = node.cloneNode(false) as Element + const allowed = new Set([ + "id", + "class", + "role", + "type", + "name", + "aria-label", + "title", + "data-testid", + "data-test", + "data-cy", + ]) + for (const attribute of [...snippet.attributes]) { + if (!allowed.has(attribute.name)) snippet.removeAttribute(attribute.name) + else snippet.setAttribute(attribute.name, attribute.value.slice(0, 180)) + } + snippet.textContent = text ?? "" + const styles = getComputedStyle(node) + const source = () => { + const file = node.getAttribute("data-source-file") + if (file) { + const line = Number(node.getAttribute("data-source-line")) + const column = Number(node.getAttribute("data-source-column")) + return { + file: file.slice(0, 4096), + line: Number.isInteger(line) && line > 0 ? line : undefined, + column: Number.isInteger(column) && column > 0 ? column : undefined, + } + } + const key = Object.keys(node).find( + (item) => item.startsWith("__reactFiber$") || item.startsWith("__reactInternalInstance$"), + ) + if (!key) return undefined + type Fiber = { + _debugSource?: { fileName?: string; lineNumber?: number; columnNumber?: number } + _debugOwner?: Fiber + return?: Fiber + } + let fiber: Fiber | undefined = (node as unknown as Record)[key] + for (let depth = 0; fiber && depth < 8; depth++) { + const origin = fiber._debugSource + if (origin?.fileName) { + return { file: origin.fileName.slice(0, 4096), line: origin.lineNumber, column: origin.columnNumber } + } + fiber = fiber._debugOwner ?? fiber.return + } + return undefined + } + return { + ...result, + text, + hierarchy: ancestry.map(label), + html: snippet.outerHTML.slice(0, 800), + styles: { color: styles.color.slice(0, 80), backgroundColor: styles.backgroundColor.slice(0, 80) }, + source: source(), + } +} diff --git a/packages/kilo-vscode/src/shared/browser-feedback.ts b/packages/kilo-vscode/src/shared/browser-feedback.ts new file mode 100644 index 0000000000..7c99868fd2 --- /dev/null +++ b/packages/kilo-vscode/src/shared/browser-feedback.ts @@ -0,0 +1,333 @@ +import { partReview, type ReviewMessageData } from "./review-comments" + +export interface BrowserReference { + id: string + sessionId: string + selector: string + text?: string + url?: string + title?: string + hierarchy?: string[] + html?: string + styles?: { color?: string; backgroundColor?: string } + source?: { file: string; line?: number; column?: number } + content?: string +} + +export interface BrowserFeedbackData { + version: 1 + references: BrowserReference[] +} + +export interface FeedbackView { + review?: ReviewMessageData + browserFeedback?: BrowserFeedbackData + body: string +} + +const REFERENCE_LIMIT = 20 +const HIERARCHY_LIMIT = 20 +const TOTAL_LIMIT = 200_000 +const ID_LIMIT = 512 +const SESSION_LIMIT = 512 +const SELECTOR_LIMIT = 4_096 +const TEXT_LIMIT = 20_000 +const URL_LIMIT = 4_096 +const TITLE_LIMIT = 2_000 +const HIERARCHY_ITEM_LIMIT = 512 +const HTML_LIMIT = 20_000 +const STYLE_LIMIT = 256 +const SOURCE_LIMIT = 4_096 + +function record(value: unknown): Record | undefined { + if (!value || typeof value !== "object" || Array.isArray(value)) return undefined + return value as Record +} + +function string(value: unknown, limit: number): string | undefined { + if (typeof value !== "string" || value.length > limit) return undefined + return value +} + +function optionalString(value: unknown, limit: number): string | false | undefined { + if (value === undefined) return undefined + const result = string(value, limit) + return result === undefined ? false : result +} + +function safePath(value: string): boolean { + const absolute = value.startsWith("/") || value.startsWith("\\") || /^[A-Za-z]:[\\/]/.test(value) + return !absolute && !value.split(/[\\/]/).includes("..") && !value.includes("\0") +} + +function url(value: string): string | undefined { + try { + const parsed = new URL(value) + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") return undefined + parsed.username = "" + parsed.password = "" + parsed.search = "" + parsed.hash = "" + return parsed.toString() + } catch { + return undefined + } +} + +function optionalUrl(value: unknown): string | false | undefined { + if (value === undefined) return undefined + if (typeof value !== "string" || value.length > URL_LIMIT) return false + const result = url(value) + return result === undefined ? false : result +} + +function positive(value: unknown): number | false | undefined { + if (value === undefined) return undefined + if (typeof value !== "number" || !Number.isInteger(value) || value < 1) return false + return value +} + +function styles(value: unknown): BrowserReference["styles"] | false | undefined { + if (value === undefined) return undefined + const item = record(value) + if (!item) return false + const color = optionalString(item.color, STYLE_LIMIT) + const backgroundColor = optionalString(item.backgroundColor, STYLE_LIMIT) + if (color === false || backgroundColor === false) return false + if (color === undefined && backgroundColor === undefined) return undefined + return { + ...(color === undefined ? {} : { color }), + ...(backgroundColor === undefined ? {} : { backgroundColor }), + } +} + +function source(value: unknown): BrowserReference["source"] | false | undefined { + if (value === undefined) return undefined + const item = record(value) + if (!item) return false + const file = string(item.file, SOURCE_LIMIT) + const line = positive(item.line) + const column = positive(item.column) + if (!file || !safePath(file) || line === false || column === false) return false + return { + file, + ...(line === undefined ? {} : { line }), + ...(column === undefined ? {} : { column }), + } +} + +function hierarchy(value: unknown): string[] | false | undefined { + if (value === undefined) return undefined + if (!Array.isArray(value) || value.length > HIERARCHY_LIMIT) return false + const result = value.map((item) => string(item, HIERARCHY_ITEM_LIMIT)) + if (result.some((item) => item === undefined)) return false + return result as string[] +} + +function reference(value: unknown): BrowserReference | undefined { + const item = record(value) + if (!item) return undefined + const id = string(item.id, ID_LIMIT) + const sessionId = string(item.sessionId, SESSION_LIMIT) + const selector = string(item.selector, SELECTOR_LIMIT) + if (!id || !sessionId || !selector) return undefined + const text = optionalString(item.text, TEXT_LIMIT) + const pageUrl = optionalUrl(item.url) + const title = optionalString(item.title, TITLE_LIMIT) + const tree = hierarchy(item.hierarchy) + const html = optionalString(item.html, HTML_LIMIT) + const style = styles(item.styles) + const verified = source(item.source) + const content = item.content === undefined ? undefined : optionalString(item.content, 100_000) + if (!valid([text, pageUrl, title, tree, html, style, verified, content])) return undefined + const safeText = unwrap(text) + const safeUrl = unwrap(pageUrl) + const safeTitle = unwrap(title) + const safeTree = unwrap(tree) + const safeHtml = unwrap(html) + const safeStyle = unwrap(style) + const safeSource = unwrap(verified) + const safeContent = unwrap(content) + return { + id, + sessionId, + selector, + ...(safeText === undefined ? {} : { text: safeText }), + ...(safeUrl === undefined ? {} : { url: safeUrl }), + ...(safeTitle === undefined ? {} : { title: safeTitle }), + ...(safeTree === undefined ? {} : { hierarchy: safeTree }), + ...(safeHtml === undefined ? {} : { html: safeHtml }), + ...(safeStyle === undefined ? {} : { styles: safeStyle }), + ...(safeSource === undefined ? {} : { source: safeSource }), + ...(safeContent === undefined ? {} : { content: safeContent }), + } +} + +function valid( + values: Array, +): boolean { + return !values.some((item) => item === false) +} + +function unwrap(item: T | false | undefined): T | undefined { + return item === false ? undefined : item +} + +function weight(item: BrowserReference): number { + return JSON.stringify(item).length +} + +function normalize(references: readonly BrowserReference[]): BrowserReference[] | undefined { + if (references.length === 0 || references.length > REFERENCE_LIMIT) return undefined + const result = references.map(reference) + if (result.some((item) => item === undefined)) return undefined + const list = result as BrowserReference[] + if (list.reduce((total, item) => total + weight(item), 0) > TOTAL_LIMIT) return undefined + return list +} + +function escapeInline(value: string): string { + return value.replace(/[\r\n]+/g, " ").replace(/([\\`*_[\]{}()#+\-!|<>])/g, "\\$1") +} + +function fenced(value: string): string[] { + const matches = value.match(/`+/g) ?? [] + const longest = matches.reduce((max, item) => Math.max(max, item.length), 0) + const fence = "`".repeat(Math.max(3, longest + 1)) + return [fence, value, fence] +} + +function page(item: BrowserReference): string | undefined { + if (!item.url && !item.title) return undefined + return `Page: ${item.title ? escapeInline(item.title) : "Untitled"}${item.url ? ` (\`${escapeInline(item.url)}\`)` : ""}` +} + +function detail(item: BrowserReference, index: number, first: BrowserReference): string[] { + const lines = [`Element ${index + 1}:`, ...fenced(item.selector)] + if (index > 0 && (item.url !== first.url || item.title !== first.title)) { + const value = page(item) + if (value) lines.push(value) + } + if (item.hierarchy?.length) lines.push(...["DOM:", ...fenced(item.hierarchy.join(" > "))]) + if (item.text && (!item.html || item.html === item.text)) lines.push(...[`Text:`, ...fenced(item.text)]) + if (item.html && item.html !== item.text) lines.push(...[`HTML:`, ...fenced(item.html)]) + if (item.styles) { + const values = [ + item.styles.color ? `color=${escapeInline(item.styles.color)}` : "", + item.styles.backgroundColor ? `background=${escapeInline(item.styles.backgroundColor)}` : "", + ].filter(Boolean) + if (values.length) lines.push(`Styles: ${values.join(", ")}`) + } + if (item.source) { + const location = [item.source.file, item.source.line, item.source.column] + .filter((value) => value !== undefined) + .join(":") + lines.push(...[`Source:`, ...fenced(location)]) + } + return lines +} + +export function formatBrowserFeedback(references: BrowserReference[]): string { + const list = normalize(references) ?? [] + if (list.length === 0) return "## Browser Feedback" + const lines = ["## Browser Feedback", ""] + const heading = page(list[0]!) + if (heading) lines.push(heading, "") + list.forEach((item, index) => lines.push(...detail(item, index, list[0]!), "")) + return lines.join("\n").trimEnd() +} + +function view(value: unknown, content: string): { data: BrowserFeedbackData; body: string } | undefined { + const data = record(value) + if (!data || data.version !== 1 || !Array.isArray(data.references)) return undefined + const references = data.references.map(reference) + if (references.length === 0 || references.length > REFERENCE_LIMIT || references.some((item) => item === undefined)) + return undefined + const list = (references as BrowserReference[]).map((item) => { + const result = { ...item } + delete result.content + return result + }) + if (list.reduce((total, item) => total + weight(item), 0) > TOTAL_LIMIT) return undefined + const prefix = formatBrowserFeedback(list) + if (content === prefix) return { data: { version: 1, references: list }, body: "" } + if (!content.startsWith(`${prefix}\n\n`)) return undefined + return { data: { version: 1, references: list }, body: content.slice(prefix.length + 2) } +} + +export function browserFeedbackData(references: BrowserReference[]): BrowserFeedbackData | undefined { + const list = normalize(references) + if (!list) return undefined + return { + version: 1, + references: list.map((item) => { + const result = { ...item } + delete result.content + return result + }), + } +} + +export function mergeBrowserReferences(current: BrowserReference[], incoming: BrowserReference): BrowserReference[] { + const selected = browserFeedbackData([incoming])?.references[0] + if (!selected) return current + const kept = current.filter( + (item) => item.id !== selected.id && (item.selector !== selected.selector || item.url !== selected.url), + ) + if (kept.length >= REFERENCE_LIMIT) return current + return [...kept, selected] +} + +export function browserFeedbackMetadata(data: BrowserFeedbackData): Record { + return { kilo: { browserFeedback: data } } +} + +export function parseBrowserFeedback(value: unknown, content: string): BrowserFeedbackData | undefined { + return view(value, content)?.data +} + +export function feedbackMetadata( + review: ReviewMessageData | undefined, + browserFeedback: BrowserFeedbackData | undefined, +): Record | undefined { + if (!review && !browserFeedback) return undefined + return { + kilo: { + ...(review ? { review } : {}), + ...(browserFeedback ? { browserFeedback } : {}), + }, + } +} + +export function partFeedback(metadata: unknown, content: string): FeedbackView | undefined { + const root = record(metadata) + const kilo = record(root?.kilo) + const reviewValue = kilo?.review + const browserValue = kilo?.browserFeedback + let body = content + let review: ReviewMessageData | undefined + let browserFeedback: BrowserFeedbackData | undefined + if (reviewValue !== undefined) { + const parsed = partReview({ kilo: { review: reviewValue } }, body) + if (!parsed) return undefined + review = parsed.data + body = parsed.body + } + if (browserValue !== undefined) { + const parsed = view(browserValue, body) + if (!parsed) return undefined + browserFeedback = parsed.data + body = parsed.body + } + if (!review && !browserFeedback) return undefined + return { review, browserFeedback, body } +} + +export function parseFeedback( + metadata: { review?: unknown; browserFeedback?: unknown }, + content: string, +): Pick | undefined { + const parsed = partFeedback({ kilo: metadata }, content) + if (!parsed) return undefined + return { review: parsed.review, browserFeedback: parsed.browserFeedback } +} diff --git a/packages/kilo-vscode/tests/unit/agent-manager-tab-bar.test.ts b/packages/kilo-vscode/tests/unit/agent-manager-tab-bar.test.ts index b57dadf589..1c0aab4b01 100644 --- a/packages/kilo-vscode/tests/unit/agent-manager-tab-bar.test.ts +++ b/packages/kilo-vscode/tests/unit/agent-manager-tab-bar.test.ts @@ -29,10 +29,22 @@ describe("Agent Manager diff toggle", () => { const source = fs.readFileSync(BROWSER_PANEL, "utf-8") expect(source).toContain("props.state?.navigation") expect(source).toContain("when={identity()}") + expect(source).toContain("frame?.contentWindow?.location.replace(props.url)") expect(source).toContain('type: "agentManager.browser.input"') expect(source).toContain("if (pointing()) input(value, false)") }) + it("keeps browser chrome compact with one close action and no duplicate footer", () => { + const source = fs.readFileSync(BROWSER_PANEL, "utf-8") + expect(source).toContain('class="am-browser-address"') + expect(source).toContain('icon="arrow-right"') + expect(source).toContain('icon="window-cursor"') + expect(source.match(/icon="close"/g)).toHaveLength(1) + expect(source).not.toContain("am-browser-footer") + expect(source).not.toContain("am-browser-selected") + expect(source).not.toContain("am-browser-devtools-toolbar") + }) + it("renders live Git stats rather than pull-request stats", () => { const source = fs.readFileSync(TAB_BAR, "utf-8") const start = source.indexOf('title={props.t("agentManager.diff.toggle")}') diff --git a/packages/kilo-vscode/tests/unit/browser-broker.test.ts b/packages/kilo-vscode/tests/unit/browser-broker.test.ts index 6c6ed7cbe0..c3565634cc 100644 --- a/packages/kilo-vscode/tests/unit/browser-broker.test.ts +++ b/packages/kilo-vscode/tests/unit/browser-broker.test.ts @@ -1,7 +1,10 @@ import { afterEach, describe, expect, test } from "bun:test" -import { createServer, request } from "node:http" +import { createServer, request, type IncomingMessage } from "node:http" +import { connect } from "node:net" +import { PassThrough } from "node:stream" import WebSocket, { WebSocketServer } from "ws" import { BrowserBroker, diagnostic } from "../../src/services/browser-automation/browser-broker" +import { BrowserDevtools } from "../../src/services/browser-automation/browser-devtools" const brokers: BrowserBroker[] = [] @@ -146,6 +149,10 @@ describe("BrowserBroker", () => { }) expect(result.status).toBe(401) expect(JSON.parse(result.body)).toEqual({ error: "Unauthorized" }) + const malformed = await fetch(`${env.KILO_BROWSER_BROKER_URL}/browser/status`, { + headers: { authorization: `Bearer ${"é".repeat(64)}` }, + }) + expect(malformed.status).toBe(401) }) test("reports experimental availability only to authenticated clients", async () => { @@ -164,6 +171,33 @@ describe("BrowserBroker", () => { expect(await (await fetch(url, { headers })).json()).toEqual({ enabled: false }) }) + test("writes an explicit forbidden response before closing an untrusted upgrade", () => { + const server = createServer() + const tools = new BrowserDevtools( + server, + 4567, + () => {}, + () => {}, + ) + const url = new URL(tools.open("browser", "page", 1234, "dark")) + const endpoint = new URL(`ws://${url.searchParams.get("ws")}`) + const socket = new PassThrough() + const chunks: Buffer[] = [] + socket.on("data", (chunk) => chunks.push(chunk)) + server.emit( + "upgrade", + { + url: endpoint.pathname, + headers: { host: "127.0.0.1:4567", origin: "http://untrusted.invalid" }, + } as IncomingMessage, + socket, + Buffer.alloc(0), + ) + expect(Buffer.concat(chunks).toString()).toStartWith("HTTP/1.1 403 Forbidden\r\n") + expect(socket.destroyed).toBe(true) + tools.dispose() + }) + test("proxies page-scoped developer tools and rejects invalid capabilities or origins", async () => { const remote = createServer((req, res) => { const path = new URL(req.url ?? "/", "http://127.0.0.1").pathname @@ -270,11 +304,29 @@ describe("BrowserBroker", () => { const endpoint = `ws://${new URL(first.url).searchParams.get("ws")}` const forbidden = await new Promise((resolve, reject) => { - const socket = new WebSocket(endpoint, { headers: { origin: "http://untrusted.invalid" } }) - socket.once("unexpected-response", (_request, response) => resolve(response.statusCode ?? 0)) + const url = new URL(endpoint) + const socket = connect({ host: url.hostname, port: Number(url.port) }, () => { + socket.write( + [ + `GET ${url.pathname} HTTP/1.1`, + `Host: ${url.host}`, + "Connection: Upgrade", + "Upgrade: websocket", + "Sec-WebSocket-Version: 13", + `Sec-WebSocket-Key: ${Buffer.from("browser-test-key").toString("base64")}`, + "Origin: http://untrusted.invalid", + "\r\n", + ].join("\r\n"), + ) + }) + socket.once("data", (data) => { + resolve(Number(data.toString().match(/^HTTP\/1\.1 (\d+)/)?.[1] ?? 0)) + socket.end() + }) + socket.once("end", () => resolve(0)) socket.once("error", reject) }) - expect(forbidden).toBe(403) + expect([0, 403]).toContain(forbidden) const socket = new WebSocket(endpoint, { headers: { origin: new URL(first.url).origin } }) await new Promise((resolve, reject) => { diff --git a/packages/kilo-vscode/tests/unit/browser-element.test.ts b/packages/kilo-vscode/tests/unit/browser-element.test.ts new file mode 100644 index 0000000000..4ec3fb146a --- /dev/null +++ b/packages/kilo-vscode/tests/unit/browser-element.test.ts @@ -0,0 +1,155 @@ +import { afterEach, describe, expect, test } from "bun:test" +import { mkdtemp, mkdir, rm, symlink, writeFile } from "node:fs/promises" +import os from "node:os" +import path from "node:path" +import { Window } from "happy-dom" +import { capture, locate } from "../../src/services/browser-automation/browser-element" + +const windows: Window[] = [] + +afterEach(async () => { + await Promise.all(windows.splice(0).map((window) => window.happyDOM.close())) +}) + +function inspect(html: string, selector: string, detail = true) { + const window = new Window({ url: "http://localhost:3000/" }) + windows.push(window) + window.document.body.innerHTML = html + const node = window.document.querySelector(selector) + if (!node) throw new Error("Selected test element is missing") + Object.defineProperty(window.document, "elementFromPoint", { value: () => node }) + const run = new Function( + "document", + "Element", + "CSS", + "innerWidth", + "innerHeight", + "getComputedStyle", + `return (${capture.toString()})(${JSON.stringify({ x: 0.5, y: 0.5, detail })})`, + ) + const result = run( + window.document, + window.Element, + window.CSS, + window.innerWidth, + window.innerHeight, + window.getComputedStyle.bind(window), + ) as ReturnType | undefined + if (!result) throw new Error("Browser element capture returned no element") + return { result, node, document: window.document } +} + +describe("browser element context", () => { + test("builds a unique selector and bounded ancestry for repeated buttons without ids", () => { + const selected = inspect( + '
', + ".actions button:first-child", + ) + expect(selected.document.querySelectorAll(selected.result.selector)).toHaveLength(1) + expect(selected.document.querySelector(selected.result.selector)).toBe(selected.node) + expect(selected.result.hierarchy).toEqual([ + "html", + "body", + "main#app", + "section.hero", + "div.actions", + "button.primary", + ]) + expect(selected.result.html).toBe('') + }) + + test("prefers stable test ids and does not trust duplicate element ids", () => { + const stable = inspect('', "[data-testid]") + expect(stable.result.selector).toBe('button[data-testid="checkout"]') + const duplicate = inspect( + '
', + "button:last-child", + ) + expect(duplicate.result.selector).not.toBe("#duplicate") + expect(duplicate.document.querySelectorAll(duplicate.result.selector)).toHaveLength(1) + expect(duplicate.document.querySelector(duplicate.result.selector)).toBe(duplicate.node) + }) + + test("excludes scripts, handlers, arbitrary attributes, hidden text, and input values", () => { + const selected = inspect( + '', + "#save", + ) + expect(selected.result.text).toBe("Save") + expect(selected.result.html).toBe('') + expect(JSON.stringify(selected.result)).not.toContain("secret") + const password = inspect('', "input") + expect(password.result.text).toBe("Password") + expect(JSON.stringify(password.result)).not.toContain("private-password") + }) + + test("provides relevant colors and keeps hover responses lightweight", () => { + const selected = inspect('', "button") + expect(selected.result.styles?.backgroundColor).toBe("rgb(22, 163, 74)") + expect(selected.result.html).not.toContain("style=") + const hover = inspect('', "button", false) + expect(hover.result.selector).toBe("#save") + expect(hover.result).not.toHaveProperty("html") + expect(hover.result).not.toHaveProperty("hierarchy") + expect(hover.result).not.toHaveProperty("source") + }) + + test("bounds text and HTML instead of copying the entire document", () => { + const selected = inspect( + `
`, + "button", + ) + expect(selected.result.text?.length).toBeLessThanOrEqual(180) + expect(selected.result.html?.length).toBeLessThanOrEqual(800) + expect(selected.result.classes?.length).toBeLessThanOrEqual(180) + }) + + test("does not fabricate ambiguous selectors for deeply repeated structures", () => { + const nested = (depth: number) => + `
${"
".repeat(depth)}${"
".repeat(depth)}
` + const selected = inspect(nested(26).repeat(2), "section:nth-of-type(2) button") + expect(selected.document.querySelectorAll(selected.result.selector)).toHaveLength(1) + expect(selected.document.querySelector(selected.result.selector)).toBe(selected.node) + expect(() => inspect(nested(150).repeat(2), "section:nth-of-type(2) button")).toThrow( + "Browser element capture returned no element", + ) + }) + + test("bounds traversal before reading a large selected subtree", () => { + const selected = inspect( + `
${"".repeat(300)}late-private-text
`, + "main", + ) + expect(selected.result.text).toBeUndefined() + expect(selected.result.html).toBe('
') + }) + + test("accepts only existing source files within the owning workspace", async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "kilo-browser-source-")) + try { + const project = path.join(root, "project") + await mkdir(path.join(project, "src"), { recursive: true }) + await writeFile(path.join(project, "src", "Button.tsx"), "export const Button = () => {\n\n\n return null\n}") + await writeFile(path.join(root, "private.ts"), "export const privateValue = true") + await writeFile(path.join(project, ".env"), "SECRET=value") + await symlink(path.join(root, "private.ts"), path.join(project, "src", "external.ts")) + expect(await locate(project, { file: "src/Button.tsx", line: 4, column: 2 })).toEqual({ + file: "src/Button.tsx", + line: 4, + column: 2, + }) + expect(await locate(project, { file: "src/Button.tsx", line: 999, column: 2 })).toEqual({ + file: "src/Button.tsx", + line: undefined, + column: undefined, + }) + expect(await locate(project, { file: "src/missing.tsx" })).toBeUndefined() + expect(await locate(project, { file: "../private.ts" })).toBeUndefined() + expect(await locate(project, { file: "src/external.ts" })).toBeUndefined() + expect(await locate(project, { file: ".env" })).toBeUndefined() + expect(await locate(project, { file: "https://example.com/Button.tsx" })).toBeUndefined() + } finally { + await rm(root, { recursive: true, force: true }) + } + }) +}) diff --git a/packages/kilo-vscode/tests/unit/browser-feedback.test.ts b/packages/kilo-vscode/tests/unit/browser-feedback.test.ts new file mode 100644 index 0000000000..870a51290b --- /dev/null +++ b/packages/kilo-vscode/tests/unit/browser-feedback.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it } from "bun:test" +import { + browserFeedbackData, + browserFeedbackMetadata, + formatBrowserFeedback, + mergeBrowserReferences, + partFeedback, + parseBrowserFeedback, + type BrowserReference, +} from "../../src/shared/browser-feedback" +import { formatReviewCommentsMarkdown } from "../../webview-ui/src/utils/review-comment-markdown" + +const reference = (overrides: Partial = {}): BrowserReference => ({ + id: "browser-1", + sessionId: "session-1", + selector: "main > button.save", + url: "https://user:secret@example.com/app?token=private#section", + title: "Settings", + hierarchy: ["main", "button.save"], + text: "Save settings", + html: '', + styles: { color: "rgb(1, 2, 3)", backgroundColor: "white" }, + source: { file: "src/settings.tsx", line: 42, column: 7 }, + content: "legacy dump and bounds", + ...overrides, +}) + +describe("browser feedback formatter", () => { + it("formats grounded fields and omits legacy content and bounds", () => { + const text = formatBrowserFeedback([reference()]) + expect(text).toContain("Page: Settings") + expect(text).toContain("https://example.com/app") + expect(text).toContain("main > button.save") + expect(text).toContain("Save settings") + expect(text).toContain("src/settings.tsx:42:7") + expect(text).not.toContain("secret") + expect(text).not.toContain("token") + expect(text).not.toContain("legacy dump") + expect(text).not.toContain("Bounds") + }) + + it("keeps equivalent text and html from duplicating context", () => { + const text = formatBrowserFeedback([reference({ html: "Save settings" })]) + expect(text.match(/Save settings/g)?.length).toBe(1) + expect(text).not.toContain("HTML:") + }) + + it("includes readable text only once when a safe HTML snippet already contains it", () => { + const text = formatBrowserFeedback([reference()]) + expect(text.match(/Save settings/g)).toHaveLength(1) + expect(text).not.toContain("Text:") + }) + + it("updates repeated selections without duplicating the same page element", () => { + const first = browserFeedbackData([reference()])!.references + const merged = mergeBrowserReferences(first, reference({ id: "new-selection", text: "Updated settings" })) + expect(merged).toHaveLength(1) + expect(merged[0]?.id).toBe("new-selection") + expect(merged[0]?.text).toBe("Updated settings") + expect(merged[0]?.url).toBe("https://example.com/app") + expect(merged[0]).not.toHaveProperty("content") + }) + + it("rejects invalid and oversized references", () => { + expect(browserFeedbackData([])).toBeUndefined() + expect(browserFeedbackData([reference({ selector: "x".repeat(5_000) })])).toBeUndefined() + expect(browserFeedbackData(Array.from({ length: 21 }, (_, id) => reference({ id: String(id) })))).toBeUndefined() + expect(browserFeedbackData([reference({ source: { file: "../secret" } })])).toBeUndefined() + expect(browserFeedbackData([reference({ url: "file:///tmp/private" })])).toBeUndefined() + expect(browserFeedbackData([reference({ text: "x".repeat(20_001) })])).toBeUndefined() + }) +}) + +describe("browser feedback metadata", () => { + it("round-trips metadata while ignoring legacy content", () => { + const data = browserFeedbackData([reference()])! + const prefix = formatBrowserFeedback(data.references) + expect(parseBrowserFeedback(data, `${prefix}\n\nFix the save action`)).toEqual(data) + expect(partFeedback(browserFeedbackMetadata(data), `${prefix}\n\nFix the save action`)).toEqual({ + browserFeedback: data, + body: "Fix the save action", + }) + }) + + it("rejects arbitrary text that does not match the metadata prefix", () => { + const data = browserFeedbackData([reference()])! + expect(parseBrowserFeedback(data, "unrelated text")).toBeUndefined() + }) + + it("coexists with local and PR review metadata", () => { + const review = { + version: 1 as const, + comments: [ + { + id: "review-1", + file: "src/app.ts", + side: "additions" as const, + line: 3, + comment: "Keep this branch safe", + selectedText: "return value", + }, + ], + } + const browser = browserFeedbackData([reference()])! + const reviewPrefix = formatReviewCommentsMarkdown(review.comments) + const browserPrefix = formatBrowserFeedback(browser.references) + const content = `${reviewPrefix}\n\n${browserPrefix}\n\nDo both` + expect(partFeedback({ kilo: { review, browserFeedback: browser } }, content)).toEqual({ + review, + browserFeedback: browser, + body: "Do both", + }) + }) +}) diff --git a/packages/kilo-vscode/tests/unit/draft-store.test.ts b/packages/kilo-vscode/tests/unit/draft-store.test.ts index 181c5f5e4b..fa640137ca 100644 --- a/packages/kilo-vscode/tests/unit/draft-store.test.ts +++ b/packages/kilo-vscode/tests/unit/draft-store.test.ts @@ -11,28 +11,32 @@ import { isPendingSend, promotePendingDraftDiscard, reviewDrafts, + browserDrafts, savePromptDraft, scrollDrafts, finishPendingSend, } from "../../webview-ui/src/utils/draft-store" -const stores = [drafts, reviewDrafts, imageDrafts, scrollDrafts] +const stores = [drafts, browserDrafts, reviewDrafts, imageDrafts, scrollDrafts] beforeEach(() => stores.forEach((store) => store.clear())) describe("prompt draft storage", () => { it("stores and clears all prompt artifacts together", () => { + const browser = [{ id: "browser", sessionId: "s1", selector: "#save", content: "legacy" }] savePromptDraft( "prompt:default:pending:sidebar-pending:1", "draft", [{ id: "review", file: "a.ts", side: "additions", line: 1, comment: "comment", selectedText: "line" }], [{ id: "image", filename: "a.png", mime: "image/png", dataUrl: "data:image/png;base64,a" }], 42, + browser, ) expect(drafts.size).toBe(1) expect(reviewDrafts.size).toBe(1) expect(imageDrafts.size).toBe(1) + expect(browserDrafts.get("prompt:default:pending:sidebar-pending:1")).toEqual(browser) expect(scrollDrafts.size).toBe(1) discardPendingDraft("sidebar-pending:1") diff --git a/packages/kilo-vscode/tests/unit/prompt-drafts.test.ts b/packages/kilo-vscode/tests/unit/prompt-drafts.test.ts index ee98402d57..09a0d8674c 100644 --- a/packages/kilo-vscode/tests/unit/prompt-drafts.test.ts +++ b/packages/kilo-vscode/tests/unit/prompt-drafts.test.ts @@ -174,20 +174,25 @@ describe("movePromptDraft", () => { const comments = new Map([[source, [comment]]]) const images = new Map([[source, [image]]]) const scrolls = new Map([[source, 128]]) + const browser = new Map([[source, [{ id: "browser-1", sessionId: "session-1", selector: "#save" }]]]) + const expected = browser.get(source) - expect(movePromptDraft({ text, comments, images, scrolls }, source, target)).toEqual({ + expect(movePromptDraft({ text, comments, images, scrolls, browsers: browser }, source, target)).toEqual({ text: "Keep this prompt", comments: [comment], images: [image], scroll: 128, + browsers: expected, }) expect(text.get(target)).toBe("Keep this prompt") expect(comments.get(target)).toEqual([comment]) expect(images.get(target)).toEqual([image]) + expect(browser.get(target)).toEqual([{ id: "browser-1", sessionId: "session-1", selector: "#save" }]) expect(scrolls.get(target)).toBe(128) expect(text.has(source)).toBe(false) expect(comments.has(source)).toBe(false) expect(images.has(source)).toBe(false) + expect(browser.has(source)).toBe(false) expect(scrolls.has(source)).toBe(false) }) }) diff --git a/packages/kilo-vscode/tests/unit/prompt-input-connection-guard.test.ts b/packages/kilo-vscode/tests/unit/prompt-input-connection-guard.test.ts index fe8d3cd312..8876561a7c 100644 --- a/packages/kilo-vscode/tests/unit/prompt-input-connection-guard.test.ts +++ b/packages/kilo-vscode/tests/unit/prompt-input-connection-guard.test.ts @@ -14,7 +14,7 @@ describe("PromptInput connection guard", () => { const attachments = src.indexOf("const gitFile = await git.resolveAttachment") const guard = src.indexOf("if (isDisabled()) {", attachments) const finish = src.indexOf("finishPending(pendingId)", guard) - const send = src.indexOf("session.sendMessage(message", guard) + const send = src.indexOf("session.sendMessage(", guard) const clear = src.indexOf("drafts.delete(key)", send) expect(attachments).toBeGreaterThan(-1) @@ -49,8 +49,8 @@ describe("PromptInput sandbox toggle", () => { }) it("captures edits made while sandbox session creation is pending", () => { - const start = src.indexOf('if (message.type === "sessionCreated")') - const end = src.indexOf('if (message.type === "action"', start) + const start = src.indexOf("const created = (message:") + const end = src.indexOf("const unsubscribe", start) const created = src.slice(start, end) const save = created.indexOf( "if (source === draftKey()) saveDraft(source, text(), reviewComments(), imageAttach.images())", @@ -61,7 +61,10 @@ describe("PromptInput sandbox toggle", () => { expect(end).toBeGreaterThan(start) expect(save).toBeGreaterThan(-1) expect(move).toBeGreaterThan(save) - expect(created).toContain("{ text: drafts, comments: reviewDrafts, images: imageDrafts, scrolls: scrollDrafts }") + expect(created).toContain( + "{ text: drafts, comments: reviewDrafts, images: imageDrafts, scrolls: scrollDrafts, browsers: references }", + ) + expect(created).toContain("saveDraft(source, text(), reviewComments(), imageAttach.images())") }) it("restores each prompt draft's textarea and highlight scroll positions", () => { @@ -70,8 +73,14 @@ describe("PromptInput sandbox toggle", () => { expect(src).toContain("textareaRef.scrollTop = scroll") expect(src).toContain("if (highlightRef) highlightRef.scrollTop = scroll") expect(src).toContain("scrollDrafts.set(draftKey(), textareaRef.scrollTop)") - expect(src).toContain("images: imageAttach.images(),\n scroll: textareaRef?.scrollTop") - expect(src).toContain("draft.text, draft.comments, draft.images, draft.scroll") + expect(src).toContain( + "images: imageAttach.images(),\n browsers: browsers(),\n scroll: textareaRef?.scrollTop", + ) + expect(src).toContain("draft.text,") + expect(src).toContain("draft.comments,") + expect(src).toContain("draft.images,") + expect(src).toContain("draft.scroll,") + expect(src).toContain("draft.browsers") }) it("tracks in-flight toggles per session while switching", () => { diff --git a/packages/kilo-vscode/tests/unit/prompt-send-contract.test.ts b/packages/kilo-vscode/tests/unit/prompt-send-contract.test.ts index 94256f262b..e9c9f6239d 100644 --- a/packages/kilo-vscode/tests/unit/prompt-send-contract.test.ts +++ b/packages/kilo-vscode/tests/unit/prompt-send-contract.test.ts @@ -387,8 +387,9 @@ describe("PromptInput send origin contract", () => { }) it("passes the captured origin to message and command sends", () => { - expect(source).toMatch(/session\.sendMessage\([\s\S]*origin \?\? null\)/) - expect(source).toMatch(/session\.sendCommand\([\s\S]*origin \?\? null\)/) + expect(source).toMatch(/session\.sendMessage\([\s\S]*origin \?\? null[\s\S]*browserData[\s\S]*\)/) + const command = source.slice(source.indexOf("session.sendCommand(")) + expect(command).toMatch(/origin \?\? null[\s\S]*\{[\s\S]*agent: matched\.agent/) }) it("records sent prompts before a pending session key change can return", () => { @@ -665,18 +666,31 @@ describe("browser element reference contract", () => { }) it("includes browser reference content only when the user sends the prompt", () => { - expect(source).toMatch(/const browser = browsers\(\)[\s\S]*?\.map\(\(item\) => item\.content\)/) - expect(source).toContain('const message = [review, browser, draft].filter(Boolean).join("\\n\\n")') + expect(source).toContain("browserFeedbackData(browsers())") + expect(source).toContain("formatBrowserFeedback(browserData.references)") + expect(source).toContain('const message = [review, browserText, draft].filter(Boolean).join("\\n\\n")') expect(source).toContain("references.delete(key)") }) it("restores browser attachments for the correct session and allows attachment-only sends", () => { expect(source).toContain("setBrowsers(references.get(key) ?? [])") - expect(source).toContain("reference.sessionId === sid()") + expect(source).toContain("if (reference.sessionId !== sid()) return") + expect(source).toContain("mergeBrowserReferences(browsers(), reference)") expect(source).toContain("browsers().length > 0") }) }) +describe("sent browser feedback rendering contract", () => { + const message = readFile(path.join(ROOT, "webview-ui/src/components/chat/VscodeUserMessage.tsx")) + + it("renders validated browser metadata as cards and exposes only the instruction body", () => { + expect(message).toContain("partFeedback") + expect(message).toContain("BrowserReferences") + expect(message).toContain("feedback()?.body") + expect(message).not.toContain("item.content") + }) +}) + describe("KiloConnectionService pruneSession contract", () => { const source = readFile(CONNECTION_SERVICE_FILE) diff --git a/packages/kilo-vscode/tests/unit/session-utils.test.ts b/packages/kilo-vscode/tests/unit/session-utils.test.ts index a401bb2ab1..3cd54d5bdb 100644 --- a/packages/kilo-vscode/tests/unit/session-utils.test.ts +++ b/packages/kilo-vscode/tests/unit/session-utils.test.ts @@ -26,6 +26,7 @@ import { revertPromptState, } from "../../webview-ui/src/context/session-utils" import type { Message, Part, ToolPart } from "../../webview-ui/src/types/messages" +import { formatBrowserFeedback } from "../../src/shared/browser-feedback" const t = (key: string) => key @@ -1030,6 +1031,24 @@ describe("revertPromptState", () => { it("returns empty collections for tool-only messages", () => { const part: Part = { type: "tool", id: "p1", tool: "bash", state: { status: "running", input: {} } } const state = revertPromptState([part]) - expect(state).toEqual({ text: "", paths: [], sessions: [], images: [] }) + expect(state).toEqual({ text: "", paths: [], sessions: [], images: [], review: [], browser: [] }) + }) + + it("restores browser and review metadata without their formatted prefixes", () => { + const browser = { + version: 1 as const, + references: [{ id: "b", sessionId: "s1", selector: "#save", text: "Save" }], + } + const content = `${formatBrowserFeedback(browser.references)}\n\nPlease update it` + const part: Part = { + type: "text", + id: "t1", + text: content, + metadata: { kilo: { browserFeedback: browser } }, + } + expect(revertPromptState([part])).toMatchObject({ + text: "Please update it", + browser: browser.references, + }) }) }) diff --git a/packages/kilo-vscode/webview-ui/agent-manager/BrowserPanel.tsx b/packages/kilo-vscode/webview-ui/agent-manager/BrowserPanel.tsx index 407e581963..64c0e44460 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/BrowserPanel.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/BrowserPanel.tsx @@ -1,11 +1,24 @@ -import { createEffect, createSignal, For, onCleanup, Show, type Accessor, type Component, type Setter } from "solid-js" -import { Button } from "@kilocode/kilo-ui/button" +import { + createEffect, + createSignal, + For, + on, + onCleanup, + Show, + type Accessor, + type Component, + type Setter, +} from "solid-js" +import { Icon } from "@kilocode/kilo-ui/icon" import { IconButton } from "@kilocode/kilo-ui/icon-button" +import { Spinner } from "@kilocode/kilo-ui/spinner" import { TextField } from "@kilocode/kilo-ui/text-field" +import { Tooltip } from "@kilocode/kilo-ui/tooltip" import { useLanguage } from "../src/context/language" import { useVSCode } from "../src/context/vscode" import type { AgentManagerBrowserInspectionMessage, ExtensionMessage, WebviewMessage } from "../src/types/messages" import { SidePanel } from "./side-panel-layout" +import { formatBrowserFeedback } from "../../src/shared/browser-feedback" export function createBrowserPanel( current: Accessor, @@ -52,30 +65,6 @@ type Inspection = AgentManagerBrowserInspectionMessage type Position = { x: number; y: number; width: number; height: number } type Pointer = MouseEvent & { currentTarget: HTMLButtonElement } -function feedback(message: Inspection): string { - const element = message.element - const attrs = [ - element?.id ? `id="${element.id}"` : undefined, - element?.classes ? `class="${element.classes}"` : undefined, - ] - .filter(Boolean) - .join(" ") - return [ - "Browser feedback", - message.url ? `URL: ${message.url}` : undefined, - message.title ? `Page: ${message.title}` : undefined, - element ? `Selected element: <${element.tag}${attrs ? ` ${attrs}` : ""}>` : "Selected element: unavailable", - element?.selector ? `Selector: ${element.selector}` : undefined, - element?.rect - ? `Bounds: x=${element.rect.x.toFixed(3)}, y=${element.rect.y.toFixed(3)}, width=${element.rect.width.toFixed(3)}, height=${element.rect.height.toFixed(3)}` - : undefined, - element?.text ? `Visible text: ${element.text}` : undefined, - message.logs.length ? `Console diagnostics:\n${message.logs.map((line) => `- ${line}`).join("\n")}` : undefined, - ] - .filter(Boolean) - .join("\n") -} - function position(event: Pointer): Position { const bounds = event.currentTarget.getBoundingClientRect() return { @@ -88,10 +77,13 @@ function position(event: Pointer): Position { const Toolbar: Component<{ url: string + title?: string active: boolean selecting: boolean docked: boolean ready: boolean + loading: boolean + errors: number onUrl: (value: string) => void onOpen: () => void onSelect: () => void @@ -100,55 +92,95 @@ const Toolbar: Component<{ onClose: () => void }> = (props) => { const t = useLanguage().t + const diagnostics = () => + props.errors + ? `${t("agentManager.browser.devtoolsTitle")}, ${t("agentManager.browser.errors", { count: props.errors })}` + : t("agentManager.browser.devtoolsTitle") return (
- { - if (event.key === "Enter") props.onOpen() + + + +
{ + event.preventDefault() + if (props.active && props.url.trim() && !props.loading) props.onOpen() }} - /> - - - - - + + event.currentTarget.select()} + /> + + + + + + + +
+ + + + 0}> + + +
+ + +
) } @@ -191,6 +223,30 @@ const Picker: Component<{ ) } +const Preview: Component<{ url: string; navigation: number }> = (props) => { + const t = useLanguage().t + let frame: HTMLIFrameElement | undefined + createEffect( + on( + () => props.navigation, + (value, previous) => { + if (previous === undefined || value === previous) return + frame?.contentWindow?.location.replace(props.url) + }, + ), + ) + return ( +