Auto cherry-pick merged OSS PRs into Enterprise (AI-assisted conflict resolution) - #13873
Auto cherry-pick merged OSS PRs into Enterprise (AI-assisted conflict resolution)#13873skoryk-oleksandr wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved security, failure-handling, idempotency, PR-resolution, notification, and release-backport concerns remain.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an AI-assisted, two-stage workflow to cherry-pick merged OSS PRs into Enterprise, resolve conflicts, create target PRs, and notify authors.
Changes:
- Adds generic cherry-pick, PR-resolution, and Slack notification scripts.
- Adds unprivileged trigger and privileged worker workflows.
- Removes the legacy merge-queue dispatch workflow.
File summaries
| File | Description |
|---|---|
.github/workflows/trigger-merge-queue.yml |
Removes the legacy merge-queue trigger. |
.github/workflows/scripts/resolve-merged-pr.js |
Resolves merged source PR metadata. |
.github/workflows/scripts/oss-pick.sh |
Performs cherry-picks and creates target PRs. |
.github/workflows/scripts/notify-slack.js |
Notifies original authors through Slack. |
.github/workflows/auto-pick-oss-to-enterprise.yml |
Runs the privileged pick and conflict-resolution flow. |
.github/workflows/auto-pick-oss-to-enterprise-trigger.yml |
Starts the zero-permission trigger stage. |
Review details
Suppressed comments (9)
.github/workflows/auto-pick-oss-to-enterprise.yml:92
- This privileged workflow executes a mutable third-party action with
CLAUDE_TOKENand repository credentials. The comparable privileged workflow pins its action to a full commit SHA (.github/workflows/cherry_pick_candidate.yml:58); pin this action to a reviewed immutable SHA to prevent tag drift or a supply-chain change from gaining those permissions.
uses: anthropics/claude-code-action@v1
.github/workflows/auto-pick-oss-to-enterprise.yml:46
- The replacement worker only handles
tigera/calico-private@master, while removingtrigger-merge-queue.ymlalso removes the repository'sOSS_MERGEdispatch. No release-branch backport workflow is added here; if merge-queue-bot used that dispatch forcherry-pick-candidaterelease backports, those backports will stop. Verify and replace that behavior before relying on this replacement.
EXTRA_LABELS: merge-oss-cherry-pick
.github/workflows/auto-pick-oss-to-enterprise.yml:145
- The escalation path aborts the cherry-pick but only says to “stop”; it does not require Claude to fail the step. With
continue-on-error, Claude can therefore report success, the Open PR condition runs, andopen-prexits successfully on the now-clean base tree, recording a successful run while silently dropping the pick. Require a nonzero exit (or a checked sentinel) on this path.
code/context that does not exist in Enterprise. In that case run
`git cherry-pick --abort`, explain precisely what a human must decide
in /tmp/resolution-report.md, and stop (do not push).
.github/workflows/auto-pick-oss-to-enterprise.yml:196
- The notification step is described as soft-fail, but this
gh apicommand runs beforenotify-slack.jsand a GitHub Actionsrunshell exits on an unhandled command failure. A transient source-PR API error will therefore fail the workflow instead of skipping the DM; handle this lookup failure explicitly and exit 0 with a warning.
J=$(gh api "repos/${SOURCE_REPO}/pulls/${SRC_PR}")
export SRC_TITLE="$(printf '%s' "$J" | jq -r '.title')"
export SRC_URL="$(printf '%s' "$J" | jq -r '.html_url')"
node "$RUNNER_TEMP/notify-slack.js"
.github/workflows/scripts/notify-slack.js:63
- This sends the Slack member ID directly as
chat.postMessage'schannel. The existing notifier inci-notify.yml:356-383documents that this returnschannel_not_foundand requiresconversations.opento obtain a DM channel first, so the advertised author notification will fail with the token contract declared here. Open the IM and post to the returned channel ID, and update the required Slack scope accordingly.
body: JSON.stringify({ channel: slackId, text, unfurl_links: false }),
signal: AbortSignal.timeout(30000),
.github/workflows/scripts/oss-pick.sh:187
- The resolver is instructed to run
git cherry-pick --abortand stop for an irreconcilable conflict, but this condition still treats the resulting base tree as a successful no-op. The workflow then loses/tmp/resolution-report.mdand completes without a PR or visible failure. TreatOUTCOME=conflictwith no post-resolution commit as an escalation/failure, or require an explicit completed-resolution marker.
if git diff --quiet "origin/${TARGET_BRANCH}" HEAD 2>/dev/null; then
echo "::notice::resolution produced no net change over origin/${TARGET_BRANCH}; nothing to pick"
exit 0
.github/workflows/scripts/oss-pick.sh:68
- These credential-bearing URLs are persisted in
.git/configbygit cloneandgit remote add, and that workspace is then handed to the Claude action with Read/Bash access to untrusted OSS content. A prompt-injected file can read the URL and exfiltrate or useTIGERA_BOT_PATto modify Enterprise; strip credentials from both remotes immediately after fetching (the lateropen-prstep already pushes with its explicit URL).
git clone "$tgt_url" .
git remote add source "$src_url"
git fetch --no-tags source "$SOURCE_REF"
.github/workflows/scripts/oss-pick.sh:130
- This fallback hides a failed reporting contract: if Claude resolves a conflict but forgets or fails to write
/tmp/resolution-report.md, the resulting PR body says “No conflicts: the cherry-pick applied cleanly.” Fail closed forOUTCOME=conflictwhen the report is missing so reviewers are not misled about an AI-resolved change.
local conflicts="No conflicts: the cherry-pick applied cleanly."
if [ "${OUTCOME:-}" = "conflict" ] && [ -n "${RESOLUTION_REPORT:-}" ] && [ -f "$RESOLUTION_REPORT" ]; then
conflicts="$(cat "$RESOLUTION_REPORT")"
fi
.github/workflows/trigger-merge-queue.yml:1
- Removing this workflow removes the only visible dispatch of the merge-queue-bot's
OSS_MERGEevent. The replacement workflow only creates the OSS-master-to-Enterprise-master PR and contains no release-backport dispatch, so any release backports driven byOSS_MERGEwill stop when this deletion ships. Keep this trigger until that behavior is confirmed or add the replacement backport flow in the same change.
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write |
There was a problem hiding this comment.
Addressed. Reduced to pull-requests: read (dropped write). Everything github.token does here is a read: source clone, the gh api search/pulls lookups, and the Claude action's actor-permission check. Every target write (push, gh pr create, labels) uses TIGERA_BOT_PAT, not github.token. Verified end to end with the reduced scope: the Claude conflict-resolution step runs, Open PR creates the PR via the PAT, and the author DM is sent, all under pull-requests: read.
| # Idempotency: if the pick branch already exists on the target, stop. | ||
| if git ls-remote --exit-code --heads "$tgt_url" "$BRANCH_NAME" >/dev/null 2>&1; then | ||
| echo "Branch $BRANCH_NAME already exists on ${TARGET_REPO}; already picked." | ||
| emit "outcome=already"; return 0 |
There was a problem hiding this comment.
Addressed. The idempotency check now keys on the target PR, not the branch: it queries gh pr list --head --state all, and only returns "already" when a PR exists. A branch with no PR is treated as a stranded push from a prior run whose PR creation failed, so it is deleted and re-picked cleanly (self-heal) instead of being skipped forever.
| return gh([ | ||
| 'api', | ||
| `search/issues?q=sha:${HEAD_SHA}+repo:${SOURCE_REPO}+is:pr`, | ||
| '--jq', '.items[0].number // empty', |
There was a problem hiding this comment.
Addressed. resolve now fetches every PR the sha: search returns and selects the one whose head.sha equals HEAD_SHA (the PR that was actually merged), instead of trusting items[0]. If none match it skips with the candidate list. Unit-tested across single-match, multi-match with a decoy first, not-merged, no-head-match, and no-candidate cases.
9812ca1 to
9eecd90
Compare
|
Thanks for the review. Addressed the following findings (validated end to end in a test repo before updating here):
Two comments left as-is, on purpose:
Also, the earlier revision deleted |
9eecd90 to
0b04a3a
Compare
Adds a GitHub Actions workflow that, when a PR merges to master, cherry-picks the change onto tigera/calico-private master and opens a PR there. A generic script does the git work (clone, cherry-pick -x, build the bot-parseable PR body, carry labels, push, open PR); the Claude action is invoked only to resolve merge conflicts by editing files, keeping Enterprise-specific code. The original author is DMed on Slack when the pick PR is opened. The workflow is split into two stages for security: a zero-permission trigger on the untrusted PR event, and a privileged worker that runs from the trusted default branch via workflow_run. The cherry-pick script and the resolve/notify helpers are generic and reused by future release-backport workflows. Required secrets/vars on the repo: - CLAUDE_TOKEN: Anthropic API key for the Claude action. - TIGERA_BOT_PAT: token with write access to tigera/calico-private. - SLACK_MERGE_CAT_TOKEN + SLACK_NOTIFY_MAP: Slack DM to the author.
0b04a3a to
450dc68
Compare
Description
Adds an AI-assisted workflow that auto cherry-picks merged OSS
masterPRsinto Enterprise (
tigera/calico-privatemaster) and opens a PR there.How it works
.github/workflows/scripts/oss-pick.sh) does all git work:clone the target,
cherry-pick -xthe merged commit, build a bot-parseablePR body (with an
Original Commit SHAblock the merge-queue-bot can parse),carry over source labels, push the branch, open the PR.
conflicts by editing files while keeping Enterprise-specific code. Every
resulting PR is human-reviewed; conflict resolutions are written into the PR
body and the PR is labelled
auto-resolved-conflict.Security split (two stages)
auto-pick-oss-to-enterprise-trigger.yml: zero-permission trigger on theuntrusted
pull_request(closed) event. Holds no secrets.auto-pick-oss-to-enterprise.yml: privileged worker, runs from the trusteddefault branch via
workflow_run. Re-derives the merged PR from thetrigger's head SHA, so no attacker-controlled PR payload reaches the
privileged context.
The cherry-pick engine and the resolve/notify helpers are generic (source and
target are passed in), so future release-backport workflows (OSS master to
OSS release, Enterprise master to Enterprise release) can reuse them.
Removes
trigger-merge-queue.yml, which dispatched the merge-queue-bot tocreate the OSS-to-Enterprise pick on every master merge. This workflow now
does that directly.
Note for reviewers: backport gap
The merge-queue-bot's
OSS_MERGEdispatch thattrigger-merge-queue.ymlfiredmay also have driven
cherry-pick-candidaterelease backports on OSS merge.This PR replaces only the OSS master to Enterprise master pick. If the bot was
also creating release backports off that same trigger, those need a follow-up
backport workflow (reusing the generic script here) before this trigger is
removed in production. Please confirm the bot's OSS_MERGE behaviour before
merging.
Required secrets/vars (set on this repo before the workflow runs)
CLAUDE_TOKEN: Anthropic API key for the Claude action.TIGERA_BOT_PAT: token with write access totigera/calico-private.SLACK_MERGE_CAT_TOKEN+SLACK_NOTIFY_MAP: Slack DM to the author.Release Note