mirror of
https://github.com/stablyai/orca.git
synced 2026-11-14 01:47:24 +00:00
fix(terminal): stop workspace activation from opening a fresh tab per orphaned counter-id PTY (#40950)
* fix(terminal): stop workspace activation from opening a fresh tab per orphaned counter-id PTY
Activation adoption minted a new tab with initialPtyId for every unowned
PTY. For bare local counter ids ("3", "13") main never reattaches by id
alone on a non-attach spawn, so each visit started a fresh shell and the
orphan stayed orphaned, producing another empty tab next time.
Counter-id orphans now still try their recorded pane (the attach-only
path that can reattach them); only the fresh-id createTab fallback is
declined and reported via declinedPtyIds. The all-digit check moves to a
shared isLocalCounterPtyId helper used by main and the renderer.
Fixes #40920
Co-authored-by: Neil <neil@stably.ai>
* test(terminal): pin the clock in the orphan counter PTY adoption test
---------
Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
@@ -6,6 +6,7 @@ import { expandWindowsEnvironmentVariables } from '../../../shared/windows-envir
|
||||
import { resolveSafePtyDefaultCwd } from '../pty-default-cwd'
|
||||
import { REMOTE_VIEWER_CLIPBOARD_ENV } from '../../../shared/remote-viewer-clipboard-env'
|
||||
import { ORCAD_SERVER_LAUNCH_ENV_KEYS } from '../../../shared/orcad-server-launch-env'
|
||||
import { isLocalCounterPtyId } from '../../../shared/terminal/pty/lifecycle/pty-session-id-format'
|
||||
|
||||
const PANE_IDENTITY_ENV_KEYS = [
|
||||
'ORCA_PANE_KEY',
|
||||
@@ -78,7 +79,7 @@ export function normalizeLocalCallerSessionId(
|
||||
allowNumeric = false
|
||||
): string | null {
|
||||
const requested = sessionId?.trim()
|
||||
return !requested || (!allowNumeric && /^\d+$/.test(requested)) ? null : requested
|
||||
return !requested || (!allowNumeric && isLocalCounterPtyId(requested)) ? null : requested
|
||||
}
|
||||
|
||||
export function normalizeForegroundProcessName(
|
||||
|
||||
+57
@@ -0,0 +1,57 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
adoptLiveWorkspacePtySurfaces,
|
||||
type LiveSurfaceAdoptionStore
|
||||
} from './worktree-agent-live-surface-adoption'
|
||||
import type { LiveTerminalSurfaceOwnerIndex } from '../ownership/worktree-live-terminal-surface-owners'
|
||||
|
||||
const WORKTREE_ID = 'repo-1::/workspace/repo'
|
||||
const RECORDED_TAB_ID = 'recorded-tab'
|
||||
const ORPHAN_PTY_ID = '3'
|
||||
const NOW = 1_700_000_000_000
|
||||
|
||||
describe('orphan local counter PTY adoption on workspace activation', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
vi.setSystemTime(NOW)
|
||||
})
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('mints no fresh tab when its recorded pane cannot be restored', async () => {
|
||||
const state = {
|
||||
tabsByWorktree: {},
|
||||
terminalLayoutsByTabId: {},
|
||||
ptyIdsByTabId: {},
|
||||
closedTerminalTabTombstonesByTabId: {
|
||||
[RECORDED_TAB_ID]: { closedAt: NOW, worktreeId: WORKTREE_ID }
|
||||
},
|
||||
createTab: vi.fn()
|
||||
}
|
||||
const census = new Map([
|
||||
[
|
||||
ORPHAN_PTY_ID,
|
||||
{
|
||||
unowned: true as const,
|
||||
recorded: {
|
||||
paneKey: `${RECORDED_TAB_ID}:4f0c2d1e-8a7b-4c3d-9e2f-1a2b3c4d5e6f`,
|
||||
ptyId: ORPHAN_PTY_ID,
|
||||
tabId: RECORDED_TAB_ID
|
||||
}
|
||||
}
|
||||
]
|
||||
]) satisfies LiveTerminalSurfaceOwnerIndex
|
||||
|
||||
const adoption = await adoptLiveWorkspacePtySurfaces(
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: adoption reads only these fields for a worktree with no tabs.
|
||||
() => state as unknown as LiveSurfaceAdoptionStore,
|
||||
WORKTREE_ID,
|
||||
[ORPHAN_PTY_ID],
|
||||
() => Promise.resolve(census)
|
||||
)
|
||||
|
||||
expect(state.createTab).not.toHaveBeenCalled()
|
||||
expect(adoption).toEqual({ surfaced: false, declinedPtyIds: [ORPHAN_PTY_ID] })
|
||||
})
|
||||
})
|
||||
@@ -2,6 +2,7 @@ import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../../shared/layout-owner-sw
|
||||
import { hasClosedTerminalTabRecord } from '../../../../../shared/workspace-layout/closed-terminal-tab-tombstones'
|
||||
import { parsePaneKey } from '../../../../../shared/workspace-layout/stable-pane-id'
|
||||
import { worktreeIdsEqual } from '../../../../../shared/worktree/id'
|
||||
import { isLocalCounterPtyId } from '../../../../../shared/terminal/pty/lifecycle/pty-session-id-format'
|
||||
import type { useAppStore } from '@/store/index'
|
||||
import type { WorktreeOperationRouteState } from '@/lib/workspace/ownership/worktree-operation-route'
|
||||
import {
|
||||
@@ -241,15 +242,18 @@ export async function adoptLiveWorkspacePtySurfaces(
|
||||
}
|
||||
continue
|
||||
}
|
||||
surfaced = true
|
||||
if (
|
||||
owner.recorded &&
|
||||
bindToRecordedSurface(getState, worktreeId, owner.recorded, materializedTabIds)
|
||||
) {
|
||||
const bound =
|
||||
(owner.recorded &&
|
||||
bindToRecordedSurface(getState, worktreeId, owner.recorded, materializedTabIds)) ||
|
||||
// A recorded tab createTab had to mint under another id already holds this PTY.
|
||||
resolveTerminalTabPtyOwnership(getState(), worktreeId, ptyId).kind !== 'none'
|
||||
// Why: only its recorded pane reattaches a local counter id; a fresh pane starts a new shell, and every visit mints another.
|
||||
if (!bound && isLocalCounterPtyId(ptyId)) {
|
||||
declinedPtyIds.push(ptyId)
|
||||
continue
|
||||
}
|
||||
// A recorded tab createTab had to mint under another id already holds this PTY.
|
||||
if (resolveTerminalTabPtyOwnership(getState(), worktreeId, ptyId).kind !== 'none') {
|
||||
surfaced = true
|
||||
if (bound) {
|
||||
continue
|
||||
}
|
||||
getState().createTab(worktreeId, undefined, undefined, {
|
||||
|
||||
@@ -29,6 +29,11 @@ export function isPtySessionIdOwnedByWorktree(sessionId: string, worktreeId: str
|
||||
return separatorIdx === -1 || sessionId.slice(0, separatorIdx) === worktreeId
|
||||
}
|
||||
|
||||
/** Bare counter ids from the in-process local provider; a start never reattaches one by id alone, only through a pane main already binds it to. */
|
||||
export function isLocalCounterPtyId(sessionId: string): boolean {
|
||||
return /^\d+$/.test(sessionId)
|
||||
}
|
||||
|
||||
const SERVE_PTY_SESSION_TAG = 'serve-'
|
||||
|
||||
/** Serve ids carry their workspace like daemon-minted ids so inventory never re-files them by cwd (#36716). */
|
||||
|
||||
Reference in New Issue
Block a user