Skip to content

ci(triage): re-triage issues when needs-info is cleared - #4108

Merged
serena-ruan merged 2 commits into
mainfrom
triage-retriage-on-needs-info-clear
Aug 5, 2026
Merged

ci(triage): re-triage issues when needs-info is cleared#4108
serena-ruan merged 2 commits into
mainfrom
triage-retriage-on-needs-info-clear

Conversation

@serena-ruan

Copy link
Copy Markdown
Collaborator

Related issue

N/A

Summary

Adds a hybrid needs-info lifecycle so an issue the reporter has clarified gets re-triaged and assigned automatically, following the common convention (remove the waiting label on author response; the existing stale bot closes ones nobody answers).

Flow:

needs-info issue
  ├─ author comments ─▶ needs-info-response.yml removes needs-info (App token)
  │                        └─▶ issue-triage.yml `unlabeled` trigger re-triages:
  │                            reads follow-up comments → classifies + assigns
  │                            (re-adds needs-info only if still too vague)
  └─ author never responds ─▶ stale.yml closes it (already exists)
  • New needs-info-response.yml — on issue_comment, when a non-bot commenter who is the issue author comments on an open issue that still has needs-info, removes the label. Uses the omnigent-ci App token, because a label removed with the default GITHUB_TOKEN does not re-trigger downstream workflows (so re-triage would never fire). Fails closed: if the App isn't configured, it skips rather than silently dropping the label.
  • issue-triage.yml — now triggers on issues: [opened, unlabeled]. The unlabeled path fires only for needs-info on an open issue and intentionally allows a bot actor (the App's removal). It feeds the author's follow-up comments into the triage prompt, clears needs-info when the LLM no longer flags it, suppresses the duplicate-of comment on re-triage, and adds a per-issue concurrency group.

No loop risk: issue-triage's own label edits use GITHUB_TOKEN, which never emits re-triggering events.

Test Plan

  • python3 -c "import yaml; yaml.safe_load(open(f))" on both workflows — valid.
  • pre-commit (incl. no-hardcoded-models) passed on commit.
  • Manual trace of the trigger/gate logic (no unit harness exists for these workflows). Live check: on a test needs-info issue, comment as the authorneeds-info should drop, then triage re-runs and assigns (or re-adds needs-info). A comment by anyone else does nothing.

Demo

N/A — non-visual (CI workflows).

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Coverage notes

These workflows have no unit-test harness (consistent with the other GitHub Actions in the repo). Verified by YAML validation, pre-commit, and tracing the trigger/gate/token logic by hand. The App-token dependency (OMNIGENT_BOT_APP_ID / OMNIGENT_BOT_APP_KEY) is already used by ~16 other workflows.

This pull request and its description were written by Isaac.

Add a hybrid needs-info lifecycle. When the issue author comments on an
issue that still carries needs-info, needs-info-response.yml removes the
label using the omnigent-ci App token (the default GITHUB_TOKEN would not
re-trigger downstream workflows). That removal fires issue-triage.yml's
new `unlabeled` trigger, which reads the reporter's follow-up comments,
reclassifies, and assigns an owner — re-adding needs-info only if the
issue is still too vague. Issues the reporter never clarifies are closed
by the existing stale.yml.

issue-triage.yml changes:
- trigger on issues [opened, unlabeled]; the unlabeled path fires only
  for needs-info on an open issue, and allows a bot actor (the App)
- feed the author's follow-up comments into the triage prompt
- remove needs-info on re-triage when the LLM no longer flags it
- suppress the duplicate-of comment on the re-triage path
- add a per-issue concurrency group

Co-authored-by: Isaac
Copilot AI lite review requested due to automatic review settings August 5, 2026 06:45
@github-actions github-actions Bot added the size/M Pull request size: M label Aug 5, 2026
@omnigent-ci

omnigent-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Blocking issues

None. I traced the trigger/gate/token logic and the label-application changes against the existing workflow, and the design is sound:

  • The loop-avoidance claim holds. needs-info-response.yml removes the label with the App token (a distinct actor → GitHub re-emits the unlabeled event), while issue-triage.yml's own label edits use GITHUB_TOKEN (never re-triggers). Re-adding needs-info on a still-vague re-triage therefore cannot restart the cycle; only a fresh author comment can. No infinite loop.
  • The two workflows use distinct concurrency groups (needs-info-response-N vs issue-triage-N), so cancel-in-progress in one never kills the other. The label removal and the downstream re-triage don't race each other out of existence.
  • The unlabeled job gate is correctly narrowed (label.name == 'needs-info' AND issue.state == 'open'); other label removals just no-op at the job level.
  • needs-info-response.yml's if correctly requires a non-bot sender, a real issue (not a PR), open state, commenter == issue author, and the label present.
  • Author-comment ingestion is safe: comments are read from JSON in Python and injected only into the file-based prompt under an explicit UNTRUSTED header — never shell-interpolated — and the LLM still has no tools/GH_TOKEN. Prompt-injection posture is preserved.
  • Label-application changes are correct: guarding if "needs-info" not in existing_labels before adding, and the else-branch safety-net removal of a lingering needs-info, are both consistent with the existing idempotent pattern.

Security vulnerabilities

None introduced. ISSUE_NUMBER/REPO are passed via env and are non-interpolated numeric/known values; the App-token step is gated on vars.OMNIGENT_BOT_APP_ID != '' and the removal step on a non-empty minted token. No new secret exposure, injection, or boundary weakening. The App-token dependency matches the pattern already used across the repo's workflows.

Non-blocking notes

  • Fail-closed dependency is real but silent. If OMNIGENT_BOT_APP_ID/OMNIGENT_BOT_APP_KEY aren't configured, the entire re-triage feature no-ops (label never removed, so author responses never re-triage — the flow degrades to stale-bot closure). This is documented as intentional, but consider emitting a ::notice:: when the App is unconfigured so maintainers can tell the feature is dormant rather than broken.
  • duplicate label on re-triage without a comment. The duplicate_of comment is correctly suppressed on the unlabeled path, but the duplicate label can still be (re)applied on re-triage. Minor inconsistency — a label with no accompanying explanation — worth a glance but not blocking.
  • Second-pass label accumulation. Re-triage only adds comp/type/priority labels; if a maintainer manually set one between open and re-triage, the LLM could add a second of the same family. Scope is limited (the needs-info path only left triaged behind), so low risk.
  • Comment volume. gh issue view --json comments returns the full comment list; only author comments are kept and capped to 4 KB, so prompt size is bounded, but very chatty issues will truncate later clarifications. Acceptable given the cap.

Summary

A well-reasoned, defensively-designed CI change. The core correctness risks for this kind of hybrid lifecycle — trigger loops, cross-workflow concurrency cancellation, prompt-injection via newly-ingested comments, and unsafe token usage — are each explicitly handled, and I could not find a real bug or security regression in the diff. The only follow-ups are operational/cosmetic (surface the unconfigured-App state, and the minor re-triage label-accumulation edge cases). No unit-test harness exists for these workflows, consistent with the repo's other Actions; the manual YAML/pre-commit verification and logic trace are appropriate here. Approvable.


Automated review by Polly · workflow run

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the GitHub issue triage automation to support a “needs-info lifecycle”: when the original reporter replies on a needs-info issue, the label is cleared (using an App token) so the existing triage workflow re-runs and can classify/assign based on the new details.

Changes:

  • Add a new needs-info-response.yml workflow to remove needs-info when the issue author comments (using the omnigent-ci App token so downstream workflows re-trigger).
  • Extend issue-triage.yml to re-run on issues: unlabeled specifically for needs-info removal, include author follow-up comments in the prompt, and avoid duplicate-of re-commenting on re-triage.
  • Add per-issue concurrency to issue-triage.yml to prevent overlapping runs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/needs-info-response.yml New workflow to clear needs-info on issue-author response using an App token so unlabeled triggers downstream triage.
.github/workflows/issue-triage.yml Re-triage on needs-info removal; incorporate author follow-up comments; prevent duplicate-of re-commenting; add concurrency.

Comment on lines +60 to +63
run: |
set -euo pipefail
echo "Author responded on #$ISSUE_NUMBER; removing needs-info to re-triage."
gh issue edit "$ISSUE_NUMBER" --repo "$REPO" --remove-label needs-info

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch — fixed in bc04dc5. The removal step now re-checks the live labels (gh issue view --json labels | grep -qx needs-info) before calling --remove-label, so a stale event payload or a race (two quick comments / concurrent removal) no-ops cleanly instead of erroring. Also added a ::notice:: when the App is unconfigured, and suppressed the duplicate label (not just the comment) on the re-triage path per Polly's note.

…tice

- needs-info-response.yml: re-check live labels before `gh --remove-label`
  so a stale event payload / race can't fail the step (gh errors on a
  missing label); emit a ::notice:: when the omnigent-ci App is
  unconfigured so a dormant feature is distinguishable from a broken one.
- issue-triage.yml: also suppress the `duplicate` label on the re-triage
  path (not just the comment), keeping the label and its explanation
  consistent; hoist `import os` to the top of the block.

Co-authored-by: Isaac
Copilot AI review requested due to automatic review settings August 5, 2026 06:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/workflows/issue-triage.yml:144

  • gh issue view now always fetches full comments into /tmp/issue.json, even on the initial opened path where there typically aren’t any. On long issues this can significantly increase API payload size and runtime for the common case. Consider only requesting comments when the event is unlabeled (re-triage).
          # see the detail the reporter added in comments, not just the original
          # body.
          gh issue view "$ISSUE_NUMBER" --repo "$REPO" \
            --json number,title,body,labels,author,comments \
            > /tmp/issue.json

.github/workflows/issue-triage.yml:307

  • The author follow-up comment extraction builds and joins all author comment bodies before slicing to 4 KB. On issues with many/large comments this can create unnecessary memory/time overhead (and it’s slightly at odds with the “Capped to 4 KB total” comment). Consider limiting to the most recent N author comments and truncating each before joining.
          # Follow-up comments by the issue author — the reporter often supplies
          # the missing detail here, so the re-triage path must read them. Only
          # the author's own comments count as clarification (others' comments
          # are noise for this purpose and are dropped). Capped to 4 KB total.
          author_login = issue.get("author", {}).get("login")

@serena-ruan

Copy link
Copy Markdown
Collaborator Author

/review

@omnigent-ci

omnigent-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Review: ci(triage): re-triage issues when needs-info is cleared

1. Blocking issues

None. I traced the trigger/gate/token logic end to end and the design holds:

  • No loop. needs-info-response removes needs-info with the App token → fires issue-triage's unlabeled path. Any re-add of needs-info by issue-triage uses GITHUB_TOKEN, which GitHub does not re-trigger workflows from, so the cycle terminates. Each re-triage is author-action-driven, not self-sustaining.
  • Fail-closed removal is correct. When OMNIGENT_BOT_APP_ID is unset the token step is skipped, its outputs.token is empty string, the warn step runs, and the remove step is skipped — so the label is never dropped with a non-re-triggering token.
  • Duplicate suppression is consistent. On re-triage dup is forced to None, so neither the duplicate label nor the comment is re-applied; the open-time label/comment stay coherent.
  • Idempotency is handled. The removal step re-checks live labels with grep -qx before --remove-label (avoids the error on an already-removed label), and re-adding triaged/needs-info on --add-label is idempotent.
  • Comment gate is properly scoped. comment.user.login == issue.user.login, non-bot sender, open issue, not a PR, label present — all correct fields for the issue_comment payload.

2. Security vulnerabilities

None new. The author's follow-up comments are now fed into the prompt, but:

  • They are same-trust as the issue body (author-controlled) already ingested, and are read from files into Python — never shell-interpolated.
  • Only the author's own comments are included (c.author.login == author_login), so a third party cannot inject via a comment even when a non-author's label removal triggers re-triage.
  • The LLM still has no shell/tool/GH_TOKEN access, and all outputs remain constrained by the component/owner/priority/duplicate allowlists. Worst case is misclassification within the allowlist, unchanged from the existing body-injection posture.

3. Non-blocking notes

  • App misconfiguration isn't fully fail-closed. Fail-closed only covers OMNIGENT_BOT_APP_ID == ''. If the ID is set but OMNIGENT_BOT_APP_KEY is empty/invalid, create-github-app-token hard-fails the job on every author comment on a needs-info issue — noisy rather than dormant. Consider tolerating a mint failure the same way as the unconfigured case.
  • cancel-in-progress: true could cancel a slow in-flight initial triage if a re-triage for the same issue overlaps. In practice the newer run completes the classification, so the outcome converges, but it's worth being aware the initial run's partial label writes could be abandoned mid-way.
  • Stale duplicate label. A duplicate label added at open persists through re-triage even if the reporter's clarification shows it isn't a dup (intentional per the comment, but it can leave a stale label).
  • Wasted work: the duplicate search still runs on the re-triage path and is then discarded — harmless, minor cost.
  • The redundant EVENT_ACTION == "opened" guard on the duplicate comment (dup is already None on re-triage) is harmless belt-and-suspenders.

4. Summary

A well-constructed, security-conscious change. The App-token requirement is correctly motivated (GITHUB_TOKEN label edits don't re-trigger), the fail-closed skip and live-label re-check make the removal robust, the prompt-injection boundary is preserved (author-only comments, no-tools LLM, output allowlists), and the loop analysis checks out. No blocking correctness or security issues found. The only items worth a follow-up are the partial fail-closed behavior on a misconfigured App key and the concurrency-cancel interaction — both operational, neither blocking.


Automated review by Polly · workflow run

@serena-ruan
serena-ruan merged commit f0ff685 into main Aug 5, 2026
66 checks passed
@serena-ruan
serena-ruan deleted the triage-retriage-on-needs-info-clear branch August 5, 2026 09:12
@github-actions github-actions Bot added the no-doc-update Merged PR does not need a docs update label Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🏷️ Doc impact: no-doc-update

Changes are confined to internal GitHub Actions issue-triage automation workflows (CI tooling), which touch no user-facing surface, integration, or built-in policy.

Auto-classified on merge. Set the label manually before merging to override. · run

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

Labels

no-doc-update Merged PR does not need a docs update size/M Pull request size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants