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

  1. dormouse-bot commented on Sep 15, 2026

    @dormouse-bot
    CollaboratorAuthor

    Triage: this is a permission request awaiting a yes/no, not a repo-local bug — review_preflight.py ships inside the pinned max-sixty/tend/claude@0.2.7 action, so there is nothing in this checkout to fix. Three things I verified that bear on the decision, and one correction to the framing above.

    The root cause is live in this session, not specific to the review workflow. This triage run sees GITHUB_EVENT_PATH set and unreadable, and /home/runner/work absent (ls: cannot access '/home/runner/work': No such file or directory). Every tend session in the sandbox is in the same state, so _event_forces_review() returns False unconditionally for all of them — #655 was not an unlucky run.

    A second instance, independent of #655. #642 flipped to ready at 2026-09-14T22:26:13Z; run 34904096802 fired two seconds later on that event against the PR's head cddbfd6bd33764cc2542bd1effb1e25288c603c9, concluded success, and posted nothing. The PR merged 5.5 minutes after that carrying only the draft-mode COMMENTED review. Across the last 20 PRs the sole APPROVED from the bot is #655's, which I placed by hand.

    Correction to my title: it is not every draft-to-ready PR. already_reviewed keys on a bot review standing at the current head, so any push after the ready flip moves the head, at_head goes back to None, and the next run posts normally — #659 shows that shape, a draft review followed by three ordinary ones. The verdict is lost precisely when ready_for_review is the last event before merge, which is the common case for a clean Renovate bump and the reason this reads as intermittent rather than total. The upstream body should say that, because the symptom as titled will not reproduce for anyone who pushes after marking ready.

    The narrower alternative is as expensive as described. This repo's tend-review.yaml passes no event data into the agent step — the prompt is /tend-ci-runner:review ${{ github.event.pull_request.number }} and nothing else — so a TEND_EVENT_ACTION env var really does require regenerating the workflow in every adopter repo. The API-derived condition needs no workflow change anywhere.

    Nothing matching this exists upstream: searches of max-sixty/tend for GITHUB_EVENT_PATH, ready_for_review draft, and draft-review turn up no open or closed issue on the gate.

    So the ask stands unchanged — OK to file at max-sixty/tend?

  2. dormouse-bot commented on Sep 23, 2026

    @dormouse-bot
    CollaboratorAuthor

    Resolved — the sandbox now mounts the runner's _temp, so _event_forces_review() reads the event file and the draft → ready verdict lands.

    The environment changed, not the upstream code: plugins/tend-ci-runner/scripts/review_preflight.py still carries the same _event_forces_review() and the same silent except OSError. What is different is that /home/runner/work/_temp/_github_workflow/event.json now exists and parses inside the agent sandbox — this nightly session read it and got {organization, repository, schedule, workflow}, the correct shape for a schedule event with no action key, where the September runs found /home/runner/work missing entirely.

    #754 is the end-to-end confirmation, and it is the exact path this issue said was broken:

    Time (UTC) Event
    00:34:04 draft-mode COMMENTED review carrying <!-- tend:draft-review -->
    01:09:37 ready_for_review by nedtwigg
    01:09:39 tend-review run 35805139428 fires on that event
    01:13:04 APPROVED review posted, no draft marker

    No hand-run gh api bypass was involved. #750 and #757 show the same draft-marker → non-draft replacement.

    Closing, and withdrawing the ask to file upstream. Two points from the analysis survive the fix and would still be worth reporting if they ever bite: the gate fails closed and logs nothing when the event file is unreadable, which is why this went unnoticed for weeks; and it cannot recover a ready_for_review event lost or coalesced by the concurrency queue, since it reads the event rather than deriving the state from isDraft plus the standing review's draft_mode. Neither is observable here today. If the verdict starts dropping again I will re-derive from this thread and ask before filing.

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