mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 16:03:26 +00:00
fix(terminal): resolve folder workspaces, and fail closed on a tab-id collision
allWorktrees() reads only worktreesByRepo, so every folder workspace looked absent and its automation died at resolution. getKnownWorktreeById covers both. On a reserved-id collision, re-keying the tab could never work: ORCA_TAB_ID and ORCA_PANE_KEY are already baked into the spawned process, so routing and hook identity would permanently disagree. Retire the launch instead. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -3,7 +3,8 @@ import { makePaneKey, type PaneKey } from '../../../shared/stable-pane-id'
|
||||
import type { AgentType } from '../../../shared/agent-status-types'
|
||||
import { bindAutomationTerminal } from '@/lib/automation-terminal-ownership'
|
||||
import { createBrowserUuid } from '@/lib/browser-uuid'
|
||||
import { retireUnownedTerminal } from '@/lib/retire-unowned-background-terminal'
|
||||
import { retireProvider, retireUnownedTerminal } from '@/lib/retire-unowned-background-terminal'
|
||||
import { isTerminalTabPresent } from '@/store/slices/terminal-tab-retirement'
|
||||
import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client'
|
||||
|
||||
type Store = ReturnType<typeof useAppStore.getState>
|
||||
@@ -74,7 +75,6 @@ export async function adoptAgentBackgroundSessionTab(args: {
|
||||
store: Store
|
||||
worktreeId: string
|
||||
reservedTabId: string
|
||||
leafId: string
|
||||
ptyId: string
|
||||
paneKey: PaneKey
|
||||
launchConfig: RegisterArgs[1]
|
||||
@@ -88,7 +88,7 @@ export async function adoptAgentBackgroundSessionTab(args: {
|
||||
paneKey: PaneKey
|
||||
terminalOwnership: ReturnType<typeof bindAutomationTerminal>
|
||||
} | null> {
|
||||
const { store, reservedTabId, leafId, ptyId, launchRegistration } = args
|
||||
const { store, reservedTabId, ptyId, launchRegistration } = args
|
||||
// Why: the run tab cannot be closed mid-spawn now that it is created after the
|
||||
// await, but the worktree can still disappear across it — adopting into a gone
|
||||
// worktree would strand a live PTY behind an unreachable tab.
|
||||
@@ -103,20 +103,29 @@ export async function adoptAgentBackgroundSessionTab(args: {
|
||||
) {
|
||||
return null
|
||||
}
|
||||
// Why fail instead of re-key: ORCA_TAB_ID / ORCA_PANE_KEY are already baked into
|
||||
// the running process's env and cannot be rewritten. On collision createTab mints
|
||||
// a different id, so renderer routing and the agent's own hook identity would
|
||||
// permanently disagree — status and completion attribution silently go to the
|
||||
// colliding tab. A dead launch the user can retry beats a mis-attributed live one.
|
||||
// (A v4 UUID collision is vanishingly unlikely; this is a fail-closed guard.)
|
||||
if (isTerminalTabPresent(store, reservedTabId)) {
|
||||
store.clearAgentLaunchConfig(args.paneKey)
|
||||
args.onRetire()
|
||||
await retireProvider({
|
||||
ptyId,
|
||||
runtimeTarget: args.runtimeTarget,
|
||||
runtimeTerminalHandle: args.runtimeTerminalHandle
|
||||
})
|
||||
return null
|
||||
}
|
||||
const tab = store.createTab(args.worktreeId, undefined, undefined, {
|
||||
id: reservedTabId,
|
||||
initialPtyId: ptyId,
|
||||
activate: false,
|
||||
recordInteraction: false
|
||||
})
|
||||
let paneKey = args.paneKey
|
||||
if (tab.id !== reservedTabId) {
|
||||
// Why: createTab mints a fresh id on collision, so the env-baked paneKey no
|
||||
// longer routes; drop the stale registration before re-keying.
|
||||
store.clearAgentLaunchConfig(paneKey)
|
||||
paneKey = makePaneKey(tab.id, leafId)
|
||||
launchRegistration.tabId = tab.id
|
||||
}
|
||||
const paneKey = args.paneKey
|
||||
store.registerAgentLaunchConfig(paneKey, args.launchConfig, launchRegistration)
|
||||
const terminalOwnership = bindAutomationTerminal(
|
||||
tab,
|
||||
|
||||
@@ -226,22 +226,16 @@ describe('launchAgentBackgroundSession', () => {
|
||||
expect(mockDispatchEvent).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps a folder-workspace launch alive across the spawn await', async () => {
|
||||
// Why: folder workspaces live outside worktreesByRepo, so a liveness check built on
|
||||
// allWorktrees() reports them as gone and kills the agent PTY the moment it resolves.
|
||||
it('launches into a folder workspace that is absent from worktreesByRepo throughout', async () => {
|
||||
// Why: a real folder workspace is NEVER in worktreesByRepo — it lives in
|
||||
// folderWorkspaces and only getKnownWorktreeById resolves it. Seeding the repo
|
||||
// list here (even briefly) would fake the one condition under test, so both the
|
||||
// initial resolution and the post-spawn retire check are exercised for real (#2989).
|
||||
state.worktreesByRepo['repo-1'] = []
|
||||
state.getKnownWorktreeById = (worktreeId: string) =>
|
||||
worktreeId === 'folder:fw-1'
|
||||
? { id: 'folder:fw-1', path: '/tmp/folder-workspace' }
|
||||
: undefined
|
||||
state.worktreesByRepo['repo-1'] = [
|
||||
{
|
||||
id: 'folder:fw-1',
|
||||
repoId: 'repo-1',
|
||||
projectId: 'repo-1',
|
||||
path: '/tmp/folder-workspace',
|
||||
displayName: 'folder-workspace'
|
||||
}
|
||||
]
|
||||
const { launchAgentBackgroundSession } = await import('./launch-agent-background-session')
|
||||
|
||||
const launch = launchAgentBackgroundSession({
|
||||
@@ -249,8 +243,6 @@ describe('launchAgentBackgroundSession', () => {
|
||||
worktreeId: 'folder:fw-1',
|
||||
prompt: 'run in a folder workspace'
|
||||
})
|
||||
// The worktree list stays empty for the whole launch; only the folder lookup resolves.
|
||||
state.worktreesByRepo['repo-1'] = []
|
||||
|
||||
await expect(launch).resolves.toMatchObject({ ptyId: 'pty-1' })
|
||||
expect(mockKill).not.toHaveBeenCalled()
|
||||
@@ -541,6 +533,32 @@ describe('launchAgentBackgroundSession', () => {
|
||||
expect(mockKill).toHaveBeenCalledWith('pty-1')
|
||||
})
|
||||
|
||||
it('retires the launch instead of adopting a colliding tab id', async () => {
|
||||
// Why: ORCA_TAB_ID / ORCA_PANE_KEY are already baked into the spawned process, so a
|
||||
// re-keyed tab would route bytes to one identity and hooks to another. Fail closed.
|
||||
mockSpawn.mockImplementationOnce((args: { tabId: string }) => {
|
||||
// Land the collision between the reservation and the adoption, exactly as a
|
||||
// concurrently-created tab would.
|
||||
state.tabsByWorktree['wt-1'].push({ id: args.tabId, title: 'Squatter' })
|
||||
return Promise.resolve({ id: 'pty-1' })
|
||||
})
|
||||
const { launchAgentBackgroundSession } = await import('./launch-agent-background-session')
|
||||
|
||||
await expect(
|
||||
launchAgentBackgroundSession({
|
||||
agent: 'claude',
|
||||
worktreeId: 'wt-1',
|
||||
prompt: 'run the automation'
|
||||
})
|
||||
).resolves.toBeNull()
|
||||
|
||||
expect(mockCreateTab).not.toHaveBeenCalled()
|
||||
expect(mockKill).toHaveBeenCalledWith('pty-1')
|
||||
expect(state.clearAgentLaunchConfig).toHaveBeenCalledWith(
|
||||
expectStableAgentBackgroundPaneSpawn(mockSpawn)
|
||||
)
|
||||
})
|
||||
|
||||
it('submits prompts for stdin-after-start agents in background mode', async () => {
|
||||
const { launchAgentBackgroundSession } = await import('./launch-agent-background-session')
|
||||
|
||||
|
||||
@@ -48,7 +48,9 @@ export async function launchAgentBackgroundSession(
|
||||
): Promise<LaunchAgentBackgroundSessionResult | null> {
|
||||
const { agent, worktreeId, prompt, launchSource, title, onData, onExit, onAgentStatus } = args
|
||||
const store = useAppStore.getState()
|
||||
const worktree = store.allWorktrees().find((entry) => entry.id === worktreeId)
|
||||
// Why: allWorktrees() reads only worktreesByRepo, so every folder workspace looks
|
||||
// absent and its automation dies here. getKnownWorktreeById covers both (#2989).
|
||||
const worktree = store.getKnownWorktreeById(worktreeId)
|
||||
const repo = worktree ? store.repos.find((entry) => entry.id === worktree.repoId) : null
|
||||
if (!worktree) {
|
||||
throw new Error('The target workspace is no longer available.')
|
||||
@@ -227,7 +229,6 @@ export async function launchAgentBackgroundSession(
|
||||
store,
|
||||
worktreeId,
|
||||
reservedTabId,
|
||||
leafId,
|
||||
ptyId,
|
||||
paneKey,
|
||||
launchConfig: returnedLaunchConfig ?? startupPlan.launchConfig,
|
||||
|
||||
Reference in New Issue
Block a user