From e71edeb3498455905098d3f36972774068c596c4 Mon Sep 17 00:00:00 2001 From: Test Date: Mon, 12 Oct 2026 18:14:00 +0200 Subject: [PATCH 1/3] feat(memory): delete a list with `stack memory list delete` A list nobody needs any more could only be emptied item by item, which left an empty page behind in the wiki and in every list overview. `stack memory list delete --as ` removes the list in one commit authored by that person. A list that still has open items is refused with those items named, unless `--yes` is given, so whoever asked can be asked whether they go too. The list's archive stays: it is the record of what was bought, and a list started again under the same name finds its known items there. The Forgejo read-modify-write now deletes the file when the transform answers `None`, against the hash it was read at, so a deletion goes through the same write path as every other list change. --- lib/stack/forgejo.py | 17 +++++- stacklets/memory/cli/list.py | 48 +++++++++++++-- stacklets/memory/lib.py | 4 +- .../unit/framework/test_forgejo_edit_file.py | 57 ++++++++++++++++++ tests/unit/stacklets/test_memory_list_cli.py | 59 +++++++++++++++++-- 5 files changed, 171 insertions(+), 14 deletions(-) create mode 100644 tests/unit/framework/test_forgejo_edit_file.py 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/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..82947ac4 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 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/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"] From 521cbededfa90723f74997ad7ce1553fe2fcee38 Mon Sep 17 00:00:00 2001 From: Test Date: Mon, 12 Oct 2026 18:14:00 +0200 Subject: [PATCH 2/3] refactor(memory): pass an empty hash, never None, when moving old todos The move from `todos.md` to lists read a file's hash with `.get`, which the type checker reads as possibly `None` where the file change wants a string. Forgejo always answers with a hash for a file it returned. No behaviour change. --- stacklets/memory/lib.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stacklets/memory/lib.py b/stacklets/memory/lib.py index 82947ac4..c9649574 100644 --- a/stacklets/memory/lib.py +++ b/stacklets/memory/lib.py @@ -2473,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( From fa2e1e25c31f94e685f69b94496d9b296c409871 Mon Sep 17 00:00:00 2001 From: Test Date: Mon, 12 Oct 2026 18:14:01 +0200 Subject: [PATCH 3/3] feat(agent): delete a list when the family asks "We don't need the hardware store list any more" made Stacky remove every item one by one, and then explain that the page itself could not be removed. `list_edit` gains `op=delete`. The store refuses a list with open items, naming them, and Stacky asks the person; only their yes sends `confirmed`, which becomes `--yes`. The user guide says how. --- docs/user-guide.md | 3 ++- stacklets/agent/runtime/list_tool.py | 20 ++++++++++++++--- .../workspace/skills/family-memory/SKILL.md | 2 ++ tests/unit/stacklets/conftest.py | 1 + .../unit/stacklets/test_agent_vault_tools.py | 22 +++++++++++++++++++ 5 files changed, 44 insertions(+), 4 deletions(-) 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/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/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.