From 4a1976a9054317ffe8bd437b0c1982d7045fdf9d Mon Sep 17 00:00:00 2001 From: Himanshu Dongre Date: Tue, 19 May 2026 00:51:33 +0530 Subject: [PATCH] Require --force to delete a non-empty or attached space MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A destructive-operations incident: `smriti space delete -y` cascade-deleted a fully populated space. `-y` skipped the only gate (the confirmation prompt), and `cmd_space_delete` never checked how much the space held — so one flag irreversibly deleted the space and every checkpoint, session, and turn under it. Gate destructive deletes in `cmd_space_delete`: a space that holds checkpoints, or that the current repo is attached to (.smriti.json), now requires an explicit `--force` — `-y` alone is refused, with a message naming the reason and the flag. `--force` and `-y` stay orthogonal, the same shape as `checkpoint delete --cascade`. Empty, unattached spaces keep the existing `-y` convenience. CLI-side only; no backend change. --- cli/smriti_cli/main.py | 39 +++++++++++++++++++ cli/tests/test_space_cli.py | 76 +++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/cli/smriti_cli/main.py b/cli/smriti_cli/main.py index a5c5223..bfcbb99 100644 --- a/cli/smriti_cli/main.py +++ b/cli/smriti_cli/main.py @@ -612,11 +612,45 @@ def cmd_space_delete(client: SmritiClient, args: argparse.Namespace) -> None: space = _resolve_space(client, args) commits = client.list_commits(space["id"]) commit_count = len(commits) + + # Is this the space the current repo is attached to? + record = attachment.read_attachment() + is_attached = bool(record) and ( + record.get("space_id") == space["id"] + or record.get("space") == space["name"] + ) + + # Destructive-delete guard. `-y` skips the confirmation prompt, but it must + # NOT, on its own, delete a space that holds real work or is attached to a + # repo. Those need an explicit, separate --force signal — the same + # "name the stronger flag" shape as `checkpoint delete --cascade`. + blockers: list[str] = [] + if commit_count > 0: + blockers.append( + f"it holds {commit_count} checkpoint(s); deletion cascades to all of " + f"them plus every session and turn, and cannot be undone" + ) + if is_attached: + blockers.append( + "this repo is attached to it (.smriti.json); deleting it unbinds the repo" + ) + if blockers and not args.force: + lines = [f"Refusing to delete space '{space['name']}' (`{space['id']}`):"] + lines += [f" - {b}" for b in blockers] + lines.append("") + lines.append( + " -y is not enough for a destructive delete like this. If you are " + "certain,\n re-run with --force (required in addition to -y or the prompt)." + ) + _fail("\n".join(lines)) + 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 is_attached: + preview += "\n This repo is attached to this space — deleting it unbinds the repo." if not _confirm(preview, args.yes): _fail("Cancelled.", code=0) client.delete_space(space["id"]) @@ -1916,6 +1950,11 @@ def _build_parser() -> argparse.ArgumentParser: sp_delete.add_argument( "-y", "--yes", action="store_true", help="Skip confirmation prompt" ) + sp_delete.add_argument( + "--force", + action="store_true", + help="Required to delete a non-empty or attached space (irreversible)", + ) sp_delete.add_argument("--json", action="store_true", help="Output structured JSON") sp_delete.set_defaults(func=cmd_space_delete) diff --git a/cli/tests/test_space_cli.py b/cli/tests/test_space_cli.py index 14a1774..a454f5f 100644 --- a/cli/tests/test_space_cli.py +++ b/cli/tests/test_space_cli.py @@ -5,6 +5,7 @@ from unittest.mock import MagicMock import pytest +from smriti_cli import attachment from smriti_cli import main as cli_main from smriti_cli.client import SmritiClient @@ -195,3 +196,78 @@ def test_cmd_space_set_project_root_here_flag_resolves_cwd(tmp_path, monkeypatch cli_main.cmd_space_set_project_root(client, args) client.set_project_root.assert_called_once_with("space-uuid", str(tmp_path)) + + +# ── space delete: destructive-delete guard ─────────────────────────────────── +# +# Incident: `smriti space delete -y` cascade-deleted a populated space. +# A non-empty or attached space must now require an explicit --force. + + +def _delete_args(space="my-project", yes=False, force=False, json=False): + return argparse.Namespace(space=space, yes=yes, force=force, json=json) + + +def test_space_delete_parser_has_force_flag(): + parser = cli_main._build_parser() + args = parser.parse_args(["space", "delete", "my-project", "--force", "-y"]) + assert args.force is True + assert args.yes is True + # --force defaults off + assert parser.parse_args(["space", "delete", "my-project"]).force is False + + +def test_cmd_space_delete_empty_unattached_space_allows_yes(tmp_path, monkeypatch): + """Empty, unattached space: -y still deletes it (convenience retained).""" + monkeypatch.chdir(tmp_path) + client = MagicMock(spec=SmritiClient) + client.resolve_space.return_value = _space_dict() + client.list_commits.return_value = [] # empty + cli_main.cmd_space_delete(client, _delete_args(yes=True)) + client.delete_space.assert_called_once_with("space-uuid") + + +def test_cmd_space_delete_nonempty_refused_without_force(tmp_path, monkeypatch, capsys): + """The incident path: -y alone must NOT delete a populated space.""" + monkeypatch.chdir(tmp_path) + client = MagicMock(spec=SmritiClient) + client.resolve_space.return_value = _space_dict() + client.list_commits.return_value = [{"id": "c1"}, {"id": "c2"}] # non-empty + with pytest.raises(SystemExit): + cli_main.cmd_space_delete(client, _delete_args(yes=True, force=False)) + client.delete_space.assert_not_called() + err = capsys.readouterr().err + assert "Refusing to delete" in err + assert "--force" in err + + +def test_cmd_space_delete_nonempty_allowed_with_force(tmp_path, monkeypatch): + """A populated space deletes only with --force (in addition to -y).""" + monkeypatch.chdir(tmp_path) + client = MagicMock(spec=SmritiClient) + client.resolve_space.return_value = _space_dict() + client.list_commits.return_value = [{"id": "c1"}, {"id": "c2"}] + cli_main.cmd_space_delete(client, _delete_args(yes=True, force=True)) + client.delete_space.assert_called_once_with("space-uuid") + + +def test_cmd_space_delete_attached_space_refused_without_force(tmp_path, monkeypatch): + """An attached space is force-gated even when it is empty.""" + monkeypatch.chdir(tmp_path) + attachment.write_attachment(tmp_path, "my-project", "space-uuid") + client = MagicMock(spec=SmritiClient) + client.resolve_space.return_value = _space_dict() + client.list_commits.return_value = [] # empty, but attached + with pytest.raises(SystemExit): + cli_main.cmd_space_delete(client, _delete_args(yes=True, force=False)) + client.delete_space.assert_not_called() + + +def test_cmd_space_delete_attached_space_allowed_with_force(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + attachment.write_attachment(tmp_path, "my-project", "space-uuid") + client = MagicMock(spec=SmritiClient) + client.resolve_space.return_value = _space_dict() + client.list_commits.return_value = [] + cli_main.cmd_space_delete(client, _delete_args(yes=True, force=True)) + client.delete_space.assert_called_once_with("space-uuid")