fix: align 4 reusable workflows to design intent

[lml2468]

Summary

Fixes all 4 reusable PR workflows to match the confirmed design intent.

---

Changes

octo-pr-review-feed.yml — remove out-of-scope actions

Design intent: notify project channel on ready_for_review / review_requested only.

• Removed "opened" and "synchronize" from ALLOWED_ACTIONS (they were never sent by any per-repo caller, but were dead code that contradicted the design)
• Removed the synchronize-specific message branch
• Simplified message construction to a single format for the two valid actions

octo-pr-result-notify.yml — remove dead code + reviewer display

• Removed "pr_reopened" from ALLOWED_KINDS — no caller sends this event_kind; it was unreachable dead code
• Added reviewer_display with (unknown) fallback for defense-in-depth if a caller ever passes an empty reviewer. All current callers do pass reviewer: ${{ github.event.review.user.login }}, so this fallback is not user-visible today
• Hoisted reviewer_display out of the per-event branches into the input-parsing section so it is computed once (review feedback)

reusable-check-sprint.yml — skip draft PRs

Design intent: "正式 PR" (formal/non-draft PRs only).

• Added isDraft query and an early exit 0 so draft PRs are never blocked by sprint validation
• Step 0 runs before sprint resolution so draft PRs never trip on board-health failures (missing Sprint field, no active iteration, etc.)
• Header usage example now recommends ready_for_review and converted_to_draft in pull_request_target.types. ready_for_review is required for the draft-skip to be safe — without it, the green result cached on the draft SHA persists after the PR is marked ready and bypasses the Sprint gate
• Runtime ::notice:: makes the same requirement explicit so caller maintainers see it in logs

reusable-pr-contributor-welcome.yml — richer welcome message

• Added Contributing Guide link
• Added Discord community invite
• Added CI-check reminder
• Welcome comment remains in English (external contributors)
• Idempotency marker <!-- octo-pr-welcome:v1 --> unchanged

---

Not Changed

• Trigger conditions (on: workflow_call:), inputs, secrets — all unchanged
• Security mitigations (allowlist, input validation, sanitization, retry) — all preserved
• All other workflow files — untouched

---

Caller-side action required

Per-repo callers of reusable-check-sprint.yml must update their pull_request_target.types to include ready_for_review (and ideally converted_to_draft). Without it, a stale green result from the draft SHA persists after promotion to ready and bypasses the Sprint gate.

[lml2468]

Review — PR #51 (.github) @ ae7518e

Verdict: APPROVED

Four reusable workflow alignment changes, all consistent with design intent:

1. octo-pr-result-notify.yml

• Removed pr_reopened from ALLOWED_KINDS and its message template — consistent with callers dropping reopened trigger ✓
• Added reviewer_display = reviewer if reviewer else "(unknown)" for review events — defensive fix for edge case where reviewer string is empty ✓

2. octo-pr-review-feed.yml

• Narrowed ALLOWED_ACTIONS from {opened, ready_for_review, review_requested, synchronize} to {ready_for_review, review_requested} — removes noisy open/push notifications from review feed ✓
• Removed synchronize message template and simplified formatting — clean ✓
• Note: callers still triggering on opened/synchronize will hit the existing early-exit guard (sys.exit(0) for unrecognized actions), so no breakage

3. reusable-check-sprint.yml

• Added isDraft to GraphQL query + early exit for draft PRs — correct, drafts shouldn't require sprint assignment ✓

4. reusable-pr-contributor-welcome.yml

• Updated welcome message: more actionable (mentions CI checks, adds Contributing Guide + Discord links) ✓

CI all green (actionlint, sanity, add-to-project). No blocking or non-blocking issues.

[lml2468]

Review — PR #51 (.github) @ ae7518e

Verdict: APPROVED

Four reusable workflow alignment changes, all consistent with design intent:

1. octo-pr-result-notify.yml

• Removed pr_reopened from ALLOWED_KINDS and its message template — consistent with callers dropping reopened trigger ✓
• Added reviewer_display = reviewer if reviewer else "(unknown)" for review events — defensive fix for edge case where reviewer string is empty ✓

2. octo-pr-review-feed.yml

• Narrowed ALLOWED_ACTIONS from {opened, ready_for_review, review_requested, synchronize} to {ready_for_review, review_requested} — removes noisy open/push notifications from review feed ✓
• Removed synchronize message template and simplified formatting — clean ✓
• Note: callers still triggering on opened/synchronize will hit the existing early-exit guard (sys.exit(0) for unrecognized actions), so no breakage

3. reusable-check-sprint.yml

• Added isDraft to GraphQL query + early exit for draft PRs — correct, drafts shouldn't require sprint assignment ✓

4. reusable-pr-contributor-welcome.yml

• Updated welcome message: more actionable (mentions CI checks, adds Contributing Guide + Discord links) ✓

CI all green (actionlint, sanity, add-to-project). No blocking or non-blocking issues.

[Jerry-Xin]

Summary: The PR is relevant to Mininglamp-OSS/.github, but the draft-PR Sprint skip does not actually run early enough to satisfy the stated design intent.

🔴 Blocking

🔴 Critical — .github/workflows/reusable-check-sprint.yml:248 and .github/workflows/reusable-check-sprint.yml:375: the workflow still resolves the current Sprint and validates the Project board before checking pr.isDraft. If the Sprint field is missing, there is no active sprint, or the project token lacks the board scopes, the job exits with failure before reaching the draft skip. That means draft PRs can still be blocked, despite the stated intent that only formal/non-draft PRs should be checked.

Fix by querying the PR’s isDraft state before the current-sprint/project-board lookup, then exiting 0 immediately for drafts. The full Sprint/project validation should only run for non-draft PRs.

💬 Non-blocking

🔵 Suggestion — .github/workflows/octo-pr-result-notify.yml:202: reviewer_display could be computed once before the event-kind branch to avoid duplication, but the current behavior is correct.

✅ Highlights

✅ .github/workflows/octo-pr-review-feed.yml: narrowing ALLOWED_ACTIONS to ready_for_review and review_requested matches the described notification scope.

✅ .github/workflows/octo-pr-result-notify.yml: the (unknown) reviewer fallback prevents blank reviewer fields without changing the workflow input contract.

✅ .github/workflows/reusable-pr-contributor-welcome.yml: the richer welcome message keeps the existing idempotency marker unchanged and links to an existing CONTRIBUTING.md.

[yujiawei]

Code Review — PR #51 (.github)

Independent review of the 4 reusable-workflow changes (head ae7518e). The notification/welcome changes are clean and well-scoped; the reusable-check-sprint.yml draft-skip change has a correctness issue relative to its stated intent and a cross-file interaction that weakens the merge gate, so changes are requested before merge.

1. Verification

Change	Verdict	Evidence
octo-pr-review-feed.yml: narrow ALLOWED_ACTIONS to {ready_for_review, review_requested} + drop synchronize branch	✅ Safe	All 15 callers trigger only on ready_for_review/review_requested and pass event_action: ${{ github.event.action }}. The removed opened/synchronize values were never sent; even if one were, the unrecognized-action guard (sys.exit(0)) handles it. Confirmed via org-wide code search.
octo-pr-result-notify.yml: remove pr_reopened from ALLOWED_KINDS	✅ Safe	No caller emits pr_reopened. All result-notify callers compute event_kind only as pr_merged/pr_closed/review_approved/review_changes_requested. Removed value falls through the unrecognized-kind guard.
octo-pr-result-notify.yml: reviewer_display (unknown) fallback	✅ Correct	Prevents a blank reviewer: line without changing the input contract. Current callers already pass reviewer: ${{ github.event.review.user.login }}, so the fallback is defensive-only.
reusable-pr-contributor-welcome.yml: richer welcome message	✅ Safe	Idempotency marker <!-- octo-pr-welcome:v1 --> unchanged; CONTRIBUTING.md link resolves (file exists at repo root); no actions/checkout added; permissions: {} + issues: write job scope preserved.
reusable-check-sprint.yml: draft-PR skip	⚠️ See P1 below

Security posture of all four reusable workflows is preserved: permissions: {}, no PR code checkout, input validation (require_group_id, require_repo_name, sanitize_text), and the API-base allowlist are all untouched.

2. Issues

🔴 P1 — Draft skip runs too late; does not match the stated "drafts never block" intent

reusable-check-sprint.yml:375 exits early for draft PRs, but the check is placed after Step 1 (current-sprint resolution, lines ~246–309), which calls sys.exit(1) when the Sprint field is missing, no active iteration covers today, or PROJECT_TOKEN lacks board scopes. The isDraft value is fetched in the second GraphQL query (line ~372), so a draft PR still hits all of Step 1's failure paths first.

Result: the PR description's claim that "draft PRs never block" only holds when the board is healthy. On an unhealthy board, drafts are blocked anyway — the opposite of the design intent.

Fix: resolve isDraft first (a tiny dedicated query, or reorder so the PR query runs before sprint resolution) and exit 0 for drafts before any board/sprint validation.

🔴 P1 — Cross-file: draft skip turns a blocking RED into a cached GREEN, weakening the gate for draft→ready

This is the more important interaction and is why I'm requesting changes rather than nit-ing.

• Before this PR: a draft PR with no Sprint produced a failing check that stayed red.
• After this PR: the same draft produces a passing check ("✅ Skipping Sprint check for draft PR.").

Branch protection caches a status per head SHA. 14 of 16 callers (e.g. octo-server, octo-web, octo-adapters, octo-lib, octo-admin) trigger check-sprint on [opened, synchronize, reopened, edited] but not ready_for_review. So the sequence:

1. Open as draft (no Sprint) → check runs, returns green "skipped", cached on the SHA.
2. Click "Ready for review" without pushing a new commit → no pull_request_target event in the caller's types list → check does not re-run.
3. The stale green "skipped" result remains, and the now-ready PR can merge without ever passing real Sprint validation.

Only octo-cli lists ready_for_review in its caller trigger, so it is the lone caller not exposed to this path.

Mitigating fact (why this is P1, not P0): check-sprint is currently not in any repo's required-status-checks ruleset — the enforced contexts are code-review, Build, Vet, Lint only. So the bypass is latent, not actively exploitable today. But this is the shared merge-gate workflow across 16 repos and the PR is classified security-sensitive; shipping a change whose net effect is "sprint-less draft → green that survives the ready transition" is the wrong direction for a gate, even while latent.

Fix options (either or both):

• In the reusable workflow, keep the draft skip but pair the rollout with adding ready_for_review to caller types: so the check re-evaluates on the draft→ready transition; or
• Have the draft branch return a neutral/pending outcome rather than a green pass, so a skipped draft cannot satisfy a required check after becoming ready.

At minimum, document this caller-trigger requirement in the workflow header so the 14 affected callers are updated alongside.

3. Suggestions (non-blocking)

• 🔵 octo-pr-result-notify.yml:203,210: reviewer_display is computed identically in both review branches. Could be hoisted once before the branch. Cosmetic; current behavior is correct.

4. Additional observations

• The octo-pr-review-feed.yml and octo-pr-result-notify.yml simplifications are good dead-code removals and verified safe against every live caller — no action needed there.
• For a human reviewer to manually verify before/after merge: confirm whether check-sprint is intended to become a required check. If yes, the P1 gate-weakening must be fixed first. If it is intentionally advisory-only, the severity of the second P1 drops to P2, but the first P1 (skip placement vs. stated intent) still stands.

Verdict

CHANGES_REQUESTED. Three of four changes are clean and safe to merge as-is. The draft-skip change should (a) run before sprint/board resolution to honor its own stated intent, and (b) be reconciled with caller triggers (or return a non-green skip) so it cannot leave a stale green on a draft→ready PR. The fixes are small.

[lml2468]

Addressed review feedback (commit d76a5d6):

Fixed

🔴 P1a — isDraft check placement (Jerry-Xin, yujiawei)

• Added a dedicated Step 0 that runs a small GraphQL query fetching only isDraft before Step 1 (sprint/board resolution).
• Draft PRs now exit 0 immediately, before any board-health query that could fail (missing Sprint field, no active iteration, token scope issues, etc.).
• Removed isDraft from the large Step 2 pr_query and the now-redundant if pr.get("isDraft"): block.

🔴 P1b — Stale green on draft→ready transition (yujiawei)

• Added ::notice:: annotations when skipping for draft PRs, explicitly stating that callers must include ready_for_review in their pull_request_target types: to ensure the check re-evaluates on the draft→ready transition.
• This documents the caller-trigger requirement inline in the workflow (per the suggested fix options).

Non-blocking (not actioned)

🔵 reviewer_display hoisting (Jerry-Xin, yujiawei): Cosmetic; current behavior is correct and the duplication is minor. Deferring.

Verification

+41 / -5 lines
Step 0 isDraft query runs before: # --- Step 1: Determine the current sprint iteration ---
isDraft removed from Step 2 pr_query ✓
Stale if pr.get("isDraft"): block removed ✓

Ready for re-review.

[lml2468]

Re-review — PR #51 (.github) @ d76a5d6

Verdict: APPROVED

Previous blocking issues (flagged by Allen + yujiawei, missed by me):

1. ✅ Fixed: draft check moved to Step 0 — separate lightweight GraphQL query for isDraft runs BEFORE Sprint/project-board resolution. Draft PRs exit immediately, never blocked by board-health failures.
2. ✅ Addressed: status caching concern documented with clear ::notice:: annotations and inline comments requiring callers to include ready_for_review in pull_request_target types.

Additional quality:

• Proper error handling (try/except) for draft status query failure
• isDraft removed from the later PR data query — no redundancy
• Separate query is one extra API call for non-draft PRs — acceptable tradeoff for correctness

No remaining blocking or non-blocking issues.

[lml2468]

Re-review — PR #51 (.github) @ d76a5d6

Verdict: APPROVED

Previous blocking issues (flagged by Allen + yujiawei, missed by me):

1. ✅ Fixed: draft check moved to Step 0 — separate lightweight GraphQL query for isDraft runs BEFORE Sprint/project-board resolution. Draft PRs exit immediately, never blocked by board-health failures.
2. ✅ Addressed: status caching concern documented with clear ::notice:: annotations and inline comments requiring callers to include ready_for_review in pull_request_target types.

Additional quality:

• Proper error handling (try/except) for draft status query failure
• isDraft removed from the later PR data query — no redundancy
• Separate query is one extra API call for non-draft PRs — acceptable tradeoff for correctness

No remaining blocking or non-blocking issues.

[Jerry-Xin]

Summary: The PR is relevant to this repository, but the new draft-PR sprint skip can bypass required Sprint validation after a draft is marked ready.

🔴 Blocking

🔴 Critical — .github/workflows/reusable-check-sprint.yml:252 and .github/workflows/reusable-check-sprint.yml:279: the workflow exits successfully for draft PRs, but the published caller example still omits ready_for_review from pull_request_target types at .github/workflows/reusable-check-sprint.yml:62. Because marking a draft PR ready does not change the head SHA, a green “skipped” check from the draft state can remain the required status for the current commit and allow merge without Sprint validation. The new comment correctly describes the bypass risk, but the PR still ships the unsafe state. Fix by making the reusable workflow safe without relying on caller changes, or update/coordinate all callers and the documented usage before merging this behavior.

💬 Non-blocking

🟡 Warning — .github/workflows/reusable-check-sprint.yml:246: the comment says draft PRs are skipped before “any board-state queries,” which is true, but the workflow still requires a successful GitHub GraphQL API call and sufficient repository PR read access before skipping. Consider tightening the wording so operators do not expect draft PRs to pass when PROJECT_TOKEN is invalid or lacks private-repo PR read access.

✅ Highlights

The notification workflow cleanups preserve the existing API allowlist, input validation, sanitization, and retry behavior.

The reviewer fallback in .github/workflows/octo-pr-result-notify.yml:203 and .github/workflows/octo-pr-result-notify.yml:210 fixes the blank reviewer output without changing the reusable workflow interface.

[yujiawei]

Code Review — PR #51 (.github)

Independent review of the 4 reusable-workflow alignments. Verdict: Changes requested — one blocking merge-gate bypass in reusable-check-sprint.yml. The other three changes are safe.

---

Verification summary

File	Change	Result
octo-pr-review-feed.yml	drop opened/synchronize from ALLOWED_ACTIONS + msg	✅ Safe — matches caller triggers
octo-pr-result-notify.yml	drop pr_reopened, add reviewer fallback	✅ Safe (⚠️ rationale inaccurate)
reusable-check-sprint.yml	skip draft PRs (Step 0)	❌ Blocking — gate bypass on draft→ready
reusable-pr-contributor-welcome.yml	richer welcome message	✅ Safe

I confirmed caller behavior across all four consuming repos (octo-server, octo-web, openclaw-channel-octo, octo-deployment), since these reusable workflows are referenced as @main and the change goes live to every caller the instant it merges.

---

P1 (blocking) — Draft-skip creates a merge-gate bypass on the draft→ready transition

reusable-check-sprint.yml (lines 652–691) now returns success (sys.exit(0), "✅ Skipping Sprint check for draft PR") for draft PRs. check-sprint / check-sprint is a required status check, and GitHub records its result against the head commit SHA.

The problem is that the safety of this skip depends entirely on the check re-running when a draft is promoted to ready — and no caller subscribes to that event. Verified trigger types:

• octo-server, octo-web: pull_request_target: types: [opened, synchronize, reopened, edited]
• openclaw-channel-octo, octo-deployment: pull_request_target: types: [opened, synchronize, reopened]

None include ready_for_review. Concrete bypass:

1. Open a PR as draft with no linked issue / no Sprint set. opened fires → check runs → "skip draft" → green cached on SHA X.
2. Click Ready for review (no new commit; head stays SHA X). ready_for_review is not in any caller's types, so no new run fires.
3. The PR is now mergeable, and the required check-sprint is green — from the draft skip, never having validated Sprint.
4. Merge succeeds, bypassing the gate entirely.

Opening-as-draft-then-marking-ready is a very common workflow, so this is not a corner case — it is a general way to defeat the Sprint gate for any PR that was ever a draft. This is a net-new false-pass: previously a draft with no Sprint produced a red check, so the persisted status could not be a spurious green.

The author is aware (the NOTE at lines 657–663 and the ::notice:: at 688–690 both say callers must add ready_for_review). But the PR does not make that change, and because callers pin @main, the bypass ships live the moment this merges — there is no flag or staged rollout protecting the window.

Required to unblock — do both, atomically with this change:

• Add ready_for_review to the pull_request_target types of all four caller check-sprint.yml workflows (and ideally converted_to_draft for completeness). These should land before or together with this reusable change, not "tracked separately."
• Recommended hardening so the gate is correct even if a caller's types drift again: also enable "Require branches to be up to date before merging" on main branch protection in each consuming repo (already recommended in the file header for the sprint-rollover case; it closes this case too).

A code-only note in the reusable workflow is not sufficient when the unsafe state is reachable by every existing caller on merge.

---

P2 — octo-pr-result-notify.yml: reviewer fallback rationale is factually wrong

The added reviewer_display = reviewer if reviewer else "(unknown)" (lines 204, 211) is harmless defensive code, but the PR description's justification — "when reviewer input is empty (current callers don't pass it)" — is incorrect. All four callers' review-result job pass:

reviewer: ${{ github.event.review.user.login }}

So the (unknown) branch is effectively unreachable in practice, and the "Follow-up" item ("per-repo callers need reviewer: added") describes work that is already done. No code change required, but please correct the PR description so it doesn't create a phantom follow-up task. (The fallback itself is fine to keep as belt-and-suspenders.)

---

Confirmed-safe changes

• octo-pr-review-feed.yml — dropping opened/synchronize from ALLOWED_ACTIONS and collapsing the message branch matches the caller, which triggers only on types: [ready_for_review, review_requested] and gates with if: ${{ !github.event.pull_request.draft }}. Pure dead-code removal. ✅
• octo-pr-result-notify.yml — dropping pr_reopened is safe: the caller's pr-result job only ever computes pr_merged/pr_closed, and review-result only review_approved/review_changes_requested. Unreachable kind removed. ✅
• reusable-pr-contributor-welcome.yml — message-only change; CONTRIBUTING.md exists at main, idempotency marker preserved, no actions/checkout added, permissions unchanged. ✅

Security model (security-sensitive PR) — no new injection surface

The new draft query in reusable-check-sprint.yml is read-only, uses parameterized GraphQL variables (no string interpolation of PR-controlled data into the query), runs under permissions: {}, checks out no PR code, and fails closed (sys.exit(1)) if isDraft cannot be read. From a pull_request_target / pwn-request standpoint this is sound. The only security-relevant issue is the authorization/gate-bypass described in P1, not code execution or secret exposure.

Nits (non-blocking)

• Hardcoded Discord invite discord.gg/vj9Vsj9hSB can expire; prefer a vanity/permanent invite.
• A stray blank line was added at the Step-2 query block (cosmetic).

---

Items for a human to manually verify (security-sensitive): (1) that the four caller check-sprint.yml workflows are updated with ready_for_review before this merges, and (2) branch-protection "Require branches to be up to date before merging" is enabled on main in the consuming repos.

[lml2468]

Re-review — PR #51 v3 (.github) @ 70ca5a1

Verdict: APPROVED

Previous blocking issues status

1. ✅ Draft skip ordering (v1 → v2): fixed — draft check at Step 0 before Sprint resolution.
2. ✅ Caller example missing ready_for_review (v2 → v3): fixed — example now includes ready_for_review + converted_to_draft with clear comment explaining bypass risk.
3. ✅ reviewer_display duplication (v2 → v3): deduplicated — computed once before branches.
4. ✅ ::notice:: strengthened: now says "REQUIRED caller config" with explicit bypass explanation.

Design note on caller enforcement

The fundamental concern raised across all reviewers (Jerry-Xin, yujiawei, Allen) is that documentation ≠ enforcement — actual caller repos need ready_for_review added to their pull_request_target.types. This is correct, but:

• GitHub Actions reusable workflows cannot enforce caller trigger configuration — documentation is the strongest available mechanism
• The example in the comments now shows the correct configuration
• The ::notice:: annotations will surface in CI logs

Recommendation: merge this PR, then open follow-up PRs/issues for each caller repo to add ready_for_review and converted_to_draft to their check-sprint caller workflows. There is a brief risk window between this merge and caller updates where draft→ready transitions without new pushes could bypass Sprint check.

CI all green.

[Jerry-Xin]

Summary: The PR is relevant to this .github workflow repository, but the draft-skip change can weaken the Sprint gate for existing callers.

🔴 Blocking

• 🔴 Critical — .github/workflows/reusable-check-sprint.yml:283-290: draft PRs now exit 0, which creates a successful required check on the PR head SHA. Existing callers that still use the previous documented trigger set and do not include ready_for_review will not re-run this workflow when the PR is promoted from draft to ready, so that cached green result can bypass Sprint validation. The PR documents this risk, but because these reusable workflows are typically consumed via @main, merging this before caller workflows are updated creates a known gate bypass. Please make the migration safe in code or coordinate/version the rollout so no existing caller can receive a green draft-skip result without a guaranteed ready-state revalidation.

💬 Non-blocking

• 🔵 Suggestion — .github/workflows/reusable-check-sprint.yml:58-68: the updated inline caller example is useful, but this migration requirement is important enough to also belong in any central operational docs used by downstream repositories.

✅ Highlights

• The PR is in scope for this repository.
• The notification simplifications in octo-pr-review-feed.yml and octo-pr-result-notify.yml align with the stated event contracts.
• YAML parsing and embedded Python/JavaScript syntax checks passed locally.

[yujiawei]

Code Review — PR #51 (.github)

Re-review of head 70ca5a1. Two of the prior round's items are now resolved; the blocking merge-gate bypass remains unaddressed in code and is actually live (not latent) for at least one consuming repo. Requesting changes.

What changed since the last review (d76a5d6 → 70ca5a1)

Item	Status
reviewer_display hoisted out of the per-branch bodies into input parsing (octo-pr-result-notify.yml)	✅ Resolved (cosmetic nit)
Draft-skip runs before sprint/board resolution (Step 0 at L250, before Step 1 at L292)	✅ Resolved
Gate bypass on draft→ready (the blocking P1)	❌ Not resolved — only the header usage example and ::notice:: wording were updated; no caller workflow changed, and the reusable workflow is still not safe-by-default

Verification summary

File	Change	Result
octo-pr-review-feed.yml	narrow ALLOWED_ACTIONS to {ready_for_review, review_requested}	✅ Safe
octo-pr-result-notify.yml	drop pr_reopened; add + hoist reviewer_display	✅ Safe
reusable-pr-contributor-welcome.yml	richer welcome message	✅ Safe
reusable-check-sprint.yml	draft-PR skip (Step 0)	❌ Blocking — see P1

P1 (blocking) — Draft-skip ships a live merge-gate bypass on the draft→ready transition

reusable-check-sprint.yml returns success (sys.exit(0), "✅ Skipping Sprint check for draft PR") for draft PRs. GitHub records a required check's result against the head commit SHA, and marking a draft "Ready for review" does not change the SHA. So the safety of this skip depends entirely on the check re-running on ready_for_review — which requires every caller to subscribe to that event.

It does not. Verified current caller trigger types (all reference the reusable workflow @main, so this change goes live to them the instant it merges):

• octo-server, octo-web: types: [opened, synchronize, reopened, edited]
• openclaw-channel-octo, octo-deployment: types: [opened, synchronize, reopened]

None include ready_for_review. Concrete bypass:

1. Open a PR as draft with no Sprint. opened fires → check runs → "skip draft" → green cached on SHA X.
2. Click Ready for review (no new commit; head stays SHA X) → ready_for_review is not in any caller's types → no re-run.
3. The now-ready PR is mergeable with a green check-sprint that came from the draft skip and never validated Sprint.

This is a net-new false-pass: before this PR a draft with no Sprint produced a red check, so no spurious green could persist.

This is LIVE, not latent

The prior round characterized this as latent because check-sprint was not a required check in the repos examined via rulesets. That missed classic branch protection: openclaw-channel-octo enforces check-sprint / check-sprint as a required status check with strict: false (GET /repos/Mininglamp-OSS/openclaw-channel-octo/branches/main/protection/required_status_checks → contexts: ["check-sprint / check-sprint"]), and its caller triggers on [opened, synchronize, reopened] only. So for that repo the bypass is immediately exploitable on merge, and strict: false means "require branches up to date" does not close it either.

The latest commit does not fix this

Commit 70ca5a1 updated the documented header example (line ~62) to recommend ready_for_review / converted_to_draft, and reworded the runtime ::notice::. Both are documentation. The four actual caller check-sprint.yml workflows are unchanged, and the reusable workflow still emits a green pass for drafts. A code-only note is not sufficient when the unsafe state is reachable by every existing caller the moment this merges.

Required to unblock (do one of these atomically with this change)

• Update the callers: add ready_for_review (and ideally converted_to_draft) to the pull_request_target.types of all four caller check-sprint.yml workflows, landing before or together with this reusable change — not "tracked separately." octo-cli already does this correctly and can serve as the template. OR
• Make the reusable workflow safe-by-default: have the draft branch produce a non-green outcome (neutral/pending/failure) instead of exit 0, so a skipped draft cannot satisfy a required check after promotion to ready.
• Recommended hardening regardless: enable "Require branches to be up to date before merging" (strict: true) on main in the consuming repos; this also closes the documented sprint-rollover stale-green case.

Confirmed-safe changes

• octo-pr-review-feed.yml — dropping opened/synchronize matches caller triggers ([ready_for_review, review_requested] + draft if: guard); unrecognized actions still hit the sys.exit(0) guard. Pure dead-code removal. ✅
• octo-pr-result-notify.yml — dropping pr_reopened is safe (no caller emits it); reviewer_display (unknown) fallback is defensive-only and the hoist is correct. ✅
• reusable-pr-contributor-welcome.yml — message-only; idempotency marker <!-- octo-pr-welcome:v1 --> preserved, CONTRIBUTING.md exists, no actions/checkout added, permissions unchanged. ✅

Security model (security-sensitive PR)

The new draft query is read-only, uses parameterized GraphQL variables (no interpolation of PR-controlled data), runs under permissions: {}, checks out no PR code, and fails closed (sys.exit(1)) if isDraft cannot be read. No new code-execution or secret-exposure surface. The only security-relevant issue is the authorization / gate-bypass in P1.

Items for a human to manually verify

1. Whether the four caller check-sprint.yml workflows are updated with ready_for_review before this merges (mandatory given the live bypass in openclaw-channel-octo).
2. Branch-protection strict ("up to date before merging") on main in the consuming repos.

Verdict

CHANGES_REQUESTED. Three of four changes are clean. The draft-skip placement fix is good, but the blocking gate-bypass from the prior round is unaddressed in code (only docs changed) and is live for openclaw-channel-octo. Resolve by updating callers atomically with this change, or by making the draft skip return a non-green outcome.

[lml2468]

Companion caller PRs — ready for merge

All 8 caller repos that were missing ready_for_review in their check-sprint.yml trigger types now have companion PRs:

Repo	PR	Status
openclaw-channel-octo (⚠️ live bypass — required check)	#49	Open
octo-server	#212	Open
octo-web	#180	Open
octo-admin	#67	Open
octo-deployment	#99	Open
octo-smart-summary	#49	Open
octo-lib	#54	Open
octo-adapters	#106	Open

Recommended merge order: merge all 8 caller PRs first → then merge this PR.

This addresses the blocking P1 raised by @Jerry-Xin and @yujiawei: the draft-skip change will not ship until all callers have ready_for_review in their trigger types, eliminating the gate bypass on draft→ready transitions.

The 7 repos that already had ready_for_review (octo-cli, octo-ios, octo-android, octo-version-sync, octo-daemon-cli, octo-im, claw-channel-octo) required no changes.

[Jerry-Xin]

Summary: The PR is in scope for Mininglamp-OSS/.github and the workflow changes align with the stated reusable-workflow design intent.

💬 Non-blocking

🟡 Warning — .github/workflows/reusable-check-sprint.yml:263: draft skipping depends on every caller adding ready_for_review to pull_request_target.types. The workflow documents this clearly and emits a notice, so this is not blocking, but downstream caller rollout should be tracked because the reusable workflow cannot enforce caller trigger configuration by itself.

✅ Highlights

🔵 .github/workflows/octo-pr-review-feed.yml:63: narrowing ALLOWED_ACTIONS to ready_for_review and review_requested removes contradictory dead paths cleanly.

🔵 .github/workflows/octo-pr-result-notify.yml:173: reviewer_display gives a defensive fallback without changing the current happy path.

🔵 .github/workflows/reusable-check-sprint.yml:250: placing the draft check before sprint/project resolution matches the intended “formal PR only” behavior and avoids board-health failures for drafts.

🔵 .github/workflows/reusable-pr-contributor-welcome.yml:109: the welcome message adds useful contributor links while preserving the existing idempotency marker.

[ploy-elison]

Summary: v3 addresses all prior blocking items cleanly. The four workflow changes are in scope and aligned with design intent. Approving.

Previous blocking items — all resolved

Round	Issue	Status
v1	Draft skip after Sprint resolution → board failures on drafts	✅ Fixed: Step 0 runs isDraft check before any project queries
v2	Caller example missing ready_for_review	✅ Fixed: example + inline comment + ::notice:: all updated
v2	reviewer_display computed inside branches	✅ Fixed: hoisted to input-parsing section
v2→v3	::notice:: too soft	✅ Fixed: upgraded to "REQUIRED caller config" with bypass risk explanation

Caller readiness audit

Checked all 13 consuming repos. Current state:

• ✅ Already have ready_for_review: octo-ios, octo-android, octo-cli (these 3 also have if: !github.event.pull_request.draft at the caller level — belt and suspenders, works correctly with the new callee-side check)
• ⚠️ Need ready_for_review added: octo-web, octo-server, octo-smart-summary, octo-deployment, octo-lib, openclaw-channel-octo, octo-admin, octo-matter, octo-speech, octo-adapters (10 repos)

The reusable workflow documents the requirement clearly (header comment + runtime notice), and GitHub Actions architecture does not allow a callee to enforce caller trigger types. This is the strongest mitigation available. The 10 repos need follow-up PRs to add ready_for_review (and ideally converted_to_draft) to their pull_request_target.types.

💬 Non-blocking

• 🟡 Recommend opening a tracking issue or batch PR for the 10 caller repos immediately after merge to minimize the window where draft→ready bypasses the gate.
• 🟡 octo-ios/android/cli have their own if: !draft guard at the caller level. After this PR lands, the callee handles draft skipping, so the caller-side guard becomes redundant (harmless but noisy). Consider removing it in a follow-up cleanup.

✅ Highlights

• Draft check placement (Step 0) is architecturally correct — avoids false negatives from board-health issues on draft PRs.
• reviewer_display fallback is a good defense-in-depth pattern.
• Removing opened/synchronize from review-feed's ALLOWED_ACTIONS and pr_reopened from result-notify's ALLOWED_KINDS eliminates dead code without behavioral regression.
• Welcome message additions (Contributing Guide, Discord, CI reminder) are user-friendly for new contributors.