fix: close remaining idle lifecycle review gaps

Why:
Recover PR #1 after the automated review loop stopped and resolve its four remaining actionable public-safety and deterministic-selection findings.

Changed:
Scan home paths in candidate names with redacted diagnostics, inspect every distribution entry including empty directories, normalize idle state-load failures into a public-safe BrokerError with a local cause, and fall back to host order unless both release timestamps are valid.

Verification:
npm run test:broker-core (228 passed); node --test client/test/public-surface.test.mjs client/test/install-distribution.test.mjs (27 passed); npm run verify:public-surface; git diff --check.

Affected:
broker-core/index.mjs, broker-core/test/broker-core.test.mjs, client/public-surface.mjs, client/test/public-surface.test.mjs, client/test/install-distribution.test.mjs, scripts/package_distribution.sh.

Refs:
#1
#1 (comment)
#1 (comment)
#1 (comment)
#1 (comment)

Session:
task-sessions/20260813-pr1-recovery

Author: VladimirBrejcha

broker-core/index.mjs

@@ -1716,12 +1716,13 @@ function sortCandidates(candidates) {
     if (tierComparison !== 0) {
       return tierComparison;
     }
-    const releaseComparison = compareNullableTimestamps(
-      right.registryEntry.lastLeaseReleasedAt,
-      left.registryEntry.lastLeaseReleasedAt,
-    );
-    if (releaseComparison !== 0) {
-      return releaseComparison;
+    const leftReleasedAt = Date.parse(left.registryEntry.lastLeaseReleasedAt ?? "");
+    const rightReleasedAt = Date.parse(right.registryEntry.lastLeaseReleasedAt ?? "");
+    if (Number.isFinite(leftReleasedAt) && Number.isFinite(rightReleasedAt)) {
+      const releaseComparison = rightReleasedAt - leftReleasedAt;
+      if (releaseComparison !== 0) {
+        return releaseComparison;
+      }
     }
     return left.index - right.index;
   });
@@ -4058,15 +4059,34 @@ export function disableIdlePolicyBroker(paths, options = {}) {
   });
 }
 
+function loadPublicSafeIdleState(loader) {
+  try {
+    return loader();
+  } catch (error) {
+    if (error instanceof BrokerError) {
+      throw error;
+    }
+    const readError = new BrokerError("Idle broker state could not be read.", {
+      reasonCode: "internal-error",
+    });
+    Object.defineProperty(readError, "cause", {
+      configurable: true,
+      value: error,
+      writable: true,
+    });
+    throw readError;
+  }
+}
+
 export function idleStatusBroker(paths, options = {}) {
   return withLeaseMutationLock(paths, () => {
     const timestamp = nowIso(options.now);
-    const state = loadBrokerState(paths, {
+    const state = loadPublicSafeIdleState(() => loadBrokerState(paths, {
       ...stateLoadOptions(options, timestamp),
       registryPersistenceDetails: {
         command: "idle.status",
       },
-    });
+    }));
     return {
       command: "idle.status",
       ok: true,
@@ -4210,12 +4230,12 @@ export function reconcileIdleBroker(paths, options = {}) {
         status: "not_configured",
       };
     }
-    const state = loadBrokerState(paths, {
+    const state = loadPublicSafeIdleState(() => loadBrokerState(paths, {
       ...stateLoadOptions(options, timestamp),
       registryPersistenceDetails: {
         command: "idle.reconcile",
       },
-    });
+    }));
     const candidates = idleEligibleCandidates(state, policy, timestamp);
     return {
       configured: true,
@@ -4239,7 +4259,8 @@ export function cleanupIdleBroker(paths, options = {}) {
   if (options.apply !== true) {
     return withLeaseMutationLock(paths, () => {
       const timestamp = nowIso(options.now);
-      const state = readBrokerStateSnapshot(paths, stateLoadOptions(options, timestamp));
+      const state = loadPublicSafeIdleState(() =>
+        readBrokerStateSnapshot(paths, stateLoadOptions(options, timestamp)));
       return idleCleanupPlan(state).publicPlan;
     }, {
       now: nowIso(options.now),
@@ -4257,12 +4278,12 @@ export function cleanupIdleBroker(paths, options = {}) {
   }
   return withLeaseMutationLock(paths, () => {
     const timestamp = nowIso(options.now);
-    const state = loadBrokerState(paths, {
+    const state = loadPublicSafeIdleState(() => loadBrokerState(paths, {
       ...stateLoadOptions(options, timestamp),
       registryPersistenceDetails: {
         command: "idle.cleanup",
       },
-    });
+    }));
     const plan = idleCleanupPlan(state);
     if (options.confirmPlanId !== plan.publicPlan.planId) {
       throw new BrokerError("Idle cleanup confirmation is stale; rerun preview and confirm the current plan.", {

broker-core/test/broker-core.test.mjs

@@ -3828,6 +3828,29 @@ test("shutdown candidates prefer the most recently released compatible alias", (
   assert.equal(selected.alias, second.alias);
 });
 
+test("candidate selection falls back to host order when either release timestamp is missing", () => {
+  const paths = makePaths();
+  writeBaseHostConfig(paths.hostConfigPath);
+  writeBaseProject(paths.projectFilePath);
+  const resolvedPaths = brokerPaths(paths);
+  initBroker(resolvedPaths, runtimeOptions(paths, { processExists: () => true }));
+  const registry = readJson(resolvedPaths.registryPath);
+  registry.aliases["ui-1"].lastLeaseReleasedAt = null;
+  registry.aliases["ui-2"].lastLeaseReleasedAt = "2026-01-01T00:00:00.000Z";
+  writeJson(resolvedPaths.registryPath, registry);
+
+  const selected = acquireLeaseBroker(resolvedPaths, {
+    actorId: "agent-host-order",
+    actorType: "agent",
+    ownerPid: process.pid,
+    processExists: (pid) => pid === process.pid,
+    purposeId: "agent-ui-session",
+    simctlAdapter: paths.simctl.adapter,
+  }).lease;
+
+  assert.equal(selected.alias, "ui-1");
+});
+
 test("idle policy is absent by default, strictly bounded, and stored outside project state", () => {
   const paths = makePaths();
   writeBaseHostConfig(paths.hostConfigPath);
@@ -3981,6 +4004,101 @@ test("idle policy read errors stay public-safe while preserving diagnostics", (t
   }
 });
 
+test("idle state read errors stay public-safe across status, reconcile, and cleanup", (t) => {
+  const originalReadFileSync = fs.readFileSync;
+  t.after(() => {
+    fs.readFileSync = originalReadFileSync;
+  });
+
+  const scenarios = [
+    {
+      name: "host config during status",
+      prepare(paths, resolvedPaths) {
+        return {
+          operation: () => idleStatusBroker(resolvedPaths, runtimeOptions(paths, { processExists: () => true })),
+          targetPath: resolvedPaths.hostConfigPath,
+        };
+      },
+    },
+    {
+      name: "registry during status",
+      prepare(paths, resolvedPaths) {
+        return {
+          operation: () => idleStatusBroker(resolvedPaths, runtimeOptions(paths, { processExists: () => true })),
+          targetPath: resolvedPaths.registryPath,
+        };
+      },
+    },
+    {
+      name: "pin during reconciliation",
+      prepare(paths, resolvedPaths) {
+        enableIdlePolicyBroker(resolvedPaths, {
+          actorId: "operator",
+          actorType: "human",
+          graceSeconds: 60,
+        });
+        const pin = createPinBroker(resolvedPaths, runtimeOptions(paths, {
+          actorId: "operator",
+          actorType: "human",
+          alias: "ui-1",
+          processExists: () => true,
+          purposeId: "agent-ui-session",
+        })).pin;
+        return {
+          operation: () => reconcileIdleBroker(resolvedPaths, runtimeOptions(paths, { processExists: () => true })),
+          targetPath: path.join(resolvedPaths.pinsDir, `${pin.pinId}.json`),
+        };
+      },
+    },
+    {
+      name: "lease during cleanup preview",
+      prepare(paths, resolvedPaths) {
+        const lease = acquireLeaseBroker(resolvedPaths, runtimeOptions(paths, {
+          actorId: "agent-cleanup-read",
+          actorType: "agent",
+          ownerPid: process.pid,
+          processExists: (pid) => pid === process.pid,
+          purposeId: "agent-ui-session",
+        })).lease;
+        return {
+          operation: () => cleanupIdleBroker(resolvedPaths, runtimeOptions(paths, {
+            processExists: (pid) => pid === process.pid,
+          })),
+          targetPath: path.join(resolvedPaths.leasesDir, `${lease.leaseId}.json`),
+        };
+      },
+    },
+  ];
+
+  for (const scenario of scenarios) {
+    fs.readFileSync = originalReadFileSync;
+    const paths = makePaths();
+    writeBaseHostConfig(paths.hostConfigPath);
+    writeBaseProject(paths.projectFilePath);
+    const resolvedPaths = brokerPaths(paths);
+    initBroker(resolvedPaths, runtimeOptions(paths, { processExists: () => true }));
+    const { operation, targetPath } = scenario.prepare(paths, resolvedPaths);
+    fs.readFileSync = (filePath, ...args) => {
+      if (filePath === targetPath) {
+        const error = new Error(`EACCES: permission denied, open '${targetPath}'`);
+        error.code = "EACCES";
+        throw error;
+      }
+      return originalReadFileSync(filePath, ...args);
+    };
+
+    assert.throws(operation, (error) => {
+      const serialized = JSON.stringify(error.payload);
+      return error instanceof BrokerError
+        && error.payload?.reasonCode === "internal-error"
+        && error.payload?.error === "Idle broker state could not be read."
+        && serialized.includes(paths.root) === false
+        && "stack" in error.payload === false
+        && error.cause?.message.includes(targetPath);
+    }, scenario.name);
+  }
+});
+
 test("malformed idle policy shapes reject non-number grace durations", () => {
   const paths = makePaths();
   writeBaseHostConfig(paths.hostConfigPath);

client/public-surface.mjs

@@ -223,7 +223,7 @@ export function scanPublicSurface({
 
   const diagnosticPathFor = (normalizedRelativeFile) => {
     let diagnosticPath = normalizedRelativeFile;
-    for (const rule of denylistRules) {
+    for (const rule of [...builtInRules, ...denylistRules]) {
       diagnosticPath = diagnosticPath.split(rule.value).join("[redacted]");
     }
     return diagnosticPath;
@@ -261,6 +261,15 @@ export function scanPublicSurface({
 
   const scanRelativePath = (normalizedRelativeFile) => {
     const diagnosticPath = diagnosticPathFor(normalizedRelativeFile);
+    for (const rule of builtInRules) {
+      if (normalizedRelativeFile.includes(rule.value)) {
+        addIssue({
+          line: 1,
+          path: diagnosticPath,
+          rule: rule.label,
+        });
+      }
+    }
     for (const rule of denylistRules) {
       if (normalizedRelativeFile.includes(rule.value)) {
         addIssue({

client/test/install-distribution.test.mjs

@@ -432,6 +432,57 @@ test("package_distribution scans copied payload filenames containing newlines",
   assert.equal(result.stderr.includes(root), false);
 });
 
+test("package_distribution scans empty copied payload directories", (t) => {
+  const root = makeTempDir();
+  const outputDir = path.join(root, "out");
+  const fakePathDir = path.join(root, "fake-path");
+  const fakeSecurityPath = path.join(fakePathDir, "security");
+  const privateDirectoryRoot = path.resolve("client/.package-distribution-public-surface-empty-directory-test");
+  const privateDirectoryPath = path.join(privateDirectoryRoot, ...root.split(path.sep).filter(Boolean));
+  const appSource = path.resolve("DerivedData/SimulatorBrokerApp/Build/Products/Release/SimulatorBrokerApp.app");
+  const createdAppSource = !fs.existsSync(appSource);
+
+  assert.equal(fs.existsSync(privateDirectoryRoot), false);
+  fs.mkdirSync(fakePathDir, { recursive: true });
+  fs.writeFileSync(fakeSecurityPath, [
+    "#!/usr/bin/env bash",
+    "printf '  1) ABC \"Developer ID Application: Example (TEAMID)\"\\n'",
+    "",
+  ].join("\n"));
+  fs.chmodSync(fakeSecurityPath, 0o755);
+  fs.mkdirSync(appSource, { recursive: true });
+  fs.mkdirSync(privateDirectoryPath, { recursive: true });
+  t.after(() => {
+    fs.rmSync(privateDirectoryRoot, { force: true, recursive: true });
+    if (createdAppSource) {
+      fs.rmSync(path.resolve("DerivedData/SimulatorBrokerApp"), { force: true, recursive: true });
+    }
+  });
+
+  const result = spawnSync("bash", [
+    path.resolve("scripts/package_distribution.sh"),
+    "--skip-build",
+    "--output-dir",
+    outputDir,
+    "--team-id",
+    "TEAMID",
+    "--signing-identity",
+    "Developer ID Application: Example (TEAMID)",
+  ], {
+    encoding: "utf8",
+    env: {
+      ...process.env,
+      HOME: root,
+      PATH: `${fakePathDir}${path.delimiter}${process.env.PATH ?? ""}`,
+    },
+  });
+
+  assert.notEqual(result.status, 0);
+  assert.match(result.stderr, /Distribution public surface verification failed/);
+  assert.match(result.stderr, /\[local-home-path\]/);
+  assert.equal(result.stderr.includes(root), false);
+});
+
 test("package scripts reject path-like archive names before cleanup", () => {
   for (const scriptPath of ["scripts/package_local.sh", "scripts/package_distribution.sh"]) {
     const root = makeTempDir();

client/test/public-surface.test.mjs

@@ -198,6 +198,28 @@ test("public surface scan applies local denylist values to relative filenames wi
   assert.equal(JSON.stringify(report).includes(privateMarker), false);
 });
 
+test("public surface scan applies the built-in home rule to relative filenames without echoing it", () => {
+  const root = makeTempDir();
+  const localHome = path.posix.join(path.posix.sep, "Users", "synthetic-operator");
+  const relativeFile = `captures${localHome}/notes.md`;
+  fs.mkdirSync(path.dirname(path.join(root, relativeFile)), { recursive: true });
+  fs.writeFileSync(path.join(root, relativeFile), "public notes\n");
+
+  const report = scanPublicSurface({
+    files: [relativeFile],
+    homePath: localHome,
+    root,
+  });
+
+  assert.equal(report.ok, false);
+  assert.deepEqual(report.issues, [{
+    line: 1,
+    path: "captures[redacted]/notes.md",
+    rule: "local-home-path",
+  }]);
+  assert.equal(JSON.stringify(report).includes(localHome), false);
+});
+
 test("public surface scan rejects tracked broker state artifacts and ignores binary content", () => {
   const root = makeTempDir();
   fs.mkdirSync(path.join(root, "fixtures"));

scripts/package_distribution.sh

@@ -59,7 +59,7 @@ scan_distribution_public_surface() {
 
   (
     cd "$bundle_root"
-    find . \( -type f -o -type l \) -print0 | LC_ALL=C sort -z > "$files_path"
+    find . -print0 | LC_ALL=C sort -z > "$files_path"
   )
 
   node --input-type=module - "$repo_root" "$bundle_root" "$files_path" "$repo_root/.public-safety.local" <<'EOF'