diff --git a/config/electron-builder.config.cjs b/config/electron-builder.config.cjs index 1a31af9dfaf..13633ed8e01 100644 --- a/config/electron-builder.config.cjs +++ b/config/electron-builder.config.cjs @@ -105,6 +105,11 @@ const winSpeechNativeResource = { to: 'node_modules/sherpa-onnx-win-x64' } +// Why mirrored, not imported: this config is CJS loaded by electron-builder outside the TS build. +// Keep in sync with isMarkdownDocumentName() in src/main/ipc/markdown-documents.ts and with +// config/nsis/orca-installer-hooks.nsh, which registers the same set on Windows. +const MARKDOWN_FILE_EXTENSIONS = ['md', 'markdown', 'mdx'] + /** @type {import('electron-builder').Configuration} */ module.exports = { appId, @@ -376,12 +381,24 @@ module.exports = { shortcutName: '${productName}', uninstallDisplayName: '${productName}', createDesktopShortcut: 'always', - // Why: on a real uninstall, stop and remove the relocated terminal daemon - // (which lives outside the install dir under LOCALAPPDATA by design). Guarded - // by ${isUpdated} inside so it never runs during an update's uninstallOldVersion. - include: resolve(__dirname, 'nsis', 'daemon-host-uninstall.nsh') + // Why: electron-builder allows one include, so both Windows installer hooks live in it - + // the relocated-daemon uninstall sweep (guarded by ${isUpdated} so it never runs during an + // update's uninstallOldVersion) and the additive markdown "Open with" registration. + // Windows markdown association is deliberately NOT done via `fileAssociations`; see the + // header comment in that file for why that would steal the user's default .md handler. + include: resolve(__dirname, 'nsis', 'orca-installer-hooks.nsh') }, mac: { + // Why rank Alternate: Orca joins Finder's "Open With" list for Markdown without claiming + // LSHandlerRank ownership, so whichever editor the user already prefers stays the default. + // Why one entry per extension: app-builder-lib globs `*.${ext}`, which an array would break. + fileAssociations: MARKDOWN_FILE_EXTENSIONS.map((ext) => ({ + ext, + name: 'Markdown Document', + description: 'Markdown Document', + role: 'Editor', + rank: 'Alternate' + })), icon: 'resources/build/icon.icns', entitlements: 'resources/build/entitlements.mac.plist', entitlementsInherit: 'resources/build/entitlements.mac.plist', @@ -468,6 +485,12 @@ module.exports = { artifactName: 'orca-macos-${arch}.${ext}' }, linux: { + // Why mimeTypes and not fileAssociations: shared-mime-info already maps *.md/*.markdown to + // text/markdown, so reusing that type puts Orca in the Open With list without shipping a glob + // override. A desktop entry's MimeType only adds a handler - mimeapps.list still owns the + // default. .mdx is deliberately absent: Ubuntu 24.04's mime database maps it to + // application/x-genesis-32x-rom, so claiming it here would need a glob override. + mimeTypes: ['text/markdown'], // Why: Ubuntu desktop ships GNOME Orca as the `orca` package and /usr/bin/orca. // The Linux installer should not claim those system package/file names. executableName: 'orca-ide', diff --git a/config/nsis/daemon-host-uninstall.nsh b/config/nsis/daemon-host-uninstall.nsh deleted file mode 100644 index dc3a497ce67..00000000000 --- a/config/nsis/daemon-host-uninstall.nsh +++ /dev/null @@ -1,23 +0,0 @@ -; Clean up the relocated terminal daemon on a REAL uninstall. -; -; Why: the daemon host is deliberately copied to a distinct image name -; (orca-terminal-daemon.exe) under %LOCALAPPDATA%\Orca\daemon-host so that app -; UPDATES cannot kill it — that relocation is what keeps terminals alive across -; updates. The same design means a normal uninstall's process sweep and file -; removal both miss it, leaving an orphaned daemon plus its runtime copy behind. -; -; The ${isUpdated} guard is essential: electron-builder runs this uninstaller as -; part of uninstallOldVersion on EVERY update, and killing the daemon there would -; defeat the whole feature. Only clean up on a genuine uninstall. -; -; The image name and the LOCALAPPDATA folder name must stay in sync with -; DAEMON_HOST_EXE_NAME and LOCAL_HOST_ROOT_NAME in -; src/main/daemon/daemon-host-relocation.ts. -!macro customUnInstall - ${ifNot} ${isUpdated} - nsExec::Exec 'taskkill /F /IM orca-terminal-daemon.exe' - ; Give the OS a moment to release the image lock before removing the tree. - Sleep 500 - RMDir /r "$LOCALAPPDATA\Orca\daemon-host" - ${endIf} -!macroend diff --git a/config/nsis/orca-installer-hooks.nsh b/config/nsis/orca-installer-hooks.nsh new file mode 100644 index 00000000000..ca80c99fc6d --- /dev/null +++ b/config/nsis/orca-installer-hooks.nsh @@ -0,0 +1,79 @@ +; electron-builder NSIS hooks for the Orca Windows installer. +; +; electron-builder accepts exactly ONE `nsis.include` file, so every customInstall / +; customUnInstall hook Orca needs lives here. + +; --------------------------------------------------------------------------- +; Markdown "Open with Orca" (issue #10138) +; +; Why hand-rolled instead of electron-builder's `fileAssociations` on Windows: +; app-builder-lib emits !insertmacro APP_ASSOCIATE, whose first line is +; WriteRegStr SHELL_CONTEXT "Software\Classes\.md" "" "" +; That overwrites whichever editor currently owns .md, with no backup, for every +; existing user on their next UPDATE - and APP_UNASSOCIATE never restores it, so +; uninstalling Orca would leave .md pointing at a deleted ProgID. +; +; These writes are additive only. Registering a ProgID plus an OpenWithProgids +; hint and an Applications\\SupportedTypes entry puts Orca in Explorer's +; "Open with" list and in "Choose another app", while the default handler stays +; exactly where the user left it. Never add a `Software\Classes\.` default +; value here. +; +; MARKDOWN_PROGID must stay in sync with the extension list handled by +; isMarkdownDocumentName() in src/main/ipc/markdown-documents.ts. +; --------------------------------------------------------------------------- +!define MARKDOWN_PROGID "Orca.Markdown" + +!macro ORCA_REGISTER_MARKDOWN_OPEN_WITH EXT + WriteRegNone SHELL_CONTEXT "Software\Classes\${EXT}\OpenWithProgids" "${MARKDOWN_PROGID}" + WriteRegStr SHELL_CONTEXT "Software\Classes\Applications\${APP_EXECUTABLE_FILENAME}\SupportedTypes" "${EXT}" "" +!macroend + +!macro ORCA_UNREGISTER_MARKDOWN_OPEN_WITH EXT + DeleteRegValue SHELL_CONTEXT "Software\Classes\${EXT}\OpenWithProgids" "${MARKDOWN_PROGID}" + DeleteRegValue SHELL_CONTEXT "Software\Classes\Applications\${APP_EXECUTABLE_FILENAME}\SupportedTypes" "${EXT}" +!macroend + +!macro customInstall + WriteRegStr SHELL_CONTEXT "Software\Classes\${MARKDOWN_PROGID}" "" "Markdown Document" + WriteRegStr SHELL_CONTEXT "Software\Classes\${MARKDOWN_PROGID}\DefaultIcon" "" "$appExe,0" + WriteRegStr SHELL_CONTEXT "Software\Classes\${MARKDOWN_PROGID}\shell\open" "" "Open with ${PRODUCT_NAME}" + WriteRegStr SHELL_CONTEXT "Software\Classes\${MARKDOWN_PROGID}\shell\open\command" "" '"$appExe" "%1"' + !insertmacro ORCA_REGISTER_MARKDOWN_OPEN_WITH ".md" + !insertmacro ORCA_REGISTER_MARKDOWN_OPEN_WITH ".markdown" + !insertmacro ORCA_REGISTER_MARKDOWN_OPEN_WITH ".mdx" + ; Why: Explorer caches the association list until told otherwise. + System::Call "shell32::SHChangeNotify(i,i,i,i) (0x08000000, 0x1000, 0, 0)" +!macroend + +; --------------------------------------------------------------------------- +; Clean up the relocated terminal daemon on a REAL uninstall. +; +; Why: the daemon host is deliberately copied to a distinct image name +; (orca-terminal-daemon.exe) under %LOCALAPPDATA%\Orca\daemon-host so that app +; UPDATES cannot kill it — that relocation is what keeps terminals alive across +; updates. The same design means a normal uninstall's process sweep and file +; removal both miss it, leaving an orphaned daemon plus its runtime copy behind. +; +; The ${isUpdated} guard is essential: electron-builder runs this uninstaller as +; part of uninstallOldVersion on EVERY update, and killing the daemon there would +; defeat the whole feature. Only clean up on a genuine uninstall. +; +; The image name and the LOCALAPPDATA folder name must stay in sync with +; DAEMON_HOST_EXE_NAME and LOCAL_HOST_ROOT_NAME in +; src/main/daemon/daemon-host-relocation.ts. +!macro customUnInstall + ${ifNot} ${isUpdated} + nsExec::Exec 'taskkill /F /IM orca-terminal-daemon.exe' + ; Give the OS a moment to release the image lock before removing the tree. + Sleep 500 + RMDir /r "$LOCALAPPDATA\Orca\daemon-host" + ${endIf} + ; Why outside the ${isUpdated} guard: customInstall rewrites these on every update, so + ; dropping them during uninstallOldVersion is correct and keeps the pair symmetric. + DeleteRegKey SHELL_CONTEXT "Software\Classes\${MARKDOWN_PROGID}" + !insertmacro ORCA_UNREGISTER_MARKDOWN_OPEN_WITH ".md" + !insertmacro ORCA_UNREGISTER_MARKDOWN_OPEN_WITH ".markdown" + !insertmacro ORCA_UNREGISTER_MARKDOWN_OPEN_WITH ".mdx" + System::Call "shell32::SHChangeNotify(i,i,i,i) (0x08000000, 0x1000, 0, 0)" +!macroend diff --git a/config/scripts/electron-builder-markdown-associations.test.mjs b/config/scripts/electron-builder-markdown-associations.test.mjs new file mode 100644 index 00000000000..7ae3b1c9428 --- /dev/null +++ b/config/scripts/electron-builder-markdown-associations.test.mjs @@ -0,0 +1,116 @@ +import { existsSync } from 'node:fs' +import { readFile } from 'node:fs/promises' +import { createRequire } from 'node:module' +import { basename } from 'node:path' +import { describe, expect, it } from 'vitest' + +const require = createRequire(import.meta.url) +const electronBuilderConfig = require('../electron-builder.config.cjs') + +const MARKDOWN_EXTENSIONS = ['md', 'markdown', 'mdx'] + +// The exact shape app-builder-lib's APP_ASSOCIATE emits: a write to the DEFAULT ("") +// value of Software\Classes\.. Additive `WriteRegNone ...\OpenWithProgids` must not +// match, or the guard below would be unfalsifiable. +const DEFAULT_HANDLER_WRITE = /WriteRegStr\s+SHELL_CONTEXT\s+"Software\\Classes\\\.[a-z]+"\s+""/i + +// The hooks file documents the forbidden line in prose, so match executable script only. +const stripNsisCommentLines = (source) => + source + .split('\n') + .filter((line) => !/^\s*[;#]/.test(line)) + .join('\n') + +const readInstallerHooks = () => readFile(electronBuilderConfig.nsis.include, 'utf8') + +describe('electron-builder markdown file associations', () => { + // Why: any top-level (or `win.`) fileAssociations entry makes app-builder-lib's NSIS + // packager emit `!insertmacro APP_ASSOCIATE`, whose first line writes that DEFAULT value + // — silently taking .md from whichever editor owns it, for every existing user on their + // next UPDATE, with APP_UNASSOCIATE never restoring it. `rank: 'Alternate'` cannot + // prevent this; it is LSHandlerRank and applies to macOS only. So the mac block must + // stay under `mac.` — hoisting it up "to share it with Windows" is what this test blocks. + it('never claims the Windows default markdown handler', () => { + expect(electronBuilderConfig.fileAssociations).toBeUndefined() + expect(electronBuilderConfig.win?.fileAssociations).toBeUndefined() + }) + + it('joins the macOS Open With list for every markdown extension without owning it', () => { + const associations = electronBuilderConfig.mac.fileAssociations + // One entry per extension: an array `ext` would break the Linux packager's `*.${ext}` glob. + expect([...associations].map((association) => association.ext).sort()).toEqual( + [...MARKDOWN_EXTENSIONS].sort() + ) + for (const association of associations) { + expect(association).toMatchObject({ role: 'Editor', rank: 'Alternate' }) + } + }) + + // Why mimeTypes and not linux.fileAssociations: shared-mime-info already maps markdown to + // text/markdown, so the desktop entry only adds a handler and mimeapps.list keeps owning + // the default. A fileAssociations entry would ship a redundant glob override instead. + it('reuses the existing shared-mime-info markdown type on Linux', () => { + expect(electronBuilderConfig.linux.mimeTypes).toContain('text/markdown') + expect(electronBuilderConfig.linux.fileAssociations).toBeUndefined() + }) + + it('points the single NSIS include at the installer hooks file on disk', () => { + const includePath = electronBuilderConfig.nsis.include + expect(existsSync(includePath)).toBe(true) + expect(basename(includePath)).toBe('orca-installer-hooks.nsh') + }) + + // Guard for the guard: proves DEFAULT_HANDLER_WRITE really matches a takeover line, so + // the assertion below is a live check rather than a regex that can never fire. + it('recognizes an APP_ASSOCIATE-style default-handler write', () => { + for (const takeover of [ + ' WriteRegStr SHELL_CONTEXT "Software\\Classes\\.md" "" "Orca.Markdown"', + 'WriteRegStr SHELL_CONTEXT "Software\\Classes\\.markdown" "" "$0"' + ]) { + expect(takeover).toMatch(DEFAULT_HANDLER_WRITE) + } + expect( + 'WriteRegNone SHELL_CONTEXT "Software\\Classes\\.md\\OpenWithProgids" "Orca.Markdown"' + ).not.toMatch(DEFAULT_HANDLER_WRITE) + // Comment stripping must drop prose that quotes the bad line without swallowing a real + // one that happens to carry a trailing comment. + const stripped = stripNsisCommentLines( + [ + '; WriteRegStr SHELL_CONTEXT "Software\\Classes\\.md" "" ""', + ' WriteRegStr SHELL_CONTEXT "Software\\Classes\\.md" "" "$0" ; oops' + ].join('\n') + ) + expect(stripped.split('\n')).toHaveLength(1) + expect(stripped).toMatch(DEFAULT_HANDLER_WRITE) + }) + + it('registers Windows markdown Open With additively, never as the default', async () => { + const hooks = await readInstallerHooks() + + expect(stripNsisCommentLines(hooks)).not.toMatch(DEFAULT_HANDLER_WRITE) + // The additive hint that puts Orca in Explorer's "Open with" list. + expect(hooks).toMatch( + /WriteRegNone\s+SHELL_CONTEXT\s+"Software\\Classes\\\$\{EXT\}\\OpenWithProgids"/ + ) + expect(hooks).toMatch(/!macro\s+ORCA_REGISTER_MARKDOWN_OPEN_WITH\s+EXT/) + for (const ext of MARKDOWN_EXTENSIONS) { + expect(hooks).toContain(`ORCA_REGISTER_MARKDOWN_OPEN_WITH ".${ext}"`) + expect(hooks).toContain(`ORCA_UNREGISTER_MARKDOWN_OPEN_WITH ".${ext}"`) + } + expect(hooks).toMatch(/!macro\s+customInstall\b/) + expect(hooks).toMatch(/!macro\s+customUnInstall\b/) + }) + + // Why: this include was renamed from daemon-host-uninstall.nsh to carry the markdown + // hooks too. electron-builder allows only one include, so a merge that drops the daemon + // sweep would silently orphan a running orca-terminal-daemon.exe on every uninstall. + it('keeps the daemon-host uninstall sweep across the include rename', async () => { + const hooks = await readInstallerHooks() + + expect(hooks).toContain('orca-terminal-daemon.exe') + expect(hooks).toContain('$LOCALAPPDATA\\Orca\\daemon-host') + // Without this guard, uninstallOldVersion would kill the daemon on every update — + // defeating the relocation that keeps terminals alive across updates. + expect(hooks).toMatch(/\$\{ifNot\}\s+\$\{isUpdated\}/) + }) +}) diff --git a/config/scripts/space-sharing-copy.mjs b/config/scripts/space-sharing-copy.mjs index 191bd8bffdc..01e9c8ac5ef 100644 --- a/config/scripts/space-sharing-copy.mjs +++ b/config/scripts/space-sharing-copy.mjs @@ -110,32 +110,60 @@ export function makeTreeReadOnly(targetPath, chmod = chmodSync) { chmod(targetPath, 0o755) } +/** + * Restore owner write permission across a private copy. + * + * Counterpart to `makeTreeReadOnly`: clonefile, reflink and `cpSync` all carry the source's mode + * across, so a tree copied from the write-protected shared cache lands read-only and every patch + * the caller then makes -- `plutil -replace`, `codesign` -- fails with EACCES. Only the owner bit + * comes back; group and other stay as the source left them. + */ +export function makeTreeWritable(targetPath, chmod = chmodSync) { + for (const entry of readdirSync(targetPath, { withFileTypes: true })) { + const entryPath = join(targetPath, entry.name) + if (entry.isDirectory()) { + makeTreeWritable(entryPath, chmod) + } else if (!entry.isSymbolicLink()) { + const mode = statSync(entryPath, { throwIfNoEntry: false })?.mode + chmod(entryPath, mode === undefined ? 0o644 : mode | 0o200) + } + } + chmod(targetPath, 0o755) +} + /** * Share storage when possible, otherwise copy the bytes. * * Never hardlinks: this is for trees the caller goes on to patch, where shared inodes would write - * through into the source. + * through into the source. The copy is unprotected on the way out for the same reason -- a private + * tree the caller cannot write to is useless to it. */ export function copyPrivateTree(sourcePath, destinationPath, options = {}) { const platform = options.platform ?? process.platform const copy = options.copy ?? copyTreeVerbatim + const unprotect = options.unprotect ?? makeTreeWritable const privateMechanisms = new Set(['clone', 'reflink']) + let result = { mechanism: null, copyError: null } if (getShareMechanisms(platform).some((mechanism) => privateMechanisms.has(mechanism))) { try { - const mechanism = shareTree(sourcePath, destinationPath, { - ...options, - hardlink: () => { - throw new Error('hardlinks would not be private') - } - }) - return { mechanism, copyError: null } + result = { + mechanism: shareTree(sourcePath, destinationPath, { + ...options, + hardlink: () => { + throw new Error('hardlinks would not be private') + } + }), + copyError: null + } } catch (copyError) { copy(sourcePath, destinationPath) - return { mechanism: null, copyError } + result = { mechanism: null, copyError } } + } else { + copy(sourcePath, destinationPath) } - copy(sourcePath, destinationPath) - return { mechanism: null, copyError: null } + unprotect(destinationPath) + return result } function copyTreeVerbatim(sourcePath, destinationPath) { diff --git a/config/scripts/space-sharing-copy.test.ts b/config/scripts/space-sharing-copy.test.ts index 3ce35aae25e..351f44b286e 100644 --- a/config/scripts/space-sharing-copy.test.ts +++ b/config/scripts/space-sharing-copy.test.ts @@ -19,6 +19,7 @@ import { copyPrivateTree, hardlinkTree, makeTreeReadOnly, + makeTreeWritable, shareTree } from './space-sharing-copy.mjs' @@ -170,7 +171,41 @@ describe('makeTreeReadOnly', () => { ) }) +describe('makeTreeWritable', () => { + it.runIf(process.platform !== 'win32')('undoes makeTreeReadOnly for the owner', () => { + const { source } = makeTree() + makeTreeReadOnly(source) + makeTreeWritable(source) + const file = path.join(source, 'nested', 'file') + expect(statSync(file).mode & 0o200).toBe(0o200) + expect(() => writeFileSync(file, 'mutated')).not.toThrow() + }) + + it.runIf(process.platform !== 'win32')('adds no write permission beyond the owner', () => { + const { source } = makeTree() + const executable = path.join(source, 'electron') + writeFileSync(executable, 'binary') + chmodSync(executable, 0o555) + makeTreeWritable(source) + expect(statSync(executable).mode & 0o777).toBe(0o755) + }) +}) + describe('copyPrivateTree', () => { + it.runIf(process.platform !== 'win32')( + 'hands back a tree the caller can patch, even from a write-protected source', + () => { + const { root, source } = makeTree() + const destination = path.join(root, 'private') + makeTreeReadOnly(source) + copyPrivateTree(source, destination) + // The regression this guards: the shared Electron dist is read-only, clonefile/reflink/cpSync + // all carry that across, and `pn dev` then died patching the copied bundle's Info.plist. + expect(() => writeFileSync(path.join(destination, 'nested', 'file'), 'patched')).not.toThrow() + expect(readFileSync(path.join(source, 'nested', 'file'), 'utf8')).toBe('contents') + } + ) + it('never hardlinks, because the caller patches what it gets back', () => { const { root, source } = makeTree() const destination = path.join(root, 'private') diff --git a/docs/reference/git-compatibility.md b/docs/reference/git-compatibility.md index 3004b8888ab..0e8b1f257d3 100644 --- a/docs/reference/git-compatibility.md +++ b/docs/reference/git-compatibility.md @@ -42,6 +42,17 @@ authority. | `merge-tree-write-tree` | Derive real-merge conflicts and no-op tree proofs | Omit the conflict summary and keep conservative branch cleanup behavior before Git 2.38 | | `merge-tree-merge-base` | Supply the already-resolved merge base | Use the older two-commit `merge-tree --write-tree` form | +### Placeholders That Fail Open + +`GitCapabilityCache` records commands Git *rejects*. A `git log --format` +placeholder Git does not know is not rejected: Git echoes it verbatim and exits +zero, so there is no error to remember and no probe to cache. Ask for both forms +in one record and pick at parse time. + +| Placeholder | Preferred behavior | Compatibility behavior | +| ---------------- | ------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------ | +| `%(decorate:…)` | Git 2.43 separates commit decorations with `\x1f`, so ref names containing commas survive | The same record also carries `%D` (Git 2.10); an unexpanded `%(decorate` placeholder selects it, at the cost of comma-splitting | + ## Why Not `simple-git` `simple-git` is a process wrapper around the installed Git binary. Its custom diff --git a/mobile/src/terminal/terminal-webview-html-source.test-support.ts b/mobile/src/terminal/terminal-webview-html-source.test-support.ts index 25902305f41..19a9cfc07ba 100644 --- a/mobile/src/terminal/terminal-webview-html-source.test-support.ts +++ b/mobile/src/terminal/terminal-webview-html-source.test-support.ts @@ -1,24 +1,31 @@ import { readFileSync } from 'node:fs' -const SOURCE_FILES = [ - './terminal-webview-html.ts', - './terminal-webview-html/document-shell.ts', - './terminal-webview-html/runtime-state-and-text-scaling.ts', - './terminal-webview-html/fit-scale-and-write-queue.ts', - './terminal-webview-html/terminal-init-and-write.ts', - './terminal-webview-html/host-message-router.ts', - './terminal-webview-html/selection-state-and-eviction.ts', - './terminal-webview-html/term-observers-and-mode-mirroring.ts', - './terminal-webview-html/mouse-report-and-scroll-routing.ts', - './terminal-webview-html/smooth-scroll-and-cell-geometry.ts', - './terminal-webview-html/selection-overlay.ts', - './terminal-webview-html/surface-touch-gestures.ts', - './terminal-webview-html/message-bridge-and-document-close.ts' -] as const +const COMPOSER_FILE = './terminal-webview-html.ts' +const SLICE_IMPORT_RE = /^import \{[^}]*\} from '(\.\/terminal-webview-html\/[\w-]+)'$/gm +const COMPOSED_ENTRY_RE = /^ {2}TERMINAL_HTML_\w+,?$/gm -/** Reads the TypeScript source that assembles the in-WebView document. */ +function readSource(relativePath: string): string { + return readFileSync(new URL(relativePath, import.meta.url), 'utf8') +} + +/** + * Reads the TypeScript source that assembles the in-WebView document. + * + * Why: the slice list is derived from the composer's own imports rather than duplicated, so a + * new slice cannot join the emitted document while staying invisible to the tests that search + * this source. The count cross-check catches an import shape the regex cannot see. + */ export function readTerminalWebViewHtmlSource(): string { - return SOURCE_FILES.map((relativePath) => - readFileSync(new URL(relativePath, import.meta.url), 'utf8') - ).join('\n') + const composer = readSource(COMPOSER_FILE) + const slices = [...composer.matchAll(SLICE_IMPORT_RE)].map((match) => `${match[1]}.ts`) + const composedCount = [...composer.matchAll(COMPOSED_ENTRY_RE)].length + if (composedCount === 0) { + throw new Error('no composed WebView document slices found') + } + if (slices.length !== composedCount) { + throw new Error( + `WebView document slice imports (${slices.length}) do not match composed entries (${composedCount})` + ) + } + return [composer, ...slices.map(readSource)].join('\n') } diff --git a/mobile/src/terminal/terminal-webview-html.ts b/mobile/src/terminal/terminal-webview-html.ts index 40b7a2db22c..17fadd4d26c 100644 --- a/mobile/src/terminal/terminal-webview-html.ts +++ b/mobile/src/terminal/terminal-webview-html.ts @@ -1,6 +1,8 @@ import { TERMINAL_HTML_DOCUMENT_SHELL } from './terminal-webview-html/document-shell' import { TERMINAL_HTML_RUNTIME_STATE_AND_TEXT_SCALING } from './terminal-webview-html/runtime-state-and-text-scaling' -import { TERMINAL_HTML_FIT_SCALE_AND_WRITE_QUEUE } from './terminal-webview-html/fit-scale-and-write-queue' +import { TERMINAL_HTML_FIT_SCALE } from './terminal-webview-html/terminal-fit-scale' +import { TERMINAL_HTML_MOUSE_MODE_DECSET_SCAN } from './terminal-webview-html/mouse-mode-decset-scan' +import { TERMINAL_HTML_WRITE_QUEUE } from './terminal-webview-html/write-queue' import { TERMINAL_HTML_INIT_AND_WRITE } from './terminal-webview-html/terminal-init-and-write' import { TERMINAL_HTML_HOST_MESSAGE_ROUTER } from './terminal-webview-html/host-message-router' import { TERMINAL_HTML_SELECTION_STATE_AND_EVICTION } from './terminal-webview-html/selection-state-and-eviction' @@ -19,7 +21,9 @@ export { MOBILE_TERMINAL_CARET_OPTIONS } from './terminal-webview-html/theme' export const XTERM_HTML = [ TERMINAL_HTML_DOCUMENT_SHELL, TERMINAL_HTML_RUNTIME_STATE_AND_TEXT_SCALING, - TERMINAL_HTML_FIT_SCALE_AND_WRITE_QUEUE, + TERMINAL_HTML_FIT_SCALE, + TERMINAL_HTML_MOUSE_MODE_DECSET_SCAN, + TERMINAL_HTML_WRITE_QUEUE, TERMINAL_HTML_INIT_AND_WRITE, TERMINAL_HTML_HOST_MESSAGE_ROUTER, TERMINAL_HTML_SELECTION_STATE_AND_EVICTION, diff --git a/mobile/src/terminal/terminal-webview-html/fit-scale-and-write-queue.ts b/mobile/src/terminal/terminal-webview-html/fit-scale-and-write-queue.ts deleted file mode 100644 index 074185d43a5..00000000000 --- a/mobile/src/terminal/terminal-webview-html/fit-scale-and-write-queue.ts +++ /dev/null @@ -1,288 +0,0 @@ -import { TERMINAL_WEBVIEW_THEME_JS } from '../terminal-webview-theme-injected' - -// Also carries the DECSET mouse-mode scanner: emitted-document order pins it between these two concerns. -export const TERMINAL_HTML_FIT_SCALE_AND_WRITE_QUEUE = `${TERMINAL_WEBVIEW_THEME_JS} - - function getCellHeight() { - if (!term || !term._core) return 15; - var core = term._core; - if (core._renderService && core._renderService.dimensions) { - return core._renderService.dimensions.css.cell.height || 15; - } - return 15; - } - - // Why: clamp pan so the terminal content always covers the viewport - // when zoomed in. When content is smaller than viewport in a - // dimension, pin to top-left (no floating in the middle). - function clampPan() { - if (!term || !term.element) return; - var ts = getTotalScale(); - var cw = term.element.scrollWidth * ts; - var ch = term.element.scrollHeight * ts; - var vpW = window.innerWidth; - var vpH = window.innerHeight; - if (cw > vpW) { - panX = Math.min(0, Math.max(vpW - cw, panX)); - } else { - panX = 0; - } - if (ch > vpH) { - panY = Math.min(0, Math.max(vpH - ch, panY)); - } else { - panY = 0; - } - } - - // Why: intentional no-op. Mobile replays a live PTY snapshot then applies - // live cursor-relative chunks from that same PTY; resizing only the WebView - // xterm changes cursor coordinates and makes TUI repaint chunks duplicate or - // overlap. Kept as a no-op so its call sites stay legible. - function adjustRowsForViewport() {} - - // Why: cold-start fit. After init() opens xterm, the renderer needs - // several frames before cell dimensions are computed. Reading too early - // gives cellWidth=0 (renderer service not ready) or scrollWidth=0 (DOM - // not laid out), and computeFitScale returns 1 → no zoom. - // - // Gate: cellWidth × cols is the canonical "logical width" of the grid - // and reflects xterm's layout decision, independent of buffer content. - // We commit when cellWidth becomes positive (renderer ready). Fallback: - // if cellWidth never becomes available, gate on stable positive - // scrollWidth (xterm rendered something). Cap at 60 frames (~1s @60Hz) - // so a backgrounded WebView never spins forever. - var FIT_RETRY_MAX_FRAMES = 60; - var fitRetryToken = 0; - function applyFitScale(reason) { - if (!term || !term.element) return; - var token = ++fitRetryToken; - var attempts = 0; - var lastScrollWidth = -1; - function attempt() { - if (token !== fitRetryToken) return; - if (!term || !term.element) return; - attempts++; - var cellW = getCellWidth(); - if (cellW > 0 && term.cols > 0) { - commitFitScale(reason, attempts, 'cellW'); - return; - } - var w = term.element.scrollWidth; - if (w > 0 && w === lastScrollWidth) { - commitFitScale(reason, attempts, 'stableSW'); - return; - } - lastScrollWidth = w; - if (attempts >= FIT_RETRY_MAX_FRAMES) { - flog('commit-timeout', { - reason: reason, - attempts: attempts, - cellW: cellW, - scrollWidth: w, - cols: term.cols - }); - commitFitScale(reason, attempts, 'timeout'); - return; - } - requestAnimationFrame(attempt); - } - requestAnimationFrame(attempt); - } - - function commitFitScale(reason, attempts, gate) { - if (!term || !term.element) return; - var preSnapScale = computeFitScale(); - currentScale = preSnapScale; - // Why: when scale is very close to 1 (e.g. 0.97 from xterm scrollbar - // sub-pixels) snap to 1 to avoid imperceptible shrinkage that prevents - // a second applyFitScale from observing a "no-op needed" state. - if (currentScale >= 0.95) currentScale = 1; - userScale = 1; - panX = 0; - panY = 0; - smoothScrollOffsetY = 0; - updateTransform(); - adjustRowsForViewport(); - - var cellW = getCellWidth(); - var sw = term.element.scrollWidth; - var vpW = window.innerWidth; - var expectedW = cellW * term.cols; - var suspect = - currentScale === 1 && term.cols > 0 && expectedW > vpW + 1; // expected wider than viewport but no zoom - if (suspect) { - flog('commit-SUSPECT', { - reason: reason, - attempts: attempts, - gate: gate, - preSnapScale: preSnapScale, - finalScale: currentScale, - cellW: cellW, - cols: term.cols, - expectedW: expectedW, - scrollWidth: sw, - vpWidth: vpW - }); - } - repositionOverlay(); - } - - function isAltScreenActive(data) { - if (typeof data !== 'string') return false; - var on = data.lastIndexOf(ESC + '[?1049h'); - var off = data.lastIndexOf(ESC + '[?1049l'); - return on !== -1 && on > off; - } - - function normalizeInitialData(data) { - if (!isAltScreenActive(data)) return data; - var on = data.lastIndexOf(ESC + '[?1049h'); - // Why: SerializeAddon can include normal-buffer scrollback before the - // active alternate-screen snapshot. Replaying both into a fresh mobile - // xterm duplicates TUI frames and can flatten SGR attributes. - return on > 0 ? data.slice(on) : data; - } - - function updateMouseModeFromData(data) { - if (typeof data !== 'string' || data.length === 0) return; - var input = mouseModeScanTail + data; - mouseModeScanTail = extractMouseModeScanTail(input); - var re = new RegExp(ESC + 'c|' + ESC + '\\\\[\\\\?([0-9;]+)([hl])|' + C1_CSI + '\\\\?([0-9;]+)([hl])', 'g'); - var match; - while ((match = re.exec(input)) !== null) { - if (match[0] === ESC + 'c') { - trackedMouseTrackingMode = 'none'; - sgrMouseMode = false; - sgrMousePixelsMode = false; - continue; - } - var enabled = (match[2] || match[4]) === 'h'; - var params = (match[1] || match[3]).split(';'); - for (var i = 0; i < params.length; i++) { - if (params[i] === '') continue; - var param = Number(params[i]); - if (!Number.isInteger(param)) continue; - if (param === 9) trackedMouseTrackingMode = enabled ? 'x10' : 'none'; - if (param === 1000) trackedMouseTrackingMode = enabled ? 'vt200' : 'none'; - if (param === 1002) trackedMouseTrackingMode = enabled ? 'drag' : 'none'; - if (param === 1003) trackedMouseTrackingMode = enabled ? 'any' : 'none'; - if (param === 1006) { - sgrMouseMode = enabled; - sgrMousePixelsMode = false; - } - if (param === 1016) { - sgrMouseMode = false; - sgrMousePixelsMode = enabled; - } - } - } - } - - function resetWriteQueue() { - writeQueue = []; - writeQueueHead = 0; - } - - function isStatusDotPresentationSelector(value) { - return value === TEXT_PRESENTATION_SELECTOR || value === EMOJI_PRESENTATION_SELECTOR; - } - - function endsWithStatusDotPresentationSequence(data) { - var i = data.length - 1; - while (i >= 0 && isStatusDotPresentationSelector(data.charAt(i))) i--; - return i >= 0 && data.charAt(i) === CLAUDE_STATUS_DOT; - } - - // Why: iOS WebKit promotes Claude's record/status dot to a colorful emoji glyph. - function normalizeStatusDotPresentation(data) { - if (typeof data !== 'string' || data.length === 0) return data; - if (statusDotPendingSelector) { - statusDotPendingSelector = false; - var strippedPendingSelectors = false; - while (data.length > 0 && isStatusDotPresentationSelector(data.charAt(0))) data = data.slice(1); - strippedPendingSelectors = data.length === 0; - if (strippedPendingSelectors) { - statusDotPendingSelector = true; - return ''; - } - } - var normalized = data.replace(CLAUDE_STATUS_DOT_PATTERN, CLAUDE_STATUS_DOT + TEXT_PRESENTATION_SELECTOR); - statusDotPendingSelector = endsWithStatusDotPresentationSequence(data); - return normalized; - } - - function enqueueWrite(data) { - writeQueue.push(normalizeStatusDotPresentation(data)); - } - - function enqueueWriteBoundary(callback) { - writeQueue.push(callback); - } - - function nextQueuedWrite() { - if (writeQueueHead >= writeQueue.length) { - resetWriteQueue(); - return undefined; - } - var next = writeQueue[writeQueueHead]; - writeQueueHead++; - // Why: high-throughput terminals can enqueue faster than xterm parses; - // compact consumed slots so drain work stays O(1) without retaining old chunks. - if (writeQueueHead > 128 && writeQueueHead * 2 > writeQueue.length) { - writeQueue = writeQueue.slice(writeQueueHead); - writeQueueHead = 0; - } - return next; - } - - function disposeTermObservers() { - var disposables = termObserverDisposables; - termObserverDisposables = []; - for (var i = 0; i < disposables.length; i++) { - try { disposables[i] && disposables[i].dispose && disposables[i].dispose(); } catch (e) {} - } - } - - function extractMouseModeScanTail(input) { - var start = Math.max(input.lastIndexOf(ESC), input.lastIndexOf(C1_CSI)); - if (start === -1) return ''; - var tail = input.slice(start); - // Why: PTY/SSH chunks can split a long combined DECSET before the final h/l. - // Keep parser state far beyond normal mode lists while still bounding memory. - if (tail.length > PRIVATE_MODE_SCAN_TAIL_LIMIT) return ''; - if (tail === ESC || tail === ESC + '[' || tail === C1_CSI) return tail; - if (tail.indexOf(ESC + '[?') === 0) { - return /^[0-9;]*$/.test(tail.slice(3)) ? tail : ''; - } - if (tail.indexOf(C1_CSI + '?') === 0) { - return /^[0-9;]*$/.test(tail.slice(2)) ? tail : ''; - } - return ''; - } - - function pumpWrites(gen) { - if (!ready || !term || writesDraining || gen !== terminalGeneration) return; - var next = nextQueuedWrite(); - if (typeof next !== 'string') { - if (typeof next === 'function') return next(), pumpWrites(gen); - var callbacks = afterDrainCallbacks; - afterDrainCallbacks = []; - for (var i = 0; i < callbacks.length; i++) callbacks[i](); - return; - } - writesDraining = true; - // Why: xterm.write() parses asynchronously. Row adjustment/resizing must - // wait until replayed SGR attributes have landed in the buffer. - term.write(next, function() { - if (gen !== terminalGeneration) return; - writesDraining = false; - pumpWrites(gen); - }); - } - - function afterWritesDrained(callback) { - afterDrainCallbacks.push(callback); - pumpWrites(terminalGeneration); - } - -` diff --git a/mobile/src/terminal/terminal-webview-html/mouse-mode-decset-scan.ts b/mobile/src/terminal/terminal-webview-html/mouse-mode-decset-scan.ts new file mode 100644 index 00000000000..6f0685df87e --- /dev/null +++ b/mobile/src/terminal/terminal-webview-html/mouse-mode-decset-scan.ts @@ -0,0 +1,52 @@ +export const TERMINAL_HTML_MOUSE_MODE_DECSET_SCAN = ` function isAltScreenActive(data) { + if (typeof data !== 'string') return false; + var on = data.lastIndexOf(ESC + '[?1049h'); + var off = data.lastIndexOf(ESC + '[?1049l'); + return on !== -1 && on > off; + } + + function normalizeInitialData(data) { + if (!isAltScreenActive(data)) return data; + var on = data.lastIndexOf(ESC + '[?1049h'); + // Why: SerializeAddon can include normal-buffer scrollback before the + // active alternate-screen snapshot. Replaying both into a fresh mobile + // xterm duplicates TUI frames and can flatten SGR attributes. + return on > 0 ? data.slice(on) : data; + } + + function updateMouseModeFromData(data) { + if (typeof data !== 'string' || data.length === 0) return; + var input = mouseModeScanTail + data; + mouseModeScanTail = extractMouseModeScanTail(input); + var re = new RegExp(ESC + 'c|' + ESC + '\\\\[\\\\?([0-9;]+)([hl])|' + C1_CSI + '\\\\?([0-9;]+)([hl])', 'g'); + var match; + while ((match = re.exec(input)) !== null) { + if (match[0] === ESC + 'c') { + trackedMouseTrackingMode = 'none'; + sgrMouseMode = false; + sgrMousePixelsMode = false; + continue; + } + var enabled = (match[2] || match[4]) === 'h'; + var params = (match[1] || match[3]).split(';'); + for (var i = 0; i < params.length; i++) { + if (params[i] === '') continue; + var param = Number(params[i]); + if (!Number.isInteger(param)) continue; + if (param === 9) trackedMouseTrackingMode = enabled ? 'x10' : 'none'; + if (param === 1000) trackedMouseTrackingMode = enabled ? 'vt200' : 'none'; + if (param === 1002) trackedMouseTrackingMode = enabled ? 'drag' : 'none'; + if (param === 1003) trackedMouseTrackingMode = enabled ? 'any' : 'none'; + if (param === 1006) { + sgrMouseMode = enabled; + sgrMousePixelsMode = false; + } + if (param === 1016) { + sgrMouseMode = false; + sgrMousePixelsMode = enabled; + } + } + } + } + +` diff --git a/mobile/src/terminal/terminal-webview-html/terminal-fit-scale.ts b/mobile/src/terminal/terminal-webview-html/terminal-fit-scale.ts new file mode 100644 index 00000000000..b756bcb550c --- /dev/null +++ b/mobile/src/terminal/terminal-webview-html/terminal-fit-scale.ts @@ -0,0 +1,130 @@ +import { TERMINAL_WEBVIEW_THEME_JS } from '../terminal-webview-theme-injected' + +// Opens with the injected theme block: it lands at this point in the emitted document. +export const TERMINAL_HTML_FIT_SCALE = `${TERMINAL_WEBVIEW_THEME_JS} + + function getCellHeight() { + if (!term || !term._core) return 15; + var core = term._core; + if (core._renderService && core._renderService.dimensions) { + return core._renderService.dimensions.css.cell.height || 15; + } + return 15; + } + + // Why: clamp pan so the terminal content always covers the viewport + // when zoomed in. When content is smaller than viewport in a + // dimension, pin to top-left (no floating in the middle). + function clampPan() { + if (!term || !term.element) return; + var ts = getTotalScale(); + var cw = term.element.scrollWidth * ts; + var ch = term.element.scrollHeight * ts; + var vpW = window.innerWidth; + var vpH = window.innerHeight; + if (cw > vpW) { + panX = Math.min(0, Math.max(vpW - cw, panX)); + } else { + panX = 0; + } + if (ch > vpH) { + panY = Math.min(0, Math.max(vpH - ch, panY)); + } else { + panY = 0; + } + } + + // Why: intentional no-op. Mobile replays a live PTY snapshot then applies + // live cursor-relative chunks from that same PTY; resizing only the WebView + // xterm changes cursor coordinates and makes TUI repaint chunks duplicate or + // overlap. Kept as a no-op so its call sites stay legible. + function adjustRowsForViewport() {} + + // Why: cold-start fit. After init() opens xterm, the renderer needs + // several frames before cell dimensions are computed. Reading too early + // gives cellWidth=0 (renderer service not ready) or scrollWidth=0 (DOM + // not laid out), and computeFitScale returns 1 → no zoom. + // + // Gate: cellWidth × cols is the canonical "logical width" of the grid + // and reflects xterm's layout decision, independent of buffer content. + // We commit when cellWidth becomes positive (renderer ready). Fallback: + // if cellWidth never becomes available, gate on stable positive + // scrollWidth (xterm rendered something). Cap at 60 frames (~1s @60Hz) + // so a backgrounded WebView never spins forever. + var FIT_RETRY_MAX_FRAMES = 60; + var fitRetryToken = 0; + function applyFitScale(reason) { + if (!term || !term.element) return; + var token = ++fitRetryToken; + var attempts = 0; + var lastScrollWidth = -1; + function attempt() { + if (token !== fitRetryToken) return; + if (!term || !term.element) return; + attempts++; + var cellW = getCellWidth(); + if (cellW > 0 && term.cols > 0) { + commitFitScale(reason, attempts, 'cellW'); + return; + } + var w = term.element.scrollWidth; + if (w > 0 && w === lastScrollWidth) { + commitFitScale(reason, attempts, 'stableSW'); + return; + } + lastScrollWidth = w; + if (attempts >= FIT_RETRY_MAX_FRAMES) { + flog('commit-timeout', { + reason: reason, + attempts: attempts, + cellW: cellW, + scrollWidth: w, + cols: term.cols + }); + commitFitScale(reason, attempts, 'timeout'); + return; + } + requestAnimationFrame(attempt); + } + requestAnimationFrame(attempt); + } + + function commitFitScale(reason, attempts, gate) { + if (!term || !term.element) return; + var preSnapScale = computeFitScale(); + currentScale = preSnapScale; + // Why: when scale is very close to 1 (e.g. 0.97 from xterm scrollbar + // sub-pixels) snap to 1 to avoid imperceptible shrinkage that prevents + // a second applyFitScale from observing a "no-op needed" state. + if (currentScale >= 0.95) currentScale = 1; + userScale = 1; + panX = 0; + panY = 0; + smoothScrollOffsetY = 0; + updateTransform(); + adjustRowsForViewport(); + + var cellW = getCellWidth(); + var sw = term.element.scrollWidth; + var vpW = window.innerWidth; + var expectedW = cellW * term.cols; + var suspect = + currentScale === 1 && term.cols > 0 && expectedW > vpW + 1; // expected wider than viewport but no zoom + if (suspect) { + flog('commit-SUSPECT', { + reason: reason, + attempts: attempts, + gate: gate, + preSnapScale: preSnapScale, + finalScale: currentScale, + cellW: cellW, + cols: term.cols, + expectedW: expectedW, + scrollWidth: sw, + vpWidth: vpW + }); + } + repositionOverlay(); + } + +` diff --git a/mobile/src/terminal/terminal-webview-html/write-queue.ts b/mobile/src/terminal/terminal-webview-html/write-queue.ts new file mode 100644 index 00000000000..ae8ed85297f --- /dev/null +++ b/mobile/src/terminal/terminal-webview-html/write-queue.ts @@ -0,0 +1,110 @@ +// Also carries disposeTermObservers() and extractMouseModeScanTail(): both belong to +// other concerns, but emitted-document order pins them inside this queue. +export const TERMINAL_HTML_WRITE_QUEUE = ` function resetWriteQueue() { + writeQueue = []; + writeQueueHead = 0; + } + + function isStatusDotPresentationSelector(value) { + return value === TEXT_PRESENTATION_SELECTOR || value === EMOJI_PRESENTATION_SELECTOR; + } + + function endsWithStatusDotPresentationSequence(data) { + var i = data.length - 1; + while (i >= 0 && isStatusDotPresentationSelector(data.charAt(i))) i--; + return i >= 0 && data.charAt(i) === CLAUDE_STATUS_DOT; + } + + // Why: iOS WebKit promotes Claude's record/status dot to a colorful emoji glyph. + function normalizeStatusDotPresentation(data) { + if (typeof data !== 'string' || data.length === 0) return data; + if (statusDotPendingSelector) { + statusDotPendingSelector = false; + var strippedPendingSelectors = false; + while (data.length > 0 && isStatusDotPresentationSelector(data.charAt(0))) data = data.slice(1); + strippedPendingSelectors = data.length === 0; + if (strippedPendingSelectors) { + statusDotPendingSelector = true; + return ''; + } + } + var normalized = data.replace(CLAUDE_STATUS_DOT_PATTERN, CLAUDE_STATUS_DOT + TEXT_PRESENTATION_SELECTOR); + statusDotPendingSelector = endsWithStatusDotPresentationSequence(data); + return normalized; + } + + function enqueueWrite(data) { + writeQueue.push(normalizeStatusDotPresentation(data)); + } + + function enqueueWriteBoundary(callback) { + writeQueue.push(callback); + } + + function nextQueuedWrite() { + if (writeQueueHead >= writeQueue.length) { + resetWriteQueue(); + return undefined; + } + var next = writeQueue[writeQueueHead]; + writeQueueHead++; + // Why: high-throughput terminals can enqueue faster than xterm parses; + // compact consumed slots so drain work stays O(1) without retaining old chunks. + if (writeQueueHead > 128 && writeQueueHead * 2 > writeQueue.length) { + writeQueue = writeQueue.slice(writeQueueHead); + writeQueueHead = 0; + } + return next; + } + + function disposeTermObservers() { + var disposables = termObserverDisposables; + termObserverDisposables = []; + for (var i = 0; i < disposables.length; i++) { + try { disposables[i] && disposables[i].dispose && disposables[i].dispose(); } catch (e) {} + } + } + + function extractMouseModeScanTail(input) { + var start = Math.max(input.lastIndexOf(ESC), input.lastIndexOf(C1_CSI)); + if (start === -1) return ''; + var tail = input.slice(start); + // Why: PTY/SSH chunks can split a long combined DECSET before the final h/l. + // Keep parser state far beyond normal mode lists while still bounding memory. + if (tail.length > PRIVATE_MODE_SCAN_TAIL_LIMIT) return ''; + if (tail === ESC || tail === ESC + '[' || tail === C1_CSI) return tail; + if (tail.indexOf(ESC + '[?') === 0) { + return /^[0-9;]*$/.test(tail.slice(3)) ? tail : ''; + } + if (tail.indexOf(C1_CSI + '?') === 0) { + return /^[0-9;]*$/.test(tail.slice(2)) ? tail : ''; + } + return ''; + } + + function pumpWrites(gen) { + if (!ready || !term || writesDraining || gen !== terminalGeneration) return; + var next = nextQueuedWrite(); + if (typeof next !== 'string') { + if (typeof next === 'function') return next(), pumpWrites(gen); + var callbacks = afterDrainCallbacks; + afterDrainCallbacks = []; + for (var i = 0; i < callbacks.length; i++) callbacks[i](); + return; + } + writesDraining = true; + // Why: xterm.write() parses asynchronously. Row adjustment/resizing must + // wait until replayed SGR attributes have landed in the buffer. + term.write(next, function() { + if (gen !== terminalGeneration) return; + writesDraining = false; + pumpWrites(gen); + }); + } + + function afterWritesDrained(callback) { + afterDrainCallbacks.push(callback); + pumpWrites(terminalGeneration); + } + +` diff --git a/mobile/src/terminal/terminal-webview-payload-hash.test.ts b/mobile/src/terminal/terminal-webview-payload-hash.test.ts new file mode 100644 index 00000000000..f8bfa4bd134 --- /dev/null +++ b/mobile/src/terminal/terminal-webview-payload-hash.test.ts @@ -0,0 +1,17 @@ +import { createHash } from 'node:crypto' +import { describe, expect, it } from 'vitest' +import { XTERM_HTML } from './terminal-webview-html' + +// Why: every other WebView test exercises one slice of the document, so an edit to an +// uncovered region ships silently. A diff here means the emitted WebView source changed — +// update these values only when that change is deliberate, and only after checking the +// document still runs. Refactors that merely move slice boundaries must leave them alone. +const EXPECTED_SHA256 = '42cc000faddc3b58b8fd4855f848c7878f0cd6166c613f66d733645e8e1b9608' +const EXPECTED_LENGTH = 729776 + +describe('terminal WebView payload', () => { + it('composes the expected document', () => { + expect(XTERM_HTML.length).toBe(EXPECTED_LENGTH) + expect(createHash('sha256').update(XTERM_HTML, 'utf8').digest('hex')).toBe(EXPECTED_SHA256) + }) +}) diff --git a/src/main/daemon/daemon-host-relocation.ts b/src/main/daemon/daemon-host-relocation.ts index a7e8f2b6db2..6d94bea06e4 100644 --- a/src/main/daemon/daemon-host-relocation.ts +++ b/src/main/daemon/daemon-host-relocation.ts @@ -34,7 +34,7 @@ export type RelocatedDaemonHost = { const HOST_SUBDIR = 'daemon-host' const MARKER_NAME = '.materialized.json' -// LOCAL appData (not roaming) so OneDrive/roaming never syncs this ~260MB runtime. Shared with NSIS uninstall (config/nsis/daemon-host-uninstall.nsh) — keep in sync. +// LOCAL appData (not roaming) so OneDrive/roaming never syncs this ~260MB runtime. Shared with NSIS uninstall (config/nsis/orca-installer-hooks.nsh) — keep in sync. const LOCAL_HOST_ROOT_NAME = 'Orca' // Copy of Orca.exe renamed to a distinct image name so the NSIS updater's `taskkill /IM Orca.exe` can't match it. diff --git a/src/main/index.ts b/src/main/index.ts index acf913b2c7a..522e59b908f 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -12,6 +12,7 @@ import { registerMainProcessIpcHandlers } from './startup/main-process-ipc-boots import { initializeMainProcessReady } from './startup/main-process-ready' import { installMainProcessQuitHandlers } from './startup/main-process-quit' import { shouldActivateDesktopForSecondInstance } from './startup/single-instance-lock' +import { resolveOpenedMarkdownDocuments } from './startup/os-opened-markdown-files' function openMainWindow(options: { revealOnDidFinishLoad?: boolean } = {}): BrowserWindow { return openMainWindowController(options) @@ -27,6 +28,7 @@ function requestDesktopActivation(argv: readonly string[] = []): void { state.skillShareDeepLinks.capture(argv, (shareId) => { state.mainWindow?.webContents.send('ui:openSkillShare', shareId) }) + state.osOpenedMarkdownFiles.capture(argv, publishOsOpenedMarkdownFiles) // Why: a duplicate `orca serve` must not drag a headless server into opening a desktop window (#11935). if (!shouldActivateDesktopForSecondInstance(argv)) { return @@ -34,6 +36,39 @@ function requestDesktopActivation(argv: readonly string[] = []): void { state.desktopActivationGate?.requestActivation() } +/** + * Hands buffered OS-opened markdown paths to a renderer that has proven it is listening. + * + * Until that proof arrives the paths stay buffered, because `webContents.send` to a renderer + * with no listener attached is dropped silently and the queue would be gone. + */ +function publishOsOpenedMarkdownFiles(): void { + const targetWindow = state.mainWindow + if (!state.markdownFileOpenListenerReady || !targetWindow || targetWindow.isDestroyed()) { + return + } + // Why consumed before the await: a renderer pull racing this resolve must not take the same + // batch again. The restore() calls hand it back if delivery turns out to be impossible. + const filePaths = state.osOpenedMarkdownFiles.consume() + if (filePaths.length === 0) { + return + } + void resolveOpenedMarkdownDocuments(filePaths) + .then((documents) => { + if (targetWindow.isDestroyed() || targetWindow.webContents.isDestroyed()) { + state.osOpenedMarkdownFiles.restore(filePaths) + return + } + if (documents.length > 0) { + targetWindow.webContents.send('ui:openMarkdownFiles', documents) + } + }) + .catch((error) => { + state.osOpenedMarkdownFiles.restore(filePaths) + console.warn('[os-open] Failed to resolve OS-opened markdown files:', error) + }) +} + const handleMacAppActivation = createMacAppActivationHandler({ getWindow: () => state.mainWindow, requestActivation: requestDesktopActivation @@ -53,7 +88,22 @@ if (preflightReady) { event.preventDefault() requestDesktopActivation([url]) }) + // Why: macOS delivers "Open With" as open-file, often before `ready`, and only to a handler + // that claims the event. Non-markdown paths stay unclaimed so the OS default handler wins. + app.on('open-file', (event, filePath) => { + if (!state.osOpenedMarkdownFiles.captureFilePaths([filePath], publishOsOpenedMarkdownFiles)) { + return + } + event.preventDefault() + // Why gated on isReady: pre-ready the cold-start window is already on its way, and + // activating the gate here would try to open one before Electron can. + if (app.isReady()) { + requestDesktopActivation() + } + }) state.skillShareDeepLinks.capture(process.argv) + // Why no publish: nothing is listening this early, so the first renderer pulls these on mount. + state.osOpenedMarkdownFiles.capture(process.argv) registerMainProcessIpcHandlers() installMainProcessQuitHandlers() void app.whenReady().then(async () => { diff --git a/src/main/ipc/worktrees-ssh-repo-owner-resolution.test.ts b/src/main/ipc/worktrees-ssh-repo-owner-resolution.test.ts index 6a398e9b02d..b3231da86f6 100644 --- a/src/main/ipc/worktrees-ssh-repo-owner-resolution.test.ts +++ b/src/main/ipc/worktrees-ssh-repo-owner-resolution.test.ts @@ -1,7 +1,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { GitWorktreeInfo, Worktree } from '../../shared/worktree/types' import type { ProviderRequestId } from '../../shared/detected-worktree-provider-contract' -import { LOCAL_EXECUTION_HOST_ID, toSshExecutionHostId } from '../../shared/execution-host' +import { + LOCAL_EXECUTION_HOST_ID, + toRuntimeExecutionHostId, + toSshExecutionHostId +} from '../../shared/execution-host' import { getSshProviderAuthority } from '../ssh/ssh-provider-authority' import { listWorktreesMock, @@ -443,7 +447,76 @@ describe('registerWorktreeHandlers', () => { expect(store.removeWorktreeMeta).not.toHaveBeenCalled() }) - it('refuses to retire metadata for non-SSH hosts and unowned repos', async () => { + // Runtime-host rows are exempt from gcStaleWorktreeMeta exactly as SSH ones are, so a paired + // client needs this path to ever drop them (#17776). + it('retires runtime-host metadata an authoritative scan proved gone', async () => { + const runtimeHostId = toRuntimeExecutionHostId('env-1') + const runtimeRepo = { + id: 'repo-1', + path: '/home/orca/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + executionHostId: runtimeHostId + } + const metaById: Record> = { + 'repo-1::/home/orca/deleted': makeWorktreeMeta({ hostId: runtimeHostId }), + 'repo-1::/home/orca/other-host': makeWorktreeMeta({ + hostId: toSshExecutionHostId('target-a') + }) + } + store.getRepos.mockReturnValue([runtimeRepo]) + store.getProjectHostSetups.mockReturnValue([]) + store.getAllWorktreeMeta.mockReturnValue(metaById) + store.removeWorktreeMeta.mockImplementation((worktreeId: string) => { + delete metaById[worktreeId] + }) + + const forgotten = await handlers['worktrees:forgetRemovedForExecutionHost'](null, { + repoId: runtimeRepo.id, + executionHostId: runtimeHostId, + worktreeIds: ['repo-1::/home/orca/deleted', 'repo-1::/home/orca/other-host'] + }) + + // The row stamped to another host needs that host's own scan, not this one's. + expect(forgotten).toEqual({ forgottenWorktreeIds: ['repo-1::/home/orca/deleted'] }) + expect(store.removeWorktreeMeta).toHaveBeenCalledExactlyOnceWith( + 'repo-1::/home/orca/deleted', + runtimeHostId + ) + }) + + // A repo that reaches its checkouts over SSH is not the runtime host's to condemn. The refusal + // comes from `findExactRepoOwner`: a runtime `executionHostId` beside a `connectionId` is + // contradictory ownership evidence, so no owner resolves at all. + it('refuses to retire a connection-backed repo under a runtime host id', async () => { + const runtimeHostId = toRuntimeExecutionHostId('env-1') + store.getRepos.mockReturnValue([ + { + id: 'repo-1', + path: '/home/orca/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + executionHostId: runtimeHostId, + connectionId: 'target-a' + } + ]) + store.getAllWorktreeMeta.mockReturnValue({ + 'repo-1::/home/orca/deleted': makeWorktreeMeta({ hostId: runtimeHostId }) + }) + + expect( + await handlers['worktrees:forgetRemovedForExecutionHost'](null, { + repoId: 'repo-1', + executionHostId: runtimeHostId, + worktreeIds: ['repo-1::/home/orca/deleted'] + }) + ).toEqual({ forgottenWorktreeIds: [] }) + expect(store.removeWorktreeMeta).not.toHaveBeenCalled() + }) + + it('refuses to retire metadata for non-executing hosts and unowned repos', async () => { const sshRepo = { id: 'repo-1', path: '/remote/repo-a', diff --git a/src/main/ipc/worktrees/listing/register-host-catalog-handlers.ts b/src/main/ipc/worktrees/listing/register-host-catalog-handlers.ts index 0d47f3638c2..4467506e790 100644 --- a/src/main/ipc/worktrees/listing/register-host-catalog-handlers.ts +++ b/src/main/ipc/worktrees/listing/register-host-catalog-handlers.ts @@ -103,11 +103,21 @@ export function registerHostCatalogHandlers(context: WorktreeIpcContext): void { const requestedExecutionHostId = args?.executionHostId ?? 'ssh:' const worktreeIds = Array.isArray(args?.worktreeIds) ? args.worktreeIds : [] const parsedHost = parseExecutionHostId(requestedExecutionHostId) - if (parsedHost?.kind !== 'ssh' || worktreeIds.length === 0) { + // Runtime hosts belong here for the same reason SSH ones do: their rows are exempt from + // gcStaleWorktreeMeta, so a scan-proven removal is the only thing that ever retires them. + if ( + (parsedHost?.kind !== 'ssh' && parsedHost?.kind !== 'runtime') || + worktreeIds.length === 0 + ) { return nothingForgotten } + // No runtime arm in the check below: `findExactRepoOwner` already refuses a repo carrying both + // a runtime `executionHostId` and a `connectionId`, because `resolveRepoOwnershipEvidence` + // calls that pair contradictory and one non-owned candidate voids the whole lookup. A second + // check would be unreachable, and unreachable code on a destructive path reads as a guarantee + // it is not making. const repo = findExactRepoOwner(store, args?.repoId ?? '', requestedExecutionHostId) - if (!repo || repo.connectionId !== parsedHost.targetId) { + if (!repo || (parsedHost.kind === 'ssh' && repo.connectionId !== parsedHost.targetId)) { return nothingForgotten } // Why: a folder workspace's meta IS the workspace record, not a checkout row — gcStaleWorktreeMeta skips diff --git a/src/main/orca-profiles/profile-project-worktree-identity.ts b/src/main/orca-profiles/profile-project-worktree-identity.ts index 1586a0e0120..8cf58dd1bd5 100644 --- a/src/main/orca-profiles/profile-project-worktree-identity.ts +++ b/src/main/orca-profiles/profile-project-worktree-identity.ts @@ -66,6 +66,24 @@ export function rekeyOwnerKey( return null } +/** + * Every worktree locator an owner key could name. + * + * Two readings, because one key can be both: with a repo literally named `worktree`, + * `worktree::/p` is a `::` locator AND parses as a `worktree:` workspace key naming + * repo `` (empty). `ownerKeyBelongsToRepo` accepts either, so a caller that reasons about a key + * without a repo id in hand has to consider both or it will disagree with the predicate. + */ +export function ownerKeyWorktreeIds(ownerKey: string): string[] { + const rawOwnerKey = isWorktreeHostIdentity(ownerKey) + ? getWorktreeIdFromHostIdentity(ownerKey) + : ownerKey + const scope = parseWorkspaceKey(ownerKey) + return scope?.type === 'worktree' && scope.worktreeId !== rawOwnerKey + ? [rawOwnerKey, scope.worktreeId] + : [rawOwnerKey] +} + export function ownerKeyBelongsToRepo(ownerKey: string, repoId: string): boolean { const rawOwnerKey = isWorktreeHostIdentity(ownerKey) ? getWorktreeIdFromHostIdentity(ownerKey) diff --git a/src/main/persistence-cohort-and-identity-migration.test.ts b/src/main/persistence-cohort-and-identity-migration.test.ts index 4081672c821..a894b8a4c63 100644 --- a/src/main/persistence-cohort-and-identity-migration.test.ts +++ b/src/main/persistence-cohort-and-identity-migration.test.ts @@ -397,6 +397,8 @@ describe('Store.migrateWorktreeIdentity', () => { it('moves persisted mobile selections across reloads', async () => { const store = await createStore() + // Registered on purpose: rows owned by an unregistered repo id are swept as orphans on load. + store.addRepo(makeRepo({ id: 'repo1', path: '/repo1' })) store.setMobileClientTabSelections({ 'device-a': { [OLD]: { activeTabId: 'tab-1', activeGroupId: null, activeTabIdByGroupId: {} } diff --git a/src/main/persistence-cross-host-pane-identity.test.ts b/src/main/persistence-cross-host-pane-identity.test.ts index 2b6a70da934..479d3727837 100644 --- a/src/main/persistence-cross-host-pane-identity.test.ts +++ b/src/main/persistence-cross-host-pane-identity.test.ts @@ -10,6 +10,7 @@ import { createStore, writeDataFile, readDataFile, + makeRepo, makeTerminalTab } from './persistence-test-harness' @@ -53,6 +54,11 @@ describe('cross-host pane identity migration', () => { it('refuses hostless alias and acknowledgement rewrites for a tab id two partitions share', async () => { writeDataFile({ schemaVersion: 1, + // Registered on purpose: rows owned by an unregistered repo id are swept as orphans on load. + repos: [ + makeRepo({ id: 'repo-local', path: '/repo-local' }), + makeRepo({ id: 'repo-a', path: '/repo-a' }) + ], workspaceSession: makeLegacyPaneSession('repo-local', 'local-pty'), workspaceSessionsByHostId: { 'ssh:host-a': makeLegacyPaneSession('repo-a', 'pty-a') diff --git a/src/main/persistence-deregistered-repo-residue.test.ts b/src/main/persistence-deregistered-repo-residue.test.ts new file mode 100644 index 00000000000..3a7a3372b3c --- /dev/null +++ b/src/main/persistence-deregistered-repo-residue.test.ts @@ -0,0 +1,240 @@ +// Why this file exists: deregistering a project used to strand every row it owned. No sweeper could +// reach them -- the missing-directory prune is gated on the repo still being registered, and a +// paired client's mirror of a remote host's rows is keyed by ids that client never registers, so the +// owning host's removal never reached it (#17776). +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { rmSync, mkdtempSync } from 'node:fs' +import { join } from 'node:path' +import { tmpdir } from 'node:os' +import { getDefaultWorkspaceSession } from '../shared/constants' +import { composeWorktreeHostIdentity } from '../shared/worktree/host-qualified-identity' +import { folderWorkspaceKey, worktreeWorkspaceKey } from '../shared/workspace-scope' +import type { PersistedState } from '../shared/persisted-state-types' +import { + testState, + createStore, + writeDataFile, + readDataFile, + makeRepo, + makeTerminalTab +} from './persistence-test-harness' + +vi.mock('./ssh/ssh-config-parser', () => ({ + loadUserSshConfig: vi.fn(), + sshConfigHostsToTargets: vi.fn() +})) + +vi.mock('electron', () => ({ + app: { getPath: () => testState.dir }, + safeStorage: { isEncryptionAvailable: () => false } +})) + +vi.mock('./telemetry/client', () => ({ track: vi.fn() })) +vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn().mockReturnValue({}) })) + +const LIVE_REPO = 'live-repo' +const GONE_REPO = 'gone-repo' +const LIVE_WORKTREE = `${LIVE_REPO}::/workspace/live` +const GONE_WORKTREE = `${GONE_REPO}::/workspace/orphan` +const RUNTIME_HOST = 'runtime:env-a' + +const sleepingAgentFor = (worktreeId: string, tabId = 'tab-1') => ({ + [`${tabId}:leaf-1`]: { + paneKey: `${tabId}:leaf-1`, + tabId, + worktreeId, + agent: 'codex' as const, + providerSession: { key: 'session_id' as const, id: 'sess-1' }, + prompt: 'sleeping', + state: 'waiting' as const, + capturedAt: 1, + updatedAt: 1, + origin: 'worktree-sleep' as const + } +}) + +const sessionFor = (worktreeId: string, tabId = 'tab-1') => ({ + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + [worktreeId]: [makeTerminalTab({ id: tabId, worktreeId })] + }, + activeTabTypeByWorktree: { [worktreeId]: 'terminal' as const }, + lastVisitedAtByWorktreeId: { [worktreeId]: 123 }, + // The residue `profile-project-session-field-disposition` flags as leaking on repo removal. + sleepingAgentSessionsByPaneKey: sleepingAgentFor(worktreeId, tabId) +}) + +describe('deregistered repo residue', () => { + beforeEach(() => { + testState.dir = mkdtempSync(join(tmpdir(), 'orca-orphan-sweep-')) + }) + + afterEach(() => { + rmSync(testState.dir, { recursive: true, force: true }) + }) + + it('drops metadata, identity rows and sessions owned by an unregistered repo id', async () => { + const seed = await createStore() + seed.addRepo(makeRepo({ id: LIVE_REPO, path: '/workspace/live' })) + seed.addRepo(makeRepo({ id: GONE_REPO, path: '/workspace/orphan' })) + seed.setWorktreeMetaForHost(LIVE_WORKTREE, 'local', { displayName: 'Live' }) + seed.setWorktreeMetaForHost(GONE_WORKTREE, 'local', { displayName: 'Orphan' }) + seed.setWorkspaceSession(sessionFor(GONE_WORKTREE), 'local') + seed.flush() + + // Deregister by hand: the point is that a row can outlive its repo however that happened. + const persisted = readDataFile() as PersistedState + persisted.repos = persisted.repos.filter((repo) => repo.id !== GONE_REPO) + writeDataFile(persisted) + + const reloaded = await createStore() + reloaded.flush() + const swept = readDataFile() as PersistedState + + expect(Object.keys(swept.worktreeMeta)).toEqual([LIVE_WORKTREE]) + expect(swept.worktreeIdentityAliases).not.toHaveProperty( + composeWorktreeHostIdentity('local', GONE_WORKTREE) + ) + expect(Object.keys(swept.worktreeMetaByIdentity ?? {})).toHaveLength(1) + const session = swept.workspaceSession + expect(session.tabsByWorktree).toEqual({}) + expect(session.lastVisitedAtByWorktreeId).toEqual({}) + expect(session.activeTabTypeByWorktree).toEqual({}) + expect(session.sleepingAgentSessionsByPaneKey ?? {}).toEqual({}) + }) + + it("sweeps a remote host's session partition the owning host's removal can never reach", async () => { + writeDataFile({ + schemaVersion: 1, + repos: [makeRepo({ id: LIVE_REPO, path: '/workspace/live' })], + worktreeMeta: {}, + workspaceSessionsByHostId: { + [RUNTIME_HOST]: sessionFor(GONE_WORKTREE) + } + }) + + const store = await createStore() + store.flush() + + const partition = store.getWorkspaceSession(RUNTIME_HOST) + expect(partition.tabsByWorktree).toEqual({}) + expect(partition.activeTabTypeByWorktree).toEqual({}) + }) + + it('keeps rows for every registered repo, on any execution host', async () => { + const remoteWorktree = `${LIVE_REPO}::/home/user/remote` + writeDataFile({ + schemaVersion: 1, + repos: [makeRepo({ id: LIVE_REPO, path: '/home/user/live', executionHostId: RUNTIME_HOST })], + worktreeMeta: { [remoteWorktree]: { hostId: RUNTIME_HOST, status: 'active' } }, + workspaceSessionsByHostId: { [RUNTIME_HOST]: sessionFor(remoteWorktree) } + }) + + const store = await createStore() + + expect(store.getWorktreeMeta(remoteWorktree)).toBeDefined() + const partition = store.getWorkspaceSession(RUNTIME_HOST) + expect(partition.tabsByWorktree[remoteWorktree]).toHaveLength(1) + // Also proves the sleeping-agent fixture is well-formed, so the sweep assertions above bite. + expect(Object.keys(partition.sleepingAgentSessionsByPaneKey ?? {})).toHaveLength(1) + }) + + it('leaves folder-workspace session rows alone: their keys name no repo', async () => { + const workspaceKey = folderWorkspaceKey('folder-1') + writeDataFile({ + schemaVersion: 1, + repos: [], + worktreeMeta: {}, + workspaceSession: { + ...getDefaultWorkspaceSession(), + lastVisitedAtByWorktreeId: { [workspaceKey]: 7 } + } + }) + + const store = await createStore() + + expect(store.getWorkspaceSession('local').lastVisitedAtByWorktreeId).toEqual({ + [workspaceKey]: 7 + }) + }) + + // Regression: the pane-keyed records are pruned by the worktreeId they name, not by their own key, + // so an orphan whose ONLY residue is a sleeping agent survived -- and re-seeded the sweep on every + // launch, so the store never self-cleared and every load scheduled another save. + it("drops a sleeping agent that is the orphan repo's only residue, and self-clears", async () => { + writeDataFile({ + schemaVersion: 1, + repos: [makeRepo({ id: LIVE_REPO, path: '/workspace/live' })], + worktreeMeta: {}, + workspaceSession: { + ...getDefaultWorkspaceSession(), + sleepingAgentSessionsByPaneKey: sleepingAgentFor(GONE_WORKTREE) + } + }) + + const store = await createStore() + store.flush() + expect(store.getWorkspaceSession('local').sleepingAgentSessionsByPaneKey ?? {}).toEqual({}) + // On disk, not just in memory: if the flush had not persisted the cleanup, the next load would + // silently redo it and the self-clearing assertion below would pass without meaning anything. + const persisted = readDataFile() as PersistedState + expect(persisted.workspaceSession.sleepingAgentSessionsByPaneKey ?? {}).toEqual({}) + + // Self-clearing: with the residue gone nothing re-seeds the orphan id, so the next launch has + // no work. Before the fix this stayed non-empty forever and every load scheduled another save. + const reloaded = await createStore() + expect(reloaded.sweepDeregisteredRepoResidue()).toEqual([]) + }) + + // The session scalars are pruned by bespoke rules, not by owner key, so no owner-key loop reaches + // them. Each has to be able to seed the sweep on its own or an orphan named only there is stuck. + it.each([ + { label: 'activeWorktreeId', session: { activeWorktreeId: GONE_WORKTREE } }, + // Canonical `worktree:` form, which needs unwrapping before the repo id is visible. + { + label: 'activeWorkspaceKey', + session: { activeWorkspaceKey: worktreeWorkspaceKey(GONE_WORKTREE) } + }, + { + label: 'activeWorktreeIdsOnShutdown', + session: { activeWorktreeIdsOnShutdown: [GONE_WORKTREE] } + } + ])("clears $label when it is the orphan repo's only residue", async ({ session }) => { + writeDataFile({ + schemaVersion: 1, + repos: [makeRepo({ id: LIVE_REPO, path: '/workspace/live' })], + worktreeMeta: {}, + workspaceSessionsByHostId: { + [RUNTIME_HOST]: { ...getDefaultWorkspaceSession(), ...session } + } + }) + + const store = await createStore() + store.flush() + + const partition = store.getWorkspaceSession(RUNTIME_HOST) + expect(partition.activeWorktreeId ?? null).toBeNull() + expect(partition.activeWorkspaceKey ?? null).toBeNull() + expect(partition.activeWorktreeIdsOnShutdown ?? []).toEqual([]) + const reloaded = await createStore() + expect(reloaded.sweepDeregisteredRepoResidue()).toEqual([]) + }) + + // Why: a sweep that dirtied every launch would rewrite the profile forever and mask real changes. + it('leaves a profile with no orphans byte-identical across reloads', async () => { + const seed = await createStore() + seed.addRepo(makeRepo({ id: LIVE_REPO, path: '/workspace/live' })) + seed.setWorktreeMetaForHost(LIVE_WORKTREE, 'local', { displayName: 'Live' }) + seed.setWorkspaceSession(sessionFor(LIVE_WORKTREE), 'local') + seed.flush() + + const canonicalizing = await createStore() + canonicalizing.flush() + const canonical = JSON.stringify(readDataFile()) + + const reloaded = await createStore() + reloaded.flush() + + expect(JSON.stringify(readDataFile())).toBe(canonical) + }) +}) diff --git a/src/main/persistence-host-partitioned-sessions.test.ts b/src/main/persistence-host-partitioned-sessions.test.ts index 023737ed386..aa2e46e52fd 100644 --- a/src/main/persistence-host-partitioned-sessions.test.ts +++ b/src/main/persistence-host-partitioned-sessions.test.ts @@ -123,6 +123,9 @@ describe('Store host-partitioned workspace sessions', () => { } }) + // Registered on purpose: rows owned by an unregistered repo id are swept as orphans on load. + const makeRepos = (...repoIds: string[]) => repoIds.map((id) => makeRepo({ id, path: `/${id}` })) + it('migrates a legacy workspaceSession blob into the local partition', async () => { writeDataFile({ schemaVersion: 1, @@ -194,6 +197,7 @@ describe('Store host-partitioned workspace sessions', () => { writeDataFile({ schemaVersion: 1, workspaceSession: makeHostSession('local-repo'), + repos: makeRepos('repo-ssh'), workspaceSessionsByHostId: { 'ssh:ssh-1': makeLegacyPaneHostSession('repo-ssh', 'remote-pty') }, @@ -224,6 +228,7 @@ describe('Store host-partitioned workspace sessions', () => { writeDataFile({ schemaVersion: 1, workspaceSession: makeHostSession('local-repo'), + repos: makeRepos('repo-a', 'repo-b'), workspaceSessionsByHostId: { 'ssh:host-a': makeLegacyPaneHostSession('repo-a', 'pty-a'), 'ssh:host-b': makeLegacyPaneHostSession('repo-b', 'pty-b') @@ -488,6 +493,7 @@ describe('Store host-partitioned workspace sessions', () => { it('removes one orphaned worktree with a host-scoped topology fence', async () => { const store = await createStore() + store.addRepo(makeRepo({ id: 'repo-gone', path: '/repo-gone' })) const worktreeId = 'repo-gone::/workspace/stale' const session = { ...makeHostSession('repo-gone'), @@ -728,6 +734,7 @@ describe('Store host-partitioned workspace sessions', () => { const worktreeId = 'repo-1::/worktree' writeDataFile({ schemaVersion: 1, + repos: makeRepos('repo-1'), workspaceSessionsByHostId: { 'runtime:good': makeHostSession('good-repo'), // activeRepoId must be string|null; a number fails the zod parse. @@ -753,6 +760,7 @@ describe('Store host-partitioned workspace sessions', () => { const worktreeId = 'repo-1::/worktree' writeDataFile({ schemaVersion: 1, + repos: makeRepos('repo-1'), workspaceSession: { ...makeHostSession('local-repo'), // A projected/truncated write can leave a top-level field the wrong type; @@ -813,6 +821,7 @@ describe('Store host-partitioned workspace sessions', () => { const worktreeId = 'repo-1::/worktree' const profile = await canonicalize({ schemaVersion: 1, + repos: makeRepos('repo-1'), workspaceSession: { ...makeHostSession('local-repo'), tabsByWorktree: { [worktreeId]: [makeTerminalTab({ id: 'tab-keep', worktreeId })] } @@ -845,6 +854,7 @@ describe('Store host-partitioned workspace sessions', () => { const worktreeId = 'repo-1::/worktree' const profile = await canonicalize({ schemaVersion: 1, + repos: makeRepos('repo-1'), workspaceSessionsByHostId: { 'runtime:env-a': { ...makeHostSession('runtime-repo'), diff --git a/src/main/persistence-initial-load.test.ts b/src/main/persistence-initial-load.test.ts index 6d0c0f11d5e..934ab44e1cb 100644 --- a/src/main/persistence-initial-load.test.ts +++ b/src/main/persistence-initial-load.test.ts @@ -150,6 +150,8 @@ describe('Store', () => { it('does not restore a terminal tab after its durable close flush returns', async () => { const store = await createStore() + // Registered on purpose: rows owned by an unregistered repo id are swept as orphans on load. + store.addRepo(makeRepo({ id: 'repo-1', path: '/repo-1' })) const worktreeId = 'repo-1::/tmp/worktree-1' const tabId = 'terminal-1' const session: WorkspaceSessionState = { diff --git a/src/main/persistence-native-chat-tab-view-mode.test.ts b/src/main/persistence-native-chat-tab-view-mode.test.ts index 3e3544c7495..9bcaab15494 100644 --- a/src/main/persistence-native-chat-tab-view-mode.test.ts +++ b/src/main/persistence-native-chat-tab-view-mode.test.ts @@ -58,7 +58,7 @@ describe('Store native-chat tab viewMode persistence', () => { const WORKTREE = 'repo1::/worktree' writeDataFile({ schemaVersion: 1, - repos: [makeRepo()], + repos: [makeRepo({ id: 'repo1', path: '/repo1' })], worktreeMeta: {}, settings: {}, ui: {}, diff --git a/src/main/persistence-repo-lifecycle.test.ts b/src/main/persistence-repo-lifecycle.test.ts index 1653f78297e..66f3fc2c32e 100644 --- a/src/main/persistence-repo-lifecycle.test.ts +++ b/src/main/persistence-repo-lifecycle.test.ts @@ -737,7 +737,10 @@ describe('Store', () => { it('reassignSshTargetId persists a worktree-meta-only re-point (no matching repo)', async () => { const store = await createStore() - // A meta on the old SSH host with no repo row — the re-point must still be persisted, not memory-only. + // A meta on the old SSH host with no repo row for that host — the re-point must still be + // persisted, not memory-only. The repo id stays registered so the load-time orphan sweep, + // which only reads repo ids, leaves the row alone. + store.addRepo(makeRepo({ id: 'r1', path: '/r1' })) store.setWorktreeMeta('r1::/remote/wt', { displayName: 'wt', hostId: 'ssh:ssh-old' }) const repoIds = store.reassignSshTargetId('ssh-old', 'ssh-new') @@ -787,6 +790,7 @@ describe('Store', () => { it('reassignSshTargetId re-keys a session partition stored under the old ssh host id', async () => { const store = await createStore() + store.addRepo(makeRepo({ id: 'r1', path: '/r1' })) store.setWorkspaceSession( { activeRepoId: null, diff --git a/src/main/persistence-settings-update.test.ts b/src/main/persistence-settings-update.test.ts index dc3a3c7ebb5..9af63ca7ccd 100644 --- a/src/main/persistence-settings-update.test.ts +++ b/src/main/persistence-settings-update.test.ts @@ -708,7 +708,7 @@ describe('Store', () => { } writeDataFile({ schemaVersion: 1, - repos: [makeRepo()], + repos: [makeRepo({ id: 'repo1', path: '/repo1' })], worktreeMeta: { 'repo1::/worktree-a': { status: 'active' }, 'repo1::/worktree-b': { status: 'active' } diff --git a/src/main/persistence-ssh-targets-and-pane-keys.test.ts b/src/main/persistence-ssh-targets-and-pane-keys.test.ts index 6c186ade077..c11ecbf0bc2 100644 --- a/src/main/persistence-ssh-targets-and-pane-keys.test.ts +++ b/src/main/persistence-ssh-targets-and-pane-keys.test.ts @@ -346,7 +346,7 @@ describe('Store', () => { const acknowledgedAt = 1_700_000_000_000 writeDataFile({ schemaVersion: 1, - repos: [makeRepo()], + repos: [makeRepo({ id: 'repo1', path: '/repo1' })], worktreeMeta: {}, settings: {}, ui: { @@ -408,7 +408,7 @@ describe('Store', () => { writeDataFile({ schemaVersion: 1, - repos: [makeRepo()], + repos: [makeRepo({ id: 'repo1', path: '/repo1' })], worktreeMeta: {}, settings: {}, ui: { diff --git a/src/main/persistence-worktree-lineage-and-backups.test.ts b/src/main/persistence-worktree-lineage-and-backups.test.ts index ef5e83745be..372e8b0e33b 100644 --- a/src/main/persistence-worktree-lineage-and-backups.test.ts +++ b/src/main/persistence-worktree-lineage-and-backups.test.ts @@ -166,6 +166,8 @@ describe('Store', () => { describe('mobileClientTabSelectionsByDeviceId', () => { it('persists device tab selections across reloads and drops malformed payloads', async () => { const store = await createStore() + // Registered on purpose: rows owned by an unregistered repo id are swept as orphans on load. + store.addRepo(makeRepo({ id: 'repo-1', path: '/repo-1' })) store.setMobileClientTabSelections({ 'device-a': { 'repo-1::/tmp/wt': { activeTabId: 'tab-1', activeGroupId: 'g1', activeTabIdByGroupId: {} } @@ -188,6 +190,7 @@ describe('Store', () => { it('prunes selections for a removed repo worktree', async () => { const store = await createStore() store.addRepo(makeRepo()) + store.addRepo(makeRepo({ id: 'other-repo', path: '/other-repo' })) store.setMobileClientTabSelections({ 'device-a': { 'r1::/tmp/wt': { diff --git a/src/main/persistence/loading-store/repo-lifecycle-operations.ts b/src/main/persistence/loading-store/repo-lifecycle-operations.ts index 095090ee1ca..c7bdbd9de37 100644 --- a/src/main/persistence/loading-store/repo-lifecycle-operations.ts +++ b/src/main/persistence/loading-store/repo-lifecycle-operations.ts @@ -12,10 +12,13 @@ import { import { mergeProjectHostSetupCompatibilityState } from '../tracking-repos/project-host-compatibility' import { RepoOrderPersistenceOperations } from '../tracking-repos/repo-order-operations' import { pruneWorktreeStateForRepo as pruneWorktreeStateForRepoOperation } from '../tracking-repos/repo-worktree-pruning' +import { collectDeregisteredRepoIds } from '../tracking-repos/deregistered-repo-residue' import { hydrateRepo as hydrateRepoOperation } from '../tracking-repos/repo-hydration' import { RepoUpdatePersistenceOperations } from '../tracking-repos/repo-update-operations' import { ProjectHostSetupPersistenceOperations } from '../tracking-repos/project-host-setup-update' import { bumpLocalWorktreeScanGeneration } from '../../local-worktree-scan-generation' +import type { PersistedState } from '../../../shared/persisted-state-types' +import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id' import type { StoreRuntimeState } from './store-runtime-state' import type { WriteSchedulingOperations } from './write-scheduling' @@ -129,6 +132,33 @@ export class RepoLifecycleOperations { scheduleSave(this[repoLifecycleOperationsContext].scheduling) } + /** + * Drop every persisted row owned by a repo id that is no longer registered. + * + * Runs at load because no removal path can: `removeProject` only fires while the repo is still in + * `state.repos`, and a paired client's mirror of a remote host's rows is keyed by ids that client + * never registers, so the owning host's removal never reaches it (#17776). An orphan has no owner + * that could object, so this ignores the session-ownership and local-execution-host gates the + * missing-directory sweeper needs. + */ + sweepDeregisteredRepoResidue(): string[] { + const state = this[repoLifecycleOperationsContext].runtime.state + const orphanRepoIds = collectDeregisteredRepoIds(state) + if (orphanRepoIds.size === 0) { + return [] + } + for (const repoId of orphanRepoIds) { + pruneWorktreeStateForRepo(this, repoId, null) + state.workspaceSession = removeRepoFromWorkspaceSession(state.workspaceSession, repoId) + state.workspaceSessionsByHostId = removeRepoFromHostWorkspaceSessions( + state.workspaceSessionsByHostId, + repoId + ) + } + pruneDeregisteredRepoUiResidue(state.ui, orphanRepoIds) + return [...orphanRepoIds] + } + updateRepo( id: string, updates: Partial< @@ -212,6 +242,26 @@ export function pruneMobileClientTabSelections( } } +function pruneDeregisteredRepoUiResidue( + ui: PersistedState['ui'], + orphanRepoIds: ReadonlySet +): void { + const isOrphanWorktree = (worktreeId: string): boolean => + orphanRepoIds.has(getRepoIdFromWorktreeId(worktreeId)) + if (ui.lastActiveRepoId && orphanRepoIds.has(ui.lastActiveRepoId)) { + ui.lastActiveRepoId = null + } + if (ui.lastActiveWorktreeId && isOrphanWorktree(ui.lastActiveWorktreeId)) { + ui.lastActiveWorktreeId = null + } + ui.filterRepoIds = ui.filterRepoIds?.filter((repoId) => !orphanRepoIds.has(repoId)) ?? [] + for (const worktreeId of Object.keys(ui.showDotfilesByWorktree ?? {})) { + if (isOrphanWorktree(worktreeId)) { + delete ui.showDotfilesByWorktree?.[worktreeId] + } + } +} + export function getRepoUpdateOperations( owner: RepoLifecycleOperations ): RepoUpdatePersistenceOperations { diff --git a/src/main/persistence/loading-store/store.ts b/src/main/persistence/loading-store/store.ts index 888c0092ed2..017583e8f86 100644 --- a/src/main/persistence/loading-store/store.ts +++ b/src/main/persistence/loading-store/store.ts @@ -64,6 +64,9 @@ export class Store { ) const adaptedProjectGroups = this.domains.adaptation.adaptFlatFolderScanProjectGroups() this.domains.adaptation.hydrateFolderWorkspaceDiffComments() + // Load is the only place an orphaned repo id can be swept: every removal path needs the repo to + // still be registered, so rows outlive their owner without one (#17776). + const sweptRepoIds = this.domains.repos.sweepDeregisteredRepoResidue() for (const entry of normalized.migrationUnsupportedEntries) { setMigrationUnsupportedPty(entry) } @@ -78,7 +81,12 @@ export class Store { this.state.legacyPaneKeyAliasEntries = entries scheduleSave(this.domains.scheduling) }) - if (normalized.changed || this.runtime.loadNeedsSave || adaptedProjectGroups) { + if ( + normalized.changed || + this.runtime.loadNeedsSave || + adaptedProjectGroups || + sweptRepoIds.length > 0 + ) { scheduleSave(this.domains.scheduling) } } diff --git a/src/main/persistence/tracking-repos/deregistered-repo-residue.ts b/src/main/persistence/tracking-repos/deregistered-repo-residue.ts new file mode 100644 index 00000000000..c52bddbb712 --- /dev/null +++ b/src/main/persistence/tracking-repos/deregistered-repo-residue.ts @@ -0,0 +1,108 @@ +import type { PersistedState } from '../../../shared/persisted-state-types' +import { getWorktreeIdFromHostIdentity } from '../../../shared/worktree/host-qualified-identity' +import { splitWorktreeId } from '../../../shared/worktree/id' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { SESSION_FIELDS_PRUNED_BY_OWNER_KEY } from '../../orca-profiles/profile-project-session-field-disposition' +import { ownerKeyWorktreeIds } from '../../orca-profiles/profile-project-worktree-identity' + +/** + * Repo ids that still own persisted rows but no longer appear in `state.repos`. + * + * Why nothing else finds them: every other sweeper is gated on the repo still being registered, so + * deregistering a project stranded the rows it owned permanently — including a paired client's + * mirror of a remote host's session partition, which no local repo removal can reach (#17776). + */ +export function collectDeregisteredRepoIds(state: PersistedState): Set { + const liveRepoIds = new Set(state.repos.map((repo) => repo.id)) + const orphanRepoIds = new Set() + // Only a full `::` locator seeds the set. A bare key -- a folder workspace id, a + // repo-keyed topology revision, a test-shaped locator -- cannot be told apart from a repo id, and + // guessing wrong here deletes live session state. + const addWorktreeId = (worktreeId: string | null | undefined): void => { + const repoId = worktreeId ? splitWorktreeId(worktreeId)?.repoId : undefined + if (repoId && !liveRepoIds.has(repoId)) { + orphanRepoIds.add(repoId) + } + } + /** + * Seed from an owner key, which can read as two different locators (see `ownerKeyWorktreeIds`). + * All or nothing: if either reading names a live repo the key is that repo's, and seeding the + * other reading would hand the removal pass -- which accepts either -- a live row to delete. + */ + const addOwnerKey = (ownerKey: string): void => { + const repoIds = ownerKeyWorktreeIds(ownerKey).flatMap((worktreeId) => { + const repoId = splitWorktreeId(worktreeId)?.repoId + return repoId ? [repoId] : [] + }) + if (repoIds.length > 0 && repoIds.every((repoId) => !liveRepoIds.has(repoId))) { + for (const repoId of repoIds) { + orphanRepoIds.add(repoId) + } + } + } + + // Deliberately not seeded from `sparsePresetsByRepo` or `retiredWorktreeNamesByRepo`: both are + // bounded, and dropping a retired-name row would let a re-added repo reissue a name onto a cwd + // that still holds a prior occupant's agent state. + for (const worktreeId of Object.keys(state.worktreeMeta)) { + addWorktreeId(worktreeId) + } + for (const alias of Object.keys(state.worktreeIdentityAliases ?? {})) { + addWorktreeId(getWorktreeIdFromHostIdentity(alias)) + } + for (const [childId, lineage] of Object.entries(state.worktreeLineageById)) { + addWorktreeId(childId) + addWorktreeId(lineage.parentWorktreeId) + } + for (const [childKey, lineage] of Object.entries(state.workspaceLineageByChildKey)) { + addOwnerKey(childKey) + addOwnerKey(lineage.parentWorkspaceKey) + } + for (const selections of Object.values(state.mobileClientTabSelectionsByDeviceId ?? {})) { + for (const worktreeId of Object.keys(selections)) { + addWorktreeId(worktreeId) + } + } + const sessions: (WorkspaceSessionState | undefined)[] = [ + state.workspaceSession, + ...Object.values(state.workspaceSessionsByHostId ?? {}) + ] + for (const session of sessions) { + if (!session) { + continue + } + for (const field of SESSION_FIELDS_PRUNED_BY_OWNER_KEY) { + for (const ownerKey of Object.keys( + (session[field] as Record | undefined) ?? {} + )) { + addOwnerKey(ownerKey) + } + } + for (const ownerKey of Object.keys(session.tabsByWorktree ?? {})) { + addOwnerKey(ownerKey) + } + for (const ownerKey of Object.keys(session.browserTabsByWorktree ?? {})) { + addOwnerKey(ownerKey) + } + // Pruned by bespoke rules rather than by owner key, so the loop above never reaches them. + for (const ownerKey of [ + session.activeWorktreeId, + session.activeWorkspaceKey, + ...(session.activeWorktreeIdsOnShutdown ?? []) + ]) { + if (ownerKey) { + addOwnerKey(ownerKey) + } + } + // Not seeded from `terminalTopologyRevisionByRepoId`: its keys are bare repo ids by contract, + // and a bare key is exactly what `addWorktreeId` refuses to trust. Rows there are removed once + // any locator seeds their repo id, which every repo that ever opened a terminal has. + for (const record of Object.values(session.sleepingAgentSessionsByPaneKey ?? {})) { + addWorktreeId(record.worktreeId) + } + for (const tombstone of Object.values(session.terminalSurfaceTombstonesByPaneKey ?? {})) { + addWorktreeId(tombstone.worktreeId) + } + } + return orphanRepoIds +} diff --git a/src/main/persistence/tracking-repos/repo-worktree-pruning.ts b/src/main/persistence/tracking-repos/repo-worktree-pruning.ts index f6d17b24aa2..a41f7c6319d 100644 --- a/src/main/persistence/tracking-repos/repo-worktree-pruning.ts +++ b/src/main/persistence/tracking-repos/repo-worktree-pruning.ts @@ -2,6 +2,7 @@ import type { WorkspaceKey } from '../../../shared/folder-workspace-types' import { LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../../../shared/execution-host' import { parseWorkspaceKey } from '../../../shared/workspace-scope' import type { PersistedState } from '../../../shared/persisted-state-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { removeWorkspaceSessionOwners } from '../restoring-sessions/session-owner-removal' import { getExecutionHostIdFromWorktreeHostIdentity, @@ -58,10 +59,26 @@ export function pruneWorktreeStateForRepo( } } } - collectPrefixedKeys(Object.keys(state.worktreeMeta)) - collectPrefixedKeys(Object.keys(state.workspaceSession?.lastVisitedAtByWorktreeId ?? {})) - for (const session of Object.values(state.workspaceSessionsByHostId ?? {})) { + // Why the pane-keyed records contribute owner keys: they are pruned by the worktreeId they name, + // not by their own key, so a worktree with no meta and no visit row would otherwise keep its + // sleeping agents and tombstones forever -- and keep re-seeding the orphan sweep every load. + const collectScannedRecordOwners = (session: WorkspaceSessionState | undefined): void => { collectPrefixedKeys(Object.keys(session?.lastVisitedAtByWorktreeId ?? {})) + collectPrefixedKeys( + Object.values(session?.sleepingAgentSessionsByPaneKey ?? {}).map( + (record) => record.worktreeId + ) + ) + collectPrefixedKeys( + Object.values(session?.terminalSurfaceTombstonesByPaneKey ?? {}).map( + (tombstone) => tombstone.worktreeId + ) + ) + } + collectPrefixedKeys(Object.keys(state.worktreeMeta)) + collectScannedRecordOwners(state.workspaceSession) + for (const session of Object.values(state.workspaceSessionsByHostId ?? {})) { + collectScannedRecordOwners(session) } for (const key of Object.keys(state.worktreeMeta)) { diff --git a/src/main/runtime/runtime-managed-worktree-metadata-sweep.test.ts b/src/main/runtime/runtime-managed-worktree-metadata-sweep.test.ts new file mode 100644 index 00000000000..6df19517d64 --- /dev/null +++ b/src/main/runtime/runtime-managed-worktree-metadata-sweep.test.ts @@ -0,0 +1,123 @@ +// Why this file exists: the authoritative missing-metadata prune had exactly one caller, +// `ipcMain.handle('worktrees:listAll')`. A headless runtime host has no renderer, so it never swept +// its own repos and their `worktreeMeta` rows grew without bound (#17776). +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { mkdirSync, mkdtempSync, rmSync } from 'node:fs' +import { join } from 'node:path' +import { tmpdir } from 'node:os' +import type { GitWorktreeInfo } from '../../shared/worktree/types' +import type { Repo } from '../../shared/repo-types' +import { testState, createStore, makeRepo } from '../persistence-test-harness' +import type { Store } from '../persistence/loading-store/store' +import { RuntimeManagedWorktreeQueries } from './runtime-managed-worktree-queries' +import type { RuntimeStore } from './runtime-store-contract' + +vi.mock('./ssh/ssh-config-parser', () => ({ + loadUserSshConfig: vi.fn(), + sshConfigHostsToTargets: vi.fn() +})) + +vi.mock('electron', () => ({ + app: { getPath: () => testState.dir }, + safeStorage: { isEncryptionAvailable: () => false } +})) + +vi.mock('./telemetry/client', () => ({ track: vi.fn() })) +vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn().mockReturnValue({}) })) + +const gitWorktree = (path: string): GitWorktreeInfo => ({ + path, + branch: 'main', + head: 'abc1234', + isBare: false, + isMainWorktree: true +}) + +function queries( + store: Store, + repo: Repo, + worktrees: readonly GitWorktreeInfo[], + ok = true +): RuntimeManagedWorktreeQueries { + return new RuntimeManagedWorktreeQueries({ + getStore: () => store as unknown as RuntimeStore, + listResolved: async () => [], + resolveRepo: async () => repo, + selectRepos: () => [repo], + scanRepo: async () => ({ ok, worktrees: [...worktrees] }) + }) +} + +describe('runtime detected-worktree listing sweeps missing local metadata', () => { + let repoPath = '' + + beforeEach(() => { + testState.dir = mkdtempSync(join(tmpdir(), 'orca-runtime-sweep-')) + repoPath = join(testState.dir, 'repo') + mkdirSync(repoPath, { recursive: true }) + }) + + afterEach(() => { + rmSync(testState.dir, { recursive: true, force: true }) + }) + + // Paired with the off-host case below: same fixture, no `connectionId`. + it('drops a metadata row whose directory is gone and the scan does not list', async () => { + const store = createStore() + const repo = makeRepo({ id: 'repo-1', path: repoPath }) + store.addRepo(repo) + const missingId = `${repo.id}::${join(testState.dir, 'deleted-worktree')}` + store.setWorktreeMetaForHost(missingId, 'local', { displayName: 'Gone' }) + expect(store.getWorktreeMeta(missingId)).toBeDefined() + + await queries(store, repo, [gitWorktree(repoPath)]).listDetected(repo) + + expect(store.getWorktreeMeta(missingId)).toBeUndefined() + }) + + it('keeps a row whose directory still exists', async () => { + const store = createStore() + const repo = makeRepo({ id: 'repo-1', path: repoPath }) + store.addRepo(repo) + const livePath = join(testState.dir, 'live-worktree') + mkdirSync(livePath, { recursive: true }) + const liveId = `${repo.id}::${livePath}` + store.setWorktreeMetaForHost(liveId, 'local', { displayName: 'Live' }) + + await queries(store, repo, [gitWorktree(repoPath)]).listDetected(repo) + + expect(store.getWorktreeMeta(liveId)).toBeDefined() + }) + + // A non-authoritative scan is a failed listing, which is no evidence any checkout is gone. + it('keeps every row when the scan is not authoritative', async () => { + const store = createStore() + const repo = makeRepo({ id: 'repo-1', path: repoPath }) + store.addRepo(repo) + const missingId = `${repo.id}::${join(testState.dir, 'deleted-worktree')}` + store.setWorktreeMetaForHost(missingId, 'local', { displayName: 'Gone' }) + + await queries(store, repo, [], false).listDetected(repo) + + expect(store.getWorktreeMeta(missingId)).toBeDefined() + }) + + // The execution host owns this verdict: this host cannot stat a checkout that lives behind an SSH + // connection, so a local miss is not evidence of absence. See docs/reference/ssh-execution-boundary.md. + // + // Deliberately identical to the first case except for `connectionId`, and the row is stamped + // `local` so it is a real prune candidate. That pairing is the proof: the same fixture without a + // connection loses the row, so the connection is the only reason this one keeps it. Removing any + // single gate would not show that -- four independent checks derive from `connectionId` here. + it('never sweeps a repo whose git runs off-host', async () => { + const store = createStore() + const repo = makeRepo({ id: 'repo-1', path: repoPath, connectionId: 'build-box' }) + store.addRepo(repo) + const missingId = `${repo.id}::${join(testState.dir, 'deleted-worktree')}` + store.setWorktreeMetaForHost(missingId, 'local', { displayName: 'Gone' }) + + await queries(store, repo, [gitWorktree(repoPath)]).listDetected(repo) + + expect(store.getWorktreeMeta(missingId)).toBeDefined() + }) +}) diff --git a/src/main/runtime/runtime-managed-worktree-queries.ts b/src/main/runtime/runtime-managed-worktree-queries.ts index d444f024e84..b0ed2bc4a3b 100644 --- a/src/main/runtime/runtime-managed-worktree-queries.ts +++ b/src/main/runtime/runtime-managed-worktree-queries.ts @@ -20,6 +20,10 @@ import { } from '../../shared/worktree/visibility-sources' import { mergeWorktree } from '../ipc/worktree-logic' import { pruneLineageForMissingRepoWorktrees } from '../worktree-lineage-pruning' +import { pruneMetadataMissingFromAuthoritativeLocalScan } from '../ipc/worktrees/listing/authoritative-local-worktree-metadata-pruning' +import type { NativeLocalWorktreeMetadataScanExpectation } from '../persistence/tracking-repos/missing-local-worktree-metadata-pruning' +import { getLocalWorktreeScanGeneration } from '../local-worktree-scan-generation' +import { getLocalProjectWorktreeGitOptions } from '../project-runtime-git-options' import type { Store } from '../persistence' import type { RuntimeStore } from './runtime-store-contract' import type { RuntimeWorktreeScanResult } from './repo-worktree-resolution-scan' @@ -36,6 +40,31 @@ type Dependencies = { scanRepo(repo: Repo): Promise } +/** + * The destructive scan expectation for one repo, or undefined when this repo must not carry one. + * + * WSL-routed repos are excluded for the same reason the desktop listing excludes them: the listing + * runs in the distro and reports Linux paths while metadata can hold UNC ones, and v1 cannot prove + * those aliases equivalent. A runtime that needs repair throws rather than resolving routing, which + * is likewise no basis for deleting rows. + */ +function captureLocalMetadataPruneExpectation( + store: RuntimeStore, + repo: Repo +): NativeLocalWorktreeMetadataScanExpectation | undefined { + if (typeof store.captureNativeLocalWorktreeMetadataScanExpectation !== 'function') { + return undefined + } + try { + if (getLocalProjectWorktreeGitOptions(store as unknown as Store, repo).wslDistro) { + return undefined + } + } catch { + return undefined + } + return store.captureNativeLocalWorktreeMetadataScanExpectation(repo) +} + export class RuntimeManagedWorktreeQueries { constructor(private readonly deps: Dependencies) {} @@ -129,6 +158,10 @@ export class RuntimeManagedWorktreeQueries { worktrees: projectResolvedWorktreeLineage(detected, store.getAllWorktreeLineage?.() ?? {}) } } + // Why capture before the scan: listing can mutate metadata synchronously before its first + // await, and the prune revalidates against the rows as they stood when the scan was issued. + const metadataScanGeneration = getLocalWorktreeScanGeneration(repo.id) + const metadataPruneExpectation = captureLocalMetadataPruneExpectation(store, repo) let scan: RuntimeWorktreeScanResult try { scan = await this.deps.scanRepo(repo) @@ -136,6 +169,17 @@ export class RuntimeManagedWorktreeQueries { scan = { ok: false, worktrees: [] } } if (scan.ok) { + // Why the runtime sweeps too: the desktop listing that used to own this runs off `ipcMain`, + // so a headless host -- which has no renderer -- never pruned its own repos' rows (#17776). + if (metadataPruneExpectation) { + await pruneMetadataMissingFromAuthoritativeLocalScan({ + store: store as unknown as Store, + repo, + gitWorktrees: scan.worktrees, + scan: metadataPruneExpectation, + scanGeneration: metadataScanGeneration + }) + } pruneLineageForMissingRepoWorktrees(store as unknown as Store, repo, scan.worktrees) } const matcher = createWorktreeVisibilitySourceMatcher( diff --git a/src/main/runtime/runtime-store-contract.ts b/src/main/runtime/runtime-store-contract.ts index 692826b3c29..e160e039533 100644 --- a/src/main/runtime/runtime-store-contract.ts +++ b/src/main/runtime/runtime-store-contract.ts @@ -30,6 +30,9 @@ export type RuntimeStore = { removeProjectForHost?: Store['removeProjectForHost'] reorderRepos?: Store['reorderRepos'] getAllWorktreeMeta: Store['getAllWorktreeMeta'] + captureNativeLocalWorktreeMetadataScanExpectation?: Store['captureNativeLocalWorktreeMetadataScanExpectation'] + pruneSessionlessMissingLocalWorktreeMetadataForRepo?: Store['pruneSessionlessMissingLocalWorktreeMetadataForRepo'] + getProfileStorageDirectory?: Store['getProfileStorageDirectory'] getWorktreeMeta: Store['getWorktreeMeta'] setWorktreeMeta: Store['setWorktreeMeta'] setWorktreeMetaForHost?: Store['setWorktreeMetaForHost'] diff --git a/src/main/ssh-reattach-pane-cardinality.test.ts b/src/main/ssh-reattach-pane-cardinality.test.ts index 51cbdb26794..96362ffbfc2 100644 --- a/src/main/ssh-reattach-pane-cardinality.test.ts +++ b/src/main/ssh-reattach-pane-cardinality.test.ts @@ -2,7 +2,13 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { rmSync, mkdtempSync } from 'node:fs' import { join } from 'node:path' import { tmpdir } from 'node:os' -import { testState, createStore, makeTerminalTab, writeDataFile } from './persistence-test-harness' +import { + testState, + createStore, + makeRepo, + makeTerminalTab, + writeDataFile +} from './persistence-test-harness' import { TEST_LEAF_1, TEST_LEAF_2 } from './persistence-session-fixtures' import { getDefaultPersistedState } from '../shared/constants' @@ -196,6 +202,8 @@ describe('STA-3077: an SSH reattach binds panes without grafting them back', () it('does not clear and rebind a retired surface loaded from an older profile', async () => { const paneKey = `${TAB}:${TEST_LEAF_1}` const persisted = getDefaultPersistedState(testState.dir) + // Registered on purpose: rows owned by an unregistered repo id are swept as orphans on load. + persisted.repos = [makeRepo({ id: 'repo1', path: '/repo1' })] persisted.workspaceSession = { ...persisted.workspaceSession, ...sessionWithPane({ tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-1' }), diff --git a/src/main/startup/main-process-ipc-bootstrap.ts b/src/main/startup/main-process-ipc-bootstrap.ts index 3f364a9335c..89be84d2119 100644 --- a/src/main/startup/main-process-ipc-bootstrap.ts +++ b/src/main/startup/main-process-ipc-bootstrap.ts @@ -2,6 +2,7 @@ import { ipcMain } from 'electron' import { recoverLegacyWorkerTerminalsForRendererStartup } from './legacy-worker-renderer-recovery' import { logStartupMilestone } from './startup-diagnostics' import { mainProcessState as state } from './main-process-state' +import { resolveOpenedMarkdownDocuments } from './os-opened-markdown-files' export function registerMainProcessIpcHandlers(): void { ipcMain.handle('app:awaitFirstWindowStartupServices', async () => { @@ -36,6 +37,20 @@ export function registerMainProcessIpcHandlers(): void { state.pendingOpenSettings.matches(event.sender.id, { consume: true }) ) ipcMain.handle('ui:consumePendingSkillShare', () => state.skillShareDeepLinks.consume()) + // Why: the renderer pulls this once its ui:openMarkdownFiles listener attaches, so a + // cold-start "Open With" queued before mount still opens. The pull doubles as the proof + // that the listener is live, which is what lets main start pushing. + ipcMain.handle('ui:consumePendingMarkdownFileOpens', async () => { + state.markdownFileOpenListenerReady = true + const filePaths = state.osOpenedMarkdownFiles.consume() + try { + return await resolveOpenedMarkdownDocuments(filePaths) + } catch (error) { + // Why restored: the renderer never received these, so a later mount must still get them. + state.osOpenedMarkdownFiles.restore(filePaths) + throw error + } + }) ipcMain.handle( 'app:startupDiagnostic', (_event, event: string, details?: Record) => { diff --git a/src/main/startup/main-process-state.ts b/src/main/startup/main-process-state.ts index ea1e0a33299..05c45d5b567 100644 --- a/src/main/startup/main-process-state.ts +++ b/src/main/startup/main-process-state.ts @@ -36,6 +36,7 @@ import type { ServeOptions } from './main-process-serve' import type { HangDetectionMarker } from '../hang-watchdog/hang-detection-marker' import { ServeReadinessPublisher } from '../server/serve-readiness' import { SkillShareDeepLinkState } from './skill-share-deep-link-state' +import { OsOpenedMarkdownFileState } from './os-opened-markdown-files' import { DEFAULT_GPU_CRASH_FALLBACK_THRESHOLD, DEFAULT_GPU_CRASH_FALLBACK_WINDOW_MS, @@ -90,6 +91,12 @@ export const mainProcessState = { // Why: a tray "Settings…" click can precede the renderer's ui:openSettings listener; it pulls this one-shot on mount. pendingOpenSettings: createWebContentsTimedFlag(), skillShareDeepLinks: new SkillShareDeepLinkState(), + // Why: a Finder/Explorer "Open With" can land before any window exists; the renderer pulls this buffer on mount. + osOpenedMarkdownFiles: new OsOpenedMarkdownFileState(), + // Why a latch and not just "a window exists": a window can be up while its renderer has not + // attached the ui:openMarkdownFiles listener yet, and a push into that gap is dropped by + // Electron with no error. Only the renderer's own pull proves the listener is live. + markdownFileOpenListenerReady: false, firstWindowStartupServicesReady: Promise.resolve(), managedWslCliReconciliationReady: Promise.resolve(), managedWslCliStartupBarrierReady: Promise.resolve(), diff --git a/src/main/startup/main-window-controller.ts b/src/main/startup/main-window-controller.ts index 57763b23ab1..0d935d5f84a 100644 --- a/src/main/startup/main-window-controller.ts +++ b/src/main/startup/main-window-controller.ts @@ -145,6 +145,9 @@ export function openMainWindow(options: { revealOnDidFinishLoad?: boolean } = {} clearExpectedRendererReload(rendererWebContentsId) recordCrashBreadcrumb('main_window_loaded') logStartupMilestone('did-finish-load') + // Why cleared here: a reload drops the old ui:openMarkdownFiles listener, and the fresh + // renderer re-attaches by pulling. Pushing into the gap between would be silently lost. + state.markdownFileOpenListenerReady = false const currentStore = state.store if (currentStore && resolveConsent(currentStore.getSettings()).effective === 'enabled') { trackAppOpenedOnce() diff --git a/src/main/startup/os-opened-markdown-delivery.test.ts b/src/main/startup/os-opened-markdown-delivery.test.ts new file mode 100644 index 00000000000..0647516e3fa --- /dev/null +++ b/src/main/startup/os-opened-markdown-delivery.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it, vi } from 'vitest' +import { OsOpenedMarkdownFileState } from './os-opened-markdown-files' + +/** + * The two ways a queued "Open With" can be lost between main and the renderer. Both are + * about ownership: main must not drop paths it has not proven the renderer received. + */ +describe('os-opened markdown delivery ownership', () => { + it('keeps the batch when resolution rejects on the pull path', async () => { + const state = new OsOpenedMarkdownFileState() + state.captureFilePaths(['/notes/a.md']) + const resolve = vi.fn().mockRejectedValue(new Error('floating root unavailable')) + + // Mirrors the ipcMain.handle('ui:consumePendingMarkdownFileOpens') body. + const pull = async (): Promise => { + const filePaths = state.consume() + try { + return await resolve(filePaths) + } catch (error) { + state.restore(filePaths) + throw error + } + } + + await expect(pull()).rejects.toThrow('floating root unavailable') + // Without the restore the file would be gone and no later mount could ever open it. + expect(state.consume()).toEqual(['/notes/a.md']) + }) + + it('holds the batch while the renderer listener is not yet attached', () => { + const state = new OsOpenedMarkdownFileState() + const send = vi.fn() + let listenerReady = false + + // Mirrors publishOsOpenedMarkdownFiles()'s guard. + const publish = (): void => { + if (!listenerReady) { + return + } + const filePaths = state.consume() + if (filePaths.length > 0) { + send(filePaths) + } + } + + // A window exists, but the renderer has not mounted its bridge yet: send() here would be + // dropped by Electron with no error, and consuming would destroy the queue. + state.captureFilePaths(['/notes/a.md'], publish) + expect(send).not.toHaveBeenCalled() + + // The renderer's pull is what proves the listener is live. + listenerReady = true + state.captureFilePaths(['/notes/b.md'], publish) + expect(send).toHaveBeenCalledExactlyOnceWith(['/notes/a.md', '/notes/b.md']) + expect(state.consume()).toEqual([]) + }) + + it('restores a batch the window could no longer receive', () => { + const state = new OsOpenedMarkdownFileState() + state.captureFilePaths(['/notes/a.md']) + const filePaths = state.consume() + + // Window died between consume and send. + state.restore(filePaths) + + expect(state.consume()).toEqual(['/notes/a.md']) + }) +}) diff --git a/src/main/startup/os-opened-markdown-files.test.ts b/src/main/startup/os-opened-markdown-files.test.ts new file mode 100644 index 00000000000..f174e10d834 --- /dev/null +++ b/src/main/startup/os-opened-markdown-files.test.ts @@ -0,0 +1,306 @@ +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join, resolve, sep } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { isMarkdownDocumentName } from '../ipc/markdown-documents' +import { + MAX_PENDING_OS_OPENED_MARKDOWN_FILES, + OsOpenedMarkdownFileState, + markdownPathsFromArguments, + resolveOpenedMarkdownDocuments +} from './os-opened-markdown-files' + +vi.mock('../ipc/filesystem-auth', () => ({ + authorizeExternalPath: vi.fn() +})) +vi.mock('../ipc/floating-workspace-directory', () => ({ + ensureDefaultFloatingWorkspacePath: vi.fn() +})) + +const { authorizeExternalPath } = await import('../ipc/filesystem-auth') +const { ensureDefaultFloatingWorkspacePath } = await import('../ipc/floating-workspace-directory') + +describe('markdownPathsFromArguments', () => { + it('keeps absolute markdown paths and drops other extensions', () => { + expect( + markdownPathsFromArguments( + [ + '/Users/dev/notes/a.md', + '/Users/dev/notes/b.markdown', + '/Users/dev/notes/c.mdx', + '/Users/dev/notes/d.txt', + '/Users/dev/src/e.tsx', + '/Users/dev/notes/README' + ], + 'darwin' + ) + ).toEqual(['/Users/dev/notes/a.md', '/Users/dev/notes/b.markdown', '/Users/dev/notes/c.mdx']) + }) + + it('drops switches, including Chromium-style ones that would otherwise look like values', () => { + expect( + markdownPathsFromArguments( + ['--serve', '-v', '--allow-file-access-from-files', '/Users/dev/notes/a.md'], + 'darwin' + ) + ).toEqual(['/Users/dev/notes/a.md']) + }) + + it('drops the executable and dev entries because none of them end in a markdown extension', () => { + const nonDocumentEntries = [ + '/Applications/Orca.app/Contents/MacOS/Orca', + '/Users/dev/orca/out/main/index.js', + '/Applications/Orca.app/Contents/Resources/app.asar' + ] + // The module documents that the extension check alone excludes these; hold it to that. + for (const entry of nonDocumentEntries) { + expect(isMarkdownDocumentName(entry), entry).toBe(false) + } + expect( + markdownPathsFromArguments([...nonDocumentEntries, '/Users/dev/notes/a.md'], 'darwin') + ).toEqual(['/Users/dev/notes/a.md']) + }) + + it('drops relative paths because a second instance has no meaningful cwd', () => { + expect( + markdownPathsFromArguments(['readme.md', './docs/a.md', '../up.md', ''], 'darwin') + ).toEqual([]) + }) + + it('accepts win32 drive-letter and UNC paths', () => { + expect( + markdownPathsFromArguments( + ['C:\\Users\\dev\\todo.md', '\\\\server\\share\\a.md', 'C:\\Users\\dev\\todo.txt'], + 'win32' + ) + ).toEqual(['C:\\Users\\dev\\todo.md', '\\\\server\\share\\a.md']) + }) + + it('dedupes case-insensitively on win32 and keeps the first spelling', () => { + expect(markdownPathsFromArguments(['C:\\notes\\A.md', 'c:\\notes\\a.md'], 'win32')).toEqual([ + 'C:\\notes\\A.md' + ]) + }) + + it('normalizes parent segments before deduping', () => { + expect( + markdownPathsFromArguments(['C:\\notes\\sub\\..\\a.md', 'C:\\notes\\a.md'], 'win32') + ).toEqual(['C:\\notes\\a.md']) + expect(markdownPathsFromArguments(['/docs/../notes/a.md', '/notes/a.md'], 'darwin')).toEqual([ + '/notes/a.md' + ]) + }) + + it('does not dedupe case-insensitively on posix, where casing is a different file', () => { + expect(markdownPathsFromArguments(['/a/A.md', '/a/a.md'], 'linux')).toEqual([ + '/a/A.md', + '/a/a.md' + ]) + }) + + it('accepts a file:// URI, which the desktop entry %U field code permits', () => { + // Why defensive rather than load-bearing: GLib decodes a local file:// URI to a plain + // path before spawning (measured on Ubuntu 24.04), so Linux hits the plain-path branch + // today. The %U spec still allows a URI, and a launcher that passes one literally would + // otherwise be dropped without a trace. + expect( + markdownPathsFromArguments( + ['file:///home/me/notes/a.md', 'file:///home/me/notes/b.txt'], + 'linux' + ) + ).toEqual(['/home/me/notes/a.md']) + }) + + it('percent-decodes a file:// URI so a path with spaces still opens', () => { + expect(markdownPathsFromArguments(['file:///home/me/design%20notes.md'], 'linux')).toEqual([ + '/home/me/design notes.md' + ]) + }) + + it('decodes win32 file:// URIs, including UNC authority form', () => { + expect( + markdownPathsFromArguments( + ['file:///C:/Users/me/todo.md', 'file://server/share/a.md'], + 'win32' + ) + ).toEqual(['C:\\Users\\me\\todo.md', '\\\\server\\share\\a.md']) + }) + + it('dedupes a path delivered as both a URI and a bare path', () => { + expect(markdownPathsFromArguments(['file:///home/me/a.md', '/home/me/a.md'], 'linux')).toEqual([ + '/home/me/a.md' + ]) + }) + + it('drops a malformed or non-file URL instead of throwing', () => { + expect(() => + markdownPathsFromArguments(['file://', 'file:///%zz.md', 'https://example.com/a.md'], 'linux') + ).not.toThrow() + expect( + markdownPathsFromArguments(['file://', 'file:///%zz.md', 'https://example.com/a.md'], 'linux') + ).toEqual([]) + }) + + it('honours the platform argument rather than the host OS', () => { + const argv = ['C:\\notes\\a.md', '/notes/b.md'] + // Same argv, two platforms: a win32 path is not absolute to posix, and posix input is + // renormalized to backslashes on win32. Neither result may depend on where the suite runs. + expect(markdownPathsFromArguments(argv, 'darwin')).toEqual(['/notes/b.md']) + expect(markdownPathsFromArguments(argv, 'win32')).toEqual(['C:\\notes\\a.md', '\\notes\\b.md']) + }) +}) + +// Why resolve(): the state uses the host platform by default, so fixture paths must already be +// spelled the way the host's path module normalizes them (`\n\a.md` and a drive on Windows). +const hostPath = (name: string): string => resolve(sep, 'notes', name) + +describe('OsOpenedMarkdownFileState', () => { + it('reports no capture and does not publish when argv carries no markdown', () => { + const state = new OsOpenedMarkdownFileState() + const publish = vi.fn() + + expect(state.capture(['/Applications/Orca.app/Contents/MacOS/Orca', '--serve'], publish)).toBe( + false + ) + expect(publish).not.toHaveBeenCalled() + expect(state.consume()).toEqual([]) + }) + + it('buffers and publishes when argv carries markdown', () => { + const state = new OsOpenedMarkdownFileState() + const publish = vi.fn() + const filePath = hostPath('a.md') + + expect(state.capture(['/Applications/Orca.app/Contents/MacOS/Orca', filePath], publish)).toBe( + true + ) + expect(publish).toHaveBeenCalledTimes(1) + expect(state.consume()).toEqual([filePath]) + }) + + it('captures a single macOS open-file path', () => { + const state = new OsOpenedMarkdownFileState() + const publish = vi.fn() + const filePath = hostPath('a.md') + + expect(state.captureFilePaths([filePath], publish)).toBe(true) + expect(state.captureFilePaths([hostPath('a.png')], publish)).toBe(false) + expect(publish).toHaveBeenCalledTimes(1) + expect(state.consume()).toEqual([filePath]) + }) + + it('does not duplicate a path captured twice', () => { + const state = new OsOpenedMarkdownFileState() + const filePath = hostPath('a.md') + + state.captureFilePaths([filePath]) + state.captureFilePaths([filePath]) + state.capture(['orca', filePath]) + + expect(state.consume()).toEqual([filePath]) + }) + + it('drains the buffer on consume', () => { + const state = new OsOpenedMarkdownFileState() + const paths = [hostPath('a.md'), hostPath('b.md')] + state.captureFilePaths(paths) + + expect(state.consume()).toEqual(paths) + expect(state.consume()).toEqual([]) + }) + + it('restores an undelivered batch at the front of the buffer', () => { + const state = new OsOpenedMarkdownFileState() + state.captureFilePaths([hostPath('later.md')]) + + state.restore([hostPath('undelivered.md')]) + + expect(state.consume()).toEqual([hostPath('undelivered.md'), hostPath('later.md')]) + }) + + it('caps the buffer when captures overflow it', () => { + const state = new OsOpenedMarkdownFileState() + const overflow = MAX_PENDING_OS_OPENED_MARKDOWN_FILES + 5 + const paths = Array.from({ length: overflow }, (_, index) => hostPath(`file-${index}.md`)) + + expect(state.captureFilePaths(paths)).toBe(true) + + expect(state.consume()).toEqual(paths.slice(0, MAX_PENDING_OS_OPENED_MARKDOWN_FILES)) + }) + + it('caps the buffer when a restore overflows it', () => { + const state = new OsOpenedMarkdownFileState() + state.captureFilePaths([hostPath('pending.md')]) + const restored = Array.from({ length: MAX_PENDING_OS_OPENED_MARKDOWN_FILES }, (_, index) => + hostPath(`restored-${index}.md`) + ) + + state.restore(restored) + + const pending = state.consume() + expect(pending).toHaveLength(MAX_PENDING_OS_OPENED_MARKDOWN_FILES) + expect(pending).toEqual(restored) + }) +}) + +describe('resolveOpenedMarkdownDocuments', () => { + let floatingRoot: string + let fileRoot: string + + beforeEach(async () => { + vi.mocked(authorizeExternalPath).mockClear() + vi.mocked(ensureDefaultFloatingWorkspacePath).mockClear() + floatingRoot = await mkdtemp(join(tmpdir(), 'orca-os-open-root-')) + fileRoot = await mkdtemp(join(tmpdir(), 'orca-os-open-files-')) + vi.mocked(ensureDefaultFloatingWorkspacePath).mockResolvedValue(floatingRoot) + }) + + afterEach(async () => { + await rm(floatingRoot, { recursive: true, force: true }) + await rm(fileRoot, { recursive: true, force: true }) + }) + + it('resolves a real file outside the floating root to a basename-relative document', async () => { + const filePath = join(fileRoot, 'design notes.md') + await writeFile(filePath, '# hi\n', 'utf8') + + const documents = await resolveOpenedMarkdownDocuments([filePath]) + + expect(documents).toEqual([ + { + filePath, + relativePath: 'design notes.md', + basename: 'design notes.md', + name: 'design notes' + } + ]) + expect(authorizeExternalPath).toHaveBeenCalledWith(filePath) + }) + + it('drops a directory that merely looks like a markdown file', async () => { + const bundlePath = join(fileRoot, 'bundle.md') + await mkdir(bundlePath) + const filePath = join(fileRoot, 'real.md') + await writeFile(filePath, '# hi\n', 'utf8') + + const documents = await resolveOpenedMarkdownDocuments([bundlePath, filePath]) + + expect(documents.map((document) => document.filePath)).toEqual([filePath]) + // Security contract: a path we never validated must never be authorized for renderer reads. + expect(authorizeExternalPath).toHaveBeenCalledTimes(1) + expect(authorizeExternalPath).toHaveBeenCalledWith(filePath) + }) + + it('drops a path that no longer exists without authorizing it', async () => { + const missingPath = join(fileRoot, 'gone.md') + + expect(await resolveOpenedMarkdownDocuments([missingPath])).toEqual([]) + expect(authorizeExternalPath).not.toHaveBeenCalled() + }) + + it('returns nothing for an empty input without touching the filesystem', async () => { + expect(await resolveOpenedMarkdownDocuments([])).toEqual([]) + expect(ensureDefaultFloatingWorkspacePath).not.toHaveBeenCalled() + expect(authorizeExternalPath).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/startup/os-opened-markdown-files.ts b/src/main/startup/os-opened-markdown-files.ts new file mode 100644 index 00000000000..dae27fb7a78 --- /dev/null +++ b/src/main/startup/os-opened-markdown-files.ts @@ -0,0 +1,149 @@ +import { stat } from 'node:fs/promises' +import path from 'node:path' +import { fileURLToPath } from 'node:url' +import type { MarkdownDocument } from '../../shared/filesystem-entry-types' +import { authorizeExternalPath } from '../ipc/filesystem-auth' +import { ensureDefaultFloatingWorkspacePath } from '../ipc/floating-workspace-directory' +import { isMarkdownDocumentName, markdownDocumentFromFilePath } from '../ipc/markdown-documents' + +// Why: a shell can only ever hand over the files the user selected; anything past this is a +// runaway argv, and buffering it unbounded would pin the paths for the whole session. +export const MAX_PENDING_OS_OPENED_MARKDOWN_FILES = 32 + +/** + * Resolves one argv entry to a local absolute path, or null if it is not one. + * + * Why file:// is accepted defensively: electron-builder appends the `%U` field code to the + * generated Linux `Exec=` line, and `%U` is specified as "URLs". GLib turns out to decode a + * local `file://` URI back to a plain path before spawning (measured on Ubuntu 24.04, via + * the same `launch_uris` call a file manager makes), so the branch below is not what fires + * there today — but the spec permits a URI, and a launcher that honours it literally would + * otherwise be silently dropped. macOS `open-file` and the Windows shell `%1` pass paths. + */ +function localPathFromArgument(argument: string, platform: NodeJS.Platform): string | null { + const pathApi = platform === 'win32' ? path.win32 : path.posix + if (argument.startsWith('file://')) { + try { + // Why the explicit windows flag: this must decode the same way on any host so the + // behaviour is testable, and it is what turns `file://server/share` back into a UNC path. + return fileURLToPath(argument, { windows: platform === 'win32' }) + } catch { + return null + } + } + return pathApi.isAbsolute(argument) ? argument : null +} + +/** + * Absolute markdown paths an OS "Open With" put on a launch or second-instance argv. + * + * Why no executable/asar/dev-entry filtering: none of those argv entries end in a markdown + * extension, so the extension check already excludes them. Relative entries are dropped + * because the shell always passes absolute paths and `cwd` is meaningless for a second instance. + */ +export function markdownPathsFromArguments( + argv: readonly string[], + platform: NodeJS.Platform = process.platform +): string[] { + const pathApi = platform === 'win32' ? path.win32 : path.posix + const seen = new Set() + const paths: string[] = [] + for (const rawArgument of argv) { + if (!rawArgument || rawArgument.startsWith('-')) { + continue + } + const argument = localPathFromArgument(rawArgument, platform) + if (!argument || !isMarkdownDocumentName(argument)) { + continue + } + const normalized = pathApi.normalize(argument) + // Why lowercased on win32: the shell round-trips drive letters and 8.3 casing + // inconsistently, and two spellings of one path must not open two tabs. + const key = platform === 'win32' ? normalized.toLowerCase() : normalized + if (seen.has(key)) { + continue + } + seen.add(key) + paths.push(normalized) + } + return paths +} + +/** + * Buffers markdown paths the OS handed us until a renderer can receive them. + * + * Mirrors SkillShareDeepLinkState: main pushes when a window is already live, and the + * renderer pulls the same buffer when its listener attaches, so a cold-start "Open With" + * that lands before mount is not dropped. + */ +export class OsOpenedMarkdownFileState { + private pending: string[] = [] + + /** Returns true when argv carried at least one markdown path. */ + capture(argv: readonly string[], publish?: () => void): boolean { + return this.add(markdownPathsFromArguments(argv), publish) + } + + /** Returns true when at least one path was a markdown document. */ + captureFilePaths(filePaths: readonly string[], publish?: () => void): boolean { + return this.add(markdownPathsFromArguments(filePaths), publish) + } + + consume(): string[] { + const pending = this.pending + this.pending = [] + return pending + } + + /** Puts an undelivered batch back at the front so the next renderer still receives it. */ + restore(filePaths: readonly string[]): void { + this.pending = [...filePaths, ...this.pending].slice(0, MAX_PENDING_OS_OPENED_MARKDOWN_FILES) + } + + private add(filePaths: readonly string[], publish?: () => void): boolean { + if (filePaths.length === 0) { + return false + } + const merged = [...this.pending] + for (const filePath of filePaths) { + if (!merged.includes(filePath)) { + merged.push(filePath) + } + } + this.pending = merged.slice(0, MAX_PENDING_OS_OPENED_MARKDOWN_FILES) + publish?.() + return true + } +} + +/** + * Turns OS-handed paths into the same `MarkdownDocument` shape the floating workspace's own + * file picker produces, authorizing each one for the renderer's later read. + */ +export async function resolveOpenedMarkdownDocuments( + filePaths: readonly string[] +): Promise { + if (filePaths.length === 0) { + return [] + } + const floatingRoot = await ensureDefaultFloatingWorkspacePath() + const documents: MarkdownDocument[] = [] + for (const filePath of filePaths) { + try { + // Why: the shell can hand over a bundle directory named `*.md`, or a path already + // deleted by the time we resolve. Authorize only something that is really a file. + if (!(await stat(filePath)).isFile()) { + continue + } + } catch { + continue + } + authorizeExternalPath(filePath) + documents.push( + markdownDocumentFromFilePath(floatingRoot, filePath, { + outsideRootRelativePath: 'basename' + }) + ) + } + return documents +} diff --git a/src/main/startup/os-opened-markdown-wiring.test.ts b/src/main/startup/os-opened-markdown-wiring.test.ts new file mode 100644 index 00000000000..179ca0820ca --- /dev/null +++ b/src/main/startup/os-opened-markdown-wiring.test.ts @@ -0,0 +1,57 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' + +const read = (relativePath: string): string => + // Why source text: this wiring is module-scope side effects in the entry point, which no + // unit test can import without booting Electron. These guards pin the call shapes instead. + readFileSync(join(process.cwd(), relativePath), 'utf8').replaceAll('"', "'") + +describe('os-opened markdown wiring', () => { + const index = read('src/main/index.ts') + const bootstrap = read('src/main/startup/main-process-ipc-bootstrap.ts') + const controller = read('src/main/startup/main-window-controller.ts') + + it('captures argv before the serve-duplicate early return', () => { + const captureIndex = index.indexOf( + 'state.osOpenedMarkdownFiles.capture(argv, publishOsOpenedMarkdownFiles)' + ) + const serveGuardIndex = index.indexOf('if (!shouldActivateDesktopForSecondInstance(argv)) {') + + expect(captureIndex).toBeGreaterThanOrEqual(0) + expect(serveGuardIndex).toBeGreaterThanOrEqual(0) + // A duplicate `orca serve` returns early; capturing after that would drop the user's files. + expect(captureIndex).toBeLessThan(serveGuardIndex) + }) + + it('claims the macOS open-file event so the default handler does not win it', () => { + const handlerIndex = index.indexOf("app.on('open-file'") + expect(handlerIndex).toBeGreaterThanOrEqual(0) + + const preventDefaultIndex = index.indexOf('event.preventDefault()', handlerIndex) + const nextRegistrationIndex = index.indexOf('app.on(', handlerIndex + 1) + expect(preventDefaultIndex).toBeGreaterThan(handlerIndex) + if (nextRegistrationIndex !== -1) { + expect(preventDefaultIndex).toBeLessThan(nextRegistrationIndex) + } + }) + + it('captures the cold-start argv and lets the renderer pull it after mount', () => { + expect(index).toContain('state.osOpenedMarkdownFiles.capture(process.argv)') + expect(bootstrap).toContain("ipcMain.handle('ui:consumePendingMarkdownFileOpens'") + }) + + // Why: `webContents.send` to a renderer that has not attached the listener is dropped with no + // error, so publishing on "a window exists" alone would consume the queue into a void. + it('only pushes once the renderer has proven its listener is attached', () => { + expect(index).toContain('!state.markdownFileOpenListenerReady') + expect(bootstrap).toContain('state.markdownFileOpenListenerReady = true') + // A reload drops the listener; the fresh renderer re-proves itself by pulling again. + expect(controller).toContain('state.markdownFileOpenListenerReady = false') + }) + + it('restores an undelivered batch on both the push and the pull path', () => { + expect(index).toContain('state.osOpenedMarkdownFiles.restore(filePaths)') + expect(bootstrap).toContain('state.osOpenedMarkdownFiles.restore(filePaths)') + }) +}) diff --git a/src/main/startup/run-electron-vite-dev.test.ts b/src/main/startup/run-electron-vite-dev.test.ts index 2146563373d..73d1bb21cdc 100644 --- a/src/main/startup/run-electron-vite-dev.test.ts +++ b/src/main/startup/run-electron-vite-dev.test.ts @@ -105,6 +105,56 @@ function devWrapperTestEnv(extra: NodeJS.ProcessEnv): NodeJS.ProcessEnv { return { ...env, ...extra } } +/** + * What the two cases below wait on: a ~280MB clone of Electron.app, two swiftc + * helper builds, and `codesign --deep` over the result. Six seconds on an idle + * machine; the swiftc builds alone pass fifteen when this file runs inside the + * full suite and every core is taken. The generous ceiling only costs time on a + * run that is already failing. + */ +const PREPARE_TIMEOUT_MS = 90_000 + +/** + * Spawns the wrapper with its output retained. + * + * Why retained: the wrapper reports its own failures on stderr, and discarding + * them turned a crash in prepare into a bare "Timed out waiting for condition" + * with nothing to act on. + */ +function spawnDevWrapper( + args: string[], + env: NodeJS.ProcessEnv +): { wrapper: ChildProcess; readOutput: () => string } { + const wrapper = spawn(process.execPath, args, { + cwd: resolve('.'), + env, + stdio: ['ignore', 'pipe', 'pipe'] + }) + let output = '' + const collect = (chunk: Buffer): void => { + output += chunk.toString() + } + wrapper.stdout?.on('data', collect) + wrapper.stderr?.on('data', collect) + return { wrapper, readOutput: () => output } +} + +async function waitForEnvFile(envFile: string, readOutput: () => string): Promise { + try { + await waitFor(() => { + try { + return readFileSync(envFile, 'utf8').trim().length > 0 + } catch { + return false + } + }, PREPARE_TIMEOUT_MS) + } catch (error) { + throw new Error( + `${(error as Error).message}: the dev wrapper never wrote ${envFile}. Wrapper output:\n${readOutput() || '(none)'}` + ) + } +} + describe('run-electron-vite-dev', () => { afterEach(async () => { for (const pid of processesToCleanUp) { @@ -351,26 +401,19 @@ describe('run-electron-vite-dev', () => { async function runWrapper(runId: string): Promise<{ electronExecPath: string }> { const pidFile = join(tempDir, `${runId}.pid`) const envFile = join(tempDir, `${runId}.json`) - const wrapper = spawn(process.execPath, [wrapperPath, '--remote-debugging-port=9448'], { - cwd: resolve('.'), - env: { + const { wrapper, readOutput } = spawnDevWrapper( + [wrapperPath, '--remote-debugging-port=9448'], + { ...baseEnv, ORCA_DEV_WRAPPER_TEST_PID_FILE: pidFile, ORCA_DEV_WRAPPER_TEST_ENV_FILE: envFile - }, - stdio: 'ignore' - }) + } + ) expect(wrapper.pid).toBeTypeOf('number') processesToCleanUp.add(wrapper.pid!) - await waitFor(() => { - try { - return readFileSync(envFile, 'utf8').trim().length > 0 - } catch { - return false - } - }, 20000) + await waitForEnvFile(envFile, readOutput) const trackedPids = trackPidFile(pidFile) @@ -409,7 +452,8 @@ describe('run-electron-vite-dev', () => { } } }, - 30000 + // Two full prepares, each budgeted at PREPARE_TIMEOUT_MS. + PREPARE_TIMEOUT_MS * 2 + 30_000 ) it.skipIf(process.platform !== 'darwin')( @@ -421,9 +465,9 @@ describe('run-electron-vite-dev', () => { const wrapperPath = resolve('config/scripts/run-electron-vite-dev.mjs') const fakeCliPath = resolve('src/main/startup/__fixtures__/fake-electron-vite-dev-cli.mjs') - const wrapper = spawn(process.execPath, [wrapperPath, '--remote-debugging-port=9448'], { - cwd: resolve('.'), - env: devWrapperTestEnv({ + const { wrapper, readOutput } = spawnDevWrapper( + [wrapperPath, '--remote-debugging-port=9448'], + devWrapperTestEnv({ ORCA_ELECTRON_VITE_CLI: fakeCliPath, ORCA_SKIP_DEV_CLI_PREPARE: '1', ORCA_SKIP_DEV_WEB_PREPARE: '1', @@ -431,20 +475,13 @@ describe('run-electron-vite-dev', () => { ORCA_DEV_WRAPPER_TEST_ENV_FILE: envFile, ORCA_DEV_BRANCH: 'feature/framework-symlinks', ORCA_DEV_WORKTREE_NAME: 'symlink-ui' - }), - stdio: 'ignore' - }) + }) + ) expect(wrapper.pid).toBeTypeOf('number') processesToCleanUp.add(wrapper.pid!) - await waitFor(() => { - try { - return readFileSync(envFile, 'utf8').trim().length > 0 - } catch { - return false - } - }, 20000) + await waitForEnvFile(envFile, readOutput) const trackedPids = trackPidFile(pidFile) @@ -464,6 +501,6 @@ describe('run-electron-vite-dev', () => { await stopWrapperAndTrackedPids(wrapper, trackedPids) }, - 30000 + PREPARE_TIMEOUT_MS + 30_000 ) }) diff --git a/src/main/worktree-identity-persistence.test.ts b/src/main/worktree-identity-persistence.test.ts index c66a2d72ce8..0b6dd178e84 100644 --- a/src/main/worktree-identity-persistence.test.ts +++ b/src/main/worktree-identity-persistence.test.ts @@ -5,12 +5,26 @@ import { tmpdir } from 'node:os' import type { PersistedState } from '../shared/persisted-state-types' import { canonicalWorktreeIdentity } from '../shared/worktree/identity' import { composeWorktreeHostIdentity } from '../shared/worktree/host-qualified-identity' -import { createStore, readDataFile, testState, writeDataFile } from './persistence-test-harness' +import type { Store } from './persistence/loading-store/store' +import { + createStore, + makeRepo, + readDataFile, + testState, + writeDataFile +} from './persistence-test-harness' describe('host-qualified worktree metadata', () => { const worktreeId = 'repo-1::/workspace/feature' const ROTATED_INSTANCE_ID = '44444444-4444-4444-8444-444444444444' + // Registered on purpose: rows owned by an unregistered repo id are swept as orphans on load. + const createStoreWithRepo = (): Store => { + const store = createStore() + store.addRepo(makeRepo({ id: 'repo-1', path: '/workspace' })) + return store + } + beforeEach(() => { testState.dir = mkdtempSync(join(tmpdir(), 'orca-worktree-identity-')) }) @@ -52,7 +66,7 @@ describe('host-qualified worktree metadata', () => { }) }) it('reloads host-specific metadata without collapsing it to the legacy locator', () => { - const store = createStore() + const store = createStoreWithRepo() store.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'Local feature' }) store.setWorktreeMetaForHost(worktreeId, 'ssh:build-box', { displayName: 'Remote feature' }) store.flush() @@ -72,7 +86,7 @@ describe('host-qualified worktree metadata', () => { expect(store.getWorktreeMetaForHost(worktreeId, 'local')?.comment).toBe('after') }) it('backfills one stable instance for legacy metadata that omitted it', () => { - const seed = createStore() + const seed = createStoreWithRepo() seed.setWorktreeMeta(worktreeId, { displayName: 'Legacy feature' }) seed.flush() const legacy = readDataFile() as PersistedState @@ -97,7 +111,7 @@ describe('host-qualified worktree metadata', () => { // Fails open on purpose: an ambiguous alias used to brick reads and throw out of the worktree // listing loop, taking every workspace in the repo down with it and never self-healing. it('collapses an ambiguous locator onto its most recently active instance', () => { - const seed = createStore() + const seed = createStoreWithRepo() const first = seed.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'First' }) seed.flush() const persisted = readDataFile() as PersistedState @@ -262,7 +276,7 @@ describe('host-qualified worktree metadata', () => { it('repairs a missing canonical instance id while re-adopting an SSH target', () => { const oldHostId = 'ssh:old-target' as const const newHostId = 'ssh:new-target' as const - const seed = createStore() + const seed = createStoreWithRepo() seed.setWorktreeMetaForHost(worktreeId, oldHostId, { displayName: 'Remote feature' }) seed.flush() const persisted = readDataFile() as PersistedState @@ -318,7 +332,7 @@ describe('host-qualified worktree metadata', () => { it('deduplicates an equivalent destination during SSH target re-adoption', () => { const oldHostId = 'ssh:old-target' as const const newHostId = 'ssh:new-target' as const - const seed = createStore() + const seed = createStoreWithRepo() seed.setWorktreeMetaForHost(worktreeId, oldHostId, { displayName: 'Remote feature' }) seed.flush() const persisted = readDataFile() as PersistedState diff --git a/src/main/zsh-startup-hook-user-config-equivalence.live-shell.test.ts b/src/main/zsh-startup-hook-user-config-equivalence.live-shell.test.ts index e5e45d2c22a..cbb7d2da7c5 100644 --- a/src/main/zsh-startup-hook-user-config-equivalence.live-shell.test.ts +++ b/src/main/zsh-startup-hook-user-config-equivalence.live-shell.test.ts @@ -16,7 +16,7 @@ */ import { existsSync, mkdirSync, mkdtempSync, renameSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' -import { dirname, join } from 'node:path' +import { basename, dirname, join } from 'node:path' import { afterAll, beforeAll, describe, expect, it } from 'vitest' import { getShellLaunchConfig } from './providers/local-pty-shell-ready' import { selectShellStartupFeatures } from './shell-startup-features' @@ -382,9 +382,12 @@ describe.skipIf(process.platform === 'win32')('the fixes the old wrapper was bui // value this wrapper cannot use degrades to $HOME, where zsh itself looks. const home = makeZshHome({ '.zshrc': 'export ORCA_TEST_FROM_ZSHRC=1\n' }) try { + // Unique per run: a fixed name here shares one path with every other run in + // the system temp dir, so a killed run leaves a stale directory behind and + // every later rename onto it fails with ENOTEMPTY. const { values } = await runFromRelocatedRoot( home, - join(dirname(userDataPath), '홍길동-wsl-view') + join(dirname(userDataPath), `홍길동-${basename(userDataPath)}`) ) expect(values.ORCA_TEST_FROM_ZSHRC).toBe('1') diff --git a/src/preload/api/ui-bridge-state-and-menu-commands.ts b/src/preload/api/ui-bridge-state-and-menu-commands.ts index 25476165cfb..34eb83886c8 100644 --- a/src/preload/api/ui-bridge-state-and-menu-commands.ts +++ b/src/preload/api/ui-bridge-state-and-menu-commands.ts @@ -1,3 +1,4 @@ +import type { MarkdownDocument } from '../../shared/filesystem-entry-types' import { ipcRenderer } from 'electron' import type { PersistedUIState } from '../../shared/persisted-ui-state-types' import type { KeybindingActionId } from '../../shared/keybindings' @@ -26,6 +27,14 @@ export const uiStateAndMenuCommandsApi = { }, consumePendingSkillShare: (): Promise => ipcRenderer.invoke('ui:consumePendingSkillShare'), + onOpenMarkdownFiles: (callback: (documents: MarkdownDocument[]) => void): (() => void) => { + const listener = (_event: Electron.IpcRendererEvent, documents: MarkdownDocument[]): void => + callback(documents) + ipcRenderer.on('ui:openMarkdownFiles', listener) + return () => ipcRenderer.removeListener('ui:openMarkdownFiles', listener) + }, + consumePendingMarkdownFileOpens: (): Promise => + ipcRenderer.invoke('ui:consumePendingMarkdownFileOpens'), onOpenSetupGuide: (callback: () => void): (() => void) => { const listener = (_event: Electron.IpcRendererEvent) => callback() ipcRenderer.on('ui:openSetupGuide', listener) diff --git a/src/preload/api/ui-command-event-api.ts b/src/preload/api/ui-command-event-api.ts index e63034b5233..0876104e471 100644 --- a/src/preload/api/ui-command-event-api.ts +++ b/src/preload/api/ui-command-event-api.ts @@ -1,3 +1,4 @@ +import type { MarkdownDocument } from '../../shared/filesystem-entry-types' import type { PersistedUIState } from '../../shared/persisted-ui-state-types' import type { TuiAgent } from '../../shared/tui-agent' import type { @@ -48,6 +49,10 @@ export type UiCommandEventApi = { consumePendingOpenSettings: () => Promise onOpenSkillShare: (callback: (shareId: string) => void) => () => void consumePendingSkillShare: () => Promise + /** OS "Open With" markdown paths pushed while a renderer is already listening. */ + onOpenMarkdownFiles: (callback: (documents: MarkdownDocument[]) => void) => () => void + /** Drains the "Open With" paths queued before this renderer's listener attached. */ + consumePendingMarkdownFileOpens: () => Promise onOpenSetupGuide: (callback: () => void) => () => void onOpenFeatureTour: (callback: () => void) => () => void onOpenCrashReport: (callback: () => void) => () => void diff --git a/src/renderer/src/components/floating-terminal/use-floating-terminal-create-actions.ts b/src/renderer/src/components/floating-terminal/use-floating-terminal-create-actions.ts index a60cdc7d93e..8ef85f7e556 100644 --- a/src/renderer/src/components/floating-terminal/use-floating-terminal-create-actions.ts +++ b/src/renderer/src/components/floating-terminal/use-floating-terminal-create-actions.ts @@ -4,7 +4,7 @@ import { resolveGroupTabFromVisibleId } from '@/components/tab-group/tab-group-v import { getConnectionId } from '@/lib/connection-context' import { createUntitledMarkdownFileWithTemplateSelection } from '@/lib/create-untitled-markdown' import { ensureClientCreationActionAllowed } from '@/lib/client-creation-action-error' -import { detectLanguage } from '@/lib/language-detect' +import { openMarkdownDocumentInFloatingWorkspace } from '@/lib/open-markdown-in-floating-workspace' import { extractIpcErrorMessage } from '@/lib/ipc-error' import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface' import { translate } from '@/i18n/i18n' @@ -123,21 +123,9 @@ export function useFloatingTerminalCreateActions({ if (!document) { return } - openFile( - { - filePath: document.filePath, - relativePath: document.relativePath, - worktreeId: FLOATING_TERMINAL_WORKTREE_ID, - language: detectLanguage(document.relativePath), - mode: 'edit', - runtimeEnvironmentId: null - }, - { - preview: false, - targetGroupId: activeGroup?.id, - suppressActiveRuntimeFallback: true - } - ) + openMarkdownDocumentInFloatingWorkspace(openFile, document, { + targetGroupId: activeGroup?.id + }) } catch (error) { toast.error(extractIpcErrorMessage(error, 'Failed to open markdown file.')) } diff --git a/src/renderer/src/components/use-worktree-jump-palette-quick-actions.ts b/src/renderer/src/components/use-worktree-jump-palette-quick-actions.ts index c6cf2c48d05..0783a7e5d23 100644 --- a/src/renderer/src/components/use-worktree-jump-palette-quick-actions.ts +++ b/src/renderer/src/components/use-worktree-jump-palette-quick-actions.ts @@ -58,6 +58,7 @@ export function useWorktreeJumpPaletteQuickActions({ groupsByWorktree, isLoading, settings, + runtimeStatusByEnvironmentId, deferredQuery, settingsResults }: WorktreeJumpPaletteQuickActionsInput) { @@ -117,6 +118,9 @@ export function useWorktreeJumpPaletteQuickActions({ openNewTerminalTabInActiveWorkspace ] ) + // Why: buildQuickActionContext() reads the store imperatively, so these voided values are the + // memo's real inputs — each one is read (some transitively, e.g. runtimeStatusByEnvironmentId + // via the managed-browser creation policy) while availability is computed. const availableActionResults = useMemo(() => { void activeView void activeWorktreeId @@ -127,6 +131,7 @@ export function useWorktreeJumpPaletteQuickActions({ void groupsByWorktree void isLoading void settings?.activeRuntimeEnvironmentId + void runtimeStatusByEnvironmentId const context = buildQuickActionContext() return actionResults.filter((action) => action.isAvailable(context).available) }, [ @@ -140,7 +145,8 @@ export function useWorktreeJumpPaletteQuickActions({ activeGroupIdByWorktree, groupsByWorktree, isLoading, - settings?.activeRuntimeEnvironmentId + settings?.activeRuntimeEnvironmentId, + runtimeStatusByEnvironmentId ]) const middleItems = useMemo<(SettingsPaletteItem | QuickActionPaletteItem)[]>( () => diff --git a/src/renderer/src/components/worktree-jump-palette-quick-action-availability.test.tsx b/src/renderer/src/components/worktree-jump-palette-quick-action-availability.test.tsx new file mode 100644 index 00000000000..69459901bfa --- /dev/null +++ b/src/renderer/src/components/worktree-jump-palette-quick-action-availability.test.tsx @@ -0,0 +1,103 @@ +// @vitest-environment happy-dom + +import { renderHook } from '@testing-library/react' +import { createRef } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { BROWSER_SCREENCAST_RUNTIME_CAPABILITY } from '../../../shared/protocol-version' +import { buildCmdJActionResults } from '@/components/cmd-j/palette-results' +import { getCmdJQuickActions } from '@/components/cmd-j/quick-actions' +import { useWorktreeJumpPaletteQuickActions } from './use-worktree-jump-palette-quick-actions' + +const mocks = vi.hoisted(() => ({ state: {} as Record })) + +vi.mock('@/store', () => ({ useAppStore: { getState: () => mocks.state } })) +vi.mock('@/lib/worktree-runtime-owner', () => ({ + getRuntimeEnvironmentIdForWorktree: () => RUNTIME_ID +})) +vi.mock('@/components/sidebar/delete-worktree-flow', () => ({ runWorktreeDelete: vi.fn() })) + +const RUNTIME_ID = 'runtime-1' +const WORKTREE_ID = 'repo-1::/repo/wt' + +function runtimeStatuses(capabilities: string[]): Map { + return new Map([[RUNTIME_ID, { status: { capabilities, hostPlatform: 'darwin' } }]]) +} + +// Every input except runtimeStatusByEnvironmentId keeps a stable identity across rerenders, +// so a recomputation can only come from the runtime status dependency itself. +function buildStableProps() { + return { + openModal: vi.fn(), + openSettingsPage: vi.fn(), + openSettingsTarget: vi.fn(), + activeGroupSnapshotRef: createRef(), + openNewBrowserTabInActiveWorkspace: vi.fn(), + openNewMarkdownInActiveWorkspace: vi.fn(), + openNewTerminalTabInActiveWorkspace: vi.fn(), + actionResults: buildCmdJActionResults(getCmdJQuickActions()), + activeView: 'terminal', + activeWorktreeId: WORKTREE_ID, + worktreesByRepo: mocks.state.worktreesByRepo, + repos: mocks.state.repos, + sshConnectionStates: mocks.state.sshConnectionStates, + activeGroupIdByWorktree: mocks.state.activeGroupIdByWorktree, + groupsByWorktree: mocks.state.groupsByWorktree, + isLoading: false, + settings: mocks.state.settings, + deferredQuery: 'new browser tab', + settingsResults: [] + } +} + +function renderQuickActions(initialStatuses: Map) { + const stable = buildStableProps() + mocks.state.runtimeStatusByEnvironmentId = initialStatuses + const harness = renderHook( + (runtimeStatusByEnvironmentId: Map) => + useWorktreeJumpPaletteQuickActions({ ...stable, runtimeStatusByEnvironmentId } as never), + { initialProps: initialStatuses } + ) + return { + offersBrowserAction: (): boolean => + harness.result.current.middleItems.some((item) => item.id === 'quick-action:new-browser-tab'), + setRuntimeStatuses: (next: Map): void => { + mocks.state.runtimeStatusByEnvironmentId = next + harness.rerender(next) + } + } +} + +describe('worktree jump palette quick action availability', () => { + beforeEach(() => { + ;(globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ = true + mocks.state = { + activeView: 'terminal', + activeWorktreeId: WORKTREE_ID, + worktreesByRepo: { 'repo-1': [{ id: WORKTREE_ID, repoId: 'repo-1' }] }, + repos: [{ id: 'repo-1' }], + sshConnectionStates: new Map(), + activeGroupIdByWorktree: { [WORKTREE_ID]: 'group-1' }, + groupsByWorktree: { [WORKTREE_ID]: [{ id: 'group-1' }] }, + settings: { activeRuntimeEnvironmentId: RUNTIME_ID } + } + }) + afterEach(() => { + delete (globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ + }) + + it('drops the paired-web browser action when the runtime loses screencast capability', () => { + const palette = renderQuickActions(runtimeStatuses([BROWSER_SCREENCAST_RUNTIME_CAPABILITY])) + expect(palette.offersBrowserAction()).toBe(true) + + palette.setRuntimeStatuses(runtimeStatuses([])) + expect(palette.offersBrowserAction()).toBe(false) + }) + + it('restores the browser action when a capable runtime comes back', () => { + const palette = renderQuickActions(runtimeStatuses([])) + expect(palette.offersBrowserAction()).toBe(false) + + palette.setRuntimeStatuses(runtimeStatuses([BROWSER_SCREENCAST_RUNTIME_CAPABILITY])) + expect(palette.offersBrowserAction()).toBe(true) + }) +}) diff --git a/src/renderer/src/hooks/ipc-events/app-lifetime-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/app-lifetime-ipc-bridge.ts index 7aa5dd418cf..d105db1d3e9 100644 --- a/src/renderer/src/hooks/ipc-events/app-lifetime-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/app-lifetime-ipc-bridge.ts @@ -12,6 +12,7 @@ import { createDirectSshBridgeRuntime } from './direct-ssh-bridge-runtime' import { registerDirectSshStateIpcBridge } from './direct-ssh-state-ipc-bridge' import { registerMobileAndTerminalCloseIpcBridge } from './mobile-terminal-close-ipc-bridge' import { registerMobileDriverIpcBridge } from './mobile-driver-ipc-bridge' +import { registerOsMarkdownFileOpenBridge } from './os-markdown-file-open-bridge' import { registerProjectCatalogIpcBridge } from './project-catalog-ipc-bridge' import { registerRateLimitIpcBridge } from './rate-limit-ipc-bridge' import { registerRemoteWorkspaceIpcBridge } from './remote-workspace-ipc-bridge' @@ -77,6 +78,7 @@ export function installAppLifetimeIpcEvents( ) registerSettingsAndSidebarIpcBridge(unsubs) registerWorkspaceShortcutIpcBridge(unsubs) + registerOsMarkdownFileOpenBridge(unsubs) unsubs.push( window.api.ui.onActivateWorktree(({ repoId, worktreeId, setup, startup, defaultTabs }) => { void worktreeRuntime diff --git a/src/renderer/src/hooks/ipc-events/os-markdown-file-open-bridge.test.ts b/src/renderer/src/hooks/ipc-events/os-markdown-file-open-bridge.test.ts new file mode 100644 index 00000000000..d4825e89206 --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/os-markdown-file-open-bridge.test.ts @@ -0,0 +1,300 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { TOGGLE_FLOATING_TERMINAL_EVENT } from '@/lib/floating-terminal' +import type { EditorFilesSlice } from '@/store/slices/editor/types/editor-files-slice' +import type { MarkdownDocument } from '../../../../shared/filesystem-entry-types' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' +import { registerOsMarkdownFileOpenBridge } from './os-markdown-file-open-bridge' + +const mocks = vi.hoisted(() => ({ + openFile: vi.fn(() => 'file-1'), + updateSettings: vi.fn(async () => {}), + isFloatingWorkspacePanelVisible: vi.fn(() => false), + toastError: vi.fn() +})) + +let storeState: { + openFile: typeof mocks.openFile + updateSettings: typeof mocks.updateSettings + settings: { floatingTerminalEnabled?: boolean } | undefined +} + +vi.mock('../../store', () => ({ useAppStore: { getState: () => storeState } })) +vi.mock('@/lib/floating-workspace-terminal-actions', () => ({ + isFloatingWorkspacePanelVisible: mocks.isFloatingWorkspacePanelVisible +})) +vi.mock('sonner', () => ({ toast: { error: mocks.toastError } })) +vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) + +type MarkdownFileOpenListener = (documents: MarkdownDocument[]) => void + +let frames: FrameRequestCallback[] = [] +let dispatchEvent = vi.fn() +let unhandledRejections: unknown[] = [] +const recordUnhandledRejection = (reason: unknown): void => void unhandledRejections.push(reason) + +function markdownDocument(overrides: Partial = {}): MarkdownDocument { + return { + filePath: '/Users/me/notes/README.md', + relativePath: 'README.md', + basename: 'README.md', + name: 'README', + ...overrides + } +} + +function stubPreload(ui: Record): void { + dispatchEvent = vi.fn() + vi.stubGlobal('window', { api: { ui }, dispatchEvent }) +} + +/** Runs the callbacks the bridge deferred to the next frame. */ +function runFrames(): void { + const pending = frames + frames = [] + for (const frame of pending) { + frame(0) + } +} + +/** Drains microtasks and lets Node emit any unhandled rejection the bridge leaked. */ +async function settle(): Promise { + await new Promise((resolve) => setImmediate(resolve)) + await new Promise((resolve) => setImmediate(resolve)) +} + +describe('registerOsMarkdownFileOpenBridge', () => { + beforeEach(() => { + vi.clearAllMocks() + frames = [] + unhandledRejections = [] + storeState = { + openFile: mocks.openFile, + updateSettings: mocks.updateSettings, + settings: { floatingTerminalEnabled: true } + } + mocks.openFile.mockReturnValue('file-1') + mocks.updateSettings.mockResolvedValue(undefined) + mocks.isFloatingWorkspacePanelVisible.mockReturnValue(false) + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => + frames.push(callback) + ) + vi.spyOn(console, 'error').mockImplementation(() => {}) + process.on('unhandledRejection', recordUnhandledRejection) + }) + + afterEach(() => { + process.off('unhandledRejection', recordUnhandledRejection) + vi.unstubAllGlobals() + vi.restoreAllMocks() + }) + + it('opens every document main queued before the listener attached', async () => { + stubPreload({ + onOpenMarkdownFiles: () => () => {}, + consumePendingMarkdownFileOpens: () => + Promise.resolve([ + markdownDocument(), + markdownDocument({ filePath: '/Users/me/notes/plan.md', relativePath: 'plan.md' }) + ]) + }) + + registerOsMarkdownFileOpenBridge([]) + await settle() + + expect(mocks.openFile).toHaveBeenCalledTimes(2) + expect(mocks.openFile.mock.calls.map((call) => call[0].filePath)).toEqual([ + '/Users/me/notes/README.md', + '/Users/me/notes/plan.md' + ]) + expect(mocks.openFile.mock.calls[0][0].worktreeId).toBe(FLOATING_TERMINAL_WORKTREE_ID) + }) + + it('opens documents pushed after startup and hands back the unsubscribe', async () => { + const listeners: MarkdownFileOpenListener[] = [] + const unsubscribe = vi.fn() + stubPreload({ + onOpenMarkdownFiles: (next: MarkdownFileOpenListener) => { + listeners.push(next) + return unsubscribe + }, + consumePendingMarkdownFileOpens: () => Promise.resolve([]) + }) + + const unsubs: (() => void)[] = [] + registerOsMarkdownFileOpenBridge(unsubs) + expect(unsubs).toEqual([unsubscribe]) + + listeners[0]([ + markdownDocument({ filePath: '/Users/me/notes/live.md', relativePath: 'live.md' }) + ]) + await settle() + + expect(mocks.openFile).toHaveBeenCalledTimes(1) + expect(mocks.openFile.mock.calls[0][0].filePath).toBe('/Users/me/notes/live.md') + + unsubs.forEach((teardown) => teardown()) + expect(unsubscribe).toHaveBeenCalledOnce() + }) + + it('enables the floating workspace when the setting is off', async () => { + storeState.settings = { floatingTerminalEnabled: false } + stubPreload({ + onOpenMarkdownFiles: () => () => {}, + consumePendingMarkdownFileOpens: () => Promise.resolve([markdownDocument()]) + }) + + registerOsMarkdownFileOpenBridge([]) + await settle() + + expect(mocks.updateSettings).toHaveBeenCalledWith({ floatingTerminalEnabled: true }) + }) + + it('leaves settings alone when the floating workspace is already enabled', async () => { + stubPreload({ + onOpenMarkdownFiles: () => () => {}, + consumePendingMarkdownFileOpens: () => Promise.resolve([markdownDocument()]) + }) + + registerOsMarkdownFileOpenBridge([]) + await settle() + + expect(mocks.updateSettings).not.toHaveBeenCalled() + }) + + it('defers the reveal a frame and toggles only while the panel is hidden', async () => { + stubPreload({ + onOpenMarkdownFiles: () => () => {}, + consumePendingMarkdownFileOpens: () => Promise.resolve([markdownDocument()]) + }) + + registerOsMarkdownFileOpenBridge([]) + await settle() + + expect(dispatchEvent).not.toHaveBeenCalled() + runFrames() + + expect(dispatchEvent).toHaveBeenCalledTimes(1) + expect(dispatchEvent.mock.calls[0][0].type).toBe(TOGGLE_FLOATING_TERMINAL_EVENT) + }) + + it('does not toggle when the panel is already visible', async () => { + mocks.isFloatingWorkspacePanelVisible.mockReturnValue(true) + stubPreload({ + onOpenMarkdownFiles: () => () => {}, + consumePendingMarkdownFileOpens: () => Promise.resolve([markdownDocument()]) + }) + + registerOsMarkdownFileOpenBridge([]) + await settle() + runFrames() + + expect(dispatchEvent).not.toHaveBeenCalled() + }) + + it('ignores an empty batch', async () => { + storeState.settings = { floatingTerminalEnabled: false } + stubPreload({ + onOpenMarkdownFiles: () => () => {}, + consumePendingMarkdownFileOpens: () => Promise.resolve([]) + }) + + registerOsMarkdownFileOpenBridge([]) + await settle() + runFrames() + + expect(mocks.openFile).not.toHaveBeenCalled() + expect(mocks.updateSettings).not.toHaveBeenCalled() + expect(dispatchEvent).not.toHaveBeenCalled() + }) + + it('reports a rejected pending drain without leaking an unhandled rejection', async () => { + stubPreload({ + onOpenMarkdownFiles: () => () => {}, + consumePendingMarkdownFileOpens: () => Promise.reject(new Error('ipc unavailable')) + }) + + registerOsMarkdownFileOpenBridge([]) + await settle() + + expect(mocks.toastError).toHaveBeenCalledWith('Failed to open the Markdown file.') + // Why: App.tsx awaits hydration around this registration and treats any throw as + // "session restore failed", so the bridge must swallow its own failures. + expect(unhandledRejections).toEqual([]) + }) + + it('reports a throwing openFile without leaking an unhandled rejection', async () => { + mocks.openFile.mockImplementation(() => { + throw new Error('editor slice exploded') + }) + const listeners: MarkdownFileOpenListener[] = [] + stubPreload({ + onOpenMarkdownFiles: (next: MarkdownFileOpenListener) => { + listeners.push(next) + return () => {} + }, + consumePendingMarkdownFileOpens: () => Promise.resolve([]) + }) + + registerOsMarkdownFileOpenBridge([]) + expect(() => listeners[0]([markdownDocument()])).not.toThrow() + await settle() + + expect(mocks.toastError).toHaveBeenCalledWith('Failed to open the Markdown file.') + expect(unhandledRejections).toEqual([]) + expect(dispatchEvent).not.toHaveBeenCalled() + }) + + it('keeps opening the rest of a batch when one document fails', async () => { + mocks.openFile.mockImplementationOnce(() => { + throw new Error('first document exploded') + }) + stubPreload({ + onOpenMarkdownFiles: () => () => {}, + consumePendingMarkdownFileOpens: () => + Promise.resolve([ + markdownDocument({ filePath: '/Users/me/notes/bad.md', relativePath: 'bad.md' }), + markdownDocument({ filePath: '/Users/me/notes/good.md', relativePath: 'good.md' }) + ]) + }) + + registerOsMarkdownFileOpenBridge([]) + await settle() + runFrames() + + // Why: a multi-file selection arrives as one batch; one bad file must not cost the rest. + expect(mocks.openFile).toHaveBeenCalledTimes(2) + expect(mocks.toastError).toHaveBeenCalledTimes(1) + expect(dispatchEvent).toHaveBeenCalledTimes(1) + expect(unhandledRejections).toEqual([]) + }) + + it('ignores a non-array payload from a mismatched preload', async () => { + stubPreload({ + onOpenMarkdownFiles: () => () => {}, + // Why: the payload crosses the preload boundary, so a stale preload can resolve with + // something that is not an array. Reading .length off it would throw inside the chain. + consumePendingMarkdownFileOpens: () => Promise.resolve(null as unknown as MarkdownDocument[]) + }) + + registerOsMarkdownFileOpenBridge([]) + await settle() + runFrames() + + expect(mocks.openFile).not.toHaveBeenCalled() + expect(mocks.updateSettings).not.toHaveBeenCalled() + expect(mocks.toastError).not.toHaveBeenCalled() + expect(unhandledRejections).toEqual([]) + }) + + it('tolerates a preload without the markdown open channel', async () => { + stubPreload({}) + + const unsubs: (() => void)[] = [] + expect(() => registerOsMarkdownFileOpenBridge(unsubs)).not.toThrow() + await settle() + + expect(unsubs).toEqual([]) + expect(mocks.openFile).not.toHaveBeenCalled() + expect(mocks.toastError).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/hooks/ipc-events/os-markdown-file-open-bridge.ts b/src/renderer/src/hooks/ipc-events/os-markdown-file-open-bridge.ts new file mode 100644 index 00000000000..3dd345da07c --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/os-markdown-file-open-bridge.ts @@ -0,0 +1,72 @@ +import { toast } from 'sonner' +import type { MarkdownDocument } from '../../../../shared/filesystem-entry-types' +import { TOGGLE_FLOATING_TERMINAL_EVENT } from '@/lib/floating-terminal' +import { isFloatingWorkspacePanelVisible } from '@/lib/floating-workspace-terminal-actions' +import { openMarkdownDocumentInFloatingWorkspace } from '@/lib/open-markdown-in-floating-workspace' +import { translate } from '@/i18n/i18n' +import { useAppStore } from '../../store' + +/** + * Opens markdown files the OS shell handed to Orca ("Open With" / double-click) in the + * floating workspace, which is the one editor surface that needs no project. + */ +async function openOsRequestedMarkdownFiles(documents: MarkdownDocument[]): Promise { + // Why the shape check: this payload crosses the preload boundary, so a stale or mismatched + // preload can hand back something that is not an array. Reading .length off that throws + // inside the promise chain rather than failing loudly at the boundary. + if (!Array.isArray(documents) || documents.length === 0) { + return + } + const store = useAppStore.getState() + let opened = 0 + for (const document of documents) { + // Why isolated: selecting several files hands us one batch, and one unopenable file + // must not cost the user the rest of the selection. + try { + openMarkdownDocumentInFloatingWorkspace(store.openFile, document) + opened += 1 + } catch (error) { + reportOsRequestedMarkdownFailure(error) + } + } + if (opened === 0) { + return + } + // Why enabled here: the user asked the OS for this file, and the tabs above are already in a + // surface a disabled floating workspace never renders. Same enable-then-reveal as the + // Settings "Edit keybindings in Orca" action. + if (store.settings?.floatingTerminalEnabled !== true) { + await store.updateSettings({ floatingTerminalEnabled: true }) + } + // Why deferred a frame: the panel only honors the toggle once the enabled flag has reached React. + requestAnimationFrame(() => { + if (!isFloatingWorkspacePanelVisible()) { + window.dispatchEvent(new CustomEvent(TOGGLE_FLOATING_TERMINAL_EVENT)) + } + }) +} + +function reportOsRequestedMarkdownFailure(error: unknown): void { + console.error('Failed to open markdown files requested by the OS:', error) + toast.error( + translate( + 'auto.hooks.ipc.events.os.markdown.file.open.bridge.1e9a1a63c4', + 'Failed to open the Markdown file.' + ) + ) +} + +export function registerOsMarkdownFileOpenBridge(unsubs: (() => void)[]): void { + const unsubscribe = window.api.ui.onOpenMarkdownFiles?.((documents) => { + void openOsRequestedMarkdownFiles(documents).catch(reportOsRequestedMarkdownFailure) + }) + if (unsubscribe) { + unsubs.push(unsubscribe) + } + + // Why: a cold-start "Open With" resolves before this listener attaches; drain what main queued. + const pending = window.api.ui.consumePendingMarkdownFileOpens?.() + if (pending && typeof pending.then === 'function') { + void pending.then(openOsRequestedMarkdownFiles).catch(reportOsRequestedMarkdownFailure) + } +} diff --git a/src/renderer/src/hooks/useIpcEvents-lifecycle.test.ts b/src/renderer/src/hooks/useIpcEvents-lifecycle.test.ts index 5155aa6881f..67872949590 100644 --- a/src/renderer/src/hooks/useIpcEvents-lifecycle.test.ts +++ b/src/renderer/src/hooks/useIpcEvents-lifecycle.test.ts @@ -55,6 +55,7 @@ const EXPECTED_DIRECT_CALLBACK_METHODS = [ 'ui.onOpenDiffFromMobile', 'ui.onOpenFeatureTour', 'ui.onOpenFileFromMobile', + 'ui.onOpenMarkdownFiles', 'ui.onOpenNewWorkspace', 'ui.onOpenQuickOpen', 'ui.onOpenSettings', @@ -135,6 +136,7 @@ const EXPECTED_CALLBACK_REGISTRATION_SEQUENCE = [ 'ui.onJumpToTabIndex', 'ui.onWorktreeHistoryNavigate', 'ui.onToggleStatusBar', + 'ui.onOpenMarkdownFiles', 'ui.onActivateWorktree', 'ui.onCreateTerminal', 'ui.onRequestTerminalTabMount', diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index c04b2f260e9..7fc9c837ebc 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -1112,6 +1112,17 @@ "events": { "browserStateIpcBridge": { "docPreviewLinkFailed": "Could not open this link in Orca Browser." + }, + "os": { + "markdown": { + "file": { + "open": { + "bridge": { + "1e9a1a63c4": "Failed to open the Markdown file." + } + } + } + } } } } diff --git a/src/renderer/src/lib/lazy-chunk-recovery-reload.test.ts b/src/renderer/src/lib/lazy-chunk-recovery-reload.test.ts index 9cdf4fa728d..8c113363861 100644 --- a/src/renderer/src/lib/lazy-chunk-recovery-reload.test.ts +++ b/src/renderer/src/lib/lazy-chunk-recovery-reload.test.ts @@ -6,6 +6,7 @@ import { requestLazyChunkRecoveryReload } from './lazy-chunk-recovery-reload' describe('requestLazyChunkRecoveryReload', () => { afterEach(() => { vi.restoreAllMocks() + vi.unstubAllGlobals() }) it('refuses the reload when the staged checkpoint never reaches disk', async () => { @@ -36,4 +37,32 @@ describe('requestLazyChunkRecoveryReload', () => { expect(order).toEqual(['flushed', 'reload']) }) + + it('joins the preload checkpoint before navigating when no override is supplied', async () => { + const order: string[] = [] + let flush: () => void = () => undefined + const awaitBeforeUnloadCheckpoint = vi.fn( + () => + new Promise((resolve) => { + flush = () => { + order.push('flushed') + resolve() + } + }) + ) + vi.stubGlobal('api', { app: { awaitBeforeUnloadCheckpoint } }) + const reload = vi.spyOn(window.location, 'reload').mockImplementation(() => { + order.push('reload') + window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT)) + }) + + const outcome = requestLazyChunkRecoveryReload(window) + await vi.waitFor(() => expect(awaitBeforeUnloadCheckpoint).toHaveBeenCalledTimes(1)) + expect(reload).not.toHaveBeenCalled() + + flush() + + await expect(outcome).resolves.toBe('unload-vetoed') + expect(order).toEqual(['flushed', 'reload']) + }) }) diff --git a/src/renderer/src/lib/open-markdown-in-floating-workspace.test.ts b/src/renderer/src/lib/open-markdown-in-floating-workspace.test.ts new file mode 100644 index 00000000000..62fc278fc7a --- /dev/null +++ b/src/renderer/src/lib/open-markdown-in-floating-workspace.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it, vi } from 'vitest' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' +import type { MarkdownDocument } from '../../../shared/filesystem-entry-types' +import type { EditorFilesSlice } from '@/store/slices/editor/types/editor-files-slice' +import { openMarkdownDocumentInFloatingWorkspace } from './open-markdown-in-floating-workspace' + +function openFileMock(): ReturnType> { + return vi.fn(() => 'file-1') +} + +function markdownDocument(overrides: Partial = {}): MarkdownDocument { + return { + filePath: '/Users/me/notes/README.md', + relativePath: 'README.md', + basename: 'README.md', + name: 'README', + ...overrides + } +} + +describe('openMarkdownDocumentInFloatingWorkspace', () => { + it('opens the document as a permanent floating-workspace edit tab', () => { + const openFile = openFileMock() + + const fileId = openMarkdownDocumentInFloatingWorkspace(openFile, markdownDocument()) + + expect(fileId).toBe('file-1') + expect(openFile).toHaveBeenCalledTimes(1) + expect(openFile.mock.calls[0][0]).toEqual({ + filePath: '/Users/me/notes/README.md', + relativePath: 'README.md', + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + language: 'markdown', + mode: 'edit', + runtimeEnvironmentId: null + }) + expect(openFile.mock.calls[0][1]).toEqual({ + preview: false, + targetGroupId: undefined, + suppressActiveRuntimeFallback: true + }) + }) + + it('pins the open to this machine instead of the active runtime', () => { + const openFile = openFileMock() + + openMarkdownDocumentInFloatingWorkspace(openFile, markdownDocument()) + + // Why: the caller already resolved an absolute local path, so a null runtime plus the + // fallback suppression is what keeps the read off a remote SSH host the user is focused on. + // Dropping either one silently reads the file on the wrong machine. + expect(openFile.mock.calls[0][0].runtimeEnvironmentId).toBeNull() + expect(openFile.mock.calls[0][1]?.suppressActiveRuntimeFallback).toBe(true) + }) + + it('derives the language from the relative path', () => { + const openFile = openFileMock() + + openMarkdownDocumentInFloatingWorkspace( + openFile, + markdownDocument({ + filePath: '/Users/me/notes/plan.mdx', + relativePath: 'plan.mdx', + basename: 'plan.mdx', + name: 'plan' + }) + ) + + expect(openFile.mock.calls[0][0].language).toBe('markdown') + }) + + it('forwards a requested target group', () => { + const openFile = openFileMock() + + openMarkdownDocumentInFloatingWorkspace(openFile, markdownDocument(), { + targetGroupId: 'group-2' + }) + + expect(openFile.mock.calls[0][1]?.targetGroupId).toBe('group-2') + }) +}) diff --git a/src/renderer/src/lib/open-markdown-in-floating-workspace.ts b/src/renderer/src/lib/open-markdown-in-floating-workspace.ts new file mode 100644 index 00000000000..3b1bd4cd08b --- /dev/null +++ b/src/renderer/src/lib/open-markdown-in-floating-workspace.ts @@ -0,0 +1,32 @@ +import type { MarkdownDocument } from '../../../shared/filesystem-entry-types' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' +import type { EditorFilesSlice } from '@/store/slices/editor/types/editor-files-slice' +import { detectLanguage } from './language-detect' + +/** + * Opens a markdown file that belongs to no workspace as a floating-workspace editor tab. + * + * Why local-only: every caller resolves an absolute path on this machine (a native picker or + * the OS shell), so routing it through the active runtime would read it on the wrong host. + */ +export function openMarkdownDocumentInFloatingWorkspace( + openFile: EditorFilesSlice['openFile'], + document: MarkdownDocument, + options: { targetGroupId?: string } = {} +): string { + return openFile( + { + filePath: document.filePath, + relativePath: document.relativePath, + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + language: detectLanguage(document.relativePath), + mode: 'edit', + runtimeEnvironmentId: null + }, + { + preview: false, + targetGroupId: options.targetGroupId, + suppressActiveRuntimeFallback: true + } + ) +} diff --git a/src/renderer/src/lib/palette-match/palette-match-budget.ts b/src/renderer/src/lib/palette-match/palette-match-budget.ts index c51144fb39d..864473a65e6 100644 --- a/src/renderer/src/lib/palette-match/palette-match-budget.ts +++ b/src/renderer/src/lib/palette-match/palette-match-budget.ts @@ -1,8 +1,14 @@ /** * Checked-in performance budget for the Cmd+J matcher, measured against the * synthetic corpus in `palette-match-performance.test.ts`. These are ceilings for - * catching order-of-magnitude regressions, not targets — the measured numbers on - * a developer machine sit roughly an order of magnitude under each one. + * catching order-of-magnitude regressions, not targets. + * + * The wall-clock ceilings are asserted against the *fastest* sample of a batch, + * never the slowest: a vitest worker sharing cores with the rest of the suite + * gets preempted mid-measurement, so the slowest sample measures the machine + * while the fastest still approximates the matcher. Fan-out regressions are + * caught by `fieldMatchesPerCandidate` instead, which counts work rather than + * time and so does not depend on machine speed at all. * * Raising any value requires a fresh measurement recorded in the PR. */ @@ -11,10 +17,17 @@ export const PALETTE_MATCH_BUDGET = { candidateCount: 800, /** Unique tokens in the worst supported query. */ tokenCount: 16, - /** p95 milliseconds to normalize every document once (cold open). */ - coldBuildP95Ms: 900, - /** p95 milliseconds to match the whole corpus against one prepared query. */ - warmMatchP95Ms: 220, + /** + * Ceiling on `matchPaletteField` calls per candidate for the worst query. + * Deterministic — it counts work, not time — so it catches a fan-out + * regression (re-matching every field per evidence unit, say) on any machine. + * Measured 45: 15 fields across the 3 tokens scanned before the first miss. + */ + fieldMatchesPerCandidate: 60, + /** Milliseconds to normalize every document once (cold open), fastest sample. */ + coldBuildMs: 900, + /** Milliseconds to match the whole corpus against one prepared query, fastest sample. */ + warmMatchMs: 220, /** * Megabytes of indexed text and offset tables the normalized documents retain. * Measured deterministically rather than from `heapUsed`, which is polluted by diff --git a/src/renderer/src/lib/palette-match/palette-match-performance.test.ts b/src/renderer/src/lib/palette-match/palette-match-performance.test.ts index aa1e6e12047..13fbced916f 100644 --- a/src/renderer/src/lib/palette-match/palette-match-performance.test.ts +++ b/src/renderer/src/lib/palette-match/palette-match-performance.test.ts @@ -1,8 +1,11 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { PALETTE_MATCH_BUDGET } from './palette-match-budget' import { matchPaletteDocument } from './match-document' +import * as matchFieldModule from './match-field' import { preparePaletteQuery } from './palette-query' import { buildWorktreePaletteDocuments } from '../worktree-palette-document' +import type { PaletteDocument } from './palette-document' +import type { PaletteQueryToken } from './palette-query' import type { Repo } from '../../../../shared/repo-types' import type { Worktree } from '../../../../shared/worktree/types' @@ -89,49 +92,75 @@ const WORST_QUERY = Array.from({ length: tokenCount }, (_, index) => index === 0 ? 'scan' : index === 1 ? 'daily' : `token${index}` ).join(' ') -function percentile95(samples: number[]): number { - const sorted = [...samples].sort((a, b) => a - b) - return sorted[Math.min(sorted.length - 1, Math.floor(sorted.length * 0.95))] +function prepareWorstQuery(): { tokens: readonly PaletteQueryToken[]; normalized: string } { + const prepared = preparePaletteQuery(WORST_QUERY) + if (prepared.state !== 'ready') { + throw new Error(`Expected a ready query, got ${prepared.state}`) + } + return { tokens: prepared.tokens, normalized: prepared.normalized } +} + +const preparedQuery = prepareWorstQuery() + +function matchEveryDocument(documents: ReadonlyMap): void { + for (const document of documents.values()) { + matchPaletteDocument({ + document, + tokens: preparedQuery.tokens, + normalizedQuery: preparedQuery.normalized + }) + } +} + +/** + * Why the fastest sample and not p95: this runs in a vitest worker competing for + * cores with the rest of the suite, so a slow sample records a preemption rather + * than the matcher. The fastest sample is the least contaminated estimate of + * intrinsic cost — measured stable within 1.6x on a fully saturated machine, + * while the slowest of the same batch swung by 17x. + */ +function fastestSample(samples: readonly number[]): number { + return Math.min(...samples) +} + +function timeRepeatedly(work: () => void, rounds: number): number[] { + const samples: number[] = [] + for (let round = 0; round < rounds; round += 1) { + const start = performance.now() + work() + samples.push(performance.now() - start) + } + return samples } describe('palette matcher performance budget', () => { it('normalizes a cold corpus within budget', () => { - const samples: number[] = [] - for (let run = 0; run < 5; run += 1) { - const start = performance.now() - buildWorktreePaletteDocuments(worktrees, sources) - samples.push(performance.now() - start) - } - expect(percentile95(samples)).toBeLessThan(PALETTE_MATCH_BUDGET.coldBuildP95Ms) + const samples = timeRepeatedly(() => buildWorktreePaletteDocuments(worktrees, sources), 5) + expect(fastestSample(samples)).toBeLessThan(PALETTE_MATCH_BUDGET.coldBuildMs) }) it('matches a 16-token query against warm documents within budget', () => { const documents = buildWorktreePaletteDocuments(worktrees, sources) - const prepared = preparePaletteQuery(WORST_QUERY) - expect(prepared.state).toBe('ready') - if (prepared.state !== 'ready') { - return - } - const matchAllDocuments = (): void => { - for (const document of documents.values()) { - matchPaletteDocument({ - document, - tokens: prepared.tokens, - normalizedQuery: prepared.normalized - }) - } - } + // Warm the matcher before timing so JIT compilation is not part of the samples. + matchEveryDocument(documents) - // Warm the matcher before timing so JIT compilation is not part of p95. - matchAllDocuments() - const samples: number[] = [] - for (let run = 0; run < 10; run += 1) { - const start = performance.now() - matchAllDocuments() - samples.push(performance.now() - start) + const samples = timeRepeatedly(() => matchEveryDocument(documents), 10) + expect(fastestSample(samples)).toBeLessThan(PALETTE_MATCH_BUDGET.warmMatchMs) + }) + + it('bounds field-match fan-out per candidate', () => { + const documents = buildWorktreePaletteDocuments(worktrees, sources) + const fieldMatch = vi.spyOn(matchFieldModule, 'matchPaletteField') + try { + matchEveryDocument(documents) + const perCandidate = fieldMatch.mock.calls.length / documents.size + // Guards the ceiling against going vacuous if the spy ever stops intercepting. + expect(perCandidate).toBeGreaterThan(0) + expect(perCandidate).toBeLessThan(PALETTE_MATCH_BUDGET.fieldMatchesPerCandidate) + } finally { + fieldMatch.mockRestore() } - expect(percentile95(samples)).toBeLessThan(PALETTE_MATCH_BUDGET.warmMatchP95Ms) }) it('keeps the retained document payload within budget', () => { diff --git a/src/renderer/src/store/slices/agent-status-ack-cleanup.test.ts b/src/renderer/src/store/slices/agent-status-ack-cleanup.test.ts index b763262a22d..4ca35b9bdc2 100644 --- a/src/renderer/src/store/slices/agent-status-ack-cleanup.test.ts +++ b/src/renderer/src/store/slices/agent-status-ack-cleanup.test.ts @@ -117,3 +117,31 @@ describe('acknowledgedAgentsByPaneKey cleanup on teardown', () => { expect(ackAt < newEntry.stateStartedAt).toBe(true) }) }) + +// Why: only the terminal-view path cleared unreadAgentCompletionPanes, so an +// Activity-page ack left the tab dot lit. +describe('acknowledgeAgents clears the unread agent-completion marker', () => { + it('drops the pane from unreadAgentCompletionPanes', () => { + const store = createTestStore() + store.getState().setAgentStatus('tab-1:0', { state: 'done', prompt: 'p', agentType: 'claude' }) + store.getState().markAgentCompletionPaneUnread('tab-1:0') + expect(store.getState().unreadAgentCompletionPanes['tab-1:0']).toBe(true) + + store.getState().acknowledgeAgents(['tab-1:0']) + + expect(store.getState().unreadAgentCompletionPanes['tab-1:0']).toBeUndefined() + }) + + it('leaves other panes and the terminal-bell unread map untouched', () => { + const store = createTestStore() + store.getState().markAgentCompletionPaneUnread('tab-1:0') + store.getState().markAgentCompletionPaneUnread('tab-2:0') + store.getState().markTerminalPaneUnread('tab-1:0') + + store.getState().acknowledgeAgents(['tab-1:0']) + + expect(store.getState().unreadAgentCompletionPanes['tab-2:0']).toBe(true) + // Why: a BEL is a separate signal; acking the agent must not silence it. + expect(store.getState().unreadTerminalPanes['tab-1:0']).toBe(true) + }) +}) diff --git a/src/renderer/src/store/slices/ui-slice-test-harness.ts b/src/renderer/src/store/slices/ui-slice-test-harness.ts index 318b24afef8..b8bbcf24b85 100644 --- a/src/renderer/src/store/slices/ui-slice-test-harness.ts +++ b/src/renderer/src/store/slices/ui-slice-test-harness.ts @@ -20,6 +20,8 @@ export function createUIStore(): StoreApi { combinedDiffFileTreeWidth: 256, rightSidebarTab: 'explorer', rightSidebarExplorerView: 'files', + // Why: acknowledgeAgents clears the agent-completion marker the terminal slice owns. + unreadAgentCompletionPanes: {}, ...createSettingsSearchState(args[0]), ...createWorktreeNavHistorySlice(...(args as Parameters)), ...createUISlice(...(args as Parameters)) diff --git a/src/renderer/src/store/slices/ui/ui-slice-agent-actions.ts b/src/renderer/src/store/slices/ui/ui-slice-agent-actions.ts index b1401e7d416..9fa15ffb81e 100644 --- a/src/renderer/src/store/slices/ui/ui-slice-agent-actions.ts +++ b/src/renderer/src/store/slices/ui/ui-slice-agent-actions.ts @@ -217,7 +217,16 @@ export function createUiAgentActions( const migrationUnsupported = Object.values(s.migrationUnsupportedByPtyId ?? {}) // Why: only reallocate if an ack advances; compare prev | null = null + // Why: one ack, two records — leaving the completion marker set keeps the tab dot, + // the ⌘J row and the floating-workspace dot lit with nothing left to read. + let nextUnreadCompletions: Record | null = null for (const key of paneKeys) { + if (s.unreadAgentCompletionPanes[key]) { + if (nextUnreadCompletions === null) { + nextUnreadCompletions = { ...s.unreadAgentCompletionPanes } + } + delete nextUnreadCompletions[key] + } const prev = s.acknowledgedAgentsByPaneKey[key] ?? 0 // Why not plain Date.now(): a remote/SSH execution host can stamp a turn ahead of this clock, // and every unread rule is `ackAt < turnTimestamp`. A behind-the-turn ack can never clear the @@ -258,7 +267,13 @@ export function createUiAgentActions( next[key] = stamp } } - return next ? { acknowledgedAgentsByPaneKey: next } : s + if (!next && !nextUnreadCompletions) { + return s + } + return { + ...(next ? { acknowledgedAgentsByPaneKey: next } : {}), + ...(nextUnreadCompletions ? { unreadAgentCompletionPanes: nextUnreadCompletions } : {}) + } }) const notificationIds = [...notificationIdsToDismiss] if (notificationIds.length > 0 && typeof window !== 'undefined') { diff --git a/src/renderer/src/store/slices/worktrees-runtime-host-metadata-retirement.test.ts b/src/renderer/src/store/slices/worktrees-runtime-host-metadata-retirement.test.ts new file mode 100644 index 00000000000..eeaf49dd5e4 --- /dev/null +++ b/src/renderer/src/store/slices/worktrees-runtime-host-metadata-retirement.test.ts @@ -0,0 +1,104 @@ +// Why this file exists: a paired client's WorktreeMeta for a runtime host is exempt from +// gcStaleWorktreeMeta (it skips any row that is not local on both the repo and the meta's hostId), +// and `forgetPersistedWorktreeMetaForRemovals` used to bail for every non-SSH host. So the client +// kept a row per remote worktree it had ever seen and dropped none (#17776). +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { AppState } from '../types' +import { makeWorktree } from './worktrees-slice-test-fixtures' +import { makeDetectedResult } from './worktrees-detected-listing-fixtures' +import { + createTestStore, + forgetRemovedForExecutionHostMock, + resetRemoteRuntimeMocks, + resetWorktreeSliceModuleMemory, + runtimeEnvironmentCall +} from './worktrees-slice-test-harness' + +const REPO_ID = 'repo-runtime' +const HOST_ID = 'runtime:env-1' + +const worktree = (path: string) => + makeWorktree({ id: `${REPO_ID}::${path}`, repoId: REPO_ID, path, hostId: HOST_ID }) + +const live = worktree('/home/orca/live') +const deletedOnHost = worktree('/home/orca/deleted') + +function seedClientWithBothRows(): ReturnType { + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' } as never, + repos: [ + { + id: REPO_ID, + path: '/home/orca/repo', + displayName: 'Runtime Repo', + badgeColor: '#000', + addedAt: 0, + executionHostId: HOST_ID + } + ], + worktreesByRepo: { [REPO_ID]: [live, deletedOnHost] } + } as Partial) + return store +} + +beforeEach(resetWorktreeSliceModuleMemory) + +describe('runtime-host persisted metadata retirement', () => { + beforeEach(() => { + vi.clearAllMocks() + resetRemoteRuntimeMocks() + }) + + it('retires metadata for rows an authoritative runtime-host scan proved gone', async () => { + const store = seedClientWithBothRows() + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-detected', + ok: true, + result: makeDetectedResult(REPO_ID, [live]), + _meta: { runtimeId: 'runtime-remote' } + }) + + await store.getState().fetchWorktrees(REPO_ID, { executionHostId: HOST_ID }) + + expect(forgetRemovedForExecutionHostMock).toHaveBeenCalledExactlyOnceWith({ + repoId: REPO_ID, + executionHostId: HOST_ID, + worktreeIds: [deletedOnHost.id] + }) + }) + + // A non-authoritative reply is a failed listing, not a report that a checkout is gone. + it('retires nothing when the runtime host could not scan', async () => { + const store = seedClientWithBothRows() + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-detected', + ok: true, + result: makeDetectedResult(REPO_ID, [live], { + authoritative: false, + source: 'metadata-fallback' + }), + _meta: { runtimeId: 'runtime-remote' } + }) + + await store.getState().fetchWorktrees(REPO_ID, { executionHostId: HOST_ID }) + + expect(forgetRemovedForExecutionHostMock).not.toHaveBeenCalled() + }) + + // `session-fallback` claims authoritative but is the truncated, visibility-filtered `worktree.list` + // reply from a host too old for `worktree.detectedList`. Its omissions prove nothing. + it('retires nothing from a legacy session-fallback listing', async () => { + const store = seedClientWithBothRows() + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-detected', + ok: true, + result: makeDetectedResult(REPO_ID, [live], { source: 'session-fallback' }), + _meta: { runtimeId: 'runtime-remote' } + }) + + await store.getState().fetchWorktrees(REPO_ID, { executionHostId: HOST_ID }) + + expect(forgetRemovedForExecutionHostMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/store/slices/worktrees/listing/authoritative-worktree-removal-memory.ts b/src/renderer/src/store/slices/worktrees/listing/authoritative-worktree-removal-memory.ts index 9793adb0e16..e2fae6b9f77 100644 --- a/src/renderer/src/store/slices/worktrees/listing/authoritative-worktree-removal-memory.ts +++ b/src/renderer/src/store/slices/worktrees/listing/authoritative-worktree-removal-memory.ts @@ -50,16 +50,18 @@ export function resetAuthoritativelyRemovedWorktreeMemoryForTests(): void { authoritativelyRemovedWorktreeIdsByHost.clear() } -// Why: SSH WorktreeMeta is exempt from gcStaleWorktreeMeta (persistence.ts:407,415) and outlives the remote -// worktree, so a scan-proven removal must retire the metadata itself — otherwise the next launch's fallback -// re-lists the deleted row before the host connects, and the in-memory suppression above is already gone. +// Why: off-host WorktreeMeta is exempt from gcStaleWorktreeMeta -- it skips any row whose repo or hostId is +// not local -- and outlives the remote worktree, so a scan-proven removal must retire the metadata itself. +// Otherwise the next launch's fallback re-lists the deleted row before the host connects, and the in-memory +// suppression above is already gone. Runtime hosts were excluded until #17776, which is why a paired client +// accumulated a row per remote worktree it had ever seen and never dropped one. export function forgetPersistedWorktreeMetaForRemovals( repoId: string, hostId: ExecutionHostId, worktreeIds: readonly string[] ): void { const parsedHost = parseExecutionHostId(hostId) - if (worktreeIds.length === 0 || parsedHost?.kind !== 'ssh') { + if (worktreeIds.length === 0 || (parsedHost?.kind !== 'ssh' && parsedHost?.kind !== 'runtime')) { return } const forget = window.api.worktrees.forgetRemovedForExecutionHost diff --git a/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts b/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts index 13e6b1572c1..fde831ee2e5 100644 --- a/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts +++ b/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts @@ -254,7 +254,14 @@ export function mergeFetchedWorktrees( // Why: applied outside the updater so a repeated updater call cannot double-apply the removal memory. forgetAuthoritativelyRemovedWorktrees(args.hostId, authoritativelySeenIds) rememberAuthoritativelyRemovedWorktrees(args.hostId, authoritativelyRemovedIds) - forgetPersistedWorktreeMetaForRemovals(args.repoId, args.hostId, authoritativelyRemovedIds) + // Only a real scan retires persisted metadata. `session-fallback` also reports authoritative, + // but it is the truncated, visibility-filtered `worktree.list` reply from a host too old for + // `worktree.detectedList` -- its omissions are not evidence a checkout is gone. + forgetPersistedWorktreeMetaForRemovals( + args.repoId, + args.hostId, + args.refresh.result.source === 'git' ? authoritativelyRemovedIds : [] + ) } return admitted } diff --git a/src/renderer/src/web/preload-api/web-ui-api.ts b/src/renderer/src/web/preload-api/web-ui-api.ts index 8c6e4d73a95..ca67f5664a9 100644 --- a/src/renderer/src/web/preload-api/web-ui-api.ts +++ b/src/renderer/src/web/preload-api/web-ui-api.ts @@ -157,6 +157,9 @@ export function createWebUiApi(): NonNullable['ui']> { consumePendingOpenSettings: () => Promise.resolve(false), onOpenSkillShare: () => noopUnsubscribe, consumePendingSkillShare: () => Promise.resolve(null), + // Why: the web client has no OS shell handing it files, so there is never a queued open. + onOpenMarkdownFiles: () => noopUnsubscribe, + consumePendingMarkdownFileOpens: () => Promise.resolve([]), onOpenSetupGuide: () => noopUnsubscribe, onOpenFeatureTour: () => noopUnsubscribe, onOpenCrashReport: () => noopUnsubscribe, diff --git a/src/shared/child-process/child-process-import-boundary.test.ts b/src/shared/child-process/child-process-import-boundary.test.ts index 4e261de5c8a..10ca4fd8522 100644 --- a/src/shared/child-process/child-process-import-boundary.test.ts +++ b/src/shared/child-process/child-process-import-boundary.test.ts @@ -23,10 +23,19 @@ const CHILD_PROCESS_IMPORT_ALLOWLIST: readonly string[] = readFileSync( .map((line) => line.trim()) .filter((line) => line.length > 0 && !line.startsWith('#')) +/** + * The true count of files importing child_process directly. + * + * May only ever be DECREASED, and only by migrating a file off + * `node:child_process`. Raising it is never the fix. + */ +const DIRECT_IMPORTER_PIN = 160 + const IMPORT_PATTERN = /(?:from\s+['"]node:child_process['"]|from\s+['"]child_process['"]|require\(\s*['"]node:child_process['"]|require\(\s*['"]child_process['"])/ -const OWNER_DIRECTORY = 'src/shared/child-process' +// Why: trailing slash, so a sibling like src/shared/child-process-foo.ts is scanned, not exempted. +const OWNER_DIRECTORY = 'src/shared/child-process/' const SCANNED_EXTENSIONS = ['.ts', '.tsx'] const IGNORED_DIRECTORIES = new Set([ 'node_modules', @@ -110,9 +119,22 @@ describe('child_process import boundary', () => { expect(stale, 'Allowlist entry no longer imports child_process — delete the line.').toEqual([]) }) - it('never grows', () => { - // The count is asserted separately from membership so a swap (one file - // migrated, one added) still fails loudly. - expect(offenders.length).toBeLessThanOrEqual(CHILD_PROCESS_IMPORT_ALLOWLIST.length) + it('holds the offender count at the pin', () => { + // Bounding by the allowlist's own length proves nothing: the two move + // together, so a swap (one file migrated off, one new file added with its + // entry) kept the bound satisfied. The pin is a literal for that reason. + expect( + offenders.length, + `${offenders.length} files import child_process directly; the pin is ${DIRECT_IMPORTER_PIN}. ` + + 'Never raise the pin -- migrate the file to runProcess/spawnProcess from ' + + 'src/shared/child-process instead.' + ).toBeLessThanOrEqual(DIRECT_IMPORTER_PIN) + // A pin left above reality is how a ratchet rots: it re-opens room for the + // next direct import to land for free. + expect( + offenders.length, + `Only ${offenders.length} files import child_process directly. Lower DIRECT_IMPORTER_PIN to ` + + `${offenders.length} to keep the ground you just took.` + ).toBeGreaterThanOrEqual(DIRECT_IMPORTER_PIN) }) }) diff --git a/src/shared/child-process/windows-console-visibility.test.ts b/src/shared/child-process/windows-console-visibility.test.ts index e98a97cfa98..7985b0ee11a 100644 --- a/src/shared/child-process/windows-console-visibility.test.ts +++ b/src/shared/child-process/windows-console-visibility.test.ts @@ -27,6 +27,15 @@ const ALLOWLIST: readonly string[] = readAllowlist( join(__dirname, '__fixtures__', 'windows-console-visibility-allowlist.txt') ) +/** + * The true count of files spawning without `windowsHide`. + * + * May only ever be DECREASED, and only by fixing a call site. Set equality with + * the allowlist does not bound this: a swap (one file fixed and delisted, one + * new file added with its entry) satisfies both membership assertions. + */ +const UNHIDDEN_SPAWNER_PIN = 68 + const CHILD_PROCESS_IMPORT = /from\s+['"](?:node:)?child_process['"]|require\(\s*['"](?:node:)?child_process['"]/ // Includes the promisified and renamed spellings -- `execAsync`, `spawnDetached`, @@ -166,4 +175,18 @@ describe('direct child-process calls hide the Windows console', () => { // A fixed file must leave the list, or the ratchet stops ratcheting. expect(ALLOWLIST.filter((path) => !offenders.includes(path))).toEqual([]) }) + + it('holds the offender count at the pin', () => { + expect( + offenders.length, + `${offenders.length} files spawn without windowsHide; the pin is ${UNHIDDEN_SPAWNER_PIN}. ` + + 'Never raise the pin -- add the flag, or route the call through run-process.ts.' + ).toBeLessThanOrEqual(UNHIDDEN_SPAWNER_PIN) + // A pin left above reality re-opens room for the next unguarded spawn. + expect( + offenders.length, + `Only ${offenders.length} files spawn without windowsHide. Lower UNHIDDEN_SPAWNER_PIN to ` + + `${offenders.length} to keep the ground you just took.` + ).toBeGreaterThanOrEqual(UNHIDDEN_SPAWNER_PIN) + }) }) diff --git a/src/shared/detected-worktree-provider-contract.ts b/src/shared/detected-worktree-provider-contract.ts index 9f146e35778..99b0f8fdc74 100644 --- a/src/shared/detected-worktree-provider-contract.ts +++ b/src/shared/detected-worktree-provider-contract.ts @@ -41,9 +41,15 @@ export type HostQualifiedKnownWorktreeResult = executionHostId: SshExecutionHostId } +/** + * Hosts whose persisted metadata a scan can retire: exactly those `gcStaleWorktreeMeta` skips, + * because it only ever condemns rows that are local on both the repo and the meta's `hostId`. + */ +export type OffHostExecutionHostId = Extract + export type ForgetRemovedWorktreesForExecutionHostArgs = { repoId: string - executionHostId: SshExecutionHostId + executionHostId: OffHostExecutionHostId /** Ids an authoritative scan of this host proved gone — the only evidence that retires persisted metadata. */ worktreeIds: readonly string[] } diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index debab394649..3a8f562dff1 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -15,6 +15,7 @@ import { isUnsupportedWorktreeListZError } from './git-worktree-command-capabilities' import { gitCredentialPromptGuardEnv } from './git-credential-prompt-env' +import { GIT_HISTORY_COMMIT_FORMAT, parseGitHistoryLog } from './git-history-log-parser' import { githubPullRequestHeadLocalRef, gitlabMergeRequestHeadLocalRef, @@ -378,4 +379,29 @@ describeBinaryCompatibility('real Git binary compatibility', () => { runGit(['show', '--end-of-options', `${pinnedOid}:absent.txt`]) ).rejects.toBeDefined() }) + // Why pin this: an older Git echoes %(decorate:…) and exits zero, so only %D + // in the same record carries the badges (#15507). Asserts the echo and the recovery. + it('reads commit decorations on both sides of the %(decorate:...) boundary', async () => { + await writeFile(join(repoPath, 'decorated.txt'), 'decorated\n') + await runGit(['add', 'decorated.txt']) + await runGit(['commit', '-qm', 'decorated commit']) + await runGit(['tag', 'compat-decorated']) + const head = (await runGit(['rev-parse', 'HEAD'])).stdout.trim() + + const log = await runGit([ + 'log', + `--format=${GIT_HISTORY_COMMIT_FORMAT}`, + '-z', + '--decorate=full', + '-n1', + head + ]) + + expect(log.stdout.includes('%(decorate')).toBe(!supports(2, 43)) + + const [item] = parseGitHistoryLog(log.stdout) + expect(item?.id).toBe(head) + expect(item?.subject).toBe('decorated commit') + expect(item?.references?.map((ref) => ref.id)).toContain('refs/tags/compat-decorated') + }) }) diff --git a/src/shared/git-history-log-parser.ts b/src/shared/git-history-log-parser.ts index 8f34002f0cc..ddc354513b9 100644 --- a/src/shared/git-history-log-parser.ts +++ b/src/shared/git-history-log-parser.ts @@ -2,9 +2,15 @@ import type { GitHistoryItem, GitHistoryItemRef } from './git-history-types' import { iterateNulDelimitedFields } from './nul-delimited-fields' const GIT_HISTORY_DECORATION_SEPARATOR = '\x1f' +const GIT_HISTORY_LEGACY_DECORATION_SEPARATOR = ',' +// Why %D too: %(decorate:…) is Git 2.43+, and older Git echoes it verbatim and exits zero. +// Callers must pass --decorate=full; both fields emit short names otherwise, which parse to no refs. export const GIT_HISTORY_COMMIT_FORMAT = - '%H%n%aN%n%aE%n%at%n%ct%n%P%n%(decorate:prefix=,suffix=,separator=%x1f)%n%B' + '%H%n%aN%n%aE%n%at%n%ct%n%P%n%(decorate:prefix=,suffix=,separator=%x1f)%n%D%n%B' + +// Why exact-match: no ref name may contain the \x1f an old Git echoes here. +const UNEXPANDED_DECORATE_PLACEHOLDER = `%(decorate:prefix=,suffix=,separator=${GIT_HISTORY_DECORATION_SEPARATOR})` export function shortGitHash(hash: string): string { return hash.slice(0, 7) @@ -15,17 +21,18 @@ function commitSubject(message: string): string { return firstLine || '(no commit message)' } -function parseGitDecorationRefs(raw: string, revision: string): GitHistoryItemRef[] { +function parseGitDecorationRefs( + raw: string, + revision: string, + separator: string +): GitHistoryItemRef[] { if (!raw.trim()) { return [] } const refs: GitHistoryItemRef[] = [] - // Why: Git permits commas in ref names, so Orca's git log format uses a - // control-character separator that Git ref names cannot contain. - const parts = raw.includes(GIT_HISTORY_DECORATION_SEPARATOR) - ? raw.split(GIT_HISTORY_DECORATION_SEPARATOR) - : raw.split(',') + // Why passed in: a lone decoration carries no separator, so sniffing `raw` split `feat,one`. + const parts = raw.split(separator) for (const part of parts) { const ref = part.trim() @@ -115,8 +122,10 @@ export function parseGitHistoryLog(stdout: string): GitHistoryItem[] { const authorEmail = lines[2] ?? '' const authorDateSeconds = Number.parseInt(lines[3] ?? '', 10) const parents = (lines[5] ?? '').trim() - const decorations = lines[6] ?? '' - const message = lines.slice(7).join('\n').replace(/\n$/, '') + const decorateField = lines[6] ?? '' + const isLegacyGit = decorateField === UNEXPANDED_DECORATE_PLACEHOLDER + const decorations = isLegacyGit ? (lines[7] ?? '') : decorateField + const message = lines.slice(8).join('\n').replace(/\n$/, '') items.push({ id: hash, @@ -127,7 +136,11 @@ export function parseGitHistoryLog(stdout: string): GitHistoryItem[] { authorEmail: authorEmail || undefined, displayId: shortGitHash(hash), timestamp: Number.isFinite(authorDateSeconds) ? authorDateSeconds * 1000 : undefined, - references: parseGitDecorationRefs(decorations, hash) + references: parseGitDecorationRefs( + decorations, + hash, + isLegacyGit ? GIT_HISTORY_LEGACY_DECORATION_SEPARATOR : GIT_HISTORY_DECORATION_SEPARATOR + ) }) } return items diff --git a/src/shared/git-history.test.ts b/src/shared/git-history.test.ts index 54aac202c71..617fa33c2ef 100644 --- a/src/shared/git-history.test.ts +++ b/src/shared/git-history.test.ts @@ -16,6 +16,7 @@ function logRecord({ hash, parents = [], decorations = '', + legacyDecorations = '', message, author = 'Ada Lovelace', timestamp = 1_700_000_000 @@ -23,6 +24,7 @@ function logRecord({ hash: string parents?: string[] decorations?: string + legacyDecorations?: string message: string author?: string timestamp?: number @@ -35,6 +37,7 @@ function logRecord({ String(timestamp), parents.join(' '), decorations, + legacyDecorations, message ].join('\n')}\0` } @@ -94,8 +97,12 @@ describe('git history parsing', () => { const stdout = logRecord({ hash: HEAD_OID, parents: [BASE_OID], - decorations: - 'HEAD -> refs/heads/feature, refs/remotes/origin/HEAD -> refs/remotes/origin/feature, refs/remotes/origin/feature, tag: refs/tags/v1.0.0', + decorations: [ + 'HEAD -> refs/heads/feature', + 'refs/remotes/origin/HEAD -> refs/remotes/origin/feature', + 'refs/remotes/origin/feature', + 'tag: refs/tags/v1.0.0' + ].join(DECORATION_SEPARATOR), message: 'feat: add graph\n\nbody line' }) @@ -117,6 +124,39 @@ describe('git history parsing', () => { ]) }) + it('falls back to %D decorations when Git predates the %(decorate:…) placeholder', () => { + // Why: Git < 2.43 echoes the placeholder and exits zero (#15507). + const stdout = logRecord({ + hash: HEAD_OID, + decorations: `%(decorate:prefix=,suffix=,separator=${DECORATION_SEPARATOR})`, + legacyDecorations: 'HEAD -> refs/heads/feature, tag: refs/tags/v1.0.0', + message: 'feat: add graph' + }) + + const [item] = parseGitHistoryLog(stdout) + + expect(item?.subject).toBe('feat: add graph') + expect(item?.references?.map((ref) => [ref.id, ref.name, ref.category])).toEqual([ + ['refs/heads/feature', 'feature', 'branches'], + ['refs/tags/v1.0.0', 'v1.0.0', 'tags'] + ]) + }) + + it('keeps a comma inside a lone decoration, which carries no separator', () => { + // Why: a lone decoration carries no separator, so sniffing for \x1f split it in two. + const stdout = logRecord({ + hash: HEAD_OID, + decorations: 'HEAD -> refs/heads/feat,one', + message: 'initial' + }) + + const [item] = parseGitHistoryLog(stdout) + + expect(item?.references?.map((ref) => [ref.id, ref.name])).toEqual([ + ['refs/heads/feat,one', 'feat,one'] + ]) + }) + it('preserves commas inside branch and tag decoration names', () => { const stdout = logRecord({ hash: HEAD_OID,