Skip to content

Add a /rebase slash command for maintainers - #332

Merged
Zaldaryon merged 1 commit into
mainfrom
ci/rebase-command
Sep 16, 2026
Merged

Zaldaryon merged 1 commit into
mainfrom
ci/rebase-command

Conversation

@Pixnop

@Pixnop Pixnop commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

A maintainer comments /rebase on a pull request and CI does the git chores we keep asking contributors for: it replays the branch's commits onto the current tip of its base branch, drops merge commits, strips trailing whitespace from the lines the pull request added, and force-pushes the result back under a lease. Authors stay authors; the committer becomes github-actions[bot], and the whitespace cleanup lands as one separate bot commit so it is visible.

Targets main because issue_comment workflows only ever run from the default branch. CONTRIBUTING gets two paragraphs on the command.

What it refuses, with a comment saying why:

  • A merge commit that carries changes of its own (a conflict resolved by hand, a fix made while merging). Flattening would silently drop that work, so every merge in the range is compared with the mechanical merge of its parents first. This is exactly what happened on Fix storyloc structure spawning #305, where the merge of indev resolved a README conflict: the command would have said so instead of pushing a branch missing that line.
  • A conflict during the replay: nothing is pushed, the conflicting paths are listed.
  • A pull request whose head is indev or main in this repository (the release pull request would otherwise get flattened and force-pushed).
  • A branch that moved between the moment the pull request was read and the push (--force-with-lease pinned to the sha seen at the start).
  • A fork pull request without Allow edits by maintainers, or without the token below.

Whitespace rules mirror .editorconfig: only lines the pull request added, never a line it did not touch; *.md and the generated trees are skipped; in a .patch only the diff's own + lines are touched, never context or removed lines, which must stay byte identical to the vanilla baseline; CRLF endings are kept; renames are detected; files that contain a NUL byte are left alone.

Security shape, since this runs on a comment event with contents: write:

  • The commenter must have write, maintain or admin on this repository, checked through the collaborators permission API before anything else happens. Anyone else gets nothing at all (no bot comment, so the command cannot be turned into a way to post on other people's pull requests).
  • Nothing from the pull request is ever built or executed. The job checks out main for its own scripts only, and all git work happens in a throwaway repository created by the script, so the pull request's files never land in the workspace.
  • Every input reaches the shell through environment variables, never through expression interpolation in a run: block. The script prints one JSON line; the comment is composed by a python script that neutralises backticks and newlines in branch and file names.
  • The token goes through a credential helper that reads it from the environment: never in a URL, an argument or a config file, and set -x is deliberately absent. persist-credentials is off on the checkout.
  • The only push the job ever makes is to the pull request's own head ref.

Setup after merge, for an admin: add a repository secret PR_MAINTENANCE_TOKEN holding a classic personal access token with the repo scope, issued by a maintainer account with write access here. GitHub's own token cannot push to a fork even when the pull request allows maintainer edits, so without that secret the command only works for branches that live in this repository and says so on forks. The run ends red on any error so the maintainer who commented gets the failure email.

Testing: .github/scripts/test-rebase-pr.sh is an offline harness (file:// remotes, no token) with 122 checks over 18 cases: plain rebase, merge commit dropped with a byte-identical tree, no-op, empty branch, conflict, head moved, lease refused when the fork branch races the push, a merge that carries changes, each whitespace rule including the .patch and rename cases, a hostile fork shipping its own copy of the strip script, workspace integrity, and every comment text. Run it from the repository root with bash .github/scripts/test-rebase-pr.sh. The credential helper form was checked separately with git credential fill. The workflow itself cannot run before it is on main; a first live test can be a same-repo branch, which needs no secret.

@Zaldaryon Zaldaryon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the workflow security boundaries, token handling, merge commit verification, and offline test suite.

The security model is solid: the workflow restricts invocation to collaborators with admin, write, or maintain permissions, runs the rebase in an isolated throwaway repository, executes scripts exclusively from the main checkout rather than the PR branch, and passes tokens via a dynamic credential helper rather than embedding them in config or command lines. Refusing to drop merge commits that carry their own resolutions prevents silent code loss during flattening. Running test-rebase-pr.sh passes all 122 checks across all 18 test cases.

Targeting main makes sense here because GitHub Actions requires issue_comment triggers to live on the default branch. LGTM.

@Zaldaryon
Zaldaryon merged commit 522385a into main Sep 16, 2026
2 checks passed
@Zaldaryon
Zaldaryon deleted the ci/rebase-command branch September 16, 2026 23:04
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