test: add TDD bug-capture test for #620 — skill add cross-process persistence #1110

Closed
brent.edwards wants to merge 2 commits from tdd/m3-skill-add-regression into master
7 changed files with 622 additions and 4 deletions
+6
View File
@@ -35,6 +35,12 @@
with `shlex.split()` and `shell=False` for defense-in-depth command injection
prevention, consistent with the existing pattern in
`cli_plan_context_commands_steps.py`. (#734)
- Added TDD bug-capture test for bug #620`skill add` does not persist
across CLI invocations. Behave BDD scenarios and Robot Framework
integration tests exercise the real `agents` CLI via `subprocess.run` to
verify cross-process skill persistence. Tagged with `@tdd_expected_fail`
until the fix is merged. Root cause identified as session mismatch between
`SkillRepository.create()` and `SkillService._commit()`. (#1091)
- Added BuiltinAdapter class and MCP automatic resource slot creation.
BuiltinAdapter wraps register_file_tools/register_git_tools/register_subplan_tool
into a unified adapter interface. McpAdapter.infer_resource_slots() analyzes
@@ -0,0 +1,282 @@
"""Step definitions for TDD Bug #620 — skill add cross-process persistence.
These steps capture bug #620 by exercising the *real* CLI binary via
``subprocess.run`` across independent process invocations. The
existing persistence tests (``skill_add_persist.feature``) verify
round-trip within the same Python process — they create two
``SkillService`` instances sharing the same in-memory SQLAlchemy
session factory. That approach cannot detect the cross-process
regression.
This test:
1. Creates a temporary CLEVERAGENTS_HOME with an on-disk SQLite DB.
2. Runs ``agents init --yes`` via ``subprocess.run`` to set up the
project and apply migrations.
3. Runs ``agents skill add --config <file> --format json`` via
``subprocess.run`` (simulating process 1).
4. Runs ``agents skill list --format json`` via a **separate**
``subprocess.run`` (simulating process 2) — the SkillService is
rebuilt from scratch in a new Python process.
5. Asserts the skill appears in the listing.
The ``@tdd_expected_fail`` tag on the feature inverts the assertion
failure to a CI pass while the bug is unfixed.
See CONTRIBUTING.md > Bug Fix Workflow and > TDD Bug Test Tags.
"""
from __future__ import annotations
import os
import shutil
import subprocess
import sys
import tempfile
from behave import given, then, when
from behave.runner import Context
from cleveragents.config.settings import Settings
_SKILL_YAML = """\
name: local/tdd-cross-process
description: "TDD cross-process persistence test skill"
tools:
- name: builtin/read_file
"""
_SKILL_NAME = "local/tdd-cross-process"
_SUBPROCESS_TIMEOUT = 30
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
def _make_subprocess_env(home_dir: str, db_url: str) -> dict[str, str]:
"""Build a clean environment for subprocess CLI calls.
Sets ``CLEVERAGENTS_HOME``, ``CLEVERAGENTS_DATABASE_URL``, and
``CLEVERAGENTS_AUTO_APPLY_MIGRATIONS`` so each subprocess finds
the correct project directory and database. ``NO_COLOR=1``
suppresses ANSI escape codes in captured output.
"""
env: dict[str, str] = os.environ.copy()
env["CLEVERAGENTS_HOME"] = home_dir
env["CLEVERAGENTS_DATABASE_URL"] = db_url
env["CLEVERAGENTS_AUTO_APPLY_MIGRATIONS"] = "true"
env["CLEVERAGENTS_TESTING_USE_MOCK_AI"] = "true"
env["NO_COLOR"] = "1"
return env
def _run_agents_cli(
args: list[str],
*,
env: dict[str, str],
cwd: str,
) -> subprocess.CompletedProcess[str]:
"""Run an ``agents`` CLI command as a subprocess.
Uses ``sys.executable -m cleveragents`` to ensure the correct
Python interpreter and installed package are used.
If the subprocess exceeds ``_SUBPROCESS_TIMEOUT`` seconds, a
synthetic ``CompletedProcess`` with return code ``-1`` is returned
instead of raising ``TimeoutExpired``. This ensures that step
assertions fail with ``AssertionError`` (not a raw exception),
which allows the ``@tdd_expected_fail`` inversion logic to work
correctly.
"""
cmd: list[str] = [sys.executable, "-m", "cleveragents", *args]
try:
return subprocess.run(
cmd,
capture_output=True,
text=True,
timeout=_SUBPROCESS_TIMEOUT,
cwd=cwd,
env=env,
encoding="utf-8",
)
except subprocess.TimeoutExpired:
return subprocess.CompletedProcess(
args=cmd,
returncode=-1,
stdout="",
stderr=f"subprocess timed out after {_SUBPROCESS_TIMEOUT}s",
)
# ---------------------------------------------------------------------------
# Given
# ---------------------------------------------------------------------------
@given("a cross-process skill persistence environment")
def step_cross_process_env(context: Context) -> None:
"""Create a temporary directory with a skill config YAML.
Sets up ``CLEVERAGENTS_HOME`` and ``CLEVERAGENTS_DATABASE_URL``
pointing to on-disk paths so that separate subprocess invocations
share the same database file.
"""
Settings._instance = None # type: ignore[attr-defined]
tmpdir: str = tempfile.mkdtemp(prefix="tdd620_skill_xproc_")
db_path: str = os.path.join(tmpdir, "cleveragents_tdd620.db")
db_url: str = f"sqlite:///{db_path}"
context.tdd_skill_home = tmpdir
context.tdd_skill_db_url = db_url
context.tdd_skill_env = _make_subprocess_env(tmpdir, db_url)
context.tdd_skill_cwd = str(
os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
)
# Write skill YAML config to a temp file
config_path: str = os.path.join(tmpdir, "test-skill.yaml")
with open(config_path, "w", encoding="utf-8") as fh:
fh.write(_SKILL_YAML)
context.tdd_skill_config_path = config_path
def _cleanup() -> None:
Settings._instance = None # type: ignore[attr-defined]
shutil.rmtree(tmpdir, ignore_errors=True)
context.add_cleanup(_cleanup)
@given("the project is initialised via subprocess")
def step_init_via_subprocess(context: Context) -> None:
"""Run ``agents init --yes`` as a subprocess to set up the database."""
result: subprocess.CompletedProcess[str] = _run_agents_cli(
["init", "--yes", "--path", context.tdd_skill_home],
env=context.tdd_skill_env,
cwd=context.tdd_skill_cwd,
)
assert result.returncode == 0, (
f"agents init failed (rc={result.returncode}).\n"
f"stdout: {result.stdout}\nstderr: {result.stderr}"
)
# ---------------------------------------------------------------------------
# When
# ---------------------------------------------------------------------------
@when("I add a skill via a CLI subprocess")
def step_add_skill_subprocess(context: Context) -> None:
"""Run ``agents skill add --config <file> --format json`` via subprocess.
This is a separate OS process — its own Python interpreter, DI
container, and database connection. The skill should be persisted
to the on-disk SQLite database.
"""
context.tdd_skill_add_result = _run_agents_cli(
[
"skill",
"add",
"--config",
context.tdd_skill_config_path,
"--format",
"json",
],
env=context.tdd_skill_env,
cwd=context.tdd_skill_cwd,
)
@when("I list skills via a separate CLI subprocess")
def step_list_skills_subprocess(context: Context) -> None:
"""Run ``agents skill list --format json`` in a separate subprocess.
This is the cross-process boundary that exposes bug #620: the new
process creates its own DI container and SkillService, which must
load skills from the database. If the previous ``skill add``
process failed to commit, this process sees an empty database.
"""
context.tdd_skill_list_result = _run_agents_cli(
["skill", "list", "--format", "json"],
env=context.tdd_skill_env,
cwd=context.tdd_skill_cwd,
)
@when("I show the skill via a separate CLI subprocess")
def step_show_skill_subprocess(context: Context) -> None:
"""Run ``agents skill show <name> --format json`` in a separate subprocess.
The process boundary is the same as the list step — a fresh Python
process with its own DI container.
"""
context.tdd_skill_show_result = _run_agents_cli(
["skill", "show", _SKILL_NAME, "--format", "json"],
env=context.tdd_skill_env,
cwd=context.tdd_skill_cwd,
)
# ---------------------------------------------------------------------------
# Then
# ---------------------------------------------------------------------------
@then("the skill add subprocess should exit successfully")
def step_assert_skill_add_succeeds(context: Context) -> None:
"""Assert that ``agents skill add`` exited with code 0."""
result: subprocess.CompletedProcess[str] = context.tdd_skill_add_result
assert result.returncode == 0, (
f"agents skill add failed (rc={result.returncode}).\n"
f"stdout: {result.stdout}\nstderr: {result.stderr}"
)
@then("the cross-process skill list should contain the added skill")
def step_assert_skill_in_list(context: Context) -> None:
"""Assert that the skill added in one process is visible in another.
Checks the combined stdout/stderr of ``skill list`` for the skill
name. This assertion will FAIL while bug #620 is present because
the second process creates a fresh SkillService that does not find
the skill in the database (the ``skills`` table is missing or was
never populated due to the DB-fallback in ``_build_skill_service``).
The ``@tdd_expected_fail`` tag inverts this failure to a CI pass.
"""
result: subprocess.CompletedProcess[str] = context.tdd_skill_list_result
combined: str = result.stdout + result.stderr
assert result.returncode == 0 and _SKILL_NAME in combined, (
f"Expected skill '{_SKILL_NAME}' in cross-process skill list output "
f"but it was not found.\n"
f"Add exit code: {context.tdd_skill_add_result.returncode}\n"
f"Add output:\n{context.tdd_skill_add_result.stdout}\n"
f"List exit code: {result.returncode}\n"
f"List output:\n{combined}"
)
@then("the cross-process skill show output should contain the skill name")
def step_assert_skill_show_contains_name(context: Context) -> None:
"""Assert that ``skill show`` in a separate process finds the skill.
This assertion will FAIL while bug #620 is present because the
second process cannot find the skill in its fresh service instance.
"""
result: subprocess.CompletedProcess[str] = context.tdd_skill_show_result
combined: str = result.stdout + result.stderr
assert result.returncode == 0 and _SKILL_NAME in combined, (
f"Expected skill '{_SKILL_NAME}' visible via cross-process "
f"'skill show' but it was not found.\n"
f"Show exit code: {result.returncode}\n"
f"Show output:\n{combined}\n"
f"Add exit code: {context.tdd_skill_add_result.returncode}\n"
f"Add output:\n{context.tdd_skill_add_result.stdout}"
)
+34
View File
@@ -0,0 +1,34 @@
@tdd_expected_fail @tdd_bug @tdd_bug_620
Feature: TDD Bug #620 — skill add cross-process persistence
As a developer
I want to verify that `agents skill add --config <file>` persists
skills across separate CLI process invocations
So that the bug is captured and will be caught by a regression test
Bug #620 reports that skills registered via `agents skill add` in one
CLI process are not visible when `agents skill list` is run in a
separate CLI process. Existing persistence tests pass because they
verify round-trip within the same Python process (creating two
SkillService instances sharing the same in-memory database).
This TDD test captures the regression by using real subprocess
invocations the skill is added via one CLI invocation and listed
via an independent CLI invocation, both sharing the same on-disk
SQLite database. The @tdd_expected_fail tag inverts the result so
CI passes while the bug is unfixed.
Scenario: skill add in one process is visible to skill list in another
Given a cross-process skill persistence environment
And the project is initialised via subprocess
When I add a skill via a CLI subprocess
Then the skill add subprocess should exit successfully
When I list skills via a separate CLI subprocess
Then the cross-process skill list should contain the added skill
Scenario: skill add persists config path across processes
Given a cross-process skill persistence environment
And the project is initialised via subprocess
When I add a skill via a CLI subprocess
Then the skill add subprocess should exit successfully
When I show the skill via a separate CLI subprocess
Then the cross-process skill show output should contain the skill name
+1
View File
@@ -933,6 +933,7 @@ def security_scan(session: nox.Session):
"""
session.install("-e", ".[dev]")
session.install("setuptools<81") # pkg_resources for semgrep
# Ensure output directory exists (CI starts with a clean checkout)
os.makedirs("build", exist_ok=True)
+253
View File
@@ -0,0 +1,253 @@
"""Helper script for tdd_skill_add_regression.robot smoke tests.
Each subcommand exercises the real ``agents`` CLI binary via
``subprocess.run`` to reproduce bug #620: skills registered via
``agents skill add --config <file>`` in one CLI invocation are not
visible when ``agents skill list`` is run in a separate CLI invocation.
The existing in-process persistence tests pass because they share an
in-memory SQLAlchemy session factory between two SkillService instances.
This helper runs each CLI command as an independent subprocess each
invocation has its own Python interpreter, DI container, and database
connection to capture the real cross-process regression.
The helper reports the **real** outcome: it exits 0 and prints the
sentinel when the skill is found cross-process (bug is fixed), and
exits 1 when the skill is not found (bug still present). The
``tdd_expected_fail_listener`` on the Robot side handles pass/fail
inversion while the bug remains open.
Bug: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/620
TDD: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/1091
"""
from __future__ import annotations
import os
import shutil
import subprocess
import sys
import tempfile
from collections.abc import Callable
from pathlib import Path
from typing import NoReturn
# Ensure local source tree is importable.
_ROOT: Path = Path(__file__).resolve().parents[1]
_SKILL_YAML = """\
name: local/tdd-cross-process
description: "TDD cross-process persistence test skill"
tools:
- name: builtin/read_file
"""
_SKILL_NAME = "local/tdd-cross-process"
_SUBPROCESS_TIMEOUT = 30
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
def _fail(message: str) -> NoReturn:
"""Print an error message to stderr and exit with code 1."""
print(message, file=sys.stderr)
sys.exit(1)
def _make_subprocess_env(home_dir: str, db_url: str) -> dict[str, str]:
"""Build environment for subprocess calls."""
env: dict[str, str] = os.environ.copy()
env["CLEVERAGENTS_HOME"] = home_dir
env["CLEVERAGENTS_DATABASE_URL"] = db_url
env["CLEVERAGENTS_AUTO_APPLY_MIGRATIONS"] = "true"
env["CLEVERAGENTS_TESTING_USE_MOCK_AI"] = "true"
env["NO_COLOR"] = "1"
return env
def _run_agents_cli(
args: list[str],
*,
env: dict[str, str],
cwd: str,
) -> subprocess.CompletedProcess[str]:
"""Run an ``agents`` CLI command as a subprocess.
If the subprocess exceeds ``_SUBPROCESS_TIMEOUT`` seconds, a
synthetic ``CompletedProcess`` with return code ``-1`` is returned
instead of raising ``TimeoutExpired``.
"""
cmd: list[str] = [sys.executable, "-m", "cleveragents", *args]
try:
return subprocess.run(
cmd,
capture_output=True,
text=True,
timeout=_SUBPROCESS_TIMEOUT,
cwd=cwd,
env=env,
encoding="utf-8",
)
except subprocess.TimeoutExpired:
return subprocess.CompletedProcess(
args=cmd,
returncode=-1,
stdout="",
stderr=f"subprocess timed out after {_SUBPROCESS_TIMEOUT}s",
)
def _setup_env() -> tuple[str, str, dict[str, str]]:
"""Create a temp directory with a skill config and initialised project.
Runs ``agents init --yes`` via subprocess to set up the database
and project structure, mirroring the real user workflow.
Returns:
Tuple of (tmpdir, config_path, subprocess_env).
"""
tmpdir: str = tempfile.mkdtemp(prefix="tdd_skill_robot_xproc_")
# Write skill YAML config
config_path: str = os.path.join(tmpdir, "test-skill.yaml")
with open(config_path, "w", encoding="utf-8") as fh:
fh.write(_SKILL_YAML)
# Database path
db_path: str = os.path.join(tmpdir, "cleveragents_tdd620.db")
db_url: str = f"sqlite:///{db_path}"
env: dict[str, str] = _make_subprocess_env(tmpdir, db_url)
cwd: str = str(_ROOT)
# Initialise the project via subprocess (creates database + migrations)
init_result: subprocess.CompletedProcess[str] = _run_agents_cli(
["init", "--yes", "--path", tmpdir],
env=env,
cwd=cwd,
)
if init_result.returncode != 0:
shutil.rmtree(tmpdir, ignore_errors=True)
_fail(
f"Project init failed (rc={init_result.returncode}).\n"
f"stdout: {init_result.stdout}\nstderr: {init_result.stderr}"
)
return tmpdir, config_path, env
# ---------------------------------------------------------------------------
# Subcommands
# ---------------------------------------------------------------------------
def cross_process_list() -> None:
"""Add a skill via subprocess, list in another subprocess.
Each CLI command runs as an independent subprocess separate
Python interpreters with separate DI containers. Exits 0 with
sentinel when the skill is found (bug fixed). Exits 1 when not
found (bug present).
"""
tmpdir, config_path, env = _setup_env()
cwd: str = str(_ROOT)
try:
# Subprocess 1: add the skill
add_result: subprocess.CompletedProcess[str] = _run_agents_cli(
["skill", "add", "--config", config_path, "--format", "json"],
env=env,
cwd=cwd,
)
if add_result.returncode != 0:
_fail(
f"skill add failed (rc={add_result.returncode}).\n"
f"stdout: {add_result.stdout}\nstderr: {add_result.stderr}"
)
# Subprocess 2: list skills (cross-process boundary)
list_result: subprocess.CompletedProcess[str] = _run_agents_cli(
["skill", "list", "--format", "json"],
env=env,
cwd=cwd,
)
combined: str = list_result.stdout + list_result.stderr
if list_result.returncode != 0 or _SKILL_NAME not in combined:
_fail(
f"Skill '{_SKILL_NAME}' not found in cross-process list.\n"
f"Add stdout: {add_result.stdout}\n"
f"List rc: {list_result.returncode}\n"
f"List output: {combined}"
)
print("tdd-skill-add-cross-process-list-ok")
finally:
shutil.rmtree(tmpdir, ignore_errors=True)
def cross_process_show() -> None:
"""Add a skill via subprocess, show in another subprocess.
Each CLI command runs as an independent subprocess. Exits 0 with
sentinel when the skill show succeeds (bug fixed). Exits 1 when
not found (bug present).
"""
tmpdir, config_path, env = _setup_env()
cwd: str = str(_ROOT)
try:
# Subprocess 1: add the skill
add_result: subprocess.CompletedProcess[str] = _run_agents_cli(
["skill", "add", "--config", config_path, "--format", "json"],
env=env,
cwd=cwd,
)
if add_result.returncode != 0:
_fail(
f"skill add failed (rc={add_result.returncode}).\n"
f"stdout: {add_result.stdout}\nstderr: {add_result.stderr}"
)
# Subprocess 2: show the skill (cross-process boundary)
show_result: subprocess.CompletedProcess[str] = _run_agents_cli(
["skill", "show", _SKILL_NAME, "--format", "json"],
env=env,
cwd=cwd,
)
combined: str = show_result.stdout + show_result.stderr
if show_result.returncode != 0 or _SKILL_NAME not in combined:
_fail(
f"Skill '{_SKILL_NAME}' not found in cross-process show.\n"
f"Add stdout: {add_result.stdout}\n"
f"Show rc: {show_result.returncode}\n"
f"Show output: {combined}"
)
print("tdd-skill-add-cross-process-show-ok")
finally:
shutil.rmtree(tmpdir, ignore_errors=True)
# ---------------------------------------------------------------------------
# Dispatcher
# ---------------------------------------------------------------------------
_COMMANDS: dict[str, Callable[[], None]] = {
"cross-process-list": cross_process_list,
"cross-process-show": cross_process_show,
}
if __name__ == "__main__":
if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS:
print(
f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>",
file=sys.stderr,
)
sys.exit(1)
cmd: Callable[[], None] = _COMMANDS[sys.argv[1]]
cmd()
+42
View File
@@ -0,0 +1,42 @@
*** Settings ***
Documentation TDD Bug #620 — skill add cross-process persistence
... Integration smoke tests verifying that skills registered via
... ``agents skill add --config <file>`` in one CLI process are
... visible when ``agents skill list`` is run in a separate CLI
... process. Bug #620 reports that the skill is lost across
... process boundaries because the SkillService falls back to
... in-memory storage when the database is not properly initialised.
...
... Each test case runs the ``agents`` binary via ``subprocess.run``
... so every CLI command executes in its own Python process with its
... own DI container and database connection.
...
... Bug: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/620
... TDD: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/1091
Resource ${CURDIR}/common.resource
Suite Setup Setup Test Environment
Suite Teardown Cleanup Test Environment
*** Variables ***
${HELPER} ${CURDIR}/helper_tdd_skill_add_regression.py
*** Test Cases ***
TDD Skill Add Cross Process Persistence
[Documentation] Verify that a skill added via CLI in one process is
... visible via ``skill list`` in a separate process.
[Tags] tdd_bug tdd_bug_620 tdd_expected_fail
${result}= Run Process ${PYTHON} ${HELPER} cross-process-list cwd=${WORKSPACE} timeout=120s on_timeout=kill
Log ${result.stdout}
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} tdd-skill-add-cross-process-list-ok
TDD Skill Add Cross Process Show
[Documentation] Verify that a skill added via CLI in one process can be
... shown via ``skill show`` in a separate process.
[Tags] tdd_bug tdd_bug_620 tdd_expected_fail
${result}= Run Process ${PYTHON} ${HELPER} cross-process-show cwd=${WORKSPACE} timeout=120s on_timeout=kill
Log ${result.stdout}
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} tdd-skill-add-cross-process-show-ok
@@ -259,14 +259,14 @@ class SandboxStrategyRegistry:
# ------------------------------------------------------------------
@staticmethod
def _validate_protocol(cls: type[Any]) -> None:
def _validate_protocol(klass: type[Any]) -> None:
"""Validate that a class satisfies :class:`SandboxStrategyProtocol`.
Uses structural subtyping: checks that all 9 required methods
are present as callable attributes on the class.
Args:
cls: The class to validate.
klass: The class to validate.
Raises:
ProtocolMismatchError: If the class is missing required methods.
@@ -286,12 +286,12 @@ class SandboxStrategyRegistry:
missing = [
method
for method in required_methods
if not callable(getattr(cls, method, None))
if not callable(getattr(klass, method, None))
]
if missing:
msg = (
f"Class '{cls.__name__}' does not satisfy "
f"Class '{klass.__name__}' does not satisfy "
f"SandboxStrategyProtocol. Missing methods: "
f"{', '.join(missing)}"
)