From 0e58525e03b65a68cff1b2283597260c6c4b77d9 Mon Sep 17 00:00:00 2001 From: Laura Barcziova Date: Wed, 12 Aug 2026 10:29:27 +0200 Subject: [PATCH 1/2] Add shared rules support for package ecosystems Implements a new MCP tool to fetch shared rules from a central registry, allowing common guidelines to be defined once per ecosystem (Python, Perl, etc.) and automatically applied to groups of packages. Core changes: - New SharedRulesTool: Looks up which shared rule sets apply to a package by querying registry.yaml in rules/shared-rules repo - Returns JSON list of ecosystem names; agents fetch actual AGENTS.md content via existing get_maintainer_rules tool - TTL-based caching (1 hour) at tool instance level for registry.yaml - Centralize GITLAB_API_URL and RULES_NAMESPACE constants (currently pointing at ymir-rules-test for testing) - Register tool in MCP gateway alongside existing MaintainerRulesTool - Add PyYAML>=5.1 dependency for registry parsing - Full test coverage (8 test cases) for lookup and caching behavior Design rationale: - Tool returns only the list, not content (keeps it simple) - Agent orchestrates fetching (maintains control over precedence logic) - Per-package rules always take precedence over shared rules - Registry cached to avoid repeated GitLab API calls - Transient errors don't cache; permanent errors do (404, bad YAML) Assisted-by: Claude Opus 4.6 --- Containerfile.c10s | 1 + Containerfile.c9s | 1 + ymir/agents/backport_agent.py | 2 +- ymir/agents/cve_applicability_agent.py | 19 ++- ymir/agents/issue_verification_agent.py | 39 +++++- ymir/agents/mr_consolidation_agent.py | 2 +- ymir/agents/prompts/backport/instructions.j2 | 17 ++- .../prompts/backport/instructions_zstream.j2 | 17 ++- .../backport/prompt_fix_build_error.j2 | 16 ++- ymir/agents/prompts/rebase/instructions.j2 | 17 ++- ymir/agents/prompts/triage/prompt.j2 | 22 ++-- ymir/agents/rebase_agent.py | 2 +- ymir/agents/triage_agent.py | 2 + ymir/tools/constants.py | 5 + ymir/tools/privileged/gateway.py | 2 + ymir/tools/privileged/maintainer_rules.py | 7 +- ymir/tools/privileged/shared_rules.py | 122 ++++++++++++++++++ .../tests/unit/test_shared_rules.py | 97 ++++++++++++++ ymir/tools/requirements.txt | 1 + 19 files changed, 348 insertions(+), 43 deletions(-) create mode 100644 ymir/tools/privileged/shared_rules.py create mode 100644 ymir/tools/privileged/tests/unit/test_shared_rules.py diff --git a/Containerfile.c10s b/Containerfile.c10s index 7eabcc0ab..d4d5b3266 100644 --- a/Containerfile.c10s +++ b/Containerfile.c10s @@ -83,6 +83,7 @@ RUN pip3 install --no-cache-dir \ pytest-asyncio \ GitPython>=3.1.0 \ unidiff \ + PyYAML>=5.1 \ sentry-sdk>=2.13.0 \ && cd /usr/local/lib/python3.12/site-packages \ && patch -p5 -i /tmp/openinference-reasoning.patch \ diff --git a/Containerfile.c9s b/Containerfile.c9s index d039f42f3..399dc5786 100644 --- a/Containerfile.c9s +++ b/Containerfile.c9s @@ -85,6 +85,7 @@ RUN python3.11 -m venv --system-site-packages /opt/beeai-venv \ koji \ GitPython>=3.1.0 \ unidiff \ + PyYAML>=5.1 \ sentry-sdk>=2.13.0 \ && cd /opt/beeai-venv/lib/python3.11/site-packages \ && patch -p5 -i /tmp/openinference-reasoning.patch \ diff --git a/ymir/agents/backport_agent.py b/ymir/agents/backport_agent.py index 9f3500250..c9dcc7208 100644 --- a/ymir/agents/backport_agent.py +++ b/ymir/agents/backport_agent.py @@ -162,7 +162,7 @@ async def create_backport_agent( BuildSrpmTool(options=local_tool_options), ] - base_tools.extend([t for t in mcp_tools if t.name == "get_maintainer_rules"]) + base_tools.extend([t for t in mcp_tools if t.name in ["get_maintainer_rules", "get_shared_rules"]]) # Add clone_repository from MCP gateway (needed for dist-git workflow with auth) if fix_version and await is_older_zstream(fix_version): diff --git a/ymir/agents/cve_applicability_agent.py b/ymir/agents/cve_applicability_agent.py index 64d622a88..6f81fe98e 100644 --- a/ymir/agents/cve_applicability_agent.py +++ b/ymir/agents/cve_applicability_agent.py @@ -22,7 +22,9 @@ def create_applicability_agent( gateway_tools: list[Tool], local_tool_options: dict, ) -> ReasoningAgent: - extra_gateway_tools = [t for t in gateway_tools if t.name in ["get_jira_details", "get_maintainer_rules"]] + extra_gateway_tools = [ + t for t in gateway_tools if t.name in ["get_jira_details", "get_maintainer_rules", "get_shared_rules"] + ] return ReasoningAgent( name="ApplicabilityAgent", llm=get_chat_model(), @@ -136,10 +138,17 @@ def build_applicability_prompt( build flags, commented-out BuildRequires). Steps: - 0. Use get_maintainer_rules with package '{package}' to check for - maintainer-specific guidelines. If rules are found, treat them - as additional context — e.g. if they indicate rebuilds are always - relevant, classify as Inconclusive rather than Not Affected. + 0. Fetch rules in this order: + a. Call get_shared_rules with package '{package}' to discover + applicable shared rule sets. For each name returned, call + get_maintainer_rules with package="shared-rules" and + file_path="{{name}}/AGENTS.md" to fetch shared ecosystem rules. + b. Call get_maintainer_rules with package '{package}' to check + for package-specific guidelines. + If rules are found at either level, treat them as additional + context — e.g. if they indicate rebuilds are always relevant, + classify as Inconclusive rather than Not Affected. + Package-specific rules take precedence over shared rules. 1. Use get_jira_details on {jira_issue} to understand the CVE context and what is affected. Also check the Jira comments — maintainers may have left notes about whether diff --git a/ymir/agents/issue_verification_agent.py b/ymir/agents/issue_verification_agent.py index 914386e04..f8447fa2d 100644 --- a/ymir/agents/issue_verification_agent.py +++ b/ymir/agents/issue_verification_agent.py @@ -1,4 +1,5 @@ import asyncio +import json import logging import os import sys @@ -83,6 +84,36 @@ def _render_testing_analyst_prompt(input: TestingAnalystInput, after_baseline: b return render_template(template_name, input) +async def _fetch_shared_rules(gateway_tools: list, package: str) -> str: + """Fetch shared rules that apply to a package from the central registry.""" + try: + shared_rules_json = await run_tool( + "get_shared_rules", + available_tools=gateway_tools, + package=package, + ) + rule_names = json.loads(shared_rules_json) if shared_rules_json else [] + except Exception: + logger.warning("Failed to look up shared rules for %s", package) + return "" + + parts = [] + for name in rule_names: + try: + content = await run_tool( + "get_maintainer_rules", + available_tools=gateway_tools, + package="shared-rules", + file_path=f"{name}/AGENTS.md", + ) + if content and "not found" not in content.lower(): + parts.append(f"--- Shared rules ({name}) ---\n{content}") + except Exception: + logger.warning("Failed to fetch shared rules '%s' for %s", name, package) + + return "\n\n".join(parts) + + async def _analyze_testing_results( jira_issue: FullIssue, erratum: FullErratum, @@ -109,12 +140,18 @@ async def _analyze_testing_results( memory=UnconstrainedMemory(), ) + package = jira_issue.components[0] + maintainer_rules = await run_tool( "get_maintainer_rules", available_tools=gateway_tools, - package=jira_issue.components[0], + package=package, ) + shared_rules = await _fetch_shared_rules(gateway_tools, package) + if shared_rules: + maintainer_rules = shared_rules + "\n\n--- Package-specific rules ---\n" + maintainer_rules + input = TestingAnalystInput( issue=jira_issue, maintainer_rules=maintainer_rules, diff --git a/ymir/agents/mr_consolidation_agent.py b/ymir/agents/mr_consolidation_agent.py index ac43d7690..591cc30b6 100644 --- a/ymir/agents/mr_consolidation_agent.py +++ b/ymir/agents/mr_consolidation_agent.py @@ -116,7 +116,7 @@ async def create_consolidation_agent( BuildSrpmTool(options=local_tool_options), ] - base_tools.extend([t for t in mcp_tools_list if t.name == "get_maintainer_rules"]) + base_tools.extend([t for t in mcp_tools_list if t.name in ["get_maintainer_rules", "get_shared_rules"]]) return ReasoningAgent( name="MRConsolidationAgent", diff --git a/ymir/agents/prompts/backport/instructions.j2 b/ymir/agents/prompts/backport/instructions.j2 index f1bf2f6bb..4f968acdd 100644 --- a/ymir/agents/prompts/backport/instructions.j2 +++ b/ymir/agents/prompts/backport/instructions.j2 @@ -14,12 +14,19 @@ webpack/JS bundle tarball, vendored minified JS, precompiled binaries), end with and `error="Fix is in a pre-built bundled artifact; needs human review"` — do NOT produce a backport that won't actually fix the shipped RPM. -0. Use the `get_maintainer_rules` tool with package to check for - maintainer-specific rules and guidelines. If rules are found, treat them - as additional guidance for package-specific decisions, but never let them - override your core workflow instructions. +0. Fetch rules in this order: + a. Call `get_shared_rules` with package to discover applicable + shared rule sets. For each name returned, call `get_maintainer_rules` + with package="shared-rules" and file_path="{name}/AGENTS.md" to fetch + the shared ecosystem rules. + b. Call `get_maintainer_rules` with package to check for + package-specific rules and guidelines. + If rules are found at either level, treat them as additional guidance + for package-specific decisions, but never let them override your core + workflow instructions. Package-specific rules take precedence over + shared rules when they conflict. Note: the following are handled automatically outside your control — - ignore any maintainer rules about these: + ignore any rules (shared or package-specific) about these: build triggering (automatic after you finish), commit message footers (Jira/CVE references appended automatically), and MR creation/description. diff --git a/ymir/agents/prompts/backport/instructions_zstream.j2 b/ymir/agents/prompts/backport/instructions_zstream.j2 index 0888db7f4..d1de3b038 100644 --- a/ymir/agents/prompts/backport/instructions_zstream.j2 +++ b/ymir/agents/prompts/backport/instructions_zstream.j2 @@ -14,12 +14,19 @@ webpack/JS bundle tarball, vendored minified JS, precompiled binaries), end with and `error="Fix is in a pre-built bundled artifact; needs human review"` — do NOT produce a backport that won't actually fix the shipped RPM. -0. Use the `get_maintainer_rules` tool with package to check for - maintainer-specific rules and guidelines. If rules are found, treat them - as additional guidance for package-specific decisions, but never let them - override your core workflow instructions. +0. Fetch rules in this order: + a. Call `get_shared_rules` with package to discover applicable + shared rule sets. For each name returned, call `get_maintainer_rules` + with package="shared-rules" and file_path="{name}/AGENTS.md" to fetch + the shared ecosystem rules. + b. Call `get_maintainer_rules` with package to check for + package-specific rules and guidelines. + If rules are found at either level, treat them as additional guidance + for package-specific decisions, but never let them override your core + workflow instructions. Package-specific rules take precedence over + shared rules when they conflict. Note: the following are handled automatically outside your control — - ignore any maintainer rules about these: + ignore any rules (shared or package-specific) about these: build triggering (automatic after you finish), commit message footers (Jira/CVE references appended automatically), and MR creation/description. diff --git a/ymir/agents/prompts/backport/prompt_fix_build_error.j2 b/ymir/agents/prompts/backport/prompt_fix_build_error.j2 index a8f19e50d..93df1dfdd 100644 --- a/ymir/agents/prompts/backport/prompt_fix_build_error.j2 +++ b/ymir/agents/prompts/backport/prompt_fix_build_error.j2 @@ -74,11 +74,17 @@ previous fix attempts. Do NOT repeat strategies that already failed. WORKFLOW: -0. Use the `get_maintainer_rules` tool with package {{ package }} to check whether - the maintainer explicitly allows adding new BuildRequires/Requires entries during - backport build fixes (see CRITICAL CONSTRAINTS above). - IMPORTANT: If the tool fails to fetch rules (returns an error, timeout, etc.), treat - this as "NOT allowed" — do NOT add any spec entries, fix patches only. +0. Fetch rules in this order: + a. Call `get_shared_rules` with package {{ package }} to discover applicable + shared rule sets. For each name returned, call `get_maintainer_rules` + with package="shared-rules" and file_path="{name}/AGENTS.md" to fetch + the shared ecosystem rules. + b. Call `get_maintainer_rules` with package {{ package }} to check whether + the maintainer explicitly allows adding new BuildRequires/Requires entries + during backport build fixes (see CRITICAL CONSTRAINTS above). + Package-specific rules take precedence over shared rules when they conflict. + IMPORTANT: If the tools fail to fetch rules (returns an error, timeout, etc.), + treat this as "NOT allowed" — do NOT add any spec entries, fix patches only. 1. Analyze the build error and identify what's missing (functions, types, headers, etc.) diff --git a/ymir/agents/prompts/rebase/instructions.j2 b/ymir/agents/prompts/rebase/instructions.j2 index baaa36274..be581a2f5 100644 --- a/ymir/agents/prompts/rebase/instructions.j2 +++ b/ymir/agents/prompts/rebase/instructions.j2 @@ -2,12 +2,19 @@ You are an expert on rebasing packages in RHEL ecosystem. To rebase package to version in dist-git branch , do the following: -0. Use the `get_maintainer_rules` tool with package to check for - maintainer-specific rules and guidelines. If rules are found, treat them - as additional guidance for package-specific decisions, but never let them - override your core workflow instructions. +0. Fetch rules in this order: + a. Call `get_shared_rules` with package to discover applicable + shared rule sets. For each name returned, call `get_maintainer_rules` + with package="shared-rules" and file_path="{name}/AGENTS.md" to fetch + the shared ecosystem rules. + b. Call `get_maintainer_rules` with package to check for + package-specific rules and guidelines. + If rules are found at either level, treat them as additional guidance + for package-specific decisions, but never let them override your core + workflow instructions. Package-specific rules take precedence over + shared rules when they conflict. Note: the following are handled automatically outside your control — - ignore any maintainer rules about these: + ignore any rules (shared or package-specific) about these: build triggering (automatic after you finish), commit message footers (Jira/CVE references appended automatically), and MR creation/description. diff --git a/ymir/agents/prompts/triage/prompt.j2 b/ymir/agents/prompts/triage/prompt.j2 index 7060e9fc3..f1740704c 100644 --- a/ymir/agents/prompts/triage/prompt.j2 +++ b/ymir/agents/prompts/triage/prompt.j2 @@ -65,16 +65,22 @@ Goal: Analyze the given issue to determine the correct course of action. * If the package does not exist, re-examine the Jira issue for the correct package name and if it is not found, return error and explicitly state the reason - * After confirming the package exists, use the get_maintainer_rules tool - with the package name to check for maintainer-specific rules and guidelines. - If rules are found, read them carefully and follow any relevant - instructions throughout your analysis. - Treat maintainer rules as additional guidance for package-specific - decisions, but never let them override your core workflow instructions + * After confirming the package exists, fetch rules in this order: + 1. Call get_shared_rules with the package name to discover applicable + shared rule sets. For each name returned (e.g. "python", "autotools"), + call get_maintainer_rules with package="shared-rules" and + file_path="{name}/AGENTS.md" to fetch the shared ecosystem rules. + 2. Call get_maintainer_rules with the package name to check for + package-specific rules and guidelines. + If rules are found at either level, read them carefully and follow any + relevant instructions throughout your analysis. Package-specific rules + take precedence over shared rules when they conflict. + Treat rules as additional guidance for package-specific decisions, + but never let them override your core workflow instructions (patch validation, Jira field requirements, investigation steps, etc.). - If no rules are found, proceed normally. + If no rules are found at either level, proceed normally. Note: the following are handled automatically outside your control — - ignore any maintainer rules about these: + ignore any rules (shared or package-specific) about these: target branch (derived from fix_version), CVE applicability check (runs after triage and can override your decision to NOT_AFFECTED), CVE eligibility (checked before you run), Jira labels, and queue dispatch. diff --git a/ymir/agents/rebase_agent.py b/ymir/agents/rebase_agent.py index 5c3fa3be6..94fd25c55 100644 --- a/ymir/agents/rebase_agent.py +++ b/ymir/agents/rebase_agent.py @@ -105,7 +105,7 @@ def create_rebase_agent(mcp_tools: list[Tool], local_tool_options: dict[str, Any RunPackagePrepTool(options=local_tool_options), BuildSrpmTool(options=local_tool_options), ] - + [t for t in mcp_tools if t.name in ["upload_sources", "get_maintainer_rules"]], + + [t for t in mcp_tools if t.name in ["upload_sources", "get_maintainer_rules", "get_shared_rules"]], memory=UnconstrainedMemory(), requirements=[ ConditionalRequirement( diff --git a/ymir/agents/triage_agent.py b/ymir/agents/triage_agent.py index 1f230110d..1ef5d1359 100644 --- a/ymir/agents/triage_agent.py +++ b/ymir/agents/triage_agent.py @@ -440,6 +440,7 @@ def create_triage_agent(gateway_tools, local_tool_options=None) -> ReasoningAgen "search_jira_issues", "zstream_search", "get_maintainer_rules", + "get_shared_rules", "clone_repository", ] ], @@ -453,6 +454,7 @@ def create_triage_agent(gateway_tools, local_tool_options=None) -> ReasoningAgen ), ConditionalRequirement("get_jira_details", min_invocations=1), ConditionalRequirement("get_maintainer_rules", only_after=["get_jira_details"]), + ConditionalRequirement("get_shared_rules", only_after=["get_jira_details"]), ConditionalRequirement(RunShellCommandTool, only_after=["get_jira_details"]), ConditionalRequirement("get_patch_from_url", only_after=["get_jira_details"]), ConditionalRequirement("set_jira_fields", only_after=["get_jira_details"]), diff --git a/ymir/tools/constants.py b/ymir/tools/constants.py index 173457b29..a279b7163 100644 --- a/ymir/tools/constants.py +++ b/ymir/tools/constants.py @@ -5,3 +5,8 @@ AIOHTTP_MAX_RETRIES = 3 AIOHTTP_RETRY_BACKOFF_BASE = 2 # seconds; delay = base * 2^attempt YMIR_USER_AGENT = "redhat-ymir-agent" + +GITLAB_API_URL = "https://gitlab.com/api/v4" +# use for production: +# RULES_NAMESPACE = "redhat/centos-stream/rules" +RULES_NAMESPACE = "ymir-rules-test" diff --git a/ymir/tools/privileged/gateway.py b/ymir/tools/privileged/gateway.py index 75c36846d..f23c0f17d 100644 --- a/ymir/tools/privileged/gateway.py +++ b/ymir/tools/privileged/gateway.py @@ -67,6 +67,7 @@ UploadSourcesTool, ) from ymir.tools.privileged.maintainer_rules import MaintainerRulesTool +from ymir.tools.privileged.shared_rules import SharedRulesTool from ymir.tools.privileged.testing_farm import ( CancelTestingFarmRequestTool, CopyFilesToRemoteTool, @@ -167,6 +168,7 @@ async def _async_main(): UploadSourcesTool(options=tool_options), ZStreamSearchTool(options=tool_options), MaintainerRulesTool(options=tool_options), + SharedRulesTool(options=tool_options), *log_detective_tools, ] ) diff --git a/ymir/tools/privileged/maintainer_rules.py b/ymir/tools/privileged/maintainer_rules.py index 3bc544f3e..8e5eecc78 100644 --- a/ymir/tools/privileged/maintainer_rules.py +++ b/ymir/tools/privileged/maintainer_rules.py @@ -9,16 +9,11 @@ from pydantic import BaseModel, Field from ymir.tools.base import CloneableTool as Tool -from ymir.tools.constants import AIOHTTP_TIMEOUT, YMIR_USER_AGENT +from ymir.tools.constants import AIOHTTP_TIMEOUT, GITLAB_API_URL, RULES_NAMESPACE, YMIR_USER_AGENT from ymir.tools.http import aiohttp_get_with_retries logger = logging.getLogger(__name__) -GITLAB_API_URL = "https://gitlab.com/api/v4" -RULES_NAMESPACE = "redhat/centos-stream/rules" -# use for testing: -# RULES_NAMESPACE = "ymir-rules-test" - class MaintainerRulesInput(BaseModel): package: str = Field(description="Name of the CentOS Stream package to fetch maintainer rules for") diff --git a/ymir/tools/privileged/shared_rules.py b/ymir/tools/privileged/shared_rules.py new file mode 100644 index 000000000..715940739 --- /dev/null +++ b/ymir/tools/privileged/shared_rules.py @@ -0,0 +1,122 @@ +import json +import logging +import os +import time +from urllib.parse import quote + +import aiohttp +import yaml +from beeai_framework.context import RunContext +from beeai_framework.emitter import Emitter +from beeai_framework.tools import StringToolOutput, Tool, ToolError, ToolRunOptions +from pydantic import BaseModel, Field + +from ymir.tools.constants import AIOHTTP_TIMEOUT, GITLAB_API_URL, RULES_NAMESPACE, YMIR_USER_AGENT +from ymir.tools.http import aiohttp_get_with_retries + +logger = logging.getLogger(__name__) +SHARED_RULES_REPO = "shared-rules" +REGISTRY_FILE = "registry.yaml" +REGISTRY_TTL_SECONDS = 3600 # 1 hour + + +class SharedRulesInput(BaseModel): + package: str = Field( + description="Name of the CentOS Stream package to find applicable shared rule sets for" + ) + + +class SharedRulesTool(Tool[SharedRulesInput, ToolRunOptions, StringToolOutput]): + name = "get_shared_rules" + description = ( + "Look up which shared rule sets apply to a package. " + 'Returns a JSON list of shared rule set names (e.g. ["python", "autotools"]) ' + "from the central registry at gitlab.com/redhat/centos-stream/rules/shared-rules/registry.yaml. " + 'For each name returned, use get_maintainer_rules with package="shared-rules" and ' + 'file_path="{name}/AGENTS.md" to fetch the actual shared rules. ' + "Returns an empty list if no shared rules apply or the registry does not exist." + ) + input_schema = SharedRulesInput + + def __init__(self, **kwargs): + super().__init__(**kwargs) + self._registry_cache: dict[str, list[str]] | None = None + self._registry_fetched_at: float = 0 + + def _create_emitter(self) -> Emitter: + return Emitter.root().child( + namespace=["tool", "rules", self.name], + creator=self, + ) + + def _cache_and_return(self, registry: dict[str, list[str]] | None) -> dict[str, list[str]]: + self._registry_cache = registry + self._registry_fetched_at = time.monotonic() + return registry or {} + + async def _fetch_registry(self) -> dict[str, list[str]]: + if ( + self._registry_fetched_at + and (time.monotonic() - self._registry_fetched_at) < REGISTRY_TTL_SECONDS + ): + return self._registry_cache or {} + + project_path = quote(f"{RULES_NAMESPACE}/{SHARED_RULES_REPO}", safe="") + file_path = quote(REGISTRY_FILE, safe="") + url = f"{GITLAB_API_URL}/projects/{project_path}/repository/files/{file_path}/raw?ref=main" + + headers: dict[str, str] = {"User-Agent": YMIR_USER_AGENT} + if token := os.getenv("GITLAB_TOKEN"): + headers["PRIVATE-TOKEN"] = token + + async with ( + aiohttp.ClientSession(timeout=AIOHTTP_TIMEOUT) as session, + aiohttp_get_with_retries(session, url, headers=headers) as response, + ): + if response.status == 404: + logger.info("Shared rules registry not found (404)") + return self._cache_and_return(None) + + if response.status != 200: + text = await response.text() + logger.warning( + "Failed to fetch shared rules registry (HTTP %d): %s", + response.status, + text, + ) + return self._cache_and_return(None) + + raw = await response.text() + + try: + parsed = yaml.safe_load(raw) + except yaml.YAMLError: + logger.warning("Malformed YAML in shared rules registry") + return self._cache_and_return(None) + + if not isinstance(parsed, dict): + logger.warning("Shared rules registry is not a YAML mapping") + return self._cache_and_return(None) + + return self._cache_and_return(parsed) + + async def _run( + self, + tool_input: SharedRulesInput, + options: ToolRunOptions | None, + context: RunContext, + ) -> StringToolOutput: + try: + registry = await self._fetch_registry() + except TimeoutError as e: + raise ToolError("Timeout while fetching shared rules registry") from e + except Exception as e: + raise ToolError(f"Error fetching shared rules registry: {e}") from e + + matching = [ + name + for name, packages in registry.items() + if isinstance(packages, list) and tool_input.package in packages + ] + + return StringToolOutput(result=json.dumps(matching)) diff --git a/ymir/tools/privileged/tests/unit/test_shared_rules.py b/ymir/tools/privileged/tests/unit/test_shared_rules.py new file mode 100644 index 000000000..046980a75 --- /dev/null +++ b/ymir/tools/privileged/tests/unit/test_shared_rules.py @@ -0,0 +1,97 @@ +import json +import time +from unittest.mock import patch + +import pytest +from beeai_framework.tools import ToolError + +from ymir.tools.privileged.shared_rules import SharedRulesTool + +SAMPLE_REGISTRY = { + "python": ["python-requests", "python-urllib3", "python-cryptography"], + "perl": ["perl-Module-Build", "perl-Test-Simple"], + "autotools": ["curl", "python-cryptography"], +} + + +def _fresh_tool(): + return SharedRulesTool(options={"working_directory": None}) + + +def _tool_with_cached_registry(registry): + tool = _fresh_tool() + tool._registry_cache = registry + tool._registry_fetched_at = time.monotonic() + return tool + + +@pytest.mark.asyncio +async def test_package_found_in_one_ecosystem(): + tool = _tool_with_cached_registry(SAMPLE_REGISTRY) + result = await tool.run({"package": "python-requests"}) + assert json.loads(result.result) == ["python"] + + +@pytest.mark.asyncio +async def test_package_found_in_multiple_ecosystems(): + tool = _tool_with_cached_registry(SAMPLE_REGISTRY) + result = await tool.run({"package": "python-cryptography"}) + assert sorted(json.loads(result.result)) == ["autotools", "python"] + + +@pytest.mark.asyncio +async def test_package_not_found(): + tool = _tool_with_cached_registry(SAMPLE_REGISTRY) + result = await tool.run({"package": "nonexistent-package"}) + assert json.loads(result.result) == [] + + +@pytest.mark.asyncio +async def test_empty_registry(): + tool = _tool_with_cached_registry({}) + result = await tool.run({"package": "python-requests"}) + assert json.loads(result.result) == [] + + +@pytest.mark.asyncio +async def test_registry_cached_as_none(): + tool = _fresh_tool() + tool._registry_cache = None + tool._registry_fetched_at = time.monotonic() + result = await tool.run({"package": "python-requests"}) + assert json.loads(result.result) == [] + + +@pytest.mark.asyncio +async def test_non_list_values_skipped(): + tool = _tool_with_cached_registry({"python": ["pkg-a"], "bad_entry": "not-a-list"}) + result = await tool.run({"package": "pkg-a"}) + assert json.loads(result.result) == ["python"] + + +@pytest.mark.asyncio +async def test_caching_multiple_lookups_same_registry(): + tool = _tool_with_cached_registry(SAMPLE_REGISTRY) + + result1 = await tool.run({"package": "python-requests"}) + assert json.loads(result1.result) == ["python"] + + result2 = await tool.run({"package": "curl"}) + assert json.loads(result2.result) == ["autotools"] + + assert tool._registry_fetched_at > 0 + assert tool._registry_cache is SAMPLE_REGISTRY + + +@pytest.mark.asyncio +@patch.object(SharedRulesTool, "_fetch_registry") +async def test_transient_error_not_cached(mock_fetch): + mock_fetch.side_effect = [TimeoutError("connection timed out"), SAMPLE_REGISTRY] + tool = _fresh_tool() + + with pytest.raises(ToolError, match="Timeout"): + await tool.run({"package": "python-requests"}) + + result = await tool.run({"package": "python-requests"}) + assert json.loads(result.result) == ["python"] + assert mock_fetch.call_count == 2 diff --git a/ymir/tools/requirements.txt b/ymir/tools/requirements.txt index 509230bb8..e62b24e88 100644 --- a/ymir/tools/requirements.txt +++ b/ymir/tools/requirements.txt @@ -6,6 +6,7 @@ flexmock>=0.12.2 GitPython>=3.1.0 nitrate>=1.9.0 ogr>=0.55.0 +PyYAML>=5.1 requests>=2.32.0 requests-gssapi>=1.3.0 rpm>=0.4.0 From 45b8cb7b7556f4568a71382ce9a0b8402abdfe1a Mon Sep 17 00:00:00 2001 From: Laura Barcziova Date: Wed, 12 Aug 2026 10:43:48 +0200 Subject: [PATCH 2/2] Clarify that shared rules apply to BuildRequires permission check Update build fix prompt to clarify that both shared and package-specific rules are checked when determining if adding BuildRequires/Requires is allowed for backport build fixes. Changes: - Step 0: Clarify that rules (both shared and package-specific) are checked for BuildRequires permission, with package-specific taking precedence - Criterion 2: Reference "rules from step 0" instead of just "maintainer rules" - CRITICAL CONSTRAINTS: Use "rules" instead of "maintainer rules" throughout for consistency This ensures that ecosystem-level permissions (e.g., "Python packages can add BuildRequires for backport fixes") are respected, while still allowing per-package overrides. Assisted-by: Claude Opus 4.6 --- .../backport/prompt_fix_build_error.j2 | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/ymir/agents/prompts/backport/prompt_fix_build_error.j2 b/ymir/agents/prompts/backport/prompt_fix_build_error.j2 index 93df1dfdd..b5b064e91 100644 --- a/ymir/agents/prompts/backport/prompt_fix_build_error.j2 +++ b/ymir/agents/prompts/backport/prompt_fix_build_error.j2 @@ -23,7 +23,7 @@ CRITICAL CONSTRAINTS: DO NOT clone it again. DO NOT reset to base commit. - Spec file modification rules: - IF maintainer rules explicitly allow adding BuildRequires/Requires for backport fixes: + IF rules (shared or package-specific) explicitly allow adding BuildRequires/Requires for backport fixes: You may add NEW BuildRequires/Requires entries to the spec file, and ONLY when: * Adding BuildRequires: the build error shows missing headers/libraries/executables that are DIRECTLY INTRODUCED by your backported patch (check the patch diff to confirm). @@ -47,7 +47,7 @@ CRITICAL CONSTRAINTS: * Loosening/removing existing BuildRequires version constraints * Any changes to %changelog, Release field, existing Patch tags, or patch ordering - IF maintainer rules do NOT explicitly allow it (or no rules exist): + IF rules do NOT explicitly allow it (or no rules exist): NEVER modify the spec file — the build worked before your patches; fix the patches instead. The build runs in COPR, not on official RHEL builders. COPR environments may have differences (e.g. unbuffer/expect wrappers, pipefail behavior, locale settings) that @@ -79,17 +79,18 @@ WORKFLOW: shared rule sets. For each name returned, call `get_maintainer_rules` with package="shared-rules" and file_path="{name}/AGENTS.md" to fetch the shared ecosystem rules. - b. Call `get_maintainer_rules` with package {{ package }} to check whether - the maintainer explicitly allows adding new BuildRequires/Requires entries - during backport build fixes (see CRITICAL CONSTRAINTS above). - Package-specific rules take precedence over shared rules when they conflict. + b. Call `get_maintainer_rules` with package {{ package }} to fetch + package-specific rules. + + Check rules (both shared and package-specific) for BuildRequires/Requires + permission. Package-specific takes precedence if they conflict. IMPORTANT: If the tools fail to fetch rules (returns an error, timeout, etc.), treat this as "NOT allowed" — do NOT add any spec entries, fix patches only. 1. Analyze the build error and identify what's missing (functions, types, headers, etc.) 2. If the build error indicates missing dependencies that are DIRECTLY INTRODUCED by your - backported patch AND maintainer rules (from step 0) explicitly allow it: + backported patch AND rules (from step 0) explicitly allow it: - For missing headers/libraries or "command not found" during %build/%check: add BuildRequires - For executables/libraries/modules needed by the installed package at runtime: add Requires - If needed during build (%build or %check) AND at runtime: add both BuildRequires and Requires @@ -132,7 +133,7 @@ SPECIAL CONSIDERATIONS FOR TEST FAILURES: 7. Append a summary to {{ local_clone }}-upstream/build-logs/fix-attempts.md documenting: - What you identified as the root cause - Which commits you cherry-picked or what manual edits you made - - Any BuildRequires or Requires additions to the spec file (if maintainer rules allowed + - Any BuildRequires or Requires additions to the spec file (if rules allowed adding new entries), including what was added and why - The build result (pass/fail and error if applicable) @@ -148,13 +149,13 @@ SPECIAL CONSIDERATIONS FOR TEST FAILURES: Criterion 2 — Spec file modifications are justified: Run `git diff HEAD -- *.spec` in {{ local_clone }} to inspect spec changes. - Check maintainer rules (from the `get_maintainer_rules` call earlier): + Check rules from step 0 (both shared and package-specific): - IF maintainer rules do NOT explicitly allow spec modifications for backport fixes: + IF rules do NOT explicitly allow spec modifications for backport fixes: Verify the spec file was NOT modified. If the diff shows any spec changes, this criterion fails. - IF maintainer rules explicitly allow adding BuildRequires/Requires for backport fixes: + IF rules explicitly allow adding BuildRequires/Requires for backport fixes: If there are NO spec changes: criterion passes. If there ARE spec changes, verify ALL of the following: @@ -186,7 +187,7 @@ SPECIAL CONSIDERATIONS FOR TEST FAILURES: Criterion 3 — No unrelated changes: From the git diff output, verify your changes are limited to: - The patch file(s) listed in step 5 - - The spec file (ONLY if maintainer rules allow it AND Criterion 2 passes) + - The spec file (ONLY if rules allow it AND Criterion 2 passes) No other files in {{ local_clone }} should be modified. @@ -204,7 +205,7 @@ SPECIAL CONSIDERATIONS FOR TEST FAILURES: Criterion 6 — Changes match intent: Review each hunk in the regenerated patch file(s) AND any spec file changes - (if maintainer rules allow spec modifications). + (if rules allow spec modifications). For patch files: Every changed line must be directly needed to fix the build error or to backport the upstream fix.