Post to Phabricator when a task link is added after the PR is opened - #6809
Open
josephseddon wants to merge 4 commits into
Open
Post to Phabricator when a task link is added after the PR is opened#6809josephseddon wants to merge 4 commits into
josephseddon wants to merge 4 commits into
Conversation
The workflow only listened for `opened` and `closed`, reading the task ID from the PR body at event time. A PR created before the `**Phabricator:**` line was filled in never notified the task, even if the link was added later -- `edited` wasn't subscribed to, so the task only heard about the PR on close. The PR template ships a `https://phabricator.wikimedia.org/T...` placeholder, so a PR opened before that is replaced with a real task ID hits this case. Subscribe to `edited` and post only for task IDs the edit actually added, diffing `github.event.changes.body.from` against the current body. Unrelated body edits no longer re-notify an already-linked task. Edit-triggered comments use the verb "linked" to distinguish them from open/close notifications. `edited` also fires for title and base-branch changes, which carry no changes.body; those exit early. The check is done in bash against toJSON(...) rather than as a GitHub expression, because `changes.body.from != null` is false for an edit from an empty body and would wrongly skip it. Also while here: - printf '%s' instead of echo -e, which was interpreting backslash escapes in the body before matching - strip CR and sort -u, so a CRLF body naming one task twice posts once - `|| true` on the extraction pipeline, so a body with no task reference is not an error under a pipefail shell - move the API token out of the inlined curl argument into env - drop the step-level `if:`, which re-checked what `on:` already guarantees The 10s pause between posts is unchanged. This mirrors the same change made to post_phab.yml in wikipedia-ios. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XyimsJL4PymoNhpfWeQ5DG
josephseddon
requested review from
Williamrai,
cooltey,
dbrant and
voyagerfan
as code owners
August 19, 2026 00:19
There was a problem hiding this comment.
Pull request overview
Extends Phabricator notifications to newly linked tasks after PR body edits.
Changes:
- Handles
editedpull request events. - Diffs task IDs before and after edits.
- Improves task extraction and secret handling.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The extraction regexes are case-insensitive, but `sort -u` and the `grep -Fxv` set difference are not. "T123" and "t123" were therefore treated as two distinct tasks: a body naming both posted twice, and an edit that changed only the identifier's casing looked like a newly added task and emitted another "linked" comment for a task already linked. The lowercase form was also sent to Phabricator as objectIdentifier. Upper-case extracted IDs before they reach `sort -u` and the diff, which fixes both and sends the canonical form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XyimsJL4PymoNhpfWeQ5DG
The two extraction branches had drifted apart: the URL branch tolerated
markdown bold around its label ("**Phabricator:**"), while the trailer branch
was anchored on a bare "^Bug:". So "**Bug:** T430815" -- a natural thing to
write, mirroring the bolded label the PR template itself uses -- matched
nothing and silently notified no one. A bare ID after a label
("**Phabricator:** T430815", no URL) was likewise unmatched.
Replace both branches with one label rule: "Bug:" or "Phabricator:",
optionally markdown-bolded, followed by either a bare T123 or a
phabricator.wikimedia.org URL; plus a bare URL with no label, as before.
A bare "T123" with neither label nor URL still does not match -- accepting
that anywhere a line happens to start with a T-number would be too loose.
Verified: the two new forms match; all previously matching forms still do;
and "T430815" alone, "Debug: T999", a mid-line URL, and the PR template's
own "T..." placeholder all still correctly match nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XyimsJL4PymoNhpfWeQ5DG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this do?
Subscribes
android_phab.ymlto theeditedpull request event, and posts to Phabricator only for task IDs that the edit actually added — diffinggithub.event.changes.body.from(the pre-edit body) against the current body.openedopenededited, task added<actor> linked <PR url>for the new task(s) onlyedited, task already presentedited, title/base-branch onlychanges.bodyin the payload)closedclosedThe 10s pause between posts is unchanged.
Why is this needed?
The workflow only listened for
openedandclosed, reading the task ID from the PR body at event time. If a PR was created before the**Phabricator:**line was filled in, the task never got an "opened" notification —editedwasn't subscribed to, and re-running the original job replays the original task-less payload. The task only heard about the PR when it closed.This repo's PR template ships a
https://phabricator.wikimedia.org/T...placeholder, which doesn't matchT[0-9]+. So every PR opened before that placeholder is replaced with a real task ID hits this case.Naively adding
editedwould over-correct: every later body edit — filling in sections, fixing a typo — would re-post to an already-linked task. Hence the set difference.One subtlety worth flagging for review: the "was the body actually edited" check is done in bash against
toJSON(...)rather than as a GitHub expression.github.event.changes.body.from != nulllooks equivalent but isn't — an edit from an empty body yields'', which compares equal tonullunder GitHub's type casting, and would skip exactly the case this fixes.Also carried along:
printf '%s'instead ofecho -e, which was interpreting backslash escapes in the body before matchingsort -u, so a CRLF body naming one task twice posts once rather than twice|| trueon the extraction pipeline, so a body with no task reference isn't an error under apipefailshell (harmless under the currentbash -e {0}, but oneshell: bashaway from breaking)curlargument intoenv:if:, which re-checked whaton:already guaranteesTest Steps
Verified locally by extracting the
run:block and driving it with stubbedcurl/sleep— 16 cases, all passing, including:T...placeholder)linkedonceclosedwith two taskssleep 10betweenConfirmed exit 0 under both
bash -e(what actually runs) andbash -eo pipefail.This PR can also serve as a live test: it was opened with the template placeholder intact, so no comment should have been posted. Editing the description to add a real task ID should post exactly one
linkedcomment to that task, and any edit after that should post nothing. Note this posts to Phabricator for real — use a scratch task.Phabricator:
https://phabricator.wikimedia.org/T...
This mirrors the same change made to
post_phab.ymlin wikipedia-ios#6079.🤖 Generated with Claude Code
https://claude.ai/code/session_01XyimsJL4PymoNhpfWeQ5DG
Generated by Claude Code