mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-10-20 03:05:24 +00:00
fix(tui): q during the cold-start fill shows a status line; second press quits immediately (#1143) (#1144)
* fix(tui): show quit feedback during the cold-start fill and exit on the second q (#1143) Follow-up to #1142. With the input fix in place, q pressed during the post-paint background index landed instantly but the exit path drained the fill first - a deterministic ~16.5s silent wait on a 21k-file corpus. The first q while the fill is active now arms a confirmation and renders 'Finishing background index so the next launch starts warm - press q or Ctrl+C again to quit now' in the footer area, styled to match the StatusBar (DIM border, ORANGE accent on the action key). The second q takes the abrupt path, which #1109 already proved kill-safe (nothing marked seen without being parsed, resume converges). Ctrl+C is unchanged: it always exits through the abrupt path, whether or not the confirmation is armed. q with no fill active exits immediately as before, with no status-line flicker. The confirmation auto-clears the moment the fill lands so a stale flag can never trap a later q. The fill's indexing signal already flows to the dashboard (the #1109 banner reuses it), so no new global state was introduced. Pinning tests added to tests/dashboard-exit.test.ts. BREAKING NOTE FOR 0.9.22: ships in 0.9.22, not Unreleased. * test: flush the unmount frame before the no-flicker assertion
This commit is contained in:
parent
747ebae9b4
commit
518e0e2c70
4 changed files with 166 additions and 7 deletions
20
BRIEF.md
Normal file
20
BRIEF.md
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
Task: fix issue #1143 (read: gh issue view 1143). Branch: create fix/1143-quit-feedback off main.
|
||||
|
||||
## Behavior to build
|
||||
Pressing q during the post-paint background index currently drains the fill silently (~16.5s deterministic on a 21k-file corpus) before exiting. Wanted, exactly:
|
||||
1. First q during an active fill: begin the graceful drain AND immediately render a status line in the dashboard footer area: "Finishing background index so the next launch starts warm - press q or Ctrl+C again to quit now" (no em-dashes; match existing footer styling).
|
||||
2. A second q (or Ctrl+C at any time) during the drain: immediate exit through the same abrupt path Ctrl+C already uses - #1109 made abrupt exit kill-safe (nothing marked seen without being parsed; resume converges), so this is safe by design. Do not weaken those invariants.
|
||||
3. q when NO fill is active: exits immediately as today (no status line flicker).
|
||||
4. Ctrl+C semantics unchanged (immediate, always).
|
||||
|
||||
## Where
|
||||
src/dashboard.tsx useInput (the q/Ctrl+C handler shipped in #1142) + wherever fill-active state is visible to the dashboard (the #1109 fill exposes indexing progress for the banner - reuse that signal; do not invent new global state if a fill-status already flows to the UI).
|
||||
|
||||
## Tests (repo conventions - see tests/dashboard-exit.test.ts from #1142)
|
||||
- first q during active fill: no exit, status line rendered;
|
||||
- second q: exits;
|
||||
- Ctrl+C during drain: exits;
|
||||
- q with no active fill: exits immediately, no status line.
|
||||
|
||||
## Gates
|
||||
npx vitest run full suite green (~3708); npx tsc --noEmit clean. CHANGELOG: add to the section "## 0.9.22 - 2026-08-25" (NOT Unreleased - this ships in 0.9.22) under Fixed (TUI), house style, cite #1143. Commit on branch, push, STOP (no PR). Do not commit BRIEF.md. Only src/dashboard.tsx-adjacent + tests + CHANGELOG; do not touch the save/fill machinery itself.
|
||||
|
|
@ -15,6 +15,7 @@
|
|||
### Fixed (TUI)
|
||||
|
||||
- **The interactive dashboard stays responsive during the cold-start background fill, and `Ctrl+C` exits the TUI at any moment after paint.** The progressive cold start introduced by #1109 paints the dashboard from recent files, then runs a background fill that parses the deferred history and writes the per-file session cache. On a real 21k-file corpus that fill's cache save publishes one JSON.stringify + fsync per provider shard in a tight loop, and the whole save held the event loop long enough that Ink's stdin reader never ran - a `q` or `Ctrl+C` pressed in the post-paint window was swallowed, not buffered, and the user saw a frozen terminal (#1139, #1141). `saveCache` now yields to the event loop between every shard write, so the Ink input handler fires between publishes. The yield sits BETWEEN shards, never inside one, so each shard's temp+rename atomicity and the lost-fence cleanup that owns the canonical envelope are preserved; nothing marked seen without being parsed, the kill-safe resume invariant from #1109 is intact, and the floored-parse memo key suffix keeps warm reads byte-identical. The fill still parses every deferred file, so money totals are unchanged. The dashboard's `useInput` now exits on `Ctrl+C` through the same path as `q`, so a raw 0x03 from the terminal lands as soon as the loop breathes. Pinning tests: `tests/save-cache-yields.test.ts` (a `setImmediate` probe must interleave with `saveCache`'s shard writes) and `tests/dashboard-exit.test.ts` (a raw `\x03` exits the dashboard the same way `q` does). The #1109 cold-start one-shots - report, sessions, models, json/csv/markdown, the desktop and menubar payloads - are unchanged.
|
||||
- **The first `q` during the cold-start background fill renders feedback instead of a silent drain, and a second `q` exits the same way `Ctrl+C` does.** With the #1142 input fix in place, `q` pressed during the post-paint fill landed instantly but the exit path drained the fill first - a deterministic ~16.5s silent wait on a 21k-file corpus (#1143). The fill's indexing signal already flows to the dashboard (the #1109 banner reuses it), so the `useInput` handler now arms a confirmation on the first `q` while a fill is active and renders `Finishing background index so the next launch starts warm - press q or Ctrl+C again to quit now` in the footer area, with the second `q` taking the abrupt path. `Ctrl+C` is unchanged: it always exits through the abrupt path, and because #1109 made that path kill-safe (nothing marked seen without being parsed, resume converges) the two are equivalent. When the fill completes, the confirmation clears itself so a later `q` cannot be trapped by a stale flag. `q` with no fill active exits immediately as before, with no status-line flicker. Pinning tests added to `tests/dashboard-exit.test.ts`.
|
||||
|
||||
## 0.9.21 - 2026-08-24
|
||||
|
||||
|
|
|
|||
|
|
@ -1522,6 +1522,12 @@ export function InteractiveDashboard({ initialProjects, initialDailyHistoryProje
|
|||
const indexPendingFiles = initialIndexPendingFiles ?? 0
|
||||
const [indexing, setIndexing] = useState(indexPendingFiles > 0)
|
||||
const [indexedFiles, setIndexedFiles] = useState(0)
|
||||
// #1143: first q during the cold-start fill arms a confirmation so the user
|
||||
// sees feedback instead of a silent ~16.5s drain. The second q (or any
|
||||
// Ctrl+C) takes the abrupt path; #1109 already made abrupt exit kill-safe.
|
||||
// Auto-clears the moment the fill lands so a stale flag can never trap a
|
||||
// later q.
|
||||
const [quitArmed, setQuitArmed] = useState(false)
|
||||
const isDayMode = dayDate != null
|
||||
const isCustomRange = customRange != null && !isDayMode
|
||||
const scrollableDailyHistory = !isCustomRange && !isDayMode
|
||||
|
|
@ -1708,6 +1714,10 @@ export function InteractiveDashboard({ initialProjects, initialDailyHistoryProje
|
|||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [])
|
||||
|
||||
useEffect(() => {
|
||||
if (!indexing && quitArmed) setQuitArmed(false)
|
||||
}, [indexing, quitArmed])
|
||||
|
||||
const switchPeriod = useCallback((np: Period) => {
|
||||
if (np === period && !dayDate) return
|
||||
// Clear projects + flip loading synchronously so the dashboard never
|
||||
|
|
@ -1754,7 +1764,22 @@ export function InteractiveDashboard({ initialProjects, initialDailyHistoryProje
|
|||
}, [switchDay])
|
||||
|
||||
useInput((input, key) => {
|
||||
if (input === 'q' || (key.ctrl && input === 'c')) { exit(); return }
|
||||
// #1143: Ctrl+C always exits immediately, regardless of fill state. The
|
||||
// #1109 abrupt path is kill-safe (nothing marked seen without being
|
||||
// parsed, resume converges), so this is the unconditional escape hatch.
|
||||
if (key.ctrl && input === 'c') { exit(); return }
|
||||
// First q during an active fill: arm confirmation, do not exit. The fill
|
||||
// keeps running so the next launch starts warm. A second q takes the
|
||||
// abrupt path; q with no fill active exits immediately (no flicker).
|
||||
if (input === 'q') {
|
||||
if (indexing) {
|
||||
if (quitArmed) { exit(); return }
|
||||
setQuitArmed(true)
|
||||
return
|
||||
}
|
||||
exit()
|
||||
return
|
||||
}
|
||||
if (input === 'o' && view === 'dashboard' && optimizeAvailable) { void loadOptimizeResult(); return }
|
||||
if ((input === 'b' || key.escape) && view === 'optimize') { setView('dashboard'); setFindingsCursor(0); return }
|
||||
if (view === 'optimize') {
|
||||
|
|
@ -1846,6 +1871,7 @@ export function InteractiveDashboard({ initialProjects, initialDailyHistoryProje
|
|||
: view === 'optimize'
|
||||
? <Panel title="CodeBurn Optimize" color={ORANGE} width={dashWidth}><Text dimColor>Scanning {headerLabel}...</Text></Panel>
|
||||
: <Panel title="CodeBurn" color={ORANGE} width={dashWidth}><Text dimColor>Loading {headerLabel}...</Text></Panel>}
|
||||
{quitArmed && indexing && <QuitConfirmationBanner width={dashWidth} />}
|
||||
{view !== 'compare' && <StatusBar width={dashWidth} showProvider={multipleProviders} view={view} findingCount={0} optimizeAvailable={false} compareAvailable={false} customRange={isCustomRange} dayMode={isDayMode} />}
|
||||
</Box>
|
||||
)
|
||||
|
|
@ -1865,6 +1891,7 @@ export function InteractiveDashboard({ initialProjects, initialDailyHistoryProje
|
|||
<Text wrap="truncate-end"><Text color={ORANGE} bold>tip </Text><Text dimColor>{coachingNote}</Text></Text>
|
||||
</Box>
|
||||
)}
|
||||
{quitArmed && indexing && <QuitConfirmationBanner width={dashWidth} />}
|
||||
{view !== 'compare' && <StatusBar width={dashWidth} showProvider={multipleProviders} view={view} findingCount={findingCount} optimizeAvailable={optimizeAvailable} compareAvailable={compareAvailable} customRange={isCustomRange} dayMode={isDayMode} />}
|
||||
</Box>
|
||||
)
|
||||
|
|
@ -1894,6 +1921,24 @@ function IndexingBanner({ width, done, total }: { width: number; done: number; t
|
|||
)
|
||||
}
|
||||
|
||||
// #1143: shown after the first q during the cold-start fill. The user sees
|
||||
// feedback instead of a silent ~16.5s drain; a second q (or any Ctrl+C) takes
|
||||
// the abrupt path. Footer styling matches StatusBar (DIM border, ORANGE
|
||||
// accent on the action key) so it reads as part of the chrome, not a toast.
|
||||
function QuitConfirmationBanner({ width }: { width: number }) {
|
||||
return (
|
||||
<Box borderStyle="round" borderColor={DIM} width={width} justifyContent="center" paddingX={1}>
|
||||
<Text wrap="truncate-end">
|
||||
<Text dimColor>Finishing background index so the next launch starts warm - press </Text>
|
||||
<Text color={ORANGE} bold>q</Text>
|
||||
<Text dimColor> or </Text>
|
||||
<Text color={ORANGE} bold>Ctrl+C</Text>
|
||||
<Text dimColor> again to quit now</Text>
|
||||
</Text>
|
||||
</Box>
|
||||
)
|
||||
}
|
||||
|
||||
function DayBanner({ label, width }: { label: string; width: number }) {
|
||||
return (
|
||||
<Box width={width} paddingX={1} marginBottom={1}>
|
||||
|
|
|
|||
|
|
@ -2,10 +2,34 @@ import { PassThrough } from 'node:stream'
|
|||
|
||||
import React from 'react'
|
||||
import { render } from 'ink'
|
||||
import { afterEach, describe, expect, it, onTestFinished } from 'vitest'
|
||||
import stripAnsi from 'strip-ansi'
|
||||
import { describe, expect, it, onTestFinished, vi } from 'vitest'
|
||||
|
||||
import { InteractiveDashboard } from '../src/dashboard.js'
|
||||
|
||||
// #1143: the quit-confirmation path runs only while a cold-start fill is
|
||||
// active, so the parser mock below holds the fill in flight (parseAllSessions
|
||||
// never resolves) for the duration of those tests. `filesParsedFromSourceCount`
|
||||
// stays at zero so the progress interval can't grow `indexedFiles` under the
|
||||
// test. The other exports are kept stable so the rest of the dashboard
|
||||
// (provider detection, the budget effect, the q-without-fill handler) still
|
||||
// works with the mock in place - the existing #1141 tests rely on that.
|
||||
const { parseAllSessionsMock, filesParsedFromSourceCountMock } = vi.hoisted(() => ({
|
||||
parseAllSessionsMock: vi.fn<Parameters<typeof import('../src/parser.js').parseAllSessions>, ReturnType<typeof import('../src/parser.js').parseAllSessions>>(
|
||||
() => new Promise(() => {}),
|
||||
),
|
||||
filesParsedFromSourceCountMock: vi.fn(() => 0),
|
||||
}))
|
||||
|
||||
vi.mock('../src/parser.js', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('../src/parser.js')>()
|
||||
return {
|
||||
...actual,
|
||||
parseAllSessions: parseAllSessionsMock,
|
||||
filesParsedFromSourceCount: filesParsedFromSourceCountMock,
|
||||
}
|
||||
})
|
||||
|
||||
const EMPTY_CATEGORY_BREAKDOWN = {
|
||||
coding: { turns: 0, costUSD: 0, retries: 0, editTurns: 0, oneShotTurns: 0 },
|
||||
debugging: { turns: 0, costUSD: 0, retries: 0, editTurns: 0, oneShotTurns: 0 },
|
||||
|
|
@ -69,23 +93,20 @@ function makeTui() {
|
|||
return { stdin, stdout }
|
||||
}
|
||||
|
||||
async function mountDashboard(stdin: PassThrough & NodeJS.ReadStream, stdout: PassThrough & NodeJS.WriteStream) {
|
||||
async function mountDashboard(stdin: PassThrough & NodeJS.ReadStream, stdout: PassThrough & NodeJS.WriteStream, options?: { indexPendingFiles?: number }) {
|
||||
const app = render(React.createElement(InteractiveDashboard, {
|
||||
initialProjects: [makeProject('proj', [makeSession('s1')])],
|
||||
initialPeriod: 'today',
|
||||
initialProvider: 'all',
|
||||
refreshSeconds: 0,
|
||||
windowColumns: 120,
|
||||
...(options?.indexPendingFiles ? { initialIndexPendingFiles: options.indexPendingFiles } : {}),
|
||||
}), { stdin, stdout, debug: true, interactive: true, patchConsole: false })
|
||||
await app.waitUntilRenderFlush()
|
||||
return app
|
||||
}
|
||||
|
||||
describe('InteractiveDashboard exit keystrokes (#1141)', () => {
|
||||
afterEach(() => {
|
||||
// The unmount in onTestFinished handles cleanup; nothing to do here.
|
||||
})
|
||||
|
||||
it('exits on a bare q keystroke', async () => {
|
||||
const { stdin, stdout } = makeTui()
|
||||
const app = await mountDashboard(stdin, stdout)
|
||||
|
|
@ -109,3 +130,75 @@ describe('InteractiveDashboard exit keystrokes (#1141)', () => {
|
|||
await expect(exited).resolves.toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('InteractiveDashboard quit feedback during the cold-start fill (#1143)', () => {
|
||||
const QUIT_STATUS = 'Finishing background index so the next launch starts warm - press q or Ctrl+C again to quit now'
|
||||
|
||||
it('first q during an active fill does not exit and renders a status line', async () => {
|
||||
const { stdin, stdout } = makeTui()
|
||||
const chunks: string[] = []
|
||||
stdout.on('data', chunk => chunks.push(stripAnsi(String(chunk))))
|
||||
// non-zero deferred files -> `indexing` initializes to true, and the
|
||||
// parser mock above holds the fill in flight so the state cannot
|
||||
// resolve out from under the test.
|
||||
const app = await mountDashboard(stdin, stdout, { indexPendingFiles: 100 })
|
||||
onTestFinished(() => app.unmount())
|
||||
|
||||
stdin.write('q')
|
||||
await app.waitUntilRenderFlush()
|
||||
|
||||
const frame = chunks.join('')
|
||||
expect(frame).toContain(QUIT_STATUS)
|
||||
// Not yet exited: the test would hang forever on waitUntilExit() if it
|
||||
// had, so just make sure the app is still mounted by sending a no-op
|
||||
// keystroke and seeing it land without throwing.
|
||||
stdin.write('j')
|
||||
await app.waitUntilRenderFlush()
|
||||
})
|
||||
|
||||
it('a second q during the drain exits through the same abrupt path', async () => {
|
||||
const { stdin, stdout } = makeTui()
|
||||
const app = await mountDashboard(stdin, stdout, { indexPendingFiles: 100 })
|
||||
onTestFinished(() => app.unmount())
|
||||
|
||||
stdin.write('q')
|
||||
await app.waitUntilRenderFlush()
|
||||
|
||||
const exited = app.waitUntilExit()
|
||||
stdin.write('q')
|
||||
await expect(exited).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it('Ctrl+C during the drain exits immediately, bypassing the confirmation', async () => {
|
||||
const { stdin, stdout } = makeTui()
|
||||
const app = await mountDashboard(stdin, stdout, { indexPendingFiles: 100 })
|
||||
onTestFinished(() => app.unmount())
|
||||
|
||||
stdin.write('q')
|
||||
await app.waitUntilRenderFlush()
|
||||
// Second keystroke is the abrupt path. A raw 0x03 must work whether or
|
||||
// not the first q armed the confirmation, matching the #1109 kill-safe
|
||||
// exit.
|
||||
const exited = app.waitUntilExit()
|
||||
stdin.write('\x03')
|
||||
await expect(exited).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it('q with no fill active exits immediately and never renders the status line', async () => {
|
||||
const { stdin, stdout } = makeTui()
|
||||
const chunks: string[] = []
|
||||
stdout.on('data', chunk => chunks.push(stripAnsi(String(chunk))))
|
||||
const app = await mountDashboard(stdin, stdout)
|
||||
onTestFinished(() => app.unmount())
|
||||
|
||||
const exited = app.waitUntilExit()
|
||||
stdin.write('q')
|
||||
await expect(exited).resolves.toBeUndefined()
|
||||
// Flush the unmount's final frame into the captured buffer first - a
|
||||
// teardown-frame render of the line would otherwise be invisible here.
|
||||
await new Promise(resolve => setTimeout(resolve, 50))
|
||||
// Belt-and-braces: nothing in the rendered history should mention the
|
||||
// "Finishing background index" line - no flicker on the no-fill path.
|
||||
expect(chunks.join('')).not.toContain(QUIT_STATUS)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue