diff --git a/apps/api/src/store/memory/reload.ts b/apps/api/src/store/memory/reload.ts index 3e53ec3..bb1c430 100644 --- a/apps/api/src/store/memory/reload.ts +++ b/apps/api/src/store/memory/reload.ts @@ -85,42 +85,31 @@ export async function reloadInMemoryStateAndFts( } /** - * Synchronously replace the contents of every Map on `live` with the - * contents from `fresh`. Object identity of `live` is preserved. + * Synchronously replace the contents of every collection on `live` with + * the contents from `fresh`. Object identity of `live` — and of every Map + * hanging off it — is preserved. + * + * Enumerates `fresh`'s own properties rather than naming each field: a + * hand-maintained list silently skipped three secondary indices + * (`projectIdByLegacyId`, `buzzIdBySlug`, `slugHistory`) and left legacy + * and slug-history redirects pointing at ids that no longer existed after + * a re-import + hot reload. Every own property of `InMemoryState` is a Map + * today; if a future field is anything else this throws so the author has + * to decide how it's swapped, instead of it being skipped again. Per + * specs/behaviors/storage.md#hot-reload → Atomicity. * * Exported for testability — production code should call * `reloadInMemoryStateAndFts`. */ export function swapInPlace(live: InMemoryState, fresh: InMemoryState): void { - // Primary entity maps. - replaceMapContents(live.projects, fresh.projects); - replaceMapContents(live.people, fresh.people); - replaceMapContents(live.tags, fresh.tags); - replaceMapContents(live.tagAssignments, fresh.tagAssignments); - replaceMapContents(live.projectMemberships, fresh.projectMemberships); - replaceMapContents(live.projectUpdates, fresh.projectUpdates); - replaceMapContents(live.projectBuzz, fresh.projectBuzz); - replaceMapContents(live.blogPosts, fresh.blogPosts); - replaceMapContents(live.helpWantedRoles, fresh.helpWantedRoles); - replaceMapContents(live.helpWantedInterest, fresh.helpWantedInterest); - - // Secondary indices. - replaceMapContents(live.projectSlugById, fresh.projectSlugById); - replaceMapContents(live.projectIdBySlug, fresh.projectIdBySlug); - replaceMapContents(live.personSlugById, fresh.personSlugById); - replaceMapContents(live.personIdBySlug, fresh.personIdBySlug); - replaceMapContents(live.tagIdByHandle, fresh.tagIdByHandle); - replaceMapContents(live.membershipsByProject, fresh.membershipsByProject); - replaceMapContents(live.membershipsByPerson, fresh.membershipsByPerson); - replaceMapContents(live.updatesByProject, fresh.updatesByProject); - replaceMapContents(live.updateByProjectAndNumber, fresh.updateByProjectAndNumber); - replaceMapContents(live.buzzByProject, fresh.buzzByProject); - replaceMapContents(live.buzzByProjectAndSlug, fresh.buzzByProjectAndSlug); - replaceMapContents(live.blogPostIdBySlug, fresh.blogPostIdBySlug); - replaceMapContents(live.blogPostIdByLegacyId, fresh.blogPostIdByLegacyId); - replaceMapContents(live.helpWantedByProject, fresh.helpWantedByProject); - replaceMapContents(live.tagAssignmentsByTaggable, fresh.tagAssignmentsByTaggable); - replaceMapContents(live.tagAssignmentsByTag, fresh.tagAssignmentsByTag); - replaceMapContents(live.interestByRoleAndPerson, fresh.interestByRoleAndPerson); - replaceMapContents(live.interestByRole, fresh.interestByRole); + for (const key of Object.keys(fresh) as (keyof InMemoryState)[]) { + const target: unknown = live[key]; + const source: unknown = fresh[key]; + if (!(target instanceof Map) || !(source instanceof Map)) { + throw new Error( + `swapInPlace: InMemoryState.${key} is not a Map — extend swapInPlace to handle it`, + ); + } + replaceMapContents(target, source); + } } diff --git a/apps/api/tests/internal-reload.test.ts b/apps/api/tests/internal-reload.test.ts index fffd5c5..85e754d 100644 --- a/apps/api/tests/internal-reload.test.ts +++ b/apps/api/tests/internal-reload.test.ts @@ -15,6 +15,12 @@ * record introduced on the "remote" must be visible via a service * call AFTER the reload completes (proves the in-memory state + * FTS index actually got rebuilt against the new tree). + * - Re-import scenario: a project is replaced on the remote by a + * record with a fresh id and slug (what the laddr importer does on + * every run). After the reload the legacy `/projects?ID=` redirect, + * the `/project-buzz/` redirect, and the slug-history 301 + * must all resolve against the NEW records — the secondary indices + * behind them were once skipped by the in-place swap. */ import { execFile } from 'node:child_process'; import { mkdtemp, rm, writeFile } from 'node:fs/promises'; @@ -117,14 +123,14 @@ async function createRig(): Promise { } /** - * Advance the bare remote by one commit on `main` via an ephemeral - * clone. Used to put the local working tree behind so a hot reload - * fast-forwards. The new commit introduces a fresh project record at - * `projects/.toml`. + * Advance the bare remote by one commit on `main` via an ephemeral clone. + * `mutate` edits the clone's working tree and stages whatever it changed + * (paths are relative to the clone root). Returns the new remote HEAD. */ -async function advanceRemoteWithProject( +async function advanceRemote( rig: Rig, - fields: { id: string; slug: string; title: string; summary?: string }, + message: string, + mutate: (wt: string) => Promise, ): Promise { const wt = `${rig.local}-advance-${Date.now()}-${Math.random() .toString(36) @@ -135,12 +141,31 @@ async function advanceRemoteWithProject( await git(wt, 'config', 'commit.gpgsign', 'false'); await git(wt, 'config', 'core.hooksPath', '/dev/null'); - // Minimal Project TOML the gitsheets reader will accept + the Zod - // schema will validate at load time. The schema allows a lot of - // optional fields; we provide only the required ones plus a couple - // for the assertion. - const toml = [ + await mutate(wt); + await git(wt, 'commit', '-m', message); + await git(wt, 'push', 'origin', 'main'); + const head = await git(wt, 'rev-parse', 'HEAD'); + await rm(wt, { recursive: true, force: true }); + return head; +} + +interface ProjectFields { + id: string; + slug: string; + title: string; + summary?: string; + legacyId?: number; +} + +/** + * Minimal Project TOML the gitsheets reader will accept + the Zod schema + * will validate at load time. The schema allows a lot of optional fields; + * we provide only the required ones plus a couple for the assertions. + */ +function projectToml(fields: ProjectFields): string { + return [ `id = '${fields.id}'`, + ...(fields.legacyId !== undefined ? [`legacyId = ${fields.legacyId}`] : []), `slug = '${fields.slug}'`, `title = '${fields.title}'`, ...(fields.summary ? [`summary = '${fields.summary}'`] : []), @@ -150,14 +175,66 @@ async function advanceRemoteWithProject( `updatedAt = '2026-05-19T00:00:00Z'`, '', ].join('\n'); +} + +async function writeProject(wt: string, fields: ProjectFields): Promise { await exec('mkdir', ['-p', join(wt, 'projects')]); - await writeFile(join(wt, 'projects', `${fields.slug}.toml`), toml); + await writeFile(join(wt, 'projects', `${fields.slug}.toml`), projectToml(fields)); await git(wt, 'add', `projects/${fields.slug}.toml`); - await git(wt, 'commit', '-m', `seed: project ${fields.slug}`); - await git(wt, 'push', 'origin', 'main'); - const head = await git(wt, 'rev-parse', 'HEAD'); - await rm(wt, { recursive: true, force: true }); - return head; +} + +async function writeBuzz( + wt: string, + fields: { id: string; projectId: string; projectSlug: string; slug: string }, +): Promise { + const rel = `project-buzz/${fields.projectSlug}/${fields.slug}.toml`; + await exec('mkdir', ['-p', join(wt, 'project-buzz', fields.projectSlug)]); + await writeFile( + join(wt, rel), + [ + `id = '${fields.id}'`, + `projectId = '${fields.projectId}'`, + `slug = '${fields.slug}'`, + `headline = 'Buzz ${fields.slug}'`, + `url = 'https://example.test/${fields.slug}'`, + `publishedAt = '2026-05-19T00:00:00Z'`, + `createdAt = '2026-05-19T00:00:00Z'`, + `updatedAt = '2026-05-19T00:00:00Z'`, + '', + ].join('\n'), + ); + await git(wt, 'add', rel); +} + +async function writeSlugHistory( + wt: string, + fields: { id: string; entityId: string; oldSlug: string; newSlug: string }, +): Promise { + const rel = `slug-history/project/${fields.oldSlug}.toml`; + await exec('mkdir', ['-p', join(wt, 'slug-history', 'project')]); + await writeFile( + join(wt, rel), + [ + `id = '${fields.id}'`, + `entityType = 'project'`, + `oldSlug = '${fields.oldSlug}'`, + `newSlug = '${fields.newSlug}'`, + `entityId = '${fields.entityId}'`, + `changedAt = '2026-05-19T00:00:00Z'`, + `expiresAt = '2099-01-01T00:00:00Z'`, + '', + ].join('\n'), + ); + await git(wt, 'add', rel); +} + +/** + * Advance the remote by one commit that introduces a fresh project record + * at `projects/.toml`. Used to put the local clone behind so a hot + * reload fast-forwards. + */ +async function advanceRemoteWithProject(rig: Rig, fields: ProjectFields): Promise { + return advanceRemote(rig, `seed: project ${fields.slug}`, (wt) => writeProject(wt, fields)); } // --------------------------------------------------------------------------- @@ -390,4 +467,85 @@ describe('POST /api/_internal/reload-data — short-circuit + reconcile', () => const contents = await git(rig.local, 'show', 'HEAD:projects/lazyloader.toml'); expect(contents).toContain("slug = 'lazyloader'"); }); + + it('re-points legacy, buzz, and slug-history redirects after a re-import mints fresh ids', async () => { + // Seed the remote with a project carrying a laddr legacy id plus one + // buzz item, then bring the local clone up to date so the app boots + // in-sync with those records already indexed (production pods + // bare-clone fresh on every boot, so in-sync at boot is the norm). + const oldProjectId = '01951a3c-0000-7000-8000-000000000101'; + await advanceRemote(rig, 'seed: alpha-v1 + buzz', async (wt) => { + await writeProject(wt, { id: oldProjectId, slug: 'alpha-v1', title: 'Alpha', legacyId: 42 }); + await writeBuzz(wt, { + id: '01951a3c-0000-7000-8000-000000000103', + projectId: oldProjectId, + projectSlug: 'alpha-v1', + slug: 'alpha-launch', + }); + }); + await git(rig.local, 'fetch', 'origin', `${rig.branch}:${rig.branch}`); + app = await buildTestApp({ CFP_DATA_RELOAD_SECRET: VALID_SECRET }); + + const legacyBefore = await app.inject({ method: 'GET', url: '/projects?ID=42' }); + expect(legacyBefore.statusCode).toBe(301); + expect(legacyBefore.headers.location).toBe('/projects/alpha-v1'); + + const buzzBefore = await app.inject({ method: 'GET', url: '/project-buzz/alpha-launch' }); + expect(buzzBefore.statusCode).toBe(301); + expect(buzzBefore.headers.location).toBe('/projects/alpha-v1/buzz/alpha-launch'); + + // Re-import: the importer replaces the tree wholesale, minting fresh + // ids. Same legacy id, new project id + slug, new buzz id + slug, and + // a slug-history record so the old slug keeps resolving. + const newProjectId = '01951a3c-0000-7000-8000-000000000201'; + const newRemoteHead = await advanceRemote(rig, 're-import: alpha-v2', async (wt) => { + await git(wt, 'rm', '-q', 'projects/alpha-v1.toml', 'project-buzz/alpha-v1/alpha-launch.toml'); + await writeProject(wt, { id: newProjectId, slug: 'alpha-v2', title: 'Alpha', legacyId: 42 }); + await writeBuzz(wt, { + id: '01951a3c-0000-7000-8000-000000000203', + projectId: newProjectId, + projectSlug: 'alpha-v2', + slug: 'alpha-relaunch', + }); + await writeSlugHistory(wt, { + id: '01951a3c-0000-7000-8000-000000000206', + entityId: newProjectId, + oldSlug: 'alpha-v1', + newSlug: 'alpha-v2', + }); + }); + + const res = await app.inject({ + method: 'POST', + url: '/api/_internal/reload-data', + headers: { authorization: `Bearer ${VALID_SECRET}` }, + payload: { branch: rig.branch, commitHash: newRemoteHead }, + }); + expect(res.statusCode).toBe(200); + expect(res.json<{ data: { rebuilt: boolean } }>().data.rebuilt).toBe(true); + + // Legacy id → the NEW project. Before the fix, projectIdByLegacyId + // still held the old id, projectSlugById no longer knew it, and the + // request fell through to the SPA. + const legacyAfter = await app.inject({ method: 'GET', url: '/projects?ID=42' }); + expect(legacyAfter.statusCode).toBe(301); + expect(legacyAfter.headers.location).toBe('/projects/alpha-v2'); + + const updatesAfter = await app.inject({ method: 'GET', url: '/project-updates?ProjectID=42' }); + expect(updatesAfter.statusCode).toBe(301); + expect(updatesAfter.headers.location).toBe('/projects/alpha-v2'); + + // Buzz slug → the NEW buzz under the NEW project slug; the retired + // buzz slug no longer redirects. + const buzzAfter = await app.inject({ method: 'GET', url: '/project-buzz/alpha-relaunch' }); + expect(buzzAfter.statusCode).toBe(301); + expect(buzzAfter.headers.location).toBe('/projects/alpha-v2/buzz/alpha-relaunch'); + const buzzRetired = await app.inject({ method: 'GET', url: '/project-buzz/alpha-launch' }); + expect(buzzRetired.statusCode).not.toBe(301); + + // Slug history → the old project URL 301s to the new slug. + const slugAfter = await app.inject({ method: 'GET', url: '/projects/alpha-v1' }); + expect(slugAfter.statusCode).toBe(301); + expect(slugAfter.headers.location).toBe('/projects/alpha-v2'); + }); }); diff --git a/apps/api/tests/reload-swap.test.ts b/apps/api/tests/reload-swap.test.ts new file mode 100644 index 0000000..b018837 --- /dev/null +++ b/apps/api/tests/reload-swap.test.ts @@ -0,0 +1,247 @@ +/** + * Unit tests for `swapInPlace` — the in-place Map replacement behind the + * hot-reload webhook (specs/behaviors/storage.md#hot-reload → Atomicity). + * + * The regression this guards: `swapInPlace` used to name each field of + * `InMemoryState` by hand and skipped `projectIdByLegacyId`, + * `buzzIdBySlug`, and `slugHistory`. Because the laddr importer mints + * fresh ids every run, a re-import + hot reload left legacy redirects + * pointing at project ids that no longer existed. These tests enumerate + * every own property of a fresh state so a newly added collection can't + * be silently skipped again. + */ +import { describe, expect, it } from 'vitest'; +import type { + BlogPost, + HelpWantedInterestExpression, + HelpWantedRole, + Person, + Project, + ProjectBuzz, + ProjectMembership, + ProjectUpdate, + SlugHistory, + Tag, + TagAssignment, +} from '@cfp/shared/schemas'; + +import { swapInPlace } from '../src/store/memory/reload.js'; +import { + createEmptyState, + indexBlogPost, + indexHelpWantedInterest, + indexHelpWantedRole, + indexMembership, + indexPerson, + indexProject, + indexProjectBuzz, + indexProjectUpdate, + indexSlugHistory, + indexTag, + indexTagAssignment, + slugHistoryKey, + type InMemoryState, +} from '../src/store/memory/state.js'; + +const NOW = '2026-06-01T00:00:00Z'; +const FAR_FUTURE = '2099-01-01T00:00:00Z'; + +function uuid(n: number): string { + return `01951a3c-0000-7000-8000-${String(n).padStart(12, '0')}`; +} + +function makeProject(n: number, slug: string, legacyId: number): Project { + return { + id: uuid(n), + legacyId, + slug, + title: slug, + summary: null, + overview: null, + stage: 'prototyping', + maintainerId: null, + featured: false, + deletedAt: null, + createdAt: NOW, + updatedAt: NOW, + }; +} + +function makePerson(n: number, slug: string): Person { + return { + id: uuid(n), + slug, + fullName: slug, + accountLevel: 'user', + createdAt: NOW, + updatedAt: NOW, + } as Person; +} + +function makeBuzz(n: number, projectId: string, slug: string): ProjectBuzz { + return { + id: uuid(n), + projectId, + slug, + headline: slug, + url: `https://example.test/${slug}`, + publishedAt: NOW, + createdAt: NOW, + updatedAt: NOW, + }; +} + +function makeTag(n: number, slug: string): Tag { + return { id: uuid(n), namespace: 'tech', slug, title: slug, createdAt: NOW, updatedAt: NOW }; +} + +function makeAssignment(n: number, tagId: string, projectId: string): TagAssignment { + return { id: uuid(n), tagId, taggableType: 'project', taggableId: projectId, createdAt: NOW }; +} + +/** + * The remaining entity types only need the fields their index helpers read + * (ids + foreign keys). Cast rather than spell out every schema field — + * this test is about index bookkeeping, not record validation. + */ +function makeMembership(n: number, projectId: string, personId: string): ProjectMembership { + return { id: uuid(n), projectId, personId, role: 'member', createdAt: NOW, updatedAt: NOW } as unknown as ProjectMembership; +} + +function makeUpdate(n: number, projectId: string, number: number): ProjectUpdate { + return { id: uuid(n), projectId, number, createdAt: NOW, updatedAt: NOW } as unknown as ProjectUpdate; +} + +function makeBlogPost(n: number, slug: string, legacyId: number): BlogPost { + return { id: uuid(n), slug, legacyId, createdAt: NOW, updatedAt: NOW } as unknown as BlogPost; +} + +function makeRole(n: number, projectId: string): HelpWantedRole { + return { id: uuid(n), projectId, createdAt: NOW, updatedAt: NOW } as unknown as HelpWantedRole; +} + +function makeInterest(n: number, roleId: string, personId: string): HelpWantedInterestExpression { + return { id: uuid(n), roleId, personId, createdAt: NOW } as unknown as HelpWantedInterestExpression; +} + +function makeSlugHistory(n: number, entityId: string, oldSlug: string, newSlug: string): SlugHistory { + return { + id: uuid(n), + entityType: 'project', + entityId, + oldSlug, + newSlug, + changedAt: NOW, + expiresAt: FAR_FUTURE, + }; +} + +/** + * Build a state holding one record of every entity type, with ids drawn + * from `base + n`. Two calls with different bases model "before" and + * "after a re-import that minted fresh ids": every collection differs. + */ +function buildState(base: number, slugs: { project: string; buzz: string; oldSlug: string }): InMemoryState { + const state = createEmptyState(); + const project = makeProject(base + 1, slugs.project, 42); + const person = makePerson(base + 2, 'jane'); + const tag = makeTag(base + 4, 'flutter'); + const role = makeRole(base + 9, project.id); + + indexProject(state, project); + indexPerson(state, person); + indexProjectBuzz(state, makeBuzz(base + 3, project.id, slugs.buzz)); + indexTag(state, tag); + indexTagAssignment(state, makeAssignment(base + 5, tag.id, project.id)); + indexMembership(state, makeMembership(base + 6, project.id, person.id)); + indexProjectUpdate(state, makeUpdate(base + 7, project.id, 1)); + indexBlogPost(state, makeBlogPost(base + 8, `${slugs.project}-post`, 7)); + indexHelpWantedRole(state, role); + indexHelpWantedInterest(state, makeInterest(base + 10, role.id, person.id)); + indexSlugHistory(state, makeSlugHistory(base + 11, project.id, slugs.oldSlug, slugs.project)); + return state; +} + +/** "Before" state: ids in the 1xx range, project slug alpha-v1. */ +function buildLiveState(): InMemoryState { + return buildState(100, { project: 'alpha-v1', buzz: 'alpha-launch', oldSlug: 'alpha-v0' }); +} + +/** + * "After re-import" state: freshly minted ids (2xx range), renamed project + * slug, a different buzz slug, and a slug-history entry pointing at the new + * slug. Same legacy ids as the live state — that's the real-world shape. + */ +function buildFreshState(): InMemoryState { + return buildState(200, { project: 'alpha-v2', buzz: 'alpha-relaunch', oldSlug: 'alpha-v1' }); +} + +describe('swapInPlace', () => { + it('replaces every collection on the live state with the fresh contents', () => { + const live = buildLiveState(); + const fresh = buildFreshState(); + const keys = Object.keys(fresh) as (keyof InMemoryState)[]; + + // Sanity: the fixture must actually exercise every field, otherwise a + // skipped field would trivially "match". + expect(keys.length).toBeGreaterThan(0); + for (const key of keys) { + expect(fresh[key], `fresh.${key} is empty — extend the fixture`).not.toEqual(live[key]); + } + + swapInPlace(live, fresh); + + for (const key of keys) { + expect(live[key], `live.${key} was not replaced`).toEqual(fresh[key]); + } + // Also catch fields present on live but somehow absent on fresh. + expect(Object.keys(live).sort()).toEqual(keys.sort()); + }); + + it('preserves the identity of the state object and of every Map', () => { + const live = buildLiveState(); + const fresh = buildFreshState(); + const before = new Map( + (Object.keys(live) as (keyof InMemoryState)[]).map((k) => [k, live[k]]), + ); + + swapInPlace(live, fresh); + + for (const [key, map] of before) { + expect(live[key], `live.${key} Map identity changed`).toBe(map); + } + }); + + it('re-points the legacy-id, buzz-by-slug, and slug-history indices at the new records', () => { + const live = buildLiveState(); + const fresh = buildFreshState(); + const oldProjectId = uuid(101); + const newProjectId = uuid(201); + + expect(live.projectIdByLegacyId.get(42)).toBe(oldProjectId); + expect(live.buzzIdBySlug.get('alpha-launch')).toBe(uuid(103)); + expect(live.slugHistory.get(slugHistoryKey('project', 'alpha-v0'))?.newSlug).toBe('alpha-v1'); + + swapInPlace(live, fresh); + + // Legacy redirect path: legacyId → projectId → slug must resolve + // end-to-end against the new records. + expect(live.projectIdByLegacyId.get(42)).toBe(newProjectId); + expect(live.projectSlugById.get(live.projectIdByLegacyId.get(42) as string)).toBe('alpha-v2'); + + expect(live.buzzIdBySlug.get('alpha-launch')).toBeUndefined(); + expect(live.buzzIdBySlug.get('alpha-relaunch')).toBe(uuid(203)); + + expect(live.slugHistory.get(slugHistoryKey('project', 'alpha-v0'))).toBeUndefined(); + expect(live.slugHistory.get(slugHistoryKey('project', 'alpha-v1'))?.newSlug).toBe('alpha-v2'); + }); + + it('throws if a field on InMemoryState is not a Map instead of skipping it', () => { + const live = buildLiveState(); + const fresh = buildFreshState(); + (fresh as unknown as Record).someFutureIndex = new Set(['x']); + (live as unknown as Record).someFutureIndex = new Set(); + + expect(() => swapInPlace(live, fresh)).toThrow(/someFutureIndex/); + }); +}); diff --git a/plans/hot-reload-stale-indices.md b/plans/hot-reload-stale-indices.md new file mode 100644 index 0000000..799eeeb --- /dev/null +++ b/plans/hot-reload-stale-indices.md @@ -0,0 +1,129 @@ +--- +status: done +depends: [] +specs: + - specs/behaviors/storage.md + - specs/behaviors/legacy-id-mapping.md + - specs/behaviors/slug-handles.md +issues: [] +pr: 159 +--- + +# Plan: hot reload leaves three secondary indices stale + +## Scope + +`swapInPlace` in `apps/api/src/store/memory/reload.ts` replaces the +contents of every Map on `InMemoryState` by an explicit, hand-maintained +list of `replaceMapContents` calls. Three indices were never added to that +list: `projectIdByLegacyId`, `buzzIdBySlug`, and `slugHistory`. After the +hot-reload webhook (`POST /api/_internal/reload-data`) those three still +hold pre-reload contents. + +The user-visible consequence: the laddr importer mints fresh UUIDv7 ids on +every run, so a re-import merged into `published` followed by a hot reload +leaves `projectIdByLegacyId` pointing at project ids that no longer exist. +Legacy `/projects?ID=` and `/project-updates?ProjectID=` redirects +fall through to the SPA (404) until the pod restarts. Buzz-by-slug +(`/project-buzz/`) and slug-history 301s go stale the same way. + +In scope: + +- Spec: make the hot-reload atomicity rule say *every* collection is + replaced, naming the three indices that were missed. +- Fix `swapInPlace` so it cannot omit a field again. +- Tests that (a) enumerate every collection on a freshly built state and + assert the swap replaced it, and (b) drive the real webhook through a + re-import scenario and assert the legacy and slug-history redirects + follow the new records. + +Out of scope: anything about the reconcile state machine, the FTS reload, +or the push daemon. Those paths were not affected. + +## Implements + +- [behaviors/storage.md](../specs/behaviors/storage.md) — Hot reload → + Atomicity: every collection on the live state is replaced from the fresh + one, including legacy-id, buzz-by-slug, and slug-history indices. +- [behaviors/legacy-id-mapping.md](../specs/behaviors/legacy-id-mapping.md) + — legacy redirects resolve against current records after a reload. +- [behaviors/slug-handles.md](../specs/behaviors/slug-handles.md) — + slug-history redirects resolve against current records after a reload. + +## Approach + +1. **Spec first.** One sentence added to the Atomicity bullet of the + hot-reload section. +2. **Enumerate, don't list.** Replace the hand-maintained list in + `swapInPlace` with a loop over `Object.keys(fresh)`. Every own property + of `InMemoryState` is a Map today; the loop asserts that at runtime and + throws a descriptive error if a future field is something else, so a + new non-Map field fails loudly in the test suite rather than being + silently skipped. Nested `Set` values inside index Maps are copied by + reference from `fresh`, which is correct — `fresh` is discarded after + the swap and nothing else holds those Sets. +3. **Unit guard.** New `apps/api/tests/reload-swap.test.ts` builds two + `InMemoryState`s from different hand-crafted records (different ids, + legacy ids, slugs, slug-history entries), swaps, and for every own + property of the fresh state asserts `live[key]` deep-equals + `fresh[key]` while `live` keeps its object and Map identities. Also + checks that the three previously stale indices no longer resolve the + old values. +4. **Integration guard.** Extend `apps/api/tests/internal-reload.test.ts` + with a re-import scenario: seed a project carrying `legacyId`, boot, + confirm the legacy redirect; advance the remote by deleting that + record and writing a replacement with a fresh id and slug plus a + slug-history record; fire the webhook; assert the legacy redirect, + the buzz redirect, and the slug-history redirect all point at the new + slug. + +## Validation + +- [x] `specs/behaviors/storage.md` hot-reload Atomicity bullet names every + collection including legacy-id, buzz-by-slug, slug-history. +- [x] `swapInPlace` replaces every own property of `InMemoryState` without + an explicit per-field list. +- [x] Unit test enumerates every collection field of a fresh state and + asserts the swap replaced each one; fails on the pre-fix code. +- [x] Integration test: after re-import + webhook, `/projects?ID=`, + `/project-buzz/`, and old-slug URLs 301 to the new slug. +- [x] `npm run type-check && npm run lint && npm test` clean from repo root. + +## Risks / unknowns + +- **A future non-Map field on `InMemoryState`.** The enumerating swap + throws if it meets one. That is deliberate: the author of the new field + has to decide how it is swapped, and the unit test surfaces the + question immediately. +- **Concurrent branch touching `apps/api/src/notify/*`, `plugins/services.ts`, + `env.ts`.** This plan does not touch those files. + +## Notes + +- **Diagnosis confirmed as stated.** Diffing the Map-typed fields of + `InMemoryState` against the `replaceMapContents` calls showed exactly the + three missing: `projectIdByLegacyId`, `buzzIdBySlug`, `slugHistory`. All + three are plain Maps (slug-history values are `{ newSlug, expiresAt }` + objects, no nested Sets), so the same copy-by-reference swap is correct + for them. No Set-typed top-level fields exist. +- **The unit test failed 3/4 on the old code** (identity test passes + either way); the webhook re-import test failed at the post-reload legacy + redirect (404 instead of 301). Both verified by temporarily restoring the + pre-fix `reload.ts`. +- **Boot-order gap found along the way.** `store` opens the gitsheets + Sheet snapshots before `reconcile` fast-forwards, and `services` builds + the in-memory state from those stale snapshots. Only bites when the + local clone is behind at boot (dev, tests) — production pods clone fresh. + The re-import test works around it with an explicit + `git fetch origin main:main` before boot. Filed as #160. +- **Web test flakes under load.** `ProjectEdit` and `ExpressInterestModal` + timed out once while `npm test` ran concurrently with type-check + lint; + both pass on their own and on a quiet full `npm test -w apps/web` run. + Unrelated to this change (no `apps/web` files touched). + +## Follow-ups + +- Issue [#160](https://github.com/CodeForPhilly/codeforphilly-ng/issues/160) + — boot-time reconcile should re-open the store snapshot (or open the + store after reconcile) so a behind-at-boot clone doesn't build + in-memory state from the pre-fast-forward tree. diff --git a/specs/behaviors/storage.md b/specs/behaviors/storage.md index 2ff0edd..334668e 100644 --- a/specs/behaviors/storage.md +++ b/specs/behaviors/storage.md @@ -332,7 +332,7 @@ A push to the configured `CFP_DATA_BRANCH` from outside the API (typically a mer - **Reconcile + rebuild** — otherwise acquire the data-repo lock, call the same reconciliation state machine the boot path uses (`fastify.reconcileDataRepo`), and: - If outcome is `'in-sync'`, skip the rebuild and return 200 noChanges with the outcome. - Otherwise rebuild the in-memory state and FTS index from the new tree, then return 200 with the outcome, the old and new commit, and `rebuilt: true`. -- **Atomicity** — the rebuild constructs a fresh `InMemoryState` first; only after that succeeds does it mutate the live Maps in place. The FTS engine exposes a `reload(state)` that drops and re-inserts every FTS5 table. If the rebuild throws partway, the route returns 500 and the operator should restart the pod. +- **Atomicity** — the rebuild constructs a fresh `InMemoryState` first; only after that succeeds does it mutate the live Maps in place. **Every** collection on the live state is replaced from the fresh one — the primary entity maps and every secondary index, including the legacy-id, buzz-by-slug, and slug-history indices — so no lookup path can serve pre-reload contents after a reload. The FTS engine exposes a `reload(state)` that drops and re-inserts every FTS5 table. If the rebuild throws partway, the route returns 500 and the operator should restart the pod. - **Concurrency** — uses the same `dataRepoLock` as boot reconciliation, so a webhook fires can't race a `transact`-driven write. 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.