Repository navigation
fix(klaud): verify older and aggregate baselines, free abandoned work / fix(klaud): 验证旧版与聚合基线并释放放弃的工作 #4137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0f46af3
a3330e2
5018d64
1e9362d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,18 +4,23 @@ | |
|
|
||
| import json | ||
| import os | ||
| import re | ||
| import subprocess | ||
| import tempfile | ||
| from pathlib import Path | ||
| from urllib.parse import urlencode | ||
|
|
||
| from . import claims, github | ||
| from .github import VerificationError | ||
| from .models import CandidateOutcome, OwnedCandidate, Ownership, utc | ||
| from .models import CandidateOutcome, OwnedCandidate, Ownership, identity, utc | ||
| from .reporting import translated | ||
| from .validation import verify_sweep | ||
|
|
||
| BOT = "Klaud-Cold" | ||
| COMMITTER = "github-actions[bot]" | ||
| ACTIVE_STATUSES = ("requested", "waiting", "pending", "queued", "in_progress") | ||
| CANDIDATE_ID = r"[0-9a-f]{16}-[0-9a-f]{16}" | ||
| RECONCILE_JOB = "Reconcile interrupted candidates" | ||
| SWEEP_LABELS = { | ||
| "full-sweep-enabled", | ||
| "non-canary-full-sweep-enabled", | ||
|
|
@@ -87,7 +92,8 @@ def pulls(self) -> list[dict]: | |
| return pulls | ||
|
|
||
| def runs(self) -> list[dict]: | ||
| query = {"per_page": 100, "created": ">=" + self.parent["created_at"]} | ||
| end = self.parent["updated_at"] if terminal(self.parent) else "*" | ||
| query = {"per_page": 100, "created": f"{self.parent['created_at']}..{end}"} | ||
| targeted = github.items( | ||
| self.repository, | ||
| "actions/workflows/e2e-tests.yml/runs?" | ||
|
|
@@ -105,7 +111,14 @@ def runs(self) -> list[dict]: | |
| sweeps = github.items( | ||
| self.repository, | ||
| "actions/workflows/run-sweep.yml/runs?" | ||
| + urlencode({**query, "event": "pull_request", "branch": self.branch}), | ||
| + urlencode( | ||
| { | ||
| "per_page": 100, | ||
| "created": ">=" + self.parent["created_at"], | ||
| "event": "pull_request", | ||
| "branch": self.branch, | ||
| } | ||
| ), | ||
| "workflow_runs", | ||
| ) | ||
| pulls = self.pulls() | ||
|
|
@@ -531,6 +544,7 @@ def recover() -> None: | |
| raise VerificationError("Untrusted ownership parent") | ||
| results = {} | ||
| blocked = set() | ||
| live = set() | ||
| for owner, _ in sorted(owners, key=lambda pair: pair[0].run_id, reverse=True): | ||
| parent = parents[owner.run_id] | ||
| for candidate in owner.candidates: | ||
|
|
@@ -540,11 +554,13 @@ def recover() -> None: | |
| results[key] = False | ||
| if not terminal(parent): | ||
| blocked.add(candidate.family) | ||
| live.add(candidate.id) | ||
| continue | ||
| session = Session(repository, parent, candidate, recovering=True) | ||
| try: | ||
| if not claims.recovery_lease(repository, parent["id"], candidate): | ||
| blocked.add(candidate.family) | ||
| live.add(candidate.id) | ||
| continue | ||
| changed = reconcile(session) | ||
| claims.release_family(repository, candidate, parent["id"]) | ||
|
|
@@ -582,6 +598,136 @@ def recover() -> None: | |
| current = github.read(repository, f"actions/runs/{owner.run_id}") | ||
| if terminal(current) and current["run_attempt"] == parents[owner.run_id]["run_attempt"]: | ||
| github.write(repository, f"actions/artifacts/{artifact['id']}", "DELETE") | ||
| try: | ||
| live |= sweep_abandoned(repository, live) | ||
| except (OSError, ValueError, KeyError, TypeError, subprocess.SubprocessError) as error: | ||
| print(f"::warning::Klaud abandoned-work sweep skipped: {type(error).__name__}") | ||
| else: | ||
| prefixes = {candidate.split("-", 1)[0] for candidate in live} | ||
| blocked = {family for family in blocked if identity(family)[:16] in prefixes} | ||
| (Path(os.environ["RUNNER_TEMP"]) / "klaud-recovery.json").write_text( | ||
| json.dumps(sorted(blocked)) | ||
| ) | ||
|
|
||
|
|
||
| def owned_activity(repository: str) -> tuple[set[str], dict[str, list[int]], set[int]]: | ||
| live: set[str] = set() | ||
| targeted: dict[str, list[int]] = {} | ||
| planners: set[int] = set() | ||
| for status in ACTIVE_STATUSES: | ||
| query = urlencode({"status": status, "per_page": 100}) | ||
| for run in github.items( | ||
| repository, f"actions/workflows/e2e-tests.yml/runs?{query}", "workflow_runs" | ||
| ): | ||
| match = re.fullmatch(rf"e2e Test - klaud-\d+-({CANDIDATE_ID})", run["display_title"]) | ||
| if match and run["actor"]["login"] == BOT: | ||
| targeted.setdefault(match[1], []).append(run["id"]) | ||
| for run in github.items( | ||
| repository, f"actions/workflows/run-sweep.yml/runs?{query}", "workflow_runs" | ||
| ): | ||
| if match := re.fullmatch(rf"klaud/auto-({CANDIDATE_ID})", run["head_branch"] or ""): | ||
| live.add(match[1]) | ||
| for run in github.items( | ||
| repository, f"actions/workflows/klaud-plan.yml/runs?{query}", "workflow_runs" | ||
| ): | ||
| for job in github.items( | ||
| repository, f"actions/runs/{run['id']}/jobs?per_page=100", "jobs" | ||
| ): | ||
| if match := re.search(rf"Run candidate ({CANDIDATE_ID})", job["name"]): | ||
| live.add(match[1]) | ||
| elif job["name"] == RECONCILE_JOB and job["status"] != "completed": | ||
| planners.add(run["id"]) | ||
| return live, targeted, planners | ||
|
|
||
|
|
||
| def bot_committed(commits: list[dict]) -> bool: | ||
| return bool(commits) and all( | ||
| (commit.get("committer") or {}).get("login") == COMMITTER for commit in commits | ||
| ) | ||
|
|
||
|
|
||
| def sweep_abandoned(repository: str, live: set[str]) -> set[str]: | ||
| sessions, targeted, planners = owned_activity(repository) | ||
| live = live | sessions | ||
| pulls = github.items(repository, "pulls?state=open&per_page=100") | ||
| abandoned = [] | ||
| for pull in pulls: | ||
| match = re.fullmatch(rf"klaud/auto-({CANDIDATE_ID})", pull["head"]["ref"]) | ||
| if not match or match[1] in live: | ||
| continue | ||
| if ( | ||
| pull["draft"] | ||
| and pull["user"]["login"] == BOT | ||
| and (pull["head"]["repo"] or {}).get("full_name") == repository | ||
| and not any(label["name"] == "klaud-handoff" for label in pull["labels"]) | ||
| and bot_committed( | ||
| github.items(repository, f"pulls/{pull['number']}/commits?per_page=100") | ||
| ) | ||
| ): | ||
| abandoned.append(pull) | ||
|
Comment on lines
+654
to
+667
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Klaud draft PRs a maintainer has re-swept, or whose recovery hit a transient error, now get closed and their branch deleted in the same recovery pass. The base left them open. sweep_abandoned only checks author, draft, labels and commit authorship. It ignores that reconcile() failed for this candidate moments earlier, for example with "New maintainer runs require explicit handoff". Fix: have recover() pass sweep_abandoned the ids whose reconcile raised anything but PendingCleanup, and skip their PR close, branch delete and claim release. Orphaned-smoke sessions still get freed. Why this was flaggedTrigger: a session hits the 6h job limit and leaves a draft Klaud PR with a sweep label. A maintainer then starts a run-sweep.yml run on it with no commit. When that run ends, the next recover() calls reconcile(). Session.runs() raises VerificationError("New maintainer runs require explicit handoff") at lifecycle.py:136-140. The except at lifecycle.py:567-574 only adds the family to blocked, not the id to live. sweep_abandoned (lifecycle.py:654-667) then finds a draft, Klaud-authored PR with only github-actions[bot] commits and no klaud-handoff label. It comments "abandoned", closes the PR and deletes the branch (lifecycle.py:691-706), and releases the family claim (lifecycle.py:676-681). On the base branch the PR, branch and claim stayed intact and the family stayed blocked until a maintainer added a handoff label. The owned_activity() liveness check only covers run-sweep runs that are still active, so it does not prevent any of this. Verification: Session.runs() raises VerificationError("New maintainer runs require explicit handoff") at lifecycle.py:136-140. The except at lifecycle.py:567-574 only does |
||
| else: | ||
| live.add(match[1]) | ||
| for candidate, runs in targeted.items(): | ||
| if candidate not in live: | ||
| for run in runs: | ||
| github.write(repository, f"actions/runs/{run}/cancel", "POST") | ||
| for owner in claims.family_owners(repository): | ||
| candidate = owner.candidates[0] | ||
| if candidate.id not in live and terminal( | ||
| github.read(repository, f"actions/runs/{owner.run_id}") | ||
| ): | ||
| claims.release_family(repository, candidate, owner.run_id) | ||
| if planners <= {int(os.environ.get("GITHUB_RUN_ID", "0"))}: | ||
| finished: dict[int, bool] = {} | ||
| for lease in github.items(repository, "git/matching-refs/heads/klaud/recovery-"): | ||
| match = re.fullmatch(rf"refs/heads/klaud/recovery-(\d+)-({CANDIDATE_ID})", lease["ref"]) | ||
| if not match or match[2] in live: | ||
| continue | ||
| parent = int(match[1]) | ||
| if parent not in finished: | ||
| finished[parent] = terminal(github.read(repository, f"actions/runs/{parent}")) | ||
| if finished[parent]: | ||
| github.write(repository, "git/" + lease["ref"], "DELETE") | ||
| for pull in abandoned: | ||
| github.write( | ||
| repository, | ||
| f"issues/{pull['number']}/comments", | ||
| "POST", | ||
| { | ||
| "body": translated( | ||
| "**abandoned** · The owning Klaud session ended without cleanup. Closed, " | ||
| "with any leftover smoke runs cancelled, so a new candidate can take this " | ||
| "family.", | ||
| "**abandoned** · 所属 Klaud 会话已结束但未完成清理。已关闭并取消残留的冒烟运行," | ||
| "新的候选可以接手该配置族。", | ||
| ) | ||
| }, | ||
| ) | ||
| github.write(repository, f"pulls/{pull['number']}", "PATCH", {"state": "closed"}) | ||
| github.write(repository, f"git/refs/heads/{pull['head']['ref']}", "DELETE") | ||
| open_heads = {pull["head"]["ref"] for pull in pulls} | ||
| for ref in github.items(repository, "git/matching-refs/heads/klaud/auto-"): | ||
| branch = ref["ref"].removeprefix("refs/heads/") | ||
| match = re.fullmatch(rf"klaud/auto-({CANDIDATE_ID})", branch) | ||
| if not match or match[1] in live or branch in open_heads: | ||
| continue | ||
| history = github.items( | ||
| repository, | ||
| "pulls?" | ||
| + urlencode( | ||
| { | ||
| "state": "all", | ||
| "head": repository.split("/", maxsplit=1)[0] + ":" + branch, | ||
| "per_page": 100, | ||
| } | ||
| ), | ||
| ) | ||
| if history: | ||
| latest = max(history, key=lambda pull: pull["created_at"]) | ||
| closer = github.read(repository, f"issues/{latest['number']}").get("closed_by") or {} | ||
| if latest["user"]["login"] != BOT or closer.get("login") != BOT: | ||
| continue | ||
| elif not bot_committed([github.read(repository, f"commits/{ref['object']['sha']}")]): | ||
| continue | ||
| github.write(repository, f"git/refs/heads/{branch}", "DELETE") | ||
| return live | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 (optional) If
claude --versionever writes to stderr, both Klaud workflows now fail the install step twice and no candidate starts. The base ignored stderr there. The new check merges stderr into the compared string with2>&1, so any warning breaks the exact match against "2.1.282 (Claude Code)". Fix: compare only stdout, which the base already did, and send stderr to the log, e.g.[ "$("$claude_cli" --version)" = "2.1.282 (Claude Code)" ]. Apply this at both sites (klaud-candidate.yml:85, klaud-plan.yml:142).Why this was flagged
This finding only applies if the pinned 2.1.282 CLI, its npm wrapper or Node prints anything to stderr on a successful
--version, such as a deprecation, ExperimentalWarning or updater notice. The new loop at klaud-candidate.yml:85 and klaud-plan.yml:142 compares$("$claude_cli" --version 2>&1)with the exact string "2.1.282 (Claude Code)". Any stderr text makes the comparison fail, so both attempts fail and[ "$attempt" = 2 ] && exit 1aborts the step. The base usedtest "$("$claude_cli" --version)" = ...and compared stdout only, so a working install with a stderr warning passed there. After this change, theclaudestep is skipped for every candidate, and the planner's overlap-review step is skipped too. The retry does not help because it repeats the same comparison.Verification: In both klaud-candidate.yml:85 and klaud-plan.yml:142 the check is now
[ "$("$claude_cli" --version 2>&1)" = "2.1.282 (Claude Code)" ] && break, which folds stderr into the string being compared. The base line captured only stdout. With any stderr text, both attempts fail,[ "$attempt" = 2 ] && exit 1aborts the install step, and no candidate or planner session starts.