diff --git a/src/main/ssh/ssh-relay-launch-ownership.integration.test.ts b/src/main/ssh/ssh-relay-launch-ownership.integration.test.ts index 4f6afcf4661..6eeb0981ff8 100644 --- a/src/main/ssh/ssh-relay-launch-ownership.integration.test.ts +++ b/src/main/ssh/ssh-relay-launch-ownership.integration.test.ts @@ -21,7 +21,9 @@ import { launchRelay } from './ssh-relay-deploy' import { LiveRelayFixture, createLocalShellConnection, + delay, isProcessAlive, + killProcessTree, relayBundleDirForHost } from './ssh-relay-live-daemon-harness' import { isRelaySocketOwnerLiveError } from './ssh-relay-socket-owner' @@ -39,6 +41,10 @@ if (process.env.ORCA_REQUIRE_RELAY_BUNDLE === '1' && !BUNDLE_DIR) { type RelayStatus = { pid: number; ptys: { active: number } } +const hostPlatform = getRemoteHostPlatform( + process.platform === 'darwin' ? 'darwin-arm64' : 'linux-x64' +) + describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('launchRelay socket ownership', () => { let fixture: LiveRelayFixture | null = null let root: string | null = null @@ -74,9 +80,6 @@ describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('launchRelay socket // that rewrote the file after the daemon had already claimed the socket. live.rotateCredential() - const hostPlatform = getRemoteHostPlatform( - process.platform === 'darwin' ? 'darwin-arm64' : 'linux-x64' - ) const error = await launchRelay( createLocalShellConnection(), root, @@ -92,4 +95,42 @@ describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('launchRelay socket expect(isProcessAlive(incumbent.pid)).toBe(true) expect(live.logMark('replacement')).toBe(0) }) + + // Why this pairs with the test above: reset no longer unlinks the socket it stopped, so the + // inode a killed relay leaves behind has to be the deploy path's problem — and it is. + it('takes over the socket a killed relay left behind', async () => { + root = mkdtempSync(join(tmpdir(), 'orca-rel-')) + const live = new LiveRelayFixture(root, BUNDLE_DIR!, REPO_ROOT, TARGET_ID) + fixture = live + live.launchDaemon('incumbent') + expect(await live.waitForSocket()).toBe(true) + + const bridge = live.openBridge() + await bridge.waitForSentinel() + const incumbent = await bridge.request('relay.status') + bridge.close() + + // A relay stopped by reset (or a crash) leaves its socket inode behind. + killProcessTree(incumbent.pid) + for (let attempt = 0; attempt < 50 && isProcessAlive(incumbent.pid); attempt++) { + await delay(100) + } + expect(existsSync(live.sockPath)).toBe(true) + + const launched = await launchRelay( + createLocalShellConnection(), + root, + hostPlatform, + process.execPath, + 0, + TARGET_ID + ) + + expect(launched.sockPath).toBe(live.sockPath) + const fresh = live.openBridge() + await fresh.waitForSentinel() + const status = await fresh.request('relay.status') + expect(status.pid).not.toBe(incumbent.pid) + fresh.close() + }) }) diff --git a/src/main/ssh/ssh-relay-reset.test.ts b/src/main/ssh/ssh-relay-reset.test.ts index 7bdd5ca550b..62bc07fd3b8 100644 --- a/src/main/ssh/ssh-relay-reset.test.ts +++ b/src/main/ssh/ssh-relay-reset.test.ts @@ -187,7 +187,9 @@ describe('forceStopRelayForTarget', () => { expect(command).toContain('lsof -t -a -U "$1"') expect(command).toContain('pgrep -f "$sock_name"') expect(command).toContain('/proc/net/unix') - expect(command).toContain('rm -f "$sock"') + // Why not: a stopped relay unlinks its own socket, and one that was killed leaves an + // inode the next deploy releases under its identity guard (STA-1756). + expect(command).not.toContain('rm -f') }) it.skipIf(process.platform === 'win32')( @@ -198,7 +200,6 @@ describe('forceStopRelayForTarget', () => { expect(result.killCalls.join(' ')).toContain('-TERM') expect(result.killCalls.join(' ')).not.toContain(UNRELATED_PID) expect(result.ownerAlive).toBe(false) - expect(result.socketExists).toBe(false) expect(result.error).toBeNull() } ) @@ -210,7 +211,6 @@ describe('forceStopRelayForTarget', () => { expect(result.pgrepCalls.length).toBeGreaterThan(0) expect(result.ownerAlive).toBe(false) - expect(result.socketExists).toBe(false) expect(result.error).toBeNull() } ) diff --git a/src/main/ssh/ssh-relay-reset.ts b/src/main/ssh/ssh-relay-reset.ts index e04d50336d8..a0410b863eb 100644 --- a/src/main/ssh/ssh-relay-reset.ts +++ b/src/main/ssh/ssh-relay-reset.ts @@ -10,6 +10,15 @@ import { relaySocketNameForInstanceId } from './ssh-relay-instance-id' const RESET_NO_OWNER_PROOF_MARKER = 'ORCA-RELAY-RESET-NO-OWNER-PROOF' const RESET_OWNER_SURVIVED_MARKER = 'ORCA-RELAY-RESET-OWNER-SURVIVED' +/** + * Stop the relay owning this target's socket. + * + * Why it no longer removes the socket: a relay that stops cleanly unlinks its own path, and + * one that had to be killed leaves an inode the next deploy releases under its identity + * guard. Removing it here would need the same guard and would still race a relay binding the + * path between the check and the unlink — for no gain, since the leftover inode blocks + * nothing. + */ export async function forceStopRelayForTarget( conn: SshConnection, relayInstanceId: string @@ -32,7 +41,7 @@ export async function forceStopRelayForTarget( ' fi', '}', // Why an inventory rather than "no pid found": a lookup that returned nothing may - // simply have been unable to look, and reset must not read that silence as absence. + // simply have been unable to look, and reset must not read that silence as a stop. // 0 = a process holds the socket, 1 = an inventory ran and none does, 2 = none ran. 'socket_listed() {', ' if [ -r /proc/net/unix ]; then', @@ -46,27 +55,25 @@ 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', ' sleep 0.2', ' fi', // Why re-check both: a refused kill (permissions, a wedged process) leaves the owner - // listening, and removing its socket then hides it instead of stopping it. + // running, and reporting success then tells the user a relay was stopped that was not. ' socket_listed "$sock"', ' listed=$?', ' find_holder "$sock"', - ' if [ "$listed" = 2 ]; then', - ` echo ${RESET_NO_OWNER_PROOF_MARKER}`, - ' continue', - ' fi', ' if [ "$listed" = 0 ] || [ -n "$holder" ]; then', ` echo ${RESET_OWNER_SURVIVED_MARKER}`, - ' continue', + ' elif [ "$listed" = 2 ] && [ "$signalled" = no ]; then', + ` echo ${RESET_NO_OWNER_PROOF_MARKER}`, ' fi', - ' rm -f "$sock"', ' done', 'fi' ].join('\n') @@ -74,14 +81,14 @@ export async function forceStopRelayForTarget( const output = await execCommand(conn, script) if (output.includes(RESET_OWNER_SURVIVED_MARKER)) { throw new Error( - 'The remote relay is still running after the reset signal, so its socket was left in ' + - 'place rather than stranding it. Stop the relay process on the host, then retry.' + 'The remote relay is still running after the reset signal. Stop the relay process on ' + + 'the host, then retry.' ) } if (output.includes(RESET_NO_OWNER_PROOF_MARKER)) { throw new Error( 'This host offers no way to see which process owns the relay socket (no /proc/net/unix ' + - 'and no lsof), so it was left in place. Stop the relay process on the host, then retry.' + 'and no lsof), so nothing was stopped. Stop the relay process on the host, then retry.' ) } }