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?
tend-reviewskips the verdict on every PR that goes draft → ready, becausethe sandbox cannot read
$GITHUB_EVENT_PATH. Asking before I file this atmax-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 replacethat COMMENT with a real verdict once the PR is marked ready. Both halves of
the replacement are gated on
_event_forces_review()inplugins/tend-ci-runner/scripts/review_preflight.py:In the agent sandbox
GITHUB_EVENT_PATHis set to/home/runner/work/_temp/_github_workflow/event.json, and/home/runner/workdoes not exist — the runner's
_tempis not mounted. Theexcept OSErrorswallows the
FileNotFoundError, so the function returnsFalseon everyrun,
ready_for_reviewincluded. Two consequences:startreportsalready_reviewed: true, so the skill's Pre-flightchecks step tells the run to finish without posting.
postskips withalready carries a COMMENTED review, so the APPROVEcannot 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_reviewat 16:39:45Z,and this run
(34996406538)
fired on that event and got:
I verified the event file is absent rather than merely unreadable
(
ls: cannot access '/home/runner/work': No such file or directory), did thefull review by hand, and posted the APPROVE with a direct
gh apicall afterre-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:
startalready readsinitial["isDraft"];postwould needisDraftaddedto its
_pr_view(pr, repo, "headRefOid,state")projection. This also fixesthe case where the
ready_for_reviewevent is lost or coalesced by theconcurrency 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 initemitTEND_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 OSErroris worth a log line: a gate that failsclosed on a missing file and says nothing is why this went unnoticed.
Ask
OK for me to file this at
max-sixty/tend?