Compare commits
21 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| bf561186fb | |||
| 0afce741eb | |||
| 7603b739cb | |||
| ff8efdd887 | |||
| b289edf5a9 | |||
| 75b67f2a6f | |||
| bcc7486b59 | |||
| 29e765b2be | |||
| d14cdc1151 | |||
| c80dbf9737 | |||
| 5331a0f110 | |||
| b995521e22 | |||
| 5610017f50 | |||
| 61e7d68d71 | |||
| 2247ec47c9 | |||
| 0d6c1926ae | |||
| 900a8e47a5 | |||
| 25e870ba43 | |||
| 16984b7cf6 | |||
| fc52353b2e | |||
| a63a983116 |
@@ -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,
|
||||
)
|
||||
@@ -34,7 +37,6 @@ from src.services.config.crud_service import (
|
||||
update_profile,
|
||||
validate_default_profiles,
|
||||
)
|
||||
from src.services.tool.instance_service import resolve_git_mounts
|
||||
from src.services.config.resolver_service import (
|
||||
resolve_default_profile,
|
||||
validate_git_url,
|
||||
@@ -45,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]]:
|
||||
@@ -115,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(
|
||||
@@ -148,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)
|
||||
@@ -170,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
|
||||
@@ -179,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(
|
||||
@@ -193,6 +232,10 @@ async def refresh_profile_git_mounts(
|
||||
status_code=status.HTTP_403_FORBIDDEN, detail="Not authorized"
|
||||
)
|
||||
|
||||
# Import lazily: instance_service imports tool schemas that transitively
|
||||
# load API routers, so importing it during router initialization cycles.
|
||||
from src.services.tool.instance_service import resolve_git_mounts
|
||||
|
||||
outcomes = await _running_profile_outcomes(session, profile.id)
|
||||
for outcome in outcomes:
|
||||
instance = await session.get(ToolInstance, uuid.UUID(outcome["instance_id"]))
|
||||
|
||||
@@ -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 staged mount sources to the container user.
|
||||
"""Recursively chown writable profile and instance mount sources.
|
||||
|
||||
Config-profile mounts, git mounts, and SSH key mounts are staged under
|
||||
instance_dir by the API process (root). Without this, the container
|
||||
user cannot write into bind-mounted directories such as ~/.config.
|
||||
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)
|
||||
|
||||
@@ -156,27 +161,118 @@ def _relative_under(parent: str, child: str) -> str | None:
|
||||
return None
|
||||
|
||||
|
||||
def _stage_profile_mounts(profile_mounts: list[dict], instance_dir: str) -> list[dict]:
|
||||
"""Copy profile bind sources into an instance-local, writable staging area."""
|
||||
staged_mounts: list[dict] = []
|
||||
staging_root = os.path.join(instance_dir, "mounts", "profiles")
|
||||
|
||||
for mount in profile_mounts:
|
||||
source = mount.get("source", "")
|
||||
target = mount.get("target", "")
|
||||
if not source or not target:
|
||||
continue
|
||||
if not os.path.exists(source):
|
||||
logger.warning("Skipping missing config profile mount source: %s", source)
|
||||
continue
|
||||
|
||||
digest = hashlib.sha256(f"{source}\0{target}".encode()).hexdigest()[:16]
|
||||
staged_source = os.path.join(staging_root, digest)
|
||||
try:
|
||||
if os.path.lexists(staged_source):
|
||||
if os.path.isdir(staged_source):
|
||||
shutil.rmtree(staged_source)
|
||||
else:
|
||||
os.unlink(staged_source)
|
||||
os.makedirs(os.path.dirname(staged_source), exist_ok=True)
|
||||
|
||||
if os.path.isdir(source):
|
||||
shutil.copytree(source, staged_source, symlinks=True)
|
||||
else:
|
||||
shutil.copy2(source, staged_source, follow_symlinks=False)
|
||||
except OSError as exc:
|
||||
logger.error("Failed to stage config profile mount %s: %s", source, exc)
|
||||
continue
|
||||
|
||||
staged_mount = dict(mount)
|
||||
staged_mount["source"] = staged_source
|
||||
staged_mounts.append(staged_mount)
|
||||
|
||||
return staged_mounts
|
||||
|
||||
|
||||
def _mounts_overlap(first_target: str, second_target: str) -> bool:
|
||||
"""Return whether two normalized container mount targets intersect."""
|
||||
return (
|
||||
_relative_under(first_target, second_target) is not None
|
||||
or _relative_under(second_target, first_target) is not None
|
||||
)
|
||||
|
||||
|
||||
def _copy_mount_source(source: str, destination: str) -> None:
|
||||
"""Copy a bind-mount source into its destination in a composite tree."""
|
||||
try:
|
||||
if os.path.isdir(source):
|
||||
os.makedirs(destination, exist_ok=True)
|
||||
for entry in os.listdir(source):
|
||||
source_entry = os.path.join(source, entry)
|
||||
destination_entry = os.path.join(destination, entry)
|
||||
if os.path.isdir(source_entry):
|
||||
shutil.copytree(
|
||||
source_entry,
|
||||
destination_entry,
|
||||
dirs_exist_ok=True,
|
||||
symlinks=True,
|
||||
)
|
||||
else:
|
||||
os.makedirs(os.path.dirname(destination_entry), exist_ok=True)
|
||||
shutil.copy2(source_entry, destination_entry, follow_symlinks=False)
|
||||
return
|
||||
|
||||
os.makedirs(os.path.dirname(destination), exist_ok=True)
|
||||
shutil.copy2(source, destination, follow_symlinks=False)
|
||||
except OSError as exc:
|
||||
raise RuntimeError(f"Unable to compose mount source {source}: {exc}") from exc
|
||||
|
||||
|
||||
def _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts: list[dict],
|
||||
git_mount_volumes: list[dict],
|
||||
instance_dir: str,
|
||||
) -> list[dict]:
|
||||
"""Leave profile and Git sources isolated.
|
||||
"""Overlay canonical profile files on Git directories without snapshots.
|
||||
|
||||
Git mount sources are shared, read-only canonical checkouts. Profile
|
||||
content must never be copied into one because doing so dirties the
|
||||
checkout and leaks one profile's content to every instance using it.
|
||||
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.
|
||||
"""
|
||||
del instance_dir
|
||||
result: list[dict] = list(git_mount_volumes)
|
||||
|
||||
for profile_mount in profile_mounts:
|
||||
profile_target = profile_mount.get("target", "")
|
||||
for git_mount in git_mount_volumes:
|
||||
if _relative_under(git_mount.get("target", ""), profile_target) is not None:
|
||||
logger.warning(
|
||||
"Config profile mount %s overlaps Git mount %s; keeping sources isolated",
|
||||
profile_target,
|
||||
git_mount.get("target"),
|
||||
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
|
||||
|
||||
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),
|
||||
}
|
||||
)
|
||||
break
|
||||
return profile_mounts
|
||||
|
||||
return result
|
||||
|
||||
|
||||
async def resolve_git_mounts(
|
||||
@@ -476,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
|
||||
|
||||
|
||||
@@ -522,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
|
||||
|
||||
@@ -539,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]:
|
||||
@@ -1435,22 +1540,24 @@ async def start_tool_instance(
|
||||
git_mount_volumes = await resolve_git_mounts(
|
||||
session, resolved, instance_dir, working_directory, home_dir
|
||||
)
|
||||
# Stack static file mounts on top of git repo mounts so they do
|
||||
# not mask each other when they target the same directory.
|
||||
stacked_profile_mounts = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mount_volumes
|
||||
# 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(
|
||||
profile_mounts,
|
||||
git_mount_volumes,
|
||||
instance_dir,
|
||||
)
|
||||
extra_volumes.extend(stacked_profile_mounts)
|
||||
extra_volumes.extend(git_mount_volumes)
|
||||
extra_volumes.extend(composed_mounts)
|
||||
logger.debug(
|
||||
"Applied config profile %s to instance %s (env=%d, files=%d, mounts=%d, git_mounts=%d, stacked=%d)",
|
||||
"Applied config profile %s to instance %s (env=%d, files=%d, profile_mounts=%d, git_mounts=%d, composed_mounts=%d)",
|
||||
resolved.profile_name,
|
||||
instance.id,
|
||||
len(profile_env),
|
||||
len(profile_files),
|
||||
len(profile_mounts),
|
||||
len(git_mount_volumes),
|
||||
len(profile_mounts) - len(stacked_profile_mounts),
|
||||
len(composed_mounts),
|
||||
)
|
||||
except ConfigProfileCycleError as exc:
|
||||
logger.error(
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -2,16 +2,20 @@
|
||||
|
||||
import hashlib
|
||||
import uuid
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
from src.services.tool.instance_service import (
|
||||
_chown_staged_mounts,
|
||||
_get_repository_mount_name,
|
||||
_stack_profile_mounts_with_git_mounts,
|
||||
_stage_profile_mounts,
|
||||
clone_git_repo,
|
||||
modify_compose_file,
|
||||
prepare_manifest_instance,
|
||||
pull_repository_updates,
|
||||
)
|
||||
|
||||
|
||||
@@ -94,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:
|
||||
@@ -123,151 +158,174 @@ class TestCloneGitRepo:
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestStackProfileMountsWithGitMounts:
|
||||
"""Tests for _stack_profile_mounts_with_git_mounts."""
|
||||
"""Tests for composing profile and Git mounts without Docker masking."""
|
||||
|
||||
def test_exact_overlap_keeps_profile_files_out_of_git_source(
|
||||
self, tmp_path
|
||||
def test_stages_profile_source_under_instance_directory(self, tmp_path) -> None:
|
||||
"""Profile sources must be instance-local before ownership is fixed."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
profile_source = tmp_path / "profile" / "settings.json"
|
||||
profile_source.parent.mkdir(parents=True)
|
||||
profile_source.write_text("{}")
|
||||
|
||||
staged = _stage_profile_mounts(
|
||||
[{"source": str(profile_source), "target": "/home/user/.pi/settings.json"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
|
||||
assert staged[0]["source"].startswith(str(instance_dir))
|
||||
assert staged[0]["source"] != str(profile_source)
|
||||
assert Path(staged[0]["source"]).read_text() == "{}"
|
||||
|
||||
def test_staged_profile_source_is_chowned_for_container_user(
|
||||
self, monkeypatch, tmp_path
|
||||
) -> None:
|
||||
"""Overlaps must not dirty the shared Git checkout."""
|
||||
"""The ownership pass must include the instance-local profile copy."""
|
||||
from src.services.tool import instance_service
|
||||
|
||||
instance_dir = tmp_path / "instance"
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "settings.json").write_text("{}")
|
||||
staged = _stage_profile_mounts(
|
||||
[{"source": str(profile_source), "target": "/home/user/.pi"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
chown = MagicMock()
|
||||
monkeypatch.setattr(instance_service, "_chown_path", chown)
|
||||
|
||||
_chown_staged_mounts(staged, str(instance_dir), 1000, 1000)
|
||||
|
||||
chown.assert_called_once_with(staged[0]["source"], 1000, 1000)
|
||||
|
||||
def test_exact_overlap_creates_instance_local_composite(self, tmp_path) -> None:
|
||||
"""Profile files extend a Git root without mutating its shared clone."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
git_source = tmp_path / "git" / "repo-clone"
|
||||
git_source.mkdir(parents=True)
|
||||
(git_source / "existing.txt").write_text("from git")
|
||||
|
||||
profile_source = tmp_path / "profile" / "home_user_.pi"
|
||||
profile_source.mkdir(parents=True)
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "settings.json").write_text("{}")
|
||||
|
||||
profile_mounts = [
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi",
|
||||
"type": "bind",
|
||||
"readonly": False,
|
||||
}
|
||||
]
|
||||
git_mount_volumes = [
|
||||
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||
]
|
||||
|
||||
profile_mounts = _stage_profile_mounts(
|
||||
[
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi",
|
||||
"type": "bind",
|
||||
}
|
||||
],
|
||||
str(instance_dir),
|
||||
)
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mount_volumes
|
||||
profile_mounts,
|
||||
[{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
|
||||
assert result == profile_mounts
|
||||
assert (git_source / "existing.txt").read_text() == "from git"
|
||||
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_overlap_keeps_sources_isolated(self, tmp_path) -> None:
|
||||
"""A child profile mount must not mutate the shared Git checkout."""
|
||||
def test_descendant_profile_mount_extends_git_root(self, tmp_path) -> None:
|
||||
"""Nested targets are composed at the Git root, preserving siblings."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
git_source = tmp_path / "git"
|
||||
git_source.mkdir()
|
||||
(git_source / "README").write_text("repo")
|
||||
|
||||
profile_source = tmp_path / "profile" / "agent"
|
||||
profile_source.mkdir(parents=True)
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "settings.json").write_text("x")
|
||||
|
||||
profile_mounts = [
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi/agent",
|
||||
"type": "bind",
|
||||
}
|
||||
]
|
||||
git_mount_volumes = [
|
||||
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||
]
|
||||
|
||||
profile_mounts = _stage_profile_mounts(
|
||||
[{"source": str(profile_source), "target": "/home/user/.pi/agent"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mount_volumes
|
||||
profile_mounts,
|
||||
[{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
|
||||
assert result == profile_mounts
|
||||
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()
|
||||
assert (git_source / "README").read_text() == "repo"
|
||||
|
||||
def test_non_overlapping_mounts_left_untouched(self, tmp_path) -> None:
|
||||
"""Profile mounts that do not overlap a git mount are returned as-is."""
|
||||
def test_file_profile_mount_extends_git_root(self, tmp_path) -> None:
|
||||
"""A file bind mount is composed into the Git directory, not masked."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
git_source = tmp_path / "git"
|
||||
git_source.mkdir()
|
||||
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "config").write_text("c")
|
||||
|
||||
profile_mounts = [
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.config",
|
||||
"type": "bind",
|
||||
}
|
||||
]
|
||||
git_mount_volumes = [
|
||||
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||
]
|
||||
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mount_volumes
|
||||
)
|
||||
|
||||
assert result == profile_mounts
|
||||
|
||||
def test_git_source_file_does_not_consume_profile_mount(self, tmp_path) -> None:
|
||||
"""If the overlapping git-mount source is a file, the profile mount
|
||||
cannot be merged and must be kept."""
|
||||
git_source = tmp_path / "file.txt"
|
||||
git_source.write_text("file")
|
||||
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "settings.json").write_text("{}")
|
||||
|
||||
profile_mounts = [
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi",
|
||||
"type": "bind",
|
||||
}
|
||||
]
|
||||
git_mount_volumes = [
|
||||
{
|
||||
"source": str(git_source),
|
||||
"target": "/home/user/.pi/file.txt",
|
||||
"type": "bind",
|
||||
}
|
||||
]
|
||||
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mount_volumes
|
||||
)
|
||||
|
||||
assert result == profile_mounts
|
||||
|
||||
def test_profile_source_file_does_not_mutate_git_source(self, tmp_path) -> None:
|
||||
"""A profile file must not be copied into a shared Git checkout."""
|
||||
git_source = tmp_path / "git"
|
||||
git_source.mkdir()
|
||||
|
||||
(git_source / "README").write_text("repo")
|
||||
profile_source = tmp_path / "settings.json"
|
||||
profile_source.write_text("{}")
|
||||
|
||||
profile_mounts = [
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi/settings.json",
|
||||
"type": "bind",
|
||||
}
|
||||
]
|
||||
git_mount_volumes = [
|
||||
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||
]
|
||||
|
||||
profile_mounts = _stage_profile_mounts(
|
||||
[
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi/settings.json",
|
||||
}
|
||||
],
|
||||
str(instance_dir),
|
||||
)
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mount_volumes
|
||||
profile_mounts,
|
||||
[{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
|
||||
assert result == profile_mounts
|
||||
assert not (git_source / "settings.json").exists()
|
||||
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."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
git_source = tmp_path / "git"
|
||||
git_source.mkdir()
|
||||
(git_source / "plugin.toml").write_text("git")
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "config.toml").write_text("profile")
|
||||
|
||||
profile_mounts = _stage_profile_mounts(
|
||||
[{"source": str(profile_source), "target": "/home/user"}], str(instance_dir)
|
||||
)
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts,
|
||||
[{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
|
||||
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."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
git_source = tmp_path / "git"
|
||||
git_source.mkdir()
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
|
||||
profile_mounts = _stage_profile_mounts(
|
||||
[{"source": str(profile_source), "target": "/home/user/.config"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
git_mounts = [{"source": str(git_source), "target": "/home/user/.pi"}]
|
||||
|
||||
assert (
|
||||
_stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mounts, str(instance_dir)
|
||||
)
|
||||
== git_mounts + profile_mounts
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
|
||||
@@ -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;
|
||||
};
|
||||
|
||||
|
||||
@@ -14,6 +14,8 @@ interface Props {
|
||||
saveStatus: "idle" | "saving" | "saved" | "error";
|
||||
previewData: ResolvedProfile | null;
|
||||
previewingId: string | null;
|
||||
isRefreshingGitMounts: boolean;
|
||||
isReloadingWorkingCopy: boolean;
|
||||
projects: ProjectWithRepos[];
|
||||
toolTypes: ToolType[];
|
||||
availableProfiles: ConfigProfile[];
|
||||
@@ -23,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;
|
||||
@@ -55,6 +58,8 @@ export const ConfigProfileEditorPanel = ({
|
||||
saveStatus,
|
||||
previewData,
|
||||
previewingId,
|
||||
isRefreshingGitMounts,
|
||||
isReloadingWorkingCopy,
|
||||
projects,
|
||||
toolTypes,
|
||||
availableProfiles,
|
||||
@@ -64,6 +69,7 @@ export const ConfigProfileEditorPanel = ({
|
||||
onSubmit,
|
||||
onReset,
|
||||
onPreview,
|
||||
onReloadWorkingCopy,
|
||||
onRefreshGitMounts,
|
||||
onAddInclude,
|
||||
onRemoveInclude,
|
||||
@@ -116,8 +122,19 @@ export const ConfigProfileEditorPanel = ({
|
||||
</div>
|
||||
{!isCreating && selectedProfile && (
|
||||
<div className="row row-sm">
|
||||
<button className="btn btn-secondary" onClick={onRefreshGitMounts}>
|
||||
<Icon name="refresh" size="sm" /> Refresh Git mounts
|
||||
<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 ? (
|
||||
<>
|
||||
<Icon name="loading" size="sm" /> Refreshing Git mounts...
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<Icon name="refresh" size="sm" /> Refresh Git mounts
|
||||
</>
|
||||
)}
|
||||
</button>
|
||||
<button className="btn btn-secondary" onClick={onPreview} disabled={previewingId === selectedProfile.id}>
|
||||
{previewingId === selectedProfile.id ? (
|
||||
|
||||
@@ -28,6 +28,8 @@ interface Props {
|
||||
availableProfiles: ConfigProfile[];
|
||||
previewData: ResolvedProfile | null;
|
||||
previewingId: string | null;
|
||||
isRefreshingGitMounts: boolean;
|
||||
isReloadingWorkingCopy: boolean;
|
||||
saveStatus: "idle" | "saving" | "saved" | "error";
|
||||
error: string | null;
|
||||
onViewChange: (view: MobileView) => void;
|
||||
@@ -64,6 +66,7 @@ interface Props {
|
||||
) => void;
|
||||
onRemoveMountFile: (mountIndex: number, path: string) => void;
|
||||
onPreview: (id: string) => void;
|
||||
onReloadWorkingCopy: () => void;
|
||||
onRefreshGitMounts: (id: string) => void;
|
||||
onClosePreview: () => void;
|
||||
}
|
||||
@@ -80,6 +83,8 @@ export const ConfigProfilesMobileView = ({
|
||||
availableProfiles,
|
||||
previewData,
|
||||
previewingId,
|
||||
isRefreshingGitMounts,
|
||||
isReloadingWorkingCopy,
|
||||
saveStatus,
|
||||
error,
|
||||
onViewChange,
|
||||
@@ -105,6 +110,7 @@ export const ConfigProfilesMobileView = ({
|
||||
onUpdateMountFile,
|
||||
onRemoveMountFile,
|
||||
onPreview,
|
||||
onReloadWorkingCopy,
|
||||
onRefreshGitMounts,
|
||||
onClosePreview,
|
||||
}: Props) => {
|
||||
@@ -365,12 +371,25 @@ 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"
|
||||
disabled={isRefreshingGitMounts}
|
||||
onClick={() => onRefreshGitMounts(selectedProfile.id)}
|
||||
>
|
||||
<Icon name="refresh" size="sm" /> Refresh Git mounts
|
||||
{isRefreshingGitMounts ? (
|
||||
<>
|
||||
<Icon name="loading" size="sm" />
|
||||
Refreshing Git mounts...
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<Icon name="refresh" size="sm" /> Refresh Git mounts
|
||||
</>
|
||||
)}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
getTerminalScrollbackLimit,
|
||||
isCurrentWebSocket,
|
||||
shouldRetryWebSocketClose,
|
||||
shouldShowTerminalCopyMenu,
|
||||
} from "./terminal.tsx";
|
||||
|
||||
describe("getTerminalScrollbackLimit", () => {
|
||||
@@ -30,3 +31,13 @@ describe("getTerminalScrollbackLimit", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldShowTerminalCopyMenu", () => {
|
||||
it("shows Copy for selected terminal output", () => {
|
||||
expect(shouldShowTerminalCopyMenu("selected output")).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps the native context menu when no output is selected", () => {
|
||||
expect(shouldShowTerminalCopyMenu("")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -68,6 +68,22 @@ export function shouldRetryWebSocketClose(code: number, reason: string): boolean
|
||||
return code !== 1000 && !(code === 4000 && reason === "New connection established");
|
||||
}
|
||||
|
||||
export function shouldShowTerminalCopyMenu(selection: string): boolean {
|
||||
return selection.length > 0;
|
||||
}
|
||||
|
||||
function copyTextWithFallback(text: string): void {
|
||||
const textarea = document.createElement("textarea");
|
||||
textarea.value = text;
|
||||
textarea.setAttribute("readonly", "");
|
||||
textarea.style.position = "fixed";
|
||||
textarea.style.opacity = "0";
|
||||
document.body.appendChild(textarea);
|
||||
textarea.select();
|
||||
document.execCommand("copy");
|
||||
textarea.remove();
|
||||
}
|
||||
|
||||
function matchesByteSequence(
|
||||
data: Uint8Array,
|
||||
start: number,
|
||||
@@ -107,6 +123,11 @@ export const TerminalComponent = React.forwardRef<TerminalRef, TerminalProps>(
|
||||
>("connecting");
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const [showResetConfirm, setShowResetConfirm] = useState(false);
|
||||
const [copyMenu, setCopyMenu] = useState<{
|
||||
x: number;
|
||||
y: number;
|
||||
text: string;
|
||||
} | null>(null);
|
||||
const activeModifierRef = useRef(activeModifier);
|
||||
activeModifierRef.current = activeModifier;
|
||||
const [fontSize, setFontSize] = useState(() => {
|
||||
@@ -462,8 +483,27 @@ export const TerminalComponent = React.forwardRef<TerminalRef, TerminalProps>(
|
||||
term.paste(text);
|
||||
};
|
||||
|
||||
const handleBrowserCopy = (event: ClipboardEvent) => {
|
||||
const selection = term.getSelection();
|
||||
if (!selection) return;
|
||||
|
||||
event.preventDefault();
|
||||
event.clipboardData?.setData("text/plain", selection);
|
||||
};
|
||||
const handleTerminalContextMenu = (event: MouseEvent) => {
|
||||
const selection = term.getSelection();
|
||||
if (!shouldShowTerminalCopyMenu(selection)) {
|
||||
setCopyMenu(null);
|
||||
return;
|
||||
}
|
||||
|
||||
event.preventDefault();
|
||||
event.stopImmediatePropagation();
|
||||
event.stopPropagation();
|
||||
setCopyMenu({ x: event.clientX, y: event.clientY, text: selection });
|
||||
};
|
||||
|
||||
const handleBrowserPaste = (event: ClipboardEvent) => {
|
||||
if (!bracketedPasteEnabledRef.current) return;
|
||||
const text = event.clipboardData?.getData("text/plain");
|
||||
if (text === undefined || wsRef.current?.readyState !== WebSocket.OPEN) {
|
||||
return;
|
||||
@@ -473,6 +513,8 @@ export const TerminalComponent = React.forwardRef<TerminalRef, TerminalProps>(
|
||||
event.stopImmediatePropagation();
|
||||
pasteTextRef.current(text);
|
||||
};
|
||||
container.addEventListener("copy", handleBrowserCopy, true);
|
||||
container.addEventListener("contextmenu", handleTerminalContextMenu, true);
|
||||
container.addEventListener("paste", handleBrowserPaste, true);
|
||||
|
||||
// Mobile touch scroll.
|
||||
@@ -720,6 +762,12 @@ export const TerminalComponent = React.forwardRef<TerminalRef, TerminalProps>(
|
||||
handleVisibilityChange,
|
||||
);
|
||||
if (touchCleanup) touchCleanup();
|
||||
container.removeEventListener("copy", handleBrowserCopy, true);
|
||||
container.removeEventListener(
|
||||
"contextmenu",
|
||||
handleTerminalContextMenu,
|
||||
true,
|
||||
);
|
||||
container.removeEventListener("paste", handleBrowserPaste, true);
|
||||
pasteTextRef.current = () => {};
|
||||
bracketedPasteEnabledRef.current = false;
|
||||
@@ -854,6 +902,7 @@ export const TerminalComponent = React.forwardRef<TerminalRef, TerminalProps>(
|
||||
|
||||
// Focus terminal on mobile to keep keyboard open
|
||||
const handleTerminalClick = () => {
|
||||
setCopyMenu(null);
|
||||
if (isMobile && termRef.current) {
|
||||
termRef.current.focus();
|
||||
}
|
||||
@@ -968,6 +1017,26 @@ export const TerminalComponent = React.forwardRef<TerminalRef, TerminalProps>(
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
{copyMenu && (
|
||||
<button
|
||||
aria-label="Copy selected terminal text"
|
||||
className="terminal-context-copy"
|
||||
onMouseDown={(event) => {
|
||||
event.preventDefault();
|
||||
copyTextWithFallback(copyMenu.text);
|
||||
setCopyMenu(null);
|
||||
}}
|
||||
style={{
|
||||
left: copyMenu.x,
|
||||
position: "fixed",
|
||||
top: copyMenu.y,
|
||||
zIndex: 1000,
|
||||
}}
|
||||
type="button"
|
||||
>
|
||||
Copy
|
||||
</button>
|
||||
)}
|
||||
{error && (
|
||||
<div className="terminal-error">
|
||||
{error}
|
||||
|
||||
@@ -43,6 +43,8 @@ export const useConfigProfiles = () => {
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
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[]>([]);
|
||||
@@ -274,8 +276,36 @@ export const useConfigProfiles = () => {
|
||||
}
|
||||
};
|
||||
|
||||
const handleRefreshGitMounts = async (id: string): Promise<boolean> => {
|
||||
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);
|
||||
try {
|
||||
const result = await refreshConfigProfileGitMounts(id);
|
||||
const refreshed = result.refresh_outcomes.filter(
|
||||
@@ -290,6 +320,8 @@ export const useConfigProfiles = () => {
|
||||
} catch (err) {
|
||||
setError(extractErrorMessage(err));
|
||||
return false;
|
||||
} finally {
|
||||
setIsRefreshingGitMounts(false);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -445,6 +477,8 @@ export const useConfigProfiles = () => {
|
||||
error,
|
||||
previewData,
|
||||
previewingId,
|
||||
isRefreshingGitMounts,
|
||||
isReloadingWorkingCopy,
|
||||
formData,
|
||||
includedProfileIds,
|
||||
dragOverIndex,
|
||||
@@ -454,6 +488,7 @@ export const useConfigProfiles = () => {
|
||||
handleSubmit,
|
||||
handleDelete,
|
||||
handlePreview,
|
||||
handleReloadWorkingCopy,
|
||||
handleRefreshGitMounts,
|
||||
updateFormField,
|
||||
addEnvVar,
|
||||
|
||||
@@ -23,6 +23,8 @@ export const ConfigProfilesPage = () => {
|
||||
error,
|
||||
previewData,
|
||||
previewingId,
|
||||
isRefreshingGitMounts,
|
||||
isReloadingWorkingCopy,
|
||||
formData,
|
||||
includedProfileIds,
|
||||
dragOverIndex,
|
||||
@@ -32,6 +34,7 @@ export const ConfigProfilesPage = () => {
|
||||
handleSubmit,
|
||||
handleDelete,
|
||||
handlePreview,
|
||||
handleReloadWorkingCopy,
|
||||
handleRefreshGitMounts,
|
||||
updateFormField,
|
||||
addEnvVar,
|
||||
@@ -111,6 +114,8 @@ export const ConfigProfilesPage = () => {
|
||||
availableProfiles={availableProfilesForInclude()}
|
||||
previewData={previewData}
|
||||
previewingId={previewingId}
|
||||
isRefreshingGitMounts={isRefreshingGitMounts}
|
||||
isReloadingWorkingCopy={isReloadingWorkingCopy}
|
||||
saveStatus={saveStatus}
|
||||
error={error}
|
||||
onViewChange={setMobileView}
|
||||
@@ -136,6 +141,7 @@ export const ConfigProfilesPage = () => {
|
||||
onUpdateMountFile={updateMountFile}
|
||||
onRemoveMountFile={removeMountFile}
|
||||
onPreview={handlePreview}
|
||||
onReloadWorkingCopy={() => void handleReloadWorkingCopy()}
|
||||
onRefreshGitMounts={(id) => void handleRefreshGitMounts(id)}
|
||||
onClosePreview={() => setPreviewData(null)}
|
||||
/>
|
||||
@@ -169,6 +175,8 @@ export const ConfigProfilesPage = () => {
|
||||
saveStatus={saveStatus}
|
||||
previewData={previewData}
|
||||
previewingId={previewingId}
|
||||
isRefreshingGitMounts={isRefreshingGitMounts}
|
||||
isReloadingWorkingCopy={isReloadingWorkingCopy}
|
||||
projects={projects}
|
||||
toolTypes={toolTypes}
|
||||
availableProfiles={availableProfilesForInclude()}
|
||||
@@ -178,7 +186,10 @@ export const ConfigProfilesPage = () => {
|
||||
onSubmit={handleSubmit}
|
||||
onReset={handleReset}
|
||||
onPreview={() => selectedProfile && handlePreview(selectedProfile.id)}
|
||||
onRefreshGitMounts={() => selectedProfile && handleRefreshGitMounts(selectedProfile.id)}
|
||||
onReloadWorkingCopy={() => void handleReloadWorkingCopy()}
|
||||
onRefreshGitMounts={() =>
|
||||
selectedProfile && handleRefreshGitMounts(selectedProfile.id)
|
||||
}
|
||||
onAddInclude={addInclude}
|
||||
onRemoveInclude={removeInclude}
|
||||
onDragStart={handleDragStart}
|
||||
|
||||
@@ -12,6 +12,7 @@ After implementing configurable tool container home directories, new `pi-agent`
|
||||
4. `npm_global` packages are installed with `RUN npm install -g ...` as root into the system npm prefix, so the non-root container user cannot update them.
|
||||
5. Once the repo mount moves out of `/workspace`, the generated `/workspace` compatibility symlink is created in the image as root. The non-root entrypoint cannot replace it (write permission is required on `/`), so container startup fails.
|
||||
6. Older images baked a literal `{{WORKSPACE_NAME}}` directory into `/home/user`, which survives alongside the real repo-named mount directory.
|
||||
7. Config-profile file mounts use canonical profile storage owned by the API process. A non-root container user therefore cannot write to writable bind mounts. When a directory-level profile mount and a Git mount share or nest under the same target, Docker bind mounting masks the earlier source rather than merging their files.
|
||||
|
||||
## Fix
|
||||
|
||||
@@ -40,7 +41,9 @@ After implementing configurable tool container home directories, new `pi-agent`
|
||||
6. Remove the explicit repo mount from the built-in `pi-agent` manifest so the repo mount is synthesized by `compile_compose` rather than depending on tool config. Add a follow-up Alembic data migration that strips the `source_type: repo` mount from the manifest.
|
||||
7. Add `_get_repository_mount_name()` helper. When the instance is bound to a workspace, the helper returns the basename of `workspace.path`. For legacy repo-only instances it falls back to parsing the remote URL like `git clone` would, then to the user-provided repository name.
|
||||
8. Switch workspace storage layout to `/data/working-copies/{workspace_id}/{repo_name}/` so `git clone` creates the repo-named directory naturally, making `workspace.path.basename` the correct container mount name. This replaces the previous `/data/working-copies/{repo_id}/{workspace_name}/` layout.
|
||||
9. Update unit tests for the new behavior.
|
||||
9. Stage every config-profile bind-mount source into the instance directory before compose generation. This makes writable mounts user-owned without changing the shared canonical profile source.
|
||||
10. Replace overlapping profile and Git bind mounts with a per-instance composite source. The composite copies Git content first and profile content second, preserving Git siblings while allowing profile files to override matching paths; it is then chowned with the other staged mounts. This removes duplicate/nested Docker mounts rather than relying on mount order to merge them.
|
||||
11. Update unit tests for the new behavior.
|
||||
|
||||
## Affected files
|
||||
|
||||
|
||||
@@ -12,5 +12,8 @@
|
||||
- [x] Remove compose-level `user: 0:0` override so entrypoint can drop privileges
|
||||
- [x] Pass manifest-declared container user to terminal sessions via `docker exec --user`
|
||||
- [x] Update unit tests for container user/terminal changes
|
||||
- [x] Stage profile bind mounts per instance so writable sources are owned by the container user
|
||||
- [x] Composite overlapping profile and Git mounts into one per-instance bind source
|
||||
- [x] Add regression tests for mount composition, ownership staging, and mount order
|
||||
- [ ] Run quality gates for container user/terminal changes
|
||||
- [ ] Commit and push
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
# Fix Web Terminal Clipboard
|
||||
|
||||
## Problem
|
||||
|
||||
Users cannot reliably copy terminal output from the browser terminal. Browser copy shortcuts may be forwarded to the terminal as input instead of copying the xterm selection, and paste behavior differs by bracketed-paste mode.
|
||||
|
||||
## Required behavior
|
||||
|
||||
- Right-clicking a selected terminal region presents an xterm-aware Copy action that writes the selection to the system clipboard.
|
||||
- Browser native context menus remain available when no terminal output is selected.
|
||||
- Terminal keyboard input, including `Ctrl+C`, remains unchanged.
|
||||
- Copy requests expose the xterm selection as plain text.
|
||||
- Pasting plain text is handled once and follows bracketed-paste mode when enabled.
|
||||
- Normal terminal interrupts still work when there is no active selection.
|
||||
@@ -0,0 +1,6 @@
|
||||
# Web Terminal Clipboard Tasks
|
||||
|
||||
- [x] Add selected-text copy interception without suppressing unselected terminal interrupts.
|
||||
- [x] Normalize browser paste handling through the existing paste transport.
|
||||
- [x] Add focused frontend coverage for copy shortcut decisions.
|
||||
- [x] Run frontend tests, typecheck/build, lint, and diagnostics.
|
||||
@@ -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,18 +6,21 @@ 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.
|
||||
- Automatic refresh of an editor buffer that has already loaded a mounted file.
|
||||
|
||||
@@ -2,28 +2,30 @@
|
||||
|
||||
## 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
|
||||
|
||||
- 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,15 +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] Add desktop/mobile feedback for refresh outcomes.
|
||||
- [ ] Add focused resolver/service/API/frontend tests.
|
||||
- [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.
|
||||
- [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