fix(ai): let the fix step own config.yaml, and require the patch-counter bump (#2973)

* fix(ai): let the fix step own config.yaml, and require the patch-counter bump

The premise that the fix step cannot touch config.yaml turned out to be wrong,
and the real problem was the opposite of what it looked like.

config.yaml was already in scope — issue-fix.md lists it among the files the
sweep reads and owns, and all three merged ai-fix PRs edited it. What they
edited, though, was the one thing hard limit 2 forbade outright:

  PR #2970  qbittorrent  version: "5.2.3.2" -> "5.2.3.3"
  PR #2912  bazarr       version: "1.6.0.1" -> "1.6.0.2"

Both bumped only the LOCAL PATCH COUNTER, leaving the upstream X.Y.Z alone —
i.e. exactly the right thing, in direct violation of the written rule. Nothing
enforces that rule (ai_guard_paths.sh only covers .github/ and .templates/), so
it has been quietly contradicted by practice, and it also contradicts CLAUDE.md's
own PR requirement to bump version.

It matters because Supervisor will not offer a rebuild without the bump: a fix
merged without one ships inert while the issue looks closed. That is the worst
outcome available — worse than not fixing it.

So the carve-out is narrowed to what addons_updater actually owns (the
`upstream` field and the upstream X.Y.Z), and bumping the trailing .N is now
required rather than forbidden, with the dot-not-hyphen trap called out
(X.Y.Z-N reads as a semver pre-release and Supervisor treats it as older).
Exotic version shapes — LSIO tags, dates, nightlies — are explicitly left alone
rather than guessed at.

Applied to all four places the rule is stated so they cannot drift:
issue-fix.md, issue-execute-plan.md, CLAUDE.md, and pr-coderabbit.md — the last
keeps the restriction, since it amends a PR whose single bump already covers it,
but now says why instead of reading as a contradiction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(ai): derive the patch counter from updater.json, not from version's shape

Six review findings, all reproduced against the repo before accepting.

Codex (P1) — the rule "increment the trailing .N" is wrong for most of this
repo, because you cannot tell a local counter from an upstream component by
looking at `version`. Checked all 134 add-ons:

  version == upstream_version (no counter, must APPEND .1):  82
  version == upstream_version + .N (counter, INCREMENT):      8
  version drifted from upstream (LEAVE ALONE):               36
  no usable updater.json (LEAVE ALONE):                       8

So the previous wording would have mutated updater-owned data on 82 add-ons:
sonarr's 4.0.19.3001 IS the upstream version, and incrementing it to
4.0.19.3002 burns the identifier of a future real release; linkwarden's 2.16.0
would have become 2.16.1, indistinguishable from an upstream minor bump.

updater.json's upstream_version is now the authority: append .1 when version
equals it, increment only the digits that follow it, otherwise leave version
alone. Validated by running the rule as written over every add-on — 0
violations of the invariant that a bumped version must still start with
upstream_version.

Copilot — there is no `upstream:` key in any config.yaml (0 of 134); upstream
tracking lives in updater.json as upstream_repo / upstream_version. That was
inherited text naming a field that does not exist, in all four places. Replaced
with the real constraint: never edit updater.json.

Copilot — the "a workflow step enforces them" headers over-claimed. Only limit
1 is machine-enforced (ai_guard_paths.sh); the rest ship silently if broken,
which is worth saying plainly given limit 2 has been quietly contradicted by
practice for months.

Copilot — Outcome B produces a plan and no PR, so "say so in the pull request
body" had no place to land. Now covers both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Alexandre
2026-08-15 19:14:29 +02:00
committed by GitHub
parent bcb7299a7d
commit 948a2722f3
4 changed files with 71 additions and 9 deletions

View File

@@ -11,10 +11,21 @@ Read:
diff, verification, risk). This is your spec.
- `/tmp/ai-exec/issue.json` — the issue it fixes (for `Closes #<n>` and context).
## Hard limits (identical to the fix sweep — a workflow step enforces them)
## Hard limits (identical to the fix sweep; only limit 1 is machine-enforced)
1. **Never modify `.github/` or `.templates/`.** Repo-wide infrastructure.
2. **Never touch the `version` or `upstream` fields in `config.yaml`.**
2. **`config.yaml` is yours to edit, except the upstream part of `version`;
never edit `updater.json`** — `addons_updater` owns both. (There is no
`upstream:` key in `config.yaml`.) You must still
bump the local patch counter, or Supervisor never offers the rebuild and the
fix ships inert. Read `updater.json` to find the boundary — you cannot tell
it from `version` alone, since upstream versions here run to four or five
components. With `U` = `upstream_version`: if `version` equals `U`, **append**
`.1` (sonarr `4.0.19.3001` -> `4.0.19.3001.1`); if it is `U` + `.` + digits,
**increment** those digits (radarr `6.3.0.10514.1` -> `6.3.0.10514.2`);
anything else — no `updater.json`, drifted version, LSIO tag, date, nightly —
leave `version` alone and say so in the pull request body. A dot, never a
hyphen.
3. **One add-on, one branch:** `ai-fix/<addon>-<issue-number>`.
4. **Never merge, never close the issue, never enable auto-merge.** Open the
pull request **ready for review** — CI (`onpr_check-pr.yaml`) validates it,

View File

@@ -13,14 +13,52 @@ about confidence matters more than the number of pull requests you open.
## Hard limits
These are not guidelines. A workflow step enforces them after you finish, and
anything that violates them gets blocked and flagged.
These are not guidelines. Limit 1 is machine-enforced — a workflow step checks
every pull request you open and blocks and flags anything that violates it. The
rest are on you: nothing checks them, so breaking one ships silently.
1. **Never modify `.github/` or `.templates/`.** Those are inherited by every
add-on in the repo. A change there is a 100-add-on incident, not a fix.
2. **Never touch the `version` or `upstream` fields in `config.yaml`.** The
`addons_updater` job owns those. Editing them causes merge conflicts you
will not be around to resolve.
2. **`config.yaml` is yours to edit, with one carve-out.** Never change the
*upstream part* of `version` — the portion that tracks the upstream release
— and never edit `updater.json` at all. The `addons_updater` job owns both,
and editing them causes merge conflicts you will not be around to resolve.
(There is no `upstream:` key in `config.yaml`; upstream tracking lives in
`updater.json` as `upstream_repo` / `upstream_version`.)
The **local patch counter** is a different thing and you must bump it. When
you change any file in an add-on, `version` must change too — otherwise
Supervisor never offers the rebuild, the add-on keeps running the old image,
and your fix ships inert: merged, doing nothing, with the issue looking
closed. That is worse than not fixing it at all.
**You cannot tell the counter from `version` alone — read `updater.json`.**
Upstream versions in this repo have anywhere from one to five components, so
a trailing `.1234` is just as likely to belong to upstream as to be a local
counter. `updater.json`'s `upstream_version` is the authority. Let `U` be
that value, and compare:
| `version` vs `U` | what to do | example |
|---|---|---|
| identical | **append** `.1` | sonarr `4.0.19.3001` -> `4.0.19.3001.1` |
| `U` + `.` + digits | **increment** those digits | radarr `6.3.0.10514.1` -> `6.3.0.10514.2` |
| anything else | **leave it alone** | plex, readarr, joal |
Getting this backwards corrupts data you do not own: sonarr's `4.0.19.3001`
*is* the upstream version, so "increment the last component" would produce
`4.0.19.3002` and burn the identifier of a future real release. 82 of the
add-ons in this repo are in that first row — appending is the common case,
incrementing the rare one.
Use a dot, never a hyphen: `X.Y.Z-N` parses as a semver pre-release, which
Supervisor treats as *older* than `X.Y.Z` and will not offer.
The third row is not a failure — it is the safe answer whenever the add-on
has no `updater.json`, its `version` has drifted from `upstream_version`, or
the format is exotic (LSIO tag `1.43.1.10611-1e34174b1-ls301`, a date, a
nightly). Do not guess a counter onto those. Leave `version` untouched and
say so — in the pull request body, or in the plan if this issue is going out
as Outcome B — so a human can decide.
3. **One add-on per branch, one branch per pull request.** Branch name
`ai-fix/<addon>-<issue-number>`.
4. **Never merge, never close an issue, never enable auto-merge.** Opening a

View File

@@ -23,7 +23,10 @@ Consider only comments authored by `coderabbitai[bot]`. Ignore its collapsed
## Hard limits (a workflow step enforces the first)
1. **Never modify `.github/` or `.templates/`.** Repo-wide infrastructure.
2. **Never touch the `version` or `upstream` fields in `config.yaml`.**
2. **Leave `version` in `config.yaml` alone, and never edit `updater.json`.** The rest of
`config.yaml` is fair game. Unlike the fix sweep, you are amending a pull
request that has *already* bumped the local patch counter — one bump covers
the whole PR, so incrementing it again here would just churn the diff.
3. **Stay within this PR's scope and branch.** Do not open a new PR, do not
touch other add-ons, do not merge, do not mark ready/draft.

View File

@@ -161,7 +161,17 @@ the sweep), `ai:plan-pending` (plan posted, awaiting `ai:approved`), `ai:fixed`,
out of the automated tiers but not the manual ones. **Kill switch:** set the
repo variable `AI_DISABLED=true` to pause every AI workflow with no file edits.
AI fixes must never touch `.github/` or `.templates/` (enforced by
`ai_guard_paths.sh`) or the `version`/`upstream` fields in `config.yaml`.
`ai_guard_paths.sh`). They may edit `config.yaml` freely except the upstream
part of `version`, and must never edit `updater.json``addons_updater` owns
both. (There is no `upstream:` key in `config.yaml`; upstream tracking lives in
`updater.json`.) They must still bump the local patch counter so Supervisor
offers the rebuild —
without it the fix ships inert. The counter boundary comes from
`updater.json`'s `upstream_version`, never from the shape of `version`: append
`.1` when the two are equal (the common case — upstream versions here run to
four or five components), increment the trailing digits only when `version` is
`upstream_version` + `.N`, and otherwise leave `version` alone. This rule is
prompt-only, not machine-enforced.
## Linting Rules