mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 16:02:32 +00:00
fix(ssh): keep the endpoints a half-run superseded sweep already classified
The Windows arm and the failed-listing arm now both leave a line. The loop between them did not: socket 1 could be fully probed and classified, and an exec on socket 2 that threw took `logSupersededRelayFindings` with it — so a half-run pass and a host with nothing to sweep produced the same silence, and socket 1's verdict was lost. Only one failure class can leave that loop, and it is the one that matters: an exec whose SSH channel never confirmed close, which may still be running remotely and which `probeRelayEndpointIncumbent` rethrows by design. Every ordinary probe failure already degrades to `unverifiable` and the pass continues — a test now pins that too, so nobody "fixes" the loop into stopping on an absence of evidence. Findings are logged before the rethrow, which propagates unchanged. The added line says how far the pass got and claims nothing about the endpoints it never reached.
This commit is contained in:
@@ -171,6 +171,47 @@ describe('sweepSupersededRelayEndpoints', () => {
|
||||
expect(warn.mock.calls.flat().join('\n')).toContain('no pass ran: exec failed')
|
||||
})
|
||||
|
||||
// Same defect as the two arms above, one level down. An ordinary probe failure degrades to
|
||||
// `unverifiable` and the loop carries on, so the only way out of it mid-pass is the one case that
|
||||
// matters most: an exec whose SSH channel never confirmed close, which may still be running
|
||||
// remotely. That rethrows by design — and it used to throw past the log, losing socket 1's
|
||||
// verdict and making a half-run pass read exactly like a host with nothing to sweep.
|
||||
it('keeps the endpoints it already classified when a later probe cannot confirm termination', async () => {
|
||||
const SECOND_SOCK = `${HOME}/.orca-remote/relay-0.1.0+cafebabe1234/${SOCK_NAME}`
|
||||
const unconfirmed = Object.assign(new Error('channel close unconfirmed'), {
|
||||
sshChannelCloseConfirmed: false
|
||||
})
|
||||
const warn = warnSpy()
|
||||
execCommand
|
||||
.mockResolvedValueOnce(`${OLD_SOCK}\n${SECOND_SOCK}\n`)
|
||||
.mockResolvedValueOnce(probe(['PRESENT=yes', 'LISTEN=unknown', 'HOLDERS_SOURCE=unavailable']))
|
||||
.mockRejectedValueOnce(unconfirmed)
|
||||
|
||||
await expect(sweepSupersededRelayEndpoints(CONN, HOST, SWEEP)).rejects.toBe(unconfirmed)
|
||||
|
||||
const logged = warn.mock.calls.flat().join('\n')
|
||||
// Socket 1's verdict survives the abandon...
|
||||
expect(logged).toContain('Superseded relay unverifiable')
|
||||
expect(logged).toContain(OLD_SOCK)
|
||||
// ...and the pass says how far it got, claiming nothing about the one it never reached.
|
||||
expect(logged).toContain('stopped after 1 of 2 endpoints')
|
||||
expect(logged).not.toContain(SECOND_SOCK)
|
||||
})
|
||||
|
||||
// The loop must not stop on a probe that merely failed: that is an absence of evidence, and the
|
||||
// remaining endpoints still deserve a pass.
|
||||
it('carries on past an ordinary probe failure and classifies the rest', async () => {
|
||||
const SECOND_SOCK = `${HOME}/.orca-remote/relay-0.1.0+cafebabe1234/${SOCK_NAME}`
|
||||
execCommand
|
||||
.mockResolvedValueOnce(`${OLD_SOCK}\n${SECOND_SOCK}\n`)
|
||||
.mockRejectedValueOnce(new Error('probe blew up'))
|
||||
.mockResolvedValueOnce(probe(['PRESENT=yes', 'LISTEN=unknown', 'HOLDERS_SOURCE=unavailable']))
|
||||
|
||||
const findings = await sweepSupersededRelayEndpoints(CONN, HOST, SWEEP)
|
||||
|
||||
expect(findings.map((f) => f.outcome)).toEqual(['unverifiable', 'unverifiable'])
|
||||
})
|
||||
|
||||
it('does not run against Windows hosts, whose endpoints are named pipes', async () => {
|
||||
const warn = warnSpy()
|
||||
await expect(sweepSupersededRelayEndpoints(CONN, WINDOWS_HOST, SWEEP)).resolves.toEqual([])
|
||||
|
||||
@@ -154,20 +154,35 @@ export async function sweepSupersededRelayEndpoints(
|
||||
.slice(0, MAX_SWEPT_ENDPOINTS)
|
||||
|
||||
const findings: SupersededRelayFinding[] = []
|
||||
for (const sockPath of sockPaths) {
|
||||
options.signal?.throwIfAborted()
|
||||
const incumbent = await probeRelayEndpointIncumbent(
|
||||
conn,
|
||||
hostPlatform,
|
||||
options.nodePath,
|
||||
sockPath,
|
||||
{ signal: options.signal }
|
||||
try {
|
||||
for (const sockPath of sockPaths) {
|
||||
options.signal?.throwIfAborted()
|
||||
const incumbent = await probeRelayEndpointIncumbent(
|
||||
conn,
|
||||
hostPlatform,
|
||||
options.nodePath,
|
||||
sockPath,
|
||||
{ signal: options.signal }
|
||||
)
|
||||
findings.push({
|
||||
sockPath,
|
||||
outcome: await applySupersededRelayDecision(conn, incumbent, options),
|
||||
incumbent
|
||||
})
|
||||
}
|
||||
} catch (err) {
|
||||
// Why log before rethrowing: a probe or a reap that throws on socket 2 of N already classified
|
||||
// socket 1, and those lines are the whole point of this pass. Dropping them made a half-run
|
||||
// sweep read exactly like a host with nothing to sweep — the same defect the Windows arm above
|
||||
// has, one level down. The throw still propagates unchanged; the caller separates
|
||||
// RelayProbeCleanupUnconfirmedError from the rest. The count says how much of the pass ran, and
|
||||
// claims nothing about the endpoints it never reached.
|
||||
logSupersededRelayFindings(findings)
|
||||
console.warn(
|
||||
`[ssh-relay] Superseded relay sweep stopped after ${findings.length} of ${sockPaths.length} ` +
|
||||
`endpoints; the rest were not examined: ${err instanceof Error ? err.message : String(err)}`
|
||||
)
|
||||
findings.push({
|
||||
sockPath,
|
||||
outcome: await applySupersededRelayDecision(conn, incumbent, options),
|
||||
incumbent
|
||||
})
|
||||
throw err
|
||||
}
|
||||
logSupersededRelayFindings(findings)
|
||||
return findings
|
||||
|
||||
Reference in New Issue
Block a user