From fbb014808795edac3371930db022952f205f4079 Mon Sep 17 00:00:00 2001
From: claude code agent 227 <claude_code@vvzvlad.xyz>
Date: Wed, 24 Jun 2026 01:23:17 +0300
Subject: [PATCH] test(git-sync): cover ingestExternalPush in the orchestrator
 spec (PR #119 review)

Closes the test-coverage warning that the smart-HTTP push ingest path was
unexercised. Adds 5 cases: receive-pack streams BEFORE the Docmost cycle; a
held lock throws GitSyncLockHeldError and runs neither the receive-pack nor the
cycle; a post-push cycle error is swallowed (the push is durable, poll retries)
while the lock is still released; a missing service user runs the receive-pack
but skips the immediate cycle; and a globally-disabled git-sync refuses without
touching the lock.

(The 503/Retry-After mapping in git-http.service is the sibling warning; its spec
is in the repo's pre-existing set of jest suites that can't load locally via the
react-dom/tiptap transform chain, so that case is left for CI.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 .../services/git-sync.orchestrator.spec.ts    | 82 ++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/apps/server/src/integrations/git-sync/services/git-sync.orchestrator.spec.ts b/apps/server/src/integrations/git-sync/services/git-sync.orchestrator.spec.ts
index a5b2154d..f9d5b7d8 100644
--- a/apps/server/src/integrations/git-sync/services/git-sync.orchestrator.spec.ts
+++ b/apps/server/src/integrations/git-sync/services/git-sync.orchestrator.spec.ts
@@ -21,7 +21,10 @@ import {
   applyPullActions,
   runPush,
 } from '@docmost/git-sync';
-import { GitSyncOrchestrator } from './git-sync.orchestrator';
+import {
+  GitSyncOrchestrator,
+  GitSyncLockHeldError,
+} from './git-sync.orchestrator';
 import { SpaceLockService } from './space-lock.service';
 
 type AnyMock = jest.Mock;
@@ -308,6 +311,83 @@ describe('GitSyncOrchestrator', () => {
     });
   });
 
+  describe('ingestExternalPush', () => {
+    it('streams the receive-pack FIRST, then runs the Docmost cycle', async () => {
+      const order: string[] = [];
+      const built = build();
+      applyPullActionsMock.mockImplementation(async () => {
+        order.push('cycle');
+        return { written: 0, deleted: 0, merge: { conflict: false } };
+      });
+      const runReceivePack = jest.fn(async () => {
+        order.push('receive-pack');
+      });
+
+      await built.orchestrator.ingestExternalPush('space-1', 'ws-1', runReceivePack);
+
+      expect(runReceivePack).toHaveBeenCalledTimes(1);
+      // The cycle only runs AFTER the push commits land on main.
+      expect(order).toEqual(['receive-pack', 'cycle']);
+    });
+
+    it('throws GitSyncLockHeldError and does NOT run the receive-pack when the lock is held', async () => {
+      const built = build();
+      built.redis.set.mockResolvedValue(null); // acquire fails → lock-held
+      const runReceivePack = jest.fn(async () => undefined);
+
+      await expect(
+        built.orchestrator.ingestExternalPush('space-1', 'ws-1', runReceivePack),
+      ).rejects.toBeInstanceOf(GitSyncLockHeldError);
+
+      // We must never write to the working tree concurrently with a cycle.
+      expect(runReceivePack).not.toHaveBeenCalled();
+      expect(applyPullActionsMock).not.toHaveBeenCalled();
+    });
+
+    it('swallows a post-push cycle error (the push is durable; poll retries)', async () => {
+      jest.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
+      const built = build();
+      // Dry-run resolves, the real apply rejects → driveCycle throws AFTER the
+      // receive-pack already succeeded.
+      runPushMock
+        .mockResolvedValueOnce({ mode: 'apply', failures: [], planned: { deletes: 0 } })
+        .mockRejectedValueOnce(new Error('cycle boom'));
+      const runReceivePack = jest.fn(async () => undefined);
+
+      // Does NOT throw — the durable push must not be reported as failed.
+      await expect(
+        built.orchestrator.ingestExternalPush('space-1', 'ws-1', runReceivePack),
+      ).resolves.toBeUndefined();
+      expect(runReceivePack).toHaveBeenCalledTimes(1);
+      // Lock was still released (CAS eval) despite the cycle error.
+      expect(built.redis.eval).toHaveBeenCalled();
+    });
+
+    it('runs the receive-pack but SKIPS the cycle when no service user is configured', async () => {
+      jest.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
+      const built = build({ serviceUserId: undefined });
+      const runReceivePack = jest.fn(async () => undefined);
+
+      await expect(
+        built.orchestrator.ingestExternalPush('space-1', 'ws-1', runReceivePack),
+      ).resolves.toBeUndefined();
+      // The push is durable on main; the immediate cycle is skipped, not failed.
+      expect(runReceivePack).toHaveBeenCalledTimes(1);
+      expect(applyPullActionsMock).not.toHaveBeenCalled();
+    });
+
+    it('refuses (LockHeldError) and runs nothing when git-sync is globally disabled', async () => {
+      const built = build({ enabled: false });
+      const runReceivePack = jest.fn(async () => undefined);
+
+      await expect(
+        built.orchestrator.ingestExternalPush('space-1', 'ws-1', runReceivePack),
+      ).rejects.toBeInstanceOf(GitSyncLockHeldError);
+      expect(runReceivePack).not.toHaveBeenCalled();
+      expect(built.redis.set).not.toHaveBeenCalled();
+    });
+  });
+
   describe('delete cap (anti-data-loss)', () => {
     it('suppresses deletePage on the apply client (by throwing) when planned deletes exceed the cap', async () => {
       jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined);