diff --git a/.github/workflows/check-in-container.yml b/.github/workflows/check-in-container.yml index 74e5ccca4..4dd11188a 100644 --- a/.github/workflows/check-in-container.yml +++ b/.github/workflows/check-in-container.yml @@ -35,6 +35,7 @@ jobs: - target: check-jira-issue-fetcher-in-container - target: check-ymir-common-in-container - target: check-supervisor-in-container + - target: check-sweep-in-container - target: check-mcp-install-in-container steps: - name: Checkout code @@ -63,3 +64,29 @@ jobs: - name: Test unprivileged gateway module run: | podman run --rm mcp-gateway-test:pr python3.13 -c "from ymir.tools.unprivileged.gateway import main; print('✓ unprivileged gateway import successful')" + + smoke-test-sweep-containerfile: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + - name: Set up Podman + run: | + sudo apt-get update + sudo apt-get install -y podman + podman --version + - name: Build sweep container + run: | + podman build --no-cache -f Containerfile.sweep -t sweep-test:pr . + # Containerfile.sweep copies only a curated subset of ymir/* (sweep, common, + # supervisor, tools) onto PYTHONPATH rather than pip-installing the package. + # The slim image's import graph is therefore an unenforced contract: if a + # package-structure change makes the sweep's imports reach outside that + # subset (or pull in a dep this Containerfile doesn't install), the image + # breaks only at runtime. These imports turn that into a build-time failure. + - name: Test sweep strategy imports + run: | + podman run --rm sweep-test:pr python3 -c "from ymir.sweep.y_stream import YStreamSweep; from ymir.tools.privileged.jira import CheckCveTriageEligibilityTool; print('✓ sweep strategy + eligibility tool import successful')" + - name: Test sweep CLI entrypoint + run: | + podman run --rm sweep-test:pr python3 -m ymir.sweep --help diff --git a/Containerfile.sweep b/Containerfile.sweep new file mode 100644 index 000000000..f7ac781b4 --- /dev/null +++ b/Containerfile.sweep @@ -0,0 +1,57 @@ +FROM quay.io/centos/centos:stream10 + +RUN dnf -y install epel-release \ + && dnf config-manager --set-enabled crb \ + && dnf -y update + +RUN dnf -y install --allowerasing \ + # Build dependencies (removed after pip install) + gcc \ + gcc-c++ \ + python3-devel \ + # Runtime system packages + python3 \ + python3-backoff \ + python3-koji \ + python3-pip \ + python3-requests +RUN pip3 install -v --no-cache-dir \ + "litellm!=1.82.7,!=1.82.8" \ + "beeai-framework==0.1.82" \ + "mcp<1.29.0" \ + pydantic \ + aiofiles \ + aiohttp \ + httpx \ + jinja2 \ + redis \ + specfile \ + && dnf -y remove gcc gcc-c++ python3-devel \ + && dnf clean all + +# Create unprivileged user +RUN useradd -m -G wheel sweepbot + +WORKDIR /home/sweepbot + +# Copy only the modules required by the sweep. +# ymir.sweep — the sweep module itself +# ymir.common — shared utilities, models, constants, config +# ymir.supervisor — jira_utils, gitlab_utils, http_utils, supervisor_types +# ymir.tools — tools constants (YMIR_USER_AGENT, imported by http_utils) and +# ymir.tools.privileged.jira (CheckCveTriageEligibilityTool, which +# the y_stream sweep re-runs to decide unblocking) +COPY ymir/sweep/ /home/sweepbot/ymir/sweep/ +COPY ymir/common/ /home/sweepbot/ymir/common/ +COPY ymir/supervisor/ /home/sweepbot/ymir/supervisor/ +COPY ymir/tools/ /home/sweepbot/ymir/tools/ + +RUN chgrp -R root /home/sweepbot && chmod -R g+rwX /home/sweepbot + +USER sweepbot +ENV HOME=/home/sweepbot +WORKDIR $HOME + +ENV PYTHONPATH=$HOME:$PYTHONPATH + +CMD ["python3", "-m", "ymir.sweep", "--all"] diff --git a/Makefile b/Makefile index 3f3b9c94e..cef2f478f 100644 --- a/Makefile +++ b/Makefile @@ -254,6 +254,24 @@ build-jira-issue-fetcher: build-mr-cleanup: $(COMPOSE) --profile manual build mr-cleanup +.PHONY: build-sweep +build-sweep: + $(CONTAINER_TOOL) build -f Containerfile.sweep -t sweep:latest . + +.PHONY: run-sweep +run-sweep: + @if [ -z "$(STRATEGY)" ]; then \ + echo "Usage: make run-sweep STRATEGY=dependency|y_stream|pr_pending|no_patch"; \ + exit 1; \ + fi + @if [ ! -f .secrets/sweep.env ]; then \ + echo "Error: .secrets/sweep.env not found"; \ + echo "Copy the template: cp templates/sweep.env .secrets/sweep.env"; \ + echo "Then edit it with your credentials"; \ + exit 1; \ + fi + $(COMPOSE) -f $(COMPOSE_FILE) --profile manual run --rm sweep python3 -m ymir.sweep --strategy $(STRATEGY) + # Usage: # make run-mr-cleanup-dry-run # dry run, lists what would be changed # make run-mr-cleanup # live run @@ -462,7 +480,7 @@ redis-cli: build-test-image: $(MAKE) -f Makefile.tests build-test-image -.PHONY: check-in-container check-agents-in-container check-unprivileged-tools-in-container check-privileged-tools-in-container check-jira-issue-fetcher-in-container check-ymir-common-in-container check-supervisor-in-container check-mcp-install-in-container check-cli-in-container +.PHONY: check-in-container check-agents-in-container check-unprivileged-tools-in-container check-privileged-tools-in-container check-jira-issue-fetcher-in-container check-ymir-common-in-container check-supervisor-in-container check-mcp-install-in-container check-cli-in-container check-sweep-in-container check-sweep-integration-in-container check-in-container: build-test-image $(MAKE) -f Makefile.tests check-in-container check-agents-in-container: build-test-image @@ -477,6 +495,10 @@ check-ymir-common-in-container: build-test-image $(MAKE) -f Makefile.tests check-ymir-common-in-container check-supervisor-in-container: build-test-image $(MAKE) -f Makefile.tests check-supervisor-in-container +check-sweep-in-container: build-test-image + $(MAKE) -f Makefile.tests check-sweep-in-container +check-sweep-integration-in-container: build-test-image + $(MAKE) -f Makefile.tests check-sweep-integration-in-container check-mcp-install-in-container: build-test-image $(MAKE) -f Makefile.tests check-mcp-install-in-container check-cli-in-container: build-test-image diff --git a/Makefile.tests b/Makefile.tests index eaa5adc94..fe68e3565 100644 --- a/Makefile.tests +++ b/Makefile.tests @@ -16,10 +16,11 @@ build-test-image-c9s: $(CONTAINER_ENGINE) build --rm --tag $(TEST_IMAGE_C9S) -f Containerfile.c9s-tests .PHONY: check check-agents check-cli check-unprivileged-tools check-privileged-tools check-jira-issue-fetcher check-ymir-common \ - check-supervisor check-mcp-install check-in-container check-agents-in-container check-cli-in-container \ + check-supervisor check-mcp-install check-sweep check-sweep-integration \ + check-in-container check-agents-in-container check-cli-in-container \ check-unprivileged-tools-in-container \ check-privileged-tools-in-container check-jira-issue-fetcher-in-container check-ymir-common-in-container \ - check-supervisor-in-container check-mcp-install-in-container + check-supervisor-in-container check-mcp-install-in-container check-sweep-in-container check-sweep-integration-in-container define RUN_TESTS PYTHONPATH=$(CURDIR) PYTHONDONTWRITEBYTECODE=1 python3 -m pytest --verbose --showlocals $(addprefix $(1),$(TEST_TARGET)) @@ -39,11 +40,16 @@ check-ymir-common: $(call RUN_TESTS,ymir/common/) check-supervisor: $(call RUN_TESTS,ymir/supervisor/) +check-sweep: + $(call RUN_TESTS,ymir/sweep/) +check-sweep-integration: TEST_TARGET = ./tests/integration +check-sweep-integration: + $(call RUN_TESTS,ymir/sweep/) check-mcp-install: PYTHONPATH= bash scripts/test_mcp_install.sh -check: check-agents check-cli check-unprivileged-tools check-privileged-tools check-jira-issue-fetcher check-ymir-common check-supervisor check-mcp-install +check: check-agents check-cli check-unprivileged-tools check-privileged-tools check-jira-issue-fetcher check-ymir-common check-supervisor check-sweep check-mcp-install define RUN_TESTS_IN_CONTAINER $(CONTAINER_ENGINE) run --rm -it -v $(CURDIR):/src:z --env TEST_TARGET $(1) make -f Makefile.tests $(2) @@ -63,10 +69,15 @@ check-ymir-common-in-container: $(call RUN_TESTS_IN_CONTAINER,$(TEST_IMAGE_C9S),check-ymir-common) check-supervisor-in-container: $(call RUN_TESTS_IN_CONTAINER,$(TEST_IMAGE_C9S),check-supervisor) +check-sweep-in-container: + $(call RUN_TESTS_IN_CONTAINER,$(TEST_IMAGE),check-sweep) +check-sweep-integration-in-container: + $(call RUN_TESTS_IN_CONTAINER,$(TEST_IMAGE),check-sweep-integration) check-mcp-install-in-container: $(call RUN_TESTS_IN_CONTAINER,$(TEST_IMAGE),check-mcp-install) check-in-container: check-agents-in-container check-cli-in-container check-unprivileged-tools-in-container \ check-privileged-tools-in-container \ check-jira-issue-fetcher-in-container check-ymir-common-in-container check-supervisor-in-container \ + check-sweep-in-container \ check-mcp-install-in-container diff --git a/compose.yaml b/compose.yaml index 8dbc2d003..dd6e8177e 100644 --- a/compose.yaml +++ b/compose.yaml @@ -451,6 +451,18 @@ services: restart: "no" profiles: ["manual"] + sweep: + image: sweep + build: + context: . + dockerfile: Containerfile.sweep + environment: + - REDIS_URL=redis://valkey:6379/0 + env_file: + - .secrets/sweep.env + restart: "no" + profiles: ["manual"] + volumes: valkey-data: diff --git a/openshift/Makefile b/openshift/Makefile index d28590d31..1b4901b2f 100644 --- a/openshift/Makefile +++ b/openshift/Makefile @@ -170,6 +170,34 @@ print(json.dumps({ \ oc exec deployment/valkey -- valkey-cli LPUSH $$queue "$$payload" && \ echo "✓ Pushed to $$queue. View with: make show-reproducer-queue" +run-sweep-dependency: + oc delete job sweep-dependency-manual --ignore-not-found + oc create job sweep-dependency-manual --from=cronjob/sweep-dependency + +run-sweep-y-stream: + oc delete job sweep-y-stream-manual --ignore-not-found + oc create job sweep-y-stream-manual --from=cronjob/sweep-y-stream + +run-sweep-pr-pending: + oc delete job sweep-pr-pending-manual --ignore-not-found + oc create job sweep-pr-pending-manual --from=cronjob/sweep-pr-pending + +run-sweep-no-patch: + oc delete job sweep-no-patch-manual --ignore-not-found + oc create job sweep-no-patch-manual --from=cronjob/sweep-no-patch + +suspend-sweeps: + oc patch cronjob sweep-dependency --type merge -p '{"spec":{"suspend":true}}' + oc patch cronjob sweep-y-stream --type merge -p '{"spec":{"suspend":true}}' + oc patch cronjob sweep-pr-pending --type merge -p '{"spec":{"suspend":true}}' + oc patch cronjob sweep-no-patch --type merge -p '{"spec":{"suspend":true}}' + +unsuspend-sweeps: + oc patch cronjob sweep-dependency --type merge -p '{"spec":{"suspend":false}}' + oc patch cronjob sweep-y-stream --type merge -p '{"spec":{"suspend":false}}' + oc patch cronjob sweep-pr-pending --type merge -p '{"spec":{"suspend":false}}' + oc patch cronjob sweep-no-patch --type merge -p '{"spec":{"suspend":false}}' + .PHONY: deploy run-jira-issue-fetcher run-jira-issue-fetcher-todo \ suspend-jira-issue-fetcher unsuspend-jira-issue-fetcher \ suspend-jira-issue-fetcher-todo unsuspend-jira-issue-fetcher-todo \ @@ -180,4 +208,6 @@ print(json.dumps({ \ logs-triage logs-backport-c9s logs-backport-c10s \ logs-rebase-c9s logs-rebase-c10s logs-rebuild-c9s logs-rebuild-c10s \ logs-reproducer logs-mcp logs-supervisor logs-valkey logs-phoenix \ - logs-redis-commander logs-otel-collector process trigger-reproducer + logs-redis-commander logs-otel-collector process trigger-reproducer \ + run-sweep-dependency run-sweep-y-stream run-sweep-pr-pending run-sweep-no-patch \ + suspend-sweeps unsuspend-sweeps diff --git a/openshift/cronjob-sweep-dependency.yml b/openshift/cronjob-sweep-dependency.yml new file mode 100644 index 000000000..9e6e67de2 --- /dev/null +++ b/openshift/cronjob-sweep-dependency.yml @@ -0,0 +1,59 @@ +apiVersion: batch/v1 +kind: CronJob +metadata: + name: sweep-dependency + labels: + app: sweep-dependency + component: scheduler +spec: + schedule: "0 */6 * * *" # Every 6 hours + concurrencyPolicy: Forbid + successfulJobsHistoryLimit: 3 + failedJobsHistoryLimit: 5 + suspend: false + jobTemplate: + metadata: + labels: + app: sweep-dependency + component: job + spec: + backoffLimit: 2 + activeDeadlineSeconds: 600 # 10 minutes max runtime + template: + metadata: + labels: + app: sweep-dependency + component: pod + spec: + restartPolicy: Never + containers: + - name: sweep-dependency + image: 'sweep:prod' + imagePullPolicy: IfNotPresent + args: ["python3", "-m", "ymir.sweep", "--strategy", "dependency"] + envFrom: + - configMapRef: + name: endpoints-env + - configMapRef: + name: jira-env + - secretRef: + name: jira-env + resources: + limits: + cpu: "200m" + memory: "256Mi" + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + capabilities: + drop: + - ALL + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + dnsPolicy: ClusterFirst + schedulerName: default-scheduler + terminationGracePeriodSeconds: 30 diff --git a/openshift/cronjob-sweep-no-patch.yml b/openshift/cronjob-sweep-no-patch.yml new file mode 100644 index 000000000..7b4caf01c --- /dev/null +++ b/openshift/cronjob-sweep-no-patch.yml @@ -0,0 +1,64 @@ +apiVersion: batch/v1 +kind: CronJob +metadata: + name: sweep-no-patch + labels: + app: sweep-no-patch + component: scheduler +spec: + schedule: "0 4 * * *" # Daily at 4am UTC + concurrencyPolicy: Forbid + successfulJobsHistoryLimit: 3 + failedJobsHistoryLimit: 5 + suspend: false + jobTemplate: + metadata: + labels: + app: sweep-no-patch + component: job + spec: + backoffLimit: 2 + # Shorter deadline: this sweep only pushes to Redis, the expensive + # LLM work happens in the triage agent pods. + activeDeadlineSeconds: 300 # 5 minutes max runtime + template: + metadata: + labels: + app: sweep-no-patch + component: pod + spec: + restartPolicy: Never + containers: + - name: sweep-no-patch + image: 'sweep:prod' + imagePullPolicy: IfNotPresent + args: ["python3", "-m", "ymir.sweep", "--strategy", "no_patch"] + env: + - name: MAX_ISSUES_PER_RUN + value: "20" + envFrom: + - configMapRef: + name: endpoints-env + - configMapRef: + name: jira-env + - secretRef: + name: jira-env + resources: + limits: + cpu: "200m" + memory: "128Mi" + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + capabilities: + drop: + - ALL + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + dnsPolicy: ClusterFirst + schedulerName: default-scheduler + terminationGracePeriodSeconds: 30 diff --git a/openshift/cronjob-sweep-pr-pending.yml b/openshift/cronjob-sweep-pr-pending.yml new file mode 100644 index 000000000..29717e694 --- /dev/null +++ b/openshift/cronjob-sweep-pr-pending.yml @@ -0,0 +1,61 @@ +apiVersion: batch/v1 +kind: CronJob +metadata: + name: sweep-pr-pending + labels: + app: sweep-pr-pending + component: scheduler +spec: + schedule: "0 */8 * * *" # Every 8 hours + concurrencyPolicy: Forbid + successfulJobsHistoryLimit: 3 + failedJobsHistoryLimit: 5 + suspend: false + jobTemplate: + metadata: + labels: + app: sweep-pr-pending + component: job + spec: + backoffLimit: 2 + activeDeadlineSeconds: 600 # 10 minutes max runtime + template: + metadata: + labels: + app: sweep-pr-pending + component: pod + spec: + restartPolicy: Never + containers: + - name: sweep-pr-pending + image: 'sweep:prod' + imagePullPolicy: IfNotPresent + args: ["python3", "-m", "ymir.sweep", "--strategy", "pr_pending"] + envFrom: + - configMapRef: + name: endpoints-env + - configMapRef: + name: jira-env + - secretRef: + name: jira-env + - secretRef: + name: gitlab-env + resources: + limits: + cpu: "200m" + memory: "128Mi" + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + capabilities: + drop: + - ALL + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + dnsPolicy: ClusterFirst + schedulerName: default-scheduler + terminationGracePeriodSeconds: 30 diff --git a/openshift/cronjob-sweep-y-stream.yml b/openshift/cronjob-sweep-y-stream.yml new file mode 100644 index 000000000..349ee9c8c --- /dev/null +++ b/openshift/cronjob-sweep-y-stream.yml @@ -0,0 +1,59 @@ +apiVersion: batch/v1 +kind: CronJob +metadata: + name: sweep-y-stream + labels: + app: sweep-y-stream + component: scheduler +spec: + schedule: "0 */12 * * *" # Every 12 hours + concurrencyPolicy: Forbid + successfulJobsHistoryLimit: 3 + failedJobsHistoryLimit: 5 + suspend: false + jobTemplate: + metadata: + labels: + app: sweep-y-stream + component: job + spec: + backoffLimit: 2 + activeDeadlineSeconds: 600 # 10 minutes max runtime + template: + metadata: + labels: + app: sweep-y-stream + component: pod + spec: + restartPolicy: Never + containers: + - name: sweep-y-stream + image: 'sweep:prod' + imagePullPolicy: IfNotPresent + args: ["python3", "-m", "ymir.sweep", "--strategy", "y_stream"] + envFrom: + - configMapRef: + name: endpoints-env + - configMapRef: + name: jira-env + - secretRef: + name: jira-env + resources: + limits: + cpu: "200m" + memory: "256Mi" + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + capabilities: + drop: + - ALL + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + dnsPolicy: ClusterFirst + schedulerName: default-scheduler + terminationGracePeriodSeconds: 30 diff --git a/openshift/deploy.sh b/openshift/deploy.sh index ac269eba8..b5bc138bb 100755 --- a/openshift/deploy.sh +++ b/openshift/deploy.sh @@ -117,6 +117,14 @@ apply imagestream-mr-cleanup.yml import_image mr-cleanup apply cronjob-mr-cleanup.yml +# Postponed Issue Sweeps +apply imagestream-sweep.yml +import_image sweep +apply cronjob-sweep-dependency.yml +apply cronjob-sweep-y-stream.yml +apply cronjob-sweep-pr-pending.yml +apply cronjob-sweep-no-patch.yml + # # Supervisor # apply imagestream-supervisor.yml # import_image supervisor diff --git a/openshift/imagestream-sweep.yml b/openshift/imagestream-sweep.yml new file mode 100644 index 000000000..e29b2c073 --- /dev/null +++ b/openshift/imagestream-sweep.yml @@ -0,0 +1,15 @@ +--- +kind: ImageStream +apiVersion: image.openshift.io/v1 +metadata: + name: sweep +spec: + tags: + - name: prod + from: + kind: DockerImage + name: quay.io/jotnar/sweep:latest + importPolicy: + scheduled: true + lookupPolicy: + local: true diff --git a/templates/sweep.env b/templates/sweep.env new file mode 100644 index 000000000..64c7ef9df --- /dev/null +++ b/templates/sweep.env @@ -0,0 +1,9 @@ +# Required: Jira instance URL +JIRA_URL=https://redhat.atlassian.net + +# Required: Jira account email for authentication +JIRA_EMAIL= + +# Required: Jira API token +# Get this from: https://id.atlassian.com/manage-profile/security/api-tokens +JIRA_TOKEN= diff --git a/ymir/agents/cve_applicability_agent.py b/ymir/agents/cve_applicability_agent.py index 64d622a88..1ec4df956 100644 --- a/ymir/agents/cve_applicability_agent.py +++ b/ymir/agents/cve_applicability_agent.py @@ -13,7 +13,7 @@ from ymir.agents.reasoning_agent import ReasoningAgent from ymir.agents.utils import get_chat_model, get_tool_call_checker_config, is_reasoning_enabled from ymir.common.logging_setup import get_trajectory_writeable -from ymir.common.models import Resolution +from ymir.common.models import POSTPONED_RESOLUTIONS, Resolution from ymir.tools.unprivileged.commands import RunShellCommandTool from ymir.tools.unprivileged.text import SearchTextTool, ViewTool @@ -68,7 +68,7 @@ def build_applicability_prompt( cve_label = cve_id or "the CVE" rebuild_context = "" - if resolution in (Resolution.REBUILD, Resolution.POSTPONED) and dep_component: + if resolution in (Resolution.REBUILD, *POSTPONED_RESOLUTIONS) and dep_component: rebuild_context = f"\nThis is a dependency rebuild against updated '{dep_component}'." if dep_issue_key: rebuild_context += ( diff --git a/ymir/agents/prompts/triage/output_format.j2 b/ymir/agents/prompts/triage/output_format.j2 index 55e9b0758..8ad251769 100644 --- a/ymir/agents/prompts/triage/output_format.j2 +++ b/ymir/agents/prompts/triage/output_format.j2 @@ -67,11 +67,12 @@ mentions a build side-tag. Do not set it otherwise. Postponed resolution (rebuild waiting for dependency): ```json { - "resolution": "postponed", + "resolution": "postponed_dependency", "data": { "summary": "Rebuild of some-package waiting for RHEL-67890 (golang) to ship", "pending_issues": ["RHEL-67890"], "jira_issue": "RHEL-12345", + "blocker_references": ["RHEL-67890"], "package": "some-package", "fix_version": "rhel-X.Y.Z", "cve_id": "CVE-1234-98765", @@ -83,6 +84,43 @@ Postponed resolution (rebuild waiting for dependency): Note: When the Jira issue covers multiple CVEs, include ALL CVE IDs in `cve_id`, e.g.: `"cve_id": "CVE-1234-98765 CVE-1234-98766"` +Postponed resolution (CVE confirmed but no upstream patch available yet): +```json +{ + "resolution": "postponed_no_patch", + "data": { + "summary": "CVE-1234-98765 confirmed in upstream but no patch available; issue will be re-triaged when upstream provides a fix", + "pending_issues": ["RHEL-12345"], + "jira_issue": "RHEL-12345", + "package": "libfoo", + "fix_version": "rhel-8.10.0", + "cve_id": "CVE-1234-98765" + } +} +``` +Note: Use this when investigation confirms a valid CVE or bug exists but no upstream fix is available yet (not even an unmerged one). The issue will be periodically re-triaged. +Note: When the Jira issue covers multiple CVEs, include ALL CVE IDs in `cve_id`, e.g.: +`"cve_id": "CVE-1234-98765 CVE-1234-98766"` + +Postponed resolution (fix identified in unmerged GitLab merge request): +```json +{ + "resolution": "postponed_pr_pending", + "data": { + "summary": "Fix for CVE-1234-98765 pending in upstream MR !123; waiting for merge", + "pending_issues": ["RHEL-12345"], + "jira_issue": "RHEL-12345", + "blocker_references": ["https://gitlab.com/example-org/example-repo/-/merge_requests/123"], + "package": "some-package", + "fix_version": "rhel-9.5.0", + "cve_id": "CVE-1234-98765" + } +} +``` +Note: Use this when you find a relevant upstream fix in an unmerged GitLab merge request or GitHub pull request. Include the full PR/MR URL(s) in blocker_references as a JSON array. Do NOT use this for other platforms (e.g. Bitbucket, SourceForge). +Note: When the Jira issue covers multiple CVEs, include ALL CVE IDs in `cve_id`, e.g.: +`"cve_id": "CVE-1234-98765 CVE-1234-98766"` + Not-affected resolution (CVE does not apply): ```json { diff --git a/ymir/agents/prompts/triage/prompt.j2 b/ymir/agents/prompts/triage/prompt.j2 index cbb414911..43089f2e5 100644 --- a/ymir/agents/prompts/triage/prompt.j2 +++ b/ymir/agents/prompts/triage/prompt.j2 @@ -229,6 +229,17 @@ You must decide between one of the following actions. Follow these guidelines to If you find a relevant but unmerged patch during your investigation, mention it in the clarification-needed note so a human can evaluate it, but do not use it as the basis for a backport decision. + * **Unmerged upstream fixes — use postponed_pr_pending**: If during your + investigation you find a relevant fix in an UNMERGED GitLab merge request + or GitHub pull request that addresses the issue but has not been merged yet: + - Validate that the PR/MR is still open (not closed/abandoned) + - Use resolution "postponed_pr_pending" instead of "clarification-needed" + - Set blocker_references to a JSON array with the full PR/MR URL: + - GitLab MR: `["https://gitlab.com/org/repo/-/merge_requests/123"]` + - GitHub PR: `["https://github.com/org/repo/pull/123"]` + - Set pending_issues to [] + - This does NOT apply to other platforms (e.g. Bitbucket, SourceForge) + The sweep system will monitor the PR/MR and re-triage once it merges. * **Check for follow-up commits**: After identifying a valid fix, you MUST check whether there are follow-up commits that are **necessary to make the fix correct and complete**. A follow-up commit should be included ONLY when: @@ -331,8 +342,18 @@ You must decide between one of the following actions. Follow these guidelines to passes all validations in step 2.3, your decision is backport * You must be able to justify why the patch is correct and how it addresses the issue {% endif %} - * If your investigation confirms a valid bug/CVE but fails to locate a specific fix, your decision - is clarification-needed + * If your investigation confirms a valid bug/CVE but fails to locate a specific fix: + - If you found NO fix anywhere (not even an unmerged MR or proposed patch), + AND you exhausted all search strategies from step 2.2 (Fedora, upstream git, + bug trackers, targeted git searches, etc.), your decision is + "postponed_no_patch". Use this when you're confident the bug/CVE is real + but upstream has not yet provided a solution. The issue will be + periodically re-queued for fresh triage. + - If you found partial information (e.g., bug tracker discussion, proposed + but not accepted patches, or you need human clarification on next steps), + your decision is "clarification-needed" + - If you found an unmerged GitLab MR or GitHub PR, use "postponed_pr_pending" + as described in step 2.3 * This is the correct choice when you are sure a problem exists but cannot find the solution yourself 2.5 Set the Jira fields as per the instructions below. @@ -375,9 +396,10 @@ You must decide between one of the following actions. Follow these guidelines to Set dependency_issue to the issue key AND dependency_component to the component name (e.g., "golang", "openssl") from the dependency issue's component field * If the dependency issue exists but has no `Fixed in Build` yet - and is still open → resolution is "postponed" + and is still open → resolution is "postponed_dependency" Set summary to explain that rebuild is waiting for the dependency to ship, and set pending_issues to the dependency issue key. + Set blocker_references to a JSON array containing the dependency issue key, e.g. ["RHEL-67890"]. Also set package, fix_version, cve_id, dependency_issue, and dependency_component (same values as you would for a rebuild resolution). * **Fallback — no matching CVE tracker found**: If no dependency CVE tracker diff --git a/ymir/agents/tests/unit/test_triage_agent.py b/ymir/agents/tests/unit/test_triage_agent.py index 4f3b60ba3..231238e4c 100644 --- a/ymir/agents/tests/unit/test_triage_agent.py +++ b/ymir/agents/tests/unit/test_triage_agent.py @@ -33,7 +33,10 @@ Resolution.BACKPORT, Resolution.REBUILD, Resolution.NOT_AFFECTED, - Resolution.POSTPONED, + Resolution.POSTPONED_DEPENDENCY, + Resolution.POSTPONED_Y_STREAM, + Resolution.POSTPONED_NO_PATCH, + Resolution.POSTPONED_PR_PENDING, Resolution.OPEN_ENDED_ANALYSIS, Resolution.CLARIFICATION_NEEDED, Resolution.ERROR, @@ -62,7 +65,10 @@ def test_non_user_triggered_skips_comment_when_mr_will_be_opened(resolution): "resolution", [ Resolution.NOT_AFFECTED, - Resolution.POSTPONED, + Resolution.POSTPONED_DEPENDENCY, + Resolution.POSTPONED_Y_STREAM, + Resolution.POSTPONED_NO_PATCH, + Resolution.POSTPONED_PR_PENDING, Resolution.OPEN_ENDED_ANALYSIS, Resolution.CLARIFICATION_NEEDED, ], @@ -575,7 +581,7 @@ def test_build_reproducer_input_skips_postponed(): state = TriageState( jira_issue="RHEL-105", triage_result=TriageOutputSchema( - resolution=Resolution.POSTPONED, + resolution=Resolution.POSTPONED_DEPENDENCY, data=PostponedData( summary="waiting", pending_issues=["RHEL-1"], @@ -586,3 +592,117 @@ def test_build_reproducer_input_skips_postponed(): ) # Builder returns a payload when package exists; eligibility is checked by enqueue. assert _build_reproducer_input(state).package == "golang" + + +# --- Eligibility → resolution mapping regression tests --- + + +@pytest.mark.asyncio +async def test_pending_dependencies_maps_to_postponed_y_stream(): + """PENDING_DEPENDENCIES eligibility must produce POSTPONED_Y_STREAM, not + POSTPONED_DEPENDENCY. The former is swept by YStreamSweep; the latter by + DependencySweep (rebuild waiting for a component's fixed build). Mixing + them up silently deadlocks one of the two sweep paths.""" + from ymir.agents.triage_agent import run_workflow + + pending_issues = ["RHEL-99998"] + eligibility_result = CVEEligibilityResult( + is_cve=True, + eligibility=TriageEligibility.PENDING_DEPENDENCIES, + reason="Waiting for Z-stream clones to ship", + pending_zstream_issues=pending_issues, + ) + + @asynccontextmanager + async def _mock_mcp_tools(*_args, **_kwargs): + yield [] + + with ( + patch("ymir.agents.triage_agent.mcp_tools", side_effect=_mock_mcp_tools), + patch( + "ymir.agents.triage_agent.run_tool", + new_callable=AsyncMock, + return_value=eligibility_result.model_dump(), + ), + patch("ymir.agents.triage_agent.get_mock_local_tool_env", return_value=None), + patch.dict( + "os.environ", + {"GIT_REPO_BASEPATH": "/tmp", "MCP_GATEWAY_URL": "http://localhost"}, + clear=False, + ), + ): + state = await run_workflow( + "RHEL-99999", + dry_run=True, + triage_agent_factory=MagicMock(), + ) + + assert state.triage_result.resolution == Resolution.POSTPONED_Y_STREAM + assert state.triage_result.data.pending_issues == pending_issues + + +@pytest.mark.asyncio +async def test_pr_pending_without_blocker_reference_raises(): + """A postponed_pr_pending resolution with no blocker_references URL is not + sweepable (PRPendingSweep has no MR/PR to poll), so run_triage_analysis must + raise rather than silently produce a permanently-stuck issue. The raise is + caught by the workflow's outer handler and routed through retry().""" + from ymir.agents.triage_agent import run_workflow + + eligibility_result = CVEEligibilityResult( + is_cve=True, + eligibility=TriageEligibility.IMMEDIATELY, + reason="Eligible for immediate triage", + ) + + # LLM output: postponed_pr_pending but blocker_references omitted. + llm_json = ( + '{"resolution": "postponed_pr_pending", "data": {' + '"summary": "Fix pending in upstream MR; waiting for merge", ' + '"pending_issues": ["RHEL-1"], "jira_issue": "RHEL-99999"}}' + ) + response = MagicMock() + response.last_message.text = llm_json + agent = MagicMock() + agent.run = AsyncMock(return_value=response) + + @asynccontextmanager + async def _mock_mcp_tools(*_args, **_kwargs): + yield [] + + with ( + patch("ymir.agents.triage_agent.mcp_tools", side_effect=_mock_mcp_tools), + patch( + "ymir.agents.triage_agent.run_tool", + new_callable=AsyncMock, + return_value=eligibility_result.model_dump(), + ), + patch("ymir.agents.triage_agent.get_mock_local_tool_env", return_value=None), + patch("ymir.agents.triage_agent.render_prompt", new_callable=AsyncMock, return_value="prompt"), + patch("ymir.agents.triage_agent.render_template", return_value="output format"), + patch("ymir.agents.triage_agent.get_agent_execution_config", return_value={}), + patch.dict( + "os.environ", + {"GIT_REPO_BASEPATH": "/tmp", "MCP_GATEWAY_URL": "http://localhost"}, + clear=False, + ), + pytest.raises(Exception) as excinfo, + ): + await run_workflow( + "RHEL-99999", + dry_run=True, + triage_agent_factory=MagicMock(return_value=agent), + ) + + # The beeai Workflow wraps a node's exception in a FrameworkError, chaining + # the original via __cause__. The outer handler in _process_triage_locked + # catches it (except Exception) and routes to retry(); here we assert the + # guard's ValueError is what propagated. + chain = [] + err = excinfo.value + while err is not None: + chain.append(err) + err = err.__cause__ + assert any(isinstance(e, ValueError) and "postponed_pr_pending" in str(e) for e in chain), ( + f"expected a chained ValueError about postponed_pr_pending, got: {chain!r}" + ) diff --git a/ymir/agents/triage_agent.py b/ymir/agents/triage_agent.py index c9a1baa17..13952b021 100644 --- a/ymir/agents/triage_agent.py +++ b/ymir/agents/triage_agent.py @@ -47,6 +47,7 @@ from ymir.common.logging_setup import configure_logging, current_jira_issue, get_trajectory_writeable from ymir.common.mock_repos import get_mock_local_tool_env from ymir.common.models import ( + POSTPONED_RESOLUTIONS, ApplicabilityResult, ClarificationNeededData, CVEEligibilityResult, @@ -107,9 +108,9 @@ def _should_update_jira(resolution: Resolution = None, user_triggered: bool = Fa return True return resolution in ( Resolution.NOT_AFFECTED, - Resolution.POSTPONED, Resolution.OPEN_ENDED_ANALYSIS, Resolution.CLARIFICATION_NEEDED, + *POSTPONED_RESOLUTIONS, ) @@ -119,9 +120,12 @@ def _should_update_jira(resolution: Resolution = None, user_triggered: bool = Fa Resolution.REBUILD: JiraLabels.TRIAGED_REBUILD, Resolution.CLARIFICATION_NEEDED: JiraLabels.NEEDS_ATTENTION, Resolution.OPEN_ENDED_ANALYSIS: JiraLabels.TRIAGED, - Resolution.POSTPONED: JiraLabels.TRIAGED_POSTPONED, Resolution.NOT_AFFECTED: JiraLabels.TRIAGED_NOT_AFFECTED, Resolution.ERROR: JiraLabels.TRIAGE_ERRORED, + Resolution.POSTPONED_DEPENDENCY: JiraLabels.YMIR_POSTPONED_DEPENDENCY, + Resolution.POSTPONED_NO_PATCH: JiraLabels.YMIR_POSTPONED_NO_PATCH, + Resolution.POSTPONED_PR_PENDING: JiraLabels.YMIR_POSTPONED_PR_PENDING, + Resolution.POSTPONED_Y_STREAM: JiraLabels.YMIR_POSTPONED_Y_STREAM, } _REPRODUCER_ELIGIBLE_RESOLUTIONS = frozenset( @@ -575,17 +579,36 @@ async def check_cve_eligibility(state): f"Issue {state.jira_issue}: eligibility is PENDING_DEPENDENCIES " f"but no pending Z-stream issues were returned — this is unexpected" ) + state.triage_result = OutputSchema( + resolution=Resolution.OPEN_ENDED_ANALYSIS, + data=OpenEndedAnalysisData( + summary=( + "Eligibility blocked on PENDING_DEPENDENCIES but " + "no Z-stream issues were returned." + ), + jira_issue=state.jira_issue, + recommendation="Check the eligibility tool output manually.", + ), + ) + return "comment_in_jira" logger.info( f"Issue {state.jira_issue} postponed — waiting for " f"{len(pending)} Z-stream issue(s): {pending}. " f"Reason: {state.cve_eligibility_result.reason}" ) + # A Y-stream CVE waiting for its Z-stream clones to ship is the + # "y_stream" postponement category (waiting for Z-stream errata), + # NOT "dependency" (a rebuild waiting for another component's + # fixed build). PENDING_DEPENDENCIES is only ever returned by the + # Y-stream eligibility path, so this always maps to y_stream and + # is swept by YStreamSweep. state.triage_result = OutputSchema( - resolution=Resolution.POSTPONED, + resolution=Resolution.POSTPONED_Y_STREAM, data=PostponedData( summary=state.cve_eligibility_result.reason, pending_issues=pending, jira_issue=state.jira_issue, + blocker_references=pending, ), ) return "comment_in_jira" @@ -660,6 +683,45 @@ async def run_triage_analysis(state): ) state.triage_result = OutputSchema.model_validate_json(response.last_message.text) + # postponed_y_stream is owned by the eligibility system (PENDING_DEPENDENCIES + # verdict). The LLM analysis node is only reached when eligibility returned + # IMMEDIATELY or force_cve_triage bypassed it — in both cases the eligibility + # tool has already decided the issue is not waiting on Z-stream clones. + # An LLM-asserted postponed_y_stream would be swept away immediately anyway, + # so coerce it to open-ended-analysis to surface the mismatch for review. + if state.triage_result.resolution == Resolution.POSTPONED_Y_STREAM: + logger.warning( + "LLM emitted postponed_y_stream for %s, which is reserved for the " + "eligibility system. Coercing to open-ended-analysis.", + state.jira_issue, + ) + state.triage_result = OutputSchema( + resolution=Resolution.OPEN_ENDED_ANALYSIS, + data=OpenEndedAnalysisData( + summary=( + "Triage agent identified a Y-stream/Z-stream dependency, " + "but the eligibility system did not return PENDING_DEPENDENCIES. " + "Manual review required." + ), + recommendation=( + "Check whether Z-stream clones exist and their shipping status. " + "If the dependency is genuine, the eligibility tool may need updating." + ), + jira_issue=state.jira_issue, + ), + ) + + # postponed_pr_pending is only sweepable if the comment carries the + # PR/MR URL in blocker_references (PRPendingSweep polls that URL). + if ( + state.triage_result.resolution == Resolution.POSTPONED_PR_PENDING + and not state.triage_result.data.blocker_references + ): + raise ValueError( + f"LLM emitted postponed_pr_pending for {state.jira_issue} without a " + "blocker MR/PR URL in blocker_references; retrying." + ) + # Jira issue key in resolution data has been generated by LLM, make sure it's upper-case state.triage_result.data.jira_issue = state.triage_result.data.jira_issue.upper() @@ -682,7 +744,7 @@ async def run_triage_analysis(state): Resolution.NOT_AFFECTED, ]: return "comment_in_jira" - if state.triage_result.resolution == Resolution.POSTPONED: + if state.triage_result.resolution in POSTPONED_RESOLUTIONS: # Route postponed-rebuild CVEs through applicability to check # if the CVE actually affects the package — if not, resolve as # NOT_AFFECTED instead of waiting for the dependency to ship. @@ -718,7 +780,7 @@ async def determine_target_branch_step(state): state.cve_eligibility_result and state.cve_eligibility_result.is_cve and state.triage_result.resolution - in (Resolution.BACKPORT, Resolution.REBUILD, Resolution.REBASE, Resolution.POSTPONED) + in (Resolution.BACKPORT, Resolution.REBUILD, Resolution.REBASE, *POSTPONED_RESOLUTIONS) ): return "check_cve_applicability" @@ -985,7 +1047,7 @@ async def verify_rebuild_buildroot(state): f"{state.target_branch} buildroot — postponing {state.jira_issue}" ) state.triage_result = OutputSchema( - resolution=Resolution.POSTPONED, + resolution=Resolution.POSTPONED_DEPENDENCY, data=PostponedData( summary=( f"Rebuild of {data.package} waiting for {dep_component} " @@ -993,6 +1055,7 @@ async def verify_rebuild_buildroot(state): ), pending_issues=[dep_issue_key], jira_issue=state.jira_issue, + blocker_references=[dep_issue_key], package=data.package, fix_version=data.fix_version, cve_id=data.cve_id, @@ -1126,6 +1189,32 @@ async def comment_in_jira(state): return response.state +async def label_postponed_issues(jira_issue: str, output: OutputSchema, dry_run: bool, user_triggered: bool): + """Remove existing postponement and triage in progress labels, place label with reason on the issue. + Log and reraise exceptions.""" + if not (reason_label := _RESOLUTION_TO_LABEL.get(output.resolution)): + raise ValueError(f"No label mapping for postponed resolution {output.resolution!r};") + + postponement_labels = [lbl for lbl in JiraLabels.postponement_labels() if lbl != reason_label.value] + [ + JiraLabels.TRIAGED_POSTPONED.value + ] + try: + await tasks.set_jira_labels( + jira_issue=jira_issue, + labels_to_add=[reason_label.value], + labels_to_remove=[JiraLabels.TRIAGE_IN_PROGRESS.value, *postponement_labels], + dry_run=dry_run, + user_triggered=user_triggered, + ) + except Exception as e: + logger.warning( + "Failed to set postponement label on %s: %s", + jira_issue, + e, + ) + raise + + async def main() -> None: init_sentry() @@ -1198,21 +1287,26 @@ async def main() -> None: (result_dir / "triage_result.json").write_text(output.model_dump_json(indent=2), encoding="utf-8") if user_triggered and not dry_run: - resolution_label = _RESOLUTION_TO_LABEL.get(output.resolution) - if resolution_label and output.resolution != Resolution.ERROR: - try: - await tasks.set_jira_labels( - jira_issue=jira_issue, - labels_to_add=[resolution_label.value], - user_triggered=True, - dry_run=dry_run, - ) - except Exception as e: - logger.warning( - "Failed to set resolution label on %s: %s", - jira_issue, - e, - ) + if output.resolution in POSTPONED_RESOLUTIONS: + await label_postponed_issues( + jira_issue=jira_issue, output=output, dry_run=dry_run, user_triggered=user_triggered + ) + else: + resolution_label = _RESOLUTION_TO_LABEL.get(output.resolution) + if resolution_label and output.resolution != Resolution.ERROR: + try: + await tasks.set_jira_labels( + jira_issue=jira_issue, + labels_to_add=[resolution_label.value], + user_triggered=True, + dry_run=dry_run, + ) + except Exception as e: + logger.warning( + "Failed to set resolution label on %s: %s", + jira_issue, + e, + ) if output.resolution == Resolution.REBUILD: for consolidated in output.data.consolidated_issues: try: @@ -1460,46 +1554,53 @@ async def retry(task, error, input=input, user_triggered=user_triggered): except Exception as e: logger.warning(f"Failed to check if {input.issue} is sibling: {e}") - resolution_label = _RESOLUTION_TO_LABEL.get(output.resolution) - if resolution_label and output.resolution != Resolution.ERROR: - # Terminal resolution label is the dedup anchor that replaces - # ymir_triage_in_progress — must be written unconditionally so - # the next fetcher sweep skips this issue. - # Exception: skip terminal label if waiting for siblings (will be set on re-triage) - # Also remove ymir_rebase_sibling if present (sibling finished triaging) - labels_to_remove = [JiraLabels.TRIAGE_IN_PROGRESS.value] - labels_to_add = [] - - if is_sibling: - labels_to_remove.append(JiraLabels.REBASE_SIBLING.value) - logger.info(f"{input.issue} is a sibling, will check if primary is ready to queue") - - # Only add terminal label if NOT waiting for siblings - # (primary waiting for siblings will be re-triaged when siblings finish, - # and terminal label will be added then) - if not ( - state.rebase_waiting_for_siblings - or JiraLabels.WAITING_FOR_SIBLINGS.value in current_labels - ): - labels_to_add.append(resolution_label.value) - else: - logger.info( - f"{input.issue} is waiting for siblings, skipping terminal label " - "(will be added on re-triage after siblings finish)" - ) - - await tasks.set_jira_labels( - jira_issue=input.issue, - labels_to_add=labels_to_add, - labels_to_remove=labels_to_remove, - dry_run=dry_run, - user_triggered=user_triggered, - critical=True, # Terminal label is dedup anchor; must succeed + if output.resolution in POSTPONED_RESOLUTIONS: + await label_postponed_issues( + jira_issue=input.issue, output=output, dry_run=dry_run, user_triggered=user_triggered ) + else: + resolution_label = _RESOLUTION_TO_LABEL.get(output.resolution) + if resolution_label and output.resolution != Resolution.ERROR: + # Terminal resolution label is the dedup anchor that replaces + # ymir_triage_in_progress — must be written unconditionally so + # the next fetcher sweep skips this issue. + # Exception: skip terminal label if waiting for siblings (will be set on re-triage) + # Also remove ymir_rebase_sibling if present (sibling finished triaging) + labels_to_remove = [JiraLabels.TRIAGE_IN_PROGRESS.value] + labels_to_add = [] + + if is_sibling: + labels_to_remove.append(JiraLabels.REBASE_SIBLING.value) + logger.info( + f"{input.issue} is a sibling, will check if primary is ready to queue" + ) + + # Only add terminal label if NOT waiting for siblings + # (primary waiting for siblings will be re-triaged when siblings finish, + # and terminal label will be added then) + if not ( + state.rebase_waiting_for_siblings + or JiraLabels.WAITING_FOR_SIBLINGS.value in current_labels + ): + labels_to_add.append(resolution_label.value) + else: + logger.info( + f"{input.issue} is waiting for siblings, skipping terminal label " + "(will be added on re-triage after siblings finish)" + ) + + await tasks.set_jira_labels( + jira_issue=input.issue, + labels_to_add=labels_to_add, + labels_to_remove=labels_to_remove, + dry_run=dry_run, + user_triggered=user_triggered, + critical=True, # Terminal label is dedup anchor; must succeed + ) - # Update current_labels to reflect the changes we just made - if is_sibling and JiraLabels.REBASE_SIBLING.value in current_labels: - current_labels.remove(JiraLabels.REBASE_SIBLING.value) + # Update current_labels to reflect the changes we just made + if is_sibling and JiraLabels.REBASE_SIBLING.value in current_labels: + current_labels.remove(JiraLabels.REBASE_SIBLING.value) if output.resolution == Resolution.REBUILD: for consolidated in output.data.consolidated_issues: try: @@ -1580,7 +1681,7 @@ async def retry(task, error, input=input, user_triggered=user_triggered): # Dispatch to downstream queues if output.resolution == Resolution.ERROR: await retry(task, output.data.model_dump_json()) - elif output.resolution == Resolution.POSTPONED: + elif output.resolution in POSTPONED_RESOLUTIONS: await fix_await( redis.lpush( RedisQueues.POSTPONED_LIST.value, diff --git a/ymir/common/constants.py b/ymir/common/constants.py index b6c3152fb..81b959289 100644 --- a/ymir/common/constants.py +++ b/ymir/common/constants.py @@ -187,7 +187,16 @@ class JiraLabels(Enum): REPRODUCER_NOT_REPRODUCIBLE = "ymir_reproducer_not_reproducible" REPRODUCER_ALREADY_EXISTS = "ymir_reproducer_already_exists" + # Deprecated general postponed label, + # replaced by labels with reason for postponement TRIAGED_POSTPONED = "ymir_triaged_postponed" + + # Reasons for postponing implementation + YMIR_POSTPONED_DEPENDENCY = "ymir_postponed_dependency" + YMIR_POSTPONED_Y_STREAM = "ymir_postponed_y_stream" + YMIR_POSTPONED_NO_PATCH = "ymir_postponed_no_patch" + YMIR_POSTPONED_PR_PENDING = "ymir_postponed_pr_pending" + TRIAGED_NOT_AFFECTED = "ymir_triaged_not_affected" RETRY_NEEDED = "ymir_retry_needed" @@ -211,7 +220,20 @@ class JiraLabels(Enum): # TRIAGE_IN_PROGRESS on enqueue. TODO = "ymir_todo" + # When issue has been postponed too many times it must be marked as abandoned + ABANDONED = "ymir_abandoned" + @classmethod def all_labels(cls) -> set[str]: """Return all Ymir labels for cleanup operations""" return {label.value for label in cls} + + @classmethod + def postponement_labels(cls) -> set[str]: + """Return all postponement labels""" + return { + cls.YMIR_POSTPONED_DEPENDENCY.value, + cls.YMIR_POSTPONED_Y_STREAM.value, + cls.YMIR_POSTPONED_NO_PATCH.value, + cls.YMIR_POSTPONED_PR_PENDING.value, + } diff --git a/ymir/common/models.py b/ymir/common/models.py index 38ab2fbff..b8cd72fbc 100644 --- a/ymir/common/models.py +++ b/ymir/common/models.py @@ -216,9 +216,28 @@ class Resolution(Enum): REBUILD = "rebuild" CLARIFICATION_NEEDED = "clarification-needed" OPEN_ENDED_ANALYSIS = "open-ended-analysis" - POSTPONED = "postponed" NOT_AFFECTED = "not-affected" ERROR = "error" + # Postponement resolutions, broken down by the reason for postponing. + # The reason is carried by the resolution itself (there is no separate + # category field); each maps to a distinct ymir_postponed_* Jira label. + POSTPONED_DEPENDENCY = "postponed_dependency" + POSTPONED_Y_STREAM = "postponed_y_stream" + POSTPONED_NO_PATCH = "postponed_no_patch" + POSTPONED_PR_PENDING = "postponed_pr_pending" + + +# The set of resolutions that postpone an issue. Use for membership checks +# (``resolution in POSTPONED_RESOLUTIONS``) instead of comparing against a +# single general POSTPONED value. +POSTPONED_RESOLUTIONS: frozenset[Resolution] = frozenset( + { + Resolution.POSTPONED_DEPENDENCY, + Resolution.POSTPONED_Y_STREAM, + Resolution.POSTPONED_NO_PATCH, + Resolution.POSTPONED_PR_PENDING, + } +) class RebaseData(BaseModel): @@ -394,6 +413,10 @@ class PostponedData(BaseModel): summary: str = Field(description="Reason for postponement") pending_issues: list[str] = Field(description="Jira issue keys of dependencies not yet shipped") jira_issue: str = Field(description="Jira issue identifier") + blocker_references: list[str] | None = Field( + default=None, + description="List of machine-readable blocker IDs: Jira issue keys, errata IDs, or MR URLs", + ) package: str | None = Field(default=None, description="Package name (for rebuild postponements)") fix_version: str | None = Field( default=None, description="Fix version in Jira (for rebuild postponements)" @@ -474,13 +497,16 @@ class ErrorData(BaseModel): "is not yet supported by Ymir. Manual action is required._" ) +POSTPONEMENT_NOTE = "\n\n_This issue will be monitored by Ymir and processed once it is unblocked._" + class TriageOutputSchema(BaseModel): """Output schema for the triage agent.""" resolution: Resolution = Field( description="Triage resolution, one of rebase, backport, rebuild, " - "clarification-needed, open-ended-analysis, postponed, error" + "clarification-needed, open-ended-analysis, not-affected, error, " + "postponed_dependency, postponed_no_patch, postponed_pr_pending" ) data: ( RebaseData @@ -615,10 +641,19 @@ def format_for_comment(self, auto_chain: bool = False) -> str: heading = "*Waiting for*:" else: heading = "*Waiting for at least one of*:" + blocker_line = ( + f"\n*Blockers*: {','.join(self.data.blocker_references)}" + if self.data.blocker_references + else "" + ) + # Changes here must be reflected in the `ymir.sweep` regular expressions + # otherwise the postponed issue processing will break return ( f"{resolution}" f"*Summary*: {self.data.summary}\n" f"{heading}\n{pending_text}" + f"{blocker_line}" + f"{POSTPONEMENT_NOTE}" f"{TRIAGE_DISCLAIMER}" ) diff --git a/ymir/common/tests/unit/test_models.py b/ymir/common/tests/unit/test_models.py index c0d4e1fd1..ae736a22a 100644 --- a/ymir/common/tests/unit/test_models.py +++ b/ymir/common/tests/unit/test_models.py @@ -1,5 +1,6 @@ from ymir.common.models import ( AUTOMATED_RESOLUTION_NOT_SUPPORTED, + POSTPONEMENT_NOTE, TRIAGE_DISCLAIMER, ApplicabilityResult, BackportData, @@ -148,15 +149,16 @@ def test_postponed_formatting_multiple_issues(): pending_issues=["RHEL-111", "RHEL-222"], jira_issue="RHEL-99999", ) - result = TriageOutputSchema(resolution=Resolution.POSTPONED, data=data) + result = TriageOutputSchema(resolution=Resolution.POSTPONED_Y_STREAM, data=data) assert result.format_for_comment() == ( - "*Resolution*: postponed\n" + "*Resolution*: postponed_y_stream\n" "*Summary*: Y-stream CVE (CVE-2025-12345): " "waiting for at least one Z-stream clone to ship\n" "*Waiting for at least one of*:\n" "* RHEL-111\n" "* RHEL-222" + f"{POSTPONEMENT_NOTE}" f"{TRIAGE_DISCLAIMER}" ) @@ -167,13 +169,14 @@ def test_postponed_formatting_single_issue(): pending_issues=["RHEL-333"], jira_issue="RHEL-99999", ) - result = TriageOutputSchema(resolution=Resolution.POSTPONED, data=data) + result = TriageOutputSchema(resolution=Resolution.POSTPONED_DEPENDENCY, data=data) assert result.format_for_comment() == ( - "*Resolution*: postponed\n" + "*Resolution*: postponed_dependency\n" "*Summary*: Rebuild waiting for dependency to ship\n" "*Waiting for*:\n" "* RHEL-333" + f"{POSTPONEMENT_NOTE}" f"{TRIAGE_DISCLAIMER}" ) @@ -195,9 +198,10 @@ def test_postponed_rebuild_with_extra_fields(): assert data.cve_id == "CVE-2026-99999" assert data.fix_version == "rhel-10.1" - result = TriageOutputSchema(resolution=Resolution.POSTPONED, data=data) + result = TriageOutputSchema(resolution=Resolution.POSTPONED_DEPENDENCY, data=data) comment = result.format_for_comment() assert "*Summary*: Rebuild of butane" in comment + assert "*Resolution*: postponed_dependency" in comment assert "RHEL-67890" in comment @@ -214,9 +218,10 @@ def test_postponed_without_rebuild_fields(): assert data.dependency_issue is None assert data.dependency_component is None - result = TriageOutputSchema(resolution=Resolution.POSTPONED, data=data) + result = TriageOutputSchema(resolution=Resolution.POSTPONED_Y_STREAM, data=data) comment = result.format_for_comment() assert "*Waiting for*:" in comment + assert "*Resolution*: postponed_y_stream" in comment def test_error_formatting(): @@ -670,3 +675,104 @@ def test_reproducer_output_retryable_error(): assert data.test_already_exists is False restored = ReproducerOutputSchema.model_validate_json(data.model_dump_json()) assert restored.retryable_error is True + + +# --- Postponement reason (resolution) and blocker_reference tests --- + + +def test_postponed_dependency_with_blocker(): + """Dependency postponement renders reason (resolution) and blocker.""" + data = PostponedData( + summary="Rebuild of butane waiting for golang to ship", + pending_issues=["RHEL-67890"], + jira_issue="RHEL-12345", + blocker_references=["RHEL-67890"], + ) + result = TriageOutputSchema(resolution=Resolution.POSTPONED_DEPENDENCY, data=data) + comment = result.format_for_comment() + + assert "*Resolution*: postponed_dependency" in comment + assert "*Blockers*: RHEL-67890" in comment + assert "*Waiting for*:" in comment + assert "* RHEL-67890" in comment + + +def test_postponed_reason_y_stream(): + """Y-stream postponement renders its resolution as the reason.""" + data = PostponedData( + summary="Y-stream CVE waiting for Z-stream clone to ship", + pending_issues=["RHEL-111"], + jira_issue="RHEL-99999", + ) + result = TriageOutputSchema(resolution=Resolution.POSTPONED_Y_STREAM, data=data) + comment = result.format_for_comment() + assert "*Resolution*: postponed_y_stream" in comment + + +def test_postponed_reason_pr_pending(): + """PR-pending postponement renders its resolution and blocker MR URL.""" + data = PostponedData( + summary="Waiting for upstream MR to be merged", + pending_issues=["RHEL-222"], + jira_issue="RHEL-88888", + blocker_references=["https://gitlab.com/redhat/rpms/pkg/-/merge_requests/42"], + ) + result = TriageOutputSchema(resolution=Resolution.POSTPONED_PR_PENDING, data=data) + comment = result.format_for_comment() + assert "*Resolution*: postponed_pr_pending" in comment + assert "*Blockers*: https://gitlab.com/redhat/rpms/pkg/-/merge_requests/42" in comment + + +def test_postponed_reason_no_patch(): + """No-patch postponement renders its resolution as the reason.""" + data = PostponedData( + summary="No upstream patch available yet", + pending_issues=["RHEL-333"], + jira_issue="RHEL-77777", + ) + result = TriageOutputSchema(resolution=Resolution.POSTPONED_NO_PATCH, data=data) + comment = result.format_for_comment() + assert "*Resolution*: postponed_no_patch" in comment + + +def test_postponed_no_blocker_reference(): + """PostponedData without blocker_references omits *Blockers*: line.""" + data = PostponedData( + summary="Waiting for dependency", + pending_issues=["RHEL-444"], + jira_issue="RHEL-66666", + ) + result = TriageOutputSchema(resolution=Resolution.POSTPONED_DEPENDENCY, data=data) + comment = result.format_for_comment() + assert "*Blockers*:" not in comment + + +def test_postponed_serialization_roundtrip(): + """PostponedData survives a JSON roundtrip.""" + data = PostponedData( + summary="Rebuild waiting for golang", + pending_issues=["RHEL-67890"], + jira_issue="RHEL-12345", + blocker_references=["RHEL-67890"], + package="butane", + fix_version="rhel-10.1", + ) + json_str = data.model_dump_json() + restored = PostponedData.model_validate_json(json_str) + + assert restored.blocker_references == ["RHEL-67890"] + assert restored.package == "butane" + assert restored.fix_version == "rhel-10.1" + assert restored.pending_issues == ["RHEL-67890"] + + +def test_postponed_minimal_fields(): + """PostponedData validates with only the required fields.""" + payload = { + "summary": "Rebuild waiting for dependency to ship", + "pending_issues": ["RHEL-333"], + "jira_issue": "RHEL-99999", + } + data = PostponedData.model_validate(payload) + assert data.blocker_references is None + assert data.pending_issues == ["RHEL-333"] diff --git a/ymir/supervisor/github_utils.py b/ymir/supervisor/github_utils.py new file mode 100644 index 000000000..1b403a387 --- /dev/null +++ b/ymir/supervisor/github_utils.py @@ -0,0 +1,32 @@ +"""GitHub REST API helpers for the supervisor and sweep layers. + +Authentication is optional: if ``GITHUB_TOKEN`` is set it is sent as a +Bearer token. +Without a token the client still works for public repositories, which +should cover most upstream open-source projects that Ymir tracks. +""" + +import logging +import os +from typing import Any + +from ymir.supervisor.http_utils import requests_session + +logger = logging.getLogger(__name__) + +GITHUB_API_URL = "https://api.github.com" + + +def _github_headers() -> dict[str, str]: + headers = {"Accept": "application/vnd.github+json"} + token = os.environ.get("GITHUB_TOKEN") + if token: + headers["Authorization"] = f"Bearer {token}" + return headers + + +def github_api_get(path: str, *, params: dict | None = None) -> Any: + url = f"{GITHUB_API_URL}/{path}" + response = requests_session().get(url, headers=_github_headers(), params=params) + response.raise_for_status() + return response.json() diff --git a/ymir/supervisor/gitlab_utils.py b/ymir/supervisor/gitlab_utils.py index 57de087a7..280f09f60 100644 --- a/ymir/supervisor/gitlab_utils.py +++ b/ymir/supervisor/gitlab_utils.py @@ -3,6 +3,7 @@ from functools import cache from typing import Any from urllib.parse import quote as urlquote +from urllib.parse import urlparse from .http_utils import requests_session from .supervisor_types import MergeRequest, MergeRequestState @@ -11,6 +12,13 @@ GITLAB_URL = "https://gitlab.com" +# GitLab hosts Ymir is allowed to talk to. ``gitlab_api_get`` attaches the +# ``GITLAB_TOKEN`` Bearer credential to every request, so the target host must +# be constrained to trusted instances -- otherwise an attacker-controlled host +# (e.g. supplied through an LLM-authored ``blocker_reference``) would receive +# the token. This is the single source of truth for that allowlist. +ALLOWED_GITLAB_HOSTS = frozenset({"gitlab.com", "gitlab.cee.redhat.com"}) + @cache def gitlab_headers() -> dict[str, str]: @@ -22,8 +30,15 @@ def gitlab_headers() -> dict[str, str]: } -def gitlab_api_get(path: str, *, params: dict | None = None) -> Any: - url = f"{GITLAB_URL}/api/v4/{path}" +def gitlab_api_get(path: str, *, params: dict | None = None, gitlab_url: str = GITLAB_URL) -> Any: + hostname = urlparse(gitlab_url).hostname + if hostname not in ALLOWED_GITLAB_HOSTS: + # Never attach the Bearer token to an untrusted host. + raise ValueError( + f"Refusing to call GitLab API on untrusted host {hostname!r}; " + f"allowed hosts: {sorted(ALLOWED_GITLAB_HOSTS)}" + ) + url = f"{gitlab_url}/api/v4/{path}" response = requests_session().get(url, headers=gitlab_headers(), params=params) response.raise_for_status() return response.json() diff --git a/ymir/supervisor/tests/unit/test_gitlab_utils.py b/ymir/supervisor/tests/unit/test_gitlab_utils.py new file mode 100644 index 000000000..eda76857e --- /dev/null +++ b/ymir/supervisor/tests/unit/test_gitlab_utils.py @@ -0,0 +1,61 @@ +"""Unit tests for ymir.supervisor.gitlab_utils.""" + +import pytest + +from ymir.supervisor import gitlab_utils +from ymir.supervisor.gitlab_utils import ALLOWED_GITLAB_HOSTS, gitlab_api_get + + +@pytest.fixture(autouse=True) +def _fake_token(monkeypatch): + monkeypatch.setenv("GITLAB_TOKEN", "test-token") + gitlab_utils.gitlab_headers.cache_clear() + yield + gitlab_utils.gitlab_headers.cache_clear() + + +@pytest.mark.parametrize( + "gitlab_url", + [ + "https://gitlab.attacker.com", + "https://gitlab.com.attacker.com", + "https://evil.example.com", + "https://GITLAB.COM.attacker.com", + ], +) +def test_gitlab_api_get_rejects_untrusted_host(gitlab_url, monkeypatch): + """The Bearer token must never be attached to an untrusted host.""" + + def _fail(*_args, **_kwargs): + raise AssertionError("network request must not be made for untrusted host") + + monkeypatch.setattr(gitlab_utils, "requests_session", _fail) + + with pytest.raises(ValueError, match="untrusted host"): + gitlab_api_get("projects/1/merge_requests/1", gitlab_url=gitlab_url) + + +@pytest.mark.parametrize("host", sorted(ALLOWED_GITLAB_HOSTS)) +def test_gitlab_api_get_allows_trusted_hosts(host, monkeypatch): + captured = {} + + class _Resp: + def raise_for_status(self): + pass + + def json(self): + return {"state": "opened"} + + class _Session: + def get(self, url, headers=None, params=None): + captured["url"] = url + captured["headers"] = headers + return _Resp() + + monkeypatch.setattr(gitlab_utils, "requests_session", lambda: _Session()) + + result = gitlab_api_get("projects/1/merge_requests/1", gitlab_url=f"https://{host}") + + assert result == {"state": "opened"} + assert captured["url"] == f"https://{host}/api/v4/projects/1/merge_requests/1" + assert captured["headers"]["Authorization"] == "Bearer test-token" diff --git a/ymir/sweep/__init__.py b/ymir/sweep/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/ymir/sweep/__main__.py b/ymir/sweep/__main__.py new file mode 100644 index 000000000..3fae78ac4 --- /dev/null +++ b/ymir/sweep/__main__.py @@ -0,0 +1,79 @@ +"""Entry point for postponed-issue sweeps: ``python -m ymir.sweep``. + +Supports running a single strategy (``--strategy dependency``) for +per-strategy CronJobs, or all strategies in sequence (``--all``) for a +combined CronJob or local development. + +The sweep runs inside ``asyncio.run()`` so that both the HTTP session +ContextVar (required by ``jira_utils``) and async buildroot checks +(``check_build_in_buildroot``) are handled in the same event loop. +""" + +import argparse +import asyncio +import logging +import os +import sys + +from ymir.common.base_utils import redis_client +from ymir.supervisor.http_utils import with_requests_session +from ymir.sweep.dependency import DependencySweep +from ymir.sweep.no_patch import NoPatchSweep +from ymir.sweep.pr_pending import PRPendingSweep +from ymir.sweep.y_stream import YStreamSweep + +logging.basicConfig( + level=logging.INFO, + format="%(asctime)s [%(levelname)s] %(name)s %(message)s", + stream=sys.stdout, +) + +logger = logging.getLogger(__name__) + +STRATEGIES: dict = { + "dependency": DependencySweep, + "y_stream": YStreamSweep, + "pr_pending": PRPendingSweep, + "no_patch": NoPatchSweep, +} + + +async def run_sweep(strategy_names: list[str]) -> None: + """Run the specified sweep strategies in sequence. + + Sets up a shared ``requests.Session`` (required by ``jira_utils``) + and creates a single synchronous Redis connection shared across all + strategies. Strategies run sequentially to avoid concurrent Jira + API pressure. + """ + async with with_requests_session(), redis_client(os.environ["REDIS_URL"]) as redis: + for name in strategy_names: + strategy = STRATEGIES[name]() + logger.info("Starting %s sweep", name) + summary = await strategy.run(redis) + logger.info( + "%s sweep result: %s", + name, + ", ".join(f"{k}={v}" for k, v in summary.items()), + ) + + +if __name__ == "__main__": + parser = argparse.ArgumentParser(description="Run postponed-issue sweep") + group = parser.add_mutually_exclusive_group(required=True) + group.add_argument( + "--strategy", + choices=STRATEGIES.keys(), + help="Run a single strategy", + ) + group.add_argument( + "--all", + action="store_true", + help="Run all strategies in sequence", + ) + args = parser.parse_args() + + if args.all: + asyncio.run(run_sweep(list(STRATEGIES.keys()))) + else: + asyncio.run(run_sweep([args.strategy])) diff --git a/ymir/sweep/base.py b/ymir/sweep/base.py new file mode 100644 index 000000000..8df14e9ef --- /dev/null +++ b/ymir/sweep/base.py @@ -0,0 +1,203 @@ +"""SweepStrategy abstract base class for postponed-issue sweeps. + +Provides the shared orchestration loop (fetch → parse comment → check → +unblock/transition) so that concrete strategies only need to implement +``is_unblocked()``. All Jira operations go through +``ymir.supervisor.jira_utils`` (synchronous ``requests``-based client). +The MCP gateway is not involved. + +The sweep runs inside ``asyncio.run()`` so that ``check_build_in_buildroot`` +can be awaited directly. ``jira_utils`` calls remain synchronous; they +block the event loop briefly, which is acceptable for a batch CronJob. +""" + +import logging +from abc import ABC, abstractmethod +from dataclasses import dataclass +from typing import Literal + +import redis + +from ymir.common.base_utils import fix_await +from ymir.common.constants import JiraLabels, RedisQueues +from ymir.common.models import Task +from ymir.supervisor.jira_utils import ( + add_issue_label, + get_current_issues, + remove_issue_label, +) +from ymir.supervisor.supervisor_types import FullIssue +from ymir.sweep.comment_parser import CommentData, parse_ymir_comment + + +@dataclass +class SweepResult: + """Per-issue result from a sweep check.""" + + issue_key: str + action: Literal["unblocked", "transitioned", "still_blocked", "error"] + detail: str + + +class SweepStrategy(ABC): + """Base class for postponed-issue sweep strategies. + + Each subclass represents one postponement category. The base class + owns issue fetching, comment parsing, error handling, label management, + and logging. Subclasses implement only ``is_unblocked()``. + + Class attributes set by each subclass: + + ``name`` + Short identifier used in logging and CLI (e.g. ``"dependency"``). + ``label`` + The ``JiraLabels`` member that tags issues belonging to this + category (e.g. ``JiraLabels.YMIR_POSTPONED_DEPENDENCY``). + """ + + name: str + label: JiraLabels + + def __init__(self) -> None: + self.logger = logging.getLogger(f"ymir.sweep.{self.name}") + + def get_blocked_issues(self) -> list[FullIssue]: + """Query Jira for all issues with this strategy's postponement label. + + Returns ``FullIssue`` objects with comments, labels, and custom + fields decoded via ``jira_utils.get_current_issues(jql, full=True)``. + """ + jql = f'labels = "{self.label.value}"' + issues = list(get_current_issues(jql, full=True)) + self.logger.info("Found %d issues with label %s", len(issues), self.label.value) + return issues + + @abstractmethod + async def is_unblocked(self, issue: FullIssue, comment_data: CommentData) -> SweepResult: + """Check whether a single issue's blocking condition is resolved. + + Concrete strategies implement this method. When the blocking + condition is resolved, return ``SweepResult(action="unblocked", + ...)``. The ``detail`` field is posted as a Jira comment by + ``on_unblock()``. + + If a state *transition* is appropriate (e.g. a closed MR means + the issue should move from ``pr_pending`` to ``no_patch``), the + strategy calls ``self.on_transition()`` directly and returns + ``SweepResult(action="transitioned", ...)``. + + Args: + issue: Decoded Jira ``FullIssue``. + comment_data: Parsed blocker reference from the latest Ymir + comment. + """ + + async def on_unblock( + self, + issue_key: str, + redis_conn: redis.Redis, + comment: str | None = None, + ) -> None: + """Push issue to triage queue, then remove the postponement label. + + Redis is written first so that a subsequent Jira API failure leaves + the issue labelled (retried on the next sweep) rather than + unlabelled but missing from the queue. The comment is posted + atomically with the label removal when provided. + """ + task = Task.from_issue(issue_key) + await fix_await(redis_conn.lpush(RedisQueues.TRIAGE_QUEUE.value, task.to_json())) + add_issue_label(issue_key, JiraLabels.TRIAGE_IN_PROGRESS.value) + remove_issue_label(issue_key, self.label.value, comment=comment) + self.logger.info( + "Unblocked %s — removed %s, pushed to triage queue", + issue_key, + self.label.value, + ) + + def on_transition( + self, + issue_key: str, + new_label: JiraLabels, + comment: str | None = None, + ) -> None: + """Swap postponement label and post an optional comment. + + Adds the new label first so that a subsequent failure to remove the + old label leaves the issue discoverable by the new strategy. The + comment is posted atomically with the new label addition. + """ + add_issue_label(issue_key, new_label.value, comment=comment) + remove_issue_label(issue_key, self.label.value) + self.logger.info( + "Transitioned %s: %s -> %s", + issue_key, + self.label.value, + new_label.value, + ) + + async def run(self, redis_conn: redis.Redis) -> dict[str, int]: + """Execute a full sweep: fetch issues, check each, handle results. + + This is the shared orchestration logic. It handles comment + parsing, error handling per issue, and summary logging. Subclasses + do not override this — they only implement ``is_unblocked()``. + + ``redis.RedisError`` and ``OSError`` exceptions are re-raised to abort + the sweep (the CronJob's ``backoffLimit`` handles retries). + ``OSError`` surfaces configuration errors such as missing environment + variables. All other per-issue exceptions are caught, logged, and + counted as errors. + + Returns: + A summary dict with counts: ``total``, ``unblocked``, + ``transitioned``, ``errors``, ``still_blocked``. + """ + issues = self.get_blocked_issues() + total = len(issues) + unblocked = 0 + transitioned = 0 + errors = 0 + + for issue in issues: + issue_key = issue.key + try: + comment_data = parse_ymir_comment(issue) + if not comment_data: + self.logger.warning("No parseable Ymir comment on %s, skipping", issue_key) + errors += 1 + continue + + result = await self.is_unblocked(issue, comment_data) + + if result.action == "unblocked": + await self.on_unblock(issue_key, redis_conn, comment=result.detail) + unblocked += 1 + elif result.action == "transitioned": + transitioned += 1 + elif result.action == "error": + self.logger.warning("Check error on %s: %s", issue_key, result.detail) + errors += 1 + except (redis.RedisError, OSError): + raise + except Exception: + self.logger.exception("Unhandled error checking %s", issue_key) + errors += 1 + + still_blocked = total - unblocked - transitioned - errors + self.logger.info( + "%s sweep complete: %d total, %d unblocked, %d transitioned, %d errors, %d still blocked", + self.name, + total, + unblocked, + transitioned, + errors, + still_blocked, + ) + return { + "total": total, + "unblocked": unblocked, + "transitioned": transitioned, + "errors": errors, + "still_blocked": still_blocked, + } diff --git a/ymir/sweep/comment_parser.py b/ymir/sweep/comment_parser.py new file mode 100644 index 000000000..8a1a1c407 --- /dev/null +++ b/ymir/sweep/comment_parser.py @@ -0,0 +1,82 @@ +"""Parse blocker references from Ymir triage comments on Jira issues. + +Extracts structured postponement data (blocker reference, pending issues) +from the machine-readable fields that the triage agent writes into Jira +comments via ``format_for_comment()``. + +The postponement *reason* is not read from the comment: sweeps select +issues by their ``ymir_postponed_*`` label (see ``SweepStrategy``), so the +label — not a comment field — is the authoritative category signal. +""" + +import os +import re +from dataclasses import dataclass + +from ymir.supervisor.supervisor_types import FullIssue + +_YMIR_TRIAGE_AGENT_COMMENT_MARKER = "Output from Ymir Triage Agent" + +_BLOCKER_RE = re.compile(r"^\*Blockers?\*:\s*(.+)$", re.MULTILINE) +_PENDING_ISSUE_RE = re.compile(r"^\* ([A-Z]+-\d+)$", re.MULTILINE) +_SUMMARY_RE = re.compile(r"^\*Summary\*:\s*(.+)$", re.MULTILINE) + + +@dataclass +class CommentData: + """Structured data extracted from a Ymir triage comment.""" + + blocker_references: list[str] | None + pending_issues: list[str] + summary: str | None + comment_id: str + + +def parse_ymir_comment(issue: FullIssue) -> CommentData | None: + """Extract blocker reference from the latest Ymir comment on an issue. + + Searches the issue's comments (``JiraComment`` objects with ``.body``, + ``.id``, ``.authorName``, ``.created`` fields) in reverse chronological + order for the latest one containing the Ymir triage marker. Extracts + structured fields via regex from the ``*Blocker*:``, ``*Waiting for*:`` + / ``*Waiting for at least one of*:``, and ``*Summary*:`` lines. + + Args: + issue: Decoded Jira ``FullIssue`` from + ``jira_utils.get_issue(key, full=True)``. + + Returns: + ``CommentData`` with blocker_references, pending_issues, summary, and + comment_id. Returns ``None`` if no Ymir comment is found. Fields + that are absent from the comment are ``None``/empty; each sweep + strategy validates the fields it needs. + """ + jira_email = os.environ.get("JIRA_EMAIL") + if not jira_email: + raise OSError("JIRA_EMAIL environment variable is not set") + + ymir_comment = None + for comment in reversed(issue.comments): + if _YMIR_TRIAGE_AGENT_COMMENT_MARKER in comment.body and comment.authorEmail == jira_email: + ymir_comment = comment + break + + if ymir_comment is None: + return None + + body = ymir_comment.body + + blocker_match = _BLOCKER_RE.search(body) + blocker_references = [b.strip() for b in blocker_match.group(1).split(",")] if blocker_match else None + + pending_issues = _PENDING_ISSUE_RE.findall(body) + + summary_match = _SUMMARY_RE.search(body) + summary = summary_match.group(1).strip() if summary_match else None + + return CommentData( + blocker_references=blocker_references, + pending_issues=pending_issues, + summary=summary, + comment_id=ymir_comment.id, + ) diff --git a/ymir/sweep/dependency.py b/ymir/sweep/dependency.py new file mode 100644 index 000000000..99168360f --- /dev/null +++ b/ymir/sweep/dependency.py @@ -0,0 +1,161 @@ +"""Dependency sweep strategy. + +Checks whether the dependency's fixed build is present in the Y-stream +buildroot. Uses ``jira_utils.get_issue()`` for blocker lookup and +``check_build_in_buildroot()`` for buildroot verification. + +Handles issues tagged ``ymir_postponed_dependency``, which are set when +the rebuild triage agent finds that a dependency's fixed build has not +yet landed in the target buildroot. +""" + +import re + +import requests + +from ymir.common.config import load_rhel_config +from ymir.common.constants import JiraLabels +from ymir.common.utils import check_build_in_buildroot +from ymir.common.version_utils import normalize_fix_version, parse_rhel_version +from ymir.supervisor.jira_utils import get_issue +from ymir.supervisor.supervisor_types import FullIssue +from ymir.sweep.base import SweepResult, SweepStrategy +from ymir.sweep.comment_parser import CommentData + +JIRA_KEY_RE = re.compile(r"^[A-Z]+-\d+$") +# Matches "to land in c9s buildroot" or "to land in rhel-9.8.0 buildroot" +_BRANCH_FROM_SUMMARY_RE = re.compile(r"to land in (\S+) buildroot") + + +def branch_from_fix_versions(fix_versions: list[str]) -> str | None: + """Derive a CentOS Stream dist-git branch from an issue's fix_versions. + + Parses the first recognisable RHEL version string (e.g. ``rhel-9.6.0``) + and maps it to the corresponding CS branch (e.g. ``c9s``). + """ + for fv in fix_versions: + parsed = parse_rhel_version(fv) + if parsed: + major, _, _ = parsed + return f"c{major}s" + return None + + +def _resolve_target_branch(comment_data: CommentData, fix_versions: list[str]) -> str | None: + """Return the target branch for the buildroot check. + + Tries to extract the branch from the ``*Summary*:`` line of the Ymir + comment (format: ``"...to land in buildroot"``), then falls + back to deriving it from the issue's fix_versions. + """ + if comment_data.summary: + m = _BRANCH_FROM_SUMMARY_RE.search(comment_data.summary) + if m: + return m.group(1) + return branch_from_fix_versions(fix_versions) + + +class DependencySweep(SweepStrategy): + """Checks whether the dependency's fixed build is present in the buildroot. + + For each issue the strategy: + + 1. Reads the blocker Jira issue key from the comment's ``*Blockers*:`` + line (or the first ``*Waiting for*:`` entry). + 2. Fetches the blocker issue via ``jira_utils.get_issue()`` and reads + its ``fixed_in_build`` field. + 3. If ``fixed_in_build`` is absent → still blocked. + 4. Derives ``target_branch`` from the comment summary or issue's + ``fix_versions`` field. + 5. Reads ``dep_component`` from the blocker's ``components`` list. + 6. Calls ``check_build_in_buildroot()`` to verify availability. + 7. If the build is present → unblocks the issue. + """ + + name = "dependency" + label = JiraLabels.YMIR_POSTPONED_DEPENDENCY + + async def is_unblocked(self, issue: FullIssue, comment_data: CommentData) -> SweepResult: + + # Resolve blocker key from comment (prefer explicit Blockers line; + # fall back to first pending issue entry). + blocker_key = comment_data.blocker_references[0] if comment_data.blocker_references else None + if not blocker_key and comment_data.pending_issues: + blocker_key = comment_data.pending_issues[0] + + if not blocker_key or not JIRA_KEY_RE.match(blocker_key): + return SweepResult( + issue_key=issue.key, + action="error", + detail=f"No valid Jira blocker key found in comment for {issue.key}", + ) + + try: + blocker = get_issue(blocker_key) + except requests.HTTPError as exc: + return SweepResult( + issue_key=issue.key, + action="error", + detail=f"Failed to fetch blocker {blocker_key}: {exc}", + ) + + if not blocker.fixed_in_build: + return SweepResult( + issue_key=issue.key, + action="still_blocked", + detail=f"{blocker_key} has no Fixed in Build yet", + ) + + if not blocker.components: + return SweepResult( + issue_key=issue.key, + action="error", + detail=f"Blocker {blocker_key} has no components — cannot determine dep_component", + ) + dep_component = blocker.components[0] + + target_branch = _resolve_target_branch(comment_data, issue.fix_versions) + if not target_branch: + return SweepResult( + issue_key=issue.key, + action="error", + detail=f"Cannot determine target branch for {issue.key} (fix_versions={issue.fix_versions})", + ) + + raw_fix_version = issue.fix_versions[0] if issue.fix_versions else "" + if raw_fix_version: + rhel_config = await load_rhel_config() + fix_version = normalize_fix_version(raw_fix_version, rhel_config) + else: + fix_version = raw_fix_version + + try: + in_buildroot = await check_build_in_buildroot( + target_branch, + dep_component, + blocker.fixed_in_build, + fix_version=fix_version, + ) + except Exception as exc: + return SweepResult( + issue_key=issue.key, + action="error", + detail=(f"Buildroot check failed for {dep_component} ({blocker.fixed_in_build}): {exc}"), + ) + + if in_buildroot: + return SweepResult( + issue_key=issue.key, + action="unblocked", + detail=( + f"Dependency {blocker_key} now has Fixed in Build " + f"({blocker.fixed_in_build}), confirmed present in " + f"{target_branch} buildroot. Re-triaging." + ), + ) + + return SweepResult( + issue_key=issue.key, + action="still_blocked", + detail=(f"{blocker.fixed_in_build} not yet in {target_branch} buildroot"), + ) diff --git a/ymir/sweep/no_patch.py b/ymir/sweep/no_patch.py new file mode 100644 index 000000000..9f57f7d2b --- /dev/null +++ b/ymir/sweep/no_patch.py @@ -0,0 +1,113 @@ +"""No-patch sweep strategy. + +Pushes postponed issues back to the triage queue for fresh evaluation by +the triage agent, subject to a ``MAX_ISSUES_PER_RUN`` cap. The sweep +itself does not check a blocking condition — the triage agent decides +whether a patch is now available. + +Handles issues tagged ``ymir_postponed_no_patch``. + +Guardrails: + - ``MAX_ISSUES_PER_RUN`` env var (default: 20) caps how many issues are + pushed per run. The remainder are checked on the next sweep. + - Issues already being triaged (``ymir_triage_in_progress`` label) are + skipped to avoid duplicates. +""" + +import os + +import redis + +from ymir.common.base_utils import fix_await +from ymir.common.constants import JiraLabels, RedisQueues +from ymir.common.models import Task +from ymir.supervisor.jira_utils import add_issue_label, remove_issue_label +from ymir.supervisor.supervisor_types import FullIssue +from ymir.sweep.base import SweepResult, SweepStrategy +from ymir.sweep.comment_parser import CommentData + +_MAX_ISSUES_DEFAULT = 20 + + +class NoPatchSweep(SweepStrategy): + """Pushes no-patch issues back to the triage queue for re-evaluation. + + Overrides ``run()`` because the base class's comment-parsing loop + does not fit this strategy: there is no blocking condition to check + per issue; instead the strategy unconditionally re-queues eligible + issues up to the configured cap. + """ + + name = "no_patch" + label = JiraLabels.YMIR_POSTPONED_NO_PATCH + + async def is_unblocked(self, issue: FullIssue, comment_data: CommentData) -> SweepResult: + # Not used — run() is overridden. + raise NotImplementedError("NoPatchSweep overrides run() directly") + + async def run(self, redis_conn: redis.Redis) -> dict[str, int]: + """Re-queue eligible no-patch issues for fresh triage. + + Issues already in triage (``ymir_triage_in_progress`` label) are + skipped. At most ``MAX_ISSUES_PER_RUN`` issues are pushed in a + single run; the rest wait for the next scheduled sweep. + + ``redis.RedisError`` is re-raised to abort the run (the CronJob's + ``backoffLimit`` handles retries). + + Returns: + A summary dict with counts compatible with the base class's + ``run()`` return format. + """ + max_issues = int(os.environ.get("MAX_ISSUES_PER_RUN", _MAX_ISSUES_DEFAULT)) + all_issues = self.get_blocked_issues() + + in_progress_count = 0 + eligible: list[FullIssue] = [] + for issue in all_issues: + if JiraLabels.TRIAGE_IN_PROGRESS.value in issue.labels: + in_progress_count += 1 + else: + eligible.append(issue) + + if in_progress_count: + self.logger.info("Skipping %d issues already in triage", in_progress_count) + + to_process = eligible[:max_issues] + capped = len(eligible) - len(to_process) + unblocked = 0 + errors = 0 + + for issue in to_process: + issue_key = issue.key + try: + task = Task.from_issue(issue_key) + await fix_await(redis_conn.lpush(RedisQueues.TRIAGE_QUEUE.value, task.to_json())) + add_issue_label(issue_key, JiraLabels.TRIAGE_IN_PROGRESS.value) + remove_issue_label(issue_key, self.label.value) + self.logger.info("Re-queued %s for fresh triage", issue_key) + unblocked += 1 + except redis.RedisError: + raise + except Exception: + self.logger.exception("Error re-queuing %s", issue_key) + errors += 1 + + still_blocked = in_progress_count + capped + self.logger.info( + "%s sweep complete: %d total, %d re-queued, %d capped (next run), " + "%d already in triage, %d errors", + self.name, + len(all_issues), + unblocked, + capped, + in_progress_count, + errors, + ) + return { + "total": len(all_issues), + "unblocked": unblocked, + "transitioned": 0, + "errors": errors, + "still_blocked": still_blocked, + } diff --git a/ymir/sweep/pr_pending.py b/ymir/sweep/pr_pending.py new file mode 100644 index 000000000..cebbe9461 --- /dev/null +++ b/ymir/sweep/pr_pending.py @@ -0,0 +1,157 @@ +"""PR-pending sweep strategy. + +Checks whether the identified upstream pull/merge request has been merged. +Supports both GitLab MRs (via the GitLab REST API) and GitHub PRs (via the +GitHub REST API). + +GitLab: uses ``gitlab_utils.gitlab_api_get()`` with the ``gitlab_url`` +parameter to support both ``gitlab.com`` and internal hosts such as +``gitlab.cee.redhat.com``. + +GitHub: uses ``github_utils.github_api_get()``. Authentication is +optional (set ``GITHUB_TOKEN`` ); all upstream +projects tracked by Ymir are public, so unauthenticated access suffices. + +Handles issues tagged ``ymir_postponed_pr_pending``. + +State transitions: + - PR/MR ``merged`` → unblock (remove label, push to triage queue) + - PR/MR ``closed`` → transition to ``ymir_postponed_no_patch`` (the + fix is no longer coming via this PR/MR) + - PR/MR ``opened`` → still blocked, no action +""" + +import re +from urllib.parse import quote as urlquote + +from ymir.common.constants import JiraLabels +from ymir.supervisor.github_utils import github_api_get +from ymir.supervisor.gitlab_utils import ALLOWED_GITLAB_HOSTS, gitlab_api_get +from ymir.supervisor.supervisor_types import FullIssue, MergeRequestState +from ymir.sweep.base import SweepResult, SweepStrategy +from ymir.sweep.comment_parser import CommentData + +# Only match MR URLs on trusted GitLab hosts. ``blocker_references`` values are +# LLM-authored and untrusted, and the extracted host is used to build an +# authenticated API call; restricting the host here (in addition to the guard +# in ``gitlab_api_get``) yields a clean "not recognisable" error rather than an +# exception for hostile URLs. Longest host first so alternation is unambiguous. +_GITLAB_HOSTS_ALT = "|".join(re.escape(h) for h in sorted(ALLOWED_GITLAB_HOSTS, key=len, reverse=True)) +_GITLAB_MR_RE = re.compile(rf"https://({_GITLAB_HOSTS_ALT})/(.+?)/-/merge_requests/(\d+)") +_GITHUB_PR_RE = re.compile(r"https://github\.com/([^/]+/[^/]+)/pull/(\d+)") + + +def _fetch_gitlab_mr_state(mr_url: str, m: re.Match) -> tuple[str, str | None]: + """Return ``(state, error_detail)`` for a GitLab MR. error_detail is None on success.""" + host, project_path, mr_iid = m.group(1), m.group(2), m.group(3) + path = f"projects/{urlquote(project_path, safe='')}/merge_requests/{mr_iid}" + try: + data = gitlab_api_get(path, gitlab_url=f"https://{host}") + except Exception as exc: + return "", f"GitLab API call failed for {mr_url}: {exc}" + return data.get("state", ""), None + + +def _fetch_github_pr_state(pr_url: str, m: re.Match) -> tuple[str, str | None]: + """Return ``(state, error_detail)`` for a GitHub PR. error_detail is None on success. + + GitHub PRs use ``state: open|closed`` plus ``merged_at`` to distinguish a + merged close from an abandoned close. The returned state is normalised to + the same values ``MergeRequestState`` uses (``opened``, ``merged``, + ``closed``) so downstream logic is identical for both platforms. + """ + owner_repo, pr_number = m.group(1), m.group(2) + try: + data = github_api_get(f"repos/{owner_repo}/pulls/{pr_number}") + except Exception as exc: + return "", f"GitHub API call failed for {pr_url}: {exc}" + + gh_state = data.get("state", "") + if gh_state == "open": + return MergeRequestState.OPEN, None + if gh_state == "closed": + if data.get("merged_at"): + return MergeRequestState.MERGED, None + return MergeRequestState.CLOSED, None + # Unknown state — pass through; the caller maps it to still_blocked. + return gh_state, None + + +class PRPendingSweep(SweepStrategy): + """Checks whether the upstream pull/merge request has been merged. + + For each issue the strategy: + + 1. Reads the PR/MR URL from the comment's ``*Blockers*:`` line. + 2. Detects the platform from the URL (GitLab or GitHub). + 3. Fetches the PR/MR state via the appropriate REST API. + 4. Acts on the state: + - ``merged`` → unblock + - ``closed`` → transition to ``ymir_postponed_no_patch`` + - ``opened`` → still blocked + """ + + name = "pr_pending" + label = JiraLabels.YMIR_POSTPONED_PR_PENDING + + async def is_unblocked(self, issue: FullIssue, comment_data: CommentData) -> SweepResult: + issue_key = issue.key + + pr_url = comment_data.blocker_references[0] if comment_data.blocker_references else None + if not pr_url: + return SweepResult( + issue_key=issue_key, + action="error", + detail=f"No *Blockers*: PR/MR URL found in comment for {issue_key}", + ) + + gl_match = _GITLAB_MR_RE.match(pr_url) + gh_match = _GITHUB_PR_RE.match(pr_url) + + if gl_match: + state, error = _fetch_gitlab_mr_state(pr_url, gl_match) + elif gh_match: + state, error = _fetch_github_pr_state(pr_url, gh_match) + else: + return SweepResult( + issue_key=issue_key, + action="error", + detail=( + f"Blocker reference is not a recognisable GitLab MR or " + f"GitHub PR URL for {issue_key}: {pr_url!r}" + ), + ) + + if error: + return SweepResult(issue_key=issue_key, action="error", detail=error) + + if state == MergeRequestState.MERGED: + return SweepResult( + issue_key=issue_key, + action="unblocked", + detail=f"Upstream PR/MR {pr_url} has been merged. Re-triaging.", + ) + + if state == MergeRequestState.CLOSED: + # PR/MR was abandoned — no fix is coming via this URL any more. + # Transition to no_patch so the no-patch sweep re-evaluates. + self.on_transition( + issue_key, + JiraLabels.YMIR_POSTPONED_NO_PATCH, + comment=( + f"Upstream PR/MR {pr_url} was closed without merging. " + "Transitioning to no-patch state for re-evaluation." + ), + ) + return SweepResult( + issue_key=issue_key, + action="transitioned", + detail=f"PR/MR {pr_url} closed — transitioned to no_patch", + ) + + # PR/MR is still open (or in an unexpected state). + return SweepResult( + issue_key=issue_key, + action="still_blocked", + detail=f"PR/MR {pr_url} is still {state!r}", + ) diff --git a/ymir/sweep/tests/__init__.py b/ymir/sweep/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/ymir/sweep/tests/integration/__init__.py b/ymir/sweep/tests/integration/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/ymir/sweep/tests/integration/conftest.py b/ymir/sweep/tests/integration/conftest.py new file mode 100644 index 000000000..e42c58a62 --- /dev/null +++ b/ymir/sweep/tests/integration/conftest.py @@ -0,0 +1,87 @@ +"""Shared fixtures and helpers for sweep integration tests.""" + +import pytest +from flexmock import flexmock + +from ymir.sweep.tests.unit.conftest import ( # noqa: F401 + _DEFAULT_RHEL_CONFIG, + make_issue, + make_ymir_comment, +) + + +@pytest.fixture(autouse=True) +def _inject_jira_email(monkeypatch): + """Inject JIRA_EMAIL for all sweep integration tests. + + parse_ymir_comment() reads JIRA_EMAIL at call time and raises OSError + when it is absent. The value must match the authorEmail set by + make_ymir_comment() so that comment look-ups succeed. + """ + monkeypatch.setenv("JIRA_EMAIL", "ymir@redhat.com") + + +@pytest.fixture(autouse=True) +def _stub_load_rhel_config(monkeypatch): + """Stub load_rhel_config for all sweep integration tests. + + DependencySweep calls load_rhel_config() to normalise fix_version before + the buildroot check. Integration tests don't have a real + rhel-config.json on disk, so we return the same default config used by + the unit tests. + + (YStreamSweep no longer calls load_rhel_config — it delegates to the + eligibility tool — so only the dependency module is patched here.) + """ + + async def _default_config(): + return _DEFAULT_RHEL_CONFIG + + monkeypatch.setattr("ymir.sweep.dependency.load_rhel_config", _default_config) + + +@pytest.fixture +def mock_env(monkeypatch): + """Set minimal required environment variables.""" + monkeypatch.setenv("JIRA_URL", "https://jira.example.com") + monkeypatch.setenv("JIRA_EMAIL", "ymir@redhat.com") + monkeypatch.setenv("JIRA_TOKEN", "test-token") + monkeypatch.setenv("REDIS_URL", "redis://localhost:6379") + monkeypatch.setenv("GITLAB_TOKEN", "test-gitlab-token") + + +@pytest.fixture +def captured_label_ops(monkeypatch): + """Patch label operations in base and no_patch modules; return captured calls. + + Returns a dict with keys ``"remove"`` and ``"add"``, each a list of + ``(issue_key, label)`` tuples recorded in call order. Patches both + ``ymir.sweep.base`` (used by ``on_unblock``/``on_transition``) and + ``ymir.sweep.no_patch`` (which calls ``remove_issue_label`` directly). + """ + ops: dict[str, list[tuple[str, str]]] = {"remove": [], "add": []} + + def capture_remove(key: str, label: str, comment: str | None = None) -> None: + ops["remove"].append((key, label)) + + def capture_add(key: str, label: str, comment: str | None = None) -> None: + ops["add"].append((key, label)) + + monkeypatch.setattr("ymir.sweep.base.remove_issue_label", capture_remove) + monkeypatch.setattr("ymir.sweep.base.add_issue_label", capture_add) + monkeypatch.setattr("ymir.sweep.no_patch.remove_issue_label", capture_remove) + monkeypatch.setattr("ymir.sweep.no_patch.add_issue_label", capture_add) + return ops + + +def make_redis() -> tuple: + """Return a (redis_mock, pushed_list) pair for asserting Redis side effects. + + ``pushed_list`` accumulates the queue name on each ``lpush`` call so tests + can assert both call count and the target queue without flexmock expectation + matching. + """ + pushed: list[str] = [] + r = flexmock() + r.should_receive("lpush").replace_with(lambda queue, payload: pushed.append(queue)) + return r, pushed diff --git a/ymir/sweep/tests/integration/test_sweep_runner.py b/ymir/sweep/tests/integration/test_sweep_runner.py new file mode 100644 index 000000000..f40cd723a --- /dev/null +++ b/ymir/sweep/tests/integration/test_sweep_runner.py @@ -0,0 +1,352 @@ +"""Integration tests for the sweep pipeline. + +Each test calls ``strategy.run(redis_conn)`` on a *real* strategy instance +with all external I/O (Jira, GitLab, buildroot) replaced by monkeypatched +stubs. Unlike the unit tests, which test individual components in isolation, +these tests verify the full cycle: + + issue fetching → comment parsing → unblock/transition check + → label removal and Redis push + +The real ``parse_ymir_comment()`` runs on fixture comments built by +``make_ymir_comment()``, so the parser and the comment format are exercised +together. +""" + +import pytest +import requests +from flexmock import flexmock + +from ymir.common import CVEEligibilityResult, TriageEligibility +from ymir.common.constants import JiraLabels, RedisQueues +from ymir.supervisor.supervisor_types import Issue, IssueStatus +from ymir.sweep.dependency import DependencySweep +from ymir.sweep.no_patch import NoPatchSweep +from ymir.sweep.pr_pending import PRPendingSweep +from ymir.sweep.tests.integration.conftest import make_redis +from ymir.sweep.tests.unit.conftest import make_issue, make_ymir_comment +from ymir.sweep.y_stream import YStreamSweep + +# --------------------------------------------------------------------------- +# Async helpers +# --------------------------------------------------------------------------- + + +async def _async_true(*a, **kw) -> bool: + return True + + +async def _async_false(*a, **kw) -> bool: + return False + + +def _fake_eligibility_tool(eligibility, *, reason="reason", error=None, pending=None): + """Return a fake ``CheckCveTriageEligibilityTool`` whose ``run()`` yields + the given eligibility verdict (mirroring ``JSONToolOutput.result``).""" + result_dict = CVEEligibilityResult( + is_cve=True, + eligibility=eligibility, + reason=reason, + error=error, + pending_zstream_issues=pending, + ).model_dump() + + class _Output: + result = result_dict + + class _Tool: + async def run(self, input): + return _Output() + + return _Tool + + +# --------------------------------------------------------------------------- +# Fixture builders +# --------------------------------------------------------------------------- + + +def _make_blocker( + key: str = "RHEL-67890", + fixed_in_build: str | None = None, + components: list[str] | None = None, +) -> Issue: + return Issue( + key=key, + url=f"https://jira.example.com/browse/{key}", + summary="Blocker issue", + components=components if components is not None else ["golang"], + status=IssueStatus.IN_PROGRESS, + labels=[], + fix_versions=["rhel-9.7.z"], + errata_link=None, + fixed_in_build=fixed_in_build, + ) + + +def _dependency_issue(blocker_key: str = "RHEL-67890"): + comment = make_ymir_comment( + summary="Rebuild of pkg waiting for dep to land in c9s buildroot", + pending_issues=[blocker_key], + blocker_reference=blocker_key, + ) + return make_issue( + labels=[JiraLabels.YMIR_POSTPONED_DEPENDENCY.value], + comments=[comment], + ) + + +def _y_stream_issue(blocker_keys: list[str]): + comment = make_ymir_comment( + summary="Y-stream CVE waiting for Z-stream dependencies", + pending_issues=blocker_keys, + blocker_reference=blocker_keys[0] if len(blocker_keys) == 1 else None, + ) + return make_issue( + labels=[JiraLabels.YMIR_POSTPONED_Y_STREAM.value], + comments=[comment], + ) + + +_MR_URL = "https://gitlab.com/redhat/centos-stream/rpms/pkg/-/merge_requests/42" + + +def _pr_pending_issue(mr_url: str = _MR_URL): + comment = make_ymir_comment( + summary="Upstream patch not yet merged", + pending_issues=["RHEL-12345"], + blocker_reference=mr_url, + ) + return make_issue( + labels=[JiraLabels.YMIR_POSTPONED_PR_PENDING.value], + comments=[comment], + ) + + +# --------------------------------------------------------------------------- +# DependencySweep +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_dependency_sweep_unblocks_when_build_in_buildroot(monkeypatch, captured_label_ops): + issue = _dependency_issue() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + monkeypatch.setattr( + "ymir.sweep.dependency.get_issue", + lambda key, full=False: _make_blocker(fixed_in_build="golang-1.21.0-1.el9"), + ) + monkeypatch.setattr("ymir.sweep.dependency.check_build_in_buildroot", _async_true) + mock_redis, pushed = make_redis() + + summary = await DependencySweep().run(mock_redis) + + assert summary["total"] == 1 + assert summary["unblocked"] == 1 + assert summary["errors"] == 0 + assert summary["still_blocked"] == 0 + assert pushed == [RedisQueues.TRIAGE_QUEUE.value] + assert (issue.key, JiraLabels.YMIR_POSTPONED_DEPENDENCY.value) in captured_label_ops["remove"] + + +@pytest.mark.asyncio +async def test_dependency_sweep_still_blocked_when_no_fixed_in_build(monkeypatch, captured_label_ops): + issue = _dependency_issue() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + monkeypatch.setattr( + "ymir.sweep.dependency.get_issue", + lambda key, full=False: _make_blocker(fixed_in_build=None), + ) + mock_redis, pushed = make_redis() + + summary = await DependencySweep().run(mock_redis) + + assert summary["still_blocked"] == 1 + assert summary["unblocked"] == 0 + assert pushed == [] + assert captured_label_ops["remove"] == [] + + +@pytest.mark.asyncio +async def test_dependency_sweep_still_blocked_when_build_not_in_buildroot(monkeypatch, captured_label_ops): + issue = _dependency_issue() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + monkeypatch.setattr( + "ymir.sweep.dependency.get_issue", + lambda key, full=False: _make_blocker(fixed_in_build="golang-1.21.0-1.el9"), + ) + monkeypatch.setattr("ymir.sweep.dependency.check_build_in_buildroot", _async_false) + mock_redis, pushed = make_redis() + + summary = await DependencySweep().run(mock_redis) + + assert summary["still_blocked"] == 1 + assert summary["unblocked"] == 0 + assert pushed == [] + + +@pytest.mark.asyncio +async def test_dependency_sweep_counts_error_on_blocker_fetch_failure(monkeypatch, captured_label_ops): + issue = _dependency_issue() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + + def raise_http(key, full=False): + resp = flexmock(status_code=404) + raise requests.HTTPError("Not found", response=resp) + + monkeypatch.setattr("ymir.sweep.dependency.get_issue", raise_http) + mock_redis, pushed = make_redis() + + summary = await DependencySweep().run(mock_redis) + + assert summary["errors"] == 1 + assert pushed == [] + + +# --------------------------------------------------------------------------- +# YStreamSweep +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_y_stream_sweep_unblocks_when_no_longer_pending(monkeypatch, captured_label_ops): + issue = _y_stream_issue(["RHEL-11111", "RHEL-22222"]) + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + monkeypatch.setattr( + "ymir.sweep.y_stream.CheckCveTriageEligibilityTool", + _fake_eligibility_tool(TriageEligibility.IMMEDIATELY, reason="at least one clone shipped"), + ) + mock_redis, pushed = make_redis() + + summary = await YStreamSweep().run(mock_redis) + + assert summary["unblocked"] == 1 + assert pushed == [RedisQueues.TRIAGE_QUEUE.value] + assert (issue.key, JiraLabels.YMIR_POSTPONED_Y_STREAM.value) in captured_label_ops["remove"] + + +@pytest.mark.asyncio +async def test_y_stream_sweep_still_blocked_when_pending(monkeypatch, captured_label_ops): + issue = _y_stream_issue(["RHEL-11111", "RHEL-22222"]) + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + monkeypatch.setattr( + "ymir.sweep.y_stream.CheckCveTriageEligibilityTool", + _fake_eligibility_tool( + TriageEligibility.PENDING_DEPENDENCIES, + reason="waiting for Z-stream clone to ship", + pending=["RHEL-22222"], + ), + ) + mock_redis, pushed = make_redis() + + summary = await YStreamSweep().run(mock_redis) + + assert summary["still_blocked"] == 1 + assert summary["unblocked"] == 0 + assert pushed == [] + + +# --------------------------------------------------------------------------- +# PRPendingSweep +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_pr_pending_sweep_unblocks_when_mr_merged(monkeypatch, captured_label_ops): + issue = _pr_pending_issue() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + monkeypatch.setattr( + "ymir.sweep.pr_pending.gitlab_api_get", + lambda path, *, gitlab_url=None, params=None: {"state": "merged"}, + ) + mock_redis, pushed = make_redis() + + summary = await PRPendingSweep().run(mock_redis) + + assert summary["unblocked"] == 1 + assert pushed == [RedisQueues.TRIAGE_QUEUE.value] + assert (issue.key, JiraLabels.YMIR_POSTPONED_PR_PENDING.value) in captured_label_ops["remove"] + + +@pytest.mark.asyncio +async def test_pr_pending_sweep_transitions_to_no_patch_when_mr_closed(monkeypatch, captured_label_ops): + issue = _pr_pending_issue() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + monkeypatch.setattr( + "ymir.sweep.pr_pending.gitlab_api_get", + lambda path, *, gitlab_url=None, params=None: {"state": "closed"}, + ) + mock_redis, pushed = make_redis() + + summary = await PRPendingSweep().run(mock_redis) + + assert summary["transitioned"] == 1 + assert pushed == [] + # New label is added before old label is removed (safety invariant from base.on_transition) + assert captured_label_ops["add"][0] == (issue.key, JiraLabels.YMIR_POSTPONED_NO_PATCH.value) + assert (issue.key, JiraLabels.YMIR_POSTPONED_PR_PENDING.value) in captured_label_ops["remove"] + + +@pytest.mark.asyncio +async def test_pr_pending_sweep_still_blocked_when_mr_open(monkeypatch, captured_label_ops): + issue = _pr_pending_issue() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + monkeypatch.setattr( + "ymir.sweep.pr_pending.gitlab_api_get", + lambda path, *, gitlab_url=None, params=None: {"state": "opened"}, + ) + mock_redis, pushed = make_redis() + + summary = await PRPendingSweep().run(mock_redis) + + assert summary["still_blocked"] == 1 + assert pushed == [] + assert captured_label_ops["remove"] == [] + + +# --------------------------------------------------------------------------- +# NoPatchSweep +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_no_patch_sweep_requeues_up_to_cap(monkeypatch, captured_label_ops): + monkeypatch.setenv("MAX_ISSUES_PER_RUN", "2") + issues = [ + make_issue(key=f"RHEL-{i}", labels=[JiraLabels.YMIR_POSTPONED_NO_PATCH.value]) for i in range(3) + ] + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter(issues)) + mock_redis, pushed = make_redis() + + summary = await NoPatchSweep().run(mock_redis) + + assert summary["total"] == 3 + assert summary["unblocked"] == 2 + assert summary["still_blocked"] == 1 + assert len(pushed) == 2 + assert all(q == RedisQueues.TRIAGE_QUEUE.value for q in pushed) + assert len(captured_label_ops["remove"]) == 2 + + +@pytest.mark.asyncio +async def test_no_patch_sweep_skips_in_progress_issues(monkeypatch, captured_label_ops): + in_progress = make_issue( + key="RHEL-100", + labels=[JiraLabels.TRIAGE_IN_PROGRESS.value, JiraLabels.YMIR_POSTPONED_NO_PATCH.value], + ) + eligible = make_issue(key="RHEL-101", labels=[JiraLabels.YMIR_POSTPONED_NO_PATCH.value]) + monkeypatch.setattr( + "ymir.sweep.base.get_current_issues", + lambda jql, full=False: iter([in_progress, eligible]), + ) + mock_redis, pushed = make_redis() + + summary = await NoPatchSweep().run(mock_redis) + + assert summary["total"] == 2 + assert summary["unblocked"] == 1 + assert summary["still_blocked"] == 1 + assert len(pushed) == 1 + removed_keys = [key for key, _ in captured_label_ops["remove"]] + assert "RHEL-100" not in removed_keys + assert "RHEL-101" in removed_keys diff --git a/ymir/sweep/tests/unit/__init__.py b/ymir/sweep/tests/unit/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/ymir/sweep/tests/unit/conftest.py b/ymir/sweep/tests/unit/conftest.py new file mode 100644 index 000000000..e607b747b --- /dev/null +++ b/ymir/sweep/tests/unit/conftest.py @@ -0,0 +1,122 @@ +"""Shared fixtures for sweep unit tests.""" + +from datetime import datetime + +import pytest + +from ymir.agents.constants import JIRA_COMMENT_TEMPLATE +from ymir.common.models import PostponedData, Resolution, TriageOutputSchema +from ymir.supervisor.supervisor_types import FullIssue, IssueStatus, JiraComment + + +def make_issue( + key: str = "RHEL-12345", + labels: list[str] | None = None, + fix_versions: list[str] | None = None, + components: list[str] | None = None, + fixed_in_build: str | None = None, + comments: list[JiraComment] | None = None, +) -> FullIssue: + """Build a minimal FullIssue for testing.""" + return FullIssue( + key=key, + url=f"https://jira.example.com/browse/{key}", + summary="Test CVE issue", + components=components if components is not None else ["test-component"], + status=IssueStatus.NEW, + labels=labels if labels is not None else [], + fix_versions=fix_versions if fix_versions is not None else ["rhel-9.6.0"], + errata_link=None, + fixed_in_build=fixed_in_build, + description="Test description", + comments=comments if comments is not None else [], + ) + + +def make_ymir_comment( + resolution: str = "postponed_dependency", + summary: str = "Test postponement summary", + pending_issues: list[str] | None = None, + blocker_reference: str | None = None, + comment_id: str = "100001", +) -> JiraComment: + """Build a Ymir triage comment using the real production formatting path. + + Delegates to ``TriageOutputSchema.format_for_comment()`` and wraps the + result in ``JIRA_COMMENT_TEMPLATE``, exactly as ``tasks.comment_in_jira`` + does at runtime. Any change to the comment format automatically propagates + to all tests that use this fixture. + """ + data = PostponedData( + summary=summary, + pending_issues=pending_issues or [], + jira_issue="RHEL-12345", + blocker_references=[blocker_reference] if blocker_reference else None, + ) + body = JIRA_COMMENT_TEMPLATE.substitute( + AGENT_TYPE="Triage", + JIRA_COMMENT=TriageOutputSchema(resolution=Resolution(resolution), data=data).format_for_comment(), + ) + return JiraComment( + id=comment_id, + authorName="ymir-bot", + authorEmail="ymir@redhat.com", + created=datetime(2025, 6, 15), + body=body, + ) + + +_DEFAULT_RHEL_CONFIG = { + "current_y_streams": { + "8": "rhel-8.10", + "9": "rhel-9.6.0", + "10": "rhel-10.0", + }, + "current_z_streams": { + "8": "rhel-8.10.z", + "9": "rhel-9.6.z", + "10": "rhel-10.0.z", + }, +} + + +@pytest.fixture(autouse=True) +def _inject_jira_email(monkeypatch): + """Inject JIRA_EMAIL for all sweep unit tests. + + parse_ymir_comment() reads JIRA_EMAIL at call time and raises OSError + when it is absent. The value must match the authorEmail set by + make_ymir_comment() so that comment look-ups succeed. + """ + monkeypatch.setenv("JIRA_EMAIL", "ymir@redhat.com") + + +@pytest.fixture(autouse=True) +def _stub_load_rhel_config(monkeypatch): + """Stub load_rhel_config for all sweep unit tests. + + DependencySweep calls load_rhel_config() to normalise fix_version before + the buildroot check. Tests that don't exercise normalisation directly + shouldn't need a real rhel-config.json on disk. + + Tests that do exercise normalisation override this stub via their own + monkeypatch call, which takes precedence for the duration of that test. + + (YStreamSweep no longer calls load_rhel_config — it delegates to the + eligibility tool — so only the dependency module is patched here.) + """ + + async def _default_config(): + return _DEFAULT_RHEL_CONFIG + + monkeypatch.setattr("ymir.sweep.dependency.load_rhel_config", _default_config) + + +@pytest.fixture +def mock_env(monkeypatch): + """Set minimal required environment variables.""" + monkeypatch.setenv("JIRA_URL", "https://jira.example.com") + monkeypatch.setenv("JIRA_EMAIL", "ymir@redhat.com") + monkeypatch.setenv("JIRA_TOKEN", "test-token") + monkeypatch.setenv("REDIS_URL", "redis://localhost:6379") + monkeypatch.setenv("GITLAB_TOKEN", "test-gitlab-token") diff --git a/ymir/sweep/tests/unit/test_base.py b/ymir/sweep/tests/unit/test_base.py new file mode 100644 index 000000000..c177b720a --- /dev/null +++ b/ymir/sweep/tests/unit/test_base.py @@ -0,0 +1,323 @@ +"""Unit tests for ymir.sweep.base (SweepStrategy orchestration).""" + +import pytest +import redis +from flexmock import flexmock + +from ymir.common.constants import JiraLabels +from ymir.supervisor.supervisor_types import FullIssue +from ymir.sweep.base import SweepResult, SweepStrategy +from ymir.sweep.comment_parser import CommentData +from ymir.sweep.tests.unit.conftest import make_issue, make_ymir_comment + +# --------------------------------------------------------------------------- +# Concrete strategy for testing the base class +# --------------------------------------------------------------------------- + + +class _UnblockedStrategy(SweepStrategy): + """Always reports every issue as unblocked.""" + + name = "test_unblocked" + label = JiraLabels.YMIR_POSTPONED_DEPENDENCY + + async def is_unblocked(self, issue: FullIssue, comment_data: CommentData) -> SweepResult: + return SweepResult(issue_key=issue.key, action="unblocked", detail="Fixed!") + + +class _StillBlockedStrategy(SweepStrategy): + name = "test_still_blocked" + label = JiraLabels.YMIR_POSTPONED_DEPENDENCY + + async def is_unblocked(self, issue: FullIssue, comment_data: CommentData) -> SweepResult: + return SweepResult(issue_key=issue.key, action="still_blocked", detail="Not yet") + + +class _ErrorStrategy(SweepStrategy): + name = "test_error" + label = JiraLabels.YMIR_POSTPONED_DEPENDENCY + + async def is_unblocked(self, issue: FullIssue, comment_data: CommentData) -> SweepResult: + return SweepResult(issue_key=issue.key, action="error", detail="Something failed") + + +class _TransitionedStrategy(SweepStrategy): + name = "test_transitioned" + label = JiraLabels.YMIR_POSTPONED_DEPENDENCY + + async def is_unblocked(self, issue: FullIssue, comment_data: CommentData) -> SweepResult: + return SweepResult(issue_key=issue.key, action="transitioned", detail="Category changed") + + +class _RaisingStrategy(SweepStrategy): + """Raises an unexpected exception from is_unblocked.""" + + name = "test_raising" + label = JiraLabels.YMIR_POSTPONED_DEPENDENCY + + async def is_unblocked(self, issue: FullIssue, comment_data: CommentData) -> SweepResult: + raise RuntimeError("Unexpected error") + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _mock_redis(): + r = flexmock() + r.should_receive("lpush").and_return(1) + return r + + +def _mock_remove_label(monkeypatch): + monkeypatch.setattr("ymir.sweep.base.remove_issue_label", lambda *a, **kw: None) + + +def _mock_add_label(monkeypatch): + monkeypatch.setattr("ymir.sweep.base.add_issue_label", lambda *a, **kw: None) + + +def _issue_with_comment(**kw): + comment = make_ymir_comment( + pending_issues=["RHEL-99"], + blocker_reference="RHEL-99", + **kw, + ) + return make_issue(comments=[comment]) + + +# --------------------------------------------------------------------------- +# Tests: get_blocked_issues +# --------------------------------------------------------------------------- + + +def test_get_blocked_issues_uses_correct_jql(monkeypatch): + captured = {} + + def mock_get_current_issues(jql, full=False): + captured["jql"] = jql + return iter([]) + + monkeypatch.setattr("ymir.sweep.base.get_current_issues", mock_get_current_issues) + + strategy = _UnblockedStrategy() + strategy.get_blocked_issues() + + assert captured["jql"] == f'labels = "{JiraLabels.YMIR_POSTPONED_DEPENDENCY.value}"' + + +# --------------------------------------------------------------------------- +# Tests: on_unblock +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_on_unblock_pushes_to_redis_before_adding_label(monkeypatch): + """Redis push must precede TRIAGE_IN_PROGRESS so a Jira failure leaves the + issue queued (processed by triage) rather than silently dropped.""" + call_order = [] + issue_key = "RHEL-12345" + mock_redis = flexmock() + mock_redis.should_receive("lpush").replace_with(lambda *a: call_order.append("redis")) + + monkeypatch.setattr( + "ymir.sweep.base.add_issue_label", + lambda *a, **kw: call_order.append("add"), + ) + monkeypatch.setattr( + "ymir.sweep.base.remove_issue_label", + lambda *a, **kw: call_order.append("remove"), + ) + + strategy = _UnblockedStrategy() + await strategy.on_unblock(issue_key, mock_redis, comment="Fixed!") + + assert call_order == ["redis", "add", "remove"] + + +@pytest.mark.asyncio +async def test_on_unblock_sets_triage_in_progress_before_removing_postponement(monkeypatch): + """TRIAGE_IN_PROGRESS must be added before the postponement label is removed + so there is no window where the issue has no Ymir tracking label.""" + call_order = [] + + mock_redis = flexmock() + mock_redis.should_receive("lpush").and_return(1) + + monkeypatch.setattr( + "ymir.sweep.base.add_issue_label", + lambda key, label, **kw: call_order.append(("add", label)), + ) + monkeypatch.setattr( + "ymir.sweep.base.remove_issue_label", + lambda key, label, **kw: call_order.append(("remove", label)), + ) + + strategy = _UnblockedStrategy() + await strategy.on_unblock("RHEL-12345", mock_redis) + + add_idx = next(i for i, op in enumerate(call_order) if op == ("add", JiraLabels.TRIAGE_IN_PROGRESS.value)) + remove_idx = next(i for i, op in enumerate(call_order) if op[0] == "remove") + assert add_idx < remove_idx, ( + f"TRIAGE_IN_PROGRESS must be added before postponement removed; got {call_order}" + ) + + +@pytest.mark.asyncio +async def test_on_unblock_passes_comment_to_remove_label(monkeypatch): + captured = {} + + mock_redis = flexmock() + mock_redis.should_receive("lpush").and_return(1) + + _mock_add_label(monkeypatch) + monkeypatch.setattr( + "ymir.sweep.base.remove_issue_label", + lambda key, label, comment=None: captured.update({"comment": comment}), + ) + + strategy = _UnblockedStrategy() + await strategy.on_unblock("RHEL-12345", mock_redis, comment="Resolved.") + + assert captured["comment"] == "Resolved." + + +# --------------------------------------------------------------------------- +# Tests: on_transition +# --------------------------------------------------------------------------- + + +def test_on_transition_adds_new_label_before_removing_old(monkeypatch): + call_order = [] + + monkeypatch.setattr( + "ymir.sweep.base.add_issue_label", + lambda *a, **kw: call_order.append(("add", a[1])), + ) + monkeypatch.setattr( + "ymir.sweep.base.remove_issue_label", + lambda *a, **kw: call_order.append(("remove", a[1])), + ) + + strategy = _UnblockedStrategy() + strategy.on_transition( + "RHEL-12345", + JiraLabels.YMIR_POSTPONED_NO_PATCH, + comment="Switching category.", + ) + + assert call_order[0] == ("add", JiraLabels.YMIR_POSTPONED_NO_PATCH.value) + assert call_order[1] == ("remove", JiraLabels.YMIR_POSTPONED_DEPENDENCY.value) + + +# --------------------------------------------------------------------------- +# Tests: run() +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_run_counts_unblocked(monkeypatch): + issue = _issue_with_comment() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + _mock_remove_label(monkeypatch) + _mock_add_label(monkeypatch) + mock_redis = _mock_redis() + + strategy = _UnblockedStrategy() + summary = await strategy.run(mock_redis) + + assert summary["total"] == 1 + assert summary["unblocked"] == 1 + assert summary["errors"] == 0 + assert summary["still_blocked"] == 0 + + +@pytest.mark.asyncio +async def test_run_counts_still_blocked(monkeypatch): + issue = _issue_with_comment() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + + strategy = _StillBlockedStrategy() + summary = await strategy.run(flexmock().should_receive("lpush").never().mock()) + + assert summary["still_blocked"] == 1 + assert summary["unblocked"] == 0 + + +@pytest.mark.asyncio +async def test_run_counts_errors(monkeypatch): + issue = _issue_with_comment() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + + strategy = _ErrorStrategy() + summary = await strategy.run(flexmock()) + + assert summary["errors"] == 1 + assert summary["unblocked"] == 0 + + +@pytest.mark.asyncio +async def test_run_skips_issues_with_no_ymir_comment(monkeypatch): + issue = make_issue(comments=[]) # no Ymir comment + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + + strategy = _UnblockedStrategy() + summary = await strategy.run(flexmock()) + + assert summary["errors"] == 1 + assert summary["unblocked"] == 0 + + +@pytest.mark.asyncio +async def test_run_catches_unexpected_exceptions(monkeypatch): + issue = _issue_with_comment() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + + strategy = _RaisingStrategy() + summary = await strategy.run(flexmock()) + + assert summary["errors"] == 1 + + +@pytest.mark.asyncio +async def test_run_reraises_redis_error(monkeypatch): + issue = _issue_with_comment() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + monkeypatch.setattr("ymir.sweep.base.remove_issue_label", lambda *a, **kw: None) + + bad_redis = flexmock() + bad_redis.should_receive("lpush").and_raise(redis.RedisError("connection lost")) + + strategy = _UnblockedStrategy() + with pytest.raises(redis.RedisError): + await strategy.run(bad_redis) + + +@pytest.mark.asyncio +async def test_run_counts_transitioned(monkeypatch): + issue = _issue_with_comment() + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter([issue])) + + strategy = _TransitionedStrategy() + summary = await strategy.run(flexmock()) + + assert summary["transitioned"] == 1 + assert summary["unblocked"] == 0 + assert summary["errors"] == 0 + assert summary["still_blocked"] == 0 + + +@pytest.mark.asyncio +async def test_run_processes_multiple_issues(monkeypatch): + issues = [_issue_with_comment(summary=f"reason {i}") for i in range(3)] + monkeypatch.setattr("ymir.sweep.base.get_current_issues", lambda jql, full=False: iter(issues)) + _mock_remove_label(monkeypatch) + _mock_add_label(monkeypatch) + mock_redis = _mock_redis() + + strategy = _UnblockedStrategy() + summary = await strategy.run(mock_redis) + + assert summary["total"] == 3 + assert summary["unblocked"] == 3 diff --git a/ymir/sweep/tests/unit/test_comment_parser.py b/ymir/sweep/tests/unit/test_comment_parser.py new file mode 100644 index 000000000..618a0f38f --- /dev/null +++ b/ymir/sweep/tests/unit/test_comment_parser.py @@ -0,0 +1,178 @@ +"""Unit tests for ymir.sweep.comment_parser.""" + +from datetime import datetime + +from ymir.supervisor.supervisor_types import JiraComment +from ymir.sweep.comment_parser import parse_ymir_comment +from ymir.sweep.tests.unit.conftest import make_issue, make_ymir_comment + + +def test_parse_dependency_comment_with_blocker(): + comment = make_ymir_comment( + resolution="postponed_dependency", + summary="Rebuild of pkg waiting for dep (nvr) to land in c9s buildroot", + pending_issues=["RHEL-67890"], + blocker_reference="RHEL-67890", + ) + issue = make_issue(comments=[comment]) + + result = parse_ymir_comment(issue) + + assert result is not None + assert result.blocker_references == ["RHEL-67890"] + assert result.pending_issues == ["RHEL-67890"] + assert "c9s buildroot" in result.summary + assert result.comment_id == "100001" + + +def test_parse_y_stream_comment_multiple_pending(): + comment = make_ymir_comment( + resolution="postponed_y_stream", + summary="Y-stream CVE waiting for Z-stream clones to ship", + pending_issues=["RHEL-11111", "RHEL-22222"], + ) + issue = make_issue(comments=[comment]) + + result = parse_ymir_comment(issue) + + assert result is not None + assert result.blocker_references is None + assert result.pending_issues == ["RHEL-11111", "RHEL-22222"] + + +def test_parse_pr_pending_comment_with_mr_url(): + mr_url = "https://gitlab.com/redhat/centos-stream/rpms/pkg/-/merge_requests/42" + comment = make_ymir_comment( + resolution="postponed_pr_pending", + summary="Upstream patch not yet merged", + pending_issues=["RHEL-55555"], + blocker_reference=mr_url, + ) + issue = make_issue(comments=[comment]) + + result = parse_ymir_comment(issue) + + assert result is not None + assert result.blocker_references == [mr_url] + + +def test_parse_no_patch_comment(): + comment = make_ymir_comment( + resolution="postponed_no_patch", + summary="No upstream patch is available for this CVE", + ) + issue = make_issue(comments=[comment]) + + result = parse_ymir_comment(issue) + + assert result is not None + assert result.pending_issues == [] + assert result.blocker_references is None + + +def test_returns_none_for_issue_with_no_ymir_comment(): + human_comment = JiraComment( + id="1", + authorName="human", + authorEmail="human@example.com", + created=datetime(2025, 1, 1), + body="This is just a human comment, no Ymir marker.", + ) + issue = make_issue(comments=[human_comment]) + + assert parse_ymir_comment(issue) is None + + +def test_returns_none_for_issue_with_no_comments(): + issue = make_issue(comments=[]) + + assert parse_ymir_comment(issue) is None + + +def test_parses_ymir_comment_without_optional_fields(): + """A Ymir comment is parseable even when blocker/pending fields are absent. + + The postponement reason lives in the issue's label, not the comment, so + the parser no longer requires any reason field to be present. + """ + comment = JiraComment( + id="1", + authorName="ymir-bot", + authorEmail="ymir@redhat.com", + created=datetime(2025, 6, 15), + body=( + "Output from Ymir Triage Agent: \n\n" + "*Resolution*: postponed_no_patch\n" + "*Summary*: Something\n" + "Warning: AI-generated content." + ), + ) + issue = make_issue(comments=[comment]) + + result = parse_ymir_comment(issue) + + assert result is not None + assert result.summary == "Something" + assert result.blocker_references is None + assert result.pending_issues == [] + + +def test_returns_latest_ymir_comment_when_multiple_exist(): + older = make_ymir_comment( + resolution="postponed_dependency", + summary="Old reason", + comment_id="10001", + pending_issues=["RHEL-1"], + ) + newer = make_ymir_comment( + resolution="postponed_y_stream", + summary="Newer reason", + comment_id="10002", + pending_issues=["RHEL-2"], + ) + # Give newer a later created timestamp + newer_comment = JiraComment( + id="10002", + authorName="ymir-bot", + authorEmail="ymir@redhat.com", + created=datetime(2025, 12, 1), + body=newer.body, + ) + older_comment = JiraComment( + id="10001", + authorName="ymir-bot", + authorEmail="ymir@redhat.com", + created=datetime(2025, 6, 1), + body=older.body, + ) + issue = make_issue(comments=[older_comment, newer_comment]) + + result = parse_ymir_comment(issue) + + assert result is not None + assert result.comment_id == "10002" + assert result.summary == "Newer reason" + + +def test_ignores_non_ymir_comments_between_ymir_comments(): + ymir_comment = make_ymir_comment( + resolution="postponed_dependency", + summary="Rebuild waiting", + comment_id="1", + pending_issues=["RHEL-99"], + blocker_reference="RHEL-99", + ) + human_comment = JiraComment( + id="2", + authorName="maintainer", + authorEmail=None, + created=datetime(2025, 8, 1), + body="I've looked at this, still waiting.", + ) + issue = make_issue(comments=[ymir_comment, human_comment]) + + result = parse_ymir_comment(issue) + + assert result is not None + assert result.comment_id == "1" + assert result.summary == "Rebuild waiting" diff --git a/ymir/sweep/tests/unit/test_dependency.py b/ymir/sweep/tests/unit/test_dependency.py new file mode 100644 index 000000000..794ab0de2 --- /dev/null +++ b/ymir/sweep/tests/unit/test_dependency.py @@ -0,0 +1,360 @@ +"""Unit tests for ymir.sweep.dependency.DependencySweep.""" + +import pytest +import requests +from flexmock import flexmock + +from ymir.supervisor.supervisor_types import IssueStatus +from ymir.sweep.dependency import DependencySweep, _resolve_target_branch, branch_from_fix_versions +from ymir.sweep.tests.unit.conftest import make_issue, make_ymir_comment + +# --------------------------------------------------------------------------- +# _branch_from_fix_versions +# --------------------------------------------------------------------------- + + +def test_branch_from_fix_versions_rhel9(): + assert branch_from_fix_versions(["rhel-9.6.0"]) == "c9s" + + +def test_branch_from_fix_versions_rhel10(): + assert branch_from_fix_versions(["rhel-10.1"]) == "c10s" + + +def test_branch_from_fix_versions_rhel8(): + assert branch_from_fix_versions(["rhel-8.10"]) == "c8s" + + +def test_branch_from_fix_versions_z_stream(): + assert branch_from_fix_versions(["rhel-9.7.z"]) == "c9s" + + +def test_branch_from_fix_versions_empty(): + assert branch_from_fix_versions([]) is None + + +def test_branch_from_fix_versions_unparseable(): + assert branch_from_fix_versions(["VHEL-9.6"]) is None + + +# --------------------------------------------------------------------------- +# _resolve_target_branch +# --------------------------------------------------------------------------- + + +def test_resolve_target_branch_prefers_summary(monkeypatch): + from ymir.sweep.comment_parser import CommentData + + cd = CommentData( + blocker_references=["RHEL-99"], + pending_issues=["RHEL-99"], + summary="Rebuild of pkg waiting for dep (nvr) to land in c9s buildroot", + comment_id="1", + ) + assert _resolve_target_branch(cd, ["rhel-10.0"]) == "c9s" + + +def test_resolve_target_branch_falls_back_to_fix_versions(monkeypatch): + from ymir.sweep.comment_parser import CommentData + + cd = CommentData( + blocker_references=["RHEL-99"], + pending_issues=["RHEL-99"], + summary="No branch indicator here", + comment_id="1", + ) + assert _resolve_target_branch(cd, ["rhel-10.2"]) == "c10s" + + +def test_resolve_target_branch_returns_none_when_undetermined(monkeypatch): + from ymir.sweep.comment_parser import CommentData + + cd = CommentData( + blocker_references=None, + pending_issues=[], + summary=None, + comment_id="1", + ) + assert _resolve_target_branch(cd, []) is None + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_blocker(fixed_in_build=None, components=None): + """Build a minimal Issue representing the blocker.""" + from ymir.supervisor.supervisor_types import Issue + + return Issue( + key="RHEL-67890", + url="https://jira.example.com/browse/RHEL-67890", + summary="Blocker issue", + components=components if components is not None else ["golang"], + status=IssueStatus.IN_PROGRESS, + labels=[], + fix_versions=["rhel-9.7.z"], + errata_link=None, + fixed_in_build=fixed_in_build, + ) + + +def _make_comment(blocker_ref="RHEL-67890", pending=None, summary=None): + return make_ymir_comment( + summary=summary or "Rebuild of pkg waiting for dep to land in c9s buildroot", + pending_issues=pending or ["RHEL-67890"], + blocker_reference=blocker_ref, + ) + + +def _make_issue_with_comment(**kw): + return make_issue(comments=[_make_comment(**kw)]) + + +# --------------------------------------------------------------------------- +# DependencySweep.is_unblocked +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_unblocked_when_build_in_buildroot(monkeypatch): + blocker = _make_blocker(fixed_in_build="golang-1.21.0-1.el9") + monkeypatch.setattr("ymir.sweep.dependency.get_issue", lambda key, full=False: blocker) + monkeypatch.setattr( + "ymir.sweep.dependency.check_build_in_buildroot", + lambda *a, **kw: _async_true(), + ) + + issue = _make_issue_with_comment() + comment_data = _parse_comment(issue) + result = await DependencySweep().is_unblocked(issue, comment_data) + + assert result.action == "unblocked" + assert "RHEL-67890" in result.detail + assert "golang-1.21.0-1.el9" in result.detail + + +@pytest.mark.asyncio +async def test_still_blocked_when_no_fixed_in_build(monkeypatch): + blocker = _make_blocker(fixed_in_build=None) + monkeypatch.setattr("ymir.sweep.dependency.get_issue", lambda key, full=False: blocker) + + issue = _make_issue_with_comment() + result = await DependencySweep().is_unblocked(issue, _parse_comment(issue)) + + assert result.action == "still_blocked" + + +@pytest.mark.asyncio +async def test_still_blocked_when_build_not_in_buildroot(monkeypatch): + blocker = _make_blocker(fixed_in_build="golang-1.21.0-1.el9") + monkeypatch.setattr("ymir.sweep.dependency.get_issue", lambda key, full=False: blocker) + monkeypatch.setattr( + "ymir.sweep.dependency.check_build_in_buildroot", + lambda *a, **kw: _async_false(), + ) + + issue = _make_issue_with_comment() + result = await DependencySweep().is_unblocked(issue, _parse_comment(issue)) + + assert result.action == "still_blocked" + + +@pytest.mark.asyncio +async def test_error_when_blocker_key_missing(monkeypatch): + comment = make_ymir_comment( + summary="No blocker here", + pending_issues=[], + blocker_reference=None, + ) + issue = make_issue(comments=[comment]) + from ymir.sweep.comment_parser import CommentData + + comment_data = CommentData( + blocker_references=None, + pending_issues=[], + summary="No blocker here", + comment_id="1", + ) + + result = await DependencySweep().is_unblocked(issue, comment_data) + + assert result.action == "error" + + +@pytest.mark.asyncio +async def test_error_when_blocker_not_jira_key(monkeypatch): + from ymir.sweep.comment_parser import CommentData + + issue = make_issue() + comment_data = CommentData( + blocker_references=["not-a-jira-key"], + pending_issues=[], + summary="test", + comment_id="1", + ) + + result = await DependencySweep().is_unblocked(issue, comment_data) + + assert result.action == "error" + + +@pytest.mark.asyncio +async def test_error_when_get_issue_raises_http_error(monkeypatch): + def raise_http(*a, **kw): + resp = flexmock(status_code=404) + raise requests.HTTPError("Not found", response=resp) + + monkeypatch.setattr("ymir.sweep.dependency.get_issue", raise_http) + + issue = _make_issue_with_comment() + result = await DependencySweep().is_unblocked(issue, _parse_comment(issue)) + + assert result.action == "error" + + +@pytest.mark.asyncio +async def test_error_when_blocker_has_no_components(monkeypatch): + blocker = _make_blocker(fixed_in_build="golang-1.21.0-1.el9", components=[]) + monkeypatch.setattr("ymir.sweep.dependency.get_issue", lambda key, full=False: blocker) + + issue = _make_issue_with_comment() + result = await DependencySweep().is_unblocked(issue, _parse_comment(issue)) + + assert result.action == "error" + + +@pytest.mark.asyncio +async def test_error_when_check_build_raises(monkeypatch): + blocker = _make_blocker(fixed_in_build="golang-1.21.0-1.el9") + monkeypatch.setattr("ymir.sweep.dependency.get_issue", lambda key, full=False: blocker) + monkeypatch.setattr( + "ymir.sweep.dependency.check_build_in_buildroot", + lambda *a, **kw: _async_raise(RuntimeError("Koji unreachable")), + ) + + issue = _make_issue_with_comment() + result = await DependencySweep().is_unblocked(issue, _parse_comment(issue)) + + assert result.action == "error" + assert "Koji unreachable" in result.detail + + +@pytest.mark.asyncio +async def test_falls_back_to_pending_issues_when_no_blocker_reference(monkeypatch): + """Blocker key resolved from pending_issues when blocker_reference is None.""" + blocker = _make_blocker(fixed_in_build="golang-1.21.0-1.el9") + monkeypatch.setattr("ymir.sweep.dependency.get_issue", lambda key, full=False: blocker) + monkeypatch.setattr( + "ymir.sweep.dependency.check_build_in_buildroot", + lambda *a, **kw: _async_true(), + ) + + from ymir.sweep.comment_parser import CommentData + + issue = make_issue() + comment_data = CommentData( + blocker_references=None, + pending_issues=["RHEL-67890"], + summary="Rebuild of pkg waiting for dep to land in c9s buildroot", + comment_id="1", + ) + + result = await DependencySweep().is_unblocked(issue, comment_data) + + assert result.action == "unblocked" + + +# --------------------------------------------------------------------------- +# fix_version normalization +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_stale_fix_version_normalised_before_buildroot_check(monkeypatch): + """A stale Y-stream fixVersion (rhel-9.8) must be normalised to rhel-9.8.z. + + Without normalisation ``_resolve_buildroot_checks`` treats it as a Y-stream + version and only checks the CS Koji buildroot, missing the Brew Z-stream + check that triage performs after normalising. + """ + rhel_config = {"current_y_streams": {"9": "rhel-9.9"}, "current_z_streams": {"9": "rhel-9.8.z"}} + + async def fake_load_rhel_config(): + return rhel_config + + captured: dict = {} + + async def capture_buildroot_check(*a, fix_version="", **kw): + captured["fix_version"] = fix_version + return True + + monkeypatch.setattr("ymir.sweep.dependency.load_rhel_config", fake_load_rhel_config) + monkeypatch.setattr( + "ymir.sweep.dependency.get_issue", + lambda key, full=False: _make_blocker(fixed_in_build="golang-1.21.0-1.el9"), + ) + monkeypatch.setattr("ymir.sweep.dependency.check_build_in_buildroot", capture_buildroot_check) + + issue = _make_issue_with_comment() + issue = make_issue(fix_versions=["rhel-9.8"], comments=[_make_comment()]) + result = await DependencySweep().is_unblocked(issue, _parse_comment(issue)) + + assert result.action == "unblocked" + assert captured.get("fix_version") == "rhel-9.8.z", ( + f"Expected normalised fix_version 'rhel-9.8.z', got {captured.get('fix_version')!r}" + ) + + +@pytest.mark.asyncio +async def test_current_y_stream_fix_version_not_normalised(monkeypatch): + """A current Y-stream fixVersion must be passed through unchanged.""" + rhel_config = {"current_y_streams": {"9": "rhel-9.9"}, "current_z_streams": {"9": "rhel-9.8.z"}} + + async def fake_load_rhel_config(): + return rhel_config + + captured: dict = {} + + async def capture_buildroot_check(*a, fix_version="", **kw): + captured["fix_version"] = fix_version + return True + + monkeypatch.setattr("ymir.sweep.dependency.load_rhel_config", fake_load_rhel_config) + monkeypatch.setattr( + "ymir.sweep.dependency.get_issue", + lambda key, full=False: _make_blocker(fixed_in_build="golang-1.21.0-1.el9"), + ) + monkeypatch.setattr("ymir.sweep.dependency.check_build_in_buildroot", capture_buildroot_check) + + issue = make_issue(fix_versions=["rhel-9.9"], comments=[_make_comment()]) + result = await DependencySweep().is_unblocked(issue, _parse_comment(issue)) + + assert result.action == "unblocked" + assert captured.get("fix_version") == "rhel-9.9", ( + f"Expected unchanged fix_version 'rhel-9.9', got {captured.get('fix_version')!r}" + ) + + +# --------------------------------------------------------------------------- +# Async helpers +# --------------------------------------------------------------------------- + + +async def _async_true(*a, **kw): + return True + + +async def _async_false(*a, **kw): + return False + + +async def _async_raise(exc): + raise exc + + +def _parse_comment(issue): + from ymir.sweep.comment_parser import parse_ymir_comment + + return parse_ymir_comment(issue) diff --git a/ymir/sweep/tests/unit/test_main.py b/ymir/sweep/tests/unit/test_main.py new file mode 100644 index 000000000..63a3b5fc7 --- /dev/null +++ b/ymir/sweep/tests/unit/test_main.py @@ -0,0 +1,121 @@ +"""Unit tests for ymir.sweep.__main__ (CLI entry point and run_sweep).""" + +from contextlib import asynccontextmanager + +import pytest +from flexmock import flexmock + +from ymir.sweep.__main__ import STRATEGIES, run_sweep +from ymir.sweep.dependency import DependencySweep +from ymir.sweep.no_patch import NoPatchSweep +from ymir.sweep.pr_pending import PRPendingSweep +from ymir.sweep.y_stream import YStreamSweep + +# --------------------------------------------------------------------------- +# STRATEGIES dict +# --------------------------------------------------------------------------- + + +def test_strategies_dict_contains_all_expected_keys(): + assert set(STRATEGIES.keys()) == {"dependency", "y_stream", "pr_pending", "no_patch"} + + +def test_strategies_dict_maps_to_correct_classes(): + assert STRATEGIES["dependency"] is DependencySweep + assert STRATEGIES["y_stream"] is YStreamSweep + assert STRATEGIES["pr_pending"] is PRPendingSweep + assert STRATEGIES["no_patch"] is NoPatchSweep + + +# --------------------------------------------------------------------------- +# run_sweep +# --------------------------------------------------------------------------- + + +def _make_mock_strategy(name: str, called: list): + """Return a strategy class whose run() records its name and returns a + zero summary, suitable for injection via STRATEGIES.""" + + class _MockStrategy: + def __init__(self): + pass + + async def run(self, redis_conn): + called.append(name) + return { + "total": 0, + "unblocked": 0, + "transitioned": 0, + "errors": 0, + "still_blocked": 0, + } + + return _MockStrategy + + +def _patch_context_managers(monkeypatch): + """Replace the async context managers that run_sweep requires so that + tests don't need a live requests session or Redis connection.""" + + @asynccontextmanager + async def _noop_requests_session(): + yield + + mock_redis = flexmock() + + @asynccontextmanager + async def _noop_redis_client(url): + yield mock_redis + + monkeypatch.setattr("ymir.sweep.__main__.with_requests_session", _noop_requests_session) + monkeypatch.setattr("ymir.sweep.__main__.redis_client", _noop_redis_client) + monkeypatch.setenv("REDIS_URL", "redis://localhost:6379") + + return mock_redis + + +@pytest.mark.asyncio +async def test_run_sweep_invokes_single_strategy(monkeypatch): + called = [] + monkeypatch.setattr( + "ymir.sweep.__main__.STRATEGIES", + {"dependency": _make_mock_strategy("dependency", called)}, + ) + _patch_context_managers(monkeypatch) + + await run_sweep(["dependency"]) + + assert called == ["dependency"] + + +@pytest.mark.asyncio +async def test_run_sweep_invokes_all_strategies_in_order(monkeypatch): + called = [] + monkeypatch.setattr( + "ymir.sweep.__main__.STRATEGIES", + { + "dependency": _make_mock_strategy("dependency", called), + "y_stream": _make_mock_strategy("y_stream", called), + "pr_pending": _make_mock_strategy("pr_pending", called), + "no_patch": _make_mock_strategy("no_patch", called), + }, + ) + _patch_context_managers(monkeypatch) + + await run_sweep(["dependency", "y_stream", "pr_pending", "no_patch"]) + + assert called == ["dependency", "y_stream", "pr_pending", "no_patch"] + + +@pytest.mark.asyncio +async def test_run_sweep_empty_list_runs_nothing(monkeypatch): + called = [] + monkeypatch.setattr( + "ymir.sweep.__main__.STRATEGIES", + {"dependency": _make_mock_strategy("dependency", called)}, + ) + _patch_context_managers(monkeypatch) + + await run_sweep([]) + + assert called == [] diff --git a/ymir/sweep/tests/unit/test_no_patch.py b/ymir/sweep/tests/unit/test_no_patch.py new file mode 100644 index 000000000..54a8f5ad1 --- /dev/null +++ b/ymir/sweep/tests/unit/test_no_patch.py @@ -0,0 +1,249 @@ +"""Unit tests for ymir.sweep.no_patch.NoPatchSweep.""" + +import pytest +import redis +from flexmock import flexmock + +from ymir.common.constants import JiraLabels, RedisQueues +from ymir.sweep.no_patch import _MAX_ISSUES_DEFAULT, NoPatchSweep +from ymir.sweep.tests.unit.conftest import make_issue + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_redis(): + r = flexmock() + r.should_receive("lpush").and_return(1) + return r + + +def _patch_remove_label(monkeypatch): + monkeypatch.setattr("ymir.sweep.no_patch.remove_issue_label", lambda *a, **kw: None) + + +def _patch_add_label(monkeypatch): + monkeypatch.setattr("ymir.sweep.no_patch.add_issue_label", lambda *a, **kw: None) + + +def _set_get_blocked(monkeypatch, issues): + monkeypatch.setattr( + "ymir.sweep.base.get_current_issues", + lambda jql, full=False: iter(issues), + ) + + +# --------------------------------------------------------------------------- +# NoPatchSweep.run +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_run_requeues_eligible_issues(monkeypatch): + issues = [make_issue(key=f"RHEL-{i}") for i in range(3)] + _set_get_blocked(monkeypatch, issues) + _patch_remove_label(monkeypatch) + _patch_add_label(monkeypatch) + + queued = [] + mock_redis = flexmock() + mock_redis.should_receive("lpush").replace_with(lambda queue, payload: queued.append(queue)) + + summary = await NoPatchSweep().run(mock_redis) + + assert summary["total"] == 3 + assert summary["unblocked"] == 3 + assert summary["still_blocked"] == 0 + assert summary["errors"] == 0 + assert all(q == RedisQueues.TRIAGE_QUEUE.value for q in queued) + + +@pytest.mark.asyncio +async def test_run_skips_in_progress_issues(monkeypatch): + in_progress = make_issue(key="RHEL-1", labels=[JiraLabels.TRIAGE_IN_PROGRESS.value]) + eligible = make_issue(key="RHEL-2", labels=[]) + _set_get_blocked(monkeypatch, [in_progress, eligible]) + _patch_remove_label(monkeypatch) + _patch_add_label(monkeypatch) + mock_redis = _make_redis() + + summary = await NoPatchSweep().run(mock_redis) + + assert summary["total"] == 2 + assert summary["unblocked"] == 1 + assert summary["still_blocked"] == 1 # the in-progress issue + + +@pytest.mark.asyncio +async def test_run_respects_max_issues_per_run_env(monkeypatch): + monkeypatch.setenv("MAX_ISSUES_PER_RUN", "2") + issues = [make_issue(key=f"RHEL-{i}") for i in range(5)] + _set_get_blocked(monkeypatch, issues) + _patch_remove_label(monkeypatch) + _patch_add_label(monkeypatch) + mock_redis = _make_redis() + + summary = await NoPatchSweep().run(mock_redis) + + assert summary["unblocked"] == 2 + # 3 issues capped for next run, counted as still_blocked + assert summary["still_blocked"] == 3 + + +@pytest.mark.asyncio +async def test_run_uses_default_max_when_env_not_set(monkeypatch): + monkeypatch.delenv("MAX_ISSUES_PER_RUN", raising=False) + issues = [make_issue(key=f"RHEL-{i}") for i in range(_MAX_ISSUES_DEFAULT + 5)] + _set_get_blocked(monkeypatch, issues) + _patch_remove_label(monkeypatch) + _patch_add_label(monkeypatch) + mock_redis = _make_redis() + + summary = await NoPatchSweep().run(mock_redis) + + assert summary["unblocked"] == _MAX_ISSUES_DEFAULT + assert summary["still_blocked"] == 5 + + +@pytest.mark.asyncio +async def test_run_posts_comment_with_remove_label(monkeypatch): + issues = [make_issue(key="RHEL-1")] + _set_get_blocked(monkeypatch, issues) + _patch_add_label(monkeypatch) + + captured = {} + monkeypatch.setattr( + "ymir.sweep.no_patch.remove_issue_label", + lambda key, label, comment=None: captured.update({"key": key, "label": label, "comment": comment}), + ) + mock_redis = _make_redis() + + await NoPatchSweep().run(mock_redis) + + assert captured["key"] == "RHEL-1" + assert captured["label"] == JiraLabels.YMIR_POSTPONED_NO_PATCH.value + + +@pytest.mark.asyncio +async def test_run_reraises_redis_error(monkeypatch): + issues = [make_issue(key="RHEL-1")] + _set_get_blocked(monkeypatch, issues) + + bad_redis = flexmock() + bad_redis.should_receive("lpush").and_raise(redis.RedisError("connection lost")) + + with pytest.raises(redis.RedisError): + await NoPatchSweep().run(bad_redis) + + +@pytest.mark.asyncio +async def test_run_counts_individual_errors(monkeypatch): + issues = [make_issue(key="RHEL-1"), make_issue(key="RHEL-2")] + _set_get_blocked(monkeypatch, issues) + _patch_add_label(monkeypatch) + + call_count = [0] + + def flaky_remove(key, label, comment=None): + call_count[0] += 1 + if call_count[0] == 1: + raise ValueError("Jira API blip") + + monkeypatch.setattr("ymir.sweep.no_patch.remove_issue_label", flaky_remove) + mock_redis = _make_redis() + + summary = await NoPatchSweep().run(mock_redis) + + assert summary["errors"] == 1 + assert summary["unblocked"] == 1 + + +@pytest.mark.asyncio +async def test_run_pushes_to_redis_before_adding_in_progress_label(monkeypatch): + """Redis push must precede TRIAGE_IN_PROGRESS so a Jira failure leaves + the issue queued (processed by triage) rather than silently dropped.""" + issues = [make_issue(key="RHEL-1")] + _set_get_blocked(monkeypatch, issues) + + call_order = [] + + mock_redis = flexmock() + mock_redis.should_receive("lpush").replace_with(lambda queue, payload: call_order.append("redis")) + monkeypatch.setattr( + "ymir.sweep.no_patch.add_issue_label", + lambda *a, **kw: call_order.append("add"), + ) + monkeypatch.setattr( + "ymir.sweep.no_patch.remove_issue_label", + lambda *a, **kw: call_order.append("remove"), + ) + + await NoPatchSweep().run(mock_redis) + + assert call_order == ["redis", "add", "remove"] + + +@pytest.mark.asyncio +async def test_run_sets_triage_in_progress_before_removing_postponement(monkeypatch): + """TRIAGE_IN_PROGRESS must be added before the postponement label is removed.""" + issues = [make_issue(key="RHEL-1")] + _set_get_blocked(monkeypatch, issues) + + label_ops = [] + monkeypatch.setattr( + "ymir.sweep.no_patch.add_issue_label", + lambda key, label, **kw: label_ops.append(("add", label)), + ) + monkeypatch.setattr( + "ymir.sweep.no_patch.remove_issue_label", + lambda key, label, **kw: label_ops.append(("remove", label)), + ) + mock_redis = _make_redis() + + await NoPatchSweep().run(mock_redis) + + add_idx = next(i for i, op in enumerate(label_ops) if op == ("add", JiraLabels.TRIAGE_IN_PROGRESS.value)) + remove_idx = next(i for i, op in enumerate(label_ops) if op[0] == "remove") + assert add_idx < remove_idx, ( + f"TRIAGE_IN_PROGRESS must be added before postponement removed; got {label_ops}" + ) + + +@pytest.mark.asyncio +async def test_run_redis_pushed_even_when_label_removal_fails(monkeypatch): + """If label removal raises after a successful redis push, the issue is in + the triage queue but still labelled — it will be re-queued on the next + sweep. The error count reflects the failure.""" + issues = [make_issue(key="RHEL-1")] + _set_get_blocked(monkeypatch, issues) + _patch_add_label(monkeypatch) + + pushed = [] + mock_redis = flexmock() + mock_redis.should_receive("lpush").replace_with(lambda queue, payload: pushed.append(queue)) + monkeypatch.setattr( + "ymir.sweep.no_patch.remove_issue_label", + lambda *a, **kw: (_ for _ in ()).throw(Exception("Jira API failure")), + ) + + summary = await NoPatchSweep().run(mock_redis) + + assert summary["errors"] == 1 + assert len(pushed) == 1 # redis push happened before the failure + + +@pytest.mark.asyncio +async def test_run_returns_zero_when_no_issues(monkeypatch): + _set_get_blocked(monkeypatch, []) + mock_redis = flexmock() + + summary = await NoPatchSweep().run(mock_redis) + + assert summary == { + "total": 0, + "unblocked": 0, + "transitioned": 0, + "errors": 0, + "still_blocked": 0, + } diff --git a/ymir/sweep/tests/unit/test_pr_pending.py b/ymir/sweep/tests/unit/test_pr_pending.py new file mode 100644 index 000000000..c873a5010 --- /dev/null +++ b/ymir/sweep/tests/unit/test_pr_pending.py @@ -0,0 +1,364 @@ +"""Unit tests for ymir.sweep.pr_pending.PRPendingSweep.""" + +import pytest + +from ymir.supervisor.supervisor_types import MergeRequestState +from ymir.sweep.comment_parser import CommentData +from ymir.sweep.pr_pending import _GITHUB_PR_RE, _GITLAB_MR_RE, PRPendingSweep +from ymir.sweep.tests.unit.conftest import make_issue + +# --------------------------------------------------------------------------- +# URL regexes +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "url,expected_host,expected_path,expected_iid", + [ + ( + "https://gitlab.com/redhat/centos-stream/rpms/pkg/-/merge_requests/42", + "gitlab.com", + "redhat/centos-stream/rpms/pkg", + "42", + ), + ( + "https://gitlab.cee.redhat.com/foo/bar/-/merge_requests/7", + "gitlab.cee.redhat.com", + "foo/bar", + "7", + ), + ( + "https://gitlab.com/group/sub/repo/-/merge_requests/100", + "gitlab.com", + "group/sub/repo", + "100", + ), + ], +) +def test_gitlab_mr_re_matches_valid_urls(url, expected_host, expected_path, expected_iid): + m = _GITLAB_MR_RE.match(url) + assert m is not None + assert m.group(1) == expected_host + assert m.group(2) == expected_path + assert m.group(3) == expected_iid + + +def test_gitlab_mr_re_rejects_non_mr_urls(): + assert _GITLAB_MR_RE.match("https://gitlab.com/group/repo") is None + assert _GITLAB_MR_RE.match("https://github.com/foo/bar/pull/1") is None + + +@pytest.mark.parametrize( + "url", + [ + # Attacker-controlled host that merely starts with "gitlab." must not match, + # otherwise the GITLAB_TOKEN would be sent there via gitlab_api_get. + "https://gitlab.attacker.com/foo/bar/-/merge_requests/1", + "https://gitlab.com.attacker.com/foo/bar/-/merge_requests/1", + "https://gitlab.evil.example/foo/bar/-/merge_requests/1", + ], +) +def test_gitlab_mr_re_rejects_untrusted_hosts(url): + assert _GITLAB_MR_RE.match(url) is None + + +@pytest.mark.asyncio +async def test_error_when_blocker_reference_points_at_untrusted_gitlab_host(monkeypatch): + """A hostile gitlab.* host is reported as unrecognisable, never reaching the API.""" + + def _fail(*_args, **_kwargs): + raise AssertionError("gitlab_api_get must not be called for an untrusted host") + + monkeypatch.setattr("ymir.sweep.pr_pending.gitlab_api_get", _fail) + + issue = make_issue() + cd = _comment_data(blocker_reference="https://gitlab.attacker.com/foo/bar/-/merge_requests/1") + result = await PRPendingSweep().is_unblocked(issue, cd) + + assert result.action == "error" + assert "not a recognisable" in result.detail + + +@pytest.mark.parametrize( + "url,expected_owner_repo,expected_pr", + [ + ("https://github.com/foo/bar/pull/1", "foo/bar", "1"), + ("https://github.com/org/repo/pull/999", "org/repo", "999"), + ], +) +def test_github_pr_re_matches_valid_urls(url, expected_owner_repo, expected_pr): + m = _GITHUB_PR_RE.match(url) + assert m is not None + assert m.group(1) == expected_owner_repo + assert m.group(2) == expected_pr + + +def test_github_pr_re_rejects_non_pr_urls(): + assert _GITHUB_PR_RE.match("https://github.com/foo/bar/issues/1") is None + assert _GITHUB_PR_RE.match("https://gitlab.com/foo/bar/-/merge_requests/1") is None + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_GITLAB_MR_URL = "https://gitlab.com/redhat/centos-stream/rpms/pkg/-/merge_requests/42" +_GITHUB_PR_URL = "https://github.com/upstream-org/upstream-pkg/pull/123" + + +def _comment_data(blocker_reference=_GITLAB_MR_URL): + return CommentData( + blocker_references=[blocker_reference] if blocker_reference else None, + pending_issues=["RHEL-12345"], + summary="Upstream patch not yet merged", + comment_id="1", + ) + + +def _mock_gitlab_api_get(state: str, monkeypatch): + monkeypatch.setattr( + "ymir.sweep.pr_pending.gitlab_api_get", + lambda path, *, gitlab_url=None, params=None: {"state": state}, + ) + + +def _mock_github_api_get(state: str, merged_at: str | None, monkeypatch): + monkeypatch.setattr( + "ymir.sweep.pr_pending.github_api_get", + lambda path, **_kw: {"state": state, "merged_at": merged_at}, + ) + + +# --------------------------------------------------------------------------- +# PRPendingSweep.is_unblocked — GitLab path +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_unblocked_when_gitlab_mr_merged(monkeypatch): + _mock_gitlab_api_get(MergeRequestState.MERGED, monkeypatch) + + issue = make_issue() + result = await PRPendingSweep().is_unblocked(issue, _comment_data()) + + assert result.action == "unblocked" + assert _GITLAB_MR_URL in result.detail + + +@pytest.mark.asyncio +async def test_still_blocked_when_gitlab_mr_open(monkeypatch): + _mock_gitlab_api_get(MergeRequestState.OPEN, monkeypatch) + + issue = make_issue() + result = await PRPendingSweep().is_unblocked(issue, _comment_data()) + + assert result.action == "still_blocked" + assert "opened" in result.detail + + +@pytest.mark.asyncio +async def test_transition_to_no_patch_when_gitlab_mr_closed(monkeypatch): + _mock_gitlab_api_get(MergeRequestState.CLOSED, monkeypatch) + + transitions = [] + + def capture_transition(issue_key, new_label, comment=None): + transitions.append((issue_key, new_label)) + + strategy = PRPendingSweep() + strategy.on_transition = capture_transition + + issue = make_issue(key="RHEL-55555") + result = await strategy.is_unblocked(issue, _comment_data()) + + assert result.action == "transitioned" + assert len(transitions) == 1 + from ymir.common.constants import JiraLabels + + assert transitions[0] == ("RHEL-55555", JiraLabels.YMIR_POSTPONED_NO_PATCH) + + +@pytest.mark.asyncio +async def test_error_when_blocker_reference_absent(monkeypatch): + issue = make_issue() + cd = CommentData( + blocker_references=None, + pending_issues=["RHEL-12345"], + summary="No URL", + comment_id="1", + ) + + result = await PRPendingSweep().is_unblocked(issue, cd) + + assert result.action == "error" + + +@pytest.mark.asyncio +async def test_error_when_url_matches_no_known_platform(monkeypatch): + issue = make_issue() + cd = _comment_data(blocker_reference="https://bitbucket.org/foo/bar/pull-requests/1") + + result = await PRPendingSweep().is_unblocked(issue, cd) + + assert result.action == "error" + assert "not a recognisable" in result.detail + + +@pytest.mark.asyncio +async def test_still_blocked_for_unknown_gitlab_mr_state(monkeypatch): + """An unrecognised MR state falls through to still_blocked.""" + _mock_gitlab_api_get("locked", monkeypatch) + + issue = make_issue() + result = await PRPendingSweep().is_unblocked(issue, _comment_data()) + + assert result.action == "still_blocked" + assert "locked" in result.detail + + +@pytest.mark.asyncio +async def test_error_when_gitlab_api_call_fails(monkeypatch): + def raise_error(path, *, gitlab_url=None, params=None): + raise Exception("Network timeout") + + monkeypatch.setattr("ymir.sweep.pr_pending.gitlab_api_get", raise_error) + + issue = make_issue() + result = await PRPendingSweep().is_unblocked(issue, _comment_data()) + + assert result.action == "error" + assert "Network timeout" in result.detail + + +# --------------------------------------------------------------------------- +# PRPendingSweep.is_unblocked — GitHub path +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_unblocked_when_github_pr_merged(monkeypatch): + _mock_github_api_get("closed", "2025-01-15T12:00:00Z", monkeypatch) + + issue = make_issue() + result = await PRPendingSweep().is_unblocked(issue, _comment_data(_GITHUB_PR_URL)) + + assert result.action == "unblocked" + assert _GITHUB_PR_URL in result.detail + + +@pytest.mark.asyncio +async def test_still_blocked_when_github_pr_open(monkeypatch): + _mock_github_api_get("open", None, monkeypatch) + + issue = make_issue() + result = await PRPendingSweep().is_unblocked(issue, _comment_data(_GITHUB_PR_URL)) + + assert result.action == "still_blocked" + assert "opened" in result.detail + + +@pytest.mark.asyncio +async def test_transition_to_no_patch_when_github_pr_closed_without_merge(monkeypatch): + _mock_github_api_get("closed", None, monkeypatch) + + transitions = [] + + def capture_transition(issue_key, new_label, comment=None): + transitions.append((issue_key, new_label)) + + strategy = PRPendingSweep() + strategy.on_transition = capture_transition + + issue = make_issue(key="RHEL-55555") + result = await strategy.is_unblocked(issue, _comment_data(_GITHUB_PR_URL)) + + assert result.action == "transitioned" + assert len(transitions) == 1 + from ymir.common.constants import JiraLabels + + assert transitions[0] == ("RHEL-55555", JiraLabels.YMIR_POSTPONED_NO_PATCH) + + +@pytest.mark.asyncio +async def test_error_when_github_api_call_fails(monkeypatch): + monkeypatch.setattr( + "ymir.sweep.pr_pending.github_api_get", + lambda path, **_kw: (_ for _ in ()).throw(Exception("Connection refused")), + ) + + issue = make_issue() + result = await PRPendingSweep().is_unblocked(issue, _comment_data(_GITHUB_PR_URL)) + + assert result.action == "error" + assert "Connection refused" in result.detail + + +# --------------------------------------------------------------------------- +# API call construction — GitLab +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_gitlab_api_get_called_with_correct_path_and_host(monkeypatch): + captured = {} + + def capture(path, *, gitlab_url=None, params=None): + captured["path"] = path + captured["gitlab_url"] = gitlab_url + return {"state": "opened"} + + monkeypatch.setattr("ymir.sweep.pr_pending.gitlab_api_get", capture) + + issue = make_issue() + await PRPendingSweep().is_unblocked( + issue, + _comment_data("https://gitlab.com/redhat/centos-stream/rpms/pkg/-/merge_requests/42"), + ) + + assert captured["gitlab_url"] == "https://gitlab.com" + assert "merge_requests/42" in captured["path"] + assert "redhat" in captured["path"] + + +@pytest.mark.asyncio +async def test_gitlab_api_get_called_with_cee_host(monkeypatch): + captured = {} + + def capture(path, *, gitlab_url=None, params=None): + captured["path"] = path + captured["gitlab_url"] = gitlab_url + return {"state": "merged"} + + monkeypatch.setattr("ymir.sweep.pr_pending.gitlab_api_get", capture) + + issue = make_issue() + await PRPendingSweep().is_unblocked( + issue, + _comment_data("https://gitlab.cee.redhat.com/foo/bar/-/merge_requests/7"), + ) + + assert captured["gitlab_url"] == "https://gitlab.cee.redhat.com" + assert "merge_requests/7" in captured["path"] + + +# --------------------------------------------------------------------------- +# API call construction — GitHub +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_github_api_get_called_with_correct_path(monkeypatch): + captured = {} + + monkeypatch.setattr( + "ymir.sweep.pr_pending.github_api_get", + lambda path, **_kw: captured.update({"path": path}) or {"state": "open", "merged_at": None}, + ) + + issue = make_issue() + await PRPendingSweep().is_unblocked( + issue, + _comment_data("https://github.com/upstream-org/upstream-pkg/pull/123"), + ) + + assert captured["path"] == "repos/upstream-org/upstream-pkg/pulls/123" diff --git a/ymir/sweep/tests/unit/test_y_stream.py b/ymir/sweep/tests/unit/test_y_stream.py new file mode 100644 index 000000000..9c539c534 --- /dev/null +++ b/ymir/sweep/tests/unit/test_y_stream.py @@ -0,0 +1,161 @@ +"""Unit tests for ymir.sweep.y_stream.YStreamSweep. + +YStreamSweep delegates entirely to ``CheckCveTriageEligibilityTool``; these +tests stub that tool and assert the verdict → SweepResult.action mapping. +""" + +import pytest + +from ymir.common import CVEEligibilityResult, TriageEligibility +from ymir.sweep.comment_parser import CommentData +from ymir.sweep.tests.unit.conftest import make_issue +from ymir.sweep.y_stream import YStreamSweep + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +# comment_data is unused by the refactored is_unblocked, but the signature +# still requires it. A minimal stand-in keeps the call sites readable. +_COMMENT_DATA = CommentData( + blocker_references=None, + pending_issues=["RHEL-11111"], + summary="Y-stream CVE waiting for Z-stream clone", + comment_id="1", +) + + +def _elig(eligibility, *, reason="reason", error=None, pending=None, duplicate_of=None): + """Build the dict shape returned by ``tool.run(...).result``.""" + return CVEEligibilityResult( + is_cve=True, + eligibility=eligibility, + reason=reason, + error=error, + pending_zstream_issues=pending, + duplicate_of=duplicate_of, + ).model_dump() + + +def _fake_eligibility_tool(result_dict=None, *, raises=None): + """Return a fake ``CheckCveTriageEligibilityTool`` class. + + Its ``run()`` either raises ``raises`` or returns an object exposing + ``.result`` (mirroring ``JSONToolOutput``). + """ + + class _Output: + result = result_dict + + class _Tool: + async def run(self, input): + if raises is not None: + raise raises + return _Output() + + return _Tool + + +def _patch_tool(monkeypatch, **kwargs): + monkeypatch.setattr( + "ymir.sweep.y_stream.CheckCveTriageEligibilityTool", + _fake_eligibility_tool(**kwargs), + ) + + +# --------------------------------------------------------------------------- +# YStreamSweep.is_unblocked +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_pending_dependencies_still_blocked(monkeypatch): + _patch_tool( + monkeypatch, + result_dict=_elig( + TriageEligibility.PENDING_DEPENDENCIES, + reason="waiting for Z-stream clone to ship", + pending=["RHEL-11111"], + ), + ) + + result = await YStreamSweep().is_unblocked(make_issue(), _COMMENT_DATA) + + assert result.action == "still_blocked" + assert result.detail == "waiting for Z-stream clone to ship" + + +@pytest.mark.asyncio +async def test_immediately_unblocked(monkeypatch): + _patch_tool( + monkeypatch, + result_dict=_elig(TriageEligibility.IMMEDIATELY, reason="at least one clone shipped"), + ) + + result = await YStreamSweep().is_unblocked(make_issue(), _COMMENT_DATA) + + assert result.action == "unblocked" + assert "immediately" in result.detail + + +@pytest.mark.asyncio +async def test_never_without_error_unblocked(monkeypatch): + """The behavioural heart of the refactor: a terminal NEVER verdict must + leave the sweep population (unblock + re-triage), not stay postponed.""" + _patch_tool( + monkeypatch, + result_dict=_elig(TriageEligibility.NEVER, reason="CentOS Stream first — fix inherited"), + ) + + result = await YStreamSweep().is_unblocked(make_issue(), _COMMENT_DATA) + + assert result.action == "unblocked" + assert "never" in result.detail + + +@pytest.mark.asyncio +async def test_never_with_error_is_error(monkeypatch): + """Guards the ordering bug: a transient failure rides on NEVER + error and + must keep the issue postponed, not un-postpone it.""" + _patch_tool( + monkeypatch, + result_dict=_elig( + TriageEligibility.NEVER, + reason="clone dependency check failed", + error="clone dependency check failed: timeout", + ), + ) + + result = await YStreamSweep().is_unblocked(make_issue(), _COMMENT_DATA) + + assert result.action == "error" + assert "timeout" in result.detail + + +@pytest.mark.asyncio +async def test_tool_raises_is_error(monkeypatch): + _patch_tool(monkeypatch, raises=RuntimeError("Jira unreachable")) + + result = await YStreamSweep().is_unblocked(make_issue(), _COMMENT_DATA) + + assert result.action == "error" + assert "Jira unreachable" in result.detail + + +@pytest.mark.asyncio +async def test_never_with_duplicate_unblocked(monkeypatch): + """A NEVER-due-to-duplicate verdict unblocks; duplicate handling is + delegated to triage, not reimplemented in the sweep.""" + _patch_tool( + monkeypatch, + result_dict=_elig( + TriageEligibility.NEVER, + reason="duplicate of RHEL-9", + duplicate_of="RHEL-9", + ), + ) + + result = await YStreamSweep().is_unblocked(make_issue(), _COMMENT_DATA) + + assert result.action == "unblocked" diff --git a/ymir/sweep/y_stream.py b/ymir/sweep/y_stream.py new file mode 100644 index 000000000..23eb1ffac --- /dev/null +++ b/ymir/sweep/y_stream.py @@ -0,0 +1,97 @@ +"""Y-stream sweep strategy. + +Re-checks issues tagged ``ymir_postponed_y_stream`` — Y-stream CVEs that +were postponed because their Z-stream clones had not shipped yet +(the ``PENDING_DEPENDENCIES`` eligibility verdict in the triage agent). + +Rather than reimplementing the shipping/dependency logic, this strategy +re-runs the very check that produced the postponement in the first place: +``CheckCveTriageEligibilityTool``. ``PENDING_DEPENDENCIES`` is the *only* +eligibility verdict that maps to the ``ymir_postponed_y_stream`` label +(see ``ymir/agents/triage_agent.py``), so the sweep is simply its inverse: +the issue stays blocked while eligibility is still ``PENDING_DEPENDENCIES`` +and is unblocked the moment it is anything else. + +Delegating to the eligibility tool keeps triage-time and sweep-time logic +from ever diverging, and inherits all of the tool's handling +(SecurityTracking, target-version normalisation, duplicate detection, +embargo, severity, Y/Z-stream branching, and the "at least one clone +shipped" rule). +""" + +from ymir.common import CVEEligibilityResult, TriageEligibility +from ymir.common.constants import JiraLabels +from ymir.supervisor.supervisor_types import FullIssue +from ymir.sweep.base import SweepResult, SweepStrategy +from ymir.sweep.comment_parser import CommentData +from ymir.tools.privileged.jira import CheckCveTriageEligibilityTool + + +class YStreamSweep(SweepStrategy): + """Re-runs the CVE eligibility check for postponed Y-stream issues. + + For each issue the strategy: + + 1. Re-runs ``CheckCveTriageEligibilityTool`` for the issue key. + 2. On a tool failure or an eligibility ``error`` → keeps the issue + postponed and retries on the next sweep (a transient Jira/Koji + hiccup must not un-postpone an issue — several transient failures + are reported as ``eligibility=NEVER`` *with* an ``error`` field, + so the ``error`` check must come before the eligibility branch). + 3. While eligibility is still ``PENDING_DEPENDENCIES`` → still blocked. + 4. For any other verdict (``IMMEDIATELY``, or ``NEVER`` without an + error) → unblocks and re-triages. Re-triage applies the correct + terminal outcome — full analysis for ``IMMEDIATELY``, or an + open-ended / duplicate comment for ``NEVER`` — and never re-adds + the ``ymir_postponed_y_stream`` label, so there is no loop. + """ + + name = "y_stream" + label = JiraLabels.YMIR_POSTPONED_Y_STREAM + + async def is_unblocked(self, issue: FullIssue, comment_data: CommentData) -> SweepResult: + # comment_data is part of the shared SweepStrategy.is_unblocked contract + # (dependency/pr_pending rely on it) but is unused here: the eligibility + # tool re-derives everything it needs from the issue key. + issue_key = issue.key + + try: + output = await CheckCveTriageEligibilityTool().run(input={"issue_key": issue_key}) + except Exception as exc: + return SweepResult( + issue_key=issue_key, + action="error", + detail=f"Eligibility check failed for {issue_key}: {exc}", + ) + + result = CVEEligibilityResult.model_validate(output.result) + + # Transient/data errors are surfaced as an ``error`` field (often with + # eligibility=NEVER, e.g. "clone dependency check failed" or "no target + # release"). Keep the issue postponed and retry next sweep — do NOT + # un-postpone on a failure. Note: "no target release" is a persistent + # data problem rather than transient, so it will be re-checked every + # sweep; that is accepted as safer than flapping the label. + if result.error: + return SweepResult( + issue_key=issue_key, + action="error", + detail=f"Eligibility error for {issue_key}: {result.error}", + ) + + # PENDING_DEPENDENCIES always carries a non-empty pending_zstream_issues + # (see ymir/tools/privileged/jira.py), so no empty-pending edge case here. + if result.eligibility == TriageEligibility.PENDING_DEPENDENCIES: + return SweepResult( + issue_key=issue_key, + action="still_blocked", + detail=result.reason, + ) + + return SweepResult( + issue_key=issue_key, + action="unblocked", + detail=( + f"No longer PENDING_DEPENDENCIES ({result.eligibility.value}): {result.reason}. Re-triaging." + ), + )