Add admin publish-as-official, hash-based status sync, and GUI fixes
Registry: - Add admin owner override for publishing tools as "official" - Add POST /api/v1/tools/status-by-hash batch endpoint for status lookup scoped to publisher_id (works for tools published under any owner) GUI: - Add "Publish as" dropdown in publish dialog for admin users - Add "installed" tool state (teal with arrow indicator) for registry-installed tools - Fix tool editing for official/* qualified tool names (_get_qualified_name helper) - Fix cancel navigation returning to wrong page (Welcome instead of Tools) - Fix collections tab not refreshing after publish - Refactor StatusSyncWorker to use batch hash lookup (1 request instead of N) with chunking (100 max) and hash collision handling CLI: - Switch registry status sync to hash-based lookup - Add collection dependency checking and unpublished dep detection Publish dialog cleanup: - Move yaml import to module level, remove duplicate _bump_patch_version - Fix owner combo using currentText() for reliable selection Co-Authored-By: Claude Opus 4.5 <[email protected]>
This commit is contained in:
Claude Opus 4.5
+262
-1
@@ -9,7 +9,8 @@ import yaml
|
||||
|
||||
from cmdforge.collection import (
|
||||
Collection, list_collections, get_collection, COLLECTIONS_DIR,
|
||||
classify_tool_reference, resolve_tool_references, ToolResolutionResult
|
||||
classify_tool_reference, resolve_tool_references, ToolResolutionResult,
|
||||
gather_local_unpublished_deps, DepCheckResult
|
||||
)
|
||||
|
||||
|
||||
@@ -296,3 +297,263 @@ class TestToolResolutionResult:
|
||||
assert len(result.registry_refs) == 2
|
||||
assert len(result.local_unpublished) == 1
|
||||
assert len(result.visibility_issues) == 1
|
||||
|
||||
|
||||
class TestDepCheckResult:
|
||||
"""Tests for DepCheckResult dataclass."""
|
||||
|
||||
def test_empty_result(self):
|
||||
result = DepCheckResult(
|
||||
unpublished=[],
|
||||
publish_order=[],
|
||||
cycles=[],
|
||||
skipped=[]
|
||||
)
|
||||
assert len(result.unpublished) == 0
|
||||
assert len(result.cycles) == 0
|
||||
|
||||
def test_with_data(self):
|
||||
result = DepCheckResult(
|
||||
unpublished=["tool-a", "tool-b"],
|
||||
publish_order=["tool-b", "tool-a", "main"],
|
||||
cycles=[["x", "y", "x"]],
|
||||
skipped=["tool-z"]
|
||||
)
|
||||
assert result.unpublished == ["tool-a", "tool-b"]
|
||||
assert result.publish_order == ["tool-b", "tool-a", "main"]
|
||||
assert result.cycles == [["x", "y", "x"]]
|
||||
assert result.skipped == ["tool-z"]
|
||||
|
||||
|
||||
class TestGatherLocalUnpublishedDeps:
|
||||
"""Tests for transitive dependency checking."""
|
||||
|
||||
@pytest.fixture
|
||||
def mock_client(self):
|
||||
"""Create a mock registry client."""
|
||||
client = MagicMock()
|
||||
client.get_me.return_value = {"slug": "testuser"}
|
||||
return client
|
||||
|
||||
@pytest.fixture
|
||||
def temp_tools_dir(self, tmp_path):
|
||||
"""Create a temporary tools directory."""
|
||||
with patch('cmdforge.tool.TOOLS_DIR', tmp_path):
|
||||
yield tmp_path
|
||||
|
||||
def _create_tool(self, tools_dir, name, deps=None, tool_steps=None):
|
||||
"""Helper to create a tool config in the temp dir."""
|
||||
tool_dir = tools_dir / name
|
||||
tool_dir.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
config = {
|
||||
"name": name,
|
||||
"description": f"Test tool {name}",
|
||||
"steps": [],
|
||||
"output": "{input}"
|
||||
}
|
||||
if deps:
|
||||
config["dependencies"] = deps
|
||||
if tool_steps:
|
||||
for step in tool_steps:
|
||||
config["steps"].append({
|
||||
"type": "tool",
|
||||
"tool": step,
|
||||
"output_var": "result"
|
||||
})
|
||||
|
||||
(tool_dir / "config.yaml").write_text(yaml.dump(config))
|
||||
|
||||
def test_no_dependencies_approved(self, mock_client, temp_tools_dir):
|
||||
"""Approved tool with no deps should return empty unpublished list."""
|
||||
mock_client.get_my_tool_status.return_value = {"status": "approved"}
|
||||
|
||||
self._create_tool(temp_tools_dir, "standalone")
|
||||
result = gather_local_unpublished_deps(["standalone"], mock_client, "testuser")
|
||||
|
||||
assert result.unpublished == []
|
||||
assert result.cycles == []
|
||||
|
||||
def test_no_dependencies_not_published(self, mock_client, temp_tools_dir):
|
||||
"""Unpublished tool with no deps should be in unpublished list."""
|
||||
from cmdforge.registry_client import RegistryError
|
||||
mock_client.get_my_tool_status.side_effect = RegistryError(
|
||||
code="TOOL_NOT_FOUND", message="Not found"
|
||||
)
|
||||
|
||||
self._create_tool(temp_tools_dir, "standalone")
|
||||
result = gather_local_unpublished_deps(["standalone"], mock_client, "testuser")
|
||||
|
||||
# The original tool itself is now checked and included if not approved
|
||||
assert "standalone" in result.unpublished
|
||||
assert result.cycles == []
|
||||
|
||||
def test_gathers_explicit_deps(self, mock_client, temp_tools_dir):
|
||||
"""Should find explicit dependencies."""
|
||||
from cmdforge.registry_client import RegistryError
|
||||
|
||||
# dep-tool is unpublished
|
||||
mock_client.get_my_tool_status.side_effect = RegistryError(
|
||||
code="TOOL_NOT_FOUND", message="Not found"
|
||||
)
|
||||
|
||||
self._create_tool(temp_tools_dir, "dep-tool")
|
||||
self._create_tool(temp_tools_dir, "main-tool", deps=["dep-tool"])
|
||||
|
||||
result = gather_local_unpublished_deps(["main-tool"], mock_client, "testuser")
|
||||
|
||||
assert "dep-tool" in result.unpublished
|
||||
# dep-tool should come before main-tool in publish order
|
||||
assert result.publish_order.index("dep-tool") < result.publish_order.index("main-tool")
|
||||
|
||||
def test_gathers_tool_step_deps(self, mock_client, temp_tools_dir):
|
||||
"""Should find implicit dependencies from ToolStep."""
|
||||
from cmdforge.registry_client import RegistryError
|
||||
|
||||
mock_client.get_my_tool_status.side_effect = RegistryError(
|
||||
code="TOOL_NOT_FOUND", message="Not found"
|
||||
)
|
||||
|
||||
self._create_tool(temp_tools_dir, "called-tool")
|
||||
self._create_tool(temp_tools_dir, "caller-tool", tool_steps=["called-tool"])
|
||||
|
||||
result = gather_local_unpublished_deps(["caller-tool"], mock_client, "testuser")
|
||||
|
||||
assert "called-tool" in result.unpublished
|
||||
|
||||
def test_gathers_transitive_deps(self, mock_client, temp_tools_dir):
|
||||
"""Should find dependencies of dependencies."""
|
||||
from cmdforge.registry_client import RegistryError
|
||||
|
||||
mock_client.get_my_tool_status.side_effect = RegistryError(
|
||||
code="TOOL_NOT_FOUND", message="Not found"
|
||||
)
|
||||
|
||||
# a -> b -> c
|
||||
self._create_tool(temp_tools_dir, "tool-c")
|
||||
self._create_tool(temp_tools_dir, "tool-b", deps=["tool-c"])
|
||||
self._create_tool(temp_tools_dir, "tool-a", deps=["tool-b"])
|
||||
|
||||
result = gather_local_unpublished_deps(["tool-a"], mock_client, "testuser")
|
||||
|
||||
assert "tool-b" in result.unpublished
|
||||
assert "tool-c" in result.unpublished
|
||||
# Order: c before b before a
|
||||
assert result.publish_order.index("tool-c") < result.publish_order.index("tool-b")
|
||||
assert result.publish_order.index("tool-b") < result.publish_order.index("tool-a")
|
||||
|
||||
def test_skips_qualified_refs(self, mock_client, temp_tools_dir):
|
||||
"""Should skip owner/name registry references but check main tool."""
|
||||
from cmdforge.registry_client import RegistryError
|
||||
|
||||
mock_client.get_my_tool_status.side_effect = RegistryError(
|
||||
code="TOOL_NOT_FOUND", message="Not found"
|
||||
)
|
||||
|
||||
# Tool depends on a registry tool
|
||||
self._create_tool(temp_tools_dir, "main-tool", deps=["official/summarize"])
|
||||
|
||||
result = gather_local_unpublished_deps(["main-tool"], mock_client, "testuser")
|
||||
|
||||
# Registry refs should NOT be in unpublished
|
||||
assert "official/summarize" not in result.unpublished
|
||||
# But the main tool itself is checked and included since not found
|
||||
assert "main-tool" in result.unpublished
|
||||
|
||||
def test_skips_qualified_refs_with_approved_main(self, mock_client, temp_tools_dir):
|
||||
"""Should skip owner/name refs; approved main tool = empty unpublished."""
|
||||
mock_client.get_my_tool_status.return_value = {"status": "approved"}
|
||||
|
||||
# Tool depends on a registry tool
|
||||
self._create_tool(temp_tools_dir, "main-tool", deps=["official/summarize"])
|
||||
|
||||
result = gather_local_unpublished_deps(["main-tool"], mock_client, "testuser")
|
||||
|
||||
assert "official/summarize" not in result.unpublished
|
||||
assert result.unpublished == []
|
||||
|
||||
def test_detects_cycles(self, mock_client, temp_tools_dir):
|
||||
"""Should detect and report circular dependencies."""
|
||||
from cmdforge.registry_client import RegistryError
|
||||
|
||||
mock_client.get_my_tool_status.side_effect = RegistryError(
|
||||
code="TOOL_NOT_FOUND", message="Not found"
|
||||
)
|
||||
|
||||
# a -> b -> a (cycle)
|
||||
self._create_tool(temp_tools_dir, "tool-a", deps=["tool-b"])
|
||||
self._create_tool(temp_tools_dir, "tool-b", deps=["tool-a"])
|
||||
|
||||
result = gather_local_unpublished_deps(["tool-a"], mock_client, "testuser")
|
||||
|
||||
assert len(result.cycles) > 0
|
||||
# Cyclic nodes should NOT be in unpublished list
|
||||
assert "tool-a" not in result.unpublished
|
||||
assert "tool-b" not in result.unpublished
|
||||
|
||||
def test_distinguishes_404_from_network_error(self, mock_client, temp_tools_dir):
|
||||
"""404 = unpublished, other errors = skipped."""
|
||||
from cmdforge.registry_client import RegistryError
|
||||
|
||||
def status_side_effect(name):
|
||||
if name == "dep-404":
|
||||
raise RegistryError(code="TOOL_NOT_FOUND", message="Not found")
|
||||
elif name == "dep-network":
|
||||
raise RegistryError(code="CONNECTION_ERROR", message="Network error")
|
||||
return {"status": "approved"}
|
||||
|
||||
mock_client.get_my_tool_status.side_effect = status_side_effect
|
||||
|
||||
self._create_tool(temp_tools_dir, "dep-404")
|
||||
self._create_tool(temp_tools_dir, "dep-network")
|
||||
self._create_tool(temp_tools_dir, "main", deps=["dep-404", "dep-network"])
|
||||
|
||||
result = gather_local_unpublished_deps(["main"], mock_client, "testuser")
|
||||
|
||||
assert "dep-404" in result.unpublished
|
||||
assert "dep-network" in result.skipped
|
||||
|
||||
def test_published_dep_not_in_unpublished(self, mock_client, temp_tools_dir):
|
||||
"""Already approved deps should not be in unpublished list."""
|
||||
mock_client.get_my_tool_status.return_value = {"status": "approved"}
|
||||
|
||||
self._create_tool(temp_tools_dir, "published-dep")
|
||||
self._create_tool(temp_tools_dir, "main-tool", deps=["published-dep"])
|
||||
|
||||
result = gather_local_unpublished_deps(["main-tool"], mock_client, "testuser")
|
||||
|
||||
assert "published-dep" not in result.unpublished
|
||||
assert result.unpublished == []
|
||||
|
||||
def test_rejected_dep_in_unpublished(self, mock_client, temp_tools_dir):
|
||||
"""Rejected deps should be in unpublished list (need republishing)."""
|
||||
mock_client.get_my_tool_status.return_value = {"status": "rejected"}
|
||||
|
||||
self._create_tool(temp_tools_dir, "rejected-dep")
|
||||
self._create_tool(temp_tools_dir, "main-tool", deps=["rejected-dep"])
|
||||
|
||||
result = gather_local_unpublished_deps(["main-tool"], mock_client, "testuser")
|
||||
|
||||
assert "rejected-dep" in result.unpublished
|
||||
|
||||
def test_pending_dep_in_unpublished(self, mock_client, temp_tools_dir):
|
||||
"""Pending deps should be in unpublished list (not yet usable)."""
|
||||
mock_client.get_my_tool_status.return_value = {"status": "pending"}
|
||||
|
||||
self._create_tool(temp_tools_dir, "pending-dep")
|
||||
self._create_tool(temp_tools_dir, "main-tool", deps=["pending-dep"])
|
||||
|
||||
result = gather_local_unpublished_deps(["main-tool"], mock_client, "testuser")
|
||||
|
||||
assert "pending-dep" in result.unpublished
|
||||
|
||||
def test_changes_requested_dep_in_unpublished(self, mock_client, temp_tools_dir):
|
||||
"""Deps with changes_requested should be in unpublished list."""
|
||||
mock_client.get_my_tool_status.return_value = {"status": "changes_requested"}
|
||||
|
||||
self._create_tool(temp_tools_dir, "needs-changes-dep")
|
||||
self._create_tool(temp_tools_dir, "main-tool", deps=["needs-changes-dep"])
|
||||
|
||||
result = gather_local_unpublished_deps(["main-tool"], mock_client, "testuser")
|
||||
|
||||
assert "needs-changes-dep" in result.unpublished
|
||||
|
||||
Reference in New Issue
Block a user