corbitsdev · TheGreatAxios · Aug 31, 2026 · Aug 30, 2026 · Aug 30, 2026 · Aug 30, 2026
@@ -163,6 +163,7 @@ import {
   createWorkflowCommandPlugin,
 } from "@corbits/commands";
 import {
+  createDrizzleRepoReviewLeaseStore,
   createDrizzleWebhookTriggerStore,
   createWebhookIngressRoutes,
   createWebhookTriggerRoutes,
@@ -1978,6 +1979,12 @@ export async function createHub(config: HubConfig) {
     db,
     credentialCipher,
   );
+  // CL-7242: the sole concurrency backstop for the GitHub connect
+  // card's start-reviewing step -- see
+  // packages/webhook-triggers/src/repo-review-lease.ts for why this
+  // lives in our own schema rather than as any change to Interchange's
+  // `grant` table.
+  const repoReviewLeaseStore = createDrizzleRepoReviewLeaseStore(db);
   // Shared by every folded-run first-turn mail send below (webhook
   // triggers and routines alike) — a `CryptoProviderCache` is keyed by
   // instance id, which is globally unique across this hub regardless of
@@ -2248,6 +2255,19 @@ export async function createHub(config: HubConfig) {
         });
         return row?.id;
       },
+      acquireRepoReviewLease: (tenantId, repo) =>
+        repoReviewLeaseStore.acquire(tenantId, repo.name),
+      releaseRepoReviewLease: (tenantId, repo) =>
+        repoReviewLeaseStore.release(tenantId, repo.name),
+      // hasRepoGrant/mintRepoGrant go through Interchange's native
+      // grants HTTP surface (never a direct `grant` table write --
+      // see native-repo-grants.ts). That table carries no unique
+      // constraint over tenant/resource/action, so a bare read-then-
+      // POST here would itself be a duplicate-grant race; safe only
+      // because the caller in connect-github-routes.ts reaches this
+      // once `acquireRepoReviewLease` has already made this call-site
+      // single-flight per (tenant, repo) -- see
+      // packages/webhook-triggers/src/repo-review-lease.ts (CL-7242).
       hasRepoGrant: (tenantId, repo, cookies) =>
         hasRepoGrantViaHttp(selfApi, tenantId, repo, cookies),
       mintRepoGrant: (tenantId, repo, cookies) =>
@@ -2258,7 +2278,14 @@ export async function createHub(config: HubConfig) {
         codeReviewDefinitionId,
         repo,
       ) => {
-        const row = await webhookTriggerStore.create({
+        // `ensure`, not `create`: a concurrent "start reviewing" call
+        // for the same repo can race this one past `hasWebhookTrigger`
+        // above, and `webhook_trigger_tenant_definition_name_unique`
+        // (packages/webhook-triggers migration 0003, CL-7242) is what
+        // actually resolves that — the loser gets the winner's real
+        // row back instead of minting a second live trigger with a
+        // different secret.
+        const row = await webhookTriggerStore.ensure({
           id: generateId("workflowRun"),
           tenantId,
           name: webhookTriggerName(repo),

@@ -67,7 +67,17 @@ function buildHarness() {
   let connected = false;
   let subscriber: ((state: ConnectGithubQuery) => void) | undefined;
 
+  const heldLeases = new Set<string>();
+
   const setupPorts: ConnectGithubSetupPorts = {
+    async acquireRepoReviewLease(repo) {
+      if (heldLeases.has(repo.name)) return false;
+      heldLeases.add(repo.name);
+      return true;
+    },
+    async releaseRepoReviewLease(repo) {
+      heldLeases.delete(repo.name);
+    },
     async hasRepoGrant() {
       return false;
     },

@@ -4,7 +4,17 @@ export {
   type ApplyWebhookTriggersMigrationsReport,
   type WebhookTriggersMigration,
 } from "./migrations";
-export { webhookTrigger, type WebhookTriggerRow } from "./schema";
+export {
+  webhookTrigger,
+  repoReviewLease,
+  type WebhookTriggerRow,
+  type RepoReviewLeaseRow,
+} from "./schema";
+export {
+  createDrizzleRepoReviewLeaseStore,
+  type RepoReviewLeaseStore,
+  type RepoReviewLeaseDb,
+} from "./repo-review-lease";
 export {
   createDrizzleWebhookTriggerStore,
   type CreateWebhookTriggerInput,

@@ -15,6 +15,7 @@ import { type } from "arktype";
 import type { RequireGrant, TenantEnv } from "@intx/hub-api";
 import { idResource } from "@intx/hub-api";
 import { generateId } from "@intx/hub-common";
+import { pgErrorCode, PG_UNIQUE_VIOLATION } from "@intx/db";
 
 import { generateWebhookSecret } from "./signature";
 import type { WebhookTriggerRow } from "./schema";
@@ -24,6 +25,21 @@ const ErrorEnvelope = (code: string, message: string) => ({
   error: { code, message },
 });
 
+/**
+ * True for a Postgres unique-violation (`23505`) — the shape a duplicate
+ * `(tenant, workflow definition, name)` now raises through
+ * `0003_webhook_trigger_tenant_definition_name_unique`. `pgErrorCode`
+ * walks Drizzle's wrapped cause chain, since a real insert failure
+ * arrives as a `DrizzleQueryError` rather than the raw driver error;
+ * this package's in-memory test fake stamps `.code` directly to match.
+ * Never silently retried as an `ensure`: this route's `create` promises
+ * a genuinely new row, so a collision is reported to the caller as a
+ * conflict rather than handed back someone else's trigger.
+ */
+function isUniqueViolation(error: unknown): boolean {
+  return pgErrorCode(error) === PG_UNIQUE_VIOLATION;
+}
+
 const CreateTriggerBody = type({
   name: "string",
   workflowDefinitionId: "string",
@@ -95,15 +111,29 @@ export function createWebhookTriggerRoutes(
 
     const secret = generateWebhookSecret();
 
-    const row = await deps.store.create({
-      id: generateId("workflowRun"),
-      tenantId: tenant.id,
-      name: body.name,
-      workflowDefinitionId: body.workflowDefinitionId,
-      inputTemplate: body.inputTemplate,
-      secret,
-      createdBy: principal.id,
-    });
+    let row: WebhookTriggerRow;
+    try {
+      row = await deps.store.create({
+        id: generateId("workflowRun"),
+        tenantId: tenant.id,
+        name: body.name,
+        workflowDefinitionId: body.workflowDefinitionId,
+        inputTemplate: body.inputTemplate,
+        secret,
+        createdBy: principal.id,
+      });
+    } catch (cause) {
+      if (isUniqueViolation(cause)) {
+        return c.json(
+          ErrorEnvelope(
+            "conflict",
+            "a trigger with this name already exists for this workflow definition",
+          ),
+          409,
+        );
+      }
+      throw cause;
+    }
 
     return c.json({ ...publicView(row), secret }, 201);
   });

@@ -49,6 +49,55 @@ export const webhookTriggersMigrations: readonly WebhookTriggersMigration[] = [
         ON "webhook_triggers"."webhook_trigger" ("tenant_id");
     `,
   },
+  {
+    // CL-7242: startReviewingRepos's check-then-act (hasWebhookTrigger
+    // then createWebhookTrigger) reads (tenant_id, workflow_definition_id,
+    // name) with no atomic backstop, so two concurrent "start reviewing"
+    // calls for the same repo can both read "no trigger yet" and both
+    // insert -- two live triggers with the same name but different
+    // secrets. Reconcile first: a database already carrying the race's
+    // duplicates would otherwise fail CREATE UNIQUE INDEX. Keep the
+    // oldest row per tuple and delete the rest. This whole migration
+    // runs inside one transaction (applyWebhookTriggersMigrations wraps
+    // each entry in `sql.begin`), so the delete and the index build
+    // can't be split by a concurrent writer.
+    name: "0003_webhook_trigger_tenant_definition_name_unique",
+    sql: `
+      DELETE FROM "webhook_triggers"."webhook_trigger" AS t
+      USING "webhook_triggers"."webhook_trigger" AS older
+      WHERE t.tenant_id = older.tenant_id
+        AND t.workflow_definition_id = older.workflow_definition_id
+        AND t.name = older.name
+        AND (older.created_at, older.id) < (t.created_at, t.id);
+
+      CREATE UNIQUE INDEX IF NOT EXISTS "webhook_trigger_tenant_definition_name_unique"
+        ON "webhook_triggers"."webhook_trigger" ("tenant_id", "workflow_definition_id", "name");
+    `,
+  },
+  {
+    // CL-7242: the paired fix to 0003, in our own schema rather than
+    // Interchange's. `startReviewingRepos` acquires a short-lived
+    // lease on `(tenant_id, repo)` before its check-then-act body
+    // (hasRepoGrant/mintRepoGrant, hasWebhookTrigger/createWebhookTrigger)
+    // runs, so two concurrent calls for the same repo can never both
+    // enter that body -- only one can hold the lease at a time. The
+    // unique index must exist before any `ON CONFLICT (tenant_id, repo)`
+    // is issued against this table at runtime (Postgres requires a
+    // matching unique constraint/index for that clause, or the insert
+    // errors), so both land in this one migration, index second.
+    name: "0004_repo_review_lease",
+    sql: `
+      CREATE TABLE IF NOT EXISTS "webhook_triggers"."repo_review_lease" (
+        "id" text PRIMARY KEY,
+        "tenant_id" text NOT NULL,
+        "repo" text NOT NULL,
+        "leased_at" timestamptz NOT NULL DEFAULT now()
+      );
+
+      CREATE UNIQUE INDEX IF NOT EXISTS "repo_review_lease_tenant_repo_unique"
+        ON "webhook_triggers"."repo_review_lease" ("tenant_id", "repo");
+    `,
+  },
 ];
 
 // Named distinctly from the platform's setup ledger and from any

@@ -0,0 +1,104 @@
+// Closes a check-then-act race in the GitHub connect card's
+// start-reviewing step (CL-7242): `startReviewingRepos`
+// (`@corbits/workflow-catalog`) loops over selected repos doing
+// hasRepoGrant/mintRepoGrant then hasWebhookTrigger/createWebhookTrigger
+// per repo, each a plain read followed by a conditional write with no
+// atomic backstop. Two concurrent calls for the same repo (a
+// double-click, or a client retrying an in-flight request) can both
+// read "not set up yet" before either write lands, so both mint a
+// grant and both create a trigger.
+//
+// A lease acquired before that per-repo body runs is the actual
+// backstop: only the caller that wins the lease proceeds into
+// hasRepoGrant/mintRepoGrant/hasWebhookTrigger/createWebhookTrigger,
+// which stay exactly as CL-7134 left them (a fast path for a
+// *sequential* retry after a mid-loop failure, safe now that they can
+// never run concurrently for the same repo). The lease is released as
+// soon as that body finishes (success or failure) so a legitimate
+// retry is never blocked by its own prior attempt; a 2-minute
+// staleness window is a crash-only backstop, in case a process dies
+// before its `finally` can run.
+//
+// This table carries no schema-level relationship to Interchange's
+// own `grant` table (or any other platform table): the workaround
+// this closes lives entirely in workbench-owned state, and the actual
+// source of truth for whether a repo's grant/trigger exist stays
+// `hasRepoGrant`/`hasWebhookTrigger` against the real platform data,
+// completely unchanged. The lease only ever asserts "someone claimed
+// responsibility for this repo's setup as of `leasedAt`" — never
+// "the grant/trigger exist" — so it can never assert something untrue
+// the way a stale "done" marker could (CL-7213's own precedent).
+import { and, eq, lt } from "drizzle-orm";
+import type { PostgresJsDatabase } from "drizzle-orm/postgres-js";
+
+import { repoReviewLease } from "./schema";
+
+export type RepoReviewLeaseDb<
+  TSchema extends Record<string, unknown> = Record<string, never>,
+> = PostgresJsDatabase<TSchema>;
+
+/** Comfortably longer than a single repo's synchronous mint-and-create
+ * work should ever take, so a live lease is never mistaken for stale;
+ * short enough that a crashed holder self-heals well within a person
+ * re-clicking "Start reviewing" a few times. */
+const LEASE_STALE_AFTER_MS = 2 * 60 * 1000;
+
+export interface RepoReviewLeaseStore {
+  /**
+   * True if this call now holds the lease on `(tenantId, repo)` —
+   * either no lease existed, or the existing one is older than the
+   * staleness window and was stolen. False means another call
+   * currently holds (or very recently held) it; the caller must skip
+   * this repo rather than proceed.
+   */
+  acquire(tenantId: string, repo: string): Promise<boolean>;
+  /** Releases a held lease so an immediate legitimate retry (e.g. the
+   * next repo in a fresh `startReviewingRepos` call) never waits out
+   * the staleness window. Safe to call even if this caller never held
+   * the lease (e.g. `acquire` returned false) — a no-op in that case. */
+  release(tenantId: string, repo: string): Promise<void>;
+}
+
+export function createDrizzleRepoReviewLeaseStore<
+  TSchema extends Record<string, unknown>,
+>(db: RepoReviewLeaseDb<TSchema>): RepoReviewLeaseStore {
+  return {
+    async acquire(tenantId, repo) {
+      const now = new Date();
+      const staleBefore = new Date(now.getTime() - LEASE_STALE_AFTER_MS);
+      // Insert-first with a conditional steal, not select-then-insert:
+      // the unique index on (tenant_id, repo) makes this one atomic
+      // compare-and-swap on the DB side. The insert succeeds when no
+      // row exists yet; the `DO UPDATE ... WHERE` steals an existing
+      // row only when it's stale, and otherwise leaves it untouched
+      // and returns nothing — Postgres, not app-level timing, decides
+      // who wins.
+      const rows = await db
+        .insert(repoReviewLease)
+        .values({
+          id: `lease_${crypto.randomUUID()}`,
+          tenantId,
+          repo,
+          leasedAt: now,
+        })
+        .onConflictDoUpdate({
+          target: [repoReviewLease.tenantId, repoReviewLease.repo],
+          set: { leasedAt: now },
+          where: lt(repoReviewLease.leasedAt, staleBefore),
+        })
+        .returning({ id: repoReviewLease.id });
+      return rows.length > 0;
+    },
+
+    async release(tenantId, repo) {
+      await db
+        .delete(repoReviewLease)
+        .where(
+          and(
+            eq(repoReviewLease.tenantId, tenantId),
+            eq(repoReviewLease.repo, repo),
+          ),
+        );
+    },
+  };
+}