From d173e4c6156f409abe1781b79c24a978d7543ec6 Mon Sep 17 00:00:00 2001 From: bboonnee Date: Tue, 4 Aug 2026 00:04:02 +0800 Subject: [PATCH] Stop deploy health-check canary probes from leaking project/task rows --- docs/implementation-gap-analysis.md | 12 ++++++++++++ supervisor/deploy_health_check.py | 33 ++++++++++++++++++++++++++++++++- supervisor/test_deploy_health_check.py | 29 +++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/docs/implementation-gap-analysis.md b/docs/implementation-gap-analysis.md index 5469672..c625a1c 100644 --- a/docs/implementation-gap-analysis.md +++ b/docs/implementation-gap-analysis.md @@ -89,6 +89,18 @@ 题，但这是个反复出现、被如实记录但从未补上的验证缺口。 - `supervisor-auto-rollback-design.md` 里健康检查超时、监控窗口时长等参数明确标注"工程经验值，需要实测校 准"，还没有真实生产数据校准过（PH5 已知交叉缺口第 4 条，`development-plan.md` 第 422-427 行）。 +- **✅ 已修复（2026-08-03）健康检查 canary 探测永久泄漏 DB 记录**：`supervisor/deploy_health_check.py` 的 + `check_canary_task()` 每次健康检查都会真实调候选进程的 `POST /api/projects`+`POST /api/tasks` 验证 HTTP+DB + 写路径，但只清理了临时 git 目录，从未清理通过 API 真实创建的 project/task 行——**每跑一次真实健康检查， + 候选进程当时用的数据库里就永久多两条垃圾记录**。真实发现：在真实的 `~/.graphlemon/graphlemon.db` 里发现 5 + 个 `deploy-canary` 项目，全部指向早已被系统清掉的 `/var/folders/.../deploy-canary-*` 临时目录，任务永远卡 + 在 `queued`，是之前某次真实健康检查验证（08-03 凌晨 05:50 那几次）留下的。已手动归档这 5 条历史记录 + （`PUT /api/projects/:id {status:'archived'}`，跟控制台"归档"按钮走的是同一条路径——没用硬删除，因为 + `task`/`artifact` 表有外键引用 `project_id`，硬删会破坏历史）。**修复**：`check_canary_task()` 的 `finally` + 块里新增：只要 project 行被真实创建过（不管后续 task 创建/校验成功还是失败），就调用同一条归档 API 清理掉 + 自己建的 project（最佳努力——归档调用本身失败不覆盖真实检查结果，避免把"没能清理自己"误判成"健康检查失 + 败"）。新增 2 个回归测试：一次成功的 canary 检查后 project 应变成 `archived`；task 创建失败时 project 也 + 不应该泄漏。`supervisor/` 全量 125 个测试 + `src/` 513 个测试全绿。 --- diff --git a/supervisor/deploy_health_check.py b/supervisor/deploy_health_check.py index 132d45c..ce02cf1 100644 --- a/supervisor/deploy_health_check.py +++ b/supervisor/deploy_health_check.py @@ -105,6 +105,16 @@ def _get_json(port, path, timeout): return resp.status, json.loads(resp.read()) +def _put_json(port, path, payload, timeout): + body = json.dumps(payload).encode("utf-8") + req = urllib.request.Request( + "http://127.0.0.1:{}{}".format(port, path), data=body, + headers={"Content-Type": "application/json"}, method="PUT", + ) + with urllib.request.urlopen(req, timeout=timeout) as resp: + return resp.status, json.loads(resp.read()) + + def check_canary_task(port, timeout=HEALTH_CHECK_CANARY_TIMEOUT): """"结果可预期的最小任务" (design doc §6): a trivial project + task creation round-trip. Doesn't run a real Org Graph (no real `claude` @@ -112,8 +122,24 @@ def check_canary_task(port, timeout=HEALTH_CHECK_CANARY_TIMEOUT): `status='queued'` synchronously, which is exactly the kind of deterministic, always-should-pass result a canary needs; the point is proving the candidate's HTTP+DB write path genuinely works, not - exercising the full pipeline.""" + exercising the full pipeline. + + 2026-08-03 real leak found via manual inspection of the real + ~/.graphlemon/graphlemon.db (5 accumulated rows from earlier real + health-check runs): `canary_dir` (the throwaway git repo) was always + cleaned up, but the `project` row this function creates through the + candidate's own HTTP API never was -- every real health check + permanently grew whatever DB the candidate was using by one dead + project (target directory long gone) with one task stuck at 'queued' + forever. Archiving the project (same PUT the console's own project-list + "归档" button uses -- a real DELETE isn't safe here, task/artifact rows + still reference project_id) in `finally` covers every return path, + including the failure branches where only the project (not the task) + got created. Best-effort: if the archive call itself fails, that must + not override the real check result -- a canary whose only problem was + "couldn't clean up after itself" would be a confusing false negative.""" canary_dir = tempfile.mkdtemp(prefix="deploy-canary-") + project = None try: subprocess.run(["git", "init", "-q"], cwd=canary_dir, check=True, capture_output=True) subprocess.run( @@ -150,6 +176,11 @@ def check_canary_task(port, timeout=HEALTH_CHECK_CANARY_TIMEOUT): return {"signal_type": "canary_task", "status": "fail", "detail": str(e)} finally: shutil.rmtree(canary_dir, ignore_errors=True) + if project is not None: + try: + _put_json(port, "/api/projects/" + project["project_id"], {"status": "archived"}, timeout) + except (urllib.error.URLError, urllib.error.HTTPError, OSError): + pass def _record_signal(conn, deployment_id, signal): diff --git a/supervisor/test_deploy_health_check.py b/supervisor/test_deploy_health_check.py index 6164bec..000b54f 100644 --- a/supervisor/test_deploy_health_check.py +++ b/supervisor/test_deploy_health_check.py @@ -127,6 +127,35 @@ class CheckCanaryTaskTest(RealServerTestBase): count = conn.execute("SELECT COUNT(*) FROM task WHERE goal_text LIKE 'deploy-supervisor canary%'").fetchone()[0] self.assertEqual(count, 1) + def test_canary_project_is_archived_after_a_passing_check(self): + # 2026-08-03 regression: found via 5 real leaked 'deploy-canary' + # rows sitting permanently active in the real ~/.graphlemon/ + # graphlemon.db -- every real health check used to grow the + # candidate's DB forever. The task itself is left alone (asserted + # above), only the project should end up archived. + dhc.check_canary_task(self.port) + conn = self.server.conn + row = conn.execute("SELECT status FROM project WHERE name = 'deploy-canary'").fetchone() + self.assertEqual(row[0], "archived") + + def test_canary_project_is_archived_even_when_task_creation_fails(self): + # The project is created before the task -- a failure on the task + # step must not leak the project either. + real_post_json = dhc._post_json + + def fake_post_json(port, path, payload, timeout): + if path == "/api/tasks": + return 500, {"error": "boom"} + return real_post_json(port, path, payload, timeout) + + with patch.object(dhc, "_post_json", side_effect=fake_post_json): + signal = dhc.check_canary_task(self.port) + + self.assertEqual(signal["status"], "fail") + conn = self.server.conn + row = conn.execute("SELECT status FROM project WHERE name = 'deploy-canary'").fetchone() + self.assertEqual(row[0], "archived") + def test_nothing_listening_returns_fail(self): signal = dhc.check_canary_task(1) self.assertEqual(signal["signal_type"], "canary_task") -- libgit2 0.21.4