feat(backend): migrate agentflare-backend onto rusqlite_migration - #166
Merged
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe backend replaces custom SQLite migration handling with ChangesSQLite migration adoption
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 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
🧹 Nitpick comments (2)
crates/agentflare-backend/src/migrations/0001_initial.sql (2)
103-107: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo guard against
item_id == depends_on_item_id.Nothing prevents an item from depending on itself, which could create a degenerate/self-referential entry that downstream dependency-graph traversal must special-case.
🔧 Proposed fix
CREATE TABLE IF NOT EXISTS item_dependencies ( item_id TEXT NOT NULL REFERENCES items(id), depends_on_item_id TEXT NOT NULL REFERENCES items(id), + CHECK (item_id != depends_on_item_id), PRIMARY KEY (item_id, depends_on_item_id) );🤖 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 `@crates/agentflare-backend/src/migrations/0001_initial.sql` around lines 103 - 107, Add a table-level CHECK constraint to item_dependencies that rejects rows where item_id equals depends_on_item_id, while preserving the existing foreign keys and composite primary key.
140-156: 🗄️ Data Integrity & Integration | 🔵 TrivialUnbounded retention of raw webhook payloads.
request_headers/request_body/response_headers/response_bodyare stored indefinitely with nodeleted_at/TTL column, and headers/bodies may carry secrets or PII (auth tokens, user data) forwarded through webhooks. Worth considering redaction before storage or a retention/purge policy for this table.🤖 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 `@crates/agentflare-backend/src/migrations/0001_initial.sql` around lines 140 - 156, Update the webhook_logs schema to support bounded retention of raw webhook data by adding a deleted_at or equivalent expiration column, and ensure the migration includes the index needed for purge queries. Preserve the existing payload fields and webhook log relationships while enabling scheduled deletion of expired records.
🤖 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 `@crates/agentflare-backend/src/migrations/0001_initial.sql`:
- Around line 74-90: Update the labels table definition so labels.project_id
declares a foreign-key reference to projects(id), while preserving its nullable
behavior for workspace-only labels and leaving the existing indexes unchanged.
- Around line 140-156: Update the webhook_logs table definition to make
webhook_id a foreign key referencing webhooks(id), while preserving its existing
nullability and indexing. Add the constraint alongside the existing workspace_id
reference in CREATE TABLE webhook_logs.
---
Nitpick comments:
In `@crates/agentflare-backend/src/migrations/0001_initial.sql`:
- Around line 103-107: Add a table-level CHECK constraint to item_dependencies
that rejects rows where item_id equals depends_on_item_id, while preserving the
existing foreign keys and composite primary key.
- Around line 140-156: Update the webhook_logs schema to support bounded
retention of raw webhook data by adding a deleted_at or equivalent expiration
column, and ensure the migration includes the index needed for purge queries.
Preserve the existing payload fields and webhook log relationships while
enabling scheduled deletion of expired records.
🪄 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: 174a1b29-f15f-43b2-92d1-7b9dcc8d7b10
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
crates/agentflare-backend/Cargo.tomlcrates/agentflare-backend/src/db.rscrates/agentflare-backend/src/migrations/0001_initial.sql
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/agentflare-backend/src/migrations/0001_initial.sql (2)
103-107: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo guard against
item_id == depends_on_item_id.Nothing prevents an item from depending on itself, which could create a degenerate/self-referential entry that downstream dependency-graph traversal must special-case.
🔧 Proposed fix
CREATE TABLE IF NOT EXISTS item_dependencies ( item_id TEXT NOT NULL REFERENCES items(id), depends_on_item_id TEXT NOT NULL REFERENCES items(id), + CHECK (item_id != depends_on_item_id), PRIMARY KEY (item_id, depends_on_item_id) );🤖 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 `@crates/agentflare-backend/src/migrations/0001_initial.sql` around lines 103 - 107, Add a table-level CHECK constraint to item_dependencies that rejects rows where item_id equals depends_on_item_id, while preserving the existing foreign keys and composite primary key.
140-156: 🗄️ Data Integrity & Integration | 🔵 TrivialUnbounded retention of raw webhook payloads.
request_headers/request_body/response_headers/response_bodyare stored indefinitely with nodeleted_at/TTL column, and headers/bodies may carry secrets or PII (auth tokens, user data) forwarded through webhooks. Worth considering redaction before storage or a retention/purge policy for this table.🤖 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 `@crates/agentflare-backend/src/migrations/0001_initial.sql` around lines 140 - 156, Update the webhook_logs schema to support bounded retention of raw webhook data by adding a deleted_at or equivalent expiration column, and ensure the migration includes the index needed for purge queries. Preserve the existing payload fields and webhook log relationships while enabling scheduled deletion of expired records.
🤖 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 `@crates/agentflare-backend/src/migrations/0001_initial.sql`:
- Around line 74-90: Update the labels table definition so labels.project_id
declares a foreign-key reference to projects(id), while preserving its nullable
behavior for workspace-only labels and leaving the existing indexes unchanged.
- Around line 140-156: Update the webhook_logs table definition to make
webhook_id a foreign key referencing webhooks(id), while preserving its existing
nullability and indexing. Add the constraint alongside the existing workspace_id
reference in CREATE TABLE webhook_logs.
---
Nitpick comments:
In `@crates/agentflare-backend/src/migrations/0001_initial.sql`:
- Around line 103-107: Add a table-level CHECK constraint to item_dependencies
that rejects rows where item_id equals depends_on_item_id, while preserving the
existing foreign keys and composite primary key.
- Around line 140-156: Update the webhook_logs schema to support bounded
retention of raw webhook data by adding a deleted_at or equivalent expiration
column, and ensure the migration includes the index needed for purge queries.
Preserve the existing payload fields and webhook log relationships while
enabling scheduled deletion of expired records.
🪄 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: 174a1b29-f15f-43b2-92d1-7b9dcc8d7b10
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
crates/agentflare-backend/Cargo.tomlcrates/agentflare-backend/src/db.rscrates/agentflare-backend/src/migrations/0001_initial.sql
🛑 Comments failed to post (2)
crates/agentflare-backend/src/migrations/0001_initial.sql (2)
74-90: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
labels.project_idlacks a foreign key.Every other tenant-scoped column in this table (
workspace_id) and table (parent_id) declaresREFERENCES, butproject_iddoesn't referenceprojects(id). Sincedb.rsnow turns onPRAGMA foreign_keys = ON, this column silently escapes referential-integrity enforcement, allowing labels to point at nonexistent/deleted projects.🔧 Proposed fix
CREATE TABLE IF NOT EXISTS labels ( id TEXT PRIMARY KEY, - project_id TEXT, + project_id TEXT REFERENCES projects(id), workspace_id TEXT NOT NULL REFERENCES workspaces(id),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.CREATE TABLE IF NOT EXISTS labels ( id TEXT PRIMARY KEY, project_id TEXT REFERENCES projects(id), workspace_id TEXT NOT NULL REFERENCES workspaces(id), name TEXT NOT NULL, color TEXT NOT NULL DEFAULT '`#60646C`', parent_id TEXT REFERENCES labels(id), sort_order REAL NOT NULL DEFAULT 65535, external_source TEXT, external_id TEXT, created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL, deleted_at INTEGER ); CREATE UNIQUE INDEX IF NOT EXISTS idx_labels_name_project ON labels(name, project_id) WHERE deleted_at IS NULL AND project_id IS NOT NULL; CREATE UNIQUE INDEX IF NOT EXISTS idx_labels_name_workspace_only ON labels(name, workspace_id) WHERE deleted_at IS NULL AND project_id IS NULL;🤖 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 `@crates/agentflare-backend/src/migrations/0001_initial.sql` around lines 74 - 90, Update the labels table definition so labels.project_id declares a foreign-key reference to projects(id), while preserving its nullable behavior for workspace-only labels and leaving the existing indexes unchanged.
140-156: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
webhook_logs.webhook_idlacks a foreign key.
workspace_idon this same table referencesworkspaces(id), butwebhook_iddoesn't referencewebhooks(id), so log rows can outlive/point past a deleted webhook without any DB-level guarantee.🔧 Proposed fix
workspace_id TEXT NOT NULL REFERENCES workspaces(id), - webhook_id TEXT NOT NULL, + webhook_id TEXT NOT NULL REFERENCES webhooks(id),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.CREATE TABLE IF NOT EXISTS webhook_logs ( id TEXT PRIMARY KEY, workspace_id TEXT NOT NULL REFERENCES workspaces(id), webhook_id TEXT NOT NULL REFERENCES webhooks(id), event_type TEXT, request_method TEXT, request_headers TEXT, request_body TEXT, response_status TEXT, response_headers TEXT, response_body TEXT, retry_count INTEGER NOT NULL DEFAULT 0, created_at INTEGER NOT NULL ); CREATE INDEX IF NOT EXISTS idx_webhook_logs_webhook ON webhook_logs(webhook_id); CREATE INDEX IF NOT EXISTS idx_webhook_logs_workspace ON webhook_logs(workspace_id);🤖 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 `@crates/agentflare-backend/src/migrations/0001_initial.sql` around lines 140 - 156, Update the webhook_logs table definition to make webhook_id a foreign key referencing webhooks(id), while preserving its existing nullability and indexing. Add the constraint alongside the existing workspace_id reference in CREATE TABLE webhook_logs.
This was referenced Jul 19, 2026
getappz
pushed a commit
that referenced
this pull request
Aug 25, 2026
Discovery tick dispatches purely on the ready-for-work label, so items #184/#185/#186/#187 (go/no-go candidates from #166's spec) whose own description says "Decision pending — not dispatched" got auto-dispatched and re-dispatched across multiple agents anyway -- the prose was never actually enforced. Add a needs-decision label that blocks run_discovery_tick even while ready-for-work is also present. Stripping ready-for-work alone wouldn't have been durable: redispatch unconditionally re-attaches it, so the new label has to keep gating on its own until a human clears it. Agentflare-Agent: claude-code Agentflare-Branch: fix/dispatch-failure-ceiling-any-reason
4 tasks
getappz
added a commit
that referenced
this pull request
Aug 26, 2026
Adds semantic (embedding-based) recall to skill search, reusing agentflare-store's existing embedding infrastructure as a library rather than building anything new -- per item #166's design-spec (candidate d). - crates/skill-registry: new skills_vec table (0003_vec.sql) mirroring observations_vec in src/memory/schema.rs. New embed_store module (upsert/delete/missing/candidates/backfill) built on agentflare_store::embed's cosine-similarity/blob-codec primitives -- these are always-compiled pure math in agentflare-store, not gated behind its `embeddings` feature, so skill-registry takes an unconditional dependency on them. - search::search_semantic blends BM25 hits with vector candidates via agentflare_store::retrieval::merge_ranked, backfilling any vector-only match's full SkillHit row. Degrades byte-identically to plain search() when the caller's embed_query returns None (feature off, no model, or a failed call) -- same graceful-degradation contract src/memory uses. - Registry gets search_semantic/backfill_embeddings methods. Like skill_detect::find_skills already does, the embedding functions are caller-supplied closures (crate::memory::engine::embed_query/embed_doc) rather than a dependency skill-registry can't have on the binary crate. - Wired into the `skill` MCP tool's search action: a small bounded backfill (25/call) runs before each search so the semantic index catches up lazily on first use, no separate background job needed. Verified: cargo build/clippy/fmt clean against CI's exact invocations; 63 skill-registry tests (9 new) + 246 mcp_server tests + 39 skill-related binary tests all pass. Agentflare-Branch: task/187-semantic-embedding-search-spike-go-no-go Agentflare-Item: 187-semantic-embedding-search-spike-go-no-go Co-authored-by: shiva <shiva@gosysinfo.tech>
getappz
pushed a commit
that referenced
this pull request
Aug 26, 2026
Reimplemented from scratch against current master -- the prior branch had overwritten sources.rs's SkillEntry scanning adapter (load.rs's default_sources/scan_sources/validate_entry dependency) with an unrelated SkillSpec/TOML module and committed a pile of debug scratch scripts, and was badly stale besides. See PR #614's close comment. Per item #185 (candidate b of item #166's design-spec): a `category` frontmatter field, defaulting to the first tag when unset, plus one read-only skill_categories MCP tool. - crates/skill-registry: category column (0004_category.sql), threaded through Frontmatter/SkillEntry/BundleEntry (so it round-trips through hub export/import, not just local scans) and db::rebuild's insert. New search::list_categories/skills_in_category read helpers and matching Registry methods. - skill_categories MCP tool: omit `category` for every category with its skill count (most populated first); pass one to list its skills. - Three pre-existing SkillEntry literals in src/cli/skill.rs (export, hub push, DB-only-source carry-forward) updated for the new field. Verified: cargo build/clippy/fmt clean against CI's exact invocations; 70 skill-registry tests (7 new) + 246 mcp_server tests + 12 cli::skill tests all pass. Agentflare-Branch: task/185-category-taxonomy-skill-categories-tool Agentflare-Item: 185-category-taxonomy-skill-categories-tool
4 tasks
getappz
added a commit
that referenced
this pull request
Aug 26, 2026
Reimplemented from scratch against current master -- the prior branch had overwritten sources.rs's SkillEntry scanning adapter (load.rs's default_sources/scan_sources/validate_entry dependency) with an unrelated SkillSpec/TOML module and committed a pile of debug scratch scripts, and was badly stale besides. See PR #614's close comment. Per item #185 (candidate b of item #166's design-spec): a `category` frontmatter field, defaulting to the first tag when unset, plus one read-only skill_categories MCP tool. - crates/skill-registry: category column (0004_category.sql), threaded through Frontmatter/SkillEntry/BundleEntry (so it round-trips through hub export/import, not just local scans) and db::rebuild's insert. New search::list_categories/skills_in_category read helpers and matching Registry methods. - skill_categories MCP tool: omit `category` for every category with its skill count (most populated first); pass one to list its skills. - Three pre-existing SkillEntry literals in src/cli/skill.rs (export, hub push, DB-only-source carry-forward) updated for the new field. Verified: cargo build/clippy/fmt clean against CI's exact invocations; 70 skill-registry tests (7 new) + 246 mcp_server tests + 12 cli::skill tests all pass. Agentflare-Branch: task/185-category-taxonomy-skill-categories-tool Agentflare-Item: 185-category-taxonomy-skill-categories-tool Co-authored-by: shiva <shiva@gosysinfo.tech>
4 tasks
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.
What
Switches
agentflare-backend'sdb.rsfrom ad-hocConnection::open+execute_batch(schema.sql)ontoagentflare-db-kit'srusqlite_migrationwiring (open_file/open_memory), as flagged as a follow-up during #165's review.schema.sqlrenamed tomigrations/0001_initial.sql(content unchanged) — future schema changes become new numbered files here, not edits to this one.db.rsbuilds aconst MIGRATIONS: Migrationsfrom that file and callsdb_kit::open_file/open_memoryinstead of rawexecute_batch.open_db/open_in_memorynow returnResult<_, db_kit::open::Error>instead ofrusqlite::Result— all 3 call sites insrc/mcp_server.rsonly use.to_string()/.unwrap(), so this is a no-op for them.Why
SQLite has no
ALTER COLUMN IF NOT EXISTS, soCREATE TABLE IF NOT EXISTSre-runs stop covering schema changes the moment anything needs to modify/drop a column.agentflare-db-kitalready hadrusqlite_migrationsupport built for this;agentflare-backendjust hadn't adopted it yet.The pre-existing-DB question
Existing installed
backend.dbfiles (from #162/#165) were created via the oldexecute_batchpath and have no migration bookkeeping.rusqlite_migrationtracks applied version via SQLite's ownPRAGMA user_version(confirmed by reading the vendored crate source — no separate bookkeeping table). Sinceuser_versiondefaults to 0 and was never touched by the old path, and0001_initial.sql'sCREATE TABLE/INDEX IF NOT EXISTSstatements are already idempotent, running migration 0001 against an already-populated DB is a harmless no-op that just stamps it touser_version = 1. No baseline/backfill step needed.Added a test (
migrate_is_safe_against_a_pre_migration_db) that reproduces this exact scenario: seeds a DB via the oldexecute_batchpath with data, then opens it through the new migration-awareopen_db, and asserts the data survives anduser_versionends up at 1.Testing
cargo test -p agentflare-backend— 57/57 passcargo build --workspacecargo clippy --locked --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedantic(CI's exact flags)cargo fmt --checkcargo deny checkSummary by CodeRabbit
New Features
Bug Fixes