Skip to content

tend-review drops the verdict on every draft-to-ready PR: the sandbox has no $GITHUB_EVENT_PATH #665

Description

@dormouse-bot

tend-review skips the verdict on every PR that goes draft → ready, because
the sandbox cannot read $GITHUB_EVENT_PATH. Asking before I file this at
max-sixty/tend.

What happens

tend-review's draft mode posts a COMMENT carrying the hidden marker
<!-- tend:draft-review -->, and the marker is what lets a later run replace
that COMMENT with a real verdict once the PR is marked ready. Both halves of
the replacement are gated on _event_forces_review() in
plugins/tend-ci-runner/scripts/review_preflight.py:

def _event_forces_review() -> bool:
    path = os.environ.get("GITHUB_EVENT_PATH")
    if not path:
        return False
    try:
        event = json.loads(Path(path).read_text())
    except (OSError, json.JSONDecodeError):
        return False
    return isinstance(event, dict) and event.get("action") == "ready_for_review"

In the agent sandbox GITHUB_EVENT_PATH is set to
/home/runner/work/_temp/_github_workflow/event.json, and /home/runner/work
does not exist — the runner's _temp is not mounted. The except OSError
swallows the FileNotFoundError, so the function returns False on every
run, ready_for_review included. Two consequences:

  • start reports already_reviewed: true, so the skill's Pre-flight
    checks
    step tells the run to finish without posting.
  • post skips with already carries a COMMENTED review, so the APPROVE
    cannot land even if the run gets that far.

This is not a corner case here: AGENTS.md says "Open every PR as a draft",
so draft → ready is the normal path for every PR in this repo, and the review
that matters is the one being dropped.

Evidence

#655 — a clean Renovate cargo
bump. The draft review landed at 16:39:03Z, ready_for_review at 16:39:45Z,
and this run
(34996406538)
fired on that event and got:

$ review_preflight.py start 655
{"head_sha": "04ccdfd...", "already_reviewed": true, ...}

$ review_preflight.py post 655
skip: 04ccdfd491fbc05547f2c213cff3e3606a864528 already carries a COMMENTED review 5212962948

I verified the event file is absent rather than merely unreadable
(ls: cannot access '/home/runner/work': No such file or directory), did the
full review by hand, and posted the APPROVE with a direct gh api call after
re-checking that the PR was open, non-draft, and still at the head I read.
That bypass is not something a session should be doing routinely, which is
why I want the gate fixed upstream.

Proposed fix

Derive the condition from the API data the script already has, instead of from
the event file. "Not a draft, and the review standing at head is a draft-mode
one" is exactly the state the marker exists to encode, and it needs no event:

def _draft_review_superseded(is_draft: bool, at_head: object) -> bool:
    return (
        not is_draft
        and isinstance(at_head, dict)
        and at_head.get("draft_mode") is True
    )

start already reads initial["isDraft"]; post would need isDraft added
to its _pr_view(pr, repo, "headRefOid,state") projection. This also fixes
the case where the ready_for_review event is lost or coalesced by the
concurrency queue, which the event check cannot recover from.

The narrower alternative is to keep the event check and feed it from the
environment — have tend init emit TEND_EVENT_ACTION: ${{ github.event.action }}
on the review job and read that, falling back to the event file. It needs a
workflow regeneration in every adopter repo, and it still leaves the lost-event
case broken.

Either way the silent except OSError is worth a log line: a gate that fails
closed on a missing file and says nothing is why this went unnoticed.

Ask

OK for me to file this at max-sixty/tend?

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions