Skip to content

fix(migration): strip the file extension when resolving a v2 description - #128

Open
cwvermaak-codeinfinity wants to merge 1 commit into
tina4stack:v3from
cwvermaak-codeinfinity:fix/v2-upgrade-strips-migration-file-extension
Open

fix(migration): strip the file extension when resolving a v2 description#128
cwvermaak-codeinfinity wants to merge 1 commit into
tina4stack:v3from
cwvermaak-codeinfinity:fix/v2-upgrade-strips-migration-file-extension

Conversation

@cwvermaak-codeinfinity

Copy link
Copy Markdown
Contributor

A v2 -> v3 upgrade replays every already-applied migration.

The defect

Python v2 (tina4_python/Migration.py, 0.2.x) recorded description straight from os.listdir, so the stored value carries the file extension:

dba.execute("insert into tina4_migration (id, description, content, passed) ...", [next_id, file, file_contents])

v3 compares against mig_file.stem, which never carries it. _resolve_migration_name tried three match strategies and none stripped the extension. Strategy 3 tests description in stem, but here the stem is a substring of the description, not the reverse, so it fell through to returning the description verbatim.

_get_executed() then returns ids ending in .sql while migrate() looks for ids without it. Nothing matches, so the entire applied history re-runs.

Impact

Hit Hertex app-portal on 2026-09-09. A 0.2.206 -> 3.13.52 upgrade replayed 90 applied migrations and crashlooped the app on the first non-idempotent one:

RuntimeError: Migration failed: 0000002_data_migration_for_show_room.sql - duplicate key value
violates unique constraint "checklist_item_group_group_name_key"

DDL migrations using CREATE TABLE IF NOT EXISTS replay silently, so the damage only surfaces at the first migration carrying data.

The fix

Each match strategy now runs against the description and its extension-stripped form. A description with no matching file on disk is still kept verbatim, so non-matching rows behave exactly as before.

Why the existing suite missed it

Every fixture in test_issue_115_v2_upgrade.py seeds description as a bare stem (000001_create_users), which is not what v2 ever wrote. Ten tests passed over a live defect.

Four tests added:

  • test_v2_description_with_sql_extension_resolves_to_the_stem
  • test_v2_filenames_do_not_replay_on_upgrade - replays the production failure against a non-idempotent migration, so a regression fails loudly instead of passing on CREATE TABLE IF NOT EXISTS
  • test_v2_python_migration_filename_also_resolves - .py migrations
  • test_unrelated_description_still_falls_back_verbatim - control against over-reach

Verification

Run Result
4 new tests before the fix 3 fail, 1 (the control) passes
4 new tests after 4 pass
test_issue_115_v2_upgrade.py 19 pass
7 migration test files 96 pass, 10 skip, 0 fail

Parity

Not a straight port. tina4-php is shaped differently and looks unaffected: its v2 table already carried migration_id, and it uses full filenames as the identifier (x.sql). Python is the one that moved to stems while v2 stored filenames. Ruby and Node have not been checked and may warrant a look.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YPVsva4pe4rXmQaCM1bfj5

Python v2 recorded `description` straight from os.listdir, so the value
carries ".sql"/".py". v3 compares against `mig_file.stem`, which never
does. _resolve_migration_name tried three matches and none stripped the
extension - strategy 3 tests `description in stem`, but here the stem is
a substring of the description, not the reverse - so it fell through to
the verbatim description and nothing ever matched.

Every already-applied migration therefore looked unapplied and the whole
history replayed on a v2 -> v3 upgrade.

This hit Hertex app-portal on 2026-09-09: a 0.2.206 -> 3.13.52 upgrade
replayed 90 applied migrations and crashlooped the app on the first
non-idempotent one with a duplicate key.

Each match strategy now runs against the description and its
extension-stripped form. A description with no file on disk is still
kept verbatim, so non-matching rows are unaffected.

The existing suite missed this because every fixture seeded a bare stem
("000001_create_users"), which is not what v2 ever wrote. Four tests
added, including a replay of the production failure against a
non-idempotent migration so a regression fails loudly rather than
passing on CREATE TABLE IF NOT EXISTS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YPVsva4pe4rXmQaCM1bfj5
@cwvermaak-codeinfinity

Copy link
Copy Markdown
Contributor Author

Parity check done — Ruby and Node are not affected. The PR body says they were unchecked; this supersedes that.

tina4-ruby — not affected. Uses the full filename as the identifier consistently on both sides: files.reject { |f| completed.include?(File.basename(f)) } when selecting pending work, and the same File.basename value written to migration_name. There is no v2→v3 backfill at all, so there is no description-to-stem resolution step to get wrong.

tina4-nodejs — not affected. Uses stems like Python (file.replace(/\.sql$/, "")) but writes the same stem it compares, and both legacy paths already hold stems:

  • the old-v3 (name + applied_at) shape stored migrationId, i.e. the stripped stem, so the UPDATE ... SET migration_name = name copy is stem to stem
  • the older description/content/passed fallback also stored migrationId, not the filename

So Python is the only one of the four where the stored identifier and the compared identifier disagree, and the cause is specific to it: Python v2 recorded os.listdir filenames while v3 moved to stems. PHP and Ruby avoid it by using filenames throughout, Node by using stems throughout.

One adjacent observation in Node, not verified and not part of this PR: upgradeMigrationTable() only backfills migration_name for the old-v3 name shape. A table carrying only the older description column falls through to the "ensure batch" branch and never gets migration_name populated. Whether that can strand a real project depends on whether such tables exist in the wild, which I have not established.

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