fix(git-sync): don't run a Docmost cycle on receive-pack info/refs (fixes deterministic push 503)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user