diff --git a/adapter/docker/runtime.py b/adapter/docker/runtime.py index 61fcaf6..d24d110 100644 --- a/adapter/docker/runtime.py +++ b/adapter/docker/runtime.py @@ -1,5 +1,6 @@ from datetime import datetime from pathlib import Path +from uuid import UUID from docker import DockerClient from docker.errors import DockerException, NotFound @@ -28,8 +29,11 @@ class DockerSandboxRuntime(SandboxRuntime): created_at: datetime, expires_at: datetime, ) -> SandboxSession: + normalized_chat_id = chat_id + try: - chat_path = self._chat_path(chat_id) + normalized_chat_id = _canonical_chat_id(chat_id) + chat_path = self._chat_path(normalized_chat_id) dependencies_path = self._readonly_host_path( self._config.dependencies_host_path ) @@ -40,19 +44,19 @@ class DockerSandboxRuntime(SandboxRuntime): container = self._client.containers.run( self._config.image, detach=True, - labels=self._labels(session_id, chat_id, expires_at), + labels=self._labels(session_id, normalized_chat_id, expires_at), mounts=self._mounts(chat_path, dependencies_path, lambda_tools_path), ) except (DockerException, OSError, ValueError) as exc: - raise SandboxStartError(chat_id) from exc + raise SandboxStartError(normalized_chat_id) from exc container_id = str(getattr(container, 'id', '')).strip() if not container_id: - raise SandboxStartError(chat_id) + raise SandboxStartError(normalized_chat_id) return SandboxSession( session_id=session_id, - chat_id=chat_id, + chat_id=normalized_chat_id, container_id=container_id, status=SandboxStatus.RUNNING, created_at=created_at, @@ -124,3 +128,7 @@ class DockerSandboxRuntime(SandboxRuntime): def _host_path(self, path_value: str) -> Path: return Path(path_value).expanduser().resolve(strict=False) + + +def _canonical_chat_id(chat_id: str) -> str: + return str(UUID(str(chat_id).strip())) diff --git a/adapter/http/fastapi/schemas.py b/adapter/http/fastapi/schemas.py index 9ca2e5f..a34f702 100644 --- a/adapter/http/fastapi/schemas.py +++ b/adapter/http/fastapi/schemas.py @@ -1,6 +1,7 @@ from datetime import datetime +from uuid import UUID -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, field_validator class HealthResponse(BaseModel): @@ -14,6 +15,11 @@ class CreateSandboxRequest(BaseModel): chat_id: str = Field(min_length=1) + @field_validator('chat_id') + @classmethod + def validate_chat_id(cls, value: str) -> str: + return str(UUID(value)) + class SandboxSessionResponse(BaseModel): session_id: str diff --git a/tasks.md b/tasks.md index 9650de1..cb1e38f 100644 --- a/tasks.md +++ b/tasks.md @@ -163,7 +163,7 @@ ### M13. Повторный boundary review после fix-pass - Субагент: `code-reviewer` -- Статус: pending +- Статус: completed - Зависимости: `M12` - Commit required: no - Scope: проверить, что must-fix и should-fix замечания из `M08` закрыты без нарушения clean architecture @@ -181,3 +181,38 @@ - Ошибки: несовместимый fake Docker client для `DockerSandboxRuntime`, неточная типизация `run_calls` и ASGI message payload, использование `object` вместо типизированных test doubles для `AppRepositories`, `AppUsecases`, `AppContainer` - Решение: сделать test doubles типизированными через совместимые fake classes или локальные protocols; убрать `object` и неиндексируемые `dict[str, object]` там, где mypy не может вывести типы - Критерии приемки: `uv run mypy .` проходит; `make pre-commit` доходит как минимум до pytest stage; production code не меняется или меняется только при явной необходимости для testability + +## Follow-up после M13 review + +### M15. Канонизация и валидация `chat_id` + +- Субагент: `feature-developer` +- Статус: completed +- Зависимости: `M13` +- Commit required: no +- Scope: сделать `chat_id` строго UUID и убрать path alias/whole-root mount риск через неканоничные значения +- Файлы: `adapter/http/fastapi/schemas.py`, `adapter/docker/runtime.py`, при необходимости `usecase/sandbox.py` и тесты в `test/*` +- Решение: принять `chat_id` как UUID на HTTP boundary, использовать его каноничную строковую форму дальше в usecase/repository/path construction и не принимать произвольные path-like строки +- Критерии приемки: не-UUID значения отклоняются на HTTP boundary с `400/422`; UUID используется как единое каноничное значение для lock key, repository key и filesystem path; появляются регрессионные тесты на invalid `chat_id` + +### M16. Lifecycle reconciliation на startup/shutdown + +- Субагент: `feature-developer` +- Статус: pending +- Зависимости: `M13` +- Commit required: no +- Scope: устранить restart-gap между in-memory registry и уже запущенными Docker containers +- Файлы: `adapter/docker/runtime.py`, `adapter/di/container.py`, `adapter/http/fastapi/app.py`, при необходимости новые outer-layer helper files и тесты в `test/*` +- Решение: основная стратегия — reconciliation по Docker labels на startup, чтобы после restart master-service продолжал видеть уже запущенные sandbox и не поднимал дубликаты; graceful shutdown cleanup остается опциональным дополнением +- Критерии приемки: после restart master-service может восстановить/синхронизировать state по Docker labels без потери работающих agent containers; one-sandbox-per-chat не нарушается из-за пустого in-memory registry; lifecycle policy явно зафиксирована и покрыта тестами + +### M17. Управление жизненным циклом per-chat locks + +- Субагент: `feature-developer` +- Статус: pending +- Зависимости: `M13` +- Commit required: no +- Scope: ограничить неограниченный рост registry locks по числу когда-либо увиденных `chat_id` +- Файлы: `repository/sandbox_lock.py`, при необходимости тесты в `test/*` +- Решение: добавить eviction/ref-count/weakref policy во внешнем lock registry без нарушения сериализации lifecycle для активного `chat_id` +- Критерии приемки: registry locks не растет бесконечно без причины; сериализация для активных чатов сохраняется; поведение покрыто тестами diff --git a/test/test_create_http.py b/test/test_create_http.py index 478b61a..bbdbc6d 100644 --- a/test/test_create_http.py +++ b/test/test_create_http.py @@ -28,6 +28,9 @@ from repository.sandbox_session import InMemorySandboxSessionRepository from usecase.interface import Attrs from usecase.sandbox import CleanupExpiredSandboxes, CreateSandbox, CreateSandboxCommand +CHAT_ID = '123e4567-e89b-12d3-a456-426614174000' +NON_CANONICAL_CHAT_ID = '123E4567E89B12D3A456426614174000' + class FakeLogger: def __init__(self) -> None: @@ -246,12 +249,12 @@ async def exercise_get_request( await app.router.shutdown() -def test_post_create_returns_session(monkeypatch) -> None: +def test_post_create_returns_session_with_canonical_chat_id(monkeypatch) -> None: config = build_config() expires_at = datetime(2026, 4, 2, 12, 5, tzinfo=UTC) session = SandboxSession( session_id='session-123', - chat_id='chat-123', + chat_id=CHAT_ID, container_id='container-123', status=SandboxStatus.RUNNING, created_at=expires_at - timedelta(minutes=5), @@ -276,19 +279,19 @@ def test_post_create_returns_session(monkeypatch) -> None: app = app_module.create_app(config=config) status_code, response = asyncio.run( - exercise_create_request(app, {'chat_id': 'chat-123'}) + exercise_create_request(app, {'chat_id': NON_CANONICAL_CHAT_ID}) ) assert status_code == 200 assert response == { 'session_id': 'session-123', - 'chat_id': 'chat-123', + 'chat_id': CHAT_ID, 'container_id': 'container-123', 'status': 'running', 'expires_at': '2026-04-02T12:05:00Z', } assert len(create_usecase.commands) == 1 - assert create_usecase.commands[0].chat_id == 'chat-123' + assert create_usecase.commands[0].chat_id == CHAT_ID assert cleanup_usecase.calls >= 1 assert any( message == 'http_request' @@ -299,10 +302,19 @@ def test_post_create_returns_session(monkeypatch) -> None: assert docker_client.close_calls == 1 -def test_post_create_maps_start_errors_to_service_unavailable(monkeypatch) -> None: +def test_post_create_rejects_non_uuid_chat_id(monkeypatch) -> None: config = build_config() + expires_at = datetime(2026, 4, 2, 12, 5, tzinfo=UTC) + session = SandboxSession( + session_id='session-123', + chat_id=CHAT_ID, + container_id='container-123', + status=SandboxStatus.RUNNING, + created_at=expires_at - timedelta(minutes=5), + expires_at=expires_at, + ) logger = FakeLogger() - create_usecase = FakeCreateSandboxUsecase(error=SandboxStartError('chat-123')) + create_usecase = FakeCreateSandboxUsecase(session=session) cleanup_usecase = FakeCleanupExpiredSandboxes() docker_client = FakeDockerClient() container = build_container( @@ -320,7 +332,37 @@ def test_post_create_maps_start_errors_to_service_unavailable(monkeypatch) -> No app = app_module.create_app(config=config) status_code, response = asyncio.run( - exercise_create_request(app, {'chat_id': 'chat-123'}) + exercise_create_request(app, {'chat_id': 'x/../y'}) + ) + + assert status_code == 422 + assert 'detail' in response + assert create_usecase.commands == [] + assert docker_client.close_calls == 1 + + +def test_post_create_maps_start_errors_to_service_unavailable(monkeypatch) -> None: + config = build_config() + logger = FakeLogger() + create_usecase = FakeCreateSandboxUsecase(error=SandboxStartError(CHAT_ID)) + cleanup_usecase = FakeCleanupExpiredSandboxes() + docker_client = FakeDockerClient() + container = build_container( + config, + create_usecase, + cleanup_usecase, + logger, + docker_client, + ) + monkeypatch.setattr(app_module, 'build_container', lambda **kwargs: container) + monkeypatch.setattr( + app_module.FastAPIInstrumentor, 'instrument_app', lambda *args, **kwargs: None + ) + + app = app_module.create_app(config=config) + + status_code, response = asyncio.run( + exercise_create_request(app, {'chat_id': CHAT_ID}) ) assert status_code == 503 @@ -349,7 +391,7 @@ def test_post_create_maps_generic_sandbox_errors_to_internal_error(monkeypatch) app = app_module.create_app(config=config) status_code, response = asyncio.run( - exercise_create_request(app, {'chat_id': 'chat-123'}) + exercise_create_request(app, {'chat_id': CHAT_ID}) ) assert status_code == 500 @@ -362,7 +404,7 @@ def test_removed_user_endpoint_returns_not_found(monkeypatch) -> None: expires_at = datetime(2026, 4, 2, 12, 5, tzinfo=UTC) session = SandboxSession( session_id='session-123', - chat_id='chat-123', + chat_id=CHAT_ID, container_id='container-123', status=SandboxStatus.RUNNING, created_at=expires_at - timedelta(minutes=5), diff --git a/test/test_docker_runtime.py b/test/test_docker_runtime.py index 338fe1f..829024d 100644 --- a/test/test_docker_runtime.py +++ b/test/test_docker_runtime.py @@ -12,6 +12,9 @@ from adapter.docker.runtime import DockerSandboxRuntime from domain.error import SandboxError, SandboxStartError from domain.sandbox import SandboxStatus +CHAT_ID = '123e4567-e89b-12d3-a456-426614174000' +NON_CANONICAL_CHAT_ID = '123E4567E89B12D3A456426614174000' + class FakeContainer: def __init__(self, container_id: str) -> None: @@ -92,7 +95,9 @@ def build_config(tmp_path: Path) -> SandboxConfig: ) -def test_runtime_create_applies_mount_policy_and_labels(tmp_path: Path) -> None: +def test_runtime_create_applies_mount_policy_and_labels_with_canonical_chat_id( + tmp_path: Path, +) -> None: config = build_config(tmp_path) (tmp_path / 'dependencies').mkdir() (tmp_path / 'lambda-tools').mkdir() @@ -103,25 +108,25 @@ def test_runtime_create_applies_mount_policy_and_labels(tmp_path: Path) -> None: session = runtime.create( session_id='session-123', - chat_id='chat-123', + chat_id=NON_CANONICAL_CHAT_ID, created_at=created_at, expires_at=expires_at, ) assert session.session_id == 'session-123' - assert session.chat_id == 'chat-123' + assert session.chat_id == CHAT_ID assert session.container_id == 'container-123' assert session.status is SandboxStatus.RUNNING assert session.created_at == created_at assert session.expires_at == expires_at - assert (tmp_path / 'chats' / 'chat-123').is_dir() + assert (tmp_path / 'chats' / CHAT_ID).is_dir() call = containers.run_calls[0] assert call['args'] == ('sandbox:latest',) assert call['kwargs']['detach'] is True assert call['kwargs']['labels'] == { 'session_id': 'session-123', - 'chat_id': 'chat-123', + 'chat_id': CHAT_ID, 'expires_at': expires_at.isoformat(), } @@ -129,7 +134,7 @@ def test_runtime_create_applies_mount_policy_and_labels(tmp_path: Path) -> None: assert [dict(mount) for mount in mounts] == [ { 'Target': '/workspace/chat', - 'Source': str((tmp_path / 'chats' / 'chat-123').resolve(strict=False)), + 'Source': str((tmp_path / 'chats' / CHAT_ID).resolve(strict=False)), 'Type': 'bind', 'ReadOnly': False, }, @@ -160,13 +165,13 @@ def test_runtime_create_raises_start_error_when_container_id_is_missing( with pytest.raises(SandboxStartError) as excinfo: runtime.create( session_id='session-123', - chat_id='chat-123', + chat_id=CHAT_ID, created_at=datetime(2026, 4, 2, 12, 0, tzinfo=UTC), expires_at=datetime(2026, 4, 2, 12, 5, tzinfo=UTC), ) assert str(excinfo.value) == 'sandbox_start_failed' - assert excinfo.value.chat_id == 'chat-123' + assert excinfo.value.chat_id == CHAT_ID def test_runtime_stop_ignores_missing_container(tmp_path: Path) -> None: @@ -192,7 +197,8 @@ def test_runtime_stop_wraps_docker_errors(tmp_path: Path) -> None: assert str(excinfo.value) == 'sandbox_stop_failed' -def test_runtime_create_rejects_chat_path_traversal(tmp_path: Path) -> None: +@pytest.mark.parametrize('chat_id', ['.', 'a/..', 'x/../y']) +def test_runtime_create_rejects_non_uuid_chat_id(tmp_path: Path, chat_id: str) -> None: config = build_config(tmp_path) (tmp_path / 'dependencies').mkdir() (tmp_path / 'lambda-tools').mkdir() @@ -202,11 +208,11 @@ def test_runtime_create_rejects_chat_path_traversal(tmp_path: Path) -> None: with pytest.raises(SandboxStartError) as excinfo: runtime.create( session_id='session-123', - chat_id='../escape', + chat_id=chat_id, created_at=datetime(2026, 4, 2, 12, 0, tzinfo=UTC), expires_at=datetime(2026, 4, 2, 12, 5, tzinfo=UTC), ) assert str(excinfo.value) == 'sandbox_start_failed' - assert excinfo.value.chat_id == '../escape' + assert excinfo.value.chat_id == chat_id assert containers.run_calls == [] diff --git a/test/test_sandbox_usecase.py b/test/test_sandbox_usecase.py index 1631d1b..26e094f 100644 --- a/test/test_sandbox_usecase.py +++ b/test/test_sandbox_usecase.py @@ -6,6 +6,14 @@ from repository.sandbox_lock import ProcessLocalSandboxLifecycleLocker from repository.sandbox_session import InMemorySandboxSessionRepository from usecase.sandbox import CleanupExpiredSandboxes, CreateSandbox, CreateSandboxCommand +CHAT_ID = '11111111-1111-1111-1111-111111111111' +NON_CANONICAL_CHAT_ID = '11111111111111111111111111111111' +EXPIRED_CHAT_ID = '22222222-2222-2222-2222-222222222222' +BOUNDARY_CHAT_ID = '33333333-3333-3333-3333-333333333333' +ACTIVE_CHAT_ID = '44444444-4444-4444-4444-444444444444' +FAIL_CHAT_ID = '55555555-5555-5555-5555-555555555555' +CLEAN_CHAT_ID = '66666666-6666-6666-6666-666666666666' + class FakeClock: def __init__(self, now: datetime) -> None: @@ -183,7 +191,7 @@ def test_create_sandbox_reuses_active_session_when_not_expired() -> None: now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) session = SandboxSession( session_id='session-1', - chat_id='chat-1', + chat_id=CHAT_ID, container_id='container-1', status=SandboxStatus.RUNNING, created_at=now - timedelta(minutes=1), @@ -203,19 +211,19 @@ def test_create_sandbox_reuses_active_session_when_not_expired() -> None: ttl=timedelta(minutes=5), ) - result = usecase.execute(CreateSandboxCommand(chat_id='chat-1')) + result = usecase.execute(CreateSandboxCommand(chat_id=CHAT_ID)) assert result == session assert runtime.create_calls == [] assert runtime.stop_calls == [] - assert repository.get_active_by_chat_id('chat-1') == session - assert locker.chat_ids == ['chat-1'] + assert repository.get_active_by_chat_id(CHAT_ID) == session + assert locker.chat_ids == [CHAT_ID] assert logger.messages == [ ( 'info', 'sandbox_reused', { - 'chat_id': 'chat-1', + 'chat_id': CHAT_ID, 'session_id': 'session-1', 'container_id': 'container-1', }, @@ -229,7 +237,7 @@ def test_create_sandbox_replaces_expired_session_and_creates_new_one( now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) expired_session = SandboxSession( session_id='session-old', - chat_id='chat-1', + chat_id=CHAT_ID, container_id='container-old', status=SandboxStatus.RUNNING, created_at=now - timedelta(minutes=10), @@ -250,33 +258,33 @@ def test_create_sandbox_replaces_expired_session_and_creates_new_one( ) monkeypatch.setattr('usecase.sandbox._new_session_id', lambda: 'session-new') - result = usecase.execute(CreateSandboxCommand(chat_id='chat-1')) + result = usecase.execute(CreateSandboxCommand(chat_id=CHAT_ID)) assert runtime.stop_calls == ['container-old'] assert runtime.create_calls == [ { 'session_id': 'session-new', - 'chat_id': 'chat-1', + 'chat_id': CHAT_ID, 'created_at': now, 'expires_at': now + timedelta(minutes=5), } ] assert result == SandboxSession( session_id='session-new', - chat_id='chat-1', + chat_id=CHAT_ID, container_id='container-session-new', status=SandboxStatus.RUNNING, created_at=now, expires_at=now + timedelta(minutes=5), ) - assert repository.get_active_by_chat_id('chat-1') == result - assert locker.chat_ids == ['chat-1'] + assert repository.get_active_by_chat_id(CHAT_ID) == result + assert locker.chat_ids == [CHAT_ID] assert logger.messages == [ ( 'info', 'sandbox_replaced', { - 'chat_id': 'chat-1', + 'chat_id': CHAT_ID, 'session_id': 'session-old', 'container_id': 'container-old', }, @@ -285,7 +293,7 @@ def test_create_sandbox_replaces_expired_session_and_creates_new_one( 'info', 'sandbox_created', { - 'chat_id': 'chat-1', + 'chat_id': CHAT_ID, 'session_id': 'session-new', 'container_id': 'container-session-new', }, @@ -308,9 +316,9 @@ def test_create_sandbox_creates_new_session_when_none_exists() -> None: ttl=timedelta(minutes=5), ) - result = usecase.execute(CreateSandboxCommand(chat_id='chat-1')) + result = usecase.execute(CreateSandboxCommand(chat_id=NON_CANONICAL_CHAT_ID)) - assert result.chat_id == 'chat-1' + assert result.chat_id == CHAT_ID assert result.container_id == f'container-{result.session_id}' assert result.status is SandboxStatus.RUNNING assert result.created_at == now @@ -318,19 +326,20 @@ def test_create_sandbox_creates_new_session_when_none_exists() -> None: assert len(runtime.create_calls) == 1 assert runtime.create_calls[0] == { 'session_id': result.session_id, - 'chat_id': 'chat-1', + 'chat_id': CHAT_ID, 'created_at': now, 'expires_at': now + timedelta(minutes=5), } assert runtime.stop_calls == [] - assert repository.get_active_by_chat_id('chat-1') == result - assert locker.chat_ids == ['chat-1'] + assert repository.get_active_by_chat_id(CHAT_ID) == result + assert repository.get_active_by_chat_id(NON_CANONICAL_CHAT_ID) is None + assert locker.chat_ids == [CHAT_ID] assert logger.messages == [ ( 'info', 'sandbox_created', { - 'chat_id': 'chat-1', + 'chat_id': CHAT_ID, 'session_id': result.session_id, 'container_id': result.container_id, }, @@ -361,7 +370,7 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id( def run_create(index: int) -> None: try: - results[index] = usecase.execute(CreateSandboxCommand(chat_id='chat-1')) + results[index] = usecase.execute(CreateSandboxCommand(chat_id=CHAT_ID)) except Exception as exc: errors.append(exc) @@ -384,7 +393,7 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id( assert results[0] == results[1] assert results[0] == SandboxSession( session_id='session-new', - chat_id='chat-1', + chat_id=CHAT_ID, container_id='container-session-new', status=SandboxStatus.RUNNING, created_at=now, @@ -392,14 +401,14 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id( ) assert len(runtime.create_calls) == 1 assert runtime.stop_calls == [] - assert repository.get_active_by_chat_id('chat-1') == results[0] - assert locker.chat_ids == ['chat-1', 'chat-1'] + assert repository.get_active_by_chat_id(CHAT_ID) == results[0] + assert locker.chat_ids == [CHAT_ID, CHAT_ID] assert logger.messages == [ ( 'info', 'sandbox_created', { - 'chat_id': 'chat-1', + 'chat_id': CHAT_ID, 'session_id': 'session-new', 'container_id': 'container-session-new', }, @@ -408,7 +417,7 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id( 'info', 'sandbox_reused', { - 'chat_id': 'chat-1', + 'chat_id': CHAT_ID, 'session_id': 'session-new', 'container_id': 'container-session-new', }, @@ -420,7 +429,7 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() -> now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) expired_session = SandboxSession( session_id='session-expired', - chat_id='chat-expired', + chat_id=EXPIRED_CHAT_ID, container_id='container-expired', status=SandboxStatus.RUNNING, created_at=now - timedelta(minutes=10), @@ -428,7 +437,7 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() -> ) boundary_session = SandboxSession( session_id='session-boundary', - chat_id='chat-boundary', + chat_id=BOUNDARY_CHAT_ID, container_id='container-boundary', status=SandboxStatus.RUNNING, created_at=now - timedelta(minutes=5), @@ -436,7 +445,7 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() -> ) active_session = SandboxSession( session_id='session-active', - chat_id='chat-active', + chat_id=ACTIVE_CHAT_ID, container_id='container-active', status=SandboxStatus.RUNNING, created_at=now - timedelta(minutes=1), @@ -461,16 +470,16 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() -> assert result == [expired_session, boundary_session] assert runtime.stop_calls == ['container-expired', 'container-boundary'] - assert repository.get_active_by_chat_id('chat-expired') is None - assert repository.get_active_by_chat_id('chat-boundary') is None - assert repository.get_active_by_chat_id('chat-active') == active_session - assert locker.chat_ids == ['chat-expired', 'chat-boundary'] + assert repository.get_active_by_chat_id(EXPIRED_CHAT_ID) is None + assert repository.get_active_by_chat_id(BOUNDARY_CHAT_ID) is None + assert repository.get_active_by_chat_id(ACTIVE_CHAT_ID) == active_session + assert locker.chat_ids == [EXPIRED_CHAT_ID, BOUNDARY_CHAT_ID] assert logger.messages == [ ( 'info', 'sandbox_cleaned', { - 'chat_id': 'chat-expired', + 'chat_id': EXPIRED_CHAT_ID, 'session_id': 'session-expired', 'container_id': 'container-expired', }, @@ -479,7 +488,7 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() -> 'info', 'sandbox_cleaned', { - 'chat_id': 'chat-boundary', + 'chat_id': BOUNDARY_CHAT_ID, 'session_id': 'session-boundary', 'container_id': 'container-boundary', }, @@ -491,7 +500,7 @@ def test_cleanup_expired_sandboxes_skips_replaced_session_from_stale_snapshot() now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) expired_snapshot = SandboxSession( session_id='session-expired', - chat_id='chat-1', + chat_id=CHAT_ID, container_id='container-expired', status=SandboxStatus.RUNNING, created_at=now - timedelta(minutes=10), @@ -499,7 +508,7 @@ def test_cleanup_expired_sandboxes_skips_replaced_session_from_stale_snapshot() ) replacement_session = SandboxSession( session_id='session-new', - chat_id='chat-1', + chat_id=CHAT_ID, container_id='container-new', status=SandboxStatus.RUNNING, created_at=now - timedelta(seconds=30), @@ -522,8 +531,8 @@ def test_cleanup_expired_sandboxes_skips_replaced_session_from_stale_snapshot() assert result == [] assert runtime.stop_calls == [] - assert repository.get_active_by_chat_id('chat-1') == replacement_session - assert locker.chat_ids == ['chat-1'] + assert repository.get_active_by_chat_id(CHAT_ID) == replacement_session + assert locker.chat_ids == [CHAT_ID] assert logger.messages == [] @@ -531,7 +540,7 @@ def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None: now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) failing_session = SandboxSession( session_id='session-fail', - chat_id='chat-fail', + chat_id=FAIL_CHAT_ID, container_id='container-fail', status=SandboxStatus.RUNNING, created_at=now - timedelta(minutes=10), @@ -539,7 +548,7 @@ def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None: ) cleaned_session = SandboxSession( session_id='session-clean', - chat_id='chat-clean', + chat_id=CLEAN_CHAT_ID, container_id='container-clean', status=SandboxStatus.RUNNING, created_at=now - timedelta(minutes=9), @@ -563,15 +572,15 @@ def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None: assert result == [cleaned_session] assert runtime.stop_calls == ['container-fail', 'container-clean'] - assert repository.get_active_by_chat_id('chat-fail') == failing_session - assert repository.get_active_by_chat_id('chat-clean') is None - assert locker.chat_ids == ['chat-fail', 'chat-clean'] + assert repository.get_active_by_chat_id(FAIL_CHAT_ID) == failing_session + assert repository.get_active_by_chat_id(CLEAN_CHAT_ID) is None + assert locker.chat_ids == [FAIL_CHAT_ID, CLEAN_CHAT_ID] assert logger.messages == [ ( 'error', 'sandbox_clean_failed', { - 'chat_id': 'chat-fail', + 'chat_id': FAIL_CHAT_ID, 'session_id': 'session-fail', 'container_id': 'container-fail', 'error': 'RuntimeError', @@ -581,7 +590,7 @@ def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None: 'info', 'sandbox_cleaned', { - 'chat_id': 'chat-clean', + 'chat_id': CLEAN_CHAT_ID, 'session_id': 'session-clean', 'container_id': 'container-clean', }, diff --git a/usecase/sandbox.py b/usecase/sandbox.py index 00946a8..d04dd82 100644 --- a/usecase/sandbox.py +++ b/usecase/sandbox.py @@ -1,6 +1,6 @@ from dataclasses import dataclass from datetime import timedelta -from uuid import uuid4 +from uuid import UUID, uuid4 from domain.sandbox import SandboxSession from usecase.interface import ( @@ -35,15 +35,17 @@ class CreateSandbox: self._ttl = ttl def execute(self, command: CreateSandboxCommand) -> SandboxSession: - with self._locker.lock(command.chat_id): - session = self._repository.get_active_by_chat_id(command.chat_id) + chat_id = _canonical_chat_id(command.chat_id) + + with self._locker.lock(chat_id): + session = self._repository.get_active_by_chat_id(chat_id) now = self._clock.now() if session is not None and session.expires_at > now: self._logger.info( 'sandbox_reused', attrs={ - 'chat_id': command.chat_id, + 'chat_id': chat_id, 'session_id': session.session_id, 'container_id': session.container_id, }, @@ -54,7 +56,7 @@ class CreateSandbox: self._logger.info( 'sandbox_replaced', attrs={ - 'chat_id': command.chat_id, + 'chat_id': chat_id, 'session_id': session.session_id, 'container_id': session.container_id, }, @@ -66,7 +68,7 @@ class CreateSandbox: expires_at = created_at + self._ttl new_session = self._runtime.create( session_id=_new_session_id(), - chat_id=command.chat_id, + chat_id=chat_id, created_at=created_at, expires_at=expires_at, ) @@ -74,7 +76,7 @@ class CreateSandbox: self._logger.info( 'sandbox_created', attrs={ - 'chat_id': command.chat_id, + 'chat_id': chat_id, 'session_id': new_session.session_id, 'container_id': new_session.container_id, }, @@ -151,3 +153,7 @@ class CleanupExpiredSandboxes: def _new_session_id() -> str: return uuid4().hex + + +def _canonical_chat_id(chat_id: str) -> str: + return str(UUID(str(chat_id).strip()))