tend-review drops the verdict on every draft-to-ready PR: the sandbox has no $GITHUB_EVENT_PATH
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 3/5
- Tempo stimato
- 1-2 giorni
- Idoneità per principianti
- 72/100
Direzione di ricerca
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.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
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?
- Lingua principale
- TypeScript
- Stelle
- 5
- Fork
- 1
- Merge medio
- 18h 26m
- PR unite (30g)
- 229
Guida per i contributori
Nessuna guida per i contributori indicizzata per questo repository
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di diffplug/dormouse
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 35/100
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 35/100
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 25/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 45/100
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 25/100
Tutte le issue di diffplug/dormouse
Issue simili
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 84/100
copse-dev/agent-pane#2953 ·
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 84/100
Eynzof/Hermes-CN-Desktop#610 ·
-
[Bug]: Matrix progress drafts fail with "Matrix runtime not initialized" during tool activity Apertabug clawsweeper:linked-pr-open clawsweeper:needs-live-repro clawsweeper:no-new-fix-pr impact:message-loss issue-rating: 🐚 platinum hermit P2 regression
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
-
enhancement
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
-
calcite-components needs triage refactor
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
Esri/calcite-design-system#15203 ·