fix(config): wait for complete owner metadata during first registration

src/lib/config-ownership.ts

@@ -149,6 +149,13 @@ let lastReconciledGeneration = 0;
  */
 const lockedManifestIdentity = new Map<string, string>();
 const warnedFullManifest = new Set<string>();
+/**
+ * The metadata state in which a creation wait already ran out for a home, keyed as
+ * {@link ownershipCache}. An unowned home whose manifest and owner marker keep the same identity
+ * has no publication left to wait for (a stopped creation, or a corrupt marker), so a registration
+ * must not pay the bounded wait again; either file changing re-arms a single wait.
+ */
+const expiredCreationWait = new Map<string, string>();
 /**
  * Open while this process runs `ocx uninstall` itself. Its own registrations then skip the
  * tombstone check: the same process runs removeOwnedConfigState afterwards, whose locked snapshot
@@ -190,6 +197,27 @@ function manifestIdentity(configDir: string): string | undefined {
   }
 }
 
+/**
+ * Which file each ownership metadata name refers to while a manifest exists, in the same
+ * dev:ino:size:mtime terms {@link manifestIdentity} uses, or null when no manifest exists: without
+ * one the publication wait ends in a single pass, so there is no spent wait to remember. The
+ * manifest is replaced atomically and the owner marker written once, so either file changing
+ * produces a new identity and re-arms a single wait.
+ */
+function creationWaitIdentity(configDir: string): string | null {
+  const describe = (name: string): string | null => {
+    try {
+      const entry = lstatSync(join(configDir, name), { bigint: true });
+      return `${entry.dev}:${entry.ino}:${entry.size}:${entry.mtimeNs}`;
+    } catch (error) {
+      const code = (error as NodeJS.ErrnoException).code;
+      return code === "ENOENT" ? null : `unreadable:${code ?? "unknown"}`;
+    }
+  };
+  const manifest = describe(CONFIG_UNINSTALL_MANIFEST);
+  return manifest === null ? null : `${manifest}|${describe(CONFIG_OWNER_FILE)}`;
+}
+
 export function listLiveConfigOwnershipRoots(currentConfigDir: string): ReadonlySet<string> {
   const currentRoot = ownershipCacheKey(currentConfigDir);
   const roots = new Set<string>([currentRoot]);
@@ -211,6 +239,7 @@ export function reconcileConfigOwnershipRoots(context: GenerationContext): numbe
       existsSync(join(root, ...rel.split("/"))));
     if (hasLiveOwnedPath) continue;
     ownershipCache.delete(root);
+    expiredCreationWait.delete(root);
     removed += 1;
   }
   lastReconciledGeneration = context.generation;
@@ -525,24 +554,24 @@ export function initializeConfigOwnership(configDir: string): boolean {
   const cacheKey = ownershipCacheKey(configDir);
   if (!existsSync(configDir)) {
     ownershipCache.delete(cacheKey);
+    expiredCreationWait.delete(cacheKey);
     mkdirSync(configDir, { recursive: true, mode: 0o700 });
   }
   let ownership = ownershipCache.get(cacheKey);
-  if (ownership === undefined) {
-    ownership = loadOrCreateOwnership(configDir);
-    ownershipCache.set(cacheKey, ownership);
-  } else if (ownership === null) {
-    // A home cached as unowned may have finished being created by another process since: re-read
-    // the metadata (no waiting) so a late owner marker is picked up. A root that has become empty
-    // since is claimed like an absent one; a nonempty unowned root still never is.
-    ownership = loadOwnership(configDir);
-    if (!ownership) {
-      try {
-        ownership = createOwnership(configDir);
-      } catch (error) {
-        if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error;
-        ownership = loadOwnership(configDir);
-      }
+  if (ownership === undefined || ownership === null) {
+    // A previously unowned home can enter creation later; use the same bounded publication wait,
+    // but spend it at most once per unchanged metadata pair. A manifest left without a valid
+    // owner marker — a stopped creation, or a corrupt marker — is not a creation still in
+    // progress, and registrations run on the request path: charging each of them the full wait
+    // stalls every request to that home. Either file changing re-arms a single wait.
+    const observed = creationWaitIdentity(configDir);
+    if (observed !== null && expiredCreationWait.get(cacheKey) === observed) {
+      ownership = null;
+    } else {
+      ownership = loadOrCreateOwnership(configDir);
+      const current = creationWaitIdentity(configDir);
+      if (ownership === null && current !== null) expiredCreationWait.set(cacheKey, current);
+      else expiredCreationWait.delete(cacheKey);
     }
     ownershipCache.set(cacheKey, ownership);
   }
@@ -573,13 +602,13 @@ function loadOwnershipForUninstall(configDir: string): { owner: ConfigOwner; man
   return loadOwnership(configDir) ?? tombstonedManifestWithoutOwner(configDir);
 }
 
-/** How long a first registration waits for another process that is creating the same home. */
+/** How long an uncached or unowned registration waits for another process creating the same home. */
 const CONCURRENT_CREATION_WAIT_MS = 100;
 
 /**
  * Load the ownership metadata, or create it in an empty root. Two processes can start on the same
  * absent home: the loser of the exclusive manifest write, or one that sees the manifest before the
- * owner marker (creation writes them in that order), waits briefly and loads the winner's metadata
+ * complete owner marker (creation writes them in that order), waits briefly and loads the winner's metadata
  * instead of caching the home as unowned for the rest of its life.
  */
 function loadOrCreateOwnership(configDir: string): { owner: ConfigOwner; manifest: ConfigUninstallManifest } | null {
@@ -593,7 +622,9 @@ function loadOrCreateOwnership(configDir: string): { owner: ConfigOwner; manifes
     } catch (error) {
       if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error;
     }
-    const creating = existsSync(join(configDir, CONFIG_UNINSTALL_MANIFEST)) && !existsSync(join(configDir, CONFIG_OWNER_FILE))
+    // Exclusive create publishes the owner file name before its JSON bytes are complete.
+    // Only a valid matching pair proves ownership; file presence alone must not end this wait.
+    const creating = existsSync(join(configDir, CONFIG_UNINSTALL_MANIFEST))
       && !tombstonedManifestWithoutOwner(configDir);
     if (!creating || Date.now() >= deadline) return loadOwnership(configDir);
     Bun.sleepSync(5);

structure/config-ownership.md

@@ -62,9 +62,11 @@ before the tombstone can still create its file afterwards; see the last item bel
   including one from a process that cached the ownership earlier, is refused as an uninstall in
   progress, not treated as an unowned home. A home cached as unowned is re-read on later
   registrations, so an owner marker another process writes late is picked up, and a root that has
-  become empty is claimed like an absent one. A first registration that finds a manifest without an
-  owner marker waits up to 100 ms for the process creating the home, then records nothing; uninstall
-  reports a file it writes meanwhile as a residual path.
+  become empty is claimed like an absent one. An uncached or previously unowned registration with a
+  manifest but no valid matching ownership pair waits up to 100 ms for publication to finish, including when the owner
+  file exists but its JSON is still being written. A complete pair loads immediately; a tombstoned
+  manifest without an owner is not treated as creation. Expiry records nothing, and damaged or
+  mismatched metadata never grants ownership; uninstall reports an unrecorded file as a residual.
 - An interrupted uninstall (a crash, or a path it could not remove) leaves the tombstone. Registrations
   keep refusing, and the next `ocx uninstall` finishes from the same manifest. The rerun's own cleanup
   steps (Desktop picker, disconnect, integrations) run inside `beginConfigOwnershipTeardown`, where this

tests/config/config-ownership-concurrent-record.test.ts

@@ -42,6 +42,31 @@ const childScript = (dir: string, rel: string, gate?: { ready: string; go: strin
 ].join("\n");
 const spawnChild = (dir: string, rel: string, gate?: { ready: string; go: string }) =>
   Bun.spawn([process.execPath, "-e", childScript(dir, rel, gate)], { stderr: "inherit" });
+
+/** Complete the owner's JSON at the first creation poll, without relying on process scheduling. */
+function observeOwnerPublication(dir: string, completedOwner: string | null, cacheUnowned = false): string {
+  const script = [
+    "import { writeFileSync } from \"node:fs\";",
+    "import { initializeConfigOwnership, registerOwnedConfigPath } from " + JSON.stringify(modulePath) + ";",
+    cacheUnowned ? "if (initializeConfigOwnership(" + JSON.stringify(dir) + ")) throw new Error(\"fixture unexpectedly owned before publication\");" : "",
+    // The spent publication wait is remembered per unchanged metadata pair; only a changed owner
+    // marker re-arms it, so the cached-unowned publication case needs a new incomplete marker to
+    // wait on. The unfinished variants keep theirs: unchanged files must not re-enter the wait.
+    cacheUnowned && completedOwner !== null ? "writeFileSync(" + JSON.stringify(join(dir, ".opencodex-owner.json")) + ", \"still-incomplete\");" : "",
+    "const completedOwner = " + JSON.stringify(completedOwner) + ";",
+    "const realNow = Date.now; let reads = 0;",
+    "Date.now = () => {",
+    "  reads++;",
+    "  if (completedOwner !== null && reads === 2) writeFileSync(" + JSON.stringify(join(dir, ".opencodex-owner.json")) + ", completedOwner);",
+    // An abandoned publication reaches the existing deadline without a wall-clock sleep.
+    "  return completedOwner === null ? reads * 20_000 : realNow();",
+    "};",
+    "console.log(JSON.stringify(registerOwnedConfigPath(" + JSON.stringify(dir) + ", " + JSON.stringify(join(dir, "after-publication")) + ")));",
+  ].join("\n");
+  const child = Bun.spawnSync([process.execPath, "-e", script]);
+  expect(child.exitCode, child.stderr.toString()).toBe(0);
+  return JSON.parse(child.stdout.toString()) as string;
+}
 const manifest = (dir: string) =>
   JSON.parse(readFileSync(join(dir, CONFIG_UNINSTALL_MANIFEST), "utf8")) as { paths: string[]; uninstallStartedAt?: number };
 
@@ -798,6 +823,34 @@ describe("ownership edge cases from review (#8093)", () => {
     expect(manifest(dir).paths).toContain("later");
   }));
 
+  for (const cached of [false, true]) for (const state of ["empty", "mismatched"] as const) {
+    const context = cached ? "cached-unowned" : "uncached";
+    test(`${context} registration waits for ${state} owner publication`, withHome(dir => {
+      expect(recordOwnedConfigPath(dir, join(dir, "seed"))).toBe(true);
+      const ownerPath = join(dir, ".opencodex-owner.json");
+      const owner = readFileSync(ownerPath, "utf8");
+      // Exclusive create exposes the file name before writeFileSync finishes its bytes.
+      const incomplete = state === "empty" ? "" : JSON.stringify({ ...JSON.parse(owner), ownerId: "00000000-0000-4000-8000-000000000000" });
+      writeFileSync(ownerPath, incomplete);
+      expect(observeOwnerPublication(dir, owner, cached)).toBe("recorded");
+      expect(readFileSync(ownerPath, "utf8")).toBe(owner);
+      expect(manifest(dir).paths).toContain("after-publication");
+    }));
+
+    test(`${context} unfinished ${state} owner publication stays unowned and unchanged`, withHome(dir => {
+      expect(recordOwnedConfigPath(dir, join(dir, "seed"))).toBe(true);
+      const ownerPath = join(dir, ".opencodex-owner.json");
+      const owner = JSON.parse(readFileSync(ownerPath, "utf8"));
+      const invalid = state === "empty" ? "" : JSON.stringify({ ...owner, ownerId: "00000000-0000-4000-8000-000000000000" });
+      writeFileSync(ownerPath, invalid);
+      const before = readFileSync(join(dir, CONFIG_UNINSTALL_MANIFEST), "utf8");
+      expect(observeOwnerPublication(dir, null, cached)).toBe("unowned");
+      expect(readFileSync(ownerPath, "utf8")).toBe(invalid);
+      expect(readFileSync(join(dir, CONFIG_UNINSTALL_MANIFEST), "utf8")).toBe(before);
+      expect(existsSync(join(dir, "after-publication"))).toBe(false);
+    }));
+  }
+
   test("a first registration waits for a home another process is still creating", withHome(dir => {
     expect(recordOwnedConfigPath(dir, join(dir, "seed"))).toBe(true);
     removeTreeWithRetry(join(dir, ".opencodex-owner.json"));
@@ -811,4 +864,32 @@ describe("ownership edge cases from review (#8093)", () => {
     expect(result.recorded).toBe(false);
     expect(result.waited).toBeGreaterThanOrEqual(90);
   }));
+
+  test("an unchanged ownerless manifest spends the publication wait once, not per registration", withHome(dir => {
+    expect(recordOwnedConfigPath(dir, join(dir, "seed"))).toBe(true);
+    const ownerPath = join(dir, ".opencodex-owner.json");
+    const owner = JSON.parse(readFileSync(ownerPath, "utf8"));
+    // A creation stopped after the manifest, or a corrupt marker: the pair never validates.
+    writeFileSync(ownerPath, JSON.stringify({ ...owner, ownerId: "00000000-0000-4000-8000-000000000000" }));
+    // The locked reload drops the stale cache entry; the next registration pays the one wait.
+    expect(recordOwnedConfigPath(dir, join(dir, "observer"))).toBe(false);
+    let sleeps = 0;
+    const realSleep = Bun.sleepSync;
+    Bun.sleepSync = (ms: number) => { sleeps += 1; realSleep(ms); };
+    let waitedMs = 0;
+    try {
+      const first = performance.now();
+      expect(recordOwnedConfigPath(dir, join(dir, "first"))).toBe(false);
+      waitedMs = performance.now() - first;
+      const firstSleeps = sleeps;
+      expect(firstSleeps).toBeGreaterThan(0);
+      const repeats = performance.now();
+      for (let i = 0; i < 20; i += 1) expect(recordOwnedConfigPath(dir, join(dir, `repeat-${i}`))).toBe(false);
+      // Twenty repeats stay under the single wait the first registration already paid: with the
+      // wait wrongly spent per call this would take twenty times as long.
+      expect(performance.now() - repeats).toBeLessThan(waitedMs);
+      expect(sleeps - firstSleeps).toBe(0);
+    } finally { Bun.sleepSync = realSleep; }
+    expect(waitedMs).toBeGreaterThanOrEqual(90);
+  }));
 });