Skip to content

fix(api): disable synchronize:true in all environments#216

Merged
FSM1 merged 3 commits into
mainfrom
chore/disable-synchronize-true
Feb 27, 2026
Merged

fix(api): disable synchronize:true in all environments#216
FSM1 merged 3 commits into
mainfrom
chore/disable-synchronize-true

Conversation

@FSM1

@FSM1 FSM1 commented Feb 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • Set synchronize: false in all environments (was true for dev/test)
  • Added migrationsRun: true so migrations auto-run on app start, replacing the auto-sync behavior
  • Added migrations glob path to TypeORM config so it finds migration files
  • Added migrate:dev convenience script to package.json
  • Updated DATABASE_EVOLUTION_PROTOCOL.md environment behavior matrix and related sections

Context

synchronize: true in dev/test masked missing CREATE TABLE migrations — tables were auto-created from entity decorators, so missing migrations only surfaced on staging deploys. This caused the Phase 14 staging failure (PR #186).

With this change, the dev server fails immediately if a new entity lacks a migration, catching the gap at development time.

All 13 entities verified to have corresponding CREATE TABLE migrations — no gaps exist.

Test plan

  • API builds successfully (pnpm --filter api build)
  • All 615 unit tests pass (pnpm --filter api test)
  • Verified all 13 entities have matching CREATE TABLE migrations
  • CI passes (unit tests use mocked repos, no real DB connection affected)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added migrate:dev npm script to run database migrations in development.
  • Documentation

    • Updated database evolution protocol: schema changes must be via migrations, synchronize is disabled everywhere, and migrations now run automatically on startup (missing migrations surface at startup).
  • Chores

    • Removed one pending TODO from planning state (13 → 12).

Replace TypeORM synchronize:true (dev/test) with synchronize:false
and migrationsRun:true in all environments. This ensures missing
CREATE TABLE migrations surface immediately during development
instead of only failing on staging deploys.

- Set synchronize: false, migrationsRun: true in app.module.ts
- Add migrations glob path so TypeORM finds migration files
- Add migrate:dev convenience script to package.json
- Update DATABASE_EVOLUTION_PROTOCOL.md environment behavior matrix

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: 2409dd078846
@coderabbitai

coderabbitai Bot commented Feb 27, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@FSM1 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 8 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 12984d6 and 49aeb2f.

📒 Files selected for processing (2)
  • .planning/STATE.md
  • .planning/todos/pending/2026-02-27-ci-migration-drift-check.md

Walkthrough

Disables TypeORM's automatic schema sync and enables running explicit migrations on startup (migrationsRun: true). Adds a developer npm script to run migrations and updates documentation and project planning state to reflect the migration-first workflow.

Changes

Cohort / File(s) Summary
TypeORM config & startup
apps/api/src/app.module.ts
Set synchronize: false, added migrations: [join(__dirname, 'migrations','*.js')] and migrationsRun: true in TypeOrmModule.forRootAsync (startup now runs migrations). Review startup/migration ordering and migration file patterns.
Dev scripts
apps/api/package.json
Added migrate:dev script invoking TypeORM CLI via pnpm: pnpm typeorm migration:run -d src/data-source.ts. Verify CLI path and dev workflow.
Database documentation
docs/DATABASE_EVOLUTION_PROTOCOL.md
Rewrote protocol to enforce migration-driven schema changes across all environments; updated environment matrix, dangerous patterns, and deployment guidance.
Project planning state
.planning/STATE.md
Removed one TODO item (disabled an entry referencing disabling synchronize in dev/CI).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as "Application\n(startup)"
  participant TypeORM as "TypeORM\nMigration Runner"
  participant DB as "Database"

  App->>TypeORM: initialize TypeOrmModule (migrationsRun: true)
  Note right of TypeORM: load migrations from\n`dist/migrations/*.js`
  TypeORM->>DB: connect
  TypeORM->>DB: run pending migrations
  DB-->>TypeORM: migration results
  TypeORM-->>App: startup completes
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly matches the main objective: disabling synchronize:true in all environments to surface missing migrations during development.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/disable-synchronize-true

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/DATABASE_EVOLUTION_PROTOCOL.md (1)

341-341: ⚠️ Potential issue | 🟡 Minor

Minor: Incorrect line reference.

The reference states "lines 83-85 for synchronize setting" but in the updated app.module.ts, the synchronize/migrations/migrationsRun settings are at lines 85-87.

📝 Suggested fix
-- **TypeORM config:** `apps/api/src/app.module.ts` (lines 83-85 for `synchronize` setting)
+- **TypeORM config:** `apps/api/src/app.module.ts` (lines 85-87 for `synchronize` setting)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/DATABASE_EVOLUTION_PROTOCOL.md` at line 341, Update the line reference
in docs/DATABASE_EVOLUTION_PROTOCOL.md: change the mention of "lines 83-85 for
`synchronize` setting" to the correct lines (where `synchronize`, `migrations`,
and `migrationsRun` are currently located) by referencing the
`apps/api/src/app.module.ts` block that contains the TypeORM config (the
`synchronize`, `migrations`, and `migrationsRun` settings) so the doc points to
the correct lines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@docs/DATABASE_EVOLUTION_PROTOCOL.md`:
- Line 341: Update the line reference in docs/DATABASE_EVOLUTION_PROTOCOL.md:
change the mention of "lines 83-85 for `synchronize` setting" to the correct
lines (where `synchronize`, `migrations`, and `migrationsRun` are currently
located) by referencing the `apps/api/src/app.module.ts` block that contains the
TypeORM config (the `synchronize`, `migrations`, and `migrationsRun` settings)
so the doc points to the correct lines.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f19d234 and a1ce00c.

📒 Files selected for processing (5)
  • .planning/STATE.md
  • .planning/todos/done/2026-02-22-disable-synchronize-true-all-envs.md
  • apps/api/package.json
  • apps/api/src/app.module.ts
  • docs/DATABASE_EVOLUTION_PROTOCOL.md

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: 47b65ee5080c

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
docs/DATABASE_EVOLUTION_PROTOCOL.md (1)

341-341: Consider removing hardcoded line numbers for maintainability.

The reference includes specific line numbers (lines 85-87) which will become stale as app.module.ts evolves. Consider using a more general reference such as:

- **TypeORM config:** `apps/api/src/app.module.ts` (TypeORM configuration block for `synchronize`/`migrations`/`migrationsRun`)

This maintains clarity while reducing maintenance burden.

♻️ Proposed refactor
-- **TypeORM config:** `apps/api/src/app.module.ts` (lines 85-87 for `synchronize`/`migrations`/`migrationsRun`)
+- **TypeORM config:** `apps/api/src/app.module.ts` (TypeORM configuration block for `synchronize`/`migrations`/`migrationsRun`)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/DATABASE_EVOLUTION_PROTOCOL.md` at line 341, Remove the hardcoded
line-number reference in the TypeORM config callout and instead point readers to
the TypeORM configuration block in apps/api/src/app.module.ts; update the line
to something like "`apps/api/src/app.module.ts` (TypeORM configuration block for
`synchronize`/`migrations`/`migrationsRun`)" so the reference remains accurate
as app.module.ts changes and still highlights the `synchronize`, `migrations`,
and `migrationsRun` settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@docs/DATABASE_EVOLUTION_PROTOCOL.md`:
- Line 341: Remove the hardcoded line-number reference in the TypeORM config
callout and instead point readers to the TypeORM configuration block in
apps/api/src/app.module.ts; update the line to something like
"`apps/api/src/app.module.ts` (TypeORM configuration block for
`synchronize`/`migrations`/`migrationsRun`)" so the reference remains accurate
as app.module.ts changes and still highlights the `synchronize`, `migrations`,
and `migrationsRun` settings.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1ce00c and 12984d6.

📒 Files selected for processing (1)
  • docs/DATABASE_EVOLUTION_PROTOCOL.md

Track idea to add a CI job that runs typeorm migration:generate
against a fresh database to detect entity-vs-migration drift
automatically.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: 1f923be1abe1
@FSM1 FSM1 enabled auto-merge (squash) February 27, 2026 14:31
@FSM1 FSM1 merged commit 4b4a3b3 into main Feb 27, 2026
14 checks passed
@FSM1 FSM1 deleted the chore/disable-synchronize-true branch March 4, 2026 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant