Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/memory/architecture/reliability.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
## 2026-09-24 — pitfall — A second Trinity stack on the same Docker daemon reads every other stack's agent volumes as orphans
**Context**: trinity-enterprise#465's local demo. `cleanup_service._sweep_orphan_agent_volumes` (#1581) reclaims an `agent-*` volume that (1) no ownership row claims, (2) no container mounts, (3) for three consecutive cycles, past a 1h creation grace — up to 100 per cycle, force-removed. All three conditions are evaluated against the stack's OWN database and the daemon's WHOLE volume list. A stack booted with a fresh database (a demo, a `--keep` verify run, a second worktree's dev stack) owns no rows, so every labelled agent volume on the host that nothing currently mounts is an orphan to it; on the developer machine where this surfaced that was 207 of 238. There is no setting that turns the sweep off, and `/verify-local`'s overlay does not neutralise it — a run that stays up past ~15 minutes is inside the window.
**Lesson**: Docker volumes are host-global state; a sweep that decides "unowned" from a per-stack database is only safe when that database is the only one on the daemon. Before booting any second stack on a shared daemon, count `docker volume ls --filter label=trinity.agent-name` minus the attached ones — if it is non-zero, disable that sweep for the extra stack (a demo-only `sitecustomize` import hook that no-ops the method works without touching the branch) or scope the sweep to volumes this stack created. The structural fix is a stack-identity label on every agent volume, checked by the sweep before it reclaims anything.
**Follow-up**: filed and fixed as trinity#3214 — agent data volumes now carry `trinity.instance=<installation_id>` and the sweep reclaims only its own; pre-#3214 (unlabelled) volumes are never auto-reclaimed (named in a WARNING instead). The manual workaround above is still needed for volumes created before that fix.
18 changes: 5 additions & 13 deletions src/backend/services/agent_service/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
get_agent_status_from_container,
)
from services.docker_utils import (
volume_get, volume_create, containers_run, container_remove
volume_get, volume_create, containers_run, container_remove,
agent_volume_labels
)
from services.agent_runtime_state import clear_agent_breakers, clear_agent_runtime_state
from services.template_service import (
Expand Down Expand Up @@ -2053,10 +2054,7 @@ async def _workspace_volume_mount(config: AgentConfig, volumes: dict) -> None:
except docker.errors.NotFound:
await volume_create(
name=agent_volume_name,
labels={
'trinity.platform': 'agent-workspace',
'trinity.agent-name': config.name
}
labels=agent_volume_labels(config.name, 'agent-workspace'),
)
volumes[agent_volume_name] = {'bind': '/home/developer', 'mode': 'rw'} # Persistent workspace

Expand Down Expand Up @@ -2089,10 +2087,7 @@ async def _shared_folder_mounts(
except docker.errors.NotFound:
await volume_create(
name=shared_volume_name,
labels={
'trinity.platform': 'agent-shared',
'trinity.agent-name': config.name
}
labels=agent_volume_labels(config.name, 'agent-shared'),
)
volume_created = True

Expand Down Expand Up @@ -2136,10 +2131,7 @@ async def _public_volume_mount(config: AgentConfig, volumes: dict) -> None:
except docker.errors.NotFound:
await volume_create(
name=public_volume_name,
labels={
'trinity.platform': 'agent-public',
'trinity.agent-name': config.name,
},
labels=agent_volume_labels(config.name, 'agent-public'),
)
public_volume_created = True

Expand Down
7 changes: 2 additions & 5 deletions src/backend/services/agent_service/deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
collect_mcp_credential_warnings,
)
from services.docker_service import get_agent_container
from services.docker_utils import container_stop, container_start
from services.docker_utils import container_stop, container_start, agent_volume_labels
from utils.helpers import sanitize_agent_name
from services.settings_service import get_agent_quota_for_role
from redis_breaker_util import SingleFlightLock # #1920 shared single-flight lock
Expand Down Expand Up @@ -544,10 +544,7 @@ def _ensure_fresh_workspace_volume(client, workspace_vol: str, version_name: str

client.volumes.create(
name=workspace_vol,
labels={
"trinity.platform": "agent-workspace",
"trinity.agent-name": version_name,
},
labels=agent_volume_labels(version_name, "agent-workspace"),
)


Expand Down
13 changes: 4 additions & 9 deletions src/backend/services/agent_service/lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
)
from services.docker_utils import (
container_stop, container_remove, container_start, container_reload,
volume_get, volume_create, containers_run, image_get, container_stop
volume_get, volume_create, containers_run, image_get, container_stop,
agent_volume_labels
)
from services.agent_service.helpers import validate_base_image
from services.agent_runtime_state import clear_agent_breakers
Expand Down Expand Up @@ -1379,10 +1380,7 @@ async def _provision_folders_and_run_agent_container(
except docker.errors.NotFound:
await volume_create(
name=shared_volume_name,
labels={
'trinity.platform': 'agent-shared',
'trinity.agent-name': agent_name
}
labels=agent_volume_labels(agent_name, 'agent-shared'),
)
volume_created = True

Expand Down Expand Up @@ -1421,10 +1419,7 @@ async def _provision_folders_and_run_agent_container(
except docker.errors.NotFound:
await volume_create(
name=public_volume_name,
labels={
'trinity.platform': 'agent-public',
'trinity.agent-name': agent_name,
},
labels=agent_volume_labels(agent_name, 'agent-public'),
)
public_volume_created = True

Expand Down
99 changes: 94 additions & 5 deletions src/backend/services/cleanup_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,9 @@ def __init__(self, poll_interval: int = CLEANUP_INTERVAL_SECONDS):
# unowned. In-process only (a restart just restarts the count — the
# safe direction).
self._unattached_volume_strikes: Dict[str, int] = {}
# #3214: the legacy unlabelled orphan volumes last named in a WARNING,
# so the report repeats only when that set changes.
self._reported_unlabelled_volumes: frozenset = frozenset()

def start(self):
"""Start the background cleanup loop."""
Expand Down Expand Up @@ -1821,15 +1824,44 @@ async def _sweep_orphan_agent_volumes(self, report: CleanupReport) -> None:
Removal still goes through the name+label guard (`remove_agent_volumes`)
as a last line of defence. Also carries a creation-grace window
(creation writes the volume before the ownership row).

**Stack scope (#3214).** Docker volumes are daemon-global while (1) is
answered from THIS stack's database, so a second stack on the same
daemon used to read every other stack's unattached agent volumes as
its own orphans and force-remove them. Candidates are therefore listed
on the `trinity.instance=<this installation_id>` label (key AND value):
a volume another stack created is never a candidate. If this stack's
id cannot be resolved, the sweep does nothing.

**Legacy volumes — fail-closed.** Volumes created before the instance
label existed carry none, and Docker labels cannot be added later. The
sweep NEVER reclaims an unlabelled volume: on a shared daemon it cannot
say whose it is. Unlabelled ones that are also unowned and unattached
are named in one WARNING (repeated only when that set changes) for a
human to remove. This stack's own agents still lose their volumes at
the retention purge, which is driven by the ownership row, labelled or
not.
"""
try:
from services.docker_utils import (
AGENT_VOLUME_INSTANCE_LABEL,
list_agent_data_volumes,
list_attached_volume_names,
remove_agent_volumes,
)
from services.instance_identity import get_instance_id

volumes = await list_agent_data_volumes()
instance_id = get_instance_id()
if not instance_id:
logger.warning(
"[#1581] skipping orphan-volume sweep: this instance's id is "
"unavailable (cannot tell its volumes from another stack's)"
)
return

await self._report_unlabelled_orphan_volumes(AGENT_VOLUME_INSTANCE_LABEL)

volumes = await list_agent_data_volumes(instance_id)
if not volumes:
self._unattached_volume_strikes.clear()
return
Expand Down Expand Up @@ -1900,22 +1932,79 @@ async def _sweep_orphan_agent_volumes(self, report: CleanupReport) -> None:
for stale in set(self._unattached_volume_strikes) - seen_volumes:
self._unattached_volume_strikes.pop(stale, None)

for volume_base in orphan_agents:
reclaimed_bases = []
for volume_base in sorted(orphan_agents):
try:
reclaimed += await remove_agent_volumes(volume_base)
n = await remove_agent_volumes(volume_base, instance_id)
reclaimed += n
if n:
reclaimed_bases.append(volume_base)
except Exception as e:
logger.warning(
f"[#1581] orphan volume reclaim for {volume_base} failed: {e}"
)

report.orphan_agent_volumes_reclaimed = reclaimed
if reclaimed > 0:
logger.info(
f"[#1581] reclaimed {reclaimed} orphaned agent volume(s)"
# Always WARNING (#3214, the #1638 rule the purge path already
# follows): this destroys agent data and is unrecoverable —
# there is no routine, unremarkable one. Each volume is also
# named on its own line by `remove_agent_volumes`.
logger.warning(
f"[#1581] reclaimed {reclaimed} orphaned agent volume(s) of "
f"this instance — unrecoverable: "
+ ", ".join(f"agent-{b}-*" for b in reclaimed_bases)
)
except Exception as e:
logger.error(f"[Cleanup] Error in orphan agent-volume sweep: {e}")

async def _report_unlabelled_orphan_volumes(self, instance_label: str) -> None:
"""#3214: name the legacy (unlabelled) agent volumes that look orphaned
— no ownership row, not mounted — for a human. Never reclaims: without
an instance label nothing says which stack's data a volume holds.

One WARNING per change of that set, not one per 5-minute cycle. Never
raises.
"""
try:
from services.docker_utils import (
list_all_agent_data_volumes,
list_attached_volume_names,
)

volumes = await list_all_agent_data_volumes()
attached = await list_attached_volume_names()
if attached is None:
return
names = []
for volume in volumes:
labels = (volume.attrs.get("Labels") or {}) if getattr(volume, "attrs", None) else {}
if labels.get(instance_label):
continue
base = labels.get("trinity.agent-name")
name = getattr(volume, "name", None)
if not base or not name or name in attached:
continue
if db.is_volume_base_reserved(base):
continue
names.append(name)
current = frozenset(names)
if current == self._reported_unlabelled_volumes:
return
self._reported_unlabelled_volumes = current
if current:
shown = sorted(current)
logger.warning(
f"[#3214] {len(shown)} unlabelled agent volume(s) with no owner "
f"and no container — NOT reclaimed automatically (created "
f"before stack-identity labels; on a shared Docker daemon they "
f"may belong to another stack). Review and remove by hand: "
+ ", ".join(shown[:50])
+ (f" … (+{len(shown) - 50} more)" if len(shown) > 50 else "")
)
except Exception as e:
logger.debug(f"[#3214] unlabelled-volume report skipped: {e}")

@staticmethod
def _volume_age_seconds(created_raw: Optional[str], now) -> Optional[float]:
"""Age in seconds of a Docker volume from its ``CreatedAt`` string.
Expand Down
Loading
Loading