mirror of
https://github.com/himanshudongre/smriti.git
synced 2026-08-28 05:14:59 +00:00
Require --force to delete a non-empty or attached space
A destructive-operations incident: `smriti space delete <space> -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.
This commit is contained in:
parent
0c8f6922f4
commit
4a1976a905
2 changed files with 115 additions and 0 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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 <space> -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")
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue