fix(git-sync): git-http stream error handlers + close test gaps (#119 review)

Addresses the stability + test-coverage warnings from the #119 review:

- git-http-backend.service.ts: add `'error'` handlers to child.stdout/stderr. An
  EventEmitter 'error' with no listener (e.g. EPIPE when the client aborts
  mid-response) is rethrown by Node as an uncaught exception and crashes the
  process; now swallowed + logged (never echoed to the client).
- TEST INFRA: a jest setupFile shims `navigator`/`MessageChannel` for the `node`
  testEnvironment. react-dom@18 reads `navigator` at module-init (pulled in via
  @docmost/editor-ext -> @tiptap/react), so every spec transitively importing the
  conversion engine — including git-http.service.spec.ts — previously FAILED TO
  LOAD ("navigator is not defined") and ran ZERO tests. With the shim those specs
  now run (git-sync integration: 11 suites / 133 tests green).
- git-http.service.spec.ts: cover the 503 lock-held push path — `ingestExternalPush`
  rejecting `GitSyncLockHeldError` -> 503 + Retry-After + "git-sync busy, retry",
  no double header write (+ the already-headers-sent no-rewrite path).
- git-http-backend.service.spec.ts: unit-test run() — child 'error'/'close' before
  headers -> 500; normal CGI parse+stream; stdout/stderr 'error' (EPIPE) swallowed;
  synchronous spawn throw -> 500.
- page-change.listener.ts: implement OnModuleDestroy to clearTimeout all pending
  debounce timers on shutdown (+ test).
- .env.example: vaults are non-bare working repos, not "bare repos".

(Docs deleted by the stray commit were restored in 9cdbce54.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-24 13:25:04 +03:00
parent 4024070589
commit 2b91e31f0c
8 changed files with 327 additions and 3 deletions

View File

@@ -14,6 +14,7 @@ import {
SpaceCaslSubject,
} from '../../../core/casl/interfaces/space-ability.type';
import { GitHttpService } from './git-http.service';
import { GitSyncLockHeldError } from '../services/git-sync.orchestrator';
type AnyMock = jest.Mock;
@@ -353,6 +354,66 @@ describe('GitHttpService.handle', () => {
expect(workspaceId).toBe('ws-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
// response is still unwritten and the handler must answer 503 itself.
built.orchestrator.ingestExternalPush.mockRejectedValue(
new GitSyncLockHeldError('space-1'),
);
const { reply, state } = fakeReply();
const req = fakeRequest({
url: '/git/space-1.git/git-receive-pack',
method: 'POST',
authorization: basic('dev@example.com', 'pw'),
});
await built.service.handle(req, reply);
// It hijacked and went through the orchestrator (write path), but the lock
// was held so the backend never ran.
expect(state.hijacked).toBe(true);
expect(built.orchestrator.ingestExternalPush).toHaveBeenCalledTimes(1);
expect(built.backend.run).not.toHaveBeenCalled();
// 503 + Retry-After were written on the raw response (headersSent was false).
const raw = reply.raw as any;
expect(raw.statusCode).toBe(503);
expect(raw.setHeader).toHaveBeenCalledWith('Content-Type', 'text/plain');
expect(raw.setHeader).toHaveBeenCalledWith('Retry-After', '1');
// The body carries the busy/retry message and the response was ended once.
expect(raw.end).toHaveBeenCalledTimes(1);
expect(raw.end).toHaveBeenCalledWith('git-sync busy, retry');
// Exactly the two headers above were set — no double write of headers.
expect(raw.setHeader).toHaveBeenCalledTimes(2);
});
it('does NOT rewrite the 503 status/headers when the response is already sent', async () => {
const built = build({ abilityCan: true });
built.orchestrator.ingestExternalPush.mockRejectedValue(
new GitSyncLockHeldError('space-1'),
);
const { reply } = fakeReply();
// Simulate the (defensive) case where headers were already flushed: the
// handler must skip statusCode/setHeader and only end() the socket.
const raw = reply.raw as any;
raw.headersSent = true;
const req = fakeRequest({
url: '/git/space-1.git/git-receive-pack',
method: 'POST',
authorization: basic('dev@example.com', 'pw'),
});
await built.service.handle(req, reply);
// No header writes when headersSent is already true (no "headers already
// sent" double-write path), but the body/end still runs.
expect(raw.setHeader).not.toHaveBeenCalled();
expect(raw.statusCode).toBe(200); // untouched default from the fake
expect(raw.end).toHaveBeenCalledTimes(1);
expect(raw.end).toHaveBeenCalledWith('git-sync busy, retry');
});
it('an unresolvable workspace -> 401 (credentials cannot be validated without one)', async () => {
const built = build({ workspace: null });
const { reply, state } = fakeReply();