Skip to content

Prevent consolidation agent from touching release number - #720

Merged
TomasKorbar merged 1 commit into
packit:mainfrom
TomasKorbar:consolidation_release_field_fix
Jul 27, 2026
Merged

TomasKorbar merged 1 commit into
packit:mainfrom
TomasKorbar:consolidation_release_field_fix

Conversation

@TomasKorbar

Copy link
Copy Markdown
Collaborator

No description provided.

@qodo-for-packit

Copy link
Copy Markdown

PR Summary by Qodo

Harden consolidation prompts to never modify RPM spec Release

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Strengthen consolidation-agent prompt constraints to forbid any RPM spec Release edits.
• Remove guidance that could cause Release resets when both source MRs bump it.
• Clarify Release is handled only by a later deterministic workflow step.
Diagram

graph TD
A["Consolidation agent"] --> B["mr_consolidation prompts"] --> C[("RPM spec file")]
C --> E["Deterministic release step"]
B -. "Release must remain unchanged" .-> C
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Post-processing guardrail (reject Release diffs)
  • ➕ Deterministic enforcement even if the model ignores prompt constraints
  • ➕ Provides clear failure signal and prevents accidental merges
  • ➖ Requires adding/maintaining a diff check in the consolidation workflow
  • ➖ May need edge-case handling for spec formatting/noise diffs
2. Spec-file patch filtering (strip Release hunks)
  • ➕ Automatically removes Release modifications without failing the job
  • ➕ Keeps consolidation output usable even with occasional model drift
  • ➖ Risk of masking broader unintended spec changes
  • ➖ More complex implementation; must avoid corrupting spec semantics
3. Add a regression test for prompts (golden diff)
  • ➕ Ensures future prompt edits don’t reintroduce Release-handling instructions
  • ➕ Fast feedback in CI
  • ➖ Tests can be brittle if prompt wording changes frequently
  • ➖ Doesn’t enforce behavior at runtime by itself

Recommendation: The prompt hardening in this PR is the right minimal fix to stop the consolidation agent from touching Release and to remove conflicting instructions (e.g., reverting to base). Consider adding a lightweight workflow guardrail that fails the consolidation job if the spec Release line changes, for deterministic protection beyond prompt adherence.

Files changed (3) +10 / -7

Other (3) +10 / -7
adapt_patch.j2Make Release prohibition explicit in patch adaptation prompt +3/-2

Make Release prohibition explicit in patch adaptation prompt

• Strengthens the critical constraint to NEVER modify the spec Release field. Adds explicit language forbidding reverting, bumping, or otherwise changing Release.

ymir/agents/prompts/mr_consolidation/adapt_patch.j2

adapt_spec.j2Make Release prohibition explicit in spec adaptation prompt +3/-2

Make Release prohibition explicit in spec adaptation prompt

• Updates the spec adaptation constraints to categorically forbid any Release field modifications. Clarifies Release is handled by a separate deterministic workflow step.

ymir/agents/prompts/mr_consolidation/adapt_spec.j2

instructions.j2Remove Release-reset guidance; mandate leaving Release untouched +4/-3

Remove Release-reset guidance; mandate leaving Release untouched

• Replaces prior conditional guidance (including reverting Release when both MRs bump it) with an unconditional rule to never modify Release. Explicitly instructs to leave Release exactly as-is regardless of source MR content.

ymir/agents/prompts/mr_consolidation/instructions.j2

@qodo-for-packit

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 7 rules

Grey Divider


Informational

1. Release bump double-count 🐞 Bug ≡ Correctness
Description
In merged-release consolidation, the workflow deterministically runs update_release after the
agent finishes; that tool increments based on the current spec.raw_release, so carrying over a
Release bump from a source MR will result in an unintended extra increment (e.g., base Release 3 →
source MR 4 → deterministic step produces 5). The updated prompt now explicitly discourages
resetting Release back to the base value even when both source MRs contain bumps, increasing the
likelihood of this compounded bump in merged strategy.
Code

ymir/agents/prompts/mr_consolidation/instructions.j2[R75-78]

+- NEVER modify the Release field in the spec file under ANY circumstances. The Release
+  field will be updated by a separate deterministic workflow step. Do not revert it,
+  bump it, reset it to the base branch value, or change it in any way — even if you
+  notice that both source MRs contain Release bumps. Leave it exactly as it is.
Relevance

⭐ Low

Similar “prevent second Release bump/update_release guard” suggestion was explicitly rejected in MR
consolidation context.

PR-#689

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The prompt explicitly forbids resetting Release even when both source MRs bump it, but the merged
consolidation workflow later runs update_release (non-per_commit) which increments from the
current spec.raw_release; therefore any Release bump that lands in the working tree before that
step will be compounded.

ymir/agents/prompts/mr_consolidation/instructions.j2[72-78]
ymir/agents/mr_consolidation_agent.py[807-810]
ymir/agents/mr_consolidation_agent.py[1316-1332]
ymir/tools/unprivileged/specfile.py[259-292]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The consolidation prompts now prohibit resetting/spec-normalizing the spec `Release` field “even if both source MRs contain Release bumps”. In the merged release strategy, the workflow later runs a deterministic `update_release` step that increments the *current* `Release`, so if the agent brings in a source bump (e.g. by cherry-picking), the deterministic step can increment again and produce an unintended EVR.

### Issue Context
`mr_consolidation_agent.py` transitions to an `update_release` step for non-`per_commit` strategies, and `UpdateReleaseTool` computes the new `Release` from `spec.raw_release` (incrementing the parsed numeric portion). The prompt should therefore guide the agent to ensure the *final* consolidated tree has `Release` unchanged from the base branch value prior to the deterministic bump, without requiring the agent to invent/choose a new release number.

### Fix Focus Areas
- ymir/agents/prompts/mr_consolidation/instructions.j2[72-78]
- ymir/agents/prompts/mr_consolidation/adapt_patch.j2[32-38]
- ymir/agents/prompts/mr_consolidation/adapt_spec.j2[36-42]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@TomasKorbar
TomasKorbar force-pushed the consolidation_release_field_fix branch from ec8f787 to dc1213f Compare July 27, 2026 15:54
@TomasKorbar
TomasKorbar merged commit 57579be into packit:main Jul 27, 2026
11 checks passed
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.

4 participants