diff --git a/.github/time/sync.py b/.github/time/sync.py index 9b8c873..5e4556d 100644 --- a/.github/time/sync.py +++ b/.github/time/sync.py @@ -158,6 +158,37 @@ def hand_back(repo: str, card: int, pr: int) -> bool: return True +REVIEWING = ("pr", "needs-human", "no-checks", "checks-failed") # a card whose PR is being or has been reviewed + + +def revised(repo: str, branch: str, before: str | None) -> list[int]: + """The copy pushed new commits to a branch whose PR is open and already reviewed (a cloud session answering the + review): each card the PR closes gets a `dispatched` beat marked `revised`, so the heartbeat reviews the new head + and a verdict from before the push no longer counts, and the PR loses `needs-human`. A card not yet reviewed (the + heartbeat hasn't adopted the PR) gets nothing: its first review sees the new head anyway. `before` is the home's + head of the branch before this import; a branch new to the home is never a revision.""" + if repo != tm.REPO or not before: + return [] + pr = next((p for p in tm.gh("GET", f"/repos/{repo}/pulls?state=open&limit=50") or [] + if (p.get("head") or {}).get("ref") == branch), None) + if not pr: + return [] + cards = {card_of(branch)} | {int(x) for x in re.findall(r"\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+#(\d+)\b", pr.get("body") or "", re.I)} + done = [] + for c in sorted(x for x in cards if x): + beats = [b for b in tm.beats_of(tm.gh("GET", f"/repos/{repo}/issues/{c}/comments?limit=100") or []) + if b["beat"] not in ("blocked", "capped")] + if not beats or beats[-1]["beat"] not in REVIEWING: + continue + tm.comment(c, f"☁️ The cloud session pushed a revision to PR #{pr['number']}. The heartbeat reviews it again; " + f"a verdict from before the push no longer counts.", + {"beat": "dispatched", "pr": pr["number"], "revised": True, "cloud": True}) + done.append(c) + if done and tm.has_label(pr, "needs-human"): + tm.unlabel(pr["number"], "needs-human") + return done + + def ensure_pr(repo: str, work: str, branch: str, base: str, copy: str, where: str) -> int | None: """A PR on the home for an imported branch, unless one is open for it, it is already in the base, or a PR for it was closed at this same commit (a human or the council said no; a new push brings it back).""" @@ -225,12 +256,18 @@ def sync_repo(entry: dict, home_url: str, force: bool = False) -> dict: if branch in out or not any(fnmatch.fnmatch(branch, p) for p in pats): continue git("fetch", "-q", url, f"+refs/heads/{branch}:refs/copy/{branch}", cwd=work) - if mine.get(branch) != sha: # the copy writes this branch: the home takes it as it is + moved = mine.get(branch) != sha + if moved: # the copy writes this branch: the home takes it as it is git("push", "-q", "--force", home_url, f"refs/copy/{branch}:refs/heads/{branch}", cwd=work) r["imported"].append(branch) n = ensure_pr(repo, work, branch, base, name, where) if n: r["prs"].append(n) + elif moved: + try: + r.setdefault("revised", []).extend(revised(repo, branch, mine.get(branch))) + except Exception as e: # noqa: BLE001 the commits are home either way; the next review sees them + print(f"revised {branch}: {scrub(str(e))[:200]}") ENV.clear() return report @@ -258,7 +295,8 @@ def main() -> int: repo = e.get("repo") or tm.REPO rep = sync_repo(e, authed(f"{tm.SERVER}/{repo}.git", token), force) for c in rep["copies"]: - print(f"sync {repo} -> {c['copy']}: out {c['out']} imported {c['imported']} prs {c['prs']} {c['note']}".rstrip()) + print(f"sync {repo} -> {c['copy']}: out {c['out']} imported {c['imported']} prs {c['prs']} " + f"revised {c.get('revised', [])} {c['note']}".rstrip()) for b, state in c["out"].items(): if state == "diverged" and repo == tm.REPO: flag(repo, c["copy"], b) diff --git a/.github/time/test_time.py b/.github/time/test_time.py index bb08a9e..cb28426 100644 --- a/.github/time/test_time.py +++ b/.github/time/test_time.py @@ -1498,6 +1498,28 @@ g83("checkout", "-qb", "claude/session-abc123", cwd=cw83); commit83(cw83, "i.txt g83("push", "-q", "origin", "claude/session-abc123", cwd=cw83) sync83() assert moves83 == [("-", 9, "cloud"), ("+", 9, "ready")], "a card the commits close leaves the lane too" +# the cloud session answers a review on the same branch: the card gets a `dispatched` beat marked `revised` (so the +# heartbeat reviews the new head and the old objection stops counting) and the PR loses `needs-human` +beat83 = lambda b: {"user": {"login": sy.tm.TIME_BOT}, "created_at": "2026-10-19T18:13:21Z", "body": "x\n"} +comments83 = {7: [beat83("pr"), beat83("needs-human")], 9: []} +pr7_open = {"number": 104, "state": "open", "head": {"ref": "claude/card-7-cloud", "sha": "?"}, "body": pr7["body"], "labels": [{"name": "needs-human"}]} +def gh83r(m, path, body=None, **k): + if path.endswith("/comments?limit=100"): + return comments83.get(int(path.split("/issues/")[1].split("/")[0]), []) + if path.startswith("/repos/o/r/pulls?state=open"): + return [pr7_open] + return sy.tm.gh_inner(m, path, body, **k) +sy.tm.gh_inner, sy.tm.gh = sy.tm.gh, gh83r +notes83.clear(); moves83.clear() +prs83[:] = [dict(p, state="open") if p["head"]["ref"] == "claude/card-7-cloud" else p for p in prs83] +g83("checkout", "-q", "claude/card-7-cloud", cwd=cw83); commit83(cw83, "g.txt", "Answer the review\n\nCloses #7"); g83("push", "-q", "origin", "claude/card-7-cloud", cwd=cw83) +c83 = sync83() +assert c83["revised"] == [7] and notes83[-1][0] == 7 and notes83[-1][2] == {"beat": "dispatched", "pr": 104, "revised": True, "cloud": True}, (c83, notes83) +assert ("-", 104, "needs-human") in moves83, moves83 +comments83[7] = []; notes83.clear() +commit83(cw83, "g.txt", "More before any review"); g83("push", "-q", "origin", "claude/card-7-cloud", cwd=cw83) +assert sync83().get("revised", []) == [] and not notes83, "a PR nobody has reviewed yet needs no revised beat" +assert sy.revised("o/r", "claude/card-7-cloud", None) == [] and sy.revised("x/y", "claude/card-7-cloud", "abc") == [] sy.tm.gh = real_gh83 # a copy whose token isn't set is skipped; tokens go in the URL and never into anything printed c83 = sy.sync_repo({"repo": "o/r", "copies": [{"name": "gh", "url": "https://github.com/o/r.git", "token": "NO_SUCH_TOKEN_83"}]}, diff --git a/CHANGELOG.md b/CHANGELOG.md index 13ef0d4..13ffd51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ - The second fix round escalates to a different model from the first round: `claude:opus` with the Claude seats (it was `venice:kimi code`, and grok 4.7 before that). `TIME_VENTURE_ESCALATE` overrides it. ### Added +- A cloud PR's revision comes from the cloud, and Time reviews it again. The branch of a cloud-lane PR is the copy's to write, so `/time retry` (the API worker pushing to it) would be overwritten by the next sync. When the reviewer objects instead, the launcher on hb-5 starts one more cloud session on the same branch with the review in its prompt (once; then a human). When the copy's branch moves under a PR that is open and already reviewed, sync.py posts a `dispatched` beat marked `revised` on each card the PR closes and takes `needs-human` off the PR, so the heartbeat reviews the new head and a verdict from before the push no longer counts. A PR not reviewed yet gets nothing; its first review sees the new head. - The cloud lane: Claude Code cloud sessions build cards on the owner's cloud-session credit, beside the API worker. With `cloud: {"per_day": N, "sizes": ["M"]}` in `time/router.json`, Astra's live run labels the best-ranked actionable cards of those sizes `cloud`, until N cards are in the lane, before the API worker's picks; a `cloud` card is lined up, so it is never queued for the API worker. A launcher on hb-5 (a user agent in the desktop session, where the claude.ai sign-in lives; it reads the public issue list and holds no Teahub credentials) starts one `claude --cloud` session per card, with the card as the prompt, on the GitHub copy. The session pushes a `claude/card-N-*` branch; sync.py brings it home as a PR that closes the card (adding `Closes #N` when the commits don't), moves the card from `cloud` to `ready` with a note that carries no beat, and the heartbeat adopts the PR, so the council reviews and lands it like any card PR. A card the commits close leaves the lane too, whatever the branch is called. `per_day: 0`, the default, is no lane. - Time brings a PR branch up to date with its base (#123). An approved PR Gitea reports as not `mergeable` (or whose merge call answers 409) gets `git merge origin/` (the PR's own base) pushed as one commit by Time, `card #N: merge `; CI runs on the new head and landing follows on a later beat (a `ci-dispatched` or `updated` beat now reaches `land()` again). A conflict is not guessed at: the merge is aborted, the branch left as it was, and the conflicting files and `<<<<<<<` blocks (trimmed) are posted on the card and the PR with the `needs-human` label; `/time retry` hands them to the worker as the reviewer's text. Known limit: the worker's `write_file` of a resolved file is a plain commit, not the merge, so Time's next try can conflict again. - A card waiting on a person holds no `ready` slot (#135). `promote_next` counts toward `TIME_PARALLEL` only the `ready` cards that are moving; one whose last real beat is `needs-human`, `error` or `stuck` (an objection, or a failure Time won't retry again) keeps its label, so `/time retry` still finds it, but frees its slot. Three such cards held every slot twice, the second time for ten hours on 2026-10-19 while 22 cards sat `queued`. The run log prints `promote: ready=N moving=M`.