-
Notifications
You must be signed in to change notification settings - Fork 138
fix: use table recreation for sub_agent_skills UNIQUE constraint migration #2214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| -- Migration: Widen the UNIQUE constraint on sub_agent_skills to include tenant_id, project_id, agent_id | ||
| -- Problem: The original UNIQUE("sub_agent_id","skill_id") was too narrow; it should be scoped per-agent. | ||
| -- Approach: backup→drop→recreate→restore to avoid Doltgres DROP CONSTRAINT bug and PGlite name collisions. | ||
| -- Step 1: Create backup table (no constraints, just data storage) | ||
| CREATE TABLE "sub_agent_skills_backup" ( | ||
| "tenant_id" varchar(256), | ||
| "id" varchar(256), | ||
| "project_id" varchar(256), | ||
| "agent_id" varchar(256), | ||
| "sub_agent_id" varchar(256), | ||
| "skill_id" varchar(64), | ||
| "index" numeric, | ||
| "always_loaded" boolean, | ||
| "created_at" timestamp, | ||
| "updated_at" timestamp | ||
| );--> statement-breakpoint | ||
| -- Step 2: Copy all existing data to backup | ||
| INSERT INTO "sub_agent_skills_backup" SELECT * FROM "sub_agent_skills";--> statement-breakpoint | ||
| -- Step 3: Drop original table (frees all constraint/index names) | ||
| DROP TABLE "sub_agent_skills";--> statement-breakpoint | ||
| -- Step 4: Recreate table with correct wider UNIQUE constraint | ||
| CREATE TABLE "sub_agent_skills" ( | ||
| "tenant_id" varchar(256) NOT NULL, | ||
| "id" varchar(256) NOT NULL, | ||
| "project_id" varchar(256) NOT NULL, | ||
| "agent_id" varchar(256) NOT NULL, | ||
| "sub_agent_id" varchar(256) NOT NULL, | ||
| "skill_id" varchar(64) NOT NULL, | ||
| "index" numeric DEFAULT 0 NOT NULL, | ||
| "always_loaded" boolean DEFAULT false NOT NULL, | ||
| "created_at" timestamp DEFAULT now() NOT NULL, | ||
| "updated_at" timestamp DEFAULT now() NOT NULL, | ||
| CONSTRAINT "sub_agent_skills_tenant_id_project_id_agent_id_id_pk" PRIMARY KEY("tenant_id","project_id","agent_id","id"), | ||
| CONSTRAINT "sub_agent_skills_sub_agent_skill_unique" UNIQUE("tenant_id","project_id","agent_id","sub_agent_id","skill_id") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 CRITICAL: Schema/migration constraint mismatch Issue: This migration creates a 5-column UNIQUE constraint, but the Drizzle schema definition at // Current schema (line 293):
unique('sub_agent_skills_sub_agent_skill_unique').on(table.subAgentId, table.skillId)
// Migration creates (line 34):
UNIQUE("tenant_id","project_id","agent_id","sub_agent_id","skill_id")Why: This mismatch causes critical issues:
Fix: Update the Drizzle schema to match the migration: // manage-schema.ts line 293 should be:
unique('sub_agent_skills_sub_agent_skill_unique').on(
table.tenantId,
table.projectId,
table.agentId,
table.subAgentId,
table.skillId
)Then either:
Refs: |
||
| );--> statement-breakpoint | ||
| -- Step 5: Restore data from backup | ||
| INSERT INTO "sub_agent_skills" SELECT * FROM "sub_agent_skills_backup";--> statement-breakpoint | ||
| -- Step 6: Drop backup table | ||
| DROP TABLE "sub_agent_skills_backup";--> statement-breakpoint | ||
| -- Step 7: Re-add foreign keys | ||
| ALTER TABLE "sub_agent_skills" ADD CONSTRAINT "sub_agent_skills_sub_agent_fk" FOREIGN KEY ("tenant_id","project_id","agent_id","sub_agent_id") REFERENCES "public"."sub_agents"("tenant_id","project_id","agent_id","id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "sub_agent_skills" ADD CONSTRAINT "sub_agent_skills_skill_fk" FOREIGN KEY ("tenant_id","project_id","skill_id") REFERENCES "public"."skills"("tenant_id","project_id","id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| -- Step 8: Re-add index | ||
| CREATE INDEX "sub_agent_skills_skill_idx" ON "sub_agent_skills" USING btree ("skill_id"); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 MAJOR: Deployment strategy for environments where original migration succeeded
Issue: This PR modifies an existing migration file (
0010_oval_angel.sql) that exists onmain. Per AGENTS.md, existing migration files should not be edited after application. While the original migration fails on Doltgres (which is why this fix is needed), it may have succeeded on PostgreSQL/PGlite environments.Why: Environments where the original
DROP CONSTRAINT + ADD CONSTRAINTmigration already ran successfully will:__drizzle_migrationsThis isn't necessarily a problem if the end-state is the same, but it creates a divergence in migration history between environments.
Fix: The current approach appears acceptable given:
However, consider documenting:
Refs: