✨ feat(migrations): run DB migrations on deploy, with audit trail, scaffolder and authoring skill - #131
Open
kuuak wants to merge 6 commits into
Open
✨ feat(migrations): run DB migrations on deploy, with audit trail, scaffolder and authoring skill#131kuuak wants to merge 6 commits into
kuuak wants to merge 6 commits into
Conversation
provision.sh never ran `wp spck migrate`, so the migration runner was only ever reachable by hand — while wordpress/README.md already claimed migrations ran automatically on deploy. Adds the block before the rewrite flush: count pending migrations, and if any exist export the database, run them, then prune old dumps. Differences from the coraasp.ch implementation this was ported from: - Dumps go to $BACKUP_PATH (default $WORDPRESS_PATH/db-backups), not loose in the webroot, and a deny-all .htaccess is written beside them. A directory we own, so WordPress's own root .htaccess is never touched. nginx needs an equivalent `location` rule — documented. - Old dumps are pruned to the newest $BACKUP_KEEP (default 10). - A failed migration prints the backup path and exits 1, so the Actions step goes red instead of leaving the already-swapped new theme live over un-migrated content with a green build. No auto-restore. - A non-numeric `--pending-count` warns instead of being fed to `-gt`. BACKUP_PATH/BACKUP_KEEP are passed through all three deploy workflows so they can be set as GitHub environment variables. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCMqJx3Ss6Esmc9UbsQ9NX
…ption The option stored a flat list of filenames, which answered "did this run?" and nothing else — no run date, no duration, no record of a failure. It was also autoloaded on every request for data only WP-CLI ever reads. Entries are now keyed by filename and hold `ran_at`, `duration` and `status`, and `--status` surfaces the two new columns. `get_completed()` shims the old flat format on read, so an existing site keeps its history and does not re-run migrations it has already applied; the next successful run rewrites the option in the new shape. `update_option()` does not reliably flip `autoload` on a row that already exists, so `save_completed()` follows it with `wp_set_option_autoload()`, guarded by `function_exists` (WP 6.6+). Verified against a stubbed `get_option`/`update_option`/`WP_CLI` harness: fresh install, run, re-run no-op, `--status`, `--dry-run` recording nothing, legacy flat-array shim, and a non-array option value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCMqJx3Ss6Esmc9UbsQ9NX
…eline fresh installs The string-only contract made two things painful or unsafe: any migration needing the table prefix had to hardcode `wp_` in a `db query` (silently a no-op on a differently-prefixed site, and still recorded as completed), and anything with a regex in it turned into quadruple-backslash string concatenation. A migration may now return a callable instead, executed with WordPress fully loaded ($wpdb, WP_Query, WP_CLI::runcommand). The array form is unchanged and still works. Commands are now run with `exit_error => false` and their return code checked, because `exit_error => true` halts the process before a failure can be recorded. A migration that fails — non-zero command, or a throwing callable — is stored with `status => 'failed'` and counts as pending again, so the next deployment retries it. Pending is therefore `not recorded OR not completed` rather than `not recorded`. Two new flags: - `--only=<file>` runs a single migration whatever its recorded state, for re-running one after a fix. Only the basename is honoured, so it cannot be pointed outside migrations/. - `--mark-complete` records migrations as done without executing them: the public escape hatch, and what baselining uses. provision.sh baselines on a fresh install (the Rails schema:load model): a database created by the current theme has nothing to migrate, so the existing migrations are marked complete instead of run. This needed FIRSTTIME_INSTALL fixed first — it was set to false at the top and never set to true anywhere, so baselining would never have fired. It is now set in the two branches that actually run `core install`. That also wakes the pre-existing first-install block (Sample Page → Home, GraphQL registry seed, rewrite structure), which is intended and only happens on genuinely fresh installs. Verified with a stub harness (no phpunit in the repo, out of scope) driving Migrations::run directly against stubbed get_option/update_option/WP_CLI — 53 checks over: the commit 2 behaviour as regressions, callables, a failing command and a throwing callable both recorded as failed and retried on the next run, a migration returning neither array nor callable, --only for both re-runs and pending files, --only rejecting unknown and traversing names, --mark-complete for all and for a single file, and its interaction with --dry-run. Plus `php -l` and `sh -n` on the changed files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCMqJx3Ss6Esmc9UbsQ9NX
…gration Writing a migration by hand means getting the timestamp prefix, the ABSPATH guard and the docblock right every time, and the two return forms are easy to forget. `npm run generate:migration "fix section cards"` now writes wordpress/theme/migrations/<YYYYMMDD_HHMMSS>_fix_section_cards.php from a template carrying all of it, prompting for the description when no argument is passed. The template presents both return forms — the array of WP-CLI commands and the callable — with the `wp_` prefix trap spelled out beside the callable, so the author picks deliberately rather than defaulting to strings. Follows the conventions of generators/lang-migration.js (docblock with both usages, `paths` object, log()/warn(), readline/promises, main().catch()), but substitutes tokens instead of copying the template verbatim. `generators/**` is outside prettier's scope, so the tab-indented style is matched by hand. Verified by running the generator on "Fix section cards — gap!" (slug collapses to fix_section_cards_gap), `php -l`-ing the result, and checking that a description with no alphanumerics exits 1 without writing a file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCMqJx3Ss6Esmc9UbsQ9NX
The judgement call agents get wrong is not how to write a migration but whether to write one at all — a deprecation in the block's `deprecated` array or a fallback in the render callback is nearly always better than a script that runs irreversibly against production content. The skill puts that decision first, as three conditions that must all hold, and stops if any fails. The rest is the authoring material commits 1–3 left implicit: scaffolding with `npm run generate:migration`, callable-by-default with the `wp_` prefix trap and escaping as the two triggers to switch away from command strings, and re-runnability — a failed migration is recorded as `failed`, counts as pending again, and is re-run on the next deployment over a database where part of the work may already be done. Deploy behaviour, backup location, the audit trail and the flag list are already documented in docs/setup/deployment.md, wordpress/README.md and wordpress/theme/migrations/README.md; the skill links to them rather than restating them. The migrations README gains a pointer back to the skill. Also fixes the link to the migrations README in wordpress/README.md, which was repo-relative in a file one level down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCMqJx3Ss6Esmc9UbsQ9NX
Standards axis: - provision.sh: BACKUP_PATH and BACKUP_KEEP sat ~250 lines below the file's own "/!\ STOP to edit here /!\" marker, while the header says "Edit only the variables below". Moved them into the top variable block beside IS_MULTILANG, rationale comment included. - provision.sh: an unparseable --pending-count printed the warning and then fell through to "No pending migrations", two contradictory lines for one condition. The unknown count is now its own branch and says only the warning. - migrations.php: the audit-trail record literal was rebuilt at three write sites differing only in status and duration. Collapsed into entry(). The legacy shim keeps its own literal — it records unknown provenance, not a run. - migrations.php: the status strings were typed at four sites and compared literally at two, where a typo would leave a migration permanently pending. Now STATUS_COMPLETED / STATUS_FAILED constants, matching the file's existing OPTION_NAME convention. - generators: the two generators shared rootDir, log()/warn(), the readline create/try/finally/close lifecycle and the main().catch() tail verbatim. Extracted to generators/lib.js and used by both. Spec axis: - The scaffold template left the array form active, so a file committed unedited was a valid pending migration that would run a site-wide search-replace on the next deploy. Both forms are now commented out: an untouched file returns nothing and the runner skips it with a warning. The migrations README and the skill say so. Verified: `sh -n provision.sh`, `php -l migrations.php`, both generators run non-interactively and interactively with php -l clean output, withPrompt closes its interface, prettier clean. Declined from the review, per the user: the coraasp divergence note, the revert-conflict finding (the series lands squashed), and the Actions-variable scope-creep flag (per-environment config is wanted). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCMqJx3Ss6Esmc9UbsQ9NX
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.
Why
The migration runner (
wp spck migrate) was already ported into the theme andwordpress/theme/migrations/already reaches servers viatheme-deploy. What was missing is the part that makes it a feature: nothing ever ran it.wordpress/README.mdclaimed "Migrations run automatically during deployment viaprovision.sh(with a DB backup beforehand)" — that sentence was false until this branch.This lands the deploy hook and the surrounding authoring workflow, deliberately not copying four problems from the coraasp.ch implementation the pattern came from: backups written loose into the webroot and never pruned, a failed migration still exiting 0, a hardcoded
wp_table prefix that silently no-ops, and unescapable command strings.What changed
Deploy hook —
provision.shcounts pending migrations, and if there are any, exports the DB before running them. Backups go to$BACKUP_PATH(default$WORDPRESS_PATH/db-backups), pruned to the newest$BACKUP_KEEP(default 10). The migrate exit code is captured: on failure the step prints thewp db importcommand for the backup just taken and exits 1, so the workflow goes red. No automatic restore. All three deploy workflows pipeprovision.shover stdin, so one change covers all of them.Storage — one option row still, but
filename => { ran_at, duration, status }instead of a flat list, withautoloadoff and a read-shim for the old format. A migration recorded asfailedcounts as pending again, so the next deploy retries it.Runner contract — a migration may now return a callable, executed with WordPress fully loaded (
$wpdb,WP_Query,WP_CLI::runcommand), as well as the original array of WP-CLI command strings. This is what fixes the prefix trap and the escaping pain. New flags:--only=<file>(re-run one migration whatever its state) and--mark-complete. Fresh installs are baselined onFIRSTTIME_INSTALL— a database created by the theme version that contains a migration never runs it.Scaffolder —
npm run generate:migration "fix section cards"writes a timestamped file with the docblock,ABSPATHguard and both return forms commented out. An untouched scaffold returns nothing and is skipped by the runner, so a file committed unedited can never execute a placeholdersearch-replace.Authoring skill —
.claude/skills/add-db-migration/leads with the judgement call that actually gets fumbled: whether a migration is warranted at all, versus a blockdeprecatedentry or a render-time fallback.Backups default to a directory inside the webroot.
provision.shwrites a deny-all.htaccessbeside them, which covers Apache. nginx needs the equivalent rule added by hand in the server block:Alternatively set
BACKUP_PATHto a directory outside the webroot. Both are documented indocs/setup/deployment.md.Out of scope
No backport to coraasp.ch, no example migration in
migrations/, nodown()/ rollback, no dedicated DB table (the option holds ~40 bytes per migration; a table is onewp db export --tables=away from being dropped, after which every migration re-runs).Verification
There is no PHP test harness in this repo and adding one was out of scope. The runner was driven directly through a throwaway stub harness (53 checks over
get_option/update_option/wp_set_option_autoloadand aWP_CLIdouble). Beyond that:sh -n provision.sh,php -lon the runner and on generated scaffolds, both generators exercised interactively and non-interactively,tsc --noEmitclean, prettier clean./code-reviewwas run over the whole branch on both the Standards and Spec axes; the findings that were accepted are fixed in6df1d44.Review notes
Six commits, intended to be squashed on merge.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JCMqJx3Ss6Esmc9UbsQ9NX