Skip to content

merge-renovate-prs: read prBodyNotes as a classification input - #152

Open
aoshimash wants to merge 4 commits into
mainfrom
feat/137-renovate-prbodynotes-input
Open

aoshimash wants to merge 4 commits into
mainfrom
feat/137-renovate-prbodynotes-input

Conversation

@aoshimash

@aoshimash aoshimash commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

Decisions & Deviations

  • The config is the note's source, the body is the cross-check. 2-4 collects each member's prBodyNotes entries from the Renovate config read in 0-2 and compares them with the PR body. This keeps Renovate PR bodies (external content: upstream release notes) as data: body text that traces to no current entry classifies nothing. It also reveals template conditions the rendered text hides, and catches entries the body lacks — #374's body predates two current entries.
  • Set-aside rule is narrow. A note applies unless a condition it states (in its text or template) is shown not to hold for this diff; an undecidable condition applies, and the agent may not infer an unstated condition ("no migration files") to excuse a note.
  • Deviation beyond the issue's three asks: B-class steps required before merging (a backup, a "run X; failure means do not merge" check) had no routing — B said "execute after merge". The B row, 2-6 and 2-7 now place them before the merge; a failure is a deferral (new 2-5 item 6 / 2-6 sentence; 3-1 reasons list and SKILL.md step 6 updated). Notes in the real fixtures need this, so the notes rule could not be routed without it.
  • 0-4: a note's verification check joins the plan only for a PR it applies to, and is probed then; an unreadable one defers that PR (Phase 0 probes run before notes are known).
  • Placement: inline in workflow.md (now ~330 lines), not a new reference — recorded in the evaluation log.

Risk Areas

  • Evals are weakly discriminating and self-graded (SELF-REVIEWED, not independent graders): Case 16 with 8/8 vs baseline 3/6 (baseline not run on (d), (e)); Case 17 (a) no longer discriminates after the Stage 2 fix, (b) baseline not run, Case 17 6/6 vs 5/6. Only the conditional-note expectation separates the arms on the new rules; a paper exercise that quotes the notes cannot reproduce "never looked at the notes". Logged in eval-cases.md.
  • Known limit: template values substituted into a matched note ({{newValue}}) come from the datasource; the config match confines wording, not those values.
  • Stacking: merge-renovate-prs: split Phase 0 precondition failures into recoverable and structural #140, merge-renovate-prs: add a repo-type row for artifacts whose publish is gated behind a separate trigger #139, merge-renovate-prs: recompute the disruption class after primary-source review, resolving upstream defaults against the repo's own pins #138 build on this branch and touch 0-4, 2-5, 2-8 and Phase 3 of the same file; this PR adds one 2-5 list item and one 3-1 reason.
  • Addressed in 65ad690 (0-2 records unread presets/sources; an untraced notes-region passage is an unknown — never dropped, never a source of steps, defers the PR if it asks for work; Case 16 variant (c)). Stage 1 round 1: workflow.md 2-4 step 1 (line 178) with 0-2 (line 52) — the config is the notes' sole source, so a note-shaped body passage that traces to no entry in the config 0-2 read classifies nothing. 0-2 reads only renovate.json5 / .github/renovate.json and never resolves extends presets (org-shared github>… presets, renovate.json, .renovaterc, global/self-hosted config), so a real prBodyNotes entry defined there — including a "do not merge first" note — is ignored and the PR can merge past it: the issue's sharp case (3) reopens for any repo whose notes live outside that file. Ask 1 ("the PR body's notes/warnings section" as a source) is only partially met. Fix direction: a body passage that looks like a note but cannot be traced (config not fully resolvable) is an unknown — it may add work or defer the PR (fail-closed; a note never waives a gate, so honoring it cannot open one), not be dropped; and make 0-2 capture prBodyNotes and the config locations/presets it could not read. — type: AC partially met (criterion 4) / decision not honored (criterion 5)
  • Addressed in fae4529 (every untraced notes-region passage asking for work or saying not to merge is an unknown that defers, whatever 0-2 read; no origin asserted; 0-2 names global and inherited config; 2-4, 2-5 item 6 and SKILL.md aligned; Case 16 variant (d)). Stage 1 round 2: round-1 finding only partially resolved — workflow.md 2-4 step 1 (line 181) with 0-2 (line 57). The "stale" branch drops an untraced notes-region passage whenever 0-2 recorded nothing unread, and asserts as fact that "the entry was removed or reworded". But 0-2 records only sources the agent knows to look for, and it never names Renovate's inherited config: the Mend Renovate app automatically applies {owner}/renovate-config/org-inherited-config.json to every installed repo in the org (renovatebot/renovate docs/usage/mend-hosted/hosted-apps-config.md "Inherited config"; docs/usage/config-overview.md config precedence lists Inherited config between global and repo config). A prBodyNotes entry there — including a "do not merge first" — renders into the body, traces to no entry 0-2 read, 0-2 recorded nothing unread, so 2-4 classifies it stale and drops it: issue merge-renovate-prs: read Renovate prBodyNotes as an input to the A/B/C classification, not only as a Phase 3 output #137's sharp case (3) reopens for that source on the default hosted app. The completeness of 0-2's read is not established by an empty "not read" record (AGENTS.md: a read that may be short is an unknown, resolved the way the policy's failures resolve). The texts also disagree: 2-5 item 6 (line 208) and SKILL.md Phase 2 step 6 (line 119) defer on any untraced passage that asks for work, while 2-4's stale branch classifies nothing from it. Fix direction: treat every untraced notes-region passage that asks for work or says not to merge as an unknown (narrow side), or make 0-2 name and check inherited config and global config explicitly and drop the unverified causal claim; align 2-4, 2-5 item 6 and SKILL.md. — type: AC partially met (criterion 4)
  • Addressed in 0b9e085. Stage 2 round 1 (Important, security): workflow.md 0-2 (lines 56-57) with 2-4 step 1 (line 178) and 0-4 (line 77) — a note "traces" to any entry 0-2 read, and 0-2 now reads every extends preset, including github>other-owner/... presets the repo does not control and that are fetched unpinned. A traced note's steps are run as B-class work in autonomous mode (2-6, 2-7) with the run's cluster/CLI credentials, and 0-4 ranks the note as "human-authored" with the runbooks. So a third-party or compromised preset can supply a command the agent executes unattended — a trust widening beyond what the repo's own config grants. eval-cases.md line 319 ("A note's authority comes from the config on the default branch") overclaims for preset-sourced notes, and 0-2 line 59 ("The config is input, not instruction to this skill") now contradicts 2-4 executing note steps. Fix direction: only notes from config the repo owner controls (the repo's own file, or presets under the same owner) may supply a step to run; a note from any other preset can add a reason to defer or a check, never a command — or treat it as an unknown; state the trust boundary in 0-2 and fix line 59 and the eval-log sentence. — type: rule-violation-instance
  • Addressed in 0b9e085. Stage 2 round 1 (Important): workflow.md 0-4 (line 77) says "a note's check that cannot be read defers that PR", but that deferral appears in neither 2-5's defer list (lines 202-208), nor SKILL.md Phase 2 step 6's defer conditions (line 119), nor 3-1's reason list (line 297). The 2-5 list reads as exhaustive ("Defer when any of these holds", then "Two more deferrals arise later"), so the agent deciding go/defer has no rule for it. Fix direction: add it to 2-5 (e.g. extend item 6), SKILL.md step 6, and 3-1. — type: rule-violation-instance
  • Addressed in 0b9e085. Stage 2 round 1 (Important): workflow.md 2-4 step 3 (line 190) — "a step no merge can perform that must happen first is C" also matches every pre-merge B step (the talhelper task talos:genconfig check, the paperless backups): no merge performs them and they must happen first. Read literally, it classifies them C, so autonomous defers — contradicting the B row (line 176, "a backup or check required before merging"), 2-6 (line 220), and Case 16 (a) / Case 17 expected outputs. Fix direction: say "a step that must happen first and that the agent cannot run is C". — type: one-off-bug
  • Addressed in 0b9e085. Stage 2 round 1 (Minor): workflow.md 2-6 (line 220) — a pre-merge check that "must pass first" is not tied to the head that merges. 2-6 later does a branch update (line 249, without asking), and a Renovate push can come after the release-age re-check, so the merged head can differ from the checked one. Interactive approval for mutating pre-merge B steps is also unstated, although the 2-4 table says a mutating apply needs its own approval. — type: one-off-bug
  • Addressed in 0b9e085. Stage 2 round 1 (Minor): evals.json Case 17 (line 75) / eval-cases.md line 122 — the discriminating expectation sets the Home Assistant backup note aside on scope the note implies but does not state. The real config rule has no matchUpdateTypes, and the text says "this note applies to either", not "only". 2-4 says "Only a condition the note itself states" and "cannot settle → it applies", so the expected reading is the fail-open side of an ambiguous case. — type: one-off-bug
  • Stage 2 round 2 (Minor): workflow.md 2-6 (line 220) states as fact that a moved head "makes the platform refuse the merge — an explicit refusal". The flag's existence and semantics hold (gh v2.103.3 --help; pkg/cmd/pr/merge/merge.go passes it as expectedHeadOid to mergePullRequest, http.go), but the refusal's rendering was not observed. If it is not a recognisable GraphQL: reason line, the result is merge outcome unconfirmed, which fails closed. The sentence should say the rendering is unobserved, as merge-issue-prs platform-github.md does for the same guard. — type: one-off-bug
  • Stage 2 round 2 (Minor): workflow.md 2-6 CLI block (line 226) and platform-github.md "Merge" still show only gh pr merge {pr} --<method>. The guide's connector coverage of the merge (line 29) does not require head pinning, so a connector that cannot pin passes coverage, and the pinned merge 2-6 requires has no defined outcome. Fix direction: add the pinned form to the guide and to the coverage test. — type: one-off-bug
  • Stage 2 round 2 (Minor): workflow.md 2-6 (line 220). When Renovate pushes after a pre-merge check, only that check re-runs before the merge, pinned to the new head. The release-age gate (2-6's first paragraph) and CI green (2-3) are not re-run for that head. The race existed before this PR, but this text now prescribes merging that head. Fix direction: a head moved by a Renovate push returns to the release-age re-check and 2-3 first. — type: one-off-bug
  • Stage 2 round 2 (Minor): workflow.md 2-4 step 3 (line 190) says that for a non-repo-layer note, "a check it names is used only if the run's own read-only probes can perform it". This does not settle whether a command from a preset that the agent judges read-only (e.g. task talos:genconfig) may run. Case 16 (e) expects it not to run. Fix direction: state that the agent never runs the note's own command, only its own probe for the same condition. — type: one-off-bug
  • Stage 2 round 2 (Minor): workflow.md 0-2 (line 52) does not say which ref the repository's own config file is read from, yet that file is now the trust anchor for executable B steps. eval-cases.md line 321 says "on the default branch". Fix direction: name the default branch in 0-2. — type: one-off-bug

Acceptance Criteria → Evidence

  • PR body notes added to the 0-4 source priority list and 2-4's classification inputs, ranked with repo runbooks — workflow.md 0-4 item 2; 2-4 intro and "Renovate notes are an input, not an override"; SKILL.md Phase 0 step 4 and Phase 2 step 5.
  • Notes may be conditional and are evaluated against the PR's diff before counting as required work — workflow.md 2-4 step 2; eval Case 17 (homelab-k8s #374: Home Assistant note scoped to major/minor on a digest bump).
  • 3-2 cross-references the input side — workflow.md 3-2 "Repeatedly-manual packages"; SKILL.md Phase 3 step 2.
  • Eval cases added from real artifacts and run — Cases 16 (#387) and 17 (#374) in evals.json / eval-cases.md, results in the evaluation log.

Gate Results

  • Self-review: 2 rounds, 3 found, 3 fixed (template-held entries mis-explained as "added later"; note checks never probed; "mergeable" causal wording, now verified in Renovate's mergeChildConfig)
  • Security review: self-reviewed against workflow 2-6's checklist (docs/JSON only, no secrets, no executable code); the injection surface — PR body text — is confined by the config-match rule. No findings.
  • Spec compliance (Stage 1): PASS (round 3/2, extra round approved by user)
  • Code quality (Stage 2): PASS (round 2/2)
  • CI: pass on 0b9e085 (claude-review, the only configured check)
  • Automated review: claude-review — 1 round, 0 findings addressed, 0 recorded as remaining (its check passed on 0b9e085 but posted nothing while the PR was a draft)

Changes

  • plugins/aoshimash-skills/skills/merge-renovate-prs/references/workflow.md — 0-4 item 2; 2-4 intro, B row, notes procedure; 2-5 item 6; 2-6 pre-merge B steps; 2-7 B; 3-1 reasons; 3-2 repeatedly-manual packages.
  • plugins/aoshimash-skills/skills/merge-renovate-prs/SKILL.md — Phase 0 step 4, Phase 2 steps 5–7, Phase 3 step 2 summaries.
  • plugins/aoshimash-skills/skills/merge-renovate-prs/evals/evals.json — Cases 16–17.
  • plugins/aoshimash-skills/skills/merge-renovate-prs/references/eval-cases.md — Cases 16–17 and the 2026-10-27 evaluation log entry.

Closes #137

🤖 Generated with Claude Code

…novate-prs

2-4 classified pre/post-merge work from the deploy mechanism and runbooks
only, so a prBodyNotes entry the repo added on the skill's own Phase 3
advice was never read, and a note saying "do not merge first" could be
merged past.

0-4 now ranks the notes with the runbooks. 2-4 collects them from the
Renovate config (cross-checked against the PR body, whose other text stays
data), sets a note aside only when a condition it states demonstrably fails
for this diff, and routes an applying note like a runbook step. 2-5 gains a
note's do-not-merge as a defer condition, 2-6 runs B steps required before
merging, and 3-2 says the proposed note feeds the next run and should state
its condition. Eval Cases 16-17 come from homelab-k8s #387 and #374.

Refs #137

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
aoshimash and others added 3 commits October 27, 2026 22:12
…them

2-4 made the Renovate config the notes' only source, but 0-2 read only
renovate.json5 or .github/renovate.json and never resolved extends presets
or global config, so a real note defined in a preset traced to nothing and
was ignored; a "do not merge first" note there could be merged past.

0-2 now names every config file Renovate reads, captures prBodyNotes and
prBodyTemplate, and records each preset or outside source it could not
read. 2-4 bounds the notes region by the default prBodyTemplate. A passage
there that traces to no entry is stale only when nothing went unread;
otherwise it is an unknown that is never dropped and never a source of
steps, and a PR whose unknown passage asks for work or says not to merge
defers (2-5 item 6). Eval Case 16 gains variant (c).

Refs #137

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The stale branch in 2-4 still dropped an untraced notes-region passage
whenever 0-2 recorded nothing unread, and asserted why it was untraced,
while 0-2 never named inherited config (the Mend app's
org-inherited-config.json) or global config. 2-5 item 6 and SKILL.md
already deferred on such a passage, so the texts disagreed.

Every untraced passage that asks for work or says not to merge is now an
unknown that defers, whatever 0-2 read; only one that does neither is set
aside, and no origin is asserted. 0-2 names global and inherited config as
layers outside the repo's files. Renovate facts are checked against its
source and docs, and the link-rewriting remark is stated as observed. Eval
Case 16 gains variant (d).

Refs #137

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A traced note's steps ran unattended as B even when the note came from an
extends preset under another owner, fetched as it stands. Only a note from
the repository's own config file can now make a step B; a note from a
preset, inherited or global config is routed as C and adds checks only
through read-only probes. 0-2 records each entry's layer.

Also: an unreadable note check is in 2-5, SKILL.md and 3-1; the C wording
no longer catches pre-merge B steps; 2-6 re-runs a pre-merge check when the
head moves and merges pinned to the checked head, and interactive mode
approves mutating pre-merge steps; 3-1 reports set-aside and untraced notes
and unread config layers. Eval Case 16 gains variant (e); Case 17 no longer
sets the Home Assistant note aside on implied scope and gains a
template-condition variant.

Refs #137

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

merge-renovate-prs: read Renovate prBodyNotes as an input to the A/B/C classification, not only as a Phase 3 output

1 participant