fix(B191,B192): post-flow lifecycle hardening — classifier coverage + benign-close precision (#247)

* fix(#243): classify startAndroidRunner startup failures as RN_ANDROID_RUNNER_DOWN (B191)

startAndroidRunner has two rejection shapes the classifier missed:
'exited before readiness' (app not installed / am instrument crash) and
'Failed to spawn' (adb missing). Both are runner-down conditions thrown
inside runAndroid's try{} — they escaped as raw exceptions instead of
the structured retryable error #243 introduced.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(#244): never classify an unparseable close payload as benign (B192)

The plain-text fallback ran the session-gone regex over the FULL raw
content when JSON.parse failed, contradicting the documented invariant
(match only the error field). A non-JSON adb error mentioning 'no
active session' anywhere would be swallowed and local session state
cleared while the underlying session might still be alive. Unparseable
payloads now surface unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(B191,B192): changeset for post-flow lifecycle hardening

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: Lykhoyda

.changeset/b191-b192-post-flow-hardening.md

@@ -0,0 +1,6 @@
+---
+"rn-dev-agent-cdp": patch
+"rn-dev-agent-plugin": patch
+---
+
+fix(B191,B192): post-flow lifecycle hardening follow-ups to #243/#244. `isAndroidConnectionFailure` now also classifies `startAndroidRunner`'s startup-failure shapes (`exited before readiness`, `Failed to spawn Android runner instrumentation`) into the structured retryable `RN_ANDROID_RUNNER_DOWN` instead of letting a startup crash escape as a raw exception. And `isBenignSessionGoneError` no longer runs its session-gone regex over unparseable (non-JSON) close payloads — with no error field to scope the match to, they surface unchanged, so a real close failure whose raw text merely mentions "no active session" can't be silently swallowed.

scripts/cdp-bridge/dist/runners/rn-android-runner-client.js

@@ -354,5 +354,5 @@ function errMessage(err) {
     return err instanceof Error ? err.message : String(err);
 }
 export function isAndroidConnectionFailure(message) {
-    return /fetch failed|ECONNREFUSED|ECONNRESET|socket hang up|rn-android-runner not started|did not become ready/i.test(message);
+    return /fetch failed|ECONNREFUSED|ECONNRESET|socket hang up|rn-android-runner not started|did not become ready|Android runner instrumentation exited before readiness|Failed to spawn Android runner instrumentation/i.test(message);
 }

scripts/cdp-bridge/dist/tools/device-session-close.js

@@ -19,7 +19,10 @@ export function isBenignSessionGoneError(result) {
         envelope = JSON.parse(text);
     }
     catch {
-        return /no active session|session not found/i.test(text);
+        // B192: unparseable payload → no error field to scope the match to. Never
+        // classify as benign; surface it so a real failure can't be swallowed just
+        // because its raw text mentions the phrase.
+        return false;
     }
     if ((envelope.meta?.code ?? envelope.code) === 'SESSION_NOT_FOUND')
         return true;

scripts/cdp-bridge/src/runners/rn-android-runner-client.ts

@@ -406,5 +406,5 @@ function errMessage(err: unknown): string {
 }
 
 export function isAndroidConnectionFailure(message: string): boolean {
-  return /fetch failed|ECONNREFUSED|ECONNRESET|socket hang up|rn-android-runner not started|did not become ready/i.test(message);
+  return /fetch failed|ECONNREFUSED|ECONNRESET|socket hang up|rn-android-runner not started|did not become ready|Android runner instrumentation exited before readiness|Failed to spawn Android runner instrumentation/i.test(message);
 }

scripts/cdp-bridge/src/tools/device-session-close.ts

@@ -27,7 +27,10 @@ export function isBenignSessionGoneError(result: ToolResult): boolean {
   try {
     envelope = JSON.parse(text) as { error?: string; code?: string; meta?: { code?: string } };
   } catch {
-    return /no active session|session not found/i.test(text);
+    // B192: unparseable payload → no error field to scope the match to. Never
+    // classify as benign; surface it so a real failure can't be swallowed just
+    // because its raw text mentions the phrase.
+    return false;
   }
   if ((envelope.meta?.code ?? envelope.code) === 'SESSION_NOT_FOUND') return true;
   return /no active session|session not found/i.test(envelope.error ?? '');

scripts/cdp-bridge/test/unit/gh-243-android-runner-health.test.js

@@ -53,6 +53,15 @@ test('#243 isAndroidConnectionFailure matches both startAndroidRunner + postComm
   assert.equal(isAndroidConnectionFailure('app not started'), false);
 });
 
+// B191: startAndroidRunner has two MORE rejection shapes — instrumentation exiting before
+// readiness (app not installed, am instrument crash) and spawn failure (adb missing). Both
+// are runner-down conditions thrown inside runAndroid's try{} and must classify into the
+// structured RN_ANDROID_RUNNER_DOWN path, not escape as raw exceptions.
+test('#243/B191 isAndroidConnectionFailure matches startAndroidRunner startup-failure shapes', () => {
+  assert.equal(isAndroidConnectionFailure('Android runner instrumentation exited before readiness (code 1)\nINSTRUMENTATION_FAILED: dev.lykhoyda.rnandroidrunner.test'), true);
+  assert.equal(isAndroidConnectionFailure('Failed to spawn Android runner instrumentation: spawn adb ENOENT'), true);
+});
+
 test('#243 runAndroid returns RN_ANDROID_RUNNER_DOWN (not bare "fetch failed") on connection failure', async () => {
   _setAndroidRunnerStateForTest({
     port: 22089,

scripts/cdp-bridge/test/unit/gh-244-close-session-gone.test.js

@@ -81,3 +81,26 @@ test('#244 isBenignSessionGoneError matches only gone-session shapes', () => {
   };
   assert.equal(isBenignSessionGoneError(withHint), false);
 });
+
+// B192: an UNPARSEABLE (non-JSON) payload is an unexpected upstream shape — there is no
+// error field to scope the match to, so it must never classify as benign, even when the
+// raw text mentions the phrase (e.g. a multi-line adb error). Surfacing it is the safe path.
+test('#244/B192 non-JSON payload mentioning the phrase is NOT benign', async () => {
+  const plainText = {
+    content: [{ type: 'text', text: 'adb: error: failed to close; no active session on device emulator-5554' }],
+    isError: true,
+  };
+  assert.equal(isBenignSessionGoneError(plainText), false);
+
+  // and closeDeviceSession must surface it unchanged, leaving local state intact
+  const calls = { clear: 0, stop: 0, release: 0 };
+  const r = await closeDeviceSession({
+    hasActiveSession: () => true,
+    closeUnderlyingSession: async () => plainText,
+    clearActiveSession: () => { calls.clear++; },
+    stopFastRunner: () => { calls.stop++; },
+    releaseDeviceLock: () => { calls.release++; },
+  });
+  assert.equal(r.isError, true);
+  assert.deepEqual(calls, { clear: 0, stop: 0, release: 0 });
+});