Repository navigation
fix: migration class names must match the RcrewAI acronym inflection - #25
Merged
Merged
Conversation
The engine registers an RcrewAI acronym so Zeitwerk resolves rcrewai/rails/span.rb to RcrewAI::Rails::Span. That same inflection makes Rails camelize create_rcrewai_tables to CreateRcrewAITables, but the migrations declared CreateRcrewaiTables. Rails resolves a migration class from its filename, so rails db:migrate raised NameError on a verbatim install of 0.7.0. Nine db/migrate files and the install template were affected. The prior generator spec hardcoded the wrong name and so asserted the bug was correct; it now derives the expected name the way Rails does, and a new spec runs the generated migration through ActiveRecord::MigrationContext - the resolution step a hand-rolled load-and-call skips.
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
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.
Reported by a user installing 0.7.0. Following the documented install verbatim fails:
Cause
The engine registers an
RcrewAIacronym inflection — Zeitwerk needs it to resolvercrewai/rails/span.rbtoRcrewAI::Rails::Span, so it cannot simply be removed.That same inflection makes Rails camelize
create_rcrewai_tablestoCreateRcrewAITables, while the shipped migrations declaredCreateRcrewaiTables. Rails resolves a migration's class from its filename, sorails db:migrateraisesNameError.Nine of eleven migrations in
db/migratewere affected, plus the install template — every one whose name containsrcrewai. This broke both paths: fresh installs via the generator, and upgrades viarcrew_ai_rails:install:migrations.Why the tests missed it
Two compounding gaps, both now closed:
CreateRcrewaiTables— the wrong name. It asserted the bug was correct and passed while the gem was broken.CreateRcrewaiTables.new.exec_migrationdirectly afterload. Referring to the class by name skips the filename→constant resolution entirely, which is exactly where the failure lives.Fix
db/migratefiles and the install template now declare the name Rails derives.spec/migrations_spec.rb(new) checks every shipped migration, deriving the expected name viaActiveSupport::Inflectorrather than hardcoding it — so it stays correct if the inflection changes.spec/real_migrate_spec.rb(new) runs the generated migration throughActiveRecord::MigrationContext#migrate, the real resolution path.Verified
Mutation-tested: restoring the 0.7.0 class name makes the new spec fail with the user's exact error —
221 examples, 0 failures.
Release
Bumps to 0.7.1. The CHANGELOG tells anyone already on 0.7.0 that a hand-renamed class needs no action, and otherwise to delete the copied migration and re-run the install.