Skip to content
67 changes: 66 additions & 1 deletion src/codex/native-profile-startup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -310,6 +311,59 @@ 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 cap is per fence, and it is
* reset by `release()`, so a restarted server starts fresh.
*/
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;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

/** 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 happens when a native request actually arrives rather than on a timer, so
* an idle proxy does no work, and it is capped so a permanently unaskable host cannot spin.
*/
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;
serviceOwnershipRefs.delete(reason);
serviceOwnershipReprobes.delete(reason);
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";
Expand All @@ -325,8 +379,12 @@ 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);
if (options?.reprobe && reason === "ownership-unknown") {
serviceOwnershipReprobes.set(reason, { probe: options.reprobe, attempts: 0 });
}
let released = false;
return {
homeId: null,
Expand All @@ -337,6 +395,7 @@ 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 (remaining === 0) serviceOwnershipReprobes.delete(reason);
},
};
}
Expand All @@ -353,7 +412,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";
}

/**
Expand Down
5 changes: 5 additions & 0 deletions src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
79 changes: 79 additions & 0 deletions src/service-manager-probe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,66 @@ 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 inspectSystemdOffline(definitionPath: string): ServiceManagerInstallation {
const presence = artifactPresence(definitionPath);
if (presence === "absent") return { kind: "absent" };
if (presence === "unreadable") {
return unknown("the session bus is unreachable and the systemd unit could not be read");
}
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`);

Expand Down Expand Up @@ -269,6 +329,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(definitionPath);
return unknown(`systemctl show exited ${String(shown.status)}: ${shown.stderr.trim()}`);
}

Expand Down Expand Up @@ -729,6 +798,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");
}
Expand Down
98 changes: 97 additions & 1 deletion tests/codex-service-manager-probe.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,10 @@ describe("could not ask is not an answer", () => {
* reached the bus, which is the opposite conclusion.
*/
test("a non-zero systemctl status is unknown even though a missing unit exits zero", () => {
const { run } = recorder(() => ({ status: 1, stderr: "Failed to connect to bus" }));
// Amended for #2114, not deleted: the rule still holds for every non-zero exit whose
// stderr does not prove the bus itself was unreachable. The bus-down family is handled
// by reading the unit file instead, and is asserted separately below.
const { run } = recorder(() => ({ status: 1, stderr: "Job for opencodex-proxy.service failed" }));
expect(inspectServiceManagerInstallation({ run, platform: "linux", home }).kind).toBe("unknown");
});

Expand Down Expand Up @@ -1008,3 +1011,96 @@ describe("ownership refuses what it cannot prove", () => {
expect(result.ownership).toBe("owned");
});
});

/*
* #2114: systemctl exists and runs, but the user session bus does not answer.
*
* The old branch called every non-zero exit `unknown`, which fences native-main for the
* whole process — the reporter's 503. But "the question never reached the bus" is evidence
* about the BUS, not evidence that a foreign service owns this home.
*
* Widening the exit code alone would fail open, because with the bus down systemctl cannot
* see a foreign unit either. So the classification asks the DISK, which needs no bus.
*/
describe("systemd probe: the bus is unreachable (#2114)", () => {
const BUS_DOWN = "Failed to connect to user scope bus via local transport: $DBUS_SESSION_BUS_ADDRESS and $XDG_RUNTIME_DIR not defined";

test("no unit file on disk means nothing can own this home — absent, not fenced", () => {
const { run } = recorder(() => ({ status: 1, stderr: BUS_DOWN }));

expect(inspectServiceManagerInstallation({ run, platform: "linux", home }).kind).toBe("absent");
});

test("a unit naming THIS home is still ours, read off disk", () => {
const definitionPath = writeUnit(join(home, ".codex"), join(home, ".opencodex"));
const { run } = recorder(() => ({ status: 1, stderr: BUS_DOWN }));

const result = inspectServiceManagerInstallation({ run, platform: "linux", home });

expect(result.kind).toBe("present");
// Registration is genuinely unknowable with the bus down; the claim must not invent it.
expect(result.kind === "present" && result.claims[0]?.definitionPath).toBe(definitionPath);
expect(result.kind === "present" && result.claims[0]?.homes.codexHome).toBe(join(home, ".codex"));
});

// The guard that keeps this fail-closed. A foreign unit is exactly the case the old
// `unknown` existed to protect, and it must survive the widening.
test("a unit naming a FOREIGN home still blocks", () => {
writeUnit("/other/.codex", "/other/.opencodex");
const { run } = recorder(() => ({ status: 1, stderr: BUS_DOWN }));

const result = inspectServiceManagerInstallation({ run, platform: "linux", home });

expect(result.kind).toBe("present");
expect(result.kind === "present" && result.claims[0]?.homes.codexHome).toBe("/other/.codex");
});

test("a non-bus failure is untouched — it stays unknown", () => {
const { run } = recorder(() => ({ status: 1, stderr: "Job for opencodex-proxy.service failed" }));

expect(inspectServiceManagerInstallation({ run, platform: "linux", home }).kind).toBe("unknown");
});
});

/*
* #2108: a reboot leaves native-main fenced until `ocx restart`.
*
* One trigger is a timed-out `sc.exe query`. WinSW is an optional backend, and a
* scheduler-only install has neither of its assets on disk — but a query that timed out
* returns "unknown", which outranks the disk and fences the whole process.
*
* With BOTH assets absent there is nothing for a WinSW registration to belong to, so a
* failed query is a question about a service that cannot exist.
*/
describe("WinSW probe: a timed-out query with no assets (#2108)", () => {
test("no xml and no exe means absent, even when the query could not be asked", () => {
// Only the WinSW query is unaskable. The scheduler answers absent for itself, so the
// whole verdict turns on whether the WinSW half fences over assets that are not on disk.
const { runRaw } = recorder((file, args) => args[0] === "/query"
? { status: 1, stderr: "ERROR: The system cannot find the file specified." }
: { timedOut: true });

const result = inspectServiceManagerInstallation({
platform: "win32", home, runRaw,
winswStatus: () => "unknown",
});

expect(result.kind).toBe("absent");
});

test("an unaskable query with WinSW assets present is still unknown", () => {
const dir = join(home, ".opencodex", "winsw");
mkdirSync(dir, { recursive: true });
writeFileSync(join(dir, "opencodex-proxy.xml"), "<service><id>opencodex-proxy</id></service>");
writeFileSync(join(dir, "opencodex-proxy.exe"), "MZ");

const { runRaw } = recorder(() => ({ timedOut: true }));

const result = inspectServiceManagerInstallation({
platform: "win32", home, runRaw,
winswStatus: () => "unknown",
});

expect(result.kind).toBe("unknown");
});
});
Loading
Loading