Skip to content

ci(octo-issue-feed): opt into webhook-only triage#62

Merged
lml2468 merged 1 commit into
mainfrom
ci/octo-issue-feed-im-toggle
Jun 27, 2026
Merged

ci(octo-issue-feed): opt into webhook-only triage#62
lml2468 merged 1 commit into
mainfrom
ci/octo-issue-feed-im-toggle

Conversation

@lml2468

@lml2468 lml2468 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

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.

@lml2468 lml2468 requested a review from a team as a code owner June 27, 2026 07:49
@lml2468 lml2468 merged commit 3555e66 into main Jun 27, 2026
4 of 5 checks passed
@lml2468 lml2468 deleted the ci/octo-issue-feed-im-toggle branch June 27, 2026 07:49
@github-actions github-actions Bot added the size/XS PR size: XS label Jun 27, 2026

@mochashanyao mochashanyao left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. Caller (octo-cli octo-issue-feed.yml): passes enable_im_notify: false to Mininglamp-OSS/.github/.github/workflows/octo-issue-notify.yml@v1
  2. Callee at v1 (tag → commit 135e280f): the workflow_call.inputs section defines 8 inputs (repo_name, issue_number, issue_title, issue_url, issue_author, event_action, api_base_url, feed_group_id). enable_im_notify is NOT among them.
  3. PR#80 merge commit (8ade879f): adds enable_im_notify input + conditional IM logic. v1 tag is 3 commits behind this merge — confirmed via gh api repos/Mininglamp-OSS/.github/compare/135e280f...8ade879fahead_by: 3.
  4. Runtime effect: GitHub Actions rejects undefined workflow_call inputs. When an issue is opened/reopened, the notify job 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).
  5. 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 if im_message ever becomes empty/falsy for a legitimate reason — the || branch would fire even when enable_im_notify is true. Low risk for the current code path (message is always built for opened events), 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 yujiawei left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 PR

But the enable_im_notify input is not defined in the reusable workflow at @v1:

  • The v1 tag resolves to commit 135e280 (2026-06-09). At that ref, on.workflow_call.inputs ends at feed_group_id — there is no enable_im_notify.
  • The input exists only on the main branch of Mininglamp-OSS/.github, introduced by Mininglamp-OSS/.github#80 (merge commit 8ade879, merged 2026-06-27 — 18 days after the v1 commit).
  • git compare 8ade879...135e280behind_by: 3, ahead_by: 0: PR #80's merge commit is not an ancestor of v1; the v1 tag 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_notify is 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:

  1. Cut a release of Mininglamp-OSS/.github that includes #80 and move/v1 (or a new v1.2.0) to point at it, then keep @v1 here. Re-pointing the floating v1 tag is the cleanest since the caller already pins @v1.
  2. Or pin the caller to a ref that actually contains #80. Today that is only main (@main or @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 file on 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 Jerry-Xin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: false

But 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 of enable_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 v1 tag 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 + keeping TRIAGE_WEBHOOK_URL correctly expresses "webhook-only triage" once the reusable supports the input.
  • The check-sprint / check-sprint CI 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS PR size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants