fix(provenance): clear the rows that went unknown after the first repair (#6982)

* fix(provenance): clear the rows that went unknown after the first repair

0005 is finished and will not run again — the runner records a name in
script_migrations and never offers it back, which is the contract a
run-once repair wants. But it cleared the backlog that existed at the
instant it ran, and the writer that produced that backlog kept running
until the fix in this branch. Nothing heals such a row in place, so each
one goes on reporting on every later read; a few dozen of them account
for thousands of log lines a week.

A second entry rather than deleting the first's tracking row: the
registry is append-only, and a repair that ran twice should say so
twice. It shares 0005's walk rather than restating it — the parent-first
lock ordering and the status re-check under that lock are subtleties
worth having once — and is idempotent, so it costs one empty query if
there is nothing left to repair.

* chore(provenance): record the deploy ordering the second pass depends on

promote-images needs migrate, so a script migration runs while the
previous image is still serving. A row an old instance creates between
this walk and the end of the rollout sits behind the cursor, and the
name is recorded on success, so it is never offered again. Widening the
walk would not help — the exposure is the minutes after it returns, not
the milliseconds during — so the requirement is to ship it in a release
after the writer fix is already promoted, which the file now says.

Author: icecrasher321

packages/db/script-migrations-paused-billing-attribution.test.ts

@@ -442,6 +442,7 @@ describe('script migration registry', () => {
       '0003_backfill_workspace_storage_usage',
       '0004_backfill_fork_kb_file_ownership',
       '0005_repair_unknown_table_row_provenance',
+      '0006_repair_unknown_table_row_provenance_second_pass',
     ])
   })
 })

packages/db/script-migrations/0005_repair_unknown_table_row_provenance.test.ts

@@ -4,6 +4,7 @@
 import type { Sql } from 'postgres'
 import { describe, expect, it, vi } from 'vitest'
 import { repairUnknownTableRowProvenance } from './0005_repair_unknown_table_row_provenance'
+import { repairUnknownTableRowProvenanceSecondPass } from './0006_repair_unknown_table_row_provenance_second_pass'
 
 function normalizeSql(value: string): string {
   return value.replace(/\s+/g, ' ').trim()
@@ -105,3 +106,31 @@ describe('0005 repair unknown table row provenance', () => {
     expect(cursors).toEqual(['', 'row-1'])
   })
 })
+
+/**
+ * 0005 is finished — the runner records a name and never offers it again — but it cleared only the
+ * backlog that existed when it ran, and the writers producing that backlog kept running. The second
+ * pass exists to clear what accumulated since, and shares the first's implementation because the
+ * lock ordering and the status re-check are subtleties worth having once.
+ */
+describe('0006 second pass', () => {
+  it('repairs on the same walk as the first pass rather than restating it', async () => {
+    const { sql, statements, cursors } = createSqlHarness([['row-1'], []])
+
+    await repairUnknownTableRowProvenanceSecondPass.up(sql)
+
+    expect(cursors).toEqual(['', 'row-1'])
+    const lockIndex = statements.findIndex((statement) => statement.includes('FOR UPDATE'))
+    const deleteIndex = statements.findIndex((statement) =>
+      statement.startsWith('DELETE FROM user_table_row_secret_provenance')
+    )
+    expect(deleteIndex).toBeGreaterThan(lockIndex)
+    expect(statements[deleteIndex]).toContain("AND status = 'unknown'")
+  })
+
+  it('is a distinct entry so a repair that ran twice is recorded twice', () => {
+    expect(repairUnknownTableRowProvenanceSecondPass.name).not.toBe(
+      repairUnknownTableRowProvenance.name
+    )
+  })
+})

packages/db/script-migrations/0005_repair_unknown_table_row_provenance.ts

@@ -107,26 +107,28 @@ async function repairUnknownProvenancePage(
  * past the page instead makes each pass finite and the whole walk terminate on the only condition
  * that means finished: a page with no candidates left in it.
  */
+export async function runUnknownTableRowProvenanceRepair(sql: Sql): Promise<void> {
+  let repaired = 0
+  let skipped = 0
+  let afterRowId = ''
+  for (;;) {
+    const page = await repairUnknownProvenancePage(
+      sql,
+      UNKNOWN_PROVENANCE_REPAIR_BATCH_SIZE,
+      afterRowId
+    )
+    if (page.candidates === 0 || page.lastRowId === null) break
+    repaired += page.repaired
+    skipped += page.candidates - page.repaired
+    afterRowId = page.lastRowId
+    console.log(`  repaired ${repaired} unknown table row(s)`)
+  }
+  console.log(
+    `Unknown table row provenance repair complete: ${repaired} row(s) repaired, ${skipped} left to a concurrent writer.`
+  )
+}
+
 export const repairUnknownTableRowProvenance: ScriptMigration = {
   name: '0005_repair_unknown_table_row_provenance',
-  async up(sql: Sql): Promise<void> {
-    let repaired = 0
-    let skipped = 0
-    let afterRowId = ''
-    for (;;) {
-      const page = await repairUnknownProvenancePage(
-        sql,
-        UNKNOWN_PROVENANCE_REPAIR_BATCH_SIZE,
-        afterRowId
-      )
-      if (page.candidates === 0 || page.lastRowId === null) break
-      repaired += page.repaired
-      skipped += page.candidates - page.repaired
-      afterRowId = page.lastRowId
-      console.log(`  repaired ${repaired} unknown table row(s)`)
-    }
-    console.log(
-      `Unknown table row provenance repair complete: ${repaired} row(s) repaired, ${skipped} left to a concurrent writer.`
-    )
-  },
+  up: runUnknownTableRowProvenanceRepair,
 }

packages/db/script-migrations/0006_repair_unknown_table_row_provenance_second_pass.ts

@@ -0,0 +1,38 @@
+import type { Sql } from 'postgres'
+import { runUnknownTableRowProvenanceRepair } from './0005_repair_unknown_table_row_provenance'
+import type { ScriptMigration } from './types'
+
+/**
+ * Clears the rows that went `unknown` after 0005 had already run.
+ *
+ * 0005 was correct and is finished: the runner records a name in `script_migrations` and never
+ * offers it again, which is exactly the contract a run-once repair wants. But it cleared the
+ * backlog that existed at the instant it ran, and the writers that produced that backlog kept
+ * running afterwards — a table write whose block could not project one input latched the run's
+ * registry, so its rows were stored unrecorded, for as long as that bug was live.
+ *
+ * Nothing heals such a row in place; a partial cell update keeps it unknown and only a full replace
+ * carrying complete provenance clears it. So each one goes on reporting on every later read, which
+ * is why a few dozen rows account for thousands of log lines a week. A second pass is the whole
+ * remedy.
+ *
+ * A new entry rather than deleting 0005's tracking row: the registry is append-only, and a repair
+ * that ran twice should say so twice.
+ *
+ * It assumes the writer that produced these is already live-fixed, and that is a deploy-ordering
+ * requirement rather than something this file can enforce. `promote-images` needs `migrate`, so a
+ * script migration runs while the previous image is still serving: any row an old instance creates
+ * between this walk and the end of the rollout is behind the cursor, and the name is recorded on
+ * success, so it is never offered again. Widening the walk would not help — it is the minutes after
+ * it returns that are exposed, not the milliseconds during. Ship this in a release *after* the
+ * writer fix is already promoted, and the window closes because nothing is producing rows to miss.
+ *
+ * Shares 0005's implementation rather than restating it. The walk locks the parent row before the
+ * sidecar to match the application writer's order, and re-checks `status` under that lock so a
+ * concurrently committed exact sidecar is never deleted — subtleties worth having once, not twice.
+ * Idempotent, so it costs one empty query when there is nothing left to repair.
+ */
+export const repairUnknownTableRowProvenanceSecondPass: ScriptMigration = {
+  name: '0006_repair_unknown_table_row_provenance_second_pass',
+  up: (sql: Sql) => runUnknownTableRowProvenanceRepair(sql),
+}

packages/db/script-migrations/index.ts

@@ -4,6 +4,7 @@ import { backfillPausedBillingAttribution } from './0002_backfill_paused_billing
 import { backfillWorkspaceStorageUsage } from './0003_backfill_workspace_storage_usage'
 import { backfillForkKnowledgeBaseFileOwnership } from './0004_backfill_fork_kb_file_ownership'
 import { repairUnknownTableRowProvenance } from './0005_repair_unknown_table_row_provenance'
+import { repairUnknownTableRowProvenanceSecondPass } from './0006_repair_unknown_table_row_provenance_second_pass'
 import type { ScriptMigration } from './types'
 
 export type { ScriptMigration } from './types'
@@ -19,6 +20,7 @@ export const scriptMigrations: readonly ScriptMigration[] = [
   backfillWorkspaceStorageUsage,
   backfillForkKnowledgeBaseFileOwnership,
   repairUnknownTableRowProvenance,
+  repairUnknownTableRowProvenanceSecondPass,
 ]
 
 /**