diff --git a/.gitignore b/.gitignore index 29eddf8..34479a3 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ npm-debug.log* # IDE .vscode/ .idea/ +.claude/ *.swp *.swo *~ diff --git a/backend/app/api/routes/chat.py b/backend/app/api/routes/chat.py index 44f4729..25cdd2c 100644 --- a/backend/app/api/routes/chat.py +++ b/backend/app/api/routes/chat.py @@ -25,7 +25,7 @@ import uuid from datetime import datetime, timezone from typing import Optional -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Response from pydantic import BaseModel, Field from sqlalchemy import select from sqlalchemy.orm import Session @@ -407,6 +407,22 @@ def list_turns_generic(session_id: uuid.UUID, db: Session = Depends(get_db)): return db.scalars(stmt).all() +@router.delete("/sessions/{session_id}", status_code=204) +def delete_session_generic(session_id: uuid.UUID, db: Session = Depends(get_db)) -> Response: + """Delete a chat session and cascade to its turn events. Commits authored + by this session are preserved (they are owned by the space, not the session).""" + session = db.get(ChatSession, session_id) + if not session: + raise HTTPException(status_code=404, detail="Session not found") + if session.repo_id is not None: + repo = db.get(RepoModel, session.repo_id) + if not repo or repo.user_id != DEMO_USER_ID: + raise HTTPException(status_code=404, detail="Session not found") + db.delete(session) + db.commit() + return Response(status_code=204) + + class AttachSessionRequest(BaseModel): repo_id: str diff --git a/backend/app/api/routes/commits.py b/backend/app/api/routes/commits.py index 05b3391..fc91c00 100644 --- a/backend/app/api/routes/commits.py +++ b/backend/app/api/routes/commits.py @@ -3,16 +3,31 @@ import json import uuid from datetime import datetime -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Query, Response from pydantic import BaseModel, Field +from sqlalchemy import select from sqlalchemy.orm import Session from app.db.database import get_db -from app.db.models import RepoModel, CommitModel +from app.db.models import RepoModel, CommitModel, ChatSession from app.api.routes.repos import CommitResponse, DEMO_USER_ID router = APIRouter(prefix="/commits", tags=["commits"]) + +class CheckpointDependent(BaseModel): + kind: str # "child_commit" | "forked_session" | "seeded_session" + id: uuid.UUID + label: str + + +class CheckpointDependentsResponse(BaseModel): + checkpoint_id: uuid.UUID + child_commits: list[CheckpointDependent] + forked_sessions: list[CheckpointDependent] + seeded_sessions: list[CheckpointDependent] + blocking_count: int + class CommitCreate(BaseModel): repo_id: str parent_commit_id: str | None = None @@ -88,9 +103,135 @@ def get_commit(commit_id: uuid.UUID, db: Session = Depends(get_db)): commit = db.get(CommitModel, commit_id) if not commit: raise HTTPException(status_code=404, detail="Commit not found") - + repo = db.get(RepoModel, commit.repo_id) if not repo or repo.user_id != DEMO_USER_ID: raise HTTPException(status_code=404, detail="Commit/Repo not found") - + return commit + + +def _dependents_payload( + commit_id: uuid.UUID, + child_commits: list[CommitModel], + forks: list[ChatSession], + seeds: list[ChatSession], +) -> CheckpointDependentsResponse: + return CheckpointDependentsResponse( + checkpoint_id=commit_id, + child_commits=[ + CheckpointDependent( + kind="child_commit", + id=c.id, + label=f"{c.commit_hash[:7]} — {(c.message or '')[:60]}", + ) + for c in child_commits + ], + forked_sessions=[ + CheckpointDependent( + kind="forked_session", + id=s.id, + label=f"{s.branch_name} — {(s.title or 'Untitled')[:60]}", + ) + for s in forks + ], + seeded_sessions=[ + CheckpointDependent( + kind="seeded_session", + id=s.id, + label=(s.title or "Untitled")[:60], + ) + for s in seeds + ], + blocking_count=len(child_commits) + len(forks), + ) + + +def _collect_descendant_subtree(root_id: uuid.UUID, db: Session) -> list[CommitModel]: + """BFS down the parent_commit_id DAG; return deepest-first so SET NULL + never fires on a live row during cascade delete.""" + frontier = [root_id] + visited: set[uuid.UUID] = set() + ordered: list[CommitModel] = [] + while frontier: + next_frontier = [] + for pid in frontier: + children = db.scalars( + select(CommitModel).where(CommitModel.parent_commit_id == pid) + ).all() + for child in children: + if child.id in visited: + continue + visited.add(child.id) + ordered.append(child) + next_frontier.append(child.id) + frontier = next_frontier + return list(reversed(ordered)) # deepest first + + +@router.delete("/{commit_id}", status_code=204) +def delete_commit( + commit_id: uuid.UUID, + cascade: bool = Query( + False, + description="Also delete descendant commits and forked sessions", + ), + db: Session = Depends(get_db), +) -> Response: + """Delete a checkpoint. Refuses if it has child commits or forked sessions + unless cascade=true is passed.""" + commit = db.get(CommitModel, commit_id) + if not commit: + raise HTTPException(status_code=404, detail="Checkpoint not found") + + repo = db.get(RepoModel, commit.repo_id) + if not repo or repo.user_id != DEMO_USER_ID: + raise HTTPException(status_code=404, detail="Checkpoint not found") + + child_commits = db.scalars( + select(CommitModel).where(CommitModel.parent_commit_id == commit_id) + ).all() + forked_sessions = db.scalars( + select(ChatSession).where(ChatSession.forked_from_checkpoint_id == commit_id) + ).all() + seeded_sessions = db.scalars( + select(ChatSession).where( + ChatSession.seeded_commit_id == commit_id, + ChatSession.forked_from_checkpoint_id != commit_id, + ) + ).all() + + blocking_count = len(child_commits) + len(forked_sessions) + + if blocking_count > 0 and not cascade: + raise HTTPException( + status_code=409, + detail={ + "message": ( + f"Cannot delete checkpoint {commit.commit_hash[:7]}: " + f"it has {len(child_commits)} child commit(s) and " + f"{len(forked_sessions)} forked session(s). " + f"Re-send with ?cascade=true to delete the subtree." + ), + "dependents": _dependents_payload( + commit_id, list(child_commits), list(forked_sessions), list(seeded_sessions) + ).model_dump(mode="json"), + }, + ) + + if cascade: + descendants = _collect_descendant_subtree(commit_id, db) # deepest first + ids_in_subtree = {c.id for c in descendants} | {commit_id} + dep_forks = db.scalars( + select(ChatSession).where( + ChatSession.forked_from_checkpoint_id.in_(ids_in_subtree) + ) + ).all() + for sess in dep_forks: + db.delete(sess) # cascades to TurnEvent + for descendant in descendants: + db.delete(descendant) + + db.delete(commit) + db.commit() + return Response(status_code=204) diff --git a/backend/app/api/routes/repos.py b/backend/app/api/routes/repos.py index 4684fd8..278ba35 100644 --- a/backend/app/api/routes/repos.py +++ b/backend/app/api/routes/repos.py @@ -4,7 +4,7 @@ from typing import Annotated from typing import Optional -from fastapi import APIRouter, Depends, HTTPException, Query +from fastapi import APIRouter, Depends, HTTPException, Query, Response from pydantic import BaseModel, Field from sqlalchemy import select from sqlalchemy.orm import Session @@ -127,3 +127,13 @@ def get_latest_commit(repo_id: uuid.UUID, branch: str = "main", db: Session = De if not commit: raise HTTPException(status_code=404, detail="No commits found for this repo/branch") return commit + +@router.delete("/{repo_id}", status_code=204) +def delete_repo(repo_id: uuid.UUID, db: Session = Depends(get_db)) -> Response: + """Delete a space and cascade to all its commits, sessions, and turns.""" + repo = db.get(RepoModel, repo_id) + if not repo or repo.user_id != DEMO_USER_ID: + raise HTTPException(status_code=404, detail="Repo not found") + db.delete(repo) + db.commit() + return Response(status_code=204) diff --git a/backend/tests/integration/test_delete_endpoints.py b/backend/tests/integration/test_delete_endpoints.py new file mode 100644 index 0000000..311a948 --- /dev/null +++ b/backend/tests/integration/test_delete_endpoints.py @@ -0,0 +1,339 @@ +""" +Integration tests for DELETE endpoints across V2 (repos, commits) and V4 (sessions). + +Covers cascade correctness, child-commit / forked-session refusal, the cascade +escape hatch, and idempotency. All tests use the SQLite-backed in-memory client +fixture from conftest.py which enables FK enforcement via PRAGMA foreign_keys=ON. +""" +import uuid + +import pytest + + +# ── Helpers ─────────────────────────────────────────────────────────────────── + + +def _create_repo(client, name="Delete Test Repo"): + r = client.post("/api/v2/repos", json={"name": name}) + assert r.status_code == 201, r.text + return r.json()["id"] + + +def _create_session(client, repo_id, title="test session"): + r = client.post(f"/api/v4/chat/spaces/{repo_id}/sessions", json={ + "title": title, "provider": "openrouter", "model": "mock", + }) + assert r.status_code == 201, r.text + return r.json()["id"] + + +def _send(client, session_id, message="hello"): + r = client.post("/api/v4/chat/send", json={ + "session_id": session_id, + "provider": "openrouter", + "model": "mock", + "message": message, + "use_mock": True, + }) + assert r.status_code == 200, r.text + return r.json() + + +def _commit(client, repo_id, session_id, message="checkpoint", **kwargs): + payload = { + "repo_id": repo_id, + "session_id": session_id, + "message": message, + **kwargs, + } + r = client.post("/api/v4/chat/commit", json=payload) + assert r.status_code == 201, r.text + return r.json() + + +def _fork(client, space_id, checkpoint_id, branch_name=""): + r = client.post("/api/v5/lineage/sessions/fork", json={ + "space_id": space_id, + "checkpoint_id": checkpoint_id, + "branch_name": branch_name, + }) + assert r.status_code == 201, r.text + return r.json() + + +# ── 1. Space delete cascades commits ───────────────────────────────────────── + + +def test_delete_space_cascades_commits(client): + repo_id = _create_repo(client, "Cascade-1") + session_id = _create_session(client, repo_id) + c1 = _commit(client, repo_id, session_id, "first") + c2 = _commit(client, repo_id, session_id, "second") + + r = client.delete(f"/api/v2/repos/{repo_id}") + assert r.status_code == 204, r.text + + assert client.get(f"/api/v2/repos/{repo_id}").status_code == 404 + assert client.get(f"/api/v2/commits/{c1['id']}").status_code == 404 + assert client.get(f"/api/v2/commits/{c2['id']}").status_code == 404 + + +# ── 2. Space delete cascades sessions and turns ────────────────────────────── + + +def test_delete_space_cascades_sessions_and_turns(client): + repo_id = _create_repo(client, "Cascade-2") + session_id = _create_session(client, repo_id) + _send(client, session_id, "hello") + _send(client, session_id, "world") + + r = client.delete(f"/api/v2/repos/{repo_id}") + assert r.status_code == 204, r.text + + # The session's turns endpoint should fail because the session itself is gone. + turns = client.get(f"/api/v4/chat/sessions/{session_id}/turns") + assert turns.status_code == 404 + + +# ── 3. Space delete rejects cross-user ─────────────────────────────────────── + + +def test_delete_space_cross_user_returns_404(client, db_session): + from app.db.models import RepoModel + + other_user = uuid.UUID("00000000-0000-0000-0000-0000000000ff") + repo = RepoModel(user_id=other_user, name="Not mine", description="", metadata_={}) + db_session.add(repo) + db_session.commit() + db_session.refresh(repo) + other_id = str(repo.id) + + r = client.delete(f"/api/v2/repos/{other_id}") + assert r.status_code == 404 + + +# ── 4. Checkpoint leaf delete succeeds ─────────────────────────────────────── + + +def test_delete_commit_leaf_succeeds(client): + repo_id = _create_repo(client, "Leaf") + session_id = _create_session(client, repo_id) + commit = _commit(client, repo_id, session_id, "solo") + + r = client.delete(f"/api/v2/commits/{commit['id']}") + assert r.status_code == 204, r.text + assert client.get(f"/api/v2/commits/{commit['id']}").status_code == 404 + + +# ── 5. Checkpoint delete refuses when children exist (no cascade) ──────────── + + +def test_delete_commit_with_child_refuses_without_cascade(client): + repo_id = _create_repo(client, "With-Child") + session_id = _create_session(client, repo_id) + c1 = _commit(client, repo_id, session_id, "parent") + _send(client, session_id, "next turn") + c2 = _commit(client, repo_id, session_id, "child") + + r = client.delete(f"/api/v2/commits/{c1['id']}") + assert r.status_code == 409, r.text + detail = r.json()["detail"] + assert "child commit" in detail["message"].lower() + assert detail["dependents"]["blocking_count"] >= 1 + assert len(detail["dependents"]["child_commits"]) == 1 + assert detail["dependents"]["child_commits"][0]["id"] == c2["id"] + + # C1 still exists + assert client.get(f"/api/v2/commits/{c1['id']}").status_code == 200 + + +# ── 6. Checkpoint cascade deletes entire subtree ───────────────────────────── + + +def test_delete_commit_with_cascade_deletes_subtree(client): + repo_id = _create_repo(client, "Subtree") + session_id = _create_session(client, repo_id) + c1 = _commit(client, repo_id, session_id, "root") + _send(client, session_id, "turn 1") + c2 = _commit(client, repo_id, session_id, "mid") + _send(client, session_id, "turn 2") + c3 = _commit(client, repo_id, session_id, "leaf") + + r = client.delete(f"/api/v2/commits/{c1['id']}?cascade=true") + assert r.status_code == 204, r.text + + assert client.get(f"/api/v2/commits/{c1['id']}").status_code == 404 + assert client.get(f"/api/v2/commits/{c2['id']}").status_code == 404 + assert client.get(f"/api/v2/commits/{c3['id']}").status_code == 404 + + +# ── 7. Checkpoint with fork refuses without cascade ────────────────────────── + + +def test_delete_commit_with_fork_refuses_without_cascade(client): + repo_id = _create_repo(client, "Fork-Refuse") + session_id = _create_session(client, repo_id) + c1 = _commit(client, repo_id, session_id, "base") + fork = _fork(client, repo_id, c1["id"], branch_name="experiment") + fork_session_id = fork["session_id"] + + r = client.delete(f"/api/v2/commits/{c1['id']}") + assert r.status_code == 409, r.text + detail = r.json()["detail"] + assert detail["dependents"]["blocking_count"] >= 1 + assert len(detail["dependents"]["forked_sessions"]) == 1 + assert detail["dependents"]["forked_sessions"][0]["id"] == fork_session_id + + +# ── 8. Checkpoint cascade also deletes forked sessions and turns ───────────── + + +def test_delete_commit_with_cascade_deletes_forked_sessions_and_turns(client): + repo_id = _create_repo(client, "Fork-Cascade") + session_id = _create_session(client, repo_id) + c1 = _commit(client, repo_id, session_id, "base") + fork = _fork(client, repo_id, c1["id"], branch_name="experiment") + fork_session_id = fork["session_id"] + _send(client, fork_session_id, "fork message 1") + _send(client, fork_session_id, "fork message 2") + + r = client.delete(f"/api/v2/commits/{c1['id']}?cascade=true") + assert r.status_code == 204, r.text + + assert client.get(f"/api/v2/commits/{c1['id']}").status_code == 404 + assert client.get(f"/api/v4/chat/sessions/{fork_session_id}").status_code == 404 + + +# ── 9. Seeded-only session does NOT block checkpoint delete ───────────────── + + +def test_delete_commit_with_seeded_only_session_succeeds(client): + repo_id = _create_repo(client, "Seeded-Only") + session_a_id = _create_session(client, repo_id, title="author session") + c1 = _commit(client, repo_id, session_a_id, "first") + + # New session — auto-seeds from head (which is C1). It is NOT a fork. + session_b_id = _create_session(client, repo_id, title="reader session") + session_b = client.get(f"/api/v4/chat/sessions/{session_b_id}").json() + assert session_b["seeded_commit_id"] == c1["id"] + assert session_b["forked_from_checkpoint_id"] is None # not a fork + + # Delete C1: informational seed should not block. + r = client.delete(f"/api/v2/commits/{c1['id']}") + assert r.status_code == 204, r.text + + # session_b still exists; seeded_commit_id is now null. + r2 = client.get(f"/api/v4/chat/sessions/{session_b_id}") + assert r2.status_code == 200 + assert r2.json()["seeded_commit_id"] is None + + +# ── 10. Session delete cascades turn events ────────────────────────────────── + + +def test_delete_chat_session_cascades_turns(client): + repo_id = _create_repo(client, "Session-Cascade") + session_id = _create_session(client, repo_id) + _send(client, session_id, "hello") + _send(client, session_id, "world") + + turns_before = client.get(f"/api/v4/chat/sessions/{session_id}/turns").json() + assert len(turns_before) >= 2 + + r = client.delete(f"/api/v4/chat/sessions/{session_id}") + assert r.status_code == 204, r.text + + assert client.get(f"/api/v4/chat/sessions/{session_id}").status_code == 404 + assert client.get(f"/api/v4/chat/sessions/{session_id}/turns").status_code == 404 + + +# ── 11. Session delete preserves commits it authored ───────────────────────── + + +def test_delete_chat_session_leaves_commits_authored_by_it(client): + repo_id = _create_repo(client, "Preserve-Commits") + session_id = _create_session(client, repo_id) + commit = _commit(client, repo_id, session_id, "survives") + + r = client.delete(f"/api/v4/chat/sessions/{session_id}") + assert r.status_code == 204, r.text + + # Commit authored by the deleted session still exists. + r2 = client.get(f"/api/v2/commits/{commit['id']}") + assert r2.status_code == 200 + + +# ── 12. Checkpoint delete rejects cross-user ──────────────────────────────── + + +def test_delete_commit_cross_user_returns_404(client, db_session): + from app.db.models import RepoModel, CommitModel + + other_user = uuid.UUID("00000000-0000-0000-0000-0000000000ff") + repo = RepoModel(user_id=other_user, name="Not mine", description="", metadata_={}) + db_session.add(repo) + db_session.commit() + db_session.refresh(repo) + + commit = CommitModel( + repo_id=repo.id, + commit_hash="deadbeef" * 8, + branch_name="main", + author_type="user", + message="foreign", + metadata_={}, + ) + db_session.add(commit) + db_session.commit() + db_session.refresh(commit) + + r = client.delete(f"/api/v2/commits/{commit.id}") + assert r.status_code == 404 + + +# ── 13. Subtree cascade ordering is safe ───────────────────────────────────── + + +def test_delete_commit_subtree_ordering_is_safe(client, db_session): + from app.db.models import CommitModel + + repo_id = _create_repo(client, "Ordering") + session_id = _create_session(client, repo_id) + c1 = _commit(client, repo_id, session_id, "depth-1") + _send(client, session_id, "t1") + c2 = _commit(client, repo_id, session_id, "depth-2") + _send(client, session_id, "t2") + c3 = _commit(client, repo_id, session_id, "depth-3") + + r = client.delete(f"/api/v2/commits/{c1['id']}?cascade=true") + assert r.status_code == 204, r.text + + # Ensure nothing left with parent_commit_id pointing at a deleted commit. + remaining = db_session.query(CommitModel).all() + deleted_ids = {uuid.UUID(c1["id"]), uuid.UUID(c2["id"]), uuid.UUID(c3["id"])} + for c in remaining: + assert c.id not in deleted_ids, f"{c.id} should have been deleted" + assert c.parent_commit_id not in deleted_ids, ( + f"{c.id} still points at deleted parent {c.parent_commit_id}" + ) + + +# ── 14. Idempotency: double-delete is not a 500 ────────────────────────────── + + +def test_delete_idempotency_double_call(client): + repo_id = _create_repo(client, "Idempotent") + session_id = _create_session(client, repo_id) + commit = _commit(client, repo_id, session_id, "solo") + + r1 = client.delete(f"/api/v2/commits/{commit['id']}") + assert r1.status_code == 204 + + r2 = client.delete(f"/api/v2/commits/{commit['id']}") + assert r2.status_code == 404 + + # And the same for space delete + r3 = client.delete(f"/api/v2/repos/{repo_id}") + assert r3.status_code == 204 + r4 = client.delete(f"/api/v2/repos/{repo_id}") + assert r4.status_code == 404 diff --git a/cli/smriti_cli/client.py b/cli/smriti_cli/client.py index fad9390..1c1ca4b 100644 --- a/cli/smriti_cli/client.py +++ b/cli/smriti_cli/client.py @@ -21,9 +21,10 @@ DEFAULT_API_URL = "http://localhost:8000" class SmritiError(Exception): """Raised for any API error or unreachable backend.""" - def __init__(self, message: str, status: int | None = None): + def __init__(self, message: str, status: int | None = None, detail: Any = None): super().__init__(message) self.status = status + self.detail = detail class SmritiClient: @@ -53,14 +54,24 @@ class SmritiClient: raise SmritiError(f"Request to {url} timed out after {self.timeout}s") if not resp.ok: - detail = None + detail_obj: Any = None + message: str try: - detail = resp.json().get("detail") + detail_obj = resp.json().get("detail") except Exception: - detail = resp.text[:200] + detail_obj = None + message = resp.text[:200] + else: + if isinstance(detail_obj, str): + message = detail_obj + elif isinstance(detail_obj, dict) and "message" in detail_obj: + message = str(detail_obj.get("message")) + else: + message = str(detail_obj) if detail_obj is not None else "" raise SmritiError( - f"{method} {path} failed: HTTP {resp.status_code} — {detail}", + f"{method} {path} failed: HTTP {resp.status_code} — {message}", status=resp.status_code, + detail=detail_obj, ) if resp.status_code == 204 or not resp.content: @@ -82,6 +93,9 @@ class SmritiClient: json={"name": name, "description": description}, ) + def delete_space(self, space_id: str) -> None: + self._request("DELETE", f"/api/v2/repos/{space_id}") + def resolve_space(self, name_or_id: str) -> dict: """Look up a space by UUID or by name. Returns the full space dict. @@ -141,6 +155,13 @@ class SmritiClient: def review_checkpoint(self, commit_id: str) -> dict: return self._request("POST", f"/api/v5/checkpoint/{commit_id}/review") + def delete_commit(self, commit_id: str, cascade: bool = False) -> None: + params = {"cascade": "true"} if cascade else None + self._request("DELETE", f"/api/v2/commits/{commit_id}", params=params) + + def delete_session(self, session_id: str) -> None: + self._request("DELETE", f"/api/v4/chat/sessions/{session_id}") + # ── sessions (used internally by CLI) ────────────────────────────────── def create_session(self, repo_id: str, title: str = "", provider: str = "anthropic", model: str = "claude-sonnet-4-6") -> dict: diff --git a/cli/smriti_cli/main.py b/cli/smriti_cli/main.py index 3067546..905c037 100644 --- a/cli/smriti_cli/main.py +++ b/cli/smriti_cli/main.py @@ -1,14 +1,16 @@ """Smriti CLI entry point. -Seven commands for agent and programmatic use: +Commands for agent and programmatic use: smriti space list smriti space create [--description] + smriti space delete [-y] smriti state smriti checkpoint create # reads JSON from stdin smriti checkpoint show smriti checkpoint list smriti checkpoint review + smriti checkpoint delete [--cascade] [-y] Every command supports --json for structured output. Default output is a readable markdown brief. @@ -42,6 +44,29 @@ def _fail(message: str, code: int = 1) -> None: sys.exit(code) +def _confirm(preview: str, yes_flag: bool) -> bool: + """Interactive 'Type yes' if stdin is a TTY, otherwise require --yes. + + Destructive commands must be approved explicitly. When stdin is piped + (agent / script use) we refuse without --yes; when interactive we + require the full word 'yes' typed back. + """ + print(preview, file=sys.stderr) + if yes_flag: + return True + if not sys.stdin.isatty(): + print( + "error: refusing to proceed without --yes in non-interactive mode.", + file=sys.stderr, + ) + return False + try: + resp = input("Type 'yes' to confirm: ").strip().lower() + except EOFError: + return False + return resp == "yes" + + _USAGE_HINT = ( "No checkpoint JSON provided. Pipe JSON on stdin, or use --from-json .\n" "Example:\n" @@ -97,6 +122,26 @@ def cmd_space_create(client: SmritiClient, args: argparse.Namespace) -> None: print(f"Created space: {space['name']} `{space['id']}`") +def cmd_space_delete(client: SmritiClient, args: argparse.Namespace) -> None: + space = client.resolve_space(args.space) + commits = client.list_commits(space["id"]) + commit_count = len(commits) + preview = ( + f"Delete space '{space['name']}' (`{space['id']}`)?\n" + f" This will permanently delete {commit_count} checkpoint(s) " + f"and all sessions/turns under this space." + ) + if not _confirm(preview, args.yes): + _fail("Cancelled.", code=0) + client.delete_space(space["id"]) + if args.json: + _print_json( + {"deleted": True, "space_id": space["id"], "commits_deleted": commit_count} + ) + else: + print(f"Deleted space '{space['name']}' and its {commit_count} checkpoint(s).") + + def cmd_state(client: SmritiClient, args: argparse.Namespace) -> None: space = client.resolve_space(args.space) head = client.get_head(space["id"]) @@ -189,6 +234,42 @@ def cmd_checkpoint_review(client: SmritiClient, args: argparse.Namespace) -> Non print(format_review(result), end="") +def cmd_checkpoint_delete(client: SmritiClient, args: argparse.Namespace) -> None: + commit = client.get_commit(args.checkpoint_id) + preview = ( + f"Delete checkpoint '{commit.get('message', '')}' " + f"(`{commit['commit_hash'][:7]}`)?" + ) + if args.cascade: + preview += "\n --cascade set: descendant commits and forked sessions will also be deleted." + if not _confirm(preview, args.yes): + _fail("Cancelled.", code=0) + try: + client.delete_commit(args.checkpoint_id, cascade=args.cascade) + except SmritiError as e: + if e.status == 409 and isinstance(e.detail, dict): + deps = e.detail.get("dependents", {}) or {} + lines = [f"Refusing to delete: {e.detail.get('message', str(e))}"] + for c in deps.get("child_commits", []): + lines.append(f" - child commit: {c['label']} ({c['id']})") + for s in deps.get("forked_sessions", []): + lines.append(f" - forked session: {s['label']} ({s['id']})") + lines.append(" Re-run with --cascade to delete the subtree.") + _fail("\n".join(lines)) + raise + if args.json: + _print_json( + { + "deleted": True, + "checkpoint_id": args.checkpoint_id, + "cascade": args.cascade, + } + ) + else: + note = " (cascade)" if args.cascade else "" + print(f"Deleted checkpoint `{commit['commit_hash'][:7]}`{note}.") + + # ── argparse wiring ────────────────────────────────────────────────────── @@ -218,6 +299,17 @@ def _build_parser() -> argparse.ArgumentParser: sp_create.add_argument("--json", action="store_true", help="Output structured JSON") sp_create.set_defaults(func=cmd_space_create) + sp_delete = space_sub.add_parser( + "delete", + help="Delete a space and all its checkpoints, sessions, and turns", + ) + sp_delete.add_argument("space", help="Space name or UUID") + sp_delete.add_argument( + "-y", "--yes", action="store_true", help="Skip confirmation prompt" + ) + sp_delete.add_argument("--json", action="store_true", help="Output structured JSON") + sp_delete.set_defaults(func=cmd_space_delete) + # state state_parser = subparsers.add_parser( "state", @@ -269,6 +361,22 @@ def _build_parser() -> argparse.ArgumentParser: cp_review.add_argument("--json", action="store_true", help="Output structured JSON") cp_review.set_defaults(func=cmd_checkpoint_review) + cp_delete = cp_sub.add_parser( + "delete", + help="Delete a checkpoint. Refuses if it has children; pass --cascade to force.", + ) + cp_delete.add_argument("checkpoint_id", help="Checkpoint UUID") + cp_delete.add_argument( + "--cascade", + action="store_true", + help="Also delete descendant commits and forked sessions", + ) + cp_delete.add_argument( + "-y", "--yes", action="store_true", help="Skip confirmation prompt" + ) + cp_delete.add_argument("--json", action="store_true", help="Output structured JSON") + cp_delete.set_defaults(func=cmd_checkpoint_delete) + return parser diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 1d2bde8..63cb030 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -2,6 +2,34 @@ import type { Artifacts, ContextPack, Session, TargetTool } from '../types'; +/** + * Error thrown from any API helper on non-2xx responses. + * + * `detail` is the parsed `detail` field from the backend's error body when + * it is present. Some endpoints (notably `DELETE /api/v2/commits/{id}`) return + * structured details like `{ message, dependents }` so the UI can render a + * second-step cascade confirmation. + */ +export class ApiError extends Error { + status: number; + detail: unknown; + constructor(message: string, status: number, detail: unknown) { + super(message); + this.name = 'ApiError'; + this.status = status; + this.detail = detail; + } +} + +function _errorMessageFromDetail(detail: unknown, fallback: string): string { + if (typeof detail === 'string') return detail; + if (detail && typeof detail === 'object' && 'message' in detail) { + const msg = (detail as { message: unknown }).message; + if (typeof msg === 'string') return msg; + } + return fallback; +} + const BASE_URL = '/api/v1'; async function request(path: string, options?: RequestInit): Promise { @@ -10,9 +38,15 @@ async function request(path: string, options?: RequestInit): Promise { ...options, }); if (!res.ok) { - const error = await res.json().catch(() => ({ detail: res.statusText })); - throw new Error(error.detail || `HTTP ${res.status}`); + const body = await res.json().catch(() => ({ detail: res.statusText })); + const detail = body?.detail; + throw new ApiError( + _errorMessageFromDetail(detail, `HTTP ${res.status}`), + res.status, + detail, + ); } + if (res.status === 204) return undefined as T; return res.json(); } @@ -59,9 +93,15 @@ async function requestV2(path: string, options?: RequestInit): Promise { ...options, }); if (!res.ok) { - const error = await res.json().catch(() => ({ detail: res.statusText })); - throw new Error(error.detail || `HTTP ${res.status}`); + const body = await res.json().catch(() => ({ detail: res.statusText })); + const detail = body?.detail; + throw new ApiError( + _errorMessageFromDetail(detail, `HTTP ${res.status}`), + res.status, + detail, + ); } + if (res.status === 204) return undefined as T; return res.json(); } @@ -167,9 +207,15 @@ async function requestV4(path: string, options?: RequestInit): Promise { ...options, }); if (!res.ok) { - const error = await res.json().catch(() => ({ detail: res.statusText })); - throw new Error(error.detail || `HTTP ${res.status}`); + const body = await res.json().catch(() => ({ detail: res.statusText })); + const detail = body?.detail; + throw new ApiError( + _errorMessageFromDetail(detail, `HTTP ${res.status}`), + res.status, + detail, + ); } + if (res.status === 204) return undefined as T; return res.json(); } @@ -280,9 +326,15 @@ async function requestV5(path: string, options?: RequestInit): Promise { ...options, }); if (!res.ok) { - const error = await res.json().catch(() => ({ detail: res.statusText })); - throw new Error(error.detail || `HTTP ${res.status}`); + const body = await res.json().catch(() => ({ detail: res.statusText })); + const detail = body?.detail; + throw new ApiError( + _errorMessageFromDetail(detail, `HTTP ${res.status}`), + res.status, + detail, + ); } + if (res.status === 204) return undefined as T; return res.json(); } @@ -364,3 +416,30 @@ export async function reviewCheckpoint(checkpointId: string): Promise { + return requestV2(`/repos/${repoId}`, { method: 'DELETE' }); +} + +/** + * Delete a checkpoint. Without cascade the backend refuses with 409 if the + * checkpoint has child commits or forked sessions, and the ApiError.detail + * payload will contain { message, dependents } that the UI can use to render + * a second-step cascade confirmation. + */ +export async function deleteCommit( + commitId: string, + opts?: { cascade?: boolean }, +): Promise { + const qs = opts?.cascade ? '?cascade=true' : ''; + return requestV2(`/commits/${commitId}${qs}`, { method: 'DELETE' }); +} + +/** Delete a chat session. Cascades to turn events; commits authored by the + * session remain (they are owned by the space). */ +export async function deleteChatSession(sessionId: string): Promise { + return requestV4(`/chat/sessions/${sessionId}`, { method: 'DELETE' }); +} + diff --git a/frontend/src/components/ConfirmDeleteModal.tsx b/frontend/src/components/ConfirmDeleteModal.tsx new file mode 100644 index 0000000..00322a6 --- /dev/null +++ b/frontend/src/components/ConfirmDeleteModal.tsx @@ -0,0 +1,180 @@ +import { useState } from 'react'; +import { Trash2, Loader2, GitCommit, GitBranch, X } from 'lucide-react'; + +/** + * A dependent is something that references the object being deleted. + * The backend returns three kinds for checkpoint delete; space delete + * does not populate this at all. + */ +export type Dependent = { + kind: 'child_commit' | 'forked_session' | 'seeded_session'; + id: string; + label: string; +}; + +export type Dependents = { + checkpoint_id?: string; + child_commits: Dependent[]; + forked_sessions: Dependent[]; + seeded_sessions: Dependent[]; + blocking_count: number; +}; + +type Props = { + /** Title shown in the modal header, e.g. "Delete space 'foo'?". */ + title: string; + /** Body text explaining the consequences. May contain newlines. */ + body: string; + /** Optional override for the primary action label. Defaults to "Delete". */ + confirmLabel?: string; + /** + * When null/undefined, the modal is in its first-pass state: plain body, + * plain Delete button. + * + * When populated with dependents the modal is in its cascade-confirm state: + * it renders the blocking list and a "I understand..." checkbox that must + * be ticked before the Delete button re-enables. The label switches to + * "Delete subtree". + * + * The modal itself is stateless about which pass it's in — the parent + * catches the 409 ApiError, reads `error.detail.dependents`, and re-passes + * the modal with the new dependents prop to move to the second pass. + */ + dependents?: Dependents | null; + onClose: () => void; + /** Returns a promise so the modal can show a loading spinner while the + * caller hits the backend. Parent decides whether to close on success. */ + onConfirm: (cascade: boolean) => Promise; +}; + +export function ConfirmDeleteModal({ + title, + body, + confirmLabel, + dependents, + onClose, + onConfirm, +}: Props) { + const [cascadeConfirmed, setCascadeConfirmed] = useState(false); + const [loading, setLoading] = useState(false); + const [err, setErr] = useState(null); + + const blocking = dependents?.blocking_count ?? 0; + const inCascadeMode = blocking > 0; + const disabled = loading || (inCascadeMode && !cascadeConfirmed); + + const handleConfirm = async () => { + setErr(null); + setLoading(true); + try { + await onConfirm(inCascadeMode); + // Parent is responsible for calling onClose() in the success path + // because it may also want to refresh state, navigate, etc. We stay + // mounted in case the parent chooses to leave us open (e.g. to + // transition us into cascade-confirm mode on a 409). + } catch (e) { + setErr(e instanceof Error ? e.message : String(e)); + } finally { + setLoading(false); + } + }; + + return ( +
{ if (e.target === e.currentTarget) onClose(); }} + > +
+ {/* Header */} +
+

+ {title} +

+ +
+ + {/* Body */} +
+

{body}

+ + {/* Cascade callout — second pass only */} + {inCascadeMode && ( +
+

+ This will also delete: +

+
    + {dependents!.child_commits.map(d => ( +
  • + + {d.label} +
    • + ))} + {dependents!.forked_sessions.map(d => ( +
  • + + {d.label} +
    • + ))} +
+ +
+ )} + + {/* Informational seeded_sessions — never blocks */} + {dependents && dependents.seeded_sessions.length > 0 && ( +

+ {dependents.seeded_sessions.length} session + {dependents.seeded_sessions.length === 1 ? ' was' : 's were'} opened + from this checkpoint; they will keep their materialised turns but lose + the seed pointer. +

+ )} + + {err &&

{err}

} +
+ + {/* Footer */} +
+ + +
+
+
+ ); +} + +export default ConfirmDeleteModal; diff --git a/frontend/src/pages/ChatWorkspacePage.tsx b/frontend/src/pages/ChatWorkspacePage.tsx index 2132745..6a82d58 100644 --- a/frontend/src/pages/ChatWorkspacePage.tsx +++ b/frontend/src/pages/ChatWorkspacePage.tsx @@ -1,8 +1,10 @@ import React, { useCallback, useEffect, useRef, useState } from 'react'; import { useNavigate, useParams } from 'react-router-dom'; import { + ApiError, createChatCommit, createChatSessionGeneric, + deleteCommit, getChatSessionGeneric, getRepo, getRepos, @@ -39,10 +41,12 @@ import { FolderInput, RotateCcw, Paperclip, + Trash2, X, ChevronDown, ChevronRight, } from 'lucide-react'; +import { ConfirmDeleteModal, type Dependents as DependentsPayload } from '../components/ConfirmDeleteModal'; // ── Provider / model config ─────────────────────────────────────────────────── @@ -831,6 +835,7 @@ function MemorySpacePanel({ providerStatus, onMount, onFork, + onDeleteCheckpoint, onClose, }: { repoId: string; @@ -843,6 +848,7 @@ function MemorySpacePanel({ providerStatus: Record | null; onMount: (info: { id: string; message: string } | null) => void; onFork: (checkpoint: Commit) => void; + onDeleteCheckpoint: (checkpoint: Commit) => void; onClose: () => void; }) { const navigate = useNavigate(); @@ -1051,6 +1057,14 @@ function MemorySpacePanel({ Fork + @@ -1316,6 +1330,11 @@ export function ChatWorkspacePage() { const [mountedAtSeq, setMountedAtSeq] = useState(null); const [showHistoryPanel, setShowHistoryPanel] = useState(false); const [forkTarget, setForkTarget] = useState(null); + // Delete-checkpoint flow state. `deleteCheckpointTarget` opens the modal; + // `deleteCheckpointDependents` is populated after a 409 response to show + // the cascade-confirm second step. + const [deleteCheckpointTarget, setDeleteCheckpointTarget] = useState(null); + const [deleteCheckpointDependents, setDeleteCheckpointDependents] = useState(null); // Artifacts captured from messages, passed to CommitModal when opening const [pendingArtifacts, setPendingArtifacts] = useState([]); @@ -2051,6 +2070,10 @@ export function ChatWorkspacePage() { } }} onFork={checkpoint => setForkTarget(checkpoint)} + onDeleteCheckpoint={checkpoint => { + setDeleteCheckpointTarget(checkpoint); + setDeleteCheckpointDependents(null); + }} onClose={() => setShowHistoryPanel(false)} /> )} @@ -2067,6 +2090,48 @@ export function ChatWorkspacePage() { }} /> )} + + {/* ── Delete Checkpoint Modal ───────────────────────────────── */} + {deleteCheckpointTarget && ( + { + setDeleteCheckpointTarget(null); + setDeleteCheckpointDependents(null); + }} + onConfirm={async (cascade) => { + try { + await deleteCommit(deleteCheckpointTarget.id, { cascade }); + if (mountedCheckpointId === deleteCheckpointTarget.id) { + setMountedCheckpointId(null); + setMountedCheckpointLabel(null); + setMountedAtSeq(null); + } + const wasForkSource = forkSourceCommit?.id === deleteCheckpointTarget.id; + setDeleteCheckpointTarget(null); + setDeleteCheckpointDependents(null); + setCheckpointRefreshKey(k => k + 1); + if (wasForkSource) { + navigate('/'); + } + } catch (e) { + if ( + e instanceof ApiError && + e.status === 409 && + e.detail && + typeof e.detail === 'object' && + 'dependents' in e.detail + ) { + setDeleteCheckpointDependents((e.detail as { dependents: DependentsPayload }).dependents); + } else { + throw e; + } + } + }} + /> + )} diff --git a/frontend/src/pages/WorkspaceOverviewPage.tsx b/frontend/src/pages/WorkspaceOverviewPage.tsx index a55dc08..aa19270 100644 --- a/frontend/src/pages/WorkspaceOverviewPage.tsx +++ b/frontend/src/pages/WorkspaceOverviewPage.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { useNavigate } from 'react-router-dom'; import { ArrowRight, @@ -7,16 +7,20 @@ import { GitCommit, Loader2, MessageSquare, + MoreVertical, Plus, RotateCcw, Settings2, + Trash2, } from 'lucide-react'; import { + deleteRepo, getRecentSessionsGeneric, getRepos, getSpaceHead, } from '../api/client'; import type { ChatSession, HeadState, Repo } from '../types'; +import { ConfirmDeleteModal } from '../components/ConfirmDeleteModal'; /** * Landing page focused on resume. @@ -35,6 +39,43 @@ export function WorkspaceOverviewPage() { const [headBySpace, setHeadBySpace] = useState>({}); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); + const [openMenuId, setOpenMenuId] = useState(null); + const [deleteTarget, setDeleteTarget] = useState(null); + const menuContainerRef = useRef(null); + + // Close the kebab menu when clicking anywhere outside of it. + useEffect(() => { + if (!openMenuId) return; + const handler = (e: MouseEvent) => { + if (!menuContainerRef.current) return; + if (!menuContainerRef.current.contains(e.target as Node)) { + setOpenMenuId(null); + } + }; + document.addEventListener('mousedown', handler); + return () => document.removeEventListener('mousedown', handler); + }, [openMenuId]); + + const refreshWorkspace = async () => { + const [s, r] = await Promise.all([getRecentSessionsGeneric(), getRepos()]); + setSessions(s); + setRepos(r); + const repoIds = Array.from( + new Set(s.map(sess => sess.repo_id).filter((id): id is string => !!id)), + ).slice(0, 8); + const heads = await Promise.all( + repoIds.map(id => + getSpaceHead(id) + .then(h => [id, h] as const) + .catch(() => null), + ), + ); + const headMap: Record = {}; + for (const entry of heads) { + if (entry) headMap[entry[0]] = entry[1]; + } + setHeadBySpace(headMap); + }; useEffect(() => { (async () => { @@ -169,6 +210,10 @@ export function WorkspaceOverviewPage() {

Pick up where you left off

+
+ + {openMenuId === resumeRepo.id && ( +
+ +
+ )} + ) : sessions.length > 0 ? ( // Fallback: no checkpointed session to resume, but sessions exist @@ -265,36 +336,66 @@ export function WorkspaceOverviewPage() { const head = headBySpace[id]; const latestSession = sessions.find(s => s.repo_id === id); return ( - + + {openMenuId === id && ( +
+ +
)} - {latestSession && ( -

- {formatAgo(latestSession.updated_at)} -

- )} - + ); })} @@ -322,6 +423,19 @@ export function WorkspaceOverviewPage() { )} + + {deleteTarget && ( + setDeleteTarget(null)} + onConfirm={async () => { + await deleteRepo(deleteTarget.id); + setDeleteTarget(null); + await refreshWorkspace(); + }} + /> + )} ); }