Skip to content

Act on the issue a community pull request closes - #103

Merged
rtibbles merged 9 commits into
mainfrom
community-pr-linked-issue-handling
Sep 29, 2026
Merged

rtibbles merged 9 commits into
mainfrom
community-pr-linked-issue-handling

Conversation

@akolson

@akolson akolson commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Summary

A community pull request got the same reply whether or not it linked an issue, and whether or not
that issue was already assigned to someone else. Maintainers closed the wrong ones by hand, and
requested review and added the label by hand for the right ones.

Contributors also received three bot comments in a row on a new pull request: the reply, the
rtibblesbot note, and the community review note.

The change

  • A pull request whose linked issue is assigned to someone else gets a closing message, is closed,
    and is reported to #support-dev-notifications. The standard reply is not sent.
  • A pull request whose linked issue is assigned to its author gets rtibblesbot requested for review
    and the community-review label.
  • A pull request with no linked issue is asked to add one under ## References, with Fixes #123
    or Closes #123 as examples. A maintainer asked for this by hand before.
  • Anything else keeps the previous behaviour.
  • The rtibblesbot and community review wording is now part of the opening reply, so the automatic
    path posts one comment instead of three.
  • review-requested and pull-request-label still post their note when a person requests the review
    or adds the label. They skip it when the bot did, because the reply already carries the text.
  • Links come from closingIssuesReferences, so only a closing keyword counts. A failed lookup falls
    back to the plain reply rather than closing anything.

No registry change, so nothing regenerates and no consumer sync follows.

References

Closes #67. The process for unplanned work that #67 also describes is tracked in #105.

Reviewer guidance

  1. Run npm test. Expect 103 passing tests, 21 of them new, one per branch and guard.
  2. Read BOT_MESSAGE_PULL_REQUEST_CLOSED in scripts/constants.js. It mirrors
    BOT_MESSAGE_ALREADY_ASSIGNED, which answers the same situation on an issue.
  3. Read the folded block at the end of BOT_MESSAGE_PULL_REQUEST. This is the wording a contributor
    now reads in place of the two follow-up comments.
  4. Branch runs in test-actions are still to come, one pull request per branch from an account
    outside the organization. I will add the links here.

Two decisions worth a second opinion:

  • A pull request is closed only when no linked issue is assigned to its author. An author assigned to
    one linked issue takes the review path even when another linked issue belongs to someone else. Create process for unplanned work and automate PR handling based on linked issue assignment #67
    does not cover this case, and I chose the reading where a wrong close is the worse error.
  • The sender check in review-requested and pull-request-label is the first place one automation
    depends on which other automation caused its event. The alternative was keeping all three comments.

AI usage

I used Claude Code to add the linked issue lookup, the three branches, the sender guards, and the
tests, and to fold the review wording into the reply. I decided the branch precedence and the
message wording, reviewed the diff, and verified it with the node test suite and prek.

A community pull request got the same reply whether or not it linked an issue,
and whether or not that issue was already assigned to someone else. Maintainers
closed the wrong ones by hand and requested review by hand.

The reply now branches on the linked issues. A pull request on an issue assigned
to someone else gets a closing message and is closed. One on an issue assigned to
its author gets rtibblesbot requested and the community-review label. Anything
else keeps the previous behaviour.

Links come from closingIssuesReferences, so only a closing keyword counts. A
failed lookup falls back to the plain reply rather than closing anything.

The rtibblesbot and community review notes are folded into the opening reply, so a
contributor reads one comment instead of three. review-requested and
pull-request-label still post their note when a person requests the review or adds
the label, and skip it when the bot did.
A pull request with no linked issue got the plain reply and nothing else, so a
maintainer had to ask for the link by hand before anything could happen. The
reply now says to add one under References, with Fixes #123 or Closes #123 as
examples, since a plain mention does not link an issue.

The community-automations page now leads with the three cases and states what
counts as a linked issue after them, rather than before.
A failure requesting rtibblesbot or adding the label threw before the reply was
sent, so a contributor could lose the welcome message entirely over an API error.
The reply goes first now. The wording still fits, because it says what will
happen rather than what has happened.

A failed lookup returned an empty list, which the reply could not tell apart from
a pull request with nothing linked. It returns null now, so a contributor who
linked an issue correctly is not asked to link one when the lookup breaks.

The linked issue title and url were mapped but never read, so they are gone.
@akolson
akolson marked this pull request as ready for review September 29, 2026 10:31
@akolson
akolson requested a review from rtibblesbot September 29, 2026 10:35
@rtibblesbot

rtibblesbot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

🟡 Waiting for changes

Last updated: 2026-09-29 18:44 UTC

The community-automations page described how review-requested and
pull-request-label react to contributor-pr-reply, which belongs in those scripts
rather than on a page that covers one automation per section.

The reply is also sent before the review request and the label now, so the order
in the bullet was wrong.
The reply is sent before the review request and the label, so the Slack output is
already set when those calls run. A failure there reached the outer catch, which
blanks the output, so the reply went out but nobody was told. The two calls now
handle their own failure and still report it.

The check for a failed lookup reads lookup directly, rather than leaning on an
empty array being truthy while taking the length from another variable.

@rtibblesbot rtibblesbot 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.

PR #103: no blocking issues. CI passing. No UI files changed.

  • suggestion: a failed close leaves a "we're closing" comment and no Slack notice (inline)
  • suggestion: cross-repo linked issues render as a local #N (inline)
  • suggestion: link-issue prompt is outside #67's stated scope; MisRob to confirm (inline)
  • suggestion: one test only restates the reply constant (inline)
  • nitpicks: multi-issue docs sentence, getLinkedIssues docstring (inline)
  • praise: failed-lookup tests pin down that an API outage never closes a PR or asks for a link.

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran a phased review pipeline over the pull request diff:

  • Classified the diff to select review passes (core, frontend, backend) and whether manual QA was required
  • Core review pass checked correctness, design, architecture, testing, completeness, and DRY/SRP/Rule-of-Three principles
  • Specialized frontend/backend review passes applied framework-specific lenses where those files changed
  • For UI changes: manual QA and an accessibility audit against a live dev server, when available
  • Checked CI status and linked issue acceptance criteria
  • Synthesized one review from those passes and chose the verdict from the findings, CI status, and QA evidence

Comment thread scripts/contributor-pr-reply.js Outdated
Comment thread scripts/contributor-pr-reply.js Outdated
Comment thread scripts/constants.js
Comment thread scripts/contributor-pr-reply.test.js Outdated
Comment thread docs/community-automations.md Outdated
Comment thread scripts/utils.js Outdated
A close that failed left the contributor reading "we're closing this pull
request" on an open pull request, and the outer catch blanked the Slack output so
nobody was told. The close now reports its own failure and still notifies.

A linked issue in another repo was named as a local #N, which points at a
different issue here. The message and the Slack line use the issue url instead,
so a cross-repo link reads correctly.

The docs sentence about several linked issues only held when the author was
assigned to one of them. The rule is simpler stated from the author's side.

The getLinkedIssues docstring said a closing keyword is the only way to link an
issue. The Development sidebar also counts.

Dropped the test that asserted three phrases of the reply. It restated the
constant, so any copy edit broke it while no logic change would.
@akolson
akolson requested a review from rtibblesbot September 29, 2026 18:24

@rtibblesbot rtibblesbot 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.

PR #103: the Slack notices added for partial failures never reach Slack (blocking, see inline).

  • CI passing.
  • 5 of 6 prior findings resolved.
  • 1 contested (link-issue prompt); left to maintainers.
Prior-finding status

RESOLVED — scripts/contributor-pr-reply.js — failed close leaves closing comment on open PR with no Slack notice
RESOLVED — scripts/contributor-pr-reply.js — cross-repo linked issue rendered as local #N
CONTESTED — scripts/constants.js:146 — link-issue prompt outside #67 scope
RESOLVED — scripts/contributor-pr-reply.test.js — test only restates reply constant
RESOLVED — docs/community-automations.md — multi-issue sentence inaccurate
RESOLVED — scripts/utils.js — docstring omits Development-sidebar links


@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Compared the current PR state against findings from a prior review:

  • Retrieved prior bot reviews via the GitHub API
  • Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
  • Only raised NEW findings for newly introduced code
  • Ran the same phased review passes as a first review (core, frontend/backend lenses, manual QA when required)
  • Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence

Comment thread scripts/contributor-pr-reply.js
Comment thread scripts/contributor-pr-reply.test.js
The step condition held no status function, so Actions treated it as success() &&
the output check. A failed close, or a failed review request, calls setFailed on
the script step, which skipped the Slack step and dropped the notice it had just
set.

A crash still posts nothing, because the outer catch clears the output first.
@akolson
akolson requested a review from rtibblesbot September 29, 2026 18:36

@rtibblesbot rtibblesbot 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.

PR #103: 6 of 7 prior findings resolved; 1 withdrawn.

  • scripts/constants.js:146: I withdraw my scope objection to the link-issue prompt. #104 depends on it. The PR body lists it. No change needed.

CI passing.

Prior-finding status

RESOLVED — scripts/contributor-pr-reply.js — closing message sent even if the close fails
RESOLVED — scripts/contributor-pr-reply.js — cross-repo linked issue renders as local #N
RESOLVED — scripts/contributor-pr-reply.test.js — test only restates the reply constant
RESOLVED — docs/community-automations.md — sentence holds only when author is assigned
RESOLVED — scripts/utils.js — docstring omits manual Development-sidebar links
CONTESTED — scripts/constants.js:146 — link-issue prompt outside #67's scope (withdrawn)
RESOLVED — scripts/contributor-pr-reply.js:48 — Slack never receives "Failed to close" notice (fixed by !cancelled())


@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Compared the current PR state against findings from a prior review:

  • Retrieved prior bot reviews via the GitHub API
  • Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
  • Only raised NEW findings for newly introduced code
  • Ran the same phased review passes as a first review (core, frontend/backend lenses, manual QA when required)
  • Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence

@rtibbles rtibbles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good to go!

@rtibbles
rtibbles merged commit 24c69d5 into main Sep 29, 2026
25 checks passed
@rtibbles
rtibbles deleted the community-pr-linked-issue-handling branch September 29, 2026 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create process for unplanned work and automate PR handling based on linked issue assignment

3 participants