fix(sidebar): route project adds to the intended host, not the global runtime (#9541)

* fix(sidebar): route local folder adds to the intended host, not the global runtime

Adding a local folder while connected to a remote runtime failed with
"<path> was checked on <host>, but that host did not report a usable folder"
because addRepoPath decides local-vs-remote purely from the global
settings.activeRuntimeEnvironmentId when no explicit host is passed.

Two local-add flows relied on that global fallback and got misrouted:

- useAddRepoLocalFolderFlow (native picker / drag-drop): the Add Project
  host selector can display "Local" (selectedRuntimeEnvironmentId = null, so
  the guard passes) while the global still points at an unavailable runtime.
  Native-picked/dropped paths are always local, so force local routing.

- AddProjectFromFolderDialog ("Add folder as project" on a subfolder): a
  subfolder lives on the active repo's host, so carry that host through the
  modal data and route by it — local for local projects, the owning runtime
  for runtime projects — instead of the globally-active runtime.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(sidebar): route runtime server-path adds by selected runtime, not global

The Add Project "server path" step (reached only when a runtime host is
selected) called addRepoPath(path, kind) with no explicit host, so it
inherited the global settings.activeRuntimeEnvironmentId. When that global
diverged from the dialog's selected runtime, the add was misrouted off the
host the user picked — the same root cause as #9541, opposite direction.

Route the server-path add by the dialog's selected runtime explicitly.
Co-locate selectedRuntimeEnvironmentId in useAddRepoHostSelection next to
selectedSshTargetId so the dialog reads it from one place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(sidebar): route the pre-add git server-path scan by the selected runtime

For kind === 'git', scanNestedRepos runs before addRepoPath and can
early-exit the flow into the nested-repo review, but it still routed by the
global active runtime — so it could scan the wrong host even after the add
itself was correctly routed to the selected runtime (CodeRabbit).

- scanNestedRepos accepts an optional runtimeEnvironmentId in its controls;
  when present it routes by that host, else falls back to the global
  (existing callers unchanged).
- useAddRepoServerPathFlow passes the selected runtime into the scan and
  derives runtimeKind/streaming support from it instead of the global-reading
  getNestedRepoRuntimeKind(null), so telemetry and the nested review target
  the same host as the add.

Adds renderer- and store-level regression tests covering scan routing to the
selected runtime, the null-override local case, and the nested-review handoff.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(sidebar): make nested-scan cancellation route by the scan's owning host

Follow-up to CodeRabbit review of the scan-routing change:

1. scanNestedRepos treated `{ runtimeEnvironmentId: undefined }` as an explicit
   local override via the `in` check. Only null or a string is now explicit;
   undefined falls back to the global (matches getAddRepoPathRouteSettings).

2. scanNestedRepos gained a routing override but cancelNestedRepoScan still
   routed by the global — an asymmetric contract where an override-routed scan
   could be un-cancellable if the global diverged mid-scan. cancelNestedRepoScan
   now takes the same override, and useAddRepoNestedReviewState remembers each
   scan's owning host by scanId (set when the scan is registered) so both stop
   and reset cancel on the host the scan actually ran on. The local folder flow
   routes its scan explicitly local so scan, cancel, and add all agree.

Adds store-level regression tests (explicit override wins, undefined falls back
to global, cancel routes by override) and a new useAddRepoNestedReviewState test
covering cancel-by-owning-runtime for stop and reset.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(sidebar): keep subfolder adds on their owning host

* fix(onboarding): keep completion on captured host

* chore(review): drop unreachable onboarding recovery

* fix(sidebar): preserve paired runtime checkout ownership

* fix(runtime): index paired worktrees by logical owner

* fix: fail closed on worktree owner alias collisions

* docs(sidebar): clarify host-routing intent flagged in review

Two Greptile P2 notes, addressed as comments (no behavior change):
- project-added-default-checkout.ts: the runtime branch's `hostId === executionHostId`
  is NOT unreachable — a colliding repo id can carry a runtime-qualified hostId with
  no runtimeOwnerEnvironmentId (see the "repo IDs collide" test). Documented why the
  comparison is reachable and load-bearing rather than replacing it.
- AddProjectFromFolderDialog.tsx: note that omitting the runtimeEnvironmentId spread
  intentionally signals local (NonGitFolderDialog coerces absence to null).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(activity): control portal readiness observer delivery

---------

Co-authored-by: fanyunqian.1 <fanyunqian.1@bytedance.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
This commit is contained in:
Yunqian Fan 2026-08-04 03:53:11 +08:00 committed by GitHub
parent 866bcda465
commit 814b87c421
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 271 additions and 32 deletions

View File

@ -84,6 +84,33 @@ function installAnimationFrameController(): {
}
}
function installMutationObserverController(): { notify: () => void } {
const callbacks = new Map<MutationObserver, MutationCallback>()
class ControlledMutationObserver implements MutationObserver {
constructor(callback: MutationCallback) {
callbacks.set(this, callback)
}
observe(): void {}
disconnect(): void {
callbacks.delete(this)
}
takeRecords(): MutationRecord[] {
return []
}
}
vi.stubGlobal('MutationObserver', ControlledMutationObserver)
return {
notify() {
for (const [observer, callback] of callbacks) {
callback([], observer)
}
}
}
}
async function flushPortalFramesUntil(
frames: ReturnType<typeof installAnimationFrameController>,
settled: () => boolean
@ -347,6 +374,7 @@ describe('Activity portal pane switching', () => {
it('releases a latched readiness once the terminal attaches', async () => {
const frames = installAnimationFrameController()
const mutations = installMutationObserverController()
const target = document.createElement('div')
document.body.append(target)
const buildRoot = (mode: 'hidden' | 'sibling' | 'ready'): void => {
@ -402,7 +430,7 @@ describe('Activity portal pane switching', () => {
const statusesBefore = statuses.length
await act(async () => {
buildRoot(mode)
await Promise.resolve()
mutations.notify()
})
expect(await flushPortalReadiness(frames)).toBe(true)
if (mode !== 'sibling') {
@ -426,7 +454,7 @@ describe('Activity portal pane switching', () => {
for (let attempt = 0; attempt < PORTAL_READY_REAPPLY_ATTEMPTS && !sawReady; attempt += 1) {
await act(async () => {
buildRoot('ready')
await Promise.resolve()
mutations.notify()
})
expect(await flushPortalReadiness(frames)).toBe(true)
await flushPortalFramesUntil(frames, () => statuses.at(-1) === 'ready')

View File

@ -56,4 +56,35 @@ describe('file explorer add project action', () => {
connectionId: 'ssh-target-1'
})
})
it('routes an execution-host-only SSH project subfolder to its SSH target', () => {
expect(
buildAddProjectFromFolderModalData(folderNode, {
...folderRepo,
executionHostId: 'ssh:ssh-target-1'
})
).toEqual({
folderPath: '/projects/child-project',
connectionId: 'ssh-target-1'
})
})
it('routes a local project subfolder explicitly to local', () => {
expect(buildAddProjectFromFolderModalData(folderNode, folderRepo)).toEqual({
folderPath: '/projects/child-project',
runtimeEnvironmentId: null
})
})
it("routes a runtime project subfolder to the repo's runtime", () => {
expect(
buildAddProjectFromFolderModalData(folderNode, {
...folderRepo,
executionHostId: 'runtime:runtime-a'
})
).toEqual({
folderPath: '/projects/child-project',
runtimeEnvironmentId: 'runtime-a'
})
})
})

View File

@ -1,10 +1,12 @@
import { isFolderRepo } from '../../../../shared/repo-kind'
import { getRepoExecutionHostId, parseExecutionHostId } from '../../../../shared/execution-host'
import type { Repo } from '../../../../shared/types'
import type { TreeNode } from './file-explorer-types'
export type AddProjectFromFolderModalData = {
folderPath: string
connectionId?: string
runtimeEnvironmentId?: string | null
}
export function canShowAddAsProjectAction(node: TreeNode, activeRepo: Repo | null): boolean {
@ -15,8 +17,13 @@ export function buildAddProjectFromFolderModalData(
node: TreeNode,
activeRepo: Repo
): AddProjectFromFolderModalData {
// Why: subfolder paths must stay on their owning repo host, not the mutable global selection.
const host = parseExecutionHostId(getRepoExecutionHostId(activeRepo))
if (host?.kind === 'ssh') {
return { folderPath: node.path, connectionId: host.targetId }
}
return {
folderPath: node.path,
...(activeRepo.connectionId ? { connectionId: activeRepo.connectionId } : {})
runtimeEnvironmentId: host?.kind === 'runtime' ? host.environmentId : null
}
}

View File

@ -150,7 +150,9 @@ describe('AddProjectFromFolderDialog', () => {
renderToStaticMarkup(<AddProjectFromFolderDialog />)
await clickAddProject()
expect(mocks.state.addRepoPath).toHaveBeenCalledWith('/projects/child')
expect(mocks.state.addRepoPath).toHaveBeenCalledWith('/projects/child', 'git', {
runtimeEnvironmentId: null
})
expect(mocks.state.fetchWorktrees).toHaveBeenCalledWith(repo.id, {
requireAuthoritative: true,
executionHostId: 'local'
@ -179,13 +181,44 @@ describe('AddProjectFromFolderDialog', () => {
renderToStaticMarkup(<AddProjectFromFolderDialog />)
await clickAddProject()
expect(mocks.state.addRepoPath).toHaveBeenCalledWith('/projects/child')
expect(mocks.state.addRepoPath).toHaveBeenCalledWith('/projects/child', 'git', {
runtimeEnvironmentId: null
})
expect(mocks.state.openModal).toHaveBeenCalledWith('confirm-non-git-folder', {
folderPath: '/projects/child'
})
expect(mocks.state.fetchWorktrees).not.toHaveBeenCalled()
})
it("routes a runtime project's subfolder and completion to the owning runtime", async () => {
const repo = makeRepo({ id: 'runtime-repo', executionHostId: 'runtime:runtime-a' })
mocks.state.modalData = {
folderPath: '/srv/projects/child',
runtimeEnvironmentId: 'runtime-a'
}
mocks.state.addRepoPath.mockResolvedValue(repo)
const { default: AddProjectFromFolderDialog } = await import('./AddProjectFromFolderDialog')
renderToStaticMarkup(<AddProjectFromFolderDialog />)
await clickAddProject()
expect(mocks.state.addRepoPath).toHaveBeenCalledWith('/srv/projects/child', 'git', {
runtimeEnvironmentId: 'runtime-a'
})
expect(mocks.state.fetchWorktrees).toHaveBeenCalledWith(repo.id, {
requireAuthoritative: true,
executionHostId: 'runtime:runtime-a'
})
expect(mocks.finishProjectAddWithDefaultCheckout).toHaveBeenCalledWith({
repoId: repo.id,
source: 'runtime_server_path',
selectedPath: '/srv/projects/child',
executionHostId: 'runtime:runtime-a',
closeModal: mocks.state.closeModal,
setHideDefaultBranchWorkspace: mocks.state.setHideDefaultBranchWorkspace
})
})
it('adds an SSH Git folder through the remote repo import path', async () => {
const repo = makeRepo({ id: 'remote-repo', connectionId: 'ssh-target-1' })
mocks.state.modalData = {

View File

@ -39,6 +39,8 @@ const AddProjectFromFolderDialog = React.memo(function AddProjectFromFolderDialo
const [previousOpen, setPreviousOpen] = useState(isOpen)
const folderPath = typeof modalData.folderPath === 'string' ? modalData.folderPath : ''
const connectionId = typeof modalData.connectionId === 'string' ? modalData.connectionId : ''
const runtimeEnvironmentId =
typeof modalData.runtimeEnvironmentId === 'string' ? modalData.runtimeEnvironmentId : null
if (isOpen !== previousOpen) {
setPreviousOpen(isOpen)
@ -55,9 +57,12 @@ const AddProjectFromFolderDialog = React.memo(function AddProjectFromFolderDialo
closeModal()
openModal('confirm-non-git-folder', {
folderPath,
...(connectionId ? { connectionId } : {})
...(connectionId ? { connectionId } : {}),
// Absence === local: NonGitFolderDialog coerces a missing/empty
// runtimeEnvironmentId to null, so omitting the spread signals local.
...(runtimeEnvironmentId ? { runtimeEnvironmentId } : {})
})
}, [closeModal, connectionId, folderPath, openModal])
}, [closeModal, connectionId, folderPath, openModal, runtimeEnvironmentId])
const handleConfirm = useCallback(async () => {
if (!folderPath || isAdding) {
@ -94,7 +99,7 @@ const AddProjectFromFolderDialog = React.memo(function AddProjectFromFolderDialo
{ description: repo.displayName }
)
} else {
repo = await addRepoPath(folderPath)
repo = await addRepoPath(folderPath, 'git', { runtimeEnvironmentId })
}
if (!mountedRef.current || gen !== addGenRef.current) {
@ -109,14 +114,18 @@ const AddProjectFromFolderDialog = React.memo(function AddProjectFromFolderDialo
}
// Why: after the repo is already added, a non-authoritative refresh
// should still close onto the project row instead of trapping the user.
const ownerOptions = worktreeRefreshOptions(null, connectionId)
const ownerOptions = worktreeRefreshOptions(runtimeEnvironmentId, connectionId)
await fetchWorktrees(repo.id, ownerOptions)
if (!mountedRef.current || gen !== addGenRef.current) {
return
}
await finishProjectAddWithDefaultCheckout({
repoId: repo.id,
source: connectionId ? 'ssh_remote_path' : 'local_folder_picker',
source: connectionId
? 'ssh_remote_path'
: runtimeEnvironmentId
? 'runtime_server_path'
: 'local_folder_picker',
selectedPath: folderPath,
executionHostId: ownerOptions.executionHostId,
closeModal,
@ -147,6 +156,7 @@ const AddProjectFromFolderDialog = React.memo(function AddProjectFromFolderDialo
isAdding,
mountedRef,
openNonGitConfirmation,
runtimeEnvironmentId,
setHideDefaultBranchWorkspace
])

View File

@ -221,6 +221,26 @@ describe('finishProjectAddWithDefaultCheckout', () => {
})
})
it('activates a runtime-owned checkout even when its physical host is private SSH', async () => {
const runtimeMain = makeWorktree({
id: 'repo-1::runtime-ssh',
hostId: 'ssh:private-target',
runtimeOwnerEnvironmentId: 'env-1'
})
mocks.state.worktreesByRepo = { 'repo-1': [runtimeMain] }
await openProjectDefaultCheckout({
repoId: 'repo-1',
source: 'runtime_server_path',
executionHostId: 'runtime:env-1',
setHideDefaultBranchWorkspace: vi.fn()
})
expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith(runtimeMain.id, {
executionHostId: 'runtime:env-1'
})
})
it('passes a contained selected path through as the initial terminal cwd', async () => {
mocks.state.worktreesByRepo = {
'repo-1': [makeWorktree()]

View File

@ -9,7 +9,7 @@ import type { DetectedWorktreeListResult, Worktree } from '../../../../shared/ty
import { relativePathInsideRoot } from '../../../../shared/cross-platform-path'
import { markOnboardingProjectAdded } from '@/lib/onboarding-project-checklist'
import { finalizeImportedRepoAfterSkip } from './add-repo-skip-finalization'
import type { ExecutionHostId } from '../../../../shared/execution-host'
import { parseExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host'
type DefaultCheckoutHandoffReason = EventProps<'add_repo_default_checkout_handoff'>['reason']
@ -24,12 +24,21 @@ function getProjectWorktreesForHost<T extends Worktree>(
if (!executionHostId) {
return [...worktrees]
}
const parsedHost = parseExecutionHostId(executionHostId)
return worktrees.filter((worktree) => {
if (worktree.hostId) {
if (parsedHost?.kind === 'runtime') {
if (worktree.runtimeOwnerEnvironmentId) {
return worktree.runtimeOwnerEnvironmentId === parsedHost.environmentId
}
// Reachable: a colliding repo id can carry a runtime-qualified hostId with
// no runtimeOwnerEnvironmentId, so match it against the execution host.
return worktree.hostId === executionHostId
}
if (worktree.runtimeOwnerEnvironmentId) {
return executionHostId === `runtime:${encodeURIComponent(worktree.runtimeOwnerEnvironmentId)}`
return false
}
if (worktree.hostId) {
return worktree.hostId === executionHostId
}
return executionHostId === 'local'
})

View File

@ -0,0 +1,90 @@
import { describe, expect, it } from 'vitest'
import { findIndexedWorktreeOwnerForHost } from './worktree-runtime-owner-index'
describe('worktree runtime owner index', () => {
it('indexes paired worktrees by both runtime owner and physical host', () => {
const paired = {
id: 'repo-1::same-id',
repoId: 'repo-1',
hostId: 'ssh:private-target' as const,
runtimeOwnerEnvironmentId: 'hub-a'
}
const directSsh = {
id: 'repo-1::direct',
repoId: 'repo-1',
hostId: 'ssh:direct-target' as const
}
const worktreesByRepo = { 'repo-1': [paired, directSsh] }
expect(findIndexedWorktreeOwnerForHost(worktreesByRepo, paired.id, 'runtime:hub-a')).toBe(
paired
)
expect(findIndexedWorktreeOwnerForHost(worktreesByRepo, paired.id, 'ssh:private-target')).toBe(
paired
)
expect(
findIndexedWorktreeOwnerForHost(worktreesByRepo, directSsh.id, 'ssh:direct-target')
).toBe(directSsh)
expect(
findIndexedWorktreeOwnerForHost(worktreesByRepo, directSsh.id, 'runtime:hub-a')
).toBeNull()
})
it('fails closed when direct and paired worktrees share a physical host alias', () => {
const direct = {
id: 'same-id',
repoId: 'direct-repo',
hostId: 'ssh:private-target' as const
}
const paired = {
id: 'same-id',
repoId: 'paired-repo',
hostId: 'ssh:private-target' as const,
runtimeOwnerEnvironmentId: 'hub-a'
}
for (const worktrees of [
[direct, paired],
[paired, direct]
]) {
const worktreesByRepo = { repo: worktrees }
expect(
findIndexedWorktreeOwnerForHost(worktreesByRepo, 'same-id', 'ssh:private-target')
).toBeNull()
expect(findIndexedWorktreeOwnerForHost(worktreesByRepo, 'same-id', 'runtime:hub-a')).toBe(
paired
)
}
})
it('fails closed when paired worktrees share a runtime host alias', () => {
const pairedA = {
id: 'same-id',
repoId: 'repo-a',
hostId: 'ssh:private-a' as const,
runtimeOwnerEnvironmentId: 'hub-a'
}
const pairedB = {
id: 'same-id',
repoId: 'repo-b',
hostId: 'ssh:private-b' as const,
runtimeOwnerEnvironmentId: 'hub-a'
}
for (const worktrees of [
[pairedA, pairedB],
[pairedB, pairedA]
]) {
const worktreesByRepo = { repo: worktrees }
expect(
findIndexedWorktreeOwnerForHost(worktreesByRepo, 'same-id', 'runtime:hub-a')
).toBeNull()
expect(findIndexedWorktreeOwnerForHost(worktreesByRepo, 'same-id', 'ssh:private-a')).toBe(
pairedA
)
expect(findIndexedWorktreeOwnerForHost(worktreesByRepo, 'same-id', 'ssh:private-b')).toBe(
pairedB
)
}
})
})

View File

@ -156,13 +156,33 @@ function worktreeOwnerIdentity(owner: WorktreeOwnerRecord): string {
])
}
function worktreeOwnerHostId(owner: WorktreeOwnerRecord): ExecutionHostId {
return (
parseExecutionHostId(owner.hostId)?.id ??
(owner.runtimeOwnerEnvironmentId
? toRuntimeExecutionHostId(owner.runtimeOwnerEnvironmentId)
: 'local')
)
function addWorktreeOwnerIndexEntry(
index: Map<string, IndexedWorktreeOwnerResolution>,
key: string,
owner: WorktreeOwnerRecord
): void {
const current = index.get(key)
if (!current) {
index.set(key, { kind: 'resolved', owner })
} else if (
current.kind === 'resolved' &&
worktreeOwnerIdentity(current.owner) !== worktreeOwnerIdentity(owner)
) {
index.set(key, { kind: 'ambiguous' })
}
}
function worktreeOwnerHostIds(owner: WorktreeOwnerRecord): ExecutionHostId[] {
const physicalHostId = parseExecutionHostId(owner.hostId)?.id
const runtimeEnvironmentId = owner.runtimeOwnerEnvironmentId?.trim()
if (!runtimeEnvironmentId) {
return [physicalHostId ?? 'local']
}
const runtimeHostId = toRuntimeExecutionHostId(runtimeEnvironmentId)
// Why: paired HUB worktrees need logical-runtime lookup without losing their physical SSH route.
return physicalHostId && physicalHostId !== runtimeHostId
? [physicalHostId, runtimeHostId]
: [runtimeHostId]
}
export function resolveIndexedWorktreeOwner(
@ -178,19 +198,10 @@ export function resolveIndexedWorktreeOwner(
for (const worktrees of Object.values(worktreesByRepo)) {
for (const worktree of worktrees) {
const id = worktree.id
const current = next.get(id)
if (!current) {
next.set(id, { kind: 'resolved', owner: worktree })
} else if (
current.kind === 'resolved' &&
worktreeOwnerIdentity(current.owner) !== worktreeOwnerIdentity(worktree)
) {
next.set(id, { kind: 'ambiguous' })
addWorktreeOwnerIndexEntry(next, id, worktree)
for (const hostId of worktreeOwnerHostIds(worktree)) {
addWorktreeOwnerIndexEntry(next, `${id}\0${hostId}`, worktree)
}
next.set(`${id}\0${worktreeOwnerHostId(worktree)}`, {
kind: 'resolved',
owner: worktree
})
}
}
index = next