Skip to content

Commit 71de4f6

Browse files
Merge pull request #251 from CodeForPhilly/chore/gitsheets-sync
refactor(api): reconcile through gitsheets repo.sync()
2 parents 76f4efa + 26bf22c commit 71de4f6

7 files changed

Lines changed: 349 additions & 485 deletions

File tree

‎apps/api/src/plugins/reconcile.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@
2121
* re-clone if needed.
2222
*
2323
* Decorates Fastify with `reconcileDataRepo({ onMoved })`: fetch outside the
24-
* lock, then reconcile under the repository's write lock — the same mutex
24+
* lock, then reconcile (gitsheets `repo.sync()`, see store/reconcile.ts)
25+
* under the repository's write lock — the same mutex
2526
* `transact` holds (gitsheets `repo.withLock`), so a reconcile can never race
2627
* a write. When the reconcile moves the branch, the public store is
2728
* refreshed and `onMoved` runs inside the same lock, so the hot-reload
@@ -74,12 +75,17 @@ async function reconcilePlugin(fastify: FastifyInstance): Promise<void> {
7475
const logger = fastify.log;
7576

7677
// Network outside the lock: writes keep flowing during a slow fetch.
77-
// The ref moves below are CAS (`update-ref <ref> <new> <old>`).
78+
// gitsheets' sync moves refs compare-and-swap.
7879
if (!(await fetchRemoteBranch({ repoPath, branch, logger }))) {
7980
const head = (await fastify.publicRepo.resolveRef('HEAD')) ?? '';
8081
return { outcome: 'fetch-failed', oldCommit: head, newCommit: head };
8182
}
8283

84+
// One critical section of the serving repository's write lock covers
85+
// the ref move, the store refresh and `onMoved` (the hot-reload
86+
// rebuild), so no transact can run between them. reconcileDataRepo
87+
// syncs through a side Repository handle because `repo.sync()` takes
88+
// its own handle's lock and refuses to nest inside `withLock`.
8389
return fastify.publicRepo.withLock(async () => {
8490
const result = await reconcileDataRepo({
8591
repoPath,

‎apps/api/src/routes/internal.ts‎

Lines changed: 23 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,14 @@
2727
* deployment surface stable across environments that haven't been
2828
* configured for hot reloads yet.
2929
*/
30-
import { execFile } from 'node:child_process';
3130
import { timingSafeEqual } from 'node:crypto';
32-
import { promisify } from 'node:util';
3331

3432
import type { FastifyInstance } from 'fastify';
3533

3634
import { errorResponse, ok } from '../lib/response.js';
3735
import { reloadInMemoryStateAndFts, type ReloadMode } from '../store/memory/reload.js';
3836
import type { ReconcileOutcome, ReconcileResult } from '../store/reconcile.js';
3937

40-
const exec = promisify(execFile);
41-
4238
/** Bearer-token regex — case-insensitive, single whitespace separator. */
4339
const BEARER_RE = /^Bearer\s+(\S+)$/i;
4440

@@ -146,44 +142,35 @@ export async function internalRoutes(fastify: FastifyInstance): Promise<void> {
146142
}
147143

148144
const startedAt = Date.now();
149-
const repoPath = fastify.config.CFP_DATA_REPO_PATH;
150145
const commitHash = body.commitHash;
151146

152147
// ---- Cheap pre-check: is `commitHash` already in local HEAD? ----
153-
// No lock acquired here — `merge-base --is-ancestor` only reads
154-
// git's object store, which is safe alongside an in-flight
155-
// gitsheets transact. Worst case (a transact lands between this
156-
// check and the answer being read) we accept a stale "no" and
157-
// proceed to the full reconcile.
148+
// No lock acquired here — `repo.isAncestor` only reads git's object
149+
// store, which is safe alongside an in-flight gitsheets transact.
150+
// Worst case (a transact lands between resolving HEAD and the answer
151+
// being read) we accept a stale "no" and proceed to the full
152+
// reconcile.
158153
if (commitHash) {
159154
try {
160-
const head = (
161-
await exec('git', ['rev-parse', 'HEAD'], { cwd: repoPath })
162-
).stdout.trim();
163-
await exec(
164-
'git',
165-
['merge-base', '--is-ancestor', commitHash, head],
166-
{ cwd: repoPath },
167-
);
168-
// `git merge-base --is-ancestor` exits 0 = is-ancestor, 1 = not.
169-
// Reaching here means exit 0; short-circuit.
170-
fastify.log.info(
171-
{ branch, commitHash, head },
172-
'hot-reload short-circuit: commit already in local HEAD',
173-
);
174-
return reply.send(
175-
ok({
176-
noChanges: true,
177-
outcome: 'in-sync' as ReconcileOutcome,
178-
head,
179-
durationMs: Date.now() - startedAt,
180-
}),
181-
);
155+
const head = await fastify.publicRepo.resolveRef('HEAD');
156+
if (head && (await fastify.publicRepo.isAncestor(commitHash, head))) {
157+
fastify.log.info(
158+
{ branch, commitHash, head },
159+
'hot-reload short-circuit: commit already in local HEAD',
160+
);
161+
return reply.send(
162+
ok({
163+
noChanges: true,
164+
outcome: 'in-sync' as ReconcileOutcome,
165+
head,
166+
durationMs: Date.now() - startedAt,
167+
}),
168+
);
169+
}
182170
} catch (err) {
183-
// exec throws with `code: 1` when not-ancestor (continue to
184-
// reconcile) and with other codes when the commit is unknown
185-
// or git itself fails. We treat all non-zero as "fall through
186-
// to reconcile" — the reconcile will fetch and try again.
171+
// RefError when the commit isn't in the local object store yet
172+
// (the usual case for an upstream push). Fall through to the
173+
// reconcile, which fetches and tries again.
187174
fastify.log.debug(
188175
{
189176
err: err instanceof Error ? err.message : String(err),

0 commit comments

Comments
 (0)