mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 08:03:22 +00:00
review: harden #8283 fix — classify folder workspaces by repo-id prefix, not suffix-strip
The committed fix keyed folder detection off the ::workspace:<uuid> suffix
strip (isFolderWorkspace = worktreePath !== parsed.worktreePath). That leaves
two folder-workspace placeholders git-classified during hydration:
- the bare root worktree id (folder-workspace:<pgid>::<path>, no suffix), and
- any instance when the suffix-less root key is processed first (the
if (!existingRepo) early insert then skips the kind:'folder' tag).
Both still hit the isGitRepoKind-gated status poll (verified reproducing).
Key folder detection off the folder-workspace:<pgid> repo-id prefix instead
(new shared isFolderWorkspaceRepoId predicate), which is exclusive to folder
repos (git repo ids are bare UUIDs) and order-independent. Still strips the
suffix via splitWorktreeIdForFilesystem for a real folder cwd. Adds root-only
and root-before-instance regression tests; strengthens the instance test so it
can't pass vacuously. Framing unchanged: root-cause/churn fix, raw ENOENT
already guarded on main.
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -318,15 +318,77 @@ describe('hydrateWorkspaceSession', () => {
|
||||
|
||||
// The synthetic `::workspace:<uuid>` suffix is not a filesystem path; if it
|
||||
// reaches a worktree path it becomes a Git cwd and fails with a continuous
|
||||
// `spawn git ENOENT` on every status poll (#8283).
|
||||
for (const worktree of store.getState().worktreesByRepo[folderRepoId] ?? []) {
|
||||
expect(worktree.path).not.toContain(FOLDER_WORKSPACE_INSTANCE_SEPARATOR)
|
||||
}
|
||||
// `spawn git ENOENT` on every status poll (#8283). Assert the placeholder
|
||||
// exists first so the path check can never pass vacuously.
|
||||
const placeholderWorktrees = store.getState().worktreesByRepo[folderRepoId] ?? []
|
||||
expect(placeholderWorktrees).toHaveLength(1)
|
||||
expect(placeholderWorktrees[0]?.path).toBe(folderPath)
|
||||
// The placeholder repo must be classified as a folder repo so the
|
||||
// isGitRepoKind-gated status poll never spawns Git for it.
|
||||
const placeholderRepo = store.getState().repos.find((repo) => repo.id === folderRepoId)
|
||||
if (placeholderRepo) {
|
||||
expect(isGitRepoKind(placeholderRepo)).toBe(false)
|
||||
expect(placeholderRepo).toBeDefined()
|
||||
expect(isGitRepoKind(placeholderRepo!)).toBe(false)
|
||||
})
|
||||
|
||||
it('classifies the bare runtime folder-workspace root (no instance suffix) as a folder (#8283)', () => {
|
||||
const store = createTestStore()
|
||||
// The folder-workspace root worktree id has no `::workspace:<uuid>` suffix,
|
||||
// so a suffix-strip signal alone misses it — it must still be folder-tagged.
|
||||
const folderRepoId = 'folder-workspace:pg1'
|
||||
const folderPath = '/home/user'
|
||||
const rootId = `${folderRepoId}::${folderPath}`
|
||||
const session: WorkspaceSessionState = {
|
||||
...getDefaultWorkspaceSession(),
|
||||
activeRepoId: folderRepoId,
|
||||
activeWorktreeId: rootId,
|
||||
activeTabId: 'folder-root-tab',
|
||||
tabsByWorktree: {
|
||||
[rootId]: [makeTab({ id: 'folder-root-tab', worktreeId: rootId, ptyId: 'folder-root' })]
|
||||
}
|
||||
}
|
||||
|
||||
store.getState().hydrateWorkspaceSession(session, {
|
||||
runtimeHostIdByWorkspaceSessionKey: { [rootId]: 'runtime:env-1' }
|
||||
})
|
||||
|
||||
const placeholderRepo = store.getState().repos.find((repo) => repo.id === folderRepoId)
|
||||
expect(placeholderRepo).toBeDefined()
|
||||
expect(isGitRepoKind(placeholderRepo!)).toBe(false)
|
||||
expect(store.getState().worktreesByRepo[folderRepoId]?.[0]?.path).toBe(folderPath)
|
||||
})
|
||||
|
||||
it('keeps the folder classification when the root key precedes the instance key (#8283)', () => {
|
||||
const store = createTestStore()
|
||||
// Insertion order must not matter: if the root inserts the placeholder repo
|
||||
// first, the later instance key still resolves to a folder-classified repo.
|
||||
const folderRepoId = 'folder-workspace:pg1'
|
||||
const folderPath = '/home/user'
|
||||
const rootId = `${folderRepoId}::${folderPath}`
|
||||
const instanceId = `${rootId}${FOLDER_WORKSPACE_INSTANCE_SEPARATOR}11111111-1111-1111-1111-111111111111`
|
||||
const session: WorkspaceSessionState = {
|
||||
...getDefaultWorkspaceSession(),
|
||||
activeRepoId: folderRepoId,
|
||||
activeWorktreeId: instanceId,
|
||||
activeTabId: 'inst-tab',
|
||||
tabsByWorktree: {
|
||||
[rootId]: [makeTab({ id: 'root-tab', worktreeId: rootId, ptyId: 'root-session' })],
|
||||
[instanceId]: [makeTab({ id: 'inst-tab', worktreeId: instanceId, ptyId: 'inst-session' })]
|
||||
}
|
||||
}
|
||||
|
||||
// Root key ordered before the instance key in the owner map.
|
||||
store.getState().hydrateWorkspaceSession(session, {
|
||||
runtimeHostIdByWorkspaceSessionKey: {
|
||||
[rootId]: 'runtime:env-1',
|
||||
[instanceId]: 'runtime:env-1'
|
||||
}
|
||||
})
|
||||
|
||||
const placeholderRepo = store.getState().repos.find((repo) => repo.id === folderRepoId)
|
||||
expect(placeholderRepo).toBeDefined()
|
||||
expect(isGitRepoKind(placeholderRepo!)).toBe(false)
|
||||
for (const worktree of store.getState().worktreesByRepo[folderRepoId] ?? []) {
|
||||
expect(worktree.path).toBe(folderPath)
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -39,6 +39,7 @@ import {
|
||||
splitWorktreeId,
|
||||
splitWorktreeIdForFilesystem
|
||||
} from '../../../../shared/worktree-id'
|
||||
import { isFolderWorkspaceRepoId } from '../../../../shared/folder-workspace-worktree'
|
||||
import { isWslUncPath } from '../../../../shared/wsl-paths'
|
||||
import type { ProjectExecutionRuntimeResolution } from '../../../../shared/project-execution-runtime'
|
||||
import type { StartupCommandDelivery } from '../../../../shared/codex-startup-delivery'
|
||||
@@ -176,18 +177,19 @@ function buildRuntimeSessionPlaceholders({
|
||||
if (!parsed) {
|
||||
continue
|
||||
}
|
||||
// Why (#8283): folder (non-git) workspace instances keep the runtime shape
|
||||
// `folder-workspace:<pgid>::<folderPath>::workspace:<uuid>` and carry no
|
||||
// `worktree:`/`folder:` prefix, so the folder guard above misses them and
|
||||
// splitWorktreeId (first `::`) leaves the `::workspace:<uuid>` suffix on the
|
||||
// path. That synthetic, nonexistent path reaches Git as a cwd and fails with
|
||||
// a continuous `spawn git ENOENT` on every status poll. Resolve the real
|
||||
// folder path and tag the placeholder as a folder repo so the
|
||||
// isGitRepoKind-gated status poll never spawns Git for it. The suffix-strip
|
||||
// itself (not a substring match) is the folder-instance signal.
|
||||
// Why (#8283): folder (non-git) workspace worktrees keep the runtime shape
|
||||
// `folder-workspace:<pgid>::<folderPath>[::workspace:<uuid>]` and carry no
|
||||
// `worktree:`/`folder:` prefix, so the folder guard above misses them.
|
||||
// splitWorktreeId (first `::`) also leaves any `::workspace:<uuid>` instance
|
||||
// suffix on the path, and that synthetic, nonexistent path reaches Git as a
|
||||
// cwd — a continuous `spawn git ENOENT` on every status poll. Strip the
|
||||
// suffix for a real folder cwd, and tag the placeholder `kind:'folder'`
|
||||
// whenever the repo id is a folder-workspace id (covers the bare root and
|
||||
// every instance, in any arrival order) so the isGitRepoKind-gated poll
|
||||
// never spawns Git for it.
|
||||
const worktreePath =
|
||||
splitWorktreeIdForFilesystem(worktreeId)?.worktreePath ?? parsed.worktreePath
|
||||
const isFolderWorkspace = worktreePath !== parsed.worktreePath
|
||||
const isFolderWorkspace = isFolderWorkspaceRepoId(parsed.repoId)
|
||||
const existingRepo = nextRepos.some((repo) => repo.id === parsed.repoId)
|
||||
if (!existingRepo) {
|
||||
// Why: remote catalogs load after hydration, but host-split session
|
||||
|
||||
@@ -1,11 +1,19 @@
|
||||
import type { FolderWorkspace, Worktree } from './types'
|
||||
import { folderWorkspaceKey } from './workspace-scope'
|
||||
|
||||
// A folder (non-git) workspace's synthetic repo id is `folder-workspace:<pgid>`.
|
||||
// Git repo ids are bare UUIDs, so this prefix uniquely identifies folder repos.
|
||||
export const FOLDER_WORKSPACE_REPO_ID_PREFIX = 'folder-workspace:'
|
||||
|
||||
export function isFolderWorkspaceRepoId(repoId: string): boolean {
|
||||
return repoId.startsWith(FOLDER_WORKSPACE_REPO_ID_PREFIX)
|
||||
}
|
||||
|
||||
export function folderWorkspaceToWorktree(folderWorkspace: FolderWorkspace): Worktree {
|
||||
const linkedTask = folderWorkspace.linkedTask
|
||||
return {
|
||||
id: folderWorkspaceKey(folderWorkspace.id),
|
||||
repoId: `folder-workspace:${folderWorkspace.projectGroupId}`,
|
||||
repoId: `${FOLDER_WORKSPACE_REPO_ID_PREFIX}${folderWorkspace.projectGroupId}`,
|
||||
displayName: folderWorkspace.name,
|
||||
comment: folderWorkspace.comment,
|
||||
linkedIssue:
|
||||
|
||||
Reference in New Issue
Block a user