diff --git a/AGENTS.md b/AGENTS.md index 669f190..9ee351e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,6 +21,7 @@ cf # Interactive tool picker - `cli/` - Routes all subcommands (list, create, run, test, providers, registry, collections, deps, install, etc.) - `tool.py` - Tool/step dataclasses, including delegated `ToolStep` context and `McpStep` +- `preflight.py` - Shared contract, dependency, secret-pattern, and registry-similarity analysis - `runner.py` - Step execution, variable substitution, nested authorization and delegation - `providers.py` - AI providers, auto-discovery, fallback chains, and tool/MCP allowlists - `skills.py` - Per-provider Agent Skills loading and validation @@ -40,6 +41,7 @@ cf # Interactive tool picker - Framework: `pytest` (see `pyproject.toml`) - Test files: `tests/test_*.py` - Use `mock` provider or `--dry-run` to avoid network calls +- Use `cmdforge inspect ` for deterministic local preflight checks - Integration tests require `@pytest.mark.integration` decorator ## Commit Guidelines diff --git a/CLAUDE.md b/CLAUDE.md index 5f75026..6bdb3fb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,6 +77,12 @@ Tools are YAML configs with: - `arguments`: Custom flags with defaults (e.g., `--max` → `{max}`) - `steps`: Ordered list of `prompt`, `code`, or `tool` steps - `output`: Template for final output (e.g., `"{response}"`) +- `input_schema`, `output_schema`: Optional tool-level JSON Schema contracts; + schemas are validated when tools are loaded or created + +Run `cmdforge inspect [--registry]` for the shared local preflight +report. `cmdforge registry publish --dry-run` runs local checks first +and, when authenticated, the registry's publish-time checks without publishing. ### Step Types diff --git a/README.md b/README.md index 4845e70..e1b289e 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,8 @@ cmdforge delete mytool # Delete tool cmdforge run mytool # Run a tool cmdforge test mytool # Test with mock provider cmdforge check mytool # Check dependencies (meta-tools) +cmdforge inspect mytool # Validate contracts and run local preflight checks +cmdforge inspect mytool --registry # Also find similar registry tools cmdforge refresh # Update executable wrappers cmdforge docs mytool # View/create tool documentation @@ -142,6 +144,7 @@ cmdforge docs mytool # View/create tool documentation cmdforge registry search "keyword" # Search for tools cmdforge registry install owner/tool # Install a tool cmdforge registry publish mytool # Publish your tool +cmdforge registry publish mytool --dry-run # Preflight without publishing cmdforge registry status # Check moderation status cmdforge registry my-tools # List your published tools diff --git a/src/cmdforge/cli/__init__.py b/src/cmdforge/cli/__init__.py index 7745afb..d6e2d45 100644 --- a/src/cmdforge/cli/__init__.py +++ b/src/cmdforge/cli/__init__.py @@ -93,6 +93,10 @@ def main(): # 'inspect' command p_inspect = subparsers.add_parser("inspect", help="Run preflight analysis on a tool") p_inspect.add_argument("name", help="Tool name") + p_inspect.add_argument( + "--registry", action="store_true", + help="Include similar tools from the configured registry", + ) p_inspect.set_defaults(func=cmd_inspect) # 'check' command @@ -530,7 +534,11 @@ def cmd_inspect(args): print(f"Error: Tool '{args.name}' not found.", file=sys.stderr) return 1 - report = analyze_tool(tool) + registry_client = None + if getattr(args, "registry", False): + from ..registry_client import get_client + registry_client = get_client() + report = analyze_tool(tool, registry_client=registry_client) if report.errors: print(f"Errors ({len(report.errors)}):") diff --git a/src/cmdforge/cli/registry_commands.py b/src/cmdforge/cli/registry_commands.py index 8a95b1d..9265c52 100644 --- a/src/cmdforge/cli/registry_commands.py +++ b/src/cmdforge/cli/registry_commands.py @@ -351,6 +351,48 @@ def _cmd_registry_update(args): return 0 +def _print_preflight_sections(report: dict, prefix: str = "") -> None: + """Render the common portions of a structured preflight report.""" + labels = ( + ("errors", "ERROR"), + ("warnings", "WARN"), + ("suggestions", "HINT"), + ) + for key, marker in labels: + values = report.get(key) or [] + if values: + print(f"{prefix}{key.title()} ({len(values)}):") + for value in values: + print(f" {marker}: {value}") + + +def _print_registry_suggestions(suggestions: dict) -> None: + """Render registry-specific category, similarity, and scrutiny evidence.""" + category = suggestions.get("category") or {} + if category.get("suggested"): + print( + f"Suggested category: {category['suggested']} " + f"({category.get('confidence', 0):.0%} confidence)" + ) + + similar = suggestions.get("similar_tools") or [] + if similar: + print(f"Similar registry tools ({len(similar)}):") + for item in similar: + print( + f" - {item.get('name', 'unknown')} " + f"({item.get('similarity', 0):.0%} similar)" + ) + + scrutiny = suggestions.get("scrutiny") or {} + findings = scrutiny.get("findings") or [] + if findings: + print(f"Scrutiny findings ({len(findings)}):") + for finding in findings: + marker = str(finding.get("result", "info")).upper() + print(f" {marker}: {finding.get('message', '')}") + + def _cmd_registry_publish(args): """Publish a tool to the registry.""" from ..registry_client import RegistryError, get_client @@ -404,6 +446,9 @@ def _cmd_registry_publish(args): # Validate try: data = yaml.safe_load(config_yaml) + if not isinstance(data, dict): + print("Error: config.yaml must contain a YAML mapping", file=sys.stderr) + return 1 name = data.get("name", "") version = data.get("version", "") if not name or not version: @@ -424,36 +469,41 @@ def _cmd_registry_publish(args): print(f" README: {len(readme)} bytes") print() + from ..preflight import analyze_tool + from ..tool import Tool + try: + local_report = analyze_tool(Tool.from_dict(data)) + except (KeyError, TypeError, ValueError) as exc: + print(f"Local preflight error: {exc}", file=sys.stderr) + return 1 + + _print_preflight_sections(local_report.to_dict(), prefix="Local ") + if not local_report.ok: + return 1 + # Attempt registry preflight if token is configured config = load_config() - if config.registry.token: - try: - client = get_client() - result = client.publish_tool( - config_yaml, readme=readme, defaults=defaults, - dry_run=True, - ) - except Exception as e: - print(f"Registry preflight error: {e}", file=sys.stderr) - return 1 - errors = result.get("errors") or [] - warnings = result.get("warnings") or [] - suggestions = result.get("suggestions") or [] - if errors: - print(f"Preflight errors ({len(errors)}):") - for err in errors: - print(f" ERROR: {err}") - if warnings: - print(f"Preflight warnings ({len(warnings)}):") - for warn in warnings: - print(f" WARN: {warn}") - if suggestions: - print(f"Suggestions ({len(suggestions)}):") - for sug in suggestions: - print(f" HINT: {sug}") - if not errors and not warnings and not suggestions: - print("Registry preflight passed.") - return 0 if not errors else 1 + if not config.registry.token: + print("No registry token configured — local preflight only.") + return 0 + + try: + client = get_client() + result = client.publish_tool( + config_yaml, readme=readme, defaults=defaults, + dry_run=True, + ) + except RegistryError as exc: + print(f"Registry preflight failed: {exc}", file=sys.stderr) + return 1 + + remote_report = result.get("preflight") or {} + _print_preflight_sections(remote_report, prefix="Registry ") + _print_registry_suggestions(result.get("suggestions") or {}) + if not remote_report.get("errors"): + print("Registry preflight passed.") + return 0 + return 1 # Check for token config = load_config() diff --git a/src/cmdforge/gui/pages/tool_builder_page.py b/src/cmdforge/gui/pages/tool_builder_page.py index 2cbd083..e95c544 100644 --- a/src/cmdforge/gui/pages/tool_builder_page.py +++ b/src/cmdforge/gui/pages/tool_builder_page.py @@ -19,6 +19,12 @@ from ...tool import ( from ..widgets.icons import get_prompt_icon, get_code_icon, get_tool_icon +def _switch_to_existing_tool(main_window, name: str) -> None: + """Remove the creation page before opening an existing tool editor.""" + main_window.close_tool_builder() + main_window.open_tool_builder(name) + + class ToolBuilderPage(QWidget): """Tool builder/editor page.""" @@ -1093,7 +1099,7 @@ class ToolBuilderPage(QWidget): clicked = msg.clickedButton() if clicked is btn_open: - self.main_window.open_tool_builder(name) + _switch_to_existing_tool(self.main_window, name) return elif clicked is btn_copy: suffix = 2 @@ -1122,9 +1128,23 @@ class ToolBuilderPage(QWidget): steps=self._tool.steps if self._tool else [], output=output, dependencies=self._tool.dependencies if self._tool else [], - system_dependencies=self._tool.system_dependencies if self._tool else [] + system_dependencies=self._tool.system_dependencies if self._tool else [], + visibility=self._tool.visibility if self._tool else "public", + input_schema=self._tool.input_schema if self._tool else None, + output_schema=self._tool.output_schema if self._tool else None, ) + from ...preflight import analyze_tool + preflight = analyze_tool(tool) + if preflight.errors: + QMessageBox.warning( + self, + "Preflight Failed", + "\n".join(preflight.errors), + ) + return + advisory_count = len(preflight.warnings) + len(preflight.suggestions) + # Preserve source if editing if self._tool and self._tool.source: tool.source = self._tool.source @@ -1159,7 +1179,10 @@ class ToolBuilderPage(QWidget): if settings_path.exists(): settings_path.unlink() - self.main_window.show_status(f"Saved tool '{name}'") + status = f"Saved tool '{name}'" + if advisory_count: + status += f" with {advisory_count} preflight suggestion(s)" + self.main_window.show_status(status) self.main_window.close_tool_builder() except Exception as e: QMessageBox.critical(self, "Error", f"Failed to save tool:\n{e}") diff --git a/src/cmdforge/preflight.py b/src/cmdforge/preflight.py index 2fd7cf1..730a77d 100644 --- a/src/cmdforge/preflight.py +++ b/src/cmdforge/preflight.py @@ -4,11 +4,10 @@ Produces a PreflightReport used by the GUI, CLI, and registry workflow. All checks are deterministic and evidence-based. """ -import sys from dataclasses import dataclass, field from typing import Any, Dict, List, Optional -from .tool import Tool +from .tool import Tool, validate_json_schema @dataclass @@ -36,8 +35,23 @@ class PreflightReport: compatibility=self.compatibility + other.compatibility, ) + def to_dict(self) -> Dict[str, Any]: + return { + "errors": list(self.errors), + "warnings": list(self.warnings), + "suggestions": list(self.suggestions), + "similar_tools": list(self.similar_tools), + "generated_tests": list(self.generated_tests), + "compatibility": list(self.compatibility), + } -def analyze_tool(tool: Tool, registry_client=None) -> PreflightReport: + +def analyze_tool( + tool: Tool, + registry_client=None, + *, + check_local_dependencies: bool = True, +) -> PreflightReport: """Run all preflight checks on a tool. Args: @@ -51,7 +65,8 @@ def analyze_tool(tool: Tool, registry_client=None) -> PreflightReport: _check_config_integrity(tool, report) _check_contracts(tool, report) _check_secrets(tool, report) - _check_dependencies(tool, report) + if check_local_dependencies: + _check_dependencies(tool, report) if registry_client: _check_similar_tools(tool, report, registry_client) return report @@ -62,7 +77,7 @@ def _check_config_integrity(tool: Tool, report: PreflightReport): report.errors.append("Tool name is required") if tool.version and not _is_semver(tool.version): report.errors.append(f"Version '{tool.version}' is not valid semver") - if not tool.steps and not tool.arguments: + if not tool.steps and not tool.arguments and tool.output == "{input}": report.warnings.append( "Tool has no steps and no arguments — output template is passthrough" ) @@ -80,15 +95,10 @@ def _check_contracts(tool: Tool, report: PreflightReport): def _validate_schema(schema: Any, label: str, report: PreflightReport): - if not isinstance(schema, dict): - report.errors.append(f"{label} must be a JSON Schema object") - return - schema_type = schema.get("type") - if schema_type and not isinstance(schema_type, (str, list)): - report.errors.append(f"{label} 'type' must be a string or array") - required = schema.get("required") - if required is not None and not isinstance(required, list): - report.errors.append(f"{label} 'required' must be an array") + try: + validate_json_schema(schema, label) + except ValueError as exc: + report.errors.append(str(exc)) def _check_secrets(tool: Tool, report: PreflightReport): @@ -113,6 +123,8 @@ def _check_dependencies(tool: Tool, report: PreflightReport): def _check_similar_tools(tool: Tool, report: PreflightReport, client): + from .registry_client import RegistryError + try: results = client.search_tools(tool.name, per_page=5) items = results.data if hasattr(results, "data") else results @@ -124,7 +136,7 @@ def _check_similar_tools(tool: Tool, report: PreflightReport, client): }) if not items: report.suggestions.append("No similar tools found on the registry") - except Exception: + except RegistryError: report.warnings.append("Could not search registry for similar tools") diff --git a/src/cmdforge/registry/app.py b/src/cmdforge/registry/app.py index 590cd9b..5e2508d 100644 --- a/src/cmdforge/registry/app.py +++ b/src/cmdforge/registry/app.py @@ -2436,6 +2436,10 @@ def create_app() -> Flask: data = yaml.safe_load(config_text) or {} except yaml.YAMLError: return error_response("VALIDATION_ERROR", "Invalid YAML in config") + if not isinstance(data, dict): + return error_response( + "VALIDATION_ERROR", "Tool config must contain a YAML mapping" + ) name = (data.get("name") or "").strip() version = (data.get("version") or "").strip() @@ -2485,6 +2489,26 @@ def create_app() -> Flask: if len(str(tag)) > MAX_TAG_LEN: return error_response("VALIDATION_ERROR", "Tag exceeds 32 characters") + # Run the same deterministic contract/configuration checks used by the + # CLI and builder. Registry hosts intentionally skip local-install + # dependency checks because publisher dependencies are resolved by name. + try: + from ..preflight import analyze_tool as run_preflight + from ..tool import Tool + + preflight_report = run_preflight( + Tool.from_dict(data), check_local_dependencies=False + ) + except (KeyError, TypeError, ValueError) as exc: + return error_response("INVALID_CONFIG", str(exc), 400) + if not preflight_report.ok: + return error_response( + "PREFLIGHT_FAILED", + preflight_report.errors[0], + 400, + details={"preflight": preflight_report.to_dict()}, + ) + # Determine owner - admins can publish as "official" or other owners owner = g.current_publisher["slug"] requested_owner = payload.get("owner", "").strip() @@ -2625,6 +2649,7 @@ def create_app() -> Flask: "version": version, "status": "validated", "suggestions": suggestions, + "preflight": preflight_report.to_dict(), } }) @@ -2730,6 +2755,7 @@ def create_app() -> Flask: "forked_from": forked_from, "forked_version": forked_version, "suggestions": suggestions, + "preflight": preflight_report.to_dict(), } }) response.status_code = 201 diff --git a/src/cmdforge/tool.py b/src/cmdforge/tool.py index bf0fcca..7b55620 100644 --- a/src/cmdforge/tool.py +++ b/src/cmdforge/tool.py @@ -44,6 +44,22 @@ def _validate_optional_patterns(patterns: Optional[List[str]], field_name: str) raise ValueError(f"{field_name} must be a list of non-empty strings or null") +def validate_json_schema(schema: Optional[dict], field_name: str) -> None: + """Validate an optional tool contract as a real JSON Schema.""" + if schema is None: + return + if not isinstance(schema, dict): + raise ValueError(f"{field_name} must be a JSON Schema object") + + from jsonschema.exceptions import SchemaError + from jsonschema.validators import validator_for + + try: + validator_for(schema).check_schema(schema) + except SchemaError as exc: + raise ValueError(f"Invalid {field_name}: {exc.message}") from exc + + @dataclass class SystemDependency: """A system-level package dependency (apt, brew, pacman, etc.).""" @@ -435,8 +451,15 @@ class Tool: output_schema: Optional[dict] = None # JSON Schema for tool output contract path: Optional[Path] = None # Path to config.yaml (set by load_tool) + def __post_init__(self) -> None: + validate_json_schema(self.input_schema, "input_schema") + validate_json_schema(self.output_schema, "output_schema") + @classmethod def from_dict(cls, data: dict) -> "Tool": + if not isinstance(data, dict): + raise ValueError("Tool configuration must be a YAML mapping") + arguments = [] for arg in data.get("arguments", []): arguments.append(ToolArgument.from_dict(arg)) @@ -455,7 +478,15 @@ class Tool: # Parse source attribution if present source = None if "source" in data: - source = ToolSource.from_dict(data["source"]) + source_data = data["source"] + if isinstance(source_data, dict): + source = ToolSource.from_dict(source_data) + elif isinstance(source_data, str) and source_data.strip(): + source = ToolSource( + type="imported", original_tool=source_data.strip() + ) + elif source_data is not None: + raise ValueError("source must be an attribution object or string") # Normalize dependencies - can be strings or dicts with name/version raw_deps = data.get("dependencies", []) diff --git a/tests/test_cli.py b/tests/test_cli.py index 5bcf16a..6a6c5c2 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2,7 +2,8 @@ import tempfile from pathlib import Path -from unittest.mock import patch, MagicMock +from types import SimpleNamespace +from unittest.mock import call, patch, MagicMock from io import StringIO import pytest @@ -441,3 +442,132 @@ class TestDocsCommand: assert result == 1 # Returns 1 when no README captured = capsys.readouterr() assert 'No documentation' in captured.out or '--edit' in captured.out + + +class TestRegistryPublishDryRun: + @pytest.fixture + def tool_dir(self, tmp_path): + directory = tmp_path / "dry-run-tool" + directory.mkdir() + (directory / "config.yaml").write_text( + "name: dry-run-tool\n" + "version: 1.0.0\n" + "description: Test dry-run behavior\n" + "output: constant\n" + ) + return directory + + @staticmethod + def args(tool_dir): + return SimpleNamespace( + path=str(tool_dir), dry_run=True, force=False, owner="" + ) + + def test_without_token_stops_after_local_preflight( + self, tool_dir, monkeypatch, capsys + ): + from cmdforge.cli.registry_commands import _cmd_registry_publish + + get_client = MagicMock() + monkeypatch.setattr( + "cmdforge.cli.registry_commands.load_config", + lambda: SimpleNamespace(registry=SimpleNamespace(token="")), + ) + monkeypatch.setattr("cmdforge.registry_client.get_client", get_client) + + assert _cmd_registry_publish(self.args(tool_dir)) == 0 + get_client.assert_not_called() + assert "local preflight only" in capsys.readouterr().out + + def test_renders_structured_registry_evidence( + self, tool_dir, monkeypatch, capsys + ): + from cmdforge.cli.registry_commands import _cmd_registry_publish + + client = MagicMock() + client.publish_tool.return_value = { + "preflight": { + "errors": [], + "warnings": ["remote warning"], + "suggestions": ["remote hint"], + }, + "suggestions": { + "similar_tools": [{ + "name": "official/similar", + "similarity": 0.8, + }], + "scrutiny": { + "findings": [{"result": "warning", "message": "review me"}] + }, + }, + } + monkeypatch.setattr( + "cmdforge.cli.registry_commands.load_config", + lambda: SimpleNamespace(registry=SimpleNamespace(token="token")), + ) + monkeypatch.setattr("cmdforge.registry_client.get_client", lambda: client) + + assert _cmd_registry_publish(self.args(tool_dir)) == 0 + output = capsys.readouterr().out + assert "remote warning" in output + assert "official/similar" in output + assert "review me" in output + client.publish_tool.assert_called_once() + + def test_registry_rejection_returns_failure( + self, tool_dir, monkeypatch, capsys + ): + from cmdforge.cli.registry_commands import _cmd_registry_publish + from cmdforge.registry_client import RegistryError + + client = MagicMock() + client.publish_tool.side_effect = RegistryError( + "SCRUTINY_FAILED", "Tool rejected" + ) + monkeypatch.setattr( + "cmdforge.cli.registry_commands.load_config", + lambda: SimpleNamespace(registry=SimpleNamespace(token="token")), + ) + monkeypatch.setattr("cmdforge.registry_client.get_client", lambda: client) + + assert _cmd_registry_publish(self.args(tool_dir)) == 1 + assert "Tool rejected" in capsys.readouterr().err + + def test_non_mapping_config_returns_failure(self, tool_dir, capsys): + from cmdforge.cli.registry_commands import _cmd_registry_publish + + (tool_dir / "config.yaml").write_text("- not\n- a\n- tool\n") + + assert _cmd_registry_publish(self.args(tool_dir)) == 1 + assert "YAML mapping" in capsys.readouterr().err + + +def test_inspect_with_registry_uses_similarity_results(monkeypatch, capsys): + from cmdforge.cli import cmd_inspect + from cmdforge.registry_client import PaginatedResponse + + tool = Tool(name="summary", output="constant") + client = MagicMock() + client.search_tools.return_value = PaginatedResponse(data=[{ + "owner": "official", + "name": "summarize", + "description": "Summarize text", + }]) + monkeypatch.setattr("cmdforge.tool.load_tool", lambda name: tool) + monkeypatch.setattr("cmdforge.registry_client.get_client", lambda: client) + + assert cmd_inspect(SimpleNamespace(name="summary", registry=True)) == 0 + assert "official/summarize" in capsys.readouterr().out + + +def test_switch_to_existing_tool_closes_creation_page_first(): + pytest.importorskip("PySide6") + from cmdforge.gui.pages.tool_builder_page import _switch_to_existing_tool + + main_window = MagicMock() + _switch_to_existing_tool(main_window, "existing") + + assert main_window.method_calls == [ + call.close_tool_builder(), + call.open_tool_builder("existing"), + ] diff --git a/tests/test_collection_api.py b/tests/test_collection_api.py index 0555e3b..2247136 100644 --- a/tests/test_collection_api.py +++ b/tests/test_collection_api.py @@ -37,7 +37,10 @@ def app(): app = create_app() app.config["TESTING"] = True - yield app + # Keep the environment override active for the entire test. Registry + # request handlers open fresh connections rather than reusing the one + # created during app initialization. + yield app # Cleanup Path(db_path).unlink(missing_ok=True) @@ -155,6 +158,74 @@ class TestToolApprovedEndpoint: assert data['data']['has_approved_public_version'] is False +@flask_required +class TestPublishPreflightEndpoint: + def test_dry_run_includes_shared_preflight(self, client, auth_headers): + response = client.post( + "/api/v1/tools", + headers=auth_headers, + json={ + "dry_run": True, + "config": ( + "name: preflight-tool\n" + "version: 1.0.0\n" + "description: Constant output\n" + "output: constant\n" + "input_schema:\n type: string\n" + "output_schema:\n type: string\n" + ), + }, + ) + + assert response.status_code == 200 + report = response.get_json()["data"]["preflight"] + assert report["errors"] == [] + + def test_invalid_contract_is_rejected(self, client, auth_headers): + response = client.post( + "/api/v1/tools", + headers=auth_headers, + json={ + "dry_run": True, + "config": ( + "name: invalid-contract\n" + "version: 1.0.0\n" + "output_schema:\n type: nonsense\n" + ), + }, + ) + + assert response.status_code == 400 + assert response.get_json()["error"]["code"] == "INVALID_CONFIG" + + def test_non_mapping_config_is_rejected(self, client, auth_headers): + response = client.post( + "/api/v1/tools", + headers=auth_headers, + json={"dry_run": True, "config": "- not\n- a\n- tool\n"}, + ) + + assert response.status_code == 400 + assert response.get_json()["error"]["code"] == "VALIDATION_ERROR" + + def test_legacy_source_string_remains_publishable(self, client, auth_headers): + response = client.post( + "/api/v1/tools", + headers=auth_headers, + json={ + "dry_run": True, + "config": ( + "name: legacy-source\n" + "version: 1.0.0\n" + "source: old/tool\n" + "output: constant\n" + ), + }, + ) + + assert response.status_code == 200 + + @flask_required class TestPostCollectionsEndpoint: """Tests for POST /api/v1/collections endpoint.""" diff --git a/tests/test_preflight.py b/tests/test_preflight.py index 9fd1e2f..a97ad08 100644 --- a/tests/test_preflight.py +++ b/tests/test_preflight.py @@ -1,8 +1,18 @@ """Tests for preflight analysis engine.""" +from types import SimpleNamespace from unittest.mock import patch -from cmdforge.preflight import PreflightReport, analyze_tool, _check_config_integrity +import pytest + +from cmdforge.preflight import ( + PreflightReport, + analyze_tool, + _check_config_integrity, + _check_similar_tools, + _is_semver, +) +from cmdforge.registry_client import PaginatedResponse, RegistryError class TestPreflightReport: @@ -22,6 +32,12 @@ class TestPreflightReport: assert c.warnings == ["w1"] assert c.suggestions == ["s1"] + def test_to_dict_returns_independent_lists(self): + report = PreflightReport(errors=["bad"]) + serialized = report.to_dict() + serialized["errors"].append("another") + assert report.errors == ["bad"] + class TestAnalyzeTool: def test_no_steps_warns(self, tmp_path): @@ -59,6 +75,12 @@ class TestAnalyzeTool: _check_config_integrity(tool, report) assert report.ok + def test_constant_tool_without_steps_is_not_passthrough_warning(self): + from cmdforge.tool import Tool + + report = analyze_tool(Tool(name="constant", output="hello")) + assert not any("passthrough" in warning for warning in report.warnings) + def test_secret_pattern_in_prompt(self, tmp_path): from cmdforge.tool import Tool, PromptStep @@ -74,3 +96,41 @@ class TestAnalyzeTool: from cmdforge.preflight import _check_secrets _check_secrets(tool, report) assert any("secret" in w.lower() for w in report.warnings) + + @pytest.mark.parametrize("version", ["1.2.3garbage", "1.2.3.4", "1.2"]) + def test_semver_rejects_trailing_or_incomplete_values(self, version): + assert _is_semver(version) is False + + def test_registry_similarity_uses_paginated_data(self): + class Client: + def search_tools(self, query, per_page): + assert (query, per_page) == ("summarize", 5) + return PaginatedResponse(data=[{ + "owner": "official", + "name": "summary", + "description": "Summarize text", + "downloads": 10, + }]) + + report = PreflightReport() + _check_similar_tools(SimpleNamespace(name="summarize"), report, Client()) + assert report.similar_tools[0]["name"] == "official/summary" + + def test_registry_error_becomes_warning(self): + class Client: + def search_tools(self, query, per_page): + raise RegistryError("CONNECTION_ERROR", "offline") + + report = PreflightReport() + _check_similar_tools(SimpleNamespace(name="summarize"), report, Client()) + assert report.warnings == ["Could not search registry for similar tools"] + + def test_programming_error_is_not_swallowed(self): + class Client: + def search_tools(self, query, per_page): + raise TypeError("broken adapter") + + with pytest.raises(TypeError, match="broken adapter"): + _check_similar_tools( + SimpleNamespace(name="summarize"), PreflightReport(), Client() + ) diff --git a/tests/test_registry_integration.py b/tests/test_registry_integration.py index 921986a..d3f31f4 100644 --- a/tests/test_registry_integration.py +++ b/tests/test_registry_integration.py @@ -561,6 +561,7 @@ category: text-processing assert data["status"] == "validated" assert data["name"] == tool_name assert data["owner"] == slug + assert data["preflight"]["errors"] == [] def test_publish_and_retrieve(self, session, base_url, auth_headers): """Test publishing a tool and retrieving it.""" @@ -588,8 +589,13 @@ tags: assert data["name"] == tool_name assert data["version"] == "1.0.0" - # Retrieve + # Pending public tools are hidden from anonymous callers, but remain + # visible to their owner while awaiting moderation. resp = session.get(f"{base_url}/tools/{slug}/{tool_name}") + assert resp.status_code == 404 + resp = session.get( + f"{base_url}/tools/{slug}/{tool_name}", headers=headers + ) assert resp.status_code == 200 tool = resp.json()["data"] assert tool["name"] == tool_name @@ -620,11 +626,20 @@ description: First version }) assert resp.status_code == 201 - # Duplicate publish + # An identical retry is idempotent. resp = session.post(f"{base_url}/tools", headers=headers, json={ "config": config, "readme": "" }) + assert resp.status_code == 200 + assert resp.json()["data"]["already_published"] is True + + # Different content at the same version is a real conflict. + changed_config = config.replace("First version", "Changed content") + resp = session.post(f"{base_url}/tools", headers=headers, json={ + "config": changed_config, + "readme": "" + }) assert resp.status_code == 409 assert resp.json()["error"]["code"] == "VERSION_EXISTS" diff --git a/tests/test_tool.py b/tests/test_tool.py index cf6830f..f3594ab 100644 --- a/tests/test_tool.py +++ b/tests/test_tool.py @@ -581,6 +581,46 @@ class TestDefaultCategories: assert "Other" in DEFAULT_CATEGORIES +class TestToolContracts: + def test_config_must_be_mapping(self): + with pytest.raises(ValueError, match="YAML mapping"): + Tool.from_dict(["not", "a", "mapping"]) + + def test_legacy_source_string_is_normalized(self): + tool = Tool.from_dict({"name": "legacy-source", "source": "old/tool"}) + assert tool.source is not None + assert tool.source.type == "imported" + assert tool.source.original_tool == "old/tool" + + def test_contracts_roundtrip(self): + tool = Tool( + name="contract-tool", + input_schema={"type": "string"}, + output_schema={"type": "array", "items": {"type": "integer"}}, + ) + restored = Tool.from_dict(tool.to_dict()) + assert restored.input_schema == {"type": "string"} + assert restored.output_schema["type"] == "array" + + def test_empty_contracts_are_preserved(self): + tool = Tool(name="open-contract", input_schema={}, output_schema={}) + serialized = tool.to_dict() + assert serialized["input_schema"] == {} + assert serialized["output_schema"] == {} + + @pytest.mark.parametrize( + ("field", "schema"), + [ + ("input_schema", "string"), + ("output_schema", {"type": "not-a-json-schema-type"}), + ("output_schema", {"required": "name"}), + ], + ) + def test_invalid_contract_is_rejected(self, field, schema): + with pytest.raises(ValueError, match=field): + Tool(name="bad-contract", **{field: schema}) + + class TestAgentContext: def test_toolstep_profile_roundtrip(self): step = ToolStep(tool="my-tool", output_var="out", profile="architect")