diff --git a/.grok/PR-COMMAND-PALETTE.md b/.grok/PR-COMMAND-PALETTE.md new file mode 100644 index 0000000..8c40c06 --- /dev/null +++ b/.grok/PR-COMMAND-PALETTE.md @@ -0,0 +1,154 @@ +## Summary + +Adds interactive command palette with autocomplete, keyboard navigation, fuzzy matching, and "did you mean" suggestions. + +**Problem**: Typing `/` in Grok TUI shows nothing - users must already know command names. No autocomplete, no command discovery, no typo assistance. + +**Solution**: Interactive command palette that appears when typing `/`, with smart filtering and keyboard navigation. + +## Features + +### 1. Live Command Filtering + +Type `/` to see all commands, then filter as you type: + +``` +> / +┌─ Commands (7 matches) ─┐ +▶ /auth - Authentication management + /clear - Clear conversation + /exit - Exit application + /help - Show help + /history - Show conversation history + /model - Set AI model + /prompt - Load prompt from file +└─────────────────────────┘ +Tab: autocomplete • ↑/↓: navigate • Esc: close • Enter: submit +``` + +### 2. Smart Matching + +**Prefix match**: +``` +> /pr +┌─ Commands (1 match) ─┐ +▶ /prompt - Load prompt from file +└───────────────────────┘ +``` + +**Alias match**: +``` +> /pf +┌─ Commands (1 match) ─┐ +▶ /prompt (via pf) - Load prompt from file +└────────────────────────────────────────┘ +``` + +**Fuzzy match** (typo tolerance): +``` +> /promt +┌─ Commands (1 match) ─┐ +▶ /prompt - Load prompt from file +└───────────────────────┘ +``` + +### 3. Keyboard Navigation + +- **↑/↓**: Navigate suggestions (only when palette visible) +- **Tab**: Autocomplete to selected command + add space +- **Esc**: Close palette +- **Enter**: Submit command as usual + +### 4. "Did You Mean" for Unknown Commands + +``` +> /promt myfile.txt +[Enter] + +Error: Unknown command: /promt + +Did you mean: /prompt? + +Use /help to see all available commands. +``` + +## Implementation + +### Phase 1: Suggestion Algorithm (`src/ui/utils/command-suggestions.ts`) + +**Levenshtein Distance**: +- Calculates edit distance for fuzzy matching +- Tolerates up to 2 character edits +- No external dependencies (30 lines, pure algorithm) + +**Matching Types** (ranked): +1. **Exact** (1000 points): `/prompt` → `prompt` +2. **Prefix** (900 points): `/pr` → `prompt` +3. **Alias Exact** (800 points): `/pf` → `prompt` +4. **Alias Prefix** (700 points): `/promptf` → `prompt` (via `promptfile`) +5. **Substring** (600 points): `/odel` → `model` +6. **Fuzzy** (500-distance*100): `/promt` → `prompt` (1 edit = 400 points) + +### Phase 2: UI Component (`src/ui/components/command-palette.tsx`) + +- Cyan bordered box (consistent with tool selection) +- Selected item: inverse text + bold + arrow indicator +- Shows matched alias when applicable +- Keyboard hint at bottom + +### Phase 3: Input Integration (`src/ui/components/input.tsx`) + +- Computes suggestions in real-time (`useMemo`) +- Shows palette when `value.startsWith('/')` and not pasting +- Arrow keys captured when palette visible (priority over tool navigation) +- Tab autocompletes and preserves any args already typed +- Escape closes palette without clearing input + +### Phase 4: Error Enhancement (`src/commands/index.ts`) + +- Unknown command errors now include fuzzy suggestion +- Uses `getDidYouMeanSuggestion()` for best match +- Shows alias if matched via alias + +## Compatibility + +**No conflicts with existing features**: +- ✅ Bracketed paste: Palette hidden during `isPasting` +- ✅ Tool navigation: Only captures arrows when palette visible (tool nav when `isActive=false`) +- ✅ Multi-line input: Palette respects multi-line display +- ✅ Command execution: Unchanged behavior + +## Test Coverage + +**New Test File**: `tests/unit/command-suggestions.test.ts` (25 tests) + +- Levenshtein distance correctness (7 tests) +- Suggestion matching (exact, prefix, alias, substring, fuzzy) (12 tests) +- Ranking and sorting (2 tests) +- "Did you mean" logic (4 tests) + +**Test Results**: 238/238 pass (+25 from 213) + +## Verification + +```bash +✅ npm run build - TypeScript compilation clean +✅ npm test - 238/238 tests pass (+25 new tests) +✅ No conflicts with paste/navigation modes +``` + +## Manual Testing + +After merge, verify in TUI: + +1. **Discovery**: Type `/` → see all 7 commands +2. **Filter**: Type `/pr` → see `/prompt` +3. **Alias**: Type `/pf` → matches `/prompt (via pf)` +4. **Fuzzy**: Type `/promt` → matches `/prompt` +5. **Navigate**: Press ↓/↑ → selection moves +6. **Autocomplete**: Press Tab → completes to `/prompt ` +7. **Typo Help**: Submit `/promt` → error shows "Did you mean: /prompt?" + +--- + +🤖 Generated with [Claude Code](https://claude.com/claude-code) diff --git a/.grok/PR-EXPLORE-QUALITY.md b/.grok/PR-EXPLORE-QUALITY.md new file mode 100644 index 0000000..3108141 --- /dev/null +++ b/.grok/PR-EXPLORE-QUALITY.md @@ -0,0 +1,98 @@ +## Summary + +Fixes explore subagent output quality - it now reliably finds code evidence using token-based searches instead of failing with phrase-based searches. + +**Problem**: When asked to find code like "where taskSuccesses increments", the explore subagent searched for English phrases ("successful Task", "count successful") instead of code tokens (`taskSuccesses`, `call.tool === 'Task'`), resulting in "no matches found." + +**Solution**: Enhanced explore.md system prompt with explicit operating rules for code evidence gathering. + +## Root Cause + +**Current behavior** (broken): +``` +User: "Find where taskSuccesses increments" +Subagent: Grep: "count successful" → no matches +``` + +**Expected behavior** (fixed): +``` +User: "Find where taskSuccesses increments" +Subagent: Grep: "taskSuccesses" → finds definition + uses +Subagent: Grep: "taskSuccesses.*\+=" → finds increment line +Subagent: Quotes file:line:content +``` + +## Changes + +**File**: `.grok/agents/explore.md` + +### Added Operating Rules + +**1. Token-Based Code Searches** (CRITICAL) +- Use code tokens/identifiers: `taskSuccesses`, `call.tool`, `runSubagent` +- NOT English phrases: "successful Task", "count successful" +- Includes examples: variable names, function names, class names, operators + +**2. File:Line Evidence Requirements** +- Use Grep with identifiers +- Include actual grep output (file:line:content) +- Provide exact line numbers and exact code +- Read file if grep output is long + +**3. Search Scope Preferences** +- **Prefer `src/**` over `tests/**`** unless tests explicitly requested +- Use patterns: `Glob: "src/**/*.ts"`, `Grep: "pattern" --glob "src/**"` + +**4. Follow-Up Strategy** +- When finding a function: do second Grep for key identifiers +- Search same file for related tokens +- Read for context if needed + +**5. Quality Checklist** +- Verify token/identifier searches used +- Verify file:line:content included +- Verify src/** searched first +- Verify exact matching code quoted + +## Regression Tests + +**File**: `tests/unit/truthfulness.test.ts` (+5 tests) + +Tests verify explore.md system prompt contains: +- ✅ Token/identifier search guidance +- ✅ File:line evidence requirements +- ✅ `src/**` preference over `tests/**` +- ✅ Follow-up strategy for function findings +- ✅ Quality checklist + +## Impact + +**Before** (phrase search fails): +``` +User: "Find where taskSuccesses increments" +=== SUBAGENT TRACE === +[1] ✓ Grep: "successful Task" + No matches found +``` + +**After** (token search succeeds): +``` +User: "Find where taskSuccesses increments" +=== SUBAGENT TRACE === +[1] ✓ Grep: "taskSuccesses" + src/agent/grok-agent.ts:106: taskSuccesses += 1; +[2] ✓ Read: src/agent/grok-agent.ts + [context around line 106] +``` + +## Verification + +```bash +✅ npm run build - TypeScript compilation clean +✅ npm test - 209/209 tests pass (+5 new tests) +✅ explore.md contains all required guidance strings +``` + +--- + +🤖 Generated with [Claude Code](https://claude.com/claude-code) diff --git a/.grok/PR-EXPLORE-TOKEN-RELIABILITY.md b/.grok/PR-EXPLORE-TOKEN-RELIABILITY.md new file mode 100644 index 0000000..c3ba8dc --- /dev/null +++ b/.grok/PR-EXPLORE-TOKEN-RELIABILITY.md @@ -0,0 +1,98 @@ +## Summary + +Fixes explore subagent token search reliability by eliminating fragile operator regex patterns that miss code with whitespace. + +**Problem**: explore subagent searches for `taskSuccesses++` or `taskSuccesses+=` (no whitespace), but actual code is `taskSuccesses += 1;` (with spaces), causing "no matches found." + +**Root Cause**: +``` +Grep: "taskSuccesses\+\+|taskSuccesses\+=" ← Misses whitespace +Actual code: taskSuccesses += 1; ← Has space before = +Result: No matches found ❌ +``` + +**Solution**: Use simple token-only searches that find ALL uses (including increments/assignments), then quote the relevant lines. + +## Changes + +**File**: `.grok/agents/explore.md` + +### New Critical Guidance (added to Section 1) + +**For increment/assignment questions**: +- ✅ **ALWAYS start with token ONLY**: `Grep: "taskSuccesses"` +- ❌ **DO NOT use operator patterns**: `taskSuccesses++`, `taskSuccesses+=` (fragile) +- ❌ **DO NOT use regex alternation**: `\+\+|\+=` (fragile) +- ❌ **DO NOT use word boundaries**: `\b`, `\s` (fragile) + +**Why this works**: +- Simple `Grep: "taskSuccesses"` finds **ALL** uses including: + - `taskSuccesses += 1;` (increment with spaces) + - `const x = taskSuccesses;` (assignment) + - `if (taskSuccesses > 0)` (condition) +- Then quote the relevant lines from grep output + +## Before vs After + +### Before (broken) + +**Subagent**: +``` +=== SUBAGENT TRACE === +[1] ✓ Grep: "taskSuccesses\+\+|taskSuccesses\+=" + No matches found +``` + +**User sees**: "no matches found" and thinks subagent failed + +### After (fixed) + +**Subagent**: +``` +=== SUBAGENT TRACE === +[1] ✓ Grep: "taskSuccesses" + src/agent/grok-agent.ts:106: taskSuccesses += 1; + src/agent/grok-agent.ts:112: const subagentsSpawned = taskSuccesses; + +[2] ✓ Read: src/agent/grok-agent.ts + [context around increment...] +``` + +**User sees**: Actual file:line:content evidence ✅ + +## Regression Tests + +**File**: `tests/unit/truthfulness.test.ts` (+3 tests in "Explore Subagent Prompt Quality") + +Tests enforce explore.md contains: +1. ✅ "ALWAYS start with" + "token ONLY" for increments +2. ✅ "DO NOT use operator patterns" warning +3. ✅ "DO NOT use regex alternation" warning + +## Impact + +**Search Reliability**: +- Before: Fragile patterns miss whitespace variations +- After: Token-only search finds all uses reliably + +**User Experience**: +- Before: "no matches found" (confusing) +- After: Actual `file:line:content` evidence (clear) + +## Verification + +```bash +✅ npm run build - TypeScript compilation clean +✅ npm test - 212/212 tests pass (+3 new tests) +✅ explore.md contains all anti-fragile guidance +``` + +**Manual smoke test** (after merge): +``` +User: "Find where taskSuccesses increments" +Expected: Grep returns src/agent/grok-agent.ts:106: taskSuccesses += 1; +``` + +--- + +🤖 Generated with [Claude Code](https://claude.com/claude-code) diff --git a/.grok/PR-SUBAGENT-UX.md b/.grok/PR-SUBAGENT-UX.md new file mode 100644 index 0000000..ac279a7 --- /dev/null +++ b/.grok/PR-SUBAGENT-UX.md @@ -0,0 +1,76 @@ +## Summary + +Fixes UX confusion where users misinterpret subagent evidence as "no spawn occurred." + +**Problem**: Subagents correctly show `Subagents spawned: 0 (subagents cannot spawn subagents)` in their own evidence block, but users misread this as indicating the subagent didn't run at all. + +**Solution**: Add explicit `=== SUBAGENT RUN (system) ===` header to Task output that unambiguously declares: +- What subagent type was spawned +- The unique agent ID +- What tools the subagent was allowed to use +- Why the subagent's own evidence shows "0" + +## Changes + +**Core Changes**: +- `src/agents/types.ts`: Add `subagentType` + `allowedTools` to `SubagentResult` +- `src/agents/subagent-runner.ts`: Include these fields in all return paths +- `src/tools/task.ts`: Prepend SUBAGENT RUN header to successful Task outputs +- `tests/unit/truthfulness.test.ts`: Add 6 regression tests for header presence + +**Header Format**: +``` +=== SUBAGENT RUN (system) === +subagent_type: explore +agentId: subagent-1768766355-x9fk2a +allowedTools: Read, Grep, Glob +note: subagent evidence shows "Subagents spawned: 0" because subagents cannot spawn subagents + +[subagent output follows...] + +=== SUBAGENT TRACE === +Tool calls: 2 +[1] ✓ Grep: "pattern" +[2] ✓ Read: file.ts + +=== EVIDENCE === +Tools executed: Grep(1), Read(1) +Total tool calls: 2 +Subagents spawned: 0 (subagents cannot spawn subagents) +``` + +## UX Impact + +**Before** (confusing): +- User sees `Subagents spawned: 0` in Task output and thinks nothing happened +- No clear indication a subagent actually ran + +**After** (unambiguous): +- Clear header declares: "SUBAGENT RUN (system)" +- Shows exact subagent type, ID, and allowed tools +- Explains why subagent evidence shows "0" +- Main agent evidence (`Subagents spawned (via Task): 1`) remains authoritative + +## Verification + +```bash +✅ npm run build - TypeScript compilation clean +✅ npm test - 204/204 tests pass (+6 new tests) +✅ Header fields tested: type, agentId, allowedTools, explanatory note +``` + +## Test Coverage + +**New Tests** (6 added to `tests/unit/truthfulness.test.ts`): +- Detects all required header fields in mock output +- Validates subagent_type field format +- Validates agentId field format +- Validates allowedTools field format +- Validates explanatory note presence +- Rejects output without proper header + +**Total**: 204 tests (was 198 in PR #7) + +--- + +🤖 Generated with [Claude Code](https://claude.com/claude-code) diff --git a/docs/CLAUDE-TEAM-EXECUTION-PROMPT.md b/docs/CLAUDE-TEAM-EXECUTION-PROMPT.md new file mode 100644 index 0000000..947905c --- /dev/null +++ b/docs/CLAUDE-TEAM-EXECUTION-PROMPT.md @@ -0,0 +1,129 @@ +# Execution Prompt for Claude Dev Team + +## Task: Implement Keytar Ignore-Scripts Detection + +**PRP Document**: `docs/PRP-KEYTAR-IGNORE-SCRIPTS-DETECTION.md` (enhanced, 100% ready) +**Branch**: Create new branch from `main` (or add to existing PR if preferred) +**Severity**: HIGH - Blocks auth on hardened systems +**Estimated Time**: 2-3 hours +**Risk**: LOW (read-only detection, graceful fallbacks) + +--- + +## Problem Summary + +Users with `npm ignore-scripts=true` (common on hardened dev machines) cannot use `grok auth login` because: +1. `npm rebuild keytar` doesn't work (scripts disabled) +2. CLI suggests this broken command +3. Users follow instructions exactly but remain stuck + +## Solution + +Detect `npm ignore-scripts=true` and provide correct fix: `npm_config_ignore_scripts=false npm rebuild keytar --foreground-scripts` + +--- + +## Execution Checklist + +### 1. Implement npm Detection (30 min) + +**Create**: `src/auth/npm-diagnostics.ts` +- Function: `getNpmIgnoreScripts(cwd?: string): Promise` +- Implementation in PRP Step 1 (complete code provided) +- Edge cases: npm not in PATH, timeout (2s), Windows env vars + +### 2. Update Remediation Logic (45 min) + +**File**: `src/auth/credential-store.ts` +- Line 291: Make `getRemediation()` async +- Line 306: Add `const npmIgnoreScripts = await getNpmIgnoreScripts()` +- Lines 327, 316, 337: Replace "npm rebuild keytar" with conditional block +- All 3 platforms: linux, darwin, win32 + +**Note**: All 6 call sites already use `await` (no caller updates needed) + +### 3. Extract Keytar Loader for Testing (15 min) + +**Create**: `src/auth/keytar-loader.ts` +```typescript +export async function loadKeytar() { + const keytar = await import('keytar'); + return keytar; +} +``` + +**Update**: `src/auth/credential-store.ts` +- Replace `await getKeytar()` calls with `await loadKeytar()` + +### 4. Add Tests (45 min) + +**File**: `tests/unit/credential-store.test.ts` +- Mock `npm-diagnostics` and `keytar-loader` +- 4 test cases (complete patterns in PRP Step 4B) +- Verify remediation includes override when ignore-scripts=true + +### 5. Update Documentation (15 min) + +**File**: `docs/USAGE.md` or existing keytar docs +- Add troubleshooting bullet for ignore-scripts + +--- + +## Verification Gates (MUST PASS) + +```bash +# Build +npm run build + +# Tests (should still be 319 or similar) +npm test + +# Security check (no new credential paths) +rg -n "process\.env\.(GROK_API_KEY|XAI_API_KEY)" src/ +# Should return: 0 matches + +# Manual verification (if you have access to npm ignore-scripts environment) +npm config set ignore-scripts true +grok auth doctor +# Should show: npm_config_ignore_scripts=false npm rebuild keytar --foreground-scripts +``` + +--- + +## Key Implementation Details from PRP + +**Injection Point**: `src/auth/credential-store.ts:327` (in Linux case, similar for darwin/win32) + +**All Call Sites Already Async**: +- `src/auth/auth-service.ts:44, 142, 282` +- `src/index.tsx:176` +- `src/commands/handlers/auth.ts:74` +- `src/commands/handlers/auth-tui.ts:86` + +**Test Mocking Pattern**: Use `vi.mock()` for npm-diagnostics and keytar-loader + +**Edge Cases Handled**: npm timeout, npm not in PATH, Windows env vars, unexpected output → all return `null` (graceful fallback) + +--- + +## Acceptance Criteria + +✅ When `keytar.node` missing + `npm ignore-scripts=true`: + - `grok auth doctor` shows `npm_config_ignore_scripts=false` override command + +✅ When `keytar.node` missing + `npm ignore-scripts=false`: + - Standard remediation (no override) + +✅ When npm detection fails: + - Graceful fallback to standard remediation + +✅ All tests pass, build passes, no security regressions + +--- + +## Reference + +**Full PRP**: `docs/PRP-KEYTAR-IGNORE-SCRIPTS-DETECTION.md` +**Readiness Review**: `docs/PRP-KEYTAR-READINESS-REVIEW.md` +**Repo**: `airplne/grok-cli` +**Current Branch**: `feature/command-palette` (or create new from `main`) diff --git a/docs/GROK-SUBAGENT-SMOKE-TEST.md b/docs/GROK-SUBAGENT-SMOKE-TEST.md new file mode 100644 index 0000000..e43d6f9 --- /dev/null +++ b/docs/GROK-SUBAGENT-SMOKE-TEST.md @@ -0,0 +1,15 @@ +Subagent smoke test (REAL Task execution) + +1) Call the Task tool with EXACT JSON: +{"subagent_type":"explore","prompt":"Use only Glob/Read/Grep. Restrict to src/**. Find computeToolCallEvidence and return exact file:line:content for (a) the taskSuccesses increment line and (b) where subagentsSpawned is derived from taskSuccesses. Start with token-only search: Grep: \"taskSuccesses\" (do NOT use operator-regex like taskSuccesses++ or taskSuccesses+=). Then Read the surrounding lines for context and quote the exact matching code."} + +2) Call the Task tool with EXACT JSON (expected to fail): +{"subagent_type":"does-not-exist","prompt":"noop"} + +After both run: +- Tell me to look at the magenta “Evidence (system-verified)” box; it must show Tools executed: Task(2) and Subagents spawned (via Task): 1 (Task 1/2 succeeded). +- Tell me to expand the [Task] outputs (↑/↓ to select, e to expand) and confirm the successful Task output starts with: + === SUBAGENT RUN (system) === + and includes: + === SUBAGENT TRACE === +- Do NOT paste or fabricate any “=== EVIDENCE ===” blocks in your message; only point me to the system evidence box and the expandable [Task] outputs. diff --git a/docs/PRP-BACKSPACE-INVESTIGATION.md b/docs/PRP-BACKSPACE-INVESTIGATION.md new file mode 100644 index 0000000..052238c --- /dev/null +++ b/docs/PRP-BACKSPACE-INVESTIGATION.md @@ -0,0 +1,333 @@ +# PRP: Backspace/Delete Key Input Failure Investigation + +**Repo**: `airplne/grok-cli` +**Branch**: `feature/command-palette` (PR #11) +**Severity**: CRITICAL - Input editing completely broken +**Environment**: Pop!_OS 22.04 GNOME Terminal + tmux +**Latest Commit**: `ee90236` + +--- + +## Problem Statement + +**Backspace and Delete keys do nothing** in the main chat input prompt. + +**Observable Behavior**: +- User types text (works) +- User presses Backspace → cursor doesn't move, text doesn't delete +- User presses Delete → no effect +- Cursor appears frozen at end of input + +**Impact**: Makes the CLI unusable for editing prompts. Users cannot correct typos or edit text. + +--- + +## Environment + +- **OS**: Pop!_OS 22.04 LTS (Ubuntu-based) +- **Terminal**: GNOME Terminal 3.44.0 +- **Shell**: Bash (also tested in tmux) +- **Node**: v22.21.1 +- **grok-cli**: v2.0.4 +- **Installed via**: `npm install -g` (symlinked to repo: `/home/aip0rt/Desktop/grok-cli`) + +**Terminal Settings**: +- Backspace key behavior: Unknown (need to verify with `cat -v` test) +- Possibly sends: `^?` (DEL/\x7f) or `^H` (BS/\x08) + +--- + +## Reproduction Steps + +1. Run: `grok` +2. Type: `hello world` +3. Press: Backspace +4. Observe: Text doesn't delete, cursor doesn't move + +**Consistent across**: +- GNOME Terminal (native) +- tmux session +- Both typing and pasted input + +--- + +## Attempted Fixes (Chronological) + +### Fix #1: Byte Code Detection (`9d44946`) +**Theory**: Ink's `key.backspace` flag not set on Pop!_OS. +**Implementation**: Added byte code fallbacks: +```typescript +const isBackspace = + key.backspace || + input === '\x7f' || // DEL + input === '\b' || // BS + input === '\x08'; // Control-H +``` +**Result**: Still broken ❌ + +### Fix #2: Remove Unconditional Return (`724ddc8`) +**Theory**: App-level handler blocking input. +**Implementation**: Removed `return;` on line 486 in tool navigation handler. +**Result**: Still broken ❌ + +### Fix #3: InputPrompt Always Active (`ee90236`) +**Theory**: `isActive={selectedToolIndex === null}` disables input. +**Implementation**: Changed to `isActive={true}`. +**Result**: Still broken ❌ + +--- + +## Current State Analysis + +### Code Architecture + +**Input Flow**: +``` +User presses key + ↓ +Ink useInput hook fires + ↓ +InputPrompt useInput handler (src/ui/components/input.tsx:103) + ↓ +Checks: isBackspace / isDelete / isArrow / isText + ↓ +Calls: setEditorState(prev => deleteBackward(prev)) + ↓ +React state update triggers re-render + ↓ +Display updates +``` + +**Current Handler** (`src/ui/components/input.tsx:232-244`): +```typescript +const isBackspace = + key.backspace || + input === '\x7f' || + input === '\b' || + input === '\x08'; + +if (isBackspace) { + setEditorState(prev => deleteBackward(prev)); + setPaletteDismissed(false); + if (debugMode) setLastKeyEvent(...); + return; +} +``` + +### Potential Root Causes (Ranked by Likelihood) + +**1. useInput Hook Not Firing** (HIGH) +- Ink's `useInput` requires `isActive` option +- Current: `}, { isActive });` where `isActive` prop should be `true` +- **Issue**: If there's ANOTHER useInput handler in the component tree consuming the event, it won't reach InputPrompt +- **Check**: App.tsx has its own useInput (line 461) - might be consuming events first + +**2. Wrong Byte Sequence** (MEDIUM) +- Pop!_OS might send non-standard Backspace sequence +- Need actual debug output to confirm +- **Check**: Debug overlay should show `input="..."` and `flags=[...]` + +**3. State Update Not Triggering Render** (LOW) +- `setEditorState` called but React doesn't re-render +- Unlikely but possible with `useMemo` dependencies +- **Check**: Debug should show state change in real-time + +**4. Event Consumed by Parent** (HIGH) +- App.tsx useInput (line 461) runs BEFORE InputPrompt useInput +- If it doesn't return early, BOTH handlers fire (correct) +- If it returns early without checking key type, input blocked +- **Check**: App.tsx handler must ONLY handle tool navigation keys (j/k/↑/↓/e/Esc), not Backspace + +--- + +## Architectural Context + +### Multiple useInput Handlers (CRITICAL) + +**Handler #1**: `src/ui/app.tsx:461` (App-level, ALWAYS ACTIVE) +```typescript +useInput((input, key) => { + if (key.ctrl && input === 'c') { exit(); } + + // Tool navigation (when idle + tools exist + !palette) + if (state === 'idle' && completedTools.length > 0 && !isCommandPaletteOpen) { + // Handles: j/k/↑/↓/e/Esc + // MUST NOT handle: Backspace, Delete, letters, arrows when not navigating + } +}); +``` + +**Handler #2**: `src/ui/components/input.tsx:103` (InputPrompt, conditional) +```typescript +useInput((input, key) => { + // Handles: Backspace, Delete, arrows, typing, palette nav +}, { isActive }); // ← Now should be true +``` + +**Problem**: If App handler doesn't return early for non-navigation keys, both handlers fire. If both call setState, which one wins? + +### State Management + +**Editor State**: +```typescript +const [editorState, setEditorState] = useState({ + value: '', + cursorIndex: 0 +}); +``` + +**Updates via**: +- `deleteBackward(state)` → returns new state with char removed +- React should re-render when state object reference changes + +--- + +## Proposed Solutions + +### Solution A: Verify Handler Ordering (IMMEDIATE) + +**Diagnostic Steps**: +1. Run `grok` +2. Press Ctrl+D (enable debug) +3. Type `a` +4. Press Backspace +5. Paste debug output showing: + - Key event details + - Action taken + - State before/after + +**If debug shows "BACKSPACE" action but state doesn't change**: +→ State update bug (immutability issue or React not re-rendering) + +**If debug shows nothing**: +→ useInput not firing (check `isActive` or handler ordering) + +### Solution B: Isolate InputPrompt (TEST) + +**Create minimal test**: +```typescript +// Temporarily comment out App-level useInput handler (line 461) +// Test if Backspace works in InputPrompt alone +``` + +**If this fixes it**: +→ App handler is interfering (consuming events or wrong return logic) + +### Solution C: Check React State Updates + +**Verify deleteBackward returns new object**: +```typescript +// src/ui/utils/input-editor.ts:53 +export function deleteBackward(state: EditorState): EditorState { + if (cursorIndex === 0) return state; // Same object! + return { value: before + after, cursorIndex: cursorIndex - 1 }; // New object +} +``` + +**Issue**: If cursor at 0, returns SAME object → React won't re-render. +**But**: This shouldn't affect middle/end deletions. + +### Solution D: Raw stdin Interference + +**Check bracketed paste listener**: +```typescript +// src/ui/components/input.tsx:77-100 +stdin.on('data', handleData) // Might be consuming Backspace bytes? +``` + +**If stdin listener sees `\x7f` before useInput**: +→ It processes as paste data, never reaches useInput + +--- + +## Verification Protocol + +### Phase 1: Capture Evidence (REQUIRED) + +**User must run these commands and paste output**: + +```bash +# 1. What does your Backspace send? +cat -v +# Press Backspace, then Ctrl+C +# Should show: ^? (DEL) or ^H (BS) + +# 2. Verify build is current +ls -la dist/ui/components/input.js | head -1 +git rev-parse HEAD + +# 3. Run grok with debug +grok +# Press Ctrl+D (enables debug) +# Type: abc +# Press: Backspace +# PASTE the magenta debug box output +``` + +### Phase 2: Code Inspection + +**Check these potential blockers**: + +1. **App-level useInput consumes event** (`src/ui/app.tsx:461-520`): + ```bash + # Search for any path that doesn't return early + rg -A5 "if \(state === 'idle'" src/ui/app.tsx + ``` + +2. **stdin listener intercepts Backspace** (`src/ui/components/input.tsx:77-100`): + ```bash + # Check if stdin.on('data') processes \x7f + grep -A20 "stdin.on('data'" src/ui/components/input.tsx + ``` + +3. **Multiple useInput hooks conflict**: + ```bash + # Count useInput calls + rg -c "useInput\(" src/ui/ + ``` + +### Phase 3: Nuclear Option (If Debug Shows Nothing) + +**Replace entire useInput with logging**: +```typescript +useInput((input, key) => { + console.error('INPUT:', JSON.stringify(input), 'BACKSPACE:', key.backspace); + // ... rest of handler +}, { isActive: true }); // Force true, ignore prop +``` + +Then check terminal stderr for logs. + +--- + +## Acceptance Criteria + +- User types text +- User presses Backspace +- Character before cursor deletes +- Cursor moves left +- Works in GNOME Terminal AND tmux +- Works whether tool outputs exist or not + +--- + +## Files Modified (PR #11) + +- `src/ui/components/input.tsx` - Backspace detection + debug overlay +- `src/ui/app.tsx` - Tool navigation handler + isActive prop +- `src/ui/utils/input-editor.ts` - deleteBackward/deleteForward logic +- `tests/unit/input-editor.test.ts` - Byte code tests + +--- + +## Next Steps for Codex Team + +**IMMEDIATE**: Get user to paste debug output (Ctrl+D in grok). + +**Based on debug output**: +- **If shows BACKSPACE action but no state change** → State immutability bug +- **If shows FILTERED** → Wrong byte sequence detection +- **If shows nothing** → useInput not firing (ordering/isActive issue) +- **If shows SKIP: pasting** → stdin listener consuming bytes + +**Fallback**: Disable App-level useInput temporarily to isolate issue. diff --git a/docs/PRP-KEYTAR-IGNORE-SCRIPTS-DETECTION.md b/docs/PRP-KEYTAR-IGNORE-SCRIPTS-DETECTION.md new file mode 100644 index 0000000..318734e --- /dev/null +++ b/docs/PRP-KEYTAR-IGNORE-SCRIPTS-DETECTION.md @@ -0,0 +1,506 @@ +# PRP: Keytar Native Binding Missing When `npm ignore-scripts=true` + +**Repo**: `airplne/grok-cli` +**Area**: Auth / Keychain (`keytar`) +**Severity**: HIGH (blocks `grok auth login`, disables AI mode) +**Target Audience**: Claude/Codex Development Team +**Status**: Ready for Execution + +--- + +## Problem Statement + +Users running `grok auth login` may see: + +``` +Error: System keychain unavailable +``` + +The CLI currently suggests rebuilding `keytar`: + +``` +npm rebuild keytar +``` + +However, on machines where **npm is configured with `ignore-scripts=true`**, `npm rebuild keytar` will **skip keytar’s install/build scripts** and **will not produce** `node_modules/keytar/build/Release/keytar.node`. Users can follow the remediation exactly and still remain broken. + +This is common on hardened dev machines where `ignore-scripts` is enabled globally for supply-chain safety. + +--- + +## Root Cause (Confirmed) + +- `keytar@7.9.0` builds its native binding via install script: + - `prebuild-install || node-gyp rebuild` +- When `npm ignore-scripts=true`, **install scripts do not run**, so the native binding is never built. +- In addition, `prebuild-install` may try to fetch prebuilt artifacts from GitHub; if the machine is offline or GitHub is blocked, that download can fail—but **the build-from-source fallback works** as long as build tooling + `libsecret-1-dev` are installed (Linux). + +--- + +## Goals + +1. Detect and surface the **`ignore-scripts=true`** scenario when keytar’s native binding is missing. +2. Provide a **command-scoped fix** that does not require permanently changing npm config: + - `npm_config_ignore_scripts=false npm rebuild keytar --foreground-scripts` +3. Keep the existing security posture: + - **KEYCHAIN-ONLY** auth remains (no env var/file credential ingestion). +4. Keep output concise, actionable, and platform-aware. + +--- + +## Non-Goals + +- Do not run `sudo` automatically or install OS packages. +- Do not automatically change user npm configuration. +- Do not add any alternate credential storage (env vars, `.env`, config files). + +--- + +## Reproduction + +### Setup a failing environment + +```bash +# Confirm scripts are disabled +npm config get ignore-scripts + +# (If needed for repro only) +npm config set ignore-scripts true + +cd /path/to/grok-cli +npm ci +node -e "import('keytar').then(()=>console.log('ok')).catch(e=>{console.error(e);process.exit(1)})" +``` + +Expected failure: +- `Cannot find module '../build/Release/keytar.node'` + +### Observe CLI behavior + +```bash +grok auth doctor +grok auth login +``` + +Expected: +- keychain reported unavailable (reason `missing-native-binding`) + +--- + +## Proposed Solution + +Enhance keychain remediation to detect **npm scripts disabled** and show the correct rebuild command. + +### Implementation Overview + +**Primary change**: `src/auth/credential-store.ts` + +When `getAvailability()` classifies `reason === 'missing-native-binding'`: +1. Determine whether `npm ignore-scripts` is enabled (best-effort). +2. If enabled, append a clear remediation block explaining why `npm rebuild keytar` did not work and how to override. + +Optionally, print the detected value explicitly in `grok auth doctor` output. + +--- + +## Implementation Plan + +### Step 1 — Add npm "ignore-scripts" detection helper + +Create `src/auth/npm-diagnostics.ts`: + +**Function Signature**: +```typescript +export async function getNpmIgnoreScripts(cwd?: string): Promise +``` + +**Implementation with Edge Case Handling**: +```typescript +import { promisify } from 'util'; +import { execFile } from 'child_process'; + +const execFilePromise = promisify(execFile); + +export async function getNpmIgnoreScripts(cwd?: string): Promise { + // Fast path: check env vars (Windows uses both forms) + const envValue = process.env.npm_config_ignore_scripts || + process.env.NPM_CONFIG_IGNORE_SCRIPTS; + + if (envValue === 'true') return true; + if (envValue === 'false') return false; + + // Slow path: run npm command with timeout + try { + const { stdout } = await execFilePromise('npm', ['config', 'get', 'ignore-scripts'], { + cwd: cwd || process.cwd(), + timeout: 2000, // 2 second max (graceful fallback if npm is slow) + windowsHide: true, // Prevent flash on Windows + }); + + const trimmed = stdout.trim().toLowerCase(); + if (trimmed === 'true') return true; + if (trimmed === 'false') return false; + + return null; // Unexpected output (e.g., "undefined", garbage) + } catch (error) { + // Edge cases: + // - npm not in PATH (ENOENT) + // - npm timeout (ETIMEDOUT) + // - Permission denied + // - npm config corruption + // All handled gracefully: return null (feature disabled, use standard remediation) + return null; + } +} +``` + +**Edge Cases Handled**: +1. **npm not in PATH**: Returns `null`, falls back to standard remediation +2. **npm timeout**: Returns `null` after 2s +3. **Windows env var naming**: Checks both `npm_config_*` and `NPM_CONFIG_*` +4. **Unexpected output**: Returns `null` instead of crashing +5. **Permission errors**: Gracefully returns `null` + +Constraints: +- Must be safe: read-only, no prompts, no writes. +- Must be resilient: do not throw; return `null` on failure. + +### Step 2 — Update `CredentialStore.getAvailability()` remediation + +**File**: `src/auth/credential-store.ts` +**Location**: Line 306 - `if (reason === 'missing-native-binding')` +**Method Signature Change**: `getAvailability()` becomes `async getAvailability()` + +**Current Code Structure** (lines 306-346): +```typescript +if (reason === 'missing-native-binding') { + switch (platform) { + case 'linux': + return [ + 'Install build tools and libsecret:', + ' sudo apt update', + ' sudo apt install -y build-essential libsecret-1-dev', + '', + 'Then rebuild keytar:', // ← Line ~325 + ' cd ' + process.cwd(), + ' npm rebuild keytar', // ← Line ~327 INJECTION POINT + ].join('\n'); + // ... macOS, Windows similar structure + } +} +``` + +**Modified Implementation**: +```typescript +// Line 291: Make method async +private static async getRemediation(reason: string): Promise { + const platform = process.platform; + + // ... existing code for other reasons ... + + if (reason === 'missing-native-binding') { + // NEW: Detect npm ignore-scripts + const npmIgnoreScripts = await getNpmIgnoreScripts(process.cwd()); + + // Build base platform instructions + const platformSteps: string[] = []; + + switch (platform) { + case 'linux': + platformSteps.push( + 'Install build tools and libsecret:', + ' sudo apt update', + ' sudo apt install -y build-essential libsecret-1-dev', + '' + ); + break; + // ... macOS, Windows cases + } + + // Add rebuild instructions with ignore-scripts detection + platformSteps.push('Then rebuild keytar:'); + platformSteps.push(' cd ' + process.cwd()); + platformSteps.push(''); + + if (npmIgnoreScripts === true) { + platformSteps.push( + 'NOTE: Detected npm ignore-scripts=true (install scripts disabled)', + 'This prevented keytar from building its native binding.', + '', + 'Fix (command-scoped override - recommended):', + ' npm_config_ignore_scripts=false npm rebuild keytar --foreground-scripts', + '', + 'Or (permanent config change):', + ' npm config set ignore-scripts false', + ' npm rebuild keytar' + ); + } else { + platformSteps.push(' npm rebuild keytar'); + } + + return platformSteps.join('\n'); + } + + // ... other cases +} +``` + +**Callers to Update** (must add `await`): +- `src/auth/credential-store.ts:235` - getAvailability() method itself becomes async +- `src/auth/credential-store.ts:273` - return statement (already in async context) +- `src/commands/handlers/auth.ts` - any call to `CredentialStore.getAvailability()` +- `src/commands/handlers/auth-tui.ts` - any call to `CredentialStore.getAvailability()` + +**Find all call sites**: +```bash +rg -n "CredentialStore\.getAvailability\(\)" src/ +``` + +Notes: +- Preserve existing platform-specific build tool instructions (Linux/macOS/Windows) because users may still need them. +- Keep remediation ordering: explain why, then give commands. + +### Step 3 — Improve `grok auth doctor` output (optional) + +If Step 2 only appends to remediation, doctor output will already include it. + +Optional improvement (if the team prefers explicit structured output): +- Add a line in `src/commands/handlers/auth.ts` and `src/commands/handlers/auth-tui.ts`: + - `npm ignore-scripts: true|false|unknown` + +### Step 4 — Tests + +**File**: `tests/unit/credential-store.test.ts` + +**Challenge**: Testing requires mocking dynamic `import('keytar')` which is complex in Vitest. + +**Recommended Approach**: Extract keytar loading to separate testable module. + +#### Step 4A: Extract Keytar Loading (Enables Testing) + +**Create**: `src/auth/keytar-loader.ts` +```typescript +export async function loadKeytar() { + const keytar = await import('keytar'); + return keytar; +} +``` + +**Update**: `src/auth/credential-store.ts` +```typescript +// Replace direct import('keytar') with loadKeytar() +import { loadKeytar } from './keytar-loader.js'; + +public static async getAvailability(): Promise { + try { + const keytar = await loadKeytar(); + // ... rest of checks + } +} +``` + +#### Step 4B: Add Tests with Mocking + +**File**: `tests/unit/credential-store.test.ts` + +```typescript +import { vi, describe, it, expect, beforeEach } from 'vitest'; + +// Mock both npm-diagnostics and keytar-loader +vi.mock('../../src/auth/npm-diagnostics.js', () => ({ + getNpmIgnoreScripts: vi.fn() +})); + +vi.mock('../../src/auth/keytar-loader.js', () => ({ + loadKeytar: vi.fn() +})); + +import { getNpmIgnoreScripts } from '../../src/auth/npm-diagnostics.js'; +import { loadKeytar } from '../../src/auth/keytar-loader.js'; +import { CredentialStore } from '../../src/auth/credential-store.js'; + +describe('ignore-scripts detection', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('shows override command when ignore-scripts=true', async () => { + // Mock keytar loading failure (native binding missing) + (loadKeytar as any).mockRejectedValue( + new Error("Cannot find module '../build/Release/keytar.node'") + ); + + // Mock npm detection: ignore-scripts enabled + (getNpmIgnoreScripts as any).mockResolvedValue(true); + + const availability = await CredentialStore.getAvailability(); + + expect(availability.available).toBe(false); + expect(availability.reason).toBe('missing-native-binding'); + expect(availability.remediation).toContain('npm_config_ignore_scripts=false'); + expect(availability.remediation).toContain('--foreground-scripts'); + }); + + it('shows standard remediation when ignore-scripts=false', async () => { + (loadKeytar as any).mockRejectedValue( + new Error("Cannot find module '../build/Release/keytar.node'") + ); + + (getNpmIgnoreScripts as any).mockResolvedValue(false); + + const availability = await CredentialStore.getAvailability(); + + expect(availability.remediation).not.toContain('npm_config_ignore_scripts=false'); + expect(availability.remediation).toContain('npm rebuild keytar'); + }); + + it('gracefully handles npm detection failure (null)', async () => { + (loadKeytar as any).mockRejectedValue( + new Error("Cannot find module '../build/Release/keytar.node'") + ); + + // npm not in PATH, timeout, or other error + (getNpmIgnoreScripts as any).mockResolvedValue(null); + + const availability = await CredentialStore.getAvailability(); + + // Should still show standard remediation (safe fallback) + expect(availability.available).toBe(false); + expect(availability.remediation).toContain('npm rebuild keytar'); + expect(availability.remediation).not.toContain('npm_config_ignore_scripts=false'); + }); + + it('does not affect successful keytar load', async () => { + // Mock successful keytar load + (loadKeytar as any).mockResolvedValue({ + getPassword: vi.fn(), + setPassword: vi.fn(), + deletePassword: vi.fn(), + }); + + const availability = await CredentialStore.getAvailability(); + + // npm check should not run when keytar works + expect(getNpmIgnoreScripts).not.toHaveBeenCalled(); + expect(availability.available).toBe(true); + }); +}); +``` + +Constraints: +- Tests must be deterministic and must not require actual system keychain access. +- All mocks must be cleared between tests (`beforeEach`). + +### Step 5 — Documentation + +Update `docs/USAGE.md` (or add a short note to `docs/PRP-KEYTAR-KEYCHAIN-AVAILABILITY-FIX.md`) to include: +- A troubleshooting bullet: "If `npm config get ignore-scripts` is `true`, rebuild with `npm_config_ignore_scripts=false`". + +--- + +## Code Anchors & Call Sites + +### Signature Change Impact + +**GOOD NEWS**: `CredentialStore.getAvailability()` is **already async** (line 239). + +**All Call Sites** (already have `await` - no changes needed): +1. `src/auth/auth-service.ts:44` ✅ +2. `src/auth/auth-service.ts:142` ✅ +3. `src/auth/auth-service.ts:282` ✅ +4. `src/index.tsx:176` ✅ +5. `src/commands/handlers/auth.ts:74` ✅ +6. `src/commands/handlers/auth-tui.ts:86` ✅ + +**Only Change Needed**: Make `getRemediation()` async (currently private static sync). + +**Current** (line ~291): +```typescript +private static getRemediation(reason: string): string { +``` + +**Updated**: +```typescript +private static async getRemediation(reason: string): Promise { +``` + +**Caller Update** (line ~275 in getAvailability): +```typescript +// OLD: +remediation: this.getRemediation('missing-native-binding'), + +// NEW: +remediation: await this.getRemediation('missing-native-binding'), +``` + +### Injection Point Details + +**File**: `src/auth/credential-store.ts` +**Function**: `getRemediation(reason: string)` (private static) +**Line**: ~306 (current), exact line may shift + +**Current Structure**: +```typescript +// Line 306 +if (reason === 'missing-native-binding') { + switch (platform) { + case 'linux': + // Lines 320-328 + return [ + 'Install build tools and libsecret:', + ' sudo apt update', + ' sudo apt install -y build-essential libsecret-1-dev', + '', + 'Then rebuild keytar:', + ' cd ' + process.cwd(), + ' npm rebuild keytar', // ← REPLACE THIS LINE + ].join('\n'); + } +} +``` + +**Modification Strategy**: +1. Change return type: `getRemediation(reason: string): string` → `async getRemediation(reason: string): Promise` +2. Insert `const npmIgnoreScripts = await getNpmIgnoreScripts();` before switch +3. Replace static "npm rebuild keytar" line with conditional block: + - If `npmIgnoreScripts === true`: insert ignore-scripts override instructions + - Otherwise: keep standard "npm rebuild keytar" +4. Repeat for all 3 platform cases (darwin, linux, win32) + +--- + +## Acceptance Criteria + +1. When `keytar.node` is missing and `npm ignore-scripts=true`: + - `grok auth doctor` output includes explicit instructions using: + - `npm_config_ignore_scripts=false npm rebuild keytar --foreground-scripts` +2. When `keytar.node` is missing and `npm ignore-scripts=false` (or unknown): + - Output remains similar to current remediation (no incorrect advice). +3. All tests pass: + - `npm test` +4. Build passes: + - `npm run build` + +--- + +## Verification Commands (Manual) + +```bash +cd /path/to/grok-cli + +# Simulate the failure mode +npm config set ignore-scripts true +npm ci +grok auth doctor + +# Fix using the recommended override +npm_config_ignore_scripts=false npm rebuild keytar --foreground-scripts +node -e "import('keytar').then(()=>console.log('keytar ok')).catch(e=>{console.error(e);process.exit(1)})" + +grok auth doctor +grok auth login + +# Clean up repro setting (optional) +npm config set ignore-scripts false +``` + diff --git a/docs/PRP-KEYTAR-READINESS-REVIEW.md b/docs/PRP-KEYTAR-READINESS-REVIEW.md new file mode 100644 index 0000000..6ad16c7 --- /dev/null +++ b/docs/PRP-KEYTAR-READINESS-REVIEW.md @@ -0,0 +1,368 @@ +# PRP Readiness Review: Keytar Ignore-Scripts Detection + +**Reviewer**: Claude Sonnet 4.5 +**PRP**: `docs/PRP-KEYTAR-IGNORE-SCRIPTS-DETECTION.md` +**Verdict**: ✅ **GO with Minor Enhancements** +**Confidence**: 95% + +--- + +## Executive Summary + +The PRP is **90% execution-ready** with clear problem definition, well-structured implementation plan, and complete verification protocol. Recommended enhancements before execution: add specific code anchors, clarify test mocking strategy, and document edge cases. + +--- + +## Strengths ✅ + +### 1. Problem Definition (Excellent) +- Clear symptom: keychain unavailable error +- Root cause identified: npm ignore-scripts prevents build +- User impact: Blocks AI mode entirely +- Environment specificity: Hardened dev machines + +### 2. Security Boundaries (Excellent) +- Explicitly preserves keychain-only auth +- No sudo automation +- No config file creation +- Read-only npm detection + +### 3. Implementation Plan Structure (Good) +- 5 logical steps +- Clear separation: detection → remediation → tests → docs +- Platform-aware (Linux/macOS/Windows) + +### 4. Verification Protocol (Excellent) +- Complete reproduction steps +- Fix verification commands +- Cleanup instructions + +--- + +## Gaps Requiring Enhancement + +### Gap #1: Missing Code Anchors (MEDIUM Priority) + +**Issue**: Implementation says "update CredentialStore.getAvailability()" but doesn't specify exact injection point. + +**Current Code** (`src/auth/credential-store.ts:306-346`): +```typescript +if (reason === 'missing-native-binding') { + switch (platform) { + case 'linux': + return [ + 'Install build tools and libsecret:', + ' sudo apt update', + ' sudo apt install -y build-essential libsecret-1-dev', + '', + 'Then rebuild keytar:', + ' cd ' + process.cwd(), + ' npm rebuild keytar', // ← INJECTION POINT + ].join('\n'); + } +} +``` + +**Enhancement Needed**: +```markdown +### Step 2 (Enhanced): +Insert ignore-scripts detection AFTER "Then rebuild keytar:" block: + +```typescript +const npmIgnoreScripts = await getNpmIgnoreScripts(process.cwd()); + +const rebuildSteps = npmIgnoreScripts === true + ? [ + 'NOTE: Detected npm ignore-scripts=true', + '', + 'Fix (command-scoped override):', + ' npm_config_ignore_scripts=false npm rebuild keytar --foreground-scripts', + '', + 'Or (permanent):', + ' npm config set ignore-scripts false', + ' npm rebuild keytar', + ] + : [ + 'Then rebuild keytar:', + ' cd ' + process.cwd(), + ' npm rebuild keytar', + ]; +``` + +Append rebuildSteps to platform-specific instructions. +``` + +### Gap #2: Test Mocking Strategy (MEDIUM Priority) + +**Issue**: Says "Mock keytar import failure" and "Mock the new npm helper" but doesn't specify the mocking approach. + +**Enhancement Needed**: +```markdown +### Step 4 (Enhanced): + +**Mocking Approach**: +1. Use `vi.mock()` to mock `src/auth/npm-diagnostics.ts` +2. Test cases: + +**Test A**: ignore-scripts=true detected +```typescript +vi.mock('../src/auth/npm-diagnostics.js', () => ({ + getNpmIgnoreScripts: vi.fn().mockResolvedValue(true) +})); + +// Then test CredentialStore with simulated keytar failure +const availability = await CredentialStore.getAvailability(); +expect(availability.remediation).toContain('npm_config_ignore_scripts=false'); +``` + +**Test B**: ignore-scripts=false (normal) +```typescript +vi.mock('../src/auth/npm-diagnostics.js', () => ({ + getNpmIgnoreScripts: vi.fn().mockResolvedValue(false) +})); + +const availability = await CredentialStore.getAvailability(); +expect(availability.remediation).not.toContain('npm_config_ignore_scripts'); +``` + +**Test C**: npm detection fails (null) +```typescript +vi.mock('../src/auth/npm-diagnostics.js', () => ({ + getNpmIgnoreScripts: vi.fn().mockResolvedValue(null) +})); + +const availability = await CredentialStore.getAvailability(); +// Should still show standard remediation (graceful fallback) +``` + +**Note**: CredentialStore.getAvailability() is currently static/sync. If adding async npm detection, signature becomes `async getAvailability()` and all callers must await. +``` + +### Gap #3: Edge Cases (LOW Priority) + +**Missing Considerations**: +1. What if `npm config get ignore-scripts` times out? (Should return `null` and fallback gracefully) +2. What if npm is not in PATH? (Should catch exec error, return `null`) +3. What if ignore-scripts is per-project vs global? (Command works for both) +4. What if Windows uses different env var name? (Test on Windows) + +**Enhancement**: +```markdown +### Step 1 (Add Error Handling): + +```typescript +export async function getNpmIgnoreScripts(cwd?: string): Promise { + // Fast path: check env vars + const envValue = process.env.npm_config_ignore_scripts || process.env.NPM_CONFIG_IGNORE_SCRIPTS; + if (envValue === 'true') return true; + if (envValue === 'false') return false; + + // Slow path: run npm command with timeout + try { + const { stdout } = await execFilePromise('npm', ['config', 'get', 'ignore-scripts'], { + cwd: cwd || process.cwd(), + timeout: 2000, // 2 second max + }); + + const trimmed = stdout.trim().toLowerCase(); + if (trimmed === 'true') return true; + if (trimmed === 'false') return false; + return null; // Unexpected output + } catch (error) { + // npm not found, timeout, or other error + return null; // Graceful fallback + } +} +``` +``` + +### Gap #4: Async Signature Change (MEDIUM Priority) + +**Issue**: `CredentialStore.getAvailability()` is currently **synchronous**. + +**Current**: +```typescript +// src/auth/credential-store.ts:235 +public static getAvailability(): KeychainAvailability { + try { + // ... synchronous checks + } +} +``` + +**Required Change**: +```typescript +public static async getAvailability(): Promise { + try { + // ... existing checks + + // NEW: Async npm detection + if (reason === 'missing-native-binding') { + const npmIgnoreScripts = await getNpmIgnoreScripts(); + // ... build remediation + } + } +} +``` + +**Callers to Update**: +- `src/commands/handlers/auth.ts` +- `src/commands/handlers/auth-tui.ts` +- Anywhere `CredentialStore.getAvailability()` is called + +**Enhancement**: PRP should list all call sites and confirm async/await added. + +--- + +## Recommended Additions to PRP + +### Section: "Implementation Details" + +Add after Step 2: + +```markdown +### Code Anchors + +**File**: `src/auth/credential-store.ts` + +**Location**: Line 306 - `if (reason === 'missing-native-binding')` + +**Current Structure**: +```typescript +switch (platform) { + case 'linux': + return [ + 'Install build tools...', + '', + 'Then rebuild keytar:', + ' cd ' + process.cwd(), + ' npm rebuild keytar', // ← Line ~327 + ].join('\n'); +} +``` + +**Modification Strategy**: +1. Make getRemediation() async +2. Call await getNpmIgnoreScripts() +3. Conditionally insert ignore-scripts block BEFORE "npm rebuild keytar" line +4. Format: + ``` + Then rebuild keytar: + cd /path/to/grok-cli + + [IF ignore-scripts=true, INSERT:] + NOTE: npm ignore-scripts is enabled (prevents keytar build) + + Fix (temporary override): + npm_config_ignore_scripts=false npm rebuild keytar --foreground-scripts + + Or (permanent): + npm config set ignore-scripts false + npm rebuild keytar + + [ELSE:] + npm rebuild keytar + ``` + +**Callers to Update**: +- `src/commands/handlers/auth.ts` → `await CredentialStore.getAvailability()` +- `src/commands/handlers/auth-tui.ts` → `await CredentialStore.getAvailability()` +- `src/index.tsx` (if used at startup) +``` + +### Section: "Testing Strategy" + +Add test implementation details: + +```markdown +### Test Mocking Pattern + +Use Vitest module mocks: + +```typescript +// tests/unit/credential-store.test.ts + +import { vi } from 'vitest'; + +// Mock npm-diagnostics module +vi.mock('../../src/auth/npm-diagnostics.js', () => ({ + getNpmIgnoreScripts: vi.fn() +})); + +import { getNpmIgnoreScripts } from '../../src/auth/npm-diagnostics.js'; +import { CredentialStore } from '../../src/auth/credential-store.js'; + +describe('ignore-scripts detection', () => { + it('shows override when ignore-scripts=true', async () => { + (getNpmIgnoreScripts as any).mockResolvedValue(true); + + // Simulate keytar missing (harder - may need to mock dynamic import) + const availability = await CredentialStore.getAvailability(); + + expect(availability.remediation).toContain('npm_config_ignore_scripts=false'); + }); +}); +``` + +**Challenge**: Testing getAvailability() with mocked keytar import failure requires: +- Mocking dynamic import() calls (complex in Vitest) +- OR: Extract keytar loading to separate testable function + +**Recommendation**: Extract keytar loading: +```typescript +// src/auth/keytar-loader.ts +export async function loadKeytar() { + const keytar = await import('keytar'); + return keytar; +} +``` + +Then mock loadKeytar() in tests. +``` + +--- + +## GO/NO-GO Decision + +### ✅ GO - Execute with Enhancements + +**Verdict**: PRP is **execution-ready** with minor clarifications. + +**Recommended Pre-Execution Steps**: + +1. **Add to PRP** (5 minutes): + - Code anchors (exact line numbers) + - Async signature change call sites + - Test mocking pattern + +2. **Quick Spike** (15 minutes): + - Verify `npm config get ignore-scripts` works on Pop!_OS + - Check if timeout is needed (test on slow machines) + +3. **Implementation Order**: + - Step 1 (npm-diagnostics.ts) → Step 4 (tests) → Step 2 (credential-store) → Step 5 (docs) + - Test-first ensures mocking works before modifying core code + +**Estimated Effort**: 2-3 hours (implementation + tests + verification) + +**Risk**: LOW (read-only detection, graceful fallbacks, well-scoped) + +--- + +## Minor Concerns (Non-Blocking) + +1. **Async getAvailability()**: Signature change affects multiple call sites (need grep to find all) +2. **Test Complexity**: Mocking dynamic imports is tricky in Vitest (may need refactor) +3. **npm Dependency**: Assumes npm is in PATH (document this assumption) + +--- + +## Final Recommendation + +**Status**: ✅ **APPROVED FOR EXECUTION** + +**Condition**: Add the 3 enhancements above to PRP before starting implementation. + +**Priority**: HIGH (blocks auth on hardened systems) + +**Next Step**: Update PRP with code anchors and test mocking details, then execute. diff --git a/src/commands/handlers/auto-edit.ts b/src/commands/handlers/auto-edit.ts new file mode 100644 index 0000000..fef79ee --- /dev/null +++ b/src/commands/handlers/auto-edit.ts @@ -0,0 +1,61 @@ +/** + * /auto-edit Command Handler + * + * Enables or disables session-level auto-accept for Edit/Write tool confirmations. + * Does NOT persist across sessions (resets on CLI exit). + * Does NOT affect Bash (always requires confirmation). + */ + +import type { Command, ParsedCommand, CommandContext, CommandResult } from '../types.js'; + +export const autoEditCommand: Command = { + name: 'auto-edit', + description: 'Enable/disable auto-accept for Edit/Write confirmations (session only)', + usage: '/auto-edit ', + arguments: [ + { + name: 'mode', + description: 'Enable (on) or disable (off) auto-accept edits', + required: true, + type: 'string', + }, + ], + examples: [ + '/auto-edit on', + '/auto-edit off', + ], + aliases: ['autoedit', 'ae'], + + async execute(parsed: ParsedCommand, context: CommandContext): Promise { + const mode = parsed.args[0]?.toLowerCase(); + + if (!mode || (mode !== 'on' && mode !== 'off')) { + return { + success: false, + error: 'Invalid argument. Usage: /auto-edit \n\n' + + 'Examples:\n' + + ' /auto-edit on - Enable auto-accept for Edit/Write\n' + + ' /auto-edit off - Disable auto-accept', + }; + } + + const enabled = mode === 'on'; + + // Note: This command can't directly set the state since it doesn't have access. + // We need to add an action type for this. + return { + success: true, + output: enabled + ? 'Auto-accept edits: ENABLED (session)\n\n' + + 'Edit and Write tools will no longer prompt for confirmation.\n' + + 'Bash will still require confirmation.\n\n' + + 'This setting resets when you exit grok.' + : 'Auto-accept edits: DISABLED\n\n' + + 'Edit and Write tools will now prompt for confirmation.', + action: { + type: 'set_auto_edit', + enabled, + }, + }; + }, +}; diff --git a/src/commands/handlers/prompt.ts b/src/commands/handlers/prompt.ts index e00987b..635e14e 100644 --- a/src/commands/handlers/prompt.ts +++ b/src/commands/handlers/prompt.ts @@ -10,6 +10,7 @@ import path from 'path'; import type { Command, ParsedCommand, CommandContext, CommandResult } from '../types.js'; import { validateAndOpen } from '../../security/path-validator.js'; +import { sanitizeControlSequences } from '../utils.js'; // Maximum file size for prompt files: 256KB const MAX_PROMPT_SIZE = 262144; @@ -34,9 +35,9 @@ export const promptCommand: Command = { aliases: ['promptfile', 'pf', 'loadprompt'], async execute(parsed: ParsedCommand, context: CommandContext): Promise { - const filePath = parsed.args[0]; + const filePathRaw = parsed.args[0]; - if (!filePath) { + if (!filePathRaw) { return { success: false, error: 'Missing file path. Usage: /prompt \n\n' + @@ -46,6 +47,10 @@ export const promptCommand: Command = { }; } + // Belt-and-suspenders: sanitize control sequences from file path + // Parser already sanitizes, but this ensures no leakage at handler level + const filePath = sanitizeControlSequences(filePathRaw).trim(); + // Resolve path relative to cwd const resolvedPath = path.isAbsolute(filePath) ? filePath diff --git a/src/commands/index.ts b/src/commands/index.ts index 63e00c1..7e2fe47 100644 --- a/src/commands/index.ts +++ b/src/commands/index.ts @@ -7,6 +7,7 @@ import type { Command, ParsedCommand, CommandContext, CommandResult } from './types.js'; import { parseInput, isCommand, parseCommand } from './parser.js'; +import { getDidYouMeanSuggestions } from '../ui/utils/command-suggestions.js'; // Import command handlers import { helpCommand } from './handlers/help.js'; @@ -16,6 +17,7 @@ import { exitCommand } from './handlers/exit.js'; import { historyCommand } from './handlers/history.js'; import { authCommand } from './handlers/auth-tui.js'; import { promptCommand } from './handlers/prompt.js'; +import { autoEditCommand } from './handlers/auto-edit.js'; /** * Command Registry class @@ -78,9 +80,26 @@ class CommandRegistry { const command = this.getCommand(parsed.name); if (!command) { + // Try to provide "did you mean" suggestions (top 3) + const suggestions = getDidYouMeanSuggestions(parsed.name, this.getAllCommands(), 3); + + let errorMessage = `Unknown command: /${parsed.name}`; + + if (suggestions.length > 0) { + errorMessage += '\n\nDid you mean:'; + for (const suggestion of suggestions) { + const suggestedName = suggestion.matchedAlias + ? `${suggestion.command.name} (alias: ${suggestion.matchedAlias})` + : suggestion.command.name; + errorMessage += `\n /${suggestedName}`; + } + } + + errorMessage += '\n\nUse /help to see all available commands.'; + return { success: false, - error: `Unknown command: /${parsed.name}\n\nUse /help to see available commands.`, + error: errorMessage, }; } @@ -135,6 +154,7 @@ export function getRegistry(): CommandRegistry { registryInstance.register(historyCommand); registryInstance.register(authCommand); registryInstance.register(promptCommand); + registryInstance.register(autoEditCommand); } return registryInstance; @@ -159,3 +179,4 @@ export { exitCommand } from './handlers/exit.js'; export { historyCommand } from './handlers/history.js'; export { authCommand } from './handlers/auth-tui.js'; export { promptCommand } from './handlers/prompt.js'; +export { autoEditCommand } from './handlers/auto-edit.js'; diff --git a/src/commands/parser.ts b/src/commands/parser.ts index f4c1197..932e318 100644 --- a/src/commands/parser.ts +++ b/src/commands/parser.ts @@ -6,6 +6,7 @@ */ import type { ParsedCommand, ParseResult } from './types.js'; +import { sanitizeControlSequences } from './utils.js'; /** * Check if input starts with a slash command @@ -132,10 +133,14 @@ function tokenize(input: string): string[] { } /** - * Parse user input and determine if it's a command or regular message + * Parse user input and determine if it's a command or regular message. + * Sanitizes control sequences before parsing to prevent corruption. */ export function parseInput(input: string): ParseResult { - const trimmed = input.trim(); + // Sanitize control sequences (including bracketed paste markers) + // This prevents terminal codes from corrupting command arguments + const sanitized = sanitizeControlSequences(input); + const trimmed = sanitized.trim(); if (!trimmed) { return { type: 'empty' }; diff --git a/src/commands/types.ts b/src/commands/types.ts index 382d7dd..c455b6b 100644 --- a/src/commands/types.ts +++ b/src/commands/types.ts @@ -30,6 +30,7 @@ export type CommandAction = | { type: 'clear' } | { type: 'set_model'; model: string } | { type: 'submit_prompt'; content: string } + | { type: 'set_auto_edit'; enabled: boolean } | { type: 'none' }; /** diff --git a/src/commands/utils.ts b/src/commands/utils.ts new file mode 100644 index 0000000..17f1923 --- /dev/null +++ b/src/commands/utils.ts @@ -0,0 +1,58 @@ +/** + * Command Utilities + * + * Shared utilities for command processing, including control sequence sanitization. + */ + +/** + * Remove control sequences from user input. + * + * Removes: + * - Bracketed paste markers: \x1b[200~, \x1b[201~, [200~, [201~ + * - General ANSI/VT control sequences: \x1b[... + * + * This prevents terminal control codes from corrupting command arguments, + * especially file paths in commands like /prompt. + * + * @param input - Raw user input that may contain control sequences + * @returns Sanitized string with control sequences removed + */ +export function sanitizeControlSequences(input: string): string { + let sanitized = input; + + // Remove bracketed paste markers (with ESC prefix) + sanitized = sanitized.replace(/\x1b\[200~/g, ''); + sanitized = sanitized.replace(/\x1b\[201~/g, ''); + + // Remove bracketed paste markers (ESC-stripped variants) + // These can appear when terminals split the ESC from the sequence + sanitized = sanitized.replace(/\[200~/g, ''); + sanitized = sanitized.replace(/\[201~/g, ''); + + // Remove general ANSI/VT control sequences + // Pattern: ESC [ followed by numbers/semicolons, ending with a letter + // Examples: \x1b[0m (reset), \x1b[1;32m (bold green), \x1b[2J (clear screen) + sanitized = sanitized.replace(/\x1b\[[0-9;]*[a-zA-Z]/g, ''); + + // Remove other common ESC sequences (OSC, CSI variants) + sanitized = sanitized.replace(/\x1b\][0-9;]*[^\x07]*\x07/g, ''); // OSC sequences + sanitized = sanitized.replace(/\x1bO[A-Z]/g, ''); // SS3 sequences + + return sanitized; +} + +/** + * Check if a string contains bracketed paste markers. + * Useful for debugging and testing. + * + * @param input - String to check + * @returns true if bracketed paste markers are present + */ +export function containsPasteMarkers(input: string): boolean { + return ( + input.includes('\x1b[200~') || + input.includes('\x1b[201~') || + input.includes('[200~') || + input.includes('[201~') + ); +} diff --git a/src/ui/app.tsx b/src/ui/app.tsx index 436f19a..2120a35 100644 --- a/src/ui/app.tsx +++ b/src/ui/app.tsx @@ -11,6 +11,7 @@ import { TodoDisplay } from './components/todo-display.js'; import { TodoTool } from '../tools/todo.js'; import { getRegistry, isCommand, CommandContext, HistoryMessage, CommandResult } from '../commands/index.js'; import { getDefaultModel, resolveModelId } from '../config/models.js'; +import { shouldAutoApprove, ConfirmDecision } from './utils/confirm-decision.js'; interface AppProps { initialPrompt?: string; @@ -40,6 +41,7 @@ interface PendingConfirmation { toolName: string; args: Record; resolve: (approved: boolean) => void; + setAutoAccept?: (enabled: boolean) => void; } // Generate stable IDs @@ -63,6 +65,12 @@ export function App({ initialPrompt, model: initialModel, apiKey, offlineMode = const [selectedToolIndex, setSelectedToolIndex] = useState(null); const [expandedTools, setExpandedTools] = useState>(new Set()); + // Command palette state (to prevent arrow key conflicts) + const [isCommandPaletteOpen, setIsCommandPaletteOpen] = useState(false); + + // Auto-accept edits state (session-only, resets on exit) + const [autoAcceptEdits, setAutoAcceptEdits] = useState(false); + // Create agent with current model (only if we have API key) const agentRef = useRef(null); @@ -110,11 +118,21 @@ export function App({ initialPrompt, model: initialModel, apiKey, offlineMode = // Handle confirmation dialog const handleConfirmation = useCallback((toolName: string, args: Record): Promise => { + // Check if auto-accept is enabled for this tool + if (shouldAutoApprove(toolName, autoAcceptEdits)) { + return Promise.resolve(true); // Auto-approve without showing dialog + } + return new Promise((resolve) => { - setPendingConfirm({ toolName, args, resolve }); + setPendingConfirm({ + toolName, + args, + resolve, + setAutoAccept: setAutoAcceptEdits, + }); setState('confirming'); }); - }, []); + }, [autoAcceptEdits]); // Create command context const createCommandContext = useCallback((): CommandContext => ({ @@ -184,6 +202,9 @@ export function App({ initialPrompt, model: initialModel, apiKey, offlineMode = case 'submit_prompt': // Return the prompt content to be submitted return { handled: true, submitPrompt: commandResult.action.content }; + case 'set_auto_edit': + setAutoAcceptEdits(commandResult.action.enabled); + break; } } @@ -406,9 +427,18 @@ export function App({ initialPrompt, model: initialModel, apiKey, offlineMode = }, [exit, executeCommand, runAgent, offlineMode]); // Handle confirmation response - const handleConfirmResponse = useCallback((approved: boolean) => { + const handleConfirmResponse = useCallback((decision: ConfirmDecision) => { if (pendingConfirm) { - pendingConfirm.resolve(approved); + // Handle decision + if (decision === 'auto_accept_edits') { + pendingConfirm.setAutoAccept?.(true); + pendingConfirm.resolve(true); // Also approve this request + } else if (decision === 'allow_once') { + pendingConfirm.resolve(true); + } else { + pendingConfirm.resolve(false); + } + setPendingConfirm(null); setState('running'); } @@ -436,7 +466,8 @@ export function App({ initialPrompt, model: initialModel, apiKey, offlineMode = // Tool output navigation: // - Enter navigation with ↑/↓ (does not interfere with typing) // - When a tool is selected: j/k or ↑/↓ to navigate, e to expand/collapse, Esc to return to input - if (state === 'idle' && completedTools.length > 0) { + // - Do NOT activate if command palette is open (palette gets arrow priority) + if (state === 'idle' && completedTools.length > 0 && !isCommandPaletteOpen) { const inputLower = input.toLowerCase(); const isNavigatingTools = selectedToolIndex !== null; @@ -452,7 +483,7 @@ export function App({ initialPrompt, model: initialModel, apiKey, offlineMode = return; } - return; + // Don't return here - let other keys pass through to InputPrompt } // Navigate down: j or down arrow @@ -522,6 +553,7 @@ export function App({ initialPrompt, model: initialModel, apiKey, offlineMode = - Claude Code for xAI Grok [{currentModel}] {offlineMode && [OFFLINE]} + {autoAcceptEdits && [AUTO-EDIT: ON]} {/* Offline Mode Banner */} @@ -602,6 +634,7 @@ export function App({ initialPrompt, model: initialModel, apiKey, offlineMode = toolName={pendingConfirm.toolName} args={pendingConfirm.args} onResponse={handleConfirmResponse} + autoAcceptEdits={autoAcceptEdits} /> )} @@ -610,7 +643,11 @@ export function App({ initialPrompt, model: initialModel, apiKey, offlineMode = {/* Input prompt */} {state === 'idle' && ( - + )} ); diff --git a/src/ui/components/command-palette.tsx b/src/ui/components/command-palette.tsx new file mode 100644 index 0000000..1ddc274 --- /dev/null +++ b/src/ui/components/command-palette.tsx @@ -0,0 +1,82 @@ +import React, { memo } from 'react'; +import { Box, Text } from 'ink'; +import type { CommandSuggestion } from '../utils/command-suggestions.js'; + +interface CommandPaletteProps { + suggestions: CommandSuggestion[]; + selectedIndex: number; +} + +/** + * Command Palette Component + * + * Displays filtered command suggestions below the input prompt. + * Shows command name, description, aliases, and usage. + */ +export const CommandPalette = memo(function CommandPalette({ + suggestions, + selectedIndex, +}: CommandPaletteProps) { + if (suggestions.length === 0) { + return null; + } + + return ( + + {/* Header */} + + + Commands + + + {' '}({suggestions.length} {suggestions.length === 1 ? 'match' : 'matches'}) + + + + {/* Suggestions list */} + + {suggestions.map((suggestion, index) => { + const isSelected = index === selectedIndex; + const { command, matchedAlias } = suggestion; + + return ( + + {/* Selection indicator */} + + {isSelected ? '▶ ' : ' '} + + + {/* Command name */} + + /{command.name} + + + {/* Matched alias indicator */} + {matchedAlias && ( + + {' '}(via {matchedAlias}) + + )} + + {/* Description */} + + {' '}- {command.description} + + + ); + })} + + + {/* Help hint */} + + + Tab: autocomplete • ↑/↓: navigate • Esc: close • Enter: submit + + + + ); +}); diff --git a/src/ui/components/confirm.tsx b/src/ui/components/confirm.tsx index b3793e8..6627031 100644 --- a/src/ui/components/confirm.tsx +++ b/src/ui/components/confirm.tsx @@ -1,37 +1,170 @@ -import React from 'react'; +import React, { useState } from 'react'; import { Box, Text, useInput } from 'ink'; +import { getConfirmOptions, ConfirmDecision } from '../utils/confirm-decision.js'; interface ConfirmDialogProps { toolName: string; args: Record; - onResponse: (approved: boolean) => void; + onResponse: (decision: ConfirmDecision) => void; + autoAcceptEdits?: boolean; } -export function ConfirmDialog({ toolName, args, onResponse }: ConfirmDialogProps) { - useInput((input) => { - if (input.toLowerCase() === 'y') { - onResponse(true); - } else if (input.toLowerCase() === 'n' || input === '\r') { - onResponse(false); +export function ConfirmDialog({ + toolName, + args, + onResponse, + autoAcceptEdits = false, +}: ConfirmDialogProps) { + const options = getConfirmOptions(toolName); + const [selectedIndex, setSelectedIndex] = useState(0); + const [debugMode, setDebugMode] = useState(false); + const [lastKeyEvent, setLastKeyEvent] = useState<{ input: string; flags: string } | null>(null); + + useInput((input, key) => { + // Debug mode: capture key events for diagnostics + if (debugMode) { + const flags = [ + key.tab ? 'tab' : null, + (key as any).shift ? 'shift' : null, + key.return ? 'return' : null, + key.escape ? 'escape' : null, + key.upArrow ? 'up' : null, + key.downArrow ? 'down' : null, + ].filter(Boolean).join(',') || 'none'; + + const escaped = input + .split('') + .map(c => { + const code = c.charCodeAt(0); + if (code < 32 || code === 127) { + return `\\x${code.toString(16).padStart(2, '0')}`; + } + return c; + }) + .join(''); + + setLastKeyEvent({ input: escaped || '(empty)', flags }); + } + + // Toggle debug mode with '?' key + if (input === '?') { + setDebugMode(prev => !prev); + return; + } + + // Shift+Tab handling varies across terminals: + // - Some send ESC [ Z + // - Some send ESC [ 1 ; 2 Z + // - Some terminals split ESC, leaving "[Z" / "[1;2Z" + // - Some set key.tab + key.shift + // - SS3 variant: ESC O Z (may arrive as 'OZ' after Ink strips ESC) + const isShiftTab = + input === '\x1b[Z' || + input === '\x1b[1;2Z' || + input === '[Z' || + input === '[1;2Z' || + input === 'OZ' || // SS3 fallback (ESC O Z with ESC stripped) + input === '\x1bOZ' || // SS3 with ESC + (key.tab && (key as any).shift); + + if (isShiftTab) { + setSelectedIndex(prev => (prev - 1 + options.length) % options.length); + return; + } + + // Tab: cycle forward + if (key.tab || input === '\t') { + setSelectedIndex(prev => (prev + 1) % options.length); + return; + } + + // Arrow keys: alternative navigation + if (key.upArrow) { + setSelectedIndex(prev => (prev - 1 + options.length) % options.length); + return; + } + + if (key.downArrow) { + setSelectedIndex(prev => (prev + 1) % options.length); + return; + } + + // Enter: select current option + if (key.return) { + onResponse(options[selectedIndex].decision); + return; + } + + // Letter shortcuts (y/a/n) + const inputLower = input.toLowerCase(); + const matchedOption = options.find(o => o.key === inputLower); + if (matchedOption) { + onResponse(matchedOption.decision); + return; + } + + // Escape: deny (optional convenience) + if (key.escape) { + onResponse('deny'); + return; } }); return ( Permission Required + + {/* Auto-accept status indicator */} + {autoAcceptEdits && ( + + + Auto-accept edits: ENABLED (session) + + + )} + Tool: {toolName} + {JSON.stringify(args, null, 2).slice(0, 200)} - - Allow? - [y]es - / - [n]o + + {/* Options */} + + {options.map((option, index) => { + const isSelected = index === selectedIndex; + return ( + + + {isSelected ? '▶ ' : ' '} + + + [{option.key}] {option.label} + + + ); + })} + + + {/* Help hint */} + + + Tab/Shift+Tab/↑/↓: navigate • Enter/letter: select • Esc: deny + {!debugMode && ' • ?: debug'} + + + {/* Debug overlay */} + {debugMode && lastKeyEvent && ( + + + DEBUG: input="{lastKeyEvent.input}" flags=[{lastKeyEvent.flags}] + + + )} ); } diff --git a/src/ui/components/input.tsx b/src/ui/components/input.tsx index a30c062..ad23667 100644 --- a/src/ui/components/input.tsx +++ b/src/ui/components/input.tsx @@ -1,23 +1,67 @@ -import React, { useState, useRef, useEffect } from 'react'; +import React, { useState, useRef, useEffect, useMemo } from 'react'; import { Box, Text, useInput, useStdin } from 'ink'; import { PASTE_START, PASTE_END, PasteState, processPasteData, - getMultiLineDisplay, } from '../utils/bracketed-paste.js'; +import { + createEditorState, + insertAtCursor, + deleteBackward, + deleteForward, + moveCursor, + moveCursorToBoundary, + setValue, + getDisplayWithCursor, + EditorState, +} from '../utils/input-editor.js'; +import { getCommandSuggestions } from '../utils/command-suggestions.js'; +import { CommandPalette } from './command-palette.js'; +import { getRegistry } from '../../commands/index.js'; +import { containsPasteMarkers } from '../../commands/utils.js'; interface InputPromptProps { onSubmit: (value: string) => void; isActive?: boolean; + onPaletteVisibilityChange?: (visible: boolean) => void; } -export function InputPrompt({ onSubmit, isActive = true }: InputPromptProps) { - const [value, setValue] = useState(''); +export function InputPrompt({ onSubmit, isActive = true, onPaletteVisibilityChange }: InputPromptProps) { + const [editorState, setEditorState] = useState(createEditorState()); const [pasteState, setPasteState] = useState({ isPasting: false, buffer: '' }); + const [selectedSuggestionIndex, setSelectedSuggestionIndex] = useState(0); + const [paletteDismissed, setPaletteDismissed] = useState(false); const { stdin } = useStdin(); + // Get available commands for suggestions + const commands = useMemo(() => getRegistry().getAllCommands(), []); + + // Compute suggestions based on current input + const suggestions = useMemo(() => { + // Don't show palette during paste or if not active + if (pasteState.isPasting || !isActive) { + return []; + } + + return getCommandSuggestions(editorState.value, commands); + }, [editorState.value, commands, pasteState.isPasting, isActive]); + + // Only show palette while editing the command token (before first space) + const isEditingCommand = editorState.value.startsWith('/') && !editorState.value.includes(' '); + const showPalette = isEditingCommand && suggestions.length > 0 && !paletteDismissed; + + // Reset selection when suggestions change + useEffect(() => { + setSelectedSuggestionIndex(0); + }, [suggestions.length]); + + // Notify parent when palette visibility changes + useEffect(() => { + onPaletteVisibilityChange?.(showPalette); + }, [showPalette, onPaletteVisibilityChange]); + // Enable bracketed paste mode when component mounts useEffect(() => { if (!stdin || !isActive) return; @@ -44,9 +88,10 @@ export function InputPrompt({ onSubmit, isActive = true }: InputPromptProps) { setPasteState(result.state); } - // Append completed paste content + // Append completed paste content at cursor if (result.contentToAppend !== null) { - setValue(prev => prev + result.contentToAppend); + setEditorState(prev => insertAtCursor(prev, result.contentToAppend!)); + setPaletteDismissed(false); } }; @@ -60,40 +105,152 @@ export function InputPrompt({ onSubmit, isActive = true }: InputPromptProps) { // Don't process input during paste (handled by raw stdin listener) if (pasteState.isPasting) return; + // Command palette keyboard handling (when visible) + if (showPalette) { + // Navigate down + if (key.downArrow) { + setSelectedSuggestionIndex(prev => Math.min(prev + 1, suggestions.length - 1)); + return; + } + + // Navigate up + if (key.upArrow) { + setSelectedSuggestionIndex(prev => Math.max(prev - 1, 0)); + return; + } + + // Tab: autocomplete selected suggestion + if (key.tab) { + const selected = suggestions[selectedSuggestionIndex]; + if (selected) { + const commandName = selected.command.name; + // Extract args portion (if any) + const parts = editorState.value.split(' '); + const args = parts.slice(1).join(' '); + // Replace command portion with selected, preserve args + setEditorState(setValue(editorState, `/${commandName}${args ? ' ' + args : ' '}`)); + setPaletteDismissed(false); // Palette will auto-hide due to space + } + return; + } + + // Escape: close palette + if (key.escape) { + setPaletteDismissed(true); + setSelectedSuggestionIndex(0); + return; + } + } + + // Cursor movement (left/right arrows, Home/End). + // Palette uses ↑/↓; left/right and Home/End are safe to use even when palette is visible. + // Home/End support varies by terminal; handle both Ink keys and common escape sequences. + const isHome = (key as any).home || input === '\x1b[H' || input === '\x1bOH' || input === '\x1b[1~'; + const isEnd = (key as any).end || input === '\x1b[F' || input === '\x1bOF' || input === '\x1b[4~'; + + if (isHome) { + setEditorState(prev => moveCursorToBoundary(prev, 'start')); + return; + } + + if (isEnd) { + setEditorState(prev => moveCursorToBoundary(prev, 'end')); + return; + } + + if (key.leftArrow || input === '\x1b[D' || input === '\x1bOD') { + setEditorState(prev => moveCursor(prev, 'left')); + return; + } + + if (key.rightArrow || input === '\x1b[C' || input === '\x1bOC') { + setEditorState(prev => moveCursor(prev, 'right')); + return; + } + + // Regular input handling if (key.return) { - if (value.trim()) { - onSubmit(value.trim()); - setValue(''); + if (editorState.value.trim()) { + onSubmit(editorState.value.trim()); + setEditorState(createEditorState()); + setSelectedSuggestionIndex(0); + setPaletteDismissed(false); } - } else if (key.backspace || key.delete) { - setValue(prev => prev.slice(0, -1)); - } else if (!key.ctrl && !key.meta && input) { - // Filter out escape sequences that might slip through - if (!input.startsWith('\x1b')) { - setValue(prev => prev + input); + return; + } + + // Backspace: detect both Ink flag and common byte codes + // Pop!_OS/tmux may send \x7f (DEL) or \b (BS) without setting key.backspace + const isBackspace = + key.backspace || + input === '\x7f' || // DEL (most common) + input === '\b' || // BS + input === '\x08'; // Control-H + + if (isBackspace) { + setEditorState(prev => deleteBackward(prev)); + setPaletteDismissed(false); + return; + } + + // Delete: detect both Ink flag and terminal sequences + const isDelete = + key.delete || + input === '\x1b[3~' || // Standard delete sequence + input === '[3~'; // ESC-stripped variant + + if (isDelete) { + setEditorState(prev => deleteForward(prev)); + setPaletteDismissed(false); + return; + } + + // Text insertion: only for printable characters + // Explicitly exclude control characters that might slip through + if (!key.ctrl && !key.meta && input && !key.tab) { + // Filter out: + // - Escape sequences + // - Bracketed paste markers + // - Control characters (< 0x20, except tab which is already filtered) + const hasControlChars = input.split('').some(c => { + const code = c.charCodeAt(0); + return code < 32 && code !== 9; // Allow tab (9) but block other control chars + }); + + if (!input.startsWith('\x1b') && !containsPasteMarkers(input) && !hasControlChars) { + setEditorState(prev => insertAtCursor(prev, input)); + setPaletteDismissed(false); } } }, { isActive }); // Get display representation - const { display, isMultiLine, lineCount } = getMultiLineDisplay(value); + const displayInfo = getDisplayWithCursor(editorState); return ( > - {display} + {displayInfo.display} - {pasteState.isPasting ? ' [pasting...]' : '|'} + {pasteState.isPasting ? ' [pasting...]' : ''} - {isMultiLine && ( + {displayInfo.isMultiLine && ( - (multi-line prompt: {value.length} chars) + (multi-line prompt: {editorState.value.length} chars) )} + + {/* Command palette (when input starts with /) */} + {showPalette && ( + + )} ); } diff --git a/src/ui/utils/command-suggestions.ts b/src/ui/utils/command-suggestions.ts new file mode 100644 index 0000000..39b3fd1 --- /dev/null +++ b/src/ui/utils/command-suggestions.ts @@ -0,0 +1,205 @@ +/** + * Command Suggestions Utility + * + * Provides filtering, ranking, and fuzzy matching for command palette. + * Supports exact match, prefix match, alias match, substring match, and fuzzy match. + */ + +import type { Command } from '../../commands/types.js'; + +/** + * Match types for suggestion ranking + */ +export type MatchType = 'exact' | 'prefix' | 'alias-exact' | 'alias-prefix' | 'substring' | 'fuzzy'; + +/** + * Command suggestion with match metadata + */ +export interface CommandSuggestion { + command: Command; + matchType: MatchType; + score: number; + matchedAlias?: string; +} + +/** + * Calculate Levenshtein distance between two strings. + * Used for fuzzy matching typos. + * + * @param a - First string + * @param b - Second string + * @returns Edit distance (lower is more similar) + */ +export function levenshteinDistance(a: string, b: string): number { + if (a === b) return 0; + if (a.length === 0) return b.length; + if (b.length === 0) return a.length; + + const matrix: number[][] = []; + + // Initialize first column + for (let i = 0; i <= b.length; i++) { + matrix[i] = [i]; + } + + // Initialize first row + for (let j = 0; j <= a.length; j++) { + matrix[0][j] = j; + } + + // Fill matrix + for (let i = 1; i <= b.length; i++) { + for (let j = 1; j <= a.length; j++) { + const cost = a[j - 1] === b[i - 1] ? 0 : 1; + matrix[i][j] = Math.min( + matrix[i - 1][j] + 1, // deletion + matrix[i][j - 1] + 1, // insertion + matrix[i - 1][j - 1] + cost // substitution + ); + } + } + + return matrix[b.length][a.length]; +} + +/** + * Determine match type and score for a query against a command. + * + * @param query - User's search query (e.g., "pr", "promt") + * @param command - Command definition + * @returns Match metadata or null if no match + */ +export function matchCommand(query: string, command: Command): CommandSuggestion | null { + const queryLower = query.toLowerCase(); + const nameLower = command.name.toLowerCase(); + + // Exact match (highest priority) + if (queryLower === nameLower) { + return { command, matchType: 'exact', score: 1000 }; + } + + // Prefix match on name + if (nameLower.startsWith(queryLower)) { + return { command, matchType: 'prefix', score: 900 }; + } + + // Check aliases + for (const alias of command.aliases) { + const aliasLower = alias.toLowerCase(); + + // Exact alias match + if (queryLower === aliasLower) { + return { command, matchType: 'alias-exact', score: 800, matchedAlias: alias }; + } + + // Prefix alias match + if (aliasLower.startsWith(queryLower)) { + return { command, matchType: 'alias-prefix', score: 700, matchedAlias: alias }; + } + } + + // Substring match in name + if (nameLower.includes(queryLower)) { + return { command, matchType: 'substring', score: 600 }; + } + + // Fuzzy match (typo tolerance) + // Only for queries >= 3 chars, distance <= 2 + if (query.length >= 3) { + const distance = levenshteinDistance(queryLower, nameLower); + if (distance <= 2) { + return { command, matchType: 'fuzzy', score: 500 - distance * 100 }; + } + + // Also check fuzzy against aliases + for (const alias of command.aliases) { + const aliasDistance = levenshteinDistance(queryLower, alias.toLowerCase()); + if (aliasDistance <= 2) { + return { command, matchType: 'fuzzy', score: 400 - aliasDistance * 100, matchedAlias: alias }; + } + } + } + + return null; +} + +/** + * Get command suggestions based on user input. + * + * @param input - Raw user input (may start with /) + * @param commands - Available commands + * @returns Sorted suggestions (best matches first) + */ +export function getCommandSuggestions(input: string, commands: Command[]): CommandSuggestion[] { + // If input doesn't start with /, return empty + if (!input.startsWith('/')) { + return []; + } + + // Extract command portion (before first space or entire string) + const commandPart = input.slice(1).split(' ')[0].trim(); + + // Empty query (just "/") - return all commands + if (commandPart === '') { + return commands.map(cmd => ({ + command: cmd, + matchType: 'prefix' as MatchType, + score: 900, + })).sort((a, b) => a.command.name.localeCompare(b.command.name)); + } + + // Match against all commands + const suggestions: CommandSuggestion[] = []; + + for (const command of commands) { + const match = matchCommand(commandPart, command); + if (match) { + suggestions.push(match); + } + } + + // Sort by score (descending), then by name (ascending) for stability + suggestions.sort((a, b) => { + if (b.score !== a.score) { + return b.score - a.score; + } + return a.command.name.localeCompare(b.command.name); + }); + + return suggestions; +} + +/** + * Get "did you mean" suggestions for an unknown command. + * + * @param unknownCommand - The command that wasn't found (without leading /) + * @param commands - Available commands + * @param maxSuggestions - Maximum number of suggestions to return (default: 3) + * @returns Top suggestions (fuzzy/substring matches only) + */ +export function getDidYouMeanSuggestions( + unknownCommand: string, + commands: Command[], + maxSuggestions: number = 3 +): CommandSuggestion[] { + const suggestions = getCommandSuggestions(`/${unknownCommand}`, commands); + + // Only return fuzzy/substring matches (not exact/prefix - those would have been found) + const relevantSuggestions = suggestions.filter( + s => s.matchType === 'fuzzy' || s.matchType === 'substring' + ); + + return relevantSuggestions.slice(0, maxSuggestions); +} + +/** + * Get single "did you mean" suggestion for an unknown command. + * @deprecated Use getDidYouMeanSuggestions() for multiple suggestions + */ +export function getDidYouMeanSuggestion( + unknownCommand: string, + commands: Command[] +): CommandSuggestion | null { + const suggestions = getDidYouMeanSuggestions(unknownCommand, commands, 1); + return suggestions[0] || null; +} diff --git a/src/ui/utils/confirm-decision.ts b/src/ui/utils/confirm-decision.ts new file mode 100644 index 0000000..6a3897a --- /dev/null +++ b/src/ui/utils/confirm-decision.ts @@ -0,0 +1,82 @@ +/** + * Confirmation Decision Logic + * + * Pure functions for determining auto-approval and confirmation options. + * Used by the confirmation dialog and app-level permission handling. + */ + +export type ConfirmDecision = 'allow_once' | 'auto_accept_edits' | 'deny'; + +export interface ConfirmOption { + key: string; + label: string; + decision: ConfirmDecision; +} + +/** + * Tools that are eligible for auto-accept edits feature. + * Bash is explicitly excluded (always requires confirmation). + */ +const AUTO_ACCEPT_ELIGIBLE_TOOLS = ['Edit', 'Write']; + +/** + * Determine if a tool call should be auto-approved without showing dialog. + * + * @param toolName - Name of the tool being called + * @param autoAcceptEdits - Whether auto-accept edits is enabled (session state) + * @returns true if should auto-approve, false if dialog should be shown + */ +export function shouldAutoApprove(toolName: string, autoAcceptEdits: boolean): boolean { + if (!autoAcceptEdits) { + return false; // Feature disabled + } + + return AUTO_ACCEPT_ELIGIBLE_TOOLS.includes(toolName); +} + +/** + * Get confirmation options for a specific tool. + * + * Edit/Write tools include "Auto-accept edits (session)" option. + * Bash and other tools only show "Allow once" and "Deny". + * + * @param toolName - Name of the tool requiring confirmation + * @returns Array of confirmation options + */ +export function getConfirmOptions(toolName: string): ConfirmOption[] { + const baseOptions: ConfirmOption[] = [ + { + key: 'y', + label: 'Allow once', + decision: 'allow_once', + }, + ]; + + // Add auto-accept option for Edit/Write + if (AUTO_ACCEPT_ELIGIBLE_TOOLS.includes(toolName)) { + baseOptions.push({ + key: 'a', + label: 'Auto-accept edits (session)', + decision: 'auto_accept_edits', + }); + } + + baseOptions.push({ + key: 'n', + label: 'Deny', + decision: 'deny', + }); + + return baseOptions; +} + +/** + * Check if a tool is eligible for auto-accept edits. + * Used for validation and UI logic. + * + * @param toolName - Name of the tool + * @returns true if tool can be auto-accepted + */ +export function isAutoAcceptEligible(toolName: string): boolean { + return AUTO_ACCEPT_ELIGIBLE_TOOLS.includes(toolName); +} diff --git a/src/ui/utils/input-editor.ts b/src/ui/utils/input-editor.ts new file mode 100644 index 0000000..6ff1b06 --- /dev/null +++ b/src/ui/utils/input-editor.ts @@ -0,0 +1,248 @@ +/** + * Input Editor State Management + * + * Pure functions for cursor-based line editing. + * Supports insertion, deletion, cursor movement, and rendering with cursor marker. + */ + +/** + * Editor state + */ +export interface EditorState { + value: string; + cursorIndex: number; +} + +/** + * Create initial editor state + */ +export function createEditorState(initialValue: string = ''): EditorState { + return { + value: initialValue, + cursorIndex: initialValue.length, + }; +} + +/** + * Insert text at cursor position. + * + * @param state - Current editor state + * @param text - Text to insert + * @returns New editor state with text inserted and cursor advanced + */ +export function insertAtCursor(state: EditorState, text: string): EditorState { + const { value, cursorIndex } = state; + const before = value.slice(0, cursorIndex); + const after = value.slice(cursorIndex); + + return { + value: before + text + after, + cursorIndex: cursorIndex + text.length, + }; +} + +/** + * Delete character before cursor (backspace). + * + * @param state - Current editor state + * @returns New editor state with character deleted + */ +export function deleteBackward(state: EditorState): EditorState { + const { value, cursorIndex } = state; + + if (cursorIndex === 0) { + return state; // Nothing to delete + } + + const before = value.slice(0, cursorIndex - 1); + const after = value.slice(cursorIndex); + + return { + value: before + after, + cursorIndex: cursorIndex - 1, + }; +} + +/** + * Delete character at cursor (delete key). + * + * @param state - Current editor state + * @returns New editor state with character deleted + */ +export function deleteForward(state: EditorState): EditorState { + const { value, cursorIndex } = state; + + if (cursorIndex >= value.length) { + return state; // Nothing to delete + } + + const before = value.slice(0, cursorIndex); + const after = value.slice(cursorIndex + 1); + + return { + value: before + after, + cursorIndex, // Cursor stays at same position + }; +} + +/** + * Move cursor left or right. + * + * @param state - Current editor state + * @param direction - 'left' or 'right' + * @returns New editor state with cursor moved + */ +export function moveCursor( + state: EditorState, + direction: 'left' | 'right' +): EditorState { + const { value, cursorIndex } = state; + + if (direction === 'left') { + return { + value, + cursorIndex: Math.max(0, cursorIndex - 1), + }; + } else { + return { + value, + cursorIndex: Math.min(value.length, cursorIndex + 1), + }; + } +} + +/** + * Move cursor to start or end of input. + * + * @param state - Current editor state + * @param position - 'start' or 'end' + * @returns New editor state with cursor at boundary + */ +export function moveCursorToBoundary( + state: EditorState, + position: 'start' | 'end' +): EditorState { + const { value } = state; + + return { + value, + cursorIndex: position === 'start' ? 0 : value.length, + }; +} + +/** + * Set cursor to specific index (clamped to valid range). + * + * @param state - Current editor state + * @param index - Desired cursor index + * @returns New editor state with cursor at index + */ +export function setCursor(state: EditorState, index: number): EditorState { + const { value } = state; + + return { + value, + cursorIndex: Math.max(0, Math.min(value.length, index)), + }; +} + +/** + * Replace entire value and move cursor to end. + * + * @param state - Current editor state + * @param newValue - New value + * @returns New editor state + */ +export function setValue(state: EditorState, newValue: string): EditorState { + return { + value: newValue, + cursorIndex: newValue.length, + }; +} + +/** + * Render value with cursor marker inserted at cursor position. + * + * @param value - Current input value + * @param cursorIndex - Current cursor position + * @param cursorChar - Character to use for cursor (default: '|') + * @returns Display string with cursor marker + */ +export function renderWithCursor( + value: string, + cursorIndex: number, + cursorChar: string = '|' +): string { + const clampedIndex = Math.max(0, Math.min(value.length, cursorIndex)); + + const before = value.slice(0, clampedIndex); + const after = value.slice(clampedIndex); + + return before + cursorChar + after; +} + +/** + * Get visual display for multi-line input with cursor. + * + * For single-line: shows full value with cursor + * For multi-line: shows first line + line count + cursor indicator + * + * @param state - Current editor state + * @returns Display info + */ +export function getDisplayWithCursor(state: EditorState): { + display: string; + isMultiLine: boolean; + lineCount: number; +} { + const { value, cursorIndex } = state; + const lines = value.split('\n'); + const isMultiLine = lines.length > 1; + + if (!isMultiLine) { + // Single line: render with cursor + return { + display: renderWithCursor(value, cursorIndex), + isMultiLine: false, + lineCount: 1, + }; + } + + // Multi-line: show a compact preview that always includes a visible cursor marker. + // We display: + // - first line + line count + // - the current cursor line (with cursor inserted) if cursor is not on line 1 + let lineStartIndex = 0; + let cursorLine = 0; + + for (let i = 0; i < lines.length; i++) { + const lineEndIndex = lineStartIndex + lines[i].length; + + if (cursorIndex <= lineEndIndex) { + cursorLine = i; + break; + } + + // Skip newline character between lines + lineStartIndex = lineEndIndex + 1; + } + + const cursorColumnIndex = Math.max( + 0, + Math.min(lines[cursorLine].length, cursorIndex - lineStartIndex) + ); + + const summary = `${lines[0]}... [${lines.length} lines]`; + const cursorLineDisplay = renderWithCursor(lines[cursorLine], cursorColumnIndex); + + const display = + cursorLine === 0 + ? `${cursorLineDisplay}... [${lines.length} lines]` + : `${summary}\n(line ${cursorLine + 1}) ${cursorLineDisplay}`; + + return { + display, + isMultiLine: true, + lineCount: lines.length, + }; +} diff --git a/tests/unit/command-parser-sanitize.test.ts b/tests/unit/command-parser-sanitize.test.ts new file mode 100644 index 0000000..9f73553 --- /dev/null +++ b/tests/unit/command-parser-sanitize.test.ts @@ -0,0 +1,175 @@ +import { describe, it, expect } from 'vitest'; +import { parseInput, parseCommand } from '../../src/commands/parser.js'; +import { sanitizeControlSequences, containsPasteMarkers } from '../../src/commands/utils.js'; + +/** + * Tests for control sequence sanitization in command parsing. + * + * Covers: + * - Bracketed paste marker removal (\x1b[200~, \x1b[201~, [200~, [201~) + * - General ANSI/VT sequence removal + * - Parser integration (parseInput sanitizes before processing) + * - Real-world scenarios (pasted /prompt commands) + */ + +describe('control sequence sanitization', () => { + describe('sanitizeControlSequences', () => { + it('should remove bracketed paste start marker with ESC', () => { + const input = '\x1b[200~hello world\x1b[201~'; + const result = sanitizeControlSequences(input); + + expect(result).toBe('hello world'); + }); + + it('should remove bracketed paste markers without ESC prefix', () => { + const input = '[200~hello world[201~'; + const result = sanitizeControlSequences(input); + + expect(result).toBe('hello world'); + }); + + it('should remove ANSI color codes', () => { + const input = 'hello \x1b[1;32mworld\x1b[0m'; + const result = sanitizeControlSequences(input); + + expect(result).toBe('hello world'); + }); + + it('should remove mixed control sequences', () => { + const input = '\x1b[200~\x1b[1mBold Text\x1b[0m\x1b[201~'; + const result = sanitizeControlSequences(input); + + expect(result).toBe('Bold Text'); + }); + + it('should preserve normal text', () => { + const input = 'normal text with no sequences'; + const result = sanitizeControlSequences(input); + + expect(result).toBe(input); + }); + + it('should handle empty string', () => { + const result = sanitizeControlSequences(''); + expect(result).toBe(''); + }); + + it('should remove paste markers from file paths', () => { + const input = 'docs/GROK-SUBAGENT-SMOKE-TEST.md[200~'; + const result = sanitizeControlSequences(input); + + expect(result).toBe('docs/GROK-SUBAGENT-SMOKE-TEST.md'); + }); + }); + + describe('containsPasteMarkers', () => { + it('should detect paste markers with ESC', () => { + expect(containsPasteMarkers('\x1b[200~text')).toBe(true); + expect(containsPasteMarkers('text\x1b[201~')).toBe(true); + }); + + it('should detect paste markers without ESC', () => { + expect(containsPasteMarkers('[200~text')).toBe(true); + expect(containsPasteMarkers('text[201~')).toBe(true); + }); + + it('should return false for clean text', () => { + expect(containsPasteMarkers('normal text')).toBe(false); + }); + }); + + describe('parseInput with sanitization', () => { + it('should parse command with bracketed paste markers (ESC prefix)', () => { + const input = '\x1b[200~/prompt docs/GROK-SUBAGENT-SMOKE-TEST.md\x1b[201~'; + const result = parseInput(input); + + expect(result.type).toBe('command'); + if (result.type === 'command') { + expect(result.command.name).toBe('prompt'); + expect(result.command.args[0]).toBe('docs/GROK-SUBAGENT-SMOKE-TEST.md'); + expect(result.command.args[0]).not.toContain('[200~'); + expect(result.command.args[0]).not.toContain('[201~'); + } + }); + + it('should parse command with ESC-stripped paste markers', () => { + const input = '[200~/prompt docs/GROK-SUBAGENT-SMOKE-TEST.md[201~'; + const result = parseInput(input); + + expect(result.type).toBe('command'); + if (result.type === 'command') { + expect(result.command.name).toBe('prompt'); + expect(result.command.args[0]).toBe('docs/GROK-SUBAGENT-SMOKE-TEST.md'); + } + }); + + it('should parse /prompt with paste markers in middle of path', () => { + const input = '/prompt docs/GROK-SUBAGENT[200~-SMOKE-TEST.md'; + const result = parseInput(input); + + expect(result.type).toBe('command'); + if (result.type === 'command') { + expect(result.command.args[0]).toBe('docs/GROK-SUBAGENT-SMOKE-TEST.md'); + } + }); + + it('should parse /prompt with ANSI codes in path', () => { + const input = '/prompt docs/\x1b[1mGROK\x1b[0m-TEST.md'; + const result = parseInput(input); + + expect(result.type).toBe('command'); + if (result.type === 'command') { + expect(result.command.args[0]).toBe('docs/GROK-TEST.md'); + } + }); + + it('should handle multiple commands with paste markers', () => { + const commands = [ + '\x1b[200~/help\x1b[201~', + '[200~/model grok-4[201~', + '/exit\x1b[200~\x1b[201~', + ]; + + for (const cmd of commands) { + const result = parseInput(cmd); + expect(result.type).toBe('command'); + if (result.type === 'command') { + expect(result.command.rawArgs).not.toContain('[200~'); + expect(result.command.rawArgs).not.toContain('[201~'); + } + } + }); + + it('should preserve message content after sanitization', () => { + const input = '\x1b[200~Hello, this is a message\x1b[201~'; + const result = parseInput(input); + + expect(result.type).toBe('message'); + if (result.type === 'message') { + expect(result.content).toBe('Hello, this is a message'); + } + }); + }); + + describe('parseCommand after sanitization', () => { + it('should extract clean arguments from corrupted input', () => { + // Simulate what happens after parseInput sanitizes + const sanitized = sanitizeControlSequences('[200~/model grok-4-1-fast[201~'); + const result = parseCommand(sanitized); + + expect(result.name).toBe('model'); + expect(result.args[0]).toBe('grok-4-1-fast'); + }); + + it('should handle quote-wrapped paths with paste markers', () => { + const input = '/prompt "docs/file[200~.md"'; + const result = parseInput(input); + + expect(result.type).toBe('command'); + if (result.type === 'command') { + // Markers inside quotes still get sanitized + expect(result.command.args[0]).not.toContain('[200~'); + } + }); + }); +}); diff --git a/tests/unit/command-suggestions.test.ts b/tests/unit/command-suggestions.test.ts new file mode 100644 index 0000000..9902b5a --- /dev/null +++ b/tests/unit/command-suggestions.test.ts @@ -0,0 +1,272 @@ +import { describe, it, expect } from 'vitest'; +import { + levenshteinDistance, + getCommandSuggestions, + getDidYouMeanSuggestions, + getDidYouMeanSuggestion, + matchCommand, + MatchType, +} from '../../src/ui/utils/command-suggestions.js'; +import type { Command } from '../../src/commands/types.js'; + +/** + * Tests for command suggestion algorithm. + * + * Covers: + * - Levenshtein distance calculation + * - Exact, prefix, alias, substring, fuzzy matching + * - Suggestion ranking and sorting + * - "Did you mean" suggestions for unknown commands + */ + +describe('command suggestions', () => { + // Mock commands for testing + const mockCommands: Command[] = [ + { + name: 'help', + description: 'Show help', + usage: '/help', + arguments: [], + examples: [], + aliases: ['h', '?'], + execute: async () => ({ success: true }), + }, + { + name: 'prompt', + description: 'Load prompt from file', + usage: '/prompt ', + arguments: [], + examples: [], + aliases: ['promptfile', 'pf', 'loadprompt'], + execute: async () => ({ success: true }), + }, + { + name: 'model', + description: 'Set model', + usage: '/model ', + arguments: [], + examples: [], + aliases: ['m'], + execute: async () => ({ success: true }), + }, + { + name: 'exit', + description: 'Exit application', + usage: '/exit', + arguments: [], + examples: [], + aliases: ['quit', 'q'], + execute: async () => ({ success: true }), + }, + ]; + + describe('levenshteinDistance', () => { + it('should return 0 for identical strings', () => { + expect(levenshteinDistance('hello', 'hello')).toBe(0); + }); + + it('should return length for empty vs non-empty', () => { + expect(levenshteinDistance('', 'test')).toBe(4); + expect(levenshteinDistance('test', '')).toBe(4); + }); + + it('should calculate single character insertion', () => { + expect(levenshteinDistance('test', 'tests')).toBe(1); + }); + + it('should calculate single character deletion', () => { + expect(levenshteinDistance('tests', 'test')).toBe(1); + }); + + it('should calculate single character substitution', () => { + expect(levenshteinDistance('test', 'text')).toBe(1); + }); + + it('should calculate distance for typos', () => { + // Note: Levenshtein doesn't count transposition as 1, it's 2 ops (delete+insert) + expect(levenshteinDistance('prompt', 'promt')).toBe(1); // delete 'p' at pos 4 + expect(levenshteinDistance('model', 'modle')).toBe(2); // transpose 'd'/'l' = del+ins + }); + + it('should be symmetric', () => { + expect(levenshteinDistance('abc', 'xyz')).toBe(levenshteinDistance('xyz', 'abc')); + }); + }); + + describe('getCommandSuggestions', () => { + it('should return empty array for non-slash input', () => { + const suggestions = getCommandSuggestions('hello', mockCommands); + expect(suggestions).toEqual([]); + }); + + it('should return all commands for bare "/" (sorted by name)', () => { + const suggestions = getCommandSuggestions('/', mockCommands); + + expect(suggestions.length).toBe(mockCommands.length); + expect(suggestions[0].command.name).toBe('exit'); + expect(suggestions[1].command.name).toBe('help'); + expect(suggestions[2].command.name).toBe('model'); + expect(suggestions[3].command.name).toBe('prompt'); + expect(suggestions.every(s => s.matchType === 'prefix')).toBe(true); + }); + + it('should match exact command name', () => { + const suggestions = getCommandSuggestions('/prompt', mockCommands); + + expect(suggestions.length).toBeGreaterThan(0); + expect(suggestions[0].command.name).toBe('prompt'); + expect(suggestions[0].matchType).toBe('exact'); + expect(suggestions[0].score).toBe(1000); + }); + + it('should match prefix', () => { + const suggestions = getCommandSuggestions('/pr', mockCommands); + + expect(suggestions.length).toBeGreaterThan(0); + expect(suggestions[0].command.name).toBe('prompt'); + expect(suggestions[0].matchType).toBe('prefix'); + expect(suggestions[0].score).toBe(900); + }); + + it('should match exact alias', () => { + const suggestions = getCommandSuggestions('/pf', mockCommands); + + expect(suggestions.length).toBeGreaterThan(0); + expect(suggestions[0].command.name).toBe('prompt'); + expect(suggestions[0].matchType).toBe('alias-exact'); + expect(suggestions[0].matchedAlias).toBe('pf'); + expect(suggestions[0].score).toBe(800); + }); + + it('should match prefix alias', () => { + const suggestions = getCommandSuggestions('/promptf', mockCommands); + + expect(suggestions.length).toBeGreaterThan(0); + const promptSuggestion = suggestions.find(s => s.command.name === 'prompt'); + expect(promptSuggestion).toBeDefined(); + expect(promptSuggestion?.matchType).toBe('alias-prefix'); + expect(promptSuggestion?.matchedAlias).toBe('promptfile'); + }); + + it('should match substring', () => { + const suggestions = getCommandSuggestions('/odel', mockCommands); + + const modelMatch = suggestions.find(s => s.command.name === 'model'); + expect(modelMatch).toBeDefined(); + expect(modelMatch?.matchType).toBe('substring'); + expect(modelMatch?.score).toBe(600); + }); + + it('should match fuzzy with typo (distance 1)', () => { + const suggestions = getCommandSuggestions('/promt', mockCommands); + + expect(suggestions.length).toBeGreaterThan(0); + const promptMatch = suggestions.find(s => s.command.name === 'prompt'); + expect(promptMatch).toBeDefined(); + expect(promptMatch?.matchType).toBe('fuzzy'); + expect(promptMatch?.score).toBe(400); // 500 - 1*100 + }); + + it('should match prefix for partial command name', () => { + const suggestions = getCommandSuggestions('/promp', mockCommands); + + // "promp" is a prefix of "prompt", not a fuzzy match + const promptMatch = suggestions.find(s => s.command.name === 'prompt'); + expect(promptMatch).toBeDefined(); + expect(promptMatch?.matchType).toBe('prefix'); + }); + + it('should not match fuzzy beyond distance 2', () => { + const suggestions = getCommandSuggestions('/xyz', mockCommands); + + // "xyz" has distance > 2 from all commands + expect(suggestions.length).toBe(0); + }); + + it('should rank exact > prefix > alias-exact > alias-prefix > substring > fuzzy', () => { + const suggestions = getCommandSuggestions('/prompt', mockCommands); + const exactMatch = suggestions.find(s => s.matchType === 'exact'); + expect(exactMatch?.score).toBe(1000); + + const prefixSuggestions = getCommandSuggestions('/prom', mockCommands); + const prefixMatch = prefixSuggestions.find(s => s.matchType === 'prefix'); + expect(prefixMatch?.score).toBe(900); + + // Verify ranking order by comparing scores + expect(1000).toBeGreaterThan(900); // exact > prefix + expect(900).toBeGreaterThan(800); // prefix > alias-exact + expect(800).toBeGreaterThan(700); // alias-exact > alias-prefix + expect(700).toBeGreaterThan(600); // alias-prefix > substring + expect(600).toBeGreaterThan(500); // substring > fuzzy (max fuzzy score) + }); + + it('should handle command with args (only match command portion)', () => { + const suggestions = getCommandSuggestions('/prompt myfile.txt', mockCommands); + + // Should still match "prompt" even though args are present + expect(suggestions[0].command.name).toBe('prompt'); + expect(suggestions[0].matchType).toBe('exact'); + }); + + it('should be deterministic (stable sort)', () => { + const suggestions1 = getCommandSuggestions('/p', mockCommands); + const suggestions2 = getCommandSuggestions('/p', mockCommands); + + expect(suggestions1.map(s => s.command.name)).toEqual( + suggestions2.map(s => s.command.name) + ); + }); + }); + + describe('getDidYouMeanSuggestions', () => { + it('should return top 3 fuzzy/substring matches', () => { + const commands: Command[] = [ + { name: 'test', description: '', usage: '', arguments: [], examples: [], aliases: [], execute: async () => ({ success: true }) }, + { name: 'text', description: '', usage: '', arguments: [], examples: [], aliases: [], execute: async () => ({ success: true }) }, + { name: 'next', description: '', usage: '', arguments: [], examples: [], aliases: [], execute: async () => ({ success: true }) }, + ]; + + const suggestions = getDidYouMeanSuggestions('txt', commands, 3); + + // All three have substring or fuzzy match to "txt" + expect(suggestions.length).toBeGreaterThan(0); + expect(suggestions.length).toBeLessThanOrEqual(3); + }); + + it('should only return fuzzy/substring matches (not exact/prefix)', () => { + const suggestions = getDidYouMeanSuggestions('promt', mockCommands, 3); + + expect(suggestions.length).toBeGreaterThan(0); + expect(suggestions.every(s => s.matchType === 'fuzzy' || s.matchType === 'substring')).toBe(true); + }); + + it('should respect maxSuggestions limit', () => { + const suggestions = getDidYouMeanSuggestions('e', mockCommands, 2); + + expect(suggestions.length).toBeLessThanOrEqual(2); + }); + + it('should return empty for no fuzzy/substring matches', () => { + const suggestions = getDidYouMeanSuggestions('prom', mockCommands, 3); + + // "prom" is a prefix match, not fuzzy/substring + expect(suggestions.length).toBe(0); + }); + }); + + describe('getDidYouMeanSuggestion (single - deprecated)', () => { + it('should suggest fuzzy match for typo', () => { + const suggestion = getDidYouMeanSuggestion('promt', mockCommands); + + expect(suggestion).toBeDefined(); + expect(suggestion?.command.name).toBe('prompt'); + expect(suggestion?.matchType).toBe('fuzzy'); + }); + + it('should return null for no fuzzy/substring match', () => { + const suggestion = getDidYouMeanSuggestion('xyz123', mockCommands); + + expect(suggestion).toBeNull(); + }); + }); +}); diff --git a/tests/unit/confirm-decision.test.ts b/tests/unit/confirm-decision.test.ts new file mode 100644 index 0000000..b86bffc --- /dev/null +++ b/tests/unit/confirm-decision.test.ts @@ -0,0 +1,206 @@ +import { describe, it, expect } from 'vitest'; +import { + shouldAutoApprove, + getConfirmOptions, + isAutoAcceptEligible, + ConfirmDecision, +} from '../../src/ui/utils/confirm-decision.js'; + +/** + * Tests for confirmation decision logic. + * + * Covers: + * - Auto-approval rules (Edit/Write auto-approved, Bash never) + * - Confirmation options (Edit/Write show auto-accept, Bash doesn't) + * - Tool eligibility checking + */ + +describe('confirm decision', () => { + describe('shouldAutoApprove', () => { + it('should not auto-approve when feature is disabled', () => { + expect(shouldAutoApprove('Edit', false)).toBe(false); + expect(shouldAutoApprove('Write', false)).toBe(false); + expect(shouldAutoApprove('Bash', false)).toBe(false); + }); + + it('should auto-approve Edit when enabled', () => { + expect(shouldAutoApprove('Edit', true)).toBe(true); + }); + + it('should auto-approve Write when enabled', () => { + expect(shouldAutoApprove('Write', true)).toBe(true); + }); + + it('should NEVER auto-approve Bash even when enabled', () => { + expect(shouldAutoApprove('Bash', true)).toBe(false); + }); + + it('should not auto-approve other tools', () => { + expect(shouldAutoApprove('Read', true)).toBe(false); + expect(shouldAutoApprove('Grep', true)).toBe(false); + expect(shouldAutoApprove('TodoWrite', true)).toBe(false); + }); + }); + + describe('getConfirmOptions', () => { + it('should return 3 options for Edit (includes auto-accept)', () => { + const options = getConfirmOptions('Edit'); + + expect(options.length).toBe(3); + expect(options[0].key).toBe('y'); + expect(options[0].label).toBe('Allow once'); + expect(options[0].decision).toBe('allow_once'); + + expect(options[1].key).toBe('a'); + expect(options[1].label).toBe('Auto-accept edits (session)'); + expect(options[1].decision).toBe('auto_accept_edits'); + + expect(options[2].key).toBe('n'); + expect(options[2].label).toBe('Deny'); + expect(options[2].decision).toBe('deny'); + }); + + it('should return 3 options for Write (includes auto-accept)', () => { + const options = getConfirmOptions('Write'); + + expect(options.length).toBe(3); + const autoAcceptOption = options.find(o => o.decision === 'auto_accept_edits'); + expect(autoAcceptOption).toBeDefined(); + expect(autoAcceptOption?.key).toBe('a'); + }); + + it('should return 2 options for Bash (NO auto-accept)', () => { + const options = getConfirmOptions('Bash'); + + expect(options.length).toBe(2); + expect(options[0].key).toBe('y'); + expect(options[0].decision).toBe('allow_once'); + expect(options[1].key).toBe('n'); + expect(options[1].decision).toBe('deny'); + + // Verify NO auto-accept option + const hasAutoAccept = options.some(o => o.decision === 'auto_accept_edits'); + expect(hasAutoAccept).toBe(false); + }); + + it('should not include auto-accept for other tools', () => { + const tools = ['Read', 'Grep', 'Glob', 'TodoWrite']; + + for (const tool of tools) { + const options = getConfirmOptions(tool); + const hasAutoAccept = options.some(o => o.decision === 'auto_accept_edits'); + expect(hasAutoAccept, `${tool} should NOT have auto-accept option`).toBe(false); + } + }); + + it('should always include Allow once and Deny options', () => { + const allTools = ['Edit', 'Write', 'Bash', 'Read']; + + for (const tool of allTools) { + const options = getConfirmOptions(tool); + + const hasAllowOnce = options.some(o => o.decision === 'allow_once'); + const hasDeny = options.some(o => o.decision === 'deny'); + + expect(hasAllowOnce, `${tool} must have Allow once`).toBe(true); + expect(hasDeny, `${tool} must have Deny`).toBe(true); + } + }); + }); + + describe('isAutoAcceptEligible', () => { + it('should return true for Edit', () => { + expect(isAutoAcceptEligible('Edit')).toBe(true); + }); + + it('should return true for Write', () => { + expect(isAutoAcceptEligible('Write')).toBe(true); + }); + + it('should return false for Bash', () => { + expect(isAutoAcceptEligible('Bash')).toBe(false); + }); + + it('should return false for other tools', () => { + expect(isAutoAcceptEligible('Read')).toBe(false); + expect(isAutoAcceptEligible('Grep')).toBe(false); + expect(isAutoAcceptEligible('TodoWrite')).toBe(false); + }); + }); + + describe('decision flow scenarios', () => { + it('should enable auto-accept after selecting auto_accept_edits', () => { + // Simulate: user sees Edit confirm, selects auto-accept + let autoAcceptEdits = false; + const options = getConfirmOptions('Edit'); + const autoAcceptOption = options.find(o => o.decision === 'auto_accept_edits'); + + // User presses 'a' or Enters on that option + if (autoAcceptOption?.decision === 'auto_accept_edits') { + autoAcceptEdits = true; // App sets this + } + + expect(autoAcceptEdits).toBe(true); + + // Next Edit call should auto-approve + expect(shouldAutoApprove('Edit', autoAcceptEdits)).toBe(true); + // But Bash still prompts + expect(shouldAutoApprove('Bash', autoAcceptEdits)).toBe(false); + }); + }); + + describe('Tab/Shift+Tab navigation helpers', () => { + // These tests document the various terminal sequences for Tab/Shift+Tab + // that the ConfirmDialog must handle + + it('should recognize standard Shift+Tab sequence (ESC [ Z)', () => { + const input = '\x1b[Z'; + const isShiftTab = input === '\x1b[Z'; + expect(isShiftTab).toBe(true); + }); + + it('should recognize extended Shift+Tab (ESC [ 1 ; 2 Z)', () => { + const input = '\x1b[1;2Z'; + const isShiftTab = input === '\x1b[1;2Z'; + expect(isShiftTab).toBe(true); + }); + + it('should recognize ESC-stripped Shift+Tab ([Z)', () => { + const input = '[Z'; + const isShiftTab = input === '[Z'; + expect(isShiftTab).toBe(true); + }); + + it('should recognize ESC-stripped extended Shift+Tab ([1;2Z)', () => { + const input = '[1;2Z'; + const isShiftTab = input === '[1;2Z'; + expect(isShiftTab).toBe(true); + }); + + it('should handle selection cycling forward (Tab)', () => { + // Simulate 3 options, start at index 0 + let index = 0; + const optionsLength = 3; + + // Tab 3 times + index = (index + 1) % optionsLength; // 1 + index = (index + 1) % optionsLength; // 2 + index = (index + 1) % optionsLength; // 0 (wraps) + + expect(index).toBe(0); + }); + + it('should handle selection cycling backward (Shift+Tab)', () => { + // Simulate 3 options, start at index 0 + let index = 0; + const optionsLength = 3; + + // Shift+Tab 3 times + index = (index - 1 + optionsLength) % optionsLength; // 2 + index = (index - 1 + optionsLength) % optionsLength; // 1 + index = (index - 1 + optionsLength) % optionsLength; // 0 (wraps) + + expect(index).toBe(0); + }); + }); +}); diff --git a/tests/unit/input-editor.test.ts b/tests/unit/input-editor.test.ts new file mode 100644 index 0000000..9951b9b --- /dev/null +++ b/tests/unit/input-editor.test.ts @@ -0,0 +1,372 @@ +import { describe, it, expect } from 'vitest'; +import { + createEditorState, + insertAtCursor, + deleteBackward, + deleteForward, + moveCursor, + moveCursorToBoundary, + setCursor, + setValue, + renderWithCursor, + getDisplayWithCursor, + EditorState, +} from '../../src/ui/utils/input-editor.js'; + +/** + * Tests for input editor utility. + * + * Covers: + * - Cursor-based insertion + * - Backspace/delete at cursor + * - Left/right/home/end cursor movement + * - Paste insertion at cursor + * - Cursor rendering in single/multi-line input + */ + +describe('input editor', () => { + describe('createEditorState', () => { + it('should create empty state', () => { + const state = createEditorState(); + expect(state.value).toBe(''); + expect(state.cursorIndex).toBe(0); + }); + + it('should create state with initial value', () => { + const state = createEditorState('hello'); + expect(state.value).toBe('hello'); + expect(state.cursorIndex).toBe(5); // Cursor at end + }); + }); + + describe('insertAtCursor', () => { + it('should insert at end of empty string', () => { + const state = createEditorState(); + const result = insertAtCursor(state, 'a'); + + expect(result.value).toBe('a'); + expect(result.cursorIndex).toBe(1); + }); + + it('should insert at cursor in middle', () => { + const state: EditorState = { value: 'helo', cursorIndex: 2 }; + const result = insertAtCursor(state, 'l'); + + expect(result.value).toBe('hello'); + expect(result.cursorIndex).toBe(3); + }); + + it('should insert at beginning', () => { + const state: EditorState = { value: 'world', cursorIndex: 0 }; + const result = insertAtCursor(state, 'hello '); + + expect(result.value).toBe('hello world'); + expect(result.cursorIndex).toBe(6); + }); + + it('should insert multi-character string', () => { + const state: EditorState = { value: 'ac', cursorIndex: 1 }; + const result = insertAtCursor(state, 'b'); + + expect(result.value).toBe('abc'); + expect(result.cursorIndex).toBe(2); + }); + }); + + describe('deleteBackward', () => { + it('should do nothing at start', () => { + const state: EditorState = { value: 'hello', cursorIndex: 0 }; + const result = deleteBackward(state); + + expect(result.value).toBe('hello'); + expect(result.cursorIndex).toBe(0); + }); + + it('should delete character before cursor', () => { + const state: EditorState = { value: 'hello', cursorIndex: 5 }; + const result = deleteBackward(state); + + expect(result.value).toBe('hell'); + expect(result.cursorIndex).toBe(4); + }); + + it('should delete from middle', () => { + const state: EditorState = { value: 'hello', cursorIndex: 2 }; + const result = deleteBackward(state); + + expect(result.value).toBe('hllo'); + expect(result.cursorIndex).toBe(1); + }); + }); + + describe('deleteForward', () => { + it('should do nothing at end', () => { + const state: EditorState = { value: 'hello', cursorIndex: 5 }; + const result = deleteForward(state); + + expect(result.value).toBe('hello'); + expect(result.cursorIndex).toBe(5); + }); + + it('should delete character at cursor', () => { + const state: EditorState = { value: 'hello', cursorIndex: 0 }; + const result = deleteForward(state); + + expect(result.value).toBe('ello'); + expect(result.cursorIndex).toBe(0); // Cursor stays + }); + + it('should delete from middle', () => { + const state: EditorState = { value: 'hello', cursorIndex: 2 }; + const result = deleteForward(state); + + expect(result.value).toBe('helo'); + expect(result.cursorIndex).toBe(2); + }); + }); + + describe('moveCursor', () => { + it('should move left', () => { + const state: EditorState = { value: 'hello', cursorIndex: 3 }; + const result = moveCursor(state, 'left'); + + expect(result.value).toBe('hello'); + expect(result.cursorIndex).toBe(2); + }); + + it('should not move left past start', () => { + const state: EditorState = { value: 'hello', cursorIndex: 0 }; + const result = moveCursor(state, 'left'); + + expect(result.cursorIndex).toBe(0); + }); + + it('should move right', () => { + const state: EditorState = { value: 'hello', cursorIndex: 2 }; + const result = moveCursor(state, 'right'); + + expect(result.cursorIndex).toBe(3); + }); + + it('should not move right past end', () => { + const state: EditorState = { value: 'hello', cursorIndex: 5 }; + const result = moveCursor(state, 'right'); + + expect(result.cursorIndex).toBe(5); + }); + }); + + describe('moveCursorToBoundary', () => { + it('should move to start', () => { + const state: EditorState = { value: 'hello world', cursorIndex: 6 }; + const result = moveCursorToBoundary(state, 'start'); + + expect(result.cursorIndex).toBe(0); + }); + + it('should move to end', () => { + const state: EditorState = { value: 'hello world', cursorIndex: 3 }; + const result = moveCursorToBoundary(state, 'end'); + + expect(result.cursorIndex).toBe(11); + }); + }); + + describe('setCursor', () => { + it('should set cursor to specific index', () => { + const state: EditorState = { value: 'hello', cursorIndex: 0 }; + const result = setCursor(state, 3); + + expect(result.cursorIndex).toBe(3); + }); + + it('should clamp negative index to 0', () => { + const state: EditorState = { value: 'hello', cursorIndex: 2 }; + const result = setCursor(state, -5); + + expect(result.cursorIndex).toBe(0); + }); + + it('should clamp index beyond end to value.length', () => { + const state: EditorState = { value: 'hello', cursorIndex: 2 }; + const result = setCursor(state, 999); + + expect(result.cursorIndex).toBe(5); + }); + }); + + describe('setValue', () => { + it('should replace value and move cursor to end', () => { + const state: EditorState = { value: 'old', cursorIndex: 1 }; + const result = setValue(state, 'new value'); + + expect(result.value).toBe('new value'); + expect(result.cursorIndex).toBe(9); + }); + }); + + describe('renderWithCursor', () => { + it('should render cursor at end', () => { + const result = renderWithCursor('hello', 5); + expect(result).toBe('hello|'); + }); + + it('should render cursor at start', () => { + const result = renderWithCursor('hello', 0); + expect(result).toBe('|hello'); + }); + + it('should render cursor in middle', () => { + const result = renderWithCursor('hello', 2); + expect(result).toBe('he|llo'); + }); + + it('should use custom cursor character', () => { + const result = renderWithCursor('hello', 2, '_'); + expect(result).toBe('he_llo'); + }); + + it('should handle empty string', () => { + const result = renderWithCursor('', 0); + expect(result).toBe('|'); + }); + + it('should clamp cursor beyond bounds', () => { + const result = renderWithCursor('hello', 999); + expect(result).toBe('hello|'); + }); + }); + + describe('getDisplayWithCursor', () => { + it('should show full single-line value with cursor', () => { + const state: EditorState = { value: 'hello', cursorIndex: 2 }; + const display = getDisplayWithCursor(state); + + expect(display.display).toBe('he|llo'); + expect(display.isMultiLine).toBe(false); + expect(display.lineCount).toBe(1); + }); + + it('should show first line + count for multi-line', () => { + const state: EditorState = { value: 'line1\nline2\nline3', cursorIndex: 3 }; + const display = getDisplayWithCursor(state); + + expect(display.display).toContain('lin|e1'); + expect(display.display).toContain('[3 lines]'); + expect(display.isMultiLine).toBe(true); + expect(display.lineCount).toBe(3); + }); + + it('should indicate cursor line for multi-line when not on first line', () => { + const state: EditorState = { value: 'line1\nline2\nline3', cursorIndex: 8 }; + const display = getDisplayWithCursor(state); + + expect(display.display).toContain('(line 2)'); + expect(display.display).toContain('li|ne2'); + expect(display.isMultiLine).toBe(true); + }); + + it('should handle cursor at very end of multi-line', () => { + const state: EditorState = { value: 'a\nb\nc', cursorIndex: 5 }; + const display = getDisplayWithCursor(state); + + expect(display.display).toContain('(line 3)'); + expect(display.display).toContain('c|'); + }); + }); + + describe('complex editing scenarios', () => { + it('should handle: type "hello", move left 2, insert "X"', () => { + let state = createEditorState(); + state = insertAtCursor(state, 'hello'); + expect(state.value).toBe('hello'); + expect(state.cursorIndex).toBe(5); + + state = moveCursor(state, 'left'); + state = moveCursor(state, 'left'); + expect(state.cursorIndex).toBe(3); + + state = insertAtCursor(state, 'X'); + expect(state.value).toBe('helXlo'); + expect(state.cursorIndex).toBe(4); + }); + + it('should handle: type "abc", home, delete forward', () => { + let state = createEditorState(); + state = insertAtCursor(state, 'abc'); + state = moveCursorToBoundary(state, 'start'); + expect(state.cursorIndex).toBe(0); + + state = deleteForward(state); + expect(state.value).toBe('bc'); + expect(state.cursorIndex).toBe(0); + }); + + it('should handle: paste multi-line at cursor', () => { + let state: EditorState = { value: 'before after', cursorIndex: 7 }; + const pastedContent = 'PASTE\nLINE2'; + + state = insertAtCursor(state, pastedContent); + + expect(state.value).toBe('before PASTE\nLINE2after'); + expect(state.cursorIndex).toBe(7 + pastedContent.length); + }); + + it('should handle: multiple backspaces from different positions', () => { + let state = createEditorState(); + state = insertAtCursor(state, 'abcdef'); + // "abcdef|" cursor at 6 + + state = deleteBackward(state); + expect(state.value).toBe('abcde'); + expect(state.cursorIndex).toBe(5); + + state = moveCursor(state, 'left'); + state = moveCursor(state, 'left'); + // "abc|de" cursor at 3 + + state = deleteBackward(state); + expect(state.value).toBe('abde'); + expect(state.cursorIndex).toBe(2); + }); + + it('should not corrupt value when control chars are filtered', () => { + // Simulate what happens if DEL (\x7f) was inserted as text (bug scenario) + let state = createEditorState(); + state = insertAtCursor(state, 'hello'); + + // This should NOT happen with the fix (but tests the editor is resilient) + // If \x7f gets inserted, it's invisible but corrupts the string + state = insertAtCursor(state, '\x7f'); + + // The value would be 'hello\x7f' which is 6 chars + expect(state.value.length).toBe(6); + expect(state.cursorIndex).toBe(6); + + // Backspace should remove the \x7f control char + state = deleteBackward(state); + expect(state.value).toBe('hello'); + }); + }); + + describe('terminal byte code compatibility', () => { + it('should detect DEL byte (\\x7f) as backspace intent', () => { + // Verify that \x7f would trigger backspace logic + const delByte = '\x7f'; + expect(delByte).toBe('\x7f'); + expect(delByte.charCodeAt(0)).toBe(127); + }); + + it('should detect BS byte (\\x08 or \\b) as backspace intent', () => { + const bsByte = '\x08'; + const bsLiteral = '\b'; + expect(bsByte).toBe('\b'); + expect(bsByte.charCodeAt(0)).toBe(8); + }); + + it('should detect delete sequence (\\x1b[3~)', () => { + const deleteSeq = '\x1b[3~'; + expect(deleteSeq).toContain('[3~'); + }); + }); +}); diff --git a/tests/unit/prompt-command.test.ts b/tests/unit/prompt-command.test.ts index c18d21a..cf2efd2 100644 --- a/tests/unit/prompt-command.test.ts +++ b/tests/unit/prompt-command.test.ts @@ -293,6 +293,29 @@ describe('/prompt command', () => { await fs.rm(tempDir, { recursive: true, force: true }); } }); + + it('should handle file path with bracketed paste markers (regression)', async () => { + const testCwd = process.cwd(); + const tempDir = path.join(testCwd, '.tmp-prompt-test-' + Date.now()); + await fs.mkdir(tempDir, { recursive: true }); + + try { + const promptFile = path.join(tempDir, 'test-file.txt'); + await fs.writeFile(promptFile, 'test content', 'utf8'); + + // Simulate what happens if paste markers leak into args + // Parser should sanitize, but handler also has belt-and-suspenders + const corruptedPath = promptFile + '[200~'; + const parsed = createParsedCommand([corruptedPath]); + const result = await promptCommand.execute(parsed, createContext(testCwd)); + + // Should succeed because handler sanitizes the path + expect(result.success).toBe(true); + expect(result.action?.type).toBe('submit_prompt'); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); }); describe('command metadata', () => {