fix: check root before sudo when creating /workspace symlink

The previous ordering checked SUDO before checking if the process was
already running as root. When Docker starts the container with a
non-root user, SUDO may be empty, but the real fix is that the
entrypoint should try root first (e.g. when the image is started as
root) and only then fall back to sudo.

- Reorder symlink creation logic: root first, then sudo, then best-effort
- Update unit test to assert root is checked before sudo

Quality gates:
- pytest tests/unit: 208 passed
- ruff: clean on changed files
- mypy: clean on changed files

apps/api/tests/unit/.pi-map.index.md

@@ -2,7 +2,7 @@
 dir: apps/api/tests/unit
 
 ## role
-Unit test suite for the API application covering configuration, database migrations, Docker operations, Git integration, file services, monitoring, notifications, and manifest compilation.
+Comprehensive unit test suite for the API application covering database migrations, configuration, git operations, Docker services, file handling, monitoring, and notification systems.
 ## parent
 index: apps/api/tests/.pi-map.index.md
 map: apps/api/tests/.pi-map.md

apps/api/tests/unit/.pi-map.md

@@ -4,7 +4,7 @@ dir: apps/api/tests/unit
 index: apps/api/tests/unit/.pi-map.index.md
 
 ## role
-Unit test suite for the API application covering configuration, database migrations, Docker operations, Git integration, file services, monitoring, notifications, and manifest compilation.
+Comprehensive unit test suite for the API application covering database migrations, configuration, git operations, Docker services, file handling, monitoring, and notification systems.
 ## files
 - __init__.py | Empty file with no functionality
 - test_alembic_migrations.py | Unit tests that verify Alembic database migrations are importable, have correct revision identifiers, and declare expected dependencies without requiring a live database. | exp: func:test_home_directory_migration_imports_and_rewrites() → None, call:Path, call:migration_path.exists, call:importlib.util.spec_from_file_location, call:importlib.util.module_from_spec, call:spec.loader.exec_module, call:callable, func:test_merge_migration_resolves_heads() → None, call:Path, call:migration_path.exists, call:importlib.util.spec_from_file_location, call:importlib.util.module_from_spec, call:spec.loader.exec_module, call:callable | dep: importlib.util, pathlib, pytest, importlib
@@ -21,7 +21,7 @@ Unit test suite for the API application covering configuration, database migrati
 - test_home_path_expansion.py | Unit tests for home directory path expansion (~ and $HOME) in container paths and manifest home directory resolution. | exp: class:TestExpandContainerPath, method:test_tilde_slash_expands(self) → None, call:expand_container_path, method:test_tilde_alone_expands(self) → None, call:expand_container_path, method:test_dollar_home_slash_expands(self) → None, call:expand_container_path, method:test_dollar_home_alone_expands(self) → None, call:expand_container_path, method:test_absolute_path_unchanged(self) → None, call:expand_container_path, method:test_relative_path_unchanged(self) → None, call:expand_container_path, method:test_tilde_in_middle_unchanged(self) → None, call:expand_container_path, method:test_dollar_home_in_middle_unchanged(self) → None, call:expand_container_path, method:test_root_home(self) → None, call:expand_container_path, class:TestGetManifestHomeDir, method:test_with_user_block(self) → None, call:get_manifest_home_dir, method:test_without_user_block(self) → None, call:get_manifest_home_dir, method:test_with_empty_user_name(self) → None, call:get_manifest_home_dir, method:test_with_none_user_name(self) → None, call:get_manifest_home_dir | dep: pytest, src.services.config.config_profile_resolver, src.services.build.manifest_compiler
 - test_instance_service.py | Unit tests for home directory expansion in docker-compose file modification via instance_service | exp: class:TestModifyComposeFile, method:test_extra_volumes_expand_home_dir(self, tmp_path), call:compose_path.write_text, call:modify_compose_file, call:str, call:compose_path.read_text, method:test_working_directory_expands_home_dir(self, tmp_path), call:compose_path.write_text, call:modify_compose_file, call:str, call:compose_path.read_text | dep: pytest, src.services.tool.instance_service
 - test_lifecycle_hooks.py | Unit tests for lifecycle hook helper functions that derive notification titles and determine whether events should trigger notifications. | exp: class:TestDeriveTitle, method:test_known_event_types(self) → None, call:_derive_title, method:test_unknown_event_type(self) → None, call:_derive_title, class:TestShouldNotify, method:test_error_events_are_notified(self) → None, call:_should_notify, method:test_health_changed_running_is_notified(self) → None, call:_should_notify, method:test_created_started_stopped_restarted_deleted_filtered(self) → None, call:_should_notify, method:test_health_changed_non_running_filtered(self) → None, call:_should_notify | dep: pytest, src.services.instance.lifecycle_hooks
-- test_manifest_compiler.py | Unit tests for a manifest compiler that generates Dockerfiles, docker-compose files, and entrypoint scripts based on manifest configurations. | exp: class:TestGetManifestHomeDir, method:test_home_directory_in_manifest_wins(self) → None, call:get_manifest_home_dir, method:test_user_name_derives_home(self) → None, call:get_manifest_home_dir, method:test_root_fallback(self) → None, call:get_manifest_home_dir, method:test_empty_home_directory_falls_back(self) → None, call:get_manifest_home_dir, class:TestCompileDockerfileHomeDirectory, method:test_env_home_and_workdir_use_home_directory(self) → None, call:compile_dockerfile, method:test_workspace_symlink_created(self) → None, call:compile_dockerfile, method:test_runtime_working_dir_overrides_home_workdir(self) → None, call:compile_dockerfile, method:test_working_dir_expands_tilde(self) → None, call:compile_dockerfile, class:TestCompileComposeHomeDirectory, method:test_default_repo_mount_synthesized(self) → None, call:compile_compose, method:test_explicit_repo_mount_preserved(self) → None, call:compile_compose, method:test_workspace_name_substituted_in_mount_target(self) → None, call:compile_compose, method:test_working_dir_expands_home(self) → None, call:compile_compose, class:TestCompileEntrypoint, method:test_entrypoint_creates_home_and_workspace(self) → None, call:compile_entrypoint, method:test_entrypoint_uses_sudo_for_workspace_symlink(self) → None, call:compile_entrypoint, method:test_entrypoint_fixes_mount_owners(self) → None, call:compile_entrypoint, func:test_compile_dockerfile_creates_config_dirs_for_user() → None, call:compile_dockerfile, func:test_compile_dockerfile_no_user_does_not_create_home() → None, call:compile_dockerfile, func:test_compile_dockerfile_uses_user_npm_prefix() → None, call:compile_dockerfile | dep: pytest, src.services.build.manifest_compiler
+- test_manifest_compiler.py | Unit tests for a manifest compiler that generates Dockerfiles, docker-compose files, and entrypoint scripts from manifest configurations. | exp: class:TestGetManifestHomeDir, method:test_home_directory_in_manifest_wins(self) → None, call:get_manifest_home_dir, method:test_user_name_derives_home(self) → None, call:get_manifest_home_dir, method:test_root_fallback(self) → None, call:get_manifest_home_dir, method:test_empty_home_directory_falls_back(self) → None, call:get_manifest_home_dir, class:TestCompileDockerfileHomeDirectory, method:test_env_home_and_workdir_use_home_directory(self) → None, call:compile_dockerfile, method:test_workspace_symlink_created(self) → None, call:compile_dockerfile, method:test_runtime_working_dir_overrides_home_workdir(self) → None, call:compile_dockerfile, method:test_working_dir_expands_tilde(self) → None, call:compile_dockerfile, class:TestCompileComposeHomeDirectory, method:test_default_repo_mount_synthesized(self) → None, call:compile_compose, method:test_explicit_repo_mount_preserved(self) → None, call:compile_compose, method:test_workspace_name_substituted_in_mount_target(self) → None, call:compile_compose, method:test_working_dir_expands_home(self) → None, call:compile_compose, class:TestCompileEntrypoint, method:test_entrypoint_creates_home_and_workspace(self) → None, call:compile_entrypoint, method:test_entrypoint_uses_root_then_sudo_for_workspace_symlink(self) → None, call:compile_entrypoint, call:entrypoint.find, method:test_entrypoint_fixes_mount_owners(self) → None, call:compile_entrypoint, func:test_compile_dockerfile_creates_config_dirs_for_user() → None, call:compile_dockerfile, func:test_compile_dockerfile_no_user_does_not_create_home() → None, call:compile_dockerfile, func:test_compile_dockerfile_uses_user_npm_prefix() → None, call:compile_dockerfile | dep: pytest, src.services.build.manifest_compiler
 - test_migration_metadata.py | Tests Alembic database migration files for correct table definitions and revision chain metadata | exp: func:test_initial_migration_defines_all_core_tables() → None, call:Path(__file__).resolve, call:spec_from_file_location, call:module_from_spec, call:spec.loader.exec_module, func:test_refresh_tokens_migration_has_expected_revision_chain() → None, call:Path(__file__).resolve, call:spec_from_file_location, call:module_from_spec, call:spec.loader.exec_module | dep: pytest, importlib.util, pathlib, pathlib.Path
 - test_monitoring_models.py | Unit tests verifying creation, persistence, and querying of monitoring models (InstanceEvent and HealthCheck) with database migration compatibility. | exp: func:test_instance_event_creation(db_session) → None, call:User, call:uuid.uuid4, call:db_session.add, call:db_session.commit, call:ToolInstance, call:InstanceEvent, call:db_session.refresh, call:isinstance, func:test_health_check_creation(db_session) → None, call:User, call:uuid.uuid4, call:db_session.add, call:db_session.commit, call:ToolInstance, call:HealthCheck, call:db_session.refresh, call:isinstance, func:test_instance_event_query_by_instance(db_session) → None, call:User, call:uuid.uuid4, call:db_session.add, call:db_session.commit, call:ToolInstance, call:InstanceEvent, call:db_session.execute, call:select(InstanceEvent).where, call:result.scalar_one | dep: uuid, datetime, pytest, sqlalchemy, src.models.system.health_check, src.models.system.instance_event, src.models.tool.tool_instance, src.models.user.user
 - test_notification_service.py | Unit tests for NotificationService covering CRUD operations, filtering, sorting, and ownership isolation. | exp: func:notification_service() → NotificationService, call:NotificationService, func:user_a(db_session: AsyncSession) → User, call:User, call:uuid.uuid4, call:db_session.add, call:db_session.commit, func:user_b(db_session: AsyncSession) → User, call:User, call:uuid.uuid4, call:db_session.add, call:db_session.commit, func:test_create_notification(db_session: AsyncSession, notification_service: NotificationService, user_a: User) → None, call:notification_service.create_notification, call:uuid.uuid4, func:test_list_notifications_orders_by_created_at_desc(db_session: AsyncSession, notification_service: NotificationService, user_a: User) → None, call:notification_service.create_notification, call:datetime.now, call:timedelta, call:db_session.commit, call:db_session.refresh, call:notification_service.list_notifications, func:test_list_notifications_excludes_dismissed(db_session: AsyncSession, notification_service: NotificationService, user_a: User) → None, call:notification_service.create_notification, call:notification_service.dismiss, call:notification_service.list_notifications, func:test_list_notifications_unread_only(db_session: AsyncSession, notification_service: NotificationService, user_a: User) → None, call:notification_service.create_notification, call:notification_service.mark_read, call:notification_service.list_notifications, func:test_get_unread_count(db_session: AsyncSession, notification_service: NotificationService, user_a: User) → None, call:range, call:notification_service.create_notification, call:notification_service.mark_read, call:notification_service.get_unread_count, func:test_mark_read_sets_read_at(db_session: AsyncSession, notification_service: NotificationService, user_a: User) → None, call:notification_service.create_notification, call:notification_service.mark_read, func:test_mark_all_read_affects_all_unread(db_session: AsyncSession, notification_service: NotificationService, user_a: User) → None, call:range, call:notification_service.create_notification, call:notification_service.mark_all_read, call:notification_service.get_unread_count, func:test_dismiss_sets_dismissed_at(db_session: AsyncSession, notification_service: NotificationService, user_a: User) → None, call:notification_service.create_notification, call:notification_service.dismiss, call:db_session.execute, call:select(Notification).where, call:result.scalar_one, func:test_mark_read_wrong_owner_raises(db_session: AsyncSession, notification_service: NotificationService, user_a: User, user_b: User) → None, call:notification_service.create_notification, call:pytest.raises, call:notification_service.mark_read, func:test_dismiss_wrong_owner_raises(db_session: AsyncSession, notification_service: NotificationService, user_a: User, user_b: User) → None, call:notification_service.create_notification, call:pytest.raises, call:notification_service.dismiss, func:test_list_notifications_mute_categories(db_session: AsyncSession, notification_service: NotificationService, user_a: User) → None, call:notification_service.create_notification, call:notification_service.list_notifications, func:test_get_unread_count_excludes_dismissed(db_session: AsyncSession, notification_service: NotificationService, user_a: User) → None, call:notification_service.create_notification, call:notification_service.dismiss, call:notification_service.get_unread_count, func:test_dismiss_all_affects_all_non_dismissed(db_session: AsyncSession, notification_service: NotificationService, user_a: User) → None, call:range, call:notification_service.create_notification, call:notification_service.dismiss_all, call:notification_service.list_notifications, func:test_dismiss_all_affects_only_caller(db_session: AsyncSession, notification_service: NotificationService, user_a: User, user_b: User) → None, call:range, call:notification_service.create_notification, call:notification_service.dismiss_all, call:notification_service.list_notifications, func:test_mark_all_read_affects_only_caller(db_session: AsyncSession, notification_service: NotificationService, user_a: User, user_b: User) → None, call:range, call:notification_service.create_notification, call:notification_service.mark_all_read, call:notification_service.get_unread_count | dep: uuid, datetime, pytest, sqlalchemy, sqlalchemy.ext.asyncio, src.models.system.notification, src.models.user.user, src.services.shared.notification_service, NotificationService, Notification, User, AsyncSession
@@ -30,7 +30,7 @@ Unit test suite for the API application covering configuration, database migrati
 - test_readiness_probe.py | Unit tests for a Docker container readiness probe service that executes commands via docker exec with retry logic. | exp: class:TestExecuteProbe, class:TestIntegrationScenarios | dep: unittest.mock, src.services.shared.readiness_probe, subprocess
 - test_ssh_keys.py | Unit tests for SSH key preparation functionality including file creation, permissions, ownership, and error handling | exp: class:TestPrepareSshKeyFiles | dep: os, pathlib, unittest.mock, pytest, src.services.shared.ssh_keys
 ## arch
-Standard pytest-based unit testing with heavy use of mocking (subprocess, filesystem, database) to test components in isolation without external dependencies; tests are organized by service/feature with descriptive naming.
+Standard Python unittest/pytest structure with test files mirroring production modules, heavy use of mocking for external dependencies (Docker, subprocess, database), and isolated tests for individual service classes without integration requirements.
 ## tags
 test, url, call:notification, git, home, call:, merge, call:db
 ## symbols

apps/api/tests/unit/test_manifest_compiler.py

@@ -24,9 +24,16 @@ def test_compile_dockerfile_creates_config_dirs_for_user() -> None:
     assert "groupadd -g 1000 dev" in dockerfile
     assert "useradd -u 1000 -g 1000 -m -s /bin/bash dev" in dockerfile
     assert "mkdir -p /home/dev && chown -R dev:dev /home/dev" in dockerfile
-    assert "mkdir -p /home/dev/.config && chown -R dev:dev /home/dev/.config" in dockerfile
-    assert "mkdir -p /home/dev/.local/share && chown -R dev:dev /home/dev/.local/share" in dockerfile
-    assert "mkdir -p /home/dev/.cache && chown -R dev:dev /home/dev/.cache" in dockerfile
+    assert (
+        "mkdir -p /home/dev/.config && chown -R dev:dev /home/dev/.config" in dockerfile
+    )
+    assert (
+        "mkdir -p /home/dev/.local/share && chown -R dev:dev /home/dev/.local/share"
+        in dockerfile
+    )
+    assert (
+        "mkdir -p /home/dev/.cache && chown -R dev:dev /home/dev/.cache" in dockerfile
+    )
 
 
 @pytest.mark.unit
@@ -175,9 +182,7 @@ class TestCompileComposeHomeDirectory:
             "interface_type": "terminal",
             "home_directory": "/home/custom",
             "user": {"name": "dev", "uid": 1000, "gid": 1000},
-            "mounts": [
-                {"source_type": "repo", "target": "~/{{WORKSPACE_NAME}}"}
-            ],
+            "mounts": [{"source_type": "repo", "target": "~/{{WORKSPACE_NAME}}"}],
         }
         variables = {
             "IMAGE_TAG": "test:latest",
@@ -250,8 +255,8 @@ class TestCompileEntrypoint:
         assert 'ln -sfn "$WORKSPACE_TARGET" /workspace' in entrypoint
         assert 'WORKSPACE_NAME="${WORKSPACE_NAME:-workspace}"' in entrypoint
 
-    def test_entrypoint_uses_sudo_for_workspace_symlink(self) -> None:
-        """/workspace is under /, so the non-root entrypoint needs sudo to recreate it."""
+    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."""
         manifest = {
             "base_image": "ubuntu:24.04",
             "interface_type": "terminal",
@@ -260,9 +265,12 @@ class TestCompileEntrypoint:
         }
         entrypoint = compile_entrypoint(manifest)
 
-        assert 'if [ -n "$SUDO" ]; then' in entrypoint
+        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 'elif [ "$(id -u)" = "0" ]; then' in entrypoint
 
     def test_entrypoint_fixes_mount_owners(self) -> None:
         manifest = {
@@ -279,4 +287,3 @@ class TestCompileEntrypoint:
 
         assert 'fix_owner "/home/custom/.config"' in entrypoint
         assert 'fix_owner "/opt/readonly"' not in entrypoint
-