Harden M8 preflight foundation

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"),
+    ]

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."""

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()
+            )

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"
 

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")