feat: gate schema changes behind an explicit migrate - #646
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe change adds automatic or deferred schema migration across core storage, workflow storage, the server, and the Java, Node, and Python SDKs. Explicit ChangesMigration flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant QueueSDK
participant NativeBinding
participant StorageBackend
CLI->>QueueSDK: open with automatic migration disabled
QueueSDK->>NativeBinding: migrate()
NativeBinding->>StorageBackend: apply core and workflow migrations
StorageBackend-->>NativeBinding: MigrationReport
NativeBinding-->>CLI: migration report as JSON
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/taskito-core/src/storage/mod.rs`:
- Around line 1303-1314: Update StorageBackend::migrate in
crates/taskito-core/src/storage/mod.rs:1303-1314 to call
ensure_contract_supported(self) after the selected backend migration succeeds
and before returning the MigrationReport. Update
crates/taskito-core/BINDING_CONTRACT.md:402-404 to document that
StorageBackend::migrate performs this deferred contract check and that gated
shells must validate an already-migrated schema before processing work.
In `@crates/taskito-java/src/queue/admin.rs`:
- Around line 479-480: Update the migration flow containing
queue.storage.migrate() and migrate_workflow_storage(queue) to acquire a
database-scoped migration lock before either operation and release it only after
both complete, covering the entire core and workflow migration sequence. Use the
database’s cross-process locking mechanism rather than the existing
workflow_init lock or an in-process Java mutex, while preserving error
propagation and cleanup.
In `@crates/taskito-node/src/queue/mod.rs`:
- Around line 49-57: Update the contract-floor enforcement around
ensure_contract_supported so it runs for existing schemas even when auto_migrate
is false, while skipping only when the migration ledger is absent. Preserve the
auto_migrate behavior and use a non-DDL readiness or ledger-presence check to
distinguish unmigrated storage from existing storage before validating the
contract floor.
In `@crates/taskito-server/src/config/backend.rs`:
- Around line 51-60: Update the backend startup validation around
ensure_contract_supported to also perform a non-DDL workflow schema readiness
check when auto_migrate is false, so gated startup rejects databases missing
workflow tables. Add coverage in crates/taskito-server/src/config/backend.rs
lines 173-198 for migrated core tables with absent workflow tables and assert
the gated open fails. In
docs/content/docs/shared/guides/operations/deployment.mdx lines 1748-1752,
retain the startup guarantee only with the workflow check in place, or narrow it
to core storage.
In `@sdks/python/taskito/cli.py`:
- Around line 457-460: Update run_migrate and the _load_queue construction path
so the application opens Queue with auto_migrate=False before calling
queue.migrate(). Use a CLI-safe application factory or pre-construction
override, ensuring importing the application cannot apply migrations implicitly
and the command’s report reflects the full explicit migration.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 76c84405-ae13-484b-b15d-559a4cab116a
📒 Files selected for processing (47)
crates/taskito-core/BINDING_CONTRACT.mdcrates/taskito-core/src/storage/migrate.rscrates/taskito-core/src/storage/mod.rscrates/taskito-core/src/storage/postgres/mod.rscrates/taskito-core/src/storage/redis_backend/mod.rscrates/taskito-core/src/storage/sqlite/mod.rscrates/taskito-core/src/storage/sqlite/tests.rscrates/taskito-java/src/backend.rscrates/taskito-java/src/convert.rscrates/taskito-java/src/queue/admin.rscrates/taskito-node/src/backend.rscrates/taskito-node/src/config.rscrates/taskito-node/src/queue/admin.rscrates/taskito-node/src/queue/mod.rscrates/taskito-node/src/queue/workflows.rscrates/taskito-python/src/py_queue/mod.rscrates/taskito-python/src/py_queue/workflow_ops/mod.rscrates/taskito-python/src/py_queue/workflow_ops/test_helpers.rscrates/taskito-server/src/config/backend.rscrates/taskito-server/src/config/mod.rscrates/taskito-server/src/runtime/mod.rscrates/taskito-server/tests/backends.rscrates/taskito-workflows/src/lib.rscrates/taskito-workflows/src/postgres_store.rscrates/taskito-workflows/src/sqlite_store.rsdocs/content/docs/shared/guides/operations/deployment.mdxsdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.javasdks/java/src/main/java/org/byteveda/taskito/Taskito.javasdks/java/src/main/java/org/byteveda/taskito/cli/Cli.javasdks/java/src/main/java/org/byteveda/taskito/internal/JniQueueBackend.javasdks/java/src/main/java/org/byteveda/taskito/internal/NativeQueue.javasdks/java/src/main/java/org/byteveda/taskito/model/MigrationReport.javasdks/java/src/main/java/org/byteveda/taskito/spi/QueueBackend.javasdks/java/src/test/java/org/byteveda/taskito/MigrateTest.javasdks/node/src/cli/commands/index.tssdks/node/src/cli/commands/migrate.tssdks/node/src/cli/connect.tssdks/node/src/cli/index.tssdks/node/src/native.tssdks/node/src/queue.tssdks/node/test/core/migrate.test.tssdks/python/taskito/_taskito.pyisdks/python/taskito/app.pysdks/python/taskito/cli.pysdks/python/taskito/mixins/settings.pysdks/python/taskito/webhooks.pysdks/python/tests/core/test_migrate.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
ByteVeda/taskito(manual)
pratyush618
force-pushed
the
feat/explicit-migrate
branch
from
August 10, 2026 17:07
c14cca6 to
70c6ac0
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/taskito-node/src/queue/admin.rs`:
- Around line 273-295: Convert the Node-facing QueueAdmin::migrate method and
its migration flow to asynchronous execution, running storage migration and
migrate_workflow_storage inside spawn_blocking so database and archive work
leave the event loop; preserve both workflow and non-workflows configurations,
including a compatible helper signature. Update the queue TypeScript API,
migrate CLI command, and migration tests to await the returned Promise.
In `@crates/taskito-python/src/py_queue/mod.rs`:
- Around line 705-724: Update migrate to call
ensure_contract_supported(&self.storage) immediately after the core
self.storage.migrate() succeeds and before
workflow_ops::migrate_workflow_storage, propagating any validation error through
PyResult. Add a regression test covering storage that skipped construction-time
validation and then explicitly migrates, verifying the contract floor is
enforced.
In `@sdks/python/taskito/app.py`:
- Around line 366-369: Update PyQueue initialization and WebhookManager setup at
sdks/python/taskito/app.py:366-369 to use actual schema readiness, preloading
webhooks for storage migrated by a prior process and deferring only
never-migrated schema-backed storage; do not use auto_migrate as the readiness
signal. At sdks/python/taskito/app.py:242-247, restrict unavailable-table
warnings to never-migrated SQLite/PostgreSQL storage and document that
schemaless Redis remains usable. Add coverage for a fresh gated runtime opened
after a separate process migrates the database.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9fc5a08e-e2a2-488f-8322-3c179e9e96b8
📒 Files selected for processing (18)
crates/taskito-core/BINDING_CONTRACT.mdcrates/taskito-core/src/contract.rscrates/taskito-core/src/storage/mod.rscrates/taskito-core/src/storage/postgres/mod.rscrates/taskito-core/src/storage/redis_backend/mod.rscrates/taskito-core/src/storage/sqlite/mod.rscrates/taskito-core/src/storage/sqlite/tests.rscrates/taskito-java/src/backend.rscrates/taskito-java/src/queue/admin.rscrates/taskito-node/src/queue/admin.rscrates/taskito-node/src/queue/mod.rscrates/taskito-python/src/py_queue/mod.rscrates/taskito-server/src/config/backend.rsdocs/content/docs/shared/guides/operations/deployment.mdxsdks/python/taskito/app.pysdks/python/taskito/cli.pysdks/python/taskito/migration_gate.pysdks/python/tests/core/test_contract_floor.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
ByteVeda/taskito(manual)
🚧 Files skipped from review as they are similar to previous changes (9)
- crates/taskito-core/BINDING_CONTRACT.md
- crates/taskito-core/src/storage/redis_backend/mod.rs
- docs/content/docs/shared/guides/operations/deployment.mdx
- crates/taskito-node/src/queue/mod.rs
- crates/taskito-java/src/queue/admin.rs
- sdks/python/taskito/cli.py
- crates/taskito-server/src/config/backend.rs
- crates/taskito-core/src/storage/sqlite/tests.rs
- crates/taskito-java/src/backend.rs
An explicit migrate has to say what it did; the runners returned unit, so the caller could only report success.
Opening was the only way to migrate, so a deployment whose credentials forbid runtime DDL had nowhere to stand. The unmigrated constructors and migrate() split the two, sweep included.
Each shell gains an auto-migrate open option, threaded into the workflow store too — otherwise the first workflow call applies the DDL the operator withheld. A gated open skips the contract-floor read, which needs the schema.
The gated open needs a way to apply the schema: a migrate() reporting what ran, and a CLI command that opens unmigrated so it is the only thing applying DDL.
Skipping it whenever migrations were gated let a build too old for the deployment join it — the schema is usually already there, so the floor is readable. Only a never-migrated storage is exempt, and migrate() covers that case.
The Python command reaches its queue by importing the application, which built a migrating queue — so the import applied the schema and the command reported work it had not done.
A fresh database means the whole schema plus the backlog sweep, which is unbounded work for a synchronous binding to hold the loop for.
Gating migrations does not mean the schema is missing: the normal flow migrates in one process and starts the application in another, still gated, against a fully migrated database — whose webhook snapshot then never loaded.
pratyush618
force-pushed
the
feat/explicit-migrate
branch
from
August 10, 2026 17:33
70c6ac0 to
25d5bf2
Compare
kartikeya-27
approved these changes
Aug 10, 2026
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
2 participants
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.
Opening a queue was the only way to migrate it, and opening always migrated. A deployment whose application credentials do not permit DDL — or where a DBA owns every schema change — had nowhere to stand: the first process to start applied the migrations whether or not anyone had approved them.
This splits the two. The default is unchanged.
Opening without applying
SqliteStorage::unmigratedandPostgresStorage::unmigratedconnect and stop there; the migrating constructors now route through the same builder with the flag set. Each shell exposes it as an open option:Queue(auto_migrate=False)new Queue({ autoMigrate: false })Taskito.builder().autoMigrate(false)taskito-serverreadsTASKITO_AUTO_MIGRATE.The flag is threaded into the workflow store too. That store is built lazily on first workflow use and migrates in its constructor, so without this the first workflow call would quietly apply the DDL the operator withheld. It gained the same
unmigrated/migratepair, andWorkflowStorageBackend::migratedispatches across backends.Applying explicitly
StorageBackend::migrate()applies pending changes and returns aMigrationReport: the core versions applied, the workflow versions applied, the rows the one-time backlog sweep archived, andschemalessfor a backend thatstores no schema and never will. To make that report possible,
run_sqlite/run_postgresnow return the versions they applied instead of unit.The backlog sweep moved with it.
archive_old_jobs(i64::MAX)used to run alongside the migrations, outside the ledger; it now lives insidemigrate(), so a gated deployment cannot skip it, and the report counts what it moved.Exposed as
migrate()in all three SDKs, plus amigrateCLI command that opens unmigrated — the command is then the only thing in the system applying DDL:Idempotent: a current database applies nothing and says so. Redis reports "nothing to migrate" rather than failing.
Interaction with the contract floor
Reading the floor needs the settings table, which a gated open has not created. So a gated open skips the check and
migrate()performs it once the schema exists. The server keeps its check unconditional — it is meant to run against an already-migrated database — but a missing schema now fails namingTASKITO_AUTO_MIGRATEinstead of leaking a bare "no such table".Verification
cargo test --workspaceand--features workflowsgreen; clippy clean including--all-targets --all-features; Postgres and Redis both compile../gradlew buildgreen. Docs lint + typecheck clean.Beyond the plan on purpose: workflow tables and the server. A migrate that leaves half the schema unapplied does not deliver the gate.
Summary by CodeRabbit
New Features
migratecommands to the Node.js, Python, and Java CLIs.Documentation
Bug Fixes