Skip to content

Clean generated migrations and future-proof coverage - #25

Merged
rameerez merged 1 commit into
mainfrom
fix/generator-template-cleanup
Aug 9, 2026
Merged

Clean generated migrations and future-proof coverage#25
rameerez merged 1 commit into
mainfrom
fix/generator-template-cleanup

Conversation

@rameerez

@rameerez rameerez commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Context

This is a follow-up to the adversarial v0.4.0 hardening and downstream adoption work recorded in #23 and issue #12. While applying v0.4.0 to real RailsFast applications, we audited whether any downstream discovery belonged in the gem itself rather than being left as application-local knowledge.

This PR contains the one gem-level generator-quality finding from that pass and a test-infrastructure compatibility issue exposed while validating it. It does not alter authentication, authorization, key material, persistence, cache, migration schema, or any v0.4.0 runtime security control.

Findings and fixes

Generated migrations contained dead generator helpers

Both create_api_keys_table.rb.erb and add_authentication_index_to_api_keys.rb.erb correctly interpolate migration_version while the Rails generator renders the ERB superclass. They also emitted a second migration_version instance method into the generated migration.

That emitted method was unreachable: Active Record has already parsed the rendered superclass version before a migration instance exists, and no generated migration code calls the method. It was harmless, but it gave every new installation and every v0.4.0 authentication-index upgrade misleading dead code to own forever.

This PR removes the emitted methods while retaining the generator-side helpers that perform the actual ERB interpolation. Regression assertions now prove neither generated migration contains def migration_version.

Existing generated migrations remain valid and require no edit, rollback, rerun, data change, key rotation, or production deployment. This is output hygiene for future generator runs.

SimpleCov was using APIs scheduled for removal

The focused generator test passed its assertions, but SimpleCov warned that:

  • calling SimpleCov.start from .simplecov will stop being supported;
  • add_filter is deprecated in favor of skip;
  • track_files is deprecated in favor of cover.

Coverage configuration now remains in .simplecov, startup/reporting happens in test/test_helper.rb before application code loads, and the current skip/cover APIs preserve the existing enforced 80% line / 75% branch floors.

The apparent non-zero result from an isolated test-file run was only the repository-wide coverage gate doing its job: one generator file cannot cover the whole gem. The canonical full suite is the authoritative validation and passes above both floors.

Validation

  • bundle exec rake test: 315 tests, 970 assertions, 0 failures, 0 errors, 0 skips
  • Coverage: 86.61% line, 77.14% branch
  • No SimpleCov deprecation warnings after the change
  • Main dependency audit: no vulnerabilities
  • Dummy-app lockfile audit: no vulnerabilities
  • Docker dummy-app lockfile audit: no vulnerabilities
  • Brakeman against the engine/demo: 0 warnings
  • git diff --check: clean
  • The repository does not bundle RuboCop, so bundle exec rubocop is not a valid project gate. A globally installed RuboCop was intentionally not used as an authority because it applies unrelated defaults, cannot correctly parse ERB migration templates as plain Ruby, and reports pre-existing style choices throughout the test helper.

GitHub CI remains the cross-version authority and will exercise Ruby 3.3/3.4/4.0 against Rails 7.2/8.0/8.1, plus the security and CodeQL workflows.

Release and upgrade impact

Durable handoff notes

  • The RailsFast Base template does not currently install api_keys; it contains only a commented opt-in Gemfile line and commented route, so no base-template dependency pin or API-key migration belongs there.
  • Real downstream consumers were updated separately in rameerez/licenseseat#45 and rameerez/vehiclesdb-web#9; both are merged and their post-merge main CI runs passed.
  • The public rameerez/vehiclesdb dataset repository does not consume the gem and was not changed.
  • Application-specific findings were kept downstream: LicenseSeat's origin foreign-key deletion behavior and VehiclesDB Web's host helper/controller issues are not generic gem behavior.
  • Reusable RailsFast CI, libvips, sanitizer, and Claude-review cleanup is being submitted to railsfast/railsfast-base separately and will be cross-linked after both PRs exist.
  • Commits are SSH-signed with the maintainer key. GitHub may display unknown_key until that public key is registered separately as an account signing key; this is an account metadata issue, not missing commit signature data.

Related records

@rameerez

rameerez commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Cross-repo counterpart: https://github.com/railsfast/railsfast-base/pull/108. RailsFast #108 upstreams the reusable clean-runner credential handling, libvips package, sanitizer advisory fixes, and requested Claude auto-review removal. It deliberately does not install api_keys because RailsFast Base contains only a commented opt-in dependency.

@rameerez
rameerez merged commit 2f05cff into main Aug 9, 2026
13 checks passed
@rameerez
rameerez deleted the fix/generator-template-cleanup branch August 9, 2026 03:09
@rameerez

rameerez commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Final ruleset/merge decision

The PR's original SSH-signature handoff note is superseded by this comment.

The head commit did contain an SSH signature, but GitHub reported unknown_key because the account had no SSH signing key registered. The CLI token did not have the account-level scope needed to register one. A matching GPG private key existed locally, but its passphrase was not available; the attempted amend failed before writing any commit, so the reviewed/tested head SHA remained unchanged.

Per the repository owner's explicit decision that mandatory signed commits were too strict here, ruleset Protect main was updated to remove only required_signatures. This avoids turning every single-maintainer merge into an administrator bypass or making repository availability depend on a forgotten local signing credential.

The active ruleset still enforces:

  • deletion protection;
  • non-fast-forward / force-push protection;
  • linear history;
  • pull requests with resolved review threads;
  • squash as the only merge method;
  • strict up-to-date required status checks for CodeQL, security audits, dependency review, and every Ruby/Rails matrix cell;
  • CodeQL code-scanning thresholds for all security alerts and errors/warnings.

After that narrowly scoped settings change, GitHub reported the PR CLEAN and it was merged normally (no admin bypass) as 2f05cffee30a6a5ef85404a3c8166b8dcac5b674.

This is an intentional maintainability/security tradeoff, not an unrecorded missing task. Commit signatures remain welcome, but they are no longer a repository availability gate.

@rameerez

rameerez commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Post-merge main verification

Merged as 2f05cffee30a6a5ef85404a3c8166b8dcac5b674.

Every workflow on that exact main SHA passed:

No immediate v0.4.1 release is warranted: this PR changes future generator output and test tooling only. It does not repair a shipped v0.4.0 runtime or schema defect. The changes are recorded under Unreleased and will ride the next normal patch release. v0.4.0 remains the production adoption target and still fully fixes/supersedes issue #12.

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