feat(branches): declare tables into a branch through @table, ensureTable and defineTable - #2523
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements support for landing table declarations (via GraphQL @table, scope.ensureTable, and defineTable) directly into an application's private database branch rather than refusing them or misdirecting them to the base database. It introduces scopedTableFactory to route declarations dynamically, updates branch lifecycle management to track and clean up opened stores and tables, and implements cross-thread schema propagation for branches via ITC signals. Feedback was provided regarding a potential resource leak in openBranchDatabase where the catch block does not pass tables to closeBranchHandles on failure, which could leak expiration/eviction timers or reclamation handlers for partially initialized tables.
The failure path of openBranchDatabase closed the stores a partial open had created but never ran Table.cleanup on the classes built over them, leaving expiration and eviction timers and reclamation handlers alive for a branch that no longer exists. (Review finding on #2523.) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
CI note: the Unit Test failures on this PR are main's, not this change's. Main's own Unit Test run at its current head (38fcbf1) fails with the same MQTT — Claude Fable 5.1 |
…ble and defineTable (#2264) A branched application's `schema.graphql`, `scope.ensureTable()` and `defineTable()` all resolved through the process-global `table()` factory, so its declared tables would have landed in the base database while its code read and wrote the branch. Those paths were fenced (`branchGuard.ts`); this replaces the fence with real branch-scoped declaration. `table()` is now the global binding of `declareTable(target, definition)`: a `TableTarget` supplies the root store, the `tables` graph a class is published into, the reload after a lost create race, and who owns the column-family wrappers a declaration opens. The global target is the same code with the same objects behind it, so an unbranched application still receives `table` by identity. `scopedTableFactory(branches)` routes each declaration to the branch of the database it names, or to `table()` for a database the application did not branch; GraphQL, `ensureTable` and `defineTable` (via `defineTableUsing`) take that factory from the application scope. A branch's schema-change signal carries the branch path; a thread holding that branch open reloads its catalog (`reloadBranchAt`) instead of rescanning the global map. Branch classes announce to no global `updateTable` subscriber. Every wrapper a declaration opens is recorded on the branch so `close()` releases it, and table classes are cleaned up before their stores. The Table statics that resolve the global schema by logical name (`dropTable`, `addAttributes`) stay refused through a branch. Implemented by the dispatch dev-agent (task harper-2264); brought onto main after harper#2517. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The failure path of openBranchDatabase closed the stores a partial open had created but never ran Table.cleanup on the classes built over them, leaving expiration and eviction timers and reclamation handlers alive for a branch that no longer exists. (Review finding on #2523.) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
3671da5 to
a33222d
Compare
|
Reviewed; no blockers found. |
1 participant
Summary
Rebased this draft onto current
mainwhile retaining branch-scoped declarations:scopedTableFactoryroutes@table,scope.ensureTable(), anddefineTable()into an application's private branch without changing the unbranched factory identity. The integration proof exercises the exported route, scoped import, and base isolation for a GraphQL declaration in the branch-schema integration suite.Closes #2264. Part of #642.
For the human reviewer
reloadBranch: schema changes must remain private while another worker refreshes its branch catalog. Reverting this choice restores the former refusal of branch declarations; a branch-specific engine would be broader and duplicate lifecycle behavior.Verification
npm run build,npm run test:unit:main,npm run test:unit:resources, andnpm run test:integration:allpassed before the rebase push.integrationTests/components/branched-database-schema.test.tspassed; its route, scoped-import, and base-isolation assertions are linked above.a33222d47248; no code was changed after that review.Complexity: complicated
🤖 Rebase and review coverage refreshed by GPT-5 Codex.
Review-Coverage: authored=codex; ran=gemini,cursor-grok; adjudicated=domain; blocked=claude(exit-1); declined=cursor-composer; rounds=1 @ a33222d
Human-Review-Need: 4 (decisions: shared-declaration-engine, unbranched-name-fallback, declaration-only-evolution) @ a33222d