Skip to content

fix: rewrite manifest commit in-place instead of blindly amending HEAD#23

Draft
Copilot wants to merge 2 commits into
mainfrom
copilot/update-action-yml-commit-handling
Draft

fix: rewrite manifest commit in-place instead of blindly amending HEAD#23
Copilot wants to merge 2 commits into
mainfrom
copilot/update-action-yml-commit-handling

Conversation

Copy link
Copy Markdown

Copilot AI commented May 18, 2026

git commit --amend --no-edit in the synchronize and merged flows always rewrites HEAD, silently destroying any user commits stacked on top of the manifest commit.

Changes

Durable marker on manifest commit creation

  • Adds X-Manifest-PR: true trailer to the initial commit so it can be located reliably across rebases without depending on commit position.
git commit -m "manifest: Update … revision (auto-manifest PR)" \
  -m "$BODY" -m "X-Manifest-PR: true" --signoff

Targeted rewrite in synchronize and merged steps

  • Locate the manifest commit via git log --grep='X-Manifest-PR: true'; falls back to --grep='auto-manifest PR' for branches predating the marker.
  • Snapshot user commits stacked above it: git rev-list --reverse "${MANIFEST_COMMIT}..HEAD".
  • Reset to the manifest commit's parent, then recreate it:
    • synchronize: cherry-pick + --amend to update committer timestamp (new SHA → CI retrigger) without touching content.
    • merged: restore west.yml from the manifest commit, update revision to merge_commit_sha, commit with the original message preserved via git commit -F.
  • Cherry-pick user commits back on top with per-commit error handling and a clear abort message on failure.
  • Existing upstream-conflict detection and rebase -X theirs fallback are preserved unchanged.
Original prompt

Update action.yml in nrfconnect/action-manifest-pr so the workflow no longer blindly amends the top commit on the auto-manifest branch. The current implementation uses git commit --amend --no-edit in the synchronize and merged flows, which rewrites HEAD even when users have added commits on top of the original manifest commit. Implement logic that instead identifies the original manifest commit in the auto-manifest PR branch, rewrites that commit with the updated west.yml, and reapplies any later commits on top of it.

Context:

  • Repository: nrfconnect/action-manifest-pr
  • File: action.yml
  • Relevant area is around action.yml#L177
  • The branch being updated is auto-manifest-${{ github.event.repository.name }}-${{ github.event.pull_request.number }}
  • This should apply to the paths used for:
    • Retrigger CI by changing commit sha and pushing
    • Change commit sha and push it

Requirements:

  1. Replace the current top-commit amend behavior with logic that targets the original manifest commit rather than HEAD.
  2. Preserve any commits that appear after the manifest commit by replaying/rebasing/cherry-picking them on top of the rewritten manifest commit.
  3. Make the manifest commit discoverable in a stable machine-readable way. Prefer adding a unique trailer or other durable marker when the manifest PR commit is originally created, then use that marker to find the commit later.
  4. Update the commit creation step for the initial manifest PR so the durable marker is added when the manifest commit is created.
  5. Ensure the workflow still rebases/updates cleanly against ${{ inputs.base-branch }} and pushes the updated branch correctly.
  6. Preserve existing behavior as much as possible aside from fixing which commit gets rewritten.
  7. If needed, refactor shell logic in action.yml to keep the workflow understandable and maintainable.

Please create a pull request with the implementation and include a concise explanation in the PR description of how the commit-rewrite logic works and why it is needed.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: sometimes a user has another commit on top of the manifest change. can you amend the first commit in the PR instead of the top commit? and rebase the other other commits on top of it? or should I tell users to put the manifest change on the bottom.

Assistant: Yes — you should amend the manifest commit, not blindly amend HEAD.

Right now this step:

git commit --amend --no-edit

will always rewrite the top commit on the branch. If users add another commit after the manifest update, your workflow mutates the wrong commit.

Best approach:

  • find the original manifest PR commit,
  • rewrite that commit,
  • then replay any later commits on top of it.

I would not rely on telling users to “keep the manifest change on the bottom” unless you want a workflow convention instead of robustness. Automation is safer if it handles stacked commits correctly.

Recommended approach

In the synchronize and merged-path flows, replace the simple amend with a targeted history rewrite, for example using an interactive rebase scripted via GIT_SEQUENCE_EDITOR / git rebase --onto, or by:

  1. locating the manifest commit by message or another stable marker,
  2. resetting to its parent,
  3. recreating the manifest commit with the updated west.yml,
  4. cherry-picking the commits that were originally above it.

Pseudo-flow:

BRANCH=auto-manifest-${{ github.event.repository.name }}-${{ github.event.pull_request.number }}
BASE=upstream/${{ inputs.base-branch }}

git checkout "$BRANCH"
git remote add upstream https://github.com/${{ inputs.target-repo }} || true
git fetch upstream

MANIFEST_COMMIT=$(git log --format='%H %s' "$BASE..HEAD" \
  | grep 'manifest: Update .* revision (auto-manifest PR)' \
  | tail -n1 | cut -d' ' -f1)

PARENT=$(git rev-parse "${MANIFEST_COMMIT}^")
AFTER_COMMITS=$(git rev-list --reverse "${MANIFEST_COMMIT}..HEAD")

git reset --hard "$PARENT"

# regenerate west.yml here
git add west.yml
git commit -m "manifest: Update ${{ github.event.repository.name }} revision (auto-manifest PR)" --no-edit --signoff

for c in $AFTER_COMMITS; do
  git cherry-pick "$c"
done

Then rebase onto the latest base branch if needed and push with --force-with-lease.

Important caveat

Matching only by commit subject is a little fragile. Better options:

  • make the manifest commit message unique and machine-detectable,
  • add a stable trailer, e.g. X-Manifest-PR: true,
  • or always create the branch so the first commit is known and tracked.

Example commit message:

git commit -m "manifest: Update ${{ github.event.repository.name }} revision (auto-manifest PR)" \
  -m "$BODY" \
  -m "X-Manifest-PR: true" \
  --signoff

Then locate it with:

git log --format='%H%x09%B' "$BASE..HEAD" | awk '...'

or a simpler grep over bodies.

My recommendation

Do bo...

This pull request was created from Copilot chat.

- Add X-Manifest-PR: true trailer to the initial manifest commit so it
  can be located reliably on later workflow runs.
- Synchronize step: find the manifest commit, collect any user commits
  stacked above it, reset to its parent, re-apply it (amend committer
  date for a new SHA to retrigger CI), then cherry-pick user commits.
- Merged step: same discovery, then update west.yml only in the manifest
  commit (preserving its message), cherry-pick user commits back on top,
  rebase onto upstream if conflicts exist, and push.
- Backwards-compatible fallback searches by 'auto-manifest PR' subject
  for branches created before the marker was introduced.
- Add per-cherry-pick error handling with clear failure messages.

Agent-Logs-Url: https://github.com/nrfconnect/action-manifest-pr/sessions/2011b57a-adad-461d-87a3-46e574e14293

Co-authored-by: thst-nordic <14976801+thst-nordic@users.noreply.github.com>
Copilot AI changed the title [WIP] Update action.yml to fix commit handling in auto-manifest branch fix: rewrite manifest commit in-place instead of blindly amending HEAD May 18, 2026
Copilot AI requested a review from thst-nordic May 18, 2026 11:15
@thst-nordic thst-nordic requested a review from karhama May 18, 2026 11:26
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.

2 participants