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
51 changes: 51 additions & 0 deletions .claude/sessions/2026-04-07.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,54 @@ 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."

## Fix startup banner position and tool cycle expanded state (PR #201)

Continued from the earlier session (picked up the two open issues).

**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.

**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.
6 changes: 2 additions & 4 deletions apps/claude-sdk-cli/src/ToolApprovalState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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). */
Expand Down
3 changes: 1 addition & 2 deletions apps/claude-sdk-cli/src/entry/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions apps/claude-sdk-cli/test/ToolApprovalState.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand All @@ -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);
});
Expand Down
Loading