Add ability to rename terminals

[dgershman]

Summary

• Add inline rename for terminal tabs via double-click or right-click context menu
• Add crow rename-terminal CLI command and rename-terminal RPC handler
• Works for both session terminals and global terminals

Closes #203

Test plan

• Double-click a terminal tab name — enters inline edit mode, Enter commits, Escape cancels
• Right-click a terminal tab — context menu shows "Rename" (and "Close" for non-managed)
• crow rename-terminal --session <uuid> --terminal <uuid> "New Name" returns updated name
• Renamed terminal persists after app restart
• Global terminals (Terminals tab) support rename as well

🤖 Generated with Claude Code

[dhilgaertner]

Code & Security Review

Summary

Well-scoped feature: inline rename UI, context menu, CLI command, and RPC handler all wired consistently with the existing rename-session pattern. No critical security issues. A few notes on UI/CLI validation parity and edit-mode lifecycle.

Security Review

Strengths

• RPC handler validates via shared Validation.isValidSessionName (length ≤ 256, no control characters) — reuses proven logic.
• CLI command validates UUIDs with the shared validateUUID helper.
• No injection surface — name is stored and displayed only; never passed to a shell.
• State mutations correctly run on MainActor and the store's lock serializes writes.

Concerns

• Validation is bypassed on the UI path. CLI → RPC validates, but UI → SessionService.renameTerminal does not. A paste with control characters or a name >256 chars via the TextField would persist without validation, producing inconsistent behavior between the two entry points.
  • Sources/Crow/App/SessionService.swift:721-729 — no validation
  • Packages/CrowUI/Sources/CrowUI/SessionDetailView.swift:394-400 — only trims + non-empty check
  • Fix: either apply Validation.isValidSessionName in SessionService.renameTerminal, or have the RPC handler delegate to the service (see next item) and validate in the service.

Code Quality

Duplicated rename logic — Sources/Crow/App/AppDelegate.swift:720-743 inlines the find/update/persist logic that already exists in SessionService.renameTerminal (SessionService.swift:721-729). Other terminal lifecycle events (onCloseTerminal, onAddTerminal) delegate to the service; the RPC handler should too. This also fixes the validation divergence above for free.

Edit mode can get stuck on focus loss — TerminalTabBar commits only on Return (onSubmit) and cancels only on Escape (onExitCommand). If the user clicks elsewhere, isEditing becomes false but editingTerminalID remains set, leaving the TextField visible with no focus. Consider adding:

.onChange(of: isEditing) { _, nowEditing in
    if !nowEditing, editingTerminalID == terminal.id {
        commitRename(terminal.id)   // or cancel — pick the UX you prefer
    }
}

TextField nested inside Button — SessionDetailView.swift:319 places the TextField inside the outer Button { onSelect(...) }. SwiftUI nested-interactive semantics are fragile; worth a manual test that typing/clicking inside the field does not fire onSelect or re-trigger beginEditing.

Managed-terminal rename is newly allowed — the context menu now offers Rename on managed terminals (Manager tab). Intentional per the PR description, but worth confirming the Manager tab was meant to be user-renamable, since its name is referenced in skills/docs (CLAUDE.md mentions "Manager tab").

No tests added — the PR adds an RPC handler, service method, and CLI command with zero coverage. At minimum:

• RPC: rejects invalid name, rejects unknown terminalID, happy path persists.
• SessionService.renameTerminal: happy path + not-found no-op.
• Existing Validation tests already cover the name rules, so a handler test would be small.

Summary Table

Priority	Issue
🟡 Yellow	UI path bypasses validation that CLI enforces — SessionService.renameTerminal does not validate
🟡 Yellow	Duplicated rename logic between RPC handler and SessionService — delegate instead
🟡 Yellow	Edit mode not committed/cancelled on focus loss (click-outside leaves UI stuck)
🟢 Green	TextField nested in Button — verify gesture routing
🟢 Green	Managed terminals are now user-renamable via context menu — confirm intent for Manager tab
🟢 Green	No tests for the new handler/command/service method

Test Status

• swift test in Packages/CrowCore — 112/112 passing, no regressions.
• swift build in Packages/CrowCLI — clean build.
• Full top-level swift build not runnable here (missing GhosttyKit.xcframework binary artifact in this checkout).

Recommendation: Comment. No blocking issues — the feature works and security posture is sound — but the UI/CLI validation divergence and duplicated logic are worth addressing before merge. Folding the RPC handler into SessionService.renameTerminal (with validation moved there) resolves two items at once.

[dhilgaertner]

Code & Security Review

Security Review

Strengths:

• Validation is shared via Validation.isValidSessionName and now applied centrally in SessionService.renameTerminal (good follow-up on the prior review). Rejects empty / >256-char / control-character payloads, which protects the persisted JSON store and rules out terminal-control-sequence injection through the tab name.
• RPC handler delegates to SessionService instead of inlining find/update/persist logic, so the UI and CLI paths have a single enforcement point.
• All mutations go through MainActor.run, consistent with the existing concurrency model — no new race surface.
• UUIDs are validated CLI-side before the RPC call and re-parsed server-side; bad input throws RPCError.invalidParams before touching state.
• No PII / secrets in logs; rename does not touch the underlying PTY / process, just metadata.

Concerns:

• None blocking.

Code Quality

Yellow (worth a follow-up):

• No tests added. New surface includes the RenameTerminal CLI command, the rename-terminal RPC handler, and SessionService.renameTerminal. Worth at least a SessionServiceTests case that covers (a) happy path, (b) terminal-not-found returns false, (c) invalid name (empty / control char / >256) returns false and leaves state unchanged, and (d) persistence to store matches in-memory state. The existing Validation.isValidSessionName tests cover the validator but not its use here.
• Silent failure for invalid names from the UI. commitRename (SessionDetailView.swift:399) calls onRename whenever trimmed is non-empty, but SessionService.renameTerminal may still reject it (e.g., 257+ chars, embedded control chars from a paste). The user sees the editor close and the old name reappear with no feedback. Consider either capping TextField input length / filtering control chars on input, or surfacing a brief error indicator on rejection.
• CLI does not trim, UI does. commitRename trims whitespace before sending; the CLI path forwards the raw argument. As a result crow rename-terminal ... " " passes validation (non-empty, no control chars) and persists a whitespace-only name that renders as a blank tab. Either trim in SessionService.renameTerminal (recommended — single source of truth) or extend Validation.isValidSessionName to reject whitespace-only.
• Rename of managed terminals is permitted via both UI and CLI. Probably intentional (lets users disambiguate multiple Claude Code tabs), but the context menu hides Close for managed terminals while still showing Rename — slight asymmetry worth a quick sanity check that this is the desired UX.

Green (nits):

• editingTerminalID = nil in the onExitCommand (Escape) path relies on the if editingTerminalID == terminal.id short-circuit inside the focus-loss onChange to skip the commit. It works, but a one-line comment on the interaction would help future readers (SessionDetailView.swift:328-335).
• onRenameTerminal callback signature (UUID, UUID, String) is positional; if any new callers appear, a small struct or named tuple would prevent the kind of arg-order mistake that's easy to miss in review (AppState.swift:179).
• RenameTerminal is missing from CLAUDE.md's Terminal Commands reference (both the dev-root copy and the one in this repo). Worth adding so the Manager Claude can discover it.

Summary Table

Priority	Issue
Red	none
Yellow	No tests for rename path; UI silently swallows server-side rejections; whitespace-only name accepted via CLI; rename allowed on managed terminals (verify intent)
Green	Missing comment on Escape/focus interplay; positional rename callback; CLAUDE.md not updated

Recommendation: Approve. The change is small, follows the existing patterns cleanly, security posture is solid, and the prior-review feedback (unified validation, focus-loss commit) was addressed correctly. The yellow items are real but non-blocking — best handled in a follow-up.