chore: restore notify-parent workflow (simplified) - #10
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdded a GitHub Actions workflow Changes
Sequence Diagram(s)sequenceDiagram
participant Push as "Developer Push\n(child repo)"
participant Actions as "GitHub Actions\nRunner"
participant Dispatch as "peter-evans/repository-dispatch"
participant Parent as "harmony-labs/meta\n(Repository Dispatch Receiver)"
rect rgba(135,206,250,0.5)
Push->>Actions: push to main triggers workflow
end
rect rgba(144,238,144,0.5)
Actions->>Dispatch: call action with event_type: child-repo-updated\nclient_payload: {repo_name, short_sha, actor}\nauth: secrets.PARENT_REPO_PAT
end
rect rgba(255,182,193,0.5)
Dispatch->>Parent: send repository_dispatch event
Parent-->>Dispatch: 200 OK (ack)
end
Dispatch-->>Actions: action result
Actions-->>Push: workflow completes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR re-adds Confidence Score: 4/5Safe to merge after resolving the short_sha naming/value mismatch; the remaining comments are non-blocking style improvements. The workflow is simple and correctly structured. The P1 concern (field named short_sha contains a full SHA) could break downstream consumers in harmony-labs/meta, so one targeted fix is needed before the companion PR is live. The two P2 comments (unused checkout, inconsistent quoting) are clean-up suggestions and do not block merge on their own. .github/workflows/notify-parent.yml — specifically the short_sha field value on line 23.
|
| Filename | Overview |
|---|---|
| .github/workflows/notify-parent.yml | Simplified notify-parent workflow: removes CI wait and commit-type parsing, dispatches only repo_name, short_sha (actually full SHA — naming bug), and actor to harmony-labs/meta. |
Sequence Diagram
sequenceDiagram
participant GH as GitHub (loop_cli)
participant WF as notify workflow
participant META as harmony-labs/meta
GH->>WF: push to main
WF->>WF: (checkout — currently unused)
WF->>META: repository_dispatch<br/>event-type: child-repo-updated<br/>payload: {repo_name, short_sha (full SHA), actor}
META-->>META: trigger release-please
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/notify-parent.yml
Line: 23
Comment:
**`short_sha` field contains the full SHA**
`${{ github.sha }}` is the full 40-character commit SHA, not a short SHA. The old version used `git rev-parse --short HEAD` to produce the 7-character abbreviated form. If the companion workflow in `harmony-labs/meta#62` reads `short_sha` expecting a short ref (e.g. for display, tagging, or matching logic), it will receive the full SHA instead, which may break those downstream consumers.
```suggestion
"sha": "${{ github.sha }}",
```
Or, if the short form is truly needed, add a dedicated step to compute it:
```yaml
- name: Compute short SHA
id: vars
run: echo "short_sha=$(git rev-parse --short HEAD)" >> "$GITHUB_OUTPUT"
```
and then use `${{ steps.vars.outputs.short_sha }}`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: .github/workflows/notify-parent.yml
Line: 11-12
Comment:
**Checkout step is no longer needed**
The simplified workflow doesn't use any files from the repository or run any shell commands that require a checkout. All values (`github.sha`, `github.event.repository.name`, `github.actor`) are available directly from the GitHub context without checking out the code. Removing this step would save a few seconds of CI time per push to `main`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: .github/workflows/notify-parent.yml
Line: 22-24
Comment:
**Inconsistent JSON string quoting**
`repo_name` and `actor` use `toJSON()` to produce properly-escaped JSON strings (e.g. `"loop_cli"`), but `short_sha` uses manual double-quote wrapping (`"${{ github.sha }}"`). While a hex SHA is safe from escaping issues in practice, it's inconsistent and would silently produce malformed JSON if the value ever contained special characters.
```suggestion
"repo_name": ${{ toJSON(github.event.repository.name) }},
"short_sha": ${{ toJSON(github.sha) }},
"actor": ${{ toJSON(github.actor) }}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "chore: restore notify-parent workflow (s..." | Re-trigger Greptile
| "type": ${{ toJSON(steps.source.outputs.type) }}, | ||
| "actor": ${{ toJSON(steps.source.outputs.actor) }} | ||
| "repo_name": ${{ toJSON(github.event.repository.name) }}, | ||
| "short_sha": "${{ github.sha }}", |
There was a problem hiding this comment.
short_sha field contains the full SHA
${{ github.sha }} is the full 40-character commit SHA, not a short SHA. The old version used git rev-parse --short HEAD to produce the 7-character abbreviated form. If the companion workflow in harmony-labs/meta#62 reads short_sha expecting a short ref (e.g. for display, tagging, or matching logic), it will receive the full SHA instead, which may break those downstream consumers.
| "short_sha": "${{ github.sha }}", | |
| "sha": "${{ github.sha }}", |
Or, if the short form is truly needed, add a dedicated step to compute it:
- name: Compute short SHA
id: vars
run: echo "short_sha=$(git rev-parse --short HEAD)" >> "$GITHUB_OUTPUT"and then use ${{ steps.vars.outputs.short_sha }}.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/notify-parent.yml
Line: 23
Comment:
**`short_sha` field contains the full SHA**
`${{ github.sha }}` is the full 40-character commit SHA, not a short SHA. The old version used `git rev-parse --short HEAD` to produce the 7-character abbreviated form. If the companion workflow in `harmony-labs/meta#62` reads `short_sha` expecting a short ref (e.g. for display, tagging, or matching logic), it will receive the full SHA instead, which may break those downstream consumers.
```suggestion
"sha": "${{ github.sha }}",
```
Or, if the short form is truly needed, add a dedicated step to compute it:
```yaml
- name: Compute short SHA
id: vars
run: echo "short_sha=$(git rev-parse --short HEAD)" >> "$GITHUB_OUTPUT"
```
and then use `${{ steps.vars.outputs.short_sha }}`.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed — renamed short_sha to sha (sender sends full SHA, parent truncates to 7 chars), removed checkout step, added permissions: {} block.
| - name: Checkout | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
The simplified workflow doesn't use any files from the repository or run any shell commands that require a checkout. All values (github.sha, github.event.repository.name, github.actor) are available directly from the GitHub context without checking out the code. Removing this step would save a few seconds of CI time per push to main.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/notify-parent.yml
Line: 11-12
Comment:
**Checkout step is no longer needed**
The simplified workflow doesn't use any files from the repository or run any shell commands that require a checkout. All values (`github.sha`, `github.event.repository.name`, `github.actor`) are available directly from the GitHub context without checking out the code. Removing this step would save a few seconds of CI time per push to `main`.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed — renamed short_sha to sha (sender sends full SHA, parent truncates to 7 chars), removed checkout step, added permissions: {} block.
| "repo_name": ${{ toJSON(github.event.repository.name) }}, | ||
| "short_sha": "${{ github.sha }}", | ||
| "actor": ${{ toJSON(github.actor) }} |
There was a problem hiding this comment.
repo_name and actor use toJSON() to produce properly-escaped JSON strings (e.g. "loop_cli"), but short_sha uses manual double-quote wrapping ("${{ github.sha }}"). While a hex SHA is safe from escaping issues in practice, it's inconsistent and would silently produce malformed JSON if the value ever contained special characters.
| "repo_name": ${{ toJSON(github.event.repository.name) }}, | |
| "short_sha": "${{ github.sha }}", | |
| "actor": ${{ toJSON(github.actor) }} | |
| "repo_name": ${{ toJSON(github.event.repository.name) }}, | |
| "short_sha": ${{ toJSON(github.sha) }}, | |
| "actor": ${{ toJSON(github.actor) }} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/notify-parent.yml
Line: 22-24
Comment:
**Inconsistent JSON string quoting**
`repo_name` and `actor` use `toJSON()` to produce properly-escaped JSON strings (e.g. `"loop_cli"`), but `short_sha` uses manual double-quote wrapping (`"${{ github.sha }}"`). While a hex SHA is safe from escaping issues in practice, it's inconsistent and would silently produce malformed JSON if the value ever contained special characters.
```suggestion
"repo_name": ${{ toJSON(github.event.repository.name) }},
"short_sha": ${{ toJSON(github.sha) }},
"actor": ${{ toJSON(github.actor) }}
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed — renamed short_sha to sha (sender sends full SHA, parent truncates to 7 chars), removed checkout step, added permissions: {} block.
e0f85d2 to
1054c7a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/notify-parent.yml:
- Line 12: Replace the mutable tag "peter-evans/repository-dispatch@v4" with the
corresponding immutable commit SHA for the release; locate the workflow step
that uses the string "uses: peter-evans/repository-dispatch@v4" and update it to
"uses: peter-evans/repository-dispatch@<COMMIT_SHA>" (the full 40-character
commit) to pin the action to an immutable reference.
- Around line 3-10: Add an explicit least-privilege permissions block to the
workflow to deny unnecessary GITHUB_TOKEN access: update
.github/workflows/notify-parent.yml and add a top-level permissions entry (e.g.,
permissions: none or only the minimal required scopes) so the notify job and the
peter-evans/repository-dispatch@v4 step do not inherit broad GITHUB_TOKEN
permissions; ensure the permissions block is present above the jobs section and
only grants any specific permission if the workflow actually needs it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96510449-7554-4938-8bc7-e2b8d13de791
📒 Files selected for processing (1)
.github/workflows/notify-parent.yml
1054c7a to
3c3d9b2
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/notify-parent.yml (2)
7-9:⚠️ Potential issue | 🟠 MajorAdd explicit least-privilege workflow permissions.
This workflow does not need
GITHUB_TOKENscopes for the dispatch step (it usessecrets.PARENT_REPO_PAT), so declare explicit minimal permissions to avoid inherited defaults.Suggested fix
on: push: branches: [main] +permissions: {} + jobs: notify:🤖 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 7 - 9, The workflow is missing explicit least-privilege permissions, so add an explicit permissions block (e.g. set permissions: none or the minimal required scopes) to avoid inheriting broader GITHUB_TOKEN rights; update the workflow (job named "notify" in notify-parent.yml) by adding a permissions entry either at the top-level or under jobs.notify (e.g. jobs.notify.permissions: none) so the workflow does not grant unnecessary token scopes.
12-12:⚠️ Potential issue | 🟠 MajorPin
peter-evans/repository-dispatchto an immutable full commit SHA.Line 12 currently uses a mutable tag (
@v4), which weakens supply-chain integrity.Suggested fix
- uses: peter-evans/repository-dispatch@v4 + uses: peter-evans/repository-dispatch@<FULL_40_CHAR_COMMIT_SHA>Use this read-only script to resolve the current
v4tag to its full commit SHA before updating:#!/bin/bash set -euo pipefail echo "Current workflow ref:" rg -n 'uses:\s*peter-evans/repository-dispatch@' .github/workflows/notify-parent.yml echo echo "Resolved full commit SHA for tag v4:" obj_type=$(gh api repos/peter-evans/repository-dispatch/git/ref/tags/v4 --jq '.object.type') obj_sha=$(gh api repos/peter-evans/repository-dispatch/git/ref/tags/v4 --jq '.object.sha') if [ "$obj_type" = "tag" ]; then gh api repos/peter-evans/repository-dispatch/git/tags/"$obj_sha" --jq '.object.sha' else echo "$obj_sha" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/notify-parent.yml at line 12, The workflow currently pins the step using the mutable tag "uses: peter-evans/repository-dispatch@v4"; replace that with the repository-dispatch action's immutable full commit SHA instead (resolve the SHA for tag v4 via the GitHub API or the provided script) and update the "uses: peter-evans/repository-dispatch@<sha>" line so the action is referenced by its full commit SHA rather than the `@v4` tag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/notify-parent.yml:
- Around line 7-9: The workflow is missing explicit least-privilege permissions,
so add an explicit permissions block (e.g. set permissions: none or the minimal
required scopes) to avoid inheriting broader GITHUB_TOKEN rights; update the
workflow (job named "notify" in notify-parent.yml) by adding a permissions entry
either at the top-level or under jobs.notify (e.g. jobs.notify.permissions:
none) so the workflow does not grant unnecessary token scopes.
- Line 12: The workflow currently pins the step using the mutable tag "uses:
peter-evans/repository-dispatch@v4"; replace that with the repository-dispatch
action's immutable full commit SHA instead (resolve the SHA for tag v4 via the
GitHub API or the provided script) and update the "uses:
peter-evans/repository-dispatch@<sha>" line so the action is referenced by its
full commit SHA rather than the `@v4` tag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e33f9285-4d5a-415e-b067-abca33acfb81
📒 Files selected for processing (1)
.github/workflows/notify-parent.yml
3c3d9b2 to
9662505
Compare
Re-adds notify-parent.yml to trigger parent meta repo's release-please when this child repo merges to main. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
9662505 to
924b2e5
Compare
Re-adds
notify-parent.ymlto trigger parent meta repo's release-please when this child repo merges to main.Simplified: no commit message parsing, just dispatches repo name and SHA. Companion to gitkb/meta#62.
🤖 Generated with Claude Code
Summary by CodeRabbit