From 4eed0aaee99a83f32d2b3571526fcce399df3da7 Mon Sep 17 00:00:00 2001 From: Zsolt Tasnadi Date: Wed, 19 Aug 2026 11:29:53 +0200 Subject: [PATCH] warp_engine 0.5.1: the device_grants migration collided with the host's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `db:migrate` on the catalog API stopped before running anything: Multiple migrations have the version number 20260819000001. The engine appends its `db/migrate` to the host's migration paths instead of copying migrations in, so engine and host share one version namespace. Both had picked 20260819000001 on the same day by the same habit — the engine for device_grants, apps/api for carry_the_store_config_in_the_registry — and neither repository could see the other's number. Worse than a clash of our own making: it takes the *host's* migrations down with it, for the whole application, before anything runs. device_grants is renumbered to 20260819093412 — a real second-resolution timestamp, which is the actual defence. A round hand-written number is precisely what another repository lands on. Nothing had run it in production, so this is a rename rather than a data migration; a host that already applied the old version renumbers its schema_migrations row. The older engine migrations keep their round numbers: renumbering one that has been deployed everywhere is worse than the risk it carries. They are named in the new spec's grandfather list rather than excused by a rule that would also let the next one through. That spec found a second thing, older than this change: the install template creates `application_tokens`, and so does one of our own migrations — so a fresh host runs CREATE TABLE twice and has to delete the block from its generated copy by hand, which is exactly what teletype-orbit's migration header describes. I had just made it worse by putting device_grants in the template too; that is out again, and the template says why. `application_tokens` is grandfathered and left for a change that is not a hotfix. Co-Authored-By: Claude Opus 5 (1M context) --- ...=> 20260819093412_create_device_grants.rb} | 0 .../templates/create_warp_engine_tables.rb | 26 ++----- lib/warp_engine/engine.rb | 9 +++ lib/warp_engine/version.rb | 2 +- spec/migrations_spec.rb | 76 +++++++++++++++++++ 5 files changed, 91 insertions(+), 22 deletions(-) rename db/migrate/{20260819000001_create_device_grants.rb => 20260819093412_create_device_grants.rb} (100%) create mode 100644 spec/migrations_spec.rb diff --git a/db/migrate/20260819000001_create_device_grants.rb b/db/migrate/20260819093412_create_device_grants.rb similarity index 100% rename from db/migrate/20260819000001_create_device_grants.rb rename to db/migrate/20260819093412_create_device_grants.rb diff --git a/lib/generators/warp_engine/install/templates/create_warp_engine_tables.rb b/lib/generators/warp_engine/install/templates/create_warp_engine_tables.rb index 6ed7bf7..95cf420 100644 --- a/lib/generators/warp_engine/install/templates/create_warp_engine_tables.rb +++ b/lib/generators/warp_engine/install/templates/create_warp_engine_tables.rb @@ -98,27 +98,11 @@ class CreateWarpEngineTables < ActiveRecord::Migration[8.0] t.index :deleted_at end - # Device sign-in for clients that have no browser of their own (RFC 8628). - # Short-lived rows: one exists for the minute or two between "the client asked" - # and "the person answered". Only used where access_token_owner_class is set. - create_table :device_grants do |t| - t.string :device_code, limit: 64, null: false - t.string :user_code, limit: 16, null: false - t.string :client_name, limit: 128 - t.string :subject_type, limit: 128 - t.bigint :subject_id - t.bigint :application_token_id - # The issued token in the clear, cleared on the poll that hands it over — see - # the model. Everything else about a token is stored as a digest. - t.string :issued_token, limit: 64 - t.datetime :approved_at, precision: 3 - t.datetime :denied_at, precision: 3 - t.datetime :expires_at, precision: 3, null: false - t.timestamps precision: 3, null: true - t.index :device_code, unique: true - t.index :user_code, unique: true - t.index :expires_at - end + # NOTE: device_grants is NOT created here. It ships as its own migration in the + # engine's db/migrate, which the host runs from the appended path — so creating it + # here as well would be a second CREATE TABLE for the same name. The same is true + # of application_tokens below, which predates this note and is why a host + # installing today has to delete that block from its copy by hand. create_table :downloads do |t| t.string :file_path, null: false diff --git a/lib/warp_engine/engine.rb b/lib/warp_engine/engine.rb index 595864f..3b10a28 100644 --- a/lib/warp_engine/engine.rb +++ b/lib/warp_engine/engine.rb @@ -10,6 +10,15 @@ module WarpEngine # A jövőbeli katalógus-migrációk az engine db/migrate-jéből futnak a host # rails db:migrate-jével, másolás nélkül. + # + # FONTOS: emiatt az engine és a host migrációi EGY névtérben vannak, és két + # azonos verziószám a host `db:migrate`-jét indulás előtt megállítja + # (DuplicateMigrationVersionError) — nem a miénket, hanem az övét, az egész + # alkalmazásban. Az engine migrációi ezért **valódi, másodperc-pontosságú + # időbélyeget** kapnak (20260819093412), soha nem kerek kézzel írt számot + # (20260819000001): pont az utóbbiakra ír rá egy host, ami ugyanaznap ugyanezzel + # a szokással ír migrációt. Így ütközött a device_grants a katalógus-API + # `carry_the_store_config_in_the_registry`-jével. initializer "warp_engine.append_migrations" do |app| unless app.root.to_s.start_with?(root.to_s) config.paths["db/migrate"].expanded.each do |path| diff --git a/lib/warp_engine/version.rb b/lib/warp_engine/version.rb index 991e73b..073c16c 100644 --- a/lib/warp_engine/version.rb +++ b/lib/warp_engine/version.rb @@ -1,5 +1,5 @@ module WarpEngine - VERSION = "0.5.0" + VERSION = "0.5.1" # The header every API response carries. Named here rather than written out at the one # place that sets it: clients read it, the README documents it, and a string in three diff --git a/spec/migrations_spec.rb b/spec/migrations_spec.rb new file mode 100644 index 0000000..f30068e --- /dev/null +++ b/spec/migrations_spec.rb @@ -0,0 +1,76 @@ +require "rails_helper" + +# The engine appends its `db/migrate` to the host's migration paths instead of copying +# migrations into the host, so the two share **one** version namespace. A collision does +# not fail politely somewhere in the engine: it stops the host's `db:migrate` before it +# runs anything, for the whole application. +# +# That is exactly what happened to `device_grants`. It was numbered 20260819000001 — a +# hand-picked round number — and the catalog API had written +# `20260819000001_carry_the_store_config_in_the_registry` on the same day with the same +# habit. Neither repository could see the other's number. +# +# The defence is that engine migrations carry a real second-resolution timestamp, which +# nobody hand-writes and nothing rounds to. This is the test that says so. +RSpec.describe "Engine migrations" do + # `202608050000 01`-style numbers: a date, then zeros, then a counter. Rails generates + # `20260819093412`; a person types this. + ROUND_VERSION = /\A\d{8}0{4}\d{2}\z/ + + # Numbered before this file existed and already deployed everywhere. Renumbering a + # migration that has run is worse than the risk it carries, so they are named here + # rather than quietly excluded by a rule that would also excuse the next one. + GRANDFATHERED = %w[ + 20260805000001 + 20260805000003 + 20260806000001 + 20260806000002 + ].freeze + + let(:versions) do + Dir.glob(WarpEngine::Engine.root.join("db/migrate/*.rb")) + .map { |path| File.basename(path)[/\A\d+/] } + end + + it "has a migration to check at all" do + expect(versions).not_to be_empty + end + + it "numbers every migration uniquely" do + expect(versions).to eq(versions.uniq) + end + + it "uses a real timestamp rather than a round hand-written number" do + round = versions.grep(ROUND_VERSION) - GRANDFATHERED + + expect(round).to be_empty, + "these would collide with a host that numbers its migrations the " \ + "same way on the same day: #{round.join(', ')}. Use a second-resolution " \ + "timestamp — `date -u +%Y%m%d%H%M%S` — not a hand-picked round number." + end + + # A table created by BOTH the install template and one of our own migrations is a + # second CREATE TABLE for the same name in whatever host installs us — the template + # runs, then the appended engine migration runs, and the second one fails. + # + # `application_tokens` is exactly that, and has been since before this file existed: + # every host installing today has to delete that block from its generated copy by + # hand, which is what teletype-orbit's own migration says in its header. It is + # grandfathered here rather than quietly excused, so the list can only shrink. + it "does not create a table the install generator also creates" do + template = WarpEngine::Engine.root.join( + "lib/generators/warp_engine/install/templates/create_warp_engine_tables.rb" + ) + engine_tables = versions.flat_map do |version| + file = Dir.glob(WarpEngine::Engine.root.join("db/migrate/#{version}_*.rb")).first + File.read(file).scan(/create_table :(\w+)/).flatten + end + template_tables = File.read(template).scan(/create_table :(\w+)/).flatten + duplicated = (engine_tables & template_tables) - [ "application_tokens" ] + + expect(duplicated).to be_empty, + "the install template and an engine migration both create " \ + "#{duplicated.join(', ')} — a host would run CREATE TABLE twice. " \ + "A table added after the initial schema belongs in a migration only." + end +end