Scale up agent deployments

[nforro]

No description provided.

[qodo-for-packit]

Code Review by Qodo

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

1. Shared checkout directory race ✓ Resolved 🐞 Bug ☼ Reliability

Description

Scaling replicas increases parallel workers against the shared /git-repos PVC, but ymir’s clone
logic deletes and recreates deterministic directories under GIT_REPO_BASEPATH, so concurrent runs
for the same key can rm -rf each other’s checkout and fail or produce incorrect results. The risk is
amplified because agent locks are per-agent-type prefixes, so different agent types can operate on
the same Jira issue without mutual exclusion while still sharing the same /git-repos/<jira_issue>
path.

Code

openshift/deployment-backport-agent-c10s.yml[7]

+  replicas: 4

Relevance

●●● Strong

Accepted similar checkout/clone concurrency race mitigations when increasing parallelism/scaling
workers.

PR-#657
PR-#73
PR-#731

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The replica increase raises the number of pods concurrently using the same mounted PVC at
/git-repos; the worker code uses deterministic on-disk clone paths and deletes them before cloning,
and agent-type-specific locks don’t provide cross-agent mutual exclusion. Together, this makes
checkout clobbering possible under higher concurrency introduced by this PR’s scaling.

openshift/deployment-backport-agent-c10s.yml[7-7]
openshift/deployment-backport-agent-c10s.yml[57-77]
openshift/pvc-mcp-server-git-repos.yml[10-16]
openshift/configmap-agents-env.yml[3-6]
ymir/agents/tasks.py[234-244]
ymir/agents/tasks.py[312-316]
ymir/agents/rebase_agent.py[447-472]
ymir/agents/backport_agent.py[855-885]
PR-#657

Agent prompt

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

## Issue description
Scaling agent deployments increases concurrency, but repository clone directories are keyed only by Jira issue (or MR URL path) and are aggressively deleted before use. This can cause concurrent workers to delete/overwrite each other’s workspaces on the shared /git-repos PVC.

## Issue Context
- All agents mount the same RWX PVC at `/git-repos`.
- `fork_and_prepare_dist_git()` uses `/git-repos/<jira_issue>` and removes it when present.
- Agent locking uses different Redis lock prefixes per agent type (e.g., `lock:rebase:` vs `lock:backport:`), so cross-agent concurrent processing of the same Jira issue is not prevented even though they share the same on-disk path.

## Fix Focus Areas
- ymir/agents/tasks.py[234-277]
- ymir/agents/tasks.py[312-335]
- ymir/agents/rebase_agent.py[447-477]
- ymir/agents/backport_agent.py[855-886]
- openshift/deployment-backport-agent-c10s.yml[7-7]

### Concrete fix options
1) Make working directories unique per task execution (recommended):
  - Include agent type + a unique ID (task id / timestamp / random suffix) in the directory name, e.g. `/git-repos/<jira_issue>/<agent_type>/<uuid>`.
  - Avoid deleting a shared parent dir; only delete the per-task directory.
2) Add a shared lock for filesystem workspace keyed by the on-disk path (e.g., `lock:workspace:<jira_issue>`) used by all agent types before deleting/cloning.
3) If neither is feasible immediately, reduce replicas back to previous values until the workspace isolation/locking is implemented.

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

---

2. agent_type path escape 🐞 Bug ⛨ Security ⭐ New

Description

fork_and_prepare_dist_git() now incorporates agent_type into working_dir and then deletes that
directory via _force_rmtree(), but it does not validate agent_type. If a caller ever passes an
absolute path (or other unsafe value), Path(base) / agent_type can escape GIT_REPO_BASEPATH, and
the subsequent rm -rf can recursively delete outside the shared repo volume.

Code

ymir/agents/tasks.py[R237-240]

+    # Scoped by agent_type so different agent types processing the same
+    # jira_issue concurrently (e.g. rebase and backport) never share a
+    # working directory and can't rm -rf each other's checkout.
+    working_dir = Path(os.environ["GIT_REPO_BASEPATH"]) / agent_type / jira_issue

Relevance

●●● Strong

Team previously accepted validating path components under GIT_REPO_BASEPATH before rmtree/rm -rf to
prevent escapes.

PR-#571
PR-#670

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code constructs working_dir from GIT_REPO_BASEPATH / agent_type / jira_issue (without
validating agent_type) and conditionally deletes it with _force_rmtree(), which executes `rm
-rf`. Call sites added in this PR pass constant strings, demonstrating the current usage pattern but
not providing safety if future callers pass an unsafe value.

ymir/agents/tasks.py[208-243]
ymir/agents/backport_agent.py[361-377]
ymir/agents/rebase_agent.py[189-203]
ymir/agents/rebuild_agent.py[101-114]
ymir/agents/mr_consolidation_agent.py[506-520]
PR-#571

Agent prompt

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

### Issue description
`fork_and_prepare_dist_git()` validates `jira_issue` but does not validate the newly-added `agent_type` before using it to build `working_dir`. Because `_force_rmtree()` runs `rm -rf` on `working_dir`, an unsafe `agent_type` (notably an absolute path) could cause deletion outside `GIT_REPO_BASEPATH`.

### Issue Context
Current callers shown in this PR pass hard-coded strings (e.g. `"Rebase"`, `"Backport"`), so immediate exploitability is limited; the main risk is future callers/refactors accidentally passing an unsafe value.

### Fix Focus Areas
- ymir/agents/tasks.py[231-243]

### Implementation notes
- Add a guard for `agent_type` similar to `jira_issue` (reject empty, absolute paths, and traversal patterns).
- Prefer an allowlist/Enum of known agent types (Backport/Rebase/Rebuild/MRConsolidation/Triage/Applicability/etc.) to prevent accidental misuse.
- (Optional hardening) Resolve `basepath = Path(os.environ["GIT_REPO_BASEPATH"]).resolve()` and verify `working_dir.resolve().is_relative_to(basepath)` **and** `working_dir.resolve() != basepath` before calling `_force_rmtree()`.

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

---

Context

✅ Compliance rules (platform): 7 rules

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous review results

Review updated until commit 13556ff

Results up to commit 12a23bb ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Shared checkout directory race ✓ Resolved 🐞 Bug ☼ Reliability

Description

Scaling replicas increases parallel workers against the shared /git-repos PVC, but ymir’s clone
logic deletes and recreates deterministic directories under GIT_REPO_BASEPATH, so concurrent runs
for the same key can rm -rf each other’s checkout and fail or produce incorrect results. The risk is
amplified because agent locks are per-agent-type prefixes, so different agent types can operate on
the same Jira issue without mutual exclusion while still sharing the same /git-repos/<jira_issue>
path.

Code

openshift/deployment-backport-agent-c10s.yml[7]

+  replicas: 4

Relevance

●●● Strong

Accepted similar checkout/clone concurrency race mitigations when increasing parallelism/scaling
workers.

PR-#657
PR-#73
PR-#731

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The replica increase raises the number of pods concurrently using the same mounted PVC at
/git-repos; the worker code uses deterministic on-disk clone paths and deletes them before cloning,
and agent-type-specific locks don’t provide cross-agent mutual exclusion. Together, this makes
checkout clobbering possible under higher concurrency introduced by this PR’s scaling.

openshift/deployment-backport-agent-c10s.yml[7-7]
openshift/deployment-backport-agent-c10s.yml[57-77]
openshift/pvc-mcp-server-git-repos.yml[10-16]
openshift/configmap-agents-env.yml[3-6]
ymir/agents/tasks.py[234-244]
ymir/agents/tasks.py[312-316]
ymir/agents/rebase_agent.py[447-472]
ymir/agents/backport_agent.py[855-885]
PR-#657

Agent prompt

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

## Issue description
Scaling agent deployments increases concurrency, but repository clone directories are keyed only by Jira issue (or MR URL path) and are aggressively deleted before use. This can cause concurrent workers to delete/overwrite each other’s workspaces on the shared /git-repos PVC.

## Issue Context
- All agents mount the same RWX PVC at `/git-repos`.
- `fork_and_prepare_dist_git()` uses `/git-repos/<jira_issue>` and removes it when present.
- Agent locking uses different Redis lock prefixes per agent type (e.g., `lock:rebase:` vs `lock:backport:`), so cross-agent concurrent processing of the same Jira issue is not prevented even though they share the same on-disk path.

## Fix Focus Areas
- ymir/agents/tasks.py[234-277]
- ymir/agents/tasks.py[312-335]
- ymir/agents/rebase_agent.py[447-477]
- ymir/agents/backport_agent.py[855-886]
- openshift/deployment-backport-agent-c10s.yml[7-7]

### Concrete fix options
1) Make working directories unique per task execution (recommended):
  - Include agent type + a unique ID (task id / timestamp / random suffix) in the directory name, e.g. `/git-repos/<jira_issue>/<agent_type>/<uuid>`.
  - Avoid deleting a shared parent dir; only delete the per-task directory.
2) Add a shared lock for filesystem workspace keyed by the on-disk path (e.g., `lock:workspace:<jira_issue>`) used by all agent types before deleting/cloning.
3) If neither is feasible immediately, reduce replicas back to previous values until the workspace isolation/locking is implemented.

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

---

[qodo-for-packit]

PR Summary by Qodo

Scale up OpenShift agent deployments (replicas + CPU limits)

⚙️ Configuration changes ✨ Enhancement 🕐 20-40 Minutes

AI Description

• Increase replica counts for agent Deployments to improve throughput and resilience.
• Add explicit CPU limits (300m) to agent containers for predictable scheduling.

Diagram

graph TD
  manifests["Agent deployment manifests"] --> ocp["OpenShift API"] --> deploys["Agent Deployments"] --> pods["Pods (more replicas)"]
  deploys --> replicas["Replica count up"]
  deploys --> limits["CPU limit 300m"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use Kustomize overlays / Helm values for shared agent settings

• ➕ Avoids repeating identical resource/replica knobs across many manifests
• ➕ Makes future fleet-wide tuning a single change
• ➖ Introduces/expands templating tooling and conventions
• ➖ May be out-of-scope if repo intentionally keeps plain manifests

2. Define a namespace-wide LimitRange for default CPU limits

• ➕ Centralizes resource limit policy enforcement
• ➕ Reduces per-Deployment boilerplate
• ➖ Less explicit at the workload level
• ➖ May unintentionally affect other workloads in the namespace

Recommendation: The PR’s direct per-Deployment edits are pragmatic and low-risk for an immediate scale-up. If these agents are tuned frequently or expected to grow, consider a follow-up to centralize common knobs (replicas/resources) via Kustomize/Helm (preferred for explicit, workload-scoped configuration) to reduce duplication and drift.

Files changed (9) +18 / -9

Other (9) +18 / -9

deployment-backport-agent-c10s.ymlScale backport agent (c10s) and add CPU limit +2/-1

Scale backport agent (c10s) and add CPU limit

• Increases Deployment replicas from 2 to 4. Adds a container CPU limit of 300m under resources.limits.

openshift/deployment-backport-agent-c10s.yml

deployment-backport-agent-c9s.ymlScale backport agent (c9s) and add CPU limit +2/-1

Scale backport agent (c9s) and add CPU limit

• Increases Deployment replicas from 2 to 4. Adds a container CPU limit of 300m under resources.limits.

openshift/deployment-backport-agent-c9s.yml

deployment-mr-consolidation-agent-c10s.ymlScale MR consolidation agent (c10s) and add CPU limit +2/-1

Scale MR consolidation agent (c10s) and add CPU limit

• Increases Deployment replicas from 1 to 2. Adds a container CPU limit of 300m under resources.limits.

openshift/deployment-mr-consolidation-agent-c10s.yml

deployment-mr-consolidation-agent-c9s.ymlScale MR consolidation agent (c9s) and add CPU limit +2/-1

Scale MR consolidation agent (c9s) and add CPU limit

• Increases Deployment replicas from 1 to 2. Adds a container CPU limit of 300m under resources.limits.

openshift/deployment-mr-consolidation-agent-c9s.yml

deployment-rebase-agent-c10s.ymlScale rebase agent (c10s) and add CPU limit +2/-1

Scale rebase agent (c10s) and add CPU limit

• Increases Deployment replicas from 1 to 2. Adds a container CPU limit of 300m under resources.limits.

openshift/deployment-rebase-agent-c10s.yml

deployment-rebase-agent-c9s.ymlScale rebase agent (c9s) and add CPU limit +2/-1

Scale rebase agent (c9s) and add CPU limit

• Increases Deployment replicas from 1 to 2. Adds a container CPU limit of 300m under resources.limits.

openshift/deployment-rebase-agent-c9s.yml

deployment-rebuild-agent-c10s.ymlScale rebuild agent (c10s) and add CPU limit +2/-1

Scale rebuild agent (c10s) and add CPU limit

• Increases Deployment replicas from 1 to 2. Adds a container CPU limit of 300m under resources.limits.

openshift/deployment-rebuild-agent-c10s.yml

deployment-rebuild-agent-c9s.ymlScale rebuild agent (c9s) and add CPU limit +2/-1

Scale rebuild agent (c9s) and add CPU limit

• Increases Deployment replicas from 1 to 2. Adds a container CPU limit of 300m under resources.limits.

openshift/deployment-rebuild-agent-c9s.yml

deployment-triage-agent.ymlScale triage agent and add CPU limit +2/-1

Scale triage agent and add CPU limit

• Increases Deployment replicas from 1 to 2. Adds a container CPU limit of 300m under resources.limits.

openshift/deployment-triage-agent.yml

[nforro]

/agentic_review

[qodo-for-packit]

Code review by qodo was updated up to the latest commit 530718e

[lbarcziova]

thanks!

[lbarcziova]

@TomasKorbar just a headsup about this as I saw a method for removing the directory in the #610