diff --git a/apps/api/src/api/workspace/workspaces.py b/apps/api/src/api/workspace/workspaces.py index 575fbc0..7b263fa 100644 --- a/apps/api/src/api/workspace/workspaces.py +++ b/apps/api/src/api/workspace/workspaces.py @@ -10,6 +10,7 @@ from sqlalchemy.orm import selectinload from src.auth.dependencies import get_current_user_id, get_db_session from src.models import GitRepository +from src.models import Project from src.models import ToolInstance from src.models import Workspace from src.services.shared.workspace_manager import WorkspaceHasInstancesError, WorkspaceManager @@ -129,9 +130,12 @@ async def create_workspace_top_level( if not name: raise HTTPException(status_code=400, detail="Workspace name is required") + project = await session.get(Project, repo.project_id) if repo.project_id else None manager = WorkspaceManager() try: - workspace = await manager.create(repo, user_id, name, branch, session=session) + workspace = await manager.create( + repo, user_id, name, branch, session=session, project=project + ) session.add(workspace) await session.commit() except HTTPException: @@ -232,9 +236,12 @@ async def create_workspace( if not branch: raise HTTPException(status_code=400, detail="Branch is required") + project = await session.get(Project, project_id) manager = WorkspaceManager() try: - workspace = await manager.create(repo, user_id, name, branch, session=session) + workspace = await manager.create( + repo, user_id, name, branch, session=session, project=project + ) session.add(workspace) await session.commit() except HTTPException: diff --git a/apps/api/src/seeds/builtin_tool_types.py b/apps/api/src/seeds/builtin_tool_types.py index 2afe1ef..c93d02f 100644 --- a/apps/api/src/seeds/builtin_tool_types.py +++ b/apps/api/src/seeds/builtin_tool_types.py @@ -102,9 +102,9 @@ services: opencode: image: node:20-slim container_name: {{TOOL_NAME}} - working_dir: /workspace + working_dir: /home/user/{{WORKSPACE_NAME}} volumes: - - {{REPO_PATH}}:/workspace + - {{REPO_PATH}}:/home/user/{{WORKSPACE_NAME}} ports: - "3000:3000" command: > @@ -116,9 +116,8 @@ services: npm bin -g && ls -la $(npm bin -g) || echo 'ERROR: global bin dir not found' && echo 'export PATH=\"$(npm bin -g):\\$PATH\"' >> /root/.bashrc && - echo 'cd /workspace' >> /root/.bashrc && + echo 'cd /home/user/{{WORKSPACE_NAME}}' >> /root/.bashrc && echo 'OpenCode installation complete' && - cd /workspace && exec tail -f /dev/null" stdin_open: true tty: true diff --git a/apps/api/src/services/build/manifest_compiler.py b/apps/api/src/services/build/manifest_compiler.py index c1b3546..0020bff 100644 --- a/apps/api/src/services/build/manifest_compiler.py +++ b/apps/api/src/services/build/manifest_compiler.py @@ -236,18 +236,16 @@ def compile_dockerfile(manifest: dict) -> str: lines.append(f"RUN chown -R {user['name']}:{user['name']} {dir_str}") lines.append("") - # Create the /workspace compatibility symlink only when the workspace name - # is known at image-build time. Otherwise the entrypoint creates it at - # runtime from the WORKSPACE_NAME environment variable. + # Ensure the project directory exists so the WORKDIR below succeeds. The + # compatibility /workspace symlink is no longer created for new images. workspace_target = f"{home_dir}/{workspace_name}" - if "{{WORKSPACE_NAME}}" not in workspace_name: + workspace_is_placeholder = "{{WORKSPACE_NAME}}" in workspace_name + if not workspace_is_placeholder: lines.append(f"RUN mkdir -p {workspace_target}") if user: lines.append( - f"RUN ln -sfn {workspace_target} /workspace && chown -R {user['name']}:{user['name']} {home_dir}" + f"RUN chown -R {user['name']}:{user['name']} {home_dir}" ) - else: - lines.append(f"RUN ln -sfn {workspace_target} /workspace") lines.append("") # Entrypoint for startup scripts @@ -260,18 +258,21 @@ def compile_dockerfile(manifest: dict) -> str: lines.append("") # Do not switch to the runtime user in the Dockerfile. The entrypoint - # starts as root so it can create the /workspace compatibility symlink - # (which lives under /) and fix mount ownership, then it drops privileges + # starts as root so it can fix mount ownership, then it drops privileges # to the container user before exec-ing the real command. - # Set WORKDIR to the configured home directory unless runtime.working_dir - # explicitly overrides it. + # Set WORKDIR to the project directory unless runtime.working_dir + # explicitly overrides it. When the workspace name is a runtime + # placeholder, the Dockerfile cannot know the literal directory, so fall + # back to the home directory; compose supplies the exact working_dir. runtime = manifest.get("runtime", {}) working_dir = runtime.get("working_dir") if working_dir: lines.append(f"WORKDIR {expand_container_path(working_dir, home_dir)}") - else: + elif workspace_is_placeholder: lines.append(f"WORKDIR {home_dir}") + else: + lines.append(f"WORKDIR {workspace_target}") lines.append("") # Entrypoint and CMD @@ -290,8 +291,8 @@ def compile_entrypoint(manifest: dict) -> str: Injects a permission-fixer preamble that runs as root (or via sudo) before any user-defined startup script. It chowns the home directory and a safe - subset of mount parents to the container user, creates the /workspace - compatibility symlink, and avoids recursive chown of large repo subtrees. + subset of mount parents to the container user, ensures the project + directory exists, and avoids recursive chown of large repo subtrees. Args: manifest: Fully resolved manifest JSON. @@ -346,7 +347,7 @@ def compile_entrypoint(manifest: dict) -> str: lines.append('mkdir -p "$HOME_DIR"') lines.append('fix_owner "$HOME_DIR"') lines.append("") - lines.append("# Ensure workspace target exists and is owned by the container user") + lines.append("# Ensure project directory exists and is owned by the container user") lines.append('mkdir -p "$WORKSPACE_TARGET"') lines.append('fix_owner "$WORKSPACE_TARGET"') lines.append("") @@ -355,16 +356,6 @@ def compile_entrypoint(manifest: dict) -> str: lines.append(' rm -rf "${HOME_DIR}/{{WORKSPACE_NAME}}"') lines.append('fi') lines.append("") - lines.append("# Create /workspace compatibility symlink") - lines.append("# / is owned by root, so we need root or passwordless sudo.") - lines.append('if [ "$(id -u)" = "0" ]; then') - lines.append(' ln -sfn "$WORKSPACE_TARGET" /workspace') - lines.append('elif [ -n "$SUDO" ]; then') - lines.append(' sudo ln -sfn "$WORKSPACE_TARGET" /workspace') - lines.append('else') - lines.append(' ln -sfn "$WORKSPACE_TARGET" /workspace 2>/dev/null || true') - lines.append('fi') - lines.append("") lines.append("# Fix ownership of declared mount targets (top-level only)") for mount in manifest.get("mounts", []): target = mount.get("target") @@ -442,11 +433,13 @@ def compile_compose(manifest: dict, variables: dict[str, Any]) -> str: service["working_dir"] = expand_container_path( runtime["working_dir"], home_dir ) + else: + service["working_dir"] = f"{home_dir}/{workspace_name}" # The entrypoint starts as root (Dockerfile does not set USER) so it can - # create the /workspace compatibility symlink and fix mount ownership. It - # drops privileges to the container user internally before exec-ing the - # real command, so do not set compose-level user override here. + # fix mount ownership. It drops privileges to the container user internally + # before exec-ing the real command, so do not set compose-level user + # override here. if user: service["user"] = "0:0" @@ -460,8 +453,8 @@ def compile_compose(manifest: dict, variables: dict[str, Any]) -> str: if env: service["environment"] = dict(env) - # Expose the workspace/repo name so the entrypoint can finalize the - # /workspace compatibility symlink at container startup. + # Expose the project name so the entrypoint can create the project + # directory at container startup. if "environment" not in service: service["environment"] = {} service["environment"]["WORKSPACE_NAME"] = workspace_name diff --git a/apps/api/src/services/git/clone.py b/apps/api/src/services/git/clone.py index 362d149..ef440b2 100644 --- a/apps/api/src/services/git/clone.py +++ b/apps/api/src/services/git/clone.py @@ -2,17 +2,27 @@ import logging import os +import re import subprocess from pathlib import Path logger = logging.getLogger(__name__) +def _slugify_directory_name(name: str) -> str: + """Return a filesystem-safe, lowercase directory slug.""" + slug = name.lower().strip() + slug = re.sub(r"[^a-z0-9_-]+", "-", slug) + slug = re.sub(r"-+", "-", slug).strip("-") + return slug or "project" + + def clone_repository( remote_url: str, ssh_key_path: str | None, instance_dir: str, branch: str = "main", + project_name: str | None = None, ) -> str: """Clone a git repository into the instance directory. @@ -21,11 +31,14 @@ def clone_repository( ssh_key_path: Path to SSH private key for authentication (optional) instance_dir: Path to instance directory branch: Branch to clone (default: main) + project_name: Optional project name used as the clone directory name + instead of the generic ``repo-clone``. Returns: Path to the cloned repository """ - clone_path = Path(instance_dir) / "repo-clone" + clone_name = _slugify_directory_name(project_name) if project_name else "repo-clone" + clone_path = Path(instance_dir) / clone_name clone_path.mkdir(parents=True, exist_ok=True) env = os.environ.copy() @@ -84,13 +97,15 @@ def check_dirty_state(clone_path: str) -> tuple[bool, list[str]]: return is_dirty, changed_files -def remove_clone_directory(instance_dir: str) -> None: +def remove_clone_directory(instance_dir: str, project_name: str | None = None) -> None: """Remove the cloned repository from the instance directory. Args: instance_dir: Path to instance directory + project_name: Optional project name used as the clone directory name. """ - clone_path = Path(instance_dir) / "repo-clone" + clone_name = _slugify_directory_name(project_name) if project_name else "repo-clone" + clone_path = Path(instance_dir) / clone_name if clone_path.exists(): import shutil shutil.rmtree(clone_path) diff --git a/apps/api/src/services/shared/workspace_manager.py b/apps/api/src/services/shared/workspace_manager.py index 1d41ace..abfce23 100644 --- a/apps/api/src/services/shared/workspace_manager.py +++ b/apps/api/src/services/shared/workspace_manager.py @@ -5,6 +5,7 @@ from __future__ import annotations import contextlib import logging import os +import re import shutil import stat import uuid @@ -17,12 +18,12 @@ from sqlalchemy import select from src.models import Workspace from src.services.git.git_service import GitService from src.services.shared.ssh_keys import _get_fernet -from src.utils.git_url_parser import extract_base_repo_url if TYPE_CHECKING: from sqlalchemy.ext.asyncio import AsyncSession from src.models import GitRepository + from src.models import Project from src.models import ToolInstance logger = logging.getLogger(__name__) @@ -49,30 +50,36 @@ class WorkspaceManager: BASE_PATH = "/data/working-copies" @staticmethod - def _repo_directory_name(repo: "GitRepository") -> str: - """Return the directory name git would create for a standard clone. + def _directory_slug(name: str) -> str: + """Return a filesystem-safe, lowercase directory slug.""" + slug = name.lower().strip() + slug = re.sub(r"[^a-z0-9_-]+", "-", slug) + slug = re.sub(r"-+", "-", slug).strip("-") + return slug or "project" - Prefers the name parsed from the remote URL and falls back to the - user-provided repository name when no remote URL is available. + @staticmethod + def _repo_directory_name(project_name: str) -> str: + """Return the project-name directory for a workspace clone. + + New workspaces are cloned under ``/data/working-copies/{workspace_id}/{project_name}/`` + so that the in-container mount target and terminal cwd can match + ``/home/user/{project_name}``. """ - if repo.remote_url: - base_url = extract_base_repo_url(repo.remote_url) or repo.remote_url - name = base_url.rstrip("/").split("/")[-1] - if name.endswith(".git"): - name = name[:-4] - if name: - return name - return repo.name + return WorkspaceManager._directory_slug(project_name) - def _workspace_path(self, workspace_id: uuid.UUID, repo: "GitRepository") -> str: + def _workspace_path( + self, + workspace_id: uuid.UUID, + project_name: str, + ) -> str: """Return the filesystem path for a workspace. - Layout: /data/working-copies/{workspace_id}/{repo_name}/ + Layout: /data/working-copies/{workspace_id}/{project_name}/ The workspace_id prevents collisions between workspaces, and the - repo_name matches the directory git clone naturally creates. + project_name becomes the in-container directory name. """ return os.path.join( - self.BASE_PATH, str(workspace_id), self._repo_directory_name(repo) + self.BASE_PATH, str(workspace_id), self._repo_directory_name(project_name) ) async def create( @@ -82,6 +89,7 @@ class WorkspaceManager: name: str, branch: str = "main", session: AsyncSession | None = None, + project: "Project | None" = None, ) -> Workspace: """Clone repo to workspace path and create DB record. @@ -91,6 +99,8 @@ class WorkspaceManager: name: The workspace name (unique per repo). branch: The branch to clone (default: "main"). session: Database session for loading SSH keys. + project: Optional project for naming the clone directory. Fetched + from the repo relationship if not provided. Returns: The created Workspace record. @@ -101,7 +111,13 @@ class WorkspaceManager: if not repo.remote_url: raise ValueError("Repository has no remote URL") - repo_dir_name = self._repo_directory_name(repo) + if project is None and repo.project_id is not None and session is not None: + from src.models import Project + + project = await session.get(Project, repo.project_id) + project_name = project.name if project else repo.name + + repo_dir_name = self._repo_directory_name(project_name) workspace_id = uuid.uuid4() parent_path = os.path.join(self.BASE_PATH, str(workspace_id)) expected_path = os.path.join(parent_path, repo_dir_name) @@ -346,11 +362,18 @@ class WorkspaceManager: return workspace from src.models import GitRepository + from src.models import Project repo = await session.get(GitRepository, instance.repository_id) if repo is None: raise RuntimeError(f"Repository {instance.repository_id} not found") + project = ( + await session.get(Project, instance.project_id) + if instance.project_id + else None + ) + base_name = ( f"{instance.name}-migrated" if instance.clone_mode == "clone" @@ -364,7 +387,7 @@ class WorkspaceManager: if instance.clone_mode == "clone": workspace = await self._migrate_clone_into_workspace( - instance, repo, session, name + instance, repo, session, name, project=project ) else: workspace = await self.create( @@ -373,6 +396,7 @@ class WorkspaceManager: name=name, branch=instance.branch or "main", session=session, + project=project, ) instance.workspace_id = workspace.id @@ -409,6 +433,7 @@ class WorkspaceManager: repo: "GitRepository", session: AsyncSession, name: str, + project: "Project | None" = None, ) -> Workspace: """Move an existing clone-mode repo into a new workspace path.""" import shutil @@ -423,7 +448,8 @@ class WorkspaceManager: workspace_id = uuid.uuid4() parent_path = os.path.join(self.BASE_PATH, str(workspace_id)) - repo_dir_name = self._repo_directory_name(repo) + project_name = project.name if project else repo.name + repo_dir_name = self._repo_directory_name(project_name) target_path = os.path.join(parent_path, repo_dir_name) os.makedirs(parent_path, exist_ok=True) @@ -433,8 +459,8 @@ class WorkspaceManager: if os.path.exists(target_path): shutil.rmtree(target_path, ignore_errors=True) - # Move the existing clone into the repo-named subdirectory so the - # workspace path matches the natural git clone layout. + # Move the existing clone into the project-named subdirectory so the + # workspace path matches the new in-container layout. shutil.move(clone_path, target_path) self._make_world_writable(target_path) diff --git a/apps/api/src/services/tool/instance_service.py b/apps/api/src/services/tool/instance_service.py index 78766f1..4849185 100644 --- a/apps/api/src/services/tool/instance_service.py +++ b/apps/api/src/services/tool/instance_service.py @@ -5,6 +5,7 @@ import contextlib import glob as glob_module import logging import os +import re import shutil import subprocess import uuid @@ -81,29 +82,27 @@ logger = logging.getLogger(__name__) _event_bus = InstanceEventBus() +def _slugify_directory_name(name: str) -> str: + """Return a filesystem-safe, lowercase directory slug.""" + slug = name.lower().strip() + slug = re.sub(r"[^a-z0-9_-]+", "-", slug) + slug = re.sub(r"-+", "-", slug).strip("-") + return slug or "project" + + def _get_repository_mount_name( + project: Project, repo: GitRepository, workspace: "Workspace | None" = None, ) -> str: """Return the directory name the repository should appear under in the container. - When a workspace exists, the on-disk layout is - ``/data/working-copies/{workspace_id}/{repo_name}/``, so the repo-named - directory is already available as the basename of ``workspace.path``. For - legacy repo-only instances we fall back to parsing the remote URL like a - standard ``git clone`` would, then to the user-provided repository name. + The in-container layout is now ``/home/user/{project_name}/`` so that the + terminal starts directly in the project directory. The workspace path is + ignored for naming purposes; it only provides the source directory to + mount. """ - if workspace is not None and workspace.path: - return os.path.basename(os.path.normpath(workspace.path)) - - if repo.remote_url: - base_url = extract_base_repo_url(repo.remote_url) or repo.remote_url - name = base_url.rstrip("/").split("/")[-1] - if name.endswith(".git"): - name = name[:-4] - if name: - return name - return repo.name + return _slugify_directory_name(project.name) def _chown_path(path: str, uid: int, gid: int) -> None: @@ -271,10 +270,11 @@ def clone_git_repo( remote_url: str, branch: str | None, clone_parent: str, + project_name: str | None = None, ) -> str: """Clone or pull a git repository. - Returns the path to the cloned repo (repo-clone directory). + Returns the path to the cloned repo directory. """ import hashlib @@ -286,7 +286,13 @@ def clone_git_repo( ).hexdigest()[:12] repo_name = remote_url.split("/")[-1].replace(".git", "") or "repo" clone_dir = os.path.join(clone_parent, "git-mounts", f"{repo_name}-{url_hash}") - repo_path = os.path.join(clone_dir, "repo-clone") + repo_path = clone_repository( + remote_url, + None, # No SSH key for now - can be added later + clone_dir, + branch or "main", + project_name=project_name, + ) if not os.path.exists(repo_path): try: @@ -296,6 +302,7 @@ def clone_git_repo( None, # No SSH key for now - can be added later clone_dir, branch or "main", + project_name=project_name, ) logger.debug("Cloned git mount repository %s to %s", remote_url, repo_path) except Exception as exc: @@ -443,7 +450,8 @@ async def resolve_single_git_mount( logger.warning("Git mount skipped: no instance_dir provided for cloning") return [] - # Clone or pull the repository + # Clone or pull the repository. Git mounts are auxiliary, so they keep + # using the repository URL basename rather than the project name. try: repo_path = await asyncio.to_thread( clone_git_repo, remote_url, branch, instance_dir @@ -1020,9 +1028,10 @@ async def prepare_manifest_instance( workspace: Workspace | None = None if instance.workspace_id: workspace = await session.get(Workspace, instance.workspace_id) + project = await session.get(Project, instance.project_id) if instance.project_id else None repo_name = ( - _get_repository_mount_name(repo, workspace) - if repo + _get_repository_mount_name(project, repo, workspace) + if project and repo else os.path.basename(os.path.normpath(repo_path)) ) variables = { @@ -1186,7 +1195,7 @@ async def create_tool_instance( ) home_dir = tool_type.home_directory or "/home/user" - mount_name = _get_repository_mount_name(repo, workspace) + mount_name = _get_repository_mount_name(project, repo, workspace) workspace_target = f"{home_dir}/{mount_name}" compose_content = f"""version: "3.8"\nservices: @@ -1223,7 +1232,7 @@ async def create_tool_instance( # Manifest templates use WORKSPACE_PATH; REPO_PATH is retained as a # deprecated alias for backward compatibility with older templates. - mount_name = _get_repository_mount_name(repo, workspace) + mount_name = _get_repository_mount_name(project, repo, workspace) variables = { "IMAGE_TAG": image_tag, "INSTANCE_NAME": instance_name.lower(), @@ -1252,7 +1261,7 @@ async def create_tool_instance( "TOOL_PORT": tool_port, "USER_ID": str(user_id), "PROJECT_ID": str(project_id), - "WORKSPACE_NAME": _get_repository_mount_name(repo, workspace), + "WORKSPACE_NAME": _get_repository_mount_name(project, repo, workspace), "HOME_DIRECTORY": tool_type.home_directory or "/home/user", } compose_content = render_compose_template(tool_type.compose_template, variables) @@ -1574,6 +1583,10 @@ async def start_tool_instance( else: # ── LEGACY FLOW ────────────────────────────────────────── if instance.clone_mode == "clone" and not instance.workspace_id: + clone_project = await session.get(Project, instance.project_id) if instance.project_id else None + clone_name = _slugify_directory_name(clone_project.name) if clone_project else "repo-clone" + repo_path = os.path.join(instance_dir, clone_name) + repo = await session.get(GitRepository, instance.repository_id) if repo and repo.ssh_key_id: ssh_key = await session.get(SSHKey, repo.ssh_key_id) @@ -1599,6 +1612,13 @@ async def start_tool_instance( exc, ) + # Ensure the legacy compose mounts the project-named clone directory. + extra_volumes.append({ + "source": repo_path, + "target": f"{home_dir}/{clone_name}", + "type": "bind", + }) + if port_override or start_command or working_directory or extra_volumes: modify_compose_file( instance.compose_path, @@ -2104,7 +2124,11 @@ async def delete_tool_instance( os.path.dirname(instance.compose_path) if instance.compose_path else None ) if instance_dir: - clone_path = os.path.join(instance_dir, "repo-clone") + clone_project = await session.get(Project, instance.project_id) if instance.project_id else None + clone_name = _slugify_directory_name(clone_project.name) if clone_project else "repo-clone" + clone_path = os.path.join(instance_dir, clone_name) + if not os.path.exists(clone_path): + clone_path = os.path.join(instance_dir, "repo-clone") if os.path.exists(clone_path): is_dirty, changed_files = check_dirty_state(clone_path) if is_dirty: diff --git a/apps/api/tests/integration/test_workspaces_api.py b/apps/api/tests/integration/test_workspaces_api.py index 07e8eec..fc18f0c 100644 --- a/apps/api/tests/integration/test_workspaces_api.py +++ b/apps/api/tests/integration/test_workspaces_api.py @@ -5,6 +5,7 @@ import uuid from unittest.mock import MagicMock, patch import pytest +from fastapi import HTTPException from fastapi.testclient import TestClient from sqlalchemy.ext.asyncio import AsyncSession @@ -165,7 +166,9 @@ class TestCreateWorkspace: asyncio.run(_commit()) with patch.object( - WorkspaceManager, "create", side_effect=Exception("duplicate") + WorkspaceManager, + "create", + side_effect=HTTPException(status_code=409, detail="duplicate"), ): response = authenticated_client.post( f"/projects/{test_repo.project_id}/repositories/{test_repo.id}/workspaces", diff --git a/apps/api/tests/unit/test_instance_service.py b/apps/api/tests/unit/test_instance_service.py index d6ec2f5..79b1c8c 100644 --- a/apps/api/tests/unit/test_instance_service.py +++ b/apps/api/tests/unit/test_instance_service.py @@ -53,37 +53,17 @@ class TestModifyComposeFile: class TestGetRepositoryMountName: """Tests for _get_repository_mount_name.""" - def test_prefers_remote_url_name_over_user_provided_name(self): + def test_uses_project_name(self): + project = MagicMock() + project.name = "My Project" repo = MagicMock() - repo.name = "src" - repo.remote_url = "git@git.example.com:acme/headquarter.git" - assert _get_repository_mount_name(repo) == "headquarter" + assert _get_repository_mount_name(project, repo) == "my-project" - def test_parses_browser_url_to_repo_name(self): + def test_slugifies_project_name(self): + project = MagicMock() + project.name = "Project v2.0!" repo = MagicMock() - repo.name = "src" - repo.remote_url = "https://github.com/acme/headquarter/tree/main" - assert _get_repository_mount_name(repo) == "headquarter" - - def test_uses_workspace_path_basename_when_workspace_provided(self): - repo = MagicMock() - repo.name = "src" - repo.remote_url = "git@git.example.com:acme/headquarter.git" - workspace = MagicMock() - workspace.path = "/data/working-copies/uuid/headquarter" - assert _get_repository_mount_name(repo, workspace) == "headquarter" - - def test_falls_back_to_repo_name_when_remote_url_missing(self): - repo = MagicMock() - repo.name = "my-cool-repo" - repo.remote_url = None - assert _get_repository_mount_name(repo) == "my-cool-repo" - - def test_falls_back_to_repo_name_for_unparseable_url(self): - repo = MagicMock() - repo.name = "my-cool-repo" - repo.remote_url = "" - assert _get_repository_mount_name(repo) == "my-cool-repo" + assert _get_repository_mount_name(project, repo) == "project-v2-0" @pytest.mark.unit @@ -277,6 +257,9 @@ async def test_prepare_manifest_instance_uses_workspace_path_basename(): session = AsyncMock() + project = MagicMock() + project.name = "acme" + async def session_get(model, obj_id): if model.__name__ == "ToolType": return tool_type @@ -286,6 +269,8 @@ async def test_prepare_manifest_instance_uses_workspace_path_basename(): return repo if model.__name__ == "Workspace": return workspace + if model.__name__ == "Project": + return project return None session.get.side_effect = session_get @@ -324,5 +309,5 @@ async def test_prepare_manifest_instance_uses_workspace_path_basename(): finally: instance_service.subprocess.run = original_run - assert "WORKSPACE_NAME: headquarter" in compose_content - assert "/data/working-copies/workspace-uuid/headquarter:/home/user/headquarter" in compose_content + assert "WORKSPACE_NAME: acme" in compose_content + assert "/data/working-copies/workspace-uuid/headquarter:/home/user/acme" in compose_content diff --git a/apps/api/tests/unit/test_manifest_compiler.py b/apps/api/tests/unit/test_manifest_compiler.py index 4ffc21a..7aedaff 100644 --- a/apps/api/tests/unit/test_manifest_compiler.py +++ b/apps/api/tests/unit/test_manifest_compiler.py @@ -78,20 +78,7 @@ class TestGetManifestHomeDir: class TestCompileDockerfileHomeDirectory: """Tests that compile_dockerfile honors manifest.home_directory.""" - def test_env_home_and_workdir_use_home_directory(self) -> None: - manifest = { - "base_image": "ubuntu:24.04", - "interface_type": "terminal", - "home_directory": "/home/custom", - "user": {"name": "dev", "uid": 1000, "gid": 1000}, - } - dockerfile = compile_dockerfile(manifest) - - assert "ENV HOME=/home/custom" in dockerfile - assert "WORKDIR /home/custom" in dockerfile - - def test_workspace_symlink_created(self) -> None: - """When workspace name is known at build time, create /workspace symlink.""" + def test_env_home_and_workdir_use_project_directory(self) -> None: manifest = { "base_image": "ubuntu:24.04", "interface_type": "terminal", @@ -101,12 +88,26 @@ class TestCompileDockerfileHomeDirectory: } dockerfile = compile_dockerfile(manifest) - assert "mkdir -p /home/custom" in dockerfile - assert "ln -sfn /home/custom/my-app /workspace" in dockerfile + assert "ENV HOME=/home/custom" in dockerfile + assert "WORKDIR /home/custom/my-app" in dockerfile + + def test_project_directory_created(self) -> None: + """When workspace name is known at build time, create the project directory.""" + manifest = { + "base_image": "ubuntu:24.04", + "interface_type": "terminal", + "home_directory": "/home/custom", + "workspace_name": "my-app", + "user": {"name": "dev", "uid": 1000, "gid": 1000}, + } + dockerfile = compile_dockerfile(manifest) + + assert "mkdir -p /home/custom/my-app" in dockerfile + assert "ln -sfn" not in dockerfile def test_runtime_workspace_not_baked_into_image(self) -> None: """When workspace name is a runtime placeholder, do not create literal - {{WORKSPACE_NAME}} directories or symlinks in the image.""" + {{WORKSPACE_NAME}} directories in the image.""" manifest = { "base_image": "ubuntu:24.04", "interface_type": "terminal", @@ -118,6 +119,8 @@ class TestCompileDockerfileHomeDirectory: assert "{{WORKSPACE_NAME}}" not in dockerfile assert "ln -sfn" not in dockerfile + assert "WORKDIR /home/custom/{{WORKSPACE_NAME}}" not in dockerfile + assert "WORKDIR /home/custom\n" in dockerfile def test_runtime_working_dir_overrides_home_workdir(self) -> None: manifest = { @@ -191,7 +194,7 @@ class TestCompileComposeHomeDirectory: compose = compile_compose(manifest, variables) assert "/host/repos/my-app:/opt/code:ro" in compose - assert "/home/custom/my-app" not in compose + assert "/host/repos/my-app:/home/custom/my-app" not in compose def test_workspace_name_substituted_in_mount_target(self) -> None: manifest = { @@ -276,7 +279,7 @@ def test_compile_dockerfile_starts_as_root_and_drops_privileges() -> None: @pytest.mark.unit def test_compile_compose_runs_as_root() -> None: - """The compose service must start as root so the entrypoint can fix /workspace.""" + """The compose service must start as root so the entrypoint can fix ownership.""" manifest = { "base_image": "ubuntu:24.04", "interface_type": "terminal", @@ -300,7 +303,7 @@ def test_compile_compose_runs_as_root() -> None: class TestCompileEntrypoint: """Tests for the generated permission-fixing entrypoint.""" - def test_entrypoint_creates_home_and_workspace(self) -> None: + def test_entrypoint_creates_home_and_project_directory(self) -> None: manifest = { "base_image": "ubuntu:24.04", "interface_type": "terminal", @@ -311,7 +314,7 @@ class TestCompileEntrypoint: assert 'mkdir -p "$HOME_DIR"' in entrypoint assert 'mkdir -p "$WORKSPACE_TARGET"' in entrypoint - assert 'ln -sfn "$WORKSPACE_TARGET" /workspace' in entrypoint + assert 'ln -sfn' not in entrypoint assert 'WORKSPACE_NAME="${WORKSPACE_NAME:-workspace}"' in entrypoint def test_entrypoint_removes_stale_placeholder_directory(self) -> None: @@ -327,8 +330,8 @@ class TestCompileEntrypoint: assert 'if [ -d "${HOME_DIR}/{{WORKSPACE_NAME}}" ]; then' in entrypoint assert 'rm -rf "${HOME_DIR}/{{WORKSPACE_NAME}}"' in entrypoint - def test_entrypoint_uses_root_then_sudo_for_workspace_symlink(self) -> None: - """/workspace is under /, so root takes precedence; non-root falls back to sudo.""" + def test_entrypoint_does_not_create_workspace_symlink(self) -> None: + """The /workspace compatibility symlink is no longer created.""" manifest = { "base_image": "ubuntu:24.04", "interface_type": "terminal", @@ -337,12 +340,7 @@ class TestCompileEntrypoint: } entrypoint = compile_entrypoint(manifest) - root_idx = entrypoint.find('if [ "$(id -u)" = "0" ]; then') - sudo_idx = entrypoint.find('elif [ -n "$SUDO" ]; then') - assert root_idx != -1 - assert sudo_idx != -1 - assert root_idx < sudo_idx - assert 'sudo ln -sfn "$WORKSPACE_TARGET" /workspace' in entrypoint + assert "/workspace" not in entrypoint def test_entrypoint_fixes_mount_owners(self) -> None: manifest = {