Skip to content

chore(db): pre-land Feb/Mar agent migrations#161

Merged
AnthonyRonning merged 2 commits into
masterfrom
chore/agent-schema-sync-feb-mar
Apr 8, 2026
Merged

chore(db): pre-land Feb/Mar agent migrations#161
AnthonyRonning merged 2 commits into
masterfrom
chore/agent-schema-sync-feb-mar

Conversation

@AnthonyRonning

@AnthonyRonning AnthonyRonning commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • pre-land the Feb/Mar agent migrations on master without enabling agent behavior
  • copy the reviewed migration files over identically from agents
  • intentionally exclude the April conversation projects and pins migration

Validation

  • cargo fmt --all -- --check
  • CARGO_INCREMENTAL=0 cargo clippy --all-targets --all-features -- -D warnings
  • cargo test --all-features

Summary by CodeRabbit

  • New Features
    • Push notification system for device registration and delivery workflows
    • User preferences management for per-user settings
    • Message reactions for users and assistants
    • Agent scheduling for automated task execution
    • Agent storage components (agents, memory blocks, conversation summaries)
    • Encrypted message attachment text storage
    • Per-user encrypted embedding storage for retrieval features

@coderabbitai

coderabbitai Bot commented Apr 7, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 544ce0b3-b9b3-4c52-b4d4-15149cb79210

📥 Commits

Reviewing files that changed from the base of the PR and between 2b9f6a5 and 0e434b6.

⛔ Files ignored due to path filters (2)
  • pcrDev.json is excluded by !pcrDev.json
  • pcrProd.json is excluded by !pcrProd.json
📒 Files selected for processing (2)
  • pcrDevHistory.json
  • pcrProdHistory.json
✅ Files skipped from review due to trivial changes (2)
  • pcrProdHistory.json
  • pcrDevHistory.json

Walkthrough

Adds multiple SQL migrations creating/dropping new tables, indexes, constraints, and triggers (user_embeddings, agent storage, push notifications, user_preferences, agent scheduling, message reactions, attachment text) and updates Diesel/Rust schema and response models to include the new/changed columns and tables.

Changes

Cohort / File(s) Summary
User Embeddings
migrations/2026-02-10-020612_user_embeddings/up.sql, migrations/2026-02-10-020612_user_embeddings/down.sql
Adds user_embeddings table with encrypted vectors/tags/content, source-type CHECKs, partial unique constraints for message-level deduplication, multiple indexes (including partial GIN on tags_enc), and updated_at trigger; down.sql conditionally drops those objects.
Maple Agent MVP Storage
migrations/2026-02-10-214235_maple_agent_mvp_storage/up.sql, migrations/2026-02-10-214235_maple_agent_mvp_storage/down.sql
Creates memory_blocks, conversation_summaries, and agents tables with FK relations, constraints (including partial unique for main agent), indexes, and updated_at triggers; down.sql conditionally drops objects.
Push Notifications
migrations/2026-03-07-120000_push_notifications_v1/up.sql, migrations/2026-03-07-120000_push_notifications_v1/down.sql
Adds push_devices, notification_events, notification_deliveries with constrained enums/checks, encrypted token/payload fields, lifecycle/scheduling/delivery fields, indexes for due queries, and updated_at triggers; down.sql cleans up conditionally.
User Preferences
migrations/2026-03-22-033535_add_user_preferences/up.sql, migrations/2026-03-22-033535_add_user_preferences/down.sql
Adds user_preferences table (UNIQUE(user_id,key)), value_enc BYTEA, index on user_id, and updated_at trigger; down.sql drops them conditionally.
Agent Scheduling
migrations/2026-03-23-180000_agent_schedules_v1/up.sql, migrations/2026-03-23-180000_agent_schedules_v1/down.sql
Creates agent_schedules and agent_schedule_runs with JSON schedule specs, timezone/recurrence CHECKs, run uniqueness, status/lease fields, indexes for due runs and schedule queries, and updated_at triggers; down.sql drops tables if present.
Message Attachments & Reactions (DB)
migrations/2026-02-24-150000_user_messages_attachment_text/up.sql, migrations/2026-02-24-150000_user_messages_attachment_text/down.sql, migrations/2026-03-24-220000_agent_message_reactions/up.sql, migrations/2026-03-24-220000_agent_message_reactions/down.sql
Adds attachment_text_enc BYTEA to user_messages, adds assistant_reaction to user_messages and user_reaction to assistant_messages; down.sql removes the columns conditionally.
Rust / Diesel Schema & Models
src/models/schema.rs, src/models/responses.rs
Adds Diesel table declarations and joinable! entries for all new tables, extends user_messages/assistant_messages schema to include new columns, and updates Rust models: UserMessage gains attachment_text_enc and assistant_reaction; AssistantMessage gains user_reaction.
PCR History Files
pcrDevHistory.json, pcrProdHistory.json
Appends a new trailing record to each PCR history JSON file (timestamped PCR entries).

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Responses clean #101 — overlaps message model and schema changes for attachment_text_enc and reaction fields (direct code-level overlap).
  • Responses refactored #100 — overlaps user_embeddings migration and related schema entries.
  • Responses #57 — touches message models and Diesel schema related to reactions/attachment additions.

Poem

🐰
New rows hop in, encrypted and bright,
Memories burrow, schedules take flight,
Devices and events chitter and sing,
Embeddings and reactions — a database spring,
I nibble a carrot and watch migrations light.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore(db): pre-land Feb/Mar agent migrations' directly and specifically describes the main change: landing database migrations for agent-related schema changes from February/March onto the master branch.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/agent-schema-sync-feb-mar

Comment @coderabbitai help to get the list of available commands and usage tips.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@migrations/2026-02-10-020612_user_embeddings/up.sql`:
- Around line 15-17: The FK checks currently ensure presence/absence of
user_message_id/assistant_message_id but do not enforce that the referenced
user_messages/assistant_messages rows belong to the same user_id and
conversation as the embedding row; update the schema to add ownership
consistency checks for user_message_id, assistant_message_id and conversation_id
by enforcing that user_messages.user_id = <embeddings>.user_id and
assistant_messages.user_id = <embeddings>.user_id and that the referenced
messages’ conversation_id = <embeddings>.conversation_id. Implement this either
by adding composite foreign-key constraints (if you can alter
user_messages/assistant_messages to expose the composite key) or by adding a
BEFORE INSERT/UPDATE trigger on the embeddings table that queries user_messages
and assistant_messages and raises an exception when the user_id or
conversation_id do not match; reference the columns user_message_id,
assistant_message_id, conversation_id, user_id, user_messages and
assistant_messages when locating where to add the constraint/trigger in the
migration.

In `@migrations/2026-02-10-214235_maple_agent_mvp_storage/up.sql`:
- Around line 60-73: The agents table allows parent_agent_id to point to agents
in other tenants/conversations; add a scoped validation so a subagent's parent
has the same user_id and conversation_id. Implement this by adding a
trigger-based check: create a function (e.g. validate_agent_parent_scope) that
on INSERT/UPDATE, when parent_agent_id IS NOT NULL selects the parent row from
agents and ensures parent.user_id = NEW.user_id AND parent.conversation_id =
NEW.conversation_id, and raise an exception if not; attach this function as a
BEFORE INSERT OR UPDATE trigger on agents (or replace/augment CONSTRAINT
agents_parent_check) so cross-tenant/conversation parent links are rejected.
- Around line 58-75: The UNIQUE(conversation_id) constraint on the agents table
prevents multiple agents (main + subagents) per conversation; remove that
UNIQUE(...) and instead enforce uniqueness only for main agents by creating a
partial unique index or constraint such as a UNIQUE index on (conversation_id)
WHERE kind = 'main' (keep the existing CONSTRAINT agents_kind_check and
CONSTRAINT agents_parent_check unchanged to preserve kind/parent rules). Locate
the agents table definition (where CONSTRAINT agents_kind_check, CONSTRAINT
agents_parent_check are declared), remove the final UNIQUE(conversation_id)
line, and add a partial unique index for main agents (e.g., CREATE UNIQUE INDEX
... ON agents (conversation_id) WHERE kind = 'main') so subagents can coexist in
the same conversation.

In `@migrations/2026-03-07-120000_push_notifications_v1/up.sql`:
- Line 4: The deliveries table currently has separate FKs (event_id ->
notification_events and device_id -> push_devices) which allows pairing records
from different users; modify the schema so deliveries stores a user_id column
and enforces that both the event and device belong to that same user by adding
composite foreign keys: have (event_id, user_id) reference
notification_events(id, user_id) and (device_id, user_id) reference
push_devices(id, user_id) (or implement an equivalent AFTER INSERT/UPDATE
trigger that rejects rows where notification_events.user_id !=
push_devices.user_id), updating the CREATE TABLE/ALTER TABLE statements that
define user_id, event_id, and device_id to use these composite constraints.
- Line 51: The migration currently defines the attempt_count column but allows
negative values; update the migration to enforce non-negative counts by adding a
CHECK constraint on attempt_count (e.g., ensure attempt_count >= 0) so the
column remains NOT NULL with DEFAULT 0 and cannot be written negative values;
either add the CHECK inline in the column definition for attempt_count or add a
separate ALTER TABLE ... ADD CONSTRAINT referencing attempt_count to enforce the
non-negative rule.

In `@migrations/2026-03-23-180000_agent_schedules_v1/up.sql`:
- Around line 4-5: agent_schedule_runs currently allows schedule_id to reference
agent_schedules while user_id and agent_id are independently constrained, which
can create inconsistent rows; fix by enforcing a composite foreign key so runs
cannot drift: add a composite FK on (schedule_id, user_id, agent_id) referencing
agent_schedules(id, user_id, agent_id) (and drop the separate user_id and
agent_id FKs or make them non-conflicting) so agent_schedule_runs.schedule_id
always matches the denormalized owner fields in agent_schedules; apply the same
change for the other referenced block (lines 38-40) to keep consistency.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 37ac7577-cb93-4375-9717-d0e00e2db1c5

📥 Commits

Reviewing files that changed from the base of the PR and between ad4158a and ff413fb.

📒 Files selected for processing (14)
  • migrations/2026-02-10-020612_user_embeddings/down.sql
  • migrations/2026-02-10-020612_user_embeddings/up.sql
  • migrations/2026-02-10-214235_maple_agent_mvp_storage/down.sql
  • migrations/2026-02-10-214235_maple_agent_mvp_storage/up.sql
  • migrations/2026-02-24-150000_user_messages_attachment_text/down.sql
  • migrations/2026-02-24-150000_user_messages_attachment_text/up.sql
  • migrations/2026-03-07-120000_push_notifications_v1/down.sql
  • migrations/2026-03-07-120000_push_notifications_v1/up.sql
  • migrations/2026-03-22-033535_add_user_preferences/down.sql
  • migrations/2026-03-22-033535_add_user_preferences/up.sql
  • migrations/2026-03-23-180000_agent_schedules_v1/down.sql
  • migrations/2026-03-23-180000_agent_schedules_v1/up.sql
  • migrations/2026-03-24-220000_agent_message_reactions/down.sql
  • migrations/2026-03-24-220000_agent_message_reactions/up.sql

Comment on lines +15 to +17
user_message_id BIGINT REFERENCES user_messages(id) ON DELETE CASCADE,
assistant_message_id BIGINT REFERENCES assistant_messages(id) ON DELETE CASCADE,
conversation_id BIGINT REFERENCES conversations(id) ON DELETE CASCADE,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Message provenance constraints don’t enforce ownership consistency.

Current checks guarantee shape (which FKs are null/non-null) but do not guarantee that referenced user_messages/assistant_messages belong to the same user_id (and conversation) on the embedding row. This can admit cross-user linkage bugs.

If helpful, I can sketch a follow-up migration strategy to enforce this with composite key constraints (or trigger-based validation where composite FKs are not practical).

Also applies to: 38-55

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@migrations/2026-02-10-020612_user_embeddings/up.sql` around lines 15 - 17,
The FK checks currently ensure presence/absence of
user_message_id/assistant_message_id but do not enforce that the referenced
user_messages/assistant_messages rows belong to the same user_id and
conversation as the embedding row; update the schema to add ownership
consistency checks for user_message_id, assistant_message_id and conversation_id
by enforcing that user_messages.user_id = <embeddings>.user_id and
assistant_messages.user_id = <embeddings>.user_id and that the referenced
messages’ conversation_id = <embeddings>.conversation_id. Implement this either
by adding composite foreign-key constraints (if you can alter
user_messages/assistant_messages to expose the composite key) or by adding a
BEFORE INSERT/UPDATE trigger on the embeddings table that queries user_messages
and assistant_messages and raises an exception when the user_id or
conversation_id do not match; reference the columns user_message_id,
assistant_message_id, conversation_id, user_id, user_messages and
assistant_messages when locating where to add the constraint/trigger in the
migration.

Comment on lines +58 to +75
conversation_id BIGINT NOT NULL REFERENCES conversations(id) ON DELETE CASCADE,
kind TEXT NOT NULL,
parent_agent_id BIGINT REFERENCES agents(id) ON DELETE SET NULL,
display_name_enc BYTEA,
purpose_enc BYTEA,
created_by TEXT NOT NULL DEFAULT 'user',

created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
updated_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,

CONSTRAINT agents_kind_check CHECK (kind IN ('main', 'subagent')),
CONSTRAINT agents_created_by_check CHECK (created_by IN ('user', 'agent')),
CONSTRAINT agents_parent_check CHECK (
(kind = 'main' AND parent_agent_id IS NULL)
OR (kind = 'subagent' AND parent_agent_id IS NOT NULL)
),
UNIQUE(conversation_id)
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

UNIQUE(conversation_id) prevents storing subagents in the same conversation.

This constraint makes only one row possible per conversation, which conflicts with supporting both a main agent and subagents for that conversation.

Suggested schema adjustment
 CREATE TABLE agents (
@@
-    UNIQUE(conversation_id)
+    -- Allow multiple agents per conversation; keep "one main" via partial unique index below.
 );
📝 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.

Suggested change
conversation_id BIGINT NOT NULL REFERENCES conversations(id) ON DELETE CASCADE,
kind TEXT NOT NULL,
parent_agent_id BIGINT REFERENCES agents(id) ON DELETE SET NULL,
display_name_enc BYTEA,
purpose_enc BYTEA,
created_by TEXT NOT NULL DEFAULT 'user',
created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
updated_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
CONSTRAINT agents_kind_check CHECK (kind IN ('main', 'subagent')),
CONSTRAINT agents_created_by_check CHECK (created_by IN ('user', 'agent')),
CONSTRAINT agents_parent_check CHECK (
(kind = 'main' AND parent_agent_id IS NULL)
OR (kind = 'subagent' AND parent_agent_id IS NOT NULL)
),
UNIQUE(conversation_id)
);
CONSTRAINT agents_kind_check CHECK (kind IN ('main', 'subagent')),
CONSTRAINT agents_created_by_check CHECK (created_by IN ('user', 'agent')),
CONSTRAINT agents_parent_check CHECK (
(kind = 'main' AND parent_agent_id IS NULL)
OR (kind = 'subagent' AND parent_agent_id IS NOT NULL)
)
);
-- Enforce only one main agent per conversation while allowing multiple subagents
CREATE UNIQUE INDEX agents_conversation_main_idx ON agents(conversation_id) WHERE kind = 'main';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@migrations/2026-02-10-214235_maple_agent_mvp_storage/up.sql` around lines 58
- 75, The UNIQUE(conversation_id) constraint on the agents table prevents
multiple agents (main + subagents) per conversation; remove that UNIQUE(...) and
instead enforce uniqueness only for main agents by creating a partial unique
index or constraint such as a UNIQUE index on (conversation_id) WHERE kind =
'main' (keep the existing CONSTRAINT agents_kind_check and CONSTRAINT
agents_parent_check unchanged to preserve kind/parent rules). Locate the agents
table definition (where CONSTRAINT agents_kind_check, CONSTRAINT
agents_parent_check are declared), remove the final UNIQUE(conversation_id)
line, and add a partial unique index for main agents (e.g., CREATE UNIQUE INDEX
... ON agents (conversation_id) WHERE kind = 'main') so subagents can coexist in
the same conversation.

Comment on lines +60 to +73
parent_agent_id BIGINT REFERENCES agents(id) ON DELETE SET NULL,
display_name_enc BYTEA,
purpose_enc BYTEA,
created_by TEXT NOT NULL DEFAULT 'user',

created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
updated_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,

CONSTRAINT agents_kind_check CHECK (kind IN ('main', 'subagent')),
CONSTRAINT agents_created_by_check CHECK (created_by IN ('user', 'agent')),
CONSTRAINT agents_parent_check CHECK (
(kind = 'main' AND parent_agent_id IS NULL)
OR (kind = 'subagent' AND parent_agent_id IS NOT NULL)
),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Parent linkage is not scoped to the same tenant/conversation.

parent_agent_id can currently reference an agent from another user_id and/or conversation_id. That weakens data isolation and can create invalid hierarchies.

I can draft a follow-up migration to enforce parent scope (same user_id and conversation_id) via composite constraints/trigger checks if you want.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@migrations/2026-02-10-214235_maple_agent_mvp_storage/up.sql` around lines 60
- 73, The agents table allows parent_agent_id to point to agents in other
tenants/conversations; add a scoped validation so a subagent's parent has the
same user_id and conversation_id. Implement this by adding a trigger-based
check: create a function (e.g. validate_agent_parent_scope) that on
INSERT/UPDATE, when parent_agent_id IS NOT NULL selects the parent row from
agents and ensures parent.user_id = NEW.user_id AND parent.conversation_id =
NEW.conversation_id, and raise an exception if not; attach this function as a
BEFORE INSERT OR UPDATE trigger on agents (or replace/augment CONSTRAINT
agents_parent_check) so cross-tenant/conversation parent links are rejected.

CREATE TABLE push_devices (
id BIGSERIAL PRIMARY KEY,
uuid UUID NOT NULL DEFAULT uuid_generate_v4() UNIQUE,
user_id UUID NOT NULL REFERENCES users(uuid) ON DELETE CASCADE,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Enforce that a delivery targets the same user as its event.

These foreign keys are independent, so the database will accept a row that pairs one user's notification_events record with another user's push_devices record. That turns a bad insert into a cross-user notification leak. Please bind deliveries to a single principal at the schema level, for example with a stored user_id plus composite FKs, or a trigger.

Also applies to: 30-30, 46-47

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@migrations/2026-03-07-120000_push_notifications_v1/up.sql` at line 4, The
deliveries table currently has separate FKs (event_id -> notification_events and
device_id -> push_devices) which allows pairing records from different users;
modify the schema so deliveries stores a user_id column and enforces that both
the event and device belong to that same user by adding composite foreign keys:
have (event_id, user_id) reference notification_events(id, user_id) and
(device_id, user_id) reference push_devices(id, user_id) (or implement an
equivalent AFTER INSERT/UPDATE trigger that rejects rows where
notification_events.user_id != push_devices.user_id), updating the CREATE
TABLE/ALTER TABLE statements that define user_id, event_id, and device_id to use
these composite constraints.

status TEXT NOT NULL DEFAULT 'pending' CHECK (
status IN ('pending', 'leased', 'sent', 'retry', 'failed', 'invalid_token', 'cancelled')
),
attempt_count INTEGER NOT NULL DEFAULT 0,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Constrain attempt_count to non-negative values.

attempt_count is queue state, but this column currently accepts negative numbers. One bad write can skew retry selection and monitoring.

Suggested migration tweak
-    attempt_count INTEGER NOT NULL DEFAULT 0,
+    attempt_count INTEGER NOT NULL DEFAULT 0 CHECK (attempt_count >= 0),
📝 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.

Suggested change
attempt_count INTEGER NOT NULL DEFAULT 0,
attempt_count INTEGER NOT NULL DEFAULT 0 CHECK (attempt_count >= 0),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@migrations/2026-03-07-120000_push_notifications_v1/up.sql` at line 51, The
migration currently defines the attempt_count column but allows negative values;
update the migration to enforce non-negative counts by adding a CHECK constraint
on attempt_count (e.g., ensure attempt_count >= 0) so the column remains NOT
NULL with DEFAULT 0 and cannot be written negative values; either add the CHECK
inline in the column definition for attempt_count or add a separate ALTER TABLE
... ADD CONSTRAINT referencing attempt_count to enforce the non-negative rule.

Comment on lines +4 to +5
user_id UUID NOT NULL REFERENCES users(uuid) ON DELETE CASCADE,
agent_id BIGINT NOT NULL REFERENCES agents(id) ON DELETE CASCADE,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep agent_schedule_runs consistent with its parent schedule.

Right now schedule_id is constrained independently from user_id and agent_id, so a bad insert can create a run whose denormalized owner fields no longer match the referenced agent_schedules row. That makes any filter keyed off agent_schedule_runs.user_id or agent_schedule_runs.agent_id unsafe. Either derive those values from schedule_id, or add a composite FK so they cannot drift.

Also applies to: 38-40

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@migrations/2026-03-23-180000_agent_schedules_v1/up.sql` around lines 4 - 5,
agent_schedule_runs currently allows schedule_id to reference agent_schedules
while user_id and agent_id are independently constrained, which can create
inconsistent rows; fix by enforcing a composite foreign key so runs cannot
drift: add a composite FK on (schedule_id, user_id, agent_id) referencing
agent_schedules(id, user_id, agent_id) (and drop the separate user_id and
agent_id FKs or make them non-conflicting) so agent_schedule_runs.schedule_id
always matches the denormalized owner fields in agent_schedules; apply the same
change for the other referenced block (lines 38-40) to keep consistency.

Preserve the agent schema history on master before behavior ships so prod and dev can converge on the same migration lineage.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
migrations/2026-02-10-020612_user_embeddings/up.sql (1)

8-17: ⚠️ Potential issue | 🟠 Major

Scope provenance to the same owner and conversation.

The current FK/check combination only validates shape. It still allows an embedding row to carry one user_id / conversation_id while user_message_id or assistant_message_id points at a different user's message or a different conversation. Please enforce same-user/same-conversation provenance with composite constraints or a BEFORE INSERT/UPDATE trigger.

Also applies to: 38-55

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@migrations/2026-02-10-020612_user_embeddings/up.sql` around lines 8 - 17, The
embedding rows currently validate column shapes but not provenance; update the
schema so that when user_message_id or assistant_message_id is set the
referenced message belongs to the same user_id and conversation_id on the
embedding row: either (A) replace the separate FKs with composite foreign keys
referencing (user_id, id, conversation_id) tuples on user_messages and
assistant_messages (or a composite key/index on those message tables) or (B) add
a BEFORE INSERT OR UPDATE trigger on the embeddings table that loads the
referenced user_messages/assistant_messages and verifies their user_id and
conversation_id match the embedding's user_id and conversation_id (raise an
error otherwise). Target the columns user_id, conversation_id, user_message_id,
assistant_message_id and the referenced tables user_messages and
assistant_messages when implementing this change.
migrations/2026-02-10-214235_maple_agent_mvp_storage/up.sql (2)

74-79: ⚠️ Potential issue | 🟠 Major

The uniqueness rules are too tight for conversation-scoped agents.

UNIQUE(conversation_id) blocks subagents entirely, and idx_agents_one_main_per_user then allows only one kind='main' row across all of a user's conversations. If the intended invariant is “one main agent per conversation,” drop the table-level unique constraint and scope the partial unique index to conversation_id instead.

Suggested schema change
-    UNIQUE(conversation_id)
 );
 
-CREATE UNIQUE INDEX idx_agents_one_main_per_user
-    ON agents(user_id)
+CREATE UNIQUE INDEX idx_agents_one_main_per_conversation
+    ON agents(conversation_id)
     WHERE kind = 'main';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@migrations/2026-02-10-214235_maple_agent_mvp_storage/up.sql` around lines 74
- 79, The table-level UNIQUE(conversation_id) prevents subagents and the partial
index idx_agents_one_main_per_user currently enforces one 'main' across a user's
all conversations; drop the table-level UNIQUE(conversation_id) and replace the
partial index with a conversation-scoped unique partial index (e.g., remove or
drop idx_agents_one_main_per_user and create a new UNIQUE INDEX
idx_agents_one_main_per_conversation ON agents(conversation_id) WHERE kind =
'main') so the invariant becomes “at most one main agent per conversation.”

56-60: ⚠️ Potential issue | 🟠 Major

Scope agent references to the same tenant/conversation.

The bare FKs here still permit an agent row to reference another user's conversation, and parent_agent_id can still point at a parent from a different user or conversation. agents_parent_check only enforces nullability. Please add composite constraints or a trigger that validates NEW.user_id / NEW.conversation_id against both referenced rows.

Also applies to: 70-73

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@migrations/2026-02-10-214235_maple_agent_mvp_storage/up.sql`:
- Around line 28-40: Add a DB-side validation (BEFORE INSERT OR UPDATE trigger)
to enforce tenant and conversation scoping: create a trigger function (e.g.,
validate_summary_scope) for the conversation_summaries table that, for NEW rows,
verifies that the referenced conversations row (conversations.id =
NEW.conversation_id) has the same user_id as NEW.user_id, and if
NEW.previous_summary_id IS NOT NULL verifies that the referenced
conversation_summaries row (conversation_summaries.id = NEW.previous_summary_id)
has both conversation_id = NEW.conversation_id and user_id = NEW.user_id; raise
an error if any check fails so summaries cannot be attached across users or
conversations.

---

Duplicate comments:
In `@migrations/2026-02-10-020612_user_embeddings/up.sql`:
- Around line 8-17: The embedding rows currently validate column shapes but not
provenance; update the schema so that when user_message_id or
assistant_message_id is set the referenced message belongs to the same user_id
and conversation_id on the embedding row: either (A) replace the separate FKs
with composite foreign keys referencing (user_id, id, conversation_id) tuples on
user_messages and assistant_messages (or a composite key/index on those message
tables) or (B) add a BEFORE INSERT OR UPDATE trigger on the embeddings table
that loads the referenced user_messages/assistant_messages and verifies their
user_id and conversation_id match the embedding's user_id and conversation_id
(raise an error otherwise). Target the columns user_id, conversation_id,
user_message_id, assistant_message_id and the referenced tables user_messages
and assistant_messages when implementing this change.

In `@migrations/2026-02-10-214235_maple_agent_mvp_storage/up.sql`:
- Around line 74-79: The table-level UNIQUE(conversation_id) prevents subagents
and the partial index idx_agents_one_main_per_user currently enforces one 'main'
across a user's all conversations; drop the table-level UNIQUE(conversation_id)
and replace the partial index with a conversation-scoped unique partial index
(e.g., remove or drop idx_agents_one_main_per_user and create a new UNIQUE INDEX
idx_agents_one_main_per_conversation ON agents(conversation_id) WHERE kind =
'main') so the invariant becomes “at most one main agent per conversation.”
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d5aad976-4f47-4021-b2b1-8b778a4268aa

📥 Commits

Reviewing files that changed from the base of the PR and between ff413fb and 2b9f6a5.

📒 Files selected for processing (16)
  • migrations/2026-02-10-020612_user_embeddings/down.sql
  • migrations/2026-02-10-020612_user_embeddings/up.sql
  • migrations/2026-02-10-214235_maple_agent_mvp_storage/down.sql
  • migrations/2026-02-10-214235_maple_agent_mvp_storage/up.sql
  • migrations/2026-02-24-150000_user_messages_attachment_text/down.sql
  • migrations/2026-02-24-150000_user_messages_attachment_text/up.sql
  • migrations/2026-03-07-120000_push_notifications_v1/down.sql
  • migrations/2026-03-07-120000_push_notifications_v1/up.sql
  • migrations/2026-03-22-033535_add_user_preferences/down.sql
  • migrations/2026-03-22-033535_add_user_preferences/up.sql
  • migrations/2026-03-23-180000_agent_schedules_v1/down.sql
  • migrations/2026-03-23-180000_agent_schedules_v1/up.sql
  • migrations/2026-03-24-220000_agent_message_reactions/down.sql
  • migrations/2026-03-24-220000_agent_message_reactions/up.sql
  • src/models/responses.rs
  • src/models/schema.rs
✅ Files skipped from review due to trivial changes (10)
  • migrations/2026-03-23-180000_agent_schedules_v1/down.sql
  • migrations/2026-03-24-220000_agent_message_reactions/up.sql
  • migrations/2026-02-24-150000_user_messages_attachment_text/down.sql
  • migrations/2026-02-24-150000_user_messages_attachment_text/up.sql
  • migrations/2026-03-22-033535_add_user_preferences/down.sql
  • migrations/2026-03-07-120000_push_notifications_v1/down.sql
  • migrations/2026-03-24-220000_agent_message_reactions/down.sql
  • migrations/2026-03-22-033535_add_user_preferences/up.sql
  • migrations/2026-02-10-020612_user_embeddings/down.sql
  • migrations/2026-03-23-180000_agent_schedules_v1/up.sql
🚧 Files skipped from review as they are similar to previous changes (2)
  • migrations/2026-02-10-214235_maple_agent_mvp_storage/down.sql
  • migrations/2026-03-07-120000_push_notifications_v1/up.sql

Comment on lines +28 to +40
user_id UUID NOT NULL REFERENCES users(uuid) ON DELETE CASCADE,
conversation_id BIGINT NOT NULL REFERENCES conversations(id) ON DELETE CASCADE,

from_created_at TIMESTAMPTZ NOT NULL,
to_created_at TIMESTAMPTZ NOT NULL,
message_count INTEGER NOT NULL,

content_enc BYTEA NOT NULL,
content_tokens INTEGER NOT NULL,

embedding_enc BYTEA,

previous_summary_id BIGINT REFERENCES conversation_summaries(id) ON DELETE SET NULL,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep summary links inside the same user/conversation.

conversation_id and previous_summary_id only prove that the referenced rows exist. As written, a summary can be attached to another user's conversation or chained onto a summary from a different conversation, which breaks tenant isolation and summary sequencing. Add scoped validation so both references share NEW.user_id and NEW.conversation_id.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@migrations/2026-02-10-214235_maple_agent_mvp_storage/up.sql` around lines 28
- 40, Add a DB-side validation (BEFORE INSERT OR UPDATE trigger) to enforce
tenant and conversation scoping: create a trigger function (e.g.,
validate_summary_scope) for the conversation_summaries table that, for NEW rows,
verifies that the referenced conversations row (conversations.id =
NEW.conversation_id) has the same user_id as NEW.user_id, and if
NEW.previous_summary_id IS NOT NULL verifies that the referenced
conversation_summaries row (conversation_summaries.id = NEW.previous_summary_id)
has both conversation_id = NEW.conversation_id and user_id = NEW.user_id; raise
an error if any check fails so summaries cannot be attached across users or
conversations.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
@AnthonyRonning
AnthonyRonning merged commit e86f7e5 into master Apr 8, 2026
6 checks passed
@AnthonyRonning
AnthonyRonning deleted the chore/agent-schema-sync-feb-mar branch April 8, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant