diff --git a/src/schema.py b/src/schema.py index deb8658..b16ad39 100644 --- a/src/schema.py +++ b/src/schema.py @@ -702,41 +702,69 @@ def migrate_add_reviewing_planner_output_status(conn): ALREADY-POPULATED database -- same create-copy-drop-rename dance as migrate_expand_artifact_types/migrate_add_loop_back_event_type. Idempotent and safe to call unconditionally, including against a freshly - create_schema()'d database (which already has the expanded constraint).""" + create_schema()'d database (which already has the expanded constraint). + + **2026-08-12 real-restart incident fix**: `task` is a PARENT table -- + work_graph_run/artifact/execution_event/bug_record all REFERENCES + task(task_id) -- unlike execution_event (a leaf table nothing points + at), which is why migrate_add_planner_review_verdict_event_type doesn't + need this. Rebuilding a referenced parent table with + `PRAGMA foreign_keys=ON` (schema.connect()'s default -- see its own + docstring) trips FK enforcement on the DROP TABLE step the instant any + child row exists, even though the surviving task_id column's type/PK + are unchanged. Caught for real restarting the live production + backend_server against ~/.graphlemon/graphlemon.db (months of real + work_graph_run/artifact/execution_event history) -- every unit test for + this migration had passed because none of them seeded a single + FK-referencing child row, so DROP TABLE never actually exercised FK + enforcement in-test. Now follows the exact + migrate_relax_org_node_constraints() precedent (that function's own + docstring explains the same PRAGMA foreign_keys=OFF/rebuild/ + foreign_key_check/ON dance in more depth) instead of the leaf-table + executescript() shortcut this originally (wrongly) copied.""" row = conn.execute("SELECT sql FROM sqlite_master WHERE type='table' AND name='task'").fetchone() if row is None or "'reviewing_planner_output'" in row[0]: return - conn.executescript( - """ - CREATE TABLE task_new ( - task_id TEXT PRIMARY KEY, - project_id TEXT NOT NULL REFERENCES project(project_id), - goal_text TEXT NOT NULL, - task_type TEXT NOT NULL CHECK (task_type IN ('goal', 'bug_fix')), - origin_ref TEXT, - created_via TEXT NOT NULL DEFAULT 'api' CHECK (created_via IN ('api', 'human_console')), - status TEXT NOT NULL CHECK (status IN ( - 'queued', 'planning_brd_prd_sdd', 'reviewing_planner_output', 'planning_failed', - 'coding', 'coding_failed', 'validating', - 'routed_fast_lane', 'routed_full_replan', - 'blocked_pending_human', 'done' - )), - route TEXT CHECK (route IN ('fast_lane', 'full_replan')), - loop_round INTEGER NOT NULL DEFAULT 0, - blocked_reason TEXT, - assigned_org_node TEXT NOT NULL CHECK (assigned_org_node != ''), - created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, - updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP - ); - INSERT INTO task_new SELECT * FROM task; - DROP TABLE task; - ALTER TABLE task_new RENAME TO task; - CREATE INDEX idx_task_status ON task(status); - CREATE INDEX idx_task_assigned_org_node ON task(assigned_org_node); - CREATE INDEX idx_task_project_id ON task(project_id); - """ - ) - conn.commit() + conn.execute("PRAGMA foreign_keys = OFF") + try: + conn.executescript( + """ + CREATE TABLE task_new ( + task_id TEXT PRIMARY KEY, + project_id TEXT NOT NULL REFERENCES project(project_id), + goal_text TEXT NOT NULL, + task_type TEXT NOT NULL CHECK (task_type IN ('goal', 'bug_fix')), + origin_ref TEXT, + created_via TEXT NOT NULL DEFAULT 'api' CHECK (created_via IN ('api', 'human_console')), + status TEXT NOT NULL CHECK (status IN ( + 'queued', 'planning_brd_prd_sdd', 'reviewing_planner_output', 'planning_failed', + 'coding', 'coding_failed', 'validating', + 'routed_fast_lane', 'routed_full_replan', + 'blocked_pending_human', 'done' + )), + route TEXT CHECK (route IN ('fast_lane', 'full_replan')), + loop_round INTEGER NOT NULL DEFAULT 0, + blocked_reason TEXT, + assigned_org_node TEXT NOT NULL CHECK (assigned_org_node != ''), + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP + ); + INSERT INTO task_new SELECT * FROM task; + DROP TABLE task; + ALTER TABLE task_new RENAME TO task; + CREATE INDEX idx_task_status ON task(status); + CREATE INDEX idx_task_assigned_org_node ON task(assigned_org_node); + CREATE INDEX idx_task_project_id ON task(project_id); + """ + ) + conn.commit() + violations = conn.execute("PRAGMA foreign_key_check").fetchall() + if violations: + raise RuntimeError( + "migrate_add_reviewing_planner_output_status left dangling foreign keys: {}".format(violations) + ) + finally: + conn.execute("PRAGMA foreign_keys = ON") def migrate_add_planner_review_verdict_event_type(conn): diff --git a/src/test_e2e_pipeline_e2e.py b/src/test_e2e_pipeline_e2e.py index 2d31556..a5d4dd8 100644 --- a/src/test_e2e_pipeline_e2e.py +++ b/src/test_e2e_pipeline_e2e.py @@ -93,5 +93,63 @@ class RealPrdConsistencyCheckTest(unittest.TestCase): self.assertTrue(ok, "false positive on a consistent PRD: {}".format(issues)) +@skip_unless_env("RUN_REAL_AGENT_E2E") +class RealPlannerReviewTest(unittest.TestCase): + """Org Graph unification Phase 1, real coverage (2026-08-08): the new + planner_reviewer role is a genuinely new LLM-judgment capability (not a + wiring change) -- validated for real once each way (real narrative-text + repro / real structured content), same discipline as + RealPrdConsistencyCheckTest above. Also the cheapest real check that the + new role actually deploys correctly (.claude/agents/planner_reviewer.md + is discoverable and loadable by a real `claude --agent planner_reviewer` + call) -- if role.json/SOUL.md/AGENTS.md have a real wiring mistake, this + is where it surfaces, not deep inside a full run_end_to_end() call.""" + + def test_detects_real_narrative_text_instead_of_structured_content(self): + with tempfile.TemporaryDirectory() as tmp: + subprocess.run(["git", "init"], cwd=tmp, check=True, capture_output=True) + agent_profile.deploy_agent_definitions(tmp) + conn = schema.connect(":memory:") + schema.create_schema(conn) + project_scaffold.create_project(conn, tmp, init_git_if_missing=False) + project_id = conn.execute("SELECT project_id FROM project").fetchone()[0] + task_id = task_create.create_task(conn, project_id, "实现贪吃蛇游戏")["task_id"] + + # real repro text from docs/todo/validator-test-script-narrative- + # output-issue.md, fed in as the SDD content. + narrative_sdd = ( + "Launched the 'graphlemon-node-f9cefb87af4c492aac738b9736d41e4e' workflow " + "(Work Node validator) in the background, run ID wf_93345ca8-8c6. It's running " + "asynchronously — I'll report results once it completes; no further action is " + "needed right now." + ) + result = e2e_pipeline.review_planner_output( + conn, task_id, "实现贪吃蛇游戏", tmp, sdd_content=narrative_sdd, + ) + self.assertIn(result["verdict"], ("FAIL", "INCONCLUSIVE")) + + def test_passes_real_structured_content(self): + with tempfile.TemporaryDirectory() as tmp: + subprocess.run(["git", "init"], cwd=tmp, check=True, capture_output=True) + agent_profile.deploy_agent_definitions(tmp) + conn = schema.connect(":memory:") + schema.create_schema(conn) + project_scaffold.create_project(conn, tmp, init_git_if_missing=False) + project_id = conn.execute("SELECT project_id FROM project").fetchone()[0] + task_id = task_create.create_task(conn, project_id, "实现一个返回两数之和的函数")["task_id"] + + real_sdd = ( + "# SDD: add(a, b)\n\n" + "## Module\n\n`add(a: int, b: int) -> int` in `math_utils.py`.\n\n" + "## Behavior\n\nReturns the sum of the two arguments. No validation needed -- " + "both arguments are guaranteed to be integers by the caller.\n\n" + "## Tests\n\n`test_add_positive`, `test_add_negative`, `test_add_zero`.\n" + ) + result = e2e_pipeline.review_planner_output( + conn, task_id, "实现一个返回两数之和的函数", tmp, sdd_content=real_sdd, + ) + self.assertEqual(result["verdict"], "PASS", result["raw_response"]) + + if __name__ == "__main__": unittest.main() diff --git a/src/test_schema.py b/src/test_schema.py index 1aee7b4..271faad 100644 --- a/src/test_schema.py +++ b/src/test_schema.py @@ -871,6 +871,31 @@ class MigrateAddReviewingPlannerOutputStatusTest(unittest.TestCase): "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " "VALUES ('task-1', 'proj-1', 'implement snake game', 'goal', 'queued', 'planner')" ) + # 2026-08-12 real-restart incident regression: task is a PARENT table -- + # every real production database has months of child rows across these + # four tables REFERENCES task(task_id). Seeding at least one of each here + # is the actual fix for why the original migration's bug (DROP TABLE task + # under PRAGMA foreign_keys=ON trips FK enforcement the instant a + # referencing child row exists) went undetected by this test class -- + # every test below previously ran against a task row with ZERO + # referencing children, so FK enforcement was never truly exercised. + self.conn.execute( + "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) " + "VALUES ('run-1', 'task-1', 'planner', '{}', 'done')" + ) + self.conn.execute( + "INSERT INTO artifact (artifact_id, artifact_type, task_id, produced_by_run_id, " + "file_path, git_commit_sha, validation_status) " + "VALUES ('art-1', 'sdd', 'task-1', 'run-1', 'f.md', 'sha', 'passed')" + ) + self.conn.execute( + "INSERT INTO execution_event (event_id, project_id, task_id, event_type, payload) " + "VALUES ('evt-1', 'proj-1', 'task-1', 'task_status_changed', '{}')" + ) + self.conn.execute( + "INSERT INTO bug_record (bug_id, task_id, source, description, status) " + "VALUES ('bug-1', 'task-1', 'validator', 'real bug', 'open')" + ) self.conn.commit() def tearDown(self): @@ -882,6 +907,24 @@ class MigrateAddReviewingPlannerOutputStatusTest(unittest.TestCase): "UPDATE task SET status = 'reviewing_planner_output' WHERE task_id = 'task-1'" ) + def test_migration_succeeds_with_real_foreign_key_referencing_child_rows(self): + # the actual regression: must not raise sqlite3.IntegrityError + # (FOREIGN KEY constraint failed) -- this is exactly what happened + # restarting the live production backend_server for real. + schema.migrate_add_reviewing_planner_output_status(self.conn) + for table, id_col, id_val in ( + ("work_graph_run", "run_id", "run-1"), + ("artifact", "artifact_id", "art-1"), + ("execution_event", "event_id", "evt-1"), + ("bug_record", "bug_id", "bug-1"), + ): + row = self.conn.execute( + "SELECT task_id FROM {} WHERE {} = ?".format(table, id_col), (id_val,) + ).fetchone() + self.assertEqual(row[0], "task-1", "{} lost its task_id linkage after migration".format(table)) + violations = self.conn.execute("PRAGMA foreign_key_check").fetchall() + self.assertEqual(violations, []) + def test_migration_preserves_existing_rows_and_accepts_new_status(self): schema.migrate_add_reviewing_planner_output_status(self.conn)