feat: introduce CLI commands for schema diffing, DDL generation, and pulling, with supporting database connection, pg_dump, and project configuration utilities. #282
Conversation
…pulling, with supporting database connection, pg_dump, and project configuration utilities.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughImplements multi-source database connection resolution for ztd-cli DDL commands, wires resolved connection context through pull/diff flows, improves pg_dump discovery/error messages (including Windows hints), adds project-config connection parsing/merging, updates tests, and introduces formatting/linting templates and EditorConfig. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as CLI user
participant CLI as ztd-cli (command)
participant Config as ztd.config.json loader
participant Env as env (DATABASE_URL)
participant Resolver as resolveDatabaseConnection
participant PgDump as pg_dump subprocess
participant FS as Local filesystem
User->>CLI: run `ztd ddl pull/diff` with flags/env
CLI->>Config: loadZtdProjectConfig()
CLI->>Env: read DATABASE_URL
CLI->>Resolver: resolveDatabaseConnection(flags, config, explicitUrl?)
Note right of Resolver: precedence: env > explicitUrl/flags > config
Resolver-->>CLI: { url, connectionContext }
CLI->>PgDump: runPgDump(url, connectionContext, extraArgs)
PgDump->>PgDump: describeConnectionContext(connectionContext)
PgDump->>PgDump: spawn pg_dump (or detect missing)
alt pg_dump found & exits 0
PgDump-->>CLI: stdout (schema SQL)
CLI->>FS: write schema files / run diff
else pg_dump missing or error
PgDump-->>CLI: error with connectionNote & Windows hint
end
CLI-->>User: success or error message (no credentials leaked)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/ztd-cli/README.md (1)
48-60: Consider documenting theurlfield option.The sample connection block shows individual connection parameters (
host,port,user,password,database). Based on theZtdConnectionConfiginterface inpackages/ztd-cli/src/utils/ztdProjectConfig.ts(lines 4-11), the connection block also supports an optionalurlfield that can contain a complete connection string.Consider adding a note or alternative example showing the
urlfield:{ "connection": { "url": "postgres://user:password@host:5432/database" } }packages/ztd-cli/src/commands/ddl.ts (1)
108-108: Fix indentation of closing brace.The closing brace for the
registerDdlCommandsfunction has extra indentation. It should be at column 0 to match the function declaration at line 18.- } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/ztd-cli/tests/__snapshots__/diff.unit.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
packages/ztd-cli/README.md(1 hunks)packages/ztd-cli/src/commands/ddl.ts(3 hunks)packages/ztd-cli/src/commands/diff.ts(2 hunks)packages/ztd-cli/src/commands/pull.ts(2 hunks)packages/ztd-cli/src/utils/dbConnection.ts(1 hunks)packages/ztd-cli/src/utils/pgDump.ts(2 hunks)packages/ztd-cli/src/utils/typeMapper.ts(1 hunks)packages/ztd-cli/src/utils/ztdProjectConfig.ts(5 hunks)packages/ztd-cli/tests/dbConnection.unit.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: All SQL rewrites must rely onrawsql-tsAST utilities (parser, analyzer, splitQueries). Regex-based rewrites are allowed only as guarded fallbacks with comments explaining why.
Block contributions introducing regex parsing when an AST alternative exists.
Remove console debugging before committing.
Files:
packages/ztd-cli/src/utils/typeMapper.tspackages/ztd-cli/src/commands/diff.tspackages/ztd-cli/src/commands/pull.tspackages/ztd-cli/tests/dbConnection.unit.test.tspackages/ztd-cli/src/utils/pgDump.tspackages/ztd-cli/src/commands/ddl.tspackages/ztd-cli/src/utils/ztdProjectConfig.tspackages/ztd-cli/src/utils/dbConnection.ts
🧠 Learnings (32)
📓 Common learnings
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/pg-testkit/AGENTS.md:0-0
Timestamp: 2025-12-05T23:20:16.602Z
Learning: Applies to packages/drivers/pg-testkit/src/**/*.{ts,tsx,js,jsx} : Load DDL-based fixtures from tests/ztd-config.ts and ddl/ directory when present, treating them as canonical schema + row map
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T00:19:02.490Z
Learning: The rawsql-ts workspace treats the database engine as a planner and type checker, not a schema host. In tests, no physical tables are created, migrated, or mutated. All schema information and seed rows must come from DDL files and fixtures.
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.329Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : In rawsql-ts/testkit-core, remain DBMS-agnostic with no Postgres/SQLite conditionals or behavior
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T00:19:02.490Z
Learning: Applies to ztd.config.json : Application SQL can omit schema qualifiers (e.g., `SELECT ... FROM users`). pg-testkit maps those references to canonical `schema.table` keys by consulting the `ddl.defaultSchema` / `ddl.searchPath` block in `ztd.config.json`. Update `ztd.config.json` whenever you change the target schema or search path.
📚 Learning: 2025-12-02T22:58:15.329Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.329Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : warn and passthrough modes in testkit-core must behave predictably and never silently rewrite incorrect SQL
Applied to files:
packages/ztd-cli/src/utils/typeMapper.ts
📚 Learning: 2025-12-05T23:20:16.602Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/pg-testkit/AGENTS.md:0-0
Timestamp: 2025-12-05T23:20:16.602Z
Learning: Applies to packages/drivers/pg-testkit/src/**/*.{ts,tsx,js,jsx} : Do not reconstruct DDL or row-type definitions inside pg-testkit when the project already ships tests/ztd-config.ts and ddl/ files—rely on the generated TestRowMap instead
Applied to files:
packages/ztd-cli/src/utils/typeMapper.tspackages/ztd-cli/src/commands/diff.tspackages/ztd-cli/README.mdpackages/ztd-cli/src/commands/pull.tspackages/ztd-cli/src/utils/pgDump.tspackages/ztd-cli/src/commands/ddl.tspackages/ztd-cli/src/utils/ztdProjectConfig.tspackages/ztd-cli/src/utils/dbConnection.ts
📚 Learning: 2025-12-02T22:57:55.626Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.626Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx} : Do not use regex-based SQL parsing inside sqlite-testkit; if temporary regex-based merge is absolutely required, add TODO and inline comment, emit logger.debug notice, and file an issue in testkit-core
Applied to files:
packages/ztd-cli/src/utils/typeMapper.ts
📚 Learning: 2025-12-05T23:20:16.602Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/pg-testkit/AGENTS.md:0-0
Timestamp: 2025-12-05T23:20:16.602Z
Learning: Applies to packages/drivers/pg-testkit/src/**/*.{ts,tsx,js,jsx} : Load DDL-based fixtures from tests/ztd-config.ts and ddl/ directory when present, treating them as canonical schema + row map
Applied to files:
packages/ztd-cli/src/utils/typeMapper.tspackages/ztd-cli/src/commands/diff.tspackages/ztd-cli/README.mdpackages/ztd-cli/src/commands/pull.tspackages/ztd-cli/tests/dbConnection.unit.test.tspackages/ztd-cli/src/utils/pgDump.tspackages/ztd-cli/src/commands/ddl.tspackages/ztd-cli/src/utils/ztdProjectConfig.tspackages/ztd-cli/src/utils/dbConnection.ts
📚 Learning: 2025-12-02T22:57:21.011Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.011Z
Learning: Applies to packages/core/**/*.{test,spec}.ts : Use SQL normalization via SqlFormatter when comparing SQL in tests instead of hand-written comparisons
Applied to files:
packages/ztd-cli/src/utils/typeMapper.ts
📚 Learning: 2025-12-02T22:58:15.329Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.329Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : In rawsql-ts/testkit-core, remain DBMS-agnostic with no Postgres/SQLite conditionals or behavior
Applied to files:
packages/ztd-cli/src/utils/typeMapper.tspackages/ztd-cli/tests/dbConnection.unit.test.tspackages/ztd-cli/src/utils/pgDump.tspackages/ztd-cli/src/commands/ddl.tspackages/ztd-cli/src/utils/dbConnection.ts
📚 Learning: 2025-12-07T00:19:02.490Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T00:19:02.490Z
Learning: Applies to ztd.config.json : Application SQL can omit schema qualifiers (e.g., `SELECT ... FROM users`). pg-testkit maps those references to canonical `schema.table` keys by consulting the `ddl.defaultSchema` / `ddl.searchPath` block in `ztd.config.json`. Update `ztd.config.json` whenever you change the target schema or search path.
Applied to files:
packages/ztd-cli/src/utils/typeMapper.tspackages/ztd-cli/src/commands/diff.tspackages/ztd-cli/README.mdpackages/ztd-cli/src/commands/ddl.tspackages/ztd-cli/src/utils/ztdProjectConfig.ts
📚 Learning: 2025-12-02T22:57:55.626Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.626Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx} : Surface clear, typed errors from the driver layer using error classes like MissingFixtureError and SchemaValidationError with descriptive messages including table/column hints
Applied to files:
packages/ztd-cli/src/utils/typeMapper.ts
📚 Learning: 2025-12-02T22:58:15.329Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.329Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : Do not rewrite SQL by string concatenation without AST in testkit-core
Applied to files:
packages/ztd-cli/src/utils/typeMapper.ts
📚 Learning: 2025-12-02T22:57:21.011Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.011Z
Learning: Applies to packages/core/src/**/*.ts : Do not introduce dialect branching (if postgres...) in parser or analyzer logic
Applied to files:
packages/ztd-cli/src/utils/typeMapper.ts
📚 Learning: 2025-12-07T00:19:02.490Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T00:19:02.490Z
Learning: Applies to packages/pg-testkit/**/*.{ts,tsx} : Do not treat the backing DB as a migration target. The DB engine is only for planning/type-checking. Never execute `CREATE TABLE`, `ALTER TABLE`, or seed `INSERT`s over a pg-testkit connection.
Applied to files:
packages/ztd-cli/src/commands/diff.tspackages/ztd-cli/src/commands/pull.tspackages/ztd-cli/tests/dbConnection.unit.test.tspackages/ztd-cli/src/utils/pgDump.tspackages/ztd-cli/src/commands/ddl.tspackages/ztd-cli/src/utils/dbConnection.ts
📚 Learning: 2025-12-07T00:19:12.317Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: playgrounds/ztd-playground/AGENTS.md:0-0
Timestamp: 2025-12-07T00:19:12.317Z
Learning: Applies to playgrounds/ztd-playground/tests/**/*.test.{ts,tsx} : Always wire Postgres execution through `tests/test-utils.ts`, which opens a `pg.Client`, passes it into `rawsql-ts/pg-testkit`, and shares the connection across tests
Applied to files:
packages/ztd-cli/src/commands/diff.tspackages/ztd-cli/src/commands/pull.tspackages/ztd-cli/tests/dbConnection.unit.test.tspackages/ztd-cli/src/utils/pgDump.tspackages/ztd-cli/src/commands/ddl.tspackages/ztd-cli/src/utils/ztdProjectConfig.tspackages/ztd-cli/src/utils/dbConnection.ts
📚 Learning: 2025-12-07T00:19:12.317Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: playgrounds/ztd-playground/AGENTS.md:0-0
Timestamp: 2025-12-07T00:19:12.317Z
Learning: Applies to playgrounds/ztd-playground/ddl/schemas/*.sql : Keep every table definition inside `ddl/schemas/*.sql` and treat it as the single source of truth
Applied to files:
packages/ztd-cli/README.md
📚 Learning: 2025-12-05T23:20:16.602Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/pg-testkit/AGENTS.md:0-0
Timestamp: 2025-12-05T23:20:16.602Z
Learning: Applies to packages/drivers/pg-testkit/ddl/**/*.sql : Use DDL for canonical schema information and INSERT statements inside DDL only for true shared baseline data—do not put per-test seeds inside schema files
Applied to files:
packages/ztd-cli/README.md
📚 Learning: 2025-12-02T22:58:15.329Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.329Z
Learning: Applies to packages/testkit-core/tests/**/*.{test,spec}.{ts,tsx} : Add test coverage for fixture resolution paths, CRUD rewrite transformations, CTE + multi-statement handling, fallback logic, identifier casing rules, and error diagnostics in testkit-core
Applied to files:
packages/ztd-cli/tests/dbConnection.unit.test.tspackages/ztd-cli/src/commands/ddl.tspackages/ztd-cli/src/utils/dbConnection.ts
📚 Learning: 2025-12-02T22:57:55.626Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.626Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.test.{ts,tsx} : Use Vitest for both unit and integration-level coverage; integration tests should exercise real better-sqlite3 connections
Applied to files:
packages/ztd-cli/tests/dbConnection.unit.test.ts
📚 Learning: 2025-12-02T22:57:55.626Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.626Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.test.{ts,tsx} : Ensure tests remain deterministic regardless of file-backed vs. memory-backed SQLite configurations
Applied to files:
packages/ztd-cli/tests/dbConnection.unit.test.ts
📚 Learning: 2025-12-02T22:57:55.626Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.626Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx} : Connection lifecycle: driver.close() must close the wrapped database handle and multiple close() calls must be idempotent for test ergonomics
Applied to files:
packages/ztd-cli/tests/dbConnection.unit.test.tspackages/ztd-cli/src/utils/dbConnection.ts
📚 Learning: 2025-12-02T22:57:55.626Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.626Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx} : Validate fixtures at construction time to keep per-query interception fast; respect passthrough tables and wildcard overrides driven by testkit-core
Applied to files:
packages/ztd-cli/tests/dbConnection.unit.test.ts
📚 Learning: 2025-12-02T22:57:55.626Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.626Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx} : Do not rely on real table state between queries even for in-memory databases; all perceived state must originate from fixtures supplied to the driver
Applied to files:
packages/ztd-cli/tests/dbConnection.unit.test.ts
📚 Learning: 2025-12-07T00:19:12.317Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: playgrounds/ztd-playground/AGENTS.md:0-0
Timestamp: 2025-12-07T00:19:12.317Z
Learning: Set `DATABASE_URL` before running `pnpm playground:test` or any spec that touches the Postgres testkit helper; the helper throws a clear error when it is missing
Applied to files:
packages/ztd-cli/tests/dbConnection.unit.test.ts
📚 Learning: 2025-12-02T22:57:55.626Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.626Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx} : withFixtures() must return a shallow copy of the driver, inherit base configuration, and layer additional fixtures/scenario-specific data on top
Applied to files:
packages/ztd-cli/tests/dbConnection.unit.test.ts
📚 Learning: 2025-12-05T23:20:16.602Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/pg-testkit/AGENTS.md:0-0
Timestamp: 2025-12-05T23:20:16.602Z
Learning: Applies to packages/drivers/pg-testkit/src/**/*.{ts,tsx,js,jsx} : Do not branch dialect behavior beyond PostgreSQL's expected behavior
Applied to files:
packages/ztd-cli/src/utils/pgDump.ts
📚 Learning: 2025-12-05T23:20:16.602Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/pg-testkit/AGENTS.md:0-0
Timestamp: 2025-12-05T23:20:16.602Z
Learning: Applies to packages/drivers/pg-testkit/src/**/*.{ts,tsx,js,jsx} : Emit debug traces when fallback paths trigger, analyzer limitations appear, or fixture mismatches surface
Applied to files:
packages/ztd-cli/src/utils/pgDump.ts
📚 Learning: 2025-12-07T00:19:12.317Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: playgrounds/ztd-playground/AGENTS.md:0-0
Timestamp: 2025-12-07T00:19:12.317Z
Learning: Applies to playgrounds/ztd-playground/tests/**/*.test.{ts,tsx} : Import `ZtdConfig`, `ZtdRowShapes`, `ZtdTableName`, and `tableFixture()` from the generated `ztd-config.ts` file and trust the generated helpers for row shapes instead of duplicating row interfaces inside tests
Applied to files:
packages/ztd-cli/src/commands/ddl.tspackages/ztd-cli/src/utils/ztdProjectConfig.ts
📚 Learning: 2025-12-07T00:19:12.317Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: playgrounds/ztd-playground/AGENTS.md:0-0
Timestamp: 2025-12-07T00:19:12.317Z
Learning: Applies to playgrounds/ztd-playground/tests/**/*.test.{ts,tsx} : Never issue DDL statements against Postgres from the playground. All CRUD operations must flow through pg-testkit so they resolve to fixture-backed `SELECT` queries
Applied to files:
packages/ztd-cli/src/commands/ddl.ts
📚 Learning: 2025-12-07T00:19:12.317Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: playgrounds/ztd-playground/AGENTS.md:0-0
Timestamp: 2025-12-07T00:19:12.317Z
Learning: Applies to playgrounds/ztd-playground/**/ztd-config.ts : Do not hand-edit `ztd-config.ts`; regenerate it with `pnpm --filter ztd-playground exec ztd ztd-config`
Applied to files:
packages/ztd-cli/src/commands/ddl.tspackages/ztd-cli/src/utils/ztdProjectConfig.ts
📚 Learning: 2025-12-02T22:57:21.011Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.011Z
Learning: Applies to packages/core/src/**/*.ts : Prefer import { ... } from 'rawsql-ts' over deep relative paths
Applied to files:
packages/ztd-cli/src/utils/ztdProjectConfig.ts
📚 Learning: 2025-12-02T22:57:21.011Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.011Z
Learning: Applies to packages/core/src/**/parser/**/*.ts : Never hard-wire DB-specific logic or branching in AST pipelines; avoid dialect checks like isPostgres or isSQLite inside parser or analyzer code
Applied to files:
packages/ztd-cli/src/utils/dbConnection.ts
📚 Learning: 2025-12-02T22:58:15.329Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.329Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : Always use SelectQueryParser, SelectAnalyzer, and splitQueries for rewriting SQL; use regex only as a fallback with a comment explaining the need, a link to a tracking issue, and a description of AST migration
Applied to files:
packages/ztd-cli/src/utils/dbConnection.ts
📚 Learning: 2025-12-02T22:58:15.329Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.329Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : Do not use DBMS branching (if postgres, if sqlite, etc.) in testkit-core implementation
Applied to files:
packages/ztd-cli/src/utils/dbConnection.ts
🧬 Code graph analysis (6)
packages/ztd-cli/src/commands/diff.ts (2)
packages/ztd-cli/src/utils/dbConnection.ts (1)
DbConnectionContext(14-20)packages/ztd-cli/src/utils/pgDump.ts (1)
runPgDump(15-57)
packages/ztd-cli/src/commands/pull.ts (1)
packages/ztd-cli/src/utils/dbConnection.ts (1)
DbConnectionContext(14-20)
packages/ztd-cli/tests/dbConnection.unit.test.ts (2)
packages/ztd-cli/src/utils/dbConnection.ts (1)
resolveDatabaseConnection(29-85)packages/ztd-cli/src/utils/ztdProjectConfig.ts (1)
DEFAULT_ZTD_CONFIG(31-36)
packages/ztd-cli/src/utils/pgDump.ts (1)
packages/ztd-cli/src/utils/dbConnection.ts (1)
DbConnectionContext(14-20)
packages/ztd-cli/src/commands/ddl.ts (5)
packages/ztd-cli/src/commands/options.ts (4)
collectValues(10-12)collectDirectories(6-8)parseExtensions(21-33)DEFAULT_EXTENSIONS(1-1)packages/ztd-cli/src/utils/dbConnection.ts (1)
resolveDatabaseConnection(29-85)packages/ztd-cli/src/utils/ztdProjectConfig.ts (1)
loadZtdProjectConfig(42-85)packages/ztd-cli/src/commands/pull.ts (1)
runPullSchema(22-73)packages/ztd-cli/src/commands/diff.ts (1)
runDiffSchema(18-44)
packages/ztd-cli/src/utils/dbConnection.ts (1)
packages/ztd-cli/src/utils/ztdProjectConfig.ts (1)
ZtdProjectConfig(13-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test (20.x)
🔇 Additional comments (21)
packages/ztd-cli/src/utils/ztdProjectConfig.ts (3)
111-158: LGTM: Connection config normalization is well-implemented.The normalization logic correctly handles various input types, validates port numbers, and intentionally preserves password whitespace while trimming other fields. The function safely returns
undefinedfor empty configs.
160-174: LGTM: Connection config merge is straightforward and correct.The merge logic appropriately uses spread operators to layer overrides on top of base configuration, and correctly returns
undefinedfor empty results.
87-109: LGTM: Config writing correctly handles optional connection field.The function properly merges the connection configuration and removes the field when undefined, keeping the serialized JSON clean.
packages/ztd-cli/src/utils/typeMapper.ts (1)
31-31: LGTM: Log prefix update aligns with CLI branding.The prefix change from "[rawsql ddl]" to "[ztd ddl]" maintains consistency with other DDL command messaging throughout the codebase.
packages/ztd-cli/src/commands/pull.ts (1)
6-6: LGTM: Connection context integration is clean.The optional
connectionContextfield is properly threaded through torunPgDump, enabling enhanced error messaging without breaking existing code.Also applies to: 14-14, 34-34
packages/ztd-cli/src/commands/diff.ts (2)
7-7: LGTM: Connection context integration mirrors pull command.The
connectionContextfield is consistently added and passed through torunPgDump, following the same pattern as the pull command.Also applies to: 15-15, 26-30
38-38: LGTM: Header update aligns with CLI branding.The diff plan header change from "rawsql ddl diff plan" to "ztd ddl diff plan" maintains consistency with other CLI messaging.
packages/ztd-cli/src/commands/ddl.ts (2)
21-54: LGTM: Pull command integration with connection resolution.The pull command correctly:
- Accepts individual connection flags (
--db-host,--db-port, etc.)- Resolves the connection using the new utility
- Passes the resolved context to
runPullSchema
72-107: LGTM: Diff command integration with connection resolution.The diff command follows the same pattern as the pull command, correctly integrating connection resolution and passing context through to
runDiffSchema.packages/ztd-cli/tests/dbConnection.unit.test.ts (2)
5-21: LGTM: Environment management helper is well-implemented.The
withEnvhelper correctly saves, modifies, and restoresprocess.env.DATABASE_URL, ensuring test isolation.
23-94: LGTM: Comprehensive test coverage for connection resolution.The test suite validates all precedence scenarios:
- Explicit flags override environment
- Environment variable fallback
- Explicit URL argument
- Configuration block fallback
- Missing connection error reporting
All tests properly verify the
sourcefield and key connection parameters.packages/ztd-cli/src/utils/pgDump.ts (3)
59-79: LGTM: Connection context description is clear and informative.The
describeConnectionContextfunction builds a readable description including source, host, port, database, and user information, which will greatly improve error diagnostics.
81-92: LGTM: Executable detection logic is robust.The
isExecutableMissingfunction correctly checks:
ENOENTerror code (standard "file not found")- stderr patterns for command not found messages
This covers both Unix and Windows error scenarios.
34-49: LGTM: Enhanced error messaging with Windows-specific guidance.The error handling provides:
- Clear missing executable detection
- Platform-specific installation hints for Windows
- Detailed connection context for debugging authentication failures
packages/ztd-cli/src/utils/dbConnection.ts (7)
1-26: LGTM!Clean type definitions with appropriate separation between CLI input types (
DbConnectionFlagswith string port) and resolved output types (DbConnectionContextwith numeric port). The source tracking aids debugging and user feedback.
87-95: LGTM!Correctly identifies partial flag usage to trigger the explicit flags path, which then validates completeness. Excluding
urlhere is appropriate since it's handled as a separate precedence level.
97-137: LGTM!Solid validation with clear, actionable error messages. The grammatical handling of singular/plural in the error message is a nice touch.
139-179: LGTM!Consistent validation pattern with
resolveFromFlags. Error messages clearly indicate which config fields are missing.
181-192: LGTM!Comprehensive port validation covering all edge cases including non-finite values, non-integers, and out-of-range values.
194-217: LGTM!Good use of the
URLconstructor which handles special character encoding automatically for usernames, passwords, and paths.
219-233: LGTM!Appropriate error handling for metadata extraction—silently returning an empty object for malformed URLs is reasonable since this context is used for display/debugging rather than connection logic.
packages/ztd-cli/README.md
Outdated
| Fetches the schema via `pg_dump`, normalizes the DDL, and writes one file per schema under `ddl/schemas/<schema>.sql` instead of a single `schema.sql`. The output drops headers, `SET` statements, and `\restrict` markers, sorts objects (schemas, tables, alterations, sequences, indexes) deterministically, and ensures each schema file ends with a clean newline so `ztd gen-config` and your AI flows always see stable input. | ||
| Fetches the schema via `pg_dump`, normalizes the DDL, and writes one file per schema under `ddl/schemas/<schema>.sql` instead of a single `schema.sql`. The output drops headers, `SET` statements, and `\restrict` markers, sorts objects (schemas, tables, alterations, sequences, indexes) deterministically, and ensures each schema file ends with a clean newline so `ztd gen-config` and your AI flows always see stable input. | ||
|
|
||
| The command resolves the database connection in three steps: prefer a `DATABASE_URL` environment variable, honor explicit flags (`--db-host`, `--db-port`, `--db-user`, `--db-password`, `--db-name`) when supplied, and finally fall back to a `connection` block in `ztd.config.json` if present. Flags that provide partial credentials will cause the CLI to error before invoking `pg_dump` so you know exactly what is missing, and the generated error message always mentions the connection target together with concrete fixes. |
There was a problem hiding this comment.
Verify connection resolution precedence order.
The documentation states the resolution order as: "prefer a DATABASE_URL environment variable, honor explicit flags (--db-host, ...) when supplied, and finally fall back to a connection block in ztd.config.json."
However, based on the implementation in packages/ztd-cli/src/utils/dbConnection.ts (lines 28-84), the actual precedence is:
- Explicit CLI flags (
--db-host,--db-port, etc.) - highest priority - Explicit URL argument
DATABASE_URLenvironment variableconnectionblock inztd.config.json- lowest priority
The documentation appears to list these in reverse order, which could confuse users about which source takes precedence.
Consider rewording to: "The command resolves the database connection in three steps: explicit flags (--db-host, --db-port, --db-user, --db-password, --db-name) take highest priority, then the DATABASE_URL environment variable, and finally the connection block in ztd.config.json if present."
🤖 Prompt for AI Agents
In packages/ztd-cli/README.md around line 46, the documented connection
resolution order is incorrect; update the sentence to match the implementation:
state that explicit CLI flags (--db-host, --db-port, --db-user, --db-password,
--db-name) take highest priority, then an explicit URL argument (if supplied),
then the DATABASE_URL environment variable, and finally the connection block in
ztd.config.json; rewrite the sentence to list that exact precedence clearly and
concisely.
| if (connectionConfig.url) { | ||
| return { | ||
| url: connectionConfig.url, | ||
| context: { | ||
| source: 'config', | ||
| ...parseUrlContext(connectionConfig.url), | ||
| ...(connectionConfig.host ? { host: connectionConfig.host } : {}), | ||
| ...(connectionConfig.user ? { user: connectionConfig.user } : {}), | ||
| ...(connectionConfig.database ? { database: connectionConfig.database } : {}), | ||
| ...(connectionConfig.port ? { port: connectionConfig.port } : {}) | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
Context may misrepresent the actual connection URL.
When connectionConfig.url is set alongside explicit fields like host, user, or database, the context will reflect those explicit fields while the actual connection uses the URL. This can mislead users when diagnosing connection issues.
Consider either:
- Using only
parseUrlContextfor the context when a URL is provided, or - Validating that explicit config fields match the URL, or
- Reconstructing the URL from explicit fields when they're provided alongside a URL.
if (connectionConfig.url) {
+ // When a URL is provided, use only the parsed context to avoid mismatches
return {
url: connectionConfig.url,
context: {
source: 'config',
- ...parseUrlContext(connectionConfig.url),
- ...(connectionConfig.host ? { host: connectionConfig.host } : {}),
- ...(connectionConfig.user ? { user: connectionConfig.user } : {}),
- ...(connectionConfig.database ? { database: connectionConfig.database } : {}),
- ...(connectionConfig.port ? { port: connectionConfig.port } : {})
+ ...parseUrlContext(connectionConfig.url)
}
};
}🤖 Prompt for AI Agents
In packages/ztd-cli/src/utils/dbConnection.ts around lines 66 to 78, the context
returned when connectionConfig.url is present incorrectly merges explicit fields
(host/user/database/port) into the context while the actual connection will use
the URL, which can mislead debugging; change the behavior so that when
connectionConfig.url exists the context is built solely from
parseUrlContext(connectionConfig.url) (plus source: 'config') rather than
merging explicit fields, or alternatively add validation to ensure any explicit
fields match the parsed URL before merging — implement the simpler fix: remove
the conditional spreads of connectionConfig.host/user/database/port when url is
present so only parseUrlContext is used for context.
…Lite affinity and add playground tests.
…ion and schema management.
…ground, and updated formatting guidelines in AGENTS.md
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
playgrounds/ztd-playground/package.json (1)
1-42: Updatepnpm-lock.yamlto sync lockfile with new dependencies.The pipeline is currently failing because 5 new devDependencies have been added to
package.json(eslint, lint-staged, prettier, prettier-plugin-sql, simple-git-hooks) but the lockfile has not been updated. Runpnpm installto regeneratepnpm-lock.yamland include it in this PR.
♻️ Duplicate comments (1)
packages/ztd-cli/src/utils/dbConnection.ts (1)
63-80: Config URL branch can still misrepresent the actual targetWhen
connectionConfig.urlis set, you connect with that URL, but the context overlayshost/user/database/portfrom the config object. If those fields don’t match the URL, the reported context can be wrong and make failures harder to debug.Given this branch already has the full URL, it’s simpler and less misleading to have:
context: { source: 'config', ...parseUrlContext(trimmedConfigUrl), }and skip merging the explicit fields when
urlis present.
🧹 Nitpick comments (6)
playgrounds/ztd-playground/package.json (1)
34-41: Consider adding linting to the pre-commit hook.The pre-commit hook currently runs only
pnpm formatvia lint-staged. For consistency with the new lint/lint:fix scripts and to catch issues before commits, consider also runningpnpm lintor an equivalent command that flags (without auto-fixing) issues. This would catch patterns that prettier cannot fix, such as unused variables, naming violations, or import ordering.packages/testkit-core/src/fixtures/ColumnAffinity.ts (1)
1-1: Consider evolving ColumnAffinity toward full DBMS-agnostic abstraction.The rename from
SqliteAffinitytoColumnAffinityimproves the API surface, but the union values (TEXT, INTEGER, REAL, NUMERIC, BLOB) remain SQLite-specific type affinities. The patterns on Lines 6-10 already handle cross-DBMS type names (bytea, serial, money), so a future iteration could map these to a truly generic abstraction.packages/ztd-cli/src/commands/init.ts (2)
394-471: Consider prompting before modifying package.json.Unlike other file operations in this command (schema, config, readme, etc.) which use
writeFileWithConsent, this function directly modifiespackage.jsonwithout asking the user. While the changes are additive and reported in the summary, the inconsistency may surprise users.Consider either:
- Wrapping the modifications in a consent prompt for consistency
- Documenting this behavior with a comment explaining why consent is skipped
446-452: Consider parameterizing or documenting dependency versions.The devDependencies versions are hardcoded and will become stale over time. Consider:
- Documenting when these versions were last updated
- Making them configurable
- Or accepting that templates ship with known-good versions at the time of CLI release
This is acceptable for templates but worth noting for future maintenance.
packages/ztd-cli/src/utils/dbConnection.ts (1)
220-233: Defensively handle non-numeric ports inparseUrlContextIf
parsed.portis set but non-numeric,Number(parsed.port)will yieldNaN, which then leaks intocontext.port. It’s harmless but a bit noisy for diagnostics.You could treat non-numeric ports as “no port” to keep context clean:
const port = parsed.port ? Number(parsed.port) : undefined; return { host: parsed.hostname || undefined, port: Number.isFinite(port) ? port : undefined, // ... };packages/ztd-cli/tests/diff.unit.test.ts (1)
45-67: Optionally assertrunPgDumparguments for stronger wiring guaranteesRight now the test only checks that
runPgDumpwas invoked. If you want to guard the new API surface more tightly, you could also assert the arguments, e.g. that it’s called with the expectedurlandconnectionContext, so accidental regressions in how diff wiring passes those options get caught here.
| }, | ||
| "dependencies": { | ||
| "@rawsql-ts/testkit-core": "^0.1.0", | ||
| "@rawsql-ts/testkit-core": "link:../../testkit-core", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check testkit-core dependency across all driver packages
echo "=== Checking testkit-core dependency in driver packages ==="
find packages/drivers -name "package.json" -type f | while read f; do
echo "--- $f ---"
grep -A1 -B1 '@rawsql-ts/testkit-core' "$f" || echo "Not found"
done
echo ""
echo "=== Checking pnpm workspace config ==="
ls -la pnpm-workspace.yaml 2>/dev/null && cat pnpm-workspace.yaml || echo "pnpm-workspace.yaml not found"
echo ""
echo "=== Checking root package.json workspaces ==="
if [ -f "package.json" ]; then
cat package.json | head -30
fiRepository: mk3008/rawsql-ts
Length of output: 2158
Standardize testkit-core dependency to pnpm workspace protocol.
The sqlite-testkit package uses link:../../testkit-core for the testkit-core dependency, but pg-testkit uses the correct pnpm workspace protocol workspace:*. Inconsistent dependency resolution strategies across driver packages breaks monorepo conventions and can cause resolution issues. Change line 33 to:
"@rawsql-ts/testkit-core": "workspace:*",This aligns sqlite-testkit with pg-testkit and respects the pnpm-workspace.yaml configuration.
🤖 Prompt for AI Agents
In packages/drivers/sqlite-testkit/package.json around line 33, the dependency
for @rawsql-ts/testkit-core uses npm link form ("link:../../testkit-core")
instead of the pnpm workspace protocol; replace that value with the workspace
protocol ("workspace:*") so the package uses pnpm workspace resolution
consistent with other driver packages and pnpm-workspace.yaml.
| const document = readFileSync(packagePath, 'utf8'); | ||
| const parsed = JSON.parse(document) as Record<string, unknown>; | ||
| let changed = false; |
There was a problem hiding this comment.
Add error handling for malformed JSON.
The code reads and parses package.json without error handling. If the file contains malformed JSON, JSON.parse will throw and crash the init command.
Apply this diff to add defensive error handling:
- const document = readFileSync(packagePath, 'utf8');
- const parsed = JSON.parse(document) as Record<string, unknown>;
+ const document = readFileSync(packagePath, 'utf8');
+ let parsed: Record<string, unknown>;
+ try {
+ parsed = JSON.parse(document) as Record<string, unknown>;
+ } catch (error) {
+ dependencies.log(`Warning: Could not parse package.json, skipping formatting configuration.`);
+ return null;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const document = readFileSync(packagePath, 'utf8'); | |
| const parsed = JSON.parse(document) as Record<string, unknown>; | |
| let changed = false; | |
| const document = readFileSync(packagePath, 'utf8'); | |
| let parsed: Record<string, unknown>; | |
| try { | |
| parsed = JSON.parse(document) as Record<string, unknown>; | |
| } catch (error) { | |
| dependencies.log(`Warning: Could not parse package.json, skipping formatting configuration.`); | |
| return null; | |
| } | |
| let changed = false; |
🤖 Prompt for AI Agents
In packages/ztd-cli/src/commands/init.ts around lines 405–407, the code reads
package.json and calls JSON.parse without handling parse errors; wrap the
JSON.parse call in a try/catch, and on catch log a clear error including the
packagePath and the error.message (using the existing logger or console.error),
then exit the process with a non-zero code (or rethrow) so the init command
fails gracefully; after successful parse continue as before, and consider
falling back to an empty object only if you explicitly want to recover instead
of exiting.
| const previousDatabaseUrl = process.env.DATABASE_URL; | ||
| process.env.DATABASE_URL = connectionString; | ||
| try { | ||
| const result = runCli(['ddl', 'pull', '--out', outDir]); | ||
| assertCliSuccess(result, 'ddl pull'); | ||
| const schemaDir = path.join(outDir, 'schemas'); | ||
| expect(existsSync(schemaDir)).toBe(true); | ||
| const schemaFiles = readdirSync(schemaDir); | ||
| expect(schemaFiles).toContain('public.sql'); | ||
| const schema = readNormalizedFile(path.join(schemaDir, 'public.sql')); | ||
| const normalizedSchema = schema.toLowerCase(); | ||
| expect(normalizedSchema).toContain('create schema public;'); | ||
| expect(normalizedSchema).toContain('create table public.products'); | ||
| expect(normalizedSchema).not.toContain('set '); | ||
| expect(existsSync(path.join(outDir, 'schema.sql'))).toBe(false); | ||
| } finally { | ||
| if (previousDatabaseUrl === undefined) { | ||
| delete process.env.DATABASE_URL; | ||
| } else { | ||
| process.env.DATABASE_URL = previousDatabaseUrl; | ||
| } | ||
| } |
There was a problem hiding this comment.
DATABASE_URL override does not reach the spawned CLI
runCli always uses the snapshot cliEnv captured at module load time, but this test mutates process.env.DATABASE_URL. That change won’t be visible to the child process, so the CLI may still see no DATABASE_URL despite the assignment here.
Consider either:
- Updating
cliEnvin the test instead ofprocess.env, with proper restore:
const previousDatabaseUrl = cliEnv.DATABASE_URL;
cliEnv.DATABASE_URL = connectionString;
// runCli(...)
if (previousDatabaseUrl === undefined) {
delete cliEnv.DATABASE_URL;
} else {
cliEnv.DATABASE_URL = previousDatabaseUrl;
}or
- Refactoring
runClito accept env overrides and merge them into{ ...cliEnv, ...overrides }per invocation.
Without this, the test’s intent (driving the CLI via DATABASE_URL) isn’t reliably met.
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.