From 4c5983ba799d1bbd319670cc15ce3e3dd0161594 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Tue, 7 Apr 2026 02:19:53 +1000 Subject: [PATCH 1/3] Fix startup banner position and tool cycle expanded state Two unrelated UX bugs fixed together as both are small. **Startup banner pushed off screen on long sessions** showStartupBanner was called before addHistoryBlocks, making the banner the first block in the conversation. On sessions with enough history to fill the viewport, the banner scrolled off the top and was never visible. Moved the call to after history replay so the banner sits immediately above the first prompt where it is always seen. **Left/right cycling collapses expanded tool input** selectPrev and selectNext both set #toolExpanded = false, so pressing an arrow key while a tool's input was expanded would immediately collapse it. The intent of resetExpanded (called on return to editor mode) already covers the right teardown point. Removed the collapse from selectPrev and selectNext so expanded state is preserved while navigating tools. Tests updated to reflect the new expected behaviour. --- apps/claude-sdk-cli/src/ToolApprovalState.ts | 6 ++---- apps/claude-sdk-cli/src/entry/main.ts | 3 +-- apps/claude-sdk-cli/test/ToolApprovalState.spec.ts | 8 ++++---- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/apps/claude-sdk-cli/src/ToolApprovalState.ts b/apps/claude-sdk-cli/src/ToolApprovalState.ts index 27ba579..fabe9d8 100644 --- a/apps/claude-sdk-cli/src/ToolApprovalState.ts +++ b/apps/claude-sdk-cli/src/ToolApprovalState.ts @@ -90,16 +90,14 @@ export class ToolApprovalState { this.#toolExpanded = !this.#toolExpanded; } - /** Select the previous tool, collapsing any expansion. */ + /** Select the previous tool. */ public selectPrev(): void { this.#selectedTool = Math.max(0, this.#selectedTool - 1); - this.#toolExpanded = false; } - /** Select the next tool, collapsing any expansion. */ + /** Select the next tool. */ public selectNext(): void { this.#selectedTool = Math.min(this.#pendingTools.length - 1, this.#selectedTool + 1); - this.#toolExpanded = false; } /** Collapse the expanded view (called when returning to editor mode). */ diff --git a/apps/claude-sdk-cli/src/entry/main.ts b/apps/claude-sdk-cli/src/entry/main.ts index 98a5e61..eb5350b 100644 --- a/apps/claude-sdk-cli/src/entry/main.ts +++ b/apps/claude-sdk-cli/src/entry/main.ts @@ -63,8 +63,6 @@ const main = async () => { rl.setLayout(layout); layout.enter(); - layout.showStartupBanner(startupBannerText()); - const agent = createAnthropicAgent({ authToken, logger, historyFile: HISTORY_FILE }); if (config.historyReplay.enabled) { @@ -73,6 +71,7 @@ const main = async () => { layout.addHistoryBlocks(replayHistory(history, config.historyReplay)); } } + layout.showStartupBanner(startupBannerText()); const store = new RefStore(); while (true) { const prompt = await layout.waitForInput(); diff --git a/apps/claude-sdk-cli/test/ToolApprovalState.spec.ts b/apps/claude-sdk-cli/test/ToolApprovalState.spec.ts index 6b483e3..f3c3a18 100644 --- a/apps/claude-sdk-cli/test/ToolApprovalState.spec.ts +++ b/apps/claude-sdk-cli/test/ToolApprovalState.spec.ts @@ -219,14 +219,14 @@ describe('ToolApprovalState — navigation', () => { expect(actual).toBe(expected); }); - it('selectPrev collapses expanded view', () => { + it('selectPrev preserves expanded state', () => { const state = new ToolApprovalState(); state.addTool(toolA); state.addTool(toolB); state.selectNext(); state.toggleExpanded(); state.selectPrev(); - const expected = false; + const expected = true; const actual = state.toolExpanded; expect(actual).toBe(expected); }); @@ -250,13 +250,13 @@ describe('ToolApprovalState — navigation', () => { expect(actual).toBe(expected); }); - it('selectNext collapses expanded view', () => { + it('selectNext preserves expanded state', () => { const state = new ToolApprovalState(); state.addTool(toolA); state.addTool(toolB); state.toggleExpanded(); state.selectNext(); - const expected = false; + const expected = true; const actual = state.toolExpanded; expect(actual).toBe(expected); }); From f24193f0eb9cc1c1d111ac41fc9c77a9e10487cd Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Tue, 7 Apr 2026 02:20:30 +1000 Subject: [PATCH 2/3] Update session log for 2026-04-07 --- .claude/sessions/2026-04-07.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/.claude/sessions/2026-04-07.md b/.claude/sessions/2026-04-07.md index 823d0d2..00963be 100644 --- a/.claude/sessions/2026-04-07.md +++ b/.claude/sessions/2026-04-07.md @@ -179,3 +179,32 @@ Final step in the AppLayout refactor series. **`isLikelyPath` stays in AppLayout**: Used only in `#handleCommandKey`. Moving it to clipboard.ts or a utils file would create a dependency for 8 lines that only ever live in one context. The benefit doesn’t justify the churn. **togglePreview invariant not addressed**: `CommandModeState.togglePreview()` can set `previewMode = true` when `commandMode = false`. The renderer handles this correctly (returns empty previewRows), the original code had the same behaviour, and fixing it would change the state contract in a way that breaks existing tests. Reviewer noted this as non-blocking; leaving as-is. + + +--- + +## Fix null compaction content silently clearing history (PR #200) + +Second session of the day. The previous session was lost because the Anthropic API sent a compaction block with `null` content (failed compaction), and the SDK silently wiped the entire conversation history instead of ignoring it. + +**Root cause chain:** JS string coercion (`'' += null` → `'null'`) → unconditional push to `#completed` → `Conversation.push()` sees a compaction block → `this.#items.length = 0`. + +**Fixes in `MessageStream.ts`:** +- Delta handler: `+= event.delta.content ?? ''` — prevents null coercing to the string `"null"` +- Stop handler: `if (acc.content)` guards the push to `#completed`; `compaction_complete` always emits (with `'No compaction summary received'` fallback) so the UI still shows something in the block + +**Fix in `replayHistory.ts`:** Compaction blocks were being looked up on `user` messages and reading a non-existent `summary` field. They live on `assistant` messages (via `handleAssistantMessages`) and the real field is `content: string | null`. Moved check to the assistant block loop. + +**Test helper fixes** (`Conversation.spec.ts`, `replayHistory.spec.ts`): Both had `compactionMsg`/`compaction` helpers with wrong role (`'user'`), wrong field (`summary`), fabricated `llm_identifier`, and `as unknown as` / `satisfies` casts to paper over the errors. Fixed to proper types — no casts needed. + +**New `MessageStream.spec.ts`:** Added per reviewer request (BananaBot9000). Tests null delta → no compaction block in result (the critical history-safe path), fallback `compaction_complete` emission, and the valid-content happy path. + +**PR #200** — merged, squash. Reviewer: BananaBot9000 (approved 🍌). "Defense in depth: ?? on delta AND if(acc.content) on stop." + +## Open issues to investigate next + +These were noted before the session was lost. The previous agent had just started reading the relevant files when compaction wiped the context. + +1. **Version banner pushed off screen** — the startup version/banner is displayed before history replay, so on sessions with long history it scrolls off the top and is never visible. + +2. **Left/right tool cycling collapses expanded state** — `selectPrev` and `selectNext` in `ToolApprovalState` both call collapse (or reset expanded), so cycling tools with arrow keys loses the expanded view. Expected: expanded state should be preserved when cycling. From f817923ef9efcfebb19ccfb5b0be133e25c051c6 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Tue, 7 Apr 2026 02:50:34 +1000 Subject: [PATCH 3/3] Update session log for 2026-04-07 --- .claude/sessions/2026-04-07.md | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/.claude/sessions/2026-04-07.md b/.claude/sessions/2026-04-07.md index 00963be..568fcec 100644 --- a/.claude/sessions/2026-04-07.md +++ b/.claude/sessions/2026-04-07.md @@ -201,10 +201,32 @@ Second session of the day. The previous session was lost because the Anthropic A **PR #200** — merged, squash. Reviewer: BananaBot9000 (approved 🍌). "Defense in depth: ?? on delta AND if(acc.content) on stop." -## Open issues to investigate next +## Fix startup banner position and tool cycle expanded state (PR #201) -These were noted before the session was lost. The previous agent had just started reading the relevant files when compaction wiped the context. +Continued from the earlier session (picked up the two open issues). -1. **Version banner pushed off screen** — the startup version/banner is displayed before history replay, so on sessions with long history it scrolls off the top and is never visible. +**Banner position (`entry/main.ts`):** `showStartupBanner` was called before `addHistoryBlocks`, so on long sessions the banner scrolled off the top. Moved it after history replay so it always sits just above the first prompt. -2. **Left/right tool cycling collapses expanded state** — `selectPrev` and `selectNext` in `ToolApprovalState` both call collapse (or reset expanded), so cycling tools with arrow keys loses the expanded view. Expected: expanded state should be preserved when cycling. +**Tool cycle expanded (`ToolApprovalState.ts`):** `selectPrev` and `selectNext` both called `this.#toolExpanded = false`, collapsing the tool input view when cycling with arrow keys. Removed those two lines. `resetExpanded` (called when returning to editor mode) still handles teardown. Tests updated: "collapses" → "preserves" with `expected = true`. + +**PR #201** — open, auto-merge enabled, awaiting review from bananabot9000. Branch: `fix/startup-banner-and-tool-cycle`. + +--- + +## Record tool approval decision in conversation history (PR #202) + +Third session (after context compaction). + +**Problem:** When the user pressed Y or N on a tool approval, the decision left no trace in conversation history. The tool name appeared in the tools block before the decision, then the approval UI disappeared silently. With batched tool calls (multiple `tool_use` blocks in one assistant message), all names were appended before any decision was recorded — so inline annotation per tool line was impossible. + +**Fix (`AgentMessageHandler.ts`):** Moved the `appendStreaming` call from the synchronous `handle()` path into the async `#toolApprovalRequest()`, writing `"ToolName ✓\n"` or `"ToolName ✗\n"` after `respond()` and `removePendingTool()`. Tools now appear in the block only once decided. FIFO approval order matches arrival order, so both single and multi-tool batches look clean. The catch path appends ✗ for exceptions. + +**Tests:** New describe block covering transitions (synchronous), auto-denied (Deny path — synchronous), auto-approved (Approve path — synchronous), manual approval and denial (Ask path — async, flushed with `await Promise.resolve()`). + +343 tests passing. **PR #202** — open, auto-merge enabled, awaiting review from bananabot9000. Branch: `feature/tool-approval-feedback`. + +--- + +## Local branch state + +`fix/startup-banner-and-tool-cycle` has one extra local commit (`620c4ec` — approval feedback) that was cherry-picked to `feature/tool-approval-feedback` and NOT pushed on this branch. Run `git reset --hard f24193f` on that branch to clean up.