Skip to content

chore: restore simplified on-child-update workflow - #62

Merged
mateodelnorte merged 1 commit into
mainfrom
fix/restore-child-notify
Mar 26, 2026
Merged

mateodelnorte merged 1 commit into
mainfrom
fix/restore-child-notify

Conversation

@mateodelnorte

@mateodelnorte mateodelnorte commented Mar 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Re-adds on-child-update.yml to create sync PRs when child repos merge to main. This feeds commits to release-please so child repo updates are included in parent releases.

Simplified from the old version that was removed in #60:

  • No commit message parsing (the old version broke on parens in subjects)
  • Concurrency group prevents race conditions on simultaneous merges
  • Graceful conflict handling (race losers exit cleanly)

Why it was removed and why it's back

Removed in #60 because the sync PRs were "empty noise." But they serve a purpose: without sync commits on main, release-please has nothing to track and child repo updates never trigger parent releases.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Added an automated synchronization workflow that responds to child repository updates: it creates or recreates deterministic sync branches, opens pull requests against main, attempts automatic merges when possible, and cleans up orphaned branches. The process tolerates race conditions and merge or push failures while ensuring updates are propagated to the parent repository.

@coderabbitai

coderabbitai Bot commented Mar 26, 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 3 minutes and 5 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: 341b48d8-729a-440a-8fd5-385848629e86

📥 Commits

Reviewing files that changed from the base of the PR and between bb5de9f and aa1ec3c.

📒 Files selected for processing (1)
  • .github/workflows/on-child-update.yml

Walkthrough

Adds a new GitHub Actions workflow triggered by repository_dispatch type child-repo-updated that creates deterministic sync/<repo>/<short_sha> branches and PRs, idempotently reconciles existing branches/PRs (merge or delete), creates a sync commit, pushes, opens a PR to main, and attempts auto-merge while tolerating race conditions.

Changes

Cohort / File(s) Summary
GitHub Actions workflow
.github/workflows/on-child-update.yml
New workflow that handles child-repo-updated dispatches: computes deterministic branch and PR title, checks for existing branch+PR (merges if found, deletes orphaned branch), creates an empty sync commit, pushes (treats push races as success), opens PR to main, and attempts auto-merge with branch deletion on success/failure handling.

Sequence Diagram(s)

sequenceDiagram
    participant Dispatcher as External Dispatcher
    participant Runner as GitHub Actions Runner
    participant GitHubAPI as GitHub API
    participant Repo as Parent Repository (git)

    Dispatcher->>Runner: repository_dispatch (child-repo-updated) payload
    Runner->>GitHubAPI: authenticate using PARENT_REPO_PAT
    Runner->>GitHubAPI: check if branch `sync/<repo>/<short_sha>` exists
    alt branch exists
        Runner->>GitHubAPI: find PR where head==branch && base==main
        alt PR exists
            Runner->>GitHubAPI: attempt auto-merge (squash, delete-branch)
            GitHubAPI-->>Runner: merge result
            Runner-->>Dispatcher: exit success
        else no PR
            Runner->>GitHubAPI: delete orphan branch
        end
    end
    Runner->>Repo: create empty commit on new sync branch
    Runner->>Repo: push branch (treat push failure from race as success)
    Runner->>GitHubAPI: create PR to `main` (title from payload)
    Runner->>GitHubAPI: attempt auto-merge (squash, delete-branch)
    GitHubAPI-->>Runner: merge result (ok/fail)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped on a dispatch, quick and spry,
I carved a sync branch up to the sky,
I nudged a tiny commit and raised a PR high,
Tried to merge and tidy—then caught a wink and a sigh. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: restore simplified on-child-update workflow' accurately reflects the main change: re-adding and simplifying the on-child-update.yml workflow file that was previously removed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/restore-child-notify

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

@greptile-apps

greptile-apps Bot commented Mar 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR restores .github/workflows/on-child-update.yml, a repository_dispatch-triggered workflow that creates empty-commit sync PRs on the parent repo whenever a child repo merges to main. The sync commits give release-please something to track so child repo updates propagate into parent releases. The new version is simpler than the one removed in #60: no commit-message parsing (which previously broke on parentheses), a static concurrency group to serialise concurrent dispatches, and graceful exit 0 for race losers.\n\nKey changes:\n- New workflow file .github/workflows/on-child-update.yml triggered on child-repo-updated repository dispatch events\n- Concurrency group child-repo-sync (non-cancelling) serialises simultaneous child updates\n- Creates a branch sync/<repo>/<sha>, pushes an empty commit, opens a PR, and enables auto-merge with --squash\n- Duplicate-dispatch guard: if the branch already exists and a PR is open, it merges that PR and exits early\n\nIssues found:\n- P1 – If gh pr create fails after the branch push, the branch is left orphaned on the remote. Subsequent re-deliveries of the same event detect the branch, find no open PR, then fail on the push step and exit silently — permanently dropping that child update.\n- P2 – The Co-authored-by trailer uses ${ACTOR}@users.noreply.github.com which lacks the required numeric GitHub user ID prefix and won't resolve to a real profile in the GitHub UI.

Confidence Score: 3/5

Safe to merge in nominal conditions, but a realistic failure mode (transient gh pr create error) permanently silences that child update with no recovery path.

The concurrency group and race-condition guards are well thought out for the happy path. However, the orphaned-branch scenario (push succeeds, PR creation fails) breaks the core guarantee the workflow exists to provide — future retries for the same SHA will silently no-op. This is a plausible failure mode (API flakiness, rate limits) rather than a theoretical one, which keeps the score at 3 rather than 4.

.github/workflows/on-child-update.yml — specifically the PR-creation error handler at lines 63–70 and the existing-branch guard at lines 42–49.

Important Files Changed

Filename Overview
.github/workflows/on-child-update.yml Re-introduces the sync-PR workflow with concurrency protection and graceful race handling; contains a P1 logic gap where an orphaned branch (push succeeded, PR creation failed) permanently prevents future re-delivery of the same SHA.

Sequence Diagram

sequenceDiagram
    participant CR as Child Repo
    participant GH as GitHub Actions
    participant CG as Concurrency Group
    participant Git as git / origin
    participant GHAPI as gh CLI (API)
    participant RP as release-please

    CR->>GH: repository_dispatch child-repo-updated
    GH->>CG: acquire child-repo-sync slot
    CG-->>GH: slot acquired
    GH->>Git: git ls-remote origin refs/heads/sync/repo/sha
    alt Branch already exists
        GH->>GHAPI: gh pr list --head branch
        alt Open PR found
            GH->>GHAPI: gh pr merge --auto --squash
            GH-->>CG: exit 0 (duplicate handled)
        else No open PR (orphaned branch)
            GH->>Git: git push (fails — branch exists)
            GH-->>CG: exit 0 (silently drops update)
        end
    else Branch does not exist
        GH->>Git: git checkout -b sync/repo/sha
        GH->>Git: git commit --allow-empty
        GH->>Git: git push -u origin branch
        alt Push succeeds
            GH->>GHAPI: gh pr create
            alt PR created
                GH->>GHAPI: gh pr merge --auto --squash --delete-branch
                GHAPI-->>RP: merge triggers on-push-main release-please
            else PR creation fails
                GH-->>CG: exit 0 (orphaned branch left behind)
            end
        else Push fails (race)
            GH-->>CG: exit 0 (race loser)
        end
    end
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/on-child-update.yml
Line: 63-70

Comment:
**Orphaned branch silently swallows future retries**

If `gh pr create` fails for any non-race reason (GitHub API error, rate limit, etc.), the branch `sync/${REPO_NAME}/${SHORT_SHA}` is left on the remote with no PR. On any subsequent dispatch for the *same* SHA, execution reaches line 42, finds the branch exists, queries for an open PR, gets nothing back, and falls through to the main path — where the push at line 58 immediately fails and the run exits cleanly at line 60. The update is silently and permanently dropped with no recovery path.

This is distinct from the intended race-condition case: the concurrency group serialises runs, so true race losers are fine. The problem is `gh pr create` failing for unrelated reasons *after* the push has already succeeded.

A safer pattern would be to check for the branch+no-PR case explicitly and attempt PR creation again, rather than treating it as a completed race:

```
# Inside the existing-branch block
EXISTING_PR=$(gh pr list --head "$BRANCH" --base main --json url --jq '.[0].url // empty')
if [[ -n "$EXISTING_PR" ]]; then
  echo "PR already exists: $EXISTING_PR"
  gh pr merge "$EXISTING_PR" --auto --squash --delete-branch || true
  exit 0
fi
# Branch exists but no PR — previous run pushed but failed to create the PR; fall through to create it
echo "Branch exists but no PR found — attempting PR creation"
git fetch origin "$BRANCH"
git checkout "$BRANCH"
# skip the commit/push steps and jump straight to gh pr create
```

Alternatively, delete the orphaned branch and start fresh so the normal flow can proceed.

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/on-child-update.yml
Line: 56

Comment:
**`Co-authored-by` email won't resolve to a GitHub profile**

GitHub's noreply address format that links to a user account requires the numeric user ID prefix: `<numericId+username@users.noreply.github.com>`. Without the ID, the trailer is syntactically valid but GitHub won't associate it with the actor's account — the attribution will appear as an unknown author in the commit UI.

Since only the username is available from the payload (no numeric ID), one reasonable alternative is to omit the trailer entirely and rely on the branch name / PR description for attribution, or use the generic bot address:

```suggestion
          Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>"
```

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

Reviews (1): Last reviewed commit: "chore: restore simplified on-child-updat..." | Re-trigger Greptile

Comment on lines +63 to +70
PR_URL=$(gh pr create \
--title "$PR_TITLE" \
--body "Synced from \`${REPO_NAME}@${SHORT_SHA}\`" \
--head "$BRANCH" \
--base main) || {
echo "PR creation failed — likely race condition"
exit 0
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Orphaned branch silently swallows future retries

If gh pr create fails for any non-race reason (GitHub API error, rate limit, etc.), the branch sync/${REPO_NAME}/${SHORT_SHA} is left on the remote with no PR. On any subsequent dispatch for the same SHA, execution reaches line 42, finds the branch exists, queries for an open PR, gets nothing back, and falls through to the main path — where the push at line 58 immediately fails and the run exits cleanly at line 60. The update is silently and permanently dropped with no recovery path.

This is distinct from the intended race-condition case: the concurrency group serialises runs, so true race losers are fine. The problem is gh pr create failing for unrelated reasons after the push has already succeeded.

A safer pattern would be to check for the branch+no-PR case explicitly and attempt PR creation again, rather than treating it as a completed race:

# Inside the existing-branch block
EXISTING_PR=$(gh pr list --head "$BRANCH" --base main --json url --jq '.[0].url // empty')
if [[ -n "$EXISTING_PR" ]]; then
  echo "PR already exists: $EXISTING_PR"
  gh pr merge "$EXISTING_PR" --auto --squash --delete-branch || true
  exit 0
fi
# Branch exists but no PR — previous run pushed but failed to create the PR; fall through to create it
echo "Branch exists but no PR found — attempting PR creation"
git fetch origin "$BRANCH"
git checkout "$BRANCH"
# skip the commit/push steps and jump straight to gh pr create

Alternatively, delete the orphaned branch and start fresh so the normal flow can proceed.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/on-child-update.yml
Line: 63-70

Comment:
**Orphaned branch silently swallows future retries**

If `gh pr create` fails for any non-race reason (GitHub API error, rate limit, etc.), the branch `sync/${REPO_NAME}/${SHORT_SHA}` is left on the remote with no PR. On any subsequent dispatch for the *same* SHA, execution reaches line 42, finds the branch exists, queries for an open PR, gets nothing back, and falls through to the main path — where the push at line 58 immediately fails and the run exits cleanly at line 60. The update is silently and permanently dropped with no recovery path.

This is distinct from the intended race-condition case: the concurrency group serialises runs, so true race losers are fine. The problem is `gh pr create` failing for unrelated reasons *after* the push has already succeeded.

A safer pattern would be to check for the branch+no-PR case explicitly and attempt PR creation again, rather than treating it as a completed race:

```
# Inside the existing-branch block
EXISTING_PR=$(gh pr list --head "$BRANCH" --base main --json url --jq '.[0].url // empty')
if [[ -n "$EXISTING_PR" ]]; then
  echo "PR already exists: $EXISTING_PR"
  gh pr merge "$EXISTING_PR" --auto --squash --delete-branch || true
  exit 0
fi
# Branch exists but no PR — previous run pushed but failed to create the PR; fall through to create it
echo "Branch exists but no PR found — attempting PR creation"
git fetch origin "$BRANCH"
git checkout "$BRANCH"
# skip the commit/push steps and jump straight to gh pr create
```

Alternatively, delete the orphaned branch and start fresh so the normal flow can proceed.

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 in 64031a6.

Comment thread .github/workflows/on-child-update.yml Outdated

Synced from ${REPO_NAME}@${SHORT_SHA}

Co-authored-by: ${ACTOR} <${ACTOR}@users.noreply.github.com>"

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 Co-authored-by email won't resolve to a GitHub profile

GitHub's noreply address format that links to a user account requires the numeric user ID prefix: <numericId+username@users.noreply.github.com>. Without the ID, the trailer is syntactically valid but GitHub won't associate it with the actor's account — the attribution will appear as an unknown author in the commit UI.

Since only the username is available from the payload (no numeric ID), one reasonable alternative is to omit the trailer entirely and rely on the branch name / PR description for attribution, or use the generic bot address:

Suggested change
Co-authored-by: ${ACTOR} <${ACTOR}@users.noreply.github.com>"
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>"
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/on-child-update.yml
Line: 56

Comment:
**`Co-authored-by` email won't resolve to a GitHub profile**

GitHub's noreply address format that links to a user account requires the numeric user ID prefix: `<numericId+username@users.noreply.github.com>`. Without the ID, the trailer is syntactically valid but GitHub won't associate it with the actor's account — the attribution will appear as an unknown author in the commit UI.

Since only the username is available from the payload (no numeric ID), one reasonable alternative is to omit the trailer entirely and rely on the branch name / PR description for attribution, or use the generic bot address:

```suggestion
          Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>"
```

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 in 64031a6.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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/on-child-update.yml:
- Around line 41-61: The workflow currently treats a remote branch without a PR
as a race and falls through to git checkout -b and git push; instead, after
detecting the remote branch exists (the git ls-remote check) but EXISTING_PR is
empty, explicitly handle that path by checking out the remote branch (git
checkout --track origin/"$BRANCH" or git fetch + git checkout), create a PR for
that branch using gh pr create --head "$BRANCH" --base main with the same
title/body metadata, then proceed to merge with gh pr merge "$PR_URL" --auto
--squash --delete-branch (or use the returned URL from gh pr create) rather than
attempting git checkout -b / git push which fails; ensure you still exit
successfully after merging to match the existing flow.
- Around line 34-40: Sanitize and validate payload-derived vars before using
them: ensure REPO_NAME, SHORT_SHA, and ACTOR are normalized (strip/replace
unsafe chars, e.g. allow only [A-Za-z0-9._-], replace others with '-') and
truncate to safe lengths, build BRANCH using the sanitized REPO_NAME and
sanitized SHORT_SHA; for SHORT_SHA prefer a real git short hash (e.g., git
rev-parse --short HEAD) and only fall back to a numeric timestamp if no commit
hash is available, and ensure PR_TITLE uses the sanitized values; when
constructing Co-authored-by trailers, derive a safe email/name from sanitized
ACTOR (escape/omit characters that would create invalid email syntax) and format
it as "Co-authored-by: Name <name@users.noreply.github.com>" so git commit
metadata cannot be malformed.
🪄 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: b91adfcf-67dc-4712-999f-7f8834278799

📥 Commits

Reviewing files that changed from the base of the PR and between dfe433f and 9927d3e.

📒 Files selected for processing (1)
  • .github/workflows/on-child-update.yml

Comment on lines +34 to +40
REPO_NAME="${REPO_NAME:-unknown}"
SHORT_SHA="${SHORT_SHA:-$(date +%s | tail -c 8)}"
ACTOR="${ACTOR:-github-actions}"

BRANCH="sync/${REPO_NAME}/${SHORT_SHA}"
PR_TITLE="chore(${REPO_NAME}): sync ${SHORT_SHA}"

@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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/on-child-update.yml

Repository: harmony-labs/meta

Length of output: 2893


🏁 Script executed:

find . -type f \( -name "*.md" -o -name "*.txt" -o -name "*test*" -o -name "*spec*" \) | head -20

Repository: harmony-labs/meta

Length of output: 762


🏁 Script executed:

rg -i "client_payload|repository_dispatch|REPO_NAME|SHORT_SHA" --type md --type txt

Repository: harmony-labs/meta

Length of output: 220


🏁 Script executed:

rg "repository_dispatch|on:.*child-repo-updated" --type yaml -A 5 -B 5

Repository: harmony-labs/meta

Length of output: 43


🏁 Script executed:

find . -name "*.md" -exec rg -l "child.*repo|payload.*struct|dispatch" {} \;

Repository: harmony-labs/meta

Length of output: 510


🏁 Script executed:

grep -n "repo_name\|short_sha\|actor\|payload" README.md | head -20

Repository: harmony-labs/meta

Length of output: 324


🏁 Script executed:

grep -n "child.*repo\|dispatch" ./docs/*.md

Repository: harmony-labs/meta

Length of output: 1632


🏁 Script executed:

rg "repository_dispatch\|gh.*dispatch\|client_payload" --type yml --type yaml --type sh --type bash -B 3 -A 3

Repository: harmony-labs/meta

Length of output: 88


🏁 Script executed:

find . -type f -name "*.yml" -o -name "*.yaml" | xargs grep -l "dispatch" 2>/dev/null

Repository: harmony-labs/meta

Length of output: 169


🏁 Script executed:

cat -n .github/workflows/on-push-main.yml

Repository: harmony-labs/meta

Length of output: 1728


🏁 Script executed:

cat -n .github/workflows/release.yml

Repository: harmony-labs/meta

Length of output: 15447


🏁 Script executed:

rg "short_sha|SHORT_SHA" -i --type md

Repository: harmony-labs/meta

Length of output: 43


Validate and sanitize dispatch payload fields before using them in git refs and commit metadata.

REPO_NAME, SHORT_SHA, and ACTOR are sourced from github.event.client_payload and used directly in branch names, PR titles, and commit messages. Malformed payload values can cause git operations to fail, and the SHORT_SHA default ($(date +%s | tail -c 8)) produces an 8-digit timestamp instead of a commit hash, undermining traceability. Additionally, unvalidated values in the Co-authored-by trailer can produce malformed email addresses.

Proposed hardening patch
-          REPO_NAME="${REPO_NAME:-unknown}"
-          SHORT_SHA="${SHORT_SHA:-$(date +%s | tail -c 8)}"
-          ACTOR="${ACTOR:-github-actions}"
+          : "${REPO_NAME:?Missing client_payload.repo_name}"
+          : "${SHORT_SHA:?Missing client_payload.short_sha}"
+          ACTOR="${ACTOR:-github-actions}"
+
+          SAFE_REPO_NAME="$(printf '%s' "$REPO_NAME" | tr '[:upper:]' '[:lower:]' | sed -E 's#[^a-z0-9._/-]+#-#g; s#(^/+|/+$)##g')"
+          SAFE_SHORT_SHA="$(printf '%s' "$SHORT_SHA" | tr -cd '[:xdigit:]' | cut -c1-12)"
+          SAFE_ACTOR="$(printf '%s' "$ACTOR" | tr -cd '[:alnum:]-')"
+
+          if [[ -z "$SAFE_REPO_NAME" || -z "$SAFE_SHORT_SHA" ]]; then
+            echo "Invalid payload after sanitization" >&2
+            exit 1
+          fi
 
-          BRANCH="sync/${REPO_NAME}/${SHORT_SHA}"
-          PR_TITLE="chore(${REPO_NAME}): sync ${SHORT_SHA}"
+          BRANCH="sync/${SAFE_REPO_NAME}/${SAFE_SHORT_SHA}"
+          PR_TITLE="chore(${SAFE_REPO_NAME}): sync ${SAFE_SHORT_SHA}"
@@
-          Synced from ${REPO_NAME}@${SHORT_SHA}
+          Synced from ${SAFE_REPO_NAME}@${SAFE_SHORT_SHA}
@@
-          Co-authored-by: ${ACTOR} <${ACTOR}@users.noreply.github.com>"
+          Co-authored-by: ${SAFE_ACTOR} <${SAFE_ACTOR}@users.noreply.github.com>"

Also applies to: 52-56

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/on-child-update.yml around lines 34 - 40, Sanitize and
validate payload-derived vars before using them: ensure REPO_NAME, SHORT_SHA,
and ACTOR are normalized (strip/replace unsafe chars, e.g. allow only
[A-Za-z0-9._-], replace others with '-') and truncate to safe lengths, build
BRANCH using the sanitized REPO_NAME and sanitized SHORT_SHA; for SHORT_SHA
prefer a real git short hash (e.g., git rev-parse --short HEAD) and only fall
back to a numeric timestamp if no commit hash is available, and ensure PR_TITLE
uses the sanitized values; when constructing Co-authored-by trailers, derive a
safe email/name from sanitized ACTOR (escape/omit characters that would create
invalid email syntax) and format it as "Co-authored-by: Name
<name@users.noreply.github.com>" so git commit metadata cannot be malformed.

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 64031a6.

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

@mateodelnorte, thanks for the fix! I'll verify the changes in commit 64031a6 to confirm the sanitization is in place.


🏁 Script executed:

#!/bin/bash
git show 64031a6 -- .github/workflows/on-child-update.yml 2>/dev/null || git log --oneline | head -10

Length of output: 3429


@mateodelnorte, thanks for making those improvements! Looking at commit 64031a6, I can confirm several concerns were resolved:

  • ✅ ACTOR removed entirely, eliminating the malformed Co-authored-by email issue.
  • ✅ Bot email updated to use the correct numeric ID (41898282+github-actions[bot]@users.noreply.github.com).
  • ✅ Orphaned branch cleanup added.
  • ✅ Failed PR creation now deletes the orphaned branch and exits with an error.

However, the core sanitization concern for REPO_NAME and SHORT_SHA was not addressed — these values are still used directly in git ref names and PR titles without stripping characters unsafe for git refs:

# Still present — no character sanitization:
REPO_NAME="${REPO_NAME:-unknown}"
SHORT_SHA="${SHORT_SHA:-$(date +%s | tail -c 8)}"

BRANCH="sync/${REPO_NAME}/${SHORT_SHA}"   # unsanitized
PR_TITLE="chore(${REPO_NAME}): sync ${SHORT_SHA}"  # unsanitized

A malformed repo_name in the payload (e.g., containing spaces, .., or control characters) could still cause git push to fail with a confusing error. If you trust that all dispatching child repos will always send well-formed values, this may be an acceptable risk — but if you'd like to close the gap, the sanitization lines from the original suggestion would still apply:

SAFE_REPO_NAME="$(printf '%s' "$REPO_NAME" | tr '[:upper:]' '[:lower:]' | sed -E 's#[^a-z0-9._/-]+#-#g; s#(^/+|/+$)##g')"
SAFE_SHORT_SHA="$(printf '%s' "$SHORT_SHA" | tr -cd '[:xdigit:]' | cut -c1-12)"

Would you like to add these, or is the current trust boundary acceptable for this workflow?

Comment thread .github/workflows/on-child-update.yml Outdated
@mateodelnorte
mateodelnorte force-pushed the fix/restore-child-notify branch from 9927d3e to 64031a6 Compare March 26, 2026 12:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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/on-child-update.yml:
- Around line 61-72: The file ends without a trailing newline; update
.github/workflows/on-child-update.yml so the last line (the block using PR_URL,
gh pr create / gh pr merge) is followed by a single newline character—ensure the
final character of the file is a LF so commands like PR_URL, gh pr create and gh
pr merge are terminated by a trailing newline.
- Around line 56-59: The current push block treats any git push failure as a
benign race; change it to capture the git push stderr/exit code from the `git
push -u origin "$BRANCH"` attempt and distinguish a race-condition
non-fast-forward/remote-rejected error from other failures: if the error
text/exit code indicates "non-fast-forward", "failed to push some refs", or a
remote rejected update, keep the existing behaviour (log "another run likely
handled this" and exit 0); for all other errors (permissions, network, branch
protection, etc.) log the full stderr and exit non-zero so CI fails; ensure you
reference and update the existing `git push -u origin "$BRANCH"` block and the
echo message currently used for push failures.
🪄 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: bc9597ac-aa9e-45e0-b437-0cfda546ddf0

📥 Commits

Reviewing files that changed from the base of the PR and between 9927d3e and 64031a6.

📒 Files selected for processing (1)
  • .github/workflows/on-child-update.yml

Comment thread .github/workflows/on-child-update.yml Outdated
Comment thread .github/workflows/on-child-update.yml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
.github/workflows/on-child-update.yml (1)

33-38: ⚠️ Potential issue | 🟠 Major

Fail fast instead of synthesizing unknown/timestamp sync branches.

When repo_name or sha is missing or malformed, this creates a brand-new branch/PR that no longer maps to a real child commit. That breaks deterministic retries and can merge an untraceable sync commit into main. Validate the payload and normalize the ref components before building BRANCH and PR_TITLE.

Proposed hardening patch
-          REPO_NAME="${REPO_NAME:-unknown}"
-          SHORT_SHA="${FULL_SHA:0:7}"
-          SHORT_SHA="${SHORT_SHA:-$(date +%s | tail -c 8)}"
-
-          BRANCH="sync/${REPO_NAME}/${SHORT_SHA}"
-          PR_TITLE="chore(${REPO_NAME}): sync ${SHORT_SHA}"
+          : "${REPO_NAME:?Missing client_payload.repo_name}"
+          : "${FULL_SHA:?Missing client_payload.sha}"
+
+          SAFE_REPO_NAME="$(printf '%s' "$REPO_NAME" | tr '[:upper:]' '[:lower:]' | sed -E 's#[^a-z0-9._/-]+#-#g; s#(^/+|/+$)##g; s#//+#/#g')"
+          SHORT_SHA="$(printf '%s' "$FULL_SHA" | tr '[:upper:]' '[:lower:]' | tr -cd '[:xdigit:]' | cut -c1-7)"
+          [[ -n "$SAFE_REPO_NAME" && -n "$SHORT_SHA" ]] || {
+            echo "Invalid repository_dispatch payload" >&2
+            exit 1
+          }
+
+          BRANCH="sync/${SAFE_REPO_NAME}/${SHORT_SHA}"
+          git check-ref-format --branch "$BRANCH" >/dev/null || {
+            echo "Invalid branch name derived from repository_dispatch payload: $BRANCH" >&2
+            exit 1
+          }
+          PR_TITLE="chore(${SAFE_REPO_NAME}): sync ${SHORT_SHA}"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/on-child-update.yml around lines 33 - 38, The current
workflow silently synthesizes fallback values for REPO_NAME and SHORT_SHA which
can create untraceable branches (variables: REPO_NAME, FULL_SHA, SHORT_SHA,
BRANCH, PR_TITLE); instead validate and normalize inputs and fail fast: check
that REPO_NAME is present and not the placeholder "unknown", verify
FULL_SHA/SHORT_SHA is at least seven hex characters (or derive SHORT_SHA from
FULL_SHA only when valid), normalize REPO_NAME and SHORT_SHA to an allowed
branch-safe charset (e.g., lowercase, replace invalid chars with '-') and if any
validation fails write an error to the log and exit non-zero so no BRANCH or
PR_TITLE is constructed or pushed. Ensure BRANCH and PR_TITLE are built only
after these checks so they always map to a real child commit.
🤖 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/on-child-update.yml:
- Around line 48-51: The orphan-branch deletion is currently treated as
best-effort (git push origin --delete "$BRANCH" || true); after a failed delete
you must re-check the remote ref and fail the workflow if the branch still
exists to avoid later non-fast-forward races. Replace the tolerant delete
pattern by attempting the delete, then run a remote existence check (e.g. git
ls-remote origin "refs/heads/$BRANCH" or git fetch && git rev-parse --verify
"origin/$BRANCH") and if the ref is found emit an error and exit non-zero so the
job stops (referencing the BRANCH variable and the existing git push origin
--delete "$BRANCH" step).

---

Duplicate comments:
In @.github/workflows/on-child-update.yml:
- Around line 33-38: The current workflow silently synthesizes fallback values
for REPO_NAME and SHORT_SHA which can create untraceable branches (variables:
REPO_NAME, FULL_SHA, SHORT_SHA, BRANCH, PR_TITLE); instead validate and
normalize inputs and fail fast: check that REPO_NAME is present and not the
placeholder "unknown", verify FULL_SHA/SHORT_SHA is at least seven hex
characters (or derive SHORT_SHA from FULL_SHA only when valid), normalize
REPO_NAME and SHORT_SHA to an allowed branch-safe charset (e.g., lowercase,
replace invalid chars with '-') and if any validation fails write an error to
the log and exit non-zero so no BRANCH or PR_TITLE is constructed or pushed.
Ensure BRANCH and PR_TITLE are built only after these checks so they always map
to a real child commit.
🪄 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: 9d565ee9-da6d-4439-bd1f-2d63e717be4b

📥 Commits

Reviewing files that changed from the base of the PR and between 64031a6 and bb5de9f.

📒 Files selected for processing (1)
  • .github/workflows/on-child-update.yml

Comment thread .github/workflows/on-child-update.yml Outdated
Re-adds on-child-update.yml to create sync PRs when child repos merge
to main. Feeds commits to release-please for parent releases.

Fixes from review:
- Orphaned branch cleanup: deletes branch+no-PR before recreating
- Failed PR creation: deletes orphaned branch and exits with error
- Co-authored-by: uses github-actions[bot] with proper numeric ID
- Removed actor trailer (can't resolve username to noreply email)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mateodelnorte
mateodelnorte force-pushed the fix/restore-child-notify branch from bb5de9f to aa1ec3c Compare March 26, 2026 13:26
@mateodelnorte
mateodelnorte merged commit 2823e71 into main Mar 26, 2026
8 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