Skip to content

feat: show Landing progress with reactions - #65

Merged
frostming merged 2 commits into
mainfrom
feat/progress-reactions
Oct 7, 2026
Merged

frostming merged 2 commits into
mainfrom
feat/progress-reactions

Conversation

@frostming

Copy link
Copy Markdown
Contributor

Purpose

Admitted GitHub work gave no sign that Landing had started until its reply or review appeared. Landing now adds an eyes reaction to the triggering PR (automatic review) or /landing command comment (issue or inline review comment) while the work runs. When it finishes, a rocket replaces the eyes for completed work and a confused reaction for failed work; superseded or terminated runs only remove the eyes. A new PR candidate removes the previous run's rocket or confused reaction so the PR shows only the latest result. Unadmitted events get no reaction.

Reactions are best effort: a token without reaction permission, or any reaction API failure, is reported on stderr and leaves the work and its publication unaffected.

Validation

  • uv run ty check: passed.
  • uv run pre-commit run --all-files: passed.
  • uv run python -m pytest: 126 passed, 1 skipped.
  • make docs-test: no issues.
  • tests/test_github.py now checks the final reactions for a completed PR review, a failed PR review, a completed inline-thread delegation, and a replayed PR review that keeps a single rocket.

Limits: the fixture models GitHub's reactions API; real reaction behavior still needs observation on a PR. In Main, Landing starts only after native checks finish, so the eyes reaction appears then rather than at PR creation. A hard-killed process leaves its eyes reaction until the next run replaces it.

AI assistance

Implemented with Claude Code (Claude Opus 5.5, claude-opus-5-5[1m]).

🤖 Generated with Claude Code

Admitted GitHub work gave no sign that it had started until its reply
or review appeared. Add an eyes reaction to the triggering PR or
command comment while the work runs, then replace it with rocket for
completed work or confused for failed work. Superseded or terminated
runs only remove it. A new PR candidate also removes the previous
run's outcome so the PR shows only the latest result.

Reactions are best effort: failures are reported on stderr and leave
the work and its publication unaffected.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommended decision: allow.

Reactions behave as the guide describes for admitted work: unadmitted events stay silent, the outcome replaces the eyes when the work finishes, and a reaction failure leaves the work and its publication unaffected. One non-blocking finding: a delegation rejected because its target head is no longer the PR head is recorded as failed work, so it clears the current candidate's outcome reaction and leaves a confused reaction, which the sentence added in this PR says does not happen.

Comment thread src/landing/adapters/github.py Outdated
action = asyncio.run(run(runtime, options, payload))
endpoint = reaction_endpoint(options.repository, payload)
progress = begin_reaction(options.repository, endpoint) if endpoint else None
status = "failed"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A delegation whose target head is no longer the PR head (a workflow_dispatch rebuild with an explicit head, or a re-run whose payload predates the current head) fails here as ordinary failed work. run() raises The PR head changed; delegate a new review for the current candidate. before any action exists, status keeps this default, and conclude_reaction adds a confused reaction. Because begin_reaction already deleted the previous outcome, the PR also loses the current candidate's rocket or confused reaction, so the guide's "superseded or terminated runs only remove it" does not hold on this path.

Reproduced through landing github event with a pull_request event whose head trails the current head and a rocket already on the PR:

Probe and result
state["reactions"] = {"repos/example/landing/issues/42": [{"id": 7, "content": "rocket", "user": {"id": 314}}]}
state["head"] = "new-candidate"  # the PR moved on after the event
invoke({"pull_request": {"number": 42, "head": {"sha": "candidate-head"}}, ...}, key="stale", error="PR head changed")
AFTER STALE DELEGATION: {'repos/example/landing/issues/42': [{'id': 2, 'content': 'confused', 'user': {'id': 314}}]}

Repair direction: treat this rejection like a supersession, so a delegation that never reaches the model neither clears nor sets an outcome — verify the target head before begin_reaction, or map that rejection to the cancelled status. If the behavior is intended, the guide sentence should say that a head mismatch marks the PR as failed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed at dafadba: begin_reaction now runs after pull_target, so this delegation is rejected before any reaction is created or removed, and test_stale_review_delegation_keeps_the_current_outcome_reaction pins the unchanged rocket. The verification-time variant of the same class is still open; the reply on docs/guides/github.md:35 describes it.

Comment thread docs/guides/github.md Outdated

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; superseded or terminated runs only remove it. Reactions are best effort: a token without reaction permission leaves the work and its publication unaffected.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence covers the mid-run head-change cancellation only. A delegation rejected before its first tool call because the target head is no longer current still leaves a confused reaction on the PR, and clears the outcome of the candidate that is actually current (the finding on src/landing/adapters/github.py:617). Hard termination is also outside "only remove it": a killed process runs no cleanup, so its eyes reaction stays until the next admitted run replaces it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rejection-before-start and hard-kill parts above are addressed at dafadba: begin_reaction now runs after pull_target, the sentence describes the hard-killed runner, and test_stale_review_delegation_keeps_the_current_outcome_reaction pins the unchanged reactions. One part of the sentence still over-claims: a superseded run that reaches publication verification replaces the eyes with a confused reaction instead of only removing it.

Replay at the merge checkout (f6fe866)

A PR review whose head moves between the last tool-batch head check and Publication.verify ends as failed work, and conclude_reaction then marks the PR confused:

status:    failed
error:     The PR head changed; delegate a new review for the current candidate.
reactions: [{'content': 'confused', 'user': {'id': 314}}]

That path removes only its own eyes reaction, so when the current candidate's run already finished, its rocket stays next to the confused reaction.

This contradicts the intent recorded above conclude_reaction ("A superseded or terminated run leaves no outcome"). Repair direction: treat that head-change rejection as supersession rather than failed work, or narrow the sentence to cancelled runs.

A delegation whose target head was no longer the PR head added the eyes
reaction, removed the current candidate's outcome, and then failed in
pull_target, leaving a confused reaction for work that never started.
Start the reaction only after the target head is confirmed, and
document that hard-killed runners leave their eyes reaction behind.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommended decision: allow.

Two non-blocking reaction edges remain.

  • Work that fails while preparing the publication keeps its eyes reaction indefinitely, so a crashed run looks like work still in progress: src/landing/adapters/github.py:501.
  • A superseded run that reaches publication verification marks the PR confused, which the sentence added here reserves for failed work: reply on docs/guides/github.md:35.


# 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An exception between this call and the try block leaves the eyes reaction in place with no outcome. Publication(...) on the next line queries the publisher identity through gh api graphql (and lists thread comments when a thread is delegated), so a transient API failure there aborts the delegation before conclude_reaction can run: the run exits non-zero while the PR or command comment keeps showing Landing at work, which the guide attributes only to a hard-killed runner.

Probe at the merge checkout (f6fe866)

With a rocket already on the PR and the viewer query failing once (gh: service unavailable (HTTP 503)), landing github event for a PR review exits 1 and leaves no outcome:

BEFORE: [{'id': 7, 'content': 'rocket', 'user': {'id': 314}}]
AFTER:  [{'id': 1, 'content': 'eyes', 'user': {'id': 314}}]

The previous revision of this PR wrapped the whole asyncio.run(run(...)) call in github_event, where the same failure was concluded as failed work.

Repair direction: build and register the Publication before begin_reaction, or start the reaction inside the try, so every failure after the eyes is concluded.

@frostming
frostming merged commit 0f4b778 into main Oct 7, 2026
9 checks passed
@frostming
frostming deleted the feat/progress-reactions branch October 7, 2026 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant