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

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,
 ]
 
 /**