mirror of
https://github.com/stablyai/orca.git
synced 2026-10-01 16:02:03 +00:00
fix(pty,remote): close the pty master fd leak and two remote-terminal defects
#8362: node-pty leaves the forkpty()/posix_openpt() master without FD_CLOEXEC, so on Linux every later child of the process -- both later pty children and plain child_process spawns -- inherits it and keeps the /dev/pts device alive. Measured on Linux with stock node-pty 1.1.0: master fd flags 0404002 (cloexec=false), and 17 -> /dev/pts/ptmx present in both a later pty child's /proc/self/fd and a later child_process child's. Extend the existing node-pty patch with pty_cloexec() on both PtyFork spawn paths; after the patch the flags read 02404002 (cloexec=true) and neither child sees the master. This covers the app and terminal daemon only -- the SSH relay installs node-pty from npm on the remote host, so it stays exposed (see the report). #8414: the inspection queue's fire-and-forget `.finally()` chain re-raised a rejecting inspection as a renderer-global unhandledrejection, which an unreachable runtime produced on every cadence tick. #9194: `tab_not_found` is the host's definitive "no such tab", but the close path cleared the close intent for it exactly like a dropped connection, so a host that keeps republishing the dead surface re-materialized the pane the user just closed. Keep that intent and drop its TTL. Also route the banner's "Remote terminal was closed." line through translate() so it stops mixing English into a localized banner.
This commit is contained in:
@@ -176,7 +176,7 @@ index 181ccabbbe9c4948a9725fb1db907a68e9de01fc..67f31facf85562b67adbfbd04ce28ddd
|
||||
process.send!({ consoleProcessList });
|
||||
process.exit(0);
|
||||
diff --git a/src/unix/pty.cc b/src/unix/pty.cc
|
||||
index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..383df0c9c48355547c65e6c9bbba593d15c4dd44 100644
|
||||
index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..2ae787c5bd4f3eba470584dc658a01a52c690e0a 100644
|
||||
--- a/src/unix/pty.cc
|
||||
+++ b/src/unix/pty.cc
|
||||
@@ -23,7 +23,9 @@
|
||||
@@ -215,7 +215,17 @@ index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..383df0c9c48355547c65e6c9bbba593d
|
||||
/* Some platforms name VWERASE and VDISCARD differently */
|
||||
#if !defined(VWERASE) && defined(VWERSE)
|
||||
#define VWERASE VWERSE
|
||||
@@ -237,13 +258,23 @@ pty_getproc(int, char *);
|
||||
@@ -228,6 +249,9 @@ Napi::Value PtyGetProc(const Napi::CallbackInfo& info);
|
||||
static int
|
||||
pty_nonblock(int);
|
||||
|
||||
+static int
|
||||
+pty_cloexec(int);
|
||||
+
|
||||
#if defined(__APPLE__)
|
||||
static char *
|
||||
pty_getproc(int);
|
||||
@@ -237,13 +261,23 @@ pty_getproc(int, char *);
|
||||
#endif
|
||||
|
||||
#if defined(__APPLE__) || defined(__OpenBSD__)
|
||||
@@ -240,7 +250,7 @@ index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..383df0c9c48355547c65e6c9bbba593d
|
||||
#endif
|
||||
|
||||
struct DelBuf {
|
||||
@@ -367,10 +398,11 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) {
|
||||
@@ -367,14 +401,18 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) {
|
||||
argv[i + 3] = strdup(arg.c_str());
|
||||
}
|
||||
|
||||
@@ -256,7 +266,48 @@ index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..383df0c9c48355547c65e6c9bbba593d
|
||||
}
|
||||
if (pty_nonblock(master) == -1) {
|
||||
throw Napi::Error::New(napiEnv, "Could not set master fd to nonblocking.");
|
||||
@@ -684,15 +716,73 @@ pty_getproc(int fd, char *tty) {
|
||||
}
|
||||
+ if (pty_cloexec(master) == -1) {
|
||||
+ throw Napi::Error::New(napiEnv, "Could not set master fd to close-on-exec.");
|
||||
+ }
|
||||
#else
|
||||
int argc = argv_.Length();
|
||||
int argl = argc + 2;
|
||||
@@ -445,6 +483,9 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) {
|
||||
if (pty_nonblock(master) == -1) {
|
||||
throw Napi::Error::New(napiEnv, "Could not set master fd to nonblocking.");
|
||||
}
|
||||
+ if (pty_cloexec(master) == -1) {
|
||||
+ throw Napi::Error::New(napiEnv, "Could not set master fd to close-on-exec.");
|
||||
+ }
|
||||
}
|
||||
#endif
|
||||
|
||||
@@ -586,6 +627,23 @@ pty_nonblock(int fd) {
|
||||
return fcntl(fd, F_SETFL, flags | O_NONBLOCK);
|
||||
}
|
||||
|
||||
+/**
|
||||
+ * Orca: close-on-exec FD
|
||||
+ *
|
||||
+ * forkpty()/posix_openpt() have no atomic O_CLOEXEC, so a master left without
|
||||
+ * FD_CLOEXEC is inherited by every later child of this process -- including
|
||||
+ * later pty children -- which keeps its /dev/pts device and buffers alive long
|
||||
+ * after its own session ends (#8362).
|
||||
+ */
|
||||
+
|
||||
+static int
|
||||
+pty_cloexec(int fd) {
|
||||
+ int flags = fcntl(fd, F_GETFD);
|
||||
+ if (flags == -1) return -1;
|
||||
+ if (flags & FD_CLOEXEC) return 0;
|
||||
+ return fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
|
||||
+}
|
||||
+
|
||||
/**
|
||||
* pty_getproc
|
||||
* Taken from tmux.
|
||||
@@ -684,15 +742,73 @@ pty_getproc(int fd, char *tty) {
|
||||
#endif
|
||||
|
||||
#if defined(__APPLE__)
|
||||
@@ -332,7 +383,7 @@ index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..383df0c9c48355547c65e6c9bbba593d
|
||||
|
||||
for (; count < 3; count++) {
|
||||
low_fds[count] = posix_openpt(O_RDWR);
|
||||
@@ -706,80 +796,118 @@ pty_posix_spawn(char** argv, char** env,
|
||||
@@ -706,80 +822,118 @@ pty_posix_spawn(char** argv, char** env,
|
||||
POSIX_SPAWN_SETSID;
|
||||
*master = posix_openpt(O_RDWR);
|
||||
if (*master == -1) {
|
||||
|
||||
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: 572a46f539dd9da26e259702da974e1e693329e299625c97eb7c28e4e642500e
|
||||
node-pty@1.1.0: 40b6b6b814c89a8a29c995495ea86cff2cf6766f42124b5702aedf4ce0565c0e
|
||||
|
||||
importers:
|
||||
|
||||
@@ -156,7 +156,7 @@ importers:
|
||||
version: 3.3.1
|
||||
node-pty:
|
||||
specifier: ^1.1.0
|
||||
version: 1.1.0(patch_hash=572a46f539dd9da26e259702da974e1e693329e299625c97eb7c28e4e642500e)
|
||||
version: 1.1.0(patch_hash=40b6b6b814c89a8a29c995495ea86cff2cf6766f42124b5702aedf4ce0565c0e)
|
||||
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=572a46f539dd9da26e259702da974e1e693329e299625c97eb7c28e4e642500e):
|
||||
node-pty@1.1.0(patch_hash=40b6b6b814c89a8a29c995495ea86cff2cf6766f42124b5702aedf4ce0565c0e):
|
||||
dependencies:
|
||||
node-addon-api: 7.1.1
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import { existsSync, renameSync } from 'node:fs'
|
||||
import { execFileSync, spawn } from 'node:child_process'
|
||||
import { once } from 'node:events'
|
||||
import { existsSync, readdirSync, readFileSync, readlinkSync, renameSync } from 'node:fs'
|
||||
import { setTimeout as delay } from 'node:timers/promises'
|
||||
import * as pty from 'node-pty'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
@@ -90,3 +91,93 @@ describeOnDarwin('node-pty macOS spawn fd handling', () => {
|
||||
expect(after - before).toBe(0)
|
||||
}, 15000)
|
||||
})
|
||||
|
||||
// Linux is the only platform where node-pty takes the forkpty() path, which has no atomic
|
||||
// O_CLOEXEC. /proc is what makes the inheritance observable, so the assertions live here.
|
||||
const describeOnLinux = process.platform === 'linux' ? describe : describe.skip
|
||||
|
||||
const O_CLOEXEC = 0o2000000
|
||||
|
||||
function ptyMasterFd(term: pty.IPty): number {
|
||||
return (term as unknown as { fd: number }).fd
|
||||
}
|
||||
|
||||
function isCloseOnExec(fd: number): boolean {
|
||||
const flags = /flags:\s*(\d+)/.exec(readFileSync(`/proc/self/fdinfo/${fd}`, 'utf8'))
|
||||
expect(flags).toBeTruthy()
|
||||
return (Number.parseInt(flags![1]!, 8) & O_CLOEXEC) !== 0
|
||||
}
|
||||
|
||||
function openFdTargets(pid: number): string[] {
|
||||
return readdirSync(`/proc/${pid}/fd`).map((entry) => {
|
||||
try {
|
||||
return readlinkSync(`/proc/${pid}/fd/${entry}`)
|
||||
} catch {
|
||||
return ''
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
describeOnLinux('node-pty Linux forkpty fd handling', () => {
|
||||
it('marks pty masters close-on-exec so later children cannot inherit them', async () => {
|
||||
const terms: pty.IPty[] = []
|
||||
let child: ReturnType<typeof spawn> | null = null
|
||||
try {
|
||||
for (let i = 0; i < 3; i++) {
|
||||
terms.push(
|
||||
pty.spawn('/bin/sh', ['-c', 'sleep 30'], {
|
||||
name: 'xterm-256color',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
cwd: process.cwd(),
|
||||
env: { ...process.env, ORCA_FD_LEAK_TEST_INDEX: String(i) }
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
// The masters this process owns must not survive an exec in any child it forks later.
|
||||
expect(terms.map((term) => isCloseOnExec(ptyMasterFd(term)))).toEqual([true, true, true])
|
||||
|
||||
child = spawn('/bin/sh', ['-c', 'sleep 5'], { stdio: 'ignore' })
|
||||
await once(child, 'spawn')
|
||||
const inherited = openFdTargets(child.pid!).filter((target) => target.includes('ptmx'))
|
||||
expect(inherited).toEqual([])
|
||||
} finally {
|
||||
child?.kill()
|
||||
for (const term of terms) {
|
||||
term.kill()
|
||||
}
|
||||
}
|
||||
}, 15000)
|
||||
|
||||
it('does not hand an earlier pty master to a later pty child', async () => {
|
||||
const first = pty.spawn('/bin/sh', ['-c', 'sleep 30'], {
|
||||
name: 'xterm-256color',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
cwd: process.cwd(),
|
||||
env: { ...process.env }
|
||||
})
|
||||
try {
|
||||
const second = pty.spawn('/bin/sh', ['-c', 'ls -l /proc/self/fd; exit 0'], {
|
||||
name: 'xterm-256color',
|
||||
cols: 200,
|
||||
rows: 24,
|
||||
cwd: process.cwd(),
|
||||
env: { ...process.env }
|
||||
})
|
||||
let output = ''
|
||||
second.onData((data) => {
|
||||
output += data
|
||||
})
|
||||
await new Promise<void>((resolve) => {
|
||||
second.onExit(() => resolve())
|
||||
})
|
||||
await delay(100)
|
||||
|
||||
expect(output).not.toMatch(/ptmx/)
|
||||
} finally {
|
||||
first.kill()
|
||||
}
|
||||
}, 15000)
|
||||
})
|
||||
|
||||
@@ -18,6 +18,10 @@ const STALE_DAEMON_CWD_MARKERS = [
|
||||
]
|
||||
// Thrown by ipc/pty.ts when a persisted pane owner can't be proven alive or dead (STA-3536).
|
||||
const PANE_OWNER_UNVERIFIED_MARKER = 'terminal_pane_owner_unverified'
|
||||
// remote-runtime-pty-transport.ts surfaces this English literal as a wire-level marker, so it is
|
||||
// translated here rather than at the source -- otherwise the banner mixes English with the
|
||||
// localized chrome around it (#9194).
|
||||
const REMOTE_TERMINAL_CLOSED_MARKER = 'Remote terminal was closed.'
|
||||
// Why one source: the test and replace forms must match the same token, and a lone /g regex carries
|
||||
// lastIndex state across .test() calls. Capture the leading boundary so replacement can restore it.
|
||||
const TERMINAL_HOST_GONE_SOURCE = '(^|[^a-z0-9_])terminal_host_gone(?=$|[^a-z0-9_])'
|
||||
@@ -102,6 +106,14 @@ export function humanizeTerminalError(error: string): string {
|
||||
)
|
||||
)
|
||||
}
|
||||
if (humanized.includes(REMOTE_TERMINAL_CLOSED_MARKER)) {
|
||||
humanized = humanized.replaceAll(REMOTE_TERMINAL_CLOSED_MARKER, () =>
|
||||
translate(
|
||||
'auto.components.terminal.pane.TerminalErrorToast.remoteTerminalClosed',
|
||||
'Remote terminal was closed.'
|
||||
)
|
||||
)
|
||||
}
|
||||
humanized = humanizeUnreattachableSession(humanized)
|
||||
if (!isExplainedTerminalError(humanized)) {
|
||||
return humanized
|
||||
|
||||
+72
@@ -0,0 +1,72 @@
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import {
|
||||
enqueueAgentProcessInspection,
|
||||
resetAgentProcessInspectionQueueForTests
|
||||
} from './agent-process-inspection-queue'
|
||||
|
||||
// Node emits 'unhandledRejection' a turn after the microtask queue drains.
|
||||
async function settleRejections(): Promise<void> {
|
||||
for (let index = 0; index < 4; index += 1) {
|
||||
await new Promise((resolve) => setTimeout(resolve, 0))
|
||||
}
|
||||
}
|
||||
|
||||
async function collectUnhandledRejections(run: () => Promise<void>): Promise<unknown[]> {
|
||||
const unhandled: unknown[] = []
|
||||
const onUnhandledRejection = (reason: unknown): void => {
|
||||
unhandled.push(reason)
|
||||
}
|
||||
process.on('unhandledRejection', onUnhandledRejection)
|
||||
try {
|
||||
await run()
|
||||
} finally {
|
||||
process.off('unhandledRejection', onUnhandledRejection)
|
||||
}
|
||||
return unhandled
|
||||
}
|
||||
|
||||
describe('agent process inspection queue rejection containment', () => {
|
||||
afterEach(() => {
|
||||
resetAgentProcessInspectionQueueForTests()
|
||||
})
|
||||
|
||||
it('contains an unreachable-runtime inspection failure instead of raising unhandledrejection', async () => {
|
||||
const unhandled = await collectUnhandledRejections(async () => {
|
||||
enqueueAgentProcessInspection({
|
||||
priority: 'cadence',
|
||||
canRun: () => true,
|
||||
run: () =>
|
||||
Promise.reject(
|
||||
new Error(
|
||||
"Error invoking remote method 'runtimeEnvironments:call': RemoteRuntimeClientError: Could not connect to the remote Orca runtime."
|
||||
)
|
||||
)
|
||||
})
|
||||
await settleRejections()
|
||||
})
|
||||
|
||||
expect(unhandled).toEqual([])
|
||||
})
|
||||
|
||||
it('keeps draining the queue after a rejecting inspection', async () => {
|
||||
const ran: string[] = []
|
||||
const unhandled = await collectUnhandledRejections(async () => {
|
||||
enqueueAgentProcessInspection({
|
||||
priority: 'cadence',
|
||||
canRun: () => true,
|
||||
run: () => Promise.reject(new Error('unreachable'))
|
||||
})
|
||||
enqueueAgentProcessInspection({
|
||||
priority: 'cadence',
|
||||
canRun: () => true,
|
||||
run: async () => {
|
||||
ran.push('second')
|
||||
}
|
||||
})
|
||||
await settleRejections()
|
||||
})
|
||||
|
||||
expect(unhandled).toEqual([])
|
||||
expect(ran).toEqual(['second'])
|
||||
})
|
||||
})
|
||||
@@ -63,12 +63,18 @@ function pumpInspectionQueue(): void {
|
||||
|
||||
activeInspections += 1
|
||||
inspectionStarts.push(now)
|
||||
void next.run().finally(() => {
|
||||
activeInspections = Math.max(0, activeInspections - 1)
|
||||
if (inspectionQueue.length > 0) {
|
||||
scheduleInspectionPump()
|
||||
}
|
||||
})
|
||||
// Why the catch before finally: an unreachable runtime rejects the inspection on a cadence, and a
|
||||
// bare `.finally()` chain re-raises it as a renderer-global unhandledrejection. Coordinators own
|
||||
// their own failure/backoff state, so the queue only has to keep its accounting running.
|
||||
void next
|
||||
.run()
|
||||
.catch(() => {})
|
||||
.finally(() => {
|
||||
activeInspections = Math.max(0, activeInspections - 1)
|
||||
if (inspectionQueue.length > 0) {
|
||||
scheduleInspectionPump()
|
||||
}
|
||||
})
|
||||
|
||||
if (inspectionQueue.length > 0) {
|
||||
scheduleInspectionPump()
|
||||
|
||||
+24
@@ -0,0 +1,24 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
// Why a locale stand-in: the banner's own chrome is already translated, so the only way to see the
|
||||
// mixed-language regression (#9194) is to render the message through a non-English catalog.
|
||||
vi.mock('@/i18n/i18n', () => ({
|
||||
translate: (key: string, fallback: string) =>
|
||||
key === 'auto.components.terminal.pane.TerminalErrorToast.remoteTerminalClosed'
|
||||
? '远程终端已关闭。'
|
||||
: fallback
|
||||
}))
|
||||
|
||||
import { humanizeTerminalError } from './TerminalErrorToast'
|
||||
|
||||
describe('remote-closed terminal banner localization', () => {
|
||||
it('translates the remote-closed line instead of pinning it to English', () => {
|
||||
expect(humanizeTerminalError('Remote terminal was closed.')).toBe('远程终端已关闭。')
|
||||
})
|
||||
|
||||
it('translates the line when it is accumulated with other errors', () => {
|
||||
expect(humanizeTerminalError('Paste failed.\nRemote terminal was closed.')).toBe(
|
||||
'Paste failed.\n远程终端已关闭。'
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -3072,7 +3072,8 @@
|
||||
"cc6d997c65": "Restart the terminal daemon from here to clear stale daemon state.",
|
||||
"7ee11bc0db": "Orca couldn't confirm whether this terminal's previous session is still running, so it left the session untouched. Reopen this pane to retry.",
|
||||
"e16012e31e": "The terminal daemon that owned this session exited, so the session and its scrollback could not be recovered. Open a new terminal to continue.",
|
||||
"sessionUnavailable": "Orca couldn't reattach to this pane's terminal session on the host. Open a new terminal to continue."
|
||||
"sessionUnavailable": "Orca couldn't reattach to this pane's terminal session on the host. Open a new terminal to continue.",
|
||||
"remoteTerminalClosed": "Remote terminal was closed."
|
||||
},
|
||||
"TerminalProcessExitOverlay": {
|
||||
"capacityTitle": "Git Bash console limit reached",
|
||||
|
||||
@@ -2741,7 +2741,8 @@
|
||||
"e4aa243f8c": "Reiniciar servicio",
|
||||
"a7e2fd2699": "abre un issue",
|
||||
"5c8ce20be6": "Si esto persiste, por favor",
|
||||
"cc6d997c65": "Reinicia el servicio del terminal desde aquí para borrar el estado obsoleto."
|
||||
"cc6d997c65": "Reinicia el servicio del terminal desde aquí para borrar el estado obsoleto.",
|
||||
"remoteTerminalClosed": "La terminal remota se cerró."
|
||||
},
|
||||
"TerminalProcessExitOverlay": {
|
||||
"capacityTitle": "Se alcanzó el límite de consolas de Git Bash",
|
||||
|
||||
@@ -2741,7 +2741,8 @@
|
||||
"e4aa243f8c": "デーモンを再起動します",
|
||||
"a7e2fd2699": "Issue を登録",
|
||||
"5c8ce20be6": "この状態が続く場合は、",
|
||||
"cc6d997c65": "ここからターミナルデーモンを再起動して、古いデーモン状態をクリアします。"
|
||||
"cc6d997c65": "ここからターミナルデーモンを再起動して、古いデーモン状態をクリアします。",
|
||||
"remoteTerminalClosed": "リモートターミナルが閉じられました。"
|
||||
},
|
||||
"TerminalProcessExitOverlay": {
|
||||
"capacityTitle": "Git Bash のコンソール上限に達しました",
|
||||
|
||||
@@ -2746,7 +2746,8 @@
|
||||
"e4aa243f8c": "데몬 재시작",
|
||||
"a7e2fd2699": "이슈 등록",
|
||||
"5c8ce20be6": "문제가 계속되면",
|
||||
"cc6d997c65": "오래된 데몬 상태를 지우려면 여기에서 terminal 데몬을 다시 시작하세요."
|
||||
"cc6d997c65": "오래된 데몬 상태를 지우려면 여기에서 terminal 데몬을 다시 시작하세요.",
|
||||
"remoteTerminalClosed": "원격 터미널이 종료되었습니다."
|
||||
},
|
||||
"TerminalProcessExitOverlay": {
|
||||
"capacityTitle": "Git Bash 콘솔 한도에 도달했습니다",
|
||||
|
||||
@@ -2756,7 +2756,8 @@
|
||||
"e4aa243f8c": "重新启动守护进程",
|
||||
"a7e2fd2699": "提交议题",
|
||||
"5c8ce20be6": "如果这种情况持续存在,请",
|
||||
"cc6d997c65": "从此处重新启动终端守护进程以清除失效的守护进程状态。"
|
||||
"cc6d997c65": "从此处重新启动终端守护进程以清除失效的守护进程状态。",
|
||||
"remoteTerminalClosed": "远程终端已关闭。"
|
||||
},
|
||||
"TerminalProcessExitOverlay": {
|
||||
"capacityTitle": "已达到 Git Bash 控制台上限",
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
recordWebSessionCloseIntent,
|
||||
resetWebSessionCloseIntentForTests
|
||||
} from './web-session-close-intent'
|
||||
import { toHostSessionTabId } from './web-terminal-surface-id'
|
||||
import { ENVIRONMENT_ID, WORKTREE_ID, makeSnapshot } from './web-runtime-session-test-harness'
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
@@ -305,6 +306,36 @@ describe('web runtime session tab actions', () => {
|
||||
).resolves.toBe(outcome)
|
||||
})
|
||||
|
||||
// #9194: a host can answer tab_not_found and still keep republishing the surface. The close
|
||||
// intent is what hides the mirror, so letting it age out handed the user back a phantom pane
|
||||
// whose handle is already gone -- and closing it again just restarted the same 10s loop.
|
||||
it.each([
|
||||
['tab_not_found', true],
|
||||
['runtime_rpc_timeout', false]
|
||||
])('keeps a %s close suppressed past the close-intent TTL: %s', async (code, stillPending) => {
|
||||
const runtimeCall = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({ id: 'close', ok: false, error: { code, message: code } })
|
||||
.mockResolvedValueOnce({ id: 'list', ok: true, result: makeSnapshot() })
|
||||
vi.stubGlobal('window', { api: { runtimeEnvironments: { call: runtimeCall } } })
|
||||
|
||||
await closeWebRuntimeSessionTab({
|
||||
worktreeId: WORKTREE_ID,
|
||||
tabId: 'local-browser-unified',
|
||||
reason: 'user'
|
||||
})
|
||||
|
||||
const hostTabId = toHostSessionTabId('local-browser-unified')
|
||||
expect(
|
||||
isWebSessionCloseIntentPending(
|
||||
{ environmentId: ENVIRONMENT_ID },
|
||||
WORKTREE_ID,
|
||||
hostTabId,
|
||||
Date.now() + 60_000
|
||||
)
|
||||
).toBe(stillPending)
|
||||
})
|
||||
|
||||
it('fails closed when reconnect routes a lifecycle close to an older host', async () => {
|
||||
const runtimeCall = vi
|
||||
.fn()
|
||||
|
||||
@@ -6,7 +6,11 @@ import type {
|
||||
import { useAppStore } from '../store'
|
||||
import { hasRuntimeRpcErrorCode, unwrapRuntimeRpcResult } from './runtime-rpc-client'
|
||||
import { toRuntimeWorktreeSelector } from './runtime-worktree-selector'
|
||||
import { clearWebSessionCloseIntent, recordWebSessionCloseIntent } from './web-session-close-intent'
|
||||
import {
|
||||
clearWebSessionCloseIntent,
|
||||
makeWebSessionCloseIntentDurable,
|
||||
recordWebSessionCloseIntent
|
||||
} from './web-session-close-intent'
|
||||
import {
|
||||
clearWebSessionFocusIntentIfMatches,
|
||||
recordWebSessionFocusIntent
|
||||
@@ -157,8 +161,16 @@ async function callWebRuntimeSessionTabMethod(
|
||||
if (activationHostTabId) {
|
||||
clearWebSessionFocusIntentIfMatches(intentOwner, args.worktreeId, activationHostTabId)
|
||||
}
|
||||
// Why the split: only 'tab_not_found' is absence proof (see the outcome doc above). Restoring the
|
||||
// mirror on it hands the user back a pane the host cannot close and whose handle is already gone
|
||||
// (#9194), so keep the suppression and drop its TTL instead. Every other failure is a "not now".
|
||||
const hostHasNoSuchTab = hasRuntimeRpcErrorCode(error, 'tab_not_found')
|
||||
for (const hostTabId of closeIntentTabIds) {
|
||||
clearWebSessionCloseIntent(intentOwner, args.worktreeId, hostTabId)
|
||||
if (hostHasNoSuchTab) {
|
||||
makeWebSessionCloseIntentDurable(intentOwner, args.worktreeId, hostTabId)
|
||||
} else {
|
||||
clearWebSessionCloseIntent(intentOwner, args.worktreeId, hostTabId)
|
||||
}
|
||||
}
|
||||
if (isLifecycleClose) {
|
||||
const { acceptReplayedWebSessionTabsSnapshot } = await import('./web-session-tabs-sync')
|
||||
@@ -171,6 +183,6 @@ async function callWebRuntimeSessionTabMethod(
|
||||
`[web-runtime-session] failed to ${isClose ? 'close' : 'activate'} tab:`,
|
||||
error instanceof Error ? error.message : String(error)
|
||||
)
|
||||
return hasRuntimeRpcErrorCode(error, 'tab_not_found') ? 'unknown-tab' : 'failed'
|
||||
return hostHasNoSuchTab ? 'unknown-tab' : 'failed'
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,7 +4,7 @@ import { webSessionIntentOwnerKey, type WebSessionIntentOwner } from './web-sess
|
||||
|
||||
const CLOSE_INTENT_TTL_MS = 10_000
|
||||
|
||||
type CloseIntent = { recordedAt: number }
|
||||
type CloseIntent = { recordedAt: number; durable: boolean }
|
||||
|
||||
const pendingCloseByOwnerAndWorktree = new Map<string, Map<string, CloseIntent>>()
|
||||
|
||||
@@ -28,7 +28,27 @@ export function recordWebSessionCloseIntent(
|
||||
byTab = new Map()
|
||||
pendingCloseByOwnerAndWorktree.set(partitionKey, byTab)
|
||||
}
|
||||
byTab.set(trimmed, { recordedAt: now })
|
||||
byTab.set(trimmed, { recordedAt: now, durable: byTab.get(trimmed)?.durable === true })
|
||||
}
|
||||
|
||||
/**
|
||||
* Why no TTL: `tab_not_found` is the host's definitive answer that it does not have this tab, yet a
|
||||
* host can keep republishing the surface in its snapshot (#9194). Letting that intent age out
|
||||
* re-materializes a pane whose handle is already gone, and the pane the user just closed comes back
|
||||
* showing "Remote terminal was closed." with no way to dismiss it. The intent still clears the
|
||||
* moment the surface leaves a snapshot, so a host that recovers the tab is never suppressed forever.
|
||||
*/
|
||||
export function makeWebSessionCloseIntentDurable(
|
||||
owner: WebSessionIntentOwner,
|
||||
worktreeId: string,
|
||||
hostTabId: string
|
||||
): void {
|
||||
const intent = pendingCloseByOwnerAndWorktree
|
||||
.get(closeIntentPartitionKey(owner, worktreeId))
|
||||
?.get(hostTabId)
|
||||
if (intent) {
|
||||
intent.durable = true
|
||||
}
|
||||
}
|
||||
|
||||
export function isWebSessionCloseIntentPending(
|
||||
@@ -43,7 +63,7 @@ export function isWebSessionCloseIntentPending(
|
||||
if (!intent) {
|
||||
return false
|
||||
}
|
||||
if (now - intent.recordedAt > CLOSE_INTENT_TTL_MS) {
|
||||
if (!intent.durable && now - intent.recordedAt > CLOSE_INTENT_TTL_MS) {
|
||||
byTab!.delete(hostTabId)
|
||||
if (byTab!.size === 0) {
|
||||
pendingCloseByOwnerAndWorktree.delete(partitionKey)
|
||||
|
||||
Reference in New Issue
Block a user