Repository navigation
docs(automatic-backports): propose automatic backports to release lines - #114
Conversation
Backport intent carried by labels stopped working: contributors cannot set labels, relative labels changed meaning when the next line opened, and most of the delay before a fix reaches a release is waiting for someone to decide on its backport. The proposal replaces per-PR decisions with written rules, a twice-weekly batcher and a check on the original PR that its title matches the change. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <[email protected]>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
…e the API review gate release-1.6 was released before the policy, so it only takes fixes merged after the policy starts. The api/ clause now follows the API review gate, because app values changes regenerate api/ types and would otherwise keep almost every app fix out of patches. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <[email protected]>
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
I checked this against the implementation in #5106, and the deterministic core is stronger than the prose suggests, which shifts where I think the risk actually sits. Credit first: the problem section is backed by real numbers (label share falling from 100% to 43%, the 9.8 day median, 16 fixes found a patch late), the rule engine in cmd/backport-batch/rules.go and apisurface.go is fully deterministic down to a CRD check that works document by document and fails closed, and the injection surface (git ref and argument injection, markdown injection into the PR body, trailer forgery) is covered in injection_test.go. That is a solid skeleton.
So this review is not about the skeleton. It is about the two LLM-based parts, which is what the open question asks, plus the batch testing model.
[MAJOR] Batch testing attributes failure to a single package, and the post-merge path is undefined
The proposal accepts testing batches instead of single backports, and handles a failure by dropping "commits that touch failing package" via TIA. That assumes a failure localizes to one package's commits. It does not hold when two backported changes interact, or when a change in package A breaks a test that exercises package B. The heuristic then drops the wrong commits, or thrashes.
The bigger gap: the real test of a release line is rc-e2e, and that runs after the batch merges, since rc is cut from the line. The edge-case list covers the pre-merge "batch CI fails" case, but not an rc-e2e failure on a batch that already merged, where bisecting N cherry-picks is a manual job. Please define the post-merge failure path.
[MAJOR] The whole risk is in the two LLM parts, and the rollout ships them with everything else
"Everything lands together," but accepting the LLM parts is deferred to the open question. If they are not accepted, the rollout plan does not hold as written.
LLM conflict resolution: fold checks which files a resolution touches, not whether it is correct. A contained but wrong resolution (wrong side of a conflict, a dropped hunk) lands on a release line behind a range-diff that a maintainer has to catch. You removed the human "should this be backported" decision and replaced it with a harder human "is this machine merge correct" decision, taken a batch at a time. That cuts against the stated goal of no per-PR human decision.
LLM title check: the deterministic Conventional Commits parse is fine. The LLM part is the semantic judgment, whether fix really fixes and whether the scope matches the touched paths, and it is advisory since maintainer review stays the final call. But every backport rule trusts the title as ground truth. An advisory, non-deterministic check that can be wrong feeds a rule engine that treats the title as fact, so one missed mislabel auto-backports a feat as a fix.
[MINOR] Prompt injection into the agent is not named
The deterministic path neutralizes mentions, keywords and links, and is injection-tested. An LLM agent that reads PR bodies and commit messages to resolve a conflict is exposed to instructions embedded in that untrusted text. Containment bounds the blast radius to the original's files, but it does not stop a contained-but-wrong resolution steered by injected text. Worth calling out in the Security section.
[MINOR] No treatment of LLM cost or availability
A semantic check on every PR to main, plus an agent twice a week, puts an external, paid, rate-limited, sometimes-down dependency in the release path. There is nothing on model, budget, timeout, or what happens when it is unavailable or returns garbage.
[MINOR / QUESTION] Rule 6 is stated as an absolute, but batches merge asynchronously
"Upgrade from older line to newer never loses a fix" is an absolute claim, yet batches land as separate per-line PRs. If the older line's batch merges before the newer line's, there is a window where the newer line lacks a fix the older one has. Please define "newer already has it" precisely (merged onto the line, versus present in an open batch PR), and say whether the invariant is steady-state or guaranteed at every instant.
[MINOR] Idempotency of "line already carries it" is fragile
Detection leans on -x trailers and a "Backport of #N" string, plus an empty-pick check. A change that reached the line by a path that dropped the trailer (a manual pick, a squash, a reword) gets a non-empty re-pick that conflicts or duplicates. The empty case is caught, the non-empty one is not.
[MINOR] A stalled batch PR silently breaks the stated cadence
The batcher opens or updates a PR on Monday and Thursday. If it is not merged, fixes pile up, and the only signal is backport-batch check at rc-cut time. Between rcs there is nothing that says batches are stalling.
[NIT]
- The
## Decisionssection is empty. - The single open question is the central design risk. Please split it: accepting an LLM for an advisory title check and accepting an LLM for code-producing conflict resolution are very different risk decisions.
On the open question directly, since you asked. The advisory title check is fine by me, as long as it stays strictly advisory, the deterministic Conventional Commits parse stays the gate, and a wrong or missing verdict degrades to "the maintainer decides" rather than to an auto-backport of a mislabeled PR. The conflict resolver is the one I would push back on: its output is code, its correctness oracle only checks file scope, and its backstop is a human reading range-diffs, which is the per-PR human judgment the proposal set out to remove, now in a heavier form. I would keep conflicts deferred to a human, which is what the deterministic batcher already does, and make agent auto-resolution an opt-in experiment gated behind an explicit human ack per conflict, not the default.
…rule 6 Spell out the human fallback for every LLM step and for batch and rc-e2e failures, keep conflict resolution with the agent by default since backport conflicts are already resolved by agents under supervision, pin rule 6 to merged state, and split the open question. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <[email protected]>
|
Thanks for checking it against #5106. I updated the proposal, answers below.
Decisions section stays empty by template until implementation. Open question is split in two now. |
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
Thanks, the update covers it, and I checked the new sections against the branch.
On conflict resolution I was wrong, and I want to say so plainly. I claimed LLM-resolved merges onto a release line were a new risk you were adding. They are not: release-1.6 already carries Assisted-by: LLM on eleven of the last forty commits, and the backport commit for #4671/#4672 even spells out the resolved hunk. So this is the supervised status quo, and my "defer conflicts to a human by default" line would reintroduce exactly the manual work the proposal removes. Withdrawn.
The rest reads as resolved to me:
- Rule 6 now pins "newer has it" to merged-on-line, with an open batch PR not counting. That was the ambiguity.
- Manual mode with the task table (fixed input, output, check in code, swappable executor) answers the cost and availability concern well, better than I expected.
- Post-merge rc-e2e now has a path: promote blocked, revert the one culprit original commit, cut a new rc. The one-commit-per-original shape makes that clean.
- Stalled batch alert and the idempotency note scoped to old history are both fine.
On the two split open questions, my vote: advisory title check, yes, as long as a wrong or missing verdict falls back to the maintainer and never to an auto-backport. Conflict resolution by agent, yes, since it is already how backports work here and the batch PR gets normal review.
One residual, not blocking: the conflict agent reads PR bodies and commit messages, and "merged to main" does not make that text non-adversarial, only reviewed. The real guard is that fold keeps the blast radius to the original's files, plus the batch PR review. Worth one line in Security naming that as the mitigation.
Moving to approve.
## What this PR does Backports become automatic. Fix merged to `main` with `fix` title, or `security` for fix that comes through security advisory, reaches release lines through batch that runs on Monday and Thursday, newest line first, nobody requests, labels or opens backports by hand. Batch runs for two newest lines with stable release (`released-lines` in policy block) and every line branched after them that has no stable release yet: with 1.6 and 1.7 released, `release-1.8` created at `v1.8.0-rc.1` joins them, and `v1.8.0` drops 1.6. Line counts as released once `vX.Y.Z` tag without pre-release suffix exists on origin. Rules are in `docs/release/backports.md`, `cmd/backport-batch` applies them, `.github/workflows/backport-batch.yaml` runs it. Design is in cozystack/community#114, merge shape of each original in cozystack/community#119. - `backport-batch run --line release-X.Y` takes PRs merged to `main` since line branched (title and body read from merge commit) and skips what rules exclude: other types and `deps`, breaking changes, platform migrations, `targetVersion`, changed CRD documents, API changes that API review gate calls sizeable (batch runs `internal/apigate` itself whenever PR changes a file the gate parses: `pkg/apiserver/apiserver.go`, an ApplicationDefinition, or YAML under `packages/` or `internal/` naming `CustomResourceDefinition` or `APIService`), and on older line what next newer line doesn't carry yet. The rest goes onto `backport-batch/<line>-<date>` in merge order. PR merged to `main` with merge commit goes as its own commits picked with `cherry-pick -x --signoff`, so each keeps its author, message and sign-offs and gets sign-off of whoever runs the batch (with `-m 1` author was whoever merged on `main`), merges of `main` into PR branch are left out, and merge commit joins them to the batch with original's merge message and `Backported-from:` trailer. Squash-merged PR goes as one pick. Conflict on any commit takes branch back to where it was before that original, it's listed with the commit that failed, run goes on. `--push` opens or updates draft PR, `--continue` carries on open batch PR as fast-forward only, PR body it didn't write is never rewritten. Same-day run over batch branch whose PR never got opened refuses and names `run --line <line> --onto <head> --push`, which continues it. Run or `--defer` that resets local batch branch over commits only it has prints `git branch` command that restores them. - `run --defer <N> --reason <why>` takes original out of open batch (usually the one failing CI is pinned on). It rebuilds branch replaying batch unit by unit without it, so merges and pushed conflict resolutions keep their shape, and only this rebuild pushes with `--force-with-lease`. Deferred original comes back in next batch. - Line carries something only if its own history shows it: picks whose `-x` line, the one git appends, names PR's merge commit or every commit of its own, or pick that changes nothing. Picked commits keep their messages and batch merge carries PR body, so text in them (`-x` lines, `Backported-from:`, `Backport of #N`) counts only on commits from before `batch-only` day in policy block, where it records old hand backports and batches. Otherwise text in a merged fix could mark another fix as already backported, and it would never reach the line. PR against line never counts. Open batch PR counts only from this repo and from author with write access or the batch's own bot (`BACKPORT_BATCH_BOT`, exact login, `cozystack-ci[bot]` when unset, so maintainer's run by hand continues scheduled batch instead of opening second one), and is read the same way. Actions bot isn't trusted anymore, any workflow token writes as it. - Any failed GitHub API or git call stops the run with message naming the call, no retries. Failed permission lookup used to count batch PR out as look-alike, so `--continue` opened second batch PR and `notify` posted second comment; now `run`, `check`, `verify` and `notify` exit 2 and write nothing. - Policy applies fully from `release-1.7`. `release-1.6` takes only fixes merged from 2026-10-26, older ones need `backport/include`. Start days are in `backport-policy` block on the policy page, test keeps page and code in sync. - `backport-batch check --cut` replaces `backport-audit` as gate before patch cut and before every rc after the first, it also fails while a fix is only on open batch PR. Plain `check`, which scheduled run uses, lets those through. `backport-batch fold` checks conflict resolution in the same shape: merge on batch head whose second parent is straight line of picks of PR's own commits in order, each with its `-x`, only PR's files (one pick for squash). `backport-batch verify` holds every original on batch to that shape, clean picks included, and `pr-labeler.yaml` runs it on every push to batch PR: `pull_request_target` so it reaches `release-*`, tool built from `main`, batch head fetched as data, read-only token. Batch PR body says per original how many commits it brought and which picks differ from their commits. `backport-batch notify` reports batch that is still open from previous run and deferred conflicts in one comment on batch PR, edited in place, with `BACKPORT_ALERT_TEAM` mention. - `hack/backport-batch-scheduled.sh` is what the workflow runs and maintainer can run by hand (`--dry-run` changes nothing on GitHub): for each line newest first `run --continue --push`, `check`, `notify`. It pushes with `BACKPORT_BATCH_TOKEN` only and fails without it, never `GITHUB_TOKEN`, because its pushes don't start CI. Workflow runs it as cozystack-ci App: it mints token for this repo only with Contents and Pull requests write, passes it as `BACKPORT_BATCH_TOKEN`, commits as `cozystack-ci[bot]`, and sets `BACKPORT_BATCH_BOT` so the tool trusts that bot's batch PR and comment. Job timeout is 30 minutes, under the token's hour. - App has no workflows permission, so workflow runs the script with `--no-workflow-files`: original that changes `.github/workflows/` isn't picked and waits on batch PR for maintainer, listed with its workflow files in body and in notify comment. It's never a conflict. `check` warns about it while it waits and `check --cut` fails on it. Maintainer brings it over with own token by `run --continue --push` without the flag, or takes it out with `--defer`. - Deferred conflicts and failed batch CI wait for maintainer. `notify` keeps one comment on batch PR listing stalled batch, conflicts and workflow changes, mentioning `BACKPORT_ALERT_TEAM` (team or login) when it's set. Failed batch CI isn't in the comment, it shows on batch PR checks, and `check --cut` fails while batch is unmerged. - Two steps that need judgement (resolve conflict, pin failed CI on original and defer it) are written as runbooks in `docs/release/backport-tasks/`: inputs, the only allowed output, check it must pass before push, what never to do. - `backport/include` and `backport/skip` let maintainer override rules for single PR, they are new `backport/` category in `labels.yml`. Skip wins over include, if open batch already picked it `--defer` takes it out. - Text written by other people (titles, release notes, reasons, file names) goes to batch PR body only inside code spans or release-note block, control, bidi and zero-width characters are dropped, file names in deferred block included, git args go after `--end-of-options`, no shell. - Removed: `cmd/backport-audit`, `backport.yaml`, `kind/backport` and `kind/backport-previous` labels with aliases and stale exemption, trailer-check exemption for bot branches. `Commit trailers` job exempts only `backport-batch/*` branches of this repo, changelog instructions read originals from batch commits. Exemption lives in main's `pre-commit.yml`, and batch PR runs its line's copy: `release-1.7` has its own copy that exempts only old `backport-<pr>-to-<branch>` branches, `release-1.6` has no such job. So on `release-1.7` picked commit with bad trailer fails batch CI until line gets main's copy. - `hack/close-manual-backport.sh` runs from `pr-labeler.yaml` on `opened` and closes PR against `release-*` from outside contributor (association contributor, first-time contributor or none, permission read or none). Owners, members, collaborators, anyone with triage or more and bots are left alone. When GitHub's answer on author's permission can't be read, it closes nothing and fails the job naming the failed read. Maintainer keeps release-only fix by reopening it. Release-only PRs are occasional and opened by maintainers, for fix with no counterpart on `main`, or one that on `main` came inside another PR or was written for `main`; if its counterpart on `main` is a fix the batch would take, the batch skips it once picking it changes nothing on the line, otherwise maintainer labels it `backport/skip`. - `docs/release.md` is split into pages under `docs/release/`, PR template doesn't ask for `kind/backport` anymore. `security` type is added to commit format in `docs/agents/contributing.md`, PR template, Gemini review guide and labeler's list (labeler maps no `kind/*` to it, `labels.yml` has no fitting one). - `SECURITY.md`, `docs/security/incident-response.md` and `GENERAL_TECHNICAL_REVIEW.md` now say two latest minor releases are supported, same window the batch runs for, and link the backport policy for which fixes go there. Dry runs without push, on 2026-11-06, with `--no-workflow-files` as the workflow runs it: `lines` gives `release-1.7` and `release-1.6`. On `release-1.7` batch picks 12 fixes, defers 3 conflicts (#4756, #4867, #5082), leaves #4888 and #4971 to a maintainer because they change workflows, and skips 49. On `release-1.6` it picks #5088, defers 3 conflicts (#5071, #5086, #5087), leaves #5142 to a maintainer, and skips 590. `verify` passes both batches. Setup after merge: no new secret. Batch runs as cozystack-ci App from existing `COZYSTACK_CI_APP_ID` and `COZYSTACK_CI_PRIVATE_KEY`, job fails when either is missing. App's installation already has Contents and Pull requests write on all org repos, and no Workflows is needed: PRs that change `.github/workflows/` wait for maintainer, who brings them over with own token as "Changes to workflows" in `docs/release/backports.md` says. Today that's #4888 and #4971 on `release-1.7` and #5142 on `release-1.6`. Set `BACKPORT_ALERT_TEAM` to a team or a login (for example `cozystack/maintainers`): unset, batch comment mentions nobody and nobody is notified. Once this PR is merged, `Backport batch shape` can be required check in the release lines ruleset (`release-*` excluding `release-*.*.*`), with "Do not require status checks on creation" on, because `cut-prerelease.yaml` creates `release-X.Y` by push. `PR Auto-Label` with both new jobs runs on `pull_request_target`, so it needs #5135 sorted before 2026-12-02. `kind/backport*` labels have to be deleted by hand, label sync doesn't delete. ### Screenshots Not a UI change. ### Downstream repositories cozystack/community `contributors/README.md` lists commit types, so it needs `security` too, it has no backport label text. Website has no backport label instructions, and its links to `docs/release.md` carry no anchor, the page stays as index. Trigger map lists commit convention change for cozystack/ccp: this adds `security` type, but not message format or trailer, and `package-bump` writes only `chore(...)` commits and restates the trailer, so nothing changes there. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: add `security` to commit types in `contributors/README.md` ### Release note ```release-note feat(release): backports are automatic. Fixes merged to main with fix or security title reach release lines through a batch built by cmd/backport-batch every Monday and Thursday, by the rules in docs/release/backports.md. The batch runs for the two newest lines with a stable release and for a line in its rc window: the release-X.Y branch the first rc creates joins at once, and its vX.Y.0 drops the oldest line. Features, chores, docs, dependency bumps, breaking changes, migrations, CRDs and API changes that the API review gate calls sizeable wait for the next minor. The policy applies fully from release-1.7, release-1.6 takes only fixes merged from 2026-10-26. Each original reaches the line as its own commits under a merge commit, keeping their authors and sign-offs. Maintainers override the rules for single PR with backport/include and backport/skip labels and take an original out of a batch with backport-batch run --defer. backport-batch check replaces backport-audit as the gate before a patch release and before a later rc. The label-driven backport workflow and kind/backport labels are removed. Backport PR opened by hand against a release branch by author with read access or less is closed automatically. The scheduled batch runs as the cozystack-ci App and needs no secret of its own; a fix that changes .github/workflows waits on the batch pull request for a maintainer, since the App may not push workflow changes. Release documentation now lives in one page per topic under docs/release/. ```
Proposal to make backports automatic: written rules instead of per-PR decisions, batcher that builds one PR per release line twice a week, and check on original PR that its title matches the code, since rules depend on it.
Implementation goes in cozystack/cozystack#5106 (policy page, docs layout, guard for hand-opened backports, batcher). First batch built this way is cozystack/cozystack#5089.
Main question for reviewers is in Open questions: agent part of batcher and check on original PR are LLM-based, agree?
Before review
decisions/directory, or says below why none is needed.New proposal, nothing merged is revised.
DCO
git commit --signoff).