Skip to content

ci(workflow): notifying plugin authors in PRs - #146

Draft
spiros132 wants to merge 1 commit into
noctalia-dev:mainfrom
spiros132:pr-notify-authors
Draft

ci(workflow): notifying plugin authors in PRs#146
spiros132 wants to merge 1 commit into
noctalia-dev:mainfrom
spiros132:pr-notify-authors

Conversation

@spiros132

Copy link
Copy Markdown
Contributor

Another workflow add to notify the plugin author, but this time for the pull requests instead of the issues

fix should be correct yml now

fix: Correctly search for id

fix

fix: Correct strip

fix: Use the earliest one to commit instead of the github name

fix: This should work correctly

As before, a lot of commits into one
@ItsLemmy

Copy link
Copy Markdown
Contributor
  1. blocking - .github/workflows/pr-notify-plugin-authors.yml:7
    The workflow requests pull-requests: write on a plain pull_request trigger. GitHub
    does not grant write scopes to GITHUB_TOKEN for pull requests from forks, and the
    permissions block cannot elevate that. Direct evidence from this PR's own run
    (Actions run 30447894608, job 90562662571), which executed this workflow from the PR
    head: the job log's "GITHUB_TOKEN Permissions" group reads Contents: read,
    Metadata: read, PullRequests: read. So
    .github/workflows/pr-notify-plugin-authors.py:67 (pr.create_issue_comment) will get
    403 and the run will fail without posting the CC.
    Every one of the last 60 pull requests in this repository is cross-repository
    (60/60), so this is the normal case, not an edge case: the feature can never fire as
    written.
    Note on the fix: switching to pull_request_target is the usual remedy and its default
    checkout is the base ref, which is safe here since the script only needs the API plus
    the base tree. Do not combine pull_request_target with checking out
    github.event.pull_request.head.sha, as that would execute untrusted PR code with a
    writable token.

  2. blocking - .github/workflows/pr-notify-plugin-authors.py:58
    New-plugin pull requests crash. actions/checkout on pull_request checks out the merge
    commit (this PR's log: "HEAD is now at 0f590c7 Merge 882e42d... into 6cee9bb..."),
    so os.path.exists at .github/workflows/pr-notify-plugin-authors.py:57 succeeds for a
    plugin directory the PR itself adds. repo.get_commits(path=...) then queries the
    default branch, where that path has no history. The API returns HTTP 200 with [] for
    an unknown path (verified against this repository), and PyGithub's
    PaginatedList.getitem indexes an empty buffer, so .reversed[0] raises
    IndexError: list index out of range (github/PaginatedList.py getitem and
    __fetchToIndex). Result: unhandled traceback, red run, no comment.
    Replaying the script's logic over the last 60 pull requests: 7 would hit this
    IndexError (feat(jetbrains-provider): add JetBrains Provider launcher plugin #148, feat(ssh-launcher): add /ssh launcher for SSH config hosts #138, feat: add arch-updater plugin #121, feat(wifi-hotspot): add NetworkManager hotspot control plugin #120, feat!: add Config Swap plugin #112, feat!: Add Config swap plugin #105, feat!: add W Engine plugin #81), 2 would reach the comment call
    and 403 per finding 1 (fix(drive-health): missing daemon WantedBy= (#142) #143, feat: add toggle for notifications on device switch and toggle for keybinds in tooltip #122), 2 would exit 1 on an unedited template
    placeholder (fix(hassio): Perpetual connecting #75, syncthing: Syncthing status & control (port of the v4 syncthing-status plugin) #71), and the remaining 49 would exit 0 silently. Zero would have
    produced a working CC comment.

  3. non-blocking - .github/workflows/pr-notify-plugin-authors.py:49
    is_same_author compares the id author segment to the PR author login with ==, which
    is case sensitive. GitHub logins are case insensitive and the validator forces id
    segments to lowercase (.github/workflows/validate-plugins.py:67, ID_SEGMENT_RE
    ^[a-z0-9][a-z0-9._-]*$), so the early exit misses its own author constantly: 24 of 51
    plugin ids have an author segment that differs from the actual first-commit login,
    most of them by case alone (cleboost vs Cleboost, apex077 vs Apex077, lucasoe vs
    LucasOe, frai3mega vs FraI3mega), and several by name (yuuto vs Reiling-Jeff, kenn vs
    cheerfulScumbag, lux vs luxqw, alexander vs Ahmed5Emad, neuro vs 13ghosts). A
    casefold comparison would short-circuit most self-authored PRs, which also removes
    most of finding 2's exposure. The second call at
    .github/workflows/pr-notify-plugin-authors.py:64 catches the case-only mismatches,
    but only after the crash-prone lookup.

  4. non-blocking - .github/workflows/pr-notify-plugin-authors.py:24
    pr.body is used without a None check, and PR bodies are optional, so a
    description-less PR produces AttributeError instead of a clean skip. Relatedly, an
    unedited template Id line ("- Id: <author>/<plugin>",
    .github/PULL_REQUEST_TEMPLATE.md:7) splits into two segments and falls through to
    exit 1 at .github/workflows/pr-notify-plugin-authors.py:62; PRs fix(hassio): Perpetual connecting #75 and syncthing: Syncthing status & control (port of the v4 syncthing-status plugin) #71 in the
    sample did exactly this. For a best-effort notifier, expected user input errors
    should exit 0 with a log line rather than turning the run red, as the label-missing
    path already does at .github/workflows/pr-notify-plugin-authors.py:36.

  5. non-blocking - .github/workflows/pr-notify-plugin-authors.py:66
    if author: is unreachable as a null guard. The API returns author: null for commits
    whose email is not linked to an account, and PyGithub then returns None from
    Commit.author, so .login at
    .github/workflows/pr-notify-plugin-authors.py:59 raises AttributeError before this
    check. All 51 current plugins resolve to a real login, so this is latent, but the
    guard belongs at the dereference.

  6. non-blocking - .github/workflows/pr-notify-plugin-authors.yml:11
    No repository guard, unlike .github/workflows/update-catalog.yml:20
    (if: github.repository == 'noctalia-dev/community-plugins'), and
    .github/workflows/pr-notify-plugin-authors.yml:18 passes python-version: 3.14
    unquoted, which YAML reads as a float. Also .github/workflows/pr-notify-plugin-authors.yml:21
    installs PyGithub unpinned; the script makes three API calls that the stdlib or
    actions/github-script covers, and this would be the repository's only third-party CI
    dependency.

  7. non-blocking - .github/workflows/pr-notify-plugin-authors.py:55
    This script duplicates roughly 25 lines of plugin resolution and author lookup from
    PR 145's .github/workflows/issues-notify-plugin-authors.py, including the same
    .reversed[0].author.login pattern and the same error handling. If both PRs land, a
    shared helper module in .github/workflows/ would keep the two notifiers from drifting
    apart, especially given the fixes above apply to both.

@ItsLemmy
ItsLemmy marked this pull request as draft July 29, 2026 13:25
@ItsLemmy ItsLemmy changed the title feat(workflow): Added workflow for notifying plugin authors in prs ci(workflow): notifying plugin authors in PRs Jul 29, 2026
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.

2 participants