Skip to content

bug: the #1581 orphan agent-volume sweep is instance-blind — a second stack on the same Docker daemon force-removes other stacks' agent volumes and logs it at INFO as a successful reclaim #3214

Closed

Description

@vybe

Summary

cleanup_service._sweep_orphan_agent_volumes (the #1581 orphan reclaim) decides "nobody owns this volume" from one stack's database while enumerating the whole Docker daemon's volumes. Nothing on an agent data volume says which Trinity stack created it, so a second stack on a shared daemon reads every other stack's unattached agent volumes as its own orphans and force-removes them — up to 100 per cycle, unrecoverably, logged at INFO as a successful reclaim.

This is the next failure mode of the same sweep as #1664 (which fixed the rename case against the same guard). It is already documented as a pitfall in this repo — docs/memory/learnings/2026-09-24-a-second-stack-reads-other-stacks-volumes-as-orphans.md, where a freshly-booted second stack on a developer machine counted 207 of 238 labelled agent volumes as orphans — but no issue was ever filed for the fix the learning names.

Mechanism (verified against dev @ 918e0b0)

  1. Enumeration is daemon-wide and unscoped. docker_utils.list_agent_data_volumes() (src/backend/services/docker_utils.py:530-547) calls volumes.list(filters={"label": _AGENT_VOLUME_LABEL_KEY}) — the label key trinity.agent-name, never a key+value scoped to this stack. Its own docstring: "All agent data volumes across the fleet (any agent)".
  2. Ownership is answered from the local DB only. _sweep_orphan_agent_volumes (cleanup_service.py:1792-1917) asks db.is_volume_base_reserved(volume_base) (:1867). A foreign stack's agent is not in this stack's agent_ownership, so condition (1) "no owner" is satisfied for every foreign volume.
  3. The remaining conditions don't distinguish foreign from dead. Unattached (:1873), ≥1h old (ORPHAN_VOLUME_GRACE_SECONDS = 3600, :407), unattached for 3 consecutive cycles (ORPHAN_VOLUME_UNATTACHED_STRIKES = 3, :415) at CLEANUP_INTERVAL_SECONDS = 300 (:338) → 3 × 300 s = 15 min. All three are properties of a foreign volume too.
  4. The last-line guard passes. is_reclaimable_agent_volume (docker_utils.py:364-392) checks name + trinity.agent-name + trinity.platform — all three written by the same code on every instance, so a foreign volume is internally consistent and the guard says yes. Its docstring says exactly why that is not a defence: "this checks that the volume's identity is INTERNALLY CONSISTENT … NOT evidence that the data is dead. … Liveness is the caller's burden." The caller discharges that burden against the wrong database.
  5. Removal is forced and the outcome reads as housekeeping. remove_agent_volumes → volume_remove(volume, force=True) (docker_utils.py:430), bounded by ORPHAN_VOLUME_RECLAIM_PER_CYCLE = 100 (:406).
  6. No off switch. The sweep is called unconditionally at cleanup_service.py:921 — no setting, no env flag; the learning doc also records that /verify-local's overlay does not neutralise it.

What limits the blast radius (and what doesn't)

list_attached_volume_names (docker_utils.py:445-477) uses containers.list(all=True) daemon-wide and unfiltered by label — deliberately, per its docstring. So a foreign volume mounted by any container, running or stopped, is protected. The exposed set is foreign agent volumes with no container at all:

  • a soft-deleted agent inside its recovery window on the other stack — soft-delete removes the container and keeps the volume (agent_service/lifecycle.py:381-384, :1547), which is precisely the recovery window this destroys;
  • an agent whose container was removed for a recreate/rebuild that didn't finish within 15 minutes;
  • any stack torn down with down (containers gone) but volumes kept.

On a developer host those accumulate, which is why the observed count was 207 of 238.

Second, separable defect: the destructive case is logged one level below the identical non-destructive-looking one

The retention purge removes the same volumes through the same function and deliberately logs WARNING, with a comment explaining why (cleanup_service.py:1781-1788):

# Always WARNING (#1638): unlike a row prune, this destroys the agent's data volumes (#1581) and is unrecoverable — there is no such thing as a routine, unremarkable one.

The orphan sweep's own line for the same act (:1912-1914) is:

logger.info(f"[#1581] reclaimed {reclaimed} orphaned agent volume(s)")

plus logger.info(f"[#1581] removed agent volume {vol_name}") per volume (docker_utils.py:432). An operator reading logs after the fact sees routine housekeeping, not unrecoverable deletion of data — so the wrong-stack case leaves no trace that reads as trouble.

Why this is worth fixing rather than documenting

The configuration is not shipped (docker-compose.sibling.yml is Redis-only by design; the ent#623 cross-instance demo puts the second stack in docker:dind specifically to avoid this — its header comment names the 15-minute window), but it is not hypothetical either: it has been hit once on a developer machine, and it arises from ordinary developer actions — a second worktree's dev stack, a --keep verify run left up, any local two-instance demo. The dind in that demo script is therefore load-bearing safety, not convenience — the exposure appears the moment someone writes a trimmed, daemon-sharing variant of it.

Proposed fix

The learning doc already names it: a stack-identity label on every agent volume, checked by the sweep before it reclaims anything.

  • Stamp a stack identifier (trinity.instance or similar) at volume creation — agent_service/crud.py:2050-2055, :2088, :2135, agent_service/deploy.py:549, system_agent_service.py:451, lifecycle.py:1384, :1426, :1738. services/instance_identity.py already resolves a durable per-install identity (TRINITY_INSTANCE_NAME → FRONTEND_URL host label → installation_id[:8]) for Canary Slack alerts don't say which instance fired them #1987's webhook attribution; reuse it rather than minting a second notion. Note it is deliberately human-label shaped and can be absent — the sweep needs an identifier that is never ambiguous, so installation_id is probably the right field even where the alert path prefers the pretty name.
  • Filter the sweep's enumeration on key and value, so a foreign volume is never a candidate in the first place.
  • The decision this needs a ruling on: pre-existing volumes carry no such label. Fail-closed (never reclaim an unlabelled volume) makes the sweep inert for every volume created before the fix — which is the entire current population. The alternative is a boot-time backfill that stamps the label on volumes this stack's agent_ownership already claims, leaving genuinely unlabelled-and-unclaimed volumes for a human. Pick one explicitly; silently keeping the current behaviour for legacy volumes preserves the bug.
  • Raise the orphan sweep's success log to WARNING with the purge path's "unrecoverable" wording, and name the volumes removed.

Acceptance criteria

  1. Every agent data volume created by any code path carries a stack-identity label derived from one source of instance identity (no second notion invented).
  2. list_agent_data_volumes (or the sweep's call to it) filters on label key and value; a volume belonging to another stack is not returned to the sweep. Test: a volume labelled with a foreign instance id, unattached, 2h old, surviving 3+ cycles, is not removed.
  3. The legacy-volume policy is decided, implemented and stated in the sweep's docstring — fail-closed or backfill, not "unchanged".
  4. The orphan sweep's reclaim log is WARNING, says "unrecoverable", and names the volumes removed; the per-volume line in remove_agent_volumes matches.
  5. Regression test for the original bug: Docker volumes are never deleted — volume_remove has zero callers; agent purge leaks workspace/public/shared volumes forever #1581 case still passes (a genuinely orphaned volume of this stack is still reclaimed).
  6. docs/memory/learnings/2026-09-24-a-second-stack-reads-other-stacks-volumes-as-orphans.md gets a line pointing at this issue and, once fixed, at the fix.

Provenance and bounds

Routed by corbin /daily-intake 2026-10-04 (D0, evidence-only) from risk-warden's RW-032 (score 8 = L2 × I4, exec pqycSL1cZg_055CX1CwboA), found incidentally while reading scripts/dev/aauth_prototype/demo.sh.

Verified first-hand here against abilityai/trinity@918e0b0d (dev). Corrections to the routed report, which relayed its line numbers as explicitly unverified:

  • The docker_utils.py pointer was accurate: list_agent_data_volumes is 530-547, filter at 543.
  • cleanup_service.py:1477 is not the ownership check; it is unrelated health-check retention. The sweep is 1792-1917, the DB ownership check 1867, the removal loop 1903-1908.
  • The report omitted the 1h creation grace, and omitted that the attached check is daemon-wide and all=True — which narrows the exposed set to container-less foreign volumes (above). Impact is narrower than a flat "a second instance can have its agent volumes force-removed", and the 207/238 observation shows the narrowed set is still the majority on a real host.
  • The report scored likelihood L2 on the premise that "nothing observed is in that configuration today". That premise is wrong: the configuration was observed, on 2026-09-24, and is in this repo's own pitfall log. Likelihood is a recurring developer-machine hazard with a documented manual workaround, not an unobserved one.
  • No exposure count is claimed for any production host. P2 rather than P1 because no shipped or hosted topology puts two backends on one daemon; the thing that would raise it is a trimmed, daemon-sharing variant of the cross-instance demo script.

Activity

  1. added
    dockerDocker base image dependency
    complexity-mediumComplexity: medium (board points 5-8)
    status-readyGreenlit and ready for development (vetted; counterpart to status-incubating)
    on Oct 4, 2026
  2. removed
    status-readyGreenlit and ready for development (vetted; counterpart to status-incubating)
    on Oct 5, 2026
  3. dolho commented on Oct 5, 2026

    @dolho
    Contributor

    /claim

  4. github-actions commented on Oct 5, 2026

    @github-actions

    Claimed by @dolho. Label status-in-progress added.

    Next steps:

    1. Create branch: feature/3214-<short-slug>
    2. Implement the feature
    3. Open a PR with Fixes #3214
  5. added a commit that references this issue on Oct 5, 2026
  6. added a commit that references this issue on Oct 5, 2026
  7. added
    status-in-devMerged to dev, awaiting release cut to main
    and removed on Oct 5, 2026
  8. added a commit that references this issue on Oct 20, 2026
  9. removed
    status-in-devMerged to dev, awaiting release cut to main
    on Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions