From aa95bdb11ad7b1b6604334399c00210041530900 Mon Sep 17 00:00:00 2001 From: JahyunBaek <60498711+JahyunBaek@users.noreply.github.com> Date: Sun, 30 Aug 2026 06:23:22 +0900 Subject: [PATCH 1/3] test(shared): stop two suites asserting POSIX separators on Windows (#16511) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both files describe paths with POSIX literals while their subjects compose paths through `node:path`, so the assertions only hold where the separator happens to be `/`. `node-markdown-document-discovery` keys its fake tree at `/repo/docs` and `/repo/one`, but `discoverMarkdownRelativePaths` descends with `join(absoluteDirectoryPath, entry.name)` — `\repo\docs` on win32. The child lookup misses, `readDirectory` yields nothing, and the walk stops at the root: `docs/guide.mdx` disappears and the depth-limit case never reaches its limit, so it resolves `[]` instead of rejecting. Keying the children with `join` walks the tree the subject actually walks. `git-fetch-head-lock` expects `cwd: '/tmp/repo'` from a subject that returns `path.resolve(cwd, 'repo')`, which is `C:\tmp\repo` on win32. Asserting through `path.resolve` pins the behaviour — that `-C` and `--git-dir` are resolved against the cwd — rather than the separator of whichever machine runs the suite. Verified on Windows 11: the two files go from 3 failed / 12 passed to 14 passed / 1 skipped, and the wider `src/shared` run shows no regression. Co-authored-by: Claude Opus 5 (1M context) --- src/shared/git-fetch-head-lock.test.ts | 6 ++++-- src/shared/node-markdown-document-discovery.test.ts | 7 +++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/shared/git-fetch-head-lock.test.ts b/src/shared/git-fetch-head-lock.test.ts index bcf1e13a7fc..4133d8d373e 100644 --- a/src/shared/git-fetch-head-lock.test.ts +++ b/src/shared/git-fetch-head-lock.test.ts @@ -26,13 +26,15 @@ describe('runWithGitFetchHeadLock', () => { expect(resolveGitFetchHeadCommand(args, '/repo').needsLock).toBe(expected) }) + // Why resolve() rather than a literal: the subject resolves these through path.resolve, so a + // POSIX literal asserts the separator of the machine running the suite instead of the behaviour. it('resolves -C and --git-dir before deriving the lock identity', () => { expect(resolveGitFetchHeadCommand(['-C', 'repo', 'fetch'], '/tmp')).toMatchObject({ - cwd: '/tmp/repo', + cwd: path.resolve('/tmp', 'repo'), needsLock: true }) expect(resolveGitFetchHeadCommand(['--git-dir=/repo/.git', 'fetch'], '/tmp')).toMatchObject({ - gitDir: '/repo/.git', + gitDir: path.resolve('/tmp', '/repo/.git'), needsLock: true }) }) diff --git a/src/shared/node-markdown-document-discovery.test.ts b/src/shared/node-markdown-document-discovery.test.ts index e151f25ad90..9c5e1bdfd14 100644 --- a/src/shared/node-markdown-document-discovery.test.ts +++ b/src/shared/node-markdown-document-discovery.test.ts @@ -1,4 +1,5 @@ import type { Dirent } from 'node:fs' +import { join } from 'node:path' import { describe, expect, it } from 'vitest' import { MarkdownDocumentListingCapacityError } from './markdown-document-listing-limits' import { discoverMarkdownRelativePaths } from './node-markdown-document-discovery' @@ -22,6 +23,8 @@ function reader(entriesByPath: Record) { describe('bounded Markdown document discovery', () => { it('preserves depth-first discovery and skips excluded and symlinked directories', async () => { + // Why join() for the child key: the subject descends with path.join, so a '/repo/docs' + // literal never matches on Windows and the walk silently stops at the root. const result = await discoverMarkdownRelativePaths('/repo', { readDirectory: reader({ '/repo': [ @@ -30,7 +33,7 @@ describe('bounded Markdown document discovery', () => { entry('docs', 'directory'), entry('linked', 'symlink') ], - '/repo/docs': [entry('guide.mdx'), entry('app.ts')] + [join('/repo', 'docs')]: [entry('guide.mdx'), entry('app.ts')] }), shouldDescend: (_relativePath, name) => name !== '.git' }) @@ -65,7 +68,7 @@ describe('bounded Markdown document discovery', () => { limits: { maxDepth: 1 }, readDirectory: reader({ '/repo': [entry('one', 'directory')], - '/repo/one': [entry('two', 'directory')] + [join('/repo', 'one')]: [entry('two', 'directory')] }), shouldDescend: () => true }) From 7655d20f04ffc11a6221c504652fb435c8e0d88d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 14:34:50 -0700 Subject: [PATCH 2/3] fix(terminal): complete OMP stale cwd recovery (#17154) --- config/reliability-gates.jsonc | 12 +- .../daemon-bash-rcfile.txt | 11 +- .../daemon-zsh-zshenv.txt | 11 +- .../local-bash-rcfile.txt | 11 +- .../local-zsh-zshenv.txt | 11 +- .../relay-bash-rcfile.txt | 11 +- .../relay-zsh-zshenv.txt | 11 +- .../pty/omp-shell-wrapper.node-pty.test.ts | 111 +++++++++++++----- src/main/pty/omp-shell-wrapper.ts | 11 +- 9 files changed, 141 insertions(+), 59 deletions(-) diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 10143d9019c..4dea7b1e3e2 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -6345,8 +6345,8 @@ "https://github.com/stablyai/orca/issues/16457", "https://github.com/stablyai/orca/pull/17128" ], - "invariant": "A fresh local renderer terminal spawn may recover from a saved startup cwd whose directory no longer exists only by spawning at the selected workspace root and printing a generic in-terminal notice; existing directories — including ones outside the worktree (#7685) — spawn as requested, and reattach, SSH, remote-runtime, runtime/API, and mobile callers keep exact cwd semantics. A long-lived POSIX shell whose cwd inode was deleted and replaced may run an extension-enabled OMP launch from the live path named by its logical PWD only in a subshell; the parent shell cwd, OMP argv, status extension, and exit status remain unchanged, while a genuinely unavailable path fails visibly without invoking OMP. A usable current directory remains authoritative when PWD is unset instead of being remapped to a workspace fallback.", - "oracle": "The shared resolver falls back to the workspace root only when the injected existence probe reports the resolved cwd missing and the workspace root present, and never probes floating terminals or a cwd equal to the root. The renderer sends cwdFallback only for fresh local IPC spawns, main honors it only when connectionId and sessionId are absent, WSL UNC paths never engage the probe-based fallback, main returns fallback metadata only after an actual fallback, the IPC transport preserves that metadata, and the connection layer writes a generic notice that omits the missing path. In real interactive Bash and Zsh PTYs, unset PWD in a usable nested directory and require OMP to stay there rather than remap to ORCA_WORKTREE_PATH; then delete and recreate the active project path, require OMP to observe the replacement inode with byte-exact extension argv and its nonzero status preserved, require the parent shell to remain on the stale inode, and finally delete the replacement and require an actionable failure before the fake OMP binary runs again.", + "invariant": "A fresh local renderer terminal spawn may recover from a saved startup cwd whose directory no longer exists only by spawning at the selected workspace root and printing a generic in-terminal notice; existing directories — including ones outside the worktree (#7685) — spawn as requested, and reattach, SSH, remote-runtime, runtime/API, and mobile callers keep exact cwd semantics. A long-lived POSIX shell whose cwd inode was deleted and replaced may run a wrapped OMP command from the live path named by its logical PWD only in a subshell; the parent shell cwd, OMP argv, status extension, and exit status remain unchanged, while a genuinely unavailable path fails visibly without invoking OMP. A usable physical cwd remains authoritative when PWD is unset or names a removed logical symlink instead of being remapped to a workspace fallback.", + "oracle": "The shared resolver falls back to the workspace root only when the injected existence probe reports the resolved cwd missing and the workspace root present, and never probes floating terminals or a cwd equal to the root. The renderer sends cwdFallback only for fresh local IPC spawns, main honors it only when connectionId and sessionId are absent, WSL UNC paths never engage the probe-based fallback, main returns fallback metadata only after an actual fallback, the IPC transport preserves that metadata, and the connection layer writes a generic notice that omits the missing path. In real interactive Bash and Zsh PTYs, unset PWD and remove a logical symlink while the physical cwd stays usable, requiring OMP to remain there; then delete and recreate the active project path, require extension-enabled and extension-skipping invocations to observe the replacement inode with byte-exact argv and nonzero status preserved, require the parent shell to remain on the stale inode, unset PWD and require recovery through ORCA_WORKTREE_PATH, clear every logical fallback and require an actionable failure without invoking OMP, and finally delete the replacement and require the same for the missing PWD path.", "commands": [ "pnpm exec vitest run --config config/vitest.config.ts src/shared/terminal-startup-cwd.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/pty-spawn-cwd-fallback.test.ts src/main/ipc/pty-wsl-cwd-validation.test.ts", @@ -6410,6 +6410,10 @@ "assertions": [ "real Bash and Zsh PTYs rebind OMP to a recreated cwd inode without changing the parent shell", "an unset PWD with a usable cwd stays unset and does not remap OMP to ORCA_WORKTREE_PATH", + "a removed logical symlink does not invalidate its still-usable physical cwd", + "extension-skipping commands such as omp --version still recover the cwd without gaining extension argv", + "an unset PWD on a stale inode recovers through ORCA_WORKTREE_PATH", + "an unset PWD on a stale inode without a logical fallback fails visibly before invoking OMP", "extension argv and a nonzero OMP exit status survive the recovery subshell", "a subsequently missing logical cwd prints an actionable error and does not invoke OMP again" ] @@ -6465,7 +6469,7 @@ "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/pty/omp-shell-wrapper.node-pty.test.ts src/main/pty/omp-shell-wrapper-alias-safety.test.ts src/main/shell-wrapper-generated-file-snapshot.test.ts", "result": "passed", "durationSeconds": 3, - "summary": "3 test files passed; real Bash and Zsh PTYs covered unset-PWD authority, stale-inode recovery, parent-shell isolation, extension argv, exit status, missing-path visibility, alias safety, and generated local/daemon/relay parity." + "summary": "3 test files passed; real Bash and Zsh PTYs covered usable physical cwd authority, stale-inode recovery with set and unset PWD, extension-enabled and extension-skipping argv, parent-shell isolation, exit status, missing-path visibility, alias safety, and generated local/daemon/relay parity." } ], "runtimeBudget": { @@ -6482,7 +6486,7 @@ }, "performanceBudget": { "required": true, - "evidence": "The spawn recovery adds at most two statSync probes on the fresh-local spawn path (the provider already stats the same paths during validation) and one bounded terminal write only when fallback actually occurs. Each extension-enabled OMP launch adds a constant number of shell filesystem predicates; only a detected stale/unusable cwd creates one short recovery subshell. No polling, retry, provider listing, hidden-pane work, startup awaits, extra OMP subprocesses, or render-loop work was added." + "evidence": "The spawn recovery adds at most two statSync probes on the fresh-local spawn path (the provider already stats the same paths during validation) and one bounded terminal write only when fallback actually occurs. Each wrapped OMP launch adds a constant number of shell filesystem predicates; only a detected stale/unusable cwd creates one short recovery subshell. No polling, retry, provider listing, hidden-pane work, startup awaits, extra OMP subprocesses, or render-loop work was added." }, "promotionCriteria": [ "Attach CI evidence for all declared test files.", diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/daemon-bash-rcfile.txt b/src/main/__fixtures__/shell-wrapper-snapshots/daemon-bash-rcfile.txt index b91f1065f84..54837bf4d65 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/daemon-bash-rcfile.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/daemon-bash-rcfile.txt @@ -45,11 +45,14 @@ __orca_omp_should_skip_extension() { return 1 } __orca_omp_cwd_is_usable() { + local __orca_physical_cwd [[ -x . ]] || return 1 - if [[ -n "${PWD:-}" ]]; then - [[ -d "${PWD}" && "${PWD}" -ef . ]] + if [[ -n "${PWD:-}" && -d "${PWD:-}" ]]; then + [[ "${PWD}" -ef . ]] else - builtin pwd -P >/dev/null 2>&1 + # Why compare the path: shell builtins can print a cached path for a deleted cwd. + __orca_physical_cwd="$(builtin pwd -P 2>/dev/null)" || return 1 + [[ -d "$__orca_physical_cwd" && "$__orca_physical_cwd" -ef . ]] fi } __orca_omp_invoke() { @@ -69,7 +72,7 @@ __orca_omp_invoke() { __orca_omp() { local __orca_use_extension=1 __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 - if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + if ! __orca_omp_cwd_is_usable; then local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" # Why: a restored shell can retain the deleted directory inode after its path is recreated. ( diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/daemon-zsh-zshenv.txt b/src/main/__fixtures__/shell-wrapper-snapshots/daemon-zsh-zshenv.txt index bce09f52ff0..3d3403ad099 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/daemon-zsh-zshenv.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/daemon-zsh-zshenv.txt @@ -83,11 +83,14 @@ __orca_deferred_init() { return 1 } __orca_omp_cwd_is_usable() { + local __orca_physical_cwd [[ -x . ]] || return 1 - if [[ -n "${PWD:-}" ]]; then - [[ -d "${PWD}" && "${PWD}" -ef . ]] + if [[ -n "${PWD:-}" && -d "${PWD:-}" ]]; then + [[ "${PWD}" -ef . ]] else - builtin pwd -P >/dev/null 2>&1 + # Why compare the path: shell builtins can print a cached path for a deleted cwd. + __orca_physical_cwd="$(builtin pwd -P 2>/dev/null)" || return 1 + [[ -d "$__orca_physical_cwd" && "$__orca_physical_cwd" -ef . ]] fi } __orca_omp_invoke() { @@ -107,7 +110,7 @@ __orca_deferred_init() { __orca_omp() { local __orca_use_extension=1 __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 - if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + if ! __orca_omp_cwd_is_usable; then local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" # Why: a restored shell can retain the deleted directory inode after its path is recreated. ( diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/local-bash-rcfile.txt b/src/main/__fixtures__/shell-wrapper-snapshots/local-bash-rcfile.txt index ee9cb7975ec..d4bb98997c2 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/local-bash-rcfile.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/local-bash-rcfile.txt @@ -48,11 +48,14 @@ __orca_omp_should_skip_extension() { return 1 } __orca_omp_cwd_is_usable() { + local __orca_physical_cwd [[ -x . ]] || return 1 - if [[ -n "${PWD:-}" ]]; then - [[ -d "${PWD}" && "${PWD}" -ef . ]] + if [[ -n "${PWD:-}" && -d "${PWD:-}" ]]; then + [[ "${PWD}" -ef . ]] else - builtin pwd -P >/dev/null 2>&1 + # Why compare the path: shell builtins can print a cached path for a deleted cwd. + __orca_physical_cwd="$(builtin pwd -P 2>/dev/null)" || return 1 + [[ -d "$__orca_physical_cwd" && "$__orca_physical_cwd" -ef . ]] fi } __orca_omp_invoke() { @@ -72,7 +75,7 @@ __orca_omp_invoke() { __orca_omp() { local __orca_use_extension=1 __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 - if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + if ! __orca_omp_cwd_is_usable; then local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" # Why: a restored shell can retain the deleted directory inode after its path is recreated. ( diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/local-zsh-zshenv.txt b/src/main/__fixtures__/shell-wrapper-snapshots/local-zsh-zshenv.txt index 222cab93896..ee18600acb1 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/local-zsh-zshenv.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/local-zsh-zshenv.txt @@ -83,11 +83,14 @@ __orca_deferred_init() { return 1 } __orca_omp_cwd_is_usable() { + local __orca_physical_cwd [[ -x . ]] || return 1 - if [[ -n "${PWD:-}" ]]; then - [[ -d "${PWD}" && "${PWD}" -ef . ]] + if [[ -n "${PWD:-}" && -d "${PWD:-}" ]]; then + [[ "${PWD}" -ef . ]] else - builtin pwd -P >/dev/null 2>&1 + # Why compare the path: shell builtins can print a cached path for a deleted cwd. + __orca_physical_cwd="$(builtin pwd -P 2>/dev/null)" || return 1 + [[ -d "$__orca_physical_cwd" && "$__orca_physical_cwd" -ef . ]] fi } __orca_omp_invoke() { @@ -107,7 +110,7 @@ __orca_deferred_init() { __orca_omp() { local __orca_use_extension=1 __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 - if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + if ! __orca_omp_cwd_is_usable; then local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" # Why: a restored shell can retain the deleted directory inode after its path is recreated. ( diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/relay-bash-rcfile.txt b/src/main/__fixtures__/shell-wrapper-snapshots/relay-bash-rcfile.txt index 9d90929085d..de9c8f95248 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/relay-bash-rcfile.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/relay-bash-rcfile.txt @@ -37,11 +37,14 @@ __orca_omp_should_skip_extension() { return 1 } __orca_omp_cwd_is_usable() { + local __orca_physical_cwd [[ -x . ]] || return 1 - if [[ -n "${PWD:-}" ]]; then - [[ -d "${PWD}" && "${PWD}" -ef . ]] + if [[ -n "${PWD:-}" && -d "${PWD:-}" ]]; then + [[ "${PWD}" -ef . ]] else - builtin pwd -P >/dev/null 2>&1 + # Why compare the path: shell builtins can print a cached path for a deleted cwd. + __orca_physical_cwd="$(builtin pwd -P 2>/dev/null)" || return 1 + [[ -d "$__orca_physical_cwd" && "$__orca_physical_cwd" -ef . ]] fi } __orca_omp_invoke() { @@ -61,7 +64,7 @@ __orca_omp_invoke() { __orca_omp() { local __orca_use_extension=1 __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 - if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + if ! __orca_omp_cwd_is_usable; then local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" # Why: a restored shell can retain the deleted directory inode after its path is recreated. ( diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/relay-zsh-zshenv.txt b/src/main/__fixtures__/shell-wrapper-snapshots/relay-zsh-zshenv.txt index 335b54c60c3..394bc4a6d10 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/relay-zsh-zshenv.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/relay-zsh-zshenv.txt @@ -57,11 +57,14 @@ __orca_deferred_init() { return 1 } __orca_omp_cwd_is_usable() { + local __orca_physical_cwd [[ -x . ]] || return 1 - if [[ -n "${PWD:-}" ]]; then - [[ -d "${PWD}" && "${PWD}" -ef . ]] + if [[ -n "${PWD:-}" && -d "${PWD:-}" ]]; then + [[ "${PWD}" -ef . ]] else - builtin pwd -P >/dev/null 2>&1 + # Why compare the path: shell builtins can print a cached path for a deleted cwd. + __orca_physical_cwd="$(builtin pwd -P 2>/dev/null)" || return 1 + [[ -d "$__orca_physical_cwd" && "$__orca_physical_cwd" -ef . ]] fi } __orca_omp_invoke() { @@ -81,7 +84,7 @@ __orca_deferred_init() { __orca_omp() { local __orca_use_extension=1 __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 - if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + if ! __orca_omp_cwd_is_usable; then local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" # Why: a restored shell can retain the deleted directory inode after its path is recreated. ( diff --git a/src/main/pty/omp-shell-wrapper.node-pty.test.ts b/src/main/pty/omp-shell-wrapper.node-pty.test.ts index 6ab30dfdbae..15bc98bdb3b 100644 --- a/src/main/pty/omp-shell-wrapper.node-pty.test.ts +++ b/src/main/pty/omp-shell-wrapper.node-pty.test.ts @@ -1,11 +1,13 @@ import { spawnSync } from 'node:child_process' import { chmodSync, + existsSync, mkdtempSync, mkdirSync, readFileSync, realpathSync, rmSync, + symlinkSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' @@ -334,18 +336,82 @@ exit 0 const homeDir = join(tempDir, 'home') const binDir = join(tempDir, 'bin') const extensionDir = join(tempDir, 'extensions') + const logicalProjectLink = join(tempDir, 'logical-project') mkdirSync(projectDir, { recursive: true }) mkdirSync(homeDir) mkdirSync(binDir) mkdirSync(extensionDir) + symlinkSync(projectDir, logicalProjectLink) const expectedProjectDir = realpathSync(projectDir) + const expectedWorkspaceDir = realpathSync(workspaceDir) const statusExtension = join(extensionDir, 'orca-agent-status.ts') writeFileSync(statusExtension, 'export default {}') writeFakeOmp(binDir) const unsetPwdCaptureFile = join(tempDir, 'unset-pwd-capture') + const deletedLinkCaptureFile = join(tempDir, 'deleted-link-capture') const staleCaptureFile = join(tempDir, 'stale-cwd-capture') + const skipCaptureFile = join(tempDir, 'skip-capture') + const staleUnsetCaptureFile = join(tempDir, 'stale-unset-capture') + const noLogicalCaptureFile = join(tempDir, 'no-logical-capture') const resultFile = join(tempDir, 'stale-cwd-result') + const scenarioFile = join(tempDir, 'stale-cwd-scenario') + writeFileSync( + scenarioFile, + `ORCA_CAPTURE_FILE="$ORCA_UNSET_PWD_CAPTURE_FILE" +unset PWD +omp +__orca_test_unset_status=$? +if [[ -z "\${PWD+x}" ]]; then + __orca_test_pwd_state=unset +else + __orca_test_pwd_state=set +fi +builtin cd -- "$ORCA_LOGICAL_PROJECT_LINK" +/bin/rm -- "$ORCA_LOGICAL_PROJECT_LINK" +ORCA_CAPTURE_FILE="$ORCA_DELETED_LINK_CAPTURE_FILE" +omp +__orca_test_deleted_link_status=$? +builtin cd -P -- "$ORCA_STALE_PROJECT_DIR" +ORCA_CAPTURE_FILE="$ORCA_STALE_CAPTURE_FILE" +/bin/rm -rf -- "$ORCA_STALE_PROJECT_DIR" +/bin/mkdir -p -- "$ORCA_STALE_PROJECT_DIR" +omp +__orca_test_first_status=$? +ORCA_CAPTURE_FILE="$ORCA_SKIP_CAPTURE_FILE" +omp --version +__orca_test_skip_status=$? +if [[ "$PWD" -ef . ]]; then + __orca_test_parent_state=live +else + __orca_test_parent_state=stale +fi +unset PWD +ORCA_CAPTURE_FILE="$ORCA_STALE_UNSET_CAPTURE_FILE" +omp +__orca_test_stale_unset_status=$? +unset ORCA_WORKTREE_PATH ORCA_ROOT_PATH +ORCA_CAPTURE_FILE="$ORCA_NO_LOGICAL_CAPTURE_FILE" +omp +__orca_test_no_logical_status=$? +PWD="$ORCA_STALE_PROJECT_DIR" +/bin/rm -rf -- "$ORCA_STALE_PROJECT_DIR" +omp +__orca_test_missing_status=$? +{ + echo "UNSET=$__orca_test_unset_status" + echo "PWD=$__orca_test_pwd_state" + echo "LINK=$__orca_test_deleted_link_status" + echo "FIRST=$__orca_test_first_status" + echo "SKIP=$__orca_test_skip_status" + echo "PARENT=$__orca_test_parent_state" + echo "STALE_UNSET=$__orca_test_stale_unset_status" + echo "NO_LOGICAL=$__orca_test_no_logical_status" + echo "MISSING=$__orca_test_missing_status" +} > "$ORCA_RESULT_FILE" +exit 0 +` + ) const output = await runInteractivePosixPty({ shell, cwd: projectDir, @@ -355,47 +421,30 @@ ${getPosixOmpShellWrapper()}`, INPUTRC: '/dev/null', PROMPT_COMMAND: '', ORCA_STALE_PROJECT_DIR: projectDir, + ORCA_LOGICAL_PROJECT_LINK: logicalProjectLink, ORCA_UNSET_PWD_CAPTURE_FILE: unsetPwdCaptureFile, + ORCA_DELETED_LINK_CAPTURE_FILE: deletedLinkCaptureFile, ORCA_STALE_CAPTURE_FILE: staleCaptureFile, + ORCA_SKIP_CAPTURE_FILE: skipCaptureFile, + ORCA_STALE_UNSET_CAPTURE_FILE: staleUnsetCaptureFile, + ORCA_NO_LOGICAL_CAPTURE_FILE: noLogicalCaptureFile, ORCA_WORKTREE_PATH: workspaceDir, HOME: homeDir, PATH: `${binDir}:/usr/bin:/bin:/usr/sbin:/sbin`, ORCA_OMP_STATUS_EXTENSION: statusExtension, ORCA_CAPTURE_FILE: staleCaptureFile, ORCA_RESULT_FILE: resultFile, + ORCA_SCENARIO_FILE: scenarioFile, ORCA_TEST_FAKE_OMP_EXIT_CODE: '23', TERM: 'xterm-256color' }, - input: `ORCA_CAPTURE_FILE="$ORCA_UNSET_PWD_CAPTURE_FILE" -unset PWD -omp -__orca_test_unset_status=$? -if [[ -z "\${PWD+x}" ]]; then - __orca_test_pwd_state=unset -else - __orca_test_pwd_state=set -fi -builtin cd -P -- "$ORCA_STALE_PROJECT_DIR" -ORCA_CAPTURE_FILE="$ORCA_STALE_CAPTURE_FILE" -/bin/rm -rf -- "$ORCA_STALE_PROJECT_DIR" -/bin/mkdir -p -- "$ORCA_STALE_PROJECT_DIR" -omp -__orca_test_first_status=$? -if [[ "$PWD" -ef . ]]; then - __orca_test_parent_state=live -else - __orca_test_parent_state=stale -fi -/bin/rm -rf -- "$ORCA_STALE_PROJECT_DIR" -omp -__orca_test_missing_status=$? -printf 'UNSET=%s\nPWD=%s\nFIRST=%s\nPARENT=%s\nMISSING=%s\n' "$__orca_test_unset_status" "$__orca_test_pwd_state" "$__orca_test_first_status" "$__orca_test_parent_state" "$__orca_test_missing_status" > "$ORCA_RESULT_FILE" -exit 0 -` + input: 'source "$ORCA_SCENARIO_FILE"\n' }) const unsetPwdCapture = readFileSync(unsetPwdCaptureFile, 'utf8') expect(unsetPwdCapture).toContain(`CWD=${expectedProjectDir}`) + const deletedLinkCapture = readFileSync(deletedLinkCaptureFile, 'utf8') + expect(deletedLinkCapture).toContain(`CWD=${expectedProjectDir}`) const staleCapture = readFileSync(staleCaptureFile, 'utf8') expect(staleCapture).toContain(`CWD=${expectedProjectDir}`) expect(staleCapture.split('\n').filter((line) => line.startsWith('ARG'))).toEqual([ @@ -403,9 +452,17 @@ exit 0 `ARG2=${statusExtension}` ]) expect(staleCapture).not.toContain('--cwd') + const skipCapture = readFileSync(skipCaptureFile, 'utf8') + expect(skipCapture).toContain(`CWD=${expectedProjectDir}`) + expect(skipCapture).toContain('ARG1=--version') + expect(skipCapture).not.toContain('--extension') + const staleUnsetCapture = readFileSync(staleUnsetCaptureFile, 'utf8') + expect(staleUnsetCapture).toContain(`CWD=${expectedWorkspaceDir}`) + expect(existsSync(noLogicalCaptureFile)).toBe(false) expect(readFileSync(resultFile, 'utf8')).toBe( - 'UNSET=23\nPWD=unset\nFIRST=23\nPARENT=stale\nMISSING=1\n' + 'UNSET=23\nPWD=unset\nLINK=23\nFIRST=23\nSKIP=23\nPARENT=stale\nSTALE_UNSET=23\nNO_LOGICAL=1\nMISSING=1\n' ) + expect(output).toContain('no terminal working directory is available') expect(output).toContain('Orca: OMP cannot access the terminal working directory') } diff --git a/src/main/pty/omp-shell-wrapper.ts b/src/main/pty/omp-shell-wrapper.ts index fb6d274dc57..f5bc25421bb 100644 --- a/src/main/pty/omp-shell-wrapper.ts +++ b/src/main/pty/omp-shell-wrapper.ts @@ -52,11 +52,14 @@ __orca_omp_should_skip_extension() { return 1 } __orca_omp_cwd_is_usable() { + local __orca_physical_cwd [[ -x . ]] || return 1 - if [[ -n "\${PWD:-}" ]]; then - [[ -d "\${PWD}" && "\${PWD}" -ef . ]] + if [[ -n "\${PWD:-}" && -d "\${PWD:-}" ]]; then + [[ "\${PWD}" -ef . ]] else - builtin pwd -P >/dev/null 2>&1 + # Why compare the path: shell builtins can print a cached path for a deleted cwd. + __orca_physical_cwd="$(builtin pwd -P 2>/dev/null)" || return 1 + [[ -d "$__orca_physical_cwd" && "$__orca_physical_cwd" -ef . ]] fi } __orca_omp_invoke() { @@ -76,7 +79,7 @@ __orca_omp_invoke() { __orca_omp() { local __orca_use_extension=1 __orca_omp_should_skip_extension "\${1:-}" && __orca_use_extension=0 - if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + if ! __orca_omp_cwd_is_usable; then local __orca_logical_cwd="\${PWD:-\${ORCA_WORKTREE_PATH:-\${ORCA_ROOT_PATH:-}}}" # Why: a restored shell can retain the deleted directory inode after its path is recreated. ( From e3757a1d30224e5686974fd491c82aab0cc64395 Mon Sep 17 00:00:00 2001 From: JahyunBaek <60498711+JahyunBaek@users.noreply.github.com> Date: Sun, 30 Aug 2026 06:36:24 +0900 Subject: [PATCH 3/3] fix(worktree): ignore orphan remote-tracking refs in branch conflicts (#16699) Refs #16646 Unify native, WSL, and direct SSH conflict checks behind the execution host, remove the duplicated SSH classifier, and cover orphan/configured remote behavior across both paths. --- src/main/git/repo-base-ref-search.ts | 23 ++++- src/main/git/repo-branch-conflict.ts | 63 +++++++----- src/main/git/repo.test.ts | 37 +++++++ ...orktree-remote-ssh-branch-conflict.test.ts | 75 +++++++++++++++ src/main/ipc/worktree-remote.ts | 96 ++----------------- 5 files changed, 181 insertions(+), 113 deletions(-) create mode 100644 src/main/ipc/worktree-remote-ssh-branch-conflict.test.ts diff --git a/src/main/git/repo-base-ref-search.ts b/src/main/git/repo-base-ref-search.ts index 5ea01a7ec1c..213868fe5a7 100644 --- a/src/main/git/repo-base-ref-search.ts +++ b/src/main/git/repo-base-ref-search.ts @@ -218,6 +218,21 @@ export function parseAndFilterSearchRefDetails( .slice(0, Math.max(0, limit)) } +export function resolveConfiguredRemoteBranchName( + fullRef: string, + longestFirstRemoteNames: readonly string[] +): string | null { + const remoteRefPrefix = 'refs/remotes/' + if (!fullRef.startsWith(remoteRefPrefix)) { + return null + } + const remoteAndBranch = fullRef.slice(remoteRefPrefix.length) + const remote = longestFirstRemoteNames.find((candidate) => + remoteAndBranch.startsWith(`${candidate}/`) + ) + return remote ? remoteAndBranch.slice(remote.length + 1) || null : null +} + export function resolveLocalBranchName( fullRef: string, shortRef: string, @@ -227,11 +242,11 @@ export function resolveLocalBranchName( if (!fullRef.startsWith(remoteRefPrefix)) { return shortRef } - const remoteAndBranch = fullRef.slice(remoteRefPrefix.length) - const remote = remotes.find((candidate) => remoteAndBranch.startsWith(`${candidate}/`)) - if (remote) { - return remoteAndBranch.slice(remote.length + 1) + const configuredBranch = resolveConfiguredRemoteBranchName(fullRef, remotes) + if (configuredBranch) { + return configuredBranch } + const remoteAndBranch = fullRef.slice(remoteRefPrefix.length) return remoteAndBranch.split('/').slice(1).join('/') || shortRef } diff --git a/src/main/git/repo-branch-conflict.ts b/src/main/git/repo-branch-conflict.ts index dbadf254d11..fcb6d8dc101 100644 --- a/src/main/git/repo-branch-conflict.ts +++ b/src/main/git/repo-branch-conflict.ts @@ -1,45 +1,50 @@ -import { listRemoteNames, resolveLocalBranchName } from './repo-base-ref-search' -import { gitExecOptions, type LocalGitExecOptions } from './repo-default-base-ref' +import { resolveConfiguredRemoteBranchName } from './repo-base-ref-search' +import { gitExecOptions, type GitExec, type LocalGitExecOptions } from './repo-default-base-ref' import { gitExecFileAsync } from './runner' export type BranchConflictKind = 'local' | 'remote' -async function hasGitRefAsync( - path: string, - ref: string, - options: LocalGitExecOptions = {} -): Promise { +async function hasGitRefAsync(exec: GitExec, ref: string): Promise { try { - await gitExecFileAsync(['rev-parse', '--verify', ref], gitExecOptions(path, options)) - return true + const { stdout } = await exec(['rev-parse', '--verify', ref]) + return stdout.trim().length > 0 } catch { return false } } -export async function getBranchConflictKind( - path: string, +async function listRemoteNamesViaExec(exec: GitExec): Promise { + try { + const { stdout } = await exec(['remote']) + return stdout + .split(/\r?\n/) + .map((line) => line.trim()) + .filter(Boolean) + .sort((a, b) => b.length - a.length) + } catch { + return [] + } +} + +/** Run branch-conflict policy through the host that owns Git execution. */ +export async function getBranchConflictKindViaExec( + exec: GitExec, branchName: string, - allowedBaseRef?: string, - options: LocalGitExecOptions = {} + allowedBaseRef?: string ): Promise { - if (await hasGitRefAsync(path, `refs/heads/${branchName}`, options)) { + if (await hasGitRefAsync(exec, `refs/heads/${branchName}`)) { return 'local' } try { - const remoteNames = (await listRemoteNames(path, options)).sort((a, b) => b.length - a.length) - const { stdout } = await gitExecFileAsync( - ['for-each-ref', '--format=%(refname)', 'refs/remotes'], - gitExecOptions(path, options) - ) - const hasRemoteConflict = stdout.split('\n').some((ref) => { + const remoteNames = await listRemoteNamesViaExec(exec) + const { stdout } = await exec(['for-each-ref', '--format=%(refname)', 'refs/remotes']) + const hasRemoteConflict = stdout.split(/\r?\n/).some((ref) => { const trimmed = ref.trim() if (isAllowedRemoteBaseRef(trimmed, allowedBaseRef)) { return false } - const shortRef = trimmed.replace(/^refs\/remotes\//, '') - return resolveLocalBranchName(trimmed, shortRef, remoteNames) === branchName + return resolveConfiguredRemoteBranchName(trimmed, remoteNames) === branchName }) return hasRemoteConflict ? 'remote' : null @@ -48,6 +53,20 @@ export async function getBranchConflictKind( } } +export function getBranchConflictKind( + path: string, + branchName: string, + allowedBaseRef?: string, + options: LocalGitExecOptions = {} +): Promise { + const execOptions = gitExecOptions(path, options) + return getBranchConflictKindViaExec( + (argv) => gitExecFileAsync(argv, execOptions), + branchName, + allowedBaseRef + ) +} + function isAllowedRemoteBaseRef(refName: string, allowedBaseRef: string | undefined): boolean { if (!allowedBaseRef) { return false diff --git a/src/main/git/repo.test.ts b/src/main/git/repo.test.ts index 90199d1c69b..7500735d72f 100644 --- a/src/main/git/repo.test.ts +++ b/src/main/git/repo.test.ts @@ -215,6 +215,7 @@ describe('searchBaseRefs (widened glob)', () => { it('allows creating a local branch from the selected matching remote base ref', async () => { const sha = getHeadSha(tmpDir) + git(tmpDir, ['remote', 'add', 'origin', 'https://example.invalid/repo.git']) createRemoteRef(tmpDir, 'origin/feature/something', sha) const result = await getBranchConflictKind( @@ -228,6 +229,10 @@ describe('searchBaseRefs (widened glob)', () => { it('still reports a remote conflict for a different tracking ref with the same branch name', async () => { const sha = getHeadSha(tmpDir) + // Why register both: a ref only tracks a branch when a remote actually owns its + // prefix, and this case is about a second *tracking* ref, not an orphan. + git(tmpDir, ['remote', 'add', 'origin', 'https://example.invalid/repo.git']) + git(tmpDir, ['remote', 'add', 'upstream', 'https://example.invalid/upstream.git']) createRemoteRef(tmpDir, 'origin/feature/something', sha) createRemoteRef(tmpDir, 'upstream/feature/something', sha) @@ -236,6 +241,38 @@ describe('searchBaseRefs (widened glob)', () => { ).toBe('remote') }) + it('ignores a ref whose prefix matches no configured remote', async () => { + const sha = getHeadSha(tmpDir) + git(tmpDir, ['remote', 'add', 'origin', 'https://example.invalid/repo.git']) + createRemoteRef(tmpDir, 'mimic-fork/my-task', sha) + + expect(await getBranchConflictKind(tmpDir, 'my-task', 'origin/main')).toBeNull() + }) + + it('stops treating leftover refs from a removed remote as conflicts', async () => { + const sha = getHeadSha(tmpDir) + git(tmpDir, ['remote', 'add', 'origin', 'https://example.invalid/repo.git']) + git(tmpDir, ['remote', 'add', 'fork', 'https://example.invalid/fork.git']) + createRemoteRef(tmpDir, 'fork/my-task', sha) + expect(await getBranchConflictKind(tmpDir, 'my-task', 'origin/main')).toBe('remote') + + // Why not `remote remove`: that prunes the tracking refs too, which would pass + // with or without the ownership guard. Dropping only the config leaves the + // orphan ref behind, which is the state this guard exists for. + git(tmpDir, ['config', '--remove-section', 'remote.fork']) + + expect(await getBranchConflictKind(tmpDir, 'my-task', 'origin/main')).toBeNull() + }) + + it('still reports a local conflict when an unrelated ref shares the name', async () => { + const sha = getHeadSha(tmpDir) + git(tmpDir, ['remote', 'add', 'origin', 'https://example.invalid/repo.git']) + createRemoteRef(tmpDir, 'mimic-fork/my-task', sha) + git(tmpDir, ['branch', 'my-task']) + + expect(await getBranchConflictKind(tmpDir, 'my-task', 'origin/main')).toBe('local') + }) + it('reports remote conflicts when the remote name contains a slash', async () => { const sha = getHeadSha(tmpDir) git(tmpDir, ['remote', 'add', 'foo/bar', 'https://example.invalid/repo.git']) diff --git a/src/main/ipc/worktree-remote-ssh-branch-conflict.test.ts b/src/main/ipc/worktree-remote-ssh-branch-conflict.test.ts new file mode 100644 index 00000000000..7ced18eb173 --- /dev/null +++ b/src/main/ipc/worktree-remote-ssh-branch-conflict.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it, vi } from 'vitest' + +import { getSshBranchConflictKind } from './worktree-remote' + +type ConflictProvider = Parameters[0] + +function providerAnswering(answers: { + remotes: string[] + localBranchHead?: string + remoteRefs: string[] +}): ConflictProvider { + const exec: ConflictProvider['exec'] = vi.fn(async (args: string[]) => { + if (args[0] === 'remote') { + return { stdout: `${answers.remotes.join('\n')}\n`, stderr: '' } + } + if (args[0] === 'rev-parse') { + if (!answers.localBranchHead) { + throw new Error('missing ref') + } + return { stdout: `${answers.localBranchHead}\n`, stderr: '' } + } + if (args[0] === 'for-each-ref') { + return { stdout: `${answers.remoteRefs.join('\n')}\n`, stderr: '' } + } + throw new Error(`unexpected git ${args.join(' ')}`) + }) + return { exec } +} + +describe('getSshBranchConflictKind remote ownership', () => { + it('reports a local branch before scanning remote refs', async () => { + const provider = providerAnswering({ + localBranchHead: 'abc123', + remotes: ['origin'], + remoteRefs: [] + }) + + await expect( + getSshBranchConflictKind(provider, '/srv/repo', 'my-task', 'origin/main') + ).resolves.toBe('local') + }) + + it('ignores a ref whose prefix matches no configured remote', async () => { + const provider = providerAnswering({ + remotes: ['origin'], + remoteRefs: ['refs/remotes/origin/main', 'refs/remotes/deleted-fork/my-task'] + }) + + await expect( + getSshBranchConflictKind(provider, '/srv/repo', 'my-task', 'origin/main') + ).resolves.toBeNull() + }) + + it('still reports a ref owned by a configured remote', async () => { + const provider = providerAnswering({ + remotes: ['origin'], + remoteRefs: ['refs/remotes/origin/main', 'refs/remotes/origin/my-task'] + }) + + await expect( + getSshBranchConflictKind(provider, '/srv/repo', 'my-task', 'origin/main') + ).resolves.toBe('remote') + }) + + it('prefers the longest configured remote name', async () => { + const provider = providerAnswering({ + remotes: ['foo', 'foo/bar'], + remoteRefs: ['refs/remotes/foo/bar/my-task'] + }) + + await expect( + getSshBranchConflictKind(provider, '/srv/repo', 'my-task', 'origin/main') + ).resolves.toBe('remote') + }) +}) diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 581872540df..d9908b7c05c 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -34,6 +34,7 @@ import { resolveDefaultBaseRefViaExec, resolveDefaultBaseRefWithLocalGit } from '../git/repo' +import { getBranchConflictKindViaExec } from '../git/repo-branch-conflict' import { resolveLocalGitUsername, getSshGitUsername } from '../git/git-username' import { hasCommitObjectViaGitExec } from '../git/commit-object-ref' import { probeWorktreeBaseRefPresence } from '../git/worktree-base-ref-probe' @@ -839,98 +840,19 @@ async function canCheckoutExistingLocalBranchSsh( return !worktrees.some((worktree) => normalizeLocalBranchName(worktree.branch) === branchName) } -async function listSshRemoteNames(provider: SshGitProvider, repoPath: string): Promise { - try { - const { stdout } = await provider.exec(['remote'], repoPath) - return stdout - .split(/\r?\n/) - .map((line) => line.trim()) - .filter(Boolean) - .sort((a, b) => b.length - a.length) - } catch { - return [] - } -} +type SshGitExecutor = Pick -function isAllowedSshRemoteBaseRef(refName: string, allowedBaseRef: string): boolean { - if (!allowedBaseRef) { - return false - } - const normalizedAllowedRef = allowedBaseRef.startsWith('refs/remotes/') - ? allowedBaseRef - : `refs/remotes/${allowedBaseRef}` - return refName === normalizedAllowedRef -} - -function resolveSshRemoteBranchName(refName: string, remoteNames: string[]): string { - const remotePrefix = 'refs/remotes/' - if (!refName.startsWith(remotePrefix)) { - return refName - } - const remoteAndBranch = refName.slice(remotePrefix.length) - const remote = remoteNames.find((candidate) => remoteAndBranch.startsWith(`${candidate}/`)) - if (remote) { - return remoteAndBranch.slice(remote.length + 1) - } - return remoteAndBranch.split('/').slice(1).join('/') || remoteAndBranch -} - -async function hasSshRemoteBranchConflict( - provider: SshGitProvider, - repoPath: string, - branchName: string, - allowedBaseRef: string -): Promise { - const remoteNames = await listSshRemoteNames(provider, repoPath) - try { - const { stdout } = await provider.exec( - ['for-each-ref', '--format=%(refname)', 'refs/remotes'], - repoPath - ) - return stdout.split(/\r?\n/).some((line) => { - const refName = line.trim() - if (!refName || /^refs\/remotes\/.+\/HEAD$/.test(refName)) { - return false - } - if (isAllowedSshRemoteBaseRef(refName, allowedBaseRef)) { - return false - } - // Why: `git branch --all --list feature/x` doesn't match `remotes/origin/feature/x`; parse remote refs directly. - return resolveSshRemoteBranchName(refName, remoteNames) === branchName - }) - } catch { - return false - } -} - -async function hasSshLocalBranchConflict( - provider: SshGitProvider, - repoPath: string, - branchName: string -): Promise { - try { - const { stdout } = await provider.exec( - ['rev-parse', '--verify', '--quiet', `refs/heads/${branchName}^{commit}`], - repoPath - ) - return stdout.trim().length > 0 - } catch { - return false - } -} - -async function getSshBranchConflictKind( - provider: SshGitProvider, +export function getSshBranchConflictKind( + provider: SshGitExecutor, repoPath: string, branchName: string, allowedBaseRef: string ): Promise<'local' | 'remote' | null> { - if (await hasSshLocalBranchConflict(provider, repoPath, branchName)) { - return 'local' - } - return (await hasSshRemoteBranchConflict(provider, repoPath, branchName, allowedBaseRef)) - ? 'remote' - : null + return getBranchConflictKindViaExec( + (argv) => provider.exec(argv, repoPath), + branchName, + allowedBaseRef + ) } type SelectedReviewBranchInput = Pick<