From 44c607780167419c68664c50208d6d07705d4900 Mon Sep 17 00:00:00 2001 From: Xinmin Zeng <135568692+fancyboi999@users.noreply.github.com> Date: Mon, 26 Oct 2026 02:01:51 -0700 Subject: [PATCH] refactor(provisioner): remove ignored legacy skills arguments (#7555) * refactor(provisioner): remove ignored legacy skills arguments * chore(provisioner): document legacy flag and align test durations --- backend/.test_durations | 6 +-- backend/tests/test_provisioner_pvc_volumes.py | 42 +++---------------- docker/provisioner/app.py | 9 +--- 3 files changed, 8 insertions(+), 49 deletions(-) diff --git a/backend/.test_durations b/backend/.test_durations index a082a36982..21c9fa45aa 100644 --- a/backend/.test_durations +++ b/backend/.test_durations @@ -19529,8 +19529,6 @@ "tests/test_provisioner_mount_contract.py::test_gateway_and_provisioner_extra_mount_contracts_match": 0.020990583114326, "tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_hostpath_has_four_mounts": 0.00571479182690382, "tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_hostpath_has_four_volumes": 0.005259708035737276, - "tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_hostpath_with_legacy_has_four_mounts": 0.00440941727720201, - "tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_hostpath_with_legacy_has_four_volumes": 0.005018542055040598, "tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_includes_extra_mounts": 0.005034750327467918, "tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_pvc_has_two_mounts": 0.005018165102228522, "tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_pvc_has_two_volumes": 0.004009666852653027, @@ -19545,10 +19543,10 @@ "tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_extra_mount_uses_pvc_subpath": 0.0031363321468234062, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_skills_custom_mount": 0.0032700421288609505, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_skills_legacy_mount": 0.0029997501987963915, + "tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_skills_projection_mounts_are_stable": 0.0029834159649908543, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_skills_public_mount": 0.004140791948884726, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_userdata_read_write": 0.004402917111292481, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_uses_three_projection_mounts": 0.0030454988591372967, - "tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_without_legacy_flag_still_has_legacy_mount": 0.0029834159649908543, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_pvc_defaults_to_default_user_subpath": 0.00381799996830523, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_pvc_no_subpath_on_userdata": 0.0029956260696053505, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_pvc_returns_two_mounts": 0.003352042054757476, @@ -19574,10 +19572,10 @@ "tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_extra_mount_uses_userdata_pvc_when_configured": 0.004007165785878897, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_skills_custom_volume": 0.0037376671098172665, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_skills_legacy_volume": 0.0033499987330287695, + "tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_skills_projection_volumes_are_stable": 0.0032199581619352102, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_skills_public_volume": 0.004053332144394517, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_userdata_includes_thread_id": 0.004025542875751853, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_uses_three_projection_volumes": 0.004587041912600398, - "tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_without_legacy_flag_still_has_empty_capable_mount": 0.0032199581619352102, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_pvc_returns_two_volumes": 0.0033919168636202812, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_pvc_volume_names_are_stable": 0.00340524991042912, "tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_skills_pvc_overrides_hostpath": 0.0032209178898483515, diff --git a/backend/tests/test_provisioner_pvc_volumes.py b/backend/tests/test_provisioner_pvc_volumes.py index bea72af39b..0a84eda413 100644 --- a/backend/tests/test_provisioner_pvc_volumes.py +++ b/backend/tests/test_provisioner_pvc_volumes.py @@ -159,17 +159,14 @@ class TestBuildVolumes: def test_hostpath_skills_legacy_volume(self, provisioner_module): """Legacy projection is per-user and stable regardless of visibility.""" provisioner_module.SKILLS_PVC_NAME = "" - volumes = provisioner_module._build_volumes( - "thread-1", - include_legacy_skills=True, - ) + volumes = provisioner_module._build_volumes("thread-1") legacy = volumes[2] assert legacy.name == "skills-legacy" assert legacy.host_path is not None assert "users/default/skills_view/legacy" in posix_path(legacy.host_path.path) assert legacy.host_path.type == "Directory" - def test_hostpath_without_legacy_flag_still_has_empty_capable_mount(self, provisioner_module): + def test_hostpath_skills_projection_volumes_are_stable(self, provisioner_module): """Visibility changes update contents without recreating the Pod.""" provisioner_module.SKILLS_PVC_NAME = "" volumes = provisioner_module._build_volumes("thread-1") @@ -394,15 +391,12 @@ class TestBuildVolumeMounts: def test_hostpath_skills_legacy_mount(self, provisioner_module): """Legacy skills mount at /mnt/skills/legacy, read-only.""" provisioner_module.SKILLS_PVC_NAME = "" - mounts = provisioner_module._build_volume_mounts( - "thread-1", - include_legacy_skills=True, - ) + mounts = provisioner_module._build_volume_mounts("thread-1") assert mounts[2].name == "skills-legacy" assert mounts[2].mount_path == "/mnt/skills/legacy" assert mounts[2].read_only is True - def test_hostpath_without_legacy_flag_still_has_legacy_mount(self, provisioner_module): + def test_hostpath_skills_projection_mounts_are_stable(self, provisioner_module): """Hidden legacy content is represented by an empty mounted view.""" provisioner_module.SKILLS_PVC_NAME = "" mounts = provisioner_module._build_volume_mounts("thread-1") @@ -665,28 +659,6 @@ class TestBuildPodVolumes: pod = provisioner_module._build_pod("sandbox-1", "thread-1") assert len(pod.spec.containers[0].volume_mounts) == 4 - def test_pod_hostpath_with_legacy_has_four_volumes(self, provisioner_module): - """Legacy volume should be present when the backend requests it.""" - provisioner_module.SKILLS_PVC_NAME = "" - provisioner_module.USERDATA_PVC_NAME = "" - pod = provisioner_module._build_pod( - "sandbox-1", - "thread-1", - include_legacy_skills=True, - ) - assert len(pod.spec.volumes) == 4 - - def test_pod_hostpath_with_legacy_has_four_mounts(self, provisioner_module): - """Legacy mount should be present when the backend requests it.""" - provisioner_module.SKILLS_PVC_NAME = "" - provisioner_module.USERDATA_PVC_NAME = "" - pod = provisioner_module._build_pod( - "sandbox-1", - "thread-1", - include_legacy_skills=True, - ) - assert len(pod.spec.containers[0].volume_mounts) == 4 - def test_pod_pvc_has_two_volumes(self, provisioner_module): """PVC Pod spec should contain exactly 2 volumes.""" provisioner_module.SKILLS_PVC_NAME = "skills-pvc" @@ -746,11 +718,7 @@ class TestBuildPodVolumes: """Ensure public/custom/legacy mount paths are correct.""" provisioner_module.SKILLS_PVC_NAME = "" provisioner_module.USERDATA_PVC_NAME = "" - pod = provisioner_module._build_pod( - "sandbox-1", - "thread-1", - include_legacy_skills=True, - ) + pod = provisioner_module._build_pod("sandbox-1", "thread-1") mount_paths = {m.name: m.mount_path for m in pod.spec.containers[0].volume_mounts} assert mount_paths["skills-public"] == "/mnt/skills/public" assert mount_paths["skills-custom"] == "/mnt/skills/custom" diff --git a/docker/provisioner/app.py b/docker/provisioner/app.py index d4c12578f6..8c9593acaa 100644 --- a/docker/provisioner/app.py +++ b/docker/provisioner/app.py @@ -510,6 +510,7 @@ class CreateSandboxRequest(BaseModel): thread_id: str | None = Field(default=None, pattern=SAFE_THREAD_ID_PATTERN) user_id: str = Field(default=DEFAULT_USER_ID, pattern=SAFE_USER_ID_PATTERN) extra_mounts: list[ExtraMount] = Field(default_factory=list) + # Accepted and logged for older Gateway requests; no longer gates mounts. include_legacy_skills: bool = False # Sent explicitly by new Gateways; the default keeps old Gateways compatible. skills_container_path: str = DEFAULT_SKILLS_CONTAINER_PATH @@ -621,7 +622,6 @@ def _build_volumes( thread_id: str, user_id: str = DEFAULT_USER_ID, *, - include_legacy_skills: bool = False, extra_mounts: list[ExtraMount] | None = None, skills_container_path: str = DEFAULT_SKILLS_CONTAINER_PATH, provision_lark_cli_runtime: bool = False, @@ -635,7 +635,6 @@ def _build_volumes( ``LocalSandboxProvider`` and ``AioSandboxProvider``. """ volumes: list[k8s_client.V1Volume] = [] - del include_legacy_skills # retained for request compatibility skills_root = _normalize_skills_container_path(skills_container_path) managed_skill_paths = _managed_skill_category_mount_paths(skills_root) validated_extra_mounts = _validated_extra_mounts( @@ -786,7 +785,6 @@ def _build_volume_mounts( thread_id: str, user_id: str = DEFAULT_USER_ID, *, - include_legacy_skills: bool = False, extra_mounts: list[ExtraMount] | None = None, skills_container_path: str = DEFAULT_SKILLS_CONTAINER_PATH, provision_lark_cli_runtime: bool = False, @@ -800,7 +798,6 @@ def _build_volume_mounts( scope that mount with ``SKILLS_PVC_SUBPATH_TEMPLATE``. """ mounts: list[k8s_client.V1VolumeMount] = [] - del include_legacy_skills # retained for request compatibility skills_root = _normalize_skills_container_path(skills_container_path) managed_skill_paths = _managed_skill_category_mount_paths(skills_root) validated_extra_mounts = _validated_extra_mounts( @@ -996,7 +993,6 @@ def _build_pod( thread_id: str, user_id: str = DEFAULT_USER_ID, *, - include_legacy_skills: bool = False, extra_mounts: list[ExtraMount] | None = None, skills_container_path: str = DEFAULT_SKILLS_CONTAINER_PATH, provision_lark_cli_runtime: bool = False, @@ -1083,7 +1079,6 @@ def _build_pod( volume_mounts=_build_volume_mounts( thread_id, user_id=user_id, - include_legacy_skills=include_legacy_skills, extra_mounts=extra_mounts, skills_container_path=skills_container_path, provision_lark_cli_runtime=provision_lark_cli_runtime, @@ -1104,7 +1099,6 @@ def _build_pod( volumes=_build_volumes( thread_id, user_id=user_id, - include_legacy_skills=include_legacy_skills, extra_mounts=extra_mounts, skills_container_path=skills_container_path, provision_lark_cli_runtime=provision_lark_cli_runtime, @@ -1322,7 +1316,6 @@ def create_sandbox(req: CreateSandboxRequest): sandbox_id, thread_id, user_id=user_id, - include_legacy_skills=include_legacy_skills, extra_mounts=req.extra_mounts, skills_container_path=skills_container_path, provision_lark_cli_runtime=provision_lark_cli_runtime,