Skip to content

[WIP] Trying to improve the naming conventions of the backport agent #507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mruprich wants to merge 1 commit into
base: main
Choose a base branch
from
+57 −28
Open

[WIP] Trying to improve the naming conventions of the backport agent #507

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 57 additions & 28 deletions agents_as_skills/backport/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,22 @@ Execute the following steps in order. Track state across steps (paths, flags, re

Determine `pkg_tool` from the branch: if `dist_git_branch` starts with `c` and ends with `s` (e.g., `c10s`, `c9s`), use `centpkg`; otherwise use `rhpkg --offline --released`.

### Patch File Naming

All patch files in dist-git (and pre-downloaded upstream patches from Step 2) must use descriptive names — not Jira issue keys. Follow these rules:
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

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

high

The instructions in this file are being updated to use descriptive patch names, but the actual instructions used by the agent are currently hardcoded as strings in ymir/agents/backport_agent.py (see BACKPORT_INSTRUCTIONS and BACKPORT_INSTRUCTIONS_ZSTREAM). Since the agent does not read SKILL.md directly, these changes will not take effect until the Python file is also updated to reflect these new rules.


1. **Descriptive filenames**: Hyphen-separated words, prefixed with the package name (`{{package}}`). The name should describe the fix (from the diff, commit message, or upstream change), not the Jira key.
2. **Single-file patches**: When a patch modifies only one source file, use that file's base name as the second field after the package prefix (e.g., `openssl-configure.ac-fix-dependency-check.patch`).
3. **`PatchN` numbering in the spec**: Assign the next sequential `Patch` number after existing entries. Use `get_package_info` to find the highest numbered patch; numbering starts at `Patch0` and increases (`Patch1`, `Patch2`, …, `Patch25`, etc.).
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

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

medium

The instruction to use get_package_info to find the highest numbered patch is problematic. The GetPackageInfoTool (defined in ymir/tools/unprivileged/specfile.py) returns a list of patch filenames but does not include the actual patch numbers from the spec file. If a package uses non-sequential numbering or starts at a high number (e.g., Patch100), the agent will be unable to determine the correct next number from the tool's output alone. Consider updating the tool to return a mapping of numbers to filenames or instructing the agent to inspect the spec file directly using the view tool.

4. **Security fixes**: When `cve_id` is set or the backport addresses a security issue, include the CVE identifier (or equivalent) in the filename (e.g., `openssl-fix-CVE-2007-0123.patch`).

**Examples** (package `openssl`, next free slots `Patch42` and `Patch43`):

- `Patch42: openssl-configure.ac-fix-dependency-check.patch`
- `Patch43: openssl-fix-CVE-2007-0123.patch`

When generating or saving a new dist-git patch, choose `<NEW_PATCH_FILE>` using these rules and use that path consistently in `git_patch_create`, spec `Patch:` tags, and build-fix steps.

### Step 1: Change JIRA Status

If `dry_run` is false:
Expand All @@ -128,7 +144,7 @@ If `dry_run` is true, skip this step.
3. Call `download_sources` with `dist_git_path` = `local_clone`, `package` = `{{package}}`, `dist_git_branch` = `{{dist_git_branch}}`.
4. Run `<pkg_tool> --name={{package}} --namespace=rpms --release={{dist_git_branch}} prep` to unpack sources.
5. Determine `unpacked_sources` — the path to the unpacked source directory tree. This is typically `<local_clone>/<name>-<version>-build/<buildsubdir>` (RPM 4.20+) or `<local_clone>/<buildsubdir>` (older RPM). Read the spec file to find `%{name}`, `%{version}`, and `%{buildsubdir}`.
6. Download each upstream patch URL to a local file named `{{jira_issue}}-<N>.patch` (0-indexed) using `get_patch_from_url`, and save the content to `<local_clone>/{{jira_issue}}-<N>.patch`.
6. Download each upstream patch URL using `get_patch_from_url`. Save each file under `<local_clone>/` with a **descriptive** name per **Patch File Naming** above (inspect the patch diff or commit message to choose the suffix). Do not use `{{jira_issue}}-<N>.patch` or other issue-key-based names. If multiple upstream URLs are provided, give each download its own descriptive filename.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

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

high

This instruction tells the agent that pre-downloaded patches have descriptive names. However, the Python implementation in ymir/agents/backport_agent.py (line 934) still downloads and saves patches using the {{jira_issue}}-{idx}.patch format. This discrepancy will cause the agent to fail when it tries to locate the pre-downloaded patches using the new naming convention. The Python code in the fork_and_prepare_dist_git step must be updated to use descriptive names as well.

7. Initialize workflow tracking flags: `used_cherry_pick_workflow = false`, `incremental_fix_attempts = 0`.

### Step 3: Determine Instruction Variant
Expand Down Expand Up @@ -442,11 +458,13 @@ and must remain unchanged.
use `cherry_pick_continue` with `allow_empty=True`, or skip the commit.

3k. Generate the final patch file from upstream:
- Choose `<NEW_PATCH_FILE>` per **Patch File Naming** (package prefix, descriptive
suffix, CVE in name if applicable).
- Use `get_package_info` to determine the next `PatchN` number for the spec.
- Use `git_patch_create` tool with:
* repository_path: <UPSTREAM_REPO>
* patch_file_path: <JIRA_ISSUE>.patch in the current working
directory (the dist-git repository root)
(e.g., if JIRA is RHEL-114639, use /path/to/distgit/RHEL-114639.patch)
* patch_file_path: `<LOCAL_CLONE>/<NEW_PATCH_FILE>`
(e.g., `openssl-configure.ac-fix-dependency-check.patch`)
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

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

medium

The example should include the <LOCAL_CLONE>/ path prefix as specified in the instruction on the previous line. This helps the agent understand it should use the full path to the patch file within the repository.

Suggested change
(e.g., `openssl-configure.ac-fix-dependency-check.patch`)
(e.g., /path/to/clone/openssl-configure.ac-fix-dependency-check.patch)

- The tool automatically uses the base commit recorded in step 3i to include
ALL cherry-picked commits, not just the last one
- IMPORTANT: Only create NEW patch files. Do NOT modify
Expand All @@ -464,29 +482,31 @@ and must remain unchanged.

This is the fallback when approaches A or B cannot be completed.

Note: For this workflow, use the pre-downloaded patch files in the current working directory.
They are called `<JIRA_ISSUE>-<N>.patch` where <N> is a 0-based index. For example,
for a `RHEL-12345` Jira issue the first patch would be called `RHEL-12345-0.patch`.
Note: For this workflow, use the pre-downloaded patch files from Step 2 in the
current working directory (`<LOCAL_CLONE>/`). Each file has a descriptive name per
**Patch File Naming** (e.g., `openssl-configure.ac-fix-dependency-check.patch`).

Backport all patches individually using steps C1 and C2 below.

C1. Backport one patch at a time using the following steps:
- If a cherry-pick is in progress, abort it first:
`git -C <UPSTREAM_REPO> cherry-pick --abort`
- Use the `git_patch_apply` tool with the patch file: <JIRA_ISSUE>-<N>.patch
- Use the `git_patch_apply` tool with each pre-downloaded patch file.
This works on <UNPACKED_SOURCES>, NOT <UPSTREAM_REPO>.
- Resolve all conflicts and leave the repository in a dirty state. Delete all *.rej files.
- Use the `git_apply_finish` tool to finish the patch application.
- Repeat for each pre-downloaded patch file.

C2. After ALL patches have been applied, generate a single combined patch:
- Choose `<NEW_PATCH_FILE>` per **Patch File Naming**.
- Use `get_package_info` to determine the next `PatchN` number for the spec.
- Use `git_patch_create` tool with:
* repository_path: <UNPACKED_SOURCES>
* patch_file_path: <JIRA_ISSUE>.patch in the current working
directory (the dist-git repository root)
* patch_file_path: `<LOCAL_CLONE>/<NEW_PATCH_FILE>`
- The tool automatically captures all applied changes into one patch file.

4. Update the spec file. Add ONE new `Patch` tag for each new patch file.
4. Update the spec file. Add ONE new `Patch` tag for each new patch file (use the next
sequential `PatchN` number per **Patch File Naming**).
Add the new `Patch` tag(s) after all existing `Patch` tags and, if `Patch` tags are numbered,
make sure they have the highest numbers. Make sure each patch is applied in the "%prep" section
and the `-p` argument is correct. Do NOT add any comments to the spec file.
Expand Down Expand Up @@ -580,7 +600,8 @@ and must remain unchanged.
- If is_distgit is False, proceed to step 4
- If is_distgit is True, continue to check if it's a spec-only change

b. Examine the pre-downloaded patch file <JIRA_ISSUE>-0.patch
b. Examine the first pre-downloaded patch file from Step 2 (descriptive name per
**Patch File Naming**)

c. Check what files the patch modifies by looking at the "diff --git" lines

Expand Down Expand Up @@ -674,11 +695,13 @@ and must remain unchanged.
use `cherry_pick_continue` with `allow_empty=True`, or skip the commit.

4g. Generate the final patch file from upstream:
- Choose `<NEW_PATCH_FILE>` per **Patch File Naming** (package prefix, descriptive
suffix, CVE in name if applicable).
- Use `get_package_info` to determine the next `PatchN` number for the spec.
- Use `git_patch_create` tool with:
* repository_path: <UPSTREAM_REPO>
* patch_file_path: <JIRA_ISSUE>.patch in the current working
directory (the dist-git repository root)
(e.g., if JIRA is RHEL-114639, use /path/to/distgit/RHEL-114639.patch)
* patch_file_path: `<LOCAL_CLONE>/<NEW_PATCH_FILE>`
(e.g., `openssl-fix-CVE-2007-0123.patch`)
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

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

medium

The example should include the <LOCAL_CLONE>/ path prefix as specified in the instruction on the previous line to ensure the agent uses the correct absolute or relative path.

Suggested change
(e.g., `openssl-fix-CVE-2007-0123.patch`)
(e.g., /path/to/clone/openssl-fix-CVE-2007-0123.patch)

- The tool automatically uses the base commit recorded in step 4e to include
ALL cherry-picked commits, not just the last one
- IMPORTANT: Only create NEW patch files. Do NOT modify
Expand All @@ -694,29 +717,31 @@ and must remain unchanged.

B. GIT AM WORKFLOW (Fallback approach):

Note: For this workflow, use the pre-downloaded patch files in the current working directory.
They are called `<JIRA_ISSUE>-<N>.patch` where <N> is a 0-based index. For example,
for a `RHEL-12345` Jira issue the first patch would be called `RHEL-12345-0.patch`.
Note: For this workflow, use the pre-downloaded patch files from Step 2 in the
current working directory (`<LOCAL_CLONE>/`). Each file has a descriptive name per
**Patch File Naming**.

Backport all patches individually using steps B1 and B2 below.

B1. Backport one patch at a time using the following steps:
- If a cherry-pick is in progress, abort it first:
`git -C <UPSTREAM_REPO> cherry-pick --abort`
- Use the `git_patch_apply` tool with the patch file: <JIRA_ISSUE>-<N>.patch
- Use the `git_patch_apply` tool with each pre-downloaded patch file.
This works on <UNPACKED_SOURCES>, NOT <UPSTREAM_REPO>.
- Resolve all conflicts and leave the repository in a dirty state. Delete all *.rej files.
- Use the `git_apply_finish` tool to finish the patch application.
- Repeat for each pre-downloaded patch file.

B2. After ALL patches have been applied, generate a single combined patch:
- Choose `<NEW_PATCH_FILE>` per **Patch File Naming**.
- Use `get_package_info` to determine the next `PatchN` number for the spec.
- Use `git_patch_create` tool with:
* repository_path: <UNPACKED_SOURCES>
* patch_file_path: <JIRA_ISSUE>.patch in the current working
directory (the dist-git repository root)
* patch_file_path: `<LOCAL_CLONE>/<NEW_PATCH_FILE>`
- The tool automatically captures all applied changes into one patch file.

5. Update the spec file. Add ONE new `Patch` tag for <JIRA_ISSUE>.patch.
5. Update the spec file. Add ONE new `Patch` tag for `<NEW_PATCH_FILE>` with the next
sequential `PatchN` number per **Patch File Naming**.
Add the new `Patch` tag after all existing `Patch` tags and, if `Patch` tags are numbered,
make sure it has the highest number. Make sure the patch is applied in the "%prep" section
and the `-p` argument is correct. Add upstream URLs as comments above
Expand Down Expand Up @@ -845,15 +870,17 @@ SPECIAL CONSIDERATIONS FOR TEST FAILURES:

STEP 4: Regenerate the patch
- After making your fixes (cherry-picked or manual), regenerate the patch file
- Use the same `<NEW_PATCH_FILE>` path as in the backport instructions (descriptive name per
**Patch File Naming** — do not rename to a Jira-key-based filename)
- Use `git_patch_create` tool with:
* repository_path: <LOCAL_CLONE>-upstream
* patch_file_path: <LOCAL_CLONE>/<JIRA_ISSUE>.patch
* patch_file_path: <LOCAL_CLONE>/<NEW_PATCH_FILE>
- The tool automatically uses the base commit to include all changes
- This creates a single patch with all changes: original commits + prerequisites/fixes
- This improved patch now includes all missing dependencies needed for a successful build

STEP 5: Test the build
- The spec file should already reference <JIRA_ISSUE>.patch
- The spec file should already reference <NEW_PATCH_FILE>
- Run `<PKG_TOOL> --name=<PACKAGE> --namespace=rpms --release=<DIST_GIT_BRANCH> prep` to verify patch applies
- Run `<PKG_TOOL> --name=<PACKAGE> --namespace=rpms --release=<DIST_GIT_BRANCH> srpm` to generate SRPM
- Test if the SRPM builds successfully using the `build_package` tool:
Expand All @@ -879,7 +906,7 @@ Report your results:
IMPORTANT RULES:
- Work in the EXISTING <LOCAL_CLONE>-upstream directory (don't clone again)
- NEVER modify the spec file - build failures are caused by incomplete patches, not spec issues
- The ONLY dist-git file you can modify is <JIRA_ISSUE>.patch (by regenerating it from upstream repo)
- The ONLY dist-git file you can modify is <NEW_PATCH_FILE> (by regenerating it from upstream repo)
- Fix build errors (compilation AND test failures) by adding missing prerequisites/dependencies to your patches in upstream repo
- For test failures: backport minimal necessary test helpers/functions to make tests pass
- You can freely explore, edit, cherry-pick, and commit in the upstream repo - it's your workspace
Expand Down Expand Up @@ -972,15 +999,17 @@ SPECIAL CONSIDERATIONS FOR TEST FAILURES:

STEP 4: Regenerate the patch
- After making your fixes (cherry-picked or manual), regenerate the patch file
- Use the same `<NEW_PATCH_FILE>` path as in the backport instructions (descriptive name per
**Patch File Naming** — do not rename to a Jira-key-based filename)
- Use `git_patch_create` tool with:
* repository_path: <LOCAL_CLONE>-upstream
* patch_file_path: <LOCAL_CLONE>/<JIRA_ISSUE>.patch
* patch_file_path: <LOCAL_CLONE>/<NEW_PATCH_FILE>
- The tool automatically uses the base commit to include all changes
- This creates a single patch with all changes: original commits + prerequisites/fixes
- This improved patch now includes all missing dependencies needed for a successful build

STEP 5: Test the build
- The spec file should already reference <JIRA_ISSUE>.patch
- The spec file should already reference <NEW_PATCH_FILE>
- Run `<PKG_TOOL> --name=<PACKAGE> --namespace=rpms --release=<DIST_GIT_BRANCH> prep` to verify patch applies
- Run `<PKG_TOOL> --name=<PACKAGE> --namespace=rpms --release=<DIST_GIT_BRANCH> srpm` to generate SRPM
- Test if the SRPM builds successfully using the `build_package` tool:
Expand All @@ -1006,7 +1035,7 @@ Report your results:
IMPORTANT RULES:
- Work in the EXISTING <LOCAL_CLONE>-upstream directory (don't clone again)
- NEVER modify the spec file - build failures are caused by incomplete patches, not spec issues
- The ONLY dist-git file you can modify is <JIRA_ISSUE>.patch (by regenerating it from upstream repo)
- The ONLY dist-git file you can modify is <NEW_PATCH_FILE> (by regenerating it from upstream repo)
- Fix build errors (compilation AND test failures) by adding missing prerequisites/dependencies to your patches in upstream repo
- For test failures: backport minimal necessary test helpers/functions to make tests pass
- You can freely explore, edit, cherry-pick, and commit in the upstream repo - it's your workspace
Expand Down
Loading