diff --git a/src/tui-opentui/command-surfaces.test.ts b/src/tui-opentui/command-surfaces.test.ts index 13dca494a..57fd6e72b 100644 --- a/src/tui-opentui/command-surfaces.test.ts +++ b/src/tui-opentui/command-surfaces.test.ts @@ -166,6 +166,27 @@ describe("settings surface", () => { }) }) + test("choosing a cycled row writes a plain-English transcript line, not the internal echo", async () => { + await withShell(async (shell) => { + const { deps } = settingsDeps() + openCommandSurface(shell, "settings", deps) + await Promise.resolve() + await Promise.resolve() + + cycleOverlaySelection(shell, 1) + await Promise.resolve() + await Promise.resolve() + acceptOverlaySelection(shell) + + const row = shell.streamLog.at(-1) + expect(row?.text).toBe("Set compaction to drop.") + expect(row?.meta).not.toBe("overlay") + expect(row?.text).not.toContain("‹") + expect(row?.text).not.toContain("›") + expect(row?.text).not.toContain("overlay") + }) + }) + test("session mode scope switch honours a local write", async () => { await withShell(async (shell) => { const { deps, calls } = settingsDeps() diff --git a/src/tui-opentui/command-surfaces.ts b/src/tui-opentui/command-surfaces.ts index bc8b47254..dbe73fea8 100644 --- a/src/tui-opentui/command-surfaces.ts +++ b/src/tui-opentui/command-surfaces.ts @@ -251,6 +251,14 @@ function cycleValue(options: readonly T[], current: T, direction: -1 | 1): T return next ?? current } +/** The active option's plain label — the value an accept echo should report, not the row's painted display string. */ +function activeOptionLabel( + options: readonly CycleOption[], + activeId: T, +): string { + return options.find((o) => o.id === activeId)?.label ?? activeId +} + const COMPACTION_OPTIONS: readonly CycleOption[] = [ { id: "llm", label: "summarize" }, { id: "pruning", label: "drop" }, @@ -273,6 +281,8 @@ const SETTINGS_NAME_WIDTH = 16 type SettingsCycleRow = { readonly id: string readonly value: string + /** Plain value the row currently holds, for the accept echo — not the painted `value` string. */ + readonly chosenLabel: string readonly describe: ItemDescription readonly cycle: (direction: -1 | 1) => void } @@ -286,6 +296,7 @@ function settingsCycleRows( { id: "compaction", value: `${"compaction".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(COMPACTION_OPTIONS, snapshot.compactionMode)}`, + chosenLabel: activeOptionLabel(COMPACTION_OPTIONS, snapshot.compactionMode), describe: { what: "how the transcript is trimmed once the context fills.", impact: @@ -299,6 +310,7 @@ function settingsCycleRows( { id: "session-mode", value: `${"session mode".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(SESSION_MODE_OPTIONS, snapshot.sessionMode)}`, + chosenLabel: activeOptionLabel(SESSION_MODE_OPTIONS, snapshot.sessionMode), describe: { what: "single agent works in-session; orchestrator delegates through a worker fleet.", impact: "orchestrator can run sub-agents concurrently and costs more per turn.", @@ -313,6 +325,7 @@ function settingsCycleRows( { id: "session-scope", value: `${" scope".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(SESSION_SCOPE_OPTIONS, snapshot.sessionModeScope)}`, + chosenLabel: activeOptionLabel(SESSION_SCOPE_OPTIONS, snapshot.sessionModeScope), describe: { what: "whether the session mode above applies to every repo or just this one.", impact: "this repo writes a local override that takes precedence over the global default.", @@ -326,6 +339,7 @@ function settingsCycleRows( { id: "wait-for-approval", value: `${"approval wait".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(ON_OFF_OPTIONS, snapshot.waitForApproval ? "on" : "off")}`, + chosenLabel: activeOptionLabel(ON_OFF_OPTIONS, snapshot.waitForApproval ? "on" : "off"), describe: { what: "whether a tool's time budget pauses while waiting on your approval.", impact: "off counts the wait against the tool's timeout, so a slow approval can time it out.", @@ -336,6 +350,7 @@ function settingsCycleRows( { id: "telemetry", value: `${"telemetry".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(ON_OFF_OPTIONS, snapshot.telemetryEnabled ? "on" : "off")}`, + chosenLabel: activeOptionLabel(ON_OFF_OPTIONS, snapshot.telemetryEnabled ? "on" : "off"), describe: { what: "anonymous usage data shared to help improve corbits.", impact: "off stops all telemetry from this session.", @@ -346,6 +361,7 @@ function settingsCycleRows( { id: "prompt-cost", value: `${"show cost".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(ON_OFF_OPTIONS, snapshot.showPromptCost ? "on" : "off")}`, + chosenLabel: activeOptionLabel(ON_OFF_OPTIONS, snapshot.showPromptCost ? "on" : "off"), describe: { what: "shows the session's spend in the prompt border, next to the context percentage.", impact: "it's a running total that draws the eye every time it changes — off by default; /cost still gives the full breakdown on demand.", @@ -434,10 +450,17 @@ function renderSettingsMenu( ]) const ids = [...cycleRows.map((r) => r.id), ...navRows.map((r) => r.id)] const items = [...cycleRows.map((r) => r.value), ...navRows.map((r) => r.value)] + // Nav rows (permissions, plugins, hooks) open a sub-surface rather than + // holding a value of their own, so they carry no echo value. + const values: readonly (string | undefined)[] = [ + ...cycleRows.map((r) => r.chosenLabel), + ...navRows.map(() => undefined), + ] openSettingsOverlay(shell, { items, itemIds: ids, + itemValues: values, activeIndex: Math.min(activeIndex, Math.max(0, items.length - 1)), describe: (id) => descById.get(id) ?? null, onCycle: (id, direction) => { diff --git a/src/tui-opentui/overlays.test.ts b/src/tui-opentui/overlays.test.ts index 7a0410c34..4f486a801 100644 --- a/src/tui-opentui/overlays.test.ts +++ b/src/tui-opentui/overlays.test.ts @@ -18,6 +18,7 @@ import { closeInsetOverlay, createAppShell, moveOverlaySelection, + openListOverlay, pageOverlaySelection, relayout, setShellOverlayHooks, @@ -223,7 +224,8 @@ describe("model / provider picker", () => { await h.renderOnce() frame = h.captureCharFrame() - expect(frame).toContain("chose (model_picker)") + expect(frame).toContain("model picker") + expect(frame).toMatch(/Chose /) } finally { shell.dispose() } @@ -444,3 +446,35 @@ describe("resize mid-overlay", () => { ) }) }) + +describe("accept echo reads the chosen value structurally", () => { + test("a label containing its own ‹ › text does not corrupt the echo", async () => { + // A row whose display label happens to contain marker glyphs for reasons + // that have nothing to do with the cycled-field convention — the echo + // must still report the caller-supplied value, not something scraped + // back out of the label. + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + wireKeys: false, + }) + try { + openListOverlay(shell, { + kind: "settings", + items: ["server name ‹ prod › staging"], + itemIds: ["server"], + itemValues: ["prod"], + }) + acceptOverlaySelection(shell) + + const row = shell.streamLog.at(-1) + expect(row?.text).toBe("Set server to prod.") + } finally { + shell.dispose() + } + }, + { width: 80, height: 24 }, + ) + }) +}) diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index 520c0b2e5..6449ccc79 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -267,6 +267,8 @@ export type OverlaySelection = { readonly label: string /** Stable id when the host provided `itemIds`; otherwise omitted. */ readonly id?: string + /** Plain chosen value when the host provided `itemValues`; otherwise omitted. */ + readonly value?: string } /** @@ -1701,6 +1703,7 @@ type PriorOverlaySnapshot = { readonly title: string readonly paletteCommands: readonly PaletteCommand[] readonly itemIds: readonly string[] + readonly itemValues: readonly (string | undefined)[] readonly onAccept: ((selection: OverlaySelection) => void) | null readonly onToggleExpand: (() => void) | null readonly onCycle: ((itemId: string, direction: -1 | 1) => void) | null @@ -1720,6 +1723,8 @@ type ShellInternals = { priorOverlay: PriorOverlaySnapshot | null /** Optional stable ids aligned with overlayItems for the open primary. */ overlayItemIds: readonly string[] + /** Optional plain chosen values aligned with overlayItems for the open primary. */ + overlayItemValues: readonly (string | undefined)[] /** Per-open accept callback; cleared on close without invoke (Esc path). */ overlayOnAccept: ((selection: OverlaySelection) => void) | null /** False while an overlay that reports its own outcome is open. */ @@ -2963,6 +2968,13 @@ export type OpenListOverlayOpts = { readonly items?: readonly string[] /** Optional stable ids aligned with `items` (permission scope ids, model ids). */ readonly itemIds?: readonly string[] + /** + * Optional plain chosen-value aligned with `items`, for rows whose display + * label carries more than the value itself (a cycled field's name, padding, + * and `‹ ›` markers around the active option). The accept echo reads this + * instead of recovering the value by parsing the label back apart. + */ + readonly itemValues?: readonly (string | undefined)[] readonly body?: string readonly activeIndex?: number readonly frameId?: string @@ -3055,6 +3067,7 @@ export function openListOverlay( title: String(shell.overlayTitle.content), paletteCommands: shell.paletteCommands, itemIds: bag.overlayItemIds, + itemValues: bag.overlayItemValues, onAccept: bag.overlayOnAccept, onToggleExpand: bag.overlayOnToggleExpand, onCycle: bag.overlayOnCycle, @@ -3085,6 +3098,7 @@ export function openListOverlay( // Palette open does not own primary accept; leave prior snapshot's callback. if (!isPalette) { bag.overlayItemIds = opts?.itemIds ? [...opts.itemIds] : [] + bag.overlayItemValues = opts?.itemValues ? [...opts.itemValues] : [] bag.overlayOnAccept = opts?.onAccept ?? null bag.overlayEchoChoice = opts?.echoChoice ?? true bag.overlayOnToggleExpand = opts?.onToggleExpand ?? null @@ -3095,6 +3109,7 @@ export function openListOverlay( } else if (!bag.priorOverlay) { // Bare palette (no primary under it): no accept payload. bag.overlayItemIds = opts?.itemIds ? [...opts.itemIds] : [] + bag.overlayItemValues = opts?.itemValues ? [...opts.itemValues] : [] bag.overlayOnAccept = opts?.onAccept ?? null bag.overlayEchoChoice = opts?.echoChoice ?? true bag.overlayOnToggleExpand = opts?.onToggleExpand ?? null @@ -3347,7 +3362,7 @@ export function handleOverlayAnswerKey( appendStreamRow(shell, { role: "system", text: `answered: ${text}`, - meta: "overlay", + meta: overlayKindWord(shell.overlayKind ?? "operator"), }) // Deliberate submit, not a dismiss — closeInsetOverlay must not also fire // the Esc/cancel path. @@ -3457,6 +3472,7 @@ export function closeInsetOverlay(shell: AppShell): void { // captured before this clears, and is invoked separately once state settles). if (bag && !prior) { bag.overlayItemIds = [] + bag.overlayItemValues = [] bag.overlayOnAccept = null bag.overlayOnToggleExpand = null bag.overlayOnCycle = null @@ -3485,6 +3501,7 @@ export function closeInsetOverlay(shell: AppShell): void { shell.paletteCommands = prior.paletteCommands shell.overlayTitle.content = prior.title bag.overlayItemIds = prior.itemIds + bag.overlayItemValues = prior.itemValues bag.overlayOnAccept = prior.onAccept bag.overlayOnToggleExpand = prior.onToggleExpand bag.overlayOnCycle = prior.onCycle @@ -3629,11 +3646,13 @@ export function setOverlayItems( shell: AppShell, items: readonly string[], itemIds?: readonly string[], + itemValues?: readonly (string | undefined)[], ): void { if (!shell.overlayList) return shell.overlayItems = items const bag = internals.get(shell) if (bag && itemIds) bag.overlayItemIds = [...itemIds] + if (bag && itemValues) bag.overlayItemValues = [...itemValues] shell.overlayList = setListCount(shell.overlayList, items.length) paintOverlayList(shell) } @@ -3720,11 +3739,13 @@ export function acceptOverlaySelection(shell: AppShell): void { const bag = internals.get(shell) const id = bag?.overlayItemIds[idx] + const value = bag?.overlayItemValues[idx] const selection: OverlaySelection = { kind, index: idx, label, ...(id !== undefined ? { id } : {}), + ...(value !== undefined ? { value } : {}), } // Capture before close clears per-open state. const perOpen = bag?.overlayOnAccept ?? null @@ -3735,14 +3756,38 @@ export function acceptOverlaySelection(shell: AppShell): void { if (bag?.overlayEchoChoice !== false) { appendStreamRow(shell, { role: "system", - text: `chose (${kind}): ${label}`, - meta: "overlay", + text: overlayChoiceText(label, id, value), + meta: overlayKindWord(kind), }) } closeInsetOverlay(shell) dispatchOverlayAccept(shell, selection, perOpen) } +/** + * Plain-English echo of an accepted choice. A cycled settings field's label + * carries every option with `‹ ›` around the active one (list-painting detail, + * not something an operator asked for), so the caller passes the value that + * actually won structurally via `itemValues` rather than leaving it to be + * recovered from the rendered label — a marker or spacing change, or a label + * that legitimately contains `‹`/`›`, would otherwise corrupt the echo + * silently. A plain list item has no separate value, so it is quoted as-is. + */ +function overlayChoiceText( + label: string, + id: string | undefined, + value: string | undefined, +): string { + if (value === undefined) return `Chose ${label.trim()}.` + const field = id === undefined ? "setting" : id.replace(/[-_]/g, " ") + return `Set ${field} to ${value}.` +} + +/** Internal overlay kinds read as words in the transcript, not identifiers. */ +function overlayKindWord(kind: PrimaryOverlayKind): string { + return kind.replace(/_/g, " ") +} + /** * Dispatch a selected palette item after the palette has closed. * - residual → `runPaletteAction` (overlays / chrome) @@ -3843,8 +3888,8 @@ export function runPaletteAction( }) appendStreamRow(shell, { role: "system", - text: on ? "goal chrome off" : "goal chrome on", - meta: "chrome", + text: on ? "goal banner off" : "goal banner on", + meta: "goal", }) return } @@ -3856,8 +3901,8 @@ export function runPaletteAction( }) appendStreamRow(shell, { role: "system", - text: on ? "task chrome off" : "task chrome on", - meta: "chrome", + text: on ? "task banner off" : "task banner on", + meta: "task", }) return } @@ -3870,7 +3915,7 @@ export function runPaletteAction( appendStreamRow(shell, { role: "system", text: on ? "agents strip off" : "agents strip on", - meta: "chrome", + meta: "agents", }) return } @@ -4174,6 +4219,8 @@ export type OpenResidualListOpts = { readonly items?: readonly string[] /** Stable ids aligned with `items` (setting keys, session ids, paths). */ readonly itemIds?: readonly string[] + /** Plain chosen value aligned with `items`, for the accept echo (see `OpenListOverlayOpts.itemValues`). */ + readonly itemValues?: readonly (string | undefined)[] readonly activeIndex?: number /** Per-open accept; host binds toggle / resume / mention insert. */ readonly onAccept?: (selection: OverlaySelection) => void @@ -4194,6 +4241,7 @@ export function openSettingsOverlay( activeIndex: opts?.activeIndex ?? 0, frameId: "overlay-settings", ...(opts?.itemIds !== undefined ? { itemIds: opts.itemIds } : {}), + ...(opts?.itemValues !== undefined ? { itemValues: opts.itemValues } : {}), ...(opts?.onAccept !== undefined ? { onAccept: opts.onAccept } : {}), ...(opts?.onCycle !== undefined ? { onCycle: opts.onCycle } : {}), ...(opts?.describe !== undefined ? { describe: opts.describe } : {}), @@ -5397,6 +5445,7 @@ export function createAppShell( overlayBodyRows: undefined, priorOverlay: null, overlayItemIds: [], + overlayItemValues: [], overlayOnAccept: null, overlayEchoChoice: true, overlayOnToggleExpand: null,