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

Open
#665 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
72/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
github-actions, python
Domain
ci-cd, devtools

Research direction

Start in plugins/tend-ci-runner/scripts/review_preflight.py, reading _event_forces_review() and the start/post entry points. Check how start reads initial["isDraft"] and how post builds _pr_view(pr, repo, "headRefOid,state"). Done means a draft-mode COMMENT is replaced by a verdict when the PR is no longer draft, even when GITHUB_EVENT_PATH is absent, with the relevant start and post commands confirming the behavior.

Written by the indexing model from the issue text.

Description

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?

Dominant language
TypeScript
Stars
5
Forks
1
Avg merge
18h 26m
Merged PRs (30d)
229

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from diffplug/dormouse

All issues in diffplug/dormouse

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.