fix: merge external manifest schema into runtime registrations#1106
fix: merge external manifest schema into runtime registrations#1106willgriffin merged 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves SMRT’s manifest/runtime hydration behavior so that when an external manifest entry is registered after a runtime class (including the same qualified key), schema/field/method metadata is merged into the existing registration—aiming to preserve STI/shared-table columns and tenancy/validation metadata during downstream “register.js → manifest discovery” flows.
Changes:
- Add
mergeManifestIntoExistingRegistration()and expandregisterFromManifest()to merge manifest fields/methods/schema/decorator config into existing registrations (including same qualified key). - Inject/normalize tenant-scoped config and ensure
tenantIdfield presence during merges. - Add a regression test covering schema merge into an already-registered external runtime class with the same qualified key; extend secrets tests around tenant-scoped audit failures.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/secrets/src/services/SecretService.ts | Passes tenantId into secret creation and audit log creation inputs. |
| packages/secrets/src/models/SecretAuditLog.ts | Updates createAuditEntry() typing and conditionally includes tenantId in the create input. |
| packages/secrets/src/tests/secret-service.test.ts | Adds a regression test asserting tenant-scoped audit failures persist even when tenancy interceptor is disabled; ensures tenancy cleanup. |
| packages/core/src/registry/class-registration.ts | Implements manifest-into-runtime merge logic (fields/methods/schema/config), tenancy normalization/injection, validation rebuild, and inheritance cache invalidation. |
| packages/core/src/tests/external-runtime-hydration.test.ts | Adds regression coverage for merging manifest schema/fields into an already-registered external runtime class with the same qualified key. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d952ac3b7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
d952ac3 to
5657c5c
Compare
Summary
register.jsthen manifest-discovery flow that was leavingdb:migrate/db:ci:verifywith base-only shared tablesVerification
pnpm --dir packages/core exec vitest run src/__tests__/external-runtime-hydration.test.ts src/__tests__/downstream-events-sti-schema.test.ts src/__tests__/issue-1102-ensure-schema-merged-sti.test.tspnpm --dir packages/core buildanytowndashboard consumer output showed merged shared-table columns on:events: includesagenda_url,council_idcontents: includesmeeting_id,council_idproperties: includesdatabase_url,database_name