Skip to content

feat(soft-delete): admin recovery endpoints (#834 Phase 1c) - #840

Merged
vybe merged 3 commits into
devfrom
feature/834-phase-1c-recover
May 21, 2026
Merged

feat(soft-delete): admin recovery endpoints (#834 Phase 1c)#840
vybe merged 3 commits into
devfrom
feature/834-phase-1c-recover

Conversation

@dolho

@dolho dolho commented May 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the user-facing side of #834. Pre-PR recovery was a direct-DB UPDATE deleted_at = NULL — unauditable, required shell access. This PR exposes the soft-deleted set + reversal via admin-only HTTP.

Stacked on #839 (which is stacked on #838). Merge order: #838#839 → this.

Endpoints

Method Path What
GET /api/admin/soft-deleted/agents List soft-deleted agents with deleted_at + purge_eta
POST /api/admin/soft-deleted/agents/{name}/recover Clear deleted_at; 404 if not in soft-deleted set
GET /api/admin/soft-deleted/schedules[?agent_name=...] Same for schedules; per-agent scope optional
POST /api/admin/soft-deleted/schedules/{id}/recover Clear schedule deleted_at

purge_eta = deleted_at + retention_days (uses agent_soft_delete_retention_days for agents = 180 default, schedule_soft_delete_retention_days for schedules = 30). Null when the corresponding retention is 0 (sweep disabled).

All four endpoints are Depends(require_admin). Every successful recovery emits a platform audit event (agent_lifecycle:recover or agent_lifecycle:schedule_recover).

Semantics

Recovery is metadata-only. For agents the container is NOT recreated automatically — operator does POST /api/agents/{name}/start if they want it running. The preserved workspace volume keeps files intact.

Live verification

1. CREATE agent + schedule, soft-delete both
2. GET soft-deleted/agents     → match, purge_eta=2026-11-10 (180d) ✓
3. GET soft-deleted/schedules  → 1 row, purge_eta=2026-06-13 (30d) ✓
4. POST recover agent          → 200, deleted_at cleared ✓
5. POST recover schedule       → 200, deleted_at cleared ✓
6. POST recover on now-live    → 404 ✓

Test plan

  • 10 unit tests in tests/unit/test_admin_recovery.py: recover/list/scope, refuse-live, refuse-nonexistent, ordering, limit, agent_name filter
  • Live smoke against running backend
  • CI: lint + 6-seed pytest matrix + regression diff

Notes

Related to #834. Phase 1c (recovery surface).

🤖 Generated with Claude Code

@dolho
dolho force-pushed the feature/834-phase-1b-schedules branch from bb8db6a to 56cf583 Compare May 15, 2026 08:46
@dolho
dolho force-pushed the feature/834-phase-1c-recover branch from 3c2c525 to cceaf78 Compare May 15, 2026 08:46
@dolho
dolho marked this pull request as ready for review May 15, 2026 08:47
@dolho
dolho force-pushed the feature/834-phase-1c-recover branch 2 times, most recently from 7870970 to 7c9523f Compare May 15, 2026 09:21

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR requires the following changes before merge:

  • Move SoftDeletedAgent and SoftDeletedSchedule to models.py (Architectural Invariant #14: response models belong in models.py, not in router files)

  • Update docs/memory/requirements.md with the Phase 1c recovery capability (admin recovery surface for soft-deleted agents/schedules)

  • Update docs/memory/architecture.md: add the 4 new endpoints (GET/POST /api/admin/soft-deleted/agents[/{name}/recover] and GET/POST /api/admin/soft-deleted/schedules[/{id}/recover]) to the API endpoint tables

  • Cap the limit query parameter on both list endpoints: limit: int = Query(200, le=500) to prevent unbounded DB reads

Please address these items and request re-review.

@dolho
dolho force-pushed the feature/834-phase-1b-schedules branch from 4c71cad to 4d0a673 Compare May 18, 2026 09:47
dolho added a commit that referenced this pull request May 18, 2026
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>
@dolho
dolho force-pushed the feature/834-phase-1c-recover branch from 7c9523f to 21b1c8f Compare May 18, 2026 09:53
@dolho

dolho commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

@vybe — all four blockers addressed (rebased onto the updated #839/#838 stack; new tip 21b1c8f3):

Stack note: --onto-rebased so #840 carries only its 2 Phase 1c commits on top of the rebased #839 (which itself sits on the #838 review-fix). No conflicts on the rebase.

Local: pytest tests/unit/test_admin_recovery.py tests/unit/test_schedule_soft_delete.py tests/unit/test_agent_soft_delete.py tests/unit/test_agent_cleanup_parity.py tests/scheduler_tests/test_database.py44 passed. Re-requesting review.

@dolho
dolho requested a review from vybe May 18, 2026 09:53
dolho added a commit that referenced this pull request May 20, 2026
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>
@dolho
dolho force-pushed the feature/834-phase-1c-recover branch from 21b1c8f to 6c6d48a Compare May 20, 2026 09:14
@dolho
dolho force-pushed the feature/834-phase-1b-schedules branch from 4d0a673 to 8593923 Compare May 20, 2026 09:14
@dolho
dolho force-pushed the feature/834-phase-1b-schedules branch from 8593923 to 69bf92b Compare May 21, 2026 09:09
dolho added a commit that referenced this pull request May 21, 2026
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>
@dolho
dolho force-pushed the feature/834-phase-1c-recover branch 2 times, most recently from f898afc to 5d5f799 Compare May 21, 2026 09:24
dolho added a commit that referenced this pull request May 21, 2026
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>
@dolho
dolho force-pushed the feature/834-phase-1c-recover branch from 5d5f799 to 064cb33 Compare May 21, 2026 09:41
dolho added a commit that referenced this pull request May 21, 2026
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
vybe changed the base branch from feature/834-phase-1b-schedules to dev May 21, 2026 15:51
dolho and others added 3 commits May 21, 2026 16:54
 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>
…ase 1c)

Smoke-testing the rebased stack on the live instance surfaced a
recovery dead-end:

1. `GET /api/agents/{name}` 404'd after a successful recover. The
   handler resolves the agent via `get_agent_by_name` (Docker-backed);
   soft-delete removed the container and recovery is metadata-only, so
   there's no container → 404. The operator literally could not see
   the agent they just recovered, let alone act on it. Recovery was
   pointless from the UI's perspective.

   Fix: when `get_agent_by_name` returns None but a live
   agent_ownership row exists (the auth dependency already proved it
   does — `get_agent_owner` filters `deleted_at IS NULL`), synthesize
   a DB-backed `status=stopped, needs_start=true` representation so
   the agent lists and the UI can offer a Start affordance.

2. The recover endpoint's success message claimed "POST /start to
   bring it back online" — but Start also 404s on a container-less
   agent (it too requires an existing container). Container recreate
   from the preserved workspace volume is the explicitly-deferred
   #834 Phase 2. Rewrote the message to be accurate: recovery
   restores all *relational* state (the hard-to-recreate part that
   #834 exists to protect); re-running the agent needs Phase 2.
   Added `needs_container_recreate: true` to the response.

What recovery delivers today (correct + tested): the agent_ownership
row, chat history, schedules, sharing, permissions, credentials
config — every relational artifact — comes back intact and visible.
What it doesn't (Phase 2): a runnable container.

Live-verified on the running stack: create → soft-delete (GET 404)
→ recover (200) → GET now 200 with needs_start=true → agent visible.
Start still 404s by design until Phase 2; message + flag now say so.

Related to #834

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
vybe force-pushed the feature/834-phase-1c-recover branch from 064cb33 to ad95514 Compare May 21, 2026 15:54

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All four CHANGES_REQUESTED items from the prior review are resolved:

  • SoftDeletedAgent / SoftDeletedSchedule moved to models.py (Architectural Invariant #14)
  • limit: int = Query(200, le=500) cap on both list endpoints
  • requirements.md §33.3 fleshed out (endpoints, recovery semantics, audit, Phase-2-deferred container recreate)
  • architecture.md endpoint table added for all 4 admin recovery endpoints

Bonus: fix(recover): make recovered agents visible + honest message catches the recovery dead-end where GET /api/agents/{name} would 404 post-recover (because get_agent_by_name is Docker-backed and the container is gone) — now synthesizes a DB-backed status=stopped, needs_start=true representation, and the recover-success message honestly defers container recreate to Phase 2.

Branch was rebased onto current dev (drops the now-squashed Phase 1b ancestors). CI fully green: 6-seed pytest matrix + regression-diff + schema-parity + lint + CodeQL.

@vybe
vybe merged commit 3f5c697 into dev May 21, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants