tend-review drops the verdict on every draft-to-ready PR: the sandbox has no $GITHUB_EVENT_PATH
Nobody has claimed this yet.
Assessment
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Newbie friendliness
- 72/100
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:
startreportsalready_reviewed: true, so the skill's Pre-flight
checks step tells the run to finish without posting.postskips withalready 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
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from diffplug/dormouse
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
-
Difficulty 4/5 3-5 days Newbie friendliness 25/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 45/100
-
Difficulty 4/5 3-5 days Newbie friendliness 25/100
All issues in diffplug/dormouse
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
Eynzof/Hermes-CN-Desktop#610 ·
-
bug clawsweeper:linked-pr-open clawsweeper:needs-live-repro clawsweeper:no-new-fix-pr impact:message-loss issue-rating: 🐚 platinum hermit P2 regression
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
calcite-components needs triage refactor
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
Esri/calcite-design-system#15203 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 78/100
fullcalendar/fullcalendar#8106 ·