feat(octo-issue-notify): add enable_im_notify input to allow callers to skip IM push

[lml2468]

Summary

The reusable workflow `octo-issue-notify.yml` currently always pushes to Octo IM and fires the triage webhook (when `TRIAGE_WEBHOOK_URL` is set). For repos that have moved to a richer external triage agent (e.g. a multica autopilot that creates a tracked issue and dispatches via leader), the IM push becomes a duplicate ping.

This PR adds an opt-in switch.

Change

• New input `enable_im_notify` (boolean, optional, default `true`).
• When `false`, the `Send` step passes an empty `im-message` to the `octo-notify` composite action.

Per the composite action's contract, an empty `im-message` disables IM mode entirely; webhook mode is independent and unaffected.

Backward compatibility

`default: true` preserves existing behavior for all current callers. Repos that want webhook-only triage opt in explicitly:

```yaml
jobs:
notify:
uses: Mininglamp-OSS/.github/.github/workflows/octo-issue-notify.yml@v1
with:
repo_name: ${{ github.event.repository.name }}
issue_number: ${{ github.event.issue.number }}
# ... other inputs ...
enable_im_notify: false # ← new opt-out
secrets:
OCTO_BOT_TOKEN: ${{ secrets.OCTO_BOT_TOKEN }}
TRIAGE_WEBHOOK_URL: ${{ secrets.TRIAGE_WEBHOOK_URL }}
```

Test plan

• Merge this PR.
• In a follow-up, opt one caller (e.g. `octo-spec/.github/workflows/octo-issue-feed.yml`) into `enable_im_notify: false`.
• Open a test issue in that repo. Expect:
  • GitHub Actions run for `Octo Issue Feed` — green.
  • Octo IM group — no message (the goal).
  • `TRIAGE_WEBHOOK_URL` endpoint — receives one POST with the expected payload.
• Confirm a non-opted-in repo still pushes to IM as before.

Rollback

Revert this PR; default behavior is unchanged either way.

[OctoBoooot]

Review: feat(octo-issue-notify): add enable_im_notify input to skip IM push (#80)

Verdict: Approve with comments.

Risk tier: infra (reusable GitHub Actions workflow — fan-out blast radius across every caller). CI green (actionlint + no-tabs). 1 file, +10/-1. Backward-compatible additive input.

Verification

• Default-preserves-behavior: enable_im_notify is boolean, required: false, default: true. Every existing caller that doesn't pass it keeps full IM-push behavior. Backward-compat 1:1.
• The gate expression is correct: im-message: ${{ inputs.enable_im_notify && steps.build.outputs.im_message || '' }} — when the input is true, evaluates to the built message; when false, the && short-circuits to the falsy branch → ''. Verified against the composite action's contract at .github/actions/octo-notify/action.yml:37,131: if im_message: gates IM mode, so empty string = IM mode off. Sound.
• Webhook independence holds: webhook-url/webhook-payload wiring is untouched, so webhook mode fires regardless of enable_im_notify. Matches the PR's claim.

Minor (worth a guard; not blocking)

• enable_im_notify: false + unset TRIAGE_WEBHOOK_URL → the notify job fails. Construct it: caller opts out of IM, but the repo has no TRIAGE_WEBHOOK_URL secret. The Send step then passes im-message: '' AND webhook-url: ''. The composite action at action.yml:171-173 hits the "neither mode" branch → print('::error::octo-notify invoked with neither im-message nor webhook-url'); sys.exit(2) → the Send step (and the job) goes red. So "disable IM" on a webhook-less repo turns a previously-green notify run into a failing one. Today that combination can't happen (IM always on), so this PR introduces the reachable path. Worth either: (a) a workflow-level guard that skips the Send step when both would be empty, or (b) a one-line note in the input description that enable_im_notify: false requires TRIAGE_WEBHOOK_URL to be set. The author's own test plan only exercises the with-webhook case, so this gap wouldn't surface there.

Praise

• The gate is placed at exactly the right layer — the reusable workflow's input boundary, leveraging the composite action's existing "empty im-message = IM off" contract rather than adding a new conditional code path inside the action. Reuses a documented behavior instead of inventing a parallel one.
• default: true + the explicit rollback note ("Revert this PR; default behavior is unchanged either way") is the right posture for a fan-out reusable workflow where you can't see all callers — opt-in, zero forced migration.
• The input description names when to flip it ("repo has already moved to an external triage agent and no longer wants duplicate IM pings") — operators get the decision criterion, not just the mechanism.

Out of scope (informational)

• The test plan is all manual post-merge checklist items (open a test issue, observe IM/webhook). For a reusable workflow this is reasonable since there's no unit-test harness for .github/workflows, but the "neither mode → job fails" edge above is exactly the case a manual plan tends to miss — calling it out so it's tested deliberately if the guard isn't added.

[yujiawei]

Code Review — PR #80 (.github)

Summary

This PR adds an opt-in enable_im_notify boolean input (default true) to the reusable octo-issue-notify.yml workflow. When false, the Send step passes an empty im-message to the octo-notify composite action, which disables IM mode while leaving the triage webhook untouched.

The change is minimal (one input + one expression edit), well-documented, and the design correctly reuses the existing composite-action contract rather than adding new branching logic.

Spec / intent compliance

✅ Matches stated intent.

• New input enable_im_notify (boolean, optional, default: true) — present and correct.
• default: true preserves behavior for all existing callers (with no input → true && im_message || '' → im_message, identical to prior behavior). Verified backward compatibility holds.
• When false → im-message becomes ''. Confirmed against the composite action: octo-notify only enables IM mode when im-message is non-empty (if im_message:), so an empty value cleanly disables IM push.
• Webhook mode is independent (if webhook_url:) and unaffected — confirmed.
• Scope is tight: no extra inputs, flags, or unrelated changes.

Correctness

The GitHub Actions ternary idiom A && B || C is safe here: B (steps.build.outputs.im_message) is only referenced when steps.build.outputs.skip != 'true', and in that path the build step always emits a non-empty message (emoji + repo + title + author + URL). So the middle term is never falsy, and the idiom can't accidentally collapse to '' when IM is enabled.

enable_im_notify is declared type: boolean on a workflow_call input, so GitHub validates/coerces it to a real boolean before the expression evaluates — the classic "string 'false' is truthy" trap that affects composite/JS action inputs does not apply to typed reusable-workflow inputs here.

No security regression: the new input only gates an existing, already-sanitized value; nothing new is interpolated into shell or JSON.

Findings

P2 (non-blocking) — silent footgun when IM is disabled and no webhook is configured.

If a caller sets enable_im_notify: false and does not supply TRIAGE_WEBHOOK_URL, then both im-message and webhook-url reach octo-notify empty. The composite action treats "neither mode" as a hard error:

::error::octo-notify invoked with neither im-message nor webhook-url
sys.exit(2)

So that misconfiguration produces a red workflow run on every opened issue rather than a clean no-op. This doesn't affect the intended use case (webhook-only triage always has a webhook) or any existing caller (default true), so it's advisory only. Two optional ways to harden it, if you want to remove the sharp edge:

• Document on the enable_im_notify input that disabling IM requires a configured TRIAGE_WEBHOOK_URL, or
• Add a guard so enable_im_notify: false + no webhook results in a clean skip instead of a failed run.

Neither is required to merge.

Verdict

No P0/P1 issues. Spec-compliant, backward-compatible, minimal, and correct. Approving; the P2 above is a suggestion, not a blocker.

[mochashanyao]

[Octo-Q · automated review]

Verdict: Approve — no blocking findings; notes below (data-flow traced).

---

.github PR#80 Review Report

PR: #80
Head SHA: face92593de9c09753e938641713f41626a7d215
Reviewer: Octo-Q (automated review)
Scope: automated review / one-file workflow change

---

1. Verification Summary

Check	Result	Evidence
Input schema (type, default, description)	✅	octo-issue-notify.yml L55–L63: boolean, default: true, clear description
Ternary expression correctness	✅	L159: ${{ inputs.enable_im_notify && steps.build.outputs.im_message || '' }} — when true returns im_message; when false returns ''
Empty-string contract in octo-notify action	✅	actions/octo-notify/action.yml: if im_message: guard — empty string is falsy in Python, skips IM mode entirely
Webhook mode independence	✅	webhook-url/webhook-payload passed separately, untouched by the change
Backward compatibility (default true)	✅	Existing callers get true && im_message || '' = im_message — identical to pre-PR behavior
Send step if gate	✅	if: steps.build.outputs.skip != 'true' — unchanged, still guards correctly

2. Findings

No P0 or P1 findings.

No P2 findings either. The change is minimal (10 additions, 1 deletion), well-scoped, and the data-flow contract is correctly honored end-to-end.

Data-Flow Trace (mandatory per rubric)

1. inputs.enable_im_notify — declared at L55, type: boolean, default: true. Caller-controlled.
2. steps.build.outputs.im_message — produced by the Python build step (L106–L142). When action == 'opened', it's set to the formatted notification string. When action != 'opened', skip='true' is emitted and the Send step is skipped entirely, so im_message is never consumed.
3. The ternary at L159: inputs.enable_im_notify && steps.build.outputs.im_message || '' — GitHub Actions &&/|| returns the operand value (not a boolean). When enable_im_notify=true, yields im_message; when false, yields ''.
4. octo-notify composite action — im-message lands in env: IM_MESSAGE. The Python script checks if im_message: (line ~78). Empty string is falsy → IM block skipped. Webhook block runs independently.

Every consumed datum traces back to a valid producer. No empty-value / short-circuit / swallowed-error traps found.

3. Suggestions

None required. Optional nit for the author's consideration (non-blocking):

• The PR body mentions a follow-up to opt one caller into enable_im_notify: false. That's fine as a separate PR — no coupling concern here.

4. Additional Observations

• The octo-notify action still receives im-token, im-group-id, and im-api-base when enable_im_notify: false. This is harmless — those inputs are only consumed inside the if im_message: block which is skipped. No wasted API calls or auth exposure.
• If a caller sets enable_im_notify: false AND omits TRIAGE_WEBHOOK_URL, the action would error with "neither im-message nor webhook-url" — but this is correct guard behavior (misconfiguration should fail loudly). The PR doesn't change this.

5. Data-Flow Backtrack Table

Consumed datum	Upstream source	Runtime value at consumption	Valid?
inputs.enable_im_notify	Workflow input declaration L55	true (default) or caller-supplied boolean	✅
steps.build.outputs.im_message	Python build step L134	Formatted notification string (when action=opened)	✅
Ternary result at L159	enable_im_notify && im_message || ''	im_message or ''	✅
env: IM_MESSAGE in octo-notify	inputs.im-message from caller	Passed through; if im_message: guards consumption	✅

6. Blind-Spot Checklist (R5)

• C1 — Dual-path parity: N/A. This is an additive opt-out toggle, not a symmetric add/remove pair.
• C2 — Control-flow ordering / nesting reuse: N/A. Single expression, no nested sanitization or reused cap logic.
• C3 — Authorization boundary ≠ capability boundary: N/A. No new endpoints or capability exposure.
• C4 — Authorization lifecycle / container-member state cascade: N/A. No auth changes.
• C5 — Build ≠ runtime path correctness: N/A. GitHub Actions YAML — no build artifacts or relative-path concerns.
• C6 — Governance / policy / security document self-consistency: N/A. This is a workflow code change, not a policy document.

7. Cross-Round Blocker Recheck (R6)

N/A — first review on this PR.

---

[Octo-Q] verdict: APPROVE — Clean, minimal, well-documented change. The enable_im_notify toggle correctly leverages the documented empty-string contract of the octo-notify composite action. Backward compatibility preserved via default: true. No P0/P1/P2 findings.

[Jerry-Xin]

The PR is in scope for Mininglamp-OSS/.github and the implementation matches the existing octo-issue-notify / octo-notify transport pattern.

💬 Non-blocking

• 🟡 Warning: .github/workflows/octo-issue-notify.yml (line 65) still requires OCTO_BOT_TOKEN even when enable_im_notify: false. That means webhook-only callers must still provide the IM token secret, although it will not be used. This does not break current callers, but it limits the “webhook-only” mode.
• 🔵 Suggestion: .github/workflows/octo-issue-notify.yml (line 159) works correctly with the composite action contract, but if enable_im_notify: false is used without TRIAGE_WEBHOOK_URL, octo-notify will fail with “neither im-message nor webhook-url”. That failure is reasonable, but a workflow-level validation message could make misconfiguration clearer.

✅ Highlights

• The new input defaults to true, preserving existing behavior.
• Passing '' for im-message correctly disables IM mode per .github/actions/octo-notify/action.yml (line 131).
• Webhook mode remains independent and continues to receive webhook-url and webhook-payload unchanged.