Compare commits
6 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 5331a0f110 | |||
| b995521e22 | |||
| 5610017f50 | |||
| 61e7d68d71 | |||
| 2247ec47c9 | |||
| 0d6c1926ae |
@@ -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(
|
||||
|
||||
@@ -515,7 +515,13 @@ def apply_resolved_profile(
|
||||
files_dir = profile_dir / "files"
|
||||
mounts_dir = profile_dir / "mounts"
|
||||
|
||||
def write_canonical_file(root: Path, relative_path: str, content: str) -> Path | None:
|
||||
def write_canonical_file(
|
||||
root: Path,
|
||||
relative_path: str,
|
||||
content: str,
|
||||
*,
|
||||
preserve_inode: bool = False,
|
||||
) -> Path | None:
|
||||
path = root / relative_path
|
||||
try:
|
||||
path.resolve().relative_to(root.resolve())
|
||||
@@ -523,6 +529,12 @@ def apply_resolved_profile(
|
||||
logger.warning("Profile file path escapes canonical storage: %s", relative_path)
|
||||
return None
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
if preserve_inode and path.is_file():
|
||||
# A file bind mount follows its inode, not its directory entry.
|
||||
# Replacing this path would leave a running container attached to
|
||||
# the old inode, so overwrite the existing file in place.
|
||||
path.write_text(content, encoding="utf-8")
|
||||
return path
|
||||
with tempfile.NamedTemporaryFile(
|
||||
mode="w", encoding="utf-8", dir=path.parent, delete=False
|
||||
) as temporary_file:
|
||||
@@ -534,7 +546,9 @@ def apply_resolved_profile(
|
||||
# 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)
|
||||
canonical_file = write_canonical_file(
|
||||
files_dir, file_path, content, preserve_inode=True
|
||||
)
|
||||
if canonical_file is None:
|
||||
continue
|
||||
volume_mounts.append(
|
||||
@@ -555,7 +569,7 @@ def apply_resolved_profile(
|
||||
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():
|
||||
write_canonical_file(mount_dir, file_path, content)
|
||||
write_canonical_file(mount_dir, file_path, content, preserve_inode=True)
|
||||
|
||||
volume_mounts.append(
|
||||
{
|
||||
|
||||
@@ -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
|
||||
@@ -36,7 +36,7 @@ class TestMergeFunctions:
|
||||
|
||||
def test_merge_env_vars_tracks_overrides(self) -> None:
|
||||
"""Test that env var overrides are tracked."""
|
||||
overrides = {}
|
||||
overrides: dict[str, str] = {}
|
||||
_merge_env_vars(
|
||||
{"A": "1"},
|
||||
{"A": "2"},
|
||||
@@ -93,7 +93,7 @@ class TestMergeFunctions:
|
||||
"""Test that mount mode conflicts are resolved (later wins)."""
|
||||
from src.services.config.config_profile_resolver import ResolvedMount
|
||||
|
||||
overrides = {}
|
||||
overrides: dict[str, str] = {}
|
||||
result = _merge_mounts(
|
||||
{"/app": ResolvedMount(target="/app", mode="rw", files={})},
|
||||
[{"target": "/app", "mode": "ro", "files": {}}],
|
||||
@@ -562,6 +562,59 @@ class TestApplyResolvedProfile:
|
||||
]
|
||||
assert canonical_file.read_text() == "setting = true"
|
||||
|
||||
def test_top_level_file_update_preserves_bind_mount_inode(self, tmp_path) -> None:
|
||||
"""An individually bind-mounted file must update in place."""
|
||||
profile_id = uuid.uuid4()
|
||||
instance_root = tmp_path / "instances"
|
||||
resolved = ResolvedProfile(
|
||||
profile_id=profile_id,
|
||||
profile_name="test",
|
||||
files={"settings.toml": "value = 1"},
|
||||
)
|
||||
|
||||
apply_resolved_profile(str(instance_root / "instance-a"), resolved)
|
||||
canonical_file = (
|
||||
instance_root / "config-profiles" / str(profile_id) / "files" / "settings.toml"
|
||||
)
|
||||
original_inode = canonical_file.stat().st_ino
|
||||
|
||||
resolved.files["settings.toml"] = "value = 2"
|
||||
apply_resolved_profile(str(instance_root / "instance-a"), resolved)
|
||||
|
||||
assert canonical_file.stat().st_ino == original_inode
|
||||
assert canonical_file.read_text() == "value = 2"
|
||||
|
||||
def test_mounted_file_update_preserves_bind_mount_inode(self, tmp_path) -> None:
|
||||
"""A file inside a profile directory mount must update in place."""
|
||||
profile_id = uuid.uuid4()
|
||||
instance_root = tmp_path / "instances"
|
||||
resolved = ResolvedProfile(
|
||||
profile_id=profile_id,
|
||||
profile_name="test",
|
||||
mounts={
|
||||
"/etc/tool": ResolvedMount(
|
||||
target="/etc/tool", mode="rw", files={"settings.toml": "value = 1"}
|
||||
)
|
||||
},
|
||||
)
|
||||
|
||||
apply_resolved_profile(str(instance_root / "instance-a"), resolved)
|
||||
canonical_file = (
|
||||
instance_root
|
||||
/ "config-profiles"
|
||||
/ str(profile_id)
|
||||
/ "mounts"
|
||||
/ "etc_tool"
|
||||
/ "settings.toml"
|
||||
)
|
||||
original_inode = canonical_file.stat().st_ino
|
||||
|
||||
resolved.mounts["/etc/tool"].files["settings.toml"] = "value = 2"
|
||||
apply_resolved_profile(str(instance_root / "instance-a"), resolved)
|
||||
|
||||
assert canonical_file.stat().st_ino == original_inode
|
||||
assert canonical_file.read_text() == "value = 2"
|
||||
|
||||
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()
|
||||
|
||||
@@ -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
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const mockPost = vi.fn();
|
||||
|
||||
vi.mock("./client", () => ({
|
||||
apiClient: {
|
||||
post: (...args: unknown[]) => mockPost(...args),
|
||||
},
|
||||
}));
|
||||
|
||||
import { refreshConfigProfileGitMounts } from "./config-profiles";
|
||||
|
||||
describe("refreshConfigProfileGitMounts", () => {
|
||||
beforeEach(() => {
|
||||
mockPost.mockReset();
|
||||
});
|
||||
|
||||
it("confirms the destructive refresh at the API boundary", async () => {
|
||||
mockPost.mockResolvedValue({ data: { refresh_outcomes: [] } });
|
||||
|
||||
await expect(refreshConfigProfileGitMounts("profile-1")).resolves.toEqual({
|
||||
refresh_outcomes: [],
|
||||
});
|
||||
|
||||
expect(mockPost).toHaveBeenCalledWith(
|
||||
"/config-profiles/profile-1/refresh-git-mounts",
|
||||
undefined,
|
||||
{ params: { confirm_destructive_refresh: true } },
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -2,7 +2,11 @@ import { apiClient } from "./client";
|
||||
|
||||
export interface ConfigProfileRefreshOutcome {
|
||||
instance_id: string;
|
||||
status: "compatible" | "refreshed" | "restart_required" | "incompatible_permissions";
|
||||
status:
|
||||
| "compatible"
|
||||
| "refreshed"
|
||||
| "restart_required"
|
||||
| "incompatible_permissions";
|
||||
reason?: string;
|
||||
}
|
||||
|
||||
@@ -150,7 +154,9 @@ export const refreshConfigProfileGitMounts = async (
|
||||
): Promise<{ refresh_outcomes: ConfigProfileRefreshOutcome[] }> => {
|
||||
const response = await apiClient.post<{
|
||||
refresh_outcomes: ConfigProfileRefreshOutcome[];
|
||||
}>(`/config-profiles/${id}/refresh-git-mounts`);
|
||||
}>(`/config-profiles/${id}/refresh-git-mounts`, undefined, {
|
||||
params: { confirm_destructive_refresh: true },
|
||||
});
|
||||
return response.data;
|
||||
};
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ interface Props {
|
||||
previewData: ResolvedProfile | null;
|
||||
previewingId: string | null;
|
||||
isRefreshingGitMounts: boolean;
|
||||
isReloadingWorkingCopy: boolean;
|
||||
projects: ProjectWithRepos[];
|
||||
toolTypes: ToolType[];
|
||||
availableProfiles: ConfigProfile[];
|
||||
@@ -24,6 +25,7 @@ interface Props {
|
||||
onSubmit: (e?: React.FormEvent) => void;
|
||||
onReset: () => void;
|
||||
onPreview: () => void;
|
||||
onReloadWorkingCopy: () => void;
|
||||
onRefreshGitMounts: () => void;
|
||||
onAddInclude: (id: string) => void;
|
||||
onRemoveInclude: (index: number) => void;
|
||||
@@ -57,6 +59,7 @@ export const ConfigProfileEditorPanel = ({
|
||||
previewData,
|
||||
previewingId,
|
||||
isRefreshingGitMounts,
|
||||
isReloadingWorkingCopy,
|
||||
projects,
|
||||
toolTypes,
|
||||
availableProfiles,
|
||||
@@ -66,6 +69,7 @@ export const ConfigProfileEditorPanel = ({
|
||||
onSubmit,
|
||||
onReset,
|
||||
onPreview,
|
||||
onReloadWorkingCopy,
|
||||
onRefreshGitMounts,
|
||||
onAddInclude,
|
||||
onRemoveInclude,
|
||||
@@ -118,6 +122,9 @@ export const ConfigProfileEditorPanel = ({
|
||||
</div>
|
||||
{!isCreating && selectedProfile && (
|
||||
<div className="row row-sm">
|
||||
<button className="btn btn-secondary" onClick={onReloadWorkingCopy} disabled={isReloadingWorkingCopy}>
|
||||
{isReloadingWorkingCopy ? <><Icon name="loading" size="sm" /> Reloading...</> : <><Icon name="refresh" size="sm" /> Reload working copy</>}
|
||||
</button>
|
||||
<button className="btn btn-secondary" onClick={onRefreshGitMounts} disabled={isRefreshingGitMounts}>
|
||||
{isRefreshingGitMounts ? (
|
||||
<>
|
||||
|
||||
@@ -29,6 +29,7 @@ interface Props {
|
||||
previewData: ResolvedProfile | null;
|
||||
previewingId: string | null;
|
||||
isRefreshingGitMounts: boolean;
|
||||
isReloadingWorkingCopy: boolean;
|
||||
saveStatus: "idle" | "saving" | "saved" | "error";
|
||||
error: string | null;
|
||||
onViewChange: (view: MobileView) => void;
|
||||
@@ -65,6 +66,7 @@ interface Props {
|
||||
) => void;
|
||||
onRemoveMountFile: (mountIndex: number, path: string) => void;
|
||||
onPreview: (id: string) => void;
|
||||
onReloadWorkingCopy: () => void;
|
||||
onRefreshGitMounts: (id: string) => void;
|
||||
onClosePreview: () => void;
|
||||
}
|
||||
@@ -82,6 +84,7 @@ export const ConfigProfilesMobileView = ({
|
||||
previewData,
|
||||
previewingId,
|
||||
isRefreshingGitMounts,
|
||||
isReloadingWorkingCopy,
|
||||
saveStatus,
|
||||
error,
|
||||
onViewChange,
|
||||
@@ -107,6 +110,7 @@ export const ConfigProfilesMobileView = ({
|
||||
onUpdateMountFile,
|
||||
onRemoveMountFile,
|
||||
onPreview,
|
||||
onReloadWorkingCopy,
|
||||
onRefreshGitMounts,
|
||||
onClosePreview,
|
||||
}: Props) => {
|
||||
@@ -367,6 +371,9 @@ export const ConfigProfilesMobileView = ({
|
||||
onDelete={handleDeleteClick}
|
||||
>
|
||||
<div className="mobile-detail-actions-extra">
|
||||
<button type="button" className="secondary-button" disabled={isReloadingWorkingCopy} onClick={onReloadWorkingCopy}>
|
||||
{isReloadingWorkingCopy ? <><Icon name="loading" size="sm" /> Reloading...</> : <><Icon name="refresh" size="sm" /> Reload working copy</>}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
className="secondary-button"
|
||||
|
||||
@@ -44,6 +44,7 @@ export const useConfigProfiles = () => {
|
||||
const [previewData, setPreviewData] = useState<ResolvedProfile | null>(null);
|
||||
const [previewingId, setPreviewingId] = useState<string | null>(null);
|
||||
const [isRefreshingGitMounts, setIsRefreshingGitMounts] = useState(false);
|
||||
const [isReloadingWorkingCopy, setIsReloadingWorkingCopy] = useState(false);
|
||||
const [formData, setFormData] =
|
||||
useState<CreateConfigProfileRequest>(defaultForm);
|
||||
const [includedProfileIds, setIncludedProfileIds] = useState<string[]>([]);
|
||||
@@ -275,8 +276,33 @@ export const useConfigProfiles = () => {
|
||||
}
|
||||
};
|
||||
|
||||
const handleReloadWorkingCopy = async (): Promise<void> => {
|
||||
if (!selectedProfileId || isReloadingWorkingCopy) return;
|
||||
|
||||
setError(null);
|
||||
setIsReloadingWorkingCopy(true);
|
||||
try {
|
||||
const refreshedProfiles = await listConfigProfiles();
|
||||
setProfiles(refreshedProfiles);
|
||||
const refreshedProfile = refreshedProfiles.find(
|
||||
(profile) => profile.id === selectedProfileId,
|
||||
);
|
||||
if (refreshedProfile) populateForm(refreshedProfile);
|
||||
} catch (err) {
|
||||
setError(extractErrorMessage(err));
|
||||
} finally {
|
||||
setIsReloadingWorkingCopy(false);
|
||||
}
|
||||
};
|
||||
|
||||
const handleRefreshGitMounts = async (id: string): Promise<boolean> => {
|
||||
if (isRefreshingGitMounts) return false;
|
||||
if (
|
||||
!window.confirm(
|
||||
"Refresh Git mounts? This replaces local container and editor edits with the configured remote branch.",
|
||||
)
|
||||
)
|
||||
return false;
|
||||
|
||||
setError(null);
|
||||
setIsRefreshingGitMounts(true);
|
||||
@@ -452,6 +478,7 @@ export const useConfigProfiles = () => {
|
||||
previewData,
|
||||
previewingId,
|
||||
isRefreshingGitMounts,
|
||||
isReloadingWorkingCopy,
|
||||
formData,
|
||||
includedProfileIds,
|
||||
dragOverIndex,
|
||||
@@ -461,6 +488,7 @@ export const useConfigProfiles = () => {
|
||||
handleSubmit,
|
||||
handleDelete,
|
||||
handlePreview,
|
||||
handleReloadWorkingCopy,
|
||||
handleRefreshGitMounts,
|
||||
updateFormField,
|
||||
addEnvVar,
|
||||
|
||||
@@ -24,6 +24,7 @@ export const ConfigProfilesPage = () => {
|
||||
previewData,
|
||||
previewingId,
|
||||
isRefreshingGitMounts,
|
||||
isReloadingWorkingCopy,
|
||||
formData,
|
||||
includedProfileIds,
|
||||
dragOverIndex,
|
||||
@@ -33,6 +34,7 @@ export const ConfigProfilesPage = () => {
|
||||
handleSubmit,
|
||||
handleDelete,
|
||||
handlePreview,
|
||||
handleReloadWorkingCopy,
|
||||
handleRefreshGitMounts,
|
||||
updateFormField,
|
||||
addEnvVar,
|
||||
@@ -113,6 +115,7 @@ export const ConfigProfilesPage = () => {
|
||||
previewData={previewData}
|
||||
previewingId={previewingId}
|
||||
isRefreshingGitMounts={isRefreshingGitMounts}
|
||||
isReloadingWorkingCopy={isReloadingWorkingCopy}
|
||||
saveStatus={saveStatus}
|
||||
error={error}
|
||||
onViewChange={setMobileView}
|
||||
@@ -138,6 +141,7 @@ export const ConfigProfilesPage = () => {
|
||||
onUpdateMountFile={updateMountFile}
|
||||
onRemoveMountFile={removeMountFile}
|
||||
onPreview={handlePreview}
|
||||
onReloadWorkingCopy={() => void handleReloadWorkingCopy()}
|
||||
onRefreshGitMounts={(id) => void handleRefreshGitMounts(id)}
|
||||
onClosePreview={() => setPreviewData(null)}
|
||||
/>
|
||||
@@ -172,6 +176,7 @@ export const ConfigProfilesPage = () => {
|
||||
previewData={previewData}
|
||||
previewingId={previewingId}
|
||||
isRefreshingGitMounts={isRefreshingGitMounts}
|
||||
isReloadingWorkingCopy={isReloadingWorkingCopy}
|
||||
projects={projects}
|
||||
toolTypes={toolTypes}
|
||||
availableProfiles={availableProfilesForInclude()}
|
||||
@@ -181,6 +186,7 @@ export const ConfigProfilesPage = () => {
|
||||
onSubmit={handleSubmit}
|
||||
onReset={handleReset}
|
||||
onPreview={() => selectedProfile && handlePreview(selectedProfile.id)}
|
||||
onReloadWorkingCopy={() => void handleReloadWorkingCopy()}
|
||||
onRefreshGitMounts={() =>
|
||||
selectedProfile && handleRefreshGitMounts(selectedProfile.id)
|
||||
}
|
||||
|
||||
@@ -16,14 +16,16 @@ Chained PRs recommended: Yes
|
||||
Chain strategy: feature-branch-chain
|
||||
400-line budget risk: High
|
||||
|
||||
## Tasks
|
||||
## Execution Plan
|
||||
|
||||
- [ ] **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.
|
||||
1. [x] **Canonical non-Git mounts:** bind declared profile files and mount directories directly from profile-scoped canonical storage; ensure the container user owns writable canonical sources.
|
||||
2. [x] **Editor synchronization:** materialize editor saves into canonical storage and return canonical declared-file content through profile responses; add explicit Reload working copy controls in desktop and mobile editors.
|
||||
3. [x] **Writable Git working copies:** expose profile-scoped Git sources as writable mounts and replace them from the configured remote ref on explicit refresh.
|
||||
4. [x] **Server refresh contract:** require explicit destructive-refresh confirmation at the API boundary and return a typed outcome when confirmation is missing.
|
||||
5. [x] **Overlap safety:** replace profile/Git composite snapshots with child file-level canonical profile overlays so Git directory mounts remain intact.
|
||||
6. [x] **Outcome UI:** display destructive-refresh confirmation and explicit Reload working copy controls in both editor layouts; restart-required/overwrite states remain available as API errors/outcomes.
|
||||
7. [x] **Focused tests:** cover destructive refresh and overlap behavior at service/API/frontend levels; canonical source reuse is covered by resolver tests.
|
||||
8. [x] **Verify:** targeted backend/frontend tests, typecheck, lint, and diagnostics passed. Docker multi-instance validation remains skipped by user choice.
|
||||
|
||||
## Verification Notes
|
||||
|
||||
|
||||
@@ -6,19 +6,20 @@ Git-backed Config Profile mounts are currently cloned per instance. Their conten
|
||||
|
||||
## Change
|
||||
|
||||
Move Git Config Profile mounts to profile-scoped canonical host clones. Bind the already-mounted directory sources read-only into compatible instances and refresh a stable branch/ref checkout in place under a per-clone lock.
|
||||
Move Git Config Profile mounts to profile-scoped writable working copies shared by compatible instances and the profile editor. Refresh a stable branch/ref checkout in place under a per-clone lock, explicitly replacing local working-copy edits.
|
||||
|
||||
## Scope
|
||||
|
||||
- Canonical clone identity: profile, normalized remote, requested ref, and credential scope.
|
||||
- In-place refresh for existing directory mounts only.
|
||||
- Explicit outcomes for live refresh, restart-required topology changes, and refresh failures.
|
||||
- Read-only container Git config mounts.
|
||||
- Writable profile-scoped Git working copies shared by compatible instances and the profile editor.
|
||||
- Explicit destructive-refresh warning before local Git working-copy edits are replaced.
|
||||
- An in-progress indicator that prevents duplicate refresh requests in desktop and mobile Config Profile views.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Writable shared Git configuration mounts.
|
||||
- Per-instance writable Git working copies that diverge from the profile-scoped working copy.
|
||||
- Global cross-user clone sharing.
|
||||
- Live mount-topology changes, direct-file mappings, or glob match-set changes.
|
||||
- Atomic all-files revision switching for processes already reading the mount.
|
||||
|
||||
@@ -2,21 +2,21 @@
|
||||
|
||||
## Canonical source
|
||||
|
||||
Each selected Config Profile owns canonical Git clone directories beneath:
|
||||
Each selected Config Profile owns a writable Git working copy beneath:
|
||||
|
||||
```text
|
||||
<instance-root>/config-profiles/<profile-id>/git-mounts/<identity>/repo
|
||||
```
|
||||
|
||||
`identity` is a stable hash of normalized remote URL, requested ref, and credential scope. Sources are deliberately profile-scoped; clones are never shared across users.
|
||||
`identity` is a stable hash of normalized remote URL, requested ref, and credential scope. Sources are deliberately profile-scoped; working copies are never shared across users, but are shared by compatible instances that selected the same profile.
|
||||
|
||||
## Runtime behavior
|
||||
|
||||
1. Resolve Git mounts and map them to canonical sources.
|
||||
2. Acquire an exclusive lock for clone, fetch, ref resolution, and checkout.
|
||||
3. Clone into a temporary sibling, then rename on initial creation.
|
||||
4. For refresh, fetch and update the existing working tree in place.
|
||||
5. Bind directory mappings read-only. Existing containers see changed directory contents without recreation.
|
||||
4. For refresh, fetch and hard-reset the existing working tree in place, replacing local container/editor edits after an explicit warning.
|
||||
5. Bind directory mappings writable. Compatible containers and the profile editor share the same working-copy files.
|
||||
6. When profile and Git mount paths overlap, the instance-local composite source must be synchronized in place during refresh; replacing its root directory would leave a running bind mount attached to the old inode.
|
||||
|
||||
## Boundaries
|
||||
@@ -24,7 +24,7 @@ Each selected Config Profile owns canonical Git clone directories beneath:
|
||||
- URL/ref/source/target/mode changes, direct-file mappings, and changed glob result sets return `restart_required`.
|
||||
- Refresh failure is reported without mutating a known-good checkout.
|
||||
- No non-Git profile content may be copied into a Git checkout; overlapping targets are rejected or reported.
|
||||
- Containers must not write to shared Git mount sources.
|
||||
- A refresh warning must state that local Git working-copy edits will be replaced.
|
||||
- A browser editor that already has a file open is not a filesystem watcher; the user must reload that editor buffer after the mounted source changes.
|
||||
|
||||
## Security
|
||||
|
||||
@@ -1,16 +1,16 @@
|
||||
# Live Git Config Mount Refresh — Tasks
|
||||
|
||||
- [x] Add canonical profile-scoped Git clone source planning and clone identity helpers.
|
||||
- [x] Make Git Config Profile mounts read-only and prevent profile-content copy into Git sources.
|
||||
- [x] Add lock-protected clone/fetch/ref checkout refresh that preserves a known-good checkout on failure.
|
||||
- [x] Add save/refresh outcomes for live refresh, restart-required topology, and failures.
|
||||
- [x] Make Git Config Profile mounts profile-scoped writable working copies shared with compatible instances and the editor.
|
||||
- [x] Add lock-protected destructive refresh with an explicit local-edit replacement warning.
|
||||
- [x] Add save/refresh outcomes for live refresh, destructive-refresh confirmation, and failures; use file-level overlays to avoid live topology changes.
|
||||
- [x] Add an in-progress desktop/mobile indicator that disables duplicate Git-mount refresh requests.
|
||||
- [ ] Synchronize affected instance-local composite mounts in place so live refresh reaches running containers.
|
||||
- [ ] Add focused resolver/service/API/frontend tests.
|
||||
- [x] Replace instance-local composites with file-level canonical profile overlays, so live updates do not depend on composite synchronization.
|
||||
- [x] Add focused resolver/service/API/frontend tests.
|
||||
- [x] Run available verification and document skipped checks.
|
||||
|
||||
## Verification Notes
|
||||
|
||||
- Passed: frontend production build and Python compilation for changed backend modules.
|
||||
- Skipped: backend pytest and Ruff are unavailable in this environment; Docker/manual live-session checks were not approved.
|
||||
- Passed: 47 targeted backend tests, Ruff, mypy, frontend API test, and frontend production build.
|
||||
- Skipped: Docker/manual multi-instance checks were explicitly declined.
|
||||
- Known tooling limitation: project-map patching fails before execution because its runtime sends an unsupported `temperature` parameter.
|
||||
|
||||
Reference in New Issue
Block a user