diff --git a/.serena/.gitignore b/.serena/.gitignore new file mode 100644 index 0000000..2e510af --- /dev/null +++ b/.serena/.gitignore @@ -0,0 +1,2 @@ +/cache +/project.local.yml diff --git a/.serena/project.yml b/.serena/project.yml new file mode 100644 index 0000000..1da7c60 --- /dev/null +++ b/.serena/project.yml @@ -0,0 +1,159 @@ +# the name by which the project can be referenced within Serena/when chatting with the LLM. +project_name: "issue-160" + +# list of languages for which language servers are started (LSP backend only); choose from: +# ada al angular ansible bash +# bsl clojure cpp cpp_ccls crystal +# csharp csharp_omnisharp cue dart elixir +# elm erlang fortran fsharp gdscript +# go groovy haskell haxe hlsl +# html java json julia kotlin +# latex lean4 lua luau markdown +# matlab msl nix ocaml pascal +# perl php php_phpactor php_phpantom powershell +# python python_jedi python_pyrefly python_ty r +# rego ruby ruby_solargraph rust scala +# scss solidity svelte swift systemverilog +# terraform toml typescript typescript_vts vue +# yaml zig +# (This list may be outdated; generated with scripts/print_language_list.py; +# For the current list, see values of Language enum here: +# https://github.com/oraios/serena/blob/main/src/solidlsp/ls_config.py) +# For some languages, there are alternative language servers, e.g. csharp_omnisharp, ruby_solargraph.) +# Note: +# - For C, use cpp +# - For JavaScript, use typescript +# - For Angular projects, use angular (subsumes typescript+html; requires `npm install` in the project root) +# - For Svelte projects, use svelte (subsumes typescript/javascript for .svelte projects; requires npm) +# - For SCSS / Sass / plain CSS, use scss (some-sass-language-server handles all three) +# - For Free Pascal/Lazarus, use pascal +# Special requirements: +# Some languages require additional setup/installations. +# See here for details: https://oraios.github.io/serena/01-about/020_programming-languages.html#language-servers +# When using multiple languages, the first language server that supports a given file will be used for that file. +# The first language is the default language and the respective language server will be used as a fallback. +# Note that when using the JetBrains backend, language servers are not used and this list is correspondingly ignored. +languages: +- python + +# the encoding used by text files in the project +# For a list of possible encodings, see https://docs.python.org/3.11/library/codecs.html#standard-encodings +encoding: "utf-8" + +# optional shell command to run before the language backend (LSP or JetBrains) is initialised. +# the command runs in the project root directory and is only executed if the project is trusted +# (see trusted_project_path_patterns in the global configuration). +# serena waits for the command to exit: a non-zero exit code is logged as an error but does not +# abort activation. a per-project timeout (activation_command_timeout, default 180s) is the safety +# backstop for non-terminating commands; on expiry the process is killed and activation continues. +# example: activation_command: "npx nx run-many -t build" +activation_command: + +# maximum time in seconds to wait for activation_command to complete before killing it (default 180s). +# must be a positive number. +activation_command_timeout: 180 + +# line ending convention to use when writing source files. +# Possible values: unset (use global setting), "lf", "crlf", or "native" (platform default) +# This does not affect Serena's own files (e.g. memories and configuration files), which always use native line endings. +line_ending: + +# The language backend to use for this project. +# If not set, the global setting from serena_config.yml is used. +# Valid values: LSP, JetBrains +# Note: the backend is fixed at startup. If a project with a different backend +# is activated post-init, an error will be returned. +language_backend: + +# whether to use project's .gitignore files to ignore files +ignore_all_files_in_gitignore: true + +# advanced configuration option allowing to configure language server-specific options. +# Maps the language key to the options. +# The settings are considered only if the project is trusted (see global configuration to define trusted projects). +# See https://oraios.github.io/serena/02-usage/050_configuration.html#language-server-specific-settings +ls_specific_settings: {} + +# list of workspace folder paths (LSP backend only). +# These folders will be used to build up Serena's symbol index. +# Paths must be within the project root and should thus be relative to the project root. +# Furthermore, the paths should not be filtered by ignore settings. +# Default setting: The entire project root folder (".") is considered. +# In (large) monorepos, this can be used to index only subfolders of the project root, e.g. +# ls_workspace_folders: +# - "./subproject1" +# - "./subproject2" +ls_workspace_folders: ["."] + +# list of additional workspace folder paths for cross-package reference support. +# Paths can be absolute or relative to the project root. +# Each folder is registered as an LSP workspace folder, enabling language servers to discover +# symbols and references across package boundaries, but these folders are not indexed by Serena, +# i.e. the respective symbols will not be found using Serena's symbol search tools. +# Example: +# additional_workspace_folders: +# - ../sibling-package +# - ../shared-lib +ls_additional_workspace_folders: [] + +# list of additional paths to ignore in this project. +# Same syntax as gitignore, so you can use * and **. +# Note: global ignored_paths from serena_config.yml are also applied additively. +ignored_paths: [] + +# whether the project is in read-only mode +# If set to true, all editing tools will be disabled and attempts to use them will result in an error +# Added on 2025-04-18 +read_only: false + +# list of tool names to exclude. +# This extends the existing exclusions (e.g. from the global configuration) +# Find the list of tools here: https://oraios.github.io/serena/01-about/035_tools.html +excluded_tools: [] + +# list of tools to include that would otherwise be disabled (particularly optional tools that are disabled by default). +# This extends the existing inclusions (e.g. from the global configuration). +# Find the list of tools here: https://oraios.github.io/serena/01-about/035_tools.html +included_optional_tools: [] + +# fixed set of tools to use as the base tool set (if non-empty), replacing Serena's default set of tools. +# This cannot be combined with non-empty excluded_tools or included_optional_tools. +# Find the list of tools here: https://oraios.github.io/serena/01-about/035_tools.html +fixed_tools: [] + +# list of mode names that are to be activated by default, overriding the setting in the global configuration. +# The full set of modes to be activated is base_modes (from global config) + default_modes + added_modes. +# If the setting is undefined/empty, the default_modes from the global configuration (serena_config.yml) apply. +# Otherwise, this overrides the setting from the global configuration (serena_config.yml). +# Therefore, you can set this to [] if you do not want the default modes defined in the global config to apply +# for this project. +# This setting can, in turn, be overridden by CLI parameters (--mode). +# See https://oraios.github.io/serena/02-usage/050_configuration.html#modes +default_modes: + +# list of mode names to be activated additionally for this project, e.g. ["query-projects"] +# The full set of modes to be activated is base_modes (from global config) + default_modes + added_modes. +# See https://oraios.github.io/serena/02-usage/050_configuration.html#modes +added_modes: + +# initial prompt for the project. It will always be given to the LLM upon activating the project +# (contrary to the memories, which are loaded on demand). +initial_prompt: "" + +# time budget (seconds) per tool call for the retrieval of additional symbol information +# such as docstrings or parameter information. +# This overrides the corresponding setting in the global configuration; see the documentation there. +# If null or missing, use the setting from the global configuration. +symbol_info_budget: + +# list of regex patterns which, when matched, mark a memory entry as read‑only. +# Extends the list from the global configuration, merging the two lists. +read_only_memory_patterns: [] + +# list of regex patterns for memories to completely ignore. +# Matching memories will not appear in list_memories or activate_project output +# and cannot be accessed via read_memory or write_memory. +# To access ignored memory files, use the read_file tool on the raw file path. +# Extends the list from the global configuration, merging the two lists. +# Example: ["_archive/.*", "_episodes/.*"] +ignored_memory_patterns: [] diff --git a/agents.md b/agents.md index 693ebae..ef8052e 100644 --- a/agents.md +++ b/agents.md @@ -48,6 +48,9 @@ shard_core/ util/ → Shared utilities async_util.py BackgroundTask, PeriodicTask, CronTask signals.py Blinker signal definitions + subprocess.py Async subprocess runner; app_compose_command() pins every app + compose call to /docker-compose.yml and project + — never rely on cwd, compose walks up to the core stack ``` ## Commands diff --git a/shard_core/service/app_tools.py b/shard_core/service/app_tools.py index bdd414e..b7fdc8c 100644 --- a/shard_core/service/app_tools.py +++ b/shard_core/service/app_tools.py @@ -17,16 +17,18 @@ from shard_core.settings import settings from shard_core.util import signals from shard_core.util.misc import throttle -from shard_core.util.subprocess import subprocess, SubprocessError, compose_command +from shard_core.util.subprocess import subprocess, SubprocessError, app_compose_command log = logging.getLogger(__name__) +def _app_compose(name: str) -> tuple[str, ...]: + return app_compose_command(get_installed_apps_path() / name) + + async def docker_create_app_containers(name: str): log.debug(f"creating containers for app {name}") - await subprocess( - *compose_command(), "up", "--no-start", cwd=get_installed_apps_path() / name - ) + await subprocess(*_app_compose(name), "up", "--no-start") @throttle(5) @@ -43,30 +45,20 @@ async def docker_start_app(name: str): if app_status in [Status.STOPPED, Status.RUNNING, Status.DOWN]: log.debug(f"starting app {name=}") try: - await subprocess( - *compose_command(), "up", "-d", cwd=get_installed_apps_path() / name - ) + await subprocess(*_app_compose(name), "up", "-d") except SubprocessError as e: if "network" in str(e) and "not found" in str(e): log.warning( f"stale network reference for app {name=}, recreating containers" ) - await subprocess( - *compose_command(), "down", cwd=get_installed_apps_path() / name - ) - await subprocess( - *compose_command(), "up", "-d", cwd=get_installed_apps_path() / name - ) + await subprocess(*_app_compose(name), "down") + await subprocess(*_app_compose(name), "up", "-d") elif "Conflict" in str(e) and "already in use" in str(e): log.warning( f"stale containers for app {name=}, removing and recreating" ) - await subprocess( - *compose_command(), "down", cwd=get_installed_apps_path() / name - ) - await subprocess( - *compose_command(), "up", "-d", cwd=get_installed_apps_path() / name - ) + await subprocess(*_app_compose(name), "down") + await subprocess(*_app_compose(name), "up", "-d") else: raise async with db_conn() as conn: @@ -84,9 +76,7 @@ async def docker_pause_app(name: str): if app_status == Status.RUNNING: log.debug(f"pausing app {name=}") pause_started = time.monotonic() - await subprocess( - *compose_command(), "pause", cwd=get_installed_apps_path() / name - ) + await subprocess(*_app_compose(name), "pause") await memory_pressure.reclaim_compose_stack(name) pause_metrics.record_pause_latency((time.monotonic() - pause_started) * 1000) pause_metrics.record_app_transition(name, Status.RUNNING, Status.PAUSED) @@ -104,9 +94,7 @@ async def docker_unpause_app(name: str): if app_status == Status.PAUSED: log.debug(f"unpausing app {name=}") unpause_started = time.monotonic() - await subprocess( - *compose_command(), "unpause", cwd=get_installed_apps_path() / name - ) + await subprocess(*_app_compose(name), "unpause") pause_metrics.record_unpause_latency( (time.monotonic() - unpause_started) * 1000 ) @@ -125,12 +113,8 @@ async def docker_stop_app(name: str, set_status: bool = True): if app_status in [Status.RUNNING, Status.PAUSED, Status.UNINSTALLING]: if app_status == Status.PAUSED: # a frozen container cannot be stopped — unfreeze first - await subprocess( - *compose_command(), "unpause", cwd=get_installed_apps_path() / name - ) - await subprocess( - *compose_command(), "stop", cwd=get_installed_apps_path() / name - ) + await subprocess(*_app_compose(name), "unpause") + await subprocess(*_app_compose(name), "stop") if set_status: pause_metrics.record_app_transition( name, Status(app_status), Status.STOPPED @@ -150,12 +134,8 @@ async def docker_shutdown_app(name: str, set_status: bool = True, force: bool = if app_status == Status.PAUSED: # only reachable with force=True (process shutdown) — unfreeze so # compose down can stop and remove the containers - await subprocess( - *compose_command(), "unpause", cwd=get_installed_apps_path() / name - ) - await subprocess( - *compose_command(), "down", cwd=get_installed_apps_path() / name - ) + await subprocess(*_app_compose(name), "unpause") + await subprocess(*_app_compose(name), "down") if set_status: async with db_conn() as conn: await db_installed_apps.update_status(conn, name, Status.DOWN) diff --git a/shard_core/service/memory_pressure.py b/shard_core/service/memory_pressure.py index 2f8ab45..5d1a621 100644 --- a/shard_core/service/memory_pressure.py +++ b/shard_core/service/memory_pressure.py @@ -5,7 +5,7 @@ from pathlib import Path from shard_core.settings import settings -from shard_core.util.subprocess import subprocess, compose_command +from shard_core.util.subprocess import subprocess, app_compose_command log = logging.getLogger(__name__) @@ -32,7 +32,7 @@ async def reclaim_compose_stack(app_name: str): """Write each container's current RSS to its cgroup memory.reclaim, proactively paging the frozen processes' anonymous pages out to swap.""" app_path = Path(settings().path_root) / "core" / "installed_apps" / app_name - stdout = await subprocess(*compose_command(), "ps", "-q", cwd=app_path) + stdout = await subprocess(*app_compose_command(app_path), "ps", "-q") container_ids = [line.strip() for line in stdout.splitlines() if line.strip()] for container_id in container_ids: # The memory.reclaim write blocks while the kernel pages out — run it diff --git a/shard_core/util/subprocess.py b/shard_core/util/subprocess.py index da28124..c37d769 100644 --- a/shard_core/util/subprocess.py +++ b/shard_core/util/subprocess.py @@ -1,10 +1,19 @@ import asyncio import functools import logging +import re import subprocess as _sp +from pathlib import Path log = logging.getLogger(__name__) +COMPOSE_FILE_NAME = "docker-compose.yml" + +# The core stack's compose project — an app command must never resolve to it. +CORE_PROJECT_NAME = "core" + +_PROJECT_NAME_INVALID_CHARS_RE = re.compile(r"[^a-z0-9_-]") + @functools.lru_cache(maxsize=1) def _detect_compose_command() -> tuple[str, ...]: @@ -25,6 +34,40 @@ def compose_command() -> tuple[str, ...]: return _detect_compose_command() +def normalize_project_name(name: str) -> str: + """Mirror compose's own directory-name -> project-name normalization: + lowercase, drop everything outside [a-z0-9_-], strip leading _ and -.""" + return _PROJECT_NAME_INVALID_CHARS_RE.sub("", name.lower()).lstrip("_-") + + +def app_compose_command(app_dir: Path) -> tuple[str, ...]: + """Build a compose command pinned to one app's compose file and project. + + Without -f/-p, compose derives both from the working directory and walks up + the tree when no compose file is there — from an app dir that lands on the + core stack, so an app `stop`/`down` takes the whole shard offline. + """ + compose_file = app_dir / COMPOSE_FILE_NAME + if not compose_file.is_file(): + raise ComposeFileNotFound(compose_file) + project_name = normalize_project_name(app_dir.name) + if not project_name: + raise ComposeProjectNotAllowed(f"app dir {app_dir} has no valid project name") + if project_name == CORE_PROJECT_NAME: + raise ComposeProjectNotAllowed( + f"app dir {app_dir} resolves to the core compose project" + ) + return ( + *compose_command(), + "-f", + str(compose_file), + "-p", + project_name, + "--project-directory", + str(app_dir), + ) + + async def subprocess(*args, cwd=None): process = await asyncio.create_subprocess_exec( *args, cwd=cwd, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE @@ -50,3 +93,11 @@ async def subprocess(*args, cwd=None): class SubprocessError(Exception): pass + + +class ComposeFileNotFound(Exception): + pass + + +class ComposeProjectNotAllowed(Exception): + pass diff --git a/tests/test_app_compose_pinning.py b/tests/test_app_compose_pinning.py new file mode 100644 index 0000000..8b2793b --- /dev/null +++ b/tests/test_app_compose_pinning.py @@ -0,0 +1,150 @@ +"""Compose commands for an app must never resolve to the core stack. + +The app dirs live under the core dir, so a cwd-based compose invocation with a +missing app compose file walks up to /core/docker-compose.yml and operates on +project "core" — stopping shard_core itself (issue #160). +""" + +from pathlib import Path +from unittest.mock import AsyncMock, patch + +import pytest + +from shard_core.data_model.app_meta import InstalledApp, Status +from shard_core.database.connection import db_conn +from shard_core.database import installed_apps as db_installed_apps +from shard_core.service import app_tools +from shard_core.util.subprocess import ( + ComposeFileNotFound, + ComposeProjectNotAllowed, + app_compose_command, + normalize_project_name, +) +from tests.conftest import settings_override + +COMPOSE_FILE_CONTENT = "services:\n a:\n image: nginx:alpine\n" + + +def _app_dir(root: Path, name: str, with_compose_file: bool = True) -> Path: + app_dir = root / "core" / "installed_apps" / name + app_dir.mkdir(parents=True) + if with_compose_file: + (app_dir / "docker-compose.yml").write_text(COMPOSE_FILE_CONTENT) + return app_dir + + +def test_app_compose_command_pins_file_and_project(tmp_path): + app_dir = _app_dir(tmp_path, "filebrowser") + + command = app_compose_command(app_dir) + + assert command[-6:] == ( + "-f", + str(app_dir / "docker-compose.yml"), + "-p", + "filebrowser", + "--project-directory", + str(app_dir), + ) + + +def test_app_compose_command_without_compose_file_raises(tmp_path): + app_dir = _app_dir(tmp_path, "filebrowser", with_compose_file=False) + + with pytest.raises(ComposeFileNotFound): + app_compose_command(app_dir) + + +def test_app_compose_command_rejects_core_project(tmp_path): + core_dir = tmp_path / "core" + core_dir.mkdir() + (core_dir / "docker-compose.yml").write_text(COMPOSE_FILE_CONTENT) + + with pytest.raises(ComposeProjectNotAllowed): + app_compose_command(core_dir) + + +def test_app_compose_command_rejects_dir_without_valid_project_name(tmp_path): + app_dir = _app_dir(tmp_path, "...") + + with pytest.raises(ComposeProjectNotAllowed): + app_compose_command(app_dir) + + +@pytest.mark.parametrize( + "dir_name, expected", + [ + ("filebrowser", "filebrowser"), + ("paperless-ngx", "paperless-ngx"), + ("always_on", "always_on"), + # verified against `docker compose config` (v5.0.2): lowercase, drop + # everything outside [a-z0-9_-], strip leading _ and - + ("My_App.v2-x", "my_appv2-x"), + ("_-Foo", "foo"), + ], +) +def test_normalize_project_name_matches_compose(dir_name, expected): + assert normalize_project_name(dir_name) == expected + + +@pytest.fixture +def subprocess_mock(): + with patch.object(app_tools, "subprocess", new=AsyncMock()) as mock: + yield mock + + +@pytest.fixture(autouse=True) +def reset_start_throttle(): + """docker_start_app's @throttle(5) is global across apps and tests — a start + triggered by an earlier test would silently drop our call.""" + wrapper = app_tools.docker_start_app + cell = wrapper.__closure__[wrapper.__code__.co_freevars.index("last_call")] + cell.cell_contents = None + + +async def _insert_app(name: str, status: Status): + async with db_conn() as conn: + await db_installed_apps.insert( + conn, InstalledApp(name=name, status=status).model_dump() + ) + + +@pytest.mark.parametrize( + "operation, status", + [ + (app_tools.docker_create_app_containers, Status.STOPPED), + (app_tools.docker_start_app, Status.STOPPED), + (app_tools.docker_pause_app, Status.RUNNING), + (app_tools.docker_unpause_app, Status.PAUSED), + (app_tools.docker_stop_app, Status.RUNNING), + (app_tools.docker_shutdown_app, Status.STOPPED), + ], +) +async def test_app_operation_without_compose_file_never_runs_compose( + db, tmp_path, subprocess_mock, operation, status +): + _app_dir(tmp_path, "brokenapp", with_compose_file=False) + await _insert_app("brokenapp", status) + + with settings_override({"path_root": str(tmp_path)}): + with pytest.raises(ComposeFileNotFound): + await operation("brokenapp") + + subprocess_mock.assert_not_called() + + +async def test_app_operation_with_compose_file_pins_the_app_project( + db, tmp_path, subprocess_mock +): + app_dir = _app_dir(tmp_path, "brokenapp") + await _insert_app("brokenapp", Status.RUNNING) + + with settings_override({"path_root": str(tmp_path)}): + await app_tools.docker_stop_app("brokenapp") + + command = subprocess_mock.await_args.args + assert command[-1] == "stop" + assert "-p" in command and command[command.index("-p") + 1] == "brokenapp" + assert "-f" in command and command[command.index("-f") + 1] == str( + app_dir / "docker-compose.yml" + ) diff --git a/tests/test_memory_pressure.py b/tests/test_memory_pressure.py index c4e9147..fbb4e11 100644 --- a/tests/test_memory_pressure.py +++ b/tests/test_memory_pressure.py @@ -6,6 +6,17 @@ import pytest from shard_core.service import memory_pressure +from shard_core.settings import settings + + +def _make_app_compose_file(app_name: str) -> Path: + """reclaim only runs right after a successful `compose pause`, so the app's + compose file is always there — app_compose_command refuses to run without it.""" + app_dir = Path(settings().path_root) / "core" / "installed_apps" / app_name + app_dir.mkdir(parents=True, exist_ok=True) + (app_dir / "docker-compose.yml").write_text("services:\n a:\n image: nginx\n") + return app_dir + PSI_SAMPLE = """some avg10=12.34 avg60=5.67 avg300=1.23 total=123456 full avg10=3.21 avg60=1.11 avg300=0.42 total=65432 @@ -108,6 +119,7 @@ def _raise_eperm(self, *args, **kwargs): async def test_reclaim_compose_stack_reclaims_each_container(fake_cgroup_root): cgroup_a = _make_cgroup(fake_cgroup_root, "docker/aaa", 100) cgroup_b = _make_cgroup(fake_cgroup_root, "docker/bbb", 200) + _make_app_compose_file("someapp") with patch.object( memory_pressure, "subprocess", new=AsyncMock(return_value="aaa\nbbb\n\n") ): diff --git a/uv.lock b/uv.lock index 98a31e0..89d036d 100644 --- a/uv.lock +++ b/uv.lock @@ -1813,7 +1813,7 @@ wheels = [ [[package]] name = "shard-core" -version = "0.39.5" +version = "0.40.0" source = { editable = "." } dependencies = [ { name = "aiofiles" },