fix: resolve project variable in create_tool_instance
create_tool_instance fetched `project` only inside the auto-display-name
branch, so callers supplying display_name left it unbound and the
manifest/dockerfile/compose branches crashed with UnboundLocalError at
_get_repository_mount_name (in-container layout /home/user/{project_name}).
Resolve project unconditionally after repo validation with a not-found
guard (narrowing Project|None -> Project), and drop the now-redundant
fetch from the auto-name branch. Add a regression test covering the
manifest + display_name path.
Quality gates: ruff, mypy, pytest (74 unit tests passed)
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
"""Unit tests for the tool instance service."""
|
||||
|
||||
import uuid
|
||||
from unittest.mock import MagicMock, AsyncMock
|
||||
|
||||
import pytest
|
||||
@@ -70,9 +71,7 @@ class TestGetRepositoryMountName:
|
||||
class TestStackProfileMountsWithGitMounts:
|
||||
"""Tests for _stack_profile_mounts_with_git_mounts."""
|
||||
|
||||
def test_exact_overlap_merges_profile_files_into_git_source(
|
||||
self, tmp_path
|
||||
) -> None:
|
||||
def test_exact_overlap_merges_profile_files_into_git_source(self, tmp_path) -> None:
|
||||
"""When a profile mount targets the same directory as a git mount,
|
||||
the profile files should be copied into the git-mount source so the
|
||||
container sees both sets of files through one bind mount."""
|
||||
@@ -310,4 +309,100 @@ async def test_prepare_manifest_instance_uses_workspace_path_basename():
|
||||
instance_service.subprocess.run = original_run
|
||||
|
||||
assert "WORKSPACE_NAME: acme" in compose_content
|
||||
assert "/data/working-copies/workspace-uuid/headquarter:/home/user/acme" in compose_content
|
||||
assert (
|
||||
"/data/working-copies/workspace-uuid/headquarter:/home/user/acme"
|
||||
in compose_content
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_create_tool_instance_resolves_project_when_display_name_given(
|
||||
monkeypatch, tmp_path
|
||||
):
|
||||
"""Regression: create_tool_instance must resolve `project` even when a
|
||||
display_name is supplied.
|
||||
|
||||
Previously `project` was only fetched inside the auto-display-name branch,
|
||||
so providing display_name left it unbound and the manifest/dockerfile/compose
|
||||
branches crashed with UnboundLocalError at `_get_repository_mount_name`.
|
||||
The mount name must derive from the project name regardless.
|
||||
"""
|
||||
from src.schemas.tool import CreateInstanceRequest
|
||||
from src.services.tool import instance_service
|
||||
|
||||
project_id = uuid.uuid4()
|
||||
repo_id = uuid.uuid4()
|
||||
user_id = uuid.uuid4()
|
||||
tool_type_id = uuid.uuid4()
|
||||
manifest_id = uuid.uuid4()
|
||||
|
||||
repo = MagicMock()
|
||||
repo.project_id = project_id
|
||||
repo.name = "src"
|
||||
repo.path = "/data/repos/src"
|
||||
|
||||
project = MagicMock()
|
||||
project.name = "My Project" # slugifies to "my-project"
|
||||
|
||||
tool_type = MagicMock()
|
||||
tool_type.id = tool_type_id
|
||||
tool_type.name = "pi-agent"
|
||||
tool_type.definition_type = "manifest"
|
||||
tool_type.manifest_id = manifest_id
|
||||
tool_type.home_directory = None
|
||||
|
||||
manifest_def = MagicMock()
|
||||
manifest_def.manifest = {
|
||||
"base_image": "ubuntu:24.04",
|
||||
"interface_type": "terminal",
|
||||
"user": {"name": "user", "uid": 1001, "gid": 1001},
|
||||
}
|
||||
manifest_def.base_definition_id = None
|
||||
|
||||
async def session_get(model, _obj_id):
|
||||
name = getattr(model, "__name__", "")
|
||||
if name == "GitRepository":
|
||||
return repo
|
||||
if name == "Project":
|
||||
return project
|
||||
if name == "ToolType":
|
||||
return tool_type
|
||||
if name == "ToolDefinitionManifest":
|
||||
return manifest_def
|
||||
return None
|
||||
|
||||
session = AsyncMock()
|
||||
session.get.side_effect = session_get
|
||||
session.add = MagicMock()
|
||||
|
||||
monkeypatch.setattr(
|
||||
instance_service, "validate_config_profile", AsyncMock(return_value=None)
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
instance_service, "ensure_instance_directory", lambda _name: str(tmp_path)
|
||||
)
|
||||
monkeypatch.setattr(instance_service, "find_free_port", lambda: 12345)
|
||||
monkeypatch.setattr(
|
||||
instance_service, "compute_image_tag", lambda *a, **k: "img:tag"
|
||||
)
|
||||
compile_compose = MagicMock(return_value="services:\n app: {}")
|
||||
monkeypatch.setattr(instance_service, "compile_compose", compile_compose)
|
||||
monkeypatch.setattr(instance_service, "write_compose_file", MagicMock())
|
||||
monkeypatch.setattr(instance_service, "publish_lifecycle_event", AsyncMock())
|
||||
|
||||
data = CreateInstanceRequest(
|
||||
tool_type_id=str(tool_type_id), display_name="Custom Name"
|
||||
)
|
||||
|
||||
instance = await instance_service.create_tool_instance(
|
||||
session, user_id, project_id, repo_id, data
|
||||
)
|
||||
|
||||
# Must not raise UnboundLocalError; instance returned.
|
||||
assert instance is not None
|
||||
|
||||
# The mount name must derive from project.name, proving `project` was
|
||||
# resolved on the display_name path (the previous crash site).
|
||||
variables = compile_compose.call_args.args[1]
|
||||
assert variables["REPO_NAME"] == "my-project"
|
||||
assert variables["WORKSPACE_NAME"] == "my-project"
|
||||
|
||||
Reference in New Issue
Block a user