diff --git a/AGENTS.md b/AGENTS.md index 29c50458e76..dad66f4bba8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -72,7 +72,7 @@ Orca targets macOS, Linux, and Windows. Keep all platform-dependent behavior beh - **Keyboard shortcuts**: Never hardcode `e.metaKey`. Use a platform check (`navigator.userAgent.includes('Mac')`) to pick `metaKey` on Mac and `ctrlKey` on Linux/Windows. Electron menu accelerators should use `CmdOrCtrl`. - **Shortcut labels in UI**: Display `⌘` / `⇧` on Mac and `Ctrl+` / `Shift+` on other platforms. - **File paths**: Use `path.join` or Electron/Node path utilities — never assume `/` or `\`. -- **Windows terminal shells**: `--shell` picks the shell a terminal *is*; `--command` is typed into whatever shell the host spawned, so a shell choice routed through `command` silently becomes a child process. See [`docs/reference/windows-terminal-shell-selection.md`](./docs/reference/windows-terminal-shell-selection.md). +- **Windows terminal shells**: `--shell` picks the shell a terminal _is_; `--command` is typed into whatever shell the host spawned, so a shell choice routed through `command` silently becomes a child process. See [`docs/reference/windows-terminal-shell-selection.md`](./docs/reference/windows-terminal-shell-selection.md). - **Windows setup scripts**: the setup/issue-command runner is a `.cmd` batch file unless the script starts with a `#!` line — never derive that from the user's terminal-shell preference, and never launch a `.cmd` runner with a bare `cmd.exe /c` from a Git Bash pane (MSYS rewrites the `/c`). See [`docs/reference/windows-setup-shell.md`](./docs/reference/windows-setup-shell.md). - **Windows child processes**: start them through `runProcess`/`spawnProcess` in `src/shared/child-process/` — never `child_process` directly. It pins `windowsHide`, refuses `shell: true`, and encodes `.cmd`/`.bat` arguments so neither `CommandLineToArgvW` nor `cmd.exe` mangles them. A ratchet test fails on any new direct import. Recognised npm/pnpm `.cmd` shims are resolved to their real target so the spawn skips `cmd.exe` entirely; see [`docs/reference/windows-cmd-shim-resolution.md`](./docs/reference/windows-cmd-shim-resolution.md) before adding a shim shape or debugging one. - **Windows process enumeration**: read the table through `src/main/windows/windows-process-table.ts`, never by forking `powershell.exe`. See [`docs/reference/windows-process-enumeration.md`](./docs/reference/windows-process-enumeration.md). diff --git a/docs/reference/remote-wire-compatibility.md b/docs/reference/remote-wire-compatibility.md index 2107120b092..d72f93770c5 100644 --- a/docs/reference/remote-wire-compatibility.md +++ b/docs/reference/remote-wire-compatibility.md @@ -102,7 +102,7 @@ gate, and a conflicted worktree that looks clean is granted a hosted-review crea should not have. Withholding an affordance is a degrade; removing the evidence a gate reads is not. -A fallback is only ever allowed to shape a *reading*. If the member is sent back to the +A fallback is only ever allowed to shape a _reading_. If the member is sent back to the host — a token the client echoes into a later call's params — pass it through as `z.string()` and let the send site keep it verbatim. `hostedReview`'s `provider` is the case: the eligibility reply names it and the create call returns it, so an diff --git a/src/main/usage/usage-scan-worker-event-loop.test.ts b/src/main/usage/usage-scan-worker-event-loop.test.ts index 7157d26373e..3f39a95ebbf 100644 --- a/src/main/usage/usage-scan-worker-event-loop.test.ts +++ b/src/main/usage/usage-scan-worker-event-loop.test.ts @@ -33,7 +33,12 @@ const EXPECTED_EVENTS = FILE_COUNT * EVENTS_PER_FILE const TOKENS_PER_EVENT = 200 const WORKTREES: UsageScanWorktreeRef[] = [ - { repoId: 'repo-1', worktreeId: 'wt-1', path: '/tmp/orca-usage-oracle-project', displayName: 'demo' } + { + repoId: 'repo-1', + worktreeId: 'wt-1', + path: '/tmp/orca-usage-oracle-project', + displayName: 'demo' + } ] let corpusRoot = '' diff --git a/src/renderer/src/components/editor/EditorPanelHeaderPath.test.tsx b/src/renderer/src/components/editor/EditorPanelHeaderPath.test.tsx new file mode 100644 index 00000000000..14fd66b266d --- /dev/null +++ b/src/renderer/src/components/editor/EditorPanelHeaderPath.test.tsx @@ -0,0 +1,228 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import { EditorPanelHeaderPath } from './EditorPanelHeaderPath' + +const renameFileOnDiskMock = vi.hoisted(() => vi.fn()) + +vi.mock('@/store/selectors', () => ({ + useWorktreeById: () => ({ path: '/repo', repoId: 'repo-1' }) +})) + +vi.mock('@/lib/rename-file', () => ({ + renameFileOnDisk: renameFileOnDiskMock +})) + +vi.mock('@/hooks/useShortcutLabel', () => ({ + useShortcutLabel: () => '' +})) + +vi.mock('@/i18n/i18n', async (importOriginal) => { + const actual = await importOriginal() // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.mock requires an inline import + return { + ...actual, + translate: (_key: string, fallback: string, options?: { value0?: string }) => + fallback.replace('{{value0}}', options?.value0 ?? '') + } +}) + +afterEach(cleanup) + +function baseFile(overrides: Partial = {}): OpenFile { + return { + id: '/repo/notes.md', + filePath: '/repo/notes.md', + relativePath: 'notes.md', + worktreeId: 'wt-1', + language: 'markdown', + isDirty: false, + mode: 'edit', + ...overrides + } +} + +function renderPath(file: OpenFile): (next: OpenFile) => void { + const view = render( + + ) + return (next) => + view.rerender( + + ) +} + +function getRenameInput(label: string): HTMLInputElement { + const input = screen.getByLabelText(label) + if (!(input instanceof HTMLInputElement)) { + throw new Error(`Missing rename input: ${label}`) + } + return input +} + +function openRenameInput(): void { + const pathRow = document.querySelector('.editor-header-path-row') + if (!pathRow) { + throw new Error('Missing editor header path row') + } + fireEvent.contextMenu(pathRow) + fireEvent.click(screen.getByRole('menuitem', { name: 'Rename' })) +} + +describe('EditorPanelHeaderPath inline rename', () => { + beforeEach(() => { + renameFileOnDiskMock.mockReset() + Object.assign(window, { api: { ui: { writeClipboardText: vi.fn() } } }) + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + }) + + it('opens a field holding the whole name', () => { + renderPath(baseFile()) + openRenameInput() + + const input = getRenameInput('Rename file notes.md') + expect(input.value).toBe('notes.md') + }) + + it('lets the field claim the full header width', () => { + renderPath(baseFile()) + openRenameInput() + + const input = getRenameInput('Rename file notes.md') + expect(input.className).toContain('w-full') + expect(input.className).toContain('max-w-full') + }) + + it('selects the basename so typing replaces just the name', () => { + renderPath(baseFile()) + openRenameInput() + + const input = getRenameInput('Rename file notes.md') + expect(document.activeElement).toBe(input) + expect(input.selectionStart).toBe(0) + expect(input.selectionEnd).toBe('notes'.length) + }) + + it('renames to the typed name verbatim', () => { + renderPath(baseFile()) + openRenameInput() + + const input = getRenameInput('Rename file notes.md') + fireEvent.change(input, { target: { value: 'renamed.mdx' } }) + fireEvent.keyDown(input, { key: 'Enter' }) + + expect(renameFileOnDiskMock).toHaveBeenCalledWith({ + oldPath: '/repo/notes.md', + newName: 'renamed.mdx', + worktreeId: 'wt-1', + worktreePath: '/repo' + }) + }) + + it('commits on blur like the tab bar and file explorer', () => { + renderPath(baseFile()) + openRenameInput() + + const input = getRenameInput('Rename file notes.md') + fireEvent.change(input, { target: { value: 'renamed.md' } }) + fireEvent.blur(input) + + expect(renameFileOnDiskMock).toHaveBeenCalledWith( + expect.objectContaining({ newName: 'renamed.md' }) + ) + }) + + it('does not request a rename when the name was not edited', () => { + renderPath(baseFile()) + openRenameInput() + + fireEvent.keyDown(getRenameInput('Rename file notes.md'), { key: 'Enter' }) + + expect(renameFileOnDiskMock).not.toHaveBeenCalled() + }) + + it('ignores an Enter that only confirms an IME candidate', () => { + renderPath(baseFile()) + openRenameInput() + + const input = getRenameInput('Rename file notes.md') + fireEvent.change(input, { target: { value: 'renamed.md' } }) + fireEvent.keyDown(input, { key: 'Enter', keyCode: 229 }) + expect(renameFileOnDiskMock).not.toHaveBeenCalled() + + fireEvent.keyDown(input, { key: 'Enter', keyCode: 13 }) + expect(renameFileOnDiskMock).toHaveBeenCalledWith( + expect.objectContaining({ newName: 'renamed.md' }) + ) + }) + + it('cancels on Escape without a trailing blur-commit, and ignores empty renames', () => { + renderPath(baseFile()) + openRenameInput() + + const input = getRenameInput('Rename file notes.md') + fireEvent.change(input, { target: { value: 'renamed.md' } }) + fireEvent.keyDown(input, { key: 'Escape' }) + fireEvent.blur(input) + expect(renameFileOnDiskMock).not.toHaveBeenCalled() + expect(screen.queryByLabelText('Rename file notes.md')).toBeNull() + + openRenameInput() + fireEvent.change(getRenameInput('Rename file notes.md'), { target: { value: ' ' } }) + fireEvent.keyDown(getRenameInput('Rename file notes.md'), { key: 'Enter' }) + expect(renameFileOnDiskMock).not.toHaveBeenCalled() + }) + + it('drops rename mode when the active file changes', () => { + const rerenderPath = renderPath(baseFile()) + openRenameInput() + + const input = getRenameInput('Rename file notes.md') + fireEvent.change(input, { target: { value: 'renamed.md' } }) + rerenderPath( + baseFile({ + id: '/repo/other.md', + filePath: '/repo/other.md', + relativePath: 'other.md' + }) + ) + + expect(screen.queryByLabelText('Rename file notes.md')).toBeNull() + expect(screen.queryByLabelText('Rename file other.md')).toBeNull() + + expect(renameFileOnDiskMock).not.toHaveBeenCalled() + }) + + it('selects the whole name when there is no extension', () => { + const file = baseFile({ + id: '/repo/Makefile', + filePath: '/repo/Makefile', + relativePath: 'Makefile' + }) + renderPath(file) + openRenameInput() + + const input = getRenameInput('Rename file Makefile') + expect(input.selectionStart).toBe(0) + expect(input.selectionEnd).toBe('Makefile'.length) + }) +}) diff --git a/src/renderer/src/components/editor/EditorPanelHeaderPath.tsx b/src/renderer/src/components/editor/EditorPanelHeaderPath.tsx index 0ea9e68a308..3ecd1e6e3d5 100644 --- a/src/renderer/src/components/editor/EditorPanelHeaderPath.tsx +++ b/src/renderer/src/components/editor/EditorPanelHeaderPath.tsx @@ -8,8 +8,8 @@ import { DropdownMenuShortcut, DropdownMenuTrigger } from '@/components/ui/dropdown-menu' -import { Input } from '@/components/ui/input' import { useShortcutLabel } from '@/hooks/useShortcutLabel' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' import { translate } from '@/i18n/i18n' import type { OpenFile } from '@/store/slices/editor' import { CLOSE_ALL_CONTEXT_MENUS_EVENT } from '../tab-bar/SortableTab' @@ -88,7 +88,7 @@ export function EditorPanelHeaderPath({ }} > {isRenaming ? ( - event.stopPropagation()} onMouseDown={(event) => event.stopPropagation()} onClick={(event) => event.stopPropagation()} onDoubleClick={(event) => event.stopPropagation()} onKeyDown={(event) => { + // Why: an Enter that only confirms a CJK IME candidate must not + // commit the rename; wait for a non-composition Enter. + if (isImeCompositionKeyDown(event)) { + return + } if (event.key === 'Enter') { event.preventDefault() event.stopPropagation() @@ -130,12 +135,17 @@ export function EditorPanelHeaderPath({ {displayPath.fileName} )} - - {headerCopyState.copyToastLabel} - + {/* Why: the toast is opacity-0 rather than display-none, so leaving it + mounted reserves ~100px of the row from the rename field for a + message that cannot fire while renaming. */} + {!isRenaming && ( + + {headerCopyState.copyToastLabel} + + )} diff --git a/src/renderer/src/components/editor/editor-header-file-rename.test.ts b/src/renderer/src/components/editor/editor-header-file-rename.test.ts new file mode 100644 index 00000000000..f62fabe4855 --- /dev/null +++ b/src/renderer/src/components/editor/editor-header-file-rename.test.ts @@ -0,0 +1,73 @@ +// @vitest-environment happy-dom + +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import { useEditorHeaderFileRename } from './editor-header-file-rename' + +const renameFileOnDiskMock = vi.hoisted(() => vi.fn()) + +vi.mock('@/store/selectors', () => ({ + useWorktreeById: () => ({ path: '/repo', repoId: 'repo-1' }) +})) + +vi.mock('@/lib/rename-file', () => ({ + renameFileOnDisk: renameFileOnDiskMock +})) + +function baseFile(overrides: Partial = {}): OpenFile { + return { + id: '/repo/notes.md', + filePath: '/repo/notes.md', + relativePath: 'notes.md', + worktreeId: 'wt-1', + language: 'markdown', + isDirty: false, + mode: 'edit', + ...overrides + } +} + +function renameInputStub(value: string): HTMLInputElement { + const input = document.createElement('input') + input.value = value + return input +} + +describe('useEditorHeaderFileRename', () => { + beforeEach(() => { + renameFileOnDiskMock.mockReset() + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + }) + + // Unmounting the focused input emits a trailing focusout, so commitRename can + // still run after the active file changed — with the old input as ref target. + it('ignores a blur-commit that arrives after the active file changed', () => { + const { result, rerender } = renderHook((file: OpenFile) => useEditorHeaderFileRename(file), { + initialProps: baseFile() + }) + + act(() => { + result.current.openRenameInput() + }) + act(() => { + result.current.renameInputRef(renameInputStub('renamed.md')) + }) + + rerender( + baseFile({ id: '/repo/other.md', filePath: '/repo/other.md', relativePath: 'other.md' }) + ) + + expect(result.current.isRenaming).toBe(false) + + act(() => { + result.current.commitRename() + }) + + expect(renameFileOnDiskMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/editor/editor-header-file-rename.ts b/src/renderer/src/components/editor/editor-header-file-rename.ts index e8a787dd2b9..5dc67d80358 100644 --- a/src/renderer/src/components/editor/editor-header-file-rename.ts +++ b/src/renderer/src/components/editor/editor-header-file-rename.ts @@ -19,9 +19,21 @@ type EditorHeaderFileRenameState = { export function useEditorHeaderFileRename(activeFile: OpenFile): EditorHeaderFileRenameState { const worktree = useWorktreeById(activeFile.worktreeId) const [isRenaming, setIsRenaming] = useState(false) + const [renameFilePath, setRenameFilePath] = useState(activeFile.filePath) const renameInputElementRef = useRef(null) - const renameCancelledRef = useRef(false) const renameFocusFrameRef = useRef(null) + // Escape fires setIsRenaming(false), which unmounts the input. The browser + // still fires focusout as the focused node is removed, so onBlur can invoke + // commitRename *after* cancel — committing the typed value against the + // user's intent. This flag suppresses the trailing blur-commit. + const renameCancelledRef = useRef(false) + // Why: the header renders one unkeyed path strip for every file, so a file + // switch mid-rename would otherwise commit the typed name against the new path. + if (renameFilePath !== activeFile.filePath) { + renameCancelledRef.current = true + setRenameFilePath(activeFile.filePath) + setIsRenaming(false) + } const currentFileName = basename(activeFile.filePath) // Why: read-only tabs (AI Vault View Log) are never renameable — rename would // rewrite the agent-owned artifact's backing path. @@ -88,8 +100,10 @@ export function useEditorHeaderFileRename(activeFile: OpenFile): EditorHeaderFil return } - // Why: focus belongs to the rename input mount; the frame preserves the - // previous timing so header layout settles before selecting text. + // Why: focus belongs to the rename input mount; the frame lets the header + // layout settle before selecting text. Preselect the basename only, so + // typing replaces the name and leaves the extension — same as the tab bar + // and the file explorer. renameFocusFrameRef.current = requestAnimationFrame(() => { renameFocusFrameRef.current = null if (renameInputElementRef.current !== el) {