fix(metro): report what the dev-server stop proved instead of assuming success

[thymikee]

Problem

When a prepared dev server never became ready, stopSpawnedMetroProcess treated a refused SIGTERM as cleanup finished and said nothing: a Metro left running behind a failed prepareMetroRuntime was reported stopped, with no pid to act on. Same class as #3540.

Change

ensureMetroProcessReady's cleanup runs on stopProcessAndConfirmExit from #3551 and reports what the stop proved: stopped leaves the readiness failure carrying no cleanup claim; a retained verdict names the pid, reason, and the refusing signal/errno; a cleanup fault that is not a stop outcome travels as the cause. Both builders share host-kit's verdict projection and never-signalled wording, so the two surfaces cannot drift.

startMetroProcess records the start time the host reports for the pid it spawned and passes it as the stop's identity, so a pid inherited during the readiness timeout is proven stale rather than signaled, and a start time the host will not confirm retains without signaling.

That verdict also survives the bridge wrapper, which flattens the inner message into a bridgeError= fragment: it is forwarded in details, verdict keys only, gated on the typed reason — the companion's own logPath and retry advice describe a file this caller did not ask about and would outrank this command's details.

Stack

Depends on #3551, whose stopper signature this calls. The daemon and app-log stoppers are not migrated here — that needs the identity-pinned wait moved out of daemon-contracts first.

Validated: check:affected --run exit 0 at 57873fc42.

[cubic-dev-ai]

All reported issues were addressed across 3 files

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	5.19 MB	5.19 MB	+717 B
Package (unpacked)	5.19 MB	5.19 MB	+717 B
Package (download)	1.56 MB	1.56 MB	+169 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	30.0 ms	29.1 ms	-0.8 ms
CLI --help	81.0 ms	83.9 ms	+2.9 ms

[thymikee]

The PR is ready at 523add4. I found nothing that needs a change before merge.

Not blocking, and you can take or leave these: metroStartupFailure in src/metro/client-metro.ts (around line 442) repeats the same four-line retained-verdict projection and the identity-unverified wording as companionStopFailure in src/client/client-companion-tunnel.ts (from #3551), so one shared helper beside ConfirmedProcessStop in @agent-device/host-kit/process could serve both builders. The comment near line 869 says npx execs into the bundler, but npm 7+ npx spawns it as a child, so the recorded argv stays the npx one; the rationale can go, since startTime alone is enough, and configureProxyBridgeViaCompanion could forward only the verdict keys instead of every detail key (it now also passes the companion's logPath and hint).

The three cubic-dev-ai threads are already fixed at this head, so please resolve them: the startTime stop identity (#3575 (comment)), the errno assertion in the companion test (#3575 (comment)), and the retained identity-unverified test case (#3575 (comment)).

Smoke Tests is still queued, not failed. The diff only touches the Metro prepare cleanup and the bridge-wrapper details forwarding, and Smoke does not exercise a Metro readiness timeout, so I expect no overlap. I did not run the tests, so I judged that the new tests catch the old behavior by reading the pre-change code. I also did not check the npx spawn behavior live. Every Metro test mocks readProcessStartTime; on a host where it returns null for a fresh npx pid, Metro is now left running and reported instead of signaled. That is by design, but nothing has run it live.

Before merge, base PR #3551 has to land first and Smoke Tests has to finish.

[thymikee]

Three quality items taken since the last review, rebased onto #3551's 9d7f73b5f.

Shared verdict reporting. metroStartupFailure and companionStopFailure were paraphrasing the same verdict — projecting reason/signal/errno/pid by hand, and worded the never-signalled case separately. Both now call host-kit's confirmedProcessStopDetails and describeUnconfirmedProcessStop, which sit beside ConfirmedProcessStop and were added in #3551 so the shared rule and its reporting stay in one file. Metro's sentence is now Dev server pid 987 could not be confirmed as the process this run started, so no signal was sent to it — the same claim the companion makes, replacing "so it was left alone". Dropping either projection fails 4 or 1 tests in the verdict matrix.

npx rationale removed. The comment near the stop said npx execs into the bundler, so argv changes. That is wrong for npm 7+, which spawns the package binary as a child. The claim was never load-bearing — startTime alone is the recorded claim, and no command matcher is passed — so the comment now just says what the start time is for.

Forwarding narrowed. configureProxyBridgeViaCompanion re-threw with the companion's whole details, so the companion's statePath, logPath and retry advice landed under a Metro command's error, where they describe a file this caller did not ask about and would outrank this command's own details. It now forwards only the verdict, via readConfirmedProcessStopDetails, which gates on the typed reason rather than error text and returns nothing for a foreign failure. The wrapper test's mock now carries those three keys and asserts they do not arrive; forwarding error.details again fails it.

pnpm check:affected --run exit 0 recorded at 57873fc42 (381 gross over #3551). The three review threads — pid-only target, errno through the wrapper, and the missing identity-unverified matrix row — stay fixed here and remain resolved.