fix(ci): finalize PR notification design (remove octo-pr-feed + fix result-notify)

[lml2468]

Summary

Finalizes the PR notification design. Clean, no-overlap ownership:

Workflow	Trigger	Sends to
octo-pr-result-notify.yml	[closed] + pull_request_review: [submitted]	global pr-feed (vars.OCTO_PR_FEED_GROUP_ID with fallback)
octo-pr-review-feed.yml	[ready_for_review, review_requested]	project group (repo-specific)

Design decisions (per product owner)

• opened/reopened events are not monitored — only terminal states (merged/closed) and review outcomes
• octo-pr-feed.yml is deleted — it was added in a prior pass but is not needed under the final design

Changes

• octo-pr-feed.yml: deleted
• octo-pr-result-notify.yml:
  • Trigger: [closed, reopened] → [closed] (terminal state only)
  • event_kind: simplified to pr_merged | pr_closed
  • feed_group_id: ${{ vars.OCTO_PR_FEED_GROUP_ID || '1c303c142e9840f2a9b46c10b0972e8d' }} (org variable with safe fallback)

No application code modified.

[Jerry-Xin]

Verdict: APPROVED

Clean follow-up to #65. Two files, +4 −29, all CI workflow changes — no application code.

What changed:

1. octo-pr-feed.yml deleted — removes opened event notifications per product owner decision (only terminal states + review outcomes monitored)
2. octo-pr-result-notify.yml:
   • Trigger narrowed [closed, reopened] → [closed] (terminal state only)
   • event_kind ternary simplified accordingly (merged == true ? pr_merged : pr_closed)
   • feed_group_id added to both pr-result and review-result jobs: ${{ vars.OCTO_PR_FEED_GROUP_ID || '1c303c142e9840f2a9b46c10b0972e8d' }}

Verified against callee contract:

• Mininglamp-OSS/.github/.github/workflows/octo-pr-result-notify.yml declares feed_group_id as type: string, required: false, default: '1c303c142e9840f2a9b46c10b0972e8d' — caller's fallback matches exactly.

CI: check-sprint and welcome failures are non-blocking chore checks; build, CodeQL, actionlint, sanity all pass or pending.

This directly addresses the blocking feedback from #65's last round (explicit feed_group_id with org variable + fallback, description aligned with implementation). Ship it.

[yujiawei]

Code Review — PR #66 (octo-admin)

Summary

This PR finalizes the PR-notification design with two changes:

1. Deletes .github/workflows/octo-pr-feed.yml (the opened-event notifier).
2. Tightens .github/workflows/octo-pr-result-notify.yml: narrows the trigger to terminal state only, simplifies event_kind, and threads a configurable feed_group_id into both jobs.

No application code is touched. I verified the change against the shared reusable workflow it calls (Mininglamp-OSS/.github/.github/workflows/octo-pr-result-notify.yml@main) and the sibling notifier (octo-pr-review-feed.yml) to confirm cross-file behavior. The change is correct and safe to merge.

Verification

• ✅ event_kind simplification is correct. Old expression branched on github.event.action, which was required when the trigger included reopened. With the trigger narrowed to [closed], action is always closed, so merged == true && 'pr_merged' || 'pr_closed' produces exactly the right kind. Both pr_merged and pr_closed are in the reusable workflow's ALLOWED_KINDS, so no event will be silently dropped. (octo-pr-result-notify.yml:21)
• ✅ Removing reopened is consistent. The pr_reopened branch is gone and no longer reachable; the reusable workflow still lists pr_reopened in ALLOWED_KINDS but that is harmless dead config living in the shared repo.
• ✅ feed_group_id fallback is valid. ${{ vars.OCTO_PR_FEED_GROUP_ID || '1c303c142e9840f2a9b46c10b0972e8d' }} — the literal fallback is a 32-char lowercase hex string and passes the reusable workflow's require_group_id regex ([0-9a-f]{32}). It also matches the reusable workflow's own default, so behavior is unchanged when the org var is unset. (octo-pr-result-notify.yml:23,42)
• ✅ Security posture preserved. Both jobs run under permissions: {}, perform no checkout, and pass metadata-only inputs that the reusable workflow sanitizes (sanitize_text, length caps). The review-result job retains the head.repo.full_name == github.repository guard against fork-based review abuse. pull_request_target is acceptable here precisely because no PR code is executed.
• ✅ Coverage handoff is clean. Dropping octo-pr-feed.yml removes opened-event notifications; ready_for_review / review_requested remain covered by octo-pr-review-feed.yml, matching the stated "terminal states + review outcomes only" design.

Findings

No P0/P1 issues.

Suggestions (non-blocking)

• P2 — Notification gap for plain opened PRs. With octo-pr-feed.yml deleted, a non-draft PR that is opened without a requested reviewer fires neither ready_for_review (only triggers on draft→ready) nor review_requested, so it produces no feed entry until it is closed/merged. The PR body states this is intentional per the product owner; flagging only so the trade-off is on record.
• P2 — feed_group_id fallback duplicates the reusable default. The hardcoded '1c303c1…' mirrors the reusable workflow's own default value. This is defensible for explicitness, but it means the literal now lives in two repos; if the canonical group ID ever changes, both must be updated. Relying on vars.OCTO_PR_FEED_GROUP_ID plus the reusable default would centralize it. Minor.
• P2 — Consistency: the pr-result (closed) job does not carry the same head.repo.full_name == github.repository guard that review-result has. Risk is negligible (terminal event, empty permissions, no checkout), but adding it would make the two jobs symmetric. Optional.

Verdict

Correct, minimal, and well-scoped. The event_kind logic is sound given the narrowed trigger, the group-id fallback validates, and the ownership split between the result-notify and review-feed workflows is clean. Approving.

[lml2468]

✅ APPROVED — clean finalization of the PR notification design.

What changed:

• octo-pr-feed.yml deleted — opened/reopened events no longer monitored per design
• octo-pr-result-notify.yml: trigger narrowed to [closed] only; event_kind simplified; feed_group_id added with || fallback

Verified:

1. feed_group_id fallback logic — ${{ vars.OCTO_PR_FEED_GROUP_ID || '1c303c142e9840f2a9b46c10b0972e8d' }} correctly avoids the empty-string bypass that broke PR #65. If the org/repo variable is unset, the hardcoded default kicks in. No empty string will reach the reusable workflow.
2. Job conditionals — pr-result gates on event_name == 'pull_request_target', review-result gates on event_name == 'pull_request_review' + state filter (approved/changes_requested only). No cross-trigger leakage.
3. event_kind simplification — with reopened removed from triggers, the old ternary is dead code. New merged == true → pr_merged || pr_closed is correct and minimal.
4. Security — permissions: {}, metadata-only automation. No code execution from PR context.

Non-blocking:
🔵 labeler.yml:11 — stale comment still references deleted octo-pr-feed.yml:

# the pattern used by auto-add-to-project.yml and octo-pr-feed.yml.
Suggest cleaning up in a follow-up.