[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]

❌ Some changes need to be made — 2× P1 doc-accuracy issues block merge

• ❌ P1: Commented-out event-type: pr-review override is invalid — shared workflow does not expose that input, and the stated default is wrong (actual default is pull_request/<action>).
• ❌ P1: # secrets: inherit doesn't pass cross-repo is incorrect — secrets: inherit works within the same org; explicit pass-through is still fine but the rationale is wrong.
• ⚠️ P2: The @main = rolling updates comment block contradicts the actual SHA pin on the next line.
• ⚠️ P2: labeled trigger fans out on every label (incl. area/*, priority/*); may queue duplicate dispatches on busy PRs.
• ❔ P2: converted_to_draft is omitted from types: but the spec treats it as a first-class trigger — pick a side (follow spec or document the deviation).
• ✅ Pinned SHA (verified exists in nsheaps/agents and the referenced reusable workflow is present at that ref) — supply-chain safe.
• ✅ Minimum-necessary permissions set at both top-level and job-level, with explicit checkov rationale.
• ✅ Secrets rationale (gate-only automation creds, reviewer identity on receiver) matches the spec.
• ✅ File extension .yaml is consistent with the rest of .github/workflows/.

Click to expand for full details

How I arrived at the scores

Quality 80% — The workflow structure is clean, well-commented, follows the spec's gate-at-consumer-template model, and uses pinned SHAs. Deductions are for three documentation inaccuracies (event-type override, secrets: inherit claim, @main vs SHA comment) and a divergence from the spec on converted_to_draft. All are comment- or docs-level — the runtime behaviour is correct.

Security 90% — Uses automation-nsheaps[bot] creds only (gate is routing only; no LLM or reviewer-identity secrets reach this workflow); SHA-pinned reusable call; minimum-necessary permissions: at top-level and job-level with an explicit nod to checkov's CKV2_GHA_1; no shell interpolation of untrusted PR data (no run: steps at all in this file). Deducted 10% because labeled fires on every label (not a security hole per se, but a quota/noise footgun; the receiver is trusted to debounce).

Simplicity 85% — Single-job thin shell with a straightforward gate expression. The if: is a short multi-line boolean; the uses: + secrets: block is minimal. Could be slightly simpler by trimming the invalid event-type suggestion and the misleading comment, which would also shorten the file.

Confidence 90% — Verified the pinned SHA exists in nsheaps/agents and that review-dispatch.yaml is present at that ref; read the shared workflow's workflow_call inputs/secrets directly to confirm the invalid event-type finding; checked the current spec at the pinned ref for the converted_to_draft divergence. One assumption I did not verify: that the nsheaps/.github sync job actually bumps this SHA on every change you care about — if it doesn't, the pin may drift far behind.

Detailed findings

❌ P1 — Invalid event-type override (line 76)

See inline comment on lines 73–76. The reusable workflow at nsheaps/agents/.github/workflows/review-dispatch.yaml exposes only one workflow_call input (target-repo). An operator uncommenting event-type: pr-review will get an invalid-input failure. Separately, the comment claims pr-review is the default — but the shared workflow dynamically sets event-type: ${{ github.event_name }}/${{ github.event.action }}, e.g. pull_request/opened.

❌ P1 — Incorrect secrets: inherit rationale (line 69)

See inline comment on line 69. secrets: inherit works for reusable workflows within the same org, including cross-repo calls. The explicit pass-through practice itself is fine (preferable, actually), but the stated reason will mislead the next maintainer. Reword to "explicit for least-privilege/audit clarity."

⚠️ P2 — @main rolling-updates comment contradicts SHA pin (lines 64–68)

See inline comment on lines 64–68. The 4-line comment block describes @main-style rolling updates and recommends SHA-pinning for stability, but line 68 is already SHA-pinned (...@c43151… # main). Rewrite the comment to describe the pin-and-sync model, or revert the uses: to @main if rolling updates are the real intent.

⚠️ P2 — labeled triggers for every label (line 33, 52–57)

See inline comment on lines 52–57. For a non-draft PR, draft != true makes the first OR-branch true for any labeled event. Suggested narrowing in the inline comment keeps request-review as the only labeled trigger. Not blocking — the receiver's own debouncing + the decider's concurrency: cancel-in-progress: false prevent correctness issues — but it's wasted CI quota on noisy PRs.

❔ P2 — converted_to_draft omitted vs spec (lines 33, 47–49)

See inline comment on lines 33–49. Spec says converted_to_draft is a first-class event the receiver short-circuits with neutral. This template omits it from types: and the inline comment handwaves it as "not reviewed until marked ready again." Either add it to types: and let the receiver handle it, or update the comment + spec to acknowledge the consumer-side deviation.

✅ Things done well

• SHA pinning: uses: ...@c431513730c7d848e302e0dc0a7cdc241756fd0f — verified to exist with review-dispatch.yaml at that ref. No floating-tag supply-chain risk.
• Permissions: contents: read / pull-requests: write / checks: write at both top-level and job-level, matching exactly what the called workflow needs. Explicit checkov CKV2_GHA_1 rationale documented inline.
• Secrets scoping: Only AUTOMATION_GITHUB_APP_ID + AUTOMATION_GITHUB_APP_PRIVATE_KEY are passed. The comment correctly explains that LLM-auth and reviewer-identity secrets live on the receiver side.
• Gate defensiveness: pull_request.state == 'open' is redundant with the event types but harmless and defensive against future types: additions.
• No run: steps: Nothing in this file inlines PR-controlled strings into shell — zero code-injection surface here.
• File extension: .yaml matches check.yaml, pages.yaml, sync-main-to-edge.yaml, sync-plugin-specs.yaml.

Scope note

This is a sync template authored by automation-nsheaps[bot] out of nsheaps/.github. The P1 findings are in the template content, so the fix needs to land upstream in nsheaps/.github/ansible/templates/.github/workflows/dispatch-review.yaml for the sync to pick it up — merging a hand-edit to just this repo would be overwritten on next sync.

Follow-ups:

• P0: none
• P1: Remove the invalid event-type override line and fix the misstated default (template-level fix in nsheaps/.github).
• P1: Fix the secrets: inherit doesn't pass cross-repo comment — reword to a least-privilege rationale.
• P2: Reconcile the @main rolling-update comment with the SHA pin on line 68.
• P2: Narrow the labeled trigger to the request-review label name (optional — depends on repo label noise).
• P2: Decide the converted_to_draft stance: add to types: (match spec) or document the intentional deviation.

References

• Workflow run (this review): https://github.com/nsheaps/.ai-agent-henry/actions/runs/39211027624
• Reusable workflow verified at pinned SHA: https://github.com/nsheaps/agents/blob/c431513730c7d848e302e0dc0a7cdc241756fd0f/.github/workflows/review-dispatch.yaml
• Spec at pinned SHA: https://github.com/nsheaps/agents/blob/c431513730c7d848e302e0dc0a7cdc241756fd0f/plugins/claude-code/review-utils/specs/review-dispatch.md
• GitHub reusable-workflow / secrets: inherit docs: https://docs.github.com/en/actions/sharing-automations/reusing-workflows#using-inputs-and-secrets-in-a-reusable-workflow
• checkov CKV2_GHA_1 (top-level permissions): https://docs.prismacloud.io/en/enterprise-edition/policy-reference/supply-chain-policies/github-policies/supply-chain-policy-index/ensure-top-level-permissions-are-not-set-to-write-all

[henry-nsheaps]

❌ P1 — event-type: pr-review is not a valid override and the "default" is misstated.

The reusable workflow at nsheaps/agents/.github/workflows/review-dispatch.yaml (verified at the pinned SHA c431513…) exposes only one workflow_call input — target-repo. There is no event-type input, so uncommenting that line would fail the workflow call with an invalid-input error.

The claimed default is also wrong. The shared workflow sets:

event-type: ${{ github.event_name }}/${{ github.event.action }}

so the actual dispatched event-type is e.g. pull_request/opened, pull_request/synchronize, pull_request/labeled — not the fixed string pr-review. The target receiver's on.repository_dispatch.types must match that pattern, which is what the misstatement risks obscuring.

Suggested fix — drop the invalid line and tighten the remaining hint:

Suggested change

	# Optional overrides (uncomment to use):
	# with:
	# target-repo: nsheaps/.ai-agent-henry # default
	# event-type: pr-review # default repository_dispatch event_type
	# Optional overrides (uncomment to use):
	# with:
	# target-repo: nsheaps/.ai-agent-henry # default target agent repo

If an event-type override is desired as a feature, add it as a workflow_call input in nsheaps/agents/.github/workflows/review-dispatch.yaml first and ship these templates together.

[henry-nsheaps]

❌ P1 — The stated reason for not using secrets: inherit is incorrect.

Per GitHub's reusable-workflow docs, secrets: inherit does work across repositories within the same organization. Both nsheaps/github-actions and nsheaps/agents sit in the nsheaps org, so secrets: inherit would in fact pass these two secrets through.

The explicit pass-through practice itself is fine (arguably better — makes the dependency obvious, limits blast radius, aids audit), but the stated rationale will mislead the next operator who hits a cross-org case or copies this snippet verbatim.

Suggested rewording:

Suggested change

	# secrets: inherit doesn't pass cross-repo (GitHub limitation).
	# Pass only what the gate needs — explicit is safer than `secrets: inherit`
	# here (makes the dependency obvious and keeps unrelated secrets out of the
	# called workflow's env). `secrets: inherit` DOES work same-org, but we
	# prefer the explicit form.

[henry-nsheaps]

⚠️ P2 — Comment describes @main rolling-update behaviour but the uses: pins a SHA.

Lines 64–67 explain that @main gives rolling updates and that operators who want stability should pin a SHA — but line 68 is already pinned (...@c43151… # main). A reader who takes the comment at face value will think this template is on a floating ref when it isn't.

Two coherent resolutions, pick one:

1. Keep the SHA pin (preferred — matches sync-template model) — rewrite the comment to describe the pin-and-sync workflow:

   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@c431513730c7d848e302e0dc0a7cdc241756fd0f # main
   	# Pinned by SHA (bumped automatically by nsheaps/.github sync alongside
   	# plugin version bumps). The trailing `# main` is a provenance hint,
   	# not a floating ref — switch to a floating `@main` only if you
   	# explicitly want rolling updates with no sync-gate.
   	uses: nsheaps/agents/.github/workflows/review-dispatch.yaml@c431513730c7d848e302e0dc0a7cdc241756fd0f # main

2. If rolling updates really are the intended default, switch uses: back to @main.

As a side note: the pinned SHA (c43151…, 2026-10-15) is already a few days behind current nsheaps/agents main (231980…). That's fine for a template that gets re-synced on each change, but worth confirming the sync job actually bumps this on every nsheaps/agents change you care about — otherwise the pin can drift arbitrarily far behind.

[henry-nsheaps]

❔ P2 — Divergence from spec on converted_to_draft — clarify intent.

The review-dispatch.md spec in nsheaps/agents (at the pinned SHA) calls converted_to_draft a first-class event that the receiver "short-circuits with a neutral check rather than running a review the PR is no longer ready for." That implies the consumer template should list converted_to_draft in types: so the receiver can post the neutral check (and optionally cancel an in-flight review).

This template deliberately omits converted_to_draft from types: (line 33) and the gate comment (lines 47–49) says "a PR converted to draft is simply not reviewed until it's marked ready again." That contradicts the spec's intent.

Two options:

• Follow the spec: add converted_to_draft to types: and let the receiver's short-circuit handle the UX.
• Deviate deliberately: update the inline comment to say "we intentionally skip converted_to_draft here because …" and open a spec doc PR noting this consumer-template difference.

Either is OK, but the current state (omit + stale comment that discusses the omitted event) will confuse future operators comparing this file against the spec.

[henry-nsheaps]

⚠️ P2 — labeled fan-out: every label change on a non-draft PR will re-trigger the dispatch.

As written, for a non-draft open PR the first OR-branch (draft != true) is satisfied regardless of action, so any labeled event — on any label — fires the full dispatch. In repos that use many labels (area/*, priority/*, size/*, Dependabot's dependencies, etc.) this means repeated re-dispatches per PR.

The decider has concurrency: cancel-in-progress: false, so these queue rather than cancel each other — but each still bills CI minutes and spends check-run slots on identical reviews.

Suggested narrowing (only treat labeled as a trigger when it's the request-review label):

Suggested change

	if: |
	github.event.pull_request.state == 'open' &&
	(
	github.event.pull_request.draft != true ||
	(github.event.action == 'labeled' && github.event.label.name == 'request-review')
	)
	if: |
	github.event.pull_request.state == 'open' &&
	(github.event.action != 'labeled' || github.event.label.name == 'request-review') &&
	(
	github.event.pull_request.draft != true ||
	(github.event.action == 'labeled' && github.event.label.name == 'request-review')
	)

Not blocking — the receiver has its own debouncing — but worth considering if downstream repos see noisy label churn. If the current behaviour is intentional (e.g. operator can tag wip to pause and remove to re-trigger), ignore.