[sync] Create .github/workflows/dispatch-review.yaml

[automation-nsheaps]

Org-sync from nsheaps/.github

Direct push was blocked by branch protection. This PR applies the
central file update via the standard PR flow.

Path: .github/workflows/dispatch-review.yaml
Action: Create

Branch org-sync/-github-workflows-dispatch-review-yaml is stable — subsequent runs update this PR.

[henry-nsheaps]

Review — .github/workflows/dispatch-review.yaml

Verdict: 💬 Comment · P0: 0 · P1: 1 · P2: 2

The gate workflow is well-structured: SHA-pinned reusable-workflow reference, explicit top-level + job-level permissions, correct cross-repo secret passing, and thorough rationale comments explaining the gate/receiver credential split. The only real friction is documentation drift on the @main block — the comment describes rolling-update behavior while the actual ref is SHA-pinned. Non-blocking, and possibly best fixed upstream in the sync template.

How I arrived here

I walked the file top-to-bottom against three lenses: security (permissions, ref pinning, secret handling), operator experience (does the comment explain what the code actually does?), and trigger semantics (does the if: match the stated gate policy?).

Security: ✅ SHA-pinned reusable workflow ref, least-privilege permissions (contents: read, pull-requests: write, checks: write), explicit secrets: passing (correct — secrets: inherit doesn't span user-scoped repos), no code from the PR head is executed here.

Trigger semantics: ✅ The if: expression correctly implements the stated policy — non-draft PRs fire on any listed event; drafts fire only when request-review is applied. Short-circuit evaluation protects github.event.label.name from being dereferenced on non-labeled events.

Operator experience: ⚠️ One P1 doc/code contradiction and two P2 clarity nits — details inline.

Strengths

• SHA-pin on the reusable workflow ref — supply-chain best practice for cross-repo uses:.
• Top-level permissions: block satisfies checkov CKV2_GHA_1 and provides defense-in-depth alongside the job-level grant.
• Explicit secrets: block (not inherit) — correct for cross-repo uses: on user-owned repos.
• Comment block explaining why automation creds are used at the gate vs reviewer creds at the receiver is genuinely useful context that would be painful to reconstruct later.
• Short-circuit safety in the if: (action == 'labeled' && label.name == ...) avoids null-deref on non-labeled events.

Follow-ups

#	Priority	Summary
1	🐛 P1	Block comment claims @main = rolling updates, but the uses: line is SHA-pinned. Fix one or the other.
2	💡 P2	Trailing # main on the SHA doesn't identify version/date — use a tag or datestamp so operators can eyeball staleness.
3	💡 P2	converted_to_draft mention in the gate comment refers to an event that's not in the trigger list — confusing.

None of these block merge. The P1 is a documentation-vs-behavior mismatch; if this file is auto-synced from nsheaps/.github, the fix likely belongs in the template source.

Not raised (considered and passed)

• Every label add on a non-draft PR invokes this workflow (if: short-circuits on draft != true regardless of label name). Downstream decider filters, and label events are cheap. Not worth an inline.
• No unlabeled in trigger types — intentional; removing request-review shouldn't re-evaluate a still-draft PR.
• secrets: inherit claim in line 69 comment — technically correct for user-owned repos: cross-repo inherit requires org-owned repos with the setting enabled. Fine as written.

---

Reviewed 76-line new file. No prior reviews on this PR.

[henry-nsheaps]

🐛 P1 — Comment contradicts the actual uses: ref

The block comment says @main = rolling updates and calls this "intentional", telling operators to replace @main with a SHA if they want pinned stability. But the uses: line right below is already pinned to a SHA:

uses: nsheaps/agents/.github/workflows/review-dispatch.yaml@57debfca1958b3632acd8b9a29bff99573b99993 # main

So a reader is told "this file uses @main for rolling updates" while looking at a SHA-pinned reference. Two failure modes:

• Ops read the comment, assume rolling behavior, and wait for updates that never arrive (the SHA has to be re-synced).
• Ops trust the pin, ignore the comment — but then the rationale block is dead weight and misleads the next reader.

Either the sync process rewrote @main → <SHA> without updating the comment, or the template intent changed. Please rewrite the rationale to match the actual behavior in the consumer's repo — pinned, refreshed on sync — and drop the "rolling" framing. Example:

Suggested change

	# @main = rolling updates: any change merged to nsheaps/agents takes effect
	# on the next PR event in repos using this template. This is intentional —
	# operators who need pinned stability should replace @main with a commit SHA
	# and update it in lock-step with plugin version bumps.
	uses: nsheaps/agents/.github/workflows/review-dispatch.yaml@57debfca1958b3632acd8b9a29bff99573b99993 # main
	# Pinned to a specific nsheaps/agents SHA; the trailing `# main` records
	# which branch the pin was taken from. The `nsheaps/.github` sync automation
	# refreshes this SHA when the upstream `main` moves — updates are NOT
	# rolling in this consumer repo. Operators wanting immediate rolling
	# updates can replace the SHA with `@main` (loses supply-chain pin).
	uses: nsheaps/agents/.github/workflows/review-dispatch.yaml@57debfca1958b3632acd8b9a29bff99573b99993 # main

If this file is supposed to use @main directly in consumer repos, then fix the uses: instead — but the SHA pin is the safer default.

[henry-nsheaps]

💡 P2 — Trailing # main doesn't identify what was pinned

The # main comment tells you which branch the SHA came from, but not when or what version. Six months from now, this file still says # main while the SHA points at a commit that's hundreds of commits behind — and a reader has no easy way to tell.

Convention in the actions ecosystem (and what Dependabot / Renovate emit) is a stable identifier — a tag, or at minimum a date:

uses: nsheaps/agents/.github/workflows/review-dispatch.yaml@57debfc... # v1.4.2
# or
uses: nsheaps/agents/.github/workflows/review-dispatch.yaml@57debfc... # main @ 2026-08-28

If nsheaps/agents doesn't tag releases, adopting even a lightweight tag on the reusable workflow (or emitting the sync-time date via the sync process) would let operators eyeball staleness without opening the compare view.

[henry-nsheaps]

💡 P2 — converted_to_draft clarification is confusing because that event isn't in the trigger list

The comment tells the reader converted_to_draft "does NOT fire a review by itself" — but that event isn't in on.pull_request.types above (line 33) at all, so of course it doesn't fire anything. Leading with an event that literally cannot fire this workflow makes the reader hunt for context.

The useful content is: "if a PR is currently a draft, the if: skips it unless request-review is applied." Suggest tightening to that:

Suggested change

	# Gate: review fires automatically on any OPEN, non-draft PR event
	# (opened, reopened, synchronize, ready_for_review) -- no label needed.
	# The `request-review` label only matters to FORCE a review on a DRAFT
	# PR (apply the label while it's still a draft). `converted_to_draft`
	# does NOT fire a review by itself -- a PR converted to draft is simply
	# not reviewed until it's marked ready again or explicitly labeled. If
	# you change the request label name, update the literal in the `==`
	# comparison below.
	# Gate: review fires automatically on any OPEN, non-draft PR event
	# (opened, reopened, synchronize, ready_for_review) -- no label needed.
	# For a DRAFT PR, the gate skips unless someone applies the
	# `request-review` label; that's the only way to force a review on a
	# draft. If you change the label name, update the literal in the `==`
	# comparison below.

(This also implicitly clarifies what happens post-converted_to_draft — subsequent synchronize events are gated out — without naming a non-triggering event.)