Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions ui-tui/src/__tests__/clipboard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,9 +391,9 @@ describe('createCopyOnSelectReporter', () => {
// paste that silently came up empty.
const report = createCopyOnSelectReporter()

expect(report(42, 'osc52', 's1')).toBe(copyResultNotice(42, 'osc52'))
expect(report(42, 'osc52', 's1')).toBe('sent 42 characters')
expect(report(42, 'osc52', 's2')).toBe(copyResultNotice(42, 'osc52'))
expect(report(42, 'osc52', 's1')).toEqual({ text: copyResultNotice(42, 'osc52'), firstOfSession: true })
expect(report(42, 'osc52', 's1')).toEqual({ text: 'sent 42 characters', firstOfSession: false })
expect(report(42, 'osc52', 's2')).toEqual({ text: copyResultNotice(42, 'osc52'), firstOfSession: true })
})

it('does not repeat the caveat when a session is returned to', () => {
Expand All @@ -404,13 +404,19 @@ describe('createCopyOnSelectReporter', () => {
report(42, 'osc52', 's1')
report(42, 'osc52', 's2')

expect(report(42, 'osc52', 's1')).toBe('sent 42 characters')
expect(report(42, 'osc52', 's1')).toEqual({ text: 'sent 42 characters', firstOfSession: false })
})

it('keeps its own tally per reporter', () => {
// Two TUI processes must not share the fact that one of them has reported.
expect(createCopyOnSelectReporter()(42, 'osc52', 's1')).toBe(copyResultNotice(42, 'osc52'))
expect(createCopyOnSelectReporter()(42, 'osc52', 's1')).toBe(copyResultNotice(42, 'osc52'))
expect(createCopyOnSelectReporter()(42, 'osc52', 's1')).toEqual({
text: copyResultNotice(42, 'osc52'),
firstOfSession: true
})
expect(createCopyOnSelectReporter()(42, 'osc52', 's1')).toEqual({
text: copyResultNotice(42, 'osc52'),
firstOfSession: true
})
})
})

Expand Down
249 changes: 249 additions & 0 deletions ui-tui/src/__tests__/copyNotice.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,249 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2026 EverMind.
// See NOTICES.md.
//
// The copy-on-select confirmation: a transient line above the composer that
// replaces itself on every new copy and clears after its duration, instead of
// stacking permanent transcript lines.

import { renderSync } from '@hermes/ink'
import React from 'react'
import { PassThrough } from 'stream'
import { afterEach, describe, expect, it, vi } from 'vitest'

import type {
AppLayoutActions,
AppLayoutComposerProps,
AppLayoutProps,
AppLayoutStatusProps,
GatewayServices
} from '../app/interfaces.js'
import type { Msg } from '../types.js'

import { $copyNotice, dismissCopyNotice, showCopyNotice } from '../app/copyNoticeStore.js'
import { GatewayProvider } from '../app/gatewayContext.js'
import { patchUiState, resetUiState } from '../app/uiStore.js'
import { AppLayout } from '../components/appLayout.js'
import { DEFAULT_VOICE_RECORD_KEY } from '../lib/platform.js'
import { stripAnsi } from '../lib/text.js'

const HISTORY: Msg[] = Array.from({ length: 12 }, (_, i) => ({
role: i % 2 === 0 ? 'user' : 'assistant',
text: `transcript line ${i} lorem ipsum`
}))

const actions: AppLayoutActions = {
answerApproval: () => {},
answerClarify: () => {},
answerConfirm: () => {},
answerSecret: () => {},
answerSudo: () => {},
clearSelection: () => {},
deleteSessionWithFallback: async () => false,
onModelSelect: () => {},
resumeById: () => {},
setStickyPrompt: () => {}
}

const status: AppLayoutStatusProps = {
cwdLabel: '~/repo',
goodVibesTick: 0,
sessionStartedAt: null,
showStickyPrompt: false,
statusColor: 'green',
stickyPrompt: '',
turnStartedAt: null,
voiceLabel: ''
}

const makeComposer = (): AppLayoutComposerProps => ({
cols: 80,
compIdx: 0,
completions: [],
empty: true,
handleTextPaste: async () => null,
input: '',
inputBuf: [],
pagerPageSize: 10,
queueEditIdx: null,
queuedDisplay: [],
submit: () => {},
updateInput: () => {},
voiceRecordKey: DEFAULT_VOICE_RECORD_KEY
})

const gwServices = { gw: {}, rpc: async () => null } as unknown as GatewayServices

const makeProps = (): AppLayoutProps => ({
actions,
composer: makeComposer(),
mouseTracking: false,
progress: { showProgressArea: false },
status,
transcript: {
historyItems: HISTORY,
scrollRef: { current: null },
virtualHistory: {
bottomSpacer: 0,
end: HISTORY.length,
measureRef: () => () => {},
offsets: HISTORY.map((_, i) => i),
start: 0,
topSpacer: 0
},
virtualRows: HISTORY.map((msg, index) => ({ index, key: `r${index}`, msg }))
}
})

const App = () => (
<GatewayProvider value={gwServices}>
<AppLayout {...makeProps()} />
</GatewayProvider>
)

// The renderer skips blank cells by moving the cursor with CSI sequences
// instead of writing spaces, so 'copied 7 characters' arrives as
// 'copied<ESC>[1C7<ESC>[1Ccharacters'. Turn those into spaces before
// stripping the rest of the ANSI, so plain-text assertions see real gaps.
const cursorForward = new RegExp(`${String.fromCharCode(27)}\\[(\\d+)?C`, 'g')

const toPlainFrame = (raw: string): string =>
stripAnsi(raw.replace(cursorForward, (_, n) => ' '.repeat(n ? parseInt(n, 10) : 1)))

const renderFrame = ({ setup }: { setup?: () => void } = {}): string => {
resetUiState()
patchUiState({ statusBar: 'bottom' })
setup?.()

const stdout = new PassThrough()
const stdin = new PassThrough()
const stderr = new PassThrough()
let output = ''

Object.assign(stdout, { columns: 80, isTTY: true, rows: 24 })
Object.assign(stdin, { isTTY: true, ref: () => {}, setRawMode: () => {}, unref: () => {} })
Object.assign(stderr, { isTTY: true })
stdout.on('data', chunk => {
output += chunk.toString()
})

const instance = renderSync(<App />, {
patchConsole: false,
stderr: stderr as NodeJS.WriteStream,
stdin: stdin as NodeJS.ReadStream,
stdout: stdout as NodeJS.WriteStream
})

instance.unmount()
instance.cleanup()

return toPlainFrame(output)
}

describe('copy notice store', () => {
afterEach(() => {
vi.useRealTimers()
dismissCopyNotice()
})

it('clears itself after its duration', () => {
vi.useFakeTimers()

showCopyNotice('copied 7 characters', 3000)

expect($copyNotice.get()).toBe('copied 7 characters')

vi.advanceTimersByTime(2999)

expect($copyNotice.get()).toBe('copied 7 characters')

vi.advanceTimersByTime(1)

expect($copyNotice.get()).toBeNull()
})

it('replaces the previous notice instead of stacking', () => {
vi.useFakeTimers()

showCopyNotice('copied 7 characters', 3000)
showCopyNotice('sent 42 characters', 3000)

expect($copyNotice.get()).toBe('sent 42 characters')
})

it("lets a re-shown notice outlive the previous one's deadline", () => {
vi.useFakeTimers()

showCopyNotice('copied 7 characters', 3000)
vi.advanceTimersByTime(2900)
showCopyNotice('sent 42 characters', 5000)

// The first notice's deadline passes; the newer one must survive it.
vi.advanceTimersByTime(100)

expect($copyNotice.get()).toBe('sent 42 characters')

vi.advanceTimersByTime(4900)

expect($copyNotice.get()).toBeNull()
})

it('honours a per-notice duration', () => {
vi.useFakeTimers()

showCopyNotice('sent 42 characters', 5000)

vi.advanceTimersByTime(3000)

expect($copyNotice.get()).toBe('sent 42 characters')

vi.advanceTimersByTime(2000)

expect($copyNotice.get()).toBeNull()
})

it('dismissing clears the notice and leaves a later one to its own timer', () => {
vi.useFakeTimers()

showCopyNotice('copied 7 characters', 3000)
dismissCopyNotice()

expect($copyNotice.get()).toBeNull()

showCopyNotice('sent 42 characters', 3000)
vi.advanceTimersByTime(2999)

expect($copyNotice.get()).toBe('sent 42 characters')

vi.advanceTimersByTime(1)

expect($copyNotice.get()).toBeNull()
})
})

describe('copy notice rendering', () => {
afterEach(() => {
dismissCopyNotice()
})

it('shows the notice above the composer while one is set', () => {
const frame = renderFrame({ setup: () => showCopyNotice('copied 7 characters', 3000) })

expect(frame).toContain('copied 7 characters')

// At statusBar='bottom' the cwd label sits below the input box, so the
// notice must render somewhere above it.
expect(frame.indexOf('copied 7 characters')).toBeLessThan(frame.indexOf('~/repo'))
})

it('renders nothing for the notice once it has been dismissed', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case cannot fail for the reason its name gives. dismissCopyNotice() runs against a store that is already null (the afterEach above dismissed, and nothing in this test ever showed a notice), so the assertion is "a frame that was never given a notice does not contain one" -- true no matter what the component does with $copyNotice.

I checked rather than guessed: deleting the {copyNotice && <Text ...>} line from appLayout.tsx gives 1 failed | 6 passed in this file. The one that fails is the case above it; this one passes with the render line gone.

Showing a notice first would make it load-bearing and cover the transition the name promises:

const frame = renderFrame({
  setup: () => {
    showCopyNotice('copied 7 characters', 3000)
    dismissCopyNotice()
  }
})

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 3b0564f: the case now shows a notice and dismisses it, so it pins the show-then-dismiss transition. Probed the other way: removing dismissCopyNotice's atom clear turns this case red.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed, and I re-ran your probe rather than taking it on trust: deleting the $copyNotice.set(null) from dismissCopyNotice turns this case red along with the store-level one -- 2 failed | 5 passed (7). On the previous head only the store case failed, so the render case now genuinely pins the transition its name describes. Nothing further from me on this one.

const frame = renderFrame({
setup: () => {
showCopyNotice('copied 7 characters', 3000)
dismissCopyNotice()
}
})

expect(frame).not.toContain('copied 7 characters')
})
})
35 changes: 35 additions & 0 deletions ui-tui/src/app/copyNoticeStore.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* One transient notice above the composer.
*
* Copy-on-select writes the clipboard on every settled drag; reporting each
* one as a transcript line stacks permanent rows. This holds a single notice
* that replaces itself and clears after its duration, so the confirmation is
* visible for a moment and then gone.
*/

import { atom } from 'nanostores'

export const $copyNotice = atom<null | string>(null)

let dismissTimer: null | ReturnType<typeof setTimeout> = null

export function showCopyNotice(text: string, ms: number): void {
if (dismissTimer !== null) {
clearTimeout(dismissTimer)
}

$copyNotice.set(text)
dismissTimer = setTimeout(() => {
dismissTimer = null
$copyNotice.set(null)
}, ms)
}

export function dismissCopyNotice(): void {
if (dismissTimer !== null) {
clearTimeout(dismissTimer)
dismissTimer = null
}

$copyNotice.set(null)
}
20 changes: 15 additions & 5 deletions ui-tui/src/app/useMainApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import { buildToolTrailLine, sameToolTrailGroup, toolTrailLabel } from '../lib/text.js'
import { estimatedMsgHeight, messageHeightKey } from '../lib/virtualHeights.js'
import { createChatStream, type ChatStreamHandle, type ChatStreamRpcClient } from './chatStream.js'
import { showCopyNotice } from './copyNoticeStore.js'
import { createGatewayEventHandler } from './createGatewayEventHandler.js'
import { createSlashHandler } from './createSlashHandler.js'
import { getInputSelection } from './inputSelectionStore.js'
Expand Down Expand Up @@ -326,10 +327,13 @@
// mouse tracking, so copy-on-select is what makes a transcript selection
// copyable at all. That holds on every platform, not just macOS.
//
// Nothing on screen changes when a drag ends, so the transcript line is the
// only confirmation the clipboard was written. Lives below `sys` because the
// dependency array is evaluated during render, while `sys` is still in its
// temporal dead zone further up.
// Nothing on screen changes when a drag ends, so the report is the only
// confirmation the clipboard was written. The first copy of a session
// carries the path caveat and stays in the transcript: it is the answer a
// user comes looking for after a paste comes up empty minutes later, and it
// cannot pile up because it is once per session by construction. The terse
// repeats, which are unbounded, show as a transient notice above the
// composer instead of stacking rows.
//
// The path caveat is per session while this hook outlives any one session:
// `newSession()` and `resumeById()` replace `ui.sid` without remounting it,
Expand All @@ -342,7 +346,13 @@
useEffect(
() =>
subscribeCopyOnSelect(selection, (text, path) => {
sys(reportCopyOnSelect.current(graphemeCount(text), path, getUiState().sid ?? 'draft'))
const report = reportCopyOnSelect.current(graphemeCount(text), path, getUiState().sid ?? 'draft')

if (report.firstOfSession) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is the whole point of 64ed001, and nothing in the suite holds it in place. I inverted it -- caveat to showCopyNotice, terse repeats to sys -- and ran the full suite: 92 passed | 1 failed (93), the single failure being the two pre-existing textInputTypingBurst cases that are equally red on github/main here. Not one new failure. So the regression this PR exists to prevent (unbounded rows stacking under the conversation) could be reintroduced by swapping two lines and CI would wave it through.

The pieces either side are well covered -- clipboard.test.ts pins which report carries the caveat, copyNotice.test.tsx pins the store and the render -- and the gap is exactly the seam between them, because useMainApp is never rendered by any test.

There is precedent in this very file for the cheap fix: modelSelectCommand and buildChatStreamHandle are exported from useMainApp.ts specifically so tests can reach them without a live hook, and modelSelectCommand.test.ts / sessionManagement.test.ts import them directly. A deliverCopyReport(report, { showNotice, sys }) in the same style would make the seam a three-line test and needs no new harness.

Non-blocking -- the behaviour is right today, and this is about keeping it right. Reasonable to take as a follow-up if you would rather not grow the file's exported surface in this PR.

sys(report.text)
} else {
showCopyNotice(report.text, 3000)
}
}),
[selection, sys]
)
Expand Down Expand Up @@ -513,7 +523,7 @@

const label = toolTrailLabel('clarify')

turnController.turnTools = turnController.turnTools.filter(line => !sameToolTrailGroup(label, line))

Check warning on line 526 in ui-tui/src/app/useMainApp.ts

View workflow job for this annotation

GitHub Actions / TUI checks

Writing to a variable defined outside a component or hook is not allowed. Consider using an effect
patchTurnState({ turnTrail: turnController.turnTools })

rpc<ClarifyRespondResponse>('clarify.respond', { answer, request_id: clarify.requestId }).then(r => {
Expand Down
4 changes: 4 additions & 0 deletions ui-tui/src/components/appLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Fragment, memo, useMemo, useRef } from 'react'

import type { AppLayoutProps } from '../app/interfaces.js'

import { $copyNotice } from '../app/copyNoticeStore.js'
import { useGateway } from '../app/gatewayContext.js'
import { $isBlocked, $overlayState, patchOverlayState } from '../app/overlayStore.js'
import { $uiState } from '../app/uiStore.js'
Expand Down Expand Up @@ -178,6 +179,7 @@ const ComposerPane = memo(function ComposerPane({
}: Pick<AppLayoutProps, 'actions' | 'composer' | 'status'>) {
const ui = useStore($uiState)
const isBlocked = useStore($isBlocked)
const copyNotice = useStore($copyNotice)
const sh = (composer.inputBuf[0] ?? composer.input).startsWith('!')
const promptText = sh ? '$' : ui.theme.brand.prompt
const promptWidth = composerPromptWidth(promptText)
Expand Down Expand Up @@ -257,6 +259,8 @@ const ComposerPane = memo(function ComposerPane({

<StatusRulePane at="top" composer={composer} status={status} />

{copyNotice && <Text color={ui.theme.color.muted}>{copyNotice}</Text>}

{/* When a blocking overlay opens the input rows unmount, collapsing this
box to height 0. At statusBar='bottom' the StatusRule sibling then
shares its computed top, tripping the renderer's height-0 skip
Expand Down
Loading
Loading