Skip to content

Allow modifying (Build)Requires on build errors under strict conditions - #739

Merged
nforro merged 1 commit into
packit:mainfrom
nforro:requires
Aug 6, 2026
Merged

nforro merged 1 commit into
packit:mainfrom
nforro:requires

Conversation

@nforro

@nforro nforro commented Aug 5, 2026

Copy link
Copy Markdown
Member

No description provided.

@nforro
nforro requested a review from opohorel August 5, 2026 10:31
@qodo-for-packit

Copy link
Copy Markdown

PR Summary by Qodo

Gate spec BuildRequires/Requires edits behind maintainer rules in build-fix prompt

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Add maintainer-rule gate to allow limited spec dependency edits during backport build fixes.
• Define strict criteria for when BuildRequires/Requires additions are permitted and when they’re
 forbidden.
• Update workflow and success criteria to validate any spec changes against the new rules.
Diagram

graph TD
  A([Build failed]) --> B["Fix-build-error prompt"] --> C["get_maintainer_rules"] --> D{Spec deps allowed?}
  D -->|"No"| E["Fix patches only"] --> F([Report result])
  D -->|"Yes"| G["Add BR/R only if patch-introduced"] --> E
  subgraph Legend
    direction LR
    _start(["Start/End"]) ~~~ _proc["Step"] ~~~ _dec{"Decision"}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Enforce policy in code (tooling/validator) instead of prompt text
  • ➕ Harder for the agent to accidentally violate constraints
  • ➕ Enables automated checks (e.g., “only BR/R lines changed”) before accepting output
  • ➖ Requires implementation effort outside the prompt layer
  • ➖ May need repo/package-specific integration to read and diff spec files reliably
2. Split into two prompt variants (spec-edit-allowed vs forbidden)
  • ➕ Reduces conditional complexity inside a single prompt
  • ➕ Makes expected behavior clearer and shorter for each scenario
  • ➖ Adds prompt selection logic elsewhere
  • ➖ Risk of drifting rules between variants over time

Recommendation: The PR’s approach (explicitly gating spec dependency edits on maintainer rules with tight “allowed vs forbidden” criteria) is a practical improvement at low implementation cost and reduces risky spec churn. Longer term, consider adding a lightweight automated validator to enforce “only BR/R additions” mechanically, but the current prompt-level guardrails are a reasonable incremental step.

Files changed (1) +79 / -21

Other (1) +79 / -21
prompt_fix_build_error.j2Gate spec dependency edits on maintainer rules with strict allow/deny criteria +79/-21

Gate spec dependency edits on maintainer rules with strict allow/deny criteria

• Extends the backport build-fix prompt to require a 'get_maintainer_rules' check before any spec edits. When explicitly allowed, limits spec changes to BuildRequires/Requires additions only if directly introduced by the backported patch and present in the build error; otherwise, forbids all spec modifications and instructs reporting environment-related failures instead of working around them.

ymir/agents/prompts/backport/prompt_fix_build_error.j2

@qodo-for-packit

qodo-for-packit Bot commented Aug 5, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Spec rule contradicts self-review ✓ Resolved 🐞 Bug ≡ Correctness
Description
The prompt now allows modifying BuildRequires/Requires in the spec (when maintainer rules allow it),
but Self-Review Criterion 3 still requires that only regenerated patch files are modified. This
contradiction will cause agents to fail self-review (and potentially revert correct spec additions)
whenever the new allowed spec-change path is used.
Code

ymir/agents/prompts/backport/prompt_fix_build_error.j2[R52-55]

+- DO NOT modify anything in {{ local_clone }} dist-git repository except:
+  a. The backport patch file(s) you created (by regenerating them from upstream repo)
+  b. BuildRequires/Requires in the spec file (ONLY if maintainer rules allow it, per above)
+
Relevance

●●● Strong

Prior accepted changes add/align self-review guardrails; fixing instruction/self-review
contradictions likely accepted.

PR-#598

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The prompt explicitly permits spec BuildRequires/Requires changes (conditional), but Criterion 3
still asserts that only patch files may be modified, which directly conflicts with the new
allowance.

ymir/agents/prompts/backport/prompt_fix_build_error.j2[52-55]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[142-145]

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 prompt allows spec `BuildRequires/Requires` changes under maintainer-rule conditions, but Self-Review Criterion 3 still says no files other than regenerated patch files may be modified. This makes the prompt internally inconsistent and will incorrectly fail self-review when spec changes are legitimately allowed.

### Issue Context
The PR introduces an allowed spec-change path (conditional on maintainer rules) but does not update Criterion 3 to include the spec-file exception.

### Fix Focus Areas
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[142-147]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[52-55]

### Suggested change
Update Criterion 3 to explicitly allow spec-file `BuildRequires/Requires` additions *only when* maintainer rules allow and Criterion 2’s sub-criteria are satisfied (otherwise keep the “no other files” restriction).

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



Remediation recommended

2. Spec review ignores commits 🐞 Bug ☼ Reliability ⭐ New
Description
Self-Review Criterion 2 relies on git diff HEAD -- *.spec and the suggested remediation uses `git
checkout HEAD -- *.spec`, which only detect/revert uncommitted changes. If a spec edit is committed
during the attempt, the prompt can incorrectly conclude there were “no spec changes” and the
suggested revert will not remove the committed modification.
Code

ymir/agents/prompts/backport/prompt_fix_build_error.j2[R129-131]

+8. Self-Review: Before reporting a result, verify your work meets all criteria
   below. Run `git diff HEAD -- *.spec` in {{ local_clone }} to inspect what
   changed in the spec file.
Relevance

●●● Strong

Team has accepted tightening prompt guardrails/self-review steps; this is a small reliability
improvement to spec-change detection.

PR-#598
PR-#652

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The prompt tells the agent to validate spec changes using a diff against the current HEAD and to
revert spec changes by checking out the file from HEAD; both approaches only work for uncommitted
changes and will miss/retain spec edits once they are committed.

ymir/agents/prompts/backport/prompt_fix_build_error.j2[129-131]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[138-156]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[223-226]

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 prompt’s spec validation/revert guidance assumes spec edits remain uncommitted. If the agent commits spec changes (intentionally or by habit), `git diff HEAD -- *.spec` becomes empty and `git checkout HEAD -- *.spec` cannot undo the committed change, undermining the new “allowed only under strict conditions” spec-edit workflow.

### Issue Context
This prompt now explicitly allows adding BuildRequires/Requires under strict conditions, increasing the chance that a spec edit exists and must be validated/reverted correctly.

### Fix Focus Areas
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[129-131]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[138-146]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[223-226]

### Suggested change
Update the self-review/remediation instructions to be robust whether changes are committed or not. For example:
1) Add an explicit rule: do **not** commit dist-git changes during the fix attempt; keep spec edits uncommitted until the build passes.
2) Or, capture a baseline commit at the start (e.g., `BASE=$(git rev-parse HEAD)`) and later check/revert against it (e.g., `git diff $BASE..HEAD -- *.spec` plus working-tree diffs; and revert via `git restore --source=$BASE -- *.spec` / `git reset` as appropriate).

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


3. Rules fetch not fail-closed ✓ Resolved 🐞 Bug ☼ Reliability
Description
The new workflow gates spec edits on get_maintainer_rules, but it doesn’t explicitly say what to
do when rules cannot be fetched (e.g., tool returns a non-404 failure message). Without an explicit
fail-closed rule, agents may behave inconsistently about whether spec edits are allowed when the
rules lookup fails.
Code

ymir/agents/prompts/backport/prompt_fix_build_error.j2[R73-76]

+0. Use the `get_maintainer_rules` tool with package {{ package }} to check whether
+   the maintainer explicitly allows adding new BuildRequires/Requires entries during
+   backport build fixes (see CRITICAL CONSTRAINTS above).
+
Relevance

●●● Strong

Team often accepts prompt guardrails and explicit error-handling; fail-closed rule improves
deterministic behavior.

PR-#598
PR-#540
PR-#706

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The prompt newly relies on a maintainer-rules lookup to authorize spec edits, but it only discusses
the cases where rules exist vs. do not exist; it doesn’t define behavior for lookup failures. The
tool implementation shows it can return a “Failed to fetch…” message for non-404 HTTP responses,
which should be treated explicitly as “not allowed” in the prompt to ensure consistent behavior.

ymir/agents/prompts/backport/prompt_fix_build_error.j2[73-76]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[50-56]
ymir/tools/privileged/maintainer_rules.py[66-76]

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 prompt instructs agents to consult `get_maintainer_rules` and only modify spec files if maintainer rules explicitly allow it, but it does not state how to handle cases where rules retrieval fails (HTTP error/timeout/etc.). The prompt should explicitly instruct a fail-closed behavior: treat any non-explicit/failed lookup as “not allowed”, avoid spec edits, and proceed with patch-only fixes.

## Issue Context
`get_maintainer_rules` can return a textual failure message when it cannot fetch rules, which is neither explicit permission nor “no rules exist” unless the prompt defines it that way.

## Fix Focus Areas
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[73-76]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[50-56]

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


4. Executable deps misclassified ✓ Resolved 🐞 Bug ≡ Correctness
Description
WORKFLOW step 2 treats missing executables in build logs as a generic “missing dependency” and
allows adding either BuildRequires or Requires, but the surrounding rules only clearly define
BuildRequires for compile/link artifacts and Requires for runtime usage. This leaves
build-time/test-time missing executables under-specified and can cause agents to add Requires (which
typically won’t be present in the buildroot during %check), leading to another failed build attempt.
Code

ymir/agents/prompts/backport/prompt_fix_build_error.j2[R74-78]

+2. If the build error indicates missing dependencies (headers, libraries, executables) that are
+   DIRECTLY INTRODUCED by your backported patch AND maintainer rules (from step 0) explicitly
+   allow it:
+   - Add the minimal required new BuildRequires or Requires entries to the spec file
+   - Additions only — do NOT modify or remove existing entries
Relevance

●●● Strong

Team often accepts prompt wording/guardrail clarifications to prevent agent mistakes; similar
backport prompt edits merged.

PR-#598
PR-#655

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Step 2 explicitly includes missing “executables” and suggests adding BuildRequires or Requires, but
the defining/validation text only gives concrete guidance for BuildRequires via compile/link
failures and for Requires via runtime behavior, leaving build-time executables ambiguous.

ymir/agents/prompts/backport/prompt_fix_build_error.j2[74-80]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[25-33]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[149-157]

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 prompt now mentions missing **executables** as a class of missing dependencies, but it doesn’t explicitly state how to classify them:
- executables needed during build/%check should be **BuildRequires**
- executables needed only at runtime by the installed package should be **Requires**

Without this, agents may add `Requires` in response to a build-log “command not found”, which usually won’t make that command available during the build in the isolated buildroot.

## Issue Context
Current text:
- Step 2 includes “executables” in missing dependencies and permits adding either BuildRequires or Requires.
- Earlier rules define BuildRequires additions primarily via missing headers/libraries and Requires via runtime behavior, leaving build/test-time executables ambiguous.

## Fix Focus Areas
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[25-33]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[74-80]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[149-157]

## Suggested change
Add an explicit rule for missing executables:
- If the build error occurs during build/%check and an executable is missing due to the backported patch, add **BuildRequires** for the package providing that executable.
- If the executable is only needed by the installed package at runtime, add **Requires** (and optionally also BuildRequires if tests invoke it during %check).

Update Criterion 2c examples to include missing executables as acceptable BuildRequires evidence (e.g., “command not found” in build logs), aligned with the rule above.

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


View more (5)
5. Approval wording inconsistent ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The prompt asks whether maintainer rules allow generic “spec file modifications”, but elsewhere it
only permits spec edits when rules explicitly allow adding new BuildRequires/Requires. This
inconsistent terminology can lead to incorrect interpretation of maintainer guidance and
inconsistent spec-edit decisions.
Code

ymir/agents/prompts/backport/prompt_fix_build_error.j2[R68-70]

+0. Use the `get_maintainer_rules` tool with package {{ package }} to check whether
+   the maintainer allows spec file modifications during backport build fixes.
+   This determines whether you can add BuildRequires/Requires (see CRITICAL CONSTRAINTS above).
Relevance

●●● Strong

Wording consistency in backport prompts has been accepted before; this is a low-risk clarity nit.

PR-#655
PR-#598

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
CRITICAL CONSTRAINTS narrowly permit only new BuildRequires/Requires additions when explicitly
allowed, but the workflow step frames the check as generic spec modification approval, and Criterion
2 mixes both phrasings.

ymir/agents/prompts/backport/prompt_fix_build_error.j2[25-33]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[68-70]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[122-132]

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 prompt uses broader phrasing (“spec file modifications”) when the intended/allowed behavior is narrower (“adding new BuildRequires/Requires entries only”). This terminology mismatch can cause the agent to misclassify maintainer guidance.

## Issue Context
The CRITICAL CONSTRAINTS and self-review criteria are built around an allow/deny decision specifically for adding new BuildRequires/Requires, but the workflow step text frames the question as generic spec modification approval.

## Fix Focus Areas
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[25-33]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[68-71]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[122-132]

## Suggested change
Reword the workflow step (and any related references) to: “check whether the maintainer explicitly allows adding new BuildRequires/Requires entries during backport build fixes”, and ensure Criterion 2 uses the same phrasing consistently.

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


6. No deps-add workflow step ✓ Resolved 🐞 Bug ☼ Reliability
Description
The prompt now permits adding BuildRequires/Requires under strict conditions, but the numbered
WORKFLOW never explicitly instructs when to make those spec additions before rebuilding. This can
cause the agent to focus only on patch regeneration/rebuild loops and miss an allowed dependency fix
path.
Code

ymir/agents/prompts/backport/prompt_fix_build_error.j2[R53-56]

+- DO NOT modify anything in {{ local_clone }} dist-git repository except:
+  a. The backport patch file(s) you created (by regenerating them from upstream repo)
+  b. Adding new BuildRequires/Requires entries to the spec file (ONLY if maintainer
+     rules allow it, per above; do NOT modify or remove existing entries)
Relevance

●●● Strong

Team often accepts prompt clarity/guardrail additions; missing explicit deps-add step risks agent
behavior drift.

PR-#598
PR-#655

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The prompt explicitly allows adding new BuildRequires/Requires entries in dist-git, but the WORKFLOW
steps do not include a step to perform those additions prior to rebuilding; they focus on patch
fixes/regeneration and build/test loops.

ymir/agents/prompts/backport/prompt_fix_build_error.j2[53-56]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[66-99]

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 prompt introduces an allowed action (conditionally add new BuildRequires/Requires) but does not add an operational workflow step telling the agent to apply those additions after diagnosing missing deps and before rebuilding.

## Issue Context
Currently, the workflow steps cover analysis, upstream fixes, patch regeneration, and rebuilding, while spec dependency additions are only described in constraints/self-review. Adding a dedicated workflow step makes the new capability actionable and reduces unnecessary repeat failures.

## Fix Focus Areas
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[53-56]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[66-99]

## Suggested change
Insert a workflow step (e.g., between steps 1 and 3, or between 3 and 4):
- If the build error indicates a missing dependency directly introduced by the backported patch AND maintainer rules explicitly allow it, add the minimal new BuildRequires/Requires line(s) in the spec (additions only), then proceed to regenerate patches/build.
Also reference Criterion 2 for validation requirements.

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


7. Spec self-review misses edits ✓ Resolved 🐞 Bug ≡ Correctness
Description
Self-Review Criterion 2 checks that spec changes are in BuildRequires/Requires sections and that
added deps are justified, but it does not explicitly fail when existing dependency lines are edited
or removed. This can allow forbidden spec edits/removals to pass self-review despite the earlier
constraint that existing BuildRequires/Requires must not be modified or removed.
Code

ymir/agents/prompts/backport/prompt_fix_build_error.j2[R132-135]

+     If there ARE spec changes, verify ALL of the following:
+     a. Changes are ONLY to BuildRequires or Requires (not %changelog, Release,
+        existing Patch tags, %prep, or any other section)
+     b. Each added dependency is justified:
Relevance

●●● Strong

Team has accepted tightening backport prompt self-review guardrails; this closes a spec-dep
edit/removal loophole.

PR-#598

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The prompt forbids modifying/removing existing BuildRequires/Requires, but Criterion 2 only
constrains the *section* touched and validates *added* dependencies, leaving deletions/edits of
existing dependency lines insufficiently checked (Criterion 6 only partially mitigates this by
mentioning version-constraint loosening/removal).

ymir/agents/prompts/backport/prompt_fix_build_error.j2[25-36]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[129-145]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[179-184]

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 prompt’s Self-Review Criterion 2 does not explicitly require that spec changes are *additions only* (no deletions/edits of existing BuildRequires/Requires lines). Because Criterion 2(b) only validates “each added dependency,” an agent could delete or edit an existing dependency line and still satisfy the listed checks.

## Issue Context
CRITICAL CONSTRAINTS already state: “Do NOT modify or remove existing BuildRequires/Requires entries.” Criterion 6 partially covers version-constraint loosening/removal, but Criterion 2 is the primary gate for spec diffs and should explicitly reject any non-additive changes to dependency lines.

## Fix Focus Areas
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[120-147]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[169-187]

## Suggested prompt changes (high level)
- In Criterion 2(a) or as a new sub-check, require that the `git diff` for `*.spec` contains **only added lines** introducing new `BuildRequires:` / `Requires:` entries, and **no deletions or edits** of any existing dependency line.
- Make “any `-` lines affecting BuildRequires/Requires” an automatic Criterion 2 failure (even if still within the same sections).

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


8. Requires validation contradicts rules ✓ Resolved 🐞 Bug ≡ Correctness
Description
The prompt allows adding new runtime Requires when the backported patch introduces runtime
dependencies, but Self-Review Criterion 2b requires every added dependency to appear in the build
error. This can incorrectly fail self-review for legitimate Requires additions that are needed for
runtime correctness but are not mentioned in build logs.
Code

ymir/agents/prompts/backport/prompt_fix_build_error.j2[R133-136]

+     b. Each added dependency corresponds to a header/library/binary that:
+        - Appears in the build error (missing header, undefined reference, etc.)
+        - Is used by code in your backported patch (verify by reading the patch)
+        - Is not a workaround for COPR environmental differences
Relevance

●●● Strong

Team frequently accepts prompt/self-review wording corrections and guardrail alignment; no matching
rejection precedent found.

PR-#598
PR-#655

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The CRITICAL CONSTRAINTS section permits adding Requires based on patch-introduced runtime
dependencies, but Criterion 2b later requires that each added dependency appears in the build error,
creating a rule mismatch for runtime dependencies not reflected in build logs.

ymir/agents/prompts/backport/prompt_fix_build_error.j2[25-32]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[130-137]

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 prompt’s spec-change allowance includes adding `Requires` for new runtime dependencies, but the self-review validation requires each added dependency to be present in the *build error*. That condition is appropriate for `BuildRequires` (compile/link failures), but it can reject correct `Requires` additions that won’t show up in build errors.

## Issue Context
This prompt is used to guide agents fixing backport build failures. The new rules add a narrow path for spec edits when maintainer rules allow it; the self-review criteria must match the allowed behavior to avoid false self-review failures.

## Fix Focus Areas
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[25-32]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[130-137]

## Suggested change
Update Criterion 2’s validation to distinguish between dependency types:
- For **BuildRequires additions**: require a direct match to missing headers/libs/symbols shown in the build error and used by the backported patch.
- For **Requires additions**: require evidence the patch introduces a runtime dependency (e.g., new executable/library usage in the patch, or test/runtime failure indicating missing dependency), and do **not** require it to appear in the build error.
Optionally tighten the earlier “Adding Requires” bullet to require some observable evidence (tests/logs/patch usage), so it aligns with the refined Criterion 2.

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


9. Ambiguous “modify” vs “add” ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The new constraints say the agent may “modify” BuildRequires/Requires, but later criteria constrain
spec changes to dependency additions only. This ambiguity can lead to inconsistent agent behavior
around editing/removing existing dependency entries (including version constraints).
Code

ymir/agents/prompts/backport/prompt_fix_build_error.j2[R29-32]

+  IF maintainer rules explicitly allow adding BuildRequires/Requires for backport fixes:
+    You may modify ONLY BuildRequires/Requires in the spec file, and ONLY when:
+    * Adding BuildRequires: the build error shows missing headers/libraries that are
+      DIRECTLY INTRODUCED by your backported patch (check the patch diff to confirm).
Relevance

●●● Strong

Team has accepted prompt wording/clarity fixes to avoid ambiguity and inconsistent agent behavior.

PR-#655

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
One section permits “modify” while the later self-review explicitly restricts allowed spec changes
to additions, creating an inconsistency in the instructions.

ymir/agents/prompts/backport/prompt_fix_build_error.j2[29-34]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[165-167]

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 prompt uses “modify BuildRequires/Requires,” but other parts of the prompt (criteria) describe the only permitted spec changes as *additions* of new `BuildRequires/Requires`. The mismatch is ambiguous and could lead to accidental edits/removals of existing requirements.

### Issue Context
The PR’s intent is to allow narrowly-scoped dependency additions. The prompt should reflect that consistently.

### Fix Focus Areas
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[29-34]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[165-167]

### Suggested change
Replace wording like “You may modify ONLY BuildRequires/Requires” with “You may **add** BuildRequires/Requires entries only,” and explicitly state that existing dependency entries (names/versions/constraints) must not be changed/removed.

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



Informational

10. Spec edits not in attempt log ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The prompt newly permits adding BuildRequires/Requires in dist-git, but the fix-attempt logging
instructions don’t explicitly require recording any spec dependency additions, reducing traceability
for future fix attempts and reviews.
Code

ymir/agents/prompts/backport/prompt_fix_build_error.j2[R53-56]

+- DO NOT modify anything in {{ local_clone }} dist-git repository except:
+  a. The backport patch file(s) you created (by regenerating them from upstream repo)
+  b. Adding new BuildRequires/Requires entries to the spec file (ONLY if maintainer
+     rules allow it, per above; do NOT modify or remove existing entries)
Relevance

●●● Strong

Team has accepted tightening backport prompt guardrails; logging spec dependency adds is a small
traceability improvement.

PR-#598

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The prompt explicitly expands allowed dist-git modifications to include adding new
BuildRequires/Requires, while the attempt-log requirements list only upstream edits/commits and the
build result, without calling out spec dependency additions.

ymir/agents/prompts/backport/prompt_fix_build_error.j2[53-56]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[106-110]

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

## Issue description
Spec dependency additions are now a permitted change type, but the instructions for updating `fix-attempts.md` don’t explicitly ask the agent to document added `BuildRequires/Requires` entries (what was added and why). This makes later debugging and avoiding repeated failed strategies harder.

## Issue Context
Step 6 currently focuses on upstream commits/manual edits and the build result. With spec changes allowed, the attempt log should also capture any dependency additions and their justification.

## Fix Focus Areas
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[53-56]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[106-110]

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


11. Duplicated rules lookup step ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The prompt describes the get_maintainer_rules lookup in both CRITICAL CONSTRAINTS and again as
workflow step 0, which can confuse agents about whether they must invoke it twice and may lead to
redundant tool calls. This adds avoidable latency/cost and increases the chance the agent
inconsistently references “the earlier call.”
Code

ymir/agents/prompts/backport/prompt_fix_build_error.j2[R69-72]

+0. Use the `get_maintainer_rules` tool with package {{ package }} to check whether
+   the maintainer allows spec file modifications during backport build fixes.
+   This determines whether you can add BuildRequires/Requires (see CRITICAL CONSTRAINTS above).
+
Relevance

●●● Strong

Team often accepts removing duplication/avoiding redundant work to reduce latency and confusion.

PR-#498
PR-#429
PR-#655

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The template contains two separate instructions to perform the get_maintainer_rules check: once
under “Spec file modification rules” and again as “WORKFLOW: 0.”

ymir/agents/prompts/backport/prompt_fix_build_error.j2[25-28]
ymir/agents/prompts/backport/prompt_fix_build_error.j2[69-72]

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 prompt repeats the `get_maintainer_rules` instruction in two places (constraints and workflow). This can be interpreted as requiring two calls, or it can be unclear which call is considered “earlier” for later criteria.

### Issue Context
This is a prompt template; redundancy can cause unnecessary tool invocations and ambiguity during self-review.

### Fix Focus Areas
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[25-28]
- ymir/agents/prompts/backport/prompt_fix_build_error.j2[69-72]

### Suggested change
Keep the detailed policy in CRITICAL CONSTRAINTS, and change workflow step 0 to something like: "If you have not already done so, call `get_maintainer_rules` once and reuse the result throughout this run." (or remove the duplicate step entirely and reference the constraints section).

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


Grey Divider

Context used
✅ Compliance rules (platform): 7 rules

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2
Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2 Outdated
Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2 Outdated
@nforro

nforro commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit bbb4201

opohorel
opohorel previously approved these changes Aug 5, 2026

@opohorel opohorel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thanks!

@nforro

nforro commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2 Outdated
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit fef1bac

@nforro

nforro commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2 Outdated
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 3714443

@nforro

nforro commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 14afb69

@nforro

nforro commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2 Outdated
Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 1a8ea0f

@nforro

nforro commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2 Outdated
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 0c79578

@nforro

nforro commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 0d8c605

@nforro

nforro commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread ymir/agents/prompts/backport/prompt_fix_build_error.j2
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 3ad941a

Signed-off-by: Nikola Forró <nforro@redhat.com>
Assisted-by: Claude Sonnet 4.5 via Claude Code
@nforro

nforro commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f36da7c

@nforro

nforro commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f36da7c

@nforro

nforro commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

It seems Qodo is finally happy, even though it didn't cross one of the issues in the review. @opohorel can I ask for a re-review/re-approval?

@opohorel opohorel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I thought this was ok even before the Qodos suggestions were implemented. on re-review it looks even better 😁

@nforro
nforro merged commit 649ec73 into packit:main Aug 6, 2026
11 checks passed
@nforro
nforro deleted the requires branch August 6, 2026 08:44
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.

2 participants