diff --git a/README.md b/README.md index 2f400cf..1dcc160 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,9 @@ workers: db: primary # database name in the database worker dir: ./migrations # folder of YYYYMMDDHHMMSS_slug.sql files auto: false # true = run migrate::up at startup - types_out: ./db.types.ts # codegen output (optional) + types_out: ./db.types.ts # codegen output (optional); when set, + # types regenerate after each migrate::up + # (opt out with codegen_on_up: false) ``` The `config:` block is a first-boot seed; afterwards settings live in the diff --git a/docs/changelog.md b/docs/changelog.md index 57a6fc5..b3f13c9 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,6 +2,22 @@ ## Unreleased +- `migrate::create` keeps timestamps monotonic with the files already on + disk: when the latest existing migration is dated ahead of the clock (a + hand-written name), the new file is stamped one second after it instead of + sorting before an applied migration. +- Future-dated migration files are surfaced: `migrate::status` returns a new + `future_dated` list and `migrate::up` logs a warning before applying such + a file (5-minute clock-skew tolerance). +- New `codegen_on_up` configuration: after a `migrate::up` that applied at + least one migration, `migrate::codegen` runs automatically so generated + types never drift from the schema. Enabled by default when `types_out` is + set; codegen failures are logged and never fail the run. `migrate::up` now + returns the written path as `types_path`. +- Agent skill rewritten: triggers on any database/schema work, and states + the hard rules (always scaffold with `migrate::create`, one migration per + logical change, never edit applied files). + - Revert the default log filter introduced in 0.1.2 that silenced `iii-helpers`' OTel connection module. Those transient pre-connection ERROR logs originate in `iii-helpers` (out of miiigrate's scope) and diff --git a/docs/configuration.md b/docs/configuration.md index 6cb5459..1702b1e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -27,6 +27,7 @@ config change, restart the worker (or the engine). | `dir` | `./migrations` | Migration folder, resolved against the worker's working directory. Only `migrate::create` creates it. | | `auto` | `false` | Run `migrate::up` once at startup. If the `database` worker is not registered yet (no ordering guarantee under docker compose or engine-managed boots), the run is retried with capped backoff for up to 2 minutes. Best-effort: any other failure is logged and the worker stays up, so `migrate::status` remains available for diagnosis. | | `types_out` | — | Output path for `migrate::codegen`. Optional; without it, codegen requires `out` in the payload. | +| `codegen_on_up` | follows `types_out` | Run `migrate::codegen` after a `migrate::up` that applied at least one migration. Defaults to `true` when `types_out` is set, `false` otherwise; set explicitly to override. Codegen failures are logged and never fail the up run. | | `dialect` | auto | `postgres` or `sqlite`. By default the dialect is detected from the `database` worker's `database::listDatabases` driver field. | ## Example diff --git a/docs/functions.md b/docs/functions.md index 1b06cd7..1e31e1f 100644 --- a/docs/functions.md +++ b/docs/functions.md @@ -16,7 +16,9 @@ migrate::create ──▶ write your SQL ──▶ migrate::up ──▶ migrate Apply every pending migration, in file-name order. - **Payload**: `{}` -- **Returns**: `{ "applied": [""], "skipped": , "duration_ms": }` +- **Returns**: `{ "applied": [""], "skipped": , "duration_ms": , "types_path": "…" }` + — `types_path` is present only when the automatic post-run codegen wrote it + (see `codegen_on_up` in the configuration). Behaviour: @@ -31,6 +33,14 @@ Behaviour: - Statement splitting is dialect-aware: a `;` inside `'strings'`, `E'…'` escapes, `"identifiers"`, `$tag$…$tag$` bodies (plpgsql functions), or nested `/* comments */` never splits a statement. +- When `codegen_on_up` is enabled (default whenever `types_out` is set) and + at least one migration was applied, `migrate::codegen` runs afterwards so + the generated types never drift from the schema. A codegen failure is + logged and reported by the absent `types_path`, never rolled into the run: + the migrations stay applied. +- A pending file whose timestamp prefix is ahead of the wall clock (beyond a + 5-minute skew tolerance) is applied but logged as a warning: it is almost + certainly a hand-written name — scaffold with `migrate::create` instead. ## `migrate::status` @@ -48,7 +58,8 @@ time, including while `migrate::up` is failing. "applied": [{ "name": "…", "checksum": "…", "applied_at": "…" }], "pending": [{ "name": "…", "checksum": "…" }], "mismatched": [{ "name": "…", "applied_checksum": "…", "file_checksum": "…", "applied_at": "…" }], - "missing": [{ "name": "…", "checksum": "…", "applied_at": "…" }] + "missing": [{ "name": "…", "checksum": "…", "applied_at": "…" }], + "future_dated": ["…"] } ``` @@ -57,6 +68,10 @@ time, including while `migrate::up` is failing. - `mismatched` — applied, but the file content changed since. `migrate::up` refuses to run while this list is non-empty. - `missing` — recorded as applied but the file no longer exists on disk. +- `future_dated` — files on disk whose timestamp prefix is ahead of the wall + clock beyond a 5-minute skew tolerance: hand-written names. Harmless once + applied (`migrate::create` keeps new timestamps monotonic with them), but a + pending one should be renamed before apply. ## `migrate::create` @@ -66,8 +81,12 @@ migrations directory if it does not exist yet. - **Payload**: `{ "name": "add_users" }` — slug of `[A-Za-z0-9_-]+` - **Returns**: `{ "name": "20260719143000_add_users.sql", "path": "./migrations/20260719143000_add_users.sql" }` -The timestamp is the current UTC time. The file starts with a forward-only -header comment; write your SQL below it. +The timestamp is the current UTC time, kept monotonic with the files already +on disk: when the latest existing migration is dated ahead of the clock (a +hand-written name), the new file is stamped one second after it so +lexicographic order — which is the apply order — is preserved. Never write +migration file names by hand; always scaffold here. The file starts with a +forward-only header comment; write your SQL below it. ## `migrate::codegen` diff --git a/skills/SKILL.md b/skills/SKILL.md index 3de7127..b221603 100644 --- a/skills/SKILL.md +++ b/skills/SKILL.md @@ -1,9 +1,14 @@ --- name: miiigrate description: >- - Apply, track, and scaffold forward-only SQL migrations through the iii - database worker, and generate TypeScript types that match the schema as it - crosses the wire. Reach for it whenever a schema change is needed. + Database schema management for iii projects: forward-only SQL migrations + and generated TypeScript types, driven through the iii database worker. + Use this skill whenever a task touches the database in any way — creating, + altering, or dropping tables, columns, indexes, enums, or constraints; + checking which migrations are applied; or typing rows returned by + database::query. Load it BEFORE writing any SQL or DDL, creating any file + in the migrations directory, or calling the database worker for a schema + change. --- # miiigrate @@ -21,22 +26,39 @@ replicas are safe: batches serialize on `pg_advisory_xact_lock` (Postgres) or `BEGIN IMMEDIATE` (SQLite), and a replica that loses the race counts the migration as skipped, not failed. +## Rules + +- ALWAYS scaffold with `migrate::create`. NEVER write a migration file name + by hand: the apply order is the lexicographic file-name order, and only + `migrate::create` guarantees unique, monotonic timestamps. A hand-written + timestamp (e.g. a rounded "120000") lands in the future and corrupts the + ordering for every migration that follows. +- One migration per logical change ("add events table", "add user names") — + not one file bundling every change of the session. +- NEVER edit or rename an applied migration file. To undo or amend, create a + new forward migration. +- Apply with `migrate::up`, then check the response: when `types_path` is + present the TypeScript types were already regenerated; when it is absent + and the project has a `types_out`, run `migrate::codegen` yourself. +- Before any schema work, call `migrate::status` and resolve `mismatched`, + `missing`, or `future_dated` entries before adding new migrations. + ## When to Use -- The user asks to add, change, or drop a table, column, or index — scaffold - with `migrate::create`, write the SQL, apply with `migrate::up`. -- You need to know the current schema state: what is applied, pending, or has - drifted (`migrate::status`, read-only, safe anytime). -- TypeScript code reads rows from `database::query` and needs types that match - the JSON wire format (`migrate::codegen`). -- A `migrate::up` run failed and you need the failing file and statement index - (`MIGRATION_FAILED` carries both, plus the database worker's error). +- Any request to add, change, or drop a table, column, index, enum, or + constraint — scaffold with `migrate::create`, write the SQL, apply with + `migrate::up`. +- You need the current schema state: applied, pending, drifted, or + future-dated files (`migrate::status`, read-only, safe anytime). +- TypeScript code reads rows from `database::query` and needs types that + match the JSON wire format (`migrate::codegen`). +- A `migrate::up` run failed and you need the failing file and statement + index (`MIGRATION_FAILED` carries both, plus the database worker's error). ## Boundaries - Not an ORM or schema DSL — migrations are plain SQL you write yourself. - No down migrations by design. To undo, write a new forward migration. -- Never edit an applied migration file; always create a new one. - Postgres and SQLite only; MySQL is rejected with `UNSUPPORTED_DIALECT`. - Dialect-specific SQL is your responsibility (e.g. SQLite cannot `ALTER COLUMN TYPE`; use the table-recreate pattern inside one migration — @@ -47,18 +69,20 @@ migration as skipped, not failed. ## Functions - `migrate::up` — apply all pending migrations in order; returns - `{ applied, skipped, duration_ms }`. Refuses to run while any applied file - has drifted on disk. + `{ applied, skipped, duration_ms, types_path? }`. Refuses to run while any + applied file has drifted on disk. With `codegen_on_up` (default when + `types_out` is set), regenerates types after a run that applied anything. - `migrate::status` — read-only report `{ applied, pending, mismatched, - missing }` with names, checksums, and apply dates. + missing, future_dated }` with names, checksums, and apply dates. - `migrate::create` — payload `{ name: "add_users" }`; creates - `/_add_users.sql` and returns `{ name, path }`. Write - the SQL into that file before calling `migrate::up`. + `/_add_users.sql` with a unique, monotonic timestamp + and returns `{ name, path }`. Write the SQL into that file before calling + `migrate::up`. - `migrate::codegen` — introspect the live schema through `database::query` and write TypeScript types to the configured `types_out` (or payload - `out`); returns `{ path, tables, enums }`. Regenerate after every applied - migration. + `out`); returns `{ path, tables, enums }`. -Configuration (`db`, `dir`, `auto`, `types_out`, `dialect`) lives in the -`configuration` worker under id `miiigrate` and is read once at startup. For -payload and response schemas, call `get function info` on the function id. +Configuration (`db`, `dir`, `auto`, `types_out`, `codegen_on_up`, `dialect`) +lives in the `configuration` worker under id `miiigrate` and is read once at +startup. For payload and response schemas, call `get function info` on the +function id. diff --git a/src/config.rs b/src/config.rs index e3f45ce..cb72525 100644 --- a/src/config.rs +++ b/src/config.rs @@ -70,6 +70,12 @@ pub struct WorkerConfig { /// SQL dialect override. Default: auto-detect via `database::listDatabases`. #[serde(default)] pub dialect: Option, + + /// Run `migrate::codegen` automatically after a `migrate::up` that + /// applied at least one migration. Default: enabled when `types_out` is + /// set, disabled otherwise. Codegen failures never fail the up run. + #[serde(default)] + pub codegen_on_up: Option, } fn default_db() -> String { @@ -88,11 +94,18 @@ impl Default for WorkerConfig { auto: false, types_out: None, dialect: None, + codegen_on_up: None, } } } impl WorkerConfig { + /// Effective `codegen_on_up`: explicit value wins, otherwise types are + /// regenerated whenever a `types_out` destination is configured. + pub fn codegen_on_up_enabled(&self) -> bool { + self.codegen_on_up.unwrap_or(self.types_out.is_some()) + } + pub fn from_yaml(yaml: &str) -> Result { serde_yml::from_str(yaml).map_err(|e| format!("yaml parse: {e}")) } @@ -147,6 +160,23 @@ mod tests { assert!(!cfg.auto); assert!(cfg.types_out.is_none()); assert!(cfg.dialect.is_none()); + assert!(cfg.codegen_on_up.is_none()); + } + + #[test] + fn codegen_on_up_defaults_follow_types_out() { + let mut cfg = WorkerConfig::default(); + assert!(!cfg.codegen_on_up_enabled()); // no types_out, no codegen + + cfg.types_out = Some("./db.types.ts".into()); + assert!(cfg.codegen_on_up_enabled()); // types_out set: on by default + + cfg.codegen_on_up = Some(false); // explicit opt-out wins + assert!(!cfg.codegen_on_up_enabled()); + + cfg.types_out = None; + cfg.codegen_on_up = Some(true); // explicit opt-in without types_out: + assert!(cfg.codegen_on_up_enabled()); // codegen will fail loudly (no out) } #[test] diff --git a/src/handlers/create.rs b/src/handlers/create.rs index 5eb8857..7f3eda6 100644 --- a/src/handlers/create.rs +++ b/src/handlers/create.rs @@ -43,9 +43,23 @@ pub async fn handle(state: &AppState, req: CreateReq) -> Result ts { + tracing::warn!( + latest = %latest.format("%Y%m%d%H%M%S"), + "latest migration on disk is ahead of the clock (hand-written \ + name?); continuing after it to keep ordering monotonic" + ); + ts = after_latest; + } + } let (name, path): (String, PathBuf) = loop { let name = format!("{}_{slug}.sql", ts.format("%Y%m%d%H%M%S")); let path = dir.join(&name); @@ -75,12 +89,26 @@ pub async fn handle(state: &AppState, req: CreateReq) -> Result Option> { + let entries = std::fs::read_dir(dir).ok()?; + entries + .filter_map(|e| e.ok()) + .filter_map(|e| e.file_name().into_string().ok()) + .filter(|name| migrations::validate_name(name).is_ok()) + .filter_map(|name| migrations::timestamp_of(&name)) + .max() +} + #[cfg(test)] mod tests { // The handler needs an AppState (IIIClient) so filesystem behavior is - // covered by the slug validation below plus the playground; slug rules - // are the pure part. - use crate::migrations::validate_name; + // covered by the pure helpers below plus the playground. + use super::latest_migration_timestamp; + use crate::migrations::{timestamp_of, validate_name}; #[test] fn generated_names_validate() { @@ -89,4 +117,28 @@ mod tests { assert!(validate_name(&name).is_ok(), "{name}"); } } + + #[test] + fn latest_timestamp_picks_max_and_ignores_strays() { + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("20260101000000_a.sql"), "").unwrap(); + std::fs::write(tmp.path().join("20991231235959_future.sql"), "").unwrap(); + std::fs::write(tmp.path().join("notes.sql"), "").unwrap(); // stray: ignored + std::fs::write(tmp.path().join("README.md"), "").unwrap(); + + assert_eq!( + latest_migration_timestamp(tmp.path()), + timestamp_of("20991231235959_future.sql") + ); + } + + #[test] + fn latest_timestamp_is_none_for_empty_or_missing_dir() { + let tmp = tempfile::tempdir().unwrap(); + assert_eq!(latest_migration_timestamp(tmp.path()), None); + assert_eq!( + latest_migration_timestamp(std::path::Path::new("/nonexistent/miiigrate")), + None + ); + } } diff --git a/src/handlers/status.rs b/src/handlers/status.rs index b5d4e11..1f5d79b 100644 --- a/src/handlers/status.rs +++ b/src/handlers/status.rs @@ -46,6 +46,11 @@ pub struct StatusResp { pub mismatched: Vec, /// Recorded as applied but the file no longer exists on disk. pub missing: Vec, + /// Files on disk whose timestamp prefix is ahead of the wall clock + /// (beyond skew tolerance) — almost certainly hand-written names. + /// Harmless once applied (`migrate::create` stays monotonic), but a + /// pending one should be renamed before apply. + pub future_dated: Vec, } pub async fn handle(state: &AppState, _req: StatusReq) -> Result { @@ -62,6 +67,19 @@ pub async fn handle(state: &AppState, _req: StatusReq) -> Result = files + .iter() + .filter(|f| migrations::is_future_dated(&f.name, now)) + .map(|f| f.name.clone()) + .collect(); + if !future_dated.is_empty() { + tracing::warn!( + count = future_dated.len(), + "migration files are dated in the future (hand-written names?)" + ); + } + for file in files { match applied_by_name.remove(&file.name) { None => pending.push(PendingEntry { @@ -95,5 +113,6 @@ pub async fn handle(state: &AppState, _req: StatusReq) -> Result, } /// Dialect-specific DDL for the tracking table. Idempotent. @@ -170,10 +177,21 @@ pub async fn handle(state: &AppState, _req: UpReq) -> Result Result types_path = Some(resp.path), + Err(e) => tracing::warn!( + error = %e, + "post-run codegen failed; migrations are applied — run \ + migrate::codegen manually" + ), + } + } + Ok(UpResp { applied, skipped, duration_ms: started.elapsed().as_millis() as u64, + types_path, }) } diff --git a/src/main.rs b/src/main.rs index e4e3964..93dc16a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -116,7 +116,9 @@ async fn main() -> Result<()> { .description( "Apply all pending migrations, each in one atomic database::transaction \ batch. Refuses to run when an applied file's checksum changed \ - (CHECKSUM_MISMATCH).", + (CHECKSUM_MISMATCH). When `codegen_on_up` is enabled (default with \ + `types_out` set), regenerates the TypeScript types after a run that \ + applied migrations.", ), ); } @@ -135,7 +137,8 @@ async fn main() -> Result<()> { }) .description( "Scaffold a new migration file `/_.sql` and \ - return its path.", + return its path. Always use this instead of hand-writing file names: \ + timestamps are guaranteed unique and monotonic with existing files.", ), ); } diff --git a/src/migrations.rs b/src/migrations.rs index 7ad2567..cf66073 100644 --- a/src/migrations.rs +++ b/src/migrations.rs @@ -10,6 +10,7 @@ //! rewritten with CRLF endings (Windows editors, `core.autocrlf`) keeps its //! identity; any other edit to an applied migration is a mismatch, by design. +use chrono::{DateTime, Duration, NaiveDateTime, Utc}; use sha2::{Digest, Sha256}; use std::path::{Path, PathBuf}; @@ -50,6 +51,28 @@ pub fn validate_name(file_name: &str) -> Result<(), String> { Ok(()) } +/// Parse the 14-digit timestamp prefix of a migration name as a UTC instant. +/// `None` when the digits do not form a real calendar date/time — +/// `validate_name` only checks that they are digits. +pub fn timestamp_of(name: &str) -> Option> { + let ts = name.get(..14)?; + NaiveDateTime::parse_from_str(ts, "%Y%m%d%H%M%S") + .ok() + .map(|naive| naive.and_utc()) +} + +/// Grace period before a migration timestamp counts as future-dated, so +/// ordinary clock skew between machines never trips the warning. +const FUTURE_SKEW_TOLERANCE_MINUTES: i64 = 5; + +/// True when the migration's timestamp prefix is ahead of `now` beyond clock +/// skew. Such a name was almost certainly written by hand instead of +/// `migrate::create`: files created later would sort before it until the +/// wall clock catches up. +pub fn is_future_dated(name: &str, now: DateTime) -> bool { + timestamp_of(name).is_some_and(|ts| ts > now + Duration::minutes(FUTURE_SKEW_TOLERANCE_MINUTES)) +} + /// SHA-256 lowercase hex of `bytes`, as-is. pub fn checksum_bytes(bytes: &[u8]) -> String { let mut hasher = Sha256::new(); @@ -157,6 +180,27 @@ mod tests { } } + #[test] + fn timestamp_of_parses_valid_prefixes() { + let ts = timestamp_of("20260719143000_add_users.sql").unwrap(); + assert_eq!(ts.to_rfc3339(), "2026-07-19T14:30:00+00:00"); + // Digits that are not a real date: validate_name accepts, we don't. + assert!(timestamp_of("20261319143000_bad_month.sql").is_none()); + assert!(timestamp_of("short.sql").is_none()); + } + + #[test] + fn future_dating_respects_skew_tolerance() { + let now = timestamp_of("20260723080000_now.sql").unwrap(); + // 4 minutes ahead: within tolerance, not flagged. + assert!(!is_future_dated("20260723080400_soon.sql", now)); + // 6 minutes ahead: flagged. + assert!(is_future_dated("20260723080600_later.sql", now)); + // Past and unparseable names are never flagged. + assert!(!is_future_dated("20260101000000_old.sql", now)); + assert!(!is_future_dated("nonsense.sql", now)); + } + #[test] fn checksum_is_sha256_lowercase_hex() { // sha256("hello") — well-known vector.