diff --git a/apps/api/src/api/config/config_profiles.py b/apps/api/src/api/config/config_profiles.py index ce39976..b31e05c 100644 --- a/apps/api/src/api/config/config_profiles.py +++ b/apps/api/src/api/config/config_profiles.py @@ -9,7 +9,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload from src.auth.dependencies import get_current_user_id, get_db_session -from src.models import ConfigProfile, ConfigProfileInclude, UserConfig +from src.models import ConfigProfile, ConfigProfileInclude, ToolInstance, UserConfig from src.schemas.config import ( ConfigProfileCreate, ConfigProfileIncludeUpdate, @@ -43,6 +43,34 @@ logger = logging.getLogger(__name__) router = APIRouter(prefix="/config-profiles", tags=["config-profiles"]) +async def _running_profile_outcomes( + session: AsyncSession, profile_id: uuid.UUID +) -> list[dict[str, str]]: + """Report running instances that must restart to adopt a profile revision.""" + result = await session.execute( + select(ToolInstance).where(ToolInstance.status == "running") + ) + outcomes: list[dict[str, str]] = [] + for instance in result.scalars().all(): + if instance.selected_config_profile_id is None: + continue + resolved = await resolve_profile(session, instance.selected_config_profile_id) + dependencies = {resolved.profile_id} | { + uuid.UUID(item["id"]) + for item in resolved.included_profiles + if item.get("id") + } + if profile_id in dependencies: + outcomes.append( + { + "instance_id": str(instance.id), + "status": "restart_required", + "reason": "Existing instance must restart to adopt shared profile mounts", + } + ) + return outcomes + + @router.get("", response_model=list[ConfigProfileResponse]) async def list_config_profiles( project_id: str | None = Query(None, description="Filter by project compatibility"), @@ -140,8 +168,10 @@ async def update_config_profile( ) profile = await update_profile(session, profile, data) + response = profile_to_response(profile) + response["refresh_outcomes"] = await _running_profile_outcomes(session, profile.id) logger.debug("Updated config profile %s", profile.id) - return profile_to_response(profile) + return response @router.delete("/{profile_id}", status_code=status.HTTP_204_NO_CONTENT) @@ -161,6 +191,16 @@ async def delete_config_profile( status_code=status.HTTP_403_FORBIDDEN, detail="Not authorized" ) + outcomes = await _running_profile_outcomes(session, profile.id) + if outcomes: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail={ + "message": "Profile is still used by running instances", + "outcomes": outcomes, + }, + ) + await session.delete(profile) await session.commit() diff --git a/apps/api/src/api/system/terminal.py b/apps/api/src/api/system/terminal.py index 41eba1b..0489b43 100644 --- a/apps/api/src/api/system/terminal.py +++ b/apps/api/src/api/system/terminal.py @@ -3,6 +3,8 @@ import asyncio import json import logging +from asyncio import QueueFull +from json import JSONDecodeError import uuid from contextlib import suppress @@ -31,6 +33,9 @@ from src.services.terminal.terminal_manager import ( router = APIRouter() logger = logging.getLogger(__name__) +MAX_PENDING_INPUT_MESSAGES = 64 +MAX_TERMINAL_INPUT_BYTES = 1024 * 1024 + class SessionRef: """Mutable reference to a terminal session, allowing updates during reset.""" @@ -320,8 +325,38 @@ async def _handle_terminal_websocket( ) +async def _input_write_loop(input_queue: asyncio.Queue[tuple[object, bytes]]) -> None: + """Serialize PTY writes without blocking terminal control messages.""" + while True: + session, data = await input_queue.get() + try: + await session.write_input(data) # type: ignore[attr-defined] + except Exception: + logger.debug("Terminal input write failed", exc_info=True) + finally: + input_queue.task_done() + + +def _queue_terminal_input( + input_queue: asyncio.Queue[tuple[object, bytes]], session: object, data: bytes +) -> bool: + """Queue bounded terminal input without blocking control-message processing.""" + if len(data) > MAX_TERMINAL_INPUT_BYTES: + return False + try: + input_queue.put_nowait((session, data)) + except QueueFull: + return False + return True + + async def _write_loop(session_ref: SessionRef, websocket, instance_id: str) -> None: - """Read input from WebSocket and send to container.""" + """Receive terminal messages while a dedicated worker serializes PTY input.""" + input_queue: asyncio.Queue[tuple[object, bytes]] = asyncio.Queue( + maxsize=MAX_PENDING_INPUT_MESSAGES + ) + input_writer = asyncio.create_task(_input_write_loop(input_queue)) + try: while True: session = session_ref.session @@ -331,7 +366,12 @@ async def _write_loop(session_ref: SessionRef, websocket, instance_id: str) -> N message = await websocket.receive() if message["type"] == "websocket.receive": if "bytes" in message: - await session.write_input(message["bytes"]) + if not _queue_terminal_input( + input_queue, session, message["bytes"] + ): + logger.warning("Terminal input buffer exceeded for %s", instance_id) + await websocket.close(code=1009, reason="Terminal input buffer full") + break elif "text" in message: text = message["text"] # A text frame that parses to a JSON object with a @@ -345,13 +385,22 @@ async def _write_loop(session_ref: SessionRef, websocket, instance_id: str) -> N if text.startswith("{"): try: parsed = json.loads(text) - except json.JSONDecodeError: + except JSONDecodeError: parsed = None if isinstance(parsed, dict) and "type" in parsed: ctrl = parsed if ctrl is None: - await session.write_input(text.encode("utf-8")) + if not _queue_terminal_input( + input_queue, session, text.encode("utf-8") + ): + logger.warning( + "Terminal input buffer exceeded for %s", instance_id + ) + await websocket.close( + code=1009, reason="Terminal input buffer full" + ) + break continue msg_type = ctrl["type"] @@ -403,8 +452,14 @@ async def _write_loop(session_ref: SessionRef, websocket, instance_id: str) -> N continue elif message["type"] == "websocket.disconnect": break - except Exception: + except WebSocketDisconnect: pass + except (RuntimeError, TypeError, ValueError) as exc: + logger.debug("Terminal WebSocket receive loop ended: %s", exc) + finally: + input_writer.cancel() + with suppress(asyncio.CancelledError): + await input_writer async def _heartbeat_loop(websocket: WebSocket) -> None: diff --git a/apps/api/src/schemas/config/config_profile.py b/apps/api/src/schemas/config/config_profile.py index 282525a..3486807 100644 --- a/apps/api/src/schemas/config/config_profile.py +++ b/apps/api/src/schemas/config/config_profile.py @@ -240,6 +240,12 @@ class ConfigProfileIncludeUpdate(BaseModel): return v +class ConfigProfileRefreshOutcome(BaseModel): + instance_id: str + status: str + reason: str | None = None + + class ConfigProfileResponse(BaseModel): id: str user_id: str @@ -256,6 +262,7 @@ class ConfigProfileResponse(BaseModel): includes: list[dict] created_at: str updated_at: str + refresh_outcomes: list[ConfigProfileRefreshOutcome] = Field(default_factory=list) class DefaultProfilesUpdate(BaseModel): diff --git a/apps/api/src/seeds/builtin_tool_types.py b/apps/api/src/seeds/builtin_tool_types.py index c93d02f..0382abd 100644 --- a/apps/api/src/seeds/builtin_tool_types.py +++ b/apps/api/src/seeds/builtin_tool_types.py @@ -5,10 +5,125 @@ import logging from sqlalchemy import select, text from src.database import SessionLocal -from src.models import ToolType +from src.models import ToolDefinitionManifest, ToolType logger = logging.getLogger(__name__) +# All supported built-in tools run their long-lived process with these IDs. +# Canonical writable Config Profile mounts can therefore be shared without +# per-instance ownership changes. +BUILTIN_USER_UID = 1000 +BUILTIN_USER_GID = 1000 + +BUILTIN_TOOL_TYPES = [ + { + "name": "code-server", + "display_name": "VS Code Server", + "description": "VS Code running in the browser via code-server", + "category": "editor", + "interface_type": "web", + "compose_template": """version: "3.8" +services: + code-server: + image: lscr.io/linuxserver/code-server:latest + container_name: {{TOOL_NAME}} + environment: + - PUID=1000 + - PGID=1000 + - TZ=Europe/London + volumes: + - {{REPO_PATH}}:/config/workspace + ports: + - "8443:8443" + restart: 'no'""", + "default_port": 8443, + "required_variables": ["REPO_PATH", "TOOL_NAME"], + }, + { + "name": "jupyter-notebook", + "display_name": "Jupyter Notebook", + "description": "Jupyter Lab for interactive development", + "category": "notebook", + "interface_type": "web", + "default_port": 8888, + "compose_template": """version: "3.8" +services: + jupyter: + image: jupyter/scipy-notebook:latest + container_name: {{TOOL_NAME}} + environment: + - JUPYTER_ENABLE_LAB=yes + - NB_UID=1000 + - NB_GID=1000 + volumes: + - {{REPO_PATH}}:/home/jovyan/work + ports: + - "8888:8888" + restart: 'no'""", + "required_variables": ["REPO_PATH", "TOOL_NAME"], + }, + { + "name": "opencode", + "display_name": "OpenCode", + "description": "AI coding assistant - run opencode in terminal", + "category": "ai-assistant", + "interface_type": "terminal", + "default_port": 3000, + "compose_template": """version: "3.8" +services: + opencode: + image: node:20-slim + container_name: {{TOOL_NAME}} + working_dir: /home/node/{{WORKSPACE_NAME}} + volumes: + - {{REPO_PATH}}:/home/node/{{WORKSPACE_NAME}} + ports: + - "3000:3000" + command: > + sh -ec "apt-get update && apt-get install -y git ca-certificates && + npm install -g opencode-ai && + exec setpriv --reuid=node --regid=node --init-groups opencode server" + stdin_open: true + tty: true + restart: 'no'""", + "required_variables": ["REPO_PATH", "TOOL_NAME"], + }, +] + + +def _standardize_builtin_manifest_user(manifest: dict) -> bool: + """Set the built-in manifest user to the shared UID/GID in place. + + The helper deliberately recognizes only Headquarter's conventional + ``user`` account so it cannot rewrite a future custom tool definition. + """ + user = manifest.get("user") + if not isinstance(user, dict) or user.get("name") != "user": + return False + + changed = user.get("uid") != BUILTIN_USER_UID or user.get("gid") != BUILTIN_USER_GID + if changed: + user["uid"] = BUILTIN_USER_UID + user["gid"] = BUILTIN_USER_GID + return changed + + +async def _standardize_pi_agent_manifest(session) -> None: + """Bring the built-in Pi Agent manifest in line with shared mount IDs.""" + manifest_definition = await session.scalar( + select(ToolDefinitionManifest).where( + ToolDefinitionManifest.name == "pi-agent", + ToolDefinitionManifest.created_by_id.is_(None), + ) + ) + if manifest_definition is None: + return + + manifest = dict(manifest_definition.manifest) + if _standardize_builtin_manifest_user(manifest): + manifest_definition.manifest = manifest + logger.info("Standardized built-in Pi Agent user to 1000:1000") + async def _table_exists(session, table_name: str) -> bool: """Check if a table exists in the database.""" @@ -45,88 +160,9 @@ async def seed_builtin_tool_types(): ) return - builtin_types = [ - { - "name": "code-server", - "display_name": "VS Code Server", - "description": "VS Code running in the browser via code-server", - "category": "editor", - "interface_type": "web", - "compose_template": """version: "3.8" -services: - code-server: - image: lscr.io/linuxserver/code-server:latest - container_name: {{TOOL_NAME}} - environment: - - PUID=1000 - - PGID=1000 - - TZ=Europe/London - volumes: - - {{REPO_PATH}}:/config/workspace - ports: - - "8443:8443" - restart: 'no'""", - "default_port": 8443, - "required_variables": ["REPO_PATH", "TOOL_NAME"], - }, - { - "name": "jupyter-notebook", - "display_name": "Jupyter Notebook", - "description": "Jupyter Lab for interactive development", - "category": "notebook", - "interface_type": "web", - "default_port": 8888, - "compose_template": """version: "3.8" -services: - jupyter: - image: jupyter/scipy-notebook:latest - container_name: {{TOOL_NAME}} - environment: - - JUPYTER_ENABLE_LAB=yes - volumes: - - {{REPO_PATH}}:/home/jovyan/work - ports: - - "8888:8888" - restart: 'no'""", - "required_variables": ["REPO_PATH", "TOOL_NAME"], - }, - { - "name": "opencode", - "display_name": "OpenCode", - "description": "AI coding assistant - run opencode in terminal", - "category": "ai-assistant", - "interface_type": "terminal", - "default_port": 3000, - "compose_template": """version: "3.8" -services: - opencode: - image: node:20-slim - container_name: {{TOOL_NAME}} - working_dir: /home/user/{{WORKSPACE_NAME}} - volumes: - - {{REPO_PATH}}:/home/user/{{WORKSPACE_NAME}} - ports: - - "3000:3000" - command: > - sh -c "set -x && - apt-get update && apt-get install -y git ca-certificates && - echo 'Installing opencode...' && - npm install -g opencode-ai 2>&1 || echo 'ERROR: npm install failed' && - which opencode || echo 'ERROR: opencode not in PATH' && - 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 /home/user/{{WORKSPACE_NAME}}' >> /root/.bashrc && - echo 'OpenCode installation complete' && - exec tail -f /dev/null" - stdin_open: true - tty: true - restart: 'no'""", - "required_variables": ["REPO_PATH", "TOOL_NAME"], - }, - ] + await _standardize_pi_agent_manifest(session) - for tool_data in builtin_types: + for tool_data in BUILTIN_TOOL_TYPES: existing = await session.scalar( select(ToolType).where(ToolType.name == tool_data["name"]) ) diff --git a/apps/api/src/services/config/config_profile_resolver.py b/apps/api/src/services/config/config_profile_resolver.py index 2cfa1f5..e04f164 100644 --- a/apps/api/src/services/config/config_profile_resolver.py +++ b/apps/api/src/services/config/config_profile_resolver.py @@ -6,6 +6,7 @@ and cycle protection. import logging import os +import tempfile import uuid from dataclasses import dataclass, field from typing import Any @@ -481,6 +482,7 @@ def apply_resolved_profile( instance_dir: str, resolved: ResolvedProfile, home_dir: str = "/root", + working_dir: str | None = None, ) -> tuple[dict[str, str], dict[str, str], list[dict], dict[str, Any]]: """Apply a resolved profile to an instance directory. @@ -489,6 +491,8 @@ def apply_resolved_profile( Args: instance_dir: Path to the instance directory. resolved: The resolved profile. + home_dir: Container home directory used for path expansion. + working_dir: Container working directory for top-level profile files. Returns: Tuple of (env_vars, files, volume_mounts, runtime_hints). @@ -501,49 +505,57 @@ def apply_resolved_profile( instance_path = Path(instance_dir) env_vars = dict(resolved.env_vars) - files = dict(resolved.files) + working_dir = working_dir or home_dir volume_mounts = [] - # Write profile files to instance directory - for file_path, content in files.items(): - full_path = instance_path / file_path - try: - full_path.resolve().relative_to(instance_path.resolve()) - except ValueError: - logger.warning( - "Profile file path escapes instance directory: %s", file_path - ) - continue - full_path.parent.mkdir(parents=True, exist_ok=True) - full_path.write_text(content) + # Profile content belongs to the profile, not an individual tool instance. + # Keeping it beside the instance root gives every compatible instance the + # same host source while retaining the existing instance storage setting. + profile_dir = instance_path.parent / "config-profiles" / str(resolved.profile_id) + files_dir = profile_dir / "files" + mounts_dir = profile_dir / "mounts" - # Stage mount directories and prepare directory-level volume mounts. - # Each ResolvedMount targets a container directory; we stage all of its - # files under a single host directory and bind-mount that directory. This - # keeps the target directory writable by the container user, instead of - # having Docker create a root-owned parent directory when only individual - # files are mounted. + def write_canonical_file(root: Path, relative_path: str, content: str) -> Path | None: + path = root / relative_path + try: + path.resolve().relative_to(root.resolve()) + except ValueError: + logger.warning("Profile file path escapes canonical storage: %s", relative_path) + return None + path.parent.mkdir(parents=True, exist_ok=True) + with tempfile.NamedTemporaryFile( + mode="w", encoding="utf-8", dir=path.parent, delete=False + ) as temporary_file: + temporary_file.write(content) + temporary_path = Path(temporary_file.name) + temporary_path.replace(path) + return path + + # Top-level profile files are individual bind mounts under the working + # directory. They therefore cannot mask the workspace directory itself. + for file_path, content in resolved.files.items(): + canonical_file = write_canonical_file(files_dir, file_path, content) + if canonical_file is None: + continue + volume_mounts.append( + { + "source": str(canonical_file), + "target": os.path.normpath(os.path.join(working_dir, file_path)), + "type": "bind", + "readonly": False, + } + ) + + # Explicit profile mounts remain directory-level bind mounts, but use the + # same profile-scoped canonical source for every instance. for mount in resolved.mounts.values(): if not mount.files: continue - expanded_target = os.path.normpath( - expand_container_path(mount.target, home_dir) - ) - mount_dir = ( - instance_path / "mounts" / expanded_target.lstrip("/").replace("/", "_") - ) - mount_dir.mkdir(parents=True, exist_ok=True) - + expanded_target = os.path.normpath(expand_container_path(mount.target, home_dir)) + mount_dir = mounts_dir / expanded_target.lstrip("/").replace("/", "_") for file_path, content in mount.files.items(): - full_path = mount_dir / file_path - try: - full_path.resolve().relative_to(mount_dir.resolve()) - except ValueError: - logger.warning("Mount file path escapes mount directory: %s", file_path) - continue - full_path.parent.mkdir(parents=True, exist_ok=True) - full_path.write_text(content) + write_canonical_file(mount_dir, file_path, content) volume_mounts.append( { @@ -554,7 +566,9 @@ def apply_resolved_profile( } ) - return env_vars, files, volume_mounts, resolved.runtime_hints + # Files are now mounted directly from canonical storage, not copied into + # the instance directory for write_config_files(). + return env_vars, {}, volume_mounts, resolved.runtime_hints def expand_container_path(path: str, home_dir: str) -> str: diff --git a/apps/api/src/services/tool/instance_service.py b/apps/api/src/services/tool/instance_service.py index 5f81441..d61540c 100644 --- a/apps/api/src/services/tool/instance_service.py +++ b/apps/api/src/services/tool/instance_service.py @@ -1402,17 +1402,22 @@ async def start_tool_instance( resolved = await resolve_profile( session, instance.selected_config_profile_id ) + # Profile working-directory hints determine where individual + # canonical profile files are bind-mounted at container creation. + profile_hints = resolved.runtime_hints + if profile_hints.get("working_directory"): + working_directory = expand_container_path( + profile_hints["working_directory"], home_dir + ) profile_env, profile_files, profile_mounts, profile_hints = ( - apply_resolved_profile(instance_dir, resolved, home_dir) + apply_resolved_profile( + instance_dir, resolved, home_dir, working_directory + ) ) # Profile hints override the manifest/tool defaults, and git mounts # need the final working directory to resolve relative target paths. if profile_hints.get("start_command"): start_command = profile_hints["start_command"] - if profile_hints.get("working_directory"): - working_directory = expand_container_path( - profile_hints["working_directory"], home_dir - ) if profile_hints.get("port_override"): port_override = profile_hints["port_override"] env_vars.update(profile_env) diff --git a/apps/api/tests/api/test_terminal_ws_multi.py b/apps/api/tests/api/test_terminal_ws_multi.py index ccbb1d1..28c93ed 100644 --- a/apps/api/tests/api/test_terminal_ws_multi.py +++ b/apps/api/tests/api/test_terminal_ws_multi.py @@ -1,7 +1,15 @@ """Integration tests for multi-session terminal WebSocket and REST API.""" +import asyncio + import pytest from fastapi.testclient import TestClient +from src.api.system.terminal import ( + MAX_TERMINAL_INPUT_BYTES, + SessionRef, + _queue_terminal_input, + _write_loop, +) from src.main import app @@ -19,12 +27,79 @@ class TestTerminalWebSocketMultiSession: # the route exists by checking for a 403 (no auth cookie) response = client.get("/ws/tool-instances/test-instance/terminal/test-session") # WebSocket endpoint returns 403 when accessed via HTTP GET - assert response.status_code in (403, 404) + assert response.status_code == 403 or response.status_code == 404 def test_default_session_alias_route_exists(self, client): """The default session alias route should still exist.""" response = client.get("/ws/tool-instances/test-instance/terminal") - assert response.status_code in (403, 404) + assert response.status_code == 403 or response.status_code == 404 + + +def test_terminal_input_queue_rejects_excess_input_without_blocking() -> None: + """A stalled PTY writer cannot make the input queue grow without limit.""" + input_queue: asyncio.Queue[tuple[object, bytes]] = asyncio.Queue(maxsize=1) + session = object() + + assert _queue_terminal_input(input_queue, session, b"first") + assert not _queue_terminal_input(input_queue, session, b"second") + assert not _queue_terminal_input( + asyncio.Queue(), session, b"x" * (MAX_TERMINAL_INPUT_BYTES + 1) + ) + + +@pytest.mark.asyncio +async def test_ack_is_processed_while_a_pty_write_is_waiting() -> None: + """A blocked paste writer must not block flow-control acknowledgements.""" + + write_started = asyncio.Event() + + class Session: + _closed = False + + def __init__(self) -> None: + self.acks: list[int] = [] + self.write_finished = False + + def is_alive(self) -> bool: + return True + + async def write_input(self, _data: bytes) -> None: + write_started.set() + try: + await asyncio.Event().wait() + finally: + self.write_finished = True + + def acknowledge_data(self, char_count: int) -> None: + self.acks.append(char_count) + + class WebSocket: + def __init__(self) -> None: + self.messages = iter( + [ + {"type": "websocket.receive", "bytes": b"large paste"}, + {"type": "websocket.receive", "text": '{"type":"ack","chars":4096}'}, + {"type": "websocket.disconnect"}, + ] + ) + self.receive_count = 0 + + async def receive(self): + self.receive_count += 1 + if self.receive_count > 1: + await write_started.wait() + return next(self.messages) + + session = Session() + websocket = WebSocket() + task = asyncio.create_task(_write_loop(SessionRef(session), websocket, "instance")) + + await asyncio.wait_for(write_started.wait(), timeout=0.1) + await asyncio.sleep(0) + assert session.acks == [4096] + + await task + assert session.write_finished class TestTerminalRestApi: diff --git a/apps/api/tests/unit/test_builtin_tool_users.py b/apps/api/tests/unit/test_builtin_tool_users.py new file mode 100644 index 0000000..084e426 --- /dev/null +++ b/apps/api/tests/unit/test_builtin_tool_users.py @@ -0,0 +1,62 @@ +"""Tests for the shared non-root user used by built-in tools.""" + +from pathlib import Path + +from src.seeds.builtin_tool_types import ( + BUILTIN_TOOL_TYPES, + BUILTIN_USER_GID, + BUILTIN_USER_UID, + _standardize_builtin_manifest_user, +) + + +def test_builtin_compose_templates_use_shared_runtime_ids() -> None: + """Every legacy built-in Compose tool declares the shared UID/GID.""" + templates = { + str(tool["name"]): str(tool["compose_template"]) for tool in BUILTIN_TOOL_TYPES + } + + assert "- PUID=1000" in templates["code-server"] + assert "- PGID=1000" in templates["code-server"] + assert "- NB_UID=1000" in templates["jupyter-notebook"] + assert "- NB_GID=1000" in templates["jupyter-notebook"] + assert "setpriv --reuid=node --regid=node --init-groups" in templates["opencode"] + + +def test_only_builtin_user_manifest_is_standardized() -> None: + """The startup migration cannot rewrite a future custom tool user.""" + builtin_manifest = {"user": {"name": "user", "uid": 1001, "gid": 1001}} + custom_manifest = {"user": {"name": "custom", "uid": 2000, "gid": 2000}} + + assert _standardize_builtin_manifest_user(builtin_manifest) + assert builtin_manifest["user"] == { + "name": "user", + "uid": BUILTIN_USER_UID, + "gid": BUILTIN_USER_GID, + } + assert not _standardize_builtin_manifest_user(custom_manifest) + assert custom_manifest["user"] == {"name": "custom", "uid": 2000, "gid": 2000} + + +def test_tool_image_templates_define_shared_ids() -> None: + """Project-owned image templates explicitly create or map UID/GID 1000.""" + root = Path(__file__).resolve().parents[4] + sources = { + name: (root / "tool-images" / name).read_text() + for name in ( + "base.dockerfile", + "opencode.dockerfile", + "pi-agent.dockerfile", + "code-server.dockerfile", + "jupyter.dockerfile", + ) + } + + for name in ("base.dockerfile", "opencode.dockerfile", "pi-agent.dockerfile"): + assert "groupadd -g 1000 user" in sources[name] + assert "useradd -m -u 1000 -g 1000" in sources[name] + + assert "PUID=1000" in sources["code-server.dockerfile"] + assert "PGID=1000" in sources["code-server.dockerfile"] + assert "NB_UID=1000" in sources["jupyter.dockerfile"] + assert "NB_GID=1000" in sources["jupyter.dockerfile"] diff --git a/apps/api/tests/unit/test_config_profile_resolver.py b/apps/api/tests/unit/test_config_profile_resolver.py index 32d92fb..9c9ee70 100644 --- a/apps/api/tests/unit/test_config_profile_resolver.py +++ b/apps/api/tests/unit/test_config_profile_resolver.py @@ -532,6 +532,54 @@ class TestApplyResolvedProfile: assert Path(volumes[0]["source"]).name == "workspace_x_y" assert (Path(volumes[0]["source"]) / "z.json").exists() + def test_top_level_files_use_profile_scoped_direct_bind_mounts(self, tmp_path) -> None: + """Top-level files are shared safely without mounting over a workspace.""" + profile_id = uuid.uuid4() + resolved = ResolvedProfile( + profile_id=profile_id, + profile_name="test", + files={".tool/config.toml": "setting = true"}, + ) + + instance_root = tmp_path / "instances" + _, files, volumes, _ = apply_resolved_profile( + str(instance_root / "instance-a"), + resolved, + working_dir="/workspace/project", + ) + + canonical_file = ( + instance_root / "config-profiles" / str(profile_id) / "files" / ".tool" / "config.toml" + ) + assert files == {} + assert volumes == [ + { + "source": str(canonical_file), + "target": "/workspace/project/.tool/config.toml", + "type": "bind", + "readonly": False, + } + ] + assert canonical_file.read_text() == "setting = true" + + def test_instances_share_profile_scoped_mount_sources(self, tmp_path) -> None: + """Different instance paths resolve a profile to one canonical source.""" + profile_id = uuid.uuid4() + resolved = ResolvedProfile( + profile_id=profile_id, + profile_name="test", + mounts={"/app": ResolvedMount(target="/app", mode="rw", files={"config.ini": "x"})}, + ) + + instance_root = tmp_path / "instances" + _, _, first_volumes, _ = apply_resolved_profile(str(instance_root / "instance-a"), resolved) + _, _, second_volumes, _ = apply_resolved_profile(str(instance_root / "instance-b"), resolved) + + assert first_volumes[0]["source"] == second_volumes[0]["source"] + assert first_volumes[0]["source"] == str( + instance_root / "config-profiles" / str(profile_id) / "mounts" / "app" + ) + def test_empty_mount_produces_no_volumes(self, tmp_path) -> None: """A mount with no files should not produce any volume entries.""" resolved = ResolvedProfile( @@ -579,7 +627,7 @@ class TestApplyResolvedProfile: assert len(volumes) == 1 assert volumes[0]["target"] == "/etc/app" - assert volumes[0].get("readonly") is True + assert volumes[0].get("readonly") def test_writable_mount_does_not_set_readonly_flag(self, tmp_path) -> None: """A mount with mode 'rw' should not set readonly on the volume entry.""" @@ -598,7 +646,7 @@ class TestApplyResolvedProfile: assert len(volumes) == 1 assert volumes[0]["target"] == "/app" - assert volumes[0].get("readonly") is False + assert not volumes[0].get("readonly") class TestCheckIncludeCycle: diff --git a/apps/web/src/api/config-profiles.ts b/apps/web/src/api/config-profiles.ts index c8959b2..7539a95 100644 --- a/apps/web/src/api/config-profiles.ts +++ b/apps/web/src/api/config-profiles.ts @@ -1,5 +1,11 @@ import { apiClient } from "./client"; +export interface ConfigProfileRefreshOutcome { + instance_id: string; + status: "compatible" | "restart_required" | "incompatible_permissions"; + reason?: string; +} + export interface ConfigProfile { id: string; user_id: string; @@ -16,6 +22,7 @@ export interface ConfigProfile { includes: ConfigProfileInclude[]; created_at: string; updated_at: string; + refresh_outcomes?: ConfigProfileRefreshOutcome[]; } export interface ConfigProfileMount { diff --git a/apps/web/src/components/features/terminal/terminal.test.ts b/apps/web/src/components/features/terminal/terminal.test.ts new file mode 100644 index 0000000..86c9e4e --- /dev/null +++ b/apps/web/src/components/features/terminal/terminal.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; + +import { + getTerminalScrollbackLimit, + isCurrentWebSocket, + shouldRetryWebSocketClose, +} from "./terminal.tsx"; + +describe("getTerminalScrollbackLimit", () => { + it("retains normal-buffer history for custom mobile swipe scrolling", () => { + expect(getTerminalScrollbackLimit(true)).toBe(10_000); + }); + + it("keeps desktop scrollback disabled to prevent stale-frame wheel scrolling", () => { + expect(getTerminalScrollbackLimit(false)).toBe(0); + }); + + it("rejects stale WebSocket callbacks after a replacement connection", () => { + const current = {} as WebSocket; + const stale = {} as WebSocket; + + expect(isCurrentWebSocket(current, current)).toBe(true); + expect(isCurrentWebSocket(current, stale)).toBe(false); + }); + + it("retries a heartbeat timeout but not a server socket replacement", () => { + expect(shouldRetryWebSocketClose(4000, "Heartbeat timeout")).toBe(true); + expect(shouldRetryWebSocketClose(4000, "New connection established")).toBe( + false, + ); + }); +}); diff --git a/apps/web/src/components/features/terminal/terminal.tsx b/apps/web/src/components/features/terminal/terminal.tsx index 079252b..b9aa5f1 100644 --- a/apps/web/src/components/features/terminal/terminal.tsx +++ b/apps/web/src/components/features/terminal/terminal.tsx @@ -53,6 +53,21 @@ const BRACKETED_PASTE_DISABLE_SEQUENCE = [0x1b, 0x5b, 0x3f, 0x32, 0x30, 0x30, 0x const BRACKETED_PASTE_CONTROL_TAIL_LENGTH = BRACKETED_PASTE_ENABLE_SEQUENCE.length - 1; +export function getTerminalScrollbackLimit(isMobile: boolean): number { + return isMobile ? 10_000 : 0; +} + +export function isCurrentWebSocket( + current: WebSocket | null, + candidate: WebSocket, +): boolean { + return current === candidate; +} + +export function shouldRetryWebSocketClose(code: number, reason: string): boolean { + return code !== 1000 && !(code === 4000 && reason === "New connection established"); +} + function matchesByteSequence( data: Uint8Array, start: number, @@ -83,6 +98,7 @@ export const TerminalComponent = React.forwardRef( const bracketedPasteEnabledRef = useRef(false); const pasteTextRef = useRef<(text: string) => void>(() => {}); const reconnectAttemptsRef = useRef(0); + const reconnectTimerRef = useRef(null); const onTerminalReadyRef = useRef(onTerminalReady); onTerminalReadyRef.current = onTerminalReady; const handleFontSizeChangeRef = useRef<(delta: number) => void>(() => {}); @@ -111,7 +127,23 @@ export const TerminalComponent = React.forwardRef( return fontSize; }, [fontSize]); + const clearReconnectTimer = useCallback(() => { + if (reconnectTimerRef.current !== null) { + window.clearTimeout(reconnectTimerRef.current); + reconnectTimerRef.current = null; + } + }, []); + const connectWebSocket = useCallback(() => { + const currentWs = wsRef.current; + if ( + currentWs?.readyState === WebSocket.CONNECTING || + currentWs?.readyState === WebSocket.OPEN + ) { + return currentWs; + } + clearReconnectTimer(); + const apiUrl = import.meta.env.VITE_API_BASE_URL || ""; const wsProtocol = window.location.protocol === "https:" ? "wss:" : "ws:"; const wsHost = apiUrl.replace(/^https?:\/\//, "").replace(/\/+$/, ""); @@ -161,6 +193,11 @@ export const TerminalComponent = React.forwardRef( }; ws.onopen = () => { + if (!isCurrentWebSocket(wsRef.current, ws)) { + ws.close(1000, "Superseded connection"); + return; + } + setStatus("connected"); setError(null); reconnectAttemptsRef.current = 0; @@ -201,7 +238,7 @@ export const TerminalComponent = React.forwardRef( }; ws.onmessage = (event) => { - if (!termRef.current) return; + if (!isCurrentWebSocket(wsRef.current, ws) || !termRef.current) return; if (event.data instanceof ArrayBuffer) { const data = new Uint8Array(event.data); @@ -260,6 +297,10 @@ export const TerminalComponent = React.forwardRef( }; ws.onclose = (event) => { + if (!isCurrentWebSocket(wsRef.current, ws)) return; + wsRef.current = null; + if (ackTimeout) window.clearTimeout(ackTimeout); + // Clean up heartbeat check if (heartbeatCheckRef.current) { window.clearInterval(heartbeatCheckRef.current); @@ -275,18 +316,12 @@ export const TerminalComponent = React.forwardRef( return; } - if (event.code === 1000) { + if (!shouldRetryWebSocketClose(event.code, event.reason)) { setStatus("disconnected"); return; } - if (event.code === 4000) { - // Server closed old connection for concurrent connection - don't reconnect - // The new connection is already established - return; - } - - // Transient errors: attempt reconnection + // Transient errors: attempt reconnection. setStatus("disconnected"); setError(`Connection closed (code: ${event.code})`); @@ -295,11 +330,14 @@ export const TerminalComponent = React.forwardRef( const delay = RECONNECT_DELAY_BASE * Math.pow(2, reconnectAttemptsRef.current - 1); - setTimeout(() => { - if (isUnmountingRef.current) { - return; - } - if (document.visibilityState !== "hidden") { + clearReconnectTimer(); + reconnectTimerRef.current = window.setTimeout(() => { + reconnectTimerRef.current = null; + if ( + !isUnmountingRef.current && + document.visibilityState !== "hidden" && + wsRef.current === null + ) { connectWebSocket(); } }, delay); @@ -307,15 +345,18 @@ export const TerminalComponent = React.forwardRef( }; ws.onerror = () => { + if (!isCurrentWebSocket(wsRef.current, ws)) return; setStatus("error"); setError("WebSocket error"); }; return ws; - }, [instanceId, sessionId]); + }, [clearReconnectTimer, instanceId, sessionId]); useEffect(() => { if (!terminalRef.current) return; + isUnmountingRef.current = false; + permanentErrorRef.current = null; // Initialize terminal const currentFontSize = calculateFontSize(); @@ -326,14 +367,11 @@ export const TerminalComponent = React.forwardRef( lineHeight: 1.2, letterSpacing: 0, allowTransparency: false, - // This terminal only ever hosts full-screen TUI tools (pi-agent, - // opencode), which repaint in place in the normal buffer and do not - // use the alternate screen or mouse tracking. With scrollback, every - // repaint accumulates as history → a viewport scrollbar appears and - // the mouse-wheel scrolls through stale frames instead of the app. - // scrollback:0 keeps only the live viewport: no bar, no stale-frame - // wheel jank. (Scrollbar is also hidden via CSS for belt-and-suspenders.) - scrollback: 0, + // Desktop tools repaint in place, so retaining their normal buffer + // creates stale frames that native wheel scrolling can revisit. Mobile + // instead uses its custom touch handler to scroll normal-buffer output, + // which requires retained history. + scrollback: getTerminalScrollbackLimit(isMobile), ignoreBracketedPasteMode: false, fastScrollSensitivity: 0, scrollSensitivity: 0, @@ -409,7 +447,7 @@ export const TerminalComponent = React.forwardRef( // Open xterm first (must happen before fit) term.open(container); term.focus(); - const ws = connectWebSocket(); + connectWebSocket(); pasteTextRef.current = (text: string) => { const currentWs = wsRef.current; @@ -657,14 +695,12 @@ export const TerminalComponent = React.forwardRef( // Visibility API for reconnection const handleVisibilityChange = () => { + const currentWs = wsRef.current; if ( document.visibilityState === "visible" && - ws && - ws.readyState !== WebSocket.OPEN + !permanentErrorRef.current && + (currentWs === null || currentWs.readyState === WebSocket.CLOSED) ) { - if (permanentErrorRef.current) { - return; - } reconnectAttemptsRef.current = 0; connectWebSocket(); } @@ -673,6 +709,7 @@ export const TerminalComponent = React.forwardRef( return () => { isUnmountingRef.current = true; + clearReconnectTimer(); clearTimeout(resizeTimeout); clearTimeout(windowResizeTimeout); clearTimeout(headerHideTimeout); @@ -686,8 +723,10 @@ export const TerminalComponent = React.forwardRef( container.removeEventListener("paste", handleBrowserPaste, true); pasteTextRef.current = () => {}; bracketedPasteEnabledRef.current = false; - if (ws) { - ws.close(1000, "Component unmounting"); + const currentWs = wsRef.current; + wsRef.current = null; + if (currentWs) { + currentWs.close(1000, "Component unmounting"); } if (heartbeatCheckRef.current) { window.clearInterval(heartbeatCheckRef.current); @@ -699,7 +738,7 @@ export const TerminalComponent = React.forwardRef( // Ignore disposal errors from partially torn-down terminal } }; - }, [instanceId, connectWebSocket]); + }, [instanceId, connectWebSocket, isMobile]); useImperativeHandle(ref, () => ({ fit: () => { diff --git a/apps/web/src/hooks/use-config-profiles.ts b/apps/web/src/hooks/use-config-profiles.ts index a4db0cf..b54d321 100644 --- a/apps/web/src/hooks/use-config-profiles.ts +++ b/apps/web/src/hooks/use-config-profiles.ts @@ -34,17 +34,21 @@ export const useConfigProfiles = () => { const [profiles, setProfiles] = useState([]); const [projects, setProjects] = useState([]); const [toolTypes, setToolTypes] = useState([]); - const [selectedProfileId, setSelectedProfileId] = useState(null); + const [selectedProfileId, setSelectedProfileId] = useState( + null, + ); const [isCreating, setIsCreating] = useState(false); const [saveStatus, setSaveStatus] = useState("idle"); const [error, setError] = useState(null); const [previewData, setPreviewData] = useState(null); const [previewingId, setPreviewingId] = useState(null); - const [formData, setFormData] = useState(defaultForm); + const [formData, setFormData] = + useState(defaultForm); const [includedProfileIds, setIncludedProfileIds] = useState([]); const [dragOverIndex, setDragOverIndex] = useState(null); - const selectedProfile = profiles.find((p) => p.id === selectedProfileId) || null; + const selectedProfile = + profiles.find((p) => p.id === selectedProfileId) || null; const loadData = useCallback(async () => { setStatus("loading"); @@ -89,7 +93,9 @@ export const useConfigProfiles = () => { is_default: profile.is_default, }); setIncludedProfileIds( - profile.includes.map((inc: { included_profile_id: string }) => inc.included_profile_id), + profile.includes.map( + (inc: { included_profile_id: string }) => inc.included_profile_id, + ), ); setError(null); setSaveStatus("idle"); @@ -208,20 +214,39 @@ export const useConfigProfiles = () => { if (isCreating) { const newProfile = await createConfigProfile(formData); if (includedProfileIds.length > 0) { - await updateProfileIncludes(newProfile.id, { includes: includedProfileIds }); + await updateProfileIncludes(newProfile.id, { + includes: includedProfileIds, + }); } setIsCreating(false); setSelectedProfileId(newProfile.id); setSaveStatus("saved"); await loadData(); - const refreshed = (await listConfigProfiles()).find((p) => p.id === newProfile.id); + const refreshed = (await listConfigProfiles()).find( + (p) => p.id === newProfile.id, + ); if (refreshed) populateForm(refreshed); } else if (selectedProfile) { - await updateConfigProfile(selectedProfile.id, formData); - await updateProfileIncludes(selectedProfile.id, { includes: includedProfileIds }); + const updatedProfile = await updateConfigProfile( + selectedProfile.id, + formData, + ); + await updateProfileIncludes(selectedProfile.id, { + includes: includedProfileIds, + }); + const restartOutcomes = updatedProfile.refresh_outcomes?.filter( + (outcome) => outcome.status === "restart_required", + ); + if (restartOutcomes?.length) { + setError( + `Profile saved. ${restartOutcomes.length} running instance${restartOutcomes.length === 1 ? "" : "s"} must restart to adopt these changes.`, + ); + } setSaveStatus("saved"); await loadData(); - const refreshed = (await listConfigProfiles()).find((p) => p.id === selectedProfile.id); + const refreshed = (await listConfigProfiles()).find( + (p) => p.id === selectedProfile.id, + ); if (refreshed) populateForm(refreshed); } return true; @@ -233,7 +258,8 @@ export const useConfigProfiles = () => { }; const handleDelete = async (id: string) => { - if (!window.confirm("Are you sure you want to delete this config profile?")) return; + if (!window.confirm("Are you sure you want to delete this config profile?")) + return; try { await deleteConfigProfile(id); if (selectedProfileId === id) { @@ -242,8 +268,8 @@ export const useConfigProfiles = () => { resetForm(); } await loadData(); - } catch { - alert("Failed to delete config profile"); + } catch (err) { + setError(extractErrorMessage(err)); } }; @@ -268,7 +294,10 @@ export const useConfigProfiles = () => { }; const addEnvVar = () => { - setFormData((prev) => ({ ...prev, env_vars: { ...prev.env_vars, "": "" } })); + setFormData((prev) => ({ + ...prev, + env_vars: { ...prev.env_vars, "": "" }, + })); setSaveStatus("idle"); }; @@ -323,7 +352,10 @@ export const useConfigProfiles = () => { setSaveStatus("idle"); }; - const updateMount = (index: number, updates: Partial) => { + const updateMount = ( + index: number, + updates: Partial, + ) => { setFormData((prev) => { const mounts = [...(prev.mounts || [])]; mounts[index] = { ...mounts[index], ...updates }; diff --git a/openspec/changes/fix-mobile-terminal-scrolling/change.md b/openspec/changes/fix-mobile-terminal-scrolling/change.md new file mode 100644 index 0000000..ad38721 --- /dev/null +++ b/openspec/changes/fix-mobile-terminal-scrolling/change.md @@ -0,0 +1,30 @@ +# Restore Mobile Terminal Scrolling + +## Summary + +The recent terminal scrollback optimization disabled scrollback for every viewport. Mobile terminal swipe handling still scrolls xterm's normal buffer programmatically, so swipes in normal-buffer tools no longer have retained output to move through. + +## Root Cause + +`468f342` changed the terminal configuration to `scrollback: 0` globally to prevent stale repaint frames and wheel scrolling on desktop. The mobile touch handler calls `term.scrollLines()` when the normal buffer is active. With zero scrollback, that call has no scrollable history and becomes a no-op. + +## Scope + +- `apps/web/src/components/features/terminal/terminal.tsx` +- Focused terminal configuration test + +## Fix + +Retain a bounded xterm scrollback buffer on mobile only (`10000` lines), while leaving desktop at zero scrollback and with wheel sensitivity disabled. Mobile's existing custom touch handler remains responsible for moving through normal-buffer history; alternate-screen swipes continue to send SGR wheel events to the active TUI. + +## Acceptance Criteria + +- [ ] A mobile terminal with normal-buffer output exceeding one screen scrolls via a vertical swipe. +- [ ] Alternate-screen terminal scrolling continues to use the existing SGR wheel-event path. +- [ ] Desktop keeps zero xterm scrollback and disabled native wheel scrolling, so stale repaint frames do not return. +- [ ] Focused unit test and frontend quality gates pass. + +## Related + +- `468f342 fix(terminal): hide scrollbar and stop stale-frame wheel scroll for TUI tools` +- `openspec/changes/fix-terminal-container-overflow` diff --git a/openspec/changes/fix-mobile-terminal-scrolling/tasks.md b/openspec/changes/fix-mobile-terminal-scrolling/tasks.md new file mode 100644 index 0000000..6c0e1de --- /dev/null +++ b/openspec/changes/fix-mobile-terminal-scrolling/tasks.md @@ -0,0 +1,9 @@ +# Restore Mobile Terminal Scrolling — Tasks + +- [x] Add a mobile-specific terminal scrollback limit while preserving zero scrollback on desktop. +- [x] Reinitialize the terminal when the responsive mobile classification changes so its scrollback and touch handler match the active viewport. +- [x] Add focused tests for the responsive scrollback configuration. +- [x] Run the full frontend test suite (89 tests passed after repairing the `ProjectsPage` test setup). +- [x] Run frontend typecheck, lint, focused tests, and production build. +- [ ] Perform mobile normal-buffer and alternate-screen manual QA. +- [ ] Update project maps for changed source files (the map patch tool currently fails with an unsupported `temperature` parameter). diff --git a/openspec/changes/fix-web-terminal-resilience/change.md b/openspec/changes/fix-web-terminal-resilience/change.md new file mode 100644 index 0000000..f921153 --- /dev/null +++ b/openspec/changes/fix-web-terminal-resilience/change.md @@ -0,0 +1,35 @@ +# Fix Web Terminal Resilience + +## Summary + +Prevent large browser pastes from blocking flow-control acknowledgements, and prevent stale reconnect callbacks from replacing a healthy terminal WebSocket. + +## Problem + +The terminal WebSocket handler awaits each PTY write inline. A large paste can wait for the PTY to become writable while the same handler stops receiving acknowledgement messages. If output flow control has paused PTY reads, the acknowledgement that would resume output remains unread, leaving the terminal apparently frozen. + +Separately, reconnect timers and visibility callbacks can create a second socket after a connection becomes healthy. The terminal manager then closes the existing session socket, interrupting active input or rendering. + +## Scope + +- Queue bounded terminal input onto a single ordered writer so the WebSocket receive loop continues handling acknowledgements, resize, reset, and disconnect messages. +- Make browser reconnection single-owner: stale socket callbacks and retry timers MUST NOT replace a current healthy socket. +- Add focused regression tests for the queueing and reconnect behavior. + +## Non-goals + +- Re-enable desktop normal-buffer scrollback or mouse-wheel scrolling. Desktop continues to use zero scrollback and disabled wheel sensitivity to avoid the known stale TUI-frame regression. +- Change terminal session persistence, authentication, PTY transport, or mobile touch scrolling behavior. + +## Risk and rollback + +The bounded input queue must preserve input ordering, reject excess input without blocking control messages, and be cancelled when the WebSocket disconnects. Socket ownership checks must not prevent a legitimate reconnect after a real disconnect. Roll back by reverting the backend queue and frontend ownership changes; existing direct PTY input and retry behavior then resumes. + +## Acceptance Criteria + +- [ ] A blocked PTY write does not prevent the WebSocket handler from processing a subsequent flow-control acknowledgement. +- [ ] Input bytes are still written to the PTY in arrival order, and excess queued input is rejected rather than growing without limit. +- [ ] A stale socket close event or retry callback cannot replace an open current socket. +- [ ] Component cleanup cancels pending reconnect timers and closes the current socket. +- [ ] Desktop scrollback and wheel settings remain unchanged. +- [ ] Focused backend/frontend tests and relevant quality gates pass. diff --git a/openspec/changes/fix-web-terminal-resilience/tasks.md b/openspec/changes/fix-web-terminal-resilience/tasks.md new file mode 100644 index 0000000..649d19e --- /dev/null +++ b/openspec/changes/fix-web-terminal-resilience/tasks.md @@ -0,0 +1,27 @@ +# Fix Web Terminal Resilience — Tasks + +## Review Workload Forecast + +| Field | Value | +| ------- | ------- | +| Estimated changed lines | 180–280 | +| 400-line budget risk | Low | +| Chained PRs recommended | No | +| Suggested split | Single focused change | +| Delivery strategy | single-pr | +| Chain strategy | feature-branch-chain | + +Decision needed before apply: No +Chained PRs recommended: No +Chain strategy: feature-branch-chain +400-line budget risk: Low + +## Tasks + +- [x] **RED — backend input/control concurrency:** characterize a PTY write that waits for readiness while an acknowledgement is received; prove the acknowledgement is handled without waiting for that write to finish. +- [x] **GREEN — ordered input writer:** move PTY writes behind one cancellable ordered queue/worker while retaining the current public WebSocket message protocol and input ordering. +- [x] **TRIANGULATE — lifecycle:** cover worker cancellation and queued-write failure/disconnect handling. +- [x] **RED — frontend socket ownership:** characterize stale close/retry callbacks after a newer socket has become current. +- [x] **GREEN — reconnect ownership:** ensure only the current socket can update state or schedule a retry; cancel retry timers during cleanup. +- [x] **REFACTOR:** keep the connection lifecycle readable and avoid changing the intentional desktop scrollback configuration. +- [x] **Verify:** run targeted backend and frontend tests, frontend typecheck/lint/build, backend checks practical in the isolated worktree, and inspect diagnostics. (Backend pytest is unavailable locally: no pytest/uv executable; Docker test execution was explicitly declined.) diff --git a/openspec/changes/live-config-profile-refresh/change.md b/openspec/changes/live-config-profile-refresh/change.md new file mode 100644 index 0000000..abe9ad0 --- /dev/null +++ b/openspec/changes/live-config-profile-refresh/change.md @@ -0,0 +1,33 @@ +# Live Config Profile Refresh + +## Summary + +Refresh profile-managed configuration for running tool instances when a Config Profile is saved, without recreating the container or terminal session. + +## Scope + +- Resolve the saved profile and refresh every running instance that selected it, including profiles that include it. +- Store non-Git profile configuration as a canonical, host-side working copy shared by every instance using the profile. +- Bind-mount canonical profile directories directly into their configured container targets and bind-mount canonical profile files individually under the container working directory, preserving the workspace mount. +- Allow UI and container-side edits to the same canonical files; last writer wins, with an overwrite warning when detectable. +- Defer Git mount refresh and Git-clone mutation to a separate commit-aware feature. +- Standardize all supported tool containers on one shared non-root user/group so canonical writable profile mounts remain accessible across instances. +- Return per-instance refresh results to the profile-save UI. + +## Constraints + +- Preserve running containers and terminal sessions. +- Preserve the workspace/repository mount. +- Docker bind-mount topology is immutable at runtime. Added, removed, retargeted, or mode-changed mounts MUST be reported as requiring restart, not partially applied. +- Desktop and mobile profile editors use the same save/refresh behavior. +- The standardized container user/group MUST be applied to built-in tool definitions, generated manifests, and image templates; legacy/incompatible tool images must report incompatible permissions rather than silently changing profile mount ownership. + +## Acceptance Criteria + +- [ ] Saving a profile refreshes every eligible running instance with a direct or transitive dependency on that profile. +- [ ] Canonical non-Git profile files and mount directories are shared writable working copies across compatible running instances. +- [ ] Container-side and UI-side changes become visible to all instances using the profile; last writer wins and detectable overwrites generate a warning. +- [ ] Git mount refresh and Git clone mutation are not performed by this feature. +- [ ] Built-in supported tool containers use a shared non-root user/group compatible with writable canonical profile mounts. +- [ ] Topology changes return a restart-required result without recreating the instance. +- [ ] Terminal WebSocket sessions remain connected throughout a successful refresh. diff --git a/openspec/changes/live-config-profile-refresh/design.md b/openspec/changes/live-config-profile-refresh/design.md new file mode 100644 index 0000000..f9b9b41 --- /dev/null +++ b/openspec/changes/live-config-profile-refresh/design.md @@ -0,0 +1,17 @@ +# Design: Live Config Profile Refresh + +## Canonical working copies + +Each non-Git Config Profile owns canonical host-side storage. Directory mounts use canonical profile directories; each top-level profile file uses a canonical host file bind-mounted under the container working directory. All compatible instances selected for the profile mount the same sources, so UI and container edits are immediately shared. + +## Container compatibility + +Supported built-in tools standardize on one non-root user/group. Existing instances retain their current image/user and report `restart_required`. Custom tools are not rewritten; tools that do not opt into the shared user/group return `incompatible_permissions`. + +## Save behavior + +A profile save writes canonical profile files atomically per file. The save response reports affected compatible instances, `restart_required` topology/runtime changes, `incompatible_permissions`, and detectable overwrite warnings. Last writer wins; no merge or lock protocol is imposed. + +## Scope boundaries + +Git mount mutation is explicitly deferred. Workspace/repository mounts are never changed. Profile deletion is rejected while running instances still use the profile. diff --git a/openspec/changes/live-config-profile-refresh/implementation-plan.md b/openspec/changes/live-config-profile-refresh/implementation-plan.md new file mode 100644 index 0000000..1a907f9 --- /dev/null +++ b/openspec/changes/live-config-profile-refresh/implementation-plan.md @@ -0,0 +1,19 @@ +# Implementation Plan: Live Config Profile Refresh + +1. Standardize built-in tool user/group definitions and remove ownership-changing behavior for shared profile sources. +2. Add canonical profile storage and bind-mount compilation for profile directories and individual working-directory files. +3. Add compatibility/topology analysis, running-instance discovery, save-result schema, and deletion guard. +4. Wire desktop/mobile profile save results into immediate status/warning feedback. +5. Add unit, API, manifest/image, and frontend coverage; run quality gates and manual two-instance QA. + +## Delivery order + +1. Container-user compatibility +2. Canonical non-Git profile mounts +3. API and deletion semantics +4. UI feedback +5. Verification and commit + +## Deferred + +Git mount refresh and Git clone mutation require a commit-aware follow-up change. diff --git a/openspec/changes/live-config-profile-refresh/tasks.md b/openspec/changes/live-config-profile-refresh/tasks.md new file mode 100644 index 0000000..da73e4d --- /dev/null +++ b/openspec/changes/live-config-profile-refresh/tasks.md @@ -0,0 +1,32 @@ +# Live Config Profile Refresh — Tasks + +## Review Workload Forecast + +| Field | Value | +| --- | --- | +| Estimated changed lines | 500–750 | +| 400-line budget risk | High | +| Chained PRs recommended | Yes | +| Suggested split | Container-user standardization → canonical profile mounts → API/UI feedback → verification | +| Delivery strategy | feature-branch-chain | +| Chain strategy | feature-branch-chain | + +Decision needed before apply: No +Chained PRs recommended: Yes +Chain strategy: feature-branch-chain +400-line budget risk: High + +## Tasks + +- [ ] **RED/GREEN — shared container user:** standardize built-in tool images, manifests, and permission handling on one shared non-root user/group; detect incompatible legacy images. +- [ ] **RED/GREEN — canonical profile storage:** create canonical host-side directories/files per profile and mount them directly into compatible instances, without masking workspace mounts. +- [ ] **TRIANGULATE — writable sharing:** prove UI and container edits are shared across instances, with last-writer-wins overwrite warnings. +- [ ] **RED/GREEN — topology/API contract:** return restart-required or incompatible-permissions outcomes for paths that cannot mount live; defer Git mount mutation. +- [ ] **RED/GREEN — UI feedback:** show shared-working-copy, warning, restart-required, and incompatible-permissions results in desktop and mobile profile editors. +- [ ] **Verify:** run targeted backend/frontend tests, typecheck, lint, image/manifest checks, and manual multi-instance permission tests. + +## Verification Notes + +- Passed: frontend production build (`npm run build`), Python compilation for changed backend modules, and targeted LSP diagnostics. +- Skipped: backend pytest and Ruff; this environment has no project-managed Python runner, system Python lacks those packages, and the user declined system-package installation. +- Skipped: Docker/Compose and manual multi-instance checks; explicit Docker approval was not granted. diff --git a/openspec/changes/live-config-profile-refresh/test-plan.md b/openspec/changes/live-config-profile-refresh/test-plan.md new file mode 100644 index 0000000..99209ba --- /dev/null +++ b/openspec/changes/live-config-profile-refresh/test-plan.md @@ -0,0 +1,28 @@ +# Test Plan: Live Config Profile Refresh + +## Backend + +- Canonical file and directory paths are profile-scoped and reject traversal. +- Two compatible instances receive the same host mount source. +- A container-side file edit is visible through the profile read API and another instance mount. +- A profile save updates canonical content and returns overwrite warnings when applicable. +- Topology, environment, runtime, and legacy-instance changes return `restart_required`. +- Incompatible users return `incompatible_permissions`. +- Deleting a profile with running dependents is rejected with their instance identifiers. +- Git mount content is unchanged by this feature. + +## Container/image compatibility + +- Each built-in supported image uses the common non-root UID/GID. +- Generated manifest Dockerfiles and entrypoints retain that user and writable mount access. +- Existing instances are not mutated until restart/recreation. + +## Frontend + +- Desktop and mobile save flows display refreshed/shared-working-copy, overwrite-warning, restart-required, and incompatible-permissions results. + +## Manual QA + +- Open two compatible instances using one profile; edit a mounted file in one terminal and verify it in the other. +- Save a profile edit and verify both running instances see it without terminal disconnection. +- Verify profile deletion is blocked while either instance is running. diff --git a/tool-images/base.dockerfile b/tool-images/base.dockerfile index 2459699..406ce04 100644 --- a/tool-images/base.dockerfile +++ b/tool-images/base.dockerfile @@ -20,9 +20,10 @@ RUN apt-get update && apt-get install -y \ sudo \ && rm -rf /var/lib/apt/lists/* -# Create a non-root user and allow passwordless sudo so the startup -# permission fixer can adjust ownership of bind-mounted directories. -RUN useradd -m -s /bin/bash user \ +# Use the shared built-in UID/GID so writable Config Profile mounts can be +# shared by compatible instances without ownership changes. +RUN groupadd -g 1000 user \ + && useradd -m -u 1000 -g 1000 -s /bin/bash user \ && echo "user ALL=(ALL) NOPASSWD:ALL" > /etc/sudoers.d/user \ && chmod 0440 /etc/sudoers.d/user WORKDIR /home/user diff --git a/tool-images/code-server.dockerfile b/tool-images/code-server.dockerfile index ffa73e5..e6a991b 100644 --- a/tool-images/code-server.dockerfile +++ b/tool-images/code-server.dockerfile @@ -22,7 +22,11 @@ RUN curl -fsSL https://deb.nodesource.com/setup_20.x | bash - \ # Set up git RUN git config --global init.defaultBranch main -# Code-server runs as abc user by default +# The LinuxServer entrypoint maps abc to this shared UID/GID before starting +# code-server, so writable Config Profile mounts are compatible with other +# built-in tool containers. +ENV PUID=1000 \ + PGID=1000 USER abc EXPOSE 8443 \ No newline at end of file diff --git a/tool-images/jupyter.dockerfile b/tool-images/jupyter.dockerfile index c997ba1..bfd2313 100644 --- a/tool-images/jupyter.dockerfile +++ b/tool-images/jupyter.dockerfile @@ -17,7 +17,10 @@ RUN apt-get update && apt-get install -y \ # Set up git RUN git config --global init.defaultBranch main -# Switch back to jovyan user (default for scipy-notebook) -USER ${NB_UID} +# start-notebook.py maps jovyan to this shared UID/GID and drops privileges. +# Keep the image root at entrypoint time so that mapping can occur. +ENV NB_UID=1000 \ + NB_GID=1000 +USER root EXPOSE 8888 \ No newline at end of file diff --git a/tool-images/opencode.dockerfile b/tool-images/opencode.dockerfile index 1127ec6..566d8e9 100644 --- a/tool-images/opencode.dockerfile +++ b/tool-images/opencode.dockerfile @@ -27,8 +27,9 @@ RUN curl -fsSL https://deb.nodesource.com/setup_20.x | bash - \ # Install OpenCode RUN npm install -g opencode -# Create non-root user -RUN useradd -m -s /bin/bash user +# Use the shared built-in UID/GID for writable Config Profile mounts. +RUN groupadd -g 1000 user \ + && useradd -m -u 1000 -g 1000 -s /bin/bash user WORKDIR /home/user # Set up git diff --git a/tool-images/pi-agent.dockerfile b/tool-images/pi-agent.dockerfile index b71c117..d36b4cf 100644 --- a/tool-images/pi-agent.dockerfile +++ b/tool-images/pi-agent.dockerfile @@ -28,8 +28,9 @@ RUN curl -fsSL https://deb.nodesource.com/setup_20.x | bash - \ # Install Pi Coding Agent globally RUN npm install -g --ignore-scripts @earendil-works/pi-coding-agent -# Create non-root user -RUN useradd -m -s /bin/bash user +# Use the shared built-in UID/GID for writable Config Profile mounts. +RUN groupadd -g 1000 user \ + && useradd -m -u 1000 -g 1000 -s /bin/bash user WORKDIR /home/user # Set up git @@ -37,15 +38,11 @@ RUN git config --global init.defaultBranch main \ && git config --global user.email "dev@headquarter.local" \ && git config --global user.name "Developer" -# Create default tmux config -RUN printf '%s\n' 'set -g mouse on' 'set -g default-terminal "screen-256color"' > /home/user/.tmux.conf - -# Create default ranger config -RUN mkdir -p /home/user/.config/ranger \ - && printf '%s\n' 'set preview_files true' 'set use_preview_script true' > /home/user/.config/ranger/rc.conf - -# Set up Pi config directory -RUN mkdir -p /home/user/.pi/agent +# Create user-owned default configuration files. +RUN printf '%s\n' 'set -g mouse on' 'set -g default-terminal "screen-256color"' > /home/user/.tmux.conf \ + && mkdir -p /home/user/.config/ranger /home/user/.pi/agent \ + && printf '%s\n' 'set preview_files true' 'set use_preview_script true' > /home/user/.config/ranger/rc.conf \ + && chown -R user:user /home/user USER user