Better parsing of pasted html as markdown so agents understand - #9190
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f11978bc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const inner = Array.from(el.childNodes).map(convertNodeToMarkdown).join(''); | ||
| return inner + '\n'; | ||
| } | ||
| return Array.from(el.childNodes).map(convertNodeToMarkdown).join(''); |
There was a problem hiding this comment.
Ignore non-content HTML nodes when building markdown
The fallback branch recursively concatenates child text for every unrecognized element, so pasted text/html that contains <style>, <script>, or other non-visible nodes (common in Office/Docs clipboard payloads) will inject CSS/metadata text into the chat input instead of just user-visible content. This breaks the new link-preservation flow for those sources because the inserted message can contain large blocks of garbage text along with the links.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 734fae43ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const inner = Array.from(el.childNodes).map(convertNodeToMarkdown).join(''); | ||
| return inner + '\n'; | ||
| } | ||
| return Array.from(el.childNodes).map(convertNodeToMarkdown).join(''); |
There was a problem hiding this comment.
Preserve separators for non-paragraph block elements
When rich-text paste contains links inside blocks other than <p>/<div> (for example <li>, table cells, or headings), the fallback branch concatenates child markdown with no delimiter, so structurally separate items collapse together (e.g. two linked list items become [A](...)[B](...)). This regresses readability versus normal paste and can materially alter the meaning of pasted content whenever link-bearing lists/tables are copied from Docs/Office pages.
Useful? React with 👍 / 👎.
| setDisplayValue(newValue); | ||
| updateValue(newValue); | ||
| setHasUserTyped(true); |
There was a problem hiding this comment.
Recompute mention popover state after link paste
This custom paste path updates displayValue directly but skips checkForMentionOrSlash, which is the only place that closes/updates mention state from text changes. If the mention popover is open (e.g. user typed @) and then pastes linked HTML, mentionPopover can stay open with stale query/position even when the new content no longer contains an active mention trigger.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eba0844dd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (doc.querySelectorAll('a[href]').length === 0) { | ||
| return null; | ||
| } | ||
| return convertNodeToMarkdown(doc.body).replace(/\n{3,}/g, '\n\n').trim(); |
There was a problem hiding this comment.
Return null when sanitized HTML yields no markdown
htmlToMarkdown decides to intercept paste based on querySelectorAll('a[href]') before removing non-content nodes, so HTML that contains links only inside stripped elements (for example, an <a> nested under <svg>) returns an empty string instead of null. In ChatInput, any non-null return triggers preventDefault(), which suppresses normal plain-text paste and can delete the current selection by replacing it with ''. This regresses paste behavior for those clipboard payloads even though there is no visible link content to preserve.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33c96f17ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| setDisplayValue(newValue); | ||
| updateValue(newValue); | ||
| setHasUserTyped(true); |
There was a problem hiding this comment.
Respect readOnly mode in rich-text paste path
The new handlePaste branch mutates displayValue/value directly after preventDefault() without checking whether the textarea is currently read-only. This bypasses the readOnly={isRecording} lock when dictation is active: in environments where paste still fires on a read-only textarea, pasting HTML that contains links will update the input even though typing is disabled. That creates inconsistent input-state behavior during recording and can inject unintended text into the pending message.
Useful? React with 👍 / 👎.
41ee340 to
3c81720
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c817208cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const newValue = | ||
| displayValue.substring(0, start) + markdown + displayValue.substring(end); | ||
| setDisplayValue(newValue); | ||
| updateValue(newValue); |
There was a problem hiding this comment.
Preserve native undo for transformed link pastes
When HTML with links is pasted, this branch calls preventDefault() and then replaces the textarea value via React state, which bypasses the browser’s normal edit transaction for paste operations. In this path, users can lose expected Ctrl/Cmd+Z behavior for the pasted content (or get coarser undo steps), which is a regression in editor usability specifically for link-bearing rich-text paste.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3aa395630a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const text = (el.textContent || '').trim().replace(/\n+/g, ' '); | ||
| return `[${text}](${href})`; |
There was a problem hiding this comment.
Use escaped link text in custom Turndown rule
The complexLinks replacement builds markdown from raw textContent ([${text}](${href})) and ignores Turndown’s already-converted content argument, so markdown metacharacters in multiline link labels (for example ], [ or *) are not escaped. In that case, pasting rich text can generate malformed markdown links or alter surrounding formatting, which defeats the goal of preserving link content reliably for copied Docs/Office text.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 165ddf1cee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, [droppedFiles.length, localDroppedFiles.length, onFilesProcessed, setLocalDroppedFiles]); | ||
|
|
||
| const handlePaste = async (evt: React.ClipboardEvent<HTMLTextAreaElement>) => { | ||
| if (isRecording) return; |
There was a problem hiding this comment.
Allow image pastes while recording
Returning immediately when isRecording is true exits handlePaste before the image branch runs, so screenshot/image paste is silently disabled during dictation even though attachment flows are still available through the file picker. In environments where paste events still fire on read-only textareas, this is a regression from previous behavior and creates inconsistent attachment UX specifically for paste-based workflows.
Useful? React with 👍 / 👎.
When pasting content with embedded hyperlinks (e.g., from Google Docs), the links were stripped because the textarea only receives plain text by default. Now the paste handler checks for HTML clipboard data containing links and converts them to markdown format [text](url) before inserting. Since the output side already renders markdown via ReactMarkdown, the full round-trip works: pasted links are preserved in the input, sent to Goose, and rendered as clickable links in responses. Fixes #6079 Signed-off-by: Douwe Osinga <douwe@squareup.com>
… to markdown Clipboard payloads from Office/Google Docs often include <style>, <script>, <meta>, and other non-visible elements. The recursive fallback in convertNodeToMarkdown was including their text content, injecting CSS and metadata garbage into the chat input. Skip these non-content tags. Signed-off-by: Douwe Osinga <douwe@squareup.com>
- Extract HTML-to-markdown conversion into pasteMarkdown.ts - Normalize tagName to uppercase (SVG/MathML elements are lowercase in DOM) - Use double newline after block elements for proper paragraph spacing - Remove redundant comment - Add 14 unit tests covering links, non-content tags, nesting, and edge cases Signed-off-by: Douwe Osinga <douwe@squareup.com>
Replace hand-rolled HTML-to-markdown converter with turndown library. This properly handles lists, headings, bold, italic, line breaks, and all other block-level elements instead of just links and paragraphs. Remove the unit test file — turndown is well-tested upstream. Signed-off-by: Douwe Osinga <douwe@squareup.com>
Signed-off-by: Douwe Osinga <douwe@squareup.com>
Links wrapping block-level content (divs, lists, headings) produce multi-line text that breaks markdown link syntax. Add a turndown rule that detects these cases and collapses the content to a single line. Signed-off-by: Douwe Osinga <douwe@squareup.com>
- Skip paste entirely when recording (readOnly mode) - Check for empty markdown after turndown conversion to avoid suppressing normal paste when links are inside stripped elements - Call checkForMentionOrSlash after paste to dismiss stale popover Signed-off-by: Douwe Osinga <douwe@squareup.com>
Use the already-escaped content parameter instead of raw textContent, so markdown metacharacters in link labels are properly escaped. Signed-off-by: Douwe Osinga <douwe@squareup.com>
165ddf1 to
c6e3aa3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6e3aa31ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const href = el.getAttribute('href')!; | ||
| const label = content.replace(/\n+/g, ' ').trim(); | ||
| return `[${label}](${href})`; |
There was a problem hiding this comment.
Escape multiline link URLs before emitting markdown
The complexLinks rule builds markdown links with a raw href ([${label}](${href})), so pasted anchors whose URLs contain markdown-significant characters (for example unmatched ) or whitespace) can produce malformed markdown and broken link destinations. This path is triggered specifically for multiline link text, so rich-text content from sources like docs/wiki pages can be converted into invalid links instead of preserving the original target reliably.
Useful? React with 👍 / 👎.
* main: (102 commits) Dynamically refresh skill instructions each turn (#9217) Build non-vulkan linux variants using ubuntu 22.04 (#9211) fix(ui): show tool name in approval prompt (#9216) feat: add Atomic Chat as declarative OpenAI-compatible provider (#9210) chore: bump package.json versions from 0.19.1 to 0.20.0 (#9218) feat: support GOOSE_OAUTH_CALLBACK_PORT for stable OAuth redirect_uri (#9209) [RFC] feat(oauth): proactive token refresh to avoid re-auth on every session (#8386) fix: resolve Azure CLI on Windows by using az.cmd (#9215) fix: handle non-interactive terminal in goose configure on Windows (#9214) Better parsing of pasted html as markdown so agents understand (#9190) fix: persist accumulated cost in session DB to survive reload (#9191) fix(publish-npm): build binary from current SHA + add compat check (#9212) feat(desktop): add goose://new-session deep link to open fresh chat (#9196) Add PR previews using cloudflare pages (#9208) fix: prevent tool-use marker leakage in toolshim output (#8310) Prompt injection mitigation: update pattern-based detection (#9198) remove goose2 related skills (#9189) Switch GH pages deploy to actions/artifact workflow (#9025) fix(summon): re-apply canonical limits when delegate overrides model (#9183) Split code signing from build (#8587) ...
…goose#9190) Signed-off-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Douwe Osinga <douwe@squareup.com>
This does a better job of pasting html into a conversation so the agents can understand it (and renders also better)
Fixes #6079