fix(probe,codex): stop fencing native-main on a probe that could not answer

devlog/_plan/260819_unclaimed_bug_selection/170_2114_2108_fences.md

@@ -0,0 +1,103 @@
+# 170 — #2114 and #2108: the fence that could not re-ask
+
+Both reports are the same shape wearing two operating systems: a probe that
+could not get an answer, a verdict of `unknown`, and a fence that holds for the
+whole process. Every native request 503s and only `ocx restart` clears it.
+
+Shipped as PR #2130, five commits.
+
+## The three changes
+
+**#2114 — the Linux session bus.** `inspectSystemd` called every non-zero exit
+`unknown`. Its own comment was right that a non-zero status means the question
+never reached the bus — and that is exactly why the verdict was wrong. It is
+evidence about the bus, not about who owns this home.
+
+Widening on the exit code alone fails open: with the bus down `systemctl`
+cannot see a foreign unit either, so "no answer" would read as "no owner" on a
+machine that has one. The classification asks the disk instead, which needs no
+bus.
+
+**#2108 (a) — an unaskable WinSW query.** WinSW is optional, and a
+scheduler-only install has neither its XML nor its exe on disk. A timed-out
+`sc.exe query` still returned `unknown`, which outranks the disk. With both
+assets gone there is nothing for a registration to belong to.
+
+**#2108 (b) — the fence re-asks.** `startServer` takes the verdict once and
+holds it, which is why waiting never helped and restart always did. Correct for
+`foreign-ownership` — a foreign owner is a fact. Wrong for
+`ownership-unknown`, which says the probe could not answer.
+
+## What the audit caught, three rounds running
+
+This is the part worth keeping. The functional idea was right in round one; the
+**ownership model was wrong three times in a row**, and each time the symptom
+was identical — a fence with no way to re-ask, which is #2108 returning by
+another route.
+
+| Round | Defect | Why it slipped |
+|---|---|---|
+| 1 | `inspectSystemdOffline` checked ONE path, so a foreign unit in `~/.local/share/systemd/user` or an XDG override was invisible → `ownership: owned` on a foreign host | The bus-up path never had the hole; I reused its constant, not its coverage |
+| 2 | The probe decremented the refcount AND the fence's own `release()` decremented again → a fence another holder owned got lifted | `Math.max(0, ...)` floored it, so it under-counted silently instead of going negative |
+| 2 | A spent hook stayed in the map, so no LATER fence could install one | The "do not reset the budget" guard was right; its scope was not |
+| 3 | Owner-identity guard was correct but **untested** — replacing it with `if (true)` left every suite green | A third route to the same wedge, with nothing pinning it |
+
+Round 1's fail-open is the one that mattered: I wrote "a unit naming a foreign
+home stays present and still blocks" in a commit message, and the auditor proved
+it end-to-end as `ownership = owned`. The claim was true only for a unit at the
+canonical path.
+
+The fix that finally held is smaller than either attempt: the hook delegates to
+the fence's own idempotent `release()`. One fence, one payment, whoever triggers
+it. Both earlier versions were modelling the ownership in the wrong place.
+
+## Guards, and the ones that were not guards
+
+Every branch was ablated. Three that looked like guards were not:
+
+- the multi-unit conflict branch — a fail-closed decision with nothing asserting
+  it; ablating it left the suite green
+- "foreign never retries" — double-implemented, so removing either half alone
+  stayed green; only removing both failed
+- the owner-identity check — round 3's finding, above
+
+All three now have tests driven red against the real code.
+
+## Stated rather than hidden
+
+**Locale.** systemd localizes the bus-failure strings, so a non-English host
+will not match and keeps the old `unknown`. That fences rather than admits,
+which is the safe direction, but the #2114 fix does not reach every affected
+user. Forcing `LC_ALL=C` on the probe would remove the caveat and is the
+obvious follow-up; it is not done here because it changes every `systemctl`
+call the module makes.
+
+**Coverage.** These are Windows and Linux platform paths. The suites that prove
+them ran on macOS and on `ssh lidge`; the platform CI legs are the check that
+actually matters.
+
+**A hostile `XDG_CONFIG_HOME`** redirects the offline check to an
+attacker-chosen directory. The auditor raised it and then cleared it: reaching
+that requires controlling the proxy's own environment, at which point
+`CODEX_HOME` is equally controllable and the comparison is moot. No privilege
+boundary is crossed.
+
+## Coordination with #2029
+
+Draft PR #2029 edits the same function for #1939 and classifies two other bus
+messages as `absent`. This branch never touched it or its branch. The two
+agree in direction; if it lands first this reconciles rather than replaces.
+
+## Verification
+
+```
+full suite on ssh lidge      13,524 pass / 15 skip / 0 fail across 855 files
+probe + startup + boundary       96 pass / 0 fail
+bun x tsc --noEmit                exit 0
+bun run privacy:scan              exit 0
+```
+
+One CI failure was investigated and dismissed on evidence: `keyring ubuntu`
+stalled eight minutes on an apt mirror and was cancelled by the job timeout —
+an infrastructure fault with no relation to the diff.
+

src/codex/native-profile-startup.ts

@@ -15,6 +15,7 @@ import {
 import { withNativeMainExclusiveClaim } from "./native-main-claim";
 import { scrubNativeMainAuthTempResidues } from "./native-main-auth-temp";
 import { NATIVE_STAGE_SWEEP_INTERVAL_MS } from "./native-profile-stage-store";
+import type { NativeCodexOwnership } from "../integrations/native/ownership-preflight";
 
 export type NativeMainStartupGateSnapshot =
   | { status: "ready"; homeId: string | null }
@@ -310,6 +311,75 @@ export function startNativeMainStartupLifecycle(
   };
 }
 
+/**
+ * How many times a service-ownership fence will re-ask before it stops asking (#2108).
+ *
+ * A host that is permanently unaskable must not re-probe on every request forever, and a
+ * host that recovers usually does so within the first few. The budget belongs to the
+ * REASON, not to an individual fence: raising a second fence deliberately does not hand
+ * out a fresh allowance, or a caller looping over fences could spin the probe forever.
+ * It is dropped when the last fence for that reason releases.
+ */
+export const NATIVE_MAIN_OWNERSHIP_RETRY_LIMIT = 5;
+
+/** Reprobe hooks for the fences currently held, keyed by the reason they were raised for. */
+const serviceOwnershipReprobes = new Map<NativeMainServiceOwnershipBlockReason, ServiceOwnershipReprobe>();
+
+interface ServiceOwnershipReprobe {
+  readonly probe: () => NativeCodexOwnership;
+  attempts: number;
+  /** The fence that installed this hook; only its own release may drop the entry. */
+  readonly owner: NativeMainStartupLifecycle;
+  /** Releases the fence that installed this hook, exactly once. */
+  readonly spend: () => void;
+}
+
+/** Test-only: the retry budget is module state and would otherwise leak across tests. */
+export function __resetNativeMainOwnershipRetries(): void {
+  for (const entry of serviceOwnershipReprobes.values()) entry.attempts = 0;
+}
+
+/**
+ * Re-ask whether this host is still unownable, and drop the fence if it is not.
+ *
+ * `startServer` takes the ownership verdict once, at boot, and holds it for the process
+ * lifetime. For `foreign-ownership` that is correct — a foreign owner is a fact, and
+ * re-asking would only hand a determined caller a second chance. For `ownership-unknown`
+ * it is wrong: that verdict means the probe could not answer, so waiting cannot help,
+ * which is precisely why the #2108 reporter had to run `ocx restart` after every reboot.
+ *
+ * The re-probe is demand-driven rather than timed: it runs when something asks whether
+ * native-main is fenced, which is usually a request but is also the background token
+ * guardian's warmup. It is capped so a permanently unaskable host cannot spin.
+ *
+ * The probe is synchronous `spawnSync` with a bounded timeout, and this function is on a
+ * request path, so the cap is what keeps a wedged host from paying that cost repeatedly.
+ */
+function reprobeServiceOwnership(reason: NativeMainServiceOwnershipBlockReason): boolean {
+  if (reason !== "ownership-unknown") return false;
+  const entry = serviceOwnershipReprobes.get(reason);
+  if (!entry) return false;
+  if (entry.attempts >= NATIVE_MAIN_OWNERSHIP_RETRY_LIMIT) return false;
+  entry.attempts += 1;
+  let answer: NativeCodexOwnership;
+  try {
+    answer = entry.probe();
+  } catch {
+    // An inspection that throws is not evidence the host became ownable.
+    return false;
+  }
+  if (answer !== "owned") return false;
+  // Release through the fence that installed this hook, and only that one.
+  //
+  // Several servers can hold a fence for the same reason while only one carries a hook, so
+  // clearing the shared refcount here would unblock fences this probe never spoke for.
+  // Decrementing here directly is just as wrong the other way: that fence's own release()
+  // would then pay a second time for one fence, leaving the count short. Delegating to the
+  // fence's idempotent release keeps exactly one payment per fence.
+  entry.spend();
+  return true;
+}
+
 function activeServiceOwnershipBlockReason(): NativeMainServiceOwnershipBlockReason | null {
   if ((serviceOwnershipRefs.get("foreign-ownership") ?? 0) > 0) return "foreign-ownership";
   if ((serviceOwnershipRefs.get("ownership-unknown") ?? 0) > 0) return "ownership-unknown";
@@ -325,10 +395,11 @@ function serviceOwnershipSnapshot(
 /** Close native-main admission without resolving or creating any CODEX_HOME artifacts. */
 export function blockNativeMainStartupForUnownedServiceHome(
   reason: NativeMainServiceOwnershipBlockReason,
+  options?: { reprobe?: () => NativeCodexOwnership },
 ): NativeMainStartupLifecycle {
   serviceOwnershipRefs.set(reason, (serviceOwnershipRefs.get(reason) ?? 0) + 1);
   let released = false;
-  return {
+  const lifecycle: NativeMainStartupLifecycle = {
     homeId: null,
     settled: Promise.resolve(serviceOwnershipSnapshot(reason)),
     async release() {
@@ -337,8 +408,25 @@ export function blockNativeMainStartupForUnownedServiceHome(
       const remaining = Math.max(0, (serviceOwnershipRefs.get(reason) ?? 0) - 1);
       if (remaining === 0) serviceOwnershipRefs.delete(reason);
       else serviceOwnershipRefs.set(reason, remaining);
+      if (serviceOwnershipReprobes.get(reason)?.owner === lifecycle) {
+        serviceOwnershipReprobes.delete(reason);
+      }
     },
   };
+  // Do NOT reset an existing budget: keying the reprobe by reason means a caller raising
+  // fences in a loop would otherwise be handed a fresh allowance each time and could spin
+  // the probe forever. But once the holder is gone its entry is removed above, so a LATER
+  // fence installs its own hook — a server started after an earlier probe must not be left
+  // needing `ocx restart`, which is the very symptom this exists to remove.
+  if (options?.reprobe && reason === "ownership-unknown" && !serviceOwnershipReprobes.has(reason)) {
+    serviceOwnershipReprobes.set(reason, {
+      probe: options.reprobe,
+      attempts: 0,
+      owner: lifecycle,
+      spend: () => { void lifecycle.release(); },
+    });
+  }
+  return lifecycle;
 }
 
 export function bindNativeMainStartupLifecycle(server: object, lifecycle: NativeMainStartupLifecycle): void {
@@ -353,7 +441,13 @@ export async function releaseNativeMainStartupLifecycle(server: object): Promise
 }
 
 export function isNativeMainTrafficBlocked(): boolean {
-  return activeServiceOwnershipBlockReason() !== null || snapshot.status === "blocked";
+  const reason = activeServiceOwnershipBlockReason();
+  if (reason !== null && reprobeServiceOwnership(reason)) {
+    // The host became ownable after boot (#2108): the fence lifts here rather than
+    // waiting for the restart the reporter had to perform by hand.
+    return activeServiceOwnershipBlockReason() !== null || snapshot.status === "blocked";
+  }
+  return reason !== null || snapshot.status === "blocked";
 }
 
 /**

src/server/index.ts

@@ -705,6 +705,11 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server<W
       ? startNativeMainStartupLifecycle(deps.nativeMainStartup)
       : blockNativeMainStartupForUnownedServiceHome(
         nativeOwnership.ownership === "foreign" ? "foreign-ownership" : "ownership-unknown",
+        // #2108: an `unknown` verdict means the probe could not answer, not that this host
+        // is unownable. Hand the fence a way to re-ask so a host that becomes answerable
+        // after boot reopens on its own instead of needing `ocx restart`. A `foreign`
+        // verdict ignores this by design — that one is a fact, not a question.
+        { reprobe: () => inspectStartupOwnership(deps).ownership },
       )
     : {
       homeId: null,

src/service-manager-probe.ts

@@ -179,6 +179,86 @@ function unitEnvValue(body: string, key: string): string | null {
   return null;
 }
 
+/**
+ * Did `systemctl --user` fail because the session bus could not be reached at all?
+ *
+ * These are the shapes reported on #2114 and #1939. The distinction that matters is
+ * "the question never left the machine" versus "systemd answered and said no" — only
+ * the former licenses reading the disk instead.
+ *
+ * **Locale caveat, stated rather than hidden:** systemd localizes these strings, so a
+ * non-English host will not match and keeps the old `unknown`. That is the safe
+ * direction — it fences rather than admits — but it does mean the fix does not reach
+ * every affected user. Forcing `LC_ALL=C` on the probe would remove the caveat and is
+ * the obvious follow-up; it is not done here because it changes every systemctl call
+ * this module makes, not just this branch.
+ */
+function busUnreachable(stderr: string): boolean {
+  const err = stderr.trim();
+  return err.includes("Failed to connect to bus")
+    || err.includes("Failed to connect to user scope bus")
+    || err.includes("Failed to get D-Bus connection")
+    || err.includes("DBUS_SESSION_BUS_ADDRESS")
+    || err.includes("System has not been booted with systemd");
+}
+
+/**
+ * Ownership from the unit file alone, for when the bus cannot answer (#2114).
+ *
+ * A unit file is proof of installation that does not require a running bus, and the homes
+ * it names are what ownership is actually decided on. What the disk cannot tell us is
+ * whether systemd has the unit LOADED, so this reports `registration: "absent"` — the
+ * honest reading of "no running manager has it" — rather than inventing a live state.
+ *
+ * A foreign home therefore still blocks, which is the whole reason this consults the disk
+ * instead of widening the exit code.
+ */
+function systemdUserUnitSearchPaths(home: string): string[] {
+  // systemd's user search path is not one directory. Checking only the canonical one and
+  // calling the rest absent is a fail-open: with the bus down a foreign unit in any other
+  // search dir is invisible, and "no answer" would be read as "no owner".
+  const xdgConfig = process.env.XDG_CONFIG_HOME?.trim();
+  const xdgData = process.env.XDG_DATA_HOME?.trim();
+  const dirs = [
+    xdgConfig ? join(xdgConfig, "systemd", "user") : join(home, ".config", "systemd", "user"),
+    join(home, ".config", "systemd", "user"),
+    xdgData ? join(xdgData, "systemd", "user") : join(home, ".local", "share", "systemd", "user"),
+    join(home, ".local", "share", "systemd", "user"),
+  ];
+  return [...new Set(dirs)].map(dir => join(dir, `${TASK}.service`));
+}
+
+function inspectSystemdOffline(home: string): ServiceManagerInstallation {
+  const candidates = systemdUserUnitSearchPaths(home);
+  const found = candidates.filter(path => artifactPresence(path) === "present");
+  if (candidates.some(path => artifactPresence(path) === "unreadable")) {
+    return unknown("the session bus is unreachable and a systemd unit could not be read");
+  }
+  if (found.length === 0) return { kind: "absent" };
+  if (found.length > 1) {
+    return unknown("the session bus is unreachable and more than one systemd unit file claims this proxy");
+  }
+  const definitionPath = found[0]!;
+  let body: string;
+  try {
+    body = readFileSync(definitionPath, "utf-8");
+  } catch (error) {
+    return unknown(`the session bus is unreachable and the systemd unit could not be read: ${String(error)}`);
+  }
+  return {
+    kind: "present",
+    claims: [{
+      backend: "systemd",
+      definitionPath,
+      homes: {
+        codexHome: unitEnvValue(body, "CODEX_HOME"),
+        opencodexHome: unitEnvValue(body, "OPENCODEX_HOME"),
+      },
+      registration: "absent",
+    }],
+  };
+}
+
 function inspectLaunchd(deps: Required<Pick<ProbeDeps, "run" | "uid" | "home">>): ServiceManagerInstallation {
   const definitionPath = join(deps.home, "Library", "LaunchAgents", `${LABEL}.plist`);
 
@@ -269,6 +349,15 @@ function inspectSystemd(deps: Required<Pick<ProbeDeps, "run" | "home">>): Servic
   if (shown.status !== 0) {
     // A missing unit still exits ZERO and says not-found; a non-zero status means
     // the question never reached the bus.
+    //
+    // That is evidence about the BUS, not evidence that a foreign service owns this home
+    // (#2114). Calling it `unknown` fences native-main for the whole process, so a laptop
+    // with no session bus answers every native request with a 503 until `ocx restart`.
+    //
+    // Widening on the exit code alone would fail open, because with the bus down systemctl
+    // cannot see a foreign unit either. So ask the disk, which needs no bus, and fall back
+    // to `unknown` for every other non-zero exit.
+    if (busUnreachable(shown.stderr)) return inspectSystemdOffline(deps.home);
     return unknown(`systemctl show exited ${String(shown.status)}: ${shown.stderr.trim()}`);
   }
 
@@ -729,6 +818,16 @@ function walkWinswChain(
   const registration = probeWinswRegistration(deps);
 
   if (xml === "absent" && exe === "absent" && registration === "absent") return { kind: "absent" };
+  // A query we could not ask is a question about a service that cannot exist: WinSW is an
+  // optional backend, and with neither its XML nor its exe on disk there is nothing for a
+  // registration to belong to. Fencing here on an `sc.exe` timeout is one of the two
+  // triggers behind #2108, where a scheduler-only install answers 503 until `ocx restart`.
+  //
+  // The disk outranks the unaskable query only when BOTH assets are gone. Either one
+  // present means a real install may be there and the old `unknown` still holds.
+  if (registration === "unknown" && xml === "absent" && exe === "absent") {
+    return { kind: "absent" };
+  }
   if (registration === "unknown") {
     return unknown("the native WinSW service registration could not be verified");
   }