Commit d173e4c6156f409abe1781b79c24a978d7543ec6
1 parent
bd3956f6
Stop deploy health-check canary probes from leaking project/task rows
check_canary_task() creates a real project + task through the candidate's own HTTP API to verify the write path works, but only ever cleaned up its throwaway git directory -- the DB rows it created via the API were never removed. Every real health check permanently grew whatever DB the candidate was using by one dead project (pointing at an already-deleted temp dir) and one task stuck at 'queued' forever. Found by inspecting the real ~/.graphlemon/graphlemon.db, which had 5 such leaked 'deploy-canary' projects from earlier real health-check runs (now archived by hand). Fix: archive the created project in a finally block that covers every return path, including task-creation/verification failures where only the project (not the task) got created. Best-effort -- the archive call failing must not override the real check result. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Showing
3 changed files
with
73 additions
and
1 deletions
docs/implementation-gap-analysis.md
| ... | ... | @@ -89,6 +89,18 @@ |
| 89 | 89 | 题,但这是个反复出现、被如实记录但从未补上的验证缺口。 |
| 90 | 90 | - `supervisor-auto-rollback-design.md` 里健康检查超时、监控窗口时长等参数明确标注"工程经验值,需要实测校 |
| 91 | 91 | 准",还没有真实生产数据校准过(PH5 已知交叉缺口第 4 条,`development-plan.md` 第 422-427 行)。 |
| 92 | +- **✅ 已修复(2026-08-03)健康检查 canary 探测永久泄漏 DB 记录**:`supervisor/deploy_health_check.py` 的 | |
| 93 | + `check_canary_task()` 每次健康检查都会真实调候选进程的 `POST /api/projects`+`POST /api/tasks` 验证 HTTP+DB | |
| 94 | + 写路径,但只清理了临时 git 目录,从未清理通过 API 真实创建的 project/task 行——**每跑一次真实健康检查, | |
| 95 | + 候选进程当时用的数据库里就永久多两条垃圾记录**。真实发现:在真实的 `~/.graphlemon/graphlemon.db` 里发现 5 | |
| 96 | + 个 `deploy-canary` 项目,全部指向早已被系统清掉的 `/var/folders/.../deploy-canary-*` 临时目录,任务永远卡 | |
| 97 | + 在 `queued`,是之前某次真实健康检查验证(08-03 凌晨 05:50 那几次)留下的。已手动归档这 5 条历史记录 | |
| 98 | + (`PUT /api/projects/:id {status:'archived'}`,跟控制台"归档"按钮走的是同一条路径——没用硬删除,因为 | |
| 99 | + `task`/`artifact` 表有外键引用 `project_id`,硬删会破坏历史)。**修复**:`check_canary_task()` 的 `finally` | |
| 100 | + 块里新增:只要 project 行被真实创建过(不管后续 task 创建/校验成功还是失败),就调用同一条归档 API 清理掉 | |
| 101 | + 自己建的 project(最佳努力——归档调用本身失败不覆盖真实检查结果,避免把"没能清理自己"误判成"健康检查失 | |
| 102 | + 败")。新增 2 个回归测试:一次成功的 canary 检查后 project 应变成 `archived`;task 创建失败时 project 也 | |
| 103 | + 不应该泄漏。`supervisor/` 全量 125 个测试 + `src/` 513 个测试全绿。 | |
| 92 | 104 | |
| 93 | 105 | --- |
| 94 | 106 | ... | ... |
supervisor/deploy_health_check.py
| ... | ... | @@ -105,6 +105,16 @@ def _get_json(port, path, timeout): |
| 105 | 105 | return resp.status, json.loads(resp.read()) |
| 106 | 106 | |
| 107 | 107 | |
| 108 | +def _put_json(port, path, payload, timeout): | |
| 109 | + body = json.dumps(payload).encode("utf-8") | |
| 110 | + req = urllib.request.Request( | |
| 111 | + "http://127.0.0.1:{}{}".format(port, path), data=body, | |
| 112 | + headers={"Content-Type": "application/json"}, method="PUT", | |
| 113 | + ) | |
| 114 | + with urllib.request.urlopen(req, timeout=timeout) as resp: | |
| 115 | + return resp.status, json.loads(resp.read()) | |
| 116 | + | |
| 117 | + | |
| 108 | 118 | def check_canary_task(port, timeout=HEALTH_CHECK_CANARY_TIMEOUT): |
| 109 | 119 | """"结果可预期的最小任务" (design doc §6): a trivial project + task |
| 110 | 120 | 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): |
| 112 | 122 | `status='queued'` synchronously, which is exactly the kind of |
| 113 | 123 | deterministic, always-should-pass result a canary needs; the point is |
| 114 | 124 | proving the candidate's HTTP+DB write path genuinely works, not |
| 115 | - exercising the full pipeline.""" | |
| 125 | + exercising the full pipeline. | |
| 126 | + | |
| 127 | + 2026-08-03 real leak found via manual inspection of the real | |
| 128 | + ~/.graphlemon/graphlemon.db (5 accumulated rows from earlier real | |
| 129 | + health-check runs): `canary_dir` (the throwaway git repo) was always | |
| 130 | + cleaned up, but the `project` row this function creates through the | |
| 131 | + candidate's own HTTP API never was -- every real health check | |
| 132 | + permanently grew whatever DB the candidate was using by one dead | |
| 133 | + project (target directory long gone) with one task stuck at 'queued' | |
| 134 | + forever. Archiving the project (same PUT the console's own project-list | |
| 135 | + "归档" button uses -- a real DELETE isn't safe here, task/artifact rows | |
| 136 | + still reference project_id) in `finally` covers every return path, | |
| 137 | + including the failure branches where only the project (not the task) | |
| 138 | + got created. Best-effort: if the archive call itself fails, that must | |
| 139 | + not override the real check result -- a canary whose only problem was | |
| 140 | + "couldn't clean up after itself" would be a confusing false negative.""" | |
| 116 | 141 | canary_dir = tempfile.mkdtemp(prefix="deploy-canary-") |
| 142 | + project = None | |
| 117 | 143 | try: |
| 118 | 144 | subprocess.run(["git", "init", "-q"], cwd=canary_dir, check=True, capture_output=True) |
| 119 | 145 | subprocess.run( |
| ... | ... | @@ -150,6 +176,11 @@ def check_canary_task(port, timeout=HEALTH_CHECK_CANARY_TIMEOUT): |
| 150 | 176 | return {"signal_type": "canary_task", "status": "fail", "detail": str(e)} |
| 151 | 177 | finally: |
| 152 | 178 | shutil.rmtree(canary_dir, ignore_errors=True) |
| 179 | + if project is not None: | |
| 180 | + try: | |
| 181 | + _put_json(port, "/api/projects/" + project["project_id"], {"status": "archived"}, timeout) | |
| 182 | + except (urllib.error.URLError, urllib.error.HTTPError, OSError): | |
| 183 | + pass | |
| 153 | 184 | |
| 154 | 185 | |
| 155 | 186 | def _record_signal(conn, deployment_id, signal): | ... | ... |
supervisor/test_deploy_health_check.py
| ... | ... | @@ -127,6 +127,35 @@ class CheckCanaryTaskTest(RealServerTestBase): |
| 127 | 127 | count = conn.execute("SELECT COUNT(*) FROM task WHERE goal_text LIKE 'deploy-supervisor canary%'").fetchone()[0] |
| 128 | 128 | self.assertEqual(count, 1) |
| 129 | 129 | |
| 130 | + def test_canary_project_is_archived_after_a_passing_check(self): | |
| 131 | + # 2026-08-03 regression: found via 5 real leaked 'deploy-canary' | |
| 132 | + # rows sitting permanently active in the real ~/.graphlemon/ | |
| 133 | + # graphlemon.db -- every real health check used to grow the | |
| 134 | + # candidate's DB forever. The task itself is left alone (asserted | |
| 135 | + # above), only the project should end up archived. | |
| 136 | + dhc.check_canary_task(self.port) | |
| 137 | + conn = self.server.conn | |
| 138 | + row = conn.execute("SELECT status FROM project WHERE name = 'deploy-canary'").fetchone() | |
| 139 | + self.assertEqual(row[0], "archived") | |
| 140 | + | |
| 141 | + def test_canary_project_is_archived_even_when_task_creation_fails(self): | |
| 142 | + # The project is created before the task -- a failure on the task | |
| 143 | + # step must not leak the project either. | |
| 144 | + real_post_json = dhc._post_json | |
| 145 | + | |
| 146 | + def fake_post_json(port, path, payload, timeout): | |
| 147 | + if path == "/api/tasks": | |
| 148 | + return 500, {"error": "boom"} | |
| 149 | + return real_post_json(port, path, payload, timeout) | |
| 150 | + | |
| 151 | + with patch.object(dhc, "_post_json", side_effect=fake_post_json): | |
| 152 | + signal = dhc.check_canary_task(self.port) | |
| 153 | + | |
| 154 | + self.assertEqual(signal["status"], "fail") | |
| 155 | + conn = self.server.conn | |
| 156 | + row = conn.execute("SELECT status FROM project WHERE name = 'deploy-canary'").fetchone() | |
| 157 | + self.assertEqual(row[0], "archived") | |
| 158 | + | |
| 130 | 159 | def test_nothing_listening_returns_fail(self): |
| 131 | 160 | signal = dhc.check_canary_task(1) |
| 132 | 161 | self.assertEqual(signal["signal_type"], "canary_task") | ... | ... |