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
7 changes: 4 additions & 3 deletions .claude/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ Only update the `Status` field — do not modify any other frontmatter or prompt

<!-- BEGIN:REPO:current-state -->
## Current State
Branch: `fix/sticky-zone-viewport-overflow`
In-progress: Nothing. PR #145 (fix #134, sticky zone viewport overflow) open, auto-merge enabled.
Branch: `fix/terminal-rendering-redesign`
In-progress: All stages complete, PR #147 open. Resize corruption fix (narrow) and widen zone-anchoring fix both applied and pushed. Awaiting SC manual verification and merge.
<!-- END:REPO:current-state -->

<!-- BEGIN:REPO:architecture -->
Expand Down Expand Up @@ -176,7 +176,7 @@ Opt-in via `shellicarMcp: true` config. Registers an in-process MCP server (`she

6. **Context thresholds hardcoded** — 85%/90% tool disable thresholds are not configurable.

7. **Cursor positioning is fragile** — `stickyLineCount` is a single point of truth and failure. Occasional off-by-1 documented but not reliably reproducible.
7. **Cursor positioning via Viewport**: `Viewport.scrollOffset` tracks visible window into layout rows. Off-by-1 errors at screen boundaries are possible but not yet observed post-Stage 5 rewrite.

8. **Null unsets in config merge are subtle** — `"model": null` in local config means "use home config's model", not "set to null". Easy to confuse.

Expand All @@ -188,6 +188,7 @@ Opt-in via `shellicarMcp: true` config. Registers an in-process MCP server (`she

- **Structured command execution via in-process MCP** (#99) — replaced freeform Bash with a structured Exec tool served by an in-process MCP server. Glob-based auto-approve (`execAutoApprove`) with custom zero-dep glob matcher (no minimatch dependency).
- **Exec tool extracted to `@shellicar/mcp-exec`** — schema, executor, pipeline, validation rules, and ANSI stripping moved to a published package. CLI retains only `autoApprove.ts` (CLI-specific config concern).
- **ZWJ sanitisation in layout pipeline**: `sanitiseZwj` strips U+200D before `wrapLine` measures width. Terminals render ZWJ sequences as individual emojis; `string-width` assumes composed form. Stripping at the layout boundary removes the mismatch.
<!-- END:REPO:recent-decisions -->

<!-- BEGIN:REPO:extra -->
Expand Down
80 changes: 80 additions & 0 deletions .claude/sessions/2026-03-29.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,83 @@
- Decisions: Phase 1 (implementation, type-check, build, test, supervisor verification) was completed before this session. This session handled commit, push, and PR only.
- Next: Await PR #145 auto-merge.
- Violations: None

### 03:44 - fix/terminal-rendering-redesign (#146) Stage 1

- Did: Created branch `fix/terminal-rendering-redesign`. Built `Screen` interface and `StdoutScreen` in `src/Screen.ts`. Built `MockScreen` test double in `test/MockScreen.ts` with character-by-character ANSI parser (cursor movement, clearLine, clearDown, scrollback violation counting). Wrote 20 tests in `test/mock-screen.spec.ts`. All pass.
- Files: `src/Screen.ts`, `test/MockScreen.ts`, `test/mock-screen.spec.ts`
- Decisions: `MockScreen` placed in `test/` per spec (test double, not production code). Biome `useConsistentMemberAccessibility` required explicit `public` on all members (unsafe fix, done manually). Wrap at last column on last row treated as scrollback violation (matches real terminal scroll behavior).
- Next: Stage 2 (Layout pure function).
- Violations: None

### 03:51 - fix/terminal-rendering-redesign (#146) Stage 2

- Did: Implemented `layout()` pure function in `src/Layout.ts` with `BuiltComponent`, `LayoutInput`, `LayoutResult` interfaces and a `wrapLine()` helper. Wrote 11 tests in `test/layout.spec.ts` covering all cases from the spec. All 140 tests pass.
- Files: `src/Layout.ts`, `test/layout.spec.ts`
- Decisions: `wrapLine` uses `stringWidth` char-by-char for correct visual width splitting. ANSI-escaped BuiltComponent rows that don't exceed `columns` pass through as-is (typical for status lines); ANSI-aware splitting for wider strings deferred to Stage 5 if needed.
- Next: Stage 3 (Viewport stateful class).
- Violations: None

### 03:54 - fix/terminal-rendering-redesign (#146) Stage 3

- Did: Implemented `Viewport` class in `src/Viewport.ts` with `scrollOffset` state, cursor chasing (up/down), and resize capping. Wrote 11 tests in `test/viewport.spec.ts` covering all spec cases. All 151 tests pass. Committed and pushed.
- Files: `src/Viewport.ts`, `test/viewport.spec.ts`
- Decisions: Biome `useConsistentMemberAccessibility` required explicit `public` on `resolve()` (unsafe fix, applied manually, consistent with Stage 1 pattern).
- Next: Stage 4 (Renderer class).
- Violations: None

### 05:29 - fix/terminal-rendering-redesign (#146) Stage 5

- Did: Wired Layout/Viewport/Renderer pipeline into Terminal class. Replaced `buildSticky()`/`clearStickyZone()` with `renderZone()` pipeline. Added `buildLayoutInput()` that maps existing builders to `BuiltComponent` format. Fixed two bugs found during manual testing: (1) render() wrote padded viewport rows causing scrollback violations on first render (fix: trim trailing empty rows before writing); (2) writeHistory() re-rendered stale `lastFrame` with cleared-question still visible (fix: split into `writeHistoryLine()` + fresh `renderZone()` for current component state). Added 8 integration tests in `test/terminal-integration.spec.ts`. All 167 tests pass. Type-check and build clean.
- Files: `src/terminal.ts`, `src/TerminalRenderer.ts`, `test/terminal-integration.spec.ts`
- Decisions: `writeHistoryLine()` added to Renderer to write a history line and reset cursor tracking without triggering a zone re-render; Terminal calls it then calls `renderZone()` separately for a fresh frame. Trailing empty row trim applied in both `render()` and `writeHistory()` paths to prevent padding rows from advancing cursor past screen bottom. No commit yet per Stage 5 instructions (do not commit; await supervisor manual verification).
- Next: Manual verification by Supreme Commander, then commit/push/PR with `Closes #146`.
- Violations: None

### 16:15 - fix/terminal-rendering-redesign (#146) Grapheme fix

- Did: Fixed `wrapLine` to iterate grapheme clusters via `Intl.Segmenter` instead of code points. Added test `multi-codepoint grapheme clusters wrap as single units` to `test/layout.spec.ts` covering ZWJ emoji sequences. All 169 tests pass.
- Files: `src/Layout.ts`, `test/layout.spec.ts`
- Decisions: `for (const char of line)` iterates Unicode code points; multi-codepoint grapheme clusters (e.g. 👨‍👩‍👦 = U+1F468 ZWJ U+1F469 ZWJ U+1F466) would be split mid-cluster, causing incorrect wrap counts. Fix mirrors the pattern already used in `src/editor.ts`.
- Next: Await PR auto-merge.
- Violations: None

### 15:46 - fix/terminal-rendering-redesign (#146) Commit/Push/PR

- Did: Committed Stage 5 changes after supervisor manual verification. Pushed branch. Created PR with `Closes #146`.
- Files: `src/terminal.ts`, `src/TerminalRenderer.ts`, `test/terminal-integration.spec.ts`, `.claude/CLAUDE.md`, `.claude/sessions/2026-03-29.md`
- Decisions: Session log and CLAUDE.md state updates included in the same commit as the code changes per session protocol.
- Next: Await PR auto-merge.
- Violations: None

### 16:50 - fix/terminal-rendering-redesign: ZWJ sanitisation in layout pipeline

- Did: Added `sanitiseZwj` to `src/sanitise.ts` (strips U+200D before width measurement). Wired it into `wrapLine` in `src/Layout.ts` so terminal rendering and `string-width` agree on emoji width. Updated tests to reflect decomposed behavior.
- Files: `src/sanitise.ts`, `src/Layout.ts`, `test/sanitise.spec.ts`, `test/layout.spec.ts`
- Decisions: ZWJ sequences render as individual emojis in terminals that do not support composed rendering; `string-width` reports them as composed (width 2). Stripping ZWJ at the layout boundary removes the ambiguity. The old "wrap as single unit" test was testing the wrong assumption and was updated. Manual verification by SC still pending.
- Next: SC to verify manually in terminal. Then fix cursor position bug.
- Violations: None

### 18:30 - fix/terminal-rendering-redesign: resize corruption fix

- Did: Diagnosed resize corruption: on terminal narrow, old zone content reflows to more rows at the new column width and can appear anywhere in the visible area. `lastVisibleCursorRow` is stale and `cursorUp` moves to the wrong row. Added TDD test to expose the bug (RED: scrollback violation). Implemented fix: `Renderer.notifyResize()` sets `pendingResize` flag; next `render()` clears the entire visible screen (`cursorAt(1,1)` + `clearDown`), then anchors the new zone at `screen.rows - zoneHeight`. Added `MockScreen` support for `ESC[row;colH` absolute positioning (required by fix). Wired `Terminal.notifyResize()` and `ClaudeCli` resize debounce. 184 tests pass. SC manual verification: fix partially resolved the issue; second iteration replaced partial clear (old zone area only) with full screen clear, which also handles the case where reflowed zone content spreads above the calculated old zone top. Build and type-check clean.
- Files: `src/TerminalRenderer.ts`, `src/terminal.ts`, `src/ClaudeCli.ts`, `test/MockScreen.ts`, `test/mock-screen.spec.ts`, `test/TerminalRenderer.spec.ts`
- Decisions: Full screen clear on resize is the correct approach because after terminal reflow the old zone can occupy an unknown number of rows anywhere in the visible area. Partial clear (based on stored zoneHeight) is unreliable. Trade-off: visible history above zone is momentarily blank after resize (still accessible in scrollback).
- Next: SC to verify manually with latest build.
- Violations: None

### 19:41 - fix/terminal-rendering-redesign: widen resize zone-anchoring fix

- Did: Diagnosed two widen-resize bugs via TDD. Bug 1: `cursorAt(1,1) + clearDown` in the `pendingResize` path erased all visible history above the zone when widening. Bug 2: `cursorAt(newZoneTop, 1)` always anchored the zone to the screen bottom, causing it to jump down even when the cursor had not moved. Fixed both by differentiating narrow vs widen in `pendingResize`: use `cursorAt(newZoneTop, 1)` only when `newZoneHeight > zoneHeight` (zone grew, cursor displaced by reflow); use `cursorUp(lastVisibleCursorRow)` otherwise (zone shrunk or same, cursor approximately in place). Added two regression tests; all 186 tests pass.
- Files: `src/TerminalRenderer.ts`, `test/TerminalRenderer.spec.ts`
- Decisions: Zone-grew heuristic (`newZoneHeight > zoneHeight`) is the narrowing signal: when the terminal narrows, wrapped content produces more rows; when it widens, fewer rows. The existing narrow-case test (`no scrollback violations when cursor displaced`) continues to pass because `newZoneHeight (4) > zoneHeight (2)` triggers the absolute path correctly.
- Next: SC to verify manually with latest build. Await PR #147 auto-merge.
- Violations: None

### 04:29 - fix/terminal-rendering-redesign (#146) Stage 4

- Did: Implemented `Renderer` class in `src/TerminalRenderer.ts` (filename avoids macOS case-insensitive collision with existing `src/renderer.ts`). Wrote 8 tests in `test/renderer.spec.ts` covering all spec cases. All 159 tests pass. Committed and pushed.
- Files: `src/TerminalRenderer.ts`, `test/renderer.spec.ts`
- Decisions: File named `TerminalRenderer.ts` not `Renderer.ts` because macOS case-insensitive FS treats `Renderer.ts` and `renderer.ts` as the same file (existing `renderer.ts` contains `prepareEditor`). `clearDown` must precede the last row write, not follow it: MockScreen clears the entire current row on `ESC[J`, so issuing clearDown after writing the last row erases it. The fix issues clearDown when cursor is on the last row position before writing content there.
- Next: Stage 5 (Wire + Replace).
- Violations: None
1 change: 1 addition & 0 deletions src/ClaudeCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,7 @@ export class ClaudeCli {
clearTimeout(this.resizeTimer);
this.resizeTimer = setTimeout(() => {
this.term.paused = false;
this.term.notifyResize();
this.redraw();
}, 300);
});
Expand Down
97 changes: 97 additions & 0 deletions src/Layout.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import stringWidth from 'string-width';
import type { EditorRender } from './renderer.js';
import { sanitiseZwj } from './sanitise.js';

const segmenter = new Intl.Segmenter(undefined, { granularity: 'grapheme' });

/**
* Output from an existing builder (status, attachment, preview).
* `rows` are logical lines as the builder produces them (may be wider than columns).
* `height` is the visual height in terminal rows (accounts for wrapping).
*/
export interface BuiltComponent {
rows: string[];
height: number;
}

export interface LayoutInput {
editor: EditorRender;
status: BuiltComponent | null;
attachments: BuiltComponent | null;
preview: BuiltComponent | null;
question: BuiltComponent | null;
columns: number;
}

/**
* Layout output. `buffer` contains one entry per visual (terminal) row.
* Layout is responsible for wrapping: a logical line wider than `columns`
* becomes multiple buffer entries.
*/
export interface LayoutResult {
buffer: string[];
cursorRow: number;
cursorCol: number;
editorStartRow: number;
}

/**
* Splits a logical line into visual rows by wrapping at `columns` visual width.
* Returns at least one entry (empty string for empty input).
*/
function wrapLine(line: string, columns: number): string[] {
const sanitised = sanitiseZwj(line);
if (stringWidth(sanitised) <= columns) {
return [sanitised];
}
const segments: string[] = [];
let current = '';
let currentWidth = 0;
for (const { segment } of segmenter.segment(sanitised)) {
const cw = stringWidth(segment);
if (currentWidth + cw > columns) {
segments.push(current);
current = segment;
currentWidth = cw;
} else {
current += segment;
currentWidth += cw;
}
}
if (current.length > 0 || segments.length === 0) {
segments.push(current);
}
return segments;
}

/**
* Pure layout function. Takes all UI components and returns an unbounded
* buffer of visual rows with cursor position metadata.
*
* Buffer order (top to bottom): question, status, attachments, preview, editor.
*/
export function layout(input: LayoutInput): LayoutResult {
const { editor, status, attachments, preview, question, columns } = input;
const buffer: string[] = [];

for (const component of [question, status, attachments, preview]) {
if (component !== null) {
for (const row of component.rows) {
buffer.push(...wrapLine(row, columns));
}
}
}

const editorStartRow = buffer.length;

for (const line of editor.lines) {
buffer.push(...wrapLine(line, columns));
}

return {
buffer,
cursorRow: editorStartRow + editor.cursorRow,
cursorCol: editor.cursorCol,
editorStartRow,
};
}
26 changes: 26 additions & 0 deletions src/Screen.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
export interface Screen {
readonly rows: number;
readonly columns: number;
write(data: string): void;
onResize(cb: (columns: number, rows: number) => void): () => void;
}

export class StdoutScreen implements Screen {
public get rows(): number {
return process.stdout.rows ?? 24;
}

public get columns(): number {
return process.stdout.columns ?? 80;
}

public write(data: string): void {
process.stdout.write(data);
}

public onResize(cb: (columns: number, rows: number) => void): () => void {
const handler = () => cb(process.stdout.columns ?? 80, process.stdout.rows ?? 24);
process.stdout.on('resize', handler);
return () => process.stdout.off('resize', handler);
}
}
Loading
Loading