diff --git a/docs/user-guide.md b/docs/user-guide.md index 0b3604a2..4902b2c3 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -148,12 +148,13 @@ More you can say: - "Stacky, Homer should book the tickets by Friday" puts a name and a day on an item. - "Stacky, what's on the supermarket list?" shows what is still open. - "Stacky, we're back from camping, reset the packing list." Start a list you use again and again (packing, the weekly chores) as a recurring list: its ticks stay until you reset it. +- "Stacky, we don't need the hardware store list any more" deletes it. If it still has open items, Stacky names them and asks first. What was bought stays in the archive, so a list started again under the same name still knows where things go. **Letters and notes put their to-dos on lists too.** When a letter in **#Documents** asks you to do something ("sign the permission slip by 23 October"), the archivist puts it on the list of the letter's topic, with the day and a link back to the letter. A short note in your own words, pinned with 📌 in a [topic room](#topic-rooms) ("we should take out the trash by Friday"), does the same. Long pasted text and links never turn into to-dos: someone else's tips are not your chores. Tick these off with Stacky like anything else. **A list belongs to a topic.** In a topic room, Stacky uses that topic's lists. To tie an existing list to a room: "Stacky, the supermarket list belongs to this room." -Every list is a page in the [family wiki](#the-family-wiki) and a plain Markdown checklist in the vault, so you can also tick items in Obsidian or in Forgejo's editor. Stacky does not hear voice messages or see photos yet: a screenshot of a list has to be typed in for now. +Every list is a page in the [family wiki](#the-family-wiki) and a plain Markdown checklist, so you can also tick items in Obsidian or in Forgejo's editor. Stacky does not hear voice messages or see photos yet: a screenshot of a list has to be typed in for now. --- diff --git a/lib/stack/forgejo.py b/lib/stack/forgejo.py index c695334d..4db1d2a6 100644 --- a/lib/stack/forgejo.py +++ b/lib/stack/forgejo.py @@ -347,7 +347,7 @@ def put_file(self, owner: str, repo: str, path: str, *, ) def edit_file(self, owner: str, repo: str, path: str, - transform: Callable[[str], str], *, + transform: Callable[[str], str | None], *, message: str | Callable[[str, str], str], branch: str = "main", author_name: str | None = None, author_email: str | None = None) -> dict | None: @@ -371,14 +371,25 @@ def edit_file(self, owner: str, repo: str, path: str, Passing the function instead is what lets a commit subject say "ticked off Kühlbox" rather than "updated todos.md", and a history worth reading is the difference. + + A transform answering `None` deletes the file, against the hash it + was read at and authored like any other edit, so removing a page is + the same decision on the same current text as changing it. Deleting + a file that is not there commits nothing. """ existing = self.get_file(owner, repo, path, ref=branch) prior = existing.get("content", "") if existing else "" sha = existing.get("sha") if existing else None merged = transform(prior) - if merged == prior: + if merged == prior or (merged is None and not existing): return None - subject = message(prior, merged) if callable(message) else message + subject = message(prior, merged or "") if callable(message) else message + if merged is None: + return self.change_files( + owner, repo, [FileChange("delete", path, sha=sha or "")], + message=subject, branch=branch, + author_name=author_name, author_email=author_email, + ) return self.put_file( owner, repo, path, content=merged, message=subject, branch=branch, sha=sha, diff --git a/stacklets/agent/runtime/list_tool.py b/stacklets/agent/runtime/list_tool.py index f0f40269..8f512fd5 100644 --- a/stacklets/agent/runtime/list_tool.py +++ b/stacklets/agent/runtime/list_tool.py @@ -17,6 +17,7 @@ from nanobot.agent.tools.base import Tool, tool_parameters from nanobot.agent.tools.schema import ( ArraySchema, + BooleanSchema, StringSchema, tool_parameters_schema, ) @@ -43,8 +44,10 @@ "list. reset reopens every ticked item. create starts a new " "list when none fits, such as a store the family has no list " "for yet. set changes a list's title, description, topic or " - "sections, such as linking it to this room's topic.", - enum=("add", "tick", "untick", "remove", "move", "reset", "create", "set"), + "sections, such as linking it to this room's topic. delete " + "removes a whole list nobody needs any more.", + enum=("add", "tick", "untick", "remove", "move", "reset", "create", "set", + "delete"), ), items=ArraySchema( StringSchema("One item's text, as it appears on the list."), @@ -78,6 +81,11 @@ "walks through it. Leave out for a list without sections.", nullable=True, ), + confirmed=BooleanSchema( + description="For delete: true only after the person said yes to " + "deleting a list that still has open items.", + nullable=True, + ), ) ) class ListEditTool(Tool): @@ -93,7 +101,7 @@ def name(self) -> str: def description(self) -> str: return ( "Add, tick, untick, remove or move items on a family list, by " - "name, or start a new list. The store matches each item and answers with what " + "name, or start or delete a list. The store matches each item and answers with what " "actually changed. Use this for every item change; use " "write_file only to restructure a whole list." ) @@ -115,6 +123,7 @@ async def execute( title: str | None = None, about: str | None = None, sections: "list[str] | None" = None, + confirmed: bool | None = None, ) -> str: try: import brief @@ -124,6 +133,11 @@ async def execute( if op in ("create", "set"): args = _details_args(op, list, title, about, topic, sections) + elif op == "delete": + args = ["stack", "memory", "list", "delete", (list or "").strip()] + # Only a real yes: nanobot casts "true"/"false" but passes any + # other string through, and a stray string is not a person's yes. + args += ["--yes"] if confirmed is True else [] else: args = _item_args(op, list, items, topic, section, people, due, to) args += ["--as", actor] diff --git a/stacklets/agent/workspace/skills/family-memory/SKILL.md b/stacklets/agent/workspace/skills/family-memory/SKILL.md index f58d6840..85cd4e8e 100644 --- a/stacklets/agent/workspace/skills/family-memory/SKILL.md +++ b/stacklets/agent/workspace/skills/family-memory/SKILL.md @@ -40,6 +40,8 @@ LISTS: "hole X lieber auf dem Markt" | "kaufe X woanders" | "X gibt es bei Y" -> move no list fits (a new store) -> op=create list= title about topic sections, then add "belongs here|rename|new order|new sections" -> op=set list= with only what changes + "delete|drop list X|don't need X any more" -> op=delete list=X; never empty it item by item + delete answer "still has N open items" -> ask whether those go too; yes -> op=delete confirmed=true add to a list with sections -> section = one of the list's own "for " | "by " -> add with people=[name] | due=YYYY-MM-DD; never guess either two items fit what was said -> ask which one, never pick diff --git a/stacklets/memory/cli/list.py b/stacklets/memory/cli/list.py index fe0a5c08..3ce1c4fa 100644 --- a/stacklets/memory/cli/list.py +++ b/stacklets/memory/cli/list.py @@ -79,9 +79,11 @@ stack memory list set einkaufsliste --topic einkauf --as marge the list's title, description, topics or walking order (--section, in order) + stack memory list delete baumarkt [--yes] --as marge + --yes: even with items still open """ -VERBS = ("show", "create", "add", "tick", "untick", "remove", "move", "reset", "set") +VERBS = ("show", "create", "add", "tick", "untick", "remove", "move", "reset", "set", "delete") # A commit subject's length, as git and Forgejo show it without wrapping. _SUBJECT_MAX = 72 @@ -147,7 +149,12 @@ def _parser() -> argparse.ArgumentParser: reset = verbs.add_parser("reset", add_help=False) reset.add_argument("slug") - for sub in (create, add, move, reset, details, *[verbs.choices[v] for v in ("tick", "untick", "remove")]): + delete = verbs.add_parser("delete", add_help=False) + delete.add_argument("slug") + delete.add_argument("--yes", action="store_true") + + for sub in (create, add, move, reset, details, delete, + *[verbs.choices[v] for v in ("tick", "untick", "remove")]): sub.add_argument("--as", dest="actor", default="") return parser @@ -180,6 +187,8 @@ def run(args, stacklet, config): return change_details(vault, config, ns, actor) if ns.verb == "move": return move(vault, config, ns.slug, ns.items, ns.to, actor) + if ns.verb == "delete": + return delete(vault, config, ns.slug, actor, even_open=ns.yes) if ns.verb == "add": return add(vault, config, ns, actor) return change(vault, config, ns, actor) @@ -453,6 +462,29 @@ def _combined(results: list[dict]) -> dict: return {"error": "; ".join(errors)} if errors else {"ok": True, "lists": results} +def delete(vault: Path, config, slug: str, actor: str, *, even_open: bool) -> dict: + """Delete a list, so a list nobody needs is gone instead of left empty. + + Open items are what someone still meant to do, so a list with any is + deleted only with `--yes`; the refusal names them, for the caller to + ask whoever wanted the list gone. The archive stays: it is the record + of what was bought, and a list started again under the same name + finds its known items there. + """ + path, lst = _find_list(vault, slug) + + def _delete(prior: str) -> None: + current = read_list(prior, path=path) + if current.open_items and not even_open: + open_items = current.open_items + count = "1 open item" if len(open_items) == 1 else f"{len(open_items)} open items" + raise ValueError(f"{current.title} still has {count}: " + f"{'; '.join(i.text for i in open_items)}. " + "Delete it with them: add --yes") + return None + return _commit(config, path, _delete, actor, f"deleted the list {lst.title}") + + def move(vault: Path, config, slug: str, names: list[str], to: str, actor: str) -> dict: """Move items to another list: added there first, so a failure never loses one.""" path, lst = _find_list(vault, slug) @@ -582,7 +614,13 @@ def _commit(config, path: str, transform, actor: str, """ def _said() -> str: return said() if callable(said) else said - result = update_memory(config, path, transform, actor=actor, + + written: dict = {} + + def _recorded(prior: str) -> str | None: + written["after"] = transform(prior) + return written["after"] + result = update_memory(config, path, _recorded, actor=actor, message=lambda before, after: _commit_message(actor, path, _said())) if "error" in result or quiet: return result @@ -591,7 +629,9 @@ def _said() -> str: return {"ok": True, "committed": False, "path": path, "summary": _said()} lag = "" if result.get("mirrored") else "\n The wiki and the vault mount catch up shortly." home_url = (config or {}).get("home_url", "") - url = public(go_page(path), f"{home_url}/go" if home_url else "") + # A transform answering None deleted the list: there is no page to link. + deleted = written.get("after", "") is None + url = "" if deleted else public(go_page(path), f"{home_url}/go" if home_url else "") link = f"\n {url}" if url else "" print(f"{_said()} (by {actor}){link}{lag}") return {"ok": True, "committed": True, "path": path, "by": actor, "summary": _said()} diff --git a/stacklets/memory/lib.py b/stacklets/memory/lib.py index f29d748d..c9649574 100644 --- a/stacklets/memory/lib.py +++ b/stacklets/memory/lib.py @@ -946,7 +946,7 @@ def _actor_identity(actor: str, config: dict | None) -> tuple[str, str]: def update_memory(config: dict, repo_path: str, - transform: Callable[[str], str], *, + transform: Callable[[str], str | None], *, actor: str, message: str | Callable[[str, str], str]) -> dict: """Commit a transform of one vault file to Forgejo, attributed to `actor`. @@ -963,6 +963,8 @@ def update_memory(config: dict, repo_path: str, current file. The vault's history is read by people and by the agent, so a subject that names what changed is worth the indirection. + A transform answering `None` deletes the file (`ForgejoClient.edit_file`). + Returns the framework envelope: `{"ok": True, "committed": bool}` on success (committed=False when the transform was a no-op, so nothing was written), or `{"error": ...}` when credentials are missing, the transform @@ -2471,10 +2473,10 @@ def move_todos_to_lists(client: ForgejoClient, *, if not any(i.text.casefold() == item.text.casefold() for i in read_list(content, path=new_path).items): content = insert_item(content, item, path=new_path) - changes.append(FileChange("update", new_path, content, sha=current.get("sha"))) + changes.append(FileChange("update", new_path, content, sha=current.get("sha") or "")) else: changes.append(FileChange("create", new_path, converted)) - changes.append(FileChange("delete", old_path, sha=old.get("sha"))) + changes.append(FileChange("delete", old_path, sha=old.get("sha") or "")) moved.append(topic) if changes: client.change_files( diff --git a/tests/unit/framework/test_forgejo_edit_file.py b/tests/unit/framework/test_forgejo_edit_file.py new file mode 100644 index 00000000..44ff36a5 --- /dev/null +++ b/tests/unit/framework/test_forgejo_edit_file.py @@ -0,0 +1,57 @@ +"""A read-modify-write of one file, whose transform can also remove it. + +`edit_file` reads the current text, hands it to the caller's transform +and commits the answer against the hash it read. A transform that +answers `None` means the file should be gone: the same seam then +deletes it, in a commit authored by the person, so a deletion is in the +history under their name like any other change. + +Forgejo is a real HTTP server here (`pytest-httpserver`) answering the +documented contents API. +""" + +from __future__ import annotations + +import base64 +import json + +from stack.forgejo import ForgejoClient + +CONTENTS = "/api/v1/repos/family/memory/contents" + + +def _client(httpserver) -> ForgejoClient: + return ForgejoClient(url=httpserver.url_for(""), token="bottoken") + + +def _file(text: str, sha: str) -> dict: + return {"type": "file", "content": base64.b64encode(text.encode()).decode(), "sha": sha} + + +def test_a_transform_answering_none_deletes_the_file_as_the_person(httpserver): + httpserver.expect_request(f"{CONTENTS}/family/lists/baumarkt.md", + method="GET").respond_with_json(_file("- [x] Dübel\n", "s1")) + httpserver.expect_request(CONTENTS, method="POST").respond_with_json({}, status=201) + + _client(httpserver).edit_file( + "family", "memory", "family/lists/baumarkt.md", lambda prior: None, + message=lambda before, after: f"deleted ({len(before)} chars, after {after!r})", + author_name="marge", author_email="marge@simpson") + + post = next(r for r, _ in httpserver.log if r.method == "POST") + body = json.loads(post.data) + assert body["files"] == [{"operation": "delete", "path": "family/lists/baumarkt.md", + "sha": "s1"}], "deleted against the hash it was read at" + assert body["author"] == {"name": "marge", "email": "marge@simpson"} + assert body["message"] == "deleted (12 chars, after '')" + + +def test_deleting_a_file_that_is_not_there_commits_nothing(httpserver): + httpserver.expect_request(f"{CONTENTS}/family/lists/baumarkt.md", + method="GET").respond_with_data("", status=404) + + result = _client(httpserver).edit_file( + "family", "memory", "family/lists/baumarkt.md", lambda prior: None, message="m") + + assert result is None + assert not [r for r, _ in httpserver.log if r.method != "GET"] diff --git a/tests/unit/stacklets/conftest.py b/tests/unit/stacklets/conftest.py index 23c9a818..8d69d8f5 100644 --- a/tests/unit/stacklets/conftest.py +++ b/tests/unit/stacklets/conftest.py @@ -168,6 +168,7 @@ def mod(name, **attrs): mod("nanobot.agent.tools.base", Tool=Tool, tool_parameters=tool_parameters) mod("nanobot.agent.tools.schema", StringSchema=_Schema, IntegerSchema=_Schema, ArraySchema=_Schema, + BooleanSchema=_Schema, tool_parameters_schema=tool_parameters_schema) mod("nanobot.agent.tools.loader", ToolLoader=ToolLoader) mod("nanobot.agent.tools.registry", ToolRegistry=ToolRegistry) diff --git a/tests/unit/stacklets/test_agent_vault_tools.py b/tests/unit/stacklets/test_agent_vault_tools.py index 87b574ba..625a00c4 100644 --- a/tests/unit/stacklets/test_agent_vault_tools.py +++ b/tests/unit/stacklets/test_agent_vault_tools.py @@ -179,6 +179,8 @@ def test_the_allowlist_stays_read_only(api): ("list_edit", "list", {"op": "create", "title": "Baumarkt", "items": ["Schrauben"]}), ("list_edit", "list", {"op": "tick", "items": ["Erlaubniszettel"]}), + ("list_edit", "list", {"list": "baumarkt", "op": "delete", "items": ["ignored"]}), + ("list_edit", "list", {"list": "baumarkt", "op": "delete", "confirmed": True}), ("list_edit", "list", {"list": "einkaufsliste", "op": "set", "topic": "einkauf"}), ("list_edit", "list", @@ -234,6 +236,26 @@ def test_search_sends_no_backend_flag(vault_tools): assert "--backend" not in argv +def test_a_list_with_open_items_is_deleted_only_once_the_person_said_yes(vault_tools): + """The store refuses to delete a list with open items unless told `--yes`. + + The model decides nothing here: `--yes` is sent only when it says the + person confirmed, so a misheard "delete" meets the refusal that names + the open items, and the person is asked. + """ + asked = argv_of(vault_tools["list_edit"], list="baumarkt", op="delete") + confirmed = argv_of(vault_tools["list_edit"], list="baumarkt", op="delete", + confirmed=True) + + unclear = argv_of(vault_tools["list_edit"], list="baumarkt", op="delete", + confirmed="maybe") + + assert "--yes" not in asked + assert "--yes" not in unclear, "only a real yes deletes open items" + assert confirmed[:5] == ["stack", "memory", "list", "delete", "baumarkt"] + assert "--yes" in confirmed + + def result_of(tool_cls, *, returncode: int, stdout: bytes = b"", stderr: bytes = b"", **kwargs) -> str: """What a tool returns to the model for a given CLI outcome. diff --git a/tests/unit/stacklets/test_memory_list_cli.py b/tests/unit/stacklets/test_memory_list_cli.py index e46bc4ef..16ff0bf4 100644 --- a/tests/unit/stacklets/test_memory_list_cli.py +++ b/tests/unit/stacklets/test_memory_list_cli.py @@ -10,8 +10,9 @@ The `update_memory` seam is substituted, because the real one talks to Forgejo. The stand-in keeps what the command depends on: the transform gets the current file, a `ValueError` becomes an error envelope, an -unchanged file commits nothing, and a commit reaches the vault on disk -the way the real write-through pull does. +unchanged file commits nothing, a `None` answer deletes the file, and a +commit reaches the vault on disk the way the real write-through pull +does. """ from __future__ import annotations @@ -67,11 +68,14 @@ def update_memory(self, config, repo_path, transform, *, actor, message): after = transform(before) except ValueError as e: return {"error": str(e)} - if after == before: + if after == before or (after is None and not before): return {"ok": True, "committed": False} - (vault / repo_path).parent.mkdir(parents=True, exist_ok=True) - (vault / repo_path).write_text(after, encoding="utf-8") - self.commits.append((repo_path, actor, message(before, after))) + if after is None: + (vault / repo_path).unlink() + else: + (vault / repo_path).parent.mkdir(parents=True, exist_ok=True) + (vault / repo_path).write_text(after, encoding="utf-8") + self.commits.append((repo_path, actor, message(before, after or ""))) return {"ok": True, "committed": True, "path": repo_path, "mirrored": True} fake = _Store() @@ -325,6 +329,49 @@ def test_a_list_that_does_not_exist_is_refused_with_the_lists(self, run, groceri assert result["error"] == "no list 'baumarkt'. Lists: einkaufsliste" +class TestDeletingAList: + """A list nobody needs any more is deleted, never emptied and left behind. + + Deleting a list that still has open items needs `--yes`: the refusal + names them, so whoever asked can be asked whether those go too. + """ + + def test_a_list_with_nothing_open_is_deleted_in_one_commit(self, run, store, groceries): + result = run("delete", "einkaufsliste", "--as", "marge") + assert result["committed"] and not store.text(groceries) + assert store.commits[-1][1:] == ("marge", "chore(lists): marge deleted the list Einkaufsliste") + + def test_the_answer_links_no_page_that_is_gone(self, run, groceries, capsys, monkeypatch): + monkeypatch.setattr(list_cli, "public", lambda page, base: f"https://home.example/go/{page}") + run("delete", "einkaufsliste", "--as", "marge") + assert capsys.readouterr().out.splitlines()[-1] == \ + "deleted the list Einkaufsliste (by marge)" + + def test_open_items_are_named_and_kept_without_yes(self, run, store, groceries): + run("add", "einkaufsliste", "Milch", "Äpfel", "--as", "marge") + result = run("delete", "einkaufsliste", "--as", "marge") + assert result["error"] == ("Einkaufsliste still has 2 open items: Milch; Äpfel. " + "Delete it with them: add --yes") + assert "Milch" in store.text(groceries) + + def test_yes_deletes_it_with_its_open_items(self, run, store, groceries): + run("add", "einkaufsliste", "Milch", "--as", "marge") + assert run("delete", "einkaufsliste", "--yes", "--as", "marge")["committed"] + assert not store.text(groceries) + assert run("show")["lists"] == [] + + def test_its_archive_stays_as_the_history_of_what_was_bought(self, run, store, groceries): + archive = store.vault / "family" / "lists" / "archive" / "einkaufsliste" / "2026-09.md" + archive.parent.mkdir(parents=True) + archive.write_text("---\ntype: list\ntitle: Einkaufsliste\n---\n\n- [x] Milch ✅ 2026-09-02\n") + run("delete", "einkaufsliste", "--as", "marge") + assert archive.exists() + + def test_a_list_that_does_not_exist_is_refused_with_the_lists(self, run, groceries): + assert run("delete", "baumarkt", "--as", "marge")["error"] == \ + "no list 'baumarkt'. Lists: einkaufsliste" + + class TestTheHistory: def test_a_long_change_counts_its_items_in_the_subject(self, run, store, groceries): names = ["Hafermilch ohne Zucker", "Vollkornbrot vom Bäcker", "Bio-Joghurt Natur"]