diff --git a/apps/api/package.json b/apps/api/package.json index fb1408a..ba6f4d8 100644 --- a/apps/api/package.json +++ b/apps/api/package.json @@ -36,7 +36,7 @@ "bcryptjs": "^3.0.3", "better-sqlite3": "^12.10.0", "fastify": "^5.8.5", - "gitsheets": "^2.9.0", + "gitsheets": "^2.11.0", "jose": "^6.2.3", "postmark": "^5.1.0", "samlify": "^2.13.0", diff --git a/apps/api/scripts/check-sheet-configs.ts b/apps/api/scripts/check-sheet-configs.ts index d6f263a..f2e72b9 100644 --- a/apps/api/scripts/check-sheet-configs.ts +++ b/apps/api/scripts/check-sheet-configs.ts @@ -22,6 +22,8 @@ import { dirname, join, relative } from 'node:path'; import { fileURLToPath } from 'node:url'; import { promisify } from 'node:util'; +import { openRepo, type Repository } from 'gitsheets'; + const exec = promisify(execFile); // The data repo is private: SSH with a key that can read it (CI: a deploy key). @@ -40,6 +42,17 @@ async function readTomlTree(dir: string): Promise> { return out; } +/** Every file path under `dir` at `ref`, recursively. */ +async function listFilesUnder(repo: Repository, ref: string, dir: string): Promise { + const out: string[] = []; + for (const entry of (await repo.listTree(ref, dir)) ?? []) { + const path = `${dir}/${entry.name}`; + if (entry.type === 'tree') out.push(...(await listFilesUnder(repo, ref, path))); + else if (entry.type === 'blob') out.push(path); + } + return out; +} + function arg(name: string, fallback: string): string { const prefix = `--${name}=`; return process.argv.find((a) => a.startsWith(prefix))?.slice(prefix.length) ?? fallback; @@ -51,15 +64,18 @@ async function main(): Promise { const gitDir = await mkdtemp(join(tmpdir(), 'cfp-sheet-configs-')); try { - const git = (...args: string[]) => exec('git', ['--git-dir', gitDir, ...args]); + // Raw git: gitsheets has no init, and fetches only from a named remote. await exec('git', ['init', '--bare', '-q', gitDir]); - await git('fetch', '-q', '--depth=1', remote, ref); + await exec('git', ['--git-dir', gitDir, 'remote', 'add', 'origin', remote]); + const repo = await openRepo({ gitDir }); + const fetched = await repo.fetch({ remote: 'origin', branch: ref }); + if (!fetched.ok) throw new Error(`fetching ${remote}#${ref} failed: ${fetched.error}`); - const { stdout: listing } = await git('ls-tree', '-r', '--name-only', 'FETCH_HEAD', '.gitsheets/'); const upstream = new Map(); - for (const path of listing.split('\n').filter((p) => p.endsWith('.toml'))) { - const { stdout } = await git('show', `FETCH_HEAD:${path}`); - upstream.set(path.slice('.gitsheets/'.length), stdout); + for (const path of await listFilesUnder(repo, fetched.remoteCommit, '.gitsheets')) { + if (!path.endsWith('.toml')) continue; + const blob = await repo.readBlob(fetched.remoteCommit, path); + upstream.set(path.slice('.gitsheets/'.length), (await blob.read()).toString('utf8')); } const vendored = await readTomlTree(VENDORED_DIR); diff --git a/apps/api/scripts/scrub-data.ts b/apps/api/scripts/scrub-data.ts index 72e47e0..f67990b 100644 --- a/apps/api/scripts/scrub-data.ts +++ b/apps/api/scripts/scrub-data.ts @@ -27,15 +27,15 @@ * would land on another record's path — fails the run instead of being * silently collapsed. * - * The source's `.gitsheets/` files (configs + vendored contracts) are copied - * with `tx.writeFile` into a detached gitsheets orphan commit that the record - * transaction builds on; the staged tree is then re-committed as the - * parentless snapshot on `snapshot--scrubbed`, HEAD pointed at it and a - * same-named tag added (`.github/workflows/snapshot.yml` pushes HEAD's - * branch). + * The whole snapshot is one parentless gitsheets transaction on + * `snapshot--scrubbed`: it stages the source's `.gitsheets/` files + * (configs + vendored contracts) with `tx.writeFile`, then upserts the + * records, whose sheets read those staged configs. HEAD is pointed at the + * branch and a same-named tag added (`.github/workflows/snapshot.yml` pushes + * HEAD's branch). * - * If verification fails the script exits non-zero and no target ref moves - * (the staged objects are left unreferenced). + * If verification fails the script exits non-zero and nothing is committed: + * the checks run inside the transaction, so a failure discards it. */ import { createHash } from 'node:crypto'; @@ -421,39 +421,18 @@ async function initOrOpenTargetRepo(targetPath: string): Promise { } /** - * Write a parentless commit holding only the source's `.gitsheets/` files - * (configs plus vendored contracts), for the record transaction to build on: - * an orphan `transact({ parent: null })` staging each file with - * `tx.writeFile`. No branch is named, so the commit is detached and no ref - * moves. - * - * The configs can't go in the record transaction itself: a transaction's - * sheets load their configs from its parent commit (from HEAD for an orphan), - * never from files staged with `tx.writeFile` in the same transaction. + * Every file path under `dir` at `ref`, recursively (gitsheets `listTree` + * lists one level). */ -async function writeConfigRootCommit( - repo: Repository, - configs: ReadonlyArray<{ path: string; content: Buffer }>, -): Promise { - const { commitHash } = await repo.transact( - { parent: null, message: 'scrub-data: sheet configs', author: SNAPSHOT_AUTHOR }, - async (tx) => { - for (const { path, content } of configs) tx.writeFile(path, content); - }, - ); - if (commitHash === null) throw new Error('scrub-data: internal: config root transaction produced no commit'); - return commitHash; -} - -function gitIdentityEnv(timestamp: string): Record { - return { - GIT_AUTHOR_NAME: SNAPSHOT_AUTHOR.name, - GIT_AUTHOR_EMAIL: SNAPSHOT_AUTHOR.email, - GIT_COMMITTER_NAME: SNAPSHOT_AUTHOR.name, - GIT_COMMITTER_EMAIL: SNAPSHOT_AUTHOR.email, - GIT_AUTHOR_DATE: timestamp, - GIT_COMMITTER_DATE: timestamp, - }; +async function listFilesUnder(repo: Repository, ref: string, dir: string): Promise { + const entries = (await repo.listTree(ref, dir)) ?? []; + const out: string[] = []; + for (const entry of entries) { + const path = `${dir}/${entry.name}`; + if (entry.type === 'tree') out.push(...(await listFilesUnder(repo, ref, path))); + else if (entry.type === 'blob') out.push(path); + } + return out; } /** @@ -545,11 +524,12 @@ export async function scrubRepo(opts: ScrubOptions): Promise { } // ------------------------------------------------------------------------- - // 6. Stage the snapshot tree through gitsheets: a transaction on top of a - // config-only root commit upserts every scrubbed record (canonical + // 6. Write the snapshot through gitsheets: one parentless transaction + // stages the `.gitsheets/` files first (its sheets read configs from the + // transaction's own tree), then upserts every scrubbed record (canonical // serialization, path templates, content-typed formats) and copies - // attachments. The transaction commits on a bare hash parent, so no ref - // moves; its tree is re-committed as the orphan snapshot in step 9. + // attachments. Count parity and verification (steps 7 and 8) run inside + // the handler, so a failure throws before anything commits. // ------------------------------------------------------------------------- const git = await initOrOpenTargetRepo(target); // Raw git: resolves a non-bare target's `.git` for openRepo. @@ -559,26 +539,42 @@ export async function scrubRepo(opts: ScrubOptions): Promise { // Everything under the source's `.gitsheets/`: sheet configs plus the // vendored schema contracts (`contracts/…`) that configs may declare. const sourceRef = sourceHeadHash ?? 'HEAD'; - // Raw git: gitsheets reads a blob at a path but has no subtree listing. - const configPaths = ( - await exec('git', ['ls-tree', '-r', '--name-only', sourceRef, '.gitsheets/'], { cwd: source }) - ).stdout - .split('\n') - .filter((p) => p.length > 0); + const configPaths = await listFilesUnder(sourceRepo, sourceRef, '.gitsheets'); const configs = await Promise.all( configPaths.map(async (path) => ({ path, content: await (await sourceRepo.readBlob(sourceRef, path)).read() })), ); - const configRoot = await writeConfigRootCommit(targetRepo, configs); + + const timestamp = new Date().toISOString(); + const branchName = `snapshot-${seed.replace(/[^a-z0-9]/gi, '-')}-scrubbed`; + const commitMessage = `snapshot: anonymized data export from ${timestamp} + +Generated by apps/api/scripts/scrub-data.ts. +Source revision: ${sourceHeadHash ?? 'unknown'} +Seed: ${seed} + +This snapshot is intended for contributor onboarding. It contains no +real names, no emails, no GitHub identities. All personal data has +been pseudonymized via the documented scrubbing rules. + +See apps/api/scripts/scrub-data.ts and specs/behaviors/storage.md.`; + + // An orphan transaction creates its branch and refuses an existing one. + // A same-seed re-run (the branch exists) commits detached instead and + // moves the branch below only if the tree changed. + const existing = await targetRepo.resolveRef(`refs/heads/${branchName}`); const writtenFiles: SnapshotFile[] = []; const scrubCounts: Record = {}; - const staged = await targetRepo.transact( + const snapshot = await targetRepo.transact( { - parent: configRoot, - message: 'scrub-data: staged snapshot tree (re-committed as an orphan)', + parent: null, + ...(existing === null ? { branch: branchName } : {}), + message: commitMessage, author: SNAPSHOT_AUTHOR, }, async (tx) => { + for (const { path, content } of configs) tx.writeFile(path, content); + const pathsBySheet = new Map>(); for (const { sheetName, record, attachments } of scrubbedRecords) { const sheet = tx.sheet(sheetName); @@ -613,82 +609,55 @@ export async function scrubRepo(opts: ScrubOptions): Promise { await sheet.setAttachments(record, bytes); } } + + // --------------------------------------------------------------------- + // 7. Count validation — a record that collapsed onto another's path, + // or failed to write, shows up here. + // --------------------------------------------------------------------- + const countMismatches: string[] = []; + for (const [sheetName, sourceCount] of Object.entries(sourceCounts)) { + const scrubCount = scrubCounts[sheetName] ?? 0; + if (sourceCount !== scrubCount) { + countMismatches.push(`Sheet "${sheetName}": source=${sourceCount} scrubbed=${scrubCount}`); + } + } + if (countMismatches.length > 0) { + throw new Error(`Record count mismatch:\n${countMismatches.join('\n')}`); + } + + // --------------------------------------------------------------------- + // 8. Verification pass (before anything commits) + // --------------------------------------------------------------------- + const verification = verifySnapshotFiles(writtenFiles, new Set(allPersonSlugs)); + if (!verification.passed) { + throw new Error(`Verification failed:\n${verification.errors.join('\n')}`); + } }, ); - const treeHash = staged.treeHash; - if (treeHash === null) throw new Error('scrub-data: internal: staging transaction produced no tree'); + const commitHash = snapshot.commitHash; + if (commitHash === null) throw new Error('scrub-data: internal: snapshot transaction produced no commit'); // ------------------------------------------------------------------------- - // 7. Count validation — a record that collapsed onto another's path, or - // failed to write, shows up here. + // 9. Point the branch, HEAD and tag at the snapshot. // ------------------------------------------------------------------------- - const countMismatches: string[] = []; - for (const [sheetName, sourceCount] of Object.entries(sourceCounts)) { - const scrubCount = scrubCounts[sheetName] ?? 0; - if (sourceCount !== scrubCount) { - countMismatches.push(`Sheet "${sheetName}": source=${sourceCount} scrubbed=${scrubCount}`); + if (existing !== null) { + // Same-seed re-run: an unchanged tree is a no-op (the detached commit is + // left unreferenced). Equal root listings mean equal trees. + const [before, after] = await Promise.all([ + targetRepo.listTree(existing), + targetRepo.listTree(commitHash), + ]); + if (JSON.stringify(before) === JSON.stringify(after)) { + return { sourceCounts, commitHash: null, branchName }; } + // Raw git: gitsheets can't repoint an existing branch at an unrelated + // commit (an orphan transact refuses an existing branch). + await git('update-ref', `refs/heads/${branchName}`, commitHash); } - if (countMismatches.length > 0) { - throw new Error(`Record count mismatch:\n${countMismatches.join('\n')}`); - } - - // ------------------------------------------------------------------------- - // 8. Verification pass (before any ref moves) - // ------------------------------------------------------------------------- - const verification = verifySnapshotFiles(writtenFiles, new Set(allPersonSlugs)); - if (!verification.passed) { - throw new Error(`Verification failed:\n${verification.errors.join('\n')}`); - } - - // ------------------------------------------------------------------------- - // 9. Commit: orphan squashed commit of the staged tree - // - // Stays on raw git, each step for want of a gitsheets API: - // - commit-tree: re-committing the staged tree as a parentless commit. - // gitsheets 2.9's orphan transact starts from an empty tree, and its - // sheets can't see configs staged in the same transaction (see - // writeConfigRootCommit), so the records can't be written straight - // into an orphan. - // - update-ref: gitsheets can't point a branch at an existing commit - // (only commit onto one), and a same-seed re-run overwrites the branch. - // - symbolic-ref HEAD: gitsheets never moves HEAD. The snapshot workflow - // pushes the branch HEAD is on, so HEAD must point at the new branch. - // - tag: gitsheets has no tag API. - // ------------------------------------------------------------------------- - const timestamp = new Date().toISOString(); - const branchName = `snapshot-${seed.replace(/[^a-z0-9]/gi, '-')}-scrubbed`; - - // Raw git: HEAD's tree hash, which gitsheets doesn't expose. - const currentTree = await git('rev-parse', '--verify', '--quiet', 'HEAD^{tree}') - .then((r) => r.stdout.trim()) - .catch(() => null); - if (currentTree === treeHash) { - return { sourceCounts, commitHash: null, branchName }; - } - - const commitMessage = `snapshot: anonymized data export from ${timestamp} - -Generated by apps/api/scripts/scrub-data.ts. -Source revision: ${sourceHeadHash ?? 'unknown'} -Seed: ${seed} - -This snapshot is intended for contributor onboarding. It contains no -real names, no emails, no GitHub identities. All personal data has -been pseudonymized via the documented scrubbing rules. - -See apps/api/scripts/scrub-data.ts and specs/behaviors/storage.md.`; - - const commitEnv = { ...process.env, ...gitIdentityEnv(timestamp) }; - const { stdout: commitHashRaw } = await exec('git', ['commit-tree', treeHash, '-m', commitMessage], { - cwd: target, - env: commitEnv, - }); - const commitHash = commitHashRaw.trim(); - - await git('update-ref', `refs/heads/${branchName}`, commitHash); + // Raw git: gitsheets never moves HEAD. The snapshot workflow pushes the + // branch HEAD is on, so HEAD must point at the new branch. await git('symbolic-ref', 'HEAD', `refs/heads/${branchName}`); - await exec('git', ['tag', '-f', branchName, commitHash], { cwd: target, env: commitEnv }); + await targetRepo.tag(branchName, commitHash, { force: true }); // Bring a non-bare target's working tree in line with the new HEAD. Raw // git: gitsheets works bare-style and never touches a working tree. diff --git a/apps/api/src/plugins/reconcile.ts b/apps/api/src/plugins/reconcile.ts index 13a9f11..96b6af0 100644 --- a/apps/api/src/plugins/reconcile.ts +++ b/apps/api/src/plugins/reconcile.ts @@ -21,24 +21,21 @@ * re-clone if needed. * * Decorates Fastify with `reconcileDataRepo({ onMoved })`: fetch outside the - * lock, then reconcile (gitsheets `repo.sync()`, see store/reconcile.ts) - * under the repository's write lock — the same mutex - * `transact` holds (gitsheets `repo.withLock`), so a reconcile can never race - * a write. When the reconcile moves the branch, the public store is - * refreshed and `onMoved` runs inside the same lock, so the hot-reload - * rebuild can't interleave with a write either. + * lock, then reconcile (gitsheets `repo.sync()` on the serving repository, + * see store/reconcile.ts) under its write lock — the same mutex `transact` + * holds, so a reconcile can never race a write. When the reconcile moves the + * branch, sync's `afterMove` refreshes the public store and runs `onMoved` + * inside that same lock, so the hot-reload rebuild can't interleave with a + * write either. * - * Pushes of the working branch go through the push daemon. At boot the - * daemon isn't registered yet; its startup backlog pushes anything ahead. + * Pushes of the working branch go through the push daemon: `repo.sync()` + * notifies it of an ahead/replayed tip. At boot the daemon isn't registered + * yet; its startup backlog pushes anything ahead. */ import type { FastifyInstance } from 'fastify'; import fp from 'fastify-plugin'; -import { - fetchRemoteBranch, - reconcileDataRepo, - type ReconcileResult, -} from '../store/reconcile.js'; +import { reconcileDataRepo, type ReconcileResult } from '../store/reconcile.js'; export interface ReconcileCallOptions { /** @@ -61,7 +58,6 @@ declare module 'fastify' { } async function reconcilePlugin(fastify: FastifyInstance): Promise { - const repoPath = fastify.config.CFP_DATA_REPO_PATH; const configuredBranch = fastify.config.CFP_DATA_BRANCH; const remote = fastify.config.CFP_DATA_REMOTE; @@ -72,35 +68,17 @@ async function reconcilePlugin(fastify: FastifyInstance): Promise { if (!branch) { throw new Error('reconcileDataRepo: CFP_DATA_BRANCH is unset'); } - const logger = fastify.log; - - // Network outside the lock: writes keep flowing during a slow fetch. - // gitsheets' sync moves refs compare-and-swap. - if (!(await fetchRemoteBranch({ repoPath, branch, logger }))) { - const head = (await fastify.publicRepo.resolveRef('HEAD')) ?? ''; - return { outcome: 'fetch-failed', oldCommit: head, newCommit: head }; - } - - // One critical section of the serving repository's write lock covers - // the ref move, the store refresh and `onMoved` (the hot-reload - // rebuild), so no transact can run between them. reconcileDataRepo - // syncs through a side Repository handle because `repo.sync()` takes - // its own handle's lock and refuses to nest inside `withLock`. - return fastify.publicRepo.withLock(async () => { - const result = await reconcileDataRepo({ - repoPath, - branch, - logger, - skipFetch: true, - push: (commit) => { - fastify.pushDaemon?.notifyCommit(commit); - }, - }); - if (result.newCommit !== result.oldCommit) { + // The fetch runs outside the lock, so writes keep flowing during a + // slow network; the ref move, the store refresh and `onMoved` (the + // hot-reload rebuild) share one critical section of the write lock. + return reconcileDataRepo({ + repo: fastify.publicRepo, + branch, + logger: fastify.log, + afterMove: async (result) => { await fastify.store.public.refresh(); await opts?.onMoved?.(result); - } - return result; + }, }); }, ); diff --git a/apps/api/src/store/memory/reload.ts b/apps/api/src/store/memory/reload.ts index 0ffcae7..d8c9209 100644 --- a/apps/api/src/store/memory/reload.ts +++ b/apps/api/src/store/memory/reload.ts @@ -20,17 +20,13 @@ * Used by `apps/api/src/routes/internal.ts` (the `POST * /api/_internal/reload-data` webhook). */ -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; - import type { FastifyInstance } from 'fastify'; +import type { Repository } from 'gitsheets'; import { invalidateFacets } from './facets.js'; import { applyChanges, collectChanges } from './incremental.js'; import { loadInMemoryState } from './loader.js'; import type { InMemoryState } from './state.js'; -const exec = promisify(execFile); - export interface ReloadOptions { /** * The commit the live state currently reflects. When given, the reload @@ -43,10 +39,12 @@ export interface ReloadOptions { export type ReloadMode = 'incremental' | 'full'; /** Whether `.gitsheets/` (the sheet configs) differs between two commits. */ -async function sheetConfigsChanged(repoPath: string, from: string, to: string): Promise { - const tree = async (commit: string): Promise => - (await exec('git', ['rev-parse', `${commit}:.gitsheets`], { cwd: repoPath })).stdout.trim(); - const [a, b] = await Promise.all([tree(from), tree(to)]); +async function sheetConfigsChanged(repo: Repository, from: string, to: string): Promise { + // Direct entries carry each blob's and subtree's hash, so equal listings + // mean equal `.gitsheets/` trees (contracts/ included). + const listing = async (commit: string): Promise => + JSON.stringify(await repo.listTree(commit, '.gitsheets')); + const [a, b] = await Promise.all([listing(from), listing(to)]); return a !== b; } @@ -83,8 +81,7 @@ export async function reloadInMemoryStateAndFts( ): Promise { const { fromCommit } = opts; if (fromCommit) { - const repoPath = fastify.config.CFP_DATA_REPO_PATH; - if (!(await sheetConfigsChanged(repoPath, fromCommit, 'HEAD'))) { + if (!(await sheetConfigsChanged(fastify.publicRepo, fromCommit, 'HEAD'))) { const changes = await collectChanges(fastify.store.public, fromCommit); if (changes) { applyChanges(fastify.inMemoryState, fastify.fts, changes); diff --git a/apps/api/src/store/reconcile.ts b/apps/api/src/store/reconcile.ts index 702ca27..b131bf8 100644 --- a/apps/api/src/store/reconcile.ts +++ b/apps/api/src/store/reconcile.ts @@ -18,31 +18,23 @@ * fetch itself fails (network blip) → 'fetch-failed', no changes * * This module maps gitsheets' outcome vocabulary onto ours ('ahead' → - * 'pushed-ahead'), strips `refs/heads/` off the conflict branch, routes the - * push of an ahead/replayed tip through the caller's `push` hand-off, and - * keeps the operator-facing log lines. + * 'pushed-ahead'), strips `refs/heads/` off the conflict branch and keeps + * the operator-facing log lines. `repo.sync()` hands an ahead/replayed tip + * to the repository's running push daemon itself; at boot there is none + * yet, and the daemon's startup backlog pushes it. * * Unrecoverable errors (unrelated histories, missing branch, corrupt repo) * propagate as rejections — the boot plugin lets the API crash and k8s * restarts the pod. * - * Locking: `repo.sync()` takes the write lock of the Repository it is called - * on, and refuses (`lock_held`) to run inside that repository's `withLock`. - * The API needs the ref move, store refresh and in-memory rebuild in ONE - * critical section of the *serving* repository's lock, so this function - * syncs through its own short-lived Repository handle on the same gitdir: - * the caller holds the serving handle's lock — the one every write contends - * on — for the whole reconcile, and the side handle's own lock is - * uncontended. The side handle has no live sheets and no push daemon, which - * is why the caller refreshes its store and `push` hands the tip to the - * serving daemon. See plugins/reconcile.ts. + * Locking: `repo.sync()` takes the write lock of the Repository it runs on + * — the serving one, the mutex every `transact` contends on — for the ref + * move, and awaits the caller's `afterMove` inside it. The API's store + * refresh and in-memory rebuild run there, so no write can commit between + * the ref move and the end of the rebuild. The fetch runs before, outside + * the lock. See plugins/reconcile.ts. */ -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; - -import { openRepo, type SyncResult } from 'gitsheets'; - -const exec = promisify(execFile); +import type { Repository, SyncResult } from 'gitsheets'; export const AUTHOR_NAME = 'Code for Philly API'; export const AUTHOR_EMAIL = 'api@users.noreply.codeforphilly.org'; @@ -77,57 +69,59 @@ export interface ReconcileLogger { } export interface ReconcileOptions { - /** Absolute path to the bare data clone. */ - readonly repoPath: string; + /** + * The serving Repository on the bare data clone. Sync takes its write + * lock, so `afterMove` is atomic with respect to its `transact`s. + */ + readonly repo: Repository; /** Branch to reconcile. */ readonly branch: string; /** Remote name to fetch/push against. Default: 'origin'. */ readonly remote?: string; /** Logger; mirrors Fastify's pino interface. */ readonly logger: ReconcileLogger; + /** Skip the fetch: the caller already ran {@link fetchRemoteBranch}. */ + readonly skipFetch?: boolean; /** - * Skip the fetch: the caller already ran {@link fetchRemoteBranch} (the - * API does, outside the repository write lock, so a slow network never - * blocks writes). + * Awaited under the repository's write lock after the reconcile moved the + * branch (gitsheets `sync`'s `afterMove`), before the lock is released: + * no `transact` can commit in between. It must not `transact` or + * `withLock` itself (`lock_held`). A throw propagates after the lock is + * released, with the branch already moved. Not called when nothing moved. */ - readonly skipFetch?: boolean; + readonly afterMove?: (result: ReconcileResult) => Promise | void; /** - * Publish `branch` after an ahead or rebased outcome. The API hands the - * commit to the gitsheets push daemon so every push of the working branch - * goes through one place; at boot (no daemon yet) the daemon's startup - * backlog pushes it. Omitted, nothing is pushed. + * Extra hand-off for an ahead or rebased tip, called after the sync. + * Only for a repository with no running push daemon: `repo.sync()` + * already notifies a running one, so passing this as well would notify + * twice. Omitted, nothing beyond sync's own hand-off happens. */ readonly push?: (commit: string) => Promise | void; } /** * Fetch `/` into `refs/remotes//` without - * touching any lock. Returns false (after logging) when the fetch fails. - * - * `repo.sync()` can fetch, but takes the write lock in the same call, and - * the API must hold the lock across sync *and* rebuild — so it fetches here - * first, outside any lock, then syncs with `skipFetch`. Same explicit - * refspec sync uses, so a single-branch or bare clone still populates the - * remote-tracking ref. + * taking the write lock (gitsheets `repo.fetch`, which never throws on a + * network or auth failure). Returns false (after logging) when the fetch + * fails. Same explicit refspec sync uses, so a single-branch or bare clone + * still populates the remote-tracking ref. */ export async function fetchRemoteBranch(opts: { - readonly repoPath: string; + readonly repo: Repository; readonly branch: string; readonly remote?: string; readonly logger: ReconcileLogger; }): Promise { const remote = opts.remote ?? 'origin'; - const refspec = `+refs/heads/${opts.branch}:refs/remotes/${remote}/${opts.branch}`; - try { - await exec('git', ['fetch', '--prune', remote, refspec], { cwd: opts.repoPath }); - return true; - } catch (err) { + const fetched = await opts.repo.fetch({ remote, branch: opts.branch }); + if (!fetched.ok) { opts.logger.warn( - { err: describe(err), remote, branch: opts.branch }, + { err: fetched.error, remote, branch: opts.branch }, 'data-repo fetch failed; continuing with local state', ); return false; } + return true; } function describe(err: unknown): string { @@ -154,48 +148,55 @@ function mapOutcome(outcome: SyncResult['outcome']): ReconcileOutcome { * Idempotent — calling repeatedly with no upstream changes is a no-op * (`outcome: 'in-sync'`). * - * Concurrency: the API calls this while holding the serving Repository's - * write lock (see the module comment). At boot there's no contention. + * Concurrency: the fetch runs outside any lock; the ref move and + * `afterMove` run in one critical section of `repo`'s write lock (see the + * module comment). */ export async function reconcileDataRepo( opts: ReconcileOptions, ): Promise { const remote = opts.remote ?? 'origin'; - const { repoPath, branch, logger } = opts; + const { repo, branch, logger } = opts; + + if (!opts.skipFetch && !(await fetchRemoteBranch({ repo, branch, remote, logger }))) { + const current = (await repo.resolveRef(`refs/heads/${branch}`)) ?? ''; + return { outcome: 'fetch-failed', oldCommit: current, newCommit: current }; + } - // A side handle — see the module comment on locking. - const repo = await openRepo({ gitDir: repoPath }); + const afterMove = opts.afterMove; const raw = await repo.sync({ remote, branch, - fetch: !opts.skipFetch, + fetch: false, onConflict: 'preserve-branch', conflictBranchPrefix: 'conflicts/', pushConflictBranch: true, committer: { name: AUTHOR_NAME, email: AUTHOR_EMAIL }, + ...(afterMove ? { afterMove: (moved: SyncResult) => afterMove(toResult(moved)) } : {}), }); - const outcome = mapOutcome(raw.outcome); - const { oldCommit, newCommit, ahead, behind } = raw; + const result = toResult(raw); + const { outcome, oldCommit, newCommit, ahead, behind } = result; switch (outcome) { case 'fetch-failed': + // Unreachable with fetch: false; kept for exhaustiveness. logger.warn( { err: raw.fetchError, remote, branch }, 'data-repo fetch failed; continuing with local state', ); - return { outcome, oldCommit, newCommit }; + return result; case 'in-sync': logger.info({ branch, commit: oldCommit }, 'data-repo in sync with remote'); - return { outcome, oldCommit, newCommit, ahead, behind }; + return result; case 'fast-forwarded': logger.info( { branch, behind, from: oldCommit, to: newCommit }, 'data-repo behind remote — fast-forwarding', ); - return { outcome, oldCommit, newCommit, ahead, behind }; + return result; case 'pushed-ahead': case 'rebased': @@ -217,15 +218,14 @@ export async function reconcileDataRepo( ); } } - return { outcome, oldCommit, newCommit, ahead, behind }; + return result; case 'conflict-escaped': { - const conflictBranch = (raw.conflictBranch ?? '').replace(/^refs\/heads\//, ''); const fields = { branch, ahead, behind, - conflictBranch, + conflictBranch: result.conflictBranch, preservedCommit: raw.conflictCommit ?? oldCommit, remoteCommit: newCommit, }; @@ -235,7 +235,18 @@ export async function reconcileDataRepo( ? 'data-repo replay conflicted — divergent commits preserved on remote; operator must investigate' : 'data-repo replay conflicted — divergent commits preserved LOCALLY only (push failed); operator must investigate', ); - return { outcome, oldCommit, newCommit, conflictBranch, ahead, behind }; + return result; } } } + +/** gitsheets' sync result → ours. */ +function toResult(raw: SyncResult): ReconcileResult { + const outcome = mapOutcome(raw.outcome); + const { oldCommit, newCommit, ahead, behind } = raw; + if (outcome === 'conflict-escaped') { + const conflictBranch = (raw.conflictBranch ?? '').replace(/^refs\/heads\//, ''); + return { outcome, oldCommit, newCommit, conflictBranch, ahead, behind }; + } + return { outcome, oldCommit, newCommit, ahead, behind }; +} diff --git a/apps/api/tests/data-repo-reconcile.test.ts b/apps/api/tests/data-repo-reconcile.test.ts index 0646032..d756fde 100644 --- a/apps/api/tests/data-repo-reconcile.test.ts +++ b/apps/api/tests/data-repo-reconcile.test.ts @@ -25,6 +25,7 @@ import { join } from 'node:path'; import { promisify } from 'node:util'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { openRepo } from 'gitsheets'; import { AUTHOR_EMAIL, @@ -193,7 +194,7 @@ describe('reconcileDataRepo', () => { const before = await git(rig.local, 'rev-parse', 'HEAD'); const result = await reconcileDataRepo({ - repoPath: rig.local, + repo: await openRepo({ gitDir: rig.local }), branch: 'main', logger, }); @@ -211,7 +212,7 @@ describe('reconcileDataRepo', () => { const { logger, lines } = makeLogger(); const result = await reconcileDataRepo({ - repoPath: rig.local, + repo: await openRepo({ gitDir: rig.local }), branch: 'main', logger, }); @@ -234,7 +235,7 @@ describe('reconcileDataRepo', () => { const pushed: string[] = []; const { logger } = makeLogger(); const result = await reconcileDataRepo({ - repoPath: rig.local, + repo: await openRepo({ gitDir: rig.local }), branch: 'main', logger, push: (commit) => { @@ -267,7 +268,7 @@ describe('reconcileDataRepo', () => { const pushed: string[] = []; const { logger } = makeLogger(); const result = await reconcileDataRepo({ - repoPath: rig.local, + repo: await openRepo({ gitDir: rig.local }), branch: 'main', logger, push: (commit) => { @@ -309,7 +310,7 @@ describe('reconcileDataRepo', () => { const pushed: string[] = []; const { logger } = makeLogger(); const result = await reconcileDataRepo({ - repoPath: rig.local, + repo: await openRepo({ gitDir: rig.local }), branch: 'main', logger, push: (commit) => { @@ -325,11 +326,11 @@ describe('reconcileDataRepo', () => { it('skips the fetch when skipFetch is set', async () => { const { logger } = makeLogger(); - expect(await fetchRemoteBranch({ repoPath: rig.local, branch: 'main', logger })).toBe(true); + expect(await fetchRemoteBranch({ repo: await openRepo({ gitDir: rig.local }), branch: 'main', logger })).toBe(true); await advanceRemote(rig, 'remote-only.txt', 'R\n', 'remote: unseen change'); const result = await reconcileDataRepo({ - repoPath: rig.local, + repo: await openRepo({ gitDir: rig.local }), branch: 'main', logger, skipFetch: true, @@ -356,7 +357,7 @@ describe('reconcileDataRepo', () => { let result; try { result = await reconcileDataRepo({ - repoPath: rig.local, + repo: await openRepo({ gitDir: rig.local }), branch: 'main', logger, }); @@ -408,7 +409,7 @@ describe('reconcileDataRepo', () => { const { logger, lines } = makeLogger(); const result = await reconcileDataRepo({ - repoPath: rig.local, + repo: await openRepo({ gitDir: rig.local }), branch: 'main', logger, }); @@ -461,7 +462,7 @@ describe('reconcileDataRepo', () => { const { logger } = makeLogger(); const result = await reconcileDataRepo({ - repoPath: altLocal, + repo: await openRepo({ gitDir: altLocal }), branch: 'main', logger, }); @@ -489,7 +490,7 @@ describe('reconcileDataRepo', () => { const before = await git(rig.local, 'rev-parse', 'HEAD'); const { logger } = makeLogger(); await expect( - reconcileDataRepo({ repoPath: rig.local, branch: 'main', logger }), + reconcileDataRepo({ repo: await openRepo({ gitDir: rig.local }), branch: 'main', logger }), ).rejects.toThrow(); expect(await git(rig.local, 'rev-parse', 'HEAD')).toBe(before); }); diff --git a/apps/api/tests/scrub-data.test.ts b/apps/api/tests/scrub-data.test.ts index a64ae36..cd6d6ac 100644 --- a/apps/api/tests/scrub-data.test.ts +++ b/apps/api/tests/scrub-data.test.ts @@ -526,6 +526,31 @@ describe('scrubRepo', () => { INTEGRATION_TIMEOUT, ); + it( + 'moves the branch, HEAD and tag to a new parentless snapshot on a same-seed re-run with changed content', + async () => { + const project = { id: uuid(2), slug: 'proj', title: 'Proj', stage: 'bootstrapping', createdAt: NOW, updatedAt: NOW }; + sourceTmp = await makeSourceRepo({ projects: [project] }); + targetTmp = await makeTmp(); + const git = async (cwd: string, ...args: string[]) => (await exec('git', args, { cwd })).stdout.trim(); + + const first = await scrubRepo({ source: sourceTmp.path, target: targetTmp.path, seed: 'rerun-test' }); + await sourceTmp.cleanup(); + sourceTmp = await makeSourceRepo({ projects: [{ ...project, title: 'Proj Renamed' }] }); + const second = await scrubRepo({ source: sourceTmp.path, target: targetTmp.path, seed: 'rerun-test' }); + + const commit = second.commitHash!; + expect(commit).toBeTruthy(); + expect(commit).not.toBe(first.commitHash); + expect(await git(targetTmp.path, 'rev-list', '--parents', '-n', '1', commit)).toBe(commit); + expect(await git(targetTmp.path, 'rev-parse', 'refs/heads/snapshot-rerun-test-scrubbed')).toBe(commit); + expect(await git(targetTmp.path, 'rev-parse', 'HEAD')).toBe(commit); + expect(await git(targetTmp.path, 'rev-parse', 'refs/tags/snapshot-rerun-test-scrubbed')).toBe(commit); + expect(await git(targetTmp.path, 'show', `${commit}:projects/proj.toml`)).toContain('Proj Renamed'); + }, + INTEGRATION_TIMEOUT, + ); + it( 'record counts match source vs snapshot', async () => { diff --git a/package-lock.json b/package-lock.json index 797d688..bdf355c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -43,7 +43,7 @@ "bcryptjs": "^3.0.3", "better-sqlite3": "^12.10.0", "fastify": "^5.8.5", - "gitsheets": "^2.9.0", + "gitsheets": "^2.11.0", "jose": "^6.2.3", "postmark": "^5.1.0", "samlify": "^2.13.0", @@ -2516,26 +2516,26 @@ } }, "node_modules/@gitsheets/core-napi": { - "version": "0.7.0", - "resolved": "https://registry.npmjs.org/@gitsheets/core-napi/-/core-napi-0.7.0.tgz", - "integrity": "sha512-zNhmETpIB0aNul57jIECWfz8o+RTmw1atQm4bkI6R12hD77QuXYgDlEc8XFKD9B2YxzWlOz4ZxLuNqoy1+o7sA==", + "version": "0.8.0", + "resolved": "https://registry.npmjs.org/@gitsheets/core-napi/-/core-napi-0.8.0.tgz", + "integrity": "sha512-ht91xJG0K8wQ/+HQkuACCCLP8LVqaZRekhJ30vI1fqfdKOE6bR+QwKWK54/5IOGuWnzmEuEMdkhE6c45xh6D5w==", "license": "Apache-2.0", "engines": { "node": ">=20" }, "optionalDependencies": { - "@gitsheets/core-napi-darwin-arm64": "0.7.0", - "@gitsheets/core-napi-darwin-x64": "0.7.0", - "@gitsheets/core-napi-linux-arm64-gnu": "0.7.0", - "@gitsheets/core-napi-linux-x64-gnu": "0.7.0", - "@gitsheets/core-napi-linux-x64-musl": "0.7.0", - "@gitsheets/core-napi-win32-x64-msvc": "0.7.0" + "@gitsheets/core-napi-darwin-arm64": "0.8.0", + "@gitsheets/core-napi-darwin-x64": "0.8.0", + "@gitsheets/core-napi-linux-arm64-gnu": "0.8.0", + "@gitsheets/core-napi-linux-x64-gnu": "0.8.0", + "@gitsheets/core-napi-linux-x64-musl": "0.8.0", + "@gitsheets/core-napi-win32-x64-msvc": "0.8.0" } }, "node_modules/@gitsheets/core-napi-darwin-arm64": { - "version": "0.7.0", - "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-darwin-arm64/-/core-napi-darwin-arm64-0.7.0.tgz", - "integrity": "sha512-EC6kqiQudIs/WRDPZqJmaajlCx3KhHuyCGKCD+l1uns6/9T+muYT4MENWtk4oWtVJALEzV/lqB4Yj93pcPPdUg==", + "version": "0.8.0", + "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-darwin-arm64/-/core-napi-darwin-arm64-0.8.0.tgz", + "integrity": "sha512-cMNpu/0CbdPvJzr5NqWe/DBSO84TUwHcsuy2YUhFvsxdqnubnEX6q+6a6XY6KXNI81rHMlS4cjYnkO3RfiBz8Q==", "cpu": [ "arm64" ], @@ -2549,9 +2549,9 @@ } }, "node_modules/@gitsheets/core-napi-darwin-x64": { - "version": "0.7.0", - "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-darwin-x64/-/core-napi-darwin-x64-0.7.0.tgz", - "integrity": "sha512-SzwSsxkdfCv1ZhYv/Wy8kt3SiRdfIi4SWVRVH5kmprvvcUBJIgUz/3I1JY0udYQy/OG/nyNWuxTbKuhXv3LSKA==", + "version": "0.8.0", + "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-darwin-x64/-/core-napi-darwin-x64-0.8.0.tgz", + "integrity": "sha512-/z6zs0GjWKNhLhxj3K5Rg9AO65VrKeSRlJMJTSEmQYZ6Uj+qHXdzNTwokIKTSvsm5US8l9gTpChKjwbFtiA5bA==", "cpu": [ "x64" ], @@ -2565,9 +2565,9 @@ } }, "node_modules/@gitsheets/core-napi-linux-arm64-gnu": { - "version": "0.7.0", - "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-linux-arm64-gnu/-/core-napi-linux-arm64-gnu-0.7.0.tgz", - "integrity": "sha512-n0SQUOpyxQQrWvdqKl0vhGCwoqDQGelQwz7xyjtEXSoleJbMM6A4iets2Ukc+r01/virye5/vEU75w3N+epWog==", + "version": "0.8.0", + "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-linux-arm64-gnu/-/core-napi-linux-arm64-gnu-0.8.0.tgz", + "integrity": "sha512-jyLfuvnlkr1Qxxi4dRUbF0rmaqWR5bmkwVXzMHjKutP44Mt8krVpOCglip/jVCgyXO3J2PisgaE5F3EP7TvJ3A==", "cpu": [ "arm64" ], @@ -2581,9 +2581,9 @@ } }, "node_modules/@gitsheets/core-napi-linux-x64-gnu": { - "version": "0.7.0", - "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-linux-x64-gnu/-/core-napi-linux-x64-gnu-0.7.0.tgz", - "integrity": "sha512-j5nF2fFQKx1lfV7uGlP6pAKvlzHxqPcgbH44n4vwHbkKOYdxAvXtXdFLYqwWA5inJGDIBzxlR+lkPZ8uf+LPGQ==", + "version": "0.8.0", + "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-linux-x64-gnu/-/core-napi-linux-x64-gnu-0.8.0.tgz", + "integrity": "sha512-qJ4t9mg1ywC5in2M7vHh3tsMaV0K3aQ4RhJYC9JQgHtuO8n2UwLvejWcfiPcsSxbf7bhvflmnlAk1S9MejN1rw==", "cpu": [ "x64" ], @@ -2597,9 +2597,9 @@ } }, "node_modules/@gitsheets/core-napi-linux-x64-musl": { - "version": "0.7.0", - "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-linux-x64-musl/-/core-napi-linux-x64-musl-0.7.0.tgz", - "integrity": "sha512-D+meQwDTi+RuwOPcuxLLq53tFeKU2PhXTcjQjn8cfp36x+bxxcBzUUfbnW4tS2weRgQXpii1tzf3bgveDfsiww==", + "version": "0.8.0", + "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-linux-x64-musl/-/core-napi-linux-x64-musl-0.8.0.tgz", + "integrity": "sha512-TWVzZNoVcP6svKK8ADGgVSnKSioT+TKo1JSXZBykvqCA0hQfznUni2YX/DGIF/iFkWkG4Na3+AywEBUYSu5Lkg==", "cpu": [ "x64" ], @@ -2613,9 +2613,9 @@ } }, "node_modules/@gitsheets/core-napi-win32-x64-msvc": { - "version": "0.7.0", - "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-win32-x64-msvc/-/core-napi-win32-x64-msvc-0.7.0.tgz", - "integrity": "sha512-B+uKhjTHQcUhXmh93dZg0G57QM3ZmEPMpblNdh1QwrvNIaG3NlJhqVOLniwvDO5/Dp2qurRae6AuEqhNop++yQ==", + "version": "0.8.0", + "resolved": "https://registry.npmjs.org/@gitsheets/core-napi-win32-x64-msvc/-/core-napi-win32-x64-msvc-0.8.0.tgz", + "integrity": "sha512-rfvnt5eanVfxdr6/nr/xcR2VzLtMr7XQNdnh5Rrd9z99grTclQYMQwiPn7hxAiFHHGgqSqRXX/zseXTITmUXxw==", "cpu": [ "x64" ], @@ -8845,12 +8845,12 @@ "license": "MIT" }, "node_modules/gitsheets": { - "version": "2.9.0", - "resolved": "https://registry.npmjs.org/gitsheets/-/gitsheets-2.9.0.tgz", - "integrity": "sha512-2fQx7W0PJ2mAOS678QT3UX5WNM0i97++Wci8OXnlwGYIlz61LuQadl8kjzZMRDN7rI8aibPnok41fGGu+5bAOw==", + "version": "2.11.0", + "resolved": "https://registry.npmjs.org/gitsheets/-/gitsheets-2.11.0.tgz", + "integrity": "sha512-bPKoGk/j/fp4h/VjNiejsmeDiknFRJECM701X0oFFk2y0B8klSdcgegicbuZGIBFqMOIXdIJvFrLtO5UsNSgqg==", "license": "Apache-2.0", "dependencies": { - "@gitsheets/core-napi": "^0.7.0", + "@gitsheets/core-napi": "^0.8.0", "csv-parse": "^7.0.2", "csv-stringify": "^6.7.0", "rfc6902": "^5.2.0", diff --git a/plans/gitsheets-2.11.md b/plans/gitsheets-2.11.md new file mode 100644 index 0000000..1ba57ea --- /dev/null +++ b/plans/gitsheets-2.11.md @@ -0,0 +1,163 @@ +--- +status: done +depends: [gitsheets-sync, gitsheets-scripts-2.9] +specs: + - specs/behaviors/storage.md +upstream-specs: + - gitsheets:specs/api/repository.md + - gitsheets:specs/api/transaction.md + - gitsheets:specs/behaviors/push-sync.md +pr: 258 +--- + +# Plan: adopt gitsheets 2.11 and retire the reconcile side handle + +## Scope + +gitsheets 2.11 ships the APIs that the last workarounds in reconcile and +scrub-data were waiting on: + +- **`repo.sync({ afterMove })`.** The hook is awaited under the write lock + after the branch moved and live sheets were rebound, before the push + daemon hand-off. The API's reconcile no longer needs a second `openRepo` + handle synced inside the serving handle's `withLock`. +- **`repo.fetch({ remote, branch })`.** Returns `{ ok, … }` and never throws + on network or auth failures. It replaces the raw `git fetch` in + `store/reconcile.ts` and pairs with `sync({ fetch: false })`. +- **`tx.sheet()` reads configs from the transaction's own tree.** Configs + staged with `tx.writeFile` before a sheet's first use now apply. That + closes the `gitsheets-scripts-2.9` follow-up: scrub-data's config root, + staging transaction and `commit-tree` re-commit collapse into one + `transact({ parent: null, branch })`. +- **`repo.tag(name, target, { force })` and `repo.listTree(ref, path)`.** + These replace `git tag -f` and `ls-tree`. + +In: the dependency bump, the reconcile rewrite onto the serving handle, +the scrub-data collapse, `check-sheet-configs` and the reload config-diff +moved onto `fetch`/`listTree` where they fit, and stale comments removed. + +Out: `backfill-*` scripts (still the open `gitsheets-scripts-2.9` +follow-up), any behavior change to the webhook, boot or snapshot outputs. + +## Implements + +- [behaviors/storage.md](../specs/behaviors/storage.md) → Hot reload, + Concurrency: one critical section of the write lock covers the ref move, + the store refresh and the rebuild. It is now `repo.sync()`'s `afterMove`. +- [behaviors/storage.md](../specs/behaviors/storage.md): every write to the + data repo goes through gitsheets; raw git stays only where gitsheets has + no API. + +## Approach + +1. `npm install gitsheets@^2.11.0 -w apps/api` as its own commit. +2. Spec wording: the Concurrency bullet names `repo.withLock`. Point it at + `sync`'s `afterMove`. +3. `store/reconcile.ts` takes the serving `Repository`. It fetches with + `repo.fetch` outside the lock, then calls `repo.sync({ fetch: false, + onConflict: 'preserve-branch', conflictBranchPrefix: 'conflicts/', + committer, afterMove })`. `plugins/reconcile.ts` drops its `withLock` + and passes an `afterMove` that refreshes the public store and runs + `onMoved`. `sync` notifies the repository's running push daemon itself, + so the plugin stops passing a `push` hand-off. +4. `scrub-data.ts`: one transaction stages `.gitsheets/` with + `tx.writeFile`, upserts records, and runs count parity and PII + verification inside the handler. If the snapshot branch is absent it + commits with `parent: null, branch`. A same-seed re-run commits detached + and compares trees: an equal tree is a no-op, otherwise it force-moves + the branch. The tag goes through `repo.tag(…, { force: true })` and + config listing through `repo.listTree`. +5. `check-sheet-configs.ts`: `repo.fetch` + `listTree` + `readBlob` against + a throwaway bare repo, if it fits. + +## Validation + +- [x] `gitsheets@^2.11.0` in `apps/api/package.json`; + `@gitsheets/core-napi-linux-x64-musl` present in `package-lock.json` +- [x] Reconcile runs through the serving handle; no second `openRepo` and + no surrounding `withLock` +- [x] `internal-reload.test.ts` "serializes a write issued during a hot + reload; neither change is lost" and "holds the write lock from the + ref move through the rebuild" pass unchanged +- [x] Outcome vocabulary, webhook response shape, conflict branch naming and + boot behavior unchanged (existing tests) +- [x] scrub-data writes the snapshot in one orphan transaction; snapshot + tests (parentless commit, HEAD + tag, same-seed no-op, failure leaves + no ref) pass +- [x] Every remaining raw `git` call in `apps/api/src` and `apps/api/scripts` + is listed in Notes with its reason +- [x] `type-check` + `lint` + full test suite green + +## Risks / unknowns + +- An orphan `transact` still refuses an existing branch, so a same-seed + re-run needs the detached path plus a raw branch move. +- `afterMove` runs inside the lock: anything in the rebuild that takes the + lock or transacts would be `lock_held`. The rebuild already ran inside + `withLock`, so it has the same constraint as before. + +## Notes + +- **Why `afterMove` keeps the guarantee.** `sync` acquires the serving + repository's write lock — the FIFO mutex every `transact` queues on — + moves the ref, rebinds live sheets, and awaits `afterMove` before + releasing it. The store refresh and the in-memory rebuild run in that + hook, so a write queued during a reload acquires the lock only after the + rebuild finished. That is the same critical section the old + `withLock` around a side-handle sync gave, now on one handle. Both + `internal-reload.test.ts` lock tests pass with no change to the file; + the lock-holding test's mechanism (`publicRepo.withLock` hold + + `onMoved`) never referenced the side handle. +- **A throw in `afterMove`** propagates after the lock is released with the + ref moved, same as a throw inside the old `withLock`. The webhook still + 500s with "pod restart required". +- **Push hand-off.** `sync` notifies the repository's running push daemon + of an ahead/replayed tip itself, so the plugin no longer passes `push`. + Passing it as well would notify twice. `reconcileDataRepo` keeps the + option for a repository without a daemon (the unit tests use it). At + boot there's no daemon yet, and its startup backlog pushes, as before. +- **Fetch-failed** now reports the branch's commit (`refs/heads/`) + rather than `HEAD`'s; the app's HEAD always points at that branch. +- **reload.ts.** The incremental reload's config-change guard used raw + `git rev-parse :.gitsheets`. It now compares + `repo.listTree(commit, '.gitsheets')` listings (each entry carries its + hash, so equal listings mean equal trees). That was the last raw git in + `apps/api/src`. +- **scrub-data re-run.** An orphan `transact` still refuses an existing + branch. A same-seed re-run commits detached, compares root `listTree` + listings with the branch (replacing `rev-parse HEAD^{tree}`), and returns + no commit when they match. When they differ it force-moves the branch + with `update-ref`. New test covers the changed-content re-run. The + commit date is now gitsheets' clock rather than the message timestamp + fed through `GIT_*_DATE`; the message still carries the timestamp. +- **check-sheet-configs** loses `--depth=1`: `repo.fetch` has no depth + option. The `empty` branch holds only `.gitsheets/`, so the full fetch + is small. Verified by hand against a local bare remote (match → exit 0, + drift → exit 1, bad remote → throws). +- **Raw `git` left:** + - `apps/api/src`: none. + - `scrub-data.ts` + - `rev-parse --git-dir`, `init`, `config`: init the target. gitsheets + has no init. + - `rev-parse --absolute-git-dir` / `--is-bare-repository`: locate a + non-bare target's git dir and detect a working tree. + - `update-ref`: same-seed re-run with changed content. gitsheets can't + repoint an existing branch at an unrelated commit. + - `symbolic-ref HEAD`: the snapshot workflow pushes HEAD's branch, and + gitsheets never moves HEAD. + - `reset --hard`: sync a non-bare target's working tree. gitsheets is + bare-style. + - `check-sheet-configs.ts`: `init --bare` (no init API) and + `remote add` (`repo.fetch` takes a remote name, not a URL). + - `backfill-laddr-events.ts`, `backfill-meetup-hack-nights.ts`: one + create-only `update-ref` each. Unchanged; see Follow-ups. + +## Follow-ups + +- Tracked as: the `gitsheets-scripts-2.9` follow-up for the two + `backfill-*` scripts (no-op-transact reads and create-only `update-ref`) + stays open. +- Tracked as: gitsheets gap. A way to point an existing branch at a given + commit (or an orphan transact that may replace a branch) would retire + scrub-data's last `update-ref`. A `repo.init` would retire the init + plumbing in scrub-data and check-sheet-configs. diff --git a/specs/behaviors/storage.md b/specs/behaviors/storage.md index 93db1ac..6ecf90a 100644 --- a/specs/behaviors/storage.md +++ b/specs/behaviors/storage.md @@ -351,7 +351,7 @@ A push to the configured `CFP_DATA_BRANCH` from outside the API (typically a mer - **Incremental by default** — the rebuild applies only what changed: for each in-memory sheet, the records that differ between the pre-reconcile commit and the new HEAD are unindexed (old version) and re-indexed (new version) in the live state and the FTS index. The result must equal a full rebuild from the new tree, every primary map and secondary index included, so no lookup path can serve pre-reload contents. - **Full-rebuild fallback** — a full rebuild replaces the incremental one when any `.gitsheets/` config changed between the two commits, or when more than 5,000 records changed (a bulk import merge). The full rebuild constructs a fresh `InMemoryState` first; only after that succeeds does it mutate the live Maps in place, replacing **every** collection from the fresh one. The FTS engine's `reload(state)` drops and re-inserts every FTS5 table. - **Failure** — if either rebuild throws partway, the route returns 500 and the operator should restart the pod. -- **Concurrency** — the fetch runs outside any lock, so a slow network never blocks writes. The ref move (`repo.sync()`), store refresh, rebuild and swap all run inside one critical section of the repository's write lock (gitsheets `repo.withLock`), the same mutex `transact` holds. A write issued during a reload waits for it and then applies on top of the reloaded state; no write can land between the ref move and the swap, so none is lost. +- **Concurrency** — the fetch runs outside any lock, so a slow network never blocks writes. The ref move, store refresh, rebuild and swap all run inside one critical section of the repository's write lock, the same mutex `transact` holds: gitsheets `repo.sync()` moves the ref under the lock and awaits its `afterMove` hook (the refresh and rebuild) before releasing it. A write issued during a reload waits for it and then applies on top of the reloaded state; no write can land between the ref move and the swap, so none is lost. The GitHub Actions workflow that calls this endpoint lives in the `codeforphilly-data` repo (`.github/workflows/notify-deployments.yml`), not in this app repo. It fires on push to `CFP_DATA_BRANCH` and posts `{ branch, commitHash: }` with the secret as a bearer token.