Skip to content

fix: simplify notify-downstream workflow - #3

Merged
mateodelnorte merged 3 commits into
mainfrom
feat/project-dependents
Mar 25, 2026
Merged

mateodelnorte merged 3 commits into
mainfrom
feat/project-dependents

Conversation

@mateodelnorte

Copy link
Copy Markdown
Contributor

Summary

  • Remove broken commit message parsing that fails on merge commits with parentheses (e.g., feat: thing (#4))
  • Remove checkout step (not needed for dispatch)
  • Skip wait-for-ci on forwarded dispatches (already passed upstream)
  • Upgrade peter-evans/repository-dispatch to v4
  • Minimal payload: just repo, sha, actor

Context

The old workflow used MSG=$(git log -1 --pretty=%s) and bash regex matching to extract commit type. Commit subjects containing () (like GitHub merge commits with PR numbers) caused unexpected EOF while looking for matching ')' errors, breaking all downstream notifications.

🤖 Generated with Claude Code

Remove broken commit message parsing that failed on subjects containing
parentheses (e.g., merge commits with PR numbers). The bash regex
matching in `MSG=$(git log -1 --pretty=%s)` would break when the
subject contained unbalanced parens.

Simplified to just dispatch the event with repo, sha, and actor.
No checkout, no commit parsing. Also:
- Skip wait-for-ci on forwarded dispatches
- Upgrade peter-evans/repository-dispatch to v4

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Mar 25, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@mateodelnorte has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 8 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: bac61657-a3dd-4477-8ac5-08f598d8e5d0

📥 Commits

Reviewing files that changed from the base of the PR and between 11bb723 and 6b8b457.

📒 Files selected for processing (1)
  • .github/workflows/notify-downstream.yml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/project-dependents

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Mar 25, 2026

Copy link
Copy Markdown

Greptile Summary

This PR simplifies the notify-downstream workflow by removing a fragile bash commit-message parser that broke on merge commits containing parentheses (e.g., feat: thing (#4)), and adds support for forwarding incoming repository_dispatch events through the dependency chain without re-waiting on CI.

Key changes:

  • Root-cause fix: Eliminates the git log -1 --pretty=%s + bash regex block that caused unexpected EOF parse errors on PR-number suffixes in commit subjects.
  • Chain forwarding: The workflow now also triggers on repository_dispatch(dependency-updated), enabling meta_plugin_protocol to sit in the middle of a multi-repo dependency notification chain.
  • Correct skip logic: wait-for-ci is guarded with if: github.event_name == 'push'; the notify job uses if: always() && (... == 'success' || ... == 'skipped') to correctly handle both the push and repository_dispatch paths.
  • Payload simplification: The dispatched payload is reduced to {repo, sha, actor} — removing repo_name, short_sha, message, and type. Downstream workflows that reference these removed fields will silently receive null without dispatch failure, so coordination with consumers is advisable.
  • Dependency upgrade: peter-evans/repository-dispatch upgraded from v3 → v4.

Confidence Score: 4/5

  • Safe to merge once downstream payload compatibility is confirmed — the core bug fix is correct and the workflow logic is sound.
  • The PR correctly fixes a real bash-parsing bug, the conditional skip/forward logic is properly implemented, and the action upgrade is straightforward. The only non-blocking concern is the silent removal of payload fields that downstream repos may reference — but this is a coordination issue rather than a workflow correctness bug.
  • .github/workflows/notify-downstream.yml — verify downstream repos don't depend on the removed payload fields before merging.

Important Files Changed

Filename Overview
.github/workflows/notify-downstream.yml Removes broken bash commit-message parsing, adds repository_dispatch trigger for forwarded events, correctly skips wait-for-ci on non-push triggers, and simplifies the dispatch payload. One non-blocking concern: removed payload fields (repo_name, short_sha, message, type) may silently break downstream consumers if not coordinated.

Sequence Diagram

sequenceDiagram
    participant UP as Upstream Repo
    participant MPP as meta_plugin_protocol
    participant DS as Downstream Repos<br/>(meta_cli, meta_git_cli, etc.)

    alt push to main
        MPP->>MPP: wait-for-ci (lewagon/wait-on-check-action)
        MPP->>DS: repository_dispatch(dependency-updated)<br/>payload: {repo, sha, actor}
    else repository_dispatch(dependency-updated) received
        UP->>MPP: repository_dispatch(dependency-updated)
        Note over MPP: skip wait-for-ci (if: github.event_name == 'push')
        MPP->>DS: repository_dispatch(dependency-updated)<br/>payload: {repo, sha (MPP HEAD), actor}
    end
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/notify-downstream.yml
Line: 42-47

Comment:
**Breaking payload change for downstream consumers**

The PR removes `repo_name`, `short_sha`, `message`, and `type` from the dispatched payload. If any downstream workflow (e.g. in `meta_cli`, `meta_git_cli`, `meta_project_cli`, or `meta_rust_cli`) currently references `github.event.client_payload.type`, `.message`, `.short_sha`, or `.repo_name`, those expressions will silently resolve to `null`/empty without any dispatch failure.

Before merging, confirm that none of the downstream repos' `repository_dispatch`-triggered workflows use these removed fields. If they do, those workflows should be updated in the same change or in a coordinated follow-up to avoid silent failures.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix: simplify notify-downstream workflow" | Re-trigger Greptile

Comment on lines 42 to 47
client-payload: >-
{
"repo": ${{ toJSON(github.repository) }},
"repo_name": ${{ toJSON(github.event.repository.name) }},
"sha": ${{ toJSON(steps.commit.outputs.sha) }},
"short_sha": ${{ toJSON(steps.commit.outputs.short_sha) }},
"message": ${{ toJSON(steps.commit.outputs.message) }},
"type": ${{ toJSON(steps.commit.outputs.type) }},
"sha": ${{ toJSON(github.sha) }},
"actor": ${{ toJSON(github.actor) }}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Breaking payload change for downstream consumers

The PR removes repo_name, short_sha, message, and type from the dispatched payload. If any downstream workflow (e.g. in meta_cli, meta_git_cli, meta_project_cli, or meta_rust_cli) currently references github.event.client_payload.type, .message, .short_sha, or .repo_name, those expressions will silently resolve to null/empty without any dispatch failure.

Before merging, confirm that none of the downstream repos' repository_dispatch-triggered workflows use these removed fields. If they do, those workflows should be updated in the same change or in a coordinated follow-up to avoid silent failures.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/notify-downstream.yml
Line: 42-47

Comment:
**Breaking payload change for downstream consumers**

The PR removes `repo_name`, `short_sha`, `message`, and `type` from the dispatched payload. If any downstream workflow (e.g. in `meta_cli`, `meta_git_cli`, `meta_project_cli`, or `meta_rust_cli`) currently references `github.event.client_payload.type`, `.message`, `.short_sha`, or `.repo_name`, those expressions will silently resolve to `null`/empty without any dispatch failure.

Before merging, confirm that none of the downstream repos' `repository_dispatch`-triggered workflows use these removed fields. If they do, those workflows should be updated in the same change or in a coordinated follow-up to avoid silent failures.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed — restored full payload, removed repository_dispatch trigger to prevent cascades, switched to heredoc output for safe commit message handling.

mateodelnorte and others added 2 commits March 25, 2026 14:18
Add `!` to regex character class so `feat!:`, `fix(scope)!:` etc.
are correctly categorized instead of falling through to "other".

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mateodelnorte
mateodelnorte merged commit 5bba5f6 into main Mar 25, 2026
7 checks passed
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.

1 participant