fix(config-profiles): synchronize shared mount working copies

Use canonical profile files and writable Git working copies so editor and container changes share one source. Require confirmation before destructive Git refreshes and overlay profile files without composite snapshots.
This commit is contained in:
2026-07-22 11:23:19 +02:00
parent 0d6c1926ae
commit 2247ec47c9
14 changed files with 285 additions and 159 deletions
+44 -4
View File
@@ -3,6 +3,7 @@
import logging
import os
import uuid
from pathlib import Path
from fastapi import APIRouter, Depends, HTTPException, Query, status
from sqlalchemy import select
@@ -10,6 +11,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.config import Settings
from src.models import ConfigProfile, ConfigProfileInclude, ToolInstance, UserConfig
from src.schemas.config import (
ConfigProfileCreate,
@@ -22,6 +24,7 @@ from src.schemas.config import (
)
from src.services.config.config_profile_resolver import (
ConfigProfileCycleError,
apply_resolved_profile,
resolve_profile,
resolved_profile_to_dict,
)
@@ -44,6 +47,31 @@ logger = logging.getLogger(__name__)
router = APIRouter(prefix="/config-profiles", tags=["config-profiles"])
def _canonical_profile_response(profile: ConfigProfile) -> dict:
"""Return profile data with edits from its shared working copy."""
response = profile_to_response(profile)
root = Path(Settings().instance_base_path) / "config-profiles" / str(profile.id)
def read_file(path: Path, fallback: str) -> str:
try:
return path.read_text() if path.is_file() else fallback
except OSError:
return fallback
response["files"] = {
relative_path: read_file(root / "files" / relative_path, content)
for relative_path, content in response["files"].items()
}
response["mounts"] = [dict(mount) for mount in response["mounts"]]
for mount in response["mounts"]:
mount_root = root / "mounts" / mount["target"].lstrip("/").replace("/", "_")
mount["files"] = {
relative_path: read_file(mount_root / relative_path, content)
for relative_path, content in mount.get("files", {}).items()
}
return response
async def _running_profile_outcomes(
session: AsyncSession, profile_id: uuid.UUID
) -> list[dict[str, str]]:
@@ -114,7 +142,7 @@ async def list_config_profiles(
result = await session.execute(query)
profiles = result.scalars().all()
return [profile_to_response(p) for p in profiles]
return [_canonical_profile_response(profile) for profile in profiles]
@router.post(
@@ -147,7 +175,7 @@ async def get_config_profile(
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN, detail="Not authorized"
)
return profile_to_response(profile)
return _canonical_profile_response(profile)
@router.put("/{profile_id}", response_model=ConfigProfileResponse)
@@ -169,7 +197,12 @@ async def update_config_profile(
)
profile = await update_profile(session, profile, data)
response = profile_to_response(profile)
resolved = await resolve_profile(session, profile.id)
apply_resolved_profile(
os.path.join(Settings().instance_base_path, "profile-refresh"),
resolved,
)
response = _canonical_profile_response(profile)
response["refresh_outcomes"] = await _running_profile_outcomes(session, profile.id)
logger.debug("Updated config profile %s", profile.id)
return response
@@ -178,10 +211,17 @@ async def update_config_profile(
@router.post("/{profile_id}/refresh-git-mounts")
async def refresh_profile_git_mounts(
profile_id: str,
confirm_destructive_refresh: bool = False,
current_user_id: uuid.UUID = Depends(get_current_user_id),
session: AsyncSession = Depends(get_db_session),
):
"""Refresh canonical Git mount sources used by running profile instances."""
"""Destructively refresh profile Git working copies used by instances."""
if not confirm_destructive_refresh:
raise HTTPException(
status_code=status.HTTP_409_CONFLICT,
detail="Confirm destructive Git refresh before replacing local edits",
)
profile = await get_profile_with_includes(session, uuid.UUID(profile_id))
if profile is None:
raise HTTPException(
+58 -104
View File
@@ -127,15 +127,20 @@ def _chown_staged_mounts(
uid: int,
gid: int,
) -> None:
"""Recursively chown instance-local mount sources to the container user.
"""Recursively chown writable profile and instance mount sources.
Profile copies, profile/Git composites, and SSH key mounts are created by
the API process. Their sources must be owned by the target container user
before Docker bind-mounts them into writable paths.
Canonical non-Git profile sources are shared by compatible instances, so
they must be writable by the container user rather than copied per
instance. Instance-local composites and SSH mounts remain supported.
"""
canonical_profile_root = os.path.join(
os.path.dirname(instance_dir), "config-profiles"
)
for vol in extra_volumes:
source = vol.get("source", "")
if not source or not source.startswith(instance_dir):
if not source or not (
source.startswith(instance_dir) or source.startswith(canonical_profile_root)
):
continue
_chown_path(source, uid, gid)
@@ -234,100 +239,38 @@ def _stack_profile_mounts_with_git_mounts(
git_mount_volumes: list[dict],
instance_dir: str,
) -> list[dict]:
"""Build instance-local composite mounts for overlapping profile and Git paths.
"""Overlay canonical profile files on Git directories without snapshots.
Docker applies one bind mount per target; it never merges their contents.
A composite therefore copies shared Git content first and profile content
second, so profile files extend (and intentionally override) the Git tree
without dirtying the shared clone.
An overlapping profile directory is expanded into individual child-file
mounts. Docker then mounts the Git working directory first and the more
specific canonical profile files last, preserving shared writable sources
instead of constructing an instance-local composite copy.
"""
records: list[tuple[str, dict]] = [
("profile", mount) for mount in profile_mounts
] + [("git", mount) for mount in git_mount_volumes]
components: list[list[int]] = []
remaining: set[int] = set(range(len(records)))
del instance_dir
result: list[dict] = list(git_mount_volumes)
while remaining:
component_indices: set[int] = {min(remaining)}
remaining.difference_update(component_indices)
pending = list(component_indices)
while pending:
current_index = pending.pop()
current_target = records[current_index][1].get("target", "")
for candidate_index in list(remaining):
candidate_target = records[candidate_index][1].get("target", "")
if _mounts_overlap(current_target, candidate_target):
remaining.remove(candidate_index)
component_indices.add(candidate_index)
pending.append(candidate_index)
components.append(sorted(component_indices))
result: list[dict] = []
for component_indexes in components:
component_records = [records[index] for index in component_indexes]
kinds = {kind for kind, _mount in component_records}
if kinds != {"profile", "git"}:
result.extend(mount for _kind, mount in component_records)
for profile_mount in profile_mounts:
source = profile_mount.get("source", "")
target = profile_mount.get("target", "")
overlaps_git = any(
_mounts_overlap(target, git_mount.get("target", ""))
for git_mount in git_mount_volumes
)
if not overlaps_git or not os.path.isdir(source):
result.append(profile_mount)
continue
targets = [
os.path.normpath(mount["target"]) for _kind, mount in component_records
]
composite_target = os.path.commonpath(targets)
if any(
not os.path.isdir(mount["source"])
and os.path.normpath(mount["target"]) == composite_target
for _kind, mount in component_records
):
composite_target = os.path.dirname(composite_target)
digest_input = "\0".join(
f"{kind}:{mount['source']}:{mount['target']}"
for kind, mount in component_records
)
composite_source = os.path.join(
instance_dir,
"mounts",
"composites",
hashlib.sha256(digest_input.encode()).hexdigest()[:16],
)
try:
if os.path.lexists(composite_source):
shutil.rmtree(composite_source)
os.makedirs(composite_source, exist_ok=True)
except OSError as exc:
raise RuntimeError(
f"Unable to prepare composite mount directory {composite_source}: {exc}"
) from exc
for kind in ("git", "profile"):
for record_kind, mount in component_records:
if record_kind != kind:
continue
relative_target = _relative_under(composite_target, mount["target"])
destination = (
composite_source
if relative_target == ""
else os.path.join(composite_source, relative_target or "")
for root, _dirs, files in os.walk(source):
for filename in files:
file_source = os.path.join(root, filename)
relative_path = os.path.relpath(file_source, source)
result.append(
{
**profile_mount,
"source": file_source,
"target": os.path.join(target, relative_path),
}
)
_copy_mount_source(mount["source"], destination)
result.append(
{
"source": composite_source,
"target": composite_target,
"type": "bind",
"readonly": all(
mount.get("readonly", False) for _kind, mount in component_records
),
}
)
logger.info(
"Composed %d profile/Git mounts at %s into %s",
len(component_records),
composite_target,
composite_source,
)
return result
@@ -629,8 +572,10 @@ async def resolve_single_git_mount(
volumes = resolve_git_mount_mappings(
repo_path, mappings, working_directory, home_dir
)
# Profile-scoped Git working copies are writable and shared by compatible
# containers. Explicit refresh replaces local edits with the remote ref.
for volume in volumes:
volume["readonly"] = True
volume["readonly"] = False
return volumes
@@ -675,10 +620,10 @@ def checkout_branch(repo_path: str, branch: str) -> bool:
def pull_repository_updates(repo_path: str, remote_url: str) -> None:
"""Pull latest updates from remote repository.
"""Replace a profile working copy with its current remote branch.
Used when starting a new container with an existing cloned repository
to ensure the latest code is mounted.
Git profile mounts are writable shared working copies. Refresh discards
local container/editor edits after fetching the remote baseline.
"""
import subprocess
@@ -692,15 +637,22 @@ def pull_repository_updates(repo_path: str, remote_url: str) -> None:
if result.returncode != 0:
raise RuntimeError(f"Failed to fetch updates: {result.stderr}")
# Pull changes for current branch
result = subprocess.run(
["git", "-C", repo_path, "pull", "origin"],
branch_result = subprocess.run(
["git", "-C", repo_path, "branch", "--show-current"],
capture_output=True,
text=True,
)
branch = branch_result.stdout.strip() if branch_result.returncode == 0 else ""
if not branch:
raise RuntimeError("Unable to determine Git working-copy branch")
result = subprocess.run(
["git", "-C", repo_path, "reset", "--hard", f"origin/{branch}"],
capture_output=True,
text=True,
)
if result.returncode != 0:
raise RuntimeError(f"Failed to pull updates: {result.stderr}")
raise RuntimeError(f"Failed to reset working copy: {result.stderr}")
def expand_glob_source(source_path: str, repo_path: str) -> list[str]:
@@ -1588,9 +1540,11 @@ async def start_tool_instance(
git_mount_volumes = await resolve_git_mounts(
session, resolved, instance_dir, working_directory, home_dir
)
staged_profile_mounts = _stage_profile_mounts(profile_mounts, instance_dir)
# Non-Git profile mounts bind directly to canonical profile
# storage so edits made by one compatible container are visible to
# every other container and the profile editor readback path.
composed_mounts = _stack_profile_mounts_with_git_mounts(
staged_profile_mounts,
profile_mounts,
git_mount_volumes,
instance_dir,
)
@@ -1601,7 +1555,7 @@ async def start_tool_instance(
instance.id,
len(profile_env),
len(profile_files),
len(staged_profile_mounts),
len(profile_mounts),
len(git_mount_volumes),
len(composed_mounts),
)
@@ -0,0 +1,23 @@
"""Tests for Config Profile refresh safety contracts."""
import uuid
from unittest.mock import AsyncMock
import pytest
from fastapi import HTTPException
from src.api.config.config_profiles import refresh_profile_git_mounts
@pytest.mark.unit
async def test_git_refresh_requires_destructive_confirmation() -> None:
"""The endpoint must not reset a writable working copy without consent."""
with pytest.raises(HTTPException) as exc_info:
await refresh_profile_git_mounts(
profile_id=str(uuid.uuid4()),
confirm_destructive_refresh=False,
current_user_id=uuid.uuid4(),
session=AsyncMock(),
)
assert exc_info.value.status_code == 409
+48 -27
View File
@@ -15,6 +15,7 @@ from src.services.tool.instance_service import (
clone_git_repo,
modify_compose_file,
prepare_manifest_instance,
pull_repository_updates,
)
@@ -97,6 +98,37 @@ class TestCloneGitRepo:
clone.assert_not_called()
pull.assert_called_once_with(str(repo_path), remote_url)
def test_refresh_resets_writable_working_copy_to_remote_branch(
self, monkeypatch
) -> None:
from src.services.tool import instance_service
results = [
MagicMock(returncode=0, stdout="", stderr=""),
MagicMock(returncode=0, stdout="main\n", stderr=""),
MagicMock(returncode=0, stdout="", stderr=""),
]
run = MagicMock(side_effect=results)
monkeypatch.setattr(instance_service.subprocess, "run", run)
pull_repository_updates("/work/profile-git", "https://example.test/repo.git")
assert run.call_args_list[0].args[0] == [
"git",
"-C",
"/work/profile-git",
"fetch",
"origin",
]
assert run.call_args_list[2].args[0] == [
"git",
"-C",
"/work/profile-git",
"reset",
"--hard",
"origin/main",
]
def test_replaces_incomplete_clone_before_retry(
self, monkeypatch, tmp_path
) -> None:
@@ -191,17 +223,12 @@ class TestStackProfileMountsWithGitMounts:
str(instance_dir),
)
assert len(result) == 1
composite = result[0]
assert composite["target"] == "/home/user/.pi"
assert composite["source"].startswith(str(instance_dir))
assert not (tmp_path / "git" / "repo-clone" / "settings.json").exists()
assert (
tmp_path / "git" / "repo-clone" / "existing.txt"
).read_text() == "from git"
assert (tmp_path / "instance" / "mounts" / "composites").exists()
assert (Path(composite["source"]) / "existing.txt").read_text() == "from git"
assert (Path(composite["source"]) / "settings.json").read_text() == "{}"
assert len(result) == 2
assert result[0]["source"] == str(git_source)
assert result[0]["target"] == "/home/user/.pi"
assert result[1]["source"].endswith("/settings.json")
assert result[1]["target"] == "/home/user/.pi/settings.json"
assert not (git_source / "settings.json").exists()
def test_descendant_profile_mount_extends_git_root(self, tmp_path) -> None:
"""Nested targets are composed at the Git root, preserving siblings."""
@@ -223,11 +250,9 @@ class TestStackProfileMountsWithGitMounts:
str(instance_dir),
)
assert len(result) == 1
composite = Path(result[0]["source"])
assert result[0]["target"] == "/home/user/.pi"
assert (composite / "README").read_text() == "repo"
assert (composite / "agent" / "settings.json").read_text() == "x"
assert len(result) == 2
assert result[0]["source"] == str(git_source)
assert result[1]["target"] == "/home/user/.pi/agent/settings.json"
assert not (git_source / "agent").exists()
def test_file_profile_mount_extends_git_root(self, tmp_path) -> None:
@@ -254,11 +279,9 @@ class TestStackProfileMountsWithGitMounts:
str(instance_dir),
)
assert len(result) == 1
composite = Path(result[0]["source"])
assert result[0]["target"] == "/home/user/.pi"
assert (composite / "README").read_text() == "repo"
assert (composite / "settings.json").read_text() == "{}"
assert len(result) == 2
assert result[0]["source"] == str(git_source)
assert result[1]["target"] == "/home/user/.pi/settings.json"
def test_parent_profile_mount_extends_nested_git_mount(self, tmp_path) -> None:
"""A profile parent mount keeps Git content and its own sibling files."""
@@ -279,11 +302,9 @@ class TestStackProfileMountsWithGitMounts:
str(instance_dir),
)
assert len(result) == 1
composite = Path(result[0]["source"])
assert result[0]["target"] == "/home/user"
assert (composite / "config.toml").read_text() == "profile"
assert (composite / ".pi" / "plugin.toml").read_text() == "git"
assert len(result) == 2
assert result[0]["source"] == str(git_source)
assert result[1]["target"] == "/home/user/config.toml"
def test_non_overlapping_mounts_remain_separate(self, tmp_path) -> None:
"""Unrelated profile and Git mounts retain their independent sources."""
@@ -303,7 +324,7 @@ class TestStackProfileMountsWithGitMounts:
_stack_profile_mounts_with_git_mounts(
profile_mounts, git_mounts, str(instance_dir)
)
== profile_mounts + git_mounts
== git_mounts + profile_mounts
)