feat(family-mongo): add live introspection and wire CLI commands#334
Merged
feat(family-mongo): add live introspection and wire CLI commands#334
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughImplements runtime Mongo schema IR and visitor, live Mongo introspection, schema→view conversion, Mongo schema diffing, concrete Mongo control methods (verify/schemaVerify/sign/introspect) with marker persistence, CLI SQL-target/IR validation, and extensive unit and e2e tests and packaging/config updates. Changes
sequenceDiagram
participant CLI as CLI
participant Family as Mongo Family
participant Adapter as Mongo Adapter
participant DB as MongoDB
participant Diff as Schema Diff
CLI->>Family: verify(driver, contract, expectedTargetId)
Family->>Adapter: introspectSchema(db)
Adapter->>DB: listCollections(), listIndexes(), getOptions()
DB-->>Adapter: collections metadata
Adapter-->>Family: MongoSchemaIR
Family->>Family: compute storage/profile hashes
Family->>Adapter: readMarker / initMarker / updateMarker (CAS)
Adapter->>DB: marker read/write commands
DB-->>Adapter: marker result
Family->>Diff: diffMongoSchemas(live, expected, strict)
Diff-->>Family: root + issues + counts
Family-->>CLI: Verify/Sign/SchemaVerify result (ok / error codes)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/emitter
@prisma-next/migration-tools
@prisma-next/vite-plugin-contract-emit
@prisma-next/runtime-executor
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-pipeline-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
test/integration/test/cli.mongo-db-sign.e2e.test.ts (1)
11-12: Replace hardcoded startup timeout with sharedtimeoutshelpers.Using
60_000directly in integration tests drifts from the shared timeout policy and makes suite-wide tuning harder.As per coding guidelines: "Use shared timeout helpers from
@prisma-next/test-utilsin tests instead of hardcoded numeric timeout values."Also applies to: 65-66, 72-76, 84-84, 93-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli.mongo-db-sign.e2e.test.ts` around lines 11 - 12, Replace hardcoded numeric timeouts in this test with the shared helpers from `@prisma-next/test-utils`: remove MONGO_STARTUP_TIMEOUT and any other literal time values (e.g., the values referenced near lines 65-66, 72-76, 84, 93) and import/use the exported timeouts (e.g., timeouts.startup or the appropriate named constants) so the test uses the centralized timeout policy; update any uses of MONGO_STARTUP_TIMEOUT to the corresponding timeouts constant and adjust imports to include the timeouts helper.packages/2-mongo-family/9-family/test/schema-diff.test.ts (1)
25-30: UseifDefined()instead of inline conditional spreads in thecollhelper.This helper currently uses inline conditional spreads twice, which diverges from the repo’s conditional-spread pattern.
♻️ Suggested refactor
+import { ifDefined } from '@prisma-next/utils/defined'; function coll( name: string, opts?: { indexes?: MongoSchemaIndex[]; validator?: MongoSchemaValidator; options?: MongoSchemaCollectionOptions; }, ): MongoSchemaCollection { return new MongoSchemaCollection({ name, indexes: opts?.indexes ?? [], - ...(opts?.validator ? { validator: opts.validator } : {}), - ...(opts?.options ? { options: opts.options } : {}), + ...ifDefined('validator', opts?.validator), + ...ifDefined('options', opts?.options), }); }As per coding guidelines: "Use
ifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/9-family/test/schema-diff.test.ts` around lines 25 - 30, The coll helper currently uses inline conditional spreads when building the MongoSchemaCollection object; import ifDefined from "@prisma-next/utils/defined" and replace the two inline spread expressions with conditional spreads using ifDefined (e.g., ...ifDefined(opts?.validator, { validator: opts.validator }) and ...ifDefined(opts?.options, { options: opts.options })), keeping the existing name and indexes fields (name, indexes: opts?.indexes ?? []), then pass that resulting object into new MongoSchemaCollection.test/integration/test/cli.mongo-db-schema.e2e.test.ts (1)
22-24: Use shared test timeout helpers instead of60_000.Please switch this suite to
@prisma-next/test-utilstimeout constants to keep timeout behavior consistent across integration tests.As per coding guidelines: "Use shared timeout helpers from
@prisma-next/test-utilsin tests instead of hardcoded numeric timeout values."Also applies to: 31-35, 43-43, 50-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli.mongo-db-schema.e2e.test.ts` around lines 22 - 24, Replace the hardcoded MONGO_STARTUP_TIMEOUT and all other numeric timeouts in this test suite with the shared timeout helpers from `@prisma-next/test-utils`: import the appropriate timeout constant(s) at the top of the file and use them in place of MONGO_STARTUP_TIMEOUT (the const currently set to 60_000) and the other numeric values referenced around the describe block and subsequent tests (lines referenced in the review: 31–35, 43, 50–57). Specifically, remove the local MONGO_STARTUP_TIMEOUT constant, add the import (e.g., import { <sharedTimeoutConstant> } from '@prisma-next/test-utils'), and substitute that shared constant into the describe({ timeout: ... }) and any test() or before()/after() calls that currently use literal millisecond numbers so all tests use the shared timeouts.test/integration/test/mongo/db-schema.test.ts (1)
64-67: Use a single object matcher for related root fields.Line 64 and Line 65 check related properties of the same object; prefer one matcher for this pattern (and apply similarly to Line 91-92 and Line 149-150).
♻️ Proposed refactor
- expect(view.root.kind).toBe('root'); - expect(view.root.id).toBe('mongo-schema'); + expect(view.root).toMatchObject({ + kind: 'root', + id: 'mongo-schema', + }); expect(view.root.children).toHaveLength(2);As per coding guidelines, "Prefer object matchers (toMatchObject) over multiple individual expect().toBe() calls when checking 2 or more related values in tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/mongo/db-schema.test.ts` around lines 64 - 67, Replace multiple expectations that assert related properties on the same object with a single object matcher: instead of separate expect(view.root.kind).toBe('root') and expect(view.root.id).toBe('mongo-schema'), use expect(view.root).toMatchObject({ kind: 'root', id: 'mongo-schema' }) and leave the children length assertion as-is (expect(view.root.children).toHaveLength(2)); apply the same refactor to the other paired assertions referenced (the checks at the later locations corresponding to lines 91-92 and 149-150) by converting their paired property assertions into single toMatchObject calls on the parent object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/1-framework/3-tooling/cli/src/commands/contract-infer.ts`:
- Around line 61-63: The code blindly casts inspectResult.value.schema to
PslPrintableSqlSchemaIR and calls printPsl(), which can crash for non‑SQL IR;
instead, check the schema's family/kind before casting (e.g.,
inspectResult.value.schema.family === 'sql' or an equivalent discriminant), only
narrow to PslPrintableSqlSchemaIR when that check passes, and emit a clear CLI
error/exit if the schema is not SQL; update the logic around inspectResult,
schema, and the call site of printPsl() to use type guarding rather than an
unconditional cast and keep resolveContractInferOutputPath usage unchanged.
In `@packages/2-mongo-family/9-family/src/core/schema-diff.ts`:
- Around line 258-273: The branch handling extra validators (when
expected.validator is falsy and live.validator is truthy) currently returns a
verification node but does not add it to the issues array; update the block in
schema-diff.ts so it constructs the same verification object (using status
computed from strict, kind 'validator', code 'EXTRA_VALIDATOR', etc.), push that
object onto the issues array (the same array used by
extra_table/extra_index/extra_options handling), and then return it (or return
an array containing it) to keep behavior consistent with the other "extra_*"
branches; reference the symbols expected.validator, live.validator, strict,
issues, and the returned verification node when making the change.
In `@test/integration/test/cli.mongo-db-schema.e2e.test.ts`:
- Around line 83-90: Capture and assert the CLI process exit code when calling
executeCommand in this test (and the similar block at 122-129): store the
command result/exit code returned by executeCommand after calling
createDbSchemaCommand()/executeCommand (referencing createDbSchemaCommand,
executeCommand and testSetup) and add an assertion that the exit code equals the
expected value for each path (e.g., 0 for the successful case, the appropriate
non-zero code for failure cases) before restoring the original cwd.
In `@test/integration/test/cli.mongo-db-verify.e2e.test.ts`:
- Around line 97-108: The teardown can crash if beforeEach fails and
cleanupMocks remains undefined; initialize cleanupMocks defensively (e.g., set
cleanupMocks = () => {} at declaration) or guard the afterEach call so it only
invokes cleanupMocks if defined; locate the symbols cleanupMocks, beforeEach,
afterEach and setupCommandMocks in the test file and ensure cleanupMocks has a
safe default or add a null-check in afterEach before calling it.
- Line 11: Replace the hardcoded MONGO_STARTUP_TIMEOUT numeric constant with the
shared timeout helper from `@prisma-next/test-utils`: add the appropriate import
for the shared timeout (e.g., TEST_TIMEOUTS or DEFAULT_TEST_TIMEOUT) at the top
of the test file and replace the MONGO_STARTUP_TIMEOUT usage with that imported
constant (remove the MONGO_STARTUP_TIMEOUT declaration). Ensure the test now
references the shared symbol instead of the raw 60_000 value.
---
Nitpick comments:
In `@packages/2-mongo-family/9-family/test/schema-diff.test.ts`:
- Around line 25-30: The coll helper currently uses inline conditional spreads
when building the MongoSchemaCollection object; import ifDefined from
"@prisma-next/utils/defined" and replace the two inline spread expressions with
conditional spreads using ifDefined (e.g., ...ifDefined(opts?.validator, {
validator: opts.validator }) and ...ifDefined(opts?.options, { options:
opts.options })), keeping the existing name and indexes fields (name, indexes:
opts?.indexes ?? []), then pass that resulting object into new
MongoSchemaCollection.
In `@test/integration/test/cli.mongo-db-schema.e2e.test.ts`:
- Around line 22-24: Replace the hardcoded MONGO_STARTUP_TIMEOUT and all other
numeric timeouts in this test suite with the shared timeout helpers from
`@prisma-next/test-utils`: import the appropriate timeout constant(s) at the top
of the file and use them in place of MONGO_STARTUP_TIMEOUT (the const currently
set to 60_000) and the other numeric values referenced around the describe block
and subsequent tests (lines referenced in the review: 31–35, 43, 50–57).
Specifically, remove the local MONGO_STARTUP_TIMEOUT constant, add the import
(e.g., import { <sharedTimeoutConstant> } from '@prisma-next/test-utils'), and
substitute that shared constant into the describe({ timeout: ... }) and any
test() or before()/after() calls that currently use literal millisecond numbers
so all tests use the shared timeouts.
In `@test/integration/test/cli.mongo-db-sign.e2e.test.ts`:
- Around line 11-12: Replace hardcoded numeric timeouts in this test with the
shared helpers from `@prisma-next/test-utils`: remove MONGO_STARTUP_TIMEOUT and
any other literal time values (e.g., the values referenced near lines 65-66,
72-76, 84, 93) and import/use the exported timeouts (e.g., timeouts.startup or
the appropriate named constants) so the test uses the centralized timeout
policy; update any uses of MONGO_STARTUP_TIMEOUT to the corresponding timeouts
constant and adjust imports to include the timeouts helper.
In `@test/integration/test/mongo/db-schema.test.ts`:
- Around line 64-67: Replace multiple expectations that assert related
properties on the same object with a single object matcher: instead of separate
expect(view.root.kind).toBe('root') and
expect(view.root.id).toBe('mongo-schema'), use expect(view.root).toMatchObject({
kind: 'root', id: 'mongo-schema' }) and leave the children length assertion
as-is (expect(view.root.children).toHaveLength(2)); apply the same refactor to
the other paired assertions referenced (the checks at the later locations
corresponding to lines 91-92 and 149-150) by converting their paired property
assertions into single toMatchObject calls on the parent object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 5f64525b-1d0a-428a-82ad-2f0217630622
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/mongo-schema-migrations/plans/m4-online-cli-commands-plan.mdis excluded by!projects/**
📒 Files selected for processing (18)
packages/1-framework/3-tooling/cli/src/commands/contract-infer.tspackages/1-framework/3-tooling/cli/src/commands/inspect-live-schema.tspackages/1-framework/3-tooling/cli/test/commands/inspect-live-schema.test.tspackages/2-mongo-family/9-family/package.jsonpackages/2-mongo-family/9-family/src/core/control-instance.tspackages/2-mongo-family/9-family/src/core/schema-diff.tspackages/2-mongo-family/9-family/test/control.test.tspackages/2-mongo-family/9-family/test/schema-diff.test.tspackages/3-mongo-target/2-mongo-adapter/src/core/introspect-schema.tspackages/3-mongo-target/2-mongo-adapter/src/exports/control.tspackages/3-mongo-target/2-mongo-adapter/test/introspect-schema.test.tstest/integration/test/cli.mongo-db-schema.e2e.test.tstest/integration/test/cli.mongo-db-sign.e2e.test.tstest/integration/test/cli.mongo-db-verify.e2e.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/mongo-db-commands/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/package.jsontest/integration/test/mongo/db-schema.test.tstest/integration/test/mongo/db-verify-sign.test.ts
Break down the implementation of Mongo live introspection and CLI command wiring into 4 milestones with 20 tasks covering: introspect + db schema, db verify + db sign, db init + db update, and migration status/show. Introduces decoupled view interface design (prototype in Mongo, extract later) and documents SQL coupling as tech debt.
Add introspectSchema() that reads collections, indexes, validators, and collection options from a live MongoDB instance and produces a MongoSchemaIR — symmetric to contractToMongoSchemaIR. Filters out the default _id_ index, system collections, views, and the _prisma_migrations collection.
Replace the introspect() stub with a real implementation that delegates to introspectSchema(db). Fix method signatures on verify/schemaVerify/sign stubs to match the ControlFamilyInstance interface. Update MongoControlFamilyInstance to use the MongoSchemaIR type parameter.
Map MongoSchemaIR to CoreSchemaView tree for CLI visualization. Collections become collection nodes with index, validator, and collection options as children. Implements SchemaViewCapable so the CLI detects it via hasSchemaView().
Change InspectLiveSchemaResult.schema from PslPrintableSqlSchemaIR to unknown. Only call validatePrintableSqlSchemaIR when the family is SQL; for other families pass the raw schema IR through. This unblocks db schema for MongoDB.
Verifies the full introspect -> toSchemaView -> tree output flow against a real MongoMemoryReplSet. Covers collections with indexes, validators, capped collections, empty databases, and JSON output.
Read the contract marker from MongoDB and compare storage/profile hashes against the contract. Returns PN-RUN-3001 (missing marker), PN-RUN-3002 (hash mismatch), PN-RUN-3003 (target mismatch), or ok when the database matches the contract.
Introspect the live MongoDB, convert the contract to expected MongoSchemaIR, and diff them to produce a SchemaVerificationNode tree and SchemaIssue list. Detects missing/extra collections, missing/extra indexes, validator mismatches, and collection options drift. Strict mode reports extra elements as fail instead of warn.
Validate the contract, read the existing marker, and write/update it via the adapter marker-ledger helpers. Uses compare-and-swap (CAS) for atomic marker updates to prevent concurrent modification. Returns created/updated/no-op status with previous hashes.
Cover all mode combinations: marker-only verify (missing, match, hash mismatch, profile mismatch), schema verify (match, missing index, extra index strict/non-strict, missing collection), and sign (create, idempotent, update with CAS).
…f.ts Move diffMongoSchemas and its helpers (diffIndexes, diffValidator, diffOptions, buildIndexLookupKey, formatIndexName) into a dedicated schema-diff.ts module. This reduces control-instance.ts from ~780 to ~420 lines and enables targeted unit testing of the diffing algorithm. Also threads strict mode through diffValidator and diffOptions so extra validators and extra options respect strict mode (fail instead of warn), matching SQL behavior where strict mode applies to all element types.
- Remove unused MIGRATIONS_COLLECTION and MARKER_ID constants - Delegate readMarker() to the adapter instead of duplicating logic - Remove redundant ?? undefined pattern in parseIndex - Move target mismatch check before marker check in verify() so configuration errors surface independently of database state
…ent types Tests cover: empty schemas, matching/missing/extra collections, matching/missing/extra indexes, validator match/mismatch/missing/extra (with validationLevel and validationAction variants), and options match/mismatch/extra. Both strict and non-strict modes are exercised for all "extra" element types.
Verify that non-SQL families (e.g. mongo) pass schema IR through without validatePrintableSqlSchemaIR. Uses a mock config with familyId "mongo" and asserts the raw schema IR is returned as-is.
Exercise the full CLI entry point for Mongo: - db schema: JSON output with collections/indexes, tree output - db verify: marker missing, matching marker+schema, schema-only (pass/fail), marker-only - db sign: create marker, idempotent re-sign, update on hash change Infrastructure: add Mongo workspace deps to cli-e2e-test-app fixture, create mongo-db-commands fixture with Mongo family config. Contract JSON is written directly to disk (no emit pipeline needed since db verify/sign read contract.json from disk).
Replace non-null assertions with narrowed variable bindings to satisfy biome noNonNullAssertion rule. Fix SchemaIssue type error where expected was set to null instead of being omitted (expected is string | undefined, not nullable).
wmadden
force-pushed
the
tml-2233-m4-online-cli-commands-live-introspection
branch
from
8f3041d to
33322e1
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/2-mongo-family/9-family/src/core/schema-diff.ts (1)
260-275:⚠️ Potential issue | 🟡 MinorMissing issue for extra validator.
When an extra validator is found (live has validator, expected doesn't), the code returns a verification node but doesn't push to the
issuesarray. This is inconsistent with howextra_table(line 59-63),extra_index(lines 207-212), andextra_options(lines 343-348) are handled.🐛 Proposed fix to add missing issue
if (!expected.validator && live.validator) { const status = strict ? 'fail' : 'warn'; + issues.push({ + kind: 'extra_validator', + table: collName, + message: `Extra validator on collection "${collName}"`, + }); return [ { status,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/9-family/src/core/schema-diff.ts` around lines 260 - 275, The extra validator branch in schema-diff.ts currently returns a verification node but does not append it to the issues array; update the block that checks "if (!expected.validator && live.validator)" to both create the verification node (with status set from strict, kind 'validator', code 'EXTRA_VALIDATOR', actual using canonicalize(live.validator.jsonSchema), etc.) and push that node onto the issues array (same pattern used for extra_table, extra_index, extra_options), then return the node (or the array) as currently expected so behavior matches other "extra_*" handlers; reference the variables expected.validator, live.validator, canonicalize, strict, and the issues array when making the change.
🧹 Nitpick comments (5)
packages/1-framework/3-tooling/cli/src/commands/inspect-live-schema.ts (1)
129-130: Move family-specific schema validation behind the control adapter/family API.Branching on
config.family.familyIdhere leaks family-specific behavior into command orchestration. Prefer delegating this normalization/validation step to the family/control layer.♻️ Suggested refactor direction
- const schema = - config.family.familyId === 'sql' ? validatePrintableSqlSchemaIR(schemaIR) : schemaIR; + const schema = client.normalizeIntrospectedSchema(schemaIR);As per coding guidelines, “Use adapters instead of branching on target. Reference
.cursor/rules/no-target-branches.mdcfor details.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/src/commands/inspect-live-schema.ts` around lines 129 - 130, The code currently branches on config.family.familyId to call validatePrintableSqlSchemaIR(schemaIR); move this family-specific validation into the family/control adapter API instead of branching in inspect-live-schema.ts. Replace the inline branch around validatePrintableSqlSchemaIR and schemaIR with a single call to a family-provided method (e.g., config.family.normalizeOrValidateSchema(schemaIR) or via the control adapter), and implement the SQL-specific logic (validatePrintableSqlSchemaIR) inside the SQL family adapter so inspect-live-schema.ts no longer references config.family.familyId or validatePrintableSqlSchemaIR directly.packages/2-mongo-family/9-family/src/core/schema-diff.ts (1)
339-362: Missing handling for expected options without live options.The
diffOptionsfunction handles:
- Both undefined (line 339)
- Live has options but expected doesn't (lines 341-362)
- Both have options with deepEqual check (lines 364-378)
- Options mismatch (lines 380-399)
However, it doesn't explicitly handle the case where
expected.optionsexists butlive.optionsis undefined (missing options). Currently this falls through to the deepEqual check which would compareundefinedwith an object, correctly producing a mismatch. This works but could be made more explicit with a dedicatedMISSING_OPTIONScode for clarity and consistency withMISSING_VALIDATOR.♻️ Optional refactor for explicit handling
if (!live.options && !expected.options) return []; + if (expected.options && !live.options) { + issues.push({ + kind: 'type_missing', + table: collName, + message: `Collection options missing on "${collName}"`, + }); + return [ + { + status: 'fail', + kind: 'options', + name: 'options', + contractPath: `storage.collections.${collName}.options`, + code: 'MISSING_OPTIONS', + message: 'Collection options missing', + expected: expected.options, + actual: null, + children: [], + }, + ]; + } + if (!expected.options && live.options) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/9-family/src/core/schema-diff.ts` around lines 339 - 362, The diffOptions branch is missing explicit handling when expected.options exists but live.options is undefined; update the diffOptions logic (around collName, issues, strict) to add a branch for if (expected.options && !live.options) that pushes a type_mismatch issue with actual: canonicalize(live.options) and returns an action object mirroring the EXTRA_OPTIONS case but with code 'MISSING_OPTIONS', expected: expected.options, actual: null, an appropriate message like 'Missing collection options found', status set to 'fail' when strict else 'warn', and children: [] so it parallels MISSING_VALIDATOR/EXTRA_OPTIONS handling.packages/3-mongo-target/2-mongo-adapter/test/introspect-schema.test.ts (1)
13-16: Remove unused imports.
MongoInspectionExecutor,contractToMongoSchemaIR, andMongoMigrationRunnerare imported but never used in this test file.🧹 Proposed cleanup
-import { MongoInspectionExecutor } from '../src/core/command-executor'; -import { contractToMongoSchemaIR } from '../src/core/contract-to-schema'; import { introspectSchema } from '../src/core/introspect-schema'; -import { MongoMigrationRunner } from '../src/core/mongo-runner';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/2-mongo-adapter/test/introspect-schema.test.ts` around lines 13 - 16, Remove the unused imports from the test file: delete the imports of MongoInspectionExecutor, contractToMongoSchemaIR, and MongoMigrationRunner so only the used introspectSchema import remains; update the import statement(s) in packages/3-mongo-target/2-mongo-adapter/test/introspect-schema.test.ts to remove references to those three symbols to avoid unused-import lint errors.packages/3-mongo-target/2-mongo-adapter/src/core/introspect-schema.ts (2)
13-19: Consider type narrowing for index key direction.The cast
direction as MongoIndexKeyDirectionassumes MongoDB always returns valid direction values. While this is generally true for indexes created through standard means, corrupted or manually manipulated metadata could contain unexpected values.For defense-in-depth, consider validating the direction value:
🛡️ Suggested validation
function parseIndexKeys(keySpec: Record<string, unknown>): MongoIndexKey[] { const keys: MongoIndexKey[] = []; for (const [field, direction] of Object.entries(keySpec)) { - keys.push({ field, direction: direction as MongoIndexKeyDirection }); + // MongoDB index directions: 1, -1, 'text', '2dsphere', '2d', 'hashed' + const validDirections = [1, -1, 'text', '2dsphere', '2d', 'hashed']; + const dir = validDirections.includes(direction as MongoIndexKeyDirection) + ? (direction as MongoIndexKeyDirection) + : 1; // fallback to ascending + keys.push({ field, direction: dir }); } return keys; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/2-mongo-adapter/src/core/introspect-schema.ts` around lines 13 - 19, The parseIndexKeys function currently casts direction to MongoIndexKeyDirection without validation; update parseIndexKeys to narrow and validate the direction value from Object.entries(keySpec) before pushing: accept known numeric directions (1, -1) and any project-specific string aliases (e.g., "asc"/"desc") mapping them to MongoIndexKeyDirection, otherwise either skip the key or throw/log an error; reference parseIndexKeys, MongoIndexKey, and MongoIndexKeyDirection so you can add a small guard (if/else or switch) that converts/validates direction and only pushes a MongoIndexKey when the value is a valid MongoIndexKeyDirection, providing a safe fallback behavior.
78-84: Consider usingifDefined()for conditional spreads.The inline conditional spread pattern could be simplified using the
ifDefined()helper from@prisma-next/utils/defined, which is the preferred pattern per coding guidelines.♻️ Optional refactor
+import { ifDefined } from '@prisma-next/utils/defined'; + // In parseCollectionOptions: - return new MongoSchemaCollectionOptions({ - ...(capped ? { capped: { size: size ?? 0, ...(max != null ? { max } : {}) } } : {}), - ...(timeseries ? { timeseries } : {}), - ...(collation ? { collation } : {}), - ...(changeStreamPreAndPostImages ? { changeStreamPreAndPostImages } : {}), - ...(clusteredIndex ? { clusteredIndex } : {}), - }); + return new MongoSchemaCollectionOptions({ + ...ifDefined(capped, () => ({ capped: { size: size ?? 0, ...ifDefined(max, () => ({ max })) } })), + ...ifDefined(timeseries, () => ({ timeseries })), + ...ifDefined(collation, () => ({ collation })), + ...ifDefined(changeStreamPreAndPostImages, () => ({ changeStreamPreAndPostImages })), + ...ifDefined(clusteredIndex, () => ({ clusteredIndex })), + });As per coding guidelines: "Use
ifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/3-mongo-target/2-mongo-adapter/src/core/introspect-schema.ts` around lines 78 - 84, Replace the inline conditional object spreads inside the MongoSchemaCollectionOptions constructor with the ifDefined() helper from `@prisma-next/utils/defined`: import ifDefined and wrap each optional property (capped, timeseries, collation, changeStreamPreAndPostImages, clusteredIndex) so you pass only defined values to the object passed to new MongoSchemaCollectionOptions; ensure the capped object still builds size and max correctly and use ifDefined(() => ({ capped: ... })) (or equivalent) for each conditional to match existing shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/2-mongo-family/9-family/src/core/schema-diff.ts`:
- Around line 260-275: The extra validator branch in schema-diff.ts currently
returns a verification node but does not append it to the issues array; update
the block that checks "if (!expected.validator && live.validator)" to both
create the verification node (with status set from strict, kind 'validator',
code 'EXTRA_VALIDATOR', actual using canonicalize(live.validator.jsonSchema),
etc.) and push that node onto the issues array (same pattern used for
extra_table, extra_index, extra_options), then return the node (or the array) as
currently expected so behavior matches other "extra_*" handlers; reference the
variables expected.validator, live.validator, canonicalize, strict, and the
issues array when making the change.
---
Nitpick comments:
In `@packages/1-framework/3-tooling/cli/src/commands/inspect-live-schema.ts`:
- Around line 129-130: The code currently branches on config.family.familyId to
call validatePrintableSqlSchemaIR(schemaIR); move this family-specific
validation into the family/control adapter API instead of branching in
inspect-live-schema.ts. Replace the inline branch around
validatePrintableSqlSchemaIR and schemaIR with a single call to a
family-provided method (e.g., config.family.normalizeOrValidateSchema(schemaIR)
or via the control adapter), and implement the SQL-specific logic
(validatePrintableSqlSchemaIR) inside the SQL family adapter so
inspect-live-schema.ts no longer references config.family.familyId or
validatePrintableSqlSchemaIR directly.
In `@packages/2-mongo-family/9-family/src/core/schema-diff.ts`:
- Around line 339-362: The diffOptions branch is missing explicit handling when
expected.options exists but live.options is undefined; update the diffOptions
logic (around collName, issues, strict) to add a branch for if (expected.options
&& !live.options) that pushes a type_mismatch issue with actual:
canonicalize(live.options) and returns an action object mirroring the
EXTRA_OPTIONS case but with code 'MISSING_OPTIONS', expected: expected.options,
actual: null, an appropriate message like 'Missing collection options found',
status set to 'fail' when strict else 'warn', and children: [] so it parallels
MISSING_VALIDATOR/EXTRA_OPTIONS handling.
In `@packages/3-mongo-target/2-mongo-adapter/src/core/introspect-schema.ts`:
- Around line 13-19: The parseIndexKeys function currently casts direction to
MongoIndexKeyDirection without validation; update parseIndexKeys to narrow and
validate the direction value from Object.entries(keySpec) before pushing: accept
known numeric directions (1, -1) and any project-specific string aliases (e.g.,
"asc"/"desc") mapping them to MongoIndexKeyDirection, otherwise either skip the
key or throw/log an error; reference parseIndexKeys, MongoIndexKey, and
MongoIndexKeyDirection so you can add a small guard (if/else or switch) that
converts/validates direction and only pushes a MongoIndexKey when the value is a
valid MongoIndexKeyDirection, providing a safe fallback behavior.
- Around line 78-84: Replace the inline conditional object spreads inside the
MongoSchemaCollectionOptions constructor with the ifDefined() helper from
`@prisma-next/utils/defined`: import ifDefined and wrap each optional property
(capped, timeseries, collation, changeStreamPreAndPostImages, clusteredIndex) so
you pass only defined values to the object passed to new
MongoSchemaCollectionOptions; ensure the capped object still builds size and max
correctly and use ifDefined(() => ({ capped: ... })) (or equivalent) for each
conditional to match existing shape.
In `@packages/3-mongo-target/2-mongo-adapter/test/introspect-schema.test.ts`:
- Around line 13-16: Remove the unused imports from the test file: delete the
imports of MongoInspectionExecutor, contractToMongoSchemaIR, and
MongoMigrationRunner so only the used introspectSchema import remains; update
the import statement(s) in
packages/3-mongo-target/2-mongo-adapter/test/introspect-schema.test.ts to remove
references to those three symbols to avoid unused-import lint errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: f61d6259-44ef-4906-bc36-3c6f368595b5
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/mongo-schema-migrations/plans/m4-online-cli-commands-plan.mdis excluded by!projects/**
📒 Files selected for processing (18)
packages/1-framework/3-tooling/cli/src/commands/contract-infer.tspackages/1-framework/3-tooling/cli/src/commands/inspect-live-schema.tspackages/1-framework/3-tooling/cli/test/commands/inspect-live-schema.test.tspackages/2-mongo-family/9-family/package.jsonpackages/2-mongo-family/9-family/src/core/control-instance.tspackages/2-mongo-family/9-family/src/core/schema-diff.tspackages/2-mongo-family/9-family/test/control.test.tspackages/2-mongo-family/9-family/test/schema-diff.test.tspackages/3-mongo-target/2-mongo-adapter/src/core/introspect-schema.tspackages/3-mongo-target/2-mongo-adapter/src/exports/control.tspackages/3-mongo-target/2-mongo-adapter/test/introspect-schema.test.tstest/integration/test/cli.mongo-db-schema.e2e.test.tstest/integration/test/cli.mongo-db-sign.e2e.test.tstest/integration/test/cli.mongo-db-verify.e2e.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/mongo-db-commands/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/package.jsontest/integration/test/mongo/db-schema.test.tstest/integration/test/mongo/db-verify-sign.test.ts
✅ Files skipped from review due to trivial changes (7)
- packages/2-mongo-family/9-family/package.json
- packages/1-framework/3-tooling/cli/test/commands/inspect-live-schema.test.ts
- packages/3-mongo-target/2-mongo-adapter/src/exports/control.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/mongo-db-commands/prisma-next.config.with-db.ts
- test/integration/test/mongo/db-schema.test.ts
- test/integration/test/cli.mongo-db-verify.e2e.test.ts
- packages/2-mongo-family/9-family/test/schema-diff.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/1-framework/3-tooling/cli/src/commands/contract-infer.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/package.json
- test/integration/test/cli.mongo-db-sign.e2e.test.ts
- test/integration/test/cli.mongo-db-schema.e2e.test.ts
- packages/2-mongo-family/9-family/src/core/control-instance.ts
The db schema command shows the live database schema, not the contract. Both Mongo and SQL families had the root tree node labeled "contract" — corrected to "database".
- Index directions: omit ascending (default), show desc/text/hashed/etc. - Validator label: use explicit keys (level: strict, action: error) - Validator subtree: expand jsonSchema properties as child nodes showing field name, bsonType, and (required) annotation
SchemaViewCapable<TSchemaIR = unknown> was always consumed as SchemaViewCapable<unknown> since hasSchemaView never propagated the concrete type. Remove the type param from SchemaViewCapable and make hasSchemaView generic over ControlFamilyInstance so it preserves TSchemaIR when narrowing.
…ror reporting The Mongo planner was not including profileHash in the migration plan destination, causing the runner to fall back to storageHash. This meant the database marker profileHash was always set to storageHash after migration, causing db verify to report a hash mismatch. Additionally, the CLI db verify command only reported storageHash in its error metadata for PN-RUN-3002, masking profileHash mismatches and producing confusing output where expected/actual were identical.
Per ADR 004, profileHash is for capability profile drift and is irrelevant to migration planning (which uses storageHash only). Revert adding profileHash to the planner destination. Instead, the runner reads profileHash from the destinationContract when writing the marker. Remove the storageHash fallback that silently corrupted the marker profileHash. The sign method also no longer falls back — it reads profileHash directly from the validated contract (which always has it).
MongoSchemaIR was the only plain interface in its own AST package — it did not extend MongoSchemaNode, had no kind, no visitor dispatch, no freeze, and used a Record instead of holding children as nodes. Convert it to a class extending MongoSchemaNode with kind "schema", sorted children array, visitor participation, and a collection(name) lookup helper. Update all construction sites (introspectSchema, contractToMongoSchemaIR) and consumers (diffMongoSchemas, toSchemaView, MongoMigrationPlanner, and all tests) to use the class API.
The MongoSchemaIR → CoreSchemaView transformation is a substantial piece of logic (~100 lines) that was embedded in the family instance class. Extracting it to a dedicated module makes the transformation independently testable and separates the labeling/ordering/expansion concerns from the family instance lifecycle/orchestration.
SchemaTreeNode was a plain data interface that families constructed as
object literals. This promoted it to a concrete class with constructor
validation, Object.freeze() for immutability, and a visitor interface
(SchemaTreeVisitor<R>) with accept() method — following the same AST
pattern established by MongoSchemaNode. All family construction sites
(Mongo schema-to-view, SQL toSchemaView) and test fixtures now use
new SchemaTreeNode({...}) instead of object literals.
SchemaViewCapable<TSchemaIR> now flows the concrete schema IR type through the interface, so families get compile-time verification that their toSchemaView method signature matches the interface contract. Both MongoControlFamilyInstance and SqlControlFamilyInstance now explicitly extend SchemaViewCapable with their concrete IR type, fixing the Liskov substitution violation where the concrete methods narrowed the input type without declaring the interface.
Use ifDefined from @prisma-next/utils/defined instead of verbose
...(x ? { x } : {}) patterns in control-instance.ts and
schema-to-view.ts. Adds @prisma-next/utils as a dependency of
the mongo family package.
Define VERIFY_CODE_MARKER_MISSING, VERIFY_CODE_HASH_MISMATCH, VERIFY_CODE_TARGET_MISMATCH, and VERIFY_CODE_SCHEMA_FAILURE in framework-components and use them in Mongo family, SQL family, and CLI instead of inline string literals.
Defers F19 (raw driver calls + layer concern) to a follow-up ticket. The refactoring requires adding RawFindOneCommand to the query AST and extending the command executor.
Remove unused imports from test file and narrow CollectionInfo options access to handle the union type correctly.
… aggregate marker-ledger is deployment infrastructure state, not domain operation bridging — it belongs in the driver layer. Also replaces the obsolete findOne call with an aggregate pipeline ($match + $limit), consistent with the modern MongoDB command surface. Adapter re-exports from driver for backward compatibility. Runner tests updated to mock the driver module. TML-2253 cancelled (no longer needed).
… calls Use RawAggregateCommand, RawInsertOneCommand, and RawFindOneAndUpdateCommand from @prisma-next/mongo-query-ast instead of calling db.collection() methods directly. Small executor functions in the driver bridge AST commands to the Db object.
marker-ledger is deployment infrastructure state — it belongs in @prisma-next/target-mongo, not in the driver (connectivity/execution) or adapter (domain bridging). Added control export to the target package. All consumers updated to import from @prisma-next/target-mongo/control.
…e explicit type args The default silently erased type safety. Framework-level consumers now pass unknown explicitly; concrete families supply their IR type.
- Add missing issues.push() in extra validator branch of diffValidator - Add extra_validator to SchemaIssue kind union - Guard contract-infer to SQL families with structured error - Replace hardcoded 60_000 timeouts with timeouts.spinUpDbServer - Initialize cleanupMocks defensively in all e2e tests
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/1-framework/3-tooling/cli/test/output.test.ts (1)
1-1037: Consider splitting this test file.At ~1037 lines, this file exceeds the 500-line guideline. Consider splitting by functionality (e.g.,
output.introspect.test.ts,output.schema-verify.test.ts,output.sign.test.ts) to improve navigability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/test/output.test.ts` around lines 1 - 1037, The test file is too large and should be split into focused test files to improve navigability; create separate test files for introspection, schema verification, and signing (e.g., tests that exercise formatIntrospectOutput/formatIntrospectJson and createSchemaView; tests for formatSchemaVerifyOutput/formatSchemaVerifyJson and createVerificationNode/createResult; tests for formatSignOutput/formatSignJson and createSignResult). Move any shared helpers (like parseGlobalFlags usage or stripAnsi import) into a small shared test helper module if needed, update each new test file to import only the functions it needs (formatIntrospectOutput, formatIntrospectJson, formatSchemaVerifyOutput, formatSchemaVerifyJson, formatSignOutput, formatSignJson, parseGlobalFlags, SchemaTreeNode), and ensure each describe block from the original file is preserved in the appropriate new file so tests and helper factory functions (createSchemaView, createResult, createVerificationNode, createSignResult) remain available and referenced correctly.test/integration/test/cli.mongo-db-sign.e2e.test.ts (1)
48-56: Consider edge case handling in extractJson.The current implementation finds the first
{and last}in the output. This works for single JSON objects but could misbehave if:
- CLI output contains multiple JSON objects
- Braces appear in non-JSON output before/after the actual JSON
For CLI
--jsonmode this is likely fine, but consider documenting this assumption or using a more robust approach if issues arise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli.mongo-db-sign.e2e.test.ts` around lines 48 - 56, The extractJson function currently slices from the first '{' to the last '}', which can break if there are multiple JSON objects or stray braces; update extractJson to locate the first '{' then scan forward tracking brace depth (increment on '{', decrement on '}') to find the matching closing brace and parse only that balanced substring, throwing a clear error if no matching closing brace is found; reference the extractJson function so you can replace the current indexOf/lastIndexOf logic with this brace-counting loop to make the extraction robust (or alternatively add a brief comment documenting the single-JSON assumption if you choose not to change behavior).packages/1-framework/3-tooling/cli/src/commands/db-verify.ts (1)
69-89: Potential logic issue: unreachable or confusing ternary condition.When
storageMatchistrue(line 75 check passes, no early return), the code falls through to lines 83-89. At this point, if the verification code isVERIFY_CODE_HASH_MISMATCHbutstorageMatchis true, thenprofileMatchmust be false (otherwise verification wouldn't have failed). The ternaryprofileMatch ? 'Contract hash does not match...' : 'Contract profileHash does not match...'at lines 84-86 will always take the false branch.Consider simplifying:
Suggested simplification
return errorHashMismatch({ - why: profileMatch - ? 'Contract hash does not match database marker' - : 'Contract profileHash does not match database marker', + why: 'Contract profileHash does not match database marker', expected: verifyResult.contract.profileHash, ...ifDefined('actual', verifyResult.marker?.profileHash), });Alternatively, if the generic message is intentional for edge cases (e.g., future mismatch types), add a comment explaining when
profileMatchcould be true here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/src/commands/db-verify.ts` around lines 69 - 89, The ternary using profileMatch inside the VERIFY_CODE_HASH_MISMATCH branch is confusing/unreachable: after the storageMatch early return, profileMatch will always be false here, so simplify the logic in the block that calls errorHashMismatch for profile mismatches (the code that computes profileMatch and returns errorHashMismatch) — replace the ternary with a single clear message like "Contract profileHash does not match database marker" (and keep expected: verifyResult.contract.profileHash and actual via ifDefined), or if you intend to keep both messages for future cases, add a clear comment above profileMatch explaining the edge-case when profileMatch could be true and why the ternary is necessary; update the errorHashMismatch call accordingly to remove unreachable branches.packages/2-mongo-family/9-family/src/core/schema-to-view.ts (1)
44-52: Consider preserving explicitsparse: falsein meta.The expression
index.sparse || undefinedconvertsfalsetoundefined, omittingsparsefrom meta when it's explicitlyfalse. If distinguishing between "not sparse" and "sparse not specified" matters for debugging or display, consider:- ...ifDefined('sparse', index.sparse || undefined), + ...ifDefined('sparse', index.sparse),However, if the intent is to only show non-default options, the current approach is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-mongo-family/9-family/src/core/schema-to-view.ts` around lines 44 - 52, The meta builder currently uses index.sparse || undefined which turns an explicit false into undefined; update the meta population (in the block that constructs meta for each index, referencing index and the helper ifDefined) to use a nullish check (e.g., index.sparse ?? undefined) so that sparse: false is preserved in meta rather than omitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/2-mongo-family/9-family/src/core/control-instance.ts`:
- Around line 204-228: contractProfileHash can be undefined but initMarker and
updateMarker require a non-optional profileHash; update the function signatures
for initMarker and updateMarker to accept profileHash?: string (and adjust any
internal logic to handle undefined), then update their type definitions/exports
and all call sites (including the calls in this file that pass
contractProfileHash and the updateMarker call that uses
existingMarker.storageHash) so the optional profileHash flows safely;
alternatively, if you prefer stricter validation instead, add a null check
before calling initMarker/updateMarker and throw or handle the missing
profileHash explicitly instead of passing undefined.
---
Nitpick comments:
In `@packages/1-framework/3-tooling/cli/src/commands/db-verify.ts`:
- Around line 69-89: The ternary using profileMatch inside the
VERIFY_CODE_HASH_MISMATCH branch is confusing/unreachable: after the
storageMatch early return, profileMatch will always be false here, so simplify
the logic in the block that calls errorHashMismatch for profile mismatches (the
code that computes profileMatch and returns errorHashMismatch) — replace the
ternary with a single clear message like "Contract profileHash does not match
database marker" (and keep expected: verifyResult.contract.profileHash and
actual via ifDefined), or if you intend to keep both messages for future cases,
add a clear comment above profileMatch explaining the edge-case when
profileMatch could be true and why the ternary is necessary; update the
errorHashMismatch call accordingly to remove unreachable branches.
In `@packages/1-framework/3-tooling/cli/test/output.test.ts`:
- Around line 1-1037: The test file is too large and should be split into
focused test files to improve navigability; create separate test files for
introspection, schema verification, and signing (e.g., tests that exercise
formatIntrospectOutput/formatIntrospectJson and createSchemaView; tests for
formatSchemaVerifyOutput/formatSchemaVerifyJson and
createVerificationNode/createResult; tests for formatSignOutput/formatSignJson
and createSignResult). Move any shared helpers (like parseGlobalFlags usage or
stripAnsi import) into a small shared test helper module if needed, update each
new test file to import only the functions it needs (formatIntrospectOutput,
formatIntrospectJson, formatSchemaVerifyOutput, formatSchemaVerifyJson,
formatSignOutput, formatSignJson, parseGlobalFlags, SchemaTreeNode), and ensure
each describe block from the original file is preserved in the appropriate new
file so tests and helper factory functions (createSchemaView, createResult,
createVerificationNode, createSignResult) remain available and referenced
correctly.
In `@packages/2-mongo-family/9-family/src/core/schema-to-view.ts`:
- Around line 44-52: The meta builder currently uses index.sparse || undefined
which turns an explicit false into undefined; update the meta population (in the
block that constructs meta for each index, referencing index and the helper
ifDefined) to use a nullish check (e.g., index.sparse ?? undefined) so that
sparse: false is preserved in meta rather than omitted.
In `@test/integration/test/cli.mongo-db-sign.e2e.test.ts`:
- Around line 48-56: The extractJson function currently slices from the first
'{' to the last '}', which can break if there are multiple JSON objects or stray
braces; update extractJson to locate the first '{' then scan forward tracking
brace depth (increment on '{', decrement on '}') to find the matching closing
brace and parse only that balanced substring, throwing a clear error if no
matching closing brace is found; reference the extractJson function so you can
replace the current indexOf/lastIndexOf logic with this brace-counting loop to
make the extraction robust (or alternatively add a brief comment documenting the
single-JSON assumption if you choose not to change behavior).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: a67ccf51-3165-4395-8f91-74849da425a0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
.agents/rules/mongo-no-obsolete-commands.mdc.cursor/rules/README.mdpackages/1-framework/1-core/config/test/config-types.test-d.tspackages/1-framework/1-core/config/test/config-validation.test.tspackages/1-framework/1-core/framework-components/src/control-capabilities.tspackages/1-framework/1-core/framework-components/src/control-instances.tspackages/1-framework/1-core/framework-components/src/control-migration-types.tspackages/1-framework/1-core/framework-components/src/control-result-types.tspackages/1-framework/1-core/framework-components/src/control-schema-view.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/1-framework/3-tooling/cli/src/commands/contract-infer.tspackages/1-framework/3-tooling/cli/src/commands/db-verify.tspackages/1-framework/3-tooling/cli/src/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-init.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-update.tspackages/1-framework/3-tooling/cli/src/control-api/operations/migration-apply.tspackages/1-framework/3-tooling/cli/test/control-api/client.test.tspackages/1-framework/3-tooling/cli/test/control-api/db-update.test.tspackages/1-framework/3-tooling/cli/test/control-api/progress.test.tspackages/1-framework/3-tooling/cli/test/output.test.tspackages/2-mongo-family/3-tooling/mongo-schema-ir/README.mdpackages/2-mongo-family/3-tooling/mongo-schema-ir/src/exports/index.tspackages/2-mongo-family/3-tooling/mongo-schema-ir/src/schema-ir.tspackages/2-mongo-family/3-tooling/mongo-schema-ir/src/types.tspackages/2-mongo-family/3-tooling/mongo-schema-ir/src/visitor.tspackages/2-mongo-family/3-tooling/mongo-schema-ir/test/schema-ir.test.tspackages/2-mongo-family/9-family/package.jsonpackages/2-mongo-family/9-family/src/core/control-instance.tspackages/2-mongo-family/9-family/src/core/schema-diff.tspackages/2-mongo-family/9-family/src/core/schema-to-view.tspackages/2-mongo-family/9-family/test/control.test.tspackages/2-mongo-family/9-family/test/schema-diff.test.tspackages/2-sql/9-family/src/core/control-instance.tspackages/3-mongo-target/1-mongo-target/package.jsonpackages/3-mongo-target/1-mongo-target/src/core/marker-ledger.tspackages/3-mongo-target/1-mongo-target/src/exports/control.tspackages/3-mongo-target/1-mongo-target/test/marker-ledger.test.tspackages/3-mongo-target/1-mongo-target/tsconfig.jsonpackages/3-mongo-target/1-mongo-target/tsdown.config.tspackages/3-mongo-target/2-mongo-adapter/package.jsonpackages/3-mongo-target/2-mongo-adapter/src/core/contract-to-schema.tspackages/3-mongo-target/2-mongo-adapter/src/core/introspect-schema.tspackages/3-mongo-target/2-mongo-adapter/src/core/mongo-planner.tspackages/3-mongo-target/2-mongo-adapter/src/core/mongo-runner.tspackages/3-mongo-target/2-mongo-adapter/src/exports/control.tspackages/3-mongo-target/2-mongo-adapter/test/contract-to-schema.test.tspackages/3-mongo-target/2-mongo-adapter/test/introspect-schema.test.tspackages/3-mongo-target/2-mongo-adapter/test/mongo-planner.test.tspackages/3-mongo-target/2-mongo-adapter/test/mongo-runner.test.tstest/integration/test/cli.mongo-db-schema.e2e.test.tstest/integration/test/cli.mongo-db-sign.e2e.test.tstest/integration/test/cli.mongo-db-verify.e2e.test.ts
✅ Files skipped from review due to trivial changes (13)
- packages/1-framework/1-core/config/test/config-validation.test.ts
- packages/3-mongo-target/2-mongo-adapter/package.json
- packages/3-mongo-target/1-mongo-target/tsconfig.json
- packages/3-mongo-target/1-mongo-target/tsdown.config.ts
- .cursor/rules/README.md
- packages/3-mongo-target/1-mongo-target/src/exports/control.ts
- packages/2-mongo-family/9-family/package.json
- .agents/rules/mongo-no-obsolete-commands.mdc
- packages/2-mongo-family/3-tooling/mongo-schema-ir/README.md
- test/integration/test/cli.mongo-db-schema.e2e.test.ts
- packages/3-mongo-target/2-mongo-adapter/src/core/introspect-schema.ts
- packages/2-mongo-family/9-family/test/schema-diff.test.ts
- test/integration/test/cli.mongo-db-verify.e2e.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/3-mongo-target/2-mongo-adapter/src/exports/control.ts
- Add explicit unknown to ControlFamilyInstance in control-capabilities.ts and control-descriptors.ts (missed in F11 commit) - Replace non-null assertions with optional chaining in introspect-schema.ts - Update e2e test assertion to match array-based MongoSchemaIR collections
…xactOptionalPropertyTypes
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/integration/test/mongo/db-verify-sign.test.ts (1)
90-93: Prefer object matchers for grouped result assertions.Several tests assert multiple related fields with separate
toBecalls. Please switch these grouped checks totoMatchObjectfor consistency with repo test conventions.♻️ Example refactor pattern
- expect(result.ok).toBe(false); - expect(result.code).toBe('PN-RUN-3001'); + expect(result).toMatchObject({ + ok: false, + code: 'PN-RUN-3001', + }); expect(result.summary).toContain('missing');- expect(result.ok).toBe(true); - expect(result.marker.created).toBe(true); - expect(result.marker.updated).toBe(false); - expect(result.contract.storageHash).toBe(baseContract.storage.storageHash); + expect(result).toMatchObject({ + ok: true, + marker: { created: true, updated: false }, + contract: { storageHash: baseContract.storage.storageHash }, + });As per coding guidelines, "Prefer object matchers (toMatchObject) over multiple individual expect().toBe() calls when checking 2 or more related values in tests."
Also applies to: 109-111, 127-129, 145-147, 164-166, 180-183, 199-201, 217-219, 231-233, 245-249, 266-269, 295-298
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/mongo/db-verify-sign.test.ts` around lines 90 - 93, Replace grouped individual assertions on the result object with an object matcher: for the three assertions shown, collapse expect(result.ok).toBe(false) and expect(result.code).toBe('PN-RUN-3001') into a single expect(result).toMatchObject({ ok: false, code: 'PN-RUN-3001' }) and keep the summary substring check (expect(result.summary).toContain('missing')) if still needed; apply the same refactor for the other listed blocks (lines referencing the same result variable) so any tests that assert two or more related fields use toMatchObject rather than multiple expect(...).toBe() calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/1-framework/3-tooling/cli/src/commands/db-verify.ts`:
- Around line 70-89: storageMatch currently treats a missing marker.storageHash
as a mismatch; update the logic so you only return the specialized storage-hash
error when the marker actually contains storageHash. Concretely, change the
storage mismatch branch to check both that verifyResult.marker?.storageHash is
defined and that it differs from verifyResult.contract.storageHash (use the
existing storageMatch/profileMatch booleans or recompute as needed), e.g. only
call errorHashMismatch with the storage-specific message when
verifyResult.marker?.storageHash != null && !storageMatch; otherwise fall
through to the profile/hash handling and use errorHashMismatch for profileHash
or a generic contract/hash mismatch. Ensure you reference storageMatch,
profileMatch, verifyResult.marker?.storageHash, errorHashMismatch, and ifDefined
when making the change.
---
Nitpick comments:
In `@test/integration/test/mongo/db-verify-sign.test.ts`:
- Around line 90-93: Replace grouped individual assertions on the result object
with an object matcher: for the three assertions shown, collapse
expect(result.ok).toBe(false) and expect(result.code).toBe('PN-RUN-3001') into a
single expect(result).toMatchObject({ ok: false, code: 'PN-RUN-3001' }) and keep
the summary substring check (expect(result.summary).toContain('missing')) if
still needed; apply the same refactor for the other listed blocks (lines
referencing the same result variable) so any tests that assert two or more
related fields use toMatchObject rather than multiple expect(...).toBe() calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 5aff5408-8927-4dad-8ae9-ca0c25bc6a5f
⛔ Files ignored due to path filters (1)
test/integration/test/__snapshots__/cli.db-introspect.e2e.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
packages/1-framework/3-tooling/cli/src/commands/db-verify.tstest/integration/test/mongo/db-verify-sign.test.ts
wmadden
force-pushed
the
tml-2233-m4-online-cli-commands-live-introspection
branch
5 times, most recently
from
40c07a4 to
cfa169e
Compare
Pin MMS to exact version 10.4.3 via pnpm catalog so all consumers resolve the same binary. Add a dedicated spinUpMongoMemoryServer timeout (60s base) to test-utils — if the beforeAll hook is killed mid-download on CI, the partially-extracted binary corrupts the shared MMS cache and causes SIGSEGV in every subsequent mongo test. Migrate all 8 MMS consumers from inline versions to catalog:, all mongo test timeouts from spinUpDbServer to spinUpMongoMemoryServer, fix the orm package which had no vitest timeout config at all, and add missing test coverage for the marker meta fallback branch. Includes an agent rule (.agents/rules/mongodb-memory-server-setup.mdc) documenting the version pinning, timeout, and new-package checklist.
wmadden
force-pushed
the
tml-2233-m4-online-cli-commands-live-introspection
branch
from
b6e29a8 to
30a5699
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes TML-2233
Intent
Enable the MongoDB family to participate in all live-database CLI commands (
db schema,db verify,db sign) by implementing live schema introspection, schema verification with strict mode, and marker-based contract signing. This is the foundation that the remaining M4 commands (db init,db update,migration status --db) will build on.Change map
MongoControlFamilyInstancemethods:verify,schemaVerify,sign,readMarker,introspect,toSchemaViewdiffMongoSchemasand helpers: structural diff of twoMongoSchemaIRinstances with strict modePslPrintableSqlSchemaIRcast (SQL-only command)toSchemaViewmappingdb schemaintegrationdb schemadb verifydb signThe story
Live introspection:
introspectSchema(db)reads a live MongoDB instance vialistCollectionsandlistIndexes, producing aMongoSchemaIR. It filters internal collections (_prisma_migrations,system.*, views) and the default_idindex.Schema diffing:
diffMongoSchemas(live, expected, strict)compares twoMongoSchemaIRinstances and produces a structuredSchemaVerificationNodetree withSchemaIssueclassifications. Strict mode escalates "extra" elements (collections, indexes, validators, options in the DB but not in the contract) fromwarntofail, consistent with SQL family behavior.Family instance wiring:
MongoControlFamilyInstanceimplements the fullControlFamilyInstance<'mongo', MongoSchemaIR>interface —verify(marker-only),schemaVerify(live diff),sign(CAS marker write),readMarker,introspect, andtoSchemaView.CLI generalization:
inspect-live-schema.tsis widened so non-SQL families pass their schema IR through directly (SQL continues throughvalidatePrintableSqlSchemaIR). TheInspectLiveSchemaResult.schematype becomesunknown. This is tracked as tech debt for full removal in TML-2251.CLI e2e tests: Three test files exercise the actual Commander command implementations for
db schema,db verify, anddb signwith a livemongodb-memory-serverinstance and fixture config.Behavior changes & evidence
Adds live MongoDB introspection:
introspectSchema(db)reads indexes (all key types and options: unique, sparse, TTL, partial filter, wildcard, collation, text weights), validators ($jsonSchemawith validation level/action), and collection options (capped, timeseries, collation, changeStream, clusteredIndex). Filters_prisma_migrations,system.*, views, and default_idindexes._prisma_migrationsfiltering, round-trip fidelityAdds schema verification with structured diff tree:
diffMongoSchemasproducesSchemaVerificationNodetrees andSchemaIssuearrays for missing/extra/matching collections, indexes, validators, and options. Strict mode consistently escalates extra elements tofailacross all element types.Adds marker-based verify and CAS sign for MongoDB:
verify()checks marker existence and hash match (codesPN-RUN-3001/3002/3003).sign()creates or updates the marker with CAS conflict detection viaupdateMarker.verify(),sign()Adds
toSchemaViewfor MongoDB: MapsMongoSchemaIRtoCoreSchemaViewtree usingcollection,index, andfieldnode kinds. Produces deterministic, sorted output.toSchemaView()inspect-live-schema.tsgeneralized for non-SQL families:InspectLiveSchemaResult.schemawidened tounknown. SQL schemas go throughvalidatePrintableSqlSchemaIR; non-SQL pass through directly.CLI e2e coverage for all three Mongo commands: Exercises Commander command implementations with live
mongodb-memory-server, fixture config, and contract JSON.--jsonoutput with index metadata, human tree output--schema-onlymatch/fail,--marker-onlyCompatibility / migration / risk
InspectLiveSchemaResult.schemawidened tounknown: The only downstream consumer iscontract-infer.ts(SQL-only), which now uses an explicit cast. No runtime behavior change.familyId === 'sql'branch in framework code: This is an interim step. Full removal is tracked in TML-2251.Follow-ups / open questions
inspect-live-schema.ts— migrate SQL to the family-agnostic path and remove thefamilyId === 'sql'conditional.sign()method vs CLIdb signcommand: The family method is marker-only; the CLI command composesschemaVerify()+sign(). The M4 plan language should clarify this distinction.db init,db update,migration status --db, andmigration showare not yet implemented.Non-goals / intentionally out of scope
contract inferfor MongoDB: Would require Mongo-to-PSL reverse mapping.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores