From 2f811a0aa84bc37bb0c6b456d08ce9403a44a9ae Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 26 Jun 2026 03:21:19 +0300 Subject: [PATCH] fix(git-sync): don't run a Docmost cycle on receive-pack info/refs (fixes deterministic push 503) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A git push is a two-request exchange: GET info/refs?service=git-receive-pack (ref advertisement) then POST git-receive-pack (the pack). The git-HTTP host classified BOTH as serviceKind 'write' and routed both through ingestExternalPush, which takes the per-space lock and runs a FULL Docmost reconcile cycle. So the read-only info/refs advertisement held the lock while a cycle ran, and the client's immediately-following POST git-receive-pack collided with that still-running cycle and got 503 — deterministically, every push (and Obsidian Git's "scan" failed for the same reason, since it probes push capability via the same receive-pack info/refs). Fix: only the actual pack-receiving write (POST git-receive-pack) runs under the lock + cycle. Everything else streams the http-backend directly with no lock and no cycle — a fetch/clone (read) AND the write-AUTHORIZED but read-only info/refs?service=git-receive-pack advertisement. Authz is unchanged (the gate still requires write permission for receive-pack refs); only the side effect of running a cycle on a read-only request is removed. Verified end-to-end on a live stand: clone, then `git push` of a new file lands the page in Docmost (was 503 on every push before). Regression test added. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../git-sync/http/git-http.service.spec.ts | 26 +++++++++++++++++++ .../git-sync/http/git-http.service.ts | 13 ++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/apps/server/src/integrations/git-sync/http/git-http.service.spec.ts b/apps/server/src/integrations/git-sync/http/git-http.service.spec.ts index 538fd040..791cc829 100644 --- a/apps/server/src/integrations/git-sync/http/git-http.service.spec.ts +++ b/apps/server/src/integrations/git-sync/http/git-http.service.spec.ts @@ -354,6 +354,32 @@ describe('GitHttpService.handle', () => { expect(workspaceId).toBe('ws-1'); }); + it('GET info/refs?service=git-receive-pack streams the backend WITHOUT a cycle/lock (so the follow-up POST never 503-collides)', async () => { + // A push is a TWO-request exchange: GET info/refs?service=git-receive-pack + // (ref advertisement) then POST git-receive-pack (the pack). The info/refs + // request is write-AUTHORIZED (push perms needed to see those refs) but is + // READ-ONLY — it must NOT run ingestExternalPush (a Docmost cycle under the + // per-space lock), or the immediately-following POST collides with the still- + // running cycle and deterministically 503s. It must just stream the backend. + const built = build({ abilityCan: true }); + const { reply } = fakeReply(); + const req = fakeRequest({ + url: '/git/space-1.git/info/refs?service=git-receive-pack', + method: 'GET', + authorization: basic('dev@example.com', 'pw'), + }); + + await built.service.handle(req, reply); + + // Authorized as a write (Manage), but executed as a plain stream. + expect(built.abilityCan).toHaveBeenCalledWith( + SpaceCaslAction.Manage, + SpaceCaslSubject.Page, + ); + expect(built.orchestrator.ingestExternalPush).not.toHaveBeenCalled(); + expect(built.backend.run).toHaveBeenCalledTimes(1); + }); + it('a push that loses the lock -> 503 with Retry-After and a busy body (headers not written twice)', async () => { const built = build({ abilityCan: true }); // The lock could not be acquired: the receive-pack closure never ran, so the diff --git a/apps/server/src/integrations/git-sync/http/git-http.service.ts b/apps/server/src/integrations/git-sync/http/git-http.service.ts index 7d58b20d..51971b24 100644 --- a/apps/server/src/integrations/git-sync/http/git-http.service.ts +++ b/apps/server/src/integrations/git-sync/http/git-http.service.ts @@ -251,8 +251,17 @@ export class GitHttpService { // response directly to the socket (mirrors the MCP transport pattern). reply.hijack(); - if (serviceKind === 'read') { - // Fetch/clone: stream http-backend directly, no lock (read-only). + // Only the ACTUAL pack-receiving write (POST git-receive-pack) runs under the + // space lock + a Docmost cycle. Everything else streams the http-backend + // directly with NO lock and NO cycle: a fetch/clone (read), AND the + // write-AUTHORIZED but READ-ONLY ref advertisement + // (GET info/refs?service=git-receive-pack). Running a cycle on info/refs is + // both wasteful and HARMFUL — it holds the per-space lock, so the push's + // immediately-following POST git-receive-pack collides with it and 503s + // (a deterministic push failure). Authz already happened above via the gate. + const isReceivePack = + req.method === 'POST' && parsedPath.subpath === 'git-receive-pack'; + if (serviceKind === 'read' || !isReceivePack) { await this.backend.run(backendRequest, rawReq, rawRes); return; }