Skip to content

Keep backport repair diagnostics out of generated patches - #822

Merged
opohorel merged 1 commit into
packit:mainfrom
opohorel:fix_attempts
Sep 11, 2026
Merged

opohorel merged 1 commit into
packit:mainfrom
opohorel:fix_attempts

Conversation

@opohorel

Copy link
Copy Markdown
Collaborator

Repair notes inside the upstream checkout could be staged by git add -A and exported into RPM patches. Store notes and archived build logs beside both Git checkouts, and pass the log path explicitly to the repair prompt.

Add c9s and c10s regression coverage for repeated repairs, persistent notes, and real Git staging and patch generation. Use flexmock for dependency mocks.

Assisted-by: Codex

Repair notes inside the upstream checkout could be staged by git add -A
and exported into RPM patches. Store notes and archived build logs beside
both Git checkouts, and pass the log path explicitly to the repair prompt.

Add c9s and c10s regression coverage for repeated repairs, persistent notes,
and real Git staging and patch generation. Use flexmock for dependency
mocks.

Assisted-by: Codex
@qodo-for-packit

Copy link
Copy Markdown

PR Summary by Qodo

Keep backport repair diagnostics out of generated patches

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Store repair notes and archived build logs outside both Git worktrees.
• Pass the diagnostics path explicitly to repair prompts across repeated attempts.
• Verify c9s and c10s patches contain only intended source changes.
Diagram

graph TD
  BA["Build Agent"] -->|archives logs| DD["Diagnostics Dir"]
  BA -->|passes path| RA["Repair Agent"] -->|reads and appends| DD
  RA -->|commits source fixes| UR["Upstream Repo"] -->|generates patch| PT["Patch Tool"] -->|writes patch| DG["Dist Git"]
Loading
High-Level Assessment

The sibling diagnostics directory is the safest approach because it preserves repair history while making accidental staging from either Git worktree impossible. Repository ignore rules were considered but would leave diagnostics inside the checkout and dependent on Git configuration, while embedding history in prompts would increase prompt size and weaken persistence.

Files changed (6) +176 / -10

Bug fix (3) +21 / -6
backport_agent.pyMove repair diagnostics outside Git worktrees +10/-3

Move repair diagnostics outside Git worktrees

• Adds a helper for deriving the sibling diagnostics directory and uses it for initial and repeated build-log archival. Supplies the directory explicitly through the repair-specific prompt schema.

ymir/agents/backport_agent.py

prompt_fix_build_error.j2Direct repair agents to external diagnostics +5/-3

Direct repair agents to external diagnostics

• Instructs repair agents to stage only intended source files and keep logs and notes outside both repositories. Reads and appends repair history using the explicitly supplied diagnostics path.

ymir/agents/prompts/backport/prompt_fix_build_error.j2

models.pyAdd repair-specific backport input schema +6/-0

Add repair-specific backport input schema

• Introduces a BackportInputSchema extension carrying the external build logs and persistent repair notes directory.

ymir/common/models.py

Tests (2) +147 / -4
test_backport_build_logs.pyAdd real-Git regression coverage for repair diagnostics +135/-0

Add real-Git regression coverage for repair diagnostics

• Adds c9s and c10s workflow tests covering two repair attempts, persistent notes, archived logs, blanket Git staging, and generated patch contents. Uses flexmock for dependencies while exercising real repositories and patch generation.

ymir/agents/tests/unit/test_backport_build_logs.py

test_jinja2_templates.pyValidate repair diagnostics paths in rendered prompts +12/-4

Validate repair diagnostics paths in rendered prompts

• Adds the repair-specific schema to template test fixtures and supplies an external build-logs path. Verifies rendered prompts reference that path and omit the former upstream build-logs location.

ymir/agents/tests/unit/test_jinja2_templates.py

Documentation (1) +8 / -0
README-agents.mdDocument external repair diagnostics storage +8/-0

Document external repair diagnostics storage

• Documents that incremental repair notes and archived logs live in a sibling build-logs directory. Explains persistence across attempts, exclusion from Git staging, prompt access, and fresh-run cleanup.

README-agents.md

@qodo-for-packit

qodo-for-packit Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Repair agents lose prior build logs ✗ Dismissed 🐞 Bug ≡ Correctness
Description
_move_build_logs archives files only from state.local_clone, while download_artifacts writes
failed-build logs to a gateway-side temporary directory that local repair tools cannot access.
Whenever production uses the gateway log tools, attempt-0 and later attempt directories remain
empty, so subsequent repairs retain notes and summarized errors but cannot inspect the promised
historical raw logs.
Code

ymir/agents/backport_agent.py[1165]

+                        _get_build_logs_dir(state.local_clone) / "attempt-0",
Relevance

●●● Strong

Gateway-downloaded logs are inaccessible to local archiving; prior incremental log archival
correctness fixes were accepted.

PR-#498

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed archive destination still receives files exclusively from the dist-git clone, but the
actual downloader writes them under tempfile.mkdtemp(). The build agent explicitly documents that
gateway-downloaded files are outside the local agent sandbox, while the new test bypasses this
production flow by manually creating logs in the clone.

ymir/agents/backport_agent.py[1043-1052]
ymir/agents/backport_agent.py[1160-1166]
ymir/agents/build_agent.py[157-180]
ymir/tools/privileged/copr.py[351-375]
ymir/agents/tests/unit/test_backport_build_logs.py[40-54]

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

## Issue description
Failed-build artifacts are downloaded into a gateway-side temporary directory, but the backport workflow archives only files found in the dist-git clone. As a result, the external diagnostics directory does not retain production build logs between repair attempts.

## Fix Focus Areas
- ymir/agents/backport_agent.py[1162-1165]
- ymir/agents/backport_agent.py[1047-1051]
- ymir/agents/build_agent.py[157-180]
- ymir/tools/privileged/copr.py[351-375]

## Recommended Fix
Add an explicit, safely validated mechanism that persists downloaded build logs into the issue's `build_logs_dir`, and invoke it for the initial failed build and every failed repair before advancing the attempt. Ensure the transfer works across the gateway and local-agent sandbox boundary, then update the regression test to exercise the real artifact-download path rather than manually placing log files in `state.local_clone`.

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


Grey Divider

Context sources
✅ Compliance rules (platform): 8 rules
Review mode: ⚖️ Balanced: This changes backport repair workflow behavior, prompt contracts, filesystem locations, Git patch generation, and schemas across multiple code paths, creating genuine regression risk but not enough independent logic density to warrant extended review.

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread ymir/agents/backport_agent.py
@opohorel
opohorel merged commit c992c9c into packit:main Sep 11, 2026
13 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.

2 participants