RomanNum3ral
/
paperclip
mirror of https://github.com/paperclipai/paperclip.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(db): correct 0130 responsible-user backfill in place (inbox resurface) (#9111) 

## 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 <noreply@paperclip.ing>
This commit is contained in:
Dotta 2026-07-06 16:56:30 -05:00 committed by GitHub
parent bded6ac5c7
commit d2e3f7dce5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 213 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

204
packages/db/src/client.test.ts
View File

@ -29,6 +29,48 @@ async function migrationHash(migrationFile: string): Promise<string> {
return createHash("sha256").update(content).digest("hex");
}
const userVisibleUpdatedAtTables = new Set([
"companies",
"heartbeat_runs",
"issue_comments",
"issues",
"routine_runs",
"routines",
]);
const migrationUpdatedAtUpdateAllowlist = new Map<string, ReadonlySet<string>>([
[
"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<string>();
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 () => {

27
packages/db/src/migrations/0130_run_responsible_user_invariant.sql
View File

@ -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