feat(sync-main-to-edge): open a PR when direct push can't fast-forward

[alex-nsheaps]

Summary

Per handler directive: the main→edge sync workflow must open a PR when it can't push directly to the edge branch, instead of failing.

The reusable sync-main-to-edge.yaml uses aormsby/Fork-Sync-With-Upstream-action to fast-forward upstream → target. That direct push fails when the target (edge) branch has diverged from upstream or is branch-protected. Previously the job just errored out.

Changes

• Added pull-requests: write to the workflow permissions block.
• Marked the "Sync upstream changes" (aormsby) step continue-on-error: true.
• Added an "Open PR if direct sync could not push" step, gated on steps.sync.outcome == 'failure'. It is idempotent: skips if a PR is already open for the same head/base, and tolerates the no-diff case via || echo.
• Updated the "Report" step to cover the new PR-fallback outcome.

Behavior matrix

Situation	Before	After
Fast-forwardable	push succeeds	push succeeds (unchanged)
Diverged history	job fails	PR opened upstream → target
Branch-protected target	job fails	PR opened upstream → target
No diff	n/a	no-op (echo)

Test plan

• YAML parses (validated locally with yaml.safe_load)
• CI green
• Henry review

🤖 Generated with Claude Code

[henry-nsheaps]

test ping

[henry-nsheaps]

Review: Correctness 88% | Security 97% | Error Handling 75% | Maintainability 90%

Summary

The PR correctly implements the handler directive: when the aormsby fast-forward push fails (diverged history or branch protection), the workflow now opens a PR upstream → target instead of erroring out. The overall approach is sound — continue-on-error on the sync step, idempotent gh pr list check before gh pr create, and set -euo pipefail throughout.

Three follow-up items below, all P2 (none block merge).

---

P2 — || echo on gh pr create swallows all errors, not just the no-diff case

File: .github/workflows/sync-main-to-edge.yaml (new "Open PR" step, gh pr create line)

gh pr create --repo "$REPO" --base "$TARGET" --head "$UPSTREAM" \
  --title "sync: $UPSTREAM -> $TARGET" \
  --body "..." \
  || echo "No PR created (possibly no diff between $UPSTREAM and $TARGET)."

|| echo catches every non-zero exit from gh pr create — not only the no-diff case, but also transient failures (API rate limit, network error, auth issue despite the pull-requests: write permission). When that happens, the step exits 0 and the Report step prints "opened/ensured a PR instead" — a false positive that could mislead operators.

gh pr create exits non-zero for at least two distinct reasons here:

1. No diff between branches (expected — swallow it)
2. Any other API/network failure (unexpected — surface it)

Suggestion: capture stderr to distinguish, or at minimum log a warning that PR creation may have failed:

if ! gh pr create --repo "$REPO" --base "$TARGET" --head "$UPSTREAM" \
    --title "sync: $UPSTREAM -> $TARGET" \
    --body "..."; then
  echo "::warning::gh pr create exited non-zero for $UPSTREAM -> $TARGET (no diff, or API error). Check logs."
fi

The ::warning:: annotation surfaces in the Actions UI without failing the step.

---

P2 — Report step says "opened/ensured a PR" even when no PR was created

File: .github/workflows/sync-main-to-edge.yaml (Report step, failure branch)

if [ "${{ steps.sync.outcome }}" = "failure" ]; then
  echo "Direct sync could not push ${{ matrix.upstream }} -> ${{ matrix.target }}; opened/ensured a PR instead."

This message prints whenever steps.sync.outcome == 'failure', regardless of whether gh pr create actually created a PR or hit the || echo fallback. When there's no diff (or a silent API failure), the report is misleading.

Suggestion: Set a step output from the "Open PR" step to track whether a PR was actually created or skipped, and use that in Report:

echo "pr_created=true" >> "$GITHUB_OUTPUT"
# vs
echo "pr_created=false" >> "$GITHUB_OUTPUT"

Then in Report:

if [ "${{ steps.sync.outcome }}" = "failure" ] && [ "${{ steps.open_pr.outputs.pr_created }}" = "true" ]; then
  echo "Opened PR ..."
elif [ "${{ steps.sync.outcome }}" = "failure" ]; then
  echo "Direct sync failed but no PR was needed (no diff)."

---

P2 — test_mode not gated in PR fallback step (low actual risk)

File: .github/workflows/sync-main-to-edge.yaml (new "Open PR" step condition)

In test mode, aormsby runs a dry-run and should succeed (no push, no failure). So steps.sync.outcome == 'failure' would not normally fire in test mode, and the fallback would not trigger. This is low actual risk — but if aormsby ever fails in test mode for a reason unrelated to divergence (e.g., a config error), the fallback would open a real PR while the caller expects test mode to be a no-op.

Suggestion: Add an early guard to the fallback step for completeness:

if [ "${{ inputs.test_mode }}" = "true" ]; then
  echo "Test mode: skipping PR creation."
  exit 0
fi

Findings:

• P2: || echo on gh pr create swallows all non-zero exits — real API failures silently become false-positive "PR opened" reports
• P2: Report step message "opened/ensured a PR instead" fires even when no PR was actually created (no-diff or silent error path)
• P2: test_mode not explicitly gated in the PR fallback step (low actual risk — aormsby succeeds in test mode, so this path is unlikely to fire)

Verdict: COMMENT — no blocking issues. The core fallback logic is correct and safe to merge. The P2s are observability improvements worth a follow-up.

---

Henry Oldenburg — automated review · nsheaps/github-actions#48

[nsheaps]

There should be a re-usable action that's commonly used in the nsheaps org that you should reuse here

[nsheaps]

https://github.com/nsheaps/.github/blob/main/.github/workflows/sync-repo-settings.yaml#L62-L122

Something like this maybe should be put in it's own re-usable action

[nsheaps]

The body should have a link back to this workflow captured the same way other nsheaps ones do.

[alex-nsheaps]

Addressing review feedback. Two-step plan per your guidance:

Step 1 — Extract the reusable composite (separate PR): #50 adds .github/actions/open-pr-if-needed/, a composite that idempotently opens a PR between two existing branches and auto-appends the Workflow run: self-link. Pattern extracted from sync-repo-settings.yaml L62-122 as you suggested.

Step 2 — This PR will consume the composite: once #50 lands, I'll push a follow-up commit here that swaps the inline gh pr create block for uses: ./.github/actions/open-pr-if-needed. Until then, this PR's inline block already addresses your second comment (workflow run self-link in body — e74d037).

This PR is blocked on #50 for the full fix. Re-requesting your review so you can see the plan; will re-request again after the consumer-swap commit lands.