fix(ui): drop transient restore status and align MCP expand hint with configured keybinding

[lavaman131]

Summary

Removes the transient "Restored N queued message(s) to editor" status from both queued-message restore paths, and switches the MCP tool-result expand hint from a hardcoded CTRL+O Expand string to the actual configured app.tools.expand keybinding (matching how built-in tools like read, ls, and grep already render their expand hints).

Changes

• packages/coding-agent: removed the notifyChatSessionStatus/showStatus calls that surfaced Restored N queued message(s) to editor in restoreQueuedMessagesToEditor (chat-session-host-actions.ts) and the dequeue handler (interactive-process-lifecycle.ts), while preserving the underlying restore behavior and the existing "No queued messages to restore" status.
• packages/mcp: tool-result-renderer.ts now renders the collapsed-result expand hint via keyText("app.tools.expand") instead of the hardcoded CTRL+O string, and exports McpToolResultDetails for test coverage; README.md's usage table example was updated from CTRL+O Expand to Ctrl+o Expand to match.
• Added a status-suppression assertion to test/unit/chat-session-host-02.test.ts (asserts no "Restored ... queued message" status is shown) and a new test/unit/mcp-tool-result-renderer.test.ts covering the collapsed/expanded hint copy and truncation behavior.
• Added [Unreleased] changelog entries for packages/coding-agent and packages/mcp.

Validation

• bun test test/unit/chat-session-host-02.test.ts test/unit/mcp-tool-result-renderer.test.ts — 7 pass / 0 fail.
• bun run typecheck — passed.
• bun run check:file-length — passed.
• Pre-commit/pre-push hooks passed, including bun run lint, bun run check:file-length, and bun run test:unit.

[claude]

Review — LGTM ✅

Small, well-scoped cleanup. I traced both restore code paths and the MCP renderer change; everything is consistent and the tests are focused and meaningful.

Correctness

• The two restoreQueuedMessagesToEditor variants are distinct and both handled correctly:
  • InteractiveModeBase.prototype.restoreQueuedMessagesToEditor returns a number (count) — handleDequeue still guards restored === 0, so removing the else branch is type-safe and behavior-preserving.
  • The chat-session-host-actions.ts helper returns a boolean and retains its internal "No queued messages to restore" notify on the empty path.
• The empty-queue feedback survives in both paths; only the transient success copy is gone. Since the editor gets repopulated on a successful restore, the user still has visible feedback — reasonable UX call.
• Confirmed no leftover Restored … queued message copy in source and no remaining CTRL+O branding in packages/mcp.

Tests

• mcp-tool-result-renderer.test.ts is a nice addition — the case-sensitive negative guards (/CTRL\+O/, /Ctrl\+o Expand/) correctly pin the new branding, and the formatMcpToolResultLines truncation cases cover both collapsed and expanded budgets.
• The chat-session-host-02.test.ts update asserts against both statusText() and the captured showStatus messages — good defense against the copy silently returning.

Changelog

• Both [Unreleased] entries are correctly placed (appended to coding-agent's existing ### Fixed, new ### Fixed for mcp) and are descriptive per repo guidelines.

Minor (non-blocking) nits

• In mcp-tool-result-renderer.test.ts, McpDetails = Record<string, never> is slightly looser than the renderer's own McpToolResultDetails (Record<string, unknown> & { error?: unknown }). Harmless since {} is structurally assignable and typecheck passes, but reusing the renderer's detail type would keep the fixture honest to the details.error branch the renderer reads.
• Purely optional: the success status removal means no explicit confirmation line on restore. If discoverability ever comes up, the populated editor is the implicit signal — worth a mental note, not a change.

Nice, tight PR. 👍

[claude]

Review: fix(ui): clean up queued-restore status and Ctrl+o keybinding copy

Thanks for this — it's a focused, well-scoped cleanup with genuinely good test coverage for its size. Both source changes are low-risk and the new tests exercise real code paths. A few notes, none blocking.

Correctness / behavior — 👍

• Removing the Restored N queued message(s) status from both paths (restoreQueuedMessagesToEditor and handleDequeue) is consistent, and the actual restore behavior (setChatSessionEditorText + clearQueue + requestRender) is preserved. The No queued messages to restore branch is correctly kept.
• The ensureDeferredStartupComplete test fakes are accurate: both handleCloneCommand and handleClearCommand do await this.ensureDeferredStartupComplete() at the top, so asserting toHaveBeenCalledTimes(1) guards a real await. In first-run-onboarding.test.ts the assertion targets host.ensureDeferredStartupComplete (same object as hostWithClear via cast), so the spy reference lines up. Good.

Main suggestion: prefer the dynamic keybinding over a hardcoded string

The renderer hardcodes the hint text:

? `\n${theme.fg("muted", "(Ctrl+o Expand)")}`

The rest of the CLI derives this from the keybinding registry via keyText("app.tools.expand") / keyHint(...) (see read.ts, bash.ts, grep.ts, todos-render.ts, etc.). Those helpers are already exported from @bastani/atomic (which this package already imports), so this could be:

? `\n${theme.fg("muted", `(${keyText("app.tools.expand")} Expand)`)}`

Two benefits: it stays correct if a user rebinds expand (the hardcoded string silently lies after a rebind), and it inherits casing automatically. There's direct precedent for exactly this move in the changelog — "Use configurable expandTools keybinding instead of hardcoded Ctrl+O (#717)" — so a hardcoded expand key is a known anti-pattern here. It's pre-existing, but since the PR touches this exact line it's a natural place to finish the job.

Casing nit re: "CLI-wide Ctrl+o branding"

The keyText/keyHint helper lowercases everything (MODIFIER_LABELS maps ctrl → "ctrl", single letters are .toLowerCase()d), so the interactive UI actually renders ctrl+o, not Ctrl+o. So Ctrl+o reads nicer than the old CTRL+O, but it doesn't literally match what the CLI renders elsewhere — ctrl+o would. Adopting the dynamic helper above resolves this automatically. (Docs are inconsistent too: CTRL+O, Ctrl+O, and ctrl+o all appear.)

Tests — 👍

• mcp-tool-result-renderer.test.ts covers the collapsed hint, expanded (no hint), and the truncation boundary of formatMcpToolResultLines — and it exercises the public helper directly.
• The status-suppression assertion in chat-session-host-02.test.ts checks both statusText() and the captured showStatus calls, correctly guarding against a regression on either channel.
• If you switch to keyText(...), consider asserting the hint contains the resolved binding rather than the literal Ctrl+o, so the test survives a rebind/casing change.

Changelog / docs

Both changelog entries are under [Unreleased] and appropriately descriptive; README table updated to match. 👍

Overall LGTM — the dynamic-keybinding suggestion is the only thing I'd genuinely encourage before merge; the rest is optional polish.

[claude]

Review — PR #1689: clean up queued-restore status and Ctrl+o keybinding copy

Overall a clean, well-scoped, well-tested change. Removing the transient "Restored N queued message(s)" status is a reasonable UX call (the messages reappearing in the editor is itself the feedback), and the tests are updated on both restore paths, which is exactly right. A few notes, mostly on the keybinding-copy change.

🟡 The new casing doesn't actually match the CLI-wide copy

The PR's stated goal is to "match the CLI-wide keybinding copy," but the canonical way the CLI renders this binding is lowercase ctrl+o, not Ctrl+o:

• Every in-app hint resolves the key via keyText("app.tools.expand") — e.g. read.ts:162 renders (ctrl+o Expand). keyText → formatKeyPart (keybinding-hints.ts:44-52) lowercases every part, and the capitalize option is explicitly deprecated/ignored ("Key labels are always normalized for display").
• The docs agree: docs/keybindings.md:121 shows app.tools.expand → `ctrl+o`.

So this PR introduces a third variant. Across the monorepo the expand hint is now:

• ctrl+o — core tools + docs (dynamic, canonical)
• Ctrl+o — MCP (this PR)
• CTRL+O — packages/web-access/result-renderers.ts:244 (untouched)

If the intent is true alignment, (ctrl+o Expand) (lowercase) matches what keyText emits everywhere else. Worth also folding in the web-access CTRL+O occurrence so the "CLI-wide" cleanup is actually complete rather than moving one of three.

🟡 The hint stays hardcoded instead of rebind-aware

Related but optional: the rest of the CLI resolves this string dynamically (keyHint/keyText("app.tools.expand")), so a user who rebinds app.tools.expand sees the correct key. The MCP renderer hardcodes the literal — so after a rebind the hint goes stale. The hardcoding predates this PR, but since this is the one line being touched for keybinding-copy consistency, it's the natural moment to switch to keyText(...) (available via getKeybindings() from @earendil-works/pi-tui). Not a blocker.

✅ Things that look good

• Queued-restore status removed on both paths (restoreQueuedMessagesToEditor and handleDequeue), with the "No queued messages to restore" branch correctly preserved.
• chat-session-host-02.test.ts asserts absence both via statusText() and a captured showStatus spy — thorough, guards against regressions on either channel.
• The ensureDeferredStartupComplete fakes + toHaveBeenCalledTimes(1) assertions correctly pin the deferred-startup await for /clone and /clear; verified those commands actually await this.ensureDeferredStartupComplete() in source.
• New mcp-tool-result-renderer.test.ts covers collapsed hint, expanded (no hint), and truncation boundary — good coverage for the exported helpers.
• Changelog entries are descriptive per CLAUDE.md guidance.

Nits

• test/unit/mcp-tool-result-renderer.test.ts:6-7 has a stray double blank line after the imports.

Nothing here blocks the queued-restore cleanup, which is the strongest part of the PR. The main ask is to reconcile the Ctrl+o casing (and ideally the stray web-access CTRL+O) so "CLI-wide" is literally true.

[claude]

Review: fix(ui) — clean up queued-restore status and Ctrl+o keybinding copy

Thanks for the cleanup! The queued-restore status removal is tidy and well-covered, and switching the MCP hint to the configured keybinding is the right direction. I found one meaningful test-quality issue and a couple of copy/consistency nits worth addressing.

🔴 mcp-tool-result-renderer.test.ts — the keybinding assertion is effectively vacuous (and has a latent regex bug)

The new renderer test never initializes keybindings. Every other test in the repo that renders a key hint explicitly does setKeybindings(new KeybindingsManager()) first (e.g. tree-selector-*, trust-selector.test.ts, oauth-selector.test.ts), and the app itself only has real keys after setKeybindings(KeybindingsManager.create()) (main-session.ts:231, startup-ui.ts:10). There is no test preload wiring this up either (bunfig.toml has none).

So in this test getKeybindings().getKeys("app.tools.expand") resolves to [], and keyText("app.tools.expand") returns "". That means:

• The rendered hint under test is actually ( Expand) — with no keybinding shown at all — not (ctrl+o Expand).
• Line 33 builds new RegExp(`\\(${keyText("app.tools.expand")} Expand\\)`), which collapses to /\( Expand\)/ and matches trivially. The test's stated purpose ("formats collapsed result hint with the configured expand keybinding") is never actually validated.

There is also a latent regex bug hiding behind this: keyText output is interpolated into a RegExp unescaped, and the real value contains + (a regex metacharacter). If you fix the test to actually set keybindings, keyText returns ctrl+o, and the pattern becomes /\(ctrl+o Expand\)/ — which does not match "(ctrl+o Expand)" (it reads + as "one or more l", so it expects ctrlo). The assertion would then fail spuriously.

Suggested fix: call setKeybindings(new KeybindingsManager()) in the test setup and either assert against the literal "(ctrl+o Expand)" or escape the interpolated value before building the RegExp. That makes the test genuinely verify the keybinding-driven copy.

🟡 "Ctrl+o" vs ctrl+o — the docs describe copy the code never emits

keyText lowercases every key part (formatKeyPart in keybinding-hints.ts), so the hint renders as (ctrl+o Expand) — lowercase. The README (packages/mcp/README.md), both CHANGELOG entries, and the PR title all say Ctrl+o. Not a code bug, but the user-facing copy is described inaccurately; worth aligning the docs to the actual lowercase ctrl+o output (or intentionally deciding the display should be capitalized, which would be a formatKeyPart change).

🟡 Alignment is incomplete outside this package

If the goal is CLI-wide consistency, note that packages/web-access/result-renderers.ts:244 still hardcodes ... CTRL+O Expand) in a live runtime hint, and packages/coding-agent/README.md / docs/sessions.md still show CTRL+O. Out of scope for this PR, but flagging so the inconsistency is tracked rather than lost.

✅ What looks good

• Removing the Restored N queued message(s) status from both paths is clean; handleDequeue keeps the restored === 0 branch intact, and the new chat-session-host-02 assertions (checking both statusText() and captured showStatus calls) are a solid regression guard.
• The formatMcpToolResultLines truncation test is precise and meaningful.
• The ensureDeferredStartupComplete fakes + call-count assertions in the clone/onboarding tests are reasonable coverage for the upstream deferred-startup await.

Overall a nice, focused PR — the main ask is tightening the MCP renderer test so it actually exercises the keybinding it claims to.

[claude]

PR Review — fix(ui): clean up queued-restore status and Ctrl+o keybinding copy

Overall this is a clean, well-scoped change. The keybinding fix is nicely aligned with existing conventions, the status removal is correct, and it comes with real test coverage and changelog entries. A few notes below.

🟡 Duplicate assertion (minor cleanup)

In packages/coding-agent/test/first-run-onboarding.test.ts the same assertion now appears twice in a row — the newly added expect(host.ensureDeferredStartupComplete).toHaveBeenCalledTimes(1) line is identical to the one already present directly below it. This looks like a copy-paste artifact. It is harmless (the test still passes) but redundant; the added line can just be dropped.

✅ Keybinding copy fix — well done

(keyText("app.tools.expand") + " Expand") in tool-result-renderer.ts matches the exact pattern already used elsewhere (e.g. skill-invocation-message.ts, bash-execution.ts), so the hint now stays in sync if the keybinding is ever remapped rather than hard-coding CTRL+O. Good improvement over a static string, and the README table was updated to match.

✅ Status removal — correct and verified

Both restore paths (restoreQueuedMessagesToEditor and the handleDequeue else branch) had the transient "Restored N queued message(s)" status removed while preserving the actual restore behavior. I grepped for other references to that string and found none left dangling. The new negative assertions in chat-session-host-02.test.ts (checking both statusText() and the captured showStatus messages) are a solid way to lock in the suppression.

✅ Test coverage

The new test/unit/mcp-tool-result-renderer.test.ts is a good addition — it covers collapsed hint copy, the expanded case, and truncation boundaries, and it asserts against keyText(...) rather than a literal so it will not rot. Exporting McpToolResultDetails purely for the test is a reasonable, low-cost change.

📝 Nit — PR description drift

The description mentions adding fakes/assertions to interactive-mode-clone-command.test.ts, but that file is not part of the diff (only first-run-onboarding.test.ts is). Likely resolved during the main merge — just flagging so the description matches what shipped.

None of these are blocking. Nice, tidy UI-copy cleanup with matching tests and changelog entries.