ci(octo-issue-feed): opt into webhook-only triage#62
Conversation
mochashanyao
left a comment
There was a problem hiding this comment.
[Octo-Q · automated review]
Verdict: Request changes — blocking findings below (data-flow traced).
octo-cli PR#62 Review Report
PR: #62
Title: ci(octo-issue-feed): opt into webhook-only triage
Head SHA: 421d899
State: MERGED
Reviewer: Octo-Q (automated review)
1. Summary
Single-file CI change: adds enable_im_notify: false to the reusable workflow call in .github/workflows/octo-issue-feed.yml to opt out of IM notifications and use webhook-only triage. Backed by Mininglamp-OSS/.github#80. Also removes the trailing newline at EOF.
2. Verification
| Item | Status | Evidence |
|---|---|---|
enable_im_notify input passed correctly |
❌ | Passed to @v1 which does NOT define this input |
| Backing PR#80 merged | ✅ | Merge commit 8ade879f on 2026-06-27T07:23:41Z |
| v1 tag updated to include PR#80 | ❌ | v1 tag at 135e280f, 3 commits behind merge commit |
| Trailing newline | Removed, leaves file without POSIX trailing newline |
3. Findings
P1 — v1 tag not bumped; enable_im_notify is undefined in the called reusable workflow
Diff-scope: new — this PR introduces a caller-side input that the callee at @v1 does not recognize.
Data flow trace:
- Caller (octo-cli
octo-issue-feed.yml): passesenable_im_notify: falsetoMininglamp-OSS/.github/.github/workflows/octo-issue-notify.yml@v1 - Callee at v1 (tag → commit
135e280f): theworkflow_call.inputssection defines 8 inputs (repo_name,issue_number,issue_title,issue_url,issue_author,event_action,api_base_url,feed_group_id).enable_im_notifyis NOT among them. - PR#80 merge commit (
8ade879f): addsenable_im_notifyinput + conditional IM logic. v1 tag is 3 commits behind this merge — confirmed viagh api repos/Mininglamp-OSS/.github/compare/135e280f...8ade879f→ahead_by: 3. - Runtime effect: GitHub Actions rejects undefined
workflow_callinputs. When an issue is opened/reopened, thenotifyjob will fail at dispatch before any step runs. Both IM notification AND triage webhook are broken — strictly worse than the pre-PR state (which at least delivered both signals). - Even if the input were silently ignored (best-case interpretation), the IM message would still fire because the conditional logic (
inputs.enable_im_notify && ... || '') is absent at v1, so the PR's stated goal is unachieved.
Severity: P1 per R1 — a previously working path (issue notification + triage) becomes non-functional. The issue-triggered workflow will fail on every invocation until v1 is bumped.
Fix: Update the v1 tag in Mininglamp-OSS/.github to include commit 8ade879f (PR#80 merge). Then re-verify that the workflow succeeds on a test issue.
P2 — Trailing newline removed
Diff-scope: new — this PR removes the POSIX-required trailing newline.
The diff's last hunk replaces the trailing newline with \ No newline at end of file. This is a minor lint issue; some YAML parsers and git diff itself flag this. No functional impact.
Fix: Restore the trailing newline after TRIAGE_WEBHOOK_URL.
4. Extra Findings
- The backing PR#80 uses a
&&/||ternary pattern:inputs.enable_im_notify && steps.build.outputs.im_message || ''. This works correctly for the intended case (non-empty message strings are truthy), but is fragile ifim_messageever becomes empty/falsy for a legitimate reason — the||branch would fire even whenenable_im_notifyis true. Low risk for the current code path (message is always built foropenedevents), but worth noting for future callers.
5. Data Flow Trace
| Consumed data | Upstream source | Reaches consumer? |
|---|---|---|
enable_im_notify: false |
Caller YAML literal | ❌ — input undefined at @v1; workflow dispatch fails |
im_message (built by Python step) |
steps.build.outputs.im_message |
❌ — if dispatch fails, step never runs |
webhook_payload (built by Python step) |
steps.build.outputs.webhook_payload |
❌ — same |
TRIAGE_WEBHOOK_URL |
Repo secret | ❌ — same |
6. Blind-spot Checklist (R5)
- C1 双路径 parity: N/A — single workflow call, no symmetric paths.
- C2 control-flow ordering: N/A — no nesting/reuse concerns in this caller.
- C3 授权边界: N/A — no permission/capability changes.
- C4 授权生命周期: N/A — no auth changes.
- C5 build ≠ runtime: HIT — the workflow YAML is syntactically valid (no build-time error), but at runtime the undefined input causes dispatch failure. This is the core P1 finding.
- C6 治理文档自洽性: N/A — no policy/governance docs changed.
7. Cross-round Blocker Review (R6)
N/A — automated review review.
[Octo-Q] verdict: REQUEST_CHANGES — P1: v1 tag not bumped after PR#80 merge; enable_im_notify is undefined at the called reusable workflow, causing the notify job to fail on every issue event (both IM and triage broken). Must bump v1 tag to include PR#80 merge commit 8ade879f before this caller-side change can work.
yujiawei
left a comment
There was a problem hiding this comment.
Code Review — PR #62 (octo-cli)
Verdict: CHANGES_REQUESTED — the change references a reusable-workflow input that does not exist at the pinned ref, which makes the workflow invalid at runtime.
Spec compliance ✅
The stated intent (opt this repo out of the redundant IM ping by setting enable_im_notify: false) is exactly what the one-line diff expresses. No over-scope, no unrelated changes. Intent is correct; the problem is in how the dependency is wired, covered below.
Blocking issue (P0 / build break)
.github/workflows/octo-issue-feed.yml calls the reusable workflow pinned at a tag:
uses: Mininglamp-OSS/.github/.github/workflows/octo-issue-notify.yml@v1
with:
...
enable_im_notify: false # <-- new input added by this PRBut the enable_im_notify input is not defined in the reusable workflow at @v1:
- The
v1tag resolves to commit135e280(2026-06-09). At that ref,on.workflow_call.inputsends atfeed_group_id— there is noenable_im_notify. - The input exists only on the
mainbranch ofMininglamp-OSS/.github, introduced byMininglamp-OSS/.github#80(merge commit8ade879, merged 2026-06-27 — 18 days after thev1commit). git compare 8ade879...135e280→behind_by: 3, ahead_by: 0: PR #80's merge commit is not an ancestor ofv1; thev1tag was never re-pointed to include it.- None of the released tags (
v1.0.0,v1.1.0,v1.1.1,v1.1.2) contain the input either.
GitHub Actions treats an undeclared input passed to a reusable workflow as a hard validation failure, not a no-op. The run fails with:
Invalid workflow file: The workflow is not valid. ... Invalid input,
enable_im_notifyis not defined in the referenced workflow.
Impact: every time an issue is opened in this repo, octo-issue-feed.yml will fail to validate and the notify/triage job will not run — a regression of the entire issue-feed path, not a silent no-op. So this change does the opposite of "webhook-only triage": it disables all triage for new issues until the pin is fixed.
How to fix
Pick one, in order of preference:
- Cut a release of
Mininglamp-OSS/.githubthat includes #80 and move/v1(or a newv1.2.0) to point at it, then keep@v1here. Re-pointing the floatingv1tag is the cleanest since the caller already pins@v1. - Or pin the caller to a ref that actually contains #80. Today that is only
main(@mainor@8ade879), which is not a released tag — acceptable only as a temporary measure and worth a comment explaining why it's not on@v1.
Either way, this PR should not land (or should be reverted/fixed-forward, since it is already merged) until the referenced ref provides the input.
Minor (P2 / nit)
- The file now ends without a trailing newline (
\ No newline at end of fileon the last line). Restoring the trailing newline keeps POSIX-clean diffs.
Coverage note
Reviewed: the one-line caller diff, the full octo-issue-feed.yml, the reusable octo-issue-notify.yml at @v1 vs main, the v1 tag target, and the backing PR .github#80. Not independently exercised: a live Actions run (the conclusion is based on GitHub's documented validation behavior for undeclared reusable-workflow inputs). The IM allowlist / transport behavior inside the reusable workflow is out of scope for this caller-side change.
Jerry-Xin
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES — the new enable_im_notify: false input is not accepted by the pinned reusable workflow version, which will hard-fail the workflow.
🔴 Blocking
1. enable_im_notify is not defined in octo-issue-notify.yml@v1 → workflow will fail with an invalid-input error.
This caller pins:
uses: Mininglamp-OSS/.github/.github/workflows/octo-issue-notify.yml@v1
with:
...
enable_im_notify: falseBut the enable_im_notify input only exists on the reusable workflow's main branch (added today in .github PR #80, commit 8ade879f). It has not been released to any tag:
@v1→ 0 occurrences ofenable_im_notify@v1.0.0,@v1.1.0,@v1.1.1,@v1.1.2→ all 0 occurrences
The v1 tag still resolves to a commit (135e280f) whose workflow_call.inputs are only: repo_name, issue_number, issue_title, issue_url, issue_author, event_action, api_base_url, feed_group_id.
Passing an undeclared input to a reusable workflow is a hard error in GitHub Actions ("Invalid input, enable_im_notify is not defined in the referenced workflow"). Net effect: the Octo Issue Feed workflow stops working entirely — neither IM notify nor webhook triage would run, which is the opposite of the PR's "webhook-only triage" goal.
Fix — pick one:
- Release the reusable change first, then bump this caller to that tag (e.g.
@v1.2.0), or - Temporarily pin to
@main/ the input-bearing SHA (8ade879f) until a tag is cut, or - Hold this PR until the reusable's
v1tag is moved to include the input.
💬 Non-blocking
- Missing trailing newline on the last line (
TRIAGE_WEBHOOK_URL: ${{ secrets.TRIAGE_WEBHOOK_URL }}\ No newline at end of file). Add a trailing newline to keep POSIX-clean and avoid noisy future diffs.
✅ Notes
- Intent is sound:
enable_im_notify: false+ keepingTRIAGE_WEBHOOK_URLcorrectly expresses "webhook-only triage" once the reusable supports the input. - The
check-sprint / check-sprintCI failure is unrelated to this change (project-board/sprint freshness check), not a regression from this YAML. permissions: {}and secret passing are unchanged and correct.
Adds enable_im_notify: false to opt this repo out of the redundant IM ping (multica triage agent creates a tracked issue). Backed by Mininglamp-OSS/.github#80.