Skip to content

chore: restore notify-parent workflow (simplified) #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mateodelnorte merged 1 commit into from
Mar 26, 2026
Merged

chore: restore notify-parent workflow (simplified) #24

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/workflows/notify-parent.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: Notify Parent Repo

on:
push:
branches: [main]
Comment on lines +3 to +5

@coderabbitai coderabbitai Bot Mar 26, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/notify-parent.yml

Repository: harmony-labs/meta_git_cli

Length of output: 868

🏁 Script executed:

# Check for branch protection configuration
git ls-files | grep -E "(branch|protection|rules)" | head -20

Repository: harmony-labs/meta_git_cli

Length of output: 51

🏁 Script executed:

# Look for any references to the parent repo or related documentation
rg "harmony-labs/meta|parent.?repo|PARENT_REPO" -i --max-count=20

Repository: harmony-labs/meta_git_cli

Length of output: 184

🏁 Script executed:

# Check if there are other workflows that might show the intended pattern
fd "\.yml$|\.yaml$" .github/workflows/ --type f

Repository: harmony-labs/meta_git_cli

Length of output: 163

🏁 Script executed:

# Look for any documentation about the workflow intent
fd "README|CONTRIBUTING|docs" -type f | head -10

Repository: harmony-labs/meta_git_cli

Length of output: 239

🏁 Script executed:

# Check the other workflows for comparison
cat -n .github/workflows/ci.yml | head -30

Repository: harmony-labs/meta_git_cli

Length of output: 793

🏁 Script executed:

# Check for README documentation
cat README.md 2>/dev/null | head -50

Repository: harmony-labs/meta_git_cli

Length of output: 51

🏁 Script executed:

# Look at git history for when notify-parent was added
git log --oneline .github/workflows/notify-parent.yml 2>/dev/null | head -5

Repository: harmony-labs/meta_git_cli

Length of output: 127

🏁 Script executed:

# Search for any comments or context about the workflow intent
git log -p --reverse .github/workflows/notify-parent.yml 2>/dev/null | head -100

Repository: harmony-labs/meta_git_cli

Length of output: 51

🏁 Script executed:

# Check if there's a branch protection config or CODEOWNERS
git ls-files | grep -iE "(codeowners|branch|protection)"

Repository: harmony-labs/meta_git_cli

Length of output: 51

🏁 Script executed:

# Search for any GitHub organization/settings files
find . -name ".github" -type d -exec ls -la {} \;

Repository: harmony-labs/meta_git_cli

Length of output: 235

🏁 Script executed:

# Look for any issues or discussions about this workflow
git log --all --grep="notify-parent" -i --oneline 2>/dev/null

Repository: harmony-labs/meta_git_cli

Length of output: 316

🏁 Script executed:

# Check the full commit that restored the workflow
git show 7ba7a37 2>/dev/null

Repository: harmony-labs/meta_git_cli

Length of output: 50382

🏁 Script executed:

# Check the previous version of notify-parent to understand what changed
git show 2b2ab14:.github/workflows/notify-parent.yml 2>/dev/null | head -50

Repository: harmony-labs/meta_git_cli

Length of output: 51

🏁 Script executed:

# Check if there's any GitHub organization-level config
ls -la .github/

Repository: harmony-labs/meta_git_cli

Length of output: 235

🏁 Script executed:

# Look for any references to branch protection or main branch policy
rg "branch|protection|main" -i .github/ -l

Repository: harmony-labs/meta_git_cli

Length of output: 163

🏁 Script executed:

# Check the parent repo to understand what it expects
# (if accessible, though likely a private repo)
rg "child-repo-updated|notify-parent" -i 2>/dev/null | head -10

Repository: harmony-labs/meta_git_cli

Length of output: 51

Align trigger with intent: only notify on merged PRs, not all pushes to main.

Commit message states this should trigger "when this child repo merges to main", but the current push trigger fires on every push, including direct pushes if branch protection allows them. This risks unintended parent release notifications.

🔧 Suggested fix 
-on:
-  push:
-    branches: [main]
+on:
+  pull_request:
+    types: [closed]
+    branches: [main]

 jobs:
   notify:
     runs-on: ubuntu-latest
+    if: ${{ github.event.pull_request.merged == true }}
     steps:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
on:
push:
branches: [main]
on:
pull_request:
types: [closed]
branches: [main]
jobs:
notify:
runs-on: ubuntu-latest
if: ${{ github.event.pull_request.merged == true }}
steps:
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/notify-parent.yml around lines 3 - 5, Replace the broad
push trigger under on: with a pull_request trigger scoped to closed PRs
targeting main (use on: pull_request with types: [closed] and branches: [main])
and add an explicit merged check (if: github.event.pull_request.merged == true)
to the workflow's jobs so the workflow only runs when a PR is merged into main;
update the existing on: and add the job-level if condition in notify-parent.yml
accordingly.

@mateodelnorte mateodelnorte Mar 26, 2026

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 in af351ea.

@coderabbitai coderabbitai Bot Mar 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
git show af351ea -- .github/workflows/notify-parent.yml 2>/dev/null

Length of output: 1347

🏁 Script executed:

#!/bin/bash
# Check the current state of the file on the PR branch
cat -n .github/workflows/notify-parent.yml

# Also check if there are any commits after af351ea on this branch
git log --oneline -5

Length of output: 1057

@mateodelnorte, it looks like commit af351ea made some improvements (removed the checkout step and fixed toJSON quoting), but the push trigger is still in place — the file currently reads:

on:
  push:
    branches: [main]

The suggested change to scope it to merged PRs only (using pull_request: types: [closed] with if: github.event.pull_request.merged == true) wasn't applied. If you'd like to address this concern, here's the fix:

on:
  pull_request:
    types: [closed]
    branches: [main]

jobs:
  notify:
    runs-on: ubuntu-latest
    if: ${{ github.event.pull_request.merged == true }}
    steps:
      - name: Notify parent repo
        uses: peter-evans/repository-dispatch@v4
        with:
          token: ${{ secrets.PARENT_REPO_PAT }}
          repository: harmony-labs/meta
          event-type: child-repo-updated
          client-payload: >-
            {
              "repo_name": ${{ toJSON(github.event.pull_request.head.repo.name) }},
              "short_sha": ${{ toJSON(github.event.pull_request.merge_commit_sha) }},
              "actor": ${{ toJSON(github.actor) }}
            }

Note that with the pull_request trigger, github.sha and github.event.repository.name still work, but github.event.pull_request.merge_commit_sha gives you the actual merge commit SHA which is more precise for the merged PR case.

If you intentionally want to keep the push trigger (e.g., because direct pushes to main should also notify the parent), please let me know and I'll dismiss this comment.

@mateodelnorte mateodelnorte Mar 26, 2026

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 — renamed short_sha to sha (sender sends full SHA, parent truncates to 7 chars), removed checkout step, added permissions: {} block.

@coderabbitai coderabbitai Bot Mar 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat -n .github/workflows/notify-parent.yml
git log --oneline -5

Length of output: 1090

@mateodelnorte, confirmed — permissions: {}, the sha rename, and the removed checkout step are all in place. Thanks for those improvements!

However, the original concern is still present: the trigger remains push on main (Lines 3–5), not scoped to merged PRs only. If direct pushes to main are prevented by branch protection, this is functionally equivalent — but if branch protection is not enforced, direct pushes would also fire the parent notification.

Could you confirm whether this is intentional (relying on branch protection to guarantee only merges reach main)? If so, I'm happy to mark this resolved.


permissions: {}

jobs:
notify:
runs-on: ubuntu-latest
steps:
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- name: Notify parent repo
# PARENT_REPO_PAT requires `repo` scope (or fine-grained `contents:write`)
# on harmony-labs/meta. Configure in Settings → Secrets → Actions.
uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4
with:
token: ${{ secrets.PARENT_REPO_PAT }}
repository: harmony-labs/meta
event-type: child-repo-updated
Comment thread
coderabbitai[bot] marked this conversation as resolved.
client-payload: >-
{
"repo_name": ${{ toJSON(github.event.repository.name) }},
"sha": ${{ toJSON(github.sha) }},
"actor": ${{ toJSON(github.actor) }}
}