fix: bound the alias lock wait in migration apply

.changeset/1691-alias-lock-timeout.md

@@ -0,0 +1,9 @@
+---
+"@donadiosolutions/lcm": patch
+---
+
+Report a migration `apply` that cannot reconcile the destination project's
+aliases because another session holds an uncommitted alias write as a
+`migration-target-identity-unresolvable` refusal with evidence
+`alias-lock-timeout`, after a 5 second wait, instead of a generic storage
+error once the statement timeout cancels the lock request.

docs/migration-operations.md

@@ -188,6 +188,12 @@ creates identity; run `abort`, then a fresh `plan` and `apply`, which adds the m
 Rollback leaves the destination project and its alias rows in place and keeps the aliases in the
 local map.
 
+Adding aliases to an existing destination project waits for other sessions that are writing
+project aliases. If that write is still uncommitted after about 5 seconds, `apply` refuses with
+`migration-target-identity-unresolvable` (exit 5) and evidence `alias-lock-timeout`, before it
+takes the hold and without writing any alias. Run `apply` again once the other session has
+finished.
+
 `--dry-run` stops immediately after the confirmation fence, before the hold is taken or anything is
 captured. The emitted envelope reports the confirmed candidate with `applied: false`.
 

src/migration/target-identity.ts

@@ -1,4 +1,5 @@
 import {
+  PostgreSqlIdentityAliasLockTimeoutError,
   PostgreSqlIdentityAliasNormalizedPathConflictError,
   PostgreSqlIdentityAliasPathConflictError,
   PostgreSqlIdentityConflictError,
@@ -152,6 +153,12 @@ const ALIAS_CONFLICT = Object.freeze({
   evidence: "alias-conflict",
 } as const);
 
+const ALIAS_LOCK_TIMEOUT = Object.freeze({
+  status: "refuse",
+  reason: "migration-target-identity-unresolvable",
+  evidence: "alias-lock-timeout",
+} as const);
+
 function isAliasConflict(error: unknown): boolean {
   return error instanceof PostgreSqlIdentityConflictError
     || error instanceof PostgreSqlIdentityAliasPathConflictError
@@ -208,6 +215,7 @@ export async function resolveMigrationTargetProjectId(input: Readonly<{
     try {
       if (await ensureProjectAliases({ machineId: input.machineId, projectId, aliases: rows }) === null) return ALIAS_CONFLICT;
     } catch (error) {
+      if (error instanceof PostgreSqlIdentityAliasLockTimeoutError) return ALIAS_LOCK_TIMEOUT;
       if (!isAliasConflict(error)) throw error;
       return ALIAS_CONFLICT;
     }

src/storage/postgresql/identity-repository.ts

@@ -5,7 +5,19 @@ import type {
   PostgreSqlQueryExecutor,
   PostgreSqlTransactionOptions,
 } from "./contracts.js";
-import { PostgreSqlCommitOutcomeUnknownError } from "./errors.js";
+import {
+  PostgreSqlCommitOutcomeUnknownError,
+  PostgreSqlStorageOperationError,
+} from "./errors.js";
+
+/**
+ * How long the alias reconcile waits for the `lcm.project_aliases` table lock
+ * before it gives up, so a conflicting uncommitted alias write is reported
+ * well before the configured statement timeout cancels the statement.
+ */
+const ALIAS_RECONCILE_LOCK_TIMEOUT_MS = 5_000;
+/** lock_not_available (lock_timeout) and query_canceled (statement_timeout). */
+const ALIAS_LOCK_WAIT_SQLSTATES: ReadonlySet<string> = new Set(["55P03", "57014"]);
 
 export interface PostgreSqlIdentityExecutor extends PostgreSqlQueryExecutor {
   transaction<T>(
@@ -233,6 +245,26 @@ export class PostgreSqlIdentityRegistrationError extends StorageOperationError {
   }
 }
 
+/**
+ * The alias reconcile could not take the `lcm.project_aliases` table lock
+ * within its bounded wait because another session holds a conflicting
+ * uncommitted alias write. Nothing was written; retrying later is safe.
+ */
+export class PostgreSqlIdentityAliasLockTimeoutError extends StorageOperationError {
+  constructor(projectId: string) {
+    super(
+      "STORAGE_OPERATION_FAILED",
+      "postgresql",
+      projectId,
+      "identity",
+      "ensureProjectAliases",
+    );
+    this.name = "PostgreSqlIdentityAliasLockTimeoutError";
+    this.message =
+      `timed out waiting for the project alias lock while reconciling project ${projectId}; another session is writing project aliases`;
+  }
+}
+
 export class PostgreSqlIdentityCreateOutcomeUnknownError
   extends PostgreSqlCommitOutcomeUnknownError {
   constructor(readonly candidate: RemoteProject) {
@@ -588,16 +620,62 @@ export class PostgreSqlIdentityRepository {
    * TABLE. After writing, the project's rows are read again and anything but
    * exactly the requested set throws, so the inserts roll back; a `null`
    * return after a write would commit them.
+   *
+   * The lock wait is bounded: the transaction sets a local lock_timeout of
+   * {@link ALIAS_RECONCILE_LOCK_TIMEOUT_MS} around the LOCK statement only.
+   * When that statement fails with a lock or statement timeout, because a
+   * conflicting alias write is still uncommitted, the call rejects with
+   * {@link PostgreSqlIdentityAliasLockTimeoutError}. The translation happens
+   * outside the transaction: the runtime rethrows the first failed query's
+   * error ahead of anything the callback throws.
    */
   async ensureProjectAliases(input: {
     readonly machineId: string;
     readonly projectId: string;
     readonly aliases: readonly RemoteProjectAliasInput[];
   }): Promise<{ readonly inserted: number } | null> {
+    const lock: { failure?: unknown } = {};
+    try {
+      return await this.reconcileProjectAliases(input, lock);
+    } catch (error) {
+      if (
+        error === lock.failure
+        && error instanceof PostgreSqlStorageOperationError
+        && error.sqlState !== null
+        && ALIAS_LOCK_WAIT_SQLSTATES.has(error.sqlState)
+      ) throw new PostgreSqlIdentityAliasLockTimeoutError(input.projectId);
+      throw error;
+    }
+  }
+
+  private reconcileProjectAliases(input: {
+    readonly machineId: string;
+    readonly projectId: string;
+    readonly aliases: readonly RemoteProjectAliasInput[];
+  }, lock: { failure?: unknown }): Promise<{ readonly inserted: number } | null> {
     return this.executor.transaction(async (transaction) => {
+      const options = projectScopeOptions("ensureProjectAliases", [input.projectId]);
+      const prior = await transaction.query<{ setting: string }>({
+        text: "SELECT pg_catalog.current_setting('lock_timeout') AS setting",
+      }, options);
+      const previousSetting = prior.rows[0]!.setting;
       await transaction.query({
-        text: "LOCK TABLE lcm.project_aliases IN SHARE ROW EXCLUSIVE MODE",
-      }, projectScopeOptions("ensureProjectAliases", [input.projectId]));
+        text: "SELECT pg_catalog.set_config('lock_timeout', $1::pg_catalog.text, true)",
+        values: [`${ALIAS_RECONCILE_LOCK_TIMEOUT_MS}ms`],
+      }, options);
+      try {
+        await transaction.query({
+          text: "LOCK TABLE lcm.project_aliases IN SHARE ROW EXCLUSIVE MODE",
+        }, options);
+      } catch (error) {
+        // Nothing may be queried after this: the transaction is aborted.
+        lock.failure = error;
+        throw error;
+      }
+      await transaction.query({
+        text: "SELECT pg_catalog.set_config('lock_timeout', $1::pg_catalog.text, true)",
+        values: [previousSetting],
+      }, options);
       const current = await transaction.query<AliasRow>({
         text: `SELECT project_id, machine_id, path, normalized_path, linked_at
                FROM lcm.project_aliases

test/codecov-ownership/1691-alias-lock-timeout.yml

@@ -0,0 +1,9 @@
+references: ["#1691"]
+rationale: >-
+  #1691 bounds the alias table lock wait in ensureProjectAliases and reports a
+  timeout as the closed alias-lock-timeout refusal in the migration identity
+  step. The new typed error lives in the PostgreSQL identity repository and
+  the mapping in target-identity.ts. No file moves; no topology change.
+pins:
+  - ["src/storage/postgresql/identity-repository.ts", "integration-postgresql-identity"]
+  - ["src/migration/target-identity.ts", "unit-migration-cutover"]

test/migration/target-identity.test.ts

@@ -5,6 +5,7 @@ import { join } from "node:path";
 import { describe, expect, it, vi } from "vitest";
 import { lookupMigrationTargetProjectId, resolveMigrationTargetProjectId } from "../../src/migration/target-identity.js";
 import {
+  PostgreSqlIdentityAliasLockTimeoutError,
   PostgreSqlIdentityAliasNormalizedPathConflictError,
   PostgreSqlIdentityAliasPathConflictError,
   PostgreSqlIdentityConflictError,
@@ -174,6 +175,35 @@ describe("resolveMigrationTargetProjectId", () => {
       expect(result).toEqual(conflict);
     });
 
+    const lockTimeout = {
+      status: "refuse",
+      reason: "migration-target-identity-unresolvable",
+      evidence: "alias-lock-timeout",
+    };
+
+    it("reuse: an alias lock wait that timed out refuses as alias-lock-timeout before any hold (#1691)", async () => {
+      const result = await resolveMigrationTargetProjectId({
+        repository: repositoryStub({
+          resolveProject: vi.fn(async () => ({ projectId: "project-1", alias: {} as never })),
+          ensureProjectAliases: vi.fn(async () => { throw new PostgreSqlIdentityAliasLockTimeoutError("project-1"); }),
+        }),
+        ...base,
+      });
+      expect(result).toEqual(lockTimeout);
+    });
+
+    it("created: losing the canonical race, then timing out on the winner's alias lock, refuses as alias-lock-timeout (#1691)", async () => {
+      const result = await resolveMigrationTargetProjectId({
+        repository: repositoryStub({
+          resolveProject: vi.fn().mockResolvedValueOnce(null).mockResolvedValueOnce({ projectId: "winner", alias: {} as never }),
+          createProject: vi.fn(async () => { throw new PostgreSqlIdentityConflictError(MACHINE_ID, "/tmp/project", "winner", "loser"); }),
+          ensureProjectAliases: vi.fn(async () => { throw new PostgreSqlIdentityAliasLockTimeoutError("winner"); }),
+        }),
+        ...base,
+      });
+      expect(result).toEqual(lockTimeout);
+    });
+
     it.each([
       ["a lexical owner", new PostgreSqlIdentityConflictError(MACHINE_ID, "/tmp/p", "other", "project-1")],
       ["a stored path", new PostgreSqlIdentityAliasPathConflictError(MACHINE_ID, "/tmp/p", "project-1", "/a", "/b")],

test/postgresql/identity.integration.ts

@@ -10,17 +10,21 @@ import {
   type IdentityRepository,
 } from "../../src/identity-service.js";
 import { recoverMachineIdentity } from "../../src/machine-identity.js";
+import { resolveMigrationTargetProjectId } from "../../src/migration/target-identity.js";
 import { clearProjectMapCache, resolveProjectIdentity } from "../../src/project-map.js";
 import {
+  PostgreSqlIdentityAliasLockTimeoutError,
   PostgreSqlIdentityAliasNormalizedPathConflictError,
   PostgreSqlIdentityAliasPathConflictError,
   PostgreSqlIdentityCreateOutcomeUnknownError,
   PostgreSqlIdentityConflictError,
   PostgreSqlIdentityRepository,
 } from "../../src/storage/postgresql/identity-repository.js";
 import { PostgreSqlCommitOutcomeUnknownError } from "../../src/storage/postgresql/errors.js";
+import { PostgreSqlRuntime } from "../../src/storage/postgresql/runtime.js";
 import {
   assertHarnessReady,
+  settings,
   type PostgreSqlTestDatabase,
   withPostgreSqlTestDatabase,
 } from "./harness.js";
@@ -1023,4 +1027,77 @@ describe("PostgreSQL 18 machine and project identities", () => {
         .resolves.toEqual({ inserted: 0 });
     });
   });
+
+  it("a migration target alias reconcile blocked by an uncommitted alias writer refuses as alias-lock-timeout and writes nothing (#1691)", async () => {
+    await withPostgreSqlTestDatabase("identity-alias-lock-timeout", async (database) => {
+      await grantIdentityRuntimePrivileges(database);
+      // The harness runtime cancels statements and idle transactions after 5 s, which would race the 5 s lock wait.
+      const runtime = new PostgreSqlRuntime(settings(database.runtimeUrl, { statementTimeoutMs: 30_000 }));
+      try {
+        const repository = new PostgreSqlIdentityRepository(runtime);
+        const machine = await repository.registerMachine(`machine:${"8".repeat(64)}`, "Machine lock timeout");
+        const project = await repository.createProject({
+          machineId: machine.machineId, displayName: "Lock timeout target", path: "/work/lock-root", normalizedPath: "/work/lock-root",
+        });
+        const scope = { domain: "identity" as const, operation: "aliasLockTimeoutWriter", projectId: project.projectId, projectIds: [project.projectId] };
+        let writerInserted!: () => void;
+        const writerHasInserted = new Promise<void>((resolve) => { writerInserted = resolve; });
+        let releaseWriter!: () => void;
+        const writerMayRelease = new Promise<void>((resolve) => { releaseWriter = resolve; });
+        const writer = runtime.transaction(async (transaction) => {
+          await transaction.query({
+            text: "INSERT INTO lcm.project_aliases (project_id, machine_id, path, normalized_path) VALUES ($1, $2, '/work/lock-stranger', '/work/lock-stranger')",
+            values: [project.projectId, machine.machineId],
+          }, scope);
+          writerInserted();
+          await writerMayRelease;
+          throw new Error("roll the blocking alias write back");
+        }, scope).catch(() => undefined);
+        const wanted = [
+          { path: "/work/lock-root", normalizedPath: "/work/lock-root" },
+          { path: "/work/lock-moved", normalizedPath: "/work/lock-moved" },
+        ];
+        let direct: unknown;
+        let directMs = 0;
+        let outcome: unknown;
+        let timer: NodeJS.Timeout | undefined;
+        try {
+          await Promise.race([writerHasInserted, writer.then(() => { throw new Error("the writer ended before it inserted"); })]);
+          const startedAt = Date.now();
+          direct = await repository.ensureProjectAliases({ machineId: machine.machineId, projectId: project.projectId, aliases: wanted })
+            .catch((error: unknown) => error);
+          directMs = Date.now() - startedAt;
+          const resolving = resolveMigrationTargetProjectId({
+            repository, machineId: machine.machineId, displayName: "Lock timeout target",
+            authorityCanonicalPath: "/work/lock-root", aliases: ["/work/lock-moved"],
+          }).catch((error: unknown) => error);
+          // The bounded wait ends well inside the 30 s statement timeout; a hang here is the defect.
+          const deadline = new Promise<string>((resolve) => { timer = setTimeout(() => resolve("still blocked on the alias table lock"), 15_000); });
+          outcome = await Promise.race([resolving, deadline]);
+        } finally {
+          clearTimeout(timer);
+          releaseWriter();
+          await writer;
+        }
+        expect(direct).toBeInstanceOf(PostgreSqlIdentityAliasLockTimeoutError);
+        expect(directMs).toBeGreaterThanOrEqual(4_000);
+        expect(directMs).toBeLessThan(15_000);
+        expect(outcome).toEqual({
+          status: "refuse",
+          reason: "migration-target-identity-unresolvable",
+          evidence: "alias-lock-timeout",
+        });
+        const rows = await database.migrator.query<{ path: string }>({
+          text: "SELECT path FROM lcm.project_aliases WHERE project_id = $1 ORDER BY path",
+          values: [project.projectId],
+        }, { domain: "factory", operation: "aliasLockTimeoutRows" });
+        expect(rows.rows.map((row) => row.path)).toEqual(["/work/lock-root"]);
+        // With the blocker gone the same reconcile completes.
+        await expect(repository.ensureProjectAliases({ machineId: machine.machineId, projectId: project.projectId, aliases: wanted }))
+          .resolves.toEqual({ inserted: 1 });
+      } finally {
+        await runtime.close();
+      }
+    });
+  }, 60_000);
 });