Embedded mode foundation (Plan 1)#12
Draft
knutties wants to merge 21 commits into
Draft
Conversation
Captures the brainstormed design for exposing Kronos's job-management functionality as Rust library crates (kronos-client + kronos-embedded-worker) that host applications embed directly, removing the need to operate the API server and worker as separate deployable services. Key decisions: Rust-only for v1; single-tenant ergonomics via workspace pinning while preserving the schema-per-workspace DB layout; pg_cron remains required for CRON triggers; service crates refactored to sit on top of the new library crates so there is one code path for both modes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Adds two cross-cutting sections raised on the draft PR: - Schema namespacing: parameterize migrations and runtime SQL on system_schema and tenant_schema_prefix. Service binaries default to "public" / "" (zero change for existing deployments). Library defaults to "kronos" / "kronos_" (avoids host-app collisions on common names like organizations / workspaces). - Coexistence with other DB libraries (Diesel, etc.): library is internally sqlx-only; hosts using Diesel run with two pools against the same Postgres database. For atomicity-critical flows, document the transactional outbox pattern. Connection-agnostic executor trait is explicitly out of v1 scope. Updates Backwards-compatibility, Risks, Phasing, and Testing sections to stay consistent with the new decisions. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Closes a small ambiguity: the spec showed KronosClient::builder(pool) and Worker::builder(pool) without naming the type. Make it explicit that pool is sqlx::PgPool (taken by value, internally Arc-cloned). Also tighten the ORM coexistence section to spell out that the host's Diesel r2d2/bb8 pool holding diesel::PgConnection stays separate from the sqlx::PgPool the host hands to Kronos. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the rough phase sketch with an explicit four-plan grouping that maps directly to four sequential PRs, each leaving the system in a green state with all existing integration tests passing. - Plan 1 (Foundation): new crates + schema parameterization - Plan 2 (Worker extraction): move poller/pipeline/backoff/dispatcher - Plan 3 (Client extraction): build kronos-client and cut over handlers - Plan 4 (Embedded validation + docs): E2E test + README updates Each plan has explicit "ships when" criteria, dependencies between plans are stated, and out-of-band follow-ups are listed for context. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Twelve bite-sized tasks covering crate scaffolding (kronos-client,
kronos-embedded-worker), the SchemaConfig value type, migration
templating with {{system_schema}}/{{tenant_schema_prefix}}
placeholders, parameterized public-schema migrations, an embedded
migration runner with apply()/MigrateError, kronos-migrate CLI,
SchemaRegistry update for the configured system schema, AppConfig
SchemaEnv (TE_SYSTEM_SCHEMA / TE_TENANT_SCHEMA_PREFIX), justfile
rewiring, and a smoke test that runs migrations into a kronos_smoke
namespace. Defaults preserve service behaviour byte-identically.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ig tests Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…stem_schema Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add `system_schema: String` field to `SchemaRegistry` and thread it through `new()`. `get_active_schemas` now builds the query with `quote_ident` instead of hard-coding `public.workspaces`. Add private `quote_ident` helper (defense in depth on top of the upstream `SchemaConfig::validate` check). Update the only call site in `crates/worker/src/poller.rs` to pass `"public".to_string()` so service-mode behaviour is unchanged until Task 8 wires `AppConfig`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…schema_config Save+restore env vars in picks_up_non_default_values to match the pattern used in defaults_match_today, preventing developer-set vars from leaking if an assertion panics. Also adds a brief doc comment to to_schema_config explaining its role and the validate() contract. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Exposes `kronos_client::migrate(&pool, &opts)` as the public library entry point wrapping `kronos_common::migrations::apply`, and adds the `kronos-migrate` binary driven by CLI flags / env vars (TE_DATABASE_URL, TE_SYSTEM_SCHEMA, TE_TENANT_SCHEMA_PREFIX) for use in the `just db-migrate` recipe.
…giene - Add hide_env_values = true to --database-url to prevent Postgres URLs (which often embed passwords) from leaking into --help output - Validate SchemaConfig before connecting so bad input fails fast with a clear error instead of an obscure RenderError::InvalidConfig later - Replace println! success message with tracing::info! for structured logging consistency with the rest of the codebase Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace four raw psql calls with a single `cargo run -p kronos-client --bin kronos-migrate` invocation. Defaults to system_schema=public and tenant_schema_prefix="" to preserve existing behaviour. Also fix the migration apply() runner to split SQL on statement boundaries (respecting dollar-quoted blocks and single-quoted strings) before executing each statement individually. This allows CREATE INDEX CONCURRENTLY to succeed because PostgreSQL rejects it when it runs inside an implicit transaction created by the simple-query protocol for multi-statement strings. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds an integration test that applies migrations under system_schema="kronos_smoke" and asserts organizations and workspaces tables exist in that schema, not public. This is the load-bearing check that Tasks 4-7 parameterization works for embedded-mode consumers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
Author
|
Plan 1 (Foundation) is now committed on this branch:
Plan 2 (Worker extraction) will follow. |
knutties
changed the title
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.
This PR lands the foundation for Kronos embedded mode: design spec plus Plan 1 implementation. Worker extraction (Plan 2) is the follow-up, stacked on top of this branch.
Stacking
feat/embedded-mode→main. Design spec + Plan 1 (Foundation).feat/worker-extraction→feat/embedded-mode. Plan 2 (Worker extraction).What's in this PR
Design spec (
docs/superpowers/specs/2026-04-29-kronos-embedded-mode-design.md) — captures the full embedded-mode initiative.Plan 1: Foundation (
docs/superpowers/plans/2026-04-29-plan-1-foundation.md) — groundwork that does NOT change service-mode behavior:kronos-client,kronos-embedded-worker.SchemaConfigvalue type inkronos-common(system_schema,tenant_schema_prefix).{{system_schema}}placeholders. Service default"public"preserves byte-identical behavior.apply()entry point inkronos-common.kronos_client::migrate(&pool, &opts)API andkronos-migrateCLI binary (clap).SchemaConfigplumbed throughAppConfigfromTE_SYSTEM_SCHEMA/TE_TENANT_SCHEMA_PREFIXenv vars.justfile db-migraterecipe now invokeskronos-migrateinstead of rawpsql.apply()now splits SQL on statement boundaries (respecting$$ ... $$and'...'quoting) soCREATE INDEX CONCURRENTLYworks under PostgreSQL's simple-query protocol. Has unit-test coverage.crates/client/tests/migrate_kronos_namespace.rs) verifies non-defaultsystem_schema = "kronos_smoke"lands tables in the configured namespace.What does not change in this PR
TE_*environment variable contract for the binaries (only adds two new ones with defaults that preserve today).Test plan
cargo build --workspace— greencargo test -p kronos-common— green (includes 8split_statementsunit tests)cargo test -p kronos-client --test migrate_kronos_namespace -- --ignored— passes against live DB (kronos_smokenamespace)just db-reset— all four migrations apply cleanly with default configjust test-e2eregression check (deferred — recipe referenceskronos-schedulercrate not on this branch; pre-existing branch state)Review focus
kronos-clientandkronos-embedded-worker(the latter is still empty in this PR — Plan 2 fills it).SchemaConfigvalue type and the{{system_schema}}template approach.split_statementsfix should be a separate PR — happy to split if preferred.🤖 Generated with Claude Code