chore(deps): upgrade PyYAML to address known security vulnerability #11012

Merged
HAL9000 merged 4 commits from pr-fix-9244-pyyaml-upgrade into master 2026-05-11 09:47:34 +00:00
6 changed files with 140 additions and 0 deletions
+5
View File
@@ -144,6 +144,11 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
### Security
- **PyYAML upgraded to >=6.0.3 to address known security vulnerability** (#9055):
Added an explicit `pyyaml>=6.0.3` dependency constraint to `pyproject.toml` to
prevent installation of vulnerable older versions with known YAML parsing security
issues.
- **aiohttp upgraded to >=3.13.4 to remediate CVE-2026-34513 and CVE-2026-34515** (#1549, #1544):
Added an explicit `aiohttp>=3.13.4` dependency constraint to `pyproject.toml` to remediate
two high-severity open redirect vulnerabilities. Both CVEs affect the CleverAgents platform's
+1
View File
6
@@ -40,3 +40,4 @@ Below are some of the specific details of various contributions.
* HAL 9000 has contributed the ACMS context path matching fix (PR #10975 / issue #10972): corrects `_path_matches()` and `_matches_pattern()` to properly match absolute fragment paths against relative glob patterns by auto-prefixing with `**/` before calling `PurePath.full_match()`, preventing silent inefficacy of include/exclude filters for absolute paths in fragment metadata.
* HAL 9000 has contributed database resource types (PostgreSQL, SQLite) with transaction-based sandbox strategy: implemented ``DatabaseResourceHandler`` providing full CRUD operations (`read`, `write`, `delete`, `list_children`) and connection validation with automatic credential masking for PostgreSQL and SQLite backends. Includes ``TransactionSandbox`` infrastructure wired into ``SandboxFactory``, BDD test coverage in ``features/database_resources.feature``, and Robot Framework integration tests in ``robot/database_resources.robot`` (PR #10591 / issue #8608, Epic #8568).
* HAL 9000 has contributed the agents plan rollback command (PR #8674 / issue #8557): implemented checkpoint-based plan state restoration with the `agents plan rollback <plan-id> [<checkpoint-id>]` CLI command as part of Epic #8493, enabling plans to be restored to previous checkpoints, discarding post-checkpoint decisions, and resuming execution from the rolled-back state. Supported by `--yes/-y`, `--to-checkpoint`, and `--format/-f` flags. Includes comprehensive BDD test coverage (>= 97%) for rollback, decision discarding, and plan resume functionality.
* HAL 9000 has contributed the PyYAML security upgrade (PR #11012 / issue #9055): added `pyyaml>=6.0.3` dependency constraint to address known YAML parsing vulnerabilities.
@@ -0,0 +1,23 @@
@tdd_issue @tdd_issue_9055
Feature: PyYAML is a declared project dependency with minimum secure version
As a CleverAgents developer
I want the PyYAML dependency to be listed in pyproject.toml with a minimum secure version
So that vulnerable older versions of PyYAML cannot be installed
Background:
Given the pyproject.toml file exists at "pyproject.toml"
@tdd_issue @tdd_issue_9055
Scenario: PyYAML is listed in project dependencies with minimum version constraint
When I read the project dependencies from pyproject.toml
Then the dependency list should include a package matching "pyyaml>="
@tdd_issue @tdd_issue_9055
Scenario: PyYAML minimum version is >=6.0.3
When I read the PyYAML dependency specification from pyproject.toml
Then the minimum version should be at least "6.0.3"
@tdd_issue @tdd_issue_9055
Scenario: yaml module is importable as a project dependency
When I attempt to import the "yaml" module
Then the import should succeed without errors
@@ -0,0 +1,108 @@
"""Step definitions for PyYAML security dependency verification (#9055).
Note: the shared steps for reading pyproject.toml and verifying importability
are defined in ``tdd_a2a_sdk_dependency_steps.py`` to avoid AmbiguousStep
Outdated
Review

BLOCKING — AmbiguousStep conflict: this step pattern is already defined in tdd_a2a_sdk_dependency_steps.py:14

Behave registers step patterns globally. When two step files define the same pattern string, Behave raises AmbiguousStep at test-run start, causing the entire unit_tests run to fail. This is the root cause of the persistent unit_tests CI failure.

The pattern @given('the pyproject.toml file exists at "{path}"') is already registered by:

features/steps/tdd_a2a_sdk_dependency_steps.py:14

Fix: Rename this step (and the corresponding Background line in the .feature file) to a unique pattern, e.g.:

@given('the pyproject.toml pyyaml config exists at "{path}"')
def step_pyyaml_pyproject_exists(context: Context, path: str) -> None:

And update features/security_pyyaml_dependency.feature Background:

  Background:
    Given the pyproject.toml pyyaml config exists at "pyproject.toml"

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — AmbiguousStep conflict: this step pattern is already defined in `tdd_a2a_sdk_dependency_steps.py:14`** Behave registers step patterns globally. When two step files define the same pattern string, Behave raises `AmbiguousStep` at test-run start, causing the entire `unit_tests` run to fail. This is the root cause of the persistent `unit_tests` CI failure. The pattern `@given('the pyproject.toml file exists at "{path}"')` is **already registered** by: ``` features/steps/tdd_a2a_sdk_dependency_steps.py:14 ``` **Fix:** Rename this step (and the corresponding Background line in the `.feature` file) to a unique pattern, e.g.: ```python @given('the pyproject.toml pyyaml config exists at "{path}"') def step_pyyaml_pyproject_exists(context: Context, path: str) -> None: ``` And update `features/security_pyyaml_dependency.feature` Background: ```gherkin Background: Given the pyproject.toml pyyaml config exists at "pyproject.toml" ``` --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
conflicts. This file contains only the PyYAML-specific step definitions.
Outdated
Review

BLOCKING — lint CI gate is still failing

The lint job (nox -s lint) is still failing on b9563a44. The likely cause is the nested function _flatten defined inside the if isinstance(data, dict): conditional block in _toml_to_python() (see lines 137–148). Ruff may flag this as a style or complexity violation.

Why this is a problem: lint is a required merge gate. The PR cannot be merged until nox -s lint exits 0.

Action required: Run nox -s lint locally and fix all ruff findings. If the issue is the nested _flatten function, consider moving it to module level or inlining the logic.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — `lint` CI gate is still failing** The `lint` job (`nox -s lint`) is still failing on `b9563a44`. The likely cause is the nested function `_flatten` defined inside the `if isinstance(data, dict):` conditional block in `_toml_to_python()` (see lines 137–148). Ruff may flag this as a style or complexity violation. **Why this is a problem:** `lint` is a required merge gate. The PR cannot be merged until `nox -s lint` exits 0. **Action required:** Run `nox -s lint` locally and fix all ruff findings. If the issue is the nested `_flatten` function, consider moving it to module level or inlining the logic. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
"""
from __future__ import annotations
import re
from typing import Any
from behave import then, when
Outdated
Review

BLOCKING — AmbiguousStep conflict: @when("I read the project dependencies from pyproject.toml") is already defined in tdd_a2a_sdk_dependency_steps.py:21

Same conflict as the @given step above — this pattern is registered globally and duplicated. Behave cannot resolve which implementation to call.

Fix: Rename to a unique pattern, e.g.:

@when("I read the pyyaml project dependencies from pyproject.toml")
def step_read_pyyaml_project_dependencies(context: Context) -> None:

And update the corresponding When line in security_pyyaml_dependency.feature scenarios.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — AmbiguousStep conflict: `@when("I read the project dependencies from pyproject.toml")` is already defined in `tdd_a2a_sdk_dependency_steps.py:21`** Same conflict as the `@given` step above — this pattern is registered globally and duplicated. Behave cannot resolve which implementation to call. **Fix:** Rename to a unique pattern, e.g.: ```python @when("I read the pyyaml project dependencies from pyproject.toml") def step_read_pyyaml_project_dependencies(context: Context) -> None: ``` And update the corresponding `When` line in `security_pyyaml_dependency.feature` scenarios. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
from behave.runner import Context
from packaging.version import parse as parse_version
@then('the dependency list should include a package matching "{pattern}"')
def step_dependency_matches_pattern(context: Context, pattern: str) -> None:
"""Assert that a dependency in the project dependencies matches the given regex pattern."""
deps: list[str] = context.project_dependencies
found = any(re.search(pattern, dep) for dep in deps)
assert found, (
f"No dependency matching '{pattern}' found in "
f"[project.dependencies]. Current deps: {deps}"
)
@when("I read the PyYAML dependency specification from pyproject.toml")
def step_read_pyyaml_dependency(context: Context) -> None:
"""Read and parse the PyYAML dependency from pyproject.toml."""
content = context.pyproject_path.read_bytes()
data: dict[str, Any] = _load_toml(content)
deps: list[str] = data.get("project", {}).get("dependencies", [])
# Find the pyyaml dependency entry
pyyaml_dep = None
for dep in deps:
if dep.strip().startswith("pyyaml"):
pyyaml_dep = dep.strip()
break
assert pyyaml_dep is not None, (
f"PyYAML dependency not found in [project.dependencies]. Current deps: {deps}"
)
# Parse the minimum version from the constraint
# Examples: "pyyaml>=6.0.3", "pyyaml>=6.0", "pyyaml==6.0.2"
match = re.search(r"(?:>=|==|~>)([\d]+(?:\.[\d]+)*(?:\.\w+)*)", pyyaml_dep)
assert match is not None, (
f"Could not parse version from PyYAML dependency: {pyyaml_dep}"
)
context.pyyaml_version_spec = pyyaml_dep
context.pyyaml_min_version = match.group(1)
@then('the minimum version should be at least "{expected}"')
def step_pyyaml_minimum_version(context: Context, expected: str) -> None:
"""Assert that the PyYAML minimum version is >= expected version."""
min_version = context.pyyaml_min_version
assert parse_version(min_version) >= parse_version(expected), (
f"PyYAML minimum version {min_version} is less than required {expected}. "
f"Dependency spec: {context.pyyaml_version_spec}"
)
def _flatten_toml_dict(d: dict[Any, Any]) -> dict[str, Any]:
Outdated
Review

BLOCKING — unit_tests CI gate is still failing

The unit_tests job (nox -s unit_tests) is still failing on b9563a44. The new Behave scenarios in security_pyyaml_dependency.feature must all pass.

Why this is a problem: unit_tests is a required merge gate, and coverage is skipped until it passes. Both are needed for merge.

Action required: Run nox -s unit_tests locally. Identify which scenario(s) are failing and what the error messages are, then fix the issue before pushing.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — `unit_tests` CI gate is still failing** The `unit_tests` job (`nox -s unit_tests`) is still failing on `b9563a44`. The new Behave scenarios in `security_pyyaml_dependency.feature` must all pass. **Why this is a problem:** `unit_tests` is a required merge gate, and `coverage` is skipped until it passes. Both are needed for merge. **Action required:** Run `nox -s unit_tests` locally. Identify which scenario(s) are failing and what the error messages are, then fix the issue before pushing. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
"""Recursively convert a tomlkit dict to a plain Python dict.
tomlkit wraps every value in a proxy object that behaves like the
Outdated
Review

BLOCKING — AmbiguousStep conflict: @when('I attempt to import the "{module_name}" module') is also defined in tdd_a2a_sdk_dependency_steps.py

This pattern is duplicated in the existing A2A SDK dependency step file.

Fix: Rename to a unique pattern, e.g.:

@when('I attempt to import the pyyaml "{module_name}" module')
def step_pyyaml_attempt_import(context: Context, module_name: str) -> None:

And update the corresponding When line in security_pyyaml_dependency.feature.

Alternatively, consider refactoring both files to import these generic utility steps from a shared step utility module.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — AmbiguousStep conflict: `@when('I attempt to import the "{module_name}" module')` is also defined in `tdd_a2a_sdk_dependency_steps.py`** This pattern is duplicated in the existing A2A SDK dependency step file. **Fix:** Rename to a unique pattern, e.g.: ```python @when('I attempt to import the pyyaml "{module_name}" module') def step_pyyaml_attempt_import(context: Context, module_name: str) -> None: ``` And update the corresponding `When` line in `security_pyyaml_dependency.feature`. Alternatively, consider refactoring both files to import these generic utility steps from a shared step utility module. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
underlying Python type but is not identical to it. This helper
Outdated
Review

BLOCKING — Version comparison uses == instead of >=

The step name says 'the minimum version should be at least {expected}' but the implementation asserts min_version == expected. These are semantically different: == 6.0.3 passes only when the constraint is pinned to exactly that version, and will fail if someone later raises the minimum to >=6.1.0 or >=7.0.

How to fix: Replace the equality check with a proper semantic version comparison:

from packaging.version import Version

assert Version(min_version) >= Version(expected), (
    f"PyYAML minimum version {min_version} is below required minimum {expected}. "
    f"Dependency spec: {context.pyyaml_version_spec}"
)

packaging is already a transitive dependency (via pip/setuptools), so no new dependency is needed. This makes the test forward-compatible.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Version comparison uses `==` instead of `>=`** The step name says 'the minimum version should be at least {expected}' but the implementation asserts `min_version == expected`. These are semantically different: `== 6.0.3` passes only when the constraint is pinned to exactly that version, and will fail if someone later raises the minimum to `>=6.1.0` or `>=7.0`. **How to fix:** Replace the equality check with a proper semantic version comparison: ```python from packaging.version import Version assert Version(min_version) >= Version(expected), ( f"PyYAML minimum version {min_version} is below required minimum {expected}. " f"Dependency spec: {context.pyyaml_version_spec}" ) ``` `packaging` is already a transitive dependency (via pip/setuptools), so no new dependency is needed. This makes the test forward-compatible. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Outdated
Review

BLOCKING — == version comparison instead of >= (UNADDRESSED from prior review)

The step is named the minimum version should be at least {expected} but the assertion uses min_version == expected. These are semantically opposite: equality fails if the minimum is ever raised above 6.0.3.

How to fix:

from packaging.version import Version

assert Version(min_version) >= Version(expected), (
    f"PyYAML minimum version {min_version} is below required minimum {expected}. "
    f"Dependency spec: {context.pyyaml_version_spec}"
)

packaging is already available as a transitive dependency — no new import needed in pyproject.toml. This also makes the test forward-compatible if the minimum is ever raised.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — `==` version comparison instead of `>=` (UNADDRESSED from prior review)** The step is named `the minimum version should be at least {expected}` but the assertion uses `min_version == expected`. These are semantically opposite: equality fails if the minimum is ever raised above 6.0.3. **How to fix:** ```python from packaging.version import Version assert Version(min_version) >= Version(expected), ( f"PyYAML minimum version {min_version} is below required minimum {expected}. " f"Dependency spec: {context.pyyaml_version_spec}" ) ``` `packaging` is already available as a transitive dependency — no new import needed in `pyproject.toml`. This also makes the test forward-compatible if the minimum is ever raised. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Outdated
Review

BLOCKING — Version equality check still present (unchanged from prior review)

The assertion assert min_version == expected is still here in this new commit. This was explicitly identified as a semantic bug in review ID 8046 and has not been addressed.

Why this is a problem: The step is named the minimum version should be at least "{expected}" but the implementation asserts exact equality. This will break the moment the minimum is raised (e.g. to >=6.1.0 or >=7.0) — the test would start failing even though the constraint is more secure than required.

How to fix (exact replacement):

from packaging.version import Version

assert Version(min_version) >= Version(expected), (
    f"PyYAML minimum version {min_version} is below required minimum {expected}. "
    f"Dependency spec: {context.pyyaml_version_spec}"
)

packaging is already a transitive dependency — no new package is needed.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Version equality check still present (unchanged from prior review)** The assertion `assert min_version == expected` is still here in this new commit. This was explicitly identified as a semantic bug in review ID 8046 and has not been addressed. **Why this is a problem:** The step is named `the minimum version should be at least "{expected}"` but the implementation asserts exact equality. This will break the moment the minimum is raised (e.g. to `>=6.1.0` or `>=7.0`) — the test would start failing even though the constraint is *more* secure than required. **How to fix (exact replacement):** ```python from packaging.version import Version assert Version(min_version) >= Version(expected), ( f"PyYAML minimum version {min_version} is below required minimum {expected}. " f"Dependency spec: {context.pyyaml_version_spec}" ) ``` `packaging` is already a transitive dependency — no new package is needed. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Outdated
Review

BLOCKING — Semantic version comparison bug: == instead of >= (3rd review, still unaddressed)

The step is named "the minimum version should be at least \"{expected}\"" but the body asserts min_version == expected. These are logically different: equality requires the constraint to be pinned to exactly 6.0.3, and will produce a false negative whenever the minimum is legitimately raised (e.g. to >=6.1.0 or >=7.0). The test is semantically incorrect relative to its own name.

Why this is a problem: This brittle assertion will break on any future security upgrade that raises the minimum, even though that is a more secure state. It also contributes to unit_tests CI failure.

Exact fix — replace lines 71–75 with:

from packaging.version import Version

assert Version(min_version) >= Version(expected), (
    f"PyYAML minimum version {min_version} is below required minimum {expected}. "
    f"Dependency spec: {context.pyyaml_version_spec}"
)

packaging is already a transitive dependency — no new package is needed.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Semantic version comparison bug: `==` instead of `>=` (3rd review, still unaddressed)** The step is named `"the minimum version should be at least \"{expected}\""` but the body asserts `min_version == expected`. These are logically different: equality requires the constraint to be pinned to exactly `6.0.3`, and will produce a false negative whenever the minimum is legitimately raised (e.g. to `>=6.1.0` or `>=7.0`). The test is semantically incorrect relative to its own name. **Why this is a problem:** This brittle assertion will break on any future security upgrade that raises the minimum, even though that is a *more* secure state. It also contributes to `unit_tests` CI failure. **Exact fix — replace lines 71–75 with:** ```python from packaging.version import Version assert Version(min_version) >= Version(expected), ( f"PyYAML minimum version {min_version} is below required minimum {expected}. " f"Dependency spec: {context.pyyaml_version_spec}" ) ``` `packaging` is already a transitive dependency — no new package is needed. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
strips those wrappers so the result is a plain ``dict[str, Any]``
Outdated
Review

BLOCKING — AmbiguousStep conflict: @then("the import should succeed without errors") is also defined in tdd_a2a_sdk_dependency_steps.py

Fourth duplicate step pattern in this file.

Fix: Rename to a unique pattern, e.g.:

@then("the pyyaml import should succeed without errors")
def step_pyyaml_import_succeeded(context: Context) -> None:

And update the corresponding Then line in security_pyyaml_dependency.feature.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — AmbiguousStep conflict: `@then("the import should succeed without errors")` is also defined in `tdd_a2a_sdk_dependency_steps.py`** Fourth duplicate step pattern in this file. **Fix:** Rename to a unique pattern, e.g.: ```python @then("the pyyaml import should succeed without errors") def step_pyyaml_import_succeeded(context: Context) -> None: ``` And update the corresponding `Then` line in `security_pyyaml_dependency.feature`. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
compatible with ``tomllib`` output.
"""
result: dict[str, Any] = {}
for key, value in d.items():
if isinstance(value, dict):
result[key] = _flatten_toml_dict(value)
Outdated
Review

BLOCKING — Version comparison uses == instead of >= (unaddressed from prior review)

The step docstring says "Assert that the PyYAML minimum version is >= expected version" but the assertion uses min_version == expected. This was flagged as a blocker in the previous review and has not been fixed. The incorrect comparison is also the direct cause of the unit_tests CI failure.

With == "6.0.3", this test will fail the moment anyone legitimately raises the minimum to >=6.1.0 or >=7.0, making it semantically wrong and fragile.

How to fix:

from packaging.version import Version

@then("the minimum version should be at least \"{expected}\"")
def step_pyyaml_minimum_version(context: Context, expected: str) -> None:
    """Assert that the PyYAML minimum version is >= expected version."""
    min_version = context.pyyaml_min_version
    assert Version(min_version) >= Version(expected), (
        f"PyYAML minimum version {min_version} is below required minimum {expected}. "
        f"Dependency spec: {context.pyyaml_version_spec}"
    )

packaging is already a transitive dependency (via pip/setuptools) — no new dependency required.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Version comparison uses `==` instead of `>=` (unaddressed from prior review)** The step docstring says "Assert that the PyYAML minimum version is >= expected version" but the assertion uses `min_version == expected`. This was flagged as a blocker in the previous review and **has not been fixed**. The incorrect comparison is also the direct cause of the `unit_tests` CI failure. With `== "6.0.3"`, this test will fail the moment anyone legitimately raises the minimum to `>=6.1.0` or `>=7.0`, making it semantically wrong and fragile. **How to fix:** ```python from packaging.version import Version @then("the minimum version should be at least \"{expected}\"") def step_pyyaml_minimum_version(context: Context, expected: str) -> None: """Assert that the PyYAML minimum version is >= expected version.""" min_version = context.pyyaml_min_version assert Version(min_version) >= Version(expected), ( f"PyYAML minimum version {min_version} is below required minimum {expected}. " f"Dependency spec: {context.pyyaml_version_spec}" ) ``` `packaging` is already a transitive dependency (via pip/setuptools) — no new dependency required. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
else:
result[key] = value
return result
def _toml_to_python(data: object) -> dict[str, Any]:
"""Convert a tomlkit document to a plain Python dict.
Delegates recursive flattening to the module-level
``_flatten_toml_dict`` helper so the logic stays testable in
isolation and ruff does not flag nested function definitions.
"""
if isinstance(data, dict):
return _flatten_toml_dict(data)
raise TypeError(f"Expected dict-like TOML table, got {type(data).__name__}")
def _load_toml(content: bytes) -> dict[str, Any]:
Outdated
Review

Suggestion — Dead code: step_uv_lock_specifier is never called from any scenario

The step @then('the version constraint in uv.lock should include specifier "{specifier}"') is defined but there is no corresponding Gherkin step in security_pyyaml_dependency.feature that invokes it. Dead step definitions pollute the step registry.

Either remove this step entirely, or add a scenario that exercises it. The uv.lock update is already validated by the commit contents, so removing it is the simpler path.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**Suggestion — Dead code: `step_uv_lock_specifier` is never called from any scenario** The step `@then('the version constraint in uv.lock should include specifier "{specifier}"')` is defined but there is no corresponding Gherkin step in `security_pyyaml_dependency.feature` that invokes it. Dead step definitions pollute the step registry. Either remove this step entirely, or add a scenario that exercises it. The uv.lock update is already validated by the commit contents, so removing it is the simpler path. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Outdated
Review

Suggestion — Dead step definition: step_uv_lock_specifier is never called (UNADDRESSED from prior review)

This step is defined but no scenario in security_pyyaml_dependency.feature invokes it. Dead step definitions pollute the Behave step registry and may trigger warnings in the quality or dead_code CI jobs (which are now failing).

Options:

  • Remove this step entirely (simplest — the uv.lock correctness is already verified by the commit contents), OR
  • Add a Gherkin scenario in the .feature file that exercises it.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**Suggestion — Dead step definition: `step_uv_lock_specifier` is never called (UNADDRESSED from prior review)** This step is defined but no scenario in `security_pyyaml_dependency.feature` invokes it. Dead step definitions pollute the Behave step registry and may trigger warnings in the quality or dead_code CI jobs (which are now failing). **Options:** - Remove this step entirely (simplest — the uv.lock correctness is already verified by the commit contents), OR - Add a Gherkin scenario in the `.feature` file that exercises it. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Outdated
Review

Suggestion — Dead step step_uv_lock_specifier still present (unresolved from prior review)

This step definition (@then('the version constraint in uv.lock should include specifier "{specifier}"')) remains defined but is still not referenced in any Gherkin scenario in security_pyyaml_dependency.feature. This was raised as a non-blocking suggestion in review ID 8046 and has not been addressed.

Recommendation: remove this step entirely since the uv.lock update is already verified by the uv.lock diff contents, and an unused step definition pollutes the step registry. If you want to keep it, add a scenario that exercises it.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**Suggestion — Dead step `step_uv_lock_specifier` still present (unresolved from prior review)** This step definition (`@then('the version constraint in uv.lock should include specifier "{specifier}"')`) remains defined but is still not referenced in any Gherkin scenario in `security_pyyaml_dependency.feature`. This was raised as a non-blocking suggestion in review ID 8046 and has not been addressed. Recommendation: remove this step entirely since the uv.lock update is already verified by the `uv.lock` diff contents, and an unused step definition pollutes the step registry. If you want to keep it, add a scenario that exercises it. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Outdated
Review

Suggestion — Dead step step_uv_lock_specifier is unreachable (3rd review, still unaddressed)

This step definition (@then('the version constraint in uv.lock should include specifier "{specifier}"')) is defined but no Gherkin scenario in security_pyyaml_dependency.feature invokes it. Unused step definitions pollute the Behave step registry and can trigger lint/dead-code warnings contributing to CI failures.

Recommendation: Remove this step entirely. The uv.lock content is already verified structurally by the diff, and any scenario calling this step would require additional fixture setup. If you want to keep it, add a scenario in the .feature file that exercises it.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**Suggestion — Dead step `step_uv_lock_specifier` is unreachable (3rd review, still unaddressed)** This step definition (`@then('the version constraint in uv.lock should include specifier "{specifier}"')`) is defined but no Gherkin scenario in `security_pyyaml_dependency.feature` invokes it. Unused step definitions pollute the Behave step registry and can trigger lint/dead-code warnings contributing to CI failures. **Recommendation:** Remove this step entirely. The uv.lock content is already verified structurally by the diff, and any scenario calling this step would require additional fixture setup. If you want to keep it, add a scenario in the `.feature` file that exercises it. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Outdated
Review

Suggestion — Dead step step_uv_lock_specifier still unreachable (4th review, still unaddressed)

This step is defined but no scenario in security_pyyaml_dependency.feature calls it. Dead Behave step definitions pollute the step registry.

Recommendation: Either remove this step or add a Gherkin scenario that exercises it. Removing it is the simpler path.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**Suggestion — Dead step `step_uv_lock_specifier` still unreachable (4th review, still unaddressed)** This step is defined but no scenario in `security_pyyaml_dependency.feature` calls it. Dead Behave step definitions pollute the step registry. **Recommendation:** Either remove this step or add a Gherkin scenario that exercises it. Removing it is the simpler path. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
"""Load a TOML file from bytes. Uses tomllib if available, else fallback."""
try:
import tomllib
return tomllib.loads(content.decode("utf-8"))
except ImportError: # pragma: no cover — Python <3.11
Outdated
Review

Suggestion — Dead step step_uv_lock_specifier is still unreferenced (unaddressed from prior review)

This step definition @then('the version constraint in uv.lock should include specifier "{specifier}"') has no corresponding Gherkin step in security_pyyaml_dependency.feature that calls it. It is dead code. This is now consistent with the quality CI job failing in this run (vulture dead-code detection), whereas it was passing before this commit.

How to fix: Either remove this step entirely, or add a Gherkin scenario in security_pyyaml_dependency.feature that exercises it. Removing is simpler — the uv.lock update is already verified by the presence of the pyyaml>=6.0.3 constraint in pyproject.toml.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**Suggestion — Dead step `step_uv_lock_specifier` is still unreferenced (unaddressed from prior review)** This step definition `@then('the version constraint in uv.lock should include specifier "{specifier}"')` has no corresponding Gherkin step in `security_pyyaml_dependency.feature` that calls it. It is dead code. This is now consistent with the `quality` CI job failing in this run (vulture dead-code detection), whereas it was passing before this commit. **How to fix:** Either remove this step entirely, or add a Gherkin scenario in `security_pyyaml_dependency.feature` that exercises it. Removing is simpler — the uv.lock update is already verified by the presence of the `pyyaml>=6.0.3` constraint in pyproject.toml. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
# Fallback for older Python versions without tomllib
import tomlkit
parsed_doc = tomlkit.parse(content.decode("utf-8"))
return _toml_to_python(parsed_doc)
+1
View File
@@ -47,6 +47,7 @@ dependencies = [
"tomlkit>=0.13.0", # TOML writing with comment preservation for config CLI
"tenacity>=8.2.0", # Retry framework for service layer resilience
"aiohttp>=3.13.4", # CVE-2026-34515 mitigation: open redirect vulnerability
"pyyaml>=6.0.3", # Security: address known YAML parsing vulnerabilities
Outdated
Review

BLOCKING — Wrong Type/ label on the PR

The PR currently carries Type/Bug, but the commit type is chore(deps): which is a dependency maintenance task, not a bug fix. The correct label is Type/Task.

Per CONTRIBUTING.md, exactly one Type/ label must be applied and it must accurately reflect the nature of the work. The linked issue #9055 is correctly labelled Type/Bug (a vulnerability report), but the PR implementing it is a chore — changing the label on the PR is required.

Action required: Change the PR label from Type/Bug to Type/Task.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Wrong `Type/` label on the PR** The PR currently carries `Type/Bug`, but the commit type is `chore(deps):` which is a dependency maintenance task, not a bug fix. The correct label is `Type/Task`. Per CONTRIBUTING.md, exactly one `Type/` label must be applied and it must accurately reflect the nature of the work. The linked issue #9055 is correctly labelled `Type/Bug` (a vulnerability report), but the PR implementing it is a chore — changing the label on the PR is required. **Action required:** Change the PR label from `Type/Bug` to `Type/Task`. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Outdated
Review

BLOCKING — Wrong Type/ label still applied to this PR (unresolved from prior review)

The PR still carries Type/Bug. As noted in review ID 8046, the commit type is chore(deps): which corresponds to Type/Task, not Type/Bug. The linked issue #9055 is correctly labelled Type/Bug (it is a vulnerability report), but this PR — which implements a dependency upgrade — is a chore/task.

Per CONTRIBUTING.md, exactly one Type/ label must be applied and it must accurately reflect the nature of the work.

Action required: Remove Type/Bug, apply Type/Task to this PR.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Wrong `Type/` label still applied to this PR (unresolved from prior review)** The PR still carries `Type/Bug`. As noted in review ID 8046, the commit type is `chore(deps):` which corresponds to `Type/Task`, not `Type/Bug`. The linked issue #9055 is correctly labelled `Type/Bug` (it is a vulnerability report), but this PR — which implements a dependency upgrade — is a chore/task. Per CONTRIBUTING.md, exactly one `Type/` label must be applied and it must accurately reflect the nature of the work. **Action required:** Remove `Type/Bug`, apply `Type/Task` to this PR. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Outdated
Review

BLOCKING — Wrong Type/ label on PR (3rd review, still unaddressed)

This PR still carries the Type/Bug label. The work performed is a chore(deps): dependency upgrade — the correct label is Type/Task.

Per CONTRIBUTING.md, exactly one Type/ label must be applied and it must accurately reflect the nature of the work. The linked issue #9055 is correctly labelled Type/Bug (it is a vulnerability report), but this PR — which implements a dependency constraint update — is a maintenance chore.

Action required: Remove Type/Bug, apply Type/Task to this PR via the Forgejo web UI or API.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Wrong `Type/` label on PR (3rd review, still unaddressed)** This PR still carries the `Type/Bug` label. The work performed is a `chore(deps):` dependency upgrade — the correct label is `Type/Task`. Per CONTRIBUTING.md, exactly one `Type/` label must be applied and it must accurately reflect the nature of the work. The linked issue #9055 is correctly labelled `Type/Bug` (it is a vulnerability report), but this PR — which implements a dependency constraint update — is a maintenance chore. **Action required:** Remove `Type/Bug`, apply `Type/Task` to this PR via the Forgejo web UI or API. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
"a2a-sdk>=0.3.0,<1.0.0", # A2A Python SDK — required transport for local (stdio) and server (HTTP) modes (ADR-047); pinned <1.0.0 (removed legacy A2AClient)
]
Generated
+2
View File
@@ -446,6 +446,7 @@ dependencies = [
{ name = "pydantic" },
{ name = "pydantic-settings" },
{ name = "python-ulid" },
{ name = "pyyaml" },
{ name = "restrictedpython" },
{ name = "rx" },
{ name = "structlog" },
@@ -528,6 +529,7 @@ requires-dist = [
{ name = "pytest", marker = "extra == 'dev'", specifier = ">=8.0.0" },
{ name = "pytest-asyncio", marker = "extra == 'dev'", specifier = ">=0.23.0" },
{ name = "pytest-cov", marker = "extra == 'dev'", specifier = ">=4.1.0" },
{ name = "pyyaml", specifier = ">=6.0.3" },
{ name = "python-ulid", specifier = ">=2.7.0" },
{ name = "radon", marker = "extra == 'dev'", specifier = ">=6.0.1" },
{ name = "restrictedpython", specifier = ">=7.0" },