test: make the leak oracle's after-close phase name the session that closed

Re-review of #1859: `after-close` accepted every capture descriptor and legacy
marker, so the closed session's unfinalized handle was indistinguishable from
another session's legitimately live one — the phase could not fail, the same
shape as the surviving-daemon blocker, and it left half of B1's stated scope
undelivered.

- the observation carries the session directories closed at the checkpoint; a
  capture handle must be finalized once its owning session is gone (every
  session after shutdown, only the closed ones after close), while another
  session's live handle stays expected and a legacy pid marker is never a
  finish record
- the oracle accepts `closedSessions` and a `sessionsDir` override, normalizing
  entries to the canonical `sessions/<name>/…` shape so an in-process harness
  rooted directly at its own sessions dir is classified the same way
- add a real route regression: a provider-backed session with a live screen
  recording is closed through the daemon route, and the oracle refuses a
  descriptor left `lifecycle: "open"`. Reverting the close-route finalization
  (session-close-lifecycle-teardown.ts, the #1325 fix) turns it red naming
  sessions/default/screen-recording.resource.json; with the pre-fix model it
  stays green, which is the P1 in one line

Refs #1781 #1431

Author: thymikee

test/integration/provider-scenarios/session-close-leak-oracle.test.ts

@@ -0,0 +1,76 @@
+import assert from 'node:assert/strict';
+import path from 'node:path';
+import { expect, test } from 'vitest';
+import { assertRpcOk } from './assertions.ts';
+import {
+  createAndroidRecordingScenarioHarness,
+  withAndroidRecordingScenario,
+} from './android-recording-fixtures.ts';
+import { createAndroidRecordingProvider } from './android-recording-provider-fixtures.ts';
+import { PROVIDER_SCENARIO_ANDROID } from './fixtures.ts';
+import { PARALLEL_PROVIDER_SCENARIO_TIMEOUT_MS } from './test-timeouts.ts';
+import { screenRecordingResourceStore } from '../../../src/daemon/screen-recording-resource-store.ts';
+import { assertNoDaemonLeaks } from '../support/daemon-leak-oracle.ts';
+
+// The `after-close` half of the #1781 B1 leak oracle, on a real daemon route.
+// The three CLI daemon lanes only ever shut a daemon down, so this is the one
+// place a closing session actually owns a resource: `record start` opens a
+// record-only session with a live screen-recording handle, and the session-close
+// teardown that #1325 added must finalize it. Removing that teardown step leaves
+// the descriptor `lifecycle: "open"` and turns this test red.
+test(
+  'Provider-backed integration closing a recording session leaves no unfinalized capture handle',
+  async () => {
+    await withAndroidRecordingScenario(
+      'agent-device-provider-scenario-close-leak-',
+      async (tmpDir) => {
+        const calls: string[][] = [];
+        const outputPath = path.join(tmpDir, 'close-leak.mp4');
+        const daemon = await createAndroidRecordingScenarioHarness({
+          androidAdbProvider: () => createAndroidRecordingProvider({ calls }),
+          deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID],
+        });
+        try {
+          const started = await daemon.callCommand('record', ['start', outputPath], {
+            platform: 'android',
+            serial: PROVIDER_SCENARIO_ANDROID.id,
+            recordingScope: 'device',
+          });
+          assert.equal(assertRpcOk<{ recording?: unknown }>(started).recording, 'started');
+
+          const sessionDir = daemon.sessionDir('default');
+          // The live handle is durable state: the oracle must be able to see the
+          // unfinalized descriptor it will later refuse.
+          assert.equal(
+            screenRecordingResourceStore.read(screenRecordingResourceStore.resolvePath(sessionDir))
+              .status,
+            'decoded',
+          );
+
+          await daemon.callCommand('close', [], {
+            platform: 'android',
+            serial: PROVIDER_SCENARIO_ANDROID.id,
+          });
+
+          // `daemonPids: []` on purpose: this harness runs the daemon route
+          // in-process, so there is no daemon process to own children and the
+          // state-dir arm is the whole check here. The process arm is exercised by
+          // the CLI lanes and pinned by daemon-leak-model.test.ts.
+          await expect(
+            assertNoDaemonLeaks({
+              stateDir: path.dirname(sessionDir),
+              sessionsDir: path.dirname(sessionDir),
+              daemonPids: [],
+              phase: 'after-close',
+              closedSessions: [path.basename(sessionDir)],
+              settleMs: 0,
+            }),
+          ).resolves.toBeUndefined();
+        } finally {
+          await daemon.close();
+        }
+      },
+    );
+  },
+  PARALLEL_PROVIDER_SCENARIO_TIMEOUT_MS,
+);

test/integration/support/daemon-leak-model.test.ts

@@ -30,6 +30,7 @@ function observe(overrides: Partial<DaemonLeakObservation> = {}): DaemonLeakObse
     phase: 'after-shutdown',
     processes: [],
     excludedPids: [OBSERVER_PID],
+    closedSessions: [],
     stateEntries: [],
     ...overrides,
   };
@@ -177,8 +178,8 @@ describe('state-dir residue rules', () => {
       'expected',
     ],
     [
-      'open capture during close',
-      file('sessions/d/screen-recording.resource.json', 'open'),
+      "another session's open capture during close",
+      file('sessions/other/screen-recording.resource.json', 'open'),
       'after-close',
       'expected',
     ],
@@ -200,6 +201,52 @@ describe('state-dir residue rules', () => {
   });
 });
 
+// The phase only means something if it can name the session that closed: without
+// that, the closed session's unfinalized capture handle is indistinguishable
+// from another session's legitimately live one, and `after-close` certifies
+// nothing. Wired through by the session-close route regression in
+// test/integration/provider-scenarios/session-close-leak-oracle.test.ts.
+describe('closed-session capture handles', () => {
+  const closedSessionCapture = (lifecycle: string) =>
+    file('sessions/closed-one/screen-recording.resource.json', lifecycle);
+  const afterClose = (stateEntries: StateEntry[]): DaemonLeakObservation =>
+    observe({
+      phase: 'after-close',
+      livePids: [DAEMON_PID],
+      closedSessions: ['closed-one'],
+      stateEntries,
+    });
+
+  test('the closed session must have finalized its capture handle', () => {
+    const snapshot = evaluateDaemonLeaks(afterClose([closedSessionCapture('open')]));
+
+    expect(snapshot.strayStateEntries).toEqual([
+      'sessions/closed-one/screen-recording.resource.json',
+    ]);
+    expect(hasDaemonLeaks(snapshot)).toBe(true);
+  });
+
+  test('a session that did not close may still hold a live capture handle', () => {
+    const snapshot = evaluateDaemonLeaks(
+      afterClose([file('sessions/still-open/screen-recording.resource.json', 'open')]),
+    );
+
+    expect(hasDaemonLeaks(snapshot)).toBe(false);
+  });
+
+  test('a finalized handle from the closed session is its finish record', () => {
+    const snapshot = evaluateDaemonLeaks(afterClose([closedSessionCapture('completed')]));
+
+    expect(hasDaemonLeaks(snapshot)).toBe(false);
+  });
+
+  test('a legacy pid marker is never a finish record for the closed session', () => {
+    const snapshot = evaluateDaemonLeaks(afterClose([file('sessions/closed-one/app-log.pid')]));
+
+    expect(snapshot.strayStateEntries).toEqual(['sessions/closed-one/app-log.pid']);
+  });
+});
+
 test('a clean shutdown reports no leak', () => {
   const snapshot = evaluateDaemonLeaks(
     observe({

test/integration/support/daemon-leak-model.ts

@@ -96,6 +96,13 @@ export type DaemonLeakObservation = {
   processes: readonly HostProcess[];
   /** Pids never treated as owned: the observer and its own ancestors. */
   excludedPids: readonly number[];
+  /**
+   * Session directory names closed at this checkpoint. Their capture handles
+   * must be finalized; other sessions may still hold live ones. A checkpoint
+   * that cannot say which session closed cannot tell an unfinalized handle from
+   * a legitimately live one, so `after-close` callers always pass them.
+   */
+  closedSessions: readonly string[];
   stateEntries: readonly StateEntry[];
 };
 
@@ -140,7 +147,11 @@ export function evaluateDaemonLeaks(observation: DaemonLeakObservation): DaemonL
     strayStateEntries: observation.stateEntries
       .filter(
         (entry) =>
-          classifyStateEntry(entry, observation.phase, daemonLegitimatelyAlive) === 'stray',
+          classifyStateEntry(entry, {
+            phase: observation.phase,
+            daemonLegitimatelyAlive,
+            closedSessions: observation.closedSessions,
+          }) === 'stray',
       )
       .map((entry) => entry.path)
       .sort(),
@@ -213,33 +224,44 @@ function directOwnershipReasons(
   return reasons;
 }
 
-function classifyStateEntry(
-  entry: StateEntry,
-  phase: DaemonLeakPhase,
-  daemonLegitimatelyAlive: boolean,
-): 'expected' | 'stray' {
+type StateEntryContext = {
+  phase: DaemonLeakPhase;
+  daemonLegitimatelyAlive: boolean;
+  closedSessions: readonly string[];
+};
+
+function classifyStateEntry(entry: StateEntry, context: StateEntryContext): 'expected' | 'stray' {
   if (MANAGED_TOOLS_ENTRY.test(entry.path)) return 'expected';
   if (entry.kind === 'empty-directory') return 'stray';
   if (entry.path.endsWith('.tmp')) return 'stray';
   if (DAEMON_LIVENESS_ENTRY.test(entry.path)) {
-    return daemonLegitimatelyAlive ? 'expected' : 'stray';
+    return context.daemonLegitimatelyAlive ? 'expected' : 'stray';
   }
   if (CAPTURE_DESCRIPTOR_ENTRY.test(entry.path) || LEGACY_APP_LOG_MARKER_ENTRY.test(entry.path)) {
-    return classifyCaptureEntry(entry, phase);
+    return classifyCaptureEntry(entry, context);
   }
   return EXPECTED_STATE_DIR_ENTRIES.some((matcher) => matcher.test(entry.path))
     ? 'expected'
     : 'stray';
 }
 
-// Another session may still hold a live capture while this one closes; after
-// shutdown only a completed capture record may remain (never a pid marker).
-function classifyCaptureEntry(entry: StateEntry, phase: DaemonLeakPhase): 'expected' | 'stray' {
-  if (phase === 'after-close') return 'expected';
+// A capture handle must be finalized once its owning session is gone: after
+// shutdown that is every session, after close only the sessions that closed.
+// Another session's live capture stays expected, and a legacy pid marker is
+// never a finish record, so it is stray whenever its session is gone.
+function classifyCaptureEntry(entry: StateEntry, context: StateEntryContext): 'expected' | 'stray' {
+  const owningSessionGone =
+    context.phase === 'after-shutdown' ||
+    context.closedSessions.includes(sessionDirectoryOf(entry.path) ?? '');
+  if (!owningSessionGone) return 'expected';
   if (!CAPTURE_DESCRIPTOR_ENTRY.test(entry.path)) return 'stray';
   return entry.descriptorLifecycle === 'completed' ? 'expected' : 'stray';
 }
 
+function sessionDirectoryOf(entryPath: string): string | undefined {
+  return /^sessions\/([^/]+)\//.exec(entryPath)?.[1];
+}
+
 export function formatDaemonLeakReport(snapshot: DaemonLeakSnapshot): string {
   const surviving = survivingDaemonPids(snapshot);
   return [

test/integration/support/daemon-leak-oracle.ts

@@ -37,6 +37,20 @@ export type DaemonLeakOracleOptions = {
   /** Every daemon pid the lane observed for this state dir (from daemon.json). */
   daemonPids: readonly number[];
   phase: DaemonLeakPhase;
+  /**
+   * Session directory names (not paths) closed at this checkpoint — required at
+   * `after-close`, where only they identify whose capture handle must be
+   * finalized. `sessionStore.resolveSessionDir(name)`'s basename, or the
+   * `sessionStateDir` an `open`/`close` response reports.
+   */
+  closedSessions?: readonly string[];
+  /**
+   * Where sessions live, when that is not `<stateDir>/sessions` — an in-process
+   * scenario harness roots its SessionStore directly at its own temp dir.
+   * Entries below it are normalized to the canonical `sessions/<name>/…` shape
+   * so the rules stay layout-independent.
+   */
+  sessionsDir?: string;
   /** How long stragglers may take to exit before they count as leaked. */
   settleMs?: number;
 };
@@ -53,7 +67,8 @@ async function captureDaemonLeakSnapshot(
     phase: options.phase,
     processes,
     excludedPids: [...ancestorsOf(process.pid, processes)],
-    stateEntries: readStateEntries(stateDir),
+    closedSessions: options.closedSessions ?? [],
+    stateEntries: readStateEntries(stateDir, options.sessionsDir),
   });
 }
 
@@ -82,12 +97,13 @@ export async function assertNoDaemonLeaks(options: DaemonLeakOracleOptions): Pro
 // Files, plus empty directories as their own entries (an unswept session
 // scaffold leaves no file behind), plus the `lifecycle` of durable capture
 // descriptors so the rules stay free of filesystem access.
-function readStateEntries(stateDir: string): StateEntry[] {
+function readStateEntries(stateDir: string, sessionsDir?: string): StateEntry[] {
   if (!fs.existsSync(stateDir)) return [];
+  const sessionsRoot = path.resolve(sessionsDir ?? path.join(stateDir, 'sessions'));
   const entries: StateEntry[] = [];
   for (const dirent of fs.readdirSync(stateDir, { recursive: true, withFileTypes: true })) {
     const absolute = path.join(dirent.parentPath, dirent.name);
-    const relative = path.relative(stateDir, absolute).split(path.sep).join('/');
+    const relative = toCanonicalEntryPath(absolute, stateDir, sessionsRoot);
     if (!dirent.isDirectory()) {
       entries.push({
         path: relative,
@@ -103,6 +119,16 @@ function readStateEntries(stateDir: string): StateEntry[] {
   return entries;
 }
 
+// `sessions/<name>/…` regardless of where the SessionStore is rooted.
+function toCanonicalEntryPath(absolute: string, stateDir: string, sessionsRoot: string): string {
+  const withinSessions = path.relative(sessionsRoot, absolute);
+  const relative =
+    withinSessions.startsWith('..') || path.isAbsolute(withinSessions)
+      ? path.relative(stateDir, absolute)
+      : path.join('sessions', withinSessions);
+  return relative.split(path.sep).join('/');
+}
+
 function isEmptyDirectory(directoryPath: string): boolean {
   try {
     return fs.readdirSync(directoryPath).length === 0;
@@ -184,7 +210,8 @@ function ancestorsOf(pid: number, processes: readonly HostProcess[]): Set<number
 // Standalone use (red-proofs, manual triage):
 //   node --experimental-strip-types test/integration/support/daemon-leak-oracle.ts \
 //     --state-dir <dir> --daemon-pid <pid> [--daemon-pid <pid>…] \
-//     [--phase after-shutdown|after-close] [--settle-ms <n>]
+//     [--phase after-shutdown|after-close] [--closed-session <dir-name>…]
+//     [--settle-ms <n>]
 // Prints the report and exits 1 on a leak.
 if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {
   const args = process.argv.slice(2);
@@ -196,6 +223,7 @@ if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.me
     stateDir,
     daemonPids: readFlag('--daemon-pid').map(Number),
     phase: (readFlag('--phase')[0] as DaemonLeakPhase | undefined) ?? 'after-shutdown',
+    closedSessions: readFlag('--closed-session'),
     settleMs: Number(readFlag('--settle-ms')[0] ?? DEFAULT_SETTLE_MS),
   });
   process.stdout.write(`${formatDaemonLeakReport(snapshot)}\n`);