Skip to content

github: Add "Make it mine" to take over a forge PR's text - #17

Merged
cgwalters-bot merged 2 commits into
mainfrom
bot/make-it-mine
Sep 29, 2026
Merged

cgwalters-bot merged 2 commits into
mainfrom
bot/make-it-mine

Conversation

@cgwalters-bot

@cgwalters-bot cgwalters-bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Adds "Make it mine" to the PR pane, for the approved fork PRs whose upstreams are human-text (coreos/rpm-ostree, coreos/bootupd, coreos/cargo-vendor-filterer, ostreedev/ostree, osbuild/*). You edit the title, description and commit messages, starting from the bot's text as it is, and confirm a diff of every change. Then the app, with your token:

  1. writes a new commit per old one through the Git Data API: same tree, parents and author, you as committer; each new commit is checked against what was asked;
  2. moves the branch with GraphQL updateRefs, beforeOid = the head you were shown (a compare-and-swap; REST PATCH git/refs has none);
  3. re-reads the body, then sets the title and description, keeping the bot-meta section byte for byte;
  4. optionally waits for the PR's head_ref_force_pushed event, then comments /promote --human-text.

Your text, your review.

  • Nothing saves while any field still has a Generated-by line, or without your "This text is mine" tick.
  • When promoting, nothing saves with the bot's title either; promote would refuse it anyway.
  • The /promote --human-text comment approves the new head. So it is ticked by default only if you approved the head it replaces; the trees are the same, so that approval carries over.
  • Otherwise the confirmation shows Approve's unseen-files and hotspots warning.
  • The committer starts as Colin Walters <walters@verbum.org>, the identity promote signs off with.

Does it pass promote's human-text gate? Yes, no promote change needed. The fork's activity log records the updateRefs call as a force_push by the token's user, so human_text_problems in bin/bot-pr sees you as the pusher; the PATCH records you as the renamer and the last body editor. One catch, found by the e2e run: promote counts a /promote only if it's strictly later, to the second, than the push, and GitHub adds the PR's force-push event about 3 seconds after the ref moves. That's why step 4 waits for the event.

Scope and safety.

  • Only the bot's open PRs from bot/ branches inside a cgwalters-forge fork (the repository must have a parent), with a bot-meta section.
  • At most 50 plain (non-merge) commits.
  • Trees never change.
  • It re-reads the PR first and stops if the head, title or body moved.
  • The confirmation step comes before any write.

A classic token needs no new scope (public_repo), except workflow for a PR that changes .github/workflows. The form checks that up front, so it never fails after writing commits. A fine-grained token needs Contents: write, plus Workflows: write for such PRs; its permissions aren't reported, so the form only warns. Code edits are a TODO.

Tests.

  • npm run check: tsc, 832 unit tests (50 new: commit rewriting against an in-memory fake GitHub, the form and confirmation), build. Passed at 19b8545 on a 4-core devspace (cgwalters-devspace-36525325806), and at both commits of the first version.
  • E2E: test/e2e/make-it-mine.ts, with the bot's token standing in for yours, against scratch PRs scratch: make-it-mine e2e 2026-09-29T05:09:08.000Z cargo-vendor-filterer#4 and, at 19b8545, Review forge PRs in one ranked queue with the board's questions #5 (both closed, branches deleted).
    • With bot-pr's REVIEWER set to cgwalters-bot, bot-pr promote --dry-run refused a bare /promote --human-text before the rewrite ("body was last edited by no one", "title was not last set", "still has the bot's Generated-by line").
    • After the rewrite it accepted: Policy: human-text, text by cgwalters-bot (/promote --human-text), no refusal.
    • The stock bot-pr (REVIEWER=cgwalters) still reports it not approved.
    • The e2e ran locally, not on the devspace, because it needs a token. It imports only the app's dependency-free modules, with no npm install.
    • The bot was also the PR author and branch creator there. So the first run with your token is the real test of whose name GitHub records; if it gets that wrong, promote refuses, which is safe.

Needs your approval before merge: this is the app's first git write with your token.

Generated-by: AI

@cgwalters-bot cgwalters-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Review guide for head 2ee4459115: where to look closely and what is safe to skim. It is advice from the bot's reviewer and doesn't replace reading the diff; the review app walks it.

Adds Make it mine: with his token the app rewrites every commit of a bot forge PR (same tree, parents, author; him as committer), moves the branch with GraphQL updateRefs using beforeOid as a compare-and-swap, sets title and body keeping bot-meta, waits for the force-push event and comments /promote --human-text. Write scope is tight: only the PR's own repo and head branch, re-read and checked before any write, each new commit's tree/parents/author/message verified, no merges. The risk is policy, not mechanics. The /promote --human-text comment (checkbox default on) is itself an approval of the new head, but this path skips the Approve confirmation's unseen-files and hotspot warning and does not require a prior approval. And the prefill is the bot's text with Generated-by already stripped, so the only edit actually needed to pass promote is a title tweak; body and commit messages can go upstream as bot text verbatim. The CAS is modelled in a fake; a real updateRefs race was never exercised, and the e2e used the bot as both PR author and pusher, so it cannot tell token-user attribution from author attribution (failure there is safe: promote refuses).

Hotspots

  1. risky · security — src/github/mineview.ts:113-124 (2ee4459): The promote checkbox defaults on and its /promote --human-text comment approves the new head, yet this path never shows unseenNote (files never expanded, hotspots unseen) nor requires that he already approved the head. Gate it on d.verdict.state === approved, or show the approve guard here.
  2. look-closely · security — src/github/mineview.ts:85-95 (2ee4459): Prefill strips Generated-by from body and commits, so accepting it unchanged passes bot text off as his with one title edit. Consider prefilling verbatim and refusing any Generated-by line in any field, plus an explicit adopt-as-mine acknowledgement.
  3. look-closely · logic — src/github/mine.ts:177-187 (2ee4459): editProblems checks only the exact LLMS trailer in the body, and nothing in commit messages; with promote on and the title unchanged it only warns, though promote will certainly refuse. Make an unchanged title a hard error when promoting.
  4. note · security — src/github/mine.ts:233-263 (2ee4459): Tree, parents, author and message are verified per new commit before the ref moves; checked, this holds.
  5. look-closely · test-gap — src/github/mine.ts:265-297 (2ee4459): updateRefs with beforeOid and force:true is the documented CAS, but only the fake tests a lost race; nothing live checks that GitHub rejects a stale beforeOid.
  6. note · logic — src/github/mine.ts:370-382 (2ee4459): The PATCH uses the bot-meta from the earlier read, so a bot edit between the fresh check and here is overwritten. A whitespace-only body normalisation also PATCHes while the confirmation says the description is unchanged.
  7. note · security — src/github/mine.ts:89-102 (2ee4459): Scope check: owner, author, head repo equals base repo, bot/ prefix, bot-meta upstream. It does not check the repo is actually a fork (d.parent), which the docs claim.
  8. note · test-gap — test/e2e/make-it-mine.ts:61-83 (2ee4459): The bot is PR author, branch creator and token user, so these checks pass whether GitHub attributes to the token user or the author. The first real run with his token is the real test; failure there is safe.

Safe to skim

  • static/style.css: styles only
  • src/github/forge.ts: exports two constants
  • README.md: docs

@cgwalters-bot
cgwalters-bot enabled auto-merge (rebase) September 29, 2026 15:20
@cgwalters-bot cgwalters-bot moved this to Todo in Workstream Sep 29, 2026
Prep for rewriting a forge PR's commits and body as cgwalters', which
must keep the bot-meta section intact and only touch branches in the
PR's own repository.

Generated-by: AI
Seven approved fork PRs are stuck because their upstreams' policy is
human-text: the title, body and commit messages must be cgwalters' own,
and `bot-pr promote` checks GitHub's record of who pushed the approved
head, who edited the body last and who set the title. Rewording and
force-pushing by hand is the slow part, so the PR pane now does it
with his token.

He edits the title, description and each commit message and confirms
a diff of every change. The app then writes one new commit per old one
through the Git Data API (same tree, parents and author, him as
committer, each checked against what was asked), moves the branch with
GraphQL's updateRefs and beforeOid set to the head he saw (REST's
PATCH git/refs has no compare-and-swap), re-reads the body and sets
the title and body with the bot-meta section kept byte for byte, and
comments `/promote --human-text`.

It must not launder the bot's text or skip his review. The fields
start as the bot's, Generated-by lines included, and nothing saves
while any field still has one, without his "This text is mine", or
(when promoting) with the bot's title, which promote refuses anyway.
The /promote --human-text comment approves the new head, so it is
ticked by default only when he approved the head it replaces (same
trees); otherwise the confirmation carries the Approve button's
unseen-files and hotspots warning. The committer defaults to the
identity promote signs off with.

Moving the ref first means a lost race leaves nothing half-changed.
The comment waits for the PR's head_ref_force_pushed event: GitHub
adds it a few seconds after the ref moves, and promote counts a
/promote only if it is strictly later than that, to the second. An
e2e run hit exactly that before the wait existed.

Checked against a scratch PR with the bot's token standing in for
his: the fork's activity log records the updateRefs call as a
force_push by the token's user, and `bot-pr promote --dry-run` with
REVIEWER set to that login accepts the human-text approval (and
refuses it before the rewrite). test/e2e/make-it-mine.ts is that run.
The bot is also the PR's author and branch creator there, so the
first run with his token is the real attribution test; failing it is
safe, since promote then refuses.

This is the one place the app writes git with his token, so it is
limited to the bot's PRs from bot/ branches inside cgwalters-forge
forks, at most 50 plain commits, and never changes a tree. A classic
token needs no new scope, except `workflow` for a PR that changes
.github/workflows, which the form checks up front; a fine-grained one
needs Contents: write. Code edits are left as a TODO.

Generated-by: AI
@cgwalters-bot
cgwalters-bot merged commit c20770c into main Sep 29, 2026
1 check passed
@cgwalters-bot
cgwalters-bot deleted the bot/make-it-mine branch September 29, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants