fix: fleet restart adopts rebuilt base images through the canonical lifecycle path (#1860)

docs/memory/architecture.md

@@ -318,7 +318,7 @@ Vector 0.43.1 (`timberio/vector:0.43.1-alpine`). Captures all container stdout/s
 
 The agent server also runs two loops: the 15-min git `auto_sync` heartbeat (see [Git Sync Health](#git-sync-health-389390)) and the 5s liveness heartbeat (see [Heartbeat Liveness](#heartbeat-liveness-reliability-004-307)).
 
-**Durable subscription-token override (#1089):** `startup.sh` exports `CLAUDE_CODE_OAUTH_TOKEN` from `/var/lib/trinity/oauth-token` (when present, non-empty) **before** launching the agent server, so a token rotated via hot-reload survives a plain stop+start (a fleet restart via `routers/ops.py` does a raw `container_stop`+`container_start` that bypasses `start_agent_internal` and would otherwise revert to the baked `Config.Env` token). The path is deliberately on the writable layer, **not** under the persisted `/home/developer` volume: it survives `stop`→`start` (same container) but is wiped on recreate (fresh layer), so a DB-driven recreate cleanly re-bakes `Config.Env` from the DB and the stale override is gone — self-reconciling, no marker logic. Dir created+chowned to UID 1000 in the base-image Dockerfile.
+**Durable subscription-token override (#1089):** `startup.sh` exports `CLAUDE_CODE_OAUTH_TOKEN` from `/var/lib/trinity/oauth-token` (when present, non-empty) **before** launching the agent server, so a token rotated via hot-reload survives any plain stop+start of the same container (historically that included `routers/ops.py`'s fleet restart, which bypassed `start_agent_internal` entirely; since #1860 a fleet restart routes through `lifecycle.restart_agent_internal` — a no-drift agent keeps its container and the override survives, a drifted agent is recreated and cleanly re-bakes `Config.Env` from the DB). The path is deliberately on the writable layer, **not** under the persisted `/home/developer` volume: it survives `stop`→`start` (same container) but is wiped on recreate (fresh layer), so a DB-driven recreate cleanly re-bakes `Config.Env` from the DB and the stale override is gone — self-reconciling, no marker logic. Dir created+chowned to UID 1000 in the base-image Dockerfile.
 
 **Template-supplied pre-check** (SCHED-COND-001, #454): if the template ships an executable `~/.trinity/pre-check`, the backend's internal endpoint `POST /api/internal/agents/{name}/pre-check` runs it via `docker exec` before a cron-triggered chat. Language-agnostic — interpreter selected by shebang. The hook's stdout becomes the chat message; empty stdout + exit 0 records a skipped execution (Claude never invoked). Uses the same `execute_command_in_container` primitive as `git_service.py`, `ssh_service.py`, and the agent terminal — no agent-server HTTP endpoint.
 
@@ -1928,6 +1928,7 @@ CREATE INDEX idx_agent_reports_created ON agent_reports(created_at);  -- retenti
 - **Compatibility fix lock**: `compat_fix:{name}` (SET NX, 30s TTL) serialises the per-agent gitignore auto-fix read-modify-write (#668).
 - **Operator-queue sync keys** (#1632): `opqueue:leader` (SET NX, TTL `max(3×poll-interval, 30s)` floor — the single-syncing-worker lease, mirror `monitoring:leader`) and the create rate-limit windows `ratelimit:operator_queue_create:{agent}` + `ratelimit:operator_queue_create:_fleet` (ZSET, via `rate_limiter.check`, fail-open). Not `agent:*`-named, so the #1560 name-keyed registry doesn't apply; both fail open.
 - **Ephemeral-agent keys** (trinity-enterprise#69): `ephemeral:quota:{owner_id}` (owner-keyed atomic ghost-quota counter — deliberately not `agent:*`, so the #1560 name-keyed registry doesn't apply) and `ephemeral:discard:{name}` (SETNX+TTL discard lock). Both fail-open.
+- **Fleet-restart lock** (#1860): `ops:fleet_restart` — SETNX + 900s TTL single-flight guard for `POST /api/ops/fleet/restart` (409 on contention); the live loop refreshes its own lease per agent, release is compare-and-delete. Not `agent:*`-named (fleet-scoped, #1560 registry doesn't apply); fail-open when Redis is down.
 
 ---
 

docs/memory/feature-flows/agent-lifecycle.md

@@ -531,6 +531,22 @@ extracted-but-never-read variable, which is precisely why `unless-stopped` silen
 recreate. Read null-safely as `(old_host_config.get("RestartPolicy") or {}).get("Name")`: the key can exist
 with a `null` value, and `.get` on `None` would abort the recreate.
 
+**Canonical cold restart — `restart_agent_internal` (#1860)** (`src/backend/services/agent_service/lifecycle.py:469-488`, directly after `start_agent_internal` :257):
+
+```python
+async def restart_agent_internal(agent_name: str, *, stop_timeout: int = 30) -> dict
+```
+
+Explicit `container_stop(container, timeout=stop_timeout)` (:487), then the full `start_agent_internal` path (:488); returns its result dict unchanged. The explicit stop is **load-bearing**: the #1809 image-drift predicate runs only on a COLD start (`not was_already_running`), and starting a running agent is a deliberate idempotent no-op — so a restart that should adopt a rebuilt base image must stop first. A missing container falls through to `start_agent_internal`, which rebuilds it from persisted config (#1559, live ownership row) or 404s. This helper is the one home for the stop→start shape and the future home of #1817's per-agent start lock; `routers/system_agent.py`, `routers/systems.py`, and `services/subscription_auto_switch.py` still carry inline copies (consolidation deferred to #1817).
+
+**First consumer — `restart_fleet`** (`POST /api/ops/fleet/restart`, `src/backend/routers/ops.py:234-484`): each agent routes `container is None` guard → `restart_agent_internal(agent_name)` (:363-365) instead of the old raw `container_stop`+`container_start`, so a fleet restart applies pending config drift and adopts a rebuilt base image exactly like a manual stop → start. Semantics:
+
+- **Skips**: system agent (:302-312) AND ephemeral ghosts (:314-330) — the config-drift predicates are not ephemeral-gated, so a cold start could recreate a drifted ghost and destroy its volume-less workspace (ent#69 "ghosts never recreate"); GC owns ghost lifecycle. Pre-existing `filter_status` / `system_prefix` / not-running skips unchanged.
+- **Principals**: `reject_agent_principal` beside `assert_admin` (:256-260) — the endpoint now *replaces* containers, and escalating a handler's destructiveness re-prices every principal that could already reach it (Invariant #8, the #1816 rule).
+- **Single-flight**: Redis SETNX lock `ops:fleet_restart` (:42-43, :262-277) — a second caller gets 409 `fleet_restart_in_progress`; 900s TTL bounds a crashed holder, the live loop refreshes its own lease each iteration (:296-300), release is compare-and-delete of the holder's own token (:437-444); fail-open when Redis is down. Guards the client-timeout-retry race (a retried POST interleaving stop/start with the still-running first loop — the #799/#1817 wedge class).
+- **Result surfacing**: per-agent entries explicitly copy `recreated` / `recreate_reason` / `credentials_injection` / `skills_injection` (:369-377) — the #1809 allowlist learning: this router builds its own result dicts, service fields do NOT flow through on their own — and `summary.recreated` counts replacements (:481). Failure rows never carry a raw exception message — HTTPException `.detail` (platform-authored) or the exception class name only, full text + traceback in the backend log (`py/stack-trace-exposure`, :396-418); a failed recreate that leaves NO container gets an actionable error naming `POST /api/agents/{name}/start` (#1559) as the recovery path (:414-418).
+- **Audit as durable record**: a partial-safe `fleet_restart` entry (`AuditEventType.SYSTEM`) is written in `finally` (:450-473) with totals + a per-agent `recreated: {name: reason}` map (:465) — the sequential loop can outlast proxy timeouts (nginx 60s / Cloudflare ~100s), so a client 504/524 does NOT mean the restart failed; the outcome lands here. `invalidate_context_stats_cache()` (:436) and the lock release run before the awaited audit call, so a shutdown `CancelledError` at the await can't leave the lock held for a full TTL.
+
 **Authentication Model** (Updated 2026-03-03 for SUB-002):
 
 Claude Code checks credentials in **priority order** (highest first):
@@ -1028,6 +1044,8 @@ await log_audit_event(
 - [ ] Delete any remaining test agents
 - [ ] `docker ps -a | grep test-` - verify no orphans
 
+**Unit tests (#1860)**: `tests/unit/test_1860_fleet_restart_adoption.py` — 16 mocked tests for the fleet-restart path (adoption surfaced per agent, failure isolation, system/ephemeral skips, agent-principal 403, single-flight 409, audit `recreated` map, fail-open lock paths).
+
 ---
 
 **Last Updated**: 2026-03-26
@@ -1040,6 +1058,7 @@ await log_audit_event(
 
 | Date | Changes |
 |------|---------|
+| 2026-07-31 | **#1860 — fleet restart adopts a rebuilt base image**: new canonical cold-restart helper `restart_agent_internal(agent_name, *, stop_timeout=30)` (`lifecycle.py:469-488` — explicit stop is load-bearing so the #1809 cold-start-only drift predicate can fire); `POST /api/ops/fleet/restart` routes each agent through it instead of raw stop+start, skips ephemeral ghosts (ent#69), adds `reject_agent_principal`, a single-flight `ops:fleet_restart` SETNX lock (409, 900s TTL, compare-and-delete release), explicit `recreated`/`recreate_reason`/injection field copy + `summary.recreated`, and a partial-safe `fleet_restart` audit entry written in `finally` with a per-agent `recreated: {name: reason}` map. Inline stop→start copies in `system_agent.py`/`systems.py`/`subscription_auto_switch.py` consolidate under #1817. See "Canonical cold restart" above. |
 | 2026-07-28 | **#1816 — base-image adoption for `trinity-system`**: `check_base_image_matches` split into a 3-state `check_base_image_state` core + an unchanged boolean wrapper (`state != "drift"`); `check_full_capabilities_match` made system-aware and `recreate_container_with_updated_config` gained a keyword-only `full_capabilities` override behind one shared `helpers.is_system_agent_name()`; `_provision_folders_and_run_agent_container` gained a keyword-only `restart_policy` and the recreate now carries `unless-stopped` forward (it was silently dropped); `start_agent_internal` gained the structural AC2 gate (`is_system AND was_already_running` ⇒ `recreate_deferred="system_agent_running"`, surfaced through `routers/agents.py`). See [internal-system-agent.md](internal-system-agent.md) → Base-image adoption. |
 | 2026-07-18 | **#1484 — `create_agent_internal` decomposition (no behavior change)**: the CC-187 / 945-SLOC monolith was cut into a thin orchestrator + fenced private phase-helpers (each CC<20 / <100 SLOC), reading a single orchestrator-populated `_RollbackHandles` dataclass; the `if docker_client: try/except/else` and CC-trivial gates stay inline. Signature/callers unchanged. Characterization-tests-first (`tests/unit/test_1484_create_agent_characterization.py`, 40 tests) pin the byte-identical contract. See `architecture.md` (Services → `agent_service/crud.py`, Network Topology `_create_agent_container`). The numbered "Business Logic" line references above are now stale (helper-relocated). |
 | 2026-03-26 | **Line number refresh + Trinity injection removal**: Updated all line numbers across agents.py (647 lines), lifecycle.py (403 lines), crud.py (552 lines), helpers.py (467 lines), terminal.py (320 lines). Removed all `inject_trinity_meta_prompt()` references and AgentClient injection code (Issue #136). Updated startup injection order to: Credentials, Skills, Read-Only Hooks. Added documentation for bulk endpoints (context-stats :157, execution-stats :163, autonomy-status :232, slots :240), queue endpoints (queue :537, clear :546, release :555), and activity endpoints (activities :568, timeline :594). Updated delete flow for EVT-001 event subscriptions (:381) and AVATAR-002 emotion image cleanup (:410-425). |

docs/memory/feature-flows/internal-system-agent.md

@@ -281,7 +281,7 @@ The dedicated `SystemAgent.vue` has been removed. The system agent now uses the
 |----------|--------|-------------|
 | `/api/ops/fleet/status` | GET | All agents with status, context, activity |
 | `/api/ops/fleet/health` | GET | Health summary with critical/warning issues |
-| `/api/ops/fleet/restart` | POST | Restart all/filtered agents |
+| `/api/ops/fleet/restart` | POST | Restart all/filtered running agents through the canonical stop→`start_agent_internal` path — applies pending config drift and adopts a rebuilt base image (#1860). Skips system + ephemeral agents; agent-scoped keys 403 (`reject_agent_principal` — system-scoped keys unaffected); 409 while another fleet restart is in flight; outcome durably recorded in the audit log (`fleet_restart`) even if the client times out |
 | `/api/ops/fleet/stop` | POST | Stop all/filtered agents |
 
 ### Schedule Control

docs/memory/feature-flows/mobile-admin-pwa.md

@@ -39,6 +39,7 @@ Browser (/m)
 - **Manual PWA** (no vite-plugin-pwa): Service worker + manifest injected dynamically in `onMounted`, cleaned up in `onUnmounted`. Follows Sparky PWA reference pattern
 - **Dark-only**: Forces `dark` class on `<html>`, OLED-friendly `#111827` background
 - **401 handling**: `main.js` axios interceptor skips redirect to `/login` when on `/m`
+- **Restart All is a real upgrade path (#1860)**: `POST /api/ops/fleet/restart` routes each running agent through the canonical stop→`start_agent_internal` path, so it applies pending config drift and adopts a rebuilt base image (container ids change). A slow fleet can outlast the proxy timeout — the loop keeps running server-side and the outcome lands in the audit log (`fleet_restart`); a second tap while one is in flight gets 409 `fleet_restart_in_progress`
 
 ## Files
 

docs/memory/learnings.md

@@ -196,3 +196,11 @@ plan or review. `/autoplan` reads this before planning; write for that reader.
 ## 2026-07-31 — pitfall — A multi-arm auth guard must select its arm from the AUTHENTICATED PRINCIPAL; selecting it from a request header collapses the guard to its weakest arm
 **Context**: ent#265 review. The new provenance guard on `parent_execution_id` inheritance had two arms — "agent caller must BE the parent's executing agent" vs "human caller must have access to the parent's agent" — and chose between them on the presence of the `X-Source-Agent` header. But the SELF-EXEC-001 spoof guard (`derive_source_and_trigger`) only fires when `current_user.agent_name` is set, so for a human caller that header is unvalidated client input. Any user could set it to the parent execution's own agent name — a value the row itself discloses — satisfy the agent arm trivially, and skip the human arm entirely, routing their task's completion report into an agent's Telegram DM / Slack thread via a bot binding they don't own. `routers/chat.py` had ALREADY documented this exact trap for the resume-session IDOR ("x_source_agent is a raw client header; a regular user could otherwise set it to skip the gate entirely") — the header-keyed guard was written a few hundred lines away from the comment warning against it. The second half was scope: the human arm used `can_user_access_agent` (owner|shared|admin) for what is a *proactive-send* capability, while every other proactive surface is owner-gated (`OwnedAgentByName`) or per-recipient-consented (#321), and a share recipient can already read the owner's execution ids from the accessor-scoped `GET /api/executions`.
 **Lesson**: (1) Arm selection in an auth guard is itself an auth decision — key it on `current_user` (`agent_name` / `connector_agent` / role), never on a header, even one that "is validated elsewhere": validation that is conditional on the principal cannot be relied on to classify the principal. (2) When a request field is spoofable for *some* principals, treat it as spoofable everywhere and use it for logging only. (3) Pick the tier of a new gate by the **capability being granted**, not by the endpoint it hangs off: if the outcome is "write into a channel the owner controls", match the owner-only tier the other write paths to that channel already use. (4) Every arm of a multi-arm guard needs its own test with a principal that can ONLY take that arm — a test that models an agent caller by setting a header instead of `User.agent_name` silently exercises the wrong arm and goes green.
+
+## 2026-07-31 — pitfall — The lifecycle's ephemeral guards cover image-drift + credential injection ONLY; any new cold-start caller must skip ghosts itself
+**Context**: #1860 fleet restart. The plan assumed "start_agent_internal already guards ephemeral" and both independent reviewers refuted it: the 8 config-drift predicates (lifecycle.py `needs_recreation`) have no ephemeral gate, so a cold start of a config-drifted ghost recreates it — destroying its volume-less workspace mid-budget (ent#69 "ghosts never recreate").
+**Lesson**: a bulk/new caller that cold-starts agents through `start_agent_internal` (or `restart_agent_internal`) must itself exclude `is_ephemeral` rows, the way heartbeat watch and fleet health already do. Do not infer "the lifecycle guards ghosts" from the two guards that exist.
+
+## 2026-07-31 — pitfall — In a `finally` that both awaits and cleans up, do sync cleanup FIRST: `except Exception` does not catch CancelledError, so an awaited call mid-finally skips everything after it
+**Context**: #1860 restart_fleet's finally block ordered audit-await → cache-invalidate → lock-release; a backend deploy mid-loop cancels the handler, the audit await re-raises CancelledError (a BaseException), and the lock stayed held for a full TTL after every deploy.
+**Lesson**: order shutdown-sensitive `finally` blocks sync-cleanup-first, awaited-best-effort-last, and remember `except Exception` around an `await` is not cancellation-safe (#1804's shutdown handlers are the same class).

docs/security-reports/cso-diff-2026-07-31-1860-fleet-restart.json

@@ -0,0 +1,65 @@
+{
+  "version": "1.1",
+  "date": "2026-07-31",
+  "mode": "daily",
+  "scope": "diff",
+  "branch": "feature/1860-fleet-restart-adoption",
+  "base": "dev@5b289996",
+  "issue": "abilityai/trinity#1860",
+  "phases_run": [
+    "P0",
+    "P1",
+    "P2-diff",
+    "P5-lock",
+    "P6",
+    "P7",
+    "P9-A01/A03/A09",
+    "P12",
+    "P13",
+    "P14"
+  ],
+  "attack_surface": {
+    "new_endpoints": 0,
+    "hardened_endpoints": [
+      "POST /api/ops/fleet/restart"
+    ],
+    "new_auth_gates": [
+      "reject_agent_principal (ops.py:260)"
+    ],
+    "new_redis_keys": [
+      "ops:fleet_restart (SETNX single-flight, backend ACL user)"
+    ],
+    "new_inputs": 0
+  },
+  "findings": [],
+  "filter_stats": {
+    "raw_candidates": 4,
+    "discarded_by_rule": {
+      "fail-open-lock (platform convention)": 1,
+      "error-text-to-admin (existing practice)": 1,
+      "dos-excluded (sequential duration)": 1,
+      "ttl-extension-benign (expire w/o token)": 1
+    }
+  },
+  "totals": {
+    "critical": 0,
+    "high": 0,
+    "medium": 0,
+    "low": 0
+  },
+  "verification": {
+    "live": [
+      "agent-key 403",
+      "concurrent 409",
+      "audit row content",
+      "image adoption 3/3",
+      "no-drift idempotence",
+      "config-drift single-agent"
+    ],
+    "unit": "16/16 tests/unit/test_1860_fleet_restart_adoption.py"
+  },
+  "trend": {
+    "note": "first diff-scope report for this branch; verdict PASS, net hardening"
+  },
+  "verdict": "PASS"
+}