From ce30ed936589b775ebcaadb817870b197f455f30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 13 Oct 2026 02:08:37 +0200 Subject: [PATCH 1/2] fix(react-devtools): let the command's own failure outrank the cleanup stop `react-devtools stop` ran its action in a try/finally, so a companion whose stop could not be confirmed replaced the failure the stop was cleaning up after. Both faults are now collected and the action's wins; the stop's verdict is still reported, whatever it rejects with, when the action passed. --- src/__tests__/cli-react-devtools.test.ts | 42 ++++++++++++++++++++++++ src/cli/commands/react-devtools.ts | 28 ++++++++++------ 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/src/__tests__/cli-react-devtools.test.ts b/src/__tests__/cli-react-devtools.test.ts index c7db174e9f..6fd95f5af7 100644 --- a/src/__tests__/cli-react-devtools.test.ts +++ b/src/__tests__/cli-react-devtools.test.ts @@ -12,6 +12,7 @@ vi.mock('../client/client-react-devtools-companion.ts', () => ({ })); import { runCmdStreaming } from '@agent-device/host-kit/command'; +import { AppError } from '@agent-device/kernel/errors'; import { withCapturedStderrAsync } from './test-utils/stream-capture.ts'; import { ensureReactDevtoolsCompanion, @@ -228,6 +229,47 @@ test('react-devtools stop cleans up remote companion', async () => { }); }); +// What the companion stop answers decides the verdict of `react-devtools stop`: an unconfirmed exit +// is reported instead of a clean teardown, and the command's own fault still outranks it. +const runRemoteStop = () => + runReactDevtoolsCommand(['stop'], { + cwd: '/tmp/project', + flags: { ...remoteBridgeScope, leaseBackend: 'ios-instance', remoteConfig: '/tmp/remote.json' }, + }); + +test('react-devtools stop reports a companion whose stop could not be confirmed', async () => { + const refusal = new AppError('COMMAND_FAILED', 'companion pid 4321 is still running', { + reason: 'signal-failed', + signal: 'SIGTERM', + pid: 4321, + }); + vi.mocked(runCmdStreaming).mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' }); + vi.mocked(stopReactDevtoolsCompanion).mockRejectedValueOnce(refusal); + + await assert.rejects(runRemoteStop, refusal); +}); + +test('react-devtools stop keeps its own failure ahead of an unconfirmed companion stop', async () => { + const actionFault = new Error('devtools package failed'); + vi.mocked(runCmdStreaming).mockRejectedValueOnce(actionFault); + vi.mocked(stopReactDevtoolsCompanion).mockRejectedValueOnce( + new AppError('COMMAND_FAILED', 'companion pid 4322 is still running'), + ); + + await assert.rejects(runRemoteStop, actionFault); + assert.equal(vi.mocked(stopReactDevtoolsCompanion).mock.calls.length, 1, 'cleanup still ran'); +}); + +// The stop's own transport can reject with anything, and an unconfirmed exit may only ever be +// reported, never swallowed: the command fails on the rejection value itself, not on its shape. +test('react-devtools stop fails on a stop rejection that is not an Error', async () => { + vi.mocked(runCmdStreaming).mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' }); + vi.mocked(stopReactDevtoolsCompanion).mockRejectedValueOnce('worker exited before answering'); + + const settled = await runRemoteStop().catch((error: unknown) => error); + assert.equal(settled, 'worker exited before answering'); +}); + test('react-devtools start configures direct remote port reverse', async () => { const configureDirectPortReverse = vi.fn(async () => undefined); vi.mocked(runCmdStreaming).mockResolvedValueOnce({ diff --git a/src/cli/commands/react-devtools.ts b/src/cli/commands/react-devtools.ts index d1adb6e8bd..abca4865f9 100644 --- a/src/cli/commands/react-devtools.ts +++ b/src/cli/commands/react-devtools.ts @@ -131,16 +131,24 @@ async function withRemoteDevtoolsCompanion( flags?.remoteConfig ?? `${bridgeConfig.tenantId}:${bridgeConfig.runId}:${bridgeConfig.leaseId}`; if (args[0] === 'stop') { - try { - return await action(); - } finally { - await stopReactDevtoolsCompanion({ - projectRoot: options.cwd ?? process.cwd(), - stateDir, - profileKey, - consumerKey: session, - }); - } + // A companion that could not be confirmed stopped is this command's own verdict, so it is + // reported — but never in place of the failure the stop was cleaning up after. + const acted = await action().then( + (value) => ({ value }) as const, + (error: unknown) => ({ error }) as const, + ); + const stopped = await stopReactDevtoolsCompanion({ + projectRoot: options.cwd ?? process.cwd(), + stateDir, + profileKey, + consumerKey: session, + }).then( + () => ({}) as const, + (error: unknown) => ({ error }) as const, + ); + if ('error' in acted) throw acted.error; + if ('error' in stopped) throw stopped.error; + return acted.value; } await ensureReactDevtoolsCompanion({ From 817b6aaa2588cd8c80a7a94ece2096e6272b7a54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 13 Oct 2026 03:56:02 +0200 Subject: [PATCH 2/2] fix(react-devtools): name the surviving companion when both faults fire The precedence change dropped the stop verdict in the both-fail path: the action's fault is the right thing to throw, but an operator who saw only `devtools package failed` never learned a companion was still holding the bridge scope. It is now written to stderr beside the thrown fault, on the same channel the file already uses for its wait hint. --- src/__tests__/cli-react-devtools.test.ts | 12 ++++++++---- src/cli/commands/react-devtools.ts | 13 +++++++++---- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/__tests__/cli-react-devtools.test.ts b/src/__tests__/cli-react-devtools.test.ts index 6fd95f5af7..5b748decda 100644 --- a/src/__tests__/cli-react-devtools.test.ts +++ b/src/__tests__/cli-react-devtools.test.ts @@ -251,13 +251,17 @@ test('react-devtools stop reports a companion whose stop could not be confirmed' test('react-devtools stop keeps its own failure ahead of an unconfirmed companion stop', async () => { const actionFault = new Error('devtools package failed'); + const stopFault = new AppError('COMMAND_FAILED', 'companion pid 4322 is still running', { + reason: 'signal-failed', + pid: 4322, + }); vi.mocked(runCmdStreaming).mockRejectedValueOnce(actionFault); - vi.mocked(stopReactDevtoolsCompanion).mockRejectedValueOnce( - new AppError('COMMAND_FAILED', 'companion pid 4322 is still running'), - ); + vi.mocked(stopReactDevtoolsCompanion).mockRejectedValueOnce(stopFault); - await assert.rejects(runRemoteStop, actionFault); + const stderr = await withCapturedStderrAsync(() => assert.rejects(runRemoteStop(), actionFault)); assert.equal(vi.mocked(stopReactDevtoolsCompanion).mock.calls.length, 1, 'cleanup still ran'); + // The action's fault is what the command reports, so the surviving companion has to be named here. + assert.match(stderr, /companion pid 4322 is still running/); }); // The stop's own transport can reject with anything, and an unconfirmed exit may only ever be diff --git a/src/cli/commands/react-devtools.ts b/src/cli/commands/react-devtools.ts index abca4865f9..2eb8842cc4 100644 --- a/src/cli/commands/react-devtools.ts +++ b/src/cli/commands/react-devtools.ts @@ -3,7 +3,7 @@ import { ensureReactDevtoolsCompanion, stopReactDevtoolsCompanion, } from '../../client/client-react-devtools-companion.ts'; -import { AppError } from '@agent-device/kernel/errors'; +import { AppError, errorMessage } from '@agent-device/kernel/errors'; import { isRemoteBridgeBackend } from './remote-bridge.ts'; import type { CliFlags } from '@agent-device/contracts/command'; import { connectionProviderCapabilities } from '../connection/provider-policy.ts'; @@ -131,8 +131,8 @@ async function withRemoteDevtoolsCompanion( flags?.remoteConfig ?? `${bridgeConfig.tenantId}:${bridgeConfig.runId}:${bridgeConfig.leaseId}`; if (args[0] === 'stop') { - // A companion that could not be confirmed stopped is this command's own verdict, so it is - // reported — but never in place of the failure the stop was cleaning up after. + // An unconfirmed companion stop fails this command, except when the action failed too: then the + // action's fault is the verdict and the surviving companion is named on stderr beside it. const acted = await action().then( (value) => ({ value }) as const, (error: unknown) => ({ error }) as const, @@ -146,7 +146,12 @@ async function withRemoteDevtoolsCompanion( () => ({}) as const, (error: unknown) => ({ error }) as const, ); - if ('error' in acted) throw acted.error; + if ('error' in acted) { + if ('error' in stopped) { + process.stderr.write(`Warning: ${errorMessage(stopped.error)}\n`); + } + throw acted.error; + } if ('error' in stopped) throw stopped.error; return acted.value; }