refactor: consolidate gateway secrets into agentflare.db (closes #138) - #146
Merged
Conversation
📝 WalkthroughWalkthroughGateway secrets are migrated into the shared ChangesShared gateway secrets database
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SecretClients
participant crate_db_open
participant LegacyGatewayDB
participant GatewaySecretsTable
SecretClients->>crate_db_open: Open agentflare.db
crate_db_open->>GatewaySecretsTable: Ensure gateway_secrets schema
crate_db_open->>LegacyGatewayDB: Read legacy secrets
LegacyGatewayDB-->>crate_db_open: Return compatible rows
crate_db_open->>GatewaySecretsTable: Insert rows with INSERT OR IGNORE
crate_db_open->>LegacyGatewayDB: Rename gateway.db to gateway.db.migrated
crate_db_open-->>SecretClients: Return shared connection
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/db.rs`:
- Around line 34-36: Update migrate_old_gateway_db and the db::open()
initialization flow to persist a migration-complete marker in agentflare.db only
after the legacy secrets copy transaction succeeds, and skip importing when that
marker already exists. Add a regression test that migrates a secret, removes it,
reopens the database, and verifies the deleted secret is not restored.
- Around line 65-70: Update the legacy gateway-secrets migration flow around the
statement preparation and `rows` iteration so schema, query, and row-decoding
failures are treated as best-effort failures rather than propagated from
`db::open()`. Skip incompatible or malformed legacy entries/database reads while
allowing shared database initialization and unrelated claims access to continue,
and add coverage for an old schema missing the expected columns.
🪄 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 Plus
Run ID: 4cea5b36-fa4e-48aa-a996-1256085a9148
📒 Files selected for processing (4)
src/cli/gateway.rssrc/db.rssrc/gateway_secrets.rssrc/mcp_server.rs
Comment on lines
+34
to
+36
| // One-time migration: copy secrets from old gateway.db | ||
| // (pre-#138 separate file) into agentflare.db. | ||
| migrate_old_gateway_db(&conn)?; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Persist migration completion so deleted secrets stay deleted.
Because the legacy file remains, every db::open() imports it again. After removing a migrated secret, the next CLI or MCP open restores it through INSERT OR IGNORE.
Record a migration-complete marker in agentflare.db after a successful transactional copy, then skip future imports. Add a remove-then-reopen regression test.
Also applies to: 43-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/db.rs` around lines 34 - 36, Update migrate_old_gateway_db and the
db::open() initialization flow to persist a migration-complete marker in
agentflare.db only after the legacy secrets copy transaction succeeds, and skip
importing when that marker already exists. Add a regression test that migrates a
secret, removes it, reopens the database, and verifies the deleted secret is not
restored.
- rename gateway.db -> gateway.db.migrated after a successful import so a deleted secret can't resurrect via INSERT OR IGNORE on the next open() - make legacy reads best-effort: an incompatible/malformed old schema is skipped instead of propagating and bricking every db::open() (which would also disable unrelated claims access) - regression tests: deleted-stays-deleted, incompatible-schema-non-fatal Addresses CodeRabbit review on #146.
Owner
Author
|
Addressed both findings in e95e41c:
|
…date-secrets # Conflicts: # src/db.rs
getappz
enabled auto-merge
July 11, 2026 13:35
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/db.rs`:
- Around line 45-56: The legacy database migration must persist completion in
agentflare.db even when renaming gateway.db fails. Update the migration logic
around the legacy import and the related open flow to check a durable migration
marker before reading gateway.db, record that marker in the same transaction as
successful secret imports, and treat the rename as best-effort cleanup only. Add
coverage for forced rename failure followed by secret deletion and a retry,
ensuring the deleted secret is not re-imported.
🪄 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 Plus
Run ID: 5a01ec97-fc1d-40b8-bcae-e8be0a681e79
📒 Files selected for processing (2)
src/db.rssrc/mcp_server.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/mcp_server.rs
Comment on lines
+45
to
+56
| /// Copy secrets from a legacy `gateway.db` (pre-#138 separate file) into the | ||
| /// shared db, then rename the legacy file so it's imported exactly once. | ||
| /// | ||
| /// Reads from the legacy db are best-effort: a missing file, an unopenable | ||
| /// db, or an incompatible/malformed `gateway_secrets` schema all skip the | ||
| /// import rather than failing `open()` — otherwise one bad legacy file would | ||
| /// brick unrelated claims access too. Only writes into our own (healthy) | ||
| /// shared db are fatal. | ||
| /// | ||
| /// Renaming to `gateway.db.migrated` on success is the migration-complete | ||
| /// marker: without it every `open()` re-imports via `INSERT OR IGNORE`, so a | ||
| /// secret the user deleted would resurrect on the next run. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Persist completion even when the legacy-file rename fails.
Ignoring rename errors leaves gateway.db active. After a user deletes an imported secret, a later open() resurrects it, so the fallback is not idempotent.
Commit a migration marker in agentflare.db with the imports, consult it before reading the legacy file, and treat the rename only as cleanup. Add coverage that forces rename failure, deletes the secret, and retries migration.
Also applies to: 80-83, 136-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/db.rs` around lines 45 - 56, The legacy database migration must persist
completion in agentflare.db even when renaming gateway.db fails. Update the
migration logic around the legacy import and the related open flow to check a
durable migration marker before reading gateway.db, record that marker in the
same transaction as successful secret imports, and treat the rename as
best-effort cleanup only. Add coverage for forced rename failure followed by
secret deletion and a retry, ensuring the deleted secret is not re-imported.
This was referenced Jul 29, 2026
Merged
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
1 participant
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.
Summary
gateway_secretstable out of standalone~/.agentflare/gateway.dbinto shared~/.agentflare/agentflare.dbgateway_secrets::migrate(conn)called fromdb::open()(next toclaims::migrate)cli/gateway.rsandmcp_server.rs::resolve_gateway_secretsto usecrate::db::open()gateway.dbon first open ofagentflare.db(leaves old file intact)mem_migrated()helper instead of raw SQL table creationCloses #138
Summary by CodeRabbit
New Features
Bug Fixes