perf(plugin-security): batch the identity boot seeds' existence read and skip no-op writes

.changeset/batch-identity-boot-seed-round-trips.md

@@ -0,0 +1,55 @@
+---
+"@objectstack/plugin-security": minor
+---
+
+Batch the identity boot seeds' existence read and stop re-writing rows that
+already match the declaration (#10946).
+
+Every permission set and every position an environment declared cost **exactly
+4 sequential database round trips on every kernel boot** — measured on a real
+per-environment kernel build with every `@libsql/client` call counted: slope
+4.0000, R² = 1.000000 on both axes, with a per-statement histogram naming the
+four legs (2 × existence `SELECT`, 1 × `UPDATE`, 1 × `SELECT`). Two of the four
+were an `UPDATE` that fired even when nothing had changed. On a local file
+database the loop is invisible; on a remote libsql/Turso database — every hosted
+environment — each leg is its own sequential HTTP request. Schema sync had
+already been batched (`TursoDriver.supports.batchSchemaSync`), which is why
+objects, views and artifact seeds add 0.00 round trips each on the same rig;
+identity content was the one content axis still paying per item.
+
+Both loops now hoist **one** `{ name: { $in: [...] } }` existence read out of the
+loop — the declaration is known in full before the loop starts — and write only
+when the stored row actually differs from what would be written. A steady-state
+rebuild of both loops is now O(1) round trips: measured in-repo against a
+call-counting ObjectQL double, a rebuild of 1, 5, 20 and 40 declared items costs
+1 round trip in every case, for permission sets and positions alike.
+
+Three things the change is careful **not** to become:
+
+- **Drift still reconciles.** The skip is on equality, never on "we have seen
+  this name": a row whose stored value differs — a package version bump, a
+  hand-edit, a partially applied write — still gets its `UPDATE`. An
+  implementation that skipped all writes would show the same round-trip curve
+  and silently stop reconciling, so the round-trip pins are paired one-for-one
+  with drift pins over the same fixtures.
+- **A read that could not answer is not the answer "none exist."** A batched
+  read fails for the whole set at once, so swallowing its failure into `[]`
+  would make every boot conclude nothing is seeded and re-create everything. The
+  seam is judged on whether the driver returned a result set, never on whether
+  the array came back empty; a failed batched read degrades to the per-item read
+  (loudly warned), and a name whose record cannot be read at all is declined
+  rather than inserted. That last step is deliberately stricter than the code it
+  replaces, which turned a failed read into an insert attempt and leaned on the
+  `name` unique index to refuse it.
+- **A converged publish is still a successful publish.** `PermissionSeedOutcome`
+  gains `unchanged` (rows that already matched) and `unreadable` (names declined
+  because their record could not be read). The ADR-0086 P2 publish materializer
+  asks "did the record end up matching the published body", which was
+  accidentally identical to "was a write issued" only because the seeder always
+  wrote; it now reads `seeded + updated + unchanged`, so every case that reported
+  a materialization before still reports one. A re-publish of a byte-identical
+  body reports `inserted: 0, updated: 0` instead of `updated: 1` — the one
+  reporting difference, and the truthful reading.
+
+`bootstrapDeclaredPositions` likewise returns `unchanged` and `unreadable`
+alongside `seeded`/`updated`.

packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts

@@ -15,7 +15,19 @@ function makeQl(declared: any[] = []) {
     async find(object: string, q: any) {
       if (object !== 'sys_permission_set') return [];
       const where = q?.where ?? {};
-      return rows.filter((r) => Object.entries(where).every(([k, v]) => { if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); return r[k] === v; }));
+      // Membership is modelled because the real engine supports it and the
+      // #10946 boot seeders now hoist ONE `$in` existence read out of their
+      // loop. A double that silently answered `[]` to `$in` would report
+      // "nothing is seeded" and make every re-seed look like a first boot.
+      return rows.filter((r) => Object.entries(where).every(([k, v]) => {
+        if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`);
+        if (v && typeof v === 'object' && !Array.isArray(v)) {
+          const inList = (v as any).$in;
+          if (Array.isArray(inList)) return inList.includes(r[k]);
+          throw new Error(`fake driver: unsupported operator ${Object.keys(v).join(',')}`);
+        }
+        return r[k] === v;
+      }));
     },
     async insert(object: string, data: any) {
       if (object !== 'sys_permission_set') return null;

packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts

@@ -36,17 +36,41 @@
  */
 
 import {
+  SYSTEM_CTX,
   genId,
   permissionSetRowFields,
-  tryFind,
+  recordDiffersFromBody,
   tryInsert,
   tryUpdate,
   type PermissionSeedOutcome,
   type ProjectionLogger,
 } from './permission-set-projection.js';
+import {
+  buildExistingByName,
+  type ExistingByNameIndex,
+  type ExistingLookupResult,
+} from './seed-name-lookup.js';
 
 export type { PermissionSeedOutcome } from './permission-set-projection.js';
 
+/**
+ * The per-name existence read used when no batched oracle was supplied (the
+ * ADR-0086 P2 publish materializer, which upserts exactly one set). Reports the
+ * same three outcomes the batched oracle does — a failed read must not read as
+ * "absent" on this path either.
+ */
+async function defaultLookup(ql: any, name: string): Promise<ExistingLookupResult> {
+  let rows: any;
+  try {
+    rows = await ql.find('sys_permission_set', { where: { name }, limit: 1 }, { context: SYSTEM_CTX });
+  } catch {
+    return { status: 'unknown' };
+  }
+  const list = Array.isArray(rows) ? rows : Array.isArray(rows?.records) ? rows.records : null;
+  if (list === null) return { status: 'unknown' };
+  return list[0] ? { status: 'present', row: list[0] } : { status: 'absent' };
+}
+
 interface SeedOptions {
   logger?: ProjectionLogger;
 }
@@ -96,8 +120,17 @@ export async function upsertPackagePermissionSet(
   ps: any,
   packageId: string | null | undefined,
   logger?: SeedOptions['logger'],
+  opts?: {
+    /**
+     * Existence oracle to consult instead of this function's own per-name
+     * `SELECT` (#10946). The boot loop passes ONE batched read covering every
+     * declared name; the publish materializer, which upserts a single set,
+     * passes nothing and keeps the per-name read.
+     */
+    existingByName?: ExistingByNameIndex;
+  },
 ): Promise<PermissionSeedOutcome> {
-  const out: PermissionSeedOutcome = { seeded: 0, updated: 0, skippedEnvAuthored: 0, skippedForeign: 0 };
+  const out: PermissionSeedOutcome = { seeded: 0, updated: 0, unchanged: 0, unreadable: 0, skippedEnvAuthored: 0, skippedForeign: 0 };
   if (!ps?.name) return out;
   // A `managed_by:'package'` row without a `package_id` would make uninstall
   // undefined again — the exact ambiguity ADR-0086 D3 exists to remove — so a
@@ -107,25 +140,59 @@ export async function upsertPackagePermissionSet(
     return out;
   }
 
-  const existing = (await tryFind(ql, 'sys_permission_set', { name: ps.name }, 1))[0];
+  // ⛔ Three outcomes, not two (#10946 / #3807). `unknown` — the read FAILED —
+  // is not "no such row": inserting on it would re-create a set that already
+  // exists, and on a batched read one failure speaks for every declared name at
+  // once. Declining is the answer; the caller's warn reports it.
+  const lookup = opts?.existingByName
+    ? await opts.existingByName.get(String(ps.name))
+    : await defaultLookup(ql, String(ps.name));
+  if (lookup.status === 'unknown') {
+    out.unreadable += 1;
+    return out;
+  }
+  const existing = lookup.status === 'present' ? lookup.row : undefined;
   if (!existing?.id) {
-    const created = await tryInsert(ql, 'sys_permission_set', {
+    const row = {
       id: genId('ps'),
       name: ps.name,
       ...permissionSetRowFields(ps),
       active: true,
       package_id: packageId,
       managed_by: 'package',
-    });
-    if (created) out.seeded += 1;
+    };
+    const created = await tryInsert(ql, 'sys_permission_set', row);
+    if (created) {
+      out.seeded += 1;
+      // A batched oracle is a snapshot taken before the loop — tell it about
+      // the row we just made, so a name declared twice in one batch still
+      // reaches the collision branch below instead of a second insert.
+      opts?.existingByName?.remember(String(ps.name), row);
+    }
     return out;
   }
 
   if (existing.managed_by === 'package') {
     if (existing.package_id === packageId) {
       // Our own row — re-seed so the record always reflects the shipped/published
       // declaration (idempotent; covers version bumps without bookkeeping).
-      if (await tryUpdate(ql, 'sys_permission_set', { id: existing.id, ...permissionSetRowFields(ps) })) {
+      //
+      // [#10946] "Idempotent" was implemented as "write the same columns every
+      // time", which on a remote libsql/Turso database is two HTTP round trips
+      // per set on every boot to change nothing. `recordDiffersFromBody` is the
+      // SAME comparison the ADR-0094 boot reconciler already trusts to decide
+      // whether a record drifted, over exactly the columns
+      // `permissionSetRowFields` writes — so a row it calls equal is a row this
+      // UPDATE could not have changed.
+      //
+      // ⚠️ The skip is on EQUALITY, never on "we have seen this name". A row
+      // whose stored value differs — a version bump, a hand-edit, a partially
+      // applied write — still gets its UPDATE, because dropping that leg would
+      // turn the loop into a no-op that reconciles nothing while showing a
+      // beautiful round-trip curve.
+      if (!recordDiffersFromBody(existing, ps)) {
+        out.unchanged += 1;
+      } else if (await tryUpdate(ql, 'sys_permission_set', { id: existing.id, ...permissionSetRowFields(ps) })) {
         out.updated += 1;
       }
     } else {
@@ -151,7 +218,7 @@ export async function bootstrapDeclaredPermissions(
   metadataService: any,
   options: SeedOptions = {},
 ): Promise<PermissionSeedOutcome> {
-  const out: PermissionSeedOutcome = { seeded: 0, updated: 0, skippedEnvAuthored: 0, skippedForeign: 0 };
+  const out: PermissionSeedOutcome = { seeded: 0, updated: 0, unchanged: 0, unreadable: 0, skippedEnvAuthored: 0, skippedForeign: 0 };
   if (!ql || typeof ql.find !== 'function' || typeof ql.insert !== 'function') return out;
 
   let sets: any[] = readDeclared(ql, 'permission');
@@ -163,18 +230,40 @@ export async function bootstrapDeclaredPermissions(
   }
   if (!Array.isArray(sets) || sets.length === 0) return out;
 
+  // [#10946] ONE existence read for the whole declaration, before the loop —
+  // the set of names is known in full here. See `seed-name-lookup.ts` for why
+  // a read that cannot ANSWER must not be read as "none of them exist".
+  const existingByName = await buildExistingByName(
+    ql,
+    'sys_permission_set',
+    sets.map((ps) => ps?.name),
+    options.logger,
+  );
+
   for (const ps of sets) {
     if (!ps?.name) continue;
     // Registry provenance first (ADR-0010 `_packageId`), author-declared
     // spec `packageId` (ADR-0086 D3) as fallback.
     const packageId: string | undefined = ps._packageId ?? ps.packageId ?? undefined;
-    const r = await upsertPackagePermissionSet(ql, ps, packageId, options.logger);
+    const r = await upsertPackagePermissionSet(ql, ps, packageId, options.logger, { existingByName });
     out.seeded += r.seeded;
     out.updated += r.updated;
+    out.unchanged += r.unchanged;
+    out.unreadable += r.unreadable;
     out.skippedEnvAuthored += r.skippedEnvAuthored;
     out.skippedForeign += r.skippedForeign;
   }
 
+  if (out.unreadable > 0) {
+    // Said once, with the count: these sets were neither seeded nor reconciled
+    // because the record could not be READ. Silence here would read exactly
+    // like "everything was already in order".
+    options.logger?.warn?.(
+      '[security] declared permission sets left untouched — their records could not be read',
+      { unreadable: out.unreadable, total: sets.length },
+    );
+  }
+
   options.logger?.info?.('[security] declared permission sets seeded into sys_permission_set (ADR-0086 D5)', {
     ...out, total: sets.length,
   });

packages/plugins/plugin-security/src/bootstrap-declared-positions.test.ts

@@ -26,7 +26,19 @@ function makeQl(declared: any[] = []) {
     async find(object: string, q: any) {
       if (object !== 'sys_position') return [];
       const where = q?.where ?? {};
-      return rows.filter((r) => Object.entries(where).every(([k, v]) => { if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); return r[k] === v; }));
+      // Membership is modelled because the real engine supports it and the
+      // #10946 boot seeders now hoist ONE `$in` existence read out of their
+      // loop. A double that silently answered `[]` to `$in` would report
+      // "nothing is seeded" and make every re-seed look like a first boot.
+      return rows.filter((r) => Object.entries(where).every(([k, v]) => {
+        if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`);
+        if (v && typeof v === 'object' && !Array.isArray(v)) {
+          const inList = (v as any).$in;
+          if (Array.isArray(inList)) return inList.includes(r[k]);
+          throw new Error(`fake driver: unsupported operator ${Object.keys(v).join(',')}`);
+        }
+        return r[k] === v;
+      }));
     },
     async insert(object: string, data: any) {
       if (object !== 'sys_position') return null;