Skip to content

review: post a PR comment when the dispatcher dies without a result #370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jwbron merged 7 commits into from
Aug 24, 2026
Merged

review: post a PR comment when the dispatcher dies without a result #370

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
60ccbfa
[jwbron/review-dispatcher-death-comment] review: post a PR comment wh…
jwbron Aug 21, 2026
7f7eae6
[jwbron/review-dispatcher-death-comment] review: drop risksPatternsKe…
jwbron Aug 21, 2026
01b73c9
Merge remote-tracking branch 'origin/main' into HEAD
jwbron Aug 24, 2026
127bda6
review: recognize the full death shape, not just a queued comment
jwbron Aug 24, 2026
77ae436
Merge remote-tracking branch 'origin/main' into HEAD
jwbron Aug 24, 2026
719b679
review: read the death shape by parseability; name the no-plan except…
jwbron Aug 24, 2026
65320eb
review: the death prose reads dispatch-result.json by parseability, l…
jwbron Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/review-dispatcher-death-comment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"review": patch
Comment thread
khan-actions-bot marked this conversation as resolved.
Outdated
---

Make a dispatcher death visible on the PR. When the dispatcher's Bash call dies without writing `dispatch-result.json` (killed at the engine ceiling, or crashed), the orchestrator now posts one standalone PR comment saying the review died mid-dispatch and posted no review, with a link to the run, instead of only writing an incomplete report. Run 32418662895 (Khan/actions#362) hit exactly this: the run stayed green, the incomplete report tried to file an issue on a repo with issues disabled, and the PR showed nothing.
20 changes: 17 additions & 3 deletions workflows/review/review.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,9 +835,23 @@ cd gh-aw-review-lib && npx -y tsx workflows/review/lib/submission.ts
`reconciliation.skipLines` is already reflected in the plan. Steps 4-6
below are the plan CLI's; continue at Step 7.
Do not dispatch any sub-agent yourself in this mode, and do not re-run
the dispatcher; if its call failed, treat the run as over budget and land
the review from whatever `out/` evidence exists (the gate decides whether
a verdict may post). Step 9's cache-memory record is also code-owned
the dispatcher; if its call failed but `dispatch-result.json` exists,

@github-actions github-actions Bot Aug 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note (non-blocking): Only the dispatcher-call death is made visible; a plan-CLI death is still silent. The new clause is conditioned on dispatch-result.json being MISSING, and the retained sibling clause ("if its call failed but dispatch-result.json exists ... land the review from whatever out/ evidence exists") speaks to the dispatcher's call, not to the plan CLI at Step 3 item 3 — I grepped review.md for every other add-comment producer (lines 845, 862, 937) and none covers the plan-missing-with-result shape. Was leaving that adjacent no-plan shape uncovered a deliberate scope call for this PR?

review details found by completeness

Comment thread
khan-actions-bot marked this conversation as resolved.
Outdated
treat the run as over budget and land the review from whatever `out/`
evidence exists (the gate decides whether a verdict may post). If
`dispatch-result.json` is MISSING after the dispatcher call (the Bash
call was killed at the engine ceiling or crashed before writing it),
there is no plan and you compose nothing by hand: post exactly one
standalone PR comment with the `add-comment` safe output stating that
Comment thread
khan-actions-bot marked this conversation as resolved.

@github-actions github-actions Bot Aug 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought (non-blocking): The death notice is a prompt rule executed by the process that may be dead. This makes visibility of a dead run depend on the orchestrator surviving its own death and following a multi-clause prose instruction, while the repo already has a deterministic chokepoint for exactly this — the post-steps dispatch-conformance gate runs if: always() in the agent job and already rewrites the finalized safe-output queue, and this PR's own new dispatch-gate test shows the gate sees the death shape. review.md's rationale for that gate (lines ~302-305) states the principle directly: "a prompt rule cannot gate an orchestrator that is already ignoring the prompt" — does the same reasoning apply to the notice that a run died?

review details found by first-principles

the automated review died mid-dispatch and posted no review, linking
this run (`$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID`),
Comment thread
khan-actions-bot marked this conversation as resolved.
Outdated
and noting that the next push triggers a fresh full review; then report
the run incomplete, skip Steps 4-8, and continue at Step 9 (the cache

@github-actions github-actions Bot Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question (non-blocking): Is the orchestrator prompt the right layer, when the root defect is that report_incomplete's only surface is an issue on an issues-disabled repo? This branch only fires when the agent survives the Bash kill and reliably follows a nested prompt instruction; the compiled workflow's report_incomplete / create_report_incomplete_issue handlers (review.lock.yml:564, 1357) are the generic path and are the thing that silently no-ops here. A deterministic if: always() step that posts a PR comment when the run queued no submission — or a PR-comment fallback for report_incomplete — would cover every silent-death shape including the job-level timeout that #369 is also raising.

review details found by first-principles

@github-actions github-actions Bot Aug 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question (non-blocking): The diagnosis names the green run as a cause; the remedy leaves it green. The description lists "the run stayed green" alongside the missing comment as why the death was invisible, but the change only adds the comment and still reports success. Was exiting non-zero on the no-plan path considered and rejected — noting that the documented reason the gate avoids red-flagging infra failures (a spurious failure issue while the queue posts anyway) does not hold here, since nothing posts and issues are disabled on this repo?

review details found by first-principles

CLI tolerates the missing result and leaves the prior fingerprints
Comment thread
khan-actions-bot marked this conversation as resolved.
Outdated
standing). Exists because run 32418662895 (Khan/actions#362) was killed
at the Bash ceiling during claim validation and posted nothing: the run
stayed green, and the incomplete report's fallback (filing an issue) is
unavailable on a repo with issues disabled, so the death was invisible
on the PR. Step 9's cache-memory record is also code-owned
(`lib/cache-record.ts`, invoked there); never write or edit
`/tmp/gh-aw/cache-memory/pr-*.json` yourself.

Expand Down
Loading