diff --git a/docs/guides/github.md b/docs/guides/github.md index 5fd1d9a..70a3d7a 100644 --- a/docs/guides/github.md +++ b/docs/guides/github.md @@ -32,6 +32,8 @@ In Actions, Landing supplies the trigger, target, and relevant revisions. The ag PR review publishes a native [GitHub Review](https://docs.github.com/en/rest/pulls/reviews#create-a-review-for-a-pull-request) for the requested commit, with findings attached to the affected lines. A clean review needs no code comments. Reviews comment by default; approvals and change requests require explicit authorization. The gate recommendation remains separate. +While admitted work runs, Landing adds an eyes reaction to the triggering PR or command comment. When the work finishes, a rocket replaces it for completed work and a confused reaction for failed work; cancelled or superseded runs only remove it. A delegation for a head that is no longer current is rejected before it starts and leaves existing reactions unchanged. A hard-killed runner cannot clean up, so its eyes reaction stays until the next run on that subject replaces it. Reactions are best effort: a token without reaction permission leaves the work and its publication unaffected. + Explicit delegations require a reply to their selected issue or PR; inline follow-ups reply in the original review thread. Landing verifies that the prepared identity published to the requested destination before reporting completion. Include `confirm_reply` when restricting tools for inline follow-ups. Automatic issuer follow-up publishes only useful new evidence, changed conditions, or verified progress. Unchanged conditions complete quietly through `no_update`, retaining the reason in SQLite. Failed tool execution cannot waive required publication. Explicit questions still require replies. diff --git a/src/landing/adapters/github.py b/src/landing/adapters/github.py index f6d504b..d73c849 100644 --- a/src/landing/adapters/github.py +++ b/src/landing/adapters/github.py @@ -373,6 +373,56 @@ def pull_target( return is_pr, thread, head +def reaction_endpoint(repository: str, event: dict | None) -> str | None: + """Locate the triggering comment or PR, which receives Landing's progress reaction.""" + comment = (event or {}).get("comment") + if comment: + kind = "pulls" if "pull_request_review_id" in comment else "issues" + return f"repos/{repository}/{kind}/comments/{comment['id']}/reactions" if "id" in comment else None + pull = (event or {}).get("pull_request") or {} + return f"repos/{repository}/issues/{pull['number']}/reactions" if "number" in pull else None + + +def react(repository: str, endpoint: str, content: str) -> dict | None: + # Reactions only signal progress; a missing reaction permission must not fail the delegated work. + try: + return json.loads(gh(["api", endpoint, "-f", f"content={content}"], repository)) + except (RuntimeError, subprocess.TimeoutExpired, ValueError) as exc: + typer.echo(f"Cannot add the {content} reaction: {exc}", err=True) + return None + + +def unreact(repository: str, endpoint: str, reaction: dict) -> None: + try: + gh(["api", f"{endpoint}/{reaction['id']}", "-X", "DELETE"], repository) + except (RuntimeError, subprocess.TimeoutExpired) as exc: + typer.echo(f"Cannot remove the {reaction['content']} reaction: {exc}", err=True) + + +def begin_reaction(repository: str, endpoint: str) -> dict | None: + progress = react(repository, endpoint, "eyes") + if progress is None: + return None + # A new PR candidate replaces the previous run's outcome, so the PR shows only the latest result. + try: + reactions = rows(endpoint, repository) + except (RuntimeError, subprocess.TimeoutExpired, ValueError) as exc: + typer.echo(f"Cannot read earlier reactions: {exc}", err=True) + reactions = [] + for item in reactions: + if item.get("user", {}).get("id") == progress["user"]["id"] and item.get("content") in {"rocket", "confused"}: + unreact(repository, endpoint, item) + return progress + + +def conclude_reaction(repository: str, endpoint: str, progress: dict | None, status: str) -> None: + if progress is not None: + unreact(repository, endpoint, progress) + # A superseded or terminated run leaves no outcome; the next run reports its own. + if status != "cancelled": + react(repository, endpoint, "rocket" if status == "completed" else "confused") + + def checkout_contains(workspace: Path, head: str, checked_revision: str) -> bool | None: executable = shutil.which("git") if not head or not checked_revision or not executable: @@ -446,6 +496,10 @@ async def run(landing: Runtime, options: GitHubSettings, event: dict | None = No checks=options.check if mode in {"fixer", "gatekeeper"} else [], ) + # pull_target has already rejected a stale candidate, so its rejection leaves the current outcome in place. + endpoint = reaction_endpoint(repository, event) + progress = begin_reaction(repository, endpoint) if endpoint else None + status = "failed" publication = Publication(landing, repository, number, stamp, expected_review, thread, head, reply_required) manager = landing.framework.plugin_manager manager.register(publication, name="github-publication") @@ -460,22 +514,31 @@ async def run(landing: Runtime, options: GitHubSettings, event: dict | None = No request.model_copy(update={"input": snapshot}), scope=repository, key=key ) publication.verify(action) - return landing.tasks.finish(action.id, "completed", result=f"Already published: {existing['html_url']}") - try: - action = await landing.run( - request, - session_id=f"github:{number or key}", - scope=repository, - key=key, - verify=publication.verify, + action = landing.tasks.finish( + action.id, "completed", result=f"Already published: {existing['html_url']}" ) - except asyncio.CancelledError: - if not publication.action_id: - raise - action = landing.tasks.get(publication.action_id) - return action + else: + try: + action = await landing.run( + request, + session_id=f"github:{number or key}", + scope=repository, + key=key, + verify=publication.verify, + ) + except asyncio.CancelledError: + if not publication.action_id: + raise + action = landing.tasks.get(publication.action_id) + status = action.status + except asyncio.CancelledError: + status = "cancelled" + raise finally: manager.unregister(publication) + if endpoint: + conclude_reaction(repository, endpoint, progress, status) + return action def write_outputs(action: Action | None) -> None: diff --git a/tests/test_github.py b/tests/test_github.py index 78bfe5f..43d3b81 100644 --- a/tests/test_github.py +++ b/tests/test_github.py @@ -33,7 +33,21 @@ def platform(tmp_path, monkeypatch): raise SystemExit("GitHub is unavailable") if args[0] != "api": raise SystemExit("Use the native API in this fixture.") -if "--input" in args: +reactions = state.setdefault("reactions", {}) +if "/reactions" in endpoint: + subject, _, reaction = endpoint.partition("/reactions") + if "DELETE" in args: + reactions[subject] = [item for item in reactions[subject] if item["id"] != int(reaction.strip("/"))] + elif "-f" in args: + content = args[args.index("-f") + 1].split("=", 1)[1] + state["reaction_id"] = state.get("reaction_id", 0) + 1 + record = {"id": state["reaction_id"], "content": content, "user": state["publisher"]} + reactions.setdefault(subject, []).append(record) + print(json.dumps(record)) + else: + print(json.dumps([reactions.get(subject, [])])) + path.write_text(json.dumps(state)) +elif "--input" in args: source = args[args.index("--input") + 1] body = json.loads(sys.stdin.read() if source == "-" else Path(source).read_text()) kind = "reviews" if endpoint.endswith("/reviews") else "comments" @@ -234,12 +248,16 @@ def test_agent_publishes_native_review_with_inline_comment_and_deduplicates(plat assert published[0]["commit_id"] == "candidate-head" assert published[0]["comments"][0]["path"] == "candidate.py" assert published[0]["comments"][0]["line"] == 2 + reactions = json.loads(platform.read_text())["reactions"] + assert [item["content"] for item in reactions["repos/example/landing/issues/42"]] == ["rocket"] state = json.loads(platform.read_text()) state["reviews"].append({"id": 9, "body": "A later independent review.", "state": "COMMENTED"}) platform.write_text(json.dumps(state)) calls = len(requests) assert invoke(event, key="review:42").id == action.id assert len(requests) == calls + reactions = json.loads(platform.read_text())["reactions"] + assert [item["content"] for item in reactions["repos/example/landing/issues/42"]] == ["rocket"] assert json.loads(platform.read_text())["reviews"] == state["reviews"] monkeypatch.setenv("INPUT_INSTRUCTION", "Review deployment behavior.") invoke(event, key="review:42", error="different request") @@ -371,6 +389,7 @@ async def reply_from_evidence(**kwargs): assert len(state["reviews"]) == 2 assert state["comments"][-1]["in_reply_to_id"] == 17 assert "initial attempt is unaffected" in state["comments"][-1]["body"] + assert [item["content"] for item in state["reactions"]["repos/example/landing/pulls/comments/18"]] == ["rocket"] assert invoke(event, key="comment:18").id == action.id assert json.loads(platform.read_text())["comments"] == state["comments"] event["comment"]["body"] = "Fixed in the latest commit; CI is pending." @@ -400,9 +419,28 @@ def test_text_without_required_publication_is_failed_work(platform, invoke, mode assert action.result == "The review is ready." assert action.error is not None assert "confirmed publication" in action.error["message"] + assert [ + item["content"] for item in json.loads(platform.read_text())["reactions"]["repos/example/landing/issues/42"] + ] == ["confused"] assert json.loads(platform.read_text())["reviews"] == state["reviews"] +def test_stale_review_delegation_keeps_the_current_outcome_reaction(platform, invoke, model): + state = json.loads(platform.read_text()) + state["head"] = "new-candidate" + current = {"repos/example/landing/issues/42": [{"id": 7, "content": "rocket", "user": state["publisher"]}]} + state["reactions"] = current + platform.write_text(json.dumps(state)) + event = { + "repository": {"full_name": "example/landing"}, + "sender": {"type": "User", "login": "maintainer"}, + "pull_request": {"number": 42, "head": {"sha": "candidate-head"}}, + } + invoke(event, key="stale", error="PR head changed") + assert json.loads(platform.read_text())["reactions"] == current + assert not model[1] + + @pytest.mark.parametrize("lookup_fails", [False, True]) def test_review_stops_queued_tools_for_superseded_or_unverifiable_head(tmp_path, platform, invoke, model, lookup_fails): responses, requests = model