test: cover crash-path reaping and the relocation guard

electron/tests/crash-recovery.test.mjs

@@ -0,0 +1,208 @@
+// Crash-path coverage for the owned-child watchdog.
+//
+// The existing reap test in runtime.test.mjs drives `watchdog.close()`, which is
+// the GRACEFUL path: close() ends the child's stdin and the child reaps from
+// `reader.on('close')`. That is the shutdown a normal quit takes, and it is not
+// the reason the watchdog exists. The crash guard exists for the case where the
+// main process dies without running any code — SIGKILL, Task Manager, a renderer
+// OOM taking the app down — leaving the sidecars parented to nothing.
+//
+// Under Tauri that case was covered by a kernel primitive with its own test
+// (`windows_job_object_kills_assigned_child_on_drop`). The migration replaced it
+// with a userland watchdog and did not replace the test, so the whole point of
+// the component was unverified. These tests close that gap from both directions:
+// the liveness-poll mechanism in isolation, and a real SIGKILL end to end.
+import assert from 'node:assert/strict';
+import { spawn } from 'node:child_process';
+import { once } from 'node:events';
+import path from 'node:path';
+import process from 'node:process';
+import readline from 'node:readline';
+import test from 'node:test';
+import { setTimeout as delay } from 'node:timers/promises';
+import { fileURLToPath } from 'node:url';
+import { OwnedChildWatchdog } from '../watchdog.js';
+
+const quietLogger = { info() {}, warn() {}, error() {}, crash() {} };
+const electronDir = fileURLToPath(new URL('..', import.meta.url));
+const watchdogScript = path.join(electronDir, 'child-watchdog.cjs');
+
+// Sized off the watchdog's own worst case rather than an observed happy path: the
+// win32 reap runs a Win32_Process CIM snapshot (spawnSync timeout 8s) and then a
+// taskkill per tracked root (10s), and the liveness poll only ticks every 1500ms.
+// A tighter budget fails on a loaded machine while the implementation is correct.
+const REAP_BUDGET_MS = 60_000;
+const POLL_INTERVAL_MS = 250;
+
+function isAlive(pid) {
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch (error) {
+    // EPERM means the pid exists but belongs to someone else — still alive.
+    return error?.code === 'EPERM';
+  }
+}
+
+async function waitForDeath(pid, budgetMs = REAP_BUDGET_MS) {
+  const deadline = Date.now() + budgetMs;
+  while (Date.now() < deadline) {
+    if (!isAlive(pid)) return true;
+    await delay(POLL_INTERVAL_MS);
+  }
+  return false;
+}
+
+function spawnDisposable() {
+  return spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], {
+    detached: process.platform !== 'win32',
+    stdio: 'ignore',
+    windowsHide: true,
+    env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' },
+  });
+}
+
+function killIfAlive(child) {
+  if (child?.pid && child.exitCode === null && child.signalCode === null) {
+    try {
+      child.kill('SIGKILL');
+    } catch {
+      // Already gone.
+    }
+  }
+}
+
+function spawnFixture(name) {
+  return spawn(process.execPath, [path.join(electronDir, 'tests', 'fixtures', name)], {
+    stdio: ['ignore', 'pipe', 'pipe'],
+    windowsHide: true,
+    env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' },
+  });
+}
+
+// Fixtures report their spawned pid on stdout once setup is complete. Waiting for
+// that line before killing anything keeps a reap from being a false pass produced
+// by killing the fixture mid-setup.
+async function firstLine(child, stderrSink) {
+  const reader = readline.createInterface({ input: child.stdout, crlfDelay: Infinity });
+  const line = await Promise.race([
+    once(reader, 'line').then(([value]) => value),
+    once(child, 'exit').then(() => null),
+    delay(REAP_BUDGET_MS).then(() => null),
+  ]);
+  reader.close();
+  assert.ok(line, `fixture never reported a pid. stderr: ${stderrSink.join('')}`);
+  return JSON.parse(line);
+}
+
+function collectStderr(child) {
+  const chunks = [];
+  child.stderr.on('data', (chunk) => chunks.push(String(chunk)));
+  return chunks;
+}
+
+test('the liveness poll alone reaps a tracked child after the guarded parent is SIGKILLed', async (t) => {
+  // Isolates the poll (`parentIsAlive` -> `reapAndExit`) from the stdin-close
+  // path: the watchdog is started by THIS process, so its stdin stays open for
+  // the whole test, while the process it is told to guard is a separate fixture.
+  // Killing that fixture leaves the pipe intact, so a reap can only come from the
+  // poll noticing the parent is gone.
+  const owner = spawnFixture('child-owner.mjs');
+  t.after(() => killIfAlive(owner));
+  const ownerStderr = collectStderr(owner);
+  const { childPid } = await firstLine(owner, ownerStderr);
+
+  const watchdog = await OwnedChildWatchdog.start({
+    scriptPath: watchdogScript,
+    logger: quietLogger,
+    parentPid: owner.pid,
+  });
+  t.after(() => watchdog.close());
+  await watchdog.track(childPid);
+
+  assert.equal(isAlive(childPid), true, 'tracked child should be alive before the parent dies');
+
+  owner.kill('SIGKILL');
+  await once(owner, 'exit');
+
+  assert.equal(
+    await waitForDeath(childPid),
+    true,
+    `tracked child ${childPid} survived its parent being SIGKILLed — the liveness poll is not reaping`,
+  );
+});
+
+test('the watchdog refuses to kill a tracked pid the OS says is not its parent’s child', async (t) => {
+  // The ownership guard, and the reason the poll test above needs a real
+  // parent/child pair. A pid can be tracked in error, or reused by an unrelated
+  // process between track and reap; killing on the tracked pid alone would then
+  // take down something that was never ours. The watchdog only vetoes on a
+  // snapshot that POSITIVELY identifies the pid as parented elsewhere — a missing
+  // or unavailable snapshot must still reap, which is the leak fixed separately.
+  if (process.platform !== 'win32') {
+    t.skip('Parentage is established from the Win32_Process snapshot; POSIX uses the start fingerprint');
+    return;
+  }
+
+  const owner = spawnFixture('child-owner.mjs');
+  t.after(() => killIfAlive(owner));
+  const ownerStderr = collectStderr(owner);
+  await firstLine(owner, ownerStderr);
+
+  // Parented by the TEST, never by `owner` — exactly the state the guard exists
+  // to refuse.
+  const bystander = spawnDisposable();
+  t.after(() => killIfAlive(bystander));
+  await once(bystander, 'spawn');
+
+  const watchdog = await OwnedChildWatchdog.start({
+    scriptPath: watchdogScript,
+    logger: quietLogger,
+    parentPid: owner.pid,
+  });
+  t.after(() => watchdog.close());
+  await watchdog.track(bystander.pid);
+
+  owner.kill('SIGKILL');
+  await once(owner, 'exit');
+
+  // Give the reap the same budget the positive cases get, then assert the
+  // bystander is STILL alive.
+  await delay(20_000);
+  assert.equal(
+    isAlive(bystander.pid),
+    true,
+    `the watchdog killed pid ${bystander.pid}, which the OS reports as parented by this test rather than by ` +
+      'the process it was guarding — the ownership guard is not holding',
+  );
+});
+
+test('a SIGKILLed main process does not orphan its sidecar grandchild', async (t) => {
+  // End-to-end shape of the real failure, using the sidecar manager's OWN spawn
+  // options (`detached: false` on win32). Nothing in the killed process gets to
+  // run — no before-quit, no stopAll, no close.
+  //
+  // What this proves is the user-facing guarantee — a hard-killed app leaves no
+  // sidecar holding :8100 — NOT that the watchdog delivered it. On Windows libuv
+  // puts non-detached children in a KILL_ON_JOB_CLOSE job, so the OS reaps them
+  // first; confirmed by mutation, this test still passes with the liveness poll
+  // disabled. The watchdog itself is covered by the detached-child test above.
+  // Both matter: the job object is what actually protects Windows users today,
+  // and it is an undocumented libuv side effect that this test pins in place.
+  const parent = spawnFixture('crash-parent.mjs');
+  t.after(() => killIfAlive(parent));
+  const stderr = collectStderr(parent);
+
+  const { grandchildPid } = await firstLine(parent, stderr);
+  assert.equal(isAlive(grandchildPid), true, 'grandchild should be alive before the crash');
+
+  parent.kill('SIGKILL');
+  await once(parent, 'exit');
+
+  assert.equal(
+    await waitForDeath(grandchildPid),
+    true,
+    `grandchild ${grandchildPid} outlived a SIGKILLed parent — this is the orphaned-sidecar defect the ` +
+      'watchdog replaced the Windows Job Object to prevent',
+  );
+});

electron/tests/fixtures/child-owner.mjs

@@ -0,0 +1,32 @@
+// Fixture for the liveness-poll crash test. Stands in for the Electron main
+// process ONLY as the owner of a sidecar: it spawns a long-lived child, reports
+// the pid, and idles. It deliberately does NOT start a watchdog — the test owns
+// that, so the watchdog's stdin stays open when this process is killed and the
+// reap can only come from the liveness poll rather than the stdin-close path.
+//
+// The child must genuinely be a child of THIS process: the watchdog refuses to
+// kill a tracked pid the OS reports as parented elsewhere.
+//
+// `detached: true` on EVERY platform, deliberately diverging from how the sidecar
+// manager spawns (`detached: false` on win32). Non-detached Windows children are
+// reaped by libuv's own job object the moment this process dies, which would make
+// the test pass whether or not the watchdog did anything — verified by mutation:
+// disabling the liveness poll left a non-detached child dying anyway. Detaching
+// puts the child outside that job so the watchdog is the only thing that can kill
+// it, which is the whole point of the test.
+//
+// Not named *.test.mjs so the `electron/tests/**/*.test.mjs` runner ignores it.
+import { spawn } from 'node:child_process';
+import process from 'node:process';
+
+const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], {
+  detached: true,
+  stdio: 'ignore',
+  windowsHide: true,
+  env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' },
+});
+child.unref();
+
+process.stdout.write(`${JSON.stringify({ childPid: child.pid })}\n`);
+
+setInterval(() => {}, 1000);

electron/tests/fixtures/crash-parent.mjs

@@ -0,0 +1,33 @@
+// Fixture for the end-to-end crash-path test. Plays the role of the Electron main
+// process: starts the owned-child watchdog, spawns a long-lived grandchild, tracks
+// it, then idles forever. The test SIGKILLs this process — no graceful close, no
+// stopAll — and asserts the grandchild still dies.
+//
+// Not named *.test.mjs so the `electron/tests/**/*.test.mjs` runner ignores it.
+import { spawn } from 'node:child_process';
+import path from 'node:path';
+import process from 'node:process';
+import { fileURLToPath } from 'node:url';
+import { OwnedChildWatchdog } from '../../watchdog.js';
+
+const quietLogger = { info() {}, warn() {}, error() {}, crash() {} };
+const scriptPath = path.join(fileURLToPath(new URL('../../', import.meta.url)), 'child-watchdog.cjs');
+
+const watchdog = await OwnedChildWatchdog.start({ scriptPath, logger: quietLogger });
+
+// A bare interval keeps the grandchild alive with no exit path of its own, so if
+// it dies it can only be because something reaped it.
+const grandchild = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], {
+  detached: process.platform !== 'win32',
+  stdio: 'ignore',
+  windowsHide: true,
+  env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' },
+});
+
+await watchdog.track(grandchild.pid);
+
+// The test waits on this line before killing us, so tracking is guaranteed to have
+// completed first — otherwise the reap could be a false pass from a race.
+process.stdout.write(`${JSON.stringify({ grandchildPid: grandchild.pid })}\n`);
+
+setInterval(() => {}, 1000);

electron/tests/runtime.test.mjs

@@ -701,6 +701,79 @@ test('relocation guard adopts a legacy LSAT bank only while the new store is emp
   assert.equal(resolveLsatDataDir({ userDataPath, platform, env }).dataDir, newDir);
 });
 
+// The guard resolves the legacy directory from `platform` + `env`, so all three
+// platform branches are testable from any host. Without this the macOS and Linux
+// branches ship entirely unverified: the electron CI job runs windows-latest only,
+// so a wrong path there would strand every Mac and Linux user's question bank and
+// nothing would catch it. A fake statFile keeps the cases pure path resolution.
+test('relocation guard resolves the legacy bank location on every platform', () => {
+  const userDataPath = path.join(path.sep, 'app-data', 'StudyVault');
+  const cases = [
+    { platform: 'win32', env: { APPDATA: path.join('C:', 'Users', 'x', 'AppData', 'Roaming') } },
+    { platform: 'darwin', env: { HOME: path.join(path.sep, 'Users', 'x') } },
+    { platform: 'linux', env: { XDG_DATA_HOME: path.join(path.sep, 'home', 'x', '.local', 'share') } },
+    // Linux falls back to ~/.local/share when XDG_DATA_HOME is unset.
+    { platform: 'linux', env: { HOME: path.join(path.sep, 'home', 'x') } },
+    // macOS keeps its own layout rather than borrowing the XDG one.
+    { platform: 'darwin', env: { HOME: path.join(path.sep, 'Users', 'x'), XDG_DATA_HOME: path.join(path.sep, 'ignored') } },
+  ];
+
+  for (const { platform, env } of cases) {
+    const legacyDir = legacyLsatDataDir({ platform, env });
+    assert.ok(legacyDir, `${platform} must resolve a legacy dir from ${JSON.stringify(env)}`);
+    assert.equal(path.basename(legacyDir), 'LSATLab', `${platform} legacy dir must be the LSATLab leaf`);
+
+    // A populated legacy store with an empty current one is the adopt case, and it
+    // must hold identically on every platform.
+    const statFile = (target) =>
+      target === lsatStorePath(legacyDir) ? { isFile: () => true, size: 4096 } : { isFile: () => true, size: 0 };
+    const decision = resolveLsatDataDir({ userDataPath, platform, env, statFile });
+    assert.equal(decision.dataDir, legacyDir, `${platform} should adopt the populated legacy bank`);
+    assert.equal(decision.relocated, true);
+  }
+
+  assert.equal(legacyLsatDataDir({ platform: 'darwin', env: { HOME: path.join(path.sep, 'Users', 'x') } }).includes('Application Support'), true);
+});
+
+test('relocation guard never relocates onto itself and survives an unresolvable environment', () => {
+  // The same-path branch guards against "adopting" a directory that is already the
+  // active one, which would log a migration that never happened. It is unreachable
+  // while the leaves differ (`lsat-backend` vs `LSATLab`), and that is exactly what
+  // is worth pinning: if someone renames either constant into a collision, the
+  // guard would start relocating a directory onto itself and this fails first.
+  const populated = { isFile: () => true, size: 4096 };
+  for (const platform of ['win32', 'darwin', 'linux']) {
+    const env = legacyAppDataEnv(path.join(path.sep, 'same-path-root'), platform);
+    const legacyDir = legacyLsatDataDir({ platform, env });
+    const decision = resolveLsatDataDir({
+      userDataPath: path.dirname(legacyDir),
+      platform,
+      env,
+      statFile: () => populated,
+    });
+    assert.equal(decision.samePath, false, `${platform}: legacy and current dirs must not collide`);
+    assert.notEqual(decision.currentDir, decision.legacyDir);
+    // Both stores read as populated, so the current one must win outright.
+    assert.equal(decision.relocated, false, `${platform}: a populated current store is never displaced`);
+    assert.equal(decision.dataDir, decision.currentDir);
+  }
+
+  // Unresolvable environment: no APPDATA, no HOME, no XDG_DATA_HOME. The guard runs
+  // during boot before any window exists, so throwing here would be an unrecoverable
+  // startup crash rather than a degraded sidecar.
+  for (const unresolvable of [{ platform: 'win32', env: {} }, { platform: 'darwin', env: {} }, { platform: 'linux', env: {} }]) {
+    assert.equal(legacyLsatDataDir(unresolvable), null);
+    const decision = resolveLsatDataDir({
+      userDataPath: path.join(path.sep, 'app-data', 'StudyVault'),
+      ...unresolvable,
+      statFile: () => populated,
+    });
+    assert.equal(decision.legacyDir, null);
+    assert.equal(decision.relocated, false, 'no legacy base means nothing to adopt');
+    assert.equal(decision.dataDir, decision.currentDir);
+  }
+});
+
 test('LSAT authorization predicate is exact and redirect requests do not receive the token', () => {
   assert.equal(isExactLsatApiUrl('http://127.0.0.1:8100/api/questions?q=1'), true);
   assert.equal(isExactLsatApiUrl('http://localhost:8100/api/questions'), false);