mirror of
https://github.com/stablyai/orca.git
synced 2026-10-02 08:02:02 +00:00
fix: close SSH and tab readiness race gaps
This commit is contained in:
@@ -815,18 +815,16 @@ diff --git a/lib/windowsTerminal.js b/lib/windowsTerminal.js
|
||||
index 3c38f89..e20b3e6 100644
|
||||
--- a/lib/windowsTerminal.js
|
||||
+++ b/lib/windowsTerminal.js
|
||||
@@ -50,6 +50,29 @@ var WindowsTerminal = /** @class */ (function (_super) {
|
||||
@@ -50,6 +50,27 @@ var WindowsTerminal = /** @class */ (function (_super) {
|
||||
// Create new termal.
|
||||
_this._agent = new windowsPtyAgent_1.WindowsPtyAgent(file, args, parsedEnv, cwd, _this._cols, _this._rows, false, opt.useConpty, opt.useConptyDll, opt.conptyInheritCursor);
|
||||
_this._socket = _this._agent.outSocket;
|
||||
+ // Attach before readiness so a broken ConPTY output pipe cannot be unhandled.
|
||||
+ _this._socket.on('error', function (err) {
|
||||
+ var code = err.code;
|
||||
+ var wasClosing = !_this._writable;
|
||||
+ // Node can report these duplicate stream errors after teardown; active
|
||||
+ // sockets still surface every other failure to the caller.
|
||||
+ var code = err && err.code;
|
||||
+ // PTY output can report EPIPE before `_close()` wins the race.
|
||||
+ _this._close();
|
||||
+ if (wasClosing && (code === 'EPIPE' || code === 'ERR_STREAM_PUSH_AFTER_EOF' || code === 'ERR_STREAM_DESTROYED')) {
|
||||
+ if (code === 'EPIPE' || code === 'ERR_STREAM_PUSH_AFTER_EOF' || code === 'ERR_STREAM_DESTROYED') {
|
||||
+ return;
|
||||
+ }
|
||||
+ // EIO, happens when someone closes our child process: the only process
|
||||
@@ -923,7 +921,7 @@ diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts
|
||||
index 13f6c6d..eda63c8 100644
|
||||
--- a/src/windowsTerminal.ts
|
||||
+++ b/src/windowsTerminal.ts
|
||||
@@ -51,6 +51,32 @@ export class WindowsTerminal extends Terminal {
|
||||
@@ -51,6 +51,30 @@ export class WindowsTerminal extends Terminal {
|
||||
this._agent = new WindowsPtyAgent(file, args, parsedEnv, cwd, this._cols, this._rows, false, opt.useConpty, opt.useConptyDll, opt.conptyInheritCursor);
|
||||
this._socket = this._agent.outSocket;
|
||||
-
|
||||
@@ -931,12 +929,10 @@ index 13f6c6d..eda63c8 100644
|
||||
+ // Attach before readiness so a broken ConPTY output pipe cannot be unhandled.
|
||||
+ this._socket.on('error', err => {
|
||||
+ const code = (<any>err).code;
|
||||
+ const wasClosing = !this._writable;
|
||||
+
|
||||
+ // Node can report these duplicate stream errors after teardown; active
|
||||
+ // sockets still surface every other failure to the caller.
|
||||
+ // PTY output can report EPIPE before `_close()` wins the race.
|
||||
+ this._close();
|
||||
+ if (wasClosing && (code === 'EPIPE' || code === 'ERR_STREAM_PUSH_AFTER_EOF' || code === 'ERR_STREAM_DESTROYED')) {
|
||||
+ if (code === 'EPIPE' || code === 'ERR_STREAM_PUSH_AFTER_EOF' || code === 'ERR_STREAM_DESTROYED') {
|
||||
+ return;
|
||||
+ }
|
||||
+
|
||||
|
||||
Generated
+3
-3
@@ -115,7 +115,7 @@ patchedDependencies:
|
||||
'@xterm/addon-webgl@0.20.0-beta.299': 94687e89a0115e6e6aa102837f986debdc029c091527ee5eb4a4e17ceaf9473e
|
||||
'@xterm/xterm@6.1.0-beta.303': 98756bcedc402bcdb7c6ab7b015d2e59cd18e97b03a2c06a27e95bb3ba429d9d
|
||||
lint-staged@16.4.0: 7333b3837f80a7fbd045964db6d76ba4fc118e49134bdbabb00585b6b7b60673
|
||||
node-pty@1.1.0: 1d4405bccb8ad7cb0c45061306a11059f02a23b9f60df8d14ef0563f88f84b0b
|
||||
node-pty@1.1.0: d4b956b791f0b898bc7708320055fbf913b54d979c26b376f9ea6b193c613df5
|
||||
|
||||
importers:
|
||||
|
||||
@@ -156,7 +156,7 @@ importers:
|
||||
version: 3.3.1
|
||||
node-pty:
|
||||
specifier: ^1.1.0
|
||||
version: 1.1.0(patch_hash=1d4405bccb8ad7cb0c45061306a11059f02a23b9f60df8d14ef0563f88f84b0b)
|
||||
version: 1.1.0(patch_hash=d4b956b791f0b898bc7708320055fbf913b54d979c26b376f9ea6b193c613df5)
|
||||
posthog-node:
|
||||
specifier: ^5.33.3
|
||||
version: 5.33.3
|
||||
@@ -12194,7 +12194,7 @@ snapshots:
|
||||
|
||||
node-int64@0.4.0: {}
|
||||
|
||||
node-pty@1.1.0(patch_hash=1d4405bccb8ad7cb0c45061306a11059f02a23b9f60df8d14ef0563f88f84b0b):
|
||||
node-pty@1.1.0(patch_hash=d4b956b791f0b898bc7708320055fbf913b54d979c26b376f9ea6b193c613df5):
|
||||
dependencies:
|
||||
node-addon-api: 7.1.1
|
||||
|
||||
|
||||
@@ -121,4 +121,35 @@ describe.skipIf(process.platform !== 'win32')('node-pty Windows input errors', (
|
||||
process.off('uncaughtException', uncaughtListener)
|
||||
}
|
||||
}, 20_000)
|
||||
|
||||
it('contains an output EPIPE that races with PTY shutdown', async () => {
|
||||
const uncaught: unknown[] = []
|
||||
const uncaughtListener = (error: unknown): void => {
|
||||
uncaught.push(error)
|
||||
}
|
||||
process.on('uncaughtException', uncaughtListener)
|
||||
|
||||
let terminal: IPty | undefined
|
||||
try {
|
||||
terminal = spawn(process.env.ComSpec ?? 'cmd.exe', ['/d', '/q'], {
|
||||
cwd: process.cwd(),
|
||||
env: process.env,
|
||||
useConptyDll: false
|
||||
})
|
||||
const output = (terminal as WindowsPtyInternals)._socket
|
||||
const exit = waitForExit(terminal)
|
||||
expect(() => {
|
||||
output.emit('error', Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }))
|
||||
terminal?.kill()
|
||||
}).not.toThrow()
|
||||
await exit
|
||||
expect(uncaught).toEqual([])
|
||||
} finally {
|
||||
try {
|
||||
terminal?.kill()
|
||||
} catch {}
|
||||
await new Promise((resolve) => setTimeout(resolve, 1_500))
|
||||
process.off('uncaughtException', uncaughtListener)
|
||||
}
|
||||
}, 20_000)
|
||||
})
|
||||
|
||||
@@ -2,6 +2,7 @@ import {
|
||||
copyFileSync,
|
||||
existsSync,
|
||||
linkSync,
|
||||
mkdirSync,
|
||||
mkdtempSync,
|
||||
readFileSync,
|
||||
rmSync,
|
||||
@@ -155,10 +156,13 @@ describeWindows('Windows Codex shell preflight runtime', () => {
|
||||
|
||||
const root = makeTempDir()
|
||||
const preflight = writeFailingPreflight(root)
|
||||
const codexExecutable = join(root, 'codex.exe')
|
||||
// Keep the fixture ahead of any host-global Codex installation in Git Bash.
|
||||
const codexExecutable = join(root, '.local', 'bin', 'codex.exe')
|
||||
mkdirSync(join(root, '.local', 'bin'), { recursive: true })
|
||||
linkNodeExecutable(codexExecutable)
|
||||
const preflightMarker = join(root, 'git-bash-preflight-ran')
|
||||
const codexMarker = join(root, 'git-bash-codex-ran')
|
||||
const codexPathMarker = join(root, 'git-bash-codex-path')
|
||||
const previousUserDataPath = process.env.ORCA_USER_DATA_PATH
|
||||
process.env.ORCA_USER_DATA_PATH = join(root, 'user data')
|
||||
|
||||
@@ -176,7 +180,7 @@ describeWindows('Windows Codex shell preflight runtime', () => {
|
||||
shellArgs: resolved.shellArgs,
|
||||
cwd: root,
|
||||
env: {
|
||||
...withPathEntry(process.env, root),
|
||||
...withPathEntry(process.env, join(root, '.local', 'bin')),
|
||||
CHERE_INVOKING: '1',
|
||||
HOME: root,
|
||||
ORCA_CODEX_LAUNCH_PREFLIGHT: preflight,
|
||||
@@ -185,7 +189,7 @@ describeWindows('Windows Codex shell preflight runtime', () => {
|
||||
TERM: 'xterm-256color'
|
||||
},
|
||||
input:
|
||||
"codex -e \"require('node:fs').writeFileSync(process.env.ORCA_CODEX_MARKER,'ran')\"\nexit\n",
|
||||
"type -P codex > git-bash-codex-path\ncodex -e \"require('node:fs').writeFileSync(process.env.ORCA_CODEX_MARKER,'ran')\"\nexit\n",
|
||||
// Paired "Windows low spec" QA measured 12.7–15.8s across four runs: Git Bash
|
||||
// cold-starts two large Node executables for AV scanning, so allow 25s without
|
||||
// inflating the faster cmd.exe budget.
|
||||
@@ -200,6 +204,11 @@ describeWindows('Windows Codex shell preflight runtime', () => {
|
||||
}
|
||||
|
||||
expect(existsSync(preflightMarker)).toBe(true)
|
||||
const resolvedCodexPath = readFileSync(codexPathMarker, 'utf8')
|
||||
.trim()
|
||||
.replaceAll('\\', '/')
|
||||
.toLowerCase()
|
||||
expect(resolvedCodexPath).toMatch(/\/\.local\/bin\/codex(?:\.exe)?$/)
|
||||
expect(readFileSync(codexMarker, 'utf8')).toBe('ran')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -139,13 +139,17 @@ export function setDocumentVisibility(state: 'visible' | 'hidden'): void {
|
||||
document.dispatchEvent(new Event('visibilitychange'))
|
||||
}
|
||||
|
||||
export async function publish(subscription: RuntimeSubscription, result: unknown): Promise<void> {
|
||||
export async function publish(
|
||||
subscription: RuntimeSubscription,
|
||||
result: unknown,
|
||||
runtimeId = 'runtime-a'
|
||||
): Promise<void> {
|
||||
await act(async () => {
|
||||
subscription.callbacks.onResponse({
|
||||
id: 'subscription-event',
|
||||
ok: true as const,
|
||||
result,
|
||||
_meta: { runtimeId: 'runtime-a' }
|
||||
_meta: { runtimeId }
|
||||
} as never)
|
||||
await settle()
|
||||
})
|
||||
|
||||
@@ -180,6 +180,66 @@ describe('mirrored-pane resume deferral against real stream frames', () => {
|
||||
expect(state.tabsByWorktree[WT]?.find((tab) => tab.id === MIRROR_TAB_ID)?.ptyId).toBeNull()
|
||||
})
|
||||
|
||||
it('does not let a late bootstrap runtime id retire a newer stream runtime', async () => {
|
||||
let resolveListAll: (response: unknown) => void = () => {}
|
||||
runtimeCall.mockImplementation((request: { method: string }) =>
|
||||
request.method === 'session.tabs.listAll'
|
||||
? new Promise((resolve) => {
|
||||
resolveListAll = resolve
|
||||
})
|
||||
: new Promise(() => {})
|
||||
)
|
||||
renderHook(() => useWebSessionTabsSync())
|
||||
await act(settle)
|
||||
|
||||
const backgroundParentTabId = 'host-tab-2'
|
||||
const backgroundSurfaceId = `${backgroundParentTabId}::${LEAF_ID}`
|
||||
const firstRuntimeB = makeHostSnapshot(BG_WT, backgroundSurfaceId, backgroundParentTabId)
|
||||
firstRuntimeB.publicationEpoch = 'runtime-b-epoch'
|
||||
if (firstRuntimeB.tabs[0]?.type !== 'terminal') {
|
||||
throw new Error('fixture must contain a terminal surface')
|
||||
}
|
||||
firstRuntimeB.tabs[0].terminal = 'runtime-b-terminal-1'
|
||||
await publish(
|
||||
findSubscription('session.tabs.subscribeAll'),
|
||||
{ type: 'updated', ...firstRuntimeB },
|
||||
'runtime-b'
|
||||
)
|
||||
|
||||
const lateRuntimeA = makeHostSnapshot(WT, HOST_SURFACE_ID, HOST_PARENT_TAB_ID)
|
||||
lateRuntimeA.publicationEpoch = 'runtime-a-epoch'
|
||||
if (lateRuntimeA.tabs[0]?.type !== 'terminal') {
|
||||
throw new Error('fixture must contain a terminal surface')
|
||||
}
|
||||
lateRuntimeA.tabs[0].terminal = 'runtime-a-terminal'
|
||||
await act(async () => {
|
||||
resolveListAll({
|
||||
id: 'listall-after-runtime-restart',
|
||||
ok: true as const,
|
||||
result: { snapshots: [lateRuntimeA] },
|
||||
_meta: { runtimeId: 'runtime-a' }
|
||||
})
|
||||
await settle()
|
||||
})
|
||||
|
||||
const secondRuntimeB = makeHostSnapshot(BG_WT, backgroundSurfaceId, backgroundParentTabId)
|
||||
secondRuntimeB.publicationEpoch = 'runtime-b-epoch'
|
||||
secondRuntimeB.snapshotVersion = 2
|
||||
if (secondRuntimeB.tabs[0]?.type !== 'terminal') {
|
||||
throw new Error('fixture must contain a terminal surface')
|
||||
}
|
||||
secondRuntimeB.tabs[0].terminal = 'runtime-b-terminal-2'
|
||||
await publish(
|
||||
findSubscription('session.tabs.subscribeAll'),
|
||||
{ type: 'updated', ...secondRuntimeB },
|
||||
'runtime-b'
|
||||
)
|
||||
|
||||
expect(useAppStore.getState().ptyIdsByTabId[BG_MIRROR_TAB_ID]).toEqual([
|
||||
`remote:${ENV}@@runtime-b-terminal-2`
|
||||
])
|
||||
})
|
||||
|
||||
it('does not relaunch when a stream frame is the first hydration signal', async () => {
|
||||
renderHook(() => useWebSessionTabsSync())
|
||||
await act(settle)
|
||||
|
||||
@@ -480,7 +480,23 @@ function isCurrentSessionTabsRuntimeFrame(environmentId: string, runtimeId?: str
|
||||
}
|
||||
|
||||
/** Returns false for a runtime identity already superseded on this environment. */
|
||||
function acceptSessionTabsRuntimeId(environmentId: string, runtimeId: string): boolean {
|
||||
function acceptSessionTabsRuntimeId(
|
||||
environmentId: string,
|
||||
runtimeId: string,
|
||||
receivedFrame?: number
|
||||
): boolean {
|
||||
const history = sessionTabsRuntimeHistoryByEnvironment.get(environmentId)
|
||||
const latestReceivedFrame = latestReceivedSessionTabsFrameByEnvironment.get(environmentId) ?? 0
|
||||
// A late bootstrap response may carry the predecessor process id. Do not
|
||||
// let that older frame retire the runtime that already published newer data.
|
||||
if (
|
||||
receivedFrame !== undefined &&
|
||||
receivedFrame < latestReceivedFrame &&
|
||||
history !== undefined &&
|
||||
history.current !== runtimeId
|
||||
) {
|
||||
return false
|
||||
}
|
||||
if (isRetiredSessionTabsRuntimeId(environmentId, runtimeId)) {
|
||||
return false
|
||||
}
|
||||
@@ -528,16 +544,16 @@ function recordReceivedWebSessionTabsSnapshot(
|
||||
const frame = receivedFrame ?? nextReceivedSessionTabsFrame()
|
||||
const key = sessionTabsFreshnessKey(environmentId, snapshot.worktree)
|
||||
const current = latestReceivedSessionTabsSnapshotByWorktree.get(key)
|
||||
if (runtimeId && !acceptSessionTabsRuntimeId(environmentId, runtimeId)) {
|
||||
return frame
|
||||
}
|
||||
recordReceivedWebSessionTabsEnvironmentFrame(environmentId, frame)
|
||||
// A bootstrap listAll reserves its frame before the request starts. If a
|
||||
// stream frame for this worktree arrived meanwhile, the late list is stale
|
||||
// evidence and must not advance epoch history.
|
||||
if (source === 'bootstrap' && current && frame < current.receivedFrame) {
|
||||
return frame
|
||||
}
|
||||
if (runtimeId && !acceptSessionTabsRuntimeId(environmentId, runtimeId, frame)) {
|
||||
return frame
|
||||
}
|
||||
recordReceivedWebSessionTabsEnvironmentFrame(environmentId, frame)
|
||||
const publicationEpoch = snapshot.publicationEpoch
|
||||
const history = sessionTabsPublicationEpochHistoryByWorktree.get(key)
|
||||
const isRetired = history?.retired.includes(publicationEpoch) ?? false
|
||||
@@ -4785,7 +4801,13 @@ function loadInitialWebSessionTabs(
|
||||
return
|
||||
}
|
||||
const runtimeId = getSessionTabsRuntimeIdFromResponse(response)
|
||||
if (runtimeId && !acceptSessionTabsRuntimeId(environmentId, runtimeId)) {
|
||||
const latestReceivedFrame =
|
||||
latestReceivedSessionTabsFrameByEnvironment.get(environmentId) ?? 0
|
||||
if (
|
||||
runtimeId &&
|
||||
latestReceivedFrame <= requestReceivedFrame &&
|
||||
!acceptSessionTabsRuntimeId(environmentId, runtimeId, requestReceivedFrame)
|
||||
) {
|
||||
return
|
||||
}
|
||||
recordReceivedWebSessionTabsEnvironmentFrame(environmentId, requestReceivedFrame)
|
||||
|
||||
Reference in New Issue
Block a user