diff --git a/src/main/ssh/ssh-relay-deploy.ts b/src/main/ssh/ssh-relay-deploy.ts index 1867df58b41..46e8a5b773f 100644 --- a/src/main/ssh/ssh-relay-deploy.ts +++ b/src/main/ssh/ssh-relay-deploy.ts @@ -77,7 +77,7 @@ import { import { detectRemoteHostPlatform } from './ssh-remote-platform-detection' import { powerShellCommand, powerShellLiteral, powerShellNativeArg } from './ssh-remote-powershell' import { relaySocketNameForInstanceId } from './ssh-relay-instance-id' -import { isRelaySocketOwnerLiveError, releaseUnownedRelaySocket } from './ssh-relay-socket-owner' +import { isRelaySocketOwnerLiveError, requireUnownedRelaySocket } from './ssh-relay-socket-owner' import { isSshSessionLimitError } from './ssh-session-limit-error' import { isWindowsRelayPipePath, @@ -1446,11 +1446,12 @@ export async function launchRelay( '[ssh-relay] Socket reconnect failed:', err instanceof Error ? err.message : String(err) ) - // Why: a refused --connect is loss of contact, never proof the relay died, so the socket - // is released only once the host proves nothing owns it. Unlinking a live owner's path - // cannot stop it — it just hides it from the fresh launch's EADDRINUSE check and orphans - // it and its PTYs forever (STA-1756). - await releaseUnownedRelaySocket(conn, nodePath, sockFile, { signal, cause: err }) + // Why: a refused --connect is loss of contact, never proof the relay died, so the path + // is only taken once the host proves nothing owns it. Launching over a live owner + // cannot stop it — it just hides it from the fresh daemon's EADDRINUSE check and + // orphans it and its PTYs forever (STA-1756). The daemon reclaims a proven-stale + // socket itself, under its own identity check. + await requireUnownedRelaySocket(conn, nodePath, sockFile, { signal, cause: err }) } } } catch (err) { diff --git a/src/main/ssh/ssh-relay-reset.test.ts b/src/main/ssh/ssh-relay-reset.test.ts index 62a532f5ed5..d77907ef4e1 100644 --- a/src/main/ssh/ssh-relay-reset.test.ts +++ b/src/main/ssh/ssh-relay-reset.test.ts @@ -116,7 +116,10 @@ async function runReset(mode: HostMode): Promise { if (mode !== 'no-process-tools') { // Why the -a split: without it lsof would also return an unrelated Unix-socket holder, // which reset must never pass to kill (#8762). - const lsofBody = + // Why the -p arm answers unconditionally: the script asks lsof about its own process + // first to prove the tool works at all, and a stub that tied that to the socket's owner + // would make a working lsof look broken the moment the relay stopped. + const lsofSocketArm = mode === 'only-pgrep-finds-owner' ? 'exit 1' : `${OWNER_ALIVE_GUARD} @@ -124,6 +127,10 @@ case " $* " in *" -a "*) printf '%s\\n' "$OWNER_PID" ;; *) printf '%s\\n' "$OWNER_PID" "$UNRELATED_PID" ;; esac` + const lsofBody = `case " $* " in + *" -p "*) printf '%s\\n' "$$" ; exit 0 ;; +esac +${lsofSocketArm}` writeExecutable(join(binDir, 'lsof'), lsofBody) writeExecutable( join(binDir, 'pgrep'), diff --git a/src/main/ssh/ssh-relay-reset.ts b/src/main/ssh/ssh-relay-reset.ts index cfc270dda19..364e4b52142 100644 --- a/src/main/ssh/ssh-relay-reset.ts +++ b/src/main/ssh/ssh-relay-reset.ts @@ -64,10 +64,8 @@ export async function forceStopRelayForTarget( 'if [ -d "$base" ]; then', ' for sock in "$base"/relay-*/"$sock_name" "$base"/"$sock_name"; do', ' [ -S "$sock" ] || continue', - ' signalled=no', ' find_holder "$sock"', ' if [ -n "$holder" ]; then', - ' signalled=yes', ' kill -TERM $holder 2>/dev/null || true', ' sleep 0.2', ' kill -KILL $holder 2>/dev/null || true', @@ -80,7 +78,9 @@ export async function forceStopRelayForTarget( ' find_holder "$sock"', ' if [ "$listed" = 0 ] || [ -n "$holder" ]; then', ` echo ${RESET_OWNER_SURVIVED_MARKER}`, - ' elif [ "$listed" = 2 ] && [ "$signalled" = no ]; then', + // Why regardless of whether we signalled: sending a signal is not evidence the process + // took it, so with no inventory to ask afterwards the stop stays unproven either way. + ' elif [ "$listed" = 2 ]; then', ` echo ${RESET_NO_OWNER_PROOF_MARKER}`, ' fi', ' done', diff --git a/src/main/ssh/ssh-relay-socket-displacement.test.ts b/src/main/ssh/ssh-relay-socket-displacement.test.ts index dce3eb5332a..b408b59b406 100644 --- a/src/main/ssh/ssh-relay-socket-displacement.test.ts +++ b/src/main/ssh/ssh-relay-socket-displacement.test.ts @@ -117,7 +117,7 @@ describe('deployAndLaunchRelay socket displacement', () => { const conn = makeMockConnection() vi.mocked(waitForSentinel).mockRejectedValueOnce(new Error('relay reconnect refused')) queueDeployUpToOccupiedSocket() - vi.mocked(execCommand).mockResolvedValueOnce('LIVE') // socket release: a live owner + vi.mocked(execCommand).mockResolvedValueOnce('LIVE') // owner probe: a live owner await expect(deployAndLaunchRelay(conn)).rejects.toSatisfy(isRelaySocketOwnerLiveError) @@ -131,7 +131,7 @@ describe('deployAndLaunchRelay socket displacement', () => { const conn = makeMockConnection() vi.mocked(waitForSentinel).mockRejectedValueOnce(new Error('relay reconnect refused')) queueDeployUpToOccupiedSocket() - vi.mocked(execCommand).mockResolvedValueOnce('') // release produced no verdict + vi.mocked(execCommand).mockResolvedValueOnce('') // probe produced no verdict await expect(deployAndLaunchRelay(conn)).rejects.toSatisfy(isRelaySocketOwnerLiveError) @@ -139,14 +139,14 @@ describe('deployAndLaunchRelay socket displacement', () => { expect(writeRelayEndpointCredential).not.toHaveBeenCalled() }) - it('still launches fresh once the host reports the stale socket released', async () => { + it('still launches fresh once the host proves the stale socket unowned', async () => { const conn = makeMockConnection() vi.mocked(waitForSentinel) .mockRejectedValueOnce(new Error('relay reconnect refused')) .mockResolvedValue({ write: vi.fn(), onData: vi.fn(), onClose: vi.fn() } as never) queueDeployUpToOccupiedSocket() vi.mocked(execCommand) - .mockResolvedValueOnce('RELEASED') // socket release: nothing owned the path + .mockResolvedValueOnce('EXITED') // owner probe: nothing owns the path .mockResolvedValueOnce('READY') // socket readiness poll await expect(deployAndLaunchRelay(conn)).resolves.toMatchObject({ nodePath: '/usr/bin/node' }) @@ -154,24 +154,21 @@ describe('deployAndLaunchRelay socket displacement', () => { expect(execChannelCommands(conn).some((command) => command.includes('--detached'))).toBe(true) }) - // Moved here from ssh-relay-deploy.test.ts: the removal it guards now lives inside the - // release command, so an unconfirmed close of that command must still stop the launch. - it('does not launch fresh after an unconfirmed socket release', async () => { + // Replaces the unconfirmed-cleanup case from ssh-relay-deploy.test.ts: deploy no longer + // removes anything, so a probe it could not run must stop the launch on its own. + it('does not launch fresh after a probe it could not run', async () => { const conn = makeMockConnection() - const unconfirmedRelease = Object.assign(new Error('socket release still running'), { - sshChannelCloseConfirmed: false - }) vi.mocked(waitForSentinel).mockRejectedValueOnce(new Error('relay reconnect refused')) queueDeployUpToOccupiedSocket() - vi.mocked(execCommand).mockRejectedValueOnce(unconfirmedRelease) + vi.mocked(execCommand).mockRejectedValueOnce(new Error('channel open failure')) - await expect(deployAndLaunchRelay(conn)).rejects.toBe(unconfirmedRelease) + await expect(deployAndLaunchRelay(conn)).rejects.toSatisfy(isRelaySocketOwnerLiveError) expect(execChannelCommands(conn)).toHaveLength(1) expect(execChannelCommands(conn).some((command) => command.includes('--detached'))).toBe(false) }) - it('decides with one host command that probes and unlinks together', async () => { + it('decides by asking the host who owns the socket, and removes nothing', async () => { const conn = makeMockConnection() vi.mocked(waitForSentinel).mockRejectedValueOnce(new Error('relay reconnect refused')) queueDeployUpToOccupiedSocket() @@ -179,13 +176,13 @@ describe('deployAndLaunchRelay socket displacement', () => { await expect(deployAndLaunchRelay(conn)).rejects.toSatisfy(isRelaySocketOwnerLiveError) - // Why one command: probing here and unlinking in a second round-trip leaves a window - // in which another client can bind the path, and the removal would then strand it. - const release = execCommands().at(-1) ?? '' - expect(release).toContain('/usr/bin/node') - expect(release).toContain('connect') - expect(release).toContain('unlinkSync') - expect(release).not.toContain('test -S') - expect(execCommands().some((command) => command.startsWith('rm -f'))).toBe(false) + // Why nothing removed: the daemon reclaims a stale path under its own identity check, + // and an unlink here would put an SSH round-trip between that check and the launch. + const probe = execCommands().at(-1) ?? '' + expect(probe).toContain('/usr/bin/node') + expect(probe).toContain('connect') + expect(probe).not.toContain('test -S') + expect(execCommands().some((command) => command.includes('rm -f'))).toBe(false) + expect(execCommands().some((command) => command.includes('unlinkSync'))).toBe(false) }) }) diff --git a/src/main/ssh/ssh-relay-socket-owner.integration.test.ts b/src/main/ssh/ssh-relay-socket-owner.integration.test.ts index fc52c75a6f0..57813641cfe 100644 --- a/src/main/ssh/ssh-relay-socket-owner.integration.test.ts +++ b/src/main/ssh/ssh-relay-socket-owner.integration.test.ts @@ -7,7 +7,7 @@ // while a PTY is held). // // The control case below is latest-main's behaviour: it performs the raw unlink and -// asserts the orphan appears. The guarded case asserts releaseUnownedRelaySocket refuses +// asserts the orphan appears. The guarded case asserts requireUnownedRelaySocket refuses // the same displacement, which restores the relay's own EADDRINUSE interlock. Which // production call site issues that release is pinned separately, by the mocked deploy // wiring in ssh-relay-socket-displacement.test.ts. @@ -27,7 +27,7 @@ import { relayBundleDirForHost, waitForExit } from './ssh-relay-live-daemon-harness' -import { isRelaySocketOwnerLiveError, releaseUnownedRelaySocket } from './ssh-relay-socket-owner' +import { isRelaySocketOwnerLiveError, requireUnownedRelaySocket } from './ssh-relay-socket-owner' const REPO_ROOT = process.cwd() const BUNDLE_DIR = relayBundleDirForHost(REPO_ROOT) @@ -89,7 +89,7 @@ describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('relay socket owner expect(isProcessAlive(pid)).toBe(false) } - it('releases the socket of a relay that is really gone', async () => { + it('clears a relay that is really gone, without touching its socket', async () => { const { fixture: live, pid } = await startRelayHoldingAPty() const conn = createLocalShellConnection() @@ -99,18 +99,20 @@ describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('relay socket owner expect(existsSync(live.sockPath)).toBe(true) await expect( - releaseUnownedRelaySocket(conn, process.execPath, live.sockPath) + requireUnownedRelaySocket(conn, process.execPath, live.sockPath) ).resolves.toBeUndefined() - expect(existsSync(live.sockPath)).toBe(false) + // Why still there: reclaiming the path belongs to the daemon that binds it, which does + // the identity check and the bind in one process. + expect(existsSync(live.sockPath)).toBe(true) }) - it('refuses to release the socket of a relay that still holds a PTY', async () => { + it('refuses to hand the socket of a relay that still holds a PTY to a replacement', async () => { const { fixture: live, pid } = await startRelayHoldingAPty() const conn = createLocalShellConnection() await expect( - releaseUnownedRelaySocket(conn, process.execPath, live.sockPath) + requireUnownedRelaySocket(conn, process.execPath, live.sockPath) ).rejects.toSatisfy(isRelaySocketOwnerLiveError) expect(existsSync(live.sockPath)).toBe(true) @@ -136,7 +138,7 @@ describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('relay socket owner const conn = createLocalShellConnection({ path: '' }) await expect( - releaseUnownedRelaySocket(conn, process.execPath, live.sockPath) + requireUnownedRelaySocket(conn, process.execPath, live.sockPath) ).rejects.toSatisfy(isRelaySocketOwnerLiveError) expect(existsSync(live.sockPath)).toBe(true) diff --git a/src/main/ssh/ssh-relay-socket-owner.test.ts b/src/main/ssh/ssh-relay-socket-owner.test.ts index f8dda2ffe18..f0cbed5e337 100644 --- a/src/main/ssh/ssh-relay-socket-owner.test.ts +++ b/src/main/ssh/ssh-relay-socket-owner.test.ts @@ -11,9 +11,9 @@ import type { SshConnection } from './ssh-connection' import { execCommand } from './ssh-relay-deploy-helpers' import { isRelaySocketOwnerLiveError, - parseRelaySocketReleaseOutcome, - relaySocketReleaseCommand, - releaseUnownedRelaySocket + parseRelaySocketOwnerVerdict, + relaySocketOwnerProbeCommand, + requireUnownedRelaySocket } from './ssh-relay-socket-owner' import { isRelayVersionMismatchError, @@ -24,8 +24,8 @@ const SOCK = '/home/u/.orca-remote/relay-0.1.0+abc/relay-deadbeef.sock' const NODE = '/usr/bin/node' const conn = {} as SshConnection -describe('relay socket release command', () => { - const command = relaySocketReleaseCommand(NODE, SOCK) +describe('relay socket owner probe command', () => { + const command = relaySocketOwnerProbeCommand(NODE, SOCK) it('proves an owner is present with lsof before falling back to a connect probe', () => { expect(command).toContain('lsof -t -a -U "$sock"') @@ -37,13 +37,12 @@ describe('relay socket release command', () => { expect(command).toContain(`'${SOCK}'`) }) - it('unlinks inside the same host command, guarded by the inode it probed', () => { - // Why: probing here and removing in a second round-trip leaves a window in which - // another client can bind the path, and the removal would then strand it. - expect(command).toContain('unlinkSync') - expect(command).toContain('statSync') - // Why ctime: inode numbers are recycled, so dev+ino alone can match a recreated socket. - expect(command).toContain('st.dev+":"+st.ino+":"+st.ctimeNs') + it('only looks, and never removes anything itself', () => { + // Why: reclaiming the path is the fresh daemon's job, which checks the socket's identity + // and binds in one process. Unlinking here would put an SSH round-trip between that check + // and the launch, and a relay that bound the path in between would be the one displaced. + expect(command).not.toContain('unlinkSync') + expect(command).not.toContain('rm -f') }) it('retries a refused connect before concluding the path is unowned', () => { @@ -67,35 +66,35 @@ describe('relay socket release command', () => { }) }) -describe('relay socket release outcome', () => { +describe('relay socket owner verdict', () => { it('reads the host markers', () => { - expect(parseRelaySocketReleaseOutcome('LIVE')).toBe('live') - expect(parseRelaySocketReleaseOutcome('RELEASED\n')).toBe('released') - expect(parseRelaySocketReleaseOutcome('UNVERIFIABLE')).toBe('unverifiable') + expect(parseRelaySocketOwnerVerdict('LIVE')).toBe('live') + expect(parseRelaySocketOwnerVerdict('EXITED\n')).toBe('exited') + expect(parseRelaySocketOwnerVerdict('UNVERIFIABLE')).toBe('unverifiable') }) it('refuses to guess from ambiguous or empty output', () => { - expect(parseRelaySocketReleaseOutcome('')).toBe('unverifiable') - expect(parseRelaySocketReleaseOutcome('LIVE\nRELEASED')).toBe('unverifiable') - expect(parseRelaySocketReleaseOutcome('command not found: node')).toBe('unverifiable') + expect(parseRelaySocketOwnerVerdict('')).toBe('unverifiable') + expect(parseRelaySocketOwnerVerdict('LIVE\nEXITED')).toBe('unverifiable') + expect(parseRelaySocketOwnerVerdict('command not found: node')).toBe('unverifiable') }) }) -describe('releaseUnownedRelaySocket', () => { +describe('requireUnownedRelaySocket', () => { beforeEach(() => { vi.mocked(execCommand).mockReset() }) - it('succeeds once the host reports the path released', async () => { - vi.mocked(execCommand).mockResolvedValueOnce('RELEASED') + it('succeeds once the host proves nothing owns the path', async () => { + vi.mocked(execCommand).mockResolvedValueOnce('EXITED') - await expect(releaseUnownedRelaySocket(conn, NODE, SOCK)).resolves.toBeUndefined() + await expect(requireUnownedRelaySocket(conn, NODE, SOCK)).resolves.toBeUndefined() }) it('refuses when a live owner still holds the socket', async () => { vi.mocked(execCommand).mockResolvedValueOnce('LIVE') - await expect(releaseUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy( + await expect(requireUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy( isRelaySocketOwnerLiveError ) }) @@ -103,7 +102,7 @@ describe('releaseUnownedRelaySocket', () => { it('refuses when ownership is merely unverifiable', async () => { vi.mocked(execCommand).mockResolvedValueOnce('UNVERIFIABLE') - await expect(releaseUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy( + await expect(requireUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy( isRelaySocketOwnerLiveError ) }) @@ -111,20 +110,11 @@ describe('releaseUnownedRelaySocket', () => { it('refuses when the command could not run at all', async () => { vi.mocked(execCommand).mockRejectedValueOnce(new Error('channel open failure')) - await expect(releaseUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy( + await expect(requireUnownedRelaySocket(conn, NODE, SOCK)).rejects.toSatisfy( isRelaySocketOwnerLiveError ) }) - it('rethrows an unconfirmed termination so its in-flight state survives', async () => { - const unconfirmed = Object.assign(new Error('socket cleanup still running'), { - sshChannelCloseConfirmed: false - }) - vi.mocked(execCommand).mockRejectedValueOnce(unconfirmed) - - await expect(releaseUnownedRelaySocket(conn, NODE, SOCK)).rejects.toBe(unconfirmed) - }) - it('propagates an abort instead of reporting an outcome', async () => { const controller = new AbortController() vi.mocked(execCommand).mockImplementationOnce(() => { @@ -133,7 +123,7 @@ describe('releaseUnownedRelaySocket', () => { }) await expect( - releaseUnownedRelaySocket(conn, NODE, SOCK, { signal: controller.signal }) + requireUnownedRelaySocket(conn, NODE, SOCK, { signal: controller.signal }) ).rejects.not.toSatisfy(isRelaySocketOwnerLiveError) }) @@ -141,10 +131,10 @@ describe('releaseUnownedRelaySocket', () => { vi.mocked(execCommand).mockResolvedValueOnce('LIVE') const cause = new Error('handshake refused') - const error = await releaseUnownedRelaySocket(conn, NODE, SOCK, { cause }).catch((err) => err) + const error = await requireUnownedRelaySocket(conn, NODE, SOCK, { cause }).catch((err) => err) expect(error.sockPath).toBe(SOCK) - expect(error.outcome).toBe('live') + expect(error.verdict).toBe('live') expect(error.cause).toBe(cause) expect(error.message).toContain('handshake refused') expect(error.message).toContain('Reset remote relay') @@ -156,7 +146,7 @@ describe('terminal causes carried by the owner error', () => { vi.mocked(execCommand).mockReset().mockResolvedValueOnce('LIVE') const mismatch = new RelayVersionMismatchError('0.1.0+new', '0.1.0+old') - const error = await releaseUnownedRelaySocket(conn, NODE, SOCK, { cause: mismatch }).catch( + const error = await requireUnownedRelaySocket(conn, NODE, SOCK, { cause: mismatch }).catch( (err) => err ) @@ -168,7 +158,7 @@ describe('terminal causes carried by the owner error', () => { it('does not classify an ordinary reconnect failure as terminal', async () => { vi.mocked(execCommand).mockReset().mockResolvedValueOnce('LIVE') - const error = await releaseUnownedRelaySocket(conn, NODE, SOCK, { + const error = await requireUnownedRelaySocket(conn, NODE, SOCK, { cause: new Error('relay sentinel timeout') }).catch((err) => err) diff --git a/src/main/ssh/ssh-relay-socket-owner.ts b/src/main/ssh/ssh-relay-socket-owner.ts index 3f4994b35d4..5a794051960 100644 --- a/src/main/ssh/ssh-relay-socket-owner.ts +++ b/src/main/ssh/ssh-relay-socket-owner.ts @@ -1,90 +1,75 @@ import type { SshConnection } from './ssh-connection' import { shellEscape } from './ssh-connection-utils' -import { execCommand, isUnconfirmedSshCommandTermination } from './ssh-relay-deploy-helpers' +import { execCommand } from './ssh-relay-deploy-helpers' /** - * What happened when we asked the host to release a relay socket path. + * Who owns a relay socket path, as observed from the client. * - * The vocabulary follows `src/shared/pty-liveness-verdict.ts`: `released` needs - * positive evidence that nothing owns the path, and anything we could not - * observe stays `unverifiable` — never read as absence. + * The vocabulary follows `src/shared/pty-liveness-verdict.ts`: `exited` needs positive + * evidence that nothing owns the path, and anything we could not observe stays + * `unverifiable` — never read as absence. */ -export type RelaySocketReleaseOutcome = 'live' | 'released' | 'unverifiable' +export type RelaySocketOwnerVerdict = 'live' | 'exited' | 'unverifiable' const PROBE_TIMEOUT_MS = 3_000 const PROBE_ATTEMPTS = 3 const PROBE_RETRY_MS = 250 -// Why one host-side script rather than probe-then-remove from here: an SSH round-trip -// between the two is wide enough for another client to bind the path, and the removal -// would then unlink a live owner's socket. The unlink is guarded by the same -// dev+ino+ctime identity the relay uses for its own stale-socket cleanup (relay.ts -// unlinkIfStillStale). +// Why this only looks: the fresh daemon reclaims a stale path itself, checking the socket's +// identity and binding in one process (relay.ts unlinkIfStillStale). Unlinking here would +// put an SSH round-trip between that check and the launch, and a relay that bound the path +// in between would be the one we displaced. // -// Why refusals alone never authorize the unlink: a live Unix listener whose accept -// backlog is full also answers ECONNREFUSED, so N refusals are an inference, not the -// positive evidence of absence this whole change is about. The unlink additionally -// requires an owner inventory that ran and did not list the path — /proc/net/unix where -// it exists, otherwise an lsof that ran and matched nothing. With neither, the answer is -// unverifiable and the socket stays. -const RELAY_SOCKET_RELEASE_SCRIPT = +// Why refusals alone are not absence: a live Unix listener whose accept backlog is full also +// answers ECONNREFUSED, so N refusals are an inference. A verdict of `exited` additionally +// needs an owner inventory that ran and did not list the path — /proc/net/unix where it +// exists, otherwise an lsof proven able to inspect this user's own processes. +const RELAY_SOCKET_OWNER_PROBE_SCRIPT = 'var net=require("net"),fs=require("fs"),p=process.argv[1],lsofProof=process.argv[2]==="lsof",' + - `left=${PROBE_ATTEMPTS},mark=null,done=false,sock=null,timer=null;` + - // "live" if an inventory lists the path, "absent" if one ran without it, null if none ran. - // Why the field strip rather than a suffix test: the pathname is everything after the + `left=${PROBE_ATTEMPTS},done=false,sock=null,timer=null;` + + // "live" if an inventory lists the path, "exited" if one ran without it, null if none ran. + // Why the column strip rather than a suffix test: the pathname is everything after the // seventh column, so a socket whose own path ends with " " + ours would match a suffix. 'function inventory(){var data;try{data=fs.readFileSync("/proc/net/unix","utf8")}' + - 'catch(e){return lsofProof?"absent":null}' + + 'catch(e){return lsofProof?"exited":null}' + 'var lines=data.split(String.fromCharCode(10));' + 'for(var i=1;i0){setTimeout(attempt,${PROBE_RETRY_MS});return}` + 'var inv=inventory();' + - 'if(inv==="live"){say("LIVE");return}' + - 'if(inv===null){say("UNVERIFIABLE");return}' + - 'release()})}attempt()' + 'say(inv==="live"?"LIVE":inv==="exited"?"EXITED":"UNVERIFIABLE")})}attempt()' -export function relaySocketReleaseCommand(nodePath: string, sockPath: string): string { - // Why lsof first: on hosts without /proc it is the only owner inventory available, and it - // answers both ways — a match proves an owner is there, and a clean run that matches - // nothing is the absence evidence the unlink needs. A missing lsof proves neither. +export function relaySocketOwnerProbeCommand(nodePath: string, sockPath: string): string { + // Why ask lsof about our own process first: an lsof that is present but cannot inspect + // anything reports nothing, and reading that silence as "no owner" is the inference this + // whole change removes. The relay runs as this same user, so an lsof that can see our + // files can see its socket. // Why -a: lsof ORs selectors by default, which would match every Unix-socket holder (#8762). return ( `sock=${shellEscape(sockPath)}; holders=; proof=none; ` + - // Why probe lsof against our own process first: an lsof that is present but cannot - // inspect anything reports nothing, and reading that silence as "no owner" is the - // inference this whole change exists to remove. The relay runs as this same user, so - // an lsof that can see our files can see its socket. 'if command -v lsof >/dev/null 2>&1 && [ -n "$(lsof -t -p $$ 2>/dev/null)" ]; then ' + 'holders=$(lsof -t -a -U "$sock" 2>/dev/null); [ -n "$holders" ] || proof=lsof; fi; ' + 'if [ -n "$holders" ]; then echo LIVE; ' + // Why argv: passing the path and proof as arguments dodges quoting issues inside -e. - `else ${shellEscape(nodePath)} -e ${shellEscape(RELAY_SOCKET_RELEASE_SCRIPT)} "$sock" "$proof"; fi` + `else ${shellEscape(nodePath)} -e ${shellEscape(RELAY_SOCKET_OWNER_PROBE_SCRIPT)} "$sock" "$proof"; fi` ) } -export function parseRelaySocketReleaseOutcome(output: string): RelaySocketReleaseOutcome { +export function parseRelaySocketOwnerVerdict(output: string): RelaySocketOwnerVerdict { const markers = new Set( output .split(/\r?\n/) .map((line) => line.trim()) - .filter((line) => line === 'LIVE' || line === 'RELEASED' || line === 'UNVERIFIABLE') + .filter((line) => line === 'LIVE' || line === 'EXITED' || line === 'UNVERIFIABLE') ) if (markers.size !== 1) { return 'unverifiable' @@ -92,7 +77,7 @@ export function parseRelaySocketReleaseOutcome(output: string): RelaySocketRelea if (markers.has('LIVE')) { return 'live' } - return markers.has('RELEASED') ? 'released' : 'unverifiable' + return markers.has('EXITED') ? 'exited' : 'unverifiable' } export class RelaySocketOwnerLiveError extends Error { @@ -100,12 +85,12 @@ export class RelaySocketOwnerLiveError extends Error { constructor( readonly sockPath: string, - readonly outcome: Exclude, + readonly verdict: Exclude, cause?: unknown ) { super( `Could not reach the relay at ${sockPath}, and its socket is ${ - outcome === 'live' ? 'still owned by a running relay' : 'of unproven ownership' + verdict === 'live' ? 'still owned by a running relay' : 'of unproven ownership' }. Leaving it in place — replacing it would strand that relay and every terminal it ` + `holds. Retry, or use "Reset remote relay" on the SSH target to stop it.${ cause instanceof Error ? ` Reconnect failed with: ${cause.message}` : '' @@ -120,42 +105,36 @@ export function isRelaySocketOwnerLiveError(err: unknown): err is RelaySocketOwn } /** - * Release a relay socket path so a fresh daemon can bind it — and only when the host - * proves nothing owns it. + * Establish that nothing owns this relay socket, so a fresh daemon may take the path. * - * Why the guard: unlinking cannot stop a relay that already bound the path. It only hides - * the live owner from the fresh launch's EADDRINUSE check, which is the one interlock that - * refuses to displace a running daemon. A failed `--connect` is loss of contact, never - * proof of death (STA-1756). + * Why this gate: a failed `--connect` is loss of contact, never proof of death. Launching + * over a path whose owner is still listening cannot stop that owner — it only hides it from + * the fresh daemon's EADDRINUSE check, and it keeps running with its PTYs forever + * (STA-1756). Reclaiming the path itself is the daemon's job, which it does under its own + * identity check. * - * POSIX hosts only: Windows relays take the named-pipe path in launchWindowsRelay, where - * there is no inode to unlink. + * POSIX hosts only: Windows relays take the named-pipe path in launchWindowsRelay. */ -export async function releaseUnownedRelaySocket( +export async function requireUnownedRelaySocket( conn: SshConnection, nodePath: string, sockPath: string, options?: { signal?: AbortSignal; cause?: unknown } ): Promise { const signal = options?.signal - let outcome: RelaySocketReleaseOutcome + let verdict: RelaySocketOwnerVerdict try { - outcome = parseRelaySocketReleaseOutcome( - await execCommand(conn, relaySocketReleaseCommand(nodePath, sockPath), { signal }) + verdict = parseRelaySocketOwnerVerdict( + await execCommand(conn, relaySocketOwnerProbeCommand(nodePath, sockPath), { signal }) ) - } catch (err) { + } catch { signal?.throwIfAborted() - // Why rethrow: an unconfirmed close means the removal may still be in flight, and the - // caller must not launch a relay over a path whose state it cannot describe. - if (isUnconfirmedSshCommandTermination(err)) { - throw err - } - // Why: a command we could not run says nothing about the owner, and the caller must not + // Why: a probe we could not run says nothing about the owner, and the caller must not // read that silence as death. - outcome = 'unverifiable' + verdict = 'unverifiable' } signal?.throwIfAborted() - if (outcome !== 'released') { - throw new RelaySocketOwnerLiveError(sockPath, outcome, options?.cause) + if (verdict !== 'exited') { + throw new RelaySocketOwnerLiveError(sockPath, verdict, options?.cause) } }