feat: add model provider clients and CLI agent harnesses - #32
Conversation
kubernetes-prow
Bot
added
the
do-not-merge/work-in-progress
kubernetes-prow
Bot
added
cncf-cla: yes
|
Hi @richackard. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds workspace-aware agent execution, Gemini and OpenClaw CLI harnesses with trajectory parsing and capability materialization, shared CLI helpers, and Claude, Gemini, and Ollama model adapters with provider-specific tests. ChangesAgent workspace and capability foundation
Gemini CLI integration
OpenClaw CLI integration
Model provider adapters
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kubernetes-prow
Bot
added
the
size/XXL
kubernetes-prow
Bot
removed
the
do-not-merge/work-in-progress
| assert servers == {"mcp0": {"command": "srv"}} | ||
|
|
||
|
|
||
| def test_materialize_skills_writes_named_skill_files(tmp_path: Path): |
There was a problem hiding this comment.
Could you add a test that rejects a malicious/unexpected skill name?
There was a problem hiding this comment.
Done, see test_materialize_skills_rejects_malicious_names
janetkuo
added
ok-to-test
| # (start_new_session=True) and os.killpg(...) on timeout. Tracked as a | ||
| # separate, more intrusive change to generalize across all CLI agents. | ||
| try: | ||
| completed = subprocess.run( |
There was a problem hiding this comment.
We should be consistent about using devops_bench.core.subprocess across all cli agents.
There was a problem hiding this comment.
Done, replacing it with the run() from the core package.
| for skill_file in sorted(source.rglob(_SKILL_FILE)): | ||
| name, _description, content = parse_skill_md(str(skill_file)) | ||
| if not name or content is None: | ||
| continue | ||
| dest_dir = skills_root / name | ||
| dest_dir.mkdir(parents=True, exist_ok=True) | ||
| (dest_dir / _SKILL_FILE).write_text(content, encoding="utf-8") | ||
| written.append(name) |
There was a problem hiding this comment.
mkdir(exist_ok=True) + write_text means a second SKILL.md with a duplicate name overwrites the first, and written lists the name twice. A warning on collision would surface a misconfigured skills dir instead of silently dropping one.
There was a problem hiding this comment.
Done, will print out a warning and skip the duplicated one.
kubernetes-prow
Bot
added
cncf-cla: no
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
tests/unit/agents/test_agents_cli_openclaw.py (1)
93-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd type annotations to the test functions and helpers.
Test functions here (e.g.
test_parse_trajectory_export_folds_call_result_pairs, and themonkeypatch, tmp_pathfixtures throughout) and helpers like_make_subprocess_result/_install_oc_runare missing return/parameter annotations, unlike the annotated sibling tests (test_agents_base.pyuses-> None). Please annotate them for consistency.As per coding guidelines, "All Python code must include type hints", and as per path instructions, "Ensure test functions have proper type annotations and clean structure."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/agents/test_agents_cli_openclaw.py` at line 93, Add complete type annotations to the test functions in this module, including fixture parameters such as monkeypatch and tmp_path, and annotate helper functions like _make_subprocess_result and _install_oc_run with parameter and return types. Follow the existing sibling-test convention by using -> None for test functions and appropriate concrete or fixture types for helpers and parameters.Sources: Coding guidelines, Path instructions
tests/unit/agents/shared/test_cli_capabilities.py (1)
29-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate the new test modules per the tests type-annotation instruction. Both new test files omit the
-> None(and helper-> str) return annotations and leave fixture parameters (caplog,monkeypatch,tmp_path) untyped. As per path instructions ("Ensure test functions have proper type annotations and clean structure") and coding guidelines ("All Python code must include type hints").
tests/unit/agents/shared/test_cli_capabilities.py#L29-L166: add-> Noneto eachtest_*and typecaplog/monkeypatch.tests/unit/agents/test_agents_cli_gemini.py#L46-L677: add-> strto_stream,-> Noneto eachtest_*, and typemonkeypatch/tmp_path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/agents/shared/test_cli_capabilities.py` around lines 29 - 166, Add the required type annotations throughout tests/unit/agents/shared/test_cli_capabilities.py lines 29-166: annotate every test_* function with -> None and type caplog, monkeypatch, and tmp_path fixtures. In tests/unit/agents/test_agents_cli_gemini.py lines 46-677, annotate _stream with -> str, every test_* function with -> None, and type monkeypatch and tmp_path fixtures.Sources: Coding guidelines, Path instructions
devops_bench/models/gemini.py (1)
38-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the project's shared logger instead of stdlib
logging.
claude.pyusesdevops_bench.core.logging.get_logger("models.claude"); this file uses rawlogging.getLogger(__name__), bypassing whatever centralized configuration (formatting, handlers, levels) the shared helper provides.♻️ Proposed fix
-import logging +from devops_bench.core.logging import get_logger ... -_log = logging.getLogger(__name__) +_log = get_logger("models.gemini")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@devops_bench/models/gemini.py` at line 38, Replace the stdlib logger initialization in devops_bench.models.gemini with the project’s shared devops_bench.core.logging.get_logger helper, matching claude.py’s model logger naming convention and preserving the existing _log symbol.tests/unit/models/test_models_claude.py (1)
1-382: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest functions are missing type annotations across all three new model test modules.
None of the
def test_...(mocker)(or..., provider)) functions in these files annotate themockerfixture or the-> Nonereturn type, and parametrized tests don't annotateprovider: str. As per path instructions,tests/**/*.pyshould "Ensure test functions have proper type annotations and clean structure."
tests/unit/models/test_models_claude.py#L1-L382: annotate alldef test_...(mocker)signatures asdef test_...(mocker: MockerFixture) -> None, andprovider: strin the parametrized test.tests/unit/models/test_models_gemini.py#L1-L341: same annotation pattern for all test functions, including the parametrizedprovidertest.tests/unit/models/test_models_ollama.py#L1-L272: same annotation pattern for all test functions.As per path instructions,
tests/**/*.py: "Verify unit test coverage, fixture scoping, mock usage, and pytest assertions. Ensure test functions have proper type annotations and clean structure."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/models/test_models_claude.py` around lines 1 - 382, Add the pytest-mock MockerFixture import and annotate every test function in tests/unit/models/test_models_claude.py (lines 1-382), tests/unit/models/test_models_gemini.py (lines 1-341), and tests/unit/models/test_models_ollama.py (lines 1-272) with MockerFixture and a None return type; annotate each parametrized provider argument as str. Apply the same signature-only change consistently across all three files.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@devops_bench/agents/cli/openclaw/agent.py`:
- Around line 104-107: Update the bin-path ordering in the Node environment
selection flow so bins are compared by parsed semantic Node version rather than
lexicographic path text. Use a version-aware sort key for the paths consumed by
node_bin, preserving the existing empty-bins return and selecting the highest
installed version.
In `@devops_bench/models/gemini.py`:
- Around line 97-161: Normalize supported schema key aliases in
filter_schema_for_gemini before returning the filtered schema: map any_of to
anyOf, one_of to oneOf, and defs to $defs. Apply the canonical names while
processing the existing _LIST_SCHEMA_FIELD_NAMES and _DICT_SCHEMA_FIELD_NAMES
branches, preserving their recursive filtering behavior and avoiding duplicate
alias keys.
---
Nitpick comments:
In `@devops_bench/models/gemini.py`:
- Line 38: Replace the stdlib logger initialization in
devops_bench.models.gemini with the project’s shared
devops_bench.core.logging.get_logger helper, matching claude.py’s model logger
naming convention and preserving the existing _log symbol.
In `@tests/unit/agents/shared/test_cli_capabilities.py`:
- Around line 29-166: Add the required type annotations throughout
tests/unit/agents/shared/test_cli_capabilities.py lines 29-166: annotate every
test_* function with -> None and type caplog, monkeypatch, and tmp_path
fixtures. In tests/unit/agents/test_agents_cli_gemini.py lines 46-677, annotate
_stream with -> str, every test_* function with -> None, and type monkeypatch
and tmp_path fixtures.
In `@tests/unit/agents/test_agents_cli_openclaw.py`:
- Line 93: Add complete type annotations to the test functions in this module,
including fixture parameters such as monkeypatch and tmp_path, and annotate
helper functions like _make_subprocess_result and _install_oc_run with parameter
and return types. Follow the existing sibling-test convention by using -> None
for test functions and appropriate concrete or fixture types for helpers and
parameters.
In `@tests/unit/models/test_models_claude.py`:
- Around line 1-382: Add the pytest-mock MockerFixture import and annotate every
test function in tests/unit/models/test_models_claude.py (lines 1-382),
tests/unit/models/test_models_gemini.py (lines 1-341), and
tests/unit/models/test_models_ollama.py (lines 1-272) with MockerFixture and a
None return type; annotate each parametrized provider argument as str. Apply the
same signature-only change consistently across all three files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0863b849-d7a5-4858-a264-b323a7888ea9
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (23)
devops_bench/agents/base.pydevops_bench/agents/cli/__init__.pydevops_bench/agents/cli/gemini_cli/__init__.pydevops_bench/agents/cli/gemini_cli/agent.pydevops_bench/agents/cli/gemini_cli/parsing.pydevops_bench/agents/cli/openclaw/__init__.pydevops_bench/agents/cli/openclaw/agent.pydevops_bench/agents/cli/openclaw/parsing.pydevops_bench/agents/shared/__init__.pydevops_bench/agents/shared/cli_capabilities.pydevops_bench/agents/shared/skills.pydevops_bench/models/claude.pydevops_bench/models/gemini.pydevops_bench/models/ollama.pypyproject.tomltests/unit/agents/shared/__init__.pytests/unit/agents/shared/test_cli_capabilities.pytests/unit/agents/test_agents_base.pytests/unit/agents/test_agents_cli_gemini.pytests/unit/agents/test_agents_cli_openclaw.pytests/unit/models/test_models_claude.pytests/unit/models/test_models_gemini.pytests/unit/models/test_models_ollama.py
1d78c56 to
091b094
Compare
kubernetes-prow
Bot
added
cncf-cla: yes
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/unit/agents/test_agents_cli_openclaw.py (2)
93-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd type annotations to test and helper functions. Test functions (
test_*) and helpers such as_make_subprocess_result,_install_oc_run, and_bundle_writerlack return annotations (-> None) and typed fixture parameters (monkeypatch: pytest.MonkeyPatch,tmp_path: Path). This applies throughout the file.As per path instructions ("Ensure test functions have proper type annotations and clean structure") and coding guidelines ("All Python code must include type hints").
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/agents/test_agents_cli_openclaw.py` at line 93, Add type annotations throughout tests/unit/agents/test_agents_cli_openclaw.py: annotate every test function with -> None, type fixture parameters such as monkeypatch: pytest.MonkeyPatch and tmp_path: Path, and add appropriate return annotations to helpers including _make_subprocess_result, _install_oc_run, and _bundle_writer. Preserve the existing test behavior and structure.Sources: Coding guidelines, Path instructions
236-237: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWeak assertion never verifies the escaping path.
shlex.quote("hi 'world'")emits the'"'"'idiom, not'\'', so the first clause is always false and the test only relies on"hi 'world'" not in cmd. Assert the actual quoted form to genuinely validate single-quote escaping.♻️ Assert the real shlex output
- # Prompt single-quote must be escaped, not break the shell line. - assert "hi '\\''world'\\''" in cmd or "hi 'world'" not in cmd + import shlex + # Prompt single-quote must be shlex-escaped, not break the shell line. + assert shlex.quote("hi 'world'") in cmd🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/agents/test_agents_cli_openclaw.py` around lines 236 - 237, Update the assertion in the single-quote escaping test to require the exact shlex.quote output, including its '"'"' sequence, for the input containing an apostrophe. Remove the alternative substring-negation clause so the test directly verifies the escaping path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/unit/agents/test_agents_cli_openclaw.py`:
- Line 93: Add type annotations throughout
tests/unit/agents/test_agents_cli_openclaw.py: annotate every test function with
-> None, type fixture parameters such as monkeypatch: pytest.MonkeyPatch and
tmp_path: Path, and add appropriate return annotations to helpers including
_make_subprocess_result, _install_oc_run, and _bundle_writer. Preserve the
existing test behavior and structure.
- Around line 236-237: Update the assertion in the single-quote escaping test to
require the exact shlex.quote output, including its '"'"' sequence, for the
input containing an apostrophe. Remove the alternative substring-negation clause
so the test directly verifies the escaping path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ae4f070f-2398-4387-a190-67172775e965
📒 Files selected for processing (9)
devops_bench/agents/cli/openclaw/__init__.pydevops_bench/agents/cli/openclaw/agent.pydevops_bench/agents/cli/openclaw/parsing.pydevops_bench/agents/shared/__init__.pydevops_bench/agents/shared/cli_capabilities.pydevops_bench/agents/shared/skills.pytests/unit/agents/shared/__init__.pytests/unit/agents/shared/test_cli_capabilities.pytests/unit/agents/test_agents_cli_openclaw.py
🚧 Files skipped from review as they are similar to previous changes (6)
- tests/unit/agents/shared/init.py
- devops_bench/agents/shared/init.py
- devops_bench/agents/cli/openclaw/init.py
- devops_bench/agents/cli/openclaw/parsing.py
- devops_bench/agents/shared/cli_capabilities.py
- tests/unit/agents/shared/test_cli_capabilities.py
091b094 to
80b7735
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
devops_bench/models/ollama.py (1)
103-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMatch the abstract base class type hint signature.
Consider updating
list[dict]tolist[dict[str, Any]]to fully type-hint the dictionary and exactly match the signature defined in theLLMClientbase class. As per path instructions, all Python code must include type hints.♻️ Proposed refactor
- def extract_function_calls(self, response: Any) -> list[dict]: - calls: list[dict] = [] + def extract_function_calls(self, response: Any) -> list[dict[str, Any]]: + calls: list[dict[str, Any]] = []🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@devops_bench/models/ollama.py` around lines 103 - 104, Update the return annotation and local calls collection in OllamaClient.extract_function_calls to use list[dict[str, Any]], matching the LLMClient abstract base signature and providing explicit dictionary value typing.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@devops_bench/models/ollama.py`:
- Around line 103-104: Update the return annotation and local calls collection
in OllamaClient.extract_function_calls to use list[dict[str, Any]], matching the
LLMClient abstract base signature and providing explicit dictionary value
typing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9b94b183-0041-4d2e-a4ef-eb0e4fddf619
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (23)
devops_bench/agents/base.pydevops_bench/agents/cli/__init__.pydevops_bench/agents/cli/gemini_cli/__init__.pydevops_bench/agents/cli/gemini_cli/agent.pydevops_bench/agents/cli/gemini_cli/parsing.pydevops_bench/agents/cli/openclaw/__init__.pydevops_bench/agents/cli/openclaw/agent.pydevops_bench/agents/cli/openclaw/parsing.pydevops_bench/agents/shared/__init__.pydevops_bench/agents/shared/cli_capabilities.pydevops_bench/agents/shared/skills.pydevops_bench/models/claude.pydevops_bench/models/gemini.pydevops_bench/models/ollama.pypyproject.tomltests/unit/agents/shared/__init__.pytests/unit/agents/shared/test_cli_capabilities.pytests/unit/agents/test_agents_base.pytests/unit/agents/test_agents_cli_gemini.pytests/unit/agents/test_agents_cli_openclaw.pytests/unit/models/test_models_claude.pytests/unit/models/test_models_gemini.pytests/unit/models/test_models_ollama.py
🚧 Files skipped from review as they are similar to previous changes (14)
- tests/unit/agents/shared/init.py
- devops_bench/agents/shared/init.py
- devops_bench/agents/cli/openclaw/init.py
- devops_bench/agents/cli/gemini_cli/init.py
- pyproject.toml
- devops_bench/agents/cli/init.py
- tests/unit/agents/test_agents_base.py
- devops_bench/agents/cli/openclaw/parsing.py
- devops_bench/agents/base.py
- devops_bench/agents/shared/cli_capabilities.py
- tests/unit/models/test_models_gemini.py
- devops_bench/models/claude.py
- tests/unit/agents/shared/test_cli_capabilities.py
- tests/unit/models/test_models_claude.py
Signed-off-by: Richard Huang <richackard@gmail.com>
Signed-off-by: Richard Huang <richackard@gmail.com>
Signed-off-by: Richard Huang <richackard@gmail.com>
80b7735 to
1854925
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
devops_bench/agents/cli/openclaw/agent.py (1)
370-376: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHonor inherited
NVM_DIRin the bash wrapper
_ensure_node_on_pathalready falls back toos.environ.get("NVM_DIR"), but this command hardcodes"$HOME/.nvm". Use the same fallback here so the agent turn still sources nvm on hosts with a custom install path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@devops_bench/agents/cli/openclaw/agent.py` around lines 370 - 376, Update the bash wrapper construction in the function containing the shown return to use the inherited NVM_DIR value, falling back to the default home-based path consistently with _ensure_node_on_path. Ensure the generated export and nvm.sh source commands target that resolved path instead of hardcoding "$HOME/.nvm", while preserving the existing oc command arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@devops_bench/agents/cli/openclaw/agent.py`:
- Around line 370-376: Update the bash wrapper construction in the function
containing the shown return to use the inherited NVM_DIR value, falling back to
the default home-based path consistently with _ensure_node_on_path. Ensure the
generated export and nvm.sh source commands target that resolved path instead of
hardcoding "$HOME/.nvm", while preserving the existing oc command arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: afcd66e9-294a-40df-9012-cd4a340f462c
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (23)
devops_bench/agents/base.pydevops_bench/agents/cli/__init__.pydevops_bench/agents/cli/gemini_cli/__init__.pydevops_bench/agents/cli/gemini_cli/agent.pydevops_bench/agents/cli/gemini_cli/parsing.pydevops_bench/agents/cli/openclaw/__init__.pydevops_bench/agents/cli/openclaw/agent.pydevops_bench/agents/cli/openclaw/parsing.pydevops_bench/agents/shared/__init__.pydevops_bench/agents/shared/cli_capabilities.pydevops_bench/agents/shared/skills.pydevops_bench/models/claude.pydevops_bench/models/gemini.pydevops_bench/models/ollama.pypyproject.tomltests/unit/agents/shared/__init__.pytests/unit/agents/shared/test_cli_capabilities.pytests/unit/agents/test_agents_base.pytests/unit/agents/test_agents_cli_gemini.pytests/unit/agents/test_agents_cli_openclaw.pytests/unit/models/test_models_claude.pytests/unit/models/test_models_gemini.pytests/unit/models/test_models_ollama.py
🚧 Files skipped from review as they are similar to previous changes (14)
- devops_bench/agents/shared/init.py
- devops_bench/agents/cli/openclaw/init.py
- devops_bench/agents/cli/init.py
- tests/unit/agents/shared/init.py
- devops_bench/agents/cli/gemini_cli/init.py
- tests/unit/agents/test_agents_base.py
- devops_bench/models/claude.py
- pyproject.toml
- devops_bench/agents/cli/openclaw/parsing.py
- tests/unit/agents/shared/test_cli_capabilities.py
- devops_bench/agents/shared/cli_capabilities.py
- tests/unit/models/test_models_gemini.py
- devops_bench/agents/base.py
- tests/unit/models/test_models_claude.py
Signed-off-by: Richard Huang <richackard@gmail.com>
Signed-off-by: Richard Huang <richackard@gmail.com>
Thread an optional workspace_path through AgentHarness.run() and _execute() so the harness can hand agents a working directory it owns and collect the files they write there afterward. Agents without a local filesystem workspace may ignore it. Signed-off-by: Pradeep Varadharajan <pvaradharajan@google.com>
1854925 to
f4ae39e
Compare
|
|
||
| import re | ||
|
|
||
| import yaml |
There was a problem hiding this comment.
Importing yaml relies on PyYAML, which is not declared in pyproject.toml (which specifies ruamel.yaml>=0.18.0)
There was a problem hiding this comment.
Done, changed to ruamel.yaml instead.
Skill names taken from SKILL.md frontmatter are validated before materialization: a name that would escape the skills root (path separators, "..", or an absolute prefix) is warned and skipped, and a duplicate name keeps the first discovered skill instead of silently overwriting it. Signed-off-by: Richard Huang <richackard@gmail.com>
f4ae39e to
459dd16
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pyproject.toml (1)
42-46: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConstrain provider SDKs to the tested compatibility window.
These lower-bound-only requirements allow consumers to install future API versions that the adapter tests may not cover. For example, OpenAI 2.x satisfies
openai>=1.0.0; use tested upper bounds or CI coverage for supported SDK versions. (pypi.org)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` around lines 42 - 46, Update the provider entries under [project.optional-dependencies] to constrain anthropic and openai SDK versions to the compatibility ranges covered by the adapter tests, adding appropriate upper bounds rather than only lower bounds. Preserve the existing optional extra names and dependency structure.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/agents/test_agents_cli_openclaw.py`:
- Line 147: Update the total cost assertion in the affected test to use
pytest.approx(0.03) instead of exact equality, while preserving the existing
tokens["cost"]["total"] value being validated.
---
Nitpick comments:
In `@pyproject.toml`:
- Around line 42-46: Update the provider entries under
[project.optional-dependencies] to constrain anthropic and openai SDK versions
to the compatibility ranges covered by the adapter tests, adding appropriate
upper bounds rather than only lower bounds. Preserve the existing optional extra
names and dependency structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7b82979e-1e09-42ce-b580-3742f23204ac
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (17)
devops_bench/agents/base.pydevops_bench/agents/cli/__init__.pydevops_bench/agents/cli/gemini_cli/__init__.pydevops_bench/agents/cli/gemini_cli/agent.pydevops_bench/agents/cli/gemini_cli/parsing.pydevops_bench/agents/cli/openclaw/__init__.pydevops_bench/agents/cli/openclaw/agent.pydevops_bench/agents/cli/openclaw/parsing.pydevops_bench/agents/shared/__init__.pydevops_bench/agents/shared/cli_capabilities.pydevops_bench/agents/shared/skills.pypyproject.tomltests/unit/agents/shared/__init__.pytests/unit/agents/shared/test_cli_capabilities.pytests/unit/agents/test_agents_base.pytests/unit/agents/test_agents_cli_gemini.pytests/unit/agents/test_agents_cli_openclaw.py
🚧 Files skipped from review as they are similar to previous changes (10)
- devops_bench/agents/shared/init.py
- devops_bench/agents/cli/gemini_cli/init.py
- tests/unit/agents/shared/init.py
- devops_bench/agents/cli/openclaw/init.py
- tests/unit/agents/test_agents_base.py
- devops_bench/agents/shared/cli_capabilities.py
- devops_bench/agents/base.py
- devops_bench/agents/cli/init.py
- devops_bench/agents/cli/openclaw/parsing.py
- tests/unit/agents/shared/test_cli_capabilities.py
The oc agent turn runs through core.subprocess.run like every other external command, invoking bash as argv (["/bin/bash", "-c", ...]) so nvm.sh can be sourced without shell=True; timeout surfaces as SubprocessError. Signed-off-by: Richard Huang <richackard@gmail.com>
459dd16 to
0ca1d5a
Compare
kubernetes-prow
Bot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: itssimrank, janetkuo, richackard The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
the
approved
Flip the entries at blob parity with upstream, by upstream PR: - kubernetes-sigs/devops-bench#32 (model clients + CLI agent harnesses): agents/cli/gemini_cli/**, agents/cli/__init__.py - kubernetes-sigs/devops-bench#31 (chaos package): chaos/spec.py, chaos/__init__.py - kubernetes-sigs/devops-bench#29 (verification base + metrics judge): verification/base.py, verification/__init__.py, its base/package tests, metrics/geval.py, metrics/_skills.py, tests/unit/metrics/test_metrics_geval.py - kubernetes-sigs/devops-bench#30 (deployer abstraction + default stack): deployers/base.py, deployers/__init__.py, deployers/noop.py, both engine tests - kubernetes-sigs/devops-bench#34 (concrete judge metric families): metrics/__init__.py Drifted entries stay commented until gke-labs and upstream reconcile.
5 participants
Adds the three concrete LLM provider adapters and the first two CLI agent harnesses, plus the shared helpers the CLI harnesses use for capability wiring.
Model provider clients (
devops_bench/models/)gemini.py—GeminiClientAdapterovergoogle-genai: API-key or Vertex AI backends (provider-forced or inferred from the environment), schema filtering for Gemini's tool-declaration subset, and retry with backoff on transient errors.claude.py—ClaudeClientAdapteroveranthropic: API, Vertex, and Bedrock backends, backend inference from the environment with explicit override, and max-token config via env or argument.ollama.py—OllamaClientAdapterover the OpenAI-compatible client for local Ollama runtimes.Each adapter registers itself in the
MODELSregistry and resolves throughget_model()/core.model_providersaliases (covered by tests). Provider SDKs are imported lazily: a missing SDK surfaces asMissingDependencyErrorat construction, not import.CLI agent harnesses (
devops_bench/agents/cli/)gemini_cli/— drives the Gemini CLI binary in a per-run workspace, delivering capabilities through the CLI's native workspace channels (settings, MCP config, skills);parsing.pyconverts the stream-JSON output into a typedAgentResult.openclaw/— drives the OpenClaw CLI analogously, including session export parsing with per-turn token accounting.shared/— the capability-wiring helpers (cli_capabilities.py,skills.py) both harnesses consume.One commit extends
AgentHarness.run()to accept a harness-ownedworkspace_path; it is identical to #28 — if #28 merges first this branch rebases and the duplicate drops out, otherwise #28 can be closed.Dependencies
anthropicandopenaiare added as optional extras (install only the providers you use) and to the dev group so the full test suite resolves.google-genaiis already a core dependency.uv.lockrefreshed.Testing
uv sync --frozen && uv run ruff check && uv run pytest tests/unit -q— 482 passed. All new files carry the Apache 2.0 header.Summary by CodeRabbit