fix: make migrate_drop_work_graph_run_org_node handle real duplicate-active-run data

Real self-hosting DB (2026-08-13) has 10 task_ids with multiple non-terminal work_graph_run rows each (legacy per-org_node-row leftovers from before the merged-graph architecture) -- the migration's final CREATE UNIQUE INDEX ux_work_graph_run_active ON work_graph_run(task_id) crashed against this real data (IntegrityError), and because executescript() commits each DDL statement individually, the crash left the live DB in a partially-migrated state: org_node column dropped, but the index never created, with the old idempotency check (`if "org_node" not in cols: return`) then permanently skipping any further attempt to create it. Adds _reconcile_duplicate_active_work_graph_runs(), which marks all but the most-recently-created non-terminal run per task_id as 'failed' (not 'blocked' -- the index's WHERE clause treats 'blocked' as still-active, which was the first fix attempt and still failed) before the index is created. Recency is ranked by rowid, not run_id: real data has started_at=NULL on every legacy row, so a run_id tiebreak would pick among duplicates by UUID string value, unrelated to actual insertion order. Also splits the idempotency check into column-drop and index-creation halves so a database already in the partially-migrated state (like the live one) gets the missing index created on the next run instead of being skipped. Dry-run verified against a copy of the live 36-row production DB before applying to the real ~/.graphlemon/graphlemon.db (backend_server was down for this, no concurrent writers). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

fix: make migrate_drop_work_graph_run_org_node handle real duplicate-active-run data
Real self-hosting DB (2026-08-13) has 10 task_ids with multiple non-terminal work_graph_run rows each (legacy per-org_node-row leftovers from before the merged-graph architecture) -- the migration's final CREATE UNIQUE INDEX ux_work_graph_run_active ON work_graph_run(task_id) crashed against this real data (IntegrityError), and because executescript() commits each DDL statement individually, the crash left the live DB in a partially-migrated state: org_node column dropped, but the index never created, with the old idempotency check (`if "org_node" not in cols: return`) then permanently skipping any further attempt to create it. Adds _reconcile_duplicate_active_work_graph_runs(), which marks all but the most-recently-created non-terminal run per task_id as 'failed' (not 'blocked' -- the index's WHERE clause treats 'blocked' as still-active, which was the first fix attempt and still failed) before the index is created. Recency is ranked by rowid, not run_id: real data has started_at=NULL on every legacy row, so a run_id tiebreak would pick among duplicates by UUID string value, unrelated to actual insertion order. Also splits the idempotency check into column-drop and index-creation halves so a database already in the partially-migrated state (like the live one) gets the missing index created on the next run instead of being skipped. Dry-run verified against a copy of the live 36-row production DB before applying to the real ~/.graphlemon/graphlemon.db (backend_server was down for this, no concurrent writers). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
bboonnee
1 parent d10a05b0
Showing 2 changed files with 169 additions and 3 deletions
src/schema.py
src/test_schema.py
@@ -847,15 +847,39 @@ def migrate_drop_work_graph_run_org_node(conn):
     (each embeds the full column/constraint set valid as of ITS place in the
     sequence, not retroactively edited by later migrations).
  
-    Idempotent: skipped if work_graph_run no longer has an org_node column,
-    so a fresh create_schema()'d database (already on this final shape) is a
-    no-op, same convention as every other migrate_* function here."""
+    Idempotent, and re-entrant against a database left PARTWAY through a
+    previous, interrupted attempt (real 2026-08-13 self-hosting incident:
+    executescript() does not wrap the whole script in one transaction, so a
+    prior run of this function got as far as dropping the org_node column
+    and renaming the table in, then died on the final CREATE UNIQUE INDEX
+    step -- see the _reconcile_duplicate_active_work_graph_runs() docstring
+    for why that step can fail against real data). Column-drop and index-
+    creation are therefore checked and completed independently rather than
+    as one all-or-nothing gate, so re-running this function against that
+    exact half-migrated state finishes the job instead of silently
+    no-op'ing (the OLD idempotency check here was just `if "org_node" not
+    in cols: return`, which would have skipped forever on that database,
+    permanently leaving ux_work_graph_run_active missing)."""
     migrate_relax_org_node_constraints(conn)
     conn.commit()
  
+    _reconcile_duplicate_active_work_graph_runs(conn)
+    conn.commit()
+
     cols = {row[1] for row in conn.execute("PRAGMA table_info(work_graph_run)").fetchall()}
+    has_index = conn.execute(
+        "SELECT 1 FROM sqlite_master WHERE type = 'index' AND name = 'ux_work_graph_run_active'"
+    ).fetchone() is not None
+
     if "org_node" not in cols:
+        if not has_index:
+            conn.execute(
+                "CREATE UNIQUE INDEX ux_work_graph_run_active ON work_graph_run(task_id) "
+                "WHERE status NOT IN ('done', 'failed')"
+            )
+            conn.commit()
         return
+
     conn.execute("PRAGMA foreign_keys = OFF")
     try:
         conn.executescript(
@@ -897,3 +921,61 @@ def migrate_drop_work_graph_run_org_node(conn):
             )
     finally:
         conn.execute("PRAGMA foreign_keys = ON")
+
+
+def _reconcile_duplicate_active_work_graph_runs(conn):
+    """Real production data (2026-08-13 incident) can have MULTIPLE
+    non-terminal work_graph_run rows for the same task_id -- a legitimate
+    leftover of the pre-Phase-2c architecture where each org_node got its
+    own row instead of sharing one merged run. ux_work_graph_run_active's
+    UNIQUE(task_id) WHERE status NOT IN ('done', 'failed') can't be created
+    while such duplicates exist (10 affected task_ids found against the
+    live self-hosting database, e.g. one task had 8 concurrently-'open'
+    rows from the old per-org_node model).
+
+    Keeps the most-recently-started non-terminal row per task_id as the
+    active one and marks every other non-terminal row for that task_id as
+    'failed' -- 'blocked' was considered but rejected: ux_work_graph_run_active's
+    WHERE clause is `status NOT IN ('done', 'failed')`, so a 'blocked' row
+    still counts as active and would keep tripping the same UNIQUE
+    violation this function exists to prevent (confirmed by re-running this
+    against real duplicate data -- the index creation still failed with
+    'blocked'). 'failed' is the only terminal-compatible status that
+    doesn't misrepresent these rows as successfully completed ('done'
+    would); it's an honest description too, since these are abandoned runs
+    from before the merged-graph architecture -- nothing will ever resume
+    against them directly again (Coder/Validator now only ever append into
+    the newest run for a task). Plain Python grouping rather than a SQL
+    window function, matching this file's existing style (no window
+    functions used elsewhere in this module) and avoiding any SQLite-
+    version assumption.
+
+    Safe to call unconditionally and repeatedly: a database with no
+    duplicates (the common case) does zero UPDATEs.
+
+    Ranks candidates by (started_at, rowid), NOT (started_at, run_id):
+    real data confirmed started_at is NULL on every one of these legacy
+    rows (never backfilled -- they predate this column being populated),
+    which would make a run_id tiebreak pick among them by UUID string
+    value, unrelated to actual chronology. rowid reflects true insertion
+    order (SQLite assigns it monotonically increasing unless the table is
+    declared WITHOUT ROWID, which this one isn't), so it's the only
+    honest proxy available for "most recently created" when started_at
+    can't answer that."""
+    rows = conn.execute(
+        "SELECT rowid, run_id, task_id, started_at FROM work_graph_run "
+        "WHERE status NOT IN ('done', 'failed')"
+    ).fetchall()
+    by_task = {}
+    for rowid, run_id, task_id, started_at in rows:
+        by_task.setdefault(task_id, []).append((started_at or "", rowid, run_id))
+
+    to_block = []
+    for task_id, entries in by_task.items():
+        if len(entries) <= 1:
+            continue
+        entries.sort()
+        to_block.extend(run_id for _, _, run_id in entries[:-1])
+
+    for run_id in to_block:
+        conn.execute("UPDATE work_graph_run SET status = 'failed' WHERE run_id = ?", (run_id,))
@@ -915,6 +915,90 @@ class MigrateDropWorkGraphRunOrgNodeTest(unittest.TestCase):
         finally:
             fresh.close()
  
+    def test_duplicate_non_terminal_rows_per_task_reconciled_before_index(self):
+        # Real 2026-08-13 incident: pre-Phase-2c databases can have several
+        # non-terminal work_graph_run rows for the same task_id (one per
+        # org_node, before runs were merged). The narrowed unique index
+        # can't be created until only one non-terminal row per task_id
+        # remains -- this must not raise.
+        self.conn.execute(
+            "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status, started_at) "
+            "VALUES ('run-2', 'task-1', 'coder', '{}', 'running', '2026-08-01T00:00:00')"
+        )
+        self.conn.execute(
+            "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status, started_at) "
+            "VALUES ('run-3', 'task-1', 'validator', '{}', 'planning', '2026-08-02T00:00:00')"
+        )
+        self.conn.commit()
+
+        schema.migrate_drop_work_graph_run_org_node(self.conn)
+
+        rows = self.conn.execute(
+            "SELECT run_id, status FROM work_graph_run WHERE task_id = 'task-1' AND run_id != 'run-1'"
+        ).fetchall()
+        statuses = dict(rows)
+        # run-3 started latest among the non-terminal duplicates -> stays active.
+        self.assertEqual(statuses["run-3"], "planning")
+        self.assertEqual(statuses["run-2"], "failed")
+        # the narrowed index must have actually been created and be enforced.
+        with self.assertRaises(sqlite3.IntegrityError):
+            self.conn.execute(
+                "INSERT INTO work_graph_run (run_id, task_id, graph_manifest, status) "
+                "VALUES ('run-4', 'task-1', '{}', 'running')"
+            )
+
+    def test_duplicate_reconciliation_uses_insertion_order_not_uuid_when_started_at_null(self):
+        # Real 2026-08-13 incident: every legacy duplicate row had
+        # started_at = NULL. A run_id-string tiebreak would pick among
+        # them by UUID value, unrelated to which one was actually created
+        # last -- e.g. 'aaaa...' sorts after 'zzzz...' alphabetically but
+        # was inserted first. rowid (SQLite's true insertion-order
+        # counter) must be used instead, so the row inserted LAST (run-2,
+        # despite its run_id sorting before run-3's alphabetically) is the
+        # one kept active.
+        self.conn.execute(
+            "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) "
+            "VALUES ('aaa-run-3', 'task-1', 'validator', '{}', 'planning')"
+        )
+        self.conn.execute(
+            "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) "
+            "VALUES ('zzz-run-2', 'task-1', 'coder', '{}', 'running')"
+        )
+        self.conn.commit()
+
+        schema.migrate_drop_work_graph_run_org_node(self.conn)
+
+        statuses = dict(
+            self.conn.execute(
+                "SELECT run_id, status FROM work_graph_run WHERE run_id IN ('aaa-run-3', 'zzz-run-2')"
+            ).fetchall()
+        )
+        self.assertEqual(statuses["aaa-run-3"], "failed")
+        self.assertEqual(statuses["zzz-run-2"], "running")
+
+    def test_reentrant_after_partial_migration_missing_index_only(self):
+        # Real 2026-08-13 incident: executescript() commits each DDL
+        # statement individually, so a process crash between ALTER TABLE
+        # ... RENAME and CREATE UNIQUE INDEX leaves a database with the
+        # org_node column already gone but ux_work_graph_run_active never
+        # created. Re-running the migration against that exact state must
+        # finish the job, not silently no-op because "org_node" is already
+        # absent.
+        schema.migrate_drop_work_graph_run_org_node(self.conn)
+        self.conn.execute("DROP INDEX ux_work_graph_run_active")
+        self.conn.commit()
+        has_index = self.conn.execute(
+            "SELECT 1 FROM sqlite_master WHERE type = 'index' AND name = 'ux_work_graph_run_active'"
+        ).fetchone()
+        self.assertIsNone(has_index)  # sanity: simulated partial state matches the real incident
+
+        schema.migrate_drop_work_graph_run_org_node(self.conn)
+
+        has_index = self.conn.execute(
+            "SELECT 1 FROM sqlite_master WHERE type = 'index' AND name = 'ux_work_graph_run_active'"
+        ).fetchone()
+        self.assertIsNotNone(has_index)
+
  
 class MigrateAddWorkGraphLoopLimitConfigTest(unittest.TestCase):
     """2026-08-08: real upgrade path for an already-populated database that