From 5dbde9ef51660b0aad33a0dd16e09ed42080dd20 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Tue, 8 Sep 2026 19:14:55 -0400 Subject: [PATCH 1/4] docs(specs): hot reload must rebuild every in-memory index The hot-reload section described the rebuild as "mutate the live Maps in place" without saying which Maps. The implementation had quietly skipped three secondary indices (legacy-id, buzz-by-slug, slug-history), which is exactly the gap an unqualified sentence leaves open. State the invariant explicitly: every collection on the live state, primary and secondary, is replaced from the fresh one, so no lookup path can serve pre-reload contents after a reload. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr --- specs/behaviors/storage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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. From 3e3c48ac0c5a15974e6ad3c17f02fc0dabeb3a47 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Tue, 8 Sep 2026 19:15:47 -0400 Subject: [PATCH 2/4] chore(plans): add hot-reload-stale-indices plan Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr --- plans/hot-reload-stale-indices.md | 106 ++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 plans/hot-reload-stale-indices.md diff --git a/plans/hot-reload-stale-indices.md b/plans/hot-reload-stale-indices.md new file mode 100644 index 0000000..63d94ba --- /dev/null +++ b/plans/hot-reload-stale-indices.md @@ -0,0 +1,106 @@ +--- +status: in-progress +depends: [] +specs: + - specs/behaviors/storage.md + - specs/behaviors/legacy-id-mapping.md + - specs/behaviors/slug-handles.md +issues: [] +--- + +# 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 + +- [ ] `specs/behaviors/storage.md` hot-reload Atomicity bullet names every + collection including legacy-id, buzz-by-slug, slug-history. +- [ ] `swapInPlace` replaces every own property of `InMemoryState` without + an explicit per-field list. +- [ ] Unit test enumerates every collection field of a fresh state and + asserts the swap replaced each one; fails on the pre-fix code. +- [ ] Integration test: after re-import + webhook, `/projects?ID=`, + `/project-buzz/`, and old-slug URLs 301 to the new slug. +- [ ] `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 + +(Populated at closeout.) + +## Follow-ups + +(Populated at closeout.) From 55c720f96962a1de4fb82503af65ae789b7a8f78 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Tue, 8 Sep 2026 19:31:16 -0400 Subject: [PATCH 3/4] fix(api): rebuild every in-memory index on hot reload swapInPlace named each Map on InMemoryState by hand and skipped three secondary indices: projectIdByLegacyId, buzzIdBySlug, and slugHistory. After POST /api/_internal/reload-data those three still described the pre-reload state. Because the laddr importer mints fresh UUIDv7 ids on every run, a re-import merged into `published` followed by a hot reload left projectIdByLegacyId pointing at project ids that no longer existed, so legacy /projects?ID= redirects fell through to the SPA until the pod restarted; /project-buzz/ and slug-history 301s went stale the same way. Enumerate the fresh state's own properties instead of maintaining a list, and throw if a property is ever not a Map so a future field is handled deliberately rather than skipped again. Guards against recurrence: - reload-swap.test.ts builds two states with one record of every entity type (different ids, same legacy ids), swaps, and asserts every own property of the fresh state was replaced while Map identities are kept. Fails on the old code for all three missing indices. - internal-reload.test.ts gains a re-import scenario through the real webhook: the project and buzz are replaced with fresh ids and slugs plus a slug-history record, and the legacy-id, buzz-slug, and old-slug redirects must all land on the new slug afterwards. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr --- apps/api/src/store/memory/reload.ts | 55 +++--- apps/api/tests/internal-reload.test.ts | 192 +++++++++++++++++-- apps/api/tests/reload-swap.test.ts | 247 +++++++++++++++++++++++++ 3 files changed, 444 insertions(+), 50 deletions(-) create mode 100644 apps/api/tests/reload-swap.test.ts 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/); + }); +}); From d7c5f9f094a5565e0bbd176164f7972bda3b193a Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Tue, 8 Sep 2026 19:33:48 -0400 Subject: [PATCH 4/4] chore(plans): mark hot-reload-stale-indices done (PR #159) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr --- plans/hot-reload-stale-indices.md | 39 ++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/plans/hot-reload-stale-indices.md b/plans/hot-reload-stale-indices.md index 63d94ba..799eeeb 100644 --- a/plans/hot-reload-stale-indices.md +++ b/plans/hot-reload-stale-indices.md @@ -1,11 +1,12 @@ --- -status: in-progress +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 @@ -78,15 +79,15 @@ or the push daemon. Those paths were not affected. ## Validation -- [ ] `specs/behaviors/storage.md` hot-reload Atomicity bullet names every +- [x] `specs/behaviors/storage.md` hot-reload Atomicity bullet names every collection including legacy-id, buzz-by-slug, slug-history. -- [ ] `swapInPlace` replaces every own property of `InMemoryState` without +- [x] `swapInPlace` replaces every own property of `InMemoryState` without an explicit per-field list. -- [ ] Unit test enumerates every collection field of a fresh state and +- [x] Unit test enumerates every collection field of a fresh state and asserts the swap replaced each one; fails on the pre-fix code. -- [ ] Integration test: after re-import + webhook, `/projects?ID=`, +- [x] Integration test: after re-import + webhook, `/projects?ID=`, `/project-buzz/`, and old-slug URLs 301 to the new slug. -- [ ] `npm run type-check && npm run lint && npm test` clean from repo root. +- [x] `npm run type-check && npm run lint && npm test` clean from repo root. ## Risks / unknowns @@ -99,8 +100,30 @@ or the push daemon. Those paths were not affected. ## Notes -(Populated at closeout.) +- **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 -(Populated at closeout.) +- 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.