refactor: Move demo user seeding to post-provisioning hook#2137
refactor: Move demo user seeding to post-provisioning hook#2137
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDemo user seeding was relocated from server startup in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Claude Code ReviewCommit: SummaryClean, well-scoped fix for a timing race where The implementation is minimal and correct: remove the startup call from Risk Assessment
FindingsNo actionable findings. Code is clean. Bot Review NotesAll 4 review threads (2 CodeRabbit, 1 claude[bot], 1 CodeRabbit on DB close) are resolved. The DB connection leak flagged by both claude[bot] and CodeRabbit was fixed in commit Previously Flagged
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/meridian/wire_services.go`:
- Around line 257-260: The demo-user seeding hook is only registered inside
startProvisioningWorker and is skipped when prov == nil (i.e.,
SCHEMA_PROVISIONING_ENABLED=false), so DEMO_OPERATOR_* env vars are ignored; fix
by adding a fallback when prov == nil that executes the demo-seeding hook at
startup (or registers/executes identitybootstrap.AsDemoUserHook(identityRepo)
immediately) instead of returning early—modify startProvisioningWorker to detect
prov == nil and call/run the AsDemoUserHook (or equivalent demo-seed invocation)
against identityRepo so demo users are created even with provisioning disabled,
while keeping the existing w.RegisterPostProvisioningHook("demo-operator",
identitybootstrap.AsDemoUserHook(identityRepo)) path for the normal provisioned
flow.
In `@services/identity/bootstrap/bootstrap.go`:
- Around line 433-448: AsDemoUserHook currently calls seedDemoUser(repo, ...)
without verifying repo; add the same nil-repo guard used elsewhere: at the start
of AsDemoUserHook (or inside the returned closure) check if repo == nil and
return tenant.ErrNilRepository (or ErrNilRepository the project uses) to avoid
panics when miswired or in tests; reference the AsDemoUserHook function and
seedDemoUser so the guard mirrors other bootstrap entry points' behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f8a3d790-d2c2-409a-8590-734a370f1f00
📒 Files selected for processing (2)
cmd/meridian/wire_services.goservices/identity/bootstrap/bootstrap.go
SeedDemoUsers ran at server startup (wire_services.go:102) before tenant schemas were provisioned. On deploys that drop and recreate schemas, the provisioning worker takes 30-60s after boot, so the startup seeder consistently failed with "relation identity does not exist" and the demo operator user was never created. Move demo user seeding to seed-dev, which already runs after provisioning completes (it polls GetTenantProvisioningStatus until ACTIVE). seed-dev is the correct home because: - It only runs in deploy workflows, never in production - It already handles all demo/dev data seeding (fixtures) - It runs inside the container (access to DATABASE_URL and DEMO_OPERATOR_* env vars) - It executes after provisioning, eliminating the timing race The seeder uses a direct DB connection (not gRPC) because the identity service's SetPassword RPC requires an invitation token by design - there is no admin "create user with password" API. This is consistent with how SeedDemoUsers worked before; only the call site moved from server boot to seed-dev. No-op when DEMO_OPERATOR_EMAIL or DEMO_OPERATOR_PASSWORD are unset, making it safe for any environment including production.
bjcoombs
force-pushed
the
fix-demo-user-provisioning-hook
branch
from
817ec45 to
4009a7a
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/seed-dev/cmd/root.go`:
- Around line 408-422: The DB connection returned by bootstrap.NewDatabase
(variable db) is never closed; after successfully creating db call db.DB() to
obtain the underlying *sql.DB (handle the error) and close it when done (e.g.,
defer sqlDB.Close() immediately after obtaining it or close it just before
returning), ensuring the connection pool is properly released around the calls
to identitypersistence.NewRepository and identitybootstrap.SeedDemoUsers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f1031598-4ea6-4af2-a9a4-a323e5b3f33d
📒 Files selected for processing (3)
.github/workflows/deploy-demo.ymlcmd/meridian/wire_services.gocmd/seed-dev/cmd/root.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/meridian/wire_services.go
Address review feedback from claude[bot] and CodeRabbit: the bootstrap.NewDatabase connection pool was never closed. Add defer sqlDB.Close() after obtaining the underlying sql.DB.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Summary
Move demo user seeding from a startup call (which races with schema provisioning) to a post-provisioning hook that runs after tenant schemas are fully migrated.
Problem
`SeedDemoUsers` ran at app startup before the provisioning worker finished creating tenant schemas. On deploys that drop and recreate schemas (every deploy since PR #2133), provisioning takes 30-60s. The startup seeder fired immediately, found no identity table, logged a warning, and gave up. The `operator@volterra.energy` demo user was never created, breaking Dex login on demo.
Fix
Register `AsDemoUserHook` as hook #7 in the post-provisioning chain. The hook:
`SeedDemoUsers` remains exported for tests and manual use but is no longer called from the startup path.
Changes
services/identity/bootstrap/bootstrap.go
cmd/meridian/wire_services.go
Test plan