[rameerez]

Two fixes. Both were found when an app on the fleet upgraded from 0.5 to 0.8.0.

1. The one-time token in transactional tests

Cause. Since 0.6.0, ApiKeys::TokenSession refuses a retrieval inside any open transaction on the ApiKey connection (claim_could_roll_back? asks transaction_open?), because a rollback could undo the token_shown_at claim after the token was shown. Rails' transactional tests run each test inside one transaction. So in every such test a retrieval got :inside_transaction and nil, and the dashboard's token page redirected. Apps had to rewrite their tests or stub a private method.

Fix. The transaction that a transactional test opens around each test does not count. When Rails.env is test, and the only open transaction is an outer, non-joinable one, the claim runs in it. Rails' transactional tests, rspec-rails and DatabaseCleaner's :transaction strategy open that transaction, and a system test's server thread shares it. Then:

• the first retrieval in the test returns the token;
• a second retrieval, a refresh or a copied session gets nothing (:already_shown or :missing);
• a transaction that the app or the test opens on top of it is still refused, as in production;
• the rollback at the end of the test removes the key and its claim.

Outside the test environment nothing changes.

2. Every upgrade generator and strong_migrations

strong_migrations refused three upgrade migrations on PostgreSQL. Those three now run with disable_ddl_transaction!, and each step checks what exists, so a run that stopped half way can run again.

• add_created_by (0.7.0). Cause: add_reference ... polymorphic: true built the index inside the migration's DDL transaction, which blocks writes to api_keys. Fix: add the columns with index: false, then build index_api_keys_on_created_by with algorithm: :concurrently on PostgreSQL and PostGIS. MySQL and SQLite refuse that option, so there the index is built as usual.
• add_key_types (0.3.0). Cause: three indexes built inside the transaction, and a four-column index that strong_migrations calls a best-practice problem on every database. Fix: concurrent indexes on PostgreSQL. The four-column index is the same one the install makes, so the migration keeps it inside safety_assured when the app has strong_migrations. change became up/down; down removes the indexes before the columns, so no index is left cut down.
• add_restrictions (0.5.0). Cause: the check constraint was validated under a lock that blocks reads and writes. Fix: on PostgreSQL, add it with validate: false, then validate_check_constraint. On MySQL the migration never ran, because MySQL refuses a literal default on a JSON column. Now the default there is the expression JSON_OBJECT(), which also gives existing keys {}. strong_migrations has no safe form of a check constraint on an existing MySQL table, so it still refuses this one on MySQL; the check shows that refusal.
• add_authentication_index (0.4.0) was already concurrent. It now also recognises PostGIS.
• add_key_activity, add_device_authorizations and add_idempotency_keys already passed. They are unchanged.

Review

An adversarial review of 79d6a8a gave FIX-FIRST: 1 major and 5 minor findings. All six are fixed in 2389a2b. 8033d5c builds the probe's broken index with add_index, because Brakeman flagged the raw SQL.

1. Major: four generated migrations failed rubocop-rails-omakase (16 offenses: 14 Layout/SpaceInsideArrayLiteralBrackets, 2 Layout/EndAlignment) in the install, add_restrictions, add_device_authorizations and add_idempotency_keys. Fix: %i[] arrays, the key types as two methods (primary_key_type, foreign_key_type), and case on its own line under expression =. New test: test/generators/generated_migrations_style_test.rb generates all eight migrations and runs RuboCop with the omakase config on them. rubocop-rails-omakase is in the test bundle. RuboCop allows json 3, which Active Support 8.0 cannot use, so the Rails 8.0 appraisal keeps json < 3.
2. The exemption checks the shape of the transaction. It stays automatic, and it does not require connection.pinned, because that would drop DatabaseCleaner :transaction. The README and the method comment now list the three cases: a depth-1 transaction(joinable: false) that the app opens in a test without its own transaction is exempt too; an environment with another name (ci) is not exempt; fixtures plus DatabaseCleaner :transaction (depth 2) is not exempt. New tests pin the depth-1 case and ci.
3. One environment check. ApiKeys.test_environment? is used by the token handoff and by idempotency_in_test_transactions. The CHANGELOG says why the token fix needs no switch: apps whose tests broke on 0.6.0 must not need new configuration.
4. An invalid index from a failed concurrent build. add_created_by, add_key_types and add_authentication_index now treat an index with respond_to?(:valid?) && !valid? as absent: they remove it concurrently and build it again. New PostgreSQL probe in test/databases/check.rb: a unique concurrent build over two equal rows leaves an invalid index, and the next run makes it valid. With the templates of 79d6a8a the probe fails.
5. MySQL with strong_migrations. The README and the add_restrictions migration say: check the size of api_keys, then wrap add_restrictions_object_constraint in safety_assured, or run the migration in a maintenance window.
6. CHANGELOG date is 2026-10-26.

A second review pass of 8033d5c resolved items 1 to 4 and 6. On item 5 it found that on MySQL strong_migrations refuses add_restrictions already at add_column, for the callable JSON_OBJECT() default, before the check constraint. 0f501b5 corrects the advice in the README, the CHANGELOG and the migration comment: check the size of api_keys, wrap the whole body of up in safety_assured, and run it when the table can be locked for the copy. A MySQL probe confirms it: the generated migration is refused with strong_migrations; with the body of up wrapped, it runs, an existing key gets {}, and the constraint exists.

Tests

• test/helpers/token_session_transactional_test.rb: the handoff under use_transactional_tests = true (once, then nothing; a transaction on top is refused; refused outside the test environment) and under real commits (a second connection sees the claim).
• test/controllers/keys_controller_test.rb: the dashboard in a transactional test. Create a key, the token page shows it once, then a reload and a copied session get "already shown".
• test/databases/check.rb now has strong_migrations. It starts from an install made before 0.3.0 that has a key, and runs all seven upgrade migrations as rails db:migrate runs them (MigrationContext, so disable_ddl_transaction! counts), then down and up again, and again after a run that stopped half way. A canary test shows that strong_migrations really checks there. strong_migrations does not support SQLite, so on SQLite the check only runs the migrations.
• test/trusted_key_check_test.rb, test/generators/add_key_types_generator_test.rb, test/generators/add_restrictions_generator_test.rb: the generated content, the options for each adapter, and a run on SQLite.
• test/generators/generated_migrations_style_test.rb: all eight generated migrations pass rubocop-rails-omakase.

Evidence

• bundle exec rake test: 790 tests, 0 failures, 0 errors, on Rails 8.1, 8.0 and 7.2, two random seeds each (774 before this change). The style test also passes on Ruby 3.3 and 4.0.
• test/databases/check.rb, all seven upgrades: 10 runs, 0 failures on PostgreSQL 18 (bigint and uuid keys) and MySQL 8.4, on Rails 8.1.4 and 7.2.4; on SQLite (Rails 8.1.4) 0 failures, with the PostgreSQL-only tests skipped.
• The checks fail on the old code:
  • the 0.8.0 templates stop the check on PostgreSQL with StrongMigrations::UnsafeMigration: add_created_by ("Adding an index non-concurrently locks the table"), add_key_types ("Adding an index non-concurrently blocks writes"), add_restrictions ("Adding a check constraint key blocks reads and writes while every row is checked");
  • the new add_created_by without disable_ddl_transaction! fails with PG::ActiveSqlTransaction ("CREATE INDEX CONCURRENTLY cannot run inside a transaction block");
  • with the 0.8.0 TokenSession, the three transactional helper tests and the dashboard test fail (the token page redirects).

🤖 Generated with Claude Code