release: v6.8.5 - #54
Conversation
There was a problem hiding this comment.
🧪 PR Review is completed: Solid release: toast/selection-copy, scroll-to-bottom chip, prompt windowing, and OSC 52 clipboard support are well-tested. Main concerns: the toast reports success even when copying fails, the prompt windowing counts logical lines while height accounting counts wrapped rows (layout overflow risk), the chip fires onClick twice per click, and the transcript no longer uses Yoga bottom-anchoring despite the retained comment claiming it does. Reviewed package.json, src/ui/components/Toast.tsx, src/ui/components/rows.tsx, src/ui/components/TranscriptViewport.tsx, and test/ui-viewport.test.tsx: no issues found.
Skipped files
CHANGELOG.md: Skipped file pattern
⬇️ Low Priority Suggestions (4)
src/ui/App.tsx (2 suggestions)
Location:
src/ui/App.tsx(Lines 417-418)🟡 Logic Error
Issue:
copyToClipboardreturns a boolean indicating whether any mechanism (native utility or OSC 52) succeeded, but the return value is ignored. When no clipboard tool is available (e.g., Linux without wl-copy/xclip/xsel and a terminal that doesn't support OSC 52), the user still sees "✓ Text copied to clipboard" — a false success message.Fix: Only show the toast when
copyToClipboardreturns true.Impact: Prevents misleading feedback; users are no longer told text was copied when it wasn't.
- copyToClipboard(text, renderer); - showToast(termCols < 40 ? "✓ Text copied" : "✓ Text copied to clipboard"); + if (copyToClipboard(text, renderer)) { + showToast(termCols < 40 ? "✓ Text copied" : "✓ Text copied to clipboard"); + }Location:
src/ui/App.tsx(Lines 1957-1961)🟡 Needs Discussion
Issue: The transcript viewport is now hardcoded to
anchorToBottom={false}with an unconditionalmarginTop={virtualTranscriptMarginTop}, andgetTranscriptPlacementwas rewritten soscrollOffset === 0yieldsmarginTop: -maxScrollOffset. Bottom-alignment at the live edge is now driven entirely by estimated heights (transcriptHeight/rowsHeightfromestimateRowLines). However, the retained comment directly above (lines 1784-1786) still states that Yoga should align the rendered content at the live edge because estimates drift when rows wrap to more lines than estimated. Withoverflow: hiddenand estimate-driven negative margins, any wrap-estimate drift can now clip the final response line behind the composer — the exact regression that comment describes. If this redesign is intentional (e.g., for consistent scroll-chip positioning), the stale comment should be updated and drift tolerance verified; otherwise the flex-end anchoring should be preserved whenscrollOffset === 0.Impact: Risk of the last streamed line being cut off or gapped above the composer when row-height estimates are off.
- <TranscriptViewport anchorToBottom={false}> - <Box - flexDirection="column" - flexShrink={0} - marginTop={virtualTranscriptMarginTop} +
src/ui/components/InputBox.tsx (1 suggestion)
Location:
src/ui/components/InputBox.tsx(Lines 542-582)🟡 Logic Error
Issue:
promptHeightis capped atMAX_PROMPT_HEIGHT(8) wrapped rows (each line contributesceil(line.length / editableWidth)), butdisplayContentwindows by logical lines (allLines.length <= MAX_PROMPT_HEIGHT/slice(startLine, startLine + MAX_PROMPT_HEIGHT)). When lines wrap — e.g., a long prompt recalled from history, or 8+ lines where some wrap — the rendered content can occupy far more rows than therenderedHeightreported to the parent viaonHeightChange. The input box then overflows its allotted space and covers the live response above it — exactly the bug this PR fixes elsewhere.Fix: Window by wrapped-row count (same
rowsFormath as the height calculation), expanding outward from the cursor line until 8 rows are used, so the rendered height always matches the cappedpromptHeight.Impact: Reported height matches actual rendered height; the composer can no longer overflow and obscure the transcript.
- const displayContent = useMemo(() => { - if (!active) { - return <Text color={COLORS.dim}>{value || "waiting…"}</Text> - } - const allLines = value.split("\n") - if (allLines.length <= MAX_PROMPT_HEIGHT) { - return ( - <> - {value.slice(0, cursor)} - <Text underline>{value[cursor] ?? " "}</Text> - {value.slice(cursor + 1)} - </> - ) - } - const linesBeforeCursor = value.slice(0, cursor).split("\n") - const cursorLineIdx = linesBeforeCursor.length - 1 - const startLine = Math.max( - 0, - Math.min(cursorLineIdx - Math.floor(MAX_PROMPT_HEIGHT / 2), allLines.length - MAX_PROMPT_HEIGHT), - ) - const endLine = startLine + MAX_PROMPT_HEIGHT - const windowedLines = allLines.slice(startLine, endLine) - - let charOffset = 0 - for (let i = 0; i < startLine; i++) { - charOffset += allLines[i].length + 1 - } - const windowedText = windowedLines.join("\n") - const relCursor = cursor - charOffset - - if (relCursor >= 0 && relCursor <= windowedText.length) { - return ( - <> - {windowedText.slice(0, relCursor)} - <Text underline>{windowedText[relCursor] ?? " "}</Text> - {windowedText.slice(relCursor + 1)} - </> - ) - } - return <>{windowedText}</> - }, [active, value, cursor]) + const displayContent = useMemo(() => { + if (!active) { + return <Text color={COLORS.dim}>{value || "waiting…"}</Text> + } + const rowsFor = (line: string) => Math.max(1, Math.ceil(Math.max(1, line.length) / editableWidth)) + const allLines = value.split("\n") + const cursorLineIdx = value.slice(0, cursor).split("\n").length - 1 + let startLine = 0 + let endLine = allLines.length + let rowsUsed = allLines.reduce((sum, line) => sum + rowsFor(line), 0) + if (rowsUsed > MAX_PROMPT_HEIGHT) { + startLine = cursorLineIdx + endLine = cursorLineIdx + 1 + rowsUsed = rowsFor(allLines[cursorLineIdx] ?? "") + while (rowsUsed < MAX_PROMPT_HEIGHT && (startLine > 0 || endLine < allLines.length)) { + const rowsUp = startLine > 0 ? rowsFor(allLines[startLine - 1] ?? "") : Infinity + const rowsDown = endLine < allLines.length ? rowsFor(allLines[endLine] ?? "") : Infinity + if (rowsUp <= rowsDown && rowsUp !== Infinity) { + startLine -= 1 + rowsUsed += rowsUp + } else if (rowsDown !== Infinity) { + endLine += 1 + rowsUsed += rowsDown + } else { + break + } + } + } + const windowedLines = allLines.slice(startLine, endLine) + + let charOffset = 0 + for (let i = 0; i < startLine; i++) { + charOffset += allLines[i].length + 1 + } + const windowedText = windowedLines.join("\n") + const relCursor = cursor - charOffset + + if (relCursor >= 0 && relCursor <= windowedText.length) { + return ( + <> + {windowedText.slice(0, relCursor)} + <Text underline>{windowedText[relCursor] ?? " "}</Text> + {windowedText.slice(relCursor + 1)} + </> + ) + } + return <>{windowedText}</> + }, [active, value, cursor, editableWidth])
src/ui/components/ScrollToBottomChip.tsx (1 suggestion)
Location:
src/ui/components/ScrollToBottomChip.tsx(Lines 30-54)🔵 Code Quality
Issue:
onClickis invoked from bothonMouseDownandonMouseUpon both the Box and the Text, so a single physical click dispatches the handler twice (and up to four times counting text/bubble duplication). The current App.tsx usage is idempotent (setScrollOffset(0)), so it's masked today, but any non-idempotent consumer of this reusable component will double-fire.Fix: Keep the press-driven handler (
onMouseDown) and remove the duplicateonMouseUphandlers.Impact: Guarantees exactly one invocation per click for all future consumers.
- onMouseDown={(e) => { - e.stopPropagation?.(); - onClick(); - }} - onMouseUp={(e) => { - e.stopPropagation?.(); - onClick(); - }} - onMouseMove={(e) => { - e.stopPropagation?.(); - if (!hovered) setHovered(true); - }} - > - <Text - color={hovered ? theme.accent : theme.dim} - bold={hovered} - selectable={false} - onMouseDown={(e) => { - e.stopPropagation?.(); - onClick(); - }} - onMouseUp={(e) => { - e.stopPropagation?.(); - onClick(); - }} + onMouseDown={(e) => { + e.stopPropagation?.(); + onClick(); + }} + onMouseMove={(e) => { + e.stopPropagation?.(); + if (!hovered) setHovered(true); + }} + > + <Text + color={hovered ? theme.accent : theme.dim} + bold={hovered} + selectable={false} + onMouseDown={(e) => { + e.stopPropagation?.(); + onClick(); + }} + >
- Show the selection-copy toast only when a clipboard mechanism actually succeeded (copyToClipboard return value was ignored). - Window the prompt display by wrapped rows using the same math as the promptHeight cap, and render the prompt glyph in its own column so text wraps at exactly editableWidth; rendered height now always matches the height reported via onHeightChange. Covered by a new regression test. - Fire ScrollToBottomChip onClick once per press (removed duplicate onMouseUp handlers). - Update the stale transcript-anchoring comment to describe the estimate-driven negative-margin layout and drop the unused anchorTranscriptToBottom variable.
|
All 4 review suggestions are addressed in df5e94f:
|
|
✅ Reviewed the changes: Release-round diff looks clean: the clipboard success check, wrapped-row prompt windowing, and single-fire chip handlers from the previous review are all correctly implemented, with a solid regression test added. Reviewed src/ui/App.tsx: no issues found. Reviewed src/ui/components/InputBox.tsx: no issues found. Reviewed src/ui/components/ScrollToBottomChip.tsx: no issues found. Reviewed test/ui-viewport.test.tsx: no issues found. Note: src/utils/clipboard.ts is not part of this diff, so the previously raised clipboard-command caching suggestion could not be re-verified here. |
…hang /quit, /exit, and Ctrl+D printed the 'Session saved' line but the process could stay alive when a background handle kept the event loop busy (an MCP child process still closing inside endAndExit's 3s cap, an in-flight fetch socket, or an FFF watcher), leaving the terminal stuck until Ctrl+C. The interactive path now calls process.exit(0) after the renderer is destroyed, matching the headless path's explicit exit.
|
Also fixed in 107c339: the terminal hang after |
|
✅ Reviewed the changes: Reviewed the explicit |
Release v6.8.5
Bumps
@matterailab/orbcodefrom6.8.4to6.8.5.Added
↓ Scroll to bottom) appears centered above the composer. Hovering highlights the chip, and clicking it (or pressing Esc when input is idle) snaps the viewport back to the live transcript edge.Fixed
marginTopderived frommaxScrollOffset, flexbox squashing is disabled on row wrappers, tab characters are expanded in user blocks, prompt input display height is capped, and multi-line/large pastes collapse into paste chips.Checklist
package.json(6.8.5)[6.8.5]v6.8.5on main and push the tag to trigger the npm publish workflow