From d2e3f7dce589050c46d8253a75af6f9ad8be2f13 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 6 Jul 2026 16:56:30 -0500 Subject: [PATCH] fix(db): correct 0130 responsible-user backfill in place (inbox resurface) (#9111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Users triage agent work through the issue inbox, which suppresses archived issues by comparing issue `updated_at` against the archive timestamp > - Migration `0130_run_responsible_user_invariant` backfilled responsible-user columns but also set `updated_at = now()` on every row it touched (companies, issues, routines, routine_runs, heartbeat_runs) > - That blanket timestamp bump made every archived issue look newly updated, resurfacing thousands of archived issues into every user's inbox > - This pull request corrects the 0130 backfill in place so it only fills `NULL` responsible-user columns and never touches timestamps, and adds tests that prevent this class of bug from being reintroduced > - The benefit is that inbox archive suppression stays intact across migrations, and no future migration backfill can silently bump `updated_at` on user-visible tables ## Linked Issues or Issue Description **Bug description (no public issue exists):** - **What happened:** after upgrading a dev instance across migration 0130, every previously archived inbox item resurfaced as unread/new for all users. - **Expected:** data backfills must not alter row modification timestamps; archived issues stay archived unless genuinely updated. - **Root cause:** the 0130 backfill's `UPDATE` statements set `updated_at = now()` alongside the responsible-user columns. ## What Changed - `packages/db/src/migrations/0130_run_responsible_user_invariant.sql`: removed all `updated_at = now()` assignments from the backfill UPDATEs; the migration now only fills `NULL` responsible-user columns. The file is corrected **in place** (no new migration number, journal untouched) because 0130 has never shipped in a published release. - `packages/db/src/client.test.ts`: added a guard test that scans every migration and rejects backfills that bump `updated_at` on user-visible tables (with an explicit allowlist for the pre-existing 0131 repair migration). - `packages/db/src/client.test.ts`: added a replay test that simulates an already-migrated database picking up the corrected file (deletes the 0130 ledger hash, re-applies mid-journal) and asserts issue `updated_at` and inbox-archive suppression ordering are untouched. ### Why an in-place edit is safe - 0130 only exists on master/canary builds; the latest published release (v2026.626.0) predates it. - The migration ledger is content-hash based: databases that already applied the old 0130 keep an orphaned hash row (harmless) and see the corrected file as pending, so they replay the corrected backfill — which is idempotent (fills `NULL`s only, no timestamp writes). - Fresh databases simply run the corrected 0130 in journal order. ## Verification - `pnpm --filter @paperclipai/db run check:migrations` — clean - `cd packages/db && npx vitest run src/client.test.ts` — 11/11 passing against embedded Postgres, including the new guard and mid-journal replay tests ## Risks - Migration safety: the corrected backfill is idempotent and only writes `NULL` columns; replay on already-migrated databases is exercised directly by the new test. No schema changes. - Databases that already ran the old 0130 keep the bumped timestamps from that run; repairing historical damage is intentionally out of scope here (no released build ever contained the bug). ## Model Used - Claude Opus 4.7 (`claude-opus-4-7`, extended thinking, tool use) via Claude Code / Paperclip agent runtime ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above (backup health alert work split into #9113; no other related open PRs) - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (none needed for a migration content fix) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- packages/db/src/client.test.ts | 204 ++++++++++++++++++ .../0130_run_responsible_user_invariant.sql | 27 +-- 2 files changed, 213 insertions(+), 18 deletions(-) diff --git a/packages/db/src/client.test.ts b/packages/db/src/client.test.ts index 75e7effcc5..fc1837962f 100644 --- a/packages/db/src/client.test.ts +++ b/packages/db/src/client.test.ts @@ -29,6 +29,48 @@ async function migrationHash(migrationFile: string): Promise { return createHash("sha256").update(content).digest("hex"); } +const userVisibleUpdatedAtTables = new Set([ + "companies", + "heartbeat_runs", + "issue_comments", + "issues", + "routine_runs", + "routines", +]); + +const migrationUpdatedAtUpdateAllowlist = new Map>([ + [ + "0105_instance_scoped_environments.sql", + new Set(["issues"]), + ], + [ + "0131_repair_run_responsible_user_context_refs.sql", + new Set(["heartbeat_runs"]), + ], +]); + +function findUserVisibleUpdatedAtBackfillViolations( + migrationFile: string, + content: string, +): string[] { + const allowedTables = migrationUpdatedAtUpdateAllowlist.get(migrationFile) ?? new Set(); + const violations: string[] = []; + + for (const statement of content.split("--> statement-breakpoint")) { + const updateMatch = statement.match(/\bUPDATE\s+"([^"]+)"/i); + if (!updateMatch) continue; + + const tableName = updateMatch[1]; + if (!userVisibleUpdatedAtTables.has(tableName)) continue; + if (!/\bSET\b[\s\S]*"updated_at"\s*=/i.test(statement)) continue; + if (allowedTables.has(tableName)) continue; + + violations.push(`${migrationFile}: UPDATE "${tableName}" sets updated_at`); + } + + return violations; +} + afterEach(async () => { while (cleanups.length > 0) { const cleanup = cleanups.pop(); @@ -43,6 +85,35 @@ if (!embeddedPostgresSupport.supported) { } describeEmbeddedPostgres("applyPendingMigrations", () => { + it("rejects unallowlisted migration backfills that bump updated_at on user-visible tables", async () => { + const entries = await fs.promises.readdir(new URL("./migrations", import.meta.url), { + withFileTypes: true, + }); + const violations: string[] = []; + + for (const entry of entries) { + if (!entry.isFile() || !entry.name.endsWith(".sql")) continue; + const content = await fs.promises.readFile( + new URL(`./migrations/${entry.name}`, import.meta.url), + "utf8", + ); + violations.push(...findUserVisibleUpdatedAtBackfillViolations(entry.name, content)); + } + + expect(violations).toEqual([]); + expect( + findUserVisibleUpdatedAtBackfillViolations( + "9999_bad_backfill.sql", + ` + UPDATE "issues" AS i + SET "responsible_user_id" = 'owner-user', + "updated_at" = now() + WHERE i."responsible_user_id" IS NULL; + `, + ), + ).toEqual(['9999_bad_backfill.sql: UPDATE "issues" sets updated_at']); + }); + it( "applies an inserted earlier migration without replaying later legacy migrations", async () => { @@ -542,6 +613,139 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { 20_000, ); + it( + "replays migration 0130 without bumping issue updated_at for inbox archives", + async () => { + const connectionString = await createTempDatabase(); + + await applyPendingMigrations(connectionString); + + const sql = postgres(connectionString, { max: 1, onnotice: () => {} }); + try { + const runResponsibleUserHash = await migrationHash( + "0130_run_responsible_user_invariant.sql", + ); + + await sql.unsafe(` + INSERT INTO "companies" ("id", "name", "issue_prefix", "created_at", "updated_at") + VALUES ( + '00000000-0000-0000-0000-000000000120', + 'Migration Inbox Co', + 'TST120', + '2026-03-26T09:00:00.000Z', + '2026-03-26T09:00:00.000Z' + ) + `); + await sql.unsafe(` + INSERT INTO "company_memberships" ( + "id", + "company_id", + "principal_type", + "principal_id", + "status", + "membership_role", + "created_at", + "updated_at" + ) + VALUES ( + '00000000-0000-0000-0000-000000000121', + '00000000-0000-0000-0000-000000000120', + 'user', + 'owner-user', + 'active', + 'owner', + '2026-03-26T09:00:00.000Z', + '2026-03-26T09:00:00.000Z' + ) + `); + await sql.unsafe(` + INSERT INTO "issues" ( + "id", + "company_id", + "title", + "status", + "responsible_user_id", + "created_at", + "updated_at" + ) + VALUES ( + '00000000-0000-0000-0000-000000000122', + '00000000-0000-0000-0000-000000000120', + 'Archived issue needing responsible user backfill', + 'todo', + NULL, + '2026-03-26T10:00:00.000Z', + '2026-03-26T10:00:00.000Z' + ) + `); + await sql.unsafe(` + INSERT INTO "issue_inbox_archives" ( + "id", + "company_id", + "issue_id", + "user_id", + "archived_at", + "created_at", + "updated_at" + ) + VALUES ( + '00000000-0000-0000-0000-000000000123', + '00000000-0000-0000-0000-000000000120', + '00000000-0000-0000-0000-000000000122', + 'owner-user', + '2026-03-26T12:00:00.000Z', + '2026-03-26T12:00:00.000Z', + '2026-03-26T12:00:00.000Z' + ) + `); + await sql.unsafe( + `DELETE FROM "drizzle"."__drizzle_migrations" WHERE hash = '${runResponsibleUserHash}'`, + ); + } finally { + await sql.end(); + } + + const pendingState = await inspectMigrations(connectionString); + expect(pendingState).toMatchObject({ + status: "needsMigrations", + pendingMigrations: ["0130_run_responsible_user_invariant.sql"], + reason: "pending-migrations", + }); + + await applyPendingMigrations(connectionString); + + const verifySql = postgres(connectionString, { max: 1, onnotice: () => {} }); + try { + const rows = await verifySql.unsafe<{ + responsible_user_id: string | null; + updated_at: Date; + inbox_archive_still_current: boolean; + }[]>(` + SELECT + i."responsible_user_id", + i."updated_at", + EXISTS ( + SELECT 1 + FROM "issue_inbox_archives" AS archive + WHERE archive."company_id" = i."company_id" + AND archive."issue_id" = i."id" + AND archive."user_id" = 'owner-user' + AND archive."archived_at" >= i."updated_at" + ) AS "inbox_archive_still_current" + FROM "issues" AS i + WHERE i."id" = '00000000-0000-0000-0000-000000000122' + `); + expect(rows).toHaveLength(1); + expect(rows[0]?.responsible_user_id).toBe("owner-user"); + expect(rows[0]?.updated_at.toISOString()).toBe("2026-03-26T10:00:00.000Z"); + expect(rows[0]?.inbox_archive_still_current).toBe(true); + } finally { + await verifySql.end(); + } + }, + 20_000, + ); + it( "replays the run responsible user repair migration when heartbeat run issue refs are identifiers", async () => { diff --git a/packages/db/src/migrations/0130_run_responsible_user_invariant.sql b/packages/db/src/migrations/0130_run_responsible_user_invariant.sql index 1ae59fa5a9..c5cc6d5eee 100644 --- a/packages/db/src/migrations/0130_run_responsible_user_invariant.sql +++ b/packages/db/src/migrations/0130_run_responsible_user_invariant.sql @@ -11,8 +11,7 @@ WITH owner_defaults AS ( ORDER BY "company_id", "created_at" ASC, "id" ASC ) UPDATE "companies" AS c -SET "default_responsible_user_id" = owner_defaults."user_id", - "updated_at" = now() +SET "default_responsible_user_id" = owner_defaults."user_id" FROM owner_defaults WHERE c."id" = owner_defaults."company_id" AND c."default_responsible_user_id" IS NULL; @@ -50,15 +49,13 @@ resolved_issue_users AS ( ORDER BY "issue_id", "depth" ASC ) UPDATE "issues" AS i -SET "responsible_user_id" = resolved_issue_users."user_id", - "updated_at" = now() +SET "responsible_user_id" = resolved_issue_users."user_id" FROM resolved_issue_users WHERE i."id" = resolved_issue_users."issue_id" AND i."responsible_user_id" IS NULL; --> statement-breakpoint UPDATE "issues" AS i -SET "responsible_user_id" = c."default_responsible_user_id", - "updated_at" = now() +SET "responsible_user_id" = c."default_responsible_user_id" FROM "companies" AS c WHERE i."company_id" = c."id" AND i."responsible_user_id" IS NULL @@ -76,8 +73,7 @@ WITH routine_responsible_users AS ( WHERE r."responsible_user_id" IS NULL ) UPDATE "routines" AS r -SET "responsible_user_id" = routine_responsible_users."user_id", - "updated_at" = now() +SET "responsible_user_id" = routine_responsible_users."user_id" FROM routine_responsible_users WHERE r."id" = routine_responsible_users."id" AND routine_responsible_users."user_id" IS NOT NULL; @@ -114,15 +110,13 @@ WITH routine_run_responsible_users AS ( WHERE rr."responsible_user_id" IS NULL ) UPDATE "routine_runs" AS rr -SET "responsible_user_id" = routine_run_responsible_users."user_id", - "updated_at" = now() +SET "responsible_user_id" = routine_run_responsible_users."user_id" FROM routine_run_responsible_users WHERE rr."id" = routine_run_responsible_users."id" AND routine_run_responsible_users."user_id" IS NOT NULL; --> statement-breakpoint UPDATE "heartbeat_runs" AS h -SET "responsible_user_id" = original."responsible_user_id", - "updated_at" = now() +SET "responsible_user_id" = original."responsible_user_id" FROM "heartbeat_runs" AS original WHERE h."retry_of_run_id" = original."id" AND h."company_id" = original."company_id" @@ -192,15 +186,13 @@ resolved_run_users AS ( ORDER BY "run_id", "ref_priority" ASC, "match_priority" ASC ) UPDATE "heartbeat_runs" AS h -SET "responsible_user_id" = resolved_run_users."responsible_user_id", - "updated_at" = now() +SET "responsible_user_id" = resolved_run_users."responsible_user_id" FROM resolved_run_users WHERE h."id" = resolved_run_users."run_id" AND h."responsible_user_id" IS NULL; --> statement-breakpoint UPDATE "heartbeat_runs" AS h -SET "responsible_user_id" = awr."requested_by_actor_id", - "updated_at" = now() +SET "responsible_user_id" = awr."requested_by_actor_id" FROM "agent_wakeup_requests" AS awr WHERE h."wakeup_request_id" = awr."id" AND h."company_id" = awr."company_id" @@ -209,8 +201,7 @@ WHERE h."wakeup_request_id" = awr."id" AND awr."requested_by_actor_id" IS NOT NULL; --> statement-breakpoint UPDATE "heartbeat_runs" AS h -SET "responsible_user_id" = c."default_responsible_user_id", - "updated_at" = now() +SET "responsible_user_id" = c."default_responsible_user_id" FROM "companies" AS c WHERE h."company_id" = c."id" AND h."responsible_user_id" IS NULL