From e4bda3ff776bdfe45a23d31a52d9589ece7765c3 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 2 Sep 2026 11:41:34 -0500 Subject: [PATCH] feat(desktop): browser comments carry the element's selector, markup, and styles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comment mode shipped the crop and the note, so an agent got a picture of the problem and had to grep for the element it showed. Each element comment now also names its CSS selector, its markup, and the computed styles that decide layout, which is what the agent needs to land in the right file. The target line stays prose — it is what the user pointed at — and the DOM detail rides labelled lines beneath it. Area pins have no element, so they still get only the crop and the note. Markup is redacted in the guest before it crosses to the host: password and hidden input values, and any attribute reading as a key/token/secret, are replaced with [redacted] on a clone, so a page's secrets never reach the composer or the model. It is clipped to a 600-char budget so one comment cannot paste a whole section. AnnotateIdentity was a hand-copy of CompactIdentity that had already drifted; it is now an alias, so the guest, the pin, and the packer cannot disagree about the shape again. --- .../src/lib/preview-annotate/identity.ts | 17 +++- .../src/lib/preview-annotate/in-page.test.ts | 81 +++++++++++++++++++ .../src/lib/preview-annotate/in-page.ts | 69 ++++++++++++++++ .../desktop/src/lib/preview-annotate/index.ts | 1 + .../src/lib/preview-annotate/pack.test.ts | 51 +++++++++--- apps/desktop/src/lib/preview-annotate/pack.ts | 25 +++++- .../src/lib/preview-annotate/stack.test.ts | 8 +- .../desktop/src/lib/preview-annotate/stack.ts | 10 +-- .../src/lib/preview-annotate/tokens.ts | 14 ++++ website/docs/user-guide/desktop.md | 2 +- 10 files changed, 258 insertions(+), 20 deletions(-) diff --git a/apps/desktop/src/lib/preview-annotate/identity.ts b/apps/desktop/src/lib/preview-annotate/identity.ts index f8b94e4ea2..578dbb86fc 100644 --- a/apps/desktop/src/lib/preview-annotate/identity.ts +++ b/apps/desktop/src/lib/preview-annotate/identity.ts @@ -1,8 +1,9 @@ -import { ANNOTATE_CSS_KEYS } from './tokens' +import { ANNOTATE_CSS_KEYS, ANNOTATE_HTML_BUDGET } from './tokens' export interface ElementSnapshot { className?: string css: Record + html?: string id?: string role?: string selector: string @@ -12,6 +13,7 @@ export interface ElementSnapshot { export interface CompactIdentity { css: Record + html: string selector: string tag: string text: string @@ -47,6 +49,16 @@ function clip(value: string, max: number): string { return `${trimmed.slice(0, max - 1)}…` } +/** + * Markup keeps its own clip: it arrives already budgeted and redacted from the + * guest, and this is the backstop for a snapshot built anywhere else. Newlines + * collapse but the tag structure survives — `clip` alone would be fine, this + * just names the different budget. + */ +function clipHtml(value: string): string { + return clip(value, ANNOTATE_HTML_BUDGET) +} + /** Keep only the curated CSS snapshot, drop empties and the whole document. */ export function compactIdentity(snapshot: ElementSnapshot): CompactIdentity { const css: Record = {} @@ -64,8 +76,9 @@ export function compactIdentity(snapshot: ElementSnapshot): CompactIdentity { const tag = (snapshot.tag || 'div').toLowerCase() const selector = clip(snapshot.selector || tag, MAX_SELECTOR) const text = clip(snapshot.text || '', MAX_TEXT) + const html = clipHtml(snapshot.html || '') - return { css, selector, tag, text } + return { css, html, selector, tag, text } } export function formatIdentityLine(identity: CompactIdentity): string { diff --git a/apps/desktop/src/lib/preview-annotate/in-page.test.ts b/apps/desktop/src/lib/preview-annotate/in-page.test.ts index 06a9799ff9..203cec3775 100644 --- a/apps/desktop/src/lib/preview-annotate/in-page.test.ts +++ b/apps/desktop/src/lib/preview-annotate/in-page.test.ts @@ -140,9 +140,90 @@ describe('annotateInPage overlay', () => { expect(event.identity.tag).toBe('button') expect(event.identity.selector).toContain('go') expect(event.identity.text).toBe('Go') + expect(event.identity.html).toContain(' { + const form = document.createElement('form') + form.setAttribute('data-api-key', 'sk-live-1234567890') + form.innerHTML = + '' + + '' + document.body.appendChild(form) + form.getBoundingClientRect = () => ({ + bottom: 60, + height: 50, + left: 0, + right: 200, + toJSON: () => ({}), + top: 10, + width: 200, + x: 0, + y: 10 + }) + document.elementFromPoint = () => form + + const api = annotateInPage(document) + api.install() + const pending = api.wait() + const host = document.querySelector('hermes-annotate') as HTMLElement + + host.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, button: 0, clientX: 20, clientY: 20 })) + host.dispatchEvent(new MouseEvent('mouseup', { bubbles: true, button: 0, clientX: 21, clientY: 21 })) + + const event = await pending + + expect(event.type).toBe('pick-element') + + if (event.type === 'pick-element') { + expect(event.identity.html).not.toContain('hunter2') + expect(event.identity.html).not.toContain('tok_abc') + expect(event.identity.html).not.toContain('sk-live-1234567890') + expect(event.identity.html).toContain('[redacted]') + // A non-secret field keeps its value — redaction is targeted, not a blanket wipe. + expect(event.identity.html).toContain('me@example.com') + } + + api.teardown() + }) + + it('budgets the markup so one comment cannot paste a whole section', async () => { + const section = document.createElement('section') + section.innerHTML = '

filler filler filler

'.repeat(200) + document.body.appendChild(section) + section.getBoundingClientRect = () => ({ + bottom: 400, + height: 400, + left: 0, + right: 300, + toJSON: () => ({}), + top: 0, + width: 300, + x: 0, + y: 0 + }) + document.elementFromPoint = () => section + + const api = annotateInPage(document) + api.install() + const pending = api.wait() + const host = document.querySelector('hermes-annotate') as HTMLElement + + host.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, button: 0, clientX: 20, clientY: 20 })) + host.dispatchEvent(new MouseEvent('mouseup', { bubbles: true, button: 0, clientX: 21, clientY: 21 })) + + const event = await pending + + if (event.type === 'pick-element') { + expect(event.identity.html.length).toBeLessThanOrEqual(600) + expect(event.identity.html.startsWith('
')).toBe(true) + } + + api.teardown() + }) + it('owns wheel scrolling instead of also allowing the native wheel action', () => { const scroller = document.createElement('div') scroller.style.overflowY = 'auto' diff --git a/apps/desktop/src/lib/preview-annotate/in-page.ts b/apps/desktop/src/lib/preview-annotate/in-page.ts index 48216ee47c..7aee415d5c 100644 --- a/apps/desktop/src/lib/preview-annotate/in-page.ts +++ b/apps/desktop/src/lib/preview-annotate/in-page.ts @@ -19,6 +19,7 @@ export interface AnnotatePageRect { export interface AnnotatePageIdentity { css: Record + html: string selector: string tag: string text: string @@ -72,16 +73,25 @@ export function annotateInPage(doc: Document): AnnotateInPage { 'position', 'width', 'height', + 'max-width', 'padding', 'margin', + 'border', 'border-radius', + 'box-shadow', 'opacity', + 'overflow', + 'z-index', + 'transform', 'flex-direction', 'gap', + 'grid-template-columns', 'justify-content', 'align-items' ] + const htmlBudget = 600 + let host: HTMLElement | null = null let shadow: ShadowRoot | null = null let hoverBox: HTMLElement | null = null @@ -240,11 +250,70 @@ export function annotateInPage(doc: Document): AnnotateInPage { return out } + /** + * Markup for the picked element, with anything secret-shaped stripped first. + * + * A comment is user-authored context, but the element under the cursor is + * whatever the page put there: a filled password box, a token in a hidden + * input, an api-key data attribute. Redaction happens on a clone here, in + * the guest, so the secret never reaches the host, the composer, or the + * model — the same reason `browser_type` masks what it types. + */ + const markupOf = (el: Element): string => { + let clone: Element + + try { + clone = el.cloneNode(true) as Element + } catch { + return '' + } + + const nodes: Element[] = [clone] + const nested = clone.querySelectorAll('input, textarea, select, [data-secret]') + + for (let i = 0; i < nested.length; i++) { + nodes.push(nested[i]) + } + + for (const node of nodes) { + const tag = node.tagName.toLowerCase() + const type = (node.getAttribute('type') || '').toLowerCase() + const secretField = tag === 'input' && (type === 'password' || type === 'hidden') + const names = node.getAttributeNames() + + for (const name of names) { + const lower = name.toLowerCase() + + if (lower === 'value' && (secretField || node.getAttribute('value'))) { + node.setAttribute(name, secretField ? '[redacted]' : node.getAttribute(name) || '') + } + + if (/key|token|secret|password|auth|session|credential/.test(lower)) { + node.setAttribute(name, '[redacted]') + } + } + + if (secretField) { + node.setAttribute('value', '[redacted]') + } + } + + const html = clone.outerHTML || '' + + if (html.length <= htmlBudget) { + return html + } + + // Keep the opening tag — where the classes and props live — over the tail. + return `${html.slice(0, htmlBudget - 1)}…` + } + const identityOf = (el: Element): AnnotatePageIdentity => { const text = (el.textContent || '').replace(/\s+/g, ' ').trim() return { css: readCss(el), + html: markupOf(el), selector: cssPath(el), tag: el.tagName.toLowerCase(), text: text.length > 80 ? `${text.slice(0, 79)}…` : text diff --git a/apps/desktop/src/lib/preview-annotate/index.ts b/apps/desktop/src/lib/preview-annotate/index.ts index ad45c02768..b63897293b 100644 --- a/apps/desktop/src/lib/preview-annotate/index.ts +++ b/apps/desktop/src/lib/preview-annotate/index.ts @@ -42,6 +42,7 @@ export { ANNOTATE_CARD_WIDTH, ANNOTATE_CROP_PAD, ANNOTATE_CSS_KEYS, + ANNOTATE_HTML_BUDGET, ANNOTATE_MARKER_SIZE, ANNOTATE_OUTLINE_WIDTH, ANNOTATE_PILL_BG, diff --git a/apps/desktop/src/lib/preview-annotate/pack.test.ts b/apps/desktop/src/lib/preview-annotate/pack.test.ts index b24b8c2da3..5f20d0d239 100644 --- a/apps/desktop/src/lib/preview-annotate/pack.test.ts +++ b/apps/desktop/src/lib/preview-annotate/pack.test.ts @@ -4,6 +4,7 @@ import { flushAnnotateStack } from './flush' import { compactIdentity } from './identity' import { annotateFlushPrompt, packageAnnotatePin, packageAnnotateStack } from './pack' import { addAnnotatePin, type AnnotatePin, emptyAnnotateStack } from './stack' +import { ANNOTATE_HTML_BUDGET } from './tokens' const png = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==' @@ -20,6 +21,7 @@ function pin(partial: Partial = {}): AnnotatePin { rect: { height: 40, width: 120, x: 8, y: 8 }, identity: { css: { color: 'rgb(24, 24, 24)', 'font-size': '14px' }, + html: '', selector: 'button.plan', tag: 'button', text: 'Select plan' @@ -29,13 +31,23 @@ function pin(partial: Partial = {}): AnnotatePin { } describe('packageAnnotatePin', () => { - it('describes text in a generic container without exposing DOM and style internals', () => { + it('carries the selector, markup, and computed styles the agent needs to find the source', () => { + const packed = packageAnnotatePin(pin()) + + expect(packed.prompt).toContain('Selector: button.plan') + expect(packed.prompt).toContain('HTML: ') + expect(packed.prompt).toContain('color: rgb(24, 24, 24)') + expect(packed.prompt).toContain('font-size: 14px') + }) + + it('keeps the target line prose while the DOM detail rides its own labelled lines', () => { const text = 'גם בקיבוץ חולית הקטן יש ילד שעושה את הצעד הראשון במערכת החינוך' const packed = packageAnnotatePin( pin({ identity: { css: { color: 'rgb(0, 0, 0)', 'font-family': 'Moses, NarkisBlock', 'font-size': '18px' }, + html: '
…
', selector: 'div.DraftEditor-editorContainer>div.public-DraftEditor-content>div>div.text_editor_paragraph.rtl:nth-of-type(9)', tag: 'div', @@ -45,11 +57,13 @@ describe('packageAnnotatePin', () => { }) ) - expect(packed.prompt).toContain(`Target: "${text}"`) + const target = packed.prompt.split('\n').find(line => line.startsWith('Target:')) + + expect(target).toBe(`Target: "${text}"`) + expect(target).not.toContain('div') + expect(target).not.toContain('DraftEditor') expect(packed.prompt).toContain('Note: תסכם את זה') - expect(packed.prompt).not.toContain('div') - expect(packed.prompt).not.toContain('DraftEditor') - expect(packed.prompt).not.toContain('font-size') + expect(packed.prompt).toContain('Selector: div.DraftEditor-editorContainer') }) it('packs a numbered crop, compact identity, and the note', () => { @@ -62,8 +76,6 @@ describe('packageAnnotatePin', () => { expect(packed.prompt).toContain('Target: button "Select plan"') expect(packed.prompt).toContain('Image 1 marks the target in blue.') expect(packed.prompt).toContain('Note: This button overflows on mobile.') - expect(packed.prompt).not.toContain('button.plan') - expect(packed.prompt).not.toContain('font-size') expect(packed.prompt).not.toContain(' { pin({ identity: { css: { display: 'block' }, + html: '
', selector: '#sales-chart', tag: 'div', text: '' @@ -81,15 +94,17 @@ describe('packageAnnotatePin', () => { ) expect(packed.prompt).toContain('Target: #sales-chart') - expect(packed.prompt).not.toContain('display: block') }) - it('packages an area pin without pretending it has a selector', () => { + it('invents no element detail for an area pin', () => { const packed = packageAnnotatePin(pin({ identity: undefined, kind: 'area', note: 'too tight' })) expect(packed.prompt).toContain('area') expect(packed.prompt).toContain('120×40px') expect(packed.prompt).toContain('too tight') + expect(packed.prompt).not.toContain('Selector:') + expect(packed.prompt).not.toContain('HTML:') + expect(packed.prompt).not.toContain('Styles:') }) }) @@ -109,6 +124,24 @@ describe('compactIdentity', () => { expect(compact.css.margin).toBeUndefined() expect(compact.css.color).toBe('red') }) + + it('clips markup to the budget rather than pasting a whole section', () => { + const compact = compactIdentity({ + css: {}, + html: `
${'

filler

'.repeat(400)}
`, + selector: 'section', + tag: 'section', + text: '' + }) + + expect(compact.html.length).toBeLessThanOrEqual(ANNOTATE_HTML_BUDGET) + expect(compact.html.startsWith('
')).toBe(true) + expect(compact.html.endsWith('…')).toBe(true) + }) + + it('tolerates a snapshot with no markup', () => { + expect(compactIdentity({ css: {}, selector: 'div', tag: 'div', text: '' }).html).toBe('') + }) }) describe('flushAnnotateStack', () => { diff --git a/apps/desktop/src/lib/preview-annotate/pack.ts b/apps/desktop/src/lib/preview-annotate/pack.ts index c1827eeb9a..245e3bf103 100644 --- a/apps/desktop/src/lib/preview-annotate/pack.ts +++ b/apps/desktop/src/lib/preview-annotate/pack.ts @@ -17,13 +17,36 @@ function identityBlock(pin: AnnotatePin): string { return formatIdentityLine(pin.identity) } +function cssBlock(identity: CompactIdentity): string { + const entries = Object.entries(identity.css) + + if (!entries.length) { + return '' + } + + return `Styles: ${entries.map(([name, value]) => `${name}: ${value}`).join('; ')}` +} + +/** + * One comment, as much as the agent needs to find the element in source. + * + * The human-readable target line stays first and stays prose — it is what the + * user actually pointed at. Selector, markup, and computed styles follow as + * labelled lines, because the crop shows what is wrong and the DOM shows where + * it lives; an agent given only the picture greps for the wrong div. Area pins + * have no element, so they get the crop and the note and nothing invented. + */ export function packageAnnotatePin(pin: AnnotatePin): ComposerReadyAnnotation { const target = identityBlock(pin) const note = pin.note.trim() + const identity = pin.identity const prompt = [ `Comment ${pin.number}`, `Target: ${target}`, + identity?.selector ? `Selector: ${identity.selector}` : '', + identity?.html ? `HTML: ${identity.html}` : '', + identity ? cssBlock(identity) : '', note ? `Note: ${note}` : '', `Image ${pin.number} marks the target in blue.` ] @@ -31,7 +54,7 @@ export function packageAnnotatePin(pin: AnnotatePin): ComposerReadyAnnotation { .join('\n') return { - identity: pin.identity, + identity, imageDataUrl: pin.imageDataUrl, note, number: pin.number, diff --git a/apps/desktop/src/lib/preview-annotate/stack.test.ts b/apps/desktop/src/lib/preview-annotate/stack.test.ts index e4edb80697..da1da95d3b 100644 --- a/apps/desktop/src/lib/preview-annotate/stack.test.ts +++ b/apps/desktop/src/lib/preview-annotate/stack.test.ts @@ -25,7 +25,13 @@ function draft(note: string, kind: 'area' | 'element' = 'element'): AnnotatePinD rect: { height: 24, width: 80, x: 10, y: 12 }, identity: kind === 'element' - ? { css: { 'font-size': '14px' }, selector: 'button.go', tag: 'button', text: 'Go' } + ? { + css: { 'font-size': '14px' }, + html: '', + selector: 'button.go', + tag: 'button', + text: 'Go' + } : undefined } } diff --git a/apps/desktop/src/lib/preview-annotate/stack.ts b/apps/desktop/src/lib/preview-annotate/stack.ts index 2c0a2ea152..f25c1faefd 100644 --- a/apps/desktop/src/lib/preview-annotate/stack.ts +++ b/apps/desktop/src/lib/preview-annotate/stack.ts @@ -1,3 +1,5 @@ +import type { CompactIdentity } from './identity' + /** * Numbered pin stack for comment mode. Saving a pin only appends — it never * sends a turn. Numbers are assigned 1..N in add order and stay put if a pin @@ -13,12 +15,8 @@ export interface AnnotateRect { y: number } -export interface AnnotateIdentity { - css: Record - selector: string - tag: string - text: string -} +/** What a pin knows about its element. One shape, owned by `identity`. */ +export type AnnotateIdentity = CompactIdentity export interface AnnotatePin { id: string diff --git a/apps/desktop/src/lib/preview-annotate/tokens.ts b/apps/desktop/src/lib/preview-annotate/tokens.ts index cafbbbb427..73f255deb1 100644 --- a/apps/desktop/src/lib/preview-annotate/tokens.ts +++ b/apps/desktop/src/lib/preview-annotate/tokens.ts @@ -34,14 +34,28 @@ export const ANNOTATE_CSS_KEYS = [ 'position', 'width', 'height', + 'max-width', 'padding', 'margin', + 'border', 'border-radius', + 'box-shadow', 'opacity', + 'overflow', + 'z-index', + 'transform', 'flex-direction', 'gap', + 'grid-template-columns', 'justify-content', 'align-items' ] as const export type AnnotateCssKey = (typeof ANNOTATE_CSS_KEYS)[number] + +/** + * Markup budget for one comment. Enough for the opening tag plus a couple of + * levels of children — the part an agent greps a component out of — without + * pasting a whole section into the composer. + */ +export const ANNOTATE_HTML_BUDGET = 600 diff --git a/website/docs/user-guide/desktop.md b/website/docs/user-guide/desktop.md index 87a4e6f419..82156a947e 100644 --- a/website/docs/user-guide/desktop.md +++ b/website/docs/user-guide/desktop.md @@ -44,7 +44,7 @@ The center of the app. You get: - **The same conversation history** as every other Hermes surface — sessions started here resume in the CLI/TUI and vice versa. - **Drag-and-drop files** anywhere in the chat area to attach them to your next message. - **A right-hand preview rail** — render web pages, files, and tool outputs side by side while you keep chatting. -- **Comment mode in the in-app browser** — click **Annotate** in the preview browser bar, then click any element (or drag a box) on the live page and type a note; each saved comment stays as a numbered pin on the page. Saving a pin never sends a turn — when you're done, **Add N comments** attaches a cropped screenshot per pin and a short prompt naming each comment to the composer, and you still hit send yourself. Pin numbers hold steady if you delete one, and switching chats clears the stack. +- **Comment mode in the in-app browser** — click **Annotate** in the preview browser bar, then click any element (or drag a box) on the live page and type a note; each saved comment stays as a numbered pin on the page. Saving a pin never sends a turn — when you're done, **Add N comments** attaches a cropped screenshot per pin and a short prompt naming each comment to the composer, and you still hit send yourself. Each element comment carries its CSS selector, its markup, and the computed styles that matter for layout, so the agent can find the element in your source instead of guessing from the picture. Password and hidden field values, and any attribute that looks like a key or token, are redacted on the page before the markup leaves it. Pin numbers hold steady if you delete one, and switching chats clears the stack. - **Composer history and queue editing** — press the up/down arrow keys in an empty composer to recall and reuse previous prompts, and edit messages you've queued up before they're sent. Pressing Stop (or Esc) while turns are queued pauses the queue and expands it above the composer; resume it from there, or send, edit, and delete individual entries. - **A conversation timeline rail** — long chats get a slim rail of markers along the edge of the transcript, one per prompt. Hover it to pop open the list of prompts, click one to jump straight to that point in the conversation. (It appears once the chat has a handful of turns.) - **Find in page** — press **Cmd/Ctrl+F** to open a find bar that searches the rendered chat transcript. Enter / Shift+Enter (or Cmd/Ctrl+G / Cmd/Ctrl+Shift+G while the bar is open) step through matches; Esc closes it.