Skip to content

feat: create separate PRs per branch for downstream repos in cve.fix#101

Open
vmrh21 wants to merge 3 commits intoambient-code:mainfrom
angaduom:fix-branch-targeting-per-repo-type
Open

feat: create separate PRs per branch for downstream repos in cve.fix#101
vmrh21 wants to merge 3 commits intoambient-code:mainfrom
angaduom:fix-branch-targeting-per-repo-type

Conversation

@vmrh21
Copy link
Copy Markdown
Contributor

@vmrh21 vmrh21 commented Apr 2, 2026

Summary

Updates cve.fix to target branches differently based on repo_type:

  • upstream / midstream: PR against default_branch only (e.g. main)
  • downstream: separate PR for default_branch + each branch in active_release_branches

Each branch gets its own independent clone → fix → test → PR cycle. Branches are never combined into a single PR.

Example for red-hat-data-services/llm-d-inference-scheduler with active branches rhoai-3.3, rhoai-3.4, rhoai-3.4-ea.1, rhoai-3.4-ea.2:

  • PR 1 → base: main
  • PR 2 → base: rhoai-3.3
  • PR 3 → base: rhoai-3.4
  • PR 4 → base: rhoai-3.4-ea.1
  • PR 5 → base: rhoai-3.4-ea.2

No mapping file changes required — logic is derived from existing repo_type, default_branch, and active_release_branches fields.

Test plan

  • Run /cve.fix llm-d with an inference-scheduler CVE — verify upstream gets 1 PR (main only) and downstream gets 5 PRs (main + 4 release branches)
  • Verify each PR targets the correct base branch
  • Verify fix branch naming includes the target branch (e.g. fix/cve-YYYY-XXXXX-urllib3-rhoai-3.4-attempt-1)

🤖 Generated with Claude Code

- upstream/midstream: PR against default_branch only (main)
- downstream: separate PR for default_branch + each active_release_branch

Each branch is independently fixed and gets its own PR — never combined.
Fix branch naming includes the target branch for clarity, e.g.:
fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1

No mapping file changes needed — logic derived from existing
repo_type, default_branch, and active_release_branches fields.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 2, 2026

Walkthrough

CVE workflow docs updated: cve.fix now determines target branches from repo_type, clones once, checks push rights via GitHub Classic PAT/gh api, falls back to forks, and loops per target branch to checkout/pull/apply fixes and open separate PRs. cve.find now verifies Jira access via /rest/api/3/myself and handles HTTP status codes instead of pre-checking env vars.

Changes

Cohort / File(s) Summary
CVE fixer workflow
workflows/cve-fixer/.claude/commands/cve.fix.md
Reworked target-branch model: upstream/midstreamdefault_branch; downstreamdefault_branch + active_release_branches. Multi-repo strategy now repeats per-repo/per-branch fix steps, creating separate PRs per branch. New repo flow: set up GitHub Classic PAT auth, clone (depth=1) to /tmp/<org>/<name>, check push permissions with gh api, fallback to fork when no write access, and perform per-branch git checkout/git pull before applying fixes and pushing branch-specific PRs.
Jira access verification
workflows/cve-fixer/.claude/commands/cve.find.md
Removed early env-var presence checks; added a lightweight Jira REST call to /rest/api/3/myself using Basic Auth and branch logic on HTTP status: 200 → proceed, 401 → prompt token/email setup, 403 → insufficient permissions, others/timeouts → network issue and single retry. Removed redundant AUTH re-derivation and deferred credential setup until 401.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant WF as Workflow
participant Git as Local Git (clone/checkout)
participant GH as GitHub API (gh api)
participant Remote as Remote Repo (origin)
participant Fork as Fork Repo
WF->>GH: authenticate using Classic PAT
WF->>Git: git clone --depth=1 /tmp//
WF->>GH: check push permission via gh api
alt has write access
WF->>Git: for each TARGET_BRANCH: checkout & git pull
WF->>Git: apply fixes, commit, create branch
Git->>Remote: git push branch
WF->>GH: create PR to origin
else no write access
WF->>Fork: push branch to fork
WF->>GH: create PR from fork -> origin
end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: creating separate PRs per branch for downstream repos in cve.fix, which is the primary objective of the changeset.
Description check ✅ Passed The description is directly related to the changeset, explaining the branch-targeting logic for different repo types, providing concrete examples, and outlining the test plan.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 163-172: Update the fenced code blocks in
workflows/cve-fixer/.claude/commands/cve.fix.md to satisfy MD031/MD040 by adding
explicit language identifiers (e.g., ```text for the plain table and ```bash for
the shell block) and ensuring there is a blank line before and after each fenced
block; locate the blocks using the existing fence markers (``` and ```bash) and
the shown content ("upstream  llm-d/..." table and the "# Clone once:" shell
example) and insert the missing blank lines and the language tag on the opening
fence for each block.
- Around line 174-179: The branch naming guidance is inconsistent: the
multi-branch example uses a target-branch token (e.g.,
fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1) but the canonical rule later
omits it (fix/cve-...-<package>-attempt-1), risking collisions across base
branches; update the canonical naming rule to include a target-branch
placeholder (e.g., <target-branch> or the actual base branch name) and make both
the example and the canonical pattern consistent so every fix branch (symbols:
fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-N) uniquely identifies the
base branch and prevents reuse/overwrite when repeating steps 4–11 for each
branch.
- Around line 183-191: The current flow that "Clone the repo once" then loops
over TARGET_BRANCHES (steps 5–11) can leak state between branch iterations;
update the procedure so each target branch runs in an isolated workspace: for
each branch in TARGET_BRANCHES either (a) create a fresh clone (git clone into a
new REPO_DIR per branch) or (b) perform a strict reset (git fetch --all; git
checkout <branch>; git reset --hard origin/<branch>; git clean -fdx) before
applying fixes, and ensure git credentials configuration (gh auth setup-git /
credential.helper / SSH fallback) is applied for each cloned workspace so
pushes/PRs cannot be affected by artifacts, uncommitted changes, or lockfile
drift from previous iterations.
- Around line 155-159: The TARGET_BRANCHES array construction can include
duplicates if ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH; change the logic
that sets TARGET_BRANCHES (the branch assembly around TARGET_BRANCHES,
DEFAULT_BRANCH and ACTIVE_RELEASE_BRANCHES) to deduplicate entries before use —
for example, build TARGET_BRANCHES by iterating DEFAULT_BRANCH and each element
of ACTIVE_RELEASE_BRANCHES and only appending branches not already present (or
use a temporary associative set) so TARGET_BRANCHES contains unique branch names
and avoids duplicate PR attempts.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: dcde3137-0bcb-4cf1-9ef6-55b5f3a67393

📥 Commits

Reviewing files that changed from the base of the PR and between 66bceaf and 96c3e63.

📒 Files selected for processing (1)
  • workflows/cve-fixer/.claude/commands/cve.fix.md

Comment on lines +155 to +159
if [ "$REPO_TYPE" = "downstream" ]; then
TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
else
TARGET_BRANCHES=("$DEFAULT_BRANCH")
fi
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

TARGET_BRANCHES needs deduplication to avoid duplicate PR attempts.

If ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH, this creates duplicate targets and can trigger redundant PR creation/push conflicts.

Suggested doc fix
-  if [ "$REPO_TYPE" = "downstream" ]; then
-    TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
-  else
-    TARGET_BRANCHES=("$DEFAULT_BRANCH")
-  fi
+  if [ "$REPO_TYPE" = "downstream" ]; then
+    TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
+  else
+    TARGET_BRANCHES=("$DEFAULT_BRANCH")
+  fi
+
+  # Deduplicate while preserving order
+  UNIQUE_TARGET_BRANCHES=()
+  for b in "${TARGET_BRANCHES[@]}"; do
+    [[ -z "$b" ]] && continue
+    [[ " ${UNIQUE_TARGET_BRANCHES[*]} " == *" $b "* ]] || UNIQUE_TARGET_BRANCHES+=("$b")
+  done
+  TARGET_BRANCHES=("${UNIQUE_TARGET_BRANCHES[@]}")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "$REPO_TYPE" = "downstream" ]; then
TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
else
TARGET_BRANCHES=("$DEFAULT_BRANCH")
fi
if [ "$REPO_TYPE" = "downstream" ]; then
TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
else
TARGET_BRANCHES=("$DEFAULT_BRANCH")
fi
# Deduplicate while preserving order
UNIQUE_TARGET_BRANCHES=()
for b in "${TARGET_BRANCHES[@]}"; do
[[ -z "$b" ]] && continue
[[ " ${UNIQUE_TARGET_BRANCHES[*]} " == *" $b "* ]] || UNIQUE_TARGET_BRANCHES+=("$b")
done
TARGET_BRANCHES=("${UNIQUE_TARGET_BRANCHES[@]}")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 155 - 159, The
TARGET_BRANCHES array construction can include duplicates if
ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH; change the logic that sets
TARGET_BRANCHES (the branch assembly around TARGET_BRANCHES, DEFAULT_BRANCH and
ACTIVE_RELEASE_BRANCHES) to deduplicate entries before use — for example, build
TARGET_BRANCHES by iterating DEFAULT_BRANCH and each element of
ACTIVE_RELEASE_BRANCHES and only appending branches not already present (or use
a temporary associative set) so TARGET_BRANCHES contains unique branch names and
avoids duplicate PR attempts.

Comment on lines +174 to 179
**Multi-repo + multi-branch strategy**:
- Fix upstream repos first, then midstream, then downstream
- For downstream: Steps 4 through 11 repeat for EACH branch independently
- Each branch produces its own PR with its own fix branch (e.g., `fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1`)
- Never combine fixes for multiple branches into a single PR

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Branch naming rule is still inconsistent and can collide across target branches.

This section says branch names include the target branch, but the canonical naming rule later still omits it (fix/cve-...-<package>-attempt-1). That can overwrite/reuse the same fix branch name for multiple base branches.

Suggested doc fix
-  - Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-attempt-1`
+  - Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-<target-branch>-attempt-1`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 174 - 179, The
branch naming guidance is inconsistent: the multi-branch example uses a
target-branch token (e.g., fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1) but
the canonical rule later omits it (fix/cve-...-<package>-attempt-1), risking
collisions across base branches; update the canonical naming rule to include a
target-branch placeholder (e.g., <target-branch> or the actual base branch name)
and make both the example and the canonical pattern consistent so every fix
branch (symbols: fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-N)
uniquely identifies the base branch and prevents reuse/overwrite when repeating
steps 4–11 for each branch.

Ambient sessions inject secrets differently — JIRA_API_TOKEN and
JIRA_EMAIL may be available to API calls even when not visible to
bash [ -z "$VAR" ] checks, causing false "missing credentials" errors.

Replace the upfront env var check with a lightweight GET /myself test
call: proceed on 200, only complain on 401/403. Never hard-stop just
because the shell check returns empty.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@workflows/cve-fixer/.claude/commands/cve.find.md`:
- Around line 83-87: Insert a single blank line before the fenced code block in
the "a. Set up variables (AUTH already set from Step 2):" list item so the
markdown has a blank line immediately before the ```bash fence (fixing MD031).
Locate the fenced block in workflows/cve-fixer/.claude/commands/cve.find.md
under the "COMPONENT_NAME" setup section and add the blank line above the
```bash line to make the block compliant.
- Around line 69-75: The fenced code block starting with "```bash" containing
the export lines currently lacks surrounding blank lines (MD031); add one blank
line immediately before the opening "```bash" and one blank line immediately
after the closing "```" so the code block is separated from the surrounding list
text (update the block in workflows/cve-fixer/.claude/commands/cve.find.md where
the export JIRA_API_TOKEN/JIRA_EMAIL lines appear).
- Around line 58-79: The curl healthcheck using JIRA_BASE_URL and TEST_RESPONSE
should not treat HTTP 403 as "insufficient permissions" because
/rest/api/3/myself returns 401 for both auth failures and permission issues;
update the logic that branches on TEST_RESPONSE to consolidate 401 handling (log
both missing/invalid creds and insufficient permissions together and instruct
the user to check API token/email and permissions) and remove or disable the
separate 403 branch, or alternatively replace the endpoint with a
permissions-specific endpoint if you really need to distinguish 403; keep the
existing note about not pre-checking env vars (do not add a [ -z
"$JIRA_API_TOKEN" ] check).
- Around line 58-77: The documentation says to "retry once" on network/timeout
but the TEST_RESPONSE curl call lacks retry options; update the curl invocation
that sets TEST_RESPONSE (the command using JIRA_BASE_URL and AUTH) to perform a
single retry on transient failures by adding curl retry flags (e.g., --retry 1
plus an appropriate --retry-delay and --retry-connrefused) while preserving
existing timeouts (--connect-timeout, --max-time) and the exit-code capture (-s
-o /dev/null -w "%{http_code}"); ensure the adjusted curl still returns an HTTP
status code into TEST_RESPONSE so the subsequent HTTP 200/401/403/Other handling
continues to work.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2f7d15b0-6ed9-419e-b03a-15674de9f5c6

📥 Commits

Reviewing files that changed from the base of the PR and between 96c3e63 and 340b108.

📒 Files selected for processing (1)
  • workflows/cve-fixer/.claude/commands/cve.find.md

Comment on lines +58 to +79
```bash
JIRA_BASE_URL="https://redhat.atlassian.net"
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
TEST_RESPONSE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \
--connect-timeout 10 --max-time 15 \
-H "Authorization: Basic ${AUTH}" \
-H "Content-Type: application/json" \
"${JIRA_BASE_URL}/rest/api/3/myself")
```

- **HTTP 200** → credentials valid, proceed
- **HTTP 401** → credentials missing or invalid. Only now inform the user:
- Check if `JIRA_API_TOKEN` and `JIRA_EMAIL` are configured as Ambient session secrets
- If not, generate a token at https://id.atlassian.com/manage-profile/security/api-tokens and export:
```bash
export JIRA_API_TOKEN="your-token-here"
export JIRA_EMAIL="your-email@redhat.com"
```
- **HTTP 403** → token valid but insufficient permissions — inform user
- **Other / timeout** → network issue — inform user and retry once

**Do NOT pre-check env vars with `[ -z "$JIRA_API_TOKEN" ]` and stop.** The variables may be available to the API call even if not visible to the shell check (e.g. Ambient secrets injection).
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

What is the recommended Jira REST API v3 endpoint for testing authentication credentials, and what HTTP status codes does it return for valid credentials, invalid credentials, and insufficient permissions?

💡 Result:

The recommended Jira REST API v3 endpoint for testing authentication credentials is GET /rest/api/3/myself. This endpoint returns details for the current authenticated user and requires only "Permission to access Jira". HTTP status codes: - Valid credentials: 200 OK - Invalid credentials: 401 Unauthorized - Insufficient permissions: This endpoint has minimal permissions required (access Jira), so insufficient permissions would also return 401 Unauthorized if the user lacks basic access. Alternative endpoint for permission checks: GET /rest/api/3/mypermissions, which returns the user's permissions and can be used to verify more granular access. It returns 200 OK for valid authenticated users. Note: There is no dedicated "health check" or ping endpoint solely for auth testing in official docs; /myself is commonly used as it confirms successful authentication and basic access.

Citations:


Reconsider the HTTP 403 status code—/rest/api/3/myself returns 401 for all authentication failures.

The endpoint is correct for credentials validation. However, the /rest/api/3/myself endpoint returns HTTP 401 for both missing/invalid credentials and insufficient permissions. The current code handles HTTP 403 as a separate "insufficient permissions" case, but this endpoint does not return 403 per official Jira REST API v3 documentation.

Consolidate error handling to treat 401 responses uniformly, or verify that a different endpoint is intended for the 403 case.

🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 72-72: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


[warning] 75-75: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 58 - 79, The
curl healthcheck using JIRA_BASE_URL and TEST_RESPONSE should not treat HTTP 403
as "insufficient permissions" because /rest/api/3/myself returns 401 for both
auth failures and permission issues; update the logic that branches on
TEST_RESPONSE to consolidate 401 handling (log both missing/invalid creds and
insufficient permissions together and instruct the user to check API token/email
and permissions) and remove or disable the separate 403 branch, or alternatively
replace the endpoint with a permissions-specific endpoint if you really need to
distinguish 403; keep the existing note about not pre-checking env vars (do not
add a [ -z "$JIRA_API_TOKEN" ] check).

Comment on lines +58 to +77
```bash
JIRA_BASE_URL="https://redhat.atlassian.net"
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
TEST_RESPONSE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \
--connect-timeout 10 --max-time 15 \
-H "Authorization: Basic ${AUTH}" \
-H "Content-Type: application/json" \
"${JIRA_BASE_URL}/rest/api/3/myself")
```

- **HTTP 200** → credentials valid, proceed
- **HTTP 401** → credentials missing or invalid. Only now inform the user:
- Check if `JIRA_API_TOKEN` and `JIRA_EMAIL` are configured as Ambient session secrets
- If not, generate a token at https://id.atlassian.com/manage-profile/security/api-tokens and export:
```bash
export JIRA_API_TOKEN="your-token-here"
export JIRA_EMAIL="your-email@redhat.com"
```
- **HTTP 403** → token valid but insufficient permissions — inform user
- **Other / timeout** → network issue — inform user and retry once
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Implement retry logic for timeout handling.

Line 77 documents that timeout/network errors should "retry once," but the test API call in lines 58-66 lacks retry flags. This creates a documentation-implementation gap.

🔄 Proposed fix to add retry logic
 TEST_RESPONSE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \
   --connect-timeout 10 --max-time 15 \
+  --retry 1 \
+  --retry-delay 2 \
+  --retry-connrefused \
   -H "Authorization: Basic ${AUTH}" \
   -H "Content-Type: application/json" \
   "${JIRA_BASE_URL}/rest/api/3/myself")
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 72-72: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


[warning] 75-75: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 58 - 77, The
documentation says to "retry once" on network/timeout but the TEST_RESPONSE curl
call lacks retry options; update the curl invocation that sets TEST_RESPONSE
(the command using JIRA_BASE_URL and AUTH) to perform a single retry on
transient failures by adding curl retry flags (e.g., --retry 1 plus an
appropriate --retry-delay and --retry-connrefused) while preserving existing
timeouts (--connect-timeout, --max-time) and the exit-code capture (-s -o
/dev/null -w "%{http_code}"); ensure the adjusted curl still returns an HTTP
status code into TEST_RESPONSE so the subsequent HTTP 200/401/403/Other handling
continues to work.

Comment on lines +69 to +75
- **HTTP 401** → credentials missing or invalid. Only now inform the user:
- Check if `JIRA_API_TOKEN` and `JIRA_EMAIL` are configured as Ambient session secrets
- If not, generate a token at https://id.atlassian.com/manage-profile/security/api-tokens and export:
```bash
export JIRA_API_TOKEN="your-token-here"
export JIRA_EMAIL="your-email@redhat.com"
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Add blank lines around fenced code blocks for markdown compliance.

Markdownlint reports missing blank lines around the code block per MD031. Add a blank line before line 72 and after line 75 to improve formatting consistency.

📝 Proposed formatting fix
   - **HTTP 401** → credentials missing or invalid. Only now inform the user:
     - Check if `JIRA_API_TOKEN` and `JIRA_EMAIL` are configured as Ambient session secrets
     - If not, generate a token at https://id.atlassian.com/manage-profile/security/api-tokens and export:
+
       ```bash
       export JIRA_API_TOKEN="your-token-here"
       export JIRA_EMAIL="your-email@redhat.com"
       ```
+
   - **HTTP 403** → token valid but insufficient permissions — inform user
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 72-72: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


[warning] 75-75: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 69 - 75, The
fenced code block starting with "```bash" containing the export lines currently
lacks surrounding blank lines (MD031); add one blank line immediately before the
opening "```bash" and one blank line immediately after the closing "```" so the
code block is separated from the surrounding list text (update the block in
workflows/cve-fixer/.claude/commands/cve.find.md where the export
JIRA_API_TOKEN/JIRA_EMAIL lines appear).

Comment on lines +83 to +87
a. Set up variables (AUTH already set from Step 2):
```bash
COMPONENT_NAME="[from step 1]"
JIRA_BASE_URL="https://redhat.atlassian.net"
JIRA_EMAIL="${JIRA_EMAIL}"
JIRA_API_TOKEN="${JIRA_API_TOKEN}"
# Jira Cloud uses Basic Auth: base64(email:api-token)
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
# AUTH already constructed in Step 2 — reuse it
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Add blank line before fenced code block for markdown compliance.

Markdownlint reports a missing blank line before the code block at line 84 per MD031.

📝 Proposed formatting fix
 3. **Query Jira for CVE Issues**
 
    a. Set up variables (AUTH already set from Step 2):
+
    ```bash
    COMPONENT_NAME="[from step 1]"
    JIRA_BASE_URL="https://redhat.atlassian.net"
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 84-84: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 83 - 87,
Insert a single blank line before the fenced code block in the "a. Set up
variables (AUTH already set from Step 2):" list item so the markdown has a blank
line immediately before the ```bash fence (fixing MD031). Locate the fenced
block in workflows/cve-fixer/.claude/commands/cve.find.md under the
"COMPONENT_NAME" setup section and add the blank line above the ```bash line to
make the block compliant.

…access repos

- Recommend Classic PAT with repo scope over fine-grained PAT or
  Ambient Code GitHub App (neither works reliably for cross-org repos
  like llm-d/*, eval-hub/*, trustyai-explainability/*)
- Add write access check: use gh api repos/X --jq '.permissions.push'
  before attempting to push
- Add fork fallback: if no write access, create fork, push fix branch
  to fork, and open PR targeting the original repo with head: user:branch
  This handles all upstream external org repos automatically

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (4)
workflows/cve-fixer/.claude/commands/cve.fix.md (4)

153-160: ⚠️ Potential issue | 🟠 Major

Deduplicate TARGET_BRANCHES before processing to prevent duplicate PR attempts.

If ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH, this list can duplicate targets and cause redundant PR/push attempts.

Suggested doc fix
 if [ "$REPO_TYPE" = "downstream" ]; then
   TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
 else
   TARGET_BRANCHES=("$DEFAULT_BRANCH")
 fi
+
+# Deduplicate while preserving order
+UNIQUE_TARGET_BRANCHES=()
+for b in "${TARGET_BRANCHES[@]}"; do
+  [ -z "$b" ] && continue
+  [[ " ${UNIQUE_TARGET_BRANCHES[*]} " == *" $b "* ]] || UNIQUE_TARGET_BRANCHES+=("$b")
+done
+TARGET_BRANCHES=("${UNIQUE_TARGET_BRANCHES[@]}")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 153 - 160, The
TARGET_BRANCHES assignment can produce duplicates when ACTIVE_RELEASE_BRANCHES
includes DEFAULT_BRANCH; update the logic that builds TARGET_BRANCHES (the block
using REPO_TYPE, DEFAULT_BRANCH and ACTIVE_RELEASE_BRANCHES) to deduplicate
entries before further processing — for example, after composing
TARGET_BRANCHES, iterate and rebuild a new array using a seen set (declare -A
seen) to skip already-added branch names so each branch appears only once;
ensure downstream consumers of TARGET_BRANCHES use the deduplicated array.

162-173: ⚠️ Potential issue | 🟡 Minor

Fix fenced code block formatting for markdownlint compliance (MD031/MD040).

This fence is missing a language identifier and blank-line framing.

Suggested doc fix
-   ```
+   
+   ```text
    upstream  llm-d/llm-d-inference-scheduler          → PR against: main
    midstream opendatahub-io/llm-d-inference-scheduler  → PR against: main
    downstream red-hat-data-services/llm-d-inference-scheduler → PRs against:
      - main
      - rhoai-3.3
      - rhoai-3.4
      - rhoai-3.4-ea.1
      - rhoai-3.4-ea.2
    ```
+
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 162 - 173, The
fenced code block for the "llm-d inference-scheduler" example is missing a
language identifier and blank-line framing; update the markdown by adding a
blank line before and after the triple-backtick fence and include a language
token (e.g., "text") after the opening ``` so the block around the "upstream
llm-d/llm-d-inference-scheduler ..." example is fenced as ```text ... ``` to
satisfy MD031/MD040.

206-207: ⚠️ Potential issue | 🟠 Major

Single-clone branch loop still contradicts per-target isolated execution.

Current guidance still uses one clone + checkout loop, which risks state leakage across target branches and conflicts with the PR objective of independent clone→fix→test→PR per branch.

Suggested doc fix
-REPO_DIR="/tmp/${REPO_ORG}/${REPO_NAME}"
+REPO_DIR_BASE="/tmp/${REPO_ORG}"
...
-Steps 5–11 run in a loop over `TARGET_BRANCHES` — for each branch:
-- `git checkout <branch>` and `git pull` to ensure it is up to date
-- Apply fix and create PR targeting that branch (from fork if no direct push access)
+Steps 5–11 run independently per `TARGET_BRANCH` — for each branch:
+- Create branch-specific workspace: `REPO_DIR="${REPO_DIR_BASE}/${REPO_NAME}-${TARGET_BRANCH}"`
+- Clone/fetch in that workspace
+- Checkout the target branch in that workspace
+- Apply fix, run tests, and create PR for that target branch only

Also applies to: 239-243, 246-258

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 206 - 207, The
workflow currently sets a single REPO_DIR and then iterates branches using
checkout loops, which causes state leakage; modify the flow so each target
branch performs an independent clone into its own temporary directory (create a
fresh REPO_DIR per-target inside the per-target loop), perform git checkout, run
the fix/test/PR sequence, and then clean up that temp clone; update mentions of
REPO_DIR, the per-target loop, and any checkout logic to reflect per-target
isolated clone→checkout→fix→test→PR execution and ensure cleanup after each
target.

174-178: ⚠️ Potential issue | 🟠 Major

Branch naming guidance is still inconsistent with canonical naming section.

These lines correctly include <target-branch> in examples, but the canonical pattern later still omits it (fix/cve-...-<package>-attempt-1), which can collide across bases.

Suggested doc fix
-- Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-attempt-1`
+- Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-<target-branch>-attempt-1`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 174 - 178,
Update the canonical branch-naming pattern to include the <target-branch> token
so it matches the earlier examples; specifically change the canonical example
`fix/cve-...-<package>-attempt-1` to the consistent pattern used above
`fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-1` (or equivalent ordering
used in the Multi-repo + multi-branch examples) and update any other occurrences
in the document so all examples and the canonical section use the same
`<target-branch>`-inclusive format to avoid cross-branch collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 153-160: The TARGET_BRANCHES assignment can produce duplicates
when ACTIVE_RELEASE_BRANCHES includes DEFAULT_BRANCH; update the logic that
builds TARGET_BRANCHES (the block using REPO_TYPE, DEFAULT_BRANCH and
ACTIVE_RELEASE_BRANCHES) to deduplicate entries before further processing — for
example, after composing TARGET_BRANCHES, iterate and rebuild a new array using
a seen set (declare -A seen) to skip already-added branch names so each branch
appears only once; ensure downstream consumers of TARGET_BRANCHES use the
deduplicated array.
- Around line 162-173: The fenced code block for the "llm-d inference-scheduler"
example is missing a language identifier and blank-line framing; update the
markdown by adding a blank line before and after the triple-backtick fence and
include a language token (e.g., "text") after the opening ``` so the block
around the "upstream llm-d/llm-d-inference-scheduler ..." example is fenced as
```text ... ``` to satisfy MD031/MD040.
- Around line 206-207: The workflow currently sets a single REPO_DIR and then
iterates branches using checkout loops, which causes state leakage; modify the
flow so each target branch performs an independent clone into its own temporary
directory (create a fresh REPO_DIR per-target inside the per-target loop),
perform git checkout, run the fix/test/PR sequence, and then clean up that temp
clone; update mentions of REPO_DIR, the per-target loop, and any checkout logic
to reflect per-target isolated clone→checkout→fix→test→PR execution and ensure
cleanup after each target.
- Around line 174-178: Update the canonical branch-naming pattern to include the
<target-branch> token so it matches the earlier examples; specifically change
the canonical example `fix/cve-...-<package>-attempt-1` to the consistent
pattern used above `fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-1` (or
equivalent ordering used in the Multi-repo + multi-branch examples) and update
any other occurrences in the document so all examples and the canonical section
use the same `<target-branch>`-inclusive format to avoid cross-branch
collisions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c2120c28-59ea-4e75-8e7d-6748971ae08e

📥 Commits

Reviewing files that changed from the base of the PR and between 340b108 and 177e6d0.

📒 Files selected for processing (1)
  • workflows/cve-fixer/.claude/commands/cve.fix.md

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.

1 participant