test(git-sync): add reviewer-requested coverage across engine, server, client

Implements the test cases called out in the PR #119 review threads (code-review, test-strategy report, red-team) — TESTS ONLY, no production code changes. packages/git-sync (vitest): - lib converter/markdown gaps: pageBreak data-loss (it.fails repro), subpages lossy round-trip, nested/fenced callouts, ol->taskList bridge, column.width number<->string drift, empty details. - engine units: parentFolderFile, planReconciliation swap/chained move, buildVaultLayout last-resort-by-id, firstDivergence, applyPushActions / applyPullActions failure isolation. - real temp-git integration: diffNameStatus -z rename+add/modify alignment, copy-line behavior, per-invocation committer identity (no leak into repo/global config). - ENFORCED type-level GitSyncClient contract via vitest typecheck over a *.test-d.ts file (tsconfig.vitest.json; build tsconfig untouched). apps/server (jest): - orchestrator: delete-cap neutralization + fail-safe, Redis lock / mutex skip ladder + release-on-throw, merge guard, pull/push order, remote template substitution, poll lifecycle. - page-change listener: loop-guard, debounce coalescing, id resolution, error swallowing. - vault registry, controller authz (trigger + status), env validation/getters, page.service git-sync provenance stamping, persistence precedence (agent > git-sync > user) + no boundary snapshot, space.service audit-delta, space.repo jsonb-merge, converter-gate corpus extension (mention/math/details/marks). apps/client (vitest + testing-library): - history-item git-sync badge: render gating + non-clickable. - edit-space-form toggle: initial state, optimistic payload, rollback on error, disabled states. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-21 17:51:35 +03:00
parent 23c999e932
commit 2ae6f8841f
20 changed files with 3331 additions and 0 deletions
@@ -0,0 +1,141 @@
+import {
+  Kysely,
+  DummyDriver,
+  PostgresAdapter,
+  PostgresIntrospector,
+  PostgresQueryCompiler,
+  CompiledQuery,
+} from 'kysely';
+import { SpaceRepo } from './space.repo';
+import type { KyselyDB } from '../../types/kysely.types';
+
+/**
+ * SQL-builder unit test for the jsonb-merge invariant of
+ * SpaceRepo.updateGitSyncSettings (review comment #694 / test-strategy item #6).
+ *
+ * The merge is RAW SQL, so a behavioural test would need a live Postgres — which
+ * is intentionally out of scope here (the reviewer's own §13.3 was deferred for
+ * the same reason). Instead we follow the existing repo-spec convention
+ * (ai-agent-roles.repo.spec.ts) of NOT executing: we compile the query with a
+ * DummyDriver Postgres dialect and assert the generated SQL preserves sibling
+ * keys. The structural invariant the SQL must encode:
+ *
+ *   settings  := COALESCE(settings, '{}') || jsonb_build_object('gitSync', ...)
+ *   gitSync   := COALESCE(settings->'gitSync', '{}') || jsonb_build_object(key, value)
+ *
+ * The OUTER `||` merges into the existing top-level `settings`, so a sibling
+ * top-level key (e.g. `sharing`) is preserved. The INNER COALESCE merges into
+ * the existing `gitSync` object, so a sibling key inside gitSync (e.g. `other`)
+ * is preserved. A naive `set settings = jsonb_build_object('gitSync', ...)`
+ * would clobber both — this test guards exactly that regression.
+ */
+describe('SpaceRepo.updateGitSyncSettings — jsonb merge SQL', () => {
+  // A real Kysely on the Postgres dialect, but with a DummyDriver: it compiles
+  // queries to real Postgres SQL without ever opening a connection.
+  function makeCompileOnlyDb() {
+    return new Kysely<any>({
+      dialect: {
+        createAdapter: () => new PostgresAdapter(),
+        createDriver: () => new DummyDriver(),
+        createIntrospector: (db) => new PostgresIntrospector(db),
+        createQueryCompiler: () => new PostgresQueryCompiler(),
+      },
+    });
+  }
+
+  // Build the repo over the compile-only db. The repo terminates the query with
+  // `.executeTakeFirst()`, so we wrap every kysely builder in a Proxy: when the
+  // repo finally calls `executeTakeFirst`, we `.compile()` that same builder
+  // ourselves to capture the exact SQL it was about to run, then delegate.
+  function makeRepoCapturingSql() {
+    const db = makeCompileOnlyDb();
+    let captured: CompiledQuery | undefined;
+
+    // kysely builders are immutable — each .set()/.where()/.returningAll()
+    // returns a NEW builder — so re-wrap any chainable result.
+    const wrap = (b: any): any =>
+      new Proxy(b, {
+        get(target, prop, receiver) {
+          const value = Reflect.get(target, prop, receiver);
+          if (typeof value !== 'function') return value;
+          return (...callArgs: unknown[]) => {
+            // Capture the SQL at the terminal execute call.
+            if (
+              (prop === 'executeTakeFirst' || prop === 'execute') &&
+              typeof target.compile === 'function'
+            ) {
+              captured = target.compile();
+            }
+            const result = value.apply(target, callArgs);
+            if (
+              result &&
+              typeof result === 'object' &&
+              typeof (result as any).compile === 'function'
+            ) {
+              return wrap(result);
+            }
+            return result;
+          };
+        },
+      });
+
+    const originalUpdateTable = db.updateTable.bind(db);
+    jest
+      .spyOn(db, 'updateTable')
+      .mockImplementation((...args: Parameters<typeof originalUpdateTable>) =>
+        wrap(originalUpdateTable(...args)),
+      );
+
+    const repo = new SpaceRepo(db as unknown as KyselyDB, {} as any);
+    return { repo, getCaptured: () => captured };
+  }
+
+  it("compiles a jsonb merge that preserves sibling top-level and gitSync keys", async () => {
+    const { repo, getCaptured } = makeRepoCapturingSql();
+
+    // DummyDriver yields no rows; executeTakeFirst resolves to undefined. The
+    // SQL is fully compiled by then, which is all we assert.
+    await repo.updateGitSyncSettings('space-1', 'ws-1', 'enabled', true);
+
+    const compiled = getCaptured();
+    expect(compiled).toBeDefined();
+    // The raw SQL template carries newlines/indentation; collapse whitespace so
+    // the structural assertions are not coupled to source formatting.
+    const sql = compiled!.sql.replace(/\s+/g, ' ');
+
+    // OUTER merge into the existing settings object -> sibling top-level keys
+    // (e.g. `sharing`) survive (NOT a bare jsonb_build_object assignment).
+    expect(sql).toContain(`set "settings" = COALESCE(settings, '{}'::jsonb) ||`);
+    // INNER merge into the existing gitSync object -> sibling gitSync keys
+    // (e.g. `other`) survive.
+    expect(sql).toContain(
+      `jsonb_build_object('gitSync', COALESCE(settings->'gitSync', '{}'::jsonb) ||`,
+    );
+    // The pref key is set via jsonb_build_object on the inner object.
+    expect(sql).toContain(`jsonb_build_object('enabled',`);
+    // Scoped to the row + workspace.
+    expect(sql).toContain(`where "id" =`);
+    expect(sql).toContain(`and "workspaceId" =`);
+
+    // Sanity: this is NOT a clobbering assignment (no top-level
+    // `set "settings" = jsonb_build_object(` without the COALESCE/merge).
+    expect(sql).not.toContain(`set "settings" = jsonb_build_object(`);
+
+    // The pref VALUE is inlined via sql.lit (matches the repo's sql.lit usage);
+    // updatedAt + id + workspaceId are the only bound parameters (the jsonb
+    // merge text is all literal). updatedAt is a Date, so assert id/workspaceId.
+    expect(compiled!.parameters).toContain('space-1');
+    expect(compiled!.parameters).toContain('ws-1');
+  });
+
+  it('inlines the prefKey/prefValue literally (sql.raw key, sql.lit value)', async () => {
+    const { repo, getCaptured } = makeRepoCapturingSql();
+
+    await repo.updateGitSyncSettings('space-1', 'ws-1', 'enabled', false);
+
+    const sql = getCaptured()!.sql.replace(/\s+/g, ' ');
+    // key via sql.raw + value via sql.lit -> both appear literally in the
+    // inner build object (no bound parameter for either).
+    expect(sql).toContain(`jsonb_build_object('enabled', false)`);
+  });
+});