Latest changes#1
Draft
donrestarone wants to merge 194 commits into
Draft
Conversation
…337) * Add multi-database rake task enhancement for Rails 7+ When a Rails app has multiple databases configured in database.yml, Rails 7+ creates namespaced rake tasks like `db:migrate:primary` and `db:rollback:primary`. Previously, Apartment only enhanced base tasks (`db:migrate`, `db:rollback`), so tenant schemas weren't migrated when using namespaced tasks. This change automatically detects databases with `database_tasks: true` and enhances their namespaced tasks to invoke the corresponding apartment task: - `db:migrate:primary` -> `apartment:migrate` - `db:rollback:primary` -> `apartment:rollback` - `db:migrate:up:primary` -> `apartment:migrate:up` - `db:migrate:down:primary` -> `apartment:migrate:down` - `db:migrate:redo:primary` -> `apartment:migrate:redo` Replica databases and databases with `database_tasks: false` are excluded from enhancement. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix connection pool isolation in migrator Wrap migration operations with `with_connection` to pin a single database connection for the entire operation. This ensures that `Tenant.switch` sets `search_path` on the same connection used by `migration_context`. Without this fix, the connection pool may return different connections for the tenant switch vs the actual migration, causing migrations to run against the wrong schema. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add specs for connection pool isolation in migrator Verifies that `with_connection` is called to pin a single connection for the entire migration operation, ensuring search_path consistency. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Refactor specs to fix RSpec/MultipleMemoizedHelpers Inline config doubles instead of using shared let blocks to reduce memoized helper count below the limit of 5. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Bump version to 3.4.1 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Add shared team plugins to .claude/settings.json (superpowers, context7, code-review, commit-commands, pr-review-toolkit, claude-md-management, feature-dev, code-simplifier, sentry, ruby-lsp). Check .mcp.json into git with generic rails-mcp-server path. Fix .gitignore to only ignore settings.local.json instead of entire .claude/ directory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Unify AI tools config: plugins and MCP servers
Comprehensive design document for the v4 ground-up rewrite covering: connection-pool-per-tenant architecture, CurrentAttributes-based tenant context, pool sizing/eviction, PgBouncer compatibility, all four tenant strategies, Header elevator, job middleware, Thor CLI, and upgrade path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Relocate spec to docs/designs/apartment-v4.md (living doc, no date prefix) - Remove tool-specific docs/superpowers/ path - Add .firecrawl/ to .gitignore (ephemeral working data) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Apartment v4 design spec
* Add v4 implementation plans, worktree scripts, and manage-worktree skill Implementation plans: - docs/plans/apartment-v4/overview.md: 8-phase decomposition with dependency graph, build order rationale, and version strategy - docs/plans/apartment-v4/phase-1-foundation.md: 10 TDD tasks for Config, Current, Errors, PoolManager, PoolReaper, Instrumentation Worktree tooling (adapted from www repo): - bin/dev/setup-worktree: copies .claude/.bundle/.vscode, assigns Peacock colors via ~/.config/campusesp/color-registry.json - bin/dev/remove-worktree: force-removes worktree, frees color, optional branch deletion - bin/dev/_gum.sh: shared UX helpers with gum fallback - .claude/skills/manage-worktree/SKILL.md: Claude Code skill for create/move/remove worktree workflows Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove private repo references, generalize examples - Remove "Adapted from CampusESP www repo" comments from worktree scripts - Generalize X-CampusESP-Tenant header example to X-Tenant-Id - Replace CampusESP-specific CloudFront reference with generic edge function pattern Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix review issues in worktree scripts and plan docs Shell scripts (addresses all critical and important review findings): - Resolve main repo via git worktree list, not pwd (works from any cwd) - Gracefully degrade when jq/color registry missing (warn, don't exit) - Use mktemp + trap for atomic registry writes (interrupt safety) - Let jq errors through instead of 2>/dev/null (better diagnostics) - Use jq instead of sed for .vscode/settings.json manipulation - Add ditto failure handling with actionable error messages - Show git worktree remove errors instead of 2>/dev/null || true - Add safety check before rm -rf (must be worktree or under worktrees dir) - Use git branch -d before -D (warn about unmerged commits) - Make registry path configurable via WORKTREE_COLOR_REGISTRY env var - Fix awk branch detection to handle paths with spaces - Add --show-error to gum spin so command errors are visible - Add explicit error handling for registry cleanup in remove-worktree Plan docs: - Fix gemspec s.files command (was mixing null-delimited with newline split) - Fix phase dependency diagram (Phase 6 depends on Phase 4 only, not 3+5) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address re-review findings in worktree scripts - Detect JSONC in .vscode/settings.json and provide actionable guidance instead of vague "failed to update" (VS Code settings commonly have comments) - Cover _SETTINGS_TMP in trap cleanup (was only covering _REGISTRY_TMP) - Replace jq -e &>/dev/null with proper exit code checking in remove-worktree registry cleanup (was hiding parse errors) - Warn on git status failure before force-removing worktree (prevents silent destruction of uncommitted changes with corrupt index) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…347) * Scaffold v4 gemspec and version (4.0.0.alpha1) Replace v3 gemspec/Gemfile with v4 dependencies: - Ruby >= 3.3, Rails >= 7.2 - Add zeitwerk, concurrent-ruby, thor - Simplify Gemfile for Phase 1 unit testing - Replace spec_helper with minimal v4 version Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add v4 exception hierarchy ApartmentError base with TenantNotFound, TenantExists, AdapterNotFound, ConfigurationError, PoolExhausted, SchemaLoadError. TenantNotFound/TenantExists accept optional tenant name in constructor. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Apartment::Current via ActiveSupport::CurrentAttributes Fiber-safe tenant context replacing v3 Thread.current approach. Attributes: tenant, previous_tenant. Thread-isolated, resettable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add v4 configuration system with validation Config class with validated tenant_strategy, tenants_provider, pool settings, parallel migration options, and per-database config (PostgreSQLConfig, MySQLConfig). validate! enforces required fields and mutual exclusion constraints. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Apartment module with configure DSL Replace v3 module with Zeitwerk-based autoloading. Provides Apartment.configure (yields Config, validates), Apartment.clear_config, and attr_readers for config/pool_manager. v3 files are ignored by Zeitwerk to coexist on this branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Move v3 unit specs to spec/unit_v3/ to avoid load conflicts v3 specs require rake, database gems, and other dependencies not present in the v4 Gemfile. They'll be replaced by v4 equivalents in later phases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add PoolManager with Concurrent::Map and LRU tracking Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add PoolReaper with idle eviction and LRU cleanup Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add AS::Notifications instrumentation for apartment events Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Wire PoolReaper eviction to AS::Notifications instrumentation Emit evict.apartment events (with :tenant and :reason payload) from evict_idle and evict_lru, and add a spec verifying the event fires on idle eviction. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add Phase 1 integration test Creates spec/unit/phase1_integration_spec.rb covering the full configure -> pool_manager -> Current -> reaper lifecycle. Also initializes @pool_manager inside Apartment.configure so the module is ready to use immediately after configuration. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix validate! to require tenants_provider, update specs Design spec requires tenants_provider to be a callable. The implementer subagent made it optional — fixed to match spec. Updated all tests that called configure/validate! without setting tenants_provider. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add PostgreSQL database-per-tenant strategy to design spec PostgreSQL now supports both :schema (schema-per-tenant) and :database_name (database-per-tenant) strategies. Adapter hierarchy updated to PostgreSQLSchemaAdapter and PostgreSQLDatabaseAdapter. Added strategy x database matrix and trade-off documentation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address comprehensive PR review findings PoolReaper: - Add mutex around start/stop to prevent race conditions - Use wait_for_termination on stop for clean shutdown - Include exception class and backtrace in rescue logging - Rescue per-tenant in eviction loops so one bad callback doesn't halt eviction of remaining tenants - Add double-start and error-resilience tests PoolManager: - Move touch() after compute_if_absent to avoid orphaned timestamps when pool creation block raises - Add #get specs (existing tenant, missing tenant, timestamp update) - Add Phase 1 stub comment on stats method Apartment module: - clear_config now stops PoolReaper (prevents orphaned timer threads) - configure stops reaper and clears old pool_manager before rebuilding - Require block for configure (fail fast with clear message) Config: - Add numeric validation for tenant_pool_size, pool_idle_timeout, max_total_connections in validate! - Fix class comment ("validated on freeze" -> "validated after block") PostgreSQLConfig: - Fix include_schemas_in_dump default from false to [] (matches design spec) - Fix comment to describe array semantics, not boolean Instrumentation: - Add block-forwarding test 84 examples, 0 failures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Improve type design and simplify per review Type design improvements: - Add attr_reader :tenant to TenantNotFound and TenantExists for programmatic error handling in rescue clauses and callbacks - Swap delete order in PoolManager#remove (pools before timestamps) to prevent orphaned timestamps from concurrent #get calls - Validate PoolReaper.start arguments (interval, idle_timeout, max_total) to fail fast on invalid config instead of cryptic TimerTask errors - Remove duplicate persistent_schemas from top-level Config (lives only on PostgreSQLConfig where it belongs) Simplification: - Remove dead EVENTS constant from Instrumentation (was defined but never referenced; known events documented in comment instead) - Simplify PoolManager#get to avoid extra hash lookup Deferred to Phase 2+: - Freeze Config after validate! (needs careful sub-config handling) - Convert PoolReaper from class singleton to instance (needs Railtie) - Single-map composite value in PoolManager (bigger refactor) - Add switch/reset methods to Current (belongs with Tenant API) - MySQLConfig expansion (when MySQL-specific options are needed) 85 examples, 0 failures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add github, firecrawl, security-guidance, skill-creator plugins Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
12 tasks covering the database engine layer: - Tenant public API (switch, current, reset, create, drop) - ConnectionHandling patches (AR::Base prepend, tenant-aware pool resolution) - AbstractAdapter (lifecycle, callbacks, excluded models, environmentify) - PostgreSQLSchemaAdapter and PostgreSQLDatabaseAdapter - MySQL2Adapter, TrilogyAdapter, SQLite3Adapter - Adapter factory and wiring - Excluded models processing - Integration tests with real databases - v3 adapter file cleanup Uses AR's public establish_connection API with shard-based pool keying for compatibility across Rails 7.2, 8.0, and 8.1. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…349) * Add sub-phase structure to Phase 2 plan Split Phase 2 into five sub-phases (2.1-2.5) with clear dependency graph. Phase 2.2 (database adapters) and 2.3 (connection handling) are independent and can execute in either order. Mapped deferred Phase 1 review items to specific sub-phases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Replace v3 Tenant module with v4 public API (Phase 2.1) Tenant now sets Current.tenant directly (fiber-safe via CurrentAttributes) instead of delegating to a thread-local adapter. Lifecycle operations (create/drop/migrate/seed) delegate to Apartment.adapter, which will be wired by the adapter factory in Task 8. - apartment.rb: add adapter accessor, remove tenant from Zeitwerk ignores, clear adapter on clear_config - 21 specs covering switch/restore/nesting, delegation, and edge cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address code review: nil adapter guard, nesting semantics docs - Add ConfigurationError when adapter is nil (clear message vs NoMethodError) - Document previous_tenant nesting semantics (single-scope, not stacked) - Add test for nil adapter guard Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Replace v3 AbstractAdapter with v4 base class (Phase 2.1) v4 AbstractAdapter provides: callback-wrapped create/drop with instrumentation, pool cleanup on drop, tenant-scoped migrate/seed, excluded model connection pinning, and configurable environmentify. Protected abstract methods (create_tenant, drop_tenant, resolve_connection_config) define the subclass contract. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address code review: align pool key format, nil-guard pool_manager - Use tenant.to_s as pool key (matches ConnectionHandling plan in Task 9) - Nil-guard Apartment.pool_manager in drop to handle unconfigured state Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add adapter factory with lazy-loading to Apartment module (Phase 2.1) Replace attr_accessor :adapter with a lazy-loading getter that builds the correct adapter on first access via a private build_adapter factory. The factory resolves adapter class based on tenant_strategy and database adapter, using require_relative for on-demand loading. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix test isolation: add ActiveRecord stub, fix deprecated matcher - Add ActiveRecord::Base stub in apartment_spec.rb so tests pass in isolation - Replace deprecated not_to raise_error(SpecificError) with explicit rescue Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address PR review: critical fixes, config freeze, test gaps Critical fixes: - configure: prepare-then-swap pattern prevents unrecoverable state on failed reconfiguration (build new config, validate, then tear down old) - PoolManager#clear: disconnect all pools before clearing to prevent connection leaks at the database level Important fixes: - Config: add freeze! method, called after validate! in configure. Deep-freezes mutable collections and sub-configs to prevent post-boot mutation - AbstractAdapter: rename @config → @connection_config to avoid naming collision with Apartment.config (the Config object) - Instrumentation: fix comment listing nonexistent events (switch, pool_stats were never instrumented) Test gaps filled (7 new specs, 159 total): - Verify Current.tenant is set during migrate and seed blocks - PoolReaper.start argument validation (zero/negative values) - process_excluded_models with invalid model name (NameError) - PoolManager#fetch_or_create does not store value when block raises Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix review round 2: MySQLConfig freeze!, frozen config test, init comment - Add freeze! to MySQLConfig for symmetry with PostgreSQLConfig - Call @mysql_config&.freeze! (not .freeze) in Config#freeze! - Add specs verifying config is frozen after configure and that failed reconfigure preserves previous working config - Fix init comment: only processes excluded models, not default tenant Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Document deferred review items in Phase 2 plan Add 15 items from Phase 2.1 comprehensive PR review, categorized by target sub-phase (2.2, 2.3, 2.4, general). Mark completed Phase 1 deferred items. Each item has a checkbox for tracking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update Appraisals for v4: Rails 7.2/8.0/8.1 × PG/MySQL/Trilogy/SQLite - Remove Rails 6.1, 7.0, 7.1 appraisals (below v4 minimum of 7.2) - Remove all JDBC appraisals (JRuby dropped in v4) - Add Trilogy appraisals (v4 supports trilogy adapter) - Add rails-main appraisals (PG + SQLite3) to catch regressions early - Add appraisal gem to Gemfile - Delete stale v3-era gemfiles, regenerate with new naming convention - Verified: 161 specs pass against Rails 7.2, 8.0, and 8.1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Bump .ruby-version to 4.0.2 Develop against latest stable Ruby. Gemspec floor (>= 3.3) unchanged — CI tests 3.3, 3.4, and 4.0. All 161 specs verified passing on all three. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Consolidate CI: single workflow with Ruby/Rails/DB matrix Replace 7 separate workflow files (5 PG versions, MySQL, SQLite) with a single ci.yml containing 4 jobs: - unit: Ruby 3.3/3.4/4.0 × Rails 7.2/8.0/8.1 (no DB, runs first) - postgresql: PG 16 + 18, gated on unit passing - mysql: MySQL 8.0, gated on unit passing - sqlite: gated on unit passing Key changes: - Drop Ruby 3.1, 3.2, JRuby (below v4 minimum) - Add Ruby 4.0 to matrix - Drop Rails 7.0, 7.1 (below v4 minimum) - Drop PG 14, 15, 17 (keep 16 as oldest supported + 18 as latest) - PG matrix pruned: PG 16 tests 7.2+8.0, PG 18 tests 8.0+8.1 - Integration jobs depend on unit job (fail fast on pure logic errors) - Update rubocop to use new gemfile path and Ruby 4.0 - Delete 7 stale v3 workflow files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update MySQL to 8.4 LTS, fix Trilogy gem constraint - CI: MySQL 8.0 → 8.4 LTS (8.0 EOL April 2026, 8.4 supported to 2032) - Appraisals: Trilogy constraint ~> 2.9 → >= 2.9 (allows current 2.11.x) - Regenerate Trilogy gemfiles with updated constraint Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Drop MySQL 8.0 and PG 12/13 from version requirements - MySQL: 5.7+/8.0+ → 8.4+ (8.0 EOL April 2026, 8.4 LTS to 2032) - PostgreSQL: 12+ → 14+ (12 and 13 already EOL) - CI already tests PG 16+18 and MySQL 8.4; docs now match Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add rubocop to Gemfile, fix autocorrect regressions - Add rubocop and plugins to Gemfile (was missing — CI failed) - Apply rubocop autocorrect to v4 files (parens on method calls, style consistency) - Revert harmful autocorrects: - Time.zone.now → Time.now (gem has no Rails Time.zone) - delegate macro → explicit methods (adapter is private) - class_double constant → string (class doesn't exist yet) - Add rubocop exclusions: - Rails/TimeZone for pool_manager.rb (pure Ruby gem) - Rails/Delegate for tenant.rb (intentional explicit delegation) - Regenerate appraisal gemfiles with rubocop deps Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use monotonic clock for pool timestamps instead of Time.now/Time.zone Pool timestamps are used for elapsed-time comparisons (idle detection, LRU ordering), not wall-clock display. Process::CLOCK_MONOTONIC is the correct choice: - Thread-safe (no thread-local Time.zone issues) - Not affected by system clock changes or NTP adjustments - No dependency on ActiveSupport Time.zone being configured Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Expose seconds_idle instead of raw monotonic timestamp in stats_for Follow ActiveRecord's convention (seconds_idle, seconds_since_last_activity, connection_age) of exposing computed durations rather than raw monotonic timestamps, which are meaningless outside the process. Before: { last_accessed: 123456.789 } # opaque monotonic float After: { seconds_idle: 42.3 } # human-meaningful duration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix rubocop: zero offenses across entire codebase - Fix DuplicateMethods: separate parallel_strategy/environmentify_strategy from attr_accessor (they have custom setters) - Fix line length in spec expect blocks (reformat alignment) - Add rubocop exclusions for intentional patterns: - Metrics complexity for build_adapter, validate!, PoolReaper.start - ThreadSafety/ClassInstanceVariable for PoolReaper (class singleton) - Rails/SkipsModelValidations for PoolManager#touch (not AR touch) - Lint/EmptyBlock for instrumentation fallback and spec assertions - ThreadSafety/NewThread for thread-safety tests - Style/OneClassPerFile for v3 postgresql_adapter - spec/dummy_engine excluded (v3 fixture) - Apply safe autocorrects to Phase 1 files (parens, string quotes) 122 files inspected, 0 offenses detected. 161 specs passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update CLAUDE.md: design/plan conventions, v4 status - Add Design & Plan Documents section documenting docs/ conventions (no date prefixes, no superpowers plugin paths) - Update header: remove stale man/spec-restart reference - Add v4 design spec and phase plan to Where to Start - Update Migration section → v4 Rewrite section with current status Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Improve CLAUDE.md files: add commands, trim verbosity, add v4 notes Root CLAUDE.md: - Add Commands section with copy-paste ready test/lint/build commands - Condense Core Concepts from ~80 lines to ~10 (detail is in docs/) - Condense Key Architecture Decisions into Key Patterns (~8 lines) - Replace verbose Testing/Debugging sections with compact versions Subdirectory CLAUDE.md files: - Add v4 staleness notes to lib/apartment/, lib/apartment/adapters/, and spec/ — these describe v3 code being replaced incrementally. Full rewrites deferred to Phase 2.5 when v3 files are deleted. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
README.md (718 → 290 lines): - Add "When to Use Apartment" section with honest comparison table (row-level vs schema-level vs database-level tenancy tradeoffs) - Update requirements: Ruby 3.3+, Rails 7.2+, PG 14+, MySQL 8.4+ - Consolidate elevator docs into compact reference - Condense config section into single block with all common options - Trim PostgreSQL extensions setup (keep essentials, cut legacy Heroku) - Simplify contribution guidelines (point to CONTRIBUTING.md) - Remove: GoRails video (v3 era), template1 hacks, Rails 4.1 notes CLAUDE.md (117 → 88 lines): - Fix stale branch reference (man/v4-adapters → development) - Update v4 status (Phase 2.1 merged, not "in PR") - Add Gotchas section: v3/v4 coexistence, frozen config, monotonic clock - Remove filler sections (Getting Help, Documentation Philosophy) lib/apartment/CLAUDE.md (303 → 81 lines): - Replace verbose per-method docs with concise v4/v3 file guide - Directory listing now shows [v4] and [v3] tags per file - Add v4 file descriptions (config, current, pool_manager, etc.) - Condense v3 section to bullet list (detail is in the source) - Add data flow summary for v4 tenant switching Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
"Eliminates...fiber safety" read like we were removing those features. Reworded to clarify: fixes the leakage bug, adds the new capabilities. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Add PostgreSQLSchemaAdapter for v4 Phase 2.2 Implements schema-based tenant isolation adapter that resolves connection configs via schema_search_path (tenant + persistent schemas). Handles nil postgres_config gracefully. Updates factory test to verify successful adapter construction now that the file exists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add PostgreSQLDatabaseAdapter for v4 Phase 2.2 Database-per-tenant strategy for PostgreSQL: resolves connection config with tenant-specific database name, creates/drops databases via DDL. Updates factory routing tests to verify instantiation now that the adapter file exists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Extract base_config to AbstractAdapter, apply conn local var pattern - Move duplicated `base_config` (connection_config.transform_keys) from PostgreSQLSchemaAdapter and PostgreSQLDatabaseAdapter to AbstractAdapter so all concrete adapters inherit it. - Apply `conn = ActiveRecord::Base.connection` local variable pattern to PostgreSQLSchemaAdapter DDL methods (consistency with DatabaseAdapter). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add MySQL2Adapter + TrilogyAdapter for v4 Phase 2.2 Replace v3 mysql2_adapter.rb and trilogy_adapter.rb with v4 versions using the same database-per-tenant pattern as PostgreSQLDatabaseAdapter. Update factory routing tests to verify proper instantiation instead of asserting NameError for missing constants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add SQLite3Adapter for v4 Phase 2.2 Replace v3 Sqlite3Adapter with v4 SQLite3Adapter using the established abstract adapter pattern. File-per-tenant strategy: create_tenant ensures directory exists (SQLite creates file on first connect), drop_tenant uses FileUtils.rm_f for idempotent deletion. Update factory routing test from NameError assertion to successful instantiation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Clarify database_file fallback in SQLite3Adapter Use explicit ternary instead of File.dirname on a synthetic path. Makes the intent clear: extract dir from config, or default to 'db'. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address Phase 2.2 deferred review items - Guard environmentify against Rails not being defined: extract rails_env private method that raises ConfigurationError with a clear message when :prepend/:append strategy is used outside Rails. - Update stale comment in apartment_spec.rb factory test. - Add test for the Rails-undefined guard. 229 examples, 0 failures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Check off Phase 2.2 deferred review items in plan Mark adapter factory tests, environmentify Rails guard, and persistent_schemas usage items as resolved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix trailing blank line in PostgreSQLDatabaseAdapter Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address PR review findings Critical fixes: - Add IF EXISTS to PostgreSQLSchemaAdapter DROP SCHEMA for idempotent drops, consistent with all other adapters. - Fix stale comment referencing nonexistent ConnectionHandling module; now references Phase 2.3 explicitly. Important improvements: - Add tests documenting that schema adapter intentionally does NOT environmentify tenant names (schemas are named directly, unlike database-per-tenant adapters). - Add class docstrings to MySQL2Adapter and SQLite3Adapter for consistency with PostgreSQL adapters. - Improve PostgreSQLSchemaAdapter docstring to note the no-environmentify design choice. 231 examples, 0 failures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update CLAUDE.md files to reflect Phase 2.2 completion - lib/apartment/CLAUDE.md: Update directory structure to show v4 adapter files, add concrete adapter descriptions, mark remaining v3 files with their replacement phase. - lib/apartment/adapters/CLAUDE.md: Update note to reflect that v4 concrete adapters are implemented, v3 files are Zeitwerk-ignored. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update spec counts and status in CLAUDE.md files - Root CLAUDE.md: 161 → 231 specs, add Phase 2.2 to status line - spec/CLAUDE.md: 161 → 231 specs, remove stale branch reference, list all concrete adapter specs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Add Phase 2.3 planning: design spec, research doc, implementation plan Phase 2.3 covers ConnectionHandling patch (tenant-aware pool resolution via AR shard keying), PoolReaper class-to-instance conversion, and Config additions (shard_key_prefix, rails_env_name). Research doc covers AR connection handling internals across Rails 7.2/8.0/8.1 and prior art (activerecord-tenanted, Julik's shardines). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Config: add shard_key_prefix and rails_env_name for Phase 2.3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * PoolReaper: convert from class singleton to instance Addresses deferred Phase 1 review item. Instance API enables test isolation and removes global mutable state. AR handler deregistration added (no-op until shard_key_prefix is configured). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Apartment module: instance PoolReaper, activate!, teardown protection - configure creates PoolReaper instance and starts it - clear_config tears down reaper instance - teardown_old_state rescues reaper.stop failures - deregister_all_tenant_pools cleans AR handler on teardown - activate! prepends ConnectionHandling patch on AR::Base Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * ConnectionHandling: tenant-aware pool resolution via AR shard keying Core Phase 2.3 deliverable. Prepends on AR::Base to override connection_pool. Reads Current.tenant, lazily creates pools via establish_connection with namespaced shard keys. Pools are immutable and tenant-scoped — no SET search_path at switch time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix connection_handling_spec graceful skip without sqlite3 The spec requires real AR + sqlite3 (only in appraisal gemfiles). When run with the base Gemfile, it skips all 15 examples gracefully instead of erroring. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Phase 2.3 cleanup: rubocop fixes, deferred items resolved, docs updated - Fix rubocop offenses (line length, method call parens) - Disable Metrics/MethodLength on connection_pool (inherently sequential) - Disable Metrics/AbcSize on validate! (additional shard_key_prefix check) - Check off Phase 2.3 deferred items in phase plan - Update lib/apartment/CLAUDE.md with patches/ directory - Load real AR in spec_helper when available (fixes test ordering) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use configurable shard_key_prefix in HashConfig spec name The HashConfig name (used in AR error messages/logging) was hardcoded to "apartment_" instead of using the configurable shard_key_prefix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix PR review findings: drop AR cleanup, TOCTOU, custom prefix test - AbstractAdapter#drop now deregisters shard from AR's ConnectionHandler (was only removing from PoolManager, leaving stale handler entry) - ConnectionHandling#connection_pool captures config in local variable to eliminate TOCTOU window on concurrent reconfigure - Add tests for custom shard_key_prefix wiring in connection_pool - Replace hardcoded :apartment_acme with config-derived shard keys in tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Document remaining test gaps from Phase 2.3 PR review Deferred to pick up opportunistically: activate! path test, deregister_all_tenant_pools effect test, concurrent connection_pool access test, drop + AR handler cleanup test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Rubocop: parenthesize require_relative in activate! Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update CLAUDE.md: Phase 2.3 status Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix type mismatch in tenant comparison, extract shared deregister_shard Code review findings: - connection_pool: tenant == default fails when tenant is a symbol and default is a string. Use .to_s on both sides. - Extract Apartment.deregister_shard(tenant) as single source of truth for AR handler deregistration. PoolReaper, AbstractAdapter#drop, and teardown all delegate to it instead of duplicating the logic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address comprehensive PR review findings - connection_pool: wrap in rescue to produce domain-specific error with tenant name context instead of raw AR exceptions - connection_pool: explicit nil-config guard (was relying on coincidental nil-safety chain) - connection_pool: add rationale comment for shard keying approach - teardown_old_state: fix comment to match actual execution order (stop reaper → deregister → clear, not deregister → clear → stop) - CLAUDE.md: update PoolReaper description from "class singleton" to "instance created by configure" - pool_manager.rb: update stale "Phase 2+" to "Phase 3" - connection_handling_spec: narrow rescue in REAL_AR_AVAILABLE guard, add warn messages for skip reasons Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Rubocop: fix spec style offenses Parenthesize require/require_relative/sleep, merge scattered before hooks in connection_handling_spec. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ests (#354) Excluded models improvements: ConfigurationError on bad model names, schema table prefix for PG, early return guard, seed raises on missing file, drop resilient to disconnect! failures. Idempotent DDL: CREATE SCHEMA IF NOT EXISTS (PG), CREATE DATABASE IF NOT EXISTS (MySQL), PG::DuplicateDatabase rescue → TenantExists. Multi-engine integration test infrastructure (V4IntegrationHelper) with SQLite/PostgreSQL/MySQL support. 60 integration specs per engine covering: tenant switching, lifecycle, excluded models, edge cases (switch!, nested create, clear_config teardown, real seed/migrate), PG schema-specific (search_path, persistent schemas, DDL, table prefix), PG database-per-tenant, MySQL-specific (DB creation/drop, environmentify, rapid switching). Stress tests: 10-thread concurrent switching, pool creation idempotency, 50-tenant scaling, PoolReaper idle+LRU eviction, parallel creation storm, PG search_path isolation across threads, concurrent schema/DB creation, concurrent drop during active query. Coverage gap tests: pool_stats, stats_for, LRU eviction, instrumentation notifications, Tenant.init. CI workflow updated to run v4 integration tests per engine.
- v4 Railtie (after_initialize, elevator middleware, rake tasks) - TenantNameValidator (pure in-memory format validation per engine) - schema_load_strategy config + import_schema in AbstractAdapter#create - ConnectionHandler swap for hermetic integration tests - Scenario-based database configs (YAML + helpers) - Dummy app upgraded to v4 + request lifecycle tests - SimpleCov + TestProf tooling - CI workflow re-enabled with matrix filter inputs
* Phase 2.5: Delete replaced v3 files, rename classes to Zeitwerk conventions Deleted: - v3 adapters: postgresql_adapter, postgis_adapter, abstract_jdbc_adapter, jdbc_mysql_adapter, jdbc_postgresql_adapter - v3 modules: deprecation, log_subscriber, console, custom_console, migrator, model - v3 active_record/ directory (replaced by patches/connection_handling) - v3 specs: spec/adapters/, spec/unit_v3/, spec/tasks/ Renamed classes to match Zeitwerk autoload expectations: - MySQL2Adapter → Mysql2Adapter - PostgreSQLSchemaAdapter → PostgresqlSchemaAdapter - PostgreSQLDatabaseAdapter → PostgresqlDatabaseAdapter - SQLite3Adapter → Sqlite3Adapter - MySQLConfig → MysqlConfig - PostgreSQLConfig → PostgresqlConfig Cleaned up Zeitwerk ignores: only railtie.rb (explicit require), elevators/ (still v3, Phase 3), and tasks/ (loaded by Railtie) remain ignored. Adapters and patches now autoload. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Delete remaining v3 orphans, update CLAUDE.md files Caught by PR review: deleting migrator.rb left dangling requires in lib/tasks/apartment.rake, task_helper.rb, and the dummy app's user_with_tenant_model.rb. Also deleted v3 integration specs and the empty lib/tasks/ directory. Updated all four CLAUDE.md files to reflect deletions and class renames (Zeitwerk conventions). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Delete v3 spec orphans and stale rubocop exclusions Found by parallel review agents: - .rubocop.yml: 5 exclusions for deleted files - spec/tenant_spec.rb: v3 test using deleted API (use_schemas, reload!) - spec/apartment_spec.rb: v3 integration test requiring Dummy::Application - spec/support/: v3 helpers (apartment_helpers, contexts, setup, config, requirements) - spec/examples/: shared examples only used by deleted v3 adapter specs - spec/schemas/: schema fixtures only used by deleted v3 specs - spec/dummy_engine/: test engine only used by deleted v3 integration spec - CLAUDE.md: stale v3/v4 coexistence gotcha Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rewrite all elevator middleware for v4: constructor-only config via elevator_options keyword args, no class-level mutable state. - Generic: **_options splat, NotImplementedError default - Subdomain: excluded_subdomains: keyword arg, remove class-level setter - FirstSubdomain: fix double-super call - Domain: tests only (no code change) - Host: ignored_first_subdomains: keyword arg, remove class-level setter - HostHash: hash: keyword arg, drop positional processor - Header: new — HTTP header-based tenant resolution with .presence guard - Railtie: **opts, resolve_elevator_class handles symbols + classes, header_trust_warning? extracted for testability - Remove Zeitwerk ignore for elevators directory - 49 new elevator unit tests, Header integration test (381 total unit specs)
* Add v4 migrations design spec (Phase 4) Covers Migrator architecture, schema dumper patch, schema cache, and rake/Thor task integration. Key decisions: threads-only parallelism, standalone PoolManager for RBAC credential separation, no per-schema advisory locks, support for both schema.rb and structure.sql. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address spec review: config placement, multi-adapter, idempotency Fixes from spec-document-reviewer: - CRITICAL: include_schemas_in_dump references existing PostgresqlConfig location instead of creating conflicting top-level attribute - CRITICAL: migration_db_config validated eagerly in Config#validate! - IMPORTANT: Adapter-specific behavior documented for MySQL/SQLite - IMPORTANT: structure.sql pg_dump flag dependency noted - IMPORTANT: Rollback syntax matches existing v4.rake argument style - IMPORTANT: parallel_strategy removal from Config made explicit - IMPORTANT: db:migrate:DBNAME idempotency clarified - MINOR: MigrationRun methods inside Data.define block - MINOR: Instrumentation event follows verb.namespace convention - MINOR: Concurrent::Array dependency noted - MINOR: PendingMigrationError flagged as new error class for Phase 5 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Respect ActiveRecord.dump_schema_after_migration in rake tasks References ros-apartment#342 — v3 used the removed ActiveRecord::Base.dump_schema_after_migration; v4 rake tasks use the module-level accessor to gate schema dump phase. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add v4 migrations implementation plan (Phase 4) 13 tasks covering: config changes, Migrator with Result/MigrationRun, credential overlay with string-key normalization, sequential + parallel execution, Phase 1 primary DB migration, schema dumper patch, rake task wiring, and Railtie db:migrate:DBNAME enhancement. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Config: add migration_db_config, schema_cache_per_tenant; remove parallel_strategy Drops VALID_PARALLEL_STRATEGIES and the parallel_strategy attr/setter (replaced by a threads parameter on the Migrator). Adds migration_db_config (nil|Symbol, validated setter) and schema_cache_per_tenant (boolean, plain attr_accessor) for Phase 4. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add Apartment::MigrationError for per-tenant migration failures * Add Migrator::Result and MigrationRun value objects * Migrator: constructor and credential overlay logic * Migrator: sequential #run with per-tenant result tracking Adds #run (Phase 1 primary + Phase 2 tenants), migrate_primary, migrate_tenant, run_sequential, resolve_migration_db_config, create_pool. 8 new specs covering all paths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Migrator: thread-based parallel execution via Queue * Schema dumper patch: strip public. prefix for Rails 8.1+ * Rake: wire apartment:migrate through Migrator with parallel support * Railtie: enhance db:migrate:DBNAME to trigger apartment:migrate * Railtie: activate schema dumper patch on Rails 8.1+ * Integration tests for Migrator (sequential, parallel, idempotency) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Update CLAUDE.md docs for Migrator and schema dumper patch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix rubocop offenses in migrator, rake tasks, and railtie Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix integration test syntax and skip parallel on SQLite - Fix SyntaxError from comma-separated expect message - Fix multi-arg RSpec.describe parsing - Skip parallel tests on SQLite (no concurrent connection support) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address PR review: primary abort, thread safety, dead code cleanup - Migrator#run returns early when primary migration fails; rake now exits non-zero without touching any tenants on primary DB failure - run_parallel rescues Exception in threads to prevent orphaned workers on Interrupt/NoMemoryError; re-raises after all workers are joined - ensure block wraps pool_manager.clear to prevent masking original exception - create_pool tracks spec_names; deregister_migration_pools does best-effort ConnectionHandler cleanup after each run to prevent ghost pool accumulation - MigrationRun#summary includes exception class+message for failed tenants - Delete MigrationError (unused); remove schema_cache_per_tenant (no consumer) - Update unit tests: primary-fail abort, parallel tenant failure, summary format Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix parallel migration: use unique owner_name per pool to prevent slot collision establish_connection without owner_name: uses ActiveRecord::Base as the owner for all pools, mapping them to a single slot per role/shard. In parallel mode, Thread 2 establishing a connection for tenant B calls disconnect_pool_from_pool_manager on Thread 1's tenant A pool, causing ConnectionNotEstablished when A's migration context tries to check out a connection. Fix: pass owner_name: spec_name so each ephemeral migration pool occupies its own slot in the ConnectionHandler, eliminating the overwrite race. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix pool creation: standalone pools + reuse AR::Base pool for primary Two changes to fix parallel migration across all adapters: 1. create_pool builds ConnectionPool directly via PoolConfig instead of registering through ConnectionHandler.establish_connection. Uses a MigrationPoolOwner duck type (responds to name/primary_class?) that works across Rails 7.2-8.1. This eliminates both the parallel slot collision (PG/MySQL) and the duplicate-pool file lock (SQLite). 2. migrate_primary reuses ActiveRecord::Base.connection_pool when no migration_db_config credential override is active. Only creates a separate pool when RBAC credential separation is needed. Prevents SQLite "database is locked" from two pools hitting the same file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add current_preventing_writes to MigrationPoolOwner for Rails 7.2 compat Rails 7.2's AbstractAdapter#preventing_writes? calls connection_class.current_preventing_writes. Without this method, standalone migration pools raise NoMethodError on any SQL execution. Rails 8.0+ ConnectionDescriptor also defines it; our duck type must match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Revert pool isolation experiments (6fc00b6, 4e884ab, 518ded3) Standalone pools don't work: Rails' Migrator#connection hardcodes DatabaseTasks.migration_connection -> AR::Base.lease_connection, bypassing any pool we create. The parallel slot collision needs a different approach — see design discussion in upcoming brainstorm. Reverts to the original handler.establish_connection approach from pre-review. The review fixes (primary abort, thread safety, dead code removal) from 1dc65ab are preserved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Rewrite Migrator to use Tenant.switch instead of standalone pools Rails' Migrator#connection hardcodes DatabaseTasks.migration_connection -> AR::Base.lease_connection, bypassing any standalone pool. But v4's ConnectionHandling patch intercepts AR::Base.connection_pool and routes to the tenant's pool when Current.tenant is set. So Tenant.switch is the correct integration point: it sets Current.tenant, and all AR::Base connection lookups (including from inside Rails' migration runner) automatically resolve to the tenant's pool. This eliminates: - PoolManager/MigrationPoolOwner/create_pool/deregister machinery - Standalone ConnectionPool construction (incompatible with Rails) - Thread-local ConnectionHandler swaps (unnecessary complexity) Parallel safety: Current.tenant uses CurrentAttributes (thread/fiber- local via IsolatedExecutionState). Each worker thread's Tenant.switch sets its own Current.tenant, so connection_pool lookups resolve to the correct tenant pool per thread. The PoolManager (Concurrent::Map) is thread-safe for concurrent reads. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Disable advisory locks for tenant migrations (PG parallel fix) PG schema-per-tenant shares one database. Rails' advisory locks are database-wide, so parallel tenant migrations block each other with ConcurrentMigrationError. Each tenant schema is independent — there is no cross-schema migration coordination needed. Disable advisory locks on the connection before running tenant migrations. Primary migration keeps advisory locks enabled (it runs single-threaded before tenants start). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix misleading comment: advisory lock disable is a known trade-off Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address PR review: remove dead code, fix silent failures, update spec Dead code removed: - migration_db_config (constructor param, config key, setter, resolver methods, rake wiring, unit tests). RBAC credential overlay deferred to Phase 5 — the Tenant.switch approach uses runtime pools, and credential separation requires adapter-level support. - CREDENTIAL_KEYS constant (only used by removed overlay methods) - schema_cache_per_tenant (removed in earlier commit, now also removed from design spec) Silent failures fixed: - Advisory lock disable now uses lease_connection (returns the same connection object for the current thread) instead of with_connection (which checks out and returns a potentially different connection) - Schema dumper patch guards on postgres_config presence — falls through to super when config is nil or non-PG - Rake tasks create/seed/rollback now track failures and abort non-zero, consistent with migrate task - Railtie db:migrate:DBNAME enhancement removes phantom NoDatabaseError rescue (configs_for doesn't touch the database) - Unit test "switches tenant" converted to spy (have_received) so the allow stub still yields — previous expect/receive override suppressed the block, making the test a false positive - should_patch? test now stubs gem_version instead of keyword defined? Design spec updated: - Documents Tenant.switch architecture (replaces standalone pools) - Documents why RBAC credential overlay is deferred - Removes migration_db_config and schema_cache_per_tenant from Phase 4 - Updates execution flow diagram and testing strategy Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix schema dumper patch, add VERSION= support, clean up stale references Schema dumper patch (critical fix): - Intercept `relation_name` on PG-specific SchemaDumper instead of `table` on the base class. Rails 8.1 PR #50020 adds public. prefix via `relation_name` in PostgreSQL::SchemaDumper — the base class `table` method receives bare names before qualification. The old patch was a no-op. The new interception covers all 5 call sites (tables, foreign keys, enums, indexes, references) through a single override point. Migrator: - Add `version:` parameter, passed through to `context.migrate(version)`. Rake task reads `ENV['VERSION']` and passes it as integer. - When version is set, skip the `needs_migration?` check (could be a targeted rollback to an older version). - Wrap advisory lock disable in ensure block to restore original value after migration completes. - Extract `with_advisory_locks_disabled` helper for clarity. Design spec cleanup: - Remove RBAC integration test from testing strategy (deferred to Phase 5) - Fix composability section to not imply migration_db_config is implemented - Add version targeting to integration test list Tests: - Add `version:` constructor and pass-through tests - Add advisory lock restore verification - Add all-success summary test (no failed section in output) - Add `apply!` test verifying PG-specific SchemaDumper prepend - Convert switches-tenant test to spy pattern (have_received) - Fix mock_connection to stub instance_variable_get for restore Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Add Phase 5 design spec: role-aware connections, RBAC, schema cache Covers five features for Apartment v4 Phase 5: 1. Role-aware ConnectionHandling — resolves tenant pool base config from the current role's default pool via Rails' native connected_to(role:). Fixes replica routing and RBAC credential separation. Pool keys become "tenant:role" format with updated lifecycle (drop, eviction, deregistration). 2. migration_role config — Migrator wraps work in connected_to(role:) for elevated DDL credentials. Per-fiber safety via IsolatedExecutionState. 3. app_role RBAC grants — built-in PG (6 statements) and MySQL (1 statement) privilege grants on tenant create, with callable escape hatch. 4. schema_cache_per_tenant — opt-in per-tenant schema cache files with explicit generation via Apartment::SchemaCache module. 5. PendingMigrationError — development-only check on tenant pool creation, suppressed during migration via Current.migrating. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Phase 5 implementation plan: 12 tasks, TDD, bite-sized steps Covers: role-aware ConnectionHandling, adapter interface change, PoolManager/PoolReaper composite key support, RBAC grants, Migrator migration_role, SchemaCache module, PendingMigrationError, rake tasks, integration tests, and final verification. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Current.migrating attribute and PendingMigrationError Implements Task 1 of Phase 5 RBAC & Schema Cache support. Current.migrating tracks whether migrations are actively running. It will be used by Migrator to suppress PendingMigrationError during tenant pool creation. PendingMigrationError is raised in development when a tenant pool is created with pending migrations. It includes the tenant name and directs users to run apartment:migrate. Both follow the established pattern: - Current.migrating as fiber-safe CurrentAttributes - PendingMigrationError extends ApartmentError with tenant accessor Tests added for Current (defaults to nil, can be set/read/reset) and PendingMigrationError (message format, tenant accessor, inheritance). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Phase 5 config keys: migration_role, app_role, schema_cache_per_tenant, check_pending_migrations Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Pool lifecycle: remove_tenant, evict_by_role, composite key deregistration, prefix guard PoolManager gains remove_tenant and evict_by_role for bulk pool removal by tenant prefix or role suffix. PoolReaper replaces the equality guard on default_tenant with default_tenant_pool? which also covers composite keys (e.g. public:writing). Apartment.deregister_shard now parses the role from the composite pool key via rpartition instead of using current_role. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Reject colons in tenant names (composite pool key delimiter) * Adapter interface: base_config_override keyword, drop with composite pool keys validated_connection_config now accepts base_config_override: so ConnectionHandling (Task 7) can pass a role-specific base config; adapters apply tenant modifications on top. drop switches from pool_manager.remove(tenant) to remove_tenant(tenant), cleaning up all role-variant pool keys and deregistering each composite pool_key from AR's ConnectionHandler. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix TestAdapter override test and protect deregister_shard in drop Issue 1: TestAdapter#resolve_connection_config was returning hardcoded hash and ignoring base_config: parameter, making the base_config_override test ineffectual. Fixed by using base_config when provided and merging with tenant database key. Updated test expectations to include all base_config keys (adapter, database, host). Issue 2: deregister_shard_from_ar_handler was outside the rescue block in drop(), meaning if deregistration raised, remaining pools would not be cleaned up. Moved deregistration into its own rescue block so all pools attempt cleanup regardless of individual failures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * RBAC grants: grant_privileges in PG schema (6 SQL) and MySQL (1 SQL) adapters Dispatch via grant_tenant_privileges in AbstractAdapter#create (after create_tenant, before import_schema); string app_role triggers built-in grants, callable is an escape hatch, nil is a no-op. PostgresqlDatabaseAdapter inherits the no-op with an explanatory comment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Role-aware ConnectionHandling: resolve base config from current role's default pool Pool key is now "tenant:role" instead of just tenant, so connected_to(role:) blocks create per-role pools with the correct base connection config. Adds check_pending_migrations? and load_tenant_schema_cache private methods. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Migrator: with_migration_role, Current.migrating flag, post-migration pool eviction Wraps migrate_primary and migrate_tenant in connected_to(role: migration_role) so tenant pools created during migration use elevated credentials. Sets Current.migrating = true per tenant to suppress PendingMigrationError during the run. Evicts all migration-role pools in an ensure block on run so elevated-credential pools don't persist after migrations complete. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add Apartment::SchemaCache module for per-tenant cache generation Implements explicit schema cache dumping for each tenant via Tenant.switch. Used by ConnectionHandling (Task 7) for loading caches at boot, and by the rake task (Task 10) for on-demand cache regeneration. - SchemaCache.dump(tenant): switches and dumps schema_cache.yml - SchemaCache.dump_all: dumps for all tenants from tenants_provider - SchemaCache.cache_path_for(tenant): resolves cache file path Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add apartment:schema:cache:dump rake task * Fix connection_handling_spec for composite pool key format Update shard key and PoolManager lookups from `tenant` to `tenant:role` format following Phase 5's composite pool key change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix connection_handling_spec: add role: to pool lookups, disable pending check - Add role: parameter to retrieve_connection_pool calls (required for role-qualified shard lookup) - Set check_pending_migrations: false in all configure blocks (prevents needs_migration? from firing on mock pools when Rails.env.local? is true) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Lint fixes and evict_migration_pools bug fix - Add rubocop:disable for Metrics offenses (ClassLength, CyclomaticComplexity, MethodLength, AbcSize) on methods that grew in Phase 5 - Suppress Rails/UnknownEnv for Rails.env.local? (valid Rails 7.1+ method) - Fix evict_migration_pools: each_key → each (evict_by_role returns array of [key, pool] pairs, not a hash) - Auto-correct style offenses from rubocop Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Suppress false-positive HashEachMethods on array destructure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add test coverage for multi-role pool keys, Current.migrating lifecycle, schema cache paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix integration tests for Phase 5 composite pool keys and pending migration check - Add `c.check_pending_migrations = false` to all 28 `Apartment.configure` blocks in integration test files to prevent PendingMigrationError in local/test envs - Update all `tracked?`, `stats_for`, `get`, and `lru_tenants` calls to use the composite key format `"tenant:role"` (e.g. `"doomed:writing"`) introduced by Phase 5's `ConnectionHandling#connection_pool` pool key change - Update `stats[:tenants]` include assertions to use composite keys Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix ConnectionHandling: super must be called inline, not from helper super in a prepended module's private method looks for that method name in the superclass (NoMethodError). Move the super call inline in the fetch_or_create block where it correctly refers to the original connection_pool method. Add rescue for ConnectionNotEstablished so role-aware resolution gracefully falls back to adapter's base_config when the default pool is not accessible (e.g., test handler swaps). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix stress test pool exhaustion: re-establish default connection with bumped pool The stress test bumps pool size to 15 for 10 concurrent threads, but only in the adapter config. Phase 5's role-aware ConnectionHandling resolves base config from the default pool (via super), which still had the original pool size (5). Re-establish the default connection with the bumped config so tenant pools inherit the larger pool size. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix all rubocop offenses: merge describe blocks, auto-correct style - Merge three separate RSpec.describe(Apartment::Migrator) blocks into one (fixes RSpec/RepeatedExampleGroupDescription, RSpec/DescribeMethod) - Auto-correct: unused variable prefix, ternary parens, receive counts, argument indentation, be vs eq, line length, block delimiters, safe navigation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Add Phase 5.2 design spec: RBAC integration tests Design for integration tests that verify Phase 5's role-aware connections, RBAC privilege grants, and Migrator migration_role against real PostgreSQL roles and MySQL users. Key decisions: - Separate connections (not SET ROLE) to align with v4's pool-per-tenant architecture - CI provisions apt_test_db_manager/apt_test_app_user roles - before(:context, :rbac) hook with local dev fallback - Three PG spec files + one MySQL grant spec Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Phase 5.2 implementation plan: RBAC integration tests 8 tasks: CI provisioning, RbacHelper module, 3 PG spec files (role-aware connections, grants, Migrator), 1 MySQL grant spec, full suite verification, and documentation updates. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * ci: add RBAC test role provisioning for PG and MySQL Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add RbacHelper module for RBAC integration tests Engine-aware role provisioning (PG CREATE ROLE, MySQL CREATE USER), connect_as/restore for grant tests, setup_connects_to! for role-aware routing tests. Auto-skip with actionable message when roles unavailable. * Add role-aware connection integration tests Verify ConnectionHandling creates separate pools per role for the same tenant, with distinct pool keys and correct username propagation from the active connected_to role's base config. * Add RBAC privilege grant integration tests (PostgreSQL) Verify app_user can DML but not DDL in tenant schemas. Verify ALTER DEFAULT PRIVILEGES fire for tables created after initial grants. Verify db_manager retains full DDL privileges. * Add Migrator RBAC integration tests (PostgreSQL) Verify Migrator with migration_role: :db_manager uses elevated credentials (table ownership check), app_user gets DML via default privileges, migration-role pools evicted after run, and parallel threads each use db_manager credentials. * Add RBAC privilege grant integration tests (MySQL) Verify app_user can DML but not CREATE TABLE or DROP DATABASE. Verify db_manager retains full DDL privileges. * Fix Rubocop offenses in RBAC integration tests Auto-corrected style offenses (trailing commas, parentheses, block delimiters, squished SQL heredocs). Aligned skip conditions with existing spec patterns. Added rbac_helper.rb to ThreadSafety ClassInstanceVariable exclusion list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update docs with RBAC integration test instructions Add RBAC test commands to CLAUDE.md and mark RBAC integration coverage in spec/CLAUDE.md. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address PR review findings: connection safety, cleanup, quoting Critical: - Add ensure blocks around inline connect_as calls in migrator_rbac and rbac_grants specs to prevent connection leaks on test failure - Add establish_default_connection! to role_aware_connection_spec after hook, matching the cleanup pattern in all other RBAC specs Important: - CI PG provisioning connects to 'postgres' db (not test db that may not exist yet) - teardown_rbac_connections! now calls restore_default_connection! when stashed config exists (name matches behavior) - Use connection.quote() for pg_tables WHERE clause instead of raw string interpolation Suggestions: - More specific error messages in provision_roles! (distinguishes privilege errors from unexpected failures) - Improved comments on setup_connects_to! and connect_as stash guard - Added function execute grant test (closes last uncovered PG grant) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix CI failures: PG public schema + MySQL GRANT OPTION PG 15+ revoked CREATE ON SCHEMA public FROM PUBLIC. db_manager needs explicit CREATE ON SCHEMA public to run migrate_primary (which creates schema_migrations in the public schema). MySQL GRANT ALL ON *.* does not include GRANT OPTION by default. db_manager needs WITH GRANT OPTION to grant DML to app_user during tenant creation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address Cursor review: doc drift, clarifying comments - Design doc CI snippets now match actual code (-d postgres, WITH GRANT OPTION) - MySQL wildcard grant comment clarifies it's a safety net; per-tenant grants come from Mysql2Adapter#grant_privileges - provision_roles! documents one-shot semantics - Migration class version [7.2] explains why it's pinned - Pool eviction test documents intentional coupling to key format Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(spec): align RbacHelper provisioning comment with before(:context, :rbac) Made-with: Cursor * Fix PG schema_migrations access + MySQL result access PG: db_manager needs ALL ON SCHEMA public (not just CREATE) plus access to existing tables in public. schema_migrations may already exist from earlier integration tests, owned by postgres. MySQL: connection.execute returns raw Mysql2::Result where .first yields an array, not a hash. Use select_value for scalar queries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix PG: ALTER DEFAULT PRIVILEGES for future public schema tables GRANT ALL ON ALL TABLES only covers tables existing at provisioning time. With RSpec randomization, non-RBAC specs may create schema_migrations in public (as postgres) AFTER RBAC provisioning runs. ALTER DEFAULT PRIVILEGES ensures db_manager gets access to tables created by the provisioning user in public going forward. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Tighten ALTER DEFAULT PRIVILEGES comment + sync design doc Fix comment: "without FOR ROLE, applies to objects created later by the current session user" (not "uses the current user as the grantor"). Sync design doc to describe the full public schema grant chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix parallel Migrator test: skip ConnectionHandler swap Worker threads can't resolve the :db_manager pool registered by setup_connects_to! when the per-example ConnectionHandler swap is active — super in ConnectionHandling falls back to nil, and the adapter uses its base_config (postgres credentials). Tag the parallel context with :stress to use the original handler, matching the pattern used by existing concurrency tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Harden cleanup, CI error handling, and design doc accuracy Cleanup ordering: - Move teardown_rbac_connections! to first line of all after blocks so stashed config is cleared before establish_default_connection! prevents overwriting the restored connection Connection safety: - restore_default_connection! clears @stashed_config before reconnecting to prevent cross-test poisoning if establish raises CI reliability: - PG provisioning uses --set ON_ERROR_STOP=on so psql fails loudly on partial state (e.g., role created but GRANT failed) - MySQL provisioning uses set -euo pipefail for the same reason Pre-existing fix: - support.rb clear_all_connections! rescue now logs the error instead of silently swallowing it Design doc: - Fix stale before(:suite) reference (now before(:context, :rbac)) - MySQL table adds WITH GRANT OPTION - YAML snippets match real CI structure (separate jobs, no if:) - Snippets include ON_ERROR_STOP and pipefail Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ing (#361) * Phase 5.3: PG database-per-tenant callable app_role + MySQL test tightening Two RBAC test gaps from Phase 5: 1. PostgresqlDatabaseAdapter callable app_role integration test: Prove the escape hatch works end-to-end with database-per-tenant PG. Creates tenant with a callable app_role that issues grants inside the tenant DB. Verifies: callable receives tenant + live connection, app_user can DML, app_user cannot DDL, ALTER DEFAULT PRIVILEGES cover future tables. 2. MySQL test tightening: Remove apartment_% wildcard grant from provisioning (CI + helper). MySQL RBAC tests now depend entirely on Mysql2Adapter#grant_privileges fired during adapter.create — no safety net masking the production grant code path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix PG database-per-tenant grants + MySQL no-database connect PG: The callable app_role receives (tenant, conn) where conn points at the default database. For database-per-tenant, grants must run inside the tenant DB. The callable now uses Tenant.switch(tenant) to get a connection to the tenant database before issuing grants. MySQL: Without the wildcard safety net, app_user has no access to the default test database. connect_as now accepts **overrides so MySQL specs can pass database: nil (all queries already use schema-qualified `db_name`.table syntax). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix PG database-per-tenant: explicitly REVOKE CREATE from PUBLIC PG template1 may or may not have REVOKE CREATE ON SCHEMA public FROM PUBLIC depending on the Docker image/version. The callable now explicitly revokes DDL from PUBLIC in the tenant database, ensuring the test verifies a real privilege boundary rather than depending on template defaults. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix REVOKE: must run as superuser, not db_manager db_manager doesn't own the public schema in tenant databases (template1 makes postgres the owner). PG silently ignores REVOKE from non-owners (WARNING, not ERROR). Move the REVOKE out of the callable and into the before block, running as postgres (superuser) after adapter.create completes. The callable now only handles DML grants — matching a realistic production pattern where DDL restrictions come from the DBA, not the application. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix REVOKE: bypass pool_manager cache for superuser connection The callable during adapter.create caches a tenant pool with db_manager credentials in pool_manager. Subsequent Tenant.switch calls reuse this cached pool, so the REVOKE ran as db_manager (who can't revoke on schemas owned by postgres) instead of postgres (superuser). Direct establish_connection to the tenant DB bypasses the pool cache, ensuring the REVOKE runs as the superuser. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Refactor PG database-per-tenant spec to use connected_to(role:) Use setup_connects_to! + connected_to(role: :db_manager) for tenant creation — the production pattern via connects_to. This creates pool "tenant:db_manager" with db_manager credentials. REVOKE runs under the :writing role (postgres/superuser) via plain Tenant.switch, creating a separate pool "tenant:writing" — no cache collision with the db_manager pool. connect_as(:app_user, database: tenant) is used only for grant verification assertions (app_user isn't an AR role; it's a PG role for privilege boundary testing). Eliminates the connect_as(:db_manager) / restore dance that caused pool key collisions on "tenant:writing". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add sequence grants to callable — INSERT needs nextval serial columns use sequences for auto-increment. The callable granted DML on tables but not sequences, so INSERT failed with "permission denied for sequence widgets_id_seq". Add GRANT USAGE, SELECT ON ALL SEQUENCES + ALTER DEFAULT PRIVILEGES for sequences — matching the schema adapter's 6-statement pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review: grant_log ivar + narrower rescue scope - Convert let(:grant_log) to @grant_log instance variable to avoid fragile let-memoization interaction with lambda capture in before - Narrow force_drop_database rescue from StandardError to StatementInvalid + ConnectionNotEstablished — lets genuine bugs (NoMethodError, TypeError) surface instead of being warned away Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Strengthen assertion + CI guard against silent RBAC skips - Assert callable receives db_manager role name (not just any String) - Add CI step that reruns :rbac specs with JSON output and fails if zero examples ran — catches silent provisioning failures that would otherwise let CI pass with all RBAC specs skipped Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix RBAC guard: tail -1 to skip migration stdout before JSON The Migrator spec outputs migration progress to stdout, which pollutes the JSON output from --format json. tail -1 grabs only the final JSON line that RSpec emits. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two production code fixes from Cursor review debt (PRs #345-361): 1. AbstractAdapter#create now validates the environmentified name (not just the raw tenant name) against engine-specific length limits. Previously, a 59-char name with :prepend strategy would pass PG's 63-char validation but produce a 64-char identifier that fails at the database with a raw PG::InvalidName error. 2. Mysql2Adapter#grant_privileges now uses connection.quote_table_name for the database identifier, matching the quoting convention used by PG adapters and the adapter's own create_tenant/drop_tenant. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Phase 6 design spec: CLI & Generator Covers Thor CLI architecture (per-file subcommands modeled on CampusESP www schema tasks), rake task refactor to thin wrappers, v4 generator template rewrite, and binstub generation. Two sub-phases: 6.1 (CLI + rake) and 6.2 (generator + binstub). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address design review feedback for Phase 6 spec - Single-tenant migrate delegates to new Migrator#migrate_one(tenant) instead of reimplementing migration logic (preserves RBAC, advisory locks, instrumentation) - Pool evict uses new PoolReaper#run_cycle public API instead of reaching into private #reap - Rake apartment:create[tenant] supports optional single-tenant arg - APARTMENT_FORCE/APARTMENT_QUIET env var aliases for CI/scripts - Template adds RBAC/roles section (migration_role, app_role, environmentify_strategy, check_pending_migrations) - Explains why schema:cache:dump stays rake-only - Notes Thor subcommand test portability across Thor versions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Final spec tweaks: env var scope, 6.1 deliverable list - APARTMENT_FORCE applies to any subcommand with --force, not just create - 6.1 deliverables include migrator.rb, pool_reaper.rb, and their specs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Phase 6 implementation plan: CLI & Generator 20 tasks across two sub-phases (6.1: CLI + rake, 6.2: generator + binstub). TDD throughout. Covers Migrator#migrate_one, PoolReaper#run_cycle, four Thor subcommand classes, rake refactor, and generator rewrite. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix implementation plan from review feedback Critical fixes: - migrate_one: remove nested with_migration_role (migrate_tenant already wraps it; double-nesting would confuse connection handling) - Zeitwerk: ignore both cli.rb and cli/ directory (cli.rb maps to Apartment::Cli without the ignore, not Apartment::CLI) - Generator spec: behaviour -> behavior (US spelling, matches Rails) Other fixes: - Replace climate_control with manual ENV save/restore (no new dep) - Fix example count: 10, not 9 - Rake drop: document behavior change (now routes through Thor) - Add rake apartment:migrate[tenant] parity with create[tenant] - Note that Thor :numeric handles large migration timestamps - Expand rubocop scope to include lib/apartment.rb Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Migrator#migrate_one for single-tenant CLI migration Public API that delegates to the private migrate_tenant (which already wraps with_migration_role, disables advisory locks, sets Current.migrating, and instruments). Calls evict_migration_pools in ensure to match the cleanup behavior of Migrator#run. Used by CLI migrations command. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix indentation in migrate_one spec, tighten comment Align describe '#migrate_one' block contents with the file's 2-space indentation convention. Simplify migrate_one comment to describe the public contract, not internal delegation details. * Add PoolReaper#run_cycle for synchronous eviction Public method that performs one idle + LRU eviction pass and returns the count of evicted pools. The background timer's private #reap now delegates to run_cycle. Used by CLI pool evict command. * Ignore cli.rb and cli/ from Zeitwerk autoloading cli.rb must be ignored because Zeitwerk's default inflector maps it to Apartment::Cli, not Apartment::CLI. The cli/ directory is ignored because Thor subcommands depend on Thor being required first. Both are loaded explicitly via require 'apartment/cli'. * Add Apartment::CLI entry point with subcommand registration Registers Tenants, Migrations, Seeds, and Pool as Thor subcommands. Subcommand classes are stubs; commands added in subsequent commits. * Add CLI tenants subcommand: create, drop, list, current Supports single-tenant and all-tenants create, confirmation-gated drop, list from tenants_provider, and current tenant display. Env var overrides: APARTMENT_FORCE, APARTMENT_QUIET. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add CLI migrations subcommand: migrate, rollback migrate delegates to Migrator#run (all) or Migrator#migrate_one (single). Supports --version, --threads, and ENV VERSION fallback. rollback iterates tenants sequentially with --step option. * Add CLI seeds subcommand Supports single-tenant and all-tenants seeding via Apartment::Tenant.seed. Collects errors across tenants and exits non-zero on failure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add CLI pool subcommand: stats, evict stats shows pool summary with optional --verbose per-tenant breakdown. evict triggers PoolReaper#run_cycle with confirmation gate. Both guard against nil pool_manager/pool_reaper. * Refactor v4.rake to thin wrappers delegating to CLI Logic now lives in Thor CLI classes. Rake tasks are one-liners that invoke the corresponding CLI method. Rake drop passes force: true (non-interactive). Schema cache dump stays as-is. Adds optional tenant arg to create and migrate for single-tenant operations. * Fix rubocop offenses in CLI files Auto-correct string quote style. Extract print_tenant_details from pool stats to resolve Metrics/AbcSize and complexity offenses. * Rewrite install generator for v4: initializer + binstub Initializer template uses v4 Config API with minimal scaffold (only tenant_strategy and tenants_provider uncommented). Adds bin/apartment binstub generation. No v3 references (tenant_names, use_schemas, manual middleware insertion). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Guard generator spec for missing Rails dependency Generator specs require rails/generators which is only available under appraisal. Wrap require in begin/rescue and early-return if Rails::Generators is not defined. Specs run under appraisal (566 examples) and are gracefully skipped under bare rspec (554). * Document rollback semantics and single-tenant migrate scope Add comment explaining why rollback bypasses Migrator (intentionally no RBAC role, advisory locks, or Current.migrating for undo ops). Clarify in long_desc that single-tenant migrate targets only the named tenant, not the primary/default schema. * Rollback respects migration_role for RBAC Rollback is DDL (same as migrate), so it must use the migration role when configured. Adds with_migration_role helper to CLI::Migrations that wraps rollback_single and rollback_all. Refactors rollback_all to reuse rollback_single for DRY and rubocop compliance. Three new tests: single-tenant with role, all-tenants with role, and no-op without role configured. * Reenable schema dump task before invoke Rake::Task#invoke is a no-op if the task already ran in the same process. Adding reenable ensures the dump fires even when chained after rake db:migrate in the same session. * Fix rubocop offenses in generator spec * Extract with_migration_role as public class method on Migrator Single source of truth for RBAC role switching. The private instance method on Migrator delegates to it; CLI::Migrations calls it directly. Eliminates duplication flagged in review. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Add Phase 7 design spec: integration & stress test gaps Identifies fiber safety, memory stability, and CLI integration as genuine gaps; thread safety and pool isolation need hardening. Single-PR approach, flat file structure in spec/integration/v4/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address Cursor review feedback on Phase 7 design spec - Use public `run_cycle` API instead of `send(:reap)` - CLI drop test passes `force: true` to bypass confirmation prompt - Clarify "additive only" for stress_spec hardening - Assert both `Tenant.current` and `Current.tenant` to lock alias - Precise per-engine isolation description for cross-tenant test - Mark COVERAGE=1 as optional manual gate, not CI requirement - Note drive-by fix for coverage_gaps_spec.rb stale reap call Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add load_async fiber test and SQLite memory smoke to Phase 7 scope - Fiber spec gains load_async integration test (conditional on AR support) - Memory stability spec gains lightweight SQLite pool-count smoke test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Phase 7 implementation plan Six tasks: fiber safety spec, memory stability spec, stress hardening, CLI integration spec, drive-by reap->run_cycle fix, and full-suite verification across all engines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add fiber safety integration spec Proves CurrentAttributes isolates tenant state across fibers: basic isolation, yield/resume cycles, nested switch blocks, conditional Fiber.scheduler and load_async tests. Requires isolation_level = :fiber (set in before block, restored in after) since MRI defaults to :thread. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add memory stability integration spec Proves pool count stays bounded under max_total_connections, create/drop cycles don't leak pools, and sustained round-robin switching doesn't create phantom pool entries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Harden stress spec with tenant identity and isolation assertions Add two new examples to the concurrent switching context: - Explicit Tenant.current + Current.tenant check per thread - Cross-tenant connection checkout isolation with dedicated tenants and barrier synchronization Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add CLI integration spec Tests Thor CLI commands (tenants list/create/drop, pool stats) against a real database. Drop verification uses pool_manager.tracked? instead of TenantNotFound (compatible with all engines). ENV['APARTMENT_FORCE'] wrapped in ensure block to prevent leakage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use public run_cycle API instead of send(:reap) Drive-by fix: PoolReaper#reap is private and delegates to run_cycle. Use the public API directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix review feedback in memory stability spec - Inline skip string instead of top-level constant (Lint/ConstantDefinitionInBlock) - Add tmp_dir cleanup in bounded and create/drop after blocks Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Clean up tmp_dir on all engines in fiber safety spec Move FileUtils.rm_rf(tmp_dir) outside the SQLite conditional so temp directories are cleaned up on PG/MySQL runs too. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address PR review findings - CLI drop test: use cleanup_tenants! helper instead of silent rescue - CLI after block: clean up tmp_dir on all engines - Memory stability: assert pool count > 5 before reap to prove reaper actually reduced it (not vacuously <= 5) - Memory stability: clean up tmp_dir unconditionally in sustained switching - Fiber spec: fix inaccurate Fiber::Scheduler comment (MRI doesn't define the constant, not "raises TypeError") - CLI spec: remove no-op `private` keyword at RSpec scope Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix Style/MultilineIfModifier rubocop offense Convert trailing if-modifier on multiline expect to normal if-statement block. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat: add Apartment::Model concern with pin_tenant + registry Introduces model-level pin_tenant declaration for pinning models to the default tenant. Concurrent::Set registry in Apartment module with pinned_model? ancestor walk for STI support. clear_config resets pinned state. activate! sets @activated flag. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: pin_tenant ordering + test assertion specificity Set @apartment_pinned before registry/process calls to close a double-processing window under concurrency. Add .with(LateLoadedModel) to verify process_pinned_model receives the correct class. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: ConnectionHandling skips tenant routing for pinned models Adds explicit registry check via Apartment.pinned_model? after the existing nil/default/pool_manager guards. Uses ancestor walk instead of connection_specification_name to avoid ApplicationRecord false positives. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: replace process_excluded_models with process_pinned_models Iterates Apartment.pinned_models registry instead of config string list. Idempotent via @apartment_connection_established class-level flag. Deprecation alias for process_excluded_models emits warning. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: deprecation shim for config.excluded_models Tenant.init resolves excluded_models strings into pinned model registrations before calling process_pinned_models. Deprecation warning on non-empty excluded_models= setter. Duplicate detection warns if model uses both paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: comprehensive pinned model integration tests Covers all strategies (schema, database-per-tenant), ApplicationRecord topology, STI inheritance, config.excluded_models shim, concurrent access. Removes pending guards for non-PG engines. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: rubocop fixes for Phase 7.1 Inline ThreadSafety/ClassInstanceVariable disables on intentional class ivars in Apartment::Model. Auto-corrected block delimiters and guard clause style in specs and adapter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: Phase 7.1 design spec and implementation plan Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: CI failures — SQLite concurrent skip + PG excluded_models shim Skip concurrent pinned model test on SQLite (BusyException from single-writer lock). Update postgresql_schema_spec to call Tenant.init instead of process_excluded_models (shim resolves config strings into pinned model registry before processing). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: rubocop line length in concurrent test skip guard Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: pinned models use base_config, not resolve_connection_config For database-per-tenant strategies (MySQL, SQLite), resolve_connection_config sets the database key to the default tenant NAME (e.g. 'default'), not the actual default database. Pinned models now use the adapter's raw base_config which points to the real default database. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: Zeitwerk collapse for concerns/ + reset @apartment_connection_established Add loader.collapse for concerns/ directory so Zeitwerk maps lib/apartment/concerns/model.rb to Apartment::Model (not Apartment::Concerns::Model). Mirrors Rails app/models/concerns/. Reset @apartment_connection_established on each pinned model in clear_config so reconfiguration with different database settings re-processes pinned models correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add resolve_excluded_models_shim unit tests + design doc accuracy Add 3 unit tests for the excluded_models shim: string resolution, NameError handling, and duplicate skip (pin_tenant + config overlap). Remove unreachable duplicate-warning branch from shim (pin_tenant always registers before shim runs). Update design doc: mark table validation as deferred, correct shim processing description (Tenant.init not activate!), remove railtie from files changed (no changes needed). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add Phase 8 design spec: Documentation & Upgrade Guide Covers README v4 rewrite, upgrade guide, install template update, dual-release process (RELEASING.md + workflow change), legacy doc migration to docs/history/, and CLAUDE.md updates. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Revise Phase 8 spec per review feedback - Fix excluded_models language: "deprecated in v4, removed in v5" (not "removed") - Drop current_tenant from breaking changes (same API in v3 and v4) - Add block-scoped invariant sentence to Quick Start outline - Sharpen connects_to distinction between Pinned Models and Known Limitations - Add GitHub Actions tag pattern verification note - Fix Step 3 find/replace patterns for accuracy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Phase 8 implementation plan 10 tasks covering: feature branch, legacy doc migration, README v4 rewrite, upgrade guide, install template, RELEASING.md dual-release, gem-publish workflow tag trigger, CLAUDE.md updates, verification, PR. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Revise Phase 8 plan per review feedback - Add README snapshot pre-check (Task 2) - Add PR #327 GitHub fallback URL (Task 4) - Remove YAML from rubocop scope, add generator spec check (Task 9) - Use dynamic spec counts instead of hardcoded 576 - Add gemspec packaging note and changelog strategy * Move legacy docs to docs/history/ Moves legacy_CHANGELOG.md to docs/history/CHANGELOG-v3.md and snapshots the current v3 README as docs/history/README-v3.md. Both get headers pointing readers to the current versions. * Rewrite README for v4 API Restructured around v4 mental model: tenant_strategy, tenants_provider, Apartment::Model + pin_tenant, pool-per-tenant, Railtie auto-wiring. Removed v3-only config (excluded_models, tenant_names, use_schemas). Added Pinned Models and Known Limitations sections. v3 README preserved in docs/history/README-v3.md. * Remove v3 features from README that don't exist in v4 Removes: Rails Console helpers (tenant_list, st(), custom_console), APARTMENT_DISABLE_INIT env var, config.tenant_presence_check. None of these were ported to v4. * Add v4 upgrade guide for external users Self-sufficient guide covering breaking changes, step-by-step migration, connects_to compatibility, and troubleshooting. Ported useful content from PR #327 with corrected requirements and API references. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update install template to recommend pin_tenant Adds commented example showing Apartment::Model + pin_tenant as the recommended v4 approach. Keeps excluded_models as deprecated fallback. * Add dual-release process for v3 maintenance Documents v3-stable branch workflow: cherry-pick fixes, tag from branch, publish via tag trigger. v3-stable never merges into main. * Add v3 tag trigger to gem-publish workflow Allows v3 maintenance releases to be published by pushing a v3.* tag from the v3-stable branch, without merging into main. * Update CLAUDE.md files for Phase 7.1 additions Adds concerns/model.rb docs, updates spec counts (576 unit specs), replaces excluded_models with pin_tenant in Core Concepts, adds connects_to gotcha, notes new spec files. * Address review feedback on README and upgrade guide - Fix tenant_strategy: only :schema and :database_name are implemented; :shard/:database_config noted as reserved/future - Remove "Drop-in replacement, same API" claim; scope to same require - Precise db:migrate wording (Railtie hooks primary task) - Remove bold-first bullet pattern from upgrade guide breaking changes - Add reset (no bang) note to tenant switching migration step - Add :switch callback to README Callbacks section * Adopt Rails-style branch strategy Branch layout: main (development), X-Y-stable (releases), tag-based publishing. Replaces the previous development→main→publish flow. - CI/Rubocop workflows: run on main only (not development) - gem-publish: triggers on v* tags (not branch push) - RELEASING.md: rewritten for stable-branch + tag workflow - README/CLAUDE.md: PR target is now main - Includes migration instructions for contributors After this PR merges to development, the GitHub branch rename (development→main, main→3-4-stable) completes the migration. * Run CI/Rubocop on stable branches, remove dead CONTRIBUTING link - CI and Rubocop workflows: push trigger on main + *-stable branches (Rails pattern; PRs cover feature branches) - Remove link to nonexistent CONTRIBUTING.md from README * Fix review findings: branch name consistency, style, config grouping - Design doc: v3-stable -> 3-4-stable (matches RELEASING.md) - RELEASING.md: unicode arrows -> ASCII arrows per writing rules - gem-publish.yml: update stale rake release comment - README: move environmentify_strategy from RBAC to Tenant Naming section * Document remaining config options, consolidate reset note README: adds seed_data_file, schema_file, check_pending_migrations, tenant_not_found_handler, active_record_log, schema_cache_per_tenant, shard_key_prefix under new Behavior subsection. Adds PostgreSQL enforce_search_path_reset and include_schemas_in_dump. Upgrade guide: removes duplicate reset!/reset note from Step 6 (already covered in Step 3). gem-publish.yml: clarifies contents:write rationale. * Remove dead config options from README tenant_not_found_handler, active_record_log, and enforce_search_path_reset are declared in config.rb but never read by v4 code. Documenting them misleads users. Keeps only options that are actually wired up. * Wire up active_record_log via ActiveSupport::TaggedLogging Tenant.switch wraps the block in Rails.logger.tagged(tenant) when config.active_record_log is true and the logger supports tagged logging. Log lines inside a switch block are prefixed with [tenant]. Handles nested switches, plain loggers, and nil config gracefully. 5 new unit specs covering tagged output, tag removal after block, nested switches, disabled config, and plain logger fallback. * Use tenant=name tag format for active_record_log Matches activerecord-tenanted convention. Key=value format avoids ambiguity when multiple tags are present (request_id, job_id, etc.). Log lines now show [tenant=acme] instead of [acme]. README: notes no-op behavior with structured loggers (Ougai). * Simplify active_record_log doc: drop Ougai-specific note * Add sql_query_tags config for ActiveRecord::QueryLogs When enabled, registers a :tenant tag with ActiveRecord::QueryLogs so every SQL query includes a /* tenant='name' */ comment. Visible in slow query logs, pg_stat_activity, and database monitoring tools. Uses the same pattern as Basecamp's activerecord-tenanted: register a zero-arity lambda in taggings that reads Apartment::Current.tenant, then append :tenant to the active tags list. Wired in the Railtie after activate!. No-op when QueryLogs is unavailable. 4 unit specs: tagging registration, tag activation, idempotency, disabled config. * Drop redundant Rails 7.1+ note from sql_query_tags doc * Address final review findings - Spec counts: 576 -> 585 in CLAUDE.md and spec/CLAUDE.md - CLAUDE.md: note branch rename is pending (still development) - CI/Rubocop: include development in push trigger until rename - README: document nested tag stacking for active_record_log - Upgrade guide: "activation time" -> "initialization time" --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
20 participants
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.
No description provided.