feat(soft-delete): agent_ownership soft-delete + retention purge (#834 Phase 1a) by dolho · Pull Request #838 · Abilityai/trinity

dolho · 2026-05-14T09:44:36Z

Summary

DELETE /api/agents/{name} is no longer destructive. Marks agent_ownership.deleted_at, preserves every child row (sharing, schedules, chat history, permissions, credentials, MCP key, workspace volume), and runs the hard-purge from a configurable retention sweep (default 30 days, agent_soft_delete_retention_days in system_settings; 0 disables).

Scope: agent_ownership only. Phase 1b will extend to agent_schedules / users / agent_shared_files / agent_sessions / chat_sessions once this pattern is validated.

Changed behavior

Action	Before	After
`DELETE /api/agents/X`	Wipes ~11 child tables + agent_ownership row + workspace volumes	Stops + removes container; marks `deleted_at`; keeps everything else
`GET /api/agents/X` (deleted)	404	404 (transparency preserved)
`GET /api/agents`	Excludes deleted	Excludes deleted (filter now driven by `deleted_at IS NULL`)
`POST /api/agents` with deleted-but-not-purged name	Would clobber the row	409 — name reserved until purge
Cleanup service	No agent-row prune	Daily sweep: agents with `deleted_at` past retention → `purge_agent_ownership()` (cascade via #816)

Read-path audit

35 SELECT/JOIN sites against agent_ownership filtered with WHERE deleted_at IS NULL. The 4 unfiltered sites are intentional and commented:

purge_agent_ownership internals — need to see the soft-deleted row
rename_agent uniqueness check on destination name — name reservation
canary/snapshot.py _collect_agent_settings — known_agents set drives L-03; treating soft-deleted-pending-purge agents as "unknown" would surface their preserved child rows as false-positive orphans

New helper is_agent_name_reserved() (unfiltered companion to get_agent_owner()) caught a real bug during smoke testing: without it, the create flow's existence check at crud.py:95 returns False for a soft-deleted name (because get_agent_owner now filters), the create proceeds, then the SQL INSERT hits the UNIQUE constraint half-way through container provisioning. The unfiltered check at the same site rejects the create up-front with a clean 409.

Live verification (uvicorn auto-reload on the running stack)

1. CREATE              → status=stopped ✓
2. DELETE              → "Agent deleted" message ✓
3. GET after delete    → 404 ✓
4. CREATE same name    → 409 (reserved) ✓
5. DB state            → deleted_at set, child rows preserved ✓
6. purge_agent_ownership() → True (cascade fires) ✓
7. After purge         → all child rows + parent gone ✓
8. CREATE same name    → 200 (name freed) ✓

#816 registry (vendored)

PR #829 was closed — its standalone-cascade approach is superseded by this PR's soft-delete pattern (the registry still ships, now firing at purge time, not at delete time). This PR is now the canonical source of src/backend/db/agent_cleanup.py.

Schema

agent_ownership.deleted_at TEXT (NULL = live)
Partial index idx_agent_ownership_deleted_at ON agent_ownership(deleted_at) WHERE deleted_at IS NOT NULL — keeps the retention sweep scan cheap as the live fleet grows
Versioned migration in db/migrations.py:_migrate_agent_ownership_soft_delete

Out of scope (later)

Phase 1b: extend to agent_schedules, users, agent_shared_files, agent_sessions, chat_sessions. Each as a separate PR.
Admin endpoint: list + recover soft-deleted agents from the UI. Today an operator does it via direct DB UPDATE agent_ownership SET deleted_at = NULL WHERE agent_name = X.
Container recreate-on-recover: preserved workspace volume + fresh container after recovery. Today recovery is metadata-only.

Test plan

Migration runs cleanly on backend reload — deleted_at column + partial index present
Soft-delete sets deleted_at, leaves child rows intact
GET /api/agents/{name} returns 404
GET /api/agents excludes soft-deleted
Recreate with same name returns 409
purge_agent_ownership() cascades + drops parent row
Name frees after purge
Unit tests in tests/unit/test_agent_soft_delete.py cover soft-delete, idempotency, purge, name-reservation, retention-cutoff math (CI will run them)
CI full suite (lint + 6-seed pytest matrix + regression diff)

Related to #834. Phase 1a only.

🤖 Generated with Claude Code

CI on PR #838 had two failures: 1. **Lint (sys.modules pollution check)**: my new `tests/unit/test_agent_soft_delete.py` had a `sys.modules[spec.name] = module` write inside the importlib loader — same lint trip as #602/#830. The modules being loaded (`utils.helpers`, `db.connection`) don't use `@dataclass(frozen=True)` so the registration is unneeded; drop it. 2. **Regression diff (5 of my tests + 18 existing)**: my new `WHERE deleted_at IS NULL` filter breaks every test that builds an ephemeral `agent_ownership` schema without the new column. And my own tests routed through the production `db.connection.get_db_connection()` which reads `DB_PATH` at module-import time — so once `db.connection` is loaded transitively (any earlier test importing `db.agents`), my `monkeypatch.setenv("TRINITY_DB_PATH", ...)` arrives too late. Fixes: - `test_agent_soft_delete.py`: replaced env-var routing with a `tmp_agent_db` fixture that `monkeypatch.setattr`s `db.connection.DB_PATH` directly. Survives whatever order pytest imports things. Also factored repeated setup into the fixture so each test is 4 lines instead of 20. - Added `deleted_at TEXT` column to the ephemeral `agent_ownership` schema in 7 affected test files: test_backlog.py, test_canary_invariants.py, test_file_sharing_mixin.py, test_guardrails.py, test_subscription_auto_switch_pingpong.py, test_watchdog_unit.py, scheduler_tests/conftest.py. The legacy schema in `test_agent_shared_files_migration.py` is intentionally pre-#834 (it tests the migration runner) and stays untouched. Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

vybe

This PR requires the following changes before merge:

Update docs/memory/requirements.md: add an entry for the soft-delete + retention lifecycle capability (Phase 1a scope, Phase 1b phases noted as pending).
Update docs/memory/architecture.md:
- (a) Add deleted_at TEXT + the partial index idx_agent_ownership_deleted_at to the agent_ownership schema table
- (b) Add the soft-delete retention sweep to the Cleanup Service background services table
- (c) Document the agent_soft_delete_retention_days setting (default 180d, 0 = disabled)
Address the scheduler/soft-delete gap: list_all_enabled_schedules() (src/backend/db/schedules.py:307) queries agent_schedules WHERE enabled = 1 with no join to agent_ownership, so the scheduler will attempt (and fail) every enabled schedule for a soft-deleted agent until the 180-day purge window expires — generating a schedule_executions failure row per attempt. Fix: add JOIN agent_ownership ao ON ao.agent_name = agent_schedules.agent_name WHERE ao.deleted_at IS NULL to the query, OR open a tracked issue for Phase 1b and document the known gap explicitly in this PR.
Either create tests/unit/test_agent_cleanup_parity.py (the enforcement test the agent_cleanup.py docstring promises), or remove the guarantee from the docstring until the test is written.

Implementation is solid overall — the read-path audit is thorough, is_agent_name_reserved() correctly catches the UNIQUE constraint edge case before container provisioning starts, the canary snapshot exclusion is well-reasoned, and iso_cutoff() is used correctly. CI is fully green. The blockers are docs + the scheduler gap.

CI on PR #838 had two failures: 1. **Lint (sys.modules pollution check)**: my new `tests/unit/test_agent_soft_delete.py` had a `sys.modules[spec.name] = module` write inside the importlib loader — same lint trip as #602/#830. The modules being loaded (`utils.helpers`, `db.connection`) don't use `@dataclass(frozen=True)` so the registration is unneeded; drop it. 2. **Regression diff (5 of my tests + 18 existing)**: my new `WHERE deleted_at IS NULL` filter breaks every test that builds an ephemeral `agent_ownership` schema without the new column. And my own tests routed through the production `db.connection.get_db_connection()` which reads `DB_PATH` at module-import time — so once `db.connection` is loaded transitively (any earlier test importing `db.agents`), my `monkeypatch.setenv("TRINITY_DB_PATH", ...)` arrives too late. Fixes: - `test_agent_soft_delete.py`: replaced env-var routing with a `tmp_agent_db` fixture that `monkeypatch.setattr`s `db.connection.DB_PATH` directly. Survives whatever order pytest imports things. Also factored repeated setup into the fixture so each test is 4 lines instead of 20. - Added `deleted_at TEXT` column to the ephemeral `agent_ownership` schema in 7 affected test files: test_backlog.py, test_canary_invariants.py, test_file_sharing_mixin.py, test_guardrails.py, test_subscription_auto_switch_pingpong.py, test_watchdog_unit.py, scheduler_tests/conftest.py. The legacy schema in `test_agent_shared_files_migration.py` is intentionally pre-#834 (it tests the migration runner) and stays untouched. Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…e 1b) Extends the #834 Phase 1a pattern to `agent_schedules`. Stacked on `feature/834-soft-delete-agents` (PR #838). Both PRs merge cleanly in either order — schema migration + retention sweep are additive. What changes - `DELETE /api/agents/{name}/schedules/{id}` marks `deleted_at = NOW` instead of hard-deleting. The schedule disappears from the scheduler's firing list immediately (the scheduler service's poll filters `enabled = 1 AND deleted_at IS NULL`), but the row + every `schedule_executions` child stay for the retention window so an operator can recover them by hand via `UPDATE agent_schedules SET deleted_at = NULL WHERE id = '...'`. - The retention sweep in `cleanup_service.py` hard-purges schedules past `schedule_soft_delete_retention_days` (default 30 days — shorter than the agent window since schedules are higher-churn). Each purge also wipes the schedule's `schedule_executions` rows, matching the previous hard-delete semantics. - Webhook tokens on soft-deleted schedules stop resolving — the `get_schedule_by_webhook_token` lookup filters `deleted_at IS NULL` too, so a leaked URL is invalidated the moment the schedule is marked deleted, not only at retention end. Where - Schema: `agent_schedules.deleted_at TEXT`, partial index `WHERE deleted_at IS NOT NULL`. Versioned migration in `db/migrations.py`. - `db/schedules.py`: - `delete_schedule()` flipped from DELETE to soft-delete. Idempotent on re-delete with permission. - `purge_schedule()` added — refuses live rows; called by the cleanup sweep. - `find_soft_deleted_schedules_past_retention()` drives the sweep. - 7 read sites filtered: `get_schedule`, `list_agent_schedules`, `list_all_enabled_schedules`, `list_all_disabled_schedules`, `list_all_schedules`, `get_schedule_by_webhook_token`, `get_webhook_status`, `get_all_agents_schedule_counts`. - `src/scheduler/database.py` (separate scheduler service process): same 4 read sites filtered. Without this the dedicated scheduler process would keep firing soft-deleted schedules. - `services/cleanup_service.py`: new sweep block calling `purge_schedule()` for eligible IDs, bounded by the existing 5000- row/cycle cap. - `services/settings_service.py`: new `schedule_soft_delete_retention_days` (default 30). Out of scope (intentional) - `delete_agent_schedules(agent_name)` (mass-delete) was left as a hard-delete. Its only caller path was removed in Phase 1a's `delete_agent_endpoint` rewrite; the function is now dormant and the cascade-on-purge path uses #816's `cascade_delete` instead. Refactoring the dead helper is out of scope. - Soft-delete does not interact with `schedule_executions` rows — they're billing-relevant (subscription_id rollup) and #772's existing retention sweep ages them out independently. Live verification on the running stack (uvicorn auto-reload picked up every change): 1. CREATE agent + schedule: 1 row in list ✓ 2. DELETE /api/agents/X/schedules/Y → 204 3. LIST after delete: 0 rows (filter working) ✓ 4. DB row still present, deleted_at populated ✓ 5. db.purge_schedule(id) → True, row + executions gone ✓ Related to #834. Phase 1b only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Phase 1c) Closes the user-facing side of the soft-delete story. Pre-#834 recovery was a direct-DB `UPDATE deleted_at = NULL` — unauditable, required shell access, soft-deleted set wasn't surfaced anywhere. Endpoints (admin-only, all audit-logged): GET /api/admin/soft-deleted/agents list soft-deleted agents with `deleted_at` + `purge_eta` (= deleted_at + agent_soft_delete_retention_days) POST /api/admin/soft-deleted/agents/{name}/recover clear deleted_at; 404 if not in the soft-deleted set GET /api/admin/soft-deleted/schedules[?agent_name=...] list soft-deleted schedules; purge_eta from schedule_soft_delete_retention_days POST /api/admin/soft-deleted/schedules/{id}/recover same shape as the agent variant Recovery is metadata-only: flipping deleted_at back to NULL makes the row reappear via all the user-facing read paths Phase 1a+1b filtered. For agents, the container is NOT recreated automatically; the operator does that explicitly via POST /api/agents/{name}/start. The preserved workspace volume keeps the agent's files intact. DB helpers (mirror Phase 1a/1b shape): - AgentOperations.recover_agent_ownership / list_soft_deleted_agents - ScheduleOperations.recover_schedule / list_soft_deleted_schedules (latter takes optional agent_name to scope per agent) All four refuse live rows (returns False / 404). All return False for nonexistent ids — admin endpoint translates to 404. Stacked on #839 (Phase 1b schedules). Stack: #838 (1a) → #839 (1b) → this (1c). Merge order matters because the schedule helpers live on Phase 1b's branch. Live verification on the running stack: 1. CREATE agent + schedule, soft-delete both 2. GET /api/admin/soft-deleted/agents → match found, purge_eta=2026-11-10 (180d from now) 3. GET /api/admin/soft-deleted/schedules?agent_name=X → 1 row, purge_eta=2026-06-13 (30d from now) 4. POST recover agent → 200, deleted_at cleared in DB ✓ 5. POST recover schedule → 200, deleted_at cleared ✓ 6. POST recover on now-live agent → 404 (correct refusal) ✓ 10 unit tests in test_admin_recovery.py cover: recover/list/scope, refuse-live-row, refuse-nonexistent, newest-first ordering, limit handling, agent_name scoping. Related to #834. Phase 1c. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…e 1a) Addresses PR #838 review (vybe, CHANGES_REQUESTED): - list_all_enabled_schedules() (backend db.schedules AND the standalone scheduler process) now JOINs agent_ownership and filters deleted_at IS NULL — a soft-deleted agent's enabled schedules stop firing immediately instead of writing a schedule_executions failure row per cron tick until the 180-day purge. - Add tests/unit/test_agent_cleanup_parity.py — the enforcement test agent_cleanup.py's docstring promises. Bidirectional schema↔AGENT_REFS parity + KEEP-policy lock. Stdlib-only loader, real CI gate (no venv skip). Plus a scheduler regression test for the agent-soft-delete gap. - requirements.md: new §32 Agent Soft-Delete & Retention Lifecycle (Phase 1a detailed; 1b/1c noted pending). - architecture.md: agent_ownership.deleted_at + idx_agent_ownership_deleted_at in the schema block; soft-delete purge added to the Cleanup Service row; agent_soft_delete_retention_days (default 180, 0=disabled). - Fix stale "default 30" comment in routers/agents.py (bumped to 180 in 45d99a1). Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

dolho · 2026-05-18T09:37:28Z

@vybe — all four blockers addressed in 3a45ef1:

requirements.md — new §32 "Agent Soft-Delete & Retention Lifecycle (Soft-delete all entities + configurable data retention policy #834)"; §32.1 Phase 1a detailed (soft-delete semantics, 180d retention purge, name reservation, scheduler-gap closure, canary handling), §32.2/§32.3 mark Phase 1b/1c pending.
architecture.md — agent_ownership.deleted_at + idx_agent_ownership_deleted_at partial index added to the schema block with a soft-delete prose paragraph; the Soft-delete all entities + configurable data retention policy #834 purge sweep + agent_soft_delete_retention_days (default 180, 0=disabled) added to the Cleanup Service background-services row.
Scheduler gap — fixed (not deferred). list_all_enabled_schedules() now JOINs agent_ownership and filters deleted_at IS NULL. Fixed in both sites: src/backend/db/schedules.py (the path you cited) and src/scheduler/database.py — the latter is the actual cron-firing path used by the standalone scheduler process (src/scheduler/service.py), so the backend-only fix alone would not have stopped the failure-row leak. Regression test added in tests/scheduler_tests/test_database.py::test_list_all_enabled_excludes_soft_deleted_agent.
Parity test — created tests/unit/test_agent_cleanup_parity.py, the enforcement test the agent_cleanup.py docstring promises. Bidirectional: every schema agent-id column must be in AGENT_REFS (forward) and every AGENT_REFS entry must exist in the schema (backward), plus a KEEP-policy lock and an _EXEMPT-dead-entry guard. Stdlib-only module loader so it's a real CI gate (no venv skip). 4 tests, all green.

Also corrected a stale default 30 comment in routers/agents.py (the default was bumped 30→180 in 45d99a1).

Local: pytest tests/unit/test_agent_soft_delete.py tests/unit/test_agent_cleanup_parity.py tests/scheduler_tests/ → 185 passed. Re-requesting review.

…e 1b) Extends the #834 Phase 1a pattern to `agent_schedules`. Stacked on `feature/834-soft-delete-agents` (PR #838). Both PRs merge cleanly in either order — schema migration + retention sweep are additive. What changes - `DELETE /api/agents/{name}/schedules/{id}` marks `deleted_at = NOW` instead of hard-deleting. The schedule disappears from the scheduler's firing list immediately (the scheduler service's poll filters `enabled = 1 AND deleted_at IS NULL`), but the row + every `schedule_executions` child stay for the retention window so an operator can recover them by hand via `UPDATE agent_schedules SET deleted_at = NULL WHERE id = '...'`. - The retention sweep in `cleanup_service.py` hard-purges schedules past `schedule_soft_delete_retention_days` (default 30 days — shorter than the agent window since schedules are higher-churn). Each purge also wipes the schedule's `schedule_executions` rows, matching the previous hard-delete semantics. - Webhook tokens on soft-deleted schedules stop resolving — the `get_schedule_by_webhook_token` lookup filters `deleted_at IS NULL` too, so a leaked URL is invalidated the moment the schedule is marked deleted, not only at retention end. Where - Schema: `agent_schedules.deleted_at TEXT`, partial index `WHERE deleted_at IS NOT NULL`. Versioned migration in `db/migrations.py`. - `db/schedules.py`: - `delete_schedule()` flipped from DELETE to soft-delete. Idempotent on re-delete with permission. - `purge_schedule()` added — refuses live rows; called by the cleanup sweep. - `find_soft_deleted_schedules_past_retention()` drives the sweep. - 7 read sites filtered: `get_schedule`, `list_agent_schedules`, `list_all_enabled_schedules`, `list_all_disabled_schedules`, `list_all_schedules`, `get_schedule_by_webhook_token`, `get_webhook_status`, `get_all_agents_schedule_counts`. - `src/scheduler/database.py` (separate scheduler service process): same 4 read sites filtered. Without this the dedicated scheduler process would keep firing soft-deleted schedules. - `services/cleanup_service.py`: new sweep block calling `purge_schedule()` for eligible IDs, bounded by the existing 5000- row/cycle cap. - `services/settings_service.py`: new `schedule_soft_delete_retention_days` (default 30). Out of scope (intentional) - `delete_agent_schedules(agent_name)` (mass-delete) was left as a hard-delete. Its only caller path was removed in Phase 1a's `delete_agent_endpoint` rewrite; the function is now dormant and the cascade-on-purge path uses #816's `cascade_delete` instead. Refactoring the dead helper is out of scope. - Soft-delete does not interact with `schedule_executions` rows — they're billing-relevant (subscription_id rollup) and #772's existing retention sweep ages them out independently. Live verification on the running stack (uvicorn auto-reload picked up every change): 1. CREATE agent + schedule: 1 row in list ✓ 2. DELETE /api/agents/X/schedules/Y → 204 3. LIST after delete: 0 rows (filter working) ✓ 4. DB row still present, deleted_at populated ✓ 5. db.purge_schedule(id) → True, row + executions gone ✓ Related to #834. Phase 1b only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Addresses PR #839 review (vybe, CHANGES_REQUESTED) + rebase onto the updated #838 (Phase 1a): - list_all_enabled_schedules() (backend + standalone scheduler) now filters BOTH s.deleted_at IS NULL (Phase 1b, this PR) AND ao.deleted_at IS NULL via the agent_ownership JOIN (Phase 1a, #838). A schedule is skipped if either it or its agent is soft-deleted. - test_schedule_soft_delete.py: ephemeral schema now creates agent_ownership + seeds a live owner row per agent so the merged firing-query JOIN resolves. - architecture.md: agent_schedules.deleted_at + idx_agent_schedules_deleted_at in the schema block; schedule soft-delete prose; Phase 1b purge added to the Cleanup Service row with schedule_soft_delete_retention_days (default 30, 0=disabled). - requirements.md: §32.2 fleshed out (read paths, idempotency, retention, execution-row ownership pre-purge vs at-purge, storage). PR body corrected: the schedule_executions "Out of scope" line now accurately states #772 owns them pre-purge while purge_schedule() cascades the delete at purge — consistent with prior hard-delete behavior (reviewer's option a). Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Phase 1c) Closes the user-facing side of the soft-delete story. Pre-#834 recovery was a direct-DB `UPDATE deleted_at = NULL` — unauditable, required shell access, soft-deleted set wasn't surfaced anywhere. Endpoints (admin-only, all audit-logged): GET /api/admin/soft-deleted/agents list soft-deleted agents with `deleted_at` + `purge_eta` (= deleted_at + agent_soft_delete_retention_days) POST /api/admin/soft-deleted/agents/{name}/recover clear deleted_at; 404 if not in the soft-deleted set GET /api/admin/soft-deleted/schedules[?agent_name=...] list soft-deleted schedules; purge_eta from schedule_soft_delete_retention_days POST /api/admin/soft-deleted/schedules/{id}/recover same shape as the agent variant Recovery is metadata-only: flipping deleted_at back to NULL makes the row reappear via all the user-facing read paths Phase 1a+1b filtered. For agents, the container is NOT recreated automatically; the operator does that explicitly via POST /api/agents/{name}/start. The preserved workspace volume keeps the agent's files intact. DB helpers (mirror Phase 1a/1b shape): - AgentOperations.recover_agent_ownership / list_soft_deleted_agents - ScheduleOperations.recover_schedule / list_soft_deleted_schedules (latter takes optional agent_name to scope per agent) All four refuse live rows (returns False / 404). All return False for nonexistent ids — admin endpoint translates to 404. Stacked on #839 (Phase 1b schedules). Stack: #838 (1a) → #839 (1b) → this (1c). Merge order matters because the schedule helpers live on Phase 1b's branch. Live verification on the running stack: 1. CREATE agent + schedule, soft-delete both 2. GET /api/admin/soft-deleted/agents → match found, purge_eta=2026-11-10 (180d from now) 3. GET /api/admin/soft-deleted/schedules?agent_name=X → 1 row, purge_eta=2026-06-13 (30d from now) 4. POST recover agent → 200, deleted_at cleared in DB ✓ 5. POST recover schedule → 200, deleted_at cleared ✓ 6. POST recover on now-live agent → 404 (correct refusal) ✓ 10 unit tests in test_admin_recovery.py cover: recover/list/scope, refuse-live-row, refuse-nonexistent, newest-first ordering, limit handling, agent_name scoping. Related to #834. Phase 1c. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Addresses PR #840 review (vybe, CHANGES_REQUESTED); rebased onto the updated #839/#838. - Move SoftDeletedAgent / SoftDeletedSchedule from routers/admin_recovery.py to models.py (Architectural Invariant #14 — response models are centralized, not declared in router files). - Cap limit on both list endpoints: limit: int = Query(200, le=500) — prevents unbounded DB reads (422 on >500). - requirements.md: §32.3 fleshed out (endpoints, recovery semantics, metadata-only/container-recreate-is-Phase-2, audit events, model location). - architecture.md: new "Soft-Delete Admin Recovery (#834 Phase 1c)" endpoint table with all 4 endpoints, auth, and behavior notes. Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

CI on PR #838 had two failures: 1. **Lint (sys.modules pollution check)**: my new `tests/unit/test_agent_soft_delete.py` had a `sys.modules[spec.name] = module` write inside the importlib loader — same lint trip as #602/#830. The modules being loaded (`utils.helpers`, `db.connection`) don't use `@dataclass(frozen=True)` so the registration is unneeded; drop it. 2. **Regression diff (5 of my tests + 18 existing)**: my new `WHERE deleted_at IS NULL` filter breaks every test that builds an ephemeral `agent_ownership` schema without the new column. And my own tests routed through the production `db.connection.get_db_connection()` which reads `DB_PATH` at module-import time — so once `db.connection` is loaded transitively (any earlier test importing `db.agents`), my `monkeypatch.setenv("TRINITY_DB_PATH", ...)` arrives too late. Fixes: - `test_agent_soft_delete.py`: replaced env-var routing with a `tmp_agent_db` fixture that `monkeypatch.setattr`s `db.connection.DB_PATH` directly. Survives whatever order pytest imports things. Also factored repeated setup into the fixture so each test is 4 lines instead of 20. - Added `deleted_at TEXT` column to the ephemeral `agent_ownership` schema in 7 affected test files: test_backlog.py, test_canary_invariants.py, test_file_sharing_mixin.py, test_guardrails.py, test_subscription_auto_switch_pingpong.py, test_watchdog_unit.py, scheduler_tests/conftest.py. The legacy schema in `test_agent_shared_files_migration.py` is intentionally pre-#834 (it tests the migration runner) and stays untouched. Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…e 1a) Addresses PR #838 review (vybe, CHANGES_REQUESTED): - list_all_enabled_schedules() (backend db.schedules AND the standalone scheduler process) now JOINs agent_ownership and filters deleted_at IS NULL — a soft-deleted agent's enabled schedules stop firing immediately instead of writing a schedule_executions failure row per cron tick until the 180-day purge. - Add tests/unit/test_agent_cleanup_parity.py — the enforcement test agent_cleanup.py's docstring promises. Bidirectional schema↔AGENT_REFS parity + KEEP-policy lock. Stdlib-only loader, real CI gate (no venv skip). Plus a scheduler regression test for the agent-soft-delete gap. - requirements.md: new §32 Agent Soft-Delete & Retention Lifecycle (Phase 1a detailed; 1b/1c noted pending). - architecture.md: agent_ownership.deleted_at + idx_agent_ownership_deleted_at in the schema block; soft-delete purge added to the Cleanup Service row; agent_soft_delete_retention_days (default 180, 0=disabled). - Fix stale "default 30" comment in routers/agents.py (bumped to 180 in 45d99a1). Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…Phase 1a) Replaces the hard-delete on `DELETE /api/agents/{name}` with a two-stage lifecycle: mark `agent_ownership.deleted_at` immediately, then hard-purge (cascading every child table via #816's primitive) after a configurable retention window. Default 30 days, settable via `agent_soft_delete_retention_days` in system_settings. What changes for an operator - Accidental `DELETE /api/agents/X` is no longer destructive. Chat history, schedules, sharing, permissions, credentials, MCP key, and on-disk workspace volumes all survive until purge. Recovery is a manual `UPDATE agent_ownership SET deleted_at=NULL` while the retention window holds (full UI/admin endpoint for recovery is Phase 1b, separate PR). - Agent names are reserved during the retention window — creating a new agent with the same name fails with 409 until the soft-deleted row is purged. Prevents accidental name collision with the deleted agent's lingering Redis state. - Docker containers remain ephemeral and are removed at delete time (issue acceptance criterion). Only the relational metadata and the workspace volume survive. What changes for an end-user (API consumer) - Nothing visible: `GET /api/agents/{name}` returns 404 for soft-deleted agents, `GET /api/agents` excludes them, every other per-agent read returns the same response as if the agent never existed. 404 transparency is an acceptance criterion of #834. Implementation 1. Schema: `deleted_at TEXT` on `agent_ownership` + partial index `WHERE deleted_at IS NOT NULL` so the retention sweep stays cheap as the live agent count grows. Versioned migration in `db/migrations.py`. 2. `delete_agent_ownership()` flipped from DELETE to `UPDATE deleted_at = NOW`. Idempotent on re-delete. `purge_agent_ownership()` runs the #816 `cascade_delete()` and then drops the parent row; refuses to operate on a row that isn't already soft-deleted. 3. `find_soft_deleted_agents_past_retention()` drives the cleanup sweep, bounded at 5000 rows/cycle per the existing #772 pattern. 4. Read-path audit: 35 SELECT/JOIN sites against `agent_ownership` now filter `WHERE deleted_at IS NULL`. The 4 unfiltered sites are intentional and commented: - `purge_agent_ownership` internals (need to see the soft-deleted row) - `rename_agent` uniqueness check on the destination name (name reservation acceptance criterion) - canary snapshot's `known_agents` (soft-deleted-pending-purge agents legitimately have child rows in live tables until the sweep runs — treating them as orphans would surface false positives in L-03) 5. `is_agent_name_reserved()` added as an unfiltered companion to `get_agent_owner()` — the create flow uses it to catch the "name held by a soft-deleted agent" case without the false-OK that filtering produces. 6. `cleanup_service.py` gains a sweep block that reads `agent_soft_delete_retention_days`, finds eligible rows, runs `purge_agent_ownership()` on each. Cycle count surfaced in `CleanupReport`. 7. Setting registered with default 30 days; "0" disables. Live verification on the running stack (uvicorn auto-reload picked up every change): - Migration ran cleanly on backend restart (`deleted_at` column present, partial index created) - Create → delete: `agent_ownership` row stays with `deleted_at` populated; `agent_sharing`, `agent_tags`, `mcp_api_keys` all survive - `GET /api/agents/{name}` returns 404 - `GET /api/agents` doesn't list the soft-deleted agent - `POST /api/agents` with the soft-deleted name returns 409 (name reserved) - `db.purge_agent_ownership(name)` cascades correctly - After purge, the name frees; recreation succeeds Dependency note This PR vendors `src/backend/db/agent_cleanup.py` from PR #829 (#816) so the cascade primitive is available even if #829 lands after this. If #829 merges first the file is identical and the merge is a no-op; if this PR merges first, #829's merge becomes a no-op for that file. Either order is safe. Out of scope (later phases) - Phase 1b: extend pattern to `agent_schedules`, `users` (auth-path implications), `agent_shared_files`, `agent_sessions`, `chat_sessions` (issue lists all six entities; doing each in its own PR per "validate the pattern before applying to risky tables"). - Admin endpoint to LIST + RECOVER soft-deleted agents (issue acceptance criterion). Today an operator does it via direct DB UPDATE — Phase 1b adds the API surface. - Container recreate-on-recover (preserved workspace volume + fresh container). Today recovery is metadata-only. Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

180 days is a more conservative recovery window — gives an operator who soft-deleted an agent in error roughly half a year to notice and recover. Disk cost for the parked relational metadata is small relative to the workspace volume that has to coexist with it anyway. Operators on a tight disk budget can override via `agent_soft_delete_retention_days` in `system_settings`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

CI on PR #838 had two failures: 1. **Lint (sys.modules pollution check)**: my new `tests/unit/test_agent_soft_delete.py` had a `sys.modules[spec.name] = module` write inside the importlib loader — same lint trip as #602/#830. The modules being loaded (`utils.helpers`, `db.connection`) don't use `@dataclass(frozen=True)` so the registration is unneeded; drop it. 2. **Regression diff (5 of my tests + 18 existing)**: my new `WHERE deleted_at IS NULL` filter breaks every test that builds an ephemeral `agent_ownership` schema without the new column. And my own tests routed through the production `db.connection.get_db_connection()` which reads `DB_PATH` at module-import time — so once `db.connection` is loaded transitively (any earlier test importing `db.agents`), my `monkeypatch.setenv("TRINITY_DB_PATH", ...)` arrives too late. Fixes: - `test_agent_soft_delete.py`: replaced env-var routing with a `tmp_agent_db` fixture that `monkeypatch.setattr`s `db.connection.DB_PATH` directly. Survives whatever order pytest imports things. Also factored repeated setup into the fixture so each test is 4 lines instead of 20. - Added `deleted_at TEXT` column to the ephemeral `agent_ownership` schema in 7 affected test files: test_backlog.py, test_canary_invariants.py, test_file_sharing_mixin.py, test_guardrails.py, test_subscription_auto_switch_pingpong.py, test_watchdog_unit.py, scheduler_tests/conftest.py. The legacy schema in `test_agent_shared_files_migration.py` is intentionally pre-#834 (it tests the migration runner) and stays untouched. Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…e 1a) Addresses PR #838 review (vybe, CHANGES_REQUESTED): - list_all_enabled_schedules() (backend db.schedules AND the standalone scheduler process) now JOINs agent_ownership and filters deleted_at IS NULL — a soft-deleted agent's enabled schedules stop firing immediately instead of writing a schedule_executions failure row per cron tick until the 180-day purge. - Add tests/unit/test_agent_cleanup_parity.py — the enforcement test agent_cleanup.py's docstring promises. Bidirectional schema↔AGENT_REFS parity + KEEP-policy lock. Stdlib-only loader, real CI gate (no venv skip). Plus a scheduler regression test for the agent-soft-delete gap. - requirements.md: new §32 Agent Soft-Delete & Retention Lifecycle (Phase 1a detailed; 1b/1c noted pending). - architecture.md: agent_ownership.deleted_at + idx_agent_ownership_deleted_at in the schema block; soft-delete purge added to the Cleanup Service row; agent_soft_delete_retention_days (default 180, 0=disabled). - Fix stale "default 30" comment in routers/agents.py (bumped to 180 in 45d99a1). Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The #834 parity test registers two synthetic modules (trinity_db_schema, trinity_db_agent_cleanup) into sys.modules at import time so @DataClass can resolve cls.__module__ while exec'ing db/agent_cleanup.py. That bare `sys.modules[mod_name] = module` tripped the `lint (sys.modules pollution check)` gate (0 → 1 vs baseline). Fix: add a top-level _STUBBED_MODULE_NAMES list + autouse _restore_sys_modules fixture — the sanctioned self-contained snapshot/restore pattern the lint whole-file-exempts (precedent: tests/unit/test_telegram_webhook_backfill.py). Also stops the synthetic modules leaking into sibling test files in the same pytest session. Parity suite still green (4 passed). Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Phase 1c) Closes the user-facing side of the soft-delete story. Pre-#834 recovery was a direct-DB `UPDATE deleted_at = NULL` — unauditable, required shell access, soft-deleted set wasn't surfaced anywhere. Endpoints (admin-only, all audit-logged): GET /api/admin/soft-deleted/agents list soft-deleted agents with `deleted_at` + `purge_eta` (= deleted_at + agent_soft_delete_retention_days) POST /api/admin/soft-deleted/agents/{name}/recover clear deleted_at; 404 if not in the soft-deleted set GET /api/admin/soft-deleted/schedules[?agent_name=...] list soft-deleted schedules; purge_eta from schedule_soft_delete_retention_days POST /api/admin/soft-deleted/schedules/{id}/recover same shape as the agent variant Recovery is metadata-only: flipping deleted_at back to NULL makes the row reappear via all the user-facing read paths Phase 1a+1b filtered. For agents, the container is NOT recreated automatically; the operator does that explicitly via POST /api/agents/{name}/start. The preserved workspace volume keeps the agent's files intact. DB helpers (mirror Phase 1a/1b shape): - AgentOperations.recover_agent_ownership / list_soft_deleted_agents - ScheduleOperations.recover_schedule / list_soft_deleted_schedules (latter takes optional agent_name to scope per agent) All four refuse live rows (returns False / 404). All return False for nonexistent ids — admin endpoint translates to 404. Stacked on #839 (Phase 1b schedules). Stack: #838 (1a) → #839 (1b) → this (1c). Merge order matters because the schedule helpers live on Phase 1b's branch. Live verification on the running stack: 1. CREATE agent + schedule, soft-delete both 2. GET /api/admin/soft-deleted/agents → match found, purge_eta=2026-11-10 (180d from now) 3. GET /api/admin/soft-deleted/schedules?agent_name=X → 1 row, purge_eta=2026-06-13 (30d from now) 4. POST recover agent → 200, deleted_at cleared in DB ✓ 5. POST recover schedule → 200, deleted_at cleared ✓ 6. POST recover on now-live agent → 404 (correct refusal) ✓ 10 unit tests in test_admin_recovery.py cover: recover/list/scope, refuse-live-row, refuse-nonexistent, newest-first ordering, limit handling, agent_name scoping. Related to #834. Phase 1c. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Addresses PR #840 review (vybe, CHANGES_REQUESTED); rebased onto the updated #839/#838. - Move SoftDeletedAgent / SoftDeletedSchedule from routers/admin_recovery.py to models.py (Architectural Invariant #14 — response models are centralized, not declared in router files). - Cap limit on both list endpoints: limit: int = Query(200, le=500) — prevents unbounded DB reads (422 on >500). - requirements.md: §32.3 fleshed out (endpoints, recovery semantics, metadata-only/container-recreate-is-Phase-2, audit events, model location). - architecture.md: new "Soft-Delete Admin Recovery (#834 Phase 1c)" endpoint table with all 4 endpoints, auth, and behavior notes. Related to #834 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

vybe

All four CHANGES_REQUESTED blockers from the prior review are resolved:

requirements.md §33 entry added
architecture.md schema block + Cleanup Service row + retention setting documented
Scheduler gap closed in both db/schedules.py:list_all_enabled_schedules() and the standalone scheduler process
tests/unit/test_agent_cleanup_parity.py created using the sanctioned _STUBBED_MODULE_NAMES pattern

CI fully green (6-seed pytest matrix + regression-diff + schema-parity + lint + CodeQL). Read-path audit is thorough; the 4 intentionally-unfiltered sites are all commented with rationale. Ready to merge.

Brings dev forward to include #838 (agent_ownership soft-delete + retention purge, #834 Phase 1a). Conflict resolved: src/backend/db/migrations.py — both sides appended a migration to the registry. Same pattern as the earlier 2026-05-17 merge: dev's migration first (`agent_ownership_soft_delete` landed on dev as part of #838), then our `execution_retry_count` (lands when PR #797 merges). Migrations are additive and independent — order doesn't matter operationally, but matching landing chronology keeps the registry readable. Both migration functions (_migrate_agent_ownership_soft_delete at :2102, _migrate_execution_retry_count at :2011) are present in the auto-merged file. Refs #678

…e-586-plan Two new commits on dev since the previous merge (c9008e4): - b892069: feat(observability): emit [METRIC] drain_outcome on slow-path orphan-killer (#586) (#837) - c6bc456: feat(soft-delete): agent_ownership soft-delete + retention purge (#834 Phase 1a) (#838) Conflicts resolved: - tests/unit/test_subprocess_pgroup.py: kept BOTH this PR's test_setsid_escapee_drained_via_orphan_killer_preserves_result_line (#586 regression) AND dev's #837 metric tests (test_emits_metric_on_natural_drain, test_emits_metric_on_force_close_path). Tests cover orthogonal aspects of the slow-path drain: setsid escape + result-line preservation vs. drain_outcome metric emission. - docs/memory/feature-flows.md: kept both this PR's #35d4e78 row and dev's #586/#837 row. Lint still green; sys.modules baseline already current after the previous dev-merge (c9008e4).