From 0b96b2929cfe9258d4fef040fd4fd9cc908addee Mon Sep 17 00:00:00 2001 From: host Date: Mon, 26 Oct 2026 15:20:02 +0800 Subject: [PATCH] pr: commit worktree changes before push; fix merge/pr order docs POST /tasks/{id}/pr used to push the branch as-is, so uncommitted worktree changes never reached the PR (empty diff). Now, after all skip checks pass, the endpoint commits the worktree on host/ (guarded: refuses when the worktree is on another branch), cleans __pycache__/*.pyc like host.py merge does, and falls back to the cloud identity when no git identity is configured. Nothing-to-commit proceeds. Docs corrected: PR comes after status-merge and before any local git merge (which deletes branch+worktree). --- cloud/README.md | 13 ++- cloud/pr.py | 86 +++++++++++++- cloud/server.py | 10 +- cloud/tests/test_cloud_dispatch_pr.py | 6 + cloud/tests/test_pr_commit.py | 158 ++++++++++++++++++++++++++ 5 files changed, 263 insertions(+), 10 deletions(-) create mode 100644 cloud/tests/test_pr_commit.py diff --git a/cloud/README.md b/cloud/README.md index 8797beb..cf78d54 100644 --- a/cloud/README.md +++ b/cloud/README.md @@ -758,10 +758,17 @@ interrupted 且终态任务 cancel 返回 400、心跳后 sweep 不扫刚心跳 reason=no_worktree；worktree 里没有 git remote origin 时 reason=no_remote；本机没有 gh 二进制时 reason=no_gh；gh 未登录 （gh auth status 非零）时 reason=gh_not_authenticated；以上全过 - 才 push 分支 + gh pr create，成功后把 pr_url/pr_number 写回任务 + 才先把 worktree 改动提交到分支上（worktree 必须正好在 + host/ 上，否则直接失败；nothing to commit 时跳过； + 缺 git 身份时回退 cloud/cloud@local 再试），再 push 分支 + + gh pr create，成功后把 pr_url/pr_number 写回任务 记录并记审计 pr_created，返回 200 含 pr_url。push 或 pr create - 失败时返回 502 且错误文本写清是哪一步（push 还是 pr_create）， - 任务状态保持 merged 不变，绝不把任务标 failed。worker 不自动推： + 失败时返回 502 且错误文本写清是哪一步（commit、push 还是 + pr_create）， + 任务状态保持 merged 不变，绝不把任务标 failed。顺序：任务 + completed -> POST /tasks/{id}/merge 改状态 -> POST + /tasks/{id}/pr，本地 host.py merge 会删分支和 worktree、须在 + PR 之后做，此后调 PR 会跳过 no_worktree。worker 不自动推： 自动 PR 只由显式调本端点触发，默认自动推会把测试任务也推到远端。 projects 表新增 `auto_pr` 列（默认 0，老库幂等补列）； `POST /projects` 接受可选 auto_pr（只接受 0/1，非法值 400）； diff --git a/cloud/pr.py b/cloud/pr.py index d5566d6..8b229ed 100644 --- a/cloud/pr.py +++ b/cloud/pr.py @@ -13,9 +13,23 @@ host/，worktree 在 <仓库根>/work/host-worktrees/ 4. 本机没有 gh 二进制 -> pr_skipped, no_gh 5. gh 未登录（gh auth status 非零） -> pr_skipped, gh_not_authenticated - 只有以上全过才 push 分支 + gh pr create；push 或 pr create - 失败时抛 PrStepError（调用方据此返回 502），任务状态保持 - merged 不变，绝不把任务标 failed。 + 只有以上全过才提交 worktree 改动、push 分支 + gh pr create； + push 或 pr create 失败时抛 PrStepError（调用方据此返回 502）， + 任务状态保持 merged 不变，绝不把任务标 failed。 + + 提交规则（create_task_pr 在全部跳过检查通过后、push 之前做）： + worktree 必须正好在分支 host/<任务名> 上，否则直接失败， + 绝不在别的分支上提交；先清掉 worktree 内的 __pycache__ / + *.pyc（与 host.py merge 同规则，只动 worktree，不动主仓）， + 再 git add -A + commit；nothing to commit 时直接继续。 + 身份用环境里的 git 身份（与 host.py merge 一致），缺身份时 + 回退到 cloud/cloud@local 再试一次。 + + 顺序约束（与云端 merge 接口配套）：任务 completed -> 调 + POST /tasks/{id}/merge 改状态 -> 调 POST /tasks/{id}/pr 推分支 + 建 PR -> 再在 GitHub 上合 PR 或本地 host.py merge。本地 + host.py merge 成功后会删分支和 worktree，此后调 PR 会跳过 + no_worktree，所以 PR 必须在本地 git merge 之前做。 worker 不自动推：自动 PR 只由显式调 POST /tasks/{id}/pr 触发。默认自动推会把测试任务也推到远端，本模块不做。 @@ -118,6 +132,68 @@ def check_pr_ready(worktree_dir, git_runner=None, gh_which=None, return True +def _commit_worktree_changes(worktree_dir, branch, task_id, + git_runner=None): + """push 之前把 worktree 的改动提交到分支上。 + + 顺序：先确认 worktree 正好在目标分支上（不在就直接失败， + 绝不在别的分支上提交，commit 到 main 里就污染主分支了）； + 再清掉 worktree 内的 __pycache__ / *.pyc（可再生的缓存， + 与 host.py merge 同规则）；再 git add -A + commit。 + nothing to commit 时直接返回；缺 git 身份时回退到 + cloud/cloud@local 再试一次；其它失败抛 PrStepError(commit)。 + """ + cur = _run(["git", "-C", worktree_dir, "branch", "--show-current"], + cwd=worktree_dir, runner=git_runner) + if (getattr(cur, "stdout", "") or "").strip() != branch: + raise PrStepError( + "commit", "worktree 不在分支 %s 上，拒绝提交" % branch) + for root, dirs, files in os.walk(worktree_dir): + if ".git" in dirs: + dirs.remove(".git") + if "__pycache__" in dirs: + shutil.rmtree(os.path.join(root, "__pycache__"), + ignore_errors=True) + dirs.remove("__pycache__") + for fn in files: + if fn.endswith(".pyc"): + try: + os.remove(os.path.join(root, fn)) + except OSError: + pass + add = _run(["git", "-C", worktree_dir, "add", "-A"], + cwd=worktree_dir, runner=git_runner) + if getattr(add, "returncode", 1) != 0: + raise PrStepError( + "commit", "git add 失败：%s" % ( + (getattr(add, "stderr", "") or "") + or (getattr(add, "stdout", "") or ""))) + msg = "cloud: %s" % task_id + c = _run(["git", "-C", worktree_dir, "commit", "-m", msg], + cwd=worktree_dir, runner=git_runner) + if getattr(c, "returncode", 1) == 0: + return + out = ((getattr(c, "stdout", "") or "") + + (getattr(c, "stderr", "") or "")) + if "nothing to commit" in out: + return + if ("tell me who you are" in out or "user.name" in out + or "user.email" in out): + c2 = _run(["git", "-C", worktree_dir, + "-c", "user.name=cloud", "-c", "user.email=cloud@local", + "commit", "-m", msg], + cwd=worktree_dir, runner=git_runner) + if getattr(c2, "returncode", 1) == 0: + return + out2 = ((getattr(c2, "stdout", "") or "") + + (getattr(c2, "stderr", "") or "")) + if "nothing to commit" in out2: + return + raise PrStepError( + "commit", "git commit 失败（已试 cloud 身份）：%s" % out2[:300]) + raise PrStepError("commit", "git commit 失败：%s" % out[:300]) + + def create_task_pr(task_id, auto_pr, worktrees_root=None, git_runner=None, gh_runner=None, gh_which=None): """给一个已 merged 的云端任务建 PR。 @@ -125,6 +201,8 @@ def create_task_pr(task_id, auto_pr, worktrees_root=None, auto_pr 不为 1 时直接抛 PrSkipped(auto_pr_disabled)，调用方 在查 worktree 之前就应先查项目 auto_pr——本函数也再查一次， 防止直接调用者漏查。成功返回 {pr_url, pr_number, branch}。 + 全部跳过检查通过后，先把 worktree 改动提交到分支再 push， + 否则未提交的改动进不了 PR。 """ if auto_pr != 1: raise PrSkipped(_SKIP_AUTO_PR) @@ -134,6 +212,8 @@ def create_task_pr(task_id, auto_pr, worktrees_root=None, branch = "host/" + host_name check_pr_ready(worktree_dir, git_runner=git_runner, gh_which=gh_which, gh_runner=gh_runner) + _commit_worktree_changes(worktree_dir, branch, task_id, + git_runner=git_runner) push = _run(["git", "-C", worktree_dir, "push", "origin", branch], cwd=worktree_dir, runner=git_runner) if getattr(push, "returncode", 1) != 0: diff --git a/cloud/server.py b/cloud/server.py index 83e83ae..8d1556b 100644 --- a/cloud/server.py +++ b/cloud/server.py @@ -1279,10 +1279,12 @@ class CloudControlPlaneHandler(BaseHTTPRequestHandler): def _merge_task(self, user_id, task_id): """把云端任务标记为已合并：只补状态，不执行合并本身。 - 合并本身仍由 host.py merge（本地或 Docker 容器内）执行， - 成功后再调本接口同步云端状态——两者分离是故意的：合并 - 是要人看 diff 才点的操作，云端不该在任务 completed 时 - 自动把主分支改了。规则：任务必须存在（404）、调用方 + 本接口不做 git 合并。自动 PR 的正确顺序是：任务 + completed -> 调本接口改状态 -> 调 POST /tasks/{id}/pr + 推分支建 PR -> 再在 GitHub 上合 PR 或本地 host.py merge。 + 本地 host.py merge 成功后会删分支和 worktree，此后调 + PR 会跳过 no_worktree，所以 PR 必须在本地 git merge + 之前做，而不是之后。规则：任务必须存在（404）、调用方 必须是项目 maintainer 以上（approve_merge，403）、任务 必须是 completed（其它状态一律 409，含已 merged 的重复 调用，避免二次合并混淆）。成功后写审计 merge_task。 diff --git a/cloud/tests/test_cloud_dispatch_pr.py b/cloud/tests/test_cloud_dispatch_pr.py index e3cd22d..62e4ab7 100644 --- a/cloud/tests/test_cloud_dispatch_pr.py +++ b/cloud/tests/test_cloud_dispatch_pr.py @@ -140,6 +140,12 @@ class _ServerCase(unittest.TestCase): if remote_fail: return _result(1, "", "no remote") return _result(0, "https://example.com/repo.git\n") + if "branch" in cmd: + # 假 worktree 正好在自己的 host/<任务名> 分支上： + # worktree 目录名就是任务 id（见 _worktree）。 + return _result(0, "host/%s\n" % os.path.basename(cwd or "")) + if "add" in cmd or "commit" in cmd: + return _result(0, "") if "push" in cmd: if push_fail: return _result(1, "", "push rejected") diff --git a/cloud/tests/test_pr_commit.py b/cloud/tests/test_pr_commit.py new file mode 100644 index 0000000..baecb4b --- /dev/null +++ b/cloud/tests/test_pr_commit.py @@ -0,0 +1,158 @@ +"""pr.py 提交步骤单测：push 之前先把 worktree 改动提交到分支。 + +只用标准库，不联网、不跑真实 git/gh：git/gh 一律用假 runner +注入；worktree 用真实临时目录，只为验证 __pycache__ / *.pyc +清理（可再生的缓存）真的删、普通文件不动。 +""" + +import os +import sys +import tempfile +import unittest +from pathlib import Path +from types import SimpleNamespace + +try: + from cloud.pr import PrStepError, create_task_pr +except ImportError: + sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + from cloud.pr import PrStepError, create_task_pr + + +def _result(returncode=0, stdout="", stderr=""): + return SimpleNamespace(returncode=returncode, stdout=stdout, + stderr=stderr) + + +class _Rec: + """记录命令的假 runner：git/gh 分开注入，commit 行为可配。""" + + def __init__(self, branch="host/t1", commit="ok"): + self.calls = [] + self.branch = branch + self.commit = commit # ok | nothing | fail | no-identity | add-fail + + def git(self, cmd, cwd): + self.calls.append(list(cmd)) + if "remote" in cmd: + return _result(0, "https://example.com/repo.git\n") + if "branch" in cmd: + return _result(0, self.branch + "\n") + if "add" in cmd: + if self.commit == "add-fail": + return _result(1, "", "add failed") + return _result(0, "") + if "commit" in cmd: + if "-c" in cmd: + return _result(0, "") + if self.commit == "nothing": + return _result(1, "", + "nothing to commit, working tree clean") + if self.commit == "fail": + return _result(1, "", "fatal: commit exploded") + if self.commit == "no-identity": + return _result(1, "", + "Please tell me who you are") + return _result(0, "") + if "push" in cmd: + return _result(0, "") + return _result(0, "") + + def gh(self, cmd, cwd): + self.calls.append(list(cmd)) + if cmd[:2] == ["gh", "auth"]: + return _result(0, "", "") + if cmd[:2] == ["gh", "pr"]: + return _result(0, "https://example.com/repo/pull/7\n") + return _result(0, "") + + def kinds(self): + """git 命令按出现顺序归类：branch/add/commit/push。""" + out = [] + for cmd in self.calls: + if cmd[:1] == ["git"]: + for k in ("branch", "add", "commit", "push"): + if k in cmd: + out.append(k) + break + return out + + +class PrCommitTestCase(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.wt = os.path.join(self.tmp.name, "t1") + os.makedirs(self.wt) + + def tearDown(self): + self.tmp.cleanup() + + def _run(self, rec): + return create_task_pr( + "t1", 1, worktrees_root=self.tmp.name, + git_runner=rec.git, gh_runner=rec.gh, gh_which="gh") + + def test_commit_before_push_and_pycache_cleaned(self): + with open(os.path.join(self.wt, "file.txt"), "w") as f: + f.write("x") + cache = os.path.join(self.wt, "__pycache__") + os.makedirs(cache) + with open(os.path.join(cache, "x.pyc"), "wb") as f: + f.write(b"\0") + with open(os.path.join(self.wt, "stale.pyc"), "wb") as f: + f.write(b"\0") + rec = _Rec() + res = self._run(rec) + self.assertEqual(res["pr_url"], "https://example.com/repo/pull/7") + self.assertEqual(res["branch"], "host/t1") + kinds = rec.kinds() + self.assertLess(kinds.index("branch"), kinds.index("add")) + self.assertLess(kinds.index("add"), kinds.index("commit")) + self.assertLess(kinds.index("commit"), kinds.index("push")) + commit_cmd = [c for c in rec.calls if "commit" in c][0] + self.assertIn("cloud: t1", commit_cmd) + self.assertFalse(os.path.exists(cache)) + self.assertFalse(os.path.exists(os.path.join(self.wt, "stale.pyc"))) + self.assertTrue(os.path.exists(os.path.join(self.wt, "file.txt"))) + + def test_nothing_to_commit_proceeds_to_push(self): + rec = _Rec(commit="nothing") + res = self._run(rec) + self.assertEqual(res["pr_number"], 7) + self.assertIn("push", rec.kinds()) + + def test_wrong_branch_fails_commit_without_push(self): + rec = _Rec(branch="main") + with self.assertRaises(PrStepError) as ctx: + self._run(rec) + self.assertEqual(ctx.exception.step, "commit") + self.assertNotIn("push", rec.kinds()) + self.assertNotIn("commit", rec.kinds()) + + def test_commit_failure_is_commit_step_without_push(self): + rec = _Rec(commit="fail") + with self.assertRaises(PrStepError) as ctx: + self._run(rec) + self.assertEqual(ctx.exception.step, "commit") + self.assertNotIn("push", rec.kinds()) + + def test_add_failure_is_commit_step_without_push(self): + rec = _Rec(commit="add-fail") + with self.assertRaises(PrStepError) as ctx: + self._run(rec) + self.assertEqual(ctx.exception.step, "commit") + self.assertNotIn("push", rec.kinds()) + + def test_identity_fallback_retries_with_cloud_identity(self): + rec = _Rec(commit="no-identity") + res = self._run(rec) + self.assertEqual(res["pr_number"], 7) + commits = [c for c in rec.calls if "commit" in c] + self.assertEqual(len(commits), 2) + self.assertIn("-c", commits[1]) + self.assertIn("user.name=cloud", commits[1]) + self.assertIn("push", rec.kinds()) + + +if __name__ == "__main__": + unittest.main()